diff --git a/apps/fabro-web/app/lib/petri-stream.test.ts b/apps/fabro-web/app/lib/petri-stream.test.ts index 7a6ed3794..985f1d454 100644 --- a/apps/fabro-web/app/lib/petri-stream.test.ts +++ b/apps/fabro-web/app/lib/petri-stream.test.ts @@ -39,7 +39,7 @@ describe("stream items", () => { expect(item.run_id).toBe(fixture.run_id); } } - expect(isPetriRun({ spec: { engine: { kind: "legacy" } } } as never)).toBe(false); + expect(isPetriRun({ spec: {} } as never)).toBe(false); expect(isStreamItemPayload({ event: "run.completed", seq: 3 })).toBe(false); }); diff --git a/apps/fabro-web/app/lib/petri-stream.ts b/apps/fabro-web/app/lib/petri-stream.ts index c1fad5f54..ed060081c 100644 --- a/apps/fabro-web/app/lib/petri-stream.ts +++ b/apps/fabro-web/app/lib/petri-stream.ts @@ -47,12 +47,14 @@ export function isPlatformItem(item: RunStreamItem): boolean { return item.kind === "platform"; } -/** Whether the projection is of a run that executes on Petri. */ +/** + * Whether the projection is of a run that executes on Petri: every run + * does, so this is whether the projection carries a spec at all. + */ export function isPetriRun( projection: RunProjection | null | undefined, ): boolean { - const engine = projection?.spec?.engine; - return isRecord(engine) && getString(engine, "kind") === "petri"; + return isRecord(projection?.spec?.admission); } /** Whether an SSE payload is a run stream item rather than a legacy event. */ diff --git a/docs/public/api-reference/fabro-api.yaml b/docs/public/api-reference/fabro-api.yaml index dc22dd1fc..5fd8850fc 100644 --- a/docs/public/api-reference/fabro-api.yaml +++ b/docs/public/api-reference/fabro-api.yaml @@ -12934,6 +12934,7 @@ components: - settings - graph - provenance + - admission properties: run_id: type: string @@ -12980,33 +12981,11 @@ components: oneOf: - $ref: "#/components/schemas/ForkSourceRef" - type: "null" - engine: - $ref: "#/components/schemas/RunEngine" + admission: + $ref: "#/components/schemas/PetriAdmission" description: | - The engine the run was created for, with what it admitted. - Absent in a spec written before the field existed, which means - the legacy executor. - - RunEngine: - description: | - The engine a run was created for. `legacy` is the in-process - executor; `petri` names the Petri workflow engine and carries what - Petri admitted at create time. - oneOf: - - type: object - required: [kind] - properties: - kind: - type: string - enum: [legacy] - - allOf: - - type: object - required: [kind] - properties: - kind: - type: string - enum: [petri] - - $ref: "#/components/schemas/PetriAdmission" + What Petri admitted for the run at create time: the graphs it + executes and resumes from. PetriAdmission: description: | diff --git a/lib/apps/fabro-cli/src/commands/run/attach.rs b/lib/apps/fabro-cli/src/commands/run/attach.rs index 85216054c..a3119b2b9 100644 --- a/lib/apps/fabro-cli/src/commands/run/attach.rs +++ b/lib/apps/fabro-cli/src/commands/run/attach.rs @@ -20,10 +20,8 @@ use std::time::Duration; use anyhow::Result; use fabro_api::types; use fabro_interview::{Answer, AnswerValue, Question}; -use fabro_store::EventEnvelope; use fabro_types::settings::run::ApprovalMode; -use fabro_types::{EventBody, InterviewOption, QuestionType, RunId}; -use fabro_util::json::normalize_json_value; +use fabro_types::{InterviewOption, QuestionType, RunId}; use fabro_util::printer::Printer; use fabro_util::terminal::Styles; use fabro_workflow::outcome::StageOutcome; @@ -37,7 +35,6 @@ use crate::server_client; const INTERVIEW_UNANSWERED_MESSAGE: &str = "Interview ended without an answer. The run is still waiting for input; reattach to answer it."; const JSON_INTERVIEW_MESSAGE: &str = "This run is waiting for human input, but --json is non-interactive. Reattach without --json to answer it."; -const ATTACH_PREMATURE_EOF_MESSAGE: &str = "Attach stream ended before terminal run event."; const PROMPT_READ_POLL_INTERVAL: TokioDuration = TokioDuration::from_millis(50); /// How long a Petri attach waits before it reconnects to the stream the /// server ended while the run was still active. @@ -185,45 +182,10 @@ pub(crate) async fn attach_run_with_client( ) -> Result { let state = client.get_run_state(run_id).await?; let auto_approve = state.spec.settings.run.execution.approval == ApprovalMode::Auto; - if state.spec.engine.is_petri() { - return Box::pin(attach_petri_run_with_client( - client, - run_id, - &state, - styles, - AttachOptions { - auto_approve, - verbose: live_verbose, - kill_on_detach, - json_output, - }, - printer, - )) - .await; - } - let events = client.list_run_events(run_id, None, None).await?; - let replay_events = events.clone(); - let next_seq = events.last().map_or(1, |event| event.seq.saturating_add(1)); - let initial_exit_code = events.iter().rev().find_map(event_exit_code); - let state_exit_code = state_exit_code(&state); - - if state_is_terminal(&state) || initial_exit_code.is_some() { - return replay_run_with_client( - live_verbose, - events, - initial_exit_code - .or(state_exit_code) - .unwrap_or(ExitCode::from(1)), - json_output, - ); - } - - let stream = client.attach_run_events(run_id, Some(next_seq)).await?; - Box::pin(attach_live_run_with_client( + Box::pin(attach_petri_run_with_client( client, run_id, - replay_events, - stream, + &state, styles, AttachOptions { auto_approve, @@ -243,103 +205,6 @@ struct AttachOptions { json_output: bool, } -fn replay_run_with_client( - verbose: bool, - events: Vec, - exit_code: ExitCode, - json_output: bool, -) -> Result { - let is_tty = std::io::stderr().is_terminal(); - let mut progress_ui = run_progress::ProgressUI::new(is_tty, verbose); - - for event in events { - let line = event_payload_line(&event)?; - emit_progress_line(&mut progress_ui, &line, json_output)?; - } - - finish_progress(&mut progress_ui, json_output); - - Ok(exit_code) -} - -async fn attach_live_run_with_client( - client: &server_client::Client, - run_id: &RunId, - existing_events: Vec, - mut stream: server_client::RunEventStream, - styles: &'static Styles, - opts: AttachOptions, - printer: Printer, -) -> Result { - let is_tty = std::io::stderr().is_terminal(); - let mut progress_ui = run_progress::ProgressUI::new(is_tty, opts.verbose); - let ctrl_c_signal = ctrl_c(); - tokio::pin!(ctrl_c_signal); - - for event in existing_events { - let line = event_payload_line(&event)?; - emit_progress_line(&mut progress_ui, &line, opts.json_output)?; - } - - if let Some(exit_code) = Box::pin(handle_pending_server_interview( - client, - run_id, - &mut stream, - opts.auto_approve, - &mut progress_ui, - styles, - opts.json_output, - opts.kill_on_detach, - printer, - )) - .await? - { - return Ok(exit_code); - } - - loop { - let next_event = tokio::select! { - _ = &mut ctrl_c_signal => { - handle_detach_signal(client, run_id, opts.kill_on_detach, printer).await; - finish_progress(&mut progress_ui, opts.json_output); - return Ok(ExitCode::from(1)); - } - result = stream.next_event() => result?, - }; - - let Some(event) = next_event else { - finish_progress(&mut progress_ui, opts.json_output); - return Err(anyhow::anyhow!(ATTACH_PREMATURE_EOF_MESSAGE)); - }; - - let line = event_payload_line(&event)?; - emit_progress_line(&mut progress_ui, &line, opts.json_output)?; - - if let Some(exit_code) = event_exit_code(&event) { - finish_progress(&mut progress_ui, opts.json_output); - return Ok(exit_code); - } - - if event_starts_interview(&event) { - if let Some(exit_code) = Box::pin(handle_pending_server_interview( - client, - run_id, - &mut stream, - opts.auto_approve, - &mut progress_ui, - styles, - opts.json_output, - opts.kill_on_detach, - printer, - )) - .await? - { - return Ok(exit_code); - } - } - } -} - /// Attach to a Petri run: replay its stream through the progress renderer, /// then follow it live from the last `stream_seq` seen. A question on the /// stream is asked at the terminal and answered through the questions API. @@ -531,77 +396,6 @@ fn emit_stream_item( Ok(()) } -async fn handle_pending_server_interview( - client: &server_client::Client, - run_id: &RunId, - stream: &mut server_client::RunEventStream, - auto_approve: bool, - progress_ui: &mut run_progress::ProgressUI, - styles: &'static Styles, - json_output: bool, - kill_on_detach: bool, - printer: Printer, -) -> Result> { - let Some(question) = client.list_run_questions(run_id).await?.into_iter().next() else { - return Ok(None); - }; - - if json_pending_interview_requires_manual_input(json_output, auto_approve) { - fabro_util::printerr!(printer, "{JSON_INTERVIEW_MESSAGE}"); - return Ok(Some(ExitCode::from(1))); - } - if json_output { - return Ok(None); - } - - hide_progress(progress_ui, json_output); - let ask = ask_attach_question(api_question_to_question(&question), styles); - tokio::pin!(ask); - let ctrl_c_signal = ctrl_c(); - tokio::pin!(ctrl_c_signal); - - let answer = loop { - let next_event = tokio::select! { - answer = &mut ask => { - break answer; - } - _ = &mut ctrl_c_signal => { - handle_detach_signal(client, run_id, kill_on_detach, printer).await; - show_progress(progress_ui, json_output); - return Ok(Some(ExitCode::from(1))); - } - result = stream.next_event() => result?, - }; - - let Some(event) = next_event else { - show_progress(progress_ui, json_output); - return Err(anyhow::anyhow!(ATTACH_PREMATURE_EOF_MESSAGE)); - }; - - let line = event_payload_line(&event)?; - emit_progress_line(progress_ui, &line, json_output)?; - - if let Some(exit_code) = event_exit_code(&event) { - show_progress(progress_ui, json_output); - return Ok(Some(exit_code)); - } - - if event_resolves_interview(&event, &question.id) { - show_progress(progress_ui, json_output); - return Ok(None); - } - }; - show_progress(progress_ui, json_output); - - if answer_requires_reattach(&answer) { - fabro_util::printerr!(printer, "{INTERVIEW_UNANSWERED_MESSAGE}"); - return Ok(Some(ExitCode::from(1))); - } - - submit_server_interview_answer(client, run_id, &question.id, &answer).await?; - Ok(None) -} - async fn handle_detach_signal( client: &server_client::Client, run_id: &RunId, @@ -898,21 +692,6 @@ fn state_is_terminal(state: &server_client::RunProjection) -> bool { state.conclusion.is_some() || state.status.is_terminal() } -fn emit_progress_line( - progress_ui: &mut run_progress::ProgressUI, - line: &str, - json_output: bool, -) -> Result<()> { - if json_output { - let stdout = std::io::stdout(); - let mut handle = stdout.lock(); - writeln!(handle, "{line}")?; - } else { - progress_ui.handle_json_line(line); - } - Ok(()) -} - fn finish_progress(progress_ui: &mut run_progress::ProgressUI, json_output: bool) { if !json_output { progress_ui.finish(); @@ -931,32 +710,6 @@ fn show_progress(progress_ui: &mut run_progress::ProgressUI, json_output: bool) } } -fn event_payload_line(event: &EventEnvelope) -> Result { - let mut value = normalize_json_value(event.event.to_value()?); - restore_empty_run_properties(&mut value); - serde_json::to_string(&value).map_err(Into::into) -} - -fn restore_empty_run_properties(value: &mut serde_json::Value) { - let Some(object) = value.as_object_mut() else { - return; - }; - let Some(event_name) = object.get("event").and_then(serde_json::Value::as_str) else { - return; - }; - if matches!(event_name, "run.submitted" | "run.running") && !object.contains_key("properties") { - let run_id = object.remove("run_id"); - let ts = object.remove("ts"); - object.insert("properties".to_string(), serde_json::json!({})); - if let Some(run_id) = run_id { - object.insert("run_id".to_string(), run_id); - } - if let Some(ts) = ts { - object.insert("ts".to_string(), ts); - } - } -} - #[cfg(test)] fn infer_storage_dir(run_dir: &Path) -> Option { let scratch_dir = run_dir.parent()?; @@ -1001,42 +754,14 @@ fn state_exit_code(state: &server_client::RunProjection) -> Option { } } -fn event_exit_code(event: &EventEnvelope) -> Option { - match &event.event.body { - EventBody::RunCompleted(props) => Some( - if props.status == "succeeded" || props.status == "partially_succeeded" { - ExitCode::from(0) - } else { - ExitCode::from(1) - }, - ), - EventBody::RunFailed(_) => Some(ExitCode::from(1)), - _ => None, - } -} - -fn event_starts_interview(event: &EventEnvelope) -> bool { - matches!(event.event.body, EventBody::InterviewStarted(_)) -} - -fn event_resolves_interview(event: &EventEnvelope, question_id: &str) -> bool { - match &event.event.body { - EventBody::InterviewCompleted(props) => props.question_id == question_id, - EventBody::InterviewInterrupted(props) => props.question_id == question_id, - EventBody::InterviewTimeout(props) => props.question_id == question_id, - _ => false, - } -} - #[cfg(test)] mod tests { #![allow( clippy::absolute_paths, reason = "This test module prefers explicit type paths over extra imports." )] - use fabro_interview::{Answer, AnswerValue}; - use fabro_types::test_support; + use fabro_types::{PetriAdmission, test_support}; use fabro_util::terminal::Styles; use httpmock::MockServer; @@ -1063,7 +788,7 @@ mod tests { spec_blob: None, git: None, fork_source_ref: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }; serde_json::json!({ "spec": serde_json::to_value(spec).unwrap(), diff --git a/lib/apps/fabro-cli/src/commands/run/events.rs b/lib/apps/fabro-cli/src/commands/run/events.rs index ab35b3c05..6c3570747 100644 --- a/lib/apps/fabro-cli/src/commands/run/events.rs +++ b/lib/apps/fabro-cli/src/commands/run/events.rs @@ -1,32 +1,11 @@ -#![expect( - clippy::disallowed_types, - reason = "sync CLI `run events` command: blocking std::io::Write is the intended output mechanism" -)] -#![expect( - clippy::disallowed_methods, - reason = "sync CLI `run events` command: streams event lines to std::io::stdout directly" -)] - -use std::fmt::Write as _; -use std::io::{self, IsTerminal, Write}; -use std::time::Duration; - use anyhow::{Context, Result, bail}; use chrono::{DateTime, Utc}; -use fabro_redact::redact_jsonl_line; -use fabro_types::RunNoticeCode; -use fabro_util::json::normalize_json_value; use fabro_util::terminal::Styles; -use tokio::time; -use tracing::{debug, info}; +use tracing::info; use super::petri_stream; use crate::args::EventsArgs; use crate::command_context::CommandContext; -use crate::server_client; -use crate::shared::format_usd_micros; - -const FOLLOW_TERMINAL_GRACE: Duration = Duration::from_millis(500); pub(crate) async fn run( args: &EventsArgs, @@ -48,98 +27,17 @@ pub(crate) async fn run( .get_run_state(&run_id) .await .context("Failed to read run state from server")?; - if state.spec.engine.is_petri() { - let pretty = args.pretty && !ctx.json_output(); - return Box::pin(petri_stream::print_events( - client.as_ref(), - &run_id, - args, - since_cutoff, - pretty, - styles, - )) - .await; - } - - let events = match (args.tail, since_cutoff.is_none()) { - (Some(tail), true) => { - // With --tail 0 --follow, fetch one event anyway so `last_seq` - // seeds the follow cursor at the true latest event instead of - // replaying the whole history; `apply_filters` drops it from - // the printed output. - let tail = if args.follow { tail.max(1) } else { tail }; - client.list_run_events_tail(&run_id, tail).await - } - _ => client.list_run_events(&run_id, None, None).await, - } - .context("Failed to list server-backed run events")?; - let last_seq = events.last().map_or(0, |event| event.seq); - let all_lines = events - .iter() - .map(event_payload_line) - .collect::>>()?; - let filtered = apply_filters(&all_lines, since_cutoff.as_ref(), args.tail); - - let stdout = io::stdout(); - let is_tty = stdout.is_terminal(); - let mut out = stdout.lock(); + let _ = state; let pretty = args.pretty && !ctx.json_output(); - let mut pretty_state = PrettyEventState::default(); - - for line in &filtered { - if pretty { - if let Some(formatted) = format_event_pretty_streamed(line, styles, &mut pretty_state) { - writeln!(out, "{formatted}")?; - } - } else { - writeln!(out, "{line}")?; - } - } - - if args.follow { - follow_store_logs( - client.as_ref(), - &run_id, - if last_seq == 0 { 1 } else { last_seq + 1 }, - pretty, - styles, - is_tty, - pretty_state, - ) - .await?; - } - - Ok(()) -} - -fn event_name(event: &fabro_store::EventEnvelope) -> &str { - event.event.event_name() -} - -fn apply_filters( - lines: &[String], - since: Option<&DateTime>, - tail: Option, -) -> Vec { - let filtered: Vec = match since { - Some(cutoff) => lines - .iter() - .filter(|line| extract_timestamp(line).is_none_or(|ts| ts >= *cutoff)) - .cloned() - .collect(), - None => lines.to_vec(), - }; - - match tail { - Some(n) if n < filtered.len() => filtered[filtered.len() - n..].to_vec(), - _ => filtered, - } -} - -fn extract_timestamp(line: &str) -> Option> { - let value: serde_json::Value = serde_json::from_str(line).ok()?; - let ts_str = value.get("ts")?.as_str()?; - ts_str.parse::>().ok() + Box::pin(petri_stream::print_events( + client.as_ref(), + &run_id, + args, + since_cutoff, + pretty, + styles, + )) + .await } pub(crate) fn parse_since(s: &str) -> Result> { @@ -176,841 +74,10 @@ fn try_parse_relative_duration(s: &str) -> Option { } } -async fn follow_store_logs( - client: &server_client::Client, - run_id: &fabro_types::RunId, - seq: u32, - pretty: bool, - styles: &Styles, - _is_tty: bool, - mut pretty_state: PrettyEventState, -) -> Result<()> { - let stdout = io::stdout(); - let mut out = stdout.lock(); - let mut next_seq = seq; - let mut terminal_deadline = None; - - loop { - match time::timeout( - Duration::from_millis(200), - client.list_run_events(run_id, Some(next_seq), None), - ) - .await - { - Ok(Ok(events)) => { - let had_events = !events.is_empty(); - let saw_terminal = events - .iter() - .any(|event| matches!(event_name(event), "run.completed" | "run.failed")); - for event in events { - let line = event_payload_line(&event)?; - if pretty { - if let Some(formatted) = - format_event_pretty_streamed(&line, styles, &mut pretty_state) - { - writeln!(out, "{formatted}")?; - } - } else { - writeln!(out, "{line}")?; - } - out.flush()?; - next_seq = event.seq.saturating_add(1); - } - if saw_terminal || (terminal_deadline.is_some() && had_events) { - terminal_deadline = Some(time::Instant::now() + FOLLOW_TERMINAL_GRACE); - } - } - Err(_) => { - if run_concluded(client, run_id).await? { - terminal_deadline - .get_or_insert_with(|| time::Instant::now() + FOLLOW_TERMINAL_GRACE); - } - } - Ok(Err(err)) => return Err(err), - } - - let Some(deadline) = terminal_deadline else { - continue; - }; - if time::Instant::now() < deadline { - continue; - } - - let flushed_next_seq = flush_remaining_store_events( - client, - run_id, - next_seq, - pretty, - styles, - &mut pretty_state, - &mut out, - ) - .await?; - if flushed_next_seq > next_seq { - next_seq = flushed_next_seq; - terminal_deadline = Some(time::Instant::now() + FOLLOW_TERMINAL_GRACE); - continue; - } - - debug!("Run reached terminal status and log tail is quiet, stopping follow"); - break; - } - - Ok(()) -} - -async fn run_concluded( - client: &server_client::Client, - run_id: &fabro_types::RunId, -) -> Result { - let state = client - .get_run_state(run_id) - .await - .context("Failed to read run state from server while following events")?; - Ok(state.conclusion.is_some() || state.status.is_terminal()) -} - -async fn flush_remaining_store_events( - client: &server_client::Client, - run_id: &fabro_types::RunId, - next_seq: u32, - pretty: bool, - styles: &Styles, - pretty_state: &mut PrettyEventState, - out: &mut dyn Write, -) -> Result { - let events = client - .list_run_events(run_id, Some(next_seq), None) - .await - .context("Failed to list server-backed run events while finalizing follow")?; - - let mut next_seq = next_seq; - for event in events { - let line = event_payload_line(&event)?; - if pretty { - if let Some(formatted) = format_event_pretty_streamed(&line, styles, pretty_state) { - writeln!(out, "{formatted}")?; - } - } else { - writeln!(out, "{line}")?; - } - next_seq = event.seq.saturating_add(1); - } - out.flush()?; - Ok(next_seq) -} - -fn event_payload_line(event: &fabro_store::EventEnvelope) -> Result { - let mut value = normalize_json_value(event.event.to_value()?); - restore_empty_run_properties(&mut value); - let line = serde_json::to_string(&value)?; - Ok(redact_jsonl_line(&line)) -} - -fn restore_empty_run_properties(value: &mut serde_json::Value) { - let Some(object) = value.as_object_mut() else { - return; - }; - let Some(event_name) = object.get("event").and_then(serde_json::Value::as_str) else { - return; - }; - if matches!(event_name, "run.submitted" | "run.running") && !object.contains_key("properties") { - let run_id = object.remove("run_id"); - let ts = object.remove("ts"); - object.insert("properties".to_string(), serde_json::json!({})); - if let Some(run_id) = run_id { - object.insert("run_id".to_string(), run_id); - } - if let Some(ts) = ts { - object.insert("ts".to_string(), ts); - } - } -} - -fn render_indented_markdown(styles: &Styles, text: &str, indent: &str) -> String { - let term_width = Styles::terminal_width(); - let wrap_width = term_width.saturating_sub(indent.len()); - let rendered = styles.render_markdown_width(text, wrap_width); - rendered - .lines() - .map(|line| format!("{indent}{line}")) - .collect::>() - .join("\n") -} - -#[derive(Debug, Default)] -struct PrettyEventState { - saw_metadata_snapshot_failure: bool, -} - -fn format_event_pretty_streamed( - line: &str, - styles: &Styles, - state: &mut PrettyEventState, -) -> Option { - let envelope: serde_json::Value = serde_json::from_str(line).ok()?; - let event = envelope.get("event")?.as_str()?; - if event == "run.notice" - && state.saw_metadata_snapshot_failure - && is_metadata_snapshot_compat_notice(&envelope) - { - return None; - } - let formatted = format_event_pretty_value(&envelope, styles); - if event == "metadata.snapshot.failed" { - state.saw_metadata_snapshot_failure = true; - } - formatted -} - -#[cfg_attr( - not(test), - allow( - dead_code, - reason = "Production pretty events use the stateful stream formatter; unit tests exercise this single-line helper." - ) -)] -pub(crate) fn format_event_pretty(line: &str, styles: &Styles) -> Option { - let envelope: serde_json::Value = serde_json::from_str(line).ok()?; - format_event_pretty_value(&envelope, styles) -} - -fn format_event_pretty_value(envelope: &serde_json::Value, styles: &Styles) -> Option { - let event = envelope.get("event")?.as_str()?; - let ts = format_timestamp(envelope.get("ts")?.as_str()?); - - match event { - "run.started" => { - let name = prop_str_field(envelope, "name").unwrap_or("?"); - let run_id = str_field(envelope, "run_id").unwrap_or("?"); - let header = format!( - "{} {} {} {}", - styles.dim.apply_to(&ts), - styles.bold_cyan.apply_to("\u{25b6}"), - styles.bold.apply_to(name), - styles.dim.apply_to(run_id), - ); - match prop_str_field(envelope, "goal") { - Some(goal) if !goal.is_empty() => { - let body = render_indented_markdown(styles, goal, " "); - Some(format!("{header}\n{body}\n")) - } - _ => Some(header), - } - } - "run.completed" => { - let duration = format_duration_ms(timing_wall_field(envelope)); - let status_str = match prop_str_field(envelope, "status") { - Some(status) if !status.is_empty() => status, - _ => "succeeded", - }; - let status_upper = status_str.to_uppercase(); - let status_style = match status_str { - "succeeded" | "partially_succeeded" => &styles.bold_green, - _ => &styles.bold_red, - }; - let usage = prop_field(envelope, "usage"); - let cost = format_cost( - usage - .and_then(|value| value.get("cost")) - .and_then(|value| value.get("usd_micros")) - .or_else(|| prop_field(envelope, "total_cost")), - ); - - let mut summary = format!( - "{} {} {}", - styles.dim.apply_to(&ts), - status_style.apply_to(format!("\u{2713} {status_upper}")), - styles.bold.apply_to(&duration), - ); - if !cost.is_empty() { - write!(summary, " {}", styles.dim.apply_to(&cost)).expect("write to string"); - } - - let mut lines = vec![summary]; - - if let Some(tokens) = usage.and_then(|value| value.get("tokens")) { - let bucket = |name: &str| tokens.get(name).and_then(serde_json::Value::as_u64); - let total = ["input", "output", "reasoning", "cache_read", "cache_write"] - .into_iter() - .filter_map(bucket) - .fold(0_u64, u64::saturating_add); - let pad = " ".repeat(ts.len() + 1); - if total > 0 { - lines.push(format!( - "{}{}", - pad, - styles - .dim - .apply_to(format!("Tokens: {}", format_tokens(total))) - )); - } - if let Some(cache_read) = bucket("cache_read") { - let cache_write = bucket("cache_write").unwrap_or(0); - lines.push(format!( - "{}{}", - pad, - styles.dim.apply_to(format!( - "Cache: {} read, {} write", - format_tokens(cache_read), - format_tokens(cache_write) - )) - )); - } - if let Some(reasoning) = bucket("reasoning") { - if reasoning > 0 { - lines.push(format!( - "{}{}", - pad, - styles.dim.apply_to(format!( - "Reasoning: {} tokens", - format_tokens(reasoning) - )) - )); - } - } - } - - Some(lines.join("\n")) - } - "run.failed" => { - let error = prop_field(envelope, "failure") - .and_then(failure_message) - .and_then(serde_json::Value::as_str) - .unwrap_or("unknown error"); - Some(format!( - "{} {} {}", - styles.dim.apply_to(&ts), - styles.bold_red.apply_to("\u{2717} Failed"), - styles.red.apply_to(error), - )) - } - "run.notice" => { - let level = prop_str_field(envelope, "level").unwrap_or("info"); - let code = prop_str_field(envelope, "code").unwrap_or(""); - let message = prop_str_field(envelope, "message").unwrap_or(""); - let label = match level { - "warn" => styles.yellow.apply_to("Warning:").to_string(), - "error" => styles.bold_red.apply_to("Error:").to_string(), - _ => styles.bold.apply_to("Info:").to_string(), - }; - let code_suffix = if code.is_empty() { - String::new() - } else { - format!(" {}", styles.dim.apply_to(format!("[{code}]"))) - }; - Some(format!( - "{} {} {}{}", - styles.dim.apply_to(&ts), - label, - message, - code_suffix, - )) - } - "metadata.snapshot.completed" => { - let phase = prop_str_field(envelope, "phase").unwrap_or("?"); - let duration = format_duration_ms(prop_field(envelope, "duration_ms")); - Some(format!( - "{} Metadata {} {}", - styles.dim.apply_to(&ts), - phase, - styles.dim.apply_to(&duration), - )) - } - "metadata.snapshot.failed" => { - let phase = prop_str_field(envelope, "phase").unwrap_or("?"); - let failure_kind = prop_str_field(envelope, "failure_kind").unwrap_or(""); - let error = prop_str_field(envelope, "error").unwrap_or("unknown error"); - let kind_suffix = if failure_kind.is_empty() { - String::new() - } else { - format!(" {}", styles.dim.apply_to(format!("[{failure_kind}]"))) - }; - Some(format!( - "{} {} Metadata {} failed: {}{}", - styles.dim.apply_to(&ts), - styles.yellow.apply_to("Warning:"), - phase, - error, - kind_suffix, - )) - } - "stage.started" => { - let label = str_field(envelope, "node_label").unwrap_or("?"); - Some(format!( - "{} {} {}", - styles.dim.apply_to(&ts), - styles.bold_cyan.apply_to("\u{25b6}"), - styles.bold.apply_to(label), - )) - } - "stage.completed" => { - let label = str_field(envelope, "node_label").unwrap_or("?"); - let duration = format_duration_ms(timing_wall_field(envelope)); - // `stage.completed.usage` is a `ModelUsage`: the model, then the usage. - let usage = prop_field(envelope, "usage").and_then(|value| value.get("usage")); - let cost = format_cost( - usage - .and_then(|value| value.get("cost")) - .and_then(|value| value.get("usd_micros")), - ); - let tokens = usage.and_then(|value| value.get("tokens")); - let input_tokens = tokens - .and_then(|value| value.get("input")) - .and_then(serde_json::Value::as_u64) - .unwrap_or(0); - let output_tokens = tokens - .and_then(|value| value.get("output")) - .and_then(serde_json::Value::as_u64) - .unwrap_or(0); - let token_total = input_tokens.saturating_add(output_tokens); - let mut line = format!( - "{} {} {} {} {}", - styles.dim.apply_to(&ts), - styles.green.apply_to("\u{2713}"), - styles.bold.apply_to(label), - cost, - duration, - ); - if token_total > 0 { - let _ = write!( - line, - " {}", - styles.dim.apply_to(format_tokens(token_total)) - ); - } - Some(line) - } - "stage.failed" => { - let label = str_field(envelope, "node_label").unwrap_or("?"); - let error = prop_str_field(envelope, "error").unwrap_or("unknown error"); - Some(format!( - "{} {} {} {}", - styles.dim.apply_to(&ts), - styles.red.apply_to("\u{2717}"), - styles.bold.apply_to(label), - styles.red.apply_to(error), - )) - } - "agent.message" => { - let stage = str_field(envelope, "node_id").unwrap_or("?"); - let model = prop_str_field(envelope, "model").unwrap_or("?"); - let text = prop_str_field(envelope, "text").unwrap_or(""); - let header = format!( - "{} {} {} {}{}{}", - styles.dim.apply_to(&ts), - "\u{1f4ac}", - styles.bold.apply_to(stage), - styles.dim.apply_to("["), - styles.dim.apply_to(model), - styles.dim.apply_to("]"), - ); - let body = render_indented_markdown(styles, text, " "); - Some(format!("{header}\n{body}\n")) - } - "agent.tool.started" => { - let tool = prop_str_field(envelope, "tool_name").unwrap_or("?"); - let detail = tool_detail(envelope); - let display = match detail { - Some(value) => format!("{tool}({value})"), - None => tool.to_string(), - }; - Some(format!( - "{} {} {}", - styles.dim.apply_to(&ts), - styles.dim.apply_to("\u{2699}"), - styles.dim.apply_to(&display), - )) - } - "agent.tool.completed" => { - let tool = prop_str_field(envelope, "tool_name").unwrap_or("?"); - let is_error = prop_field(envelope, "is_error") - .and_then(serde_json::Value::as_bool) - .unwrap_or(false); - let detail = tool_detail(envelope); - let display = match detail { - Some(value) => format!("{tool}({value})"), - None => tool.to_string(), - }; - let glyph = if is_error { "\u{2717}" } else { "\u{2713}" }; - let style = if is_error { &styles.red } else { &styles.green }; - Some(format!( - "{} {} {}", - styles.dim.apply_to(&ts), - style.apply_to(glyph), - display, - )) - } - "edge.selected" => { - let to = prop_str_field(envelope, "to_node").unwrap_or("?"); - let reason = prop_str_field(envelope, "reason").unwrap_or("?"); - let condition = prop_str_field(envelope, "condition"); - let detail = match condition { - Some(value) => format!(" [{value}]"), - None => String::new(), - }; - Some(format!( - "{} {} {} {}{}", - styles.dim.apply_to(&ts), - styles.dim.apply_to("\u{2192}"), - to, - styles.dim.apply_to(reason), - styles.dim.apply_to(&detail), - )) - } - "sandbox.ready" => { - let provider = prop_str_field(envelope, "provider").unwrap_or("?"); - let duration = format_duration_ms(prop_field(envelope, "duration_ms")); - Some(format!( - "{} Sandbox: {} {}", - styles.dim.apply_to(&ts), - provider, - styles.dim.apply_to(&duration), - )) - } - "git.identity.resolved" => { - let name = prop_str_field(envelope, "name").unwrap_or("?"); - let email = prop_str_field(envelope, "email").unwrap_or("?"); - let source = prop_str_field(envelope, "source").unwrap_or("?"); - Some(format!( - "{} Git identity: {} <{}> {}", - styles.dim.apply_to(&ts), - name, - email, - styles.dim.apply_to(source), - )) - } - "sandbox.create.progress" => { - let code = envelope - .pointer("/properties/progress/code") - .and_then(serde_json::Value::as_str)?; - if code != "image.pull" { - return None; - } - let message = envelope - .pointer("/properties/progress/message") - .and_then(serde_json::Value::as_str) - .unwrap_or("image"); - let name = message.strip_prefix("pulling image ").unwrap_or(message); - Some(format!( - "{} Sandbox: pulling {}", - styles.dim.apply_to(&ts), - name, - )) - } - "snapshot.create.started" => { - let name = driver_subject_name(envelope); - Some(format!( - "{} Sandbox: building {}", - styles.dim.apply_to(&ts), - name, - )) - } - "snapshot.create.completed" => { - let name = driver_subject_name(envelope); - let duration = format_duration_ms(driver_duration_ms(envelope).as_ref()); - Some(format!( - "{} Sandbox snapshot: {} {}", - styles.dim.apply_to(&ts), - name, - styles.dim.apply_to(&duration), - )) - } - "snapshot.create.failed" => { - let name = driver_subject_name(envelope); - let error = envelope - .pointer("/properties/error/message") - .and_then(serde_json::Value::as_str) - .unwrap_or("unknown error"); - Some(format!( - "{} {} Sandbox snapshot {} failed: {}", - styles.dim.apply_to(&ts), - styles.bold_red.apply_to("\u{2717}"), - name, - styles.red.apply_to(error), - )) - } - "setup.completed" => { - let count = prop_field(envelope, "command_count").and_then(serde_json::Value::as_u64); - let duration = format_duration_ms(prop_field(envelope, "duration_ms")); - Some(match count { - Some(count) => format!( - "{} Setup: {} commands {}", - styles.dim.apply_to(&ts), - count, - styles.dim.apply_to(&duration), - ), - None => format!( - "{} Setup: {}", - styles.dim.apply_to(&ts), - styles.dim.apply_to(&duration), - ), - }) - } - "agent.compaction.completed" => { - let original = prop_field(envelope, "original_turn_count") - .and_then(serde_json::Value::as_u64) - .unwrap_or(0); - let preserved = prop_field(envelope, "preserved_turn_count") - .and_then(serde_json::Value::as_u64) - .unwrap_or(0); - Some(format!( - "{} {}", - styles.dim.apply_to(&ts), - styles - .dim - .apply_to(format!("compaction: {original}\u{2192}{preserved} turns")), - )) - } - "parallel.started" => { - let count = prop_field(envelope, "branch_count") - .and_then(serde_json::Value::as_u64) - .unwrap_or(0); - Some(format!( - "{} {} Parallel {} branches", - styles.dim.apply_to(&ts), - styles.bold_cyan.apply_to("\u{25b6}"), - count, - )) - } - "parallel.branch.started" => { - let label = str_field(envelope, "node_label").unwrap_or("?"); - Some(format!( - "{} {} {}", - styles.dim.apply_to(&ts), - styles.cyan.apply_to("\u{25b6}"), - label, - )) - } - "parallel.branch.completed" => { - let label = str_field(envelope, "node_label").unwrap_or("?"); - Some(format!( - "{} {} {}", - styles.dim.apply_to(&ts), - styles.green.apply_to("\u{2713}"), - label, - )) - } - "parallel.completed" => { - let duration = format_duration_ms(prop_field(envelope, "duration_ms")); - Some(format!( - "{} {} Parallel {}", - styles.dim.apply_to(&ts), - styles.green.apply_to("\u{2713}"), - duration, - )) - } - "pull_request.created" => { - let url = prop_str_field(envelope, "pr_url").unwrap_or("?"); - let draft = prop_field(envelope, "draft") - .and_then(serde_json::Value::as_bool) - .unwrap_or(false); - let label = if draft { "Draft PR:" } else { "PR:" }; - Some(format!( - "{} {} {}", - styles.dim.apply_to(&ts), - styles.bold.apply_to(label), - url, - )) - } - "pull_request.linked" => Some(format_pull_request_record_event( - envelope, - styles, - &ts, - "PR linked:", - )), - "pull_request.unlinked" => Some(format_pull_request_record_event( - envelope, - styles, - &ts, - "PR unlinked:", - )), - "pull_request.failed" => { - let error = prop_str_field(envelope, "error").unwrap_or("unknown error"); - Some(format!( - "{} {} {}", - styles.dim.apply_to(&ts), - styles.bold_red.apply_to("PR failed:"), - styles.red.apply_to(error), - )) - } - _ => None, - } -} - -fn is_metadata_snapshot_compat_notice(envelope: &serde_json::Value) -> bool { - prop_str_field(envelope, "code") - .and_then(|code| code.parse::().ok()) - .is_some_and(RunNoticeCode::is_metadata_snapshot_compat) -} - -fn str_field<'a>(value: &'a serde_json::Value, key: &str) -> Option<&'a str> { - value.get(key)?.as_str() -} - -/// The name of the resource a sandbox driver event is about, falling back -/// to its id. -fn driver_subject_name(envelope: &serde_json::Value) -> &str { - envelope - .pointer("/properties/subject/name") - .or_else(|| envelope.pointer("/properties/subject/id")) - .and_then(serde_json::Value::as_str) - .unwrap_or("?") -} - -/// A sandbox driver operation's duration, in milliseconds, as the number -/// [`format_duration_ms`] reads. -fn driver_duration_ms(envelope: &serde_json::Value) -> Option { - let duration = envelope.pointer("/properties/duration")?; - let secs = duration.get("secs").and_then(serde_json::Value::as_u64)?; - let nanos = duration - .get("nanos") - .and_then(serde_json::Value::as_u64) - .unwrap_or(0); - Some(serde_json::Value::from( - secs.saturating_mul(1000).saturating_add(nanos / 1_000_000), - )) -} - -fn prop_field<'a>(value: &'a serde_json::Value, key: &str) -> Option<&'a serde_json::Value> { - value.get("properties")?.get(key) -} - -fn prop_str_field<'a>(value: &'a serde_json::Value, key: &str) -> Option<&'a str> { - prop_field(value, key)?.as_str() -} - -/// Read `properties.timing.wall_time_ms` from a stage/run terminal event -/// envelope. Returns `None` when timing is absent, which falls back to a -/// blank display via `format_duration_ms`. -fn timing_wall_field(envelope: &serde_json::Value) -> Option<&serde_json::Value> { - prop_field(envelope, "timing")?.get("wall_time_ms") -} - -fn failure_message(failure: &serde_json::Value) -> Option<&serde_json::Value> { - failure - .get("detail") - .and_then(|detail| detail.get("message")) - .or_else(|| failure.get("message")) -} - -fn format_pull_request_record_event( - envelope: &serde_json::Value, - styles: &Styles, - ts: &str, - label: &str, -) -> String { - let url = prop_field(envelope, "pull_request") - .and_then(|record| record.get("html_url")) - .and_then(serde_json::Value::as_str) - .unwrap_or("?"); - format!( - "{} {} {}", - styles.dim.apply_to(ts), - styles.bold.apply_to(label), - url, - ) -} - -fn format_timestamp(ts: &str) -> String { - ts.parse::>() - .map_or_else(|_| ts.to_string(), |dt| dt.format("%H:%M:%S").to_string()) -} - -fn format_duration_ms(value: Option<&serde_json::Value>) -> String { - let ms = value.and_then(serde_json::Value::as_u64).unwrap_or(0); - if ms < 1000 { - format!("{ms}ms") - } else { - let secs = ms as f64 / 1000.0; - if secs < 60.0 { - format!("{secs:.0}s") - } else { - let mins = secs / 60.0; - format!("{mins:.1}m") - } - } -} - -fn format_cost(value: Option<&serde_json::Value>) -> String { - match value { - Some(value) => { - if let Some(usd_micros) = value.as_u64() { - if usd_micros > 0 { - return format_usd_micros(usd_micros); - } - } - let cost = value.as_f64().unwrap_or(0.0); - if cost > 0.0 { - format!("${cost:.2}") - } else { - String::new() - } - } - None => String::new(), - } -} - -fn format_tokens(tokens: u64) -> String { - if tokens >= 1000 { - format!("{:.1}k toks", tokens as f64 / 1000.0) - } else { - format!("{tokens} toks") - } -} - -fn tool_detail(envelope: &serde_json::Value) -> Option { - let tool_name = prop_str_field(envelope, "tool_name")?; - let arguments = prop_field(envelope, "arguments")?; - let arg = |key: &str| arguments.get(key).and_then(|v| v.as_str()); - - match tool_name { - "bash" | "shell" | "execute_command" => arg("command").map(|c| truncate(c, 60)), - "glob" => arg("pattern").map(String::from), - "grep" | "ripgrep" => arg("pattern").map(|p| truncate(p, 40)), - "read_file" | "read" => arg("path") - .or_else(|| arg("file_path")) - .map(|p| truncate(p, 60)), - "write_file" | "write" | "create_file" => arg("path") - .or_else(|| arg("file_path")) - .map(|p| truncate(p, 60)), - "edit_file" | "edit" => arg("path") - .or_else(|| arg("file_path")) - .map(|p| truncate(p, 60)), - "list_dir" => arg("path") - .or_else(|| arg("file_path")) - .map(|p| truncate(p, 60)), - "web_search" => arg("query").map(|q| truncate(q, 60)), - "web_fetch" => arg("url").map(|u| truncate(u, 60)), - "spawn_agent" => arg("task").map(|t| truncate(t, 60)), - "wait" | "send_input" | "close_agent" => arg("agent_id").map(String::from), - "use_skill" => arg("skill_name").map(String::from), - "apply_patch" => Some("…".into()), - "read_many_files" => arguments - .get("paths") - .and_then(|v| v.as_array()) - .map(|a| format!("{} files", a.len())), - _ => None, - } -} - -fn truncate(s: &str, max: usize) -> String { - if s.len() <= max { - s.to_string() - } else { - let boundary = s.floor_char_boundary(max.saturating_sub(1)); - format!("{}\u{2026}", &s[..boundary]) - } -} - #[cfg(test)] mod tests { use super::*; - fn no_color_styles() -> Styles { - Styles::new(false) - } - #[test] fn parse_since_relative_minutes() { let before = Utc::now(); @@ -1054,400 +121,4 @@ mod tests { fn parse_since_overflow_is_invalid() { assert!(parse_since("9223372036854775808s").is_err()); } - - #[test] - fn tail_returns_last_n_lines() { - let lines: Vec = (0..10).map(|i| format!("line {i}")).collect(); - let result = apply_filters(&lines, None, Some(3)); - assert_eq!(result.len(), 3); - assert_eq!(result[0], "line 7"); - assert_eq!(result[2], "line 9"); - } - - #[test] - fn tail_all_when_n_exceeds_total() { - let lines: Vec = (0..3).map(|i| format!("line {i}")).collect(); - let result = apply_filters(&lines, None, Some(100)); - assert_eq!(result.len(), 3); - } - - #[test] - fn since_filters_by_timestamp() { - let cutoff = "2026-01-01T12:00:00Z".parse::>().unwrap(); - let lines = vec![ - r#"{"ts":"2026-01-01T11:00:00Z","event":"stage.started"}"#.to_string(), - r#"{"ts":"2026-01-01T12:30:00Z","event":"stage.completed"}"#.to_string(), - r#"{"ts":"2026-01-01T13:00:00Z","event":"run.completed"}"#.to_string(), - ]; - let result = apply_filters(&lines, Some(&cutoff), None); - assert_eq!(result.len(), 2); - } - - #[test] - fn raw_lines_pass_through_verbatim() { - let lines = vec![ - r#"{"ts":"2026-01-01T12:00:00Z","event":"stage.started","node_label":"plan"}"# - .to_string(), - ]; - let result = apply_filters(&lines, None, None); - assert_eq!(result, lines); - } - - #[test] - fn pretty_stage_started() { - let styles = no_color_styles(); - let line = r#"{"ts":"2026-01-01T14:23:09Z","event":"stage.started","node_label":"plan","node_id":"plan","properties":{"index":0}}"#; - let result = format_event_pretty(line, &styles).unwrap(); - assert!(result.contains("plan"), "got: {result}"); - assert!(result.contains("\u{25b6}"), "got: {result}"); - } - - #[test] - fn pretty_stage_completed() { - let styles = no_color_styles(); - let line = r#"{"ts":"2026-01-01T14:23:15Z","event":"stage.completed","node_label":"plan","properties":{"timing":{"wall_time_ms":8000,"inference_time_ms":0,"tool_time_ms":0,"active_time_ms":0},"status":"succeeded","usage":{"model":{"provider":"openai","model_id":"gpt-5.4"},"usage":{"tokens":{"input":10000,"output":5200},"cost":{"usd_micros":120000,"source":"catalog"}}}}}"#; - let result = format_event_pretty(line, &styles).unwrap(); - assert!(result.contains("plan"), "got: {result}"); - assert!(result.contains("$0.12"), "got: {result}"); - assert!(result.contains("8s"), "got: {result}"); - assert!(result.contains("15.2k toks"), "got: {result}"); - } - - #[test] - fn pretty_assistant_message() { - let styles = no_color_styles(); - let line = r#"{"ts":"2026-01-01T14:23:12Z","event":"agent.message","node_id":"plan","properties":{"model":"claude-opus-4-6","text":"I'll start by reading the code.","usage":{"input_tokens":100,"output_tokens":50},"tool_call_count":0}}"#; - let result = format_event_pretty(line, &styles).unwrap(); - assert!(result.contains("plan"), "got: {result}"); - assert!(result.contains("claude-opus-4-6"), "got: {result}"); - assert!(result.contains("reading the code"), "got: {result}"); - } - - #[test] - fn pretty_tool_call_started() { - let styles = no_color_styles(); - let line = r#"{"ts":"2026-01-01T14:23:12Z","event":"agent.tool.started","properties":{"tool_name":"read_file","tool_call_id":"tc_1","arguments":{"path":"src/main.rs"}}}"#; - let result = format_event_pretty(line, &styles).unwrap(); - assert!(result.contains("read_file"), "got: {result}"); - assert!(result.contains("src/main.rs"), "got: {result}"); - } - - #[test] - fn pretty_skips_noise_events() { - let styles = no_color_styles(); - let line = r#"{"ts":"2026-01-01T14:23:12Z","event":"agent.text.delta","properties":{"delta":"hello"}}"#; - assert!(format_event_pretty(line, &styles).is_none()); - } - - #[test] - fn pretty_skips_assistant_output_replace_noise_event() { - let styles = no_color_styles(); - let line = r#"{"ts":"2026-01-01T14:23:12Z","event":"agent.output.replace","properties":{"text":""}}"#; - assert!(format_event_pretty(line, &styles).is_none()); - } - - #[test] - fn pretty_unknown_events_return_none() { - let styles = no_color_styles(); - let line = r#"{"ts":"2026-01-01T14:23:12Z","event":"SomeFutureEvent","data":123}"#; - assert!(format_event_pretty(line, &styles).is_none()); - } - - #[test] - fn pretty_workflow_run_started() { - let styles = no_color_styles(); - let line = r#"{"ts":"2026-01-01T14:23:01Z","run_id":"abc123","event":"run.started","properties":{"name":"smoke"}}"#; - let result = format_event_pretty(line, &styles).unwrap(); - assert!(result.contains("smoke"), "got: {result}"); - assert!(result.contains("abc123"), "got: {result}"); - } - - #[test] - fn pretty_workflow_run_started_with_goal() { - let styles = no_color_styles(); - let line = r#"{"ts":"2026-01-01T14:23:01Z","run_id":"abc123","event":"run.started","properties":{"name":"smoke","goal":"Fix the bug"}}"#; - let result = format_event_pretty(line, &styles).unwrap(); - assert!(result.contains("smoke"), "got: {result}"); - assert!(result.contains("abc123"), "got: {result}"); - assert!(result.contains("Fix the bug"), "got: {result}"); - assert!(result.contains('\n'), "got: {result}"); - } - - #[test] - fn pretty_workflow_run_started_without_goal_no_extra_lines() { - let styles = no_color_styles(); - let line = r#"{"ts":"2026-01-01T14:23:01Z","run_id":"abc123","event":"run.started","properties":{"name":"smoke"}}"#; - let result = format_event_pretty(line, &styles).unwrap(); - assert!(!result.contains('\n'), "got: {result}"); - } - - #[test] - fn pretty_workflow_run_completed() { - let styles = no_color_styles(); - let line = r#"{"ts":"2026-01-01T14:23:32Z","run_id":"abc123","event":"run.completed","properties":{"timing":{"wall_time_ms":25000,"inference_time_ms":0,"tool_time_ms":0,"active_time_ms":0},"status":"succeeded","usage":{"tokens":{"input":5000,"output":2000,"cache_read":3000,"cache_write":500,"reasoning":800},"cost":{"usd_micros":570000,"source":"catalog"}}}}"#; - let result = format_event_pretty(line, &styles).unwrap(); - assert!(result.contains("SUCCEEDED"), "got: {result}"); - assert!(result.contains("25s"), "got: {result}"); - assert!(result.contains("$0.57"), "got: {result}"); - assert!(result.contains("11.3k toks"), "got: {result}"); - assert!(result.contains("Cache:"), "got: {result}"); - assert!(result.contains("3.0k toks read"), "got: {result}"); - assert!(result.contains("Reasoning:"), "got: {result}"); - } - - #[test] - fn pretty_workflow_run_completed_backward_compat() { - let styles = no_color_styles(); - let line = r#"{"ts":"2026-01-01T14:23:32Z","run_id":"abc123","event":"run.completed","properties":{"timing":{"wall_time_ms":25000,"inference_time_ms":0,"tool_time_ms":0,"active_time_ms":0},"total_cost":0.57}}"#; - let result = format_event_pretty(line, &styles).unwrap(); - assert!(result.contains("SUCCEEDED"), "got: {result}"); - assert!(result.contains("25s"), "got: {result}"); - assert!(result.contains("$0.57"), "got: {result}"); - assert!(!result.contains("Tokens:"), "got: {result}"); - } - - #[test] - fn pretty_workflow_run_completed_fail_status() { - let styles = no_color_styles(); - let line = r#"{"ts":"2026-01-01T14:23:32Z","event":"run.completed","properties":{"timing":{"wall_time_ms":25000,"inference_time_ms":0,"tool_time_ms":0,"active_time_ms":0},"status":"failed"}}"#; - let result = format_event_pretty(line, &styles).unwrap(); - assert!(result.contains("FAIL"), "got: {result}"); - } - - #[test] - fn pretty_pull_request_created() { - let styles = no_color_styles(); - let line = r#"{"ts":"2026-01-01T14:25:00Z","event":"pull_request.created","properties":{"pr_url":"https://github.com/owner/repo/pull/42","pr_number":42,"draft":false}}"#; - let result = format_event_pretty(line, &styles).unwrap(); - assert!(result.contains("PR:"), "got: {result}"); - assert!( - result.contains("https://github.com/owner/repo/pull/42"), - "got: {result}" - ); - } - - #[test] - fn pretty_pull_request_created_draft() { - let styles = no_color_styles(); - let line = r#"{"ts":"2026-01-01T14:25:00Z","event":"pull_request.created","properties":{"pr_url":"https://github.com/owner/repo/pull/42","pr_number":42,"draft":true}}"#; - let result = format_event_pretty(line, &styles).unwrap(); - assert!(result.contains("Draft PR:"), "got: {result}"); - } - - #[test] - fn pretty_pull_request_linked() { - let styles = no_color_styles(); - let line = r#"{"ts":"2026-01-01T14:25:00Z","event":"pull_request.linked","properties":{"pull_request":{"owner":"owner","repo":"repo","number":42,"html_url":"https://github.com/owner/repo/pull/42"}}}"#; - let result = format_event_pretty(line, &styles).unwrap(); - assert!(result.contains("PR linked:"), "got: {result}"); - assert!( - result.contains("https://github.com/owner/repo/pull/42"), - "got: {result}" - ); - } - - #[test] - fn pretty_pull_request_unlinked() { - let styles = no_color_styles(); - let line = r#"{"ts":"2026-01-01T14:25:00Z","event":"pull_request.unlinked","properties":{"pull_request":{"owner":"owner","repo":"repo","number":42,"html_url":"https://github.com/owner/repo/pull/42"}}}"#; - let result = format_event_pretty(line, &styles).unwrap(); - assert!(result.contains("PR unlinked:"), "got: {result}"); - } - - #[test] - fn pretty_pull_request_failed() { - let styles = no_color_styles(); - let line = r#"{"ts":"2026-01-01T14:25:00Z","event":"pull_request.failed","properties":{"error":"auth token expired"}}"#; - let result = format_event_pretty(line, &styles).unwrap(); - assert!(result.contains("PR failed:"), "got: {result}"); - assert!(result.contains("auth token expired"), "got: {result}"); - } - - #[test] - fn pretty_run_notice_warn() { - let styles = no_color_styles(); - let code = RunNoticeCode::SandboxCleanupFailed.to_string(); - let line = serde_json::json!({ - "ts": "2026-01-01T14:25:00Z", - "event": "run.notice", - "properties": { - "level": "warn", - "code": code, - "message": "sandbox cleanup failed: boom", - }, - }) - .to_string(); - let result = format_event_pretty(&line, &styles).unwrap(); - assert!(result.contains("Warning:"), "got: {result}"); - assert!( - result.contains("sandbox cleanup failed: boom"), - "got: {result}" - ); - assert!( - result.contains(&format!("[{}]", RunNoticeCode::SandboxCleanupFailed)), - "got: {result}" - ); - } - - #[test] - fn pretty_run_notice_error() { - let styles = no_color_styles(); - let line = r#"{"ts":"2026-01-01T14:25:00Z","event":"run.notice","properties":{"level":"error","code":"launch_failed","message":"failed to start engine"}}"#; - let result = format_event_pretty(line, &styles).unwrap(); - assert!(result.contains("Error:"), "got: {result}"); - assert!(result.contains("failed to start engine"), "got: {result}"); - assert!(result.contains("[launch_failed]"), "got: {result}"); - } - - #[test] - fn pretty_metadata_snapshot_completed() { - let styles = no_color_styles(); - let line = r#"{"ts":"2026-01-01T14:25:00Z","event":"metadata.snapshot.completed","properties":{"phase":"checkpoint","branch":"fabro/meta","duration_ms":2800,"entry_count":2,"bytes":42,"commit_sha":"abc123"}}"#; - let result = format_event_pretty(line, &styles).unwrap(); - assert!(result.contains("Metadata checkpoint"), "got: {result}"); - assert!(result.contains("3s"), "got: {result}"); - } - - #[test] - fn pretty_metadata_snapshot_failed() { - let styles = no_color_styles(); - let line = r#"{"ts":"2026-01-01T14:25:00Z","event":"metadata.snapshot.failed","properties":{"phase":"finalize","branch":"fabro/meta","duration_ms":900,"failure_kind":"push","error":"push rejected","commit_sha":"abc123","entry_count":2,"bytes":42}}"#; - let result = format_event_pretty(line, &styles).unwrap(); - assert!(result.contains("Warning:"), "got: {result}"); - assert!( - result.contains("Metadata finalize failed: push rejected"), - "got: {result}" - ); - assert!(result.contains("[push]"), "got: {result}"); - } - - #[test] - fn pretty_sandbox_snapshot_pulling() { - let styles = no_color_styles(); - let line = r#"{"ts":"2026-01-01T14:25:00Z","event":"sandbox.create.progress","properties":{"id":{"source_id":"t","sequence":1},"occurred_at":"2026-01-01T14:25:00Z","provider":"docker","subject":{"type":"sandbox"},"type":"operation_progress","action":"create","progress":{"code":"image.pull","message":"pulling image buildpack-deps:noble"}}}"#; - let result = format_event_pretty(line, &styles).unwrap(); - assert!(result.contains("Sandbox: pulling"), "got: {result}"); - assert!(result.contains("buildpack-deps:noble"), "got: {result}"); - } - - #[test] - fn pretty_sandbox_snapshot_creating() { - let styles = no_color_styles(); - let line = r#"{"ts":"2026-01-01T14:25:00Z","event":"snapshot.create.started","properties":{"id":{"source_id":"t","sequence":1},"occurred_at":"2026-01-01T14:25:00Z","provider":"daytona","subject":{"type":"snapshot","name":"fabro-v9-test"},"type":"operation_started","action":"create"}}"#; - let result = format_event_pretty(line, &styles).unwrap(); - assert!(result.contains("Sandbox: building"), "got: {result}"); - assert!(result.contains("fabro-v9-test"), "got: {result}"); - } - - #[test] - fn pretty_sandbox_snapshot_ready() { - let styles = no_color_styles(); - let line = r#"{"ts":"2026-01-01T14:25:00Z","event":"snapshot.create.completed","properties":{"id":{"source_id":"t","sequence":1},"occurred_at":"2026-01-01T14:25:00Z","provider":"daytona","subject":{"type":"snapshot","name":"buildpack-deps:noble"},"type":"operation_completed","action":"create","duration":{"secs":8,"nanos":200000000}}}"#; - let result = format_event_pretty(line, &styles).unwrap(); - assert!(result.contains("Sandbox snapshot:"), "got: {result}"); - assert!(result.contains("buildpack-deps:noble"), "got: {result}"); - assert!(result.contains("8s"), "got: {result}"); - } - - #[test] - fn pretty_sandbox_snapshot_failed() { - let styles = no_color_styles(); - let line = r#"{"ts":"2026-01-01T14:25:00Z","event":"snapshot.create.failed","properties":{"id":{"source_id":"t","sequence":1},"occurred_at":"2026-01-01T14:25:00Z","provider":"docker","subject":{"type":"snapshot","name":"buildpack-deps:noble"},"type":"operation_failed","action":"create","duration":{"secs":1,"nanos":0},"error":{"kind":"provider","message":"pull failed","retryable":false,"causes":[]}}}"#; - let result = format_event_pretty(line, &styles).unwrap(); - assert!( - result.contains("Sandbox snapshot buildpack-deps:noble failed: pull failed"), - "got: {result}" - ); - } - - #[test] - fn pretty_stream_suppresses_metadata_compat_notice_only() { - let styles = no_color_styles(); - let failed = r#"{"ts":"2026-01-01T14:25:00Z","event":"metadata.snapshot.failed","properties":{"phase":"checkpoint","branch":"fabro/meta","duration_ms":900,"failure_kind":"write","error":"write failed"}}"#; - let compat_notice = serde_json::json!({ - "ts": "2026-01-01T14:25:01Z", - "event": "run.notice", - "properties": { - "level": "warn", - "code": RunNoticeCode::CheckpointMetadataWriteFailed, - "message": "legacy metadata warning", - }, - }) - .to_string(); - let degraded_notice = serde_json::json!({ - "ts": "2026-01-01T14:25:02Z", - "event": "run.notice", - "properties": { - "level": "warn", - "code": RunNoticeCode::CheckpointMetadataDegraded, - "message": "metadata snapshots disabled", - }, - }) - .to_string(); - let mut state = PrettyEventState::default(); - - assert!(format_event_pretty_streamed(failed, &styles, &mut state).is_some()); - assert!(format_event_pretty_streamed(&compat_notice, &styles, &mut state).is_none()); - let degraded = format_event_pretty_streamed(°raded_notice, &styles, &mut state).unwrap(); - assert!( - degraded.contains("metadata snapshots disabled"), - "got: {degraded}" - ); - } - - #[test] - fn pretty_workflow_run_failed() { - let styles = no_color_styles(); - let line = r#"{"ts":"2026-01-01T14:23:32Z","run_id":"abc123","event":"run.failed","properties":{"failure":{"reason":"workflow_error","detail":{"message":"sandbox timeout","category":"deterministic"}}}}"#; - let result = format_event_pretty(line, &styles).unwrap(); - assert!(result.contains("Failed"), "got: {result}"); - assert!(result.contains("sandbox timeout"), "got: {result}"); - } - - #[test] - fn pretty_setup_completed_without_command_count() { - let styles = no_color_styles(); - let line = r#"{"ts":"2026-01-01T14:23:32Z","event":"setup.completed","properties":{"duration_ms":800}}"#; - let result = format_event_pretty(line, &styles).unwrap(); - assert!(result.contains("Setup:"), "got: {result}"); - assert!(result.contains("800ms"), "got: {result}"); - assert!(!result.contains("0 commands"), "got: {result}"); - } - - #[test] - fn format_duration_ms_subsecond() { - assert_eq!(format_duration_ms(Some(&serde_json::json!(500))), "500ms"); - } - - #[test] - fn format_duration_ms_seconds() { - assert_eq!(format_duration_ms(Some(&serde_json::json!(8000))), "8s"); - } - - #[test] - fn format_duration_ms_minutes() { - assert_eq!(format_duration_ms(Some(&serde_json::json!(90000))), "1.5m"); - } - - #[test] - fn format_tokens_small() { - assert_eq!(format_tokens(500), "500 toks"); - } - - #[test] - fn format_tokens_thousands() { - assert_eq!(format_tokens(15200), "15.2k toks"); - } - - #[test] - fn truncate_short_string() { - assert_eq!(truncate("hello", 10), "hello"); - } - - #[test] - fn truncate_long_string() { - let result = truncate("a very long command string here", 15); - assert!(result.chars().count() <= 15, "got: {result}"); - assert!(result.ends_with('\u{2026}')); - } } diff --git a/lib/apps/fabro-cli/src/commands/run/petri_worker.rs b/lib/apps/fabro-cli/src/commands/run/petri_worker.rs index a67ad14cf..6917b899f 100644 --- a/lib/apps/fabro-cli/src/commands/run/petri_worker.rs +++ b/lib/apps/fabro-cli/src/commands/run/petri_worker.rs @@ -1,12 +1,11 @@ //! A Petri run in the worker process. //! -//! When `fabro run __run-worker` finds that its run's stored spec names -//! Petri as the engine, the run executes here instead of through the legacy -//! executor, over the same worker services: the authenticated client, the -//! control channel the server pushes cancels through, the signal handlers, -//! the vault snapshot and the CLI catalog. The engine assembly itself is -//! `fabro_petri::engine`, shared with the server's in-process test path, so -//! the run gets the same runtime, options and interviewer either way. +//! `fabro run __run-worker` executes its run here, over the worker services: +//! the authenticated client, the control channel the server pushes cancels +//! through, the signal handlers, the vault snapshot and the CLI catalog. The +//! engine assembly itself is `fabro_petri::engine`, shared with the server's +//! in-process test path, so the run gets the same runtime, options and +//! interviewer either way. //! //! The run's record is [`HttpRunStore`] over the worker's client, leased //! for this launch: the worker mints one owner id at start, logs it, and @@ -31,8 +30,7 @@ //! projection agree with Petri's own `run.paused` and `run.unpaused` //! records. A resumed run that was paused when its worker died comes back //! paused, and the mirror reports that too. The interrupt and pair -//! controls have no Petri adapter yet and are ignored with a warning; the -//! `SIGUSR1`/`SIGUSR2` pause signals reach only the legacy executor. A +//! controls have no Petri adapter yet and are ignored with a warning. A //! control channel that is lost for good cancels the run the same way, and //! the worker exits with that loss as its error once the run has settled. //! @@ -59,7 +57,7 @@ use std::path::{Path, PathBuf}; use std::sync::Arc; use std::time::Instant; -use anyhow::{Context, Result, anyhow, bail}; +use anyhow::{Context, Result, anyhow}; use fabro_auth::VaultCredentialSource; use fabro_client::{Client, ServerTarget}; use fabro_interview::{ControlInterviewer, WorkerControlMessage}; @@ -81,7 +79,6 @@ use fabro_types::{FailureReason, RunId, RunNoticeLevel, RunTiming, StageOutcome, use fabro_vault::Vault; use fabro_workflow::Error as WorkflowError; use fabro_workflow::event::{self as workflow_event, Event, RunEventSink}; -use fabro_workflow::run_control::RunControlState; use fabro_workflow::runtime_store::RunStoreHandle; use fabro_workflow::services::FabroRunToolServices; use tokio::sync::RwLock as AsyncRwLock; @@ -89,7 +86,7 @@ use tokio::task::JoinHandle; use tokio_util::sync::CancellationToken; use tracing::{info, warn}; -use super::runner::{self, WorkerControls, WorkerTitlePhase}; +use super::runner::{self, WorkerTitlePhase}; use crate::args::RunWorkerMode; use crate::command_context; @@ -116,9 +113,7 @@ pub(super) struct PetriWorker<'a> { /// worker exits as the legacy worker does for a failed run. pub(super) async fn execute(worker: PetriWorker<'_>) -> Result<()> { let run_id = worker.run_id; - let Some(admission) = worker.run_state.spec.engine.petri().cloned() else { - bail!("run {run_id} names Petri as its engine but carries no admission"); - }; + let admission = worker.run_state.spec.admission.clone(); let owner = OwnerId::mint(); info!( run_id = %run_id, @@ -132,26 +127,21 @@ pub(super) async fn execute(worker: PetriWorker<'_>) -> Result<()> { )); let cancel_token = CancellationToken::new(); - let run_control = RunControlState::new(); - runner::install_signal_handlers(Arc::clone(&run_control), cancel_token.clone())?; + runner::install_signal_handlers(cancel_token.clone())?; let interviewer = Arc::new(ControlInterviewer::new()); let sink = RunEventSink::map( runner::stamp_system_worker, RunEventSink::backend(worker.run_store.clone()), ); let controls = RunControls::new(); - let petri_controls = Arc::new(PetriControls { - run_id, - controls: controls.clone(), - sink: sink.clone(), - }); + let petri_controls = Arc::new(PetriControls::new(run_id, controls.clone(), sink.clone())); let mut control_manager = runner::spawn_worker_control_manager( worker.target.clone(), run_id, worker.worker_token.to_owned(), Arc::clone(&interviewer), cancel_token.clone(), - WorkerControls::Petri(petri_controls), + petri_controls, ); control_manager.wait_for_first_connection().await?; let approval = if worker.run_state.spec.settings.run.execution.approval == ApprovalMode::Auto { @@ -309,6 +299,20 @@ pub(super) struct PetriControls { } impl PetriControls { + pub(super) fn new(run_id: RunId, controls: RunControls, sink: RunEventSink) -> Self { + Self { + run_id, + controls, + sink, + } + } + + /// The run's controls, for a test that reads the paused state back. + #[cfg(test)] + pub(super) fn controls(&self) -> &RunControls { + &self.controls + } + pub(super) async fn apply(&self, message: WorkerControlMessage) { match message { WorkerControlMessage::RunPause => { diff --git a/lib/apps/fabro-cli/src/commands/run/run_progress/event.rs b/lib/apps/fabro-cli/src/commands/run/run_progress/event.rs index 93a7fc7cb..37bb18d81 100644 --- a/lib/apps/fabro-cli/src/commands/run/run_progress/event.rs +++ b/lib/apps/fabro-cli/src/commands/run/run_progress/event.rs @@ -374,6 +374,7 @@ fn parallel_branch_display(node_id: &str, index: usize, item_label: Option<&str> ) } +#[cfg(test)] pub(super) fn from_json_line(line: &str) -> Option { let stored = RunEvent::from_json_str(line).ok()?; from_run_event(&stored) diff --git a/lib/apps/fabro-cli/src/commands/run/run_progress/mod.rs b/lib/apps/fabro-cli/src/commands/run/run_progress/mod.rs index 020ea4332..0c27c994c 100644 --- a/lib/apps/fabro-cli/src/commands/run/run_progress/mod.rs +++ b/lib/apps/fabro-cli/src/commands/run/run_progress/mod.rs @@ -13,7 +13,9 @@ mod setup_display; mod stage_display; mod styles; -use event::{ProgressEvent, from_json_line, from_run_event}; +#[cfg(test)] +use event::from_json_line; +use event::{ProgressEvent, from_run_event}; use info_display::InfoDisplay; use petri::PetriProgressState; use renderer::ProgressRenderer; @@ -93,6 +95,7 @@ impl ProgressUI { } } + #[cfg(test)] pub(crate) fn handle_json_line(&mut self, line: &str) { if let Some(progress_event) = from_json_line(line) { self.dispatch(progress_event); diff --git a/lib/apps/fabro-cli/src/commands/run/runner.rs b/lib/apps/fabro-cli/src/commands/run/runner.rs index 14acc08c4..d3b4f1092 100644 --- a/lib/apps/fabro-cli/src/commands/run/runner.rs +++ b/lib/apps/fabro-cli/src/commands/run/runner.rs @@ -6,7 +6,7 @@ use std::time::Duration; use anyhow::{Context, Result, anyhow}; use async_trait::async_trait; use fabro_client::ServerTarget; -use fabro_config::{ServerSettingsBuilder, Storage}; +use fabro_config::Storage; use fabro_interview::{ AnswerSubmission, ControlInterviewer, WORKER_CONTROL_INVALID_CURSOR_REASON, WORKER_CONTROL_PONG_TIMEOUT_REASON, WORKER_CONTROL_WS_LIVENESS_TIMEOUT, @@ -16,13 +16,8 @@ use fabro_interview::{ use fabro_manifest::SuppliedWorkflowVersionPackager; use fabro_store::{EventEnvelope, RunProjection, RunProjectionReducer}; use fabro_tool::fabro_client::ClientBackend; -use fabro_types::settings::run::{RunMode, RunNamespace}; -use fabro_types::{ArtifactUpload, BlobHash, EventBody, FailureReason, Principal, RunEvent, RunId}; +use fabro_types::{BlobHash, Principal, RunEvent, RunId}; use fabro_vault::{SecretStore, Vault}; -use fabro_workflow::artifact_upload::{ArtifactSink, StageArtifactUploader}; -use fabro_workflow::event::{Emitter, RunEventSink}; -use fabro_workflow::operations::{self, StartServices}; -use fabro_workflow::run_control::RunControlState; use fabro_workflow::runtime_store::{RunStoreBackend, RunStoreHandle}; use fabro_workflow::services::FabroRunToolServices; use futures::{SinkExt, StreamExt}; @@ -46,8 +41,7 @@ use tokio_util::sync::CancellationToken; use super::petri_worker::{self, PetriControls, PetriWorker}; use crate::args::RunWorkerMode; -use crate::shared::github::build_github_credentials; -use crate::{command_context, server_client}; +use crate::server_client; const RUN_STORE_RETRY_DELAYS: [Duration; 3] = [ Duration::from_millis(50), @@ -59,9 +53,7 @@ const RUN_STORE_RETRY_DELAYS: [Duration; 3] = [ pub(super) enum WorkerTitlePhase { Start, Resume, - Init, Running, - Waiting, Paused, Succeeded, Failed, @@ -87,125 +79,19 @@ pub(crate) async fn execute( .state() .await .with_context(|| format!("failed to load run state for {run_id}"))?; - if run_state.spec.engine.is_petri() { - return Box::pin(petri_worker::execute(PetriWorker { - run_id, - target, - client, - run_store, - run_state, - storage_dir: &storage_dir, - run_dir, - mode, - fabro_home, - worker_token, - })) - .await; - } - let run_spec = &run_state.spec; - let catalog = Arc::new( - command_context::load_cli_catalog().context("failed to build worker LLM catalog")?, - ); - let artifact_sink = Some(ArtifactSink::Uploader(build_artifact_uploader( + Box::pin(petri_worker::execute(PetriWorker { run_id, - client.clone_for_reuse(), - worker_token.to_owned(), - ))); - let fabro_run_tools = if fabro_run_tools_enabled_from_worker_token(worker_token) { - build_fabro_run_tool_services(worker_token, client.clone_for_reuse(), run_id) - } else { - None - }; - let interviewer = Arc::new(ControlInterviewer::new()); - let cancel_token = CancellationToken::new(); - let emitter = Arc::new(Emitter::new(run_id)); - let steering_hub = Arc::new(fabro_workflow::SteeringHub::new(Arc::clone(&emitter))); - let run_control = RunControlState::new(); - install_signal_handlers(Arc::clone(&run_control), cancel_token.clone())?; - let mut control_manager = if run_state.status.is_terminal() { - None - } else { - Some(spawn_worker_control_manager( - target.clone(), - run_id, - worker_token.to_owned(), - Arc::clone(&interviewer), - cancel_token.clone(), - WorkerControls::Legacy { - steering_hub: Arc::clone(&steering_hub), - run_control: Arc::clone(&run_control), - }, - )) - }; - if let Some(control_manager) = &mut control_manager { - control_manager.wait_for_first_connection().await?; - } - let vault = load_worker_vault(&storage_dir).await?; - let github_app = { - let vault_guard = vault.read().await; - maybe_build_github_credentials(run_spec, &vault_guard)? - }; - let sandbox_providers = ServerSettingsBuilder::load_default() - .map(|settings| settings.server.sandbox.providers) - .unwrap_or_default(); - let services = StartServices { - run_id, - cancel_token: cancel_token.clone(), - emitter, - interviewer, - steering_hub, - run_store: run_store.clone(), - event_sink: RunEventSink::map( - stamp_system_worker, - RunEventSink::fanout(vec![ - RunEventSink::backend(run_store), - RunEventSink::callback(move |event| { - update_worker_title_from_event(&event); - async move { Ok(()) } - }), - ]), - ), - artifact_sink, - run_control: Some(run_control), - github_app, - github_integration: run_spec - .settings - .run - .integrations - .github - .resolve_integration() - .context("failed to resolve github integration")?, - vault, - sandbox_providers, - catalog, - on_node: None, - registry_override: None, - fabro_run_tools, - }; - - let execution = async { - match mode { - RunWorkerMode::Start => operations::start(&run_dir, services).await, - RunWorkerMode::Resume => operations::resume(&run_dir, services).await, - } - }; - - if let Some(mut control_manager) = control_manager { - tokio::select! { - result = execution => { - control_manager.finish(); - result?; - } - fatal = control_manager.fatal_control_loss() => { - control_manager.finish(); - return Err(fatal); - } - } - } else { - execution.await?; - } - - Ok(()) + target, + client, + run_store, + run_state, + storage_dir: &storage_dir, + run_dir, + mode, + fabro_home, + worker_token, + })) + .await } const WORKER_TOKEN_SCOPE: &str = "run:worker"; @@ -305,16 +191,9 @@ impl AppliedWorkerControlDeliveryIds { } } -/// Where the run's pause, unpause, steer and pair controls go: the legacy -/// executor's hub and pause flag, or the Petri run's controls. Cancel and -/// answers are applied by the channel itself, the same way for both. -pub(super) enum WorkerControls { - Legacy { - steering_hub: Arc, - run_control: Arc, - }, - Petri(Arc), -} +/// Where the run's pause, unpause and steer controls go: the Petri run's +/// controls. Cancel and answers are applied by the channel itself. +pub(super) type WorkerControls = Arc; pub(super) struct WorkerControlManagerHandle { first_connection: Option>>, @@ -780,124 +659,7 @@ async fn apply_worker_control_message( cancel_token.cancel(); interviewer.interrupt_all().await; } - other => match controls { - WorkerControls::Legacy { - steering_hub, - run_control, - } => apply_legacy_control(steering_hub, run_control, other), - WorkerControls::Petri(petri) => petri.apply(other).await, - }, - } -} - -/// The legacy executor's pause flag and steering hub. -fn apply_legacy_control( - steering_hub: &fabro_workflow::SteeringHub, - run_control: &RunControlState, - message: WorkerControlMessage, -) { - match message { - WorkerControlMessage::InterviewAnswer { .. } | WorkerControlMessage::RunCancel => {} - WorkerControlMessage::RunPause => { - run_control.request_pause(); - } - WorkerControlMessage::RunUnpause => { - run_control.request_unpause(); - } - WorkerControlMessage::Steer { text, actor } => { - steering_hub.deliver_steer(text, Some(actor)); - } - WorkerControlMessage::Interrupt { actor } => { - steering_hub.interrupt(Some(&actor)); - } - WorkerControlMessage::InterruptThenSteer { text, actor } => { - steering_hub.interrupt_then_steer(&text, Some(&actor)); - } - WorkerControlMessage::PairStart { - run_id, - pair_id, - target, - actor, - } => { - let _ = steering_hub.start_pair(run_id, pair_id, target, Some(actor)); - } - WorkerControlMessage::PairMessage { - pair_id, - message_id, - text, - client_message_id, - actor, - } => { - let _ = steering_hub.send_pair_message( - pair_id, - message_id, - text, - client_message_id, - Some(actor), - ); - } - WorkerControlMessage::PairEnd { pair_id, actor } => { - let _ = steering_hub.end_pair(pair_id, Some(actor)); - } - } -} - -fn build_artifact_uploader( - run_id: RunId, - client: server_client::Client, - worker_token: String, -) -> Arc { - Arc::new(HttpArtifactUploader { - run_id, - client, - worker_token, - }) -} - -struct HttpArtifactUploader { - run_id: RunId, - client: server_client::Client, - worker_token: String, -} - -#[async_trait] -impl StageArtifactUploader for HttpArtifactUploader { - async fn upload_stage_artifacts( - &self, - stage_id: &fabro_types::StageId, - retry: u32, - artifact_capture_dir: &Path, - artifacts: &[ArtifactUpload], - ) -> Result<()> { - if artifacts.is_empty() { - return Ok(()); - } - - if artifacts.len() == 1 { - let artifact = &artifacts[0]; - return self - .client - .upload_stage_artifact_file( - &self.run_id, - stage_id, - retry, - &artifact.path, - &artifact_capture_dir.join(&artifact.path), - &self.worker_token, - ) - .await; - } - - self.client - .upload_stage_artifact_batch( - &self.run_id, - stage_id, - retry, - artifact_capture_dir, - artifacts, - &self.worker_token, - ) - .await + other => controls.apply(other).await, } } @@ -1061,9 +823,7 @@ fn worker_title(run_id: &RunId, phase: WorkerTitlePhase) -> String { let phase = match phase { WorkerTitlePhase::Start => "start", WorkerTitlePhase::Resume => "resume", - WorkerTitlePhase::Init => "init", WorkerTitlePhase::Running => "running", - WorkerTitlePhase::Waiting => "waiting", WorkerTitlePhase::Paused => "paused", WorkerTitlePhase::Succeeded => "succeeded", WorkerTitlePhase::Failed => "failed", @@ -1072,31 +832,6 @@ fn worker_title(run_id: &RunId, phase: WorkerTitlePhase) -> String { format!("fabro {short_id} {phase}") } -fn worker_title_phase_for_event(body: &EventBody) -> Option { - match body { - EventBody::RunStarting(_) => Some(WorkerTitlePhase::Init), - EventBody::RunRunning(_) | EventBody::RunUnpaused(_) => Some(WorkerTitlePhase::Running), - EventBody::InterviewStarted(_) => Some(WorkerTitlePhase::Waiting), - EventBody::InterviewCompleted(_) | EventBody::InterviewTimeout(_) => { - Some(WorkerTitlePhase::Running) - } - EventBody::RunPaused(_) => Some(WorkerTitlePhase::Paused), - EventBody::RunCompleted(_) => Some(WorkerTitlePhase::Succeeded), - EventBody::RunFailed(props) => Some(if props.failure.reason == FailureReason::Cancelled { - WorkerTitlePhase::Cancelled - } else { - WorkerTitlePhase::Failed - }), - _ => None, - } -} - -fn update_worker_title_from_event(event: &RunEvent) { - if let Some(phase) = worker_title_phase_for_event(&event.body) { - set_worker_title(&event.run_id, phase); - } -} - pub(super) fn stamp_system_worker(mut event: RunEvent) -> RunEvent { if event.actor.is_none() { event.actor = Some(Principal::Worker { @@ -1106,77 +841,10 @@ pub(super) fn stamp_system_worker(mut event: RunEvent) -> RunEvent { event } -fn maybe_build_github_credentials( - run_spec: &fabro_types::RunSpec, - vault: &fabro_vault::Vault, -) -> Result> { - let resolved_run = &run_spec.settings.run; - let has_repo_origin = run_spec - .repo_origin_url() - .is_some_and(|origin| !origin.trim().is_empty()); - let resolved_server = ServerSettingsBuilder::load_default().ok(); - let server_ns = resolved_server.as_ref().map(|s| &s.server); - let strategy = server_ns - .map(|server| server.integrations.github.strategy) - .unwrap_or_default(); - let app_id = server_ns.and_then(|server| server.integrations.github.app_id.clone()); - let app_slug = server_ns.and_then(|server| server.integrations.github.slug.clone()); - - if requires_github_credentials(resolved_run, has_repo_origin) { - return build_github_credentials(strategy, app_id.as_deref(), app_slug.as_deref(), vault); - } - - let pull_request_enabled = - resolved_run.execution.mode != RunMode::DryRun && resolved_run.pull_request.is_some(); - if pull_request_enabled { - return Ok(build_github_credentials( - strategy, - app_id.as_deref(), - app_slug.as_deref(), - vault, - ) - .ok() - .flatten()); - } - - Ok(None) -} - -/// Hard-gate for the CLI worker path: a run-level token is requested, or -/// a clone-based sandbox in non-dry-run mode will clone a repository and -/// needs credentials to pull it. A run without a repository origin creates -/// an empty workspace and needs none. Pull-request-driven credential -/// acquisition is handled separately by the caller as a soft fallback. -fn requires_github_credentials(run: &RunNamespace, has_repo_origin: bool) -> bool { - if run.integrations.github.is_token_requested() { - return true; - } - run.execution.mode != RunMode::DryRun - && run.environment.provider.clones_workspace() - && has_repo_origin -} - -pub(super) fn install_signal_handlers( - run_control: Arc, - cancel_token: CancellationToken, -) -> Result<()> { +/// `SIGTERM` and `SIGINT` cancel the run, the way the server's cancel does. +pub(super) fn install_signal_handlers(cancel_token: CancellationToken) -> Result<()> { #[cfg(unix)] { - let mut pause = signal(SignalKind::user_defined1())?; - let pause_control = Arc::clone(&run_control); - tokio::spawn(async move { - while pause.recv().await.is_some() { - pause_control.request_pause(); - } - }); - - let mut unpause = signal(SignalKind::user_defined2())?; - tokio::spawn(async move { - while unpause.recv().await.is_some() { - run_control.request_unpause(); - } - }); - let mut terminate = signal(SignalKind::terminate())?; let terminate_cancel = cancel_token.clone(); tokio::spawn(async move { @@ -1211,41 +879,34 @@ mod tests { use fabro_interview::{ AnswerValue, ControlInterviewer, Interviewer, Question, WorkerControlEnvelope, }; - use fabro_types::run_event::{ - InterviewCompletedProps, InterviewStartedProps, RunCompletedProps, RunControlEffectProps, - RunFailedProps, RunStatusTransitionProps, - }; - use fabro_types::{ - AuthMethod, EventBody, FailureCategory, FailureDetail, FailureReason, IdpIdentity, - Principal, QuestionType, RunFailure, SuccessReason, fixtures, - }; + use fabro_types::run_event::RunStatusTransitionProps; + use fabro_types::{AuthMethod, EventBody, IdpIdentity, Principal, QuestionType, fixtures}; use fabro_vault::{SecretType, Vault}; use fabro_workflow::event::RunEventSink; - use fabro_workflow::run_control::RunControlState; use tokio::time; use tokio_tungstenite::tungstenite::protocol::{Message as TestWebSocketMessage, Role}; use tokio_util::sync::CancellationToken; + use super::super::petri_worker::PetriControls; use super::{ AppliedWorkerControlDeliveryIds, WorkerControlConnectError, WorkerControlSocket, WorkerControls, WorkerTitlePhase, apply_worker_control_delivery_frame, apply_worker_control_message, build_worker_control_stream_request, connect_worker_control_stream, handle_worker_control_socket, initial_worker_title_phase, load_worker_vault, next_worker_control_reconnect_backoff, stamp_system_worker, - worker_title, worker_title_phase_for_event, + worker_title, }; use crate::args::RunWorkerMode; - fn test_steering_hub() -> Arc { - let emitter = Arc::new(fabro_workflow::event::Emitter::new(fixtures::RUN_1)); - Arc::new(fabro_workflow::SteeringHub::new(emitter)) - } - - fn test_controls(run_control: &Arc) -> WorkerControls { - WorkerControls::Legacy { - steering_hub: test_steering_hub(), - run_control: Arc::clone(run_control), - } + /// A run's controls over a sink that keeps nothing: what the channel + /// tests drive. + fn test_controls() -> WorkerControls { + let sink = RunEventSink::callback(|_event| async move { Ok(()) }); + Arc::new(PetriControls::new( + fixtures::RUN_1, + fabro_petri::controls::RunControls::new(), + sink, + )) } #[test] @@ -1342,82 +1003,6 @@ mod tests { ); } - #[test] - fn worker_title_phase_tracks_lifecycle_events() { - assert_eq!( - worker_title_phase_for_event(&EventBody::RunStarting(RunStatusTransitionProps {})), - Some(WorkerTitlePhase::Init) - ); - assert_eq!( - worker_title_phase_for_event(&EventBody::RunPaused(RunControlEffectProps::default())), - Some(WorkerTitlePhase::Paused) - ); - assert_eq!( - worker_title_phase_for_event(&EventBody::InterviewStarted(InterviewStartedProps { - question_id: "q-1".to_string(), - question: "Approve?".to_string(), - stage: "gate".to_string(), - question_type: "yes_no".to_string(), - options: Vec::new(), - allow_freeform: false, - timeout_seconds: None, - context_display: None, - review_target: None, - })), - Some(WorkerTitlePhase::Waiting) - ); - assert_eq!( - worker_title_phase_for_event(&EventBody::InterviewCompleted(InterviewCompletedProps { - question_id: "q-1".to_string(), - question: "Approve?".to_string(), - answer: "yes".to_string(), - duration_ms: 10, - })), - Some(WorkerTitlePhase::Running) - ); - assert_eq!( - worker_title_phase_for_event(&EventBody::RunCompleted(RunCompletedProps { - timing: fabro_types::RunTiming::wall_only(10), - artifact_count: 0, - status: "succeeded".to_string(), - reason: SuccessReason::Completed, - final_git_commit_sha: None, - final_patch: None, - diff_summary: None, - usage: None, - })), - Some(WorkerTitlePhase::Succeeded) - ); - assert_eq!( - worker_title_phase_for_event(&EventBody::RunFailed(RunFailedProps { - failure: RunFailure { - reason: FailureReason::Cancelled, - detail: FailureDetail::new("cancelled", FailureCategory::Canceled), - }, - timing: fabro_types::RunTiming::wall_only(10), - final_git_commit_sha: None, - final_patch: None, - diff_summary: None, - usage: None, - })), - Some(WorkerTitlePhase::Cancelled) - ); - assert_eq!( - worker_title_phase_for_event(&EventBody::RunFailed(RunFailedProps { - failure: RunFailure { - reason: FailureReason::Terminated, - detail: FailureDetail::new("boom", FailureCategory::Deterministic), - }, - timing: fabro_types::RunTiming::wall_only(10), - final_git_commit_sha: None, - final_patch: None, - diff_summary: None, - usage: None, - })), - Some(WorkerTitlePhase::Failed) - ); - } - #[test] fn stamp_system_worker_fills_missing_actor_only() { let stamped = stamp_system_worker(running_event(None)); @@ -1483,13 +1068,12 @@ mod tests { async fn worker_control_routes_answer_by_question_id() { let interviewer = Arc::new(ControlInterviewer::new()); let cancel_token = CancellationToken::new(); - let run_control = RunControlState::new(); let mut question = Question::new("Approve?", QuestionType::YesNo); question.id = "q-1".to_string(); let ask_interviewer = Arc::clone(&interviewer); let answer_task = tokio::spawn(async move { ask_interviewer.ask(question).await }); - let controls = test_controls(&run_control); + let controls = test_controls(); apply_worker_control_message( &interviewer, &cancel_token, @@ -1513,14 +1097,13 @@ mod tests { async fn worker_control_cancel_sets_cancel_token_and_interrupts_pending_interviews() { let interviewer = Arc::new(ControlInterviewer::new()); let cancel_token = CancellationToken::new(); - let run_control = RunControlState::new(); let mut question = Question::new("Approve?", QuestionType::YesNo); question.id = "q-1".to_string(); let ask_interviewer = Arc::clone(&interviewer); let answer_task = tokio::spawn(async move { ask_interviewer.ask(question).await }); tokio::task::yield_now().await; - let controls = test_controls(&run_control); + let controls = test_controls(); apply_worker_control_message( &interviewer, &cancel_token, @@ -1535,11 +1118,10 @@ mod tests { } #[tokio::test] - async fn worker_control_pause_and_unpause_route_to_run_control() { + async fn worker_control_pause_and_unpause_route_to_the_run_controls() { let interviewer = Arc::new(ControlInterviewer::new()); let cancel_token = CancellationToken::new(); - let run_control = RunControlState::new(); - let controls = test_controls(&run_control); + let controls = test_controls(); apply_worker_control_message( &interviewer, @@ -1548,7 +1130,7 @@ mod tests { WorkerControlEnvelope::pause_run(), ) .await; - assert!(run_control.pause_requested()); + assert!(controls.controls().is_paused()); apply_worker_control_message( &interviewer, @@ -1557,15 +1139,14 @@ mod tests { WorkerControlEnvelope::unpause_run(), ) .await; - assert!(!run_control.pause_requested()); + assert!(!controls.controls().is_paused()); } #[tokio::test] async fn duplicate_delivery_ids_are_not_applied_twice() { let interviewer = Arc::new(ControlInterviewer::new()); let cancel_token = CancellationToken::new(); - let run_control = RunControlState::new(); - let controls = test_controls(&run_control); + let controls = test_controls(); let mut applied_ids = AppliedWorkerControlDeliveryIds::default(); let frame = fabro_interview::WorkerControlDeliveryFrame { id: "local:1".to_string(), @@ -1672,8 +1253,7 @@ mod tests { let mut socket = WorkerControlSocket::Test(Box::new(worker_ws)); let interviewer = Arc::new(ControlInterviewer::new()); let cancel_token = CancellationToken::new(); - let run_control = RunControlState::new(); - let controls = test_controls(&run_control); + let controls = test_controls(); let mut applied_ids = AppliedWorkerControlDeliveryIds::default(); let done = CancellationToken::new(); @@ -1747,79 +1327,4 @@ mod tests { assert!(credential.contains("vault-key")); } - - mod requires_github_credentials_truth_table { - //! Truth-table coverage for the worker-side credential gate. - //! `InterpString` → `String` resolution is tested in `fabro-types` - //! next to `RunIntegrationsGithubSettings::resolve_permissions`. - - use std::collections::HashMap; - - use fabro_types::SandboxProviderKind; - use fabro_types::settings::InterpString; - use fabro_types::settings::run::{ - RunIntegrationsGithubSettings, RunIntegrationsSettings, RunMode, RunNamespace, - }; - - use super::super::requires_github_credentials; - - fn run_with( - permissions: HashMap, - provider: &str, - mode: RunMode, - ) -> RunNamespace { - let mut run = RunNamespace::default(); - run.execution.mode = mode; - run.environment.provider = provider - .parse::() - .expect("test provider should parse"); - run.integrations = RunIntegrationsSettings { - github: RunIntegrationsGithubSettings { - permissions, - ..RunIntegrationsGithubSettings::default() - }, - }; - run - } - - #[test] - fn requires_github_credentials_when_permissions_non_empty() { - let permissions = HashMap::from([("issues".to_string(), InterpString::parse("read"))]); - // Even with local sandbox + dry-run, non-empty permissions - // force credential acquisition. - let run = run_with(permissions, "local", RunMode::DryRun); - assert!(requires_github_credentials(&run, false)); - } - - #[test] - fn requires_github_credentials_for_clone_based_provider_with_an_origin() { - let run = run_with(HashMap::new(), "docker", RunMode::Normal); - assert!(requires_github_credentials(&run, true)); - - let daytona = run_with(HashMap::new(), "daytona", RunMode::Normal); - assert!(requires_github_credentials(&daytona, true)); - - let plugin = run_with(HashMap::new(), "host", RunMode::Normal); - assert!(requires_github_credentials(&plugin, true)); - } - - #[test] - fn does_not_require_github_credentials_without_a_repository_origin() { - // A `none` target creates an empty workspace; nothing is cloned. - let run = run_with(HashMap::new(), "docker", RunMode::Normal); - assert!(!requires_github_credentials(&run, false)); - } - - #[test] - fn does_not_require_github_credentials_for_local_clean_run() { - let run = run_with(HashMap::new(), "local", RunMode::Normal); - assert!(!requires_github_credentials(&run, true)); - } - - #[test] - fn does_not_require_github_credentials_for_clone_provider_in_dry_run() { - let run = run_with(HashMap::new(), "docker", RunMode::DryRun); - assert!(!requires_github_credentials(&run, true)); - } - } } diff --git a/lib/apps/fabro-cli/src/commands/server/start.rs b/lib/apps/fabro-cli/src/commands/server/start.rs index de35cb1b4..233465885 100644 --- a/lib/apps/fabro-cli/src/commands/server/start.rs +++ b/lib/apps/fabro-cli/src/commands/server/start.rs @@ -15,7 +15,6 @@ use fabro_server::jwt_auth::auth_method_name; use fabro_server::serve::{DEFAULT_TCP_PORT, ServeArgs, resolve_runtime_server_settings_for_start}; use fabro_server::{process_env_snapshot, validate_startup, validate_startup_configuration}; use fabro_static::EnvVars; -use fabro_types::Engine; use fabro_types::settings::{LogDestination, ServerAuthMethod}; use fabro_util::printer::Printer; use fabro_util::terminal::Styles; @@ -151,7 +150,6 @@ async fn ensure_server_running_with_bind( provider: None, environment: None, max_concurrent_runs: server_max_concurrent_runs_override(), - engine: server_engine_override()?, config: Some(config_path.to_path_buf()), #[cfg(debug_assertions)] watch_web: false, @@ -226,25 +224,6 @@ fn server_max_concurrent_runs_override() -> Option { .filter(|value| *value > 0) } -/// `FABRO_SERVER_ENGINE` names the engine for runs whose workflow version -/// names none; a value that is not an engine is an error rather than a -/// silent fallback to the legacy executor. -fn server_engine_override() -> Result> { - let Some(value) = std::env::var_os(EnvVars::FABRO_SERVER_ENGINE) else { - return Ok(None); - }; - let value = value.to_string_lossy(); - if value.trim().is_empty() { - return Ok(None); - } - value.trim().parse::().map(Some).map_err(|_| { - anyhow!( - "{} is `{value}`, which is not an engine (expected `legacy` or `petri`)", - EnvVars::FABRO_SERVER_ENGINE - ) - }) -} - fn configured_auth_methods(config_path: Option<&Path>) -> Vec { local_server::LocalServerConfig::load(config_path, None) .ok() @@ -356,9 +335,6 @@ async fn execute_daemon( if let Some(max) = serve_args.max_concurrent_runs { cmd.args(["--max-concurrent-runs", &max.to_string()]); } - if let Some(engine) = serve_args.engine { - cmd.args(["--engine", &engine.to_string()]); - } if let Some(ref config) = serve_args.config { cmd.arg("--config").arg(config); } @@ -622,7 +598,6 @@ destination = "{destination}" provider: None, environment: None, max_concurrent_runs: None, - engine: None, config: Some(config_path.to_path_buf()), #[cfg(debug_assertions)] watch_web: false, diff --git a/lib/apps/fabro-cli/src/server_client.rs b/lib/apps/fabro-cli/src/server_client.rs index d8ad0a9cc..f25ab00c6 100644 --- a/lib/apps/fabro-cli/src/server_client.rs +++ b/lib/apps/fabro-cli/src/server_client.rs @@ -7,7 +7,7 @@ use fabro_client::{ AuthEntry, AuthStore, Credential, OAuthSession, ServerTarget, TransportConnector, apply_bearer_token_auth, }; -pub(crate) use fabro_client::{Client, RunEventStream, RunStreamItemStream}; +pub(crate) use fabro_client::{Client, RunStreamItemStream}; use fabro_config::Storage; use fabro_config::bind::Bind; pub(crate) use fabro_types::RunProjection; diff --git a/lib/apps/fabro-cli/src/shared/github.rs b/lib/apps/fabro-cli/src/shared/github.rs deleted file mode 100644 index f6e6ec9f0..000000000 --- a/lib/apps/fabro-cli/src/shared/github.rs +++ /dev/null @@ -1,49 +0,0 @@ -use anyhow::anyhow; -use fabro_github::GitHubCredentials; -use fabro_static::EnvVars; -use fabro_types::settings::server::GithubIntegrationStrategy; -use fabro_vault::Vault; - -pub(crate) fn build_github_credentials( - strategy: GithubIntegrationStrategy, - app_id: Option<&str>, - app_slug: Option<&str>, - vault: &Vault, -) -> anyhow::Result> { - match strategy { - GithubIntegrationStrategy::App => { - GitHubCredentials::from_env_with_slug(app_id, app_slug).map_err(|err| anyhow!(err)) - } - GithubIntegrationStrategy::Token => { - let token = lookup_github_token(vault); - match token { - Some(t) => { - fabro_github::validate_static_github_token(&t)?; - Ok(Some(GitHubCredentials::Pat(t))) - } - None => Err(anyhow!( - "GITHUB_TOKEN not configured — run fabro install or set GITHUB_TOKEN" - )), - } - } - } -} - -/// Look up GitHub token: GITHUB_TOKEN env -> vault GITHUB_TOKEN -> GH_TOKEN env -/// -> vault GH_TOKEN -fn lookup_github_token(vault: &Vault) -> Option { - lookup_env_or_vault(EnvVars::GITHUB_TOKEN, vault) - .or_else(|| lookup_env_or_vault(EnvVars::GH_TOKEN, vault)) -} - -#[expect( - clippy::disallowed_methods, - reason = "GitHub credential resolution intentionally falls back from vault to documented process-env names." -)] -fn lookup_env_or_vault(name: &str, vault: &Vault) -> Option { - std::env::var(name) - .ok() - .or_else(|| vault.get(name).map(str::to_string)) - .map(|t| t.trim().to_string()) - .filter(|t| !t.is_empty()) -} diff --git a/lib/apps/fabro-cli/src/shared/mod.rs b/lib/apps/fabro-cli/src/shared/mod.rs index 25e76c1ce..4d850e4db 100644 --- a/lib/apps/fabro-cli/src/shared/mod.rs +++ b/lib/apps/fabro-cli/src/shared/mod.rs @@ -1,4 +1,3 @@ -pub(crate) mod github; pub(crate) mod provider_auth; pub(crate) mod repo; mod utilities; diff --git a/lib/apps/fabro-cli/tests/it/cmd/server_start.rs b/lib/apps/fabro-cli/tests/it/cmd/server_start.rs index da6e866b3..68c1ceb13 100644 --- a/lib/apps/fabro-cli/tests/it/cmd/server_start.rs +++ b/lib/apps/fabro-cli/tests/it/cmd/server_start.rs @@ -212,8 +212,6 @@ fn help() { Named environment for agent tools --max-concurrent-runs Maximum number of concurrent run executions - --engine - The engine for every run whose workflow version names none (`legacy` or `petri`); overrides `[server.execution] engine` --config Path to server config file (default: ~/.fabro/settings.toml) -h, --help diff --git a/lib/apps/fabro-cli/tests/it/scenario/petri.rs b/lib/apps/fabro-cli/tests/it/scenario/petri.rs index f0ad0ec44..865ea8c78 100644 --- a/lib/apps/fabro-cli/tests/it/scenario/petri.rs +++ b/lib/apps/fabro-cli/tests/it/scenario/petri.rs @@ -409,8 +409,8 @@ pub(super) fn write_petri_workflow(context: &fabro_test::TestContext, dot: &str) std::fs::write(workspace.join("workflow.fabro"), dot).expect("the workflow writes"); std::fs::write( workspace.join("workflow.toml"), - "_version = 1\n\n[workflow]\ngraph = \"workflow.fabro\"\nengine = \"petri\"\n\n[run]\ngoal \ - = \"Run one command\"\n", + "_version = 1\n\n[workflow]\ngraph = \"workflow.fabro\"\n\n[run]\ngoal = \"Run one \ + command\"\n", ) .expect("the settings write"); workspace @@ -662,7 +662,10 @@ async fn a_petri_run_executes_in_the_server_launched_worker() { let run = run_json(&server, &format!("runs/{run_id}")).await; assert_eq!(status, "succeeded", "run: {run}"); let state = run_json(&server, &format!("runs/{run_id}/state")).await; - assert_eq!(state["spec"]["engine"]["kind"], "petri", "state: {state}"); + assert!( + state["spec"]["admission"]["graph"]["digest"].is_string(), + "state: {state}" + ); let names = stream_names(&settled_stream(&server, &run_id).await); assert_eq!(count_of(&names, "lifecycle:succeeded"), 1, "{names:?}"); @@ -1201,13 +1204,16 @@ async fn a_finished_petri_run_reads_back_through_the_cli() { let stderr = String::from_utf8_lossy(&wait.stderr); assert!(stderr.contains("Succeeded"), "{stderr}"); - // Inspect reads the projection, whose spec names the engine. + // Inspect reads the projection, whose spec names the admission. let inspect = cli(&context, &server, &["inspect", &run_id]); let inspected: serde_json::Value = serde_json::from_slice(&inspect.stdout).expect("inspect prints JSON"); let entry = &inspected[0]; assert_eq!(entry["run_id"], run_id, "{entry}"); - assert_eq!(entry["run_spec"]["engine"]["kind"], "petri", "{entry}"); + assert!( + entry["run_spec"]["admission"]["graph"]["digest"].is_string(), + "{entry}" + ); assert_eq!(entry["conclusion"]["status"], "succeeded", "{entry}"); server.shutdown(); } diff --git a/lib/apps/fabro-cli/tests/it/support/mod.rs b/lib/apps/fabro-cli/tests/it/support/mod.rs index 9251d1a91..a8b0c7be7 100644 --- a/lib/apps/fabro-cli/tests/it/support/mod.rs +++ b/lib/apps/fabro-cli/tests/it/support/mod.rs @@ -1,4 +1,4 @@ -use fabro_types::test_support; +use fabro_types::{PetriAdmission, test_support}; mod auth_harness; mod auth_tokens; @@ -57,7 +57,7 @@ pub(crate) fn run_projection_json(run_id: &str, status: &serde_json::Value) -> s spec_blob: None, git: None, fork_source_ref: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }; serde_json::json!({ diff --git a/lib/apps/fabro-server/src/demo/mod.rs b/lib/apps/fabro-server/src/demo/mod.rs index 18d6cfeab..5f5b5fbb4 100644 --- a/lib/apps/fabro-server/src/demo/mod.rs +++ b/lib/apps/fabro-server/src/demo/mod.rs @@ -1101,8 +1101,9 @@ mod runs { }; use fabro_types::settings::{InterpString, ProjectNamespace, WorkflowNamespace}; use fabro_types::{ - AuthMethod, IdpIdentity, PendingReason, Principal, RepositoryRef, RunId, RunLifecycle, - RunLinks, RunOrigin, RunSize, RunTimestamps, StageId, WorkflowRef, WorkflowSettings, + AuthMethod, IdpIdentity, PendingReason, PetriAdmission, Principal, RepositoryRef, RunId, + RunLifecycle, RunLinks, RunOrigin, RunSize, RunTimestamps, StageId, WorkflowRef, + WorkflowSettings, }; use lithos_llm::catalog::ProviderId; use lithos_llm::types::{Cost, CostSource, TokenCounts, Usage}; @@ -1746,7 +1747,7 @@ mod runs { spec_blob: None, git: None, fork_source_ref: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }; let mut projection = RunProjection::new( "Detect and fix environment drift".to_string(), diff --git a/lib/apps/fabro-server/src/run_compiler.rs b/lib/apps/fabro-server/src/run_compiler.rs index b4b5cad66..bae1e2c80 100644 --- a/lib/apps/fabro-server/src/run_compiler.rs +++ b/lib/apps/fabro-server/src/run_compiler.rs @@ -11,12 +11,9 @@ //! settings from every configured source, substitute the run-scoped variable //! snapshot, then parse/transform/validate the graph through the //! fabro-workflow pipeline. -//! 3. Model pinning — materialize run-level model settings against the catalog -//! and the configured provider set. Stages 2's graph compilation and stage 3 -//! share one blocking dispatch via [`compile_and_pin`]. A run Petri admitted -//! takes [`compile_admitted`] instead: Petri compiled, linted and pinned -//! models at its own admission, so only the Fabro graph the read side -//! displays is parsed here. +//! 3. [`compile_admitted`] — Petri compiled, linted and pinned models at its +//! admission, so only the Fabro graph the read side displays is parsed here, +//! and the admission is recorded on the run. //! 4. [`assemble_run`] — purely assemble the complete persistence input; no //! field is mutated after assembly. //! @@ -29,28 +26,25 @@ use std::collections::HashMap; use std::path::PathBuf; -use std::sync::Arc; use fabro_config::parse::{self, ParseError, SettingsSource}; use fabro_config::{ EnvironmentDockerfileLayer, EnvironmentImageLayer, EnvironmentLayer, MergeMap, RunLayer, SettingsLayer, WorkflowSettingsBuilder, }; -use fabro_llm::lithos_catalog::Catalog; use fabro_types::settings::interp::{InterpString, ResolveError}; use fabro_types::settings::run::{McpServerSettings, RunGoal}; use fabro_types::{ - AutomationRef, GitContext, ManifestPath, PetriAdmission, RunEngine, RunId, RunProvenance, - RunTarget, WorkflowSettings, WorkflowVersionId, + AutomationRef, GitContext, ManifestPath, PetriAdmission, RunId, RunProvenance, RunTarget, + WorkflowSettings, WorkflowVersionId, }; use fabro_util::workspace_glob::{WorkspaceGlob, WorkspaceGlobError}; use fabro_workflow::Error as WorkflowError; use fabro_workflow::operations::{ - self, CompiledRun, CreateRunCompileInput, CreateRunPersistenceInput, - CreateRunPersistenceMetadata, MaterializedRun, WorkflowInput, + self, CreateRunCompileInput, CreateRunPersistenceInput, CreateRunPersistenceMetadata, + MaterializedRun, WorkflowInput, }; use fabro_workflow::workflow_bundle::{BundledWorkflow, WorkflowBundle}; -use lithos_llm::catalog::ProviderId; use tokio::task; /// Transport-neutral inputs for compiling one submitted run. @@ -139,6 +133,11 @@ impl PreparedRun { &self.layered.settings } + /// The run-variable snapshot, for the engine's `{{ vars.* }}`. + pub(crate) fn vars(&self) -> &HashMap { + &self.vars + } + /// The acquired bundle, for an engine that compiles it itself. pub(crate) fn workflow_bundle(&self) -> &WorkflowBundle { &self.layered.workflow_bundle @@ -178,19 +177,12 @@ impl PreparedRun { } } -/// Graph-compiled stage output, retaining the metadata needed by later pure -/// assembly. -struct GraphCompiledRun { - compiled: CompiledRun, - metadata: RunMetadata, -} - /// Model-pinned stage output ready for pure persistence-input assembly. pub(crate) struct PinnedRun { materialized: MaterializedRun, metadata: RunMetadata, - /// The engine the run was created for, with what it admitted. - engine: RunEngine, + /// What Petri admitted for the run. + admission: PetriAdmission, } #[derive(Debug, thiserror::Error)] @@ -388,27 +380,6 @@ pub(crate) fn apply_run_variables( Ok(PreparedRun { layered, vars }) } -/// Compile and validate the graph, then pin run-level model settings, in one -/// dispatch on Tokio's blocking pool: graph compilation is CPU-heavy and may -/// read a goal file, and pinning is pure CPU that belongs alongside it. -pub(crate) async fn compile_and_pin( - prepared: PreparedRun, - configured_providers: Vec, - catalog: Arc, -) -> Result { - task::spawn_blocking(move || { - let compiled = compile_graph(prepared, configured_providers, Arc::clone(&catalog))?; - pin_models(compiled, &catalog) - }) - .await - .map_err(|source| { - RunCompilerError::Workflow(WorkflowError::engine_with_source( - "workflow create task failed", - source, - )) - })? -} - /// Stages two and three for a run Petri admitted: parse the Fabro graph /// the read side displays, with no lint and no model pinning, and record /// the admission on the run. @@ -441,7 +412,7 @@ pub(crate) async fn compile_admitted( Ok(PinnedRun { materialized: operations::materialize_admitted_run(compiled), metadata, - engine: RunEngine::Petri(admission), + admission, }) }) .await @@ -453,54 +424,6 @@ pub(crate) async fn compile_admitted( })? } -/// Stage two's graph compilation: parse, transform, and validate through the -/// fabro-workflow pipeline, with undefined template variables promoted to -/// hard errors. -fn compile_graph( - prepared: PreparedRun, - configured_providers: Vec, - catalog: Arc, -) -> Result { - let PreparedRun { - layered: - LayeredRun { - workflow_bundle, - entrypoint, - workflow, - settings, - cwd, - metadata, - }, - vars, - } = prepared; - let compiled = operations::compile_create_run( - CreateRunCompileInput { - workflow: WorkflowInput::Bundled(workflow), - settings, - vars, - cwd, - workflow_path: Some(entrypoint), - workflow_bundle: Some(workflow_bundle), - configured_providers, - }, - catalog, - )?; - - Ok(GraphCompiledRun { compiled, metadata }) -} - -/// Stage three: pin concrete model and provider selections against the -/// catalog and the configured provider set. -fn pin_models(compiled: GraphCompiledRun, catalog: &Catalog) -> Result { - let GraphCompiledRun { compiled, metadata } = compiled; - let materialized = operations::materialize_create_run(compiled, catalog)?; - Ok(PinnedRun { - materialized, - metadata, - engine: RunEngine::Legacy, - }) -} - /// Stage four: purely assemble the complete persistence input. Every durable /// field — run id, captured definition, automation reference — is set here /// once; nothing mutates the result afterwards. @@ -508,7 +431,7 @@ pub(crate) fn assemble_run(pinned: PinnedRun) -> CreateRunPersistenceInput { let PinnedRun { materialized, metadata, - engine, + admission, } = pinned; let RunMetadata { run_id, @@ -536,7 +459,7 @@ pub(crate) fn assemble_run(pinned: PinnedRun) -> CreateRunPersistenceInput { parent_id, provenance, web_url, - engine, + admission, }) } @@ -623,7 +546,6 @@ mod tests { use std::error::Error as _; use fabro_config::EnvironmentDockerfileLayer; - use fabro_graphviz::graph::AttrValue; use fabro_types::settings::interp::ResolveCtx; use fabro_types::settings::run::RunGoal; use fabro_types::{AutomationRef, Principal, RunProvenance, SystemActorKind}; @@ -709,13 +631,6 @@ mod tests { } } - fn test_provider_ids() -> Vec { - fabro_llm::test_support::test_catalog() - .enabled_provider_ids() - .into_iter() - .collect() - } - fn prepare_run( input: RawRunCompilerInput, vars: HashMap, @@ -916,45 +831,8 @@ include = ["reports/{{ vars.path }}/*.json"] )); } - #[test] - fn graph_vars_are_hard_errors_and_successfully_render_when_present() { - let catalog = Arc::new(fabro_llm::test_support::test_catalog()); - let missing = prepare_run(raw_input(None, HashMap::new()), HashMap::new()) - .expect("settings preparation should not compile graph vars"); - let Err(error) = compile_graph(missing, test_provider_ids(), Arc::clone(&catalog)) else { - panic!("missing graph variable should be a hard error"); - }; - assert!(matches!( - error, - RunCompilerError::Workflow(WorkflowError::ValidationFailed { .. }) - )); - - let mut input = raw_input(None, HashMap::new()); - input.input_overrides.insert( - "target".to_string(), - toml::Value::String("checkout".to_string()), - ); - let prepared = prepare_run( - input, - HashMap::from([("owner".to_string(), "payments".to_string())]), - ) - .expect("settings should prepare"); - let compiled = compile_graph(prepared, test_provider_ids(), catalog) - .expect("graph variables should render"); - let work = &compiled.compiled.validated().graph().nodes["work"]; - - assert_eq!( - work.attrs.get("prompt").and_then(AttrValue::as_str), - Some("Ship checkout for payments") - ); - assert_eq!( - work.attrs.get("provider").and_then(AttrValue::as_str), - Some("openai") - ); - } - - #[test] - fn assembly_retains_entrypoint_and_run_metadata() { + #[tokio::test] + async fn assembly_retains_entrypoint_and_run_metadata() { let run_id = RunId::new(); let parent_id = RunId::new(); let automation = AutomationRef { @@ -977,16 +855,15 @@ include = ["reports/{{ vars.path }}/*.json"] toml::Value::String("checkout".to_string()), ); let expected_entrypoint = input.entrypoint.clone(); - let catalog = Arc::new(fabro_llm::test_support::test_catalog()); let prepared = prepare_run( input, HashMap::from([("owner".to_string(), "payments".to_string())]), ) .expect("settings should prepare"); - let compiled = compile_graph(prepared, test_provider_ids(), Arc::clone(&catalog)) - .expect("graph should compile"); - let pinned = pin_models(compiled, &catalog).expect("models should pin"); + let pinned = compile_admitted(prepared, PetriAdmission::default()) + .await + .expect("the admitted graph should compile"); let persistence = assemble_run(pinned); assert_eq!(persistence.run_id(), run_id); diff --git a/lib/apps/fabro-server/src/run_files.rs b/lib/apps/fabro-server/src/run_files.rs index d242f11a9..efa8d6646 100644 --- a/lib/apps/fabro-server/src/run_files.rs +++ b/lib/apps/fabro-server/src/run_files.rs @@ -1681,7 +1681,7 @@ mod tests { use fabro_sandbox::Termination; use fabro_sandbox::test_support::exec_result; - use fabro_types::{RunId, test_support}; + use fabro_types::{PetriAdmission, RunId, test_support}; use tokio::time::{Duration, sleep}; use super::*; @@ -2298,7 +2298,7 @@ index 1111111..2222222 160000 spec_blob: None, git: None, fork_source_ref: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }, chrono::Utc::now(), ); diff --git a/lib/apps/fabro-server/src/serve.rs b/lib/apps/fabro-server/src/serve.rs index f9a581401..512f531e6 100644 --- a/lib/apps/fabro-server/src/serve.rs +++ b/lib/apps/fabro-server/src/serve.rs @@ -8,23 +8,22 @@ use clap::Args; use fabro_config::bind::{self, Bind, BindRequest}; use fabro_config::user::active_settings_path; use fabro_config::{ - RunEnvironmentLayer, RunLayer, RunModelLayer, ServerExecutionLayer, ServerLayer, - ServerWebLayer, Storage, load_config_file, load_server_runtime_settings, + RunEnvironmentLayer, RunLayer, RunModelLayer, ServerLayer, ServerWebLayer, Storage, + load_config_file, load_server_runtime_settings, }; use fabro_install::{OBJECT_STORE_ACCESS_KEY_ID_ENV, OBJECT_STORE_SECRET_ACCESS_KEY_ENV}; use fabro_static::EnvVars; +use fabro_types::ServerSettings; use fabro_types::settings::server::{GithubIntegrationStrategy, LogDestination, WebhookStrategy}; use fabro_types::settings::{ GithubIntegrationSettings, ObjectStoreSettings, ServerListenSettings, ServerNamespace, }; -use fabro_types::{Engine, ServerSettings}; use fabro_util::terminal::Styles; use object_store::aws::{AmazonS3Builder, AmazonS3ConfigKey}; use object_store::client::{HttpClient, HttpConnector}; use object_store::local::LocalFileSystem; use object_store::memory::InMemory; use object_store::{ClientOptions, ObjectStore, RetryConfig}; -use strum::VariantArray as _; use tokio::net::{TcpListener, UnixListener}; use tokio::task::JoinHandle; use tokio::time::{interval, sleep}; @@ -211,11 +210,6 @@ pub struct ServeArgs { #[arg(long)] pub max_concurrent_runs: Option, - /// The engine for every run whose workflow version names none - /// (`legacy` or `petri`); overrides `[server.execution] engine` - #[arg(long, value_parser = parse_engine)] - pub engine: Option, - /// Path to server config file (default: ~/.fabro/settings.toml) #[arg(long)] pub config: Option, @@ -227,17 +221,6 @@ pub struct ServeArgs { pub watch_web: bool, } -fn parse_engine(value: &str) -> Result { - value.parse::().map_err(|_| { - let known = Engine::VARIANTS - .iter() - .map(ToString::to_string) - .collect::>() - .join(", "); - format!("unknown engine `{value}`; expected one of: {known}") - }) -} - fn serve_overrides(args: &ServeArgs) -> (Option, Option) { let mut run = RunLayer::default(); let mut server = ServerLayer::default(); @@ -245,12 +228,6 @@ fn serve_overrides(args: &ServeArgs) -> (Option, Option) let web = server.web.get_or_insert_with(ServerWebLayer::default); web.enabled = Some(args.web); } - if let Some(engine) = args.engine { - let execution = server - .execution - .get_or_insert_with(ServerExecutionLayer::default); - execution.engine = Some(engine); - } if let Some(ref model) = args.model { let model_layer = run.model.get_or_insert_with(RunModelLayer::default); model_layer.name = Some(model.clone()); @@ -1486,7 +1463,6 @@ destination = "file" web: true, no_web: false, max_concurrent_runs: None, - engine: None, config: None, #[cfg(debug_assertions)] watch_web: false, @@ -1513,7 +1489,6 @@ destination = "file" web: false, no_web: true, max_concurrent_runs: None, - engine: None, config: None, #[cfg(debug_assertions)] watch_web: false, diff --git a/lib/apps/fabro-server/src/server.rs b/lib/apps/fabro-server/src/server.rs index 39f6dec53..e1fcfd575 100644 --- a/lib/apps/fabro-server/src/server.rs +++ b/lib/apps/fabro-server/src/server.rs @@ -84,12 +84,12 @@ use fabro_store::{ #[cfg(test)] use fabro_types::BlockedReason; use fabro_types::settings::RunNamespace; -use fabro_types::settings::run::{NotificationRouteSettings, RunMode}; +use fabro_types::settings::run::NotificationRouteSettings; use fabro_types::settings::server::{ GithubIntegrationSettings, GithubIntegrationStrategy, LogDestination, }; use fabro_types::{ - AgentBackend, AskFabro, AskFabroUnavailableReason, BlobHash, Engine, EventBody, + AgentBackend, AskFabro, AskFabroUnavailableReason, BlobHash, EventBody, InterviewQuestionRecord, ModelRef, ModelTestMode, PairId, PairMessageId, PairTarget, PendingReason, Principal, PullRequestLink, QuestionType, RunControlAction, RunEvent, RunId, RunRunnableSource, RunStatusKind, SandboxProviderKind, ServerSettings, SessionCapability, @@ -100,12 +100,10 @@ use fabro_util::error::{ use fabro_util::version::FABRO_VERSION; use fabro_variable::{Error as VariableError, VariableStore}; use fabro_vault::{SecretStore, SecretStoreError, SecretType, Vault}; -use fabro_workflow::artifact_upload::ArtifactSink; #[cfg(test)] use fabro_workflow::command_log::command_log_path; -use fabro_workflow::event::{self as workflow_event, Emitter}; +use fabro_workflow::event::{self as workflow_event}; use fabro_workflow::handler::HandlerRegistry; -use fabro_workflow::pipeline::Persisted; use fabro_workflow::records::Checkpoint; use fabro_workflow::run_lookup::{ RunInfo, StatusFilter, filter_runs, scan_runs_with_summaries, scratch_base, @@ -122,9 +120,7 @@ use tokio::io::{AsyncBufReadExt, AsyncRead, AsyncWriteExt, BufReader}; use tokio::process::Command; use tokio::runtime::Builder as TokioRuntimeBuilder; use tokio::sync::broadcast::error::RecvError; -use tokio::sync::{ - Mutex as AsyncMutex, Notify, RwLock as AsyncRwLock, Semaphore, broadcast, mpsc, oneshot, -}; +use tokio::sync::{Mutex as AsyncMutex, Notify, Semaphore, broadcast, mpsc, oneshot}; use tokio::task::spawn_blocking; use tokio::time::{sleep, timeout}; use tokio_stream::StreamExt; @@ -313,11 +309,6 @@ enum RunExecutionMode { Resume, } -enum ExecutionResult { - Completed(Box>), - CancelledBySignal, -} - const WORKER_CANCEL_GRACE: Duration = Duration::from_secs(5); const TERMINAL_DELETE_WORKER_GRACE: Duration = Duration::from_millis(50); const WORKER_CONTROL_ENQUEUE_TIMEOUT: Duration = Duration::from_secs(1); @@ -2753,6 +2744,12 @@ async fn delete_run_internal( .delete_run(&id) .await .map_err(|err| ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()))?; + state.petri_runs.worker_exited(id); + state + .petri_projector + .delete_run(id) + .await + .map_err(|err| ApiError::new(StatusCode::INTERNAL_SERVER_ERROR, err.to_string()))?; state .artifact_store .delete_for_run(&id) @@ -3168,16 +3165,13 @@ pub(crate) async fn reconcile_incomplete_runs_on_startup( for summary in summaries { let run_store = state.stores.runs.open_run(&summary.id).await?; - // A Petri run continues from its records in a new worker, unless a - // cancel was pending or the run was being removed: those end as a - // legacy run's do. + // A run continues from its records in a new worker, unless a cancel + // was pending or the run was being removed: those end failed. if petri_run_resumes_on_restart(&summary) { let run_state = run_store.state().await?; - if run_state.spec.engine.is_petri() { - petri_runs::reconcile_on_startup(state, summary.id, &run_store, &run_state).await?; - reconciled += 1; - continue; - } + petri_runs::reconcile_on_startup(state, summary.id, &run_store, &run_state).await?; + reconciled += 1; + continue; } let (error, reason) = failure_for_incomplete_run( summary.lifecycle.pending_control, @@ -3353,23 +3347,6 @@ async fn persist_cancelled_run_status(state: &AppState, run_id: RunId) -> anyhow workflow_event::append_event(&run_store, &run_id, &failure_event).await } -async fn finish_cancelled_run_before_execution(state: &Arc, run_id: RunId) { - if let Err(err) = persist_cancelled_run_status(state.as_ref(), run_id).await { - error!(run_id = %run_id, error = %err, "Failed to persist cancelled run status"); - } - - let mut runs = state.runs.lock().expect("runs lock poisoned"); - if let Some(managed_run) = runs.get_mut(&run_id) { - managed_run.status = RunStatus::Failed { - reason: FailureReason::Cancelled, - }; - clear_live_run_state(managed_run); - } - drop(runs); - cleanup_worker_control_bus_for_run(state.as_ref(), run_id); - state.scheduler_notify.notify_one(); -} - /// Reject the run before execution if its effective sandbox provider is /// disabled by server policy. Returns `true` when the run was rejected. async fn reject_run_if_sandbox_provider_disabled( @@ -4036,367 +4013,17 @@ async fn execute_run(state: Arc, run_id: RunId) { return; } - // A Petri run takes the worker path a legacy run takes. Under the test - // override it executes in this process instead, so the scenario tests - // need no worker binary. - match run_engine(&state, run_id).await { - Ok(Engine::Petri) if state.registry_factory_override.is_some() => { - Box::pin(petri_runs::execute(state, run_id)).await; - return; - } - Ok(Engine::Petri | Engine::Legacy) => {} - Err(err) => { - tracing::error!(run_id = %run_id, error = %err, "Failed to read the run's engine"); - fail_managed_run( - &state, - run_id, - FailureReason::WorkflowError, - format!("Failed to read the run's engine: {err}"), - ); - state.scheduler_notify.notify_one(); - return; - } - } - + // A run executes in its worker process. Under the test override it + // executes in this process instead, so the scenario tests need no worker + // binary. if state.registry_factory_override.is_some() { - Box::pin(execute_run_in_process(state, run_id)).await; + Box::pin(petri_runs::execute(state, run_id)).await; return; } Box::pin(execute_run_subprocess(state, run_id)).await; } -/// The engine the run was created for, from its stored spec. -async fn run_engine(state: &AppState, run_id: RunId) -> anyhow::Result { - let run_store = state.stores.runs.open_run(&run_id).await?; - let run_state = run_store.state().await?; - Ok(run_state.spec.engine.engine()) -} - -async fn execute_run_in_process(state: Arc, run_id: RunId) { - // Transition to Starting and set up cancel infrastructure - let (cancel_rx, run_dir, event_tx, cancel_token, execution_mode) = { - let mut runs = state.runs.lock().expect("runs lock poisoned"); - let managed_run = match runs.get_mut(&run_id) { - Some(r) if r.status == RunStatus::Runnable => r, - _ => return, - }; - let Some(run_dir) = managed_run.run_dir.clone() else { - return; - }; - - let (cancel_tx, cancel_rx) = oneshot::channel::<()>(); - let cancel_token = CancellationToken::new(); - let (event_tx, _) = broadcast::channel(256); - - managed_run.status = RunStatus::Starting; - managed_run.cancel_tx = Some(cancel_tx); - managed_run.cancel_token = Some(cancel_token.clone()); - managed_run.event_tx = Some(event_tx); - - ( - cancel_rx, - run_dir, - managed_run.event_tx.clone(), - cancel_token, - managed_run.execution_mode, - ) - }; - - // Create interviewer and event plumbing (this is the "provisioning" phase) - let interviewer = Arc::new(ControlInterviewer::new()); - let interview_runtime: Arc = interviewer.clone(); - let emitter = Emitter::new(run_id); - if let Some(tx_clone) = event_tx { - emitter.on_event(move |event| { - let _ = tx_clone.send(event.clone()); - }); - } - let registry_override = state - .registry_factory_override - .as_ref() - .map(|factory| Arc::new(factory(Arc::clone(&interview_runtime)))); - let emitter = Arc::new(emitter); - let steering_hub = Arc::new(fabro_workflow::SteeringHub::new(Arc::clone(&emitter))); - - // Transition to Running, populate interviewer - let cancelled_during_setup = { - let mut runs = state.runs.lock().expect("runs lock poisoned"); - if let Some(managed_run) = runs.get_mut(&run_id) { - if managed_run.status == RunStatus::Starting { - managed_run.status = RunStatus::Running; - managed_run.answer_transport = Some(RunAnswerTransport::InProcess { - interviewer: Arc::clone(&interviewer), - steering_hub: Arc::clone(&steering_hub), - }); - false - } else { - // Was cancelled during setup - clear_live_run_state(managed_run); - state.scheduler_notify.notify_one(); - true - } - } else { - false - } - }; - if cancelled_during_setup { - if let Err(err) = persist_cancelled_run_status(state.as_ref(), run_id).await { - error!(run_id = %run_id, error = %err, "Failed to persist cancelled run status"); - } - return; - } - - let run_store = match state.stores.runs.open_run(&run_id).await { - Ok(run_store) => run_store, - Err(e) => { - tracing::error!(run_id = %run_id, error = %e, "Failed to open run store"); - let mut runs = state.runs.lock().expect("runs lock poisoned"); - if let Some(managed_run) = runs.get_mut(&run_id) { - managed_run.status = RunStatus::Failed { - reason: FailureReason::WorkflowError, - }; - managed_run.error = Some(format!("Failed to open run store: {e}")); - clear_live_run_state(managed_run); - } - state.scheduler_notify.notify_one(); - return; - } - }; - tokio::spawn(forward_run_events_to_global( - Arc::clone(&state), - run_id, - run_store.subscribe(), - )); - let persisted = match Persisted::load_from_store(&run_store.clone().into(), &run_dir).await { - Ok(persisted) => persisted, - Err(e) => { - tracing::error!(run_id = %run_id, error = %e, "Failed to load persisted run"); - fail_run_before_execution( - &state, - run_id, - FailureReason::WorkflowError, - format!("Failed to load persisted run: {e}"), - ) - .await; - return; - } - }; - let server_settings = state.server_settings(); - let github_settings = &server_settings.server.integrations.github; - if cancel_token.is_cancelled() { - finish_cancelled_run_before_execution(&state, run_id).await; - return; - } - if reject_run_if_sandbox_provider_disabled( - &state, - &server_settings, - run_id, - &persisted.run_spec().settings.run, - ) - .await - { - return; - } - let github_app_result = { - let run_spec = persisted.run_spec(); - let settings = &run_spec.settings.run; - let clone_can_use_github_credentials = settings.execution.mode != RunMode::DryRun - && settings.environment.provider.clones_workspace() - && run_spec - .repo_origin_url() - .is_some_and(|origin| !origin.trim().is_empty()); - let pull_request_can_use_github_credentials = - settings.execution.mode != RunMode::DryRun && settings.pull_request.is_some(); - if settings.integrations.github.is_token_requested() { - state.github_credentials(github_settings).await - } else if clone_can_use_github_credentials || pull_request_can_use_github_credentials { - match state.github_credentials(github_settings).await { - Ok(github_app) => Ok(github_app), - Err(err) => { - tracing::warn!( - run_id = %run_id, - error = %err, - "GitHub credentials unavailable; pull request creation will be skipped" - ); - Ok(None) - } - } - } else { - Ok(None) - } - }; - let github_app = match github_app_result { - Ok(github_app) => github_app, - Err(e) => { - if cancel_token.is_cancelled() { - finish_cancelled_run_before_execution(&state, run_id).await; - return; - } - tracing::error!(run_id = %run_id, error = %e, "Invalid GitHub credentials"); - fail_run_before_execution( - &state, - run_id, - FailureReason::WorkflowError, - format!("Invalid GitHub credentials: {e}"), - ) - .await; - return; - } - }; - let github_integration = match persisted - .run_spec() - .settings - .run - .integrations - .github - .resolve_integration() - { - Ok(integration) => integration, - Err(err) => { - tracing::error!( - run_id = %run_id, - error = %err, - "GitHub permission interpolation failed" - ); - fail_run_before_execution( - &state, - run_id, - FailureReason::WorkflowError, - format!("Failed to resolve GitHub permissions: {err}"), - ) - .await; - return; - } - }; - let vault = match state.stores.vault.snapshot().await { - Ok(vault) => vault, - Err(err) => { - tracing::error!(run_id = %run_id, error = ?err, "Loading run secrets failed"); - fail_run_before_execution( - &state, - run_id, - FailureReason::WorkflowError, - "Loading run secrets failed".to_string(), - ) - .await; - return; - } - }; - let services = operations::StartServices { - run_id, - cancel_token: cancel_token.clone(), - emitter: Arc::clone(&emitter), - interviewer: Arc::clone(&interview_runtime), - steering_hub: Arc::clone(&steering_hub), - run_store: run_store.clone().into(), - event_sink: workflow_event::RunEventSink::store(run_store.clone()), - artifact_sink: Some(ArtifactSink::Store(state.artifact_store.clone())), - run_control: None, - github_app, - github_integration, - vault: Arc::new(AsyncRwLock::new(vault.into_vault())), - sandbox_providers: state.server_settings().server.sandbox.providers.clone(), - catalog: state.catalog(), - on_node: None, - registry_override, - fabro_run_tools: None, - }; - - let execution = async { - match execution_mode { - RunExecutionMode::Start => operations::start(&run_dir, services).await, - RunExecutionMode::Resume => operations::resume(&run_dir, services).await, - } - }; - - let result = tokio::select! { - result = execution => ExecutionResult::Completed(Box::new(result)), - _ = cancel_rx => { - cancel_token.cancel(); - ExecutionResult::CancelledBySignal - } - }; - - if matches!(&result, ExecutionResult::CancelledBySignal) { - if let Err(err) = persist_cancelled_run_status(state.as_ref(), run_id).await { - error!(run_id = %run_id, error = %err, "Failed to persist cancelled run status"); - } - } - - // Save final projection - let final_projection = match run_store.state().await { - Ok(state) => Some(state), - Err(err) => { - tracing::warn!(run_id = %run_id, error = %err, "Failed to load run state from store"); - None - } - }; - - // Accumulate aggregate usage after execution completes. - if let Some(ref projection) = final_projection { - if projection.current_checkpoint().is_some() { - let mut agg = state - .aggregate_usage - .lock() - .expect("aggregate_usage lock poisoned"); - accumulate_usage_rollup( - &mut agg, - &fabro_workflow::usage_rollup_from_projection(projection), - ); - } - } - - let mut runs = state.runs.lock().expect("runs lock poisoned"); - if let Some(managed_run) = runs.get_mut(&run_id) { - match &result { - ExecutionResult::Completed(result) => { - // A run can fail either before it produces a `Started` or in - // its own outcome; both carry the same `WorkflowError`. - let outcome = match result.as_ref() { - Ok(started) => started.finalized.outcome.as_ref().map(|_| ()), - Err(e) => Err(e), - }; - match outcome { - Ok(()) => { - info!(run_id = %run_id, "Run completed"); - managed_run.status = RunStatus::Succeeded { - reason: SuccessReason::Completed, - }; - } - Err(WorkflowError::Cancelled) => { - info!(run_id = %run_id, "Run cancelled"); - managed_run.status = RunStatus::Failed { - reason: FailureReason::Cancelled, - }; - } - Err(e) => { - let detail = e.display_with_causes(); - error!(run_id = %run_id, error = %detail, "Run failed"); - managed_run.status = RunStatus::Failed { - reason: e.failure_reason(), - }; - managed_run.error = Some(detail); - } - } - } - ExecutionResult::CancelledBySignal => { - info!(run_id = %run_id, "Run cancelled"); - managed_run.status = RunStatus::Failed { - reason: FailureReason::Cancelled, - }; - } - } - managed_run.checkpoint = final_projection - .as_ref() - .and_then(|projection| projection.current_checkpoint().cloned()); - managed_run.run_dir = Some(run_dir); - clear_live_run_state(managed_run); - } - drop(runs); - state.scheduler_notify.notify_one(); -} - async fn execute_run_subprocess(state: Arc, run_id: RunId) { let (run_dir, execution_mode) = { let mut runs = state.runs.lock().expect("runs lock poisoned"); diff --git a/lib/apps/fabro-server/src/server/handler/events.rs b/lib/apps/fabro-server/src/server/handler/events.rs index 6fe7b3ed7..655462aad 100644 --- a/lib/apps/fabro-server/src/server/handler/events.rs +++ b/lib/apps/fabro-server/src/server/handler/events.rs @@ -288,22 +288,25 @@ async fn list_run_events( } let limit = params.limit(); - match run_is_petri(&state, &id).await { - Ok(true) => { - if let Some(detail) = params.stream_cursor_error() { - return ApiError::bad_request(detail).into_response(); - } - return list_run_stream(&state, id, params.after.unwrap_or(0), limit).await; - } - Ok(false) => {} - Err(response) => return response, + if let Err(response) = ensure_run_exists(&state, &id).await { + return response; } - if params.after.is_some() { - return ApiError::bad_request( - "after is the run stream cursor of a Petri run; this run's events use since_seq.", - ) - .into_response(); + if let Some(detail) = params.stream_cursor_error() { + return ApiError::bad_request(detail).into_response(); } + list_run_stream(&state, id, params.after.unwrap_or(0), limit).await +} + +#[expect( + dead_code, + reason = "the legacy event list goes with the legacy events table" +)] +async fn list_run_events_legacy( + state: Arc, + id: RunId, + params: RunEventListParams, + limit: usize, +) -> Response { match state.stores.runs.open_run_reader(&id).await { Ok(run_store) => { let events = match params.order() { @@ -339,14 +342,13 @@ async fn list_run_events( } } -/// Whether the run executes on Petri, from its stored spec; the canonical -/// 404 when there is no such run. -async fn run_is_petri(state: &AppState, id: &RunId) -> Result { - let projection = state +/// The canonical 404 when there is no such run. +async fn ensure_run_exists(state: &AppState, id: &RunId) -> Result<(), Response> { + state .load_run_projection(id) .await - .map_err(IntoResponse::into_response)?; - Ok(projection.spec.engine.is_petri()) + .map(|_| ()) + .map_err(IntoResponse::into_response) } /// One page of a Petri run's stream past `after`. @@ -659,11 +661,14 @@ async fn attach_run_events( Ok(id) => id, Err(response) => return response, }; - match run_is_petri(&state, &id).await { - Ok(true) => return attach_run_stream(state, id, params.after).await, - Ok(false) => {} + match ensure_run_exists(&state, &id).await { + Ok(()) => return attach_run_stream(state, id, params.after).await, Err(response) => return response, } + #[expect( + unreachable_code, + reason = "the legacy attach goes with the legacy events table" + )] let Ok(run_store) = state.stores.runs.open_run_reader(&id).await else { return ApiError::not_found("Run not found.").into_response(); }; @@ -816,7 +821,7 @@ mod stage_events_tests { use axum::body::{Body, to_bytes}; use axum::http::{Request, StatusCode, header}; use fabro_store::EventPayload; - use fabro_types::{Graph, RunId, WorkflowSettings, test_support}; + use fabro_types::{Graph, PetriAdmission, RunId, WorkflowSettings, test_support}; use fabro_workflow::event as workflow_event; use http_body_util::BodyExt; use serde_json::json; @@ -857,7 +862,7 @@ mod stage_events_tests { retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }) .await .expect("run.created should append"); diff --git a/lib/apps/fabro-server/src/server/handler/pair.rs b/lib/apps/fabro-server/src/server/handler/pair.rs index cce6f2ba6..7b8fb75ed 100644 --- a/lib/apps/fabro-server/src/server/handler/pair.rs +++ b/lib/apps/fabro-server/src/server/handler/pair.rs @@ -869,8 +869,8 @@ mod tests { use axum::http::{Request, StatusCode}; use chrono::{TimeZone, Utc}; use fabro_types::{ - AgentEventProps, EventEnvelope, Graph, PairMessageId, RunEvent, StageId, WorkflowSettings, - fixtures, test_support, + AgentEventProps, EventEnvelope, Graph, PairMessageId, PetriAdmission, RunEvent, StageId, + WorkflowSettings, fixtures, test_support, }; use fabro_workflow::event as workflow_event; use pebble_coding_agent::events::{CodingAgentEvent, Usage}; @@ -1057,7 +1057,7 @@ mod tests { retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }) .await .expect("run.created should append"); diff --git a/lib/apps/fabro-server/src/server/handler/runs.rs b/lib/apps/fabro-server/src/server/handler/runs.rs index 43030cb64..377eec71b 100644 --- a/lib/apps/fabro-server/src/server/handler/runs.rs +++ b/lib/apps/fabro-server/src/server/handler/runs.rs @@ -27,11 +27,11 @@ use fabro_store::{ RunSummaryListQuery, RunSummarySort, RunSummarySortDirection, RunSummaryVisibility, }; use fabro_types::{ - AutomationRef, ContextWindowStaleness, Engine, ManifestPath, Principal, Run, - RunClientProvenance, RunId, RunProvenance, RunServerProvenance, RunStatusKind, RunTarget, - SandboxProviderKind, StageContextWindow, StageContextWindowUnavailableReason, StageHandler, - StageModelUsage, StageProjection, SystemActorKind, ValidatedRunTarget, - json_scalar_to_toml_value, parse_blob_ref, + AutomationRef, ContextWindowStaleness, ManifestPath, Principal, Run, RunClientProvenance, + RunId, RunProvenance, RunServerProvenance, RunStatusKind, RunTarget, SandboxProviderKind, + StageContextWindow, StageContextWindowUnavailableReason, StageHandler, StageModelUsage, + StageProjection, SystemActorKind, ValidatedRunTarget, json_scalar_to_toml_value, + parse_blob_ref, }; use fabro_util::error as error_util; use fabro_util::version::FABRO_VERSION; @@ -748,7 +748,6 @@ async fn finalize_created_run( explicit_title_supplied: bool, title_generation_target: ManifestPath, ) -> Response { - let catalog = state.catalog(); // Resolve once: we need both the provider IDs (for the run create input // and ask-fabro-readiness) and the LLM client itself (for the spawned // title-generation task). `ready_llm_provider_ids` would otherwise call @@ -759,7 +758,7 @@ async fn finalize_created_run( #[cfg(any(test, feature = "test-support"))] { server_test_support::test_run_materialization_provider_ids( - catalog.as_ref(), + state.catalog().as_ref(), &ready_provider_ids, ) } @@ -768,22 +767,13 @@ async fn finalize_created_run( ready_provider_ids.clone() } }; - // Petri compiles a Petri run: the bundle goes to `Runtime::check`, its + // Petri compiles the run: the bundle goes to `Runtime::check`, its // diagnostics come back in Fabro's shape, and the admitted graph is what - // the run executes. The legacy compile, lint and model pinning are - // skipped for it; Fabro's own settings resolution ran above as for any - // run. - let engine = petri_runs::engine_for(prepared.settings(), &state.server_settings()); - let pinned = match engine { - Engine::Legacy => { - run_compiler::compile_and_pin(prepared, run_materialization_provider_ids, catalog).await - } - Engine::Petri => { - match petri_runs::admit(&state, &prepared, &run_materialization_provider_ids).await { - Ok(admission) => run_compiler::compile_admitted(prepared, admission).await, - Err(error) => Err(error), - } - } + // the run executes. Fabro's own settings resolution ran above. + let pinned = match petri_runs::admit(&state, &prepared, &run_materialization_provider_ids).await + { + Ok(admission) => run_compiler::compile_admitted(prepared, admission).await, + Err(error) => Err(error), }; let pinned = match pinned { Ok(pinned) => pinned, diff --git a/lib/apps/fabro-server/src/server/handler/sessions.rs b/lib/apps/fabro-server/src/server/handler/sessions.rs index 5cf809869..da4237fdf 100644 --- a/lib/apps/fabro-server/src/server/handler/sessions.rs +++ b/lib/apps/fabro-server/src/server/handler/sessions.rs @@ -1493,7 +1493,7 @@ fn parse_turn_id(value: &str) -> Result { mod tests { use std::collections::HashMap; - use fabro_types::test_support; + use fabro_types::{PetriAdmission, test_support}; use pebble_coding_agent::events::{ToolCategory, ToolSource}; use super::*; @@ -1898,7 +1898,7 @@ enabled = true spec_blob: None, git: None, fork_source_ref: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }; let mut projection = fabro_types::RunProjection::new(String::new(), spec, now); for (index, node_id) in ["start", "plan", "code", "test", "review", "deploy"] diff --git a/lib/apps/fabro-server/src/server/petri_runs.rs b/lib/apps/fabro-server/src/server/petri_runs.rs index 3a4a0a85f..cb5902568 100644 --- a/lib/apps/fabro-server/src/server/petri_runs.rs +++ b/lib/apps/fabro-server/src/server/petri_runs.rs @@ -47,10 +47,7 @@ use fabro_petri::runtime::{self, RuntimeSpec}; use fabro_petri::secrets::VaultSecrets; use fabro_petri::{SqliteRunStore, admission}; use fabro_types::settings::run::{ApprovalMode, RunMode}; -use fabro_types::{ - Engine, PetriAdmission, RunId, RunRunnableSource, RunTarget, RunTiming, ServerSettings, - StageOutcome, -}; +use fabro_types::{PetriAdmission, RunId, RunRunnableSource, RunTarget, RunTiming, StageOutcome}; use fabro_util::error as error_util; use fabro_validate::{Diagnostic as FabroDiagnostic, Severity}; use fabro_workflow::Error as WorkflowError; @@ -65,18 +62,6 @@ use super::{AppState, RunAnswerTransport, RunExecutionMode, clear_live_run_state use crate::petri_runs::PetriRuns; use crate::run_compiler::{PreparedRun, RunCompilerError}; -/// The engine a run gets: the one its workflow version names, else the -/// server's default. -pub(crate) fn engine_for( - settings: &fabro_types::WorkflowSettings, - server: &ServerSettings, -) -> Engine { - settings - .workflow - .engine - .unwrap_or(server.server.execution.engine) -} - /// The runtime Petri gets, at create and at execution: the server's run /// defaults as the settings layer, the model client over the server's /// catalog and credentials for the eligible providers, and the run mode. @@ -185,6 +170,11 @@ pub(crate) async fn admit( project_toml: None, }, inputs, + vars: prepared + .vars() + .iter() + .map(|(name, value)| (name.clone(), value.clone())) + .collect(), launch: Launch { model, provider, @@ -298,10 +288,7 @@ pub(crate) async fn execute(state: Arc, run_id: RunId) { return; } }; - let Some(admission) = run_state.spec.engine.petri().cloned() else { - fail_before_execution(&state, &run_store, run_id, "the run has no Petri admission").await; - return; - }; + let admission = run_state.spec.admission.clone(); let server_settings = state.server_settings(); if super::reject_run_if_sandbox_provider_disabled( &state, diff --git a/lib/apps/fabro-server/src/server/tests.rs b/lib/apps/fabro-server/src/server/tests.rs index 724422b1b..bd00ab4db 100644 --- a/lib/apps/fabro-server/src/server/tests.rs +++ b/lib/apps/fabro-server/src/server/tests.rs @@ -22,14 +22,14 @@ use fabro_interview::{ }; use fabro_llm::lithos_catalog::Catalog; use fabro_types::settings::ServerAuthMethod; -use fabro_types::settings::run::ApprovalMode; +use fabro_types::settings::run::{ApprovalMode, RunMode}; use fabro_types::{ AgentBackend, AttrValue, AuthMethod, BlobHash, CommandTermination, ContextWindowBreakdownItem, ContextWindowCategory, ContextWindowCountMethod, ContextWindowSnapshot, ContextWindowStaleness, ContextWindowWarning, FailureCategory, FailureDetail, GitRunTarget, Graph, - InterviewQuestionRecord, ModelRef, Node, Outcome, ParallelBranchId, QuestionType, RunId, - RunSpec, RunTarget, SandboxProviderKind, StageModelUsage, StageTiming, SuccessReason, - SystemActorKind, WorkflowSettings, fixtures, test_support, + InterviewQuestionRecord, ModelRef, Node, Outcome, ParallelBranchId, PetriAdmission, + QuestionType, RunId, RunSpec, RunTarget, SandboxProviderKind, StageModelUsage, StageTiming, + SuccessReason, SystemActorKind, WorkflowSettings, fixtures, test_support, }; use fabro_util::check_report::CheckStatus; use fabro_workflow::records::CheckpointExt; @@ -5815,7 +5815,7 @@ async fn append_default_run_created(run_store: &fabro_store::RunDatabase, run_id retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }) .await .unwrap(); @@ -5869,7 +5869,7 @@ async fn create_slack_notification_run( retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }) .await .unwrap(); @@ -6946,7 +6946,7 @@ async fn list_run_stages_distinguishes_visits() { retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }, workflow_event::Event::RunStarting, workflow_event::Event::RunRunning, @@ -7086,7 +7086,7 @@ async fn list_run_stages_exposes_execution_identity_for_resumed_stage() { retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }, workflow_event::Event::RunStarting, workflow_event::Event::RunRunning, @@ -8276,7 +8276,7 @@ async fn create_completed_run_ready_for_pull_request( definition_blob: None, spec_blob: None, fork_source_ref: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }; create_durable_run_with_events(state, run_id, &[ @@ -8299,7 +8299,7 @@ async fn create_completed_run_ready_for_pull_request( retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }, workflow_event::Event::WorkflowRunStarted { name: "test".to_string(), @@ -15369,7 +15369,7 @@ async fn create_preserved_local_sandbox_run(state: &Arc, run_id: RunId retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }, workflow_event::Event::RunSubmitted { definition_blob: None, @@ -16121,7 +16121,7 @@ async fn delete_run_retry_after_missing_provider_resource_removes_metadata() { retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }, workflow_event::Event::RunSubmitted { definition_blob: None, diff --git a/lib/apps/fabro-server/tests/it/api/run_files.rs b/lib/apps/fabro-server/tests/it/api/run_files.rs index 18552e96b..162887b1e 100644 --- a/lib/apps/fabro-server/tests/it/api/run_files.rs +++ b/lib/apps/fabro-server/tests/it/api/run_files.rs @@ -14,7 +14,9 @@ use axum::body::Body; use axum::http::{Request, StatusCode}; use fabro_server::test_support::test_app_state_with_store; use fabro_store::{ArtifactStore, Database}; -use fabro_types::{Graph, RunId, SandboxProviderKind, WorkflowSettings, test_support}; +use fabro_types::{ + Graph, PetriAdmission, RunId, SandboxProviderKind, WorkflowSettings, test_support, +}; use fabro_workflow::event as workflow_event; use fabro_workflow::run_status::SuccessReason; use object_store::memory::InMemory as MemoryObjectStore; @@ -76,7 +78,7 @@ async fn append_completed_run_with_final_patch( retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }) .await .expect("append RunCreated"); diff --git a/lib/apps/fabro-server/tests/it/api/runs.rs b/lib/apps/fabro-server/tests/it/api/runs.rs index c4318acda..8b3a964b7 100644 --- a/lib/apps/fabro-server/tests/it/api/runs.rs +++ b/lib/apps/fabro-server/tests/it/api/runs.rs @@ -372,17 +372,20 @@ async fn link_relink_and_unlink_parent_are_idempotent() { format!("GET /api/v1/runs/{child_id}/events"), ) .await; - let event_names = events["data"] + // The run's stream holds Fabro's platform records: the run's creation, + // its submission, and one `run.parent` record per link. + let record_kinds = events["data"] .as_array() .unwrap() .iter() - .map(|event| event["event"].as_str().unwrap()) + .filter(|item| item["kind"] == "platform") + .map(|item| item["item"]["record"]["kind"].as_str().unwrap().to_string()) .collect::>(); - assert_eq!(event_names, vec![ + assert_eq!(record_kinds, vec![ "run.created", - "run.submitted", - "run.parent.linked", - "run.parent.linked" + "run.lifecycle", + "run.parent", + "run.parent" ]); let unlink_request = Request::builder() diff --git a/lib/apps/fabro-server/tests/it/api/sessions.rs b/lib/apps/fabro-server/tests/it/api/sessions.rs index c4ac167c1..936b4545f 100644 --- a/lib/apps/fabro-server/tests/it/api/sessions.rs +++ b/lib/apps/fabro-server/tests/it/api/sessions.rs @@ -114,13 +114,13 @@ async fn run_bound_session_is_created_as_run_event_and_resolves_by_flat_id() { let events_request = Request::builder() .method("GET") - .uri(api(&format!("/runs/{run_id}/events"))) + .uri(api(&format!("/sessions/{session_id}/events"))) .body(Body::empty()) - .expect("run-events request should build"); + .expect("session-events request should build"); let events = response_json( app.clone().oneshot(events_request).await.unwrap(), StatusCode::OK, - format!("GET /api/v1/runs/{run_id}/events"), + format!("GET /api/v1/sessions/{session_id}/events"), ) .await; let session_events: Vec<_> = events["data"] @@ -240,16 +240,17 @@ async fn supplied_session_model_alias_is_canonicalized() { let created = create_session_with_model(&app, &run_id, "Ask Fabro", "gpt54").await; assert_eq!(created["model"], "gpt-5.4"); + let session_id = created["id"].as_str().expect("session id"); let events_request = Request::builder() .method("GET") - .uri(api(&format!("/runs/{run_id}/events"))) + .uri(api(&format!("/sessions/{session_id}/events"))) .body(Body::empty()) - .expect("run-events request should build"); + .expect("session-events request should build"); let events = response_json( app.clone().oneshot(events_request).await.unwrap(), StatusCode::OK, - format!("GET /api/v1/runs/{run_id}/events"), + format!("GET /api/v1/sessions/{session_id}/events"), ) .await; diff --git a/lib/apps/fabro-server/tests/it/api/system.rs b/lib/apps/fabro-server/tests/it/api/system.rs index 8b0db89d2..0309162d2 100644 --- a/lib/apps/fabro-server/tests/it/api/system.rs +++ b/lib/apps/fabro-server/tests/it/api/system.rs @@ -422,7 +422,7 @@ async fn test_app_state_with_options_respects_max_concurrent_runs() { start_run(&app, &second_run).await; let question = wait_for_question(&app, &first_run).await; - assert_eq!(question["stage"], "gate"); + assert_eq!(question["stage"], "gate@1"); tokio::time::sleep(POLL_INTERVAL * 5).await; diff --git a/lib/apps/fabro-server/tests/it/api/tcp.rs b/lib/apps/fabro-server/tests/it/api/tcp.rs index 8cec35082..d9ea549e6 100644 --- a/lib/apps/fabro-server/tests/it/api/tcp.rs +++ b/lib/apps/fabro-server/tests/it/api/tcp.rs @@ -80,7 +80,6 @@ async fn spawn_served_listener( provider: None, environment: None, max_concurrent_runs: None, - engine: None, config: Some(config_path), #[cfg(debug_assertions)] watch_web: false, diff --git a/lib/apps/fabro-server/tests/it/api/variables.rs b/lib/apps/fabro-server/tests/it/api/variables.rs index caec2775d..04749d401 100644 --- a/lib/apps/fabro-server/tests/it/api/variables.rs +++ b/lib/apps/fabro-server/tests/it/api/variables.rs @@ -276,10 +276,8 @@ async fn run_create_interpolates_variables_into_node_prompts() { // inside a node `prompt` (a DOT graph attribute the settings substitution // pass never touches), proving the variable store is snapshotted into the // template render context at create time. - let app = fabro_server::test_support::build_test_router(test_app_state_with_options( - test_settings(), - 5, - )); + let state = test_app_state_with_options(test_settings(), 5); + let app = fabro_server::test_support::build_test_router(std::sync::Arc::clone(&state)); let create_variable = app .clone() @@ -316,7 +314,12 @@ async fn run_create_interpolates_variables_into_node_prompts() { .as_str() .expect("create run response should include id"); - // The persisted `run.created` event carries the fully-rendered graph. + // The run's stream holds its `run.created` record, whose spec carries + // the fully-rendered graph. The view trails the record, so wait for it. + state + .test_petri_projector() + .settle(run_id.parse().expect("run id")) + .await; let events = app .oneshot(empty_request( Method::GET, @@ -334,12 +337,12 @@ async fn run_create_interpolates_variables_into_node_prompts() { .as_array() .expect("events response should include data") .iter() - .find(|event| event["event"] == "run.created") - .expect("expected a run.created event"); + .find(|item| item["item"]["record"]["kind"] == "run.created") + .expect("expected a run.created record"); assert_eq!( - created["properties"]["graph"]["nodes"]["work"]["attrs"]["prompt"]["String"], + created["item"]["record"]["spec"]["graph"]["nodes"]["work"]["attrs"]["prompt"]["String"], "Service: billing", - "node prompt should interpolate the run variable; event: {created}" + "node prompt should interpolate the run variable; record: {created}" ); } diff --git a/lib/apps/fabro-server/tests/it/scenario/petri.rs b/lib/apps/fabro-server/tests/it/scenario/petri.rs index 365755a23..21583541a 100644 --- a/lib/apps/fabro-server/tests/it/scenario/petri.rs +++ b/lib/apps/fabro-server/tests/it/scenario/petri.rs @@ -1,7 +1,6 @@ -//! Runs on Petri through the server: a run goes to Petri when its workflow -//! version names `engine = "petri"` or when the server's -//! `[server.execution] engine` says so, Petri's record of the run agrees -//! with Fabro's status, and Petri's diagnostics refuse a run at create. +//! Runs on Petri through the server: every run executes on Petri, Petri's +//! record of the run agrees with Fabro's status, and Petri's diagnostics +//! refuse a run at create. //! //! The runs here execute in the server process under the handler-registry //! test override; outside it the scheduler launches a worker for a Petri @@ -106,8 +105,6 @@ const PARALLEL_DOT: &str = r#"digraph Parallel { }"#; pub(super) const PLAIN_SETTINGS: &str = "_version = 1\n\n[workflow]\ngraph = \"workflow.fabro\"\n"; -const PETRI_SETTINGS: &str = - "_version = 1\n\n[workflow]\ngraph = \"workflow.fabro\"\nengine = \"petri\"\n"; /// The host plugin as Petri's lookup finds it: the override variable, else /// the executable on `PATH`. `None`, after saying so, when the test should @@ -159,22 +156,11 @@ pub(super) fn intent(version_id: &str, workspace: &std::path::Path) -> serde_jso }) } -/// The `hello` bundle checked into this repository, with `engine = "petri"` -/// added to its `[workflow]` table. +/// The `hello` bundle checked into this repository. fn hello_files() -> [(&'static str, String); 2] { let workflow = read_repo_file(".fabro/workflows/hello/workflow.fabro"); let settings = read_repo_file(".fabro/workflows/hello/workflow.toml"); - assert!( - settings.trim_end().ends_with("graph = \"workflow.fabro\""), - "the hello settings end with the [workflow] table, so an engine key appends to it" - ); - [ - ("workflow.fabro", workflow), - ( - "workflow.toml", - format!("{}\nengine = \"petri\"\n", settings.trim_end()), - ), - ] + [("workflow.fabro", workflow), ("workflow.toml", settings)] } /// The run's record in Petri's store, read through the same database the @@ -221,7 +207,7 @@ async fn petri_stream_len(state: &AppState, run_id: &str) -> usize { .len() } -async fn run_engine(app: &axum::Router, run_id: &str) -> serde_json::Value { +async fn run_admission(app: &axum::Router, run_id: &str) -> serde_json::Value { let req = Request::builder() .method("GET") .uri(api(&format!("/runs/{run_id}/state"))) @@ -238,7 +224,7 @@ async fn run_engine(app: &axum::Router, run_id: &str) -> serde_json::Value { format!("GET /api/v1/runs/{run_id}/state"), ) .await; - body["spec"]["engine"].clone() + body["spec"]["admission"].clone() } async fn create_run_response(app: &axum::Router, intent: serde_json::Value) -> serde_json::Value { @@ -263,12 +249,11 @@ async fn create_run_response(app: &axum::Router, intent: serde_json::Value) -> s .await } -/// The `hello` bundle, whose one stage is a prompt, runs on Petri when its -/// version names the engine: the prompt reaches the twin through Petri's -/// model client, Fabro reports the run succeeded, and Petri's record of the -/// run says the same. +/// The `hello` bundle, whose one stage is a prompt, runs on Petri: the +/// prompt reaches the twin through Petri's model client, Fabro reports the +/// run succeeded, and Petri's record of the run says the same. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn the_hello_bundle_runs_on_petri_when_the_version_names_the_engine() { +async fn the_hello_bundle_runs_on_petri() { if host_plugin().is_none() { return; } @@ -309,7 +294,10 @@ async fn the_hello_bundle_runs_on_petri_when_the_version_names_the_engine() { let status = wait_for_run_status(&app, &run_id, &["succeeded", "failed"]).await; let run = run_json(&app, &run_id).await; assert_eq!(status, "succeeded", "run: {run}"); - assert_eq!(run_engine(&app, &run_id).await["kind"], "petri"); + assert!( + run_admission(&app, &run_id).await["graph"]["digest"].is_string(), + "the run's spec names what Petri admitted" + ); let outcome = petri_outcome(&state, &run_id).await; assert_eq!(outcome.status, RunStatus::Success, "{outcome:?}"); assert!(outcome.complete, "{:?}", outcome.incomplete); @@ -342,18 +330,14 @@ async fn the_hello_bundle_runs_on_petri_when_the_version_names_the_engine() { super::petri_stream::capture_settled(&state, &app, &run_id, "hello").await; } -/// A command-only bundle runs on Petri when the server's setting names the -/// engine and the version names none, and Petri's record agrees. +/// A command-only bundle runs on Petri, and Petri's record agrees. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn a_command_bundle_runs_on_petri_under_the_server_setting() { +async fn a_command_bundle_runs_on_petri() { if host_plugin().is_none() { return; } let workspace = tempfile::tempdir().expect("workspace tempdir"); - let settings = settings_from_toml( - "_version = 1\n\n[run.environment]\nid = \"local\"\n\n[server.execution]\nengine = \ - \"petri\"\n", - ); + let settings = settings_from_toml("_version = 1\n\n[run.environment]\nid = \"local\"\n"); let state = test_app_state_with_options(settings, 5); let app = test_app_with_scheduler(Arc::clone(&state)); @@ -368,7 +352,10 @@ async fn a_command_bundle_runs_on_petri_under_the_server_setting() { let status = wait_for_run_status(&app, &run_id, &["succeeded", "failed"]).await; let run = run_json(&app, &run_id).await; assert_eq!(status, "succeeded", "run: {run}"); - assert_eq!(run_engine(&app, &run_id).await["kind"], "petri"); + assert!( + run_admission(&app, &run_id).await["graph"]["digest"].is_string(), + "the run's spec names what Petri admitted" + ); let outcome = petri_outcome(&state, &run_id).await; assert_eq!(outcome.status, RunStatus::Success, "{outcome:?}"); assert!(outcome.complete, "{:?}", outcome.incomplete); @@ -396,10 +383,7 @@ async fn a_parallel_bundle_projects_its_branches_through_the_server() { return; } let workspace = tempfile::tempdir().expect("workspace tempdir"); - let settings = settings_from_toml( - "_version = 1\n\n[run.environment]\nid = \"local\"\n\n[server.execution]\nengine = \ - \"petri\"\n", - ); + let settings = settings_from_toml("_version = 1\n\n[run.environment]\nid = \"local\"\n"); let state = test_app_state_with_options(settings, 5); let app = test_app_with_scheduler(Arc::clone(&state)); @@ -436,26 +420,6 @@ async fn a_parallel_bundle_projects_its_branches_through_the_server() { ); } -/// A version that names no engine on a server whose setting is the default -/// keeps the legacy executor: the run's spec records no Petri admission. -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn a_version_that_names_no_engine_stays_on_the_legacy_executor() { - let workspace = tempfile::tempdir().expect("workspace tempdir"); - let state = test_app_state_with_options(test_settings(), 5); - let app = test_app_with_scheduler(state); - - let version_id = register_version(&app, &[ - ("workflow.fabro", COMMAND_DOT), - ("workflow.toml", PLAIN_SETTINGS), - ]) - .await; - let mut intent = intent(&version_id, workspace.path()); - intent["args"]["dry_run"] = serde_json::json!(true); - let run_id = create_and_start_run_from_intent(&app, intent).await; - - assert_eq!(run_engine(&app, &run_id).await, serde_json::Value::Null); -} - /// A workflow with an attribute the language does not have is refused at /// create with Petri's code in Fabro's diagnostic shape. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] @@ -466,7 +430,7 @@ async fn an_unknown_attribute_is_refused_at_create_with_petris_code() { let version_id = register_version(&app, &[ ("workflow.fabro", UNKNOWN_ATTRIBUTE_DOT), - ("workflow.toml", PETRI_SETTINGS), + ("workflow.toml", PLAIN_SETTINGS), ]) .await; let body = create_run_response(&app, intent(&version_id, workspace.path())).await; @@ -488,7 +452,7 @@ async fn an_edge_to_an_undeclared_node_is_refused_at_create_with_petris_code() { let version_id = register_version(&app, &[ ("workflow.fabro", UNDECLARED_NODE_DOT), - ("workflow.toml", PETRI_SETTINGS), + ("workflow.toml", PLAIN_SETTINGS), ]) .await; let body = create_run_response(&app, intent(&version_id, workspace.path())).await; @@ -511,7 +475,7 @@ async fn an_unknown_model_is_refused_at_create_with_attractor_model_unknown() { let version_id = register_version(&app, &[ ("workflow.fabro", UNKNOWN_MODEL_DOT), - ("workflow.toml", PETRI_SETTINGS), + ("workflow.toml", PLAIN_SETTINGS), ]) .await; let body = create_run_response(&app, intent(&version_id, workspace.path())).await; @@ -581,10 +545,7 @@ async fn a_human_gate_is_answered_through_the_questions_api() { } let workspace = tempfile::tempdir().expect("workspace tempdir"); let markers = tempfile::tempdir().expect("marker tempdir"); - let settings = settings_from_toml( - "_version = 1\n\n[run.environment]\nid = \"local\"\n\n[server.execution]\nengine = \ - \"petri\"\n", - ); + let settings = settings_from_toml("_version = 1\n\n[run.environment]\nid = \"local\"\n"); let state = test_app_state_with_options(settings, 5); let app = test_app_with_scheduler(Arc::clone(&state)); diff --git a/lib/apps/fabro-server/tests/it/scenario/petri_stream.rs b/lib/apps/fabro-server/tests/it/scenario/petri_stream.rs index 3aab3d53b..f084a5891 100644 --- a/lib/apps/fabro-server/tests/it/scenario/petri_stream.rs +++ b/lib/apps/fabro-server/tests/it/scenario/petri_stream.rs @@ -237,10 +237,7 @@ async fn a_reconnecting_client_receives_every_stream_item_once_in_order() { } let workspace = tempfile::tempdir().expect("workspace tempdir"); let markers = tempfile::tempdir().expect("marker tempdir"); - let settings = settings_from_toml( - "_version = 1\n\n[run.environment]\nid = \"local\"\n\n[server.execution]\nengine = \ - \"petri\"\n", - ); + let settings = settings_from_toml("_version = 1\n\n[run.environment]\nid = \"local\"\n"); let state = test_app_state_with_options(settings, 5); let app = test_app_with_scheduler(Arc::clone(&state)); diff --git a/lib/components/fabro-petri/README.md b/lib/components/fabro-petri/README.md index 6f2b740cd..f338a88ae 100644 --- a/lib/components/fabro-petri/README.md +++ b/lib/components/fabro-petri/README.md @@ -42,7 +42,7 @@ Every adapter the integration plan describes lands here. admitted graphs or Petri's diagnostics come back in a shape the server maps onto Fabro's. Nothing is written to disk. - `admission`: the admitted graphs in Fabro's blob store, named on the run - spec as `RunEngine::Petri(PetriAdmission)`, verified by digest on load. + spec as its `PetriAdmission`, verified by digest on load. - `engine`: a run executed by Petri, started from its admitted graphs or resumed from its records, with the outcome read from the run's record through `inspect_run` and mapped to the conclusion Fabro's read side @@ -122,16 +122,12 @@ yet, keep their default value in the projection: `StageProjection.diff` and `description` and `preview`, the pull request `creation` state, and the run's notices, notifications and pairings (recorded, not shown). -A run goes to Petri when its workflow version's `workflow.toml` names -`engine = "petri"` in `[workflow]`, or when the server's -`[server.execution] engine` (`FABRO_SERVER_ENGINE`, `fabro server start ---engine`) says so for versions that name none. The server side of both -halves is `fabro-server`'s `server::petri_runs`; the worker side is -`fabro-cli`'s `commands::run::petri_worker`, which `fabro run __run-worker` -takes when the run's stored spec names Petri. After a server restart, a -Petri run left in flight goes back to a worker in `--mode resume`: the run -continues from its records, as Petri's own resume does, and full recovery -of the workspace to a durable snapshot is the plan's F3.5. +Every run executes on Petri. The server side is `fabro-server`'s +`server::petri_runs`; the worker side is `fabro-cli`'s +`commands::run::petri_worker`, which `fabro run __run-worker` takes. After +a server restart, a run left in flight goes back to a worker in `--mode +resume`: the run continues from its records, as Petri's own resume does, +on workspaces the recovery protocol brought to their durable snapshots. ## How it is tested @@ -194,8 +190,8 @@ ulimit -n 4096 && cargo nextest run -p fabro-petri The server's end-to-end coverage is `lib/apps/fabro-server/tests/it/scenario/petri.rs`: the `hello` bundle on the OpenAI twin, a command-only bundle and a two-branch parallel bundle run to completion through the create handler and -the scheduler, in the server process under its test override, under the -version flag and under the server setting, with `GET /runs/{id}/state` +the scheduler, in the server process under its test override, with +`GET /runs/{id}/state` serving the projection over Petri's records; a human gate is answered through the questions API; and Petri's diagnostics refuse a run at create. The server's `petri_runs` unit tests cover the lease ending at worker exit diff --git a/lib/components/fabro-petri/src/check.rs b/lib/components/fabro-petri/src/check.rs index 29d57671d..5106df8ab 100644 --- a/lib/components/fabro-petri/src/check.rs +++ b/lib/components/fabro-petri/src/check.rs @@ -14,7 +14,8 @@ //! `petri.launch_model` and `petri.launch_provider` as the model default //! below every file layer, and `petri.repository` as the repository the root //! `start` stage checks out. A caller with no local repository binds `null`, -//! and the run starts from an empty workspace. +//! and the run starts from an empty workspace. The server's run variables +//! (`{{ vars.NAME }}`) are bound as compile variables beside them. use std::collections::BTreeMap; use std::path::PathBuf; @@ -69,12 +70,15 @@ pub struct Launch { pub repository: Option, } -/// One check: the bundle, the run's inputs, the launch and the runtime. +/// One check: the bundle, the run's inputs and variables, the launch and +/// the runtime. #[derive(Clone, Default)] pub struct CheckRequest { pub bundle: Bundle, /// The intent's inputs, under which `[run.inputs]` defaults fill in. pub inputs: BTreeMap, + /// The server's run variables, read by `{{ vars.NAME }}`. + pub vars: BTreeMap, pub launch: Launch, pub runtime: RuntimeSpec, } @@ -141,7 +145,7 @@ pub fn check(request: &CheckRequest) -> Result { entrypoint: bundle.entrypoint.clone(), })?; let runtime = request.runtime.runtime(false); - let inputs = compile_inputs(&request.inputs, &request.launch); + let inputs = compile_inputs(&request.inputs, &request.vars, &request.launch); let lowered = runtime .check_source(&bundle.entrypoint, text, &bundle.files(), None, &inputs) .map_err(CheckError::Load)?; @@ -156,12 +160,22 @@ pub fn check(request: &CheckRequest) -> Result { } } -/// The compile inputs: the intent's inputs, and the launch variables. -fn compile_inputs(inputs: &BTreeMap, launch: &Launch) -> CompileInputs { +/// The compile inputs: the intent's inputs, the run variables, and the +/// launch variables. +fn compile_inputs( + inputs: &BTreeMap, + vars: &BTreeMap, + launch: &Launch, +) -> CompileInputs { let mut compile = CompileInputs::new(); for (name, value) in inputs { compile.inputs.insert(name.as_str().into(), value.clone()); } + for (name, value) in vars { + compile + .vars + .insert(name.as_str().into(), Value::String(value.clone())); + } let text = |value: &Option| match value { Some(text) if !text.trim().is_empty() => Value::String(text.clone()), _ => Value::Null, diff --git a/lib/components/fabro-petri/src/projector.rs b/lib/components/fabro-petri/src/projector.rs index 0c3720a26..39da40bfe 100644 --- a/lib/components/fabro-petri/src/projector.rs +++ b/lib/components/fabro-petri/src/projector.rs @@ -209,6 +209,41 @@ impl Projector { stream_after(&self.pool, run_id, after, limit).await } + /// Delete everything the store and the view tables hold for the run: + /// its Petri records and lease, its platform records, its projection + /// and its stream. The caller has ended the run's worker, so no writer + /// holds the lease. + pub async fn delete_run(&self, run_id: RunId) -> Result<(), ProjectError> { + let id = run_id.to_string(); + let mut views = self.pool.begin().await.map_err(ProjectError::Database)?; + for delete in [ + "DELETE FROM petri_stream WHERE run_id = ?", + "DELETE FROM petri_projection WHERE run_id = ?", + "DELETE FROM platform_records WHERE run_id = ?", + ] { + sqlx::query(delete) + .bind(&id) + .execute(&mut *views) + .await + .map_err(ProjectError::Database)?; + } + views.commit().await.map_err(ProjectError::Database)?; + let mut records = self.records.begin().await.map_err(ProjectError::Database)?; + for delete in [ + "DELETE FROM petri_records WHERE run_id = ?", + "DELETE FROM petri_runs WHERE run_id = ?", + ] { + sqlx::query(delete) + .bind(&id) + .execute(&mut *records) + .await + .map_err(ProjectError::Database)?; + } + records.commit().await.map_err(ProjectError::Database)?; + lock(&self.slots).remove(&run_id); + Ok(()) + } + /// The last delivery sequence the run's view holds, or `None` when no /// pass has committed a view for it. pub async fn stream_head(&self, run_id: RunId) -> Result, ProjectError> { diff --git a/lib/components/fabro-petri/tests/check.rs b/lib/components/fabro-petri/tests/check.rs index 84cce9111..c07741663 100644 --- a/lib/components/fabro-petri/tests/check.rs +++ b/lib/components/fabro-petri/tests/check.rs @@ -68,6 +68,7 @@ fn request(bundle: Bundle, runtime: RuntimeSpec) -> CheckRequest { CheckRequest { bundle, inputs: BTreeMap::new(), + vars: BTreeMap::new(), launch: Launch::default(), runtime, } @@ -135,6 +136,7 @@ async fn a_launch_binds_the_repository_and_the_model_default() { ("workflow.toml", SETTINGS), ]), inputs: BTreeMap::new(), + vars: BTreeMap::new(), launch: Launch { model: Some("gpt-5.4".to_string()), provider: None, diff --git a/lib/components/fabro-petri/tests/hooks.rs b/lib/components/fabro-petri/tests/hooks.rs index 2c17eed9e..cd90195ff 100644 --- a/lib/components/fabro-petri/tests/hooks.rs +++ b/lib/components/fabro-petri/tests/hooks.rs @@ -92,6 +92,7 @@ fn admit(workflow: &str, settings: &str) -> AdmittedGraphs { project_toml: None, }, inputs: BTreeMap::new(), + vars: BTreeMap::new(), launch: Launch::default(), runtime: RuntimeSpec::default(), }; diff --git a/lib/components/fabro-petri/tests/projection.rs b/lib/components/fabro-petri/tests/projection.rs index 42cf6876d..9c4698137 100644 --- a/lib/components/fabro-petri/tests/projection.rs +++ b/lib/components/fabro-petri/tests/projection.rs @@ -36,8 +36,8 @@ use fabro_store::platform_records::{ }; use fabro_store::test_support; use fabro_types::{ - BlobHash, PetriAdmission, PetriGraphRef, RunEngine, RunId, RunStatus, StageHandler, StageId, - StageState, test_support as types_support, + BlobHash, PetriAdmission, PetriGraphRef, RunId, RunStatus, StageHandler, StageId, StageState, + test_support as types_support, }; use petri_execution::host::{self, HostRun}; use petri_frontend_fabro::Fabro; @@ -161,13 +161,13 @@ async fn create_run(pool: &DbPool, run_id: RunId, goal: &str) { let store = PlatformRecordStore::new(pool.clone()); let mut spec = types_support::test_run_spec(); spec.run_id = run_id; - spec.engine = RunEngine::Petri(PetriAdmission { + spec.admission = PetriAdmission { graph: PetriGraphRef { blob: BlobHash::new(b"graph"), digest: "digest".to_string(), }, children: Vec::new(), - }); + }; store .append( &run_id, diff --git a/lib/components/fabro-petri/tests/support/mod.rs b/lib/components/fabro-petri/tests/support/mod.rs index 95e2adeb5..0458427e7 100644 --- a/lib/components/fabro-petri/tests/support/mod.rs +++ b/lib/components/fabro-petri/tests/support/mod.rs @@ -85,6 +85,7 @@ pub(crate) fn admit( let request = CheckRequest { bundle: bundle(files), inputs: BTreeMap::new(), + vars: BTreeMap::new(), launch, runtime: runtime.clone(), }; diff --git a/lib/components/fabro-store/src/platform_records.rs b/lib/components/fabro-store/src/platform_records.rs index 1759be7eb..7d82c5761 100644 --- a/lib/components/fabro-store/src/platform_records.rs +++ b/lib/components/fabro-store/src/platform_records.rs @@ -725,7 +725,7 @@ fn run_created_record(run_id: RunId, props: &RunCreatedProps) -> RunCreatedRecor spec_blob: props.spec_blob, git: props.git.clone(), fork_source_ref: props.fork_source_ref.clone(), - engine: props.engine.clone(), + admission: props.admission.clone(), }, title: props.title.clone(), parent_id: props.parent_id, diff --git a/lib/components/fabro-store/src/run_state.rs b/lib/components/fabro-store/src/run_state.rs index 002475f05..081a0f27e 100644 --- a/lib/components/fabro-store/src/run_state.rs +++ b/lib/components/fabro-store/src/run_state.rs @@ -868,7 +868,7 @@ fn projection_from_created(event: &EventEnvelope) -> Result { spec_blob: props.spec_blob, git: props.git.clone(), fork_source_ref: props.fork_source_ref.clone(), - engine: props.engine.clone(), + admission: props.admission.clone(), }; let mut projection = RunProjection::new(title, spec, stored.ts); diff --git a/lib/components/fabro-store/src/run_summary_store.rs b/lib/components/fabro-store/src/run_summary_store.rs index a98576e44..24b38756e 100644 --- a/lib/components/fabro-store/src/run_summary_store.rs +++ b/lib/components/fabro-store/src/run_summary_store.rs @@ -908,89 +908,11 @@ WHERE id = ? .ok_or_else(|| Error::RunNotFound(entry.run_id.to_string()))?; let run = &record.run; - let diff = run.diff.unwrap_or_default(); verify_run_field(&row, run, "id", &run.id.to_string())?; verify_run_field(&row, run, "source_last_seq", &i64::from(record.last_seq))?; - if entry.projection.spec.engine.is_petri() { - // A Petri run's row is written by its projector from Petri's - // records and the platform records; the legacy fold knows the - // lifecycle alone, so only the identity and the legacy guard - // are checked here. - return Ok(()); - } - verify_run_field( - &row, - run, - "created_at_ms", - &run.timestamps.created_at.timestamp_millis(), - )?; - verify_run_field( - &row, - run, - "started_at_ms", - &run.timestamps - .started_at - .map(|value| value.timestamp_millis()), - )?; - verify_run_field( - &row, - run, - "last_event_at_ms", - &run.timestamps - .last_event_at - .unwrap_or(run.timestamps.created_at) - .timestamp_millis(), - )?; - verify_run_field( - &row, - run, - "completed_at_ms", - &run.timestamps - .completed_at - .map(|value| value.timestamp_millis()), - )?; - verify_run_field( - &row, - run, - "status", - &run.lifecycle.status.kind().to_string(), - )?; - verify_run_field( - &row, - run, - "archived_at_ms", - &run.lifecycle - .archived_at - .map(|value| value.timestamp_millis()), - )?; - verify_run_field( - &row, - run, - "parent_id", - &run.parent_id.map(|value| value.to_string()), - )?; - verify_run_field(&row, run, "title", &run.title)?; - verify_run_field(&row, run, "workflow_slug", &run.workflow.slug)?; - verify_run_field(&row, run, "workflow_name", &record.workflow_name)?; - verify_run_field(&row, run, "repository_name", &record.repository_name)?; - verify_run_field( - &row, - run, - "automation_id", - &run.automation - .as_ref() - .map(|automation| automation.id.clone()), - )?; - verify_run_field(&row, run, "diff_files_changed", &diff.files_changed)?; - verify_run_field(&row, run, "diff_additions", &diff.additions)?; - verify_run_field(&row, run, "diff_deletions", &diff.deletions)?; - verify_run_field(&row, run, "input_tokens", &record.input_tokens)?; - verify_run_field(&row, run, "output_tokens", &record.output_tokens)?; - verify_run_field(&row, run, "reasoning_tokens", &record.reasoning_tokens)?; - verify_run_field(&row, run, "cache_read_tokens", &record.cache_read_tokens)?; - verify_run_field(&row, run, "cache_write_tokens", &record.cache_write_tokens)?; - verify_run_field(&row, run, "total_usd_micros", &record.total_usd_micros)?; - verify_run_json_field(&row, run)?; + // The run's row is written by its projector from Petri's records + // and the platform records; the legacy fold knows the lifecycle + // alone, so only the identity and the legacy guard are checked here. Ok(()) } @@ -1264,9 +1186,7 @@ pub(crate) fn platform_record_written( entry: &ProjectedRun, envelope: &EventEnvelope, ) -> Option { - if !entry.projection.spec.engine.is_petri() { - return None; - } + let _ = entry; platform_records::platform_record_for(&envelope.event) } @@ -1322,19 +1242,6 @@ where Ok(()) } -fn verify_run_json_field(row: &SqliteRow, run: &Run) -> Result<()> { - let stored_json: String = row.try_get("summary_json")?; - let stored: serde_json::Value = serde_json::from_str(&stored_json)?; - let expected = serde_json::to_value(run)?; - if stored != expected { - return Err(Error::RunSummaryMismatch { - run_id: run.id.to_string(), - field: "summary_json", - }); - } - Ok(()) -} - fn decode_event_row( row: &SqliteRow, expected_run_id: &RunId, @@ -1675,9 +1582,9 @@ mod tests { use chrono::{DateTime, Utc}; use fabro_types::{ AutomationRef, BlockedReason, Conclusion, DiffSummary, EventEnvelope, FailureReason, Graph, - PendingReason, PullRequestCreationId, RunDiff, RunId, RunProjection, RunSize, RunSpec, - RunStatus, RunStatusKind, RunTiming, SessionId, StageId, StageOutcome, SuccessReason, - WorkflowSettings, test_support, + PendingReason, PetriAdmission, PullRequestCreationId, RunDiff, RunId, RunProjection, + RunSize, RunSpec, RunStatus, RunStatusKind, RunTiming, SessionId, StageId, StageOutcome, + SuccessReason, WorkflowSettings, test_support, }; use lithos_llm::types::{Cost, CostSource, TokenCounts, Usage}; use strum::VariantArray as _; @@ -1718,7 +1625,7 @@ mod tests { spec_blob: None, git: None, fork_source_ref: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }, created_at, ) diff --git a/lib/components/fabro-store/src/slate/mod.rs b/lib/components/fabro-store/src/slate/mod.rs index b55e047e5..8fa4d3a05 100644 --- a/lib/components/fabro-store/src/slate/mod.rs +++ b/lib/components/fabro-store/src/slate/mod.rs @@ -252,10 +252,8 @@ impl Database { Err(error) => return Err(error), } }; - if legacy.spec.engine.is_petri() { - if let Some(petri) = self.run_summary_store.load_petri_projection(run_id).await? { - return Ok(Some(petri)); - } + if let Some(petri) = self.run_summary_store.load_petri_projection(run_id).await? { + return Ok(Some(petri)); } Ok(Some(legacy)) } @@ -340,8 +338,8 @@ fn active_run_from( mod tests { use chrono::{DateTime, Utc}; use fabro_types::{ - AttrValue, FailureReason, Graph, RunControlAction, RunSpec, RunStatus, StageId, - SuccessReason, WorkflowSettings, test_support, + AttrValue, FailureReason, Graph, PetriAdmission, RunControlAction, RunSpec, RunStatus, + StageId, SuccessReason, WorkflowSettings, test_support, }; use futures::TryStreamExt; use object_store::memory::InMemory; @@ -498,7 +496,7 @@ mod tests { dirty: fabro_types::DirtyStatus::Clean, }), fork_source_ref: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), } } diff --git a/lib/components/fabro-workflow/src/event/convert.rs b/lib/components/fabro-workflow/src/event/convert.rs index a1fd73fb7..bd48b3241 100644 --- a/lib/components/fabro-workflow/src/event/convert.rs +++ b/lib/components/fabro-workflow/src/event/convert.rs @@ -76,7 +76,7 @@ fn event_body_from_event(event: &Event) -> EventBody { retried_from, parent_id, web_url, - engine, + admission, .. } => EventBody::RunCreated(fabro_types::RunCreatedProps { title: title.clone(), @@ -97,7 +97,7 @@ fn event_body_from_event(event: &Event) -> EventBody { retried_from: *retried_from, parent_id: *parent_id, web_url: web_url.clone(), - engine: engine.clone(), + admission: admission.clone(), }), Event::WorkflowRunStarted { name, @@ -2092,7 +2092,7 @@ mod tests { retried_from: None, parent_id: None, web_url: None, - engine: ::fabro_types::RunEngine::Legacy, + admission: ::fabro_types::PetriAdmission::default(), }); let actor = stored.actor.as_ref().expect("actor set"); assert_eq!(actor, &user_principal("alice")); diff --git a/lib/components/fabro-workflow/src/event/events.rs b/lib/components/fabro-workflow/src/event/events.rs index 20435048b..710e4081a 100644 --- a/lib/components/fabro-workflow/src/event/events.rs +++ b/lib/components/fabro-workflow/src/event/events.rs @@ -3,8 +3,8 @@ use std::collections::BTreeMap; use ::fabro_types::{ AutomationRef, BlobHash, BlockedReason, CommandTermination, DiffSummary, FailureReason, ForkSourceRef, GitContext, PairId, PairMessageId, PairSystemMessageKind, PairTarget, - ParallelBranchId, ParallelBranchResult, PendingReason, PermissionLevel, Principal, - PullRequestCreationId, PullRequestLink, ReviewTarget, RunEngine, RunFailure, RunId, + ParallelBranchId, ParallelBranchResult, PendingReason, PermissionLevel, PetriAdmission, + Principal, PullRequestCreationId, PullRequestLink, ReviewTarget, RunFailure, RunId, RunNoticeLevel, RunPairEndedReason, RunPairFailedReason, RunProvenance, RunRunnableSource, RunTarget, RunTiming, SandboxProviderKind, StageId, StageOutcome, StageTiming, SuccessReason, WorkflowVersionId, run_event as fabro_types, @@ -55,8 +55,7 @@ pub enum Event { #[serde(default, skip_serializing_if = "Option::is_none")] web_url: Option, /// The engine the run was created for, with what it admitted. - #[serde(default, skip_serializing_if = "RunEngine::is_legacy")] - engine: RunEngine, + admission: PetriAdmission, }, WorkflowRunStarted { name: String, diff --git a/lib/components/fabro-workflow/src/event/sink.rs b/lib/components/fabro-workflow/src/event/sink.rs index eae2c8553..165b5b5b2 100644 --- a/lib/components/fabro-workflow/src/event/sink.rs +++ b/lib/components/fabro-workflow/src/event/sink.rs @@ -353,7 +353,7 @@ mod tests { use std::sync::atomic::{AtomicUsize, Ordering}; use ::fabro_types::{Graph, RunNoticeLevel, WorkflowSettings, fixtures}; - use fabro_types::test_support; + use fabro_types::{PetriAdmission, test_support}; use lithos_llm::types::ReasoningOutput; use pebble_coding_agent::events::{CodingAgentEvent, CodingEvent, Usage}; use tokio::sync::Mutex as AsyncMutex; @@ -393,7 +393,7 @@ mod tests { retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }) .await .unwrap(); diff --git a/lib/components/fabro-workflow/src/git.rs b/lib/components/fabro-workflow/src/git.rs index 20499d98e..88a3527ed 100644 --- a/lib/components/fabro-workflow/src/git.rs +++ b/lib/components/fabro-workflow/src/git.rs @@ -375,7 +375,9 @@ mod tests { use fabro_dump::RunDump; use fabro_store::Database; - use fabro_types::{CommandTermination, StageModelUsage, fixtures, test_support}; + use fabro_types::{ + CommandTermination, PetriAdmission, StageModelUsage, fixtures, test_support, + }; use object_store::memory::InMemory; use super::*; @@ -552,7 +554,7 @@ mod tests { retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }) .await .unwrap(); diff --git a/lib/components/fabro-workflow/src/handler/agent.rs b/lib/components/fabro-workflow/src/handler/agent.rs index 72a18753c..9d58cdb74 100644 --- a/lib/components/fabro-workflow/src/handler/agent.rs +++ b/lib/components/fabro-workflow/src/handler/agent.rs @@ -470,7 +470,7 @@ mod tests { use fabro_graphviz::graph::AttrValue; use fabro_store::{Database, RunDatabase, StageId}; - use fabro_types::{fixtures, test_support}; + use fabro_types::{PetriAdmission, fixtures, test_support}; use lithos_llm::types::{ReasoningEffort, Speed}; use object_store::memory::InMemory; use tempfile::TempDir; @@ -532,7 +532,7 @@ mod tests { retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }, ) .await diff --git a/lib/components/fabro-workflow/src/handler/command.rs b/lib/components/fabro-workflow/src/handler/command.rs index f1fbffad6..c53f1fdfc 100644 --- a/lib/components/fabro-workflow/src/handler/command.rs +++ b/lib/components/fabro-workflow/src/handler/command.rs @@ -335,7 +335,9 @@ mod tests { use fabro_sandbox::Termination; use fabro_sandbox::test_support::{MockSandbox, exec_result}; use fabro_store::{Database, RunDatabase, StageId}; - use fabro_types::{Graph, RunProjection, RunSpec, WorkflowSettings, fixtures, test_support}; + use fabro_types::{ + Graph, PetriAdmission, RunProjection, RunSpec, WorkflowSettings, fixtures, test_support, + }; use object_store::memory::InMemory; use tokio::sync::Mutex; @@ -390,7 +392,7 @@ mod tests { spec_blob: None, git: None, fork_source_ref: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }, chrono::Utc::now(), )) @@ -496,7 +498,7 @@ mod tests { retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }, ) .await diff --git a/lib/components/fabro-workflow/src/handler/parallel.rs b/lib/components/fabro-workflow/src/handler/parallel.rs index ee8b53cf7..40303d4e9 100644 --- a/lib/components/fabro-workflow/src/handler/parallel.rs +++ b/lib/components/fabro-workflow/src/handler/parallel.rs @@ -964,7 +964,7 @@ mod tests { use fabro_graphviz::graph::{AttrValue, Edge}; use fabro_store::{Database, StageId}; - use fabro_types::{fixtures, format_blob_ref, test_support}; + use fabro_types::{PetriAdmission, fixtures, format_blob_ref, test_support}; use object_store::memory::InMemory; use super::*; @@ -1007,7 +1007,7 @@ mod tests { retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }, ) .await diff --git a/lib/components/fabro-workflow/src/handler/prompt.rs b/lib/components/fabro-workflow/src/handler/prompt.rs index 734d8395f..e138fe50c 100644 --- a/lib/components/fabro-workflow/src/handler/prompt.rs +++ b/lib/components/fabro-workflow/src/handler/prompt.rs @@ -201,7 +201,7 @@ mod tests { use fabro_graphviz::graph::AttrValue; use fabro_store::{Database, RunDatabase, StageId}; - use fabro_types::{fixtures, test_support}; + use fabro_types::{PetriAdmission, fixtures, test_support}; use lithos_llm::catalog::ProviderId; use lithos_llm::types::{ReasoningEffort, Speed}; use object_store::memory::InMemory; @@ -267,7 +267,7 @@ mod tests { retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }, ) .await diff --git a/lib/components/fabro-workflow/src/operations/archive.rs b/lib/components/fabro-workflow/src/operations/archive.rs index 7baccdc77..3f19adac9 100644 --- a/lib/components/fabro-workflow/src/operations/archive.rs +++ b/lib/components/fabro-workflow/src/operations/archive.rs @@ -137,7 +137,7 @@ mod tests { use fabro_store::Database; use fabro_types::{ - FailureReason, RunId, SuccessReason, TerminalStatus, fixtures, test_support, + FailureReason, PetriAdmission, RunId, SuccessReason, TerminalStatus, fixtures, test_support, }; use object_store::memory::InMemory; @@ -233,7 +233,7 @@ mod tests { retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }) .await .unwrap(); diff --git a/lib/components/fabro-workflow/src/operations/create.rs b/lib/components/fabro-workflow/src/operations/create.rs index 555d900f5..c63325a3a 100644 --- a/lib/components/fabro-workflow/src/operations/create.rs +++ b/lib/components/fabro-workflow/src/operations/create.rs @@ -16,7 +16,7 @@ use fabro_llm::lithos_catalog::Catalog; use fabro_store::{BlobStore, Database}; use fabro_template::TemplateContext; use fabro_types::{ - AutomationRef, BlobHash, ForkSourceRef, GitContext, ManifestPath, RunEngine, RunId, + AutomationRef, BlobHash, ForkSourceRef, GitContext, ManifestPath, PetriAdmission, RunId, RunProvenance, RunTarget, WorkflowSettings, WorkflowVersionId, }; use fabro_util::json::normalize_json_value; @@ -58,6 +58,8 @@ pub struct CreateRunInput { /// has the web UI enabled. Recorded on the `run.created` event so attach /// replays can surface the link. pub web_url: Option, + /// What Petri admitted for the run. + pub admission: PetriAdmission, } impl CreateRunInput { @@ -86,6 +88,7 @@ impl CreateRunInput { provenance, configured_providers, web_url, + admission, } = self; ( CreateRunCompileInput { @@ -110,7 +113,7 @@ impl CreateRunInput { parent_id, provenance, web_url, - engine: fabro_types::RunEngine::Legacy, + admission, }, ) } @@ -145,8 +148,8 @@ pub struct CreateRunPersistenceMetadata { pub parent_id: Option, pub provenance: RunProvenance, pub web_url: Option, - /// The engine the run was created for, with what it admitted. - pub engine: RunEngine, + /// What Petri admitted for the run. + pub admission: PetriAdmission, } #[derive(Debug)] @@ -217,7 +220,7 @@ pub struct CreateRunPersistenceInput { parent_id: Option, provenance: RunProvenance, web_url: Option, - engine: RunEngine, + admission: PetriAdmission, } impl CreateRunPersistenceInput { @@ -503,7 +506,7 @@ pub fn assemble_create_run_persistence_input( parent_id, provenance, web_url, - engine, + admission, } = metadata; let run_dir = Storage::new(storage_root) .run_scratch(&run_id) @@ -525,7 +528,7 @@ pub fn assemble_create_run_persistence_input( parent_id, provenance, web_url, - engine, + admission, } } @@ -548,7 +551,7 @@ pub async fn persist_create_run( parent_id, provenance, web_url, - engine, + admission, } = input; let MaterializedRun { validated, @@ -583,7 +586,7 @@ pub async fn persist_create_run( spec_blob: None, git, fork_source_ref, - engine, + admission, }; pipeline::persist(validated, PersistOptions { run_dir: persisted_run_dir, @@ -662,7 +665,7 @@ async fn persist_created_run( retried_from: None, parent_id, web_url, - engine: record.engine.clone(), + admission: record.admission.clone(), }; let run_store = event::create_run( store, @@ -770,7 +773,7 @@ mod tests { use fabro_store::Database; use fabro_types::settings::InterpString; use fabro_types::settings::run::RunMode; - use fabro_types::{EventBody, WorkflowSettings, fixtures, test_support}; + use fabro_types::{EventBody, PetriAdmission, WorkflowSettings, fixtures, test_support}; use fabro_util::error::collect_chain; use fabro_validate::Severity; use lithos_llm::catalog::builtin; @@ -1705,6 +1708,7 @@ mod tests { workflow_source: None, }; let request = CreateRunInput { + admission: PetriAdmission::default(), workflow: WorkflowInput::DotSource { source: MINIMAL_DOT.to_string(), base_dir: None, @@ -1825,7 +1829,7 @@ mod tests { parent_id: None, provenance: test_support::test_run_provenance(), web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }); let definition = input .definition() @@ -1847,6 +1851,7 @@ mod tests { workflow_source: None, }; let request = CreateRunInput { + admission: PetriAdmission::default(), workflow: WorkflowInput::Path(dot_path.clone()), settings: test_default_settings(), vars: HashMap::new(), @@ -1919,6 +1924,7 @@ mod tests { let err = create( &store, CreateRunInput { + admission: PetriAdmission::default(), workflow: WorkflowInput::DotSource { source: dot.to_string(), base_dir: None, @@ -1966,6 +1972,7 @@ mod tests { let created = create( &store, CreateRunInput { + admission: PetriAdmission::default(), workflow: WorkflowInput::DotSource { source: MINIMAL_DOT.to_string(), base_dir: None, @@ -2119,6 +2126,7 @@ mod tests { let created = create( store.as_ref(), CreateRunInput { + admission: PetriAdmission::default(), workflow: WorkflowInput::DotSource { source: MODEL_DOT.replace("MODEL_SELECTOR", selector), base_dir: None, @@ -2208,6 +2216,7 @@ mod tests { let created = create( &store, CreateRunInput { + admission: PetriAdmission::default(), workflow: WorkflowInput::DotSource { source: MINIMAL_DOT.to_string(), base_dir: None, @@ -2296,6 +2305,7 @@ mod tests { let created = create( &store, CreateRunInput { + admission: PetriAdmission::default(), workflow: WorkflowInput::DotSource { source: MINIMAL_DOT.to_string(), base_dir: None, @@ -2346,6 +2356,7 @@ mod tests { let created = create( &store, CreateRunInput { + admission: PetriAdmission::default(), workflow: WorkflowInput::DotSource { source: MINIMAL_DOT.to_string(), base_dir: None, @@ -2402,6 +2413,7 @@ mod tests { let created = create( &store, CreateRunInput { + admission: PetriAdmission::default(), workflow: WorkflowInput::DotSource { source: MINIMAL_DOT.to_string(), base_dir: None, @@ -2452,6 +2464,7 @@ mod tests { let created = create( &store, CreateRunInput { + admission: PetriAdmission::default(), workflow: WorkflowInput::DotSource { source: MINIMAL_DOT.to_string(), base_dir: None, @@ -2532,6 +2545,7 @@ mod tests { let created = create( store.as_ref(), CreateRunInput { + admission: PetriAdmission::default(), workflow: WorkflowInput::DotSource { source: MINIMAL_DOT.to_string(), base_dir: None, @@ -2586,6 +2600,7 @@ mod tests { let created = create( store.as_ref(), CreateRunInput { + admission: PetriAdmission::default(), workflow: WorkflowInput::DotSource { source: MINIMAL_DOT.to_string(), base_dir: None, diff --git a/lib/components/fabro-workflow/src/operations/fork.rs b/lib/components/fabro-workflow/src/operations/fork.rs index 498eb757e..220364ad7 100644 --- a/lib/components/fabro-workflow/src/operations/fork.rs +++ b/lib/components/fabro-workflow/src/operations/fork.rs @@ -178,7 +178,7 @@ async fn persist_forked_run( retried_from: None, parent_id: None, web_url: None, - engine: spec.engine.clone(), + admission: spec.admission.clone(), }; let run_store = event::create_run(store, &spec.run_id, &first_event, Utc::now()) .await @@ -296,7 +296,7 @@ mod tests { use fabro_graphviz::graph::Graph; use fabro_store::{Database, RunProjectionReducer}; - use fabro_types::{StageId, WorkflowSettings, fixtures, test_support}; + use fabro_types::{PetriAdmission, StageId, WorkflowSettings, fixtures, test_support}; use object_store::memory::InMemory; use super::*; @@ -414,7 +414,7 @@ mod tests { retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }) .await .unwrap(); diff --git a/lib/components/fabro-workflow/src/operations/retry.rs b/lib/components/fabro-workflow/src/operations/retry.rs index aff40065d..0307b2abc 100644 --- a/lib/components/fabro-workflow/src/operations/retry.rs +++ b/lib/components/fabro-workflow/src/operations/retry.rs @@ -59,7 +59,7 @@ pub async fn retry_run( spec_blob, git, fork_source_ref, - engine, + admission, } = source.spec; let settings = serde_json::to_value(&settings).map_err(|err| Error::engine(err.to_string()))?; @@ -86,9 +86,9 @@ pub async fn retry_run( retried_from: Some(source_run_id), parent_id, web_url: input.web_url.clone(), - // The admitted graph is content-addressed, so a retry runs on the - // same engine from the same admission. - engine, + // The admitted graph is content-addressed, so a retry runs from the + // same admission. + admission, }; let retry_store = event::create_run(store, &new_run_id, &first_event, Utc::now()) .await @@ -125,8 +125,8 @@ mod tests { use fabro_store::{Database, RunProjectionReducer}; use fabro_types::{ AuthMethod, BlobHash, DirtyStatus, FailureReason, ForkSourceRef, GitContext, Graph, - IdpIdentity, Principal, PullRequestLink, RunRunnableSource, RunServerProvenance, RunTarget, - RunTiming, WorkflowSettings, fixtures, test_support, + IdpIdentity, PetriAdmission, Principal, PullRequestLink, RunRunnableSource, + RunServerProvenance, RunTarget, RunTiming, WorkflowSettings, fixtures, test_support, }; use object_store::memory::InMemory; @@ -207,7 +207,7 @@ mod tests { retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }) .await .unwrap(); @@ -461,7 +461,7 @@ mod tests { retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }) .await .unwrap(); @@ -526,7 +526,7 @@ mod tests { retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }) .await .unwrap(); diff --git a/lib/components/fabro-workflow/src/operations/start.rs b/lib/components/fabro-workflow/src/operations/start.rs index 27e915f93..3afbd215c 100644 --- a/lib/components/fabro-workflow/src/operations/start.rs +++ b/lib/components/fabro-workflow/src/operations/start.rs @@ -1303,8 +1303,8 @@ mod tests { RunPrepareSettings, }; use fabro_types::{ - GitContext, ManifestPath, ModelUsage, RunTarget, StageTiming, WorkflowSettings, fixtures, - test_support, + GitContext, ManifestPath, ModelUsage, PetriAdmission, RunTarget, StageTiming, + WorkflowSettings, fixtures, test_support, }; use fabro_vault::SecretType; use lithos_llm::catalog::builtin; @@ -2407,6 +2407,7 @@ mod tests { provenance: test_support::test_run_provenance(), configured_providers: test_provider_ids(), web_url: None, + admission: PetriAdmission::default(), }, storage_root.to_path_buf(), test_catalog(), @@ -2983,6 +2984,7 @@ mod tests { provenance: test_support::test_run_provenance(), configured_providers: test_provider_ids(), web_url: None, + admission: PetriAdmission::default(), }, storage_root, test_catalog(), diff --git a/lib/components/fabro-workflow/src/operations/timeline.rs b/lib/components/fabro-workflow/src/operations/timeline.rs index 80887a6a9..b201fb65c 100644 --- a/lib/components/fabro-workflow/src/operations/timeline.rs +++ b/lib/components/fabro-workflow/src/operations/timeline.rs @@ -203,8 +203,8 @@ mod tests { use chrono::Utc; use fabro_types::{ - Checkpoint, CheckpointRecord, Graph, RunDiff, RunSpec, WorkflowSettings, fixtures, - test_support, + Checkpoint, CheckpointRecord, Graph, PetriAdmission, RunDiff, RunSpec, WorkflowSettings, + fixtures, test_support, }; use super::*; @@ -256,7 +256,7 @@ mod tests { spec_blob: None, git: None, fork_source_ref: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }, Utc::now(), ) diff --git a/lib/components/fabro-workflow/src/pipeline/execute/tests.rs b/lib/components/fabro-workflow/src/pipeline/execute/tests.rs index dff5352d6..f33d78eb2 100644 --- a/lib/components/fabro-workflow/src/pipeline/execute/tests.rs +++ b/lib/components/fabro-workflow/src/pipeline/execute/tests.rs @@ -20,7 +20,8 @@ use fabro_sandbox::{ProviderAccess, RunSandbox, SandboxSpec}; use fabro_store::Database; use fabro_types::settings::run::RunModelControls; use fabro_types::{ - Principal, RunId, SystemActorKind, WorkflowSettings, fixtures, format_blob_ref, test_support, + PetriAdmission, Principal, RunId, SystemActorKind, WorkflowSettings, fixtures, format_blob_ref, + test_support, }; use object_store::memory::InMemory; @@ -175,7 +176,7 @@ fn persisted_workflow(graph: Graph, source: String, run_dir: &Path, run_id: RunI definition_blob: None, spec_blob: None, fork_source_ref: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }, ) } @@ -227,7 +228,7 @@ async fn seed_created_and_starting( retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }) .await .unwrap(); diff --git a/lib/components/fabro-workflow/src/pipeline/finalize.rs b/lib/components/fabro-workflow/src/pipeline/finalize.rs index 6b17e0610..2c289d8bd 100644 --- a/lib/components/fabro-workflow/src/pipeline/finalize.rs +++ b/lib/components/fabro-workflow/src/pipeline/finalize.rs @@ -367,8 +367,8 @@ mod tests { use fabro_sandbox::test_support::MockSandbox; use fabro_store::{Database, RunDatabase, RunProjection}; use fabro_types::{ - EventBody, RunEvent, RunId, RunSpec, StageCompletion, WorkflowSettings, first_event_seq, - fixtures, test_support, + EventBody, PetriAdmission, RunEvent, RunId, RunSpec, StageCompletion, WorkflowSettings, + first_event_seq, fixtures, test_support, }; use object_store::memory::InMemory; @@ -470,7 +470,7 @@ mod tests { retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }) .await .unwrap(); @@ -588,7 +588,7 @@ mod tests { spec_blob: None, git: None, fork_source_ref: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }, chrono::Utc::now(), ) diff --git a/lib/components/fabro-workflow/src/pipeline/initialize.rs b/lib/components/fabro-workflow/src/pipeline/initialize.rs index c2334a39b..31af82025 100644 --- a/lib/components/fabro-workflow/src/pipeline/initialize.rs +++ b/lib/components/fabro-workflow/src/pipeline/initialize.rs @@ -778,7 +778,8 @@ mod tests { use fabro_store::{Database, RunDatabase}; use fabro_types::settings::run::RunModelControls; use fabro_types::{ - EventBody, ForkSourceRef, RunEvent, RunId, WorkflowSettings, fixtures, test_support, + EventBody, ForkSourceRef, PetriAdmission, RunEvent, RunId, WorkflowSettings, fixtures, + test_support, }; use fabro_vault::{SecretType, Vault}; use object_store::memory::InMemory; @@ -846,7 +847,7 @@ mod tests { retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }) .await .unwrap(); @@ -1011,7 +1012,7 @@ mod tests { definition_blob: None, spec_blob: None, fork_source_ref, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }, ) } diff --git a/lib/components/fabro-workflow/src/pipeline/persist.rs b/lib/components/fabro-workflow/src/pipeline/persist.rs index 455bd6171..98baa767b 100644 --- a/lib/components/fabro-workflow/src/pipeline/persist.rs +++ b/lib/components/fabro-workflow/src/pipeline/persist.rs @@ -94,7 +94,7 @@ mod tests { use fabro_graphviz::graph::{AttrValue, Edge, Graph, Node}; use fabro_store::{Database, RunDatabase}; - use fabro_types::{fixtures, test_support}; + use fabro_types::{PetriAdmission, fixtures, test_support}; use object_store::memory::InMemory; use super::*; @@ -188,7 +188,7 @@ mod tests { definition_blob: None, spec_blob: None, fork_source_ref: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), } } @@ -231,7 +231,7 @@ mod tests { retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }) .await .unwrap(); diff --git a/lib/components/fabro-workflow/src/pipeline/pull_request.rs b/lib/components/fabro-workflow/src/pipeline/pull_request.rs index 5f1d98303..63b990837 100644 --- a/lib/components/fabro-workflow/src/pipeline/pull_request.rs +++ b/lib/components/fabro-workflow/src/pipeline/pull_request.rs @@ -695,8 +695,8 @@ mod tests { use fabro_llm::{Response, ResponseStream}; use fabro_store::Database; use fabro_types::{ - RunProjection, RunSpec, SuccessReason, WorkflowSettings, first_event_seq, fixtures, - test_support, + PetriAdmission, RunProjection, RunSpec, SuccessReason, WorkflowSettings, first_event_seq, + fixtures, test_support, }; use fabro_vault::{SecretType, Vault}; use httpmock::Method::{GET, POST}; @@ -831,7 +831,7 @@ capabilities = { text = true, tools = true, response_format = { json_object = tr spec_blob: None, git: None, fork_source_ref: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }, Utc::now(), ) @@ -1123,7 +1123,7 @@ capabilities = { text = true, tools = true, response_format = { json_object = tr definition_blob: None, spec_blob: None, fork_source_ref: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }; append_event(&run_store, &fixtures::RUN_1, &Event::RunCreated { run_id: fixtures::RUN_1, @@ -1144,7 +1144,7 @@ capabilities = { text = true, tools = true, response_format = { json_object = tr retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }) .await .unwrap(); @@ -1196,7 +1196,7 @@ capabilities = { text = true, tools = true, response_format = { json_object = tr definition_blob: None, spec_blob: None, fork_source_ref: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }; append_event(&run_store, &fixtures::RUN_1, &Event::RunCreated { run_id: fixtures::RUN_1, @@ -1217,7 +1217,7 @@ capabilities = { text = true, tools = true, response_format = { json_object = tr retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }) .await .unwrap(); @@ -1620,7 +1620,7 @@ capabilities = { text = true, tools = true, response_format = { json_object = tr definition_blob: None, spec_blob: None, fork_source_ref: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }; append_event(&run_store, &fixtures::RUN_1, &Event::RunCreated { run_id: fixtures::RUN_1, @@ -1641,7 +1641,7 @@ capabilities = { text = true, tools = true, response_format = { json_object = tr retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }) .await .unwrap(); @@ -1844,7 +1844,7 @@ capabilities = { text = true, tools = true, response_format = { json_object = tr definition_blob: None, spec_blob: None, fork_source_ref: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }; append_event(&run_store, &fixtures::RUN_1, &Event::RunCreated { run_id: fixtures::RUN_1, @@ -1865,7 +1865,7 @@ capabilities = { text = true, tools = true, response_format = { json_object = tr retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }) .await .unwrap(); diff --git a/lib/components/fabro-workflow/src/run_lookup.rs b/lib/components/fabro-workflow/src/run_lookup.rs index 7e6b32262..2b0521328 100644 --- a/lib/components/fabro-workflow/src/run_lookup.rs +++ b/lib/components/fabro-workflow/src/run_lookup.rs @@ -449,7 +449,7 @@ mod tests { use std::time::Duration; use fabro_store::Database; - use fabro_types::{RunStatus, fixtures, test_support}; + use fabro_types::{PetriAdmission, RunStatus, fixtures, test_support}; use object_store::memory::InMemory; use super::scan_runs_combined; @@ -508,7 +508,7 @@ mod tests { retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }) .await .unwrap(); diff --git a/lib/components/fabro-workflow/src/runtime_store.rs b/lib/components/fabro-workflow/src/runtime_store.rs index 02f177077..94e2c9054 100644 --- a/lib/components/fabro-workflow/src/runtime_store.rs +++ b/lib/components/fabro-workflow/src/runtime_store.rs @@ -117,7 +117,7 @@ mod tests { use chrono::Utc; use fabro_types::run_event::RunSubmittedProps; - use fabro_types::{EventBody, RunEvent, fixtures, test_support}; + use fabro_types::{EventBody, PetriAdmission, RunEvent, fixtures, test_support}; use object_store::memory::InMemory; use super::RunStoreHandle; @@ -163,7 +163,7 @@ mod tests { retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }) .await .unwrap(); diff --git a/lib/components/fabro-workflow/src/stage_execution.rs b/lib/components/fabro-workflow/src/stage_execution.rs index c1f2e5507..3910cafa2 100644 --- a/lib/components/fabro-workflow/src/stage_execution.rs +++ b/lib/components/fabro-workflow/src/stage_execution.rs @@ -192,7 +192,9 @@ mod tests { use std::num::NonZeroU32; use chrono::Utc; - use fabro_types::{Graph, RunId, RunSpec, StageId, WorkflowSettings, test_support}; + use fabro_types::{ + Graph, PetriAdmission, RunId, RunSpec, StageId, WorkflowSettings, test_support, + }; use super::*; @@ -213,7 +215,7 @@ mod tests { spec_blob: None, git: None, fork_source_ref: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }; let mut projection = RunProjection::new(String::new(), spec, Utc::now()); for (node_id, visit, seq) in stages { diff --git a/lib/components/fabro-workflow/src/test_support.rs b/lib/components/fabro-workflow/src/test_support.rs index dab86df72..c18c1b593 100644 --- a/lib/components/fabro-workflow/src/test_support.rs +++ b/lib/components/fabro-workflow/src/test_support.rs @@ -12,7 +12,7 @@ use fabro_llm::lithos_catalog::Catalog; use fabro_llm::test_support::test_catalog; use fabro_sandbox::RunSandbox; use fabro_store::{ArtifactStore, RunProjection, test_support as store_test_support}; -use fabro_types::ModelRef; +use fabro_types::{ModelRef, PetriAdmission}; #[cfg(feature = "test-support")] use lithos_llm::catalog::ProviderId; use lithos_llm::catalog::{ModelId, builtin}; @@ -232,7 +232,7 @@ async fn initialized( retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }) .await .expect("failed to seed run.created event in run store"); diff --git a/lib/foundation/fabro-api/build.rs b/lib/foundation/fabro-api/build.rs index 1580c2020..9c527af4e 100644 --- a/lib/foundation/fabro-api/build.rs +++ b/lib/foundation/fabro-api/build.rs @@ -660,7 +660,6 @@ fn main() { ("EventEnvelope", "fabro_types::EventEnvelope", &[]), ("RunStreamItem", "fabro_types::RunStreamItem", &[]), ("RunStreamItemKind", "fabro_types::RunStreamItemKind", &[]), - ("RunEngine", "fabro_types::RunEngine", &[]), ("PetriAdmission", "fabro_types::PetriAdmission", &[]), ("PetriGraphRef", "fabro_types::PetriGraphRef", &[]), ("PullRequest", "fabro_types::PullRequest", &[]), diff --git a/lib/foundation/fabro-api/src/lib.rs b/lib/foundation/fabro-api/src/lib.rs index 0fa5c0abf..ac4621284 100644 --- a/lib/foundation/fabro-api/src/lib.rs +++ b/lib/foundation/fabro-api/src/lib.rs @@ -56,13 +56,13 @@ pub mod types { PullRequestCreation, PullRequestCreationId, PullRequestCreationStatus, PullRequestDetails, PullRequestDetailsStatus, PullRequestDetailsUnavailableReason, PullRequestLink, PullRequestMeta, PullRequestResponse, QuestionType, RepositoryRef, ReviewTarget, - ReviewTargetKind, Run, RunApproval, RunApprovalState, RunClientProvenance, RunEngine, - RunEvent, RunEventDetailContentKind, RunEventDetailResponse, RunFailure, RunIntent, - RunIntentArgs, RunPairStatusResponse, RunProjection, RunProvenance, RunRunnableSource, - RunSandbox, RunSandboxFailure, RunSandboxInstance, RunSandboxKind, RunSandboxPlan, - RunSandboxRuntime, RunServerProvenance, RunSessionMetadata, RunSize, RunStreamItem, - RunStreamItemKind, RunTarget, SandboxDetails, SandboxInfo, SandboxListMeta, - SandboxListResponse, SandboxProviderKind, SandboxProviderLookupError, SandboxService, + ReviewTargetKind, Run, RunApproval, RunApprovalState, RunClientProvenance, RunEvent, + RunEventDetailContentKind, RunEventDetailResponse, RunFailure, RunIntent, RunIntentArgs, + RunPairStatusResponse, RunProjection, RunProvenance, RunRunnableSource, RunSandbox, + RunSandboxFailure, RunSandboxInstance, RunSandboxKind, RunSandboxPlan, RunSandboxRuntime, + RunServerProvenance, RunSessionMetadata, RunSize, RunStreamItem, RunStreamItemKind, + RunTarget, SandboxDetails, SandboxInfo, SandboxListMeta, SandboxListResponse, + SandboxProviderKind, SandboxProviderLookupError, SandboxService, SandboxServiceListResponse, SecretMetadata, SecretType, ServerSettings, SessionDetail, SessionId, SessionStatus, SessionSummary, SessionTurn, SkillActivationSource, SkillSummary, StageCompletion, StageContextWindow, StageContextWindowUnavailableReason, StageHandler, diff --git a/lib/foundation/fabro-api/tests/run_engine_round_trip.rs b/lib/foundation/fabro-api/tests/petri_admission_round_trip.rs similarity index 50% rename from lib/foundation/fabro-api/tests/run_engine_round_trip.rs rename to lib/foundation/fabro-api/tests/petri_admission_round_trip.rs index 914ae56cf..dc0366cb9 100644 --- a/lib/foundation/fabro-api/tests/run_engine_round_trip.rs +++ b/lib/foundation/fabro-api/tests/petri_admission_round_trip.rs @@ -1,31 +1,18 @@ use std::any::{TypeId, type_name}; -use fabro_api::types::{ - PetriAdmission as ApiPetriAdmission, PetriGraphRef as ApiPetriGraphRef, - RunEngine as ApiRunEngine, -}; -use fabro_types::{PetriAdmission, PetriGraphRef, RunEngine}; +use fabro_api::types::{PetriAdmission as ApiPetriAdmission, PetriGraphRef as ApiPetriGraphRef}; +use fabro_types::{PetriAdmission, PetriGraphRef}; use serde_json::json; #[test] -fn run_engine_reuses_canonical_types() { - assert_same_type::(); +fn the_admission_reuses_canonical_types() { assert_same_type::(); assert_same_type::(); } #[test] -fn the_legacy_engine_round_trips_as_its_kind_alone() { - let value = json!({ "kind": "legacy" }); - let engine: RunEngine = serde_json::from_value(value.clone()).unwrap(); - assert!(engine.is_legacy()); - assert_eq!(serde_json::to_value(&engine).unwrap(), value); -} - -#[test] -fn the_petri_engine_round_trips_with_its_admission_flattened() { +fn the_admission_round_trips_with_its_children() { let value = json!({ - "kind": "petri", "graph": { "blob": "2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824", "digest": "sha256:root" @@ -37,13 +24,10 @@ fn the_petri_engine_round_trips_with_its_admission_flattened() { } ] }); - let engine: RunEngine = serde_json::from_value(value.clone()).unwrap(); - let admission = engine - .petri() - .expect("a Petri engine carries its admission"); + let admission: PetriAdmission = serde_json::from_value(value.clone()).unwrap(); assert_eq!(admission.graph.digest, "sha256:root"); assert_eq!(admission.children.len(), 1); - assert_eq!(serde_json::to_value(&engine).unwrap(), value); + assert_eq!(serde_json::to_value(&admission).unwrap(), value); } fn assert_same_type() { diff --git a/lib/foundation/fabro-config/src/defaults.toml b/lib/foundation/fabro-config/src/defaults.toml index bf120f513..15789b719 100644 --- a/lib/foundation/fabro-config/src/defaults.toml +++ b/lib/foundation/fabro-config/src/defaults.toml @@ -39,9 +39,6 @@ url = "http://localhost:3000" [server.scheduler] max_concurrent_runs = 5 -[server.execution] -engine = "legacy" - [server.artifacts] provider = "local" prefix = "" diff --git a/lib/foundation/fabro-config/src/layers/combine.rs b/lib/foundation/fabro-config/src/layers/combine.rs index e9e13aee6..c818a808e 100644 --- a/lib/foundation/fabro-config/src/layers/combine.rs +++ b/lib/foundation/fabro-config/src/layers/combine.rs @@ -1,5 +1,6 @@ use std::collections::{BTreeMap, HashMap}; +use fabro_types::PermissionLevel; use fabro_types::settings::cli::{CliAuthStrategy, OutputFormat, OutputVerbosity}; use fabro_types::settings::run::{ApprovalMode, EnvironmentNetworkMode, MergeStrategy, RunMode}; use fabro_types::settings::server::{ @@ -7,7 +8,6 @@ use fabro_types::settings::server::{ WebhookStrategy, }; use fabro_types::settings::{Duration, InterpString, Size}; -use fabro_types::{Engine, PermissionLevel}; use super::LogFilter; use super::cli::{CliAuthLayer, CliLoggingLayer, CliTargetLayer}; @@ -75,7 +75,6 @@ impl_combine_or_option!( HookTlsMode, MergeStrategy, RunMode, - Engine, GithubIntegrationStrategy, LogDestination, ObjectStoreProvider, diff --git a/lib/foundation/fabro-config/src/layers/mod.rs b/lib/foundation/fabro-config/src/layers/mod.rs index ec17e6cc4..f8b625c1d 100644 --- a/lib/foundation/fabro-config/src/layers/mod.rs +++ b/lib/foundation/fabro-config/src/layers/mod.rs @@ -36,10 +36,10 @@ pub use run::{ pub use server::{ GithubIntegrationLayer, IntegrationWebhooksLayer, ObjectStoreLocalLayer, ObjectStoreS3Layer, ServerApiLayer, ServerArtifactsLayer, ServerAuthGithubLayer, ServerAuthLayer, - ServerExecutionLayer, ServerIntegrationsLayer, ServerLayer, ServerListenLayer, - ServerLoggingLayer, ServerSandboxLayer, ServerSandboxProviderLayer, - ServerSandboxProvidersLayer, ServerSchedulerLayer, ServerSlateDbLayer, ServerStorageLayer, - ServerWebLayer, SlackIntegrationLayer, + ServerIntegrationsLayer, ServerLayer, ServerListenLayer, ServerLoggingLayer, + ServerSandboxLayer, ServerSandboxProviderLayer, ServerSandboxProvidersLayer, + ServerSchedulerLayer, ServerSlateDbLayer, ServerStorageLayer, ServerWebLayer, + SlackIntegrationLayer, }; pub use settings::SettingsLayer; pub use workflow::WorkflowLayer; diff --git a/lib/foundation/fabro-config/src/layers/server.rs b/lib/foundation/fabro-config/src/layers/server.rs index 5dc659e86..7a0661bdc 100644 --- a/lib/foundation/fabro-config/src/layers/server.rs +++ b/lib/foundation/fabro-config/src/layers/server.rs @@ -2,12 +2,12 @@ use std::collections::BTreeMap; +use fabro_types::SandboxProviderKind; use fabro_types::settings::server::{ GithubIntegrationStrategy, LogDestination, ObjectStoreProvider, ServerAuthMethod, WebhookStrategy, }; use fabro_types::settings::{Duration, InterpString}; -use fabro_types::{Engine, SandboxProviderKind}; use serde::{Deserialize, Serialize}; use super::LogFilter; @@ -35,8 +35,6 @@ pub struct ServerLayer { #[serde(default, skip_serializing_if = "Option::is_none")] pub scheduler: Option, #[serde(default, skip_serializing_if = "Option::is_none")] - pub execution: Option, - #[serde(default, skip_serializing_if = "Option::is_none")] pub logging: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub integrations: Option, @@ -217,16 +215,6 @@ pub struct ServerSchedulerLayer { pub max_concurrent_runs: Option, } -/// `[server.execution]` — how this server executes the runs it admits. -#[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize, fabro_macros::Combine)] -#[serde(deny_unknown_fields)] -pub struct ServerExecutionLayer { - /// The engine for every run whose workflow version names none: - /// `"legacy"` (the default) or `"petri"`. - #[serde(default, skip_serializing_if = "Option::is_none")] - pub engine: Option, -} - /// `[server.logging]` — process-owned logging configuration for the server. #[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize, fabro_macros::Combine)] #[serde(deny_unknown_fields)] diff --git a/lib/foundation/fabro-config/src/layers/workflow.rs b/lib/foundation/fabro-config/src/layers/workflow.rs index ff2d9fd7a..5a20da896 100644 --- a/lib/foundation/fabro-config/src/layers/workflow.rs +++ b/lib/foundation/fabro-config/src/layers/workflow.rs @@ -1,6 +1,5 @@ //! Sparse `[workflow]` settings layer definitions. -use fabro_types::Engine; use serde::{Deserialize, Serialize}; use super::maps::ReplaceMap; @@ -18,8 +17,4 @@ pub struct WorkflowLayer { pub graph: Option, #[serde(default, skip_serializing_if = "ReplaceMap::is_empty")] pub metadata: ReplaceMap, - /// The engine the workflow asks to run on: `"petri"` or `"legacy"`. - /// Unset leaves the choice to the server's `[server.execution] engine`. - #[serde(default, skip_serializing_if = "Option::is_none")] - pub engine: Option, } diff --git a/lib/foundation/fabro-config/src/lib.rs b/lib/foundation/fabro-config/src/lib.rs index b845bd36c..33eeed889 100644 --- a/lib/foundation/fabro-config/src/lib.rs +++ b/lib/foundation/fabro-config/src/lib.rs @@ -52,10 +52,10 @@ pub use layers::{ RunIntegrationsLayer, RunLayer, RunMetaBranchLayer, RunModelControlsLayer, RunModelLayer, RunPrepareLayer, RunPullRequestLayer, RunRunBranchLayer, RunScmLayer, ScmGitHubLayer, ServerApiLayer, ServerArtifactsLayer, ServerAuthGithubLayer, ServerAuthLayer, - ServerExecutionLayer, ServerIntegrationsLayer, ServerLayer, ServerListenLayer, - ServerLoggingLayer, ServerSandboxLayer, ServerSandboxProviderLayer, - ServerSandboxProvidersLayer, ServerSchedulerLayer, ServerSlateDbLayer, ServerStorageLayer, - ServerWebLayer, SettingsLayer, SlackIntegrationLayer, StickyMap, StringOrSplice, WorkflowLayer, + ServerIntegrationsLayer, ServerLayer, ServerListenLayer, ServerLoggingLayer, + ServerSandboxLayer, ServerSandboxProviderLayer, ServerSandboxProvidersLayer, + ServerSchedulerLayer, ServerSlateDbLayer, ServerStorageLayer, ServerWebLayer, SettingsLayer, + SlackIntegrationLayer, StickyMap, StringOrSplice, WorkflowLayer, }; pub use logging::{resolve_log_destination, resolve_log_destination_with_env}; pub use parse::ParseError; diff --git a/lib/foundation/fabro-config/src/resolve/server.rs b/lib/foundation/fabro-config/src/resolve/server.rs index 097a8bc43..a4321c1a4 100644 --- a/lib/foundation/fabro-config/src/resolve/server.rs +++ b/lib/foundation/fabro-config/src/resolve/server.rs @@ -6,11 +6,10 @@ use fabro_types::settings::server::{ GithubIntegrationSettings, GithubIntegrationStrategy, IntegrationWebhooksSettings, ObjectStoreProvider, ObjectStoreSettings, SandboxPluginSettings, ServerApiSettings, ServerArtifactsSettings, ServerAuthGithubSettings, ServerAuthMethod, ServerAuthSettings, - ServerExecutionSettings, ServerIntegrationsSettings, ServerListenSettings, - ServerLoggingSettings, ServerNamespace, ServerSandboxProviderSettings, - ServerSandboxProvidersSettings, ServerSandboxSettings, ServerSchedulerSettings, - ServerSlateDbSettings, ServerStorageSettings, ServerWebSettings, SlackIntegrationSettings, - WebhookStrategy, + ServerIntegrationsSettings, ServerListenSettings, ServerLoggingSettings, ServerNamespace, + ServerSandboxProviderSettings, ServerSandboxProvidersSettings, ServerSandboxSettings, + ServerSchedulerSettings, ServerSlateDbSettings, ServerStorageSettings, ServerWebSettings, + SlackIntegrationSettings, WebhookStrategy, }; use fabro_util::Home; @@ -53,13 +52,6 @@ pub fn resolve_server(layer: &ServerLayer, errors: &mut Vec) -> Se .and_then(|scheduler| scheduler.max_concurrent_runs) .expect("defaults.toml should provide server.scheduler.max_concurrent_runs"), }, - execution: ServerExecutionSettings { - engine: layer - .execution - .as_ref() - .and_then(|execution| execution.engine) - .expect("defaults.toml should provide server.execution.engine"), - }, logging: ServerLoggingSettings { level: layer .logging diff --git a/lib/foundation/fabro-config/src/resolve/workflow.rs b/lib/foundation/fabro-config/src/resolve/workflow.rs index cbf54e22f..d6db3a897 100644 --- a/lib/foundation/fabro-config/src/resolve/workflow.rs +++ b/lib/foundation/fabro-config/src/resolve/workflow.rs @@ -15,6 +15,5 @@ pub fn resolve_workflow( .clone() .expect("defaults.toml should provide workflow.graph"), metadata: layer.metadata.clone().into_inner(), - engine: layer.engine, } } diff --git a/lib/foundation/fabro-config/src/tests/resolve_server.rs b/lib/foundation/fabro-config/src/tests/resolve_server.rs index 435c4d767..0fc982b98 100644 --- a/lib/foundation/fabro-config/src/tests/resolve_server.rs +++ b/lib/foundation/fabro-config/src/tests/resolve_server.rs @@ -67,7 +67,6 @@ fn resolves_server_defaults_from_empty_settings() { assert!(settings.web.enabled); assert_eq!(settings.web.url, "http://localhost:3000"); assert_eq!(settings.scheduler.max_concurrent_runs, 5); - assert_eq!(settings.execution.engine, fabro_types::Engine::Legacy); assert_eq!(settings.logging.destination, LogDestination::File); match settings.listen { @@ -713,17 +712,3 @@ methods = ["dev-token", "github"] assert!(dev_token_auth_enabled(&both)); assert!(!dev_token_auth_enabled(&SettingsLayer::default())); } - -#[test] -fn server_execution_engine_names_petri() { - let settings = resolve_server(&parse( - r#" -_version = 1 - -[server.execution] -engine = "petri" -"#, - )); - - assert_eq!(settings.execution.engine, fabro_types::Engine::Petri); -} diff --git a/lib/foundation/fabro-config/src/tests/resolve_workflow.rs b/lib/foundation/fabro-config/src/tests/resolve_workflow.rs index 5a1758fb3..d9e2c6b42 100644 --- a/lib/foundation/fabro-config/src/tests/resolve_workflow.rs +++ b/lib/foundation/fabro-config/src/tests/resolve_workflow.rs @@ -42,8 +42,9 @@ tier = "gold" } #[test] -fn resolves_workflow_engine_when_named() { - let workflow = super::workflow_settings_from_toml( +fn a_workflow_engine_key_is_unknown() { + // Every run executes on Petri; the workflow table names no engine. + let error = super::workflow_settings_from_toml( r#" _version = 1 @@ -51,35 +52,6 @@ _version = 1 engine = "petri" "#, ) - .expect("workflow settings should resolve") - .workflow; - - assert_eq!(workflow.engine, Some(fabro_types::Engine::Petri)); -} - -#[test] -fn workflow_engine_is_unset_when_unnamed() { - let workflow = super::workflow_settings_from_layer(SettingsLayer::default()) - .expect("empty settings should resolve") - .workflow; - - assert_eq!(workflow.engine, None); -} - -#[test] -fn rejects_an_unknown_workflow_engine() { - let error = super::workflow_settings_from_toml( - r#" -_version = 1 - -[workflow] -engine = "steam" -"#, - ) - .expect_err("an unknown engine should not parse"); - - assert!( - error.to_string().contains("engine") || format!("{error:#}").contains("steam"), - "unexpected error: {error:#}" - ); + .expect_err("an engine key is not a workflow setting"); + assert!(error.to_string().contains("engine"), "{error}"); } diff --git a/lib/foundation/fabro-static/src/env_vars.rs b/lib/foundation/fabro-static/src/env_vars.rs index ff3f29fbb..c2dccc58a 100644 --- a/lib/foundation/fabro-static/src/env_vars.rs +++ b/lib/foundation/fabro-static/src/env_vars.rs @@ -27,7 +27,6 @@ impl EnvVars { "FABRO_PUSH_CRED_REFRESH_INTERVAL_SECONDS"; pub const FABRO_QUIET: &'static str = "FABRO_QUIET"; pub const FABRO_SERVER: &'static str = "FABRO_SERVER"; - pub const FABRO_SERVER_ENGINE: &'static str = "FABRO_SERVER_ENGINE"; pub const FABRO_SERVER_MAX_CONCURRENT_RUNS: &'static str = "FABRO_SERVER_MAX_CONCURRENT_RUNS"; pub const FABRO_SLACK_APP_TOKEN: &'static str = "FABRO_SLACK_APP_TOKEN"; pub const FABRO_SLACK_BOT_TOKEN: &'static str = "FABRO_SLACK_BOT_TOKEN"; @@ -238,7 +237,6 @@ mod tests { EnvVars::FABRO_PUSH_CRED_REFRESH_INTERVAL_SECONDS, EnvVars::FABRO_QUIET, EnvVars::FABRO_SERVER, - EnvVars::FABRO_SERVER_ENGINE, EnvVars::FABRO_SERVER_MAX_CONCURRENT_RUNS, EnvVars::FABRO_SLACK_APP_TOKEN, EnvVars::FABRO_SLACK_BOT_TOKEN, diff --git a/lib/foundation/fabro-types/src/engine.rs b/lib/foundation/fabro-types/src/engine.rs index bdf88198e..2deae48cc 100644 --- a/lib/foundation/fabro-types/src/engine.rs +++ b/lib/foundation/fabro-types/src/engine.rs @@ -1,48 +1,15 @@ -//! Which engine runs a workflow, and what Petri admitted for a run. +//! What Petri admitted for a run. //! -//! A run goes to Petri when its workflow version says so (`engine = "petri"` -//! in the `[workflow]` table of `workflow.toml`) or when the server's -//! `[server.execution] engine` default says so. The choice is recorded on -//! the run's spec as [`RunEngine`], so every later reader (the executor, the -//! projection, the API) sees the same answer without re-reading settings. -//! -//! A Petri run carries the graph Petri lowered and admitted at create time: -//! [`PetriAdmission`] names the root graph and its pre-lowered children by -//! blob and digest. The run executes and resumes from that graph, never from -//! a fresh lowering, so admission-time decisions such as the pinned model -//! routes hold for the run's whole life. +//! Every run executes on Petri. A run carries the graph Petri lowered and +//! admitted at create time: [`PetriAdmission`] names the root graph and its +//! pre-lowered children by blob and digest. The run executes and resumes +//! from that graph, never from a fresh lowering, so admission-time +//! decisions such as the pinned model routes hold for the run's whole life. use serde::{Deserialize, Serialize}; -use strum::{Display, EnumString, IntoStaticStr, VariantArray}; use crate::BlobHash; -/// The engine a workflow version or a server names. -#[derive( - Debug, - Clone, - Copy, - Default, - PartialEq, - Eq, - Hash, - Serialize, - Deserialize, - Display, - EnumString, - IntoStaticStr, - VariantArray, -)] -#[serde(rename_all = "lowercase")] -#[strum(serialize_all = "lowercase")] -pub enum Engine { - /// Fabro's own executor in `fabro-workflow`. - #[default] - Legacy, - /// The Petri workflow engine, reached through `fabro-petri`. - Petri, -} - /// One lowered graph in the blob store: its bytes by hash, and Petri's own /// content digest of it, which is how a nested-workflow step names its child. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] @@ -60,79 +27,22 @@ pub struct PetriAdmission { pub children: Vec, } -/// The engine a run was created for, with what that engine admitted. -/// -/// Defaults to the legacy executor when absent, so specs serialized before -/// the field existed still decode. -#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] -#[serde(tag = "kind", rename_all = "lowercase")] -pub enum RunEngine { - #[default] - Legacy, - Petri(PetriAdmission), -} - -impl RunEngine { - #[must_use] - pub fn engine(&self) -> Engine { - match self { - Self::Legacy => Engine::Legacy, - Self::Petri(_) => Engine::Petri, - } - } - - #[must_use] - pub fn is_legacy(&self) -> bool { - matches!(self, Self::Legacy) - } - - #[must_use] - pub fn is_petri(&self) -> bool { - matches!(self, Self::Petri(_)) - } - - /// What Petri admitted, for a Petri run. - #[must_use] - pub fn petri(&self) -> Option<&PetriAdmission> { - match self { - Self::Legacy => None, - Self::Petri(admission) => Some(admission), - } - } -} - #[cfg(test)] mod tests { use super::*; #[test] - fn engine_names_are_lowercase_in_both_directions() { - assert_eq!(Engine::Petri.to_string(), "petri"); - assert_eq!("petri".parse::(), Ok(Engine::Petri)); - assert_eq!("legacy".parse::(), Ok(Engine::Legacy)); - assert_eq!( - serde_json::to_value(Engine::Petri).expect("engine serializes"), - serde_json::json!("petri") - ); - assert_eq!(Engine::default(), Engine::Legacy); - } - - #[test] - fn run_engine_defaults_to_legacy_and_tags_petri() { - assert_eq!(RunEngine::default(), RunEngine::Legacy); - let petri = RunEngine::Petri(PetriAdmission { + fn an_admission_without_children_omits_them() { + let admission = PetriAdmission { graph: PetriGraphRef { blob: BlobHash::new(b"graph"), digest: "abc".to_string(), }, children: Vec::new(), - }); - let value = serde_json::to_value(&petri).expect("run engine serializes"); - assert_eq!(value["kind"], "petri"); + }; + let value = serde_json::to_value(&admission).expect("admission serializes"); assert!(value.get("children").is_none()); - let decoded: RunEngine = serde_json::from_value(value).expect("run engine decodes"); - assert_eq!(decoded, petri); - assert_eq!(decoded.engine(), Engine::Petri); - assert!(decoded.petri().is_some()); + let decoded: PetriAdmission = serde_json::from_value(value).expect("admission decodes"); + assert_eq!(decoded, admission); } } diff --git a/lib/foundation/fabro-types/src/lib.rs b/lib/foundation/fabro-types/src/lib.rs index 543c02c62..38d0769b2 100644 --- a/lib/foundation/fabro-types/src/lib.rs +++ b/lib/foundation/fabro-types/src/lib.rs @@ -73,7 +73,7 @@ pub use command_output::{CommandOutputStream, CommandTermination}; pub use conclusion::{Conclusion, StageSummary}; pub use dense::{ServerSettings, UserSettings, WorkflowSettings}; pub use diff::{DiffStats, DiffSummary, RunDiff}; -pub use engine::{Engine, PetriAdmission, PetriGraphRef, RunEngine}; +pub use engine::{PetriAdmission, PetriGraphRef}; pub use event_envelope::EventEnvelope; pub use failure_signature::FailureSignature; pub use git_identity::{GitIdentity, GitIdentitySource}; diff --git a/lib/foundation/fabro-types/src/run.rs b/lib/foundation/fabro-types/src/run.rs index 4da379097..996a8849a 100644 --- a/lib/foundation/fabro-types/src/run.rs +++ b/lib/foundation/fabro-types/src/run.rs @@ -4,7 +4,7 @@ use serde::{Deserialize, Serialize}; use crate::WorkflowSettings; use crate::blob_hash::BlobHash; -use crate::engine::RunEngine; +use crate::engine::PetriAdmission; use crate::graph::Graph; use crate::principal::Principal; use crate::run_id::RunId; @@ -90,11 +90,9 @@ pub struct RunSpec { pub git: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub fork_source_ref: Option, - /// The engine the run was created for, with what it admitted. Absent in - /// a spec written before the field existed, which means the legacy - /// executor. - #[serde(default, skip_serializing_if = "RunEngine::is_legacy")] - pub engine: RunEngine, + /// What Petri admitted for the run at create time: the graphs it + /// executes and resumes from. + pub admission: PetriAdmission, } impl RunSpec { diff --git a/lib/foundation/fabro-types/src/run_event/run.rs b/lib/foundation/fabro-types/src/run_event/run.rs index 91a3b178e..8c2710b96 100644 --- a/lib/foundation/fabro-types/src/run_event/run.rs +++ b/lib/foundation/fabro-types/src/run_event/run.rs @@ -7,7 +7,7 @@ use super::{ExecOutputTail, RunNoticeLevel}; use crate::status::{BlockedReason, PendingReason, SuccessReason}; use crate::{ AutomationRef, BlobHash, DiffSummary, ForkSourceRef, GitContext, Graph, PairId, PairTarget, - RunControlAction, RunEngine, RunFailure, RunId, RunProvenance, RunTarget, RunTiming, + PetriAdmission, RunControlAction, RunFailure, RunId, RunProvenance, RunTarget, RunTiming, WorkflowSettings, WorkflowVersionId, }; @@ -47,10 +47,8 @@ pub struct RunCreatedProps { pub parent_id: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub web_url: Option, - /// The engine the run was created for, with what it admitted; absent - /// means the legacy executor. - #[serde(default, skip_serializing_if = "RunEngine::is_legacy")] - pub engine: RunEngine, + /// What Petri admitted for the run at create time. + pub admission: PetriAdmission, } #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] diff --git a/lib/foundation/fabro-types/src/settings/mod.rs b/lib/foundation/fabro-types/src/settings/mod.rs index f4c68b90b..11be8842e 100644 --- a/lib/foundation/fabro-types/src/settings/mod.rs +++ b/lib/foundation/fabro-types/src/settings/mod.rs @@ -47,9 +47,9 @@ pub use run::{ pub use server::{ GithubIntegrationSettings, IntegrationWebhooksSettings, LogDestination, ObjectStoreSettings, ServerApiSettings, ServerArtifactsSettings, ServerAuthGithubSettings, ServerAuthMethod, - ServerAuthSettings, ServerExecutionSettings, ServerIntegrationsSettings, ServerListenSettings, - ServerLoggingSettings, ServerNamespace, ServerSchedulerSettings, ServerSlateDbSettings, - ServerStorageSettings, ServerWebSettings, SlackIntegrationSettings, + ServerAuthSettings, ServerIntegrationsSettings, ServerListenSettings, ServerLoggingSettings, + ServerNamespace, ServerSchedulerSettings, ServerSlateDbSettings, ServerStorageSettings, + ServerWebSettings, SlackIntegrationSettings, }; pub use size::{ParseSizeError, Size}; pub use workflow::WorkflowNamespace; diff --git a/lib/foundation/fabro-types/src/settings/server.rs b/lib/foundation/fabro-types/src/settings/server.rs index 5b4277491..04ed82333 100644 --- a/lib/foundation/fabro-types/src/settings/server.rs +++ b/lib/foundation/fabro-types/src/settings/server.rs @@ -2,7 +2,7 @@ //! //! `[server]` is a namespace container; actual settings live in named //! subdomains (listen, api, web, auth, storage, artifacts, slatedb, -//! scheduler, execution, logging, integrations). Same-host and split-host +//! scheduler, logging, integrations). Same-host and split-host //! deployments use the same schema. use std::collections::BTreeMap; @@ -13,7 +13,7 @@ use serde::de::Error as _; use serde::{Deserialize, Deserializer, Serialize, Serializer}; use super::duration::Duration; -use crate::{Engine, SandboxProviderKind}; +use crate::SandboxProviderKind; /// A structurally resolved `[server]` view for consumers. /// @@ -33,10 +33,6 @@ pub struct ServerNamespace { pub artifacts: ServerArtifactsSettings, pub slatedb: ServerSlateDbSettings, pub scheduler: ServerSchedulerSettings, - /// `[server.execution]`: the engine a run gets when its workflow version - /// names none. Absent in settings serialized before the section existed. - #[serde(default)] - pub execution: ServerExecutionSettings, pub logging: ServerLoggingSettings, pub integrations: ServerIntegrationsSettings, } @@ -58,7 +54,6 @@ impl ServerNamespace { artifacts: ServerArtifactsSettings::default(), slatedb: ServerSlateDbSettings::default(), scheduler: ServerSchedulerSettings::default(), - execution: ServerExecutionSettings::default(), logging: ServerLoggingSettings::default(), integrations: ServerIntegrationsSettings::default(), } @@ -275,14 +270,6 @@ pub struct ServerSchedulerSettings { pub max_concurrent_runs: usize, } -/// `[server.execution]`: how this server executes the runs it admits. -#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] -pub struct ServerExecutionSettings { - /// The engine for every run whose workflow version names none. - #[serde(default)] - pub engine: Engine, -} - #[derive( Debug, Clone, diff --git a/lib/foundation/fabro-types/src/settings/workflow.rs b/lib/foundation/fabro-types/src/settings/workflow.rs index d6fedad18..9fd8b156c 100644 --- a/lib/foundation/fabro-types/src/settings/workflow.rs +++ b/lib/foundation/fabro-types/src/settings/workflow.rs @@ -1,15 +1,12 @@ //! Workflow domain. //! //! `[workflow]` is descriptive: `name`, `description`, optional `graph` (a -//! path override for the default `workflow.fabro` file), `metadata`, and the -//! optional `engine` the workflow asks to run on. +//! path override for the default `workflow.fabro` file) and `metadata`. use std::collections::HashMap; use serde::{Deserialize, Serialize}; -use crate::Engine; - /// A structurally resolved `[workflow]` view for consumers. #[derive(Debug, Clone, Default, PartialEq, Serialize, Deserialize)] pub struct WorkflowNamespace { @@ -17,8 +14,4 @@ pub struct WorkflowNamespace { pub description: Option, pub graph: String, pub metadata: HashMap, - /// The engine the workflow names; `None` leaves the choice to the - /// server's default. - #[serde(default, skip_serializing_if = "Option::is_none")] - pub engine: Option, } diff --git a/lib/foundation/fabro-types/src/test_support.rs b/lib/foundation/fabro-types/src/test_support.rs index 121388c27..22f637a82 100644 --- a/lib/foundation/fabro-types/src/test_support.rs +++ b/lib/foundation/fabro-types/src/test_support.rs @@ -1,8 +1,8 @@ use std::collections::HashMap; use crate::{ - AuthMethod, BlobHash, Graph, IdpIdentity, Principal, RunEngine, RunProvenance, RunSpec, - WorkflowSettings, WorkflowVersionId, fixtures, + AuthMethod, BlobHash, Graph, IdpIdentity, PetriAdmission, PetriGraphRef, Principal, + RunProvenance, RunSpec, WorkflowSettings, WorkflowVersionId, fixtures, }; #[must_use] @@ -54,7 +54,27 @@ pub fn test_run_spec() -> RunSpec { spec_blob: None, git: None, fork_source_ref: None, - engine: RunEngine::Legacy, + admission: test_admission(), + } +} + +/// An admission whose graph blob names nothing a store holds: enough for a +/// spec that is never executed. It is also the `Default` a test fixture +/// takes for the field. +#[must_use] +pub fn test_admission() -> PetriAdmission { + PetriAdmission { + graph: PetriGraphRef { + blob: BlobHash::new(b"test-admission"), + digest: "sha256:test-admission".to_string(), + }, + children: Vec::new(), + } +} + +impl Default for PetriAdmission { + fn default() -> Self { + test_admission() } } diff --git a/lib/foundation/fabro-types/tests/run_event_serde.rs b/lib/foundation/fabro-types/tests/run_event_serde.rs index 2b013ddc7..eadb1b793 100644 --- a/lib/foundation/fabro-types/tests/run_event_serde.rs +++ b/lib/foundation/fabro-types/tests/run_event_serde.rs @@ -8,8 +8,8 @@ use fabro_types::settings::InterpString; use fabro_types::settings::run::RunGoal; use fabro_types::test_support::{test_run_provenance, test_workflow_version_id}; use fabro_types::{ - AutomationRef, EventBody, GitRunTarget, ResolvedAutomationGitWorkflowSource, RunTarget, TurnId, - WorkflowSettings, fixtures, + AutomationRef, EventBody, GitRunTarget, PetriAdmission, ResolvedAutomationGitWorkflowSource, + RunTarget, TurnId, WorkflowSettings, fixtures, }; fn templated_settings() -> WorkflowSettings { @@ -64,7 +64,7 @@ fn run_created_props_round_trip_templated_settings() { web_url: Some( "http://localhost:3000/runs/01JNQVR7M0EJ5GKAT2SC4ERS1Z".to_string(), ), - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }; let json = serde_json::to_value(&props).expect("props should serialize"); @@ -130,7 +130,7 @@ fn run_created_props_omits_web_url_when_absent() { retried_from: None, parent_id: None, web_url: None, - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }; let json = serde_json::to_value(&props).expect("props should serialize"); diff --git a/lib/foundation/fabro-types/tests/run_spec_serde.rs b/lib/foundation/fabro-types/tests/run_spec_serde.rs index 417e6b755..b2d0d6a4e 100644 --- a/lib/foundation/fabro-types/tests/run_spec_serde.rs +++ b/lib/foundation/fabro-types/tests/run_spec_serde.rs @@ -6,8 +6,8 @@ use fabro_types::settings::InterpString; use fabro_types::settings::run::RunGoal; use fabro_types::test_support::{test_run_provenance, test_workflow_version_id}; use fabro_types::{ - AutomationRef, GitRunTarget, ResolvedAutomationGitWorkflowSource, RunTarget, WorkflowSettings, - fixtures, + AutomationRef, GitRunTarget, PetriAdmission, ResolvedAutomationGitWorkflowSource, RunTarget, + WorkflowSettings, fixtures, }; fn templated_settings() -> WorkflowSettings { @@ -58,14 +58,14 @@ fn run_spec_round_trips_templated_settings() { source_run_id: fixtures::RUN_2, checkpoint_sha: "def456".to_string(), }), - engine: fabro_types::RunEngine::Legacy, + admission: PetriAdmission::default(), }; let json = serde_json::to_value(&record).expect("record should serialize"); assert!(json.get("working_directory").is_none()); - assert!( - json.get("engine").is_none(), - "a legacy run's spec omits the engine so older readers see the same shape" + assert_eq!( + json["admission"]["graph"]["digest"], "sha256:test-admission", + "the spec names what Petri admitted" ); assert!(json.get("host_repo_path").is_none()); assert_eq!(json["source_directory"], "/Users/client/project"); diff --git a/lib/packages/fabro-api-client/src/.openapi-generator/FILES b/lib/packages/fabro-api-client/src/.openapi-generator/FILES index 893cb3ea1..c135442a7 100644 --- a/lib/packages/fabro-api-client/src/.openapi-generator/FILES +++ b/lib/packages/fabro-api-client/src/.openapi-generator/FILES @@ -390,9 +390,6 @@ models/run-commit.ts models/run-commits-meta.ts models/run-control-action.ts models/run-diff.ts -models/run-engine-one-of.ts -models/run-engine-one-of1.ts -models/run-engine.ts models/run-environment-settings.ts models/run-error.ts models/run-event-detail-response-content.ts diff --git a/lib/packages/fabro-api-client/src/models/index.ts b/lib/packages/fabro-api-client/src/models/index.ts index 96c56b604..a8c0e3b4a 100644 --- a/lib/packages/fabro-api-client/src/models/index.ts +++ b/lib/packages/fabro-api-client/src/models/index.ts @@ -361,9 +361,6 @@ export * from './run-commit-person'; export * from './run-commits-meta'; export * from './run-control-action'; export * from './run-diff'; -export * from './run-engine'; -export * from './run-engine-one-of'; -export * from './run-engine-one-of1'; export * from './run-environment-settings'; export * from './run-error'; export * from './run-event'; diff --git a/lib/packages/fabro-api-client/src/models/run-engine-one-of.ts b/lib/packages/fabro-api-client/src/models/run-engine-one-of.ts deleted file mode 100644 index 86bf37439..000000000 --- a/lib/packages/fabro-api-client/src/models/run-engine-one-of.ts +++ /dev/null @@ -1,25 +0,0 @@ -/* tslint:disable */ -/* eslint-disable */ -/** - * Fabro Run API - * HTTP API for managing Fabro workflow run executions. - * - * The version of the OpenAPI document: 0.2.0 - * - * - * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). - * https://openapi-generator.tech - * Do not edit the class manually. - */ - - - -export interface RunEngineOneOf { - 'kind': RunEngineOneOfKindEnum; -} - -export const RunEngineOneOfKindEnum = { - LEGACY: 'legacy' -} as const; - -export type RunEngineOneOfKindEnum = typeof RunEngineOneOfKindEnum[keyof typeof RunEngineOneOfKindEnum]; diff --git a/lib/packages/fabro-api-client/src/models/run-engine-one-of1.ts b/lib/packages/fabro-api-client/src/models/run-engine-one-of1.ts deleted file mode 100644 index e4ab32ac8..000000000 --- a/lib/packages/fabro-api-client/src/models/run-engine-one-of1.ts +++ /dev/null @@ -1,26 +0,0 @@ -/* tslint:disable */ -/* eslint-disable */ -/** - * Fabro Run API - * HTTP API for managing Fabro workflow run executions. - * - * The version of the OpenAPI document: 0.2.0 - * - * - * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). - * https://openapi-generator.tech - * Do not edit the class manually. - */ - - -// May contain unused imports in some cases -// @ts-ignore -import type { PetriAdmission } from './petri-admission'; -// May contain unused imports in some cases -// @ts-ignore -import type { PetriGraphRef } from './petri-graph-ref'; - -/** - * @type RunEngineOneOf1 - */ -export type RunEngineOneOf1 = PetriAdmission; diff --git a/lib/packages/fabro-api-client/src/models/run-engine.ts b/lib/packages/fabro-api-client/src/models/run-engine.ts deleted file mode 100644 index c8f4929c8..000000000 --- a/lib/packages/fabro-api-client/src/models/run-engine.ts +++ /dev/null @@ -1,30 +0,0 @@ -/* tslint:disable */ -/* eslint-disable */ -/** - * Fabro Run API - * HTTP API for managing Fabro workflow run executions. - * - * The version of the OpenAPI document: 0.2.0 - * - * - * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). - * https://openapi-generator.tech - * Do not edit the class manually. - */ - - -// May contain unused imports in some cases -// @ts-ignore -import type { PetriGraphRef } from './petri-graph-ref'; -// May contain unused imports in some cases -// @ts-ignore -import type { RunEngineOneOf } from './run-engine-one-of'; -// May contain unused imports in some cases -// @ts-ignore -import type { RunEngineOneOf1 } from './run-engine-one-of1'; - -/** - * @type RunEngine - * The engine a run was created for. `legacy` is the in-process executor; `petri` names the Petri workflow engine and carries what Petri admitted at create time. - */ -export type RunEngine = RunEngineOneOf | RunEngineOneOf1; diff --git a/lib/packages/fabro-api-client/src/models/run-spec.ts b/lib/packages/fabro-api-client/src/models/run-spec.ts index b8e169a99..89b62d955 100644 --- a/lib/packages/fabro-api-client/src/models/run-spec.ts +++ b/lib/packages/fabro-api-client/src/models/run-spec.ts @@ -24,7 +24,7 @@ import type { ForkSourceRef } from './fork-source-ref'; import type { GitContext } from './git-context'; // May contain unused imports in some cases // @ts-ignore -import type { RunEngine } from './run-engine'; +import type { PetriAdmission } from './petri-admission'; // May contain unused imports in some cases // @ts-ignore import type { RunProvenance } from './run-provenance'; @@ -58,7 +58,7 @@ export interface RunSpec { 'git'?: GitContext | null; 'fork_source_ref'?: ForkSourceRef | null; /** - * The engine the run was created for, with what it admitted. Absent in a spec written before the field existed, which means the legacy executor. + * What Petri admitted for the run at create time: the graphs it executes and resumes from. */ - 'engine'?: RunEngine; + 'admission': PetriAdmission; }