fb7135e1d9
The brief tells the planner "a command outside its policy is refused when you seal, not later". It was not. The only caller of VerificationPolicy.Allows was PlanPhaseCommands, which runs when the implementer asks to verify: one phase, one session and one rotation after the planner could have fixed it. Run 9 sealed ["bash", "scripts/test_healthcheck.sh"] against a policy that allows neither shape, and the phase request was accepted. The check now runs beside citation resolution, on the coordinator, where the project is already in scope. A project with no verification policy can still seal a plan; it cannot seal one that declares run: lines, which matches what an absent policy already meant at verification time. Test fixtures gained a policy for the same reason. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CVbaKucEYBjMqVeUgJUsc1
319 lines
12 KiB
Go
319 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")
|
|
}
|
|
}
|
|
}
|