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) <noreply@runfusion.ai>
This commit is contained in:
7
.changeset/fn-7175-create-pr-dialog-fixes.md
Normal file
7
.changeset/fn-7175-create-pr-dialog-fixes.md
Normal file
@@ -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 <details> closed, and time-bounds generatePrMetadata with PR_METADATA_TIMEOUT_MS so hangs use the existing error/manual-body fallback.
|
||||
@@ -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/<task-id-lower>` 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.
|
||||
|
||||
@@ -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<T>(promise: Promise<T>): Promise<T> {
|
||||
let timeoutId: ReturnType<typeof setTimeout> | undefined;
|
||||
const timeout = new Promise<T>((_, 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.
|
||||
*/}
|
||||
<div
|
||||
ref={modalRef}
|
||||
@@ -654,7 +678,11 @@ export function PrCreateModal({
|
||||
includeColor
|
||||
/>
|
||||
|
||||
<details className="pr-create-collapsible" open>
|
||||
{/**
|
||||
* 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.
|
||||
*/}
|
||||
<details className="pr-create-collapsible">
|
||||
<summary>{t("pr.previewTitle", "Diff & commit preview")}</summary>
|
||||
<div className="pr-create-modal__preview">
|
||||
<h4>{t("pr.commitsLabel", "Commits")}</h4>
|
||||
|
||||
@@ -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(<PrCreateModal open={false} taskId="FN-4756" onClose={vi.fn()} onCreated={vi.fn()} addToast={vi.fn()} />);
|
||||
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);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user