diff --git a/internal/agentctx/agentctx.go b/internal/agentctx/agentctx.go index cef93af..0061dbc 100644 --- a/internal/agentctx/agentctx.go +++ b/internal/agentctx/agentctx.go @@ -314,6 +314,9 @@ func renderTask(in Input) string { fmt.Fprintf(&b, "- head: %s\n", fallback(in.Git.HeadSHA)) fmt.Fprintf(&b, "- uncommitted changes: %t\n", in.Git.Dirty) + // Above the sealed artifacts, because it is the reason this session exists + // and it changes how the artifacts below should be read. + b.WriteString(renderReopen(in)) b.WriteString(renderSealed(in)) // Below the plan, above continuity. Progress is a fact about the plan, so // it follows the plan; continuity is one predecessor's account, so it @@ -499,6 +502,35 @@ func renderPlanProgress(in Input) string { return b.String() } +// renderReopen states the contradiction that reopened this phase. Without it +// the planning session a replan convenes gets the ordinary plan brief and an +// empty template, with nothing saying a plan already exists, what contradicted +// it, or that its output supersedes an accepted plan (F66). Everything here is +// durable on PlanMismatchRecorded; it simply never reached the agent. +// +// Only the phases a mismatch can reopen render it. An implementer that finds +// the next contradiction is looking at the code, not at this account of the +// last one. +func renderReopen(in Input) string { + m := in.Task.PlanMismatch + if m == nil || (in.Phase != domain.WorkPhasePlan && in.Phase != domain.WorkPhaseResearch) { + return "" + } + var b strings.Builder + b.WriteString("\n## Why this phase reopened\n\n") + fmt.Fprintf(&b, "A plan was already accepted and the code contradicted it. Orchestra reopened this phase to settle that, and the session that found it is gone.\n\n") + fmt.Fprintf(&b, "- phase: %s\n", collapse(m.PhaseID)) + fmt.Fprintf(&b, "- observed: %s\n", collapse(m.Observed)) + fmt.Fprintf(&b, "- the plan says: %s\n", collapse(m.Contradicts)) + for _, e := range m.Evidence { + fmt.Fprintf(&b, "- evidence: %s\n", collapse(e)) + } + if in.Phase == domain.WorkPhasePlan { + b.WriteString("\nThe accepted plan stays accepted until you seal a replacement, and sealing one supersedes it along with every phase it had verified. Address the contradiction above: a replacement that repeats it will be contradicted again.\n") + } + return b.String() +} + func renderSealed(in Input) string { var b strings.Builder research := in.Research diff --git a/internal/agentctx/agentctx_test.go b/internal/agentctx/agentctx_test.go index ac887d9..5aa17f1 100644 --- a/internal/agentctx/agentctx_test.go +++ b/internal/agentctx/agentctx_test.go @@ -734,3 +734,61 @@ func TestPendingManualGateRendersStale(t *testing.T) { t.Errorf("a pending manual gate at HEAD rendered stale:\n%s", fresh) } } + +// F66, found live on run 20. A replan convenes a planning session to settle a +// contradiction, and that session was told nothing about it: the ordinary plan +// brief, an empty template, and no mention that a plan already exists. +func TestAReopenedPlanPhaseIsToldWhatContradictedThePlan(t *testing.T) { + in := input() + in.Phase = domain.WorkPhasePlan + in.Task.PlanRef = "plan-a" + in.Task.PlanMismatch = &domain.PlanMismatch{ + PlanRef: "plan-a", PhaseID: "phase-3", AtSHA: "18ccaf00000000000000000000000000000000aa", + Observed: "the aggregation runs per figure, not per person", + Contradicts: "the plan states the pipeline already aggregates per person", + Evidence: []string{"internal/figures/aggregate.go:88"}, + RequestedAction: domain.PlanMismatchReplan, + } + out, err := Build(in) + if err != nil { + t.Fatal(err) + } + for _, want := range []string{ + "## Why this phase reopened", + "phase: phase-3", + "the aggregation runs per figure", + "the plan states the pipeline already aggregates per person", + "internal/figures/aggregate.go:88", + "stays accepted until you seal a replacement", + } { + if !strings.Contains(out.Task, want) { + t.Fatalf("the planner was not told %q:\n%s", want, out.Task) + } + } + // It must come before the artifacts it changes the reading of. + in.Research = &workphase.Research{Findings: []workphase.Finding{{ + ID: "r1", Confidence: "fact", Claim: "aggregation is per figure", + Evidence: "internal/figures/aggregate.go:88", + }}} + out, err = Build(in) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(out.Task, "## Accepted research") { + t.Fatal("the research this planner works from is missing") + } + if strings.Index(out.Task, "## Why this phase reopened") > strings.Index(out.Task, "## Accepted research") { + t.Fatal("the reason this session exists is rendered below the material it qualifies") + } + + // An implementer is looking at the code, not at an account of the last + // contradiction, so it renders nowhere else. + in.Phase = domain.WorkPhaseImplement + out, err = Build(in) + if err != nil { + t.Fatal(err) + } + if strings.Contains(out.Task, "## Why this phase reopened") { + t.Fatal("a settled contradiction reached the implementation phase") + } +} diff --git a/internal/domain/domain.go b/internal/domain/domain.go index dd17a3e..a406d36 100644 --- a/internal/domain/domain.go +++ b/internal/domain/domain.go @@ -234,6 +234,13 @@ type Task struct { // current one, oldest first. A superseded plan stays queryable: the // verification recorded against it is provenance, not garbage. PlanHistory []string `json:"plan_history,omitempty"` + // PlanMismatch is the contradiction that reopened this task's phase. The + // planning session convened by a replan has to be told what it is there to + // fix, and the implementer that found it is gone by then (F66). It is + // cleared when a replacement plan is accepted, or when the stop it caused + // is answered, because after either it is history rather than a live + // instruction. + PlanMismatch *PlanMismatch `json:"plan_mismatch,omitempty"` LastError string `json:"last_error,omitempty"` } diff --git a/internal/operations/planmismatch_test.go b/internal/operations/planmismatch_test.go index c56d770..40ea21e 100644 --- a/internal/operations/planmismatch_test.go +++ b/internal/operations/planmismatch_test.go @@ -108,6 +108,11 @@ func TestReplanKeepsTheOldPlanUntilAReplacementIsSealed(t *testing.T) { if len(during.PlanHistory) != 0 { t.Fatalf("the plan was moved to history early: %v", during.PlanHistory) } + // F66: the planning session this reopen convenes has to be told what it is + // there to fix, and the implementer that found it is gone by then. + if during.PlanMismatch == nil || during.PlanMismatch.PhaseID != "phase-1" { + t.Fatalf("the contradiction did not survive to the phase it reopened: %+v", during.PlanMismatch) + } } // Sealing the replacement is the moment the old plan is superseded. Progress @@ -151,6 +156,11 @@ func TestSealingTheReplacementSupersedesThePlanAndItsProgress(t *testing.T) { if _, err := s.Artifact(oldRef); err != nil { t.Fatalf("the superseded plan is unreadable: %v", err) } + // The replacement answers the contradiction, so it stops being a live + // instruction and stays in the log as history. + if after, _ := s.Task(id); after.PlanMismatch != nil { + t.Fatalf("a settled contradiction is still live: %+v", after.PlanMismatch) + } assertPhase(t, s, id, domain.WorkPhaseImplement) } @@ -229,6 +239,11 @@ func TestHumanAnswerResumesTheSamePlanWithoutResealing(t *testing.T) { if after.BlockReason != "" || after.Blocker != "" { t.Fatalf("a resumed task still reports its blocker: %q %q", after.BlockReason, after.Blocker) } + // The answer outranks the plan and stands as an ordinary decision, so the + // contradiction it settled is not carried into later sessions either. + if after.PlanMismatch != nil { + t.Fatalf("an answered contradiction is still live: %+v", after.PlanMismatch) + } if after.PlanRef != planRef { t.Fatal("answering the question replaced the plan") } diff --git a/internal/store/store.go b/internal/store/store.go index f537ef2..a32ae1d 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -312,6 +312,9 @@ func (s *Store) apply(e domain.Event) error { } t.PlanRef = p.ArtifactRef t.PlanProgress = nil + // The replacement answers the contradiction. Carrying it + // further would put a solved problem in every later context. + t.PlanMismatch = nil } } t.WorkPhase = p.Phase @@ -487,6 +490,19 @@ func (s *Store) apply(e domain.Event) error { t.Version = e.Version s.replaceTask(e.TaskID, t) return nil + case domain.EventPlanMismatchRecorded: + // Projected so the phase this reopens can be told what reopened it. + // The event is the record; this is the live instruction derived from + // it, and it stops being live as soon as a replacement seals or the + // stop is answered. + var m domain.PlanMismatch + if err := json.Unmarshal(e.Payload, &m); err != nil { + return err + } + t.PlanMismatch = &m + t.Version = e.Version + s.replaceTask(e.TaskID, t) + return nil case domain.EventPlanPhaseVerified: var pp domain.PlanPhaseRecord if err := json.Unmarshal(e.Payload, &pp); err != nil { @@ -596,6 +612,9 @@ func (s *Store) apply(e domain.Event) error { if t.State != domain.StateBlocked { t.DecisionRequest = nil t.Blocker, t.BlockReason = "", "" + // The human's answer outranks the plan and stands as an ordinary + // decision, so the contradiction it settled is no longer live. + t.PlanMismatch = nil } if phase, ok := p["lifecycle_phase"].(string); ok && phase != "" { t.LifecyclePhase = phase