scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-09-06 08:18:58 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

fix(resume): reject succeeded runs and check PID before checkpoint parse

Browse source 
1. Succeeded runs now rejected — a completed run keeps checkpoint.json
   around, so resume would happily restart and overwrite start.json and
   conclusion.json. Now checks status.json and bails on Succeeded.

2. PID liveness check moved before checkpoint validation. The engine
   writes checkpoint.json with a plain fs::write, so a concurrent
   resume could see a half-written file and report "corrupt" for a
   run that is simply still alive. Order is now: PID → status →
   checkpoint parse → cleanup → spawn.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-03-26 13:19:41 -04:00
parent 4c6b846fb9
commit 75f2148f43
No known key found for this signature in database
1 changed files with 17 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

23
lib/crates/fabro-cli/src/commands/resume.rs
View file

@ -2,7 +2,7 @@ use anyhow::bail;
use clap::Args;
use fabro_util::terminal::Styles;
use fabro_workflows::records::{Checkpoint, RunRecord};
use fabro_workflows::run_status::RunStatus;
use fabro_workflows::run_status::{RunStatus, RunStatusRecord};
#[derive(Debug, Args)]
pub struct ResumeArgs {
@ -29,6 +29,22 @@ pub async fn resume_command(args: ResumeArgs, styles: &'static Styles) -> anyhow
}
let run_id = RunRecord::load(&run_dir)?.run_id;
// Guard against resuming a live run — must happen before checkpoint
// validation because the engine writes checkpoint.json with a plain
// fs::write, so a mid-write read would see a truncated file and
// report "corrupt" for a run that is simply still alive.
if is_pid_alive(&run_dir.join("run.pid")) {
bail!("an engine process is still running for this run — cannot resume");
}
// Reject runs that completed successfully — only failed/interrupted
// runs should be resumed.
if let Ok(record) = RunStatusRecord::load(&run_dir.join("status.json")) {
if record.status == RunStatus::Succeeded {
bail!("run already succeeded — nothing to resume");
}
}
// Validate checkpoint is parseable before touching any state.
// A crash during the original run can leave a truncated file;
// we must not destroy the old conclusion/failure evidence and
@ -42,11 +58,6 @@ pub async fn resume_command(args: ResumeArgs, styles: &'static Styles) -> anyhow
}
})?;
// Guard against resuming a live run
if is_pid_alive(&run_dir.join("run.pid")) {
bail!("an engine process is still running for this run — cannot resume");
}
// Clean stale artifacts from previous execution
for name in &[
"conclusion.json",