Skip to content

Commit 368aed4

Browse files
authored
🤖 Suspend tool calls until workspace init completes (#455)
## Problem SSH workspaces require asynchronous initialization (rsync, checkout, init hook) before tools can execute. Without proper synchronization, tools may race with initialization and fail with confusing errors. ## Solution **Promise-based init waiting**: Tools wait for workspace initialization to complete before executing. If init fails or times out (5 minutes), tools proceed anyway and fail naturally with their own errors (better UX than blocking). ### Architecture **`InitStateManager`** - Tracks init state with promise-based waiting: - `startInit(workspaceId, hookPath)` - Creates completion promise - `endInit(workspaceId, exitCode)` - Resolves promise (success or failure) - `waitForInit(workspaceId)` - **Never throws**. Returns immediately if no init needed, init already done, or timeout (5 min) **`wrapWithInitWait()`** - Wrapper for runtime-dependent tools: - Wraps: `bash`, `file_read`, `file_edit_insert`, `file_edit_replace_string` - Does NOT wrap: `propose_plan`, `todo_*`, `web_search` (execute immediately) **Clean separation**: `initStateManager` only passed to wrapper, not to individual tools. Each tool receives clean config without init-related fields. ### Before/After | Before | After | |--------|-------| | Tools execute immediately | Runtime tools wait for init first | | Race conditions on SSH | Init completes before tools run | | Tools called `waitForInit()` inline | Centralized in `wrapWithInitWait()` | | initStateManager in all tool configs | Only in wrapper layer | ## Changes **Core:** - `src/services/initStateManager.ts` (362 lines) - Promise-based state tracking - `src/services/tools/wrapWithInitWait.ts` (38 lines) - Init wait wrapper - `src/utils/tools/tools.ts` - Apply wrapper to runtime tools only **Tests:** - `tests/ipcMain/initWorkspace.test.ts` - Extended integration tests (7 tests, all pass) - `tests/ipcMain/helpers.ts` - New helpers: `collectInitEvents()`, `waitForInitEnd()` - Deduplicated ~100 lines of test code ## Integration Test Results **Verified behavior:** 1. Init hook output streams correctly (not batched) 2. Tools wait for init before executing (both success and failure cases) 3. Workspace remains usable after init failure (tools work, fail naturally) 4. Init state persists across page reloads 5. Works correctly for both local and SSH runtimes ``` ✓ 7 integration tests pass (initWorkspace.test.ts) ✓ 831 unit tests pass ✓ Init wait adds ~5 seconds on first tool use (expected) ✓ Subsequent tools execute immediately (no redundant waiting) ``` ## Key Design Decision **Why `waitForInit()` never throws**: Init hook is optional setup, not a prerequisite. If init fails or times out, tools should proceed and fail naturally with their own errors (e.g., "file not found"). This provides better error messages than blocking on init. _Generated with `cmux`_
1 parent 5c383ca commit 368aed4

21 files changed

+759
-168
lines changed

src/debug/agentSessionCli.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@ import { parseArgs } from "util";
77
import { Config } from "@/config";
88
import { HistoryService } from "@/services/historyService";
99
import { PartialService } from "@/services/partialService";
10-
import { AIService } from "@/services/aiService";
1110
import { InitStateManager } from "@/services/initStateManager";
11+
import { AIService } from "@/services/aiService";
1212
import { AgentSession, type AgentSessionChatEvent } from "@/services/agentSession";
1313
import {
1414
isCaughtUpMessage,
@@ -209,8 +209,8 @@ async function main(): Promise<void> {
209209

210210
const historyService = new HistoryService(config);
211211
const partialService = new PartialService(config, historyService);
212-
const aiService = new AIService(config, historyService, partialService);
213212
const initStateManager = new InitStateManager(config);
213+
const aiService = new AIService(config, historyService, partialService, initStateManager);
214214
ensureProvidersConfig(config);
215215

216216
const session = new AgentSession({

src/debug/replay-history.ts

100755100644
Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { parseArgs } from "util";
1717
import { defaultConfig } from "@/config";
1818
import type { CmuxMessage } from "@/types/message";
1919
import { createCmuxMessage } from "@/types/message";
20+
import { InitStateManager } from "@/services/initStateManager";
2021
import { AIService } from "@/services/aiService";
2122
import { HistoryService } from "@/services/historyService";
2223
import { PartialService } from "@/services/partialService";
@@ -123,7 +124,8 @@ async function main() {
123124
const config = defaultConfig;
124125
const historyService = new HistoryService(config);
125126
const partialService = new PartialService(config, historyService);
126-
const aiService = new AIService(config, historyService, partialService);
127+
const initStateManager = new InitStateManager(config);
128+
const aiService = new AIService(config, historyService, partialService, initStateManager);
127129

128130
const modelString = values.model ?? "openai:gpt-5-codex";
129131
const thinkingLevel = (values.thinking ?? "high") as "low" | "medium" | "high";

src/services/aiService.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { describe, it, expect, beforeEach } from "bun:test";
66
import { AIService } from "./aiService";
77
import { HistoryService } from "./historyService";
88
import { PartialService } from "./partialService";
9+
import { InitStateManager } from "./initStateManager";
910
import { Config } from "@/config";
1011

1112
describe("AIService", () => {
@@ -15,7 +16,8 @@ describe("AIService", () => {
1516
const config = new Config();
1617
const historyService = new HistoryService(config);
1718
const partialService = new PartialService(config, historyService);
18-
service = new AIService(config, historyService, partialService);
19+
const initStateManager = new InitStateManager(config);
20+
service = new AIService(config, historyService, partialService, initStateManager);
1921
});
2022

2123
// Note: These tests are placeholders as Bun doesn't support Jest mocking

src/services/aiService.ts

Lines changed: 31 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import type { CmuxMessage, CmuxTextPart } from "@/types/message";
1212
import { createCmuxMessage } from "@/types/message";
1313
import type { Config } from "@/config";
1414
import { StreamManager } from "./streamManager";
15+
import type { InitStateManager } from "./initStateManager";
1516
import type { SendMessageError } from "@/types/errors";
1617
import { getToolsForModel } from "@/utils/tools/tools";
1718
import { createRuntime } from "@/runtime/runtimeFactory";
@@ -108,17 +109,24 @@ export class AIService extends EventEmitter {
108109
private readonly historyService: HistoryService;
109110
private readonly partialService: PartialService;
110111
private readonly config: Config;
112+
private readonly initStateManager: InitStateManager;
111113
private readonly mockModeEnabled: boolean;
112114
private readonly mockScenarioPlayer?: MockScenarioPlayer;
113115

114-
constructor(config: Config, historyService: HistoryService, partialService: PartialService) {
116+
constructor(
117+
config: Config,
118+
historyService: HistoryService,
119+
partialService: PartialService,
120+
initStateManager: InitStateManager
121+
) {
115122
super();
116123
// Increase max listeners to accommodate multiple concurrent workspace listeners
117124
// Each workspace subscribes to stream events, and we expect >10 concurrent workspaces
118125
this.setMaxListeners(50);
119126
this.config = config;
120127
this.historyService = historyService;
121128
this.partialService = partialService;
129+
this.initStateManager = initStateManager;
122130
this.streamManager = new StreamManager(historyService, partialService);
123131
void this.ensureSessionsDir();
124132
this.setupStreamEventForwarding();
@@ -423,12 +431,17 @@ export class AIService extends EventEmitter {
423431

424432
// Get tool names early for mode transition sentinel (stub config, no workspace context needed)
425433
const earlyRuntime = createRuntime({ type: "local", srcBaseDir: process.cwd() });
426-
const earlyAllTools = await getToolsForModel(modelString, {
427-
cwd: process.cwd(),
428-
runtime: earlyRuntime,
429-
runtimeTempDir: os.tmpdir(),
430-
secrets: {},
431-
});
434+
const earlyAllTools = await getToolsForModel(
435+
modelString,
436+
{
437+
cwd: process.cwd(),
438+
runtime: earlyRuntime,
439+
runtimeTempDir: os.tmpdir(),
440+
secrets: {},
441+
},
442+
"", // Empty workspace ID for early stub config
443+
this.initStateManager
444+
);
432445
const earlyTools = applyToolPolicy(earlyAllTools, toolPolicy);
433446
const toolNamesForSentinel = Object.keys(earlyTools);
434447

@@ -533,12 +546,17 @@ export class AIService extends EventEmitter {
533546
const runtimeTempDir = await this.streamManager.createTempDirForStream(streamToken, runtime);
534547

535548
// Get model-specific tools with workspace path (correct for local or remote)
536-
const allTools = await getToolsForModel(modelString, {
537-
cwd: workspacePath,
538-
runtime,
539-
secrets: secretsToRecord(projectSecrets),
540-
runtimeTempDir,
541-
});
549+
const allTools = await getToolsForModel(
550+
modelString,
551+
{
552+
cwd: workspacePath,
553+
runtime,
554+
secrets: secretsToRecord(projectSecrets),
555+
runtimeTempDir,
556+
},
557+
workspaceId,
558+
this.initStateManager
559+
);
542560

543561
// Apply tool policy to filter tools (if policy provided)
544562
const tools = applyToolPolicy(allTools, toolPolicy);

src/services/initStateManager.test.ts

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,10 @@ describe("InitStateManager", () => {
5353
manager.appendOutput(workspaceId, "Installing deps...", false);
5454
manager.appendOutput(workspaceId, "Done!", false);
5555
expect(manager.getInitState(workspaceId)?.lines).toEqual([
56-
{ line: "Installing deps...", isError: false, timestamp: expect.any(Number) as number },
57-
{ line: "Done!", isError: false, timestamp: expect.any(Number) as number },
56+
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
57+
{ line: "Installing deps...", isError: false, timestamp: expect.any(Number) },
58+
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
59+
{ line: "Done!", isError: false, timestamp: expect.any(Number) },
5860
]);
5961

6062
// End init (await to ensure event fires)
@@ -79,8 +81,10 @@ describe("InitStateManager", () => {
7981

8082
const state = manager.getInitState(workspaceId);
8183
expect(state?.lines).toEqual([
82-
{ line: "stdout line", isError: false, timestamp: expect.any(Number) as number },
83-
{ line: "stderr line", isError: true, timestamp: expect.any(Number) as number },
84+
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
85+
{ line: "stdout line", isError: false, timestamp: expect.any(Number) },
86+
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
87+
{ line: "stderr line", isError: true, timestamp: expect.any(Number) },
8488
]);
8589
});
8690

@@ -109,8 +113,10 @@ describe("InitStateManager", () => {
109113
expect(diskState?.status).toBe("success");
110114
expect(diskState?.exitCode).toBe(0);
111115
expect(diskState?.lines).toEqual([
112-
{ line: "Line 1", isError: false, timestamp: expect.any(Number) as number },
113-
{ line: "Line 2", isError: true, timestamp: expect.any(Number) as number },
116+
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
117+
{ line: "Line 1", isError: false, timestamp: expect.any(Number) },
118+
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
119+
{ line: "Line 2", isError: true, timestamp: expect.any(Number) },
114120
]);
115121
});
116122

@@ -221,15 +227,25 @@ describe("InitStateManager", () => {
221227
expect(stateAfterDelete).toBeNull();
222228
});
223229

224-
it("should clear in-memory state", () => {
230+
it("should clear in-memory state", async () => {
225231
const workspaceId = "test-workspace";
226232
manager.startInit(workspaceId, "/path/to/hook");
227233

228234
expect(manager.getInitState(workspaceId)).toBeTruthy();
229235

236+
// Get the init promise before clearing
237+
const initPromise = manager.waitForInit(workspaceId);
238+
239+
// Clear in-memory state (rejects internal promise, but waitForInit catches it)
230240
manager.clearInMemoryState(workspaceId);
231241

242+
// Verify state is cleared
232243
expect(manager.getInitState(workspaceId)).toBeUndefined();
244+
245+
// waitForInit never throws - it resolves even when init is canceled
246+
// This allows tools to proceed and fail naturally with their own errors
247+
// eslint-disable-next-line @typescript-eslint/await-thenable
248+
await expect(initPromise).resolves.toBeUndefined();
233249
});
234250
});
235251

src/services/initStateManager.ts

Lines changed: 115 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,15 +47,29 @@ type InitHookState = InitStatus;
4747
* - Permanent persistence (init logs kept forever as workspace metadata)
4848
*
4949
* Lifecycle:
50-
* 1. startInit() - Create in-memory state, emit init-start
50+
* 1. startInit() - Create in-memory state, emit init-start, create completion promise
5151
* 2. appendOutput() - Accumulate lines, emit init-output
52-
* 3. endInit() - Finalize state, write to disk, emit init-end
52+
* 3. endInit() - Finalize state, write to disk, emit init-end, resolve promise
5353
* 4. State remains in memory until cleared or process restart
5454
* 5. replayInit() - Re-emit events from in-memory or disk state (via EventStore)
55+
*
56+
* Waiting: Tools use waitForInit() which returns a promise that resolves when
57+
* init completes. This promise is stored in initPromises map and resolved by
58+
* endInit(). No event listeners needed, eliminating race conditions.
5559
*/
5660
export class InitStateManager extends EventEmitter {
5761
private readonly store: EventStore<InitHookState, WorkspaceInitEvent & { workspaceId: string }>;
5862

63+
/**
64+
* Promise-based completion tracking for running inits.
65+
* Each running init has a promise that resolves when endInit() is called.
66+
* Multiple tools can await the same promise without race conditions.
67+
*/
68+
private readonly initPromises = new Map<
69+
string,
70+
{ promise: Promise<void>; resolve: () => void; reject: (error: Error) => void }
71+
>();
72+
5973
constructor(config: Config) {
6074
super();
6175
this.store = new EventStore(
@@ -111,7 +125,7 @@ export class InitStateManager extends EventEmitter {
111125

112126
/**
113127
* Start tracking a new init hook execution.
114-
* Creates in-memory state and emits init-start event.
128+
* Creates in-memory state, completion promise, and emits init-start event.
115129
*/
116130
startInit(workspaceId: string, hookPath: string): void {
117131
const startTime = Date.now();
@@ -127,6 +141,21 @@ export class InitStateManager extends EventEmitter {
127141

128142
this.store.setState(workspaceId, state);
129143

144+
// Create completion promise for this init
145+
// This allows multiple tools to await the same init without event listeners
146+
let resolve: () => void;
147+
let reject: (error: Error) => void;
148+
const promise = new Promise<void>((res, rej) => {
149+
resolve = res;
150+
reject = rej;
151+
});
152+
153+
this.initPromises.set(workspaceId, {
154+
promise,
155+
resolve: resolve!,
156+
reject: reject!,
157+
});
158+
130159
log.debug(`Init hook started for workspace ${workspaceId}: ${hookPath}`);
131160

132161
// Emit init-start event
@@ -167,7 +196,7 @@ export class InitStateManager extends EventEmitter {
167196

168197
/**
169198
* Finalize init hook execution.
170-
* Updates state, persists to disk, and emits init-end event.
199+
* Updates state, persists to disk, emits init-end event, and resolves completion promise.
171200
*/
172201
async endInit(workspaceId: string, exitCode: number): Promise<void> {
173202
const state = this.store.getState(workspaceId);
@@ -197,6 +226,13 @@ export class InitStateManager extends EventEmitter {
197226
timestamp: endTime,
198227
} satisfies WorkspaceInitEvent & { workspaceId: string });
199228

229+
// Resolve completion promise for waiting tools
230+
const promiseEntry = this.initPromises.get(workspaceId);
231+
if (promiseEntry) {
232+
promiseEntry.resolve();
233+
this.initPromises.delete(workspaceId);
234+
}
235+
200236
// Keep state in memory for replay (unlike streams which delete immediately)
201237
}
202238

@@ -244,8 +280,83 @@ export class InitStateManager extends EventEmitter {
244280
* Clear in-memory state for a workspace.
245281
* Useful for testing or cleanup after workspace deletion.
246282
* Does NOT delete disk file (use deleteInitStatus for that).
283+
*
284+
* Also cancels any running init promises to prevent orphaned waiters.
247285
*/
248286
clearInMemoryState(workspaceId: string): void {
249287
this.store.deleteState(workspaceId);
288+
289+
// Cancel any running init promise for this workspace
290+
const promiseEntry = this.initPromises.get(workspaceId);
291+
if (promiseEntry) {
292+
promiseEntry.reject(new Error(`Workspace ${workspaceId} was deleted`));
293+
this.initPromises.delete(workspaceId);
294+
}
295+
}
296+
297+
/**
298+
* Wait for workspace initialization to complete.
299+
* Used by tools (bash, file_*) to ensure files are ready before executing.
300+
*
301+
* Behavior:
302+
* - No init state: Returns immediately (init not needed or backwards compat)
303+
* - Init succeeded/failed: Returns immediately (tools proceed regardless of init outcome)
304+
* - Init running: Waits for completion promise (up to 5 minutes, then proceeds anyway)
305+
*
306+
* This method NEVER throws - tools should always proceed. If init fails or times out,
307+
* the tool will either succeed (if init wasn't critical) or fail with its own error
308+
* (e.g., file not found). This provides better error messages than blocking on init.
309+
*
310+
* Promise-based approach eliminates race conditions:
311+
* - Multiple tools share the same promise (no duplicate listeners)
312+
* - No event cleanup needed (promise auto-resolves once)
313+
* - Timeout races handled by Promise.race()
314+
*
315+
* @param workspaceId Workspace ID to wait for
316+
*/
317+
async waitForInit(workspaceId: string): Promise<void> {
318+
const state = this.getInitState(workspaceId);
319+
320+
// No init state - proceed immediately (backwards compat or init not needed)
321+
if (!state) {
322+
return;
323+
}
324+
325+
// Init already completed (success or failure) - proceed immediately
326+
// Tools should work regardless of init outcome
327+
if (state.status !== "running") {
328+
return;
329+
}
330+
331+
// Init is running - wait for completion promise with timeout
332+
const promiseEntry = this.initPromises.get(workspaceId);
333+
334+
if (!promiseEntry) {
335+
// State says running but no promise exists (shouldn't happen, but handle gracefully)
336+
log.error(`Init state is running for ${workspaceId} but no promise found, proceeding`);
337+
return;
338+
}
339+
340+
const INIT_TIMEOUT_MS = 5 * 60 * 1000; // 5 minutes
341+
const timeoutPromise = new Promise<void>((resolve) => {
342+
setTimeout(() => {
343+
log.error(
344+
`Init timeout for ${workspaceId} after 5 minutes - tools will proceed anyway. ` +
345+
`Init will continue in background.`
346+
);
347+
resolve(); // Resolve, don't reject - tools should proceed
348+
}, INIT_TIMEOUT_MS);
349+
});
350+
351+
// Race between completion and timeout
352+
// Both resolve (no rejection), so tools always proceed
353+
try {
354+
await Promise.race([promiseEntry.promise, timeoutPromise]);
355+
} catch (error) {
356+
// Init promise was rejected (e.g., workspace deleted)
357+
// Log and proceed anyway - let the tool fail with its own error if needed
358+
const errorMsg = error instanceof Error ? error.message : String(error);
359+
log.error(`Init wait interrupted for ${workspaceId}: ${errorMsg} - proceeding anyway`);
360+
}
250361
}
251362
}

src/services/ipcMain.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,9 +128,14 @@ export class IpcMain {
128128
this.config = config;
129129
this.historyService = new HistoryService(config);
130130
this.partialService = new PartialService(config, this.historyService);
131-
this.aiService = new AIService(config, this.historyService, this.partialService);
132-
this.bashService = new BashExecutionService();
133131
this.initStateManager = new InitStateManager(config);
132+
this.aiService = new AIService(
133+
config,
134+
this.historyService,
135+
this.partialService,
136+
this.initStateManager
137+
);
138+
this.bashService = new BashExecutionService();
134139
}
135140

136141
private getOrCreateSession(workspaceId: string): AgentSession {
@@ -917,6 +922,7 @@ export class IpcMain {
917922

918923
// Create bash tool with workspace's cwd and secrets
919924
// All IPC bash calls are from UI (background operations) - use truncate to avoid temp file spam
925+
// No init wait needed - IPC calls are user-initiated, not AI tool use
920926
const bashTool = createBashTool({
921927
cwd: workspacePath, // Bash executes in the workspace directory
922928
runtime,

0 commit comments

Comments
 (0)