From a3d0fd7642b975dc779277fc008ee2f75488a8d2 Mon Sep 17 00:00:00 2001 From: Adam Cheng <63501289+627150795@users.noreply.github.com> Date: Sat, 29 Aug 2026 20:09:58 +0800 Subject: [PATCH 1/4] fix(subagents): preserve transport payload and surface artifact failures --- README.md | 4 +- SETUP.md | 2 +- extensions/setup/index.ts | 2 +- extensions/shared/setup-config.ts | 4 +- extensions/subagents/index.ts | 145 +++++++--- extensions/subagents/src/prompt.ts | 36 ++- extensions/subagents/src/result-artifact.ts | 4 + extensions/subagents/src/ui/wait-result.ts | 53 +++- tests/extensions/setup/index.test.ts | 2 +- tests/extensions/subagents/index.test.ts | 150 ++++++++++- .../subagents/result-artifact.test.ts | 1 + .../subagents/result-rendering.test.ts | 255 ++++++++++++++++-- .../extensions/subagents/wait-result.test.ts | 28 +- tests/extensions/suggestions/config.test.ts | 10 +- 14 files changed, 615 insertions(+), 81 deletions(-) diff --git a/README.md b/README.md index f237a321..504b3bdf 100644 --- a/README.md +++ b/README.md @@ -360,7 +360,7 @@ Footer 使用一套 Codicon 线性图标：`` 模型、`` context、`` - 默认把高频的模型与 context 放在最左侧，把项目定位信息归到右侧，并以当前目录作为最右锚点；支持 `powerline`、`powerline-mono`、`compact`，也支持自定义多行布局； - 终端变窄时按优先级隐藏次要指标，不机械截断尾部； - Subagent 与 Workflow 活动时自动出现，空闲时不占空间； -- Bash、Write/Edit 与 Subagent 结果可独立选择 `full` 或 `compact`；普通 `read`、`grep`、`find`、`ls` 以及 compact Bash/Write/Edit 默认显示一行语义活动摘要，包含目标、状态与关键规模；Nerd Font 可为读取、终端、编辑、搜索和目录动作显示 Codex 风格线框图标，未安装时动词与全部信息仍保持可读； +- Bash、Write/Edit 与 Subagent 结果可独立选择 `full` 或 `compact`，默认均为 `compact`；普通 `read`、`grep`、`find`、`ls` 以及 compact Bash/Write/Edit 默认显示一行语义活动摘要，包含目标、状态与关键规模；Nerd Font 可为读取、终端、编辑、搜索和目录动作显示 Codex 风格线框图标，未安装时动词与全部信息仍保持可读； - 折叠内容用 Pi 的 `app.tools.expand` 快捷键临时展开（默认 `Ctrl+O`），展开后直接恢复 Pi 原生参数、输出、错误、diff、耗时与 full-output 证据； - Git 状态本地刷新；只有显式运行 `/pr` 才查询 GitHub PR。 @@ -430,7 +430,7 @@ Footer 布局以 `footerLines` 作为唯一持久化格式。旧版 `footerItems | Workflow 并发 / 总调用 | 8 / 128；硬上限 64 / 1024 | | 大型 Header | 关闭 | | Dashboard Footer | 开启；单行 `plain` | -| Subagent / Bash / Write/Edit | `full` / `compact` / `compact` | +| Subagent / Bash / Write/Edit | `compact` / `compact` / `compact` | | Post-edit 命令 | 关闭；单条命令最多 500 字符 | | 内置角色模型 | 全部继承父模型 | | 主题 | 保留用户现有选择 | diff --git a/SETUP.md b/SETUP.md index 60f9e06e..0a5d8a97 100644 --- a/SETUP.md +++ b/SETUP.md @@ -56,7 +56,7 @@ Use the single canonical package-owned command. `/my-pi-setup` remains a compati /openpi-setup 清除 explorer 的模型，让它继承父模型 ``` -Capability discovery defaults to `explicit`, preserving the zero-resident OpenPI tool surface until the user asks for a capability. The case-insensitive English words `subagent` and `workflow` are reserved authorization words: entering either word is sufficient to load its capability group, and the interactive editor shows it in Claude Code-style lavender before submission, with a darker purple fallback for light themes. This makes discussion that contains either English word an intentional opt-in tradeoff; conditional and negated clauses remain inert, while Chinese capability names still require a recognized action request. `adaptive` is an explicit opt-in that keeps only `openpi_load_tools` visible and allows the model to load a useful group on its own; because this can start Subagents, Workflows, or background processes, normal permission and configured concurrency/call limits still apply. Changing the setting updates the current Session immediately, while already loaded groups remain stable for that Session. Accepted Suggestion text is classified only after it becomes real editor input. The visual feedback and runtime activation share one fail-closed intent classifier. Next-action suggestions default to off. Run `/openpi-setup` to explicitly choose an available model and reasoning level. After a fully settled main-agent run, one suggestion may appear as dim inline text on the first row of an empty editor; reserved cells at the row end keep CJK IME preedit from overwriting it. `Right` accepts it into the editor without submitting, while any other editor input dismisses it. Suggestions are ephemeral and never enter session history or model context. Workflows default to 8 concurrent agents and 128 total agent calls per run; configurable hard maxima are 64 and 1024. The large decorative header defaults off and the custom dashboard footer defaults on with a one-line plain layout (`model context |flex| git pr cwd`; `thinking`, `cache`, `cost`, and `throughput` remain opt-in metrics). Footer presets are `powerline`, `powerline-mono`, and `compact`; style can also be set independently to `plain`, `powerline`, or `powerline-mono`. Custom layouts use a 2D `footerLines` array with at most one `flex` per row for left/right alignment. Nerd Font affects powerline separator glyphs (``) and adds Codex-style outline icons to compact read, terminal, edit, search, and directory activity rows; all accompanying text remains readable without it. Footer metrics use one Codicon outline family (`` model, `` context, and `` directory) plus `⎇` for the branch. A Nerd Font containing Codicons renders them as designed; without one, the text labels remain readable even if an icon falls back to an empty box. Footer changes apply immediately in the active TUI session. Subagent results default to the existing full display; compact mode shows only bounded status rows and keeps raw child reports behind `app.tools.expand` (`Ctrl+O` by default). Ordinary `read`, `grep`, `find`, and `ls` operations render as one-line semantic activity summaries. Bash and Write/Edit default to the same activity-row projection, showing the target, running/success/failure state, and useful line or diff counts without replaying stdout or previews. Select full independently for Bash or Write/Edit to keep Pi's native rendering expanded. `app.tools.expand` temporarily restores the native arguments, output, errors, diff, timing, images, truncation notices, and full-output metadata; it never changes Session history or model context. An optional post-edit command is off by default: set one (for example `npm run format`, maximum 500 characters) and it runs once in the background after each interactive-TUI turn with successful Write/Edit operations, with failures reported as a notification. It deliberately does not guess whether arbitrary Bash commands changed files. Built-in Agent roles `explorer`, `implementer`, `reviewer`, and `advisor` are shared by `subagent_spawn.agent_type` and Workflow `agent(..., { agent_type })`; all inherit the parent model by default. `/openpi-setup` may assign a currently available Registry model to any subset; clearing one returns it to inheritance and omitted roles stay unchanged. Model precedence is explicit call > selected role-file model > setup assignment > parent inheritance; effort is explicit call > selected role > parent. A trusted project `.pi/agents/.md` overrides global `~/.pi/agent/agents/.md`, which overrides the complete built-in role definition; overrides are diagnosed. Role-model changes apply to the next spawn or Workflow agent call without reload. Configuration is stored privately at `~/.pi/agent/my-pi-setup.json`. +Capability discovery defaults to `explicit`, preserving the zero-resident OpenPI tool surface until the user asks for a capability. The case-insensitive English words `subagent` and `workflow` are reserved authorization words: entering either word is sufficient to load its capability group, and the interactive editor shows it in Claude Code-style lavender before submission, with a darker purple fallback for light themes. This makes discussion that contains either English word an intentional opt-in tradeoff; conditional and negated clauses remain inert, while Chinese capability names still require a recognized action request. `adaptive` is an explicit opt-in that keeps only `openpi_load_tools` visible and allows the model to load a useful group on its own; because this can start Subagents, Workflows, or background processes, normal permission and configured concurrency/call limits still apply. Changing the setting updates the current Session immediately, while already loaded groups remain stable for that Session. Accepted Suggestion text is classified only after it becomes real editor input. The visual feedback and runtime activation share one fail-closed intent classifier. Next-action suggestions default to off. Run `/openpi-setup` to explicitly choose an available model and reasoning level. After a fully settled main-agent run, one suggestion may appear as dim inline text on the first row of an empty editor; reserved cells at the row end keep CJK IME preedit from overwriting it. `Right` accepts it into the editor without submitting, while any other editor input dismisses it. Suggestions are ephemeral and never enter session history or model context. Workflows default to 8 concurrent agents and 128 total agent calls per run; configurable hard maxima are 64 and 1024. The large decorative header defaults off and the custom dashboard footer defaults on with a one-line plain layout (`model context |flex| git pr cwd`; `thinking`, `cache`, `cost`, and `throughput` remain opt-in metrics). Footer presets are `powerline`, `powerline-mono`, and `compact`; style can also be set independently to `plain`, `powerline`, or `powerline-mono`. Custom layouts use a 2D `footerLines` array with at most one `flex` per row for left/right alignment. Nerd Font affects powerline separator glyphs (``) and adds Codex-style outline icons to compact read, terminal, edit, search, and directory activity rows; all accompanying text remains readable without it. Footer metrics use one Codicon outline family (`` model, `` context, and `` directory) plus `⎇` for the branch. A Nerd Font containing Codicons renders them as designed; without one, the text labels remain readable even if an icon falls back to an empty box. Footer changes apply immediately in the active TUI session. Subagent results default to a compact status summary; full mode remains available as a per-user setting, and `app.tools.expand` (`Ctrl+O` by default) reveals the available child report. Ordinary `read`, `grep`, `find`, and `ls` operations render as one-line semantic activity summaries. Bash and Write/Edit default to the same activity-row projection, showing the target, running/success/failure state, and useful line or diff counts without replaying stdout or previews. Select full independently for Bash or Write/Edit to keep Pi's native rendering expanded. `app.tools.expand` temporarily restores the native arguments, output, errors, diff, timing, images, truncation notices, and full-output metadata; it never changes Session history or model context. An optional post-edit command is off by default: set one (for example `npm run format`, maximum 500 characters) and it runs once in the background after each interactive-TUI turn with successful Write/Edit operations, with failures reported as a notification. It deliberately does not guess whether arbitrary Bash commands changed files. Built-in Agent roles `explorer`, `implementer`, `reviewer`, and `advisor` are shared by `subagent_spawn.agent_type` and Workflow `agent(..., { agent_type })`; all inherit the parent model by default. `/openpi-setup` may assign a currently available Registry model to any subset; clearing one returns it to inheritance and omitted roles stay unchanged. Model precedence is explicit call > selected role-file model > setup assignment > parent inheritance; effort is explicit call > selected role > parent. A trusted project `.pi/agents/.md` overrides global `~/.pi/agent/agents/.md`, which overrides the complete built-in role definition; overrides are diagnosed. Role-model changes apply to the next spawn or Workflow agent call without reload. Configuration is stored privately at `~/.pi/agent/my-pi-setup.json`. Legacy `footerItems` is accepted and migrated at the input boundary, but new setup writes persist only canonical `footerLines`. Configurations written by this version are not guaranteed to retain their Footer layout when read by an older OpenPI version. diff --git a/extensions/setup/index.ts b/extensions/setup/index.ts index 1769c73b..b0f700c4 100644 --- a/extensions/setup/index.ts +++ b/extensions/setup/index.ts @@ -133,7 +133,7 @@ export function buildInteractiveSetupPrompt(options: { "- UI: the large header costs vertical space; the custom footer is a declarative dashboard. Presets: powerline (one-line ANSI256 blocks), powerline-mono (one-line high-contrast gray powerline), and compact (one-line plain text); the default is plain with model/context on the left and git/pr/cwd on the right. Style can also be set independently: plain, powerline, powerline-mono. Custom lines are a 2D layout of cwd/model/thinking/context/cache/cost/throughput/git/pr plus at most one flex per line for left/right alignment. Footer metrics use Codicon outline glyphs for model, context, and directory; a Nerd Font renders them as designed while the text stays readable without it. Changes apply immediately in the active TUI session.", "- Operational activity for Subagents, Workflows, and background terminals is core status and always remains visible whenever the custom footer is enabled.", "- Post-edit command: one optional shell command (maximum 500 characters) run in the background after a turn with successful Write/Edit operations (e.g. `npm run format`). Off by default, interactive TUI sessions only, failures surface as a notification. This is a single command, not an event-hook system.", - "- Result detail display: Subagent results, Bash operations, and Write/Edit operations can each default to full or compact. Compact Subagent results show only bounded status rows and keep raw child reports behind app.tools.expand; compact Bash and Write/Edit operations use one-line semantic activity summaries. Read, grep, find, and ls use the same compact activity-row projection. Ctrl+O restores Pi's native full arguments, output, errors, diffs, and timing. Bash and Write/Edit default to compact. Recommend compact for users who scan activity first and inspect evidence on demand.", + "- Result detail display: Subagent results, Bash operations, and Write/Edit operations can each default to full or compact; all three default to compact. Compact Subagent results show only bounded status rows and keep raw child reports behind app.tools.expand; compact Bash and Write/Edit operations use one-line semantic activity summaries. Read, grep, find, and ls use the same compact activity-row projection. Ctrl+O restores Pi's native full arguments, output, errors, diffs, and timing. Recommend compact for users who scan activity first and inspect evidence on demand.", "- Agent role models: built-in explorer, implementer, reviewer, and advisor roles are shared by subagent_spawn and workflow agent_type, and inherit the parent model by default. Assign only an available registry model to an individual role when needed; clearing that role returns it to inheritance. Custom agent-type files still override a built-in role's complete definition.", "", "Natural-language configuration examples the user might ask for:", diff --git a/extensions/shared/setup-config.ts b/extensions/shared/setup-config.ts index ca1d66b9..25795a26 100644 --- a/extensions/shared/setup-config.ts +++ b/extensions/shared/setup-config.ts @@ -183,7 +183,7 @@ export const DEFAULT_SETUP_CONFIG: MyPiSetupConfig = { customFooter: true, footerStyle: DEFAULT_FOOTER_STYLE, footerLines: DEFAULT_FOOTER_LINES, - subagentResultDisplay: "full", + subagentResultDisplay: "compact", bashToolDisplay: "compact", fileMutationDisplay: "compact", }, @@ -493,7 +493,7 @@ export function parseSetupConfig(value: unknown): MyPiSetupConfig { ui.subagentResultDisplay as DetailDisplay, ) ? (ui.subagentResultDisplay as DetailDisplay) - : "full", + : "compact", bashToolDisplay: DETAIL_DISPLAYS.includes( ui.bashToolDisplay as DetailDisplay, ) diff --git a/extensions/subagents/index.ts b/extensions/subagents/index.ts index 9c74aba5..7288d43b 100644 --- a/extensions/subagents/index.ts +++ b/extensions/subagents/index.ts @@ -49,6 +49,7 @@ import { hasActivity, unreadActivityCounts, } from "../shared/activity-status.ts"; +import { sanitizeText } from "../shared/agent-transcript.ts"; import { BelowEditorNavigationEditor, BelowEditorStripState, @@ -107,6 +108,7 @@ import { } from "./src/id-sequence.ts"; import { SubagentManager, type SubagentManagerShape } from "./src/manager.ts"; import { + buildSubagentResultDisplayMessage, buildSubagentResultMessage, buildSubagentSendResult, buildSubagentSpawnResult, @@ -120,10 +122,15 @@ import { SUBAGENT_SEND_TOOL_DESCRIPTION, SUBAGENT_SPAWN_PROMPT_GUIDELINES, SUBAGENT_SPAWN_PROMPT_SNIPPET, + stripSubagentResultTransportInstruction, SUBAGENT_WAIT_PARAMETER_DESCRIPTIONS, SUBAGENT_WAIT_TOOL_DESCRIPTION, } from "./src/prompt.ts"; -import { persistResultArtifact, projectResult } from "./src/result-artifact.ts"; +import { + persistResultArtifact, + projectResult, + type ResultProjection, +} from "./src/result-artifact.ts"; import { allocateResultBudgets, type ParentContextUsage, @@ -136,7 +143,7 @@ import { } from "./src/runtime.ts"; import { openSubagentPicker, openSubagentTakeover } from "./src/ui/takeover.ts"; import { - buildWaitResultPreview, + renderWaitResultPreview, renderWaitResult, type WaitResultDetails, } from "./src/ui/wait-result.ts"; @@ -169,12 +176,18 @@ interface SubagentResultDetails { readonly id?: string; readonly title?: string; readonly status?: SubagentSnapshot["status"]; + readonly elapsed?: string; + readonly artifactSaveFailed?: boolean; readonly count?: number; readonly results?: ReadonlyArray<{ readonly id: string; readonly title: string; readonly status: SubagentSnapshot["status"]; + readonly elapsed?: string; + readonly artifactSaveFailed?: boolean; }>; + /** Display-only projection for the custom message renderer. */ + readonly displayContent?: string; } interface SubagentResultEntryData { @@ -216,12 +229,32 @@ export function truncatedOutput( }).text; } +function projectSubagentOutput( + snap: SubagentSnapshot, + maxBytes: number, +): ResultProjection { + const output = snap.finalText || "(no output)"; + return projectResult(output, { + maxBytes: Math.min(maxBytes, DEFAULT_MAX_BYTES), + maxLines: Math.min(600, DEFAULT_MAX_LINES), + writeArtifact: (content) => persistResultArtifact(getAgentDir(), content), + }); +} + +type OutputProjection = Pick; + +function normalizeProjection( + output: string | OutputProjection, +): OutputProjection { + return typeof output === "string" ? { text: output } : output; +} + export function createSubagentResultDispatcher( pi: ExtensionAPI, outputFor: ( snap: SubagentSnapshot, maxBytes: number, - ) => string = truncatedOutput, + ) => string | OutputProjection = projectSubagentOutput, getContextUsage: () => ParentContextUsage | undefined = () => undefined, ) { return (snaps: readonly SubagentSnapshot[]) => { @@ -259,6 +292,21 @@ export function createSubagentResultDispatcher( fixedBytes: wrapperBytes, }, ); + const projections = snaps.map((snap, index) => + normalizeProjection(outputFor(snap, allocation.budgets[index]!)), + ); + const outputs = projections.map((projection) => projection.text); + const displayContent = snaps + .map((snap, index) => + buildSubagentResultDisplayMessage({ + id: snap.id, + title: snap.title, + status: snap.status, + errorText: snap.errorText, + output: outputs[index]!, + }), + ) + .join("\n\n"); const content = snaps .map((snap, index) => buildSubagentResultMessage({ @@ -266,7 +314,7 @@ export function createSubagentResultDispatcher( title: snap.title, status: snap.status, errorText: snap.errorText, - output: outputFor(snap, allocation.budgets[index]!), + output: outputs[index]!, }), ) .join("\n\n"); @@ -276,17 +324,25 @@ export function createSubagentResultDispatcher( id: snaps[0]!.id, title: snaps[0]!.title, status: snaps[0]!.status, + elapsed: formatElapsed(snaps[0]!), + ...(projections[0]!.artifactSaveFailed + ? { artifactSaveFailed: true } + : {}), } : { count: snaps.length, - results: snaps.map((snap) => ({ + results: snaps.map((snap, index) => ({ id: snap.id, title: snap.title, status: snap.status, + elapsed: formatElapsed(snap), + ...(projections[index]!.artifactSaveFailed + ? { artifactSaveFailed: true } + : {}), })), }; pi.appendEntry("subagent-result", { - content, + content: displayContent, details, }); pi.sendMessage( @@ -294,7 +350,7 @@ export function createSubagentResultDispatcher( customType: "subagent-result", content, display: false, - details, + details: { ...details, displayContent }, }, { deliverAs: "followUp", triggerTurn: true }, ); @@ -309,34 +365,46 @@ function renderSubagentResult( expanded: boolean, theme: SubagentResultTheme, ) { + const displayContent = sanitizeText( + stripSubagentResultTransportInstruction(details.displayContent ?? content), + ); + const results = details.results?.length + ? details.results + : details.id + ? [ + { + id: details.id, + title: details.title, + status: details.status, + elapsed: details.elapsed, + artifactSaveFailed: details.artifactSaveFailed, + }, + ] + : []; if (!expanded && loadSetupConfig().ui.subagentResultDisplay === "compact") { - const results = details.results?.length - ? details.results - : details.id - ? [ - { - id: details.id, - title: details.title, - status: details.status, - }, - ] - : []; - return new Text(buildWaitResultPreview(content, { results }, theme), 0, 0); + return renderWaitResultPreview(displayContent, { results }, theme); } - const failed = details.status === "error"; + const failed = results.some((result) => result.status === "error"); + const batched = results.length > 1; const icon = failed ? theme.fg("error", "x") : theme.fg("success", "✓"); - const header = - `${icon} ` + - theme.fg("accent", theme.bold(`subagent ${details.id ?? "?"}`)) + - theme.fg( - "muted", - ` · ${details.title ?? ""} · ${failed ? "failed" : "finished"}`, - ); + const header = batched + ? `${icon} ${theme.fg("accent", theme.bold(`${results.length} subagents`))}${theme.fg("muted", ` · ${failed ? `${results.filter((result) => result.status === "error").length} failed` : "finished"}`)}` + : `${icon} ` + + theme.fg( + "accent", + theme.bold(`subagent ${sanitizeText(details.id ?? "?")}`), + ) + + theme.fg( + "muted", + ` · ${sanitizeText(details.title ?? "")} · ${failed ? "failed" : "finished"}${details.elapsed ? ` · ${sanitizeText(details.elapsed)}` : ""}`, + ); - // Remove only the summary line. The following Error line (when present) - // is part of the actual result and must remain visible. - const body = content.split("\n").slice(1).join("\n").trim(); + // Remove only the single-result summary line. Error lines and batched result + // summaries are part of the display projection and must remain visible. + const body = batched + ? displayContent.trim() + : displayContent.split("\n").slice(1).join("\n").trim(); const md = new Markdown(body, 0, 0, getMarkdownTheme()); const container = new Text(header, 0, 0); return { @@ -377,7 +445,7 @@ export default function (pi: ExtensionAPI) { let dashboardOpen = false; const dispatchResults = createSubagentResultDispatcher( pi, - truncatedOutput, + projectSubagentOutput, () => sessionContext?.getContextUsage(), ); const resultDelivery = createSubagentResultDelivery({ @@ -1001,10 +1069,13 @@ export default function (pi: ExtensionAPI) { }, ); let resultIndex = 0; + const artifactSaveFailures = new Set(); const sections = entries.map((entry) => { if ("section" in entry) return entry.section; const outputBudget = allocation.budgets[resultIndex++]!; - return `${entry.header}\n\n${truncatedOutput(entry.snap, outputBudget)}`; + const projection = projectSubagentOutput(entry.snap, outputBudget); + if (projection.artifactSaveFailed) artifactSaveFailures.add(entry.id); + return `${entry.header}\n\n${projection.text}`; }); const combined = sections.join("\n\n---\n\n"); @@ -1020,7 +1091,15 @@ export default function (pi: ExtensionAPI) { details: { results: ids.map((id) => { const snap = manager.view.get(id); - return { id, title: snap?.title, status: snap?.status }; + return { + id, + title: snap?.title, + status: snap?.status, + ...(snap ? { elapsed: formatElapsed(snap) } : {}), + ...(artifactSaveFailures.has(id) + ? { artifactSaveFailed: true } + : {}), + }; }), }, }; diff --git a/extensions/subagents/src/prompt.ts b/extensions/subagents/src/prompt.ts index 024fbc90..438893e6 100644 --- a/extensions/subagents/src/prompt.ts +++ b/extensions/subagents/src/prompt.ts @@ -285,8 +285,11 @@ export const SUBAGENT_CHECK_PARAMETER_DESCRIPTIONS = { export const SUBAGENT_LIST_TOOL_DESCRIPTION = "List all subagents (running and finished) with their status."; -/** Builds the child completion/failure wrapper injected into the parent model's context. */ -export function buildSubagentResultMessage(options: { +const SUBAGENT_RESULT_TRANSPORT_INSTRUCTION = + "(This result is already shown to the user. Act on it and relay only the decisions or next steps — do not repeat it verbatim.)"; + +/** Builds the user-visible child completion/failure projection. */ +export function buildSubagentResultDisplayMessage(options: { id: string; title: string; status: "running" | "done" | "error"; @@ -297,9 +300,34 @@ export function buildSubagentResultMessage(options: { let text = `Subagent ${options.id} "${options.title}" ${verb}.`; if (options.errorText) text += `\nError: ${options.errorText}`; text += `\n\n${options.output}`; + return text; +} + +/** Builds the child completion/failure wrapper injected into the parent model's context. */ +export function buildSubagentResultMessage(options: { + id: string; + title: string; + status: "running" | "done" | "error"; + errorText?: string; + output: string; +}) { + let text = buildSubagentResultDisplayMessage(options); // This message is already displayed to the user, so tell the parent to act on // it rather than reprint it verbatim. - text += - "\n\n(This result is already shown to the user. Act on it and relay only the decisions or next steps — do not repeat it verbatim.)"; + text += `\n\n${SUBAGENT_RESULT_TRANSPORT_INSTRUCTION}`; return text; } + +/** Remove the transport-only suffix from results persisted before the split. */ +export function stripSubagentResultTransportInstruction(content: string) { + const separator = `\n\n${SUBAGENT_RESULT_TRANSPORT_INSTRUCTION}`; + const withoutBatchedSeparators = content.replaceAll( + `${separator}\n\nSubagent `, + "\n\nSubagent ", + ); + return ( + withoutBatchedSeparators.endsWith(separator) + ? withoutBatchedSeparators.slice(0, -separator.length) + : withoutBatchedSeparators + ).trimEnd(); +} diff --git a/extensions/subagents/src/result-artifact.ts b/extensions/subagents/src/result-artifact.ts index 795a8bd0..066dd3da 100644 --- a/extensions/subagents/src/result-artifact.ts +++ b/extensions/subagents/src/result-artifact.ts @@ -20,6 +20,7 @@ export interface ResultProjection { readonly text: string; readonly truncated: boolean; readonly artifactPath?: string; + readonly artifactSaveFailed?: boolean; } function sliceStartToUtf8Bytes(content: string, maxBytes: number) { @@ -96,11 +97,13 @@ export function projectResult( const tailLines = Math.max(1, options.maxLines - headLines); let artifactPath: string | undefined; + let artifactSaveFailed = false; try { artifactPath = options.writeArtifact(content); } catch { // Delivery is more important than the optional recovery cache. The footer // below stays explicit so a failed write never advertises a false path. + artifactSaveFailed = true; } let bodyBudget = options.maxBytes; @@ -138,5 +141,6 @@ export function projectResult( text, truncated: true, ...(artifactPath ? { artifactPath } : {}), + ...(artifactSaveFailed ? { artifactSaveFailed: true } : {}), }; } diff --git a/extensions/subagents/src/ui/wait-result.ts b/extensions/subagents/src/ui/wait-result.ts index db880541..6f56542a 100644 --- a/extensions/subagents/src/ui/wait-result.ts +++ b/extensions/subagents/src/ui/wait-result.ts @@ -3,7 +3,11 @@ import { keyHint, type Theme, } from "@earendil-works/pi-coding-agent"; -import { Markdown, Text } from "@earendil-works/pi-tui"; +import { + type Component, + Markdown, + truncateToWidth, +} from "@earendil-works/pi-tui"; import { sanitizeText } from "../../../shared/agent-transcript.ts"; const MAX_STATUS_ROWS = 4; @@ -12,12 +16,27 @@ export interface WaitResultItem { readonly id: string; readonly title?: string; readonly status?: string; + readonly elapsed?: string; + readonly artifactSaveFailed?: boolean; } export interface WaitResultDetails { readonly results?: readonly WaitResultItem[]; } +function singleLine(value: string) { + return sanitizeText(value).replace(/\s+/gu, " ").trim(); +} + +function fixedRows(rows: readonly string[]): Component { + return { + render(width) { + return rows.map((row) => truncateToWidth(row, Math.max(1, width), "…")); + }, + invalidate() {}, + }; +} + export function buildWaitResultPreview( content: string, details: WaitResultDetails | undefined, @@ -25,10 +44,19 @@ export function buildWaitResultPreview( ) { const results = details?.results ?? []; const failed = results.filter((result) => result.status === "error").length; + const artifactFailures = results.filter( + (result) => result.artifactSaveFailed, + ).length; const header = theme.fg(failed > 0 ? "warning" : "success", failed > 0 ? "!" : "✓") + ` ${theme.fg("accent", theme.bold(`${results.length} subagent${results.length === 1 ? "" : "s"} settled`))}` + - (failed > 0 ? theme.fg("error", ` · ${failed} failed`) : ""); + (failed > 0 ? theme.fg("error", ` · ${failed} failed`) : "") + + (artifactFailures > 0 + ? theme.fg( + "warning", + ` · ${artifactFailures} artifact${artifactFailures === 1 ? "" : "s"} not saved`, + ) + : ""); const lines = [header]; for (const result of results.slice(0, MAX_STATUS_ROWS)) { @@ -37,15 +65,22 @@ export function buildWaitResultPreview( isFailure ? "error" : "success", isFailure ? "x" : "✓", ); + const id = singleLine(result.id); + const title = result.title ? singleLine(result.title) : ""; + const status = singleLine(result.status ?? "settled"); + const elapsed = result.elapsed ? singleLine(result.elapsed) : ""; + const artifact = result.artifactSaveFailed + ? theme.fg("warning", " · artifact not saved") + : ""; lines.push( - ` ${icon} ${theme.fg("accent", result.id)}${result.title ? theme.fg("muted", ` · ${result.title}`) : ""}${theme.fg("dim", ` · ${result.status ?? "settled"}`)}`, + ` ${icon} ${theme.fg("accent", id)}${title ? theme.fg("muted", ` · ${title}`) : ""}${theme.fg("dim", ` · ${status}${elapsed ? ` · ${elapsed}` : ""}`)}${artifact}`, ); } if (results.length > MAX_STATUS_ROWS) { lines.push(theme.fg("dim", ` … ${results.length - MAX_STATUS_ROWS} more`)); } - if (content.trim()) { + if (sanitizeText(content).trim()) { lines.push( theme.fg( "dim", @@ -56,6 +91,14 @@ export function buildWaitResultPreview( return lines.join("\n"); } +export function renderWaitResultPreview( + content: string, + details: WaitResultDetails | undefined, + theme: Theme, +) { + return fixedRows(buildWaitResultPreview(content, details, theme).split("\n")); +} + export function renderWaitResult( content: string, details: WaitResultDetails | undefined, @@ -63,7 +106,7 @@ export function renderWaitResult( theme: Theme, ) { if (!expanded) { - return new Text(buildWaitResultPreview(content, details, theme), 0, 0); + return renderWaitResultPreview(content, details, theme); } const markdown = new Markdown( diff --git a/tests/extensions/setup/index.test.ts b/tests/extensions/setup/index.test.ts index 053ba5e3..8883a3c9 100644 --- a/tests/extensions/setup/index.test.ts +++ b/tests/extensions/setup/index.test.ts @@ -691,7 +691,7 @@ test("builds a model-guided first-run setup prompt with impacts", () => { assert.match(message, /ui_footer_preset=powerline/); assert.match(message, /activity.*core status/); assert.match(message, /Result detail display/); - assert.match(message, /Bash and Write\/Edit default to compact/); + assert.match(message, /all three default to compact/); assert.match(message, /Recommend compact/); assert.match(message, /Post-edit defaults off/); assert.match(message, /Agent role models/); diff --git a/tests/extensions/subagents/index.test.ts b/tests/extensions/subagents/index.test.ts index d5a82b74..c12d0e12 100644 --- a/tests/extensions/subagents/index.test.ts +++ b/tests/extensions/subagents/index.test.ts @@ -8,6 +8,7 @@ import type { ExtensionAPI, ExtensionContext, } from "@earendil-works/pi-coding-agent"; +import { initTheme } from "@earendil-works/pi-coding-agent"; import { PLAN_MODE_CHANNEL } from "../../../extensions/shared/plan-mode-state.ts"; import subagents, { createSubagentResultDispatcher, @@ -15,6 +16,8 @@ import subagents, { } from "../../../extensions/subagents/index.ts"; import { projectResult } from "../../../extensions/subagents/src/result-artifact.ts"; +initTheme("dark", false); + const emptySessionManager = { getBranch: () => [] }; test("subagent results render before the hidden wake-up message", () => { @@ -56,12 +59,12 @@ test("subagent results render before the hidden wake-up message", () => { kind: "entry", customType: "subagent-result", data: { - content: - 'Subagent sa-3 "investigate plan mode" finished.\n\nreport\n\n(This result is already shown to the user. Act on it and relay only the decisions or next steps — do not repeat it verbatim.)', + content: 'Subagent sa-3 "investigate plan mode" finished.\n\nreport', details: { id: "sa-3", title: "investigate plan mode", status: "done", + elapsed: "1s", }, }, }, @@ -76,6 +79,9 @@ test("subagent results render before the hidden wake-up message", () => { id: "sa-3", title: "investigate plan mode", status: "done", + elapsed: "1s", + displayContent: + 'Subagent sa-3 "investigate plan mode" finished.\n\nreport', }, }, options: { deliverAs: "followUp", triggerTurn: true }, @@ -119,6 +125,94 @@ test("automatic result projection keeps both ends and persists the exact final a assert.match(text, /Full final answer: "\/tmp\/subagent-final\.txt"/); }); +test("automatic projection carries artifact save failures into result details", () => { + let entryDetails: Record | undefined; + const pi = { + appendEntry( + _customType: string, + data: { details: Record }, + ) { + entryDetails = data.details; + }, + sendMessage() {}, + } as unknown as ExtensionAPI; + const dispatch = createSubagentResultDispatcher(pi, () => ({ + text: "Full final answer could not be saved; only the head and tail above are available.", + artifactSaveFailed: true, + })); + + dispatch([ + { + id: "sa-artifact", + origin: "model", + backend: "pi", + title: "artifact test", + prompt: "inspect", + cwd: process.cwd(), + status: "done", + createdAt: 0, + settledAt: 1_000, + meta: { backend: "pi" }, + usage: {}, + transcript: [], + liveTools: [], + queued: [], + finalText: "x".repeat(40 * 1024), + turns: 1, + }, + ]); + + assert.equal(entryDetails?.artifactSaveFailed, true); +}); + +test("automatic delivery reports real artifact save failures", async () => { + const directory = await mkdtemp(path.join(tmpdir(), "openpi-artifact-dir-")); + const previousAgentDir = process.env.PI_CODING_AGENT_DIR; + process.env.PI_CODING_AGENT_DIR = directory; + + try { + await writeFile(path.join(directory, "cache"), "not a directory"); + let entryDetails: Record | undefined; + const pi = { + appendEntry( + _customType: string, + data: { details: Record }, + ) { + entryDetails = data.details; + }, + sendMessage() {}, + } as unknown as ExtensionAPI; + const dispatch = createSubagentResultDispatcher(pi); + + dispatch([ + { + id: "sa-real-artifact", + origin: "model", + backend: "pi", + title: "artifact test", + prompt: "inspect", + cwd: process.cwd(), + status: "done", + createdAt: 0, + settledAt: 1_000, + meta: { backend: "pi" }, + usage: {}, + transcript: [], + liveTools: [], + queued: [], + finalText: "x".repeat(40 * 1024), + turns: 1, + }, + ]); + + assert.equal(entryDetails?.artifactSaveFailed, true); + } finally { + if (previousAgentDir === undefined) delete process.env.PI_CODING_AGENT_DIR; + else process.env.PI_CODING_AGENT_DIR = previousAgentDir; + await rm(directory, { recursive: true, force: true }); + } +}); + test("automatic result delivery shrinks a batch against authoritative parent headroom", () => { const budgets: number[] = []; const pi = { @@ -265,6 +359,58 @@ test("the visible subagent result entry renders the completed report", () => { ); }); +test("the compact result renderer shows artifact save failures", () => { + const renderers = new Map(); + const pi = { + on() {}, + events: { on() {} }, + registerTool() {}, + getActiveTools: () => [], + setActiveTools() {}, + registerMessageRenderer() {}, + registerEntryRenderer(customType: string, renderer: EntryRenderer) { + renderers.set(customType, renderer); + }, + registerCommand() {}, + } as unknown as ExtensionAPI; + subagents(pi); + + const renderer = renderers.get("subagent-result"); + assert.ok(renderer); + const theme = { + fg: (_color: string, text: string) => text, + bg: (_color: string, text: string) => text, + bold: (text: string) => text, + italic: (text: string) => text, + underline: (text: string) => text, + strikethrough: (text: string) => text, + inverse: (text: string) => text, + } as unknown as Parameters[2]; + const component = renderer( + { + type: "custom", + id: "entry-2", + parentId: null, + timestamp: new Date().toISOString(), + customType: "subagent-result", + data: { + content: "Subagent sa-artifact finished.\n\nReport", + details: { + id: "sa-artifact", + title: "artifact", + status: "done", + artifactSaveFailed: true, + }, + }, + }, + { expanded: false }, + theme, + ); + + assert.ok(component); + assert.match(component.render(120).join("\n"), /artifact not saved/); +}); + test("session start preserves the complete registered subagent family", () => { let active = ["read", "third_party_tool"]; const registered: string[] = []; diff --git a/tests/extensions/subagents/result-artifact.test.ts b/tests/extensions/subagents/result-artifact.test.ts index 16714454..2749289c 100644 --- a/tests/extensions/subagents/result-artifact.test.ts +++ b/tests/extensions/subagents/result-artifact.test.ts @@ -123,6 +123,7 @@ test("artifact failure is explicit and never advertises a false path", () => { assert.equal(result.truncated, true); assert.equal(result.artifactPath, undefined); + assert.equal(result.artifactSaveFailed, true); assert.match(result.text, /could not be saved/); assert.doesNotMatch(result.text, /Full final answer:/); }); diff --git a/tests/extensions/subagents/result-rendering.test.ts b/tests/extensions/subagents/result-rendering.test.ts index 8b88adfd..aeaf38a5 100644 --- a/tests/extensions/subagents/result-rendering.test.ts +++ b/tests/extensions/subagents/result-rendering.test.ts @@ -5,13 +5,38 @@ import path from "node:path"; import test from "node:test"; import { initTheme, + type MessageRenderer, type EntryRenderer, type ExtensionAPI, } from "@earendil-works/pi-coding-agent"; +import { visibleWidth } from "@earendil-works/pi-tui"; +import { stripSubagentResultTransportInstruction } from "../../../extensions/subagents/src/prompt.ts"; initTheme("dark", false); -test("compact automatic results keep the report behind expansion", async () => { +const theme = { + fg: (_color: string, text: string) => text, + bg: (_color: string, text: string) => text, + bold: (text: string) => text, + italic: (text: string) => text, + underline: (text: string) => text, + strikethrough: (text: string) => text, + inverse: (text: string) => text, +} as unknown as Parameters[2]; + +test("legacy transport cleanup preserves identical text in the child answer", () => { + const instruction = + "(This result is already shown to the user. Act on it and relay only the decisions or next steps — do not repeat it verbatim.)"; + const childAnswer = `Subagent sa-1 "review" finished.\n\nThe child quoted this instruction:\n\n${instruction}\n\nThe actual conclusion follows.`; + const visible = stripSubagentResultTransportInstruction( + `${childAnswer}\n\n${instruction}`, + ); + + assert.ok(visible.includes(`quoted this instruction:\n\n${instruction}`)); + assert.ok(!visible.endsWith(`\n\n${instruction}`)); +}); + +test("automatic subagent results split model payload from bounded UI projection", async () => { const agentDir = await mkdtemp(path.join(tmpdir(), "openpi-result-render-")); const previousAgentDir = process.env.PI_CODING_AGENT_DIR; process.env.PI_CODING_AGENT_DIR = agentDir; @@ -24,32 +49,34 @@ test("compact automatic results keep the report behind expansion", async () => { const { default: subagents } = await import( "../../../extensions/subagents/index.ts" ); - const renderers = new Map(); + const entryRenderers = new Map(); + const messageRenderers = new Map(); const pi = { on() {}, events: { on() {} }, registerTool() {}, getActiveTools: () => [], setActiveTools() {}, - registerMessageRenderer() {}, + registerMessageRenderer(customType: string, renderer: MessageRenderer) { + messageRenderers.set(customType, renderer); + }, registerEntryRenderer(customType: string, renderer: EntryRenderer) { - renderers.set(customType, renderer); + entryRenderers.set(customType, renderer); }, registerCommand() {}, } as unknown as ExtensionAPI; subagents(pi); - const renderer = renderers.get("subagent-result"); - assert.ok(renderer); - const theme = { - fg: (_color: string, text: string) => text, - bg: (_color: string, text: string) => text, - bold: (text: string) => text, - italic: (text: string) => text, - underline: (text: string) => text, - strikethrough: (text: string) => text, - inverse: (text: string) => text, - } as unknown as Parameters[2]; + const entryRenderer = entryRenderers.get("subagent-result"); + const messageRenderer = messageRenderers.get("subagent-result"); + assert.ok(entryRenderer); + assert.ok(messageRenderer); + + const instruction = + "(This result is already shown to the user. Act on it and relay only the decisions or next steps — do not repeat it verbatim.)"; + const displayContent = + 'Subagent sa-3 "investigate plan mode" finished.\n\nPlan Mode investigation report'; + const modelContent = `${displayContent}\n\n${instruction}`; const entry = { type: "custom" as const, id: "entry-1", @@ -57,30 +84,216 @@ test("compact automatic results keep the report behind expansion", async () => { timestamp: new Date().toISOString(), customType: "subagent-result", data: { - content: - 'Subagent sa-3 "investigate plan mode" finished.\n\nPlan Mode investigation report', + content: displayContent, details: { id: "sa-3", title: "investigate plan mode", status: "done" as const, + elapsed: "2s", }, }, }; - const compact = renderer(entry, { expanded: false }, theme); + const compact = entryRenderer(entry, { expanded: false }, theme); assert.ok(compact); const compactText = compact.render(120).join("\n"); assert.match(compactText, /1 subagent settled/); - assert.match(compactText, /sa-3 · investigate plan mode · done/); + assert.match(compactText, /sa-3 · investigate plan mode · done · 2s/); assert.match(compactText, /Results passed to main agent/); assert.doesNotMatch(compactText, /Plan Mode investigation report/); + assert.doesNotMatch(compactText, /This result is already shown/); - const expanded = renderer(entry, { expanded: true }, theme); + const expanded = entryRenderer(entry, { expanded: true }, theme); assert.ok(expanded); + const expandedText = expanded.render(120).join("\n"); + assert.match(expandedText, /Plan Mode investigation report/); + assert.doesNotMatch(expandedText, /This result is already shown/); + + const message = messageRenderer( + { + role: "custom", + customType: "subagent-result", + content: modelContent, + display: false, + details: { ...entry.data.details, displayContent }, + timestamp: Date.now(), + }, + { expanded: true, outputPad: 0 }, + theme, + ); + assert.ok(message); + const messageText = message.render(120).join("\n"); + assert.match(messageText, /Plan Mode investigation report/); + assert.doesNotMatch(messageText, /This result is already shown/); + + const legacy = entryRenderer( + { + ...entry, + id: "entry-legacy", + data: { ...entry.data, content: modelContent }, + }, + { expanded: true }, + theme, + ); + assert.ok(legacy); + const legacyText = legacy.render(120).join("\n"); + assert.match(legacyText, /Plan Mode investigation report/); + assert.doesNotMatch(legacyText, /This result is already shown/); + + await writeFile( + path.join(agentDir, "my-pi-setup.json"), + JSON.stringify({ ui: { subagentResultDisplay: "full" } }), + ); + const fullByDefault = entryRenderer(entry, { expanded: false }, theme); + assert.ok(fullByDefault); assert.match( - expanded.render(120).join("\n"), + fullByDefault.render(120).join("\n"), /Plan Mode investigation report/, ); + await writeFile( + path.join(agentDir, "my-pi-setup.json"), + JSON.stringify({ ui: { subagentResultDisplay: "compact" } }), + ); + + const failureContent = + 'Subagent sa-4 "run tests" failed.\nError: child crashed\n\npartial output'; + const failure = entryRenderer( + { + ...entry, + id: "entry-2", + data: { + content: failureContent, + details: { + id: "sa-4", + title: "run tests", + status: "error" as const, + elapsed: "3s", + }, + }, + }, + { expanded: false }, + theme, + ); + assert.ok(failure); + const failureText = failure.render(120).join("\n"); + assert.match(failureText, /x .*sa-4 · run tests · error · 3s/); + assert.doesNotMatch(failureText, /partial output/); + const expandedFailure = entryRenderer( + { + ...entry, + id: "entry-3", + data: { + content: failureContent, + details: { + id: "sa-4", + title: "run tests", + status: "error" as const, + elapsed: "3s", + }, + }, + }, + { expanded: true }, + theme, + ); + assert.ok(expandedFailure); + assert.match(expandedFailure.render(120).join("\n"), /child crashed/); + + const batchDisplay = [ + 'Subagent sa-5 "review" finished.\n\nreview report', + 'Subagent sa-6 "tests" failed.\nError: failed\n\ntest failure', + ].join("\n\n"); + const batch = entryRenderer( + { + ...entry, + id: "entry-4", + data: { + content: batchDisplay, + details: { + count: 2, + results: [ + { + id: "sa-5", + title: "review", + status: "done" as const, + elapsed: "1s", + }, + { + id: "sa-6", + title: "tests", + status: "error" as const, + elapsed: "4s", + }, + ], + }, + }, + }, + { expanded: false }, + theme, + ); + assert.ok(batch); + const batchText = batch.render(120).join("\n"); + assert.match(batchText, /2 subagents settled · 1 failed/); + assert.match(batchText, /sa-5 · review · done · 1s/); + assert.match(batchText, /sa-6 · tests · error · 4s/); + assert.doesNotMatch(batchText, /review report|test failure/); + const expandedBatch = entryRenderer( + { + ...entry, + id: "entry-5", + data: { + content: batchDisplay, + details: { + count: 2, + results: [ + { + id: "sa-5", + title: "review", + status: "done" as const, + elapsed: "1s", + }, + { + id: "sa-6", + title: "tests", + status: "error" as const, + elapsed: "4s", + }, + ], + }, + }, + }, + { expanded: true }, + theme, + ); + assert.ok(expandedBatch); + const expandedBatchText = expandedBatch.render(120).join("\n"); + assert.match(expandedBatchText, /review report/); + assert.match(expandedBatchText, /test failure/); + assert.match(expandedBatchText, /2 subagents · 1 failed/); + assert.doesNotMatch(expandedBatchText, /This result is already shown/); + + const longTitle = `bad\n\x1b[31m${"title ".repeat(30)}`; + const narrow = entryRenderer( + { + ...entry, + id: "entry-6", + data: { + content: `Subagent sa-7 "${longTitle}" finished.\n\n${"long detail ".repeat(200)}`, + details: { + id: "sa-7\x1b[2J", + title: longTitle, + status: "done" as const, + elapsed: "5s", + }, + }, + }, + { expanded: false }, + theme, + ); + assert.ok(narrow); + const narrowLines = narrow.render(24); + assert.ok(narrowLines.every((line) => visibleWidth(line) <= 24)); + assert.doesNotMatch(narrowLines.join("\n"), /\u001b\[31m/); + assert.doesNotMatch(narrowLines.join("\n"), /long detail/); } finally { if (previousAgentDir === undefined) delete process.env.PI_CODING_AGENT_DIR; else process.env.PI_CODING_AGENT_DIR = previousAgentDir; diff --git a/tests/extensions/subagents/wait-result.test.ts b/tests/extensions/subagents/wait-result.test.ts index aa12aada..baba5c4b 100644 --- a/tests/extensions/subagents/wait-result.test.ts +++ b/tests/extensions/subagents/wait-result.test.ts @@ -20,8 +20,8 @@ test("wait result preview shows status only and keeps full output behind expand" content, { results: [ - { id: "sa-1", title: "review", status: "done" }, - { id: "sa-2", title: "tests", status: "error" }, + { id: "sa-1", title: "review", status: "done", elapsed: "2s" }, + { id: "sa-2", title: "tests", status: "error", elapsed: "5s" }, ], }, theme, @@ -30,8 +30,8 @@ test("wait result preview shows status only and keeps full output behind expand" assert.ok(lines.length <= 4); assert.match(preview, /2 subagents settled · 1 failed/); - assert.match(preview, /sa-1 · review · done/); - assert.match(preview, /sa-2 · tests · error/); + assert.match(preview, /sa-1 · review · done · 2s/); + assert.match(preview, /sa-2 · tests · error · 5s/); assert.match(preview, /Results passed to main agent/); assert.match(preview, /expand/); assert.doesNotMatch(preview, /finding 1/); @@ -54,3 +54,23 @@ test("wait result preview bounds status rows across large fan-out", () => { assert.match(preview, /… 4 more/); assert.doesNotMatch(preview, /sa-8/); }); + +test("wait result preview exposes artifact save failures", () => { + const preview = buildWaitResultPreview( + "## sa-1 finished\n\npartial result", + { + results: [ + { + id: "sa-1", + title: "review", + status: "done", + elapsed: "2s", + artifactSaveFailed: true, + }, + ], + }, + theme, + ); + + assert.match(preview, /artifact not saved/); +}); diff --git a/tests/extensions/suggestions/config.test.ts b/tests/extensions/suggestions/config.test.ts index 4d0601e4..0c8c06d4 100644 --- a/tests/extensions/suggestions/config.test.ts +++ b/tests/extensions/suggestions/config.test.ts @@ -18,7 +18,7 @@ const defaultUi = { customFooter: true, footerStyle: "plain" as const, footerLines: DEFAULT_FOOTER_LINES, - subagentResultDisplay: "full" as const, + subagentResultDisplay: "compact" as const, bashToolDisplay: "compact" as const, fileMutationDisplay: "compact" as const, }; @@ -27,7 +27,7 @@ test("setup defaults to disabled next-action suggestions", () => { assert.deepEqual(parseSetupConfig(undefined), DEFAULT_SETUP_CONFIG); assert.equal( formatSetupConfig(parseSetupConfig(undefined)), - `Capability discovery: explicit\nNext-action suggestions: disabled\nWorkflows: 8 concurrent agents · 128 total calls\nUI: large header off · custom footer on · plain · ${formatFooterLines(DEFAULT_FOOTER_LINES)}\nSubagent results: full by default\nBash operations: one-line activity summary (Ctrl+O restores native evidence)\nWrite/Edit operations: one-line activity summary (Ctrl+O restores native evidence)\nPost-edit command: off\nAgent role models (Subagents + Workflows): explorer inherit · implementer inherit · reviewer inherit · advisor inherit`, + `Capability discovery: explicit\nNext-action suggestions: disabled\nWorkflows: 8 concurrent agents · 128 total calls\nUI: large header off · custom footer on · plain · ${formatFooterLines(DEFAULT_FOOTER_LINES)}\nSubagent results: compact status summary (Ctrl+O expands full output)\nBash operations: one-line activity summary (Ctrl+O restores native evidence)\nWrite/Edit operations: one-line activity summary (Ctrl+O restores native evidence)\nPost-edit command: off\nAgent role models (Subagents + Workflows): explorer inherit · implementer inherit · reviewer inherit · advisor inherit`, ); }); @@ -59,7 +59,7 @@ test("setup config accepts suggestion models and migrates the recap key", () => }); assert.equal( formatSetupConfig(configured), - `Capability discovery: explicit\nNext-action suggestions: seal/deepseek-v4-flash · off · Right accepts\nWorkflows: 8 concurrent agents · 128 total calls\nUI: large header off · custom footer on · plain · ${formatFooterLines(DEFAULT_FOOTER_LINES)}\nSubagent results: full by default\nBash operations: one-line activity summary (Ctrl+O restores native evidence)\nWrite/Edit operations: one-line activity summary (Ctrl+O restores native evidence)\nPost-edit command: off\nAgent role models (Subagents + Workflows): explorer inherit · implementer inherit · reviewer inherit · advisor inherit`, + `Capability discovery: explicit\nNext-action suggestions: seal/deepseek-v4-flash · off · Right accepts\nWorkflows: 8 concurrent agents · 128 total calls\nUI: large header off · custom footer on · plain · ${formatFooterLines(DEFAULT_FOOTER_LINES)}\nSubagent results: compact status summary (Ctrl+O expands full output)\nBash operations: one-line activity summary (Ctrl+O restores native evidence)\nWrite/Edit operations: one-line activity summary (Ctrl+O restores native evidence)\nPost-edit command: off\nAgent role models (Subagents + Workflows): explorer inherit · implementer inherit · reviewer inherit · advisor inherit`, ); assert.deepEqual( @@ -130,7 +130,7 @@ test("UI defaults to a compact header and one-line plain footer", () => { customFooter: false, footerStyle: "plain", footerLines: DEFAULT_FOOTER_LINES, - subagentResultDisplay: "full", + subagentResultDisplay: "compact", bashToolDisplay: "compact", fileMutationDisplay: "compact", }, @@ -143,7 +143,7 @@ test("UI defaults to a compact header and one-line plain footer", () => { assert.equal( parseSetupConfig({ ui: { subagentResultDisplay: "unknown" } }).ui .subagentResultDisplay, - "full", + "compact", ); assert.equal( parseSetupConfig({ ui: { bashToolDisplay: "full" } }).ui.bashToolDisplay, From b77933a7a320ccf7d578ad594fd3de413fe8cea2 Mon Sep 17 00:00:00 2001 From: Adam Cheng <63501289+627150795@users.noreply.github.com> Date: Sat, 29 Aug 2026 22:17:12 +0800 Subject: [PATCH 2/4] test(subagents): update snapshots for transcript version --- tests/extensions/subagents/index.test.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/extensions/subagents/index.test.ts b/tests/extensions/subagents/index.test.ts index c12d0e12..ba75effb 100644 --- a/tests/extensions/subagents/index.test.ts +++ b/tests/extensions/subagents/index.test.ts @@ -154,6 +154,7 @@ test("automatic projection carries artifact save failures into result details", settledAt: 1_000, meta: { backend: "pi" }, usage: {}, + transcriptVersion: 0, transcript: [], liveTools: [], queued: [], @@ -197,6 +198,7 @@ test("automatic delivery reports real artifact save failures", async () => { settledAt: 1_000, meta: { backend: "pi" }, usage: {}, + transcriptVersion: 0, transcript: [], liveTools: [], queued: [], From 200fa4a5125272d367993cac4e98e059bd4149c5 Mon Sep 17 00:00:00 2001 From: Adam Cheng <63501289+627150795@users.noreply.github.com> Date: Sun, 30 Aug 2026 11:01:27 +0800 Subject: [PATCH 3/4] fix(subagents): surface canonical result exceptions Preserve separated display content, propagate interrupted and recovery metadata, and isolate compact renderer configuration in tests. --- extensions/subagents/index.ts | 35 ++++++++- extensions/subagents/src/domain.ts | 3 + extensions/subagents/src/manager.ts | 9 +++ extensions/subagents/src/ui/wait-result.ts | 73 +++++++++++++++---- tests/extensions/subagents/index.test.ts | 60 ++++++++++++++- tests/extensions/subagents/manager.test.ts | 13 +++- .../subagents/result-rendering.test.ts | 23 +++++- .../extensions/subagents/wait-result.test.ts | 70 +++++++++++++++++- 8 files changed, 265 insertions(+), 21 deletions(-) diff --git a/extensions/subagents/index.ts b/extensions/subagents/index.ts index 7288d43b..43bbaa0d 100644 --- a/extensions/subagents/index.ts +++ b/extensions/subagents/index.ts @@ -176,15 +176,21 @@ interface SubagentResultDetails { readonly id?: string; readonly title?: string; readonly status?: SubagentSnapshot["status"]; + readonly outcome?: SubagentSnapshot["outcome"]; + readonly worktreeBranch?: string; readonly elapsed?: string; readonly artifactSaveFailed?: boolean; + readonly fullResultSaved?: boolean; readonly count?: number; readonly results?: ReadonlyArray<{ readonly id: string; readonly title: string; readonly status: SubagentSnapshot["status"]; + readonly outcome?: SubagentSnapshot["outcome"]; + readonly worktreeBranch?: string; readonly elapsed?: string; readonly artifactSaveFailed?: boolean; + readonly fullResultSaved?: boolean; }>; /** Display-only projection for the custom message renderer. */ readonly displayContent?: string; @@ -241,7 +247,10 @@ function projectSubagentOutput( }); } -type OutputProjection = Pick; +type OutputProjection = Pick< + ResultProjection, + "text" | "artifactPath" | "artifactSaveFailed" +>; function normalizeProjection( output: string | OutputProjection, @@ -324,7 +333,12 @@ export function createSubagentResultDispatcher( id: snaps[0]!.id, title: snaps[0]!.title, status: snaps[0]!.status, + ...(snaps[0]!.outcome ? { outcome: snaps[0]!.outcome } : {}), + ...(snaps[0]!.worktreeBranch + ? { worktreeBranch: snaps[0]!.worktreeBranch } + : {}), elapsed: formatElapsed(snaps[0]!), + ...(projections[0]!.artifactPath ? { fullResultSaved: true } : {}), ...(projections[0]!.artifactSaveFailed ? { artifactSaveFailed: true } : {}), @@ -335,7 +349,14 @@ export function createSubagentResultDispatcher( id: snap.id, title: snap.title, status: snap.status, + ...(snap.outcome ? { outcome: snap.outcome } : {}), + ...(snap.worktreeBranch + ? { worktreeBranch: snap.worktreeBranch } + : {}), elapsed: formatElapsed(snap), + ...(projections[index]!.artifactPath + ? { fullResultSaved: true } + : {}), ...(projections[index]!.artifactSaveFailed ? { artifactSaveFailed: true } : {}), @@ -366,7 +387,7 @@ function renderSubagentResult( theme: SubagentResultTheme, ) { const displayContent = sanitizeText( - stripSubagentResultTransportInstruction(details.displayContent ?? content), + details.displayContent ?? stripSubagentResultTransportInstruction(content), ); const results = details.results?.length ? details.results @@ -376,8 +397,11 @@ function renderSubagentResult( id: details.id, title: details.title, status: details.status, + outcome: details.outcome, + worktreeBranch: details.worktreeBranch, elapsed: details.elapsed, artifactSaveFailed: details.artifactSaveFailed, + fullResultSaved: details.fullResultSaved, }, ] : []; @@ -1070,11 +1094,13 @@ export default function (pi: ExtensionAPI) { ); let resultIndex = 0; const artifactSaveFailures = new Set(); + const fullResultsSaved = new Set(); const sections = entries.map((entry) => { if ("section" in entry) return entry.section; const outputBudget = allocation.budgets[resultIndex++]!; const projection = projectSubagentOutput(entry.snap, outputBudget); if (projection.artifactSaveFailed) artifactSaveFailures.add(entry.id); + if (projection.artifactPath) fullResultsSaved.add(entry.id); return `${entry.header}\n\n${projection.text}`; }); @@ -1095,7 +1121,12 @@ export default function (pi: ExtensionAPI) { id, title: snap?.title, status: snap?.status, + ...(snap?.outcome ? { outcome: snap.outcome } : {}), + ...(snap?.worktreeBranch + ? { worktreeBranch: snap.worktreeBranch } + : {}), ...(snap ? { elapsed: formatElapsed(snap) } : {}), + ...(fullResultsSaved.has(id) ? { fullResultSaved: true } : {}), ...(artifactSaveFailures.has(id) ? { artifactSaveFailed: true } : {}), diff --git a/extensions/subagents/src/domain.ts b/extensions/subagents/src/domain.ts index 3c87a14d..503bd351 100644 --- a/extensions/subagents/src/domain.ts +++ b/extensions/subagents/src/domain.ts @@ -31,6 +31,7 @@ export const REASONING_EFFORTS = [ export type ReasoningEffort = (typeof REASONING_EFFORTS)[number]; export type SubagentStatus = "running" | "done" | "error"; +export type SubagentOutcome = "completed" | "failed" | "interrupted"; /** Parent-session context resolved by the tool layer and passed opaquely. */ export interface ParentContext { @@ -215,6 +216,8 @@ export interface SubagentSnapshot { readonly prompt: string; readonly cwd: string; readonly status: SubagentStatus; + readonly outcome?: SubagentOutcome; + readonly worktreeBranch?: string; readonly createdAt: number; readonly settledAt?: number; readonly errorText?: string; diff --git a/extensions/subagents/src/manager.ts b/extensions/subagents/src/manager.ts index 1a111497..64c6fccd 100644 --- a/extensions/subagents/src/manager.ts +++ b/extensions/subagents/src/manager.ts @@ -109,6 +109,8 @@ interface MutableSnapshot { prompt: string; cwd: string; status: SubagentStatus; + outcome?: SubagentSnapshot["outcome"]; + worktreeBranch?: string; createdAt: number; settledAt?: number; errorText?: string; @@ -345,11 +347,13 @@ const makeManager = (config: SubagentManagerConfig = {}) => switch (outcome._tag) { case "Completed": s.status = "done"; + s.outcome = "completed"; s.errorText = undefined; s.finalText = outcome.finalText.slice(0, FINAL_TEXT_MAX_LENGTH); break; case "Failed": s.status = "error"; + s.outcome = "failed"; s.errorText = bounded(outcome.errorText); // Never let a failed run report the previous run's successful output. s.finalText = (outcome.partialText ?? "").slice( @@ -359,6 +363,7 @@ const makeManager = (config: SubagentManagerConfig = {}) => break; case "Interrupted": s.status = "error"; + s.outcome = "interrupted"; s.errorText = "Run was aborted"; s.finalText = (outcome.partialText ?? "").slice( 0, @@ -425,6 +430,7 @@ const makeManager = (config: SubagentManagerConfig = {}) => case "RunStarted": entry.restarting = false; s.status = "running"; + s.outcome = undefined; s.settledAt = undefined; s.errorText = undefined; armWatchdog(entry); @@ -586,6 +592,9 @@ const makeManager = (config: SubagentManagerConfig = {}) => prompt: task.prompt, cwd: task.cwd, status: "running", + ...(task.worktree + ? { worktreeBranch: task.worktree.branch } + : {}), createdAt: Date.now(), meta, usage: { contextWindow: meta.contextWindow }, diff --git a/extensions/subagents/src/ui/wait-result.ts b/extensions/subagents/src/ui/wait-result.ts index 6f56542a..b6f1fe25 100644 --- a/extensions/subagents/src/ui/wait-result.ts +++ b/extensions/subagents/src/ui/wait-result.ts @@ -16,8 +16,11 @@ export interface WaitResultItem { readonly id: string; readonly title?: string; readonly status?: string; + readonly outcome?: "completed" | "failed" | "interrupted"; + readonly worktreeBranch?: string; readonly elapsed?: string; readonly artifactSaveFailed?: boolean; + readonly fullResultSaved?: boolean; } export interface WaitResultDetails { @@ -37,20 +40,49 @@ function fixedRows(rows: readonly string[]): Component { }; } +function selectStatusRows(results: readonly WaitResultItem[]) { + const visible = results.slice(0, MAX_STATUS_ROWS); + if (visible.some((result) => result.status === "error")) { + return visible; + } + + const hiddenFailure = results + .slice(MAX_STATUS_ROWS) + .find((result) => result.status === "error"); + return hiddenFailure ? [...visible.slice(0, -1), hiddenFailure] : visible; +} + export function buildWaitResultPreview( content: string, details: WaitResultDetails | undefined, theme: Theme, ) { const results = details?.results ?? []; - const failed = results.filter((result) => result.status === "error").length; + const uncertain = results.filter( + (result) => result.outcome === "interrupted", + ).length; + const failed = results.filter( + (result) => result.status === "error" && result.outcome !== "interrupted", + ).length; const artifactFailures = results.filter( (result) => result.artifactSaveFailed, ).length; + const exceptions = [ + failed > 0 ? theme.fg("error", `${failed} failed`) : "", + uncertain > 0 ? theme.fg("warning", `${uncertain} uncertain`) : "", + ].filter(Boolean); const header = - theme.fg(failed > 0 ? "warning" : "success", failed > 0 ? "!" : "✓") + - ` ${theme.fg("accent", theme.bold(`${results.length} subagent${results.length === 1 ? "" : "s"} settled`))}` + - (failed > 0 ? theme.fg("error", ` · ${failed} failed`) : "") + + theme.fg( + failed > 0 || uncertain > 0 ? "warning" : "success", + failed > 0 || uncertain > 0 ? "!" : "✓", + ) + + ` ${exceptions.length > 0 ? `${exceptions.join(" · ")} · ` : ""}` + + theme.fg( + "accent", + theme.bold( + `${results.length} subagent${results.length === 1 ? "" : "s"} settled`, + ), + ) + (artifactFailures > 0 ? theme.fg( "warning", @@ -58,26 +90,39 @@ export function buildWaitResultPreview( ) : ""); const lines = [header]; + const statusRows = selectStatusRows(results); - for (const result of results.slice(0, MAX_STATUS_ROWS)) { - const isFailure = result.status === "error"; + for (const result of statusRows) { + const isUncertain = result.outcome === "interrupted"; + const isFailure = result.status === "error" && !isUncertain; const icon = theme.fg( - isFailure ? "error" : "success", - isFailure ? "x" : "✓", + isFailure ? "error" : isUncertain ? "warning" : "success", + isFailure ? "x" : isUncertain ? "?" : "✓", ); const id = singleLine(result.id); const title = result.title ? singleLine(result.title) : ""; - const status = singleLine(result.status ?? "settled"); + const status = isUncertain + ? "uncertain" + : singleLine(result.status ?? "settled"); const elapsed = result.elapsed ? singleLine(result.elapsed) : ""; - const artifact = result.artifactSaveFailed - ? theme.fg("warning", " · artifact not saved") + const notices = [ + result.artifactSaveFailed ? "artifact not saved" : "", + result.worktreeBranch + ? `worktree handoff · ${singleLine(result.worktreeBranch)}` + : "", + result.fullResultSaved ? "full result saved" : "", + ].filter(Boolean); + const attention = notices.length + ? theme.fg("warning", `${notices.join(" · ")} · `) : ""; lines.push( - ` ${icon} ${theme.fg("accent", id)}${title ? theme.fg("muted", ` · ${title}`) : ""}${theme.fg("dim", ` · ${status}${elapsed ? ` · ${elapsed}` : ""}`)}${artifact}`, + ` ${icon} ${attention}${theme.fg("accent", id)}${title ? theme.fg("muted", ` · ${title}`) : ""}${theme.fg("dim", ` · ${status}${elapsed ? ` · ${elapsed}` : ""}`)}`, ); } - if (results.length > MAX_STATUS_ROWS) { - lines.push(theme.fg("dim", ` … ${results.length - MAX_STATUS_ROWS} more`)); + if (results.length > statusRows.length) { + lines.push( + theme.fg("dim", ` … ${results.length - statusRows.length} more`), + ); } if (sanitizeText(content).trim()) { diff --git a/tests/extensions/subagents/index.test.ts b/tests/extensions/subagents/index.test.ts index ba75effb..421d1e52 100644 --- a/tests/extensions/subagents/index.test.ts +++ b/tests/extensions/subagents/index.test.ts @@ -166,6 +166,52 @@ test("automatic projection carries artifact save failures into result details", assert.equal(entryDetails?.artifactSaveFailed, true); }); +test("automatic projection carries canonical outcome and recovery metadata", () => { + let entryDetails: Record | undefined; + const pi = { + appendEntry( + _customType: string, + data: { details: Record }, + ) { + entryDetails = data.details; + }, + sendMessage() {}, + } as unknown as ExtensionAPI; + const dispatch = createSubagentResultDispatcher(pi, () => ({ + text: "projected result", + truncated: true, + artifactPath: "/tmp/subagent-final.txt", + })); + + dispatch([ + { + id: "sa-recovery", + origin: "model", + backend: "pi", + title: "recovery test", + prompt: "inspect", + cwd: process.cwd(), + status: "error", + outcome: "interrupted", + worktreeBranch: "pi/impl-1", + createdAt: 0, + settledAt: 1_000, + meta: { backend: "pi" }, + usage: {}, + transcriptVersion: 0, + transcript: [], + liveTools: [], + queued: [], + finalText: "result", + turns: 1, + }, + ]); + + assert.equal(entryDetails?.outcome, "interrupted"); + assert.equal(entryDetails?.worktreeBranch, "pi/impl-1"); + assert.equal(entryDetails?.fullResultSaved, true); +}); + test("automatic delivery reports real artifact save failures", async () => { const directory = await mkdtemp(path.join(tmpdir(), "openpi-artifact-dir-")); const previousAgentDir = process.env.PI_CODING_AGENT_DIR; @@ -361,7 +407,19 @@ test("the visible subagent result entry renders the completed report", () => { ); }); -test("the compact result renderer shows artifact save failures", () => { +test("the compact result renderer shows artifact save failures", async (t) => { + const agentDir = await mkdtemp(path.join(tmpdir(), "openpi-compact-render-")); + const previousAgentDir = process.env.PI_CODING_AGENT_DIR; + process.env.PI_CODING_AGENT_DIR = agentDir; + t.after(async () => { + if (previousAgentDir === undefined) delete process.env.PI_CODING_AGENT_DIR; + else process.env.PI_CODING_AGENT_DIR = previousAgentDir; + await rm(agentDir, { recursive: true, force: true }); + }); + await writeFile( + path.join(agentDir, "my-pi-setup.json"), + JSON.stringify({ ui: { subagentResultDisplay: "compact" } }), + ); const renderers = new Map(); const pi = { on() {}, diff --git a/tests/extensions/subagents/manager.test.ts b/tests/extensions/subagents/manager.test.ts index 192141be..e0f7498e 100644 --- a/tests/extensions/subagents/manager.test.ts +++ b/tests/extensions/subagents/manager.test.ts @@ -185,6 +185,7 @@ test("stub subagent completes and delivers a final result", async () => { const done = manager.view.get(snap.id); assert.ok(done); assert.equal(done.status, "done"); + assert.equal(done.outcome, "completed"); assert.match( done.finalText, /\[stub:pi\] completed: Say hello to the tests/, @@ -216,6 +217,7 @@ test("FAIL: prompts settle as errors; unconsumed settles are delivered", async ( ); const failed = manager.view.get(snap.id); assert.equal(failed?.status, "error"); + assert.equal(failed?.outcome, "failed"); assert.match(failed?.errorText ?? "", /task failed/); assert.deepEqual(settled, [{ id: snap.id, consumed: false }]); }); @@ -225,13 +227,22 @@ test("cancel interrupts a running stub subagent", async () => { await withManager(async (manager, runtime) => { const snap = await runTool( runtime, - manager.spawn("pi", task("Long running task")), + manager.spawn("pi", { + ...task("Long running task"), + worktree: { + path: "/repo/.git/pi-worktrees/impl-1", + branch: "pi/impl-1", + repoCwd: "/repo", + }, + }), ); + assert.equal(snap.worktreeBranch, "pi/impl-1"); const report = await runTool(runtime, manager.cancel([snap.id])); assert.deepEqual(report, [ { id: snap.id, title: "test", status: "error", cancelled: true }, ]); assert.equal(manager.view.get(snap.id)?.errorText, "Run was aborted"); + assert.equal(manager.view.get(snap.id)?.outcome, "interrupted"); }); }); diff --git a/tests/extensions/subagents/result-rendering.test.ts b/tests/extensions/subagents/result-rendering.test.ts index aeaf38a5..7921782d 100644 --- a/tests/extensions/subagents/result-rendering.test.ts +++ b/tests/extensions/subagents/result-rendering.test.ts @@ -109,6 +109,27 @@ test("automatic subagent results split model payload from bounded UI projection" assert.match(expandedText, /Plan Mode investigation report/); assert.doesNotMatch(expandedText, /This result is already shown/); + const quotedInstruction = entryRenderer( + { + ...entry, + id: "entry-quoted-instruction", + data: { + content: `${displayContent}\n\n${instruction}`, + details: { + ...entry.data.details, + displayContent: `${displayContent}\n\n${instruction}`, + }, + }, + }, + { expanded: true }, + theme, + ); + assert.ok(quotedInstruction); + assert.match( + quotedInstruction.render(120).join("\n"), + /This result is already shown to the user/, + ); + const message = messageRenderer( { role: "custom", @@ -232,7 +253,7 @@ test("automatic subagent results split model payload from bounded UI projection" ); assert.ok(batch); const batchText = batch.render(120).join("\n"); - assert.match(batchText, /2 subagents settled · 1 failed/); + assert.match(batchText, /1 failed · 2 subagents settled/); assert.match(batchText, /sa-5 · review · done · 1s/); assert.match(batchText, /sa-6 · tests · error · 4s/); assert.doesNotMatch(batchText, /review report|test failure/); diff --git a/tests/extensions/subagents/wait-result.test.ts b/tests/extensions/subagents/wait-result.test.ts index baba5c4b..3d6ea232 100644 --- a/tests/extensions/subagents/wait-result.test.ts +++ b/tests/extensions/subagents/wait-result.test.ts @@ -1,7 +1,11 @@ import assert from "node:assert/strict"; import test from "node:test"; import { initTheme, type Theme } from "@earendil-works/pi-coding-agent"; -import { buildWaitResultPreview } from "../../../extensions/subagents/src/ui/wait-result.ts"; +import { visibleWidth } from "@earendil-works/pi-tui"; +import { + buildWaitResultPreview, + renderWaitResultPreview, +} from "../../../extensions/subagents/src/ui/wait-result.ts"; initTheme("dark", false); @@ -29,7 +33,7 @@ test("wait result preview shows status only and keeps full output behind expand" const lines = preview.split("\n"); assert.ok(lines.length <= 4); - assert.match(preview, /2 subagents settled · 1 failed/); + assert.match(preview, /1 failed · 2 subagents settled/); assert.match(preview, /sa-1 · review · done · 2s/); assert.match(preview, /sa-2 · tests · error · 5s/); assert.match(preview, /Results passed to main agent/); @@ -74,3 +78,65 @@ test("wait result preview exposes artifact save failures", () => { assert.match(preview, /artifact not saved/); }); + +test("narrow wait result preview keeps artifact warning understandable", () => { + const rendered = renderWaitResultPreview( + "partial result", + { + results: [ + { + id: "sa-1", + title: "review", + status: "done", + elapsed: "2s", + artifactSaveFailed: true, + }, + ], + }, + theme, + ).render(32); + + assert.ok(rendered.every((line) => visibleWidth(line) <= 32)); + assert.match(rendered.join("\n"), /artifact not saved/); +}); + +test("compact preview keeps a failed result visible beyond the status row limit", () => { + const rendered = renderWaitResultPreview( + "", + { + results: Array.from({ length: 6 }, (_, index) => ({ + id: `sa-${index + 1}`, + title: `task ${index + 1}`, + status: index === 4 ? "error" : "done", + })), + }, + theme, + ).render(120); + const preview = rendered.join("\n"); + + assert.match(preview, /1 failed · 6 subagents settled/); + assert.match(preview, /sa-5 · task 5 · error/); +}); + +test("compact preview surfaces uncertain worktree recovery state", () => { + const preview = buildWaitResultPreview( + "partial result", + { + results: [ + { + id: "sa-1", + title: "implementation", + status: "error", + outcome: "interrupted", + worktreeBranch: "pi/impl-1", + fullResultSaved: true, + }, + ], + }, + theme, + ); + + assert.match(preview, /uncertain/); + assert.match(preview, /worktree handoff · pi\/impl-1/); + assert.match(preview, /full result saved/); +}); From bdce2d7b17bced573978cf8c7a213d5f27c7368d Mon Sep 17 00:00:00 2001 From: tt-a1i Date: Sun, 30 Aug 2026 12:51:10 +0800 Subject: [PATCH 4/4] fix(subagents): enforce automatic result batch cap --- extensions/subagents/index.ts | 93 +++++++++----- tests/extensions/subagents/index.test.ts | 114 +++++++++++++++--- .../subagents/result-rendering.test.ts | 46 +++---- 3 files changed, 184 insertions(+), 69 deletions(-) diff --git a/extensions/subagents/index.ts b/extensions/subagents/index.ts index 43bbaa0d..d0324534 100644 --- a/extensions/subagents/index.ts +++ b/extensions/subagents/index.ts @@ -69,7 +69,7 @@ import { planModeAllowsDeclaredTools, planModeChildTools, } from "../shared/plan-mode-state.ts"; -import { loadSetupConfig } from "../shared/setup-config.ts"; +import { loadSetupConfig, type DetailDisplay } from "../shared/setup-config.ts"; import { OPENPI_TOOL_SURFACE, patchOwnedTools, @@ -156,6 +156,8 @@ const WAIT_PER_AGENT_MAX_BYTES = 16 * 1024; const WAIT_MIN_RESULT_BYTES = 512; const RESULT_HEADROOM_SHARE = 0.5; const ESTIMATED_BYTES_PER_TOKEN = 4; +const AUTOMATIC_BATCH_TRUNCATION_NOTICE = + "\n\n[Automatic subagent result batch truncated at the 48 KiB total limit.]"; interface SpawnResultDetails { readonly id?: string; @@ -258,6 +260,24 @@ function normalizeProjection( return typeof output === "string" ? { text: output } : output; } +function boundAutomaticResultBatch(content: string) { + const probe = truncateHead(content, { + maxBytes: AUTOMATIC_OUTPUT_MAX_BYTES, + maxLines: Number.MAX_SAFE_INTEGER, + }); + if (!probe.truncated) return content; + + const noticeBytes = Buffer.byteLength( + AUTOMATIC_BATCH_TRUNCATION_NOTICE, + "utf8", + ); + const bounded = truncateHead(content, { + maxBytes: Math.max(0, AUTOMATIC_OUTPUT_MAX_BYTES - noticeBytes), + maxLines: Number.MAX_SAFE_INTEGER, + }); + return `${bounded.content}${AUTOMATIC_BATCH_TRUNCATION_NOTICE}`; +} + export function createSubagentResultDispatcher( pi: ExtensionAPI, outputFor: ( @@ -284,7 +304,7 @@ export function createSubagentResultDispatcher( ) + Math.max(0, snaps.length - 1) * 2; const projectionBatchBytes = Math.max( - AUTOMATIC_MIN_RESULT_BYTES * snaps.length, + 0, AUTOMATIC_OUTPUT_MAX_BYTES - wrapperBytes, ); const allocation = allocateResultBudgets( @@ -305,28 +325,32 @@ export function createSubagentResultDispatcher( normalizeProjection(outputFor(snap, allocation.budgets[index]!)), ); const outputs = projections.map((projection) => projection.text); - const displayContent = snaps - .map((snap, index) => - buildSubagentResultDisplayMessage({ - id: snap.id, - title: snap.title, - status: snap.status, - errorText: snap.errorText, - output: outputs[index]!, - }), - ) - .join("\n\n"); - const content = snaps - .map((snap, index) => - buildSubagentResultMessage({ - id: snap.id, - title: snap.title, - status: snap.status, - errorText: snap.errorText, - output: outputs[index]!, - }), - ) - .join("\n\n"); + const displayContent = boundAutomaticResultBatch( + snaps + .map((snap, index) => + buildSubagentResultDisplayMessage({ + id: snap.id, + title: snap.title, + status: snap.status, + errorText: snap.errorText, + output: outputs[index]!, + }), + ) + .join("\n\n"), + ); + const content = boundAutomaticResultBatch( + snaps + .map((snap, index) => + buildSubagentResultMessage({ + id: snap.id, + title: snap.title, + status: snap.status, + errorText: snap.errorText, + output: outputs[index]!, + }), + ) + .join("\n\n"), + ); const details: SubagentResultDetails = snaps.length === 1 ? { @@ -384,6 +408,7 @@ function renderSubagentResult( content: string, details: SubagentResultDetails, expanded: boolean, + resultDisplay: DetailDisplay, theme: SubagentResultTheme, ) { const displayContent = sanitizeText( @@ -405,7 +430,7 @@ function renderSubagentResult( }, ] : []; - if (!expanded && loadSetupConfig().ui.subagentResultDisplay === "compact") { + if (!expanded && resultDisplay === "compact") { return renderWaitResultPreview(displayContent, { results }, theme); } @@ -443,7 +468,17 @@ function renderSubagentResult( }; } -export default function (pi: ExtensionAPI) { +interface SubagentExtensionOptions { + readonly getResultDisplay?: () => DetailDisplay; +} + +export default function ( + pi: ExtensionAPI, + options: SubagentExtensionOptions = {}, +) { + const getResultDisplay = + options.getResultDisplay ?? + (() => loadSetupConfig().ui.subagentResultDisplay); let runtime: SubagentRuntime | undefined; let managerPromise: Promise | undefined; let restoredIdCounters: SubagentIdCounters = { @@ -1155,7 +1190,7 @@ export default function (pi: ExtensionAPI) { return renderWaitResult( content, result.details as WaitResultDetails | undefined, - expanded || loadSetupConfig().ui.subagentResultDisplay === "full", + expanded || getResultDisplay() === "full", theme, ); }, @@ -1350,6 +1385,7 @@ export default function (pi: ExtensionAPI) { content, (message.details ?? {}) as SubagentResultDetails, expanded, + getResultDisplay(), theme, ); }, @@ -1362,6 +1398,7 @@ export default function (pi: ExtensionAPI) { entry.data?.content ?? "", entry.data?.details ?? {}, expanded, + getResultDisplay(), theme, ), ); @@ -1404,7 +1441,7 @@ export default function (pi: ExtensionAPI) { .filter(Boolean) .join("\n\n"); - if (expanded || loadSetupConfig().ui.subagentResultDisplay === "full") { + if (expanded || getResultDisplay() === "full") { const md = new Markdown(body, 0, 0, getMarkdownTheme()); const container = new Text(header, 0, 0); return { diff --git a/tests/extensions/subagents/index.test.ts b/tests/extensions/subagents/index.test.ts index 421d1e52..a183ec3a 100644 --- a/tests/extensions/subagents/index.test.ts +++ b/tests/extensions/subagents/index.test.ts @@ -351,6 +351,102 @@ test("automatic result wrappers and projections stay inside the shared batch cap } }); +test("automatic delivery keeps 64 results inside the hard batch cap", () => { + let displayContent = ""; + let modelContent = ""; + const pi = { + appendEntry(_customType: string, data: { content: string }) { + displayContent = data.content; + }, + sendMessage(message: { content: string }) { + modelContent = message.content; + }, + } as unknown as ExtensionAPI; + const dispatch = createSubagentResultDispatcher( + pi, + (snap, maxBytes) => + projectResult(snap.finalText, { + maxBytes, + maxLines: 600, + writeArtifact: () => `/tmp/${snap.id}.txt`, + }).text, + () => ({ tokens: 100_000, contextWindow: 100_000 }), + ); + const snapshots = Array.from({ length: 64 }, (_, index) => { + const id = `sa-${index + 1}`; + return { + id, + origin: "model" as const, + backend: "pi" as const, + title: `long report ${id}`, + prompt: "inspect", + cwd: process.cwd(), + status: "done" as const, + createdAt: 0, + settledAt: 1_000, + meta: { backend: "pi" as const }, + usage: {}, + transcriptVersion: 0, + transcript: [], + liveTools: [], + queued: [], + finalText: `BEGIN-${id}\n${"evidence\n".repeat(10_000)}END-${id}`, + turns: 1, + }; + }); + + dispatch(snapshots); + + assert.ok(Buffer.byteLength(displayContent, "utf8") <= 48 * 1024); + assert.ok(Buffer.byteLength(modelContent, "utf8") <= 48 * 1024); + for (const { id } of snapshots) { + assert.match(displayContent, new RegExp(`Subagent ${id} `)); + assert.match(modelContent, new RegExp(`Subagent ${id} `)); + } +}); + +test("automatic delivery fails closed when wrapper metadata exceeds the cap", () => { + let displayContent = ""; + let modelContent = ""; + const pi = { + appendEntry(_customType: string, data: { content: string }) { + displayContent = data.content; + }, + sendMessage(message: { content: string }) { + modelContent = message.content; + }, + } as unknown as ExtensionAPI; + const dispatch = createSubagentResultDispatcher(pi, () => "report"); + + dispatch([ + { + id: "sa-oversized", + origin: "model", + backend: "pi", + title: "title ".repeat(20_000), + prompt: "inspect", + cwd: process.cwd(), + status: "error", + errorText: "failure ".repeat(20_000), + createdAt: 0, + settledAt: 1_000, + meta: { backend: "pi" }, + usage: {}, + transcriptVersion: 0, + transcript: [], + liveTools: [], + queued: [], + finalText: "report", + turns: 1, + }, + ]); + + for (const content of [displayContent, modelContent]) { + assert.ok(Buffer.byteLength(content, "utf8") <= 48 * 1024); + assert.match(content, /truncated at the 48 KiB total limit/); + } +}); + test("the visible subagent result entry renders the completed report", () => { const renderers = new Map(); const pi = { @@ -365,7 +461,7 @@ test("the visible subagent result entry renders the completed report", () => { }, registerCommand() {}, } as unknown as ExtensionAPI; - subagents(pi); + subagents(pi, { getResultDisplay: () => "full" }); const renderer = renderers.get("subagent-result"); assert.ok(renderer); @@ -407,19 +503,7 @@ test("the visible subagent result entry renders the completed report", () => { ); }); -test("the compact result renderer shows artifact save failures", async (t) => { - const agentDir = await mkdtemp(path.join(tmpdir(), "openpi-compact-render-")); - const previousAgentDir = process.env.PI_CODING_AGENT_DIR; - process.env.PI_CODING_AGENT_DIR = agentDir; - t.after(async () => { - if (previousAgentDir === undefined) delete process.env.PI_CODING_AGENT_DIR; - else process.env.PI_CODING_AGENT_DIR = previousAgentDir; - await rm(agentDir, { recursive: true, force: true }); - }); - await writeFile( - path.join(agentDir, "my-pi-setup.json"), - JSON.stringify({ ui: { subagentResultDisplay: "compact" } }), - ); +test("the compact result renderer shows artifact save failures", () => { const renderers = new Map(); const pi = { on() {}, @@ -433,7 +517,7 @@ test("the compact result renderer shows artifact save failures", async (t) => { }, registerCommand() {}, } as unknown as ExtensionAPI; - subagents(pi); + subagents(pi, { getResultDisplay: () => "compact" }); const renderer = renderers.get("subagent-result"); assert.ok(renderer); diff --git a/tests/extensions/subagents/result-rendering.test.ts b/tests/extensions/subagents/result-rendering.test.ts index 7921782d..c352f598 100644 --- a/tests/extensions/subagents/result-rendering.test.ts +++ b/tests/extensions/subagents/result-rendering.test.ts @@ -1,7 +1,4 @@ import assert from "node:assert/strict"; -import { mkdtemp, rm, writeFile } from "node:fs/promises"; -import { tmpdir } from "node:os"; -import path from "node:path"; import test from "node:test"; import { initTheme, @@ -24,6 +21,21 @@ const theme = { inverse: (text: string) => text, } as unknown as Parameters[2]; +async function withResultDisplay( + run: ( + getResultDisplay: () => "compact" | "full", + setResultDisplay: (display: "compact" | "full") => void, + ) => Promise, +) { + let display = "compact" as "compact" | "full"; + await run( + () => display, + (next) => { + display = next; + }, + ); +} + test("legacy transport cleanup preserves identical text in the child answer", () => { const instruction = "(This result is already shown to the user. Act on it and relay only the decisions or next steps — do not repeat it verbatim.)"; @@ -37,15 +49,7 @@ test("legacy transport cleanup preserves identical text in the child answer", () }); test("automatic subagent results split model payload from bounded UI projection", async () => { - const agentDir = await mkdtemp(path.join(tmpdir(), "openpi-result-render-")); - const previousAgentDir = process.env.PI_CODING_AGENT_DIR; - process.env.PI_CODING_AGENT_DIR = agentDir; - - try { - await writeFile( - path.join(agentDir, "my-pi-setup.json"), - JSON.stringify({ ui: { subagentResultDisplay: "compact" } }), - ); + await withResultDisplay(async (getResultDisplay, setResultDisplay) => { const { default: subagents } = await import( "../../../extensions/subagents/index.ts" ); @@ -65,7 +69,7 @@ test("automatic subagent results split model payload from bounded UI projection" }, registerCommand() {}, } as unknown as ExtensionAPI; - subagents(pi); + subagents(pi, { getResultDisplay }); const entryRenderer = entryRenderers.get("subagent-result"); const messageRenderer = messageRenderers.get("subagent-result"); @@ -161,20 +165,14 @@ test("automatic subagent results split model payload from bounded UI projection" assert.match(legacyText, /Plan Mode investigation report/); assert.doesNotMatch(legacyText, /This result is already shown/); - await writeFile( - path.join(agentDir, "my-pi-setup.json"), - JSON.stringify({ ui: { subagentResultDisplay: "full" } }), - ); + setResultDisplay("full"); const fullByDefault = entryRenderer(entry, { expanded: false }, theme); assert.ok(fullByDefault); assert.match( fullByDefault.render(120).join("\n"), /Plan Mode investigation report/, ); - await writeFile( - path.join(agentDir, "my-pi-setup.json"), - JSON.stringify({ ui: { subagentResultDisplay: "compact" } }), - ); + setResultDisplay("compact"); const failureContent = 'Subagent sa-4 "run tests" failed.\nError: child crashed\n\npartial output'; @@ -315,9 +313,5 @@ test("automatic subagent results split model payload from bounded UI projection" assert.ok(narrowLines.every((line) => visibleWidth(line) <= 24)); assert.doesNotMatch(narrowLines.join("\n"), /\u001b\[31m/); assert.doesNotMatch(narrowLines.join("\n"), /long detail/); - } finally { - if (previousAgentDir === undefined) delete process.env.PI_CODING_AGENT_DIR; - else process.env.PI_CODING_AGENT_DIR = previousAgentDir; - await rm(agentDir, { recursive: true, force: true }); - } + }); });