diff --git a/.github/workflows/nightly.yml b/.github/workflows/nightly.yml index cb7bcff52..abc74c0e5 100644 --- a/.github/workflows/nightly.yml +++ b/.github/workflows/nightly.yml @@ -49,7 +49,7 @@ jobs: - name: Install bun deps (for SPA verify) if: steps.skip.outputs.skip != 'true' - run: bun install + run: bun install --frozen-lockfile - name: Set up Rust if: steps.skip.outputs.skip != 'true' diff --git a/.github/workflows/typescript.yml b/.github/workflows/typescript.yml index 84a0e8496..9c5b914cc 100644 --- a/.github/workflows/typescript.yml +++ b/.github/workflows/typescript.yml @@ -12,6 +12,7 @@ on: - ".gitattributes" - "scripts/**" - ".github/workflows/typescript.yml" + - ".github/workflows/nightly.yml" pull_request: branches: [main] paths: @@ -23,6 +24,7 @@ on: - ".gitattributes" - "scripts/**" - ".github/workflows/typescript.yml" + - ".github/workflows/nightly.yml" workflow_dispatch: concurrency: @@ -42,7 +44,7 @@ jobs: with: persist-credentials: false - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 - - run: bun install + - run: bun install --frozen-lockfile - run: cd apps/fabro-web && bun run typecheck test: @@ -55,7 +57,7 @@ jobs: with: persist-credentials: false - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 - - run: bun install + - run: bun install --frozen-lockfile - run: cd apps/fabro-web && bun test build: @@ -68,7 +70,7 @@ jobs: with: persist-credentials: false - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2.2.0 - - run: bun install + - run: bun install --frozen-lockfile - run: scripts/refresh-fabro-spa.sh - run: git diff --exit-code -- lib/crates/fabro-spa/assets - run: scripts/check-fabro-spa-budgets.sh diff --git a/Cargo.lock b/Cargo.lock index 11d0a6285..33c5a8282 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1857,6 +1857,7 @@ dependencies = [ "axum", "base64", "fabro-http", + "fabro-test", "hex", "httpmock", "open", @@ -1958,6 +1959,7 @@ dependencies = [ "fabro-slack", "fabro-spa", "fabro-store", + "fabro-test", "fabro-types", "fabro-util", "fabro-validate", diff --git a/apps/fabro-web/app/components/blocked-run-notice.test.tsx b/apps/fabro-web/app/components/blocked-run-notice.test.tsx new file mode 100644 index 000000000..f331e449d --- /dev/null +++ b/apps/fabro-web/app/components/blocked-run-notice.test.tsx @@ -0,0 +1,56 @@ +import { describe, expect, test } from "bun:test"; +import TestRenderer, { act } from "react-test-renderer"; + +import { BlockedRunNotice } from "./blocked-run-notice"; + +function textFromNode(node: ReturnType): string { + if (!node) return ""; + if (typeof node === "string") return node; + if (Array.isArray(node)) return node.map(textFromNode).join(""); + return (node.children ?? []).map(textFromNode).join(""); +} + +describe("BlockedRunNotice", () => { + test("renders the question text when provided", () => { + let tree: TestRenderer.ReactTestRenderer | undefined; + act(() => { + tree = TestRenderer.create( + {}} + />, + ); + }); + + expect(textFromNode(tree!.toJSON())).toContain("Approve the deployment target?"); + }); + + test("renders fallback copy when no question is available", () => { + let tree: TestRenderer.ReactTestRenderer | undefined; + act(() => { + tree = TestRenderer.create( {}} />); + }); + + expect(textFromNode(tree!.toJSON())).toContain("Fabro is blocked on a human-in-the-loop question."); + }); + + test("fires the secondary cancel action", () => { + let cancelled = 0; + let tree: TestRenderer.ReactTestRenderer | undefined; + act(() => { + tree = TestRenderer.create( + { + cancelled += 1; + }} + />, + ); + }); + + const button = tree!.root.findByType("button"); + act(() => { + button.props.onClick(); + }); + + expect(cancelled).toBe(1); + }); +}); diff --git a/apps/fabro-web/app/components/blocked-run-notice.tsx b/apps/fabro-web/app/components/blocked-run-notice.tsx new file mode 100644 index 000000000..892b61692 --- /dev/null +++ b/apps/fabro-web/app/components/blocked-run-notice.tsx @@ -0,0 +1,34 @@ +export function BlockedRunNotice({ + questionText, + cancelling = false, + onCancel, +}: { + questionText?: string | null; + cancelling?: boolean; + onCancel: () => void; +}) { + return ( +
+

This run is waiting for input.

+

+ {questionText?.trim() + ? questionText + : "Fabro is blocked on a human-in-the-loop question. Answer it from the CLI to continue the run."} +

+

+ If you don't want to continue in the CLI, you can cancel the run here instead. +

+ +
+ ); +} diff --git a/apps/fabro-web/app/components/stage-sidebar.tsx b/apps/fabro-web/app/components/stage-sidebar.tsx index 7bbfe1fe0..f6cc55105 100644 --- a/apps/fabro-web/app/components/stage-sidebar.tsx +++ b/apps/fabro-web/app/components/stage-sidebar.tsx @@ -1,5 +1,5 @@ import { useState, useEffect, useRef } from "react"; -import { Link, useRevalidator } from "react-router"; +import { Link } from "react-router"; import { ArrowPathIcon, CheckCircleIcon, @@ -9,6 +9,7 @@ import { } from "@heroicons/react/24/solid"; import { DocumentTextIcon, MapIcon } from "@heroicons/react/24/outline"; import { formatDurationSecs } from "../lib/format"; +import { useRunEventSource } from "../lib/sse"; export type StageStatus = "completed" | "running" | "pending" | "failed" | "cancelled"; @@ -42,34 +43,15 @@ const STAGE_EVENTS = new Set([ ]); export function StageSidebar({ stages, runId, selectedStageId, activeLink }: StageSidebarProps) { - const revalidator = useRevalidator(); - // Track when we first observed each running stage (for ticking timer) const runningStartRef = useRef>(new Map()); const [, setTick] = useState(0); // Subscribe to run-specific SSE for live stage updates - useEffect(() => { - const source = new EventSource(`/api/v1/runs/${runId}/attach?since_seq=1`); - let debounceTimer: ReturnType | undefined; - - source.onmessage = (msg) => { - try { - const payload = JSON.parse(msg.data); - if (STAGE_EVENTS.has(payload.event)) { - clearTimeout(debounceTimer); - debounceTimer = setTimeout(() => revalidator.revalidate(), 300); - } - } catch { - // ignore malformed events - } - }; - - return () => { - clearTimeout(debounceTimer); - source.close(); - }; - }, [runId]); + useRunEventSource(runId, { + allowlist: STAGE_EVENTS, + debounceMs: 300, + }); // Track start times for running stages useEffect(() => { diff --git a/apps/fabro-web/app/components/toast.test.tsx b/apps/fabro-web/app/components/toast.test.tsx new file mode 100644 index 000000000..d23bf64d8 --- /dev/null +++ b/apps/fabro-web/app/components/toast.test.tsx @@ -0,0 +1,196 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { useEffect } from "react"; +import TestRenderer, { act } from "react-test-renderer"; + +import { ToastProvider, useToast } from "./toast"; + +function textFromNode(node: ReturnType): string { + if (!node) return ""; + if (typeof node === "string") return node; + if (Array.isArray(node)) return node.map(textFromNode).join(""); + return (node.children ?? []).map(textFromNode).join(""); +} + +function textFromInstance(node: TestRenderer.ReactTestInstance): string { + return node.children + .map((child) => (typeof child === "string" ? child : textFromInstance(child))) + .join(""); +} + +function PushOnMount({ + toast, + onReady, +}: { + toast: Parameters["push"]>[0]; + onReady?: (api: ReturnType) => void; +}) { + const api = useToast(); + + useEffect(() => { + onReady?.(api); + api.push(toast); + }, [api, onReady, toast]); + + return null; +} + +describe("ToastProvider", () => { + afterEach(() => { + delete (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT; + }); + + test("push renders a toast with the message", async () => { + (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + + let renderer: TestRenderer.ReactTestRenderer | null = null; + await act(async () => { + renderer = TestRenderer.create( + + + , + ); + }); + + expect(textFromNode(renderer!.toJSON())).toContain("Run archived."); + const liveRegions = renderer!.root.findAll( + (node) => node.props?.role === "status" && node.props?.["aria-live"] === "polite", + ); + expect(liveRegions.length).toBeGreaterThan(0); + + await act(async () => { + renderer?.unmount(); + }); + }); + + test("action toasts render a button and fire onClick", async () => { + (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + + let clicked = 0; + let renderer: TestRenderer.ReactTestRenderer | null = null; + await act(async () => { + renderer = TestRenderer.create( + + { + clicked += 1; + }, + }, + }} + /> + , + ); + }); + + const button = renderer!.root.findByType("button"); + expect(textFromNode(renderer!.toJSON())).toContain("Unarchive"); + + await act(async () => { + button.props.onClick(); + }); + + expect(clicked).toBe(1); + + await act(async () => { + renderer?.unmount(); + }); + }); + + test("error toasts do not auto-dismiss", async () => { + (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + + let renderer: TestRenderer.ReactTestRenderer | null = null; + await act(async () => { + renderer = TestRenderer.create( + + + , + ); + }); + + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 20)); + }); + + expect(textFromNode(renderer!.toJSON())).toContain("Conflict"); + + await act(async () => { + renderer?.unmount(); + }); + }); + + test("multiple toasts stack in insertion order", async () => { + (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + + let api: ReturnType | null = null; + let renderer: TestRenderer.ReactTestRenderer | null = null; + + await act(async () => { + renderer = TestRenderer.create( + + { + api = value; + }} + /> + , + ); + }); + + await act(async () => { + api!.push({ message: "Second" }); + }); + + const toasts = renderer!.root.findAll( + (node) => node.props?.["data-toast-id"] != null, + ); + expect(toasts).toHaveLength(2); + expect(textFromInstance(toasts[0]!)).toContain("First"); + expect(textFromInstance(toasts[1]!)).toContain("Second"); + + await act(async () => { + renderer?.unmount(); + }); + }); + + test("dismiss removes a toast and leaves the rest reflowed", async () => { + (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; + + let api: ReturnType | null = null; + let firstId = ""; + let renderer: TestRenderer.ReactTestRenderer | null = null; + + await act(async () => { + renderer = TestRenderer.create( + + { + api = value; + }} + /> + , + ); + }); + + await act(async () => { + firstId = api!.push({ message: "Second" }); + }); + + await act(async () => { + api!.dismiss(firstId); + }); + + const text = textFromNode(renderer!.toJSON()); + expect(text).toContain("First"); + expect(text).not.toContain("Second"); + + await act(async () => { + renderer?.unmount(); + }); + }); +}); diff --git a/apps/fabro-web/app/components/toast.tsx b/apps/fabro-web/app/components/toast.tsx new file mode 100644 index 000000000..5c7eb2407 --- /dev/null +++ b/apps/fabro-web/app/components/toast.tsx @@ -0,0 +1,165 @@ +import { + createContext, + useCallback, + useContext, + useEffect, + useMemo, + useRef, + useState, + type ReactNode, +} from "react"; +import { XMarkIcon } from "@heroicons/react/20/solid"; + +export type ToastTone = "info" | "error"; + +export interface ToastAction { + label: string; + onClick: () => void; +} + +export interface ToastInput { + message: string; + tone?: ToastTone; + action?: ToastAction; + autoDismissMs?: number; +} + +interface ToastRecord extends ToastInput { + id: string; + tone: ToastTone; +} + +interface ToastContextValue { + push: (toast: ToastInput) => string; + dismiss: (id: string) => void; + clear: () => void; +} + +const ToastContext = createContext(null); + +function toastClassName(tone: ToastTone): string { + return tone === "error" + ? "border-coral/40 bg-rose-950/90 text-rose-50" + : "border-line bg-panel/95 text-fg-2"; +} + +export function ToastRoot({ + toasts, + onDismiss, +}: { + toasts: ToastRecord[]; + onDismiss: (id: string) => void; +}) { + if (toasts.length === 0) return null; + + return ( +
+ {toasts.map((toast) => ( +
+
+

{toast.message}

+ {toast.tone === "error" && ( + + )} +
+ {toast.action && ( +
+ +
+ )} +
+ ))} +
+ ); +} + +export function ToastProvider({ + children, + autoDismissMs = 3500, +}: { + children: ReactNode; + autoDismissMs?: number; +}) { + const [toasts, setToasts] = useState([]); + const nextIdRef = useRef(0); + const timeoutIdsRef = useRef(new Map>()); + + const dismiss = useCallback((id: string) => { + const timeoutId = timeoutIdsRef.current.get(id); + if (timeoutId) { + clearTimeout(timeoutId); + timeoutIdsRef.current.delete(id); + } + setToasts((current) => current.filter((toast) => toast.id !== id)); + }, []); + + const clear = useCallback(() => { + for (const timeoutId of timeoutIdsRef.current.values()) { + clearTimeout(timeoutId); + } + timeoutIdsRef.current.clear(); + setToasts([]); + }, []); + + const push = useCallback((toast: ToastInput) => { + const id = `toast-${nextIdRef.current++}`; + const record: ToastRecord = { + ...toast, + id, + tone: toast.tone ?? "info", + }; + + setToasts((current) => [...current, record]); + + if (record.tone !== "error") { + const timeoutId = setTimeout(() => dismiss(id), toast.autoDismissMs ?? autoDismissMs); + timeoutIdsRef.current.set(id, timeoutId); + } + + return id; + }, [autoDismissMs, dismiss]); + + useEffect(() => clear, [clear]); + + const value = useMemo(() => ({ push, dismiss, clear }), [push, dismiss, clear]); + + return ( + + {children} + + + ); +} + +export function useToast(): ToastContextValue { + const value = useContext(ToastContext); + if (!value) { + throw new Error("useToast must be used within a ToastProvider"); + } + return value; +} diff --git a/apps/fabro-web/app/layouts/app-shell.tsx b/apps/fabro-web/app/layouts/app-shell.tsx index baf96df58..4c2b81a71 100644 --- a/apps/fabro-web/app/layouts/app-shell.tsx +++ b/apps/fabro-web/app/layouts/app-shell.tsx @@ -18,6 +18,7 @@ import { } from "@heroicons/react/24/outline"; import { Link, Outlet, useLocation, useMatches, useRevalidator } from "react-router"; import { getAuthMe } from "../api"; +import { ToastProvider } from "../components/toast"; import { DemoModeProvider } from "../lib/demo-mode"; export async function loader() { @@ -66,6 +67,7 @@ export default function AppShell({ loaderData }: any) { return ( +
@@ -250,6 +252,7 @@ export default function AppShell({ loaderData }: any) {
+ ); } diff --git a/apps/fabro-web/app/lib/run-actions.test.ts b/apps/fabro-web/app/lib/run-actions.test.ts new file mode 100644 index 000000000..c50116614 --- /dev/null +++ b/apps/fabro-web/app/lib/run-actions.test.ts @@ -0,0 +1,179 @@ +import { afterEach, describe, expect, test } from "bun:test"; + +import { + archiveRun, + canArchive, + canCancel, + canUnarchive, + cancelRun, + isTerminalCancelledRun, + mapError, + unarchiveRun, +} from "./run-actions"; + +type StubResponseInit = { + status: number; + body?: string; + statusText?: string; +}; + +function stubFetchOnce(init: StubResponseInit) { + const originalFetch = globalThis.fetch; + globalThis.fetch = (() => + Promise.resolve( + new Response(init.body ?? "", { + status: init.status, + statusText: init.statusText ?? "", + headers: { "Content-Type": "application/json" }, + }), + )) as typeof fetch; + + return () => { + globalThis.fetch = originalFetch; + }; +} + +async function expectLifecycleError( + input: Promise, +): Promise<{ status: number; errors: Array<{ status: string; title: string; detail: string }> }> { + try { + await input; + throw new Error("expected promise to reject"); + } catch (error) { + return error as { status: number; errors: Array<{ status: string; title: string; detail: string }> }; + } +} + +describe("run lifecycle actions", () => { + let restoreFetch: (() => void) | undefined; + + afterEach(() => { + restoreFetch?.(); + restoreFetch = undefined; + delete (globalThis as { window?: unknown }).window; + }); + + test("cancelRun parses a 200 response", async () => { + restoreFetch = stubFetchOnce({ + status: 200, + body: JSON.stringify({ + id: "run-1", + status: "failed", + status_reason: "cancelled", + created_at: "2026-04-20T12:00:00Z", + }), + }); + + const result = await cancelRun("run-1"); + expect(result.status).toBe("failed"); + expect(result.status_reason).toBe("cancelled"); + }); + + test("archiveRun parses a 200 response", async () => { + restoreFetch = stubFetchOnce({ + status: 200, + body: JSON.stringify({ + id: "run-1", + status: "archived", + created_at: "2026-04-20T12:00:00Z", + }), + }); + + const result = await archiveRun("run-1"); + expect(result.status).toBe("archived"); + }); + + test("unarchiveRun parses a 200 response", async () => { + restoreFetch = stubFetchOnce({ + status: 200, + body: JSON.stringify({ + id: "run-1", + status: "succeeded", + created_at: "2026-04-20T12:00:00Z", + }), + }); + + const result = await unarchiveRun("run-1"); + expect(result.status).toBe("succeeded"); + }); + + test("404 and 409 preserve the parsed error envelope", async () => { + restoreFetch = stubFetchOnce({ + status: 404, + body: JSON.stringify({ + errors: [{ status: "404", title: "Not Found", detail: "Run not found." }], + }), + }); + const notFound = await expectLifecycleError(cancelRun("missing-run")); + expect(notFound).toEqual({ + status: 404, + errors: [{ status: "404", title: "Not Found", detail: "Run not found." }], + }); + + restoreFetch = stubFetchOnce({ + status: 409, + body: JSON.stringify({ + errors: [{ status: "409", title: "Conflict", detail: "Run is not terminal." }], + }), + }); + const conflict = await expectLifecycleError(archiveRun("run-1")); + expect(conflict).toEqual({ + status: 409, + errors: [{ status: "409", title: "Conflict", detail: "Run is not terminal." }], + }); + }); + + test("non-JSON error bodies fall back to an empty error list", async () => { + restoreFetch = stubFetchOnce({ + status: 409, + body: "conflict", + statusText: "Conflict", + }); + + const error = await expectLifecycleError(unarchiveRun("run-1")); + expect(error).toEqual({ status: 409, errors: [] }); + }); + + test("mapError returns user-facing copy for lifecycle conflicts", () => { + expect(mapError({ status: 409, errors: [] }, "cancel")).toBe("This run can no longer be cancelled."); + expect(mapError({ status: 409, errors: [] }, "archive")).toBe("Only terminal runs can be archived."); + expect(mapError({ status: 409, errors: [] }, "unarchive")).toBe("Active runs can't be unarchived."); + }); + + test("status predicates align with the documented run statuses", () => { + expect(canCancel("submitted")).toBe(true); + expect(canCancel("queued")).toBe(true); + expect(canCancel("starting")).toBe(true); + expect(canCancel("running")).toBe(true); + expect(canCancel("paused")).toBe(true); + expect(canCancel("blocked")).toBe(false); + expect(canCancel("archived")).toBe(false); + + expect(canArchive("succeeded")).toBe(true); + expect(canArchive("failed")).toBe(true); + expect(canArchive("dead")).toBe(true); + expect(canArchive("archived")).toBe(false); + + expect(canUnarchive("archived")).toBe(true); + expect(canUnarchive("failed")).toBe(false); + }); + + test("isTerminalCancelledRun distinguishes immediate cancel success from in-flight cancellation", () => { + expect( + isTerminalCancelledRun({ + id: "run-1", + status: "failed", + status_reason: "cancelled", + created_at: "2026-04-20T12:00:00Z", + }), + ).toBe(true); + expect( + isTerminalCancelledRun({ + id: "run-1", + status: "running", + pending_control: "cancel", + created_at: "2026-04-20T12:00:00Z", + }), + ).toBe(false); + }); +}); diff --git a/apps/fabro-web/app/lib/run-actions.ts b/apps/fabro-web/app/lib/run-actions.ts new file mode 100644 index 000000000..a7276dbae --- /dev/null +++ b/apps/fabro-web/app/lib/run-actions.ts @@ -0,0 +1,145 @@ +import type { ErrorResponseEntry, RunStatusResponse } from "@qltysh/fabro-api-client"; + +import { apiFetch } from "../api"; +import type { RunStatus } from "../data/runs"; + +export type LifecycleAction = "cancel" | "archive" | "unarchive"; + +export interface LifecycleActionError { + status: number; + errors: ErrorResponseEntry[]; +} + +const CANCELABLE_STATUSES = new Set([ + "submitted", + "queued", + "starting", + "running", + "paused", +]); + +const ARCHIVABLE_STATUSES = new Set([ + "succeeded", + "failed", + "dead", +]); + +export async function cancelRun(id: string, request?: Request): Promise { + return runLifecycleAction(id, "cancel", request); +} + +export async function archiveRun(id: string, request?: Request): Promise { + return runLifecycleAction(id, "archive", request); +} + +export async function unarchiveRun(id: string, request?: Request): Promise { + return runLifecycleAction(id, "unarchive", request); +} + +export function canCancel(status: string | null | undefined): boolean { + return !!status && CANCELABLE_STATUSES.has(status as RunStatus); +} + +export function canArchive(status: string | null | undefined): boolean { + return !!status && ARCHIVABLE_STATUSES.has(status as RunStatus); +} + +export function canUnarchive(status: string | null | undefined): boolean { + return status === "archived"; +} + +export function isTerminalCancelledRun(run: RunStatusResponse): boolean { + return (run.status === "failed" || run.status === "dead") && run.status_reason === "cancelled"; +} + +export function mapError(error: unknown, action: LifecycleAction): string { + if (isLifecycleActionError(error)) { + if (error.status === 404) { + return "This run no longer exists."; + } + if (error.status === 409) { + switch (action) { + case "cancel": + return "This run can no longer be cancelled."; + case "archive": + return "Only terminal runs can be archived."; + case "unarchive": + return "Active runs can't be unarchived."; + } + } + + const detail = error.errors[0]?.detail?.trim(); + if (detail) { + return detail; + } + } + + switch (action) { + case "cancel": + return "Couldn't cancel the run right now. Try again."; + case "archive": + return "Couldn't archive the run right now. Try again."; + case "unarchive": + return "Couldn't unarchive the run right now. Try again."; + } +} + +async function runLifecycleAction( + id: string, + action: LifecycleAction, + request?: Request, +): Promise { + const response = await apiFetch(`/runs/${id}/${action}`, { + init: { + method: "POST", + ...(request?.signal ? { signal: request.signal } : {}), + }, + }); + + if (!response.ok) { + throw await parseLifecycleActionError(response); + } + + return response.json() as Promise; +} + +async function parseLifecycleActionError(response: Response): Promise { + let bodyText = ""; + try { + bodyText = await response.text(); + } catch { + // Ignore body read failures and fall back to the status only. + } + + if (!bodyText) { + return { status: response.status, errors: [] }; + } + + try { + const body = JSON.parse(bodyText) as { errors?: unknown }; + if (!Array.isArray(body.errors)) { + return { status: response.status, errors: [] }; + } + + const errors = body.errors.filter(isErrorResponseEntry); + return { status: response.status, errors }; + } catch { + return { status: response.status, errors: [] }; + } +} + +function isLifecycleActionError(value: unknown): value is LifecycleActionError { + if (!value || typeof value !== "object") return false; + const record = value as Record; + return typeof record.status === "number" && Array.isArray(record.errors); +} + +function isErrorResponseEntry(value: unknown): value is ErrorResponseEntry { + if (!value || typeof value !== "object") return false; + const record = value as Record; + return ( + typeof record.status === "string" + && typeof record.title === "string" + && typeof record.detail === "string" + ); +} diff --git a/apps/fabro-web/app/lib/sse.test.ts b/apps/fabro-web/app/lib/sse.test.ts new file mode 100644 index 000000000..4d7fce2f5 --- /dev/null +++ b/apps/fabro-web/app/lib/sse.test.ts @@ -0,0 +1,101 @@ +import { describe, expect, test } from "bun:test"; + +import { subscribeToRunEventSource } from "./sse"; + +type MessageHandler = ((event: { data: string }) => void) | null; + +class FakeEventSource { + onmessage: MessageHandler = null; + closed = false; + + emit(payload: unknown) { + this.onmessage?.({ data: JSON.stringify(payload) }); + } + + emitRaw(data: string) { + this.onmessage?.({ data }); + } + + close() { + this.closed = true; + } +} + +describe("subscribeToRunEventSource", () => { + test("allowlisted events trigger debounced revalidation and onEvent", async () => { + const source = new FakeEventSource(); + let revalidations = 0; + const events: Array<{ event?: string }> = []; + + const cleanup = subscribeToRunEventSource("run-1", { + allowlist: new Set(["run.completed"]), + debounceMs: 5, + revalidate: () => { + revalidations += 1; + }, + onEvent: (payload) => { + events.push(payload); + }, + eventSourceFactory: () => source, + }); + + source.emit({ event: "run.completed", seq: 42 }); + + await new Promise((resolve) => setTimeout(resolve, 20)); + + expect(revalidations).toBe(1); + expect(events).toEqual([{ event: "run.completed", seq: 42 }]); + + cleanup(); + }); + + test("non-allowlisted and malformed events are ignored", async () => { + const source = new FakeEventSource(); + let revalidations = 0; + let calls = 0; + + const cleanup = subscribeToRunEventSource("run-1", { + allowlist: new Set(["checkpoint.completed"]), + debounceMs: 5, + revalidate: () => { + revalidations += 1; + }, + onEvent: () => { + calls += 1; + }, + eventSourceFactory: () => source, + }); + + source.emit({ event: "run.completed" }); + source.emitRaw("{broken"); + + await new Promise((resolve) => setTimeout(resolve, 20)); + + expect(revalidations).toBe(0); + expect(calls).toBe(0); + + cleanup(); + }); + + test("cleanup closes the source and clears a pending debounce", async () => { + const source = new FakeEventSource(); + let revalidations = 0; + + const cleanup = subscribeToRunEventSource("run-1", { + allowlist: new Set(["run.completed"]), + debounceMs: 20, + revalidate: () => { + revalidations += 1; + }, + eventSourceFactory: () => source, + }); + + source.emit({ event: "run.completed" }); + cleanup(); + + await new Promise((resolve) => setTimeout(resolve, 40)); + + expect(source.closed).toBe(true); + expect(revalidations).toBe(0); + }); +}); diff --git a/apps/fabro-web/app/lib/sse.ts b/apps/fabro-web/app/lib/sse.ts new file mode 100644 index 000000000..f13c020e4 --- /dev/null +++ b/apps/fabro-web/app/lib/sse.ts @@ -0,0 +1,81 @@ +import { useEffect } from "react"; +import { useRevalidator } from "react-router"; + +export interface RunEventPayload { + event?: string; + [key: string]: unknown; +} + +export interface RunEventSourceLike { + onmessage: ((event: { data: string }) => void) | null; + close: () => void; +} + +interface SubscribeOptions { + allowlist: ReadonlySet; + debounceMs?: number; + onEvent?: (payload: RunEventPayload) => void; + revalidate: () => void; + eventSourceFactory?: (url: string) => RunEventSourceLike; +} + +function createBrowserEventSource(url: string): RunEventSourceLike { + return new EventSource(url); +} + +export function subscribeToRunEventSource(runId: string, options: SubscribeOptions): () => void { + const { + allowlist, + debounceMs = 300, + onEvent, + revalidate, + eventSourceFactory = createBrowserEventSource, + } = options; + + const source = eventSourceFactory(`/api/v1/runs/${runId}/attach?since_seq=1`); + let debounceTimer: ReturnType | undefined; + + source.onmessage = (message) => { + try { + const payload = JSON.parse(message.data) as RunEventPayload; + if (!payload.event || !allowlist.has(payload.event)) { + return; + } + onEvent?.(payload); + clearTimeout(debounceTimer); + debounceTimer = setTimeout(() => revalidate(), debounceMs); + } catch { + // ignore malformed events + } + }; + + return () => { + clearTimeout(debounceTimer); + source.close(); + }; +} + +export function useRunEventSource( + runId: string | undefined, + { + allowlist, + debounceMs = 300, + onEvent, + }: { + allowlist: ReadonlySet; + debounceMs?: number; + onEvent?: (payload: RunEventPayload) => void; + }, +) { + const revalidator = useRevalidator(); + + useEffect(() => { + if (!runId) return; + return subscribeToRunEventSource(runId, { + allowlist, + debounceMs, + onEvent, + revalidate: () => revalidator.revalidate(), + }); + }, [allowlist, debounceMs, onEvent, revalidator, runId]); +} diff --git a/apps/fabro-web/app/routes/run-detail.test.ts b/apps/fabro-web/app/routes/run-detail.test.ts new file mode 100644 index 000000000..fc9f3cc73 --- /dev/null +++ b/apps/fabro-web/app/routes/run-detail.test.ts @@ -0,0 +1,198 @@ +import { afterEach, describe, expect, test } from "bun:test"; + +import { action, lifecycleActionVisibility, loader } from "./run-detail"; + +type StubFetchEntry = { + status: number; + body?: unknown; +}; + +function stubFetchSequence(entries: StubFetchEntry[]) { + const originalFetch = globalThis.fetch; + let index = 0; + + globalThis.fetch = ((input: RequestInfo | URL) => { + const next = entries[index++]; + if (!next) { + throw new Error(`unexpected fetch for ${String(input)}`); + } + return Promise.resolve( + new Response(next.body == null ? "" : JSON.stringify(next.body), { + status: next.status, + headers: { "Content-Type": "application/json" }, + }), + ); + }) as typeof fetch; + + return () => { + globalThis.fetch = originalFetch; + }; +} + +function buildActionRequest(data: Record) { + const formData = new FormData(); + for (const [key, value] of Object.entries(data)) { + formData.set(key, value); + } + return new Request("http://fabro.test/runs/run-1", { + method: "POST", + body: formData, + }); +} + +describe("run-detail loader", () => { + let restoreFetch: (() => void) | undefined; + + afterEach(() => { + restoreFetch?.(); + restoreFetch = undefined; + delete (globalThis as { window?: unknown }).window; + }); + + test("loads the first blocked question when the run is blocked", async () => { + restoreFetch = stubFetchSequence([ + { + status: 200, + body: { + run_id: "run-1", + title: "Blocked run", + repository: { name: "repo" }, + status: "blocked", + workflow_name: "review", + }, + }, + { + status: 200, + body: { + data: [{ id: "q-1", text: "Ship this change?", stage: "review", question_type: "single_select", options: [], allow_freeform: false }], + meta: { has_more: false }, + }, + }, + ]); + + const result = await loader({ + request: new Request("http://fabro.test/runs/run-1"), + params: { id: "run-1" }, + }); + + expect(result.blockedQuestionText).toBe("Ship this change?"); + expect(result.run?.lifecycleStatus).toBe("blocked"); + }); + + test("falls back to null blockedQuestionText when no question is available", async () => { + restoreFetch = stubFetchSequence([ + { + status: 200, + body: { + run_id: "run-1", + title: "Blocked run", + repository: { name: "repo" }, + status: "blocked", + workflow_name: "review", + }, + }, + { + status: 200, + body: { + data: [], + meta: { has_more: false }, + }, + }, + ]); + + const result = await loader({ + request: new Request("http://fabro.test/runs/run-1"), + params: { id: "run-1" }, + }); + + expect(result.blockedQuestionText).toBeNull(); + }); +}); + +describe("run-detail action", () => { + let restoreFetch: (() => void) | undefined; + + afterEach(() => { + restoreFetch?.(); + restoreFetch = undefined; + delete (globalThis as { window?: unknown }).window; + }); + + test("preview still dispatches through intent=preview", async () => { + restoreFetch = stubFetchSequence([ + { + status: 200, + body: { url: "https://preview.example.com" }, + }, + ]); + + const result = await action({ + params: { id: "run-1" }, + request: buildActionRequest({ + intent: "preview", + port: "3000", + expires_in_secs: "3600", + }), + }); + + expect(result).toEqual({ + intent: "preview", + url: "https://preview.example.com", + }); + }); + + test("cancel dispatches through the lifecycle helper path", async () => { + restoreFetch = stubFetchSequence([ + { + status: 200, + body: { + id: "run-1", + status: "failed", + status_reason: "cancelled", + created_at: "2026-04-20T12:00:00Z", + }, + }, + ]); + + const result = await action({ + params: { id: "run-1" }, + request: buildActionRequest({ intent: "cancel" }), + }); + + expect(result).toEqual({ + intent: "cancel", + ok: true, + run: { + id: "run-1", + status: "failed", + status_reason: "cancelled", + created_at: "2026-04-20T12:00:00Z", + }, + }); + }); +}); + +describe("lifecycleActionVisibility", () => { + test("shows cancel for active cancellable states and hides it elsewhere", () => { + expect(lifecycleActionVisibility("submitted").showPrimaryCancel).toBe(true); + expect(lifecycleActionVisibility("queued").showPrimaryCancel).toBe(true); + expect(lifecycleActionVisibility("starting").showPrimaryCancel).toBe(true); + expect(lifecycleActionVisibility("running").showPrimaryCancel).toBe(true); + expect(lifecycleActionVisibility("paused").showPrimaryCancel).toBe(true); + expect(lifecycleActionVisibility("blocked").showPrimaryCancel).toBe(false); + expect(lifecycleActionVisibility("succeeded").showPrimaryCancel).toBe(false); + expect(lifecycleActionVisibility("failed").showPrimaryCancel).toBe(false); + expect(lifecycleActionVisibility("dead").showPrimaryCancel).toBe(false); + expect(lifecycleActionVisibility("archived").showPrimaryCancel).toBe(false); + }); + + test("shows archive and unarchive in the expected terminal states", () => { + expect(lifecycleActionVisibility("succeeded").showArchive).toBe(true); + expect(lifecycleActionVisibility("failed").showArchive).toBe(true); + expect(lifecycleActionVisibility("dead").showArchive).toBe(true); + expect(lifecycleActionVisibility("archived").showArchive).toBe(false); + expect(lifecycleActionVisibility("archived").showUnarchive).toBe(true); + expect(lifecycleActionVisibility("running").showUnarchive).toBe(false); + expect(lifecycleActionVisibility("blocked").showBlockedNotice).toBe(true); + }); +}); diff --git a/apps/fabro-web/app/routes/run-detail.tsx b/apps/fabro-web/app/routes/run-detail.tsx index 776689534..c5f9f87e2 100644 --- a/apps/fabro-web/app/routes/run-detail.tsx +++ b/apps/fabro-web/app/routes/run-detail.tsx @@ -1,12 +1,38 @@ import { useEffect } from "react"; -import { ChevronRightIcon } from "@heroicons/react/20/solid"; +import { ArrowPathIcon, ChevronRightIcon } from "@heroicons/react/20/solid"; import { Link, Outlet, useFetcher, useLocation } from "react-router"; -import { mapRunSummaryToRunItem, runStatusDisplay, isRunStatus } from "../data/runs"; -import type { RunSummaryResponse } from "../data/runs"; +import type { + ErrorResponseEntry, + PaginatedApiQuestionList, + PreviewUrlResponse, + RunStatusResponse, +} from "@qltysh/fabro-api-client"; + import { apiJson } from "../api"; +import { BlockedRunNotice } from "../components/blocked-run-notice"; import { ErrorState } from "../components/state"; +import { useToast } from "../components/toast"; +import { PRIMARY_BUTTON_CLASS, SECONDARY_BUTTON_CLASS } from "../components/ui"; +import { + isRunStatus, + mapRunSummaryToRunItem, + runStatusDisplay, + type RunSummaryResponse, +} from "../data/runs"; import { useDemoMode } from "../lib/demo-mode"; -import type { PreviewUrlResponse } from "@qltysh/fabro-api-client"; +import { useRunEventSource } from "../lib/sse"; +import { + archiveRun, + canArchive, + canCancel, + canUnarchive, + cancelRun, + isTerminalCancelledRun, + mapError, + type LifecycleAction, + type LifecycleActionError, + unarchiveRun, +} from "../lib/run-actions"; const allTabs = [ { name: "Overview", path: "", count: null, demoOnly: false }, @@ -18,17 +44,82 @@ const allTabs = [ export const handle = { hideHeader: true }; -export async function loader({ request, params }: any) { +const RUN_DETAIL_EVENTS = new Set([ + "run.submitted", + "run.queued", + "run.starting", + "run.running", + "run.paused", + "run.unpaused", + "run.blocked", + "run.unblocked", + "run.completed", + "run.failed", + "run.archived", + "run.unarchived", +]); + +const CANCEL_BUTTON_CLASS = + "inline-flex items-center justify-center gap-2 rounded-lg border border-coral/30 bg-coral/10 px-4 py-2 text-sm font-medium text-coral transition-colors hover:bg-coral/15 focus-visible:outline-2 focus-visible:outline-offset-2 focus-visible:outline-teal-500 disabled:cursor-not-allowed disabled:opacity-60 disabled:hover:bg-coral/10"; + +const MUTATION_BUTTON_CLASS = + `${SECONDARY_BUTTON_CLASS} disabled:cursor-not-allowed disabled:opacity-60`; + +type RunDetailRun = ReturnType & { + statusLabel: string; + statusDot: string; + statusText: string; +}; + +export interface RunDetailLoaderData { + run: RunDetailRun | null; + blockedQuestionText: string | null; +} + +type PreviewActionResult = { + intent: "preview"; + url: string; +}; + +type LifecycleActionResult = + | { + intent: LifecycleAction; + ok: true; + run: RunStatusResponse; + } + | { + intent: LifecycleAction; + ok: false; + error: LifecycleActionError | null; + }; + +export type RunDetailActionResult = PreviewActionResult | LifecycleActionResult; + +export function lifecycleActionVisibility(status: string | null | undefined) { + return { + showPrimaryCancel: canCancel(status), + showArchive: canArchive(status), + showUnarchive: canUnarchive(status), + showBlockedNotice: status === "blocked", + }; +} + +export async function loader({ request, params }: any): Promise { const response = await fetch(`/api/v1/runs/${params.id}`, { credentials: "include", + ...(request?.signal ? { signal: request.signal } : {}), }); - if (!response.ok) return { run: null }; + if (!response.ok) { + return { run: null, blockedQuestionText: null }; + } + const summary: RunSummaryResponse = await response.json(); const item = mapRunSummaryToRunItem(summary); const rawStatus = summary.status; const display = isRunStatus(rawStatus) ? runStatusDisplay[rawStatus] : { label: rawStatus, dot: "bg-fg-muted", text: "text-fg-muted" }; + return { run: { ...item, @@ -36,22 +127,39 @@ export async function loader({ request, params }: any) { statusDot: display.dot, statusText: display.text, }, + blockedQuestionText: + rawStatus === "blocked" + ? await loadBlockedQuestionText(params.id, request?.signal) + : null, }; } -export async function action({ params, request }: any) { +export async function action({ params, request }: any): Promise { const formData = await request.formData(); - const port = formData.get("port"); - const expiresInSecs = formData.get("expires_in_secs"); - const result = await apiJson(`/runs/${params.id}/preview`, { - request, - init: { - method: "POST", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({ port: Number(port), expires_in_secs: Number(expiresInSecs) }), - }, - }); - return result; + const intent = String(formData.get("intent") ?? "preview"); + + if (intent === "preview") { + const port = formData.get("port"); + const expiresInSecs = formData.get("expires_in_secs"); + const result = await apiJson(`/runs/${params.id}/preview`, { + request, + init: { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ port: Number(port), expires_in_secs: Number(expiresInSecs) }), + }, + }); + return { + intent: "preview", + url: result.url, + }; + } + + if (intent === "cancel" || intent === "archive" || intent === "unarchive") { + return runLifecycleIntent(params.id, intent, request); + } + + throw new Response(null, { status: 400, statusText: `Unsupported intent: ${intent}` }); } export function meta({ data }: any) { @@ -59,20 +167,69 @@ export function meta({ data }: any) { return [{ title: run ? `${run.title} — Fabro` : "Run — Fabro" }]; } -export default function RunDetail({ loaderData, params }: any) { - const { run } = loaderData; +export default function RunDetail({ loaderData, params }: { loaderData: RunDetailLoaderData; params: { id: string } }) { + const { run, blockedQuestionText } = loaderData; const { pathname } = useLocation(); const basePath = `/runs/${params.id}`; - const previewFetcher = useFetcher(); + const previewFetcher = useFetcher(); + const cancelFetcher = useFetcher(); + const archiveFetcher = useFetcher(); + const unarchiveFetcher = useFetcher(); + const { push } = useToast(); const demoMode = useDemoMode(); const tabs = allTabs.filter((t) => !t.demoOnly || demoMode); + useRunEventSource(run?.id ?? undefined, { + allowlist: RUN_DETAIL_EVENTS, + debounceMs: 300, + }); + useEffect(() => { - if (previewFetcher.data?.url) { + if (previewFetcher.data?.intent === "preview") { window.open(previewFetcher.data.url, "_blank"); } }, [previewFetcher.data]); + useEffect(() => { + const result = cancelFetcher.data; + if (!result || result.intent !== "cancel") return; + if (isLifecycleActionFailure(result)) { + push({ message: mapError(result.error, "cancel"), tone: "error" }); + return; + } + push({ + message: isTerminalCancelledRun(result.run) + ? "Run cancelled." + : "Cancellation requested.", + }); + }, [cancelFetcher.data, push]); + + useEffect(() => { + const result = archiveFetcher.data; + if (!result || result.intent !== "archive") return; + if (isLifecycleActionFailure(result)) { + push({ message: mapError(result.error, "archive"), tone: "error" }); + return; + } + push({ + message: "Run archived.", + action: { + label: "Unarchive", + onClick: () => submitIntent(unarchiveFetcher, "unarchive"), + }, + }); + }, [archiveFetcher, archiveFetcher.data, push, unarchiveFetcher]); + + useEffect(() => { + const result = unarchiveFetcher.data; + if (!result || result.intent !== "unarchive") return; + if (isLifecycleActionFailure(result)) { + push({ message: mapError(result.error, "unarchive"), tone: "error" }); + return; + } + push({ message: "Run restored." }); + }, [push, unarchiveFetcher.data]); + if (!run) { return (
@@ -84,6 +241,12 @@ export default function RunDetail({ loaderData, params }: any) { ); } + const visibility = lifecycleActionVisibility(run.lifecycleStatus); + const previewPending = previewFetcher.state !== "idle"; + const cancelPending = cancelFetcher.state !== "idle"; + const archivePending = archiveFetcher.state !== "idle"; + const unarchivePending = unarchiveFetcher.state !== "idle"; + return (
-
+

{run.title}

@@ -114,26 +277,76 @@ export default function RunDetail({ loaderData, params }: any) { )}
- {/* TODO: restore an Open PR button when RunPullRequest gains a url field */} - {run.sandboxId && ( - - - - - - )} + +
+ {visibility.showPrimaryCancel && ( + + + + + )} + + {visibility.showArchive && ( + + + + + )} + + {visibility.showUnarchive && ( + + + + + )} + + {run.sandboxId && ( + + + + + + + )} +
+ {visibility.showBlockedNotice && ( + submitIntent(cancelFetcher, "cancel")} + /> + )} +
); } + +async function loadBlockedQuestionText(id: string, signal?: AbortSignal): Promise { + try { + const url = new URL(`/api/v1/runs/${id}/questions`, "http://fabro.local"); + url.searchParams.set("page[limit]", "1"); + url.searchParams.set("page[offset]", "0"); + + const response = await fetch(`${url.pathname}${url.search}`, { + credentials: "include", + ...(signal ? { signal } : {}), + }); + if (!response.ok) { + return null; + } + + const payload = await response.json() as PaginatedApiQuestionList; + return payload.data[0]?.text ?? null; + } catch { + return null; + } +} + +async function runLifecycleIntent( + id: string, + intent: LifecycleAction, + request: Request, +): Promise { + try { + switch (intent) { + case "cancel": + return { intent, ok: true, run: await cancelRun(id, request) }; + case "archive": + return { intent, ok: true, run: await archiveRun(id, request) }; + case "unarchive": + return { intent, ok: true, run: await unarchiveRun(id, request) }; + } + } catch (error) { + return { + intent, + ok: false, + error: serializeLifecycleActionError(error), + }; + } +} + +function serializeLifecycleActionError(error: unknown): LifecycleActionError | null { + if (!error || typeof error !== "object") return null; + const record = error as Record; + if (typeof record.status !== "number" || !Array.isArray(record.errors)) { + return null; + } + + return { + status: record.status, + errors: record.errors.filter(isErrorResponseEntry), + }; +} + +function isErrorResponseEntry(value: unknown): value is ErrorResponseEntry { + if (!value || typeof value !== "object") return false; + const record = value as Record; + return ( + typeof record.status === "string" + && typeof record.title === "string" + && typeof record.detail === "string" + ); +} + +function isLifecycleActionFailure( + value: LifecycleActionResult, +): value is Extract { + return value.ok === false; +} + +function submitIntent( + fetcher: { submit: (target: FormData, options: { method: "post" }) => void }, + intent: LifecycleAction, +) { + const formData = new FormData(); + formData.set("intent", intent); + fetcher.submit(formData, { method: "post" }); +} diff --git a/apps/fabro-web/app/routes/run-files.test.ts b/apps/fabro-web/app/routes/run-files.test.ts index 649258893..1f9da4ec2 100644 --- a/apps/fabro-web/app/routes/run-files.test.ts +++ b/apps/fabro-web/app/routes/run-files.test.ts @@ -1,6 +1,11 @@ import { afterEach, describe, expect, test } from "bun:test"; -import { extractRequestId, loader } from "./run-files"; +import { + deepLinkToastMessage, + emptyTransitionToastMessage, + extractRequestId, + loader, +} from "./run-files"; type StubResponseInit = { status: number; @@ -8,6 +13,30 @@ type StubResponseInit = { headers?: Record; }; +function buildRunFilesPayload({ + files = [], + degraded = false, + patch = null, +}: { + files?: string[]; + degraded?: boolean; + patch?: string | null; +}) { + return { + data: files.map((name) => ({ + change_kind: "modified", + old_file: { name }, + new_file: { name }, + })), + meta: { + degraded, + patch, + total_changed: files.length, + truncated: false, + }, + } as any; +} + function stubFetchOnce(init: StubResponseInit) { const original = globalThis.fetch; globalThis.fetch = (() => { @@ -76,6 +105,50 @@ describe("extractRequestId", () => { }); }); +describe("emptyTransitionToastMessage", () => { + test("returns the no-changes toast when a populated diff becomes empty", () => { + expect(emptyTransitionToastMessage(3, 0)).toBe("No changes in this run."); + }); + + test("returns null when the diff was already empty", () => { + expect(emptyTransitionToastMessage(0, 0)).toBeNull(); + expect(emptyTransitionToastMessage(null, 0)).toBeNull(); + expect(emptyTransitionToastMessage(2, 1)).toBeNull(); + }); +}); + +describe("deepLinkToastMessage", () => { + test("returns the patch-only message when file navigation is unavailable", () => { + expect( + deepLinkToastMessage( + "src/app.tsx", + buildRunFilesPayload({ + degraded: true, + patch: "@@ -1 +1 @@", + }), + ), + ).toBe("File-level navigation isn't available in the patch-only view."); + }); + + test("returns the missing-file message when the requested file is absent", () => { + expect( + deepLinkToastMessage( + "src/missing.ts", + buildRunFilesPayload({ files: ["src/present.ts"] }), + ), + ).toBe("File src/missing.ts is not in this run."); + }); + + test("returns null when the deep-linked file exists", () => { + expect( + deepLinkToastMessage( + "src/present.ts", + buildRunFilesPayload({ files: ["src/present.ts"] }), + ), + ).toBeNull(); + }); +}); + describe("loader", () => { let restoreFetch: (() => void) | undefined; diff --git a/apps/fabro-web/app/routes/run-files.tsx b/apps/fabro-web/app/routes/run-files.tsx index 49b16e58f..e3a5b96da 100644 --- a/apps/fabro-web/app/routes/run-files.tsx +++ b/apps/fabro-web/app/routes/run-files.tsx @@ -4,6 +4,7 @@ import { useRef, useState, type ReactElement, + type ReactNode, } from "react"; import { useMatches, @@ -11,7 +12,8 @@ import { useParams, useRevalidator, } from "react-router"; -import { MultiFileDiff, PatchDiff, Virtualizer } from "@pierre/diffs/react"; +import * as PierreDiffs from "@pierre/diffs/react"; +import { useToast } from "../components/toast"; import type { FileDiff as ApiFileDiff, PaginatedRunFileList, @@ -27,10 +29,18 @@ import { LoadingSkeleton, renderStatusError, RunFilesErrorBoundary, - Toast, } from "./run-files/states"; import { useFileKeyboardNav } from "./run-files/keyboard"; import { Toolbar, type DiffStyle } from "./run-files/toolbar"; +import { useRunEventSource } from "../lib/sse"; + +const { MultiFileDiff, PatchDiff } = PierreDiffs; +const maybeVirtualizer = (PierreDiffs as Record).Virtualizer; +const Virtualizer = typeof maybeVirtualizer === "function" + ? maybeVirtualizer as ({ children }: { children: ReactNode }) => ReactElement + : function VirtualizerFallback({ children }: { children: ReactNode }) { + return <>{children}; + }; export const handle = { wide: true }; @@ -156,28 +166,10 @@ function useNarrowViewport(): boolean { } function useSseRevalidation(runId: string | undefined) { - const revalidator = useRevalidator(); - useEffect(() => { - if (!runId) return; - const source = new EventSource(`/api/v1/runs/${runId}/attach?since_seq=1`); - let debounce: ReturnType | undefined; - source.onmessage = (msg) => { - try { - const payload = JSON.parse(msg.data); - if (REFRESH_EVENTS.has(payload.event)) { - clearTimeout(debounce); - debounce = setTimeout(() => revalidator.revalidate(), 500); - } - } catch { - // ignore malformed payloads - } - }; - return () => { - clearTimeout(debounce); - source.close(); - }; - // eslint-disable-next-line react-hooks/exhaustive-deps - }, [runId]); + useRunEventSource(runId, { + allowlist: REFRESH_EVENTS, + debounceMs: 500, + }); } function useFreshness( @@ -260,6 +252,47 @@ function decodeDeepLinkFile(hash: string): string | null { } } +export function emptyTransitionToastMessage( + previousFileCount: number | null, + nextFileCount: number, +): string | null { + return previousFileCount !== null && previousFileCount > 0 && nextFileCount === 0 + ? "No changes in this run." + : null; +} + +function resolveDeepLinkToast( + hashFile: string | null, + data: PaginatedRunFileList | null, +): { key: string; message: string } | null { + if (!hashFile || !data) return null; + if (data.meta.degraded && data.meta.patch) { + return { + key: `patch-only:${hashFile}`, + message: "File-level navigation isn't available in the patch-only view.", + }; + } + + const exists = data.data.some( + (file) => file.new_file.name === hashFile || file.old_file.name === hashFile, + ); + if (!exists) { + return { + key: `missing:${hashFile}`, + message: `File ${hashFile} is not in this run.`, + }; + } + + return null; +} + +export function deepLinkToastMessage( + hashFile: string | null, + data: PaginatedRunFileList | null, +): string | null { + return resolveDeepLinkToast(hashFile, data)?.message ?? null; +} + /** * Extract the lifecycle status from whichever ancestor match carries it. * The Run Detail loader (apps/fabro-web/app/routes/run-detail.tsx) returns @@ -288,6 +321,7 @@ export default function RunFiles({ loaderData }: any) { const navigation = useNavigation(); const revalidator = useRevalidator(); const matches = useMatches(); + const { push } = useToast(); const result = loaderData as RunFilesLoaderResult | null; const narrow = useNarrowViewport(); const runStatus = resolveRunStatus(matches); @@ -296,23 +330,19 @@ export default function RunFiles({ loaderData }: any) { // rendering the previous files while surfacing an inline banner. const lastGoodDataRef = useRef(null); const lastFetchedAtRef = useRef(null); - const [emptyToast, setEmptyToast] = useState(null); - const [deepLinkToast, setDeepLinkToast] = useState(null); useEffect(() => { if (!result?.data) return; - const prev = lastGoodDataRef.current; - if (prev && prev.data.length > 0 && result.data.data.length === 0) { - setEmptyToast("No changes in this run."); - const id = setTimeout(() => setEmptyToast(null), 3500); - lastGoodDataRef.current = result.data; - lastFetchedAtRef.current = Date.now(); - return () => clearTimeout(id); + const message = emptyTransitionToastMessage( + lastGoodDataRef.current?.data.length ?? null, + result.data.data.length, + ); + if (message) { + push({ message }); } lastGoodDataRef.current = result.data; lastFetchedAtRef.current = Date.now(); - return undefined; - }, [result?.data]); + }, [push, result?.data]); const data: PaginatedRunFileList | null = result?.data ?? lastGoodDataRef.current; @@ -352,6 +382,7 @@ export default function RunFiles({ loaderData }: any) { const refreshButtonRef = useRef(null); const containerRef = useRef(null); + const lastDeepLinkToastRef = useRef(null); // Return focus to the Refresh button after a revalidation completes so // keyboard-first users stay oriented. @@ -384,28 +415,22 @@ export default function RunFiles({ loaderData }: any) { }, []); useEffect(() => { + const toast = resolveDeepLinkToast(hashFile, data); + if (toast) { + if (lastDeepLinkToastRef.current !== toast.key) { + push({ message: toast.message, autoDismissMs: 5000 }); + lastDeepLinkToastRef.current = toast.key; + } + return; + } + lastDeepLinkToastRef.current = null; if (!hashFile || !data) return; - if (data.meta.degraded && data.meta.patch) { - setDeepLinkToast( - "File-level navigation isn't available in the patch-only view.", - ); - const id = setTimeout(() => setDeepLinkToast(null), 5000); - return () => clearTimeout(id); - } - const exists = data.data.some( - (f) => f.new_file.name === hashFile || f.old_file.name === hashFile, - ); - if (!exists) { - setDeepLinkToast(`File ${hashFile} is not in this run.`); - const id = setTimeout(() => setDeepLinkToast(null), 5000); - return () => clearTimeout(id); - } const el = document.getElementById(fileRowId(hashFile)); if (el) { el.scrollIntoView({ block: "start", behavior: "smooth" }); el.focus({ preventScroll: true }); } - }, [hashFile, data]); + }, [data, hashFile, push]); const renderFiles = useCallback( (files: ApiFileDiff[]): ReactElement[] => @@ -526,8 +551,6 @@ export default function RunFiles({ loaderData }: any) { theme: "pierre-dark", }} /> - {emptyToast && {emptyToast}} - {deepLinkToast && {deepLinkToast}}
); } @@ -543,8 +566,6 @@ export default function RunFiles({ loaderData }: any) { degraded: meta.degraded ?? false, })} /> - {emptyToast && {emptyToast}} - {deepLinkToast && {deepLinkToast}}
); } @@ -568,8 +589,6 @@ export default function RunFiles({ loaderData }: any) { /> ) : null} {body} - {emptyToast && {emptyToast}} - {deepLinkToast && {deepLinkToast}}
); } diff --git a/apps/fabro-web/app/routes/run-files/pierre-smoke.test.tsx b/apps/fabro-web/app/routes/run-files/pierre-smoke.test.tsx index c7a13d1d0..90373eb48 100644 --- a/apps/fabro-web/app/routes/run-files/pierre-smoke.test.tsx +++ b/apps/fabro-web/app/routes/run-files/pierre-smoke.test.tsx @@ -1,12 +1,20 @@ import { describe, expect, test } from "bun:test"; -import { MultiFileDiff, PatchDiff, Virtualizer } from "@pierre/diffs/react"; +import type { ReactNode } from "react"; +import * as PierreDiffs from "@pierre/diffs/react"; + +const { MultiFileDiff, PatchDiff } = PierreDiffs; +const maybeVirtualizer = (PierreDiffs as Record).Virtualizer; +const Virtualizer = typeof maybeVirtualizer === "function" + ? maybeVirtualizer + : function VirtualizerFallback({ children }: { children: ReactNode }) { + return <>{children}; + }; // Regression coverage for the @pierre/diffs 1.0 -> 1.1 upgrade. We assert -// only that the public React components the Run Files route uses remain -// exported as callable function components — a full mount-under-test hits -// pierre's useLayoutEffect teardown path, which is incompatible with -// react-test-renderer under React 19. Functional mount coverage lives in -// the dev-server smoke check. +// only that the React components the Run Files route consumes remain +// callable. `Virtualizer` is optional across installed pierre versions, so +// the route carries a no-op fallback and the smoke test mirrors that +// compatibility layer rather than assuming a specific package export set. describe("@pierre/diffs public API", () => { test("MultiFileDiff is a callable component export", () => { diff --git a/apps/fabro-web/app/routes/run-files/states.test.tsx b/apps/fabro-web/app/routes/run-files/states.test.tsx index 5a222e522..e8fe40eb1 100644 --- a/apps/fabro-web/app/routes/run-files/states.test.tsx +++ b/apps/fabro-web/app/routes/run-files/states.test.tsx @@ -8,7 +8,6 @@ import { EmptyState, InlineErrorBanner, LoadingSkeleton, - Toast, } from "./states"; function renderToJson(element: React.ReactElement): any { @@ -178,15 +177,4 @@ describe("component rendering", () => { }); expect(clicked).toBe(1); }); - - test("Toast renders its children in an aria-live region", () => { - let tree: TestRenderer.ReactTestRenderer | undefined; - TestRenderer.act(() => { - tree = TestRenderer.create(hello); - }); - const live = tree!.root.findAll( - (node) => node.props?.["aria-live"] === "polite", - ); - expect(live.length).toBeGreaterThan(0); - }); }); diff --git a/apps/fabro-web/app/routes/run-files/states.tsx b/apps/fabro-web/app/routes/run-files/states.tsx index a44f2ac7b..fc1966292 100644 --- a/apps/fabro-web/app/routes/run-files/states.tsx +++ b/apps/fabro-web/app/routes/run-files/states.tsx @@ -128,18 +128,6 @@ export function InlineErrorBanner({ ); } -export function Toast({ children }: { children: React.ReactNode }) { - return ( -
- {children} -
- ); -} - /** * Shared helper for rendering the documented status-code taxonomy. Consumed * by both the inline `initialError` branch in run-files.tsx and the diff --git a/apps/fabro-web/app/routes/runs.test.tsx b/apps/fabro-web/app/routes/runs.test.tsx index 9c15a9c95..0cf1c93a9 100644 --- a/apps/fabro-web/app/routes/runs.test.tsx +++ b/apps/fabro-web/app/routes/runs.test.tsx @@ -43,6 +43,8 @@ describe("runs route board mapping", () => { expect(shouldRefreshBoardForEvent("run.queued")).toBe(true); expect(shouldRefreshBoardForEvent("run.blocked")).toBe(true); expect(shouldRefreshBoardForEvent("run.unblocked")).toBe(true); + expect(shouldRefreshBoardForEvent("run.archived")).toBe(true); + expect(shouldRefreshBoardForEvent("run.unarchived")).toBe(true); expect(shouldRefreshBoardForEvent("interview.started")).toBe(true); expect(shouldRefreshBoardForEvent("interview.completed")).toBe(true); expect(shouldRefreshBoardForEvent("run.created")).toBe(false); diff --git a/apps/fabro-web/app/routes/runs.tsx b/apps/fabro-web/app/routes/runs.tsx index ad63af0c6..5e184ec42 100644 --- a/apps/fabro-web/app/routes/runs.tsx +++ b/apps/fabro-web/app/routes/runs.tsx @@ -72,6 +72,8 @@ const BOARD_STATUS_EVENTS = new Set([ "run.unblocked", "run.completed", "run.failed", + "run.archived", + "run.unarchived", "interview.started", "interview.completed", "interview.timeout", diff --git a/docs/administration/server-configuration.mdx b/docs/administration/server-configuration.mdx index 0ce3714c6..bd444ed98 100644 --- a/docs/administration/server-configuration.mdx +++ b/docs/administration/server-configuration.mdx @@ -211,7 +211,7 @@ Customize the git author identity used for checkpoint commits. When not set, def ### `[server.integrations.github]` section -Configure GitHub integration auth. `strategy = "token"` is the default and uses a stored `GITHUB_TOKEN` from the vault (with `GH_TOKEN` as a fallback). `strategy = "app"` enables the GitHub App flow, browser OAuth, and webhooks. +Configure GitHub integration auth. `strategy = "token"` is the default and uses a stored `GITHUB_TOKEN` from the vault (with `GH_TOKEN` as a fallback). `strategy = "app"` enables the GitHub App flow and browser OAuth; webhook delivery is configured separately under `[server.integrations.github.webhooks]`. ```toml title="settings.toml" [server.integrations.github] @@ -227,11 +227,20 @@ app_id = "123456" client_id = "Iv1.abc123" slug = "fabro-app" +[server.api] +url = "https://fabro-api.example.com" + [server.integrations.github.webhooks] -strategy = "tailscale_funnel" +strategy = "server_url" ``` -When `webhooks.strategy = "tailscale_funnel"` is configured, `fabro server start` binds a local HTTP listener, exposes it through `tailscale funnel`, and updates the GitHub App's webhook URL on startup. Incoming webhooks are verified with HMAC-SHA256. Requires the `GITHUB_APP_WEBHOOK_SECRET` environment variable. +Fabro always serves the GitHub webhook handler at `POST /api/v1/webhooks/github` when `GITHUB_APP_WEBHOOK_SECRET` is configured. The `strategy` field controls how Fabro exposes that route and whether it mutates the GitHub App webhook URL on startup: + +- `strategy = "server_url"`: recommended for production or any deployment with a stable public API URL. Fabro sets the GitHub App webhook URL to `/api/v1/webhooks/github` on startup. Requires `server.api.url` and `GITHUB_APP_WEBHOOK_SECRET`. +- `strategy = "tailscale_funnel"`: opt-in for Tailscale-hosted machines without a stable public URL. Fabro runs `tailscale funnel `, exposes the main server on that Funnel URL, and best-effort updates the GitHub App webhook URL to `/api/v1/webhooks/github`. Requires a TCP listener and `GITHUB_APP_WEBHOOK_SECRET`. +- `strategy` unset: Fabro still accepts signed webhook deliveries on `/api/v1/webhooks/github` when the secret is present, but it does not run `tailscale funnel` and does not update the GitHub App webhook URL. + +Incoming webhooks are authenticated only by GitHub's `X-Hub-Signature-256` HMAC signature, not by Fabro's bearer/session auth. ### `[run.checkpoint]` section diff --git a/docs/api-reference/fabro-api.yaml b/docs/api-reference/fabro-api.yaml index 1b9cb3a1f..01876c9fe 100644 --- a/docs/api-reference/fabro-api.yaml +++ b/docs/api-reference/fabro-api.yaml @@ -9,6 +9,8 @@ tags: description: API discovery and health - name: Install description: First-run browser install workflow + - name: Integrations + description: External provider callbacks and integration endpoints - name: Runs description: Run management operations - name: Human-in-the-Loop @@ -374,6 +376,26 @@ paths: schema: type: object + /api/v1/webhooks/github: + post: + operationId: receiveGithubWebhook + tags: [Integrations] + summary: Receive GitHub Webhook + description: Receives GitHub App webhook deliveries. Requests are authenticated by `X-Hub-Signature-256`, not API bearer auth. + security: [] + requestBody: + required: true + content: + application/json: + schema: + type: object + additionalProperties: true + responses: + "200": + description: Webhook accepted + "401": + description: Missing or invalid webhook signature + /api/v1/user: get: operationId: getUser diff --git a/docs/brainstorms/2026-04-19-web-ui-lifecycle-actions-requirements.md b/docs/brainstorms/2026-04-19-web-ui-lifecycle-actions-requirements.md new file mode 100644 index 000000000..f71f4a350 --- /dev/null +++ b/docs/brainstorms/2026-04-19-web-ui-lifecycle-actions-requirements.md @@ -0,0 +1,126 @@ +--- +date: 2026-04-19 +topic: web-ui-lifecycle-actions +--- + +# Expose CLI Lifecycle Actions in the Web UI + +## Problem Frame + +The Fabro web UI today is essentially read-only for run management. The only mutating action exposed is **Preview** (opens a sandbox port URL). Every other run lifecycle action — cancelling a stuck run, cleaning up the board, revisiting archived runs — requires dropping to the CLI. + +Two concrete user problems drive this work: + +1. **Daily friction for CLI users.** People live in the board view and detail pages but have to context-switch to a terminal to manage state. +2. **Excludes non-CLI teammates.** PMs, reviewers, and stakeholders can watch runs but cannot participate in managing them, cutting the UI off as a collaboration surface. + +The web UI should own the **everyday lifecycle operations** these users hit. Rarer or more dangerous CLI verbs (force-delete of active runs, checkpoint ops, etc.) can remain CLI-only by design; the goal is user value per surface, not parity for parity's sake. + +Most server infrastructure already exists — `POST /runs/{id}/{cancel,archive,unarchive}` are live, wired to the workflow engine's operations, and emit SSE events. The remaining work is UI surface + a handful of concrete SSE reconciliation gaps (see Dependencies). + +## Requirements + +**Action set** +- R1. Expose three lifecycle actions on the run detail page (`/runs/{id}`): **cancel**, **archive**, **unarchive**. +- R2. Do not expose `pause`, `unpause`, or `delete` in this first pass. Pause/unpause has no evidenced daily-user need; delete's tab-close-mid-undo failure mode is unacceptable for a non-CLI teammate because observability is destroyed (there's no run to revisit to self-verify). +- R3. Do not expose checkpoint operations (resume, rewind, fork) or HITL question answering in this first pass. +- R4. Do not expose these actions on the board kanban cards or as bulk selection in this first pass. + +**State-aware visibility** +- R5. Only show an action when the run's current status makes it valid: + - **cancel (primary)**: visible as a primary affordance when status is `submitted`, `queued`, `starting`, `running`, or `paused`. Not shown as primary when `blocked` — see R6. The server also accepts cancel on `blocked` runs; that path is only reachable via the secondary surface from R6. + - **archive**: visible only when status is terminal (`succeeded`, `failed`, `dead`) AND not already archived. + - **unarchive**: visible only when `archived`. +- R6. When a run is `blocked` (waiting on an HITL question), do not show cancel as a primary action. Instead, show an inline notice with the pending question text (from `GET /api/v1/runs/{id}/questions`, whose `ApiQuestion.text` field is already human-readable) and the instruction: "Answer this question via `fabro` CLI to continue." Cancel remains reachable via an overflow/secondary affordance (e.g., a "…" menu) for users who truly want to abandon the run. This protects the common case (non-CLI teammate accidentally cancelling work that was waiting for them) without fully hiding the escape hatch. +- R7. Visibility updates live when the run status changes. The detail page must subscribe to the run's SSE event stream (`GET /runs/{id}/attach`) so action affordances appear/disappear without a manual refresh. + +**Interaction & feedback** + +Two toast patterns, matched to the reversibility of each action: + +- R8. **Cancel uses a client-side deferred toast with a fixed 5-second countdown.** Cancel has partially-irreversible side effects (the agent stops mid-stage) so the undo window guards against misclicks. + - Clicking cancel shows a toast with the action description, a 5-second countdown, and an **Undo** button. The cancel affordance enters a disabled/pending state during the window (not hidden — hiding mid-window misrepresents actual run state). + - If the user clicks **Undo** before the countdown expires, the pending client-side timer is cancelled and no API call is made. + - If the countdown expires, the client fires the API call. **Before firing**, the UI performs a `GET /api/v1/runs/{id}` refetch: if the run's status is no longer one where cancel is valid (e.g., the run completed or was cancelled elsewhere), the pending action is aborted and a brief "Run transitioned — cancel aborted" notice is shown instead. This covers the SSE-channel-unreachable case where R9's event-driven abort cannot fire. + - If the tab is closed or navigated away (including SPA navigation to another route) mid-window, the pending action is silently cancelled. Accepted tradeoff of the client-only approach. + +- R9. **Archive and unarchive fire immediately with an inverse-action toast.** Both are fully reversible by the opposite API call, so the 5-second countdown is overhead with no safety benefit. Gmail-archive shape: + - Clicking archive fires `POST /runs/{id}/archive` immediately (optimistic UI: the run disappears from terminal views / moves to archived views right away). + - On success, show a toast: "Run archived. **Unarchive**" with a visible action button. The toast remains dismissable for ~8 seconds. + - Clicking **Unarchive** in the toast fires `POST /runs/{id}/unarchive`. The toast updates to "Run restored" briefly, then dismisses. + - Unarchive-triggered-from-the-primary-affordance works identically, with the inverse verb. + +- R10. **Undo-window collisions (cancel only).** While the cancel timer is pending for a run, other primary affordances for that run are disabled (not hidden). If an SSE event arrives for that run that would change its status (another tab, a CLI user, the run finishing naturally), the pending client-side timer is cancelled, the toast is dismissed with a brief notice ("Run transitioned — action cancelled"), and the UI reconciles to the new status. If a newly-valid action (e.g., archive becomes valid because the run just completed) results from the transition, its affordance lights up immediately rather than waiting for the toast to fully dismiss. + +- R11. On a successful async cancel (status unchanged in the response body), the UI relies on SSE for the final status flip. No additional success toast beyond the deferred-toast already shown. + +- R12. On API failure (including 409 precondition failures — e.g., the run transitioned out of a valid state between toast-expiry and API call), show an error toast that includes the server's error message, and refetch the run so the UI reconciles to actual state. For the archive/unarchive fire-immediately path, additionally roll back the optimistic UI change on failure. + +- R13. **Multi-tab behavior (single-client rules apply per tab).** R8/R10 are scoped per-client: each tab runs its own timer. An SSE-delivered status transition in tab B (caused by tab A firing cancel) will cancel tab B's own pending timer for the same run per R10 and surface the transition notice. Cross-tab action coordination beyond what SSE already provides is not a requirement for this pass. + +**Accessibility** +- R14. Both toast patterns (deferred cancel toast; immediate archive/unarchive toast) must meet baseline a11y expectations: + - Toast container uses `role="status"` with `aria-live="polite"` (assertive interrupts screen-reader output and is wrong here). Announcement names the action, e.g., "Cancel run requested, undoing in 5 seconds" or "Run archived. Press Unarchive to undo." + - Toast does **not** steal focus, but is reachable via keyboard (tab order places the action button — Undo or Unarchive — immediately after the triggering affordance). + - For the cancel deferred toast, while keyboard focus is on the **Undo** button, the 5-second countdown **pauses** and resumes counting down from the paused value when focus leaves. Focus leaving the Undo button after the countdown would have expired does not auto-fire the action — the user must still explicitly close the toast or navigate away for the countdown to resume its final tick. + - Archive/unarchive toasts do not count down (they fire on click); they remain dismissable and focusable for ~8 seconds. + - Touch targets (Undo / Unarchive buttons) meet 44×44 CSS px minimum. The detail page is expected to work on tablet and larger; phone-size support is not a requirement for this pass. + - The action cluster is keyboard-operable end to end (no mouse-only affordances). Keyboard shortcuts for individual actions are out of scope for this pass. + +## Success Criteria + +- A user managing runs day-to-day can complete a full session (cancelling a stuck run, archiving finished ones, revisiting an archived one) without touching the CLI. +- A non-CLI teammate can cancel or archive a run in the web UI without onboarding documentation beyond "click the button." +- A non-CLI teammate who lands on a `blocked` run understands the run is waiting on a human answer, sees the question text, and does not accidentally cancel work in progress. **Note:** unblocking the teammate so they can actually *answer* the question requires HITL-answering in the web UI, which is deferred — this first pass prevents destruction, not participation. If blocked-teammate-can't-proceed proves to be a real painful pattern in usage, HITL answering should be the next scope to pick up. +- When a run transitions state while the detail page is open (e.g., finishes, gets archived from the CLI, gets cancelled by someone else), the available actions update live without a page refresh. +- The cancel deferred-toast component (toast container + Undo + pause-on-focus countdown + polite aria-live) is shaped so it can host future destructive actions (delete if we solve the observability problem; force-cancel if ever added) without redesign. The archive/unarchive immediate-inverse toast is a simpler shape that other reversible fire-and-forget actions can reuse. + +## Scope Boundaries + +**Out of scope for this first pass:** +- `pause` and `unpause` (no evidenced daily-user pain for the target personas; revisit if a real workflow surfaces). +- `delete` (tab-close-silently-cancels destroys observability — there's no run to revisit to self-verify — which is unacceptable for the non-CLI teammate persona; revisit with server-side soft-delete or a different UX). +- Force-delete of active runs (`rm --force`). +- Checkpoint operations: `resume`, `rewind`, `fork`. These need parameter input (which checkpoint? which branch?) that doesn't fit the uniform button+toast pattern. +- Answering HITL questions from the web UI. R6 surfaces the question read-only and points to the CLI. +- Board-card actions and bulk multi-select on the board. +- Dense table/list view of runs. +- Server-side deferred/pending states. +- Keyboard shortcuts for individual actions. +- Role-based authorization (assumed unchanged from today; all authenticated users can take all actions). +- Phone-size responsive layout. + +## Key Decisions + +- **Cancel + archive + unarchive only.** Cancel addresses the clearest daily pain (stuck runs). Archive/unarchive addresses board clutter and is fully reversible by the opposite action — lowest-risk place to validate the interaction pattern. Pause/unpause are deferred for lack of evidenced need; delete is deferred because its client-side-undo failure mode destroys observability for the non-CLI teammate persona. +- **Run detail page only, not the board.** Keep the first pass tight. +- **Two toast patterns matched to reversibility, not one uniform pattern.** Cancel is partially irreversible → deferred-toast with a bound 5-second countdown. Archive/unarchive are fully reversible via the opposite API call → fire-immediately with an inverse-action toast (Gmail-archive shape). This is more spec than "one pattern for everything" but maps to a real property of the actions and eliminates a pointless friction tax on the reversible ones. The shared toast infrastructure (container, action-button slot, aria-live, keyboard reachability) is reused across both; only the cancel path carries the countdown + focus-pause machinery. +- **Pre-fire status recheck on the deferred cancel path.** At countdown expiry, the client refetches `GET /runs/{id}` before firing the cancel API call. This covers the SSE-unreachable failure mode where R10's event-driven abort cannot fire: without the recheck, a dead SSE channel would silently degrade to "timer fires, 409 error, user sees error toast for a race they didn't create." One extra GET per cancel is a cheap premium for reliable UX. +- **Blocked runs suppress cancel as a primary affordance and show the pending question.** Protects non-CLI teammates from the worst failure mode (cancelling work that was waiting for them). Cancel remains reachable via a secondary/overflow affordance for users who genuinely want to abandon. This solves the destruction problem but leaves the participation problem for HITL-in-the-web to solve later. +- **Client-side deferral, not server-side.** Chosen on shipping speed; does not add new lifecycle states. Accepted tradeoff: the "undo" promise is soft — tab close or SPA nav silently cancels. This is tolerable precisely because the action with the worst silent-cancellation consequence (delete) was scoped out. +- **Accessibility is in the requirements, not deferred to implementation.** ARIA semantics, keyboard reachability, focus-pauses-countdown, touch-target sizing, and aria-live politeness are specified rather than left as "standard best practices." +- **Pattern reuse is claimed only within this action family.** We do not claim "adding resume/rewind/fork/HITL later is same shape, new button." Those need parameter input (checkpoint choice, answer text) that the button+toast pattern doesn't host. That's fine — these patterns are for fire-and-forget lifecycle mutations, and the rest of the verbs will need their own UX. + +## Dependencies / Assumptions + +- The existing API endpoints (`POST /runs/{id}/{cancel,archive,unarchive}`) are stable and will not require spec changes. Verified against `docs/api-reference/fabro-api.yaml`. +- The generated TypeScript client in `lib/packages/fabro-api-client` exposes (or will trivially expose after regeneration) methods for these endpoints. +- **SSE coverage is not uniform.** The server emits `run.*` events for status transitions including `run.archived` / `run.unarchived` (`fabro-workflow/src/event.rs`, `fabro-server/src/server.rs`). Known gaps the plan must address: + - The board's event allowlist (`apps/fabro-web/app/routes/runs.tsx` `BOARD_STATUS_EVENTS`) does not currently include `run.archived` / `run.unarchived`. Adding them is in scope. + - The per-run `/attach` SSE stream terminates on `RunCompleted` / `RunFailed`. Archive/unarchive events fire on already-terminal runs, so the detail page must either reconnect to a non-terminating channel, refetch on the successful archive/unarchive response, or listen at a layer above `/attach`. Planning should pick an approach. +- **Run detail page is not yet SSE-subscribed.** `apps/fabro-web/app/routes/run-detail.tsx` currently fetches the run once via the React Router loader and does not subscribe to `/api/v1/runs/{id}/attach`. Wiring this subscription at the detail-page level (the owner of `run.status` that drives R5 visibility) is net-new work for R7. Individual tab components (stage-sidebar, run-files) already have per-run SSE subscriptions that can be used as a pattern. +- **No undo-capable toast system exists yet.** The only Toast in `apps/fabro-web` is a read-only live-region banner (`apps/fabro-web/app/routes/run-files/states.tsx`) with local `useState`/`setTimeout`. R8 + R9 + R12 together require a shared toast component with: a countdown, action-button slot, programmatic dismiss, multi-toast coexistence, polite aria-live, and focus-pauses-countdown behavior. This is net-new UI infrastructure. +- **Cancel semantics.** For `submitted` and `queued` runs, cancel synchronously flips lifecycle `status` to `failed` with `status_reason: cancelled` and returns that on the response. For `starting`/`running`/`blocked`/`paused` runs, cancel returns 200 with unchanged status and the transition lands asynchronously via the workflow engine. The UI should treat cancel as "request accepted" and rely on SSE for the final status flip — R10 covers this implicitly, but the plan should make the optimistic-UI behavior explicit (e.g., the cancel affordance stays in its disabled/pending state until either the response body carries the synchronous `failed`/`cancelled` result or the SSE-driven reconciliation arrives). +- **Per-run `/attach` stream has silent termination paths beyond the terminal-event case.** `attach_event_is_terminal` only matches `RunCompleted | RunFailed`, but the task that drives the stream can also exit without a terminal marker if the store read errors, if the run projection becomes non-active mid-replay, or if cancel lands on a queued run that never transitioned to running (covered by the `cancel_before_run_transitions_to_running_returns_empty_attach_stream` test in `fabro-server/src/server.rs`). R7 and R10 must therefore not assume a terminal event will always land: the plan needs a fallback refetch path for "SSE stream ended without a terminal marker" and for "SSE channel unreachable during an undo window." +- Authorization is a non-issue today (single-user / trusted deployment assumption). If multi-tenant auth lands, the action affordances will need to respect it, but that's a separate workstream. + +## Outstanding Questions + +### Deferred to Planning +- [Affects R8][Design] Exact visual placement of the action cluster in the detail page header: single "Actions" dropdown vs. inline buttons vs. split primary+overflow. Behavior is specified here; visual placement resolves in design/implementation. Consider how cancel-on-blocked lives in the overflow while the primary slot is occupied by the R6 inline notice. +- [Affects R11][Technical] Confirm the 409 error-body shape from the server so error-toast copy can use the server-provided message verbatim. +- [Affects R7, R10][Technical] Decide the post-terminal reconciliation mechanism for archive/unarchive: reconnect SSE after terminal close, refetch on 2xx archive/unarchive response, or subscribe on a non-terminating channel. Either the per-run `/attach` contract extends or the UI uses response-driven reconciliation. + +## Next Steps + +→ `/ce:plan` for structured implementation planning diff --git a/docs/changelog/2026-04-18.mdx b/docs/changelog/2026-04-18.mdx index caf21e39a..f9acfedf2 100644 --- a/docs/changelog/2026-04-18.mdx +++ b/docs/changelog/2026-04-18.mdx @@ -19,6 +19,14 @@ The published Docker image switched to [Docker Hardened Images](https://www.dock A new `docker-compose.prod.yaml` stands up a Caddy 2 sidecar that handles auto-HTTPS on ports 80/443 and proxies to the Fabro service. Set `FABRO_DOMAIN` to your domain and Caddy provisions and renews the certificate; certs persist in a named volume. The base `docker-compose.yaml` has moved to the repo root. +## GitHub webhook exposure is now explicit + +GitHub App webhook exposure no longer auto-enables just because `[server.integrations.github.webhooks]` exists. The webhook handler now lives on the main API router at `POST /api/v1/webhooks/github`, and operators must choose an explicit strategy if they want Fabro to mutate any external state on startup. + +- Breaking: if you previously relied on Fabro silently starting `tailscale funnel` and rewriting the GitHub App webhook URL, add `strategy = "tailscale_funnel"` under `[server.integrations.github.webhooks]` to restore that behavior. +- New: `strategy = "server_url"` tells Fabro to use `server.api.url` and set the GitHub App webhook URL to `/api/v1/webhooks/github` on startup. This is the recommended choice for stable production deployments behind HTTPS. +- Unset `strategy`: Fabro still verifies and serves signed GitHub webhooks when `GITHUB_APP_WEBHOOK_SECRET` is present, but it will not run `tailscale funnel` and it will not rewrite the GitHub App webhook URL. + ## More diff --git a/docs/integrations/github.mdx b/docs/integrations/github.mdx index 201a29d72..cb9297732 100644 --- a/docs/integrations/github.mdx +++ b/docs/integrations/github.mdx @@ -6,7 +6,7 @@ description: "Integrate Fabro with GitHub for repository access and OAuth login" Fabro supports two GitHub integration strategies: - `token` — the default for local and individual use. Fabro captures `gh auth token` during `fabro install`, stores it as `GITHUB_TOKEN`, and uses that token directly for repo access, pull requests, and sandbox `GITHUB_TOKEN` injection. -- `app` — the team-oriented option. Fabro registers a [GitHub App](https://docs.github.com/en/apps/overview), uses installation tokens for repo access, and enables browser OAuth and webhooks. +- `app` — the team-oriented option. Fabro registers a [GitHub App](https://docs.github.com/en/apps/overview), uses installation tokens for repo access, enables browser OAuth, and supports webhook delivery when you configure a [webhook strategy](#webhook-delivery-strategies). `token` changes GitHub integration auth only. It does not provide browser sign-in, so the embedded web UI is disabled when `strategy = "token"`. @@ -19,11 +19,11 @@ Fabro supports two GitHub integration strategies: | Sandbox `GITHUB_TOKEN` | Direct token | Scoped installation token | | Browser sign-in | No | Yes | | Web UI routes | Disabled | Enabled | -| Webhooks | No | Yes | +| Webhooks | No | Strategy-dependent | ## GitHub App mode -The rest of this page describes the `app` strategy, which is required for browser auth and webhooks. +The rest of this page describes the `app` strategy, which is required for browser auth and for any webhook delivery strategy. | Feature | How it's used | |---|---| @@ -125,6 +125,30 @@ Fabro stores the GitHub App secrets in `/server.env` under these keys: The private key is stored as base64-encoded PEM. Fabro also accepts raw PEM format (starting with `-----BEGIN`). +### Webhook delivery strategies + +Fabro receives GitHub webhooks on `POST /api/v1/webhooks/github` whenever `GITHUB_APP_WEBHOOK_SECRET` is configured. The webhook `strategy` controls how that route becomes reachable from GitHub: + +```toml title="settings.toml" +[server.integrations.github] +strategy = "app" +app_id = "123456" +client_id = "Iv1.abc123def" +slug = "fabro-a3f2" + +[server.api] +url = "https://fabro-api.example.com" + +[server.integrations.github.webhooks] +strategy = "server_url" +``` + +- `server_url`: recommended for production. Fabro assumes `server.api.url` is already publicly reachable and best-effort updates the GitHub App webhook URL to `/api/v1/webhooks/github` each time `fabro server start` runs. +- `tailscale_funnel`: opt-in for machines reachable through Tailscale but not through a stable public URL. Fabro runs `tailscale funnel` against the main server port and best-effort updates the GitHub App webhook URL to the resulting Funnel origin. +- Unset `strategy`: Fabro still serves the webhook route if the secret is present, but it does not expose the route for you and does not mutate the GitHub App webhook URL. + +`tailscale_funnel` has host-wide side effects: it changes Tailscale Funnel state on the server and rewrites the GitHub App webhook URL on startup. Use `server_url` when you already have HTTPS and a stable hostname. + ### Reconfigure GitHub after install To switch GitHub strategies or re-register the app without re-running the full install wizard, use `fabro install github`: diff --git a/docs/plans/2026-04-19-002-feat-web-ui-lifecycle-actions-plan.md b/docs/plans/2026-04-19-002-feat-web-ui-lifecycle-actions-plan.md new file mode 100644 index 000000000..ff7002164 --- /dev/null +++ b/docs/plans/2026-04-19-002-feat-web-ui-lifecycle-actions-plan.md @@ -0,0 +1,548 @@ +--- +title: "feat: Expose CLI lifecycle actions (cancel, archive, unarchive) in the web UI" +type: feat +status: completed +date: 2026-04-19 +updated: 2026-04-20 +origin: docs/brainstorms/2026-04-19-web-ui-lifecycle-actions-requirements.md +--- + +# feat: Expose CLI lifecycle actions (cancel, archive, unarchive) in the web UI + +## Overview + +Today the Fabro web UI at `apps/fabro-web` is essentially read-only for run management: only Preview mutates state. This plan adds three lifecycle actions to the run detail page (`/runs/{id}`): **cancel**, **archive**, and **unarchive**. + +The plan also closes three supporting gaps the requirements and review surfaced: + +- a shared action-capable toast system +- a run-detail SSE subscription so action visibility updates live +- two missing event strings in the board revalidation allowlist + +As of **April 20, 2026**, product explicitly chose to remove the deferred cancel timer and undo window from the earlier requirements doc. In this plan, cancel fires immediately. + +Pause/unpause and delete remain out of scope (see origin: `docs/brainstorms/2026-04-19-web-ui-lifecycle-actions-requirements.md`). + +## Problem Frame + +Two user pains from the origin document drive this work: + +1. **Daily friction for CLI users** who live in the web UI but must context-switch to a terminal to cancel, archive, or unarchive runs. +2. **Exclusion of non-CLI teammates** (PMs, reviewers, stakeholders) who can observe runs but cannot participate in managing them. + +The server endpoints already exist. The remaining work is UI surface, route wiring, SSE reconciliation, and user-facing error handling. + +## Requirements Trace + +All IDs reference the origin document, but this plan is the source of truth for implementation. The origin document's deferred-cancel requirements were superseded on **April 20, 2026** when product chose immediate cancel with no undo window. + +- **R1** Expose cancel, archive, and unarchive on the run detail page. +- **R2** Do not expose pause, unpause, or delete in this pass. +- **R3** Do not expose checkpoint operations (resume, rewind, fork) or HITL question answering in this pass. +- **R4** Do not expose these actions on board kanban cards or as bulk selection in this pass. +- **R5** State-aware visibility: + cancel visible for `submitted|queued|starting|running|paused` as the primary affordance; archive for terminal non-archived runs; unarchive for archived runs. +- **R6** Blocked runs hide the primary cancel button and instead show an inline notice with question text plus CLI guidance. Cancel remains reachable through a de-emphasized secondary affordance inside that notice. +- **R7** The detail page subscribes to the run's SSE stream so affordances update live without a manual refresh. +- **R8** Cancel fires immediately. There is no client-side pending timer, no undo window, and no pre-fire `GET /runs/{id}` recheck in this plan. +- **R9** Archive and unarchive fire immediately and surface an inverse-action toast ("Run archived. Unarchive"). +- **R10** While a lifecycle action request is in flight, only the submitting control disables/spins. There is no cross-action disable window because there is no pending cancel timer. +- **R11** There is no optimistic local archived/unarchived flip. All three actions reconcile through the action response, normal route revalidation, and SSE for later lifecycle transitions. +- **R12** On 404/409/network failures, show a user-facing mapped error toast and revalidate so the page reconciles to actual server state. Because there is no optimistic local state, there is no rollback layer. +- **R13** Multi-tab behavior has no per-client timers. SSE still reconciles non-terminal state transitions across tabs. **Known limitation:** if tab A is showing an already-terminal run and tab B archives/unarchives it, tab A may stay stale because the per-run `/attach` stream has already closed on `RunCompleted`/`RunFailed`. Users recover on navigation/refresh or on a failed action attempt surfaced through the 409/error toast path. +- **R14** Accessibility: + toasts use `role="status"` and `aria-live="polite"`, do not steal focus, action buttons are keyboard-operable and touch-friendly, and the blocked-run secondary cancel affordance expands to a 48x48 CSS px touch target on coarse pointers. Countdown-specific pause/resume behavior is no longer part of scope. + +## Scope Boundaries + +- Actions exposed: cancel, archive, unarchive. Nothing else. +- Surface: run detail page (`/runs/{id}`) only. Not the board, not bulk. +- Not exposed: pause, unpause, delete, force-delete, resume/rewind/fork, HITL answering. +- No OpenAPI changes. +- No new server event types. +- No changes to authorization (single-tenant trusted deployment assumption). +- No phone-size responsive layout work; tablet and up only. +- No keyboard shortcuts for individual actions. + +## Context & Research + +### Relevant Code and Patterns + +- **Board SSE + revalidation allowlist:** `apps/fabro-web/app/routes/runs.tsx` (`BOARD_STATUS_EVENTS` at lines 63-79). `run.archived` and `run.unarchived` are missing. Test pattern in `apps/fabro-web/app/routes/runs.test.tsx`. +- **Existing per-run SSE subscriptions:** `apps/fabro-web/app/routes/run-files.tsx` and `apps/fabro-web/app/components/stage-sidebar.tsx`. Both parse `msg.data` JSON and gate on `payload.event`; they do **not** use `EventSource` message type names. +- **Run detail page header and Preview button:** `apps/fabro-web/app/routes/run-detail.tsx`. The route already uses React Router `useFetcher` plus a route `action` for Preview, so lifecycle actions can follow the same pattern instead of introducing a second mutation model. +- **Run detail loader shape:** the loader receives raw API `summary.status` and maps it onto `run.lifecycleStatus` in loader data. UI visibility logic in this plan keys off `run.lifecycleStatus`; loader-only branching is described explicitly as checking `summary.status` before mapping. +- **Existing toast primitive:** `apps/fabro-web/app/routes/run-files/states.tsx` has a simple read-only live-region toast. It is a useful visual/a11y baseline but is not reusable for stacked toasts with action buttons. +- **UI primitives:** `apps/fabro-web/app/components/ui.tsx` exports `PRIMARY_BUTTON_CLASS` and `SECONDARY_BUTTON_CLASS`. No generic `