diff --git a/packages/coding-agent/src/prompts/tools/bash.md b/packages/coding-agent/src/prompts/tools/bash.md index 54114deed..686959cc9 100644 --- a/packages/coding-agent/src/prompts/tools/bash.md +++ b/packages/coding-agent/src/prompts/tools/bash.md @@ -24,10 +24,10 @@ Executes bash command in shell session for terminal operations like git, bun, ca {{/if}} {{#if asyncEnabled}} - Use `read jobs://` to inspect all background jobs and `read jobs://` for detailed status/output when needed. -- When you need to wait for async results before continuing, call `await` — it blocks until jobs complete. Do NOT poll `read jobs://` in a loop or yield and hope for delivery. +- When you need to wait for async results before continuing, call `poll` — it blocks until jobs complete. Do NOT poll `read jobs://` in a loop or yield and hope for delivery. {{else}} {{#if autoBackgroundEnabled}} -- If a command auto-backgrounds, use `read jobs://` to inspect jobs and `await` when you need to wait for completion instead of polling in a loop. +- If a command auto-backgrounds, use `read jobs://` to inspect jobs and `poll` when you need to wait for completion instead of polling in a loop. {{/if}} {{/if}} diff --git a/packages/coding-agent/src/prompts/tools/await.md b/packages/coding-agent/src/prompts/tools/poll.md similarity index 50% rename from packages/coding-agent/src/prompts/tools/await.md rename to packages/coding-agent/src/prompts/tools/poll.md index c9c66e771..19e588a91 100644 --- a/packages/coding-agent/src/prompts/tools/await.md +++ b/packages/coding-agent/src/prompts/tools/poll.md @@ -1,5 +1,5 @@ Blocks until one or more background jobs complete, fail, or are cancelled. -You **MUST** use this instead of polling `read jobs://` in a loop when you need to wait for background task or bash results before continuing. +You **MUST** use the `poll` tool instead of polling `read jobs://` in a loop when you need to wait for background task or bash results before continuing. Returns the status and results of all watched jobs once at least one finishes. diff --git a/packages/coding-agent/src/prompts/tools/task.md b/packages/coding-agent/src/prompts/tools/task.md index 8f7cc0bd4..7208b1eca 100644 --- a/packages/coding-agent/src/prompts/tools/task.md +++ b/packages/coding-agent/src/prompts/tools/task.md @@ -2,7 +2,7 @@ Launches subagents to parallelize workflows. {{#if asyncEnabled}} - Use `read jobs://` to inspect state; `read jobs://` for detail. -- Use the `await` tool to wait until completion. You **MUST NOT** poll `read jobs://` in a loop. +- Use the `poll` tool to wait until completion. You **MUST NOT** poll `read jobs://` in a loop. {{/if}} Subagents lack your conversation history. Every decision, file content, and user requirement they need **MUST** be explicit in `context` or `assignment`. diff --git a/packages/coding-agent/src/tools/bash.ts b/packages/coding-agent/src/tools/bash.ts index a07b375b9..54fee3794 100644 --- a/packages/coding-agent/src/tools/bash.ts +++ b/packages/coding-agent/src/tools/bash.ts @@ -73,6 +73,7 @@ export interface BashToolInput { export interface BashToolDetails { meta?: OutputMeta; + timeoutSeconds?: number; async?: { state: "running" | "completed" | "failed"; jobId: string; @@ -290,14 +291,20 @@ export class BashTool implements AgentTool { tailLines?: number, ): AgentToolResult { const outputText = this.#formatResultOutput(result, headLines, tailLines); - const details: BashToolDetails = {}; + const details: BashToolDetails = { timeoutSeconds: timeoutSec }; const resultBuilder = toolResult(details).text(outputText).truncationFromSummary(result, { direction: "tail" }); this.#buildResultText(result, timeoutSec, outputText); return resultBuilder.done(); } - #buildBackgroundStartResult(jobId: string, label: string, previewText: string): AgentToolResult { + #buildBackgroundStartResult( + jobId: string, + label: string, + previewText: string, + timeoutSec: number, + ): AgentToolResult { const details: BashToolDetails = { + timeoutSeconds: timeoutSec, async: { state: "running", jobId, type: "bash" }, }; const lines: string[] = []; @@ -307,7 +314,7 @@ export class BashTool implements AgentTool { } lines.push(`Background job ${jobId} started: ${label}`); lines.push("Result will be delivered automatically when complete."); - lines.push(`Use \`await\`, \`read jobs://${jobId}\`, or \`cancel_job\` if needed.`); + lines.push(`Use \`poll\`, \`read jobs://${jobId}\`, or \`cancel_job\` if needed.`); return { content: [{ type: "text", text: lines.join("\n") }], details, @@ -430,6 +437,12 @@ export class BashTool implements AgentTool { } } + #resolveAutoBackgroundWaitMs(timeoutMs: number): number { + if (this.#autoBackgroundThresholdMs <= 0) return 0; + const timeoutBufferMs = 1_000; + return Math.max(0, Math.min(this.#autoBackgroundThresholdMs, timeoutMs - timeoutBufferMs)); + } + async execute( _toolCallId: string, { @@ -536,10 +549,12 @@ export class BashTool implements AgentTool { onUpdate, startBackgrounded: true, }); - return this.#buildBackgroundStartResult(job.jobId, job.label, ""); + return this.#buildBackgroundStartResult(job.jobId, job.label, "", timeoutSec); } if (this.#autoBackgroundEnabled && !pty && this.session.asyncJobManager) { + const autoBackgroundWaitMs = this.#resolveAutoBackgroundWaitMs(timeoutMs); + const startBackgrounded = autoBackgroundWaitMs === 0; const job = this.#startManagedBashJob({ command, commandCwd, @@ -549,9 +564,12 @@ export class BashTool implements AgentTool { tailLines, resolvedEnv, onUpdate, - startBackgrounded: false, + startBackgrounded, }); - const waitResult = await this.#waitForManagedBashJob(job, this.#autoBackgroundThresholdMs, signal); + if (startBackgrounded) { + return this.#buildBackgroundStartResult(job.jobId, job.label, "", timeoutSec); + } + const waitResult = await this.#waitForManagedBashJob(job, autoBackgroundWaitMs, signal); if (waitResult.kind === "completed") { this.session.asyncJobManager.acknowledgeDeliveries([job.jobId]); return waitResult.result; @@ -566,7 +584,7 @@ export class BashTool implements AgentTool { throw new ToolAbortError(job.getLatestText() || "Command aborted"); } job.setBackgrounded(true); - return this.#buildBackgroundStartResult(job.jobId, job.label, job.getLatestText()); + return this.#buildBackgroundStartResult(job.jobId, job.label, job.getLatestText(), timeoutSec); } // Track output for streaming updates (tail only) @@ -689,7 +707,7 @@ export const bashToolRenderer = { const showingFullOutput = expanded && renderContext?.isFullOutput === true; // Build truncation warning - const timeoutSeconds = renderContext?.timeout; + const timeoutSeconds = details?.timeoutSeconds ?? renderContext?.timeout; const timeoutLine = typeof timeoutSeconds === "number" ? uiTheme.fg( diff --git a/packages/coding-agent/src/tools/index.ts b/packages/coding-agent/src/tools/index.ts index 5b12ba089..736fb43e8 100644 --- a/packages/coding-agent/src/tools/index.ts +++ b/packages/coding-agent/src/tools/index.ts @@ -23,7 +23,7 @@ import { SearchTool } from "../web/search"; import { AskTool } from "./ask"; import { AstEditTool } from "./ast-edit"; import { AstGrepTool } from "./ast-grep"; -import { AwaitTool } from "./await-tool"; +import { PollTool } from "./poll-tool"; import { BashTool } from "./bash"; import { BrowserTool } from "./browser"; @@ -73,7 +73,7 @@ export * from "../web/search"; export * from "./ask"; export * from "./ast-edit"; export * from "./ast-grep"; -export * from "./await-tool"; +export * from "./poll-tool"; export * from "./bash"; export * from "./browser"; export * from "./calculator"; @@ -243,7 +243,7 @@ export const BUILTIN_TOOLS: Record = { rewind: RewindTool.createIf, task: TaskTool.create, cancel_job: CancelJobTool.createIf, - await: AwaitTool.createIf, + poll: PollTool.createIf, todo_write: s => new TodoWriteTool(s), web_search: s => new SearchTool(s), search_tool_bm25: SearchToolBm25Tool.createIf, diff --git a/packages/coding-agent/src/tools/await-tool.ts b/packages/coding-agent/src/tools/poll-tool.ts similarity index 83% rename from packages/coding-agent/src/tools/await-tool.ts rename to packages/coding-agent/src/tools/poll-tool.ts index 580ec7f20..290b4a21b 100644 --- a/packages/coding-agent/src/tools/await-tool.ts +++ b/packages/coding-agent/src/tools/poll-tool.ts @@ -2,10 +2,10 @@ import type { AgentTool, AgentToolContext, AgentToolResult, AgentToolUpdateCallb import { prompt } from "@oh-my-pi/pi-utils"; import { type Static, Type } from "@sinclair/typebox"; import { isBackgroundJobSupportEnabled } from "../async"; -import awaitDescription from "../prompts/tools/await.md" with { type: "text" }; +import pollDescription from "../prompts/tools/poll.md" with { type: "text" }; import type { ToolSession } from "./index"; -const awaitSchema = Type.Object({ +const pollSchema = Type.Object({ jobs: Type.Optional( Type.Array(Type.String(), { description: "Specific job IDs to wait for. If omitted, waits for any running job.", @@ -13,9 +13,9 @@ const awaitSchema = Type.Object({ ), }); -type AwaitParams = Static; +type PollParams = Static; -interface AwaitResult { +interface PollResult { id: string; type: "bash" | "task"; status: "running" | "completed" | "failed" | "cancelled"; @@ -25,33 +25,33 @@ interface AwaitResult { errorText?: string; } -export interface AwaitToolDetails { - jobs: AwaitResult[]; +export interface PollToolDetails { + jobs: PollResult[]; } -export class AwaitTool implements AgentTool { - readonly name = "await"; - readonly label = "Await"; +export class PollTool implements AgentTool { + readonly name = "poll"; + readonly label = "Poll"; readonly description: string; - readonly parameters = awaitSchema; + readonly parameters = pollSchema; readonly strict = true; constructor(private readonly session: ToolSession) { - this.description = prompt.render(awaitDescription); + this.description = prompt.render(pollDescription); } - static createIf(session: ToolSession): AwaitTool | null { + static createIf(session: ToolSession): PollTool | null { if (!isBackgroundJobSupportEnabled(session.settings)) return null; - return new AwaitTool(session); + return new PollTool(session); } async execute( _toolCallId: string, - params: AwaitParams, + params: PollParams, signal?: AbortSignal, - _onUpdate?: AgentToolUpdateCallback, + _onUpdate?: AgentToolUpdateCallback, _context?: AgentToolContext, - ): Promise> { + ): Promise> { const manager = this.session.asyncJobManager; if (!manager) { return { @@ -124,12 +124,12 @@ export class AwaitTool implements AgentTool { + ): AgentToolResult { const now = Date.now(); - const jobResults: AwaitResult[] = jobs.map(j => ({ + const jobResults: PollResult[] = jobs.map(j => ({ id: j.id, type: j.type, - status: j.status as AwaitResult["status"], + status: j.status as PollResult["status"], label: j.label, durationMs: Math.max(0, now - j.startTime), ...(j.resultText ? { resultText: j.resultText } : {}), diff --git a/packages/coding-agent/test/bash-executor.test.ts b/packages/coding-agent/test/bash-executor.test.ts index ccf18d8b2..8a4ab171b 100644 --- a/packages/coding-agent/test/bash-executor.test.ts +++ b/packages/coding-agent/test/bash-executor.test.ts @@ -105,7 +105,7 @@ describe("executeBash", () => { return; } - const result = await executeBash("sleep 10 & echo $!", { + const result = await executeBash('python3 -c "import time; time.sleep(10)" & echo $!', { cwd: tempDir, timeout: 5000, }); @@ -113,9 +113,7 @@ describe("executeBash", () => { expect(Number.isInteger(pid)).toBe(true); expect(pid).toBeGreaterThan(0); expect(() => process.kill(pid, 0)).not.toThrow(); - process.kill(pid, "SIGTERM"); - await Bun.sleep(100); - expect(() => process.kill(pid, 0)).toThrow(); + expect(() => process.kill(pid, "SIGKILL")).not.toThrow(); }); diff --git a/packages/coding-agent/test/tools.test.ts b/packages/coding-agent/test/tools.test.ts index 441153e82..8c281d545 100644 --- a/packages/coding-agent/test/tools.test.ts +++ b/packages/coding-agent/test/tools.test.ts @@ -10,7 +10,7 @@ import { DEFAULT_BASH_INTERCEPTOR_RULES, Settings } from "@oh-my-pi/pi-coding-ag import { EditTool } from "@oh-my-pi/pi-coding-agent/edit"; import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager"; import type { ToolSession } from "@oh-my-pi/pi-coding-agent/tools"; -import { AwaitTool } from "@oh-my-pi/pi-coding-agent/tools/await-tool"; +import { PollTool } from "@oh-my-pi/pi-coding-agent/tools/poll-tool"; import { BashTool } from "@oh-my-pi/pi-coding-agent/tools/bash"; import { CancelJobTool } from "@oh-my-pi/pi-coding-agent/tools/cancel-job"; import { FindTool } from "@oh-my-pi/pi-coding-agent/tools/find"; @@ -790,7 +790,7 @@ function b() { const result = await bashTool.execute("test-call-8", { command: "echo 'test output'" }); expect(getTextOutput(result)).toContain("test output"); - expect(result.details).toBeUndefined(); + expect(result.details?.timeoutSeconds).toBe(300); }); it("should expose built-in interceptor defaults truthfully", () => { @@ -989,7 +989,8 @@ function b() { const result = await autoBackgroundBashTool.execute("test-call-9-auto-inline", { command: "echo short" }); expect(getTextOutput(result)).toContain("short"); - expect(result.details).toBeUndefined(); + expect(result.details?.timeoutSeconds).toBe(300); + expect(result.details?.async).toBeUndefined(); await Bun.sleep(150); expect(deliveries).toEqual([]); await asyncJobManager.dispose(); @@ -1041,6 +1042,51 @@ function b() { await asyncJobManager.dispose(); }); + it("should background instead of timing out when auto-background wait exceeds the effective timeout", async () => { + const deliveries: Array<{ jobId: string; text: string }> = []; + const asyncJobManager = new AsyncJobManager({ + onJobComplete: async (jobId, text) => { + deliveries.push({ jobId, text }); + }, + }); + const autoBackgroundBashTool = wrapToolWithMetaNotice( + new BashTool( + createTestToolSession( + testDir, + Settings.isolated({ + "bash.autoBackground.enabled": true, + "bash.autoBackground.thresholdMs": 60_000, + }), + { + asyncJobManager, + getSessionId: () => "test-session", + }, + ), + ), + ); + + const result = await autoBackgroundBashTool.execute("test-call-9-auto-timeout-background", { + command: "printf 'start\\n'; sleep 1.2; printf 'done\\n'", + timeout: 1, + }); + + expect(result.details?.timeoutSeconds).toBe(1); + expect(result.details?.async?.state).toBe("running"); + expect(getTextOutput(result)).toContain("Background job"); + const jobId = result.details?.async?.jobId; + if (!jobId) { + throw new Error("expected an auto-backgrounded job id"); + } + const runningJob = asyncJobManager.getJob(jobId); + expect(runningJob?.status).toBe("running"); + await runningJob?.promise; + await Bun.sleep(50); + expect(deliveries).toHaveLength(1); + expect(deliveries[0]?.jobId).toBe(jobId); + expect(deliveries[0]?.text).toContain("Command timed out after 1 seconds"); + await asyncJobManager.dispose(); + }); + it("should respect timeout", async () => { await expect(bashTool.execute("test-call-10", { command: "sleep 5", timeout: 1 })).rejects.toThrow( /timed out/i, @@ -1074,12 +1120,12 @@ function b() { Settings.isolated({ "bash.autoBackground.enabled": true }), ); - expect(AwaitTool.createIf(autoBackgroundSession)).not.toBeNull(); + expect(PollTool.createIf(autoBackgroundSession)).not.toBeNull(); expect(CancelJobTool.createIf(autoBackgroundSession)).not.toBeNull(); }); }); - describe("AwaitTool", () => { + describe("PollTool", () => { it("should wait for jobs and acknowledge deliveries to prevent race conditions", async () => { const manager = new AsyncJobManager({ onJobComplete: async () => {}, @@ -1087,14 +1133,14 @@ function b() { const session = createTestToolSession(testDir, Settings.isolated({ "bash.autoBackground.enabled": true }), { asyncJobManager: manager, }); - const awaitTool = AwaitTool.createIf(session)!; + const pollTool = PollTool.createIf(session)!; const jobId = manager.register("bash", "test job", async () => "success"); - // Job is running, call await - const resultPromise = awaitTool.execute("test-call-await-1", { jobs: [jobId] }); + // Job is running, call poll + const resultPromise = pollTool.execute("test-call-poll-1", { jobs: [jobId] }); - // Ensure await finished + // Ensure poll finished const result = await resultPromise; expect(getTextOutput(result)).toContain("Completed"); diff --git a/packages/coding-agent/test/tools/bash-sixel-render.test.ts b/packages/coding-agent/test/tools/bash-sixel-render.test.ts index f801d2d55..dd29d9423 100644 --- a/packages/coding-agent/test/tools/bash-sixel-render.test.ts +++ b/packages/coding-agent/test/tools/bash-sixel-render.test.ts @@ -69,6 +69,21 @@ describe("bashToolRenderer", () => { expect(rendered).not.toContain("\t"); }); + it("shows the effective timeout from result details when it differs from call args", async () => { + const theme = await getThemeByName("dark"); + expect(theme).toBeDefined(); + const uiTheme = theme!; + const component = bashToolRenderer.renderResult( + { content: [{ type: "text", text: "" }], details: { timeoutSeconds: 120 }, isError: false }, + { expanded: false, isPartial: false, renderContext: { timeout: 1200 } }, + uiTheme, + { command: "python3 scripts/vim-edit-benchmark.py", timeout: 1200 }, + ); + const rendered = sanitizeText(component.render(120).join("\n")); + expect(rendered).toContain("Timeout: 120s"); + expect(rendered).not.toContain("Timeout: 1200s"); + }); + it("bypasses truncation/styling for SIXEL lines", async () => { terminal.imageProtocol = ImageProtocol.Sixel; const theme = await getThemeByName("dark");