diff --git a/src/browser/components/tools/AskUserQuestionToolCall.tsx b/src/browser/components/tools/AskUserQuestionToolCall.tsx index 14fc7b6c25..a3187a7cd5 100644 --- a/src/browser/components/tools/AskUserQuestionToolCall.tsx +++ b/src/browser/components/tools/AskUserQuestionToolCall.tsx @@ -27,9 +27,10 @@ import type { AskUserQuestionQuestion, AskUserQuestionToolArgs, AskUserQuestionToolResult, - AskUserQuestionToolSuccessResult, + AskUserQuestionUiOnlyPayload, ToolErrorResult, } from "@/common/types/tools"; +import { getToolOutputUiOnly } from "@/common/utils/tools/toolOutputUiOnly"; const OTHER_VALUE = "__other__"; @@ -59,7 +60,7 @@ function unwrapJsonContainer(value: unknown): unknown { return value; } -function isAskUserQuestionToolSuccessResult(val: unknown): val is AskUserQuestionToolSuccessResult { +function isAskUserQuestionPayload(val: unknown): val is AskUserQuestionUiOnlyPayload { if (!val || typeof val !== "object") { return false; } @@ -262,8 +263,11 @@ export function AskUserQuestionToolCall(props: { return unwrapJsonContainer(props.result); }, [props.result]); + const uiOnlyPayload = getToolOutputUiOnly(resultUnwrapped)?.ask_user_question; + const successResult = - resultUnwrapped && isAskUserQuestionToolSuccessResult(resultUnwrapped) ? resultUnwrapped : null; + uiOnlyPayload ?? + (resultUnwrapped && isAskUserQuestionPayload(resultUnwrapped) ? resultUnwrapped : null); const errorResult = resultUnwrapped && isToolErrorResult(resultUnwrapped) ? resultUnwrapped : null; diff --git a/src/browser/components/tools/BashToolCall.tsx b/src/browser/components/tools/BashToolCall.tsx index 2c9c7ee544..a623d5eb60 100644 --- a/src/browser/components/tools/BashToolCall.tsx +++ b/src/browser/components/tools/BashToolCall.tsx @@ -204,8 +204,8 @@ export const BashToolCall: React.FC = ({ sendToBackground(toolCallId); } : undefined; - const truncatedInfo = result && "truncated" in result ? result.truncated : undefined; + const note = result && "note" in result ? result.note : undefined; const handleToggle = () => { userToggledRef.current = true; @@ -331,6 +331,15 @@ export const BashToolCall: React.FC = ({ {result && ( <> + {note && ( + + Notice +
+ {note} +
+ + )} + {result.success === false && result.error && ( Error diff --git a/src/browser/components/tools/FileEditToolCall.tsx b/src/browser/components/tools/FileEditToolCall.tsx index c96b9b53ae..dfd648aa83 100644 --- a/src/browser/components/tools/FileEditToolCall.tsx +++ b/src/browser/components/tools/FileEditToolCall.tsx @@ -9,6 +9,7 @@ import type { FileEditReplaceLinesToolArgs, FileEditReplaceLinesToolResult, } from "@/common/types/tools"; +import { getToolOutputUiOnly } from "@/common/utils/tools/toolOutputUiOnly"; import { ToolContainer, ToolHeader, @@ -104,6 +105,8 @@ export const FileEditToolCall: React.FC = ({ const { expanded, toggleExpanded } = useToolExpansion(initialExpanded); const [showRaw, setShowRaw] = React.useState(false); + const uiOnlyDiff = getToolOutputUiOnly(result)?.file_edit?.diff; + const diff = result && result.success ? (uiOnlyDiff ?? result.diff) : undefined; const filePath = "file_path" in args ? args.file_path : undefined; // Copy to clipboard with feedback @@ -111,11 +114,11 @@ export const FileEditToolCall: React.FC = ({ // Build kebab menu items for successful edits with diffs const kebabMenuItems: KebabMenuItem[] = - result && result.success && result.diff + result && result.success && diff ? [ { label: copied ? "Copied" : "Copy Patch", - onClick: () => void copyToClipboard(result.diff), + onClick: () => void copyToClipboard(diff), }, { label: showRaw ? "Show Parsed" : "Show Patch", @@ -139,7 +142,7 @@ export const FileEditToolCall: React.FC = ({ {filePath} - {!(result && result.success && result.diff) && ( + {!(result && result.success && diff) && ( {getStatusDisplay(status)} )} {kebabMenuItems.length > 0 && ( @@ -161,15 +164,15 @@ export const FileEditToolCall: React.FC = ({ )} {result.success && - result.diff && + diff && (showRaw ? ( 
-                      {result.diff}
+                      {diff}
) : ( - renderDiff(result.diff, filePath, onReviewNote) + renderDiff(diff, filePath, onReviewNote) ))} )} diff --git a/src/browser/components/tools/shared/ToolPrimitives.tsx b/src/browser/components/tools/shared/ToolPrimitives.tsx index 848f0cdf37..042f2edda0 100644 --- a/src/browser/components/tools/shared/ToolPrimitives.tsx +++ b/src/browser/components/tools/shared/ToolPrimitives.tsx @@ -230,13 +230,12 @@ export const ToolIcon: React.FC = ({ toolName, emoji, className } /** * Error display box with danger styling */ -export const ErrorBox: React.FC> = ({ - className, - ...props -}) => ( +type ErrorBoxProps = React.HTMLAttributes; + +export const ErrorBox: React.FC = ({ className, ...props }) => (
&1 | head -20`, }, }; +/** + * Overflow notice: output saved to temp file with informational notice + */ +export const OverflowNotice: AppStory = { + render: () => ( + + setupSimpleChatStory({ + workspaceId: "ws-bash-overflow", + messages: [ + createUserMessage("msg-1", "Search the logs for failures", { + historySequence: 1, + timestamp: STABLE_TIMESTAMP - 140000, + }), + createAssistantMessage( + "msg-2", + "Scanning logs (output too large, saved to temp file):", + { + historySequence: 2, + timestamp: STABLE_TIMESTAMP - 135000, + toolCalls: [ + createBashOverflowTool( + "call-1", + 'rg "ERROR" /var/log/app/*.log', + [ + "[OUTPUT OVERFLOW - Total output exceeded display limit: 18432 bytes > 16384 bytes (at line 312)]", + "", + "Full output (1250 lines) saved to /home/user/.mux/tmp/bash-1a2b3c4d.txt", + "", + "Use selective filtering tools (e.g. grep) to extract relevant information and continue your task", + "", + "File will be automatically cleaned up when stream ends.", + ].join("\n"), + { + reason: + "Total output exceeded display limit: 18432 bytes > 16384 bytes (at line 312)", + totalLines: 1250, + }, + 5, + 4200, + "Log Scan" + ), + ], + } + ), + ], + }) + } + /> + ), + play: async ({ canvasElement }: { canvasElement: HTMLElement }) => { + await expandAllBashTools(canvasElement); + }, +}; + /** * Background bash workflow: spawn, output states (running/exited/error/filtered/empty), * process list, and terminate diff --git a/src/browser/stories/mockFactory.ts b/src/browser/stories/mockFactory.ts index a3af863a41..88828d4d37 100644 --- a/src/browser/stories/mockFactory.ts +++ b/src/browser/stories/mockFactory.ts @@ -282,6 +282,37 @@ export function createFileEditTool(toolCallId: string, filePath: string, diff: s }; } +export function createBashOverflowTool( + toolCallId: string, + script: string, + notice: string, + truncated: { reason: string; totalLines: number }, + timeoutSecs = 3, + durationMs = 50, + displayName = "Bash" +): MuxPart { + return { + type: "dynamic-tool", + toolCallId, + toolName: "bash", + state: "output-available", + input: { + script, + run_in_background: false, + timeout_secs: timeoutSecs, + display_name: displayName, + }, + output: { + success: true, + output: "", + note: notice, + exitCode: 0, + wall_duration_ms: durationMs, + truncated, + }, + }; +} + export function createBashTool( toolCallId: string, script: string, diff --git a/src/browser/utils/messages/StreamingMessageAggregator.ts b/src/browser/utils/messages/StreamingMessageAggregator.ts index 137bbb5cde..51cfc1275f 100644 --- a/src/browser/utils/messages/StreamingMessageAggregator.ts +++ b/src/browser/utils/messages/StreamingMessageAggregator.ts @@ -23,6 +23,7 @@ import type { } from "@/common/types/stream"; import type { LanguageModelV2Usage } from "@ai-sdk/provider"; import type { TodoItem, StatusSetToolResult, NotifyToolResult } from "@/common/types/tools"; +import { getToolOutputUiOnly } from "@/common/utils/tools/toolOutputUiOnly"; import type { WorkspaceChatMessage, StreamErrorMessage, DeleteMessage } from "@/common/orpc/types"; import { isInitStart, isInitOutput, isInitEnd, isMuxMessage } from "@/common/orpc/types"; @@ -1417,8 +1418,13 @@ export class StreamingMessageAggregator { // Handle browser notifications when Electron wasn't available if (toolName === "notify" && hasSuccessResult(output)) { const result = output as Extract; - if (result.notifiedVia === "browser") { - this.sendBrowserNotification(result.title, result.message, result.workspaceId); + const uiOnlyNotify = getToolOutputUiOnly(output)?.notify; + const legacyNotify = output as { notifiedVia?: string; workspaceId?: string }; + const notifiedVia = uiOnlyNotify?.notifiedVia ?? legacyNotify.notifiedVia; + const workspaceId = uiOnlyNotify?.workspaceId ?? legacyNotify.workspaceId; + + if (notifiedVia === "browser") { + this.sendBrowserNotification(result.title, result.message, workspaceId); } } diff --git a/src/browser/utils/messages/applyToolOutputRedaction.ts b/src/browser/utils/messages/applyToolOutputRedaction.ts index ee3171d61d..bd2290d147 100644 --- a/src/browser/utils/messages/applyToolOutputRedaction.ts +++ b/src/browser/utils/messages/applyToolOutputRedaction.ts @@ -1,9 +1,9 @@ /** - * Apply centralized tool-output redaction to a list of MuxMessages. + * Strip UI-only tool output before sending to providers. * Produces a cloned array safe for sending to providers without touching persisted history/UI. */ import type { MuxMessage } from "@/common/types/message"; -import { redactToolOutput } from "./toolOutputRedaction"; +import { stripToolOutputUiOnly } from "@/common/utils/tools/toolOutputUiOnly"; export function applyToolOutputRedaction(messages: MuxMessage[]): MuxMessage[] { return messages.map((msg) => { @@ -15,7 +15,7 @@ export function applyToolOutputRedaction(messages: MuxMessage[]): MuxMessage[] { return { ...part, - output: redactToolOutput(part.toolName, part.output), + output: stripToolOutputUiOnly(part.output), }; }); diff --git a/src/browser/utils/messages/toolOutputRedaction.ts b/src/browser/utils/messages/toolOutputRedaction.ts deleted file mode 100644 index a30bd72ac7..0000000000 --- a/src/browser/utils/messages/toolOutputRedaction.ts +++ /dev/null @@ -1,185 +0,0 @@ -/** - * Centralized registry for redacting heavy tool outputs before sending to providers. - * - * Phase 1 policy: - * - Keep tool results intact in persisted history and UI. - * - When building provider requests, redact/compact known heavy fields. - * - * Why centralize: - * - Single source of truth for redaction logic. - * - Type safety: if a tool's result type changes, these redactors should fail type-checks. - */ - -import type { - AskUserQuestionToolSuccessResult, - FileEditInsertToolResult, - FileEditReplaceStringToolResult, - FileEditReplaceLinesToolResult, - NotifyToolResult, -} from "@/common/types/tools"; - -// Tool-output from AI SDK is often wrapped like: { type: 'json', value: } -// Keep this helper local so all redactors handle both wrapped and plain objects consistently. -function unwrapJsonContainer(output: unknown): { wrapped: boolean; value: unknown } { - if (output && typeof output === "object" && "type" in output && "value" in output) { - const obj = output as { type: unknown; value: unknown }; - if (obj.type === "json") { - return { wrapped: true, value: obj.value }; - } - } - return { wrapped: false, value: output }; -} - -function rewrapJsonContainer(wrapped: boolean, value: unknown): unknown { - if (wrapped) { - const result: { type: string; value: unknown } = { type: "json", value }; - return result; - } - return value; -} - -function isFileEditInsertResult(v: unknown): v is FileEditInsertToolResult { - return ( - typeof v === "object" && - v !== null && - "success" in v && - typeof (v as { success: unknown }).success === "boolean" - ); -} -// Narrowing helpers for our tool result types -function isFileEditResult( - v: unknown -): v is FileEditReplaceStringToolResult | FileEditReplaceLinesToolResult { - return ( - typeof v === "object" && - v !== null && - "success" in v && - typeof (v as { success: unknown }).success === "boolean" - ); -} - -// Redactors per tool - unified for both string and line replace -function redactFileEditReplace(output: unknown): unknown { - const unwrapped = unwrapJsonContainer(output); - const val = unwrapped.value; - - if (!isFileEditResult(val)) return output; // unknown structure, leave as-is - - if (val.success && "edits_applied" in val) { - // Build base compact result - const compact: Record = { - success: true, - edits_applied: val.edits_applied, - diff: "[diff omitted in context - call file_read on the target file if needed]", - }; - - // Preserve line metadata for line-based edits (only if present) - if ("lines_replaced" in val) { - compact.lines_replaced = val.lines_replaced; - } - if ("line_delta" in val) { - compact.line_delta = val.line_delta; - } - - return rewrapJsonContainer(unwrapped.wrapped, compact); - } - - // Failure payloads are small; pass through unchanged - return output; -} - -function redactFileEditInsert(output: unknown): unknown { - const unwrapped = unwrapJsonContainer(output); - const val = unwrapped.value; - - if (!isFileEditInsertResult(val)) return output; - - if (val.success) { - const compact: FileEditInsertToolResult = { - success: true, - diff: "[diff omitted in context - call file_read on the target file if needed]", - }; - return rewrapJsonContainer(unwrapped.wrapped, compact); - } - - return output; -} - -function isAskUserQuestionToolSuccessResult(val: unknown): val is AskUserQuestionToolSuccessResult { - if (!val || typeof val !== "object") return false; - const record = val as Record; - - if (!Array.isArray(record.questions)) return false; - if (!record.answers || typeof record.answers !== "object") return false; - - // answers is Record - for (const [k, v] of Object.entries(record.answers as Record)) { - if (typeof k !== "string" || typeof v !== "string") return false; - } - - return true; -} - -function redactAskUserQuestion(output: unknown): unknown { - const unwrapped = unwrapJsonContainer(output); - const val = unwrapped.value; - - if (!isAskUserQuestionToolSuccessResult(val)) { - return output; - } - - const pairs = Object.entries(val.answers) - .map(([question, answer]) => `"${question}"="${answer}"`) - .join(", "); - - const summary = - pairs.length > 0 - ? `User has answered your questions: ${pairs}. You can now continue with the user's answers in mind.` - : "User has answered your questions. You can now continue with the user's answers in mind."; - - return rewrapJsonContainer(unwrapped.wrapped, summary); -} - -function isNotifyResult(val: unknown): val is NotifyToolResult { - return ( - typeof val === "object" && - val !== null && - "success" in val && - typeof (val as { success: unknown }).success === "boolean" - ); -} - -/** - * Redact notify tool output to remove internal routing fields (notifiedVia, workspaceId). - * The model only needs to know the notification succeeded. - */ -function redactNotify(output: unknown): unknown { - const unwrapped = unwrapJsonContainer(output); - const val = unwrapped.value; - - if (!isNotifyResult(val)) return output; - - if (val.success) { - return rewrapJsonContainer(unwrapped.wrapped, { success: true }); - } - - // Failure payloads pass through unchanged - return output; -} - -// Public API - registry entrypoint. Add new tools here as needed. -export function redactToolOutput(toolName: string, output: unknown): unknown { - switch (toolName) { - case "ask_user_question": - return redactAskUserQuestion(output); - case "file_edit_replace_string": - case "file_edit_replace_lines": - return redactFileEditReplace(output); - case "file_edit_insert": - return redactFileEditInsert(output); - case "notify": - return redactNotify(output); - default: - return output; - } -} diff --git a/src/common/orpc/schemas/tools.ts b/src/common/orpc/schemas/tools.ts index f8fc19c770..2a00c7a557 100644 --- a/src/common/orpc/schemas/tools.ts +++ b/src/common/orpc/schemas/tools.ts @@ -1,33 +1,61 @@ import { z } from "zod"; +const ToolOutputUiOnlySchema = z.object({ + ask_user_question: z + .object({ + questions: z.array(z.unknown()), + answers: z.record(z.string(), z.string()), + }) + .optional(), + file_edit: z + .object({ + diff: z.string(), + }) + .optional(), + notify: z + .object({ + notifiedVia: z.enum(["electron", "browser"]), + workspaceId: z.string().optional(), + }) + .optional(), +}); + +const ToolOutputUiOnlyFieldSchema = { + ui_only: ToolOutputUiOnlySchema.optional(), +}; + export const BashToolResultSchema = z.discriminatedUnion("success", [ - z.object({ - success: z.literal(true), - wall_duration_ms: z.number(), - output: z.string(), - exitCode: z.literal(0), - note: z.string().optional(), - truncated: z - .object({ - reason: z.string(), - totalLines: z.number(), - }) - .optional(), - }), - z.object({ - success: z.literal(false), - wall_duration_ms: z.number(), - output: z.string().optional(), - exitCode: z.number(), - error: z.string(), - note: z.string().optional(), - truncated: z - .object({ - reason: z.string(), - totalLines: z.number(), - }) - .optional(), - }), + z + .object({ + success: z.literal(true), + wall_duration_ms: z.number(), + output: z.string(), + exitCode: z.literal(0), + note: z.string().optional(), + truncated: z + .object({ + reason: z.string(), + totalLines: z.number(), + }) + .optional(), + }) + .extend(ToolOutputUiOnlyFieldSchema), + z + .object({ + success: z.literal(false), + wall_duration_ms: z.number(), + output: z.string().optional(), + exitCode: z.number(), + error: z.string(), + note: z.string().optional(), + truncated: z + .object({ + reason: z.string(), + totalLines: z.number(), + }) + .optional(), + }) + .extend(ToolOutputUiOnlyFieldSchema), ]); export const FileTreeNodeSchema = z.object({ diff --git a/src/common/types/tools.ts b/src/common/types/tools.ts index 864362afa5..17a70b5480 100644 --- a/src/common/types/tools.ts +++ b/src/common/types/tools.ts @@ -54,16 +54,42 @@ export type AgentSkillReadFileToolArgs = z.infer< >; export type AgentSkillReadFileToolResult = z.infer; +export interface AskUserQuestionUiOnlyPayload { + questions: AskUserQuestionQuestion[]; + answers: Record; +} + +export interface FileEditUiOnlyPayload { + diff: string; +} + +export interface NotifyUiOnlyPayload { + notifiedVia: "electron" | "browser"; + workspaceId?: string; +} + +export interface ToolOutputUiOnly { + ask_user_question?: AskUserQuestionUiOnlyPayload; + file_edit?: FileEditUiOnlyPayload; + notify?: NotifyUiOnlyPayload; +} + +export interface ToolOutputUiOnlyFields { + ui_only?: ToolOutputUiOnly; +} // FileReadToolResult derived from Zod schema (single source of truth) export type FileReadToolResult = z.infer; -export interface FileEditDiffSuccessBase { +export interface FileEditDiffSuccessBase extends ToolOutputUiOnlyFields { success: true; diff: string; warning?: string; } -export interface FileEditErrorResult { +export const FILE_EDIT_DIFF_OMITTED_MESSAGE = + "[diff omitted in context - call file_read on the target file if needed]"; + +export interface FileEditErrorResult extends ToolOutputUiOnlyFields { success: false; error: string; note?: string; // Agent-only message (not displayed in UI) @@ -147,7 +173,7 @@ export const TOOL_EDIT_WARNING = "Always check the tool result before proceeding with other operations."; // Generic tool error shape emitted via streamManager on tool-error parts. -export interface ToolErrorResult { +export interface ToolErrorResult extends ToolOutputUiOnlyFields { success: false; error: string; } @@ -313,14 +339,11 @@ export type WebFetchToolResult = z.infer; // Notify Tool Types export type NotifyToolResult = - | { + | (ToolOutputUiOnlyFields & { success: true; - notifiedVia: "electron" | "browser"; title: string; message?: string; - /** Workspace ID for navigation on notification click */ - workspaceId?: string; - } + }) | { success: false; error: string; diff --git a/src/common/utils/messages/extractEditedFiles.test.ts b/src/common/utils/messages/extractEditedFiles.test.ts index 21aeea44d8..6937083ba5 100644 --- a/src/common/utils/messages/extractEditedFiles.test.ts +++ b/src/common/utils/messages/extractEditedFiles.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from "bun:test"; import { createPatch } from "diff"; +import { FILE_EDIT_DIFF_OMITTED_MESSAGE } from "@/common/types/tools"; import type { MuxMessage } from "@/common/types/message"; import { extractEditedFileDiffs, extractEditedFilePaths } from "./extractEditedFiles"; @@ -11,6 +12,7 @@ function createAssistantMessage( toolName: string; filePath: string; diff: string; + uiOnlyDiff?: string; success?: boolean; }> ): MuxMessage { @@ -23,7 +25,19 @@ function createAssistantMessage( toolName: tc.toolName, state: "output-available" as const, input: { file_path: tc.filePath }, - output: { success: tc.success ?? true, diff: tc.diff }, + output: { + success: tc.success ?? true, + diff: tc.diff, + ...(tc.uiOnlyDiff + ? { + ui_only: { + file_edit: { + diff: tc.uiOnlyDiff, + }, + }, + } + : {}), + }, })), }; } @@ -129,6 +143,27 @@ describe("extractEditedFileDiffs", () => { expect(result[0].diff).toBe(diff); }); + it("should prefer ui_only diffs when present", () => { + const originalContent = "line1\nline2\nline3"; + const newContent = "line1\nmodified\nline3"; + const diff = makeDiff("/path/to/file.ts", originalContent, newContent); + + const messages: MuxMessage[] = [ + createAssistantMessage([ + { + toolName: "file_edit_replace_string", + filePath: "/path/to/file.ts", + diff: FILE_EDIT_DIFF_OMITTED_MESSAGE, + uiOnlyDiff: diff, + }, + ]), + ]; + + const result = extractEditedFileDiffs(messages); + expect(result).toHaveLength(1); + expect(result[0].diff).toBe(diff); + }); + it("should combine multiple non-overlapping diffs for the same file", () => { // Edit 1: change line 2 const original = "line1\nline2\nline3\nline4\nline5"; diff --git a/src/common/utils/messages/extractEditedFiles.ts b/src/common/utils/messages/extractEditedFiles.ts index dfc518d86e..917a6da45b 100644 --- a/src/common/utils/messages/extractEditedFiles.ts +++ b/src/common/utils/messages/extractEditedFiles.ts @@ -1,4 +1,5 @@ import type { MuxMessage } from "@/common/types/message"; +import { getToolOutputUiOnly } from "@/common/utils/tools/toolOutputUiOnly"; import { FILE_EDIT_TOOL_NAMES } from "@/common/types/tools"; import { MAX_EDITED_FILES, MAX_FILE_CONTENT_SIZE } from "@/common/constants/attachments"; import { applyPatch, createPatch, parsePatch } from "diff"; @@ -198,7 +199,11 @@ export function extractEditedFileDiffs(messages: MuxMessage[]): FileEditDiff[] { if (part.state !== "output-available") continue; const output = part.output as FileEditToolOutput | undefined; - if (!output?.success || !output.diff) continue; + if (!output?.success) continue; + + const uiOnly = getToolOutputUiOnly(output); + const diff = uiOnly?.file_edit?.diff ?? output.diff; + if (!diff) continue; const input = part.input as FileEditToolInput | undefined; const filePath = input?.file_path; @@ -208,7 +213,7 @@ export function extractEditedFileDiffs(messages: MuxMessage[]): FileEditDiff[] { if (!diffsByPath.has(filePath)) { diffsByPath.set(filePath, []); } - diffsByPath.get(filePath)!.push(output.diff); + diffsByPath.get(filePath)!.push(diff); // Update edit order (move to end if already exists) const idx = editOrder.indexOf(filePath); diff --git a/src/common/utils/tools/askUserQuestionSummary.ts b/src/common/utils/tools/askUserQuestionSummary.ts new file mode 100644 index 0000000000..b2f239eab8 --- /dev/null +++ b/src/common/utils/tools/askUserQuestionSummary.ts @@ -0,0 +1,9 @@ +export function buildAskUserQuestionSummary(answers: Record): string { + const pairs = Object.entries(answers) + .map(([question, answer]) => `"${question}"="${answer}"`) + .join(", "); + + return pairs.length > 0 + ? `User has answered your questions: ${pairs}. You can now continue with the user's answers in mind.` + : "User has answered your questions. You can now continue with the user's answers in mind."; +} diff --git a/src/common/utils/tools/toolDefinitions.ts b/src/common/utils/tools/toolDefinitions.ts index 25aae4d43b..5e537c3619 100644 --- a/src/common/utils/tools/toolDefinitions.ts +++ b/src/common/utils/tools/toolDefinitions.ts @@ -58,6 +58,30 @@ export const AskUserQuestionQuestionSchema = z } }); +const AskUserQuestionUiOnlySchema = z.object({ + questions: z.array(AskUserQuestionQuestionSchema), + answers: z.record(z.string(), z.string()), +}); + +const ToolOutputUiOnlySchema = z.object({ + ask_user_question: AskUserQuestionUiOnlySchema.optional(), + file_edit: z + .object({ + diff: z.string(), + }) + .optional(), + notify: z + .object({ + notifiedVia: z.enum(["electron", "browser"]), + workspaceId: z.string().optional(), + }) + .optional(), +}); + +const ToolOutputUiOnlyFieldSchema = { + ui_only: ToolOutputUiOnlySchema.optional(), +}; + export const AskUserQuestionToolArgsSchema = z .object({ questions: z.array(AskUserQuestionQuestionSchema).min(1).max(4), @@ -77,13 +101,24 @@ export const AskUserQuestionToolArgsSchema = z } }); -export const AskUserQuestionToolResultSchema = z +const AskUserQuestionToolSummarySchema = z + .object({ + summary: z.string(), + }) + .extend(ToolOutputUiOnlyFieldSchema); + +const AskUserQuestionToolLegacySchema = z .object({ questions: z.array(AskUserQuestionQuestionSchema), answers: z.record(z.string(), z.string()), }) .strict(); +export const AskUserQuestionToolResultSchema = z.union([ + AskUserQuestionToolSummarySchema, + AskUserQuestionToolLegacySchema, +]); + // ----------------------------------------------------------------------------- // task (sub-workspaces as subagents) // ----------------------------------------------------------------------------- @@ -816,27 +851,30 @@ const TruncatedInfoSchema = z.object({ /** * Bash tool result - success, background spawn, or failure. */ -export const BashToolResultSchema = z.union([ - // Foreground success - z.object({ +const BashToolSuccessSchema = z + .object({ success: z.literal(true), output: z.string(), exitCode: z.literal(0), wall_duration_ms: z.number(), note: z.string().optional(), truncated: TruncatedInfoSchema.optional(), - }), - // Background spawn success - z.object({ + }) + .extend(ToolOutputUiOnlyFieldSchema); + +const BashToolBackgroundSchema = z + .object({ success: z.literal(true), output: z.string(), exitCode: z.literal(0), wall_duration_ms: z.number(), taskId: z.string(), backgroundProcessId: z.string(), - }), - // Failure - z.object({ + }) + .extend(ToolOutputUiOnlyFieldSchema); + +const BashToolFailureSchema = z + .object({ success: z.literal(false), output: z.string().optional(), exitCode: z.number(), @@ -844,7 +882,16 @@ export const BashToolResultSchema = z.union([ wall_duration_ms: z.number(), note: z.string().optional(), truncated: TruncatedInfoSchema.optional(), - }), + }) + .extend(ToolOutputUiOnlyFieldSchema); + +export const BashToolResultSchema = z.union([ + // Foreground success + BashToolSuccessSchema, + // Background spawn success + BashToolBackgroundSchema, + // Failure + BashToolFailureSchema, ]); /** @@ -945,33 +992,41 @@ export const AgentSkillReadFileToolResultSchema = FileReadToolResultSchema; * File edit insert tool result - diff or error. */ export const FileEditInsertToolResultSchema = z.union([ - z.object({ - success: z.literal(true), - diff: z.string(), - warning: z.string().optional(), - }), - z.object({ - success: z.literal(false), - error: z.string(), - note: z.string().optional(), - }), + z + .object({ + success: z.literal(true), + diff: z.string(), + warning: z.string().optional(), + }) + .extend(ToolOutputUiOnlyFieldSchema), + z + .object({ + success: z.literal(false), + error: z.string(), + note: z.string().optional(), + }) + .extend(ToolOutputUiOnlyFieldSchema), ]); /** * File edit replace string tool result - diff with edit count or error. */ export const FileEditReplaceStringToolResultSchema = z.union([ - z.object({ - success: z.literal(true), - diff: z.string(), - edits_applied: z.number(), - warning: z.string().optional(), - }), - z.object({ - success: z.literal(false), - error: z.string(), - note: z.string().optional(), - }), + z + .object({ + success: z.literal(true), + diff: z.string(), + edits_applied: z.number(), + warning: z.string().optional(), + }) + .extend(ToolOutputUiOnlyFieldSchema), + z + .object({ + success: z.literal(false), + error: z.string(), + note: z.string().optional(), + }) + .extend(ToolOutputUiOnlyFieldSchema), ]); /** diff --git a/src/common/utils/tools/toolOutputUiOnly.ts b/src/common/utils/tools/toolOutputUiOnly.ts new file mode 100644 index 0000000000..5cf8c2e669 --- /dev/null +++ b/src/common/utils/tools/toolOutputUiOnly.ts @@ -0,0 +1,150 @@ +import type { + AskUserQuestionUiOnlyPayload, + FileEditUiOnlyPayload, + NotifyUiOnlyPayload, + ToolOutputUiOnly, +} from "@/common/types/tools"; + +export const TOOL_OUTPUT_UI_ONLY_FIELD = "ui_only" as const; + +interface JsonContainer { + type: "json"; + value: unknown; +} + +function unwrapJsonContainer(output: unknown): { wrapped: boolean; value: unknown } { + if (output && typeof output === "object" && "type" in output && "value" in output) { + const record = output as { type?: unknown; value?: unknown }; + if (record.type === "json") { + return { wrapped: true, value: record.value }; + } + } + return { wrapped: false, value: output }; +} + +function rewrapJsonContainer(wrapped: boolean, value: unknown): unknown { + if (!wrapped) { + return value; + } + + const container: JsonContainer = { + type: "json", + value, + }; + return container; +} + +function stripUiOnlyDeep(value: unknown): unknown { + if (!value || typeof value !== "object") { + return value; + } + + if (Array.isArray(value)) { + return value.map(stripUiOnlyDeep); + } + + const record = value as Record; + const stripped: Record = {}; + + for (const [key, nested] of Object.entries(record)) { + if (key === TOOL_OUTPUT_UI_ONLY_FIELD) { + continue; + } + stripped[key] = stripUiOnlyDeep(nested); + } + + return stripped; +} + +function isRecord(value: unknown): value is Record { + return !!value && typeof value === "object" && !Array.isArray(value); +} + +function isStringRecord(value: unknown): value is Record { + if (!isRecord(value)) { + return false; + } + + return Object.values(value).every((entry) => typeof entry === "string"); +} + +function isAskUserQuestionUiOnly(value: unknown): value is AskUserQuestionUiOnlyPayload { + if (!isRecord(value)) { + return false; + } + + if (!Array.isArray(value.questions)) { + return false; + } + + return isStringRecord(value.answers); +} + +function isFileEditUiOnly(value: unknown): value is FileEditUiOnlyPayload { + return isRecord(value) && typeof value.diff === "string"; +} + +function isNotifyUiOnly(value: unknown): value is NotifyUiOnlyPayload { + if (!isRecord(value)) { + return false; + } + + const notifiedVia = value.notifiedVia; + if (notifiedVia !== "electron" && notifiedVia !== "browser") { + return false; + } + + if ( + "workspaceId" in value && + value.workspaceId !== undefined && + typeof value.workspaceId !== "string" + ) { + return false; + } + + return true; +} + +function isUiOnlyRecord(value: unknown): value is ToolOutputUiOnly { + if (!isRecord(value)) { + return false; + } + + const record = value; + + if ("ask_user_question" in record && !isAskUserQuestionUiOnly(record.ask_user_question)) { + return false; + } + + if ("file_edit" in record && !isFileEditUiOnly(record.file_edit)) { + return false; + } + + if ("notify" in record && !isNotifyUiOnly(record.notify)) { + return false; + } + + return true; +} + +export function getToolOutputUiOnly(output: unknown): ToolOutputUiOnly | undefined { + const unwrapped = unwrapJsonContainer(output); + const value = unwrapped.value; + + if (!value || typeof value !== "object" || Array.isArray(value)) { + return undefined; + } + + if (!(TOOL_OUTPUT_UI_ONLY_FIELD in value)) { + return undefined; + } + + const uiOnly = (value as Record)[TOOL_OUTPUT_UI_ONLY_FIELD]; + return isUiOnlyRecord(uiOnly) ? uiOnly : undefined; +} + +export function stripToolOutputUiOnly(output: unknown): unknown { + const unwrapped = unwrapJsonContainer(output); + const stripped = stripUiOnlyDeep(unwrapped.value); + return rewrapJsonContainer(unwrapped.wrapped, stripped); +} diff --git a/src/node/services/mock/mockAiRouter.ts b/src/node/services/mock/mockAiRouter.ts index c4bcfce6eb..1d1e61d6c0 100644 --- a/src/node/services/mock/mockAiRouter.ts +++ b/src/node/services/mock/mockAiRouter.ts @@ -261,9 +261,13 @@ function buildToolNotifyReply(): MockAiRouterReply { }, result: { success: true, - notifiedVia: "electron", title: "Task Complete", message: "Your requested task has been completed successfully.", + ui_only: { + notify: { + notifiedVia: "electron", + }, + }, }, }, ], diff --git a/src/node/services/tools/ask_user_question.ts b/src/node/services/tools/ask_user_question.ts index 3507a20332..3ff2def68d 100644 --- a/src/node/services/tools/ask_user_question.ts +++ b/src/node/services/tools/ask_user_question.ts @@ -3,6 +3,7 @@ import assert from "node:assert/strict"; import { tool } from "ai"; import type { AskUserQuestionToolResult } from "@/common/types/tools"; +import { buildAskUserQuestionSummary } from "@/common/utils/tools/askUserQuestionSummary"; import type { ToolConfiguration, ToolFactory } from "@/common/utils/tools/tools"; import { TOOL_DEFINITIONS } from "@/common/utils/tools/toolDefinitions"; import { askUserQuestionManager } from "@/node/services/askUserQuestionManager"; @@ -15,7 +16,15 @@ export const createAskUserQuestionTool: ToolFactory = (config: ToolConfiguration // Claude Code allows passing pre-filled answers directly. If provided, we can short-circuit // and return immediately without prompting. if (args.answers && Object.keys(args.answers).length > 0) { - return { questions: args.questions, answers: args.answers }; + return { + summary: buildAskUserQuestionSummary(args.answers), + ui_only: { + ask_user_question: { + questions: args.questions, + answers: args.answers, + }, + }, + }; } assert(config.workspaceId, "ask_user_question requires a workspaceId"); @@ -29,7 +38,15 @@ export const createAskUserQuestionTool: ToolFactory = (config: ToolConfiguration if (!abortSignal) { const answers = await pendingPromise; - return { questions: args.questions, answers }; + return { + summary: buildAskUserQuestionSummary(answers), + ui_only: { + ask_user_question: { + questions: args.questions, + answers, + }, + }, + }; } if (abortSignal.aborted) { @@ -60,7 +77,15 @@ export const createAskUserQuestionTool: ToolFactory = (config: ToolConfiguration const answers = await Promise.race([pendingPromise, abortPromise]); assert(answers && typeof answers === "object", "Expected answers to be an object"); - return { questions: args.questions, answers }; + return { + summary: buildAskUserQuestionSummary(answers), + ui_only: { + ask_user_question: { + questions: args.questions, + answers, + }, + }, + }; }, }); }; diff --git a/src/node/services/tools/bash.test.ts b/src/node/services/tools/bash.test.ts index b7c012a027..9f483b8615 100644 --- a/src/node/services/tools/bash.test.ts +++ b/src/node/services/tools/bash.test.ts @@ -9,6 +9,14 @@ import { TestTempDir, createTestToolConfig, getTestDeps } from "./testHelpers"; import { createRuntime } from "@/node/runtime/runtimeFactory"; import { sshConnectionPool } from "@/node/runtime/sshConnectionPool"; import type { ToolCallOptions } from "ai"; + +// Type guard to narrow foreground success result (has note, no backgroundProcessId) +function isForegroundSuccess( + result: BashToolResult +): result is BashToolResult & { success: true; note?: string } { + return result.success && !("backgroundProcessId" in result); +} + import { BackgroundProcessManager } from "@/node/services/backgroundProcessManager"; // Mock ToolCallOptions for testing @@ -115,7 +123,7 @@ describe("bash tool", () => { } }); - it("should fail when hard cap (300 lines) is exceeded", async () => { + it("should report overflow when hard cap (300 lines) is exceeded", async () => { using testEnv = createTestBashTool(); const tool = testEnv.tool; const args: BashToolArgs = { @@ -127,11 +135,13 @@ describe("bash tool", () => { const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error).toContain("Line count exceeded"); - expect(result.error).toContain("300 lines"); - expect(result.exitCode).toBe(-1); + expect(result.success).toBe(true); + if (isForegroundSuccess(result)) { + expect(result.output).toBe(""); + expect(result.note).toContain("[OUTPUT OVERFLOW"); + expect(result.note).toContain("Line count exceeded"); + expect(result.note).toContain("300 lines"); + expect(result.exitCode).toBe(0); } }); @@ -147,21 +157,23 @@ describe("bash tool", () => { const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error).toContain("[OUTPUT OVERFLOW"); + expect(result.success).toBe(true); + if (isForegroundSuccess(result)) { + expect(result.output).toBe(""); + expect(result.note).toContain("[OUTPUT OVERFLOW"); // Should contain specific overflow reason (one of the three types) - expect(result.error).toMatch( + expect(result.note).toMatch( /Line count exceeded|Total output exceeded|exceeded per-line limit/ ); - expect(result.error).toContain("Full output"); - expect(result.error).toContain("lines) saved to"); - expect(result.error).toContain("bash-"); - expect(result.error).toContain(".txt"); - expect(result.error).toContain("File will be automatically cleaned up when stream ends"); - - // Extract file path from error message (handles both "lines saved to" and "lines) saved to") - const match = /saved to (\/.+?\.txt)/.exec(result.error); + expect(result.note).toContain("Full output"); + expect(result.note).toContain("lines) saved to"); + expect(result.note).toContain("bash-"); + expect(result.note).toContain(".txt"); + expect(result.note).toContain("File will be automatically cleaned up when stream ends"); + expect(result.exitCode).toBe(0); + + // Extract file path from output message (handles both "lines saved to" and "lines) saved to") + const match = /saved to (\/.+?\.txt)/.exec(result.note ?? ""); expect(match).toBeDefined(); if (match) { const overflowPath = match[1]; @@ -186,13 +198,13 @@ describe("bash tool", () => { } }); - it("should fail early when hard cap is reached", async () => { + it("should report overflow quickly when hard cap is reached", async () => { using testEnv = createTestBashTool(); const tool = testEnv.tool; const startTime = performance.now(); const args: BashToolArgs = { - // This will generate 500 lines quickly - should fail at 300 + // This will generate 500 lines quickly - should report overflow at 300 run_in_background: false, script: "for i in {1..500}; do echo line$i; done", timeout_secs: 5, @@ -202,13 +214,15 @@ describe("bash tool", () => { const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; const duration = performance.now() - startTime; - expect(result.success).toBe(false); - if (!result.success) { + expect(result.success).toBe(true); + if (isForegroundSuccess(result)) { // Should complete quickly since we stop at 300 lines expect(duration).toBeLessThan(4000); - expect(result.error).toContain("Line count exceeded"); - expect(result.error).toContain("300 lines"); - expect(result.exitCode).toBe(-1); + expect(result.output).toBe(""); + expect(result.note).toContain("[OUTPUT OVERFLOW"); + expect(result.note).toContain("Line count exceeded"); + expect(result.note).toContain("300 lines"); + expect(result.exitCode).toBe(0); } }); @@ -350,12 +364,14 @@ describe("bash tool", () => { const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; - expect(result.success).toBe(false); - if (!result.success) { + expect(result.success).toBe(true); + if (isForegroundSuccess(result)) { // Should use tmpfile behavior - expect(result.error).toContain("[OUTPUT OVERFLOW"); - expect(result.error).toContain("saved to"); - expect(result.error).not.toContain("[OUTPUT TRUNCATED"); + expect(result.output).toBe(""); + expect(result.note).toContain("[OUTPUT OVERFLOW"); + expect(result.note).toContain("saved to"); + expect(result.note).not.toContain("[OUTPUT TRUNCATED"); + expect(result.exitCode).toBe(0); // Verify temp file was created in runtimeTempDir expect(fs.existsSync(tempDir.path)).toBe(true); @@ -388,14 +404,16 @@ describe("bash tool", () => { const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; - expect(result.success).toBe(false); - if (!result.success) { + expect(result.success).toBe(true); + if (isForegroundSuccess(result)) { // Should hit display limit and save to temp file - expect(result.error).toContain("[OUTPUT OVERFLOW"); - expect(result.error).toContain("saved to"); + expect(result.output).toBe(""); + expect(result.note).toContain("[OUTPUT OVERFLOW"); + expect(result.note).toContain("saved to"); + expect(result.exitCode).toBe(0); // Extract and verify temp file - const match = /saved to (\/.*?\.txt)/.exec(result.error); + const match = /saved to (\/.*?\.txt)/.exec(result.note ?? ""); expect(match).toBeDefined(); if (match) { const overflowPath = match[1]; @@ -443,13 +461,15 @@ describe("bash tool", () => { const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; - expect(result.success).toBe(false); - if (!result.success) { + expect(result.success).toBe(true); + if (isForegroundSuccess(result)) { // Should hit file limit - expect(result.error).toContain("file preservation limit"); + expect(result.output).toBe(""); + expect(result.note).toContain("file preservation limit"); + expect(result.exitCode).toBe(0); // Extract and verify temp file - const match = /saved to (\/.*?\.txt)/.exec(result.error); + const match = /saved to (\/.*?\.txt)/.exec(result.note ?? ""); expect(match).toBeDefined(); if (match) { const overflowPath = match[1]; @@ -489,13 +509,14 @@ describe("bash tool", () => { const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; - expect(result.success).toBe(false); - if (!result.success) { + expect(result.success).toBe(true); + if (isForegroundSuccess(result)) { // Should hit display limit - expect(result.error).toContain("display limit"); + expect(result.note).toContain("display limit"); + expect(result.exitCode).toBe(0); // Extract and verify temp file contains the completion marker - const match = /saved to (\/.*?\.txt)/.exec(result.error); + const match = /saved to (\/.*?\.txt)/.exec(result.note ?? ""); expect(match).toBeDefined(); if (match) { const overflowPath = match[1]; @@ -532,13 +553,15 @@ describe("bash tool", () => { const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; - expect(result.success).toBe(false); - if (!result.success) { + expect(result.success).toBe(true); + if (isForegroundSuccess(result)) { // Should hit per-line limit (file truncation, not display) - expect(result.error).toContain("per-line limit"); + expect(result.output).toBe(""); + expect(result.note).toContain("per-line limit"); + expect(result.exitCode).toBe(0); // Extract and verify temp file does NOT contain the second echo - const match = /saved to (\/.*?\.txt)/.exec(result.error); + const match = /saved to (\/.*?\.txt)/.exec(result.note ?? ""); expect(match).toBeDefined(); if (match) { const overflowPath = match[1]; @@ -609,10 +632,13 @@ describe("bash tool", () => { const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; // Should trigger display truncation at exactly 300 lines - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error).toContain("300 lines"); - expect(result.error).toContain("display limit"); + expect(result.success).toBe(true); + if (isForegroundSuccess(result)) { + expect(result.output).toBe(""); + expect(result.note).toContain("[OUTPUT OVERFLOW"); + expect(result.note).toContain("300 lines"); + expect(result.note).toContain("display limit"); + expect(result.exitCode).toBe(0); } tempDir[Symbol.dispose](); @@ -908,7 +934,7 @@ describe("bash tool", () => { } }); - it("should fail when line exceeds max line bytes", async () => { + it("should report overflow when line exceeds max line bytes", async () => { using testEnv = createTestBashTool(); const tool = testEnv.tool; const longLine = "x".repeat(2000); @@ -921,14 +947,15 @@ describe("bash tool", () => { const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error).toMatch(/exceeded per-line limit|OUTPUT OVERFLOW/); - expect(result.exitCode).toBe(-1); + expect(result.success).toBe(true); + if (isForegroundSuccess(result)) { + expect(result.output).toBe(""); + expect(result.note).toMatch(/exceeded per-line limit|OUTPUT OVERFLOW/); + expect(result.exitCode).toBe(0); } }); - it("should fail when total bytes limit exceeded", async () => { + it("should report overflow when total bytes limit exceeded", async () => { using testEnv = createTestBashTool(); const tool = testEnv.tool; const lineContent = "x".repeat(100); @@ -942,14 +969,15 @@ describe("bash tool", () => { const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error).toMatch(/Total output exceeded limit|OUTPUT OVERFLOW/); - expect(result.exitCode).toBe(-1); + expect(result.success).toBe(true); + if (isForegroundSuccess(result)) { + expect(result.output).toBe(""); + expect(result.note).toMatch(/Total output exceeded limit|OUTPUT OVERFLOW/); + expect(result.exitCode).toBe(0); } }); - it("should fail early when byte limit is reached", async () => { + it("should report overflow when byte limit is reached", async () => { using testEnv = createTestBashTool(); const tool = testEnv.tool; const args: BashToolArgs = { @@ -961,10 +989,11 @@ describe("bash tool", () => { const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult; - expect(result.success).toBe(false); - if (!result.success) { - expect(result.error).toMatch(/Total output exceeded limit|OUTPUT OVERFLOW/); - expect(result.exitCode).toBe(-1); + expect(result.success).toBe(true); + if (isForegroundSuccess(result)) { + expect(result.output).toBe(""); + expect(result.note).toMatch(/Total output exceeded limit|OUTPUT OVERFLOW/); + expect(result.exitCode).toBe(0); } }); diff --git a/src/node/services/tools/bash.ts b/src/node/services/tools/bash.ts index dfa129cdb0..ae62a75c02 100644 --- a/src/node/services/tools/bash.ts +++ b/src/node/services/tools/bash.ts @@ -759,9 +759,13 @@ ${scriptWithEnv}`; // Handle tmpfile overflow policy separately (writes to file) if (truncated && (config.overflow_policy ?? "tmpfile") === "tmpfile") { - // tmpfile policy: Save overflow output to temp file instead of returning an error + // tmpfile policy: Save overflow output to temp file and return a successful response. // We don't show ANY of the actual output to avoid overwhelming context. // Instead, save it to a temp file and encourage the agent to use filtering tools. + const truncationInfo = { + reason: overflowReason ?? "unknown reason", + totalLines: lines.length, + }; try { // Use 8 hex characters for short, memorable temp file IDs const fileId = Math.random().toString(16).substring(2, 10); @@ -779,7 +783,7 @@ ${scriptWithEnv}`; await writerInstance.write(encoder.encode(fullOutput)); await writerInstance.close(); - const output = `[OUTPUT OVERFLOW - ${overflowReason ?? "unknown reason"}] + const notice = `[OUTPUT OVERFLOW - ${overflowReason ?? "unknown reason"}] Full output (${lines.length} lines) saved to ${overflowPath} @@ -788,10 +792,12 @@ Use selective filtering tools (e.g. grep) to extract relevant information and co File will be automatically cleaned up when stream ends.`; return { - success: false, - error: output, - exitCode: -1, + success: true, + output: "", + note: notice, + exitCode: 0, wall_duration_ms, + truncated: truncationInfo, }; } catch (err) { // If temp file creation fails, fall back to original error diff --git a/src/node/services/tools/file_edit_insert.ts b/src/node/services/tools/file_edit_insert.ts index 08e772436f..40ea47d247 100644 --- a/src/node/services/tools/file_edit_insert.ts +++ b/src/node/services/tools/file_edit_insert.ts @@ -1,6 +1,10 @@ import { tool } from "ai"; import type { FileEditInsertToolArgs, FileEditInsertToolResult } from "@/common/types/tools"; -import { EDIT_FAILED_NOTE_PREFIX, NOTE_READ_FILE_RETRY } from "@/common/types/tools"; +import { + EDIT_FAILED_NOTE_PREFIX, + FILE_EDIT_DIFF_OMITTED_MESSAGE, + NOTE_READ_FILE_RETRY, +} from "@/common/types/tools"; import type { ToolConfiguration, ToolFactory } from "@/common/utils/tools/tools"; import { TOOL_DEFINITIONS } from "@/common/utils/tools/toolDefinitions"; import { generateDiff, validateAndCorrectPath, validatePlanModeAccess } from "./fileCommon"; @@ -98,7 +102,12 @@ export const createFileEditInsertTool: ToolFactory = (config: ToolConfiguration) const diff = generateDiff(resolvedPath, "", content); return { success: true, - diff, + diff: FILE_EDIT_DIFF_OMITTED_MESSAGE, + ui_only: { + file_edit: { + diff, + }, + }, ...(pathWarning && { warning: pathWarning }), }; } diff --git a/src/node/services/tools/file_edit_operation.ts b/src/node/services/tools/file_edit_operation.ts index dc30dea76a..5142487b22 100644 --- a/src/node/services/tools/file_edit_operation.ts +++ b/src/node/services/tools/file_edit_operation.ts @@ -1,4 +1,8 @@ -import type { FileEditDiffSuccessBase, FileEditErrorResult } from "@/common/types/tools"; +import { + FILE_EDIT_DIFF_OMITTED_MESSAGE, + type FileEditDiffSuccessBase, + type FileEditErrorResult, +} from "@/common/types/tools"; import type { ToolConfiguration } from "@/common/utils/tools/tools"; import { generateDiff, @@ -143,7 +147,12 @@ export async function executeFileEditOperation({ return { success: true, - diff, + diff: FILE_EDIT_DIFF_OMITTED_MESSAGE, + ui_only: { + file_edit: { + diff, + }, + }, ...operationResult.metadata, ...(pathWarning && { warning: pathWarning }), }; diff --git a/src/node/services/tools/file_edit_replace_lines.ts b/src/node/services/tools/file_edit_replace_lines.ts index 891c14b5fa..d4ec62e1fc 100644 --- a/src/node/services/tools/file_edit_replace_lines.ts +++ b/src/node/services/tools/file_edit_replace_lines.ts @@ -1,18 +1,20 @@ import { tool } from "ai"; +import type { ToolOutputUiOnlyFields } from "@/common/types/tools"; import type { ToolConfiguration, ToolFactory } from "@/common/utils/tools/tools"; import { TOOL_DEFINITIONS } from "@/common/utils/tools/toolDefinitions"; import { executeFileEditOperation } from "./file_edit_operation"; import { handleLineReplace, type LineReplaceArgs } from "./file_edit_replace_shared"; -export interface FileEditReplaceLinesResult { +export interface FileEditReplaceLinesResult extends ToolOutputUiOnlyFields { success: true; diff: string; edits_applied: number; lines_replaced: number; line_delta: number; + warning?: string; } -export interface FileEditReplaceLinesError { +export interface FileEditReplaceLinesError extends ToolOutputUiOnlyFields { success: false; error: string; } @@ -43,6 +45,8 @@ export const createFileEditReplaceLinesTool: ToolFactory = (config: ToolConfigur return { success: true, diff: result.diff, + ui_only: result.ui_only, + warning: result.warning, edits_applied: result.edits_applied, lines_replaced: result.lines_replaced!, line_delta: result.line_delta!, diff --git a/src/node/services/tools/notify.test.ts b/src/node/services/tools/notify.test.ts index 9c3a98b335..fe6ef9d9b0 100644 --- a/src/node/services/tools/notify.test.ts +++ b/src/node/services/tools/notify.test.ts @@ -80,7 +80,7 @@ describe("notify tool", () => { expect(result.success).toBe(true); if (result.success) { - expect(result.notifiedVia).toBe("browser"); + expect(result.ui_only?.notify?.notifiedVia).toBe("browser"); expect(result.title).toBe("Test Notification"); expect(result.message).toBe("This is a test"); } @@ -100,7 +100,7 @@ describe("notify tool", () => { // In non-Electron, returns success with browser fallback expect(result.success).toBe(true); if (result.success) { - expect(result.notifiedVia).toBe("browser"); + expect(result.ui_only?.notify?.notifiedVia).toBe("browser"); expect(result.title).toBe("Test Notification"); expect(result.message).toBeUndefined(); } @@ -123,7 +123,7 @@ describe("notify tool", () => { expect(result.success).toBe(true); if (result.success) { - expect(result.workspaceId).toBe("test-workspace-123"); + expect(result.ui_only?.notify?.workspaceId).toBe("test-workspace-123"); } }); }); diff --git a/src/node/services/tools/notify.ts b/src/node/services/tools/notify.ts index 960955ebf3..222421c614 100644 --- a/src/node/services/tools/notify.ts +++ b/src/node/services/tools/notify.ts @@ -137,10 +137,14 @@ export const createNotifyTool: ToolFactory = (config) => { if (result.success) { return { success: true, - notifiedVia: "electron", title: truncatedTitle, message: truncatedMessage, - workspaceId: config.workspaceId, + ui_only: { + notify: { + notifiedVia: "electron", + workspaceId: config.workspaceId, + }, + }, }; } @@ -148,10 +152,14 @@ export const createNotifyTool: ToolFactory = (config) => { // This is not an error; the notification will be delivered via Web Notifications API return { success: true, - notifiedVia: "browser", title: truncatedTitle, message: truncatedMessage, - workspaceId: config.workspaceId, + ui_only: { + notify: { + notifiedVia: "browser", + workspaceId: config.workspaceId, + }, + }, }; }, }); diff --git a/src/node/services/workspaceService.ts b/src/node/services/workspaceService.ts index 5801f7a904..0acd55c1ea 100644 --- a/src/node/services/workspaceService.ts +++ b/src/node/services/workspaceService.ts @@ -45,6 +45,7 @@ import type { WorkspaceActivitySnapshot, } from "@/common/types/workspace"; import { isDynamicToolPart } from "@/common/types/toolParts"; +import { buildAskUserQuestionSummary } from "@/common/utils/tools/askUserQuestionSummary"; import { AskUserQuestionToolArgsSchema, AskUserQuestionToolResultSchema, @@ -1910,8 +1911,13 @@ export class WorkspaceService extends EventEmitter { } const nextOutput: AskUserQuestionToolSuccessResult = { - questions: parsedArgs.data.questions, - answers, + summary: buildAskUserQuestionSummary(answers), + ui_only: { + ask_user_question: { + questions: parsedArgs.data.questions, + answers, + }, + }, }; output = nextOutput;