feat(FN-4764): complete Step 2-4 — surface structured gh errors in API UI CLI

Fusion-Task-Id: FN-4764 Fusion-Task-Lineage: 14809ad5-8463-4e16-a934-1414225f17ae
2026-05-17 14:27:53 -07:00
parent 71dfc916f4
commit 26c3d330d3
8 changed files with 242 additions and 25 deletions
--- a/packages/dashboard/app/components/PrCreateModal.tsx
+++ b/packages/dashboard/app/components/PrCreateModal.tsx
@@ -1,6 +1,6 @@
 import { useCallback, useEffect, useId, useMemo, useRef, useState } from "react";
 import { AlertTriangle, CheckCircle2, RefreshCw, Sparkles, X, XCircle } from "lucide-react";
-import { getErrorMessage, type PrInfo } from "@fusion/core";
+import { getErrorMessage, type PrInfo, type StructuredGhError } from "@fusion/core";
 import {
  createPr,
  fetchPrOptions,
@@ -24,6 +24,8 @@ interface PrCreateModalProps {
  addToast: (message: string, type?: ToastType) => void;
 }

+type ModalGhError = StructuredGhError & { operation: "create" };
+
 type PreflightCheck = {
  key: string;
  label: string;
@@ -126,6 +128,7 @@ export function PrCreateModal({
  const [loading, setLoading] = useState(false);
  const [submitting, setSubmitting] = useState(false);
  const [error, setError] = useState<string | null>(null);
+  const [lastGhError, setLastGhError] = useState<ModalGhError | null>(null);
  const [aiTitle, setAiTitle] = useState("");
  const [aiBody, setAiBody] = useState("");
  const [title, setTitle] = useState("");
@@ -292,13 +295,19 @@ export function PrCreateModal({
    if (!payload.title || submitting) return;
    setSubmitting(true);
    setError(null);
+    setLastGhError(null);
    try {
      const prInfo = await createPr(taskId, payload, projectId);
      onCreated(prInfo);
      addToast(`Created PR #${prInfo.number}`, "success");
      onClose();
    } catch (submitError) {
-      setError(getErrorMessage(submitError));
+      const details = (submitError as { details?: { githubError?: StructuredGhError } })?.details?.githubError;
+      const structured: ModalGhError = details
+        ? { ...details, operation: "create" }
+        : { code: "unknown", message: getErrorMessage(submitError), retryable: true, action: { kind: "retry" }, operation: "create" };
+      setLastGhError(structured);
+      setError(structured.message);
    } finally {
      setSubmitting(false);
    }
@@ -430,7 +439,10 @@ export function PrCreateModal({
            {error && (
              <div className="form-error" role="alert">
                <p>{error}</p>
-                <button type="button" className="btn btn-sm" onClick={() => void submit()}>Retry</button>
+                {lastGhError?.hint ? <p>{lastGhError.hint}</p> : null}
+                {lastGhError?.action?.kind === "shell" ? <p>Action: run <code>{lastGhError.action.command}</code></p> : null}
+                {lastGhError?.action?.kind === "open" ? <p>Action: open <a href={lastGhError.action.url} target="_blank" rel="noreferrer">docs</a></p> : null}
+                {lastGhError?.retryable ? <button type="button" className="btn btn-sm" onClick={() => void submit()}>Retry</button> : null}
              </div>
            )}
          </>
--- a/packages/dashboard/app/components/PrPanel.tsx
+++ b/packages/dashboard/app/components/PrPanel.tsx
@@ -1,6 +1,6 @@
 import { useCallback, useEffect, useMemo, useState } from "react";
 import { GitPullRequest, ExternalLink, RefreshCw, Plus, MessageSquare, CircleDot, XCircle, GitMerge } from "lucide-react";
-import { getErrorMessage, type DirectMergeCommitStrategy } from "@fusion/core";
+import { getErrorMessage, type DirectMergeCommitStrategy, type StructuredGhError } from "@fusion/core";
 import { fetchPrReviews, mergePr, reclaimPrConflict, refreshPrStatus, setAutoMergeOnGreen, type PrCheckStatus, type PrInfo, type PrRefreshResponse, type PrReviewsResponse } from "../api";
 import { usePrChecksStream } from "../hooks/usePrChecksStream";
 import { PrChecksList } from "./PrChecksList";
@@ -60,6 +60,7 @@ export function PrPanel({
  const [refreshState, setRefreshState] = useState<PrRefreshResponse | null>(null);
  const [reviewsState, setReviewsState] = useState<PrReviewsResponse | null>(null);
  const [isMerging, setIsMerging] = useState(false);
+  const [lastGhError, setLastGhError] = useState<(StructuredGhError & { operation: "refresh" }) | null>(null);
  const [isReclaimingConflict, setIsReclaimingConflict] = useState(false);
  const [mergeStrategy, setMergeStrategy] = useState<"merge" | "squash" | "rebase">(
    directMergeCommitStrategy === "always-rebase"
@@ -83,6 +84,7 @@ export function PrPanel({
    if (!prInfo) return;

    setIsRefreshing(true);
+    setLastGhError(null);
    try {
      const updated = await refreshPrStatus(taskId, projectId);
      setRefreshState(updated);
@@ -91,7 +93,10 @@ export function PrPanel({
      setReviewsState(latestReviews);
      addToast("PR status refreshed", "success");
    } catch (err) {
-      addToast(getErrorMessage(err) || "Failed to refresh PR", "error");
+      const details = (err as { details?: { githubError?: StructuredGhError } })?.details?.githubError;
+      const structured = details ? { ...details, operation: "refresh" as const } : { code: "unknown" as const, message: getErrorMessage(err) || "Failed to refresh PR", retryable: true, action: { kind: "retry" as const }, operation: "refresh" as const };
+      setLastGhError(structured);
+      addToast(structured.message || "Failed to refresh PR", "error");
    } finally {
      setIsRefreshing(false);
    }
@@ -229,6 +234,14 @@ export function PrPanel({
          </button>
        </div>
        <div className="pr-title">{prInfo.title}</div>
+        {lastGhError ? (
+          <div className="pr-hint pr-hint--warning" role="alert">
+            <div>{lastGhError.message}</div>
+            {lastGhError.hint ? <div>{lastGhError.hint}</div> : null}
+            {lastGhError.action?.kind === "shell" ? <div>Action: run <code>{lastGhError.action.command}</code></div> : null}
+            {lastGhError.retryable ? <button className="btn btn-sm" onClick={() => void handleRefresh()}>Retry</button> : null}
+          </div>
+        ) : null}
        <div className="pr-meta">
          <span>{prInfo.headBranch}</span>
          <span className="pr-meta-arrow">→</span>
--- a/packages/dashboard/app/components/tests/PrCreateModal.test.tsx
+++ b/packages/dashboard/app/components/tests/PrCreateModal.test.tsx
@@ -133,6 +133,25 @@ describe("PrCreateModal", () => {
    expect(mocks.createPr.mock.calls[0][1]).toEqual(mocks.createPr.mock.calls[1][1]);
  });

+  it("renders structured gh auth hint", async () => {
+    const err = Object.assign(new Error("auth failed"), {
+      details: {
+        githubError: {
+          code: "not-authenticated",
+          message: "GitHub CLI is not authenticated.",
+          hint: "Run 'gh auth login' to authenticate with GitHub.",
+          action: { kind: "shell", command: "gh auth login" },
+          retryable: true,
+        },
+      },
+    });
+    mocks.createPr.mockRejectedValueOnce(err);
+    renderModal();
+    await screen.findByDisplayValue("AI title");
+    fireEvent.click(screen.getByRole("button", { name: "Create PR" }));
+    expect((await screen.findAllByText(/gh auth login/i)).length).toBeGreaterThan(0);
+  });
+
  it("closes on escape", async () => {
    const { onClose } = renderModal();
    await screen.findByDisplayValue("AI title");
--- a/packages/dashboard/app/components/tests/PrPanel.test.tsx
+++ b/packages/dashboard/app/components/tests/PrPanel.test.tsx
@@ -215,15 +215,28 @@ describe("PrPanel", () => {
    expect(await screen.findByRole("button", { name: /Retry conflict reclaim/i })).toBeInTheDocument();
  });

-  it("shows error toast when refresh fails", async () => {
-    (refreshPrStatus as ReturnType<typeof vi.fn>).mockRejectedValue(new Error("refresh failed"));
+  it("shows inline structured refresh error and retries", async () => {
+    (refreshPrStatus as ReturnType<typeof vi.fn>)
+      .mockRejectedValueOnce(Object.assign(new Error("auth failed"), {
+        details: {
+          githubError: {
+            code: "not-authenticated",
+            message: "GitHub CLI is not authenticated.",
+            hint: "Run 'gh auth login' to authenticate with GitHub.",
+            action: { kind: "shell", command: "gh auth login" },
+            retryable: true,
+          },
+        },
+      }))
+      .mockResolvedValueOnce({ prInfo: mockPrInfo, checks: [], reviewDecision: null, blockingReasons: [] });

    render(<PrPanel taskId="FN-001" prInfo={mockPrInfo} prAuthAvailable={true} onPrUpdated={mockOnPrUpdated} addToast={mockAddToast} />);
    fireEvent.click(screen.getByTitle("Refresh PR status"));

+    expect((await screen.findAllByText(/gh auth login/i)).length).toBeGreaterThan(0);
+    fireEvent.click(screen.getByRole("button", { name: "Retry" }));
    await waitFor(() => {
-      expect(mockAddToast).toHaveBeenCalledWith("refresh failed", "error");
+      expect(refreshPrStatus).toHaveBeenCalledTimes(2);
    });
-    expect(mockOnPrUpdated).not.toHaveBeenCalled();
  });
 });
--- a/packages/dashboard/src/tests/register-git-github.pr-errors.test.ts
+++ b/packages/dashboard/src/tests/register-git-github.pr-errors.test.ts
@@ -0,0 +1,113 @@
+// @vitest-environment node
+
+import { afterEach, describe, expect, it, vi } from "vitest";
+import type { Task, TaskStore } from "@fusion/core";
+import { createServer } from "../server.js";
+import { request as performRequest } from "../test-request.js";
+import { GitHubClient } from "../github.js";
+
+function createTask(overrides: Partial<Task> = {}): Task {
+  return {
+    id: "FN-001",
+    title: "Task",
+    description: "desc",
+    column: "in-review",
+    status: "in-review",
+    createdAt: new Date().toISOString(),
+    updatedAt: new Date().toISOString(),
+    prInfo: {
+      url: "https://github.com/owner/repo/pull/1",
+      number: 1,
+      status: "open",
+      title: "PR",
+      headBranch: "feature",
+      baseBranch: "main",
+      commentCount: 0,
+    },
+    comments: [],
+    ...overrides,
+  } as Task;
+}
+
+function createStore(task: Task): TaskStore {
+  return {
+    getTask: vi.fn().mockResolvedValue(task),
+    updatePrInfo: vi.fn().mockResolvedValue(undefined),
+    logEntry: vi.fn().mockResolvedValue(undefined),
+    getRootDir: vi.fn().mockReturnValue("/tmp/project"),
+    applyPrMergedTransition: vi.fn().mockResolvedValue(undefined),
+    listTasks: vi.fn().mockResolvedValue([]),
+    createTask: vi.fn(),
+    moveTask: vi.fn().mockResolvedValue(task),
+    updateTask: vi.fn(),
+    deleteTask: vi.fn(),
+    mergeTask: vi.fn(),
+    archiveTask: vi.fn(),
+    unarchiveTask: vi.fn(),
+    getSettings: vi.fn().mockResolvedValue({}),
+    updateSettings: vi.fn(),
+    getAgentLogs: vi.fn().mockResolvedValue([]),
+    addSteeringComment: vi.fn(),
+    updateIssueInfo: vi.fn(),
+    addComment: vi.fn().mockResolvedValue(task),
+    upsertTaskDocument: vi.fn().mockResolvedValue({ key: "review-feedback" }),
+    getFusionDir: vi.fn().mockReturnValue("/tmp/project/.fusion"),
+    getDatabase: vi.fn().mockReturnValue({
+      exec: vi.fn(),
+      prepare: vi.fn().mockReturnValue({ run: vi.fn().mockReturnValue({ changes: 0 }), get: vi.fn(), all: vi.fn().mockReturnValue([]) }),
+    }),
+    getMissionStore: vi.fn().mockReturnValue({ listMissions: vi.fn().mockReturnValue([]) }),
+    on: vi.fn(),
+    off: vi.fn(),
+  } as unknown as TaskStore;
+}
+
+describe("PR route structured GitHub errors", () => {
+  const originalRepoEnv = process.env.GITHUB_REPOSITORY;
+
+  afterEach(() => {
+    vi.restoreAllMocks();
+    if (originalRepoEnv === undefined) {
+      delete process.env.GITHUB_REPOSITORY;
+    } else {
+      process.env.GITHUB_REPOSITORY = originalRepoEnv;
+    }
+  });
+
+  it("maps not-authenticated to 401 with githubError details", async () => {
+    process.env.GITHUB_REPOSITORY = "owner/repo";
+    const task = createTask({ prInfo: undefined });
+    vi.spyOn(GitHubClient.prototype, "findPrForBranch").mockRejectedValue(new Error("authentication required 401"));
+
+    const app = createServer(createStore(task));
+    const response = await performRequest(app, "POST", "/api/tasks/FN-001/pr/create", JSON.stringify({ title: "PR title" }), { "content-type": "application/json" });
+
+    expect(response.status).toBe(401);
+    expect(response.body.details.githubError.code).toBe("not-authenticated");
+    expect(response.body.details.githubError.hint).toContain("gh auth login");
+  });
+
+  it("maps rate-limited to 429 with retryAfterMs", async () => {
+    const task = createTask();
+    vi.spyOn(GitHubClient.prototype, "getPrReviewSnapshot").mockRejectedValue({ message: "403 API rate limit exceeded", stderr: "Retry-After: 7" });
+
+    const app = createServer(createStore(task));
+    const response = await performRequest(app, "POST", "/api/tasks/FN-001/pr/refresh", "{}", { "content-type": "application/json" });
+
+    expect(response.status).toBe(429);
+    expect(response.body.details.githubError.code).toBe("rate-limited");
+    expect(response.body.details.githubError.retryAfterMs).toBe(7000);
+  });
+
+  it("maps unknown errors to 502 and retryable true", async () => {
+    const task = createTask();
+    vi.spyOn(GitHubClient.prototype, "getPrReviewSnapshot").mockRejectedValue(new Error("kaboom"));
+
+    const app = createServer(createStore(task));
+    const response = await performRequest(app, "POST", "/api/tasks/FN-001/pr/refresh", "{}", { "content-type": "application/json" });
+
+    expect(response.status).toBe(502);
+    expect(response.body.details.githubError.code).toBe("unknown");
+    expect(response.body.details.githubError.retryable).toBe(true);
+  });
+});
--- a/packages/dashboard/src/routes/register-git-github.ts
+++ b/packages/dashboard/src/routes/register-git-github.ts
@@ -8,10 +8,11 @@ import type {
  IssueInfo,
  PrInfo,
  RunAuditEventInput,
+  StructuredGhError,
  Task,
  TaskStore,
 } from "@fusion/core";
-import { getCurrentRepo, isGhAuthenticated } from "@fusion/core";
+import { classifyGhError, getCurrentRepo, isGhAuthenticated } from "@fusion/core";
 import {
  ApiError,
  badRequest,
@@ -46,6 +47,32 @@ function getCommandErrorMessage(error: unknown): string {
  return String(error);
 }

+function mapStructuredGhErrorToStatus(code: StructuredGhError["code"]): number {
+  switch (code) {
+    case "not-authenticated":
+      return 401;
+    case "permission":
+      return 403;
+    case "rate-limited":
+      return 429;
+    case "not-found":
+      return 404;
+    case "validation":
+    case "merge-conflict":
+      return 422;
+    default:
+      return 502;
+  }
+}
+
+function toPrApiError(err: unknown, fallbackMessage: string): ApiError {
+  const githubError = classifyGhError(err);
+  return new ApiError(mapStructuredGhErrorToStatus(githubError.code), githubError.message || fallbackMessage, {
+    githubError,
+    ...(typeof githubError.retryAfterMs === "number" ? { retryAfterMs: githubError.retryAfterMs } : {}),
+  });
+}
+
 export { runGitCommand };

 /** Git remote info returned by the remotes endpoint */
@@ -3299,7 +3326,7 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void {
      } else if ((err instanceof Error ? err.message : String(err)).includes("No commits between")) {
        throw badRequest("Branch has no commits. Push changes before creating PR.");
      } else {
-        rethrowAsApiError(err, "Failed to create PR");
+        throw toPrApiError(err, "Failed to create PR");
      }
    }
  });
@@ -3463,10 +3490,8 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void {
      }
      if ((err as NodeJS.ErrnoException).code === "ENOENT") {
        throw notFound(`Task ${req.params.id} not found`);
-      } else if ((err instanceof Error ? err.message : String(err)).includes("not found")) {
-        throw notFound(err instanceof Error ? err.message : String(err));
      } else {
-        rethrowAsApiError(err);
+        throw toPrApiError(err, "Failed to refresh PR status");
      }
    }
  });
@@ -3523,7 +3548,7 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void {
      if (err instanceof ApiError) {
        throw err;
      }
-      rethrowAsApiError(err, "Failed to merge PR");
+      throw toPrApiError(err, "Failed to merge PR");
    }
  });

@@ -3620,7 +3645,7 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void {
      if ((err as NodeJS.ErrnoException).code === "ENOENT") {
        throw notFound(`Task ${req.params.id} not found`);
      }
-      rethrowAsApiError(err);
+      throw toPrApiError(err, "Failed to fetch PR reviews");
    }
  });

@@ -3685,10 +3710,8 @@ export function registerGitGitHubRoutes(ctx: ApiRoutesContext): void {
      }
      if ((err as NodeJS.ErrnoException).code === "ENOENT") {
        throw notFound(`Task ${req.params.id} not found`);
-      } else if ((err instanceof Error ? err.message : String(err)).includes("not found")) {
-        throw notFound(err instanceof Error ? err.message : String(err));
      } else {
-        rethrowAsApiError(err);
+        throw toPrApiError(err, "Failed to fetch PR checks");
      }
    }
  });