mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-01 02:04:24 +00:00
Check a workflow version in memory with Runtime::check_source
`fabro_petri::check` materialized the version's bundle into a temporary directory because `Runtime::check` read the workflow and its settings files from disk. Petri now has `Runtime::check_source`, which takes the workflow's repository-relative path, its text and a `FileSource`, so the bundle goes into a `frontend::MapFiles` map instead: every file at its bundle-relative path, `workflow.toml` beside the workflow, and `.fabro/project.toml` at the root when the caller has one. Nothing is written to disk, and the diagnostics name the bundle-relative paths directly, with no root to strip. The compile inputs are unchanged: the intent's inputs, the launch model and provider, and `petri.repository` bound by Fabro itself (the launch's path, or `null`). An entrypoint that is not one of the bundle's files is now `CheckError::MissingEntrypoint`; `CheckError::Materialize` goes away. `tempfile` becomes a dev-dependency, as only the tests use it. New tests: a version whose `workflow.toml` names `engine = "petri"` is admitted (the new Petri pin knows the key), an unknown `[workflow]` key is refused with `unsupported.workflow_toml.key` and named in `workflow.toml`, the project settings are read from the map, and a missing entrypoint is an error. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
parent
f4fe505bac
commit
a745d0b75d
4 changed files with 161 additions and 106 deletions
|
|
@ -39,7 +39,6 @@ async-trait.workspace = true
|
|||
serde.workspace = true
|
||||
serde_json.workspace = true
|
||||
sqlx.workspace = true
|
||||
tempfile = "3"
|
||||
thiserror.workspace = true
|
||||
tokio.workspace = true
|
||||
tokio-util.workspace = true
|
||||
|
|
@ -50,4 +49,5 @@ fabro-auth = { path = "../../foundation/fabro-auth", features = ["test-support"]
|
|||
fabro-llm = { path = "../fabro-llm", features = ["test-support"] }
|
||||
fabro-store = { path = "../fabro-store", features = ["test-support"] }
|
||||
petri_testkit.workspace = true
|
||||
tempfile = "3"
|
||||
tokio = { workspace = true, features = ["macros", "rt-multi-thread"] }
|
||||
|
|
|
|||
|
|
@ -23,10 +23,12 @@ Every adapter the integration plan describes lands here.
|
|||
and at execution: the Fabro frontend with the server's settings layer, the
|
||||
Attractor step kinds (real, or simulated for a dry run), the model client
|
||||
as the `PebbleClient` capability, the Fabro home.
|
||||
- `check`: Petri compiles at create time. The workflow version's bundle is
|
||||
materialized into a temporary directory (`Runtime::check` reads files from
|
||||
disk), lowered with the run's inputs and launch, and the admitted graphs or
|
||||
Petri's diagnostics come back in a shape the server maps onto Fabro's.
|
||||
- `check`: Petri compiles at create time. The workflow version's bundle goes
|
||||
into an in-memory file map (`frontend::MapFiles`, laid out as the bundle:
|
||||
`workflow.toml` beside the workflow, `.fabro/project.toml` at the root),
|
||||
`Runtime::check_source` lowers it with the run's inputs and launch, and the
|
||||
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.
|
||||
- `engine`: a run executed by Petri, started from its admitted graphs or
|
||||
|
|
@ -71,8 +73,11 @@ Integration tests live under `tests/`:
|
|||
is not on `PATH` (every run takes its scope's environment through it);
|
||||
the sandbox-plugins CI job requires them.
|
||||
- `check.rs` admits the `hello` bundle and round-trips its graph through
|
||||
the blob store, binds the launch, and refuses an unknown attribute and,
|
||||
with a model client over the test catalog, an unknown model
|
||||
the blob store, binds the launch, admits a version whose `workflow.toml`
|
||||
names `engine = "petri"`, reads the project settings from the map, and
|
||||
refuses an unknown attribute, an unknown `[workflow]` key
|
||||
(`unsupported.workflow_toml.key`, named in `workflow.toml`) and, with a
|
||||
model client over the test catalog, an unknown model
|
||||
(`attractor.model.unknown`). No plugin is needed.
|
||||
- `sqlite_store.rs` runs Petri's store conformance suite
|
||||
(`petri_testkit::run_store::conformance`) against `SqliteRunStore`, plus the
|
||||
|
|
|
|||
|
|
@ -1,14 +1,14 @@
|
|||
//! Petri compiles: the create handler hands a workflow version's files, the
|
||||
//! run's inputs and the launch to `Runtime::check`, and gets back either the
|
||||
//! admitted graphs or Petri's diagnostics.
|
||||
//! run's inputs and the launch to `Runtime::check_source`, and gets back
|
||||
//! either the admitted graphs or Petri's diagnostics.
|
||||
//!
|
||||
//! `Runtime::check` reads the workflow and its settings files from disk, so
|
||||
//! the bundle is materialized into a temporary directory first, laid out the
|
||||
//! way the Fabro frontend expects: the workflow file with `workflow.toml`
|
||||
//! beside it under a bundle root that holds a `.fabro` directory (with
|
||||
//! `.fabro/project.toml` when the caller has one). The directory is removed
|
||||
//! when the check returns. An in-memory `FileSource` entry point on
|
||||
//! `Runtime` would remove the round trip; that is a Petri follow-up.
|
||||
//! The version's files never touch the disk. They go into a
|
||||
//! `frontend::MapFiles` map laid out the way the Fabro frontend expects a
|
||||
//! bundle: every file at its bundle-relative path, so `workflow.toml` sits
|
||||
//! beside the workflow file, and `.fabro/project.toml` at the root when the
|
||||
//! caller has one. The frontend reads the settings files and `@file`
|
||||
//! references from that map, and every diagnostic names the bundle-relative
|
||||
//! path the map holds the file under.
|
||||
//!
|
||||
//! The launch binds the compile variables the Fabro frontend reads:
|
||||
//! `petri.launch_model` and `petri.launch_provider` as the model default
|
||||
|
|
@ -17,12 +17,11 @@
|
|||
//! and the run starts from an empty workspace.
|
||||
|
||||
use std::collections::BTreeMap;
|
||||
use std::io;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::path::PathBuf;
|
||||
|
||||
use petri_runtime::LoadError;
|
||||
use petri_runtime::frontend::{
|
||||
self, CompileInputs, LAUNCH_MODEL_VAR, LAUNCH_PROVIDER_VAR, REPOSITORY_VAR, Severity,
|
||||
self, CompileInputs, LAUNCH_MODEL_VAR, LAUNCH_PROVIDER_VAR, MapFiles, REPOSITORY_VAR, Severity,
|
||||
};
|
||||
use petri_runtime::ir::Graph;
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
|
@ -30,13 +29,8 @@ use serde_json::Value;
|
|||
|
||||
use crate::runtime::RuntimeSpec;
|
||||
|
||||
/// The directory under the temporary bundle root the version's files land
|
||||
/// in. Its parent holds `.fabro`, so the Fabro frontend takes the parent as
|
||||
/// the bundle root.
|
||||
const BUNDLE_DIR: &str = "bundle";
|
||||
|
||||
/// The project settings file the Fabro frontend reads at the bundle root.
|
||||
const PROJECT_FILE: &str = ".fabro/project.toml";
|
||||
const PROJECT_FILE: &str = petri_frontend_fabro::PROJECT_FILE;
|
||||
|
||||
/// One workflow bundle to check: its files by bundle-relative path.
|
||||
#[derive(Clone, Debug, Default)]
|
||||
|
|
@ -48,9 +42,23 @@ pub struct Bundle {
|
|||
/// The workflow file to check, one of `files`.
|
||||
pub entrypoint: String,
|
||||
/// `.fabro/project.toml` at the bundle root, when the caller has one.
|
||||
/// It takes that path in the map, over a bundle file of the same name.
|
||||
pub project_toml: Option<String>,
|
||||
}
|
||||
|
||||
impl Bundle {
|
||||
/// The bundle as the Fabro frontend reads it: every file at its
|
||||
/// bundle-relative path, and the project settings at
|
||||
/// `.fabro/project.toml`.
|
||||
fn files(&self) -> MapFiles {
|
||||
let mut files = self.files.clone();
|
||||
if let Some(project) = &self.project_toml {
|
||||
files.insert(PROJECT_FILE.to_string(), project.clone());
|
||||
}
|
||||
MapFiles(files)
|
||||
}
|
||||
}
|
||||
|
||||
/// What the launch binds below the file layers.
|
||||
#[derive(Clone, Debug, Default)]
|
||||
pub struct Launch {
|
||||
|
|
@ -111,38 +119,33 @@ pub enum CheckError {
|
|||
/// included; at least one is an error.
|
||||
#[error("Petri refused the workflow with {} diagnostic(s)", .0.len())]
|
||||
Rejected(Vec<Diagnostic>),
|
||||
/// The bundle could not be materialized for the check.
|
||||
#[error("could not materialize the workflow bundle at `{path}`")]
|
||||
Materialize {
|
||||
path: PathBuf,
|
||||
#[source]
|
||||
source: io::Error,
|
||||
},
|
||||
/// The bundle's entrypoint is not one of its files, or no frontend
|
||||
/// claims it.
|
||||
/// The bundle's entrypoint is not one of its files.
|
||||
#[error("the workflow entrypoint `{entrypoint}` is not one of the bundle's files")]
|
||||
MissingEntrypoint { entrypoint: String },
|
||||
/// No frontend claims the bundle's entrypoint.
|
||||
#[error("the workflow could not be loaded")]
|
||||
Load(#[source] LoadError),
|
||||
}
|
||||
|
||||
/// Materialize the bundle, run `Runtime::check`, and hand back the admitted
|
||||
/// graphs or the diagnostics. Blocking: it reads and writes files and
|
||||
/// lowers the graph, so a server calls it from its blocking pool.
|
||||
/// Run `Runtime::check_source` over the bundle, and hand back the admitted
|
||||
/// graphs or the diagnostics. Blocking: it lowers the graph and runs the
|
||||
/// admission passes synchronously, so a server calls it from its blocking
|
||||
/// pool.
|
||||
pub fn check(request: &CheckRequest) -> Result<Admitted, CheckError> {
|
||||
let root = tempfile::tempdir().map_err(|source| CheckError::Materialize {
|
||||
path: std::env::temp_dir(),
|
||||
source,
|
||||
})?;
|
||||
let workflow = materialize(root.path(), &request.bundle)?;
|
||||
let bundle = &request.bundle;
|
||||
let text =
|
||||
bundle
|
||||
.files
|
||||
.get(&bundle.entrypoint)
|
||||
.ok_or_else(|| CheckError::MissingEntrypoint {
|
||||
entrypoint: bundle.entrypoint.clone(),
|
||||
})?;
|
||||
let runtime = request.runtime.runtime(false);
|
||||
let inputs = compile_inputs(&request.inputs, &request.launch);
|
||||
let lowered = runtime
|
||||
.check(&workflow, None, None, &inputs)
|
||||
.check_source(&bundle.entrypoint, text, &bundle.files(), None, &inputs)
|
||||
.map_err(CheckError::Load)?;
|
||||
let diagnostics: Vec<Diagnostic> = lowered
|
||||
.diagnostics
|
||||
.iter()
|
||||
.map(|diagnostic| convert(diagnostic, root.path()))
|
||||
.collect();
|
||||
let diagnostics: Vec<Diagnostic> = lowered.diagnostics.iter().map(convert).collect();
|
||||
match lowered.graph {
|
||||
Some(graph) => Ok(Admitted {
|
||||
graph,
|
||||
|
|
@ -153,39 +156,6 @@ pub fn check(request: &CheckRequest) -> Result<Admitted, CheckError> {
|
|||
}
|
||||
}
|
||||
|
||||
/// Write the bundle under `root/bundle/`, with `root/.fabro` beside it so
|
||||
/// the frontend takes `root` as the bundle root. Returns the entrypoint's
|
||||
/// path.
|
||||
#[expect(
|
||||
clippy::disallowed_methods,
|
||||
reason = "the check is a blocking function; its caller runs it on the blocking pool"
|
||||
)]
|
||||
fn materialize(root: &Path, bundle: &Bundle) -> Result<PathBuf, CheckError> {
|
||||
let write = |relative: &str, text: &str| -> Result<(), CheckError> {
|
||||
let path = root.join(relative);
|
||||
let materialize = |source| CheckError::Materialize {
|
||||
path: path.clone(),
|
||||
source,
|
||||
};
|
||||
if let Some(parent) = path.parent() {
|
||||
std::fs::create_dir_all(parent).map_err(materialize)?;
|
||||
}
|
||||
std::fs::write(&path, text).map_err(materialize)
|
||||
};
|
||||
let fabro_dir = root.join(".fabro");
|
||||
std::fs::create_dir_all(&fabro_dir).map_err(|source| CheckError::Materialize {
|
||||
path: fabro_dir,
|
||||
source,
|
||||
})?;
|
||||
if let Some(project) = &bundle.project_toml {
|
||||
write(PROJECT_FILE, project)?;
|
||||
}
|
||||
for (relative, text) in &bundle.files {
|
||||
write(&format!("{BUNDLE_DIR}/{relative}"), text)?;
|
||||
}
|
||||
Ok(root.join(BUNDLE_DIR).join(&bundle.entrypoint))
|
||||
}
|
||||
|
||||
/// The compile inputs: the intent's inputs, and the launch variables.
|
||||
fn compile_inputs(inputs: &BTreeMap<String, Value>, launch: &Launch) -> CompileInputs {
|
||||
let mut compile = CompileInputs::new();
|
||||
|
|
@ -202,8 +172,9 @@ fn compile_inputs(inputs: &BTreeMap<String, Value>, launch: &Launch) -> CompileI
|
|||
compile
|
||||
.vars
|
||||
.insert(LAUNCH_PROVIDER_VAR.into(), text(&launch.provider));
|
||||
// Bound even when absent: `Runtime::lower` would otherwise bind the
|
||||
// temporary bundle root, which is gone by the time the run starts.
|
||||
// `Runtime::check_source` uses the inputs as given, so the repository
|
||||
// is the host's to bind: the launch's path, or `null` for a run that
|
||||
// starts from an empty workspace.
|
||||
let repository = launch.repository.as_ref().map_or(Value::Null, |path| {
|
||||
Value::String(path.to_string_lossy().into_owned())
|
||||
});
|
||||
|
|
@ -211,30 +182,20 @@ fn compile_inputs(inputs: &BTreeMap<String, Value>, launch: &Launch) -> CompileI
|
|||
compile
|
||||
}
|
||||
|
||||
/// Petri's diagnostic in Fabro's shape, with the file made relative to the
|
||||
/// bundle.
|
||||
fn convert(diagnostic: &frontend::Diagnostic, root: &Path) -> Diagnostic {
|
||||
let file = diagnostic.span.file.as_str();
|
||||
let prefix = format!("{BUNDLE_DIR}/");
|
||||
let file = Path::new(file)
|
||||
.strip_prefix(root)
|
||||
.map_or(file, |relative| relative.to_str().unwrap_or(file))
|
||||
.to_string();
|
||||
let file = file
|
||||
.strip_prefix(&prefix)
|
||||
.map_or(file.as_str(), |relative| relative)
|
||||
.to_string();
|
||||
/// Petri's diagnostic in Fabro's shape. The file is the path the map holds
|
||||
/// it under, which is bundle-relative already.
|
||||
fn convert(diagnostic: &frontend::Diagnostic) -> Diagnostic {
|
||||
Diagnostic {
|
||||
severity: match diagnostic.severity {
|
||||
Severity::Error => DiagnosticSeverity::Error,
|
||||
Severity::Warning => DiagnosticSeverity::Warning,
|
||||
},
|
||||
code: diagnostic.code.to_string(),
|
||||
message: diagnostic.message.clone(),
|
||||
hint: diagnostic.hint.clone(),
|
||||
file,
|
||||
line: (diagnostic.span.line > 0).then_some(diagnostic.span.line),
|
||||
column: (diagnostic.span.column > 0).then_some(diagnostic.span.column),
|
||||
code: diagnostic.code.to_string(),
|
||||
message: diagnostic.message.clone(),
|
||||
hint: diagnostic.hint.clone(),
|
||||
file: diagnostic.span.file.to_string(),
|
||||
line: (diagnostic.span.line > 0).then_some(diagnostic.span.line),
|
||||
column: (diagnostic.span.column > 0).then_some(diagnostic.span.column),
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -1,7 +1,8 @@
|
|||
//! Petri compiles at create time: `fabro_petri::check` materializes a bundle,
|
||||
//! hands it to `Runtime::check`, and returns the admitted graphs or Petri's
|
||||
//! diagnostics in Fabro's shape; `fabro_petri::admission` round-trips the
|
||||
//! admitted graphs through Fabro's blob store.
|
||||
//! Petri compiles at create time: `fabro_petri::check` hands a bundle to
|
||||
//! `Runtime::check_source` as an in-memory file map, and returns the
|
||||
//! admitted graphs or Petri's diagnostics in Fabro's shape;
|
||||
//! `fabro_petri::admission` round-trips the admitted graphs through Fabro's
|
||||
//! blob store.
|
||||
//!
|
||||
//! No sandbox plugin is needed: nothing here runs a graph.
|
||||
|
||||
|
|
@ -152,6 +153,94 @@ async fn a_launch_binds_the_repository_and_the_model_default() {
|
|||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn a_version_that_names_the_petri_engine_is_admitted() {
|
||||
let settings = format!("{SETTINGS}engine = \"petri\"\n");
|
||||
let request = request(
|
||||
bundle(&[
|
||||
("workflow.fabro", COMMAND_WORKFLOW),
|
||||
("workflow.toml", &settings),
|
||||
]),
|
||||
RuntimeSpec::default(),
|
||||
);
|
||||
|
||||
let admitted = check::check(&request).expect("`engine = \"petri\"` is a known key");
|
||||
|
||||
assert!(
|
||||
admitted
|
||||
.warnings
|
||||
.iter()
|
||||
.all(|w| w.code != "unsupported.workflow_toml.key"),
|
||||
"{:?}",
|
||||
admitted.warnings
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn an_unknown_workflow_key_is_refused_and_named_in_workflow_toml() {
|
||||
let settings = format!("{SETTINGS}bogus = \"1\"\n");
|
||||
let request = request(
|
||||
bundle(&[
|
||||
("workflow.fabro", COMMAND_WORKFLOW),
|
||||
("workflow.toml", &settings),
|
||||
]),
|
||||
RuntimeSpec::default(),
|
||||
);
|
||||
|
||||
let Err(CheckError::Rejected(diagnostics)) = check::check(&request) else {
|
||||
panic!("an unknown `[workflow]` key should be refused");
|
||||
};
|
||||
|
||||
let error = diagnostics
|
||||
.iter()
|
||||
.find(|d| d.code == "unsupported.workflow_toml.key")
|
||||
.unwrap_or_else(|| panic!("no unknown-key diagnostic in {diagnostics:?}"));
|
||||
assert!(error.is_error());
|
||||
assert!(error.message.contains("workflow.bogus"), "{error:?}");
|
||||
assert_eq!(error.file, "workflow.toml");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn the_project_settings_are_read_from_the_map() {
|
||||
let workflow = UNKNOWN_MODEL_WORKFLOW.replace(", model=\"no-such-model-9000\"", "");
|
||||
let request = request(
|
||||
Bundle {
|
||||
project_toml: Some("[run.model]\nname = \"gpt-5.4\"\n".to_string()),
|
||||
..bundle(&[("workflow.fabro", &workflow), ("workflow.toml", SETTINGS)])
|
||||
},
|
||||
runtime_with_openai(),
|
||||
);
|
||||
|
||||
let admitted = check::check(&request).expect("the project model is admitted");
|
||||
|
||||
let work = admitted
|
||||
.graph
|
||||
.body
|
||||
.nodes
|
||||
.iter()
|
||||
.find(|node| node.name == "work")
|
||||
.expect("the work node is in the graph");
|
||||
assert_eq!(work.step.config["provider"], "openai");
|
||||
assert_eq!(work.step.config["model"], "gpt-5.4");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn a_missing_entrypoint_is_an_error() {
|
||||
let request = request(
|
||||
Bundle {
|
||||
entrypoint: "missing.fabro".to_string(),
|
||||
..bundle(&[("workflow.fabro", COMMAND_WORKFLOW)])
|
||||
},
|
||||
RuntimeSpec::default(),
|
||||
);
|
||||
|
||||
let Err(CheckError::MissingEntrypoint { entrypoint }) = check::check(&request) else {
|
||||
panic!("an entrypoint outside the bundle should be an error");
|
||||
};
|
||||
|
||||
assert_eq!(entrypoint, "missing.fabro");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn an_unknown_attribute_is_refused_with_petris_code() {
|
||||
let request = request(
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue