From f4dbcf70f64ed8e84281709268ff5f5e68f6da1d Mon Sep 17 00:00:00 2001 From: kami Date: Fri, 28 Aug 2026 14:41:59 +0400 Subject: [PATCH] Stop telling a successor the tree is settled when every phase is stale Run 8 rendered three phases as "verified at 14654d6cab63, stale because the tree is now at 56d9b9acdd59" and then printed "Every phase is verified" under them. The staleness was honest and the conclusion was not. Stale still counts as done. The usual reason a phase goes stale is the next phase's own commit, and treating that as unverified would send the implementer back to phase 1 after every commit. What the successor needed was the names: which phases were verified against code that has since changed, and an instruction to reverify what its work touches. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01CVbaKucEYBjMqVeUgJUsc1 --- internal/agentctx/agentctx.go | 25 ++++++++++++++++++++- internal/agentctx/agentctx_test.go | 36 ++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 1 deletion(-) diff --git a/internal/agentctx/agentctx.go b/internal/agentctx/agentctx.go index c109cd8..37a2506 100644 --- a/internal/agentctx/agentctx.go +++ b/internal/agentctx/agentctx.go @@ -416,6 +416,13 @@ func fallback(s string) string { // plan -> accepted research // implement -> accepted research and accepted plan // review -> accepted plan +func plural(n int, one, many string) string { + if n == 1 { + return one + } + return many +} + // renderPlanProgress states what Orchestra established about the accepted // plan, which is the half a rotated successor cannot reconstruct. A verified // phase is named with the commit it was verified at, and labelled stale when @@ -452,7 +459,23 @@ func renderPlanProgress(in Input) string { } } if current == "" { - b.WriteString("\nEvery phase is verified.\n") + // Stale still counts as done: the usual reason a phase goes stale is + // the next phase's own commit, and treating that as unverified would + // send the implementer back to phase 1 after every commit. What must + // not happen is telling a successor the tree is settled when the code + // under a verification has since changed, so name the stale phases + // rather than claiming the plan is finished. + var stale []string + for _, rec := range records { + if rec.Status == domain.PlanPhaseVerified && rec.Stale(in.Git.HeadSHA) { + stale = append(stale, rec.PhaseID) + } + } + if len(stale) == 0 { + b.WriteString("\nEvery phase is verified at the current tree.\n") + return b.String() + } + fmt.Fprintf(&b, "\nEvery phase is verified, but %s %s verified against code that has since changed. Reverify what your work touches by writing .orchestra/plan-progress.json for that phase.\n", strings.Join(stale, ", "), plural(len(stale), "was", "were")) return b.String() } b.WriteString(planMismatchBrief) diff --git a/internal/agentctx/agentctx_test.go b/internal/agentctx/agentctx_test.go index 4323bc0..3885a15 100644 --- a/internal/agentctx/agentctx_test.go +++ b/internal/agentctx/agentctx_test.go @@ -660,3 +660,39 @@ func TestLegacyPlanSaysProgressIsUnavailable(t *testing.T) { t.Fatal("the legacy plan text was lost") } } + +// A successor must never be told the tree is settled when every verification +// was earned against code that has since changed. Run 8 printed "Every phase +// is verified" under three phases all rendered stale in the same block. +func TestAllPhasesStaleIsNotReportedAsFinished(t *testing.T) { + plan := &workphase.PlanDoc{Phases: []workphase.PlanPhase{ + {ID: "phase-1", Name: "one"}, {ID: "phase-2", Name: "two"}, + }} + task := domain.Task{ + ID: "t1", PlanRef: "ref-a", + PlanProgress: &domain.PlanProgress{PlanRef: "ref-a", Phases: []domain.PlanPhaseRecord{ + {PlanRef: "ref-a", PhaseID: "phase-1", Status: domain.PlanPhaseVerified, AtSHA: strings.Repeat("a", 40)}, + {PlanRef: "ref-a", PhaseID: "phase-2", Status: domain.PlanPhaseVerified, AtSHA: strings.Repeat("a", 40)}, + }}, + } + stale := renderPlanProgress(Input{ + Task: task, Phase: domain.WorkPhaseImplement, Plan: plan, + Git: GitState{HeadSHA: strings.Repeat("b", 40)}, + }) + if strings.Contains(stale, "Every phase is verified at the current tree") { + t.Error("a fully stale plan was reported as verified at the current tree") + } + for _, want := range []string{"phase-1, phase-2", "since changed"} { + if !strings.Contains(stale, want) { + t.Errorf("stale render missing %q:\n%s", want, stale) + } + } + + current := renderPlanProgress(Input{ + Task: task, Phase: domain.WorkPhaseImplement, Plan: plan, + Git: GitState{HeadSHA: strings.Repeat("a", 40)}, + }) + if !strings.Contains(current, "Every phase is verified at the current tree") { + t.Errorf("a plan verified at HEAD was not reported as finished:\n%s", current) + } +}