From 0bd86e28c600b19613379640692c1cb204bd81aa Mon Sep 17 00:00:00 2001 From: kami Date: Fri, 28 Aug 2026 01:58:36 +0400 Subject: [PATCH] Tell the agent the artifact shape, and tell it when the shape is wrong Two halves of the same failure, live on run 5. F38: the phase brief named .orchestra/research.json and described its contents in prose, never its schema. The agent guessed dead_ends as strings where the decoder wants {tried, why_failed} objects. The brief now carries the shape, and a test decodes each documented shape with the same function the worker uses, so a struct change that is not mirrored fails the build. F39: the local artifact check refused the request through recordError alone. answerRefusedPhase only ran on a coordinator 409, so a decode failure told the agent nothing. The session sat at a boundary rewriting nothing, which is the silent-loop shape the comment above that block warns about, reached by the one path with no delivery. Both local refusals now reach the agent. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_011xsXyr5J1RACo71YeKG3Pu --- cmd/orchestra-worker/main.go | 23 +++++++++++++------- internal/agentctx/agentctx.go | 27 ++++++++++++++++++++++++ internal/agentctx/agentctx_test.go | 34 ++++++++++++++++++++++++++++++ 3 files changed, 76 insertions(+), 8 deletions(-) diff --git a/cmd/orchestra-worker/main.go b/cmd/orchestra-worker/main.go index 3481a25..d8eacbb 100644 --- a/cmd/orchestra-worker/main.go +++ b/cmd/orchestra-worker/main.go @@ -1834,19 +1834,26 @@ func (w *worker) requestPhase(ctx context.Context, id string, s herdr.Session) b artifact, err = os.ReadFile(filepath.Join(s.Worktree, ".orchestra", name)) if err != nil { w.recordError(fmt.Errorf("phase request %s: work phase %q must seal .orchestra/%s first: %w", id, req.From, name, err)) + w.answerRefusedPhase(ctx, id, s, path, fmt.Sprintf("work phase %q must seal .orchestra/%s first: %v", req.From, name, err)) return false } + var decErr error switch req.From { case domain.WorkPhaseResearch: - if _, decErr := workphase.DecodeResearch(artifact); decErr != nil { - w.recordError(fmt.Errorf("phase request %s: research artifact: %w", id, decErr)) - return false - } + _, decErr = workphase.DecodeResearch(artifact) case domain.WorkPhasePlan: - if _, decErr := workphase.DecodePlan(artifact); decErr != nil { - w.recordError(fmt.Errorf("phase request %s: plan artifact: %w", id, decErr)) - return false - } + _, decErr = workphase.DecodePlan(artifact) + } + if decErr != nil { + w.recordError(fmt.Errorf("phase request %s: %s artifact: %w", id, req.From, decErr)) + // A local refusal is still a refusal, and the agent is the only + // party that can fix it. Recording it in worker health alone left + // a live session parked at a boundary forever with nothing telling + // it what was wrong (F39) — the silent-loop shape the comment + // above this block warns about, reached by the one path that had + // no delivery. + w.answerRefusedPhase(ctx, id, s, path, fmt.Sprintf(".orchestra/%s does not match the schema: %v", name, decErr)) + return false } } l := w.leases[id] diff --git a/internal/agentctx/agentctx.go b/internal/agentctx/agentctx.go index 72faf29..94e0c7b 100644 --- a/internal/agentctx/agentctx.go +++ b/internal/agentctx/agentctx.go @@ -125,6 +125,13 @@ func phaseRequestBrief(phase domain.WorkPhase) string { b.WriteString("\nAsk for one step. A request the project's path does not allow is refused, and the refusal names the phase you may ask for.\n") if artifact := phaseSealFile[phase]; artifact != "" { fmt.Fprintf(&b, "\nSeal .orchestra/%s before you ask. The request is refused without it.\n", artifact) + // The shape, not just the filename. Without it the agent has to guess + // a strict JSON schema from prose, and run 5 guessed dead_ends as + // strings where the decoder wants objects (F38). The request was then + // refused on every boundary for a field nobody had described. + if schema := phaseSealSchema[phase]; schema != "" { + fmt.Fprintf(&b, "\nIt must decode as this shape. Optional keys may be omitted, but no key may hold a different type:\n\n%s\n", schema) + } } b.WriteString("\nAn accepted request ends this session and starts the next phase with your sealed result. Saying you are ready in the pane is not a request and nothing reads it.\n") return b.String() @@ -138,6 +145,26 @@ var phaseSealFile = map[domain.WorkPhase]string{ domain.WorkPhasePlan: "plan.json", } +// phaseSealSchema is the shape of each sealed artifact, written out for the +// agent. TestPhaseSealSchemasDecode keeps these honest: each one is decoded by +// the same function the worker uses, so a struct change that is not mirrored +// here fails the build rather than a live run. +var phaseSealSchema = map[domain.WorkPhase]string{ + domain.WorkPhaseResearch: ` { + "findings": [{"id": "", "claim": "", "evidence": "", "confidence": "fact|inference|assumption"}], + "relevant_code": [{"path": "", "why": ""}], + "invariants": [""], + "dead_ends": [{"tried": "", "why_failed": ""}], + "unknowns": [""] + }`, + domain.WorkPhasePlan: ` { + "changes": [{"target": "", "intent": ""}], + "verification": [""], + "risks": [""], + "human_decisions_needed": [""] + }`, +} + // askingBrief narrows step 4 per phase. The bar is not the same everywhere: a // research phase that has not looked yet has no standing to ask, and an // implementation phase asks only when a discovery invalidates the trajectory diff --git a/internal/agentctx/agentctx_test.go b/internal/agentctx/agentctx_test.go index 9c0dfcf..c8cc773 100644 --- a/internal/agentctx/agentctx_test.go +++ b/internal/agentctx/agentctx_test.go @@ -7,6 +7,7 @@ import ( "orchestra/internal/continuity" "orchestra/internal/domain" + "orchestra/internal/workphase" ) func input() Input { @@ -351,3 +352,36 @@ func TestPhaseBriefNamesTheArtifactToSeal(t *testing.T) { } } } + +// TestPhaseSealSchemasDecode guards F38. The brief tells the agent to seal an +// artifact; until this existed it did not say what shape. Run 5 guessed +// dead_ends as strings where the decoder wants objects, and every phase +// request was refused for a field nobody had described. +// +// Each documented shape is decoded by the same function the worker uses, so a +// struct change that is not mirrored in the brief fails here instead of in a +// live run. +func TestPhaseSealSchemasDecode(t *testing.T) { + for phase, schema := range phaseSealSchema { + if schema == "" { + t.Fatalf("%s has a seal file but no documented shape", phase) + } + var decErr error + switch phase { + case domain.WorkPhaseResearch: + _, decErr = workphase.DecodeResearch([]byte(schema)) + case domain.WorkPhasePlan: + _, decErr = workphase.DecodePlan([]byte(schema)) + default: + t.Fatalf("%s has a documented shape with nothing to decode it", phase) + } + if decErr != nil && !strings.Contains(decErr.Error(), "empty") && !strings.Contains(decErr.Error(), "required") && !strings.Contains(decErr.Error(), "must") { + t.Fatalf("%s brief shape does not match the decoder: %v", phase, decErr) + } + } + for phase := range phaseSealFile { + if phaseSealSchema[phase] == "" { + t.Fatalf("%s names a seal file but the brief never states its shape", phase) + } + } +}