76da8c40b7
Run 20 showed the leak precisely: the reopen rotated the session, and 31 seconds later the planning session that replaced it recorded phase-2 of the plan being replaced. The implementer's .orchestra/plan-progress.json had outlived it in the worktree, and the successor executed it. Two changes. Phase verification now belongs to the implement phase and is refused anywhere else, which covers every worker and every path rather than the one that produced it. The rotation also drops the ended session's request files, so a successor never inherits work from a trajectory Orchestra has already invalidated. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CVbaKucEYBjMqVeUgJUsc1
195 lines
7.8 KiB
Go
195 lines
7.8 KiB
Go
package operations
|
|
|
|
import (
|
|
"encoding/json"
|
|
"fmt"
|
|
"strings"
|
|
"time"
|
|
|
|
"orchestra/internal/authz"
|
|
"orchestra/internal/domain"
|
|
"orchestra/internal/registry"
|
|
"orchestra/internal/store"
|
|
"orchestra/internal/workphase"
|
|
)
|
|
|
|
// ErrPlanPhase reports that a verification request cannot proceed. The reason
|
|
// is always specific, because it is delivered to a live implementer that has
|
|
// to act on it.
|
|
var ErrPlanPhase = fmt.Errorf("plan phase verification refused")
|
|
|
|
// VerificationRun is the outcome of one command the worker executed.
|
|
type VerificationRun struct {
|
|
Command []string `json:"command"`
|
|
ExitCode int `json:"exit_code"`
|
|
Output string `json:"output,omitempty"`
|
|
}
|
|
|
|
// PlanPhaseCommands resolves the commands a phase's verification will run.
|
|
//
|
|
// The commands come from the accepted plan, never from the request. An
|
|
// implementer asks to verify a phase; what that phase is checked with was
|
|
// settled when the plan was sealed, so a request cannot smuggle in a command
|
|
// the planner did not write.
|
|
//
|
|
// Every command is checked against the project's policy before any of them
|
|
// runs. A partial execution followed by a refusal would leave side effects
|
|
// behind with nothing recording them.
|
|
func PlanPhaseCommands(s *store.Store, project registry.Project, taskID, phaseID string) (workphase.PlanPhase, error) {
|
|
t, ok := s.Task(taskID)
|
|
if !ok {
|
|
return workphase.PlanPhase{}, domain.ErrNotFound
|
|
}
|
|
if t.PlanRef == "" {
|
|
return workphase.PlanPhase{}, fmt.Errorf("%w: this task has no accepted plan", ErrPlanPhase)
|
|
}
|
|
// Verification is implementation work. A request that arrives in another
|
|
// phase belongs to a trajectory Orchestra has already ended: run 20's
|
|
// reopened planning session executed the implementer's leftover request
|
|
// and recorded a phase of a plan that was being replaced.
|
|
if current(t) != domain.WorkPhaseImplement {
|
|
return workphase.PlanPhase{}, fmt.Errorf("%w: phase verification belongs to the implement phase, and this task is in %s", ErrPlanPhase, current(t))
|
|
}
|
|
raw, err := s.Artifact(t.PlanRef)
|
|
if err != nil {
|
|
return workphase.PlanPhase{}, fmt.Errorf("read accepted plan: %w", err)
|
|
}
|
|
doc, err := workphase.DecodeStoredPlan(raw)
|
|
if err != nil {
|
|
return workphase.PlanPhase{}, fmt.Errorf("read accepted plan: %w", err)
|
|
}
|
|
if len(doc.Phases) == 0 {
|
|
// A plan sealed before plan.md names no executable unit. Saying so is
|
|
// the honest answer; inventing a phase would make progress against a
|
|
// plan that never had any.
|
|
return workphase.PlanPhase{}, fmt.Errorf("%w: the accepted plan predates plan.md and declares no phases, so phase progress does not apply to it", ErrPlanPhase)
|
|
}
|
|
phase, ok := doc.Phase(phaseID)
|
|
if !ok {
|
|
return workphase.PlanPhase{}, fmt.Errorf("%w: the accepted plan has no %s; it has %s", ErrPlanPhase, phaseID, phaseNames(doc))
|
|
}
|
|
for _, argv := range phase.Automated {
|
|
if allowed, why := project.Verification.Allows(argv); !allowed {
|
|
// Refused before anything ran. The planner wrote a command the
|
|
// project does not permit, and the implementer is told rather
|
|
// than left retrying a phase that can never verify.
|
|
return workphase.PlanPhase{}, fmt.Errorf("%w: %s declares a command %s cannot run: %s", ErrPlanPhase, phaseID, project.ID, why)
|
|
}
|
|
}
|
|
return phase, nil
|
|
}
|
|
|
|
func phaseNames(doc workphase.PlanDoc) string {
|
|
out := make([]string, 0, len(doc.Phases))
|
|
for _, p := range doc.Phases {
|
|
out = append(out, p.ID)
|
|
}
|
|
return strings.Join(out, ", ")
|
|
}
|
|
|
|
// RecordPlanPhaseVerification establishes what the runs prove, and appends the
|
|
// durable record.
|
|
//
|
|
// The status is derived here and never taken from the caller. An implementer
|
|
// may request verification; only this function decides whether a phase is
|
|
// verified, awaiting a human, or still in progress.
|
|
func RecordPlanPhaseVerification(s *store.Store, project registry.Project, taskID, phaseID, atSHA string, runs []VerificationRun) (domain.Event, error) {
|
|
t, ok := s.Task(taskID)
|
|
if !ok {
|
|
return domain.Event{}, domain.ErrNotFound
|
|
}
|
|
if len(atSHA) != 40 {
|
|
return domain.Event{}, fmt.Errorf("%w: verification must name the commit it ran against", ErrPlanPhase)
|
|
}
|
|
phase, err := PlanPhaseCommands(s, project, taskID, phaseID)
|
|
if err != nil {
|
|
return domain.Event{}, err
|
|
}
|
|
if len(runs) != len(phase.Automated) {
|
|
return domain.Event{}, fmt.Errorf("%w: %s declares %d automated commands but %d results were reported", ErrPlanPhase, phaseID, len(phase.Automated), len(runs))
|
|
}
|
|
record := domain.PlanPhaseRecord{
|
|
PlanRef: t.PlanRef, PhaseID: phaseID, AtSHA: atSHA,
|
|
Status: domain.PlanPhaseVerified, At: time.Now().UTC(),
|
|
}
|
|
for i, r := range runs {
|
|
// The result must describe the command the plan named. A reordered or
|
|
// substituted result would attribute one command's exit code to
|
|
// another.
|
|
if strings.Join(r.Command, "\x00") != strings.Join(phase.Automated[i], "\x00") {
|
|
return domain.Event{}, fmt.Errorf("%w: result %d reports %q but the plan declares %q", ErrPlanPhase, i, strings.Join(r.Command, " "), strings.Join(phase.Automated[i], " "))
|
|
}
|
|
record.Commands = append(record.Commands, r.Command)
|
|
record.ExitCodes = append(record.ExitCodes, r.ExitCode)
|
|
if r.ExitCode != 0 {
|
|
record.Status = domain.PlanPhaseInProgress
|
|
}
|
|
}
|
|
if record.Status == domain.PlanPhaseVerified && len(phase.Manual) > 0 {
|
|
// Automated checks passing is not the whole phase. A human owns the
|
|
// manual steps, and the phase waits rather than claiming more than
|
|
// was established.
|
|
record.Status = domain.PlanPhaseAwaitingManual
|
|
}
|
|
// Carry the confirmed tree forward as provenance. Without it a second
|
|
// rerun would compare against nothing and re-inherit the sign-off.
|
|
if prior, ok := t.PlanPhase(phaseID); ok {
|
|
record.ManualAtSHA = prior.ManualAtSHA
|
|
}
|
|
if record.Status != domain.PlanPhaseInProgress && manuallySignedOff(s, t, phaseID, record) {
|
|
record.Status = domain.PlanPhaseVerified
|
|
}
|
|
if ref, err := s.PutArtifact(verificationEvidence(runs)); err == nil {
|
|
record.EvidenceRef = ref
|
|
} else {
|
|
return domain.Event{}, fmt.Errorf("store verification evidence: %w", err)
|
|
}
|
|
payload := map[string]any{
|
|
"plan_ref": record.PlanRef, "phase_id": record.PhaseID, "status": string(record.Status),
|
|
"commands": record.Commands, "exit_codes": record.ExitCodes, "at_sha": record.AtSHA,
|
|
"evidence_ref": record.EvidenceRef, "at": record.At, "manual_at_sha": record.ManualAtSHA,
|
|
}
|
|
if t.Lease != nil {
|
|
payload["harness_id"], payload["lease_epoch"] = t.Lease.HarnessID, t.Lease.Epoch
|
|
}
|
|
b, err := json.Marshal(payload)
|
|
if err != nil {
|
|
return domain.Event{}, err
|
|
}
|
|
e := domain.Event{ID: domain.NewID(), Type: domain.EventPlanPhaseVerified, TaskID: taskID, Version: t.Version + 1, Payload: b, Surface: string(authz.System)}
|
|
return e, s.Append(e)
|
|
}
|
|
|
|
// manuallySignedOff reports whether a human has already approved this exact
|
|
// phase of this exact plan, against the tree this run examined. The subject
|
|
// carries plan and phase, so a later "looks good" on an unrelated thread
|
|
// cannot satisfy a gate nobody was discussing.
|
|
//
|
|
// The tree matters as much as the subject (F63). A sign-off is a human saying
|
|
// they read what this code prints; an edit afterwards can change exactly that.
|
|
// A record whose ManualAtSHA names a different commit is therefore not signed
|
|
// off, and waits for the human again. A sign-off given before any run has no
|
|
// confirmed tree to compare against and still counts, which keeps the ordinary
|
|
// ordering unchanged.
|
|
func manuallySignedOff(s *store.Store, t domain.Task, phaseID string, record domain.PlanPhaseRecord) bool {
|
|
if record.ManualAtSHA != "" && record.ManualAtSHA != record.AtSHA {
|
|
return false
|
|
}
|
|
intent, err := s.EffectiveIntent(t.ID)
|
|
if err != nil {
|
|
return false
|
|
}
|
|
subject := domain.PlanPhaseSubject(t.PlanRef, phaseID)
|
|
for _, d := range intent.Decisions {
|
|
if d.Subject == subject {
|
|
return true
|
|
}
|
|
}
|
|
return false
|
|
}
|
|
|
|
func verificationEvidence(runs []VerificationRun) []byte {
|
|
b, _ := json.Marshal(runs)
|
|
return b
|
|
}
|