fix(core): preserve shell display and recording diagnostics

ytahdn · Sep 21, 2026 · Sep 20, 2026 · Sep 20, 2026 · Sep 20, 2026 · Sep 20, 2026
commit aacd8e52f34f9c91398e45e6ee3e42e736c0b392
diff --git a/docs/design/structured-shell-results.md b/docs/design/structured-shell-results.md
@@ -4,11 +4,11 @@
 
 ## Scope
 
-Replace the Web Shell completed-result text parser with a versioned structured Shell result. Keep model-facing llmContent unchanged and keep the existing human display text for terminal consumers. Legacy records render verbatim. Background launch acknowledgments are not completed process results and retain their existing format. No streaming transport changes.
+Replace the Web Shell completed-result text parser with a versioned structured Shell result. Keep model-facing llmContent unchanged and keep the existing human display text for terminal consumers. Legacy records render verbatim. Background launch acknowledgments and early returns (including cancellation before execution) retain their existing string format. No streaming transport changes.
 
 ## Contract and flow
 
-Core produces a shell_result version 1 returnDisplay containing text, output (combined stdout/stderr), directory, exitCode, signal, pid, error, outcome, notices, truncated, and outputFiles. Outcome distinguishes completed, failed, cancelled and timed_out. Empty output is an empty string, without sentinels. Notices never become stdout. The scheduler and recording/replay retain the existing resultDisplay path; ACP exposes it as rawOutput. Web Shell validates the discriminator and fields, never parses text. Terminal and text-only consumers use text.
+Core produces a shell_result version 1 returnDisplay containing text, output (combined stdout/stderr), directory, exitCode, signal, pid, error, outcome, notices, truncated, and outputFiles. Outcome distinguishes completed, failed, cancelled and timed_out. Empty output is an empty string, without sentinels. Notices never become stdout. The scheduler and recording/replay retain the existing resultDisplay path; ACP exposes it as rawOutput. Web Shell validates the discriminator and fields, never parses text. Terminal and text-only consumers use text. The producer keeps its original human display text, including short failure messages. On non-timeout execution errors the scheduler replaces text with its final error message (including failure-hook context), matching the legacy error-display fallback; structured output remains the raw command output.
 
 ## Bounds and compatibility
 
@@ -18,6 +18,6 @@ Keep producer display text intact through PostToolUse; apply existing display co
 
 Check real producer success, nonzero exit, empty/literal output, timeout/cancellation, long-run notices, truncation, saved record replay, ACP byte bounds, terminal text, SDK preview/export and Web Shell rendering. Run scoped tests/build/typecheck and review the full diff. Existing repository build failures are reported separately.
 
-Shell outcome follows the existing exit-error policy: exit 1 from grep/rg/diff/test is a completed negative result, not an execution failure. Web Shell trusts the structured outcome and retains the numeric exit code in details. PostToolUse and PostToolBatch hooks keep string display fields through shared normalization; UI/history retain structured data. Running elapsed time is visible beside the status.
+Shell outcome follows the existing exit-error policy: exit 1 from grep/rg/diff/test is a completed negative result, not an execution failure. Web Shell trusts the structured outcome and retains the numeric exit code in details. PostToolUse and PostToolBatch hooks keep string display fields through shared normalization. Failed PostToolBatch calls already carried the scheduler error message before this change and continue to do so; UI/history retain structured data. Running elapsed time is visible beside the status.
 
 Exported version 1 documents without structured metadata render the complete text fallback without the live categorized card. The directory field is the resolved execution directory. Signal termination, cancellation and timeout do not display a potentially synthetic exit code. Container cleanup failures append a notice to compatible text and structured notices, leaving command output intact.
diff --git a/docs/design/structured-shell-results.zh-CN.md b/docs/design/structured-shell-results.zh-CN.md
@@ -4,11 +4,11 @@
 
 ## 范围
 
-用版本化结构替换 Web Shell 完成态的文本解析器。保持面向模型的 llmContent 不变，并为终端保留现有人类可读文本。旧记录原文展示。后台启动确认不属于进程完成结果，保留既有格式。不修改实时输出传输。
+用版本化结构替换 Web Shell 完成态的文本解析器。保持面向模型的 llmContent 不变，并为终端保留现有人类可读文本。旧记录原文展示。后台启动确认和提前返回（包括执行前取消）保留既有字符串格式。不修改实时输出传输。
 
 ## 契约与链路
 
-Core 产生 shell_result 版本 1 的 returnDisplay，包含 text、output（合并stdout/stderr）、directory、exitCode、signal、pid、error、outcome、notices、truncated、outputFiles。outcome 区分完成、失败、取消和超时。空输出就是空字符串，不使用占位符。提示不混入输出。调度器及记录/回放复用现有 resultDisplay 链路，ACP 通过 rawOutput 暴露。Web Shell 校验类型标识和字段，不解析文本。终端与纯文本消费者使用 text。
+Core 产生 shell_result 版本 1 的 returnDisplay，包含 text、output（合并stdout/stderr）、directory、exitCode、signal、pid、error、outcome、notices、truncated、outputFiles。outcome 区分完成、失败、取消和超时。空输出就是空字符串，不使用占位符。提示不混入输出。调度器及记录/回放复用现有 resultDisplay 链路，ACP 通过 rawOutput 暴露。Web Shell 校验类型标识和字段，不解析文本。终端与纯文本消费者使用 text。生产端保留原有人类可读展示文本，包括简短失败信息。发生非超时执行错误时，调度器将 text 替换为最终错误信息（包含失败 hook 上下文），与旧有错误展示回退一致；结构化 output 仍保留原始命令输出。
 
 ## 限额与兼容
 
@@ -18,6 +18,6 @@ Core 产生 shell_result 版本 1 的 returnDisplay，包含 text、output（合
 
 覆盖真实生产端成功、非零退出、空输出/字面值、超时/取消、长命令提示、截断、历史回放、ACP字节限额、终端文本、SDK预览/导出及Web Shell展示。执行相关测试、构建、类型检查及完整代码审查。仓库已有构建失败单独说明。
 
-Shell outcome 沿用现有退出错误策略：grep/rg/diff/test 的退出码 1 表示已完成但结果为否，不属于执行失败。Web Shell 遵循结构化 outcome，数字退出码仍显示在详情里。PostToolUse 和 PostToolBatch 通过共享归一化保持字符串展示字段；UI/历史继续保留结构化数据。运行耗时显示在状态旁，默认可见。
+Shell outcome 沿用现有退出错误策略：grep/rg/diff/test 的退出码 1 表示已完成但结果为否，不属于执行失败。Web Shell 遵循结构化 outcome，数字退出码仍显示在详情里。PostToolUse 和 PostToolBatch 通过共享归一化保持字符串展示字段。失败的 PostToolBatch 调用在改动前已经携带调度器错误信息，改动后继续保留；UI/历史继续保留结构化数据。运行耗时显示在状态旁，默认可见。
 
 版本 1 导出文档缺少结构化元数据时，展示完整回退文本，不套用实时分类卡片。directory 表示解析后的实际执行目录。信号终止、取消和超时不展示可能由执行层补出的退出码。容器清理失败提示追加到兼容文本与结构化 notices，命令输出保持原样。
@@ -70,6 +70,7 @@ import {
   MOCK_TOOL_GET_CONFIRMATION_DETAILS,
 } from '../test-utils/mock-tool.js';
 import type { MediaPolicyToolDescriptor } from '../tools/tools.js';
+import { shellResultText } from '../utils/shell-result.js';
 import { LlmChat } from './llm-chat.js';
 import { MessageBusType } from '../confirmation-bus/types.js';
 import type { HookExecutionResponse } from '../confirmation-bus/types.js';
@@ -13278,6 +13279,7 @@ describe('CoreToolScheduler telemetry spans', () => {
     tools?: AnyDeclarativeTool[];
     messageBus?: { request: ReturnType<typeof vi.fn> };
     disableHooks?: boolean;
+    hasPostToolBatchHook?: boolean;
     canUpdateOutput?: boolean;
     isInteractive?: boolean;
     inputFormat?: InputFormat;
@@ -13350,6 +13352,7 @@ describe('CoreToolScheduler telemetry spans', () => {
       getChatRecordingService: () => undefined,
       getMessageBus: vi.fn().mockReturnValue(options.messageBus),
       getDisableAllHooks: vi.fn().mockReturnValue(options.disableHooks ?? true),
+      hasHooksForEvent: () => options.hasPostToolBatchHook ?? false,
       // Confirmation-prompt capability stubs — consumed by
       // canPromptForAskBounce when a PreToolUse hook returns 'ask'.
       isInteractive: () => options.isInteractive ?? true,
@@ -13392,6 +13395,7 @@ describe('CoreToolScheduler telemetry spans', () => {
       ) => Promise<ToolResult>;
       messageBus?: { request: ReturnType<typeof vi.fn> };
       disableHooks?: boolean;
+      hasPostToolBatchHook?: boolean;
       abortController?: AbortController;
       canUpdateOutput?: boolean;
       throwSpanSetAttribute?: boolean;
@@ -13858,58 +13862,74 @@ describe('CoreToolScheduler telemetry spans', () => {
     }
   });
 
-  it('preserves failure hook context in structured shell display text', async () => {
-    const display = {
-      type: 'shell_result',
-      version: 1,
-      text: 'before',
-      output: 'before',
-      directory: '/tmp',
-      exitCode: 7,
-      signal: null,
-      pid: null,
-      error: null,
-      outcome: 'failed',
-      notices: [],
-      truncated: false,
-      outputFiles: [],
-    };
-    const messageBus = {
-      request: vi.fn(async (request: { eventName: string }) => ({
-        type: MessageBusType.HOOK_EXECUTION_RESPONSE,
-        correlationId: `${request.eventName}-hook`,
-        success: true,
-        output:
-          request.eventName === 'PostToolUseFailure'
-            ? {
-                hookSpecificOutput: {
-                  additionalContext: 'Inspect failure report',
-                },
-              }
-            : { decision: 'allow' },
-      })),
-    };
-    const { completedCalls } = await runSingleTool({
-      messageBus,
-      disableHooks: false,
-      execute: vi.fn().mockResolvedValue({
-        llmContent: 'Exit Code: 7',
-        returnDisplay: display,
-        error: {
-          message: 'Exit Code: 7',
-          type: ToolErrorType.SHELL_EXECUTE_ERROR,
-        },
-      }),
-    });
-    const call = completedCalls[0];
-    expect(call.status).toBe('error');
-    if (call.status !== 'error') throw new Error('Expected failure');
-    expect(call.response.resultDisplay).toEqual({
-      ...display,
-      text: 'Exit Code: 7\n\nInspect failure report',
-    });
-    expect(display.text).toBe('before');
-  });
+  it.each(['legacy', 'structured'])(
+    'preserves failure display and batch payload with %s shell results',
+    async (format) => {
+      const display = {
+        type: 'shell_result',
+        version: 1,
+        text: 'before',
+        output: 'before',
+        directory: '/tmp',
+        exitCode: 7,
+        signal: null,
+        pid: null,
+        error: null,
+        outcome: 'failed',
+        notices: [],
+        truncated: false,
+        outputFiles: [],
+      };
+      const messageBus = {
+        request: vi.fn(async (request: { eventName: string }) => ({
+          type: MessageBusType.HOOK_EXECUTION_RESPONSE,
+          correlationId: `${request.eventName}-hook`,
+          success: true,
+          output:
+            request.eventName === 'PostToolUseFailure'
+              ? {
+                  hookSpecificOutput: {
+                    additionalContext: 'Inspect failure report',
+                  },
+                }
+              : { decision: 'allow' },
+        })),
+      };
+      const { completedCalls } = await runSingleTool({
+        messageBus,
+        disableHooks: false,
+        hasPostToolBatchHook: true,
+        execute: vi.fn().mockResolvedValue({
+          llmContent: 'Exit Code: 7',
+          returnDisplay: format === 'legacy' ? display.text : display,
+          error: {
+            message: 'Exit Code: 7',
+            type: ToolErrorType.SHELL_EXECUTE_ERROR,
+          },
+        }),
+      });
+      const call = completedCalls[0];
+      expect(call.status).toBe('error');
+      if (call.status !== 'error') throw new Error('Expected failure');
+      const expectedText = 'Exit Code: 7\n\nInspect failure report';
+      expect(call.response.resultDisplay).toEqual(
+        format === 'legacy' ? expectedText : { ...display, text: expectedText },
+      );
+      const batch = messageBus.request.mock.calls.find(
+        ([request]) => request.eventName === 'PostToolBatch',
+      )?.[0] as
+        | {
+            input: {
+              tool_calls: Array<{ tool_response: Record<string, unknown> }>;
+            };
+          }
+        | undefined;
+      const response = batch?.input.tool_calls[0].tool_response;
+      expect(response?.['error']).toBe(expectedText);
+      expect(shellResultText(response?.['result_display'])).toBe(expectedText);
+      expect(display.text).toBe('before');
+    },
+  );
 
   it('preserves successful execution when cancellation arrives during PostToolUse', async () => {
     const abortController = new AbortController();

@@ -6638,6 +6638,7 @@ export class CoreToolScheduler {
         }
 
         const error = new Error(errorMessage);
+        // Match createErrorResponse's legacy string fallback, including failure-hook context.
         let errorResponse = createErrorResponse(
           scheduledCall.request,
           error,

@@ -1436,9 +1436,13 @@ describe('HookEventHandler', () => {
     });
   });
 
-  it.each(['use', 'batch'])(
-    'preserves shell text for %s hooks without mutating the UI result',
-    async (event) => {
+  it.each([
+    ['use', 'completed'],
+    ['batch', 'completed'],
+    ['batch', 'failed'],
+  ] as const)(
+    'preserves shell text for %s hooks with %s results without mutating the UI result',
+    async (event, outcome) => {
       vi.mocked(mockHookPlanner.createExecutionPlan).mockReturnValue(
         createMockExecutionPlan([
           {
@@ -1455,14 +1459,14 @@ describe('HookEventHandler', () => {
       const display = Object.freeze({
         type: 'shell_result',
         version: 1,
-        text: 'line one\nline two',
+        text: outcome === 'failed' ? 'Exit Code: 7' : 'line one\nline two',
         output: 'line one\nline two',
         directory: '/tmp',
-        exitCode: 0,
+        exitCode: outcome === 'failed' ? 7 : 0,
         signal: null,
         pid: 42,
         error: null,
-        outcome: 'completed',
+        outcome,
         notices: [],
         truncated: false,
         outputFiles: [],
@@ -1485,7 +1489,7 @@ describe('HookEventHandler', () => {
             tool_name: 'run_shell_command',
             tool_input: {},
             tool_use_id: 'shell-1',
-            status: 'success',
+            status: outcome === 'failed' ? 'error' : 'success',
             tool_response: response,
           },
         ]);

@@ -46,6 +46,10 @@ import type {
   GoalStateRecordPayloadV2,
   GoalTurnPermit,
 } from '../goals/goal-protocol.js';
+import {
+  shellResultText,
+  type ShellResultDisplay,
+} from '../utils/shell-result.js';
 import type { ToolResultBoundaryObservation } from '../tools/tool-result-boundary-diagnostics.js';
 
 function branchTestRecord(
@@ -2395,6 +2399,56 @@ describe('ChatRecordingService', () => {
       }
     });
 
+    it.each(['', 'small display', 'x'.repeat(40_000)])(
+      'observes structured shell display before and after recording (case %#)',
+      async (text) => {
+        const display: ShellResultDisplay = {
+          type: 'shell_result',
+          version: 1,
+          text,
+          output: text,
+          directory: '/tmp',
+          exitCode: 0,
+          signal: null,
+          pid: null,
+          error: null,
+          outcome: 'completed',
+          notices: [],
+          truncated: false,
+          outputFiles: [],
+        };
+        chatRecordingService.recordToolResult([{ text: 'model response' }], {
+          callId: 'shell-1',
+          status: 'success',
+          resultDisplay: display,
+        });
+        await chatRecordingService.flush();
+
+        const record = vi.mocked(jsonl.writeLine).mock
+          .calls[0][1] as ChatRecord;
+        const savedText = shellResultText(record.toolCallResult?.resultDisplay);
+        expect(savedText).toBeDefined();
+        expect(savedText!.length).toBeLessThanOrEqual(
+          MAX_RETAINED_TOOL_RESULT_DISPLAY_CHARS,
+        );
+        for (const [stage, value] of [
+          ['recorder_input', text],
+          ['recorder_output', savedText],
+        ]) {
+          const observation = boundaryObserveMock.mock.calls.find(
+            ([entry]) => entry.stage === stage,
+          )?.[0];
+          const values = observation?.values;
+          expect(
+            (typeof values === 'function' ? values() : values)?.filter(
+              (entry) => entry.representation === 'display',
+            ),
+          ).toEqual([{ representation: 'display', value }]);
+        }
+        expect(display.text).toBe(text);
+      },
+    );
+
     it('should keep small file diff resultDisplay unchanged', async () => {
       const toolResultParts: Part[] = [
         {

@@ -2437,11 +2437,11 @@ export class ChatRecordingService {
         mutated,
         values: () => [
           ...toolResultPartDiagnosticValues(message),
-          ...(typeof outputDisplay === 'string'
+          ...(shellResultText(outputDisplay) !== undefined
             ? [
                 {
                   representation: 'display' as const,
-                  value: outputDisplay,
+                  value: shellResultText(outputDisplay)!,
                 },
               ]
             : []),