fix(github-sdlc-planning): live GraphQL fallback for hygiene-check subagent/race blind spots

docs/decisions/adr-0007-ticket-hygiene-reinforcement-hooks.md

@@ -521,6 +521,65 @@ AD-4.
 
 **Action Required:** None; issue #289 is resolved.
 
+### 2026-07-18
+
+**Status:** Compliant
+
+**Findings:**
+
+| Finding | Files | Lines | Assessment |
+| --- | --- | --- | --- |
+| Issue #324 resolved (follow-up to #320/#323): `scanTranscriptForComment` is structurally blind to two cases no amount of parent-transcript scanning can fix -- (1) a lifecycle comment posted by a background workflow subagent, whose tool calls execute and are logged in its OWN transcript, never the parent session's; and (2) a same-turn parallel tool-call dispatch, where a comment call's own `tool_result` may not yet be flushed to the transcript file at the instant a sibling `set_field_value` call's `PostToolUse` hook fires and reads it, even though the underlying GitHub API call already completed. Fixed by adding `checkRecentCommentViaGraphQL`, a live GraphQL fallback `checkLifecycleComment` tries only when the transcript scan resolves but finds nothing: it queries the issue's own recent comments directly from GitHub (bypassing the transcript file entirely) and treats any comment created within a 5-minute window of "now" as satisfying the transition. Wired into both `checkLifecycleComment` (the `PostToolUse`-time check) and `isLifecycleFindingNowResolved` (the `Stop`-time aggregator's own re-validation, `hygiene-aggregate.mjs`'s lib) -- 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. Deliberately over-inclusive (any recent comment, not just the acting user's) per this hook's existing false-negative-over-false-positive tolerance for an advisory nudge; fails open (no live confirmation, the pre-existing transcript-only finding stands) on a missing `runGraphQL`, a malformed response, or any thrown error. | plugins/*/hooks/lib/hygiene-check.mjs, plugins/*/hooks/lib/hygiene-aggregate.mjs, plugins/*/hooks/hygiene-aggregate.mjs | - | fixed |
+
+**Summary:** Both gaps trace to the same structural limitation --
+`scanTranscriptForComment` can only ever see what's written to ONE
+transcript file, and neither a subagent's own transcript nor a
+not-yet-flushed same-turn write is guaranteed to be in it at the instant a
+check runs. Rather than attempting to read a subagent's transcript
+directly (no stable, discoverable path to it exists from the parent's own
+hook context) or forcing sequential tool-call dispatch (outside a hook's
+control), both gaps are closed with the one signal that doesn't depend on
+transcript timing at all: asking GitHub directly. `hygiene-aggregate.mjs`'s
+entrypoint gained the same `runGraphQL` wrapper `hygiene-check.mjs`'s
+entrypoint already had, and `buildConsolidatedContext`/
+`isLifecycleFindingNowResolved` became `async` to accommodate it -- the
+same kind of sync-to-async migration issue #172 already made to
+`checkLifecycleComment` itself. Regression tests cover: a comment invisible
+to the transcript scan but confirmed via a live GraphQL check (both the
+`PostToolUse`-time check and the `Stop`-time aggregator); no live comment
+found (finding still fires, unchanged from pre-#324 behavior); and the
+live check failing open (a thrown GraphQL error, a missing `runGraphQL`)
+without ever suppressing a genuine finding. Propagated identically to
+`github-pull-requests`/`github-bug-capture`'s byte-identical copies per
+AD-4.
+
+**Action Required:** None; issue #324 is resolved.
+
+### 2026-07-18 (PR #325 Copilot review follow-up)
+
+**Status:** Compliant
+
+**Findings:**
+
+| Finding | Files | Lines | Assessment |
+| --- | --- | --- | --- |
+| `checkRecentCommentViaGraphQL`'s query only fetched `repository.issue(number:)`. A Projects v2 tracked item's `content` can be a `PullRequest` (`resolveItemIdentity` already handles both), so a PR-backed item's `number` would resolve to a `null` `issue` field and the fallback could never live-confirm it -- reintroducing, for PR-backed items specifically, the exact blind spot this PR exists to close. Fixed by adding a sibling `pullRequest(number:)` field to the same query (GraphQL resolves whichever type the number actually is; the other side comes back `null`) and merging both `issue`/`pullRequest` `comments.nodes` arrays before the recency check. | plugins/*/hooks/lib/hygiene-check.mjs | - | fixed |
+
+**Summary:** Copilot's review of PR #325 caught that the live-fallback
+query addressed only Issue-backed tracked items, not PR-backed ones, even
+though `resolveItemIdentity` (the function that produces the `identity`
+this fallback consumes) already treats both as first-class. Both `issue`
+and `pullRequest` are now queried by number in the same request; response
+parsing merges whichever side actually returned comment nodes (the other
+is `null` by construction, never both at once). Three new regression tests
+cover: a recent comment surfacing only via `pullRequest.comments.nodes`; a
+too-old comment on the `pullRequest` side; and the fail-open path when both
+`issue` and `pullRequest` resolve to `null` (item not found by either
+type). Propagated identically to `github-pull-requests`/
+`github-bug-capture`'s byte-identical copies per AD-4.
+
+**Action Required:** None; the gap Copilot flagged on PR #325 is resolved.
+
 [adr-0003]: adr-0003-board-status-hygiene.md
 [adr-0004]: adr-0004-project-config-surface.md
 [adr-0005]: adr-0005-project-config-cwd-resolution.md

plugins/github-bug-capture/hooks/hygiene-aggregate.mjs

@@ -9,10 +9,31 @@
 // relative path (including this copy, if you're reading it from one of
 // those plugins right now), kept in sync by a build-time drift check
 // (AD-4, .github/workflows/ci.yml's hygiene-hook-drift-check job).
+import { execFileSync } from 'node:child_process';
 import { readFileSync } from 'node:fs';
 import { readScratchEntries, scratchFilePath, clearScratch } from './lib/hygiene-scratch.mjs';
 import { buildConsolidatedContext } from './lib/hygiene-aggregate.mjs';
 
+/** gdlc#324: thin wrapper around `gh api graphql`, byte-for-byte the same
+ * shape as hygiene-check.mjs's own `runGraphQL` -- this is now the second
+ * (and only other) place in this hook family that talks to GitHub, needed
+ * so `buildConsolidatedContext`'s live-comment fallback can fire at Stop
+ * time, not just at PostToolUse time. */
+function runGraphQL(query, variables) {
+  const args = ['api', 'graphql', '-f', `query=${query}`];
+  for (const [key, value] of Object.entries(variables)) {
+    if (typeof value === 'number' || typeof value === 'boolean') {
+      args.push('-F', `${key}=${value}`);
+    } else {
+      args.push('-f', `${key}=${value}`);
+    }
+  }
+  const raw = execFileSync('gh', args, { encoding: 'utf8' });
+  const parsed = JSON.parse(raw);
+  if (parsed.errors?.length) throw new Error(parsed.errors.map((e) => e.message).join('; '));
+  return parsed.data;
+}
+
 function readStdin() {
   try {
     const parsed = JSON.parse(readFileSync(0, 'utf8'));
@@ -34,7 +55,9 @@ function emitContext(hookEventName, text) {
   process.stdout.write(JSON.stringify({ hookSpecificOutput: { hookEventName, additionalContext: text } }));
 }
 
-function main() {
+// gdlc#324: async now that buildConsolidatedContext's live-comment fallback
+// can make a network call (matching hygiene-check.mjs's own async main).
+async function main() {
   const input = readStdin();
   if (!input.session_id) {
     emitEmpty();
@@ -43,7 +66,7 @@ function main() {
 
   const path = scratchFilePath(input.session_id);
   const entries = readScratchEntries(path);
-  const context = buildConsolidatedContext(entries, input.transcript_path);
+  const context = await buildConsolidatedContext(entries, input.transcript_path, undefined, runGraphQL);
 
   // Clear regardless of whether there was anything to report: a turn with
   // zero findings should not have its (empty-findings) entries re-read and
@@ -61,8 +84,4 @@ function main() {
 // must hold even on an unanticipated throw anywhere above (e.g. a
 // malformed scratch-file entry reaching buildConsolidatedContext) -- a
 // hook must never break the tool call it observes.
-try {
-  main();
-} catch {
-  emitEmpty();
-}
+main().catch(() => emitEmpty());

plugins/github-bug-capture/hooks/lib/hygiene-aggregate.mjs

@@ -11,10 +11,22 @@
  * re-runs hygiene-check.mjs's own `scanTranscriptForComment` against the
  * turn's transcript file -- reading that file's logged `tool_name`/
  * `tool_input` history is how that scan works, distinct from this hook's
- * own input envelope (which still carries none). No network call is
- * involved either way.
+ * own input envelope (which still carries none).
+ *
+ * gdlc#324: a re-scan of the parent's OWN transcript can never resolve a
+ * lifecycle-comment finding whose comment was posted by a background
+ * workflow subagent -- that comment lives only in the subagent's own
+ * transcript, no matter how long the turn runs or how many times this
+ * re-scan repeats. `isLifecycleFindingNowResolved` now also tries
+ * `checkRecentCommentViaGraphQL` (the same live fallback
+ * `checkLifecycleComment` itself uses) when the transcript re-scan alone
+ * doesn't resolve it, so this aggregator no longer keeps reporting a
+ * finding forever for a comment that, in reality, GitHub already has. This
+ * is the one place in this file that can make a network call, and only
+ * when `runGraphQL` is supplied and the transcript re-scan alone wasn't
+ * enough.
  */
-import { scanTranscriptForComment } from './hygiene-check.mjs';
+import { scanTranscriptForComment, checkRecentCommentViaGraphQL } from './hygiene-check.mjs';
 
 /** Matches exactly the string `checkLifecycleComment` (hygiene-check.mjs)
  * emits, recovering the `owner`/`repo`/`number` identity it already embeds
@@ -36,16 +48,30 @@ const LIFECYCLE_FINDING_RE = /^([^/\s]+)\/([^#\s]+)#(\d+): transitioned with no
  * scan now resolves as found -- the live end-of-turn truth wins over the
  * stale scratch-time snapshot. Only this one finding *kind* is re-checked:
  * it's the one cheap and unambiguous enough to revalidate purely from its
- * own message text (identity + a single deterministic, network-free
- * transcript scan), unlike e.g. a sub-issue-linkage finding, which would
- * need a fresh GraphQL round trip this backstop deliberately never makes.
- * `scanFn` is injectable for tests, defaulting to the real transcript scan. */
-function isLifecycleFindingNowResolved(finding, transcriptPath, scanFn) {
+ * own message text (identity + a single deterministic transcript scan,
+ * plus -- gdlc#324 -- one live GraphQL fallback), unlike e.g. a
+ * sub-issue-linkage finding, which would need a fresh GraphQL round trip of
+ * its own this backstop deliberately never makes for that kind.
+ * `scanFn` is injectable for tests, defaulting to the real transcript scan.
+ *
+ * gdlc#324: when the re-scan alone doesn't resolve the finding, this also
+ * tries `checkRecentCommentViaGraphQL` -- the same live fallback
+ * `checkLifecycleComment` itself uses -- before giving up. This is the
+ * fix for a comment posted by a background subagent: that comment can
+ * never appear in the PARENT transcript `scanFn` reads, no matter how many
+ * times this aggregator re-scans it, so without this fallback the
+ * end-of-turn reminder would report the same "gap" forever even though
+ * GitHub already has the comment. `runGraphQL` is optional; when absent
+ * (or the live check can't confirm anything) the finding stands, same as
+ * before this fix. */
+async function isLifecycleFindingNowResolved(finding, transcriptPath, scanFn, runGraphQL) {
   const match = LIFECYCLE_FINDING_RE.exec(finding);
   if (!match) return false;
   const [, owner, repo, numberStr] = match;
-  const scan = scanFn(transcriptPath, { owner, repo, number: Number(numberStr) });
-  return scan.resolved === true && scan.found === true;
+  const identity = { owner, repo, number: Number(numberStr) };
+  const scan = scanFn(transcriptPath, identity);
+  if (scan.resolved === true && scan.found === true) return true;
+  return checkRecentCommentViaGraphQL(identity, runGraphQL);
 }
 
 /** Build ONE consolidated reminder from a turn's scratch entries (NFR-6):
@@ -55,10 +81,19 @@ function isLifecycleFindingNowResolved(finding, transcriptPath, scanFn) {
  * de-duplicated verbatim (the same finding can legitimately recur across
  * multiple touches of the same issue in one turn). Returns `null` when
  * there is nothing to report -- no entries, no findings at all, or every
- * finding turned out to be already resolved (gdlc#278) -- so the caller
- * can stay silent rather than emit an empty-handed "everything is fine"
- * message no one asked for. */
-export function buildConsolidatedContext(entries, transcriptPath, scanFn = scanTranscriptForComment) {
+ * finding turned out to be already resolved (gdlc#278, plus gdlc#324's live
+ * GraphQL fallback) -- so the caller can stay silent rather than emit an
+ * empty-handed "everything is fine" message no one asked for.
+ *
+ * `runGraphQL` (gdlc#324, 4th param, appended rather than folded into
+ * `scanFn` as an options object to keep every existing positional call --
+ * production and test alike -- working unchanged) is optional and passed
+ * straight through to `isLifecycleFindingNowResolved`'s own live fallback;
+ * omitting it just means that fallback never fires, the pre-#324
+ * behavior. `async` since that fallback can make a network call --
+ * the same kind of sync-to-async migration issue #172 already made to
+ * `checkLifecycleComment` itself. */
+export async function buildConsolidatedContext(entries, transcriptPath, scanFn = scanTranscriptForComment, runGraphQL) {
   if (!Array.isArray(entries) || entries.length === 0) return null;
 
   const findings = [];
@@ -79,7 +114,8 @@ export function buildConsolidatedContext(entries, transcriptPath, scanFn = scanT
   }
   if (findings.length === 0) return null;
 
-  const liveFindings = findings.filter((finding) => !isLifecycleFindingNowResolved(finding, transcriptPath, scanFn));
+  const resolutions = await Promise.all(findings.map((finding) => isLifecycleFindingNowResolved(finding, transcriptPath, scanFn, runGraphQL)));
+  const liveFindings = findings.filter((_finding, index) => !resolutions[index]);
   if (liveFindings.length === 0) return null;
 
   const touchCount = entries.length;