1f5bf7e66e
The brief told the agent to ask for a phase change and never carried the asking. The agent asked in prose, no code represented the request, and the session idled until its lease expired. That is what failed run 3. F21. The agent asks with .orchestra/phase-request.json, and seals research.json or plan.json where the phase it is leaving produces one. At a verified turn boundary the worker checks the phase belief, the transition and the artifact, then calls the coordinator with its lease epoch and a derived operation id. AdvanceWorkPhase is unchanged, so a request cannot reach a move the operator surface could not also make. Redelivery is idempotent. F22. A session now records the phase it was launched to run. One that no longer matches its task rotates with reason phase_changed, whether this worker asked for the change or an operator made it. F20. CLIAdapter.prompt sent handoff and rotation prompts without confirming them, which is the failure F20 exists to catch. Fixed at the shared call site. F23 needed no change. Issue comments already become decisions with no submission, through Reconciler.Reconcile at PreLease and at every turn boundary. The earlier finding searched internal/operations alone and was wrong. Tests now cover the boundary it turns on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011xsXyr5J1RACo71YeKG3Pu
168 lines
5.7 KiB
Go
168 lines
5.7 KiB
Go
package operations
|
|
|
|
import (
|
|
"encoding/json"
|
|
"errors"
|
|
"testing"
|
|
|
|
"orchestra/internal/domain"
|
|
"orchestra/internal/registry"
|
|
"orchestra/internal/store"
|
|
)
|
|
|
|
// epochOf returns the epoch of the task's current lease, which fences every
|
|
// worker-driven call.
|
|
func epochOf(t *testing.T, s *store.Store, id string) string {
|
|
t.Helper()
|
|
task, ok := s.Task(id)
|
|
if !ok || task.Lease == nil {
|
|
t.Fatal("task has no lease")
|
|
}
|
|
return task.Lease.Epoch
|
|
}
|
|
|
|
func TestRequestWorkPhaseAdvancesAndSealsEachArtifact(t *testing.T) {
|
|
s, id := phaseStore(t)
|
|
project := registry.Project{ID: "p"}
|
|
lease(t, s, id)
|
|
epoch := epochOf(t, s, id)
|
|
|
|
// frame -> research seals nothing: framing produces no artifact.
|
|
if _, err := RequestWorkPhase(s, project, id, epoch, "op-1", domain.WorkPhaseFrame, domain.WorkPhaseResearch, nil); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if got, _ := s.Task(id); got.WorkPhase != domain.WorkPhaseResearch {
|
|
t.Fatalf("phase = %q", got.WorkPhase)
|
|
}
|
|
|
|
// research -> plan is refused until the research is sealed.
|
|
if _, err := RequestWorkPhase(s, project, id, epoch, "op-2", domain.WorkPhaseResearch, domain.WorkPhasePlan, nil); !errors.Is(err, domain.ErrInvalid) {
|
|
t.Fatalf("leaving research unsealed must fail, got %v", err)
|
|
}
|
|
if _, err := RequestWorkPhase(s, project, id, epoch, "op-3", domain.WorkPhaseResearch, domain.WorkPhasePlan, sealed(t, research)); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
got, _ := s.Task(id)
|
|
if got.WorkPhase != domain.WorkPhasePlan || got.ResearchRef == "" {
|
|
t.Fatalf("task = %+v", got)
|
|
}
|
|
|
|
// plan -> implement is refused until the plan is sealed.
|
|
if _, err := RequestWorkPhase(s, project, id, epoch, "op-4", domain.WorkPhasePlan, domain.WorkPhaseImplement, nil); !errors.Is(err, domain.ErrInvalid) {
|
|
t.Fatalf("leaving plan unsealed must fail, got %v", err)
|
|
}
|
|
if _, err := RequestWorkPhase(s, project, id, epoch, "op-5", domain.WorkPhasePlan, domain.WorkPhaseImplement, sealed(t, plan)); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
got, _ = s.Task(id)
|
|
if got.WorkPhase != domain.WorkPhaseImplement || got.PlanRef == "" || got.ResearchRef == "" {
|
|
t.Fatalf("task = %+v", got)
|
|
}
|
|
}
|
|
|
|
func TestRequestWorkPhaseRefusesASkippedPhase(t *testing.T) {
|
|
s, id := phaseStore(t)
|
|
lease(t, s, id)
|
|
// 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)
|
|
if !errors.Is(err, ErrPhaseRequest) {
|
|
t.Fatalf("err = %v", err)
|
|
}
|
|
if got, _ := s.Task(id); got.WorkPhase != "" {
|
|
t.Fatalf("phase moved to %q", got.WorkPhase)
|
|
}
|
|
}
|
|
|
|
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 {
|
|
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)
|
|
if !errors.Is(err, ErrPhaseRequest) {
|
|
t.Fatalf("err = %v", err)
|
|
}
|
|
}
|
|
|
|
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)
|
|
if !errors.Is(err, domain.ErrConflict) {
|
|
t.Fatalf("err = %v", err)
|
|
}
|
|
if got, _ := s.Task(id); got.WorkPhase != "" {
|
|
t.Fatalf("phase moved to %q", got.WorkPhase)
|
|
}
|
|
}
|
|
|
|
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)
|
|
if !errors.Is(err, domain.ErrInvalid) {
|
|
t.Fatalf("err = %v", err)
|
|
}
|
|
}
|
|
|
|
// A lost response is the ordinary case, not the exotic one: the worker resends
|
|
// 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"}
|
|
lease(t, s, id)
|
|
epoch := epochOf(t, s, id)
|
|
|
|
first, err := RequestWorkPhase(s, project, id, epoch, "op-1", domain.WorkPhaseFrame, domain.WorkPhaseResearch, nil)
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
second, err := RequestWorkPhase(s, project, id, epoch, "op-1", domain.WorkPhaseFrame, domain.WorkPhaseResearch, nil)
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if second.ID != first.ID {
|
|
t.Fatalf("redelivery produced a new event: %s then %s", first.ID, second.ID)
|
|
}
|
|
if got, _ := s.Task(id); got.WorkPhase != domain.WorkPhaseResearch {
|
|
t.Fatalf("phase = %q", got.WorkPhase)
|
|
}
|
|
var changes int
|
|
for _, e := range s.Events(0) {
|
|
if e.TaskID == id && e.Type == domain.EventWorkPhaseChanged {
|
|
changes++
|
|
}
|
|
}
|
|
if changes != 1 {
|
|
t.Fatalf("recorded %d phase changes, want 1", changes)
|
|
}
|
|
}
|
|
|
|
// The operation id is what makes redelivery safe, so it has to survive into
|
|
// the event the next redelivery reads.
|
|
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)
|
|
if err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
var p struct {
|
|
OperationID string `json:"operation_id"`
|
|
From string `json:"from"`
|
|
Phase string `json:"phase"`
|
|
}
|
|
if err := json.Unmarshal(e.Payload, &p); err != nil {
|
|
t.Fatal(err)
|
|
}
|
|
if p.OperationID != "op-1" || p.From != "frame" || p.Phase != "research" {
|
|
t.Fatalf("payload = %+v", p)
|
|
}
|
|
}
|