From e770cdc4d9c28a56480f8e4fa5d62d4b33acf148 Mon Sep 17 00:00:00 2001 From: can1357 Date: Mon, 13 Jul 2026 05:16:36 +0200 Subject: [PATCH] fix(coding-agent): unified tool renderer and clarified launch diagnostics - Implemented a consolidated TUI renderer for launch tool operations to unify status headers and metadata. - Added tracking for unfulfilled readiness conditions to distinguish between timeout causes and daemon states. - Enhanced timeout reporting to explicitly surface specific unmet log or port conditions. - Included comprehensive unit and regression tests for tool rendering and start-timeout diagnostics. --- packages/coding-agent/CHANGELOG.md | 2 + .../src/cli/gallery-fixtures/shell.ts | 74 +++- packages/coding-agent/src/launch/broker.ts | 18 + packages/coding-agent/src/launch/protocol.ts | 13 + .../coding-agent/src/modes/theme/theme.ts | 4 + packages/coding-agent/src/tools/launch.ts | 330 +++++++++++++++--- packages/coding-agent/src/tools/renderers.ts | 2 + .../test/tools/launch-renderer.test.ts | 159 +++++++++ .../coding-agent/test/tools/launch.test.ts | 43 +++ 9 files changed, 601 insertions(+), 44 deletions(-) create mode 100644 packages/coding-agent/test/tools/launch-renderer.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 536cac0bd..d0eedf4e0 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -30,6 +30,8 @@ - Improved auto-compaction to automatically drop images and elide content when context is tight, and added persistent warning badges to the compaction divider when manual intervention is required - Fixed backgrounded Bash blocks continuing to repaint with live and final job output; they now freeze with a compact job notice while completion is delivered separately - Fixed `--reasoning-slide-plan` silently ending the run with no code written when the model answered the plan nudge with a text-only reply (no tool call): the agent loop treats a tool-call-free turn as a natural stop and never prompts again, which the nudge's own "write the plan in your next reply" instruction makes common. The nudge now explicitly tells the model this is a checkpoint, not a final answer, and the session forces one more turn whenever a post-nudge reply lands with zero tool calls +- Fixed launch tool rendering stacking a stale pending header over a bare `✓ Launch` line and raw text: the tool now uses a merged registry renderer with one per-op status header (op, target, `state · pid · uptime` meta), stripped log cursor suffixes, capped collapsed log/list previews, and a launch tool glyph +- Fixed confusing launch start/wait results when readiness timed out with the log pattern already matched (readiness needs log AND port): the result printed a contradictory `Ready: ` next to `Readiness timed out` without naming the failing condition. Daemon snapshots now carry the unmet conditions (`readyPending`), and start/wait results state exactly what never happened (e.g. `port 3100 on 127.0.0.1 never accepted connections`); the TUI shows a `waiting on port` badge on starting daemons - Fixed the in-process `stat` builtin mangling BSD-style invocations like `stat -f "%Sm %N" file` (macOS muscle memory): GNU `-f` means `--file-system`, so the format string was treated as a file operand — printing filesystem info for the real operands and erroring with `cannot read file system information for '%Sm %N'`. A `-f` whose format value contains `%` is now detected as BSD syntax and translated to the GNU equivalent (`%Sm`→`%y`, `%N`→`%n`, `%z`→`%s`, epoch/`S`-form times, owner/group/permission and `H`/`L` sub-field directives, `-L`/`-n`/`-q`/`-F` flag clusters, with `%n`/`%t` as literal newline/tab); directives with no GNU counterpart fail with a clear `unsupported BSD format directive` error - Fixed the remaining GNU-flavored shell builtins that broke under macOS/BSD muscle memory, using the same unambiguous-detection approach as the `stat` fix (only invocations that are invalid or nonsensical under GNU semantics are reinterpreted; unsupported BSD forms fail loudly instead of producing wrong output): `date -r ` formats the epoch when no such file exists (GNU `-r FILE` mtime preserved), signed `date -v±N` adjustments translate to `-d` relative dates and `-j` is accepted (`-j -f` strptime parse mode and field-set `-v` error clearly); `sed -i '' 's/…/…/' file` drops the BSD empty backup-suffix token instead of treating it as the script; `mktemp -t prefix` without X's creates `$TMPDIR/prefix.XXXXXXXXXX` (the GNU `too few X's` error path); `tail -r` reverses input by delegating to `tac` (with `-n`/`-c`/`-f` combinations erroring clearly); `find -E` maps to `-regextype posix-extended` ahead of the expression; `base64 -D` decodes as an alias of `-d`; and `ln -sfh` works via a `-h` alias of `--no-dereference` (clap's `-h` help short is dropped to match real GNU/BSD ln; `--help` unchanged) diff --git a/packages/coding-agent/src/cli/gallery-fixtures/shell.ts b/packages/coding-agent/src/cli/gallery-fixtures/shell.ts index 2dc810bef..d734a7da3 100644 --- a/packages/coding-agent/src/cli/gallery-fixtures/shell.ts +++ b/packages/coding-agent/src/cli/gallery-fixtures/shell.ts @@ -1,4 +1,4 @@ -/** Gallery fixtures for the shell tools (bash, eval). */ +/** Gallery fixtures for the shell tools (bash, eval, launch). */ import type { GalleryFixture } from "./types"; export const shellFixtures: Record = { @@ -56,6 +56,78 @@ export const shellFixtures: Record = { }, }, + launch: { + label: "Launch", + streamingArgs: { op: "start", name: "web" }, + args: { + op: "start", + name: "web", + application: "bun", + args: ["run", "dev"], + ready: { log: "Local:.*http", port: 5173, timeout: 30 }, + }, + result: { + content: [ + { + type: "text", + text: "Started web: ready pid=51234 uptime=1.2s restarts=0\nReady: Local: http://localhost:5173", + }, + ], + details: { + op: "start", + daemon: { + name: "web", + id: "d-1", + state: "ready", + pid: 51234, + createdAt: 0, + startedAt: Date.now() - 1_200, + readyAt: Date.now(), + restartCount: 0, + outputBytes: 2048, + readyMatch: "Local: http://localhost:5173/", + persist: false, + detached: false, + }, + timedOut: false, + }, + }, + errorResult: { + content: [{ type: "text", text: "start requires application" }], + isError: true, + details: { op: "start" }, + }, + }, + + launch_logs: { + label: "Launch", + renderer: "launch", + args: { op: "logs", name: "web", lines: 100, follow: true, cursor: 1842, timeout: 30 }, + result: { + content: [ + { + type: "text", + text: [ + "$ bun run dev", + " VITE v6.0.3 ready in 312 ms", + "", + " ➜ Local: http://localhost:5173/", + " ➜ Network: use --host to expose", + "12:04:11 [vite] hmr update /src/App.tsx", + "12:04:15 [vite] hmr update /src/components/Chart.tsx", + "[web: running; cursor=2210]", + ].join("\n"), + }, + ], + details: { op: "logs", cursor: 2210, timedOut: false, state: "running" }, + }, + errorResult: { + content: [{ type: "text", text: "No daemon named web" }], + isError: true, + details: { op: "logs" }, + }, + }, + eval: { label: "Eval", streamingArgs: { diff --git a/packages/coding-agent/src/launch/broker.ts b/packages/coding-agent/src/launch/broker.ts index 549710aa9..c2bc6ef5c 100644 --- a/packages/coding-agent/src/launch/broker.ts +++ b/packages/coding-agent/src/launch/broker.ts @@ -85,6 +85,18 @@ function terminalState(state: DaemonSnapshot["state"]): boolean { return state === "exited" || state === "failed"; } +/** Mirror per-condition readiness progress into the snapshot so clients can see which condition is unmet. */ +function syncReadyPending(record: ManagedDaemon): void { + if (record.snapshot.state !== "starting") { + record.snapshot.readyPending = undefined; + return; + } + const pending: ("log" | "port")[] = []; + if (!record.logReady) pending.push("log"); + if (!record.portReady) pending.push("port"); + record.snapshot.readyPending = pending.length > 0 ? pending : undefined; +} + async function fileTextSlice(filePath: string, head: boolean): Promise { try { const stat = await fs.stat(filePath); @@ -455,6 +467,7 @@ class DaemonBroker { consecutiveFailures: 0, persistQueue: Promise.resolve(), }; + syncReadyPending(record); this.#records.set(spec.name, record); await this.#launch(record); let readyTimedOut = false; @@ -480,6 +493,7 @@ class DaemonBroker { record.snapshot.readyMatch = undefined; record.logReady = !record.spec.ready?.log; record.portReady = record.spec.ready?.port === undefined; + syncReadyPending(record); record.readinessBuffer = ""; record.outputOffset = 0; this.#persist(record); @@ -645,6 +659,7 @@ class DaemonBroker { if (match) { record.logReady = true; record.snapshot.readyMatch = match[0].slice(0, 500); + syncReadyPending(record); } } this.#markReady(record); @@ -667,6 +682,7 @@ class DaemonBroker { while (generation === record.generation && !terminalState(record.snapshot.state)) { if (await connectPort(host, port)) { record.portReady = true; + syncReadyPending(record); this.#markReady(record); return; } @@ -696,6 +712,7 @@ class DaemonBroker { record.snapshot.exitedAt = Date.now(); record.snapshot.exitCode = exitCode; record.snapshot.exitReason = error; + record.snapshot.readyPending = undefined; const failed = error !== undefined || (exitCode !== undefined && exitCode !== 0); const shouldRestart = !record.stopRequested && @@ -920,6 +937,7 @@ class DaemonBroker { consecutiveFailures: 0, persistQueue: Promise.resolve(), }; + syncReadyPending(record); this.#records.set(snapshot.name, record); if (detached && spec.ready?.port !== undefined && snapshot.state !== "ready") { void this.#pollPort(record, record.generation, spec.ready); diff --git a/packages/coding-agent/src/launch/protocol.ts b/packages/coding-agent/src/launch/protocol.ts index bb61cb7ee..97bf0631c 100644 --- a/packages/coding-agent/src/launch/protocol.ts +++ b/packages/coding-agent/src/launch/protocol.ts @@ -57,6 +57,8 @@ export interface DaemonSnapshot { outputBytes: number; owner?: string; readyMatch?: string; + /** Readiness conditions still unmet while `state` is `starting`; absent once ready or without a ready spec. */ + readyPending?: ("log" | "port")[]; persist: boolean; detached: boolean; } @@ -188,6 +190,16 @@ function daemonSignal(value: unknown): DaemonSignal { throw new Error(`Unknown daemon signal: ${signal}`); } +function readyPendingList(value: unknown): ("log" | "port")[] { + if (!Array.isArray(value)) throw new Error("daemon.readyPending must be an array"); + const result: ("log" | "port")[] = []; + for (const item of value) { + if (item !== "log" && item !== "port") throw new Error(`Unknown readiness condition: ${String(item)}`); + result.push(item); + } + return result; +} + function readySpec(value: unknown): DaemonReadySpec { const source = record(value, "ready"); const log = optionalString(source.log, "ready.log"); @@ -234,6 +246,7 @@ export function parseDaemonSnapshot(value: unknown): DaemonSnapshot { outputBytes: numberValue(source.outputBytes, "daemon.outputBytes"), owner: optionalString(source.owner, "daemon.owner"), readyMatch: optionalString(source.readyMatch, "daemon.readyMatch"), + readyPending: source.readyPending === undefined ? undefined : readyPendingList(source.readyPending), persist: booleanValue(source.persist, "daemon.persist"), detached: source.detached === undefined ? false : booleanValue(source.detached, "daemon.detached"), }; diff --git a/packages/coding-agent/src/modes/theme/theme.ts b/packages/coding-agent/src/modes/theme/theme.ts index 9d4151529..e6ae57aa4 100644 --- a/packages/coding-agent/src/modes/theme/theme.ts +++ b/packages/coding-agent/src/modes/theme/theme.ts @@ -225,6 +225,7 @@ export type SymbolKey = | "tool.debug" | "tool.mcp" | "tool.job" + | "tool.launch" | "tool.task" | "tool.todo" | "tool.memory" @@ -433,6 +434,7 @@ const UNICODE_SYMBOLS: SymbolMap = { "tool.debug": "🐞", "tool.mcp": "🔌", "tool.job": "⚙", + "tool.launch": "🚀", "tool.task": "⇶", "tool.todo": "☑", "tool.memory": "🧠", @@ -742,6 +744,7 @@ const NERD_SYMBOLS: SymbolMap = { "tool.debug": "\uEAD8", "tool.mcp": "\uEB2D", "tool.job": "\uEBA2", + "tool.launch": "\uF135", "tool.task": "\uf4a0", "tool.todo": "\uEAB3", "tool.memory": "\uEACE", @@ -946,6 +949,7 @@ const ASCII_SYMBOLS: SymbolMap = { "tool.debug": "dbg", "tool.mcp": "<>", "tool.job": "job", + "tool.launch": "run", "tool.task": ">>>", "tool.todo": "[x]", "tool.memory": "mem", diff --git a/packages/coding-agent/src/tools/launch.ts b/packages/coding-agent/src/tools/launch.ts index 2491f8bf6..879c1555e 100644 --- a/packages/coding-agent/src/tools/launch.ts +++ b/packages/coding-agent/src/tools/launch.ts @@ -12,13 +12,26 @@ import { prompt } from "@oh-my-pi/pi-utils"; import { type } from "arktype"; import type { RenderResultOptions } from "../extensibility/custom-tools/types"; import { daemonClientForProject } from "../launch/client"; -import type { DaemonOperation, DaemonRpcResult, DaemonSnapshot, DaemonSpec } from "../launch/protocol"; -import type { Theme } from "../modes/theme/theme"; +import type { DaemonOperation, DaemonRpcResult, DaemonSnapshot, DaemonSpec, DaemonState } from "../launch/protocol"; +import type { Theme, ThemeColor } from "../modes/theme/theme"; import launchDescription from "../prompts/tools/launch.md" with { type: "text" }; import { renderStatusLine } from "../tui"; import type { ToolSession } from "."; import { resolveToCwd } from "./path-utils"; -import { formatDuration, replaceTabs, shortenPath, TRUNCATE_LENGTHS, truncateToWidth } from "./render-utils"; +import { + capPreviewLines, + createCachedComponent, + formatDuration, + formatExpandHint, + formatMoreItems, + PREVIEW_LIMITS, + pluralize, + previewLine, + replaceTabs, + shortenPath, + TRUNCATE_LENGTHS, + truncateToWidth, +} from "./render-utils"; import { ToolError } from "./tool-errors"; const launchSchema = type({ @@ -77,6 +90,12 @@ export interface LaunchToolDetails { daemons?: DaemonSnapshot[]; cursor?: number; timedOut?: boolean; + /** logs: daemon lifecycle state at read time. */ + state?: DaemonState; + /** wait: output line that satisfied the pattern. */ + matched?: string; + /** describe: immutable launch spec backing the command/cwd detail lines. */ + spec?: DaemonSpec; } function requiredName(params: LaunchParams): string { @@ -180,18 +199,44 @@ function daemonLabel(daemon: DaemonSnapshot): string { )} restarts=${daemon.restartCount}${daemon.detached ? " detached" : daemon.persist ? " persistent" : ""}`; } -function toolContent(result: DaemonRpcResult): string { +/** + * Human sentences for the readiness conditions still unmet, e.g. + * `port 5173 on 127.0.0.1 never accepted connections`. `ready` (from the start + * params) adds the concrete pattern/port; absent it falls back to generic labels. + */ +function readyPendingSummary(daemon: DaemonSnapshot, ready?: LaunchParams["ready"]): string[] { + const parts: string[] = []; + for (const condition of daemon.readyPending ?? []) { + if (condition === "log") { + parts.push(ready?.log ? `log pattern /${ready.log}/ never matched` : "the log pattern never matched"); + } else { + parts.push( + ready?.port !== undefined + ? `port ${ready.port} on ${ready.host ?? "127.0.0.1"} never accepted connections` + : "the port never accepted connections", + ); + } + } + return parts; +} + +function toolContent(result: DaemonRpcResult, params: LaunchParams): string { switch (result.op) { case "ping": case "shutdown": throw new ToolError(`Internal daemon result ${result.op} is not tool-visible`); case "start": { - const lines = [ - `${result.daemon.state === "failed" ? "Failed to launch" : "Started"} ${daemonLabel(result.daemon)}`, - ]; - if (result.daemon.readyMatch) lines.push(`Ready: ${result.daemon.readyMatch}`); - if (result.readyTimedOut) - lines.push("Readiness timed out; the daemon remains running. Inspect logs or stop it."); + const daemon = result.daemon; + const lines = [`${daemon.state === "failed" ? "Failed to launch" : "Started"} ${daemonLabel(daemon)}`]; + if (daemon.state === "failed" && daemon.exitReason) lines.push(`Reason: ${daemon.exitReason}`); + if (daemon.readyMatch) lines.push(`Ready log matched: ${daemon.readyMatch}`); + if (result.readyTimedOut) { + const pending = readyPendingSummary(daemon, params.ready); + const cause = pending.length > 0 ? `: ${pending.join("; ")}` : ""; + lines.push( + `NOT ready — readiness timed out after ${params.ready?.timeout ?? 30}s${cause}. The process is still running (state: ${daemon.state}); follow its logs or stop it.`, + ); + } return lines.join("\n"); } case "list": @@ -200,8 +245,15 @@ function toolContent(result: DaemonRpcResult): string { : "No daemons."; case "logs": return `${result.text}${result.text && !result.text.endsWith("\n") ? "\n" : ""}[${result.name}: ${result.state}; cursor=${result.cursor}${result.timedOut ? "; follow timed out" : ""}]`; - case "wait": - return `${daemonLabel(result.daemon)}${result.matched ? `\nMatched: ${result.matched}` : ""}${result.timedOut ? "\nWait timed out." : ""}`; + case "wait": { + const lines = [daemonLabel(result.daemon)]; + if (result.matched) lines.push(`Matched: ${result.matched}`); + if (result.timedOut) { + const pending = readyPendingSummary(result.daemon); + lines.push(`Wait timed out${pending.length > 0 ? ` (still waiting on: ${pending.join("; ")})` : ""}.`); + } + return lines.join("\n"); + } case "send": return `Sent input to ${daemonLabel(result.daemon)}`; case "stop": @@ -225,9 +277,9 @@ function toolDetails(result: DaemonRpcResult): LaunchToolDetails { case "list": return { op: "list", daemons: result.daemons }; case "logs": - return { op: "logs", cursor: result.cursor, timedOut: result.timedOut }; + return { op: "logs", cursor: result.cursor, timedOut: result.timedOut, state: result.state }; case "wait": - return { op: "wait", daemon: result.daemon, timedOut: result.timedOut }; + return { op: "wait", daemon: result.daemon, timedOut: result.timedOut, matched: result.matched }; case "send": return { op: "send", daemon: result.daemon }; case "stop": @@ -235,7 +287,7 @@ function toolDetails(result: DaemonRpcResult): LaunchToolDetails { case "restart": return { op: "restart", daemon: result.daemon }; case "describe": - return { op: "describe", daemon: result.daemon }; + return { op: "describe", daemon: result.daemon, spec: result.spec }; case "ping": case "shutdown": throw new ToolError(`Internal daemon result ${result.op} is not tool-visible`); @@ -319,38 +371,230 @@ export class LaunchTool implements AgentTool; - renderResult( - result: AgentToolResult, - _options: RenderResultOptions, - theme: Theme, - ): Component { - const raw = result.content.find(item => item.type === "text")?.text ?? ""; - const text = replaceTabs(raw) - .split("\n") - .map(line => truncateToWidth(line, TRUNCATE_LENGTHS.CONTENT)) - .join("\n"); - const status = renderStatusLine({ icon: result.isError ? "error" : "success", title: "Launch" }, theme); - return new Text(`${status}${text ? `\n${text}` : ""}`, 0, 0); +function stateColor(state: DaemonState): ThemeColor { + switch (state) { + case "running": + case "ready": + return "success"; + case "failed": + return "error"; + case "exited": + return "muted"; + default: + return "warning"; } } + +/** Compact `state · pid · uptime` fragments for the status-line meta slot. */ +function daemonMeta(daemon: DaemonSnapshot, theme: Theme): string[] { + const meta = [theme.fg(stateColor(daemon.state), daemon.state)]; + if (daemon.readyPending?.length) meta.push(theme.fg("warning", `waiting on ${daemon.readyPending.join("+")}`)); + if (daemon.exitCode !== undefined) { + meta.push(theme.fg(daemon.exitCode === 0 ? "muted" : "error", `exit ${daemon.exitCode}`)); + } else if (daemon.pid !== undefined) { + meta.push(`pid ${daemon.pid}`); + } + const lifespan = formatDuration((daemon.exitedAt ?? Date.now()) - daemon.startedAt); + meta.push(daemon.exitedAt === undefined ? `up ${lifespan}` : `ran ${lifespan}`); + if (daemon.restartCount > 0) meta.push(`restarts ${daemon.restartCount}`); + if (daemon.detached) meta.push("detached"); + else if (daemon.persist) meta.push("persistent"); + return meta; +} + +/** Op-specific call context (command line, log filters, wait condition, send payload). */ +function callMeta(args: LaunchRenderArgs): string[] { + const meta: string[] = []; + switch (args.op) { + case "start": + if (args.application) meta.push([args.application, ...(args.args ?? [])].join(" ")); + break; + case "logs": + if (args.follow) meta.push("follow"); + if (args.grep) meta.push(`grep /${args.grep}/`); + break; + case "wait": + meta.push(args.pattern ? `for /${args.pattern}/` : `for ${args.for ?? "exit"}`); + break; + case "send": + if (args.signal) meta.push(args.signal); + else if (args.text) meta.push(args.text); + if (args.keys?.length) meta.push(args.keys.join(" ")); + break; + } + return meta.map(entry => previewLine(replaceTabs(entry), TRUNCATE_LENGTHS.SHORT)); +} + +/** TUI renderer: one status header per op, meta from structured details, capped body lines. */ +export const launchToolRenderer = { + inline: true, + mergeCallAndResult: true, + animatedPendingPreview: true, + + renderCall(args: LaunchRenderArgs, options: RenderResultOptions, theme: Theme): Component { + const target = args.name ?? args.application; + const header = renderStatusLine( + { + icon: options.spinnerFrame !== undefined ? "running" : "pending", + spinnerFrame: options.spinnerFrame, + title: `Launch ${args.op ?? "…"}`, + description: target ? replaceTabs(target) : undefined, + meta: callMeta(args), + }, + theme, + ); + return new Text(header, 0, 0); + }, + + renderResult( + result: { content: Array<{ type: string; text?: string }>; details?: LaunchToolDetails; isError?: boolean }, + options: RenderResultOptions, + theme: Theme, + args?: LaunchRenderArgs, + ): Component { + const details = result.details; + const params = args ?? {}; + const op = details?.op ?? params.op; + const isError = result.isError === true; + const daemon = details?.daemon; + const failed = isError || daemon?.state === "failed"; + const text = + result.content + ?.filter(item => item.type === "text") + .map(item => item.text ?? "") + .join("\n") ?? ""; + + const meta: string[] = []; + const body: string[] = []; + let description = params.name ?? daemon?.name; + + if (isError) { + for (const line of replaceTabs(text.trimEnd()).split("\n")) body.push(theme.fg("error", line)); + } else { + switch (op) { + case "start": { + meta.push(...callMeta(params)); + if (daemon) meta.push(...daemonMeta(daemon, theme)); + if (daemon?.readyMatch) body.push(theme.fg("dim", `log matched: ${replaceTabs(daemon.readyMatch)}`)); + if (daemon?.state === "failed" && daemon.exitReason) + body.push(theme.fg("error", replaceTabs(daemon.exitReason))); + if (details?.timedOut) { + const pending = daemon ? readyPendingSummary(daemon, params.ready) : []; + body.push( + theme.fg( + "warning", + pending.length > 0 + ? `Not ready — ${pending.join("; ")}. Still running.` + : "Readiness timed out; the process is still running.", + ), + ); + } + break; + } + case "send": + meta.push(...callMeta(params)); + if (daemon) meta.push(...daemonMeta(daemon, theme)); + break; + case "stop": + case "restart": + if (daemon) meta.push(...daemonMeta(daemon, theme)); + break; + case "wait": { + meta.push(...callMeta(params)); + if (daemon) meta.push(...daemonMeta(daemon, theme)); + if (details?.matched) body.push(theme.fg("dim", `matched: ${replaceTabs(details.matched)}`)); + if (details?.timedOut) { + const pending = daemon ? readyPendingSummary(daemon) : []; + body.push( + theme.fg( + "warning", + pending.length > 0 + ? `Wait timed out — still waiting on ${pending.join("; ")}.` + : "Wait timed out.", + ), + ); + } + break; + } + case "list": { + const daemons = details?.daemons ?? []; + description = `${daemons.length || "no"} ${pluralize("process", daemons.length)}`; + for (const item of daemons) { + body.push( + `${theme.fg("accent", replaceTabs(item.name))} ${theme.fg("dim", daemonMeta(item, theme).join(theme.sep.dot))}`, + ); + } + break; + } + case "logs": { + if (details?.state) meta.push(theme.fg(stateColor(details.state), details.state)); + if (details?.cursor !== undefined) meta.push(`cursor ${details.cursor}`); + if (details?.timedOut) meta.push(theme.fg("warning", "follow timed out")); + // Strip the trailing `[name: state; cursor=N]` status suffix `toolContent` appends. + const logText = text.replace(/\n?\[[^\n]*\]$/, "").trimEnd(); + if (logText) { + for (const line of logText.split("\n")) body.push(theme.fg("toolOutput", replaceTabs(line))); + } + break; + } + case "describe": { + if (daemon) meta.push(...daemonMeta(daemon, theme)); + const spec = details?.spec; + if (spec) { + body.push(theme.fg("toolOutput", replaceTabs([spec.application, ...spec.args].join(" ")))); + body.push(theme.fg("dim", `cwd ${shortenPath(spec.cwd)}`)); + const flags = [`pty ${spec.pty}`, `restart ${spec.restart}`]; + if (spec.detached) flags.push("detached"); + else if (spec.persist) flags.push("persistent"); + body.push(theme.fg("dim", flags.join(theme.sep.dot))); + } + break; + } + default: + if (text.trim()) { + for (const line of replaceTabs(text.trimEnd()).split("\n")) body.push(theme.fg("toolOutput", line)); + } + } + } + + const header = renderStatusLine( + { + ...(failed + ? { icon: "error" as const } + : options.isPartial + ? { icon: "pending" as const } + : { iconOverride: theme.styledSymbol("tool.launch", "accent") }), + title: `Launch ${op ?? ""}`.trimEnd(), + description: description ? replaceTabs(description) : undefined, + meta, + }, + theme, + ); + + return createCachedComponent( + () => options.expanded, + (width, expanded) => { + let visible = body; + if (op === "logs") { + visible = capPreviewLines(body, theme, { expanded }); + } else if (!expanded && op === "list" && body.length > PREVIEW_LIMITS.COLLAPSED_ITEMS) { + const remaining = body.length - PREVIEW_LIMITS.COLLAPSED_ITEMS; + visible = [ + ...body.slice(0, PREVIEW_LIMITS.COLLAPSED_ITEMS), + theme.fg("dim", `${formatMoreItems(remaining, "process")} ${formatExpandHint(theme, false, true)}`), + ]; + } + return [header, ...visible].map(line => truncateToWidth(line, width)); + }, + ); + }, +}; diff --git a/packages/coding-agent/src/tools/renderers.ts b/packages/coding-agent/src/tools/renderers.ts index d76bd1e9c..5e8850b42 100644 --- a/packages/coding-agent/src/tools/renderers.ts +++ b/packages/coding-agent/src/tools/renderers.ts @@ -24,6 +24,7 @@ import { grepToolRenderer } from "./grep"; import { inspectImageToolRenderer } from "./inspect-image-renderer"; import { ircToolRenderer } from "./irc"; import { jobToolRenderer } from "./job"; +import { launchToolRenderer } from "./launch"; import { recallToolRenderer, reflectToolRenderer, retainToolRenderer } from "./memory-render"; import { readToolRenderer } from "./read"; import { resolveToolRenderer } from "./resolve"; @@ -93,6 +94,7 @@ export const toolRenderers: Record = { lsp: lspToolRenderer as ToolRenderer, inspect_image: inspectImageToolRenderer as ToolRenderer, irc: ircToolRenderer as ToolRenderer, + launch: launchToolRenderer as ToolRenderer, read: readToolRenderer as ToolRenderer, job: jobToolRenderer as ToolRenderer, resolve: resolveToolRenderer as ToolRenderer, diff --git a/packages/coding-agent/test/tools/launch-renderer.test.ts b/packages/coding-agent/test/tools/launch-renderer.test.ts new file mode 100644 index 000000000..e9c2723b3 --- /dev/null +++ b/packages/coding-agent/test/tools/launch-renderer.test.ts @@ -0,0 +1,159 @@ +/** + * Launch renderer contract: one merged status header per op carrying the op, + * target name, and daemon state — replacing the old stacked "pending header + + * bare `✓ Launch` + raw text" render — plus per-op body rules (logs strip the + * LLM-facing `[name: state; cursor=N]` suffix, list caps collapsed rows). + */ +import { describe, expect, it } from "bun:test"; +import type { DaemonSnapshot } from "@oh-my-pi/pi-coding-agent/launch/protocol"; +import { getThemeByName } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; +import { type LaunchToolDetails, launchToolRenderer } from "@oh-my-pi/pi-coding-agent/tools/launch"; +import { toolRenderers } from "@oh-my-pi/pi-coding-agent/tools/renderers"; +import { sanitizeText } from "@oh-my-pi/pi-utils"; + +async function theme() { + const t = await getThemeByName("dark"); + expect(t).toBeDefined(); + return t!; +} + +const lines = (component: { render: (w: number) => readonly string[] }, width = 200) => + sanitizeText(component.render(width).join("\n")).split("\n"); + +const daemon = (overrides: Partial): DaemonSnapshot => ({ + name: "web", + id: "d-1", + state: "running", + pid: 51234, + createdAt: 0, + startedAt: Date.now() - 22_600, + restartCount: 0, + outputBytes: 0, + persist: false, + detached: false, + ...overrides, +}); + +describe("launchToolRenderer", () => { + it("is registered with merged call/result so the pending header is replaced, not stacked", () => { + expect(Object.is(toolRenderers.launch.renderResult, launchToolRenderer.renderResult)).toBe(true); + expect(toolRenderers.launch.mergeCallAndResult).toBe(true); + }); + + it("folds a stop result into one header with op, name, and exit state", async () => { + const uiTheme = await theme(); + const rendered = lines( + launchToolRenderer.renderResult( + { + content: [{ type: "text", text: "Stopped pyc-profile-run: exited exit=0 uptime=22.6s restarts=0" }], + details: { + op: "stop", + daemon: daemon({ name: "pyc-profile-run", state: "exited", exitedAt: Date.now(), exitCode: 0 }), + } satisfies LaunchToolDetails, + }, + { expanded: false, isPartial: false }, + uiTheme, + { op: "stop", name: "pyc-profile-run" }, + ), + ); + expect(rendered).toHaveLength(1); + expect(rendered[0]).toContain("Launch stop"); + expect(rendered[0]).toContain("pyc-profile-run"); + expect(rendered[0]).toContain("exited"); + expect(rendered[0]).toContain("exit 0"); + }); + + it("renders log lines without the trailing cursor-status suffix, surfacing it as header meta", async () => { + const uiTheme = await theme(); + const rendered = lines( + launchToolRenderer.renderResult( + { + content: [{ type: "text", text: "line one\nline two\n[web: running; cursor=2210]" }], + details: { op: "logs", cursor: 2210, timedOut: false, state: "running" } satisfies LaunchToolDetails, + }, + { expanded: false, isPartial: false }, + uiTheme, + { op: "logs", name: "web" }, + ), + ); + expect(rendered[0]).toContain("Launch logs"); + expect(rendered[0]).toContain("cursor 2210"); + expect(rendered).toContain("line one"); + expect(rendered).toContain("line two"); + expect(rendered.some(line => line.includes("[web: running"))).toBe(false); + }); + + it("caps a collapsed list to the preview item limit with a more-items row", async () => { + const uiTheme = await theme(); + const daemons = Array.from({ length: 11 }, (_, i) => daemon({ name: `svc-${i}`, id: `d-${i}` })); + const rendered = lines( + launchToolRenderer.renderResult( + { + content: [{ type: "text", text: "" }], + details: { op: "list", daemons } satisfies LaunchToolDetails, + }, + { expanded: false, isPartial: false }, + uiTheme, + { op: "list" }, + ), + ); + expect(rendered[0]).toContain("11 processes"); + expect(rendered.some(line => line.includes("svc-0"))).toBe(true); + expect(rendered.some(line => line.includes("svc-10"))).toBe(false); + expect(rendered.some(line => line.includes("3 more processes"))).toBe(true); + }); + + it("marks a failed start with the daemon's exit reason even though the result is not an error", async () => { + const uiTheme = await theme(); + const rendered = lines( + launchToolRenderer.renderResult( + { + content: [{ type: "text", text: "Failed to launch web: failed exit=127" }], + details: { + op: "start", + daemon: daemon({ + state: "failed", + exitedAt: Date.now(), + exitCode: 127, + exitReason: "spawn bun ENOENT", + }), + } satisfies LaunchToolDetails, + }, + { expanded: false, isPartial: false }, + uiTheme, + { op: "start", name: "web", application: "bun", args: ["run", "dev"] }, + ), + ); + expect(rendered[0]).toContain("Launch start"); + expect(rendered[0]).toContain("failed"); + expect(rendered.some(line => line.includes("spawn bun ENOENT"))).toBe(true); + }); + + it("names the unmet readiness condition instead of a contradictory Ready + timed-out pair", async () => { + const uiTheme = await theme(); + const rendered = lines( + launchToolRenderer.renderResult( + { + content: [{ type: "text", text: "" }], + details: { + op: "start", + timedOut: true, + daemon: daemon({ + state: "starting", + readyMatch: "Local: http://localhost:3100", + readyPending: ["port"], + }), + } satisfies LaunchToolDetails, + }, + { expanded: false, isPartial: false }, + uiTheme, + { op: "start", name: "web", application: "bunx", args: ["vite"], ready: { log: "Local:", port: 3100 } }, + ), + ); + expect(rendered[0]).toContain("waiting on port"); + expect(rendered.some(line => line.includes("log matched: Local: http://localhost:3100"))).toBe(true); + expect(rendered.some(line => line.includes("port 3100 on 127.0.0.1 never accepted connections"))).toBe(true); + // The old render labeled the log match a bare "ready:" while also saying readiness timed out. + expect(rendered.some(line => line.includes("ready: Local:"))).toBe(false); + }); +}); diff --git a/packages/coding-agent/test/tools/launch.test.ts b/packages/coding-agent/test/tools/launch.test.ts index 76dc68c2b..eb5c95f3e 100644 --- a/packages/coding-agent/test/tools/launch.test.ts +++ b/packages/coding-agent/test/tools/launch.test.ts @@ -252,4 +252,47 @@ setInterval(() => {}, 1000); } } }, 20_000); + + // Regression: a start whose log pattern matched but whose port never accepted + // used to report "Ready: " AND "Readiness timed out" with no hint of + // which condition failed. The snapshot now names the unmet condition(s). + it("names the unmet readiness condition when start times out", async () => { + const projectDir = await tempDir("omp-daemon-ready-project-"); + const runtimeDir = await tempDir("omp-daemon-ready-runtime-"); + const scriptPath = path.join(projectDir, "service.ts"); + await Bun.write(scriptPath, `process.stdout.write("LISTENING\\n"); setInterval(() => {}, 1000);\n`); + // Reserve an ephemeral port and release it so nothing accepts connections there. + const probe = Bun.listen({ hostname: "127.0.0.1", port: 0, socket: { data() {} } }); + const deadPort = probe.port; + probe.stop(true); + const client = await createDaemonBrokerClient(projectDir, { runtimeDir, idleGraceMs: 5_000 }); + try { + const spec: DaemonSpec = { + name: "never-ready", + application: process.execPath, + args: [scriptPath], + env: {}, + cwd: projectDir, + pty: false, + ready: { log: "LISTENING", port: deadPort, timeoutMs: 3_000 }, + restart: "no", + persist: false, + detached: false, + }; + const started = await client.request({ op: "start", spec }); + expect(started.op).toBe("start"); + if (started.op !== "start") throw new Error("unexpected start result"); + expect(started.readyTimedOut).toBeTrue(); + expect(started.daemon.state).toBe("starting"); + expect(started.daemon.readyMatch).toBe("LISTENING"); + expect(started.daemon.readyPending).toEqual(["port"]); + + const stopped = await client.request({ op: "stop", name: "never-ready", timeoutMs: 2_000 }); + if (stopped.op !== "stop") throw new Error("unexpected stop result"); + // Terminal states carry no stale readiness noise. + expect(stopped.daemon.readyPending).toBeUndefined(); + } finally { + await shutdown(client); + } + }, 20_000); });