mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-10 03:30:59 +00:00
Merge remote-tracking branch 'origin/main' into refactor/remove-env-interpolation
# Conflicts: # lib/foundation/fabro-types/src/settings/interp.rs
This commit is contained in:
Bryan Helmkamp
commit
00228383dd
15 changed files with 1342 additions and 123 deletions
1
Cargo.lock
generated
1
Cargo.lock
generated
|
|
@ -3028,6 +3028,7 @@ dependencies = [
|
|||
"fabro-spa",
|
||||
"fabro-static",
|
||||
"fabro-store",
|
||||
"fabro-template",
|
||||
"fabro-test",
|
||||
"fabro-tool",
|
||||
"fabro-types",
|
||||
|
|
|
|||
|
|
@ -104,7 +104,7 @@ test [label="Run Tests", shape=parallelogram, script="cargo test 2>&1 || true"]
|
|||
|
||||
| Attribute | Description |
|
||||
|---|---|
|
||||
| `script` | The shell command to execute |
|
||||
| `script` | The shell command to execute. Substitutes `{{ goal }}`, `{{ inputs.NAME }}`, and `{{ vars.NAME }}` — see [command node scripts](/workflows/variables#command-node-scripts) |
|
||||
| `language` | `"shell"` (default) or `"python"` |
|
||||
|
||||
### Human
|
||||
|
|
|
|||
|
|
@ -3,7 +3,7 @@ title: "Variables"
|
|||
description: "Using templates in workflows"
|
||||
---
|
||||
|
||||
Fabro renders `{{ ... }}` templates in exactly two workflow attributes: the graph `goal` and node `prompt`s. Every other attribute is literal text.
|
||||
Fabro renders `{{ ... }}` templates in exactly two workflow attributes: the graph `goal` and node `prompt`s. A command node's `script` gets narrower treatment — [simple value substitution](#command-node-scripts), not templating. Every other attribute is literal text.
|
||||
|
||||
## Template context
|
||||
|
||||
|
|
@ -51,7 +51,7 @@ digraph Check {
|
|||
}
|
||||
```
|
||||
|
||||
Other attributes — `script`, `label`, `model`, `provider`, `condition`, and all edge attributes — do not render templates. If one of them contains `{{ … }}` or `{% … %}`, the syntax is treated as literal text and Fabro records a `detemplated_attribute` warning suggesting you move the dynamic value into a `prompt` or `goal`.
|
||||
Other attributes — `label`, `model`, `provider`, `condition`, and all edge attributes — do not render templates. If one of them contains `{{ … }}` or `{% … %}`, the syntax is treated as literal text and Fabro records a `detemplated_attribute` warning suggesting you move the dynamic value into a `prompt` or `goal`.
|
||||
|
||||
Override individual inputs at run time with repeatable `-I` / `--input` flags:
|
||||
|
||||
|
|
@ -61,6 +61,62 @@ fabro run .fabro/workflows/check/workflow.toml -I repo_name=fabro-2 --input lang
|
|||
|
||||
CLI input values use TOML scalar parsing when possible. Quoted strings, booleans, integers, and floats keep their typed values; unquoted bare text falls back to a string. Empty values such as `foo=` are accepted as empty strings. Arrays, inline tables, and datetimes are rejected.
|
||||
|
||||
## Command node scripts
|
||||
|
||||
A [command node](/workflows/stages-and-nodes#command) `script` substitutes `{{ goal }}`, `{{ inputs.NAME }}`, and `{{ vars.NAME }}`:
|
||||
|
||||
```dot title="check.fabro"
|
||||
digraph Check {
|
||||
test [shape=parallelogram, script="cargo test -p {{ inputs.crate }} --profile {{ vars.PROFILE }}"]
|
||||
pr [shape=parallelogram, script="gh pr create --title {{ goal }}"]
|
||||
}
|
||||
```
|
||||
|
||||
This is value substitution, not templating. Only those three forms are recognized; every other brace sequence reaches the shell untouched, so `jq` filters, `awk` programs, Go templates, and brace expansion all keep working:
|
||||
|
||||
```dot
|
||||
report [shape=parallelogram, script="kubectl get pod -o go-template='{{ .status.phase }}' | jq '{phase: .}'"]
|
||||
```
|
||||
|
||||
There is no `{% if %}`, no filters, and no loops, and `{{ goal }}` has no dotted form — `{{ goal.title }}` stays literal. Put branching in a [conditional node](/workflows/stages-and-nodes#conditional) or in the shell itself.
|
||||
|
||||
In a shell script, Fabro quotes each substituted value as one shell argument. Put the token where one shell word is valid. Do not add quotes around the token, and do not use a token to inject multiple flags or shell syntax:
|
||||
|
||||
```dot
|
||||
build [shape=parallelogram, script="docker build -t {{ inputs.image }} ."]
|
||||
```
|
||||
|
||||
For `language="python"`, Fabro inserts each value as a quoted string literal. Put the token where a Python expression is valid:
|
||||
|
||||
```dot
|
||||
report [shape=parallelogram, language="python", script="print({{ goal }})"]
|
||||
```
|
||||
|
||||
Substituted text is never scanned again, so a goal or input containing `{{ ... }}` reaches the command as literal characters rather than being interpolated a second time.
|
||||
|
||||
### `env` and `secrets` are not substituted
|
||||
|
||||
`{{ env.NAME }}` and `{{ secrets.NAME }}` are rejected in a `script` with a validation error. Read environment variables with `$NAME` in a shell script:
|
||||
|
||||
```dot
|
||||
deploy [shape=parallelogram, script="deploy --token $DEPLOY_TOKEN"]
|
||||
```
|
||||
|
||||
In a Python script, read them with `os.environ["NAME"]`.
|
||||
|
||||
To make a secret available that way, put it in the [environment's env map](/execution/run-configuration#run-environment-and-environments-slug), where `{{ secrets.* }}` does resolve — at run start, into the sandbox environment rather than into the script text:
|
||||
|
||||
```toml title="run.toml"
|
||||
[environments.ci.env]
|
||||
DEPLOY_TOKEN = "{{ secrets.DEPLOY_TOKEN }}"
|
||||
```
|
||||
|
||||
This keeps secret values out of the `command.started` event, which records the script verbatim.
|
||||
|
||||
### When values resolve
|
||||
|
||||
Inputs and variables are substituted when the run is created, at the same time as goals and prompts — the persisted workflow already contains the final script. An unbound input or variable is a warning from `fabro validate` and an error at run creation, so a run never executes a partially substituted command.
|
||||
|
||||
## Server-managed run config variables
|
||||
|
||||
Use server-managed variables for non-sensitive values that should be shared across runs, such as deployment environments, default branches, regions, or image tags:
|
||||
|
|
@ -115,8 +171,9 @@ Fabro keeps workflow structure static and renders workflow templates once:
|
|||
2. Literal `import`, `@file`, graph-goal file, and child-workflow references are resolved.
|
||||
3. The graph `goal` is rendered with the `{ inputs, vars }` context.
|
||||
4. Node `prompt` attributes are rendered with the `{ goal, inputs, vars }` context.
|
||||
5. Node `script` attributes have their `{{ goal }}`, `{{ inputs.* }}`, and `{{ vars.* }}` values substituted.
|
||||
|
||||
Templates are not supported in graph syntax, node IDs, edge structure, `import` paths, `@file` paths, child workflow paths, other file references, or any attribute besides `prompt` and `goal`.
|
||||
Templates are not supported in graph syntax, node IDs, edge structure, `import` paths, `@file` paths, child workflow paths, other file references, or any attribute besides `prompt` and `goal` — and `script`, which takes value substitution rather than templates.
|
||||
|
||||
Fabro renders the graph `goal` first and stores the rendered value back onto the graph. Prompts that use `{{ goal }}` receive that rendered value.
|
||||
|
||||
|
|
@ -124,6 +181,8 @@ Fabro renders the graph `goal` first and stores the rendered value back onto the
|
|||
|
||||
Fabro renders undefined workflow variables as empty text and records a `template_undefined_variable` diagnostic. `fabro validate` reports that diagnostic as a warning so you can validate workflow structure before all inputs are known. Offline validation does not read a server's variable store, so `{{ vars.* }}` references also warn there. Run-style commands such as `fabro run`, `fabro create`, and preflight use the server snapshot and promote any still-undefined reference to an error before proceeding.
|
||||
|
||||
In a `script`, an undefined value records the same diagnostic but leaves the token in place rather than emptying it, so validation output shows what is unbound.
|
||||
|
||||
## Template includes
|
||||
|
||||
Prompt and goal templates support static MiniJinja loader dependencies such as `{% include "partial.md" %}`. Includes are resolved relative to the template file being rendered and can be nested.
|
||||
|
|
|
|||
|
|
@ -41,6 +41,7 @@ fabro-manifest = { path = "../../components/fabro-manifest" }
|
|||
fabro-mcp-store = { path = "../../components/fabro-mcp-store" }
|
||||
fabro-model = { path = "../../foundation/fabro-model" }
|
||||
fabro-proc = { path = "../../foundation/fabro-proc" }
|
||||
fabro-template = { path = "../../foundation/fabro-template" }
|
||||
fabro-tool = { path = "../../components/fabro-tool" }
|
||||
fabro-types = { path = "../../foundation/fabro-types" }
|
||||
fabro-util = { path = "../../foundation/fabro-util" }
|
||||
|
|
|
|||
45
lib/apps/fabro-server/src/prompts/run_title.md.j2
Normal file
45
lib/apps/fabro-server/src/prompts/run_title.md.j2
Normal file
|
|
@ -0,0 +1,45 @@
|
|||
Generate a concise, human-readable title for this Fabro workflow run.
|
||||
|
||||
Base the title on the workflow identity, workflow goal, and run input values.
|
||||
Write it the way a person would title a pull request:
|
||||
|
||||
- Start with the verb for the work: Implement, Fix, Migrate, Review, Add. When
|
||||
the goal opens with Markdown heading marks or a document-type prefix such as
|
||||
"Plan:", "Goal:", or "Spec:", that prefix names the document rather than the
|
||||
work — drop it and start from the verb that follows.
|
||||
- If the run names an identifier — work order, ticket, issue, PR number — put it
|
||||
next in canonical uppercase, then a colon, then a short description:
|
||||
"Implement WRK-044: Separate routine and nightly verification".
|
||||
- Take that description from the most specific human-readable text available.
|
||||
Drop directory paths, date prefixes, and file extensions; turn slug hyphens
|
||||
and underscores into spaces; write the result in sentence case.
|
||||
- Keep a repository, branch, or environment only when it is what distinguishes
|
||||
this run from an otherwise identical one.
|
||||
- Do not end with a period.
|
||||
|
||||
Examples:
|
||||
|
||||
- goal "Implement Delivery Work Order docs/planning/orders/2026-03-04-wrk-018-retry-backoff.md"
|
||||
-> "Implement WRK-018: Retry backoff"
|
||||
- goal "## Plan: migrate the billing service to Postgres 17"
|
||||
-> "Migrate billing service to Postgres 17"
|
||||
- goal "Fix flaky checkout test" with input branch "release-9.2"
|
||||
-> "Fix flaky checkout test on release-9.2"
|
||||
|
||||
Return only structured JSON with one field: {"title":"..."}.
|
||||
The title must be a single line, not blank, and no more than {{ inputs.max_chars }} characters.
|
||||
|
||||
Workflow identity:
|
||||
```json
|
||||
{{ inputs.workflow_identity }}
|
||||
```
|
||||
|
||||
Run inputs (raw values, not redacted):
|
||||
```json
|
||||
{{ inputs.run_inputs }}
|
||||
```
|
||||
|
||||
Workflow summary:
|
||||
```json
|
||||
{{ inputs.workflow_summary }}
|
||||
```
|
||||
|
|
@ -5,13 +5,18 @@ use fabro_llm::client::Client;
|
|||
use fabro_llm::generate::{self, GenerateParams};
|
||||
use fabro_llm::types::TimeoutOptions;
|
||||
use fabro_model::ProviderId;
|
||||
use fabro_template::{TemplateContext, TemplateError};
|
||||
use fabro_types::{Graph, MAX_RUN_TITLE_CHARS, RunId};
|
||||
use fabro_util::error;
|
||||
use serde::Serialize;
|
||||
use toml::Value as TomlValue;
|
||||
|
||||
const TRUNCATED_MARKER: &str = "...[truncated]";
|
||||
const MAX_PROMPT_SECTION_CHARS: usize = 4_000;
|
||||
|
||||
const TITLE_PROMPT_NAME: &str = "run_title.md.j2";
|
||||
const TITLE_PROMPT_TEMPLATE: &str = include_str!("prompts/run_title.md.j2");
|
||||
|
||||
pub(crate) struct TitlePromptInput<'a> {
|
||||
pub(crate) run_id: &'a RunId,
|
||||
pub(crate) current_title: &'a str,
|
||||
|
|
@ -29,7 +34,16 @@ pub(crate) struct GenerateTitleInput<'a> {
|
|||
|
||||
pub(crate) async fn generate_title_or_current(input: GenerateTitleInput<'_>) -> String {
|
||||
let current_title = input.prompt.current_title.to_string();
|
||||
let prompt = build_title_prompt(&input.prompt);
|
||||
let prompt = match build_title_prompt(&input.prompt) {
|
||||
Ok(prompt) => prompt,
|
||||
Err(err) => {
|
||||
// A checked-in template that will not render is a bug, not a
|
||||
// transient failure, so this is louder than a generation miss.
|
||||
let rendered_error = error::collect_chain(&err).join(": ");
|
||||
tracing::warn!(error = %rendered_error, "Run title prompt template failed to render");
|
||||
return current_title;
|
||||
}
|
||||
};
|
||||
let params = GenerateParams::new(input.model_id, input.client)
|
||||
.provider(input.provider_id.to_string())
|
||||
.prompt(prompt)
|
||||
|
|
@ -56,7 +70,7 @@ pub(crate) async fn generate_title_or_current(input: GenerateTitleInput<'_>) ->
|
|||
.unwrap_or(current_title)
|
||||
}
|
||||
|
||||
fn build_title_prompt(input: &TitlePromptInput<'_>) -> String {
|
||||
fn build_title_prompt(input: &TitlePromptInput<'_>) -> Result<String, TemplateError> {
|
||||
let workflow_identity = serde_json::json!({
|
||||
"run_id": input.run_id.to_string(),
|
||||
"current_deterministic_title": input.current_title,
|
||||
|
|
@ -68,33 +82,30 @@ fn build_title_prompt(input: &TitlePromptInput<'_>) -> String {
|
|||
let inputs = pretty_json(input.run_inputs);
|
||||
let workflow = pretty_json(input.workflow);
|
||||
|
||||
format!(
|
||||
r#"Generate a concise, human-readable title for this Fabro workflow run.
|
||||
|
||||
Base the title on the workflow identity, workflow goal, and run input values.
|
||||
Preserve meaningful proper nouns, ticket IDs, repositories, branches, environments, and explicit user goals.
|
||||
Return only structured JSON with one field: {{"title":"..."}}.
|
||||
The title must be a single line, not blank, and no more than {MAX_RUN_TITLE_CHARS} characters.
|
||||
|
||||
Workflow identity:
|
||||
```json
|
||||
{}
|
||||
```
|
||||
|
||||
Run inputs (raw values, not redacted):
|
||||
```json
|
||||
{}
|
||||
```
|
||||
|
||||
Workflow summary:
|
||||
```json
|
||||
{}
|
||||
```
|
||||
"#,
|
||||
truncate_section(&identity, MAX_PROMPT_SECTION_CHARS),
|
||||
truncate_section(&inputs, MAX_PROMPT_SECTION_CHARS),
|
||||
truncate_section(&workflow, MAX_PROMPT_SECTION_CHARS),
|
||||
)
|
||||
let template_inputs = HashMap::from([
|
||||
(
|
||||
"max_chars".to_string(),
|
||||
TomlValue::Integer(
|
||||
MAX_RUN_TITLE_CHARS
|
||||
.try_into()
|
||||
.expect("run title character limit should fit in i64"),
|
||||
),
|
||||
),
|
||||
(
|
||||
"workflow_identity".to_string(),
|
||||
TomlValue::String(truncate_section(&identity, MAX_PROMPT_SECTION_CHARS)),
|
||||
),
|
||||
(
|
||||
"run_inputs".to_string(),
|
||||
TomlValue::String(truncate_section(&inputs, MAX_PROMPT_SECTION_CHARS)),
|
||||
),
|
||||
(
|
||||
"workflow_summary".to_string(),
|
||||
TomlValue::String(truncate_section(&workflow, MAX_PROMPT_SECTION_CHARS)),
|
||||
),
|
||||
]);
|
||||
let ctx = TemplateContext::new().with_inputs(template_inputs);
|
||||
fabro_template::render_named(TITLE_PROMPT_NAME, TITLE_PROMPT_TEMPLATE, &ctx)
|
||||
}
|
||||
|
||||
fn normalize_generated_title(title: &str) -> Option<String> {
|
||||
|
|
@ -203,6 +214,29 @@ mod tests {
|
|||
.unwrap()
|
||||
}
|
||||
|
||||
/// Strict rendering already fails on a variable the template asks for and
|
||||
/// the caller does not supply. This covers the other direction: a variable
|
||||
/// dropped from the template renders fine but silently starves the model.
|
||||
#[test]
|
||||
fn prompt_template_renders_every_variable_the_caller_supplies() {
|
||||
let run_id = RunId::new();
|
||||
let graph = title_test_graph();
|
||||
let summary = workflow_summary(&graph);
|
||||
let prompt = build_title_prompt(&TitlePromptInput {
|
||||
run_id: &run_id,
|
||||
current_title: "Current",
|
||||
workflow_target: Some("workflow.fabro"),
|
||||
run_inputs: &HashMap::new(),
|
||||
workflow: &summary,
|
||||
})
|
||||
.unwrap();
|
||||
|
||||
assert!(prompt.contains(&format!("no more than {MAX_RUN_TITLE_CHARS} characters")));
|
||||
assert!(prompt.contains(&run_id.to_string()));
|
||||
assert!(prompt.contains("\"stage_count\": 4"));
|
||||
assert!(!prompt.contains("{{"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn prompt_includes_goal_inputs_and_workflow_summary_without_redaction() {
|
||||
let run_id = RunId::new();
|
||||
|
|
@ -225,7 +259,8 @@ mod tests {
|
|||
workflow_target: Some("workflows/deploy.fabro"),
|
||||
run_inputs: &inputs,
|
||||
workflow: &summary,
|
||||
});
|
||||
})
|
||||
.unwrap();
|
||||
|
||||
assert!(prompt.contains("Deploy API token SECRET_123 to production"));
|
||||
assert!(prompt.contains("\"api_key\": \"SECRET_123\""));
|
||||
|
|
@ -251,7 +286,8 @@ mod tests {
|
|||
workflow_target: Some("workflow.fabro"),
|
||||
run_inputs: &inputs,
|
||||
workflow: &summary,
|
||||
});
|
||||
})
|
||||
.unwrap();
|
||||
|
||||
// Section truncation: per-section budget × 3 + small boilerplate.
|
||||
assert!(prompt.chars().count() < MAX_PROMPT_SECTION_CHARS * 3 + 1_000);
|
||||
|
|
|
|||
|
|
@ -5,6 +5,7 @@ use std::sync::Mutex;
|
|||
use std::sync::{Arc, OnceLock};
|
||||
use std::time::Duration;
|
||||
|
||||
use anyhow::Context as _;
|
||||
use axum::extract::Request;
|
||||
#[cfg(test)]
|
||||
use axum::extract::State as AxumState;
|
||||
|
|
@ -257,7 +258,7 @@ impl TestAppStateBuilder {
|
|||
pub fn try_build(mut self) -> anyhow::Result<Arc<AppState>> {
|
||||
let (store, artifact_store) = self.store_bundle.unwrap_or_else(test_store_bundle);
|
||||
let vault_path = self.vault_path.unwrap_or_else(test_secret_store_path);
|
||||
self.server_settings = redirect_default_storage_root(self.server_settings, &vault_path);
|
||||
self.server_settings = redirect_default_storage_root(self.server_settings, &vault_path)?;
|
||||
if !self.vault_entries.is_empty() {
|
||||
let mut vault = Vault::load(vault_path.clone()).expect("test vault should load");
|
||||
for (name, value) in &self.vault_entries {
|
||||
|
|
@ -686,13 +687,17 @@ pub fn test_secret_store_path() -> PathBuf {
|
|||
/// that chose its own root keeps it. The redirect goes through
|
||||
/// [`ServerSettings::with_storage_override`] so the derived local object-store
|
||||
/// roots move with it instead of pointing back at the real storage tree.
|
||||
fn redirect_default_storage_root(settings: ServerSettings, vault_path: &Path) -> ServerSettings {
|
||||
fn redirect_default_storage_root(
|
||||
settings: ServerSettings,
|
||||
vault_path: &Path,
|
||||
) -> anyhow::Result<ServerSettings> {
|
||||
if Path::new(&settings.server.storage.root) != default_storage_dir() {
|
||||
return settings;
|
||||
return Ok(settings);
|
||||
}
|
||||
let root = vault_path.with_file_name("storage");
|
||||
std::fs::create_dir_all(&root).expect("test storage root should be creatable");
|
||||
settings.with_storage_override(&root)
|
||||
std::fs::create_dir_all(&root)
|
||||
.with_context(|| format!("creating test storage root at {}", root.display()))?;
|
||||
Ok(settings.with_storage_override(&root))
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
|
|
@ -772,3 +777,71 @@ pub(crate) async fn capture_auth_context(
|
|||
.push(slot.snapshot());
|
||||
response
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use fabro_types::settings::ObjectStoreSettings;
|
||||
|
||||
use super::*;
|
||||
|
||||
fn local_store_root(store: &ObjectStoreSettings) -> &Path {
|
||||
let ObjectStoreSettings::Local { root } = store else {
|
||||
panic!("test server settings should use a local object store");
|
||||
};
|
||||
Path::new(root)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn default_storage_redirect_updates_derived_local_store_roots() {
|
||||
let temp_dir = tempfile::tempdir().expect("test temp dir should be created");
|
||||
let vault_path = temp_dir.path().join("secrets.json");
|
||||
|
||||
let settings = redirect_default_storage_root(default_test_server_settings(), &vault_path)
|
||||
.expect("default test storage root should redirect");
|
||||
let storage_root = temp_dir.path().join("storage");
|
||||
|
||||
assert_eq!(Path::new(&settings.server.storage.root), storage_root);
|
||||
assert_eq!(
|
||||
local_store_root(&settings.server.artifacts.store),
|
||||
storage_root.join("objects/artifacts")
|
||||
);
|
||||
assert_eq!(
|
||||
local_store_root(&settings.server.slatedb.store),
|
||||
storage_root.join("objects/slatedb")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn default_storage_redirect_preserves_explicit_storage_root() {
|
||||
let temp_dir = tempfile::tempdir().expect("test temp dir should be created");
|
||||
let vault_path = temp_dir.path().join("secrets.json");
|
||||
let expected =
|
||||
default_test_server_settings().with_storage_override(&temp_dir.path().join("custom"));
|
||||
|
||||
let settings = redirect_default_storage_root(expected.clone(), &vault_path)
|
||||
.expect("explicit test storage root should be preserved");
|
||||
|
||||
assert_eq!(settings, expected);
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[expect(
|
||||
clippy::disallowed_methods,
|
||||
reason = "synchronous fixture setup creates a blocking file before exercising the helper"
|
||||
)]
|
||||
fn default_storage_redirect_preserves_directory_creation_error_chain() {
|
||||
let temp_dir = tempfile::tempdir().expect("test temp dir should be created");
|
||||
let vault_path = temp_dir.path().join("secrets.json");
|
||||
std::fs::write(temp_dir.path().join("storage"), "not a directory")
|
||||
.expect("blocking storage path should be created");
|
||||
|
||||
let error = redirect_default_storage_root(default_test_server_settings(), &vault_path)
|
||||
.expect_err("storage redirect should reject a file at the directory path");
|
||||
|
||||
assert!(error.to_string().contains("creating test storage root at"));
|
||||
assert!(
|
||||
error.chain().count() >= 2,
|
||||
"filesystem error should remain in the source chain"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -7,7 +7,7 @@ use fabro_model::ModelSelectionError;
|
|||
use fabro_template::TemplateError;
|
||||
pub use fabro_types::failure_signature::FailureSignature;
|
||||
pub use fabro_types::outcome::FailureCategory;
|
||||
use fabro_types::settings::AmbiguousModelRef;
|
||||
use fabro_types::settings::{AmbiguousModelRef, ResolveError};
|
||||
use fabro_types::{ExecOutputTail, FailureReason, RunFailure};
|
||||
use fabro_util::error::{SharedError, collect_causes, collect_chain, render_with_causes};
|
||||
use fabro_validate::Diagnostic;
|
||||
|
|
@ -280,6 +280,14 @@ pub enum Error {
|
|||
#[error("Validation failed")]
|
||||
ValidationFailed { diagnostics: Vec<Diagnostic> },
|
||||
|
||||
#[error("Validation error: script interpolation failed in {owner}: {source} ({fix})")]
|
||||
ScriptInterpolation {
|
||||
owner: String,
|
||||
fix: String,
|
||||
#[source]
|
||||
source: ResolveError,
|
||||
},
|
||||
|
||||
#[error("Model selection failed: {0}")]
|
||||
ModelSelection(#[from] ModelSelectionError),
|
||||
|
||||
|
|
@ -451,6 +459,7 @@ impl Error {
|
|||
.as_ref()
|
||||
.map_or_else(Vec::new, |source| collect_chain(source)),
|
||||
Self::Template { source, .. } => collect_chain(source),
|
||||
Self::ScriptInterpolation { source, .. } => collect_chain(source),
|
||||
Self::Llm(err) => collect_causes(err),
|
||||
_ => Vec::new(),
|
||||
}
|
||||
|
|
@ -478,6 +487,7 @@ impl Error {
|
|||
Self::Parse(_)
|
||||
| Self::Validation(_)
|
||||
| Self::ValidationFailed { .. }
|
||||
| Self::ScriptInterpolation { .. }
|
||||
| Self::ModelSelection(_)
|
||||
| Self::ModelReference(_)
|
||||
| Self::Template { .. }
|
||||
|
|
@ -501,6 +511,7 @@ impl Error {
|
|||
Self::Parse(_)
|
||||
| Self::Validation(_)
|
||||
| Self::ValidationFailed { .. }
|
||||
| Self::ScriptInterpolation { .. }
|
||||
| Self::ModelSelection(_)
|
||||
| Self::ModelReference(_)
|
||||
| Self::Template { .. }
|
||||
|
|
|
|||
|
|
@ -722,6 +722,58 @@ reasoning = false
|
|||
assert!(validated.has_errors());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn vars_resolve_in_command_script_through_create_pipeline() {
|
||||
let dot = r#"digraph Test {
|
||||
graph [goal="Ship it"]
|
||||
start [shape=Mdiamond, label="Start"]
|
||||
exit [shape=Msquare, label="Exit"]
|
||||
work [label="Work", shape=parallelogram, script="deploy --stage {{ vars.STAGE }}"]
|
||||
start -> work -> exit
|
||||
}"#;
|
||||
let vars = HashMap::from([("STAGE".to_string(), "staging".to_string())]);
|
||||
let validated = validate_dot_with_vars(dot, vars);
|
||||
validated.raise_on_errors().unwrap();
|
||||
|
||||
assert_eq!(
|
||||
validated.graph().nodes["work"]
|
||||
.attrs
|
||||
.get("script")
|
||||
.and_then(fabro_graphviz::graph::AttrValue::as_str),
|
||||
Some("deploy --stage staging"),
|
||||
);
|
||||
}
|
||||
|
||||
/// The script diagnostic must carry the same rule as the prompt one so the
|
||||
/// existing run-create promotion catches an unbound value before a run
|
||||
/// executes a half-interpolated command.
|
||||
#[test]
|
||||
fn unknown_input_in_script_warns_at_validate_then_errors_at_run_create() {
|
||||
let dot = r#"digraph Test {
|
||||
graph [goal="Ship it"]
|
||||
start [shape=Mdiamond, label="Start"]
|
||||
exit [shape=Msquare, label="Exit"]
|
||||
work [label="Work", shape=parallelogram, script="deploy --stage {{ inputs.stage }}"]
|
||||
start -> work -> exit
|
||||
}"#;
|
||||
let mut validated = validate_dot_with_vars(dot, HashMap::new());
|
||||
|
||||
let diagnostic = validated
|
||||
.diagnostics()
|
||||
.iter()
|
||||
.find(|d| d.rule == TEMPLATE_UNDEFINED_VARIABLE_RULE)
|
||||
.expect("expected a template_undefined_variable diagnostic");
|
||||
assert_eq!(diagnostic.severity, Severity::Warning);
|
||||
assert!(
|
||||
diagnostic.message.contains("inputs.stage"),
|
||||
"message: {}",
|
||||
diagnostic.message
|
||||
);
|
||||
|
||||
validated.promote_template_undefined_variables_to_errors();
|
||||
assert!(validated.has_errors());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn promote_template_undefined_rule_turns_warning_into_error() {
|
||||
let dot = r#"digraph Test {
|
||||
|
|
|
|||
|
|
@ -3,8 +3,8 @@ use std::sync::Arc;
|
|||
use super::types::{Parsed, TransformOptions, Transformed};
|
||||
use crate::error::Error;
|
||||
use crate::transforms::{
|
||||
FileInliningTransform, ImportTransform, StylesheetApplicationTransform, TemplateTransform,
|
||||
Transform,
|
||||
FileInliningTransform, ImportTransform, ScriptInterpolationTransform,
|
||||
StylesheetApplicationTransform, TemplateTransform, Transform,
|
||||
};
|
||||
|
||||
/// TRANSFORM phase: apply built-in and custom transforms to a parsed graph.
|
||||
|
|
@ -62,6 +62,13 @@ pub fn transform(parsed: Parsed, options: &TransformOptions) -> Result<Transform
|
|||
}
|
||||
.apply_with_diagnostics(graph)?;
|
||||
diagnostics.extend(transform_diagnostics);
|
||||
let (graph, transform_diagnostics) = ScriptInterpolationTransform {
|
||||
context: options.template_context.clone(),
|
||||
source_name: options.source_name.clone(),
|
||||
render_mode: options.render_mode,
|
||||
}
|
||||
.apply_with_diagnostics(graph)?;
|
||||
diagnostics.extend(transform_diagnostics);
|
||||
let graph = StylesheetApplicationTransform.apply(graph)?;
|
||||
let graph = match &options.model_resolution {
|
||||
Some(model_resolution) => model_resolution.apply(graph)?,
|
||||
|
|
@ -238,6 +245,92 @@ mod tests {
|
|||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn imported_script_does_not_rescan_substituted_token_syntax() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
write_file(
|
||||
&dir.path().join("child.fabro"),
|
||||
r#"digraph child {
|
||||
start [shape=Mdiamond]
|
||||
run [shape=parallelogram, script="printf '%s' {{ inputs.payload }}"]
|
||||
exit [shape=Msquare]
|
||||
start -> run -> exit
|
||||
}"#,
|
||||
);
|
||||
let parsed = parse(
|
||||
r#"digraph Test {
|
||||
graph [goal="Test"]
|
||||
start [shape=Mdiamond]
|
||||
child [import="./child.fabro"]
|
||||
exit [shape=Msquare]
|
||||
start -> child -> exit
|
||||
}"#,
|
||||
)
|
||||
.unwrap();
|
||||
let transformed = transform(parsed, &TransformOptions {
|
||||
current_dir: Some(dir.path().to_path_buf()),
|
||||
file_resolver: Some(Arc::new(FilesystemFileResolver::new(None))),
|
||||
template_context: fabro_template::TemplateContext::new().with_inputs(HashMap::from([
|
||||
(
|
||||
"payload".to_string(),
|
||||
toml::Value::String("{{ secrets.API_KEY }}".to_string()),
|
||||
),
|
||||
])),
|
||||
..transform_options()
|
||||
})
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(
|
||||
transformed.graph.nodes["child.run"]
|
||||
.attrs
|
||||
.get("script")
|
||||
.and_then(AttrValue::as_str),
|
||||
Some("printf '%s' '{{ secrets.API_KEY }}'")
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn imported_script_reports_a_missing_value_once() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
write_file(
|
||||
&dir.path().join("child.fabro"),
|
||||
r#"digraph child {
|
||||
start [shape=Mdiamond]
|
||||
run [shape=parallelogram, script="echo {{ inputs.missing }}"]
|
||||
exit [shape=Msquare]
|
||||
start -> run -> exit
|
||||
}"#,
|
||||
);
|
||||
let parsed = parse(
|
||||
r#"digraph Test {
|
||||
graph [goal="Test"]
|
||||
start [shape=Mdiamond]
|
||||
child [import="./child.fabro"]
|
||||
exit [shape=Msquare]
|
||||
start -> child -> exit
|
||||
}"#,
|
||||
)
|
||||
.unwrap();
|
||||
let transformed = transform(parsed, &TransformOptions {
|
||||
current_dir: Some(dir.path().to_path_buf()),
|
||||
file_resolver: Some(Arc::new(FilesystemFileResolver::new(None))),
|
||||
render_mode: crate::operations::RenderMode::Structural,
|
||||
..transform_options()
|
||||
})
|
||||
.unwrap();
|
||||
|
||||
let missing_value_diagnostics = transformed
|
||||
.diagnostics
|
||||
.iter()
|
||||
.filter(|diagnostic| diagnostic.rule == TEMPLATE_UNDEFINED_VARIABLE_RULE)
|
||||
.count();
|
||||
assert_eq!(
|
||||
missing_value_diagnostics, 1,
|
||||
"{:?}",
|
||||
transformed.diagnostics
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn transform_interpolates_vars_in_node_prompt() {
|
||||
let dot = r#"digraph Test {
|
||||
|
|
|
|||
|
|
@ -22,4 +22,4 @@ pub use import::ImportTransform;
|
|||
pub use model_resolution::ModelResolutionTransform;
|
||||
pub use preamble::PreambleTransform;
|
||||
pub use stylesheet_application::StylesheetApplicationTransform;
|
||||
pub use variable_expansion::{RenderMode, TemplateTransform};
|
||||
pub use variable_expansion::{RenderMode, ScriptInterpolationTransform, TemplateTransform};
|
||||
|
|
|
|||
|
|
@ -1,13 +1,17 @@
|
|||
use std::borrow::Cow;
|
||||
use std::collections::HashMap;
|
||||
use std::fmt::Write as _;
|
||||
use std::sync::Arc;
|
||||
|
||||
use fabro_graphviz::graph::{AttrValue, Graph};
|
||||
use fabro_graphviz::graph::{AttrValue, Graph, Node};
|
||||
use fabro_template::{
|
||||
TemplateContext, TemplateError, TemplateRenderMode, TemplateSource, TemplateSourceOrigin,
|
||||
TemplateStore,
|
||||
};
|
||||
use fabro_types::settings::interp::Namespace;
|
||||
use fabro_types::settings::{InterpString, ResolveCtx, ResolveError, ResolveErrorKind};
|
||||
use fabro_util::error::collect_chain;
|
||||
use fabro_util::shell;
|
||||
use fabro_validate::{Diagnostic, Severity};
|
||||
|
||||
use super::Transform;
|
||||
|
|
@ -223,9 +227,7 @@ fn template_diagnostic(error: &TemplateError, target: &TemplateRenderTarget) ->
|
|||
message,
|
||||
node_id: target.node_id.clone(),
|
||||
edge: target.edge.clone(),
|
||||
fix: Some(format!(
|
||||
"bind `{name}` via `[run.inputs]` in workflow.toml, or pass `--input {name}=<value>`"
|
||||
)),
|
||||
fix: Some(input_binding_fix(name)),
|
||||
source_path: location.source_name.or_else(|| target.source_name.clone()),
|
||||
line: location.line,
|
||||
column: location.column,
|
||||
|
|
@ -235,6 +237,143 @@ fn template_diagnostic(error: &TemplateError, target: &TemplateRenderTarget) ->
|
|||
}
|
||||
}
|
||||
|
||||
fn input_binding_fix(name: &str) -> String {
|
||||
format!("bind `{name}` via `[run.inputs]` in workflow.toml, or pass `--input {name}=<value>`")
|
||||
}
|
||||
|
||||
/// Substitutes `{{ goal }}`, `{{ inputs.* }}`, and `{{ vars.* }}` in one
|
||||
/// command node `script`.
|
||||
///
|
||||
/// Scripts interpolate through [`InterpString`] tokens rather than the
|
||||
/// MiniJinja pass that renders prompts. Shell source is full of brace syntax
|
||||
/// that must survive untouched — `jq` filters, `awk` programs, Go templates,
|
||||
/// brace expansion — and `InterpString` claims only the bare `{{ goal }}` and
|
||||
/// `{{ <known-namespace>.NAME }}`, leaving everything else literal.
|
||||
///
|
||||
/// `env` and `secrets` are deliberately not wired, so a token in either
|
||||
/// namespace fails as [`ResolveErrorKind::Unavailable`]. A script reads the
|
||||
/// environment with `$NAME`, which needs no interpolation, and a resolved
|
||||
/// secret would be baked into the `CommandStarted` event that records the
|
||||
/// script verbatim.
|
||||
///
|
||||
/// Shell values are quoted as one argument. Python values are quoted as string
|
||||
/// literals. In both languages the token must stand where one value is valid;
|
||||
/// callers must not wrap it in another string literal.
|
||||
fn interpolate_script<'a>(
|
||||
text: &'a str,
|
||||
ctx: &TemplateContext,
|
||||
language: &str,
|
||||
render_mode: RenderMode,
|
||||
target: &TemplateRenderTarget,
|
||||
diagnostics: &mut Vec<Diagnostic>,
|
||||
) -> Result<Cow<'a, str>, Error> {
|
||||
if !text.contains("{{") {
|
||||
return Ok(Cow::Borrowed(text));
|
||||
}
|
||||
|
||||
let parsed = InterpString::parse(text);
|
||||
if parsed.is_literal() {
|
||||
return Ok(Cow::Borrowed(text));
|
||||
}
|
||||
|
||||
let mut resolve_ctx = ResolveCtx::new()
|
||||
.with_inputs(|name| {
|
||||
ctx.input(name)
|
||||
.map(|value| quote_script_value(&value, language))
|
||||
})
|
||||
.with_vars(|name| {
|
||||
ctx.var(name)
|
||||
.map(|value| quote_script_value(&value, language))
|
||||
});
|
||||
// The graph goal is rendered before any node attribute, so by here it is
|
||||
// the final text. Substituting it does not re-interpolate: whatever the
|
||||
// goal contains lands in the script as literal characters.
|
||||
if let Some(goal) = ctx.goal() {
|
||||
resolve_ctx = resolve_ctx.with_goal(quote_script_value(goal, language));
|
||||
}
|
||||
match parsed.resolve_with(&mut resolve_ctx) {
|
||||
Ok(resolved) => Ok(Cow::Owned(resolved)),
|
||||
// An unbound input or variable is the same authoring gap the prompt
|
||||
// pass reports, so it follows the same mode split: a hard error at
|
||||
// run-create, a diagnostic during `fabro validate`.
|
||||
Err(err) if err.kind == ResolveErrorKind::Missing => match render_mode {
|
||||
RenderMode::Strict => Err(script_interpolation_error(target, err, language)),
|
||||
RenderMode::Structural => {
|
||||
diagnostics.push(script_undefined_variable_diagnostic(&err, target));
|
||||
// Leave the script in source form. Validation never executes
|
||||
// it, and showing the unresolved token beats emptying it.
|
||||
Ok(Cow::Borrowed(text))
|
||||
}
|
||||
},
|
||||
// An unsupported namespace can never resolve here, however the inputs
|
||||
// are bound, so it fails in both modes — the same treatment
|
||||
// `render_attrs` gives an invalid static reference.
|
||||
Err(err) => Err(script_interpolation_error(target, err, language)),
|
||||
}
|
||||
}
|
||||
|
||||
fn quote_script_value(value: &str, language: &str) -> String {
|
||||
if language == "python" {
|
||||
serde_json::to_string(value).expect("serializing a string to JSON should not fail")
|
||||
} else {
|
||||
shell::shell_quote(value)
|
||||
}
|
||||
}
|
||||
|
||||
fn script_interpolation_error(
|
||||
target: &TemplateRenderTarget,
|
||||
source: ResolveError,
|
||||
language: &str,
|
||||
) -> Error {
|
||||
let fix = script_interpolation_fix(&source, Some(language));
|
||||
Error::ScriptInterpolation {
|
||||
owner: target.owner.clone(),
|
||||
fix,
|
||||
source,
|
||||
}
|
||||
}
|
||||
|
||||
fn script_undefined_variable_diagnostic(
|
||||
err: &ResolveError,
|
||||
target: &TemplateRenderTarget,
|
||||
) -> Diagnostic {
|
||||
Diagnostic {
|
||||
rule: TEMPLATE_UNDEFINED_VARIABLE_RULE.to_owned(),
|
||||
severity: Severity::Warning,
|
||||
message: format!("{err} in {}", target.owner),
|
||||
node_id: target.node_id.clone(),
|
||||
edge: target.edge.clone(),
|
||||
fix: Some(script_interpolation_fix(err, None)),
|
||||
source_path: target.source_name.clone(),
|
||||
..Diagnostic::default()
|
||||
}
|
||||
}
|
||||
|
||||
fn script_interpolation_fix(err: &ResolveError, language: Option<&str>) -> String {
|
||||
let name = &err.name;
|
||||
match err.namespace {
|
||||
Namespace::Inputs => input_binding_fix(name),
|
||||
Namespace::Vars => format!("set it with `fabro variable set {name} <value>`"),
|
||||
Namespace::Env if language == Some("python") => format!(
|
||||
"`script` does not interpolate environment variables; read it in Python as \
|
||||
`os.environ[\"{name}\"]` instead"
|
||||
),
|
||||
Namespace::Env => format!(
|
||||
"`script` does not interpolate environment variables; read it in the shell as \
|
||||
`${name}` instead"
|
||||
),
|
||||
Namespace::Secrets if language == Some("python") => format!(
|
||||
"`script` does not interpolate secrets; expose `{name}` to the sandbox through \
|
||||
`[environments.<slug>.env]` and read it in Python as `os.environ[\"{name}\"]`"
|
||||
),
|
||||
Namespace::Secrets => format!(
|
||||
"`script` does not interpolate secrets; expose `{name}` to the sandbox through \
|
||||
`[environments.<slug>.env]` and read it in the shell as `${name}`"
|
||||
),
|
||||
Namespace::Goal => "set a graph `goal` on the workflow".to_string(),
|
||||
}
|
||||
}
|
||||
|
||||
const DETEMPLATED_ATTRIBUTE_RULE: &str = "detemplated_attribute";
|
||||
|
||||
/// Warning emitted when an attribute that is no longer a template still
|
||||
|
|
@ -245,7 +384,9 @@ fn detemplated_attribute_diagnostic(attr_name: &str, target: &TemplateRenderTarg
|
|||
severity: Severity::Warning,
|
||||
message: format!(
|
||||
"`{attr_name}` in {} is no longer a template; `{{{{ … }}}}` / `{{% … %}}` is treated \
|
||||
as literal text. Only node `prompt` and graph `goal` support templating.",
|
||||
as literal text. Only node `prompt` and graph `goal` support templating, and node \
|
||||
command `script` supports `{{{{ goal }}}}`, `{{{{ inputs.* }}}}`, and \
|
||||
`{{{{ vars.* }}}}` interpolation.",
|
||||
target.owner
|
||||
),
|
||||
node_id: target.node_id.clone(),
|
||||
|
|
@ -290,8 +431,8 @@ fn goal_self_reference_diagnostic(
|
|||
}
|
||||
}
|
||||
|
||||
/// Expands `{{ goal }}` / `{{ inputs.* }}` / `{{ vars.* }}` across all string
|
||||
/// attributes.
|
||||
/// Renders graph goals and node prompts, and diagnoses template syntax in
|
||||
/// attributes that do not support it.
|
||||
pub struct TemplateTransform {
|
||||
pub context: TemplateContext,
|
||||
pub source_name: Option<String>,
|
||||
|
|
@ -374,6 +515,11 @@ impl TemplateTransform {
|
|||
if attr_name == "stack.child_dot_source" {
|
||||
continue;
|
||||
}
|
||||
// Command scripts use narrow value interpolation in a separate
|
||||
// one-shot transform after imports are expanded.
|
||||
if matches!(scope, AttributeScope::Node) && attr_name == "script" {
|
||||
continue;
|
||||
}
|
||||
if let Some(kind) = reference_kind_for_attribute(scope, attr_name, text) {
|
||||
validate_static_reference(text, kind)
|
||||
.map_err(|error| Error::Validation(error.to_string()))?;
|
||||
|
|
@ -383,7 +529,8 @@ impl TemplateTransform {
|
|||
.with_source_name(source_name.cloned().unwrap_or_else(|| "workflow".into()))
|
||||
.with_source_origin(source_text, text);
|
||||
if matches!(scope, AttributeScope::Node) && attr_name == "prompt" {
|
||||
// `prompt` is the only templated node attribute.
|
||||
// `prompt` is the only node attribute rendered as a full
|
||||
// MiniJinja template.
|
||||
*text =
|
||||
render_template_for_target(text, ctx, render_mode, &target, diagnostics)?;
|
||||
} else if fabro_template::contains_template_syntax(text) {
|
||||
|
|
@ -473,6 +620,83 @@ impl Transform for TemplateTransform {
|
|||
}
|
||||
}
|
||||
|
||||
/// Interpolates command node scripts once, after import expansion is complete.
|
||||
///
|
||||
/// Keeping this pass separate from [`TemplateTransform`] prevents imported
|
||||
/// scripts from being scanned once in their source graph and again after they
|
||||
/// are merged into the root graph.
|
||||
pub struct ScriptInterpolationTransform {
|
||||
pub context: TemplateContext,
|
||||
pub source_name: Option<String>,
|
||||
pub render_mode: RenderMode,
|
||||
}
|
||||
|
||||
impl ScriptInterpolationTransform {
|
||||
fn command_script_language(node: &Node) -> Option<&'static str> {
|
||||
let is_command = matches!(node.handler_type(), Some("command" | "tool"));
|
||||
is_command.then(|| {
|
||||
if node.attrs.get("language").and_then(AttrValue::as_str) == Some("python") {
|
||||
"python"
|
||||
} else {
|
||||
"shell"
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
pub(crate) fn apply_with_diagnostics(
|
||||
&self,
|
||||
graph: Graph,
|
||||
) -> Result<(Graph, Vec<Diagnostic>), Error> {
|
||||
let mut graph = graph;
|
||||
let mut diagnostics = Vec::new();
|
||||
let ctx = self.context.clone().with_goal(graph.goal().to_string());
|
||||
|
||||
for (node_id, node) in &mut graph.nodes {
|
||||
let language = Self::command_script_language(node);
|
||||
let Some(AttrValue::String(text)) = node.attrs.get_mut("script") else {
|
||||
continue;
|
||||
};
|
||||
let target = TemplateRenderTarget::node_attr(
|
||||
self.source_name.clone(),
|
||||
node_id.clone(),
|
||||
"script",
|
||||
)
|
||||
.with_source_name(
|
||||
self.source_name
|
||||
.clone()
|
||||
.unwrap_or_else(|| "workflow".to_string()),
|
||||
);
|
||||
|
||||
if let Some(language) = language {
|
||||
if let Cow::Owned(resolved) = interpolate_script(
|
||||
text,
|
||||
&ctx,
|
||||
language,
|
||||
self.render_mode,
|
||||
&target,
|
||||
&mut diagnostics,
|
||||
)? {
|
||||
*text = resolved;
|
||||
}
|
||||
} else if fabro_template::contains_template_syntax(text) {
|
||||
diagnostics.push(detemplated_attribute_diagnostic("script", &target));
|
||||
}
|
||||
}
|
||||
|
||||
Ok((graph, diagnostics))
|
||||
}
|
||||
}
|
||||
|
||||
impl Transform for ScriptInterpolationTransform {
|
||||
fn apply(&self, graph: Graph) -> Result<Graph, Error> {
|
||||
let (graph, diagnostics) = self.apply_with_diagnostics(graph)?;
|
||||
if !diagnostics.is_empty() {
|
||||
return Err(Error::ValidationFailed { diagnostics });
|
||||
}
|
||||
Ok(graph)
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use std::collections::HashMap;
|
||||
|
|
@ -557,6 +781,322 @@ mod tests {
|
|||
);
|
||||
}
|
||||
|
||||
/// Build a one-node graph whose `test` node carries `script`.
|
||||
fn script_graph(script: &str) -> Graph {
|
||||
let mut graph = Graph::new("test");
|
||||
graph
|
||||
.attrs
|
||||
.insert("goal".to_string(), AttrValue::String("Ship it".to_string()));
|
||||
let mut node = Node::new("test");
|
||||
node.attrs.insert(
|
||||
"shape".to_string(),
|
||||
AttrValue::String("parallelogram".to_string()),
|
||||
);
|
||||
node.attrs
|
||||
.insert("script".to_string(), AttrValue::String(script.to_string()));
|
||||
graph.nodes.insert("test".to_string(), node);
|
||||
graph
|
||||
}
|
||||
|
||||
fn script_transform(
|
||||
inputs: &[(&str, toml::Value)],
|
||||
vars: &[(&str, &str)],
|
||||
render_mode: RenderMode,
|
||||
) -> ScriptInterpolationTransform {
|
||||
ScriptInterpolationTransform {
|
||||
context: TemplateContext::new()
|
||||
.with_inputs(
|
||||
inputs
|
||||
.iter()
|
||||
.map(|(k, v)| ((*k).to_string(), v.clone()))
|
||||
.collect(),
|
||||
)
|
||||
.with_vars(
|
||||
vars.iter()
|
||||
.map(|(k, v)| ((*k).to_string(), (*v).to_string()))
|
||||
.collect(),
|
||||
),
|
||||
source_name: None,
|
||||
render_mode,
|
||||
}
|
||||
}
|
||||
|
||||
fn script_of(graph: &Graph) -> &str {
|
||||
graph.nodes["test"]
|
||||
.attrs
|
||||
.get("script")
|
||||
.and_then(AttrValue::as_str)
|
||||
.expect("script attribute should still be a string")
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn script_interpolates_inputs_and_vars() {
|
||||
let graph = script_graph("cargo test -p {{ inputs.crate }} --profile {{ vars.PROFILE }}");
|
||||
let transform = script_transform(
|
||||
&[("crate", toml::Value::String("fabro-workflow".into()))],
|
||||
&[("PROFILE", "ci")],
|
||||
RenderMode::Structural,
|
||||
);
|
||||
|
||||
let (graph, diagnostics) = transform.apply_with_diagnostics(graph).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
script_of(&graph),
|
||||
"cargo test -p fabro-workflow --profile ci"
|
||||
);
|
||||
assert!(diagnostics.is_empty(), "unexpected: {diagnostics:?}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn script_substitutes_the_rendered_goal() {
|
||||
let graph = script_graph("gh pr create --title {{ goal }}");
|
||||
let transform = script_transform(&[], &[], RenderMode::Structural);
|
||||
|
||||
let (graph, diagnostics) = transform.apply_with_diagnostics(graph).unwrap();
|
||||
|
||||
assert_eq!(script_of(&graph), "gh pr create --title 'Ship it'");
|
||||
assert!(diagnostics.is_empty(), "unexpected: {diagnostics:?}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn shell_script_quotes_substituted_values_as_one_argument() {
|
||||
let graph = script_graph("deploy --release {{ inputs.release }}");
|
||||
let transform = script_transform(
|
||||
&[(
|
||||
"release",
|
||||
toml::Value::String("stable; touch /tmp/pwned".to_string()),
|
||||
)],
|
||||
&[],
|
||||
RenderMode::Strict,
|
||||
);
|
||||
|
||||
let (graph, diagnostics) = transform.apply_with_diagnostics(graph).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
script_of(&graph),
|
||||
"deploy --release 'stable; touch /tmp/pwned'"
|
||||
);
|
||||
assert!(diagnostics.is_empty(), "unexpected: {diagnostics:?}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn python_script_quotes_substituted_values_as_string_literals() {
|
||||
let mut graph = script_graph("print({{ inputs.value }})");
|
||||
graph.nodes.get_mut("test").unwrap().attrs.insert(
|
||||
"language".to_string(),
|
||||
AttrValue::String("python".to_string()),
|
||||
);
|
||||
let transform = script_transform(
|
||||
&[(
|
||||
"value",
|
||||
toml::Value::String("'); __import__('os').system('id'); #".to_string()),
|
||||
)],
|
||||
&[],
|
||||
RenderMode::Strict,
|
||||
);
|
||||
|
||||
let (graph, diagnostics) = transform.apply_with_diagnostics(graph).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
script_of(&graph),
|
||||
r#"print("'); __import__('os').system('id'); #")"#
|
||||
);
|
||||
assert!(diagnostics.is_empty(), "unexpected: {diagnostics:?}");
|
||||
}
|
||||
|
||||
/// The reason `script` uses `InterpString` rather than MiniJinja: shell
|
||||
/// source is full of brace syntax that must reach the shell untouched.
|
||||
#[test]
|
||||
fn script_leaves_non_token_braces_literal() {
|
||||
let script = "jq '{name: .name}' f.json | awk '{print $1}'; echo {{ .Values.image }}; \
|
||||
touch {a,b}.txt; {% raw %}";
|
||||
let graph = script_graph(script);
|
||||
let transform = script_transform(&[], &[], RenderMode::Structural);
|
||||
|
||||
let (graph, diagnostics) = transform.apply_with_diagnostics(graph).unwrap();
|
||||
|
||||
assert_eq!(script_of(&graph), script);
|
||||
assert!(diagnostics.is_empty(), "unexpected: {diagnostics:?}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn script_rejects_secrets_tokens_in_both_render_modes() {
|
||||
for render_mode in [RenderMode::Structural, RenderMode::Strict] {
|
||||
let graph = script_graph("curl -H \"Authorization: {{ secrets.API_KEY }}\" $URL");
|
||||
let transform = script_transform(&[], &[], render_mode);
|
||||
|
||||
let err = transform
|
||||
.apply_with_diagnostics(graph)
|
||||
.expect_err("secrets must never interpolate into a script");
|
||||
|
||||
let message = err.to_string();
|
||||
assert!(
|
||||
message.contains("does not interpolate secrets"),
|
||||
"unexpected message: {message}"
|
||||
);
|
||||
assert!(
|
||||
message.contains("$API_KEY"),
|
||||
"should point at the shell alternative: {message}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn script_rejects_env_tokens_and_points_at_shell_expansion() {
|
||||
let graph = script_graph("echo {{ env.HOME }}");
|
||||
let transform = script_transform(&[], &[], RenderMode::Structural);
|
||||
|
||||
let err = transform
|
||||
.apply_with_diagnostics(graph)
|
||||
.expect_err("env is not interpolated in a script");
|
||||
|
||||
let message = err.to_string();
|
||||
assert!(message.contains("$HOME"), "unexpected message: {message}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn script_missing_input_warns_and_preserves_source_in_structural_mode() {
|
||||
let script = "cargo test -p {{ inputs.crate }}";
|
||||
let graph = script_graph(script);
|
||||
let transform = script_transform(&[], &[], RenderMode::Structural);
|
||||
|
||||
let (graph, diagnostics) = transform.apply_with_diagnostics(graph).unwrap();
|
||||
|
||||
assert_eq!(script_of(&graph), script);
|
||||
let diagnostic = diagnostics
|
||||
.iter()
|
||||
.find(|d| d.rule == TEMPLATE_UNDEFINED_VARIABLE_RULE)
|
||||
.expect("an unbound input should warn");
|
||||
assert_eq!(diagnostic.severity, Severity::Warning);
|
||||
assert_eq!(diagnostic.node_id.as_deref(), Some("test"));
|
||||
assert!(
|
||||
diagnostic
|
||||
.fix
|
||||
.as_deref()
|
||||
.unwrap_or_default()
|
||||
.contains("--input crate="),
|
||||
"unexpected fix: {:?}",
|
||||
diagnostic.fix
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn script_missing_input_is_an_error_in_strict_mode() {
|
||||
let graph = script_graph("cargo test -p {{ inputs.crate }}");
|
||||
let transform = script_transform(&[], &[], RenderMode::Strict);
|
||||
|
||||
let err = transform
|
||||
.apply_with_diagnostics(graph)
|
||||
.expect_err("strict mode must reject an unbound input");
|
||||
|
||||
assert!(
|
||||
err.to_string().contains("inputs.crate"),
|
||||
"unexpected message: {err}"
|
||||
);
|
||||
let source = std::error::Error::source(&err)
|
||||
.expect("script interpolation errors should preserve ResolveError as their source");
|
||||
assert!(
|
||||
source.to_string().contains("inputs.crate"),
|
||||
"unexpected source: {source}"
|
||||
);
|
||||
}
|
||||
|
||||
/// A typed input must produce the same text in a script as in a prompt, so
|
||||
/// authors do not have to reason about two stringification rules.
|
||||
#[test]
|
||||
fn script_and_prompt_stringify_typed_inputs_identically() {
|
||||
let mut graph =
|
||||
script_graph("retry --times {{ inputs.attempts }} --fast {{ inputs.fast }}");
|
||||
graph.nodes.get_mut("test").unwrap().attrs.insert(
|
||||
"prompt".to_string(),
|
||||
AttrValue::String(
|
||||
"retry --times {{ inputs.attempts }} --fast {{ inputs.fast }}".to_string(),
|
||||
),
|
||||
);
|
||||
let context = TemplateContext::new().with_inputs(HashMap::from([
|
||||
("attempts".to_string(), toml::Value::Integer(3)),
|
||||
("fast".to_string(), toml::Value::Boolean(true)),
|
||||
]));
|
||||
let (graph, _) = TemplateTransform {
|
||||
context: context.clone(),
|
||||
source_name: None,
|
||||
source_text: None,
|
||||
render_mode: RenderMode::Structural,
|
||||
}
|
||||
.apply_with_diagnostics(graph)
|
||||
.unwrap();
|
||||
let (graph, _) = ScriptInterpolationTransform {
|
||||
context,
|
||||
source_name: None,
|
||||
render_mode: RenderMode::Structural,
|
||||
}
|
||||
.apply_with_diagnostics(graph)
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(script_of(&graph), "retry --times 3 --fast true");
|
||||
assert_eq!(
|
||||
graph.nodes["test"]
|
||||
.attrs
|
||||
.get("prompt")
|
||||
.and_then(AttrValue::as_str),
|
||||
Some(script_of(&graph)),
|
||||
);
|
||||
}
|
||||
|
||||
/// `script` is node-scoped. A graph or edge attribute of the same name is
|
||||
/// not a command node script and keeps the demoted-attribute behavior.
|
||||
#[test]
|
||||
fn script_interpolation_is_node_scoped() {
|
||||
let mut graph = script_graph("echo ok");
|
||||
graph.attrs.insert(
|
||||
"script".to_string(),
|
||||
AttrValue::String("echo {{ inputs.crate }}".to_string()),
|
||||
);
|
||||
let (graph, mut diagnostics) = TemplateTransform::new(HashMap::new())
|
||||
.apply_with_diagnostics(graph)
|
||||
.unwrap();
|
||||
let transform = script_transform(&[], &[], RenderMode::Structural);
|
||||
let (graph, script_diagnostics) = transform.apply_with_diagnostics(graph).unwrap();
|
||||
diagnostics.extend(script_diagnostics);
|
||||
|
||||
assert_eq!(
|
||||
graph.attrs.get("script"),
|
||||
Some(&AttrValue::String("echo {{ inputs.crate }}".to_string()))
|
||||
);
|
||||
assert!(
|
||||
diagnostics
|
||||
.iter()
|
||||
.any(|d| d.rule == DETEMPLATED_ATTRIBUTE_RULE),
|
||||
"graph-scope `script` should still warn: {diagnostics:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn non_command_node_script_stays_literal() {
|
||||
let mut graph = script_graph("echo {{ inputs.value }}");
|
||||
graph
|
||||
.nodes
|
||||
.get_mut("test")
|
||||
.unwrap()
|
||||
.attrs
|
||||
.insert("shape".to_string(), AttrValue::String("box".to_string()));
|
||||
let transform = script_transform(
|
||||
&[("value", toml::Value::String("changed".to_string()))],
|
||||
&[],
|
||||
RenderMode::Structural,
|
||||
);
|
||||
|
||||
let (graph, diagnostics) = transform.apply_with_diagnostics(graph).unwrap();
|
||||
|
||||
assert_eq!(script_of(&graph), "echo {{ inputs.value }}");
|
||||
assert!(
|
||||
diagnostics
|
||||
.iter()
|
||||
.any(|diagnostic| diagnostic.rule == DETEMPLATED_ATTRIBUTE_RULE),
|
||||
"non-command scripts should keep the literal-template warning: {diagnostics:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn template_transform_leaves_non_string_attrs_unchanged() {
|
||||
let mut graph = Graph::new("test");
|
||||
|
|
|
|||
|
|
@ -1316,15 +1316,17 @@ impl Catalog {
|
|||
}
|
||||
|
||||
/// Small default model for a provider — the small/cheap utility model used
|
||||
/// for metadata enrichment. Falls back to the provider's normal default
|
||||
/// when no explicit small default is configured.
|
||||
/// for metadata enrichment. `None` when the provider marks no small
|
||||
/// default. Deliberately does not substitute the provider's normal
|
||||
/// default, which is typically a large reasoning model: callers asking for
|
||||
/// a small model give it a small token budget and a short timeout, and a
|
||||
/// flagship model silently exceeds both.
|
||||
#[must_use]
|
||||
pub fn small_default_for_provider(&self, p: &ProviderId) -> Option<&Model> {
|
||||
let provider_id = self.provider(p).map_or(p, |provider| &provider.id);
|
||||
self.models
|
||||
.iter()
|
||||
.find(|m| &m.provider == provider_id && m.small_default)
|
||||
.or_else(|| self.default_for_provider(provider_id))
|
||||
}
|
||||
|
||||
/// Default model for the best-available provider (based on API keys),
|
||||
|
|
@ -1357,10 +1359,7 @@ impl Catalog {
|
|||
if configured.is_empty() {
|
||||
return self.default_model();
|
||||
}
|
||||
let configured = configured
|
||||
.iter()
|
||||
.filter_map(|id| self.provider(id).map(|provider| provider.id.clone()))
|
||||
.collect::<HashSet<_>>();
|
||||
let configured = self.canonical_provider_ids(configured);
|
||||
self.providers
|
||||
.iter()
|
||||
.filter(|provider| configured.contains(&provider.id))
|
||||
|
|
@ -1368,22 +1367,38 @@ impl Catalog {
|
|||
.unwrap_or_else(|| self.default_model())
|
||||
}
|
||||
|
||||
/// Small default model for the best-available built-in provider IDs,
|
||||
/// falling back to the global catalog default.
|
||||
/// Small default model for the best-available built-in provider IDs.
|
||||
///
|
||||
/// Configured providers that mark no small default are skipped in favour
|
||||
/// of a lower-priority provider that has one. Only when none of them does
|
||||
/// is the ordinary default used.
|
||||
#[must_use]
|
||||
pub fn small_default_for_configured_ids(&self, configured: &[ProviderId]) -> &Model {
|
||||
if configured.is_empty() {
|
||||
return self.default_model();
|
||||
}
|
||||
let configured = configured
|
||||
let configured = self.canonical_provider_ids(configured);
|
||||
let mut fallback = None;
|
||||
for model in self
|
||||
.models
|
||||
.iter()
|
||||
.filter(|model| configured.contains(&model.provider))
|
||||
{
|
||||
if model.small_default {
|
||||
return model;
|
||||
}
|
||||
if model.default && fallback.is_none() {
|
||||
fallback = Some(model);
|
||||
}
|
||||
}
|
||||
fallback.unwrap_or_else(|| self.default_model())
|
||||
}
|
||||
|
||||
fn canonical_provider_ids(&self, provider_ids: &[ProviderId]) -> HashSet<ProviderId> {
|
||||
provider_ids
|
||||
.iter()
|
||||
.filter_map(|id| self.provider(id).map(|provider| provider.id.clone()))
|
||||
.collect::<HashSet<_>>();
|
||||
self.providers
|
||||
.iter()
|
||||
.filter(|provider| configured.contains(&provider.id))
|
||||
.find_map(|provider| self.small_default_for_provider(&provider.id))
|
||||
.unwrap_or_else(|| self.default_model())
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Probe model for a provider — the cheapest model suitable for
|
||||
|
|
@ -5064,7 +5079,7 @@ reasoning = false
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn small_default_for_provider_falls_back_to_provider_default_when_no_small_default_marked() {
|
||||
fn small_default_for_provider_returns_none_when_no_small_default_marked() {
|
||||
let layer = minimal_settings(
|
||||
r#"
|
||||
[providers.test]
|
||||
|
|
@ -5102,12 +5117,10 @@ reasoning = false
|
|||
);
|
||||
let catalog = Catalog::from_settings(&layer).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
assert!(
|
||||
catalog
|
||||
.small_default_for_provider(&ProviderId::new("test"))
|
||||
.unwrap()
|
||||
.id,
|
||||
"default_model"
|
||||
.is_none()
|
||||
);
|
||||
}
|
||||
|
||||
|
|
@ -5257,6 +5270,66 @@ reasoning = false
|
|||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn small_default_for_configured_ids_skips_provider_without_a_small_model() {
|
||||
let layer = minimal_settings(
|
||||
r#"
|
||||
[providers.low]
|
||||
display_name = "Low"
|
||||
adapter = "openai"
|
||||
agent_profile = "openai"
|
||||
priority = 10
|
||||
|
||||
[providers.high]
|
||||
display_name = "High"
|
||||
adapter = "openai"
|
||||
agent_profile = "openai"
|
||||
priority = 20
|
||||
|
||||
[models.low_small]
|
||||
provider = "low"
|
||||
display_name = "Low Small"
|
||||
family = "test"
|
||||
small_default = true
|
||||
|
||||
[models.low_small.limits]
|
||||
context_window = 1000
|
||||
|
||||
[models.low_small.features]
|
||||
tools = false
|
||||
vision = false
|
||||
reasoning = false
|
||||
|
||||
[models.high_default]
|
||||
provider = "high"
|
||||
display_name = "High Default"
|
||||
family = "test"
|
||||
default = true
|
||||
|
||||
[models.high_default.limits]
|
||||
context_window = 1000
|
||||
|
||||
[models.high_default.features]
|
||||
tools = false
|
||||
vision = false
|
||||
reasoning = false
|
||||
"#,
|
||||
);
|
||||
let catalog = Catalog::from_settings(&layer).unwrap();
|
||||
|
||||
// `high` outranks `low` but marks no small default, so selection moves
|
||||
// on rather than substituting `high_default`.
|
||||
assert_eq!(
|
||||
catalog
|
||||
.small_default_for_configured_ids(&[
|
||||
ProviderId::new("low"),
|
||||
ProviderId::new("high")
|
||||
])
|
||||
.id,
|
||||
"low_small"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn small_default_for_configured_ids_falls_back_to_provider_default() {
|
||||
let layer = minimal_settings(
|
||||
|
|
@ -5365,10 +5438,7 @@ small_default = false
|
|||
.expect("sparse built-in model override should build");
|
||||
|
||||
let openai = ProviderId::openai();
|
||||
assert_eq!(
|
||||
catalog.small_default_for_provider(&openai).unwrap().id,
|
||||
catalog.default_for_provider(&openai).unwrap().id
|
||||
);
|
||||
assert!(catalog.small_default_for_provider(&openai).is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
|
|
|||
|
|
@ -97,6 +97,35 @@ impl TemplateContext {
|
|||
Self::new().with_goal("{{ goal }}").with_inputs(inputs)
|
||||
}
|
||||
|
||||
/// The rendered goal `{{ goal }}` resolves to, or `None` before the goal
|
||||
/// is known.
|
||||
#[must_use]
|
||||
pub fn goal(&self) -> Option<&str> {
|
||||
self.goal.as_deref()
|
||||
}
|
||||
|
||||
/// The text `{{ inputs.NAME }}` renders to, or `None` when unbound.
|
||||
///
|
||||
/// Non-template consumers — currently command node `script` interpolation,
|
||||
/// which uses `InterpString` tokens rather than MiniJinja — read values
|
||||
/// through here so an input produces the same text in a script as it does
|
||||
/// in a prompt.
|
||||
#[must_use]
|
||||
pub fn input(&self, name: &str) -> Option<String> {
|
||||
Self::rendered_member(&self.inputs, name)
|
||||
}
|
||||
|
||||
/// The text `{{ vars.NAME }}` renders to, or `None` when unset.
|
||||
#[must_use]
|
||||
pub fn var(&self, name: &str) -> Option<String> {
|
||||
Self::rendered_member(&self.vars, name)
|
||||
}
|
||||
|
||||
fn rendered_member(container: &Value, name: &str) -> Option<String> {
|
||||
let member = container.get_attr(name).ok()?;
|
||||
(!member.is_undefined()).then(|| member.to_string())
|
||||
}
|
||||
|
||||
fn into_value(self) -> Value {
|
||||
let goal = self.goal.map(Value::from);
|
||||
let inputs = self.inputs;
|
||||
|
|
@ -786,6 +815,53 @@ mod tests {
|
|||
assert_eq!(rendered, "Goal: Fix bugs");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn workflow_values_return_none_when_unbound() {
|
||||
let ctx = TemplateContext::new();
|
||||
|
||||
assert_eq!(ctx.goal(), None);
|
||||
assert_eq!(ctx.input("missing"), None);
|
||||
assert_eq!(ctx.var("MISSING"), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn goal_returns_the_rendered_value() {
|
||||
let ctx = TemplateContext::new().with_goal("Ship it");
|
||||
|
||||
assert_eq!(ctx.goal(), Some("Ship it"));
|
||||
}
|
||||
|
||||
/// `input`/`var` exist so non-template consumers render a value the same
|
||||
/// way a prompt does. Pin that equivalence across the scalar TOML types.
|
||||
#[test]
|
||||
fn input_matches_what_a_template_renders() {
|
||||
let ctx = TemplateContext::new().with_inputs(HashMap::from([
|
||||
("name".to_string(), toml::Value::String("fabro".into())),
|
||||
("attempts".to_string(), toml::Value::Integer(3)),
|
||||
("ratio".to_string(), toml::Value::Float(1.5)),
|
||||
("fast".to_string(), toml::Value::Boolean(true)),
|
||||
]));
|
||||
|
||||
for name in ["name", "attempts", "ratio", "fast"] {
|
||||
let rendered = render(&format!("{{{{ inputs.{name} }}}}"), &ctx).unwrap();
|
||||
assert_eq!(ctx.input(name), Some(rendered), "mismatch for `{name}`");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn var_matches_what_a_template_renders() {
|
||||
let ctx = TemplateContext::new().with_vars(HashMap::from([(
|
||||
"STAGE".to_string(),
|
||||
"staging".to_string(),
|
||||
)]));
|
||||
|
||||
assert_eq!(ctx.var("STAGE").as_deref(), Some("staging"));
|
||||
assert_eq!(
|
||||
ctx.var("STAGE"),
|
||||
Some(render("{{ vars.STAGE }}", &ctx).unwrap())
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn references_top_level_variable_detects_goal_self_reference() {
|
||||
assert!(references_top_level_variable("Do {{ goal }} now", "goal"));
|
||||
|
|
|
|||
|
|
@ -2,26 +2,29 @@
|
|||
//!
|
||||
//! An [`InterpString`] field may contain narrow `{{ <namespace>.NAME }}`
|
||||
//! tokens — no template logic. Which [`Namespace`]s resolve is
|
||||
//! scope-determined by the caller through [`ResolveCtx`]. Run-scope settings
|
||||
//! provide `vars` during run creation and `secrets` at consumption time. A
|
||||
//! token whose namespace has no lookup in the resolution context fails loudly
|
||||
//! rather than passing through as literal text.
|
||||
//! scope-determined by the caller through [`ResolveCtx`]. Run settings provide
|
||||
//! `vars` during run creation and `secrets` at consumption time. Workflow
|
||||
//! rendering additionally provides `inputs` and the bare `goal` value for
|
||||
//! supported fields. A token whose namespace has no lookup in the resolution
|
||||
//! context fails loudly rather than passing through as literal text.
|
||||
//!
|
||||
//! Two namespaces parse but have no [`ResolveCtx`] lookup. `inputs` is handled
|
||||
//! by the workflow template layer, not general config interpolation. `env`
|
||||
//! resolves nowhere: the process environment is not a configuration source.
|
||||
//! Use `{{ vars.NAME }}` for non-sensitive server-owned values or
|
||||
//! `{{ secrets.NAME }}` for vault-backed values. Keeping both namespaces
|
||||
//! parseable lets an out-of-scope token fail with a useful message instead of
|
||||
//! reaching a consumer as literal text.
|
||||
//! Most call sites do not wire `inputs` or `goal`. They are bound where the
|
||||
//! run's typed values are in scope, which is the workflow graph rather than
|
||||
//! general config. Elsewhere their tokens still parse, so they fail with a
|
||||
//! clear message instead of reaching a consumer as literal text.
|
||||
//!
|
||||
//! Resolution timing is split: `vars` substitute early (server-side, at run
|
||||
//! creation) via [`InterpString::substitute_with`], while `secrets` resolve
|
||||
//! late, at consumption time in the process that owns the value, via
|
||||
//! [`InterpString::resolve_with`]. Resolved secret values are plain strings;
|
||||
//! sensitivity is not tracked. Redaction of run output is content-based
|
||||
//! (entropy analysis plus credential patterns), applied where output is
|
||||
//! serialized.
|
||||
//! `env` parses but has no [`ResolveCtx`] lookup. The process environment is
|
||||
//! not a configuration source. Use `{{ vars.NAME }}` for non-sensitive
|
||||
//! server-owned values or `{{ secrets.NAME }}` for vault-backed values.
|
||||
//! Keeping the namespace parseable lets it fail with a useful migration
|
||||
//! message.
|
||||
//!
|
||||
//! Resolution timing is split. `vars` substitute during run creation, and
|
||||
//! `inputs` and `goal` substitute during workflow rendering. `secrets` resolve
|
||||
//! late, at consumption time in the process that owns the value. Resolved
|
||||
//! secret values are plain strings; sensitivity is not tracked. Redaction of
|
||||
//! run output is content-based (entropy analysis plus credential patterns),
|
||||
//! applied where output is serialized.
|
||||
|
||||
use std::borrow::Cow;
|
||||
use std::fmt;
|
||||
|
|
@ -32,7 +35,7 @@ use serde::{Deserialize, Deserializer, Serialize, Serializer};
|
|||
use crate::variable::is_env_style_name;
|
||||
|
||||
/// A config string that may contain `{{ env.NAME }}`, `{{ vars.NAME }}`,
|
||||
/// `{{ secrets.NAME }}`, or `{{ inputs.NAME }}` tokens.
|
||||
/// `{{ secrets.NAME }}`, `{{ inputs.NAME }}`, or `{{ goal }}` tokens.
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct InterpString {
|
||||
segments: Vec<Segment>,
|
||||
|
|
@ -47,6 +50,9 @@ enum Segment {
|
|||
},
|
||||
}
|
||||
|
||||
/// The sole token body with no `namespace.name` shape.
|
||||
const GOAL_TOKEN: &str = "goal";
|
||||
|
||||
/// The interpolation namespaces recognized inside `{{ ... }}` tokens.
|
||||
#[derive(
|
||||
Debug, Clone, Copy, PartialEq, Eq, Hash, strum::Display, strum::EnumString, strum::IntoStaticStr,
|
||||
|
|
@ -62,6 +68,12 @@ pub enum Namespace {
|
|||
Secrets,
|
||||
/// `{{ inputs.NAME }}` — workflow run inputs, substituted early.
|
||||
Inputs,
|
||||
/// The bare `{{ goal }}` token — the run goal, substituted early.
|
||||
///
|
||||
/// Unlike the others this names a single value rather than a namespace of
|
||||
/// them, so it has no dotted form: only the exact body `goal` produces it,
|
||||
/// and `{{ goal.anything }}` stays literal text.
|
||||
Goal,
|
||||
}
|
||||
|
||||
impl Namespace {
|
||||
|
|
@ -72,15 +84,27 @@ impl Namespace {
|
|||
Self::Vars => "variable",
|
||||
Self::Secrets => "secret",
|
||||
Self::Inputs => "input",
|
||||
Self::Goal => "run goal",
|
||||
}
|
||||
}
|
||||
|
||||
/// Whether this namespace is written as a bare token rather than
|
||||
/// `namespace.name`.
|
||||
fn is_bare(self) -> bool {
|
||||
matches!(self, Self::Goal)
|
||||
}
|
||||
|
||||
/// Parse a trimmed `{{ ... }}` token body into a namespace + name, or
|
||||
/// `None` when the body is not a recognized token (it then stays literal).
|
||||
fn parse_token(token: &str) -> Option<(Self, String)> {
|
||||
let trimmed = token.trim();
|
||||
if trimmed == GOAL_TOKEN {
|
||||
return Some((Self::Goal, GOAL_TOKEN.to_owned()));
|
||||
}
|
||||
let (prefix, name) = trimmed.split_once('.')?;
|
||||
let namespace = prefix.parse::<Self>().ok()?;
|
||||
// A bare namespace has no dotted spelling, so `{{ goal.title }}` is not
|
||||
// a token and reaches the consumer as literal text.
|
||||
let namespace = prefix.parse::<Self>().ok().filter(|ns| !ns.is_bare())?;
|
||||
namespace
|
||||
.is_valid_name(name)
|
||||
.then(|| (namespace, name.to_owned()))
|
||||
|
|
@ -107,6 +131,7 @@ impl Namespace {
|
|||
}
|
||||
chars.all(|ch| ch.is_ascii_alphanumeric() || ch == '_' || ch == '-')
|
||||
}
|
||||
Self::Goal => name == GOAL_TOKEN,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -122,6 +147,8 @@ impl Namespace {
|
|||
pub struct ResolveCtx<'a> {
|
||||
vars: Option<LookupFn<'a>>,
|
||||
secrets: Option<LookupFn<'a>>,
|
||||
inputs: Option<LookupFn<'a>>,
|
||||
goal: Option<LookupFn<'a>>,
|
||||
}
|
||||
|
||||
type LookupFn<'a> = Box<dyn FnMut(&str) -> Option<String> + 'a>;
|
||||
|
|
@ -144,17 +171,40 @@ impl<'a> ResolveCtx<'a> {
|
|||
self
|
||||
}
|
||||
|
||||
/// Typed `[run.inputs]` values, available only where a run's inputs are in
|
||||
/// scope. Leaving this unwired — which every config-layer call site does —
|
||||
/// keeps `{{ inputs.* }}` unavailable, so `resolve_with` fails loudly and
|
||||
/// `substitute_with` preserves the token for a goal (an `InterpString`
|
||||
/// that feeds a template) to forward to the template layer.
|
||||
#[must_use]
|
||||
pub fn with_inputs(mut self, lookup: impl FnMut(&str) -> Option<String> + 'a) -> Self {
|
||||
self.inputs = Some(Box::new(lookup));
|
||||
self
|
||||
}
|
||||
|
||||
/// The rendered run goal, available only where a run's goal is in scope.
|
||||
/// Like [`ResolveCtx::with_inputs`], leaving it unwired keeps
|
||||
/// `{{ goal }}` unavailable for that call site.
|
||||
///
|
||||
/// Takes the value rather than a lookup: unlike a namespace there is
|
||||
/// nothing to look up by name, so a wired goal can never be `Missing`.
|
||||
#[must_use]
|
||||
pub fn with_goal(mut self, goal: impl Into<String>) -> Self {
|
||||
let goal = goal.into();
|
||||
self.goal = Some(Box::new(move |_| Some(goal.clone())));
|
||||
self
|
||||
}
|
||||
|
||||
fn lookup_for(&mut self, namespace: Namespace) -> Option<&mut LookupFn<'a>> {
|
||||
match namespace {
|
||||
// The process environment is not a configuration source. Keep the
|
||||
// namespace parseable so full resolution reports the migration
|
||||
// error instead of passing the token through as literal text.
|
||||
Namespace::Env => None,
|
||||
Namespace::Vars => self.vars.as_mut(),
|
||||
Namespace::Secrets => self.secrets.as_mut(),
|
||||
// Neither is ever wired. `env` has no lookup because the process
|
||||
// environment is not a configuration source; `inputs` is
|
||||
// template-only. Both variants exist so the token fails with a
|
||||
// message naming where the value belongs, and `substitute_with`
|
||||
// still preserves them so a goal (an `InterpString` that feeds a
|
||||
// template) can forward `{{ inputs.* }}` to the template layer.
|
||||
Namespace::Env | Namespace::Inputs => None,
|
||||
Namespace::Inputs => self.inputs.as_mut(),
|
||||
Namespace::Goal => self.goal.as_mut(),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -266,9 +316,11 @@ impl InterpString {
|
|||
Segment::Literal(text) => out.push_str(text),
|
||||
Segment::Token { namespace, name } => {
|
||||
out.push_str("{{ ");
|
||||
out.push_str(namespace.into());
|
||||
out.push('.');
|
||||
out.push_str(name);
|
||||
out.push_str((*namespace).into());
|
||||
if !namespace.is_bare() {
|
||||
out.push('.');
|
||||
out.push_str(name);
|
||||
}
|
||||
out.push_str(" }}");
|
||||
}
|
||||
}
|
||||
|
|
@ -308,10 +360,11 @@ impl InterpString {
|
|||
/// for the namespaces it does not — their resolution happens later,
|
||||
/// possibly in a different process.
|
||||
///
|
||||
/// This is the early, server-side `vars` pass. `secrets` survive in token
|
||||
/// form for consumption-time [`InterpString::resolve_with`]. Unsupported
|
||||
/// `env` tokens and template-only `inputs` tokens also remain so the next
|
||||
/// full-resolution boundary can reject or route them explicitly.
|
||||
/// Callers substitute the namespaces available at their boundary: `vars`
|
||||
/// during run creation, then `inputs` and `goal` during workflow rendering.
|
||||
/// `secrets` survive for consumption-time [`InterpString::resolve_with`].
|
||||
/// Unsupported `env` tokens also survive so the next full-resolution
|
||||
/// boundary can reject them with a migration message.
|
||||
pub fn substitute_with(&self, ctx: &mut ResolveCtx<'_>) -> Result<Self, ResolveError> {
|
||||
let mut segments = Vec::new();
|
||||
for seg in &self.segments {
|
||||
|
|
@ -434,14 +487,19 @@ impl fmt::Display for ResolveError {
|
|||
self.name, self.name
|
||||
),
|
||||
ResolveErrorKind::Unavailable => match namespace {
|
||||
// `inputs` is template-only: it never resolves in an
|
||||
// `InterpString` field. Point the user at where it works.
|
||||
// `inputs` and `goal` resolve only where a run's values are in
|
||||
// scope. Point the user at where that is.
|
||||
Namespace::Inputs => write!(
|
||||
f,
|
||||
"{{{{ inputs.{} }}}} is only available in prompts and goals, not in other \
|
||||
config fields",
|
||||
"{{{{ inputs.{} }}}} is only available in prompts, goals, and command node \
|
||||
`script` attributes, not in other config fields",
|
||||
self.name
|
||||
),
|
||||
Namespace::Goal => write!(
|
||||
f,
|
||||
"{{{{ goal }}}} is only available in prompts and command node `script` \
|
||||
attributes, not in other config fields"
|
||||
),
|
||||
// `env` resolves nowhere. Name the replacement rather than
|
||||
// reporting a generic out-of-scope error.
|
||||
Namespace::Env => write!(
|
||||
|
|
@ -485,7 +543,7 @@ impl<'de> Deserialize<'de> for InterpString {
|
|||
fn expecting(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
f.write_str(
|
||||
"a string, optionally containing {{ env.NAME }}, {{ vars.NAME }}, \
|
||||
{{ secrets.NAME }}, or {{ inputs.NAME }} interpolation tokens",
|
||||
{{ secrets.NAME }}, {{ inputs.NAME }}, or {{ goal }} interpolation tokens",
|
||||
)
|
||||
}
|
||||
|
||||
|
|
@ -648,7 +706,8 @@ mod tests {
|
|||
s: InterpString,
|
||||
}
|
||||
|
||||
let input = r#"{"s":"{{ env.A }}/{{ vars.B }}/{{ secrets.C }}/{{ inputs.d-key }}"}"#;
|
||||
let input =
|
||||
r#"{"s":"{{ env.A }}/{{ vars.B }}/{{ secrets.C }}/{{ inputs.d-key }}/{{ goal }}"}"#;
|
||||
let parsed: Wrap = serde_json::from_str(input).unwrap();
|
||||
let rendered = serde_json::to_string(&parsed).unwrap();
|
||||
assert_eq!(rendered, input);
|
||||
|
|
@ -741,10 +800,9 @@ mod tests {
|
|||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_with_rejects_inputs_as_template_only() {
|
||||
// `inputs` is template-only. An `{{ inputs.* }}` token never resolves
|
||||
// in an `InterpString` field — it fails loudly, pointing the
|
||||
// user at prompts and goals.
|
||||
fn resolve_with_rejects_inputs_when_not_wired() {
|
||||
// A context that does not opt into `inputs` fails loudly, pointing the
|
||||
// user at the scopes where inputs do resolve.
|
||||
let s = InterpString::parse("run-{{ inputs.ticket-id }}");
|
||||
|
||||
let err = s.resolve_with(&mut ResolveCtx::new()).unwrap_err();
|
||||
|
|
@ -752,12 +810,115 @@ mod tests {
|
|||
assert_eq!(err.namespace, Namespace::Inputs);
|
||||
assert_eq!(err.kind, ResolveErrorKind::Unavailable);
|
||||
assert!(
|
||||
err.to_string()
|
||||
.contains("only available in prompts and goals"),
|
||||
err.to_string().contains("only available in prompts, goals"),
|
||||
"unexpected message: {err}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_with_resolves_inputs_when_wired() {
|
||||
let s = InterpString::parse("run-{{ inputs.ticket-id }}-{{ vars.STAGE }}");
|
||||
|
||||
let resolved = s
|
||||
.resolve_with(
|
||||
&mut ResolveCtx::new()
|
||||
.with_inputs(lookup_from(&[("ticket-id", "4821")]))
|
||||
.with_vars(lookup_from(&[("STAGE", "staging")])),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(resolved, "run-4821-staging");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_with_resolves_the_bare_goal_token_when_wired() {
|
||||
let s = InterpString::parse("gh pr create --title \"{{ goal }}\"");
|
||||
|
||||
let resolved = s
|
||||
.resolve_with(&mut ResolveCtx::new().with_goal("Fix the login bug"))
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(resolved, "gh pr create --title \"Fix the login bug\"");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_with_rejects_the_goal_token_when_not_wired() {
|
||||
let s = InterpString::parse("echo {{ goal }}");
|
||||
|
||||
let err = s.resolve_with(&mut ResolveCtx::new()).unwrap_err();
|
||||
|
||||
assert_eq!(err.namespace, Namespace::Goal);
|
||||
assert_eq!(err.kind, ResolveErrorKind::Unavailable);
|
||||
assert!(
|
||||
err.to_string().contains("{{ goal }} is only available"),
|
||||
"unexpected message: {err}"
|
||||
);
|
||||
}
|
||||
|
||||
/// `goal` names a single value, so it has no dotted spelling. Anything of
|
||||
/// the form `{{ goal.x }}` must stay literal rather than becoming a token
|
||||
/// that could never resolve.
|
||||
#[test]
|
||||
fn goal_has_no_dotted_form() {
|
||||
for source in ["{{ goal.title }}", "{{ goal. }}", "{{ goals }}"] {
|
||||
let s = InterpString::parse(source);
|
||||
let resolved = s
|
||||
.resolve_with(&mut ResolveCtx::new().with_goal("Ship it"))
|
||||
.unwrap_or_else(|err| panic!("`{source}` should stay literal, got: {err}"));
|
||||
assert_eq!(resolved, source);
|
||||
}
|
||||
}
|
||||
|
||||
/// Substituted text is output, not more input. A resolved value that
|
||||
/// happens to contain token syntax must land verbatim rather than being
|
||||
/// scanned again — otherwise a goal or input could smuggle in a token the
|
||||
/// call site never wired.
|
||||
#[test]
|
||||
fn resolve_with_does_not_rescan_substituted_values() {
|
||||
let s = InterpString::parse("{{ goal }} | {{ vars.PAYLOAD }}");
|
||||
|
||||
let resolved = s
|
||||
.resolve_with(
|
||||
&mut ResolveCtx::new()
|
||||
.with_goal("{{ secrets.API_KEY }}")
|
||||
.with_vars(lookup_from(&[("PAYLOAD", "{{ env.HOME }}")])),
|
||||
)
|
||||
.unwrap();
|
||||
|
||||
assert_eq!(resolved, "{{ secrets.API_KEY }} | {{ env.HOME }}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn goal_token_round_trips_through_source_form() {
|
||||
// Unwired, `substitute_with` preserves the token; `as_source` must
|
||||
// reproduce the bare spelling rather than `{{ goal.goal }}`.
|
||||
let s = InterpString::parse("deploy # {{ goal }}");
|
||||
|
||||
let preserved = s.substitute_with(&mut ResolveCtx::new()).unwrap();
|
||||
|
||||
#[expect(clippy::disallowed_methods, reason = "asserting the source round-trip")]
|
||||
let source = preserved.as_source();
|
||||
assert_eq!(source, "deploy # {{ goal }}");
|
||||
}
|
||||
|
||||
/// Wiring `inputs` at one call site must not make it resolvable anywhere
|
||||
/// else. Every config-layer context leaves it unwired, and this pins that
|
||||
/// the availability stays per-context rather than global.
|
||||
#[test]
|
||||
fn wiring_inputs_does_not_leak_into_other_contexts() {
|
||||
let s = InterpString::parse("{{ inputs.id }}");
|
||||
|
||||
let wired = s
|
||||
.resolve_with(&mut ResolveCtx::new().with_inputs(lookup_from(&[("id", "7")])))
|
||||
.unwrap();
|
||||
assert_eq!(wired, "7");
|
||||
|
||||
let err = s
|
||||
.resolve_with(&mut ResolveCtx::new().with_vars(lookup_from(&[("id", "7")])))
|
||||
.unwrap_err();
|
||||
assert_eq!(err.kind, ResolveErrorKind::Unavailable);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn substitute_variables_preserves_late_bound_tokens() {
|
||||
let s =
|
||||
|
|
@ -812,5 +973,6 @@ mod tests {
|
|||
assert_eq!(Namespace::Vars.to_string(), "vars");
|
||||
assert_eq!(Namespace::Secrets.to_string(), "secrets");
|
||||
assert_eq!(Namespace::Inputs.to_string(), "inputs");
|
||||
assert_eq!(Namespace::Goal.to_string(), "goal");
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue