refactor(system): simplify repair-runs flow and rm --force

Mark SystemRepairRunsResponse and SystemRepairRunIssue fields required so
generated Rust/TS types stop forcing Some(...) wrapping on the producer
and defensive .unwrap_or("-") on consumers. Collapse the two-arm dispatch
in fabro rm --force into a single resolve_target step + shared
delete/account block, eliminating ~20 lines of duplicated error handling.
Loosen the brittle "no events" assertion to a substring check.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-05-05 19:19:34 -04:00
parent b64352dccd
commit d2e6f09780
No known key found for this signature in database
12 changed files with 56 additions and 65 deletions

View file

@ -8034,6 +8034,7 @@ components:
SystemRepairRunsResponse:
description: Runs that need manual repair or deletion because they cannot be loaded.
type: object
required: [runs, total_count]
properties:
runs:
type: array
@ -8047,6 +8048,7 @@ components:
SystemRepairRunIssue:
description: One cataloged run that cannot be loaded from durable storage.
type: object
required: [run_id, created_at, error]
properties:
run_id:
type: string

View file

@ -21,49 +21,26 @@ async fn remove_from(args: &RunsRemoveArgs, ctx: &CommandContext) -> Result<()>
let mut errors = Vec::new();
for identifier in &args.runs {
if args.force {
if let Ok(run_id) = identifier.parse::<fabro_types::RunId>() {
let run_id_string = run_id.to_string();
if let Err(err) = delete_server_run(client, &run_id, true).await {
let error = err.to_string();
if !json {
fabro_util::printerr!(printer, "error: {identifier}: {error}");
}
errors.push(serde_json::json!({
"identifier": identifier,
"error": error,
}));
had_errors = true;
continue;
}
removed.push(run_id_string.clone());
if !json {
fabro_util::printerr!(printer, "{}", short_run_id(&run_id_string));
}
continue;
}
}
let run = match client.resolve_run(identifier).await {
Ok(run) => run,
let run_id = match resolve_target(client, identifier, args.force).await {
Ok(run_id) => run_id,
Err(err) => {
let error = err.to_string();
if !json {
fabro_util::printerr!(printer, "error: {identifier}: {err}");
fabro_util::printerr!(printer, "error: {identifier}: {error}");
}
errors.push(serde_json::json!({
"identifier": identifier,
"error": err.to_string(),
"error": error,
}));
had_errors = true;
continue;
}
};
let run_id = run.run_id.to_string();
if let Err(err) = delete_server_run(client, &run.run_id, args.force).await {
if let Err(err) = delete_server_run(client, &run_id, args.force).await {
let error = err.to_string();
if !json {
if error.starts_with("cannot remove active run ") {
if !args.force && error.starts_with("cannot remove active run ") {
fabro_util::printerr!(printer, "{error}");
} else {
fabro_util::printerr!(printer, "error: {identifier}: {error}");
@ -76,9 +53,11 @@ async fn remove_from(args: &RunsRemoveArgs, ctx: &CommandContext) -> Result<()>
had_errors = true;
continue;
}
removed.push(run_id.clone());
let run_id_string = run_id.to_string();
removed.push(run_id_string.clone());
if !json {
fabro_util::printerr!(printer, "{}", short_run_id(&run_id));
fabro_util::printerr!(printer, "{}", short_run_id(&run_id_string));
}
}
@ -95,6 +74,19 @@ async fn remove_from(args: &RunsRemoveArgs, ctx: &CommandContext) -> Result<()>
Ok(())
}
async fn resolve_target(
client: &server_client::Client,
identifier: &str,
force: bool,
) -> Result<fabro_types::RunId> {
if force {
if let Ok(run_id) = identifier.parse::<fabro_types::RunId>() {
return Ok(run_id);
}
}
Ok(client.resolve_run(identifier).await?.run_id)
}
async fn delete_server_run(
client: &server_client::Client,
run_id: &fabro_types::RunId,

View file

@ -1,5 +1,4 @@
use anyhow::Result;
use chrono::{DateTime, Utc};
use fabro_api::types;
use fabro_util::printer::Printer;
@ -33,33 +32,26 @@ fn repair_runs_from(
return Ok(());
}
let runs = response.runs.as_slice();
if runs.is_empty() {
if response.runs.is_empty() {
fabro_util::printout!(printer, "No run repair issues found.");
return Ok(());
}
fabro_util::printout!(printer, "Unreadable runs:");
for run in runs {
for run in &response.runs {
fabro_util::printout!(
printer,
" {} {} {}",
run.run_id.as_deref().unwrap_or("-"),
format_created_at(run.created_at.as_ref()),
run.error.as_deref().unwrap_or("-"),
run.run_id,
run.created_at.to_rfc3339(),
run.error,
);
}
fabro_util::printout!(printer, "");
fabro_util::printout!(printer, "Delete with:");
for run in runs {
if let Some(run_id) = run.run_id.as_deref() {
fabro_util::printout!(printer, " fabro rm --force {run_id}");
}
for run in &response.runs {
fabro_util::printout!(printer, " fabro rm --force {}", run.run_id);
}
Ok(())
}
fn format_created_at(created_at: Option<&DateTime<Utc>>) -> String {
created_at.map_or_else(|| "-".to_string(), DateTime::to_rfc3339)
}

View file

@ -130,22 +130,19 @@ async fn get_system_repair_runs(
.into_response();
}
};
let total_count = issues.len();
let total_count = to_i64(issues.len());
let runs = issues
.into_iter()
.map(|issue| SystemRepairRunIssue {
run_id: Some(issue.run_id.to_string()),
created_at: Some(issue.created_at),
error: Some(issue.error),
run_id: issue.run_id.to_string(),
created_at: issue.created_at,
error: issue.error,
})
.collect();
(
StatusCode::OK,
Json(SystemRepairRunsResponse {
runs,
total_count: Some(to_i64(total_count)),
}),
Json(SystemRepairRunsResponse { runs, total_count }),
)
.into_response()
}

View file

@ -2071,7 +2071,14 @@ async fn system_repair_runs_lists_catalog_entries_without_projection() {
.parse::<chrono::DateTime<Utc>>()
.unwrap();
assert_eq!(created_at, run_id.created_at());
assert_eq!(body["runs"][0]["error"], "run has no events");
assert!(
body["runs"][0]["error"]
.as_str()
.unwrap()
.contains("no events"),
"got: {}",
body["runs"][0]["error"]
);
}
#[tokio::test]

View file

@ -230,9 +230,9 @@ models/run-git-settings.ts
models/run-goal-file.ts
models/run-goal-inline.ts
models/run-goal.ts
models/run-interviews-settings.ts
models/run-integrations-github-settings.ts
models/run-integrations-settings.ts
models/run-interviews-settings.ts
models/run-list-item.ts
models/run-manifest.ts
models/run-mode.ts

View file

@ -209,9 +209,9 @@ export * from './run-git-settings';
export * from './run-goal';
export * from './run-goal-file';
export * from './run-goal-inline';
export * from './run-interviews-settings';
export * from './run-integrations-github-settings';
export * from './run-integrations-settings';
export * from './run-interviews-settings';
export * from './run-list-item';
export * from './run-manifest';
export * from './run-mode';

View file

@ -18,4 +18,3 @@ export interface RunIntegrationsGithubSettings {
'permissions': { [key: string]: string; };
}

View file

@ -21,4 +21,3 @@ export interface RunIntegrationsSettings {
'github': RunIntegrationsGithubSettings;
}

View file

@ -47,3 +47,6 @@ export interface RunStage {
*/
'started_at'?: string | null;
}

View file

@ -21,14 +21,14 @@ export interface SystemRepairRunIssue {
/**
* Run identifier.
*/
'run_id'?: string;
'run_id': string;
/**
* Timestamp encoded in the run identifier.
*/
'created_at'?: string;
'created_at': string;
/**
* Error produced while loading the run projection.
*/
'error'?: string;
'error': string;
}

View file

@ -21,10 +21,10 @@ import type { SystemRepairRunIssue } from './system-repair-run-issue';
* Runs that need manual repair or deletion because they cannot be loaded.
*/
export interface SystemRepairRunsResponse {
'runs'?: Array<SystemRepairRunIssue>;
'runs': Array<SystemRepairRunIssue>;
/**
* Count of run repair issues.
*/
'total_count'?: number;
'total_count': number;
}