diff --git a/internal/agent/loop.go b/internal/agent/loop.go index 7c3d17813..9859bc595 100644 --- a/internal/agent/loop.go +++ b/internal/agent/loop.go @@ -689,7 +689,7 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options) } messages = append(messages, zeroruntime.Message{ Role: zeroruntime.MessageRoleTool, - Content: toolResult.Output, + Content: toolResult.ModelOutput(), ToolCallID: toolResult.ToolCallID, IsError: toolResult.Status == tools.StatusError, ChangedFiles: append([]string(nil), toolResult.ChangedFiles...), @@ -721,7 +721,7 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options) if stopReason := stopReasonFromToolResult(toolResult); stopReason != "" { messages = appendAbortedToolResults(messages, collected.ToolCalls[index+1:]) messages = append(messages, toolImageMessages...) - result.FinalAnswer = toolResult.Output + result.FinalAnswer = toolResult.ModelOutput() result.StopReason = stopReason result.Messages = copyMessages(messages) return result, nil @@ -734,7 +734,7 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options) // aren't fixed by reformatting the call, so a "match this schema" hint // would misdirect the model toward JSON shape or blocked behavior. retriableFailure := isRetriableToolError(toolResult) - outcome := guards.observeToolResult(call.Name, retriableFailure, toolResult.Output) + outcome := guards.observeToolResult(call.Name, retriableFailure, toolResult.ModelOutput()) posture.observeToolOutcome(outcome, toolResult) if outcome.Stop { // The assistant message advertised EVERY collected tool call, but @@ -750,7 +750,7 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options) return result, nil } if outcome.InjectHint && failureHint == "" { - failureHint = toolFailureHint(call.Name, toolSchemaJSON(registry, call.Name), toolResult.Output) + failureHint = toolFailureHint(call.Name, toolSchemaJSON(registry, call.Name), toolResult.ModelOutput()) } // Collect the files this successful mutating tool changed. Self-correct @@ -922,6 +922,21 @@ func recordOutputBudgetTrace(recorder *trace.Recorder, result ToolResult) { if recorder == nil || result.Meta["output_budget_category"] == "" { return } + if result.Outcome.Finalized() { + diagnostics := result.Outcome.Diagnostics + recorder.EmitOutputBudget(trace.OutputBudgetEvent{ + Tool: result.Name, + Category: diagnostics.Category, + OriginalBytes: diagnostics.OriginalBytes, + RetainedBytes: diagnostics.ModelBytes, + EstimatedOriginalTokens: diagnostics.EstimatedOriginalTokens, + EstimatedRetainedTokens: diagnostics.EstimatedModelTokens, + Truncated: diagnostics.Truncated, + Reason: diagnostics.Reason, + SpillCreated: result.Outcome.Artifact != nil, + }) + return + } parseInt := func(key string) int { value, _ := strconv.Atoi(result.Meta[key]) return value @@ -1457,14 +1472,15 @@ func executeToolCall(ctx context.Context, registry *tools.Registry, call ToolCal ToolCallID: call.ID, Name: call.Name, Status: result.Status, - Output: result.Output, + Output: result.ModelOutput(), Truncated: result.Truncated, Meta: result.Meta, Images: result.Images, Redacted: result.Redacted, ChangedFiles: result.ChangedFiles, ChangeSummaries: result.ChangeSummaries, - Display: result.Display, + Display: result.HumanDisplay(), + Outcome: result.Outcome, LoadedTools: loadedToolsFromResult(result.Meta), // A tool may signal a mid-run model escalation by carrying the target id // in Meta["escalate_to_model"]. Lift it into the typed loop-level field; @@ -2061,13 +2077,14 @@ func askUserFallbackResult(ctx context.Context, registry *tools.Registry, call T ToolCallID: call.ID, Name: call.Name, Status: result.Status, - Output: result.Output, + Output: result.ModelOutput(), Truncated: result.Truncated, Meta: result.Meta, Redacted: result.Redacted, ChangedFiles: result.ChangedFiles, ChangeSummaries: result.ChangeSummaries, - Display: result.Display, + Display: result.HumanDisplay(), + Outcome: result.Outcome, } } return ToolResult{ diff --git a/internal/agent/types.go b/internal/agent/types.go index 7e6cab927..3d94be2bb 100644 --- a/internal/agent/types.go +++ b/internal/agent/types.go @@ -79,6 +79,7 @@ type ToolResult struct { // command execution; callers must not schedule per-file work from them. ChangeSummaries []execution.Change Display tools.Display + Outcome tools.ToolOutcome // DenialReason categorizes why a tool call was blocked (empty when it ran). // It lets a surface distinguish the cause precisely instead of parsing Output. DenialReason DenialCategory @@ -99,6 +100,24 @@ type ToolResult struct { RequestedModel string } +// ModelOutput returns the bounded provider-facing result while preserving +// compatibility with synthetic and restored results created before outcomes +// were finalized. +func (result ToolResult) ModelOutput() string { + if result.Outcome.Finalized() { + return result.Outcome.ModelView + } + return result.Output +} + +// HumanDisplay returns the presentation intended for interactive surfaces. +func (result ToolResult) HumanDisplay() tools.Display { + if result.Outcome.Finalized() { + return result.Outcome.HumanView + } + return result.Display +} + // DenialCategory classifies why a tool call was blocked before it executed. type DenialCategory string diff --git a/internal/cli/exec_writer.go b/internal/cli/exec_writer.go index 824b9c00f..2ee858480 100644 --- a/internal/cli/exec_writer.go +++ b/internal/cli/exec_writer.go @@ -142,13 +142,15 @@ func (writer *execEventWriter) checkpoint(event sessions.Event) { } func (writer *execEventWriter) toolResult(result agent.ToolResult) { + modelOutput := result.ModelOutput() + display := result.HumanDisplay() if writer.format == execOutputJSON { payload := map[string]any{ "type": "tool_result", "tool_call_id": result.ToolCallID, "name": result.Name, "status": string(result.Status), - "output": result.Output, + "output": modelOutput, } if len(result.Meta) > 0 { payload["meta"] = result.Meta @@ -162,14 +164,14 @@ func (writer *execEventWriter) toolResult(result agent.ToolResult) { if len(result.ChangedFiles) > 0 { payload["changed_files"] = result.ChangedFiles } - if result.Display.Summary != "" || result.Display.Kind != "" { - payload["display"] = map[string]string{"summary": result.Display.Summary, "kind": result.Display.Kind} + if display.Summary != "" || display.Kind != "" { + payload["display"] = map[string]string{"summary": display.Summary, "kind": display.Kind} } writer.writeJSON(payload) return } if writer.format == execOutputStreamJSON { - output, surfaceTruncated := truncateForStreamJSONOutput(result.Output) + output, surfaceTruncated := truncateForStreamJSONOutput(modelOutput) truncated := result.Truncated || surfaceTruncated event := streamjson.Event{ Type: streamjson.EventToolResult, @@ -186,13 +188,13 @@ func (writer *execEventWriter) toolResult(result agent.ToolResult) { redacted := true event.Redacted = &redacted } - if result.Display.Summary != "" || result.Display.Kind != "" { - event.Display = &streamjson.Display{Summary: result.Display.Summary, Kind: result.Display.Kind} + if display.Summary != "" || display.Kind != "" { + event.Display = &streamjson.Display{Summary: display.Summary, Kind: display.Kind} } writer.writeStreamJSON(event) return } - writer.writeStderr("[result] " + truncateForStatus(result.Output) + "\n") + writer.writeStderr("[result] " + truncateForStatus(modelOutput) + "\n") } func (writer *execEventWriter) permission(event agent.PermissionEvent) { diff --git a/internal/tools/output_boundary.go b/internal/tools/output_boundary.go index e346f7a95..34fcef32c 100644 --- a/internal/tools/output_boundary.go +++ b/internal/tools/output_boundary.go @@ -99,15 +99,18 @@ func selfManagedOutputBudget(toolName string, args map[string]any) outputBudget // newly combined result so hooks cannot bypass the established safety ceiling. func (registry *Registry) RebudgetAfterHook(toolName string, args map[string]any, result Result) Result { result = scrubResultSecrets(result) + boundaryOutput := result.Output tool, _ := registry.Get(toolName) if _, ok := tool.(selfBudgeting); ok { // Match the primary registry boundary: self-managed tools keep their // call-specific capture/output budget instead of being tightened or // loosened to the generic registry ceiling after hook feedback. - return applySelfManagedOutputBudget(tool, toolName, args, result) + result = applySelfManagedOutputBudget(tool, toolName, args, result) + return finalizeToolOutcome(result, boundaryOutput) } result = applyRegistryOutputBudget(tool, toolName, args, result) - return enforceOutputCeiling(toolName, result) + result = enforceOutputCeiling(toolName, result) + return finalizeToolOutcome(result, boundaryOutput) } func registryOutputBudget(toolName string) outputBudget { diff --git a/internal/tools/registry.go b/internal/tools/registry.go index a03209370..1b5bc8e67 100644 --- a/internal/tools/registry.go +++ b/internal/tools/registry.go @@ -143,6 +143,7 @@ func (registry *Registry) RunWithOptions(ctx context.Context, name string, args var ok bool defer func() { result = scrubResultSecrets(result) + boundaryOutput := result.Output result = reduceCommandOutput(name, args, result) if selfManagedOutput { result = applySelfManagedOutputBudget(tool, name, args, result) @@ -150,6 +151,7 @@ func (registry *Registry) RunWithOptions(ctx context.Context, name string, args result = applyRegistryOutputBudget(tool, name, args, result) result = enforceOutputCeiling(name, result) } + result = finalizeToolOutcome(result, boundaryOutput) if commitFileObservation { result = registry.CommitFileObservation(result, options.FileTracker) } diff --git a/internal/tools/tool_outcome.go b/internal/tools/tool_outcome.go new file mode 100644 index 000000000..439aab58b --- /dev/null +++ b/internal/tools/tool_outcome.go @@ -0,0 +1,115 @@ +package tools + +import ( + "encoding/json" + "strings" +) + +const humanOutcomePreviewBytes = 8 * 1024 + +type serializedToolOutcome struct { + ModelView string `json:"modelView,omitempty"` + HumanView Display `json:"humanView,omitempty"` + Artifact *ToolArtifact `json:"artifact,omitempty"` + Diagnostics OutcomeDiagnostics `json:"diagnostics,omitempty"` + Finalized bool `json:"finalized,omitempty"` +} + +// MarshalJSON preserves the private finalized marker without making outcome +// state mutable to callers. +func (outcome ToolOutcome) MarshalJSON() ([]byte, error) { + return json.Marshal(serializedToolOutcome{ + ModelView: outcome.ModelView, + HumanView: outcome.HumanView, + Artifact: outcome.Artifact, + Diagnostics: outcome.Diagnostics, + Finalized: outcome.finalized, + }) +} + +// UnmarshalJSON restores the finalized marker used to select canonical views. +func (outcome *ToolOutcome) UnmarshalJSON(data []byte) error { + var serialized serializedToolOutcome + if err := json.Unmarshal(data, &serialized); err != nil { + return err + } + outcome.ModelView = serialized.ModelView + outcome.HumanView = serialized.HumanView + outcome.Artifact = serialized.Artifact + outcome.Diagnostics = serialized.Diagnostics + outcome.finalized = serialized.Finalized + return nil +} + +// finalizeToolOutcome is the single seam between tool execution and its three +// consumers: provider context, human presentation, and recoverable artifacts. +// boundaryOutput must already be redacted. It is the text seen immediately +// before command reduction and semantic budgeting. +func finalizeToolOutcome(result Result, boundaryOutput string) Result { + previous := result.Outcome + human := result.Display + if human.Preview == "" && result.Meta["command_output_reduced"] == "true" { + human.Preview = boundedHumanOutcomePreview(boundaryOutput, result.Meta["spill_path"]) + } + // A tool-provided success preview (for example, a prospective file diff) + // must not hide the actual error. Command reduction is different: its + // preview is the redacted execution output containing that same failure plus + // the evidence omitted from the model view. + if result.Status == StatusError && result.Meta["command_output_reduced"] != "true" { + human.Preview = "" + } + + originalBytes := len(boundaryOutput) + originalTokens := estimateOutputTokens(boundaryOutput) + if previous.Finalized() { + originalBytes = previous.Diagnostics.OriginalBytes + originalTokens = previous.Diagnostics.EstimatedOriginalTokens + } + modelBytes := len(result.Output) + modelTokens := estimateOutputTokens(result.Output) + + var artifact *ToolArtifact + if previous.Finalized() { + artifact = previous.Artifact + } + if path := strings.TrimSpace(result.Meta["spill_path"]); path != "" { + if artifact == nil || artifact.Path != path { + artifact = &ToolArtifact{ + Path: path, + CompleteAtBoundary: result.Meta["command_output_reduced"] == "true" || result.Meta[outputBudgetSpillCreatedMeta] == "true", + } + } + } + + result.Display = human + result.Outcome = ToolOutcome{ + ModelView: result.Output, + HumanView: human, + Artifact: artifact, + Diagnostics: OutcomeDiagnostics{ + Category: result.Meta[outputBudgetCategoryMeta], + OriginalBytes: originalBytes, + ModelBytes: modelBytes, + EstimatedOriginalTokens: originalTokens, + EstimatedModelTokens: modelTokens, + Truncated: result.Truncated, + Redacted: result.Redacted, + Reason: result.Meta["truncation_reason"], + }, + finalized: true, + } + return result +} + +func boundedHumanOutcomePreview(output string, artifactPath string) string { + if len(output) <= humanOutcomePreviewBytes { + return output + } + marker := "\n[zero] human preview shortened" + if strings.TrimSpace(artifactPath) != "" { + marker += "; exact output: " + artifactPath + } + marker += "\n" + contentBudget := max(0, humanOutcomePreviewBytes-len(marker)) + return utf8Prefix(output, contentBudget*3/5) + marker + utf8Suffix(output, contentBudget*2/5) +} diff --git a/internal/tools/tool_outcome_test.go b/internal/tools/tool_outcome_test.go new file mode 100644 index 000000000..700e3bbf0 --- /dev/null +++ b/internal/tools/tool_outcome_test.go @@ -0,0 +1,187 @@ +package tools + +import ( + "context" + "os" + "strings" + "testing" +) + +type outcomeErrorTool struct { + ceilingFakeTool + display Display +} + +func (tool outcomeErrorTool) Run(context.Context, map[string]any) Result { + return Result{Status: StatusError, Output: tool.output, Display: tool.display} +} + +func TestRegistryFinalizesSeparateModelHumanAndArtifactViews(t *testing.T) { + setTestTempDir(t) + lines := []string{"output:"} + for index := 0; index < 24; index++ { + lines = append(lines, "ok \texample.test/package\t0.01s") + } + lines = append(lines, "exit_code: 0") + raw := strings.Join(lines, "\n") + + registry := NewRegistry() + registry.Register(newCeilingFakeTool(ExecCommandToolName, raw)) + result := registry.Run(context.Background(), ExecCommandToolName, map[string]any{"cmd": "go test ./..."}) + + if !result.Outcome.Finalized() { + t.Fatal("registry result did not finalize a tool outcome") + } + if result.ModelOutput() != result.Output || result.Outcome.ModelView != result.Output { + t.Fatalf("model representations drifted: output=%q outcome=%q", result.Output, result.Outcome.ModelView) + } + if strings.Count(result.ModelOutput(), "ok \t") != 0 { + t.Fatalf("model view retained repetitive passing-package lines: %q", result.ModelOutput()) + } + if !strings.Contains(result.HumanDisplay().Preview, "ok \texample.test/package") { + t.Fatalf("human preview lost the raw passing-package evidence: %q", result.HumanDisplay().Preview) + } + if result.Outcome.Artifact == nil || !result.Outcome.Artifact.CompleteAtBoundary { + t.Fatalf("missing complete boundary artifact: %#v", result.Outcome.Artifact) + } + artifact, err := os.ReadFile(result.Outcome.Artifact.Path) + if err != nil { + t.Fatalf("read outcome artifact: %v", err) + } + if string(artifact) != raw { + t.Fatal("outcome artifact differs from the exact boundary output") + } + diagnostics := result.Outcome.Diagnostics + if diagnostics.OriginalBytes != len(raw) || diagnostics.ModelBytes != len(result.Output) { + t.Fatalf("incorrect outcome byte diagnostics: %#v", diagnostics) + } + if diagnostics.EstimatedModelTokens >= diagnostics.EstimatedOriginalTokens { + t.Fatalf("expected reduced model view: %#v", diagnostics) + } +} + +func TestRegistryOutcomeRedactsModelHumanAndArtifactViews(t *testing.T) { + setTestTempDir(t) + secret := "ghp_" + strings.Repeat("s", 36) + raw := "output:\n" + secret + "\n" + strings.Repeat("ok \texample.test/package\t0.01s\n", 24) + "exit_code: 0" + + registry := NewRegistry() + registry.Register(newCeilingFakeTool(ExecCommandToolName, raw)) + result := registry.Run(context.Background(), ExecCommandToolName, map[string]any{"cmd": "go test ./..."}) + + if !result.Outcome.Diagnostics.Redacted { + t.Fatal("outcome did not record redaction") + } + for name, value := range map[string]string{ + "model": result.ModelOutput(), + "human": result.HumanDisplay().Preview, + } { + if strings.Contains(value, secret) { + t.Fatalf("%s view leaked secret", name) + } + } + artifact, err := os.ReadFile(result.Outcome.Artifact.Path) + if err != nil { + t.Fatalf("read outcome artifact: %v", err) + } + if strings.Contains(string(artifact), secret) { + t.Fatal("artifact leaked secret") + } +} + +func TestDirectToolResultUsesLegacyViewFallbacks(t *testing.T) { + result := Result{ + Output: "direct output", + Display: Display{Summary: "direct summary"}, + } + if result.Outcome.Finalized() { + t.Fatal("direct result unexpectedly finalized") + } + if result.ModelOutput() != result.Output || result.HumanDisplay() != result.Display { + t.Fatalf("direct result fallbacks changed: %#v", result) + } +} + +func TestRebudgetAfterHookPreservesExecutionArtifactAndRefreshesModelView(t *testing.T) { + setTestTempDir(t) + raw := "output:\n" + strings.Repeat("ok \texample.test/package\t0.01s\n", 24) + "exit_code: 0" + registry := NewRegistry() + registry.Register(newCeilingFakeTool(ExecCommandToolName, raw)) + result := registry.Run(context.Background(), ExecCommandToolName, map[string]any{"cmd": "go test ./..."}) + artifact := result.Outcome.Artifact + originalBytes := result.Outcome.Diagnostics.OriginalBytes + + result.Output += "\nafter-hook diagnostic" + result = registry.RebudgetAfterHook(ExecCommandToolName, map[string]any{"cmd": "go test ./..."}, result) + + if result.Outcome.Artifact != artifact { + t.Fatalf("rebudget replaced the execution artifact: before=%#v after=%#v", artifact, result.Outcome.Artifact) + } + if result.Outcome.Diagnostics.OriginalBytes != originalBytes { + t.Fatalf("rebudget lost original execution size: before=%d after=%d", originalBytes, result.Outcome.Diagnostics.OriginalBytes) + } + if !strings.Contains(result.ModelOutput(), "after-hook diagnostic") { + t.Fatalf("rebudget did not refresh model view: %q", result.ModelOutput()) + } +} + +func TestToolOutcomeCorpusMetrics(t *testing.T) { + setTestTempDir(t) + totalOriginalTokens := 0 + totalModelTokens := 0 + for _, testCase := range commandReducerCorpus() { + t.Run(testCase.name, func(t *testing.T) { + registry := NewRegistry() + registry.Register(newCeilingFakeTool(ExecCommandToolName, testCase.output)) + result := registry.Run(context.Background(), ExecCommandToolName, map[string]any{"cmd": testCase.command}) + + if !result.Outcome.Finalized() || result.Outcome.Artifact == nil { + t.Fatalf("corpus result lacks a finalized recoverable outcome: %#v", result.Outcome) + } + if result.HumanDisplay().Preview != testCase.output { + t.Fatal("bounded corpus output was not preserved exactly for the human view") + } + diagnostics := result.Outcome.Diagnostics + totalOriginalTokens += diagnostics.EstimatedOriginalTokens + totalModelTokens += diagnostics.EstimatedModelTokens + t.Logf("original_tokens=%d model_tokens=%d human_bytes=%d artifact_complete=%t", + diagnostics.EstimatedOriginalTokens, + diagnostics.EstimatedModelTokens, + len(result.HumanDisplay().Preview), + result.Outcome.Artifact.CompleteAtBoundary, + ) + }) + } + if totalModelTokens >= totalOriginalTokens { + t.Fatalf("outcome corpus did not reduce model context: original=%d model=%d", totalOriginalTokens, totalModelTokens) + } + t.Logf("aggregate original_tokens=%d model_tokens=%d reduction_pct=%d", + totalOriginalTokens, + totalModelTokens, + 100*(totalOriginalTokens-totalModelTokens)/totalOriginalTokens, + ) +} + +func TestErrorOutcomeUsesRawCommandEvidenceButDropsUnrelatedPreview(t *testing.T) { + setTestTempDir(t) + raw := "output:\n" + strings.Repeat("ok \texample.test/package\t0.01s\n", 24) + + "--- FAIL: TestImportant\nexpected 7, got 9\nFAIL\nexit_code: 1" + + registry := NewRegistry() + registry.Register(outcomeErrorTool{ceilingFakeTool: newCeilingFakeTool(ExecCommandToolName, raw)}) + result := registry.Run(context.Background(), ExecCommandToolName, map[string]any{"cmd": "go test ./..."}) + if !strings.Contains(result.HumanDisplay().Preview, "TestImportant") || + !strings.Contains(result.HumanDisplay().Preview, "ok \texample.test/package") { + t.Fatalf("error outcome lost raw command evidence: %q", result.HumanDisplay().Preview) + } + + registry = NewRegistry() + registry.Register(outcomeErrorTool{ + ceilingFakeTool: newCeilingFakeTool("write_failure", "Error: permission denied"), + display: Display{Preview: "prospective diff must not hide error"}, + }) + result = registry.Run(context.Background(), "write_failure", map[string]any{}) + if result.HumanDisplay().Preview != "" { + t.Fatalf("ordinary error retained an unrelated preview: %q", result.HumanDisplay().Preview) + } +} diff --git a/internal/tools/types.go b/internal/tools/types.go index 3929484cd..d10a4368d 100644 --- a/internal/tools/types.go +++ b/internal/tools/types.go @@ -126,11 +126,73 @@ type Result struct { ChangeSummaries []execution.Change // Display carries a short, structured summary for the TUI / stream. Display Display + // Outcome is the finalized, typed representation produced at the registry + // seam. ModelView, HumanView, and Artifact deliberately serve different + // consumers; Output, Display, and spill metadata remain synchronized for + // compatibility with direct tool callers and persisted sessions. + Outcome ToolOutcome // pendingFileObservation is proposed by read_file and committed only after // the final model-visible output boundary confirms the exact content survived. pendingFileObservation *pendingFileObservation } +// ToolOutcome is the canonical post-execution representation of one tool +// result. It separates the bounded provider payload from the human-facing +// presentation and the recoverable output retained outside model context. +type ToolOutcome struct { + ModelView string + HumanView Display + Artifact *ToolArtifact + Diagnostics OutcomeDiagnostics + finalized bool +} + +// Finalized reports whether the outcome crossed the registry boundary. Direct +// Tool.Run results deliberately return false and use their legacy fields. +func (outcome ToolOutcome) Finalized() bool { + return outcome.finalized +} + +// ToolArtifact identifies recoverable output saved by the tool boundary. +// CompleteAtBoundary means the artifact contains every redacted byte received +// by that boundary; an underlying process may already have applied its own +// capture limit before producing the result. +type ToolArtifact struct { + Path string + CompleteAtBoundary bool +} + +// OutcomeDiagnostics describes how the model-facing representation differs +// from the redacted output received by the registry boundary. +type OutcomeDiagnostics struct { + Category string + OriginalBytes int + ModelBytes int + EstimatedOriginalTokens int + EstimatedModelTokens int + Truncated bool + Redacted bool + Reason string +} + +// ModelOutput returns the finalized provider-facing text, falling back to the +// legacy field for direct Tool.Run callers that have not crossed the registry. +func (result Result) ModelOutput() string { + if result.Outcome.finalized { + return result.Outcome.ModelView + } + return result.Output +} + +// HumanDisplay returns the finalized presentation, falling back to the legacy +// display for direct Tool.Run callers. +func (result Result) HumanDisplay() Display { + if result.Outcome.finalized { + return result.Outcome.HumanView + } + return result.Display +} + // Display carries a short, structured summary of a tool result for the TUI/stream. type Display struct { Summary string diff --git a/internal/tui/model.go b/internal/tui/model.go index 916b26221..c638863af 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -5666,10 +5666,11 @@ func (m model) sendAgentUsage(runID int, modelID string, event zeroruntime.Usage // (a code/diff preview) when present on a successful result, else the Output that // the model also saw. Error results keep their Output so the failure shows. func toolResultDetail(result agent.ToolResult) string { - if result.Status != tools.StatusError && strings.TrimSpace(result.Display.Preview) != "" { - return result.Display.Preview + display := result.HumanDisplay() + if strings.TrimSpace(display.Preview) != "" && (result.Status != tools.StatusError || result.Outcome.Finalized()) { + return display.Preview } - return result.Output + return result.ModelOutput() } func toolResultRowText(result agent.ToolResult) string { @@ -5677,5 +5678,5 @@ func toolResultRowText(result agent.ToolResult) string { if status == "" { status = tools.StatusOK } - return fmt.Sprintf("tool result: %s %s %s", result.Name, status, truncateTUIOutput(result.Output, tuiToolOutputLimit)) + return fmt.Sprintf("tool result: %s %s %s", result.Name, status, truncateTUIOutput(result.ModelOutput(), tuiToolOutputLimit)) } diff --git a/internal/tui/tool_outcome_test.go b/internal/tui/tool_outcome_test.go new file mode 100644 index 000000000..dad3a9926 --- /dev/null +++ b/internal/tui/tool_outcome_test.go @@ -0,0 +1,92 @@ +package tui + +import ( + "context" + "encoding/json" + "strings" + "testing" + + "github.com/Gitlawb/zero/internal/agent" + "github.com/Gitlawb/zero/internal/tools" +) + +type tuiOutcomeErrorTool struct { + output string +} + +func (tool tuiOutcomeErrorTool) Name() string { return tools.ExecCommandToolName } +func (tool tuiOutcomeErrorTool) Description() string { return "test tool" } +func (tool tuiOutcomeErrorTool) Parameters() tools.Schema { return tools.Schema{Type: "object"} } +func (tool tuiOutcomeErrorTool) Safety() tools.Safety { + return tools.Safety{Permission: tools.PermissionAllow} +} +func (tool tuiOutcomeErrorTool) Run(context.Context, map[string]any) tools.Result { + return tools.Result{Status: tools.StatusError, Output: tool.output} +} + +func setToolOutcomeTempDir(t *testing.T) { + t.Helper() + tempDir := t.TempDir() + t.Setenv("TMPDIR", tempDir) + t.Setenv("TMP", tempDir) + t.Setenv("TEMP", tempDir) +} + +func TestToolResultDetailUsesFinalizedHumanEvidenceForReducedError(t *testing.T) { + setToolOutcomeTempDir(t) + raw := "output:\n" + strings.Repeat("ok \texample.test/package\t0.01s\n", 24) + + "--- FAIL: TestImportant\nexpected 7, got 9\nFAIL\nexit_code: 1" + registry := tools.NewRegistry() + registry.Register(tuiOutcomeErrorTool{output: raw}) + result := registry.Run(context.Background(), tools.ExecCommandToolName, map[string]any{"cmd": "go test ./..."}) + + detail := toolResultDetail(agent.ToolResult{ + Name: tools.ExecCommandToolName, + Status: result.Status, + Output: result.Output, + Display: result.Display, + Outcome: result.Outcome, + }) + for _, want := range []string{"ok \texample.test/package", "TestImportant", "expected 7, got 9"} { + if !strings.Contains(detail, want) { + t.Fatalf("human error detail missing %q: %q", want, detail) + } + } + if detail == result.ModelOutput() { + t.Fatal("human detail collapsed back to the reduced model view") + } +} + +func TestToolResultDetailSurvivesOutcomeJSONRoundTrip(t *testing.T) { + setToolOutcomeTempDir(t) + raw := "output:\n" + strings.Repeat("ok \texample.test/package\t0.01s\n", 24) + + "--- FAIL: TestImportant\nexpected 7, got 9\nFAIL\nexit_code: 1" + registry := tools.NewRegistry() + registry.Register(tuiOutcomeErrorTool{output: raw}) + toolResult := registry.Run(context.Background(), tools.ExecCommandToolName, map[string]any{"cmd": "go test ./..."}) + original := agent.ToolResult{ + Name: tools.ExecCommandToolName, + Status: toolResult.Status, + Output: toolResult.Output, + Display: toolResult.Display, + Outcome: toolResult.Outcome, + } + + data, err := json.Marshal(original) + if err != nil { + t.Fatalf("marshal tool result: %v", err) + } + var restored agent.ToolResult + if err := json.Unmarshal(data, &restored); err != nil { + t.Fatalf("unmarshal tool result: %v", err) + } + if !restored.Outcome.Finalized() { + t.Fatal("restored outcome lost its finalized state") + } + detail := toolResultDetail(restored) + for _, want := range []string{"ok \texample.test/package", "TestImportant", "expected 7, got 9"} { + if !strings.Contains(detail, want) { + t.Fatalf("restored human detail missing %q: %q", want, detail) + } + } +}