From 0f50b5aba5520af13e4b1b332711082d71ee9b6c Mon Sep 17 00:00:00 2001 From: Derek Hearst Date: Wed, 23 Sep 2026 16:39:05 -0600 Subject: [PATCH 01/11] fix(engine): name the shell tools the way the bundled CLI does `BashOutput` is only the Bash tool's output type in SDK 0.3.278, the CLI removed the polling tool (and `TaskOutput`), and `KillShell` / `KillBash` are aliases of `TaskStop`. The shell list now says `Bash` and `TaskStop`; the old names stay recognised as built-ins so a stored agent config that uses them is not turned into an MCP name, and a scope drops the removed ones instead of handing them to the CLI. The guard holds only `Bash` for approval on a host with no sandbox, and only `Bash` is distilled into a terminal card. Co-Authored-By: Claude Opus 5.5 --- src/lib/engine/builtin-tools.ts | 25 +++++++++++++++++++---- src/lib/engine/tool-result-details.ts | 6 +++++- src/lib/engine/tool-scope.ts | 4 +++- src/lib/engine/workspace-guard.ts | 10 +++++++-- src/routes/chat/[id]/stop-task/+server.ts | 6 +++--- tests/chat.stop-task.spec.ts | 6 +++--- tests/engine.tool-decision.spec.ts | 14 ++++++++++++- tests/engine.tool-result-details.spec.ts | 8 ++++++++ tests/engine.workspace-guard.spec.ts | 11 ++++++++++ 9 files changed, 75 insertions(+), 15 deletions(-) diff --git a/src/lib/engine/builtin-tools.ts b/src/lib/engine/builtin-tools.ts index 2d57ec7c..27c0f013 100644 --- a/src/lib/engine/builtin-tools.ts +++ b/src/lib/engine/builtin-tools.ts @@ -13,11 +13,24 @@ export const BUILTIN_FILE_TOOLS = ['Read', 'Write', 'Edit', 'MultiEdit', 'Glob', 'Grep'] as const /** - * `TaskStop` is what the CLI calls `KillShell` now (see `LEGACY_TOOL_NAMES`). `BashOutput` - * no longer exists in the bundled CLI at all — a background command's output is read with - * `Read` — and stays listed only so an agent configured with it keeps validating. + * The shell surface as the bundled CLI names it (#35): `Bash` runs a command, in the + * foreground or with `run_in_background`, and `TaskStop` stops a background one. + * + * `BashOutput` is not a tool any more. In `sdk-tools.d.ts` it is only the *output type* of + * `Bash`; the CLI removed the polling tool (and `TaskOutput` after it — `sdk.d.ts` says to + * read a background task's output file instead). `KillShell` / `KillBash` are aliases the CLI + * resolves to `TaskStop` (`LEGACY_TOOL_NAMES`). Old names an agent may still be configured + * with are kept recognisable below, so they are not mistaken for our own MCP tools. + */ +export const BUILTIN_SHELL_TOOLS = ['Bash', 'TaskStop'] as const + +/** + * Built-ins the bundled CLI no longer has. Still recognised as built-ins, so a stored agent + * config that names one stays a bare name instead of becoming `mcp__agentstudio__BashOutput`, + * but never handed to the CLI as part of a tool scope (`./tool-scope`): there is nothing for + * it to enable. */ -export const BUILTIN_SHELL_TOOLS = ['Bash', 'BashOutput', 'KillShell', 'TaskStop'] as const +export const REMOVED_BUILTIN_TOOLS: ReadonlySet = new Set(['BashOutput', 'TaskOutput']) /** * Built-ins we deliberately refuse, because an in-house tool does the same job *and* more. @@ -65,6 +78,10 @@ export const BUILTIN_TOOL_SET: ReadonlySet = new Set([ ...BUILTIN_SHELL_TOOLS, 'NotebookEdit', 'TodoWrite', + // Old spellings a stored config may still use — see `BUILTIN_SHELL_TOOLS`. + 'KillShell', + 'KillBash', + ...REMOVED_BUILTIN_TOOLS, ]) /** diff --git a/src/lib/engine/tool-result-details.ts b/src/lib/engine/tool-result-details.ts index d5a72a37..280f57d9 100644 --- a/src/lib/engine/tool-result-details.ts +++ b/src/lib/engine/tool-result-details.ts @@ -113,7 +113,11 @@ export type ToolResultDetails = FileEditDetails | ShellDetails | TodoDetails /** Built-ins whose output this module knows how to distil. */ const FILE_EDIT_TOOLS = new Set(['Edit', 'MultiEdit']) const FILE_WRITE_TOOLS = new Set(['Write']) -const SHELL_TOOLS = new Set(['Bash', 'BashOutput', 'KillShell']) +/** + * Only `Bash`. `BashOutput` and `TaskOutput` were removed from the CLI, and `TaskStop` (the + * old `KillShell`) answers with `{ message, task_id, task_type }` — no streams to render. + */ +const SHELL_TOOLS = new Set(['Bash']) export function hasToolResultDetails(toolName: string): boolean { return ( diff --git a/src/lib/engine/tool-scope.ts b/src/lib/engine/tool-scope.ts index 3cd61727..e45dc5fd 100644 --- a/src/lib/engine/tool-scope.ts +++ b/src/lib/engine/tool-scope.ts @@ -32,7 +32,7 @@ * Pure and dependency-free apart from the name lists, so a spec can import it. */ -import { BUILTIN_TOOL_SET, SUBAGENT_TOOL, canonicalToolName } from './builtin-tools' +import { BUILTIN_TOOL_SET, REMOVED_BUILTIN_TOOLS, SUBAGENT_TOOL, canonicalToolName } from './builtin-tools' export type ToolScope = { /** Every tool the run may call, by bare canonical name (our own MCP namespace stripped). */ @@ -65,6 +65,8 @@ export function resolveToolScope( for (const raw of names) { const name = canonicalToolName(String(raw ?? '').trim()) if (!name) continue + // `BashOutput` and friends: the CLI has nothing by that name to enable any more. + if (REMOVED_BUILTIN_TOOLS.has(name)) continue if (isBuiltin(name)) { if (!builtins.includes(name)) builtins.push(name) } else { diff --git a/src/lib/engine/workspace-guard.ts b/src/lib/engine/workspace-guard.ts index de1c262f..cab107a6 100644 --- a/src/lib/engine/workspace-guard.ts +++ b/src/lib/engine/workspace-guard.ts @@ -80,8 +80,14 @@ const HOME_PREFIX = /^~[A-Za-z0-9._-]*(?:[\\/]|$)/ /** A `..` component, with either slash (a stricter reading than POSIX needs, never looser). */ const PARENT_SEGMENT = /(?:^|[\\/])\.\.(?:[\\/]|$)/ -/** Tools that run a command rather than touch a named path. Not decidable from arguments. */ -const COMMAND_TOOLS = new Set(['Bash', 'BashOutput', 'KillShell']) +/** + * Tools that run a command rather than touch a named path. Not decidable from arguments. + * + * Only `Bash`. `TaskStop` (the CLI's name for `KillShell`) names no path and can only stop a + * task this session started, so it is not held for approval under `bashPolicy: 'ask'`; + * `BashOutput` no longer exists as a tool (see `./builtin-tools`). + */ +const COMMAND_TOOLS = new Set(['Bash']) /** * Tools that can create, change or remove a file, and the arguments that name it. diff --git a/src/routes/chat/[id]/stop-task/+server.ts b/src/routes/chat/[id]/stop-task/+server.ts index 6d07ca28..4fddd127 100644 --- a/src/routes/chat/[id]/stop-task/+server.ts +++ b/src/routes/chat/[id]/stop-task/+server.ts @@ -8,9 +8,9 @@ import { logger } from '$lib/observability/logger' /** * Stop one background task (#35). * - * The model can already background a command — `BUILTIN_SHELL_TOOLS` allow-lists `Bash`, - * `BashOutput` and `KillShell`, and the SDK reports every live task through - * `background_tasks_changed`. What was missing is the other direction: a way to stop one. + * The model can already background a command — `Bash` takes `run_in_background`, `TaskStop` + * stops one, and the SDK reports every live task through `background_tasks_changed`. What was + * missing is the other direction: a way for the user to stop one. * `Query.stopTask(id)` is a control request on the live session, so it needs the handle the * run registry publishes — which is exactly why the registry exists, since "stop" arrives * as a separate HTTP call from the run it is stopping. diff --git a/tests/chat.stop-task.spec.ts b/tests/chat.stop-task.spec.ts index 21ba9ded..553411fa 100644 --- a/tests/chat.stop-task.spec.ts +++ b/tests/chat.stop-task.spec.ts @@ -11,9 +11,9 @@ import { /** * #35 — stopping one background task. * - * The model has been able to background a `Bash` command since #15 (`BUILTIN_SHELL_TOOLS` - * allow-lists `Bash` / `BashOutput` / `KillShell`), and the SDK reports every live task - * through `background_tasks_changed`. The missing direction was stopping one: + * The model has been able to background a `Bash` command since #15 (`run_in_background`, + * stopped with `TaskStop`), and the SDK reports every live task through + * `background_tasks_changed`. The missing direction was stopping one: * `Query.stopTask(id)` is a control request on the live session, which is only reachable * through the run registry. * diff --git a/tests/engine.tool-decision.spec.ts b/tests/engine.tool-decision.spec.ts index 2a23efbf..eb6b1766 100644 --- a/tests/engine.tool-decision.spec.ts +++ b/tests/engine.tool-decision.spec.ts @@ -17,7 +17,7 @@ import { readFileSync } from 'node:fs' import { resolve } from 'node:path' import { expect, test } from '@playwright/test' import { READ_ONLY_TOOL_NAMES } from '../src/lib/agents/builtin-agents.server' -import { SUBAGENT_TOOL } from '../src/lib/engine/builtin-tools' +import { BUILTIN_SHELL_TOOLS, BUILTIN_TOOL_SET, SUBAGENT_TOOL } from '../src/lib/engine/builtin-tools' import { decideToolCall, type ToolDecisionContext } from '../src/lib/engine/tool-decision' import { isToolInScope, resolveToolScope, scopeBuiltinTools } from '../src/lib/engine/tool-scope' @@ -73,6 +73,18 @@ test.describe('tool scope', () => { expect(scope!.inHouse.size).toBe(0) }) + test('a list naming a tool the CLI removed keeps it out of the scope, and out of ours (#35)', () => { + // `BashOutput` is only the Bash tool's output type now, and `TaskOutput` is gone too. + // Neither is ours either, so neither may become `mcp__agentstudio__BashOutput`. + expect([...BUILTIN_SHELL_TOOLS]).toEqual(['Bash', 'TaskStop']) + const scope = resolveToolScope(['Bash', 'BashOutput', 'TaskOutput', 'KillShell'], { delegation: false }) + expect(scopeBuiltinTools(scope)).toEqual(['Bash', 'TaskStop']) + expect(scope!.inHouse.size).toBe(0) + for (const name of ['BashOutput', 'TaskOutput', 'KillShell', 'KillBash']) { + expect(BUILTIN_TOOL_SET.has(name), name).toBe(true) + } + }) + test("the read-only agents' scope actually leaves out the tools that change things", () => { // This is what `allowedTools` never did: Bash and Edit were simply not removed. const scope = resolveToolScope(READ_ONLY_TOOL_NAMES, { delegation: false }) diff --git a/tests/engine.tool-result-details.spec.ts b/tests/engine.tool-result-details.spec.ts index 3e1282d5..69c7f9ad 100644 --- a/tests/engine.tool-result-details.spec.ts +++ b/tests/engine.tool-result-details.spec.ts @@ -175,6 +175,14 @@ test.describe('shell', () => { // Something else wearing the name is not a shell result. expect(toolResultDetails('Bash', { somethingElse: true })).toBeNull() }) + + test('only Bash is a terminal: the removed polling tools and TaskStop are not (#35)', () => { + const streams = { stdout: 'x', stderr: '', interrupted: false } + expect(toolResultDetails('BashOutput', streams)).toBeNull() + expect(toolResultDetails('KillShell', streams)).toBeNull() + // TaskStop's own output has no streams to show. + expect(toolResultDetails('TaskStop', { message: 'Stopped', task_id: 'b1', task_type: 'local_bash' })).toBeNull() + }) }) test.describe('todos', () => { diff --git a/tests/engine.workspace-guard.spec.ts b/tests/engine.workspace-guard.spec.ts index d08e1745..128ff6da 100644 --- a/tests/engine.workspace-guard.spec.ts +++ b/tests/engine.workspace-guard.spec.ts @@ -132,6 +132,17 @@ test('Bash never runs outside the sandbox, whatever the policy says', () => { ).toBe('allow') }) +test('stopping a background task is not a shell command (#35)', () => { + // `TaskStop` is the CLI's name for the old `KillShell`: it names no path and can only stop + // a task this session started, so a host with no sandbox does not ask before it — while a + // `Bash` call on the same host still does. + const WS = process.platform === 'win32' ? 'C:\\sandbox\\user-aaa' : '/sandbox/user-aaa' + const g = (toolName: string, toolInput: unknown) => + guardWorkspaceAccess({ toolName, toolInput, workspaceRoot: WS, bashPolicy: 'ask' }).verdict + expect(g('TaskStop', { task_id: 'b1' })).toBe('allow') + expect(g('Bash', { command: 'ls' })).toBe('ask') +}) + test.describe("the agent's own configuration needs approval to change", () => { // A trusted project's `.claude/settings.json` is loaded by the next run, and its hooks // run as the app user outside the sandbox. An agent that could rewrite it silently could From 0603603f09aa8c0e0e1645e6fca967e5d0e8cf91 Mon Sep 17 00:00:00 2001 From: Derek Hearst Date: Wed, 23 Sep 2026 19:59:48 -0600 Subject: [PATCH 02/11] feat(engine): a failed command is still a terminal, with its exit code (#26) A non-zero exit makes the CLI throw, so a failed `Bash` has no `BashOutput`: `tool_use_result` is the string `Error: Exit code N` followed by the merged output. That fell back to the generic card, which is exactly the case where the output matters most. Only the CLI's first line is parsed; anything else (a denial, a pre-spawn error) still goes to the generic card. `ShellDetails` gains optional `exitCode`, `returnCodeInterpretation` (grep's "No matches found") and a background `status`, all optional so persisted blocks still type-check. `appendStreamTail` grows an already-capped stream to the same 16k tail `clipTail` keeps, for the live output that follows. Co-Authored-By: Claude Opus 5.5 --- src/lib/engine/tool-result-details.ts | 81 +++++++++++++++++++++++- tests/engine.tool-result-details.spec.ts | 46 ++++++++++++++ 2 files changed, 125 insertions(+), 2 deletions(-) diff --git a/src/lib/engine/tool-result-details.ts b/src/lib/engine/tool-result-details.ts index 280f57d9..a44e9c3c 100644 --- a/src/lib/engine/tool-result-details.ts +++ b/src/lib/engine/tool-result-details.ts @@ -70,9 +70,18 @@ export type FileEditDetails = { truncated: boolean } +/** + * Where a backgrounded command is in its life (#35). + * + * Background commands end with the turn that started them — the engine closes the CLI after + * each turn, and the CLI's own processes go with it — so a command that is still running when + * the reply ends is `ended_with_turn`, not `running` forever. + */ +export type BackgroundShellStatus = 'running' | 'completed' | 'failed' | 'stopped' | 'ended_with_turn' + export type ShellDetails = { kind: 'shell' - /** Bash, BashOutput or KillShell. */ + /** Always `Bash` now; kept as data because older persisted blocks name other tools. */ tool: string command: string | null /** The model's own one-line description of the command, when it supplied one. */ @@ -80,7 +89,7 @@ export type ShellDetails = { stdout: string stderr: string interrupted: boolean - /** Present when the command was backgrounded; the handle `BashOutput` / `KillShell` take. */ + /** Present when the command was backgrounded: the task id `TaskStop` and the stop chip take. */ backgroundTaskId: string | null /** Set when the command hit its timeout and the CLI auto-backgrounded it. */ timedOutAfterMs: number | null @@ -88,6 +97,20 @@ export type ShellDetails = { persistedOutputPath: string | null /** True when either stream was clipped to `MAX_STREAM_CHARS` (the tail is what was kept). */ truncated: boolean + /* + * Everything below is optional so that blocks persisted before it existed still type-check + * and render exactly as they did. + */ + /** + * The exit code, when the CLI reported one. It does not for a command that succeeded in + * the foreground — `BashOutput` has no such field — only for one that failed (its result + * begins `Exit code N`) and for a background command's completion (`… (exit code N)`). + */ + exitCode?: number | null + /** The CLI's reading of a non-error exit code with a meaning, e.g. grep's 1: "No matches found". */ + returnCodeInterpretation?: string | null + /** A backgrounded command's status, updated live while its turn runs. Absent in the foreground. */ + background?: { status: BackgroundShellStatus } | null } export type TodoStatus = 'pending' | 'in_progress' | 'completed' @@ -149,6 +172,58 @@ function clipTail(value: unknown): { text: string; truncated: boolean } { return { text: value.slice(value.length - MAX_STREAM_CHARS), truncated: true } } +/** + * Add live output to a stream that is already capped, keeping the same tail `clipTail` keeps. + * + * Shared by the engine, which grows a background command's persisted block as its output + * file grows, and by the chat page, which grows the card from the `shell_output` frames — so + * the two cannot disagree about what the last 16k characters are. + */ +export function appendStreamTail(current: string, chunk: string): { text: string; truncated: boolean } { + return clipTail(`${current}${chunk}`) +} + +/** + * A failed `Bash` call, as the CLI reports one. + * + * A non-zero exit makes the CLI throw, so there is no `BashOutput` object: `tool_use_result` + * is the string `Error: Exit code N` followed by the command's output, and the tool_result + * the model reads is the same text inside ``. The first line is the CLI's own + * and the only thing parsed; everything after it is shown as it came. Anything else — a + * denial, a pre-spawn error — is not a command's output and is left to the generic card. + */ +const FAILED_COMMAND = /^Exit code (-?\d+)[ \t]*(?:\r?\n|$)/ + +function failedShellDetails( + toolName: string, + result: string, + args: Record | null, +): ShellDetails | null { + const body = result + .replace(/^Error: /, '') + .replace(/^([\s\S]*)<\/tool_use_error>\s*$/, '$1') + const match = FAILED_COMMAND.exec(body) + if (!match) return null + const exitCode = Number(match[1]) + if (!Number.isSafeInteger(exitCode)) return null + // The CLI merges the streams before it throws, so this is one stream, not stderr. + const output = clipTail(body.slice(match[0].length)) + return { + kind: 'shell', + tool: toolName, + command: str(args?.command), + description: str(args?.description), + stdout: output.text, + stderr: '', + interrupted: false, + backgroundTaskId: null, + timedOutAfterMs: null, + persistedOutputPath: null, + truncated: output.truncated, + exitCode, + } +} + function readHunks(value: unknown): DiffHunk[] { if (!Array.isArray(value)) return [] const hunks: DiffHunk[] = [] @@ -264,6 +339,7 @@ function shellDetails( timedOutAfterMs: num(result.timedOutAfterMs), persistedOutputPath: str(result.persistedOutputPath) ?? str(result.rawOutputPath), truncated: stdout.truncated || stderr.truncated, + returnCodeInterpretation: str(result.returnCodeInterpretation), } } @@ -327,6 +403,7 @@ export function toolResultDetails( return result ? fileEditDetails(toolName, result, args) : null } if (SHELL_TOOLS.has(toolName)) { + if (typeof toolUseResult === 'string') return failedShellDetails(toolName, toolUseResult, args) return result ? shellDetails(toolName, result, args) : null } if (toolName === 'TodoWrite') { diff --git a/tests/engine.tool-result-details.spec.ts b/tests/engine.tool-result-details.spec.ts index 69c7f9ad..40f76f40 100644 --- a/tests/engine.tool-result-details.spec.ts +++ b/tests/engine.tool-result-details.spec.ts @@ -3,6 +3,7 @@ import { MAX_DIFF_LINES, MAX_STREAM_CHARS, MAX_TODO_ITEMS, + appendStreamTail, hasToolResultDetails, toolResultDetails, } from '../src/lib/engine/tool-result-details' @@ -176,6 +177,51 @@ test.describe('shell', () => { expect(toolResultDetails('Bash', { somethingElse: true })).toBeNull() }) + test('a failed command is still a terminal, with the exit code the CLI reported (#26)', () => { + // A non-zero exit makes the CLI throw: there is no BashOutput, only its error text — + // `Exit code N` on the first line, then the command's merged output. + const details = toolResultDetails( + 'Bash', + 'Error: Exit code 2\nnpm ERR! Missing script: "tset"\n', + { command: 'npm run tset', description: 'Run the tests' }, + ) + expect(details?.kind).toBe('shell') + if (details?.kind !== 'shell') return + expect(details.exitCode).toBe(2) + expect(details.command).toBe('npm run tset') + expect(details.stdout).toBe('npm ERR! Missing script: "tset"\n') + expect(details.stderr).toBe('') + + // The same text inside the model-facing wrapper reads the same way. + const wrapped = toolResultDetails('Bash', 'Exit code 1\nboom', {}) + expect(wrapped?.kind === 'shell' && wrapped.exitCode).toBe(1) + expect(wrapped?.kind === 'shell' && wrapped.stdout).toBe('boom') + }) + + test('an error that is not a command exiting stays on the generic card', () => { + expect(toolResultDetails('Bash', 'Error: Path is outside this run\'s workspace: /etc', {})).toBeNull() + // "Exit code" later in the text is the command talking, not the CLI. + expect(toolResultDetails('Bash', 'Error: something\nExit code 3', {})).toBeNull() + }) + + test("the CLI's reading of a special exit code is kept", () => { + const details = toolResultDetails('Bash', { + stdout: '', + stderr: '', + interrupted: false, + returnCodeInterpretation: 'No matches found', + }) + expect(details?.kind === 'shell' && details.returnCodeInterpretation).toBe('No matches found') + }) + + test('live output grows the same tail the result keeps', () => { + expect(appendStreamTail('ab', 'cd')).toEqual({ text: 'abcd', truncated: false }) + const grown = appendStreamTail('x'.repeat(MAX_STREAM_CHARS), 'END') + expect(grown.truncated).toBe(true) + expect(grown.text).toHaveLength(MAX_STREAM_CHARS) + expect(grown.text.endsWith('END')).toBe(true) + }) + test('only Bash is a terminal: the removed polling tools and TaskStop are not (#35)', () => { const streams = { stdout: 'x', stderr: '', interrupted: false } expect(toolResultDetails('BashOutput', streams)).toBeNull() From ca00468c3acd6b324a05ddaa236eeb3e24888c64 Mon Sep 17 00:00:00 2001 From: Derek Hearst Date: Wed, 23 Sep 2026 19:59:48 -0600 Subject: [PATCH 03/11] feat(engine): stream a background command's output while the turn runs (#35) The SDK sends no output for a running command (`tool_progress` is elapsed seconds only) and the `BashOutput` polling tool is gone, but the CLI writes a backgrounded command's output to `//tasks/.output` and names the path in the call's result. The engine now tails that file on the host, once a second, and sends each new piece as a live-only `shell_output` frame keyed on the call, growing the persisted block to the same tail. The path is trusted only when it is absolute, has no `..`, ends `/tasks/.output` with both ids the CLI's own, is the only such path in the text, and on every read is still a regular file with one link whose real path (through the containment helper) keeps that shape. Anything else means no live output, never an error in the run loop. `task_notification` now keeps `task_id`, `tool_use_id`, `output_file` and the summary's exit code, and settles the card with a persisted `shell_task_done` that carries the final output. A command still running when the turn ends gets a final read, is marked `ended_with_turn`, and the turn gets one notice listing what was stopped: the engine closes the CLI after every turn, and its background commands go with it. A subagent's commands are left alone. Co-Authored-By: Claude Opus 5.5 --- src/lib/engine/background-shells.server.ts | 212 ++++++++++++++++ src/lib/engine/sdk-notices.ts | 59 ++++- src/lib/engine/stream.server.ts | 29 ++- src/lib/engine/task-output.server.ts | 273 +++++++++++++++++++++ src/routes/chat/[id]/stream/+server.ts | 6 +- tests/engine.sdk-notices.spec.ts | 59 +++++ tests/engine.stream-routing.spec.ts | 248 +++++++++++++++++++ tests/engine.task-output.spec.ts | 234 ++++++++++++++++++ 8 files changed, 1110 insertions(+), 10 deletions(-) create mode 100644 src/lib/engine/background-shells.server.ts create mode 100644 src/lib/engine/task-output.server.ts create mode 100644 tests/engine.task-output.spec.ts diff --git a/src/lib/engine/background-shells.server.ts b/src/lib/engine/background-shells.server.ts new file mode 100644 index 00000000..bcc2dfa7 --- /dev/null +++ b/src/lib/engine/background-shells.server.ts @@ -0,0 +1,212 @@ +/** + * Backgrounded `Bash` commands, followed from the moment they start to the end of the turn (#35). + * + * The model can run a command with `run_in_background`; the call returns at once with a + * task id, and the command keeps going. This keeps that command's card honest while it does: + * + * - **Live output.** The command's output file is tailed on the host (`./task-output.server`) + * and each new piece goes out as a `shell_output` frame against the call's id. The same + * text grows the call's persisted block, capped to its tail like any other shell output. + * - **How it ended.** `task_notification` settles it — completed, failed or stopped, with the + * exit code when the CLI's summary gives one — as a `shell_task_done` frame. That frame is + * persisted and carries the final output, so a client that reconnected mid-turn (and missed + * the live-only `shell_output` frames) still ends up with what the command printed. + * - **The turn ending first.** Background commands are turn-scoped here: the engine closes + * the CLI after every turn, and the commands it started end with it. A command still running + * when the reply is done is marked `ended_with_turn`, and the turn gets one notice saying so, + * rather than a card that says "running" forever. + * + * `background_tasks_changed` is deliberately not used to settle anything. The SDK calls it a + * level signal with ids only, and says not to correlate it with the `task_started` / + * `task_notification` edges — so the header chips follow it, and this follows the edges. + * + * A subagent's background commands are not followed: they are owned by the subagent and end + * with its final response, and their calls live in the subagent's card, not the parent's. + * + * Never throws into the run loop. A command whose output file cannot be found or trusted + * simply has no live output; its card still settles. + */ + +import { appendStreamTail, MAX_STREAM_CHARS, type BackgroundShellStatus, type ShellDetails } from './tool-result-details' +import type { RunNotice, TaskSettled } from './sdk-notices' +import { isTaskOutputPath, tailTaskOutput, taskOutputPathFromResult, type TaskOutputTail } from './task-output.server' +import { logger } from '../observability/logger' + +type Emit = (event: string, payload: unknown) => Promise + +/** `shell_output` — a piece of a background command's output, live-only. */ +export type ShellOutputFrame = { + /** The `Bash` call's tool_use id — the id the chat keys the card on. */ + id: string + taskId: string + /** New output. At most `MAX_STREAM_CHARS`: anything before that would be cut from the tail anyway. */ + chunk: string + /** Replace what the card shows with `chunk` instead of adding to it. */ + reset: boolean + /** Whether the card's output is now only a tail. */ + truncated: boolean + /** Characters of output streamed before this chunk, since the last reset — lets a client see a gap. */ + from: number + /** …and after it. */ + to: number +} + +/** `shell_task_done` — a background command settled, or its turn ended. Persisted. */ +export type ShellTaskDoneFrame = { + id: string + taskId: string + status: Exclude + exitCode: number | null + /** The command's output as the card should finally show it (already capped). */ + stdout: string + truncated: boolean +} + +type TrackedShell = { + toolUseId: string + taskId: string + sessionId: string | null + /** The object on the call's block — mutated in place, so the persisted block follows along. */ + details: ShellDetails + tail: TaskOutputTail | null + /** Characters streamed since the last reset. */ + streamed: number + settled: boolean +} + +export type BackgroundShells = { + /** + * Start following a parent `Bash` call that was backgrounded. Marks `details` as running + * before the call's `tool_result` frame goes out, so the card opens in its live state. + */ + track(call: { toolUseId: string; sessionId: string | null; resultText: string; details: ShellDetails }): void + /** A `task_notification` arrived. No-op for a task this is not following. */ + settle(task: TaskSettled): Promise + /** + * The turn is over: read each still-running command's output one last time and mark it + * `ended_with_turn`. Returns the notice to add to the turn, or null when nothing was running. + */ + endTurn(): Promise + /** Stop every tail without reading. For the engine's `finally`; idempotent. */ + stopAll(): Promise +} + +/** Longest list of commands the end-of-turn notice names. */ +const MAX_NOTICE_DETAIL = 300 + +export function createBackgroundShells(input: { + emit: Emit + /** Only a spec changes this. */ + pollMs?: number +}): BackgroundShells { + const shells = new Map() + + const applyChunk = async (entry: TrackedShell, chunk: string, reset: boolean, skipped: boolean) => { + const from = reset ? 0 : entry.streamed + const next = reset ? appendStreamTail('', chunk) : appendStreamTail(entry.details.stdout, chunk) + entry.details.stdout = next.text + entry.details.truncated = next.truncated || skipped || (!reset && entry.details.truncated) + entry.streamed = from + chunk.length + const frame: ShellOutputFrame = { + id: entry.toolUseId, + taskId: entry.taskId, + chunk: chunk.length > MAX_STREAM_CHARS ? chunk.slice(-MAX_STREAM_CHARS) : chunk, + reset, + truncated: entry.details.truncated, + from, + to: entry.streamed, + } + await input.emit('shell_output', frame) + } + + const startTail = (entry: TrackedShell, path: string): TaskOutputTail | null => { + if (!entry.sessionId) return null + return tailTaskOutput({ + path, + anchor: { sessionId: entry.sessionId, taskId: entry.taskId }, + intervalMs: input.pollMs, + onChunk: (chunk, info) => applyChunk(entry, chunk, info.reset, info.skipped), + onGiveUp: (reason) => + logger.warn('[engine] no live output for a background command', { taskId: entry.taskId, reason }), + }) + } + + const close = async (entry: TrackedShell, status: ShellTaskDoneFrame['status'], exitCode: number | null) => { + entry.settled = true + entry.details.background = { status } + if (exitCode !== null) entry.details.exitCode = exitCode + const frame: ShellTaskDoneFrame = { + id: entry.toolUseId, + taskId: entry.taskId, + status, + exitCode, + stdout: entry.details.stdout, + truncated: entry.details.truncated, + } + await input.emit('shell_task_done', frame) + } + + return { + track({ toolUseId, sessionId, resultText, details }) { + const taskId = details.backgroundTaskId + if (!taskId || shells.has(taskId)) return + details.background = { status: 'running' } + const entry: TrackedShell = { + toolUseId, + taskId, + sessionId, + details, + tail: null, + streamed: 0, + settled: false, + } + shells.set(taskId, entry) + const path = sessionId ? taskOutputPathFromResult(resultText, { sessionId, taskId }) : null + if (path) entry.tail = startTail(entry, path) + else logger.info('[engine] background command has no readable output path', { taskId }) + }, + + async settle(task) { + const entry = + shells.get(task.taskId) ?? + (task.toolUseId ? [...shells.values()].find((e) => e.toolUseId === task.toolUseId) : undefined) + if (!entry || entry.settled) return + // The result named no usable path; the notification's own gets one final read. + if ( + !entry.tail && + task.outputFile && + entry.sessionId && + isTaskOutputPath(task.outputFile, { sessionId: entry.sessionId, taskId: entry.taskId }) + ) { + entry.tail = startTail(entry, task.outputFile) + } + await entry.tail?.finish() + await close(entry, task.status, task.exitCode) + }, + + async endTurn() { + const running = [...shells.values()].filter((entry) => !entry.settled) + if (running.length === 0) return null + for (const entry of running) { + await entry.tail?.finish() + await close(entry, 'ended_with_turn', null) + } + const names = running.map((entry) => entry.details.command ?? entry.details.description ?? entry.taskId) + const list = names.join(', ') + return { + kind: 'task_finished', + level: 'warn', + title: + running.length === 1 + ? 'A background command was stopped when the turn ended' + : `${running.length} background commands were stopped when the turn ended`, + detail: list.length > MAX_NOTICE_DETAIL ? `${list.slice(0, MAX_NOTICE_DETAIL)}…` : list, + persist: true, + } + }, + + async stopAll() { + await Promise.all([...shells.values()].map((entry) => entry.tail?.stop())) + }, + } +} diff --git a/src/lib/engine/sdk-notices.ts b/src/lib/engine/sdk-notices.ts index 30d9ac01..6cd17bac 100644 --- a/src/lib/engine/sdk-notices.ts +++ b/src/lib/engine/sdk-notices.ts @@ -62,8 +62,27 @@ export type BackgroundTask = { description: string } +/** + * A background task that settled, as `task_notification` reports it (#35) — carried beside + * the notice so whatever tracks the task's tool call can close it. `tool_use_id` and + * `output_file` are what the notice alone threw away. + */ +export type TaskSettled = { + taskId: string + /** The call that started the task, when the CLI says. */ + toolUseId: string | null + status: 'completed' | 'failed' | 'stopped' + /** + * The CLI's path for the task's output file. A host path the CLI wrote: check it before + * reading anything through it (`./task-output.server`). + */ + outputFile: string | null + /** From the CLI's summary — `Background command "x" completed (exit code 0)` — when it says. */ + exitCode: number | null +} + export type SdkInterpretation = - | { kind: 'notice'; notice: RunNotice } + | { kind: 'notice'; notice: RunNotice; task?: TaskSettled } /** REPLACE semantics: this is the whole live set, not a delta. */ | { kind: 'background_tasks'; tasks: BackgroundTask[] } | { kind: 'tool_progress'; toolUseId: string; elapsedSeconds: number } @@ -113,6 +132,20 @@ function compactionTitle(meta: Record | null): string { : `${how}: ${before.toLocaleString()} → ${after.toLocaleString()} tokens` } +/** + * The exit code in a background command's summary, which the CLI writes as + * `Background command "…" completed (exit code 0)` or `… failed with exit code 1`. Display + * text, so read loosely and never relied on: absent means "not said", not "zero". Anchored + * at the end, because the quoted description before it is the model's and could say anything. + */ +function summaryExitCode(value: unknown): number | null { + if (typeof value !== 'string') return null + const match = /(?:\(exit code (-?\d+)\)|with exit code (-?\d+))\s*$/.exec(value) + if (!match) return null + const code = Number(match[1] ?? match[2]) + return Number.isSafeInteger(code) ? code : null +} + function backgroundTasks(value: unknown): BackgroundTask[] { if (!Array.isArray(value)) return [] const tasks: BackgroundTask[] = [] @@ -209,22 +242,32 @@ function systemNotice(subtype: string, msg: Record): SdkInterpr // `worker_restart` is the one machine-readable cause, and it means the task was // orphaned rather than that it failed on its own terms. const orphaned = msg.reason === 'worker_restart' + let interpreted: SdkInterpretation | null = null if (status === 'completed') { - return notice('task_finished', 'info', 'A background task finished', summary, true) - } - if (status === 'stopped') { - return notice( + interpreted = notice('task_finished', 'info', 'A background task finished', summary, true) + } else if (status === 'stopped') { + interpreted = notice( 'task_finished', 'warn', orphaned ? 'A background task was lost to a worker restart' : 'A background task was stopped', summary, true, ) + } else if (status === 'failed') { + interpreted = notice('task_finished', 'error', 'A background task failed', summary, true) } - if (status === 'failed') { - return notice('task_finished', 'error', 'A background task failed', summary, true) + const taskId = str(msg.task_id) + if (!interpreted || interpreted.kind !== 'notice' || !taskId) return interpreted + return { + ...interpreted, + task: { + taskId, + toolUseId: str(msg.tool_use_id), + status: status as TaskSettled['status'], + outputFile: str(msg.output_file), + exitCode: summaryExitCode(msg.summary), + }, } - return null } default: diff --git a/src/lib/engine/stream.server.ts b/src/lib/engine/stream.server.ts index b382689e..2e147c0c 100644 --- a/src/lib/engine/stream.server.ts +++ b/src/lib/engine/stream.server.ts @@ -13,6 +13,8 @@ * tool_progress { id, elapsedSeconds } * notice { kind, level, title, detail, persist } * background_tasks { tasks: [{ id, type, description }] } + * shell_output { id, taskId, chunk, reset, truncated, from, to } (live-only) + * shell_task_done { id, taskId, status, exitCode, stdout, truncated } * done { ... } * * Every frame carries a monotonic `id:` so the resume endpoint can replay from @@ -20,7 +22,8 @@ * * `notice`, `background_tasks` and `tool_progress` carry what the loop used to discard — * see `./sdk-notices`. A client that does not know them ignores unknown frames, as it - * always has. + * always has. `shell_output` / `shell_task_done` follow a backgrounded `Bash` command + * against its call's id — see `./background-shells.server`. * * `details` is the SDK's typed tool output, distilled by * `./tool-result-details` for the built-ins whose result is worth rendering as something @@ -44,6 +47,7 @@ import type { ToolScope } from './tool-scope' import { toolResultDetails, type ToolResultDetails } from './tool-result-details' import { toolResultText } from './tool-result-content' import { interpretSdkMessage } from './sdk-notices' +import { createBackgroundShells } from './background-shells.server' import { readTurnUsage, resultErrorMessage, type EngineUsage, type SessionUsage } from './run-result' import type { EngineQueryHandle } from './run-registry.server' import type { StreamBlock } from '$lib/runs/runs.schema' @@ -186,6 +190,8 @@ export type EngineRunInput = { * Only a spec passes this. See `EngineQuerySource` for why the seam exists. */ createQuery?: CreateEngineQuery + /** How often a background command's output file is read (`./background-shells.server`). Only a spec sets it. */ + backgroundOutputPollMs?: number /** * Writes one frame. Owned by the caller because sequence ids come from * `chat_runs.nextEventSeq` via `appendRunEvent` — the same counter the resume @@ -251,6 +257,18 @@ export async function runEngineStream(input: EngineRunInput): Promise() const toolInputs = new Map() + /* + * #35 — backgrounded `Bash` commands: live output into their cards while the turn runs, + * and an honest ending when it stops. They end with the turn (see the module). + */ + const shells = createBackgroundShells({ emit, pollMs: input.backgroundOutputPollMs }) + const endBackgroundShells = async () => { + const ended = await shells.endTurn() + if (!ended) return + blocks.push({ kind: 'notice', notice: ended }) + await emit('notice', ended) + } + /* * The UI keys its blocks on the tool_use id, and the permission callbacks have to emit * frames against the same id or the pending block and the call block render as two @@ -600,6 +618,8 @@ export async function runEngineStream(input: EngineRunInput): Promise. Output is being written to: . … + * + * `task_notification` carries the same path as `output_file` when the task settles. The file + * lives in the CLI's temp tree, `/claude///tasks/.output` + * (seen in CLI 2.1.278), which is on the host, beside the sandbox rather than inside it — so + * the web process can read it directly and stream what arrives into the command's card. + * + * ## What is trusted + * + * Nothing the model or the command wrote. The path comes out of CLI-authored text, but a + * timed-out command's result can carry its own stdout too, and a sandboxed command may be + * able to write under the CLI's temp tree. So a path is only read when: + * + * - it is absolute, with no `..`, and ends `/tasks/.output` — both ids the + * CLI's own, taken from the session and the typed `backgroundTaskId`, never from text; + * - it is the only such path the result names (a second, different one means something is + * imitating the template, and neither is read); + * - the file really is one: opened without following a final symlink, a regular file with a + * single link, whose real path — every link resolved, through `$lib/workspace/containment` + * — still has that same shape. + * + * Every failure means "no live output", never an error: this runs beside the run loop, and a + * card that stays quiet is always an acceptable answer. The CLI's layout is outside the typed + * contract, so an SDK bump that moves the file lands here, as silence, not as a crash. + */ + +import { constants as fsConstants } from 'node:fs' +import { open, type FileHandle } from 'node:fs/promises' +import { basename, dirname, isAbsolute } from 'node:path' +import { resolveRealPath } from '$lib/workspace/containment.server' + +/** The CLI's two ids a task output path must carry. */ +export type TaskOutputAnchor = { sessionId: string; taskId: string } + +/** What the CLI's template says just before the path. */ +const OUTPUT_MARKER = 'Output is being written to: ' + +/** Ids the CLI mints: no separators, no dot-only names. Anything else is not an id. */ +const SAFE_ID = /^[A-Za-z0-9_-][A-Za-z0-9._-]{0,127}$/ + +/** How often a running command's file is read. The frames are live-only, so once a second is plenty. */ +export const TASK_OUTPUT_POLL_MS = 1_000 + +/** + * The most read in one go. A command that writes faster than this skips ahead to the newest + * bytes — the card keeps only the tail anyway (`MAX_STREAM_CHARS`), and a reader that fell + * behind would show output from minutes ago. + */ +export const TASK_OUTPUT_MAX_READ_BYTES = 64 * 1024 + +function hasParentSegment(path: string): boolean { + return path.split(/[\\/]+/).includes('..') +} + +/** True when `path` has the shape the CLI gives this task's output file. Lexical only. */ +export function isTaskOutputPath(path: string, anchor: TaskOutputAnchor): boolean { + if (!SAFE_ID.test(anchor.sessionId) || !SAFE_ID.test(anchor.taskId)) return false + if (typeof path !== 'string' || path.length === 0 || path.includes('\0')) return false + if (!isAbsolute(path) || hasParentSegment(path)) return false + const tasksDir = dirname(path) + return ( + basename(path) === `${anchor.taskId}.output` && + basename(tasksDir) === 'tasks' && + basename(dirname(tasksDir)) === anchor.sessionId + ) +} + +/** + * The output file a backgrounded `Bash` result names, or null. + * + * Paths can contain spaces (a Windows profile directory), so the path is not cut at + * whitespace: it runs from the marker to the task's own file name on the same line. Every + * occurrence is considered, and the answer is only given when they all agree. + */ +export function taskOutputPathFromResult(resultText: string, anchor: TaskOutputAnchor): string | null { + if (typeof resultText !== 'string') return null + const fileName = `${anchor.taskId}.output` + const found = new Set() + let from = 0 + for (;;) { + const at = resultText.indexOf(OUTPUT_MARKER, from) + if (at < 0) break + from = at + OUTPUT_MARKER.length + const lineEnd = resultText.indexOf('\n', from) + const line = resultText.slice(from, lineEnd < 0 ? undefined : lineEnd) + for (let end = line.indexOf(fileName); end >= 0; end = line.indexOf(fileName, end + 1)) { + const candidate = line.slice(0, end + fileName.length) + if (isTaskOutputPath(candidate, anchor)) { + found.add(candidate) + break + } + } + } + return found.size === 1 ? [...found][0] : null +} + +/** The real path, every link resolved, still has the task output shape. */ +function staysATaskOutput(path: string, anchor: TaskOutputAnchor): boolean { + try { + return isTaskOutputPath(resolveRealPath(path), anchor) + } catch { + return false + } +} + +export type TaskOutputChunk = { + /** + * True when this chunk is the start of what should be shown, not an addition to it: the + * first read of the file, a file that was truncated, or a skip ahead to its newest bytes. + */ + reset: boolean + /** True when bytes before this chunk were never read (a skip ahead). */ + skipped: boolean +} + +export type TaskOutputTail = { + /** One last read of whatever has arrived, then stop. Resolves once that read is done. */ + finish(): Promise + /** Stop without reading. Idempotent; resolves once any read in flight is done. */ + stop(): Promise +} + +/** O_NOFOLLOW where the platform has it (not on Windows, which has no such flag). */ +const OPEN_FLAGS = fsConstants.O_RDONLY | (fsConstants.O_NOFOLLOW ?? 0) + +/** A UTF-8 continuation byte: a read that starts on one is in the middle of a character. */ +const isContinuationByte = (byte: number) => (byte & 0xc0) === 0x80 + +/** + * Read `path` from where the last read stopped, every `intervalMs`, until told to stop. + * + * A file that does not exist yet is waited for. A file that stops looking like this task's + * output (a symlink, a hard link, a real path somewhere else) is given up on for good. Reads + * never overlap, and `onChunk` is awaited, so the caller sees chunks strictly in order. + * Never throws; `onGiveUp` says why a tail went quiet. + */ +export function tailTaskOutput(input: { + path: string + anchor: TaskOutputAnchor + onChunk: (chunk: string, info: TaskOutputChunk) => void | Promise + onGiveUp?: (reason: string) => void + intervalMs?: number + maxReadBytes?: number +}): TaskOutputTail { + const intervalMs = Math.max(10, input.intervalMs ?? TASK_OUTPUT_POLL_MS) + const maxReadBytes = Math.max(4, input.maxReadBytes ?? TASK_OUTPUT_MAX_READ_BYTES) + + let offset = 0 + let decoder = new TextDecoder('utf-8') + /** The next chunk starts what should be shown (see `TaskOutputChunk.reset`). */ + let reset = true + /** …and bytes before it were skipped rather than read. */ + let skipped = false + let gaveUp = false + let stopped = false + let timer: ReturnType | null = null + let chain: Promise = Promise.resolve() + + const giveUp = (reason: string) => { + if (gaveUp) return + gaveUp = true + input.onGiveUp?.(reason) + } + + const readOnce = async (final: boolean) => { + if (gaveUp) return + let handle: FileHandle + try { + handle = await open(input.path, OPEN_FLAGS) + } catch (error) { + const code = (error as NodeJS.ErrnoException | undefined)?.code + // Not written yet: the command has not produced anything. Try again next time. + if (code === 'ENOENT') return + giveUp(`cannot open the output file (${code ?? 'unknown error'})`) + return + } + try { + const stat = await handle.stat() + // Checked on the open descriptor, so a swap after the open cannot change the answer. + if (!stat.isFile() || stat.nlink > 1) return giveUp('the output file is not a plain file') + if (!staysATaskOutput(input.path, input.anchor)) return giveUp('the output file resolves elsewhere') + + let skippedNow = false + if (stat.size < offset) { + // Truncated or replaced: start again from the top. + offset = 0 + decoder = new TextDecoder('utf-8') + reset = true + skipped = false + } + if (stat.size - offset > maxReadBytes) { + offset = stat.size - maxReadBytes + decoder = new TextDecoder('utf-8') + reset = true + skipped = true + skippedNow = true + } + + let text = '' + const length = stat.size - offset + if (length > 0) { + const buffer = Buffer.alloc(length) + const { bytesRead } = await handle.read(buffer, 0, length, offset) + offset += bytesRead + let bytes = buffer.subarray(0, bytesRead) + if (skippedNow) { + // A skip lands wherever it lands; drop a partial character rather than print U+FFFD. + let drop = 0 + while (drop < 3 && drop < bytes.length && isContinuationByte(bytes[drop])) drop++ + bytes = bytes.subarray(drop) + } + text = decoder.decode(bytes, { stream: !final }) + } else if (final) { + text = decoder.decode() + } + if (text.length === 0) return + + const info: TaskOutputChunk = { reset, skipped: reset && skipped } + reset = false + skipped = false + await input.onChunk(text, info) + } finally { + await handle.close().catch(() => {}) + } + } + + const run = (final: boolean) => { + chain = chain.then(() => readOnce(final)).catch(() => {}) + return chain + } + + const schedule = () => { + if (stopped) return + timer = setTimeout(() => { + timer = null + void run(false).then(schedule) + }, intervalMs) + // A tail must never be what keeps the process alive. + timer.unref?.() + } + + const halt = () => { + stopped = true + if (timer) clearTimeout(timer) + timer = null + } + + schedule() + + return { + async finish() { + if (stopped) return chain + halt() + await run(true) + }, + async stop() { + halt() + await chain + }, + } +} diff --git a/src/routes/chat/[id]/stream/+server.ts b/src/routes/chat/[id]/stream/+server.ts index 982de827..a366a0db 100644 --- a/src/routes/chat/[id]/stream/+server.ts +++ b/src/routes/chat/[id]/stream/+server.ts @@ -459,8 +459,12 @@ export const POST: RequestHandler = async ({ request, locals }) => { * `notice` and `background_tasks` are NOT in this set on purpose: they are sparse, * and a client that reconnects mid-turn should still learn that the context was * compacted or that three commands are running in the background. + * + * `shell_output` (#35) is a background command's output, read once a second while it + * runs — a row per tick would be the same flood. `shell_task_done` stays persisted: + * it carries the final output, so a replay still ends with what the command printed. */ - const NON_PERSISTED = new Set(['delta', 'reasoning', 'tool_progress']) + const NON_PERSISTED = new Set(['delta', 'reasoning', 'tool_progress', 'shell_output']) /** Closing a cancelled controller throws as well, and is just as harmless. */ const closeStream = (c: ReadableStreamDefaultController) => { diff --git a/tests/engine.sdk-notices.spec.ts b/tests/engine.sdk-notices.spec.ts index 390b5dc3..624e9645 100644 --- a/tests/engine.sdk-notices.spec.ts +++ b/tests/engine.sdk-notices.spec.ts @@ -216,6 +216,65 @@ test.describe('background tasks', () => { expect(result?.kind === 'notice' && result.notice.title.includes('worker restart')).toBe(true) }) + + test('the task itself comes back beside the notice, so its call can be closed (#35)', () => { + const result = interpretSdkMessage({ + type: 'system', + subtype: 'task_notification', + task_id: 'b1', + tool_use_id: 'toolu_1', + status: 'completed', + summary: 'Background command "npm test" completed (exit code 0)', + output_file: '/tmp/claude/p/s/tasks/b1.output', + }) + expect(result?.kind === 'notice' && result.notice.title).toBe('A background task finished') + expect(result?.kind === 'notice' && result.task).toEqual({ + taskId: 'b1', + toolUseId: 'toolu_1', + status: 'completed', + outputFile: '/tmp/claude/p/s/tasks/b1.output', + exitCode: 0, + }) + + const failed = interpretSdkMessage({ + type: 'system', + subtype: 'task_notification', + task_id: 'b2', + status: 'failed', + summary: 'Background command "make" failed with exit code 2', + output_file: '', + }) + expect(failed?.kind === 'notice' && failed.task).toEqual({ + taskId: 'b2', + toolUseId: null, + status: 'failed', + outputFile: null, + exitCode: 2, + }) + }) + + test('an exit code is only read where the CLI puts it, never out of the description', () => { + // The quoted description is the model's; only the CLI's closing words count. + const stopped = interpretSdkMessage({ + type: 'system', + subtype: 'task_notification', + task_id: 'b3', + status: 'stopped', + summary: 'Background command "echo (exit code 5)" was stopped', + output_file: '/x', + }) + expect(stopped?.kind === 'notice' && stopped.task?.exitCode).toBeNull() + + const noCode = interpretSdkMessage({ + type: 'system', + subtype: 'task_notification', + task_id: 'b4', + status: 'completed', + summary: 'Background command "npm run dev" completed', + output_file: '/x', + }) + expect(noCode?.kind === 'notice' && noCode.task?.exitCode).toBeNull() + }) }) test.describe('rate limits and progress', () => { diff --git a/tests/engine.stream-routing.spec.ts b/tests/engine.stream-routing.spec.ts index be5d781e..0de61048 100644 --- a/tests/engine.stream-routing.spec.ts +++ b/tests/engine.stream-routing.spec.ts @@ -1,3 +1,6 @@ +import { appendFileSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' import { expect, test } from '@playwright/test' import { runEngineStream, type EngineQuerySource } from '../src/lib/engine/stream.server' import type { StreamBlock } from '../src/lib/runs/runs.schema' @@ -250,3 +253,248 @@ test.describe('subagent routing (#5)', () => { ]) }) }) + +test.describe('background commands (#35)', () => { + /* + * A backgrounded `Bash` returns at once with a task id and keeps running. The engine tails + * the CLI's output file for it — a real file here, written by the spec while the scripted + * stream is paused — and settles the card when the task does, or when the turn ends first. + */ + const SESSION = '0f6d3c1e-5a4b-4c2d-9e8f-123456789abc' + const INIT = { type: 'system', subtype: 'init', session_id: SESSION } + + function outputFile(taskId: string, session = SESSION) { + const root = mkdtempSync(join(tmpdir(), 'as-bg-shell-')) + const dir = join(root, session, 'tasks') + mkdirSync(dir, { recursive: true }) + return { root, file: join(dir, `${taskId}.output`) } + } + + /** The CLI's backgrounded Bash result: its template text, and a `BashOutput` with the task id. */ + function backgroundResult(id: string, taskId: string, file: string, parent: string | null = null) { + return { + type: 'user', + parent_tool_use_id: parent, + tool_use_result: { stdout: '', stderr: '', interrupted: false, backgroundTaskId: taskId }, + message: { + content: [ + { + type: 'tool_result', + tool_use_id: id, + content: `Command running in background with ID: ${taskId}. Output is being written to: ${file}. You will be notified when it completes. To check interim output, use Read on that file path.`, + }, + ], + }, + } + } + + function notification(taskId: string, toolUseId: string, status: string, file: string, summary: string) { + return { + type: 'system', + subtype: 'task_notification', + task_id: taskId, + tool_use_id: toolUseId, + status, + output_file: file, + summary, + } + } + + type Step = unknown | ((frames: Frame[]) => Promise) + + /** Run a script whose function steps pause the stream; frames are snapshots, not live objects. */ + async function runGated(steps: Step[]) { + const frames: Frame[] = [] + const summary = await runEngineStream({ + prompt: 'go', + options: {}, + backgroundOutputPollMs: 20, + requiresApproval: () => false, + createQuery: () => ({ + async *[Symbol.asyncIterator]() { + for (const step of steps) { + if (typeof step === 'function') await (step as (frames: Frame[]) => Promise)(frames) + else yield step as never + } + }, + }), + emit: async (event, payload) => { + frames.push({ event, payload: JSON.parse(JSON.stringify(payload ?? {})) }) + }, + }) + return { frames, summary } + } + + const shellOutput = (frames: Frame[]) => frames.filter((f) => f.event === 'shell_output') + const streamedText = (frames: Frame[]) => + shellOutput(frames) + .map((f) => String(f.payload.chunk)) + .join('') + const waitForOutput = (text: string) => async (frames: Frame[]) => { + await expect.poll(() => streamedText(frames), { timeout: 10_000 }).toContain(text) + } + const pause = (ms: number) => () => new Promise((resolve) => setTimeout(resolve, ms)) + const toolBlock = (blocks: StreamBlock[]) => + blocks.find((b): b is Extract => b.kind === 'tool') + + test('output streams into the card while it runs, and the notification settles it', async () => { + const { root, file } = outputFile('b1') + try { + const { frames, summary } = await runGated([ + INIT, + toolUse('bg1', 'Bash', { command: 'npm run dev', run_in_background: true }), + backgroundResult('bg1', 'b1', file), + async (f: Frame[]) => { + appendFileSync(file, 'ready on :5173\n') + await waitForOutput('ready on :5173')(f) + }, + async (f: Frame[]) => { + appendFileSync(file, 'compiled\n') + await waitForOutput('compiled')(f) + }, + notification('b1', 'bg1', 'completed', file, 'Background command "npm run dev" completed (exit code 0)'), + RESULT, + ]) + + // The card opens live: its result frame already says so. + const result = frames.find((f) => f.event === 'tool_result') + expect((result?.payload.details as { background?: unknown } | undefined)?.background).toEqual({ + status: 'running', + }) + + // Keyed on the call, in order, the first one starting the card over. + const live = shellOutput(frames) + expect(live.every((f) => f.payload.id === 'bg1' && f.payload.taskId === 'b1')).toBe(true) + expect(live[0].payload.reset).toBe(true) + expect(streamedText(frames)).toBe('ready on :5173\ncompiled\n') + + const done = frames.filter((f) => f.event === 'shell_task_done') + expect(done).toHaveLength(1) + expect(done[0].payload).toMatchObject({ + id: 'bg1', + taskId: 'b1', + status: 'completed', + exitCode: 0, + stdout: 'ready on :5173\ncompiled\n', + }) + + // The persisted block ends where the card did. + const tool = toolBlock(summary.blocks) + expect(tool?.details?.kind).toBe('shell') + if (tool?.details?.kind !== 'shell') return + expect(tool.details.stdout).toBe('ready on :5173\ncompiled\n') + expect(tool.details.background).toEqual({ status: 'completed' }) + expect(tool.details.exitCode).toBe(0) + // Settled in time, so the turn has nothing to say about it ending. + expect(frames.some((f) => f.event === 'notice' && /turn ended/.test(String(f.payload.title)))).toBe(false) + } finally { + rmSync(root, { recursive: true, force: true }) + } + }) + + test('a command still running when the turn ends is marked so, with one notice', async () => { + const { root, file } = outputFile('b1') + try { + const { frames, summary } = await runGated([ + INIT, + toolUse('bg1', 'Bash', { command: 'npm run dev', run_in_background: true }), + backgroundResult('bg1', 'b1', file), + async (f: Frame[]) => { + appendFileSync(file, 'listening\n') + await waitForOutput('listening')(f) + }, + // Written just before the reply ends; the final read still has it. + async () => appendFileSync(file, 'last words\n'), + RESULT, + ]) + + const done = frames.filter((f) => f.event === 'shell_task_done') + expect(done).toHaveLength(1) + expect(done[0].payload).toMatchObject({ id: 'bg1', status: 'ended_with_turn', exitCode: null }) + expect(String(done[0].payload.stdout)).toBe('listening\nlast words\n') + + const notices = summary.blocks.filter((b) => b.kind === 'notice') + expect(notices).toHaveLength(1) + expect(notices[0].kind === 'notice' && notices[0].notice).toMatchObject({ + title: 'A background command was stopped when the turn ended', + detail: 'npm run dev', + persist: true, + }) + + const tool = toolBlock(summary.blocks) + expect(tool?.details?.kind === 'shell' && tool.details.background).toEqual({ status: 'ended_with_turn' }) + } finally { + rmSync(root, { recursive: true, force: true }) + } + }) + + test("a subagent's background command is left to the subagent", async () => { + const { root, file } = outputFile('b2') + try { + writeFileSync(file, 'child output\n') + const { frames, summary } = await runGated([ + INIT, + toolUse('task1', 'Task', { subagent_type: 'reviewer' }), + toolUse('c1', 'Bash', { command: 'sleep 60', run_in_background: true }, 'task1'), + backgroundResult('c1', 'b2', file, 'task1'), + pause(120), + RESULT, + ]) + + expect(shellOutput(frames)).toHaveLength(0) + expect(frames.filter((f) => f.event === 'shell_task_done')).toHaveLength(0) + expect(summary.blocks.filter((b) => b.kind === 'notice')).toHaveLength(0) + } finally { + rmSync(root, { recursive: true, force: true }) + } + }) + + test("a path outside this session's tree is never read, and the card still settles", async () => { + // The file exists and has output, but under another session's directory: neither the + // result's path nor the notification's may be read. + const { root, file } = outputFile('b1', 'someone-elses-session') + try { + writeFileSync(file, 'not yours\n') + const { frames, summary } = await runGated([ + INIT, + toolUse('bg1', 'Bash', { command: 'make test', run_in_background: true }), + backgroundResult('bg1', 'b1', file), + pause(120), + notification('b1', 'bg1', 'failed', file, 'Background command "make test" failed with exit code 2'), + RESULT, + ]) + + expect(shellOutput(frames)).toHaveLength(0) + const done = frames.find((f) => f.event === 'shell_task_done') + expect(done?.payload).toMatchObject({ status: 'failed', exitCode: 2, stdout: '' }) + const tool = toolBlock(summary.blocks) + expect(tool?.details?.kind === 'shell' && tool.details.stdout).toBe('') + } finally { + rmSync(root, { recursive: true, force: true }) + } + }) + + test("the notification's own path gives a final read when the result named none", async () => { + const { root, file } = outputFile('b1') + try { + writeFileSync(file, 'built in 3s\n') + const { frames } = await runGated([ + INIT, + toolUse('bg1', 'Bash', { command: 'bun run build', run_in_background: true }), + // A result with the task id but no template text to read a path from. + { + type: 'user', + tool_use_result: { stdout: '', stderr: '', interrupted: false, backgroundTaskId: 'b1' }, + message: { content: [{ type: 'tool_result', tool_use_id: 'bg1', content: 'Started.' }] }, + }, + notification('b1', 'bg1', 'completed', file, 'Background command "bun run build" completed (exit code 0)'), + RESULT, + ]) + + const done = frames.find((f) => f.event === 'shell_task_done') + expect(done?.payload).toMatchObject({ status: 'completed', exitCode: 0, stdout: 'built in 3s\n' }) + } finally { + rmSync(root, { recursive: true, force: true }) + } + }) +}) diff --git a/tests/engine.task-output.spec.ts b/tests/engine.task-output.spec.ts new file mode 100644 index 00000000..4433494b --- /dev/null +++ b/tests/engine.task-output.spec.ts @@ -0,0 +1,234 @@ +import { appendFileSync, linkSync, mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join, sep } from 'node:path' +import { expect, test } from '@playwright/test' +import { + isTaskOutputPath, + tailTaskOutput, + taskOutputPathFromResult, + type TaskOutputChunk, +} from '../src/lib/engine/task-output.server' + +/** + * #35 — a background command's output file: finding it in the CLI's text, and tailing it. + * + * The path is CLI-authored text on a host path outside the sandbox, so the part worth pinning + * is what is refused: anything that is not exactly this session's, this task's output file. + * The tail is exercised on real files — the whole point of it is how it meets a file that + * grows, is not there yet, is cut short, or is not what it claims to be. + */ + +const SESSION = '0f6d3c1e-5a4b-4c2d-9e8f-123456789abc' +const TASK = 'b7k2m9' +const anchor = { sessionId: SESSION, taskId: TASK } + +const ROOT = process.platform === 'win32' ? 'C:\\Users\\A User\\AppData\\Local\\Temp\\claude\\proj' : '/tmp/claude/-work-proj' +const good = [ROOT, SESSION, 'tasks', `${TASK}.output`].join(sep) + +/** The CLI's template for a backgrounded Bash result, as seen in CLI 2.1.278. */ +const template = (path: string) => + `Command running in background with ID: ${TASK}. Output is being written to: ${path}. You will be notified when it completes. To check interim output, use Read on that file path.` + +test.describe('which path is this task\'s output file', () => { + test('the CLI\'s own path is accepted, spaces and all', () => { + expect(isTaskOutputPath(good, anchor)).toBe(true) + expect(taskOutputPathFromResult(template(good), anchor)).toBe(good) + }) + + test('anything not shaped like this session\'s, this task\'s file is refused', () => { + const refused = { + 'no session segment': [ROOT, 'tasks', `${TASK}.output`].join(sep), + 'another session': [ROOT, 'another-session', 'tasks', `${TASK}.output`].join(sep), + 'another task': [ROOT, SESSION, 'tasks', 'zz9.output'].join(sep), + 'not under tasks/': [ROOT, SESSION, 'other', `${TASK}.output`].join(sep), + relative: ['tmp', SESSION, 'tasks', `${TASK}.output`].join(sep), + 'a .. on the way': [ROOT, '..', 'x', SESSION, 'tasks', `${TASK}.output`].join(sep), + } + for (const [label, path] of Object.entries(refused)) { + expect(isTaskOutputPath(path, anchor), label).toBe(false) + expect(taskOutputPathFromResult(template(path), anchor), label).toBeNull() + } + // Ids that are not ids cannot anchor anything. + expect(isTaskOutputPath(good, { sessionId: '..', taskId: TASK })).toBe(false) + expect(isTaskOutputPath(good, { sessionId: SESSION, taskId: `a${sep}b` })).toBe(false) + }) + + test('two different paths in one result mean neither is read', () => { + // A timed-out command's result carries its own output, which could imitate the template. + const decoy = [process.platform === 'win32' ? 'C:\\evil' : '/evil', SESSION, 'tasks', `${TASK}.output`].join(sep) + expect(taskOutputPathFromResult(`${template(good)}\n${template(decoy)}`, anchor)).toBeNull() + // The same path twice is still one path. + expect(taskOutputPathFromResult(`${template(good)}\n${template(good)}`, anchor)).toBe(good) + }) + + test('a result with no template names nothing', () => { + expect(taskOutputPathFromResult('Command completed.', anchor)).toBeNull() + expect(taskOutputPathFromResult(`Output is being written to: ${ROOT}`, anchor)).toBeNull() + }) +}) + +/** A real `//tasks/` directory in the OS temp dir. */ +function tasksDir() { + const root = mkdtempSync(join(tmpdir(), 'as-task-output-')) + const dir = join(root, SESSION, 'tasks') + mkdirSync(dir, { recursive: true }) + return { root, dir, file: join(dir, `${TASK}.output`) } +} + +/** Collect chunks, and wait for the text so far to satisfy a predicate. */ +function collector() { + const chunks: Array<{ text: string } & TaskOutputChunk> = [] + return { + chunks, + onChunk: (text: string, info: TaskOutputChunk) => { + chunks.push({ text, ...info }) + }, + text: () => chunks.map((c) => c.text).join(''), + } +} + +test.describe('tailing the output file', () => { + test('waits for the file, then delivers what is appended, in order', async () => { + const { root, file } = tasksDir() + const seen = collector() + const tail = tailTaskOutput({ path: file, anchor, intervalMs: 15, onChunk: seen.onChunk }) + try { + // Not there yet: nothing, and no giving up. + await new Promise((r) => setTimeout(r, 60)) + expect(seen.chunks).toEqual([]) + + writeFileSync(file, 'one\n') + await expect.poll(seen.text).toBe('one\n') + appendFileSync(file, 'two\n') + await expect.poll(seen.text).toBe('one\ntwo\n') + appendFileSync(file, 'three\n') + await expect.poll(seen.text).toBe('one\ntwo\nthree\n') + + // Only the first chunk starts the card over. + expect(seen.chunks.map((c) => c.reset)).toEqual([true, false, false]) + } finally { + await tail.stop() + rmSync(root, { recursive: true, force: true }) + } + }) + + test('a character split across two reads comes out whole', async () => { + const { root, file } = tasksDir() + const seen = collector() + const euro = Buffer.from('€', 'utf8') // three bytes + writeFileSync(file, Buffer.concat([Buffer.from('cost: '), euro.subarray(0, 1)])) + const tail = tailTaskOutput({ path: file, anchor, intervalMs: 15, onChunk: seen.onChunk }) + try { + await expect.poll(seen.text).toBe('cost: ') + appendFileSync(file, Buffer.concat([euro.subarray(1), Buffer.from('5\n')])) + await expect.poll(seen.text).toBe('cost: €5\n') + expect(seen.text()).not.toContain('\uFFFD') + } finally { + await tail.stop() + rmSync(root, { recursive: true, force: true }) + } + }) + + test('a file cut short starts over from the top', async () => { + const { root, file } = tasksDir() + const seen = collector() + writeFileSync(file, 'a long first line\n') + const tail = tailTaskOutput({ path: file, anchor, intervalMs: 15, onChunk: seen.onChunk }) + try { + await expect.poll(seen.text).toBe('a long first line\n') + writeFileSync(file, 'new\n') + await expect.poll(() => seen.chunks.at(-1)?.text).toBe('new\n') + expect(seen.chunks.at(-1)?.reset).toBe(true) + } finally { + await tail.stop() + rmSync(root, { recursive: true, force: true }) + } + }) + + test('a command that writes faster than a read skips to its newest output', async () => { + const { root, file } = tasksDir() + const seen = collector() + writeFileSync(file, `${'x'.repeat(500)}TAIL`) + const tail = tailTaskOutput({ path: file, anchor, intervalMs: 15, maxReadBytes: 64, onChunk: seen.onChunk }) + try { + await expect.poll(() => seen.chunks.length).toBe(1) + expect(seen.chunks[0].text.endsWith('TAIL')).toBe(true) + expect(seen.chunks[0].text.length).toBe(64) + expect(seen.chunks[0]).toMatchObject({ reset: true, skipped: true }) + } finally { + await tail.stop() + rmSync(root, { recursive: true, force: true }) + } + }) + + test('stop() ends the reads; finish() makes one last one first', async () => { + const { root, file } = tasksDir() + writeFileSync(file, '') + const stopped = collector() + const stopTail = tailTaskOutput({ path: file, anchor, intervalMs: 15, onChunk: stopped.onChunk }) + await stopTail.stop() + appendFileSync(file, 'after stop\n') + await new Promise((r) => setTimeout(r, 80)) + expect(stopped.chunks).toEqual([]) + + // A long interval: only the final read can see this. + const finished = collector() + const finishTail = tailTaskOutput({ path: file, anchor, intervalMs: 60_000, onChunk: finished.onChunk }) + await finishTail.finish() + expect(finished.text()).toBe('after stop\n') + rmSync(root, { recursive: true, force: true }) + }) + + test('a hard link to some other file is not read', async () => { + const { root, file } = tasksDir() + const secret = join(root, 'secret.txt') + writeFileSync(secret, 'do not stream me\n') + linkSync(secret, file) + const seen = collector() + const reasons: string[] = [] + const tail = tailTaskOutput({ + path: file, + anchor, + intervalMs: 15, + onChunk: seen.onChunk, + onGiveUp: (reason) => reasons.push(reason), + }) + try { + await expect.poll(() => reasons.length).toBe(1) + expect(seen.chunks).toEqual([]) + } finally { + await tail.stop() + rmSync(root, { recursive: true, force: true }) + } + }) + + test('a symlink out of the tasks directory is not followed', async () => { + const { root, file } = tasksDir() + const secret = join(root, 'secret.txt') + writeFileSync(secret, 'do not stream me\n') + try { + symlinkSync(secret, file) + } catch { + // Creating a symlink needs a privilege Windows does not grant by default. + rmSync(root, { recursive: true, force: true }) + test.skip(true, 'symlinks cannot be created here') + return + } + const seen = collector() + const reasons: string[] = [] + const tail = tailTaskOutput({ + path: file, + anchor, + intervalMs: 15, + onChunk: seen.onChunk, + onGiveUp: (reason) => reasons.push(reason), + }) + try { + await expect.poll(() => reasons.length).toBe(1) + expect(seen.chunks).toEqual([]) + } finally { + await tail.stop() + rmSync(root, { recursive: true, force: true }) + } + }) +}) From 008598c860f8abb06372314800a8540869232165 Mon Sep 17 00:00:00 2001 From: Derek Hearst Date: Wed, 23 Sep 2026 20:00:00 -0600 Subject: [PATCH 04/11] feat(chat): the shell card shows the tail, live, and says how it ended (#26, #35) - Escapes: `$lib/chat/terminal-text` strips OSC links and titles, charset escapes (`tput sgr0`'s `ESC(B`), truecolour and private-mode CSI, and resolves bare-CR progress-bar redraws, where the old regex let all of those through as text. - Long output opens on its last 20 lines with Show all / Show last 20, instead of the whole card starting closed and hiding a failure's output. Copy still copies everything. - One badge: `live`, `exit N`, finished / failed / stopped, `ended with turn`, background, timed out or interrupted. The CLI's reading of a special exit code is shown. - A live background card follows the newest output unless the reader has scrolled up. The page applies `shell_output` / `shell_task_done` through pure helpers in `streaming-blocks`, notices a gap after a reconnect, and saves a card still running at a partial save as ended with the turn. The header chips clear on `done`. Co-Authored-By: Claude Opus 5.5 --- src/lib/chat-console/console.css | 22 +++ src/lib/chat/ShellOutputCard.svelte | 163 ++++++++++++++++----- src/lib/chat/streaming-blocks.ts | 88 +++++++++++- src/lib/chat/terminal-text.ts | 55 ++++++++ src/routes/chat/[id]/+page.svelte | 14 ++ tests/chat.shell-output-render.spec.ts | 187 +++++++++++++++++++++++++ tests/chat.shell-output.spec.ts | 145 +++++++++++++++++++ 7 files changed, 635 insertions(+), 39 deletions(-) create mode 100644 src/lib/chat/terminal-text.ts create mode 100644 tests/chat.shell-output-render.spec.ts create mode 100644 tests/chat.shell-output.spec.ts diff --git a/src/lib/chat-console/console.css b/src/lib/chat-console/console.css index 60513113..9583ae25 100644 --- a/src/lib/chat-console/console.css +++ b/src/lib/chat-console/console.css @@ -2324,11 +2324,33 @@ a.console-crumbs__seg:hover { border-radius: 999px; border: 1px solid var(--color-base-300); color: color-mix(in oklab, var(--color-base-content) 65%, transparent); + white-space: nowrap; } .console-term__badge.is-bg { border-color: var(--color-warning); color: var(--color-warning); } .console-term__badge.is-warn { border-color: var(--color-warning); color: var(--color-warning); } .console-term__badge.is-err { border-color: var(--color-error); color: var(--color-error); } +.console-term__badge.is-ok { border-color: var(--color-success); color: var(--color-success); } +.console-term__badge.is-live { + display: inline-flex; + align-items: center; + gap: 4px; + border-color: var(--color-info); + color: var(--color-info); +} + +/* A live background command (#35): same pulse as the header's background-task chips. */ +.console-term__pulse { + width: 6px; + height: 6px; + border-radius: 999px; + background: currentColor; + animation: console-pulse 1.6s ease-in-out infinite; +} + +@media (prefers-reduced-motion: reduce) { + .console-term__pulse { animation: none; } +} .console-term__body { border: 1px solid var(--color-base-300); diff --git a/src/lib/chat/ShellOutputCard.svelte b/src/lib/chat/ShellOutputCard.svelte index ec533c88..f8178907 100644 --- a/src/lib/chat/ShellOutputCard.svelte +++ b/src/lib/chat/ShellOutputCard.svelte @@ -1,18 +1,22 @@ -
+
@@ -72,19 +128,14 @@
-
- {#if isBackground} - background - {/if} - {#if details.timedOutAfterMs !== null} - timed out - {/if} - {#if details.interrupted} - interrupted - {:else if !success} - failed - {/if} -
+ {#if badge} +
+ + {#if badge.tone === 'live'}{/if} + {badge.text} + +
+ {/if}
@@ -93,19 +144,36 @@ {/if} {#if hasOutput} -
+
{#if details.truncated}
Earlier output trimmed — showing the tail.
{/if} + {#if canCollapse && !showAll} +
+ {stdoutTail.hidden + stderrTail.hidden} earlier {stdoutTail.hidden + stderrTail.hidden === 1 + ? 'line' + : 'lines'} hidden. +
+ {/if} {#if stdout.trim()} - 
{stdout}
+ 
{shownStdout}
{/if} {#if stderr.trim()} - 
{stderr}
+ 
{shownStderr}
{/if}
+ {#if canCollapse} + + {/if}
+ {:else if live} +

+ Running in the background as {details.backgroundTaskId}. Its output appears + here as it arrives. +

+ {:else if isBackground && status} +

The command printed nothing.

{:else if isBackground}

- Running in the background as {details.backgroundTaskId}. Output arrives when - the agent checks on it. + Ran in the background as {details.backgroundTaskId}. Its output was not + captured here.

- {:else} + {:else if !details.returnCodeInterpretation}

Command produced no output.

{/if} + + {#if details.returnCodeInterpretation} +

{details.returnCodeInterpretation}

+ {/if} + {#if status === 'ended_with_turn'} +

+ Stopped when the turn ended — background commands only run until the reply is finished. +

+ {:else if isBackground && details.timedOutAfterMs !== null} +

+ Moved to the background after its {Math.max(1, Math.round(details.timedOutAfterMs / 1000))}s + timeout. +

+ {/if}
diff --git a/src/lib/chat/streaming-blocks.ts b/src/lib/chat/streaming-blocks.ts index 2cf4efb0..cf982f83 100644 --- a/src/lib/chat/streaming-blocks.ts +++ b/src/lib/chat/streaming-blocks.ts @@ -10,7 +10,11 @@ */ import { parseJsonFallback } from '$lib/chat/tool-block-helpers' -import type { ToolResultDetails } from '../engine/tool-result-details' +import { + appendStreamTail, + type BackgroundShellStatus, + type ToolResultDetails, +} from '../engine/tool-result-details' import type { RunNotice } from '../engine/sdk-notices' export type ToolStatus = 'pending' | 'approved' | 'executing' | 'completed' | 'failed' | 'denied' @@ -43,6 +47,8 @@ export type ToolBlock = { * generic card the default rather than a fallback. */ details?: ToolResultDetails + /** Characters of a background command's output received live (#35) — see `applyShellOutput`. */ + shellStreamed?: number } export type ThinkingBlock = { @@ -101,6 +107,16 @@ export function getLatestReasoningTokens(blocks: StreamingBlock[]): number | nul return null } +/** + * A block saved from the page is saved because its turn is over (Stop, an error, a lost + * stream), and a background command does not outlive its turn (#35). One still marked + * running would read as running forever in the saved transcript. + */ +function settledDetails(details: ToolResultDetails): ToolResultDetails { + if (details.kind !== 'shell' || details.background?.status !== 'running') return details + return { ...details, background: { status: 'ended_with_turn' } } +} + /** * Build the metadata payload that gets persisted on the assistant message row. * Drops empty text/thinking blocks (they're noise in the persisted history) and @@ -147,7 +163,7 @@ export function getSerializableBlocksForMetadata(blocks: StreamingBlock[]): Arra executionMs: block.executionMs ?? 0, // Persisted so a reloaded conversation renders the same diff / terminal / todo // card as the live stream did, rather than degrading to the generic one. - ...(block.details ? { details: block.details } : {}), + ...(block.details ? { details: settledDetails(block.details) } : {}), }) } } @@ -364,6 +380,74 @@ export function applyToolProgress( ) } +/** + * `shell_output` frame (#35) — new output from a backgrounded command, added to its card. + * + * Capped to the same tail the server keeps (`appendStreamTail`), so the live card and the + * persisted block agree. `reset` replaces instead of adding: the first read of the output + * file, a truncated file, or a jump ahead to the newest output. A chunk that does not start + * where the card left off (a page that reconnected mid-turn missed the frames in between — + * they are live-only) marks the output as a tail, so the card says earlier output is missing + * rather than passing a fragment off as the whole thing. + */ +export function applyShellOutput( + blocks: StreamingBlock[], + payload: { id: string; chunk?: string; reset?: boolean; truncated?: boolean; from?: number; to?: number }, +): StreamingBlock[] { + const chunk = typeof payload.chunk === 'string' ? payload.chunk : '' + return blocks.map((b) => { + if (b.kind !== 'tool' || b.id !== payload.id || b.details?.kind !== 'shell') return b + // Already settled: `shell_task_done` carried the final output, and nothing comes after it. + if (b.details.background && b.details.background.status !== 'running') return b + const reset = payload.reset === true + const next = appendStreamTail(reset ? '' : b.details.stdout, chunk) + const gap = !reset && typeof payload.from === 'number' && payload.from !== (b.shellStreamed ?? 0) + return { + ...b, + shellStreamed: typeof payload.to === 'number' ? payload.to : (b.shellStreamed ?? 0) + chunk.length, + details: { + ...b.details, + stdout: next.text, + truncated: next.truncated || gap || payload.truncated === true || (!reset && b.details.truncated), + background: { status: 'running' as const }, + }, + } + }) +} + +/** + * `shell_task_done` frame (#35) — a background command finished, was stopped, or its turn + * ended. Carries the final output, which replaces whatever the live frames built up: it is + * what the server persisted, so the card now matches the saved transcript exactly. + */ +export function applyShellTaskDone( + blocks: StreamingBlock[], + payload: { + id: string + status?: BackgroundShellStatus + exitCode?: number | null + stdout?: string + truncated?: boolean + }, +): StreamingBlock[] { + const status = payload.status + if (status !== 'completed' && status !== 'failed' && status !== 'stopped' && status !== 'ended_with_turn') { + return blocks + } + return blocks.map((b) => { + if (b.kind !== 'tool' || b.id !== payload.id || b.details?.kind !== 'shell') return b + return { + ...b, + details: { + ...b.details, + ...(typeof payload.stdout === 'string' ? { stdout: payload.stdout, truncated: payload.truncated === true } : {}), + ...(typeof payload.exitCode === 'number' ? { exitCode: payload.exitCode } : {}), + background: { status }, + }, + } + }) +} + export function applyToolDenied(blocks: StreamingBlock[], toolId: string): StreamingBlock[] { return blocks.map((b) => b.kind === 'tool' && b.id === toolId ? { ...b, status: 'denied' as const, expanded: true } : b, diff --git a/src/lib/chat/terminal-text.ts b/src/lib/chat/terminal-text.ts new file mode 100644 index 00000000..687adf8c --- /dev/null +++ b/src/lib/chat/terminal-text.ts @@ -0,0 +1,55 @@ +/** + * Terminal output, made readable as plain text (#26). + * + * Commands write for a terminal: colour codes, hyperlinks, window titles, and progress bars + * that redraw one line with a carriage return. The shell card shows text, so all of that is + * taken out rather than rendered — half-supported colour reads worse than none, and nothing + * in this workload emits colour worth keeping. + * + * Pure, with no `$lib` imports, so specs can load it directly. + */ + +/** CSI: colours and cursor movement, with any parameter bytes (`38:2:…` truecolour, `?25l`, `>4;2m`). */ +const CSI = /\u001b\[[0-?]*[ -/]*[@-~]/g +/** OSC: hyperlinks (`ESC]8;;url ESC\`), window titles — ended by BEL or ST. */ +const OSC = /\u001b\][^\u0007\u001b]*(?:\u0007|\u001b\\)/g +/** Character-set selection (`ESC(B`, which `tput sgr0` prints) and the two-byte escapes. */ +const SHORT_ESCAPE = /\u001b[()*+][0-9A-Za-z]|\u001b[=>78cDEHMNOZ]/g +/** Whatever control characters are left, a lone ESC included — never `\n` or `\t`. */ +const CONTROL = /[\u0000-\u0008\u000b\u000c\u000e-\u001f\u007f]/g + +/** + * What a bare carriage return leaves on screen: the cursor goes back to the start of the + * line and what follows overwrites what was there. `10%\r50%\r100%` is `100%`. + */ +function overwriteLine(line: string): string { + if (!line.includes('\r')) return line + return line.split('\r').reduce((screen, segment) => segment + screen.slice(segment.length), '') +} + +/** Strip escapes and control characters, and resolve carriage-return redraws. */ +export function cleanTerminalText(text: string): string { + if (!text) return '' + const stripped = text.replace(OSC, '').replace(CSI, '').replace(SHORT_ESCAPE, '') + // CRLF is a line break, not a redraw. + const lines = stripped.replace(/\r\n/g, '\n').split('\n').map(overwriteLine) + return lines.join('\n').replace(CONTROL, '') +} + +export type LinePreview = { + /** The text to show. */ + shown: string + /** Lines left out at the top. */ + hidden: number + /** Lines in the whole text. A trailing newline does not start another. */ + total: number +} + +/** The last `limit` lines of `text` — where a command explains how it ended. */ +export function tailLines(text: string, limit: number): LinePreview { + if (!text) return { shown: '', hidden: 0, total: 0 } + const body = text.endsWith('\n') ? text.slice(0, -1) : text + const lines = body.split('\n') + if (lines.length <= limit) return { shown: text, hidden: 0, total: lines.length } + return { shown: lines.slice(-limit).join('\n'), hidden: lines.length - limit, total: lines.length } +} diff --git a/src/routes/chat/[id]/+page.svelte b/src/routes/chat/[id]/+page.svelte index c665158b..044b1e64 100644 --- a/src/routes/chat/[id]/+page.svelte +++ b/src/routes/chat/[id]/+page.svelte @@ -62,6 +62,8 @@ applyToolPending, applyToolProgress, applyToolResult, + applyShellOutput, + applyShellTaskDone, buildDisplayedMessages, estimateTokens, finalizeText, @@ -1048,6 +1050,14 @@ }); } + // #35 — a backgrounded command's live output, and how it ended. + if (eventName === 'shell_output') { + streamingBlocks = applyShellOutput(streamingBlocks, payload as Parameters[1]); + } + if (eventName === 'shell_task_done') { + streamingBlocks = applyShellTaskDone(streamingBlocks, payload as Parameters[1]); + } + if (eventName === 'todo_list') { // #21 — the agent rewrote its plan. Replace, never merge: `TodoWrite` // sends the whole list every time, and a merge would resurrect an item @@ -1133,6 +1143,10 @@ if (eventName === 'done') { doneReceived = true; waitingForFirstToken = false; + // The turn is over, and the CLI session that owned its background commands + // with it (#35). Clear the chips now rather than after the reload below. + backgroundTasks = []; + stoppingTasks = []; if (payload.error) { const message = String(payload.error); setRecoverableError( diff --git a/tests/chat.shell-output-render.spec.ts b/tests/chat.shell-output-render.spec.ts new file mode 100644 index 00000000..eafabcac --- /dev/null +++ b/tests/chat.shell-output-render.spec.ts @@ -0,0 +1,187 @@ +import { randomUUID } from 'node:crypto' +import { expect, test, type Page } from '@playwright/test' +import { authenticateContext, cleanupPrefixedRecords, getActiveUserId, getSql, seedConversation, uniquePrefix } from './helpers' +import { openAndSend, scriptHeldRun, type Frame } from './chat-stream-script' + +/** + * #26 / #35 — the shell card on the page. + * + * A live background command, driven by scripted frames (no model): the card opens live, + * shows the newest lines of what the command printed with a control to show the rest, and + * says how it ended — "ended with turn" when the reply finished first. And a saved failed + * command, rendered from the transcript after a reload, with its exit code. + */ + +const lines = (count: number) => Array.from({ length: count }, (_, i) => `row-${String(i + 1).padStart(2, '0')}`).join('\n') + '\n' + +function shellDetails(overrides: Record = {}) { + return { + kind: 'shell', + tool: 'Bash', + command: 'npm run dev', + description: 'Start the dev server', + stdout: '', + stderr: '', + interrupted: false, + backgroundTaskId: 'b1', + timedOutAfterMs: null, + persistedOutputPath: null, + truncated: false, + ...overrides, + } +} + +/** The frames of a turn that backgrounded `npm run dev` and has seen 40 lines of its output. */ +function backgroundFrames(runId: string, extra: Frame[] = []): Frame[] { + const output = lines(40) + return [ + { id: 1, event: 'context_stats', data: { runId, tokenEstimate: 10, contextWindow: 200_000 } }, + { + id: 2, + event: 'tool_call', + data: { id: 'toolu_bg1', name: 'Bash', arguments: JSON.stringify({ command: 'npm run dev', run_in_background: true }) }, + }, + { + id: 3, + event: 'tool_result', + data: { + id: 'toolu_bg1', + name: 'Bash', + success: true, + executionMs: null, + result: 'Command running in background with ID: b1.', + details: shellDetails({ background: { status: 'running' } }), + }, + }, + // Live-only, so no sequence id — exactly as the server sends it. + { event: 'shell_output', data: { id: 'toolu_bg1', taskId: 'b1', chunk: output, reset: true, truncated: false, from: 0, to: output.length } }, + ...extra, + ] +} + +async function openCard(page: Page) { + const card = page.getByTestId('shell-output-card').filter({ visible: true }).first() + await expect(card).toBeVisible({ timeout: 30_000 }) + return card +} + +test.describe('a background command on the page (#35)', () => { + test('streams live, shows its newest lines, and expands to all of them', async ({ page }) => { + test.setTimeout(90_000) + const prefix = uniquePrefix('chat-shell-live') + await cleanupPrefixedRecords(prefix) + await authenticateContext(page.context()) + const conversation = await seedConversation(prefix, { userId: await getActiveUserId() }) + const { release } = await scriptHeldRun(page, conversation.id, backgroundFrames(randomUUID())) + + try { + await openAndSend(page, conversation.id, `${prefix} start the dev server`) + const card = await openCard(page) + + await expect(card).toHaveAttribute('data-status', 'running') + await expect(card.locator('.console-term__badge')).toHaveText('live') + await expect(card.getByText('row-40')).toBeVisible() + // The first 20 lines wait behind the toggle. + await expect(card.getByText('row-01')).toHaveCount(0) + await expect(card.getByText('20 earlier lines hidden.')).toBeVisible() + + await card.getByRole('button', { name: 'Show all 40 lines' }).click() + await expect(card.getByText(/row-01/)).toBeVisible() + await expect(card.getByRole('button', { name: 'Show last 20 lines' })).toBeVisible() + await expect(card.getByRole('button', { name: 'Copy output' })).toBeVisible() + + // The summary row still has room for the command on a phone. + const command = card.locator('.console-term__cmd') + expect((await command.boundingBox())?.width ?? 0).toBeGreaterThan(40) + } finally { + release() + await page.unrouteAll({ behavior: 'ignoreErrors' }) + await cleanupPrefixedRecords(prefix) + } + }) + + test('a command the turn outlived says it was stopped when the turn ended', async ({ page }) => { + test.setTimeout(90_000) + const prefix = uniquePrefix('chat-shell-ended') + await cleanupPrefixedRecords(prefix) + await authenticateContext(page.context()) + const conversation = await seedConversation(prefix, { userId: await getActiveUserId() }) + const final = lines(3) + const { release } = await scriptHeldRun( + page, + conversation.id, + backgroundFrames(randomUUID(), [ + { + id: 4, + event: 'shell_task_done', + data: { id: 'toolu_bg1', taskId: 'b1', status: 'ended_with_turn', exitCode: null, stdout: final, truncated: false }, + }, + ]), + ) + + try { + await openAndSend(page, conversation.id, `${prefix} start the dev server`) + const card = await openCard(page) + + await expect(card).toHaveAttribute('data-status', 'ended_with_turn') + await expect(card.locator('.console-term__badge')).toHaveText('ended with turn') + await expect(card.getByText(/Stopped when the turn ended/)).toBeVisible() + // The final output replaced what the live frames built up. + await expect(card.getByText(/row-03/)).toBeVisible() + await expect(card.getByText('row-40')).toHaveCount(0) + } finally { + release() + await page.unrouteAll({ behavior: 'ignoreErrors' }) + await cleanupPrefixedRecords(prefix) + } + }) +}) + +test.describe('a saved shell card (#26)', () => { + test('a failed command opens on its last lines with its exit code', async ({ page }) => { + test.setTimeout(60_000) + const prefix = uniquePrefix('chat-shell-saved') + await cleanupPrefixedRecords(prefix) + await authenticateContext(page.context()) + + try { + const conversation = await seedConversation(prefix, { userId: await getActiveUserId() }) + const sql = getSql() + const blocks = [ + { + kind: 'tool', + name: 'Bash', + arguments: { command: 'npm test' }, + result: 'Exit code 1', + success: false, + executionMs: 0, + details: shellDetails({ + command: 'npm test', + description: null, + backgroundTaskId: null, + stdout: `\u001b[31m${lines(25)}\u001b[0m`, + exitCode: 1, + }), + }, + { kind: 'text', content: `${prefix} the tests failed` }, + ] + await sql` + update messages set metadata = ${sql.json({ blocks } as never)} + where conversation_id = ${conversation.id} and role = 'assistant' + ` + + await page.goto('/', { waitUntil: 'domcontentloaded' }) + await page.goto(`/chat/${conversation.id}`, { waitUntil: 'domcontentloaded' }) + const card = await openCard(page) + + await expect(card).toHaveAttribute('open', '') + await expect(card.locator('.console-term__badge')).toHaveText('exit 1') + await expect(card.getByText(/row-25/)).toBeVisible() + await expect(card.getByText('row-01')).toHaveCount(0) + // Colour codes do not leak into the text. + await expect(card.locator('pre').first()).not.toContainText('[31m') + } finally { + await cleanupPrefixedRecords(prefix) + } + }) +}) diff --git a/tests/chat.shell-output.spec.ts b/tests/chat.shell-output.spec.ts new file mode 100644 index 00000000..97d7042f --- /dev/null +++ b/tests/chat.shell-output.spec.ts @@ -0,0 +1,145 @@ +import { expect, test } from '@playwright/test' +import { cleanTerminalText, tailLines } from '../src/lib/chat/terminal-text' +import { + applyShellOutput, + applyShellTaskDone, + getSerializableBlocksForMetadata, + type StreamingBlock, + type ToolBlock, +} from '../src/lib/chat/streaming-blocks' +import { MAX_STREAM_CHARS, type ShellDetails } from '../src/lib/engine/tool-result-details' + +/** + * #26 / #35 — the shell card's text, and how a live background command's card is built up. + * + * Pure: the card's rendering rules and the page's frame handlers, without a page. What the + * card looks like is `chat.shell-output-render.spec.ts`. + */ + +const ESC = '\u001b' + +test.describe('terminal text', () => { + test('colour, truecolour and private-mode escapes are stripped', () => { + expect(cleanTerminalText(`${ESC}[31mred${ESC}[0m plain`)).toBe('red plain') + expect(cleanTerminalText(`${ESC}[38:2:255:0:0mtrue${ESC}[m`)).toBe('true') + expect(cleanTerminalText(`${ESC}[>4;2mkeys${ESC}[?25l`)).toBe('keys') + }) + + test('hyperlinks and window titles are stripped, their text kept', () => { + expect(cleanTerminalText(`${ESC}]8;;https://example.com${ESC}\\link${ESC}]8;;${ESC}\\`)).toBe('link') + expect(cleanTerminalText(`${ESC}]0;my title\u0007after`)).toBe('after') + }) + + test('the charset reset tput sgr0 prints is stripped', () => { + expect(cleanTerminalText(`bold${ESC}(B${ESC}[m done`)).toBe('bold done') + }) + + test('a progress bar redrawn with carriage returns shows its last state', () => { + expect(cleanTerminalText('10%\r50%\r100%\ndone\n')).toBe('100%\ndone\n') + // What a terminal shows: a shorter redraw leaves the rest of the longer line. + expect(cleanTerminalText('abcdef\rXY')).toBe('XYcdef') + // A trailing carriage return moves the cursor and erases nothing. + expect(cleanTerminalText('ready\r')).toBe('ready') + }) + + test('CRLF is a line break, not a redraw', () => { + expect(cleanTerminalText('one\r\ntwo\r\n')).toBe('one\ntwo\n') + }) + + test('the last lines are what a long output shows first', () => { + const text = Array.from({ length: 40 }, (_, i) => `line ${i + 1}`).join('\n') + '\n' + const preview = tailLines(text, 20) + expect(preview.total).toBe(40) + expect(preview.hidden).toBe(20) + expect(preview.shown.split('\n')[0]).toBe('line 21') + expect(preview.shown.endsWith('line 40')).toBe(true) + expect(tailLines('a\nb\n', 20)).toEqual({ shown: 'a\nb\n', hidden: 0, total: 2 }) + expect(tailLines('', 20)).toEqual({ shown: '', hidden: 0, total: 0 }) + }) +}) + +function shellBlock(overrides: Partial = {}): ToolBlock { + return { + kind: 'tool', + id: 'bg1', + name: 'Bash', + arguments: '{"command":"npm run dev","run_in_background":true}', + status: 'completed', + expanded: true, + details: { + kind: 'shell', + tool: 'Bash', + command: 'npm run dev', + description: null, + stdout: '', + stderr: '', + interrupted: false, + backgroundTaskId: 'b1', + timedOutAfterMs: null, + persistedOutputPath: null, + truncated: false, + background: { status: 'running' }, + ...overrides, + }, + } +} + +const detailsOf = (blocks: StreamingBlock[]) => { + const block = blocks[0] + if (block.kind !== 'tool' || block.details?.kind !== 'shell') throw new Error('not a shell block') + return block.details +} + +test.describe('the live card (#35)', () => { + test('output is added in order; a reset starts the card over', () => { + let blocks: StreamingBlock[] = [shellBlock({ stdout: 'placeholder' })] + blocks = applyShellOutput(blocks, { id: 'bg1', chunk: 'one\n', reset: true, from: 0, to: 4 }) + blocks = applyShellOutput(blocks, { id: 'bg1', chunk: 'two\n', reset: false, from: 4, to: 8 }) + expect(detailsOf(blocks).stdout).toBe('one\ntwo\n') + expect(detailsOf(blocks).truncated).toBe(false) + + blocks = applyShellOutput(blocks, { id: 'bg1', chunk: 'fresh\n', reset: true, from: 0, to: 6 }) + expect(detailsOf(blocks).stdout).toBe('fresh\n') + }) + + test('the card keeps the same tail the server does', () => { + let blocks: StreamingBlock[] = [shellBlock({ stdout: 'x'.repeat(MAX_STREAM_CHARS - 2) })] + blocks = applyShellOutput(blocks, { id: 'bg1', chunk: 'END', from: 0, to: 3 }) + expect(detailsOf(blocks).stdout).toHaveLength(MAX_STREAM_CHARS) + expect(detailsOf(blocks).stdout.endsWith('END')).toBe(true) + expect(detailsOf(blocks).truncated).toBe(true) + }) + + test('a chunk that does not start where the card left off marks the output as a tail', () => { + // A page that reconnected mid-turn missed the live-only frames in between. + let blocks: StreamingBlock[] = [shellBlock()] + blocks = applyShellOutput(blocks, { id: 'bg1', chunk: 'late\n', reset: false, from: 900, to: 905 }) + expect(detailsOf(blocks).stdout).toBe('late\n') + expect(detailsOf(blocks).truncated).toBe(true) + }) + + test('a settled card takes the final output and ignores anything after it', () => { + let blocks: StreamingBlock[] = [shellBlock({ stdout: 'partial' })] + blocks = applyShellTaskDone(blocks, { id: 'bg1', status: 'completed', exitCode: 0, stdout: 'all of it\n', truncated: false }) + expect(detailsOf(blocks)).toMatchObject({ stdout: 'all of it\n', exitCode: 0, background: { status: 'completed' } }) + + blocks = applyShellOutput(blocks, { id: 'bg1', chunk: 'stray', from: 10, to: 15 }) + expect(detailsOf(blocks).stdout).toBe('all of it\n') + expect(detailsOf(blocks).background).toEqual({ status: 'completed' }) + }) + + test('frames for another call, or with no status, change nothing', () => { + const blocks: StreamingBlock[] = [shellBlock({ stdout: 'mine' })] + expect(applyShellOutput(blocks, { id: 'other', chunk: 'x' })).toEqual(blocks) + expect(applyShellTaskDone(blocks, { id: 'other', status: 'completed' })).toEqual(blocks) + expect(applyShellTaskDone(blocks, { id: 'bg1' })).toEqual(blocks) + }) + + test('a card still running when the page saves a partial is saved as ended with the turn', () => { + // The page only saves blocks itself once the turn is over (Stop, an error), and a + // background command does not outlive its turn. + const saved = getSerializableBlocksForMetadata([shellBlock({ stdout: 'up\n' })]) + expect((saved[0].details as ShellDetails).background).toEqual({ status: 'ended_with_turn' }) + expect((saved[0].details as ShellDetails).stdout).toBe('up\n') + }) +}) From 0dbeb3224e8242b05d1e13fe06027921407c30c1 Mon Sep 17 00:00:00 2001 From: Derek Hearst Date: Wed, 23 Sep 2026 20:00:00 -0600 Subject: [PATCH 05/11] fix(chat): be honest that background commands end with the turn (#35) The CLI tells the model a backgrounded command "will be notified when it completes" and to Read its output file. Neither holds here: the engine closes the CLI after every turn, and the output file is outside the workspace, where the containment guard refuses the Read. Both tool policies now say so, and point at `tee` into the workspace for output the model needs to read itself. Deleting a conversation only deleted the row, leaving its live turn (and the commands it had backgrounded) running with nothing that could reach it. The delete now interrupts the live run first, through the Stop button's path; with no `perTaskStopAffordance` declared, the CLI's interrupt kills the session's background tasks too. Co-Authored-By: Claude Opus 5.5 --- src/lib/chat/chat.remote.ts | 4 +- src/lib/chat/conversation-delete.server.ts | 34 +++++ src/lib/chat/stream-slots.server.ts | 17 +++ src/lib/runs/runs.server.ts | 4 +- tests/chat.background-commands.spec.ts | 139 +++++++++++++++++++++ 5 files changed, 196 insertions(+), 2 deletions(-) create mode 100644 src/lib/chat/conversation-delete.server.ts create mode 100644 tests/chat.background-commands.spec.ts diff --git a/src/lib/chat/chat.remote.ts b/src/lib/chat/chat.remote.ts index d37d478c..fbf653b0 100644 --- a/src/lib/chat/chat.remote.ts +++ b/src/lib/chat/chat.remote.ts @@ -18,6 +18,7 @@ import { BUILTIN_AGENT_KEYS } from '$lib/agents/builtin-agents.server' import { insertMessageWithSequence } from '$lib/chat/insert-message.server' import { listRecentConversations } from '$lib/chat/conversation-list.server' import { findLiveChatRun } from '$lib/runs/live-chat-run.server' +import { deleteConversationForUser } from '$lib/chat/conversation-delete.server' import { describePermissionMode, PERMISSION_MODES, @@ -139,7 +140,8 @@ export const createConversation = command(createConversationSchema, async (input export const deleteConversation = command(conversationIdSchema, async (conversationId) => { const user = requireAuthenticatedRequestUser() - await db.delete(conversations).where(and(eq(conversations.id, conversationId), eq(conversations.userId, user.id))) + // Stops a turn still running in it first, background commands included (#35). + await deleteConversationForUser(user.id, conversationId) return { success: true } }) diff --git a/src/lib/chat/conversation-delete.server.ts b/src/lib/chat/conversation-delete.server.ts new file mode 100644 index 00000000..a43ef0a6 --- /dev/null +++ b/src/lib/chat/conversation-delete.server.ts @@ -0,0 +1,34 @@ +import { and, eq } from 'drizzle-orm' +import { db } from '$lib/db.server' +import { conversations } from '$lib/sessions/sessions.schema' +import { stopChatRun } from '$lib/runs/runs.server' + +/** + * Delete a conversation, stopping whatever it is still running first (#35). + * + * Deleting only removed the row. A turn in progress kept going — the CLI session, the + * commands it had put in the background, the dev server one of them started — with nothing + * left in the app that could reach it: its run rows cascade away with the conversation, so + * no Stop could find it afterwards. So the live run is interrupted first, through the same + * path as the Stop button. With no `perTaskStopAffordance` declared (`options.server` leaves + * it unset), the CLI's interrupt also kills the session's background tasks (`sdk.d.ts`), and + * the turn then ends, which closes the session. + * + * Stop first, then delete: once the row is gone, nothing can find the run by its + * conversation. The interrupted turn still tries to save its reply into the deleted + * conversation; that write fails and is logged by the stream route, which is the right + * outcome for a reply nobody can see. + * + * Ownership is the WHERE clause on both halves, as everywhere else. + */ +export async function deleteConversationForUser( + userId: string, + conversationId: string, +): Promise<{ deleted: boolean; stoppedRun: boolean }> { + const stop = await stopChatRun({ userId, conversationId, reason: 'Conversation deleted' }) + const deleted = await db + .delete(conversations) + .where(and(eq(conversations.id, conversationId), eq(conversations.userId, userId))) + .returning({ id: conversations.id }) + return { deleted: deleted.length > 0, stoppedRun: stop.stopped } +} diff --git a/src/lib/chat/stream-slots.server.ts b/src/lib/chat/stream-slots.server.ts index fd5eb65a..fa3f693c 100644 --- a/src/lib/chat/stream-slots.server.ts +++ b/src/lib/chat/stream-slots.server.ts @@ -240,12 +240,28 @@ export async function buildProjectContextSlot(input: { } } +/** + * #35 — what a background command really is here. + * + * The CLI tells the model a backgrounded command "will be notified when it completes" and + * to Read its output file for interim output. Neither holds in AgentStudio: the engine + * closes the CLI after every turn, which ends every command it started, and the output file + * is outside the run's workspace, where the containment guard refuses a Read. Without this + * the model promises a user a dev server that is already gone. The user does see the output: + * the command's card streams it while the turn runs. + */ +export const BACKGROUND_COMMAND_POLICY_LINES = [ + '- A Bash command run with run_in_background only runs until your reply ends: it is stopped when you finish, whatever its tool result says about notifying you later. Finish any work that needs it in this same reply, and never tell the user that something is still running after you have answered.', + "- A background command's output file is outside your workspace, so Read cannot open it; the user sees that output live in the chat. If you need to read it yourself, have the command also write it into your workspace (for example `npm run dev 2>&1 | tee dev.log`) and Read that file.", +] + const ORCHESTRATOR_TOOL_POLICY = [ 'Tool usage policy:', '- If the user asks you to ask questions, gather preferences with options, or confirm choices before continuing, you MUST call the ask_user tool.', "- Do not only say you'll ask a question in plain text when ask_user is appropriate.", '- Use concise questions with clear option labels, and allow freeform input when the request is open-ended.', '- For ask_user: aim for ~3 prefilled answer options per question. Prefer asking more focused questions (split complex choices across multiple questions) rather than listing many options in one question.', + ...BACKGROUND_COMMAND_POLICY_LINES, '', ...SUBAGENT_RESULT_POLICY_LINES, ].join('\n') @@ -254,6 +270,7 @@ const AGENT_TOOL_POLICY = [ 'Tool usage policy:', '- You cannot call ask_user directly in agent conversations.', '- If you need user input, summarize missing information and return control to orchestrator for follow-up.', + ...BACKGROUND_COMMAND_POLICY_LINES, ].join('\n') /** diff --git a/src/lib/runs/runs.server.ts b/src/lib/runs/runs.server.ts index 30d381e5..3e72a77e 100644 --- a/src/lib/runs/runs.server.ts +++ b/src/lib/runs/runs.server.ts @@ -216,6 +216,8 @@ export async function stopChatRun(input: { userId: string conversationId: string runId?: string | null + /** What the interrupt is logged as. The Stop button's, unless a caller says otherwise. */ + reason?: string }): Promise { const live = await db .select({ id: chatRuns.id }) @@ -234,7 +236,7 @@ export async function stopChatRun(input: { let stopped = false for (const { id } of live) { - if (await interruptRun(id, 'Stopped by the user')) stopped = true + if (await interruptRun(id, input.reason ?? 'Stopped by the user')) stopped = true } return stopped ? { stopped: true } : { stopped: false, reason: 'not_reachable' } } diff --git a/tests/chat.background-commands.spec.ts b/tests/chat.background-commands.spec.ts new file mode 100644 index 00000000..7a469006 --- /dev/null +++ b/tests/chat.background-commands.spec.ts @@ -0,0 +1,139 @@ +import { randomUUID } from 'node:crypto' +import { expect, test } from '@playwright/test' +import { cleanupPrefixedRecords, getActiveUserId, getSql, seedConversation, uniquePrefix } from './helpers' +import { registerRunHandle, type EngineQueryHandle } from '../src/lib/engine/run-registry.server' + +/** + * #35 — background commands are turn-scoped, and everything around them has to say so. + * + * - The model is told. The CLI's own tool result promises a notification "when it + * completes" and suggests reading the output file; in AgentStudio the command ends with the + * reply and the file is outside the workspace, so the system prompt corrects both. + * - Deleting a conversation stops its live turn, and with it the commands that turn started. + * Before, the row was deleted and the run carried on with nothing left that could reach it. + * + * These call the server modules directly, so the registry they touch is this worker's: a + * fake handle registered here is the one the delete reaches. + */ + +async function seedRun(conversationId: string, opts: { finished?: boolean } = {}) { + const sql = getSql() + const userId = await getActiveUserId() + const [run] = await sql<{ id: string }[]>` + insert into chat_runs (conversation_id, user_id, state, source, started_at, finished_at) + values ( + ${conversationId}, + ${userId}, + ${opts.finished ? 'completed' : 'running'}, + 'chat_stream', + now(), + ${opts.finished ? sql`now()` : null} + ) + returning id + ` + return run.id +} + +function fakeHandle(onInterrupt: () => void): EngineQueryHandle { + return { + interrupt: async () => onInterrupt(), + stopTask: async () => {}, + getContextUsage: async () => null, + } +} + +async function conversationExists(id: string) { + const sql = getSql() + const rows = await sql`select 1 from conversations where id = ${id}` + return rows.length > 0 +} + +test.describe('the model is told background commands end with its reply', () => { + test('both tool policies carry it', async () => { + const { buildToolPolicySlot, BACKGROUND_COMMAND_POLICY_LINES } = await import('../src/lib/chat/stream-slots.server') + expect(BACKGROUND_COMMAND_POLICY_LINES.join('\n')).toMatch(/run_in_background only runs until your reply ends/) + expect(BACKGROUND_COMMAND_POLICY_LINES.join('\n')).toMatch(/outside your workspace, so Read cannot open it/) + for (const orchestrator of [true, false]) { + const slot = buildToolPolicySlot(orchestrator) + for (const line of BACKGROUND_COMMAND_POLICY_LINES) { + expect(slot.content, `orchestrator=${orchestrator}`).toContain(line) + } + } + }) +}) + +test.describe('deleting a conversation stops what it is running', () => { + test('the live turn is interrupted before the conversation goes', async () => { + const prefix = uniquePrefix('chat-delete-live') + await cleanupPrefixedRecords(prefix) + try { + const userId = await getActiveUserId() + const conv = await seedConversation(prefix, { userId }) + const runId = await seedRun(conv.id) + let interrupted = 0 + let existedWhenInterrupted: boolean | null = null + const release = registerRunHandle(runId, { + ...fakeHandle(() => {}), + // The delete waits for the interrupt, so this sees the order the two happen in. + interrupt: async () => { + interrupted++ + existedWhenInterrupted = await conversationExists(conv.id) + }, + }) + try { + const { deleteConversationForUser } = await import('../src/lib/chat/conversation-delete.server') + expect(await deleteConversationForUser(userId, conv.id)).toEqual({ deleted: true, stoppedRun: true }) + expect(interrupted).toBe(1) + // Stopped while the run could still be found by its conversation, then deleted. + expect(existedWhenInterrupted).toBe(true) + expect(await conversationExists(conv.id)).toBe(false) + } finally { + release() + } + } finally { + await cleanupPrefixedRecords(prefix) + } + }) + + test('a conversation with nothing running is simply deleted', async () => { + const prefix = uniquePrefix('chat-delete-idle') + await cleanupPrefixedRecords(prefix) + try { + const userId = await getActiveUserId() + const conv = await seedConversation(prefix, { userId }) + const finishedId = await seedRun(conv.id, { finished: true }) + let interrupted = 0 + const release = registerRunHandle(finishedId, fakeHandle(() => interrupted++)) + try { + const { deleteConversationForUser } = await import('../src/lib/chat/conversation-delete.server') + expect(await deleteConversationForUser(userId, conv.id)).toEqual({ deleted: true, stoppedRun: false }) + expect(interrupted).toBe(0) + } finally { + release() + } + } finally { + await cleanupPrefixedRecords(prefix) + } + }) + + test("someone else's conversation is neither stopped nor deleted", async () => { + const prefix = uniquePrefix('chat-delete-stranger') + await cleanupPrefixedRecords(prefix) + try { + const conv = await seedConversation(prefix, { userId: await getActiveUserId() }) + const runId = await seedRun(conv.id) + let interrupted = 0 + const release = registerRunHandle(runId, fakeHandle(() => interrupted++)) + try { + const { deleteConversationForUser } = await import('../src/lib/chat/conversation-delete.server') + expect(await deleteConversationForUser(randomUUID(), conv.id)).toEqual({ deleted: false, stoppedRun: false }) + expect(interrupted).toBe(0) + expect(await conversationExists(conv.id)).toBe(true) + } finally { + release() + } + } finally { + await cleanupPrefixedRecords(prefix) + } + }) +}) From 75fe76c8ce7a208fe696c85f0b855fba12c2bb10 Mon Sep 17 00:00:00 2001 From: Derek Hearst Date: Wed, 23 Sep 2026 20:00:00 -0600 Subject: [PATCH 06/11] docs: command output and turn-scoped background commands (#26, #35) The chat spec gains plain-English sections on the shell card and on background commands, including what was verified in the SDK 0.3.278 typings and what is only observed in the bundled CLI. The tools doc, README and parity table follow; the re-audit corrects its "poll `BashOutput`" plan and marks both issues shipped, #35 as turn-scoped. Closes #26 Closes #35 Co-Authored-By: Claude Opus 5.5 --- README.md | 2 + docs/chat/spec.md | 40 ++++++++++- docs/structure/claude-desktop-parity.md | 6 +- docs/structure/parity-reaudit-2026-09-22.md | 73 ++++++++++++++++++++- docs/tools/tools.md | 2 + 5 files changed, 117 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index b9947a7b..ef695e8f 100644 --- a/README.md +++ b/README.md @@ -8,6 +8,8 @@ Self-hosted autonomous AI agent platform for a single owner, with a sandboxed wo AgentStudio provides a streaming chat interface where the assistant can call tools such as web search and sandboxed code execution. The filesystem toolset supports ranged file reads, full writes, unified-diff patch apply, deterministic string replace, recursive directory listing, search, move/rename, delete, and file metadata lookups. Chat supports editing and branching, interleaved tool and thinking blocks, per-message performance and cost metrics, model selection, and per-prompt reasoning effort selection. +Shell commands render as a small terminal: the command, its output in a fixed-width font with colour codes cleaned out, the last 20 lines first with a control to show the rest, a copy button, and a badge for how it ended (such as `exit 2`). A command the agent runs in the background (a dev server, a long build) streams its output into that card while the turn runs, shows as a header chip with a stop button, and ends when the reply does; the card and the agent both say so. Deleting a conversation stops its running turn and those commands first. See [docs/chat/spec.md](docs/chat/spec.md#command-output). + Replies can be read aloud: a speaker button on each reply plays it through an OpenRouter text-to-speech model (code blocks are skipped), and an opt-in, per-device **Auto-read** switch above the message box reads each new reply when its turn finishes, for hands-free use. The model and voice are set in Settings → Model & AI; spend is recorded in the usage ledger under "Read Aloud" and counts toward budget limits. See [docs/speech/speech.md](docs/speech/speech.md). Creation workflows are chat-led: New Agent and New Skill actions launch a fresh conversation with a seeded creation prompt. The assistant gathers missing requirements (optionally with ask_user), then executes directly with tool-level approvals where configured. diff --git a/docs/chat/spec.md b/docs/chat/spec.md index 61d9f37a..75823ec0 100644 --- a/docs/chat/spec.md +++ b/docs/chat/spec.md @@ -194,6 +194,44 @@ On mobile, the right panel collapses into a bottom sheet tab drawer. Approval requests and `ask_user` questions render inline in the thread, but are also reflected in the global Review Inbox. Resolving from either place updates the same durable state. +### Command output + +When the agent runs a shell command, the reply shows it as a small terminal instead of a generic tool card. It looks the same while the turn runs and after a reload. + +- The first line is the command. Below it is what the command printed, in a fixed-width font with its line breaks kept. Normal output and error output are shown separately; error output is red. +- Colour codes, terminal links, window titles and redrawn progress bars are cleaned out, so the text reads as plain text. A progress bar that redraws itself on one line shows only its last state (`100%`, not `10% 50% 100%`). +- Long output opens on its **last 20 lines**, because that is where a command says how it went. **Show all N lines** opens the rest and **Show last 20 lines** folds it back. **Copy output** always copies everything. +- The card opens on its own when there is output, when the command failed, or when it ran in the background. +- One badge says how it ended: `exit 2` for a command that failed with an exit code, `failed`, `interrupted`, `timed out`, or no badge for a command that succeeded. Claude Code only reports an exit code for a failed command and for a finished background command, so a successful ordinary command shows no badge rather than `exit 0`. +- Some commands use an exit code to mean something other than failure (for example, `grep` returns 1 when it finds nothing). When Claude Code explains the code ("No matches found"), the card shows the explanation. +- Each stream keeps its last 16,000 characters. When earlier output was cut, the card says so. + +### Background commands + +The agent can start a command in the background, such as a dev server, a watcher or a long build, and keep working while it runs. + +1. The command's card opens straight away with a **live** badge. What the command prints appears in the card about once a second. The card follows the newest line, unless you have scrolled up to read something. +2. A chip in the header shows each running background command, with a button to stop it. +3. When the command finishes, the badge changes to how it ended: its exit code (`exit 0`, `exit 2`), or `finished`, `failed` or `stopped`. A notice in the transcript says it finished. +4. **Background commands end with the turn.** When the agent finishes its reply, every command it started is stopped, because the Claude Code process that ran them is closed after each turn. A command still running at that point is marked **ended with turn**, its card says "Stopped when the turn ended", and one notice in the transcript lists the commands that were stopped. The header chips clear at the same moment. +5. The agent is told this in its instructions. It should finish any work that needs the command in the same reply, and never tell you a server is still running after it has answered. It is also told that it cannot open the command's output file itself (the file is outside its workspace), and to copy output into its workspace, for example with `tee`, when it needs to read it. + +A page that is reloaded mid-turn picks the card up again from the next piece of output. If it missed some output in between, the card says earlier output is missing. When the command ends, the card shows the final output that was saved with the reply. + +**Deleting a conversation stops it first.** If a turn is still running when a conversation is deleted, that turn is stopped, and every background command it started goes with it. Then the conversation is deleted. Before, the conversation disappeared but its turn and commands kept running, with nothing left in the app that could stop them. + +Rules: + +- The live output is read by the server from the file Claude Code writes the command's output to. The server only reads that file if Claude Code's own message named it, it sits exactly where Claude Code keeps this session's output for this command (`…//tasks/.output`), and it is a plain file rather than a link to somewhere else. A path the agent or the command wrote is never read. If anything about the file looks wrong, the card shows no live output, but it still shows how the command ended. +- The live pieces of output are not saved one by one. The reply saves the card's final output (its last 16,000 characters) and how the command ended, so a page that reconnects still ends up with both. +- Background commands started by a subagent are not followed. They belong to the subagent and end with its answer. +- When the agent stops one of its own background commands (Claude Code's `TaskStop` tool), the call is never held for approval, even on a machine without the shell sandbox where every command asks first: it can only stop a command this session started, and it names no file. + +What this relies on, checked against the installed Agent SDK (0.3.278, bundled Claude Code 2.1.278): + +- From the SDK's published types: `task_notification` reports a finished task with its task id, the id of the call that started it, `completed` / `failed` / `stopped`, a one-line summary and the output file's path. `background_tasks_changed` is the whole set of running tasks each time, and is not to be paired with the finish notices. There is no output for a running ordinary command (`tool_progress` carries elapsed seconds only). The old `BashOutput` / `TaskOutput` polling tools were removed; `BashOutput` is now only the name of the `Bash` result's shape. Without the `perTaskStopAffordance` option, which AgentStudio does not set, stopping a turn also kills its background tasks. +- Seen in Claude Code itself, not promised by the types, so a future update could change it: the "Output is being written to: …" sentence in a backgrounded command's result, the `/tasks/.output` file layout, `KillShell` and `KillBash` as old names for `TaskStop`, and a failed command's result starting with `Exit code N`. If any of these change, the cost is quiet cards (no live output, no exit code), never a failed turn. + ### Diff and artifact preview When a coding run changes files or saves artifacts, the workbench can show: @@ -244,7 +282,7 @@ How the controls that exist today behave: - **Answering a question.** When the agent asks a question (`ask_user`), the answer only counts once the server has recorded it. If the question is no longer waiting (it timed out, was answered in another tab, or its turn ended), the page says so and shows the conversation as the server has it, rather than closing the question as if the answer had gone through. An answered question shows the answer under it straight away, and again after a reload. It no longer keeps a live Submit button that does nothing. - **Switching conversations mid-turn.** Opening another conversation while a reply is streaming shows only the other conversation. Nothing from the first one comes along: not its reply, its tool cards, its Stop button, its error message or its Retry. Leaving is not a Stop. The first turn keeps running, saves its own reply, and shows again with Stop when you go back to it. - **A turn that ends in an error.** Some turns end in an error after the reply was already saved, for example when the agent reaches its maximum number of steps or the model provider is overloaded. The page shows the error and keeps the one saved reply. It used to save a second, partial copy of the same reply. -- **Background tasks.** A command the agent starts in the background (a dev server, a watcher) shows as a chip in the header while the turn runs, with a button to stop it. The chips go away when the turn's stream ends, because ending a turn also ends the commands it started. If a stop does not work, a short message under the header says why — for example that the turn had already ended. +- **Background tasks.** A command the agent starts in the background (a dev server, a watcher) shows as a chip in the header while the turn runs, with a button to stop it, and streams its output into its card (see [Background commands](#background-commands)). The chips go away when the turn ends, because ending a turn also ends the commands it started. If a stop does not work, a short message under the header says why — for example that the turn had already ended. - **Pinned checklist.** The panel above the composer shows the main agent's latest plan. When the agent hands a step to a subagent, the subagent's own checklist does not replace it. ### Context meter diff --git a/docs/structure/claude-desktop-parity.md b/docs/structure/claude-desktop-parity.md index dd4c58de..2f6daf01 100644 --- a/docs/structure/claude-desktop-parity.md +++ b/docs/structure/claude-desktop-parity.md @@ -53,14 +53,14 @@ This is the Cowork comparison, and it is the one I got wrong in the first draft: | Diff rendering | **far behind** (#16) | raw JSON in a tool card | inline diffs with per-hunk accept/reject | | Checkpoints and rewind | **far behind** (#24) | rewinds the transcript only; files stay written | auto-checkpoint per turn, Esc-Esc or `/rewind`, restore code / conversation / both | | Todo list | **absent** (#21) | nothing consumes it | pinned, updated in place | -| Terminal / command output | **far behind** (#26) | JSON-escaped blob in a card, nothing streams | streamed terminal output | +| Terminal / command output | **behind**, narrowly (#26, #35 landed) | a terminal card: the command, its output with colour codes cleaned out, the last 20 lines first with Show all, copy, and an exit-code badge. A background command streams its output into the card while the turn runs; an ordinary command shows elapsed seconds only, because the SDK sends no output until it finishes | streamed terminal output | | Git worktrees | **absent** | one working directory per project | worktree-per-agent with cleanup safety | | Repo import and clone | **win** | first-class: import creates a project, clones into a sandbox, sidecar repo row | you point it at a directory | | Commit / push / PR | **even** | approval-gated tools, PR recorded and surfaced in `/review` | same, plus richer GitHub triggers | | Code review of a PR | **absent** | — | `/ultrareview`, merge-aware follow-up reviews | | CI watch and fix | **even** (#20 landed) | an opened PR's checks are watched (webhook, or polling every few minutes) for up to 14 days; a red check opens a review item and a notification, and **Fix it** hands the failure back to the conversation that wrote the code — a button, not an automatic run | cloud sessions react to CI | | Hooks | **even** | `/settings/hooks`, event bus, skill hooks, per-agent bindings. Until the September 2026 audit the bus was never called on the chat engine path, so bindings and the built-in activity hooks only fired for automations; chats now raise run, tool, approval and question events too ([hooks.md](../hooks/hooks.md)) | same idea, dialog-managed | -| Background tasks | **behind** (#35) | the job queue backgrounds automations and research; inside a chat turn a long command blocks the turn | background bash that survives turns, with a completion notice | +| Background tasks | **behind** (#35 landed, turn-scoped) | the job queue backgrounds automations and research. Inside a chat turn the agent can background a command: a header chip with a stop button, live output in its card, a completion notice with the exit code. Commands end with the turn (the engine closes Claude Code after each reply); the card, a transcript notice and the agent's instructions all say so, and deleting a conversation stops its turn and commands first. Keeping one alive between messages needs a conversation-long session, not built | background bash that survives turns, with a completion notice | | Session cost accounting | **win** | per-run rows, `/runs/[id]`, a ledger row per tool call, and the `/activity` usage strip (#38). Built-in calls write $0 call-count rows again since the ledger fix; tool calls made through the older loop (agent-attached automations, monitors, PR fix) are still not counted | session cost in a dialog | ## Chat @@ -135,7 +135,7 @@ Ordered for how this box is used — NAS host, repeating jobs, files rarely open **Wave 3 — the session surface.** -[#29](https://github.com/derekhearst/AgentStudio/issues/29) preview pane · [#19](https://github.com/derekhearst/AgentStudio/issues/19) permission modes · [#24](https://github.com/derekhearst/AgentStudio/issues/24) filesystem checkpoints · [#21](https://github.com/derekhearst/AgentStudio/issues/21) todo list · [#16](https://github.com/derekhearst/AgentStudio/issues/16) diff view · [#26](https://github.com/derekhearst/AgentStudio/issues/26) terminal output · [#35](https://github.com/derekhearst/AgentStudio/issues/35) background work in a turn +[#29](https://github.com/derekhearst/AgentStudio/issues/29) preview pane · [#19](https://github.com/derekhearst/AgentStudio/issues/19) permission modes · [#24](https://github.com/derekhearst/AgentStudio/issues/24) filesystem checkpoints · [#21](https://github.com/derekhearst/AgentStudio/issues/21) todo list · [#16](https://github.com/derekhearst/AgentStudio/issues/16) diff view · [#26](https://github.com/derekhearst/AgentStudio/issues/26) terminal output (landed) · [#35](https://github.com/derekhearst/AgentStudio/issues/35) background work in a turn (landed, turn-scoped) **Wave 4 — the long tail.** diff --git a/docs/structure/parity-reaudit-2026-09-22.md b/docs/structure/parity-reaudit-2026-09-22.md index cfb214b1..b46f9b51 100644 --- a/docs/structure/parity-reaudit-2026-09-22.md +++ b/docs/structure/parity-reaudit-2026-09-22.md @@ -245,9 +245,9 @@ plan), **fold** (belongs inside another issue), **delete** (close it). | # | Title | Verdict | One line | | --- | --- | --- | --- | | #16 | Render diffs for file edits | **rebuild** | the diff is already in `tool_use_result`; no handler changes needed | -| #26 | Shell output as a terminal | **rebuild** | same adapter; "stream it live" means background + poll, not a new transport | +| #26 | Shell output as a terminal | **rebuild** — shipped | same adapter; the card is polished, and live output is #35's host-side tail of the task's output file (there is no `BashOutput` tool to poll) | | #21 | Render the todo list | **rebuild** — shipped | same adapter; pinned above the composer, kept on the conversation | -| #35 | Background work in a turn | **rebuild** — mostly shipped | chips, notices and a stop control land; the live output card is left, with #26 | +| #35 | Background work in a turn | **rebuild** — shipped, turn-scoped | chips, notices, stop, live output in the card, honest end-of-turn, stop on delete; surviving between turns needs a conversation-long session (not built) | | #24 | Filesystem checkpoints | **rebuild** | `enableFileCheckpointing` + `rewindFiles()`, not hand-rolled git stashes | | #23 | Per-project instructions | **rebuild** — shipped | `settingSources` was 90% of it; instructions and the knowledge directory close the rest | | #17 | Connect external MCP servers | **as filed** | plumbing confirmed trivial; the policy layer is the actual work | @@ -288,6 +288,31 @@ I would also drop "ANSI colour rendering" from the scope. Monospace, preserved n exit-code badge, copy button, collapse past ~20 lines; strip ANSI rather than render it, and revisit only if something actually emits colour worth keeping. +**Correction (2026-09-23).** The paragraph above is wrong about the mechanism. In SDK 0.3.278 +`BashOutput` is only the *output type* of `Bash` (`sdk-tools.d.ts`); as a tool it is gone, +and `sdk.d.ts` says the `TaskOutput` tool was removed too ("Read a background task's output +file with the Read tool instead"). `KillShell` / `KillBash` are aliases the CLI resolves to +`TaskStop`. There is nothing to poll. What the CLI does do is write a backgrounded command's +output to `//tasks/.output` and name that path in the call's +result text ("Output is being written to: …") and in `task_notification.output_file`. + +**Shipped.** The card (17e2567 plus this pass): monospace with newlines kept, stdout and +stderr apart, a copy button that copies everything. The polish this pass added: +- `$lib/chat/terminal-text` strips every escape family rather than only simple CSI: OSC + hyperlinks and titles, `ESC(B`, `38:2:` truecolour, private modes. It also resolves + bare-`\r` progress-bar redraws to their last state. +- A long output opens on its last 20 lines with Show all / Show last 20, instead of the + whole card starting closed. A failing command's error output used to hide behind a click. +- An exit-code badge where the CLI reports one. A failed foreground `Bash` has no + `BashOutput` at all: the CLI throws, and `tool_use_result` is the string + `Error: Exit code N` plus the merged output. That is now distilled into the same card + instead of falling back to the generic one. A finished background command's code comes + from its `task_notification` summary. A successful foreground command reports no code, so + it shows no badge rather than a made-up `exit 0`. +- `returnCodeInterpretation` (grep's "No matches found") is shown. + +Live output is #35's, below. + ### #21 — todo `TodoWriteOutput` gives `oldTodos` and `newTodos`, so we get the delta for free. Two @@ -332,6 +357,50 @@ What is left is the first bullet: reading `backgroundTaskId` off `BashOutput` an is a card that keeps polling `BashOutput`, which is the same piece of work as #26's live output and should be done once, for both. +**Shipped (2026-09-23), turn-scoped.** The "keeps polling `BashOutput`" plan could not work; +see the #26 correction. The owner chose option A, "turn-scoped, made honest", over a +conversation-long SDK session. The reason is how the engine runs a turn: it closes the CLI +after every `result`, and a backgrounded `local_bash` does not hold the result back, so every +background command dies with its turn. That was true before this work too. What changed is +that nothing pretends otherwise any more: + +- **Tool names.** `BUILTIN_SHELL_TOOLS` is `['Bash', 'TaskStop']`. The old names stay + recognised, so a stored agent list that says `KillShell` keeps working and is not turned + into an MCP name. A scope drops `BashOutput` / `TaskOutput`. `TaskStop` is not held for + approval under `bashPolicy: 'ask'`. +- **Live output.** `$lib/engine/task-output.server` finds the output file in the CLI's + result text. It accepts the path only when all of these hold: + - it is absolute, has no `..`, and ends `/tasks/.output`, with both + ids the CLI's own (from `init` and the typed `backgroundTaskId`, never from text); + - it is the only such path the text names; + - on every read, the file is still a regular file with one link, and its real path + (through `$lib/workspace/containment`) keeps that shape. + + The file is read once a second on the host. `$lib/engine/background-shells.server` turns + each read into a live-only `shell_output` frame and grows the call's persisted block to + the same 16k tail. +- **How it ended.** `task_notification` now keeps `task_id`, `tool_use_id`, `output_file` + and the summary's exit code. The command's card settles through a persisted + `shell_task_done` frame, which carries the final output, so a client that reconnected + mid-turn still gets it. +- **The end of the turn.** A command still running when the turn ends gets one final read + and is marked `ended_with_turn`. One persisted notice lists the commands that were + stopped. The page clears the header chips on `done`. +- **The model is told.** Both tool policies say a background command only runs until the + reply ends. They also say its output file is outside the workspace (the containment guard + refuses the Read the CLI suggests), and to `tee` into the workspace instead. +- **Delete.** Deleting a conversation interrupts its live run before the row goes. With no + `perTaskStopAffordance` declared, the CLI's interrupt also kills its background tasks + (`sdk.d.ts`). + +Not done: +- A command outliving its turn (option B: one `query()` per conversation, fed turn by turn, + with a reaper and a channel for turns the model starts on its own). It belongs with #24's + control session. +- "Move to background" for a running foreground command (`Query.backgroundTasks`). +- A live check on the Linux image that a backgrounded process really dies with + `session.close()`. The CLI spawns detached shells. + ### #24 — checkpoints Close the hand-rolled plan. `enableFileCheckpointing` plus `query.rewindFiles(userMessageId, diff --git a/docs/tools/tools.md b/docs/tools/tools.md index 4556d612..df25b8fe 100644 --- a/docs/tools/tools.md +++ b/docs/tools/tools.md @@ -61,6 +61,8 @@ There is no separate code tool. The agent writes a script into the workspace and - In production, `Bash` runs inside an operating-system sandbox (bubblewrap) that keeps it to the workspace, so it runs without asking. - Where the sandbox is not available, such as a developer's Windows or Mac machine, every `Bash` call asks for approval first. +- A command can run in the background (`Bash` with `run_in_background`), for a dev server or a long build. Its output streams into its card while the turn runs, and the agent can stop it with `TaskStop`, which never asks for approval. Background commands end when the agent's reply does. See [Background commands](../chat/spec.md#background-commands). +- Claude Code's older names still work in an agent's tool list: `KillShell` and `KillBash` mean `TaskStop`. `BashOutput` and `TaskOutput` no longer exist and are ignored. A script cannot call AgentStudio's tools. The agent calls those itself, several in one step if they do not depend on each other. From c6552ede6d9f23259fc9f90b100a38c3f89b00e0 Mon Sep 17 00:00:00 2001 From: Derek Hearst Date: Wed, 23 Sep 2026 20:25:15 -0600 Subject: [PATCH 07/11] fix(engine): refuse a named pipe in place of a task's output file at once (#35) The output file was opened O_RDONLY | O_NOFOLLOW and only checked with isFile() after the open. A FIFO swapped in for .output (the same sandboxed-writer threat the symlink and hard-link checks exist for) made open() wait for a writer that may never come: the read never resolved, so the final read at the end of the turn never did either, and the libuv threadpool thread it held stalled every fs call queued behind it. Open the way the bundled CLI opens its own task output files, adding O_NONBLOCK where the platform has it. A FIFO then opens at once, fails the isFile() check on the descriptor, and the tail gives up. Regular files ignore the flag. Co-Authored-By: Claude Opus 5.5 --- src/lib/engine/task-output.server.ts | 26 ++++++++++++++++++-------- tests/engine.task-output.spec.ts | 28 ++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 8 deletions(-) diff --git a/src/lib/engine/task-output.server.ts b/src/lib/engine/task-output.server.ts index 66b96e32..d0248819 100644 --- a/src/lib/engine/task-output.server.ts +++ b/src/lib/engine/task-output.server.ts @@ -27,9 +27,9 @@ * CLI's own, taken from the session and the typed `backgroundTaskId`, never from text; * - it is the only such path the result names (a second, different one means something is * imitating the template, and neither is read); - * - the file really is one: opened without following a final symlink, a regular file with a - * single link, whose real path — every link resolved, through `$lib/workspace/containment` - * — still has that same shape. + * - the file really is one: opened without following a final symlink and without waiting on + * a named pipe, a regular file with a single link, whose real path — every link resolved, + * through `$lib/workspace/containment` — still has that same shape. * * Every failure means "no live output", never an error: this runs beside the run loop, and a * card that stays quiet is always an acceptable answer. The CLI's layout is outside the typed @@ -132,8 +132,18 @@ export type TaskOutputTail = { stop(): Promise } -/** O_NOFOLLOW where the platform has it (not on Windows, which has no such flag). */ -const OPEN_FLAGS = fsConstants.O_RDONLY | (fsConstants.O_NOFOLLOW ?? 0) +/** + * How the file is opened, the way the bundled CLI opens its own task output files. + * + * - `O_NOFOLLOW`: a final symlink fails the open instead of being followed. + * - `O_NONBLOCK`: a named pipe put where the file should be opens at once, and then fails the + * `isFile()` check below. Without it, `open()` on a FIFO waits for a writer that may never + * come — holding a libuv threadpool thread, and every read queued behind it, for good. + * Regular files ignore the flag. + * + * Neither exists on Windows, which has no such files at a path like this one. + */ +const OPEN_FLAGS = fsConstants.O_RDONLY | (fsConstants.O_NOFOLLOW ?? 0) | (fsConstants.O_NONBLOCK ?? 0) /** A UTF-8 continuation byte: a read that starts on one is in the middle of a character. */ const isContinuationByte = (byte: number) => (byte & 0xc0) === 0x80 @@ -142,9 +152,9 @@ const isContinuationByte = (byte: number) => (byte & 0xc0) === 0x80 * Read `path` from where the last read stopped, every `intervalMs`, until told to stop. * * A file that does not exist yet is waited for. A file that stops looking like this task's - * output (a symlink, a hard link, a real path somewhere else) is given up on for good. Reads - * never overlap, and `onChunk` is awaited, so the caller sees chunks strictly in order. - * Never throws; `onGiveUp` says why a tail went quiet. + * output (a symlink, a hard link, a named pipe, a real path somewhere else) is given up on for + * good. Reads never overlap, and `onChunk` is awaited, so the caller sees chunks strictly in + * order. Never throws; `onGiveUp` says why a tail went quiet. */ export function tailTaskOutput(input: { path: string diff --git a/tests/engine.task-output.spec.ts b/tests/engine.task-output.spec.ts index 4433494b..2c631f04 100644 --- a/tests/engine.task-output.spec.ts +++ b/tests/engine.task-output.spec.ts @@ -1,3 +1,4 @@ +import { execFileSync } from 'node:child_process' import { appendFileSync, linkSync, mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join, sep } from 'node:path' @@ -179,6 +180,33 @@ test.describe('tailing the output file', () => { rmSync(root, { recursive: true, force: true }) }) + test('a named pipe in place of the file is given up on at once, not waited on', async () => { + // Without O_NONBLOCK, opening a FIFO waits for a writer that never comes: the read never + // returns, and neither does `finish()` — nor the end of the turn that awaits it. + test.skip(process.platform === 'win32', 'named pipes are not files at a path on Windows') + const { root, file } = tasksDir() + execFileSync('mkfifo', [file]) + const seen = collector() + const reasons: string[] = [] + const tail = tailTaskOutput({ + path: file, + anchor, + intervalMs: 15, + onChunk: seen.onChunk, + onGiveUp: (reason) => reasons.push(reason), + }) + try { + await expect.poll(() => reasons, { timeout: 5_000 }).toEqual(['the output file is not a plain file']) + const started = Date.now() + await tail.finish() + expect(Date.now() - started).toBeLessThan(1_000) + expect(seen.chunks).toEqual([]) + } finally { + await tail.stop() + rmSync(root, { recursive: true, force: true }) + } + }) + test('a hard link to some other file is not read', async () => { const { root, file } = tasksDir() const secret = join(root, 'secret.txt') From 206c240f2a10e5a276bd51dcbd3ac10335985545 Mon Sep 17 00:00:00 2001 From: Derek Hearst Date: Wed, 23 Sep 2026 20:25:26 -0600 Subject: [PATCH 08/11] fix(engine): background commands save output for reconnects and never stall a turn (#35) Three review findings in how a backgrounded Bash command is followed. A reloaded or reconnected page never saw live output again. `shell_output` is live-only, and the resume replay reads only persisted run events, so a card on a page that reattached mid-turn stayed frozen under its "live" badge until the command ended. What arrived since the last save now also goes out as a persisted `shell_output_checkpoint` (same shape, `from`/`to` positions), at most every 5 s and only when there is something new; the first output is saved at once. The tail's new `onPoll` hook runs it in the same queue as the chunks, so a checkpoint never lands out of order with the live frames, and output that goes quiet is still saved. The end of a turn could hang on a read. endTurn() and settle() awaited the tail's final read with no limit, so a read that never returned (the FIFO case, fixed at the open in the previous commit) kept the engine from closing the CLI and sending `done`. Final reads and stopAll() are now bounded at 2 s, and a read that comes back after its card was closed changes nothing. A command that exits at once could be reported as stopped with the turn. The CLI emits task_notification the moment a task ends, which can be before the call's tool_result; settle() ignored a task it was not tracking yet. Such settlements are now held (by task id or call id) and track() creates the card settled, with its output and exit code, before the tool_result frame goes out; track() is awaited. Co-Authored-By: Claude Opus 5.5 --- src/lib/engine/background-shells.server.ts | 223 ++++++++++++--- src/lib/engine/stream.server.ts | 7 +- src/lib/engine/task-output.server.ts | 16 +- src/routes/chat/[id]/stream/+server.ts | 5 +- tests/engine.background-shells.spec.ts | 312 +++++++++++++++++++++ tests/engine.stream-routing.spec.ts | 40 +++ tests/engine.task-output.spec.ts | 45 +++ 7 files changed, 599 insertions(+), 49 deletions(-) create mode 100644 tests/engine.background-shells.spec.ts diff --git a/src/lib/engine/background-shells.server.ts b/src/lib/engine/background-shells.server.ts index bcc2dfa7..fe2a5486 100644 --- a/src/lib/engine/background-shells.server.ts +++ b/src/lib/engine/background-shells.server.ts @@ -7,10 +7,15 @@ * - **Live output.** The command's output file is tailed on the host (`./task-output.server`) * and each new piece goes out as a `shell_output` frame against the call's id. The same * text grows the call's persisted block, capped to its tail like any other shell output. + * - **Catching up.** `shell_output` is live-only (one a second would be a flood of rows), so a + * page that reloads or reconnects mid-turn — a phone tab brought back, a network blip — would + * never see another one. What arrived since the last save therefore also goes out as a + * persisted `shell_output_checkpoint`, at most every `SHELL_CHECKPOINT_MS` and only when + * there is something new. The resume replay delivers those, so a reattached card catches up + * and then keeps moving, a few seconds behind rather than frozen. * - **How it ended.** `task_notification` settles it — completed, failed or stopped, with the * exit code when the CLI's summary gives one — as a `shell_task_done` frame. That frame is - * persisted and carries the final output, so a client that reconnected mid-turn (and missed - * the live-only `shell_output` frames) still ends up with what the command printed. + * persisted and carries the final output, so every client ends up with exactly what was saved. * - **The turn ending first.** Background commands are turn-scoped here: the engine closes * the CLI after every turn, and the commands it started end with it. A command still running * when the reply is done is marked `ended_with_turn`, and the turn gets one notice saying so, @@ -20,11 +25,17 @@ * level signal with ids only, and says not to correlate it with the `task_started` / * `task_notification` edges — so the header chips follow it, and this follows the edges. * + * A notification can also arrive before the call's own `tool_result`: the CLI emits it the + * moment the task ends, and a command that exits at once (a typo, `ls`) can end before its + * result is written out. Such a settlement is held until the call is tracked, and the card + * is then created already settled instead of running until the turn ends. + * * A subagent's background commands are not followed: they are owned by the subagent and end * with its final response, and their calls live in the subagent's card, not the parent's. * - * Never throws into the run loop. A command whose output file cannot be found or trusted - * simply has no live output; its card still settles. + * Never throws into the run loop, and never holds it up for long: a final read that does not + * come back within `TAIL_FINISH_TIMEOUT_MS` is abandoned. A command whose output file cannot be + * found or trusted simply has no live output; its card still settles. */ import { appendStreamTail, MAX_STREAM_CHARS, type BackgroundShellStatus, type ShellDetails } from './tool-result-details' @@ -34,7 +45,13 @@ import { logger } from '../observability/logger' type Emit = (event: string, payload: unknown) => Promise -/** `shell_output` — a piece of a background command's output, live-only. */ +/** + * `shell_output` — a piece of a background command's output. Live-only. + * + * `shell_output_checkpoint` has the same shape and is persisted: everything since the + * previous checkpoint, in one chunk. A client applies both the same way, and the positions + * let it skip what it already has. + */ export type ShellOutputFrame = { /** The `Bash` call's tool_use id — the id the chat keys the card on. */ id: string @@ -45,9 +62,12 @@ export type ShellOutputFrame = { reset: boolean /** Whether the card's output is now only a tail. */ truncated: boolean - /** Characters of output streamed before this chunk, since the last reset — lets a client see a gap. */ + /** + * Where `chunk` starts, in characters of output since the last reset. Lets a client see a + * gap (it has less than this) or an overlap (it already has some of the chunk). + */ from: number - /** …and after it. */ + /** …and where it ends: `from + chunk.length`. */ to: number } @@ -62,6 +82,19 @@ export type ShellTaskDoneFrame = { truncated: boolean } +/** The shortest time between two persisted checkpoints of one command's output. */ +export const SHELL_CHECKPOINT_MS = 5_000 + +/** + * The longest one final read may hold up the end of a turn. A read cannot normally take more + * than milliseconds; this is the backstop for one that never returns, so the engine still + * closes the CLI and still sends `done`. + */ +export const TAIL_FINISH_TIMEOUT_MS = 2_000 + +/** Settlements held for calls not tracked yet. Bounded: a subagent's tasks are never claimed. */ +const MAX_EARLY_SETTLEMENTS = 64 + type TrackedShell = { toolUseId: string taskId: string @@ -72,15 +105,26 @@ type TrackedShell = { /** Characters streamed since the last reset. */ streamed: number settled: boolean + /** + * False for a command that had already settled when its result arrived. Its card is created + * final by the `tool_result` frame, so there is nothing to stream to and nothing to save. + */ + live: boolean + /** Output since the last checkpoint: a reset in between makes the next checkpoint one too. */ + unsaved: { chunk: string; reset: boolean } | null + /** When the last checkpoint went out; 0 until one has, so the first output is saved at once. */ + savedAt: number } export type BackgroundShells = { /** * Start following a parent `Bash` call that was backgrounded. Marks `details` as running - * before the call's `tool_result` frame goes out, so the card opens in its live state. + * before the call's `tool_result` frame goes out, so the card opens in its live state — + * or, when the task already settled, reads its output once and marks it settled, so the + * card opens final. Await it before sending the `tool_result` frame. */ - track(call: { toolUseId: string; sessionId: string | null; resultText: string; details: ShellDetails }): void - /** A `task_notification` arrived. No-op for a task this is not following. */ + track(call: { toolUseId: string; sessionId: string | null; resultText: string; details: ShellDetails }): Promise + /** A `task_notification` arrived. Held for later when the call is not tracked yet. */ settle(task: TaskSettled): Promise /** * The turn is over: read each still-running command's output one last time and mark it @@ -94,48 +138,104 @@ export type BackgroundShells = { /** Longest list of commands the end-of-turn notice names. */ const MAX_NOTICE_DETAIL = 300 +const keepTail = (text: string) => (text.length > MAX_STREAM_CHARS ? text.slice(-MAX_STREAM_CHARS) : text) + +/** Wait for `work`, but no longer than `ms`. True when it finished in time. Never rejects. */ +async function within(work: Promise | undefined, ms: number): Promise { + if (!work) return true + let timer: ReturnType | undefined + const timedOut = new Promise((resolve) => { + timer = setTimeout(() => resolve(false), ms) + }) + try { + return await Promise.race([work.then(() => true, () => true), timedOut]) + } finally { + clearTimeout(timer) + } +} + export function createBackgroundShells(input: { emit: Emit - /** Only a spec changes this. */ + /** Only a spec changes these. */ pollMs?: number + checkpointMs?: number + finishTimeoutMs?: number + tail?: typeof tailTaskOutput }): BackgroundShells { const shells = new Map() + /** `task_notification`s that arrived before their call's result, by task id. */ + const early = new Map() + const checkpointMs = Math.max(0, input.checkpointMs ?? SHELL_CHECKPOINT_MS) + const finishTimeoutMs = Math.max(1, input.finishTimeoutMs ?? TAIL_FINISH_TIMEOUT_MS) + const tail = input.tail ?? tailTaskOutput + /** Set by `stopAll`: nothing may be emitted after the engine's `finally`. */ + let closed = false + + const frame = (entry: TrackedShell, chunk: string, reset: boolean): ShellOutputFrame => ({ + id: entry.toolUseId, + taskId: entry.taskId, + chunk, + reset, + truncated: entry.details.truncated, + from: entry.streamed - chunk.length, + to: entry.streamed, + }) const applyChunk = async (entry: TrackedShell, chunk: string, reset: boolean, skipped: boolean) => { - const from = reset ? 0 : entry.streamed + // A read that came back after its command was closed (a final read that timed out). + if (entry.settled || closed) return const next = reset ? appendStreamTail('', chunk) : appendStreamTail(entry.details.stdout, chunk) entry.details.stdout = next.text entry.details.truncated = next.truncated || skipped || (!reset && entry.details.truncated) - entry.streamed = from + chunk.length - const frame: ShellOutputFrame = { - id: entry.toolUseId, - taskId: entry.taskId, - chunk: chunk.length > MAX_STREAM_CHARS ? chunk.slice(-MAX_STREAM_CHARS) : chunk, - reset, - truncated: entry.details.truncated, - from, - to: entry.streamed, - } - await input.emit('shell_output', frame) + entry.streamed = (reset ? 0 : entry.streamed) + chunk.length + if (!entry.live) return + + const kept = keepTail(chunk) + entry.unsaved = + reset || !entry.unsaved + ? { chunk: kept, reset } + : { chunk: keepTail(entry.unsaved.chunk + kept), reset: entry.unsaved.reset } + await input.emit('shell_output', frame(entry, kept, reset)) + } + + /** Persist what arrived since the last checkpoint, when there is some and one is due. */ + const checkpoint = async (entry: TrackedShell) => { + if (!entry.live || entry.settled || closed || !entry.unsaved) return + const now = Date.now() + if (entry.savedAt !== 0 && now - entry.savedAt < checkpointMs) return + const { chunk, reset } = entry.unsaved + entry.unsaved = null + entry.savedAt = now + await input.emit('shell_output_checkpoint', frame(entry, chunk, reset)) } const startTail = (entry: TrackedShell, path: string): TaskOutputTail | null => { if (!entry.sessionId) return null - return tailTaskOutput({ + return tail({ path, anchor: { sessionId: entry.sessionId, taskId: entry.taskId }, intervalMs: input.pollMs, onChunk: (chunk, info) => applyChunk(entry, chunk, info.reset, info.skipped), + onPoll: () => checkpoint(entry), onGiveUp: (reason) => logger.warn('[engine] no live output for a background command', { taskId: entry.taskId, reason }), }) } + /** One last read, bounded: a read that never returns must not hold up the end of the turn. */ + const finishTail = async (entry: TrackedShell) => { + if (await within(entry.tail?.finish(), finishTimeoutMs)) return + logger.warn('[engine] gave up on the final read of a background command', { + taskId: entry.taskId, + timeoutMs: finishTimeoutMs, + }) + } + const close = async (entry: TrackedShell, status: ShellTaskDoneFrame['status'], exitCode: number | null) => { entry.settled = true entry.details.background = { status } if (exitCode !== null) entry.details.exitCode = exitCode - const frame: ShellTaskDoneFrame = { + const done: ShellTaskDoneFrame = { id: entry.toolUseId, taskId: entry.taskId, status, @@ -143,14 +243,25 @@ export function createBackgroundShells(input: { stdout: entry.details.stdout, truncated: entry.details.truncated, } - await input.emit('shell_task_done', frame) + await input.emit('shell_task_done', done) + } + + const findTracked = (task: TaskSettled) => + shells.get(task.taskId) ?? + (task.toolUseId ? [...shells.values()].find((e) => e.toolUseId === task.toolUseId) : undefined) + + const takeEarly = (taskId: string, toolUseId: string): TaskSettled | null => { + const found = early.get(taskId) ?? [...early.values()].find((t) => t.toolUseId === toolUseId) + if (!found) return null + early.delete(found.taskId) + return found } return { - track({ toolUseId, sessionId, resultText, details }) { + async track({ toolUseId, sessionId, resultText, details }) { const taskId = details.backgroundTaskId if (!taskId || shells.has(taskId)) return - details.background = { status: 'running' } + const settled = takeEarly(taskId, toolUseId) const entry: TrackedShell = { toolUseId, taskId, @@ -159,18 +270,43 @@ export function createBackgroundShells(input: { tail: null, streamed: 0, settled: false, + live: !settled, + unsaved: null, + savedAt: 0, } shells.set(taskId, entry) - const path = sessionId ? taskOutputPathFromResult(resultText, { sessionId, taskId }) : null - if (path) entry.tail = startTail(entry, path) - else logger.info('[engine] background command has no readable output path', { taskId }) + const anchor = sessionId ? { sessionId, taskId } : null + const fromNotification = + anchor && settled?.outputFile && isTaskOutputPath(settled.outputFile, anchor) ? settled.outputFile : null + const path = (anchor ? taskOutputPathFromResult(resultText, anchor) : null) ?? fromNotification + + if (!settled) { + details.background = { status: 'running' } + if (path) entry.tail = startTail(entry, path) + else logger.info('[engine] background command has no readable output path', { taskId }) + return + } + + // It already ended: one read for its output, and the card is created settled. The + // `tool_result` frame that follows carries all of it, so no frame goes out from here. + if (path) { + entry.tail = startTail(entry, path) + await finishTail(entry) + } + entry.settled = true + details.background = { status: settled.status } + if (settled.exitCode !== null) details.exitCode = settled.exitCode }, async settle(task) { - const entry = - shells.get(task.taskId) ?? - (task.toolUseId ? [...shells.values()].find((e) => e.toolUseId === task.toolUseId) : undefined) - if (!entry || entry.settled) return + const entry = findTracked(task) + if (!entry) { + // Its call's result has not arrived yet; `track` picks this up. + if (early.size >= MAX_EARLY_SETTLEMENTS) early.delete(early.keys().next().value as string) + early.set(task.taskId, task) + return + } + if (entry.settled) return // The result named no usable path; the notification's own gets one final read. if ( !entry.tail && @@ -180,17 +316,16 @@ export function createBackgroundShells(input: { ) { entry.tail = startTail(entry, task.outputFile) } - await entry.tail?.finish() + await finishTail(entry) await close(entry, task.status, task.exitCode) }, async endTurn() { + early.clear() const running = [...shells.values()].filter((entry) => !entry.settled) if (running.length === 0) return null - for (const entry of running) { - await entry.tail?.finish() - await close(entry, 'ended_with_turn', null) - } + await Promise.all(running.map(finishTail)) + for (const entry of running) await close(entry, 'ended_with_turn', null) const names = running.map((entry) => entry.details.command ?? entry.details.description ?? entry.taskId) const list = names.join(', ') return { @@ -206,7 +341,13 @@ export function createBackgroundShells(input: { }, async stopAll() { - await Promise.all([...shells.values()].map((entry) => entry.tail?.stop())) + closed = true + const stopping = Promise.all([...shells.values()].map((entry) => entry.tail?.stop())) + if (!(await within(stopping, finishTimeoutMs))) { + logger.warn('[engine] a background command read was still running when the turn closed', { + timeoutMs: finishTimeoutMs, + }) + } }, } } diff --git a/src/lib/engine/stream.server.ts b/src/lib/engine/stream.server.ts index 2e147c0c..7ffd0525 100644 --- a/src/lib/engine/stream.server.ts +++ b/src/lib/engine/stream.server.ts @@ -14,6 +14,7 @@ * notice { kind, level, title, detail, persist } * background_tasks { tasks: [{ id, type, description }] } * shell_output { id, taskId, chunk, reset, truncated, from, to } (live-only) + * shell_output_checkpoint { same } (persisted, at most every few seconds) * shell_task_done { id, taskId, status, exitCode, stdout, truncated } * done { ... } * @@ -22,8 +23,8 @@ * * `notice`, `background_tasks` and `tool_progress` carry what the loop used to discard — * see `./sdk-notices`. A client that does not know them ignores unknown frames, as it - * always has. `shell_output` / `shell_task_done` follow a backgrounded `Bash` command - * against its call's id — see `./background-shells.server`. + * always has. `shell_output` / `shell_output_checkpoint` / `shell_task_done` follow a + * backgrounded `Bash` command against its call's id — see `./background-shells.server`. * * `details` is the SDK's typed tool output, distilled by * `./tool-result-details` for the built-ins whose result is worth rendering as something @@ -815,7 +816,7 @@ export async function runEngineStream(input: EngineRunInput): Promise (byte & 0xc0) === 0x80 * * A file that does not exist yet is waited for. A file that stops looking like this task's * output (a symlink, a hard link, a named pipe, a real path somewhere else) is given up on for - * good. Reads never overlap, and `onChunk` is awaited, so the caller sees chunks strictly in - * order. Never throws; `onGiveUp` says why a tail went quiet. + * good. Reads never overlap, and `onChunk` and `onPoll` are awaited, so the caller sees chunks + * strictly in order. Never throws; `onGiveUp` says why a tail went quiet. */ export function tailTaskOutput(input: { path: string anchor: TaskOutputAnchor onChunk: (chunk: string, info: TaskOutputChunk) => void | Promise + /** + * After every scheduled read, whether it found anything or not — never after the final + * one. Runs in the same queue as `onChunk`, so whatever it sends lands in order with the + * chunks. For work that is due on a clock rather than on new output. + */ + onPoll?: () => void | Promise onGiveUp?: (reason: string) => void intervalMs?: number maxReadBytes?: number @@ -247,7 +253,11 @@ export function tailTaskOutput(input: { } const run = (final: boolean) => { - chain = chain.then(() => readOnce(final)).catch(() => {}) + chain = chain + .then(() => readOnce(final)) + .catch(() => {}) + .then(() => (final || stopped || !input.onPoll ? undefined : input.onPoll())) + .catch(() => {}) return chain } diff --git a/src/routes/chat/[id]/stream/+server.ts b/src/routes/chat/[id]/stream/+server.ts index a366a0db..2a4b5aca 100644 --- a/src/routes/chat/[id]/stream/+server.ts +++ b/src/routes/chat/[id]/stream/+server.ts @@ -461,8 +461,9 @@ export const POST: RequestHandler = async ({ request, locals }) => { * compacted or that three commands are running in the background. * * `shell_output` (#35) is a background command's output, read once a second while it - * runs — a row per tick would be the same flood. `shell_task_done` stays persisted: - * it carries the final output, so a replay still ends with what the command printed. + * runs — a row per tick would be the same flood. A reconnecting client catches up from + * `shell_output_checkpoint` instead, the same output saved every few seconds, and from + * `shell_task_done`, which carries the final output. */ const NON_PERSISTED = new Set(['delta', 'reasoning', 'tool_progress', 'shell_output']) diff --git a/tests/engine.background-shells.spec.ts b/tests/engine.background-shells.spec.ts new file mode 100644 index 00000000..513c6277 --- /dev/null +++ b/tests/engine.background-shells.spec.ts @@ -0,0 +1,312 @@ +import { execFileSync } from 'node:child_process' +import { appendFileSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { expect, test } from '@playwright/test' +import { createBackgroundShells, type ShellOutputFrame } from '../src/lib/engine/background-shells.server' +import type { tailTaskOutput } from '../src/lib/engine/task-output.server' +import type { ShellDetails } from '../src/lib/engine/tool-result-details' +import { applyShellOutput, type StreamingBlock } from '../src/lib/chat/streaming-blocks' + +/** + * #35 — following a background command: what is sent, what is saved, and what can never hold + * up the end of a turn. + * + * `createBackgroundShells` is driven directly, against real output files, with `emit` + * recorded. The engine's side of it (which SDK message calls what) is + * `engine.stream-routing.spec.ts`; the page's side is `chat.shell-output.spec.ts`. + */ + +const SESSION = '0f6d3c1e-5a4b-4c2d-9e8f-123456789abc' + +type Frame = { event: string; payload: Record } + +function outputFile(taskId: string) { + const root = mkdtempSync(join(tmpdir(), 'as-bg-shells-')) + const dir = join(root, SESSION, 'tasks') + mkdirSync(dir, { recursive: true }) + return { root, file: join(dir, `${taskId}.output`) } +} + +/** The CLI's backgrounded Bash result text. */ +const template = (taskId: string, file: string) => + `Command running in background with ID: ${taskId}. Output is being written to: ${file}. You will be notified when it completes.` + +function shellDetails(taskId: string, command = 'npm run dev'): ShellDetails { + return { + kind: 'shell', + tool: 'Bash', + command, + description: null, + stdout: '', + stderr: '', + interrupted: false, + backgroundTaskId: taskId, + timedOutAfterMs: null, + persistedOutputPath: null, + truncated: false, + } +} + +function recorder() { + const frames: Frame[] = [] + return { + frames, + emit: async (event: string, payload: unknown) => { + frames.push({ event, payload: JSON.parse(JSON.stringify(payload ?? {})) }) + }, + of: (event: string) => frames.filter((f) => f.event === event), + text: (event: string) => + frames + .filter((f) => f.event === event) + .map((f) => String(f.payload.chunk)) + .join(''), + } +} + +const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)) + +/** The card as the `tool_result` frame creates it, before any output. */ +function card(details: ShellDetails): StreamingBlock[] { + return [ + { + kind: 'tool', + id: 'bg1', + name: 'Bash', + arguments: '{}', + status: 'completed', + expanded: true, + details: JSON.parse(JSON.stringify(details)), + }, + ] +} + +function stdoutOf(blocks: StreamingBlock[]) { + const block = blocks[0] + if (block.kind !== 'tool' || block.details?.kind !== 'shell') throw new Error('not a shell block') + return { stdout: block.details.stdout, truncated: block.details.truncated } +} + +test.describe('saved output, for a page that reconnects mid-turn', () => { + test('each new piece is saved, and a page that sees only saved frames builds the same card', async () => { + const { root, file } = outputFile('b1') + const rec = recorder() + const shells = createBackgroundShells({ emit: rec.emit, pollMs: 15, checkpointMs: 0 }) + const details = shellDetails('b1') + try { + await shells.track({ toolUseId: 'bg1', sessionId: SESSION, resultText: template('b1', file), details }) + const initial = card(details) + + for (const line of ['one\n', 'two\n', 'three\n']) { + appendFileSync(file, line) + await expect.poll(() => rec.text('shell_output_checkpoint')).toContain(line) + } + // Polls that find nothing new save nothing. + const saved = rec.of('shell_output_checkpoint').length + await sleep(80) + expect(rec.of('shell_output_checkpoint')).toHaveLength(saved) + expect(rec.text('shell_output_checkpoint')).toBe('one\ntwo\nthree\n') + + // A connected page gets every frame, live and saved; a reloaded one only the saved. + const everything = rec.frames.filter((f) => f.event === 'shell_output' || f.event === 'shell_output_checkpoint') + const connected = everything.reduce( + (blocks, f) => applyShellOutput(blocks, f.payload as ShellOutputFrame), + initial, + ) + const reloaded = rec + .of('shell_output_checkpoint') + .reduce((blocks, f) => applyShellOutput(blocks, f.payload as ShellOutputFrame), initial) + expect(stdoutOf(connected)).toEqual({ stdout: 'one\ntwo\nthree\n', truncated: false }) + expect(stdoutOf(reloaded)).toEqual({ stdout: 'one\ntwo\nthree\n', truncated: false }) + } finally { + await shells.endTurn() + await shells.stopAll() + rmSync(root, { recursive: true, force: true }) + } + }) + + test('saves are spaced out, and each carries everything since the one before', async () => { + const { root, file } = outputFile('b1') + const rec = recorder() + const shells = createBackgroundShells({ emit: rec.emit, pollMs: 15, checkpointMs: 1_000 }) + try { + await shells.track({ toolUseId: 'bg1', sessionId: SESSION, resultText: template('b1', file), details: shellDetails('b1') }) + + // The first output is saved at once, so a reload early on is not left with nothing. + appendFileSync(file, 'one\n') + await expect.poll(() => rec.of('shell_output_checkpoint').length).toBe(1) + + // More output inside the interval goes out live, and waits to be saved. + appendFileSync(file, 'two\n') + await expect.poll(() => rec.text('shell_output')).toContain('two\n') + appendFileSync(file, 'three\n') + await expect.poll(() => rec.text('shell_output')).toContain('three\n') + expect(rec.of('shell_output_checkpoint')).toHaveLength(1) + + await expect.poll(() => rec.of('shell_output_checkpoint').length, { timeout: 5_000 }).toBe(2) + expect(rec.of('shell_output_checkpoint')[1].payload).toMatchObject({ + id: 'bg1', + taskId: 'b1', + chunk: 'two\nthree\n', + reset: false, + from: 4, + to: 14, + }) + } finally { + await shells.endTurn() + await shells.stopAll() + rmSync(root, { recursive: true, force: true }) + } + }) + + test('a file that starts over is saved as a reset', async () => { + const { root, file } = outputFile('b1') + const rec = recorder() + const shells = createBackgroundShells({ emit: rec.emit, pollMs: 15, checkpointMs: 0 }) + const details = shellDetails('b1') + try { + await shells.track({ toolUseId: 'bg1', sessionId: SESSION, resultText: template('b1', file), details }) + const initial = card(details) + appendFileSync(file, 'a long first line\n') + await expect.poll(() => rec.of('shell_output_checkpoint').length).toBe(1) + writeFileSync(file, 'new\n') + await expect.poll(() => rec.of('shell_output_checkpoint').length).toBe(2) + expect(rec.of('shell_output_checkpoint')[1].payload).toMatchObject({ chunk: 'new\n', reset: true, from: 0, to: 4 }) + + const reloaded = rec + .of('shell_output_checkpoint') + .reduce((blocks, f) => applyShellOutput(blocks, f.payload as ShellOutputFrame), initial) + expect(stdoutOf(reloaded).stdout).toBe('new\n') + } finally { + await shells.endTurn() + await shells.stopAll() + rmSync(root, { recursive: true, force: true }) + } + }) +}) + +test.describe('a notification that arrives before its result', () => { + test('creates the card settled, with its exit code and output, instead of running', async () => { + // A command that fails at once can end before the CLI writes its call's result out. + const { root, file } = outputFile('b1') + const rec = recorder() + const shells = createBackgroundShells({ emit: rec.emit, pollMs: 15 }) + const details = shellDetails('b1', 'nmp run dev') + try { + writeFileSync(file, 'bash: nmp: command not found\n') + await shells.settle({ taskId: 'b1', toolUseId: 'bg1', status: 'failed', outputFile: file, exitCode: 127 }) + expect(rec.frames).toEqual([]) + + await shells.track({ toolUseId: 'bg1', sessionId: SESSION, resultText: template('b1', file), details }) + expect(details.background).toEqual({ status: 'failed' }) + expect(details.exitCode).toBe(127) + expect(details.stdout).toBe('bash: nmp: command not found\n') + // Nothing is sent from here: the `tool_result` frame that follows carries all of it. + expect(rec.frames).toEqual([]) + + // Nothing is still running, so the turn has nothing to say when it ends. + await sleep(60) + expect(await shells.endTurn()).toBeNull() + expect(rec.frames).toEqual([]) + } finally { + await shells.stopAll() + rmSync(root, { recursive: true, force: true }) + } + }) + + test("is matched by the call's id too, and reads the notification's path when the result names none", async () => { + const { root, file } = outputFile('b1') + const rec = recorder() + const shells = createBackgroundShells({ emit: rec.emit, pollMs: 15 }) + const details = shellDetails('b1', 'ls') + try { + writeFileSync(file, 'README.md\n') + // Held under another task id: only the call id ties it to this command. + await shells.settle({ taskId: 'x9', toolUseId: 'bg1', status: 'completed', outputFile: file, exitCode: 0 }) + await shells.track({ toolUseId: 'bg1', sessionId: SESSION, resultText: 'Started.', details }) + expect(details.background).toEqual({ status: 'completed' }) + expect(details.exitCode).toBe(0) + // Checked against this command's own session and task id, which it matches. + expect(details.stdout).toBe('README.md\n') + } finally { + await shells.endTurn() + await shells.stopAll() + rmSync(root, { recursive: true, force: true }) + } + }) +}) + +test.describe('the end of a turn is never held up', () => { + /** A tail whose reads never come back, and a way to deliver one late anyway. */ + function stuckTail() { + const late: Array<(text: string) => Promise> = [] + const factory: typeof tailTaskOutput = (opts) => { + late.push(async (text) => { + await opts.onChunk(text, { reset: false, skipped: false }) + }) + return { finish: () => new Promise(() => {}), stop: () => new Promise(() => {}) } + } + return { factory, late } + } + + test('a final read that never returns is abandoned, and a late one changes nothing', async () => { + const rec = recorder() + const { factory, late } = stuckTail() + const shells = createBackgroundShells({ emit: rec.emit, tail: factory, finishTimeoutMs: 50 }) + const file = join(tmpdir(), SESSION, 'tasks', 'b1.output') + const details = shellDetails('b1') + await shells.track({ toolUseId: 'bg1', sessionId: SESSION, resultText: template('b1', file), details }) + expect(late).toHaveLength(1) + + const started = Date.now() + const notice = await shells.endTurn() + expect(Date.now() - started).toBeLessThan(1_500) + expect(notice?.title).toBe('A background command was stopped when the turn ended') + expect(rec.of('shell_task_done').map((f) => f.payload.status)).toEqual(['ended_with_turn']) + + // Stopping is bounded the same way. + await shells.stopAll() + + const sent = rec.frames.length + await late[0]('too late\n') + expect(details.stdout).toBe('') + expect(rec.frames).toHaveLength(sent) + }) + + test('a notification whose final read never returns still settles the card', async () => { + const rec = recorder() + const { factory } = stuckTail() + const shells = createBackgroundShells({ emit: rec.emit, tail: factory, finishTimeoutMs: 50 }) + const file = join(tmpdir(), SESSION, 'tasks', 'b1.output') + await shells.track({ toolUseId: 'bg1', sessionId: SESSION, resultText: template('b1', file), details: shellDetails('b1') }) + await shells.settle({ taskId: 'b1', toolUseId: 'bg1', status: 'completed', outputFile: file, exitCode: 0 }) + expect(rec.of('shell_task_done').map((f) => f.payload)).toEqual([ + expect.objectContaining({ status: 'completed', exitCode: 0 }), + ]) + expect(await shells.endTurn()).toBeNull() + await shells.stopAll() + }) + + test('a named pipe in place of the output file does not stall the end of the turn', async () => { + test.skip(process.platform === 'win32', 'named pipes are not files at a path on Windows') + const { root, file } = outputFile('b1') + const rec = recorder() + const shells = createBackgroundShells({ emit: rec.emit, pollMs: 15 }) + try { + execFileSync('mkfifo', [file]) + await shells.track({ toolUseId: 'bg1', sessionId: SESSION, resultText: template('b1', file), details: shellDetails('b1') }) + // A few polls against the pipe. + await sleep(100) + const started = Date.now() + const notice = await shells.endTurn() + // Well inside the backstop: the pipe is refused on open, not timed out on. + expect(Date.now() - started).toBeLessThan(1_000) + expect(notice).not.toBeNull() + expect(rec.of('shell_output')).toHaveLength(0) + expect(rec.of('shell_task_done')[0]?.payload).toMatchObject({ status: 'ended_with_turn', stdout: '' }) + } finally { + await shells.stopAll() + rmSync(root, { recursive: true, force: true }) + } + }) +}) diff --git a/tests/engine.stream-routing.spec.ts b/tests/engine.stream-routing.spec.ts index 0de61048..55d504d0 100644 --- a/tests/engine.stream-routing.spec.ts +++ b/tests/engine.stream-routing.spec.ts @@ -368,6 +368,12 @@ test.describe('background commands (#35)', () => { expect(live[0].payload.reset).toBe(true) expect(streamedText(frames)).toBe('ready on :5173\ncompiled\n') + // The first output is also saved at once, for a page that reconnects; the rest waits + // for the next save a few seconds on, which this turn ends before. + const saved = frames.filter((f) => f.event === 'shell_output_checkpoint') + expect(saved.length).toBeGreaterThanOrEqual(1) + expect(saved[0].payload).toMatchObject({ id: 'bg1', taskId: 'b1', chunk: 'ready on :5173\n', reset: true }) + const done = frames.filter((f) => f.event === 'shell_task_done') expect(done).toHaveLength(1) expect(done[0].payload).toMatchObject({ @@ -428,6 +434,40 @@ test.describe('background commands (#35)', () => { } }) + test('a notification that arrives before its result settles the card, not the end of the turn', async () => { + // The CLI emits `task_notification` the moment a task ends, and a command that fails at + // once can end before its call's result is written out. + const { root, file } = outputFile('b1') + try { + writeFileSync(file, 'bash: nmp: command not found\n') + const { frames, summary } = await runGated([ + INIT, + toolUse('bg1', 'Bash', { command: 'nmp run dev', run_in_background: true }), + notification('b1', 'bg1', 'failed', file, 'Background command "nmp run dev" failed with exit code 127'), + backgroundResult('bg1', 'b1', file), + pause(80), + RESULT, + ]) + + // The card opens settled: its result frame already says how it ended. + const result = frames.find((f) => f.event === 'tool_result') + expect(result?.payload.details).toMatchObject({ + background: { status: 'failed' }, + exitCode: 127, + stdout: 'bash: nmp: command not found\n', + }) + expect(shellOutput(frames)).toHaveLength(0) + expect(frames.filter((f) => f.event === 'shell_task_done')).toHaveLength(0) + // Not "stopped when the turn ended": it had finished. + expect(frames.some((f) => f.event === 'notice' && /turn ended/.test(String(f.payload.title)))).toBe(false) + + const tool = toolBlock(summary.blocks) + expect(tool?.details?.kind === 'shell' && tool.details.background).toEqual({ status: 'failed' }) + } finally { + rmSync(root, { recursive: true, force: true }) + } + }) + test("a subagent's background command is left to the subagent", async () => { const { root, file } = outputFile('b2') try { diff --git a/tests/engine.task-output.spec.ts b/tests/engine.task-output.spec.ts index 2c631f04..7b32f238 100644 --- a/tests/engine.task-output.spec.ts +++ b/tests/engine.task-output.spec.ts @@ -180,6 +180,51 @@ test.describe('tailing the output file', () => { rmSync(root, { recursive: true, force: true }) }) + test('onPoll follows every scheduled read, after its chunk, whether or not there was one', async () => { + const { root, file } = tasksDir() + writeFileSync(file, 'one\n') + const events: string[] = [] + const tail = tailTaskOutput({ + path: file, + anchor, + intervalMs: 15, + onChunk: (text) => { + events.push(`chunk:${text}`) + }, + onPoll: () => { + events.push('poll') + }, + }) + try { + await expect.poll(() => events.filter((e) => e === 'poll').length).toBeGreaterThanOrEqual(3) + expect(events.slice(0, 3)).toEqual(['chunk:one\n', 'poll', 'poll']) + } finally { + await tail.stop() + rmSync(root, { recursive: true, force: true }) + } + }) + + test('the final read is not followed by onPoll', async () => { + const { root, file } = tasksDir() + writeFileSync(file, 'last\n') + const events: string[] = [] + // A long interval: only the final read runs. + const tail = tailTaskOutput({ + path: file, + anchor, + intervalMs: 60_000, + onChunk: (text) => { + events.push(`chunk:${text}`) + }, + onPoll: () => { + events.push('poll') + }, + }) + await tail.finish() + expect(events).toEqual(['chunk:last\n']) + rmSync(root, { recursive: true, force: true }) + }) + test('a named pipe in place of the file is given up on at once, not waited on', async () => { // Without O_NONBLOCK, opening a FIFO waits for a writer that never comes: the read never // returns, and neither does `finish()` — nor the end of the turn that awaits it. From 7c3a84b2fa4c99f6f7aa0cc5a15a929c9d6b9654 Mon Sep 17 00:00:00 2001 From: Derek Hearst Date: Wed, 23 Sep 2026 20:25:35 -0600 Subject: [PATCH 09/11] fix(chat): a reconnected shell card catches up from saved output (#35, #26) The page applies `shell_output_checkpoint` the way it applies `shell_output`, so a card on a page that reloaded or reconnected mid-turn keeps moving, a few seconds behind, instead of freezing under its "live" badge. applyShellOutput now reads the frame's `from`/`to` positions: a chunk the card already has is skipped, one that overlaps adds only its new part, and one that starts past the card's end still marks the output as a tail. A stream clipped to its 16k tail is cut at an arbitrary character, which can land inside a colour code and leave `[31m` at the top of the card. The card now drops the first, partial line of a clipped stream when a line follows it. Co-Authored-By: Claude Opus 5.5 --- src/lib/chat/ShellOutputCard.svelte | 15 +++++++--- src/lib/chat/streaming-blocks.ts | 39 +++++++++++++++++++------- src/lib/chat/terminal-text.ts | 18 ++++++++++-- src/routes/chat/[id]/+page.svelte | 2 +- tests/chat.shell-output-render.spec.ts | 32 +++++++++++++++++++++ tests/chat.shell-output.spec.ts | 25 ++++++++++++++++- 6 files changed, 112 insertions(+), 19 deletions(-) diff --git a/src/lib/chat/ShellOutputCard.svelte b/src/lib/chat/ShellOutputCard.svelte index f8178907..6ee8ca47 100644 --- a/src/lib/chat/ShellOutputCard.svelte +++ b/src/lib/chat/ShellOutputCard.svelte @@ -1,7 +1,7 @@