fix(github-sdlc-planning): live GraphQL fallback for hygiene-check subagent/race blind spots - #325
Merged
Merged
Conversation
…k lifecycle-comment blind spots (#324) checkLifecycleComment's transcript scan is structurally blind to a comment posted by a background workflow subagent (its tool calls never appear in the parent's own transcript and to a same-turn parallel-dispatch race (a comment call's tool_result may not yet be flushed to the transcript file when a sibling set_field_value call's PostToolUse hook fires). Adds checkRecentCommentViaGraphQL, a live fallback tried only when the local scan resolves but finds nothing: it queries the issue's own recent comments directly from GitHub, bypassing the transcript file entirely. Wired into both checkLifecycleComment (PostToolUse) and isLifecycleFindingNowResolved (hygiene-aggregate.mjs's Stop-time re-validation, gdlc#278) -- the aggregator needed the same fallback in its own right, since a subagent-posted comment can never resolve via a parent-transcript re-scan no matter how long the turn runs. buildConsolidatedContext and isLifecycleFindingNowResolved are now async to accommodate the network call, the same migration issue #172 already made to checkLifecycleComment itself. Propagated identically to github-pull-requests/github-bug-capture's byte-identical copies per AD-4/hygiene-hook-drift-check. Closes #324 EOF )
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses remaining false-positive cases in the hygiene-check lifecycle-comment detector by adding a live GitHub GraphQL fallback when transcript-based detection can’t see (or hasn’t yet flushed) the comment, and by applying the same fallback in the Stop-time aggregator so resolved gaps don’t persist across the end-of-turn summary.
Changes:
- Add
checkRecentCommentViaGraphQLand invoke it as a fallback when transcript scanning resolves but finds no comment. - Make
buildConsolidatedContext/isLifecycleFindingNowResolvedasync and wire in the same live fallback for Stop-time re-validation. - Add/adjust unit tests and update ADR-0007 audit entry; propagate changes to byte-identical hook copies across plugins.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| plugins/github-sdlc-planning/hooks/lib/hygiene-check.mjs | Adds live GraphQL recent-comment fallback and integrates it into checkLifecycleComment. |
| plugins/github-sdlc-planning/hooks/lib/hygiene-aggregate.mjs | Makes consolidated-context building async and adds live fallback during Stop-time re-validation. |
| plugins/github-sdlc-planning/hooks/hygiene-aggregate.mjs | Adds runGraphQL wrapper and awaits async consolidated-context building. |
| plugins/github-sdlc-planning/mcp-server/test/unit/hygiene-check-hook.test.ts | Adds unit tests for the new GraphQL fallback and new checkLifecycleComment behaviors. |
| plugins/github-sdlc-planning/mcp-server/test/unit/hygiene-scratch-aggregate.test.ts | Updates tests to await async aggregation and adds coverage for Stop-time live fallback. |
| plugins/github-pull-requests/hooks/lib/hygiene-check.mjs | Propagates the same live GraphQL fallback logic. |
| plugins/github-pull-requests/hooks/lib/hygiene-aggregate.mjs | Propagates async aggregation + Stop-time live fallback. |
| plugins/github-pull-requests/hooks/hygiene-aggregate.mjs | Propagates runGraphQL wrapper and async main. |
| plugins/github-bug-capture/hooks/lib/hygiene-check.mjs | Propagates the same live GraphQL fallback logic. |
| plugins/github-bug-capture/hooks/lib/hygiene-aggregate.mjs | Propagates async aggregation + Stop-time live fallback. |
| plugins/github-bug-capture/hooks/hygiene-aggregate.mjs | Propagates runGraphQL wrapper and async main. |
| docs/decisions/adr-0007-ticket-hygiene-reinforcement-hooks.md | Adds a new audit entry documenting the gdlc#324 resolution approach. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…items checkRecentCommentViaGraphQL only queried repository.issue(number:), so a Projects v2 item whose content is a PullRequest (resolveItemIdentity already resolves both types) could never be live-confirmed -- reopening, for PR-backed items, the exact blind spot gdlc#324/PR #325 exists to close. Add a sibling pullRequest(number:) field to the same query (the type that doesn't match resolves to null) and merge whichever side returns comment nodes before the recency check. Adds three regression tests covering the pullRequest-only match path, a too-old pullRequest comment, and the fail-open path when both fields resolve to null. Propagated identically to github-pull-requests's and github-bug-capture's byte-identical copies per AD-4; verified no drift. Addresses Copilot review feedback on PR #325.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Follow-up to #320/#323. Fixes two remaining
hygiene-checklifecycle-commentdetector blind spots, both structural properties of scanning only the
parent session's own transcript file:
transcript — a subagent's tool calls execute and are logged in its own
transcript, never the parent's.
write — a comment call's own
tool_resultmay not yet be flushed tothe transcript file at the instant a sibling
set_field_valuecall'sPostToolUsehook fires and reads it.Fix
Adds
checkRecentCommentViaGraphQL, a live fallbackcheckLifecycleCommenttries only when the local transcript scan resolves but finds nothing: it
queries the issue's own recent comments directly from GitHub (a 5-minute
recency window), bypassing the transcript file entirely. This closes (1)
completely (the subagent's own API call has already completed by the time
the parent resumes) and mitigates (2) to whatever extent the comment's
underlying network request has already completed even if its local
bookkeeping hasn't caught up yet.
Also wired into
isLifecycleFindingNowResolved(hygiene-aggregate.mjs'sStop-time re-validation, gdlc#278) — the aggregator needed the samefallback in its own right, since a subagent-posted comment can never
resolve via a parent-transcript re-scan no matter how long the turn runs;
without this, the end-of-turn reminder would report the same "gap"
forever even after the comment genuinely exists on GitHub.
buildConsolidatedContext/isLifecycleFindingNowResolvedare nowasyncto accommodate the network call — the same sync-to-async migration issue
#172 already made to
checkLifecycleCommentitself. Both fallbacks failopen (the pre-existing transcript-only finding stands) on a missing
runGraphQL, a malformed response, or a thrown error — never a guess,matching every other check in this file.
Propagated identically to
github-pull-requests/github-bug-capture'sbyte-identical copies per AD-4 (
hygiene-hook-drift-checkverified cleanlocally). ADR-0007 updated with a new dated audit entry.
Testing
checkRecentCommentViaGraphQL(window match, customwindow, no comments, malformed response, thrown error, unparseable
createdAt).checkLifecycleCommenttests covering: transcript-scan-miss +live-confirm (no finding), transcript-scan-miss + live-miss (still
flags, unchanged), live fallback throwing (fails open, still flags), no
wasted network call once the transcript scan alone already resolved it,
and the
set_field_value/itemId-resolution path.buildConsolidatedContexttests covering the same matrix for theStop-time aggregator, plus the "no wasted network call" case.awaitthe now-asyncbuildConsolidatedContextcalls; no existing test's expected behaviorchanged.
npm run typecheck && npm run lint && npm testgreen in all threeaffected plugins (
github-sdlc-planning,github-pull-requests,github-bug-capture);dist/rebuilt and confirmed to match thecommitted artifact in all three (no
src/undermcp-server/changed —only the plain-node
hooks/scripts, run directly, outside the bundle).Closes #324