From 731a8cc6e0a245e76ea344ae9976253ca8129ec7 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sun, 19 Apr 2026 17:06:06 -0400 Subject: [PATCH] feat(web): rewrite Run Files tab, surface it in navigation Rewrites apps/fabro-web/app/routes/run-files.tsx to consume the real PaginatedRunFileList response and removes the fallbackFiles fixture and the Steer subsystem. The new component: - Loads via apiJsonOrNull, so a 404/501 (dev without the route) renders the empty state instead of the root error boundary - Branches on meta.degraded + meta.patch to render PatchDiff with a DegradedBanner whose copy reflects degraded_reason - Renders per-entry placeholders for sensitive, binary, symlink/ submodule, and truncated files with the priority order sensitive > binary > symlink/submodule > truncated -- security flags never get hidden behind a lesser placeholder - Renders one MultiFileDiff per regular entry - Uses role="region" + aria-label on each file row Also unhides the Files Changed tab in run-detail.tsx by flipping broken: true -> false. Adds missing final_patch: None to the runner RunFailed test fixtures to match the lifecycle change from Unit 2. Refs plan docs/plans/2026-04-19-002-feat-run-files-changed-tab-plan.md Co-Authored-By: Claude Opus 4.7 (1M context) --- apps/fabro-web/app/routes/run-detail.tsx | 2 +- apps/fabro-web/app/routes/run-files.tsx | 689 +++++------------- .../fabro-cli/src/commands/run/runner.rs | 2 + lib/crates/fabro-spa/assets/assets/app.css | 2 +- .../{entry-h33x4hjk.js => entry-963f7qc4.js} | 456 ++++-------- lib/crates/fabro-spa/assets/index.html | 2 +- 6 files changed, 327 insertions(+), 826 deletions(-) rename lib/crates/fabro-spa/assets/assets/{entry-h33x4hjk.js => entry-963f7qc4.js} (56%) diff --git a/apps/fabro-web/app/routes/run-detail.tsx b/apps/fabro-web/app/routes/run-detail.tsx index 974ca66ec..b2b434280 100644 --- a/apps/fabro-web/app/routes/run-detail.tsx +++ b/apps/fabro-web/app/routes/run-detail.tsx @@ -11,7 +11,7 @@ import type { PreviewUrlResponse } from "@qltysh/fabro-api-client"; const allTabs = [ { name: "Overview", path: "", count: null, demoOnly: false, broken: false }, { name: "Stages", path: "/stages", count: null, demoOnly: false, broken: false }, - { name: "Files Changed", path: "/files", count: null, demoOnly: false, broken: true }, + { name: "Files Changed", path: "/files", count: null, demoOnly: false, broken: false }, { name: "Graph", path: "/graph", count: null, demoOnly: false, broken: false }, { name: "Billing", path: "/billing", count: null, demoOnly: false, broken: false }, ]; diff --git a/apps/fabro-web/app/routes/run-files.tsx b/apps/fabro-web/app/routes/run-files.tsx index c8da42c63..a4fd29c9b 100644 --- a/apps/fabro-web/app/routes/run-files.tsx +++ b/apps/fabro-web/app/routes/run-files.tsx @@ -1,544 +1,209 @@ -import { useCallback, useEffect, useRef, useState } from "react"; -import { - MultiFileDiff, - type AnnotationSide, - type DiffLineAnnotation, -} from "@pierre/diffs/react"; +import type { ReactElement } from "react"; +import { MultiFileDiff, PatchDiff } from "@pierre/diffs/react"; import { useTheme } from "../lib/theme"; -import { apiJson } from "../api"; -import type { PaginatedRunFileList } from "@qltysh/fabro-api-client"; +import { apiJsonOrNull } from "../api"; +import type { + FileDiff as ApiFileDiff, + PaginatedRunFileList, +} from "@qltysh/fabro-api-client"; export const handle = { wide: true }; export async function loader({ request, params }: any) { - const data = await apiJson(`/runs/${params.id}/files`, { request }); + const data = await apiJsonOrNull( + `/runs/${params.id}/files`, + { request }, + ); return data; } -const fallbackFiles = [ - { - oldFile: { - name: "src/commands/run.ts", - contents: `import { parseArgs } from "node:util"; -import { loadConfig } from "../config.js"; -import { execute } from "../executor.js"; +const PLACEHOLDER_CLASSES = + "flex items-center justify-between rounded-md border border-line bg-panel/60 px-4 py-3 text-sm text-fg-muted"; -interface RunOptions { - config: string; - dryRun: boolean; -} - -export async function run(argv: string[]) { - const { values } = parseArgs({ - args: argv, - options: { - config: { type: "string", short: "c", default: ".fabro/project.toml" }, - "dry-run": { type: "boolean", default: false }, - }, - }); - - const opts: RunOptions = { - config: values.config ?? ".fabro/project.toml", - dryRun: values["dry-run"] ?? false, - }; - - const config = await loadConfig(opts.config); - const result = await execute(config, { dryRun: opts.dryRun }); - - if (result.success) { - console.log("Run completed successfully."); - } else { - console.error("Run failed:", result.error); - process.exitCode = 1; - } -} -`, - }, - newFile: { - name: "src/commands/run.ts", - contents: `import { parseArgs } from "node:util"; -import { loadConfig } from "../config.js"; -import { execute } from "../executor.js"; -import { createLogger, type Logger } from "../logger.js"; - -interface RunOptions { - config: string; - dryRun: boolean; - verbose: boolean; -} - -export async function run(argv: string[]) { - const { values } = parseArgs({ - args: argv, - options: { - config: { type: "string", short: "c", default: ".fabro/project.toml" }, - "dry-run": { type: "boolean", default: false }, - verbose: { type: "boolean", short: "v", default: false }, - }, - }); - - const opts: RunOptions = { - config: values.config ?? ".fabro/project.toml", - dryRun: values["dry-run"] ?? false, - verbose: values.verbose ?? false, - }; - - const logger: Logger = createLogger({ verbose: opts.verbose }); - - const config = await loadConfig(opts.config); - logger.debug("Loaded config from %s", opts.config); - - const result = await execute(config, { dryRun: opts.dryRun, logger }); - logger.debug("Execution finished in %dms", result.elapsed); - - if (result.success) { - console.log("Run completed successfully."); - } else { - console.error("Run failed:", result.error); - process.exitCode = 1; - } -} -`, - }, - }, - { - oldFile: { - name: "src/logger.ts", - contents: "", - }, - newFile: { - name: "src/logger.ts", - contents: `export interface Logger { - info(message: string, ...args: unknown[]): void; - debug(message: string, ...args: unknown[]): void; - error(message: string, ...args: unknown[]): void; -} - -interface LoggerOptions { - verbose: boolean; -} - -export function createLogger({ verbose }: LoggerOptions): Logger { - return { - info(message, ...args) { - console.log(message, ...args); - }, - debug(message, ...args) { - if (verbose) { - console.log("[debug]", message, ...args); - } - }, - error(message, ...args) { - console.error(message, ...args); - }, - }; -} -`, - }, - }, - { - oldFile: { - name: "src/executor.ts", - contents: `import type { Config } from "./config.js"; - -interface ExecuteOptions { - dryRun: boolean; -} - -interface ExecuteResult { - success: boolean; - error?: string; -} - -export async function execute( - config: Config, - options: ExecuteOptions, -): Promise { - if (options.dryRun) { - console.log("Dry run — skipping execution."); - return { success: true }; - } - - try { - for (const step of config.steps) { - await step.run(); - } - return { success: true }; - } catch (err) { - const message = err instanceof Error ? err.message : String(err); - return { success: false, error: message }; - } -} -`, - }, - newFile: { - name: "src/executor.ts", - contents: `import type { Config } from "./config.js"; -import type { Logger } from "./logger.js"; - -interface ExecuteOptions { - dryRun: boolean; - logger: Logger; -} - -interface ExecuteResult { - success: boolean; - elapsed: number; - error?: string; -} - -export async function execute( - config: Config, - options: ExecuteOptions, -): Promise { - const start = performance.now(); - - if (options.dryRun) { - options.logger.info("Dry run — skipping execution."); - return { success: true, elapsed: performance.now() - start }; - } - - try { - for (const step of config.steps) { - options.logger.debug("Running step: %s", step.name); - await step.run(); - } - return { success: true, elapsed: performance.now() - start }; - } catch (err) { - const message = err instanceof Error ? err.message : String(err); - return { success: false, elapsed: performance.now() - start, error: message }; - } -} -`, - }, - }, -]; - -interface SteerAnnotation { - fileName: string; - lineNumber: number; - side: AnnotationSide; -} - -function steerKey(fileName: string, side: AnnotationSide, lineNumber: number) { - return `${fileName}:${side}:${lineNumber}`; -} - -function DiffHeaderToggles({ - diffStyle, - onDiffStyleChange, - disableBackground, - onDisableBackgroundChange, -}: { - diffStyle: "split" | "unified"; - onDiffStyleChange: (style: "split" | "unified") => void; - disableBackground: boolean; - onDisableBackgroundChange: (disabled: boolean) => void; -}) { +function DegradedBanner({ reason }: { reason?: string }) { + const copy = banner_copy_for_reason(reason); return ( -
- - +
+ {copy}
); } -function DiffWithSteer({ - oldFile, - newFile, - openSteers, - submittedSteers, - onSteer, - onSubmit, - onCancel, -}: { - oldFile: { name: string; contents: string }; - newFile: { name: string; contents: string }; - openSteers: Map; - submittedSteers: Map; - onSteer: (annotation: SteerAnnotation) => void; - onSubmit: (annotation: SteerAnnotation, text: string) => void; - onCancel: (annotation: SteerAnnotation) => void; -}) { - const [diffStyle, setDiffStyle] = useState<"split" | "unified">("split"); - const [disableBackground, setDisableBackground] = useState(false); - const { theme } = useTheme(); - - const containerRef = useRef(null); - const buttonRef = useRef(null); - const hoveredRef = useRef<{ lineNumber: number; side: AnnotationSide } | null>(null); - const leaveTimeoutRef = useRef(0); - - const showButton = useCallback((lineElement: HTMLElement, lineNumber: number, side: AnnotationSide) => { - clearTimeout(leaveTimeoutRef.current); - hoveredRef.current = { lineNumber, side }; - - const btn = buttonRef.current; - const container = containerRef.current; - if (!btn || !container) return; - - const containerRect = container.getBoundingClientRect(); - const lineRect = lineElement.getBoundingClientRect(); - - btn.style.top = `${lineRect.top - containerRect.top}px`; - btn.style.height = `${lineRect.height}px`; - btn.style.display = "flex"; - }, []); - - const hideButton = useCallback(() => { - leaveTimeoutRef.current = window.setTimeout(() => { - hoveredRef.current = null; - if (buttonRef.current) { - buttonRef.current.style.display = "none"; - } - }, 100); - }, []); +function banner_copy_for_reason(reason: string | undefined): string { + switch (reason) { + case "sandbox_gone": + return "Showing final patch only. This run's sandbox has been cleaned up, so individual file contents are no longer available."; + case "provider_unsupported": + return "Live diff isn't supported for this sandbox provider. Showing the patch captured at the last checkpoint."; + case "sandbox_unreachable": + default: + return "Couldn't reach this run's sandbox. Showing the patch captured at the last checkpoint — refresh to try again."; + } +} +function SensitivePlaceholder({ name }: { name: string }) { return ( -
- - oldFile={oldFile} - newFile={newFile} - options={{ - diffStyle, - disableBackground, - theme: theme === "dark" ? "pierre-dark" : "pierre-light", - lineDiffType: "word", - onLineEnter({ lineNumber, annotationSide, lineElement }) { - showButton(lineElement, lineNumber, annotationSide); - }, - onLineLeave() { - hideButton(); - }, - }} - renderHeaderMetadata={() => ( - - )} - lineAnnotations={buildAnnotationsForFile( - newFile.name, - openSteers, - submittedSteers, - )} - renderAnnotation={(annotation) => { - const meta = annotation.metadata; - if ("text" in meta && meta.text != null) { - return ; - } - return ( - onCancel(meta)} - /> - ); - }} +
+ {name} + + sensitive — contents omitted + +
+ ); +} + +function BinaryPlaceholder({ name }: { name: string }) { + return ( +
+ {name} + + binary — not shown inline + +
+ ); +} + +function TruncatedPlaceholder({ + name, + reason, +}: { + name: string; + reason?: string; +}) { + const label = + reason === "budget_exhausted" + ? "omitted — too many files changed" + : "too large to render inline"; + return ( +
+ {name} + + {label} + +
+ ); +} + +function SymlinkOrSubmodulePlaceholder({ + name, + kind, +}: { + name: string; + kind: "symlink" | "submodule"; +}) { + return ( +
+ {name} + + {kind} + +
+ ); +} + +function EmptyState({ message }: { message: string }) { + return ( +
+ {message} +
+ ); +} + +function pick_placeholder(file: ApiFileDiff): ReactElement | null { + const display_name = file.new_file.name || file.old_file.name; + // Priority: sensitive > binary > symlink/submodule > truncated. Security + // flags must never be hidden by a lesser placeholder. + if (file.sensitive) { + return ; + } + if (file.binary) { + return ; + } + if (file.change_kind === "symlink") { + return ; + } + if (file.change_kind === "submodule") { + return ( + + ); + } + if (file.truncated) { + return ( + -
clearTimeout(leaveTimeoutRef.current)} - onMouseLeave={() => { - hoveredRef.current = null; - if (buttonRef.current) { - buttonRef.current.style.display = "none"; - } - }} - > - -
-
- ); -} - -function SteerCommentForm({ - annotation, - onSubmit, - onCancel, -}: { - annotation: SteerAnnotation; - onSubmit: (annotation: SteerAnnotation, text: string) => void; - onCancel: () => void; -}) { - const [text, setText] = useState(""); - const textareaRef = useRef(null); - - useEffect(() => { - textareaRef.current?.focus(); - }, []); - - return ( -
-