From b5bb134890fe4321e3e116afc611de753bf5bb7d Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sun, 19 Apr 2026 11:35:30 -0400 Subject: [PATCH] fix(runs): bound board enrichment and demo normalization Paginate board-eligible summaries before enriching them from run state, add safety caps to paginated web fetches, and make demo run summaries follow the production title and status-reason normalization rules. --- apps/fabro-web/app/api.test.ts | 38 ++++++++++ apps/fabro-web/app/api.ts | 23 +++++- lib/crates/fabro-server/src/demo/mod.rs | 93 +++++++++++++++++++++++-- lib/crates/fabro-server/src/server.rs | 80 ++++++++++++++++++--- 4 files changed, 215 insertions(+), 19 deletions(-) diff --git a/apps/fabro-web/app/api.test.ts b/apps/fabro-web/app/api.test.ts index 5201fbcc7..52824a779 100644 --- a/apps/fabro-web/app/api.test.ts +++ b/apps/fabro-web/app/api.test.ts @@ -122,4 +122,42 @@ describe("apiPaginatedJson", () => { }, ); }); + + test("stops after a bounded number of pages when the server keeps advertising more data", async () => { + const warnMock = mock(() => {}); + const originalWarn = console.warn; + console.warn = warnMock; + + let callCount = 0; + const fetchMock = mock(() => { + callCount += 1; + if (callCount > 50) { + throw new Error("apiPaginatedJson should have stopped at the page cap"); + } + + return Promise.resolve( + new Response( + JSON.stringify({ + data: [{ id: `run-${callCount}` }], + meta: { has_more: true }, + }), + { + status: 200, + headers: { "Content-Type": "application/json" }, + }, + ), + ); + }); + globalThis.fetch = fetchMock as typeof fetch; + + try { + const result = await apiPaginatedJson<{ id: string }>("/boards/runs"); + + expect(result.data).toHaveLength(50); + expect(result.meta).toEqual({ has_more: true }); + expect(warnMock).toHaveBeenCalledTimes(1); + } finally { + console.warn = originalWarn; + } + }); }); diff --git a/apps/fabro-web/app/api.ts b/apps/fabro-web/app/api.ts index da43e2c09..96aa1d72c 100644 --- a/apps/fabro-web/app/api.ts +++ b/apps/fabro-web/app/api.ts @@ -8,6 +8,9 @@ export interface PaginatedEnvelope { meta: { has_more: boolean }; } +const PAGINATED_API_MAX_PAGES = 50; +const PAGINATED_API_MAX_ITEMS = 5000; + function buildApiPath(path: string): string { return `/api/v1${path}`; } @@ -51,6 +54,7 @@ export async function apiPaginatedJson( let offset = 0; const data: TItem[] = []; let extras: TExtra | null = null; + let pagesLoaded = 0; while (true) { const response = await fetch(buildPaginatedApiPath(path, limit, offset), { @@ -74,7 +78,10 @@ export async function apiPaginatedJson( extras = rest as TExtra; } - data.push(...page.data); + pagesLoaded += 1; + const remainingItemBudget = PAGINATED_API_MAX_ITEMS - data.length; + const pageItems = remainingItemBudget > 0 ? page.data.slice(0, remainingItemBudget) : []; + data.push(...pageItems); if (!page.meta.has_more || page.data.length === 0) { return { ...(extras ?? ({} as TExtra)), @@ -82,6 +89,20 @@ export async function apiPaginatedJson( meta: { has_more: false }, }; } + if ( + pagesLoaded >= PAGINATED_API_MAX_PAGES || + pageItems.length < page.data.length || + data.length >= PAGINATED_API_MAX_ITEMS + ) { + console.warn( + `Stopped paginated API fetch for ${path} after ${pagesLoaded} pages and ${data.length} items because the safety cap was reached.`, + ); + return { + ...(extras ?? ({} as TExtra)), + data, + meta: { has_more: true }, + }; + } offset += page.data.length; } diff --git a/lib/crates/fabro-server/src/demo/mod.rs b/lib/crates/fabro-server/src/demo/mod.rs index 65fa4f781..5bd058a0d 100644 --- a/lib/crates/fabro-server/src/demo/mod.rs +++ b/lib/crates/fabro-server/src/demo/mod.rs @@ -647,6 +647,7 @@ mod runs { use fabro_api::types::*; use super::ts; + use crate::server::truncate_goal; fn labels(entries: &[(&str, &str)]) -> HashMap { entries @@ -669,12 +670,7 @@ mod runs { total_usd_micros: Option, entries: &[(&str, &str)], ) -> StoreRunSummary { - let status_reason = match status_reason { - Some("completed") => Some(StatusReason::Completed), - Some("workflow_error") => Some(StatusReason::WorkflowError), - Some(other) => panic!("unsupported demo status_reason: {other}"), - None => None, - }; + let status_reason = status_reason.and_then(parse_status_reason); StoreRunSummary { created_at: ts(created_at), @@ -691,13 +687,30 @@ mod runs { start_time: Some(ts(created_at)), status: status.map(str::to_string), status_reason, - title: goal.into(), + title: truncate_goal(goal), total_usd_micros, workflow_name: Some(workflow_name.into()), workflow_slug: Some(workflow_slug.into()), } } + fn parse_status_reason(reason: &str) -> Option { + match reason { + "completed" => Some(StatusReason::Completed), + "partial_success" => Some(StatusReason::PartialSuccess), + "workflow_error" => Some(StatusReason::WorkflowError), + "cancelled" => Some(StatusReason::Cancelled), + "terminated" => Some(StatusReason::Terminated), + "transient_infra" => Some(StatusReason::TransientInfra), + "budget_exhausted" => Some(StatusReason::BudgetExhausted), + "launch_failed" => Some(StatusReason::LaunchFailed), + "bootstrap_failed" => Some(StatusReason::BootstrapFailed), + "sandbox_init_failed" => Some(StatusReason::SandboxInitFailed), + "sandbox_initializing" => Some(StatusReason::SandboxInitializing), + _ => None, + } + } + fn take_summary( summaries: &mut HashMap, run_id: &str, @@ -951,6 +964,72 @@ mod runs { ] } + #[cfg(test)] + mod tests { + use super::*; + + #[test] + fn summary_parses_known_status_reason_values() { + let summary = summary( + "run-test", + "demo-repo", + "implement", + "Implement", + "Goal", + Some("failed"), + "2026-03-06T14:30:00Z", + Some(1.0), + Some("cancelled"), + None, + None, + &[], + ); + + assert_eq!(summary.status_reason, Some(StatusReason::Cancelled)); + } + + #[test] + fn summary_ignores_unknown_status_reason() { + let summary = summary( + "run-test", + "demo-repo", + "implement", + "Implement", + "Goal", + Some("failed"), + "2026-03-06T14:30:00Z", + Some(1.0), + Some("unexpected_reason"), + None, + None, + &[], + ); + + assert_eq!(summary.status_reason, None); + } + + #[test] + fn summary_derives_title_like_server() { + let goal = format!("## Plan: {}", "a".repeat(120)); + let summary = summary( + "run-test", + "demo-repo", + "implement", + "Implement", + &goal, + Some("running"), + "2026-03-06T14:30:00Z", + Some(1.0), + None, + None, + None, + &[], + ); + + assert_eq!(summary.title, format!("{}...", "a".repeat(97))); + } + } + pub(super) fn stages() -> Vec { vec![ RunStage { diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index 603380d4b..09393a7a0 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -2579,7 +2579,7 @@ fn board_columns() -> serde_json::Value { ]) } -fn truncate_goal(goal: &str) -> String { +pub(crate) fn truncate_goal(goal: &str) -> String { const MAX_LEN: usize = 100; let stripped = strip_goal_decoration(goal); @@ -2698,23 +2698,26 @@ async fn list_board_runs( .into_response(); } }; - let mut all_items = Vec::new(); - for summary in summaries { - let Some(status) = summary.status else { - continue; - }; - let Some(column) = board_column(status) else { - continue; - }; + let board_summaries: Vec<_> = summaries + .into_iter() + .filter_map(|summary| { + let status = summary.status?; + let column = board_column(status)?; + Some((summary, column)) + }) + .collect(); + let (page_summaries, has_more) = paginate_items(board_summaries, pagination); + + let mut data = Vec::with_capacity(page_summaries.len()); + for (summary, column) in page_summaries { let run_id = summary.run_id; let mut item = summary_to_api_run_summary(summary); item["column"] = serde_json::json!(column); if let Some(object) = item.as_object_mut() { object.extend(board_run_metadata(state.as_ref(), run_id).await); } - all_items.push(item); + data.push(item); } - let (data, has_more) = paginate_items(all_items, pagination); ( StatusCode::OK, Json(serde_json::json!({ @@ -9468,6 +9471,61 @@ timeout = "30s" assert_eq!(item["question"]["text"].as_str(), Some("Ship it?")); } + #[tokio::test] + async fn boards_runs_page_limit_preserves_metadata_for_paged_items() { + let state = create_app_state(); + let app = build_router(Arc::clone(&state), AuthMode::Disabled); + + let first_run_id = create_and_start_run(&app, MINIMAL_DOT) + .await + .parse::() + .unwrap(); + let second_run_id = create_and_start_run(&app, MINIMAL_DOT) + .await + .parse::() + .unwrap(); + + for (run_id, sandbox_id) in [ + (first_run_id, "sb-first"), + (second_run_id, "sb-second"), + ] { + let run_store = state.store.open_run(&run_id).await.unwrap(); + for event in [ + workflow_event::Event::RunRunning { reason: None }, + workflow_event::Event::SandboxInitialized { + provider: "local".to_string(), + working_directory: "/sandbox/workdir".to_string(), + identifier: Some(sandbox_id.to_string()), + host_working_directory: Some("/tmp/repo".to_string()), + container_mount_point: None, + }, + ] { + workflow_event::append_event(&run_store, &run_id, &event) + .await + .unwrap(); + } + } + + let req = Request::builder() + .method("GET") + .uri(api("/boards/runs?page[limit]=1")) + .body(Body::empty()) + .unwrap(); + let response = app.oneshot(req).await.unwrap(); + assert_eq!(response.status(), StatusCode::OK); + let body = body_json(response.into_body()).await; + assert_eq!(body["meta"]["has_more"].as_bool(), Some(true)); + + let data = body["data"].as_array().expect("data should be array"); + assert_eq!(data.len(), 1); + + let item = &data[0]; + let sandbox_id = item["sandbox"]["id"] + .as_str() + .expect("paged item should still include sandbox metadata"); + assert!(matches!(sandbox_id, "sb-first" | "sb-second")); + } + #[test] fn validate_github_slug_accepts_real_names() { assert!(super::validate_github_slug("owner", "anthropic", 39).is_ok());