mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-13 04:01:43 +00:00
Propagate run Git identity to commands and agents
This commit is contained in:
parent
5f82fb086e
commit
dc30d9c1fa
4 changed files with 323 additions and 1 deletions
|
|
@ -84,9 +84,9 @@ use crate::admission::AdmittedGraphs;
|
|||
use crate::blobs::{Blobs, RunBlobs};
|
||||
use crate::controls::RunControls;
|
||||
use crate::hooks::{FabroHooks, HooksSpec};
|
||||
use crate::projection;
|
||||
use crate::runtime::RuntimeSpec;
|
||||
use crate::secrets::SharedSecrets;
|
||||
use crate::{git_identity, projection};
|
||||
|
||||
/// How the run is entered: fresh, from the admitted graphs, or continued
|
||||
/// from its records.
|
||||
|
|
@ -247,6 +247,13 @@ pub async fn run(request: RunRequest) -> Result<RunOutcome, RunError> {
|
|||
if let Some(secrets) = request.secrets {
|
||||
runtime = runtime.secrets(SharedSecrets(secrets));
|
||||
}
|
||||
let author = request
|
||||
.hooks
|
||||
.as_ref()
|
||||
.map(|hooks| hooks.git.author.clone())
|
||||
.unwrap_or_default();
|
||||
runtime =
|
||||
runtime.executor_layer(move |executor| git_identity::executor(executor, author.clone()));
|
||||
if let Some(credentials) = request.runtime.stage_credentials.clone() {
|
||||
let masker = runtime.masker();
|
||||
runtime =
|
||||
|
|
|
|||
102
lib/components/fabro-petri/src/git_identity.rs
Normal file
102
lib/components/fabro-petri/src/git_identity.rs
Normal file
|
|
@ -0,0 +1,102 @@
|
|||
//! The checkpoint identity on every process a run's scopes start.
|
||||
//!
|
||||
//! This is independent of GitHub credentials and repository configuration:
|
||||
//! commands, agent tools and additional repositories all get the same identity.
|
||||
|
||||
use std::collections::BTreeMap;
|
||||
use std::sync::Arc;
|
||||
|
||||
use async_trait::async_trait;
|
||||
use fabro_checkpoint::author::GitAuthor;
|
||||
use petri_runtime::executor::{
|
||||
AcquireContext, EnvError, EnvHandle, Executor, ReleaseReport, ScopeOutcome, ScopeSpec,
|
||||
SpawnEnv, SpawnTarget,
|
||||
};
|
||||
use smol_str::SmolStr;
|
||||
|
||||
pub(crate) fn executor(inner: Arc<dyn Executor>, author: GitAuthor) -> Arc<dyn Executor> {
|
||||
Arc::new(IdentityExecutor {
|
||||
inner,
|
||||
identity: Arc::new(IdentityEnv(author)),
|
||||
})
|
||||
}
|
||||
|
||||
struct IdentityExecutor {
|
||||
inner: Arc<dyn Executor>,
|
||||
identity: Arc<IdentityEnv>,
|
||||
}
|
||||
|
||||
#[async_trait]
|
||||
impl Executor for IdentityExecutor {
|
||||
async fn acquire(
|
||||
&self,
|
||||
scope: &ScopeSpec,
|
||||
ctx: &AcquireContext,
|
||||
) -> Result<EnvHandle, EnvError> {
|
||||
let handle = self.inner.acquire(scope, ctx).await?;
|
||||
Ok(handle.with_spawn_env(self.identity.clone()))
|
||||
}
|
||||
|
||||
async fn release(&self, handle: EnvHandle, outcome: ScopeOutcome) -> ReleaseReport {
|
||||
self.inner.release(handle, outcome).await
|
||||
}
|
||||
}
|
||||
|
||||
struct IdentityEnv(GitAuthor);
|
||||
|
||||
#[async_trait]
|
||||
impl SpawnEnv for IdentityEnv {
|
||||
async fn apply(
|
||||
&self,
|
||||
_target: SpawnTarget,
|
||||
env: &mut BTreeMap<SmolStr, SmolStr>,
|
||||
) -> Result<(), EnvError> {
|
||||
// The run selects one identity for workflow and checkpoint commits,
|
||||
// replacing conflicting stage or ambient values without writing Git config.
|
||||
for (key, value) in [
|
||||
("GIT_AUTHOR_NAME", &self.0.name),
|
||||
("GIT_AUTHOR_EMAIL", &self.0.email),
|
||||
("GIT_COMMITTER_NAME", &self.0.name),
|
||||
("GIT_COMMITTER_EMAIL", &self.0.email),
|
||||
] {
|
||||
env.insert(key.into(), value.as_str().into());
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[tokio::test]
|
||||
async fn identity_reaches_processes_and_containers_and_replaces_conflicts() {
|
||||
let identity = IdentityEnv(GitAuthor {
|
||||
name: "Run Author".into(),
|
||||
email: "run@example.com".into(),
|
||||
});
|
||||
for target in [SpawnTarget::Process, SpawnTarget::Container] {
|
||||
let mut env = BTreeMap::from([
|
||||
("GIT_AUTHOR_NAME".into(), "Stage Author".into()),
|
||||
("GIT_AUTHOR_EMAIL".into(), "stage@example.com".into()),
|
||||
("GIT_COMMITTER_NAME".into(), "Stage Committer".into()),
|
||||
("GIT_COMMITTER_EMAIL".into(), "committer@example.com".into()),
|
||||
("OTHER".into(), "preserved".into()),
|
||||
]);
|
||||
identity
|
||||
.apply(target, &mut env)
|
||||
.await
|
||||
.expect("identity applies");
|
||||
assert_eq!(
|
||||
env,
|
||||
BTreeMap::from([
|
||||
("GIT_AUTHOR_NAME".into(), "Run Author".into()),
|
||||
("GIT_AUTHOR_EMAIL".into(), "run@example.com".into()),
|
||||
("GIT_COMMITTER_NAME".into(), "Run Author".into()),
|
||||
("GIT_COMMITTER_EMAIL".into(), "run@example.com".into()),
|
||||
("OTHER".into(), "preserved".into()),
|
||||
])
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -69,6 +69,7 @@ pub mod checkpoint;
|
|||
pub mod controls;
|
||||
pub mod engine;
|
||||
pub mod fork;
|
||||
mod git_identity;
|
||||
pub mod hooks;
|
||||
pub mod host_tools;
|
||||
pub mod http_store;
|
||||
|
|
|
|||
212
lib/components/fabro-petri/tests/git_identity.rs
Normal file
212
lib/components/fabro-petri/tests/git_identity.rs
Normal file
|
|
@ -0,0 +1,212 @@
|
|||
//! Workflow-created commits and checkpoints share the run's Git identity,
|
||||
//! without configuring Git, on the host and in a clean Docker sandbox.
|
||||
|
||||
use std::fmt::Write as _;
|
||||
use std::sync::Arc;
|
||||
|
||||
use fabro_checkpoint::author::GitAuthor;
|
||||
use fabro_petri::artifacts::StoreArtifactWriter;
|
||||
use fabro_petri::check::Launch;
|
||||
use fabro_petri::engine::{self, RunStatus};
|
||||
use fabro_petri::hooks::HooksSpec;
|
||||
use fabro_petri::providers::SandboxProviderConfig;
|
||||
use fabro_petri::prune::{self, PruneRequest};
|
||||
use fabro_petri::runtime::RuntimeSpec;
|
||||
use fabro_petri::source::{RunSource, SourceRevision};
|
||||
use fabro_petri::test_support::{MemoryBlobs, MemoryPlatformRecords};
|
||||
use fabro_store::ArtifactStore;
|
||||
use fabro_types::settings::run::RunNamespace;
|
||||
use fabro_types::{RunId, SandboxProviderKind};
|
||||
use fabro_util::shell;
|
||||
use object_store::memory::InMemory;
|
||||
use pebble_coding_agent::test_support::{self, ScriptedCall};
|
||||
use petri_store::{MemoryRunStore, RunStore};
|
||||
use serde_json::json;
|
||||
|
||||
mod support;
|
||||
|
||||
async fn commits_share_identity(
|
||||
provider: SandboxProviderKind,
|
||||
name: Option<&str>,
|
||||
email: Option<&str>,
|
||||
) {
|
||||
let namespace: RunNamespace = serde_json::from_value(json!({
|
||||
"git": {"author": {"name": name, "email": email}}
|
||||
}))
|
||||
.expect("run settings");
|
||||
let mut author_settings = String::from("[run.git.author]\n");
|
||||
for (key, value) in [("name", name), ("email", email)] {
|
||||
if let Some(value) = value {
|
||||
// These fixture values have the same escaping in JSON and TOML.
|
||||
writeln!(
|
||||
author_settings,
|
||||
"{key} = {}",
|
||||
serde_json::to_string(value).expect("author quoting")
|
||||
)
|
||||
.expect("author settings");
|
||||
}
|
||||
}
|
||||
let settings = format!(
|
||||
r#"{base}
|
||||
[run.environment]
|
||||
id = "test"
|
||||
[environments.test]
|
||||
provider = "{provider}"
|
||||
[environments.test.image]
|
||||
docker = "buildpack-deps:bookworm"
|
||||
[environments.test.env]
|
||||
GIT_CONFIG_GLOBAL = "/dev/null"
|
||||
GIT_CONFIG_NOSYSTEM = "1"
|
||||
{author_settings}
|
||||
"#,
|
||||
base = support::SETTINGS,
|
||||
);
|
||||
let author = namespace
|
||||
.git
|
||||
.author
|
||||
.as_ref()
|
||||
.map(GitAuthor::from)
|
||||
.unwrap_or_default();
|
||||
let identity = format!(
|
||||
"{}|{}|{}|{}",
|
||||
author.name, author.email, author.name, author.email
|
||||
);
|
||||
let expected = shell::shell_quote(&identity);
|
||||
let check_env = format!(
|
||||
"test \"$GIT_AUTHOR_NAME|$GIT_AUTHOR_EMAIL|$GIT_COMMITTER_NAME|$GIT_COMMITTER_EMAIL\" = {expected}"
|
||||
);
|
||||
let commit =
|
||||
"git -c core.hooksPath=/dev/null -c commit.gpgsign=false commit -q --allow-empty -m";
|
||||
let command = format!(
|
||||
"set -eu; {check_env}; ! git config --get user.name; ! git config --get user.email; \
|
||||
{commit} command-commit"
|
||||
);
|
||||
let agent = format!(
|
||||
"set -eu; {check_env}; {commit} agent-commit; mkdir extra; git -C extra init -q; \
|
||||
cd extra; {commit} extra-commit"
|
||||
);
|
||||
// Native tools report errors to the model; assert the commits exist in a
|
||||
// later command so a scripted final response cannot hide a failed tool.
|
||||
let verify = format!(
|
||||
"set -eu; {check_env}; git log --format=%s | grep -Fx command-commit; \
|
||||
git log --format=%s | grep -Fx agent-commit; \
|
||||
git log --format=%s | grep -F 'fabro('; \
|
||||
test \"$(git log --format='%an|%ae|%cn|%ce' --extended-regexp --grep='^(fabro\\(|command-commit$|agent-commit$)' | sort -u)\" = {expected}; \
|
||||
test \"$(git -C extra log -1 --format='%an|%ae|%cn|%ce')\" = {expected}; \
|
||||
! git config --get user.name; ! git config --get user.email; \
|
||||
! git -C extra config --get user.name; ! git -C extra config --get user.email"
|
||||
);
|
||||
// JSON quoting is also valid DOT string quoting for these shell scripts.
|
||||
let workflow = format!(
|
||||
r#"digraph Identity {{
|
||||
graph [goal="Verify commit identities", backend="api", default_max_retries=0]
|
||||
start [shape=Mdiamond]
|
||||
exit [shape=Msquare]
|
||||
command [shape=parallelogram, goal_gate=true, script={}]
|
||||
agent [shape=box, prompt="Create commits", model="test/model"]
|
||||
verify [shape=parallelogram, goal_gate=true, script={}]
|
||||
start -> command -> agent -> verify -> exit
|
||||
}}"#,
|
||||
serde_json::to_string(&command).expect("command quoting"),
|
||||
serde_json::to_string(&verify).expect("verify quoting"),
|
||||
);
|
||||
let (client, scripted) = test_support::scripted_client(vec![
|
||||
ScriptedCall::response(test_support::tool_call_response(
|
||||
"shell",
|
||||
"commit",
|
||||
json!({"command":agent}),
|
||||
)),
|
||||
ScriptedCall::response(test_support::text_response("Created commits.")),
|
||||
]);
|
||||
let runtime = RuntimeSpec {
|
||||
model_client: Some(client),
|
||||
..RuntimeSpec::default()
|
||||
};
|
||||
let graphs = support::admit(
|
||||
&[("workflow.fabro", &workflow), ("workflow.toml", &settings)],
|
||||
Launch::default(),
|
||||
&runtime,
|
||||
);
|
||||
let root = tempfile::tempdir().expect("isolated run");
|
||||
let run_id = RunId::new().to_string();
|
||||
let store = Arc::new(MemoryRunStore::new());
|
||||
let mut request = support::run_request(
|
||||
&run_id,
|
||||
&root.path().join("run"),
|
||||
graphs,
|
||||
store.clone() as Arc<dyn RunStore>,
|
||||
runtime,
|
||||
support::no_questions(Arc::new(support::Silent)),
|
||||
);
|
||||
request.provider = provider.clone();
|
||||
let mut hooks = HooksSpec::for_run(
|
||||
Arc::new(MemoryPlatformRecords::new()),
|
||||
&namespace,
|
||||
Arc::new(StoreArtifactWriter::new(ArtifactStore::new(
|
||||
Arc::new(InMemory::new()),
|
||||
"identity-test",
|
||||
))),
|
||||
);
|
||||
hooks.git.host_workspaces = provider == SandboxProviderKind::LOCAL;
|
||||
if provider == SandboxProviderKind::DOCKER {
|
||||
// Remote sandboxes checkpoint only Git-backed runs. Use the same
|
||||
// public fixture as the existing Docker checkpoint integration test.
|
||||
hooks.source = Some(RunSource {
|
||||
origin: "https://github.com/octocat/Hello-World.git".into(),
|
||||
revision: SourceRevision::Branch("master".into()),
|
||||
branch: "master".into(),
|
||||
depth: Some(1),
|
||||
credentials: None,
|
||||
});
|
||||
}
|
||||
request.hooks = Some(hooks);
|
||||
request.blobs = Some(Arc::new(MemoryBlobs::new()));
|
||||
let outcome = engine::run(request).await.expect("run executes");
|
||||
if provider == SandboxProviderKind::DOCKER {
|
||||
prune::prune(PruneRequest {
|
||||
store: store.clone(),
|
||||
run_id: run_id.clone(),
|
||||
sandbox: SandboxProviderConfig::default(),
|
||||
run_dir: root.path().join("run"),
|
||||
provider,
|
||||
})
|
||||
.await
|
||||
.expect("test sandboxes pruned");
|
||||
}
|
||||
assert_eq!(outcome.status, RunStatus::Success, "{outcome:?}");
|
||||
assert_eq!(
|
||||
scripted.requests().len(),
|
||||
2,
|
||||
"the native agent executed its tool"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn configured_identity_reaches_commands_agents_and_checkpoints() {
|
||||
commits_share_identity(
|
||||
SandboxProviderKind::LOCAL,
|
||||
Some("Run O'Author"),
|
||||
Some("run@example.com"),
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn default_identity_reaches_commands_agents_and_checkpoints() {
|
||||
commits_share_identity(SandboxProviderKind::LOCAL, None, None).await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn partial_identity_reaches_commands_agents_and_checkpoints() {
|
||||
commits_share_identity(SandboxProviderKind::LOCAL, Some("Run O'Author"), None).await;
|
||||
}
|
||||
|
||||
#[fabro_macros::e2e_test()]
|
||||
async fn clean_docker_commands_agents_and_checkpoints_share_identity() {
|
||||
commits_share_identity(
|
||||
SandboxProviderKind::DOCKER,
|
||||
Some("Run O'Author"),
|
||||
Some("run@example.com"),
|
||||
)
|
||||
.await;
|
||||
}
|
||||
Loading…
Add table
Reference in a new issue