Skip to content

Commit 9ad54ea

Browse files
Adopt the step identities the store issued
The run's step identities and the store's are different vocabularies, paired by position. The pairing read the plan built to *send* to the store rather than the one the store *returned*. Those are the same object on an ordinary write and need not be on an idempotent one, where the store answers with the revision it already had — and the identities the run then adopts are ones the store never issued. Every later read of such a step is refused with "step does not belong to run plan": a true sentence about a situation nothing else reports, naming neither the step nor the cause. It cost two entire ladder passes before the widened stage trace made it legible at all. Pairing now reads recorded.Plan.Steps, keeps the local build only as a fallback for a store that echoes nothing, and traces the mismatch when the lengths disagree. Together with the earlier change that drops an unpaired step rather than keeping it, a run carries only steps the store knows about. Unproven against the ladder: the OpenAI account has no credits remaining and every run 429s at about fifteen seconds. It explains the symptom and the message exactly, and the three tests cover the rule without needing a store. Change-Log: CL-20260803-178 Dev-Log: DL-20260803-188 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 622d8c2 commit 9ad54ea

4 files changed

Lines changed: 186 additions & 5 deletions

File tree

CHANGELOG

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,40 @@ Dev-Log:
5050

5151
Entries
5252
-------
53-
Change-ID: CL-20260803-177
53+
Change-ID: CL-20260803-178
5454
Commit: pending
5555
Date: 2026-08-04
56+
Type: Fix -- LAD-003. The run adopted step identities it generated itself and
57+
assumed the store had written the same ones
58+
Request-or-TODO: LAD-003
59+
Outcome: The run's step identities and the store's are different vocabularies,
60+
paired by position. The pairing read the plan built to send to the store
61+
rather than the one the store returned. Those are the same object on an
62+
ordinary write and need not be on an idempotent one, where the store answers
63+
with the revision it already had -- and the identities the run then adopts are
64+
ones the store never issued. Every later read of such a step is refused with
65+
"load plan step state: database constraint: step does not belong to run plan",
66+
which is a true sentence about a situation nothing else reports and which
67+
names neither the step nor the cause. It cost two entire ladder passes on
68+
2026-08-04 before the widened stage trace made it legible at all. Pairing now
69+
reads recorded.Plan.Steps, falls back to the local build only when the store
70+
echoes nothing, and traces the mismatch when the two lengths disagree.
71+
Together with CL-20260803-174, which drops an unpaired step rather than
72+
keeping it, a run now carries only steps the store knows about.
73+
Affected-behavior: A run adopts the identities the store issued. A mismatch is
74+
visible in the trace instead of surfacing as a constraint violation three
75+
attempts later.
76+
Compatibility-or-migration: None.
77+
Verification: Three new tests over the extracted pairing: identities come from
78+
the store, a short recorded plan leaves the extra steps unpaired and adoption
79+
then drops them, and an empty plan pairs nothing rather than half of
80+
something. The ladder cannot confirm this today -- the OpenAI account has no
81+
credits remaining and every run 429s at about fifteen seconds.
82+
Dev-Log: DL-20260803-188
83+
84+
Change-ID: CL-20260803-177
85+
Commit: 622d8c2
86+
Date: 2026-08-04
5687
Type: Fix -- a hard gate deferred itself behind every softer one and was never
5788
reached
5889
Request-or-TODO: Ladder rung 19

DEVLOG

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,31 @@ Next-safe-step:
2525

2626
Entries
2727
-------
28+
Dev-Log: DL-20260803-188
29+
Date: 2026-08-04
30+
Status: Committed as CL-20260803-178
31+
Change-ID: CL-20260803-178
32+
Request-or-TODO: LAD-003
33+
Goal: Make the run adopt identities the store actually issued
34+
Assumptions: That recorded.Plan carries the stored steps. Checked: PlanRevision
35+
declares a Plan AgentPlan field, which is the plan as the store holds it
36+
Decisions: Kept the local build as a fallback for a store that echoes no plan,
37+
because removing it would turn a degraded write into no mapping at all. Traced
38+
rather than refused on a length mismatch, because the caller already handles a
39+
missing pairing by dropping the step and this should not become a second way
40+
to end a run
41+
Files-or-schemas: internal/coordinator/agent_plan_record.go --
42+
pairStepIdentities extracted so the rule can be tested without a store
43+
Validation: Three new tests. The pre-existing PIPE048 failure is unchanged and
44+
is LAD-002's, not this
45+
Failures-or-discarded-approaches: None here, though this took three earlier
46+
wrong readings to reach, all recorded in LAD-003
47+
Known-limitations: Unproven against the ladder. It explains the symptom and the
48+
message exactly, and the run that produced them cannot be repeated until the
49+
account has credits
50+
Next-safe-step: Rerun rung 19 once credits are restored, and watch for the new
51+
"the recorded plan holds N step(s)" line
52+
2853
Dev-Log: DL-20260803-187
2954
Date: 2026-08-04
3055
Status: Committed as CL-20260803-177

internal/coordinator/agent_plan_record.go

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -137,13 +137,53 @@ func (execution *AgentExecution) recordDurablePlan(
137137
// The loop's step identities and the durable plan's are different
138138
// vocabularies, so they are paired by position. A run attributing its work
139139
// to a step the plan does not have would be worse than no attribution.
140-
result := durablePlan{Revision: recorded.Revision, Steps: map[string]string{}}
140+
//
141+
// Paired against what the store returned, not against the plan built to
142+
// send it. Those are the same object on an ordinary write and need not be
143+
// on an idempotent one, where the store answers with the revision it
144+
// already had — and the identities the run then adopts would be ones the
145+
// store never issued. Every later read of such a step is refused with
146+
// "step does not belong to run plan", which is a true sentence about a
147+
// situation nothing else reports and which names neither the step nor the
148+
// cause. See LAD-003.
149+
durableSteps := recorded.Plan.Steps
150+
if len(durableSteps) == 0 {
151+
// A store that does not echo the plan back leaves nothing to pair
152+
// against; the locally built one is all there is, and using it is what
153+
// this has always done.
154+
durableSteps = plan.Steps
155+
}
156+
return durablePlan{
157+
Revision: recorded.Revision,
158+
Steps: pairStepIdentities(steps, durableSteps),
159+
}, nil
160+
}
161+
162+
// pairStepIdentities maps each of the run's step identities onto the durable
163+
// one at the same position.
164+
//
165+
// By position, because the two are different vocabularies with nothing in
166+
// common to join on. That is safe only while the two lists describe the same
167+
// steps in the same order, which is true of a plan the store has just written
168+
// and is worth saying out loud when the lengths disagree: a step with no
169+
// durable identity is one every later read is refused for, and the refusal —
170+
// "step does not belong to run plan" — names neither the step nor the reason.
171+
func pairStepIdentities(
172+
steps []agentloop.PlanStep,
173+
durable []storage.AgentPlanStep,
174+
) map[string]string {
175+
if len(durable) != len(steps) {
176+
tracef("plan", "the recorded plan holds %d step(s) and the run holds "+
177+
"%d, so %d will have no durable identity",
178+
len(durable), len(steps), len(steps)-len(durable))
179+
}
180+
paired := make(map[string]string, len(steps))
141181
for index, step := range steps {
142-
if index < len(plan.Steps) {
143-
result.Steps[step.ID] = plan.Steps[index].ID
182+
if index < len(durable) {
183+
paired[step.ID] = durable[index].ID
144184
}
145185
}
146-
return result, nil
186+
return paired
147187
}
148188

149189
// durablePlanStepKind maps a loop step onto the plan vocabulary.
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
package coordinator
2+
3+
import (
4+
"testing"
5+
6+
agentloop "codeflux.dev/codeflux/internal/agent"
7+
"codeflux.dev/codeflux/internal/storage"
8+
)
9+
10+
// TestStepIdentitiesArePairedWithWhatTheStoreHolds is the assumption behind an
11+
// error that names neither the step nor the cause.
12+
//
13+
// The run's step identities and the store's are different vocabularies, paired
14+
// by position. The pairing used to read the plan that was built to *send* to
15+
// the store rather than the one the store *returned*. Those are the same object
16+
// on an ordinary write and need not be on an idempotent one, where the store
17+
// answers with the revision it already had — and the identities the run then
18+
// adopts are ones the store never issued.
19+
//
20+
// Every later read of such a step is refused with "step does not belong to run
21+
// plan". That is a true sentence about a situation nothing else reports, and it
22+
// cost two whole ladder passes on 2026-08-04 before it was even legible. See
23+
// LAD-003.
24+
func TestStepIdentitiesArePairedWithWhatTheStoreHolds(t *testing.T) {
25+
steps := []agentloop.PlanStep{
26+
{ID: "edit-1"}, {ID: "edit-2"}, {ID: "verify"},
27+
}
28+
stored := []storage.AgentPlanStep{
29+
{ID: "stp_a"}, {ID: "stp_b"}, {ID: "stp_c"},
30+
}
31+
32+
paired := pairStepIdentities(steps, stored)
33+
for local, want := range map[string]string{
34+
"edit-1": "stp_a", "edit-2": "stp_b", "verify": "stp_c",
35+
} {
36+
if paired[local] != want {
37+
t.Errorf("%s paired to %q, want %q — the run would attribute its "+
38+
"work to a step the store never issued",
39+
local, paired[local], want)
40+
}
41+
}
42+
}
43+
44+
// TestAShortRecordedPlanLeavesTheExtraStepsUnpaired is the case that has to
45+
// stay visible rather than silent.
46+
//
47+
// A step with no durable identity cannot be attributed to and every read of it
48+
// is refused. Pairing must not invent one, and the mismatch must not pass
49+
// unremarked — the whole cost of this defect was that it did.
50+
func TestAShortRecordedPlanLeavesTheExtraStepsUnpaired(t *testing.T) {
51+
steps := []agentloop.PlanStep{
52+
{ID: "edit-1"}, {ID: "edit-2"}, {ID: "verify"},
53+
}
54+
stored := []storage.AgentPlanStep{{ID: "stp_a"}}
55+
56+
paired := pairStepIdentities(steps, stored)
57+
if paired["edit-1"] != "stp_a" {
58+
t.Errorf("the recorded step was not paired: %v", paired)
59+
}
60+
if _, present := paired["edit-2"]; present {
61+
t.Error("a step the store never wrote was given a durable identity")
62+
}
63+
if _, present := paired["verify"]; present {
64+
t.Error("a step the store never wrote was given a durable identity")
65+
}
66+
// adoptDurablePlanSteps drops what pairing leaves out, so the two together
67+
// mean the run carries only steps the store knows.
68+
adopted := adoptDurablePlanSteps(steps, durablePlan{Steps: paired})
69+
if len(adopted) != 1 || adopted[0].ID != "stp_a" {
70+
t.Errorf("the run kept steps the store never wrote: %+v", adopted)
71+
}
72+
}
73+
74+
// TestPairingAnEmptyPlanPairsNothing keeps the degraded path honest.
75+
//
76+
// A store that wrote no steps leaves nothing to pair against. Pairing has to
77+
// answer with an empty map rather than a partial one, so the caller's own
78+
// fallback decides what happens rather than inheriting half a mapping.
79+
func TestPairingAnEmptyPlanPairsNothing(t *testing.T) {
80+
paired := pairStepIdentities(
81+
[]agentloop.PlanStep{{ID: "edit-1"}}, nil)
82+
if len(paired) != 0 {
83+
t.Errorf("want no pairings, got %v", paired)
84+
}
85+
}

0 commit comments

Comments
 (0)