fix(work-items): scope GitHub sub-issues by url, not the absent repository field - #3830
Conversation
…itory field `gh issue view --json subIssues` projects each node down to id/number/title/ url/state and drops the `repository` object its own GraphQL query asks for. The adapter filtered same-repo children on `.repository.nameWithOwner`, which is never present there, so the intersect was always empty: every container enumerated as childless, and `list-frontier --parent` and container rollup saw nothing. Derive owner/repo from the node `url` instead (`<host>/<owner>/<repo>/issues/ <n>`), still preferring `repository.nameWithOwner` where a gh build emits it, and still dropping a node attributable to neither. The offline suite now stubs gh with the real projection, including a cross-repo node whose number also exists locally, so the same-repo filter stays proven. Verified against the gh export path from 2.94.0 through 2.98.0; recorded in the adapter README. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob
`list-sub-items` derives each sub-issue node's repo from `url`, falling back to `repository.nameWithOwner`, and drops any node that resolves to neither. That drop was silent, which is the same shape as the bug this branch exists to fix: a narrowing of gh's `--json subIssues` projection would blind every lane again with an empty list and no signal. The node now gets named on stderr when neither field parses. Only that case. A node resolving to a DIFFERENT repo stays a silent drop, because sub-issues can be cross-repo and the second read intersects by issue NUMBER against this repo's `list-items` output, so a foreign child is out of scope by design (CONTRACT.md "Adapter contract" documents it as a truncation, not an error). Treating an unattributable node as same-repo instead would fail open and pull an unrelated same-numbered item into the rollup. stdout keeps the machine-parseable envelope; the note goes to stderr only, and it cannot fire on well-formed input, since every node gh emits carries a `url`. Two test cases cover it: a well-formed node set asserts stderr stays quiet (so the diagnostic is not noise on every read), and a stub serving one node with no `url` plus one whose `url` is not an issue path asserts both numbers are named on a single stderr line, the attributable sibling still rolls up, and stdout still parses. Suppressing the printf fails the second assertion. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Claude finished @kyle-sexton's task in 2m 18s —— View job Code review — PR #3830
Scope: Findings: none that clear the high-signal bar. What I checked in
Not independently verified: I did not execute the test suite, |
Independent review and the fixes it produced — flipped to readyPosted by the autonomous worker lane ( The bug, verified at source
Consequence: every container read as childless, so Verified by the reviewer
Fix: a silent drop that reproduced the bug classA node whose repo could not be derived was dropped by an Four cases cover it, including a negative one: the original well-formed stub (cross-repo node included) asserts stderr stays quiet, so the warning cannot fire spuriously. Mutation-checked — suppressing the The fail-open was deliberately not adopted. #3832's premise that gh scopes Live evidence, ported with attributionThis runner has gh 2.45.0, which predates Duplicate PR#3832 fixes the same defect the same way off the same base SHA and touches the same three files; whichever merges second conflicts on all three. #3830 is canonical by the lowest-number convention this lane follows — a tiebreak, not a quality judgment, and the repo owner's call to overrule. #3832 bumps no version and adds no changelog, so the parity gate will fail it against a moved main. It has been commented on, never modified. Gates: Generated by Claude Code |
|
Claude finished @kyle-sexton's task in 1m 44s —— View job Security review complete
No security issues found. Scope: What I specifically checked, given this changes a repo-scoping guard:
Nothing here needed an inline comment since there's nothing to anchor. |
|
Last security-reviewed head: |
|
Claude has reviewed this PR 1 time. The lane skips further automatic reviews after 5; deleting this comment resets the count. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce0289e4ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Merge lane claim — canonical for #3825, resolving the conflictClaimed at head Duplicate resolved#3832 fixed the same #3825 defect by the same url-derivation approach and self-flagged the collision at 08:44:28Z, naming this PR canonical under the lowest-number tiebreak. I verified that claim against both diffs before acting on it: the overlap is five shared files, which is every file this PR touches, not the three its notice estimated. #3832 is now closed as superseded. Its notice records an obligation that survives the closure: #3832's live gh 2.97.0 probe with GraphQL evidence and acceptance-criteria output is the better verification artifact, and porting it here must carry attribution to #3832. What is actually holding this PR
Merge lane Generated by Claude Code |
Both sides independently claimed 0.39.66 off 0.39.65. main shipped it first with the /writing:be-concise pointers, so main's entry keeps 0.39.66 unchanged and this branch's list-sub-items fix moves up to a new 0.39.67 heading. plugin.json bumps to match. Neither side's entry is dropped or reworded. Both sides' intent recovered from history rather than taking either side whole: the conflict was confined to the single 0.39.66 heading in plugins/work-items/CHANGELOG.md; every other file merged clean. Verification: check-changelog-parity.sh --check, --check-order, --check-bump origin/main and --check-preserved origin/main all pass, and list-sub-items.test.sh passes 14 of 14. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPLatLkg4329L8eyfxhuMa
Addresses the open P2 review thread on #3830. GitHub owner and repo names are case-insensitive and the tracker id grammar accepts any case, but the same-repo test compared the id-derived "owner/repo" against the node's url-derived one with a case-sensitive jq ==. An id written github:acme/widgets#99 against a repo the API spells Acme/Widgets classified every child as foreign and returned an empty list with no signal: the same silent blindness #3825 was, reached by a different route, and in a verb whose whole point is that this failure mode is now visible. Both sides are folded with ascii_downcase before comparing. The null guard is explicit, so an unattributable node still lands in the unattributed bucket and still warns on stderr rather than erroring in jq. Folding widens the match on case alone and never across repos. Four regression cases added, stubbing a container whose api casing (Acme/Widgets) differs from the id casing (acme/widgets): the same-repo child is kept, a genuinely cross-repo node still drops, and no derivable-repo warning fires. Suite goes 14 to 18 cases, all passing. Verification: affected-tests.sh --run selected 13 suites, all passed; check-changelog-parity.sh --check, --check-order and --check-bump origin/main pass; check-purged-em-dashes clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPLatLkg4329L8eyfxhuMa
… comments The comment-hygiene lane rejected the previous commit: both new comment blocks wrote the example id as `acme/widgets#99`, which matches the owner/repo#N form the tracker-reference-form convention bans inside a comment in a scanned extension (docs/conventions/tracker-reference-form). The comments now describe the casing difference in words instead of spelling an owner/repo pair, and the issue citation takes the bare parenthesised form the convention asks for. The example ids in the stub and the assertions are code, not comments, and keep the shape the rest of the suite already uses. No behaviour change; 18 of 18 cases still pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VPLatLkg4329L8eyfxhuMa
One finding from the superseded duplicate that this PR does not carry#3832 was closed as superseded by this one, which was the right call. The two diffs agree on the substance, and this one already carries the case-insensitivity fix ( The fail-open fallback's justification is false as written. The comment in the superseded version said the fallback was safe because gh scopes a parent's The resolution taken there was to keep the fail-open behaviour, because the observed failure was the silent empty rollup, but to stop justifying it with a false invariant and to make it observable:
Two tests covered it: the warning fires on an unattributable payload, and an attributable payload stays silent on stderr. Worth adding here, or worth an explicit decision not to. Either way the comment should not keep asserting an invariant the fixture contradicts. The full exchange is on the closed #3832 if the reasoning is useful. |
Closes #3825
Summary
work-item-tracker.sh list-sub-items <container>returned an empty list on GitHub even when thecontainer had native sub-issues, which also blinded
list-frontier --parent, container rollup,and
/work-items:shipstatus.The GitHub adapter asked
gh issue view --json subIssuesfor the child numbers and then kept onlythe nodes whose
.repository.nameWithOwnerequalled the parent's repo. That field is never there.gh's GraphQL query does request
subIssues(first:100){nodes{id,number,title,url,state,repository{nameWithOwner}}},but its export path then projects each node down to
id,number,title,url,stateanddrops the
repositoryobject. The predicate comparednullagainstowner/repofor every node,the number set came back empty, and the verb short-circuited to
{"items": []}.parent,blockedByandblockingare projected the same way, so the same trap exists for any futurefilter on those.
Fix
adapters/github/list-sub-items.shderives the node's repo from a field gh actually emits:repository.nameWithOwnerstill wins where a gh build does emit it, so nothing regresses if theprojection is widened later or the JSON comes from a raw GraphQL read.
url, which is<host>/<owner>/<repo>/issues/<n>:the two path segments before
issues.because the second read intersects by issue NUMBER against this repo's
list-itemsoutput, so aforeign sub-issue sharing a local number would otherwise pull in an unrelated item.
Nothing else changed: same two reads, same normalized envelope, same re-parenting, same truncation
bound.
lib/frontier.shandlib/labels.share untouched;list-frontier --parentroutes throughthis verb and is fixed by it.
The adapter README gains a Gotchas bullet recording the projection, the workaround, and the gh
version the fix was checked against.
The underivable drop is no longer silent
Two different drops happen in that predicate, and only one of them is expected:
silent drop, because that is the behaviour CONTRACT.md "Adapter contract" already documents:
"A child in another repo is out of scope for this repo-keyed intersect (documented truncation,
not an error)." Sub-issues genuinely can be cross-repo, so this is the guard, not a defect.
work-items/tracker: list-sub-items returns empty because gh --json subIssues nodes carry no repository field #3825: an empty list and no signal. Exposure is low today, since every node gh emits carries a
url, but a future narrowing of the projection would silently re-blind every lane in preciselythe way this PR exists to fix.
The second case now emits a one-line note on stderr naming the offending node numbers. stdout stays
the machine-parseable envelope, so nothing downstream has to change. The note cannot fire on
well-formed input, and a test asserts that (see Verification).
This deliberately does not fail open. Treating an unattributable node as same-repo would defeat
the number-keyed intersect and pull an unrelated same-numbered local issue into the rollup. The drop
is kept; only its invisibility is fixed.
Verification
All foreground, from
/home/user/wt-3825, on this branch.list-sub-items over stubbed subIssuesinadapters/github/list-sub-items.test.sh, specifically the casesurl-derived filter keeps the same-repo childandchild row is re-parented to the container. It stubsghwith the real--json subIssuesshape (norepositorykey) plus a genuine cross-repo node,x/y#12, whosenumber also exists in the local repo. Reverting only
list-sub-items.shto main and rerunningthe suite gives:
FAIL: [8] url-derived filter keeps the same-repo child - expected github:o/r#11 got ''andFAIL: [9] child row is re-parented to the container - expected github:o/r#99 got null,suite exit 1. With the fix, 14/14 pass, exit 0. Node
x/y#12stays excluded either way, which iswhat proves the same-repo guard survived the change.
well-formed nodes emit no derivable-repo warningruns the existing well-formed stub(including its cross-repo node) and asserts stderr carries no note. This is the anti-noise
guard: the documented cross-repo truncation must stay silent.
urlat all and one whoseurlis not an issue path.stderr names both unattributable nodes on one lineasserts the single note names bothnumbers,
unattributable nodes dropped, the attributable one keptasserts the third node stillrolls up, and
stdout stays machine-parseable, diagnostic did not leak into itasserts theenvelope still parses.
printfturnsFAIL: [14] stderr names both unattributable nodes on one line - expected 1 got 0, so theassertion is load-bearing rather than vacuous.
scripts/affected-tests.sh --run->All 13 selected suites passed or were skipped., exit 0.plugins/work-items/tools/work-item-tracker/run directly: 58 suites,0 failures.
scripts/check-changelog-parity.sh --check-> exit 0.scripts/check-changelog-parity.sh --check-bump origin/main-> exit 0.scripts/check-changelog-parity.sh --check-preserved origin/main-> exit 0(177 headings compared).
scripts/check-changelog-parity.sh --check-order-> exit 0 (90 changelogs).shellcheck -x list-sub-items.sh list-sub-items.test.sh-> exit 0.shfmt -d list-sub-items.sh list-sub-items.test.sh-> exit 0, no diff.markdownlint-cli2on the changed CHANGELOG and README -> 0 issues.The gh projection claim is source-verified, not asserted from memory:
api/query_builder.goandapi/export_pr.gowere read at tags v2.94.0, v2.95.0, v2.96.0, v2.97.0 and v2.98.0. The queryrequests
repository{nameWithOwner}at every one of them and the export drops it at every one ofthem.
Live evidence, ported from #3832 (not produced on this branch)
The gh on this runner is 2.45.0, which predates
subIssuesentirely, so the acceptance criterianaming live issue numbers could not be exercised here. They were exercised on a gh 2.97.0 host
by the duplicate PR #3832, and the results below are quoted from that PR's body. This is another
PR's run, not this branch's validation. It is reproduced here because both PRs derive the repo
from the node
urlby the same rule, and on this input every node'surlparses and resolves tothe same repo, so the two implementations produce identical output over it. Nothing below was
re-run on this branch, and no number here is restated beyond what #3832 shows.
#3832 reports
gh --versionasgh version 2.97.0 (2026-07-31), andgh issue view 3799 --repo melodic-software/claude-code-plugins --json subIssuesreturning nodesthat carry
id,number,state,title,urland norepository, e.g.{"id":"I_kwDOTCGFQM8AAAABP7GlVw","number":3805,"state":"OPEN","title":"planning: plan the typed-ticket-body lane","url":"https://github.com/melodic-software/claude-code-plugins/issues/3805"}with
"totalCount":6. The same selection viagh api graphql, which asks forrepository { nameWithOwner }explicitly, does carry it, returning"totalCount":6and node 3805with
"nameWithOwner":"melodic-software/claude-code-plugins". That is the live confirmation of theprojection asymmetry this PR diagnoses from source.
Against the acceptance criteria, #3832 reports
list-sub-itemsongithub:melodic-software/claude-code-plugins#3799returning all six sub-issues (#3824, #3823,#3822, #3821, #3814, #3805), each with
parent_idset to#3799andblocked_by_count1 except#3805 at 0, where the pre-fix output was
{"schema_version":"1.0","items":[]}; andlist-frontier --parenton the same container returning exactly["github:melodic-software/claude-code-plugins#3805"], #3805 being the only child with zero openblockers and no assignee.
A spot check on a 2.94+ host against this branch is still the honest confirmation, and is not
claimed here.
Related
(
fix/3825-subissue-repo-filter) fixes the same issue off the same base SHA with the sameurl-derivation approach over the same adapter verb. By the worker lane's lowest-numbered-PR
convention, fix(work-items): scope GitHub sub-issues by url, not the absent repository field #3830 survives and fix(work-items): scope github sub-issue rollup off the node url #3832 is superseded. Its live 2.97.0 probe and acceptance-criteria
output are ported above under explicit attribution. Its fail-open branch is deliberately not
adopted: it rests on the premise that gh scopes
subIssuesto the parent's own repo, which isnot true (sub-issues can be cross-repo), and failing open would defeat the number-keyed intersect
that CONTRACT.md documents. Nothing in this PR touches fix(work-items): scope github sub-issue rollup off the node url #3832.
plugins/work-items/CHANGELOG.mdand
.claude-plugin/plugin.json. fix(work-items): floor C4/C5 work classes out of the autonomous frontier #3745 was drafted against0.39.63, which main has sincepassed: main is at
0.39.65, so this PR lands0.39.66with its entry above the preserved0.39.65. fix(work-items): floor C4/C5 work classes out of the autonomous frontier #3745 will need to renumber to0.39.67or later once this merges. Resolve anyconflict by merging and keeping both entries under their own headings; do not fold them
together, and do not relabel a released heading, which is what
--check-preservedcatches.lib/frontier.shandlib/labels.shin this plugin; neither is touched here.🤖 Generated with Claude Code
https://claude.ai/code/session_01ViPsHkL3ng9xWt2GjEQJob
Generated by Claude Code