From db4803c55736a11c6e2d2488772e16149fc5dfa7 Mon Sep 17 00:00:00 2001 From: Suriya Date: Tue, 22 Sep 2026 13:15:39 +0530 Subject: [PATCH] Report an unsaved trajectory somewhere that survives MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A failed save must not fail the run -- the answer already exists -- but until now the only record of the loss was an error entry appended to the trajectory that had just failed to save. §6 says the trajectory is not optional telemetry; losing one silently is the worst version of losing one. The runtime has no logger by design, so ExecutionResult gains an Unsaved list the surface reads and turns into a §10 log line with run_id, tenant_id, agent_key and agent_version. Never serialised to the client. Covered on all three run paths, streaming included. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PJvibeSc1JYXjatankqM1g --- go-api/internal/httpserver/runs.go | 16 ++++++++++++++++ go-api/internal/runtime/loop.go | 4 ++++ go-api/internal/runtime/loop_test.go | 22 ++++++++++++++++++++++ go-api/internal/runtime/types.go | 12 ++++++++++++ 4 files changed, 54 insertions(+) diff --git a/go-api/internal/httpserver/runs.go b/go-api/internal/httpserver/runs.go index b453a9c..994fbc6 100644 --- a/go-api/internal/httpserver/runs.go +++ b/go-api/internal/httpserver/runs.go @@ -162,10 +162,24 @@ func (s *Server) handleAgentRun(w http.ResponseWriter, r *http.Request) { writeError(w, s.log, runLoadError(runErr)) return } + s.logUnsaved(ident, res) writeJSON(w, http.StatusOK, buildRunResponse(res)) } +// logUnsaved is the operator's record of a run whose trajectory did not +// persist. The run itself already answered; §6 says the trajectory is not +// optional telemetry, so losing one is an error even when nothing else went +// wrong, and it carries every field §10 asks a log line to carry. +func (s *Server) logUnsaved(ident authctx.Identity, res *runtime.ExecutionResult) { + for _, detail := range res.Unsaved { + s.log.Error("trajectory unsaved", + "run_id", res.RunID, "tenant_id", ident.OrgID, + "agent_key", res.AgentID, "agent_version", res.AgentVersion, + "detail", detail) + } +} + // buildRunResponse turns a runtime result into the client's shape. // // Every termination answers 200. That looks wrong at first and is not: the @@ -336,6 +350,7 @@ func (s *Server) streamAgentRun(w http.ResponseWriter, r *http.Request, ident au writeError(w, s.log, runLoadError(runErr)) return } + s.logUnsaved(ident, res) writeJSON(w, http.StatusOK, buildRunResponse(res)) return } @@ -383,6 +398,7 @@ func (s *Server) streamAgentRun(w http.ResponseWriter, r *http.Request, ident au return } + s.logUnsaved(ident, res) send(map[string]any{"run": buildRunResponse(res)}) fmt.Fprint(w, "data: [DONE]\n\n") flusher.Flush() diff --git a/go-api/internal/runtime/loop.go b/go-api/internal/runtime/loop.go index 29b0305..7662ab7 100644 --- a/go-api/internal/runtime/loop.go +++ b/go-api/internal/runtime/loop.go @@ -605,8 +605,10 @@ func (m *ModelExecutor) finish( // A sink that fails must not fail the run — the answer was already // produced. It is recorded in the trajectory we could not save, which is // the best available place for it. + var unsaved []string if err := m.sink.Save(ctx, traj); err != nil { rec.Error("runtime.trajectory_unsaved", err.Error()) + unsaved = append(unsaved, traj.RunID+": "+err.Error()) } // Delegated runs are written AFTER this one, because parent_run_id is a @@ -617,10 +619,12 @@ func (m *ModelExecutor) finish( if err := m.sink.Save(ctx, child); err != nil { rec.Error("runtime.subrun_unsaved", fmt.Sprintf("%s: %s", child.RunID, err.Error())) + unsaved = append(unsaved, child.RunID+": "+err.Error()) } } res := &ExecutionResult{ + Unsaved: unsaved, Success: term == TerminationCompleted, Output: output, AgentID: agent.ID, diff --git a/go-api/internal/runtime/loop_test.go b/go-api/internal/runtime/loop_test.go index e468450..b20e2bd 100644 --- a/go-api/internal/runtime/loop_test.go +++ b/go-api/internal/runtime/loop_test.go @@ -265,6 +265,28 @@ func TestSinkFailureDoesNotFailTheRun(t *testing.T) { if res.Output != "the answer" { t.Errorf("Output = %q, want the answer through", res.Output) } + // ...but the loss must be reported somewhere that survives. The runtime + // has no logger; the surface reads this and writes the operator's line. + // Before this field existed the only record was an entry in the very + // trajectory that had failed to save. + if len(res.Unsaved) != 1 || !strings.Contains(res.Unsaved[0], res.RunID) || + !strings.Contains(res.Unsaved[0], context.DeadlineExceeded.Error()) { + t.Errorf("Unsaved = %v, want one entry naming run %s and the error", res.Unsaved, res.RunID) + } +} + +// A healthy run reports nothing unsaved. Guarded so the surface never logs a +// phantom loss. +func TestHealthySaveReportsNothingUnsaved(t *testing.T) { + gw := &fakeGateway{text: "the answer"} + exec := NewModelExecutor(gw, &MemorySink{}, nil) + res, err := exec.ExecuteAgent(context.Background(), testAgent(), testInput("q")) + if err != nil { + t.Fatal(err) + } + if len(res.Unsaved) != 0 { + t.Errorf("Unsaved = %v on a healthy run, want none", res.Unsaved) + } } type failingSink struct{} diff --git a/go-api/internal/runtime/types.go b/go-api/internal/runtime/types.go index d613f95..9c8fd5c 100644 --- a/go-api/internal/runtime/types.go +++ b/go-api/internal/runtime/types.go @@ -167,6 +167,18 @@ type ExecutionResult struct { // the token of whichever the person approves as ExecutionInput.Confirmation // on the next call. Confirmations []*tools.Confirmation `json:"confirmations,omitempty"` + + // Unsaved names the trajectories this run produced that could not be + // persisted, as ": ". Empty on every healthy run. + // + // A failed save must not fail the run — the answer already exists — but + // it must not vanish either. Until 2026-09-22 the only record of it was + // an entry appended to the trajectory that had just failed to save, which + // is a note left in a bottle that sank. The runtime has no logger by + // design; the surface does, and reads this to write the §10 line an + // operator can grep for. Never serialised to the client: it is an + // operator's concern, not the caller's. + Unsaved []string `json:"-"` } // RuntimeError is a structured error containing context for execution failures.