diff --git a/docs/bash-tool-runtime.md b/docs/bash-tool-runtime.md index 89014abe5..11f986d73 100644 --- a/docs/bash-tool-runtime.md +++ b/docs/bash-tool-runtime.md @@ -101,6 +101,8 @@ Session-level bang-command executions pass `sessionKey: this.sessionId`. Tool-call executions pass `sessionKey: this.session.getSessionId?.()`, when available. In both surfaces, a session key isolates shell reuse per session; without one, reuse falls back to shell config/snapshot/env. +Concurrent calls never share one `Shell`: the native session runs one command at a time and `Shell.abort()` kills every in-flight run on it. `executeBash()` tracks in-flight keys in `shellSessionsInUse`; while a key is busy, overlapping calls skip the cache and run through one-shot `executeShell()` (same isolation as quarantined sessions). Only the owning call releases the in-use flag or deletes the cached session in its `finally`. + ## Shell config and snapshot behavior At each call, executor loads settings shell config (`shell`, `env`, optional `prefix`). diff --git a/docs/tools/bash.md b/docs/tools/bash.md index e4bbb7eea..d739e7b3a 100644 --- a/docs/tools/bash.md +++ b/docs/tools/bash.md @@ -144,7 +144,7 @@ Stdout and stderr are merged before the model sees them. Definite non-zero exit - Artifact allocation / artifact save failures are swallowed in `saveBashOriginalArtifact()` and `OutputSink.#createFileSink()`; execution continues without that artifact. ## Notes -- `strict = true` and `concurrency = "exclusive"` are set on `BashTool`; the tool does not run concurrently with another bash tool call in the same session. +- `strict = true` is set on `BashTool`; `concurrency` is resolved per call: `pty: true` is `"exclusive"` (it takes over the terminal UI), everything else is `"shared"`, so multiple non-pty bash calls in one assistant message run in parallel. When parallel calls overlap on the same shell session key, the first owns the persistent `Shell`; the rest run in isolated one-shot shells (see `shellSessionsInUse` in `bash-executor.ts`). - `command` URL expansions shell-escape replacements; `env` and `cwd` expansion use `noEscape: true` because they become environment values / filesystem paths, not shell text. - `checkBashInterception()` blocks only when the matching rule's `tool` name is present in `ctx.toolNames`; missing tools disable their corresponding rule. - Default interceptor rules come from `DEFAULT_BASH_INTERCEPTOR_RULES` in `packages/coding-agent/src/config/settings-schema.ts`: diff --git a/packages/agent/CHANGELOG.md b/packages/agent/CHANGELOG.md index d5e77e233..7b23b6759 100644 --- a/packages/agent/CHANGELOG.md +++ b/packages/agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Added + +- `AgentTool.concurrency` now also accepts a per-call resolver function `(args) => "shared" | "exclusive"`, letting tools pick the scheduling mode from the call's arguments (a throwing resolver falls back to `"exclusive"`) + ### Fixed - Fixed whitespace-only error tool results so Anthropic requests no longer 400 with `tool_result: content cannot be empty if is_error is true` and wedge the session on every subsequent turn diff --git a/packages/agent/src/agent-loop.ts b/packages/agent/src/agent-loop.ts index 3c3f9362d..a4fee4e2d 100644 --- a/packages/agent/src/agent-loop.ts +++ b/packages/agent/src/agent-loop.ts @@ -1563,7 +1563,19 @@ async function executeToolCalls( for (let index = 0; index < records.length; index++) { const record = records[index]; - const concurrency = record.tool?.concurrency ?? "shared"; + const concurrencyMode = record.tool?.concurrency; + let concurrency: "shared" | "exclusive"; + if (typeof concurrencyMode === "function") { + // Resolved from raw pre-validation args; a throwing resolver must not + // take down the whole batch, so fall back to the safe (serial) mode. + try { + concurrency = concurrencyMode(record.args); + } catch { + concurrency = "exclusive"; + } + } else { + concurrency = concurrencyMode ?? "shared"; + } const start = concurrency === "exclusive" ? Promise.all([lastExclusive, ...sharedTasks]) : lastExclusive; const task = start.then(() => runTool(record, index)); tasks.push(task); diff --git a/packages/agent/src/types.ts b/packages/agent/src/types.ts index f0ad52a15..d37929820 100644 --- a/packages/agent/src/types.ts +++ b/packages/agent/src/types.ts @@ -456,8 +456,9 @@ export interface AgentTool>) => "shared" | "exclusive"); /** If true, argument validation errors are non-fatal: raw args are passed to execute() instead of returning an error to the LLM. */ lenientArgValidation?: boolean; /** diff --git a/packages/agent/test/agent-loop.test.ts b/packages/agent/test/agent-loop.test.ts index 6883855d7..73354631d 100644 --- a/packages/agent/test/agent-loop.test.ts +++ b/packages/agent/test/agent-loop.test.ts @@ -569,6 +569,83 @@ describe("agentLoop with AgentMessage", () => { expect(turnEndEvent.toolResults.map(result => result.toolCallId)).toEqual(["tool-2", "tool-1"]); }); + it("resolves function-form concurrency per call", async () => { + const toolSchema = z.object({ value: z.string(), exclusive: z.boolean().optional() }); + const startTimes: Record = {}; + const finishTimes: Record = {}; + const { promise: slowContinue, resolve: slowResolve } = Promise.withResolvers(); + const { promise: slowStarted, resolve: slowStartedResolve } = Promise.withResolvers(); + const { promise: fastFinished, resolve: fastFinishedResolve } = Promise.withResolvers(); + + const tool: AgentTool = { + name: "echo", + label: "Echo", + description: "Echo tool", + parameters: toolSchema, + concurrency: args => (args.exclusive === true ? "exclusive" : "shared"), + async execute(_toolCallId, params) { + if (params.value === "slow") { + startTimes.slow = Bun.nanoseconds(); + slowStartedResolve(); + await slowContinue; + finishTimes.slow = Bun.nanoseconds(); + } else if (params.value === "fast") { + await slowStarted; + startTimes.fast = Bun.nanoseconds(); + finishTimes.fast = Bun.nanoseconds(); + fastFinishedResolve(); + } else { + startTimes.exclusive = Bun.nanoseconds(); + finishTimes.exclusive = Bun.nanoseconds(); + } + return { + content: [{ type: "text", text: `echoed: ${params.value}` }], + details: { value: params.value }, + }; + }, + }; + + const context: AgentContext = { systemPrompt: [""], messages: [], tools: [tool] }; + + const mock = createMockModel({ + responses: [ + { + content: [ + { type: "toolCall", id: "tool-1", name: "echo", arguments: { value: "slow" } }, + { type: "toolCall", id: "tool-2", name: "echo", arguments: { value: "fast" } }, + { + type: "toolCall", + id: "tool-3", + name: "echo", + arguments: { value: "last", exclusive: true }, + }, + ], + }, + { content: ["done"] }, + ], + }); + const config: AgentLoopConfig = { model: mock.model, convertToLlm: identityConverter }; + + const events: AgentEvent[] = []; + const stream = agentLoop([createUserMessage("start")], context, config, undefined, mock.stream); + const streamTask = (async () => { + for await (const event of stream) { + events.push(event); + } + })(); + + await fastFinished; + slowResolve(); + await streamTask; + + // Both shared calls overlapped: fast started and finished while slow was running. + expect(startTimes.fast).toBeLessThan(finishTimes.slow); + expect(finishTimes.fast).toBeLessThan(finishTimes.slow); + // The exclusive call waited for every shared call to finish. + expect(startTimes.exclusive).toBeGreaterThan(finishTimes.slow); + expect(startTimes.exclusive).toBeGreaterThan(finishTimes.fast); + }); + it("drops incomplete tool calls when assistant aborts before toolcall_end", async () => { const context: AgentContext = { systemPrompt: ["You are helpful."], diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 0576141db..1a9e3dd0a 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -9,6 +9,10 @@ - Added the Expert Elixir language server (`expert`, invoked as `expert --stdio`) to the built-in LSP server list, auto-detected for Mix projects (`mix.exs`/`mix.lock`). When both are installed, `elixir-ls` remains the primary navigation server (Expert is ordered after it). +### Changed + +- Bash tool calls in one assistant message now run in parallel instead of serializing the batch: non-pty `bash` is scheduled as a shared tool (`pty: true` stays exclusive), and overlapping calls on the same shell session key degrade to isolated one-shot shells so they cannot queue behind or abort each other + ### Fixed - Fixed local memory consolidation on Responses-style models that reject user-only requests by sending a dedicated stage-two system prompt. diff --git a/packages/coding-agent/src/exec/bash-executor.ts b/packages/coding-agent/src/exec/bash-executor.ts index 306235534..f781289a6 100644 --- a/packages/coding-agent/src/exec/bash-executor.ts +++ b/packages/coding-agent/src/exec/bash-executor.ts @@ -56,6 +56,8 @@ export interface BashResult { const shellSessions = new Map(); const brokenShellSessions = new Set(); const shellSessionQuarantines = new Map>(); +/** Session keys with a command currently in flight on the persistent Shell. */ +const shellSessionsInUse = new Set(); function quarantineShellSession( sessionKey: string, @@ -223,8 +225,14 @@ export async function executeBash(command: string, options?: BashExecutorOptions shellSessions.delete(sessionKey); } - let shellSession = persistentSessionBroken ? undefined : shellSessions.get(sessionKey); - if (!shellSession && !persistentSessionBroken) { + // A persistent Shell runs one command at a time (the native session is a + // mutex-guarded queue and `abort()` kills every in-flight run on it). When + // parallel bash calls overlap on the same key, the first one owns the + // persistent session; the rest degrade to isolated one-shot shells — the + // same path quarantined sessions take. + const sessionBusy = shellSessionsInUse.has(sessionKey); + let shellSession = persistentSessionBroken || sessionBusy ? undefined : shellSessions.get(sessionKey); + if (!shellSession && !persistentSessionBroken && !sessionBusy) { shellSession = new Shell({ sessionEnv: shellEnv, snapshotPath: snapshotPath ?? undefined, @@ -232,6 +240,10 @@ export async function executeBash(command: string, options?: BashExecutorOptions }); shellSessions.set(sessionKey, shellSession); } + const ownsPersistentSession = shellSession !== undefined; + if (ownsPersistentSession) { + shellSessionsInUse.add(sessionKey); + } const userSignal = options?.signal; const runAbortController = new AbortController(); let abortCleanupPromise: Promise | undefined; @@ -393,10 +405,13 @@ export async function executeBash(command: string, options?: BashExecutorOptions if (userSignal) { userSignal.removeEventListener("abort", abortHandler); } - if (resetSession || options?.sessionKey?.includes(":async:")) { - // `:async:` keys are per-job (jobId is unique), so the Shell would - // otherwise stay in the process-global map forever after completion. - shellSessions.delete(sessionKey); + if (ownsPersistentSession) { + shellSessionsInUse.delete(sessionKey); + if (resetSession || options?.sessionKey?.includes(":async:")) { + // `:async:` keys are per-job (jobId is unique), so the Shell would + // otherwise stay in the process-global map forever after completion. + shellSessions.delete(sessionKey); + } } } } diff --git a/packages/coding-agent/src/prompts/tools/bash.md b/packages/coding-agent/src/prompts/tools/bash.md index 665b9b9c3..726f6f41e 100644 --- a/packages/coding-agent/src/prompts/tools/bash.md +++ b/packages/coding-agent/src/prompts/tools/bash.md @@ -6,6 +6,7 @@ Executes bash command in shell session for terminal operations like git, bun, ca - Quote variable expansions like `"$NAME"` to preserve exact content - PTY mode is opt-in: set `pty: true` only when the command needs a real terminal (e.g. `sudo`, `ssh` requiring user input); default is `false` - Use `;` only when later commands should run regardless of earlier failures +- Multiple bash calls in one message run concurrently. NEVER split order-dependent commands across parallel calls — chain them with `&&` in a single call. - Internal URIs (`skill://`, `agent://`, etc.) are auto-resolved to filesystem paths {{#if asyncEnabled}} - Use `async: true` for long-running commands when you don't need immediate output; the call returns a background job ID and the result is delivered automatically as a follow-up. diff --git a/packages/coding-agent/src/tools/bash.ts b/packages/coding-agent/src/tools/bash.ts index ce7dda67b..fc9805592 100644 --- a/packages/coding-agent/src/tools/bash.ts +++ b/packages/coding-agent/src/tools/bash.ts @@ -368,7 +368,11 @@ export class BashTool implements AgentTool { readonly loadMode = "essential"; readonly description: string; readonly parameters: BashToolSchema; - readonly concurrency = "exclusive"; + // Non-pty calls run alongside each other (the executor isolates overlapping + // runs on the same shell session); pty takes over the terminal UI and must + // run alone. + readonly concurrency = (args: Partial): "shared" | "exclusive" => + args.pty === true ? "exclusive" : "shared"; readonly strict = true; readonly #asyncEnabled: boolean; readonly #autoBackgroundEnabled: boolean; diff --git a/packages/coding-agent/test/bash-executor.test.ts b/packages/coding-agent/test/bash-executor.test.ts index 10bef886a..6135c8f6c 100644 --- a/packages/coding-agent/test/bash-executor.test.ts +++ b/packages/coding-agent/test/bash-executor.test.ts @@ -548,6 +548,56 @@ exit 64 }); expect(afterAbort.output.trim()).toBe("unset"); }); + + it("runs overlapping calls on the same session key concurrently", async () => { + if (process.platform === "win32") return; + + const sessionKey = "parallel-overlap"; + const order: string[] = []; + const slow = executeBash('sleep 0.6 && echo "A-done"', { cwd: tempDir, timeout: 5000, sessionKey }).then( + result => { + order.push("slow"); + return result; + }, + ); + const fast = executeBash('echo "B-done"', { cwd: tempDir, timeout: 5000, sessionKey }).then(result => { + order.push("fast"); + return result; + }); + + const [slowResult, fastResult] = await Promise.all([slow, fast]); + expect(slowResult.exitCode).toBe(0); + expect(slowResult.output).toContain("A-done"); + expect(fastResult.exitCode).toBe(0); + expect(fastResult.output).toContain("B-done"); + // If the second call had queued behind the persistent session it could + // not finish before the 600ms sleep of the first. + expect(order).toEqual(["fast", "slow"]); + }); + + it("keeps the owner session usable when an overlapping call times out", async () => { + if (process.platform === "win32") return; + + const sessionKey = "parallel-timeout-isolation"; + const owner = executeBash('sleep 1.3 && echo "owner-done"', { + cwd: tempDir, + timeout: 5000, + sessionKey, + }); + // Overlaps with the owner for its whole lifetime; times out at the 1s floor. + const overlapping = await executeBash("sleep 5", { cwd: tempDir, timeout: 1000, sessionKey }); + expect(overlapping.cancelled).toBe(true); + + const ownerResult = await owner; + expect(ownerResult.exitCode).toBe(0); + expect(ownerResult.output).toContain("owner-done"); + + // The overlapping timeout must not quarantine or delete the persistent + // session owned by the first call. + const after = await executeBash('echo "still-ok"', { cwd: tempDir, timeout: 5000, sessionKey }); + expect(after.exitCode).toBe(0); + expect(after.output).toContain("still-ok"); + }); it("streams output chunks", async () => { const chunks: string[] = []; const result = await executeBash("i=1; while [ $i -le 20 ]; do echo line$i; i=$((i+1)); done", {