mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-08-28 05:27:41 +00:00
Remove legacy runtime IPC fallbacks
This commit is contained in:
parent
91a265c3a5
commit
e1e433ea17
6 changed files with 39 additions and 165 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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<RunStatusRecord> {
|
|||
}
|
||||
|
||||
fn read_launcher_pid(run_dir: &Path) -> Option<u32> {
|
||||
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::<u32>().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<InterviewPaths> {
|
||||
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<Self> {
|
||||
if try_claim_interview_request(base_dir) {
|
||||
fn acquire(claim_path: &Path) -> Option<Self> {
|
||||
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::<u32>() {
|
||||
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]
|
||||
|
|
|
|||
|
|
@ -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::<u32>().ok())
|
||||
.map(process_alive)
|
||||
})
|
||||
.unwrap_or(false)
|
||||
.is_some_and(|record| process_alive(record.pid))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
|
|
|||
|
|
@ -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]
|
||||
|
|
|
|||
|
|
@ -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"),
|
||||
)
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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));
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue