Skip to content

Commit 5db9e90

Browse files
committed
🤖 fix: run forked task tool calls in parallel again
Exempt only provably-isolated `task` tool calls from the per-stream sequential-execution mutex, restoring parallel sub-agent launches without reintroducing shared-working-tree races. #2906 serializes every sibling tool execute() in a stream behind one mutex to prevent races on shared mutable state. That also serializes parallel `task` launches: only the first runs, the rest block on the long foreground waitForAgentReport. A forking `task` runs in its own isolated checkout and shares no mutable state with sibling tools, so it is safe to run in parallel. But a `task` that runs in the PARENT checkout (local runtime, or worktree/ssh with isolation:"none") shares the working tree and must stay serialized. Decide per call, by runtime mode + isolation arg, via the new taskCallSharesParentWorkspace() helper. runtimeMode is threaded (optionally, defaulting to serialize) from the AIService send path into the stream-request builder. All other tools remain fully serialized.
1 parent ec3314e commit 5db9e90

6 files changed

Lines changed: 359 additions & 21 deletions

File tree

src/common/types/runtime.test.ts

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import {
55
parseRuntimeModeAndHost,
66
RUNTIME_MODE,
77
runtimeModeSupportsSharedTaskWorkspace,
8+
taskCallSharesParentWorkspace,
89
} from "./runtime";
910

1011
describe("runtimeModeSupportsSharedTaskWorkspace", () => {
@@ -21,6 +22,19 @@ describe("runtimeModeSupportsSharedTaskWorkspace", () => {
2122
});
2223
});
2324

25+
describe("taskCallSharesParentWorkspace", () => {
26+
it("matches the runtime/isolation sharing matrix", () => {
27+
expect(taskCallSharesParentWorkspace(RUNTIME_MODE.LOCAL, undefined)).toBe(true);
28+
expect(taskCallSharesParentWorkspace(undefined, undefined)).toBe(true);
29+
expect(taskCallSharesParentWorkspace(RUNTIME_MODE.WORKTREE, undefined)).toBe(false);
30+
expect(taskCallSharesParentWorkspace(RUNTIME_MODE.WORKTREE, "none")).toBe(true);
31+
expect(taskCallSharesParentWorkspace(RUNTIME_MODE.WORKTREE, "fork")).toBe(false);
32+
expect(taskCallSharesParentWorkspace(RUNTIME_MODE.SSH, "none")).toBe(true);
33+
expect(taskCallSharesParentWorkspace(RUNTIME_MODE.DOCKER, undefined)).toBe(false);
34+
expect(taskCallSharesParentWorkspace(RUNTIME_MODE.DEVCONTAINER, undefined)).toBe(false);
35+
});
36+
});
37+
2438
describe("parseRuntimeModeAndHost", () => {
2539
it("parses SSH mode with host", () => {
2640
expect(parseRuntimeModeAndHost("ssh user@host")).toEqual({

src/common/types/runtime.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,26 @@ export function runtimeModeSupportsSharedTaskWorkspace(mode: RuntimeMode | undef
4747
return mode != null && SHARED_TASK_WORKSPACE_RUNTIME_MODES.includes(mode);
4848
}
4949

50+
/**
51+
* Whether a `task` tool call will run in the PARENT workspace's checkout (shared working tree)
52+
* instead of an isolated fork. Sibling tool calls that mutate the shared tree must be serialized
53+
* against such a task; forking task calls are isolated and safe to run in parallel.
54+
*
55+
* - local (or unknown runtime): forking is a no-op / unknown — treat as shared (serialize).
56+
* - worktree/ssh: shares only when the model opts in via `isolation: "none"`.
57+
* - docker/devcontainer: always fork, so never shared.
58+
*
59+
* `isolation` is typed `unknown` so callers can pass raw, pre-validation tool args.
60+
*/
61+
export function taskCallSharesParentWorkspace(
62+
mode: RuntimeMode | undefined,
63+
isolation: unknown
64+
): boolean {
65+
if (mode == null || mode === RUNTIME_MODE.LOCAL) return true;
66+
if (!runtimeModeSupportsSharedTaskWorkspace(mode)) return false;
67+
return isolation === "none";
68+
}
69+
5070
/**
5171
* Runtime IDs that can be enabled/disabled in Settings → Runtimes.
5272
* Note: includes "coder" which is a UI-level choice (not a RuntimeMode).

src/node/services/aiService.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2671,7 +2671,8 @@ export class AIService extends EventEmitter {
26712671
}
26722672
: undefined,
26732673
runtimeTempDir,
2674-
modelFallback
2674+
modelFallback,
2675+
runtimeType
26752676
);
26762677
recordStartupPhaseTiming("startStreamMs", startStreamStartedAt);
26772678

src/node/services/streamManager.ts

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ import { stripWorkflowRunRecordsFromModelMessages } from "@/node/utils/messages/
5757
import { normalizeToCanonical } from "@/common/utils/ai/models";
5858
import { MUX_GATEWAY_SESSION_EXPIRED_MESSAGE } from "@/common/constants/muxGatewayOAuth";
5959
import { getModelStats, getModelStatsResolved } from "@/common/utils/tokens/modelStats";
60+
import type { RuntimeMode } from "@/common/types/runtime";
6061
import { withSequentialExecution } from "@/node/services/tools/withSequentialExecution";
6162
import type { ResolvedCallSettingsOverrides } from "@/common/config/schemas/modelParameters";
6263
import { resolveModelForMetadata } from "@/common/utils/providers/modelEntries";
@@ -488,6 +489,7 @@ interface WorkspaceStreamInfo {
488489
metadataModel: string;
489490
/** Effective thinking level after model policy clamping */
490491
thinkingLevel?: string;
492+
runtimeMode?: RuntimeMode;
491493
initialMetadata?: Partial<MuxMetadata>;
492494
toolModelUsages: PersistedToolModelUsage[];
493495
request: StreamRequestConfig;
@@ -1288,7 +1290,8 @@ export class StreamManager extends EventEmitter {
12881290
headers?: Record<string, string | undefined>,
12891291
anthropicCacheTtlOverride?: AnthropicCacheTtl,
12901292
onChunk?: StreamTextOnChunk,
1291-
onStepMessages?: (messages: ModelMessage[]) => void
1293+
onStepMessages?: (messages: ModelMessage[]) => void,
1294+
runtimeMode?: RuntimeMode
12921295
): StreamRequestConfig {
12931296
const finalProviderOptions = providerOptions;
12941297

@@ -1335,7 +1338,7 @@ export class StreamManager extends EventEmitter {
13351338
system: finalSystem,
13361339
// Keep provider-level parallel tool planning enabled, but serialize sibling
13371340
// execute() handlers inside this stream so shared mutable state cannot race.
1338-
tools: withSequentialExecution(finalTools),
1341+
tools: withSequentialExecution(finalTools, runtimeMode),
13391342
providerOptions: finalProviderOptions,
13401343
headers,
13411344
maxOutputTokens: effectiveMaxOutputTokens,
@@ -1469,7 +1472,8 @@ export class StreamManager extends EventEmitter {
14691472
anthropicCacheTtlOverride?: AnthropicCacheTtl,
14701473
onChunk?: StreamTextOnChunk,
14711474
onStepMessages?: (messages: ModelMessage[]) => void,
1472-
modelFallback?: ModelFallbackOptions
1475+
modelFallback?: ModelFallbackOptions,
1476+
runtimeMode?: RuntimeMode
14731477
): WorkspaceStreamInfo {
14741478
// abortController is created and linked to the caller-provided abortSignal in startStream().
14751479

@@ -1489,7 +1493,8 @@ export class StreamManager extends EventEmitter {
14891493
headers,
14901494
anthropicCacheTtlOverride,
14911495
onChunk,
1492-
onStepMessages
1496+
onStepMessages,
1497+
runtimeMode
14931498
);
14941499

14951500
// Start streaming - this can throw immediately if API key is missing
@@ -1517,6 +1522,7 @@ export class StreamManager extends EventEmitter {
15171522
model: modelString,
15181523
metadataModel,
15191524
thinkingLevel,
1525+
runtimeMode,
15201526
initialMetadata,
15211527
toolModelUsages: [],
15221528
didRetryPreviousResponseIdAtStep: false,
@@ -2133,7 +2139,8 @@ export class StreamManager extends EventEmitter {
21332139
prepared.data.headers,
21342140
prepared.data.anthropicCacheTtl,
21352141
streamInfo.request.onChunk,
2136-
streamInfo.request.onStepMessages
2142+
streamInfo.request.onStepMessages,
2143+
streamInfo.runtimeMode
21372144
);
21382145
// createStreamResult may eagerly prepare the first fallback step and update
21392146
// latestMessages. Clear stale source-step messages before starting it so a
@@ -3527,7 +3534,8 @@ export class StreamManager extends EventEmitter {
35273534
onChunk?: StreamTextOnChunk,
35283535
onStepMessages?: (messages: ModelMessage[]) => void,
35293536
providedRuntimeTempDir?: string,
3530-
modelFallback?: ModelFallbackOptions
3537+
modelFallback?: ModelFallbackOptions,
3538+
runtimeMode?: RuntimeMode
35313539
): Promise<Result<StreamToken, SendMessageError>> {
35323540
const typedWorkspaceId = workspaceId as WorkspaceId;
35333541

@@ -3610,7 +3618,8 @@ export class StreamManager extends EventEmitter {
36103618
anthropicCacheTtlOverride,
36113619
onChunk,
36123620
onStepMessages,
3613-
modelFallback
3621+
modelFallback,
3622+
runtimeMode
36143623
);
36153624

36163625
// Guard against a narrow race:

0 commit comments

Comments
 (0)