Files
orchestra/internal/operations/planmismatch_test.go
T
kami c76112a309 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
2026-08-28 12:19:17 +04:00

320 lines
12 KiB
Go

package operations
import (
"errors"
"strings"
"testing"
"orchestra/internal/domain"
"orchestra/internal/store"
)
func mismatch(planRef string) domain.PlanMismatch {
return domain.PlanMismatch{
PlanRef: planRef, PhaseID: "phase-1", AtSHA: shaOne,
Observed: "a.go already caches per person",
Contradicts: "the plan says a.go caches per figure",
Evidence: []string{"a.go:88"},
RequestedAction: domain.PlanMismatchReplan,
}
}
// A report written against a plan the task no longer works from says nothing
// about the current one. Replaying it would reopen planning over a
// contradiction that may not exist any more.
func TestStalePlanRefIsRefused(t *testing.T) {
s, project, id := planWith(t, twoPhasePlan)
_, err := RecordPlanMismatch(s, project, id, mismatch("an-older-plan-ref"), shaOne)
if !errors.Is(err, ErrPlanMismatchStale) {
t.Fatalf("a stale plan ref was accepted: %v", err)
}
assertNoMismatchRecorded(t, s, id)
assertPhase(t, s, id, domain.WorkPhaseImplement)
}
// A contradiction observed at an older commit may already be fixed.
func TestStaleCommitIsRefused(t *testing.T) {
s, project, id := planWith(t, twoPhasePlan)
task, _ := s.Task(id)
_, err := RecordPlanMismatch(s, project, id, mismatch(task.PlanRef), shaTwo)
if !errors.Is(err, ErrPlanMismatchStale) {
t.Fatalf("a stale commit was accepted: %v", err)
}
assertNoMismatchRecorded(t, s, id)
assertPhase(t, s, id, domain.WorkPhaseImplement)
}
func TestUnknownPhaseIsRefused(t *testing.T) {
s, project, id := planWith(t, twoPhasePlan)
task, _ := s.Task(id)
m := mismatch(task.PlanRef)
m.PhaseID = "phase-9"
if _, err := RecordPlanMismatch(s, project, id, m, shaOne); err == nil {
t.Fatal("a mismatch against a phase the plan does not have was accepted")
}
assertNoMismatchRecorded(t, s, id)
}
// The observation is durable before anything moves. A reopen that failed
// partway would otherwise leave a task in an earlier phase with nothing in the
// log explaining why.
func TestMismatchIsRecordedBeforeThePhaseMoves(t *testing.T) {
s, project, id := planWith(t, twoPhasePlan)
task, _ := s.Task(id)
if _, err := RecordPlanMismatch(s, project, id, mismatch(task.PlanRef), shaOne); err != nil {
t.Fatal(err)
}
var mismatchSeq, phaseSeq uint64
for _, e := range s.Events(0) {
if e.TaskID != id {
continue
}
switch e.Type {
case domain.EventPlanMismatchRecorded:
mismatchSeq = e.Seq
case domain.EventWorkPhaseChanged:
phaseSeq = e.Seq
}
}
if mismatchSeq == 0 {
t.Fatal("no mismatch was recorded")
}
if phaseSeq < mismatchSeq {
t.Fatalf("the phase moved at %d before the mismatch was durable at %d", phaseSeq, mismatchSeq)
}
assertPhase(t, s, id, domain.WorkPhasePlan)
}
// Recording a mismatch is not the same as superseding the plan. Until a
// replacement is sealed the task still works from the plan it has, with the
// progress it earned.
func TestReplanKeepsTheOldPlanUntilAReplacementIsSealed(t *testing.T) {
s, project, id := planWith(t, twoPhasePlan)
if _, err := RecordPlanPhaseVerification(s, project, id, "phase-1", shaOne,
[]VerificationRun{{Command: []string{"go", "build", "./..."}, ExitCode: 0}}); err != nil {
t.Fatal(err)
}
before, _ := s.Task(id)
if _, err := RecordPlanMismatch(s, project, id, mismatch(before.PlanRef), shaOne); err != nil {
t.Fatal(err)
}
during, _ := s.Task(id)
if during.PlanRef != before.PlanRef {
t.Fatal("the plan was superseded by the mismatch alone")
}
if _, ok := during.PlanPhase("phase-1"); !ok {
t.Fatal("progress was discarded before a replacement plan existed")
}
if len(during.PlanHistory) != 0 {
t.Fatalf("the plan was moved to history early: %v", during.PlanHistory)
}
}
// Sealing the replacement is the moment the old plan is superseded. Progress
// goes with it, and the old ref stays queryable as provenance.
func TestSealingTheReplacementSupersedesThePlanAndItsProgress(t *testing.T) {
s, project, id := planWith(t, twoPhasePlan)
if _, err := RecordPlanPhaseVerification(s, project, id, "phase-1", shaOne,
[]VerificationRun{{Command: []string{"go", "build", "./..."}, ExitCode: 0}}); err != nil {
t.Fatal(err)
}
before, _ := s.Task(id)
oldRef := before.PlanRef
if _, err := RecordPlanMismatch(s, project, id, mismatch(oldRef), shaOne); err != nil {
t.Fatal(err)
}
revised := strings.Replace(twoPhasePlan, "# Two phase plan", "# Revised two phase plan", 1)
if _, err := AdvanceWorkPhase(s, project, id, []byte(revised)); err != nil {
t.Fatal(err)
}
after, _ := s.Task(id)
if after.PlanRef == oldRef {
t.Fatal("the replacement did not become the accepted plan")
}
if rec, ok := after.PlanPhase("phase-1"); ok {
t.Fatalf("verification from the superseded plan still counts: %+v", rec)
}
if len(after.PlanHistory) != 1 || after.PlanHistory[0] != oldRef {
t.Fatalf("the superseded plan is not queryable: %v", after.PlanHistory)
}
// Provenance: the old verification is still in the log, and the artifact
// it names is still readable.
found := false
for _, e := range s.Events(0) {
if e.TaskID == id && e.Type == domain.EventPlanPhaseVerified && strings.Contains(string(e.Payload), oldRef) {
found = true
}
}
if !found {
t.Fatal("the old verification was erased from the log")
}
if _, err := s.Artifact(oldRef); err != nil {
t.Fatalf("the superseded plan is unreadable: %v", err)
}
assertPhase(t, s, id, domain.WorkPhaseImplement)
}
// research is the other reopen: the plan rests on something the repository
// does not establish, so planning again would repeat the mistake.
func TestResearchActionReopensResearch(t *testing.T) {
s, project, id := planWith(t, twoPhasePlan)
task, _ := s.Task(id)
m := mismatch(task.PlanRef)
m.RequestedAction = domain.PlanMismatchResearch
if _, err := RecordPlanMismatch(s, project, id, m, shaOne); err != nil {
t.Fatal(err)
}
assertPhase(t, s, id, domain.WorkPhaseResearch)
}
// A contradiction about intent stops for the human instead of reopening.
// Otherwise every ambiguity becomes a replan and the planner outranks the
// person who set the goal.
func TestHumanDecisionBlocksInsteadOfMovingPhases(t *testing.T) {
s, project, id := planWith(t, twoPhasePlan)
task, _ := s.Task(id)
m := mismatch(task.PlanRef)
m.RequestedAction = domain.PlanMismatchHumanDecision
if _, err := RecordPlanMismatch(s, project, id, m, shaOne); err != nil {
t.Fatal(err)
}
blocked, _ := s.Task(id)
if blocked.State != domain.StateBlocked || blocked.BlockReason != domain.BlockReasonPlanMismatch {
t.Fatalf("task = %s / %s, want blocked on plan_mismatch", blocked.State, blocked.BlockReason)
}
if blocked.WorkPhase != domain.WorkPhaseImplement {
t.Fatalf("the phase moved to %q without the human", blocked.WorkPhase)
}
for _, want := range []string{"a.go already caches per person", "the plan says a.go caches per figure", "a.go:88"} {
if !strings.Contains(blocked.Blocker, want) {
t.Fatalf("the packet omits %q:\n%s", want, blocked.Blocker)
}
}
if PlanMismatchAnswered(s, id) {
t.Fatal("an unanswered block reported answered")
}
}
// A human answer can resolve the contradiction without a replan. The plan and
// its progress survive, and the answer outranks the plan wherever they differ.
func TestHumanAnswerResumesTheSamePlanWithoutResealing(t *testing.T) {
s, project, id := planWith(t, twoPhasePlan)
if _, err := RecordPlanPhaseVerification(s, project, id, "phase-1", shaOne,
[]VerificationRun{{Command: []string{"go", "build", "./..."}, ExitCode: 0}}); err != nil {
t.Fatal(err)
}
task, _ := s.Task(id)
planRef := task.PlanRef
m := mismatch(planRef)
m.RequestedAction = domain.PlanMismatchHumanDecision
if _, err := RecordPlanMismatch(s, project, id, m, shaOne); err != nil {
t.Fatal(err)
}
humanReply(t, s, id, "d1", "the per-person cache is correct, keep it and continue phase 2")
if !PlanMismatchAnswered(s, id) {
t.Fatal("the human answered and the task is still waiting")
}
after, _ := s.Task(id)
if after.PlanRef != planRef {
t.Fatal("answering the question replaced the plan")
}
if _, ok := after.PlanPhase("phase-1"); !ok {
t.Fatal("answering the question discarded verified progress")
}
if len(after.PlanHistory) != 0 {
t.Fatalf("the plan was superseded by an answer: %v", after.PlanHistory)
}
if after.WorkPhase != domain.WorkPhaseImplement {
t.Fatalf("the phase moved to %q", after.WorkPhase)
}
// The answer is standing authority, above the plan.
intent, err := s.EffectiveIntent(id)
if err != nil {
t.Fatal(err)
}
if len(intent.Decisions) != 1 {
t.Fatalf("the answer is not standing authority: %+v", intent.Decisions)
}
}
// A project whose path omits planning cannot be reopened into it, so the
// contradiction goes to the human rather than stranding the task in a phase it
// has no brief for.
func TestReopenIntoAPhaseTheProjectDoesNotDeclareBlocksInstead(t *testing.T) {
s, project, id := planWith(t, twoPhasePlan)
trimmed := project
trimmed.WorkPhases = []domain.WorkPhase{domain.WorkPhaseFrame, domain.WorkPhaseImplement, domain.WorkPhaseReview}
task, _ := s.Task(id)
if _, err := RecordPlanMismatch(s, trimmed, id, mismatch(task.PlanRef), shaOne); err != nil {
t.Fatal(err)
}
blocked, _ := s.Task(id)
if blocked.BlockReason != domain.BlockReasonPlanMismatch {
t.Fatalf("block reason = %q", blocked.BlockReason)
}
if blocked.WorkPhase != domain.WorkPhaseImplement {
t.Fatalf("the task was reopened into a phase the project does not declare: %q", blocked.WorkPhase)
}
}
// The backward edge is Orchestra's alone. An agent asks by reporting a
// mismatch, and phase-request.json still refuses a move back.
func TestAgentCannotAskForABackwardPhaseMove(t *testing.T) {
if domain.CanTransitionPhase(domain.WorkPhaseImplement, domain.WorkPhasePlan) {
t.Fatal("the agent-facing phase graph allows implement to plan")
}
if domain.CanTransitionPhase(domain.WorkPhaseImplement, domain.WorkPhaseResearch) {
t.Fatal("the agent-facing phase graph allows implement to research")
}
if !domain.CanReopenPhase(domain.WorkPhaseImplement, domain.WorkPhasePlan) {
t.Fatal("Orchestra cannot reopen planning")
}
if domain.CanReopenPhase(domain.WorkPhaseReview, domain.WorkPhasePlan) {
t.Fatal("review is reopenable into planning, which nothing asked for")
}
}
// A request may report an observation. It may not carry the next plan: writing
// one is the planning phase's work.
func TestMismatchRequiresAnObservationAndAnAction(t *testing.T) {
base := mismatch("ref")
cases := map[string]func(m *domain.PlanMismatch){
"no observed": func(m *domain.PlanMismatch) { m.Observed = "" },
"no contradicts": func(m *domain.PlanMismatch) { m.Contradicts = "" },
"no phase": func(m *domain.PlanMismatch) { m.PhaseID = "" },
"short sha": func(m *domain.PlanMismatch) { m.AtSHA = "abc" },
"bad action": func(m *domain.PlanMismatch) { m.RequestedAction = "rewrite_it_yourself" },
"no action": func(m *domain.PlanMismatch) { m.RequestedAction = "" },
"essay": func(m *domain.PlanMismatch) { m.Observed = strings.Repeat("x", 1001) },
}
for name, mutate := range cases {
m := base
mutate(&m)
if err := m.Validate(); err == nil {
t.Errorf("%s: accepted, want a refusal", name)
}
}
if err := base.Validate(); err != nil {
t.Fatalf("a well-formed report was refused: %v", err)
}
}
func assertPhase(t *testing.T, s *store.Store, id string, want domain.WorkPhase) {
t.Helper()
task, _ := s.Task(id)
if task.WorkPhase != want {
t.Fatalf("work phase = %q, want %q", task.WorkPhase, want)
}
}
func assertNoMismatchRecorded(t *testing.T, s *store.Store, id string) {
t.Helper()
for _, e := range s.Events(0) {
if e.TaskID == id && e.Type == domain.EventPlanMismatchRecorded {
t.Fatal("a refused report was recorded anyway")
}
}
}