Let the claude harness reach the turn boundary at all
F25. rotationTick returned early for claude before reaching federatedTurn, which has one call site below that return. On the harness both burn-in runs used, no phase request could ever be read and every human decision recorded against a live session went undelivered. Claude still skips the occupancy state machine below, because it owns its context rollover through the installed hook. A turn boundary is not a rotation. F26. The phase brief listed every domain-legal target, so run 4's frame session read "research, implement" and asked for implement, which the project's path refuses. The path is Orchestra's to know: the brief now names one step and says a wrong target comes back with the right one. F27. A refused request only reached recordError, leaving the agent to rewrite the same rejected file forever with nothing telling it why. federation. StatusError makes a 409 classifiable, and the refusal is delivered through sendPrompt under the F20 guarantee. A transport failure is not an answer: the request survives and the agent is told nothing. The F25 regression test fails against the unfixed rotationTick. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011xsXyr5J1RACo71YeKG3Pu
This commit is contained in:
@@ -12,6 +12,7 @@ import (
|
||||
"errors"
|
||||
"fmt"
|
||||
"log"
|
||||
"net/http"
|
||||
"orchestra/internal/agentctx"
|
||||
"orchestra/internal/buildinfo"
|
||||
"orchestra/internal/continuity"
|
||||
@@ -609,6 +610,23 @@ func (w *worker) rotationTick(ctx context.Context, id string, s herdr.Session) {
|
||||
if err := w.advanceClaudeContextReset(ctx, id, s); err != nil {
|
||||
w.recordError(fmt.Errorf("Claude context reset %s: %w", id, err))
|
||||
}
|
||||
// A turn boundary is not a rotation. Claude owns its context rollover
|
||||
// through the installed hook, which is why the occupancy state machine
|
||||
// below is skipped, but phase requests and human decisions are carried
|
||||
// at the boundary and returning here left both unreachable on this
|
||||
// harness. Every decision recorded against a live Claude session went
|
||||
// undelivered, and no phase request could ever be read.
|
||||
t, ok := w.tasks[id]
|
||||
if !ok {
|
||||
w.recordError(fmt.Errorf("turn boundary %s: task cache missing", id))
|
||||
return
|
||||
}
|
||||
p, err := w.project(t)
|
||||
if err != nil {
|
||||
w.recordError(err)
|
||||
return
|
||||
}
|
||||
w.federatedTurn(ctx, id, w.adapter(s, p.Remote), orchestrator.TurnContinue)
|
||||
return
|
||||
}
|
||||
t, ok := w.tasks[id]
|
||||
@@ -1727,6 +1745,22 @@ func (w *worker) rotateForPhase(ctx context.Context, id string, a herdr.Adapter,
|
||||
log.Printf("phase changed for %s: session rotating", id)
|
||||
}
|
||||
|
||||
// answerRefusedPhase tells the agent why its request was refused and drops the
|
||||
// file so it can write a corrected one. The request survives a failed send, so
|
||||
// the refusal is delivered at the next boundary instead of being lost.
|
||||
func (w *worker) answerRefusedPhase(ctx context.Context, id string, s herdr.Session, path, reason string) {
|
||||
text := "Orchestra refused your phase request: " + reason +
|
||||
"\n\nWrite a corrected .orchestra/phase-request.json, or keep working in the current phase. Do not repeat the refused request."
|
||||
if err := w.sendPrompt(ctx, s, text); err != nil {
|
||||
w.recordError(fmt.Errorf("deliver phase refusal %s: %w", id, err))
|
||||
return
|
||||
}
|
||||
if err := os.Remove(path); err != nil {
|
||||
w.recordError(fmt.Errorf("phase request %s: %w", id, err))
|
||||
}
|
||||
log.Printf("phase request %s refused: %s", id, reason)
|
||||
}
|
||||
|
||||
// requestPhase carries an agent's phase request to the coordinator (F21).
|
||||
// It reports whether the phase moved.
|
||||
//
|
||||
@@ -1786,6 +1820,15 @@ func (w *worker) requestPhase(ctx context.Context, id string, s herdr.Session) b
|
||||
phase, err := w.api.AdvancePhase(ctx, id, l.Epoch, op, req.From, req.To, artifact)
|
||||
if err != nil {
|
||||
w.recordError(fmt.Errorf("phase request %s: %w", id, err))
|
||||
// A refusal is an answer, and it names the phase the agent may ask
|
||||
// for. Recording it only in worker health would leave the agent
|
||||
// rewriting the same rejected file at every boundary with nothing
|
||||
// telling it why, which is the silent-loop shape this codebase keeps
|
||||
// producing. A transport failure is not an answer and is retried.
|
||||
var status *federation.StatusError
|
||||
if errors.As(err, &status) && status.Code == http.StatusConflict {
|
||||
w.answerRefusedPhase(ctx, id, s, path, status.Body)
|
||||
}
|
||||
return false
|
||||
}
|
||||
if phase == "" {
|
||||
|
||||
@@ -199,29 +199,59 @@ func TestPhaseRequestRefusesAMalformedArtifact(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// A refused request keeps the pane's own state out of it: the file stays, so
|
||||
// the same ask is retried under the same operation id once the reason clears.
|
||||
func TestRefusedPhaseRequestIsRetried(t *testing.T) {
|
||||
calls := 0
|
||||
// A refusal is an answer. The agent is told why, in the same confirmed
|
||||
// delivery path every other Orchestra-originated input uses, and the request
|
||||
// is cleared so it can write a corrected one instead of resending the same
|
||||
// rejected file at every boundary.
|
||||
func TestRefusedPhaseRequestIsAnsweredAndCleared(t *testing.T) {
|
||||
w, backend, wt, done := phaseWorker(t, func(rw http.ResponseWriter, r *http.Request) {
|
||||
calls++
|
||||
if calls == 1 {
|
||||
http.Error(rw, "conflict", http.StatusConflict)
|
||||
if r.URL.Path == "/v1/federation/phase" {
|
||||
http.Error(rw, "phase request refused: task may only move to \"research\", not \"implement\"", http.StatusConflict)
|
||||
return
|
||||
}
|
||||
_ = json.NewEncoder(rw).Encode(map[string]string{"phase": "research"})
|
||||
rw.Write([]byte(`{"verdict":"continue"}`))
|
||||
})
|
||||
defer done()
|
||||
writeRequest(t, wt, domain.WorkPhaseFrame, domain.WorkPhaseImplement)
|
||||
|
||||
a := herdr.CLIAdapter{Backend: backend, Harness: "claude"}
|
||||
w.federatedTurn(context.Background(), "task", a, "continue")
|
||||
|
||||
if s := w.sessions["task"]; s.HandoffRequested {
|
||||
t.Fatal("a refused request rotated the session")
|
||||
}
|
||||
if len(backend.prompts) == 0 || !strings.Contains(backend.prompts[0], `may only move to "research"`) {
|
||||
t.Fatalf("the agent was not told why: %q", backend.prompts)
|
||||
}
|
||||
if _, err := os.Stat(filepath.Join(wt, ".orchestra", phaseRequestFile)); !os.IsNotExist(err) {
|
||||
t.Fatal("the refused request survived, so the agent will resend it")
|
||||
}
|
||||
}
|
||||
|
||||
// A coordinator that cannot be reached has not refused anything. Telling the
|
||||
// agent its request was rejected would be a lie, and dropping the file would
|
||||
// lose a request that is still valid.
|
||||
func TestTransientPhaseFailureKeepsTheRequest(t *testing.T) {
|
||||
w, backend, wt, done := phaseWorker(t, func(rw http.ResponseWriter, r *http.Request) {
|
||||
if r.URL.Path == "/v1/federation/phase" {
|
||||
http.Error(rw, "upstream down", http.StatusServiceUnavailable)
|
||||
return
|
||||
}
|
||||
rw.Write([]byte(`{"verdict":"continue"}`))
|
||||
})
|
||||
defer done()
|
||||
writeRequest(t, wt, domain.WorkPhaseFrame, domain.WorkPhaseResearch)
|
||||
a := herdr.CLIAdapter{Backend: backend, Harness: "claude"}
|
||||
|
||||
a := herdr.CLIAdapter{Backend: backend, Harness: "claude"}
|
||||
w.federatedTurn(context.Background(), "task", a, "continue")
|
||||
|
||||
if _, err := os.Stat(filepath.Join(wt, ".orchestra", phaseRequestFile)); err != nil {
|
||||
t.Fatal("a refused request must survive for the retry")
|
||||
t.Fatal("a transient failure discarded the request")
|
||||
}
|
||||
w.federatedTurn(context.Background(), "task", a, "continue")
|
||||
if s := w.sessions["task"]; !s.HandoffRequested {
|
||||
t.Fatalf("the retry did not advance the phase: %+v", s)
|
||||
for _, p := range backend.prompts {
|
||||
if strings.Contains(p, "refused") {
|
||||
t.Fatalf("a transient failure was reported to the agent as a refusal: %q", p)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -290,3 +320,42 @@ func TestDecisionNoticeStaysUndeliveredUntilConfirmed(t *testing.T) {
|
||||
t.Fatalf("sends = %d, want the correction retried once", len(backend.prompts))
|
||||
}
|
||||
}
|
||||
|
||||
// The bug that made run 4 stall exactly like run 3. rotationTick returned
|
||||
// early for the claude harness before reaching the turn boundary, so
|
||||
// federatedTurn had one call site that this harness never took. Phase requests
|
||||
// were never read and human decisions were never delivered on the harness both
|
||||
// burn-in runs actually used.
|
||||
//
|
||||
// Claude still skips the occupancy state machine below that branch, because it
|
||||
// owns its own context rollover. A turn boundary is not a rotation.
|
||||
func TestClaudeHarnessReachesTheTurnBoundary(t *testing.T) {
|
||||
reached := make(chan string, 4)
|
||||
w, backend, wt, done := phaseWorker(t, func(rw http.ResponseWriter, r *http.Request) {
|
||||
reached <- r.URL.Path
|
||||
if r.URL.Path == "/v1/federation/phase" {
|
||||
_ = json.NewEncoder(rw).Encode(map[string]string{"phase": "research"})
|
||||
return
|
||||
}
|
||||
rw.Write([]byte(`{"verdict":"continue"}`))
|
||||
})
|
||||
defer done()
|
||||
writeRequest(t, wt, domain.WorkPhaseFrame, domain.WorkPhaseResearch)
|
||||
|
||||
// rotationTick, not federatedTurn: the dead path was the route in.
|
||||
w.rotationTick(context.Background(), "task", w.sessions["task"])
|
||||
|
||||
var saw bool
|
||||
for len(reached) > 0 {
|
||||
if <-reached == "/v1/federation/phase" {
|
||||
saw = true
|
||||
}
|
||||
}
|
||||
if !saw {
|
||||
t.Fatal("the claude harness never reached the phase boundary")
|
||||
}
|
||||
if s := w.sessions["task"]; !s.HandoffRequested || s.HandoffReason != "phase_changed" {
|
||||
t.Fatalf("session did not rotate: %+v", s)
|
||||
}
|
||||
_ = backend
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user