From a36bea15d29f6e8a6869d60a494319243a467ea0 Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Fri, 18 Sep 2026 09:58:21 -0400 Subject: [PATCH] Remove the engine flag: every run is a Petri run Delete `Engine`, `RunEngine`, `[workflow] engine`, `[server.execution] engine`, `FABRO_SERVER_ENGINE` and `fabro server start --engine`. The run spec records what Petri admitted as `admission: PetriAdmission`; the create handler always admits through `Runtime::check`; `execute_run` always launches the Petri worker (or executes in process under the test override); the CLI runner takes only the Petri worker path, and its legacy control arm, artifact uploader, signal pause handlers and credential helpers go with it. The CLI's `attach` and `events` read the run stream only. Two gaps this surfaced are closed here: the check adapter binds the server's run variables as Petri compile variables (`{{ vars.* }}` in a prompt no longer fails admission), and deleting a run removes its Petri records, lease, platform records, projection and stream. Tests: the config engine tests are replaced (an engine key is unknown), the API round-trip test covers `PetriAdmission`, the server and CLI Petri scenarios drop their engine settings, and the API tests that read legacy event names now read the run stream or the session events. The remaining red tests are fixtures and scenarios of the legacy executor and the legacy event store (`fabro-store` `slate` and `run_state`, `fabro-types` legacy `run.created` JSON, the server's handler-registry scenarios, the CLI dry-run snapshots), which the next steps of the F4.3 series delete or port. Co-Authored-By: Claude Fable 5.1 --- apps/fabro-web/app/lib/petri-stream.test.ts | 2 +- apps/fabro-web/app/lib/petri-stream.ts | 8 +- docs/public/api-reference/fabro-api.yaml | 31 +- lib/apps/fabro-cli/src/commands/run/attach.rs | 285 +--- lib/apps/fabro-cli/src/commands/run/events.rs | 1351 +---------------- .../src/commands/run/petri_worker.rs | 50 +- .../src/commands/run/run_progress/event.rs | 1 + .../src/commands/run/run_progress/mod.rs | 5 +- lib/apps/fabro-cli/src/commands/run/runner.rs | 579 +------ .../fabro-cli/src/commands/server/start.rs | 25 - lib/apps/fabro-cli/src/server_client.rs | 2 +- lib/apps/fabro-cli/src/shared/github.rs | 49 - lib/apps/fabro-cli/src/shared/mod.rs | 1 - .../fabro-cli/tests/it/cmd/server_start.rs | 2 - lib/apps/fabro-cli/tests/it/scenario/petri.rs | 16 +- lib/apps/fabro-cli/tests/it/support/mod.rs | 4 +- lib/apps/fabro-server/src/demo/mod.rs | 7 +- lib/apps/fabro-server/src/run_compiler.rs | 167 +- lib/apps/fabro-server/src/run_files.rs | 4 +- lib/apps/fabro-server/src/serve.rs | 31 +- lib/apps/fabro-server/src/server.rs | 411 +---- .../fabro-server/src/server/handler/events.rs | 55 +- .../fabro-server/src/server/handler/pair.rs | 6 +- .../fabro-server/src/server/handler/runs.rs | 34 +- .../src/server/handler/sessions.rs | 4 +- .../fabro-server/src/server/petri_runs.rs | 27 +- lib/apps/fabro-server/src/server/tests.rs | 24 +- .../fabro-server/tests/it/api/run_files.rs | 6 +- lib/apps/fabro-server/tests/it/api/runs.rs | 15 +- .../fabro-server/tests/it/api/sessions.rs | 13 +- lib/apps/fabro-server/tests/it/api/system.rs | 2 +- lib/apps/fabro-server/tests/it/api/tcp.rs | 1 - .../fabro-server/tests/it/api/variables.rs | 21 +- .../fabro-server/tests/it/scenario/petri.rs | 93 +- .../tests/it/scenario/petri_stream.rs | 5 +- lib/components/fabro-petri/README.md | 22 +- lib/components/fabro-petri/src/check.rs | 24 +- lib/components/fabro-petri/src/projector.rs | 35 + lib/components/fabro-petri/tests/check.rs | 2 + lib/components/fabro-petri/tests/hooks.rs | 1 + .../fabro-petri/tests/projection.rs | 8 +- .../fabro-petri/tests/support/mod.rs | 1 + .../fabro-store/src/platform_records.rs | 2 +- lib/components/fabro-store/src/run_state.rs | 2 +- .../fabro-store/src/run_summary_store.rs | 109 +- lib/components/fabro-store/src/slate/mod.rs | 12 +- .../fabro-workflow/src/event/convert.rs | 6 +- .../fabro-workflow/src/event/events.rs | 7 +- .../fabro-workflow/src/event/sink.rs | 4 +- lib/components/fabro-workflow/src/git.rs | 6 +- .../fabro-workflow/src/handler/agent.rs | 4 +- .../fabro-workflow/src/handler/command.rs | 8 +- .../fabro-workflow/src/handler/parallel.rs | 4 +- .../fabro-workflow/src/handler/prompt.rs | 4 +- .../fabro-workflow/src/operations/archive.rs | 4 +- .../fabro-workflow/src/operations/create.rs | 39 +- .../fabro-workflow/src/operations/fork.rs | 6 +- .../fabro-workflow/src/operations/retry.rs | 18 +- .../fabro-workflow/src/operations/start.rs | 6 +- .../fabro-workflow/src/operations/timeline.rs | 6 +- .../src/pipeline/execute/tests.rs | 7 +- .../fabro-workflow/src/pipeline/finalize.rs | 8 +- .../fabro-workflow/src/pipeline/initialize.rs | 7 +- .../fabro-workflow/src/pipeline/persist.rs | 6 +- .../src/pipeline/pull_request.rs | 22 +- .../fabro-workflow/src/run_lookup.rs | 4 +- .../fabro-workflow/src/runtime_store.rs | 4 +- .../fabro-workflow/src/stage_execution.rs | 6 +- .../fabro-workflow/src/test_support.rs | 4 +- lib/foundation/fabro-api/build.rs | 1 - lib/foundation/fabro-api/src/lib.rs | 14 +- ..._trip.rs => petri_admission_round_trip.rs} | 28 +- lib/foundation/fabro-config/src/defaults.toml | 3 - .../fabro-config/src/layers/combine.rs | 3 +- lib/foundation/fabro-config/src/layers/mod.rs | 8 +- .../fabro-config/src/layers/server.rs | 14 +- .../fabro-config/src/layers/workflow.rs | 5 - lib/foundation/fabro-config/src/lib.rs | 8 +- .../fabro-config/src/resolve/server.rs | 16 +- .../fabro-config/src/resolve/workflow.rs | 1 - .../fabro-config/src/tests/resolve_server.rs | 15 - .../src/tests/resolve_workflow.rs | 38 +- lib/foundation/fabro-static/src/env_vars.rs | 2 - lib/foundation/fabro-types/src/engine.rs | 114 +- lib/foundation/fabro-types/src/lib.rs | 2 +- lib/foundation/fabro-types/src/run.rs | 10 +- .../fabro-types/src/run_event/run.rs | 8 +- .../fabro-types/src/settings/mod.rs | 6 +- .../fabro-types/src/settings/server.rs | 17 +- .../fabro-types/src/settings/workflow.rs | 9 +- .../fabro-types/src/test_support.rs | 26 +- .../fabro-types/tests/run_event_serde.rs | 8 +- .../fabro-types/tests/run_spec_serde.rs | 12 +- .../src/.openapi-generator/FILES | 3 - .../fabro-api-client/src/models/index.ts | 3 - .../src/models/run-engine-one-of.ts | 25 - .../src/models/run-engine-one-of1.ts | 26 - .../fabro-api-client/src/models/run-engine.ts | 30 - .../fabro-api-client/src/models/run-spec.ts | 6 +- 99 files changed, 579 insertions(+), 3627 deletions(-) delete mode 100644 lib/apps/fabro-cli/src/shared/github.rs rename lib/foundation/fabro-api/tests/{run_engine_round_trip.rs => petri_admission_round_trip.rs} (50%) delete mode 100644 lib/packages/fabro-api-client/src/models/run-engine-one-of.ts delete mode 100644 lib/packages/fabro-api-client/src/models/run-engine-one-of1.ts delete mode 100644 lib/packages/fabro-api-client/src/models/run-engine.ts 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; }