Files
orchestra/internal/operations/planprogress_test.go
T
kami b5c37f693b Bind a manual sign-off to the tree it was given against
F63, found live on run 19. A manual check on these projects is a human
reading what the code prints. RecordPlanPhaseVerification asked only
whether a sign-off for that plan and phase existed, and one exists forever,
so rerunning a phase's automated checks at a new commit carried the human
half along with it. The rig proved it twice: two operator commits and two
re-verification requests, each coming back verified without anyone looking.

The reducer now records which tree the human confirmed, the record carries
it forward as provenance, and a run whose commit does not match it waits
for the human again. A sign-off given before any run has no confirmed tree
and still counts, so the ordinary ordering is unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CVbaKucEYBjMqVeUgJUsc1
2026-08-29 19:41:50 +04:00

328 lines
11 KiB
Go

package operations
import (
"errors"
"strings"
"testing"
"orchestra/internal/authz"
"orchestra/internal/domain"
"orchestra/internal/registry"
"orchestra/internal/store"
)
const shaOne = "1111111111111111111111111111111111111111"
const shaTwo = "2222222222222222222222222222222222222222"
const shaThree = "3333333333333333333333333333333333333333"
func planProject() registry.Project {
p := registry.Project{
ID: "demo", MachineAffinity: []string{"m"},
WorkPhases: []domain.WorkPhase{domain.WorkPhaseFrame, domain.WorkPhaseResearch, domain.WorkPhasePlan, domain.WorkPhaseImplement, domain.WorkPhaseReview},
}
p.Verification.Allowed = [][]string{{"go", "test", "./internal/..."}, {"go", "build", "./..."}}
return p
}
// planWith seals research and a plan, leaving the task in implement.
func planWith(t *testing.T, markdown string) (*store.Store, registry.Project, string) {
t.Helper()
s, id := phaseStore(t)
project := planProject()
if _, err := AdvanceWorkPhase(s, project, id, nil); err != nil {
t.Fatal(err)
}
if _, err := AdvanceWorkPhase(s, project, id, sealed(t, research)); err != nil {
t.Fatal(err)
}
if _, err := AdvanceWorkPhase(s, project, id, []byte(markdown)); err != nil {
t.Fatal(err)
}
return s, project, id
}
const twoPhasePlan = "# Two phase plan\n" + `
## Overview
Two phases.
## Current state
Nothing, per research:r1.
## Desired end state
Both phases done.
## Non-goals
None.
## Approach
Straightforward.
## Phase 1: Build
### Files
- a.go
### Changes
Add a.
### Verification
#### Automated
- run: ["go", "build", "./..."]
## Phase 2: Test
### Files
- b.go
### Changes
Add b.
### Verification
#### Automated
- run: ["go", "test", "./internal/..."]
#### Manual
- Confirm the output by eye.
## Testing strategy
Per phase.
## Risks and edge cases
None.
## Migration
None.
## References
- research:r1
`
func TestPassingCommandsVerifyThePhase(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)
rec, ok := task.PlanPhase("phase-1")
if !ok || rec.Status != domain.PlanPhaseVerified {
t.Fatalf("phase-1 = %+v", rec)
}
if rec.AtSHA != shaOne || rec.PlanRef != task.PlanRef {
t.Fatalf("record is not bound to the plan and the commit: %+v", rec)
}
}
// A failing command leaves the phase where it was. "Verified" is a conclusion
// about the commands, never about the request that asked for them.
func TestFailingCommandLeavesThePhaseInProgress(t *testing.T) {
s, project, id := planWith(t, twoPhasePlan)
if _, err := RecordPlanPhaseVerification(s, project, id, "phase-1", shaOne,
[]VerificationRun{{Command: []string{"go", "build", "./..."}, ExitCode: 2}}); err != nil {
t.Fatal(err)
}
task, _ := s.Task(id)
rec, _ := task.PlanPhase("phase-1")
if rec.Status != domain.PlanPhaseInProgress {
t.Fatalf("status = %q, want in_progress", rec.Status)
}
if len(rec.ExitCodes) != 1 || rec.ExitCodes[0] != 2 {
t.Fatalf("the exit code was not recorded: %+v", rec)
}
}
// Automated checks passing is not the whole phase when manual steps exist.
func TestManualStepsHoldThePhaseUntilAHumanSignsOff(t *testing.T) {
s, project, id := planWith(t, twoPhasePlan)
if _, err := RecordPlanPhaseVerification(s, project, id, "phase-2", shaOne,
[]VerificationRun{{Command: []string{"go", "test", "./internal/..."}, ExitCode: 0}}); err != nil {
t.Fatal(err)
}
task, _ := s.Task(id)
rec, _ := task.PlanPhase("phase-2")
if rec.Status != domain.PlanPhaseAwaitingManual {
t.Fatalf("status = %q, want awaiting_manual_verification", rec.Status)
}
// A generic later comment must not satisfy the gate.
humanReply(t, s, id, "d-generic", "looks good")
task, _ = s.Task(id)
rec, _ = task.PlanPhase("phase-2")
if rec.Status != domain.PlanPhaseAwaitingManual {
t.Fatal("an unrelated comment satisfied a manual verification gate")
}
// The sign-off names the plan and the phase it approves.
signOff(t, s, id, domain.PlanPhaseSubject(task.PlanRef, "phase-2"))
task, _ = s.Task(id)
rec, _ = task.PlanPhase("phase-2")
if rec.Status != domain.PlanPhaseVerified {
t.Fatalf("status = %q after a bound sign-off, want verified", rec.Status)
}
}
// A sign-off is bound to one plan. Replanning does not inherit it.
func TestSignOffForAnotherPlanDoesNotVerify(t *testing.T) {
s, project, id := planWith(t, twoPhasePlan)
if _, err := RecordPlanPhaseVerification(s, project, id, "phase-2", shaOne,
[]VerificationRun{{Command: []string{"go", "test", "./internal/..."}, ExitCode: 0}}); err != nil {
t.Fatal(err)
}
signOff(t, s, id, domain.PlanPhaseSubject("some-other-plan-ref", "phase-2"))
task, _ := s.Task(id)
rec, _ := task.PlanPhase("phase-2")
if rec.Status != domain.PlanPhaseAwaitingManual {
t.Fatalf("a sign-off naming another plan verified this one: %q", rec.Status)
}
}
// Verified at X, code moves to Y: the record stands as provenance and must
// read as stale, never as a claim about the current tree.
func TestVerificationGoesStaleWhenTheTreeMoves(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)
rec, _ := task.PlanPhase("phase-1")
if rec.Stale(shaOne) {
t.Fatal("a verification at the current head reported stale")
}
if !rec.Stale(shaTwo) {
t.Fatal("a verification at an older commit did not report stale")
}
}
// The commands come from the accepted plan. A request cannot substitute one.
func TestReportedResultsMustMatchThePlansCommands(t *testing.T) {
s, project, id := planWith(t, twoPhasePlan)
_, err := RecordPlanPhaseVerification(s, project, id, "phase-1", shaOne,
[]VerificationRun{{Command: []string{"echo", "ok"}, ExitCode: 0}})
if !errors.Is(err, ErrPlanPhase) {
t.Fatalf("a substituted command was accepted: %v", err)
}
}
// A command outside project policy is refused before anything runs, and the
// refusal names the project so the planner learns its real reach.
func TestPolicyRefusalHappensBeforeExecution(t *testing.T) {
s, project, id := planWith(t, twoPhasePlan)
project.Verification.Allowed = [][]string{{"go", "build", "./..."}}
_, err := PlanPhaseCommands(s, project, id, "phase-2")
if !errors.Is(err, ErrPlanPhase) {
t.Fatalf("an unauthorised command was resolved: %v", err)
}
if !strings.Contains(err.Error(), "demo") || !strings.Contains(err.Error(), "go test") {
t.Fatalf("the refusal does not name the project and the command: %v", err)
}
task, _ := s.Task(id)
if _, ok := task.PlanPhase("phase-2"); ok {
t.Fatal("a refused phase produced a record")
}
}
// A plan sealed before plan.md declares no executable unit, and saying so
// beats inventing a phase it never had.
func TestLegacyPlanIsExplicitlyNonProgressable(t *testing.T) {
s, id := phaseStore(t)
project := planProject()
if _, err := AdvanceWorkPhase(s, project, id, nil); err != nil {
t.Fatal(err)
}
if _, err := AdvanceWorkPhase(s, project, id, sealed(t, research)); err != nil {
t.Fatal(err)
}
// A pre-markdown plan, appended straight to the CAS the way an old ref
// would already be there.
ref, err := s.PutArtifact([]byte(`{"changes":[{"target":"a.go","intent":"do a thing"}]}`))
if err != nil {
t.Fatal(err)
}
task, _ := s.Task(id)
task.PlanRef = ref
_, err = PlanPhaseCommands(s, project, id, "phase-1")
if err == nil {
t.Fatal("a legacy plan resolved a phase")
}
}
// signOff records a human decision bound to one phase of one plan, which is
// the only thing that satisfies a manual verification gate.
func signOff(t *testing.T, s *store.Store, taskID, subject string) {
t.Helper()
signOffFrom(t, s, taskID, subject, "signoff-"+subject)
}
// signOffFrom names the comment the sign-off came from. Two sign-offs on one
// subject are a real sequence once a rerun sends a phase back to the human,
// and provenance is unique per comment.
func signOffFrom(t *testing.T, s *store.Store, taskID, subject, externalID string) {
t.Helper()
task, _ := s.Task(taskID)
if err := s.Append(domain.Event{
ID: domain.NewID(), Type: domain.EventHumanDecisionRecorded, TaskID: taskID,
Version: task.Version + 1, Surface: string(authz.System),
Payload: mustJSONBytes(t, map[string]any{
"decision_id": domain.NewID(), "kind": "answer", "subject": subject,
"value": "manual steps confirmed",
"source": map[string]any{"provider": "gitea", "external_id": externalID},
}),
}); err != nil {
t.Fatalf("sign off: %v", err)
}
}
// F63, found live on run 19. A manual sign-off says a human read what this
// code prints. An edit afterwards can change exactly that, so rerunning the
// automated half at a new commit must not carry the human half with it.
func TestASignOffDoesNotSurviveTheTreeItWasGivenAgainst(t *testing.T) {
s, project, id := planWith(t, twoPhasePlan)
run := []VerificationRun{{Command: []string{"go", "test", "./internal/..."}, ExitCode: 0}}
if _, err := RecordPlanPhaseVerification(s, project, id, "phase-2", shaOne, run); err != nil {
t.Fatal(err)
}
task, _ := s.Task(id)
signOff(t, s, id, domain.PlanPhaseSubject(task.PlanRef, "phase-2"))
task, _ = s.Task(id)
rec, _ := task.PlanPhase("phase-2")
if rec.Status != domain.PlanPhaseVerified || rec.ManualAtSHA != shaOne {
t.Fatalf("sign-off did not bind to the tree it read: %+v", rec)
}
// The tree moves and the phase is re-verified. The commands pass again;
// the human has not seen the new output.
if _, err := RecordPlanPhaseVerification(s, project, id, "phase-2", shaTwo, run); err != nil {
t.Fatal(err)
}
task, _ = s.Task(id)
rec, _ = task.PlanPhase("phase-2")
if rec.Status != domain.PlanPhaseAwaitingManual {
t.Fatalf("status = %q at a tree the human never saw, want awaiting_manual_verification", rec.Status)
}
if rec.ManualAtSHA != shaOne {
t.Fatalf("the confirmed tree was lost: %+v", rec)
}
// A second rerun must not re-inherit it either, which is what carrying
// ManualAtSHA forward is for.
if _, err := RecordPlanPhaseVerification(s, project, id, "phase-2", shaThree, run); err != nil {
t.Fatal(err)
}
task, _ = s.Task(id)
rec, _ = task.PlanPhase("phase-2")
if rec.Status != domain.PlanPhaseAwaitingManual {
t.Fatalf("a second rerun re-inherited the sign-off: %q", rec.Status)
}
// Signing off again, on the tree that is now current, verifies it.
signOffFrom(t, s, id, domain.PlanPhaseSubject(task.PlanRef, "phase-2"), "signoff-second")
task, _ = s.Task(id)
rec, _ = task.PlanPhase("phase-2")
if rec.Status != domain.PlanPhaseVerified || rec.ManualAtSHA != shaThree {
t.Fatalf("a fresh sign-off did not verify the current tree: %+v", rec)
}
}