From 9e54643a7e28e41096d67e13e2eb228c07f4a65e Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Thu, 7 May 2026 16:09:26 -0700 Subject: [PATCH 01/63] fix(server): hide synthetic start node from billing rollup The billing rollup filtered out the synthetic exit handler but left start visible, so the billing tab showed an asymmetric pair. Both are no-op workflow boundary handlers and shouldn't appear as billable stages. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/server/handler/billing.rs | 6 +++--- .../fabro-workflow/src/billing_rollup.rs | 21 +++++++++---------- 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/lib/crates/fabro-server/src/server/handler/billing.rs b/lib/crates/fabro-server/src/server/handler/billing.rs index 49a2720ae..abe22703f 100644 --- a/lib/crates/fabro-server/src/server/handler/billing.rs +++ b/lib/crates/fabro-server/src/server/handler/billing.rs @@ -144,7 +144,7 @@ fn live_billing_rows(projection: &RunProjection, now: DateTime) -> Vec bool { || stage.state.is_some() } -fn is_exit_stage(projection: &RunProjection, node_id: &str) -> bool { +fn is_boundary_stage(projection: &RunProjection, node_id: &str) -> bool { projection .spec() .and_then(|spec| spec.graph().nodes.get(node_id)) - .is_some_and(|node| node.handler_type() == Some("exit")) + .is_some_and(|node| matches!(node.handler_type(), Some("start" | "exit"))) } diff --git a/lib/crates/fabro-workflow/src/billing_rollup.rs b/lib/crates/fabro-workflow/src/billing_rollup.rs index 03d5207d3..08d66e5f6 100644 --- a/lib/crates/fabro-workflow/src/billing_rollup.rs +++ b/lib/crates/fabro-workflow/src/billing_rollup.rs @@ -43,7 +43,7 @@ pub fn billing_rollup_from_projection(projection: &RunProjection) -> ProjectionB let mut billed_visit_count = 0_usize; for (stage_id, stage) in projection.iter_stages() { - if is_exit_stage(projection, stage_id.node_id()) { + if is_boundary_stage(projection, stage_id.node_id()) { continue; } if stage.completion.is_none() && stage.duration_ms.is_none() && stage.usage.is_none() { @@ -97,11 +97,11 @@ pub fn billing_rollup_from_projection(projection: &RunProjection) -> ProjectionB } } -fn is_exit_stage(projection: &RunProjection, node_id: &str) -> bool { +fn is_boundary_stage(projection: &RunProjection, node_id: &str) -> bool { projection .spec() .and_then(|spec| spec.graph().nodes.get(node_id)) - .is_some_and(|node| node.handler_type() == Some("exit")) + .is_some_and(|node| matches!(node.handler_type(), Some("start" | "exit"))) } fn accumulate_usage(counts: &mut BilledTokenCounts, usage: &BilledModelUsage) { @@ -205,7 +205,7 @@ mod tests { #[test] fn rollup_includes_completed_non_llm_stage_rows_with_zero_billing() { let mut projection = RunProjection::default(); - let stage = projection.stage_entry("start", 1, first_event_seq(1)); + let stage = projection.stage_entry("build", 1, first_event_seq(1)); stage.duration_ms = Some(25); stage.completion = Some(StageCompletion { outcome: StageOutcome::Succeeded, @@ -217,7 +217,7 @@ mod tests { let rollup = billing_rollup_from_projection(&projection); assert_eq!(rollup.stages.len(), 1); - assert_eq!(rollup.stages[0].node_id, "start"); + assert_eq!(rollup.stages[0].node_id, "build"); assert_eq!(rollup.stages[0].duration_ms, 25); assert!(rollup.stages[0].model_id.is_none()); assert_eq!(rollup.stages[0].billing.input_tokens, 0); @@ -227,9 +227,9 @@ mod tests { } #[test] - fn rollup_excludes_terminal_exit_stage_rows() { + fn rollup_excludes_workflow_boundary_stage_rows() { let mut projection = RunProjection::default(); - projection.spec = Some(run_spec_with_exit_node()); + projection.spec = Some(run_spec_with_boundary_nodes()); let start = projection.stage_entry("start", 1, first_event_seq(1)); start.duration_ms = Some(25); start.completion = Some(StageCompletion { @@ -249,12 +249,11 @@ mod tests { let rollup = billing_rollup_from_projection(&projection); - assert_eq!(rollup.stages.len(), 1); - assert_eq!(rollup.stages[0].node_id, "start"); - assert_eq!(rollup.runtime_ms, 25); + assert_eq!(rollup.stages.len(), 0); + assert_eq!(rollup.runtime_ms, 0); } - fn run_spec_with_exit_node() -> RunSpec { + fn run_spec_with_boundary_nodes() -> RunSpec { let mut graph = Graph::new("test"); graph.nodes.insert("start".to_string(), { let mut node = Node::new("start"); From 089f6befe86ee2478da218f51caa1df8aed6d7a5 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Thu, 7 May 2026 16:11:09 -0700 Subject: [PATCH 02/63] feat(web): show LLM model name in stage view toolbar Extracts the model from `stage.prompt`, `agent.session.activated`, and `agent.cli.started` events that the stage already loads, and renders it on the right side of the events toolbar with a CpuChipIcon. Co-Authored-By: Claude Opus 4.7 (1M context) --- apps/fabro-web/app/routes/run-stages.test.ts | 59 +++++++++++++++++++- apps/fabro-web/app/routes/run-stages.tsx | 41 ++++++++++++++ 2 files changed, 99 insertions(+), 1 deletion(-) diff --git a/apps/fabro-web/app/routes/run-stages.test.ts b/apps/fabro-web/app/routes/run-stages.test.ts index 742910b94..e8639b0ee 100644 --- a/apps/fabro-web/app/routes/run-stages.test.ts +++ b/apps/fabro-web/app/routes/run-stages.test.ts @@ -1,7 +1,7 @@ import { describe, expect, test } from "bun:test"; import type { EventEnvelope } from "@qltysh/fabro-api-client"; -import { eventsToActivity } from "./run-stages"; +import { eventsToActivity, extractStageModel } from "./run-stages"; function envelope(seq: number, partial: Partial): EventEnvelope { return { @@ -163,6 +163,63 @@ describe("eventsToActivity", () => { } }); + test("extractStageModel pulls model from agent.session.activated, ignoring other stages", () => { + const events: EventEnvelope[] = [ + envelope(1, { + event: "agent.session.activated", + stage_id: "simplify@1", + node_id: "simplify", + properties: { provider: "anthropic", model: "claude-sonnet-4-5" }, + }), + envelope(2, { + event: "agent.session.activated", + stage_id: "verify@1", + node_id: "verify", + properties: { provider: "openai", model: "gpt-5" }, + }), + ]; + + expect(extractStageModel(events, "simplify@1")).toBe("claude-sonnet-4-5"); + expect(extractStageModel(events, "verify@1")).toBe("gpt-5"); + expect(extractStageModel(events, "fmt@1")).toBe(null); + }); + + test("extractStageModel uses latest stage event with a model", () => { + const events: EventEnvelope[] = [ + envelope(1, { + event: "stage.prompt", + stage_id: "agent@1", + node_id: "agent", + properties: { model: "claude-opus-4-5" }, + }), + envelope(2, { + event: "agent.cli.started", + stage_id: "agent@1", + node_id: "agent", + properties: { + provider: "anthropic", + model: "claude-sonnet-4-6", + command: "claude", + }, + }), + ]; + + expect(extractStageModel(events, "agent@1")).toBe("claude-sonnet-4-6"); + }); + + test("extractStageModel ignores model from unrelated event types", () => { + const events: EventEnvelope[] = [ + envelope(1, { + event: "agent.message", + stage_id: "agent@1", + node_id: "agent", + properties: { text: "hi", model: "should-be-ignored" }, + }), + ]; + + expect(extractStageModel(events, "agent@1")).toBe(null); + }); + test("ignores unknown event types and events for other stages", () => { const events: EventEnvelope[] = [ envelope(1, { diff --git a/apps/fabro-web/app/routes/run-stages.tsx b/apps/fabro-web/app/routes/run-stages.tsx index 5d7b7e578..d8991a604 100644 --- a/apps/fabro-web/app/routes/run-stages.tsx +++ b/apps/fabro-web/app/routes/run-stages.tsx @@ -10,6 +10,7 @@ import { XMarkIcon } from "@heroicons/react/24/outline"; import { CheckIcon, ChevronUpDownIcon, + CpuChipIcon, FunnelIcon, MagnifyingGlassIcon, } from "@heroicons/react/16/solid"; @@ -195,6 +196,26 @@ export function eventsToActivity(events: EventEnvelope[], stageId: string): Turn return turns; } +const STAGE_MODEL_EVENT_NAMES = new Set([ + "stage.prompt", + "agent.session.activated", + "agent.cli.started", +]); + +export function extractStageModel( + events: EventEnvelope[], + stageId: string, +): string | null { + let model: string | null = null; + for (const e of events) { + if (activityEventStageId(e) !== stageId) continue; + if (!e.event || !STAGE_MODEL_EVENT_NAMES.has(e.event)) continue; + const candidate = getString(e.properties ?? {}, "model"); + if (candidate) model = candidate; + } + return model; +} + function turnLabel(turn: TurnType): string { switch (turn.kind) { case "system": @@ -816,6 +837,7 @@ function EventsToolbar({ onSearchChange, filteredCount, totalCount, + model, }: { tab: EventsTab; onTabChange: (tab: EventsTab) => void; @@ -828,6 +850,7 @@ function EventsToolbar({ onSearchChange: (value: string) => void; filteredCount: number; totalCount: number; + model: string | null; }) { const transcriptAllSelected = selectedKinds.length === EVENT_KINDS.length; const debugAllSelected = @@ -879,6 +902,15 @@ function EventsToolbar({ {filteredCount.toLocaleString()} of {totalCount.toLocaleString()} events )} + {model && ( + + + )} ); } @@ -949,6 +981,14 @@ export default function RunStages() { } return Array.from(set).sort(); }, [debugEvents]); + const stageModel = useMemo( + () => + selectedStageId + ? extractStageModel(stageEventsQuery.data ?? [], selectedStageId) + : null, + [stageEventsQuery.data, selectedStageId], + ); + const filteredDebugEvents = useMemo(() => { const useCategoryFilter = selectedDebugCategories.length > 0; const cats = new Set(selectedDebugCategories); @@ -1008,6 +1048,7 @@ export default function RunStages() { onSearchChange={setSearch} filteredCount={tab === "transcript" ? filteredTurns.length : filteredDebugEvents.length} totalCount={tab === "transcript" ? turns.length : debugEvents.length} + model={stageModel} /> From 7f659df9acdb2eb33735a4de04f7062dffa0345e Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Thu, 7 May 2026 16:14:39 -0700 Subject: [PATCH 03/63] feat(web): add icons next to token and duration metrics in events feed Tool/command rows show a clock icon next to the duration; agent rows show a tokens icon next to the input/output token count. Adds left padding between the metric column and the elapsed timestamp column so the two values read as separate fields. Co-Authored-By: Claude Opus 4.7 (1M context) --- apps/fabro-web/app/routes/run-stages.tsx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/apps/fabro-web/app/routes/run-stages.tsx b/apps/fabro-web/app/routes/run-stages.tsx index d8991a604..b3396d01d 100644 --- a/apps/fabro-web/app/routes/run-stages.tsx +++ b/apps/fabro-web/app/routes/run-stages.tsx @@ -14,6 +14,7 @@ import { FunnelIcon, MagnifyingGlassIcon, } from "@heroicons/react/16/solid"; +import { CircleStackIcon, ClockIcon } from "@heroicons/react/20/solid"; import { Marked } from "marked"; import { StageSidebar } from "../components/stage-sidebar"; @@ -401,6 +402,7 @@ function EventRow({ onSelect: () => void; }) { const metric = turnMetric(turn); + const MetricIcon = metric == null ? null : turn.kind === "assistant" ? CircleStackIcon : ClockIcon; return ( From 858caf7434ccad58f84337015937efc369ef3d41 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Thu, 7 May 2026 16:16:16 -0700 Subject: [PATCH 04/63] style(web): show errored tool calls with red Error label, not red pill Co-Authored-By: Claude Opus 4.7 (1M context) --- apps/fabro-web/app/routes/run-stages.tsx | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/apps/fabro-web/app/routes/run-stages.tsx b/apps/fabro-web/app/routes/run-stages.tsx index b3396d01d..887bc1d62 100644 --- a/apps/fabro-web/app/routes/run-stages.tsx +++ b/apps/fabro-web/app/routes/run-stages.tsx @@ -231,9 +231,6 @@ function turnLabel(turn: TurnType): string { } function turnTone(turn: TurnType): string { - if (turn.kind === "tool" && turn.isError) { - return "bg-coral/15 text-coral"; - } switch (turn.kind) { case "system": return "bg-amber/15 text-amber"; @@ -421,6 +418,11 @@ function EventRow({ {turnSummary(turn)} + {turn.kind === "tool" && turn.isError && ( + + Error + + )} {MetricIcon && From 0aceebbd9ecd2115d42dfca8f505c8a24a4a11b4 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Thu, 7 May 2026 16:21:56 -0700 Subject: [PATCH 05/63] feat(web): render command stages with a Logs tab showing real stdout/stderr Command nodes now show a "Logs" tab (in place of "Transcript") that fetches the actual stdout/stderr bytes via the stage log endpoint, rather than the blob:// refs carried in command.completed events. Exit code and duration sit on the right side of the toolbar alongside the tab toggle. Agent stages are unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) --- apps/fabro-web/app/lib/queries.ts | 12 + apps/fabro-web/app/routes/run-stages.test.ts | 56 +++- apps/fabro-web/app/routes/run-stages.tsx | 304 ++++++++++++++++--- 3 files changed, 322 insertions(+), 50 deletions(-) diff --git a/apps/fabro-web/app/lib/queries.ts b/apps/fabro-web/app/lib/queries.ts index d804d3c4b..a13c92817 100644 --- a/apps/fabro-web/app/lib/queries.ts +++ b/apps/fabro-web/app/lib/queries.ts @@ -162,6 +162,18 @@ export function fetchRunCommandLog( ); } +export function useRunStageLog( + id: string | undefined, + stageId: string | undefined, + stream: CommandOutputStream, + enabled: boolean, +) { + return useSWR( + enabled && id && stageId ? queryKeys.runs.stageLog(id, stageId, stream) : null, + apiFetcher, + ); +} + export function useWorkflows() { return useSWR( queryKeys.workflows.list(), diff --git a/apps/fabro-web/app/routes/run-stages.test.ts b/apps/fabro-web/app/routes/run-stages.test.ts index e8639b0ee..d64b47972 100644 --- a/apps/fabro-web/app/routes/run-stages.test.ts +++ b/apps/fabro-web/app/routes/run-stages.test.ts @@ -1,7 +1,7 @@ import { describe, expect, test } from "bun:test"; import type { EventEnvelope } from "@qltysh/fabro-api-client"; -import { eventsToActivity, extractStageModel } from "./run-stages"; +import { eventsToActivity, extractStageModel, turnsToStageKind } from "./run-stages"; function envelope(seq: number, partial: Partial): EventEnvelope { return { @@ -79,8 +79,10 @@ describe("eventsToActivity", () => { event: "command.completed", node_id: "fmt", properties: { - stdout: "ok", - stderr: "", + stdout: "blob://sha256/abc", + stderr: "blob://sha256/def", + stdout_bytes: 42, + stderr_bytes: 0, exit_code: 0, duration_ms: 12, termination: "exited", @@ -94,6 +96,8 @@ describe("eventsToActivity", () => { kind: "command", script: "cargo fmt", running: false, + stdoutBytes: 42, + stderrBytes: 0, }); }); @@ -251,3 +255,49 @@ describe("eventsToActivity", () => { } }); }); + +describe("turnsToStageKind", () => { + test("classifies a stage with only command events as command", () => { + const events: EventEnvelope[] = [ + envelope(1, { + event: "stage.prompt", + node_id: "fmt", + properties: { text: "run formatter" }, + }), + envelope(2, { + event: "command.started", + node_id: "fmt", + properties: { script: "cargo fmt", language: "shell" }, + }), + envelope(3, { + event: "command.completed", + node_id: "fmt", + properties: { + stdout: "blob://sha256/abc", + stderr: "blob://sha256/def", + exit_code: 0, + duration_ms: 5, + termination: "exited", + }, + }), + ]; + + expect(turnsToStageKind(eventsToActivity(events, "fmt"))).toBe("command"); + }); + + test("classifies a stage with agent events as agent", () => { + const events: EventEnvelope[] = [ + envelope(1, { + event: "agent.message", + node_id: "simplify", + properties: { text: "thinking…" }, + }), + ]; + + expect(turnsToStageKind(eventsToActivity(events, "simplify"))).toBe("agent"); + }); + + test("defaults to agent for empty turns", () => { + expect(turnsToStageKind([])).toBe("agent"); + }); +}); diff --git a/apps/fabro-web/app/routes/run-stages.tsx b/apps/fabro-web/app/routes/run-stages.tsx index 887bc1d62..d9c14c9ac 100644 --- a/apps/fabro-web/app/routes/run-stages.tsx +++ b/apps/fabro-web/app/routes/run-stages.tsx @@ -20,11 +20,17 @@ import { Marked } from "marked"; import { StageSidebar } from "../components/stage-sidebar"; import type { Stage } from "../components/stage-sidebar"; import { EmptyState } from "../components/state"; -import { useRun, useRunStageEvents, useRunStages } from "../lib/queries"; +import { formatBytes } from "../lib/format"; +import { + useRun, + useRunStageEvents, + useRunStageLog, + useRunStages, +} from "../lib/queries"; import { STAGE_ACTIVITY_EVENT_TYPES, type StageActivityEventType } from "../lib/run-events"; import { mapRunStagesToSidebarStages } from "../lib/stage-sidebar"; import { getNumber, getString, type UnknownRecord } from "../lib/unknown"; -import type { EventEnvelope } from "@qltysh/fabro-api-client"; +import type { CommandOutputStream, EventEnvelope } from "@qltysh/fabro-api-client"; export const handle = { wide: true, fullHeight: true }; @@ -32,7 +38,19 @@ type TurnType = | { kind: "system"; ts: string; content: string } | { kind: "assistant"; ts: string; content: string; inputTokens: number; outputTokens: number } | { kind: "tool"; ts: string; toolName: string; input: string; result: string; isError: boolean; durationMs: number } - | { kind: "command"; ts: string; script: string; running: boolean; exitCode: number | null; durationMs: number }; + | { + kind: "command"; + ts: string; + script: string; + running: boolean; + exitCode: number | null; + durationMs: number; + stdoutBytes: number; + stderrBytes: number; + }; + +type CommandTurn = Extract; +type StageKind = "agent" | "command"; const STAGE_ACTIVITY_EVENT_SET = new Set(STAGE_ACTIVITY_EVENT_TYPES); @@ -72,10 +90,10 @@ function debugCategoryTone(category: string): string { const EVENTS_TABS = ["transcript", "debug"] as const; type EventsTab = (typeof EVENTS_TABS)[number]; -const EVENTS_TAB_LABEL: Record = { - transcript: "Transcript", - debug: "Debug", -}; +function eventsTabLabel(tab: EventsTab, stageKind: StageKind): string { + if (tab === "debug") return "Debug"; + return stageKind === "command" ? "Logs" : "Transcript"; +} function assertNever(value: never): never { throw new Error(`Unhandled stage activity event type: ${value}`); @@ -174,6 +192,8 @@ export function eventsToActivity(events: EventEnvelope[], stageId: string): Turn running: false, exitCode: getNumber(props, "exit_code") ?? null, durationMs: getNumber(props, "duration_ms") ?? 0, + stdoutBytes: getNumber(props, "stdout_bytes") ?? 0, + stderrBytes: getNumber(props, "stderr_bytes") ?? 0, }); pendingCommand = undefined; break; @@ -191,12 +211,23 @@ export function eventsToActivity(events: EventEnvelope[], stageId: string): Turn running: true, exitCode: null, durationMs: 0, + stdoutBytes: 0, + stderrBytes: 0, }); } return turns; } +export function turnsToStageKind(turns: TurnType[]): StageKind { + let hasCommand = false; + for (const t of turns) { + if (t.kind === "assistant" || t.kind === "tool") return "agent"; + if (t.kind === "command") hasCommand = true; + } + return hasCommand ? "command" : "agent"; +} + const STAGE_MODEL_EVENT_NAMES = new Set([ "stage.prompt", "agent.session.activated", @@ -614,6 +645,157 @@ function EventDetails({ turn, runStart }: { turn: TurnType; runStart: string | u ); } +function decodeBase64Utf8(b64: string): string { + const binary = atob(b64); + const bytes = new Uint8Array(binary.length); + for (let i = 0; i < binary.length; i += 1) bytes[i] = binary.charCodeAt(i); + return new TextDecoder("utf-8", { fatal: false }).decode(bytes); +} + +function LogStream({ + runId, + stageId, + stream, + label, + byteCount, + enabled, + tone, +}: { + runId: string; + stageId: string; + stream: CommandOutputStream; + label: string; + byteCount: number; + enabled: boolean; + tone?: "stderr"; +}) { + const { data, error, isLoading } = useRunStageLog(runId, stageId, stream, enabled && byteCount > 0); + const text = useMemo(() => { + if (!data?.bytes_base64) return ""; + try { + return decodeBase64Utf8(data.bytes_base64); + } catch { + return ""; + } + }, [data]); + const truncated = + data && data.total_bytes > data.next_offset ? data.total_bytes - data.next_offset : 0; + + return ( +
+
+

+ {label} +

+ {byteCount > 0 && ( + + {formatBytes(byteCount)} + + )} +
+ 
+        {byteCount === 0 ? (
+          empty
+        ) : isLoading && !data ? (
+          loading…
+        ) : error ? (
+          Failed to load {stream}.
+        ) : (
+          text || empty
+        )}
+
+ {truncated > 0 && ( +

+ Showing first {formatBytes(data!.next_offset)} of {formatBytes(data!.total_bytes)}. +

+ )} +
+ ); +} + +function CommandStatus({ turn }: { turn: CommandTurn }) { + const exitTone = + turn.exitCode == null + ? "text-fg-muted" + : turn.exitCode === 0 + ? "text-mint" + : "text-coral"; + return ( + + {turn.running ? ( + + + Running… + + ) : ( + + exit {turn.exitCode ?? "?"} + + )} + {turn.durationMs > 0 && ( + + {formatDurationMs(turn.durationMs)} + + )} + + ); +} + +function CommandScript({ script }: { script: string }) { + return ( +
+

+ Command +

+ 
+        {script || empty}
+
+
+ ); +} + +function CommandLogs({ + runId, + stageId, + turn, +}: { + runId: string; + stageId: string; + turn: CommandTurn | null; +}) { + if (!turn) { + return ( +
No command output yet.
+ ); + } + return ( +
+ + + +
+ ); +} + function DetailsPanel({ title, isOpen, @@ -711,9 +893,11 @@ function DebugEventDetailsPanel({ function EventsTabToggle({ tab, + stageKind, onTabChange, }: { tab: EventsTab; + stageKind: StageKind; onTabChange: (tab: EventsTab) => void; }) { return ( @@ -736,7 +920,7 @@ function EventsTabToggle({ : "text-fg-muted hover:text-fg-2" }`} > - {EVENTS_TAB_LABEL[value]} + {eventsTabLabel(value, stageKind)} ); })} @@ -832,6 +1016,8 @@ function SearchInput({ function EventsToolbar({ tab, + stageKind, + commandTurn, onTabChange, selectedKinds, onKindsChange, @@ -845,6 +1031,8 @@ function EventsToolbar({ model, }: { tab: EventsTab; + stageKind: StageKind; + commandTurn: CommandTurn | null; onTabChange: (tab: EventsTab) => void; selectedKinds: EventKind[]; onKindsChange: (kinds: EventKind[]) => void; @@ -857,13 +1045,16 @@ function EventsToolbar({ totalCount: number; model: string | null; }) { + const showFilters = !(tab === "transcript" && stageKind === "command"); const transcriptAllSelected = selectedKinds.length === EVENT_KINDS.length; const debugAllSelected = selectedDebugCategories.length === 0 || selectedDebugCategories.length === availableDebugCategories.length; - const isFiltering = tab === "transcript" - ? !transcriptAllSelected || search.length > 0 - : !debugAllSelected || search.length > 0; + const isFiltering = + showFilters && + (tab === "transcript" + ? !transcriptAllSelected || search.length > 0 + : !debugAllSelected || search.length > 0); function clearFilters() { if (tab === "transcript") onKindsChange([...EVENT_KINDS]); @@ -873,35 +1064,37 @@ function EventsToolbar({ return (
- -
- {tab === "transcript" ? ( - - selected={selectedKinds} - options={EVENT_KINDS} - labelOf={(k) => EVENT_KIND_LABEL[k]} - onChange={onKindsChange} - /> - ) : ( - - selected={selectedDebugCategories} - options={availableDebugCategories} - labelOf={debugCategoryLabel} - onChange={onDebugCategoriesChange} - emptyMeansAll - /> - )} - - {isFiltering && ( - - )} -
+ + {showFilters && ( +
+ {tab === "transcript" ? ( + + selected={selectedKinds} + options={EVENT_KINDS} + labelOf={(k) => EVENT_KIND_LABEL[k]} + onChange={onKindsChange} + /> + ) : ( + + selected={selectedDebugCategories} + options={availableDebugCategories} + labelOf={debugCategoryLabel} + onChange={onDebugCategoriesChange} + emptyMeansAll + /> + )} + + {isFiltering && ( + + )} +
+ )} {isFiltering && totalCount > 0 && ( {filteredCount.toLocaleString()} of {totalCount.toLocaleString()} events @@ -909,13 +1102,16 @@ function EventsToolbar({ )} {model && ( )} + {!showFilters && commandTurn && }
); } @@ -939,6 +1135,14 @@ export default function RunStages() { : [], [stageEventsQuery.data, selectedStageId], ); + const stageKind = useMemo(() => turnsToStageKind(turns), [turns]); + const commandTurn = useMemo(() => { + for (let i = turns.length - 1; i >= 0; i -= 1) { + const t = turns[i]; + if (t.kind === "command") return t; + } + return null; + }, [turns]); const [openIndex, setOpenIndex] = useState(null); const [openDebugSeq, setOpenDebugSeq] = useState(null); @@ -1043,6 +1247,8 @@ export default function RunStages() {
{tab === "transcript" ? ( - turns.length > 0 && filteredTurns.length === 0 ? ( + stageKind === "command" ? ( + + ) : turns.length > 0 && filteredTurns.length === 0 ? (
No events match these filters.
@@ -1093,11 +1301,13 @@ export default function RunStages() {
{tab === "transcript" ? ( - setOpenIndex(null)} - /> + stageKind === "command" ? null : ( + setOpenIndex(null)} + /> + ) ) : ( Date: Thu, 7 May 2026 16:24:53 -0700 Subject: [PATCH 06/63] style(web): suppress sandbox_gone degraded banner on run files page The "Showing final patch only" banner fired for every finished run whose sandbox had been reaped, which is the normal post-run state and just adds noise. Other degraded reasons (provider_unsupported, sandbox_unreachable) still show their banners. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../routes/run-files/placeholders.test.tsx | 19 ++++++++++++------- .../app/routes/run-files/placeholders.tsx | 8 +++++--- 2 files changed, 17 insertions(+), 10 deletions(-) diff --git a/apps/fabro-web/app/routes/run-files/placeholders.test.tsx b/apps/fabro-web/app/routes/run-files/placeholders.test.tsx index 01324d4e9..c5aa6514d 100644 --- a/apps/fabro-web/app/routes/run-files/placeholders.test.tsx +++ b/apps/fabro-web/app/routes/run-files/placeholders.test.tsx @@ -93,13 +93,14 @@ describe("pickPlaceholder priority", () => { }); describe("bannerCopyForReason", () => { - test("each known reason gets distinct copy", () => { - const a = bannerCopyForReason("sandbox_gone"); + test("sandbox_gone is suppressed", () => { + expect(bannerCopyForReason("sandbox_gone")).toBeNull(); + }); + + test("provider_unsupported and sandbox_unreachable get distinct copy", () => { const b = bannerCopyForReason("provider_unsupported"); const c = bannerCopyForReason("sandbox_unreachable"); - expect(a).not.toBe(b); expect(b).not.toBe(c); - expect(a).toContain("cleaned up"); expect(b).toContain("provider"); expect(c).toContain("refresh"); }); @@ -146,9 +147,13 @@ describe("placeholder rendering", () => { ).toContain("submodule"); }); + test("DegradedBanner renders nothing for sandbox_gone", () => { + expect(renderedText()).toBe(""); + }); + test("DegradedBanner picks copy based on reason", () => { - expect(renderedText()).toContain( - "cleaned up", - ); + expect( + renderedText(), + ).toContain("provider"); }); }); diff --git a/apps/fabro-web/app/routes/run-files/placeholders.tsx b/apps/fabro-web/app/routes/run-files/placeholders.tsx index befac9f2b..369e148aa 100644 --- a/apps/fabro-web/app/routes/run-files/placeholders.tsx +++ b/apps/fabro-web/app/routes/run-files/placeholders.tsx @@ -103,19 +103,21 @@ export function DegradedBanner({ }: { reason?: RunFilesMetaDegradedReasonEnum; }) { + const copy = bannerCopyForReason(reason); + if (copy === null) return null; return (
- {bannerCopyForReason(reason)} + {copy}
); } export function bannerCopyForReason( reason: RunFilesMetaDegradedReasonEnum | undefined | string, -): string { +): string | null { 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."; + return null; case "provider_unsupported": return "Live diff isn't supported for this sandbox provider. Showing the patch captured at the last checkpoint."; case "sandbox_unreachable": From 6e979ef85aab9a0d2bd3855f1ebb84ef6a0ab1d5 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Thu, 7 May 2026 16:30:19 -0700 Subject: [PATCH 07/63] feat(web): add hover tooltip with absolute datetime on event timestamps Replaces native title attribute with a portal-based Tooltip component on the events-feed elapsed times and the run header's last-event timestamp, showing the full datetime (e.g. "04/24/2026, 1:23:40 PM") on hover. Co-Authored-By: Claude Opus 4.7 (1M context) --- apps/fabro-web/app/components/ui.tsx | 49 +++++++++++++++++++- apps/fabro-web/app/lib/format.ts | 18 +++++++ apps/fabro-web/app/routes/run-detail.test.ts | 7 ++- apps/fabro-web/app/routes/run-detail.tsx | 31 +++++++------ apps/fabro-web/app/routes/run-stages.tsx | 23 +++++---- 5 files changed, 104 insertions(+), 24 deletions(-) diff --git a/apps/fabro-web/app/components/ui.tsx b/apps/fabro-web/app/components/ui.tsx index 06565cf76..6c4701ca8 100644 --- a/apps/fabro-web/app/components/ui.tsx +++ b/apps/fabro-web/app/components/ui.tsx @@ -2,7 +2,8 @@ // exposes the primary button, secondary button, input, error message, and // copy button so the auth and in-app surfaces can match. -import { useState } from "react"; +import { useId, useRef, useState } from "react"; +import { createPortal } from "react-dom"; import { ClipboardDocumentCheckIcon, ClipboardIcon, @@ -62,3 +63,49 @@ export function CopyButton({ ); } + +export function Tooltip({ + label, + children, +}: { + label: string; + children: React.ReactNode; +}) { + const [open, setOpen] = useState(false); + const triggerRef = useRef(null); + const id = useId(); + const rect = open ? triggerRef.current?.getBoundingClientRect() : null; + const portalTarget = typeof document === "undefined" ? null : document.body; + + return ( + <> + setOpen(true)} + onMouseLeave={() => setOpen(false)} + onFocus={() => setOpen(true)} + onBlur={() => setOpen(false)} + aria-describedby={open ? id : undefined} + className="inline-flex" + > + {children} + + {rect && portalTarget + ? createPortal( + + {label} + , + portalTarget, + ) + : null} + + ); +} diff --git a/apps/fabro-web/app/lib/format.ts b/apps/fabro-web/app/lib/format.ts index 4a2703d3c..27c47e43b 100644 --- a/apps/fabro-web/app/lib/format.ts +++ b/apps/fabro-web/app/lib/format.ts @@ -47,6 +47,24 @@ export function formatRelativeTime(iso: string, now: number = Date.now()): strin return `${days}d ago`; } +/** + * Format an ISO 8601 timestamp as an absolute, human-readable datetime + * (e.g., "04/24/2026, 1:23:40 PM"). Falls back to the input if unparseable. + */ +export function formatAbsoluteTs(iso: string): string { + const ms = Date.parse(iso); + if (Number.isNaN(ms)) return iso; + return new Date(ms).toLocaleString("en-US", { + month: "2-digit", + day: "2-digit", + year: "numeric", + hour: "numeric", + minute: "2-digit", + second: "2-digit", + hour12: true, + }); +} + /** * Format seconds into a duration string for display (e.g., "1m 12s", "23s"). */ diff --git a/apps/fabro-web/app/routes/run-detail.test.ts b/apps/fabro-web/app/routes/run-detail.test.ts index 13222f298..564391980 100644 --- a/apps/fabro-web/app/routes/run-detail.test.ts +++ b/apps/fabro-web/app/routes/run-detail.test.ts @@ -369,7 +369,12 @@ describe("RunDetail full-height child routes", () => { expect(fullHeightRoot).toHaveLength(0); const outletWrappers = renderer.root.findAll( - (node) => node.type === "div" && node.props.className === "mt-6", + (node) => + node.type === "div" && + hasClasses(node.props.className, [ + "mt-6", + "pb-[var(--fabro-interview-dock-clearance)]", + ]), ); expect(outletWrappers).toHaveLength(1); }); diff --git a/apps/fabro-web/app/routes/run-detail.tsx b/apps/fabro-web/app/routes/run-detail.tsx index 4fa6f4a7a..3f38db945 100644 --- a/apps/fabro-web/app/routes/run-detail.tsx +++ b/apps/fabro-web/app/routes/run-detail.tsx @@ -21,7 +21,7 @@ import { SteerBar, type SteerBarHandle } from "../components/steer-bar"; import { SteerComposer } from "../components/steer-composer"; import { ErrorState } from "../components/state"; import { useToast } from "../components/toast"; -import { SECONDARY_BUTTON_CLASS } from "../components/ui"; +import { SECONDARY_BUTTON_CLASS, Tooltip } from "../components/ui"; import { isRunStatus, mapRunSummaryToRunItem, @@ -38,7 +38,7 @@ import { type LifecycleMutationResult, type PreviewMutationResult, } from "../lib/mutations"; -import { formatRelativeTime } from "../lib/format"; +import { formatAbsoluteTs, formatRelativeTime } from "../lib/format"; import { useRunEvents } from "../lib/run-events"; import { useRunToasts } from "../hooks/use-run-toasts"; import { useRun, useRunQuestions } from "../lib/queries"; @@ -217,9 +217,9 @@ export default function RunDetail({ params }: { params: { id: string } }) { const unarchivePending = unarchiveMutation.isMutating; const hasPendingQuestions = isBlocked && pendingQuestions.length > 0; const dockClearance = hasPendingQuestions ? "18rem" : "5rem"; - const rootStyle = fullHeight - ? ({ "--fabro-interview-dock-clearance": dockClearance } as CSSProperties) - : undefined; + const rootStyle = { + "--fabro-interview-dock-clearance": dockClearance, + } as CSSProperties; return (
)} {run.lastEventAt && ( - - + + + + )}
@@ -348,7 +347,13 @@ export default function RunDetail({ params }: { params: { id: string } }) { -
+
diff --git a/apps/fabro-web/app/routes/run-stages.tsx b/apps/fabro-web/app/routes/run-stages.tsx index d9c14c9ac..8258311a5 100644 --- a/apps/fabro-web/app/routes/run-stages.tsx +++ b/apps/fabro-web/app/routes/run-stages.tsx @@ -20,7 +20,8 @@ import { Marked } from "marked"; import { StageSidebar } from "../components/stage-sidebar"; import type { Stage } from "../components/stage-sidebar"; import { EmptyState } from "../components/state"; -import { formatBytes } from "../lib/format"; +import { Tooltip } from "../components/ui"; +import { formatAbsoluteTs, formatBytes } from "../lib/format"; import { useRun, useRunStageEvents, @@ -457,9 +458,11 @@ function EventRow({ {MetricIcon &&
-
+
{isOpen ? children : null}
@@ -1263,7 +1268,7 @@ export default function RunStages() { /> -
+
{tab === "transcript" ? ( stageKind === "command" ? ( From ebf9bc7ed0d6c903a0a056b0e569d1b341a6661a Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Thu, 7 May 2026 16:36:20 -0700 Subject: [PATCH 08/63] refactor(web): replace Graph tab with a Graph Source sidebar page The Graph tab duplicated the Overview's diagram. Drop it and the /runs/:id/graph route, and move the DOT source view to a dedicated /runs/:id/source page reachable from the sidebar. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../app/components/stage-sidebar.tsx | 8 +- apps/fabro-web/app/router.tsx | 4 +- apps/fabro-web/app/routes/run-detail.tsx | 1 - apps/fabro-web/app/routes/run-graph.tsx | 307 ------------------ apps/fabro-web/app/routes/run-source.tsx | 57 ++++ 5 files changed, 63 insertions(+), 314 deletions(-) delete mode 100644 apps/fabro-web/app/routes/run-graph.tsx create mode 100644 apps/fabro-web/app/routes/run-source.tsx diff --git a/apps/fabro-web/app/components/stage-sidebar.tsx b/apps/fabro-web/app/components/stage-sidebar.tsx index 0799a9ada..d18f85a1d 100644 --- a/apps/fabro-web/app/components/stage-sidebar.tsx +++ b/apps/fabro-web/app/components/stage-sidebar.tsx @@ -39,7 +39,7 @@ interface StageSidebarProps { stages: Stage[]; runId: string; selectedStageId?: string; - activeLink?: "settings" | "graph" | "logs"; + activeLink?: "settings" | "source" | "logs"; } export function StageSidebar({ stages, runId, selectedStageId, activeLink }: StageSidebarProps) { @@ -127,15 +127,15 @@ export function StageSidebar({ stages, runId, selectedStageId, activeLink }: Sta
  • - Workflow Graph + Graph Source
  • diff --git a/apps/fabro-web/app/router.tsx b/apps/fabro-web/app/router.tsx index 72d960f01..5d2c2e653 100644 --- a/apps/fabro-web/app/router.tsx +++ b/apps/fabro-web/app/router.tsx @@ -16,7 +16,7 @@ import * as RunDetail from "./routes/run-detail"; import * as RunOverview from "./routes/run-overview"; import * as RunStages from "./routes/run-stages"; import * as RunSettings from "./routes/run-settings"; -import * as RunGraph from "./routes/run-graph"; +import * as RunSource from "./routes/run-source"; import * as RunLogs from "./routes/run-logs"; import * as RunFiles from "./routes/run-files"; import * as RunBilling from "./routes/run-billing"; @@ -92,7 +92,7 @@ export const routes: RouteObject[] = [ route("stages", RunStages), route("stages/:stageId", RunStages), route("settings", RunSettings), - route("graph", RunGraph), + route("source", RunSource), route("logs", RunLogs), route("files", RunFiles), route("billing", RunBilling), diff --git a/apps/fabro-web/app/routes/run-detail.tsx b/apps/fabro-web/app/routes/run-detail.tsx index 3f38db945..e1162e641 100644 --- a/apps/fabro-web/app/routes/run-detail.tsx +++ b/apps/fabro-web/app/routes/run-detail.tsx @@ -56,7 +56,6 @@ const allTabs = [ { name: "Overview", path: "", count: null, demoOnly: false }, { name: "Stages", path: "/stages", count: null, demoOnly: false }, { name: "Files Changed", path: "/files", count: null, demoOnly: false }, - { name: "Graph", path: "/graph", count: null, demoOnly: false }, { name: "Billing", path: "/billing", count: null, demoOnly: false }, ]; diff --git a/apps/fabro-web/app/routes/run-graph.tsx b/apps/fabro-web/app/routes/run-graph.tsx deleted file mode 100644 index c42f5113f..000000000 --- a/apps/fabro-web/app/routes/run-graph.tsx +++ /dev/null @@ -1,307 +0,0 @@ -import { useCallback, useEffect, useMemo, useRef, useState } from "react"; -import { useParams } from "react-router"; -import type { BundledLanguage } from "@pierre/diffs"; -import { graphTheme } from "../lib/graph-theme"; -import { useRunGraph, useRunGraphSource, useRunStages } from "../lib/queries"; -import { LoadingState } from "../components/state"; -import { StageSidebar } from "../components/stage-sidebar"; -import { - GRAPH_DEFAULT_ZOOM_INDEX, - GRAPH_ZOOM_STEPS, - GraphToolbar, -} from "../components/graph-toolbar"; -import { CollapsibleFile } from "../components/collapsible-file"; -import { registerDotLanguage } from "../data/register-dot-language"; -import { mapRunStagesToSidebarStages } from "../lib/stage-sidebar"; - -export const handle = { wide: true }; - -type Direction = "LR" | "TB"; - -function buildDot(direction: Direction) { - return `digraph sync { - graph [label="Sync"] - rankdir=${direction} - bgcolor="transparent" - pad=0.5 - - node [ - fontname="ui-monospace, monospace" - fontsize=11 - fontcolor="${graphTheme.nodeText}" - color="${graphTheme.edgeColor}" - fillcolor="${graphTheme.nodeFill}" - style=filled - penwidth=1.2 - ] - edge [ - fontname="ui-monospace, monospace" - fontsize=9 - fontcolor="${graphTheme.fontcolor}" - color="${graphTheme.edgeColor}" - arrowsize=0.7 - penwidth=1.2 - ] - - start [shape=Mdiamond, label="Start", fillcolor="${graphTheme.startFill}", color="${graphTheme.startBorder}", fontcolor="${graphTheme.startText}"] - exit [shape=Msquare, label="Exit", fillcolor="${graphTheme.startFill}", color="${graphTheme.startBorder}", fontcolor="${graphTheme.startText}"] - - detect [label="Detect\\nDrift"] - propose [label="Propose\\nChanges"] - review [shape=hexagon, label="Review\\nChanges", fillcolor="${graphTheme.gateFill}", color="${graphTheme.gateBorder}", fontcolor="${graphTheme.gateText}"] - apply [label="Apply\\nChanges"] - - start -> detect - detect -> exit [label="No drift", style=dashed] - detect -> propose [label="Drift found"] - propose -> review - review -> apply [label="Accept"] - review -> propose [label="Revise", style=dashed] - apply -> exit -}`; -} - -function stripGraphTitle(svg: SVGSVGElement) { - const title = svg.querySelector(".graph > title"); - if (!title) return; - let sibling = title.nextElementSibling; - while (sibling && sibling.tagName === "text") { - const next = sibling.nextElementSibling; - sibling.remove(); - sibling = next; - } - title.remove(); -} - -type View = "graph" | "source"; - -export default function RunGraph() { - const { id } = useParams(); - const [direction, setDirection] = useState("LR"); - const [view, setView] = useState("graph"); - const stagesQuery = useRunStages(id); - const graphQuery = useRunGraph(id, direction); - const sourceQuery = useRunGraphSource(id, view === "source"); - const stages = useMemo( - () => mapRunStagesToSidebarStages(stagesQuery.data), - [stagesQuery.data], - ); - const graphSvg = graphQuery.data; - const containerRef = useRef(null); - const innerRef = useRef(null); - const svgRef = useRef(null); - const [error, setError] = useState(null); - const [zoomIndex, setZoomIndex] = useState(GRAPH_DEFAULT_ZOOM_INDEX); - const [pan, setPan] = useState({ x: 0, y: 0 }); - const [dotReady, setDotReady] = useState(false); - - useEffect(() => { - let cancelled = false; - registerDotLanguage().then(() => { - if (!cancelled) setDotReady(true); - }); - return () => { - cancelled = true; - }; - }, []); - const dragState = useRef<{ startX: number; startY: number; startPanX: number; startPanY: number } | null>(null); - const zoom = GRAPH_ZOOM_STEPS[zoomIndex]; - - useEffect(() => { - if (graphSvg === undefined && !graphQuery.error) return; - - let cancelled = false; - - async function render() { - try { - setError(null); - - if (graphQuery.error) { - setError("Failed to load graph"); - return; - } - - let svg: SVGSVGElement; - - if (graphSvg) { - const parser = new DOMParser(); - const doc = parser.parseFromString(graphSvg, "image/svg+xml"); - const parsed = doc.documentElement; - if (!(parsed instanceof SVGSVGElement)) { - setError("Invalid SVG from server"); - return; - } - svg = parsed; - } else { - // Fall back to hardcoded demo graph rendered client-side. - const { instance } = await import("@viz-js/viz"); - const viz = await instance(); - if (cancelled) return; - svg = viz.renderSVGElement(buildDot(direction)); - } - - stripGraphTitle(svg); - - svgRef.current = svg; - if (innerRef.current) { - innerRef.current.replaceChildren(svg); - } - } catch (e) { - setError(e instanceof Error ? e.message : "Failed to render diagram"); - } - } - - setPan({ x: 0, y: 0 }); - render(); - return () => { cancelled = true; }; - }, [direction, graphQuery.error, graphSvg]); - - const onPointerDown = useCallback((e: React.PointerEvent) => { - if ((e.target as HTMLElement).closest("button")) return; - e.currentTarget.setPointerCapture(e.pointerId); - dragState.current = { startX: e.clientX, startY: e.clientY, startPanX: pan.x, startPanY: pan.y }; - }, [pan]); - - const onPointerMove = useCallback((e: React.PointerEvent) => { - const drag = dragState.current; - if (!drag) return; - setPan({ - x: drag.startPanX + e.clientX - drag.startX, - y: drag.startPanY + e.clientY - drag.startY, - }); - }, []); - - const onPointerUp = useCallback(() => { - dragState.current = null; - }, []); - - const fitToWindow = useCallback(() => { - const svg = svgRef.current; - const container = containerRef.current; - if (!svg || !container) return; - - const svgW = svg.viewBox.baseVal.width || svg.getBoundingClientRect().width; - const svgH = svg.viewBox.baseVal.height || svg.getBoundingClientRect().height; - const padPx = 48; - const containerW = container.clientWidth - padPx; - const containerH = container.clientHeight - padPx; - - const fitPct = Math.min(containerW / svgW, containerH / svgH) * 100; - let best = 0; - for (let i = GRAPH_ZOOM_STEPS.length - 1; i >= 0; i--) { - if (GRAPH_ZOOM_STEPS[i] <= fitPct) { best = i; break; } - } - setZoomIndex(best); - setPan({ x: 0, y: 0 }); - }, []); - - if (error) { - return

    {error}

    ; - } - - return ( -
    - - -
    -
    - -
    - - - - {view === "source" && ( - - )} -
    -
    - ); -} - -function ViewToggle({ view, setView }: { view: View; setView: (v: View) => void }) { - const btn = - "rounded px-3 py-1.5 text-xs font-medium transition-colors"; - return ( -
    - - -
    - ); -} - -function SourcePanel({ - source, - loading, - dotReady, -}: { - source: string | null | undefined; - loading: boolean; - dotReady: boolean; -}) { - if (loading || !dotReady) { - return ( -
    - -
    - ); - } - if (!source) { - return ( -
    -

    No graph source available for this run.

    -
    - ); - } - return ( - - ); -} diff --git a/apps/fabro-web/app/routes/run-source.tsx b/apps/fabro-web/app/routes/run-source.tsx new file mode 100644 index 000000000..621e94de0 --- /dev/null +++ b/apps/fabro-web/app/routes/run-source.tsx @@ -0,0 +1,57 @@ +import { useEffect, useMemo, useState } from "react"; +import { useParams } from "react-router"; +import type { BundledLanguage } from "@pierre/diffs"; +import { useRunGraphSource, useRunStages } from "../lib/queries"; +import { LoadingState } from "../components/state"; +import { StageSidebar } from "../components/stage-sidebar"; +import { CollapsibleFile } from "../components/collapsible-file"; +import { registerDotLanguage } from "../data/register-dot-language"; +import { mapRunStagesToSidebarStages } from "../lib/stage-sidebar"; + +export const handle = { wide: true }; + +export default function RunSource() { + const { id } = useParams(); + const stagesQuery = useRunStages(id); + const sourceQuery = useRunGraphSource(id, true); + const stages = useMemo( + () => mapRunStagesToSidebarStages(stagesQuery.data), + [stagesQuery.data], + ); + const [dotReady, setDotReady] = useState(false); + + useEffect(() => { + let cancelled = false; + registerDotLanguage().then(() => { + if (!cancelled) setDotReady(true); + }); + return () => { + cancelled = true; + }; + }, []); + + const source = sourceQuery.data; + const loading = source === undefined && !sourceQuery.error; + + return ( +
    + + +
    + {loading || !dotReady ? ( +
    + +
    + ) : !source ? ( +
    +

    No graph source available for this run.

    +
    + ) : ( + + )} +
    +
    + ); +} From de26face0fddeba424a0cb25ab1f248cc3a33981 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Thu, 7 May 2026 17:05:13 -0700 Subject: [PATCH 09/63] feat(web): collapse consecutive same-tool calls into one event row Successful tool calls of the same tool that run back-to-back (e.g., five Bash curls to the same endpoint) now collapse into a single Tool group row labelled "Bash x5", summing durations and using the first call's start time. Errored calls and tool-name boundaries break the run, so distinct work is never hidden. Clicking the group opens a panel that lists each child with its input preview and per-call duration; an inline accordion reveals the full Tool use / Tool result for one child at a time. Co-Authored-By: Claude Opus 4.7 (1M context) --- apps/fabro-web/app/routes/run-stages.test.ts | 154 ++++++++- apps/fabro-web/app/routes/run-stages.tsx | 329 ++++++++++++++++++- 2 files changed, 468 insertions(+), 15 deletions(-) diff --git a/apps/fabro-web/app/routes/run-stages.test.ts b/apps/fabro-web/app/routes/run-stages.test.ts index d64b47972..dd8282d23 100644 --- a/apps/fabro-web/app/routes/run-stages.test.ts +++ b/apps/fabro-web/app/routes/run-stages.test.ts @@ -1,7 +1,12 @@ import { describe, expect, test } from "bun:test"; import type { EventEnvelope } from "@qltysh/fabro-api-client"; -import { eventsToActivity, extractStageModel, turnsToStageKind } from "./run-stages"; +import { + eventsToActivity, + extractStageModel, + groupConsecutiveTools, + turnsToStageKind, +} from "./run-stages"; function envelope(seq: number, partial: Partial): EventEnvelope { return { @@ -256,6 +261,153 @@ describe("eventsToActivity", () => { }); }); +describe("groupConsecutiveTools", () => { + type Filtered = Parameters[0]; + + function tool(opts: { + ts: string; + toolName: string; + durationMs?: number; + isError?: boolean; + input?: string; + result?: string; + }) { + return { + kind: "tool" as const, + ts: opts.ts, + toolName: opts.toolName, + input: opts.input ?? "", + result: opts.result ?? "", + isError: opts.isError ?? false, + durationMs: opts.durationMs ?? 0, + }; + } + + function entry(turn: ReturnType | { kind: "system"; ts: string; content: string } | { kind: "assistant"; ts: string; content: string; inputTokens: number; outputTokens: number }, index: number): Filtered[number] { + return { turn, index }; + } + + test("empty input returns empty output", () => { + expect(groupConsecutiveTools([])).toEqual([]); + }); + + test("single tool turn becomes a single, not a group", () => { + const t = tool({ ts: "2026-04-09T12:00:00Z", toolName: "shell", durationMs: 100 }); + expect(groupConsecutiveTools([entry(t, 0)])).toEqual([ + { kind: "single", turn: t, turnIndex: 0 }, + ]); + }); + + test("two consecutive same-tool successes form a group of 2", () => { + const a = tool({ ts: "2026-04-09T12:00:00Z", toolName: "shell", durationMs: 1000 }); + const b = tool({ ts: "2026-04-09T12:00:01Z", toolName: "shell", durationMs: 2000 }); + const result = groupConsecutiveTools([entry(a, 0), entry(b, 1)]); + expect(result).toEqual([ + { + kind: "group", + toolName: "shell", + ts: "2026-04-09T12:00:00Z", + durationMs: 3000, + children: [ + { turn: a, turnIndex: 0 }, + { turn: b, turnIndex: 1 }, + ], + }, + ]); + }); + + test("five consecutive same-tool successes form one group; durations summed; ts is first", () => { + const turns = [0, 1, 2, 3, 4].map((i) => + tool({ + ts: `2026-04-09T12:00:0${i}Z`, + toolName: "shell", + durationMs: (i + 1) * 1000, + }), + ); + const filtered = turns.map((t, i) => entry(t, i)); + const result = groupConsecutiveTools(filtered); + expect(result).toHaveLength(1); + const item = result[0]; + expect(item.kind).toBe("group"); + if (item.kind === "group") { + expect(item.ts).toBe("2026-04-09T12:00:00Z"); + expect(item.durationMs).toBe(15000); + expect(item.children.map((c) => c.turnIndex)).toEqual([0, 1, 2, 3, 4]); + } + }); + + test("a different tool between same-tool calls breaks the group boundary", () => { + const a = tool({ ts: "2026-04-09T12:00:00Z", toolName: "shell", durationMs: 1 }); + const b = tool({ ts: "2026-04-09T12:00:01Z", toolName: "shell", durationMs: 1 }); + const c = tool({ ts: "2026-04-09T12:00:02Z", toolName: "read_file", durationMs: 1 }); + const d = tool({ ts: "2026-04-09T12:00:03Z", toolName: "shell", durationMs: 1 }); + const e = tool({ ts: "2026-04-09T12:00:04Z", toolName: "shell", durationMs: 1 }); + const result = groupConsecutiveTools([ + entry(a, 0), + entry(b, 1), + entry(c, 2), + entry(d, 3), + entry(e, 4), + ]); + expect(result.map((r) => r.kind)).toEqual(["group", "single", "group"]); + if (result[0].kind === "group") { + expect(result[0].children.map((c) => c.turnIndex)).toEqual([0, 1]); + } + if (result[1].kind === "single") { + expect(result[1].turnIndex).toBe(2); + } + if (result[2].kind === "group") { + expect(result[2].children.map((c) => c.turnIndex)).toEqual([3, 4]); + } + }); + + test("an errored tool call is never grouped and breaks the run", () => { + const a = tool({ ts: "2026-04-09T12:00:00Z", toolName: "shell" }); + const errored = tool({ ts: "2026-04-09T12:00:01Z", toolName: "shell", isError: true }); + const c = tool({ ts: "2026-04-09T12:00:02Z", toolName: "shell" }); + const d = tool({ ts: "2026-04-09T12:00:03Z", toolName: "shell" }); + const result = groupConsecutiveTools([ + entry(a, 0), + entry(errored, 1), + entry(c, 2), + entry(d, 3), + ]); + expect(result.map((r) => r.kind)).toEqual(["single", "single", "group"]); + if (result[1].kind === "single") { + expect(result[1].turn).toBe(errored); + } + if (result[2].kind === "group") { + expect(result[2].children.map((c) => c.turnIndex)).toEqual([2, 3]); + } + }); + + test("non-tool turns flush the buffer correctly", () => { + const a = tool({ ts: "2026-04-09T12:00:00Z", toolName: "shell" }); + const b = tool({ ts: "2026-04-09T12:00:01Z", toolName: "shell" }); + const msg = { + kind: "assistant" as const, + ts: "2026-04-09T12:00:02Z", + content: "thinking", + inputTokens: 0, + outputTokens: 0, + }; + const c = tool({ ts: "2026-04-09T12:00:03Z", toolName: "shell" }); + const result = groupConsecutiveTools([ + entry(a, 0), + entry(b, 1), + entry(msg, 2), + entry(c, 3), + ]); + expect(result.map((r) => r.kind)).toEqual(["group", "single", "single"]); + if (result[0].kind === "group") { + expect(result[0].children.map((c) => c.turnIndex)).toEqual([0, 1]); + } + if (result[2].kind === "single") { + expect(result[2].turnIndex).toBe(3); + } + }); +}); + describe("turnsToStageKind", () => { test("classifies a stage with only command events as command", () => { const events: EventEnvelope[] = [ diff --git a/apps/fabro-web/app/routes/run-stages.tsx b/apps/fabro-web/app/routes/run-stages.tsx index 8258311a5..4d502fce3 100644 --- a/apps/fabro-web/app/routes/run-stages.tsx +++ b/apps/fabro-web/app/routes/run-stages.tsx @@ -9,6 +9,8 @@ import { import { XMarkIcon } from "@heroicons/react/24/outline"; import { CheckIcon, + ChevronDownIcon, + ChevronRightIcon, ChevronUpDownIcon, CpuChipIcon, FunnelIcon, @@ -53,6 +55,10 @@ type TurnType = type CommandTurn = Extract; type StageKind = "agent" | "command"; +type PanelSelection = + | { kind: "single"; turnIndex: number } + | { kind: "group"; childTurnIndices: number[] }; + const STAGE_ACTIVITY_EVENT_SET = new Set(STAGE_ACTIVITY_EVENT_TYPES); const EVENT_KINDS = ["system", "assistant", "tool", "command"] as const; @@ -229,6 +235,59 @@ export function turnsToStageKind(turns: TurnType[]): StageKind { return hasCommand ? "command" : "agent"; } +type ToolTurn = Extract; + +export type DisplayItem = + | { kind: "single"; turn: TurnType; turnIndex: number } + | { + kind: "group"; + toolName: string; + ts: string; + durationMs: number; + children: { turn: ToolTurn; turnIndex: number }[]; + }; + +export function groupConsecutiveTools( + filtered: { turn: TurnType; index: number }[], +): DisplayItem[] { + const out: DisplayItem[] = []; + let buf: { turn: ToolTurn; turnIndex: number }[] = []; + + function flush() { + if (buf.length === 0) return; + if (buf.length === 1) { + out.push({ kind: "single", turn: buf[0].turn, turnIndex: buf[0].turnIndex }); + } else { + const first = buf[0].turn; + const totalMs = buf.reduce((sum, b) => sum + b.turn.durationMs, 0); + out.push({ + kind: "group", + toolName: first.toolName, + ts: first.ts, + durationMs: totalMs, + children: buf, + }); + } + buf = []; + } + + for (const { turn, index } of filtered) { + const groupable = turn.kind === "tool" && !turn.isError; + if (groupable && (buf.length === 0 || buf[0].turn.toolName === turn.toolName)) { + buf.push({ turn, turnIndex: index }); + continue; + } + flush(); + if (groupable) { + buf.push({ turn, turnIndex: index }); + } else { + out.push({ kind: "single", turn, turnIndex: index }); + } + } + flush(); + return out; +} + const STAGE_MODEL_EVENT_NAMES = new Set([ "stage.prompt", "agent.session.activated", @@ -467,6 +526,50 @@ function EventRow({ ); } +const TOOL_GROUP_TONE = "bg-mint/15 text-mint"; + +function ToolGroupRow({ + group, + runStart, + selected, + onSelect, +}: { + group: Extract; + runStart: string | undefined; + selected: boolean; + onSelect: () => void; +}) { + const metric = group.durationMs > 0 ? formatDurationMs(group.durationMs) : null; + return ( + + ); +} + function DebugRow({ event, runStart, @@ -868,6 +971,152 @@ function EventDetailsPanel({ ); } +const TOOL_INPUT_PREVIEW_KEYS = ["command", "path", "pattern", "url", "query", "script"]; + +function toolInputPreview(turn: ToolTurn): string { + const raw = turn.input; + if (!raw) return ""; + try { + const parsed = JSON.parse(raw); + if (typeof parsed === "string") return oneLine(parsed); + if (parsed && typeof parsed === "object" && !Array.isArray(parsed)) { + const obj = parsed as Record; + for (const k of TOOL_INPUT_PREVIEW_KEYS) { + const v = obj[k]; + if (typeof v === "string" && v) return oneLine(v); + } + } + } catch { + // input wasn't valid JSON; fall through to oneLine of the raw string + } + return oneLine(raw); +} + +function ToolGroupChildRow({ + child, + runStart, + expanded, + onToggle, +}: { + child: { turn: ToolTurn; turnIndex: number }; + runStart: string | undefined; + expanded: boolean; + onToggle: () => void; +}) { + const { turn } = child; + const metric = turn.durationMs > 0 ? formatDurationMs(turn.durationMs) : null; + const elapsed = formatElapsed(turn.ts, runStart); + const Chevron = expanded ? ChevronDownIcon : ChevronRightIcon; + return ( + + ); +} + +function ToolGroupDetails({ + group, + runStart, +}: { + group: Extract; + runStart: string | undefined; +}) { + const [expandedIndex, setExpandedIndex] = useState(null); + useEffect(() => { + setExpandedIndex(null); + }, [group]); + + const elapsed = formatElapsed(group.ts, runStart); + const totalDuration = group.durationMs > 0 ? formatDurationMs(group.durationMs) : null; + + return ( +
    +
    + + Tool + + + {humanizeToolName(group.toolName)}{" "} + x{group.children.length} + + + {elapsed} + {totalDuration && ( + <> + + +
    +
      + {group.children.map((child, i) => ( +
    • + + setExpandedIndex((current) => (current === i ? null : i)) + } + /> + {expandedIndex === i && ( +
      + +
      + )} +
      • + ))} +
    +
    + ); +} + +function ToolGroupDetailsPanel({ + group, + runStart, + onClose, +}: { + group: Extract | null; + runStart: string | undefined; + onClose: () => void; +}) { + return ( + + {group ? : null} + + ); +} + function DebugEventDetails({ event }: { event: EventEnvelope }) { const text = useMemo(() => JSON.stringify(event, null, 2), [event]); const tokens = useMemo(() => highlightJson(text), [text]); @@ -1149,13 +1398,12 @@ export default function RunStages() { return null; }, [turns]); - const [openIndex, setOpenIndex] = useState(null); + const [panelSelection, setPanelSelection] = useState(null); const [openDebugSeq, setOpenDebugSeq] = useState(null); useEffect(() => { - setOpenIndex(null); + setPanelSelection(null); setOpenDebugSeq(null); }, [selectedStageId]); - const openTurn = openIndex != null ? turns[openIndex] ?? null : null; const [tab, setTab] = useState("transcript"); const [selectedKinds, setSelectedKinds] = useState([ @@ -1174,6 +1422,24 @@ export default function RunStages() { }); return out; }, [turns, selectedKinds, search]); + const displayItems = useMemo( + () => groupConsecutiveTools(filteredTurns), + [filteredTurns], + ); + + const openTurn = + panelSelection?.kind === "single" ? turns[panelSelection.turnIndex] ?? null : null; + const openGroup = useMemo | null>(() => { + if (panelSelection?.kind !== "group") return null; + const wanted = panelSelection.childTurnIndices; + for (const item of displayItems) { + if (item.kind !== "group") continue; + if (item.children.length !== wanted.length) continue; + const matches = item.children.every((c, i) => c.turnIndex === wanted[i]); + if (matches) return item; + } + return null; + }, [displayItems, panelSelection]); const debugEvents = useMemo(() => { if (!selectedStageId) return []; @@ -1277,15 +1543,44 @@ export default function RunStages() { No events match these filters.
    • ) : ( - filteredTurns.map(({ turn, index }) => ( - setOpenIndex(index)} - /> - )) + displayItems.map((item) => { + if (item.kind === "single") { + return ( + + setPanelSelection({ kind: "single", turnIndex: item.turnIndex }) + } + /> + ); + } + const childIndices = item.children.map((c) => c.turnIndex); + const groupKey = `group-${childIndices.join("-")}`; + const isSelected = + panelSelection?.kind === "group" && + panelSelection.childTurnIndices.length === childIndices.length && + panelSelection.childTurnIndices.every((v, i) => v === childIndices[i]); + return ( + + setPanelSelection({ + kind: "group", + childTurnIndices: childIndices, + }) + } + /> + ); + }) ) ) : debugEvents.length > 0 && filteredDebugEvents.length === 0 ? (
    @@ -1306,11 +1601,17 @@ export default function RunStages() {
    {tab === "transcript" ? ( - stageKind === "command" ? null : ( + stageKind === "command" ? null : panelSelection?.kind === "group" ? ( + setPanelSelection(null)} + /> + ) : ( setOpenIndex(null)} + onClose={() => setPanelSelection(null)} /> ) ) : ( From 23cb211cce777eb1d8cd5302dc62a70ffc97b3cf Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Thu, 7 May 2026 17:34:32 -0700 Subject: [PATCH 10/63] feat(runs): surface diff summary counts Compute cheap diff stats on checkpoint and terminal events, roll them into run summaries, and use them for the Files Changed tab badge without fetching full file diffs. --- apps/fabro-web/app/routes/run-detail.test.ts | 35 ++++- apps/fabro-web/app/routes/run-detail.tsx | 7 +- docs/public/api-reference/fabro-api.yaml | 29 ++++ lib/crates/fabro-api/build.rs | 1 + lib/crates/fabro-api/src/lib.rs | 2 +- .../tests/diff_summary_round_trip.rs | 37 +++++ .../fabro-api/tests/run_summary_round_trip.rs | 15 +- .../fabro-cli/src/commands/run/attach.rs | 4 +- .../fabro-cli/src/commands/run/runner.rs | 3 + lib/crates/fabro-server/src/demo/mod.rs | 1 + lib/crates/fabro-server/src/server.rs | 10 ++ lib/crates/fabro-server/src/server/tests.rs | 9 ++ .../fabro-server/tests/it/api/run_files.rs | 1 + lib/crates/fabro-store/src/run_state.rs | 113 ++++++++++++++ lib/crates/fabro-types/src/diff.rs | 7 + lib/crates/fabro-types/src/event_envelope.rs | 2 + lib/crates/fabro-types/src/lib.rs | 2 +- lib/crates/fabro-types/src/run_event/mod.rs | 68 +++++++++ lib/crates/fabro-types/src/run_event/run.rs | 8 +- lib/crates/fabro-types/src/run_event/stage.rs | 4 +- lib/crates/fabro-types/src/run_projection.rs | 8 +- lib/crates/fabro-types/src/run_summary.rs | 39 ++++- .../fabro-workflow/src/event/convert.rs | 8 + lib/crates/fabro-workflow/src/event/events.rs | 12 +- lib/crates/fabro-workflow/src/git.rs | 1 + .../fabro-workflow/src/lifecycle/event.rs | 2 + .../fabro-workflow/src/lifecycle/git.rs | 143 +++++++++++++++++- .../fabro-workflow/src/operations/archive.rs | 2 + .../fabro-workflow/src/operations/fork.rs | 2 + .../fabro-workflow/src/operations/start.rs | 8 + .../fabro-workflow/src/pipeline/finalize.rs | 136 ++++++++++++++++- .../src/pipeline/pull_request.rs | 2 + .../fabro-workflow/src/pipeline/retro.rs | 1 + lib/crates/fabro-workflow/src/sandbox_git.rs | 23 ++- lib/crates/fabro-workflow/src/test_support.rs | 1 + .../src/.openapi-generator/FILES | 1 + .../src/models/diff-summary.ts | 33 ++++ .../fabro-api-client/src/models/index.ts | 1 + .../src/models/run-projection.ts | 4 + .../src/models/run-summary.ts | 4 + 40 files changed, 759 insertions(+), 30 deletions(-) create mode 100644 lib/crates/fabro-api/tests/diff_summary_round_trip.rs create mode 100644 lib/packages/fabro-api-client/src/models/diff-summary.ts diff --git a/apps/fabro-web/app/routes/run-detail.test.ts b/apps/fabro-web/app/routes/run-detail.test.ts index 564391980..4cffe15fa 100644 --- a/apps/fabro-web/app/routes/run-detail.test.ts +++ b/apps/fabro-web/app/routes/run-detail.test.ts @@ -69,7 +69,7 @@ type RunDetailActionResult = import("./run-detail").RunDetailActionResult; const h = createElement; -function makeRunSummary(status = "succeeded") { +function makeRunSummary(status = "succeeded", diffSummary: any = null) { return { run_id: "run_1", title: "Run 1", @@ -80,6 +80,7 @@ function makeRunSummary(status = "succeeded") { duration_ms: null, elapsed_secs: null, source_directory: null, + diff_summary: diffSummary, }; } @@ -105,12 +106,14 @@ async function renderRunDetail({ initialEntry, status = "succeeded", questions = [], + diffSummary = null, }: { initialEntry: string; status?: string; questions?: any[]; + diffSummary?: any; }) { - currentRunSummary = makeRunSummary(status); + currentRunSummary = makeRunSummary(status, diffSummary); currentQuestions = questions; (globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true; @@ -154,6 +157,12 @@ function hasClasses(value: unknown, classes: string[]) { return classes.every((className) => tokens.includes(className)); } +function tabCountBadges(renderer: TestRenderer.ReactTestRenderer) { + return renderer.root.findAll( + (node) => node.type === "span" && hasClasses(node.props.className, ["tabular-nums"]), + ); +} + describe("lifecycleActionVisibility", () => { test("shows cancel for active cancellable states and hides it elsewhere", () => { expect(lifecycleActionVisibility("submitted").showPrimaryCancel).toBe(true); @@ -330,6 +339,28 @@ describe("RunDetail full-height child routes", () => { expect(outletWrappers).toHaveLength(1); }); + test("shows the Files Changed tab badge from run summary diff stats", async () => { + const renderer = await renderRunDetail({ + initialEntry: "/runs/run_1/files", + diffSummary: { + files_changed: 7, + additions: 30, + deletions: 11, + }, + }); + + const badges = tabCountBadges(renderer); + expect(badges.map((badge) => badge.children.join(""))).toContain("7"); + }); + + test("hides the Files Changed tab badge when diff stats are absent", async () => { + const renderer = await renderRunDetail({ + initialEntry: "/runs/run_1/files", + }); + + expect(tabCountBadges(renderer)).toHaveLength(0); + }); + test("keeps blocked full-height children clear of the interview dock without an h-72 sibling", async () => { const renderer = await renderRunDetail({ initialEntry: "/runs/run_1/files", diff --git a/apps/fabro-web/app/routes/run-detail.tsx b/apps/fabro-web/app/routes/run-detail.tsx index e1162e641..041c3ae72 100644 --- a/apps/fabro-web/app/routes/run-detail.tsx +++ b/apps/fabro-web/app/routes/run-detail.tsx @@ -149,7 +149,12 @@ export default function RunDetail({ params }: { params: { id: string } }) { const unarchiveMutation = useUnarchiveRun(params.id); const interruptMutation = useInterruptRun(params.id); const { push, dismiss } = useToast(); - const tabs = allTabs.filter((t) => !t.demoOnly || demoMode); + const filesCount = runQuery.data?.diff_summary?.files_changed ?? null; + const tabs = allTabs + .map((tab) => + tab.name === "Files Changed" ? { ...tab, count: filesCount } : tab, + ) + .filter((t) => !t.demoOnly || demoMode); const lifecycleToastStateRef = useRef(INITIAL_LIFECYCLE_TOAST_STATE); const steerBarRef = useRef(null); const [steerOpen, setSteerOpen] = useState(false); diff --git a/docs/public/api-reference/fabro-api.yaml b/docs/public/api-reference/fabro-api.yaml index 08991e412..aa0965337 100644 --- a/docs/public/api-reference/fabro-api.yaml +++ b/docs/public/api-reference/fabro-api.yaml @@ -5652,6 +5652,10 @@ components: additionalProperties: true final_patch: type: ["string", "null"] + diff_summary: + oneOf: + - $ref: "#/components/schemas/DiffSummary" + - type: "null" pull_request: type: ["object", "null"] additionalProperties: true @@ -5727,6 +5731,10 @@ components: format: int64 superseded_by: type: ["string", "null"] + diff_summary: + oneOf: + - $ref: "#/components/schemas/DiffSummary" + - type: "null" ForkRequest: description: Request body for creating a new run from a source run checkpoint. @@ -6615,6 +6623,27 @@ components: description: Total lines deleted. example: 234 + DiffSummary: + description: Cheap aggregate file and line counts for a run diff. + type: object + required: + - files_changed + - additions + - deletions + properties: + files_changed: + type: integer + description: Total number of changed files, including binary files. + example: 42 + additions: + type: integer + description: Total lines added across text files. + example: 567 + deletions: + type: integer + description: Total lines deleted across text files. + example: 234 + RunFilesMeta: description: | Metadata for a `PaginatedRunFileList` response. diff --git a/lib/crates/fabro-api/build.rs b/lib/crates/fabro-api/build.rs index b63e23029..50b868312 100644 --- a/lib/crates/fabro-api/build.rs +++ b/lib/crates/fabro-api/build.rs @@ -179,6 +179,7 @@ fn main() { &[], ), ("RunSummary", "fabro_types::RunSummary", &[]), + ("DiffSummary", "fabro_types::DiffSummary", &[]), ( "RepositoryReference", "fabro_types::RepositoryReference", diff --git a/lib/crates/fabro-api/src/lib.rs b/lib/crates/fabro-api/src/lib.rs index 37a92ec8c..41433565c 100644 --- a/lib/crates/fabro-api/src/lib.rs +++ b/lib/crates/fabro-api/src/lib.rs @@ -30,7 +30,7 @@ pub mod types { }; pub use fabro_types::{ AuthMethod, BilledTokenCounts, CommandOutputStream, CommandTermination, DiffStats, - DirtyStatus, EventEnvelope, GitContext, IdpIdentity, InterviewOption, + DiffSummary, DirtyStatus, EventEnvelope, GitContext, IdpIdentity, InterviewOption, InterviewQuestionRecord, PendingInterviewRecord, PreRunPushOutcome, Principal, QuestionType, RepositoryReference, RunClientProvenance, RunEvent, RunProjection, RunProvenance, RunServerProvenance, RunSummary, SecretMetadata, SecretType, ServerSettings, diff --git a/lib/crates/fabro-api/tests/diff_summary_round_trip.rs b/lib/crates/fabro-api/tests/diff_summary_round_trip.rs new file mode 100644 index 000000000..09f79bb34 --- /dev/null +++ b/lib/crates/fabro-api/tests/diff_summary_round_trip.rs @@ -0,0 +1,37 @@ +use std::any::{TypeId, type_name}; + +use fabro_api::types::DiffSummary as ApiDiffSummary; +use fabro_types::DiffSummary; +use serde_json::json; + +#[test] +fn diff_summary_reuses_canonical_type() { + assert_same_type::(); +} + +#[test] +fn diff_summary_serializes_with_required_integer_fields() { + let summary = DiffSummary { + files_changed: 3, + additions: 12, + deletions: 4, + }; + assert_eq!( + serde_json::to_value(summary).unwrap(), + json!({ + "files_changed": 3, + "additions": 12, + "deletions": 4, + }) + ); +} + +fn assert_same_type() { + assert_eq!( + TypeId::of::(), + TypeId::of::(), + "{} should be the same type as {}", + type_name::(), + type_name::() + ); +} diff --git a/lib/crates/fabro-api/tests/run_summary_round_trip.rs b/lib/crates/fabro-api/tests/run_summary_round_trip.rs index 8aef77553..69b32fb66 100644 --- a/lib/crates/fabro-api/tests/run_summary_round_trip.rs +++ b/lib/crates/fabro-api/tests/run_summary_round_trip.rs @@ -6,7 +6,7 @@ use fabro_api::types::{ RepositoryReference as ApiRepositoryReference, RunSummary as ApiRunSummary, }; use fabro_types::status::{RunStatus, SuccessReason, TerminalStatus}; -use fabro_types::{RepositoryReference, RunId, RunSummary}; +use fabro_types::{DiffSummary, RepositoryReference, RunId, RunSummary}; use serde_json::json; #[test] @@ -41,6 +41,11 @@ fn run_summary_json_matches_openapi_shape() { Some(42_000), Some(123), Some(superseded_by), + Some(DiffSummary { + files_changed: 3, + additions: 12, + deletions: 4, + }), ); assert_eq!( @@ -74,7 +79,12 @@ fn run_summary_json_matches_openapi_shape() { "duration_ms": 42000, "elapsed_secs": 42.0, "total_usd_micros": 123, - "superseded_by": superseded_by.to_string() + "superseded_by": superseded_by.to_string(), + "diff_summary": { + "files_changed": 3, + "additions": 12, + "deletions": 4 + } }) ); } @@ -117,6 +127,7 @@ fn run_summary_deserializes_when_optional_fields_are_absent() { assert_eq!(summary.elapsed_secs, None); assert_eq!(summary.total_usd_micros, None); assert_eq!(summary.superseded_by, None); + assert_eq!(summary.diff_summary, None); } fn assert_same_type() { diff --git a/lib/crates/fabro-cli/src/commands/run/attach.rs b/lib/crates/fabro-cli/src/commands/run/attach.rs index fe97b1d5e..5684932b1 100644 --- a/lib/crates/fabro-cli/src/commands/run/attach.rs +++ b/lib/crates/fabro-cli/src/commands/run/attach.rs @@ -106,7 +106,7 @@ pub(crate) async fn attach_run_with_client( } let stream = client.attach_run_events(run_id, Some(next_seq)).await?; - attach_live_run_with_client( + Box::pin(attach_live_run_with_client( client, run_id, replay_events, @@ -119,7 +119,7 @@ pub(crate) async fn attach_run_with_client( json_output, }, printer, - ) + )) .await } diff --git a/lib/crates/fabro-cli/src/commands/run/runner.rs b/lib/crates/fabro-cli/src/commands/run/runner.rs index f7c9e59e3..607468fb4 100644 --- a/lib/crates/fabro-cli/src/commands/run/runner.rs +++ b/lib/crates/fabro-cli/src/commands/run/runner.rs @@ -773,6 +773,7 @@ mod tests { total_usd_micros: None, final_git_commit_sha: None, final_patch: None, + diff_summary: None, billing: None, })), Some(WorkerTitlePhase::Succeeded) @@ -785,6 +786,7 @@ mod tests { reason: FailureReason::Cancelled, git_commit_sha: None, final_patch: None, + diff_summary: None, })), Some(WorkerTitlePhase::Cancelled) ); @@ -796,6 +798,7 @@ mod tests { reason: FailureReason::Terminated, git_commit_sha: None, final_patch: None, + diff_summary: None, })), Some(WorkerTitlePhase::Failed) ); diff --git a/lib/crates/fabro-server/src/demo/mod.rs b/lib/crates/fabro-server/src/demo/mod.rs index 836051585..474a08f43 100644 --- a/lib/crates/fabro-server/src/demo/mod.rs +++ b/lib/crates/fabro-server/src/demo/mod.rs @@ -889,6 +889,7 @@ mod runs { elapsed_secs.and_then(duration_ms_from_secs), total_usd_micros, None, + None, ) } diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index f08a63c7c..ce4fca90f 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -1983,6 +1983,7 @@ pub(crate) async fn reconcile_incomplete_runs_on_startup( reason, git_commit_sha: None, final_patch: None, + diff_summary: None, }, ) .await?; @@ -2036,6 +2037,7 @@ async fn persist_shutdown_run_failures( reason, git_commit_sha: None, final_patch: None, + diff_summary: None, }, ) .await?; @@ -2110,6 +2112,7 @@ async fn persist_cancelled_run_status(state: &AppState, run_id: RunId) -> anyhow reason: FailureReason::Cancelled, git_commit_sha: None, final_patch: None, + diff_summary: None, }, ) .await @@ -2148,6 +2151,7 @@ async fn fail_run_before_execution( reason, git_commit_sha: None, final_patch: None, + diff_summary: None, }, ) .await @@ -2425,6 +2429,7 @@ async fn append_worker_exit_failure( reason, git_commit_sha: None, final_patch: None, + diff_summary: None, }, ) .await @@ -3085,6 +3090,7 @@ async fn execute_run_subprocess(state: Arc, run_id: RunId) { reason: FailureReason::LaunchFailed, git_commit_sha: None, final_patch: None, + diff_summary: None, }, ) .await; @@ -3112,6 +3118,7 @@ async fn execute_run_subprocess(state: Arc, run_id: RunId) { reason: FailureReason::LaunchFailed, git_commit_sha: None, final_patch: None, + diff_summary: None, }, ) .await; @@ -3142,6 +3149,7 @@ async fn execute_run_subprocess(state: Arc, run_id: RunId) { reason: FailureReason::LaunchFailed, git_commit_sha: None, final_patch: None, + diff_summary: None, }, ) .await; @@ -3163,6 +3171,7 @@ async fn execute_run_subprocess(state: Arc, run_id: RunId) { reason: FailureReason::LaunchFailed, git_commit_sha: None, final_patch: None, + diff_summary: None, }, ) .await; @@ -3196,6 +3205,7 @@ async fn execute_run_subprocess(state: Arc, run_id: RunId) { reason: FailureReason::Terminated, git_commit_sha: None, final_patch: None, + diff_summary: None, }, ) .await; diff --git a/lib/crates/fabro-server/src/server/tests.rs b/lib/crates/fabro-server/src/server/tests.rs index de9c20afe..8e1e8d608 100644 --- a/lib/crates/fabro-server/src/server/tests.rs +++ b/lib/crates/fabro-server/src/server/tests.rs @@ -2668,6 +2668,7 @@ async fn run_billing_dedups_retried_nodes_and_sums_their_durations() { restart_failure_signatures: std::collections::BTreeMap::new(), node_visits: std::collections::BTreeMap::from([("verify".to_string(), 2usize)]), diff: None, + diff_summary: None, }, ) .await @@ -2796,6 +2797,7 @@ async fn run_billing_sums_usage_across_retry_visits_and_uses_latest_model() { restart_failure_signatures: std::collections::BTreeMap::new(), node_visits: std::collections::BTreeMap::from([("verify".to_string(), 2usize)]), diff: None, + diff_summary: None, }, ) .await @@ -3407,6 +3409,7 @@ async fn create_completed_run_ready_for_pull_request( total_usd_micros: None, final_git_commit_sha: None, final_patch: Some(final_patch.to_string()), + diff_summary: None, billing: None, }, ]) @@ -6964,6 +6967,7 @@ async fn archive_and_unarchive_updates_listing_visibility() { total_usd_micros: None, final_git_commit_sha: None, final_patch: None, + diff_summary: None, billing: None, }, ]) @@ -8304,6 +8308,7 @@ async fn boards_runs_excludes_archived_by_default() { total_usd_micros: None, final_git_commit_sha: None, final_patch: None, + diff_summary: None, billing: None, }, workflow_event::Event::RunArchived { actor: None }, @@ -8352,6 +8357,7 @@ async fn boards_runs_includes_archived_when_flag_set() { total_usd_micros: None, final_git_commit_sha: None, final_patch: None, + diff_summary: None, billing: None, }, workflow_event::Event::RunArchived { actor: None }, @@ -8371,6 +8377,7 @@ async fn boards_runs_includes_archived_when_flag_set() { total_usd_micros: None, final_git_commit_sha: None, final_patch: None, + diff_summary: None, billing: None, }, ]) @@ -8440,6 +8447,7 @@ async fn get_run_exposes_canonical_operator_statuses() { total_usd_micros: None, final_git_commit_sha: None, final_patch: None, + diff_summary: None, billing: None, }, ]) @@ -8521,6 +8529,7 @@ async fn boards_runs_maps_statuses_to_columns() { total_usd_micros: None, final_git_commit_sha: None, final_patch: None, + diff_summary: None, billing: None, }, ]) diff --git a/lib/crates/fabro-server/tests/it/api/run_files.rs b/lib/crates/fabro-server/tests/it/api/run_files.rs index 079544c3f..fb6d469a1 100644 --- a/lib/crates/fabro-server/tests/it/api/run_files.rs +++ b/lib/crates/fabro-server/tests/it/api/run_files.rs @@ -73,6 +73,7 @@ async fn append_completed_run_with_final_patch( total_usd_micros: None, final_git_commit_sha: Some("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb".to_string()), final_patch: Some(final_patch.to_string()), + diff_summary: None, billing: None, }, ) diff --git a/lib/crates/fabro-store/src/run_state.rs b/lib/crates/fabro-store/src/run_state.rs index 6476483ab..8b38ea485 100644 --- a/lib/crates/fabro-store/src/run_state.rs +++ b/lib/crates/fabro-store/src/run_state.rs @@ -155,6 +155,7 @@ impl RunProjectionReducer for RunProjection { self.pending_control = None; self.conclusion = Some(conclusion_from_completed(props, ts)?); self.final_patch.clone_from(&props.final_patch); + self.diff_summary = props.diff_summary.or(self.diff_summary); self.pending_interviews.clear(); } EventBody::RunFailed(props) => { @@ -167,6 +168,7 @@ impl RunProjectionReducer for RunProjection { self.pending_control = None; self.conclusion = Some(conclusion_from_failed(props, ts)); self.final_patch.clone_from(&props.final_patch); + self.diff_summary = props.diff_summary.or(self.diff_summary); self.pending_interviews.clear(); } EventBody::RunSupersededBy(props) => { @@ -196,6 +198,7 @@ impl RunProjectionReducer for RunProjection { } EventBody::CheckpointCompleted(props) => { let checkpoint = checkpoint_from_props(props, ts); + self.diff_summary = props.diff_summary.or(self.diff_summary); if let Some(node_id) = stored.node_id.as_deref() { let visit = checkpoint .node_visits @@ -562,6 +565,7 @@ pub(crate) fn build_summary(state: &RunProjection, run_id: &RunId) -> RunSummary .and_then(|conclusion| conclusion.billing.as_ref()) .and_then(|billing| billing.total_usd_micros), state.superseded_by, + state.diff_summary, ) } @@ -1416,6 +1420,7 @@ mod tests { restart_failure_signatures: BTreeMap::new(), node_visits: BTreeMap::from([("skip_me".to_string(), 1usize)]), diff: None, + diff_summary: None, }), None, )) @@ -1750,6 +1755,7 @@ mod tests { reason: FailureReason::WorkflowError, git_commit_sha: Some("abc123".to_string()), final_patch: Some(patch.to_string()), + diff_summary: None, }), None, )) @@ -1758,6 +1764,109 @@ mod tests { assert_eq!(state.final_patch.as_deref(), Some(patch)); } + #[test] + fn patch_bearing_events_roll_up_diff_summary_without_blanking_prior_value() { + let mut state = RunProjection::default(); + + state + .apply_event(&test_raw_event( + 1, + "checkpoint.completed", + &json!({ + "status": "running", + "current_node": "build", + "completed_nodes": ["build"], + "diff_summary": { + "files_changed": 2, + "additions": 10, + "deletions": 3 + } + }), + Some("build"), + )) + .unwrap(); + assert_eq!( + serde_json::to_value(build_summary(&state, &fixtures::RUN_1)).unwrap()["diff_summary"], + json!({ + "files_changed": 2, + "additions": 10, + "deletions": 3 + }) + ); + + state + .apply_event(&test_raw_event( + 2, + "checkpoint.completed", + &json!({ + "status": "running", + "current_node": "review", + "completed_nodes": ["build", "review"] + }), + Some("review"), + )) + .unwrap(); + assert_eq!( + serde_json::to_value(build_summary(&state, &fixtures::RUN_1)).unwrap()["diff_summary"] + ["files_changed"], + 2 + ); + + state + .apply_event(&test_raw_event( + 3, + "run.completed", + &json!({ + "duration_ms": 42, + "artifact_count": 0, + "status": "succeeded", + "reason": "completed", + "diff_summary": { + "files_changed": 4, + "additions": 18, + "deletions": 7 + } + }), + None, + )) + .unwrap(); + assert_eq!( + serde_json::to_value(build_summary(&state, &fixtures::RUN_1)).unwrap()["diff_summary"], + json!({ + "files_changed": 4, + "additions": 18, + "deletions": 7 + }) + ); + + let mut failed_state = RunProjection::default(); + failed_state + .apply_event(&test_raw_event( + 1, + "run.failed", + &json!({ + "error": "boom", + "duration_ms": 42, + "reason": "workflow_error", + "diff_summary": { + "files_changed": 5, + "additions": 20, + "deletions": 8 + } + }), + None, + )) + .unwrap(); + assert_eq!( + serde_json::to_value(build_summary(&failed_state, &fixtures::RUN_1)).unwrap()["diff_summary"], + json!({ + "files_changed": 5, + "additions": 20, + "deletions": 8 + }) + ); + } + #[test] fn run_failed_projection_renders_causes() { let mut state = RunProjection::default(); @@ -1774,6 +1883,7 @@ mod tests { reason: FailureReason::WorkflowError, git_commit_sha: None, final_patch: None, + diff_summary: None, }), None, )) @@ -1803,6 +1913,7 @@ mod tests { total_usd_micros: None, final_git_commit_sha: None, final_patch: None, + diff_summary: None, billing: None, }), None, @@ -1866,6 +1977,7 @@ mod tests { total_usd_micros: None, final_git_commit_sha: None, final_patch: None, + diff_summary: None, billing: None, }), None, @@ -1996,6 +2108,7 @@ mod tests { total_usd_micros: None, final_git_commit_sha: None, final_patch: None, + diff_summary: None, billing: None, }), None, diff --git a/lib/crates/fabro-types/src/diff.rs b/lib/crates/fabro-types/src/diff.rs index 201c26515..0f893973e 100644 --- a/lib/crates/fabro-types/src/diff.rs +++ b/lib/crates/fabro-types/src/diff.rs @@ -5,3 +5,10 @@ pub struct DiffStats { pub additions: i64, pub deletions: i64, } + +#[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +pub struct DiffSummary { + pub files_changed: i64, + pub additions: i64, + pub deletions: i64, +} diff --git a/lib/crates/fabro-types/src/event_envelope.rs b/lib/crates/fabro-types/src/event_envelope.rs index cac7dc13f..b36b176cd 100644 --- a/lib/crates/fabro-types/src/event_envelope.rs +++ b/lib/crates/fabro-types/src/event_envelope.rs @@ -42,6 +42,7 @@ mod tests { total_usd_micros: None, final_git_commit_sha: None, final_patch: None, + diff_summary: None, billing: None, }), }; @@ -85,6 +86,7 @@ mod tests { total_usd_micros: None, final_git_commit_sha: None, final_patch: None, + diff_summary: None, billing: None, }), }; diff --git a/lib/crates/fabro-types/src/lib.rs b/lib/crates/fabro-types/src/lib.rs index 7b1073bca..b3bc4715f 100644 --- a/lib/crates/fabro-types/src/lib.rs +++ b/lib/crates/fabro-types/src/lib.rs @@ -45,7 +45,7 @@ pub use checkpoint::Checkpoint; pub use command_output::{CommandOutputStream, CommandTermination}; pub use conclusion::{Conclusion, StageSummary}; pub use dense::{ServerSettings, UserSettings, WorkflowSettings}; -pub use diff::DiffStats; +pub use diff::{DiffStats, DiffSummary}; pub use event_envelope::EventEnvelope; pub use failure_signature::FailureSignature; pub use graph::{ diff --git a/lib/crates/fabro-types/src/run_event/mod.rs b/lib/crates/fabro-types/src/run_event/mod.rs index 6ac1f331d..339c8c22d 100644 --- a/lib/crates/fabro-types/src/run_event/mod.rs +++ b/lib/crates/fabro-types/src/run_event/mod.rs @@ -981,6 +981,74 @@ mod tests { )); } + #[test] + fn patch_bearing_events_round_trip_diff_summary() { + for (event_name, properties) in [ + ( + "checkpoint.completed", + json!({ + "status": "running", + "current_node": "build", + "completed_nodes": ["build"], + "diff_summary": { + "files_changed": 2, + "additions": 10, + "deletions": 3 + } + }), + ), + ( + "run.completed", + json!({ + "duration_ms": 42, + "artifact_count": 0, + "status": "succeeded", + "reason": "completed", + "diff_summary": { + "files_changed": 2, + "additions": 10, + "deletions": 3 + } + }), + ), + ( + "run.failed", + json!({ + "error": "boom", + "duration_ms": 42, + "reason": "workflow_error", + "diff_summary": { + "files_changed": 2, + "additions": 10, + "deletions": 3 + } + }), + ), + ] { + let line = json!({ + "id": format!("evt_{event_name}"), + "ts": "2026-04-04T12:00:00Z", + "run_id": fixtures::RUN_1, + "event": event_name, + "node_id": "build", + "properties": properties + }); + + let parsed = RunEvent::from_value(line).unwrap(); + let serialized = parsed.to_value().unwrap(); + + assert_eq!( + serialized["properties"]["diff_summary"], + json!({ + "files_changed": 2, + "additions": 10, + "deletions": 3 + }), + "{event_name} should preserve diff_summary" + ); + } + } + #[test] fn run_submitted_round_trip_preserves_definition_blob() { let line = json!({ diff --git a/lib/crates/fabro-types/src/run_event/run.rs b/lib/crates/fabro-types/src/run_event/run.rs index 176260ac6..aeef18879 100644 --- a/lib/crates/fabro-types/src/run_event/run.rs +++ b/lib/crates/fabro-types/src/run_event/run.rs @@ -5,8 +5,8 @@ use serde::{Deserialize, Serialize}; use super::{BilledTokenCounts, ExecOutputTail, RunNoticeLevel}; use crate::status::{BlockedReason, FailureReason, SuccessReason}; use crate::{ - ForkSourceRef, GitContext, Graph, RunBlobId, RunControlAction, RunId, RunProvenance, - WorkflowSettings, + DiffSummary, ForkSourceRef, GitContext, Graph, RunBlobId, RunControlAction, RunId, + RunProvenance, WorkflowSettings, }; #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] @@ -139,6 +139,8 @@ pub struct RunCompletedProps { #[serde(default, skip_serializing_if = "Option::is_none")] pub final_patch: Option, #[serde(default, skip_serializing_if = "Option::is_none")] + pub diff_summary: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] pub billing: Option, } @@ -155,6 +157,8 @@ pub struct RunFailedProps { // pre-change events replay with `final_patch: None` via serde default. #[serde(default, skip_serializing_if = "Option::is_none")] pub final_patch: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub diff_summary: Option, } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] diff --git a/lib/crates/fabro-types/src/run_event/stage.rs b/lib/crates/fabro-types/src/run_event/stage.rs index 1b6609884..188441d6c 100644 --- a/lib/crates/fabro-types/src/run_event/stage.rs +++ b/lib/crates/fabro-types/src/run_event/stage.rs @@ -4,7 +4,7 @@ use serde::{Deserialize, Serialize}; use serde_json::Value; use super::ExecOutputTail; -use crate::{BilledModelUsage, FailureDetail, Outcome, StageOutcome}; +use crate::{BilledModelUsage, DiffSummary, FailureDetail, Outcome, StageOutcome}; #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct StageStartedProps { @@ -114,6 +114,8 @@ pub struct CheckpointCompletedProps { pub node_visits: BTreeMap, #[serde(default, skip_serializing_if = "Option::is_none")] pub diff: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub diff_summary: Option, } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] diff --git a/lib/crates/fabro-types/src/run_projection.rs b/lib/crates/fabro-types/src/run_projection.rs index 29842afdb..9d7bdc105 100644 --- a/lib/crates/fabro-types/src/run_projection.rs +++ b/lib/crates/fabro-types/src/run_projection.rs @@ -4,9 +4,9 @@ use std::num::NonZeroU32; use chrono::{DateTime, Utc}; use crate::{ - BilledModelUsage, Checkpoint, Conclusion, InterviewQuestionRecord, InvalidTransition, - PullRequestRecord, Retro, RunControlAction, RunId, RunSpec, RunStatus, SandboxRecord, - StageCompletion, StageId, StageState, StartRecord, + BilledModelUsage, Checkpoint, Conclusion, DiffSummary, InterviewQuestionRecord, + InvalidTransition, PullRequestRecord, Retro, RunControlAction, RunId, RunSpec, RunStatus, + SandboxRecord, StageCompletion, StageId, StageState, StartRecord, }; #[derive(Debug, Clone, Default, serde::Serialize, serde::Deserialize)] @@ -27,6 +27,8 @@ pub struct RunProjection { pub retro_response: Option, pub sandbox: Option, pub final_patch: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub diff_summary: Option, pub pull_request: Option, pub superseded_by: Option, pub pending_interviews: BTreeMap, diff --git a/lib/crates/fabro-types/src/run_summary.rs b/lib/crates/fabro-types/src/run_summary.rs index 7ce93769c..b756cef4e 100644 --- a/lib/crates/fabro-types/src/run_summary.rs +++ b/lib/crates/fabro-types/src/run_summary.rs @@ -4,7 +4,7 @@ use chrono::{DateTime, Utc}; use fabro_util::text::strip_goal_decoration; use serde::{Deserialize, Serialize}; -use crate::{RepositoryReference, RunControlAction, RunId, RunStatus}; +use crate::{DiffSummary, RepositoryReference, RunControlAction, RunId, RunStatus}; #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct RunSummary { @@ -39,6 +39,8 @@ pub struct RunSummary { pub total_usd_micros: Option, #[serde(default)] pub superseded_by: Option, + #[serde(default)] + pub diff_summary: Option, } impl RunSummary { @@ -62,6 +64,7 @@ impl RunSummary { duration_ms: Option, total_usd_micros: Option, superseded_by: Option, + diff_summary: Option, ) -> Self { let title = truncate_goal(&goal); let repository = RepositoryReference { @@ -90,6 +93,7 @@ impl RunSummary { elapsed_secs, total_usd_micros, superseded_by, + diff_summary, } } } @@ -161,6 +165,7 @@ mod tests { use std::collections::HashMap; use chrono::{TimeZone, Utc}; + use serde_json::json; use super::RunSummary; use crate::{BlockedReason, RepositoryReference, RunControlAction, RunStatus, fixtures}; @@ -185,6 +190,7 @@ mod tests { Some(42), Some(123), Some(fixtures::RUN_2), + None, ); assert_eq!(summary.title, "ship it"); @@ -214,6 +220,35 @@ mod tests { assert_eq!(parsed, summary); } + #[test] + fn summary_round_trips_diff_summary() { + let summary: RunSummary = serde_json::from_value(json!({ + "run_id": fixtures::RUN_1, + "goal": "ship it", + "title": "ship it", + "labels": {}, + "status": { "kind": "running" }, + "repository": { "name": "fabro" }, + "created_at": fixtures::RUN_1.created_at(), + "diff_summary": { + "files_changed": 3, + "additions": 12, + "deletions": 4 + } + })) + .unwrap(); + + let value = serde_json::to_value(&summary).unwrap(); + assert_eq!( + value["diff_summary"], + json!({ + "files_changed": 3, + "additions": 12, + "deletions": 4 + }) + ); + } + #[test] fn summary_falls_back_to_source_directory_then_unknown() { let source_only = RunSummary::new( @@ -232,6 +267,7 @@ mod tests { None, None, None, + None, ); assert_eq!(source_only.repository.name, "local-checkout"); assert_eq!(source_only.last_event_at, None); @@ -252,6 +288,7 @@ mod tests { None, None, None, + None, ); assert_eq!(unknown.repository.name, "unknown"); } diff --git a/lib/crates/fabro-workflow/src/event/convert.rs b/lib/crates/fabro-workflow/src/event/convert.rs index e5a287443..a6deda2c5 100644 --- a/lib/crates/fabro-workflow/src/event/convert.rs +++ b/lib/crates/fabro-workflow/src/event/convert.rs @@ -159,6 +159,7 @@ fn event_body_from_event(event: &Event) -> EventBody { total_usd_micros, final_git_commit_sha, final_patch, + diff_summary, billing, } => EventBody::RunCompleted(fabro_types::RunCompletedProps { duration_ms: *duration_ms, @@ -168,6 +169,7 @@ fn event_body_from_event(event: &Event) -> EventBody { total_usd_micros: *total_usd_micros, final_git_commit_sha: final_git_commit_sha.clone(), final_patch: final_patch.clone(), + diff_summary: *diff_summary, billing: billing.clone(), }), Event::WorkflowRunFailed { @@ -176,6 +178,7 @@ fn event_body_from_event(event: &Event) -> EventBody { reason, git_commit_sha, final_patch, + diff_summary, } => EventBody::RunFailed(fabro_types::RunFailedProps { error: error.to_string(), causes: error.causes(), @@ -183,6 +186,7 @@ fn event_body_from_event(event: &Event) -> EventBody { reason: *reason, git_commit_sha: git_commit_sha.clone(), final_patch: final_patch.clone(), + diff_summary: *diff_summary, }), Event::RunNotice { level, @@ -428,6 +432,7 @@ fn event_body_from_event(event: &Event) -> EventBody { restart_failure_signatures, node_visits, diff, + diff_summary, .. } => EventBody::CheckpointCompleted(fabro_types::CheckpointCompletedProps { status: status.clone(), @@ -442,6 +447,7 @@ fn event_body_from_event(event: &Event) -> EventBody { restart_failure_signatures: restart_failure_signatures.clone(), node_visits: node_visits.clone(), diff: diff.clone(), + diff_summary: *diff_summary, }), Event::CheckpointFailed { error, @@ -1458,6 +1464,7 @@ mod tests { reason: FailureReason::WorkflowError, git_commit_sha: Some("abc123".to_string()), final_patch: None, + diff_summary: None, }); assert_eq!(stored.event_name(), "run.failed"); @@ -1475,6 +1482,7 @@ mod tests { reason: FailureReason::WorkflowError, git_commit_sha: None, final_patch: None, + diff_summary: None, }); let properties = stored.properties().unwrap(); diff --git a/lib/crates/fabro-workflow/src/event/events.rs b/lib/crates/fabro-workflow/src/event/events.rs index f752b7925..5db0ae1a2 100644 --- a/lib/crates/fabro-workflow/src/event/events.rs +++ b/lib/crates/fabro-workflow/src/event/events.rs @@ -1,9 +1,9 @@ use std::collections::BTreeMap; use ::fabro_types::{ - BilledTokenCounts, BlockedReason, CommandTermination, FailureReason, ForkSourceRef, GitContext, - ParallelBranchId, Principal, PullRequestRecord, RunBlobId, RunId, RunNoticeLevel, - RunProvenance, StageId, SuccessReason, run_event as fabro_types, + BilledTokenCounts, BlockedReason, CommandTermination, DiffSummary, FailureReason, + ForkSourceRef, GitContext, ParallelBranchId, Principal, PullRequestRecord, RunBlobId, RunId, + RunNoticeLevel, RunProvenance, StageId, SuccessReason, run_event as fabro_types, }; use fabro_agent::{AgentEvent, SandboxEvent}; use serde::{Deserialize, Serialize}; @@ -123,6 +123,8 @@ pub enum Event { #[serde(default, skip_serializing_if = "Option::is_none")] final_patch: Option, #[serde(default, skip_serializing_if = "Option::is_none")] + diff_summary: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] billing: Option, }, WorkflowRunFailed { @@ -133,6 +135,8 @@ pub enum Event { git_commit_sha: Option, #[serde(default, skip_serializing_if = "Option::is_none")] final_patch: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + diff_summary: Option, }, RunNotice { level: RunNoticeLevel, @@ -321,6 +325,8 @@ pub enum Event { node_visits: BTreeMap, #[serde(default, skip_serializing_if = "Option::is_none")] diff: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + diff_summary: Option, }, CheckpointFailed { node_id: String, diff --git a/lib/crates/fabro-workflow/src/git.rs b/lib/crates/fabro-workflow/src/git.rs index 70c438479..b574ad662 100644 --- a/lib/crates/fabro-workflow/src/git.rs +++ b/lib/crates/fabro-workflow/src/git.rs @@ -545,6 +545,7 @@ mod tests { restart_failure_signatures: std::collections::BTreeMap::new(), node_visits: std::collections::BTreeMap::from([("work".into(), 2)]), diff: Some("diff --git a/story.txt b/story.txt".into()), + diff_summary: None, }) .await .unwrap(); diff --git a/lib/crates/fabro-workflow/src/lifecycle/event.rs b/lib/crates/fabro-workflow/src/lifecycle/event.rs index 82dbd0501..fd04a186f 100644 --- a/lib/crates/fabro-workflow/src/lifecycle/event.rs +++ b/lib/crates/fabro-workflow/src/lifecycle/event.rs @@ -369,6 +369,7 @@ impl RunLifecycle for EventLifecycle { let git_sha = git_result.as_ref().and_then(|r| r.commit_sha.clone()); let diff = git_result.as_ref().and_then(|r| r.diff.clone()); + let diff_summary = git_result.as_ref().and_then(|r| r.diff_summary); let (loop_failure_signatures, restart_failure_signatures) = snapshot_failure_signatures(&self.circuit_breaker); let context_values = artifact::durable_context_snapshot(&state.context); @@ -400,6 +401,7 @@ impl RunLifecycle for EventLifecycle { .into_iter() .collect::>(), diff, + diff_summary, }, &scope, ); diff --git a/lib/crates/fabro-workflow/src/lifecycle/git.rs b/lib/crates/fabro-workflow/src/lifecycle/git.rs index 840894756..0c74c5f27 100644 --- a/lib/crates/fabro-workflow/src/lifecycle/git.rs +++ b/lib/crates/fabro-workflow/src/lifecycle/git.rs @@ -8,8 +8,8 @@ use fabro_core::lifecycle::RunLifecycle; use fabro_core::outcome::NodeResult; use fabro_core::state::ExecutionState; use fabro_dump::RunDump; -use fabro_types::RunId; use fabro_types::run_event::{MetadataSnapshotFailureKind, MetadataSnapshotPhase}; +use fabro_types::{DiffSummary, RunId}; use fabro_util::error::collect_causes; use fabro_util::time::elapsed_ms; @@ -21,7 +21,9 @@ use crate::outcome::BilledModelUsage; use crate::run_metadata::{MetadataSnapshot, RunMetadataRuntime, RunMetadataWriterHandle}; use crate::run_options::RunOptions; use crate::runtime_store::RunStoreHandle; -use crate::sandbox_git::{checked_git_checkpoint, git_diff}; +use crate::sandbox_git::{ + checked_git_checkpoint, git_diff, list_diff_numstat, summarize_diff_numstat, +}; use crate::sandbox_git_runtime::SandboxGitRuntime; type WfRunState = ExecutionState>; @@ -61,6 +63,7 @@ pub(crate) struct GitCheckpointResult { pub commit_sha: Option, pub push_results: Vec, pub diff: Option, + pub diff_summary: Option, } #[derive(Debug, Clone)] @@ -279,6 +282,7 @@ impl RunLifecycle for GitLifecycle { commit_sha: Some(sha.clone()), push_results: Vec::new(), diff: None, + diff_summary: None, }; // Push run branch (skip in dry-run mode) @@ -326,7 +330,21 @@ impl RunLifecycle for GitLifecycle { .and_then(|g| g.base_sha.clone()) }); if let Some(prev) = prev.filter(|p| p != &sha) { - match git_diff(&*self.sandbox, &prev).await { + let summary_base = self + .run_options + .git + .as_ref() + .and_then(|git| git.base_sha.clone()); + let (patch_result, numstat_result) = + tokio::join!(git_diff(&*self.sandbox, &prev), async { + match summary_base.as_deref() { + Some(base) if base != sha => { + Some(list_diff_numstat(&*self.sandbox, base, &sha).await) + } + _ => None, + } + },); + match patch_result { Ok(patch) if !patch.is_empty() => { git_result.diff = Some(patch); } @@ -342,6 +360,22 @@ impl RunLifecycle for GitLifecycle { ); } } + match numstat_result { + Some(Ok(numstat)) => { + git_result.diff_summary = Some(summarize_diff_numstat(&numstat)); + } + Some(Err(err)) => { + let exec_output_tail = + fabro_sandbox::default_redacted_output_tail(&err); + self.emitter.notice_with_tail( + RunNoticeLevel::Warn, + RunNoticeCode::GitDiffFailed, + format!("[node: {node_id}] git diff stats failed: {err}"), + exec_output_tail, + ); + } + None => {} + } } // Update shared state @@ -586,6 +620,39 @@ mod tests { assert!(commit.status.success()); } + #[expect( + clippy::disallowed_methods, + reason = "metadata event tests use synchronous git commands to set up temporary repositories" + )] + fn git_commit_all(repo: &Path, msg: &str) -> String { + let add = std::process::Command::new("git") + .args(["add", "."]) + .current_dir(repo) + .output() + .unwrap(); + assert!(add.status.success()); + let commit = std::process::Command::new("git") + .args(["commit", "-m", msg]) + .current_dir(repo) + .output() + .unwrap(); + assert!( + commit.status.success(), + "git commit failed: {}", + String::from_utf8_lossy(&commit.stderr) + ); + let rev_parse = std::process::Command::new("git") + .args(["rev-parse", "HEAD"]) + .current_dir(repo) + .output() + .unwrap(); + assert!(rev_parse.status.success()); + String::from_utf8(rev_parse.stdout) + .unwrap() + .trim() + .to_string() + } + fn workflow_graph() -> WorkflowGraph { let mut graph = Graph::new("metadata"); let mut start = Node::new("start"); @@ -954,6 +1021,76 @@ mod tests { ); } + #[tokio::test] + async fn checkpoint_git_result_includes_diff_summary() { + let repo_dir = tempfile::tempdir().unwrap(); + let repo = repo_dir.path(); + init_git_repo(repo); + tokio::fs::write(repo.join("notes.txt"), "one\n") + .await + .unwrap(); + let base = git_commit_all(repo, "base"); + tokio::fs::write(repo.join("notes.txt"), "one\ntwo\n") + .await + .unwrap(); + + let mut options = run_options(repo, "fabro/metadata/run").as_ref().clone(); + options.git = Some(GitCheckpointOptions { + base_sha: Some(base), + run_branch: None, + meta_branch: None, + }); + let lifecycle = git_lifecycle_with_writer( + repo, + Arc::new(Emitter::new(fixtures::RUN_1)), + RunStoreHandle::local(run_store(fixtures::RUN_1).await), + Arc::new(options), + Arc::new(RunMetadataRuntime::new()), + None, + ); + let graph = workflow_graph(); + let node = graph.get_node("build").unwrap(); + let mut state = ExecutionState::new(&graph).unwrap(); + state.increment_visits("build"); + let result = WfNodeResult::new(Outcome::success(), Duration::from_millis(10), 1, 1); + + lifecycle + .on_checkpoint(&node, &result, Some("exit"), &state) + .await + .unwrap(); + + let git_result = lifecycle + .checkpoint_git_result + .lock() + .unwrap() + .clone() + .unwrap(); + let diff_summary = git_result.diff_summary.expect("diff summary"); + assert_eq!(diff_summary.files_changed, 1); + assert_eq!(diff_summary.additions, 1); + assert_eq!(diff_summary.deletions, 0); + + tokio::fs::write(repo.join("notes.txt"), "one\ntwo\nthree\n") + .await + .unwrap(); + state.increment_visits("build"); + lifecycle + .on_checkpoint(&node, &result, Some("exit"), &state) + .await + .unwrap(); + + let git_result = lifecycle + .checkpoint_git_result + .lock() + .unwrap() + .clone() + .unwrap(); + let diff_summary = git_result.diff_summary.expect("diff summary"); + assert_eq!(diff_summary.files_changed, 1); + assert_eq!(diff_summary.additions, 2); + assert_eq!(diff_summary.deletions, 0); + } + #[tokio::test] async fn degraded_metadata_runtime_skips_snapshot_events() { let repo_dir = tempfile::tempdir().unwrap(); diff --git a/lib/crates/fabro-workflow/src/operations/archive.rs b/lib/crates/fabro-workflow/src/operations/archive.rs index 87f2e5b50..9d1065b91 100644 --- a/lib/crates/fabro-workflow/src/operations/archive.rs +++ b/lib/crates/fabro-workflow/src/operations/archive.rs @@ -159,6 +159,7 @@ mod tests { total_usd_micros: None, final_git_commit_sha: None, final_patch: None, + diff_summary: None, billing: None, }) .await @@ -173,6 +174,7 @@ mod tests { reason: FailureReason::WorkflowError, git_commit_sha: None, final_patch: None, + diff_summary: None, }) .await .unwrap(); diff --git a/lib/crates/fabro-workflow/src/operations/fork.rs b/lib/crates/fabro-workflow/src/operations/fork.rs index 66f6baa67..58759b0aa 100644 --- a/lib/crates/fabro-workflow/src/operations/fork.rs +++ b/lib/crates/fabro-workflow/src/operations/fork.rs @@ -289,6 +289,7 @@ fn checkpoint_completed_event(checkpoint: &Checkpoint) -> Event { .collect(), node_visits: checkpoint.node_visits.clone().into_iter().collect(), diff: None, + diff_summary: None, } } @@ -412,6 +413,7 @@ mod tests { restart_failure_signatures: BTreeMap::new(), node_visits, diff: None, + diff_summary: None, }) .await .unwrap(); diff --git a/lib/crates/fabro-workflow/src/operations/start.rs b/lib/crates/fabro-workflow/src/operations/start.rs index cedce5386..e6b3337b4 100644 --- a/lib/crates/fabro-workflow/src/operations/start.rs +++ b/lib/crates/fabro-workflow/src/operations/start.rs @@ -274,6 +274,7 @@ async fn persist_terminal_engine_failure( reason, git_commit_sha: None, final_patch: None, + diff_summary: None, }) .await { @@ -899,6 +900,7 @@ impl Drop for DetachedRunBootstrapGuard { reason, git_commit_sha: None, final_patch: None, + diff_summary: None, }) .await; }); @@ -964,6 +966,7 @@ impl Drop for DetachedRunCompletionGuard { reason, git_commit_sha: None, final_patch: None, + diff_summary: None, }) .await; let _ = append_event_to_sink(&event_sink, &run_id, &Event::RunNotice { @@ -994,6 +997,7 @@ async fn persist_detached_failure( reason, git_commit_sha: None, final_patch: None, + diff_summary: None, }) .await { @@ -1177,6 +1181,7 @@ mod tests { restart_failure_signatures: HashMap::new().into_iter().collect(), node_visits: HashMap::new().into_iter().collect(), diff: None, + diff_summary: None, }); } }); @@ -1389,6 +1394,7 @@ mod tests { .collect(), node_visits: checkpoint.node_visits.clone().into_iter().collect(), diff: None, + diff_summary: None, }, ) .await @@ -1479,6 +1485,7 @@ mod tests { .collect(), node_visits: checkpoint.node_visits.clone().into_iter().collect(), diff: None, + diff_summary: None, }) .await .unwrap(); @@ -1496,6 +1503,7 @@ mod tests { total_usd_micros: None, final_git_commit_sha: None, final_patch: None, + diff_summary: None, billing: None, }) .await diff --git a/lib/crates/fabro-workflow/src/pipeline/finalize.rs b/lib/crates/fabro-workflow/src/pipeline/finalize.rs index a1cc84d19..3189ee21f 100644 --- a/lib/crates/fabro-workflow/src/pipeline/finalize.rs +++ b/lib/crates/fabro-workflow/src/pipeline/finalize.rs @@ -4,7 +4,7 @@ use std::time::Instant; use fabro_dump::RunDump; use fabro_hooks::{HookContext, HookEvent}; use fabro_types::run_event::{MetadataSnapshotFailureKind, MetadataSnapshotPhase}; -use fabro_types::{BilledTokenCounts, EventBody, RunProjection}; +use fabro_types::{BilledTokenCounts, DiffSummary, EventBody, RunProjection}; use fabro_util::error::collect_causes; use fabro_util::time::elapsed_ms; @@ -17,7 +17,7 @@ use crate::run_metadata::MetadataSnapshot; use crate::run_options::RunOptions; use crate::run_status::{FailureReason, RunStatus, SuccessReason}; use crate::runtime_store::RunStoreHandle; -use crate::sandbox_git::git_diff_with_timeout; +use crate::sandbox_git::{git_diff_with_timeout, list_diff_numstat, summarize_diff_numstat}; use crate::services::RunServices; use crate::{ProjectionBillingRollup, billing_rollup_from_projection}; @@ -391,13 +391,20 @@ async fn compute_final_patch( run_options: &RunOptions, services: &RunServices, status: StageOutcome, -) -> Option { - let base_sha = run_options.git.as_ref().and_then(|g| g.base_sha.clone())?; +) -> (Option, Option) { + let Some(base_sha) = run_options.git.as_ref().and_then(|g| g.base_sha.clone()) else { + return (None, None); + }; let timeout_ms = match status { StageOutcome::Succeeded | StageOutcome::PartiallySucceeded => 30_000, _ => 10_000, }; - match git_diff_with_timeout(&*services.sandbox, &base_sha, timeout_ms).await { + let to_sha = "HEAD"; + let (patch_result, numstat_result) = tokio::join!( + git_diff_with_timeout(&*services.sandbox, &base_sha, timeout_ms), + list_diff_numstat(&*services.sandbox, &base_sha, to_sha), + ); + let final_patch = match patch_result { Ok(patch) if !patch.is_empty() => Some(patch), Ok(_) => None, Err(err) => { @@ -408,7 +415,19 @@ async fn compute_final_patch( ); None } - } + }; + let diff_summary = match numstat_result { + Ok(numstat) => Some(summarize_diff_numstat(&numstat)), + Err(err) => { + services.emitter.notice( + RunNoticeLevel::Warn, + RunNoticeCode::GitDiffFailed, + format!("final diff stats failed: {err}"), + ); + None + } + }; + (final_patch, diff_summary) } pub(crate) fn billing_from_projection(projection: &RunProjection) -> Option { @@ -421,6 +440,7 @@ pub(crate) fn build_terminal_event( artifact_count: usize, final_git_commit_sha: Option, final_patch: Option, + diff_summary: Option, billing: Option, ) -> Event { if matches!(outcome, Err(Error::Cancelled)) { @@ -430,6 +450,7 @@ pub(crate) fn build_terminal_event( reason: FailureReason::Cancelled, git_commit_sha: final_git_commit_sha, final_patch, + diff_summary, }; } @@ -455,6 +476,7 @@ pub(crate) fn build_terminal_event( total_usd_micros, final_git_commit_sha, final_patch, + diff_summary, billing, }; } @@ -473,6 +495,7 @@ pub(crate) fn build_terminal_event( reason: FailureReason::WorkflowError, git_commit_sha: final_git_commit_sha, final_patch, + diff_summary, } } @@ -542,7 +565,7 @@ pub async fn finalize(retroed: Retroed, options: &FinalizeOptions) -> Result Result String { + let add = std::process::Command::new("git") + .args(["add", "."]) + .current_dir(repo) + .output() + .unwrap(); + assert!(add.status.success()); + let commit = std::process::Command::new("git") + .args(["commit", "-m", msg]) + .current_dir(repo) + .output() + .unwrap(); + assert!( + commit.status.success(), + "git commit failed: {}", + String::from_utf8_lossy(&commit.stderr) + ); + let rev_parse = std::process::Command::new("git") + .args(["rev-parse", "HEAD"]) + .current_dir(repo) + .output() + .unwrap(); + assert!(rev_parse.status.success()); + String::from_utf8(rev_parse.stdout) + .unwrap() + .trim() + .to_string() + } + fn record_events(emitter: &Arc) -> Arc>> { let events = Arc::new(std::sync::Mutex::new(Vec::new())); let captured = Arc::clone(&events); @@ -1160,6 +1217,71 @@ mod tests { ]); } + #[tokio::test] + async fn finalize_terminal_event_includes_diff_summary() { + let repo_dir = tempfile::tempdir().unwrap(); + let repo = repo_dir.path(); + init_git_repo(repo); + tokio::fs::write(repo.join("notes.txt"), "one\n") + .await + .unwrap(); + let base = git_commit_all(repo, "base"); + tokio::fs::write(repo.join("notes.txt"), "one\ntwo\nthree\n") + .await + .unwrap(); + let head = git_commit_all(repo, "head"); + + let run_store = seeded_run_store().await; + let emitter = Arc::new(Emitter::new(test_run_id())); + let events = record_events(&emitter); + let services = test_services( + RunStoreHandle::local(run_store), + Arc::clone(&emitter), + Arc::new(fabro_agent::LocalSandbox::new(repo.to_path_buf())), + Arc::new(RunMetadataRuntime::new()), + None, + ); + let mut run_options = test_git_run_options(repo, "fabro/metadata/run"); + run_options.git = Some(GitCheckpointOptions { + base_sha: Some(base), + run_branch: None, + meta_branch: None, + }); + let retroed = Retroed { + graph: Graph::new("test"), + outcome: Ok(Outcome::success()), + run_options, + duration_ms: 5, + services, + retro: None, + }; + + finalize(retroed, &FinalizeOptions { + run_dir: repo.to_path_buf(), + run_id: test_run_id(), + workflow_name: "test".to_string(), + preserve_sandbox: true, + last_git_sha: Some(head), + }) + .await + .unwrap(); + + let events = events.lock().unwrap(); + let run_completed = events + .iter() + .find(|event| event.event_name() == "run.completed") + .expect("run.completed event"); + let properties = run_completed.properties().unwrap(); + assert_eq!( + properties["diff_summary"], + serde_json::json!({ + "files_changed": 1, + "additions": 2, + "deletions": 0 + }) + ); + } + struct FailingStateStore; #[async_trait] diff --git a/lib/crates/fabro-workflow/src/pipeline/pull_request.rs b/lib/crates/fabro-workflow/src/pipeline/pull_request.rs index 03edcbc0e..cfef50473 100644 --- a/lib/crates/fabro-workflow/src/pipeline/pull_request.rs +++ b/lib/crates/fabro-workflow/src/pipeline/pull_request.rs @@ -1709,6 +1709,7 @@ mod tests { final_patch: Some( "diff --git a/src/lib.rs b/src/lib.rs\n+fn from_store() {}\n".to_string(), ), + diff_summary: None, billing: None, }) .await @@ -1997,6 +1998,7 @@ mod tests { final_patch: Some( "diff --git a/src/lib.rs b/src/lib.rs\n+fn from_store() {}\n".to_string(), ), + diff_summary: None, billing: None, }) .await diff --git a/lib/crates/fabro-workflow/src/pipeline/retro.rs b/lib/crates/fabro-workflow/src/pipeline/retro.rs index 2749cf023..224ab5043 100644 --- a/lib/crates/fabro-workflow/src/pipeline/retro.rs +++ b/lib/crates/fabro-workflow/src/pipeline/retro.rs @@ -294,6 +294,7 @@ mod tests { .collect(), node_visits: checkpoint.node_visits.clone().into_iter().collect(), diff: None, + diff_summary: None, }) .await .unwrap(); diff --git a/lib/crates/fabro-workflow/src/sandbox_git.rs b/lib/crates/fabro-workflow/src/sandbox_git.rs index c370a2d89..817c0fa53 100644 --- a/lib/crates/fabro-workflow/src/sandbox_git.rs +++ b/lib/crates/fabro-workflow/src/sandbox_git.rs @@ -564,7 +564,7 @@ fn classify_entry( }) } -pub use fabro_types::DiffStats; +pub use fabro_types::{DiffStats, DiffSummary}; /// Output of `git diff --numstat`: which paths are binary, plus per-path /// `+/-` line totals for text files in the range. Both pieces come from a @@ -577,6 +577,27 @@ pub struct DiffNumstat { pub line_stats_by_path: HashMap, } +pub fn summarize_diff_numstat(numstat: &DiffNumstat) -> DiffSummary { + let text_files = i64::try_from(numstat.line_stats_by_path.len()).unwrap_or(i64::MAX); + let binary_files = i64::try_from(numstat.binary_paths.len()).unwrap_or(i64::MAX); + let (additions, deletions) = + numstat + .line_stats_by_path + .values() + .fold((0_i64, 0_i64), |(adds, dels), stats| { + ( + adds.saturating_add(stats.additions), + dels.saturating_add(stats.deletions), + ) + }); + + DiffSummary { + files_changed: text_files.saturating_add(binary_files), + additions, + deletions, + } +} + /// Run `git diff --numstat` once and return both the set of binary paths and /// text-file `+/-` totals. The single call replaces the previous binary-only /// helper. diff --git a/lib/crates/fabro-workflow/src/test_support.rs b/lib/crates/fabro-workflow/src/test_support.rs index c3e1fe517..487e395e9 100644 --- a/lib/crates/fabro-workflow/src/test_support.rs +++ b/lib/crates/fabro-workflow/src/test_support.rs @@ -43,6 +43,7 @@ async fn execute_and_emit_terminal(initialized: InitializedState) -> Executed { 0, None, None, + None, billing, ); executed.engine.run.emitter.emit(&event); diff --git a/lib/packages/fabro-api-client/src/.openapi-generator/FILES b/lib/packages/fabro-api-client/src/.openapi-generator/FILES index 0f91dd09c..564c81ccc 100644 --- a/lib/packages/fabro-api-client/src/.openapi-generator/FILES +++ b/lib/packages/fabro-api-client/src/.openapi-generator/FILES @@ -65,6 +65,7 @@ models/diagnostics-report.ts models/diagnostics-section.ts models/diff-file.ts models/diff-stats.ts +models/diff-summary.ts models/dirty-status.ts models/discord-integration-settings.ts models/disk-usage-response.ts diff --git a/lib/packages/fabro-api-client/src/models/diff-summary.ts b/lib/packages/fabro-api-client/src/models/diff-summary.ts new file mode 100644 index 000000000..f5dbcea5e --- /dev/null +++ b/lib/packages/fabro-api-client/src/models/diff-summary.ts @@ -0,0 +1,33 @@ +/* tslint:disable */ +/* eslint-disable */ +/** + * Fabro Run API + * HTTP API for managing Fabro workflow run executions. + * + * The version of the OpenAPI document: 0.1.0 + * + * + * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). + * https://openapi-generator.tech + * Do not edit the class manually. + */ + + + +/** + * Cheap aggregate file and line counts for a run diff. + */ +export interface DiffSummary { + /** + * Total number of changed files, including binary files. + */ + 'files_changed': number; + /** + * Total lines added across text files. + */ + 'additions': number; + /** + * Total lines deleted across text files. + */ + 'deletions': number; +} diff --git a/lib/packages/fabro-api-client/src/models/index.ts b/lib/packages/fabro-api-client/src/models/index.ts index 34479400e..361a4f7ec 100644 --- a/lib/packages/fabro-api-client/src/models/index.ts +++ b/lib/packages/fabro-api-client/src/models/index.ts @@ -45,6 +45,7 @@ export * from './diagnostics-report'; export * from './diagnostics-section'; export * from './diff-file'; export * from './diff-stats'; +export * from './diff-summary'; export * from './dirty-status'; export * from './discord-integration-settings'; export * from './disk-usage-response'; diff --git a/lib/packages/fabro-api-client/src/models/run-projection.ts b/lib/packages/fabro-api-client/src/models/run-projection.ts index 6deb882ea..4bd3b6512 100644 --- a/lib/packages/fabro-api-client/src/models/run-projection.ts +++ b/lib/packages/fabro-api-client/src/models/run-projection.ts @@ -13,6 +13,9 @@ */ +// May contain unused imports in some cases +// @ts-ignore +import type { DiffSummary } from './diff-summary'; // May contain unused imports in some cases // @ts-ignore import type { PendingInterviewRecord } from './pending-interview-record'; @@ -57,6 +60,7 @@ export interface RunProjection { 'retro_response'?: string | null; 'sandbox'?: { [key: string]: any; } | null; 'final_patch'?: string | null; + 'diff_summary'?: DiffSummary | null; 'pull_request'?: { [key: string]: any; } | null; 'superseded_by'?: string | null; 'pending_interviews'?: { [key: string]: PendingInterviewRecord; }; diff --git a/lib/packages/fabro-api-client/src/models/run-summary.ts b/lib/packages/fabro-api-client/src/models/run-summary.ts index 290190f83..6e387ac06 100644 --- a/lib/packages/fabro-api-client/src/models/run-summary.ts +++ b/lib/packages/fabro-api-client/src/models/run-summary.ts @@ -13,6 +13,9 @@ */ +// May contain unused imports in some cases +// @ts-ignore +import type { DiffSummary } from './diff-summary'; // May contain unused imports in some cases // @ts-ignore import type { RepositoryReference } from './repository-reference'; @@ -46,6 +49,7 @@ export interface RunSummary { 'elapsed_secs'?: number | null; 'total_usd_micros'?: number | null; 'superseded_by'?: string | null; + 'diff_summary'?: DiffSummary | null; } From befb2e00ec57bb61885de16df912f969203b849b Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Thu, 7 May 2026 22:07:13 -0700 Subject: [PATCH 11/63] feat(runs): merge command output streams Route command stderr into stdout at execution time and expose a single output log across events, projections, API clients, and the web UI. Keep replay compatibility for older command.completed events that still contain split stdout/stderr fields. --- apps/fabro-web/app/lib/queries.ts | 9 +- apps/fabro-web/app/lib/query-keys.test.ts | 4 +- apps/fabro-web/app/lib/query-keys.ts | 12 +- apps/fabro-web/app/routes/run-stages.test.ts | 15 +-- apps/fabro-web/app/routes/run-stages.tsx | 37 ++--- docs/public/api-reference/fabro-api.yaml | 41 +----- lib/crates/fabro-agent/src/tools.rs | 12 +- lib/crates/fabro-api/build.rs | 5 - lib/crates/fabro-api/src/lib.rs | 13 +- .../tests/command_output_stream_round_trip.rs | 44 ------ .../tests/run_projection_round_trip.rs | 3 +- .../tests/stage_projection_round_trip.rs | 3 +- lib/crates/fabro-cli/tests/it/cmd/runner.rs | 4 +- lib/crates/fabro-cli/tests/it/cmd/support.rs | 10 +- lib/crates/fabro-dump/src/lib.rs | 21 +-- lib/crates/fabro-retro/src/retro_agent.rs | 73 ++++------ .../fabro-server/src/principal_middleware.rs | 20 +-- lib/crates/fabro-server/src/server.rs | 2 - .../fabro-server/src/server/handler/mod.rs | 2 +- .../fabro-server/src/server/handler/runs.rs | 22 +-- lib/crates/fabro-server/src/server/tests.rs | 82 +++++------- .../fabro-server/tests/it/scenario/usage.rs | 16 ++- lib/crates/fabro-store/src/run_state.rs | 48 ++++--- .../src/serializable_projection.rs | 3 +- .../tests/serializable_projection.rs | 6 +- lib/crates/fabro-types/src/run_event/misc.rs | 108 +++++++++++++-- lib/crates/fabro-types/src/run_projection.rs | 16 +-- lib/crates/fabro-workflow/src/artifact.rs | 7 +- lib/crates/fabro-workflow/src/command_log.rs | 79 ++++------- lib/crates/fabro-workflow/src/context.rs | 1 - .../fabro-workflow/src/event/convert.rs | 22 ++- lib/crates/fabro-workflow/src/event/events.rs | 23 ++-- lib/crates/fabro-workflow/src/git.rs | 17 +-- .../fabro-workflow/src/handler/command.rs | 126 +++++++----------- .../src/handler/llm/preamble.rs | 125 ++++++----------- .../fabro-workflow/tests/it/integration.rs | 8 +- .../src/.openapi-generator/FILES | 1 - .../src/api/run-internals-api.ts | 35 ++--- .../src/models/command-log-response.ts | 10 +- .../src/models/command-output-stream.ts | 29 ---- .../fabro-api-client/src/models/index.ts | 1 - .../src/models/stage-projection.ts | 7 +- 42 files changed, 421 insertions(+), 701 deletions(-) delete mode 100644 lib/crates/fabro-api/tests/command_output_stream_round_trip.rs delete mode 100644 lib/packages/fabro-api-client/src/models/command-output-stream.ts diff --git a/apps/fabro-web/app/lib/queries.ts b/apps/fabro-web/app/lib/queries.ts index a13c92817..a958d743a 100644 --- a/apps/fabro-web/app/lib/queries.ts +++ b/apps/fabro-web/app/lib/queries.ts @@ -7,7 +7,6 @@ import type { PaginatedRunList, PaginatedRunStageList, CommandLogResponse, - CommandOutputStream, RunBilling, RunProjection, ServerSettings, @@ -153,23 +152,21 @@ export function useRunStageEvents(id: string | undefined, stageId: string | unde export function fetchRunCommandLog( id: string, stageId: string, - stream: CommandOutputStream, offset: number, limit?: number, ) { return apiFetcher( - queryKeys.runs.stageLog(id, stageId, stream, offset, limit), + queryKeys.runs.stageLog(id, stageId, offset, limit), ); } export function useRunStageLog( id: string | undefined, stageId: string | undefined, - stream: CommandOutputStream, enabled: boolean, ) { return useSWR( - enabled && id && stageId ? queryKeys.runs.stageLog(id, stageId, stream) : null, + enabled && id && stageId ? queryKeys.runs.stageLog(id, stageId) : null, apiFetcher, ); } @@ -209,4 +206,4 @@ export function useServerSettings() { return useSWR(queryKeys.settings.server(), apiFetcher, immutableOptions); } -export { apiTextFetcher }; \ No newline at end of file +export { apiTextFetcher }; diff --git a/apps/fabro-web/app/lib/query-keys.test.ts b/apps/fabro-web/app/lib/query-keys.test.ts index a06acaedb..43b9bb349 100644 --- a/apps/fabro-web/app/lib/query-keys.test.ts +++ b/apps/fabro-web/app/lib/query-keys.test.ts @@ -8,8 +8,8 @@ describe("queryKeys", () => { expect(queryKeys.auth.me()).toBe("/api/v1/auth/me"); expect(queryKeys.runs.files("run 1")).toBe("/api/v1/runs/run%201/files"); expect(queryKeys.runs.graph("run-1", "TB")).toBe("/api/v1/runs/run-1/graph?direction=TB"); - expect(queryKeys.runs.stageLog("run 1", "build step@2", "stderr", 12, 34)).toBe( - "/api/v1/runs/run%201/stages/build%20step%402/logs/stderr?offset=12&limit=34", + expect(queryKeys.runs.stageLog("run 1", "build step@2", 12, 34)).toBe( + "/api/v1/runs/run%201/stages/build%20step%402/logs/output?offset=12&limit=34", ); expect(queryKeys.runs.stageEvents("run 1", "build step", 7, 25)).toBe( "/api/v1/runs/run%201/stages/build%20step/events?since_seq=7&limit=25", diff --git a/apps/fabro-web/app/lib/query-keys.ts b/apps/fabro-web/app/lib/query-keys.ts index 1b1ffabf0..2fa6669f1 100644 --- a/apps/fabro-web/app/lib/query-keys.ts +++ b/apps/fabro-web/app/lib/query-keys.ts @@ -52,15 +52,9 @@ export const queryKeys = { since_seq: sinceSeq, limit, }), - stageLog: ( - id: string, - stageId: string, - stream: "stdout" | "stderr", - offset = 0, - limit = 65_536, - ) => + stageLog: (id: string, stageId: string, offset = 0, limit = 65_536) => withQuery( - `/api/v1/runs/${pathSegment(id)}/stages/${pathSegment(stageId)}/logs/${stream}`, + `/api/v1/runs/${pathSegment(id)}/stages/${pathSegment(stageId)}/logs/output`, { offset, limit }, ), preview: (id: string) => `/api/v1/runs/${pathSegment(id)}/preview`, @@ -81,4 +75,4 @@ export const queryKeys = { settings: { server: () => "/api/v1/settings", }, -}; \ No newline at end of file +}; diff --git a/apps/fabro-web/app/routes/run-stages.test.ts b/apps/fabro-web/app/routes/run-stages.test.ts index dd8282d23..b82264d6c 100644 --- a/apps/fabro-web/app/routes/run-stages.test.ts +++ b/apps/fabro-web/app/routes/run-stages.test.ts @@ -84,10 +84,8 @@ describe("eventsToActivity", () => { event: "command.completed", node_id: "fmt", properties: { - stdout: "blob://sha256/abc", - stderr: "blob://sha256/def", - stdout_bytes: 42, - stderr_bytes: 0, + output: "blob://sha256/abc", + output_bytes: 42, exit_code: 0, duration_ms: 12, termination: "exited", @@ -101,8 +99,7 @@ describe("eventsToActivity", () => { kind: "command", script: "cargo fmt", running: false, - stdoutBytes: 42, - stderrBytes: 0, + outputBytes: 42, }); }); @@ -119,8 +116,7 @@ describe("eventsToActivity", () => { stage_id: "verify@2", node_id: "verify", properties: { - stdout: "hi", - stderr: "", + output: "hi", exit_code: 0, duration_ms: 5, termination: "exited", @@ -425,8 +421,7 @@ describe("turnsToStageKind", () => { event: "command.completed", node_id: "fmt", properties: { - stdout: "blob://sha256/abc", - stderr: "blob://sha256/def", + output: "blob://sha256/abc", exit_code: 0, duration_ms: 5, termination: "exited", diff --git a/apps/fabro-web/app/routes/run-stages.tsx b/apps/fabro-web/app/routes/run-stages.tsx index 4d502fce3..efd1c7d36 100644 --- a/apps/fabro-web/app/routes/run-stages.tsx +++ b/apps/fabro-web/app/routes/run-stages.tsx @@ -33,7 +33,7 @@ import { import { STAGE_ACTIVITY_EVENT_TYPES, type StageActivityEventType } from "../lib/run-events"; import { mapRunStagesToSidebarStages } from "../lib/stage-sidebar"; import { getNumber, getString, type UnknownRecord } from "../lib/unknown"; -import type { CommandOutputStream, EventEnvelope } from "@qltysh/fabro-api-client"; +import type { EventEnvelope } from "@qltysh/fabro-api-client"; export const handle = { wide: true, fullHeight: true }; @@ -48,8 +48,7 @@ type TurnType = running: boolean; exitCode: number | null; durationMs: number; - stdoutBytes: number; - stderrBytes: number; + outputBytes: number; }; type CommandTurn = Extract; @@ -199,8 +198,7 @@ export function eventsToActivity(events: EventEnvelope[], stageId: string): Turn running: false, exitCode: getNumber(props, "exit_code") ?? null, durationMs: getNumber(props, "duration_ms") ?? 0, - stdoutBytes: getNumber(props, "stdout_bytes") ?? 0, - stderrBytes: getNumber(props, "stderr_bytes") ?? 0, + outputBytes: getNumber(props, "output_bytes") ?? 0, }); pendingCommand = undefined; break; @@ -218,8 +216,7 @@ export function eventsToActivity(events: EventEnvelope[], stageId: string): Turn running: true, exitCode: null, durationMs: 0, - stdoutBytes: 0, - stderrBytes: 0, + outputBytes: 0, }); } @@ -763,21 +760,17 @@ function decodeBase64Utf8(b64: string): string { function LogStream({ runId, stageId, - stream, label, byteCount, enabled, - tone, }: { runId: string; stageId: string; - stream: CommandOutputStream; label: string; byteCount: number; enabled: boolean; - tone?: "stderr"; }) { - const { data, error, isLoading } = useRunStageLog(runId, stageId, stream, enabled && byteCount > 0); + const { data, error, isLoading } = useRunStageLog(runId, stageId, enabled && byteCount > 0); const text = useMemo(() => { if (!data?.bytes_base64) return ""; try { @@ -802,16 +795,14 @@ function LogStream({ )} 
             {byteCount === 0 ? (
           empty
         ) : isLoading && !data ? (
           loading…
         ) : error ? (
-          Failed to load {stream}.
+          Failed to load output.
         ) : (
           text || empty
         )}
@@ -886,20 +877,10 @@ function CommandLogs({
       
-
    ); } diff --git a/docs/public/api-reference/fabro-api.yaml b/docs/public/api-reference/fabro-api.yaml index aa0965337..a1c6289b5 100644 --- a/docs/public/api-reference/fabro-api.yaml +++ b/docs/public/api-reference/fabro-api.yaml @@ -1877,16 +1877,15 @@ paths: schema: $ref: "#/components/schemas/ErrorResponse" - /api/v1/runs/{id}/stages/{stageId}/logs/{stream}: + /api/v1/runs/{id}/stages/{stageId}/logs/output: get: operationId: getRunStageCommandLog tags: [Run Internals] summary: Tail Command Log - description: Returns a byte-offset slice of a command stage stdout or stderr log. Bytes are base64-encoded and are not snapped to UTF-8 boundaries. + description: Returns a byte-offset slice of a command stage output log. Bytes are base64-encoded and are not snapped to UTF-8 boundaries. parameters: - $ref: "#/components/parameters/RunId" - $ref: "#/components/parameters/StageId" - - $ref: "#/components/parameters/CommandLogStream" - $ref: "#/components/parameters/CommandLogOffset" - $ref: "#/components/parameters/CommandLogLimit" responses: @@ -1897,7 +1896,7 @@ paths: schema: $ref: "#/components/schemas/CommandLogResponse" "400": - description: Invalid stage, stream, offset, or limit. + description: Invalid stage, offset, or limit. headers: x-request-id: $ref: "#/components/headers/XRequestId" @@ -3054,15 +3053,6 @@ components: type: string example: code@2 - CommandLogStream: - name: stream - in: path - required: true - description: Command output stream to read. - schema: - $ref: "#/components/schemas/CommandOutputStream" - example: stdout - CommandLogOffset: name: offset in: query @@ -5183,13 +5173,6 @@ components: description: Blob identifier. example: 550e8400-e29b-41d4-a716-446655440000 - CommandOutputStream: - description: Command output stream name. - type: string - enum: - - stdout - - stderr - CommandTermination: description: Terminal state for a command execution. type: string @@ -5202,7 +5185,6 @@ components: description: Byte-offset command log slice. type: object required: - - stream - offset - next_offset - total_bytes @@ -5211,8 +5193,6 @@ components: - cas_ref - live_streaming properties: - stream: - $ref: "#/components/schemas/CommandOutputStream" offset: type: integer minimum: 0 @@ -5226,7 +5206,7 @@ components: total_bytes: type: integer minimum: 0 - description: Total bytes currently available for the stream. + description: Total bytes currently available for the output log. example: 8192 bytes_base64: type: string @@ -5234,7 +5214,7 @@ components: example: aGVsbG8K eof: type: boolean - description: Whether the stream is finalized. + description: Whether the output log is finalized. example: false cas_ref: oneOf: @@ -5440,18 +5420,11 @@ components: items: type: object description: Per-branch result objects produced by a parallel stage. - stdout: + output: type: ["string", "null"] - stderr: - type: ["string", "null"] - stdout_bytes: + output_bytes: type: ["integer", "null"] minimum: 0 - stderr_bytes: - type: ["integer", "null"] - minimum: 0 - streams_separated: - type: ["boolean", "null"] live_streaming: type: ["boolean", "null"] termination: diff --git a/lib/crates/fabro-agent/src/tools.rs b/lib/crates/fabro-agent/src/tools.rs index e3b255c4d..ea72fdde8 100644 --- a/lib/crates/fabro-agent/src/tools.rs +++ b/lib/crates/fabro-agent/src/tools.rs @@ -235,6 +235,7 @@ pub fn make_shell_tool_with_config(config: &SessionOptions) -> RegisteredTool { executor: Arc::new(move |args, ctx| { Box::pin(async move { let command = required_str(&args, "command")?; + let command = format!("exec 2>&1\n{command}"); let timeout_ms = args .get("timeout_ms") .and_then(serde_json::Value::as_u64) @@ -249,7 +250,7 @@ pub fn make_shell_tool_with_config(config: &SessionOptions) -> RegisteredTool { let result = ctx .env .exec_command( - command, + &command, timeout_ms, None, tool_env.as_ref(), @@ -266,12 +267,11 @@ pub fn make_shell_tool_with_config(config: &SessionOptions) -> RegisteredTool { } let _ = write!( output, - "Exit code: {}\nstdout:\n{}\nstderr:\n{}", + "Exit code: {}\noutput:\n{}", result .exit_code .map_or_else(|| "none".to_string(), |code| code.to_string()), - result.stdout, - result.stderr + result.stdout ); Ok(output) }) @@ -916,8 +916,8 @@ mod tests { let tool = make_shell_tool(); let env: Arc = Arc::new(MockSandbox { exec_result: ExecResult { - stdout: String::new(), - stderr: "error".into(), + stdout: "error".into(), + stderr: String::new(), exit_code: Some(1), termination: CommandTermination::Exited, duration_ms: 10, diff --git a/lib/crates/fabro-api/build.rs b/lib/crates/fabro-api/build.rs index 50b868312..0aaee3837 100644 --- a/lib/crates/fabro-api/build.rs +++ b/lib/crates/fabro-api/build.rs @@ -337,11 +337,6 @@ fn main() { ("StageCompletion", "fabro_types::StageCompletion", &[]), ("StageOutcome", "fabro_types::StageOutcome", &[]), ("StageState", "fabro_types::StageState", &[]), - ( - "CommandOutputStream", - "fabro_types::CommandOutputStream", - &[], - ), ("CommandTermination", "fabro_types::CommandTermination", &[]), ("StageProjection", "fabro_types::StageProjection", &[]), ("SecretMetadata", "fabro_types::SecretMetadata", &[]), diff --git a/lib/crates/fabro-api/src/lib.rs b/lib/crates/fabro-api/src/lib.rs index 41433565c..5dc80b0db 100644 --- a/lib/crates/fabro-api/src/lib.rs +++ b/lib/crates/fabro-api/src/lib.rs @@ -29,13 +29,12 @@ pub mod types { BlockedReason, FailureReason, RunControlAction, RunStatus, SuccessReason, TerminalStatus, }; pub use fabro_types::{ - AuthMethod, BilledTokenCounts, CommandOutputStream, CommandTermination, DiffStats, - DiffSummary, DirtyStatus, EventEnvelope, GitContext, IdpIdentity, InterviewOption, - InterviewQuestionRecord, PendingInterviewRecord, PreRunPushOutcome, Principal, - QuestionType, RepositoryReference, RunClientProvenance, RunEvent, RunProjection, - RunProvenance, RunServerProvenance, RunSummary, SecretMetadata, SecretType, ServerSettings, - StageCompletion, StageOutcome, StageProjection, StageState, SystemActorKind, UserPrincipal, - WorkflowSettings, + AuthMethod, BilledTokenCounts, CommandTermination, DiffStats, DiffSummary, DirtyStatus, + EventEnvelope, GitContext, IdpIdentity, InterviewOption, InterviewQuestionRecord, + PendingInterviewRecord, PreRunPushOutcome, Principal, QuestionType, RepositoryReference, + RunClientProvenance, RunEvent, RunProjection, RunProvenance, RunServerProvenance, + RunSummary, SecretMetadata, SecretType, ServerSettings, StageCompletion, StageOutcome, + StageProjection, StageState, SystemActorKind, UserPrincipal, WorkflowSettings, }; pub use crate::generated::types::*; diff --git a/lib/crates/fabro-api/tests/command_output_stream_round_trip.rs b/lib/crates/fabro-api/tests/command_output_stream_round_trip.rs deleted file mode 100644 index 9abd55162..000000000 --- a/lib/crates/fabro-api/tests/command_output_stream_round_trip.rs +++ /dev/null @@ -1,44 +0,0 @@ -use std::any::{TypeId, type_name}; - -use fabro_api::types::CommandOutputStream as ApiCommandOutputStream; -use fabro_types::CommandOutputStream; -use serde_json::json; - -#[test] -fn command_output_stream_reuses_canonical_type() { - assert_same_type::(); -} - -#[test] -fn command_output_stream_serializes_as_stream_names() { - assert_eq!( - serde_json::to_value(CommandOutputStream::Stdout).unwrap(), - json!("stdout") - ); - assert_eq!( - serde_json::to_value(CommandOutputStream::Stderr).unwrap(), - json!("stderr") - ); -} - -#[test] -fn command_output_stream_deserializes_representative_values() { - assert_eq!( - serde_json::from_value::(json!("stdout")).unwrap(), - CommandOutputStream::Stdout - ); - assert_eq!( - serde_json::from_value::(json!("stderr")).unwrap(), - CommandOutputStream::Stderr - ); -} - -fn assert_same_type() { - assert_eq!( - TypeId::of::(), - TypeId::of::(), - "{} should be the same type as {}", - type_name::(), - type_name::() - ); -} diff --git a/lib/crates/fabro-api/tests/run_projection_round_trip.rs b/lib/crates/fabro-api/tests/run_projection_round_trip.rs index ba3c91f2d..c43c1e8bb 100644 --- a/lib/crates/fabro-api/tests/run_projection_round_trip.rs +++ b/lib/crates/fabro-api/tests/run_projection_round_trip.rs @@ -68,8 +68,7 @@ fn run_projection_round_trips_populated_projection() { "script_invocation": null, "script_timing": null, "parallel_results": null, - "stdout": "done", - "stderr": null + "output": "done" } } }); diff --git a/lib/crates/fabro-api/tests/stage_projection_round_trip.rs b/lib/crates/fabro-api/tests/stage_projection_round_trip.rs index ad4dcb579..a7b397be3 100644 --- a/lib/crates/fabro-api/tests/stage_projection_round_trip.rs +++ b/lib/crates/fabro-api/tests/stage_projection_round_trip.rs @@ -26,8 +26,7 @@ fn stage_projection_round_trips_representative_json() { "script_invocation": { "command": "cargo test" }, "script_timing": { "duration_ms": 42 }, "parallel_results": [{ "branch": 0, "status": "succeeded" }], - "stdout": "ok", - "stderr": "", + "output": "ok", "termination": "exited", "started_at": "2026-04-29T12:34:00Z", "duration_ms": 56000, diff --git a/lib/crates/fabro-cli/tests/it/cmd/runner.rs b/lib/crates/fabro-cli/tests/it/cmd/runner.rs index 5efe2c58c..56e929ff0 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/runner.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/runner.rs @@ -16,7 +16,7 @@ use fabro_store::EventEnvelope; use fabro_test::{ assert_reqwest_status, expect_reqwest_json, fabro_json_snapshot, fabro_snapshot, test_context, }; -use fabro_types::{CommandOutputStream, EventBody, FailureReason, RunEvent, StageId}; +use fabro_types::{EventBody, FailureReason, RunEvent, StageId}; use httpmock::MockServer; use super::support::{ @@ -505,7 +505,7 @@ methods = ["dev-token"] let _probe = state .stage(&probe_stage_id) .expect("probe node state should exist"); - let stdout = command_log_text(&run_dir, &probe_stage_id, CommandOutputStream::Stdout); + let stdout = command_log_text(&run_dir, &probe_stage_id); assert!( stdout.contains("probe-ran"), "probe stage should have executed, got stdout:\n{stdout}" diff --git a/lib/crates/fabro-cli/tests/it/cmd/support.rs b/lib/crates/fabro-cli/tests/it/cmd/support.rs index e295ff5d7..6cf1977e4 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/support.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/support.rs @@ -21,7 +21,7 @@ use fabro_config::daemon::ServerDaemon; use fabro_config::{Storage, envfile}; use fabro_store::EventEnvelope; use fabro_test::{TestContext, expect_reqwest_status}; -use fabro_types::{CommandOutputStream, RunId, StageId}; +use fabro_types::{RunId, StageId}; use httpmock::{Mock, MockServer}; use serde_json::Value; use shlex::try_quote; @@ -704,15 +704,11 @@ pub(crate) fn run_events(run_dir: &Path) -> Vec { crate::support::parse_event_envelopes(&response) } -pub(crate) fn command_log_text( - run_dir: &Path, - stage_id: &StageId, - stream: CommandOutputStream, -) -> String { +pub(crate) fn command_log_text(run_dir: &Path, stage_id: &StageId) -> String { let run_id = infer_run_id(run_dir); let response: CommandLogResponseRecord = block_on(get_server_json( run_dir, - &format!("/api/v1/runs/{run_id}/stages/{stage_id}/logs/{stream}?offset=0&limit=1048576"), + &format!("/api/v1/runs/{run_id}/stages/{stage_id}/logs/output?offset=0&limit=1048576"), )); let bytes = BASE64_STANDARD .decode(&response.bytes_base64) diff --git a/lib/crates/fabro-dump/src/lib.rs b/lib/crates/fabro-dump/src/lib.rs index 5b2da9403..b9fce2d99 100644 --- a/lib/crates/fabro-dump/src/lib.rs +++ b/lib/crates/fabro-dump/src/lib.rs @@ -129,16 +129,10 @@ impl RunDump { parallel_results.clone(), )); } - if let Some(stdout) = stage.stdout.as_ref() { + if let Some(output) = stage.output.as_ref() { entries.push(RunDumpEntry::text_path( - &base.join("stdout.log"), - stdout.clone(), - )); - } - if let Some(stderr) = stage.stderr.as_ref() { - entries.push(RunDumpEntry::text_path( - &base.join("stderr.log"), - stderr.clone(), + &base.join("output.log"), + output.clone(), )); } } @@ -597,8 +591,7 @@ mod tests { stage.script_invocation = Some(serde_json::json!({ "command": "cargo test" })); stage.script_timing = Some(serde_json::json!({ "duration_ms": 10 })); stage.parallel_results = Some(serde_json::json!([{ "stage": "fanout@1" }])); - stage.stdout = Some("stdout".to_string()); - stage.stderr = Some("stderr".to_string()); + stage.output = Some("output".to_string()); let dump = RunDump::from_projection(&projection).unwrap(); let paths: Vec<&str> = dump @@ -619,8 +612,7 @@ mod tests { assert!(paths.contains(&"stages/001-build@2/script_invocation.json")); assert!(paths.contains(&"stages/001-build@2/script_timing.json")); assert!(paths.contains(&"stages/001-build@2/parallel_results.json")); - assert!(paths.contains(&"stages/001-build@2/stdout.log")); - assert!(paths.contains(&"stages/001-build@2/stderr.log")); + assert!(paths.contains(&"stages/001-build@2/output.log")); assert!(!paths.contains(&"start.json")); assert!(!paths.contains(&"status.json")); assert!(!paths.contains(&"checkpoint.json")); @@ -648,8 +640,7 @@ mod tests { assert_eq!(node.prompt, None); assert_eq!(node.response, None); assert_eq!(node.diff, None); - assert_eq!(node.stdout, None); - assert_eq!(node.stderr, None); + assert_eq!(node.output, None); assert_eq!( node.provider_used, Some(serde_json::json!({ "provider": "openai" })) diff --git a/lib/crates/fabro-retro/src/retro_agent.rs b/lib/crates/fabro-retro/src/retro_agent.rs index 30d5dd2d6..4a630662f 100644 --- a/lib/crates/fabro-retro/src/retro_agent.rs +++ b/lib/crates/fabro-retro/src/retro_agent.rs @@ -24,7 +24,7 @@ You have access to the run's data files: - `graph.fabro` — the workflow source for the run - `checkpoints/{seq:04}.json` — zero-padded checkpoint snapshots captured during the run - `run.log` — server/worker log output for the run when available -- `stages/{rank:03}-{node_id}@{visit}/...` — execution-order-prefixed per-stage prompt, response, status, diff, stdout/stderr, and tool metadata files +- `stages/{rank:03}-{node_id}@{visit}/...` — execution-order-prefixed per-stage prompt, response, status, diff, output, and tool metadata files ## Your task @@ -430,8 +430,7 @@ mod tests { stage.script_invocation = Some(serde_json::json!({ "command": "cargo test" })); stage.script_timing = Some(serde_json::json!({ "duration_ms": 10 })); stage.parallel_results = Some(serde_json::json!([{ "stage": "fanout@1" }])); - stage.stdout = Some("stdout".to_string()); - stage.stderr = Some("stderr".to_string()); + stage.output = Some("output".to_string()); upload_data_files( &sandbox, @@ -473,10 +472,10 @@ mod tests { "done" ); assert_eq!( - fs::read_to_string(target_dir.join("stages/001-build@2/stdout.log")) + fs::read_to_string(target_dir.join("stages/001-build@2/output.log")) .await - .expect("stdout file should exist"), - "stdout" + .expect("output file should exist"), + "output" ); assert_eq!( fs::read_to_string(target_dir.join("events.jsonl")) @@ -501,7 +500,7 @@ mod tests { } #[tokio::test] - async fn upload_data_files_resolves_command_stdout_stderr_blob_refs() { + async fn upload_data_files_resolves_command_output_blob_refs() { let sandbox_root = tempfile::tempdir().expect("sandbox tempdir should exist"); let sandbox: Arc = Arc::new(LocalSandbox::new(sandbox_root.path().to_path_buf())); @@ -509,37 +508,28 @@ mod tests { let target_dir = output_dir.path().join("retro"); let target_dir_str = target_dir.to_string_lossy().to_string(); - let stdout_blob = serde_json::to_vec("resolved stdout").unwrap(); - let stderr_blob = serde_json::to_vec("resolved stderr").unwrap(); - let stdout_id = fabro_types::RunBlobId::new(&stdout_blob); - let stderr_id = fabro_types::RunBlobId::new(&stderr_blob); + let output_blob = serde_json::to_vec("resolved output").unwrap(); + let output_id = fabro_types::RunBlobId::new(&output_blob); let stage_id = StageId::new("build", 1); let mut state = RunProjection::default(); - let stdout_ref = fabro_types::format_blob_ref(&stdout_id); - let stderr_ref = fabro_types::format_blob_ref(&stderr_id); + let output_ref = fabro_types::format_blob_ref(&output_id); let stage = state.stage_entry(stage_id.node_id(), stage_id.visit(), first_event_seq(1)); stage.script_invocation = Some(serde_json::json!({ "command": "cargo test", - "stdout": stdout_ref, - "stderr": stderr_ref, + "output": output_ref, })); stage.script_timing = Some(serde_json::json!({ "exit_code": 0, - "stdout": stdout_ref, - "stderr": stderr_ref, + "output": output_ref, })); - stage.stdout = Some(stdout_ref); - stage.stderr = Some(stderr_ref); + stage.output = Some(output_ref); let reader: BlobReader = Box::new(move |blob_id| { - let stdout_blob = stdout_blob.clone(); - let stderr_blob = stderr_blob.clone(); + let output_blob = output_blob.clone(); Box::pin(async move { - if blob_id == stdout_id { - Ok(Some(stdout_blob.into())) - } else if blob_id == stderr_id { - Ok(Some(stderr_blob.into())) + if blob_id == output_id { + Ok(Some(output_blob.into())) } else { Ok(None) } @@ -551,16 +541,10 @@ mod tests { .expect("retro files should upload"); assert_eq!( - fs::read_to_string(target_dir.join("stages/001-build@1/stdout.log")) + fs::read_to_string(target_dir.join("stages/001-build@1/output.log")) .await - .expect("stdout file should exist"), - "resolved stdout" - ); - assert_eq!( - fs::read_to_string(target_dir.join("stages/001-build@1/stderr.log")) - .await - .expect("stderr file should exist"), - "resolved stderr" + .expect("output file should exist"), + "resolved output" ); let script_timing: serde_json::Value = serde_json::from_str( @@ -569,8 +553,7 @@ mod tests { .expect("script timing should exist"), ) .expect("script timing should parse"); - assert_eq!(script_timing["stdout"], "resolved stdout"); - assert_eq!(script_timing["stderr"], "resolved stderr"); + assert_eq!(script_timing["output"], "resolved output"); let script_invocation: serde_json::Value = serde_json::from_str( &fs::read_to_string(target_dir.join("stages/001-build@1/script_invocation.json")) @@ -578,8 +561,7 @@ mod tests { .expect("script invocation should exist"), ) .expect("script invocation should parse"); - assert_eq!(script_invocation["stdout"], "resolved stdout"); - assert_eq!(script_invocation["stderr"], "resolved stderr"); + assert_eq!(script_invocation["output"], "resolved output"); let run_json: serde_json::Value = serde_json::from_str( &fs::read_to_string(target_dir.join("run.json")) @@ -588,18 +570,13 @@ mod tests { ) .expect("run.json should parse"); assert_eq!( - run_json["stages"]["build@1"]["script_timing"]["stdout"], - "resolved stdout" + run_json["stages"]["build@1"]["script_timing"]["output"], + "resolved output" ); assert_eq!( - run_json["stages"]["build@1"]["script_timing"]["stderr"], - "resolved stderr" + run_json["stages"]["build@1"]["script_invocation"]["output"], + "resolved output" ); - assert_eq!( - run_json["stages"]["build@1"]["script_invocation"]["stdout"], - "resolved stdout" - ); - assert!(run_json["stages"]["build@1"]["stdout"].is_null()); - assert!(run_json["stages"]["build@1"]["stderr"].is_null()); + assert!(run_json["stages"]["build@1"]["output"].is_null()); } } diff --git a/lib/crates/fabro-server/src/principal_middleware.rs b/lib/crates/fabro-server/src/principal_middleware.rs index 822d87dc6..f1a84a5fa 100644 --- a/lib/crates/fabro-server/src/principal_middleware.rs +++ b/lib/crates/fabro-server/src/principal_middleware.rs @@ -6,7 +6,7 @@ use axum::http::StatusCode; use axum::http::request::Parts; use axum::middleware::Next; use axum::response::{IntoResponse, Response}; -use fabro_types::{CommandOutputStream, Principal, RunBlobId, RunId, StageId, UserPrincipal}; +use fabro_types::{Principal, RunBlobId, RunId, StageId, UserPrincipal}; use jsonwebtoken::decode_header; use strum::IntoStaticStr; @@ -54,11 +54,7 @@ pub(crate) struct RequireRunScoped(pub(crate) RunId); pub(crate) struct RequireRunBlob(pub(crate) RunId, pub(crate) RunBlobId); pub(crate) struct RequireRunStageScoped(pub(crate) RunId, pub(crate) String); pub(crate) struct RequireStageArtifact(pub(crate) RunId, pub(crate) StageId); -pub(crate) struct RequireCommandLog( - pub(crate) RunId, - pub(crate) StageId, - pub(crate) CommandOutputStream, -); +pub(crate) struct RequireCommandLog(pub(crate) RunId, pub(crate) StageId); #[derive(Clone, Debug)] pub(crate) struct AuthenticatedUser { @@ -246,18 +242,14 @@ impl FromRequestParts> for RequireCommandLog { parts: &mut Parts, state: &Arc, ) -> Result { - let Path((id, stage_id, stream)): Path<(String, String, String)> = - Path::from_request_parts(parts, state) - .await - .map_err(IntoResponse::into_response)?; + let Path((id, stage_id)): Path<(String, String)> = Path::from_request_parts(parts, state) + .await + .map_err(IntoResponse::into_response)?; let run_id = parse_run_id_path(&id)?; let stage_id = parse_stage_id_path(&stage_id)?; - let stream = stream - .parse::() - .map_err(|_| ApiError::bad_request("Invalid command log stream.").into_response())?; require_worker_or_user_for_run(&auth_slot_from_parts(parts), &run_id) .map_err(IntoResponse::into_response)?; - Ok(Self(run_id, stage_id, stream)) + Ok(Self(run_id, stage_id)) } } diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index ce4fca90f..f1b56c0fc 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -71,8 +71,6 @@ use fabro_store::{ }; #[cfg(test)] use fabro_types::BlockedReason; -#[cfg(test)] -use fabro_types::CommandOutputStream; use fabro_types::settings::run::RunMode; use fabro_types::settings::server::{ GithubIntegrationSettings, GithubIntegrationStrategy, LogDestination, diff --git a/lib/crates/fabro-server/src/server/handler/mod.rs b/lib/crates/fabro-server/src/server/handler/mod.rs index 451b9e7d9..715a15862 100644 --- a/lib/crates/fabro-server/src/server/handler/mod.rs +++ b/lib/crates/fabro-server/src/server/handler/mod.rs @@ -44,7 +44,7 @@ pub(super) fn demo_routes() -> Router> { .route("/runs/{id}/blobs", post(not_implemented)) .route("/runs/{id}/blobs/{blobId}", get(not_implemented)) .route( - "/runs/{id}/stages/{stageId}/logs/{stream}", + "/runs/{id}/stages/{stageId}/logs/output", get(not_implemented), ) .route("/runs/{id}/checkpoint", get(demo::checkpoint_stub)) diff --git a/lib/crates/fabro-server/src/server/handler/runs.rs b/lib/crates/fabro-server/src/server/handler/runs.rs index 77133e63b..ab3e92860 100644 --- a/lib/crates/fabro-server/src/server/handler/runs.rs +++ b/lib/crates/fabro-server/src/server/handler/runs.rs @@ -16,8 +16,8 @@ use fabro_api::types::{ use fabro_config::Storage; use fabro_interview::AnswerSubmission; use fabro_types::{ - CommandOutputStream, Principal, RunClientProvenance, RunId, RunProvenance, RunServerProvenance, - UserPrincipal, parse_blob_ref, + Principal, RunClientProvenance, RunId, RunProvenance, RunServerProvenance, UserPrincipal, + parse_blob_ref, }; use fabro_util::version::FABRO_VERSION; use fabro_workflow::command_log::{command_log_path, read_json_string_blob, read_log_slice}; @@ -57,7 +57,7 @@ pub(super) fn routes() -> Router> { .route("/runs/{id}/state", get(get_run_state)) .route("/runs/{id}/logs", get(get_run_logs)) .route( - "/runs/{id}/stages/{stageId}/logs/{stream}", + "/runs/{id}/stages/{stageId}/logs/output", get(get_run_stage_command_log), ) .route("/runs/{id}/settings", get(get_run_settings)) @@ -302,7 +302,6 @@ struct CommandLogQuery { #[derive(Debug, serde::Serialize)] struct CommandLogResponseBody { - stream: CommandOutputStream, offset: u64, next_offset: u64, total_bytes: u64, @@ -677,7 +676,7 @@ async fn get_run_logs( } async fn get_run_stage_command_log( - RequireCommandLog(id, stage_id, stream): RequireCommandLog, + RequireCommandLog(id, stage_id): RequireCommandLog, State(state): State>, Query(query): Query, ) -> Response { @@ -701,10 +700,7 @@ async fn get_run_stage_command_log( return ApiError::not_found("Stage not found.").into_response(); }; - let stream_value = match stream { - CommandOutputStream::Stdout => node.stdout.as_deref(), - CommandOutputStream::Stderr => node.stderr.as_deref(), - }; + let stream_value = node.output.as_deref(); let cas_ref = stream_value .filter(|value| parse_blob_ref(value).is_some()) .map(str::to_string); @@ -715,12 +711,11 @@ async fn get_run_stage_command_log( .run_scratch(&id) .root() .to_path_buf(); - let scratch_path = command_log_path(&run_dir, &stage_id, stream); + let scratch_path = command_log_path(&run_dir, &stage_id); match read_log_slice(&scratch_path, query.offset, limit).await { Ok((bytes, total_bytes)) => { return build_command_log_response( - stream, query.offset, limit, LogSource::Sliced { bytes, total_bytes }, @@ -746,7 +741,6 @@ async fn get_run_stage_command_log( } }; return build_command_log_response( - stream, query.offset, limit, LogSource::Full(text.as_bytes()), @@ -758,7 +752,6 @@ async fn get_run_stage_command_log( if let Some(inline_text) = stream_value { return build_command_log_response( - stream, query.offset, limit, LogSource::Full(inline_text.as_bytes()), @@ -769,7 +762,6 @@ async fn get_run_stage_command_log( } build_command_log_response( - stream, query.offset, limit, LogSource::Full(&[]), @@ -788,7 +780,6 @@ enum LogSource<'a> { } fn build_command_log_response( - stream: CommandOutputStream, requested_offset: u64, limit: u64, source: LogSource<'_>, @@ -812,7 +803,6 @@ fn build_command_log_response( } }; Json(CommandLogResponseBody { - stream, offset, next_offset: offset + u64::try_from(body_bytes.len()).unwrap_or(u64::MAX), total_bytes, diff --git a/lib/crates/fabro-server/src/server/tests.rs b/lib/crates/fabro-server/src/server/tests.rs index 8e1e8d608..0c43973f0 100644 --- a/lib/crates/fabro-server/src/server/tests.rs +++ b/lib/crates/fabro-server/src/server/tests.rs @@ -4173,7 +4173,7 @@ async fn get_run_stage_command_log_returns_scratch_slice() { .run_scratch(&run_id) .root() .to_path_buf(); - let log_path = command_log_path(&run_dir, &stage_id, CommandOutputStream::Stdout); + let log_path = command_log_path(&run_dir, &stage_id); tokio::fs::create_dir_all(log_path.parent().unwrap()) .await .unwrap(); @@ -4182,7 +4182,7 @@ async fn get_run_stage_command_log_returns_scratch_slice() { let req = Request::builder() .method("GET") .uri(api(&format!( - "/runs/{run_id}/stages/{stage_id}/logs/stdout?offset=6&limit=5" + "/runs/{run_id}/stages/{stage_id}/logs/output?offset=6&limit=5" ))) .body(Body::empty()) .unwrap(); @@ -4193,7 +4193,7 @@ async fn get_run_stage_command_log_returns_scratch_slice() { .decode(body["bytes_base64"].as_str().unwrap()) .unwrap(); - assert_eq!(body["stream"], "stdout"); + assert!(body.get("stream").is_none()); assert_eq!(body["offset"], 6); assert_eq!(body["next_offset"], 11); assert_eq!(body["total_bytes"], 11); @@ -4209,16 +4209,11 @@ async fn get_run_stage_command_log_returns_cas_slice() { let app = crate::test_support::build_test_router(Arc::clone(&state)); let run_id = RunId::new(); let run_store = state.store.create_run(&run_id).await.unwrap(); - let stdout_blob = run_store + let output_blob = run_store .write_blob(&serde_json::to_vec("hello world").unwrap()) .await .unwrap(); - let stderr_blob = run_store - .write_blob(&serde_json::to_vec("").unwrap()) - .await - .unwrap(); - let stdout_ref = format!("blob://sha256/{stdout_blob}"); - let stderr_ref = format!("blob://sha256/{stderr_blob}"); + let output_ref = format!("blob://sha256/{output_blob}"); for event in [ workflow_event::Event::RunSubmitted { definition_blob: None, @@ -4232,16 +4227,13 @@ async fn get_run_stage_command_log_returns_cas_slice() { max_attempts: 1, }, workflow_event::Event::CommandCompleted { - node_id: "script_node".to_string(), - stdout: stdout_ref.clone(), - stderr: stderr_ref, - exit_code: Some(0), - duration_ms: 5, - termination: CommandTermination::Exited, - stdout_bytes: 11, - stderr_bytes: 0, - streams_separated: true, - live_streaming: false, + node_id: "script_node".to_string(), + output: output_ref.clone(), + exit_code: Some(0), + duration_ms: 5, + termination: CommandTermination::Exited, + output_bytes: 11, + live_streaming: false, }, ] { workflow_event::append_event(&run_store, &run_id, &event) @@ -4252,7 +4244,7 @@ async fn get_run_stage_command_log_returns_cas_slice() { let req = Request::builder() .method("GET") .uri(api(&format!( - "/runs/{run_id}/stages/script_node@1/logs/stdout?offset=6&limit=5" + "/runs/{run_id}/stages/script_node@1/logs/output?offset=6&limit=5" ))) .body(Body::empty()) .unwrap(); @@ -4263,13 +4255,13 @@ async fn get_run_stage_command_log_returns_cas_slice() { .decode(body["bytes_base64"].as_str().unwrap()) .unwrap(); - assert_eq!(body["stream"], "stdout"); + assert!(body.get("stream").is_none()); assert_eq!(body["offset"], 6); assert_eq!(body["next_offset"], 11); assert_eq!(body["total_bytes"], 11); assert_eq!(bytes, b"world"); assert_eq!(body["eof"], true); - assert_eq!(body["cas_ref"], stdout_ref); + assert_eq!(body["cas_ref"], output_ref); assert_eq!(body["live_streaming"], false); } @@ -4280,16 +4272,11 @@ async fn get_run_stage_command_log_prefers_scratch_when_cas_ref_exists() { let run_id = RunId::new(); let stage_id = StageId::new("script_node", 1); let run_store = state.store.create_run(&run_id).await.unwrap(); - let stdout_blob = run_store + let output_blob = run_store .write_blob(&serde_json::to_vec("cas log").unwrap()) .await .unwrap(); - let stderr_blob = run_store - .write_blob(&serde_json::to_vec("").unwrap()) - .await - .unwrap(); - let stdout_ref = format!("blob://sha256/{stdout_blob}"); - let stderr_ref = format!("blob://sha256/{stderr_blob}"); + let output_ref = format!("blob://sha256/{output_blob}"); for event in [ workflow_event::Event::RunSubmitted { definition_blob: None, @@ -4303,16 +4290,13 @@ async fn get_run_stage_command_log_prefers_scratch_when_cas_ref_exists() { max_attempts: 1, }, workflow_event::Event::CommandCompleted { - node_id: "script_node".to_string(), - stdout: stdout_ref.clone(), - stderr: stderr_ref, - exit_code: Some(0), - duration_ms: 5, - termination: CommandTermination::Exited, - stdout_bytes: 7, - stderr_bytes: 0, - streams_separated: true, - live_streaming: false, + node_id: "script_node".to_string(), + output: output_ref.clone(), + exit_code: Some(0), + duration_ms: 5, + termination: CommandTermination::Exited, + output_bytes: 7, + live_streaming: false, }, ] { workflow_event::append_event(&run_store, &run_id, &event) @@ -4324,7 +4308,7 @@ async fn get_run_stage_command_log_prefers_scratch_when_cas_ref_exists() { .run_scratch(&run_id) .root() .to_path_buf(); - let log_path = command_log_path(&run_dir, &stage_id, CommandOutputStream::Stdout); + let log_path = command_log_path(&run_dir, &stage_id); tokio::fs::create_dir_all(log_path.parent().unwrap()) .await .unwrap(); @@ -4333,7 +4317,7 @@ async fn get_run_stage_command_log_prefers_scratch_when_cas_ref_exists() { let req = Request::builder() .method("GET") .uri(api(&format!( - "/runs/{run_id}/stages/{stage_id}/logs/stdout?offset=0&limit=64" + "/runs/{run_id}/stages/{stage_id}/logs/output?offset=0&limit=64" ))) .body(Body::empty()) .unwrap(); @@ -4344,13 +4328,13 @@ async fn get_run_stage_command_log_prefers_scratch_when_cas_ref_exists() { .decode(body["bytes_base64"].as_str().unwrap()) .unwrap(); - assert_eq!(body["stream"], "stdout"); + assert!(body.get("stream").is_none()); assert_eq!(body["offset"], 0); assert_eq!(body["next_offset"], 11); assert_eq!(body["total_bytes"], 11); assert_eq!(bytes, b"scratch log"); assert_eq!(body["eof"], true); - assert_eq!(body["cas_ref"], stdout_ref); + assert_eq!(body["cas_ref"], output_ref); assert_eq!(body["live_streaming"], false); } @@ -4366,7 +4350,7 @@ async fn get_run_stage_command_log_returns_not_found_for_missing_stage() { let req = Request::builder() .method("GET") - .uri(api(&format!("/runs/{run_id}/stages/missing@1/logs/stdout"))) + .uri(api(&format!("/runs/{run_id}/stages/missing@1/logs/output"))) .body(Body::empty()) .unwrap(); @@ -6004,7 +5988,7 @@ async fn worker_token_controls_command_log_route() { .clone() .oneshot(bearer_request( Method::GET, - &format!("/runs/{run_id}/stages/code@1/logs/stdout"), + &format!("/runs/{run_id}/stages/code@1/logs/output"), &worker_token, Body::empty(), )) @@ -6016,7 +6000,7 @@ async fn worker_token_controls_command_log_route() { .clone() .oneshot(bearer_request( Method::GET, - &format!("/runs/{run_id}/stages/code@1/logs/stdout"), + &format!("/runs/{run_id}/stages/code@1/logs/output"), &user_jwt, Body::empty(), )) @@ -6028,7 +6012,7 @@ async fn worker_token_controls_command_log_route() { .clone() .oneshot(bearer_request( Method::GET, - &format!("/runs/{run_id}/stages/code@1/logs/stdout"), + &format!("/runs/{run_id}/stages/code@1/logs/output"), &mismatched_worker_token, Body::empty(), )) @@ -6040,7 +6024,7 @@ async fn worker_token_controls_command_log_route() { .oneshot( Request::builder() .method(Method::GET) - .uri(api(&format!("/runs/{run_id}/stages/code@1/logs/stdout"))) + .uri(api(&format!("/runs/{run_id}/stages/code@1/logs/output"))) .body(Body::empty()) .unwrap(), ) diff --git a/lib/crates/fabro-server/tests/it/scenario/usage.rs b/lib/crates/fabro-server/tests/it/scenario/usage.rs index 264124bcf..db5959e3c 100644 --- a/lib/crates/fabro-server/tests/it/scenario/usage.rs +++ b/lib/crates/fabro-server/tests/it/scenario/usage.rs @@ -17,6 +17,14 @@ const COMMAND_DOT: &str = r#"digraph Test { start -> echo_task -> exit }"#; +const WAIT_DOT: &str = r#"digraph Test { + graph [goal="Test"] + start [shape=Mdiamond] + wait_task [shape=insulator, duration="1ms"] + exit [shape=Msquare] + start -> wait_task -> exit +}"#; + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn aggregate_billing_increments_after_run_completes() { let state = test_app_state_with_options(test_settings(), 5); @@ -59,15 +67,13 @@ async fn run_billing_includes_completed_non_llm_stages() { let state = test_app_state_with_options(test_settings(), 5); let app = test_app_with_scheduler(state); - let run_id = - create_and_start_run_from_manifest(&app, minimal_manifest_json_with_dry_run(MINIMAL_DOT)) - .await; + let run_id = create_and_start_run_from_manifest(&app, minimal_manifest_json(WAIT_DOT)).await; let status = wait_for_run_status(&app, &run_id, &["succeeded", "failed"]).await; assert_eq!(status, "succeeded"); let billing = run_billing(&app, &run_id).await; - assert_non_llm_billing(&billing, &["start"]); + assert_non_llm_billing(&billing, &["wait_task"]); } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] @@ -81,7 +87,7 @@ async fn run_billing_includes_completed_command_stages() { assert_eq!(status, "succeeded"); let billing = run_billing(&app, &run_id).await; - assert_non_llm_billing(&billing, &["echo_task", "start"]); + assert_non_llm_billing(&billing, &["echo_task"]); } async fn run_billing(app: &axum::Router, run_id: &str) -> serde_json::Value { diff --git a/lib/crates/fabro-store/src/run_state.rs b/lib/crates/fabro-store/src/run_state.rs index 8b38ea485..d1b1be619 100644 --- a/lib/crates/fabro-store/src/run_state.rs +++ b/lib/crates/fabro-store/src/run_state.rs @@ -386,11 +386,8 @@ impl RunProjectionReducer for RunProjection { let Some(stage) = stage_at_stored_or_current_visit(self, stored, event.seq) else { return Ok(()); }; - stage.stdout = Some(props.stdout.clone()); - stage.stderr = Some(props.stderr.clone()); - stage.stdout_bytes = Some(props.stdout_bytes); - stage.stderr_bytes = Some(props.stderr_bytes); - stage.streams_separated = Some(props.streams_separated); + stage.output = Some(props.output.clone()); + stage.output_bytes = Some(props.output_bytes); stage.live_streaming = Some(props.live_streaming); stage.termination = Some(props.termination); stage.script_timing = Some(script_timing); @@ -402,8 +399,7 @@ impl RunProjectionReducer for RunProjection { apply_agent_cli_terminal( stage, props, - &props.stdout, - &props.stderr, + merge_agent_cli_output(&props.stdout, &props.stderr), CommandTermination::Exited, )?; } @@ -414,8 +410,7 @@ impl RunProjectionReducer for RunProjection { apply_agent_cli_terminal( stage, props, - &props.stdout, - &props.stderr, + merge_agent_cli_output(&props.stdout, &props.stderr), CommandTermination::Cancelled, )?; } @@ -426,8 +421,7 @@ impl RunProjectionReducer for RunProjection { apply_agent_cli_terminal( stage, props, - &props.stdout, - &props.stderr, + merge_agent_cli_output(&props.stdout, &props.stderr), CommandTermination::TimedOut, )?; } @@ -718,19 +712,26 @@ fn provider_used_from_agent_cli_started(props: &AgentCliStartedProps) -> Value { fn apply_agent_cli_terminal( stage: &mut StageProjection, props: &impl serde::Serialize, - stdout: &str, - stderr: &str, + output: String, termination: CommandTermination, ) -> Result<()> { let script_timing = serde_json::to_value(props) .map_err(|err| Error::InvalidEvent(format!("invalid agent.cli terminal payload: {err}")))?; - stage.stdout = Some(stdout.to_string()); - stage.stderr = Some(stderr.to_string()); + stage.output = Some(output); stage.termination = Some(termination); stage.script_timing = Some(script_timing); Ok(()) } +fn merge_agent_cli_output(stdout: &str, stderr: &str) -> String { + match (stdout.is_empty(), stderr.is_empty()) { + (true, true) => String::new(), + (false, true) => stdout.to_string(), + (true, false) => stderr.to_string(), + (false, false) => format!("{stdout}\n{stderr}"), + } +} + #[cfg(test)] mod tests { use std::collections::{BTreeMap, HashMap}; @@ -911,7 +912,7 @@ mod tests { "build@2": { "first_event_seq": 1, "diff": "diff --git a/file b/file", - "stdout": "done" + "output": "done" } } })) @@ -928,7 +929,7 @@ mod tests { serde_json::from_value(serde_json::to_value(&state).unwrap()).unwrap(); let serialized = serde_json::to_value(&state).unwrap(); let round_tripped_node = round_tripped.stage(&stage_id).unwrap(); - assert_eq!(round_tripped_node.stdout.as_deref(), Some("done")); + assert_eq!(round_tripped_node.output.as_deref(), Some("done")); assert_eq!(round_tripped.list_node_visits("build"), vec![2]); assert_eq!( round_tripped.pending_control, @@ -955,7 +956,7 @@ mod tests { restart_failure_signatures: HashMap::new(), node_visits: HashMap::from([("build".to_string(), 2usize)]), })]; - state.stage_entry("build", 2, first_event_seq(7)).stdout = Some("done".to_string()); + state.stage_entry("build", 2, first_event_seq(7)).output = Some("done".to_string()); let round_tripped: RunProjection = serde_json::from_value(serde_json::to_value(&state).unwrap()).unwrap(); @@ -964,7 +965,7 @@ mod tests { round_tripped .stage(&StageId::new("build", 2)) .unwrap() - .stdout + .output .as_deref(), Some("done") ); @@ -1127,8 +1128,7 @@ mod tests { .unwrap(); let stage = state.stage(&stage_id).unwrap(); - assert_eq!(stage.stdout.as_deref(), Some("done")); - assert_eq!(stage.stderr.as_deref(), Some("warn")); + assert_eq!(stage.output.as_deref(), Some("done\nwarn")); assert_eq!(stage.termination, Some(CommandTermination::Exited)); assert_eq!( stage.script_timing.as_ref().unwrap()["duration_ms"], @@ -1155,8 +1155,7 @@ mod tests { .unwrap(); let stage = state.stage(&stage_id).unwrap(); - assert_eq!(stage.stdout.as_deref(), Some("partial")); - assert_eq!(stage.stderr.as_deref(), Some("cancelled")); + assert_eq!(stage.output.as_deref(), Some("partial\ncancelled")); assert_eq!(stage.termination, Some(CommandTermination::Cancelled)); assert_eq!( stage.script_timing.as_ref().unwrap()["duration_ms"], @@ -1183,8 +1182,7 @@ mod tests { .unwrap(); let stage = state.stage(&stage_id).unwrap(); - assert_eq!(stage.stdout.as_deref(), Some("partial")); - assert_eq!(stage.stderr.as_deref(), Some("timeout")); + assert_eq!(stage.output.as_deref(), Some("partial\ntimeout")); assert_eq!(stage.termination, Some(CommandTermination::TimedOut)); assert_eq!( stage.script_timing.as_ref().unwrap()["duration_ms"], diff --git a/lib/crates/fabro-store/src/serializable_projection.rs b/lib/crates/fabro-store/src/serializable_projection.rs index 7d888b4a3..d574488bc 100644 --- a/lib/crates/fabro-store/src/serializable_projection.rs +++ b/lib/crates/fabro-store/src/serializable_projection.rs @@ -14,8 +14,7 @@ impl Serialize for SerializableProjection<'_> { stage.prompt = None; stage.response = None; stage.diff = None; - stage.stdout = None; - stage.stderr = None; + stage.output = None; } projection.serialize(serializer) diff --git a/lib/crates/fabro-store/tests/serializable_projection.rs b/lib/crates/fabro-store/tests/serializable_projection.rs index 8809acda2..85565c402 100644 --- a/lib/crates/fabro-store/tests/serializable_projection.rs +++ b/lib/crates/fabro-store/tests/serializable_projection.rs @@ -118,8 +118,7 @@ fn serializable_projection_round_trips_and_trims_bulky_node_fields() { stage.parallel_results = Some(json!([{ "stage": "fanout@1" }])); stage.duration_ms = Some(1234); stage.usage = Some(sample_usage()); - stage.stdout = Some("stdout".to_string()); - stage.stderr = Some("stderr".to_string()); + stage.output = Some("output".to_string()); let serialized = serde_json::to_value(SerializableProjection(&projection)) .expect("projection should serialize"); @@ -144,8 +143,7 @@ fn serializable_projection_round_trips_and_trims_bulky_node_fields() { assert_eq!(node.prompt, None); assert_eq!(node.response, None); assert_eq!(node.diff, None); - assert_eq!(node.stdout, None); - assert_eq!(node.stderr, None); + assert_eq!(node.output, None); assert_eq!(node.first_event_seq, first_event_seq(2)); assert_eq!( node.completion diff --git a/lib/crates/fabro-types/src/run_event/misc.rs b/lib/crates/fabro-types/src/run_event/misc.rs index 5123d4c8b..11ee8acf0 100644 --- a/lib/crates/fabro-types/src/run_event/misc.rs +++ b/lib/crates/fabro-types/src/run_event/misc.rs @@ -1,4 +1,4 @@ -use serde::{Deserialize, Serialize}; +use serde::{Deserialize, Serialize, de}; use serde_json::Value; use super::ExecOutputTail; @@ -204,22 +204,104 @@ pub struct CommandStartedProps { pub timeout_ms: Option, } -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +#[derive(Debug, Clone, PartialEq, Serialize)] pub struct CommandCompletedProps { - pub stdout: String, - pub stderr: String, + pub output: String, #[serde(default, skip_serializing_if = "Option::is_none")] - pub exit_code: Option, - pub duration_ms: u64, - pub termination: CommandTermination, + pub exit_code: Option, + pub duration_ms: u64, + pub termination: CommandTermination, #[serde(default)] - pub stdout_bytes: u64, + pub output_bytes: u64, #[serde(default)] - pub stderr_bytes: u64, - #[serde(default)] - pub streams_separated: bool, - #[serde(default)] - pub live_streaming: bool, + pub live_streaming: bool, +} + +impl<'de> Deserialize<'de> for CommandCompletedProps { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + #[derive(Deserialize)] + struct Wire { + #[serde(default)] + output: Option, + #[serde(default)] + stdout: Option, + #[serde(default)] + stderr: Option, + #[serde(default)] + exit_code: Option, + duration_ms: u64, + termination: CommandTermination, + #[serde(default)] + output_bytes: Option, + #[serde(default)] + stdout_bytes: Option, + #[serde(default)] + stderr_bytes: Option, + #[serde(default)] + live_streaming: bool, + } + + let wire = Wire::deserialize(deserializer)?; + let (output, output_bytes) = if let Some(output) = wire.output { + (output, wire.output_bytes.unwrap_or(0)) + } else { + let stdout_bytes = wire.stdout_bytes.unwrap_or(0); + let stderr_bytes = wire.stderr_bytes.unwrap_or(0); + let legacy_output = if stdout_bytes == 0 && stderr_bytes > 0 && wire.stderr.is_some() { + wire.stderr + } else { + wire.stdout.or(wire.stderr) + } + .ok_or_else(|| de::Error::missing_field("output"))?; + let legacy_bytes = if stdout_bytes == 0 && stderr_bytes > 0 { + stderr_bytes + } else { + stdout_bytes + }; + (legacy_output, legacy_bytes) + }; + + Ok(Self { + output, + exit_code: wire.exit_code, + duration_ms: wire.duration_ms, + termination: wire.termination, + output_bytes, + live_streaming: wire.live_streaming, + }) + } +} + +#[cfg(test)] +mod tests { + use serde_json::json; + + use super::*; + + #[test] + fn command_completed_deserializes_legacy_stdout_stderr_shape() { + let props: CommandCompletedProps = serde_json::from_value(json!({ + "stdout": "blob://sha256/stdout", + "stderr": "blob://sha256/stderr", + "exit_code": 1, + "duration_ms": 42, + "termination": "exited", + "stdout_bytes": 0, + "stderr_bytes": 12, + "streams_separated": true, + "live_streaming": true + })) + .unwrap(); + + assert_eq!(props.output, "blob://sha256/stderr"); + assert_eq!(props.output_bytes, 12); + assert_eq!(props.exit_code, Some(1)); + assert_eq!(props.termination, CommandTermination::Exited); + assert!(props.live_streaming); + } } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] diff --git a/lib/crates/fabro-types/src/run_projection.rs b/lib/crates/fabro-types/src/run_projection.rs index 9d7bdc105..10d1d90a8 100644 --- a/lib/crates/fabro-types/src/run_projection.rs +++ b/lib/crates/fabro-types/src/run_projection.rs @@ -52,14 +52,9 @@ pub struct StageProjection { pub script_invocation: Option, pub script_timing: Option, pub parallel_results: Option, - pub stdout: Option, - pub stderr: Option, + pub output: Option, #[serde(default, skip_serializing_if = "Option::is_none")] - pub stdout_bytes: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub stderr_bytes: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] - pub streams_separated: Option, + pub output_bytes: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub live_streaming: Option, #[serde(default, skip_serializing_if = "Option::is_none")] @@ -99,11 +94,8 @@ impl StageProjection { script_invocation: None, script_timing: None, parallel_results: None, - stdout: None, - stderr: None, - stdout_bytes: None, - stderr_bytes: None, - streams_separated: None, + output: None, + output_bytes: None, live_streaming: None, termination: None, started_at: None, diff --git a/lib/crates/fabro-workflow/src/artifact.rs b/lib/crates/fabro-workflow/src/artifact.rs index 9d63604a1..c991bf78c 100644 --- a/lib/crates/fabro-workflow/src/artifact.rs +++ b/lib/crates/fabro-workflow/src/artifact.rs @@ -122,7 +122,7 @@ pub async fn resolve_context_for_edge_selection( run_store: &RunStoreHandle, ) -> Result { let mut values = context.snapshot(); - for key in [context::keys::COMMAND_OUTPUT, context::keys::COMMAND_STDERR] { + for key in [context::keys::COMMAND_OUTPUT] { if let Some(Value::String(current)) = values.get_mut(key) { *current = resolve_text_or_blob_ref_str(current, run_store).await?; } @@ -274,10 +274,7 @@ fn resolve_execution_value<'a>( Box::pin(async move { match value { Value::String(current) => { - if matches!( - key, - Some(context::keys::COMMAND_OUTPUT | context::keys::COMMAND_STDERR) - ) { + if matches!(key, Some(context::keys::COMMAND_OUTPUT)) { *current = resolve_text_or_blob_ref_str(current, run_store).await?; } else if let Some(blob_id) = parse_blob_ref(current) { *current = materialize_blob_ref(&blob_id, run_store, env, run_dir).await?; diff --git a/lib/crates/fabro-workflow/src/command_log.rs b/lib/crates/fabro-workflow/src/command_log.rs index 1365f26cc..0c6519660 100644 --- a/lib/crates/fabro-workflow/src/command_log.rs +++ b/lib/crates/fabro-workflow/src/command_log.rs @@ -3,7 +3,7 @@ use std::sync::Arc; use fabro_config::RunScratch; use fabro_store::stage_storage_segment; -use fabro_types::{CommandOutputStream, StageId, format_blob_ref}; +use fabro_types::{StageId, format_blob_ref}; use serde_json::Value; use tokio::fs::{self, File, OpenOptions}; use tokio::io::{AsyncReadExt, AsyncSeekExt, AsyncWriteExt}; @@ -14,26 +14,20 @@ use crate::runtime_store::RunStoreHandle; #[derive(Debug, Clone)] pub struct FinalizedCommandLogs { - pub stdout_ref: String, - pub stderr_ref: String, - pub stdout_bytes: u64, - pub stderr_bytes: u64, - pub stdout_text: String, - pub stderr_text: String, + pub output_ref: String, + pub output_bytes: u64, + pub output_text: String, } pub struct CommandLogRecorder { - stdout: Mutex, - stderr: Mutex, - stdout_path: PathBuf, - stderr_path: PathBuf, + output: Mutex, + output_path: PathBuf, } impl CommandLogRecorder { pub async fn create(run_dir: &Path, stage_id: &StageId) -> Result> { - let stdout_path = command_log_path(run_dir, stage_id, CommandOutputStream::Stdout); - let stderr_path = command_log_path(run_dir, stage_id, CommandOutputStream::Stderr); - if let Some(parent) = stdout_path.parent() { + let output_path = command_log_path(run_dir, stage_id); + if let Some(parent) = output_path.parent() { fs::create_dir_all(parent).await.map_err(|err| { Error::Io(format!( "creating command log directory {}: {err}", @@ -41,82 +35,59 @@ impl CommandLogRecorder { )) })?; } - let stdout = open_truncated(&stdout_path).await?; - let stderr = open_truncated(&stderr_path).await?; + let output = open_truncated(&output_path).await?; Ok(Arc::new(Self { - stdout: Mutex::new(stdout), - stderr: Mutex::new(stderr), - stdout_path, - stderr_path, + output: Mutex::new(output), + output_path, })) } - pub async fn append(&self, stream: CommandOutputStream, bytes: &[u8]) -> Result<()> { + pub async fn append(&self, bytes: &[u8]) -> Result<()> { if bytes.is_empty() { return Ok(()); } - let mut file = match stream { - CommandOutputStream::Stdout => self.stdout.lock().await, - CommandOutputStream::Stderr => self.stderr.lock().await, - }; + let mut file = self.output.lock().await; file.write_all(bytes) .await - .map_err(|err| Error::Io(format!("writing command {stream} log failed: {err}")))?; + .map_err(|err| Error::Io(format!("writing command output log failed: {err}")))?; Ok(()) } pub async fn finalize(&self, run_store: &RunStoreHandle) -> Result { self.flush_all().await?; - let (stdout_text, stdout_bytes) = read_lossy_text(&self.stdout_path).await?; - let (stderr_text, stderr_bytes) = read_lossy_text(&self.stderr_path).await?; - let stdout_ref = write_json_string_blob(run_store, &stdout_text).await?; - let stderr_ref = write_json_string_blob(run_store, &stderr_text).await?; + let (output_text, output_bytes) = read_lossy_text(&self.output_path).await?; + let output_ref = write_json_string_blob(run_store, &output_text).await?; Ok(FinalizedCommandLogs { - stdout_ref, - stderr_ref, - stdout_bytes, - stderr_bytes, - stdout_text, - stderr_text, + output_ref, + output_bytes, + output_text, }) } pub async fn discard(self: Arc) -> Result<()> { self.flush_all().await?; - let stdout_path = self.stdout_path.clone(); - let stderr_path = self.stderr_path.clone(); + let output_path = self.output_path.clone(); drop(self); - remove_if_exists(&stdout_path).await?; - remove_if_exists(&stderr_path).await + remove_if_exists(&output_path).await } async fn flush_all(&self) -> Result<()> { - self.stdout + self.output .lock() .await .flush() .await - .map_err(|err| Error::Io(format!("flushing stdout command log failed: {err}")))?; - self.stderr - .lock() - .await - .flush() - .await - .map_err(|err| Error::Io(format!("flushing stderr command log failed: {err}")))?; + .map_err(|err| Error::Io(format!("flushing command output log failed: {err}")))?; Ok(()) } } -pub fn command_log_path( - run_dir: &Path, - stage_id: &StageId, - stream: CommandOutputStream, -) -> PathBuf { +pub fn command_log_path(run_dir: &Path, stage_id: &StageId) -> PathBuf { RunScratch::new(run_dir) .runtime_dir() .join("stages") .join(stage_storage_segment(stage_id)) - .join(stream.command_log_relative_path()) + .join("output.log") } pub async fn read_log_slice( diff --git a/lib/crates/fabro-workflow/src/context.rs b/lib/crates/fabro-workflow/src/context.rs index abe31bb43..af1233586 100644 --- a/lib/crates/fabro-workflow/src/context.rs +++ b/lib/crates/fabro-workflow/src/context.rs @@ -31,7 +31,6 @@ pub mod keys { // --- command.* keys --- pub const COMMAND_OUTPUT: &str = "command.output"; - pub const COMMAND_STDERR: &str = "command.stderr"; // --- human.gate.* keys --- pub const HUMAN_GATE_SELECTED: &str = "human.gate.selected"; diff --git a/lib/crates/fabro-workflow/src/event/convert.rs b/lib/crates/fabro-workflow/src/event/convert.rs index a6deda2c5..0fb72c9aa 100644 --- a/lib/crates/fabro-workflow/src/event/convert.rs +++ b/lib/crates/fabro-workflow/src/event/convert.rs @@ -965,26 +965,20 @@ fn event_body_from_event(event: &Event) -> EventBody { timeout_ms: *timeout_ms, }), Event::CommandCompleted { - stdout, - stderr, + output, exit_code, duration_ms, termination, - stdout_bytes, - stderr_bytes, - streams_separated, + output_bytes, live_streaming, .. } => EventBody::CommandCompleted(fabro_types::CommandCompletedProps { - stdout: stdout.clone(), - stderr: stderr.clone(), - exit_code: *exit_code, - duration_ms: *duration_ms, - termination: *termination, - stdout_bytes: *stdout_bytes, - stderr_bytes: *stderr_bytes, - streams_separated: *streams_separated, - live_streaming: *live_streaming, + output: output.clone(), + exit_code: *exit_code, + duration_ms: *duration_ms, + termination: *termination, + output_bytes: *output_bytes, + live_streaming: *live_streaming, }), Event::AgentCliStarted { visit, diff --git a/lib/crates/fabro-workflow/src/event/events.rs b/lib/crates/fabro-workflow/src/event/events.rs index 5db0ae1a2..16f43f710 100644 --- a/lib/crates/fabro-workflow/src/event/events.rs +++ b/lib/crates/fabro-workflow/src/event/events.rs @@ -519,17 +519,14 @@ pub enum Event { timeout_ms: Option, }, CommandCompleted { - node_id: String, - stdout: String, - stderr: String, + node_id: String, + output: String, #[serde(default, skip_serializing_if = "Option::is_none")] - exit_code: Option, - duration_ms: u64, - termination: CommandTermination, - stdout_bytes: u64, - stderr_bytes: u64, - streams_separated: bool, - live_streaming: bool, + exit_code: Option, + duration_ms: u64, + termination: CommandTermination, + output_bytes: u64, + live_streaming: bool, }, AgentCliStarted { node_id: String, @@ -1304,8 +1301,7 @@ impl Event { exit_code, duration_ms, termination, - stdout_bytes, - stderr_bytes, + output_bytes, .. } => { debug!( @@ -1313,8 +1309,7 @@ impl Event { exit_code, duration_ms, termination = %termination, - stdout_bytes, - stderr_bytes, + output_bytes, "Command completed" ); } diff --git a/lib/crates/fabro-workflow/src/git.rs b/lib/crates/fabro-workflow/src/git.rs index b574ad662..358d00211 100644 --- a/lib/crates/fabro-workflow/src/git.rs +++ b/lib/crates/fabro-workflow/src/git.rs @@ -508,16 +508,13 @@ mod tests { .await .unwrap(); append_event(&run, &fixtures::RUN_1, &Event::CommandCompleted { - node_id: "work".into(), - stdout: "hi\n".into(), - stderr: String::new(), - exit_code: Some(0), - duration_ms: 10, - termination: CommandTermination::Exited, - stdout_bytes: 3, - stderr_bytes: 0, - streams_separated: true, - live_streaming: true, + node_id: "work".into(), + output: "hi\n".into(), + exit_code: Some(0), + duration_ms: 10, + termination: CommandTermination::Exited, + output_bytes: 3, + live_streaming: true, }) .await .unwrap(); diff --git a/lib/crates/fabro-workflow/src/handler/command.rs b/lib/crates/fabro-workflow/src/handler/command.rs index ea525b30d..cc7313208 100644 --- a/lib/crates/fabro-workflow/src/handler/command.rs +++ b/lib/crates/fabro-workflow/src/handler/command.rs @@ -49,9 +49,6 @@ impl Handler for CommandHandler { outcome .context_updates .insert(keys::COMMAND_OUTPUT.to_string(), serde_json::json!("")); - outcome - .context_updates - .insert(keys::COMMAND_STDERR.to_string(), serde_json::json!("")); Ok(outcome) } @@ -91,6 +88,7 @@ impl Handler for CommandHandler { } else { script.to_string() }; + let command = format!("exec 2>&1\n{command}"); let stage_scope = StageScope::for_handler(context, &node.id); services.run.emitter.emit_scoped( &Event::CommandStarted { @@ -114,11 +112,11 @@ impl Handler for CommandHandler { let recorder = CommandLogRecorder::create(run_dir, &stage_id).await?; let output_callback: CommandOutputCallback = { let recorder = recorder.clone(); - std::sync::Arc::new(move |stream, bytes| { + std::sync::Arc::new(move |_stream, bytes| { let recorder = recorder.clone(); Box::pin(async move { recorder - .append(stream, &bytes) + .append(&bytes) .await .map_err(|err| fabro_sandbox::Error::message(err.to_string())) }) @@ -150,29 +148,26 @@ impl Handler for CommandHandler { services.run.emitter.emit_scoped( &Event::CommandCompleted { - node_id: node.id.clone(), - stdout: finalized.stdout_ref.clone(), - stderr: finalized.stderr_ref.clone(), - exit_code: result.exit_code, - duration_ms: result.duration_ms, - termination: result.termination, - stdout_bytes: finalized.stdout_bytes, - stderr_bytes: finalized.stderr_bytes, - streams_separated: streaming.streams_separated, - live_streaming: streaming.live_streaming, + node_id: node.id.clone(), + output: finalized.output_ref.clone(), + exit_code: result.exit_code, + duration_ms: result.duration_ms, + termination: result.termination, + output_bytes: finalized.output_bytes, + live_streaming: streaming.live_streaming, }, &stage_scope, ); if result.termination == CommandTermination::TimedOut { let mut reason = format!("Script timed out after {timeout_ms}ms: {script}"); - append_output_tails(&mut reason, &finalized.stdout_text, &finalized.stderr_text); + append_output_tail(&mut reason, &finalized.output_text); return Err(Error::handler(reason)); } if result.termination == CommandTermination::Cancelled { let mut reason = format!("Script cancelled: {script}"); - append_output_tails(&mut reason, &finalized.stdout_text, &finalized.stderr_text); + append_output_tail(&mut reason, &finalized.output_text); return Err(Error::handler(reason)); } @@ -180,11 +175,7 @@ impl Handler for CommandHandler { let mut outcome = Outcome::success(); outcome.context_updates.insert( keys::COMMAND_OUTPUT.to_string(), - serde_json::json!(finalized.stdout_ref), - ); - outcome.context_updates.insert( - keys::COMMAND_STDERR.to_string(), - serde_json::json!(finalized.stderr_ref), + serde_json::json!(finalized.output_ref), ); outcome.notes = Some(format!("Script completed: {script}")); Ok(outcome) @@ -193,31 +184,22 @@ impl Handler for CommandHandler { "Script failed with exit code: {}", result.exit_code.unwrap_or(-1) ); - append_output_tails(&mut reason, &finalized.stdout_text, &finalized.stderr_text); + append_output_tail(&mut reason, &finalized.output_text); let mut outcome = Outcome::fail_classify(reason); outcome.context_updates.insert( keys::COMMAND_OUTPUT.to_string(), - serde_json::json!(finalized.stdout_ref), - ); - outcome.context_updates.insert( - keys::COMMAND_STDERR.to_string(), - serde_json::json!(finalized.stderr_ref), + serde_json::json!(finalized.output_ref), ); Ok(outcome) } } } -fn append_output_tails(reason: &mut String, stdout: &str, stderr: &str) { - let stdout_tail = tail_bytes(stdout, 4096); - let stderr_tail = tail_bytes(stderr, 4096); - if !stdout_tail.trim().is_empty() { - reason.push_str("\n\n## stdout\n"); - reason.push_str(&stdout_tail); - } - if !stderr_tail.trim().is_empty() { - reason.push_str("\n\n## stderr\n"); - reason.push_str(&stderr_tail); +fn append_output_tail(reason: &mut String, output: &str) { + let output_tail = tail_bytes(output, 4096); + if !output_tail.trim().is_empty() { + reason.push_str("\n\n## output\n"); + reason.push_str(&output_tail); } } @@ -240,7 +222,7 @@ mod tests { use bytes::Bytes; use fabro_graphviz::graph::AttrValue; use fabro_store::{Database, RunDatabase, StageId}; - use fabro_types::{CommandOutputStream, fixtures}; + use fabro_types::fixtures; use object_store::memory::InMemory; use tokio::sync::Mutex; @@ -375,10 +357,7 @@ mod tests { outcome.context_updates.get(keys::COMMAND_OUTPUT), Some(&serde_json::json!("")) ); - assert_eq!( - outcome.context_updates.get(keys::COMMAND_STDERR), - Some(&serde_json::json!("")) - ); + assert!(!outcome.context_updates.contains_key("command.stderr")); } #[tokio::test] @@ -435,8 +414,7 @@ mod tests { .await .contains("hello") ); - let command_stderr = outcome.context_updates.get(keys::COMMAND_STDERR).unwrap(); - assert_eq!(command_text(&services, command_stderr).await, ""); + assert!(!outcome.context_updates.contains_key("command.stderr")); } #[tokio::test] @@ -508,7 +486,7 @@ mod tests { let snapshot = run_store.state().await.unwrap(); let node_state = snapshot.stage(&StageId::new("script_node", 1)).unwrap(); let json = node_state.script_invocation.as_ref().unwrap(); - assert_eq!(json["command"], "echo hello"); + assert_eq!(json["command"], "exec 2>&1\necho hello"); assert_eq!(json["language"], "shell"); assert_eq!(json["timeout_ms"], serde_json::Value::Null); } @@ -539,13 +517,13 @@ mod tests { let snapshot = run_store.state().await.unwrap(); let node_state = snapshot.stage(&StageId::new("script_node", 1)).unwrap(); let json = node_state.script_invocation.as_ref().unwrap(); - assert_eq!(json["command"], "echo hello"); + assert_eq!(json["command"], "exec 2>&1\necho hello"); assert_eq!(json["language"], "shell"); assert_eq!(json["timeout_ms"], 5000); } #[tokio::test] - async fn writes_stdout_and_stderr_logs() { + async fn writes_output_log() { let handler = CommandHandler; let mut node = Node::new("script_node"); node.attrs.insert( @@ -565,18 +543,14 @@ mod tests { let snapshot = run_store.state().await.unwrap(); let node_state = snapshot.stage(&StageId::new("script_node", 1)).unwrap(); - let stdout = node_state.stdout.as_deref().unwrap(); - assert_eq!(command_log_text(&services, stdout).await.trim(), "hello"); - let stderr = node_state.stderr.as_deref().unwrap(); - assert_eq!(command_log_text(&services, stderr).await, ""); - assert_eq!(node_state.stdout_bytes, Some(6)); - assert_eq!(node_state.stderr_bytes, Some(0)); - assert_eq!(node_state.streams_separated, Some(true)); + let output = node_state.output.as_deref().unwrap(); + assert_eq!(command_log_text(&services, output).await.trim(), "hello"); + assert_eq!(node_state.output_bytes, Some(6)); assert_eq!(node_state.live_streaming, Some(true)); } #[tokio::test] - async fn writes_stderr_log_on_failure() { + async fn writes_stderr_to_output_log_on_failure() { let handler = CommandHandler; let mut node = Node::new("script_node"); node.attrs.insert( @@ -596,8 +570,8 @@ mod tests { let snapshot = run_store.state().await.unwrap(); let node_state = snapshot.stage(&StageId::new("script_node", 1)).unwrap(); - let stderr = node_state.stderr.as_deref().unwrap(); - assert_eq!(command_log_text(&services, stderr).await.trim(), "oops"); + let output = node_state.output.as_deref().unwrap(); + assert_eq!(command_log_text(&services, output).await.trim(), "oops"); } #[tokio::test] @@ -824,7 +798,7 @@ mod tests { } #[tokio::test] - async fn script_handler_captures_stderr() { + async fn script_handler_merges_stderr_into_output() { let handler = CommandHandler; let mut node = Node::new("script_node"); node.attrs.insert( @@ -841,13 +815,13 @@ mod tests { .await .unwrap(); assert_eq!(outcome.status, StageOutcome::Succeeded); - let command_stderr = outcome.context_updates.get(keys::COMMAND_STDERR).unwrap(); + let command_output = outcome.context_updates.get(keys::COMMAND_OUTPUT).unwrap(); assert!( - command_text(&services, command_stderr) + command_text(&services, command_output) .await .contains("err"), - "command.stderr should contain 'err', got: {:?}", - command_stderr + "command.output should contain 'err', got: {:?}", + command_output ); } @@ -1034,8 +1008,8 @@ mod tests { ); assert_eq!( spy.captured_command().as_deref(), - Some("echo hello"), - "sandbox should receive the script as the command" + Some("exec 2>&1\necho hello"), + "sandbox should receive the wrapped script as the command" ); } @@ -1077,7 +1051,7 @@ mod tests { assert_eq!(outcome.status, StageOutcome::Succeeded); let captured = spy.captured_command().unwrap(); assert!( - captured.starts_with("python3 -c ") && captured.contains("print"), + captured.starts_with("exec 2>&1\npython3 -c ") && captured.contains("print"), "sandbox command should invoke python3 with the script, got: {captured}" ); } @@ -1238,11 +1212,11 @@ mod tests { assert!(message.contains("timed out"), "got: {message}"); assert!( message.contains("partial stdout"), - "timeout error should include stdout tail, got: {message}" + "timeout error should include output tail, got: {message}" ); assert!( message.contains("partial stderr"), - "timeout error should include stderr tail, got: {message}" + "timeout error should include merged output tail, got: {message}" ); } @@ -1271,7 +1245,7 @@ mod tests { } #[tokio::test] - async fn script_handler_failure_includes_stdout() { + async fn script_handler_failure_includes_output() { let handler = CommandHandler; let mut node = Node::new("script_node"); node.attrs.insert( @@ -1293,11 +1267,11 @@ mod tests { let reason = outcome.failure_reason().unwrap(); assert!( reason.contains("build output"), - "failure_reason should contain stdout, got: {reason}" + "failure_reason should contain output, got: {reason}" ); assert!( reason.contains("oops"), - "failure_reason should contain stderr, got: {reason}" + "failure_reason should contain merged stderr, got: {reason}" ); assert!( reason.contains("exit code: 1"), @@ -1326,12 +1300,8 @@ mod tests { assert!(err.to_string().contains("Failed to spawn script")); let stage_id = StageId::new("script_node", 1); assert!( - !command_log_path(run_dir.path(), &stage_id, CommandOutputStream::Stdout).exists(), - "spawn failure should remove pre-created stdout scratch log" - ); - assert!( - !command_log_path(run_dir.path(), &stage_id, CommandOutputStream::Stderr).exists(), - "spawn failure should remove pre-created stderr scratch log" + !command_log_path(run_dir.path(), &stage_id).exists(), + "spawn failure should remove pre-created output scratch log" ); } @@ -1363,7 +1333,7 @@ mod tests { command_text(&services, command_output) .await .contains("build output"), - "command.output should contain stdout, got: {command_output:?}" + "command.output should contain output, got: {command_output:?}" ); } } diff --git a/lib/crates/fabro-workflow/src/handler/llm/preamble.rs b/lib/crates/fabro-workflow/src/handler/llm/preamble.rs index 9eb0848b3..4777e150b 100644 --- a/lib/crates/fabro-workflow/src/handler/llm/preamble.rs +++ b/lib/crates/fabro-workflow/src/handler/llm/preamble.rs @@ -151,7 +151,6 @@ fn tail_lines(text: &str, max_lines: usize, indent: &str) -> String { fn stage_rendered_keys(node_id: &str, outcome: &Outcome) -> HashSet { let candidates = [ keys::COMMAND_OUTPUT.to_string(), - keys::COMMAND_STDERR.to_string(), keys::LAST_STAGE.to_string(), keys::LAST_RESPONSE.to_string(), keys::response_key(node_id), @@ -182,25 +181,14 @@ fn render_compact_stage_details( lines.push(format!(" - Script: `{cmd}`")); } } - if let Some(stdout_val) = outcome.context_updates.get(keys::COMMAND_OUTPUT) { - let stdout = format_value(stdout_val); - if stdout.trim().is_empty() { - lines.push(" - Stdout: (empty)".to_string()); + if let Some(output_val) = outcome.context_updates.get(keys::COMMAND_OUTPUT) { + let output = format_value(output_val); + if output.trim().is_empty() { + lines.push(" - Output: (empty)".to_string()); } else { - lines.push(" - Stdout:".to_string()); + lines.push(" - Output:".to_string()); lines.push(" ```".to_string()); - lines.push(tail_lines(stdout.trim(), COMPACT_OUTPUT_MAX_LINES, " ")); - lines.push(" ```".to_string()); - } - } - if let Some(stderr_val) = outcome.context_updates.get(keys::COMMAND_STDERR) { - let stderr = format_value(stderr_val); - if stderr.trim().is_empty() { - lines.push(" - Stderr: (empty)".to_string()); - } else { - lines.push(" - Stderr:".to_string()); - lines.push(" ```".to_string()); - lines.push(tail_lines(stderr.trim(), COMPACT_OUTPUT_MAX_LINES, " ")); + lines.push(tail_lines(output.trim(), COMPACT_OUTPUT_MAX_LINES, " ")); lines.push(" ```".to_string()); } } @@ -254,37 +242,18 @@ fn render_summary_high_stage_section( lines.push(format!("- Script: `{cmd}`")); } } - if let Some(stdout_val) = outcome.context_updates.get(keys::COMMAND_OUTPUT) { - if let Some(path) = artifact_path(stdout_val) { - lines.push(format!("- Stdout: {}", format_artifact_reference(path))); + if let Some(output_val) = outcome.context_updates.get(keys::COMMAND_OUTPUT) { + if let Some(path) = artifact_path(output_val) { + lines.push(format!("- Output: {}", format_artifact_reference(path))); } else { - let stdout = format_value(stdout_val); - if stdout.trim().is_empty() { - lines.push("- Stdout: (empty)".to_string()); + let output = format_value(output_val); + if output.trim().is_empty() { + lines.push("- Output: (empty)".to_string()); } else { - lines.push("- Stdout:".to_string()); + lines.push("- Output:".to_string()); lines.push(" ```".to_string()); lines.push(tail_lines( - stdout.trim(), - SUMMARY_HIGH_OUTPUT_MAX_LINES, - " ", - )); - lines.push(" ```".to_string()); - } - } - } - if let Some(stderr_val) = outcome.context_updates.get(keys::COMMAND_STDERR) { - if let Some(path) = artifact_path(stderr_val) { - lines.push(format!("- Stderr: {}", format_artifact_reference(path))); - } else { - let stderr = format_value(stderr_val); - if stderr.trim().is_empty() { - lines.push("- Stderr: (empty)".to_string()); - } else { - lines.push("- Stderr:".to_string()); - lines.push(" ```".to_string()); - lines.push(tail_lines( - stderr.trim(), + output.trim(), SUMMARY_HIGH_OUTPUT_MAX_LINES, " ", )); @@ -877,7 +846,7 @@ mod tests { // --- compact handler-specific details --- #[test] - fn compact_command_stage_shows_command_stdout_stderr() { + fn compact_command_stage_shows_command_output() { let mut graph = Graph::new("test"); let mut run_tests = Node::new("run_tests"); run_tests.attrs.insert( @@ -898,9 +867,6 @@ mod tests { keys::COMMAND_OUTPUT.to_string(), serde_json::json!("10 passed\n"), ); - outcome - .context_updates - .insert(keys::COMMAND_STDERR.to_string(), serde_json::json!("")); node_outcomes.insert("run_tests".to_string(), outcome); let preamble = build_preamble( @@ -915,11 +881,11 @@ mod tests { preamble.contains("Script: `echo '10 passed'`"), "should show script command" ); - assert!(preamble.contains("Stdout:"), "should show stdout label"); - assert!(preamble.contains("10 passed"), "should show stdout content"); + assert!(preamble.contains("Output:"), "should show output label"); + assert!(preamble.contains("10 passed"), "should show output content"); assert!( - preamble.contains("Stderr: (empty)"), - "should show empty stderr" + !preamble.contains("Stderr:"), + "should not show stderr label" ); } @@ -1028,16 +994,12 @@ mod tests { // command.output is set in context (the engine copies context_updates to // context) context.set(keys::COMMAND_OUTPUT, serde_json::json!("hi\n")); - context.set(keys::COMMAND_STDERR, serde_json::json!("")); let completed_nodes = vec!["step".to_string()]; let mut node_outcomes: HashMap = HashMap::new(); let mut outcome = Outcome::success(); outcome .context_updates .insert(keys::COMMAND_OUTPUT.to_string(), serde_json::json!("hi\n")); - outcome - .context_updates - .insert(keys::COMMAND_STDERR.to_string(), serde_json::json!("")); node_outcomes.insert("step".to_string(), outcome); let preamble = build_preamble( @@ -1175,10 +1137,10 @@ mod tests { preamble.contains("Script: `cargo test`"), "should show script command" ); - // Low mode should NOT include stdout/stderr + // Low mode should NOT include output assert!( - !preamble.contains("Stdout:"), - "should not show stdout in low mode" + !preamble.contains("Output:"), + "should not show output in low mode" ); } @@ -1326,9 +1288,6 @@ mod tests { keys::COMMAND_OUTPUT.to_string(), serde_json::json!("All tests passed\n"), ); - outcome - .context_updates - .insert(keys::COMMAND_STDERR.to_string(), serde_json::json!("")); node_outcomes.insert("run_tests".to_string(), outcome); let preamble = build_preamble( @@ -1345,7 +1304,7 @@ mod tests { ); assert!( preamble.contains("All tests passed"), - "should show stdout via compact renderer" + "should show output via compact renderer" ); assert!( !preamble.contains("set command.output"), @@ -1529,11 +1488,7 @@ mod tests { let mut outcome = Outcome::success(); outcome.context_updates.insert( keys::COMMAND_OUTPUT.to_string(), - serde_json::json!("All tests passed\n"), - ); - outcome.context_updates.insert( - keys::COMMAND_STDERR.to_string(), - serde_json::json!("warning: unused var\n"), + serde_json::json!("All tests passed\nwarning: unused var\n"), ); node_outcomes.insert("run_tests".to_string(), outcome); @@ -1556,11 +1511,11 @@ mod tests { ); assert!( preamble.contains("All tests passed"), - "should include stdout" + "should include output" ); assert!( preamble.contains("warning: unused var"), - "should include stderr" + "should include merged stderr" ); } @@ -2136,7 +2091,7 @@ mod tests { } #[test] - fn compact_command_stage_truncates_long_stdout() { + fn compact_command_stage_truncates_long_output() { let mut graph = Graph::new("test"); let mut build = Node::new("build"); build.attrs.insert( @@ -2153,14 +2108,14 @@ mod tests { let completed_nodes = vec!["build".to_string()]; let mut node_outcomes: HashMap = HashMap::new(); let mut outcome = Outcome::success(); - // Generate >25 lines of stdout - let long_stdout: String = (1..=30) + // Generate >25 lines of output + let long_output: String = (1..=30) .map(|i| format!("output line {i}")) .collect::>() .join("\n"); outcome.context_updates.insert( keys::COMMAND_OUTPUT.to_string(), - serde_json::json!(long_stdout), + serde_json::json!(long_output), ); node_outcomes.insert("build".to_string(), outcome); @@ -2174,7 +2129,7 @@ mod tests { assert!( preamble.contains("(5 lines omitted)"), - "should show omission indicator for long stdout, got:\n{preamble}" + "should show omission indicator for long output, got:\n{preamble}" ); assert!( preamble.contains("output line 30"), @@ -2187,7 +2142,7 @@ mod tests { } #[test] - fn summary_high_command_stage_truncates_long_stdout() { + fn summary_high_command_stage_truncates_long_output() { let mut graph = Graph::new("test"); let mut build = Node::new("build"); build.attrs.insert( @@ -2204,14 +2159,14 @@ mod tests { let completed_nodes = vec!["build".to_string()]; let mut node_outcomes: HashMap = HashMap::new(); let mut outcome = Outcome::success(); - // Generate >50 lines of stdout - let long_stdout: String = (1..=60) + // Generate >50 lines of output + let long_output: String = (1..=60) .map(|i| format!("output line {i}")) .collect::>() .join("\n"); outcome.context_updates.insert( keys::COMMAND_OUTPUT.to_string(), - serde_json::json!(long_stdout), + serde_json::json!(long_output), ); node_outcomes.insert("build".to_string(), outcome); @@ -2225,7 +2180,7 @@ mod tests { assert!( preamble.contains("(10 lines omitted)"), - "should show omission indicator for long stdout, got:\n{preamble}" + "should show omission indicator for long output, got:\n{preamble}" ); assert!( preamble.contains("output line 60"), @@ -2238,7 +2193,7 @@ mod tests { } #[test] - fn summary_high_artifact_stdout_not_truncated() { + fn summary_high_artifact_output_not_truncated() { let mut graph = Graph::new("test"); let mut build = Node::new("build"); build.attrs.insert( @@ -2251,10 +2206,10 @@ mod tests { let completed_nodes = vec!["build".to_string()]; let mut node_outcomes: HashMap = HashMap::new(); let mut outcome = Outcome::success(); - // Artifact pointer — should NOT be truncated + // Artifact pointer should not be truncated. outcome.context_updates.insert( keys::COMMAND_OUTPUT.to_string(), - serde_json::json!("file:///tmp/artifacts/stdout.txt"), + serde_json::json!("file:///tmp/artifacts/output.txt"), ); node_outcomes.insert("build".to_string(), outcome); @@ -2271,7 +2226,7 @@ mod tests { "artifact pointers should not be truncated, got:\n{preamble}" ); assert!( - preamble.contains("/tmp/artifacts/stdout.txt"), + preamble.contains("/tmp/artifacts/output.txt"), "should show artifact path" ); } diff --git a/lib/crates/fabro-workflow/tests/it/integration.rs b/lib/crates/fabro-workflow/tests/it/integration.rs index cdffc7837..5e948f1a3 100644 --- a/lib/crates/fabro-workflow/tests/it/integration.rs +++ b/lib/crates/fabro-workflow/tests/it/integration.rs @@ -8762,8 +8762,8 @@ async fn fidelity_prompt_compact() { "compact: should show script sub-item for run_tests" ); assert!( - prompt.contains("Stdout:"), - "compact: should show stdout sub-item for run_tests" + prompt.contains("Output:"), + "compact: should show output sub-item for run_tests" ); // Original prompt at the end @@ -8833,8 +8833,8 @@ async fn fidelity_prompt_summary_medium() { "summary:medium: should show script sub-item for run_tests" ); assert!( - prompt.contains("Stdout:"), - "summary:medium: should show stdout sub-item for run_tests" + prompt.contains("Output:"), + "summary:medium: should show output sub-item for run_tests" ); // Original prompt at the end diff --git a/lib/packages/fabro-api-client/src/.openapi-generator/FILES b/lib/packages/fabro-api-client/src/.openapi-generator/FILES index 564c81ccc..aee106d74 100644 --- a/lib/packages/fabro-api-client/src/.openapi-generator/FILES +++ b/lib/packages/fabro-api-client/src/.openapi-generator/FILES @@ -42,7 +42,6 @@ models/check-run.ts models/close-run-pull-request-response.ts models/code-location.ts models/command-log-response.ts -models/command-output-stream.ts models/command-termination.ts models/completion-content-part.ts models/completion-message.ts diff --git a/lib/packages/fabro-api-client/src/api/run-internals-api.ts b/lib/packages/fabro-api-client/src/api/run-internals-api.ts index 6520b0fc6..8735a190f 100644 --- a/lib/packages/fabro-api-client/src/api/run-internals-api.ts +++ b/lib/packages/fabro-api-client/src/api/run-internals-api.ts @@ -28,8 +28,6 @@ import type { ArtifactListResponse } from '../models'; // @ts-ignore import type { CommandLogResponse } from '../models'; // @ts-ignore -import type { CommandOutputStream } from '../models'; -// @ts-ignore import type { ErrorResponse } from '../models'; // @ts-ignore import type { PaginatedEventList } from '../models'; @@ -185,27 +183,23 @@ export const RunInternalsApiAxiosParamCreator = function (configuration?: Config }; }, /** - * Returns a byte-offset slice of a command stage stdout or stderr log. Bytes are base64-encoded and are not snapped to UTF-8 boundaries. + * Returns a byte-offset slice of a command stage output log. Bytes are base64-encoded and are not snapped to UTF-8 boundaries. * @summary Tail Command Log * @param {string} id Unique run identifier (ULID). * @param {string} stageId Identifier of a stage within a run\'s workflow graph, serialized as `node_id@visit`. - * @param {CommandOutputStream} stream Command output stream to read. * @param {number} [offset] Byte offset to start reading from. Defaults to `0`. * @param {number} [limit] Maximum bytes to return. Defaults to 65536 and is capped at 1048576. * @param {*} [options] Override http request option. * @throws {RequiredError} */ - getRunStageCommandLog: async (id: string, stageId: string, stream: CommandOutputStream, offset?: number, limit?: number, options: RawAxiosRequestConfig = {}): Promise => { + getRunStageCommandLog: async (id: string, stageId: string, offset?: number, limit?: number, options: RawAxiosRequestConfig = {}): Promise => { // verify required parameter 'id' is not null or undefined assertParamExists('getRunStageCommandLog', 'id', id) // verify required parameter 'stageId' is not null or undefined assertParamExists('getRunStageCommandLog', 'stageId', stageId) - // verify required parameter 'stream' is not null or undefined - assertParamExists('getRunStageCommandLog', 'stream', stream) - const localVarPath = `/api/v1/runs/{id}/stages/{stageId}/logs/{stream}` + const localVarPath = `/api/v1/runs/{id}/stages/{stageId}/logs/output` .replace(`{${"id"}}`, encodeURIComponent(String(id))) - .replace(`{${"stageId"}}`, encodeURIComponent(String(stageId))) - .replace(`{${"stream"}}`, encodeURIComponent(String(stream))); + .replace(`{${"stageId"}}`, encodeURIComponent(String(stageId))); // use dummy base URL string because the URL constructor only accepts absolute URLs. const localVarUrlObj = new URL(localVarPath, DUMMY_BASE_URL); let baseOptions; @@ -859,18 +853,17 @@ export const RunInternalsApiFp = function(configuration?: Configuration) { return (axios, basePath) => createRequestFunction(localVarAxiosArgs, globalAxios, BASE_PATH, configuration)(axios, localVarOperationServerBasePath || basePath); }, /** - * Returns a byte-offset slice of a command stage stdout or stderr log. Bytes are base64-encoded and are not snapped to UTF-8 boundaries. + * Returns a byte-offset slice of a command stage output log. Bytes are base64-encoded and are not snapped to UTF-8 boundaries. * @summary Tail Command Log * @param {string} id Unique run identifier (ULID). * @param {string} stageId Identifier of a stage within a run\'s workflow graph, serialized as `node_id@visit`. - * @param {CommandOutputStream} stream Command output stream to read. * @param {number} [offset] Byte offset to start reading from. Defaults to `0`. * @param {number} [limit] Maximum bytes to return. Defaults to 65536 and is capped at 1048576. * @param {*} [options] Override http request option. * @throws {RequiredError} */ - async getRunStageCommandLog(id: string, stageId: string, stream: CommandOutputStream, offset?: number, limit?: number, options?: RawAxiosRequestConfig): Promise<(axios?: AxiosInstance, basePath?: string) => AxiosPromise> { - const localVarAxiosArgs = await localVarAxiosParamCreator.getRunStageCommandLog(id, stageId, stream, offset, limit, options); + async getRunStageCommandLog(id: string, stageId: string, offset?: number, limit?: number, options?: RawAxiosRequestConfig): Promise<(axios?: AxiosInstance, basePath?: string) => AxiosPromise> { + const localVarAxiosArgs = await localVarAxiosParamCreator.getRunStageCommandLog(id, stageId, offset, limit, options); const localVarOperationServerIndex = configuration?.serverIndex ?? 0; const localVarOperationServerBasePath = operationServerMap['RunInternalsApi.getRunStageCommandLog']?.[localVarOperationServerIndex]?.url; return (axios, basePath) => createRequestFunction(localVarAxiosArgs, globalAxios, BASE_PATH, configuration)(axios, localVarOperationServerBasePath || basePath); @@ -1090,18 +1083,17 @@ export const RunInternalsApiFactory = function (configuration?: Configuration, b return localVarFp.getRunLogs(id, options).then((request) => request(axios, basePath)); }, /** - * Returns a byte-offset slice of a command stage stdout or stderr log. Bytes are base64-encoded and are not snapped to UTF-8 boundaries. + * Returns a byte-offset slice of a command stage output log. Bytes are base64-encoded and are not snapped to UTF-8 boundaries. * @summary Tail Command Log * @param {string} id Unique run identifier (ULID). * @param {string} stageId Identifier of a stage within a run\'s workflow graph, serialized as `node_id@visit`. - * @param {CommandOutputStream} stream Command output stream to read. * @param {number} [offset] Byte offset to start reading from. Defaults to `0`. * @param {number} [limit] Maximum bytes to return. Defaults to 65536 and is capped at 1048576. * @param {*} [options] Override http request option. * @throws {RequiredError} */ - getRunStageCommandLog(id: string, stageId: string, stream: CommandOutputStream, offset?: number, limit?: number, options?: RawAxiosRequestConfig): AxiosPromise { - return localVarFp.getRunStageCommandLog(id, stageId, stream, offset, limit, options).then((request) => request(axios, basePath)); + getRunStageCommandLog(id: string, stageId: string, offset?: number, limit?: number, options?: RawAxiosRequestConfig): AxiosPromise { + return localVarFp.getRunStageCommandLog(id, stageId, offset, limit, options).then((request) => request(axios, basePath)); }, /** * Returns the internal event-sourced run projection. This is not a stable public contract. @@ -1283,18 +1275,17 @@ export class RunInternalsApi extends BaseAPI { } /** - * Returns a byte-offset slice of a command stage stdout or stderr log. Bytes are base64-encoded and are not snapped to UTF-8 boundaries. + * Returns a byte-offset slice of a command stage output log. Bytes are base64-encoded and are not snapped to UTF-8 boundaries. * @summary Tail Command Log * @param {string} id Unique run identifier (ULID). * @param {string} stageId Identifier of a stage within a run\'s workflow graph, serialized as `node_id@visit`. - * @param {CommandOutputStream} stream Command output stream to read. * @param {number} [offset] Byte offset to start reading from. Defaults to `0`. * @param {number} [limit] Maximum bytes to return. Defaults to 65536 and is capped at 1048576. * @param {*} [options] Override http request option. * @throws {RequiredError} */ - public getRunStageCommandLog(id: string, stageId: string, stream: CommandOutputStream, offset?: number, limit?: number, options?: RawAxiosRequestConfig) { - return RunInternalsApiFp(this.configuration).getRunStageCommandLog(id, stageId, stream, offset, limit, options).then((request) => request(this.axios, this.basePath)); + public getRunStageCommandLog(id: string, stageId: string, offset?: number, limit?: number, options?: RawAxiosRequestConfig) { + return RunInternalsApiFp(this.configuration).getRunStageCommandLog(id, stageId, offset, limit, options).then((request) => request(this.axios, this.basePath)); } /** diff --git a/lib/packages/fabro-api-client/src/models/command-log-response.ts b/lib/packages/fabro-api-client/src/models/command-log-response.ts index b6ad7b365..0134ca6a2 100644 --- a/lib/packages/fabro-api-client/src/models/command-log-response.ts +++ b/lib/packages/fabro-api-client/src/models/command-log-response.ts @@ -13,15 +13,11 @@ */ -// May contain unused imports in some cases -// @ts-ignore -import type { CommandOutputStream } from './command-output-stream'; /** * Byte-offset command log slice. */ export interface CommandLogResponse { - 'stream': CommandOutputStream; /** * Actual byte offset used for this slice. */ @@ -31,7 +27,7 @@ export interface CommandLogResponse { */ 'next_offset': number; /** - * Total bytes currently available for the stream. + * Total bytes currently available for the output log. */ 'total_bytes': number; /** @@ -39,7 +35,7 @@ export interface CommandLogResponse { */ 'bytes_base64': string; /** - * Whether the stream is finalized. + * Whether the output log is finalized. */ 'eof': boolean; 'cas_ref': string | null; @@ -49,5 +45,3 @@ export interface CommandLogResponse { 'live_streaming': boolean; } - - diff --git a/lib/packages/fabro-api-client/src/models/command-output-stream.ts b/lib/packages/fabro-api-client/src/models/command-output-stream.ts deleted file mode 100644 index 8fb234b4e..000000000 --- a/lib/packages/fabro-api-client/src/models/command-output-stream.ts +++ /dev/null @@ -1,29 +0,0 @@ -/* tslint:disable */ -/* eslint-disable */ -/** - * Fabro Run API - * HTTP API for managing Fabro workflow run executions. - * - * The version of the OpenAPI document: 0.1.0 - * - * - * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). - * https://openapi-generator.tech - * Do not edit the class manually. - */ - - - -/** - * Command output stream name. - */ - -export const CommandOutputStream = { - STDOUT: 'stdout', - STDERR: 'stderr' -} as const; - -export type CommandOutputStream = typeof CommandOutputStream[keyof typeof CommandOutputStream]; - - - diff --git a/lib/packages/fabro-api-client/src/models/index.ts b/lib/packages/fabro-api-client/src/models/index.ts index 361a4f7ec..194da2b0b 100644 --- a/lib/packages/fabro-api-client/src/models/index.ts +++ b/lib/packages/fabro-api-client/src/models/index.ts @@ -22,7 +22,6 @@ export * from './check-run-status'; export * from './close-run-pull-request-response'; export * from './code-location'; export * from './command-log-response'; -export * from './command-output-stream'; export * from './command-termination'; export * from './completion-content-part'; export * from './completion-message'; diff --git a/lib/packages/fabro-api-client/src/models/stage-projection.ts b/lib/packages/fabro-api-client/src/models/stage-projection.ts index dcba95d60..2aae825f7 100644 --- a/lib/packages/fabro-api-client/src/models/stage-projection.ts +++ b/lib/packages/fabro-api-client/src/models/stage-projection.ts @@ -48,11 +48,8 @@ export interface StageProjection { * Per-branch result objects produced by a parallel stage. */ 'parallel_results'?: Array | null; - 'stdout'?: string | null; - 'stderr'?: string | null; - 'stdout_bytes'?: number | null; - 'stderr_bytes'?: number | null; - 'streams_separated'?: boolean | null; + 'output'?: string | null; + 'output_bytes'?: number | null; 'live_streaming'?: boolean | null; 'termination'?: CommandTermination | null; /** From 6290b5a3f8fb23f3491d32862c7f16acc743fa46 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Thu, 7 May 2026 22:11:27 -0700 Subject: [PATCH 12/63] style(web): drop redundant tool labels in tool group details panel Co-Authored-By: Claude Opus 4.7 (1M context) --- apps/fabro-web/app/routes/run-stages.tsx | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/apps/fabro-web/app/routes/run-stages.tsx b/apps/fabro-web/app/routes/run-stages.tsx index efd1c7d36..d3983350b 100644 --- a/apps/fabro-web/app/routes/run-stages.tsx +++ b/apps/fabro-web/app/routes/run-stages.tsx @@ -993,15 +993,10 @@ function ToolGroupChildRow({ type="button" onClick={onToggle} aria-expanded={expanded} - className={`grid w-full grid-cols-[auto_1fr_auto_auto] items-center gap-3 px-5 py-2.5 text-left transition-colors hover:bg-overlay focus-visible:outline-2 focus-visible:-outline-offset-2 focus-visible:outline-teal-500 ${ + className={`grid w-full grid-cols-[1fr_auto_auto] items-center gap-3 px-5 py-2.5 text-left transition-colors hover:bg-overlay focus-visible:outline-2 focus-visible:-outline-offset-2 focus-visible:outline-teal-500 ${ expanded ? "bg-overlay" : "" }`} > - - {humanizeToolName(turn.toolName)} - {toolInputPreview(turn)} @@ -1035,11 +1030,6 @@ function ToolGroupDetails({ return (
    - - Tool - {humanizeToolName(group.toolName)}{" "} x{group.children.length} From 49c1263c8a5deac30403431f7ed247943d01c289 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Thu, 7 May 2026 22:16:10 -0700 Subject: [PATCH 13/63] feat(web): sync runs page filters and view to URL params Search query, repo, workflow, created-time, archived toggle, and columns/list view are now read from and written to URL search params, so the selected state survives reloads and is shareable. Defaults are omitted from the URL to keep links clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- apps/fabro-web/app/routes/runs.tsx | 60 ++++++++++++++++++++++++++---- 1 file changed, 52 insertions(+), 8 deletions(-) diff --git a/apps/fabro-web/app/routes/runs.tsx b/apps/fabro-web/app/routes/runs.tsx index 54febf2aa..4dca2c7a9 100644 --- a/apps/fabro-web/app/routes/runs.tsx +++ b/apps/fabro-web/app/routes/runs.tsx @@ -1,5 +1,5 @@ import { useState, useCallback, useEffect, useMemo, useRef } from "react"; -import { Link } from "react-router"; +import { Link, useSearchParams } from "react-router"; import { ArchiveBoxIcon, ChevronDownIcon, CommandLineIcon, MagnifyingGlassIcon } from "@heroicons/react/24/outline"; import { EllipsisVerticalIcon } from "@heroicons/react/20/solid"; import { Menu, MenuButton, MenuItem, MenuItems } from "@headlessui/react"; @@ -595,6 +595,23 @@ const createdFilterOptions: { value: CreatedFilter; label: string }[] = [ { value: "30d", label: "Last 30 days" }, ]; +function parseCreatedFilter(raw: string | null): CreatedFilter { + switch (raw) { + case "today": + case "1h": + case "1d": + case "7d": + case "30d": + return raw; + default: + return "all"; + } +} + +function parseView(raw: string | null): ViewMode { + return raw === "list" ? "list" : "columns"; +} + function createdCutoffMsFor(filter: CreatedFilter): number | null { const now = Date.now(); switch (filter) { @@ -796,7 +813,39 @@ function RunsLandingEmpty({ } export default function Runs() { - const [includeArchived, setIncludeArchived] = useState(false); + const [searchParams, setSearchParams] = useSearchParams(); + const query = searchParams.get("search") ?? ""; + const repoFilter = searchParams.get("repo") ?? "all"; + const workflowFilter = searchParams.get("workflow") ?? "all"; + const createdFilter = parseCreatedFilter(searchParams.get("created")); + const includeArchived = searchParams.get("archived") === "1"; + const view = parseView(searchParams.get("view")); + + const updateParam = useCallback( + (key: string, value: string | null) => { + setSearchParams( + (prev) => { + const next = new URLSearchParams(prev); + if (value == null || value === "") { + next.delete(key); + } else { + next.set(key, value); + } + return next; + }, + { replace: true }, + ); + }, + [setSearchParams], + ); + + const setQuery = (value: string) => updateParam("search", value || null); + const setRepoFilter = (value: string) => updateParam("repo", value === "all" ? null : value); + const setWorkflowFilter = (value: string) => updateParam("workflow", value === "all" ? null : value); + const setCreatedFilter = (value: CreatedFilter) => updateParam("created", value === "all" ? null : value); + const setIncludeArchived = (value: boolean) => updateParam("archived", value ? "1" : null); + const setView = (value: ViewMode) => updateParam("view", value === "columns" ? null : value); + const boardRuns = useBoardsRuns(includeArchived); const authConfig = useAuthConfig(); const systemInfo = useSystemInfo(); @@ -823,11 +872,6 @@ export default function Runs() { initialColumns.flatMap((col: Column) => col.items.map((item: RunItem) => String(item.workflow))), ), ].sort(); - const [query, setQuery] = useState(""); - const [repoFilter, setRepoFilter] = useState("all"); - const [workflowFilter, setWorkflowFilter] = useState("all"); - const [createdFilter, setCreatedFilter] = useState("all"); - const [view, setView] = useState("columns"); const [columns, setColumns] = useState(initialColumns); const lowerQuery = query.toLowerCase(); useBoardEvents(); @@ -943,7 +987,7 @@ export default function Runs() {