[Bugfix #966] derivePrReady ignores merged → merged-but-gate-pending builder vanishes

codev/projects/bugfix-966-needs-attention-merged-pr-with/status.yaml

@@ -0,0 +1,17 @@
+id: bugfix-966
+title: needs-attention-merged-pr-with
+protocol: bugfix
+phase: pr
+plan_phases: []
+current_plan_phase: null
+gates:
+  pr:
+    status: approved
+    requested_at: '2026-06-03T03:25:02.400Z'
+    approved_at: '2026-06-03T13:39:33.573Z'
+iteration: 1
+build_complete: false
+history: []
+started_at: '2026-06-02T20:38:06.755Z'
+updated_at: '2026-06-03T13:39:33.573Z'
+pr_ready_for_human: false

codev/state/bugfix-966_thread.md

@@ -0,0 +1,63 @@
+# bugfix-966 — Needs Attention: merged PR with still-pending pr gate vanishes
+
+## Issue
+`derivePrReady` in `packages/codev/src/agent-farm/servers/overview.ts` returns `true`
+for a builder whose `pr` gate is `pending` + `requested_at` present, *even when the PR
+has already merged*. A merged PR lives in `recentlyClosed`, not `pendingPRs`, so:
+- `NeedsAttentionList` loop (A) emits no PR row (PR not in `pendingPRs`)
+- loop (B) suppresses the builder row (`if (b.prReady) continue;`)
+→ builder vanishes from the Work surface entirely.
+
+## Investigation findings
+- `merged: true` lives inside the `pr_history:` list in status.yaml (4-space indent,
+  one entry per PR/stage). Last entry corresponds to the PR tied to the current pr gate.
+- The parser (`parseStatusYaml`) does NOT currently parse `pr_history` at all.
+- Confirmed fix is sufficient (no NeedsAttentionList change): once `prReady` is false,
+  the builder is no longer suppressed in loop (B). Its `pr` gate is still `pending` with
+  `requested_at`, so `detectBlocked` returns 'PR review' → builder surfaces as a gate row.
+
+## Plan
+1. Parse `merged` boolean from the LAST `pr_history` entry (handles SPIR checkpoint
+   multi-PR case: an earlier merged PR + a later open PR awaiting review → not merged).
+2. `derivePrReady` returns false when `parsed.merged` is true.
+3. Unit tests: parseStatusYaml parses pr_history merged; derivePrReady false when merged.
+
+## Implementation (done)
+- `overview.ts`: added `merged: boolean` to `ParsedStatus`; parse it from the LAST
+  `pr_history` entry; `derivePrReady` now `&& !parsed.merged`.
+- `overview.test.ts`: +8 `#966` tests (5 parseStatusYaml, 2 derivePrReady, 1 end-to-end
+  repro proving the builder surfaces via the gate row, not vanishes). All 8 pass. tsc clean.
+
+## Environment blocker (NOT my code)
+- Worktree had no node_modules. `pnpm install` fails compiling `better-sqlite3@12.9.0`:
+  node-gyp reports "No Xcode or CLT version detected" (CLT pkgutil receipt missing) even
+  though `xcode-select -p` is valid. node v26 (ABI 147) has no prebuilt; falls back to a
+  compile that fails. Main workspace's binary is ABI 141 → incompatible with node 26.
+- Effect: repo-wide `npm test` fails on better-sqlite3 (consult/metrics + OverviewCache
+  tests). My change adds ZERO new failures — the 5 failing tests in overview.test.ts are
+  all `new Database(...)` ABI errors; the 153 non-sqlite tests pass.
+- This will block porch's `fix`-phase `tests` check. Needs CLT reinstall (user/sudo) or a
+  node downgrade. Escalating to architect rather than bypassing.
+
+## Environment UNBLOCKED (architect-authorized)
+- Bumped better-sqlite3 ^12.5.0 → ^12.10.0 (commit 45da56b7). 12.10.0 ships a node-26
+  ABI-147 prebuilt → `pnpm install` downloads it (prebuild-install, no compile, no CLT).
+- `porch check`: build ✓, tests ✓ (full suite green under node 26, incl. my 8 #966 tests).
+
+## PR
+- PR #980 opened (Fixes #966). Net diff ~ overview.ts (57) + tests (152) + bump/lockfile.
+- Running 3-way CMAP (gemini/codex/claude --protocol bugfix --type pr).
+
+## CMAP (3-way) — all APPROVE / HIGH / no key issues
+- Gemini ✅, Codex ✅, Claude ✅. One non-blocking note (Claude): test comment named an
+  external adopter → scrubbed in 61333c38 per anonymization policy.
+
+## Status
+- [x] Investigate / root cause
+- [x] Implement fix
+- [x] Tests (8 new #966 tests pass; tsc clean)
+- [x] Environment unblocked (better-sqlite3 bump)
+- [x] PR #980 created
+- [x] CMAP review (all APPROVE) + scrub
+- [x] porch done → pr gate requested; architect notified
+- [ ] AWAITING human pr-gate approval (do not self-approve / self-merge)

packages/codev/package.json

@@ -41,7 +41,7 @@
     "@anthropic-ai/claude-agent-sdk": "^0.2.41",
     "@google/genai": "^1.0.0",
     "@openai/codex-sdk": "^0.130.0",
-    "better-sqlite3": "^12.5.0",
+    "better-sqlite3": "^12.10.0",
     "chalk": "^5.3.0",
     "commander": "^12.1.0",
     "glob": "^11.0.0",

packages/codev/src/agent-farm/__tests__/overview.test.ts

@@ -348,6 +348,102 @@ describe('overview', () => {
       const result = parseStatusYaml(yaml);
       expect(result.gateApprovedAt).toEqual({});
     });
+
+    it('defaults merged to false when pr_history is absent (#966)', () => {
+      const result = parseStatusYaml("id: '0100'\nphase: review");
+      expect(result.merged).toBe(false);
+    });
+
+    it('parses merged: true from the pr_history entry (#966)', () => {
+      const yaml = [
+        "id: '2019'",
+        'protocol: bugfix',
+        'phase: review',
+        'gates:',
+        '  pr:',
+        '    status: pending',
+        "    requested_at: '2026-06-02T15:14:59.000Z'",
+        'pr_history:',
+        '  - phase: review',
+        '    pr_number: 2030',
+        '    branch: builder/spir-2019',
+        "    created_at: '2026-06-02T15:10:00.000Z'",
+        '    merged: true',
+        "    merged_at: '2026-06-02T15:14:34.000Z'",
+        'pr_ready_for_human: true',
+      ].join('\n');
+
+      const result = parseStatusYaml(yaml);
+      expect(result.merged).toBe(true);
+      // sanity: the surrounding fields still parse correctly around the new section
+      expect(result.gates['pr']).toBe('pending');
+      expect(result.gateRequestedAt['pr']).toBe('2026-06-02T15:14:59.000Z');
+    });
+
+    it('parses merged: false (open PR) in pr_history (#966)', () => {
+      const yaml = [
+        "id: '0100'",
+        'phase: review',
+        'pr_history:',
+        '  - phase: review',
+        '    pr_number: 50',
+        '    branch: builder/bugfix-100',
+        "    created_at: '2026-06-02T15:10:00.000Z'",
+        '    merged: false',
+      ].join('\n');
+
+      const result = parseStatusYaml(yaml);
+      expect(result.merged).toBe(false);
+    });
+
+    it('reflects the LAST pr_history entry — earlier merged PR + later open PR (#966)', () => {
+      // SPIR/PIR checkpoint workflow: an earlier checkpoint PR merged, but a later
+      // PR is still open and awaiting review. The current pr gate is for the LATEST
+      // PR (not merged), so the builder is genuinely PR-ready.
+      const yaml = [
+        "id: '0100'",
+        'protocol: spir',
+        'phase: review',
+        'pr_history:',
+        '  - phase: implement',
+        '    pr_number: 40',
+        '    branch: builder/spir-100',
+        "    created_at: '2026-06-01T00:00:00.000Z'",
+        '    merged: true',
+        "    merged_at: '2026-06-01T01:00:00.000Z'",
+        '  - phase: review',
+        '    pr_number: 41',
+        '    branch: builder/spir-100',
+        "    created_at: '2026-06-02T00:00:00.000Z'",
+        '    merged: false',
+      ].join('\n');
+
+      const result = parseStatusYaml(yaml);
+      expect(result.merged).toBe(false);
+    });
+
+    it('reflects the LAST pr_history entry when the later entry has no merged key (#966)', () => {
+      // The later (open) PR has no `merged` key at all — must still read as not-merged.
+      const yaml = [
+        "id: '0100'",
+        'protocol: spir',
+        'phase: review',
+        'pr_history:',
+        '  - phase: implement',
+        '    pr_number: 40',
+        '    branch: builder/spir-100',
+        "    created_at: '2026-06-01T00:00:00.000Z'",
+        '    merged: true',
+        "    merged_at: '2026-06-01T01:00:00.000Z'",
+        '  - phase: review',
+        '    pr_number: 41',
+        '    branch: builder/spir-100',
+        "    created_at: '2026-06-02T00:00:00.000Z'",
+      ].join('\n');
+
+      const result = parseStatusYaml(yaml);
+      expect(result.merged).toBe(false);
+    });
   });
 
   // ==========================================================================
@@ -730,6 +826,7 @@ describe('overview', () => {
         planPhases: [],
         startedAt: '2026-05-26T00:00:00.000Z',
         prReadyForHuman: null,
+        merged: false,
         ...overrides,
       };
     }
@@ -763,6 +860,28 @@ describe('overview', () => {
       }))).toBe(false);
     });
 
+    it('returns false when the PR has merged but the pr gate is still pending (#966)', () => {
+      // Repro: porch left the `pr` gate pending after an out-of-band merge. The
+      // merged PR is in recentlyClosed (never pendingPRs), so a stale prReady:true
+      // would suppress the builder row without emitting a PR row → vanishes.
+      expect(derivePrReady(makeParsed({
+        protocol: 'bugfix',
+        phase: 'review',
+        gates: { pr: 'pending' },
+        gateRequestedAt: { pr: '2026-06-02T15:14:59Z' },
+        merged: true,
+      }))).toBe(false);
+    });
+
+    it('still returns true when the pr gate is pending, requested, and NOT merged (#966)', () => {
+      // The companion to the merged case: an open PR genuinely awaiting review.
+      expect(derivePrReady(makeParsed({
+        gates: { pr: 'pending' },
+        gateRequestedAt: { pr: '2026-05-26T12:00:00Z' },
+        merged: false,
+      }))).toBe(true);
+    });
+
     it('reads the pr gate directly and ignores pr_ready_for_human (#927)', () => {
       // Field true but gate not pending → not ready (kills the sticky-field hazard #919).
       expect(derivePrReady(makeParsed({
@@ -791,6 +910,39 @@ describe('overview', () => {
     it('returns false when no pr-gate signal is present', () => {
       expect(derivePrReady(makeParsed({ protocol: 'spir', phase: 'implement' }))).toBe(false);
     });
+
+    it('repro #966: merged-but-gate-pending builder surfaces via the gate row, not as PR-ready', () => {
+      // Full chain on the real #966 repro shape: porch recorded
+      // merged: true but left the pr gate pending. The builder must NOT
+      // read as PR-ready (else NeedsAttentionList suppresses its row while no PR
+      // row exists — it vanishes). It must instead surface as a blocked "PR review"
+      // gate row, since the pr gate is genuinely still pending. This is why fixing
+      // derivePrReady alone is sufficient (no NeedsAttentionList change needed).
+      const yaml = [
+        "id: '2019'",
+        'protocol: bugfix',
+        'phase: review',
+        'gates:',
+        '  pr:',
+        '    status: pending',
+        "    requested_at: '2026-06-02T15:14:59.000Z'",
+        'pr_history:',
+        '  - phase: review',
+        '    pr_number: 2030',
+        '    branch: builder/spir-2019',
+        "    created_at: '2026-06-02T15:10:00.000Z'",
+        '    merged: true',
+        "    merged_at: '2026-06-02T15:14:34.000Z'",
+        'pr_ready_for_human: true',
+      ].join('\n');
+
+      const parsed = parseStatusYaml(yaml);
+      // No longer suppressed as a (now-merged) PR-ready builder...
+      expect(derivePrReady(parsed)).toBe(false);
+      // ...and surfaces via the gate-row path instead (pr gate still pending).
+      expect(detectBlocked(parsed)).toBe('PR review');
+      expect(detectBlockedSince(parsed)).toBe('2026-06-02T15:14:59.000Z');
+    });
   });
 
   describe('computeIdleMs', () => {

packages/codev/src/agent-farm/servers/overview.ts

@@ -196,6 +196,19 @@ interface ParsedStatus {
    * value and the v3.1.3 fallback derivation.
    */
   prReadyForHuman: boolean | null;
+  /**
+   * Whether the PR tied to the current `pr` gate has already been merged
+   * (Issue #966). Read from the LAST entry of porch's `pr_history` list — the
+   * most-recent PR is the one the current `pr` gate was requested for. The
+   * last-entry semantics handle SPIR/PIR checkpoint workflows where an earlier
+   * PR merged but a later PR is still open and awaiting review.
+   *
+   * `derivePrReady` consults this so a merged-but-gate-still-pending builder
+   * (porch left the gate pending after an out-of-band merge — see #966) no
+   * longer reads as a PR awaiting human review. Defaults to `false` when
+   * `pr_history` is absent or its last entry has no `merged: true`.
+   */
+  merged: boolean;
 }
 
 /**
@@ -215,10 +228,11 @@ export function parseStatusYaml(content: string): ParsedStatus {
     planPhases: [],
     startedAt: '',
     prReadyForHuman: null,
+    merged: false,
   };
 
   const lines = content.split('\n');
-  let section: 'none' | 'gates' | 'plan_phases' = 'none';
+  let section: 'none' | 'gates' | 'plan_phases' | 'pr_history' = 'none';
   let currentGate = '';
   let currentPlanPhase: Partial<PlanPhase> | null = null;
 
@@ -261,6 +275,12 @@ export function parseStatusYaml(content: string): ParsedStatus {
       continue;
     }
 
+    if (/^pr_history:\s*$/.test(line)) {
+      if (currentPlanPhase) { pushPlanPhase(result, currentPlanPhase); currentPlanPhase = null; }
+      section = 'pr_history';
+      continue;
+    }
+
     // Stop section at next top-level key
     if (/^\S/.test(line) && line.trim() !== '') {
       if (currentPlanPhase) { pushPlanPhase(result, currentPlanPhase); currentPlanPhase = null; }
@@ -318,6 +338,30 @@ export function parseStatusYaml(content: string): ParsedStatus {
         continue;
       }
     }
+
+    // pr_history section (#966). Each list item is one PR/stage. We only need the
+    // merged status of the LAST entry (the PR the current `pr` gate was requested
+    // for). On a new list item we reset to its default (`false`); a `merged: true`
+    // line within the entry flips it. The final value therefore reflects the last
+    // entry — so an earlier merged checkpoint PR followed by a later still-open PR
+    // correctly reads as not-merged.
+    if (section === 'pr_history') {
+      const itemStartMatch = line.match(/^\s{2}-\s+(.*)$/);
+      if (itemStartMatch) {
+        result.merged = false;
+        // The dash line carries the entry's first key/value; handle the (unusual)
+        // case where that first key is `merged` itself.
+        const firstKv = itemStartMatch[1].match(/^merged:\s*(true|false)\s*$/);
+        if (firstKv) result.merged = firstKv[1] === 'true';
+        continue;
+      }
+
+      const mergedMatch = line.match(/^\s{4}merged:\s*(true|false)\s*$/);
+      if (mergedMatch) {
+        result.merged = mergedMatch[1] === 'true';
+        continue;
+      }
+    }
   }
 
   // Flush last plan phase if we were in that section
@@ -504,9 +548,18 @@ export function detectBlockedSince(parsed: ParsedStatus): string | null {
  * rollback hazard from #919) nor the old `bugfix && phase === 'verified'`
  * fallback (a crutch for a gateless BUGFIX variant — gateless PR-producing
  * protocols do not surface PR rows, by design).
+ *
+ * The `!parsed.merged` conjunct (#966) handles the case where porch left the
+ * `pr` gate `pending` after the PR already merged (an out-of-band GitHub merge
+ * plus a resume can scramble the ordering). A merged PR lives in
+ * `recentlyClosed`, never `pendingPRs`, so a stale `prReady: true` would
+ * suppress the builder's row in NeedsAttentionList without ever emitting a PR
+ * row — making the builder vanish. Once merged, it is no longer "a PR awaiting
+ * human review": derivePrReady returns false, and the still-pending `pr` gate
+ * surfaces the builder via the gate-row path instead.
  */
 export function derivePrReady(parsed: ParsedStatus): boolean {
-  return parsed.gates['pr'] === 'pending' && !!parsed.gateRequestedAt['pr'];
+  return parsed.gates['pr'] === 'pending' && !!parsed.gateRequestedAt['pr'] && !parsed.merged;
 }
 
 /**