scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-09-13 23:14:17 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

Honor the workflow's configured run.scm repository in target derivation

Browse source 
The replaced manifest builder resolved the run's repository identity from
the workflow's run.scm settings before falling back to the checkout's
origin. The new standalone derivation always used the checkout's origin,
so a fork checkout of a workflow that names its upstream repository
silently targeted the fork and pushed there.

Read the run.scm layer from the resolved workflow.toml and project.toml
(or from the inline workflow.toml bytes) and pass it through both the CLI
and the standalone run-tool adapter. When the configured repository is
not the checkout's origin, nothing can be proven about it, so derivation
now fails with a message naming that mismatch instead of the generic
"push the commit" hint.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Scott Werner 2026-09-04 16:55:28 -04:00
parent 20f3766e75
commit 5e79285a43
3 changed files with 227 additions and 30 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
lib/apps/fabro-cli/src/commands/run/create.rs
View file

@ -71,13 +71,15 @@ pub(crate) async fn create_run(
},
resolve_run_environment(client.as_ref(), args.environment.as_deref()),
)?;
let configured_repo_origin_url =
fabro_manifest::configured_repo_origin_url_for_location(package.workflow_location())?;
let fabro_manifest::DerivedRunTarget {
target,
dirty_worktree,
} = fabro_manifest::derive_run_target_for_provider(
environment.settings.provider,
&canonical_cwd,
None,
configured_repo_origin_url.as_deref(),
)?;
if dirty_worktree {
fabro_util::printerr!(

147
lib/apps/fabro-server/src/run_tool_create.rs
View file

@ -4,9 +4,9 @@ use anyhow::{Context, Result, bail};
use async_trait::async_trait;
use fabro_environment::DEFAULT_ENVIRONMENT_ID;
use fabro_manifest::{
CollectedWorkflowClosure, DerivedRunTarget, ResolvedLocalWorkflowPackage,
collect_inline_workflow_versions, derive_run_target_for_provider,
resolve_local_workflow_package,
CollectedWorkflowClosure, DerivedRunTarget, collect_inline_workflow_versions,
configured_repo_origin_url_for_location, configured_repo_origin_url_from_workflow_toml,
derive_run_target_for_provider, resolve_local_workflow_package,
};
use fabro_tool::{
CreateRunWorkflowSource, PreparedRunCreate, RunCreateAdapter, ValidatedCreateRunSpec,
@ -104,6 +104,7 @@ impl ServerRunCreateAdapter {
client: &fabro_client::Client,
spec: &ValidatedCreateRunSpec,
cwd: &Path,
configured_repo_origin_url: Option<&str>,
) -> Result<ResolvedTarget> {
if let Some(target) = &spec.target {
if matches!(target, RunTarget::Folder { .. }) && !self.has_shared_filesystem() {
@ -152,11 +153,16 @@ impl ServerRunCreateAdapter {
let canonical_cwd = fs::canonicalize(cwd).await.with_context(|| {
format!("failed to canonicalize run directory {}", cwd.display())
})?;
let configured_repo_origin_url = configured_repo_origin_url.map(str::to_owned);
let DerivedRunTarget {
target,
dirty_worktree,
} = task::spawn_blocking(move || {
derive_run_target_for_provider(provider, &canonical_cwd, None)
derive_run_target_for_provider(
provider,
&canonical_cwd,
configured_repo_origin_url.as_deref(),
)
})
.await
.context("run target derivation task failed")??;
@ -172,11 +178,7 @@ impl ServerRunCreateAdapter {
}
}
async fn collect_selector(
&self,
selector: &str,
cwd: &Path,
) -> Result<CollectedWorkflowClosure> {
async fn collect_selector(&self, selector: &str, cwd: &Path) -> Result<CollectedWorkflow> {
if !self.has_shared_filesystem() {
bail!(
"workflow selectors require a shared Local filesystem; send inline files or an exact stored workflow version ID from Docker or Daytona"
@ -186,15 +188,27 @@ impl ServerRunCreateAdapter {
let cwd = cwd.to_path_buf();
let user_workflows_root = self.user_workflows_root().map(Path::to_path_buf);
task::spawn_blocking(move || {
resolve_local_workflow_package(&selector, &cwd, user_workflows_root.as_deref())
.map(ResolvedLocalWorkflowPackage::into_closure)
.map_err(anyhow::Error::new)
let package =
resolve_local_workflow_package(&selector, &cwd, user_workflows_root.as_deref())?;
let configured_repo_origin_url =
configured_repo_origin_url_for_location(package.workflow_location())?;
Ok(CollectedWorkflow {
closure: package.into_closure(),
configured_repo_origin_url,
})
})
.await
.context("workflow package collection task failed")?
}
}
/// A packaged workflow closure plus the `run.scm` repository its config names,
/// which standalone target derivation honors over the checkout's own origin.
struct CollectedWorkflow {
closure: CollectedWorkflowClosure,
configured_repo_origin_url: Option<String>,
}
#[async_trait]
impl RunCreateAdapter for ServerRunCreateAdapter {
async fn prepare(
@ -204,11 +218,16 @@ impl RunCreateAdapter for ServerRunCreateAdapter {
cwd: &Path,
) -> Result<PreparedRunCreate> {
let goal = self.resolve_goal(spec, cwd).await?;
let closure = match &spec.workflow {
let CollectedWorkflow {
closure,
configured_repo_origin_url,
} = match &spec.workflow {
CreateRunWorkflowSource::Stored {
workflow_version_id,
} => {
let resolved_target = self.resolve_target(client, spec, cwd).await?;
// A stored version's config is not available locally, so
// derivation uses the checkout's own origin.
let resolved_target = self.resolve_target(client, spec, cwd, None).await?;
return Ok(PreparedRunCreate {
workflow_version_id: *workflow_version_id,
target: resolved_target.target,
@ -221,7 +240,9 @@ impl RunCreateAdapter for ServerRunCreateAdapter {
}
CreateRunWorkflowSource::Inline(source) => collect_inline_workflow(source).await?,
};
let resolved_target = self.resolve_target(client, spec, cwd).await?;
let resolved_target = self
.resolve_target(client, spec, cwd, configured_repo_origin_url.as_deref())
.await?;
let versions = closure
.versions()
.map(|(_, version)| version.version())
@ -246,11 +267,30 @@ struct ResolvedTarget {
/// exact key of the file map, never a checkout selector.
async fn collect_inline_workflow(
source: &fabro_tool::InlineWorkflowSource,
) -> Result<CollectedWorkflowClosure> {
) -> Result<CollectedWorkflow> {
let entrypoint = source.entrypoint.clone();
let files = source.files.clone();
task::spawn_blocking(move || {
collect_inline_workflow_versions(&entrypoint, &files).map_err(anyhow::Error::new)
let closure = collect_inline_workflow_versions(&entrypoint, &files)?;
// The inline config is either the entrypoint itself or the
// `workflow.toml` beside the entrypoint graph.
let config_path = if Path::new(entrypoint.as_str())
.extension()
.is_some_and(|ext| ext == "toml")
{
Some(entrypoint.clone())
} else {
entrypoint.resolve_reference("workflow.toml").ok()
};
let configured_repo_origin_url = config_path
.and_then(|path| files.get(&path))
.map(|source| configured_repo_origin_url_from_workflow_toml(source))
.transpose()?
.flatten();
Ok(CollectedWorkflow {
closure,
configured_repo_origin_url,
})
})
.await
.context("inline workflow package collection task failed")?
@ -931,4 +971,77 @@ mod tests {
assert_eq!(prepared.target, RunTarget::None {});
}
#[tokio::test]
async fn workflow_version_standalone_honors_configured_scm_repository_over_checkout_origin() {
let temp = tempfile::tempdir().unwrap();
let workspace = temp.path().join("workspace");
let origin = temp.path().join("origin.git");
fs::create_dir(&workspace).await.unwrap();
run_git(temp.path(), &[
"init",
"--bare",
"--quiet",
origin.to_str().unwrap(),
]);
run_git(&workspace, &[
"init",
"--quiet",
"--initial-branch",
"feature",
]);
run_git(&workspace, &["config", "user.name", "Fabro Test"]);
run_git(&workspace, &["config", "user.email", "fabro@example.com"]);
let workflow_dir = workspace.join(".fabro/workflows/demo");
fs::create_dir_all(&workflow_dir).await.unwrap();
fs::write(workspace.join(".fabro/project.toml"), "_version = 1\n")
.await
.unwrap();
fs::write(
workflow_dir.join("workflow.toml"),
"_version = 1\n[workflow]\ngraph = \"workflow.fabro\"\n[run.scm]\nowner = \"acme\"\nrepository = \"widgets\"\n",
)
.await
.unwrap();
fs::write(
workflow_dir.join("workflow.fabro"),
"digraph Demo { start [shape=Mdiamond] exit [shape=Msquare] start -> exit }",
)
.await
.unwrap();
run_git(&workspace, &["add", "."]);
run_git(&workspace, &["commit", "--quiet", "-m", "initial"]);
// The checkout is a fork; the workflow names the upstream repository.
run_git(&workspace, &[
"remote",
"add",
"origin",
"https://github.com/alice/widgets.git",
]);
let push_url = format!("file://{}", origin.display());
run_git(&workspace, &[
"remote", "set-url", "--push", "origin", &push_url,
]);
let server = MockServer::start_async().await;
mock_environment(&server, "default", "docker").await;
let registered = Arc::new(Mutex::new(Vec::new()));
let registration =
dynamic_version_registration_mock(&server, Arc::clone(&registered)).await;
let client = no_proxy_client(&server.url(""));
let spec = validated_spec(&json!({ "workflow": "demo" }));
let adapter = ServerRunCreateAdapter::standalone(None);
let error = adapter
.prepare(&client, &spec, &workspace)
.await
.expect_err("a fork checkout must not silently become the run's repository");
registration.assert_calls_async(0).await;
assert!(
error
.to_string()
.contains("run.scm repository that is not the local checkout's origin"),
"unexpected error: {error:#}"
);
}
}

106
lib/components/fabro-manifest/src/lib.rs
View file

@ -17,10 +17,13 @@ use std::path::{Component, Path, PathBuf};
use anyhow::{Context, Result, anyhow};
use fabro_api::types;
use fabro_config::project::{self, WorkflowLocation, discover_project_config};
use fabro_config::run::{resolve_run_goal_from_layer, resolve_run_goal_from_namespace};
use fabro_config::run::{
parse_run_layer_from_settings_toml, resolve_run_goal_from_layer,
resolve_run_goal_from_namespace,
};
use fabro_config::{
CliLayer, EnvironmentLayer, EnvironmentLifecycleLayer, MergeMap, ReplaceMap,
RunEnvironmentLayer, RunExecutionLayer, RunGoalLayer, RunLayer, RunModelLayer,
RunEnvironmentLayer, RunExecutionLayer, RunGoalLayer, RunLayer, RunModelLayer, RunScmLayer,
WorkflowSettingsBuilder,
};
use fabro_graphviz::graph::AttrValue;
@ -547,9 +550,42 @@ fn none_target_for_unversioned_directory(
///
/// Returns an error when the source cannot be parsed or resolved.
pub fn configured_repo_origin_url_from_workflow_toml(source: &str) -> Result<Option<String>> {
let settings = WorkflowSettingsBuilder::from_toml(source)
.context("failed to resolve workflow settings from workflow.toml")?;
Ok(configured_repo_origin_url(&settings))
let run = parse_run_layer_from_settings_toml(source)
.context("failed to parse run settings from workflow.toml")?;
Ok(configured_repo_origin_url_from_scm_layer(
&run.scm.unwrap_or_default(),
))
}
/// The configured `run.scm` GitHub repository for a resolved local workflow.
/// `workflow.toml` values take precedence over the discovered
/// `.fabro/project.toml`, field by field, matching run settings layering.
/// `None` when neither names a repository.
///
/// # Errors
///
/// Returns an error when either config exists but cannot be read or parsed.
pub fn configured_repo_origin_url_for_location(
location: &WorkflowLocation,
) -> Result<Option<String>> {
let workflow = location
.toml
.as_deref()
.map(read_scm_layer)
.transpose()?
.unwrap_or_default();
let project = discover_project_config(&location.dir)?
.as_deref()
.map(read_scm_layer)
.transpose()?
.unwrap_or_default();
let scm = RunScmLayer {
provider: workflow.provider.or(project.provider),
owner: workflow.owner.or(project.owner),
repository: workflow.repository.or(project.repository),
github: workflow.github.or(project.github),
};
Ok(configured_repo_origin_url_from_scm_layer(&scm))
}
struct LocalGitObservation {
@ -626,15 +662,23 @@ fn github_run_target(origin_url: &str, branch: &str) -> Option<GitRunTarget> {
fn configured_repo_origin_url(settings: &WorkflowSettings) -> Option<String> {
let scm = &settings.run.scm;
if !scm
.provider
.as_deref()
.is_none_or(|provider| provider.eq_ignore_ascii_case("github"))
{
configured_repo_origin_url_from_scm(
scm.provider.as_deref(),
scm.owner.as_deref(),
scm.repository.as_deref(),
)
}
fn configured_repo_origin_url_from_scm(
provider: Option<&str>,
owner: Option<&str>,
repository: Option<&str>,
) -> Option<String> {
if !provider.is_none_or(|provider| provider.eq_ignore_ascii_case("github")) {
return None;
}
let owner = scm.owner.as_deref()?;
let repository = scm.repository.as_deref()?;
let owner = owner?;
let repository = repository?;
if owner.trim().is_empty() || repository.trim().is_empty() {
return None;
}
@ -643,6 +687,22 @@ fn configured_repo_origin_url(settings: &WorkflowSettings) -> Option<String> {
(!normalized.is_empty()).then_some(normalized)
}
fn configured_repo_origin_url_from_scm_layer(scm: &RunScmLayer) -> Option<String> {
configured_repo_origin_url_from_scm(
scm.provider.as_deref(),
scm.owner.as_deref(),
scm.repository.as_deref(),
)
}
fn read_scm_layer(path: &Path) -> Result<RunScmLayer> {
let source = std::fs::read_to_string(path)
.with_context(|| format!("failed to read {}", path.display()))?;
let run = parse_run_layer_from_settings_toml(&source)
.with_context(|| format!("failed to parse run settings from {}", path.display()))?;
Ok(run.scm.unwrap_or_default())
}
struct ManifestRepoInfo {
/// The `origin` URL as libgit2 reports it (after `insteadOf` rewrites).
origin_url: Option<String>,
@ -825,6 +885,28 @@ pub(crate) mod test_fixtures {
#[cfg(test)]
mod tests {
#[test]
fn configured_repo_origin_url_reads_run_scm_from_workflow_toml() {
let configured = super::configured_repo_origin_url_from_workflow_toml(
"_version = 1\n[run.scm]\nowner = \"acme\"\nrepository = \"widgets\"\n",
)
.unwrap();
assert_eq!(
configured.as_deref(),
Some("https://github.com/acme/widgets")
);
let unconfigured =
super::configured_repo_origin_url_from_workflow_toml("_version = 1\n").unwrap();
assert_eq!(unconfigured, None);
let other_provider = super::configured_repo_origin_url_from_workflow_toml(
"_version = 1\n[run.scm]\nprovider = \"gitlab\"\nowner = \"acme\"\nrepository = \"widgets\"\n",
)
.unwrap();
assert_eq!(other_provider, None);
}
use fabro_workflow::git::head_sha;
use super::*;