diff --git a/docs/dashboard-guide.md b/docs/dashboard-guide.md index 34667f283b..2901300749 100644 --- a/docs/dashboard-guide.md +++ b/docs/dashboard-guide.md @@ -629,6 +629,7 @@ Inspect task definition, logs, review feedback, comments, documents, workflow ou - In `in-review`, pull-request controls/status (including stall badges) are in a dedicated **Pull Request** tab instead of the Definition tab. - The **Create Pull Request** modal now offers in-app remediation for every blocking preflight check. If `branchOnRemote` is false, use **Push branch to remote** and Fusion will publish `fusion/` to `origin` and refresh preflight. If `conflictsWithBase` is true, use **Resolve conflicts with AI** and Fusion will use an AI coding agent to resolve merge markers on the task branch, commit the result, push the branch, and refresh preflight so normal PR creation can continue once all checks pass. - The modal shell renders immediately: preflight checks and PR options load independently of AI-generated title/body metadata, so slow AI suggestions no longer block base-branch selection, diagnostics, or manual PR authoring. +- AI title/body generation is bounded to 60 seconds and is canceled if the dialog request disconnects; on timeout/cancel, Fusion falls back to deterministic task-based PR title/body content instead of leaving the spinner stuck forever. - The **Review** tab is separate from **Comments**: Review shows actionable PR/reviewer feedback and same-task revision controls, while Comments remains the general collaboration thread. - **Request revision** in Review resumes work on the same task ID (no refinement task): `in-progress` tasks get steering injection, while `in-review` tasks are moved back to `in-progress` for the same branch/worktree revision pass. - Review supports a manual **Refresh** action in-place: PR mode pulls latest GitHub review state/decision, while direct mode rehydrates reviewer-agent feedback from persisted task data (no GitHub call). diff --git a/packages/dashboard/src/__tests__/pr-metadata-generator.test.ts b/packages/dashboard/src/__tests__/pr-metadata-generator.test.ts index 500cc5dabe..f425d95a86 100644 --- a/packages/dashboard/src/__tests__/pr-metadata-generator.test.ts +++ b/packages/dashboard/src/__tests__/pr-metadata-generator.test.ts @@ -4,21 +4,26 @@ import { join } from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import type { Task } from "@fusion/core"; -const { execMock, promptMock } = vi.hoisted(() => ({ +const { execMock, promptMock, disposeMock } = vi.hoisted(() => ({ execMock: vi.fn(), promptMock: vi.fn(), + disposeMock: vi.fn(), })); -vi.mock("node:child_process", () => ({ - exec: execMock, -})); +vi.mock("node:child_process", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + exec: execMock, + }; +}); vi.mock("@fusion/engine", () => ({ listCliAdapterDescriptors: () => [], createFnAgent: vi.fn(async () => ({ session: { prompt: promptMock, - dispose: vi.fn(), + dispose: disposeMock, }, })), })); @@ -59,6 +64,8 @@ describe("generatePrMetadata", () => { repoRoot = mkdtempSync(join(tmpdir(), "pr-metadata-")); mkdirSync(join(repoRoot, ".fusion", "tasks", "FN-4991"), { recursive: true }); writeFileSync(join(repoRoot, ".fusion", "tasks", "FN-4991", "PROMPT.md"), "# Prompt"); + promptMock.mockResolvedValue(undefined); + disposeMock.mockReset(); vi.mocked(createFnAgent).mockImplementation(async ({ onText }: { onText?: (t: string) => void }) => { onText?.(JSON.stringify({ title: "feat: add routes", @@ -70,7 +77,7 @@ describe("generatePrMetadata", () => { return { session: { prompt: promptMock, - dispose: vi.fn(), + dispose: disposeMock, }, } as never; }); @@ -82,6 +89,7 @@ describe("generatePrMetadata", () => { }); afterEach(() => { + vi.useRealTimers(); vi.clearAllMocks(); }); @@ -98,6 +106,7 @@ describe("generatePrMetadata", () => { expect(result.body).toContain("## Testing"); expect(result.body).toContain("## Linked Task"); expect(result.templateUsed).toBe(false); + expect(promptMock).toHaveBeenCalledWith(expect.any(String), expect.objectContaining({ signal: expect.any(AbortSignal) })); }); it("fills known sections when template exists and preserves unknown headings", async () => { @@ -125,7 +134,7 @@ describe("generatePrMetadata", () => { return { session: { prompt: promptMock, - dispose: vi.fn(), + dispose: disposeMock, }, } as never; }); @@ -142,4 +151,74 @@ describe("generatePrMetadata", () => { templateUsed: false, }); }); + + it("returns fallback when prompt times out", async () => { + vi.useFakeTimers(); + promptMock.mockImplementation(() => new Promise(() => undefined)); + + const resultPromise = generatePrMetadata({ + task: createTask(), + repoRoot, + settings: {} as never, + timeoutMs: 25, + }); + + await vi.advanceTimersByTimeAsync(25); + + await expect(resultPromise).resolves.toEqual({ + title: "Route contracts", + body: expect.stringContaining("Closes FN-4991"), + templateUsed: false, + }); + expect(disposeMock).toHaveBeenCalledTimes(1); + }); + + it("returns fallback immediately when caller signal is already aborted", async () => { + const controller = new AbortController(); + controller.abort(); + + const result = await generatePrMetadata({ + task: createTask(), + repoRoot, + settings: {} as never, + signal: controller.signal, + }); + + expect(result).toEqual({ + title: "Route contracts", + body: expect.stringContaining("Closes FN-4991"), + templateUsed: false, + }); + expect(createFnAgent).not.toHaveBeenCalled(); + expect(promptMock).not.toHaveBeenCalled(); + }); + + it("returns fallback and disposes the session when caller aborts mid-generation", async () => { + const controller = new AbortController(); + let promptStarted!: () => void; + const promptStartedPromise = new Promise((resolve) => { + promptStarted = resolve; + }); + promptMock.mockImplementation(() => new Promise(() => { + promptStarted(); + })); + + const resultPromise = generatePrMetadata({ + task: createTask(), + repoRoot, + settings: {} as never, + signal: controller.signal, + }); + + await promptStartedPromise; + controller.abort(); + + await expect(resultPromise).resolves.toEqual({ + title: "Route contracts", + body: expect.stringContaining("Closes FN-4991"), + templateUsed: false, + }); + expect(promptMock).toHaveBeenCalledWith(expect.any(String), expect.objectContaining({ signal: expect.any(AbortSignal) })); + expect(disposeMock).toHaveBeenCalledTimes(1); + }); }); diff --git a/packages/dashboard/src/__tests__/register-git-github.pr-options-preflight-metadata.test.ts b/packages/dashboard/src/__tests__/register-git-github.pr-options-preflight-metadata.test.ts index 682b849f3a..6de406ed01 100644 --- a/packages/dashboard/src/__tests__/register-git-github.pr-options-preflight-metadata.test.ts +++ b/packages/dashboard/src/__tests__/register-git-github.pr-options-preflight-metadata.test.ts @@ -134,13 +134,17 @@ describe("PR metadata/preflight/options routes", () => { } }); - it("POST /pr/generate-metadata returns generated metadata", async () => { + it("POST /pr/generate-metadata returns generated metadata and forwards an abort signal", async () => { const app = createServer(createStore(createTask())); const response = await performRequest(app, "POST", "/api/tasks/FN-001/pr/generate-metadata", "{}", { "content-type": "application/json" }); expect(response.status).toBe(200); expect(response.body).toEqual({ title: "Generated title", body: "Generated body", templateUsed: true }); - expect(mockGeneratePrMetadata).toHaveBeenCalledWith(expect.objectContaining({ task: expect.objectContaining({ id: "FN-001" }), repoRoot: "/tmp/project" })); + expect(mockGeneratePrMetadata).toHaveBeenCalledWith(expect.objectContaining({ + task: expect.objectContaining({ id: "FN-001" }), + repoRoot: "/tmp/project", + signal: expect.any(AbortSignal), + })); }); it("POST /pr/generate-metadata returns 404 for missing task", async () => { diff --git a/packages/dashboard/src/pr-metadata-generator.ts b/packages/dashboard/src/pr-metadata-generator.ts index af6fd7614d..b7e1a08720 100644 --- a/packages/dashboard/src/pr-metadata-generator.ts +++ b/packages/dashboard/src/pr-metadata-generator.ts @@ -7,6 +7,7 @@ import { resolveTaskPlanningModel } from "@fusion/core"; import { createFnAgent } from "@fusion/engine"; const execAsync = promisify(execCb); +const PR_METADATA_TIMEOUT_MS = 60_000; export interface GeneratedPrMetadata { title: string; @@ -134,22 +135,58 @@ function fillTemplate(template: string, result: AiMetadataResult, taskId: string return out.join("\n"); } -async function runCommand(command: string, cwd: string): Promise { +async function runCommand(command: string, cwd: string, signal?: AbortSignal): Promise { const { stdout } = await execAsync(command, { cwd, timeout: 15_000, maxBuffer: 10 * 1024 * 1024, + signal, }); return stdout.trim(); } -async function resolveBaseBranch(task: Task, repoRoot: string): Promise { +function createAbortError(): Error { + const error = new Error("PR metadata generation aborted"); + error.name = "AbortError"; + return error; +} + +function throwIfAborted(signal?: AbortSignal): void { + if (!signal?.aborted) { + return; + } + throw signal.reason instanceof Error ? signal.reason : createAbortError(); +} + +function waitForAbort(signal: AbortSignal): Promise { + return new Promise((_, reject) => { + if (signal.aborted) { + reject(signal.reason instanceof Error ? signal.reason : createAbortError()); + return; + } + signal.addEventListener( + "abort", + () => reject(signal.reason instanceof Error ? signal.reason : createAbortError()), + { once: true }, + ); + }); +} + +function isAbortLikeError(error: unknown): boolean { + return Boolean( + error + && typeof error === "object" + && ("name" in error ? (error as { name?: unknown }).name === "AbortError" : false), + ); +} + +async function resolveBaseBranch(task: Task, repoRoot: string, signal?: AbortSignal): Promise { if (task.prInfo?.baseBranch) { return task.prInfo.baseBranch; } try { - const stdout = await runCommand("gh repo view --json defaultBranchRef -q .defaultBranchRef.name", repoRoot); + const stdout = await runCommand("gh repo view --json defaultBranchRef -q .defaultBranchRef.name", repoRoot, signal); if (stdout) return stdout; } catch { // fallback below @@ -162,81 +199,109 @@ export async function generatePrMetadata(input: { repoRoot: string; settings: ProjectSettings & GlobalSettings; signal?: AbortSignal; + timeoutMs?: number; }): Promise { - const { task, repoRoot, settings, signal } = input; + const { task, repoRoot, settings, signal, timeoutMs = PR_METADATA_TIMEOUT_MS } = input; const fallback = buildFallback(task); + const controller = new AbortController(); + const abortFromCaller = () => controller.abort(signal?.reason instanceof Error ? signal.reason : createAbortError()); + const timeoutId = setTimeout(() => controller.abort(createAbortError()), timeoutMs); + const combinedSignal = controller.signal; + const callerSignalWasActive = Boolean(signal && !signal.aborted); - const baseBranch = await resolveBaseBranch(task, repoRoot); - const [logOut, diffStatOut] = await Promise.all([ - runCommand(`git log --no-merges ${baseBranch}..HEAD --format=%s%n%b`, repoRoot).catch(() => ""), - runCommand(`git diff --stat ${baseBranch}..HEAD`, repoRoot).catch(() => ""), - ]); - - let promptContent = ""; - try { - const promptPath = join(repoRoot, ".fusion", "tasks", task.id, "PROMPT.md"); - promptContent = (await readFile(promptPath, "utf8")).trim(); - } catch { - promptContent = ""; + if (signal) { + if (signal.aborted) { + abortFromCaller(); + } else { + signal.addEventListener("abort", abortFromCaller, { once: true }); + } } - const templatePath = join(repoRoot, ".github", "pull_request_template.md"); - const templateExists = await access(templatePath).then(() => true).catch(() => false); - const template = templateExists ? await readFile(templatePath, "utf8") : ""; - - const model = resolveTaskPlanningModel(task, settings as Partial); - let aiText = ""; - const { session } = await createFnAgent({ - cwd: repoRoot, - tools: "readonly", - defaultProvider: model.provider, - defaultModelId: model.modelId, - systemPrompt: [ - "Generate GitHub PR metadata.", - "Respond with strict JSON only.", - "Schema: {title, summary, changes, testing, linkedTask}", - ].join("\n"), - onText: (delta: string) => { - aiText += delta; - }, - }); - try { - const contextPrompt = [ - `Task ID: ${task.id}`, - `Task title: ${task.title}`, - `Task description: ${task.description ?? ""}`, - `Base branch: ${baseBranch}`, - "Commit log:", - logOut || "(none)", - "Diff stat:", - diffStatOut || "(none)", - "Task prompt:", - promptContent || "(none)", - ].join("\n\n"); + throwIfAborted(combinedSignal); - if (signal?.aborted) { - throw new Error("Metadata generation aborted"); - } + const baseBranch = await resolveBaseBranch(task, repoRoot, combinedSignal); + const [logOut, diffStatOut] = await Promise.all([ + runCommand(`git log --no-merges ${baseBranch}..HEAD --format=%s%n%b`, repoRoot, combinedSignal).catch(() => ""), + runCommand(`git diff --stat ${baseBranch}..HEAD`, repoRoot, combinedSignal).catch(() => ""), + ]); - await session.prompt(contextPrompt); - } finally { + let promptContent = ""; try { - session.dispose(); + const promptPath = join(repoRoot, ".fusion", "tasks", task.id, "PROMPT.md"); + promptContent = (await readFile(promptPath, "utf8")).trim(); } catch { - // best effort + promptContent = ""; + } + + const templatePath = join(repoRoot, ".github", "pull_request_template.md"); + const templateExists = await access(templatePath).then(() => true).catch(() => false); + const template = templateExists ? await readFile(templatePath, "utf8") : ""; + + const model = resolveTaskPlanningModel(task, settings as Partial); + let aiText = ""; + const { session } = await createFnAgent({ + cwd: repoRoot, + tools: "readonly", + defaultProvider: model.provider, + defaultModelId: model.modelId, + systemPrompt: [ + "Generate GitHub PR metadata.", + "Respond with strict JSON only.", + "Schema: {title, summary, changes, testing, linkedTask}", + ].join("\n"), + onText: (delta: string) => { + aiText += delta; + }, + }); + + try { + const contextPrompt = [ + `Task ID: ${task.id}`, + `Task title: ${task.title}`, + `Task description: ${task.description ?? ""}`, + `Base branch: ${baseBranch}`, + "Commit log:", + logOut || "(none)", + "Diff stat:", + diffStatOut || "(none)", + "Task prompt:", + promptContent || "(none)", + ].join("\n\n"); + + throwIfAborted(combinedSignal); + await Promise.race([ + (session.prompt as (prompt: string, options?: { signal?: AbortSignal }) => Promise)(contextPrompt, { signal: combinedSignal }), + waitForAbort(combinedSignal), + ]); + } finally { + try { + session.dispose(); + } catch { + // best effort + } + } + + const parsed = parseAiResult(aiText); + if (!parsed) { + return fallback; + } + + const body = templateExists ? fillTemplate(template, parsed, task.id) : buildBody(parsed, task.id); + return { + title: parsed.title, + body, + templateUsed: templateExists, + }; + } catch (error) { + if (isAbortLikeError(error)) { + return fallback; + } + throw error; + } finally { + clearTimeout(timeoutId); + if (callerSignalWasActive) { + signal?.removeEventListener("abort", abortFromCaller); } } - - const parsed = parseAiResult(aiText); - if (!parsed) { - return fallback; - } - - const body = templateExists ? fillTemplate(template, parsed, task.id) : buildBody(parsed, task.id); - return { - title: parsed.title, - body, - templateUsed: templateExists, - }; } diff --git a/packages/dashboard/src/routes/register-git-github.ts b/packages/dashboard/src/routes/register-git-github.ts index b638ea3a1a..a42f74fec8 100644 --- a/packages/dashboard/src/routes/register-git-github.ts +++ b/packages/dashboard/src/routes/register-git-github.ts @@ -4849,6 +4849,10 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void { * Returns: { title, body, templateUsed } */ router.post("/tasks/:id/pr/generate-metadata", async (req, res) => { + const controller = new AbortController(); + const abortRequest = () => controller.abort(); + req.on("close", abortRequest); + try { const { store: scopedStore } = await getProjectContext(req); const task = await scopedStore.getTask(req.params.id); @@ -4857,7 +4861,11 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void { task, repoRoot: scopedStore.getRootDir(), settings, + signal: controller.signal, }); + if (req.destroyed || res.writableEnded || res.writableFinished || controller.signal.aborted) { + return; + } res.json(metadata); } catch (err: unknown) { if (err instanceof ApiError) { @@ -4867,6 +4875,8 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void { throw notFound(`Task ${req.params.id} not found`); } rethrowAsApiError(err, "Failed to generate PR metadata"); + } finally { + req.off("close", abortRequest); } });