Make a contradicted plan a typed report, and the reopen Orchestra's
An implementer that finds the plan contradicted by the code had two options,
both bad: work around it silently, or improvise a different plan inside the
phase meant to execute one. PlanMismatch is the third.
The report carries an observation and nothing else. It may not propose a
replacement plan, because writing the next plan is the planning phase's work.
requested_action stays advisory: replan, research, or human_decision is a
recommendation, and Orchestra decides.
Staleness is checked before anything is recorded. A report names the plan ref
and the commit it was written against, both filled by the worker from what it
can verify rather than from what the agent asserted. A report against an older
plan says nothing about the current one, and one against an older tree may
already be fixed. Neither is replayed.
The reducer keeps two things apart that are easy to conflate:
mismatch recorded != plan superseded
A plan stops being accepted only when a replacement is actually sealed, so an
abandoned replan leaves the accepted plan and its verified progress intact. On
a real re-seal the old ref moves to PlanHistory and its progress stops counting,
while the verification events stay in the log as provenance.
human_decision never reopens. It blocks with a packet stating what was observed
and what it contradicts, and a human answer can resolve the contradiction
without resealing anything: the plan, its progress and the phase all survive,
and the answer outranks the plan where they differ. Turning every ambiguity
into a replan would put the planner above the person who set the goal.
The backward edge is Orchestra's alone. CanReopenPhase is separate from
CanTransitionPhase, which every path validating an agent's request uses, so
phase-request.json still refuses a move back. An agent asks by reporting a
mismatch.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CVbaKucEYBjMqVeUgJUsc1
This commit is contained in:
@@ -566,3 +566,97 @@ None.
|
||||
## References
|
||||
- research:r1
|
||||
`
|
||||
|
||||
// The implement brief has to name both routes, or an agent that finds the plan
|
||||
// contradicted either works around it or rewrites the plan itself.
|
||||
func TestImplementBriefNamesProgressAndMismatch(t *testing.T) {
|
||||
doc, err := workphase.ParsePlan([]byte(planFixture))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
out, err := Build(Input{
|
||||
Task: domain.Task{ID: "t1", Title: "demo", PlanRef: "ref"},
|
||||
Phase: domain.WorkPhaseImplement,
|
||||
Git: GitState{Worktree: "/w", Branch: "orchestra/t1", HeadSHA: "abc"},
|
||||
Plan: &doc,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
for _, want := range []string{
|
||||
".orchestra/plan-progress.json",
|
||||
`"status": "ready_for_verification"`,
|
||||
"You cannot write it",
|
||||
".orchestra/plan-mismatch.json",
|
||||
"requested_action",
|
||||
"phase-1",
|
||||
} {
|
||||
if !strings.Contains(out.Task, want) {
|
||||
t.Fatalf("implement brief omits %q:\n%s", want, out.Task)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// A verified phase whose commit has moved must read as stale. Otherwise
|
||||
// "verified" becomes another artifact that outlives what made it true.
|
||||
func TestPlanProgressLabelsAStaleVerification(t *testing.T) {
|
||||
doc, err := workphase.ParsePlan([]byte(planFixture))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
const verifiedAt = "1111111111111111111111111111111111111111"
|
||||
const nowAt = "2222222222222222222222222222222222222222"
|
||||
task := domain.Task{
|
||||
ID: "t1", Title: "demo", PlanRef: "ref",
|
||||
PlanProgress: &domain.PlanProgress{PlanRef: "ref", Phases: []domain.PlanPhaseRecord{
|
||||
{PlanRef: "ref", PhaseID: "phase-1", Status: domain.PlanPhaseVerified, AtSHA: verifiedAt},
|
||||
}},
|
||||
}
|
||||
fresh, err := Build(Input{Task: task, Phase: domain.WorkPhaseImplement, Plan: &doc,
|
||||
Git: GitState{Worktree: "/w", Branch: "b", HeadSHA: verifiedAt}})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if strings.Contains(fresh.Task, "stale") {
|
||||
t.Fatal("a verification at the current head was labelled stale")
|
||||
}
|
||||
moved, err := Build(Input{Task: task, Phase: domain.WorkPhaseImplement, Plan: &doc,
|
||||
Git: GitState{Worktree: "/w", Branch: "b", HeadSHA: nowAt}})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if !strings.Contains(moved.Task, "stale") {
|
||||
t.Fatalf("a verification at an older commit reads as current:\n%s", moved.Task)
|
||||
}
|
||||
// Phase 1 is verified but stale, so phase 2 is still what to work on.
|
||||
if !strings.Contains(moved.Task, "Your current phase is phase-2") {
|
||||
t.Fatalf("the current phase is wrong:\n%s", moved.Task)
|
||||
}
|
||||
}
|
||||
|
||||
// A plan sealed before plan.md declares no executable unit, and the brief has
|
||||
// to say so rather than showing an empty progress section.
|
||||
func TestLegacyPlanSaysProgressIsUnavailable(t *testing.T) {
|
||||
legacy, err := workphase.DecodeStoredPlan([]byte(`{"changes":[{"target":"a.go","intent":"do a thing"}]}`))
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
out, err := Build(Input{
|
||||
Task: domain.Task{ID: "t1", Title: "demo", PlanRef: "ref"},
|
||||
Phase: domain.WorkPhaseImplement,
|
||||
Git: GitState{Worktree: "/w", Branch: "b", HeadSHA: "abc"},
|
||||
Plan: &legacy,
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if !strings.Contains(out.Task, "legacy accepted plan") || !strings.Contains(out.Task, "Phase progress is unavailable") {
|
||||
t.Fatalf("a legacy plan does not say progress is unavailable:\n%s", out.Task)
|
||||
}
|
||||
if strings.Contains(out.Task, "## Plan progress") {
|
||||
t.Fatal("a legacy plan rendered a progress section it cannot have")
|
||||
}
|
||||
if !strings.Contains(out.Task, "do a thing") {
|
||||
t.Fatal("the legacy plan text was lost")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user