A re-run of an executed plan is unguarded and overwrites its own audit record #195

New issue

Closed

opened 2026-08-16 16:19:39 +00:00 by john · 0 comments

john commented

2026-08-16 16:19:39 +00:00

Owner

The defect

Re-running a plan destroys the record of the previous run, and nothing stops the
re-run from happening by accident.

Two halves, one fix site each.

1. `locate` does not refuse an already-executed phase

cadence-core/workflows/execute.md's locate step stops on unplanned and on
missing plan files, and on nothing else. A phase whose cursor status is
executed is dispatched again exactly like a fresh one: a new executor starts at
task 1 against a plan whose tasks are already committed.

The within-run protections do not cover this. The partial and timeout arms
read reports/plan-<k>.md for completed task numbers specifically so a
continuation cannot repeat a finished task, and that reasoning applies just as
well across runs. It is simply not applied there.

2. The report file is overwritten in place

skills/cad-executor-contract/SKILL.md:74 has the executor rewrite
<plandir>/reports/plan-<k>.md after every task commit, and :197 fixes the
path at plan-<k>.md with no run component. So the second run's first task
commit overwrites the first run's report before anything has read it.

That report is the only per-task record of what was run and what it printed. The
SUMMARY manifest survives (it is a separate file), but the evidence behind each
row does not, which is precisely the artifact you would want when comparing two
runs of the same plan.

Why it matters

The audit trail is otherwise sufficient to reconstruct a run: SUMMARY maps task
to hash, the report carries the verify command and its output per commit, the
risk-check record is keyed to the run, and the trace brackets carry the token
figure. Every one of those is run-scoped or append-only EXCEPT the report, which
is the most detailed of them.

The correlation id already exists and is already unique per run - it derives from
PHASE_START (2-392ae60 for the run that closed phase 2), so the scoping key is
in hand and costs nothing to adopt.

The fix

locate refuses a phase whose cursor status is executed, naming the
supported path (/cad-undo <N> then /cad-execute <N>) rather than silently
re-executing. An explicit override flag is a reasonable escape hatch; a silent
re-run is not.
Report files become run-scoped. Either reports/<corr>/plan-<k>.md or
reports/plan-<k>.<corr>.md, with the readers that already derive the path
(the summary step, the continuation branches, /cad-undo's manifest
fallback) deriving the same one.

Blast radius

The path is derived in cadence-core/workflows/execute.md at three read sites and
declared in skills/cad-executor-contract/SKILL.md at two. Any change has to move
all five together, and the executor contract is a preloaded surface under a weight
budget, so the shorter spelling is the cheaper one.

Watched failure before the fix

Execute a phase, note the report contents, execute the same phase again, and show
the report replaced with a run that re-did committed work. That is a two-command
reproduction and it should be the failing case committed first.

## The defect Re-running a plan destroys the record of the previous run, and nothing stops the re-run from happening by accident. Two halves, one fix site each. ### 1. `locate` does not refuse an already-executed phase `cadence-core/workflows/execute.md`'s `locate` step stops on `unplanned` and on missing plan files, and on nothing else. A phase whose cursor status is `executed` is dispatched again exactly like a fresh one: a new executor starts at task 1 against a plan whose tasks are already committed. The within-run protections do not cover this. The `partial` and `timeout` arms read `reports/plan-<k>.md` for completed task numbers specifically so a continuation cannot repeat a finished task, and that reasoning applies just as well across runs. It is simply not applied there. ### 2. The report file is overwritten in place `skills/cad-executor-contract/SKILL.md:74` has the executor rewrite `<plandir>/reports/plan-<k>.md` after every task commit, and `:197` fixes the path at `plan-<k>.md` with no run component. So the second run's first task commit overwrites the first run's report before anything has read it. That report is the only per-task record of what was run and what it printed. The SUMMARY manifest survives (it is a separate file), but the evidence behind each row does not, which is precisely the artifact you would want when comparing two runs of the same plan. ## Why it matters The audit trail is otherwise sufficient to reconstruct a run: SUMMARY maps task to hash, the report carries the verify command and its output per commit, the `risk-check` record is keyed to the run, and the trace brackets carry the token figure. Every one of those is run-scoped or append-only EXCEPT the report, which is the most detailed of them. The correlation id already exists and is already unique per run - it derives from PHASE_START (`2-392ae60` for the run that closed phase 2), so the scoping key is in hand and costs nothing to adopt. ## The fix - `locate` refuses a phase whose cursor status is `executed`, naming the supported path (`/cad-undo <N>` then `/cad-execute <N>`) rather than silently re-executing. An explicit override flag is a reasonable escape hatch; a silent re-run is not. - Report files become run-scoped. Either `reports/<corr>/plan-<k>.md` or `reports/plan-<k>.<corr>.md`, with the readers that already derive the path (the `summary` step, the continuation branches, `/cad-undo`'s manifest fallback) deriving the same one. ## Blast radius The path is derived in `cadence-core/workflows/execute.md` at three read sites and declared in `skills/cad-executor-contract/SKILL.md` at two. Any change has to move all five together, and the executor contract is a preloaded surface under a weight budget, so the shorter spelling is the cheaper one. ## Watched failure before the fix Execute a phase, note the report contents, execute the same phase again, and show the report replaced with a run that re-did committed work. That is a two-command reproduction and it should be the failing case committed first.