Fix #1637: spawn open-PR collision guard counts merged PRs

codev/projects/bugfix-1637-afx-spawn-open-pr-collision-gu/status.yaml

@@ -0,0 +1,22 @@
+id: bugfix-1637
+title: afx-spawn-open-pr-collision-gu
+protocol: bugfix
+phase: verified
+plan_phases: []
+current_plan_phase: null
+gates:
+  pr:
+    status: approved
+    requested_at: '2026-09-10T02:46:18.290Z'
+    approved_at: '2026-09-11T10:21:55.540Z'
+iteration: 1
+build_complete: false
+history: []
+started_at: '2026-09-10T02:24:06.981Z'
+updated_at: '2026-09-11T10:22:00.571Z'
+pr_history:
+  - phase: pr
+    pr_number: 1673
+    branch: builder/bugfix-1637
+    created_at: '2026-09-10T02:40:32.738Z'
+pr_ready_for_human: false

codev/state/bugfix-1637_thread.md

@@ -0,0 +1,120 @@
+# bugfix-1637 — spawn open-PR collision guard counts merged PRs
+
+Issue #1637. Strict BUGFIX. Worktree: .builders/bugfix-1637.
+
+## Investigate (complete)
+
+**Symptom**: `afx spawn` refuses with "Found N open PR(s) referencing issue #X" when the
+referenced PRs are actually *merged*, forcing `--force`.
+
+**Root cause** — a #1619 (3.3.3) side effect:
+- Guard: `checkBugfixCollisions` in `packages/codev/src/agent-farm/commands/spawn-worktree.ts`
+  lines 590-607. It calls the `pr-search` forge concept and treats **every** result as an
+  open PR (`openPRs.length` → fatal).
+- `pr-search` was changed to `--state all` (github) / `--all` (gitlab) by #1619 so
+  `consult --type pr` finds merged PRs (#759). Correct for consult; the guard inherited
+  merged/closed PRs into its "open" count.
+- Compounding: `scripts/forge/github/pr-search.sh` outputs only
+  `number,headRefName,baseRefName` — **no `state`** — so the guard can't filter client-side.
+
+**Two consumers, opposite needs** (pinned by a paired regression test):
+- consult `findPRForIssue` (consult/index.ts:2055) — wants `--state all`; takes first result.
+- spawn guard — must ignore non-OPEN.
+
+**Fix plan** (scope ~small, fits BUGFIX):
+1. `scripts/forge/github/pr-search.sh`: add `state` to `--json` fields (gh → OPEN/MERGED/CLOSED).
+2. `scripts/forge/gitlab/pr-search.sh`: add `state`, normalize glab's lowercase
+   `opened`→`OPEN` via jq to match github convention.
+3. `PrSearchItem` contract (forge-contracts.ts:149): add optional `state?: string`.
+4. Guard: filter to items that are OPEN **or missing state** (conservative — legacy overrides
+   without `state` keep pre-fix behavior; existing test at spawn-worktree.test.ts:642 passes an
+   item with no `state` and expects a fatal, so "missing = assume open" is required for
+   backward-compat). Keep `--state all` in scripts — consult needs it, the guard filters.
+5. Regression test pairing both consumers: guard ignores MERGED/CLOSED, consult still sees them.
+
+Scripts ship via `packages/codev/package.json` `files: [scripts/forge]`; no codev-skeleton copy.
+
+## Fix (complete)
+
+Implemented exactly the 5-point plan:
+- github/pr-search.sh: `--json …,state` (gh → OPEN/MERGED/CLOSED).
+- gitlab/pr-search.sh: jq normalizes glab's lowercase `opened`→`OPEN` (missing → "").
+- forge-contracts.ts: `PrSearchItem.state?: string`, documented as optional/normalized.
+- spawn-worktree.ts guard: `filter(pr => pr.state === undefined || pr.state === 'OPEN')`;
+  missing state kept as "assume open" (conservative — existing test passes a stateless item
+  and expects a fatal).
+- Tests: 4 new guard cases in spawn-worktree.test.ts (merged ignored, open fatals, mixed
+  counts only open, missing-state still fatals) + 2 script-contract cases in
+  bugfix-759-pr-search-state-all.test.ts (github emits state; gitlab maps to OPEN).
+
+**Verification**:
+- Regression proven: temporarily reverted the filter → the merged-ignored and mixed-count
+  tests FAIL; restored → all 4 PASS.
+- tsc --noEmit clean. Full `pnpm build` clean (skeleton + dashboard copy).
+- Full non-e2e suite from packages/codev: 6059 passed, 0 failed. Remaining failures are all
+  `.e2e.test.ts`/tower-shellper integration needing live services — pre-existing, environmental,
+  none touch forge/pr-search/spawn.
+
+NOTE on running tests: run vitest FROM packages/codev (the vitest-setup.ts sandbox-pin path is
+cwd-relative); a root-level `npx vitest packages/codev` spuriously fails the isolation/metrics
+tests because the setup file doesn't resolve.
+
+## PR (complete)
+
+PR #1673 opened (`Fixes #1637`), recorded with porch. Thread committed.
+
+CMAP (all scoped with `--project-id bugfix-1637`; bare invocation hit "Multiple
+projects found"):
+- gemini = APPROVE (HIGH, no key issues)
+- codex  = APPROVE (HIGH, no key issues)
+- claude = APPROVE (HIGH) — 2 non-blocking nits addressed in commit a3b072ab8:
+  (1) gitlab/pr-search.sh now captures glab output before piping to jq (#1645
+  convention) so a glab failure surfaces; (2) guard predicate broadened to
+  `!pr.state` so gitlab's empty-string state fallback is treated as open
+  (conservative), with a blank-state regression test.
+
+All three APPROVE. Handing off at the pr gate — awaiting architect approval
+before merge (CMAP APPROVE is not merge authorization).
+
+## CI fix (post-gate-request)
+
+Architect flagged real CI red on 83756b9c5: `forge.test.ts` #1645 check
+'no built-in provider resolves a concept to a shell builtin'. Cause: my CMAP
+follow-up (a3b072ab8) reshaped gitlab/pr-search.sh to capture-then-pipe, which
+hid `glab` from extractExecutable's first-line heuristic (it skips the `out=`
+assignment, lands on the `printf` builtin). Fix (a834106b1): added explicit
+`# forge-executable: glab` declaration, matching github/pr-list.sh.
+
+Lesson: after changing a forge SCRIPT's shape, re-run forge.test.ts — I only
+re-ran guard+script tests on the CMAP follow-up. Full non-e2e suite now 6060
+passing; forge.test.ts 80 passing. Awaiting CI 7/7 on a834106b1 before
+re-requesting the gate.
+
+## Provider audit + locked-state safety (owner-requested)
+
+Audited all four forge providers' pr-search resolution (verified empirically via
+resolveAllConcepts):
+- github -> github/pr-search.sh (gh: OPEN/CLOSED/MERGED) ✅
+- gitlab -> gitlab/pr-search.sh (jq normalization) ✅
+- gitea  -> DISABLED in preset; guard never calls pr-search, whole check skipped — no regression
+- linear -> falls through to github/pr-search.sh (Linear PRs live on GitHub) ✅
+
+Owner asked to lock in the gitlab `locked` safety change: `locked` (transient
+merging state) now maps to OPEN alongside `opened`, so the guard errs toward the
+recoverable --force prompt instead of silently skipping a live collision
+(commit 48df843c3).
+
+KNOWN pre-existing gap (NOT introduced here, flag for separate issue): gitlab's
+`glab mr list --output json` returns GitLab-shaped objects (iid/source_branch),
+not the contract's number/headRefName — so the guard's message reads
+`PR #undefined` on gitlab. forge.ts documents non-github presets as best-effort
+/ may-not-conform. State filtering works; field mapping is the older gap.
+
+## gitlab field-mapping fix (owner-requested "fix it now")
+
+Grounded the gitlab shape against the repo's own authority (gitlab/pr-list.sh
+comment + pr-exists.sh): glab emits iid/source_branch/target_branch, not
+number/headRefName/baseRefName. Fixed pr-search.sh to map all three in the jq
+pass (commit fced6d1a1), so the guard shows the real MR number and consult's
+findPRForIssue can resolve the base branch. Marked UNVERIFIED per #920 (no live
+glab in authoring/CI). CI baseline before this: 7/7 green on e292b1889.

packages/codev/scripts/forge/github/pr-search.sh

@@ -1,7 +1,9 @@
 #!/bin/sh
 # Forge concept: pr-search (GitHub via gh CLI)
 # Input: CODEV_SEARCH_QUERY
-# Output: JSON [{number, headRefName, baseRefName}]
+# Output: JSON [{number, headRefName, baseRefName, state}]
 # --state all is required so the search includes merged/closed PRs; without it
 # `gh pr list` defaults to --state open and post-merge lookups return nothing (#759).
-exec gh pr list --state all --search "$CODEV_SEARCH_QUERY" --json number,headRefName,baseRefName
+# `state` (OPEN/MERGED/CLOSED) lets consumers that only want open PRs filter
+# client-side — the spawn collision guard must ignore merged/closed ones (#1637).
+exec gh pr list --state all --search "$CODEV_SEARCH_QUERY" --json number,headRefName,baseRefName,state

packages/codev/scripts/forge/gitlab/pr-search.sh

@@ -1,5 +1,38 @@
 #!/bin/sh
-# Forge concept: pr-search (GitLab via glab CLI)
+# Forge concept: pr-search (GitLab via glab CLI — merge requests)
+# Output: JSON [{number, headRefName, baseRefName, state, ...glab fields}]
+#
+# ⚠️ UNVERIFIED — `glab` is not available in the authoring environment (#920);
+#    written against glab's documented `mr list --output json` shape (confirmed
+#    against the field mappings already documented in gitlab/pr-list.sh and
+#    gitlab/pr-exists.sh). Smoke-test before relying on it.
+#
 # --all is required so the search includes merged/closed MRs; without it
 # `glab mr list` defaults to opened only and post-merge lookups return nothing (#759).
-exec glab mr list --all --search "$CODEV_SEARCH_QUERY" --output json
+#
+# Maps glab's GitLab-shaped fields into the forge-neutral PrSearchItem contract
+# (forge-contracts.ts); `. + {…}` is non-destructive for keys we don't touch:
+#   iid           -> number        (the MR number glab mr view/merge expects)
+#   source_branch -> headRefName
+#   target_branch -> baseRefName
+# Without this the spawn guard's message reads `PR #undefined` and consult's
+# findPRForIssue can't resolve the MR's base branch on GitLab.
+#
+# glab reports state lowercase (opened/merged/closed/locked); normalize to the
+# GitHub convention (OPEN/MERGED/CLOSED) so consumers filter with one comparison.
+# The spawn collision guard keys off state === "OPEN" to ignore merged MRs (#1637).
+# `locked` (a transient merging state) maps to OPEN too: the guard should err
+# toward the recoverable --force prompt, never silently skip a live collision.
+#
+# Not `glab … | jq`: POSIX sh has no pipefail, so the caller would see jq's exit
+# status and a failed glab would look like a successful empty list (#1645).
+# The capture-then-pipe shape hides `glab` from extractExecutable's first-line
+# heuristic (it lands on the `printf` builtin), so declare the backend explicitly
+# — same as github/pr-list.sh (#1645).
+# forge-executable: glab
+out="$(glab mr list --all --search "$CODEV_SEARCH_QUERY" --output json)" || exit 1
+printf '%s' "$out" | jq 'map(.
+  + {number: .iid}
+  + {headRefName: .source_branch}
+  + {baseRefName: .target_branch}
+  + {state: ((.state // "") | ascii_downcase as $s | if $s == "opened" or $s == "locked" then "OPEN" else ($s | ascii_upcase) end)})'

packages/codev/src/agent-farm/__tests__/spawn-worktree.test.ts

@@ -650,6 +650,66 @@ describe('spawn-worktree', () => {
       );
     });
 
+    it('ignores merged PRs referencing the issue (#1637)', async () => {
+      // pr-search runs --state all (#759), so merged PRs come back too. The guard
+      // must not count them as open, else it blocks spawns on already-resolved work.
+      const { existsSync } = await import('node:fs');
+      vi.mocked(existsSync).mockReturnValueOnce(false);
+      executeForgeCommandMock.mockResolvedValueOnce([
+        { number: 4747, headRefName: 'fix-a', state: 'MERGED' },
+        { number: 4766, headRefName: 'fix-b', state: 'CLOSED' },
+      ]);
+      const { fatal } = await import('../utils/logger.js');
+      await expect(
+        checkBugfixCollisions(4750, '/tmp/wt', baseIssue, false),
+      ).resolves.toBeUndefined();
+      expect(fatal).not.toHaveBeenCalled();
+    });
+
+    it('still fatals for an open PR referencing the issue (#1637)', async () => {
+      const { existsSync } = await import('node:fs');
+      vi.mocked(existsSync).mockReturnValueOnce(false);
+      executeForgeCommandMock.mockResolvedValueOnce([{ number: 99, headRefName: 'fix-42', state: 'OPEN' }]);
+      const { fatal } = await import('../utils/logger.js');
+      await checkBugfixCollisions(42, '/tmp/wt', baseIssue, false);
+      expect(fatal).toHaveBeenCalledWith(expect.stringContaining('open PR'));
+    });
+
+    it('counts only the open PRs when results mix open and merged (#1637)', async () => {
+      const { existsSync } = await import('node:fs');
+      vi.mocked(existsSync).mockReturnValueOnce(false);
+      executeForgeCommandMock.mockResolvedValueOnce([
+        { number: 10, headRefName: 'merged', state: 'MERGED' },
+        { number: 11, headRefName: 'open', state: 'OPEN' },
+      ]);
+      const { fatal } = await import('../utils/logger.js');
+      await checkBugfixCollisions(42, '/tmp/wt', baseIssue, false);
+      expect(fatal).toHaveBeenCalledWith(expect.stringContaining('Found 1 open PR(s)'));
+    });
+
+    it('treats a missing state as open for stale pr-search overrides (#1637)', async () => {
+      // A project-local pr-search.sh predating the state field returns no state;
+      // the guard keeps the conservative pre-fix behavior rather than silently
+      // dropping the collision check.
+      const { existsSync } = await import('node:fs');
+      vi.mocked(existsSync).mockReturnValueOnce(false);
+      executeForgeCommandMock.mockResolvedValueOnce([{ number: 99, headRefName: 'fix-42' }]);
+      const { fatal } = await import('../utils/logger.js');
+      await checkBugfixCollisions(42, '/tmp/wt', baseIssue, false);
+      expect(fatal).toHaveBeenCalledWith(expect.stringContaining('open PR'));
+    });
+
+    it('treats a blank state as open (gitlab pr-search leaves it empty) (#1637)', async () => {
+      // gitlab/pr-search.sh maps a missing glab state to "" via jq; the guard must
+      // treat that the same as absent — conservatively counting it as open.
+      const { existsSync } = await import('node:fs');
+      vi.mocked(existsSync).mockReturnValueOnce(false);
+      executeForgeCommandMock.mockResolvedValueOnce([{ number: 99, headRefName: 'fix-42', state: '' }]);
+      const { fatal } = await import('../utils/logger.js');
+      await checkBugfixCollisions(42, '/tmp/wt', baseIssue, false);
+      expect(fatal).toHaveBeenCalledWith(expect.stringContaining('open PR'));
+    });
+
     it('warns when issue is already closed', async () => {
       const { existsSync } = await import('node:fs');
       vi.mocked(existsSync).mockReturnValueOnce(false);

packages/codev/src/agent-farm/commands/spawn-worktree.ts

@@ -587,20 +587,29 @@ export async function checkBugfixCollisions(
     }
   }
 
-  // 3. Check for open PRs referencing this issue via pr-search concept
+  // 3. Check for open PRs referencing this issue via pr-search concept.
+  // pr-search runs `--state all` (#759) so consult can find merged PRs; this guard
+  // must ignore merged/closed ones, else it blocks spawns on already-resolved work
+  // (#1637). Filter to state === "OPEN"; treat an absent/empty state (a stale
+  // pr-search.sh override predating the field, or a forge script that leaves it
+  // blank) as unknown → keep counting it, the conservative pre-#1637 behavior,
+  // with --force still available.
   try {
     const result = await executeForgeCommand('pr-search', {
       CODEV_SEARCH_QUERY: `in:body #${issueNumber}`,
     }, { forgeConfig });
-    if (result && Array.isArray(result) && result.length > 0) {
-      const openPRs = result as Array<{ number: number; title?: string; headRefName?: string }>;
-      if (!force) {
-        const prList = openPRs.slice(0, 5).map((pr) =>
-          `  - PR #${pr.number}${pr.title ? `: ${pr.title}` : ''}`,
-        ).join('\n');
-        fatal(`Found ${openPRs.length} open PR(s) referencing issue #${issueNumber}:\n${prList}\nUse --force to proceed anyway.`);
+    if (result && Array.isArray(result)) {
+      const prs = result as Array<{ number: number; title?: string; headRefName?: string; state?: string }>;
+      const openPRs = prs.filter((pr) => !pr.state || pr.state === 'OPEN');
+      if (openPRs.length > 0) {
+        if (!force) {
+          const prList = openPRs.slice(0, 5).map((pr) =>
+            `  - PR #${pr.number}${pr.title ? `: ${pr.title}` : ''}`,
+          ).join('\n');
+          fatal(`Found ${openPRs.length} open PR(s) referencing issue #${issueNumber}:\n${prList}\nUse --force to proceed anyway.`);
+        }
+        logger.warn(`Warning: Found ${openPRs.length} open PR(s) referencing issue - proceeding with --force`);
       }
-      logger.warn(`Warning: Found ${openPRs.length} open PR(s) referencing issue - proceeding with --force`);
     }
   } catch {
     // Non-fatal: continue if PR search concept unavailable

packages/codev/src/commands/porch/__tests__/bugfix-759-pr-search-state-all.test.ts

@@ -32,6 +32,14 @@ describe('pr-search forge scripts', () => {
       const content = fs.readFileSync(scriptPath, 'utf-8');
       expect(content).toContain('--search "$CODEV_SEARCH_QUERY"');
     });
+
+    it('emits state so the spawn guard can filter merged PRs (#1637)', () => {
+      // Paired-consumer contract: --state all keeps merged PRs visible for consult
+      // (#759), while the `state` field lets the spawn collision guard ignore them.
+      const content = fs.readFileSync(scriptPath, 'utf-8');
+      expect(content).toContain('--state all');
+      expect(content).toMatch(/--json\s+\S*\bstate\b/);
+    });
   });
 
   describe('gitlab/pr-search.sh', () => {
@@ -50,5 +58,26 @@ describe('pr-search forge scripts', () => {
       const content = fs.readFileSync(scriptPath, 'utf-8');
       expect(content).toContain('--search "$CODEV_SEARCH_QUERY"');
     });
+
+    it('normalizes opened/locked MRs to the OPEN state so the guard can filter (#1637)', () => {
+      // glab reports state lowercase; the script maps opened -> OPEN so the spawn
+      // collision guard's state === "OPEN" comparison is forge-agnostic. `locked`
+      // (a transient merging state) also maps to OPEN so the guard errs toward the
+      // recoverable --force prompt rather than silently skipping a live collision.
+      const content = fs.readFileSync(scriptPath, 'utf-8');
+      expect(content).toContain('--all');
+      expect(content).toContain('"OPEN"');
+      expect(content).toMatch(/"opened"\s+or\s+\$s\s*==\s*"locked"/);
+    });
+
+    it('maps glab fields to the PrSearchItem contract (#1637)', () => {
+      // glab emits iid/source_branch/target_branch, not number/headRefName/
+      // baseRefName. Without the mapping the spawn guard prints `PR #undefined`
+      // and consult can't resolve the MR base branch.
+      const content = fs.readFileSync(scriptPath, 'utf-8');
+      expect(content).toContain('number: .iid');
+      expect(content).toContain('headRefName: .source_branch');
+      expect(content).toContain('baseRefName: .target_branch');
+    });
   });
 });

packages/codev/src/lib/forge-contracts.ts

@@ -149,6 +149,16 @@ export type RecentlyMergedResult = MergedPrItem[];
 export interface PrSearchItem {
   number: number;
   headRefName: string;
+  /**
+   * PR/MR state, normalized to the GitHub convention: `OPEN` | `MERGED` |
+   * `CLOSED`. Because `pr-search` runs `--state all` (#759), consumers that
+   * only want open PRs must filter on this — the spawn collision guard ignores
+   * non-open results (#1637). Optional: a stale project-local override of
+   * `pr-search.sh` may predate this field (and the gitlab script leaves it
+   * blank when glab omits it), so consumers treat an absent or empty value as
+   * "unknown" rather than assuming a state.
+   */
+  state?: string;
 }
 
 /** Output of the `pr-search` concept command. */