Skip to content
Merged
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
id: bugfix-113
title: porch-verify-approval-cannot-p
protocol: bugfix
phase: pr
plan_phases: []
current_plan_phase: null
gates:
pr:
status: approved
requested_at: '2026-08-24T23:03:47.476Z'
approved_at: '2026-08-24T23:19:14.798Z'
iteration: 1
build_complete: false
history: []
started_at: '2026-08-24T22:29:49.851Z'
updated_at: '2026-08-24T23:19:14.799Z'
pr_history:
- phase: pr
pr_number: 137
branch: builder/bugfix-113
created_at: '2026-08-24T22:58:03.624Z'
pr_ready_for_human: false
21 changes: 21 additions & 0 deletions codev/state/bugfix-113_thread.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# bugfix-113 thread

Investigate (2026-08-24).

Reproduced. `porch check 83` from this worktree fails `pr_exists` in 0.6s. Same command from main passes only because `findStatusPath` hits `.builders/air-106`, whose unrelated merged PR #132 satisfies the check.

Project 83 is still in `review`. `pr` is approved. PR #104 is MERGED. `verify-approval` is pending. The builder never ran `porch done` after the pr gate, so verify was never entered.

`porch approve 83 verify-approval` runs checks for `state.phase` (review), not for the phase that owns the gate. Review's first check is `pr_exists`, which asks `git branch --show-current` in whichever worktree owns the status.yaml. After merge that is almost never `builder/spir-83`.

The issue title's premise is wrong. `pr_exists` already uses `--state all`, so a merged PR matches (#568 / #16 / upstream #1331). The real cause is the `--head` argument: `git branch --show-current` runs in whichever worktree `findStatusPath` returns, and after merge that is almost never the PR head. The search is for the wrong head, not the wrong state. Against `CODEV_BRANCH_NAME=builder/spir-83` the script returns true.

Verify phase has no checks. Scope is small: approve the named gate's phase checks, and if verify-approval is requested while still in review with `pr` already approved, enter verify first so the existing auto-advance can reach `verified`.

Fix: `approve()` enters verify when verify-approval is requested from review with `pr` already approved. Review's `pr_exists` no longer runs. Regression test fails without the block (process.exit from `false` pr_exists) and passes with it.

PR #137: https://github.com/pseudoseed/codev/pull/137

CMAP: gemini skipped (agy exit 1, quota). Substitute opencode=APPROVE. codex=COMMENT (branch trails main by a docs merge, no code change). claude=APPROVE. No REQUEST_CHANGES.

Claude noted a residual: `porch done` / `porch check` still fail review's `pr_exists` after merge when cwd is not the PR head. Left as a follow-up; this bugfix unblocks verify-approval.
Original file line number Diff line number Diff line change
@@ -0,0 +1,156 @@
/**
* Regression test for issue #113: porch approve verify-approval cannot pass
* after a normal merge because it re-runs review's pr_exists.
*
* Reproduction:
* 1. SPIR project is still in review. The pr gate is already approved.
* 2. The PR is merged (closed). That is how you get to verify-approval.
* 3. `porch approve <id> verify-approval` runs checks for state.phase
* (review), whose first check is pr_exists.
* 4. pr_exists keys off `git branch --show-current` in whichever worktree
* findStatusPath returns. After merge that is almost never the PR head,
* so the check fails and the gate cannot be approved.
*
* A test that stops at the pr gate cannot catch this.
*/

import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
import * as fs from 'node:fs';
import * as path from 'node:path';
import { tmpdir } from 'node:os';
import { approve } from '../index.js';
import { writeState, getStatusPath, readState } from '../state.js';
import type { ProjectState } from '../types.js';

function createTestDir(): string {
const dir = path.join(tmpdir(), `porch-113-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`);
fs.mkdirSync(dir, { recursive: true });
return dir;
}

function setupProtocol(testDir: string, protocol: object): void {
const protocolDir = path.join(testDir, 'codev', 'protocols', 'spir');
fs.mkdirSync(protocolDir, { recursive: true });
fs.writeFileSync(path.join(protocolDir, 'protocol.json'), JSON.stringify(protocol, null, 2));
}

function setupState(testDir: string, state: ProjectState): string {
const statusPath = getStatusPath(testDir, state.id, state.title);
fs.mkdirSync(path.dirname(statusPath), { recursive: true });
writeState(statusPath, state);
return statusPath;
}

function makeState(overrides: Partial<ProjectState> = {}): ProjectState {
return {
id: '83',
title: 'merged-pr-project',
protocol: 'spir',
phase: 'review',
plan_phases: [],
current_plan_phase: null,
gates: {
'spec-approval': { status: 'approved', approved_at: new Date().toISOString() },
'plan-approval': { status: 'approved', approved_at: new Date().toISOString() },
pr: { status: 'approved', approved_at: new Date().toISOString() },
'verify-approval': { status: 'pending' },
},
iteration: 1,
build_complete: true,
history: [],
started_at: new Date().toISOString(),
updated_at: new Date().toISOString(),
...overrides,
};
}

// Review's pr_exists is a guaranteed fail. Without the fix, approve
// verify-approval runs this check and cannot pass. With the fix, approve
// enters verify first and never runs it.
const spirProtocol = {
name: 'spir',
version: '1.0.0',
phases: [
{
id: 'review',
name: 'Review',
type: 'once',
checks: {
pr_exists: {
command: 'false',
description: 'Would fail after merge when the current branch is not the PR head',
},
},
gate: 'pr',
next: 'verify',
},
{
id: 'verify',
name: 'Verify',
type: 'once',
gate: 'verify-approval',
next: null,
},
],
};

describe('bugfix #113 — verify-approval after merged PR', () => {
let testDir: string;
let exitSpy: ReturnType<typeof vi.spyOn>;
let logSpy: ReturnType<typeof vi.spyOn>;

beforeEach(() => {
testDir = createTestDir();
setupProtocol(testDir, spirProtocol);
exitSpy = vi.spyOn(process, 'exit').mockImplementation((code?: number) => {
throw new Error(`process.exit(${code})`);
});
logSpy = vi.spyOn(console, 'log').mockImplementation(() => {});
});

afterEach(() => {
fs.rmSync(testDir, { recursive: true, force: true });
exitSpy.mockRestore();
logSpy.mockRestore();
});

it('approve verify-approval succeeds after pr is approved even when pr_exists would fail', async () => {
const statusPath = setupState(testDir, makeState());

await approve(testDir, '83', 'verify-approval', true);

const updated = readState(statusPath);
expect(updated.gates['verify-approval'].status).toBe('approved');
expect(updated.gates['verify-approval'].approved_at).toBeDefined();
expect(updated.phase).toBe('verified');
});

it('approve verify-approval refuses when the pr gate is not approved', async () => {
const statusPath = setupState(testDir, makeState({
gates: {
'spec-approval': { status: 'approved', approved_at: new Date().toISOString() },
'plan-approval': { status: 'approved', approved_at: new Date().toISOString() },
pr: { status: 'pending' },
'verify-approval': { status: 'pending' },
},
}));

await expect(approve(testDir, '83', 'verify-approval', true)).rejects.toThrow(
/pr gate must be approved first/,
);

const updated = readState(statusPath);
expect(updated.gates['verify-approval'].status).toBe('pending');
expect(updated.phase).toBe('review');
});

it('approve verify-approval still works when already in verify', async () => {
const statusPath = setupState(testDir, makeState({ phase: 'verify' }));

await approve(testDir, '83', 'verify-approval', true);

const updated = readState(statusPath);
expect(updated.gates['verify-approval'].status).toBe('approved');
expect(updated.phase).toBe('verified');
});
});
21 changes: 21 additions & 0 deletions packages/codev/src/commands/porch/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -874,6 +874,27 @@ export async function approve(

// Run phase checks before approving
const protocol = loadProtocol(workspaceRoot, state.protocol);

// Issue #113: verify-approval is the post-merge gate. After a normal merge
// the project is still in review (porch done after the pr gate is a separate
// step). Approving this gate must not re-run review's pr_exists — that check
// keys off git branch --show-current in whichever worktree findStatusPath
// returns, which after merge is almost never the PR head. Enter verify first
// so the checks below are the verify phase's (none) and the existing
// auto-advance can reach verified.
if (gateName === 'verify-approval' && state.phase !== 'verify') {
if (state.phase === 'review' && state.gates['pr']?.status === 'approved') {
state.phase = 'verify';
state.build_complete = true;
await writeStateAndCommit(statusPath, state, `chore(porch): ${state.id} verify phase-transition (verify-approval)`);
} else {
throw new Error(
`Cannot approve verify-approval from phase '${state.phase}'. ` +
`The pr gate must be approved first.`,
);
}
}

const overrides = loadCheckOverrides(workspaceRoot, state.protocol);
const phaseConfig = getPhaseConfig(protocol, state.phase);
const phaseCheckNames = phaseConfig?.checks ?? [];
Expand Down
Loading