fix(sdd-dispatch): resolve task→tracker via GraphQL parent#149
Conversation
The close-triggered route walk seeded from `payload.issue.parent_issue_url`
and re-fetched the closed issue via REST in the prior fix, but GitHub does
not populate the child→parent pointer on either the `issues.closed` webhook
OR on REST `GET /issues/{n}` for native sub-issues. The walk read empty,
broke at hop 0, and the cascade never advanced past layer 1 (issue #133).
Replace REST seeding with GraphQL `Issue.parent`, which is authoritative
for native sub-issues. When GraphQL returns null (the closed issue is not
a native sub-issue at all), fall back to scanning open `sdd:dispatched`
trackers in the repo and walking their `sub_issues` trees to locate the
closed task number. The two-hop ceiling, the `walked === 2` success
condition, and the `sdd:dispatched` gate on the resolved tracker are
unchanged.
Closes #133
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughUpdated dispatch workflow's task-closure detection to resolve parent chains via GraphQL (up to two hops) and fall back to a paged reverse lookup across open ChangesTask-Closure Detection via GraphQL and Reverse Lookup
Sequence DiagramsequenceDiagram
participant GHWebhook as GitHub Webhook
participant Dispatch as Dispatch Workflow
participant GraphQL as GraphQL Parent Walk
participant ReverseLookup as Reverse Lookup
participant Cascade as Cascade Arming Logic
GHWebhook->>Dispatch: issues.closed event (closed_issue_number)
Dispatch->>GraphQL: Resolve parent chain via GraphQL (up to 2 hops)
alt GraphQL Parent Found
GraphQL->>Dispatch: Return parent id, number, labels
else GraphQL Parent Not Found or Error
GraphQL->>Dispatch: No parent chain / error
Dispatch->>ReverseLookup: Scan open sdd:dispatched trackers' sub-issues
ReverseLookup->>Dispatch: Match closed_issue_number in tracker tree
end
Dispatch->>Cascade: Pass resolved tracker (or nil) + hop count
alt Tracker at 2 hops with sdd:dispatched label
Cascade->>Cascade: Arm dispatch cascade
else Other outcome
Cascade->>Cascade: Log non-task-closure or unresolved tracking
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@wrappers/sdd-dispatch.yml`:
- Around line 294-300: The current fallback reverse-lookup uses a single
github.rest.issues.listForRepo call (owner/repo, state/open, labels:
'sdd:dispatched', per_page:100) which only fetches the first page; change this
to paginate and aggregate all pages (e.g. use octokit.paginate or a loop with
page/per_page) so you collect all armed trackers before doing reverse
lookups—mirror the paging approach used by compute when it pages sub_issues.
Replace the single listForRepo call in the shown block and the two similar
blocks (the calls around the other occurrences referencing
github.rest.issues.listForRepo) to fetch all pages and then continue the
existing filtering/lookup logic on the aggregated results.
- Around line 258-261: The current catch around parentViaGraphql() collapses
lookup failures into a "no parent" result, incrementing gqlWalked and causing
valid task closures to be misclassified; change the error handling so lookup
errors are distinguished from a legitimate null parent (e.g., have
parentViaGraphql() return an explicit {parent: null, error: true} or rethrow and
catch upstream), do NOT increment or set gqlWalked on transient lookup errors,
and adjust the fallback logic that decides Unit/tracker vs task closure to
consider tracker unresolved OR any lookup error (not only gqlWalked === 0) so a
failed hop triggers the reverse lookup path instead of silently treating it as
no-parent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3fe6e5cf-6591-4ef0-911b-49b1cc7fa9a3
📒 Files selected for processing (1)
wrappers/sdd-dispatch.yml
Two CodeRabbit findings on the cascade-arming logic:
1. parentViaGraphql() collapsed both "no parent" and lookup failure
into null, so a transient GraphQL error on hop 2 incremented
gqlWalked and routed the closure into the Unit/tracker-close
branch — silently dropping a real task closure. Switch to a
discriminated return ({ kind: 'ok' | 'none' | 'error' }) and
surface 'stoppedBy' from walkViaGraphql(). On 'error', try the
reverse lookup before giving up.
2. trackerViaReverseLookup() called listForRepo / sub_issues with
per_page:100 and no paging. A closed task beyond the first page
of armed trackers, Units, or sibling tasks was reported as having
no resolvable tracker. Extract listSubIssuesAll() and
listDispatchedTrackersAll() helpers that mirror the paging
pattern compute already uses.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Root cause
sdd-dispatch's close-triggered route walk seeded frompayload.issue.parent_issue_urland re-fetched the closed issue via REST in the prior fix, but GitHub does not populate the child→parent pointer on either theissues.closedwebhook OR on a RESTGET /issues/{n}re-fetch for native sub-issues. The walk read empty, broke at hop 0, and the cascade never advanced past layer 1 (#133).Evidence on
gominimal/spectacles-testtracker #157: runs 26272436357 and 26272460894 loggedwalked 0 hops; ignoringfor both task closures.#170/#176REST.parentisnull; webhookparent_issue_urlis empty.Fix
Replace REST seeding with GraphQL
Issue.parentas the primary mechanism — it is authoritative for native sub-issues and returns the parent node with one query per hop. The walk seed is the closed issue'snode_id; each hop returns the parent's node id for the next hop.When GraphQL returns null (the closed issue is not a native sub-issue at all — e.g. linked by body reference instead of the native parent connection), fall back to a reverse lookup: list open issues in the repo carrying
sdd:dispatched, walk each tracker'ssub_issues→ Unitsub_issueslooking for the closed issue's number. Bounded by the small number of armed trackers and tolerant of races because the GraphQL path is primary.The two-hop ceiling, the
walked === 2task-closure success condition, and thesdd:dispatchedlabel gate on the resolved tracker are unchanged. The diagnosticwalked N hops; ignoringlog line now reflects the GraphQL result rather than implying a missingparent_issue_url.Acceptance
sdd-dispatchcompute+dispatchand arms every newly-unblocked task within one cycle.sdd:donewith no manual/dispatch.routejob's existingissues: readpermission is sufficient for GraphQLIssue.parent; no permission change needed.References
gh aw compilerequired).🤖 Generated with Claude Code