diff --git a/docs/reference/run-directory.mdx b/docs/reference/run-directory.mdx index 0d681b60d..b4adaa576 100644 --- a/docs/reference/run-directory.mdx +++ b/docs/reference/run-directory.mdx @@ -22,7 +22,7 @@ The naming format is `YYYYMMDD-{run_id}`, where `run_id` is the ULID assigned to | `run.json` | JSON | Run create | Run metadata — `run_id`, `created_at`, `config` (FabroConfig), `graph` (Graph), `workflow_slug`, `working_directory`, `host_repo_path`, `base_branch`, `labels` | | `start.json` | JSON | Run start | Start metadata — `run_id`, `start_time`, `run_branch`, `base_sha` | | `workflow.fabro` | Graphviz | Run create | Copy of the original workflow graph when the raw DOT source is available | -| `run.pid` | Text | Legacy fallback only | Legacy process ID file from older runs. Current detached launches use launcher records instead, but attach/resume still read `run.pid` for backward compatibility. | +| `run.pid` | Text | Legacy only | Legacy process ID file from older runs. Current detached launches use launcher records instead, and current attach/resume no longer read `run.pid`. | | `workflow.toml` | TOML | Run create | Copy of the original workflow file (only when the workflow is defined in TOML) | | `progress.jsonl` | JSONL | Continuous | Event stream — one JSON object per line for every significant event (stage starts, completions, tool calls, retries, etc.). See [Observability](/execution/observability) for the full event catalog. | | `live.json` | JSON | Continuous | Current execution state snapshot, overwritten on each event. Used for live monitoring. | @@ -94,7 +94,7 @@ fabro ps --filter workflow=my-workflow │ ├── run.json │ ├── start.json │ ├── workflow.fabro -│ ├── run.pid # Legacy fallback; older runs may contain this +│ ├── run.pid # Legacy only; older runs may contain this │ ├── workflow.toml │ ├── progress.jsonl │ ├── live.json diff --git a/lib/crates/fabro-cli/src/commands/run/attach.rs b/lib/crates/fabro-cli/src/commands/run/attach.rs index 1b8ed5098..5bdc63a7a 100644 --- a/lib/crates/fabro-cli/src/commands/run/attach.rs +++ b/lib/crates/fabro-cli/src/commands/run/attach.rs @@ -39,7 +39,6 @@ pub(crate) async fn attach_run( let status_path = run_dir.join("status.json"); let runtime_state = RuntimeState::new(run_dir); let runtime_interview_paths = InterviewPaths::from_runtime_state(&runtime_state); - let legacy_interview_paths = InterviewPaths::from_base_dir(run_dir); let mut engine_guard = engine_child.map(EngineChildGuard::new); @@ -167,11 +166,11 @@ pub(crate) async fn attach_run( } // Check for interview request - if let Some(interview_paths) = - active_interview_paths(&runtime_interview_paths, &legacy_interview_paths) - { + if runtime_interview_paths.request_path.exists() { + let interview_paths = &runtime_interview_paths; if !interview_paths.response_path.exists() { - if let Some(_claim_guard) = InterviewClaimGuard::acquire(&interview_paths.base_dir) + if let Some(_claim_guard) = + InterviewClaimGuard::acquire(&interview_paths.claim_path) { if let Ok(request_data) = std::fs::read_to_string(&interview_paths.request_path) { @@ -283,13 +282,7 @@ fn read_status_record(path: &Path) -> Option { } fn read_launcher_pid(run_dir: &Path) -> Option { - super::launcher::active_launcher_record_for_run(run_dir) - .map(|record| record.pid) - .or_else(|| { - std::fs::read_to_string(run_dir.join("run.pid")) - .ok() - .and_then(|pid| pid.trim().parse::().ok()) - }) + super::launcher::active_launcher_record_for_run(run_dir).map(|record| record.pid) } fn progress_file_is_empty(path: &Path) -> bool { @@ -298,58 +291,33 @@ fn progress_file_is_empty(path: &Path) -> bool { .unwrap_or(true) } +#[allow(clippy::struct_field_names)] #[derive(Debug, Clone, PartialEq, Eq)] struct InterviewPaths { - base_dir: PathBuf, + claim_path: PathBuf, request_path: PathBuf, response_path: PathBuf, } impl InterviewPaths { - fn from_base_dir(base_dir: &Path) -> Self { - let base_dir = base_dir.to_path_buf(); - Self { - request_path: base_dir.join("interview_request.json"), - response_path: base_dir.join("interview_response.json"), - base_dir, - } - } - fn from_runtime_state(runtime_state: &RuntimeState) -> Self { Self { - base_dir: runtime_state.runtime_dir(), + claim_path: runtime_state.interview_claim_path(), request_path: runtime_state.interview_request_path(), response_path: runtime_state.interview_response_path(), } } } -fn active_interview_paths( - runtime_paths: &InterviewPaths, - legacy_paths: &InterviewPaths, -) -> Option { - if runtime_paths.request_path.exists() { - Some(runtime_paths.clone()) - } else if legacy_paths.request_path.exists() { - Some(legacy_paths.clone()) - } else { - None - } -} - -fn interview_claim_path(base_dir: &Path) -> PathBuf { - base_dir.join("interview_request.claim") -} - struct InterviewClaimGuard { claim_path: PathBuf, } impl InterviewClaimGuard { - fn acquire(base_dir: &Path) -> Option { - if try_claim_interview_request(base_dir) { + fn acquire(claim_path: &Path) -> Option { + if try_claim_interview_request(claim_path) { Some(Self { - claim_path: interview_claim_path(base_dir), + claim_path: claim_path.to_path_buf(), }) } else { None @@ -390,25 +358,26 @@ impl Drop for EngineChildGuard { } } -fn try_claim_interview_request(base_dir: &Path) -> bool { - if std::fs::create_dir_all(base_dir).is_err() { - return false; +fn try_claim_interview_request(claim_path: &Path) -> bool { + if let Some(parent) = claim_path.parent() { + if std::fs::create_dir_all(parent).is_err() { + return false; + } } - let claim_path = interview_claim_path(base_dir); - if let Ok(existing) = std::fs::read_to_string(&claim_path) { + if let Ok(existing) = std::fs::read_to_string(claim_path) { if let Ok(pid) = existing.trim().parse::() { if process_alive(pid) { return pid == std::process::id(); } } - let _ = std::fs::remove_file(&claim_path); + let _ = std::fs::remove_file(claim_path); } match std::fs::OpenOptions::new() .create_new(true) .write(true) - .open(&claim_path) + .open(claim_path) { Ok(mut file) => { let _ = writeln!(file, "{}", std::process::id()); @@ -559,11 +528,13 @@ mod tests { #[test] fn try_claim_interview_request_reclaims_stale_claim() { let dir = tempfile::tempdir().unwrap(); - std::fs::write(interview_claim_path(dir.path()), "999999\n").unwrap(); + let claim_path = dir.path().join("runtime").join("interview_request.claim"); + std::fs::create_dir_all(claim_path.parent().unwrap()).unwrap(); + std::fs::write(&claim_path, "999999\n").unwrap(); - assert!(try_claim_interview_request(dir.path())); + assert!(try_claim_interview_request(&claim_path)); assert_eq!( - std::fs::read_to_string(interview_claim_path(dir.path())).unwrap(), + std::fs::read_to_string(claim_path).unwrap(), format!("{}\n", std::process::id()) ); } @@ -571,45 +542,14 @@ mod tests { #[test] fn interview_claim_guard_releases_claim_on_drop() { let dir = tempfile::tempdir().unwrap(); + let claim_path = dir.path().join("runtime").join("interview_request.claim"); { - let _guard = InterviewClaimGuard::acquire(dir.path()).unwrap(); - assert!(interview_claim_path(dir.path()).exists()); + let _guard = InterviewClaimGuard::acquire(&claim_path).unwrap(); + assert!(claim_path.exists()); } - assert!(!interview_claim_path(dir.path()).exists()); - } - - #[test] - fn active_interview_paths_prefers_runtime_paths() { - let dir = tempfile::tempdir().unwrap(); - let runtime_state = RuntimeState::new(dir.path()); - let runtime_paths = InterviewPaths::from_runtime_state(&runtime_state); - let legacy_paths = InterviewPaths::from_base_dir(dir.path()); - - std::fs::create_dir_all(runtime_state.runtime_dir()).unwrap(); - std::fs::write(&runtime_paths.request_path, "{}").unwrap(); - std::fs::write(&legacy_paths.request_path, "{}").unwrap(); - - assert_eq!( - active_interview_paths(&runtime_paths, &legacy_paths), - Some(runtime_paths) - ); - } - - #[test] - fn active_interview_paths_falls_back_to_legacy_paths() { - let dir = tempfile::tempdir().unwrap(); - let runtime_state = RuntimeState::new(dir.path()); - let runtime_paths = InterviewPaths::from_runtime_state(&runtime_state); - let legacy_paths = InterviewPaths::from_base_dir(dir.path()); - - std::fs::write(&legacy_paths.request_path, "{}").unwrap(); - - assert_eq!( - active_interview_paths(&runtime_paths, &legacy_paths), - Some(legacy_paths) - ); + assert!(!claim_path.exists()); } #[test] diff --git a/lib/crates/fabro-cli/src/commands/run/resume.rs b/lib/crates/fabro-cli/src/commands/run/resume.rs index 597cc6d85..ecc7fa469 100644 --- a/lib/crates/fabro-cli/src/commands/run/resume.rs +++ b/lib/crates/fabro-cli/src/commands/run/resume.rs @@ -46,14 +46,7 @@ pub(crate) async fn resume_command( fn launcher_pid_alive(run_dir: &std::path::Path) -> bool { super::launcher::active_launcher_record_for_run(run_dir) - .map(|record| process_alive(record.pid)) - .or_else(|| { - std::fs::read_to_string(run_dir.join("run.pid")) - .ok() - .and_then(|pid| pid.trim().parse::().ok()) - .map(process_alive) - }) - .unwrap_or(false) + .is_some_and(|record| process_alive(record.pid)) } #[cfg(test)] diff --git a/lib/crates/fabro-cli/tests/cli.rs b/lib/crates/fabro-cli/tests/cli.rs index 49a38c248..db578952e 100644 --- a/lib/crates/fabro-cli/tests/cli.rs +++ b/lib/crates/fabro-cli/tests/cli.rs @@ -1286,10 +1286,11 @@ fn bug3_attach_leaves_interview_request_until_engine_consumes_response() { ], ); - // Status: running + // Terminal status still allows attach to answer the interview once before exiting. std::fs::write( run_dir.join("status.json"), - serde_json::json!({"status": "running", "updated_at": "2026-01-01T00:00:00Z"}).to_string(), + serde_json::json!({"status": "succeeded", "updated_at": "2026-01-01T00:00:00Z"}) + .to_string(), ) .unwrap(); @@ -1312,9 +1313,6 @@ fn bug3_attach_leaves_interview_request_until_engine_consumes_response() { ) .unwrap(); - // Dead engine so attach exits after one iteration - std::fs::write(run_dir.join("run.pid"), "99999999").unwrap(); - // Pipe "y\n" so ConsoleInterviewer doesn't block on stdin let _ = arc() .env("HOME", home.path()) @@ -1375,8 +1373,6 @@ fn attach_closed_stdin_keeps_interview_pending() { ) .unwrap(); - std::fs::write(run_dir.join("run.pid"), "99999999").unwrap(); - let assert = arc() .env("HOME", home.path()) .env("NO_COLOR", "1") @@ -1404,59 +1400,6 @@ fn attach_closed_stdin_keeps_interview_pending() { ); } -#[test] -fn attach_supports_legacy_root_interview_paths() { - let home = tempfile::tempdir().unwrap(); - - let run_dir = setup_run_dir( - home.path(), - "attach-legacy-interview-paths", - serde_json::json!({}), - &[ - r#"{"ts":"2026-01-01T00:00:01Z","run_id":"attach-legacy-interview-paths","event":"StageStarted","node_id":"gate","name":"Gate","index":0,"attempt":1,"max_attempts":1}"#, - ], - ); - - std::fs::write( - run_dir.join("status.json"), - serde_json::json!({"status": "running", "updated_at": "2026-01-01T00:00:00Z"}).to_string(), - ) - .unwrap(); - - let question = serde_json::json!({ - "text": "Approve?", - "question_type": "YesNo", - "options": [], - "allow_freeform": false, - "default": {"value": "Yes", "selected_option": null, "text": null}, - "timeout_seconds": 1.0, - "stage": "gate", - "metadata": {} - }); - std::fs::write( - run_dir.join("interview_request.json"), - serde_json::to_string(&question).unwrap(), - ) - .unwrap(); - std::fs::write(run_dir.join("run.pid"), "99999999").unwrap(); - - let _ = arc() - .env("HOME", home.path()) - .env("NO_COLOR", "1") - .args(["attach", "attach-legacy-interview-paths"]) - .write_stdin("y\n") - .timeout(std::time::Duration::from_secs(5)) - .output(); - - assert!(run_dir.join("interview_request.json").exists()); - assert!(run_dir.join("interview_response.json").exists()); - assert!( - !RuntimeState::new(&run_dir) - .interview_response_path() - .exists() - ); -} - // Bug 4: attach should respect the verbose flag from run.json. // Currently ProgressUI is created with verbose=false regardless of config. #[test] diff --git a/lib/crates/fabro-interview/src/file.rs b/lib/crates/fabro-interview/src/file.rs index 698c7d06a..99f9f295c 100644 --- a/lib/crates/fabro-interview/src/file.rs +++ b/lib/crates/fabro-interview/src/file.rs @@ -15,7 +15,7 @@ const REATTACH_WINDOW: Duration = Duration::from_secs(30); #[cfg(test)] use std::path::Path; -/// An interviewer that communicates via JSON files in the run directory. +/// An interviewer that communicates via JSON files in the runtime directory. /// /// The engine process writes `interview_request.json` and polls for /// `interview_response.json`. The attach process watches for the request @@ -148,10 +148,11 @@ mod tests { use crate::{AnswerValue, QuestionType}; fn interviewer_paths(run_dir: &Path) -> (PathBuf, PathBuf, PathBuf) { + let runtime_dir = run_dir.join("runtime"); ( - run_dir.join("interview_request.json"), - run_dir.join("interview_response.json"), - run_dir.join("interview_request.claim"), + runtime_dir.join("interview_request.json"), + runtime_dir.join("interview_response.json"), + runtime_dir.join("interview_request.claim"), ) } diff --git a/lib/crates/fabro-workflows/src/operations/resume.rs b/lib/crates/fabro-workflows/src/operations/resume.rs index ddeadccb2..111433a61 100644 --- a/lib/crates/fabro-workflows/src/operations/resume.rs +++ b/lib/crates/fabro-workflows/src/operations/resume.rs @@ -53,9 +53,6 @@ fn cleanup_resume_artifacts(run_dir: &Path) { "conclusion.json", "pull_request.json", "detached_failure.json", - "interview_request.json", - "interview_response.json", - "interview_request.claim", "progress.jsonl", ] { let _ = std::fs::remove_file(run_dir.join(name));