feat(dispatcher): support cross-repo deps in check_deps_resolved (closes #157)#160
Conversation
#157) `check_deps_resolved` greedy-extracted every `#NNN` between `## Dependencies` and the next `## ` heading via `grep -oE '#[0-9]+'`, then queried each in $REPO. Two related bugs fell out: 1. `owner/repo#NNN` cross-repo refs collapsed to bare `NNN` and were looked up in the wrong repo. When the lookup failed, `|| true` swallowed the error, $state was empty, the != "CLOSED" check was true, and the issue was silently blocked forever. 2. The same greedy match swept up `#NNN` tokens inside prose, blockquotes, and inline references — same silent-block path. Fix: - Restrict extraction to list-item lines (`-`, `*`, `1.`) only. - Recognize two ref shapes per item: `owner/repo#N` (cross-repo, longer- first match-and-strip) and bare `#N` (same-repo). Both require a left boundary so URL fragments and inline punctuation don't misparse. - On lookup failure (empty state from `gh issue view`), fail-safe block AND emit a stderr warning naming the failed `<repo>#N`. Without the warning, a typo would silently recreate the original bug class. Documentation: - New INV-39 in invariants.md with the parsing rule + lookup-failure semantics; INV-11 cross-references it. - dispatcher-flow.md Step 2 updated to reflect list-only + cross-repo. - create-issue SKILL.md and templates document the user-facing rules. Tests (tests/unit/test-check-deps-resolved.sh, 20 cases): - All existing #61/#73 cases still pass. - Cross-repo CLOSED/MERGED/OPEN; same-repo + cross-repo mixed. - Same number, different repos resolves independently. - Prose, blockquote, URL-fragment refs ignored. - Numbered list extraction. - Lookup-failure blocks AND emits the stderr warning.
![amazon-q-developer[bot]](https://avatars.githubusercontent.com/in/1220912?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR successfully implements cross-repo dependency support and eliminates false positives from prose/blockquotes. The implementation is well-designed and thoroughly tested.
Key strengths:
- Two-stage parsing correctly restricts to list items only, eliminating false positives from prose/blockquotes
- Longest-first matching prevents double-counting of
owner/repo#Nrefs - Comprehensive test coverage (20/20 tests passing) including cross-repo, mixed dependencies, and edge cases
- Proper error handling with stderr warnings for lookup failures
- Documentation updated with new INV-39 invariant and cross-references
Implementation verified:
- Stage 1 filters to list-item lines only using
grep -E '^[[:space:]]*([-*]|[0-9]+\.)[[:space:]]' - Stage 2a matches cross-repo refs first with left boundary protection
- Stage 2b matches bare
#Nrefs on the residue - Both stages correctly check
state ∉ {CLOSED, MERGED}per INV-11 - Failed lookups emit observable warnings and block dispatch (fail-safe)
All acceptance criteria from #157 are met. No defects found.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
…#157) Address agy review on PR #160: 1. Restore `(` to the left-boundary anchor in both Stage 2a (cross-repo) and Stage 2b (bare #N) regexes — `(^|[[:space:]\(])` matches the approved spec at docs/superpowers/specs/2026-05-27-cross-repo-deps-design.md. Without `(`, parenthesized refs like `- (owner/repo#42)` and `- (#7)` were silently ignored. 2. Strengthen the URL-fragment regression test to put the URL on a list-item line so Stage 2's boundary logic actually runs (the previous prose-only variant was filtered out at Stage 1). Register competing OPEN states for the would-be-extracted numbers so a future greedy- extraction regression would flip the test to BLOCKED. 3. Add explicit parenthesized-ref tests covering both CLOSED → resolved and OPEN → blocked. INV-39 updated to document the `(`-boundary in both the rule statement and the regex citation. Tests: 23/23 PASS in test-check-deps-resolved.sh.
agy reviewVerdict: COMMENT (now addressed in Findings
Confirmed (no change needed)
Suggestions not adopted
Test count: 23/23 PASS in |
agy review (second pass — verdict: ✅ APPROVE)All three findings from the prior review confirmed resolved by
Regression checks
23/23 tests pass. No new findings. |
1 participant
Summary
check_deps_resolvedtwo-bug class (feat(dispatcher): support cross-repo dependency references in check_deps_resolved #157): cross-repoowner/repo#Nrefs were collapsing to bareNqueried in the wrong repo, and prose/blockquote#Nmentions were greedy-extracted — both ending in the same silent-block-forever path.owner/repo#N(cross-repo) and#N(same-repo) with left-boundary anchors so URL fragments and inline punctuation don't misparse.$statefromgh issue view), fail-safe block AND emit a stderr warning naming the failed ref — without that, a typo would silently recreate the original bug class.Spec & docs
docs/superpowers/specs/2026-05-27-cross-repo-deps-design.mdstate ∉ {CLOSED, MERGED}rule applies to both same-repo and cross-repo refs).docs/pipeline/dispatcher-flow.mdStep 2 description updated.skills/create-issue/SKILL.mdandreferences/issue-templates.mddocument list-item scope +owner/repo#Nsyntax.Test plan
#61/#73regression cases continue to pass (no behavior change for bare-#N-on-list-item).owner/repo#N: CLOSED/MERGED → resolved; OPEN → blocked.#42OPEN in this repo +other/other#42CLOSED → resolved).#Nbetween headings → ignored (was the silent-block bug).> ... owner/repo#N ...→ ignored.https://github.com/.../issues/123→ ignored.1./2.) → extracted correctly.gh issue view(404 / unauthorized) → blocks AND emits stderr warning naming the failed ref.bash tests/unit/test-check-deps-resolved.sh— 20/20 PASS.bash tests/unit/test-create-issue-dependencies-guidance.sh— 10/10 PASS.bash tests/unit/test-lib-dispatch.sh— 21/21 PASS.Closes #157.