Skip to content

Commit f6d130f

Browse files
committed
🤖 fix: register foreground task promise before startTask
Address Codex review comment: foreground tasks now register their pending completion promise BEFORE calling startTask, ensuring that if startTask fails early (e.g., sendMessage returns an error), handleTaskFailure can find and reject the promise, preventing the parent stream from hanging indefinitely. Added test case to verify this behavior. Change-Id: Ib83798e7f7602edbe291686c8936519c8703cb6a Signed-off-by: Thomas Kosiewski <tk@coder.com>
1 parent 1fd25d0 commit f6d130f

File tree

2 files changed

+62
-4
lines changed

2 files changed

+62
-4
lines changed

src/node/services/taskService.test.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -406,6 +406,56 @@ describe("TaskService", () => {
406406
).rejects.toThrow(/Maximum task nesting depth/);
407407
});
408408

409+
it("rejects foreground task promise when startTask fails early", async () => {
410+
// This test verifies that foreground tasks properly reject their promise
411+
// when sendMessage fails, instead of leaving the parent stream hanging.
412+
const config = new FakeConfig();
413+
const parent = createWorkspaceMetadata("parent");
414+
config.addWorkspace(parent);
415+
416+
const aiService = new FakeAIService();
417+
aiService.setWorkspaceMetadata(parent);
418+
419+
const workspaceService = new FakeWorkspaceService(config, aiService);
420+
const historyService = new FakeHistoryService();
421+
const partialService = new FakePartialService();
422+
423+
// Make sendMessage fail
424+
workspaceService.sendMessageResult = Err({
425+
type: "unknown",
426+
raw: "simulated sendMessage failure",
427+
});
428+
429+
const taskService = new TaskService(
430+
config as unknown as Config,
431+
workspaceService as unknown as WorkspaceService,
432+
historyService as unknown as HistoryService,
433+
partialService as unknown as PartialService,
434+
aiService as unknown as AIService
435+
);
436+
437+
// Foreground task (runInBackground: false) should reject when startTask fails
438+
let error: Error | null = null;
439+
try {
440+
await taskService.createTask({
441+
parentWorkspaceId: "parent",
442+
agentType: "research",
443+
prompt: "will fail",
444+
runInBackground: false,
445+
parentToolCallId: "tool_call_1",
446+
});
447+
} catch (e) {
448+
error = e as Error;
449+
}
450+
451+
expect(error).not.toBeNull();
452+
expect(error?.message).toBe("simulated sendMessage failure");
453+
454+
// Verify task state is set to failed
455+
const taskState = config.getWorkspaceTaskState("task_1");
456+
expect(taskState?.taskStatus).toBe("failed");
457+
});
458+
409459
it("queues tasks when maxParallelAgentTasks reached and inherits parent runtime", async () => {
410460
const config = new FakeConfig();
411461
config.setTaskSettings({ maxParallelAgentTasks: 1 });

src/node/services/taskService.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,16 @@ export class TaskService extends EventEmitter {
234234

235235
this.emit("task-created", { taskId, parentWorkspaceId });
236236

237+
// For foreground tasks, register the pending completion BEFORE starting the task.
238+
// This ensures that if startTask fails synchronously, handleTaskFailure can find
239+
// and reject the promise, preventing the parent stream from hanging indefinitely.
240+
let completionPromise: Promise<CreateTaskResult> | undefined;
241+
if (!runInBackground) {
242+
completionPromise = new Promise<CreateTaskResult>((resolve, reject) => {
243+
this.pendingCompletions.set(taskId, { resolve, reject });
244+
});
245+
}
246+
237247
if (shouldQueue) {
238248
// Add to queue
239249
this.taskQueue.push({ taskId, options });
@@ -252,10 +262,8 @@ export class TaskService extends EventEmitter {
252262
};
253263
}
254264

255-
// Wait for completion
256-
return new Promise<CreateTaskResult>((resolve, reject) => {
257-
this.pendingCompletions.set(taskId, { resolve, reject });
258-
});
265+
// Wait for completion (promise was registered before startTask to handle early failures)
266+
return completionPromise!;
259267
}
260268

261269
/**

0 commit comments

Comments
 (0)