6cb2f932d8
F64. A plan mismatch that asks for a human decision blocks the task, and nothing came back. Two independent gaps, either one enough to strand it: the reconciler ran only before a lease and at a turn boundary, so a blocked task's reply was never even read, and ResumeAnsweredBlockers listed two block reasons, not this one. PlanMismatchAnswered had no caller anywhere. Blocked tasks awaiting a reply are now reconciled on their own loop, the same reasoning the submitted-work loop above it already uses: a task that cannot be leased cannot be reconciled behind a pre-lease hook. One predicate, BlockReasonAwaitsReply, now names the set for both loops so they cannot drift apart again. The existing test asserted the predicate and never the resume, which is how this survived. It asserts the resume now, and fails without the fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CVbaKucEYBjMqVeUgJUsc1
328 lines
12 KiB
Go
328 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")
|
|
}
|
|
// F64: asserting the predicate is not asserting the resume. This test
|
|
// passed for as long as the predicate had no caller, while a task blocked
|
|
// on a mismatch stayed blocked forever however the human replied.
|
|
if events, err := ResumeAnsweredBlockers(s); err != nil || len(events) != 1 {
|
|
t.Fatalf("an answered mismatch did not return to the queue: events=%v err=%v", events, err)
|
|
}
|
|
after, _ := s.Task(id)
|
|
if after.State != domain.StateQueued {
|
|
t.Fatalf("state = %s after the human answered, want queued", after.State)
|
|
}
|
|
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")
|
|
}
|
|
}
|
|
}
|