From ac92174cba6c3f98b36669c7d833e051fc8923de Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Wed, 3 Jun 2026 07:50:59 -0700 Subject: [PATCH] FN-5907: harden planning task creation side effects Keep Planning Mode task creation responses successful when follow-up work fails. - catch and log planning create-task/create-tasks side-effect failures so the API can still return 201 after task creation succeeds - safely handle async task lifecycle listener failures in the core store to avoid leaking unhandled rejections during follow-up updates - add regression coverage for single-task and multi-task Planning Mode creation across live/persisted sessions, branch selection surfaces, and GitHub tracking failures Files changed: .changeset/fn-5907-planning-create-fetch.md | 5 + .../core/src/__tests__/task-creation-hook.test.ts | 25 ++ packages/core/src/store.ts | 38 ++- .../src/__tests__/routes-planning-tracking.test.ts | 108 ++++++++- .../src/__tests__/routes-planning.test.ts | 267 +++++++++++++++++++++ .../src/routes/register-planning-subtask-routes.ts | 69 +++++- 6 files changed, 496 insertions(+), 16 deletions(-) Fusion-Task-Id: FN-5907 Fusion-Task-Lineage: d9f6574b-e19b-4459-a612-7a3d2510836f --- .changeset/fn-5907-planning-create-fetch.md | 5 + .../src/__tests__/task-creation-hook.test.ts | 25 ++ packages/core/src/store.ts | 38 ++- .../routes-planning-tracking.test.ts | 108 ++++++- .../src/__tests__/routes-planning.test.ts | 267 ++++++++++++++++++ .../register-planning-subtask-routes.ts | 69 ++++- 6 files changed, 496 insertions(+), 16 deletions(-) create mode 100644 .changeset/fn-5907-planning-create-fetch.md diff --git a/.changeset/fn-5907-planning-create-fetch.md b/.changeset/fn-5907-planning-create-fetch.md new file mode 100644 index 0000000000..03a08faecd --- /dev/null +++ b/.changeset/fn-5907-planning-create-fetch.md @@ -0,0 +1,5 @@ +--- +"@runfusion/fusion": patch +--- + +Fix a Planning Mode reliability bug where creating a single task could fail with a browser-level `Failed to fetch` error when post-create side effects threw or rejected before the dashboard finished responding. diff --git a/packages/core/src/__tests__/task-creation-hook.test.ts b/packages/core/src/__tests__/task-creation-hook.test.ts index 24c7c17178..4f0668a325 100644 --- a/packages/core/src/__tests__/task-creation-hook.test.ts +++ b/packages/core/src/__tests__/task-creation-hook.test.ts @@ -108,6 +108,31 @@ describe("task creation hook", () => { expect(created2.id).toMatch(/^FN-/); }); + it("does not leak async task:updated listener rejections during create follow-up updates", async () => { + const store = harness.store(); + const unhandledRejections: unknown[] = []; + const onUnhandledRejection = (reason: unknown) => { + unhandledRejections.push(reason); + }; + process.on("unhandledRejection", onUnhandledRejection); + + store.on("task:updated", async (task) => { + if (task.id.startsWith("FN-")) { + throw new Error(`listener boom for ${task.id}`); + } + }); + + try { + const task = await store.createTask({ description: "planning create listener safety" }); + await store.updateTask(task.id, { size: "M" }); + await store.logEntry(task.id, "Created via Planning Mode", "Initial plan: test"); + await new Promise((resolve) => setImmediate(resolve)); + expect(unhandledRejections).toHaveLength(0); + } finally { + process.off("unhandledRejection", onUnhandledRejection); + } + }); + it("can clear hook with undefined", async () => { const store = harness.store(); const hook = vi.fn(); diff --git a/packages/core/src/store.ts b/packages/core/src/store.ts index f5684fbc5c..aa291da4bf 100644 --- a/packages/core/src/store.ts +++ b/packages/core/src/store.ts @@ -1224,6 +1224,34 @@ export class TaskStore extends EventEmitter { this.globalSettingsStore = new GlobalSettingsStore(resolvedGlobalSettingsDir); } + private emitTaskLifecycleEventSafely( + event: "task:created" | "task:updated", + args: TaskStoreEvents["task:created"] | TaskStoreEvents["task:updated"], + ): boolean { + const listeners = super.listeners(event) as Array<(...listenerArgs: typeof args) => unknown>; + if (listeners.length === 0) { + return false; + } + + const [task] = args; + const taskId = task && typeof task === "object" && "id" in task ? String(task.id) : "unknown"; + + for (const listener of listeners) { + try { + const result = listener(...args); + if (result && typeof (result as PromiseLike).then === "function") { + void Promise.resolve(result).catch((error) => { + storeLog.warn(`[${event}] listener failed for ${taskId}: ${getErrorMessage(error)}`); + }); + } + } catch (error) { + storeLog.warn(`[${event}] listener failed for ${taskId}: ${getErrorMessage(error)}`); + } + } + + return true; + } + /** * Get the SQLite database, initializing it on first access. * Also performs auto-migration from legacy file-based storage if needed. @@ -4001,7 +4029,7 @@ export class TaskStore extends EventEmitter { await this._maybeAutoArchiveSameAgentDuplicate(task, input); - this.emit("task:created", task); + this.emitTaskLifecycleEventSafely("task:created", [task]); if (options?.invokeTaskCreatedHook !== false) { await this.invokeTaskCreatedHook(task); } @@ -5850,7 +5878,7 @@ export class TaskStore extends EventEmitter { if (movedToTriage) { this.emit("task:moved", { task, from: "todo" as Column, to: "triage" as Column, source: "engine" }); } - this.emit("task:updated", task); + this.emitTaskLifecycleEventSafely("task:updated", [task]); return task; }); } @@ -6521,7 +6549,7 @@ export class TaskStore extends EventEmitter { if (movedToTriage) { this.emit("task:moved", { task, from: "todo" as Column, to: "triage" as Column, source: "engine" }); } - this.emit("task:updated", task); + this.emitTaskLifecycleEventSafely("task:updated", [task]); return task; }); } @@ -6772,12 +6800,12 @@ export class TaskStore extends EventEmitter { if (this.isWatching) { this.taskCache.set(id, { ...current }); } - this.emit("task:updated", current); + this.emitTaskLifecycleEventSafely("task:updated", [current]); return current; } const emittedTask = ({ id, log, updatedAt } as unknown) as Task; - this.emit("task:updated", emittedTask); + this.emitTaskLifecycleEventSafely("task:updated", [emittedTask]); return emittedTask; }); } diff --git a/packages/dashboard/src/__tests__/routes-planning-tracking.test.ts b/packages/dashboard/src/__tests__/routes-planning-tracking.test.ts index c0e7d2b9c7..27971f6416 100644 --- a/packages/dashboard/src/__tests__/routes-planning-tracking.test.ts +++ b/packages/dashboard/src/__tests__/routes-planning-tracking.test.ts @@ -200,7 +200,39 @@ describe("planning routes github tracking background dispatch", () => { expect(response.status).toBe(201); await vi.waitFor(() => { - expect(planningWarn).toHaveBeenCalled(); + expect(planningWarn).toHaveBeenCalledWith(expect.stringContaining("[github-tracking] Failed to create issue")); + }); + }); + + it("POST /planning/create-task still returns 201 when createIssue throws synchronously", async () => { + createIssueSpy.mockImplementation(() => { + throw new Error("sync github crash"); + }); + + sessions.set("plan-2-sync", { + summary: { + title: "Planned task 2", + description: "Planned task description 2", + suggestedSize: "M", + priority: "normal", + suggestedDependencies: [], + keyDeliverables: [], + }, + initialPlan: "initial", + history: [], + }); + + const response = await performRequest( + app, + "POST", + "/planning/create-task", + JSON.stringify({ sessionId: "plan-2-sync" }), + { "content-type": "application/json" }, + ); + + expect(response.status).toBe(201); + await vi.waitFor(() => { + expect(planningWarn).toHaveBeenCalledWith(expect.stringContaining("[github-tracking] Failed to create issue")); }); }); @@ -250,4 +282,78 @@ describe("planning routes github tracking background dispatch", () => { expect(createIssueSpy).toHaveBeenCalledTimes(2); }); }); + + it("POST /planning/create-tasks still returns 201 when createIssue rejects asynchronously", async () => { + createIssueSpy.mockRejectedValue(new Error("github down")); + + sessions.set("plan-3-reject", { + summary: { + title: "Plan", + description: "Plan", + suggestedSize: "M", + priority: "normal", + suggestedDependencies: [], + keyDeliverables: [], + }, + initialPlan: "initial", + history: [], + }); + + const response = await performRequest( + app, + "POST", + "/planning/create-tasks", + JSON.stringify({ + planningSessionId: "plan-3-reject", + subtasks: [ + { id: "tmp-1", title: "Subtask 1", description: "D1" }, + { id: "tmp-2", title: "Subtask 2", description: "D2" }, + ], + }), + { "content-type": "application/json" }, + ); + + expect(response.status).toBe(201); + await vi.waitFor(() => { + expect(planningWarn).toHaveBeenCalledWith(expect.stringContaining("[github-tracking] Failed to create issue")); + }); + }); + + it("POST /planning/create-tasks still returns 201 when createIssue throws synchronously", async () => { + createIssueSpy.mockImplementation(() => { + throw new Error("sync github crash"); + }); + + sessions.set("plan-3-sync", { + summary: { + title: "Plan", + description: "Plan", + suggestedSize: "M", + priority: "normal", + suggestedDependencies: [], + keyDeliverables: [], + }, + initialPlan: "initial", + history: [], + }); + + const response = await performRequest( + app, + "POST", + "/planning/create-tasks", + JSON.stringify({ + planningSessionId: "plan-3-sync", + subtasks: [ + { id: "tmp-1", title: "Subtask 1", description: "D1" }, + { id: "tmp-2", title: "Subtask 2", description: "D2" }, + ], + }), + { "content-type": "application/json" }, + ); + + expect(response.status).toBe(201); + await vi.waitFor(() => { + expect(planningWarn).toHaveBeenCalledWith(expect.stringContaining("[github-tracking] Failed to create issue")); + }); + }); }); diff --git a/packages/dashboard/src/__tests__/routes-planning.test.ts b/packages/dashboard/src/__tests__/routes-planning.test.ts index 055ff8644b..0222a6cb75 100644 --- a/packages/dashboard/src/__tests__/routes-planning.test.ts +++ b/packages/dashboard/src/__tests__/routes-planning.test.ts @@ -421,6 +421,23 @@ describe("Planning Mode Routes", () => { return app; } + async function createCompletedPlanningSession(initialPlan = "Build a user auth system"): Promise { + const startRes = await REQUEST( + buildApp(), + "POST", + "/api/planning/start", + JSON.stringify({ initialPlan }), + { "Content-Type": "application/json" } + ); + const sessionId = startRes.body.sessionId; + + await REQUEST(buildApp(), "POST", "/api/planning/respond", JSON.stringify({ sessionId, responses: { scope: "medium" } }), { "Content-Type": "application/json" }); + await REQUEST(buildApp(), "POST", "/api/planning/respond", JSON.stringify({ sessionId, responses: { requirements: "Must have login" } }), { "Content-Type": "application/json" }); + await REQUEST(buildApp(), "POST", "/api/planning/respond", JSON.stringify({ sessionId, responses: { confirm: true } }), { "Content-Type": "application/json" }); + + return sessionId; + } + async function connectPlanningStreamUntilComplete(sessionId: string): Promise { const streamPromise = REQUEST(buildApp(), "GET", `/api/planning/${sessionId}/stream`); setTimeout(() => { @@ -1952,6 +1969,256 @@ describe("Planning Mode Routes", () => { }); }); + it.each([ + { + sessionSource: "live", + useSummaryOverride: false, + branchSelection: { mode: "project-default" }, + expectedBranch: undefined, + expectedBaseBranch: undefined, + }, + { + sessionSource: "live", + useSummaryOverride: true, + branchSelection: { mode: "auto-new", baseBranch: "develop" }, + expectedBranch: undefined, + expectedBaseBranch: "develop", + }, + { + sessionSource: "persisted", + useSummaryOverride: false, + branchSelection: { mode: "existing", branchName: "feature/shared-auth", baseBranch: "develop" }, + expectedBranch: "feature/shared-auth", + expectedBaseBranch: "develop", + }, + { + sessionSource: "persisted", + useSummaryOverride: true, + branchSelection: { mode: "custom-new", branchName: "feature/planned-auth", baseBranch: "main" }, + expectedBranch: "feature/planned-auth", + expectedBaseBranch: "main", + }, + ])("returns 201 for $sessionSource create-task sessions across branch selection surfaces (summary override: $useSummaryOverride)", async ({ + sessionSource, + useSummaryOverride, + branchSelection, + expectedBranch, + expectedBaseBranch, + }) => { + (store.createTask as ReturnType).mockResolvedValue({ + id: `FN-${sessionSource}-${branchSelection.mode}`, + description: "Build auth flow", + column: "triage", + dependencies: [], + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + }); + (store.updateTask as ReturnType).mockResolvedValue({}); + (store.logEntry as ReturnType).mockResolvedValue(undefined); + + let app = buildApp(); + let sessionId: string; + + if (sessionSource === "live") { + sessionId = await createCompletedPlanningSession(); + } else { + sessionId = `persisted-${branchSelection.mode}-${useSummaryOverride ? "override" : "default"}`; + const persistedSession = { + id: sessionId, + type: "planning", + status: "complete", + title: "Build persisted planning", + inputPayload: JSON.stringify({ initialPlan: "Build resumable planning sessions" }), + conversationHistory: "[]", + currentQuestion: null, + result: JSON.stringify({ + title: "Persisted planning output", + description: "Persist planning results so users can create tasks later", + suggestedSize: "M", + priority: "normal", + suggestedDependencies: ["FN-100"], + keyDeliverables: ["Persist sessions"], + }), + thinkingOutput: "", + error: null, + projectId: null, + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + archived: 0, + }; + const mockAiSessionStore = { + get: vi.fn((id: string) => (id === sessionId ? persistedSession : null)), + listAll: vi.fn(() => []), + delete: vi.fn(), + }; + app = express(); + app.use(express.json()); + app.use("/api", createApiRoutes(store, { aiSessionStore: mockAiSessionStore as any })); + } + + const summary = useSummaryOverride + ? { + title: "Edited auth task", + description: "Edited description from summary view", + suggestedSize: "S", + suggestedDependencies: ["FN-500"], + keyDeliverables: ["Login flow"], + } + : undefined; + + const res = await REQUEST( + app, + "POST", + "/api/planning/create-task", + JSON.stringify({ sessionId, branchSelection, ...(summary ? { summary } : {}) }), + { "Content-Type": "application/json" } + ); + + expect(res.status).toBe(201); + expect(store.createTask).toHaveBeenCalledWith( + expect.objectContaining({ + title: useSummaryOverride ? "Edited auth task" : expect.any(String), + branch: expectedBranch, + baseBranch: expectedBaseBranch, + }), + ); + }); + + it.each([ + { label: "size update rejection", configure: () => (store.updateTask as ReturnType).mockRejectedValueOnce(new Error("size update failed")) }, + { label: "log entry rejection", configure: () => (store.logEntry as ReturnType).mockRejectedValueOnce(new Error("log entry failed")) }, + ])("still returns 201 when planning create-task post-create side effects fail (%s)", async ({ configure }) => { + (store.createTask as ReturnType).mockResolvedValue({ + id: "FN-250", + description: "Build a user auth system", + column: "triage", + dependencies: [], + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + }); + (store.updateTask as ReturnType).mockResolvedValue({}); + (store.logEntry as ReturnType).mockResolvedValue(undefined); + configure(); + + const sessionId = await createCompletedPlanningSession(); + const res = await REQUEST( + buildApp(), + "POST", + "/api/planning/create-task", + JSON.stringify({ sessionId }), + { "Content-Type": "application/json" } + ); + + expect(res.status).toBe(201); + }); + + it("still returns 201 when planning create-task session release throws", async () => { + (store.createTask as ReturnType).mockResolvedValue({ + id: "FN-251", + description: "Build a user auth system", + column: "triage", + dependencies: [], + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + }); + (store.updateTask as ReturnType).mockResolvedValue({}); + (store.logEntry as ReturnType).mockResolvedValue(undefined); + const releaseSessionSpy = vi.spyOn(planningModule, "releaseSession").mockImplementation(() => { + throw new Error("release exploded"); + }); + + try { + const sessionId = await createCompletedPlanningSession(); + const res = await REQUEST( + buildApp(), + "POST", + "/api/planning/create-task", + JSON.stringify({ sessionId }), + { "Content-Type": "application/json" } + ); + + expect(res.status).toBe(201); + } finally { + releaseSessionSpy.mockRestore(); + } + }); + + it("still returns 201 when planning create-tasks post-create updates fail", async () => { + (store.createTask as ReturnType) + .mockResolvedValueOnce({ + id: "FN-260", + description: "First", + column: "triage", + dependencies: [], + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + }) + .mockResolvedValueOnce({ + id: "FN-261", + description: "Second", + column: "triage", + dependencies: [], + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + }); + (store.updateTask as ReturnType) + .mockRejectedValueOnce(new Error("size update failed")) + .mockResolvedValueOnce({ + id: "FN-261", + description: "Second", + column: "triage", + dependencies: ["FN-260"], + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", + }); + (store.logEntry as ReturnType).mockResolvedValue(undefined); + + const planningSessionId = await createCompletedPlanningSession(); + const breakdownRes = await REQUEST( + buildApp(), + "POST", + "/api/planning/start-breakdown", + JSON.stringify({ sessionId: planningSessionId }), + { "Content-Type": "application/json" } + ); + + const generatedSubtasks = breakdownRes.body.subtasks as Array<{ + id: string; + title: string; + description: string; + suggestedSize: "S" | "M" | "L"; + dependsOn: string[]; + }>; + + const res = await REQUEST( + buildApp(), + "POST", + "/api/planning/create-tasks", + JSON.stringify({ + planningSessionId, + subtasks: [ + { + id: generatedSubtasks[0]!.id, + title: "Auth backend", + description: "Implement backend", + suggestedSize: "L", + dependsOn: [], + }, + { + id: generatedSubtasks[1]!.id, + title: "Auth frontend", + description: "Implement frontend", + dependsOn: [generatedSubtasks[0]!.id], + }, + ], + }), + { "Content-Type": "application/json" } + ); + + expect(res.status).toBe(201); + expect(res.body.tasks).toHaveLength(2); + }); + it("creates task with explicit summary priority", async () => { (store.createTask as ReturnType).mockResolvedValue({ id: "FN-100", diff --git a/packages/dashboard/src/routes/register-planning-subtask-routes.ts b/packages/dashboard/src/routes/register-planning-subtask-routes.ts index 8105cbe6ca..f0ab6518ec 100644 --- a/packages/dashboard/src/routes/register-planning-subtask-routes.ts +++ b/packages/dashboard/src/routes/register-planning-subtask-routes.ts @@ -983,6 +983,25 @@ export function registerPlanningSubtaskRoutes(ctx: ApiRoutesContext, deps: Plann }; }; + const logPlanningCreateWarning = (message: string, error: unknown, metadata?: Record): void => { + planningLogger.warn(message, { + ...metadata, + error: error instanceof Error ? error.message : String(error), + }); + }; + + const runPlanningCreateSideEffect = async ( + message: string, + work: () => Promise | unknown, + metadata?: Record, + ): Promise => { + try { + await work(); + } catch (error) { + logPlanningCreateWarning(message, error, metadata); + } + }; + /** * POST /api/planning/create-task * Create a task from a completed planning session. @@ -1103,18 +1122,30 @@ export function registerPlanningSubtaskRoutes(ctx: ApiRoutesContext, deps: Plann baseBranch: resolvedBaseBranch, }); - // Update task with suggested size if provided + // Update task with suggested size if provided. if (summary.suggestedSize) { - await scopedStore.updateTask(task.id, { size: summary.suggestedSize }); + await runPlanningCreateSideEffect( + "Planning create-task size update failed", + () => scopedStore.updateTask(task.id, { size: summary.suggestedSize }), + { taskId: task.id, sessionId }, + ); } - // Log the planning mode creation - await scopedStore.logEntry(task.id, "Created via Planning Mode", `Initial plan: ${(initialPlan ?? "").slice(0, 200)}`); + // Log the planning mode creation. + await runPlanningCreateSideEffect( + "Planning create-task log entry failed", + () => scopedStore.logEntry(task.id, "Created via Planning Mode", `Initial plan: ${(initialPlan ?? "").slice(0, 200)}`), + { taskId: task.id, sessionId }, + ); // Release any live in-memory planning runtime for this session, but // keep the persisted completed row so planning history can still list // and restore the summary after single-task creation. - releaseSession(sessionId); + await runPlanningCreateSideEffect( + "Planning create-task session release failed", + () => releaseSession(sessionId), + { taskId: task.id, sessionId }, + ); res.status(201).json(task); } catch (err: unknown) { @@ -1314,7 +1345,11 @@ export function registerPlanningSubtaskRoutes(ctx: ApiRoutesContext, deps: Plann createdTasks.push(task); if (item.suggestedSize === "S" || item.suggestedSize === "M" || item.suggestedSize === "L") { - await scopedStore.updateTask(task.id, { size: item.suggestedSize }); + await runPlanningCreateSideEffect( + "Planning create-tasks size update failed", + () => scopedStore.updateTask(task.id, { size: item.suggestedSize }), + { taskId: task.id, planningSessionId }, + ); } } @@ -1326,14 +1361,28 @@ export function registerPlanningSubtaskRoutes(ctx: ApiRoutesContext, deps: Plann : []; if (resolvedDependencies.length > 0) { - const updated = await scopedStore.updateTask(created.id, { dependencies: resolvedDependencies }); - createdTasks[index] = updated; + await runPlanningCreateSideEffect( + "Planning create-tasks dependency update failed", + async () => { + const updated = await scopedStore.updateTask(created.id, { dependencies: resolvedDependencies }); + createdTasks[index] = updated; + }, + { taskId: created.id, planningSessionId }, + ); } - await scopedStore.logEntry(created.id, "Created via Planning Mode (multi-task)", logDetails); + await runPlanningCreateSideEffect( + "Planning create-tasks log entry failed", + () => scopedStore.logEntry(created.id, "Created via Planning Mode (multi-task)", logDetails), + { taskId: created.id, planningSessionId }, + ); } - cleanupSession(planningSessionId); + await runPlanningCreateSideEffect( + "Planning create-tasks session cleanup failed", + () => cleanupSession(planningSessionId), + { planningSessionId }, + ); res.status(201).json({ tasks: createdTasks }); } catch (err: unknown) {