diff --git a/AGENTS.md b/AGENTS.md index c95fc60..774cf21 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -340,9 +340,14 @@ modifier routes to the composer), and `Alt+A`/`Alt+D`/`Alt+T` compact composer footers must fit one terminal row and retain actionable keys. - Home hints wrap by display cells, including wide Unicode paths. Keep the home free of a second wordmark or an additional feature dashboard. -- Structured batch summaries must not invent successful execution when normalized - results omit exit/status metadata. Expanded plan and batch views remain behind - existing details controls and preserve the chronological transcript. +- Tool summaries must not invent successful execution when results omit + exit/status metadata. Expanded plan views remain behind existing details + controls and preserve the chronological transcript. +- odek v2.26.0 retired `parallel_shell`, `batch_patch`, `batch_read`, `multi_grep`, + and `http_batch`. Keep historical entries on the generic raw/JSON path; do not + restore typed cards, argument previews, icons, summaries, or result schemas. + Preserve `delegate_tasks`, all `bg_*` views, and supported individual `shell`, + `patch`, `read_file`, `search_files`, and `http_request` rendering. - `TestTerminalWorkflowLayouts` checks real views across four themes at 40, 80, and 120 columns. Set `BODEK_RENDER_PREVIEW_DIR` to write optional ANSI fixtures for visual review without an engine/provider; generated captures are not source. @@ -376,11 +381,10 @@ modifier routes to the composer), and `Alt+A`/`Alt+D`/`Alt+T` connection ends; keep allow-once and deny visible at narrow widths. Default turn footers right-align outcome, elapsed time, tool count, and known cost; full telemetry remains under `^E` and `/stats`. -- Keep normalized `step.result` for copying/error compatibility and bounded - `step.detailResult` for structured display. Preserve sanitized command/path - identity through live and history ingestion; never infer item boundaries from - `[N]` text. Report omitted bodies/items and total subset counts explicitly. - Generic previews cap at 128 KiB/200 lines; structured details at 64 KiB/256 items. +- Keep normalized `step.result` for copying/error compatibility. Preserve + sanitized command/path identity through live and history ingestion; never + infer item boundaries from `[N]` text. Generic previews cap at 128 KiB/200 + lines and report omitted output explicitly. - Completed plans include the producer's `all N steps complete` format. - All footer modes fit one display row, prioritizing primary and exit actions. Recompute viewport height when the new-output shelf changes. Approval details diff --git a/README.md b/README.md index a48f06c..fa64ff5 100644 --- a/README.md +++ b/README.md @@ -167,10 +167,9 @@ own front-end settings are separate; see [Configuration](#configuration). structured output only: test verdicts (`✓ 5 passed · 2 skipped`, go coverage), git commits/pushes (`⎇ a1b2c3d`, `↑ main`), lint results, compiler warning counts, HTTP statuses, and search hit counts. Prose like - "Build passed" never goes green. Plans show compact progress and step rows; - parallel shell, batched reads/patches, and HTTP batches show item counts and - failures, with per-item detail behind `^E`. Missing success metadata stays - neutral rather than claiming that a command passed. + "Build passed" never goes green. Plans show compact progress and step rows. + Missing success metadata stays neutral rather than claiming that a command + passed. - **Streaming answers** rendered as Markdown ([glamour](https://github.com/charmbracelet/glamour)). - **Tool activity** — every `tool_call`/`tool_result` shown live with a glyph @@ -457,12 +456,14 @@ Use `PgUp`/`PgDn` to page, `alt+i` to copy the displayed invocation, The global `^E` details toggle uses the same page limits. Control and invisible characters in invocations appear as safe escape text. -Batch results retain command/file labels and original item counts; bracketed -log lines are never treated as extra commands. Plans render creation, updates, -blocked steps, and completion. Text previews retain up to 128 KiB and 200 lines; -structured detail metadata is capped at 64 KiB and 256 items. Omitted content -and subset counts are labelled, so a preview is never presented as the full -result when it was shortened. +Plans render creation, updates, blocked steps, and completion. Text previews +retain up to 128 KiB and 200 lines, with omitted content labelled. + +odek v2.26.0 retired `parallel_shell`, `batch_patch`, `batch_read`, `multi_grep`, +and `http_batch`. Historical entries for these tools use generic raw/JSON +inspection without typed cards, icons, or summaries. Supported `shell`, `patch`, +`read_file`, `search_files`, and `http_request` rendering remains available, +as do delegation and background-job views. ### The prompt queue diff --git a/internal/tui/coverage_test.go b/internal/tui/coverage_test.go index 1cdcfd2..fbe6b27 100644 --- a/internal/tui/coverage_test.go +++ b/internal/tui/coverage_test.go @@ -41,7 +41,7 @@ func TestListenDrainsPendingBatch(t *testing.T) { } func TestGlyphsAllBranches(t *testing.T) { - for _, n := range []string{"shell", "bash", "write_file", "patch", "read_file", "list_dir", "search_files", "web_search", "browser", "http_batch", "delegate_tasks", "memory", "vision", "transcribe", "unknown_x"} { + for _, n := range []string{"shell", "bash", "write_file", "patch", "read_file", "list_dir", "search_files", "web_search", "browser", "http_request", "delegate_tasks", "memory", "vision", "transcribe", "unknown_x"} { if toolGlyph(n) == "" { t.Errorf("empty glyph for %q", n) } diff --git a/internal/tui/events.go b/internal/tui/events.go index f8ccb50..4b2af21 100644 --- a/internal/tui/events.go +++ b/internal/tui/events.go @@ -207,9 +207,8 @@ func (m *Model) handleEvent(ev client.Event) (tea.Model, tea.Cmd) { for j := range steps { if steps[j].name == nm && !steps[j].done { steps[j].done = true - steps[j].result = resultPreview(ev.Data) - steps[j].detailResult = boundedStructuredDetail(nm, ev.Data) - steps[j].isErr = looksLikeError(steps[j].result) || hasFailedExit(ev.Data) || structuredResultFailed(nm, ev.Data) + steps[j].result = toolResultPreview(nm, ev.Data) + steps[j].isErr = looksLikeError(steps[j].result) || hasFailedExit(ev.Data) if steps[j].isErr && !steps[j].expanded { // A failing step is why anyone expands anything — // unfold it once so the diagnosis is on screen @@ -745,10 +744,9 @@ func eventTail(ev client.Event) string { // wrapper shows up as byteLimit { cut := byteLimit diff --git a/internal/tui/icons.go b/internal/tui/icons.go index 38fae1b..3ac9842 100644 --- a/internal/tui/icons.go +++ b/internal/tui/icons.go @@ -16,6 +16,9 @@ const ( // feed reads at a glance. Matching is by substring to cover odek's native // tools, MCP tools (server__tool), and sub-agent variants. func toolGlyph(name string) string { + if retiredTool(name) { + return "✦" + } n := strings.ToLower(name) switch { case strings.Contains(n, "shell"), strings.Contains(n, "bash"), strings.Contains(n, "exec"): @@ -54,3 +57,14 @@ func resourceGlyph(typ string) string { return "≡" } } + +// retiredTool keeps historical tool names on the generic display path instead +// of matching the substring-based renderers for supported tools and MCP names. +func retiredTool(name string) bool { + switch strings.ToLower(strings.TrimSpace(name)) { + case "parallel_shell", "batch_patch", "batch_read", "multi_grep", "http_batch": + return true + default: + return false + } +} diff --git a/internal/tui/invocation_test.go b/internal/tui/invocation_test.go index 0448223..d477340 100644 --- a/internal/tui/invocation_test.go +++ b/internal/tui/invocation_test.go @@ -127,12 +127,12 @@ func TestInvocationDisplaysUnsafeCharactersAndLimit(t *testing.T) { t.Fatalf("invalid UTF-8 byte was hidden: %q", got) } - raw := `{"commands":["first","second"],"note":"` + strings.Repeat("x", toolArgsLimit) + `"}` + raw := `{"tasks":["first","second"],"note":"` + strings.Repeat("x", toolArgsLimit) + `"}` retained, omitted := retainToolArgs(raw) if !omitted || len(retained) > toolArgsLimit { t.Fatal("tool arguments were not bounded") } - limited := invocationText(step{name: "parallel_shell", callArgs: retained, argsOmitted: omitted}) + limited := invocationText(step{name: "delegate_tasks", callArgs: retained, argsOmitted: omitted}) if !strings.Contains(limited, "limited to 256 KiB") || !strings.Contains(limited, "remaining invocation arguments omitted") { t.Fatal("bounded invocation has no visible limit marker") } @@ -154,7 +154,7 @@ func TestExpandedApprovalCommandWrapsWideCharacters(t *testing.T) { } func TestNestedInvocationAndCopyTarget(t *testing.T) { - s := step{name: "parallel_shell", callArgs: `{"commands":[{"command":"go test ./..."},{"command":"go vet ./..."}]}`} + s := step{name: "delegate_tasks", callArgs: `{"tasks":[{"prompt":"go test ./..."},{"prompt":"go vet ./..."}]}`} got := invocationText(s) for _, want := range []string{"go test ./...", "go vet ./..."} { if !strings.Contains(got, want) { diff --git a/internal/tui/model.go b/internal/tui/model.go index ad5e970..3e4439b 100644 --- a/internal/tui/model.go +++ b/internal/tui/model.go @@ -35,7 +35,6 @@ type step struct { callArgs string // retained tool-call arguments for deliberate inspection argsOmitted bool // callArgs exceeded the bounded inspection limit result string // sanitized tool output (multi-line); excerpted at render - detailResult string // bounded structured display data; normalized result remains copyable detailOffset int // first visible line in the expanded response done bool isErr bool // the result reads as a failure (tints the status glyph red) diff --git a/internal/tui/narrative.go b/internal/tui/narrative.go index 53ba2c7..b8f17c5 100644 --- a/internal/tui/narrative.go +++ b/internal/tui/narrative.go @@ -195,6 +195,9 @@ func scanReceipt(msg message) receipt { files := map[string]struct{}{} var r receipt for _, s := range msg.steps { + if retiredTool(s.name) { + continue + } if p := touchedPath(s.name, s.arg); p != "" { files[p] = struct{}{} } @@ -235,6 +238,9 @@ func formatReceipt(r receipt) string { // touchedPath is the file a write/patch/edit step named. Reads do not // count — the receipt is what the turn changed, not what it looked at. func touchedPath(name, arg string) string { + if retiredTool(name) { + return "" + } n := strings.ToLower(name) if !strings.Contains(n, "write") && !strings.Contains(n, "patch") && !strings.Contains(n, "edit") { return "" @@ -247,6 +253,9 @@ func touchedPath(name, arg string) string { } func isShellTool(name string) bool { + if retiredTool(name) { + return false + } n := strings.ToLower(name) return strings.Contains(n, "shell") || strings.Contains(n, "bash") || strings.Contains(n, "exec") } diff --git a/internal/tui/normalize_test.go b/internal/tui/normalize_test.go index b317d3c..1210ee8 100644 --- a/internal/tui/normalize_test.go +++ b/internal/tui/normalize_test.go @@ -60,13 +60,13 @@ func TestResultPreviewMalformedEnvelope(t *testing.T) { } } -// Parallel tools (parallel_shell) return a top-level results array of per- +// Delegation tools return a top-level results array of per- // call objects. resultPreview extracts each item's display body — stdout, // stderr, non-zero exit codes — and drops the JSON noise (command echoes, // index, duration). Wrappers inside stdout fold away entirely. -func TestResultPreviewParallelResults(t *testing.T) { - raw := `{"results":[{"index":0,"command":"gofmt -l .","description":"check formatting","stdout":"\u003cuntrusted_content_abc123 source=\"parallel_shell:0:stdout\"\u003e\nREADME.md\n\u003c/untrusted_content_abc123\u003e","stderr":"","exit_code":0,"duration_ms":12},{"index":1,"command":"go vet ./...","description":"vet","stdout":"","stderr":"vets hate this","exit_code":1,"duration_ms":300}]}` - got := resultPreview(raw) +func TestDelegateResultPreviewResults(t *testing.T) { + raw := `{"results":[{"index":0,"command":"gofmt -l .","description":"check formatting","stdout":"\u003cuntrusted_content_abc123 source=\"delegate_tasks:0:stdout\"\u003e\nREADME.md\n\u003c/untrusted_content_abc123\u003e","stderr":"","exit_code":0,"duration_ms":12},{"index":1,"command":"go vet ./...","description":"vet","stdout":"","stderr":"vets hate this","exit_code":1,"duration_ms":300}]}` + got := toolResultPreview("delegate_tasks", raw) for _, want := range []string{"[1] README.md", "[2] exit status 1", "stderr: vets hate this"} { if !strings.Contains(got, want) { t.Errorf("parallel results missing %q in:\n%s", want, got) @@ -82,7 +82,7 @@ func TestResultPreviewParallelResults(t *testing.T) { // A single-item results array renders the body bare — no index label. func TestResultPreviewSingleResult(t *testing.T) { raw := `{"results":[{"index":0,"command":"ls","stdout":"main.go\nutil.go","stderr":"","exit_code":0,"duration_ms":5}]}` - got := resultPreview(raw) + got := toolResultPreview("delegate_tasks", raw) if !strings.Contains(got, "main.go") || !strings.Contains(got, "util.go") { t.Errorf("stdout body lost:\n%s", got) } @@ -94,7 +94,7 @@ func TestResultPreviewSingleResult(t *testing.T) { // Non-stdout result shapes (delegate_tasks headlines) still extract. func TestResultPreviewResultsHeadline(t *testing.T) { raw := `{"results":[{"headline":"built 3 sub-agents","artifacts":[{"id":"a1","path":"x.go","bytes":10}],"cost_usd":0.5}]}` - got := resultPreview(raw) + got := toolResultPreview("delegate_tasks", raw) if !strings.Contains(got, "built 3 sub-agents") { t.Errorf("headline body lost:\n%s", got) } @@ -131,7 +131,7 @@ func TestResultPreviewResultsFailSafe(t *testing.T) { `{"results":[{"weird":{"a":1}}]}`, `{"results":[{"other":"only unknown scalars"}]}`, } { - if got := resultPreview(s); got != s { + if got := toolResultPreview("delegate_tasks", s); got != s { t.Errorf("resultPreview(%q) = %q, want unchanged", s, got) } } diff --git a/internal/tui/panels.go b/internal/tui/panels.go index 4a47554..1f791ed 100644 --- a/internal/tui/panels.go +++ b/internal/tui/panels.go @@ -1175,7 +1175,6 @@ func (m *Model) replayTranscript(msgs []client.SessionMessage) { // live tool_result events carry the raw output — strip it so both // render identically. resultPreview sanitizes the unwrapped output. rawResult := stripToolResultFrame(mm.Content) - result := resultPreview(rawResult) // Match by tool_call_id first; fall back to the live tool_result // behavior of scanning backwards by name for an unfinished step. idx, ok := stepByCallID[mm.ToolCallID] @@ -1193,10 +1192,10 @@ func (m *Model) replayTranscript(msgs []client.SessionMessage) { if !ok { continue } + result := toolResultPreview(cur.steps[idx].name, rawResult) cur.steps[idx].done = true cur.steps[idx].result = result - cur.steps[idx].detailResult = boundedStructuredDetail(cur.steps[idx].name, rawResult) - cur.steps[idx].isErr = looksLikeError(result) || hasFailedExit(rawResult) || structuredResultFailed(cur.steps[idx].name, rawResult) + cur.steps[idx].isErr = looksLikeError(result) || hasFailedExit(rawResult) } } flush() diff --git a/internal/tui/progress.go b/internal/tui/progress.go index 145f250..d58c3ff 100644 --- a/internal/tui/progress.go +++ b/internal/tui/progress.go @@ -8,6 +8,9 @@ import ( // toolProgress returns a playful, context-aware status line for a running tool, // derived from the tool name and its argument preview. func toolProgress(name, arg string) string { + if retiredTool(name) { + return "🔧 running " + name + } n := strings.ToLower(name) switch { case strings.Contains(n, "shell"), strings.Contains(n, "bash"), strings.Contains(n, "exec"): diff --git a/internal/tui/realtime_tabs_test.go b/internal/tui/realtime_tabs_test.go index 9aad576..9991c9f 100644 --- a/internal/tui/realtime_tabs_test.go +++ b/internal/tui/realtime_tabs_test.go @@ -32,9 +32,9 @@ func TestSubagentStateKicksAgentsFetch(t *testing.T) { // telemetry (tool/step, elapsed) — not just the REST snapshot's coarse data. func TestAgentsRowsPreferLiveCard(t *testing.T) { m := stateFixture(t) - // Live card: the agent is on step 3 running "multi_grep", 40s elapsed. + // Live card: the agent is on step 3 running "search_files", 40s elapsed. m.handleEvent(client.Event{Type: "subagent_state", TaskID: "t1", TaskIdx: 0, - Phase: "active", Status: "running", Step: 3, Tool: "multi_grep"}) + Phase: "active", Status: "running", Step: 3, Tool: "search_files"}) // REST row is stale: server snapshot still says step 1 / shell, 2s. m.panel = panelAgents // handleMgmtMsg drops cross-tab results m.handleMgmtMsg(mgmtMsg{tab: panelAgents, sag: []client.SubagentEntry{ @@ -45,7 +45,7 @@ func TestAgentsRowsPreferLiveCard(t *testing.T) { t.Fatal("no rows rendered") } joined := strings.Join(rows, " ") - if !strings.Contains(joined, "multi_grep") { + if !strings.Contains(joined, "search_files") { t.Errorf("row ignores the live card's tool: %q", joined) } if strings.Contains(joined, "shell") { diff --git a/internal/tui/renderers.go b/internal/tui/renderers.go index 1084376..71017d5 100644 --- a/internal/tui/renderers.go +++ b/internal/tui/renderers.go @@ -8,8 +8,6 @@ import ( "regexp" "strconv" "strings" - - "github.com/charmbracelet/lipgloss" ) // ── typed tool renderers ─────────────────────────────────────────────────── @@ -196,7 +194,7 @@ func renderDiff(s string, width int, th theme) []string { // ── numbered file view ────────────────────────────────────────────────────── // renderNumbered renders plain file content with line numbers — the shape -// read_file/batch_read return — so excerpts read as code, not wrapped prose. +// read_file returns — so excerpts read as code, not wrapped prose. func renderNumbered(s string, width int, th theme) []string { w := detailWidth(width) lines := strings.Split(s, "\n") @@ -224,7 +222,7 @@ func renderNumbered(s string, width int, th theme) []string { // fileReadTool reports whether a tool's result is raw file content. func fileReadTool(name string) bool { n := strings.ToLower(name) - return strings.Contains(n, "read_file") || strings.Contains(n, "batch_read") + return strings.Contains(n, "read_file") } // ── JSON ──────────────────────────────────────────────────────────────────── @@ -309,371 +307,6 @@ var ( planStepRe = regexp.MustCompile(`^(\S+)\s+\[([^\]]+)\]\s*(.*)$`) ) -// structuredToolItem is the small, display-oriented subset shared by odek's -// batch result envelopes. Keep this deliberately narrower than the wire -// schema: unknown fields and nested values stay on the generic safe path. -type structuredToolItem struct { - label string - command string - stdout string - stderr string - content string - diff string - error string - status int - contentLen int64 - totalLines int - durationMS int64 - exitCode int - hasStatus bool - hasExitCode bool - hasSuccess bool - success bool -} - -// structuredTool reports the built-in tools whose results are arrays of -// independent work items. The name gate is intentional: arbitrary JSON from -// an MCP tool must continue through the normal renderer without guesswork. -func structuredTool(name string) bool { - n := strings.ToLower(strings.TrimSpace(name)) - return n == "parallel_shell" || n == "http_batch" || - n == "batch_read" || n == "batch_patch" -} - -func rawString(m map[string]json.RawMessage, key string) string { - v, ok := m[key] - if !ok || string(v) == "null" { - return "" - } - var s string - if json.Unmarshal(v, &s) != nil { - return "" - } - return sanitize(foldUntrustedWrappers(s)) -} - -func rawInt64(m map[string]json.RawMessage, key string) (int64, bool) { - v, ok := m[key] - if !ok || string(v) == "null" { - return 0, false - } - var n int64 - if json.Unmarshal(v, &n) != nil { - return 0, false - } - return n, true -} - -func rawBool(m map[string]json.RawMessage, key string) (bool, bool) { - v, ok := m[key] - if !ok || string(v) == "null" { - return false, false - } - var b bool - if json.Unmarshal(v, &b) != nil { - return false, false - } - return b, true -} - -// structuredJSONItems decodes only an object with a non-empty results array -// of known scalar fields. A malformed or foreign envelope returns ok=false, -// preserving the fail-safe generic JSON renderer. -func structuredJSONItems(name, data string) ([]structuredToolItem, bool) { - if !structuredTool(name) { - return nil, false - } - var env map[string]json.RawMessage - if err := json.Unmarshal([]byte(strings.TrimSpace(sanitize(data))), &env); err != nil { - return nil, false - } - raw, ok := env["results"] - if !ok { - return nil, false - } - var rows []map[string]json.RawMessage - if err := json.Unmarshal(raw, &rows); err != nil || len(rows) == 0 { - return nil, false - } - items := make([]structuredToolItem, 0, len(rows)) - for _, row := range rows { - item := structuredToolItem{ - label: rawString(row, "path"), - command: rawString(row, "command"), - stdout: rawString(row, "stdout"), - stderr: rawString(row, "stderr"), - content: rawString(row, "content"), - diff: rawString(row, "diff"), - error: rawString(row, "error"), - } - if item.label == "" { - item.label = rawString(row, "url") - } - if n, ok := rawInt64(row, "status"); ok { - item.status, item.hasStatus = int(n), true - } - if n, ok := rawInt64(row, "exit_code"); ok { - item.exitCode, item.hasExitCode = int(n), true - } - if n, ok := rawInt64(row, "content_length"); ok { - item.contentLen = n - } - if n, ok := rawInt64(row, "total_lines"); ok { - item.totalLines = int(n) - } - if n, ok := rawInt64(row, "duration_ms"); ok { - item.durationMS = n - } - if b, ok := rawBool(row, "success"); ok { - item.success, item.hasSuccess = b, true - } - // Require at least one field this renderer understands. In particular, - // do not turn {"results":[{"metadata":{...}}]} into an empty card. - known := item.label != "" || item.command != "" || item.stdout != "" || - item.stderr != "" || item.content != "" || item.diff != "" || item.error != "" || - item.hasStatus || item.hasExitCode || item.hasSuccess || item.totalLines > 0 || item.contentLen > 0 - if !known { - return nil, false - } - items = append(items, item) - } - return items, true -} - -const ( - structuredDetailCap = 64 * 1024 - structuredDetailRows = 256 - structuredDetailString = 2048 -) - -type structuredDisplayMeta struct { - totalItems int - displayTruncated bool - bodiesOmitted bool -} - -func structuredDisplayMetadata(data string, fallbackItems int) structuredDisplayMeta { - meta := structuredDisplayMeta{totalItems: fallbackItems} - var env struct { - TotalItems int `json:"total_items"` - DisplayTruncated bool `json:"display_truncated"` - BodiesOmitted bool `json:"bodies_omitted"` - } - if json.Unmarshal([]byte(strings.TrimSpace(sanitize(data))), &env) == nil { - if env.TotalItems > meta.totalItems { - meta.totalItems = env.TotalItems - } - meta.displayTruncated = env.DisplayTruncated && meta.totalItems > fallbackItems - meta.bodiesOmitted = env.BodiesOmitted - } - return meta -} - -// boundedStructuredDetail keeps the small, display-oriented metadata needed -// by typed batch renderers. It is deliberately re-encoded after parsing: -// truncating the raw JSON could leave an invalid document, while encoding -// sanitized bounded fields always leaves a parser-safe payload. Unknown or -// malformed shapes fall back to the ordinary normalized display text. -func boundedStructuredDetail(name, raw string) string { - if !structuredTool(name) { - return "" - } - items, ok := structuredJSONItems(name, raw) - if !ok { - return boundedStructuredFallback(raw) - } - totalItems := len(items) - if len(items) > structuredDetailRows { - items = items[:structuredDetailRows] - } - displayTruncated := len(items) < totalItems - - // Re-encode through maps so the renderer sees the same field names it - // understands on the wire (path/url, command, stdout, and so on). - build := func(limit int, bodies bool) string { - rows := make([]map[string]any, 0, len(items)) - for _, item := range items { - row := make(map[string]any, 12) - if item.label != "" { - if strings.EqualFold(name, "http_batch") { - row["url"] = boundedStructuredString(item.label, min(limit, 512)) - } else { - row["path"] = boundedStructuredString(item.label, min(limit, 512)) - } - } - if item.command != "" { - row["command"] = boundedStructuredString(item.command, limit) - } - if bodies { - if item.stdout != "" { - row["stdout"] = boundedStructuredString(item.stdout, limit) - } - if item.stderr != "" { - row["stderr"] = boundedStructuredString(item.stderr, limit) - } - if item.content != "" { - row["content"] = boundedStructuredString(item.content, limit) - } - if item.diff != "" { - row["diff"] = boundedStructuredString(item.diff, limit) - } - if item.error != "" { - row["error"] = boundedStructuredString(item.error, limit) - } - } - if item.hasStatus { - row["status"] = item.status - } - if item.hasExitCode { - row["exit_code"] = item.exitCode - } - if item.hasSuccess { - row["success"] = item.success - } - if item.contentLen != 0 { - row["content_length"] = item.contentLen - } - if item.totalLines != 0 { - row["total_lines"] = item.totalLines - } - if item.durationMS != 0 { - row["duration_ms"] = item.durationMS - } - rows = append(rows, row) - } - envelope := map[string]any{"results": rows} - if displayTruncated { - envelope["display_truncated"] = true - envelope["total_items"] = totalItems - } - if !bodies { - envelope["bodies_omitted"] = true - } - encoded, err := json.Marshal(envelope) - if err != nil || len(encoded) > structuredDetailCap { - return "" - } - return string(encoded) - } - - for limit := structuredDetailString; limit >= 128; limit /= 2 { - if encoded := build(limit, true); encoded != "" { - return encoded - } - } - // Labels, status, and exit metadata are more useful than an unbounded body - // when a result contains hundreds of large outputs. - if encoded := build(256, false); encoded != "" { - return encoded - } - return boundedStructuredFallback(raw) -} - -func boundedStructuredFallback(raw string) string { - s := resultPreview(raw) - if len(s) <= structuredDetailCap { - return s - } - // This fallback is plain display text, so a rune-safe cap is preferable to - // slicing a JSON document and leaving the typed parser with broken syntax. - cut := structuredDetailCap - len("…") - for cut > 0 && cut < len(s) && (s[cut]&0xc0) == 0x80 { - cut-- - } - return s[:cut] + "…" -} - -func structuredResultFailed(name, raw string) bool { - items, ok := structuredJSONItems(name, raw) - if !ok { - return false - } - for _, item := range items { - if structuredItemFailed(item) { - return true - } - } - return false -} - -func boundedStructuredString(s string, max int) string { - s = sanitize(foldUntrustedWrappers(s)) - return truncate(s, max) -} - -// stepDetailResult selects the structured bounded payload when one was kept -// during ingestion, while preserving compatibility with hand-built and old -// history steps that only have the normalized result. -func stepDetailResult(s step) string { - if s.detailResult != "" { - return s.detailResult - } - return s.result -} - -func structuredItems(name, data string) ([]structuredToolItem, bool) { - // Structured grouping is authoritative only when it comes from the - // supported JSON envelope. Normalized legacy text may contain arbitrary - // bracketed lines such as "[2] warning"; treating those as item headers - // invents counts and can hide the real failure shape. - return structuredJSONItems(name, data) -} - -func structuredItemFailed(it structuredToolItem) bool { - return (it.hasExitCode && it.exitCode != 0) || - (it.hasStatus && (it.status < 200 || it.status >= 400)) || - (it.hasSuccess && !it.success) || it.error != "" -} - -func structuredHeadSuffix(name, result string, th theme) string { - items, ok := structuredItems(name, result) - if !ok || len(items) == 0 { - return "" - } - failed := 0 - confirmed := 0 - for _, it := range items { - if structuredItemFailed(it) { - failed++ - } - if (it.hasExitCode && it.exitCode == 0) || (it.hasSuccess && it.success) || (it.hasStatus && it.status >= 200 && it.status < 400) { - confirmed++ - } - } - n := len(items) - label := "items" - switch strings.ToLower(name) { - case "parallel_shell": - label = "commands" - case "batch_read": - label = "files" - case "batch_patch": - label = "patches" - case "http_batch": - label = "urls" - } - meta := structuredDisplayMetadata(result, n) - count := fmt.Sprintf("%d %s", n, label) - if meta.displayTruncated { - count = fmt.Sprintf("%d/%d %s shown", n, meta.totalItems, label) - } - if failed > 0 { - return th.stepErr.Render(fmt.Sprintf("%s · %d failed", count, failed)) - } - // Normalized bodies can lose success metadata; count them without - // claiming a verified successful execution. - if confirmed != n { - return th.stepRes.Render(count) - } - if meta.displayTruncated { - return th.stepRes.Render(count) - } - if strings.ToLower(name) == "batch_patch" { - return th.stepDone.Render(fmt.Sprintf("✓ %d patched", n)) - } - return th.stepDone.Render(fmt.Sprintf("✓ %d %s", n, label)) -} - func planSnapshotLines(result string, width int, th theme) []string { s := sanitize(foldUntrustedWrappers(result)) lines := strings.Split(s, "\n") @@ -736,144 +369,6 @@ func planHeadSuffix(result string, th theme) string { return "" } -func structuredItemHead(name string, item structuredToolItem, index int) (string, bool) { - label := item.label - if label == "" { - label = fmt.Sprintf("item %d", index+1) - } - switch strings.ToLower(name) { - case "parallel_shell": - if item.hasExitCode && item.exitCode != 0 { - return fmt.Sprintf("%s · exit %d", label, item.exitCode), true - } - return label, structuredItemFailed(item) - case "batch_patch": - if item.hasSuccess && !item.success || item.error != "" { - return "✗ " + label, true - } - if item.hasSuccess { - return "✓ " + label, false - } - return label, false - case "batch_read": - return label, structuredItemFailed(item) - case "http_batch": - if item.hasStatus { - return fmt.Sprintf("%s · %d", label, item.status), structuredItemFailed(item) - } - return label, structuredItemFailed(item) - default: - return label, structuredItemFailed(item) - } -} - -func appendStructuredLine(out *[]string, line string, width int, style lipgloss.Style) bool { - if len(*out) >= maxDetailLines { - return false - } - *out = append(*out, style.Render(truncate(strings.TrimRight(sanitize(line), " \t"), detailWidth(width)))) - return true -} - -// structuredDetailLines renders one result row per batch item. It is used -// only after a deliberate expand, and therefore may show command/path/URL -// labels that are deliberately absent from the calm one-line preview. -func structuredDetailLines(name, result string, width int, th theme) []string { - items, ok := structuredItems(name, result) - if !ok || len(items) == 0 { - return nil - } - meta := structuredDisplayMetadata(result, len(items)) - out := make([]string, 0, min(len(items)*3, maxDetailLines)) - if meta.displayTruncated { - omitted := meta.totalItems - len(items) - if omitted > 0 { - appendStructuredLine(&out, fmt.Sprintf("… %d more items omitted", omitted), width, th.stepArg) - } - } - if meta.bodiesOmitted { - appendStructuredLine(&out, "… item output omitted to stay within the detail limit", width, th.stepArg) - } - for i, item := range items { - head, failed := structuredItemHead(name, item, i) - style := th.stepName - if failed { - style = th.stepErr - } - if !appendStructuredLine(&out, head, width, style) { - break - } - if item.command != "" { - if !appendStructuredLine(&out, "$ "+item.command, width, th.stepArg) { - break - } - } - - appendBody := func(body string, bodyStyle lipgloss.Style) bool { - body = sanitize(foldUntrustedWrappers(body)) - for _, ln := range strings.Split(body, "\n") { - if strings.TrimSpace(ln) == "" { - continue - } - if !appendStructuredLine(&out, " "+ln, width, bodyStyle) { - return false - } - } - return true - } - - if item.diff != "" { - diff := sanitize(foldUntrustedWrappers(item.diff)) - var body []string - switch { - case hasFencedDiff(diff): - body = renderMixedDiff(diff, width, th) - case diffLooksLike(diff): - body = renderDiff(diff, width, th) - default: - body = []string{th.stepRes.Render(truncate(diff, detailWidth(width)))} - } - for _, ln := range body { - if len(out) >= maxDetailLines { - break - } - out = append(out, ln) - } - } - if item.content != "" && !appendBody(item.content, th.stepRes) { - break - } - if item.stdout != "" && !appendBody(item.stdout, th.stepRes) { - break - } - if item.stderr != "" && !appendBody("stderr: "+item.stderr, th.stepErr) { - break - } - if item.error != "" && item.stderr == "" && !appendBody(item.error, th.stepErr) { - break - } - if strings.ToLower(name) == "parallel_shell" && item.durationMS > 0 { - if !appendStructuredLine(&out, fmt.Sprintf(" %dms", item.durationMS), width, th.stepArg) { - break - } - } - if strings.ToLower(name) == "http_batch" && item.contentLen > 0 { - if !appendStructuredLine(&out, fmt.Sprintf(" %d bytes", item.contentLen), width, th.stepArg) { - break - } - } - if strings.ToLower(name) == "batch_read" && item.totalLines > 0 { - if !appendStructuredLine(&out, fmt.Sprintf(" %d total lines", item.totalLines), width, th.stepArg) { - break - } - } - } - if len(out) >= maxDetailLines { - out = append(out[:maxDetailLines-1], th.stepArg.Render("… output truncated")) - } - return out -} - // testSummary extracts a compact pass/fail summary from test-runner output // (go test / pytest / jest / vitest / cargo / TAP). ok=false when nothing // recognizable — only structured runner patterns produce a verdict. The @@ -1021,8 +516,8 @@ func stepDetail(name, result string, width int, th theme) []string { return out } } - if out := structuredDetailLines(name, result, width, th); len(out) > 0 { - return out + if retiredTool(name) { + return genericDetail(result, width, th) } switch { case hasFencedDiff(result): @@ -1036,7 +531,12 @@ func stepDetail(name, result string, width int, th theme) []string { } case fileReadTool(name): return renderNumbered(result, width, th) - case jsonLooksLike(result): + } + return genericDetail(result, width, th) +} + +func genericDetail(result string, width int, th theme) []string { + if jsonLooksLike(result) { if out := renderJSON(result, width, th); len(out) > 0 { return out } @@ -1073,8 +573,8 @@ func stepHeadSuffixFor(name, arg, result string, isErr bool, th theme) string { return chip } } - if chip := structuredHeadSuffix(name, result, th); chip != "" { - return chip + if retiredTool(name) { + return "" } if adds, dels, ok := diffStatOf(result); ok { return th.diffAdd.Render(fmt.Sprintf(" +%d", adds)) + @@ -1123,6 +623,9 @@ func stepHeadSuffixFor(name, arg, result string, isErr bool, th theme) string { // isSearchTool reports whether a tool's hits deserve the hit-count chip — // same substring matching style as toolGlyph. func isSearchTool(name string) bool { + if retiredTool(name) { + return false + } n := strings.ToLower(name) for _, k := range []string{"grep", "search", "glob", "find"} { if strings.Contains(n, k) { diff --git a/internal/tui/structured_ingestion_test.go b/internal/tui/structured_ingestion_test.go index f91d67b..e18b98c 100644 --- a/internal/tui/structured_ingestion_test.go +++ b/internal/tui/structured_ingestion_test.go @@ -7,114 +7,64 @@ import ( "github.com/BackendStack21/bodek/internal/client" ) -func TestStructuredBatchBoundsAreHonest(t *testing.T) { - rows := make([]string, 300) - for i := range rows { - body := strings.Repeat("x", 3000) - rows[i] = `{"path":"file-` + string(rune('a'+i%26)) + `.go","content":"` + body + `","stdout":"` + body + `"}` - } - raw := `{"results":[` + strings.Join(rows, ",") + `]}` - detail := boundedStructuredDetail("batch_read", raw) - th := newTheme() - if got := plain(structuredHeadSuffix("batch_read", detail, th)); got != "256/300 files shown" { - t.Fatalf("bounded batch summary = %q", got) - } - details := plain(strings.Join(stepDetail("batch_read", detail, 80, th), "\n")) - if !strings.Contains(details, "44 more items omitted") { - t.Fatalf("missing item omission marker:\n%s", details[:min(len(details), 1000)]) - } - if !strings.Contains(details, "item output omitted") { - t.Fatalf("missing body omission marker:\n%s", details[:min(len(details), 1000)]) - } -} - -func TestStructuredUnknownFallbackStaysGeneric(t *testing.T) { - raw := `{"results":[{"stdout":"known"},{"metadata":{"nested":true}}]}` - detail := boundedStructuredDetail("parallel_shell", raw) - if _, ok := structuredJSONItems("parallel_shell", detail); ok { - t.Fatal("unknown mixed results unexpectedly became structured items") - } - if got := structuredHeadSuffix("parallel_shell", detail, newTheme()); got != "" { - t.Fatalf("unknown fallback inferred batch summary: %q", plain(got)) - } -} - -func TestStructuredBatchDetailSurvivesLiveIngestion(t *testing.T) { - m := newTestModel() - m.msgs = append(m.msgs, message{role: roleAsst, streaming: true}) - m.curIdx = 0 - m.busy = true - m.handleEvent(client.Event{Type: "tool_call", Name: "batch_read", Data: `{"paths":["a.go","b.go"]}`}) - raw := `{"results":[{"path":"a.go","content":"1|package main","total_lines":9},{"path":"b.go","content":"1|package test","total_lines":4}]}` - m.handleEvent(client.Event{Type: "tool_result", Name: "batch_read", Data: raw}) - - s := m.msgs[0].steps[0] - if s.detailResult == "" { - t.Fatal("structured detail was discarded during live ingestion") - } - details := plain(strings.Join(stepDetail(s.name, stepDetailResult(s), 60, newTheme()), "\n")) - if !strings.Contains(details, "a.go") || !strings.Contains(details, "b.go") { - t.Fatalf("batch labels missing from detail:\n%s", details) - } - if s.result == s.detailResult { - t.Fatal("normalized result and structured detail should remain separate") - } -} - -func TestStructuredBatchLogBracketDoesNotChangeItemCount(t *testing.T) { - m := newTestModel() - m.msgs = append(m.msgs, message{role: roleAsst, streaming: true}) - m.curIdx = 0 - m.busy = true - m.handleEvent(client.Event{Type: "tool_call", Name: "parallel_shell", Data: `{}`}) - raw := `{"results":[{"command":"first","stdout":"start\n[2] warning","exit_code":0},{"command":"second","stdout":"done","exit_code":0}]}` - m.handleEvent(client.Event{Type: "tool_result", Name: "parallel_shell", Data: raw}) +// Retired names remain here deliberately: old sessions must be inspectable +// without restoring tool-specific renderers or normalizing away JSON fields. +func TestRetiredToolsUseGenericLiveAndReplayRendering(t *testing.T) { + for _, name := range retiredToolNames { + t.Run(name, func(t *testing.T) { + args := `{"command":"echo historical","paths":["a.go"],"requests":[{"url":"https://example.test"}]}` + raw := `{"results":[{"path":"a.go","command":"echo historical","stdout":"old output","exit_code":0,"duration_ms":12,"success":true,"status":200}],"metadata":{"keep":"unknown fields"}}` + live := newTestModel() + live.handleEvent(client.Event{Type: "tool_call", Name: name, Data: args}) + live.handleEvent(client.Event{Type: "tool_result", Name: name, Data: raw}) + live.handleEvent(client.Event{Type: "done"}) - s := m.msgs[0].steps[0] - if got := plain(stepHeadSuffix(s.name, s.arg, stepDetailResult(s), newTheme())); got != "✓ 2 commands" { - t.Fatalf("bracketed log changed batch count: %q", got) + call := client.SessionToolCall{ID: "call-1"} + call.Function.Name, call.Function.Arguments = name, args + replay := newTestModel() + replay.replayTranscript([]client.SessionMessage{ + {Role: "assistant", ToolCalls: []client.SessionToolCall{call}}, + {Role: "tool", Name: name, ToolCallID: "call-1", Content: "┌── TOOL RESULT: " + name + "\n" + raw + "\n└── END TOOL RESULT: " + name}, + }) + for _, m := range []*Model{live, replay} { + if len(m.msgs) != 1 || len(m.msgs[0].steps) != 1 { + t.Fatalf("unexpected transcript: %#v", m.msgs) + } + s := m.msgs[0].steps[0] + if !s.done || s.result != raw || s.callArgs != args || s.subagent || s.resultCard != nil { + t.Fatalf("historical step lost generic data: %+v", s) + } + if got := stepHeadSuffix(s.name, s.arg, s.result, m.th); got != "" { + t.Fatalf("retired tool received a typed summary: %q", plain(got)) + } + // Page the real expanded view through both invocation and JSON result. + m.msgs[0].steps[0].expanded = true + m.inspect = &inspectTarget{msgIdx: 0, stepIdx: 0, itemIdx: -1} + m.invalidateInspect() + var rendered strings.Builder + for page := 0; page < 6; page++ { + out, _, _ := m.renderStep(m.msgs[0].steps[0], false, 0, 0, 0) + rendered.WriteString(plain(out)) + m.Update(key("pgdown")) + } + for _, want := range []string{"invocation · arguments", `"results"`, `"duration_ms"`, `"metadata"`, "unknown fields"} { + if !strings.Contains(rendered.String(), want) { + t.Errorf("generic expanded view missing %q: %s", want, rendered.String()[:min(rendered.Len(), 1500)]) + } + } + } + }) } } -func TestStructuredBatchDetailPreservesFailureAfterLiveAndReplay(t *testing.T) { - raw := `{"results":[{"path":"missing.go","error":"file not found","success":false}]}` - - m := newTestModel() - m.msgs = append(m.msgs, message{role: roleAsst, streaming: true}) - m.curIdx = 0 - m.busy = true - m.handleEvent(client.Event{Type: "tool_call", Name: "batch_patch", Data: `{"patches":[]}`}) - m.handleEvent(client.Event{Type: "tool_result", Name: "batch_patch", Data: raw}) - live := m.msgs[0].steps[0] - if !live.isErr || !live.expanded { - t.Fatalf("live structured failure not retained: %+v", live) - } - - replay := newTestModel() - call := client.SessionToolCall{ID: "call-1"} - call.Function.Name = "batch_patch" - call.Function.Arguments = `{"patches":[]}` - replay.replayTranscript([]client.SessionMessage{ - {Role: "user", Content: "patch it"}, - {Role: "assistant", ToolCalls: []client.SessionToolCall{call}}, - {Role: "tool", Name: "batch_patch", ToolCallID: "call-1", Content: "┌── TOOL RESULT: batch_patch\n" + raw + "\n└── END TOOL RESULT: batch_patch"}, - }) - if len(replay.msgs) != 2 || !replay.msgs[1].steps[0].isErr { - t.Fatalf("restored structured failure not retained: %#v", replay.msgs) - } - step := replay.msgs[1].steps[0] - if got := plain(strings.Join(stepDetail(step.name, stepDetailResult(step), 60, newTheme()), "\n")); !strings.Contains(got, "missing.go") { - t.Fatalf("restored batch label missing from detail: %s", got) - } -} - -func TestStructuredTextDoesNotInventBatchGrouping(t *testing.T) { - th := newTheme() - if got := structuredHeadSuffix("parallel_shell", "[2] warning", th); got != "" { - t.Fatalf("legacy bracketed text inferred a batch: %q", plain(got)) - } - if got := stepDetail("parallel_shell", "[2] warning", 60, th); len(got) != 1 || !strings.Contains(plain(got[0]), "[2] warning") { - t.Fatalf("legacy bracketed text was not kept plain: %#v", got) +func TestRetiredToolFallbackRemainsBounded(t *testing.T) { + for _, name := range retiredToolNames { + for _, raw := range []string{strings.Repeat("界", 100000), strings.Repeat("line\n", 1000)} { + got := toolResultPreview(name, raw) + if len(got) > 128*1024+100 || len(strings.Split(got, "\n")) > 201 || !strings.Contains(got, "…") { + t.Fatalf("%s fallback is unbounded or lacks an omission marker", name) + } + } } } diff --git a/internal/tui/structured_tools_test.go b/internal/tui/structured_tools_test.go index b9aa509..828a832 100644 --- a/internal/tui/structured_tools_test.go +++ b/internal/tui/structured_tools_test.go @@ -1,66 +1,75 @@ package tui import ( + "reflect" "strings" "testing" "github.com/charmbracelet/lipgloss" ) -func TestStructuredToolSummaryAndDetails(t *testing.T) { +var retiredToolNames = []string{"parallel_shell", "batch_patch", "batch_read", "multi_grep", "http_batch"} + +func TestRetiredToolsHaveNoTypedRendering(t *testing.T) { th := newTheme() - parallel := `{"results":[{"command":"go test ./...","stdout":"ok","exit_code":0,"duration_ms":12},{"command":"go vet ./...","stderr":"bad","exit_code":1,"duration_ms":4}]}` - if got := plain(stepHeadSuffix("parallel_shell", "", parallel, th)); got != "2 commands · 1 failed" { - t.Fatalf("parallel summary = %q", got) - } - details := plain(strings.Join(stepDetail("parallel_shell", parallel, 48, th), "\n")) - for _, want := range []string{"go test ./...", "go vet ./...", "exit 1", "stderr: bad"} { - if !strings.Contains(details, want) { - t.Errorf("parallel details missing %q:\n%s", want, details) + for _, name := range retiredToolNames { + for _, variant := range []string{name, " " + strings.ToUpper(name) + " "} { + if toolGlyph(variant) != "✦" || isShellTool(variant) || isSearchTool(variant) || fileReadTool(variant) || touchedPath(variant, "a.go") != "" { + t.Errorf("%q matched a supported tool family", variant) + } + if got := toolProgress(variant, "go test ./..."); got != "🔧 running "+variant { + t.Errorf("retired progress = %q", got) + } + if got := invocationText(step{name: variant, callArgs: `{"command":"go test ./..."}`}); !strings.HasPrefix(got, "invocation · arguments\n") { + t.Errorf("retired tool got a shell invocation: %q", got) + } + for _, raw := range []string{ + `{"results":[{"command":"echo x","stdout":"safe","exit_code":0,"duration_ms":5,"path":"a.go","success":true,"status":200}]}`, + `{"results":[]}`, `{"results":["not an object"]}`, `{"results":[{"metadata":{"nested":true}}]}`, `{"results":[{"path":123}]}`, + `{"content":"keep envelope","total_lines":1}`, + "@@ -1 +1 @@\n-old\n+new", "```diff\n-old\n+new\n```", + "found 3 matches", "ok example.test/pkg 0.5s", "[2] warning", + `{"results":[{"command":"echo \u001b]52;c;secret\u0007","stdout":"safe"}]}`, + "plain \x1b]52;c;secret\x07", `{"results":`, + } { + result := toolResultPreview(variant, raw) + if got := stepHeadSuffix(variant, "go test ./...", result, th); got != "" { + t.Errorf("%s inferred a typed summary: %q", name, plain(got)) + } + got := stepDetail(variant, result, 60, th) + if want := genericDetail(result, 60, th); !reflect.DeepEqual(got, want) { + t.Errorf("%s bypassed generic rendering: %q", name, got) + } + if strings.ContainsAny(plain(strings.Join(got, "\n")), "\x1b\x07") { + t.Errorf("%s exposed terminal controls", name) + } + if got := scanReceipt(message{steps: []step{{name: variant, arg: "a.go", result: result}}}); got != (receipt{}) { + t.Errorf("%s contributed a typed receipt: %+v", name, got) + } + } } } - if strings.Contains(details, `"duration_ms"`) || strings.Contains(details, `"results"`) { - t.Errorf("parallel details leaked wire metadata:\n%s", details) - } } -func TestStructuredToolKinds(t *testing.T) { +func TestSupportedReplacementToolsKeepRendering(t *testing.T) { th := newTheme() - cases := []struct { - name string - result string - want string - body string - }{ - { - name: "batch_patch", - result: `{"results":[{"path":"a.go","success":true,"diff":"--- a/a.go\n+++ b/a.go\n@@ -1 +1 @@\n-old\n+new"},{"path":"b.go","success":false,"error":"old_string not found"}]}`, - want: "2 patches · 1 failed", - body: "old_string not found", - }, - { - name: "batch_read", - result: `{"results":[{"path":"a.go","content":"1|package main","total_lines":9},{"path":"b.go","error":"file not found"}]}`, - want: "2 files · 1 failed", - body: "a.go", - }, - { - name: "http_batch", - result: `{"results":[{"url":"https://example.test/ok","status":200,"content_length":42},{"url":"https://example.test/missing","status":404,"error":"not found"}]}`, - want: "2 urls · 1 failed", - body: "404", - }, - } - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - if got := plain(stepHeadSuffix(tc.name, "", tc.result, th)); got != tc.want { - t.Fatalf("summary = %q, want %q", got, tc.want) - } - got := plain(strings.Join(stepDetail(tc.name, tc.result, 60, th), "\n")) - if !strings.Contains(got, tc.body) { - t.Fatalf("details missing %q:\n%s", tc.body, got) - } - }) + for _, tc := range []struct{ name, arg, raw, glyph, chip, body string }{ + {"shell", "go test ./...", "ok example.test/pkg 0.5s", "❯", "✓ tests pass", "ok"}, + {"patch", "a.go", "@@ -1 +1 @@\n-old\n+new", "✎", " +1 −1", "+new"}, + {"read_file", "a.go", `{"content":"package main","total_lines":1}`, "◰", "", "1 │ package main"}, + {"search_files", "needle", "found 3 matches", "⌕", "3 hits", "found 3 matches"}, + {"http_request", "https://example.test", `{"status":200,"body":"response"}`, "⌖", "", `"status": 200`}, + } { + result := toolResultPreview(tc.name, tc.raw) + if got := toolGlyph(tc.name); got != tc.glyph { + t.Errorf("%s glyph = %q", tc.name, got) + } + if got := plain(stepHeadSuffix(tc.name, tc.arg, result, th)); got != tc.chip { + t.Errorf("%s chip = %q, want %q", tc.name, got, tc.chip) + } + if got := plain(strings.Join(stepDetail(tc.name, result, 80, th), "\n")); !strings.Contains(got, tc.body) { + t.Errorf("%s detail missing %q: %s", tc.name, tc.body, got) + } } } @@ -92,29 +101,6 @@ func TestPlanSnapshotDetailsAreCompactAndBounded(t *testing.T) { } } -func TestStructuredToolsFailSafeAndSanitize(t *testing.T) { - th := newTheme() - malformed := []string{ - `{"results":[]}`, - `{"results":["not an object"]}`, - `{"results":[{"metadata":{"nested":true}}]}`, - `{"results":[{"path":123}]}`, - } - for _, raw := range malformed { - if got := structuredHeadSuffix("batch_read", raw, th); got != "" { - t.Errorf("malformed result %q got chip %q", raw, plain(got)) - } - if got := structuredDetailLines("batch_read", raw, 40, th); got != nil { - t.Errorf("malformed result %q got structured details %q", raw, got) - } - } - hostile := `{"results":[{"command":"echo \u001b]52;c;secret\u0007","stdout":"safe"}]}` - got := plain(strings.Join(stepDetail("parallel_shell", hostile, 40, th), "\n")) - if strings.ContainsAny(got, "\x1b\x07") { - t.Fatalf("terminal control payload survived: %q", got) - } -} - func TestStructuredNormalizedItemsStayChronological(t *testing.T) { th := newTheme() result := "[1] first\n\n[2] exit status 2\nstderr: second" diff --git a/internal/tui/view.go b/internal/tui/view.go index c13fb14..4c561d7 100644 --- a/internal/tui/view.go +++ b/internal/tui/view.go @@ -1232,7 +1232,7 @@ func (m *Model) renderStep(s step, streaming bool, msgIdx, stepIdx, startLine in right := "" live := !s.done && streaming if s.done { - right = stepHeadSuffixFor(s.name, s.arg, stepDetailResult(s), s.isErr, th) + right = stepHeadSuffixFor(s.name, s.arg, s.result, s.isErr, th) // The sealed duration keeps the live clock's slot — “how long did // this tool take” survives completion instead of vanishing with // the running timer. Resumed history (dur 0) shows none. @@ -1320,7 +1320,7 @@ func (m *Model) renderStep(s step, streaming bool, msgIdx, stepIdx, startLine in if s.resultCard != nil { details = append(details, agentResultLines(m, s.resultCard, detailBudget)...) } else if s.result != "" { - details = append(details, stepDetail(s.name, stepDetailResult(s), m.vp.Width, th)...) + details = append(details, stepDetail(s.name, s.result, m.vp.Width, th)...) } if !focusedParent { sectionBreak = len(details) @@ -1353,7 +1353,7 @@ func (m *Model) renderStep(s step, streaming bool, msgIdx, stepIdx, startLine in if s.resultCard != nil { details = append(details, agentResultLines(m, s.resultCard, detailBudget)...) } else { - details = append(details, stepDetail(s.name, stepDetailResult(s), m.vp.Width, th)...) + details = append(details, stepDetail(s.name, s.result, m.vp.Width, th)...) } for i, d := range m.toolDetailPage(&s, details, detailBudget, invocationRows, "invocation", "result") { conn := " "