mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-10 03:30:59 +00:00
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.
This commit is contained in:
parent
6226858648
commit
b5bb134890
4 changed files with 215 additions and 19 deletions
|
|
@ -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;
|
||||
}
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -8,6 +8,9 @@ export interface PaginatedEnvelope<T> {
|
|||
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<TItem, TExtra extends object = {}>(
|
|||
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<TItem, TExtra extends object = {}>(
|
|||
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<TItem, TExtra extends object = {}>(
|
|||
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;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -647,6 +647,7 @@ mod runs {
|
|||
use fabro_api::types::*;
|
||||
|
||||
use super::ts;
|
||||
use crate::server::truncate_goal;
|
||||
|
||||
fn labels(entries: &[(&str, &str)]) -> HashMap<String, String> {
|
||||
entries
|
||||
|
|
@ -669,12 +670,7 @@ mod runs {
|
|||
total_usd_micros: Option<i64>,
|
||||
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<StatusReason> {
|
||||
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<String, StoreRunSummary>,
|
||||
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<RunStage> {
|
||||
vec![
|
||||
RunStage {
|
||||
|
|
|
|||
|
|
@ -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::<RunId>()
|
||||
.unwrap();
|
||||
let second_run_id = create_and_start_run(&app, MINIMAL_DOT)
|
||||
.await
|
||||
.parse::<RunId>()
|
||||
.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());
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue