diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index beac76c7ce..59b7ff9545 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -156,6 +156,14 @@ jobs: timeout-minutes: 3 run: go test -timeout=2m -run '^TestBashSharesSessionTempAcrossCalls$' ./internal/tool/builtin + # Shell execution contract: PowerShell identity, Chinese workspace paths, + # UTF-8 output, and ExitCode retention must gate PRs on native Windows. + # Keep this focused (not full ./internal/tool) so the smoke budget holds. + - name: test (Windows shell execution contract) + if: env.RUN_STEPS == 'true' && runner.os == 'Windows' && github.event_name == 'pull_request' + timeout-minutes: 5 + run: go test -timeout=3m -run '^TestBashPowerShellExecuteDetailedContract$|^TestBashPowerShell51PreflightRejectsAndAndDetailed$|^TestBashPowerShellOutputIsUTF8$|^TestBashPowerShellSurfacesNonZeroExit$|^TestBashPowerShellRejectsChaining$' ./internal/tool/builtin + - name: test (full) if: env.RUN_STEPS == 'true' && runner.os == 'Windows' && github.event_name != 'pull_request' timeout-minutes: 10 diff --git a/desktop/app.go b/desktop/app.go index b35b6a9bf1..74655ed93c 100644 --- a/desktop/app.go +++ b/desktop/app.go @@ -5538,12 +5538,15 @@ type HistoryMessage struct { ToolName string `json:"toolName,omitempty"` ToolResultArchived bool `json:"toolResultArchived,omitempty"` ToolResultError string `json:"toolResultError,omitempty"` - Pending bool `json:"pending,omitempty"` - Trigger string `json:"trigger,omitempty"` - Messages int `json:"messages,omitempty"` - Summary string `json:"summary,omitempty"` - Archive string `json:"archive,omitempty"` - DecisionReceipt *provider.DecisionReceipt `json:"decisionReceipt,omitempty"` + // Execution is local shell metadata restored onto ToolCards after history + // reload. Omitted when absent so older frontends ignore it safely. + Execution *provider.ToolExecution `json:"execution,omitempty"` + Pending bool `json:"pending,omitempty"` + Trigger string `json:"trigger,omitempty"` + Messages int `json:"messages,omitempty"` + Summary string `json:"summary,omitempty"` + Archive string `json:"archive,omitempty"` + DecisionReceipt *provider.DecisionReceipt `json:"decisionReceipt,omitempty"` } type HistoryToolCall struct { @@ -5966,6 +5969,7 @@ func historyMessagesWithPlannerDisplaysAndLookups( hm.ToolCallID = m.ToolCallID hm.ToolName = m.Name hm.Content, hm.ToolResultArchived, hm.ToolResultError = historyToolResultContent(m.Content, m.ToolCallID != "") + hm.Execution = m.ToolExecution } hasVisibleLocalContent := strings.TrimSpace(hm.Content) != "" || strings.TrimSpace(hm.Reasoning) != "" || len(hm.ToolCalls) > 0 || (!m.LocalOnly && m.Role == provider.RoleTool) if !m.LocalOnly || hasVisibleLocalContent { diff --git a/desktop/frontend/package.json b/desktop/frontend/package.json index 349dc72d41..fe6dd6b6fe 100644 --- a/desktop/frontend/package.json +++ b/desktop/frontend/package.json @@ -18,7 +18,7 @@ "test:settings-responsive": "tsx src/__tests__/settings-responsive-layout.test.ts", "pretest": "pnpm test:terminal && pnpm test:task-monitor && tsx src/__tests__/rich-composer-selection.test.tsx && tsx src/__tests__/context-center-contract.test.ts && tsx src/__tests__/provider-model-cache.test.ts && tsx src/__tests__/format-tokens.test.ts && pnpm test:usage-stats && pnpm test:settings-responsive && pnpm test:composer-menu-viewport", "test:composer-menu-viewport": "tsx src/__tests__/composer-menu-viewport.test.ts && tsx src/__tests__/virtual-menu-identity.test.tsx", - "test": "node scripts/test-todo-visibility.mjs && tsx src/__tests__/goal-action-errors.test.tsx && tsx src/__tests__/goal-activation-tab-routing.test.tsx && tsx src/__tests__/session-export.test.ts && tsx src/__tests__/conversation-width.test.ts && tsx src/__tests__/external-opener.test.tsx && tsx src/__tests__/anchored-popover-scroll.test.tsx && tsx src/__tests__/confirm-dialog.test.tsx && tsx src/__tests__/scroll-manager.test.tsx && tsx src/__tests__/layout-style-defaults.test.ts && tsx src/__tests__/scroll-content-observer.test.ts && tsx src/__tests__/math-golden.test.ts && tsx src/__tests__/mermaid-rendering.test.tsx && tsx src/__tests__/github-link-rendering.test.tsx && tsx src/__tests__/markdown-image-proxy.test.ts && tsx src/__tests__/text-size.test.ts && tsx src/__tests__/font-availability.test.ts && tsx src/__tests__/typography-preferences.test.ts && tsx src/__tests__/theme-auto-background.test.ts && tsx src/__tests__/theme-pack.test.ts && tsx src/__tests__/theme-editor-keyboard.test.tsx && tsx src/__tests__/typography-overflow-contract.test.ts && tsx src/__tests__/ask-card-layout.test.ts && tsx src/__tests__/approval-modal-file-reference.test.tsx && tsx src/__tests__/ref-token.test.tsx && tsx src/__tests__/decision-surface.test.tsx && tsx src/__tests__/extension-surface.test.tsx && tsx src/__tests__/provider-editor-model-picker.test.tsx && tsx src/__tests__/provider-model-refresh.test.ts && tsx src/__tests__/model-switcher-refresh.test.tsx && tsx src/__tests__/composer-profile.test.ts && tsx src/__tests__/composer-history.test.ts && tsx src/__tests__/session-title-contract.test.ts && tsx src/__tests__/composer-keyboard.test.ts && tsx src/__tests__/composer-goal-toggle.test.tsx && tsx src/__tests__/composer-run-strip.test.tsx && tsx src/__tests__/composer-session-draft.test.tsx && tsx src/__tests__/language-highlighting.test.ts && tsx src/__tests__/composer-image-capability.test.tsx && tsx src/__tests__/composer-context-menu-clipboard.test.tsx && tsx src/__tests__/composer-resized-input-css.test.ts && tsx src/__tests__/use-controller-meta.test.ts && tsx src/__tests__/use-controller-stream-progress.test.ts && tsx src/__tests__/use-controller-live-context.test.tsx && tsx src/__tests__/use-controller-cancel-reconcile.test.tsx && tsx src/__tests__/pending-prompt-stale-status.test.tsx && tsx src/__tests__/new-session-load-race.test.tsx && tsx src/__tests__/tab-switch-hydration.test.tsx && tsx src/__tests__/activate-topic-stale.test.tsx && tsx src/__tests__/ready-event-meta-sync.test.tsx && tsx src/__tests__/ready-meta-reconcile.test.tsx && tsx src/__tests__/recovery-banner-privacy.test.ts && tsx src/__tests__/recovery-quiet-notifications.test.ts && tsx src/__tests__/workspace-layout.test.ts && tsx src/__tests__/footer-decision-overflow.test.ts && tsx src/__tests__/workspace-change-status.test.ts && tsx src/__tests__/workspace-changes-errors.test.tsx && tsx src/__tests__/workspace-split.test.ts && tsx src/__tests__/workspace-preview-css.test.ts && tsx src/__tests__/resize-drag.test.ts && tsx src/__tests__/wails-resize-fix.test.tsx && tsx src/__tests__/tool-approval-mode.test.ts && tsx src/__tests__/sound.test.ts && tsx src/__tests__/heartbeat-next-run.test.ts && tsx src/__tests__/keyboard-shortcuts.test.ts && tsx src/__tests__/shortcuts-recorder-focus.test.tsx && tsx src/__tests__/send-failed.test.ts && tsx src/__tests__/edit-replay.test.ts && tsx src/__tests__/message-pasted-blocks.test.ts && tsx src/__tests__/history-tool-status.test.ts && tsx src/__tests__/history-recovery-copies.test.tsx && tsx src/__tests__/message-selection-copy.test.ts && tsx src/__tests__/transcript-selection-menu.test.tsx && tsx src/__tests__/turn-action-copy.test.ts && tsx src/__tests__/turn-actions-rendering.test.ts && tsx src/__tests__/code-block-copy-position.test.tsx && tsx src/__tests__/line-number-code.test.tsx && tsx src/__tests__/message-reasoning-panel.test.tsx && tsx src/__tests__/transcript-process-fold.test.ts && tsx src/__tests__/transcript-fold-preference.test.tsx && tsx src/__tests__/reasoning-display.test.ts && tsx src/__tests__/attachment-display.test.ts && tsx src/__tests__/crash-reporting.test.ts && tsx src/__tests__/bridge-drag-rejection.test.ts && tsx src/__tests__/startup-settings-contract.test.ts && tsx src/__tests__/bundle-contract.test.ts && tsx src/__tests__/command-palette-css.test.ts && tsx src/__tests__/command-palette-interactions.test.tsx && tsx src/__tests__/context-panel-breakdown.test.ts && tsx src/__tests__/context-window-ring.test.tsx && tsx src/__tests__/statusbar-workspace.test.tsx && tsx src/__tests__/settings-refresh-snapshot.test.tsx && tsx src/__tests__/capabilities-panel-actions.test.ts && tsx src/__tests__/diagnostics-settings.test.tsx && tsx src/__tests__/tool-data-archive.test.ts && tsx src/__tests__/tool-subject.test.ts && tsx src/__tests__/subagent-progress.test.ts && tsx src/__tests__/subagent-progress-card.test.tsx && tsx src/__tests__/diff-rendering.test.ts && tsx src/__tests__/render-optimization.test.ts && tsx src/__tests__/transcript-grouping.test.ts && tsx src/__tests__/project-tree-runtime.test.ts && tsx src/__tests__/app-chrome-tabs.test.ts && tsx src/__tests__/open-topic-coalescing.test.tsx && tsx src/__tests__/delivery-worktree.test.ts && tsx src/__tests__/memory-compiler-display.test.ts && tsx src/__tests__/memory-citation-visibility.test.ts", + "test": "node scripts/test-todo-visibility.mjs && tsx src/__tests__/goal-action-errors.test.tsx && tsx src/__tests__/goal-activation-tab-routing.test.tsx && tsx src/__tests__/session-export.test.ts && tsx src/__tests__/conversation-width.test.ts && tsx src/__tests__/external-opener.test.tsx && tsx src/__tests__/anchored-popover-scroll.test.tsx && tsx src/__tests__/confirm-dialog.test.tsx && tsx src/__tests__/scroll-manager.test.tsx && tsx src/__tests__/layout-style-defaults.test.ts && tsx src/__tests__/scroll-content-observer.test.ts && tsx src/__tests__/math-golden.test.ts && tsx src/__tests__/mermaid-rendering.test.tsx && tsx src/__tests__/github-link-rendering.test.tsx && tsx src/__tests__/markdown-image-proxy.test.ts && tsx src/__tests__/text-size.test.ts && tsx src/__tests__/font-availability.test.ts && tsx src/__tests__/typography-preferences.test.ts && tsx src/__tests__/theme-auto-background.test.ts && tsx src/__tests__/theme-pack.test.ts && tsx src/__tests__/theme-editor-keyboard.test.tsx && tsx src/__tests__/typography-overflow-contract.test.ts && tsx src/__tests__/ask-card-layout.test.ts && tsx src/__tests__/approval-modal-file-reference.test.tsx && tsx src/__tests__/ref-token.test.tsx && tsx src/__tests__/decision-surface.test.tsx && tsx src/__tests__/extension-surface.test.tsx && tsx src/__tests__/provider-editor-model-picker.test.tsx && tsx src/__tests__/provider-model-refresh.test.ts && tsx src/__tests__/model-switcher-refresh.test.tsx && tsx src/__tests__/composer-profile.test.ts && tsx src/__tests__/composer-history.test.ts && tsx src/__tests__/session-title-contract.test.ts && tsx src/__tests__/composer-keyboard.test.ts && tsx src/__tests__/composer-goal-toggle.test.tsx && tsx src/__tests__/composer-run-strip.test.tsx && tsx src/__tests__/composer-session-draft.test.tsx && tsx src/__tests__/language-highlighting.test.ts && tsx src/__tests__/composer-image-capability.test.tsx && tsx src/__tests__/composer-context-menu-clipboard.test.tsx && tsx src/__tests__/composer-resized-input-css.test.ts && tsx src/__tests__/use-controller-meta.test.ts && tsx src/__tests__/use-controller-stream-progress.test.ts && tsx src/__tests__/use-controller-live-context.test.tsx && tsx src/__tests__/use-controller-cancel-reconcile.test.tsx && tsx src/__tests__/pending-prompt-stale-status.test.tsx && tsx src/__tests__/new-session-load-race.test.tsx && tsx src/__tests__/tab-switch-hydration.test.tsx && tsx src/__tests__/activate-topic-stale.test.tsx && tsx src/__tests__/ready-event-meta-sync.test.tsx && tsx src/__tests__/ready-meta-reconcile.test.tsx && tsx src/__tests__/recovery-banner-privacy.test.ts && tsx src/__tests__/recovery-quiet-notifications.test.ts && tsx src/__tests__/workspace-layout.test.ts && tsx src/__tests__/footer-decision-overflow.test.ts && tsx src/__tests__/workspace-change-status.test.ts && tsx src/__tests__/workspace-changes-errors.test.tsx && tsx src/__tests__/workspace-split.test.ts && tsx src/__tests__/workspace-preview-css.test.ts && tsx src/__tests__/resize-drag.test.ts && tsx src/__tests__/wails-resize-fix.test.tsx && tsx src/__tests__/tool-approval-mode.test.ts && tsx src/__tests__/sound.test.ts && tsx src/__tests__/heartbeat-next-run.test.ts && tsx src/__tests__/keyboard-shortcuts.test.ts && tsx src/__tests__/shortcuts-recorder-focus.test.tsx && tsx src/__tests__/send-failed.test.ts && tsx src/__tests__/edit-replay.test.ts && tsx src/__tests__/message-pasted-blocks.test.ts && tsx src/__tests__/history-tool-status.test.ts && tsx src/__tests__/history-recovery-copies.test.tsx && tsx src/__tests__/message-selection-copy.test.ts && tsx src/__tests__/transcript-selection-menu.test.tsx && tsx src/__tests__/turn-action-copy.test.ts && tsx src/__tests__/turn-actions-rendering.test.ts && tsx src/__tests__/code-block-copy-position.test.tsx && tsx src/__tests__/line-number-code.test.tsx && tsx src/__tests__/message-reasoning-panel.test.tsx && tsx src/__tests__/transcript-process-fold.test.ts && tsx src/__tests__/transcript-fold-preference.test.tsx && tsx src/__tests__/reasoning-display.test.ts && tsx src/__tests__/attachment-display.test.ts && tsx src/__tests__/crash-reporting.test.ts && tsx src/__tests__/bridge-drag-rejection.test.ts && tsx src/__tests__/startup-settings-contract.test.ts && tsx src/__tests__/bundle-contract.test.ts && tsx src/__tests__/command-palette-css.test.ts && tsx src/__tests__/command-palette-interactions.test.tsx && tsx src/__tests__/context-panel-breakdown.test.ts && tsx src/__tests__/context-window-ring.test.tsx && tsx src/__tests__/statusbar-workspace.test.tsx && tsx src/__tests__/settings-refresh-snapshot.test.tsx && tsx src/__tests__/capabilities-panel-actions.test.ts && tsx src/__tests__/diagnostics-settings.test.tsx && tsx src/__tests__/tool-data-archive.test.ts && tsx src/__tests__/tool-card-shell-execution.test.tsx && tsx src/__tests__/tool-subject.test.ts && tsx src/__tests__/subagent-progress.test.ts && tsx src/__tests__/subagent-progress-card.test.tsx && tsx src/__tests__/diff-rendering.test.ts && tsx src/__tests__/render-optimization.test.ts && tsx src/__tests__/transcript-grouping.test.ts && tsx src/__tests__/project-tree-runtime.test.ts && tsx src/__tests__/app-chrome-tabs.test.ts && tsx src/__tests__/open-topic-coalescing.test.tsx && tsx src/__tests__/delivery-worktree.test.ts && tsx src/__tests__/memory-compiler-display.test.ts && tsx src/__tests__/memory-citation-visibility.test.ts", "test:remote": "tsx src/__tests__/remote-workspace-launch.test.ts && tsx src/__tests__/remote-store.test.ts && tsx src/__tests__/remote-error-ux.test.tsx && tsx src/__tests__/remote-hosts-page.test.tsx && tsx src/__tests__/remote-secret-dialog.test.tsx && node --import ./scripts/svg-stub-register.mjs --import tsx src/__tests__/remote-server-panel.test.tsx && tsx src/__tests__/statusbar-workspace.test.tsx", "test:performance": "tsx src/__tests__/history-performance-benchmark.tsx", "test:typecheck": "tsc --noEmit -p tsconfig.test.json", diff --git a/desktop/frontend/scripts/check-bundle-budget.mjs b/desktop/frontend/scripts/check-bundle-budget.mjs index 54fcc65867..cfea88cbb8 100644 --- a/desktop/frontend/scripts/check-bundle-budget.mjs +++ b/desktop/frontend/scripts/check-bundle-budget.mjs @@ -53,9 +53,10 @@ if (localeChunks.length !== 2) { for (const path of localeChunks) { const name = basename(path); // Task Monitor adds 37 user-facing labels to each on-demand locale, while - // Extension UI adds its own status and action copy. Keep both dictionaries + // Extension UI adds its own status and action copy. Shell execution contract + // cards add a small set of verification/risk strings. Keep both dictionaries // within narrowly measured, explicit allowances. - const budget = name.startsWith("zh-TW-") ? 53.25 * 1024 : 52.5 * 1024; + const budget = name.startsWith("zh-TW-") ? 53.5 * 1024 : 52.75 * 1024; assertBudget(`${name} gzip`, gzipBytes(path), budget); } diff --git a/desktop/frontend/src/__tests__/tool-card-shell-execution.test.tsx b/desktop/frontend/src/__tests__/tool-card-shell-execution.test.tsx new file mode 100644 index 0000000000..824c2de485 --- /dev/null +++ b/desktop/frontend/src/__tests__/tool-card-shell-execution.test.tsx @@ -0,0 +1,422 @@ +// Run: tsx src/__tests__/tool-card-shell-execution.test.tsx +// +// Desktop ToolCard shell-execution presentation for the three host paths: +// 1) live tool_result event (reducer → card) +// 2) history recovery (historyMessagesToItems) +// 3) archived expand (ToolResultForTab injects execution back onto fullData) + +import { JSDOM } from "jsdom"; +import React from "react"; +import { act } from "react"; +import { createRoot } from "react-dom/client"; +import gsap from "gsap"; +import { ToolCard } from "../components/ToolCard"; +import { LocaleProvider } from "../lib/i18n"; +import { historyMessagesToItems, initialState, reducer, type Item } from "../lib/useController"; +import type { HistoryMessage, WireShellExecution } from "../lib/types"; + +type ToolItem = Extract; + +// jsdom has no layout engine: stub GSAP the same way as subagent-progress-card. +type GsapToOptions = { onComplete?: () => void }; +const gsapForTests = gsap as unknown as { + to: (target: unknown, vars: GsapToOptions) => unknown; + fromTo: (target: unknown, from: unknown, vars: GsapToOptions) => unknown; + set: (target: unknown, vars: unknown) => unknown; + killTweensOf: (target: unknown) => void; +}; +gsapForTests.to = (_target: unknown, vars: GsapToOptions) => { + vars.onComplete?.(); + return {}; +}; +gsapForTests.fromTo = (_target: unknown, _from: unknown, vars: GsapToOptions) => { + vars.onComplete?.(); + return {}; +}; +gsapForTests.set = () => ({}); +gsapForTests.killTweensOf = () => {}; + +let passed = 0; +let failed = 0; + +function ok(value: unknown, label: string) { + if (value) { + process.stdout.write(` PASS ${label}\n`); + passed += 1; + } else { + process.stdout.write(` FAIL ${label}\n`); + failed += 1; + } +} + +function eq(actual: unknown, expected: unknown, label: string) { + if (actual === expected) ok(true, label); + else ok(false, `${label}: expected ${JSON.stringify(expected)}, got ${JSON.stringify(actual)}`); +} + +function flushTimers(): Promise { + return new Promise((resolve) => setTimeout(resolve, 0)); +} + +function installDom() { + const dom = new JSDOM("
", { + pretendToBeVisual: true, + url: "http://localhost/", + }); + (globalThis as typeof globalThis & { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + globalThis.window = dom.window as unknown as Window & typeof globalThis; + globalThis.document = dom.window.document; + Object.defineProperty(globalThis, "navigator", { configurable: true, value: dom.window.navigator }); + globalThis.Node = dom.window.Node; + globalThis.Element = dom.window.Element; + globalThis.HTMLElement = dom.window.HTMLElement; + globalThis.Event = dom.window.Event; + globalThis.MouseEvent = dom.window.MouseEvent; + globalThis.requestAnimationFrame = dom.window.requestAnimationFrame.bind(dom.window); + globalThis.cancelAnimationFrame = dom.window.cancelAnimationFrame.bind(dom.window); + dom.window.matchMedia = () => ({ + matches: true, + media: "(prefers-reduced-motion: reduce)", + onchange: null, + addListener: () => undefined, + removeListener: () => undefined, + addEventListener: () => undefined, + removeEventListener: () => undefined, + dispatchEvent: () => false, + }); + return dom; +} + +async function renderCard(item: ToolItem, tabId?: string) { + const dom = installDom(); + const rootEl = document.getElementById("root"); + if (!rootEl) throw new Error("missing root"); + const root = createRoot(rootEl); + await act(async () => { + root.render( + React.createElement(LocaleProvider, null, + React.createElement(ToolCard, { item, tabId }), + ), + ); + await flushTimers(); + }); + return { + root, + dom, + async expand() { + const head = document.querySelector(".tool__head") as HTMLButtonElement | null; + if (!head) throw new Error("tool head missing"); + await act(async () => { + head.click(); + await flushTimers(); + }); + }, + async cleanup() { + await act(async () => { + root.unmount(); + }); + dom.window.close(); + }, + }; +} + +const failedPSExecution: WireShellExecution = { + kind: "shell", + shell: "powershell", + shellVersion: "5.1", + platform: "windows", + supportsAndAnd: false, + state: "failed", + failurePhase: "execution", + exitCode: 1, + outputTail: "Select-String : 找不到路径“C:\\中文\\app.ps1”。\nAt line:1 char:1", + mutationRisk: "may_be_partial", + verification: "not_verification", + durationMs: 42, +}; + +const preflightExecution: WireShellExecution = { + kind: "shell", + shell: "bash", + state: "not_run", + failurePhase: "preflight", + mutationRisk: "not_started", + verification: "not_run", + durationMs: 0, +}; + +console.log("\ntool card shell execution"); + +// ── Path 1: live tool_result event ── +{ + let s = reducer(initialState, { type: "event", e: { kind: "turn_started" } }); + s = reducer(s, { + type: "event", + e: { + kind: "tool_dispatch", + tool: { + id: "live-ps", + name: "bash", + args: `{"command":"Get-Content .\\\\中文\\\\app.ps1"}`, + readOnly: false, + }, + }, + }); + s = reducer(s, { + type: "event", + e: { + kind: "tool_result", + tool: { + id: "live-ps", + name: "bash", + readOnly: false, + output: "error: command exited: exit status 1\nSelect-String failed", + err: "command exited: exit status 1", + durationMs: 42, + execution: failedPSExecution, + }, + }, + }); + const item = s.items.find((it): it is ToolItem => it.kind === "tool" && it.id === "live-ps"); + ok(!!item, "live path: tool item created"); + eq(item?.isShell, true, "live path: bash result marked isShell"); + eq(item?.execution?.shell, "powershell", "live path: execution.shell preserved"); + eq(item?.execution?.exitCode, 1, "live path: exitCode preserved"); + eq(item?.execution?.failurePhase, "execution", "live path: failurePhase preserved"); + + const ui = await renderCard(item!); + const name = document.querySelector(".tool__name")?.textContent ?? ""; + ok(name.includes("Windows PowerShell"), `live path: card shows Windows PowerShell (got ${JSON.stringify(name)})`); + const duration = document.querySelector(".tool__duration")?.textContent ?? ""; + ok(duration.includes("exit 1") || duration.includes("execution"), `live path: summary shows exit/phase (got ${JSON.stringify(duration)})`); + const risk = document.body.textContent ?? ""; + ok(risk.includes("partially modified") || risk.includes("部分"), "live path: partial mutation risk visible"); + // Execution failure must not claim the command never ran. + ok( + !(risk.includes("did not run") || risk.includes("command did not run") || risk.includes("命令未执行") || risk.includes("未执行")), + "live path: execution failure must not show not-run label", + ); + + await ui.expand(); + const details = document.querySelector("details.tool__error-details, .tool__error-details, details"); + ok(!!details, "live path: stderr details element present after expand affordance"); + if (details) { + await act(async () => { + details.setAttribute("open", ""); + details.dispatchEvent(new Event("toggle")); + await flushTimers(); + }); + } + const afterExpand = document.body.textContent ?? ""; + ok( + afterExpand.includes("中文") || afterExpand.includes("找不到路径"), + "live path: Chinese stderr from execution.outputTail is visible in the DOM", + ); + + await ui.cleanup(); +} + +// timed_out / cancelled must surface partial-write risk (backend may_be_partial). +{ + for (const state of ["timed_out", "cancelled"] as const) { + const item: ToolItem = { + kind: "tool", + id: `risk-${state}`, + name: "bash", + args: `{"command":"long-run"}`, + readOnly: false, + status: "error", + error: state === "timed_out" ? "command timed out" : "context canceled", + isShell: true, + execution: { + kind: "shell", + shell: "bash", + state, + failurePhase: state === "timed_out" ? "timeout" : "cancellation", + mutationRisk: "may_be_partial", + verification: "not_verification", + durationMs: 100, + }, + }; + const ui = await renderCard(item); + const body = document.body.textContent ?? ""; + ok( + body.includes("partially modified") || body.includes("部分"), + `${state}: shows partial mutation risk`, + ); + ok( + !(body.includes("did not run") || body.includes("命令未执行")), + `${state}: does not claim command never ran`, + ); + await ui.cleanup(); + } +} + +// ── Path 2: history recovery ── +{ + const messages: HistoryMessage[] = [ + { + role: "assistant", + content: "", + toolCalls: [{ id: "hist-bash", name: "bash", arguments: "{\"command\":\"go test ./...\"}" }], + }, + { + role: "tool", + content: "blocked: mixed mutation and verification", + toolCallId: "hist-bash", + toolName: "bash", + toolResultError: "blocked: mixed mutation and verification command", + execution: preflightExecution, + }, + ]; + const items = historyMessagesToItems(messages, "h").items.filter((it): it is ToolItem => it.kind === "tool"); + eq(items.length, 1, "history path: one tool item"); + eq(items[0]?.isShell, true, "history path: bash isShell"); + eq(items[0]?.execution?.failurePhase, "preflight", "history path: execution restored"); + eq(items[0]?.execution?.mutationRisk, "not_started", "history path: not_started risk"); + + const ui = await renderCard(items[0]!); + const name = document.querySelector(".tool__name")?.textContent ?? ""; + ok(name === "bash" || name.includes("bash"), `history path: shell name bash (got ${JSON.stringify(name)})`); + const body = document.body.textContent ?? ""; + ok( + body.includes("did not run") || body.includes("not run") || body.includes("未执行") || body.includes("命令未执行"), + "history path: preflight shows command-not-run (not partial)", + ); + ok( + !(body.includes("partially modified") || body.includes("部分修改文件")), + "history path: preflight must not claim partial file modification", + ); + await ui.cleanup(); +} + +// ── Path 3a: archive compaction keeps execution from the live result ── +{ + let s = reducer(initialState, { type: "event", e: { kind: "turn_started" } }); + s = reducer(s, { + type: "event", + e: { + kind: "tool_dispatch", + tool: { id: "arch-live", name: "bash", args: `{"command":"exit 1"}`, readOnly: false }, + }, + }); + s = reducer(s, { + type: "event", + e: { + kind: "tool_result", + tool: { + id: "arch-live", + name: "bash", + readOnly: false, + output: "error: exit 1\n" + "y".repeat(5000), + err: "command exited: exit status 1", + durationMs: 42, + execution: failedPSExecution, + }, + }, + }); + const item = s.items.find((it): it is ToolItem => it.kind === "tool" && it.id === "arch-live"); + ok(!!item?.dataArchived, "archive path: tool_result archives large output"); + eq(item?.output, undefined, "archive path: output dropped after archive"); + eq(item?.execution?.shell, "powershell", "archive path: execution survives compactArchivedToolItems"); + eq(item?.execution?.exitCode, 1, "archive path: exitCode survives archive"); + + const ui = await renderCard(item!); + ok((document.querySelector(".tool__name")?.textContent ?? "").includes("Windows PowerShell"), + "archive path: card still shows Windows PowerShell without re-fetch"); + ok((document.body.textContent ?? "").includes("partially modified") || (document.body.textContent ?? "").includes("部分"), + "archive path: partial risk still visible when execution retained"); + await ui.cleanup(); +} + +// ── Path 3b: ToolResultForTab rehydrates execution when only fullData has it ── +{ + const archived: ToolItem = { + kind: "tool", + id: "arch-ps", + name: "bash", + args: "", + readOnly: false, + status: "error", + error: "command exited: exit status 1", + dataArchived: true, + isShell: true, + execution: undefined, + durationMs: 42, + }; + + const dom = installDom(); + // Inject a Wails-shaped binding so bridge.realApp() picks up our stub + // instead of the browser mock (which always returns null for ToolResultForTab). + (window as unknown as { go: { main: { App: Record } } }).go = { + main: { + App: { + ToolResultForTab: async () => ({ + args: `{"command":"Get-Content .\\\\中文\\\\app.ps1"}`, + output: "error: command exited: exit status 1\nSelect-String failed", + execution: failedPSExecution, + }), + }, + }, + }; + const rootEl = document.getElementById("root"); + if (!rootEl) throw new Error("missing root"); + const root = createRoot(rootEl); + + try { + await act(async () => { + root.render( + React.createElement(LocaleProvider, null, + React.createElement(ToolCard, { item: archived, tabId: "tab-1" }), + ), + ); + await flushTimers(); + }); + const head = document.querySelector(".tool__head") as HTMLButtonElement | null; + if (!head) throw new Error("tool head missing"); + await act(async () => { + head.click(); + await flushTimers(); + await flushTimers(); + await flushTimers(); + }); + + const name = document.querySelector(".tool__name")?.textContent ?? ""; + ok(name.includes("Windows PowerShell"), `archive rehydrate: shell name after expand (got ${JSON.stringify(name)})`); + const duration = document.querySelector(".tool__duration")?.textContent ?? ""; + ok( + duration.includes("exit 1") || duration.includes("execution"), + `archive rehydrate: exit/phase after expand (got ${JSON.stringify(duration)})`, + ); + const body = document.body.textContent ?? ""; + ok(body.includes("partially modified") || body.includes("部分"), "archive rehydrate: partial risk after expand"); + } finally { + await act(async () => { + root.unmount(); + }); + delete (window as unknown as { go?: unknown }).go; + dom.window.close(); + } +} + +// ── Nil execution safety ── +{ + const plain: ToolItem = { + kind: "tool", + id: "plain", + name: "bash", + args: `{"command":"echo hi"}`, + readOnly: false, + status: "done", + output: "hi\n", + isShell: true, + }; + const ui = await renderCard(plain); + ok(document.querySelector(".tool__name")?.textContent === "bash", "nil execution falls back to bash label"); + ok(!document.querySelector("[data-shell]") || document.querySelector("[data-shell]")?.getAttribute("data-shell") === "bash", + "nil execution still renders shell card without throwing"); + await ui.cleanup(); +} + +console.log(`\n${passed} passed, ${failed} failed, ${passed + failed} total`); +if (failed > 0) process.exit(1); diff --git a/desktop/frontend/src/components/ToolCard.tsx b/desktop/frontend/src/components/ToolCard.tsx index 09a3d12f65..71ed6003b4 100644 --- a/desktop/frontend/src/components/ToolCard.tsx +++ b/desktop/frontend/src/components/ToolCard.tsx @@ -50,6 +50,76 @@ function formatToolDuration(ms?: number): string { return `${Math.round(ms)} ms`; } +function shellDisplayName(execution?: { shell?: string; shellVersion?: string }): string { + switch (execution?.shell) { + case "git-bash": + return "Git Bash"; + case "powershell": + return "Windows PowerShell"; + case "pwsh": + return "PowerShell 7+"; + case "bash": + return "bash"; + default: + return execution?.shell || "bash"; + } +} + +function shellSettledSummary( + t: Translator, + execution: NonNullable, + durationMs?: number, +): string { + const parts: string[] = []; + if (typeof execution.exitCode === "number") { + parts.push(t("tool.shell.exitCode", { code: execution.exitCode })); + } + if (execution.failurePhase) { + parts.push(execution.failurePhase); + } + const ms = execution.durationMs || durationMs; + if (typeof ms === "number" && Number.isFinite(ms) && ms >= 0) { + parts.push(formatToolDuration(ms)); + } + return parts.join(" · "); +} + +function shellVerificationLabel(t: Translator, verification?: string): string { + switch (verification) { + case "passed": + return t("tool.shell.verificationPassed"); + case "failed": + return t("tool.shell.verificationFailed"); + case "not_run": + return t("tool.shell.verificationNotRun"); + default: + return ""; + } +} + +function shellRiskLabel(t: Translator, execution?: ToolItem["execution"]): string { + if (!execution) return ""; + const phase = execution.failurePhase || ""; + // Pre-run / not-started phases never touched disk. + if (phase === "preflight" || phase === "authorization" || phase === "dependency" || phase === "launch") { + return t("tool.shell.notExecuted"); + } + // Backend marks failed, timed_out, and cancelled execution as may_be_partial + // when the process may already have written files. Show the warning for any + // such risk, not only state=failed. + if (execution.mutationRisk === "may_be_partial") { + return t("tool.shell.mayBePartial"); + } + return ""; +} + +function firstTailLine(tail?: string): string { + if (!tail) return ""; + const line = tail.replace(/\r\n/g, "\n").trim().split("\n")[0]?.trim() ?? ""; + if (line.length <= ERROR_SUMMARY_MAX_CHARS) return line; + return `${line.slice(0, ERROR_SUMMARY_MAX_CHARS - 1)}…`; +} + function formatArgChars(chars: number): string { if (chars >= 1000) return `${(chars / 1000).toFixed(1)}k`; return String(chars); @@ -146,14 +216,21 @@ export const ToolCard = memo(function ToolCard({ item, subcalls, tabId, displayN const [showErrorDetails, setShowErrorDetails] = useState(false); // Lazy-load full tool data from the backend when the card is expanded and // the in-memory copy was archived for memory efficiency. - const [fullData, setFullData] = useState<{ args: string; output?: string } | null>(null); + const [fullData, setFullData] = useState<{ args: string; output?: string; execution?: ToolItem["execution"] } | null>(null); const archivedWithoutFullData = Boolean(item.dataArchived && !fullData); const effectiveArgs = archivedWithoutFullData ? "" : fullData?.args ?? item.args; const effectiveOutput = fullData?.output ?? item.output; + const execution = fullData?.execution ?? item.execution; + const isShellCard = Boolean(item.isShell || item.name === "bash" || execution); const displayOutput = toolOutputDuplicatesError(effectiveOutput, item.error) ? undefined : effectiveOutput; const previewDiff = item.fileDiff?.diff ? item.fileDiff : undefined; const diffs = previewDiff || archivedWithoutFullData ? [] : diffsFor(item.name, effectiveArgs); const subject = fullData ? subjectOf(item.name, effectiveArgs) : item.subject || subjectOf(item.name, effectiveArgs); + const shellName = isShellCard ? shellDisplayName(execution) : (displayName ?? item.name); + const shellSummary = execution && item.status !== "running" ? shellSettledSummary(t, execution, item.durationMs) : ""; + const verificationLabel = shellVerificationLabel(t, execution?.verification); + const riskLabel = shellRiskLabel(t, execution); + const tailSummary = firstTailLine(execution?.outputTail); // Reset cached fullData when the item identity changes (e.g. after rewind). useEffect(() => { return () => setFullData(null); @@ -166,9 +243,10 @@ export const ToolCard = memo(function ToolCard({ item, subcalls, tabId, displayN const hasArgsOrOutput = !previewDiff && diffs.length === 0 && (!!effectiveArgs || !!displayOutput || hasArchivedOnDemandBody); // Shell output: split into preview + "show all" toggle. - const shellOutput = item.isShell && displayOutput ? displayOutput : null; + const shellOutput = isShellCard && displayOutput ? displayOutput : null; const shellPreview = shellOutput ? splitPreview(shellOutput, SHELL_PREVIEW_LINES) : null; - const hasBody = Boolean(previewDiff || diffs.length || hasNested || shellPreview || (!shellPreview && hasArgsOrOutput) || item.error || hasSubagentPreview); + const hasStderrDetails = Boolean(execution?.outputTail && execution.outputTail.trim()); + const hasBody = Boolean(previewDiff || diffs.length || hasNested || shellPreview || (!shellPreview && hasArgsOrOutput) || item.error || hasSubagentPreview || hasStderrDetails || riskLabel || verificationLabel); const errorText = item.error ? normalizeErrorText(item.error) : ""; const errorSummary = errorText ? summarizeToolError(errorText, t("tool.errorReceiptMismatch")) : ""; const hasErrorDetails = errorText ? errorNeedsDetails(errorText, errorSummary) : false; @@ -188,9 +266,9 @@ export const ToolCard = memo(function ToolCard({ item, subcalls, tabId, displayN // registered closure flipping the current state, not a stale one. const shellExpand = useShellExpand(); useEffect(() => { - if (!item.isShell || !shellExpand) return; + if (!isShellCard || !shellExpand) return; return shellExpand.register(item.id, () => setUserOpen(!openRef.current)); - }, [item.isShell, item.id, shellExpand]); + }, [isShellCard, item.id, shellExpand]); // Read-only "research" calls (read/grep/ls/glob/web_fetch) are hidden after // completion so they don't clutter the transcript. During execution they still @@ -198,27 +276,33 @@ export const ToolCard = memo(function ToolCard({ item, subcalls, tabId, displayN const quiet = item.readOnly && !hasNested && item.status !== "error" && item.status !== "stopped"; - const duration = item.status === "running" ? "" : formatToolDuration(item.durationMs); + const duration = item.status === "running" ? "" : (shellSummary || formatToolDuration(item.durationMs)); // While the model is still streaming this call's arguments (partial // dispatch), show the received volume as the live subject so a long // write_file body reads as progress instead of a silent stall. const streamingArgs = item.status === "running" && !item.args && (item.argChars ?? 0) > 0 ? t("tool.receivingArgs", { chars: formatArgChars(item.argChars ?? 0) }) : ""; - const summary = item.status === "running" ? streamingArgs : item.summary || summarizeFileDiff(item.fileDiff) || (item.error ? errorSummary : archivedWithoutFullData ? "" : summarize(item.name, effectiveArgs, displayOutput, item.error)); + const summary = item.status === "running" + ? streamingArgs + : (verificationLabel || item.summary || summarizeFileDiff(item.fileDiff) || (item.error ? (tailSummary || errorSummary) : archivedWithoutFullData ? "" : summarize(item.name, effectiveArgs, displayOutput, item.error))); + const a11yLabel = isShellCard + ? `${shellName} ${item.status}${shellSummary || summary ? ` ${shellSummary || summary}` : ""}` + : undefined; // GSAP-driven collapse/expand for tool body const toolBodyRef = useRef(null); useGSAPCollapse(toolBodyRef, open); return ( -
+
)} + {isShellCard && (riskLabel || verificationLabel) && ( +
+ {[riskLabel, verificationLabel].filter(Boolean).join(" · ")} +
+ )} + {shellPreview && ( <> @@ -328,6 +418,13 @@ export const ToolCard = memo(function ToolCard({ item, subcalls, tabId, displayN )} + {hasStderrDetails && ( +
+ {tailSummary || t("tool.showErrorDetails")} + +
+ )} + {!shellPreview && hasArgsOrOutput && ( <> {effectiveArgs && } diff --git a/desktop/frontend/src/lib/bridge.ts b/desktop/frontend/src/lib/bridge.ts index d86b60cf8e..eda62c8efa 100644 --- a/desktop/frontend/src/lib/bridge.ts +++ b/desktop/frontend/src/lib/bridge.ts @@ -304,7 +304,7 @@ export interface AppBindings { WorkspaceConflictForTab(tabID: string): Promise; RevealWorkspaceWriterForTab(tabID: string): Promise; CloseTabWithPolicy(tabID: string, policy: "keep_running" | "stop_and_close"): Promise; - ToolResultForTab(tabID: string, toolID: string): Promise<{ args: string; output: string } | null>; + ToolResultForTab(tabID: string, toolID: string): Promise<{ args: string; output: string; execution?: import("./types").WireShellExecution } | null>; Meta(): Promise; MetaForTab(tabID: string): Promise; AutoResearchCurrent(): Promise; diff --git a/desktop/frontend/src/lib/types.ts b/desktop/frontend/src/lib/types.ts index 0b923842b4..8bfcb7f279 100644 --- a/desktop/frontend/src/lib/types.ts +++ b/desktop/frontend/src/lib/types.ts @@ -50,6 +50,21 @@ export interface WireProfile { effort?: string; } +export interface WireShellExecution { + kind?: string; + shell?: string; + shellVersion?: string; + platform?: string; + supportsAndAnd?: boolean; + state?: string; + failurePhase?: string; + exitCode?: number; + outputTail?: string; + mutationRisk?: string; + verification?: string; + durationMs?: number; +} + export interface WireTool { id?: string; name: string; @@ -71,6 +86,7 @@ export interface WireTool { added?: number; removed?: number; profile?: WireProfile; // subagent model/effort resolved for this call + execution?: WireShellExecution; // local shell metadata; never provider-visible } export interface WireCacheDiagnostics { @@ -526,6 +542,7 @@ export interface HistoryMessage { toolName?: string; toolResultArchived?: boolean; toolResultError?: string; + execution?: WireShellExecution; pending?: boolean; trigger?: string; messages?: number; diff --git a/desktop/frontend/src/lib/useController.ts b/desktop/frontend/src/lib/useController.ts index 408e5e31ac..912d4302fa 100644 --- a/desktop/frontend/src/lib/useController.ts +++ b/desktop/frontend/src/lib/useController.ts @@ -44,6 +44,7 @@ import type { WireFinalReadiness, WireTool, WireUsage, + WireShellExecution, } from "./types"; export type ToolStatus = "running" | "done" | "error" | "stopped"; @@ -255,7 +256,8 @@ export type Item = subject?: string; // stable collapsed subject from archived history payloads summary?: string; // stable collapsed readout kept even after args/output archive fileDiff?: ToolFileDiff; // previewed whole-file diff from writer dispatch - isShell?: boolean; // true for !-prefix shell commands (controls default expand) + isShell?: boolean; // bash tool or !command — structured shell card presentation + execution?: WireShellExecution; // local shell metadata parentId?: string; // a sub-agent call nests under the `task` call with this id profile?: { model?: string; effort?: string }; // subagent model/effort from tool event argChars?: number; // args still streaming from the model: cumulative chars received @@ -863,7 +865,8 @@ export function historyMessagesToItems(messages: HistoryMessage[], idPrefix: str subject: tc.subject, summary: summarizeFileDiff(fileDiff) || tc.summary, fileDiff, - isShell: (tc.id || "").startsWith("shell-"), + isShell: tc.name === "bash" || (tc.id || "").startsWith("shell-"), + execution: result?.execution, }); seq++; } @@ -883,7 +886,8 @@ export function historyMessagesToItems(messages: HistoryMessage[], idPrefix: str output, error, dataArchived: m.toolResultArchived || undefined, - isShell: (m.toolCallId || "").startsWith("shell-"), + isShell: (m.toolName || "") === "bash" || (m.toolCallId || "").startsWith("shell-"), + execution: m.execution, }); seq++; continue; @@ -1437,7 +1441,7 @@ function applyEvent(s: State, e: WireEvent): State { } const args = t.args ?? ""; const fileDiff = fileDiffFromWire(t); - const created: ToolItem = { kind: "tool", id, name: t.name, args, readOnly: t.readOnly, resolvedName: t.resolvedName, capabilityId: t.capabilityId, status: "running", summary: summarizeFileDiff(fileDiff) || summarize(t.name, args), fileDiff, isShell: id.startsWith("shell-"), parentId: t.parentId, profile: t.profile, subagentProgress: SUBAGENT_PROGRESS_TOOLS.has(t.name) ? freshSubagentProgress() : undefined }; + const created: ToolItem = { kind: "tool", id, name: t.name, args, readOnly: t.readOnly, resolvedName: t.resolvedName, capabilityId: t.capabilityId, status: "running", summary: summarizeFileDiff(fileDiff) || summarize(t.name, args), fileDiff, isShell: t.name === "bash" || id.startsWith("shell-"), execution: t.execution, parentId: t.parentId, profile: t.profile, subagentProgress: SUBAGENT_PROGRESS_TOOLS.has(t.name) ? freshSubagentProgress() : undefined }; const items = [...s.items, created]; // A sub-agent call nested under a task card refreshes that card's // recent activity and switches its phase to "tool". @@ -1497,6 +1501,8 @@ function applyEvent(s: State, e: WireEvent): State { truncated: t.truncated, durationMs: t.durationMs, summary, + isShell: existing.isShell || existing.name === "bash" || t.name === "bash", + execution: t.execution ?? existing.execution, }; } } diff --git a/desktop/frontend/src/locales/en.ts b/desktop/frontend/src/locales/en.ts index 3fdbd5c6c3..250a359c50 100644 --- a/desktop/frontend/src/locales/en.ts +++ b/desktop/frontend/src/locales/en.ts @@ -2921,6 +2921,12 @@ export const en = { "tool.replacementOther": "{n} replacements", "tool.emptyFile": "empty file", "tool.noOutput": "no output", + "tool.shell.exitCode": "exit {code}", + "tool.shell.verificationPassed": "verification passed", + "tool.shell.verificationFailed": "verification failed", + "tool.shell.verificationNotRun": "verification not run", + "tool.shell.notExecuted": "command did not run", + "tool.shell.mayBePartial": "command may have partially modified files", "subagent.phase.queued": "queued", "subagent.phase.running": "running", "subagent.phase.reasoning": "reasoning", diff --git a/desktop/frontend/src/locales/zh-TW.ts b/desktop/frontend/src/locales/zh-TW.ts index 3ba7791b4e..c818b6991d 100644 --- a/desktop/frontend/src/locales/zh-TW.ts +++ b/desktop/frontend/src/locales/zh-TW.ts @@ -2042,6 +2042,12 @@ export const zhTW: Record = { "tool.replacementOther": "{n} 處替換", "tool.emptyFile": "空檔案", "tool.noOutput": "無輸出", + "tool.shell.exitCode": "結束代碼 {code}", + "tool.shell.verificationPassed": "驗證通過", + "tool.shell.verificationFailed": "驗證失敗", + "tool.shell.verificationNotRun": "驗證未執行", + "tool.shell.notExecuted": "命令未執行", + "tool.shell.mayBePartial": "可能已部分修改檔案", "subagent.phase.queued": "排隊中", "subagent.phase.running": "執行中", "subagent.phase.reasoning": "思考中", diff --git a/desktop/frontend/src/locales/zh.ts b/desktop/frontend/src/locales/zh.ts index 0dae297951..8773b400b8 100644 --- a/desktop/frontend/src/locales/zh.ts +++ b/desktop/frontend/src/locales/zh.ts @@ -2924,6 +2924,12 @@ export const zh: Record = { "tool.replacementOther": "{n} 处替换", "tool.emptyFile": "空文件", "tool.noOutput": "无输出", + "tool.shell.exitCode": "退出码 {code}", + "tool.shell.verificationPassed": "验证通过", + "tool.shell.verificationFailed": "验证失败", + "tool.shell.verificationNotRun": "验证未执行", + "tool.shell.notExecuted": "命令未执行", + "tool.shell.mayBePartial": "可能已部分修改文件", "subagent.phase.queued": "排队中", "subagent.phase.running": "运行中", "subagent.phase.reasoning": "思考中", diff --git a/internal/acp/clientio.go b/internal/acp/clientio.go index 3fb3cc0d46..5d35840d30 100644 --- a/internal/acp/clientio.go +++ b/internal/acp/clientio.go @@ -7,6 +7,8 @@ import ( "sort" "strings" "time" + + "reasonix/internal/tool/builtin" ) // requester is the slice of Conn that clientIO drives: agent → client requests. @@ -136,11 +138,14 @@ func (c *clientIO) RunCommand(ctx context.Context, command, cwd string, timeout case ctx.Err() != nil: return output, true, ctx.Err() case timedOut: - return output, true, fmt.Errorf("command timed out after %s (terminal killed)", timeout) + // Typed timeout so bash.ExecuteDetailed can set state=timed_out / + // failurePhase=timeout instead of a generic failed/execution. + return output, true, builtin.TerminalTimeoutError{Timeout: timeout} case waitErr != nil: return output, true, waitErr case exit != nil && exit.ExitCode != nil && *exit.ExitCode != 0: - return output, true, fmt.Errorf("exit status %d", *exit.ExitCode) + // Preserve the real exit code on ShellExecution via TerminalExitError. + return output, true, builtin.TerminalExitError{Code: *exit.ExitCode} case exit != nil && exit.Signal != nil && *exit.Signal != "": return output, true, fmt.Errorf("terminated by signal %s", *exit.Signal) } diff --git a/internal/agent/agent.go b/internal/agent/agent.go index 9614f12b47..ccacc0a514 100644 --- a/internal/agent/agent.go +++ b/internal/agent/agent.go @@ -25,6 +25,7 @@ import ( "reasonix/internal/jobs" "reasonix/internal/memory" "reasonix/internal/nilutil" + "reasonix/internal/permission" "reasonix/internal/planmode" "reasonix/internal/provider" "reasonix/internal/sandbox" @@ -334,6 +335,13 @@ type Agent struct { // for the agent's lifetime and validates proxy calls after resolution. readOnlyExecution bool + // mutationDependencyBarrier is set for the remainder of a provider tool + // batch after any mutating call fails or is blocked. executeOne re-checks + // it after proxy resolution so use_capability cannot bypass the barrier by + // advertising schema-level ReadOnly()==true. Parallel read-only segments + // never set it. Cleared at the start of each executeBatch. + mutationDependencyBarrier atomic.Bool + // plannerMCPExecution relaxes the strict read-only MCP boundary for the // two-model Planner only: authorized, non-destructive MCP targets may run // through use_capability even without readOnlyHint. Ordinary writers, bash, @@ -3275,6 +3283,7 @@ func (a *Agent) systemPrompt() string { type batchExecution struct { results []string images [][]string + executions []*tool.ShellExecution recoveryStopTurn bool recoveryStopReason string } @@ -3398,6 +3407,63 @@ func (a *Agent) executeBatch(ctx context.Context, calls []provider.ToolCall) bat } } + // mutationBatchStop is the deterministic dependency barrier: after any + // mutating call fails or is blocked, later mutating and verification calls + // in the same provider batch are skipped (not_run/dependency). Host-proven + // read-only diagnosis may still run. executeOne also re-checks after proxy + // resolution so use_capability cannot bypass this pass. + mutationBatchStop := false + a.mutationDependencyBarrier.Store(false) + markDependencySkipped := func(start int) { + a.mutationDependencyBarrier.Store(true) + for j := start; j < len(calls); j++ { + if results[j] != "" { + continue + } + // Pre-classify when statically certain. Proxies and ambiguous + // targets fall through to run() so executeOne can resolve the real + // target and re-apply the barrier before Commit/Execute. + if !batchCallStaticallySkippable(a, calls[j]) { + continue + } + isVerification := calls[j].Name == "bash" && evidence.IsDeliveryVerificationCommand(bashCommandFromArgs(json.RawMessage(calls[j].Arguments))) + msg := "blocked: skipped because an earlier modification in this tool batch failed or was blocked. " + + "Fix or re-run the failed change first; verification was not executed." + var ex *tool.ShellExecution + if calls[j].Name == "bash" { + ex = &tool.ShellExecution{ + Kind: "shell", + State: tool.ShellStateNotRun, + FailurePhase: tool.ShellPhaseDependency, + MutationRisk: tool.ShellMutationNotStarted, + Verification: tool.ShellVerificationNotVerification, + } + if isVerification { + ex.Verification = tool.ShellVerificationNotRun + } + if t, _, amb := a.tools.ResolveCall(calls[j].Name); t != nil && len(amb) == 0 { + if bt, ok := t.(tool.DetailedExecutor); ok { + if desc := bt.ExecutionDescriptor(json.RawMessage(calls[j].Arguments)); desc != nil { + ex.Shell = desc.Shell + ex.ShellVersion = desc.ShellVersion + ex.Platform = desc.Platform + ex.SupportsAndAnd = desc.SupportsAndAnd + } + } + } + } + results[j] = msg + outcomes[j] = toolOutcome{ + output: msg, + blocked: true, + errMsg: firstLine(msg), + execution: ex, + } + durations[j] = 0 + } + mutationBatchStop = true + } + for _, batch := range partitionToolCalls(a.tools, calls) { if ctx.Err() != nil { markCancelled(batch.start) @@ -3408,6 +3474,7 @@ func (a *Agent) executeBatch(ctx context.Context, calls []provider.ToolCall) bat break } if batch.parallel && batch.end-batch.start > 1 { + // Parallel segments are read-only by construction; no mutation barrier. ranUntil := runParallel(ctx, batch.start, batch.end, run) for i := batch.start; i < ranUntil; i++ { finalize(i) @@ -3444,6 +3511,33 @@ func (a *Agent) executeBatch(ctx context.Context, calls []provider.ToolCall) bat markRecoveryStopped(i, recoveryStopReason) break } + if mutationBatchStop { + // Fill dependency skips for remaining mutating/verify calls, then + // allow any residual read-only diagnosis to run individually. + if results[i] != "" { + continue + } + t, _, ambiguous := a.tools.ResolveCall(calls[i].Name) + known := t != nil && len(ambiguous) == 0 + readOnly := known && t.ReadOnly() + if calls[i].Name == "bash" && permission.BashCommandIsReadOnly(json.RawMessage(calls[i].Arguments)) { + readOnly = true + } + isVerification := calls[i].Name == "bash" && evidence.IsDeliveryVerificationCommand(bashCommandFromArgs(json.RawMessage(calls[i].Arguments))) + mutates := evidence.ToolCallMutates(calls[i].Name, json.RawMessage(calls[i].Arguments), readOnly) + if mutates || isVerification { + markDependencySkipped(i) + // markDependencySkipped fills this index; move on. + if results[i] != "" { + continue + } + } + } + if results[i] != "" { + // Pre-filled dependency skip. + finalize(i) + continue + } run(i) finalize(i) if outcomes[i].recoveryStopTurn { @@ -3452,6 +3546,11 @@ func (a *Agent) executeBatch(ctx context.Context, calls []provider.ToolCall) bat markRecoveryStopped(i+1, recoveryStopReason) break } + // Mutation/verification failure barrier for the rest of this batch. + if batchCallIsMutatingFailure(a, calls[i], outcomes[i]) { + mutationBatchStop = true + markDependencySkipped(i + 1) + } // After each tool execution, also check if the context was cancelled. // If so, stop executing remaining tools and return immediately so // the agent loop can detect the cancellation and exit. @@ -3473,7 +3572,7 @@ func (a *Agent) executeBatch(ctx context.Context, calls []provider.ToolCall) bat if c.ResolvedReadOnly != nil { readOnly = *c.ResolvedReadOnly } - a.sink.Emit(event.Event{Kind: event.ToolResult, Tool: event.Tool{ + tr := event.Tool{ ID: c.ID, Name: c.Name, Args: c.Arguments, @@ -3484,7 +3583,9 @@ func (a *Agent) executeBatch(ctx context.Context, calls []provider.ToolCall) bat ReadOnly: readOnly, Truncated: o.truncated, DurationMs: durations[i], - }}) + Execution: toEventShellExecution(o.execution, durations[i]), + } + a.sink.Emit(event.Event{Kind: event.ToolResult, Tool: tr}) if o.truncated && o.truncMsg != "" { a.sink.Emit(event.Event{Kind: event.Notice, Level: event.LevelInfo, Text: o.truncMsg}) } @@ -3493,8 +3594,10 @@ func (a *Agent) executeBatch(ctx context.Context, calls []provider.ToolCall) bat a.applyStormBreaker(calls, outcomes, results, receiptMark) } images := make([][]string, len(calls)) + executions := make([]*tool.ShellExecution, len(calls)) for i := range outcomes { images[i] = outcomes[i].images + executions[i] = outcomes[i].execution if outcomes[i].recoveryStopTurn { recoveryBatchStop = true if outcomes[i].recoveryStopReason != "" { @@ -3505,11 +3608,134 @@ func (a *Agent) executeBatch(ctx context.Context, calls []provider.ToolCall) bat return batchExecution{ results: results, images: images, + executions: executions, recoveryStopTurn: recoveryBatchStop, recoveryStopReason: recoveryStopReason, } } +func toEventShellExecution(in *tool.ShellExecution, durationMs int64) *event.ShellExecution { + if in == nil { + return nil + } + out := &event.ShellExecution{ + Kind: in.Kind, + Shell: in.Shell, + ShellVersion: in.ShellVersion, + Platform: in.Platform, + SupportsAndAnd: in.SupportsAndAnd, + State: in.State, + FailurePhase: in.FailurePhase, + OutputTail: in.OutputTail, + MutationRisk: in.MutationRisk, + Verification: in.Verification, + DurationMs: in.DurationMs, + } + if out.DurationMs == 0 && durationMs > 0 { + out.DurationMs = durationMs + } + if in.ExitCode != nil { + code := *in.ExitCode + out.ExitCode = &code + } + return out +} + +func toProviderToolExecution(in *tool.ShellExecution) *provider.ToolExecution { + if in == nil { + return nil + } + out := &provider.ToolExecution{ + Kind: in.Kind, + Shell: in.Shell, + ShellVersion: in.ShellVersion, + Platform: in.Platform, + SupportsAndAnd: in.SupportsAndAnd, + State: in.State, + FailurePhase: in.FailurePhase, + OutputTail: in.OutputTail, + MutationRisk: in.MutationRisk, + Verification: in.Verification, + DurationMs: in.DurationMs, + } + if in.ExitCode != nil { + code := *in.ExitCode + out.ExitCode = &code + } + return out +} + +// batchCallIsMutatingFailure reports whether a finished call was a mutation +// (file write / non-readonly bash mutation) that failed or was blocked, so later +// mutations and verifications in the same batch must not run. +func batchCallIsMutatingFailure(a *Agent, call provider.ToolCall, o toolOutcome) bool { + if o.errMsg == "" && !o.blocked { + return false + } + readOnly := false + t, _, ambiguous := a.tools.ResolveCall(call.Name) + known := t != nil && len(ambiguous) == 0 + if known { + readOnly = t.ReadOnly() + } + if call.ResolvedReadOnly != nil { + readOnly = *call.ResolvedReadOnly + } + if o.resolved { + readOnly = o.resolvedReadOnly + } + if call.Name == "bash" && permission.BashCommandIsReadOnly(json.RawMessage(call.Arguments)) { + readOnly = true + } + // Verification failures do not open the dependency barrier by themselves — + // only a failed modification does. + if call.Name == "bash" && evidence.IsDeliveryVerificationCommand(bashCommandFromArgs(json.RawMessage(call.Arguments))) { + return false + } + // Resolved writers (including MCP targets behind use_capability) count even + // when the provider-visible proxy advertised ReadOnly. + if o.resolved && !o.resolvedReadOnly { + return true + } + if evidence.ToolCallMutates(call.Name, json.RawMessage(call.Arguments), readOnly) { + return true + } + // Fail closed only for a target the host could not classify at all. A blanket + // !readOnly fallback here would re-admit exactly the writers ToolCallMutates + // deliberately exempts (todo_write, complete_step, ask, bash_output, wait and + // the other non-mutation meta tools): a failed todo update would then block + // every real edit left in the batch. Resolved writer proxies already returned + // true above, so narrowing this does not reopen the use_capability path. + return !known +} + +// batchCallStaticallySkippable reports whether a remaining call can be marked +// not_run/dependency without resolving a proxy. Proxies and unknown tools +// return false so executeOne can resolve the real target first. +func batchCallStaticallySkippable(a *Agent, call provider.ToolCall) bool { + t, _, ambiguous := a.tools.ResolveCall(call.Name) + if t == nil || len(ambiguous) > 0 { + // Unknown / ambiguous: fail closed via executeOne path. + return false + } + // Proxy resolution may consult a live connected capability and its result + // can change between calls. Do not resolve here merely to pre-fill a skip: + // executeOne resolves exactly once, then applyMutationDependencyBarrier + // classifies the real target before Commit or Execute. + if _, ok := t.(tool.CallResolver); ok { + return false + } + readOnly := t.ReadOnly() + if call.Name == "bash" && permission.BashCommandIsReadOnly(json.RawMessage(call.Arguments)) { + readOnly = true + } + isVerification := call.Name == "bash" && evidence.IsDeliveryVerificationCommand(bashCommandFromArgs(json.RawMessage(call.Arguments))) + if isVerification { + return true + } + return !readOnly || evidence.ToolCallMutates(call.Name, json.RawMessage(call.Arguments), readOnly) +} + func (a *Agent) emitFullToolDispatch(c provider.ToolCall, refreshed bool) { t, _, ambiguous := a.tools.ResolveCall(c.Name) ok := t != nil && len(ambiguous) == 0 @@ -3848,6 +4074,9 @@ type toolOutcome struct { resolvedName string capabilityID string resolvedReadOnly bool + // execution is local shell metadata (optional). Provider messages strip it + // via ModelMessages; UI/event sinks surface it on ToolResult cards. + execution *tool.ShellExecution // recoveryGeneration is the gate generation captured before execution so // ObserveResult can ignore stale results after a mode switch. recoveryGeneration uint64 diff --git a/internal/agent/dependent_writer_preview_test.go b/internal/agent/dependent_writer_preview_test.go index 5f06a9f00d..e444d5425b 100644 --- a/internal/agent/dependent_writer_preview_test.go +++ b/internal/agent/dependent_writer_preview_test.go @@ -109,7 +109,11 @@ func TestDependentSameBatchEditRefreshesPreviewBeforeExecution(t *testing.T) { } } -func TestDependentPreviewRefreshesAfterFailedWriterSideEffect(t *testing.T) { +func TestDependentMutationSkippedAfterFailedWriterInBatch(t *testing.T) { + // Shell execution contract: after any mutating call fails or is blocked, + // later mutations (and verifications) in the same provider batch are not + // executed. The first tool may still have written to disk; the second must + // return not_run/dependency rather than apply a follow-up edit. dir := t.TempDir() path := filepath.Join(dir, "task.txt") if err := os.WriteFile(path, []byte("status=\"draft\"\n"), 0o600); err != nil { @@ -128,13 +132,7 @@ func TestDependentPreviewRefreshesAfterFailedWriterSideEffect(t *testing.T) { }, {{Type: provider.ChunkText, Text: "done"}, {Type: provider.ChunkDone}}, }} - var refreshed bool - a := New(prov, reg, NewSession(""), Options{}, event.FuncSink(func(e event.Event) { - if e.Kind == event.ToolDispatch && e.Tool.ID == "c2" && e.Tool.Refreshed && - strings.Contains(e.Tool.Diff, `-status="ready"`) && strings.Contains(e.Tool.Diff, `+status="done"`) { - refreshed = true - } - })) + a := New(prov, reg, NewSession(""), Options{}, event.Discard) if err := a.Run(context.Background(), "run dependent edit after a partial failure"); err != nil { t.Fatal(err) } @@ -142,10 +140,11 @@ func TestDependentPreviewRefreshesAfterFailedWriterSideEffect(t *testing.T) { if err != nil { t.Fatal(err) } - if string(data) != "status=\"done\"\n" { - t.Fatalf("final file = %q", data) + // First tool wrote "ready" then failed; second edit must not run. + if string(data) != "status=\"ready\"\n" { + t.Fatalf("final file = %q, want partial first write preserved", data) } - if !refreshed { - t.Fatal("dependent preview was not refreshed after a failed writer changed disk") + if got := toolResultByID(a.session, "c2"); !strings.Contains(got, "earlier modification") { + t.Fatalf("second edit result = %q, want dependency skip", got) } } diff --git a/internal/agent/execute_one.go b/internal/agent/execute_one.go index c9e6f5ca81..0ca218a779 100644 --- a/internal/agent/execute_one.go +++ b/internal/agent/execute_one.go @@ -173,12 +173,57 @@ func (a *Agent) resolveToolPolicy(ctx context.Context, plan *toolCallPlan) (tool if blocked, early := a.applyDeliveryPolicyGates(plan); early { return blocked, true } + // After proxy resolution, re-apply the batch mutation barrier using the + // real target classification. Provider-visible proxies such as + // use_capability advertise ReadOnly()==true before resolution and would + // otherwise slip past the pre-run skip pass. + if blocked, early := a.applyMutationDependencyBarrier(plan); early { + return blocked, true + } if blocked, early := a.applyRecoveryAndPermission(ctx, plan); early { return blocked, true } return toolOutcome{}, false } +// applyMutationDependencyBarrier blocks later mutations and verifications in the +// same provider batch after an earlier modification failed. Host-proven +// read-only diagnosis (resolved ReadOnly with no verification classification) +// still runs. +func (a *Agent) applyMutationDependencyBarrier(plan *toolCallPlan) (toolOutcome, bool) { + if a == nil || plan == nil || !a.mutationDependencyBarrier.Load() { + return toolOutcome{}, false + } + verification := plan.evidenceName == "bash" && evidence.IsDeliveryVerificationCommand(bashCommandFromArgs(plan.evidenceArgs)) + // Prefer the post-gate mutates flag; fall back to !readOnly so a resolved + // writer proxy cannot claim non-mutation by skipping ToolCallMutates. + mutates := plan.mutates || !plan.readOnly + if !mutates && !verification { + return toolOutcome{}, false + } + msg := "blocked: skipped because an earlier modification in this tool batch failed or was blocked. " + + "Fix or re-run the failed change first; verification was not executed." + var ex *tool.ShellExecution + // Structured shell metadata only for bash cards; other tools keep plain text. + if plan.evidenceName == "bash" || plan.call.Name == "bash" { + ex = shellPreflightExecution(plan, verification) + if ex != nil { + ex.FailurePhase = tool.ShellPhaseDependency + ex.State = tool.ShellStateNotRun + ex.MutationRisk = tool.ShellMutationNotStarted + if verification { + ex.Verification = tool.ShellVerificationNotRun + } + } + } + return toolOutcome{ + output: msg, + blocked: true, + errMsg: firstLine(msg), + execution: ex, + }, true +} + // applyPlanModeAndProxy handles initial Plan mode, proxy resolution / skip path, // resolved-target Plan re-check, and MCP Plan availability. func (a *Agent) applyPlanModeAndProxy(ctx context.Context, plan *toolCallPlan) (toolOutcome, bool) { @@ -298,28 +343,67 @@ func (a *Agent) applyPlanModeAndProxy(ctx context.Context, plan *toolCallPlan) ( return toolOutcome{}, false } -// applyDeliveryPolicyGates enforces delivery-profile bash and criteria rules and -// classifies whether the call mutates workspace state. +// applyDeliveryPolicyGates enforces global deterministic shell contracts plus +// delivery-profile-only criteria rules, and classifies mutation/verification. func (a *Agent) applyDeliveryPolicyGates(plan *toolCallPlan) (toolOutcome, bool) { - if a.deliveryProfile && plan.evidenceName == "bash" && evidence.BashToolCallMasksVerificationExit(plan.evidenceArgs) { - return toolOutcome{ - output: "blocked: the trailing echo/printf of $? masks the verifier's exit status, so this command would look successful even when the check failed. Run the verifier or read-only extraction pipeline by itself and let its exit status be the tool result; for example: tail ... | head ... | node --check -", - blocked: true, - errMsg: "blocked: verification exit status masked", - }, true - } - if a.deliveryProfile && plan.evidenceName == "bash" && evidence.BashToolCallMixesMutationAndVerification(plan.evidenceArgs) { - return toolOutcome{ - output: "blocked: this command mixes a verification check with a segment that may write state. Run the state-changing preparation separately while a todo is in_progress, then run a read-only verification command. For generated input, prefer a host-recognized read-only pipeline into the verifier (for example: tail ... | head ... | node --check -) instead of writing a temporary file.", - blocked: true, - errMsg: "blocked: mixed mutation and verification command", - }, true + // Global deterministic shell contract (ordinary + Delivery). PowerShell 5.1 + // &&/|| is enforced inside the bash tool itself so descriptor and error text + // stay shell-accurate; the agent layers apply command-shape protections. + // Delivery keeps its longer recovery copy so existing delivery guidance tests + // and model recovery prompts stay stable. + // + // Ordinary mode blocks only shapes where a later segment can actually hide an + // earlier failure. Delivery keeps the broader classifier because a mutation + // invalidates the verification receipt even when the exit status is honest. + // Without that split, `go build ./... && go test ./...`, `npm install && + // npm test`, and every other short-circuit chain would be rejected for every + // user, though bash already reports the failing step's status for them. + if plan.evidenceName == "bash" { + if evidence.BashToolCallMasksVerificationExit(plan.evidenceArgs) { + msg := evidence.ShellContractPreflightMessage("mask_exit") + if a.deliveryProfile { + msg = "blocked: the trailing echo/printf of $? masks the verifier's exit status, so this command would look successful even when the check failed. Run the verifier or read-only extraction pipeline by itself and let its exit status be the tool result; for example: tail ... | head ... | node --check -" + } + return toolOutcome{ + output: msg, + blocked: true, + errMsg: "blocked: verification exit status masked", + execution: shellPreflightExecution(plan, true), + }, true + } + mixed := evidence.BashToolCallMixesMutationAndMaskableVerification + if a.deliveryProfile { + mixed = evidence.BashToolCallMixesMutationAndVerification + } + if mixed(plan.evidenceArgs) { + msg := evidence.ShellContractPreflightMessage("mixed") + if a.deliveryProfile { + msg = "blocked: this command mixes a verification check with a segment that may write state. Run the state-changing preparation separately while a todo is in_progress, then run a read-only verification command. For generated input, prefer a host-recognized read-only pipeline into the verifier (for example: tail ... | head ... | node --check -) instead of writing a temporary file." + } + return toolOutcome{ + output: msg, + blocked: true, + errMsg: "blocked: mixed mutation and verification command", + execution: shellPreflightExecution(plan, true), + }, true + } + if evidence.BashToolCallUsesNonTerminalInlineInterpreter(plan.evidenceArgs) { + msg := evidence.ShellContractPreflightMessage("inline_nonterminal") + return toolOutcome{ + output: msg, + blocked: true, + errMsg: "blocked: non-terminal inline interpreter command", + execution: shellPreflightExecution(plan, false), + }, true + } } + // Delivery-only: any opaque inline interpreter is unauditable as evidence. if a.deliveryProfile && plan.evidenceName == "bash" && evidence.BashToolCallUsesOpaqueInlineInterpreter(plan.evidenceArgs) { return toolOutcome{ - output: "blocked: delivery mode cannot audit inline interpreter source such as node -e or python -c, so executing it would become an opaque mutation and invalidate prior verification. For inspection, use read_file/grep or another host-proven read-only command. For validation, use a conventional verifier such as node --check, a project test/check/lint command, or a read-only extraction pipeline into the verifier. For an intentional state change, use a file tool or a script file under the current in_progress todo. " + evidence.VerificationCommandSummary(), - blocked: true, - errMsg: "blocked: opaque inline interpreter command", + output: "blocked: delivery mode cannot audit inline interpreter source such as node -e or python -c, so executing it would become an opaque mutation and invalidate prior verification. For inspection, use read_file/grep or another host-proven read-only command. For validation, use a conventional verifier such as node --check, a project test/check/lint command, or a read-only extraction pipeline into the verifier. For an intentional state change, use a file tool or a script file under the current in_progress todo. " + evidence.VerificationCommandSummary(), + blocked: true, + errMsg: "blocked: opaque inline interpreter command", + execution: shellPreflightExecution(plan, false), }, true } @@ -641,7 +725,29 @@ func (a *Agent) finishToolExecution(ctx context.Context, plan *toolCallPlan) too if a.plannerMCPExecution && isMCPExecutionTarget(runTool, permName) && mcpServerAuthorized(runTool) && !mcpDestructiveHint(runTool) { cctx = tool.WithNonDestructiveMCPExecutionIntent(cctx) } - if it, ok := runTool.(tool.ImageTool); ok { + var execution *tool.ShellExecution + if de, ok := runTool.(tool.DetailedExecutor); ok { + var detailed tool.DetailedResult + detailed, err = de.ExecuteDetailed(cctx, runArgs) + result, images, execution = detailed.Output, detailed.Images, detailed.Execution + // Annotate verification outcome when the host classified this call as a verifier. + if execution != nil && plan.verification { + switch { + case err != nil: + execution.Verification = tool.ShellVerificationFailed + default: + execution.Verification = tool.ShellVerificationPassed + } + } else if execution != nil && execution.Verification == "" { + execution.Verification = tool.ShellVerificationNotVerification + } + // Sole opaque inline interpreters are allowed outside Delivery but cannot + // prove mutation completeness. + if execution != nil && evidence.BashCommandMayBeOpaqueMutation(runArgs) && + execution.MutationRisk == tool.ShellMutationMayHaveCompleted { + execution.MutationRisk = tool.ShellMutationUnknown + } + } else if it, ok := runTool.(tool.ImageTool); ok { result, images, err = it.ExecuteWithImages(cctx, runArgs) } else { result, err = runTool.Execute(cctx, runArgs) @@ -706,7 +812,10 @@ func (a *Agent) finishToolExecution(ctx context.Context, plan *toolCallPlan) too } a.recordRepeatFailure(call, t, err) body, truncMsg := truncateToolOutput(fmt.Sprintf("error: %v\n%s", err, detail)) - return toolOutcome{output: body, errMsg: firstLine(err.Error()), truncated: truncMsg != "", truncMsg: truncMsg, recoveryGeneration: recoveryGen} + return toolOutcome{ + output: body, errMsg: firstLine(err.Error()), truncated: truncMsg != "", truncMsg: truncMsg, + execution: execution, recoveryGeneration: recoveryGen, + } } if mutates { a.clearRepeatFailuresAfterMutation(evidenceName, evidenceArgs, readOnly) @@ -719,7 +828,35 @@ func (a *Agent) finishToolExecution(ctx context.Context, plan *toolCallPlan) too a.hooks.SubagentStop(ctx, result) } body, truncMsg := truncateToolOutput(result) - return toolOutcome{output: body, images: images, truncated: truncMsg != "", truncMsg: truncMsg, recoveryGeneration: recoveryGen} + return toolOutcome{ + output: body, images: images, truncated: truncMsg != "", truncMsg: truncMsg, + execution: execution, recoveryGeneration: recoveryGen, + } +} + +// shellPreflightExecution builds not_run/preflight metadata for a blocked bash call. +func shellPreflightExecution(plan *toolCallPlan, hasVerification bool) *tool.ShellExecution { + ex := &tool.ShellExecution{ + Kind: "shell", + State: tool.ShellStateNotRun, + FailurePhase: tool.ShellPhasePreflight, + MutationRisk: tool.ShellMutationNotStarted, + Verification: tool.ShellVerificationNotVerification, + } + if hasVerification { + ex.Verification = tool.ShellVerificationNotRun + } + if plan != nil { + if de, ok := plan.execTool.(tool.DetailedExecutor); ok { + if desc := de.ExecutionDescriptor(plan.execArgs); desc != nil { + ex.Shell = desc.Shell + ex.ShellVersion = desc.ShellVersion + ex.Platform = desc.Platform + ex.SupportsAndAnd = desc.SupportsAndAnd + } + } + } + return ex } // observeBeforeMutation captures preimages for Previewable writers and records diff --git a/internal/agent/run_loop.go b/internal/agent/run_loop.go index 82593f167b..2978ef1c86 100644 --- a/internal/agent/run_loop.go +++ b/internal/agent/run_loop.go @@ -958,13 +958,17 @@ func (a *Agent) handleToolRound(ctx context.Context, state *runLoopState, step i batch := a.executeBatch(ctx, calls) results, images := batch.results, batch.images for i, call := range calls { - a.session.Add(provider.Message{ + msg := provider.Message{ Role: provider.RoleTool, Content: results[i], Images: images[i], ToolCallID: call.ID, Name: call.Name, - }) + } + if i < len(batch.executions) { + msg.ToolExecution = toProviderToolExecution(batch.executions[i]) + } + a.session.Add(msg) } // If the context was cancelled during tool execution, return after storing // the batch results so the session keeps paired tool-call history. diff --git a/internal/agent/shell_contract_test.go b/internal/agent/shell_contract_test.go new file mode 100644 index 0000000000..ac1aacc293 --- /dev/null +++ b/internal/agent/shell_contract_test.go @@ -0,0 +1,418 @@ +package agent + +import ( + "context" + "encoding/json" + "fmt" + "os" + "path/filepath" + "strings" + "testing" + + "reasonix/internal/event" + "reasonix/internal/provider" + "reasonix/internal/tool" + "reasonix/internal/tool/builtin" +) + +func TestOrdinaryModeBlocksMixedMutationAndVerification(t *testing.T) { + // Preflight runs before Execute, so a fake bash is enough — the process + // must never start for a mixed mutation+verification command. `;` is the + // shape that matters: the verifier's exit status replaces go generate's. + reg := tool.NewRegistry() + reg.Add(fakeTool{name: "bash", readOnly: false}) + prov := &scriptedProvider{name: "p", turns: [][]provider.Chunk{ + {toolCallChunk("m1", "bash", `{"command":"go generate ./... ; go test ./..."}`), {Type: provider.ChunkDone}}, + {{Type: provider.ChunkText, Text: "ok"}, {Type: provider.ChunkDone}}, + }} + a := New(prov, reg, NewSession(""), Options{}, event.Discard) + if err := a.Run(context.Background(), "test"); err != nil { + t.Fatal(err) + } + got := toolResultByID(a.session, "m1") + if strings.Contains(got, "bash done") { + t.Fatal("mixed command was executed") + } + if !strings.Contains(got, "state-changing segment") { + t.Fatalf("result = %q, want ordinary-mode mixed block", got) + } + for _, msg := range a.session.Snapshot() { + if msg.ToolCallID != "m1" { + continue + } + if msg.ToolExecution == nil || msg.ToolExecution.State != tool.ShellStateNotRun { + t.Fatalf("execution = %+v, want not_run", msg.ToolExecution) + } + if msg.ToolExecution.FailurePhase != tool.ShellPhasePreflight { + t.Fatalf("phase = %q", msg.ToolExecution.FailurePhase) + } + return + } + t.Fatal("tool result missing") +} + +// TestOrdinaryModeRunsShortCircuitBuildAndVerify guards the everyday shape the +// preflight must not touch. `go build ./... && go test ./...` cannot report a +// false success: bash stops at the failing build and returns its status. Only +// Delivery blocks it, because there a mutation invalidates the verification +// receipt regardless of exit status. +func TestOrdinaryModeRunsShortCircuitBuildAndVerify(t *testing.T) { + commands := []string{ + "go build ./... && go test ./...", + "npm install && npm test", + "mkdir -p out && go test ./...", + } + for _, command := range commands { + t.Run(command, func(t *testing.T) { + reg := tool.NewRegistry() + reg.Add(fakeTool{name: "bash", readOnly: false}) + args, err := json.Marshal(map[string]string{"command": command}) + if err != nil { + t.Fatal(err) + } + prov := &scriptedProvider{name: "p", turns: [][]provider.Chunk{ + {toolCallChunk("m1", "bash", string(args)), {Type: provider.ChunkDone}}, + {{Type: provider.ChunkText, Text: "ok"}, {Type: provider.ChunkDone}}, + }} + a := New(prov, reg, NewSession(""), Options{}, event.Discard) + if err := a.Run(context.Background(), "test"); err != nil { + t.Fatal(err) + } + got := toolResultByID(a.session, "m1") + if strings.Contains(got, "blocked:") { + t.Fatalf("ordinary mode blocked %q: %s", command, got) + } + if !strings.Contains(got, "bash done") { + t.Fatalf("command did not run: result = %q", got) + } + }) + } +} + +func TestOrdinaryModeBlocksMaskedVerifierExit(t *testing.T) { + reg := tool.NewRegistry() + reg.Add(fakeTool{name: "bash", readOnly: false}) + prov := &scriptedProvider{name: "p", turns: [][]provider.Chunk{ + {toolCallChunk("m1", "bash", `{"command":"go test ./...; echo $?"}`), {Type: provider.ChunkDone}}, + {{Type: provider.ChunkText, Text: "ok"}, {Type: provider.ChunkDone}}, + }} + a := New(prov, reg, NewSession(""), Options{}, event.Discard) + if err := a.Run(context.Background(), "test"); err != nil { + t.Fatal(err) + } + got := toolResultByID(a.session, "m1") + if strings.Contains(got, "bash done") { + t.Fatal("masked exit command was executed") + } + if !strings.Contains(got, "masks") && !strings.Contains(got, "exit status") { + t.Fatalf("result = %q, want mask block", got) + } +} + +func TestOrdinaryModeBlocksNonTerminalInlineInterpreter(t *testing.T) { + reg := tool.NewRegistry() + reg.Add(fakeTool{name: "bash", readOnly: false}) + prov := &scriptedProvider{name: "p", turns: [][]provider.Chunk{ + {toolCallChunk("m1", "bash", `{"command":"python3 -c 'open(\"x\",\"w\").write(\"y\")' ; node verify_frontend_logic.js"}`), {Type: provider.ChunkDone}}, + // A `&&` variant of the same pair is covered by the allow-list test above. + {{Type: provider.ChunkText, Text: "ok"}, {Type: provider.ChunkDone}}, + }} + a := New(prov, reg, NewSession(""), Options{}, event.Discard) + if err := a.Run(context.Background(), "test"); err != nil { + t.Fatal(err) + } + got := toolResultByID(a.session, "m1") + if strings.Contains(got, "bash done") { + t.Fatal("non-terminal inline interpreter was executed") + } + if !strings.Contains(got, "inline interpreter") { + t.Fatalf("result = %q, want non-terminal inline block", got) + } +} + +func TestBatchDependencyBarrierSkipsVerificationAfterFailedMutation(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "x.txt") + if err := os.WriteFile(path, []byte("a\n"), 0o600); err != nil { + t.Fatal(err) + } + reg := tool.NewRegistry() + for _, tl := range (builtin.Workspace{Dir: dir}).Tools("edit_file") { + reg.Add(tl) + } + // Verification would return "bash done" if it ran — the barrier must prevent that. + reg.Add(fakeTool{name: "bash", readOnly: false}) + prov := &scriptedProvider{name: "p", turns: [][]provider.Chunk{ + { + toolCallChunk("e1", "edit_file", `{"path":"x.txt","old_string":"missing","new_string":"b"}`), + toolCallChunk("v1", "bash", `{"command":"go test ./..."}`), + {Type: provider.ChunkDone}, + }, + {{Type: provider.ChunkText, Text: "done"}, {Type: provider.ChunkDone}}, + }} + a := New(prov, reg, NewSession(""), Options{}, event.Discard) + if err := a.Run(context.Background(), "edit then verify"); err != nil { + t.Fatal(err) + } + if got := toolResultByID(a.session, "v1"); !strings.Contains(got, "earlier modification") { + t.Fatalf("verify result = %q, want dependency skip", got) + } + if strings.Contains(toolResultByID(a.session, "v1"), "bash done") { + t.Fatal("verification process should not have started") + } + for _, msg := range a.session.Snapshot() { + if msg.ToolCallID != "v1" { + continue + } + if msg.ToolExecution == nil { + t.Fatal("missing execution metadata on skipped verify") + } + if msg.ToolExecution.State != tool.ShellStateNotRun || msg.ToolExecution.FailurePhase != tool.ShellPhaseDependency { + t.Fatalf("execution = %+v", msg.ToolExecution) + } + if msg.ToolExecution.Verification != tool.ShellVerificationNotRun { + t.Fatalf("verification = %q, want not_run (not failed)", msg.ToolExecution.Verification) + } + return + } + t.Fatal("verify tool result missing") +} + +// TestBatchDependencyBarrierIgnoresFailedNonMutationMetaTool keeps bookkeeping +// writers out of the barrier. todo_write, complete_step, ask, bash_output and +// wait all report ReadOnly()==false, but evidence.ToolCallMutates deliberately +// exempts them: they never touch workspace state. A failed todo update must not +// block the real edits queued behind it in the same batch. +func TestBatchDependencyBarrierIgnoresFailedNonMutationMetaTool(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "x.txt") + if err := os.WriteFile(path, []byte("a\n"), 0o600); err != nil { + t.Fatal(err) + } + reg := tool.NewRegistry() + for _, tl := range (builtin.Workspace{Dir: dir}).Tools("edit_file") { + reg.Add(tl) + } + reg.Add(fakeTool{name: "todo_write", readOnly: false, err: fmt.Errorf("todo store unavailable")}) + prov := &scriptedProvider{name: "p", turns: [][]provider.Chunk{ + { + toolCallChunk("t1", "todo_write", `{"todos":[]}`), + toolCallChunk("e1", "edit_file", `{"path":"x.txt","old_string":"a","new_string":"b"}`), + {Type: provider.ChunkDone}, + }, + {{Type: provider.ChunkText, Text: "done"}, {Type: provider.ChunkDone}}, + }} + a := New(prov, reg, NewSession(""), Options{}, event.Discard) + if err := a.Run(context.Background(), "track then edit"); err != nil { + t.Fatal(err) + } + if got := toolResultByID(a.session, "e1"); strings.Contains(got, "earlier modification") { + t.Fatalf("edit was blocked by a failed todo_write: %s", got) + } + got, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + if string(got) != "b\n" { + t.Fatalf("file = %q, want the edit to have been applied", string(got)) + } +} + +// TestBatchDependencyBarrierStopsAfterFailedWorkspaceWrite is the other half of +// the same boundary: a genuine workspace mutation failing still stops the batch. +func TestBatchDependencyBarrierStopsAfterFailedWorkspaceWrite(t *testing.T) { + dir := t.TempDir() + if err := os.WriteFile(filepath.Join(dir, "x.txt"), []byte("a\n"), 0o600); err != nil { + t.Fatal(err) + } + reg := tool.NewRegistry() + for _, tl := range (builtin.Workspace{Dir: dir}).Tools("edit_file") { + reg.Add(tl) + } + prov := &scriptedProvider{name: "p", turns: [][]provider.Chunk{ + { + toolCallChunk("e1", "edit_file", `{"path":"x.txt","old_string":"missing","new_string":"b"}`), + toolCallChunk("e2", "edit_file", `{"path":"x.txt","old_string":"a","new_string":"c"}`), + {Type: provider.ChunkDone}, + }, + {{Type: provider.ChunkText, Text: "done"}, {Type: provider.ChunkDone}}, + }} + a := New(prov, reg, NewSession(""), Options{}, event.Discard) + if err := a.Run(context.Background(), "two edits"); err != nil { + t.Fatal(err) + } + if got := toolResultByID(a.session, "e2"); !strings.Contains(got, "earlier modification") { + t.Fatalf("second edit result = %q, want dependency skip", got) + } + got, err := os.ReadFile(filepath.Join(dir, "x.txt")) + if err != nil { + t.Fatal(err) + } + if string(got) != "a\n" { + t.Fatalf("file = %q, want it untouched after the barrier", string(got)) + } +} + +// writerProxy is a use_capability-shaped CallResolver: schema ReadOnly is true, +// but ResolveCall points at a real writer. The batch barrier must not let this +// run after an earlier mutation failed. +type writerProxy struct { + target tool.Tool + resolves *int +} + +func (writerProxy) Name() string { return "use_capability" } +func (writerProxy) Description() string { return "proxy" } +func (writerProxy) Schema() json.RawMessage { + return json.RawMessage(`{"type":"object","properties":{"action":{"type":"string"}}}`) +} +func (writerProxy) ReadOnly() bool { return true } +func (p writerProxy) Execute(context.Context, json.RawMessage) (string, error) { + return "", fmt.Errorf("proxy Execute must not run; ResolveCall provides the target") +} +func (p writerProxy) ResolveCall(_ context.Context, args json.RawMessage) (tool.ResolvedCall, error) { + if p.resolves != nil { + (*p.resolves)++ + } + return tool.ResolvedCall{ + DisplayName: "use_capability", + TargetName: p.target.Name(), + Args: args, + Target: p.target, + ReadOnly: false, + ProxyAction: "call", + CapabilityID: "mcp-tool:test/write", + }, nil +} + +type capturingWriter struct { + name string + path string + calls *int +} + +func (c *capturingWriter) Name() string { return c.name } +func (c *capturingWriter) Description() string { return "" } +func (c *capturingWriter) Schema() json.RawMessage { return json.RawMessage(`{"type":"object"}`) } +func (c *capturingWriter) ReadOnly() bool { return false } +func (c *capturingWriter) Execute(context.Context, json.RawMessage) (string, error) { + if c.calls != nil { + *c.calls++ + } + if c.path != "" { + _ = os.WriteFile(c.path, []byte("proxy-wrote\n"), 0o600) + } + return "wrote", nil +} + +func TestBatchDependencyBarrierBlocksResolvedMCPWriterAfterFailedMutation(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "x.txt") + if err := os.WriteFile(path, []byte("a\n"), 0o600); err != nil { + t.Fatal(err) + } + proxyWrote := filepath.Join(dir, "proxy-out.txt") + var writerCalls int + var resolves int + writer := &capturingWriter{name: "mcp__test__write", path: proxyWrote, calls: &writerCalls} + reg := tool.NewRegistry() + for _, tl := range (builtin.Workspace{Dir: dir}).Tools("edit_file") { + reg.Add(tl) + } + reg.Add(writerProxy{target: writer, resolves: &resolves}) + reg.Add(writer) // real target available for ResolveCall + + prov := &scriptedProvider{name: "p", turns: [][]provider.Chunk{ + { + toolCallChunk("e1", "edit_file", `{"path":"x.txt","old_string":"missing","new_string":"b"}`), + toolCallChunk("m1", "use_capability", `{"action":"call","capability_id":"mcp-tool:test/write"}`), + {Type: provider.ChunkDone}, + }, + {{Type: provider.ChunkText, Text: "done"}, {Type: provider.ChunkDone}}, + }} + a := New(prov, reg, NewSession(""), Options{}, event.Discard) + if err := a.Run(context.Background(), "fail then mcp write"); err != nil { + t.Fatal(err) + } + if writerCalls != 0 { + t.Fatalf("MCP writer Execute ran %d times; dependency barrier must block after failed edit", writerCalls) + } + if resolves != 1 { + t.Fatalf("proxy ResolveCall ran %d times, want exactly once before the dependency barrier", resolves) + } + if _, err := os.Stat(proxyWrote); err == nil { + t.Fatal("proxy writer mutated disk after failed edit") + } + got := toolResultByID(a.session, "m1") + if !strings.Contains(got, "earlier modification") { + t.Fatalf("proxy result = %q, want dependency skip", got) + } +} + +func TestBatchDependencyBarrierAllowsReadOnlyDiagnosisAfterFailedMutation(t *testing.T) { + // After a mutating failure, host-proven read-only diagnosis must still run. + // Only subsequent mutations and verification commands are skipped. + dir := t.TempDir() + path := filepath.Join(dir, "x.txt") + if err := os.WriteFile(path, []byte("a\n"), 0o600); err != nil { + t.Fatal(err) + } + reg := tool.NewRegistry() + for _, name := range []string{"edit_file", "read_file"} { + for _, tl := range (builtin.Workspace{Dir: dir}).Tools(name) { + reg.Add(tl) + } + } + reg.Add(fakeTool{name: "bash", readOnly: false}) + prov := &scriptedProvider{name: "p", turns: [][]provider.Chunk{ + { + toolCallChunk("e1", "edit_file", `{"path":"x.txt","old_string":"missing","new_string":"b"}`), + toolCallChunk("r1", "read_file", `{"path":"x.txt"}`), + toolCallChunk("v1", "bash", `{"command":"go test ./..."}`), + {Type: provider.ChunkDone}, + }, + {{Type: provider.ChunkText, Text: "done"}, {Type: provider.ChunkDone}}, + }} + a := New(prov, reg, NewSession(""), Options{}, event.Discard) + if err := a.Run(context.Background(), "fail then diagnose"); err != nil { + t.Fatal(err) + } + readOut := toolResultByID(a.session, "r1") + if strings.Contains(readOut, "earlier modification") { + t.Fatalf("read_file was incorrectly dependency-skipped: %q", readOut) + } + trimmed := strings.TrimSpace(readOut) + if strings.HasPrefix(trimmed, "error:") || strings.HasPrefix(trimmed, "blocked:") { + t.Fatalf("read_file should have executed successfully, got %q", readOut) + } + if !strings.Contains(readOut, "a") { + t.Fatalf("read_file body missing original file content: %q", readOut) + } + if got := toolResultByID(a.session, "v1"); !strings.Contains(got, "earlier modification") { + t.Fatalf("verification should be dependency-skipped, got %q", got) + } + if strings.Contains(toolResultByID(a.session, "v1"), "bash done") { + t.Fatal("verification process must not start after failed mutation") + } +} + +func TestModelMessagesStripsToolExecution(t *testing.T) { + code := 1 + in := []provider.Message{ + {Role: provider.RoleUser, Content: "hi"}, + {Role: provider.RoleAssistant, Content: "", ToolCalls: []provider.ToolCall{{ID: "c1", Name: "bash", Arguments: `{"command":"false"}`}}}, + {Role: provider.RoleTool, ToolCallID: "c1", Name: "bash", Content: "error", ToolExecution: &provider.ToolExecution{ + Kind: "shell", Shell: "bash", State: "failed", ExitCode: &code, FailurePhase: "execution", + }}, + } + out := provider.ModelMessages(in) + if len(out) != 3 { + t.Fatalf("len = %d", len(out)) + } + if out[2].ToolExecution != nil { + t.Fatalf("ToolExecution leaked into model messages: %+v", out[2].ToolExecution) + } + if in[2].ToolExecution == nil { + t.Fatal("session copy was mutated") + } +} diff --git a/internal/agent/textsink.go b/internal/agent/textsink.go index b6930a2800..6bca3fb57f 100644 --- a/internal/agent/textsink.go +++ b/internal/agent/textsink.go @@ -96,8 +96,39 @@ func (s *TextSink) Emit(e event.Event) { name := e.Tool.Name if e.Tool.Name == "use_capability" { name = textSinkToolHead(e.Tool.Name, e.Tool.Args) + } else if e.Tool.Name == "bash" && e.Tool.Execution != nil && e.Tool.Execution.Shell != "" { + name = e.Tool.Execution.Shell + switch e.Tool.Execution.Shell { + case "powershell": + name = "Windows PowerShell" + case "pwsh": + name = "PowerShell 7+" + case "git-bash": + name = "Git Bash" + } } - fmt.Fprintf(s.out, " ⊘ %s %s\n", name, e.Tool.Err) + errText := e.Tool.Err + if e.Tool.Execution != nil { + var parts []string + if e.Tool.Execution.ExitCode != nil { + parts = append(parts, fmt.Sprintf("exit %d", *e.Tool.Execution.ExitCode)) + } + if e.Tool.Execution.FailurePhase != "" { + parts = append(parts, e.Tool.Execution.FailurePhase) + } + switch e.Tool.Execution.FailurePhase { + case "preflight", "authorization", "dependency", "launch": + parts = append(parts, "not executed") + default: + if e.Tool.Execution.MutationRisk == "may_be_partial" { + parts = append(parts, "may be partial") + } + } + if len(parts) > 0 { + errText = strings.Join(parts, " · ") + " · " + errText + } + } + fmt.Fprintf(s.out, " ⊘ %s %s\n", name, errText) s.wroteAnything = true } diff --git a/internal/cli/chat_tui.go b/internal/cli/chat_tui.go index 75e5ce04dc..49196807b5 100644 --- a/internal/cli/chat_tui.go +++ b/internal/cli/chat_tui.go @@ -4377,7 +4377,13 @@ func (m *chatTUI) ingestEvent(e event.Event) { } if e.Tool.Err != "" { m.finalizeStreamed() - m.commitLine(" " + red("●") + " " + bold(toolDisplayName(e.Tool.Name)) + " " + red("⊘ "+e.Tool.Err)) + label := shellToolDisplayName(e.Tool.Name, e.Tool.Execution) + detail := shellFailureDetail(e.Tool.Execution) + errText := e.Tool.Err + if detail != "" { + errText = detail + " · " + errText + } + m.commitLine(" " + red("●") + " " + bold(label) + " " + red("⊘ "+errText)) } case event.Usage: diff --git a/internal/cli/toolcard.go b/internal/cli/toolcard.go index 657cfe4b2c..24251616d3 100644 --- a/internal/cli/toolcard.go +++ b/internal/cli/toolcard.go @@ -4,9 +4,12 @@ package cli import ( "encoding/json" + "fmt" "strconv" "strings" + "reasonix/internal/event" + "reasonix/internal/shellrun" "reasonix/internal/tool" ) @@ -116,6 +119,39 @@ func toolDisplayName(name string) string { return name } +// shellToolDisplayName prefers the actual interpreter label when structured +// execution metadata is present (Git Bash / Windows PowerShell / PowerShell 7+). +func shellToolDisplayName(name string, ex *event.ShellExecution) string { + if name == "bash" && ex != nil && ex.Shell != "" { + return shellrun.DisplayName(&tool.ShellExecution{Shell: ex.Shell, ShellVersion: ex.ShellVersion}) + } + return toolDisplayName(name) +} + +// shellFailureDetail appends exit code, failure phase, and mutation-risk hints +// for failed shell results. Empty when there is no structured metadata. +func shellFailureDetail(ex *event.ShellExecution) string { + if ex == nil { + return "" + } + var parts []string + if ex.ExitCode != nil { + parts = append(parts, fmt.Sprintf("exit %d", *ex.ExitCode)) + } + if ex.FailurePhase != "" { + parts = append(parts, ex.FailurePhase) + } + switch ex.FailurePhase { + case tool.ShellPhasePreflight, tool.ShellPhaseAuthorization, tool.ShellPhaseDependency, tool.ShellPhaseLaunch: + parts = append(parts, "not executed") + default: + if ex.MutationRisk == tool.ShellMutationMayBePartial { + parts = append(parts, "may be partial") + } + } + return strings.Join(parts, " · ") +} + // toolArg pulls the primary argument shown in the card's parentheses. func toolArg(name, args string) string { var m map[string]any diff --git a/internal/control/controller.go b/internal/control/controller.go index 3d435bfe67..cf33a5da46 100644 --- a/internal/control/controller.go +++ b/internal/control/controller.go @@ -11,16 +11,13 @@ package control import ( - "bytes" "context" "encoding/json" "errors" "fmt" - "io" "log/slog" "net/http" "os" - "os/exec" "path/filepath" "sort" "strconv" @@ -51,11 +48,11 @@ import ( "reasonix/internal/nilutil" "reasonix/internal/permission" "reasonix/internal/plugin" - "reasonix/internal/proc" "reasonix/internal/provider" "reasonix/internal/recovery" "reasonix/internal/sandbox" "reasonix/internal/sessiontemp" + "reasonix/internal/shellrun" "reasonix/internal/skill" "reasonix/internal/store" "reasonix/internal/taskmonitor" @@ -1733,15 +1730,6 @@ const shellTimeout = 120 * time.Second // the child's pipes to drain, matching the bash tool's WaitDelay. const shellWaitDelay = 5 * time.Second -// shellWriter forwards each chunk of shell output to a callback, so RunShell -// can stream live progress to the frontend as the command produces output. -type shellWriter struct{ emit func(string) } - -func (w *shellWriter) Write(p []byte) (int, error) { - w.emit(string(p)) - return len(p), nil -} - func shellCommandPreview(command string) string { command = strings.TrimSpace(strings.ReplaceAll(command, "\n", " ")) const max = 48 @@ -1776,6 +1764,7 @@ func (c *Controller) RunShell(command string) { } id := "shell-" + string(preview) diagnosticPreview := shellCommandPreview(command) + desc := shellrun.DescriptorFromShell(sh) c.sink.Emit(event.Event{ Kind: event.ToolDispatch, @@ -1783,60 +1772,77 @@ func (c *Controller) RunShell(command string) { ID: id, Name: "bash", Args: fmt.Sprintf(`{"command":%q}`, command), + Execution: &event.ShellExecution{ + Kind: desc.Kind, Shell: desc.Shell, ShellVersion: desc.ShellVersion, + Platform: desc.Platform, SupportsAndAnd: desc.SupportsAndAnd, + State: tool.ShellStateRunning, + }, }, }) - ctx, cancel := context.WithTimeout(ctx, shellTimeout) - defer cancel() - - cmd := exec.CommandContext(ctx, argv[0], argv[1:]...) - cmd.WaitDelay = shellWaitDelay - cmd.Dir = c.workspaceRoot - var buf bytes.Buffer - w := io.MultiWriter(&buf, &shellWriter{emit: func(chunk string) { - c.sink.Emit(event.Event{ - Kind: event.ToolProgress, - Tool: event.Tool{ID: id, Output: chunk}, - }) - }}) - cmd.Stdout = w - cmd.Stderr = w start := time.Now() - _, err := proc.RunCommand(ctx, cmd, proc.RunOptions{ - Track: true, - CancelWaitGrace: shellWaitDelay + time.Second, - Source: "user_shell", - ShellKind: sh.Kind.String(), - ShellPath: sh.Path, - CommandPreview: diagnosticPreview, + res := shellrun.RunForeground(ctx, shellrun.Request{ + Argv: argv, + Dir: c.workspaceRoot, + Timeout: shellTimeout, + WaitDelay: shellWaitDelay, + CommandPreview: diagnosticPreview, + ShellKind: sh.Kind.String(), + ShellPath: sh.Path, + Source: "user_shell", + Track: true, + Progress: func(chunk string) { + c.sink.Emit(event.Event{ + Kind: event.ToolProgress, + Tool: event.Tool{ID: id, Output: chunk}, + }) + }, }) durationMs := time.Since(start).Milliseconds() - out := buf.String() + ex := &event.ShellExecution{ + Kind: desc.Kind, Shell: desc.Shell, ShellVersion: desc.ShellVersion, + Platform: desc.Platform, SupportsAndAnd: desc.SupportsAndAnd, + State: res.State, FailurePhase: res.FailurePhase, + OutputTail: res.OutputTail, DurationMs: durationMs, + MutationRisk: tool.ShellMutationNone, + Verification: tool.ShellVerificationNotVerification, + } + if res.ExitCode != nil { + code := *res.ExitCode + ex.ExitCode = &code + } + switch res.State { + case tool.ShellStateCompleted: + ex.MutationRisk = tool.ShellMutationNone + case tool.ShellStateNotRun: + ex.MutationRisk = tool.ShellMutationNotStarted + case tool.ShellStateFailed: + if res.FailurePhase == tool.ShellPhaseLaunch { + ex.MutationRisk = tool.ShellMutationNotStarted + } else { + ex.MutationRisk = tool.ShellMutationMayBePartial + } + case tool.ShellStateTimedOut, tool.ShellStateCancelled: + ex.MutationRisk = tool.ShellMutationMayBePartial + } - if ctx.Err() == context.Canceled { - c.sink.Emit(event.Event{ - Kind: event.ToolResult, - Tool: event.Tool{ID: id, Name: "bash", Output: out, Err: i18n.M.TurnCancelled, DurationMs: durationMs}, - }) - return nil - } - if ctx.Err() == context.DeadlineExceeded { - c.sink.Emit(event.Event{ - Kind: event.ToolResult, - Tool: event.Tool{ID: id, Name: "bash", Output: out, Err: fmt.Sprintf(i18n.M.ShellExecTimeoutFmt, shellTimeout), DurationMs: durationMs}, - }) - return nil - } - if err != nil { - c.sink.Emit(event.Event{ - Kind: event.ToolResult, - Tool: event.Tool{ID: id, Name: "bash", Output: out, Err: fmt.Sprintf(i18n.M.ShellExecFailedFmt, err), DurationMs: durationMs}, - }) - return nil + errText := "" + switch res.State { + case tool.ShellStateCancelled: + errText = i18n.M.TurnCancelled + case tool.ShellStateTimedOut: + errText = fmt.Sprintf(i18n.M.ShellExecTimeoutFmt, shellTimeout) + case tool.ShellStateFailed, tool.ShellStateNotRun: + if res.Err != nil { + errText = fmt.Sprintf(i18n.M.ShellExecFailedFmt, res.Err) + } } c.sink.Emit(event.Event{ Kind: event.ToolResult, - Tool: event.Tool{ID: id, Name: "bash", Output: out, DurationMs: durationMs}, + Tool: event.Tool{ + ID: id, Name: "bash", Output: res.Combined, Err: errText, + DurationMs: durationMs, Execution: ex, + }, }) return nil }) @@ -5077,8 +5083,9 @@ func (c *Controller) Todos() []evidence.TodoItem { // ToolResultData holds the full arguments and output for one tool call, loaded // on demand when a frontend expands a collapsed tool card. type ToolResultData struct { - Args string `json:"args"` - Output string `json:"output"` + Args string `json:"args"` + Output string `json:"output"` + Execution *provider.ToolExecution `json:"execution,omitempty"` } // ToolResult looks up a tool call by its ID in the session history and returns @@ -5097,8 +5104,9 @@ func (c *Controller) ToolResult(toolID string) *ToolResultData { continue } out := &ToolResultData{ - Args: "", - Output: msgs[i].Content, + Args: "", + Output: msgs[i].Content, + Execution: msgs[i].ToolExecution, } // Walk back to find the assistant turn that issued this call. for j := i; j >= 0; j-- { diff --git a/internal/event/event.go b/internal/event/event.go index 3d4023557f..6f40a2edd1 100644 --- a/internal/event/event.go +++ b/internal/event/event.go @@ -215,6 +215,26 @@ type Tool struct { AttemptID string FileDiff Profile *Profile // ToolDispatch: subagent model/effort (set for task/skill calls) + // Execution is optional local shell metadata (ToolResult). Never sent to + // model providers; omitempty keeps old wire readers compatible. + Execution *ShellExecution +} + +// ShellExecution mirrors tool.ShellExecution for event sinks without importing +// the tool package (event is a lower-level dependency of tool consumers). +type ShellExecution struct { + Kind string `json:"kind,omitempty"` + Shell string `json:"shell,omitempty"` + ShellVersion string `json:"shellVersion,omitempty"` + Platform string `json:"platform,omitempty"` + SupportsAndAnd bool `json:"supportsAndAnd"` + State string `json:"state,omitempty"` + FailurePhase string `json:"failurePhase,omitempty"` + ExitCode *int `json:"exitCode,omitempty"` + OutputTail string `json:"outputTail,omitempty"` + MutationRisk string `json:"mutationRisk,omitempty"` + Verification string `json:"verification,omitempty"` + DurationMs int64 `json:"durationMs,omitempty"` } // FileDiff is a previewed change carried on a writer tool's full ToolDispatch diff --git a/internal/eventwire/wire.go b/internal/eventwire/wire.go index f60142aa68..1be6795a04 100644 --- a/internal/eventwire/wire.go +++ b/internal/eventwire/wire.go @@ -76,6 +76,9 @@ func ToWire(e event.Event) Event { if e.Tool.Profile != nil { wt.Profile = &Profile{Model: e.Tool.Profile.Model, Effort: e.Tool.Profile.Effort} } + if e.Tool.Execution != nil { + wt.Execution = toWireShellExecution(e.Tool.Execution) + } w.Tool = wt case event.Usage: if u := e.Usage; u != nil { @@ -249,25 +252,60 @@ type Profile struct { // Tool is the JSON form of an event.Tool. type Tool struct { - ID string `json:"id,omitempty"` - Name string `json:"name"` - Args string `json:"args,omitempty" externalizable:"true"` - ResolvedName string `json:"resolvedName,omitempty"` - CapabilityID string `json:"capabilityId,omitempty"` - Output string `json:"output,omitempty" externalizable:"true"` - Err string `json:"err,omitempty" externalizable:"true"` - ReadOnly bool `json:"readOnly"` - Truncated bool `json:"truncated,omitempty"` - DurationMs int64 `json:"durationMs,omitempty"` - Partial bool `json:"partial,omitempty"` - ArgChars int `json:"argChars,omitempty"` - Refreshed bool `json:"refreshed,omitempty"` - ParentID string `json:"parentId,omitempty"` - AttemptID string `json:"attemptId,omitempty"` // host-local stream_attempt id for speculative partials - Diff string `json:"diff,omitempty" externalizable:"true"` - Added int `json:"added,omitempty"` - Removed int `json:"removed,omitempty"` - Profile *Profile `json:"profile,omitempty"` + ID string `json:"id,omitempty"` + Name string `json:"name"` + Args string `json:"args,omitempty" externalizable:"true"` + ResolvedName string `json:"resolvedName,omitempty"` + CapabilityID string `json:"capabilityId,omitempty"` + Output string `json:"output,omitempty" externalizable:"true"` + Err string `json:"err,omitempty" externalizable:"true"` + ReadOnly bool `json:"readOnly"` + Truncated bool `json:"truncated,omitempty"` + DurationMs int64 `json:"durationMs,omitempty"` + Partial bool `json:"partial,omitempty"` + ArgChars int `json:"argChars,omitempty"` + Refreshed bool `json:"refreshed,omitempty"` + ParentID string `json:"parentId,omitempty"` + AttemptID string `json:"attemptId,omitempty"` // host-local stream_attempt id for speculative partials + Diff string `json:"diff,omitempty" externalizable:"true"` + Added int `json:"added,omitempty"` + Removed int `json:"removed,omitempty"` + Profile *Profile `json:"profile,omitempty"` + Execution *ShellExecution `json:"execution,omitempty"` +} + +// ShellExecution is the JSON form of event.ShellExecution (local UI metadata). +type ShellExecution struct { + Kind string `json:"kind,omitempty"` + Shell string `json:"shell,omitempty"` + ShellVersion string `json:"shellVersion,omitempty"` + Platform string `json:"platform,omitempty"` + SupportsAndAnd bool `json:"supportsAndAnd"` + State string `json:"state,omitempty"` + FailurePhase string `json:"failurePhase,omitempty"` + ExitCode *int `json:"exitCode,omitempty"` + OutputTail string `json:"outputTail,omitempty"` + MutationRisk string `json:"mutationRisk,omitempty"` + Verification string `json:"verification,omitempty"` + DurationMs int64 `json:"durationMs,omitempty"` +} + +func toWireShellExecution(in *event.ShellExecution) *ShellExecution { + if in == nil { + return nil + } + out := &ShellExecution{ + Kind: in.Kind, Shell: in.Shell, ShellVersion: in.ShellVersion, + Platform: in.Platform, SupportsAndAnd: in.SupportsAndAnd, + State: in.State, FailurePhase: in.FailurePhase, + OutputTail: in.OutputTail, MutationRisk: in.MutationRisk, + Verification: in.Verification, DurationMs: in.DurationMs, + } + if in.ExitCode != nil { + code := *in.ExitCode + out.ExitCode = &code + } + return out } // Usage is the JSON form of provider usage telemetry. diff --git a/internal/evidence/evidence.go b/internal/evidence/evidence.go index ecfd8c4afc..0526b4bf63 100644 --- a/internal/evidence/evidence.go +++ b/internal/evidence/evidence.go @@ -1678,6 +1678,29 @@ func BashToolCallMixesMutationAndVerification(args json.RawMessage) bool { return bashContainsVerificationSegment(command) && bashMayMutate(command) } +// BashToolCallMixesMutationAndMaskableVerification is the ordinary-mode subset of +// BashToolCallMixesMutationAndVerification: the same mixed shape, but only when +// the shell's exit status can actually hide the earlier step's failure. +// +// Delivery mode blocks the broad shape because a mutation invalidates the +// verification *receipt* regardless of exit status. Ordinary mode has no receipt +// to protect — its only concern is a result that looks successful while an +// earlier step failed. `build && test` cannot produce that (bash short-circuits +// and reports the failing status), so blocking it would reject the single most +// common shell shape in real projects for no safety gain. `build; test` can, +// and stays blocked. +func BashToolCallMixesMutationAndMaskableVerification(args json.RawMessage) bool { + if !BashToolCallMixesMutationAndVerification(args) { + return false + } + command, ok := bashCommandFromArgs(args) + if !ok { + return false + } + canMask, analyzed := shellparse.CanMaskEarlierFailure(command) + return analyzed && canMask +} + // BashToolCallMasksVerificationExit reports the common `check; echo $?` shape. // The trailing reporter makes the shell call itself succeed even when the // verifier failed, so a successful tool receipt cannot prove the check passed. @@ -1724,48 +1747,115 @@ func BashToolCallMasksVerificationExit(args json.RawMessage) bool { // this shape before execution and directs callers to auditable file tools, // script files, or conventional verifier commands instead. func BashToolCallUsesOpaqueInlineInterpreter(args json.RawMessage) bool { + command, ok := bashCommandFromArgs(args) + if !ok { + return false + } + return bashCommandUsesOpaqueInlineInterpreter(command) +} + +// BashToolCallUsesNonTerminalInlineInterpreter reports whether an opaque +// inline interpreter (python -c, node -e, …) is not the last top-level segment +// *and* a later segment can overwrite its exit status. Ordinary mode blocks that +// shape deterministically without rewriting the command. An `&&` chain is left +// alone: bash short-circuits it, so the interpreter's failure is still the +// call's exit status and nothing is hidden. +func BashToolCallUsesNonTerminalInlineInterpreter(args json.RawMessage) bool { + command, ok := bashCommandFromArgs(args) + if !ok { + return false + } + segments, _, ok := shellparse.SplitTopLevel(command) + if !ok || len(segments) < 2 { + // Unknown / unparseable syntax: do not pretend full analysis. + return false + } + if canMask, analyzed := shellparse.CanMaskEarlierFailure(command); !analyzed || !canMask { + return false + } + for i, segment := range segments { + if !bashSegmentUsesOpaqueInlineInterpreter(segment) { + continue + } + if i < len(segments)-1 { + return true + } + } + return false +} + +// BashCommandMayBeOpaqueMutation reports whether a sole opaque inline +// interpreter call is allowed to run but cannot be proven read-only for +// mutation-risk labeling. +func BashCommandMayBeOpaqueMutation(args json.RawMessage) bool { + return BashToolCallUsesOpaqueInlineInterpreter(args) +} + +func bashCommandFromArgs(args json.RawMessage) (string, bool) { var fields map[string]json.RawMessage if err := json.Unmarshal(args, &fields); err != nil { - return false + return "", false } command := strings.TrimSpace(stringField(fields, "command")) - if command == "" { - return false - } + return command, command != "" +} + +func bashCommandUsesOpaqueInlineInterpreter(command string) bool { segments, _, ok := shellparse.SplitTopLevel(command) if !ok { return false } for _, segment := range segments { - normalized, _ := shellsafe.NormalizeBashSafeRedirectsForMatch(segment) - argv, malformed := shellparse.StaticFields(normalized) - if malformed != "" || len(argv) == 0 { - continue - } - base := strings.ToLower(filepath.Base(argv[0])) - args := argv[1:] - switch base { - case "node", "bun": - if hasCommandArg(args, "-e", "--eval", "-p", "--print") { - return true - } - case "python", "python3", "ruby", "perl": - if hasCommandArg(args, "-c", "-e") { - return true - } - case "php": - if hasCommandArg(args, "-r") { - return true - } - case "deno": - if len(args) > 0 && strings.EqualFold(args[0], "eval") { - return true - } + if bashSegmentUsesOpaqueInlineInterpreter(segment) { + return true } } return false } +func bashSegmentUsesOpaqueInlineInterpreter(segment string) bool { + normalized, _ := shellsafe.NormalizeBashSafeRedirectsForMatch(segment) + argv, malformed := shellparse.StaticFields(normalized) + if malformed != "" || len(argv) == 0 { + return false + } + base := strings.ToLower(filepath.Base(argv[0])) + args := argv[1:] + switch base { + case "node", "bun": + return hasCommandArg(args, "-e", "--eval", "-p", "--print") + case "python", "python3", "ruby", "perl": + return hasCommandArg(args, "-c", "-e") + case "php": + return hasCommandArg(args, "-r") + case "deno": + return len(args) > 0 && strings.EqualFold(args[0], "eval") + } + return false +} + +// ShellContractPreflightMessage is the model-facing recovery text when a +// deterministic shell contract blocks a call before launch. +func ShellContractPreflightMessage(reason string) string { + switch reason { + case "mixed": + return "blocked: this command runs a verification check after a state-changing segment, separated so the " + + "check's exit status would hide a failure in that earlier segment. " + + "Chain them with '&&' so a failed step stops the command and stays the result, " + + "or run the modification and the verification as separate calls." + case "mask_exit": + return "blocked: the trailing echo/printf of $? masks the verifier's exit status, so this command would look successful even when the check failed. " + + "Run the verifier by itself and let its exit status be the tool result." + case "inline_nonterminal": + return "blocked: an inline interpreter (python -c, node -e, …) is followed by a segment that can hide its failure. " + + "Chain with '&&' so the interpreter's exit status survives, run it as the final command, " + + "or use edit_file for file changes and put script source in a file." + default: + return "blocked: this shell command violates the host execution contract. " + + "Use edit_file for modifications and a separate shell call for verification." + } +} + func bashContainsVerificationSegment(command string) bool { command = strings.TrimSpace(command) if command == "" { diff --git a/internal/evidence/shell_contract_test.go b/internal/evidence/shell_contract_test.go new file mode 100644 index 0000000000..f9c17fa9a0 --- /dev/null +++ b/internal/evidence/shell_contract_test.go @@ -0,0 +1,100 @@ +package evidence + +import ( + "encoding/json" + "testing" +) + +func TestBashToolCallUsesNonTerminalInlineInterpreter(t *testing.T) { + tests := []struct { + command string + want bool + }{ + {command: `python3 -c 'open("x","w").write("y")' ; node verify_frontend_logic.js`, want: true}, + {command: `node -e 'console.log(1)' || go test ./...`, want: true}, + {command: `node -e 'console.log(1)' | tee out.txt`, want: true}, + // `&&` short-circuits: a failing interpreter is still the call's exit + // status, so nothing is hidden and the shape stays allowed. + {command: `node -e 'console.log(1)' && go test ./...`, want: false}, + {command: `python3 -c 'print(1)'`, want: false}, + {command: `go test ./...`, want: false}, + {command: `node --check app.js`, want: false}, + } + for _, tt := range tests { + args, err := json.Marshal(map[string]string{"command": tt.command}) + if err != nil { + t.Fatal(err) + } + if got := BashToolCallUsesNonTerminalInlineInterpreter(args); got != tt.want { + t.Errorf("%q => %v, want %v", tt.command, got, tt.want) + } + } +} + +func TestOrdinaryModeShellContractClassifiers(t *testing.T) { + // deliveryMixed is the broad receipt-integrity classifier Delivery keeps; + // ordinaryMixed additionally requires that the earlier failure can be hidden. + cases := []struct { + command string + deliveryMixed bool + ordinaryMixed bool + mask bool + inline bool + }{ + // Arbitrary node scripts are not host-recognized verifiers; the + // non-terminal inline interpreter rule still blocks this shape. + {command: `python3 -c 'open("/tmp/x","w").write("x")' ; node verify_frontend_logic.js`, inline: true}, + // `;` lets the verifier's status stand in for the generate step's. + {command: `go generate ./... ; go test ./...`, deliveryMixed: true, ordinaryMixed: true}, + // `&&` reports the failing step, so ordinary mode has nothing to protect. + {command: `go generate ./... && go test ./...`, deliveryMixed: true}, + {command: `go build ./... && go test ./...`, deliveryMixed: true}, + {command: `npm install && npm test`, deliveryMixed: true}, + {command: `cargo build && cargo clippy`, deliveryMixed: true}, + // Masked exit is also mixed (echo is not a verifier); agent checks mask first. + {command: `go test ./...; echo $?`, deliveryMixed: true, ordinaryMixed: true, mask: true}, + {command: `python3 -c 'print(1)'`}, + {command: `go test ./...`}, + {command: `tail -n +1 file | node --check -`}, + } + for _, tt := range cases { + args, _ := json.Marshal(map[string]string{"command": tt.command}) + if got := BashToolCallMixesMutationAndVerification(args); got != tt.deliveryMixed { + t.Errorf("deliveryMixed(%q)=%v want %v", tt.command, got, tt.deliveryMixed) + } + if got := BashToolCallMixesMutationAndMaskableVerification(args); got != tt.ordinaryMixed { + t.Errorf("ordinaryMixed(%q)=%v want %v", tt.command, got, tt.ordinaryMixed) + } + if got := BashToolCallMasksVerificationExit(args); got != tt.mask { + t.Errorf("mask(%q)=%v want %v", tt.command, got, tt.mask) + } + if got := BashToolCallUsesNonTerminalInlineInterpreter(args); got != tt.inline { + t.Errorf("inlineNT(%q)=%v want %v", tt.command, got, tt.inline) + } + } +} + +// TestOrdinaryModeAllowsShortCircuitBuildAndVerify pins the regression that +// motivated the narrow ordinary-mode classifier: the everyday +// "build, then verify" chain must stay runnable outside Delivery. +func TestOrdinaryModeAllowsShortCircuitBuildAndVerify(t *testing.T) { + allowed := []string{ + `go build ./... && go test ./...`, + `npm install && npm test`, + `pnpm install && pnpm test`, + `cargo build && cargo test`, + `make build && make test`, + `git pull && go test ./...`, + `mkdir -p out && go test ./...`, + `go mod tidy && go test ./...`, + } + for _, command := range allowed { + args, _ := json.Marshal(map[string]string{"command": command}) + if BashToolCallMixesMutationAndMaskableVerification(args) { + t.Errorf("ordinary mode must allow %q: bash reports the failing step's status", command) + } + if !BashToolCallMixesMutationAndVerification(args) { + t.Errorf("delivery mode should still classify %q as mixed", command) + } + } +} diff --git a/internal/provider/anthropic/tool_execution_wire_test.go b/internal/provider/anthropic/tool_execution_wire_test.go new file mode 100644 index 0000000000..3026bf2aba --- /dev/null +++ b/internal/provider/anthropic/tool_execution_wire_test.go @@ -0,0 +1,63 @@ +package anthropic + +import ( + "context" + "encoding/json" + "strings" + "testing" + + "reasonix/internal/provider" +) + +func TestBuildRequestExcludesToolExecution(t *testing.T) { + code := 1 + msgs := provider.ModelMessages([]provider.Message{ + {Role: provider.RoleUser, Content: "run tests"}, + {Role: provider.RoleAssistant, ToolCalls: []provider.ToolCall{ + {ID: "call_1", Name: "bash", Arguments: `{"command":"go test ./..."}`}, + }}, + { + Role: provider.RoleTool, ToolCallID: "call_1", Name: "bash", Content: "FAIL", + ToolExecution: &provider.ToolExecution{ + Kind: "shell", Shell: "bash", State: "failed", ExitCode: &code, + FailurePhase: "execution", OutputTail: "中文stderr-marker-must-not-leak", + }, + }, + }) + req := (&client{model: "claude-test"}).buildRequest(context.Background(), provider.Request{Messages: msgs}) + body, err := json.Marshal(req) + if err != nil { + t.Fatal(err) + } + s := string(body) + for _, banned := range []string{"tool_execution", "outputTail", "中文stderr-marker-must-not-leak", "failurePhase", "mutationRisk"} { + if strings.Contains(s, banned) { + t.Fatalf("anthropic wire leaked %q: %s", banned, s) + } + } + if !strings.Contains(s, "tool_result") || !strings.Contains(s, "call_1") { + t.Fatalf("anthropic tool_result missing: %s", s) + } +} + +func TestBuildRequestStableWhenLocalExecutionAdded(t *testing.T) { + base := []provider.Message{ + {Role: provider.RoleUser, Content: "run"}, + {Role: provider.RoleAssistant, ToolCalls: []provider.ToolCall{{ID: "c1", Name: "bash", Arguments: `{"command":"true"}`}}}, + {Role: provider.RoleTool, ToolCallID: "c1", Name: "bash", Content: "ok"}, + } + code := 0 + withMeta := append([]provider.Message(nil), base...) + withMeta[2].ToolExecution = &provider.ToolExecution{Kind: "shell", OutputTail: "noise", ExitCode: &code} + a, err := json.Marshal((&client{model: "claude-test"}).buildRequest(context.Background(), provider.Request{Messages: provider.ModelMessages(base)})) + if err != nil { + t.Fatal(err) + } + b, err := json.Marshal((&client{model: "claude-test"}).buildRequest(context.Background(), provider.Request{Messages: provider.ModelMessages(withMeta)})) + if err != nil { + t.Fatal(err) + } + if string(a) != string(b) { + t.Fatalf("anthropic request diverged\nbase=%s\nmeta=%s", a, b) + } +} diff --git a/internal/provider/openai/tool_execution_wire_test.go b/internal/provider/openai/tool_execution_wire_test.go new file mode 100644 index 0000000000..e63eda8f6f --- /dev/null +++ b/internal/provider/openai/tool_execution_wire_test.go @@ -0,0 +1,86 @@ +package openai + +import ( + "encoding/json" + "strings" + "testing" + + "reasonix/internal/provider" +) + +func TestBuildRequestExcludesToolExecution(t *testing.T) { + code := 1 + // Agent applies ModelMessages before Stream; mirror that order. + msgs := provider.ModelMessages([]provider.Message{ + {Role: provider.RoleUser, Content: "run tests"}, + {Role: provider.RoleAssistant, ToolCalls: []provider.ToolCall{ + {ID: "call_1", Name: "bash", Arguments: `{"command":"go test ./..."}`}, + }}, + { + Role: provider.RoleTool, ToolCallID: "call_1", Name: "bash", Content: "FAIL", + ToolExecution: &provider.ToolExecution{ + Kind: "shell", Shell: "bash", State: "failed", ExitCode: &code, + FailurePhase: "execution", OutputTail: "中文stderr-marker-must-not-leak", + MutationRisk: "may_be_partial", Verification: "failed", + }, + }, + }) + // Also prove Sanitize-only path (without ModelMessages) still cannot emit + // local fields because chatMessage is a closed wire struct. + withMeta := []provider.Message{ + {Role: provider.RoleUser, Content: "hi"}, + {Role: provider.RoleAssistant, ToolCalls: []provider.ToolCall{{ID: "c2", Name: "bash", Arguments: `{"command":"false"}`}}}, + {Role: provider.RoleTool, ToolCallID: "c2", Name: "bash", Content: "x", + ToolExecution: &provider.ToolExecution{Kind: "shell", OutputTail: "中文stderr-marker-must-not-leak"}}, + } + + for _, tc := range []struct { + name string + msgs []provider.Message + }{ + {name: "after ModelMessages", msgs: msgs}, + {name: "raw with metadata", msgs: withMeta}, + } { + t.Run(tc.name, func(t *testing.T) { + req := (&client{model: "deepseek-v4"}).buildRequest(provider.Request{Messages: tc.msgs}) + body, err := json.Marshal(req) + if err != nil { + t.Fatal(err) + } + s := string(body) + for _, banned := range []string{"tool_execution", "outputTail", "中文stderr-marker-must-not-leak", "failurePhase", "mutationRisk"} { + if strings.Contains(s, banned) { + t.Fatalf("openai wire leaked %q: %s", banned, s) + } + } + if !strings.Contains(s, `"role":"tool"`) { + t.Fatalf("missing tool role: %s", s) + } + }) + } +} + +func TestBuildRequestStableWhenLocalExecutionAdded(t *testing.T) { + base := []provider.Message{ + {Role: provider.RoleUser, Content: "run"}, + {Role: provider.RoleAssistant, ToolCalls: []provider.ToolCall{{ID: "c1", Name: "bash", Arguments: `{"command":"true"}`}}}, + {Role: provider.RoleTool, ToolCallID: "c1", Name: "bash", Content: "ok"}, + } + code := 0 + withMeta := append([]provider.Message(nil), base...) + withMeta[2].ToolExecution = &provider.ToolExecution{ + Kind: "shell", Shell: "bash", State: "completed", ExitCode: &code, OutputTail: "noise", + } + // Same ModelMessages strip the agent uses. + a, err := json.Marshal((&client{model: "deepseek-v4"}).buildRequest(provider.Request{Messages: provider.ModelMessages(base)})) + if err != nil { + t.Fatal(err) + } + b, err := json.Marshal((&client{model: "deepseek-v4"}).buildRequest(provider.Request{Messages: provider.ModelMessages(withMeta)})) + if err != nil { + t.Fatal(err) + } + if string(a) != string(b) { + t.Fatalf("openai request diverged after local execution metadata\nbase=%s\nmeta=%s", a, b) + } +} diff --git a/internal/provider/provider.go b/internal/provider/provider.go index a7d5a8bf26..99cc55e548 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -96,6 +96,27 @@ type Message struct { // ModelMessages strips the field before handing requests to providers. DecisionReceipts []*DecisionReceipt `json:"decision_receipts,omitempty"` InterruptedTurn *InterruptedTurnRecovery `json:"interrupted_turn,omitempty"` + // ToolExecution is local shell UI metadata on tool-result messages. It is + // persisted for Desktop/CLI/Serve cards and stripped by ModelMessages before + // any provider request so tool schemas and prompt-cache prefixes stay stable. + ToolExecution *ToolExecution `json:"tool_execution,omitempty"` +} + +// ToolExecution is host-local shell metadata mirrored from tool.ShellExecution. +// Provider serializers must never emit this object on the wire. +type ToolExecution struct { + Kind string `json:"kind,omitempty"` + Shell string `json:"shell,omitempty"` + ShellVersion string `json:"shellVersion,omitempty"` + Platform string `json:"platform,omitempty"` + SupportsAndAnd bool `json:"supportsAndAnd"` + State string `json:"state,omitempty"` + FailurePhase string `json:"failurePhase,omitempty"` + ExitCode *int `json:"exitCode,omitempty"` + OutputTail string `json:"outputTail,omitempty"` + MutationRisk string `json:"mutationRisk,omitempty"` + Verification string `json:"verification,omitempty"` + DurationMs int64 `json:"durationMs,omitempty"` } // DecisionReceipt is durable, provider-excluded evidence of a user-owned @@ -249,7 +270,7 @@ func SanitizeToolPairing(msgs []Message) []Message { return NormalizeMessages(ms func ModelMessages(msgs []Message) []Message { needsCopy := false for _, m := range msgs { - if m.LocalOnly || m.RawContent != "" || m.ProviderContent != "" || m.DecisionReceipt != nil || len(m.DecisionReceipts) > 0 { + if m.LocalOnly || m.RawContent != "" || m.ProviderContent != "" || m.DecisionReceipt != nil || len(m.DecisionReceipts) > 0 || m.ToolExecution != nil { needsCopy = true break } @@ -269,6 +290,8 @@ func ModelMessages(msgs []Message) []Message { candidate.RawContent = "" candidate.DecisionReceipt = nil candidate.DecisionReceipts = nil + // Local shell metadata must never enter provider request bytes. + candidate.ToolExecution = nil out = append(out, candidate) } return out diff --git a/internal/provider/responses/tool_execution_wire_test.go b/internal/provider/responses/tool_execution_wire_test.go new file mode 100644 index 0000000000..d588bb9846 --- /dev/null +++ b/internal/provider/responses/tool_execution_wire_test.go @@ -0,0 +1,63 @@ +package responses + +import ( + "encoding/json" + "strings" + "testing" + + "reasonix/internal/provider" +) + +func TestBuildRequestBodyExcludesToolExecution(t *testing.T) { + code := 1 + msgs := []provider.Message{ + {Role: provider.RoleUser, Content: "run tests"}, + {Role: provider.RoleAssistant, ToolCalls: []provider.ToolCall{ + {ID: "call_1", Name: "bash", Arguments: `{"command":"go test ./..."}`}, + }}, + { + Role: provider.RoleTool, ToolCallID: "call_1", Name: "bash", Content: "FAIL", + ToolExecution: &provider.ToolExecution{ + Kind: "shell", Shell: "bash", State: "failed", ExitCode: &code, + FailurePhase: "execution", OutputTail: "中文stderr-marker-must-not-leak", + }, + }, + } + // Production Responses path applies ModelMessages inside buildRequestBody. + bodyMap, _, _ := (&client{model: "gpt-test"}).buildRequestBody(provider.Request{Messages: msgs}) + body, err := json.Marshal(bodyMap) + if err != nil { + t.Fatal(err) + } + s := string(body) + for _, banned := range []string{"tool_execution", "outputTail", "中文stderr-marker-must-not-leak", "failurePhase", "mutationRisk"} { + if strings.Contains(s, banned) { + t.Fatalf("responses wire leaked %q: %s", banned, s) + } + } +} + +func TestBuildRequestBodyStableWhenLocalExecutionAdded(t *testing.T) { + base := []provider.Message{ + {Role: provider.RoleUser, Content: "run"}, + {Role: provider.RoleAssistant, ToolCalls: []provider.ToolCall{{ID: "c1", Name: "bash", Arguments: `{"command":"true"}`}}}, + {Role: provider.RoleTool, ToolCallID: "c1", Name: "bash", Content: "ok"}, + } + code := 0 + withMeta := append([]provider.Message(nil), base...) + withMeta[2].ToolExecution = &provider.ToolExecution{Kind: "shell", OutputTail: "noise", ExitCode: &code} + + aMap, _, _ := (&client{model: "gpt-test"}).buildRequestBody(provider.Request{Messages: base}) + bMap, _, _ := (&client{model: "gpt-test"}).buildRequestBody(provider.Request{Messages: withMeta}) + a, err := json.Marshal(aMap) + if err != nil { + t.Fatal(err) + } + b, err := json.Marshal(bMap) + if err != nil { + t.Fatal(err) + } + if string(a) != string(b) { + t.Fatalf("responses request diverged\nbase=%s\nmeta=%s", a, b) + } +} diff --git a/internal/provider/tool_execution_cache_test.go b/internal/provider/tool_execution_cache_test.go new file mode 100644 index 0000000000..16f4cd3021 --- /dev/null +++ b/internal/provider/tool_execution_cache_test.go @@ -0,0 +1,39 @@ +package provider + +import ( + "encoding/json" + "strings" + "testing" +) + +// TestToolExecutionDoesNotAffectProviderVisibleBytes proves that attaching +// local shell execution metadata to a tool result leaves ModelMessages and +// therefore provider-visible conversation bytes unchanged. +func TestToolExecutionDoesNotAffectProviderVisibleBytes(t *testing.T) { + base := []Message{ + {Role: RoleUser, Content: "run tests"}, + {Role: RoleAssistant, Content: "", ToolCalls: []ToolCall{{ID: "c1", Name: "bash", Arguments: `{"command":"go test ./..."}`}}}, + {Role: RoleTool, ToolCallID: "c1", Name: "bash", Content: "ok"}, + } + code := 0 + withMeta := append([]Message(nil), base...) + withMeta[2].ToolExecution = &ToolExecution{ + Kind: "shell", Shell: "bash", State: "completed", ExitCode: &code, + MutationRisk: "none", Verification: "passed", OutputTail: "noise", + } + + visBase, err := json.Marshal(ModelMessages(base)) + if err != nil { + t.Fatal(err) + } + visMeta, err := json.Marshal(ModelMessages(withMeta)) + if err != nil { + t.Fatal(err) + } + if string(visBase) != string(visMeta) { + t.Fatalf("provider-visible messages diverged after local execution metadata\nbase: %s\nmeta: %s", visBase, visMeta) + } + if strings.Contains(string(visMeta), "tool_execution") || strings.Contains(string(visMeta), "outputTail") { + t.Fatalf("local execution field leaked into provider-visible JSON: %s", visMeta) + } +} diff --git a/internal/serve/index.html b/internal/serve/index.html index 030767a995..57a94e9243 100644 --- a/internal/serve/index.html +++ b/internal/serve/index.html @@ -1128,16 +1128,40 @@ function renderToolDispatch(tool) { if(copy)copy.onclick=async e=>{e.stopPropagation();const ok=await copyText(body.textContent||card.dataset.toolArgs||'');copy.title=ok?__('tool_copied'):__('tool_copy');setTimeout(()=>{copy.title=__('tool_copy');},1200);}; card.appendChild(head);card.appendChild(body);log.appendChild(card);toolCards[tool.id]=card;scrollDown(); } +function shellDisplayName(ex){ + if(!ex||!ex.shell)return 'bash'; + if(ex.shell==='git-bash')return 'Git Bash'; + if(ex.shell==='powershell')return 'Windows PowerShell'; + if(ex.shell==='pwsh')return 'PowerShell 7+'; + return ex.shell; +} function renderToolResult(tool) { const card=toolCards[tool.id]; if(!card)return; const elapsed=Math.max(0,Date.now()-Number(card.dataset.startedAt||Date.now())); + const ex=tool.execution||null; + if(ex&&tool.name==='bash'){ + const title=card.querySelector('.card-title .name'); + if(title)title.textContent=shellDisplayName(ex); + } setToolStatus(card,tool.err?'danger':'success',tool.err?__('tool_failed'):__('tool_done')); - if(tool.err){card.appendChild(el('div','err-body',tool.err));updateToolMeta(card,fmtElapsed(elapsed));} - else { + let meta=fmtElapsed(elapsed); + if(ex){ + const bits=[]; + if(typeof ex.exitCode==='number')bits.push('exit '+ex.exitCode); + if(ex.failurePhase)bits.push(ex.failurePhase); + if(ex.verification&&ex.verification!=='not_verification')bits.push(ex.verification); + if(bits.length)meta=bits.join(' · ')+' · '+meta; + } + if(tool.err){ + const err=el('div','err-body',tool.err); + if(ex&&ex.outputTail){const pre=el('pre','err-stderr',ex.outputTail.slice(0,16000));err.appendChild(pre);} + card.appendChild(err); + updateToolMeta(card,meta); + } else { const body=card.querySelector('.card-body'); const output=String(tool.output||''); if(body)body.textContent=output?output.slice(0,2000)+(tool.truncated?'\n...[truncated]':''):__('tool_no_output'); - updateToolMeta(card,fmtElapsed(elapsed)+' · '+lineCount(output)+' '+__('tool_lines')); + updateToolMeta(card,meta+' · '+lineCount(output)+' '+__('tool_lines')); } scrollDown(); } diff --git a/internal/shellparse/bash.go b/internal/shellparse/bash.go index 1dc2927b44..950519aa4c 100644 --- a/internal/shellparse/bash.go +++ b/internal/shellparse/bash.go @@ -313,6 +313,72 @@ func hasUnescapedMeta(value, meta string) bool { return false } +// CanMaskEarlierFailure reports whether a later part of command can hide the +// failure of an earlier part, so the shell's final exit status is not evidence +// that every step succeeded. +// +// Only `&&` chains are exempt: bash short-circuits them and reports the first +// failing command's status, so `build && test` already surfaces a failed build. +// Everything else can mask — `;` and newlines run the next command regardless, +// `||` runs it precisely when the previous one failed, `|` reports only the +// last stage, and `&` detaches the status entirely. +// +// ok is false when the command cannot be analyzed statically (parse failure, +// here-documents, unsupported control syntax); callers must not read canMask +// as proven-safe in that case. +func CanMaskEarlierFailure(command string) (canMask bool, ok bool) { + if strings.TrimSpace(command) == "" { + return false, true + } + file, err := ParseBash(command) + if err != nil || HasHereDoc(file) { + return false, false + } + // Two or more top-level statements are `;`/newline separated. + if len(file.Stmts) > 1 { + return true, true + } + for _, stmt := range file.Stmts { + masks, stmtOK := stmtCanMaskEarlierFailure(stmt) + if !stmtOK { + return false, false + } + if masks { + return true, true + } + } + return false, true +} + +func stmtCanMaskEarlierFailure(stmt *syntax.Stmt) (bool, bool) { + if stmt == nil || stmt.Negated || stmt.Coprocess || stmt.Disown { + return false, false + } + if stmt.Background { + return true, true + } + switch cmd := stmt.Cmd.(type) { + case *syntax.BinaryCmd: + if cmd.Op != syntax.AndStmt { + // `||`, `|`, and `|&` all let a later stage decide the status. + return true, true + } + xMasks, xOK := stmtCanMaskEarlierFailure(cmd.X) + if !xOK { + return false, false + } + yMasks, yOK := stmtCanMaskEarlierFailure(cmd.Y) + if !yOK { + return false, false + } + return xMasks || yMasks, true + case *syntax.CallExpr: + return false, true + default: + return false, false + } +} + // SplitTopLevel returns simple command segments split at top-level shell // control operators. It preserves each segment's original source text. ok is // false when the command cannot be decomposed without losing safety. diff --git a/internal/shellparse/mask_test.go b/internal/shellparse/mask_test.go new file mode 100644 index 0000000000..6317680a2a --- /dev/null +++ b/internal/shellparse/mask_test.go @@ -0,0 +1,41 @@ +package shellparse + +import "testing" + +func TestCanMaskEarlierFailure(t *testing.T) { + tests := []struct { + command string + canMask bool + ok bool + }{ + // `&&` short-circuits: bash reports the first failing command's status. + {command: `go build ./... && go test ./...`, canMask: false, ok: true}, + {command: `a && b && c`, canMask: false, ok: true}, + {command: `go test ./...`, canMask: false, ok: true}, + {command: ``, canMask: false, ok: true}, + + // Everything else lets a later command decide the exit status. + {command: `go build ./... ; go test ./...`, canMask: true, ok: true}, + {command: "go build ./...\ngo test ./...", canMask: true, ok: true}, + {command: `go build ./... || true`, canMask: true, ok: true}, + {command: `go test ./... | tee out.txt`, canMask: true, ok: true}, + {command: `go test ./... |& tee out.txt`, canMask: true, ok: true}, + {command: `sleep 5 &`, canMask: true, ok: true}, + // A masking operator anywhere in the chain is enough. + {command: `a && b ; c`, canMask: true, ok: true}, + {command: `a ; b && c`, canMask: true, ok: true}, + {command: `a && b || c`, canMask: true, ok: true}, + + // Unanalyzable input must report ok=false rather than guess. + {command: `if true; then go test ./...; fi`, canMask: false, ok: false}, + {command: `go test ./... &&`, canMask: false, ok: false}, + {command: "cat < 0 { + runCtx, cancel = context.WithTimeoutCause(ctx, req.Timeout, errForegroundTimeout) + defer cancel() + } + + cmd := exec.CommandContext(runCtx, req.Argv[0], req.Argv[1:]...) + cmd.Dir = req.Dir + cmd.Env = req.Env + cmd.WaitDelay = waitDelay + + collector := newOutputCollector(tool.OutputTailMaxBytes) + var writers []io.Writer + writers = append(writers, collector.combined, collector.tail) + if req.Progress != nil { + writers = append(writers, &progressWriter{emit: req.Progress}) + } + // Stdout and Stderr must stay the *same* writer value: os/exec then hands the + // child a single pipe, so the two streams interleave in the order the child + // wrote them and only one copy goroutine calls Progress. Two MultiWriters + // would mean two pipes, and combined output would be reordered per stream. + // The bounded tail therefore covers combined output rather than stderr only; + // failing commands routinely report on stdout, so the tail stays useful. + w := io.MultiWriter(writers...) + cmd.Stdout = w + cmd.Stderr = w + + run := req.Run + if run == nil { + run = proc.RunCommand + } + source := req.Source + if source == "" { + source = "shellrun" + } + tracked, err := run(runCtx, cmd, proc.RunOptions{ + Track: req.Track, + CancelWaitGrace: waitDelay + time.Second, + Source: source, + ShellKind: req.ShellKind, + ShellPath: req.ShellPath, + CommandPreview: req.CommandPreview, + }) + + out := Result{ + Combined: collector.combined.String(), + OutputTail: collector.tailString(), + Started: processStarted(cmd, err), + Tracked: tracked, + Cmd: cmd, + } + + if req.PreserveWaitDelay && runCtx.Err() == nil && errors.Is(err, exec.ErrWaitDelay) { + err = nil + } + + // Timeout takes precedence when the tool-local deadline fired. + if errors.Is(context.Cause(runCtx), errForegroundTimeout) { + out.State = tool.ShellStateTimedOut + out.FailurePhase = tool.ShellPhaseTimeout + out.ExitCode = exitCodeFromErr(err) + out.Err = fmt.Errorf("command timed out (> %s)", req.Timeout) + return out + } + // Parent cancellation (user stop / session cancel). + if err != nil && (errors.Is(err, context.Canceled) || errors.Is(runCtx.Err(), context.Canceled) || isCanceledWait(err)) { + out.State = tool.ShellStateCancelled + out.FailurePhase = tool.ShellPhaseCancellation + out.ExitCode = exitCodeFromErr(err) + if cause := context.Cause(runCtx); cause != nil { + out.Err = cause + } else { + out.Err = err + } + return out + } + if err == nil { + code := 0 + out.ExitCode = &code + out.State = tool.ShellStateCompleted + // The tail exists to explain a failure. Dropping it on success keeps + // successful runs from persisting up to 16 KiB of ordinary stdout into + // every session record and tool card. + out.OutputTail = "" + return out + } + if code := exitCodeFromErr(err); code != nil { + out.ExitCode = code + out.Started = true + out.State = tool.ShellStateFailed + out.FailurePhase = tool.ShellPhaseExecution + out.Err = fmt.Errorf("command exited: %w", err) + return out + } + // Process never produced an exit status — launch / dependency style failure. + out.State = tool.ShellStateFailed + if out.Started { + out.FailurePhase = tool.ShellPhaseExecution + } else { + out.FailurePhase = tool.ShellPhaseLaunch + } + out.Err = err + return out +} + +func processStarted(cmd *exec.Cmd, err error) bool { + if cmd != nil && cmd.Process != nil { + return true + } + // ExitError means the process ran. + var ee *exec.ExitError + return errors.As(err, &ee) +} + +func exitCodeFromErr(err error) *int { + if err == nil { + code := 0 + return &code + } + var ee *exec.ExitError + if errors.As(err, &ee) { + code := ee.ExitCode() + return &code + } + return nil +} + +func isCanceledWait(err error) bool { + var c proc.CanceledWaitError + return errors.As(err, &c) +} + +// outputCollector owns the combined buffer and a bounded tail ring. Writes stay +// serialized behind one mutex so a caller that does wire two pipes cannot race +// on the Buffer. +type outputCollector struct { + mu sync.Mutex + combined *lockedBuffer + tail *tailWriter +} + +func newOutputCollector(tailLimit int) *outputCollector { + c := &outputCollector{} + c.combined = &lockedBuffer{mu: &c.mu} + c.tail = &tailWriter{mu: &c.mu, limit: tailLimit} + return c +} + +func (c *outputCollector) tailString() string { + c.mu.Lock() + defer c.mu.Unlock() + return string(c.tail.buf) +} + +// lockedBuffer is a bytes.Buffer guarded by an external mutex so MultiWriter +// concurrent writes from stdout and stderr stay race-free. +type lockedBuffer struct { + mu *sync.Mutex + buf bytes.Buffer +} + +func (b *lockedBuffer) Write(p []byte) (int, error) { + b.mu.Lock() + defer b.mu.Unlock() + return b.buf.Write(p) +} + +func (b *lockedBuffer) String() string { + b.mu.Lock() + defer b.mu.Unlock() + return b.buf.String() +} + +type tailWriter struct { + mu *sync.Mutex + limit int + buf []byte +} + +func (w *tailWriter) Write(p []byte) (int, error) { + w.mu.Lock() + defer w.mu.Unlock() + w.buf = append(w.buf, p...) + if w.limit > 0 && len(w.buf) > w.limit { + w.buf = append([]byte(nil), w.buf[len(w.buf)-w.limit:]...) + } + return len(p), nil +} + +type progressWriter struct{ emit func(string) } + +func (w *progressWriter) Write(p []byte) (int, error) { + if w.emit != nil && len(p) > 0 { + w.emit(string(p)) + } + return len(p), nil +} diff --git a/internal/shellrun/runner_test.go b/internal/shellrun/runner_test.go new file mode 100644 index 0000000000..52ce847f56 --- /dev/null +++ b/internal/shellrun/runner_test.go @@ -0,0 +1,273 @@ +package shellrun + +import ( + "context" + "errors" + "fmt" + "os/exec" + "strings" + "testing" + "time" + + "reasonix/internal/proc" + "reasonix/internal/sandbox" + "reasonix/internal/tool" +) + +func TestDescriptorFromShell(t *testing.T) { + tests := []struct { + name string + sh sandbox.Shell + wantShell string + wantVersion string + wantAndAnd bool + }{ + { + name: "posix bash", + sh: sandbox.Shell{Kind: sandbox.ShellBash, Path: "/bin/bash"}, + wantShell: tool.ShellNameBash, + wantAndAnd: true, + }, + { + name: "git bash path", + sh: sandbox.Shell{Kind: sandbox.ShellBash, Path: `C:\Program Files\Git\bin\bash.exe`}, + wantShell: tool.ShellNameGitBash, + wantAndAnd: true, + }, + { + name: "windows powershell 5.1", + sh: sandbox.Shell{Kind: sandbox.ShellPowerShell, Path: `C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe`}, + wantShell: tool.ShellNamePowerShell, + wantVersion: tool.ShellVersionPS51, + wantAndAnd: false, + }, + { + name: "pwsh 7+", + sh: sandbox.Shell{Kind: sandbox.ShellPowerShell, Path: `C:\Program Files\PowerShell\7\pwsh.exe`}, + wantShell: tool.ShellNamePwsh, + wantVersion: tool.ShellVersionPS7, + wantAndAnd: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := DescriptorFromShell(tt.sh) + if got.Shell != tt.wantShell { + t.Fatalf("Shell = %q, want %q", got.Shell, tt.wantShell) + } + if got.ShellVersion != tt.wantVersion { + t.Fatalf("ShellVersion = %q, want %q", got.ShellVersion, tt.wantVersion) + } + if got.SupportsAndAnd != tt.wantAndAnd { + t.Fatalf("SupportsAndAnd = %v, want %v", got.SupportsAndAnd, tt.wantAndAnd) + } + if got.Kind != "shell" { + t.Fatalf("Kind = %q", got.Kind) + } + if got.Platform == "" { + t.Fatal("Platform empty") + } + }) + } +} + +func TestDisplayName(t *testing.T) { + if got := DisplayName(DescriptorFromShell(sandbox.Shell{Kind: sandbox.ShellPowerShell, Path: "powershell"})); got != "Windows PowerShell" { + t.Fatalf("got %q", got) + } + if got := DisplayName(DescriptorFromShell(sandbox.Shell{Kind: sandbox.ShellPowerShell, Path: "pwsh"})); got != "PowerShell 7+" { + t.Fatalf("got %q", got) + } + if got := DisplayName(DescriptorFromShell(sandbox.Shell{Kind: sandbox.ShellBash, Path: `C:\Program Files\Git\bin\bash.exe`})); got != "Git Bash" { + t.Fatalf("got %q", got) + } +} + +func TestRunForegroundSuccess(t *testing.T) { + argv, sh := shellArgv(t, "printf 'ok\\n'") + res := RunForeground(context.Background(), Request{ + Argv: argv, + ShellKind: sh.Kind.String(), + ShellPath: sh.Path, + Track: true, + }) + if res.Err != nil { + t.Fatalf("err = %v", res.Err) + } + if res.State != tool.ShellStateCompleted { + t.Fatalf("state = %q", res.State) + } + if res.ExitCode == nil || *res.ExitCode != 0 { + t.Fatalf("exitCode = %v", res.ExitCode) + } + if !strings.Contains(res.Combined, "ok") { + t.Fatalf("combined = %q", res.Combined) + } +} + +func TestRunForegroundNonZeroExit(t *testing.T) { + argv, sh := shellArgv(t, "exit 7") + res := RunForeground(context.Background(), Request{ + Argv: argv, + ShellKind: sh.Kind.String(), + ShellPath: sh.Path, + Track: true, + }) + if res.Err == nil { + t.Fatal("expected error") + } + if res.State != tool.ShellStateFailed || res.FailurePhase != tool.ShellPhaseExecution { + t.Fatalf("state/phase = %s/%s", res.State, res.FailurePhase) + } + if res.ExitCode == nil || *res.ExitCode == 0 { + t.Fatalf("exitCode = %v", res.ExitCode) + } +} + +func TestRunForegroundTimeout(t *testing.T) { + cmd := "sleep 5" + sh := sandbox.ResolveShell("auto", "", nil) + if sh.Kind == sandbox.ShellPowerShell { + cmd = "Start-Sleep -Seconds 5" + } + argv, _ := shellArgv(t, cmd) + res := RunForeground(context.Background(), Request{ + Argv: argv, + Timeout: 200 * time.Millisecond, + ShellKind: sh.Kind.String(), + ShellPath: sh.Path, + Track: true, + }) + if res.State != tool.ShellStateTimedOut || res.FailurePhase != tool.ShellPhaseTimeout { + t.Fatalf("state/phase = %s/%s err=%v", res.State, res.FailurePhase, res.Err) + } +} + +func TestRunForegroundLaunchFailure(t *testing.T) { + res := RunForeground(context.Background(), Request{ + Argv: []string{"/nonexistent/reasonix-shell-binary-xyz", "-c", "echo hi"}, + Track: false, + Run: func(ctx context.Context, cmd *exec.Cmd, opts proc.RunOptions) (*proc.TrackedCommand, error) { + return nil, errors.New("exec: no such file") + }, + }) + if res.State != tool.ShellStateFailed || res.FailurePhase != tool.ShellPhaseLaunch { + t.Fatalf("state/phase = %s/%s", res.State, res.FailurePhase) + } + if res.ExitCode != nil { + t.Fatalf("exitCode should be nil for launch failure, got %v", *res.ExitCode) + } +} + +func TestRunForegroundOutputTailBounded(t *testing.T) { + payload := strings.Repeat("中文", 3000) + // Keep the command under typical argv length limits. + if len(payload) > 4000 { + payload = payload[:4000] + } + sh := sandbox.ResolveShell("auto", "", nil) + var command string + if sh.Kind == sandbox.ShellPowerShell { + command = `[Console]::Error.Write('` + strings.ReplaceAll(payload, "'", "''") + `')` + } else { + command = "printf '%s' '" + strings.ReplaceAll(payload, "'", `'\"'\"'`) + "' 1>&2" + } + argv := shellArgvWith(sh, command) + res := RunForeground(context.Background(), Request{ + Argv: argv, + ShellKind: sh.Kind.String(), + ShellPath: sh.Path, + Track: true, + }) + if len(res.OutputTail) > tool.OutputTailMaxBytes { + t.Fatalf("output tail %d > %d", len(res.OutputTail), tool.OutputTailMaxBytes) + } + if !strings.Contains(res.Combined, "中文") && !strings.Contains(res.OutputTail, "中文") { + t.Fatalf("UTF-8 Chinese lost: combined=%q tail=%q", trim(res.Combined, 80), trim(res.OutputTail, 80)) + } +} + +// TestRunForegroundSharesOnePipeForStdoutAndStderr pins the mechanism behind +// ordered combined output: os/exec reuses a single pipe and a single copy +// goroutine only while Stdout and Stderr hold the same writer value. Giving them +// two writers (for example to tee stderr into its own tail) silently splits the +// child's streams into two pipes, and the model then reads reordered output. +func TestRunForegroundSharesOnePipeForStdoutAndStderr(t *testing.T) { + var captured *exec.Cmd + RunForeground(context.Background(), Request{ + Argv: []string{"irrelevant"}, + Progress: func(string) {}, + Run: func(_ context.Context, cmd *exec.Cmd, _ proc.RunOptions) (*proc.TrackedCommand, error) { + captured = cmd + return nil, nil + }, + }) + if captured == nil { + t.Fatal("runner never built a command") + } + if captured.Stdout == nil || captured.Stdout != captured.Stderr { + t.Fatalf("Stdout and Stderr must be the same writer value; got %p and %p", captured.Stdout, captured.Stderr) + } +} + +// TestRunForegroundPreservesInterleaving is the behavioral half of the same +// contract: what the child wrote first must still come first. +func TestRunForegroundPreservesInterleaving(t *testing.T) { + sh := sandbox.ResolveShell("auto", "", nil) + if sh.Kind == sandbox.ShellPowerShell { + t.Skip("stream-buffering semantics differ on PowerShell; the pipe-identity test covers the mechanism") + } + const rounds = 8 + var want strings.Builder + for i := 1; i <= rounds; i++ { + fmt.Fprintf(&want, "out%d\nerr%d\n", i, i) + } + argv := shellArgvWith(sh, "for i in 1 2 3 4 5 6 7 8; do echo out$i; echo err$i 1>&2; done") + // Repeat: two pipes reorder probabilistically, so one run can pass by luck. + for run := 0; run < 10; run++ { + res := RunForeground(context.Background(), Request{Argv: argv, Timeout: 30 * time.Second}) + if res.Combined != want.String() { + t.Fatalf("run %d lost child write order:\ngot %q\nwant %q", run, res.Combined, want.String()) + } + } +} + +// TestRunForegroundDropsTailOnSuccess keeps a successful command from carrying +// up to 16 KiB of ordinary stdout into the session record and the tool card. +func TestRunForegroundDropsTailOnSuccess(t *testing.T) { + argv, _ := shellArgv(t, "echo hello") + res := RunForeground(context.Background(), Request{Argv: argv, Timeout: 30 * time.Second}) + if res.State != tool.ShellStateCompleted { + t.Fatalf("State = %q, want %q", res.State, tool.ShellStateCompleted) + } + if !strings.Contains(res.Combined, "hello") { + t.Fatalf("Combined = %q, want it to contain the output", res.Combined) + } + if res.OutputTail != "" { + t.Fatalf("OutputTail = %q, want empty on success", res.OutputTail) + } +} + +func shellArgv(t *testing.T, command string) ([]string, sandbox.Shell) { + t.Helper() + sh := sandbox.ResolveShell("auto", "", nil) + return shellArgvWith(sh, command), sh +} + +func shellArgvWith(sh sandbox.Shell, command string) []string { + path := sh.Path + if path == "" { + path = sh.Kind.String() + } + if sh.Kind == sandbox.ShellPowerShell { + return []string{path, "-NoProfile", "-NonInteractive", "-Command", sandbox.PowerShellUTF8Script(command)} + } + return []string{path, "-c", command} +} + +func trim(s string, n int) string { + if len(s) <= n { + return s + } + return s[:n] +} diff --git a/internal/tool/builtin/bash.go b/internal/tool/builtin/bash.go index 630300284b..7fb3c23a14 100644 --- a/internal/tool/builtin/bash.go +++ b/internal/tool/builtin/bash.go @@ -1,7 +1,6 @@ package builtin import ( - "bytes" "context" "encoding/json" "errors" @@ -24,6 +23,7 @@ import ( "reasonix/internal/secrets" "reasonix/internal/sessiontemp" "reasonix/internal/shellparse" + "reasonix/internal/shellrun" "reasonix/internal/tool" ) @@ -31,8 +31,6 @@ const ( bashWaitDelay = 5 * time.Second ) -var errBashTimeout = errors.New("bash foreground timeout") - func init() { tool.RegisterBuiltin(bash{}) } var bashShellPATH = cachedBashShellPATH @@ -156,18 +154,52 @@ func (bash) SnipHint() tool.SnipHint { return tool.SnipHint{Head: 40, Tail: 40, HeadChars: 8000, TailChars: 8000} } +// Execute is the compatibility wrapper: all structured metadata is produced by +// ExecuteDetailed and discarded here so plugin/hook callers keep the old shape. func (b bash) Execute(ctx context.Context, args json.RawMessage) (string, error) { + res, err := b.ExecuteDetailed(ctx, args) + return res.Output, err +} + +// ExecutionDescriptor returns shell identity for the bound interpreter without +// launching a process. Invalid args still yield a descriptor from the shell. +func (b bash) ExecutionDescriptor(args json.RawMessage) *tool.ShellExecution { + return shellrun.DescriptorFromShell(b.resolved()) +} + +// ExecuteDetailed runs the shell command and returns structured execution +// metadata for host UI / session persistence. Provider-visible output stays in +// DetailedResult.Output; metadata never enters tool schemas. +func (b bash) ExecuteDetailed(ctx context.Context, args json.RawMessage) (tool.DetailedResult, error) { + start := time.Now() + ex := shellrun.DescriptorFromShell(b.resolved()) + ex.State = tool.ShellStateRunning + ex.MutationRisk = tool.ShellMutationUnknown + ex.Verification = tool.ShellVerificationNotVerification + var p bashParams if err := json.Unmarshal(args, &p); err != nil { - return "", fmt.Errorf("invalid args: %w", err) + ex.State = tool.ShellStateNotRun + ex.FailurePhase = tool.ShellPhasePreflight + ex.MutationRisk = tool.ShellMutationNotStarted + ex.DurationMs = time.Since(start).Milliseconds() + return tool.DetailedResult{Execution: ex}, fmt.Errorf("invalid args: %w", err) } if p.Command == "" { - return "", fmt.Errorf("command is required") + ex.State = tool.ShellStateNotRun + ex.FailurePhase = tool.ShellPhasePreflight + ex.MutationRisk = tool.ShellMutationNotStarted + ex.DurationMs = time.Since(start).Milliseconds() + return tool.DetailedResult{Execution: ex}, fmt.Errorf("command is required") } sh := b.resolved() if !sh.SupportsChaining() && (hasUnquotedSeq(p.Command, "&&") || hasUnquotedSeq(p.Command, "||")) { - return "", fmt.Errorf("this shell is Windows PowerShell, which does not parse '&&' or '||'. " + + ex.State = tool.ShellStateNotRun + ex.FailurePhase = tool.ShellPhasePreflight + ex.MutationRisk = tool.ShellMutationNotStarted + ex.DurationMs = time.Since(start).Milliseconds() + return tool.DetailedResult{Execution: ex}, fmt.Errorf("this shell is Windows PowerShell, which does not parse '&&' or '||'. " + "Sequence with ';' (both run regardless of the first's result), use 'if ($?) { ... }' for " + "conditional chaining, or issue the commands as separate calls") } @@ -177,7 +209,14 @@ func (b bash) Execute(ctx context.Context, args json.RawMessage) (string, error) // so a failed start still releases the lease. prepared, lease, err := b.prepareLaunch(ctx, sh, p.Command, args) if err != nil { - return "", err + ex.State = tool.ShellStateNotRun + ex.FailurePhase = tool.ShellPhaseAuthorization + if strings.Contains(err.Error(), "session temporary") { + ex.FailurePhase = tool.ShellPhaseLaunch + } + ex.MutationRisk = tool.ShellMutationNotStarted + ex.DurationMs = time.Since(start).Milliseconds() + return tool.DetailedResult{Execution: ex}, err } // Background jobs take ownership of the lease until the job goroutine ends. // Foreground/terminal paths release after the process exits. @@ -197,7 +236,10 @@ func (b bash) Execute(ctx context.Context, args json.RawMessage) (string, error) if b.terminal != nil && !p.RunInBackground && !b.sb.Enforce() && !secrets.FilterSubprocessEnv() { envMap := sandbox.SessionTempEnvMap(prepared.SessionTemp, prepared.LinuxSandboxed) if out, ok, termErr := b.terminal.RunCommand(ctx, p.Command, b.workDir, b.timeout, envMap); ok { - return appendSessionDataHint(out, b.guard.CommandHint(b.workDir, p.Command)), termErr + out = appendSessionDataHint(out, b.guard.CommandHint(b.workDir, p.Command)) + applyTerminalResult(ex, termErr) + ex.DurationMs = time.Since(start).Milliseconds() + return tool.DetailedResult{Output: out, Execution: ex}, termErr } } @@ -207,7 +249,11 @@ func (b bash) Execute(ctx context.Context, args json.RawMessage) (string, error) if p.RunInBackground { jm, ok := jobs.FromContext(ctx) if !ok { - return "", fmt.Errorf("background execution is not available in this context") + ex.State = tool.ShellStateNotRun + ex.FailurePhase = tool.ShellPhaseDependency + ex.MutationRisk = tool.ShellMutationNotStarted + ex.DurationMs = time.Since(start).Milliseconds() + return tool.DetailedResult{Execution: ex}, fmt.Errorf("background execution is not available in this context") } workDir := b.workDir // Transfer lease ownership to the job closure; it releases when the @@ -233,11 +279,75 @@ func (b bash) Execute(ctx context.Context, args json.RawMessage) (string, error) return "", normalizeBashRunError(jobCtx, runErr, p.PreserveBackgroundProcesses) }) msg := fmt.Sprintf("Started background job %q. It keeps running across turns; read new output with bash_output(job_id=%q), wait for it with wait, or stop it with kill_shell(job_id=%q).", job.ID, job.ID, job.ID) - return appendSessionDataHint(msg, b.guard.CommandHint(b.workDir, p.Command)), nil + // Background start is not a completed execution: completion is reported + // later by bash_output/wait. Do not masquerade as success with exit 0. + ex.State = tool.ShellStateBackgroundStarted + ex.MutationRisk = tool.ShellMutationUnknown + ex.DurationMs = time.Since(start).Milliseconds() + return tool.DetailedResult{ + Output: appendSessionDataHint(msg, b.guard.CommandHint(b.workDir, p.Command)), + Execution: ex, + }, nil } - out, err := b.runForeground(ctx, p, sh, argv, wrapped, cmdEnv) - return appendSessionDataHint(out, b.guard.CommandHint(b.workDir, p.Command)), err + out, runEx, err := b.runForegroundDetailed(ctx, p, sh, argv, wrapped, cmdEnv) + mergeRunInto(ex, runEx) + ex.DurationMs = time.Since(start).Milliseconds() + return tool.DetailedResult{ + Output: appendSessionDataHint(out, b.guard.CommandHint(b.workDir, p.Command)), + Execution: ex, + }, err +} + +func applyTerminalResult(ex *tool.ShellExecution, err error) { + if ex == nil { + return + } + if err == nil { + ex.State = tool.ShellStateCompleted + ex.ExitCode = tool.IntPtr(0) + ex.MutationRisk = tool.ShellMutationMayHaveCompleted + return + } + if errors.Is(err, context.Canceled) { + ex.State = tool.ShellStateCancelled + ex.FailurePhase = tool.ShellPhaseCancellation + ex.MutationRisk = tool.ShellMutationMayBePartial + return + } + var timeoutErr TerminalTimeoutError + if errors.As(err, &timeoutErr) || errors.Is(err, context.DeadlineExceeded) { + ex.State = tool.ShellStateTimedOut + ex.FailurePhase = tool.ShellPhaseTimeout + ex.MutationRisk = tool.ShellMutationMayBePartial + return + } + var exitErr TerminalExitError + if errors.As(err, &exitErr) { + code := exitErr.Code + ex.ExitCode = &code + ex.State = tool.ShellStateFailed + ex.FailurePhase = tool.ShellPhaseExecution + ex.MutationRisk = tool.ShellMutationMayBePartial + return + } + // Legacy plain errors from older host runners. + ex.State = tool.ShellStateFailed + ex.FailurePhase = tool.ShellPhaseExecution + ex.MutationRisk = tool.ShellMutationMayBePartial +} + +func mergeRunInto(dst *tool.ShellExecution, src *tool.ShellExecution) { + if dst == nil || src == nil { + return + } + dst.State = src.State + dst.FailurePhase = src.FailurePhase + dst.ExitCode = src.ExitCode + dst.OutputTail = src.OutputTail + if src.MutationRisk != "" { + dst.MutationRisk = src.MutationRisk + } } // prepareLaunch acquires a session-temp lease (when a Manager is available), @@ -370,45 +480,64 @@ func bashSandboxEscapeSessionAllowed(ctx context.Context, command string, args j }) } -func (b bash) runForeground(ctx context.Context, p bashParams, sh sandbox.Shell, argv []string, wrapped bool, cmdEnv []string) (string, error) { - runCtx := ctx - timeout := b.foregroundTimeout() - if timeout > 0 { - var cancel context.CancelFunc - runCtx, cancel = context.WithTimeoutCause(ctx, timeout, errBashTimeout) - defer cancel() - } - - cmd := exec.CommandContext(runCtx, argv[0], argv[1:]...) - cmd.Dir = b.workDir // "" lets exec use the process working directory - cmd.Env = cmdEnv - cmd.WaitDelay = bashWaitDelay - var buf bytes.Buffer - w := io.Writer(&buf) +// runForegroundDetailed uses the shared shellrun collector so model bash and +// user !command share exit-code / phase / output-tail classification. +func (b bash) runForegroundDetailed(ctx context.Context, p bashParams, sh sandbox.Shell, argv []string, wrapped bool, cmdEnv []string) (string, *tool.ShellExecution, error) { + ex := shellrun.DescriptorFromShell(sh) + var progress func(string) if emit, ok := tool.ProgressFrom(ctx); ok { - w = io.MultiWriter(&buf, newProgressWriter(emit)) + progress = emit } - cmd.Stdout = w - cmd.Stderr = w - tracked, err := runShellProcess(runCtx, cmd, sh, p.Command, shouldTrackShellProcess(wrapped, sh, p.Command, p.PreserveBackgroundProcesses)) + track := shouldTrackShellProcess(wrapped, sh, p.Command, p.PreserveBackgroundProcesses) + res := shellrun.RunForeground(ctx, shellrun.Request{ + Argv: argv, + Dir: b.workDir, + Env: cmdEnv, + Timeout: b.foregroundTimeout(), + WaitDelay: bashWaitDelay, + CommandPreview: commandPreview(p.Command), + ShellKind: sh.Kind.String(), + ShellPath: sh.Path, + Source: "bash_tool", + Track: track, + PreserveWaitDelay: p.PreserveBackgroundProcesses, + Progress: progress, + }) // A foreground command that spawned a lingering child (e.g. `bazel run`'s // server) leaves it in the process group; Wait only reaped the shell leader. // Kill the group so those don't accumulate into an OOM (#3702). On cancel/ // timeout the command's Cancel path already did this; this covers normal exit. - if shouldReapAfterRun(runCtx, sh, p.Command, p.PreserveBackgroundProcesses) { - reapShellProcess(cmd, tracked) + // shellrun owns the tool-local timeout context, so treat timed_out/cancelled + // as ctx.Err()!=nil for the reap decision. + reapCtx := ctx + if res.State == tool.ShellStateTimedOut || res.State == tool.ShellStateCancelled || ctx.Err() != nil { + // Force reap on forced stops even when preserve_background_processes is set. + reapShellProcess(res.Cmd, res.Tracked) + } else if shouldReapAfterRun(reapCtx, sh, p.Command, p.PreserveBackgroundProcesses) { + reapShellProcess(res.Cmd, res.Tracked) } - err = normalizeBashRunError(runCtx, err, p.PreserveBackgroundProcesses) - out := buf.String() - if errors.Is(context.Cause(runCtx), errBashTimeout) { - return out, fmt.Errorf("command timed out (> %s)", timeout) + ex.State = res.State + ex.FailurePhase = res.FailurePhase + ex.ExitCode = res.ExitCode + ex.OutputTail = res.OutputTail + switch res.State { + case tool.ShellStateCompleted: + ex.MutationRisk = tool.ShellMutationMayHaveCompleted + case tool.ShellStateNotRun: + ex.MutationRisk = tool.ShellMutationNotStarted + case tool.ShellStateFailed: + if res.FailurePhase == tool.ShellPhaseLaunch || res.FailurePhase == tool.ShellPhasePreflight { + ex.MutationRisk = tool.ShellMutationNotStarted + } else { + ex.MutationRisk = tool.ShellMutationMayBePartial + } + case tool.ShellStateTimedOut, tool.ShellStateCancelled: + ex.MutationRisk = tool.ShellMutationMayBePartial + default: + ex.MutationRisk = tool.ShellMutationUnknown } - if err != nil { - // Non-zero exit: feed output and error back so the model can self-correct. - return out, fmt.Errorf("command exited: %w", err) - } - return out, nil + return res.Combined, ex, res.Err } func normalizeBashRunError(ctx context.Context, err error, preserveBackgroundProcesses bool) error { @@ -495,17 +624,6 @@ func reapShellProcess(cmd *exec.Cmd, tracked *proc.TrackedCommand) { proc.KillTree(cmd) } -// progressWriter forwards each chunk the command writes to a tool.ProgressFunc, -// so foreground bash output streams to the frontend as it is produced. -type progressWriter struct{ emit tool.ProgressFunc } - -func newProgressWriter(emit tool.ProgressFunc) *progressWriter { return &progressWriter{emit: emit} } - -func (w *progressWriter) Write(p []byte) (int, error) { - w.emit(string(p)) - return len(p), nil -} - // hasUnquotedSeq reports whether seq appears in s outside any single- or // double-quoted span, so a literal "a && b" string argument doesn't trip the // PowerShell chaining guard. diff --git a/internal/tool/builtin/bash_powershell_test.go b/internal/tool/builtin/bash_powershell_test.go index c579f8cc69..e0b0ce6221 100644 --- a/internal/tool/builtin/bash_powershell_test.go +++ b/internal/tool/builtin/bash_powershell_test.go @@ -3,12 +3,16 @@ package builtin import ( "context" "encoding/json" + "os" "os/exec" + "path/filepath" "runtime" "strings" "testing" + "unicode/utf8" "reasonix/internal/sandbox" + "reasonix/internal/tool" ) func powershellPath(t *testing.T) string { @@ -101,6 +105,131 @@ func TestBashPowerShellOutputIsUTF8(t *testing.T) { } } +// TestBashPowerShellExecuteDetailedContract is the Windows CI contract for the +// shell execution metadata path: Chinese workspace path, UTF-8 output, exit +// code preservation (including 0), and PowerShell 5.1 vs pwsh identity. +func TestBashPowerShellExecuteDetailedContract(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skip("powershell e2e is windows-only; Linux/macOS CI covers bash path") + } + // Windows PowerShell 5.1 is the compatibility-critical path: unlike pwsh, + // it does not parse &&/|| and commonly runs under a legacy console code page. + // Require it on native Windows instead of silently selecting pwsh first. + ps51Path, err := exec.LookPath("powershell") + if err != nil { + t.Fatalf("Windows PowerShell 5.1 is required for this contract: %v", err) + } + paths := []struct { + name string + path string + }{{name: "powershell-5.1", path: ps51Path}} + // PowerShell 7 is optional for ordinary Windows installations, but the + // GitHub Windows runner provides it; exercise it whenever available. + if pwshPath, lookupErr := exec.LookPath("pwsh"); lookupErr == nil { + paths = append(paths, struct { + name string + path string + }{name: "pwsh-7", path: pwshPath}) + } + for _, tc := range paths { + t.Run(tc.name, func(t *testing.T) { + assertPowerShellDetailedContract(t, tc.path) + }) + } +} + +func assertPowerShellDetailedContract(t *testing.T, psPath string) { + t.Helper() + // Chinese directory name — native Windows CI must keep path + UTF-8 intact. + work := filepath.Join(t.TempDir(), "中文目录-reasonix") + if err := os.MkdirAll(work, 0o755); err != nil { + t.Fatal(err) + } + marker := filepath.Join(work, "标记.txt") + if err := os.WriteFile(marker, []byte("内容-utf8"), 0o644); err != nil { + t.Fatal(err) + } + + b := bash{shell: sandbox.Shell{Kind: sandbox.ShellPowerShell, Path: psPath}, workDir: work} + + // Success: exit 0 must be retained (not omitted) and UTF-8 content preserved. + argsOK, _ := json.Marshal(map[string]string{ + "command": "Get-Content -LiteralPath .\\标记.txt -Encoding utf8; Write-Output '中文-ok'", + }) + res, err := b.ExecuteDetailed(context.Background(), argsOK) + if err != nil { + t.Fatalf("success path: %v out=%q", err, res.Output) + } + if res.Execution == nil { + t.Fatal("missing execution metadata") + } + if res.Execution.State != tool.ShellStateCompleted { + t.Fatalf("state=%q", res.Execution.State) + } + if res.Execution.ExitCode == nil || *res.Execution.ExitCode != 0 { + t.Fatalf("exitCode=%v want 0", res.Execution.ExitCode) + } + if !strings.Contains(res.Output, "内容-utf8") || !strings.Contains(res.Output, "中文-ok") { + t.Fatalf("UTF-8/Chinese lost in combined output: %q", res.Output) + } + if !utf8.ValidString(res.Output) { + t.Fatal("combined output is not valid UTF-8") + } + // Descriptor identity: powershell.exe → 5.1; pwsh → 7+. + base := strings.ToLower(filepath.Base(psPath)) + base = strings.TrimSuffix(base, ".exe") + if base == "pwsh" { + if res.Execution.Shell != tool.ShellNamePwsh || res.Execution.ShellVersion != tool.ShellVersionPS7 { + t.Fatalf("pwsh identity = %s/%s", res.Execution.Shell, res.Execution.ShellVersion) + } + if !res.Execution.SupportsAndAnd { + t.Fatal("pwsh should support &&") + } + } else { + if res.Execution.Shell != tool.ShellNamePowerShell || res.Execution.ShellVersion != tool.ShellVersionPS51 { + t.Fatalf("powershell identity = %s/%s", res.Execution.Shell, res.Execution.ShellVersion) + } + if res.Execution.SupportsAndAnd { + t.Fatal("Windows PowerShell 5.1 must not claim && support") + } + } + + // Non-zero exit: preserve real code and execution failure phase. + argsFail, _ := json.Marshal(map[string]string{"command": "exit 17"}) + fail, err := b.ExecuteDetailed(context.Background(), argsFail) + if err == nil { + t.Fatal("exit 17 should error") + } + if fail.Execution == nil || fail.Execution.ExitCode == nil || *fail.Execution.ExitCode != 17 { + t.Fatalf("exit metadata = %+v", fail.Execution) + } + if fail.Execution.State != tool.ShellStateFailed || fail.Execution.FailurePhase != tool.ShellPhaseExecution { + t.Fatalf("fail state/phase = %s/%s", fail.Execution.State, fail.Execution.FailurePhase) + } +} + +func TestBashPowerShell51PreflightRejectsAndAndDetailed(t *testing.T) { + // Runs on every OS: pure preflight, no process launch. + b := bash{shell: sandbox.Shell{Kind: sandbox.ShellPowerShell, Path: "powershell"}} + args, _ := json.Marshal(map[string]string{"command": "echo a && echo b"}) + res, err := b.ExecuteDetailed(context.Background(), args) + if err == nil { + t.Fatal("expected preflight rejection") + } + if res.Execution == nil { + t.Fatal("missing execution") + } + if res.Execution.State != tool.ShellStateNotRun || res.Execution.FailurePhase != tool.ShellPhasePreflight { + t.Fatalf("state/phase = %s/%s", res.Execution.State, res.Execution.FailurePhase) + } + if res.Execution.MutationRisk != tool.ShellMutationNotStarted { + t.Fatalf("mutationRisk = %q", res.Execution.MutationRisk) + } + if res.Execution.ExitCode != nil { + t.Fatalf("exitCode must be unset for preflight, got %v", *res.Execution.ExitCode) + } +} + func TestBashDescriptionReflectsShell(t *testing.T) { ps := bash{shell: sandbox.Shell{Kind: sandbox.ShellPowerShell, Path: "powershell"}} psDesc := ps.Description() diff --git a/internal/tool/builtin/bash_timeout_test.go b/internal/tool/builtin/bash_timeout_test.go index 90cd9006b1..0b6ec050ca 100644 --- a/internal/tool/builtin/bash_timeout_test.go +++ b/internal/tool/builtin/bash_timeout_test.go @@ -113,7 +113,7 @@ func BenchmarkBashForegroundTimeoutConfiguredCap(b *testing.B) { timeout := bt.foregroundTimeout() if timeout > 0 { var cancel context.CancelFunc - runCtx, cancel = context.WithTimeoutCause(ctx, timeout, errBashTimeout) + runCtx, cancel = context.WithTimeoutCause(ctx, timeout, errors.New("bash foreground timeout")) cancel() } if runCtx == nil { diff --git a/internal/tool/builtin/clientio.go b/internal/tool/builtin/clientio.go index a693d78387..92e1071f2f 100644 --- a/internal/tool/builtin/clientio.go +++ b/internal/tool/builtin/clientio.go @@ -2,6 +2,7 @@ package builtin import ( "context" + "fmt" "time" ) @@ -33,6 +34,38 @@ type FileOverlay interface { // terminal should set for the command (typically TMPDIR/TMP/TEMP for the // session-private temporary directory). Callers must not pass a full host // environment dump — only the overrides Reasonix owns. +// +// Prefer typed outcomes when possible: +// - TerminalExitError for a non-zero process exit (Code is the real exit code) +// - TerminalTimeoutError when the host-enforced timeout fired +// - context.Canceled / context.DeadlineExceeded for parent cancellation +// +// Plain fmt.Errorf strings remain accepted for older host runners. type TerminalRunner interface { RunCommand(ctx context.Context, command, cwd string, timeout time.Duration, envOverrides map[string]string) (output string, ok bool, err error) } + +// TerminalExitError is returned by a host TerminalRunner when the command ran +// and produced a non-zero exit status. bash.ExecuteDetailed preserves Code on +// ShellExecution.ExitCode. +type TerminalExitError struct { + Code int +} + +func (e TerminalExitError) Error() string { + return fmt.Sprintf("exit status %d", e.Code) +} + +// TerminalTimeoutError is returned when the host terminal killed the command +// after the tool-local timeout. bash.ExecuteDetailed maps this to +// state=timed_out / failurePhase=timeout. +type TerminalTimeoutError struct { + Timeout time.Duration +} + +func (e TerminalTimeoutError) Error() string { + if e.Timeout > 0 { + return fmt.Sprintf("command timed out after %s (terminal killed)", e.Timeout) + } + return "command timed out (terminal killed)" +} diff --git a/internal/tool/builtin/terminal_result_test.go b/internal/tool/builtin/terminal_result_test.go new file mode 100644 index 0000000000..f32c326805 --- /dev/null +++ b/internal/tool/builtin/terminal_result_test.go @@ -0,0 +1,61 @@ +package builtin + +import ( + "context" + "errors" + "testing" + "time" + + "reasonix/internal/tool" +) + +func TestApplyTerminalResultTypedOutcomes(t *testing.T) { + t.Run("success", func(t *testing.T) { + ex := &tool.ShellExecution{Kind: "shell"} + applyTerminalResult(ex, nil) + if ex.State != tool.ShellStateCompleted || ex.ExitCode == nil || *ex.ExitCode != 0 { + t.Fatalf("%+v", ex) + } + }) + t.Run("exit code", func(t *testing.T) { + ex := &tool.ShellExecution{Kind: "shell"} + applyTerminalResult(ex, TerminalExitError{Code: 7}) + if ex.State != tool.ShellStateFailed || ex.FailurePhase != tool.ShellPhaseExecution { + t.Fatalf("%+v", ex) + } + if ex.ExitCode == nil || *ex.ExitCode != 7 { + t.Fatalf("exitCode=%v", ex.ExitCode) + } + if ex.MutationRisk != tool.ShellMutationMayBePartial { + t.Fatalf("mutationRisk=%q", ex.MutationRisk) + } + }) + t.Run("timeout", func(t *testing.T) { + ex := &tool.ShellExecution{Kind: "shell"} + applyTerminalResult(ex, TerminalTimeoutError{Timeout: 2 * time.Second}) + if ex.State != tool.ShellStateTimedOut || ex.FailurePhase != tool.ShellPhaseTimeout { + t.Fatalf("%+v", ex) + } + if ex.MutationRisk != tool.ShellMutationMayBePartial { + t.Fatalf("mutationRisk=%q", ex.MutationRisk) + } + }) + t.Run("cancel", func(t *testing.T) { + ex := &tool.ShellExecution{Kind: "shell"} + applyTerminalResult(ex, context.Canceled) + if ex.State != tool.ShellStateCancelled || ex.FailurePhase != tool.ShellPhaseCancellation { + t.Fatalf("%+v", ex) + } + }) + t.Run("legacy plain error", func(t *testing.T) { + ex := &tool.ShellExecution{Kind: "shell"} + applyTerminalResult(ex, errors.New("exit status 3")) + if ex.State != tool.ShellStateFailed || ex.ExitCode != nil { + // Plain errors cannot recover the code; still mark partial risk. + t.Fatalf("legacy outcome = %+v", ex) + } + if ex.MutationRisk != tool.ShellMutationMayBePartial { + t.Fatalf("mutationRisk=%q", ex.MutationRisk) + } + }) +} diff --git a/internal/tool/shell_execution.go b/internal/tool/shell_execution.go new file mode 100644 index 0000000000..b445047d35 --- /dev/null +++ b/internal/tool/shell_execution.go @@ -0,0 +1,130 @@ +package tool + +import ( + "context" + "encoding/json" +) + +// ShellExecution is local host metadata for one shell invocation. It is never +// part of the provider-visible tool schema or request bytes; ModelMessages and +// provider serializers must strip it before a model request leaves the host. +// +// Kind is always "shell" for shell invocations so UIs can distinguish this +// optional payload from other future execution kinds without guessing. +type ShellExecution struct { + Kind string `json:"kind"` + Shell string `json:"shell,omitempty"` // bash | git-bash | powershell | pwsh + ShellVersion string `json:"shellVersion,omitempty"` // 5.1 | 7+ (PowerShell only) + Platform string `json:"platform,omitempty"` // windows | darwin | linux + // SupportsAndAnd is explicit even when false so UIs can show PowerShell 5.1 + // chaining limits without treating omission as "unknown". + SupportsAndAnd bool `json:"supportsAndAnd"` + State string `json:"state,omitempty"` // running | completed | failed | timed_out | cancelled | background_started | not_run + FailurePhase string `json:"failurePhase,omitempty"` // preflight | authorization | dependency | launch | execution | timeout | cancellation + // ExitCode is set only when a child process started and produced an exit + // status. Zero is a valid successful code (*int keeps 0 distinct from unset). + ExitCode *int `json:"exitCode,omitempty"` + // OutputTail is the bounded tail of combined stdout+stderr, set only for a + // run that did not succeed. Both streams share one pipe so model-visible + // interleaving stays in child-write order, which rules out a stderr-only + // tail. At most 16 KiB; never a shell executable absolute path. + OutputTail string `json:"outputTail,omitempty"` + MutationRisk string `json:"mutationRisk,omitempty"` // none | not_started | may_have_completed | may_be_partial | unknown + Verification string `json:"verification,omitempty"` // not_verification | not_run | passed | failed + DurationMs int64 `json:"durationMs,omitempty"` +} + +// Shell execution state values. +const ( + ShellStateRunning = "running" + ShellStateCompleted = "completed" + ShellStateFailed = "failed" + ShellStateTimedOut = "timed_out" + ShellStateCancelled = "cancelled" + ShellStateBackgroundStarted = "background_started" + ShellStateNotRun = "not_run" +) + +// Shell failure phase values. +const ( + ShellPhasePreflight = "preflight" + ShellPhaseAuthorization = "authorization" + ShellPhaseDependency = "dependency" + ShellPhaseLaunch = "launch" + ShellPhaseExecution = "execution" + ShellPhaseTimeout = "timeout" + ShellPhaseCancellation = "cancellation" +) + +// Shell mutation risk values. +const ( + ShellMutationNone = "none" + ShellMutationNotStarted = "not_started" + ShellMutationMayHaveCompleted = "may_have_completed" + ShellMutationMayBePartial = "may_be_partial" + ShellMutationUnknown = "unknown" +) + +// Shell verification values. +const ( + ShellVerificationNotVerification = "not_verification" + ShellVerificationNotRun = "not_run" + ShellVerificationPassed = "passed" + ShellVerificationFailed = "failed" +) + +// Shell name values for ShellExecution.Shell. +const ( + ShellNameBash = "bash" + ShellNameGitBash = "git-bash" + ShellNamePowerShell = "powershell" + ShellNamePwsh = "pwsh" +) + +// PowerShell version labels. +const ( + ShellVersionPS51 = "5.1" + ShellVersionPS7 = "7+" +) + +// OutputTailMaxBytes bounds the output tail retained on ShellExecution. +const OutputTailMaxBytes = 16 << 10 + +// DetailedResult is the structured outcome of a DetailedExecutor call. +// Output remains the model-visible text; Execution is host/UI metadata only. +type DetailedResult struct { + Output string + Images []string + Execution *ShellExecution +} + +// DetailedExecutor is an optional Tool capability that returns structured +// execution metadata alongside the model-visible result text. Tools that do +// not implement it continue to use ImageTool/Tool.Execute. +type DetailedExecutor interface { + // ExecutionDescriptor returns a descriptor for the would-be execution + // before the process starts (shell identity, platform, chaining support). + // It must not launch a process. Args may be empty or invalid — return a + // best-effort descriptor from the bound shell configuration. + ExecutionDescriptor(args json.RawMessage) *ShellExecution + // ExecuteDetailed runs the tool and returns structured metadata. On + // policy/preflight blocks, Execution must still be populated (state=not_run). + ExecuteDetailed(ctx context.Context, args json.RawMessage) (DetailedResult, error) +} + +// CloneShellExecution returns a deep copy suitable for attaching to events or +// session messages without sharing mutable pointers (e.g. ExitCode). +func CloneShellExecution(in *ShellExecution) *ShellExecution { + if in == nil { + return nil + } + out := *in + if in.ExitCode != nil { + code := *in.ExitCode + out.ExitCode = &code + } + return &out +} + +// IntPtr returns a pointer to v for ShellExecution.ExitCode. +func IntPtr(v int) *int { return &v }