Skip to content

porch done is not idempotent on 'verified' state — writes redundant status.yaml + commit #903

@mohidmakhdoomi

Description

@mohidmakhdoomi

Bug

Running porch done <id> when status.yaml is already in the verified terminal state performs a redundant write to status.yaml and creates a redundant git commit (chore(porch): <id> protocol complete).

porch done should be idempotent on terminal states: re-running it on an already-verified project should be a silent no-op, not a fresh state mutation.

Root cause

packages/codev/src/commands/porch/index.ts:334-517 (done() function) has no early-exit when state.phase === 'verified'. The function falls through every check (no phase config, no gate, not phased, not build_verify) and reaches advanceProtocolPhase() at line 519.

In advanceProtocolPhase() (lines 519-528), getNextPhase(protocol, 'verified') returns null because verified is a terminal pseudo-phase not present in protocol.phases. The if (!nextPhase) branch then unconditionally re-assigns state.phase = 'verified' (it already was) and calls writeStateAndCommit(...) with message ```chore(porch): ${state.id} protocol complete``` — producing the redundant write and commit.

For comparison, the rollback handler at index.ts:813 already treats 'verified' and 'complete' as special terminal states. The same recognition is missing from done().

Reproduction

  1. Run any protocol to completion so status.yaml reaches phase: verified
  2. Run `porch done ` again
  3. Observe a new commit: `chore(porch): protocol complete` and a touched `status.yaml` (mtime updated, content unchanged)

Expected behavior

  • Bare `porch done ` on a project whose `state.phase === 'verified'` should print a brief message (e.g. "Project already verified — nothing to do."), exit 0, and perform no state write and no git commit.
  • Record-only modes (`--pr` and `--merged`) must continue to work on verified projects — recording PR metadata after a project completes is a legitimate use case.

Fix scope

  • Add an early-exit at the top of `done()` in `packages/codev/src/commands/porch/index.ts` (after the record-only handlers at lines 344-366, before phase-check loading at line 367) that detects `state.phase === 'verified'`, prints a friendly message, and returns without writing state.
  • Add a regression test to `packages/codev/src/commands/porch/tests/done-verification.test.ts` that constructs a `verified` state, calls `done()`, and asserts no `writeStateAndCommit` invocation occurred.

Out of scope

  • Auditing `porch next` for similar idempotency gaps (separate concern).
  • Changing behavior of `'complete'` state (the user-confirmed scope is `'verified'` only).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions