diff --git a/codev/projects/bugfix-903-porch-done-is-not-idempotent-o/status.yaml b/codev/projects/bugfix-903-porch-done-is-not-idempotent-o/status.yaml new file mode 100644 index 000000000..cf7255184 --- /dev/null +++ b/codev/projects/bugfix-903-porch-done-is-not-idempotent-o/status.yaml @@ -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 diff --git a/codev/state/bugfix-903_thread.md b/codev/state/bugfix-903_thread.md new file mode 100644 index 000000000..401db85bb --- /dev/null +++ b/codev/state/bugfix-903_thread.md @@ -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 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. diff --git a/packages/codev/src/commands/porch/__tests__/done-verification.test.ts b/packages/codev/src/commands/porch/__tests__/done-verification.test.ts index 861f98713..2a4e51c9e 100644 --- a/packages/codev/src/commands/porch/__tests__/done-verification.test.ts +++ b/packages/codev/src/commands/porch/__tests__/done-verification.test.ts @@ -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'); diff --git a/packages/codev/src/commands/porch/index.ts b/packages/codev/src/commands/porch/index.ts index 375091c7b..932773b11 100644 --- a/packages/codev/src/commands/porch/index.ts +++ b/packages/codev/src/commands/porch/index.ts @@ -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);