fix: guard cross-repo issue references #63

Merged
codex-bot-andresmgsl merged 2 commits from build/61-cross-repo-reference-guards into main 2026-07-23 12:24:22 +00:00
codex-bot-andresmgsl commented 2026-07-23 11:38:39 +00:00 (Migrated from github.com)

Closes #61

Summary

  • classify local and repository-qualified issue references through one shared helper
  • prevent cross-repo blockers from being promoted automatically and flag them for triage
  • keep sibling-repo status references out of epic child resolution
  • document the manual triage path and record the behavior change

Worklog

  • Extract one shared local/cross-repo reference helper and route both parsers through it.
  • Add FLAG_CROSS_REPO precedence and the blocked-cross-repo API-edge comment.
  • Pin the specified reference shapes, complete real epic #1 task-list corpus, and blocker decisions with contract tests.
  • Prove the blocked-cross-repo comment is idempotent across two sweeps.
  • Re-run parsing against the complete live epic #1 body and record the exact local reference set.
  • Add the TRIAGE.md cross-repo dependency rule.
  • Add the CHANGELOG.md entry under ## Unreleased.
  • Run the focused issueflow tests and the full contract suite.
  • Run shellcheck/actionlint and complete final diff review.

Acceptance criteria

  • epic_references on the real #1 task list omits #112 and retains every local reference.
  • rig#112 and heavy-duty/box#9 drop; (#13), #14/#15, and #11, #12 survive.
  • Cross-repo-only blockers decide FLAG_CROSS_REPO.
  • Closed local blockers mixed with a cross-repo blocker decide FLAG_CROSS_REPO.
  • Existing local-only issueflow behavior remains green.
  • The marked cross-repo comment is idempotent across two sweeps.
  • TRIAGE.md carries the cross-repo rule.

Verification

  • bash test/issueflow-reconcile.test.sh — 50 passed, 0 failed
  • bash test/run.sh — 13 test files passed, 0 failed
  • bash .github/scripts/shellcheck-all.sh — 29 tracked scripts clean
  • bash .github/scripts/actionlint-all.sh — 6 workflows clean
  • live #1 body through epic_references2–16, 18, 19, 23, 24, 30, 32, 50, 55–58, 61; no phantom 112
  • read-only DRY_RUN=1 sweep completed against the live repository
  • GitHub checks — all required jobs green on 3935bf8
Closes #61 ## Summary - classify local and repository-qualified issue references through one shared helper - prevent cross-repo blockers from being promoted automatically and flag them for triage - keep sibling-repo status references out of epic child resolution - document the manual triage path and record the behavior change ## Worklog - [x] Extract one shared local/cross-repo reference helper and route both parsers through it. - [x] Add `FLAG_CROSS_REPO` precedence and the `blocked-cross-repo` API-edge comment. - [x] Pin the specified reference shapes, complete real epic #1 task-list corpus, and blocker decisions with contract tests. - [x] Prove the `blocked-cross-repo` comment is idempotent across two sweeps. - [x] Re-run parsing against the complete live epic #1 body and record the exact local reference set. - [x] Add the TRIAGE.md cross-repo dependency rule. - [x] Add the CHANGELOG.md entry under `## Unreleased`. - [x] Run the focused issueflow tests and the full contract suite. - [x] Run shellcheck/actionlint and complete final diff review. ## Acceptance criteria - [x] `epic_references` on the real #1 task list omits `#112` and retains every local reference. - [x] `rig#112` and `heavy-duty/box#9` drop; `(#13)`, `#14/#15`, and `#11, #12` survive. - [x] Cross-repo-only blockers decide `FLAG_CROSS_REPO`. - [x] Closed local blockers mixed with a cross-repo blocker decide `FLAG_CROSS_REPO`. - [x] Existing local-only issueflow behavior remains green. - [x] The marked cross-repo comment is idempotent across two sweeps. - [x] TRIAGE.md carries the cross-repo rule. ## Verification - `bash test/issueflow-reconcile.test.sh` — 50 passed, 0 failed - `bash test/run.sh` — 13 test files passed, 0 failed - `bash .github/scripts/shellcheck-all.sh` — 29 tracked scripts clean - `bash .github/scripts/actionlint-all.sh` — 6 workflows clean - live #1 body through `epic_references` — `2–16, 18, 19, 23, 24, 30, 32, 50, 55–58, 61`; no phantom `112` - read-only `DRY_RUN=1` sweep completed against the live repository - GitHub checks — all required jobs green on `3935bf8`
danmt (Migrated from github.com) reviewed 2026-07-23 11:38:39 +00:00
grok-bot-andresmgsl commented 2026-07-23 11:45:37 +00:00 (Migrated from github.com)

🔎 reviewing head 3935bf82d9

🔎 reviewing head 3935bf82d9a76e03a28ac8954c77f1a973d4343f
kimi-bot-andresmgsl commented 2026-07-23 11:46:44 +00:00 (Migrated from github.com)

🔎 reviewing head 3935bf82d9

🔎 reviewing head 3935bf82d9a76e03a28ac8954c77f1a973d4343f
grok-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-23 11:46:51 +00:00
grok-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: approve

Reviewed against #61 (acceptance criteria + D1–D5) at head 3935bf82d9a76e03a28ac8954c77f1a973d4343f.

Spec / acceptance

Criterion Result
Shared local/cross helper, both parsers routed through it (D1) issue_references + blocked_reference_records / epic_references
rig#112 / heavy-duty/box#9 drop; (#13), #14/#15, #11, #12 survive (D2) contract tests + manual re-run
Cross-repo-only → FLAG_CROSS_REPO, not READY / FLAG_UNPARSEABLE (D3) pinned
Closed locals + cross-repo → FLAG_CROSS_REPO (false-promotion pin) pinned
Local-only decisions unchanged (D5) existing suite still green
blocked-cross-repo comment idempotent across two sweeps pinned via ensure_comment marker
TRIAGE.md cross-repo hand-flip rule present under Dependencies
CHANGELOG under ## Unreleased present, cites #61

Live epic #1 body through epic_references: 2–16, 18, 19, 23, 24, 30, 32, 50, 55–58, 61 — no phantom 112. Cross-repo link token classifies as CROSS\trig#112.

Correctness notes

  • FLAG_CROSS_REPO is checked before empty-refs / READY, so a declaration that only names siblings cannot promote and is not misdiagnosed as unparseable.
  • API edge only comments on FLAG_CROSS_REPO (no label flip) — matches “triage flips by hand.”
  • Epic path drops CROSS and leaves empty-refs → KEEP behavior intact.

Verification (this review)

  • bash test/issueflow-reconcile.test.sh — 50 passed, 0 failed
  • bash test/run.sh — 13 test files passed, 0 failed
  • GitHub checks on this head — all required jobs green

Non-blocking

None that gate merge. Optional later polish: blocked_references and blocked_cross_references each re-walk the body; a single parse into locals + cross would be slightly cheaper, not required by the issue.

Meets every acceptance row of #61. Approve.

## Verdict: approve Reviewed against #61 (acceptance criteria + D1–D5) at head `3935bf82d9a76e03a28ac8954c77f1a973d4343f`. ### Spec / acceptance | Criterion | Result | |---|---| | Shared local/cross helper, both parsers routed through it (D1) | `issue_references` + `blocked_reference_records` / `epic_references` | | `rig#112` / `heavy-duty/box#9` drop; `(#13)`, `#14/#15`, `#11, #12` survive (D2) | contract tests + manual re-run | | Cross-repo-only → `FLAG_CROSS_REPO`, not READY / FLAG_UNPARSEABLE (D3) | pinned | | Closed locals + cross-repo → `FLAG_CROSS_REPO` (false-promotion pin) | pinned | | Local-only decisions unchanged (D5) | existing suite still green | | `blocked-cross-repo` comment idempotent across two sweeps | pinned via `ensure_comment` marker | | TRIAGE.md cross-repo hand-flip rule | present under Dependencies | | CHANGELOG under `## Unreleased` | present, cites #61 | Live epic #1 body through `epic_references`: `2–16, 18, 19, 23, 24, 30, 32, 50, 55–58, 61` — no phantom `112`. Cross-repo link token classifies as `CROSS\trig#112`. ### Correctness notes - `FLAG_CROSS_REPO` is checked before empty-refs / READY, so a declaration that only names siblings cannot promote and is not misdiagnosed as unparseable. - API edge only comments on `FLAG_CROSS_REPO` (no label flip) — matches “triage flips by hand.” - Epic path drops CROSS and leaves empty-refs → KEEP behavior intact. ### Verification (this review) - `bash test/issueflow-reconcile.test.sh` — 50 passed, 0 failed - `bash test/run.sh` — 13 test files passed, 0 failed - GitHub checks on this head — all required jobs green ### Non-blocking None that gate merge. Optional later polish: `blocked_references` and `blocked_cross_references` each re-walk the body; a single parse into locals + cross would be slightly cheaper, not required by the issue. Meets every acceptance row of #61. Approve.
kimi-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-23 11:50:25 +00:00
kimi-bot-andresmgsl (Migrated from github.com) left a comment

Approve — head 3935bf82d9a76e03a28ac8954c77f1a973d4343f.

Reviewed against issue #61's spec and acceptance criteria; all seven hold, verified by running rather than reading:

  • D1 (one shared helper). issue_references (actions/issueflow-reconcile/issueflow-reconcile.sh:108) classifies the whole token before number extraction; both blocked_references and epic_references route through it. Two parsers, one regex — the drift class is closed.
  • D2 (reference shapes). Probed the helper directly: rig#112, heavy-duty/box#9, and [rig#112](…) classify CROSS; (#13), #14/#15, #11, #12 classify LOCAL. Also confirmed a PR-URL with a #issuecomment-… fragment yields no phantom reference, and #10's possessive prose still parses local. Each spec'd shape is also pinned by its own test case.
  • D3 (never promote). FLAG_CROSS_REPO takes precedence over both READY and FLAG_UNPARSEABLE, with the all-local-closed-plus-cross-repo false-promotion case pinned explicitly — and the blocked-cross-repo comment is a distinct diagnosis, not the unparseable one.
  • D4 (epic status citations). The real epic #1 task-list corpus is pinned verbatim as a fixture and yields exactly the local children 2–16, 18, 19, 23, 24, 30, 32, 50, 55–58, 61#112 absent, nothing else lost.
  • D5 (no local-only change). The pre-existing test lines are untouched (+38 −0 on the test file); the full issueflow suite passes unmodified: bash test/issueflow-reconcile.test.sh → 50 passed, 0 failed.
  • Idempotency. The two-sweep blocked-cross-repo case passes against a fake gh; the marker mechanism is the same ensure_comment used by every other warning.
  • TRIAGE.md + CHANGELOG.md. The cross-repo rule sits in the issue contract's Dependencies bullet, and the changelog line is inserted at the top of ## Unreleased per the house convention.

Suite caveat, not caused by this PR: bash test/run.sh shows 5 failing test files, all package-json cases, and they fail identically on a clean main checkout — this environment has no node/npm, and the affected code paths are untouched by this diff. shellcheck/actionlint binaries are likewise absent locally, so those two verification lines I take from the PR's report plus CI.

One non-blocking observation: FLAG_CROSS_REPO also fires when local blockers are still OPEN, so an issue that is legitimately blocked on open locals and names a cross-repo dep gets the "triage flips by hand" comment a little early. It is one idempotent comment and the advice is not wrong; the builder may consider gating the flag behind "locals resolved" if the noise ever matters.

Ship it.

**Approve** — head `3935bf82d9a76e03a28ac8954c77f1a973d4343f`. Reviewed against issue #61's spec and acceptance criteria; all seven hold, verified by running rather than reading: - **D1 (one shared helper).** `issue_references` (actions/issueflow-reconcile/issueflow-reconcile.sh:108) classifies the whole token before number extraction; both `blocked_references` and `epic_references` route through it. Two parsers, one regex — the drift class is closed. - **D2 (reference shapes).** Probed the helper directly: `rig#112`, `heavy-duty/box#9`, and `[rig#112](…)` classify CROSS; `(#13)`, `#14/#15`, `#11, #12` classify LOCAL. Also confirmed a PR-URL with a `#issuecomment-…` fragment yields no phantom reference, and `#10's` possessive prose still parses local. Each spec'd shape is also pinned by its own test case. - **D3 (never promote).** `FLAG_CROSS_REPO` takes precedence over both `READY` and `FLAG_UNPARSEABLE`, with the all-local-closed-plus-cross-repo false-promotion case pinned explicitly — and the `blocked-cross-repo` comment is a distinct diagnosis, not the unparseable one. - **D4 (epic status citations).** The real epic #1 task-list corpus is pinned verbatim as a fixture and yields exactly the local children `2–16, 18, 19, 23, 24, 30, 32, 50, 55–58, 61` — `#112` absent, nothing else lost. - **D5 (no local-only change).** The pre-existing test lines are untouched (`+38 −0` on the test file); the full issueflow suite passes unmodified: `bash test/issueflow-reconcile.test.sh` → 50 passed, 0 failed. - **Idempotency.** The two-sweep `blocked-cross-repo` case passes against a fake `gh`; the marker mechanism is the same `ensure_comment` used by every other warning. - **TRIAGE.md + CHANGELOG.md.** The cross-repo rule sits in the issue contract's Dependencies bullet, and the changelog line is inserted at the top of `## Unreleased` per the house convention. Suite caveat, not caused by this PR: `bash test/run.sh` shows 5 failing test files, all `package-json` cases, and they fail identically on a clean `main` checkout — this environment has no node/npm, and the affected code paths are untouched by this diff. shellcheck/actionlint binaries are likewise absent locally, so those two verification lines I take from the PR's report plus CI. One non-blocking observation: `FLAG_CROSS_REPO` also fires when local blockers are still OPEN, so an issue that is legitimately blocked on open locals *and* names a cross-repo dep gets the "triage flips by hand" comment a little early. It is one idempotent comment and the advice is not wrong; the builder may consider gating the flag behind "locals resolved" if the noise ever matters. Ship it.
claude-bot-andresmgsl commented 2026-07-23 12:10:45 +00:00 (Migrated from github.com)

🔎 reviewing head 3935bf82d9

🔎 reviewing head 3935bf82d9a76e03a28ac8954c77f1a973d4343f
claude-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-23 12:14:04 +00:00
claude-bot-andresmgsl (Migrated from github.com) left a comment

Approve — head 3935bf8.

I verified rather than opined: ran the suites, probed the classifier with adversarial inputs, and re-ran the parser against the live epic body.

What I ran and saw:

  • bash test/issueflow-reconcile.test.sh — 50 passed, 0 failed. bash test/run.sh — 13 files passed. .github/scripts/shellcheck-all.sh clean. All required checks green on 3935bf8.
  • The test diff is purely additive (zero removed/modified lines), so D5 is pinned exactly as specified: every pre-existing case passes unmodified.
  • Live corpus: I fetched the current #1 body (triage has since edited it — rig#112 merged) and ran it through the new epic_references: 2–16, 18, 19, 23, 24, 30, 32, 50, 55–58, 61 — no phantom 112, matching the PR's recorded verification even on the newer body. The fixture carries the PR [rig#112](…) row verbatim per the corpus rule.
  • Shape probes beyond the pinned five all classify correctly: [rig#112](url) in a link label → CROSS; (heavy-duty/box#9), #11,#12 → cross dropped, locals kept; Blocked by rig#9 (the false-promotion body from the issue) → FLAG_CROSS_REPO, never READY; a URL fragment like #issuecomment-99 matches nothing. The awk is mawk-safe (index/substr, no escapes).
  • D1–D4 hold structurally: one shared issue_references helper feeds both parsers; FLAG_CROSS_REPO takes precedence in blocked_decision before the empty-refs check, so a cross-only declaration can never be misdiagnosed FLAG_UNPARSEABLE; the third argument defaults empty so existing two-arg calls are untouched. The comment interpolation is injection-safe by construction — the token regex admits no backticks, quotes, or $.

Non-blocking observations (no action required for this PR):

  1. The pre-existing sentence scanner in blocked_reference_records truncates at the first . — which a markdown link's URL always contains. Blocked by [rig#112](https://github.com/…) and #14. therefore drops the trailing local #14. The decision is still FLAG_CROSS_REPO (conservative, stays blocked), and this is strictly better than pre-PR behavior, which read a phantom local #112 from the same shape. If link-wrapped refs in Blocked by prose ever become house style, the truncation deserves its own discussion — it predates this PR and is out of scope here.
  2. When cross_refs is non-empty, reference_states is still called on the local refs though the result is unused — a few wasted API calls per sweep, harmless.
  3. A degenerate a/b/c#9 classifies CROSS (correct) but the comment names it as b/c#9 — cosmetic, unreachable with real GitHub owner/repo names.

Every acceptance criterion on #61 is met and pinned by a test that would fail without the fix. Ship-worthy; over to the human once the panel converges.

**Approve** — head `3935bf8`. I verified rather than opined: ran the suites, probed the classifier with adversarial inputs, and re-ran the parser against the live epic body. **What I ran and saw:** - `bash test/issueflow-reconcile.test.sh` — 50 passed, 0 failed. `bash test/run.sh` — 13 files passed. `.github/scripts/shellcheck-all.sh` clean. All required checks green on `3935bf8`. - The test diff is **purely additive** (zero removed/modified lines), so D5 is pinned exactly as specified: every pre-existing case passes unmodified. - Live corpus: I fetched the current #1 body (triage has since edited it — rig#112 merged) and ran it through the new `epic_references`: `2–16, 18, 19, 23, 24, 30, 32, 50, 55–58, 61` — no phantom `112`, matching the PR's recorded verification even on the newer body. The fixture carries the `PR [rig#112](…)` row verbatim per the corpus rule. - Shape probes beyond the pinned five all classify correctly: `[rig#112](url)` in a link label → CROSS; `(heavy-duty/box#9), #11,#12` → cross dropped, locals kept; `Blocked by rig#9` (the false-promotion body from the issue) → `FLAG_CROSS_REPO`, never `READY`; a URL fragment like `#issuecomment-99` matches nothing. The awk is mawk-safe (`index`/`substr`, no escapes). - D1–D4 hold structurally: one shared `issue_references` helper feeds both parsers; `FLAG_CROSS_REPO` takes precedence in `blocked_decision` before the empty-refs check, so a cross-only declaration can never be misdiagnosed `FLAG_UNPARSEABLE`; the third argument defaults empty so existing two-arg calls are untouched. The comment interpolation is injection-safe by construction — the token regex admits no backticks, quotes, or `$`. **Non-blocking observations** (no action required for this PR): 1. The pre-existing sentence scanner in `blocked_reference_records` truncates at the first `.` — which a markdown link's URL always contains. `Blocked by [rig#112](https://github.com/…) and #14.` therefore drops the trailing local `#14`. The decision is still `FLAG_CROSS_REPO` (conservative, stays blocked), and this is strictly better than pre-PR behavior, which read a phantom local `#112` from the same shape. If link-wrapped refs in `Blocked by` prose ever become house style, the truncation deserves its own discussion — it predates this PR and is out of scope here. 2. When `cross_refs` is non-empty, `reference_states` is still called on the local refs though the result is unused — a few wasted API calls per sweep, harmless. 3. A degenerate `a/b/c#9` classifies CROSS (correct) but the comment names it as `b/c#9` — cosmetic, unreachable with real GitHub owner/repo names. Every acceptance criterion on #61 is met and pinned by a test that would fail without the fix. Ship-worthy; over to the human once the panel converges.
codex-bot-andresmgsl commented 2026-07-23 12:16:20 +00:00 (Migrated from github.com)

Closing round summary

What shipped

  • One shared local/cross-repository issue-reference classifier now feeds both blocker and epic parsing.
  • Cross-repo blockers can no longer promote an issue automatically: reconciliation returns FLAG_CROSS_REPO and posts the idempotent blocked-cross-repo triage comment.
  • Epic child resolution drops sibling-repo status references while preserving local reference forms.
  • The manual triage rule is documented in TRIAGE.md, and the behavior change is recorded under CHANGELOG.mdUnreleased.

Rounds

  • Round 1 (3935bf8): shipped the implementation and additive contract coverage. Grok, Kimi, and Claude each approved this exact head. No corrective commit was required after review; all observations were explicitly non-blocking.

Verified

  • bash test/issueflow-reconcile.test.sh — 50 passed, 0 failed
  • bash test/run.sh — 13 test files passed, 0 failed in the builder environment and CI
  • shellcheck — 29 tracked scripts clean
  • actionlint — 6 workflows clean
  • live epic #1 parsing — local refs 2–16, 18, 19, 23, 24, 30, 32, 50, 55–58, 61; no phantom 112
  • read-only DRY_RUN=1 sweep against the live repository
  • all required GitHub checks green on 3935bf8; PR reports clean and mergeable

Post-merge residue

No merge blocker remains. Reviewers recorded optional follow-up observations only: the blocker parser's pre-existing period-based sentence truncation around Markdown URLs; duplicate body walks / avoidable local-state API calls when cross-repo refs already determine the outcome; potentially early but harmless FLAG_CROSS_REPO signaling when open local blockers coexist; and cosmetic rendering of degenerate multi-slash repository tokens. Per scope discipline, none was folded into #61.

## Closing round summary ### What shipped - One shared local/cross-repository issue-reference classifier now feeds both blocker and epic parsing. - Cross-repo blockers can no longer promote an issue automatically: reconciliation returns `FLAG_CROSS_REPO` and posts the idempotent `blocked-cross-repo` triage comment. - Epic child resolution drops sibling-repo status references while preserving local reference forms. - The manual triage rule is documented in `TRIAGE.md`, and the behavior change is recorded under `CHANGELOG.md` → `Unreleased`. ### Rounds - **Round 1 (`3935bf8`)**: shipped the implementation and additive contract coverage. Grok, Kimi, and Claude each approved this exact head. No corrective commit was required after review; all observations were explicitly non-blocking. ### Verified - `bash test/issueflow-reconcile.test.sh` — 50 passed, 0 failed - `bash test/run.sh` — 13 test files passed, 0 failed in the builder environment and CI - shellcheck — 29 tracked scripts clean - actionlint — 6 workflows clean - live epic #1 parsing — local refs `2–16, 18, 19, 23, 24, 30, 32, 50, 55–58, 61`; no phantom `112` - read-only `DRY_RUN=1` sweep against the live repository - all required GitHub checks green on `3935bf8`; PR reports clean and mergeable ### Post-merge residue No merge blocker remains. Reviewers recorded optional follow-up observations only: the blocker parser's pre-existing period-based sentence truncation around Markdown URLs; duplicate body walks / avoidable local-state API calls when cross-repo refs already determine the outcome; potentially early but harmless `FLAG_CROSS_REPO` signaling when open local blockers coexist; and cosmetic rendering of degenerate multi-slash repository tokens. Per scope discipline, none was folded into #61.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: heavy-duty/ceremony#63
No description provided.