From fb7135e1d92aadee1c1062bf2409361e9689e198 Mon Sep 17 00:00:00 2001 From: kami Date: Fri, 28 Aug 2026 15:27:39 +0400 Subject: [PATCH] Refuse a plan command outside project policy when the plan seals 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 Claude-Session: https://claude.ai/code/session_01CVbaKucEYBjMqVeUgJUsc1 --- internal/integration/end_to_end_test.go | 2 +- internal/integration/phase_protocol_test.go | 2 +- internal/operations/human_decision_test.go | 8 ++-- internal/operations/phase_request_test.go | 16 +++---- internal/operations/planmismatch_test.go | 1 - internal/operations/review_test.go | 10 ++--- internal/operations/submission_test.go | 2 +- internal/operations/trajectory_test.go | 4 +- internal/operations/workphase.go | 23 +++++++++++ internal/operations/workphase_test.go | 46 +++++++++++++++++++-- 10 files changed, 87 insertions(+), 27 deletions(-) diff --git a/internal/integration/end_to_end_test.go b/internal/integration/end_to_end_test.go index e31a182..b362e73 100644 --- a/internal/integration/end_to_end_test.go +++ b/internal/integration/end_to_end_test.go @@ -74,7 +74,7 @@ func setup(t *testing.T) (*store.Store, registry.Registry, string) { t.Fatal(err) } r, err := registry.New(registry.Config{ - Projects: []registry.Project{{ID: "p", MachineAffinity: []string{"m"}}}, + Projects: []registry.Project{{ID: "p", MachineAffinity: []string{"m"}, Verification: registry.VerificationPolicy{Allowed: [][]string{{"go", "test", "*"}}}}}, Machines: []registry.Machine{{ID: "m", Address: "unused"}}, Herdrs: []registry.Herdr{{ID: "h1", MachineID: "m", Capabilities: []string{"go"}, Concurrency: 1, QuotaLimit: 100}}, }) diff --git a/internal/integration/phase_protocol_test.go b/internal/integration/phase_protocol_test.go index df4c0c7..de30181 100644 --- a/internal/integration/phase_protocol_test.go +++ b/internal/integration/phase_protocol_test.go @@ -91,7 +91,7 @@ func TestPhaseRequestPathSealsAndFences(t *testing.T) { } leased, _ := s.Task(task.ID) epoch := leased.Lease.Epoch - project := registry.Project{ID: "p"} + project := registry.Project{ID: "p", Verification: registry.VerificationPolicy{Allowed: [][]string{{"go", "test", "*"}}}} if _, err := operations.RequestWorkPhase(s, project, task.ID, epoch, "op-1", domain.WorkPhaseFrame, domain.WorkPhaseResearch, nil); err != nil { t.Fatal(err) diff --git a/internal/operations/human_decision_test.go b/internal/operations/human_decision_test.go index 3009dfe..2f7bc2b 100644 --- a/internal/operations/human_decision_test.go +++ b/internal/operations/human_decision_test.go @@ -26,7 +26,7 @@ func request(q string) domain.DecisionRequest { func TestDecisionRequestBlocksAndResumes(t *testing.T) { s, id := phaseStore(t) lease(t, s, id) - project := registry.Project{ID: "p"} + project := registry.Project{ID: "p", Verification: registry.VerificationPolicy{Allowed: [][]string{{"go", "test", "*"}}}} if _, err := RequestHumanDecision(s, project, id, request("should the old cache contract stay compatible?")); err != nil { t.Fatal(err) @@ -78,7 +78,7 @@ func TestDecisionRequestBlocksAndResumes(t *testing.T) { func TestResolvedQuestionIsNotRepeatedAfterRotation(t *testing.T) { s, id := phaseStore(t) lease(t, s, id) - project := registry.Project{ID: "p"} + project := registry.Project{ID: "p", Verification: registry.VerificationPolicy{Allowed: [][]string{{"go", "test", "*"}}}} if _, err := RequestHumanDecision(s, project, id, request("preserve compatibility?")); err != nil { t.Fatal(err) } @@ -100,7 +100,7 @@ func TestResolvedQuestionIsNotRepeatedAfterRotation(t *testing.T) { // The budget stops a task turning into an interview. func TestDecisionBudgetBecomesOperatorRequired(t *testing.T) { s, id := phaseStore(t) - project := registry.Project{ID: "p"} + project := registry.Project{ID: "p", Verification: registry.VerificationPolicy{Allowed: [][]string{{"go", "test", "*"}}}} project.HumanDecisions.MaxRequestsPerTask = 2 for i, q := range []string{"first?", "second?"} { @@ -134,7 +134,7 @@ func TestDecisionBudgetBecomesOperatorRequired(t *testing.T) { // Bounds are the whole defence against an interview arriving as one request. func TestDecisionRequestBoundsRejectInterviews(t *testing.T) { s, id := phaseStore(t) - project := registry.Project{ID: "p"} + project := registry.Project{ID: "p", Verification: registry.VerificationPolicy{Allowed: [][]string{{"go", "test", "*"}}}} cases := map[string]domain.DecisionRequest{ "no question": {Why: "w"}, "no why": {Question: "q"}, diff --git a/internal/operations/phase_request_test.go b/internal/operations/phase_request_test.go index 16371d7..e4e11d4 100644 --- a/internal/operations/phase_request_test.go +++ b/internal/operations/phase_request_test.go @@ -23,7 +23,7 @@ func epochOf(t *testing.T, s *store.Store, id string) string { func TestRequestWorkPhaseAdvancesAndSealsEachArtifact(t *testing.T) { s, id := phaseStore(t) - project := registry.Project{ID: "p"} + project := registry.Project{ID: "p", Verification: registry.VerificationPolicy{Allowed: [][]string{{"go", "test", "*"}}}} lease(t, s, id) epoch := epochOf(t, s, id) @@ -66,7 +66,7 @@ func TestRequestWorkPhaseRefusesASkippedPhase(t *testing.T) { // frame -> implement is a legal domain transition, but not the next step // on this project's declared path. The agent is refused rather than // silently corrected. - _, err := RequestWorkPhase(s, registry.Project{ID: "p"}, id, epochOf(t, s, id), "op-1", domain.WorkPhaseFrame, domain.WorkPhaseImplement, nil) + _, err := RequestWorkPhase(s, registry.Project{ID: "p", Verification: registry.VerificationPolicy{Allowed: [][]string{{"go", "test", "*"}}}}, id, epochOf(t, s, id), "op-1", domain.WorkPhaseFrame, domain.WorkPhaseImplement, nil) if !errors.Is(err, ErrPhaseRequest) { t.Fatalf("err = %v", err) } @@ -79,12 +79,12 @@ func TestRequestWorkPhaseRefusesAStalePhaseBelief(t *testing.T) { s, id := phaseStore(t) lease(t, s, id) epoch := epochOf(t, s, id) - if _, err := RequestWorkPhase(s, registry.Project{ID: "p"}, id, epoch, "op-1", domain.WorkPhaseFrame, domain.WorkPhaseResearch, nil); err != nil { + if _, err := RequestWorkPhase(s, registry.Project{ID: "p", Verification: registry.VerificationPolicy{Allowed: [][]string{{"go", "test", "*"}}}}, id, epoch, "op-1", domain.WorkPhaseFrame, domain.WorkPhaseResearch, nil); err != nil { t.Fatal(err) } // The agent still believes it is framing. Acting on this would advance a // phase it never ran. - _, err := RequestWorkPhase(s, registry.Project{ID: "p"}, id, epoch, "op-2", domain.WorkPhaseFrame, domain.WorkPhaseResearch, nil) + _, err := RequestWorkPhase(s, registry.Project{ID: "p", Verification: registry.VerificationPolicy{Allowed: [][]string{{"go", "test", "*"}}}}, id, epoch, "op-2", domain.WorkPhaseFrame, domain.WorkPhaseResearch, nil) if !errors.Is(err, ErrPhaseRequest) { t.Fatalf("err = %v", err) } @@ -93,7 +93,7 @@ func TestRequestWorkPhaseRefusesAStalePhaseBelief(t *testing.T) { func TestRequestWorkPhaseRefusesAStaleLeaseEpoch(t *testing.T) { s, id := phaseStore(t) lease(t, s, id) - _, err := RequestWorkPhase(s, registry.Project{ID: "p"}, id, "not-the-epoch", "op-1", domain.WorkPhaseFrame, domain.WorkPhaseResearch, nil) + _, err := RequestWorkPhase(s, registry.Project{ID: "p", Verification: registry.VerificationPolicy{Allowed: [][]string{{"go", "test", "*"}}}}, id, "not-the-epoch", "op-1", domain.WorkPhaseFrame, domain.WorkPhaseResearch, nil) if !errors.Is(err, domain.ErrConflict) { t.Fatalf("err = %v", err) } @@ -105,7 +105,7 @@ func TestRequestWorkPhaseRefusesAStaleLeaseEpoch(t *testing.T) { func TestRequestWorkPhaseRequiresAnOperationID(t *testing.T) { s, id := phaseStore(t) lease(t, s, id) - _, err := RequestWorkPhase(s, registry.Project{ID: "p"}, id, epochOf(t, s, id), "", domain.WorkPhaseFrame, domain.WorkPhaseResearch, nil) + _, err := RequestWorkPhase(s, registry.Project{ID: "p", Verification: registry.VerificationPolicy{Allowed: [][]string{{"go", "test", "*"}}}}, id, epochOf(t, s, id), "", domain.WorkPhaseFrame, domain.WorkPhaseResearch, nil) if !errors.Is(err, domain.ErrInvalid) { t.Fatalf("err = %v", err) } @@ -115,7 +115,7 @@ func TestRequestWorkPhaseRequiresAnOperationID(t *testing.T) { // the same request and must not advance the phase a second time. func TestRequestWorkPhaseIsIdempotentPerOperationID(t *testing.T) { s, id := phaseStore(t) - project := registry.Project{ID: "p"} + project := registry.Project{ID: "p", Verification: registry.VerificationPolicy{Allowed: [][]string{{"go", "test", "*"}}}} lease(t, s, id) epoch := epochOf(t, s, id) @@ -149,7 +149,7 @@ func TestRequestWorkPhaseIsIdempotentPerOperationID(t *testing.T) { func TestRequestWorkPhaseRecordsTheOperationID(t *testing.T) { s, id := phaseStore(t) lease(t, s, id) - e, err := RequestWorkPhase(s, registry.Project{ID: "p"}, id, epochOf(t, s, id), "op-1", domain.WorkPhaseFrame, domain.WorkPhaseResearch, nil) + e, err := RequestWorkPhase(s, registry.Project{ID: "p", Verification: registry.VerificationPolicy{Allowed: [][]string{{"go", "test", "*"}}}}, id, epochOf(t, s, id), "op-1", domain.WorkPhaseFrame, domain.WorkPhaseResearch, nil) if err != nil { t.Fatal(err) } diff --git a/internal/operations/planmismatch_test.go b/internal/operations/planmismatch_test.go index 68a05f3..388fcf6 100644 --- a/internal/operations/planmismatch_test.go +++ b/internal/operations/planmismatch_test.go @@ -316,4 +316,3 @@ func assertNoMismatchRecorded(t *testing.T, s *store.Store, id string) { } } } - diff --git a/internal/operations/review_test.go b/internal/operations/review_test.go index 6543c78..f4b3f47 100644 --- a/internal/operations/review_test.go +++ b/internal/operations/review_test.go @@ -50,7 +50,7 @@ func finding(id string, sev review.Severity) review.Finding { // Minor findings are reported and the task stays eligible. The review is bound // to the commit it examined. func TestMinorOnlyReviewIsAccepted(t *testing.T) { - project := registry.Project{ID: "p", QualityGate: "go test ./..."} + project := registry.Project{ID: "p", QualityGate: "go test ./...", Verification: registry.VerificationPolicy{Allowed: [][]string{{"go", "test", "*"}}}} s, id := atImplement(t, project) if _, err := EnterReview(s, project, id, evidence(shaA)); err != nil { t.Fatal(err) @@ -77,7 +77,7 @@ func TestMinorOnlyReviewIsAccepted(t *testing.T) { // A blocking finding returns the task to implementation, and the old review // cannot satisfy the new commit. func TestBlockingReviewReturnsWorkAndGoesStale(t *testing.T) { - project := registry.Project{ID: "p", QualityGate: "go test ./..."} + project := registry.Project{ID: "p", QualityGate: "go test ./...", Verification: registry.VerificationPolicy{Allowed: [][]string{{"go", "test", "*"}}}} s, id := atImplement(t, project) if _, err := EnterReview(s, project, id, evidence(shaA)); err != nil { t.Fatal(err) @@ -119,7 +119,7 @@ func TestBlockingReviewReturnsWorkAndGoesStale(t *testing.T) { // A review sealed against a commit other than the one under review is // rejected while the reviewing session still exists to redo it. func TestReviewForTheWrongCommitIsRejected(t *testing.T) { - project := registry.Project{ID: "p", QualityGate: "go test ./..."} + project := registry.Project{ID: "p", QualityGate: "go test ./...", Verification: registry.VerificationPolicy{Allowed: [][]string{{"go", "test", "*"}}}} s, id := atImplement(t, project) if _, err := EnterReview(s, project, id, evidence(shaA)); err != nil { t.Fatal(err) @@ -139,7 +139,7 @@ func TestReviewForTheWrongCommitIsRejected(t *testing.T) { } func TestReviewEntryConditions(t *testing.T) { - project := registry.Project{ID: "p", QualityGate: "go test ./..."} + project := registry.Project{ID: "p", QualityGate: "go test ./...", Verification: registry.VerificationPolicy{Allowed: [][]string{{"go", "test", "*"}}}} // Wrong phase. s, id := phaseStore(t) @@ -175,7 +175,7 @@ func TestReviewEntryConditions(t *testing.T) { // A review can only be sealed by a reviewing session, and only in a shape that // is actually reviewable. func TestReviewResultRejections(t *testing.T) { - project := registry.Project{ID: "p", QualityGate: "go test ./..."} + project := registry.Project{ID: "p", QualityGate: "go test ./...", Verification: registry.VerificationPolicy{Allowed: [][]string{{"go", "test", "*"}}}} s, id := atImplement(t, project) // Not in the review phase yet. diff --git a/internal/operations/submission_test.go b/internal/operations/submission_test.go index 1acaa92..76bb60d 100644 --- a/internal/operations/submission_test.go +++ b/internal/operations/submission_test.go @@ -52,7 +52,7 @@ func gate(sha string) domain.GateResult { // reviewed walks a task to a reviewed state at one commit. func reviewed(t *testing.T, findings ...review.Finding) (*store.Store, string, registry.Project) { t.Helper() - project := registry.Project{ID: "p", QualityGate: "go test ./..."} + project := registry.Project{ID: "p", QualityGate: "go test ./...", Verification: registry.VerificationPolicy{Allowed: [][]string{{"go", "test", "*"}}}} s, id := atImplement(t, project) if _, err := EnterReview(s, project, id, evidence(shaA)); err != nil { t.Fatal(err) diff --git a/internal/operations/trajectory_test.go b/internal/operations/trajectory_test.go index 3faa7c6..b4539eb 100644 --- a/internal/operations/trajectory_test.go +++ b/internal/operations/trajectory_test.go @@ -13,7 +13,7 @@ import ( ) func gatedProject() registry.Project { - return registry.Project{ID: "p", TrajectoryGate: map[string]string{"plan_to_implement": "required"}} + return registry.Project{ID: "p", Verification: registry.VerificationPolicy{Allowed: [][]string{{"go", "test", "*"}}}, TrajectoryGate: map[string]string{"plan_to_implement": "required"}} } func humanReply(t *testing.T, s *store.Store, taskID, id, value string) { @@ -150,7 +150,7 @@ None. // An ungated project never stops. func TestUngatedProjectAdvances(t *testing.T) { s, id := phaseStore(t) - project := registry.Project{ID: "p"} + project := registry.Project{ID: "p", Verification: registry.VerificationPolicy{Allowed: [][]string{{"go", "test", "*"}}}} if _, err := AdvanceWorkPhase(s, project, id, nil); err != nil { t.Fatal(err) } diff --git a/internal/operations/workphase.go b/internal/operations/workphase.go index c3ef9d3..e9f5fd3 100644 --- a/internal/operations/workphase.go +++ b/internal/operations/workphase.go @@ -84,6 +84,15 @@ func advanceWorkPhase(s *store.Store, project registry.Project, taskID string, a if err := resolvePlanReferences(s, t, doc); err != nil { return domain.Event{}, err } + // The project decides what a plan command may execute, and the + // brief promises the planner it learns that at seal time. Run 9 + // sealed ["bash", "scripts/test_healthcheck.sh"] against a policy + // that allows neither shape: the only check lived in + // PlanPhaseCommands, which runs when the implementer asks to + // verify, one phase and one session too late. + if err := resolvePlanCommands(project, doc); err != nil { + return domain.Event{}, err + } } ref, err := s.PutArtifact(artifact) if err != nil { @@ -180,6 +189,20 @@ func phaseOperation(s *store.Store, taskID, operationID string) (domain.Event, b return domain.Event{}, false } +// resolvePlanCommands refuses a plan whose automated checks fall outside the +// project's verification policy. It fails on the planner, whose session is +// still alive to correct it, rather than on the implementer that inherits it. +func resolvePlanCommands(project registry.Project, doc workphase.PlanDoc) error { + for _, phase := range doc.Phases { + for _, argv := range phase.Automated { + if allowed, why := project.Verification.Allows(argv); !allowed { + return fmt.Errorf("%w: %s verification: %s", domain.ErrInvalid, phase.ID, why) + } + } + } + return nil +} + // resolvePlanReferences refuses a plan that cites research the task never // sealed. Its cost is one CAS read against a ref the coordinator already // holds. diff --git a/internal/operations/workphase_test.go b/internal/operations/workphase_test.go index e7aa879..494b5be 100644 --- a/internal/operations/workphase_test.go +++ b/internal/operations/workphase_test.go @@ -3,6 +3,7 @@ package operations import ( "encoding/json" "errors" + "strings" "testing" "time" @@ -47,6 +48,7 @@ func sealed(t *testing.T, v interface{ Validate() error }) []byte { } var research = workphase.Research{Findings: []workphase.Finding{{ID: "r1", Confidence: workphase.Fact, Claim: "runs per figure", Evidence: "attr.go:88"}}} + // planDoc is a real sealed specification. It cites research:r1, which the // research fixture above contains, so the reference check has something to // resolve. @@ -94,7 +96,7 @@ None. func TestFullPhasePathSealsEachArtifact(t *testing.T) { s, id := phaseStore(t) - project := registry.Project{ID: "p"} + project := registry.Project{ID: "p", Verification: registry.VerificationPolicy{Allowed: [][]string{{"go", "test", "*"}}}} // frame -> research needs no artifact: framing produces none. if _, err := AdvanceWorkPhase(s, project, id, nil); err != nil { @@ -154,7 +156,7 @@ func TestFullPhasePathSealsEachArtifact(t *testing.T) { // A project that declares a short path skips the phases it omits. func TestProjectPathSkipsUndeclaredPhases(t *testing.T) { s, id := phaseStore(t) - project := registry.Project{ID: "p", WorkPhases: []domain.WorkPhase{domain.WorkPhaseFrame, domain.WorkPhaseImplement, domain.WorkPhaseReview}} + project := registry.Project{ID: "p", Verification: registry.VerificationPolicy{Allowed: [][]string{{"go", "test", "*"}}}, WorkPhases: []domain.WorkPhase{domain.WorkPhaseFrame, domain.WorkPhaseImplement, domain.WorkPhaseReview}} if _, err := AdvanceWorkPhase(s, project, id, nil); err != nil { t.Fatal(err) } @@ -180,7 +182,7 @@ func TestIllegalTransitionRejectedAtTheAppendBoundary(t *testing.T) { func TestEndOfPathIsRefused(t *testing.T) { s, id := phaseStore(t) - project := registry.Project{ID: "p", WorkPhases: []domain.WorkPhase{domain.WorkPhaseFrame}} + project := registry.Project{ID: "p", Verification: registry.VerificationPolicy{Allowed: [][]string{{"go", "test", "*"}}}, WorkPhases: []domain.WorkPhase{domain.WorkPhaseFrame}} if _, err := AdvanceWorkPhase(s, project, id, nil); !errors.Is(err, domain.ErrInvalid) { t.Fatalf("want ErrInvalid at the end of the path, got %v", err) } @@ -192,7 +194,7 @@ func TestPhaseChangeDoesNotTouchLifecycle(t *testing.T) { t.Fatal(err) } before, _ := s.Task(id) - if _, err := AdvanceWorkPhase(s, registry.Project{ID: "p"}, id, nil); err != nil { + if _, err := AdvanceWorkPhase(s, registry.Project{ID: "p", Verification: registry.VerificationPolicy{Allowed: [][]string{{"go", "test", "*"}}}}, id, nil); err != nil { t.Fatal(err) } after, _ := s.Task(id) @@ -206,3 +208,39 @@ func TestPhaseChangeDoesNotTouchLifecycle(t *testing.T) { t.Fatalf("phase = %q", after.WorkPhase) } } + +// The brief promises the planner that a command outside the project's policy +// is refused when the plan seals. Run 9 sealed +// ["bash", "scripts/test_healthcheck.sh"] against a policy allowing neither +// shape: the only check lived in PlanPhaseCommands, one phase too late. +func TestPlanSealRefusesACommandOutsideProjectPolicy(t *testing.T) { + s, id := phaseStore(t) + project := registry.Project{ID: "p", Verification: registry.VerificationPolicy{Allowed: [][]string{{"go", "vet", "./..."}}}} + 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) + } + _, err := AdvanceWorkPhase(s, project, id, planDoc) + if !errors.Is(err, domain.ErrInvalid) { + t.Fatalf("a plan command outside policy must be refused at seal, got %v", err) + } + for _, want := range []string{"phase-1", "go test"} { + if !strings.Contains(err.Error(), want) { + t.Errorf("refusal never names %q: %v", want, err) + } + } + if got, _ := s.Task(id); got.PlanRef != "" || got.WorkPhase != domain.WorkPhasePlan { + t.Fatalf("a refused plan was sealed anyway: %+v", got) + } + + // The same plan seals once the project allows the command. + project.Verification.Allowed = append(project.Verification.Allowed, []string{"go", "test", "*"}) + if _, err := AdvanceWorkPhase(s, project, id, planDoc); err != nil { + t.Fatal(err) + } + if got, _ := s.Task(id); got.PlanRef == "" { + t.Fatal("an allowed plan did not seal") + } +}