From 01fe47d50fb8d6319371e9145563c60124b8ecee Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sun, 28 Jun 2026 00:15:21 -0700 Subject: [PATCH] FN-7175: fix Create PR dialog recovery and dismissal Stabilize the Create PR dialog so metadata hangs and stray clicks no longer block manual PR creation. - Bound AI metadata generation in the dialog to a 15 second timeout that falls back to manual title/body recovery. - Keep the floating Create PR shell dismissible only via explicit close actions and default the diff preview collapsed. - Expand modal tests and dashboard docs for timeout recovery, collapsed preview, and dismissal behavior. - Add a patch changeset for the published Fusion package. Files changed: .changeset/fn-7175-create-pr-dialog-fixes.md | 7 +++ docs/dashboard-guide.md | 6 +- .../dashboard/app/components/PrCreateModal.tsx | 34 ++++++++++- .../components/__tests__/PrCreateModal.test.tsx | 65 ++++++++++++++++++++-- 4 files changed, 101 insertions(+), 11 deletions(-) Fusion-Task-Id: FN-7175 Fusion-Task-Lineage: 675d2212-54a1-465f-a9bf-6af4ba33674a Co-authored-by: Fusion (runfusion.ai) --- .changeset/fn-7175-create-pr-dialog-fixes.md | 7 ++ docs/dashboard-guide.md | 6 +- .../app/components/PrCreateModal.tsx | 34 +++++++++- .../__tests__/PrCreateModal.test.tsx | 65 +++++++++++++++++-- 4 files changed, 101 insertions(+), 11 deletions(-) create mode 100644 .changeset/fn-7175-create-pr-dialog-fixes.md diff --git a/.changeset/fn-7175-create-pr-dialog-fixes.md b/.changeset/fn-7175-create-pr-dialog-fixes.md new file mode 100644 index 0000000000..fc2bdcc221 --- /dev/null +++ b/.changeset/fn-7175-create-pr-dialog-fixes.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Fix the Create PR dialog spinner, diff preview default, and stray-click dismissal behavior. +category: fix +dev: PrCreateModal keeps the FloatingWindow no-backdrop-dismiss path, defaults the diff/commit
closed, and time-bounds generatePrMetadata with PR_METADATA_TIMEOUT_MS so hangs use the existing error/manual-body fallback. diff --git a/docs/dashboard-guide.md b/docs/dashboard-guide.md index 3b52ac2a84..c20a047f44 100644 --- a/docs/dashboard-guide.md +++ b/docs/dashboard-guide.md @@ -1001,9 +1001,9 @@ Inspect task definition, logs, review feedback, comments, artifacts, workflow ou - Task Detail and list split-pane PR affordances follow the live project auto-merge setting: when auto-merge is off, manual **Create PR** / merge actions are shown; when it is on, the tab shows the automatic auto-merge hint unless a per-task override changes the effective behavior. - The **Workflow** tab resolves the effective workflow for both explicitly selected and default-inherited tasks. Its overview, expandable graph preview, configured step details, and live step results refresh when switching tasks or projects without showing stale rows from the previous task. - 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 and push real merge changes, or report success without an empty commit when the selected base is already merged; preflight then refreshes so normal PR creation can continue once all checks pass. -- The **Create Pull Request** modal is a floating pop-out like Plan Mission, New Task, and Automations: drag its header or resize from desktop edges/corners, while mobile keeps the full-screen dialog layout. Close it with **X**, **Cancel**, or **Escape**. -- 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 **Create Pull Request** modal is a floating pop-out like Plan Mission, New Task, and Automations: drag its header or resize from desktop edges/corners, while mobile keeps the full-screen dialog layout. Close it with **X**, **Cancel**, or **Escape**; stray clicks inside or outside the floating shell do not dismiss it. +- 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. The **Diff & commit preview** section starts collapsed and can be expanded on demand. +- AI title/body generation in the dialog is bounded to 15 seconds and is canceled if the request disconnects; on timeout/cancel, Fusion falls back to deterministic task-based PR title/body content instead of leaving the spinner stuck forever. - The **Artifacts** tab combines task documents written by agents or users with task-scoped registered media artifacts. The gallery uses thumbnail-first image/video cards, image and video previews can expand into a dismissible full-size lightbox, video and audio use native controls, document artifacts show text previews, and generic artifacts open through their media URL. - 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. - Review comments hide GitHub template HTML comments in both Markdown and Plain modes, show author avatars or User/Bot fallbacks, label Human vs Bot/agent authors, and include All/Human/Bot filtering. diff --git a/packages/dashboard/app/components/PrCreateModal.tsx b/packages/dashboard/app/components/PrCreateModal.tsx index 417a5bb22a..5f1b7bce35 100644 --- a/packages/dashboard/app/components/PrCreateModal.tsx +++ b/packages/dashboard/app/components/PrCreateModal.tsx @@ -38,6 +38,27 @@ type PreflightCheck = { warning?: boolean; }; +const PR_METADATA_TIMEOUT_MS = 15000; + +/* +FNXC:PrCreateModal 2026-06-27-23:48: +AI PR metadata generation must never leave the Create PR dialog in a permanent loading state. Bound the call to the same 15s budget as PR view fetches, then route timeout failures through the existing metadata error/manual-body fallback path so users can recover manually. +*/ +async function withPrMetadataTimeout(promise: Promise): Promise { + let timeoutId: ReturnType | undefined; + const timeout = new Promise((_, reject) => { + timeoutId = setTimeout(() => reject(new Error("Timed out generating PR metadata")), PR_METADATA_TIMEOUT_MS); + }); + + try { + return await Promise.race([promise, timeout]); + } finally { + if (timeoutId !== undefined) { + clearTimeout(timeoutId); + } + } +} + /* FNXC:PrCreateModal 2026-06-23-00:00: The Create PR modal must stay manually usable after metadata generation fails, but non-interactive GitHub PR creation cannot accept a title-only payload. Seed an editable body fallback with the required sections so users can complete or revise the PR instead of submitting an empty body. @@ -217,7 +238,7 @@ export function PrCreateModal({ setUserEditedBody(false); } try { - const metadata = await generatePrMetadata(taskId, projectId); + const metadata = await withPrMetadataTimeout(generatePrMetadata(taskId, projectId)); if (requestId !== requestSeqRef.current.metadata) { return; } @@ -360,7 +381,7 @@ export function PrCreateModal({ setMetadataLoading(true); setMetadataError(null); try { - const metadata = await generatePrMetadata(taskId, projectId); + const metadata = await withPrMetadataTimeout(generatePrMetadata(taskId, projectId)); if (requestId !== requestSeqRef.current.metadata) { return; } @@ -510,6 +531,9 @@ export function PrCreateModal({ {/** * FNXC:PrCreateModal 2026-06-27-00:00: * FN-7170 moves Create PR onto the shared FloatingWindow shell so it matches Plan Mission, Automations, and New Task: desktop users can drag the embedded modal header and resize from every FloatingWindow edge/corner, mobile stays full-screen through CSS, and geometry persists with persistGeometryKey="floating-window:pr-create". Overlay click-to-dismiss is intentionally dropped because FloatingWindow is non-blocking/click-through; close remains available via X, Cancel, and Escape. + * + * FNXC:PrCreateModal 2026-06-27-23:48: + * Do not reintroduce a naive overlay onClick target check here. Before FloatingWindow, self-removing buttons and resize-grip releases could retarget synthesized clicks to the backdrop and close the dialog; the floating shell avoids that footgun by having no backdrop-dismiss path for Create PR. */}
-
+ {/** + * FNXC:PrCreateModal 2026-06-27-23:48: + * The diff and commit preview defaults collapsed so long commit/file lists do not push the editable PR form below the fold. Users can expand the summary on demand without changing the preview content. + */} +
{t("pr.previewTitle", "Diff & commit preview")}

{t("pr.commitsLabel", "Commits")}

diff --git a/packages/dashboard/app/components/__tests__/PrCreateModal.test.tsx b/packages/dashboard/app/components/__tests__/PrCreateModal.test.tsx index 16b12b7d1f..e4e8f64eb5 100644 --- a/packages/dashboard/app/components/__tests__/PrCreateModal.test.tsx +++ b/packages/dashboard/app/components/__tests__/PrCreateModal.test.tsx @@ -1,8 +1,8 @@ import { readFileSync } from "node:fs"; -import { fireEvent, render, screen, waitFor, within } from "@testing-library/react"; +import { cleanup, fireEvent, render, screen, waitFor, within } from "@testing-library/react"; import { act } from "react"; import type { ComponentProps } from "react"; -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { PrCreateModal } from "../PrCreateModal"; import type { PrInfo } from "@fusion/core"; @@ -90,6 +90,7 @@ function stubPointerCapture(element: HTMLElement) { describe("PrCreateModal", () => { beforeEach(() => { localStorage.clear(); + vi.useRealTimers(); vi.clearAllMocks(); mocks.generatePrMetadata.mockResolvedValue(metadata); mocks.fetchPrPreflight.mockResolvedValue(preflight); @@ -99,6 +100,10 @@ describe("PrCreateModal", () => { mocks.resolvePrConflicts.mockResolvedValue({ result: { resolved: true, pushed: true, conflictedFiles: ["a.ts"], message: "resolved" }, preflight }); }); + afterEach(() => { + vi.useRealTimers(); + }); + it("renders nothing when closed", () => { render(); expect(screen.queryByRole("dialog")).toBeNull(); @@ -354,6 +359,32 @@ describe("PrCreateModal", () => { await waitFor(() => expect(screen.getByRole("button", { name: "Create PR" })).toBeEnabled()); }); + it("times out hung metadata generation into the manual fallback state", async () => { + vi.useFakeTimers(); + mocks.generatePrMetadata.mockReturnValueOnce(new Promise(() => {})); + + renderModal(); + + expect(screen.getByText(/generating ai title/i)).toBeInTheDocument(); + expect(screen.getByText(/generating ai body/i)).toBeInTheDocument(); + + act(() => { + vi.advanceTimersByTime(15001); + }); + await act(async () => { + await Promise.resolve(); + }); + vi.useRealTimers(); + + expect(await screen.findByText("Timed out generating PR metadata")).toBeInTheDocument(); + expect(screen.queryByText(/generating ai title/i)).not.toBeInTheDocument(); + expect(screen.queryByText(/generating ai body/i)).not.toBeInTheDocument(); + + const bodyInput = screen.getByLabelText(/body/i) as HTMLTextAreaElement; + expect(bodyInput.value).toContain("## Summary"); + expect(bodyInput.value).toContain("Closes FN-4756"); + }); + it("requires a non-empty body before submitting manual PR metadata", async () => { mocks.generatePrMetadata.mockRejectedValueOnce(new Error("metadata blew up")); renderModal(); @@ -398,12 +429,15 @@ describe("PrCreateModal", () => { expect(screen.getByRole("button", { name: "Create PR" })).toBeEnabled(); }); - it("renders empty option and preview states without throwing", async () => { + it("renders empty option and preview states collapsed by default without throwing", async () => { mocks.fetchPrPreflight.mockResolvedValueOnce({ ...preflight, commits: [], changedFiles: [] }); mocks.fetchPrOptions.mockResolvedValueOnce({ ...options, baseBranches: [], reviewers: [], assignees: [], labels: [] }); await renderModalLoaded(); + const preview = document.querySelector(".pr-create-collapsible"); + expect(preview).toBeInstanceOf(HTMLDetailsElement); + expect(preview).not.toHaveAttribute("open"); expect(screen.getByText("No commits found.")).toBeInTheDocument(); expect(screen.getByText("No changed files detected.")).toBeInTheDocument(); expect(screen.getByLabelText(/base branch/i)).toBeDisabled(); @@ -595,9 +629,30 @@ describe("PrCreateModal", () => { expect(screen.getByTestId("floating-window-pr-create")).toBeInTheDocument(); }); - it("closes on escape", async () => { + it("keeps self-removing inner controls from closing the floating modal", async () => { const { onClose } = await renderModalLoaded(); + + fireEvent.change(screen.getByLabelText(/title/i), { target: { value: "Edited title" } }); + fireEvent.click(screen.getByRole("button", { name: /revert to ai version/i })); + + expect(onClose).not.toHaveBeenCalled(); + expect(screen.getByRole("dialog", { name: "Create Pull Request" })).toBeInTheDocument(); + expect(screen.getByDisplayValue("AI title")).toBeInTheDocument(); + }); + + it("closes on escape, header close, and cancel", async () => { + const escapeHandles = await renderModalLoaded(); fireEvent.keyDown(document, { key: "Escape" }); - expect(onClose).toHaveBeenCalledTimes(1); + expect(escapeHandles.onClose).toHaveBeenCalledTimes(1); + + cleanup(); + const headerHandles = await renderModalLoaded(); + fireEvent.click(screen.getByRole("button", { name: "Close" })); + expect(headerHandles.onClose).toHaveBeenCalledTimes(1); + + cleanup(); + const cancelHandles = await renderModalLoaded(); + fireEvent.click(screen.getByRole("button", { name: "Cancel" })); + expect(cancelHandles.onClose).toHaveBeenCalledTimes(1); }); });