diff --git a/README.md b/README.md index 0ed999a9..4ff905c1 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). + Editing a message or regenerating a reply cuts the conversation back so the model sees only what was kept plus the message it is answering. In a project (or an agent with a persistent workspace) the agent's file changes are checkpointed every turn, and Edit and Regenerate offer to restore the files the dropped replies changed — showing the files and line counts first, and asking again before overwriting uncommitted changes in an imported repository. **Compact Conversation** runs the SDK's own `/compact`. See [docs/chat/chat.md](docs/chat/chat.md). When the agent needs a decision it asks with the Agent SDK's own AskUserQuestion, shown as a question card: a header chip, option cards with a recommended badge, a sandboxed HTML preview of each option where one helps, multi-select, and a free-text "Other". It can be answered in the chat, from the composer, or from the /review inbox, and never answers itself. See [docs/chat/spec.md](docs/chat/spec.md#questions-from-the-agent). diff --git a/docs/chat/spec.md b/docs/chat/spec.md index 1c82a3c1..fcdb92e3 100644 --- a/docs/chat/spec.md +++ b/docs/chat/spec.md @@ -282,6 +282,46 @@ Some questions say **Choose any that apply**: tick as many options as you like. Transcripts from before the change keep their `ask_user` blocks, and those still show each question with its answer. +### 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, and leaves out the first line of what is left: the cut lands mid-line, and can land in the middle of a colour code, which would otherwise show up as stray text like `[31m`. + +### 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. + +**Reloading or reconnecting mid-turn.** A page that is reloaded while the turn runs, or that reconnects after a dropped connection (a phone tab brought back to the front, a network blip), gets only what the server saved. So the server also saves the command's new output every five seconds or so while it keeps changing. A reloaded card catches up to what the command had printed a few seconds earlier, then keeps updating every few seconds instead of every second, and its **live** badge stays true. When the command ends, the card shows the final output that was saved with the reply. If some output never reached the page at all, the card says earlier output is missing rather than passing a fragment off as the whole thing. + +**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 once-a-second pieces of output are not saved one by one. What is saved is what arrived since the last save, at most every five seconds and only when there is something new, plus the card's final output (its last 16,000 characters) and how the command ended. A quiet dev server costs a few small saves; a chatty build costs at most one 16,000-character save every five seconds. +- A command that ends almost at once (a typo, a quick `ls`) can finish before Claude Code reports that it started. Its card opens already settled, with its output and exit code, instead of running until the turn ends. +- Reading a command's output can never hold up the end of a turn: a last read that has not come back within two seconds is abandoned, and the turn ends normally. A file replaced by something that is not a plain file (for example a named pipe) is refused as soon as it is opened. +- 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 rule that makes every command ask first on a machine without the shell sandbox does not apply: `TaskStop` can only stop a command this session started, and it names no file. The conversation's permission mode and the per-tool approval settings still apply to it, as to any other tool (plan mode, for example, refuses it). + +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`. Claude Code sends a finish notice the moment a task ends, so for a command that exits at once the notice could come before the command's own result; that order was read from Claude Code's code rather than seen happen, and both orders are handled. Claude Code also adds a last line such as `[exited with code 0]` or `[killed]` to the output file, so a finished card usually ends with it. 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: @@ -332,7 +372,7 @@ How the controls that exist today behave: - **Answering a question.** When the agent asks a question (see [Questions from the agent](#questions-from-the-agent)), 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 9a5bb507..64d880a7 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 | **behind** (#24 landed) | auto-checkpoint per turn in a project or persistent workspace; Edit and Regenerate cut the SDK session back to the kept history and offer to restore the files the dropped turns changed, with a dry-run preview and an explicit confirmation over uncommitted changes in an imported repo | auto-checkpoint per turn, Esc-Esc or `/rewind`, restore code / conversation / both — including a code-only restore we do not offer yet | | 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 @@ -137,7 +137,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 4b0b5192..c0db6c6e 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** — shipped | `enableFileCheckpointing` + `rewindFiles()` through a short-lived control session; edit/regenerate now cut the SDK session too | | #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,60 @@ 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'` (it never was a command tool); permission modes still + gate it like any other tool, and plan mode refuses it. +- **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. It is opened `O_NOFOLLOW | + O_NONBLOCK`, as the CLI opens it, so a named pipe put in its place fails the check at + once instead of blocking a threadpool thread on `open()`. + + 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. Because the resume replay only has persisted events, what arrived since + the last save also goes out as a persisted `shell_output_checkpoint`, at most every 5 s + and only when there is something new. A reloaded or reconnected page catches up from + those and keeps moving, a few seconds behind. `from` / `to` positions let a connected + page skip the checkpoint of what it already has. +- **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. A notification that arrives before its call's `tool_result` (the + CLI emits it the moment a task ends, and a command that exits at once can end before its + result is written) is held, and the card is created settled rather than running. +- **The end of the turn.** A command still running when the turn ends gets one final read, + bounded at 2 s so a read that never returns cannot stop the engine closing the CLI or + sending `done`, 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, @@ -640,8 +719,9 @@ gone: a research run's page links back to its chat, and a reply's stats popover topbar. Fixed on the way: the phone drawer rendered empty (the mobile CSS hid every `.console-rail`), a hydration race showed the previous chat's preview after a quick switch, "Open file" on the already-open file did nothing from the Files tab, and the home page showed -the last chat's rail. The background-task chips stay in the topbar until #35 defines what a -task row shows. See [`docs/chat-console/chat-console.md`](../chat-console/chat-console.md). +the last chat's rail. The background-task chips stay in the topbar: #35 kept them there, with +a stop control each, and shows a command's live output in its own card in the thread rather +than in a rail panel. See [`docs/chat-console/chat-console.md`](../chat-console/chat-console.md). ### #27 — TTS diff --git a/docs/tools/tools.md b/docs/tools/tools.md index 90896a45..19dbaf15 100644 --- a/docs/tools/tools.md +++ b/docs/tools/tools.md @@ -60,6 +60,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`. Stopping is not treated as running a command, so it does not ask for approval on a machine without the sandbox; the conversation's permission mode still applies. 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. diff --git a/src/lib/chat-console/console.css b/src/lib/chat-console/console.css index 71789480..e7befe14 100644 --- a/src/lib/chat-console/console.css +++ b/src/lib/chat-console/console.css @@ -2293,11 +2293,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..6ee8ca47 100644 --- a/src/lib/chat/ShellOutputCard.svelte +++ b/src/lib/chat/ShellOutputCard.svelte @@ -1,18 +1,24 @@ -
+
@@ -72,19 +135,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 +151,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/chat.remote.ts b/src/lib/chat/chat.remote.ts index 32e36d3b..888ed348 100644 --- a/src/lib/chat/chat.remote.ts +++ b/src/lib/chat/chat.remote.ts @@ -20,6 +20,7 @@ import { editUserMessage, truncateAfterMessage } from '$lib/chat/message-branch. import { previewMessageRewind } from '$lib/chat/rewind.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, @@ -155,7 +156,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 5fd6714b..cd6412e3 100644 --- a/src/lib/chat/stream-slots.server.ts +++ b/src/lib/chat/stream-slots.server.ts @@ -240,6 +240,21 @@ 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 AskUserQuestion tool.', @@ -247,6 +262,7 @@ const ORCHESTRATOR_TOOL_POLICY = [ '- Use concise questions with clear option labels. The user can always pick "Other" and type their own answer, so never add an "Other" option yourself.', '- For AskUserQuestion: aim for ~3 options per question. Prefer asking more focused questions (split complex choices across multiple questions) rather than listing many options in one question. Set multiSelect when the choices are not mutually exclusive.', '- When the options are things to compare by eye (layouts, snippets, configurations), give each one an HTML preview. The user sees it in a sandboxed pane, so keep it self-contained with inline styles.', + ...BACKGROUND_COMMAND_POLICY_LINES, '', ...SUBAGENT_RESULT_POLICY_LINES, ].join('\n') @@ -255,6 +271,7 @@ const AGENT_TOOL_POLICY = [ 'Tool usage policy:', '- You cannot ask the user questions directly (AskUserQuestion) 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/chat/streaming-blocks.ts b/src/lib/chat/streaming-blocks.ts index 39c68dd4..7322350f 100644 --- a/src/lib/chat/streaming-blocks.ts +++ b/src/lib/chat/streaming-blocks.ts @@ -16,7 +16,11 @@ import { isAskUserToolName, type AskQuestion, } from '../engine/ask-user-question' -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' @@ -49,6 +53,8 @@ export type ToolBlock = { * generic card the default rather than a fallback. */ details?: ToolResultDetails + /** Characters of a background command's output the card has, since its last reset (#35) — see `applyShellOutput`. */ + shellStreamed?: number } export type ThinkingBlock = { @@ -107,6 +113,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 @@ -153,7 +169,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) } : {}), }) } } @@ -370,6 +386,93 @@ export function applyToolProgress( ) } +/** + * `shell_output` and `shell_output_checkpoint` frames (#35) — output from a backgrounded + * command, added to its card. + * + * `shell_output` arrives about once a second and is live-only. `shell_output_checkpoint` is + * the same output saved every few seconds, and the resume replay delivers it, so a page that + * reloaded or reconnected mid-turn — which gets only saved frames from then on — still catches + * up and keeps moving, a few seconds behind. A connected page gets both; the positions + * (`from` / `to`, in characters since the last reset) sort that out: + * + * - `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 ends where the card already is, or before, is one it has. One that overlaps + * adds only its new part. + * - A chunk that starts past where the card left off means output was missed, so the card is + * marked as a tail rather than passing a fragment off as the whole thing. + * + * Capped to the same tail the server keeps (`appendStreamTail`), so the live card and the + * persisted block agree. + */ +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 have = b.shellStreamed ?? 0 + const to = typeof payload.to === 'number' ? payload.to : (reset ? 0 : have) + chunk.length + const from = typeof payload.from === 'number' ? payload.from : to - chunk.length + let added = chunk + let gap = false + if (!reset) { + if (to <= have) return b + if (from > have) gap = true + else added = chunk.slice(Math.max(0, chunk.length - (to - have))) + } + const next = appendStreamTail(reset ? '' : b.details.stdout, added) + return { + ...b, + shellStreamed: to, + 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..62d4e75f --- /dev/null +++ b/src/lib/chat/terminal-text.ts @@ -0,0 +1,67 @@ +/** + * 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. + * + * `clipped` says the text is the tail of something longer, cut at an arbitrary character. Its + * first line is then a fragment, and can start with half an escape sequence — `[31m` with its + * ESC cut off — that no pattern here can tell from real text. So that line is dropped, as long + * as there is a line after it; the card already says earlier output was trimmed. + */ +export function cleanTerminalText(text: string, options: { clipped?: boolean } = {}): string { + if (!text) return '' + let source = text + if (options.clipped) { + const firstBreak = source.indexOf('\n') + if (firstBreak >= 0 && firstBreak < source.length - 1) source = source.slice(firstBreak + 1) + } + const stripped = source.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/lib/engine/background-shells.server.ts b/src/lib/engine/background-shells.server.ts new file mode 100644 index 00000000..fe2a5486 --- /dev/null +++ b/src/lib/engine/background-shells.server.ts @@ -0,0 +1,353 @@ +/** + * 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. + * - **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 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, + * 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 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, 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' +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. + * + * `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 + 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 + /** + * 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 where it ends: `from + chunk.length`. */ + 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 +} + +/** 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 + 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 + /** + * 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 — + * 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 }): 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 + * `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 + +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 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) => { + // 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 = (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 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 done: ShellTaskDoneFrame = { + id: entry.toolUseId, + taskId: entry.taskId, + status, + exitCode, + stdout: entry.details.stdout, + truncated: entry.details.truncated, + } + 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 { + async track({ toolUseId, sessionId, resultText, details }) { + const taskId = details.backgroundTaskId + if (!taskId || shells.has(taskId)) return + const settled = takeEarly(taskId, toolUseId) + const entry: TrackedShell = { + toolUseId, + taskId, + sessionId, + details, + tail: null, + streamed: 0, + settled: false, + live: !settled, + unsaved: null, + savedAt: 0, + } + shells.set(taskId, entry) + 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 = 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 && + task.outputFile && + entry.sessionId && + isTaskOutputPath(task.outputFile, { sessionId: entry.sessionId, taskId: entry.taskId }) + ) { + entry.tail = startTail(entry, task.outputFile) + } + 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 + 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 { + 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() { + 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/builtin-tools.ts b/src/lib/engine/builtin-tools.ts index 4f0d0810..09d8e403 100644 --- a/src/lib/engine/builtin-tools.ts +++ b/src/lib/engine/builtin-tools.ts @@ -15,11 +15,24 @@ import { ASK_USER_QUESTION_TOOL } from './ask-user-question' 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. @@ -74,6 +87,10 @@ export const BUILTIN_TOOL_SET: ReadonlySet = new Set([ 'NotebookEdit', 'TodoWrite', ASK_USER_QUESTION_TOOL, + // Old spellings a stored config may still use — see `BUILTIN_SHELL_TOOLS`. + 'KillShell', + 'KillBash', + ...REMOVED_BUILTIN_TOOLS, ]) /** 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 b58207e4..61a953d8 100644 --- a/src/lib/engine/stream.server.ts +++ b/src/lib/engine/stream.server.ts @@ -13,6 +13,9 @@ * 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_output_checkpoint { same } (persisted, at most every few seconds) + * 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 +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. + * 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 @@ -44,6 +48,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 { ASK_USER_QUESTION_TOOL, answerAskUserQuestion, type AskUserHost } from './ask-user-question' import { askUserQuestionDetails } from './tool-result-details' import { readTurnUsage, resultErrorMessage, type EngineUsage, type SessionUsage } from './run-result' @@ -200,6 +205,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 @@ -275,6 +282,18 @@ export async function runEngineStream(input: EngineRunInput): Promise>() + /* + * #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 @@ -634,6 +653,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 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 + * 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 +} + +/** + * 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 + +/** + * 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 named pipe, a real path somewhere else) is given up on for + * 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 +}): 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(() => {}) + .then(() => (final || stopped || !input.onPoll ? undefined : input.onPoll())) + .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/lib/engine/tool-result-details.ts b/src/lib/engine/tool-result-details.ts index 663b9d97..061235dd 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' @@ -126,7 +149,11 @@ export const MAX_ANSWER_CHARS = 4_000 /** 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 ( @@ -159,6 +186,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[] = [] @@ -274,6 +353,7 @@ function shellDetails( timedOutAfterMs: num(result.timedOutAfterMs), persistedOutputPath: str(result.persistedOutputPath) ?? str(result.rawOutputPath), truncated: stdout.truncated || stderr.truncated, + returnCodeInterpretation: str(result.returnCodeInterpretation), } } @@ -360,6 +440,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/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/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/src/routes/chat/[id]/+page.svelte b/src/routes/chat/[id]/+page.svelte index e8b62147..f9d86599 100644 --- a/src/routes/chat/[id]/+page.svelte +++ b/src/routes/chat/[id]/+page.svelte @@ -73,6 +73,8 @@ applyToolPending, applyToolProgress, applyToolResult, + applyShellOutput, + applyShellTaskDone, buildDisplayedMessages, estimateTokens, finalizeText, @@ -1069,6 +1071,14 @@ }); } + // #35 — a backgrounded command's live output, and how it ended. + if (eventName === 'shell_output' || eventName === 'shell_output_checkpoint') { + 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 @@ -1154,6 +1164,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/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/src/routes/chat/[id]/stream/+server.ts b/src/routes/chat/[id]/stream/+server.ts index e4ba3770..eff6b468 100644 --- a/src/routes/chat/[id]/stream/+server.ts +++ b/src/routes/chat/[id]/stream/+server.ts @@ -477,8 +477,13 @@ 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. 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']) + 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/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) + } + }) +}) diff --git a/tests/chat.shell-output-render.spec.ts b/tests/chat.shell-output-render.spec.ts new file mode 100644 index 00000000..787baea5 --- /dev/null +++ b/tests/chat.shell-output-render.spec.ts @@ -0,0 +1,219 @@ +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('the saved checkpoints alone keep the card moving, as a reconnected page gets them', async ({ page }) => { + // A page that reloads or reconnects mid-turn is replayed only what was saved: no + // `shell_output`, just `shell_output_checkpoint` every few seconds. + test.setTimeout(90_000) + const prefix = uniquePrefix('chat-shell-saved-live') + await cleanupPrefixedRecords(prefix) + await authenticateContext(page.context()) + const conversation = await seedConversation(prefix, { userId: await getActiveUserId() }) + const first = lines(3) + const frames = backgroundFrames(randomUUID()).filter((f) => f.event !== 'shell_output') + frames.push( + { id: 4, event: 'shell_output_checkpoint', data: { id: 'toolu_bg1', taskId: 'b1', chunk: first, reset: true, truncated: false, from: 0, to: first.length } }, + { id: 5, event: 'shell_output_checkpoint', data: { id: 'toolu_bg1', taskId: 'b1', chunk: 'compiled\n', reset: false, truncated: false, from: first.length, to: first.length + 9 } }, + ) + const { release } = await scriptHeldRun(page, conversation.id, frames) + + 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-03/)).toBeVisible() + await expect(card.getByText(/compiled/)).toBeVisible() + await expect(card.getByText('Earlier output trimmed — showing the tail.')).toHaveCount(0) + } 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..f90f7a76 --- /dev/null +++ b/tests/chat.shell-output.spec.ts @@ -0,0 +1,168 @@ +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('a tail cut mid-escape loses its fragment of a first line, not the rest', () => { + // The ESC was cut off with the head, so `[31m` is no longer an escape anything can spot. + expect(cleanTerminalText('[31mred start\nnext line\n', { clipped: true })).toBe('next line\n') + expect(cleanTerminalText('[31mred start\nnext line\n')).toBe('[31mred start\nnext line\n') + // A single line with nothing after it is all there is: kept. + expect(cleanTerminalText('one long line', { clipped: true })).toBe('one long line') + }) + + 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 saved checkpoint the card already has changes nothing; one that overlaps adds only what is new', () => { + let blocks: StreamingBlock[] = [shellBlock()] + 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 }) + + // A connected page gets the checkpoint of what it already has, just after it. + expect(applyShellOutput(blocks, { id: 'bg1', chunk: 'two\n', reset: false, from: 4, to: 8 })).toEqual(blocks) + const sameReset = applyShellOutput(blocks, { id: 'bg1', chunk: 'one\ntwo\n', reset: true, from: 0, to: 8 }) + expect(detailsOf(sameReset)).toMatchObject({ stdout: 'one\ntwo\n', truncated: false }) + + // A page that reconnected has up to 8; the replayed checkpoint covers 4 to 12. + const caughtUp = applyShellOutput(blocks, { id: 'bg1', chunk: 'two\nsix\n', reset: false, from: 4, to: 12 }) + expect(detailsOf(caughtUp)).toMatchObject({ stdout: 'one\ntwo\nsix\n', truncated: false }) + }) + + test('a chunk that does not start where the card left off marks the output as a tail', () => { + // Output the page never saw: a chunk cut to the cap, or frames lost on the way. + 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') + }) +}) 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.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.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..55d504d0 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,288 @@ 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') + + // 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({ + 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 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 { + 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..7b32f238 --- /dev/null +++ b/tests/engine.task-output.spec.ts @@ -0,0 +1,307 @@ +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' +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('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. + 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') + 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 }) + } + }) +}) 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..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' @@ -175,6 +176,59 @@ test.describe('shell', () => { // Something else wearing the name is not a shell result. 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() + 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..425a4e30 100644 --- a/tests/engine.workspace-guard.spec.ts +++ b/tests/engine.workspace-guard.spec.ts @@ -132,6 +132,18 @@ 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. This already held before #35 (calls arrive as + // `TaskStop`, which was never a command tool); it is pinned because the docs now say so. + 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