mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-08 03:10:26 +00:00
Unify @file inlining under an ImportableTemplate type (prompt + goal) (#527)
## What
Introduces an `ImportableTemplate` type that unifies the "inline content
**or**
`@path` file import" concept used by node `prompt`s, the graph `goal`,
and
`output_schema`. This is the last template-side piece of the
interpolation
unification: a single named type now owns the `@`-classification and
static-reference validation that was previously hand-rolled in three
places.
This is a **behavior-preserving refactor** — no user-visible change.
## How
- New `ImportableTemplate { Inline(String), Import { path } }` in
`transforms/importable_template.rs`, with `parse` (classifies a value —
a
leading `@` marks a file import), `import_path`, and `validate` (rejects
template syntax in an import path). Callers of templated fields classify
the
**already-rendered** string, because a leading `@` can be produced by
rendering (e.g. `{{ inputs.prompt_file }}` → `@prompts/work.md`).
- `prompt` + `goal`: render the inline value, then — if it's an `@file`
import —
load and render the file contents via the type. The missing-file →
literal
passthrough is preserved.
- `output_schema`: shares the same classification but is loaded
**verbatim** (it
is intentionally not a template), keeping its hard-error-on-missing-file
behavior.
- Deletes the dead `resolve_file_ref` helper (no non-test callers) and
inlines
the trivial `render_file_contents` wrapper.
- Migrates the `FilesystemFileResolver` coverage (tilde, `..`,
fallback-dir
precedence, missing file) — which previously only existed through
`resolve_file_ref`'s tests — onto direct `file_resolver` tests.
`TemplateTransform` and the import transform are untouched, so
goal-before-
prompts ordering and the goal-self-reference guard are preserved
exactly.
## Scope
Covers the DOT node `prompt` + graph `goal` `@file` path. The
settings-layer
`run.goal` resolution is intentionally left as-is — it uses a different
model
(interpolates env into the file path and does not render file contents),
so
folding it in would be a semantic change, not a refactor. That
convergence can
be a deliberate follow-up.
## Testing
- `cargo nextest run -p fabro-workflow` — 1182 passed (31
e2e/credentialed
skipped). New unit tests on the type (classification, validation) and
the
migrated `FilesystemFileResolver` tests.
- Regression net kept green: file-inlining (prompt/goal, output_schema
verbatim/error/routing, `{% include %}` rooting, fallback dir), the
`TemplateTransform` goal/self-reference/ordering tests, and the
cross-pass
`reports_goal_self_reference_once_across_passes`.
- `cargo +nightly fmt --check --all` and nightly
`clippy --workspace --all-targets -- -D warnings` clean.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
8a7ad7200b
commit
ee7453418b
14 changed files with 360 additions and 306 deletions
|
|
@ -345,7 +345,7 @@ pub(crate) async fn execute(mut args: ExecArgs, ctx: &CommandContext) -> AnyResu
|
|||
// against the CLI process env — the mirror of the `fabro run` worker
|
||||
// boundary in `fabro_workflow::operations::start::runtime_mcp_server`.
|
||||
// Both consumers read the same source-form settings; missing env is a hard
|
||||
// error (D3) and reserved secrets/inputs tokens surface loudly rather than
|
||||
// error and reserved secrets/inputs tokens surface loudly rather than
|
||||
// leaking.
|
||||
let mcp_servers = mcp_servers
|
||||
.into_iter()
|
||||
|
|
|
|||
|
|
@ -154,7 +154,7 @@ fn resolve_git(git: Option<&RunGitLayer>) -> RunGitSettings {
|
|||
#[expect(
|
||||
clippy::disallowed_methods,
|
||||
reason = "known leak: prepare step templates collapse to raw source unresolved; strict \
|
||||
resolution scheduled in the interpolation unification (Phase 2)"
|
||||
resolution scheduled for follow-up interpolation cleanup"
|
||||
)]
|
||||
fn resolve_prepare(
|
||||
prepare: Option<&RunPrepareLayer>,
|
||||
|
|
|
|||
|
|
@ -647,7 +647,7 @@ name = "sonnet"
|
|||
}
|
||||
other => panic!("expected file goal, got {other:?}"),
|
||||
}
|
||||
// run.working_dir is demoted (D11): the env token stays literal text.
|
||||
// run.working_dir is demoted: the env token stays literal text.
|
||||
assert_eq!(
|
||||
settings.working_dir.as_deref(),
|
||||
Some("{{ env.FABRO_WORKDIR }}")
|
||||
|
|
|
|||
|
|
@ -13,7 +13,7 @@ use tokio::{fs, task};
|
|||
|
||||
use crate::git_checkout::{
|
||||
GitCheckoutError, GitRepoCache, WorktreePrepareInput, github_metadata_url,
|
||||
parse_github_repository_slug, resolve_git_auth_config,
|
||||
resolve_git_auth_config,
|
||||
};
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
|
|
@ -46,7 +46,6 @@ pub(crate) enum RunMaterializeError {
|
|||
impl From<GitCheckoutError> for RunMaterializeError {
|
||||
fn from(value: GitCheckoutError) -> Self {
|
||||
match value {
|
||||
GitCheckoutError::InvalidTarget(message) => Self::InvalidTarget(message),
|
||||
GitCheckoutError::CloneFailed(message) => Self::CloneFailed(message),
|
||||
}
|
||||
}
|
||||
|
|
@ -93,7 +92,7 @@ impl AutomationRunMaterializer for ProductionAutomationRunMaterializer {
|
|||
&self,
|
||||
input: AutomationRunMaterializeInput,
|
||||
) -> Result<AutomationRunMaterialized, RunMaterializeError> {
|
||||
let repo = parse_github_repository_slug(&input.target.repository)?;
|
||||
let repo = parse_target_repository(&input.target.repository)?;
|
||||
fs::create_dir_all(&input.temp_root).await.map_err(|err| {
|
||||
RunMaterializeError::CloneFailed(format!(
|
||||
"failed to create temp root {}: {err}",
|
||||
|
|
@ -157,6 +156,11 @@ fn render_error_chain(error: &(dyn std::error::Error + 'static)) -> String {
|
|||
collect_chain(error).join(": ")
|
||||
}
|
||||
|
||||
fn parse_target_repository(value: &str) -> Result<GitHubRepositorySlug, RunMaterializeError> {
|
||||
fabro_automation::parse_github_repository_slug(value)
|
||||
.map_err(|err| RunMaterializeError::InvalidTarget(err.to_string()))
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
pub(crate) struct ManifestFromCheckoutInput {
|
||||
workflow: String,
|
||||
|
|
@ -329,7 +333,7 @@ mod tests {
|
|||
let user_settings_path = temp.path().join("settings.toml");
|
||||
fs::write(&user_settings_path, "_version = 1\n").unwrap();
|
||||
let run_id = RunId::new();
|
||||
let repo = parse_github_repository_slug("workspace-org/app").unwrap();
|
||||
let repo = parse_target_repository("workspace-org/app").unwrap();
|
||||
let sha = "0123456789abcdef0123456789abcdef01234567".to_string();
|
||||
|
||||
let materialized = build_manifest_from_checkout(ManifestFromCheckoutInput {
|
||||
|
|
|
|||
|
|
@ -17,8 +17,6 @@ const GIT_REV_PARSE_TIMEOUT: Duration = Duration::from_secs(10);
|
|||
/// Error returned while preparing a checkout from a git source.
|
||||
#[derive(thiserror::Error, Debug, Clone, PartialEq, Eq)]
|
||||
pub(crate) enum GitCheckoutError {
|
||||
#[error("invalid repository target: {0}")]
|
||||
InvalidTarget(String),
|
||||
#[error("failed to clone repository: {0}")]
|
||||
CloneFailed(String),
|
||||
}
|
||||
|
|
@ -58,9 +56,9 @@ impl GitRepoCache {
|
|||
/// `<cache>/<owner>/<repo>.git`. Subsequent calls reuse the bare clone
|
||||
/// and only `git fetch --depth 1` the requested ref. In both cases a
|
||||
/// short-lived worktree is added at `worktree_dir`; the caller owns its
|
||||
/// lifetime (typically a `TempDir`). Stale worktree admin entries from
|
||||
/// crashed prior calls are pruned at the start of each call so they do
|
||||
/// not accumulate.
|
||||
/// lifetime (typically a `TempDir`). If stale worktree admin entries from
|
||||
/// crashed prior calls block the add, the cache prunes those entries and
|
||||
/// retries once.
|
||||
pub(crate) async fn prepare_worktree(
|
||||
&self,
|
||||
args: WorktreePrepareInput<'_>,
|
||||
|
|
@ -87,7 +85,14 @@ impl GitRepoCache {
|
|||
// and network failures don't trip this branch because
|
||||
// `bare_clone_may_be_corrupt` only returns true when the
|
||||
// bare repo's `HEAD` file is missing or empty.
|
||||
let _ = fs::remove_dir_all(&bare_dir).await;
|
||||
if let Err(remove_err) = fs::remove_dir_all(&bare_dir).await {
|
||||
tracing::warn!(
|
||||
path = %bare_dir.display(),
|
||||
error = %remove_err,
|
||||
"failed to remove corrupt cached git repository"
|
||||
);
|
||||
return Err(first_err);
|
||||
}
|
||||
self.try_prepare_worktree(&bare_dir, &args, clone_url)
|
||||
.await
|
||||
.map_err(|_| first_err)
|
||||
|
|
@ -105,11 +110,7 @@ impl GitRepoCache {
|
|||
let bare_exists = fs::try_exists(&bare_dir.join("HEAD"))
|
||||
.await
|
||||
.unwrap_or(false);
|
||||
if bare_exists {
|
||||
// Drop any worktree admin entries whose working trees were
|
||||
// deleted by previous `TempDir` cleanup. Cheap and idempotent.
|
||||
let _ = run_git_plan(build_worktree_prune_plan(bare_dir)).await;
|
||||
} else {
|
||||
if !bare_exists {
|
||||
if let Some(parent) = bare_dir.parent() {
|
||||
fs::create_dir_all(parent).await.map_err(|err| {
|
||||
GitCheckoutError::CloneFailed(format!(
|
||||
|
|
@ -133,7 +134,7 @@ impl GitRepoCache {
|
|||
.await
|
||||
.map(|stdout| String::from_utf8_lossy(&stdout).trim().to_string())?;
|
||||
|
||||
run_git_plan(build_worktree_add_plan(bare_dir, args.worktree_dir)).await?;
|
||||
add_worktree_with_stale_retry(bare_dir, args.worktree_dir, &checked_out_sha).await?;
|
||||
|
||||
Ok(checked_out_sha)
|
||||
}
|
||||
|
|
@ -149,17 +150,20 @@ pub(crate) struct WorktreePrepareInput<'a> {
|
|||
async fn bare_clone_may_be_corrupt(bare_dir: &Path) -> bool {
|
||||
match fs::metadata(&bare_dir.join("HEAD")).await {
|
||||
Ok(meta) => meta.len() == 0,
|
||||
Err(_) => bare_dir.exists(),
|
||||
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
|
||||
fs::try_exists(bare_dir).await.unwrap_or(false)
|
||||
}
|
||||
Err(err) => {
|
||||
tracing::warn!(
|
||||
path = %bare_dir.join("HEAD").display(),
|
||||
%err,
|
||||
"Failed to inspect cached git repository HEAD"
|
||||
);
|
||||
false
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn parse_github_repository_slug(
|
||||
value: &str,
|
||||
) -> Result<GitHubRepositorySlug, GitCheckoutError> {
|
||||
fabro_automation::parse_github_repository_slug(value)
|
||||
.map_err(|err| GitCheckoutError::InvalidTarget(err.to_string()))
|
||||
}
|
||||
|
||||
fn github_clone_url(repo: &GitHubRepositorySlug) -> String {
|
||||
format!("https://github.com/{}/{}.git", repo.owner(), repo.repo())
|
||||
}
|
||||
|
|
@ -326,7 +330,7 @@ fn build_bare_fetch_plan(
|
|||
.with_auth(clone_url, auth)
|
||||
}
|
||||
|
||||
fn build_worktree_add_plan(bare_dir: &Path, worktree_dir: &Path) -> GitCommandPlan {
|
||||
fn build_worktree_add_plan(bare_dir: &Path, worktree_dir: &Path, target: &str) -> GitCommandPlan {
|
||||
GitCommandPlan::new(
|
||||
[
|
||||
"worktree".to_string(),
|
||||
|
|
@ -334,7 +338,7 @@ fn build_worktree_add_plan(bare_dir: &Path, worktree_dir: &Path) -> GitCommandPl
|
|||
"--detach".to_string(),
|
||||
"--force".to_string(),
|
||||
worktree_dir.display().to_string(),
|
||||
"FETCH_HEAD".to_string(),
|
||||
target.to_string(),
|
||||
],
|
||||
GIT_WORKTREE_ADD_TIMEOUT,
|
||||
)
|
||||
|
|
@ -349,6 +353,34 @@ fn build_rev_parse_fetch_head_plan(bare_dir: &Path) -> GitCommandPlan {
|
|||
GitCommandPlan::new(["rev-parse", "FETCH_HEAD"], GIT_REV_PARSE_TIMEOUT).current_dir(bare_dir)
|
||||
}
|
||||
|
||||
async fn add_worktree_with_stale_retry(
|
||||
bare_dir: &Path,
|
||||
worktree_dir: &Path,
|
||||
target: &str,
|
||||
) -> Result<(), GitCheckoutError> {
|
||||
match run_git_plan(build_worktree_add_plan(bare_dir, worktree_dir, target)).await {
|
||||
Ok(_) => Ok(()),
|
||||
Err(first_err) => {
|
||||
tracing::warn!(
|
||||
%first_err,
|
||||
bare_dir = %bare_dir.display(),
|
||||
worktree_dir = %worktree_dir.display(),
|
||||
"git worktree add failed; pruning stale worktree entries and retrying"
|
||||
);
|
||||
if let Err(prune_err) = run_git_plan(build_worktree_prune_plan(bare_dir)).await {
|
||||
tracing::warn!(
|
||||
%prune_err,
|
||||
bare_dir = %bare_dir.display(),
|
||||
"failed to prune stale git worktree entries"
|
||||
);
|
||||
}
|
||||
run_git_plan(build_worktree_add_plan(bare_dir, worktree_dir, target))
|
||||
.await
|
||||
.map(|_| ())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async fn run_git_plan(plan: GitCommandPlan) -> Result<Vec<u8>, GitCheckoutError> {
|
||||
let mut command = Command::new(&plan.program);
|
||||
command.args(&plan.args);
|
||||
|
|
@ -432,9 +464,13 @@ mod tests {
|
|||
|
||||
use super::*;
|
||||
|
||||
fn repository_slug(value: &str) -> GitHubRepositorySlug {
|
||||
fabro_automation::parse_github_repository_slug(value).expect("slug should parse")
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn target_repository_urls_are_github_metadata_urls_without_credentials() {
|
||||
let repo = parse_github_repository_slug("fabro-sh/fabro").expect("slug should parse");
|
||||
let repo = repository_slug("fabro-sh/fabro");
|
||||
|
||||
assert_eq!(repo.owner(), "fabro-sh");
|
||||
assert_eq!(repo.repo(), "fabro");
|
||||
|
|
@ -449,25 +485,9 @@ mod tests {
|
|||
assert!(!github_clone_url(&repo).contains('@'));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn target_repository_validation_rejects_non_github_owner_repo_shapes() {
|
||||
for value in [
|
||||
"fabro-sh",
|
||||
"https://github.com/fabro-sh/fabro",
|
||||
"fabro-sh/fabro/extra",
|
||||
"-owner/repo",
|
||||
] {
|
||||
let error = parse_github_repository_slug(value).expect_err("invalid slug should fail");
|
||||
assert!(
|
||||
error.to_string().contains("invalid repository target"),
|
||||
"unexpected error for {value}: {error}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn target_repository_validation_matches_automation_validation() {
|
||||
let repo = parse_github_repository_slug("owner/.github").expect("slug should parse");
|
||||
let repo = repository_slug("owner/.github");
|
||||
|
||||
assert_eq!(repo.owner(), "owner");
|
||||
assert_eq!(repo.repo(), ".github");
|
||||
|
|
@ -479,7 +499,7 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn bare_cache_command_plans_use_argv_prompt_disable_and_timeouts() {
|
||||
let repo = parse_github_repository_slug("fabro-sh/fabro").unwrap();
|
||||
let repo = repository_slug("fabro-sh/fabro");
|
||||
let clone_url = github_clone_url(&repo);
|
||||
let temp = TempDir::new().unwrap();
|
||||
let bare_dir = temp.path().join("fabro-sh/fabro.git");
|
||||
|
|
@ -511,14 +531,18 @@ mod tests {
|
|||
assert_eq!(fetch.timeout, Duration::from_mins(1));
|
||||
assert_eq!(fetch.env_value("GIT_TERMINAL_PROMPT"), Some("0"));
|
||||
|
||||
let worktree = build_worktree_add_plan(&bare_dir, &worktree_dir);
|
||||
let worktree = build_worktree_add_plan(
|
||||
&bare_dir,
|
||||
&worktree_dir,
|
||||
"0123456789abcdef0123456789abcdef01234567",
|
||||
);
|
||||
assert_eq!(worktree.args, vec![
|
||||
"worktree",
|
||||
"add",
|
||||
"--detach",
|
||||
"--force",
|
||||
worktree_dir.to_str().unwrap(),
|
||||
"FETCH_HEAD",
|
||||
"0123456789abcdef0123456789abcdef01234567",
|
||||
]);
|
||||
assert_eq!(worktree.current_dir.as_deref(), Some(bare_dir.as_path()));
|
||||
assert_eq!(worktree.timeout, Duration::from_secs(30));
|
||||
|
|
@ -563,7 +587,7 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn credential_config_env_keeps_clone_url_uncredentialed() {
|
||||
let repo = parse_github_repository_slug("fabro-sh/fabro").unwrap();
|
||||
let repo = repository_slug("fabro-sh/fabro");
|
||||
let clone_url = github_clone_url(&repo);
|
||||
let auth = GitAuthConfig::new(
|
||||
Some("x-access-token".to_string()),
|
||||
|
|
@ -671,7 +695,7 @@ mod tests {
|
|||
let upstream = temp.path().join("upstream.git");
|
||||
let expected_sha = seed_upstream(&upstream);
|
||||
let cache = GitRepoCache::new(temp.path().join("cache"));
|
||||
let repo = parse_github_repository_slug("fabro-sh/fabro").unwrap();
|
||||
let repo = repository_slug("fabro-sh/fabro");
|
||||
let upstream_url = upstream.to_str().unwrap().to_string();
|
||||
|
||||
let worktree_a = temp.path().join("wt-a");
|
||||
|
|
@ -723,7 +747,7 @@ mod tests {
|
|||
let upstream = temp.path().join("upstream.git");
|
||||
let expected_sha = seed_upstream(&upstream);
|
||||
let cache = GitRepoCache::new(temp.path().join("cache"));
|
||||
let repo = parse_github_repository_slug("fabro-sh/fabro").unwrap();
|
||||
let repo = repository_slug("fabro-sh/fabro");
|
||||
let upstream_url = upstream.to_str().unwrap().to_string();
|
||||
|
||||
let worktree_a = temp.path().join("wt-a");
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@
|
|||
//!
|
||||
//! An [`InterpString`] field may contain narrow `{{ <namespace>.NAME }}`
|
||||
//! tokens — no template logic. Three [`Namespace`]s resolve here: `env`,
|
||||
//! `vars`, and `secrets`. `inputs` is **template-only** (D12): it is a
|
||||
//! `vars`, and `secrets`. `inputs` is **template-only**: it is a
|
||||
//! recognized namespace so an `{{ inputs.* }}` token fails loudly with a clear
|
||||
//! message instead of passing through as literal text, but it never resolves
|
||||
//! in an `InterpString` field — it belongs in prompts and goals. Which of the
|
||||
|
|
@ -149,7 +149,7 @@ impl<'a> ResolveCtx<'a> {
|
|||
Namespace::Env => self.env.as_mut(),
|
||||
Namespace::Vars => self.vars.as_mut(),
|
||||
Namespace::Secrets => self.secrets.as_mut(),
|
||||
// `inputs` is template-only (D12): an `InterpString` resolve context
|
||||
// `inputs` is template-only: an `InterpString` resolve context
|
||||
// never provides it, so an `{{ inputs.* }}` token is always
|
||||
// unavailable here. `substitute_with` still preserves the token so a
|
||||
// goal (an `InterpString` that feeds a template) can forward it.
|
||||
|
|
@ -361,7 +361,7 @@ impl InterpString {
|
|||
clippy::disallowed_methods,
|
||||
reason = "intentional raw-source fallback so a missing env var surfaces as a \
|
||||
recognizable diagnostic; slated for hard-error semantics in the \
|
||||
interpolation unification (D3)"
|
||||
interpolation cleanup"
|
||||
)]
|
||||
#[must_use]
|
||||
pub fn resolve_or_source<F>(&self, lookup: F) -> String
|
||||
|
|
@ -506,7 +506,7 @@ impl fmt::Display for ResolveError {
|
|||
self.name, self.name
|
||||
),
|
||||
ResolveErrorKind::Unavailable => match namespace {
|
||||
// `inputs` is template-only (D12): it never resolves in an
|
||||
// `inputs` is template-only: it never resolves in an
|
||||
// `InterpString` field. Point the user at where it works.
|
||||
Namespace::Inputs => write!(
|
||||
f,
|
||||
|
|
@ -816,8 +816,8 @@ mod tests {
|
|||
|
||||
#[test]
|
||||
fn resolve_with_rejects_inputs_as_template_only() {
|
||||
// D12: `inputs` is template-only. An `{{ inputs.* }}` token never
|
||||
// resolves in an `InterpString` field — it fails loudly, pointing the
|
||||
// `inputs` is template-only. An `{{ inputs.* }}` token never resolves
|
||||
// in an `InterpString` field — it fails loudly, pointing the
|
||||
// user at prompts and goals.
|
||||
let s = InterpString::parse("run-{{ inputs.ticket-id }}");
|
||||
|
||||
|
|
|
|||
|
|
@ -181,4 +181,109 @@ mod tests {
|
|||
|
||||
assert_eq!(resolved.content, "prompt content");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filesystem_resolver_reads_existing_file() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
std::fs::write(dir.path().join("prompt.md"), "inlined content").unwrap();
|
||||
|
||||
let resolved = FilesystemFileResolver::new(None)
|
||||
.resolve(dir.path(), "prompt.md")
|
||||
.expect("file should resolve");
|
||||
|
||||
assert_eq!(resolved.content, "inlined content");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filesystem_resolver_returns_none_for_missing_file() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
|
||||
assert!(
|
||||
FilesystemFileResolver::new(None)
|
||||
.resolve(dir.path(), "nonexistent.md")
|
||||
.is_none()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filesystem_resolver_expands_tilde() {
|
||||
let home = dirs::home_dir().expect("home dir must exist");
|
||||
let test_file = home.join(".fabro_test_tilde_tmp");
|
||||
std::fs::write(&test_file, "tilde content").unwrap();
|
||||
let _cleanup = scopeguard::guard((), |()| {
|
||||
let _ = std::fs::remove_file(&test_file);
|
||||
});
|
||||
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let resolved = FilesystemFileResolver::new(None)
|
||||
.resolve(dir.path(), "~/.fabro_test_tilde_tmp")
|
||||
.expect("tilde path should resolve");
|
||||
|
||||
assert_eq!(resolved.content, "tilde content");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filesystem_resolver_resolves_dotdot() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
std::fs::write(dir.path().join("file.md"), "dotdot content").unwrap();
|
||||
std::fs::create_dir(dir.path().join("subdir")).unwrap();
|
||||
|
||||
let resolved = FilesystemFileResolver::new(None)
|
||||
.resolve(dir.path(), "subdir/../file.md")
|
||||
.expect("dotdot path should resolve");
|
||||
|
||||
assert_eq!(resolved.content, "dotdot content");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filesystem_resolver_falls_back_to_fallback_dir() {
|
||||
let base = tempfile::tempdir().unwrap();
|
||||
let fallback = tempfile::tempdir().unwrap();
|
||||
std::fs::write(fallback.path().join("shared.md"), "shared content").unwrap();
|
||||
|
||||
let resolved = FilesystemFileResolver::new(Some(fallback.path().to_path_buf()))
|
||||
.resolve(base.path(), "shared.md")
|
||||
.expect("file should resolve from the fallback dir");
|
||||
|
||||
assert_eq!(resolved.content, "shared content");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filesystem_resolver_base_dir_takes_precedence_over_fallback() {
|
||||
let base = tempfile::tempdir().unwrap();
|
||||
let fallback = tempfile::tempdir().unwrap();
|
||||
std::fs::write(base.path().join("prompt.md"), "base content").unwrap();
|
||||
std::fs::write(fallback.path().join("prompt.md"), "fallback content").unwrap();
|
||||
|
||||
let resolved = FilesystemFileResolver::new(Some(fallback.path().to_path_buf()))
|
||||
.resolve(base.path(), "prompt.md")
|
||||
.expect("file should resolve from the base dir");
|
||||
|
||||
assert_eq!(resolved.content, "base content");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filesystem_resolver_no_fallback_for_tilde_path() {
|
||||
let base = tempfile::tempdir().unwrap();
|
||||
let fallback = tempfile::tempdir().unwrap();
|
||||
std::fs::write(fallback.path().join("file.md"), "fallback").unwrap();
|
||||
|
||||
// A tilde path to a nonexistent file does not fall back to the fallback dir.
|
||||
assert!(
|
||||
FilesystemFileResolver::new(Some(fallback.path().to_path_buf()))
|
||||
.resolve(base.path(), "~/nonexistent_fabro_test.md")
|
||||
.is_none()
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn filesystem_resolver_returns_none_without_fallback() {
|
||||
let base = tempfile::tempdir().unwrap();
|
||||
|
||||
assert!(
|
||||
FilesystemFileResolver::new(None)
|
||||
.resolve(base.path(), "missing.md")
|
||||
.is_none()
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -68,7 +68,6 @@ pub struct CreatedRun {
|
|||
|
||||
struct PersistCreateOptions {
|
||||
settings: WorkflowSettings,
|
||||
vars: HashMap<String, String>,
|
||||
run_id: Option<RunId>,
|
||||
run_dir: Option<PathBuf>,
|
||||
workflow_slug: Option<String>,
|
||||
|
|
@ -146,9 +145,9 @@ pub async fn create(
|
|||
let persisted = spawn_blocking(move || {
|
||||
create_from_source(
|
||||
&raw_source,
|
||||
vars,
|
||||
PersistCreateOptions {
|
||||
settings,
|
||||
vars,
|
||||
run_id: Some(run_id),
|
||||
run_dir: Some(persisted_run_dir),
|
||||
workflow_slug: workflow_slug.or(resolved_workflow_slug),
|
||||
|
|
@ -288,19 +287,20 @@ fn store_error(err: impl std::fmt::Display) -> Error {
|
|||
|
||||
fn create_from_source(
|
||||
dot_source: &str,
|
||||
vars: HashMap<String, String>,
|
||||
options: PersistCreateOptions,
|
||||
current_dir: Option<PathBuf>,
|
||||
file_resolver: Option<Arc<dyn FileResolver>>,
|
||||
goal_override: Option<&str>,
|
||||
) -> Result<Persisted, Error> {
|
||||
let template_context = template_context(Some(&options.settings), vars);
|
||||
let mut validated = preprocess_and_validate(
|
||||
dot_source,
|
||||
options.source_name.clone(),
|
||||
current_dir,
|
||||
file_resolver,
|
||||
Vec::new(),
|
||||
Some(&options.settings),
|
||||
options.vars.clone(),
|
||||
template_context,
|
||||
goal_override,
|
||||
RenderMode::Structural,
|
||||
&options.catalog,
|
||||
|
|
@ -322,16 +322,13 @@ pub(super) fn preprocess_and_validate(
|
|||
current_dir: Option<PathBuf>,
|
||||
file_resolver: Option<Arc<dyn FileResolver>>,
|
||||
custom_transforms: Vec<Box<dyn Transform>>,
|
||||
settings: Option<&WorkflowSettings>,
|
||||
vars: HashMap<String, String>,
|
||||
template_context: TemplateContext,
|
||||
goal_override: Option<&str>,
|
||||
render_mode: RenderMode,
|
||||
catalog: &Arc<Catalog>,
|
||||
) -> Result<Validated, Error> {
|
||||
let inputs = run_inputs(settings);
|
||||
let mut parsed = pipeline::parse(dot_source)?;
|
||||
apply_goal_override(&mut parsed.graph, goal_override);
|
||||
let template_context = TemplateContext::new().with_inputs(inputs).with_vars(vars);
|
||||
|
||||
let transformed = pipeline::transform(parsed, &TransformOptions {
|
||||
current_dir,
|
||||
|
|
@ -345,6 +342,15 @@ pub(super) fn preprocess_and_validate(
|
|||
Ok(pipeline::validate(transformed, catalog.as_ref(), &[]))
|
||||
}
|
||||
|
||||
pub(super) fn template_context(
|
||||
settings: Option<&WorkflowSettings>,
|
||||
vars: HashMap<String, String>,
|
||||
) -> TemplateContext {
|
||||
TemplateContext::new()
|
||||
.with_inputs(run_inputs(settings))
|
||||
.with_vars(vars)
|
||||
}
|
||||
|
||||
fn run_inputs(settings: Option<&WorkflowSettings>) -> HashMap<String, toml::Value> {
|
||||
settings
|
||||
.map(|settings| settings.run.inputs.clone())
|
||||
|
|
@ -366,7 +372,6 @@ fn persist_validated(
|
|||
) -> Result<Persisted, Error> {
|
||||
let PersistCreateOptions {
|
||||
settings,
|
||||
vars: _,
|
||||
run_id,
|
||||
run_dir,
|
||||
workflow_slug,
|
||||
|
|
@ -503,8 +508,7 @@ mod tests {
|
|||
Some(PathBuf::from(".")),
|
||||
None,
|
||||
Vec::new(),
|
||||
Some(&WorkflowSettings::default()),
|
||||
vars,
|
||||
template_context(Some(&WorkflowSettings::default()), vars),
|
||||
None,
|
||||
RenderMode::Structural,
|
||||
&test_catalog(),
|
||||
|
|
@ -672,8 +676,7 @@ mod tests {
|
|||
Some(PathBuf::from(".")),
|
||||
None,
|
||||
Vec::new(),
|
||||
Some(&WorkflowSettings::default()),
|
||||
HashMap::new(),
|
||||
template_context(Some(&WorkflowSettings::default()), HashMap::new()),
|
||||
None,
|
||||
RenderMode::Strict,
|
||||
&test_catalog(),
|
||||
|
|
@ -709,8 +712,7 @@ mod tests {
|
|||
None,
|
||||
))),
|
||||
Vec::new(),
|
||||
Some(&WorkflowSettings::default()),
|
||||
HashMap::new(),
|
||||
template_context(Some(&WorkflowSettings::default()), HashMap::new()),
|
||||
None,
|
||||
RenderMode::Strict,
|
||||
&test_catalog(),
|
||||
|
|
|
|||
|
|
@ -5,7 +5,7 @@ use std::sync::Arc;
|
|||
use fabro_model::Catalog;
|
||||
use fabro_types::WorkflowSettings;
|
||||
|
||||
use super::create::preprocess_and_validate;
|
||||
use super::create::{preprocess_and_validate, template_context};
|
||||
use super::source::{ResolveWorkflowInput, WorkflowInput, resolve_workflow};
|
||||
use crate::error::Error;
|
||||
use crate::operations::RenderMode;
|
||||
|
|
@ -44,8 +44,7 @@ pub fn validate(input: ValidateInput) -> Result<Validated, Error> {
|
|||
resolved.current_dir,
|
||||
resolved.file_resolver,
|
||||
input.custom_transforms,
|
||||
Some(&resolved.settings),
|
||||
input.vars,
|
||||
template_context(Some(&resolved.settings), input.vars),
|
||||
resolved.goal_override.as_deref(),
|
||||
RenderMode::Structural,
|
||||
&input.catalog,
|
||||
|
|
|
|||
|
|
@ -1,4 +1,3 @@
|
|||
use std::collections::HashMap;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::sync::Arc;
|
||||
|
||||
|
|
@ -8,33 +7,13 @@ use fabro_types::ManifestPath;
|
|||
use fabro_validate::Diagnostic;
|
||||
|
||||
use super::Transform;
|
||||
use super::importable_field::ImportableField;
|
||||
use crate::error::Error;
|
||||
use crate::file_resolver::{FileResolver, FileResolverTemplateStore, ResolvedFile};
|
||||
use crate::static_reference::{ReferenceKind, validate_static_reference};
|
||||
use crate::transforms::variable_expansion::{
|
||||
RenderMode, TemplateRenderStore, TemplateRenderTarget, render_template_for_target,
|
||||
};
|
||||
|
||||
/// Resolve a potential `@path` file reference.
|
||||
///
|
||||
/// If `value` starts with `@` and the referenced file exists locally, the file
|
||||
/// contents are returned (inlined). Otherwise the original value is returned
|
||||
/// unchanged.
|
||||
pub fn resolve_file_ref(
|
||||
value: &str,
|
||||
current_dir: &Path,
|
||||
resolver: &dyn FileResolver,
|
||||
) -> Result<String, Error> {
|
||||
let Some(path_str) = value.strip_prefix('@') else {
|
||||
return Ok(value.to_string());
|
||||
};
|
||||
validate_static_reference(path_str, ReferenceKind::FileInline)
|
||||
.map_err(|error| Error::Validation(error.to_string()))?;
|
||||
Ok(resolver
|
||||
.resolve(current_dir, path_str)
|
||||
.map_or_else(|| value.to_string(), |resolved| resolved.content))
|
||||
}
|
||||
|
||||
fn parent_dir_or_dot(path: &Path) -> PathBuf {
|
||||
path.parent()
|
||||
.filter(|parent| !parent.as_os_str().is_empty())
|
||||
|
|
@ -168,13 +147,6 @@ impl FileInliningTransform {
|
|||
self
|
||||
}
|
||||
|
||||
/// Run-scoped `{{ vars.* }}` available to prompts and the goal.
|
||||
#[must_use]
|
||||
pub fn with_vars(mut self, vars: HashMap<String, String>) -> Self {
|
||||
self.context = self.context.with_vars(vars);
|
||||
self
|
||||
}
|
||||
|
||||
pub(crate) fn apply_with_diagnostics(
|
||||
&self,
|
||||
graph: Graph,
|
||||
|
|
@ -222,7 +194,7 @@ impl FileInliningTransform {
|
|||
&mut diagnostics,
|
||||
)?;
|
||||
let value = self
|
||||
.render_resolved_file_ref(&rendered, &ctx, target, &mut diagnostics)?
|
||||
.render_import(&rendered, &ctx, target, &mut diagnostics)?
|
||||
.unwrap_or(rendered);
|
||||
node.attrs
|
||||
.insert("prompt".to_string(), AttrValue::String(value));
|
||||
|
|
@ -265,7 +237,7 @@ impl FileInliningTransform {
|
|||
let rendered =
|
||||
render_template_for_target(goal, &ctx, self.render_mode, &target, diagnostics)?;
|
||||
let value = self
|
||||
.render_resolved_file_ref(&rendered, &ctx, target, diagnostics)?
|
||||
.render_import(&rendered, &ctx, target, diagnostics)?
|
||||
.unwrap_or(rendered);
|
||||
graph
|
||||
.attrs
|
||||
|
|
@ -273,19 +245,22 @@ impl FileInliningTransform {
|
|||
Ok(())
|
||||
}
|
||||
|
||||
fn render_resolved_file_ref(
|
||||
/// Resolve a node `prompt` / graph `goal` value to its final text. The
|
||||
/// `rendered` value is the already-MiniJinja-rendered inline content; when
|
||||
/// it is an `@path` import, the file is loaded and its contents rendered
|
||||
/// too. Returns `Ok(None)` for inline content or a missing file, so the
|
||||
/// caller falls back to the rendered inline value.
|
||||
fn render_import(
|
||||
&self,
|
||||
value: &str,
|
||||
rendered: &str,
|
||||
ctx: &TemplateContext,
|
||||
owner_target: TemplateRenderTarget,
|
||||
diagnostics: &mut Vec<Diagnostic>,
|
||||
) -> Result<Option<String>, Error> {
|
||||
let Some(path_str) = value.strip_prefix('@') else {
|
||||
let Some(path) = ImportableField::parse(rendered).import_path()? else {
|
||||
return Ok(None);
|
||||
};
|
||||
validate_static_reference(path_str, ReferenceKind::FileInline)
|
||||
.map_err(|error| Error::Validation(error.to_string()))?;
|
||||
let Some(resolved) = self.resolver.resolve(&self.current_dir, path_str) else {
|
||||
let Some(resolved) = self.resolver.resolve(&self.current_dir, path) else {
|
||||
return Ok(None);
|
||||
};
|
||||
let (source, store) = self.template_source_for_resolved_file(&resolved)?;
|
||||
|
|
@ -293,8 +268,8 @@ impl FileInliningTransform {
|
|||
.with_source_name(resolved.path.display().to_string())
|
||||
.with_source_origin(Some(&resolved.content), &resolved.content)
|
||||
.with_template_store(TemplateRenderStore::new(source, store));
|
||||
Ok(Some(render_file_contents(
|
||||
&resolved,
|
||||
Ok(Some(render_template_for_target(
|
||||
&resolved.content,
|
||||
ctx,
|
||||
self.render_mode,
|
||||
&target,
|
||||
|
|
@ -303,16 +278,14 @@ impl FileInliningTransform {
|
|||
}
|
||||
|
||||
/// Resolve an `output_schema` value. An inline JSON string is returned
|
||||
/// as-is; an `@file` reference is loaded verbatim. Unlike `prompt`,
|
||||
/// as-is; an `@file` import is loaded verbatim. Unlike `prompt`,
|
||||
/// `output_schema` is not a template, so neither the value nor the loaded
|
||||
/// file contents are MiniJinja-rendered.
|
||||
fn resolve_output_schema_ref(&self, node_id: &str, value: &str) -> Result<String, Error> {
|
||||
let Some(path_str) = value.strip_prefix('@') else {
|
||||
let Some(path) = ImportableField::parse(value).import_path()? else {
|
||||
return Ok(value.to_string());
|
||||
};
|
||||
validate_static_reference(path_str, ReferenceKind::FileInline)
|
||||
.map_err(|error| Error::Validation(error.to_string()))?;
|
||||
let Some(resolved) = self.resolver.resolve(&self.current_dir, path_str) else {
|
||||
let Some(resolved) = self.resolver.resolve(&self.current_dir, path) else {
|
||||
return Err(Error::Validation(format!(
|
||||
"node '{node_id}' output_schema has unresolved file reference: {value}"
|
||||
)));
|
||||
|
|
@ -377,16 +350,6 @@ impl Transform for FileInliningTransform {
|
|||
}
|
||||
}
|
||||
|
||||
pub(crate) fn render_file_contents(
|
||||
resolved: &ResolvedFile,
|
||||
ctx: &TemplateContext,
|
||||
render_mode: RenderMode,
|
||||
target: &TemplateRenderTarget,
|
||||
diagnostics: &mut Vec<Diagnostic>,
|
||||
) -> Result<String, Error> {
|
||||
render_template_for_target(&resolved.content, ctx, render_mode, target, diagnostics)
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
#![expect(
|
||||
|
|
@ -394,6 +357,7 @@ mod tests {
|
|||
reason = "These unit tests use the real git CLI to build repositories for file-inlining transform coverage."
|
||||
)]
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::sync::Arc;
|
||||
|
||||
use fabro_graphviz::graph::{AttrValue, Graph, Node};
|
||||
|
|
@ -407,45 +371,6 @@ mod tests {
|
|||
ManifestPath::from_wire(value).expect("path should parse")
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_file_ref_passthrough_non_at() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
assert_eq!(
|
||||
resolve_file_ref(
|
||||
"hello world",
|
||||
dir.path(),
|
||||
&FilesystemFileResolver::new(None),
|
||||
)
|
||||
.unwrap(),
|
||||
"hello world"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_file_ref_passthrough_missing_file() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
assert_eq!(
|
||||
resolve_file_ref(
|
||||
"@nonexistent.md",
|
||||
dir.path(),
|
||||
&FilesystemFileResolver::new(None),
|
||||
)
|
||||
.unwrap(),
|
||||
"@nonexistent.md"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_file_ref_inlines_existing_file() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
std::fs::write(dir.path().join("prompt.md"), "inlined content").unwrap();
|
||||
|
||||
assert_eq!(
|
||||
resolve_file_ref("@prompt.md", dir.path(), &FilesystemFileResolver::new(None)).unwrap(),
|
||||
"inlined content"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn file_inlining_transform_inlines_prompt_and_goal() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
|
|
@ -764,127 +689,6 @@ mod tests {
|
|||
assert!(matches!(err, fabro_template::TemplateError::Load { .. }));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_file_ref_expands_tilde() {
|
||||
let home = dirs::home_dir().expect("home dir must exist");
|
||||
let test_file = home.join(".fabro_test_tilde_tmp");
|
||||
std::fs::write(&test_file, "tilde content").unwrap();
|
||||
let _cleanup = scopeguard::guard((), |()| {
|
||||
let _ = std::fs::remove_file(&test_file);
|
||||
});
|
||||
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
|
||||
assert_eq!(
|
||||
resolve_file_ref(
|
||||
"@~/.fabro_test_tilde_tmp",
|
||||
dir.path(),
|
||||
&FilesystemFileResolver::new(None),
|
||||
)
|
||||
.unwrap(),
|
||||
"tilde content"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_file_ref_resolves_dotdot() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
std::fs::write(dir.path().join("file.md"), "dotdot content").unwrap();
|
||||
std::fs::create_dir(dir.path().join("subdir")).unwrap();
|
||||
|
||||
assert_eq!(
|
||||
resolve_file_ref(
|
||||
"@subdir/../file.md",
|
||||
dir.path(),
|
||||
&FilesystemFileResolver::new(None),
|
||||
)
|
||||
.unwrap(),
|
||||
"dotdot content"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_file_ref_falls_back_to_fallback_dir() {
|
||||
let base = tempfile::tempdir().unwrap();
|
||||
let fallback = tempfile::tempdir().unwrap();
|
||||
std::fs::write(fallback.path().join("shared.md"), "shared content").unwrap();
|
||||
|
||||
assert_eq!(
|
||||
resolve_file_ref(
|
||||
"@shared.md",
|
||||
base.path(),
|
||||
&FilesystemFileResolver::new(Some(fallback.path().to_path_buf())),
|
||||
)
|
||||
.unwrap(),
|
||||
"shared content"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_file_ref_base_dir_takes_precedence_over_fallback() {
|
||||
let base = tempfile::tempdir().unwrap();
|
||||
let fallback = tempfile::tempdir().unwrap();
|
||||
std::fs::write(base.path().join("prompt.md"), "base content").unwrap();
|
||||
std::fs::write(fallback.path().join("prompt.md"), "fallback content").unwrap();
|
||||
|
||||
assert_eq!(
|
||||
resolve_file_ref(
|
||||
"@prompt.md",
|
||||
base.path(),
|
||||
&FilesystemFileResolver::new(Some(fallback.path().to_path_buf())),
|
||||
)
|
||||
.unwrap(),
|
||||
"base content"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_file_ref_no_fallback_for_tilde_path() {
|
||||
let base = tempfile::tempdir().unwrap();
|
||||
let fallback = tempfile::tempdir().unwrap();
|
||||
std::fs::write(fallback.path().join("file.md"), "fallback").unwrap();
|
||||
|
||||
// Tilde path to nonexistent file should return original value, not try fallback
|
||||
let result = resolve_file_ref(
|
||||
"@~/nonexistent_fabro_test.md",
|
||||
base.path(),
|
||||
&FilesystemFileResolver::new(Some(fallback.path().to_path_buf())),
|
||||
)
|
||||
.unwrap();
|
||||
assert_eq!(result, "@~/nonexistent_fabro_test.md");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_file_ref_fallback_none_behaves_as_before() {
|
||||
let base = tempfile::tempdir().unwrap();
|
||||
assert_eq!(
|
||||
resolve_file_ref(
|
||||
"@missing.md",
|
||||
base.path(),
|
||||
&FilesystemFileResolver::new(None)
|
||||
)
|
||||
.unwrap(),
|
||||
"@missing.md"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn resolve_file_ref_rejects_template_path() {
|
||||
let base = tempfile::tempdir().unwrap();
|
||||
let err = resolve_file_ref(
|
||||
"@prompts/{{ inputs.prompt_file }}",
|
||||
base.path(),
|
||||
&FilesystemFileResolver::new(None),
|
||||
)
|
||||
.unwrap_err();
|
||||
|
||||
assert!(
|
||||
err.to_string()
|
||||
.contains("templates are not supported in file inline references"),
|
||||
"unexpected error: {err}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn file_inlining_transform_falls_back_to_fallback_dir() {
|
||||
let base = tempfile::tempdir().unwrap();
|
||||
|
|
|
|||
|
|
@ -68,14 +68,6 @@ impl ImportTransform {
|
|||
}
|
||||
}
|
||||
|
||||
/// Run-scoped `{{ vars.* }}`, propagated into imported subgraphs so their
|
||||
/// prompts and goals interpolate variables too.
|
||||
#[must_use]
|
||||
pub fn with_vars(mut self, vars: HashMap<String, String>) -> Self {
|
||||
self.context = self.context.with_vars(vars);
|
||||
self
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn with_template_options(
|
||||
mut self,
|
||||
|
|
|
|||
129
lib/crates/fabro-workflow/src/transforms/importable_field.rs
Normal file
129
lib/crates/fabro-workflow/src/transforms/importable_field.rs
Normal file
|
|
@ -0,0 +1,129 @@
|
|||
//! The `ImportableField` type: a workflow field that is either inline
|
||||
//! content or an `@path` file import.
|
||||
//!
|
||||
//! Three field consumers share this classification:
|
||||
//! - node `prompt` and the graph `goal` are *templated* importable fields — the
|
||||
//! inline value (or an imported file's contents) is MiniJinja-rendered;
|
||||
//! - `output_schema` is a *verbatim* importable field — inline content and
|
||||
//! imported file contents are used as-is, never rendered.
|
||||
//!
|
||||
//! This type owns the `@`-classification and static-reference validation that
|
||||
//! used to be hand-rolled at each call site. The render-vs-verbatim handling
|
||||
//! and the file-store plumbing stay with each consumer in
|
||||
//! [`super::file_inlining`], where the `FileResolver` and current-dir context
|
||||
//! live.
|
||||
|
||||
use crate::error::Error;
|
||||
use crate::static_reference::{ReferenceKind, validate_static_reference};
|
||||
|
||||
/// A field value that is either inline content or an `@path` file import.
|
||||
///
|
||||
/// Borrows the classified string: callers always already hold the inline value
|
||||
/// (and fall back to it), so the type never needs to own a copy.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub(crate) enum ImportableField<'a> {
|
||||
/// Inline content — the literal value or, for templated fields, the
|
||||
/// already-rendered text. The caller keeps the value itself; this variant
|
||||
/// carries no payload.
|
||||
Inline,
|
||||
/// An `@path` file import. `path` has the leading `@` stripped.
|
||||
Import { path: &'a str },
|
||||
}
|
||||
|
||||
impl<'a> ImportableField<'a> {
|
||||
/// Classify a value: a leading `@` marks a file import, everything else is
|
||||
/// inline.
|
||||
///
|
||||
/// Callers of templated fields (`prompt`/`goal`) classify the
|
||||
/// *already-rendered* string, because a leading `@` may be produced by
|
||||
/// rendering (e.g. `{{ inputs.prompt_file }}` expanding to
|
||||
/// `@prompts/work.md`).
|
||||
pub(crate) fn parse(value: &'a str) -> Self {
|
||||
match value.strip_prefix('@') {
|
||||
Some(path) => Self::Import { path },
|
||||
None => Self::Inline,
|
||||
}
|
||||
}
|
||||
|
||||
/// The validated import path (leading `@` stripped), or `None` for inline
|
||||
/// content. Validating here means a caller cannot extract a path without it
|
||||
/// being checked: an import is a static reference and must not contain
|
||||
/// template syntax (e.g. `@prompts/{{ inputs.x }}.md`).
|
||||
pub(crate) fn import_path(&self) -> Result<Option<&'a str>, Error> {
|
||||
match self {
|
||||
Self::Import { path } => {
|
||||
validate_static_reference(path, ReferenceKind::FileInline)
|
||||
.map_err(|error| Error::Validation(error.to_string()))?;
|
||||
Ok(Some(path))
|
||||
}
|
||||
Self::Inline => Ok(None),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn parse_classifies_inline_value() {
|
||||
assert_eq!(
|
||||
ImportableField::parse("Do the work"),
|
||||
ImportableField::Inline
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_classifies_at_reference_as_import() {
|
||||
assert_eq!(
|
||||
ImportableField::parse("@prompts/work.md"),
|
||||
ImportableField::Import {
|
||||
path: "prompts/work.md",
|
||||
}
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parse_strips_only_the_leading_at() {
|
||||
// A non-leading `@` (e.g. an email address) is inline, not an import.
|
||||
assert_eq!(
|
||||
ImportableField::parse("ping me@example.com"),
|
||||
ImportableField::Inline
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn import_path_returns_validated_path_for_imports_only() {
|
||||
assert_eq!(
|
||||
ImportableField::parse("@goal.md").import_path().unwrap(),
|
||||
Some("goal.md")
|
||||
);
|
||||
assert_eq!(
|
||||
ImportableField::parse("inline").import_path().unwrap(),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn import_path_accepts_inline_and_plain_import_paths() {
|
||||
ImportableField::parse("plain inline text")
|
||||
.import_path()
|
||||
.unwrap();
|
||||
ImportableField::parse("@prompts/work.md")
|
||||
.import_path()
|
||||
.unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn import_path_rejects_template_syntax() {
|
||||
let err = ImportableField::parse("@prompts/{{ inputs.prompt_file }}")
|
||||
.import_path()
|
||||
.unwrap_err();
|
||||
|
||||
assert!(
|
||||
err.to_string()
|
||||
.contains("templates are not supported in file inline references"),
|
||||
"unexpected error: {err}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
@ -10,13 +10,14 @@ pub trait Transform {
|
|||
|
||||
mod file_inlining;
|
||||
mod import;
|
||||
mod importable_field;
|
||||
mod model_resolution;
|
||||
mod preamble;
|
||||
pub mod stylesheet;
|
||||
mod stylesheet_application;
|
||||
pub mod variable_expansion;
|
||||
|
||||
pub use file_inlining::{FileInliningTransform, resolve_file_ref};
|
||||
pub use file_inlining::FileInliningTransform;
|
||||
pub use import::ImportTransform;
|
||||
pub use model_resolution::ModelResolutionTransform;
|
||||
pub use preamble::PreambleTransform;
|
||||
|
|
|
|||
|
|
@ -309,12 +309,6 @@ impl TemplateTransform {
|
|||
}
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn with_vars(mut self, vars: HashMap<String, String>) -> Self {
|
||||
self.context = self.context.with_vars(vars);
|
||||
self
|
||||
}
|
||||
|
||||
pub(crate) fn resolved_goal(
|
||||
&self,
|
||||
graph: &Graph,
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue