Skip to content
Merged
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
id: bugfix-903
title: porch-done-is-not-idempotent-o
protocol: bugfix
phase: pr
plan_phases: []
current_plan_phase: null
gates:
pr:
status: approved
requested_at: '2026-05-28T02:23:35.865Z'
approved_at: '2026-05-28T02:36:20.117Z'
iteration: 1
build_complete: false
history: []
started_at: '2026-05-28T02:09:33.028Z'
updated_at: '2026-05-28T02:36:20.117Z'
pr_history:
- phase: fix
pr_number: 904
branch: builder/bugfix-903
created_at: '2026-05-28T02:18:03.431Z'
- phase: fix
pr_number: 904
branch: builder/bugfix-903
created_at: '2026-05-28T02:18:15.419Z'
pr_ready_for_human: false
21 changes: 21 additions & 0 deletions codev/state/bugfix-903_thread.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# bugfix-903 thread

## Investigate → Fix

Architect confirmed root cause + decided fix (early-exit on `state.phase === 'verified'`
after record-only handlers, before protocol loading). Implemented as directed.

**Fix** — `packages/codev/src/commands/porch/index.ts` `done()`: 8-line early-exit
inserted after the `--merged` handler. Prints "Project <id> already verified — nothing
to do." in dim and returns. No state write, no commit. Record-only `--pr` / `--merged`
short-circuit before this guard so they continue to work on verified projects.

**Regression tests** — added 2 cases to `done-verification.test.ts`:
1. `phase: 'verified'` + `done()` → status.yaml byte-identical, output contains
"already verified", no "PROTOCOL COMPLETE" banner.
2. `phase: 'verified'` + `done(... { pr: 42, branch })` → pr_history recorded,
phase still 'verified'.

All 14 tests in the file pass. Unrelated workspace test failures observed
(harness-integration, session-manager) are pre-existing — `@cluesmith/codev-core`
package is not built in the worktree; not caused by this fix.
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,57 @@ describe('porch done — verification enforcement', () => {
expect(updated.gates['verify-approval'].requested_at).toBeDefined();
});

// --------------------------------------------------------------------------
// Test: porch done is idempotent on terminal 'verified' state (#903)
// --------------------------------------------------------------------------

it('is a no-op when state.phase is already verified (#903)', async () => {
const state = makeState({
phase: 'verified',
build_complete: true,
gates: {
'spec-approval': { status: 'approved' as const },
'plan-approval': { status: 'approved' as const },
'pr': { status: 'approved' as const },
},
});
setupState(testDir, state);

const statusPath = getStatusPath(testDir, '0001', 'test-feature');
const before = fs.readFileSync(statusPath, 'utf-8');

await done(testDir, '0001');

const after = fs.readFileSync(statusPath, 'utf-8');
// status.yaml must be byte-identical — no redundant write
expect(after).toBe(before);

const output = logSpy.mock.calls.map(c => c.join(' ')).join('\n');
expect(output).toContain('already verified');
// Must NOT print the "protocol complete" banner from advanceProtocolPhase
expect(output).not.toContain('PROTOCOL COMPLETE');
});

it('record-only --pr still works on verified projects (#903)', async () => {
const state = makeState({
phase: 'verified',
build_complete: true,
gates: {
'spec-approval': { status: 'approved' as const },
'plan-approval': { status: 'approved' as const },
'pr': { status: 'approved' as const },
},
});
setupState(testDir, state);

await done(testDir, '0001', undefined, { pr: 42, branch: 'feature/x' });

const updated = readState(getStatusPath(testDir, '0001', 'test-feature'));
expect(updated.pr_history).toBeDefined();
expect(updated.pr_history![0].pr_number).toBe(42);
expect(updated.phase).toBe('verified');
});

it('readState migrates phase complete to verified (backward compat)', () => {
const state = makeState({ phase: 'complete' as string });
const statusPath = getStatusPath(testDir, '0001', 'test-feature');
Expand Down
8 changes: 8 additions & 0 deletions packages/codev/src/commands/porch/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,14 @@ export async function done(workspaceRoot: string, projectId: string, resolver?:
console.log(chalk.green(`Marked PR #${options.merged} as merged.`));
return;
}

// Idempotency for terminal state: re-running `porch done` on an already-verified
// project must be a silent no-op, not a fresh state write + commit (#903).
if (state.phase === 'verified') {
console.log(chalk.dim(`Project ${state.id} already verified — nothing to do.`));
return;
}

const protocol = loadProtocol(workspaceRoot, state.protocol);
const overrides = loadCheckOverrides(workspaceRoot);
const phaseConfig = getPhaseConfig(protocol, state.phase);
Expand Down
Loading