From ee6576cff7ec47b1ea83a1c240d8506900d7e7fa Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Sat, 12 Sep 2026 11:35:46 -0600 Subject: [PATCH] Resolve one Git identity per run and inject it everywhere A run now resolves a single author and committer identity once, after its GitHub credentials are selected and before anything can commit, and uses it for every commit it creates. Resolution order: a complete explicit `run.git.author`; the run's GitHub App bot account (`[bot] `); the authenticated user of the run's PAT; the generic `Fabro `. A partial explicit author overlays the fields it supplies. Only the selected credential is consulted; a failed lookup is a setup error. A standalone installation token falls back to the generic identity with a warning. The resolved identity is carried on `RunOptions` and `EngineServices`, recorded as a `git.identity.resolved` event and `RunProjection.git_identity` so resume reuses it, and exposed through the run state API. Engine checkpoints and metadata commits read it through `RunOptions::git_author`. Every workflow execution path receives it as `GIT_AUTHOR_NAME`, `GIT_AUTHOR_EMAIL`, `GIT_COMMITTER_NAME`, and `GIT_COMMITTER_EMAIL`, applied last so it wins over inherited host variables and `[run.environment]` entries: prepare steps, command stages, native agent shell tools, and ACP launches. The identity is injected even without a Git origin, and the old local `git config user.*` write is removed. fabro-github gains `GET /user` and `/users/{slug}[bot]` lookups with mocked tests for success, unauthorized, malformed, and transient cases. Real-Git integration tests commit in the primary checkout, a clone, and a fresh repository under conflicting local config, `[run.environment]`, and host variables, and prove concurrent runs do not leak identities. CLI workflow tests cover host script stages and ACP launch env through `fabro run`. Docs and generated option metadata now describe the credential-derived defaults instead of the stale `fabro`/`fabro@local` values. Co-Authored-By: Claude Fable 5.1 --- .../administration/server-configuration.mdx | 16 +- docs/public/api-reference/fabro-api.yaml | 36 ++ docs/public/execution/checkpoints.mdx | 2 + docs/public/integrations/github.mdx | 4 + docs/public/reference/user-configuration.mdx | 4 +- lib/apps/fabro-cli/src/commands/run/events.rs | 12 + lib/apps/fabro-cli/tests/it/cmd/attach.rs | 15 + lib/apps/fabro-cli/tests/it/cmd/dump.rs | 6 +- .../tests/it/workflow/git_identity.rs | 278 +++++++++++++++ lib/apps/fabro-cli/tests/it/workflow/mod.rs | 1 + lib/components/fabro-checkpoint/src/author.rs | 14 +- lib/components/fabro-github/src/identity.rs | 328 +++++++++++++++++ lib/components/fabro-github/src/lib.rs | 4 +- lib/components/fabro-store/src/run_state.rs | 3 + .../fabro-workflow/src/event/convert.rs | 5 + .../fabro-workflow/src/event/events.rs | 13 + .../fabro-workflow/src/event/names.rs | 1 + .../fabro-workflow/src/git_identity.rs | 329 ++++++++++++++++++ .../src/handler/manager_loop.rs | 3 + lib/components/fabro-workflow/src/lib.rs | 1 + .../fabro-workflow/src/lifecycle/git.rs | 1 + .../fabro-workflow/src/operations/start.rs | 1 + .../src/pipeline/execute/tests.rs | 1 + .../fabro-workflow/src/pipeline/finalize.rs | 1 + .../fabro-workflow/src/pipeline/initialize.rs | 263 +++++++++++--- .../fabro-workflow/src/run_metadata.rs | 1 + .../fabro-workflow/src/run_options.rs | 11 +- lib/components/fabro-workflow/src/services.rs | 68 +++- .../fabro-workflow/src/test_support.rs | 28 ++ .../tests/it/daytona_integration.rs | 5 + .../tests/it/git_integration.rs | 238 ++++++++++++- .../fabro-workflow/tests/it/integration.rs | 120 +++++++ .../fabro-workflow/tests/it/pebble_agent.rs | 1 + .../tests/run_projection_round_trip.rs | 5 + lib/foundation/fabro-config/src/layers/run.rs | 17 +- .../fabro-types/src/git_identity.rs | 93 +++++ lib/foundation/fabro-types/src/lib.rs | 2 + .../fabro-types/src/run_event/infra.rs | 11 +- .../fabro-types/src/run_event/mod.rs | 4 + .../fabro-types/src/run_projection.rs | 7 +- .../src/.openapi-generator/FILES | 2 + .../src/models/git-identity-source.ts | 28 ++ .../src/models/git-identity.ts | 27 ++ .../fabro-api-client/src/models/index.ts | 2 + .../src/models/run-projection.ts | 4 + 45 files changed, 1951 insertions(+), 65 deletions(-) create mode 100644 lib/apps/fabro-cli/tests/it/workflow/git_identity.rs create mode 100644 lib/components/fabro-github/src/identity.rs create mode 100644 lib/components/fabro-workflow/src/git_identity.rs create mode 100644 lib/foundation/fabro-types/src/git_identity.rs create mode 100644 lib/packages/fabro-api-client/src/models/git-identity-source.ts create mode 100644 lib/packages/fabro-api-client/src/models/git-identity.ts diff --git a/docs/public/administration/server-configuration.mdx b/docs/public/administration/server-configuration.mdx index 9c0f53508..d299ec116 100644 --- a/docs/public/administration/server-configuration.mdx +++ b/docs/public/administration/server-configuration.mdx @@ -330,12 +330,22 @@ The CLI has its own `[cli.logging]` section. ### `[run.git.author]` section -Customize the git author identity used for checkpoint commits. When not set, defaults to `fabro` / `fabro@local`. +Override the Git author and committer identity for every commit a run creates: Fabro's own checkpoint and metadata commits, and any `git commit` a prepare step, command stage, or agent tool runs inside the sandbox. Fabro resolves one identity per run and injects it as `GIT_AUTHOR_NAME`, `GIT_AUTHOR_EMAIL`, `GIT_COMMITTER_NAME`, and `GIT_COMMITTER_EMAIL` into every workflow command, so the primary checkout, additional clones, and repositories a workflow clones itself all commit as the same identity. It does not write to any Git configuration file. + +When a field is not set, Fabro derives it from the run's GitHub credential: + +| Credential | Name | Email | +|---|---|---| +| GitHub App (`strategy = "app"`) | `[bot]` | `+[bot]@users.noreply.github.com` | +| Token (`strategy = "token"`) | the token's user login | `+@users.noreply.github.com` | +| None | `Fabro` | `noreply@fabro.sh` | + +Setting both `name` and `email` skips the credential lookup. Setting one field overlays it on the derived identity. A lookup failure for the selected credential fails the run at setup; Fabro never silently switches to another author. The resolved identity is recorded in the run's event stream as `git.identity.resolved` and in the run state as `git_identity`. | Key | Description | Default | |---|---|---| -| `name` | Git author name | `"fabro"` | -| `email` | Git author email | `"fabro@local"` | +| `name` | Git author and committer name | derived from the run's GitHub credential | +| `email` | Git author and committer email | derived from the run's GitHub credential | ### `[server.integrations.github]` section diff --git a/docs/public/api-reference/fabro-api.yaml b/docs/public/api-reference/fabro-api.yaml index 9ec0dbb2d..790392461 100644 --- a/docs/public/api-reference/fabro-api.yaml +++ b/docs/public/api-reference/fabro-api.yaml @@ -9557,6 +9557,35 @@ components: dirty: $ref: "#/components/schemas/DirtyStatus" + GitIdentitySource: + description: Where a run's Git author/committer identity came from. + type: string + enum: + - explicit + - github_app + - github_pat + - default + + GitIdentity: + description: > + The Git author and committer identity a run resolved once and uses for + every commit it creates: engine checkpoints, metadata commits, and any + commit a workflow command or agent tool runs. + type: object + required: + - name + - email + - source + properties: + name: + type: string + example: "fabro-sh[bot]" + email: + type: string + example: "123456+fabro-sh[bot]@users.noreply.github.com" + source: + $ref: "#/components/schemas/GitIdentitySource" + ManifestGoal: description: Resolved goal kind and content. type: object @@ -11936,6 +11965,13 @@ components: retried_from: type: ["string", "null"] description: Source run ID when this run was created by manual retry. + git_identity: + oneOf: + - $ref: "#/components/schemas/GitIdentity" + - type: "null" + description: > + The Git author/committer identity the run resolved for its + commits. Absent until the run's first initialization resolves it. pending_interviews: type: object additionalProperties: diff --git a/docs/public/execution/checkpoints.mdx b/docs/public/execution/checkpoints.mdx index 814edf950..a605b068c 100644 --- a/docs/public/execution/checkpoints.mdx +++ b/docs/public/execution/checkpoints.mdx @@ -41,6 +41,8 @@ The `Fabro-Checkpoint` trailer links each run branch commit to its metadata bran Fabro disables Git commit and tag signing for checkpoint commits created inside a sandbox. Your personal or repository-level signing settings can stay enabled, but sandbox bookkeeping does not need access to your signing key. +Checkpoint commits, metadata commits, and any commit a workflow command or agent creates all carry the run's one resolved Git identity. Fabro derives it from the run's GitHub App bot account or token user, or from the generic `Fabro ` identity when the run has no GitHub credential, and `[run.git.author]` overrides it. See [`[run.git.author]`](/administration/server-configuration#rungitauthor-section). + ### Metadata branch The metadata branch (`fabro/meta/{run_id}`) is an orphan branch that stores structured run data using Git's object storage directly. Fabro writes it from inside the sandbox with Git plumbing commands, without checking out a metadata worktree. It is initialized at run start with: diff --git a/docs/public/integrations/github.mdx b/docs/public/integrations/github.mdx index d28b954e0..bf5dee942 100644 --- a/docs/public/integrations/github.mdx +++ b/docs/public/integrations/github.mdx @@ -313,6 +313,10 @@ The permissions table follows the standard layer-merge order (workflow > project The upper bound on what Fabro will mint is whatever permissions the GitHub App installation has been granted. Fabro does **not** impose a separate server-side cap on the run-level `permissions` map: any value the App has been granted can be requested by run config. Operators must not run untrusted workflow, project, or user TOML against a broadly-scoped GitHub App installation. Preflight prints the resolved permission set so reviewers can see what each run will request. +### Commit attribution + +Every commit a run creates is authored and committed by the run's GitHub credential identity. In App mode that is the App's bot account, `[bot] `, so GitHub attributes checkpoint commits and workflow-created commits to the App. In token mode it is the token's user with their GitHub noreply address. Fabro resolves the identity once per run and passes it to every prepare step, command stage, agent tool, and ACP agent as `GIT_AUTHOR_*` / `GIT_COMMITTER_*` variables, so a workflow that clones or initializes another repository commits as the same identity without configuring Git itself. `[run.git.author]` overrides either field; see [server configuration](/administration/server-configuration#rungitauthor-section). + ### Checkpoint pushing After each workflow stage, Fabro [checkpoints](/execution/checkpoints) by pushing the run branch and metadata branch to origin. Before a successful run becomes terminal, the publish stage pushes the final commit again and treats failure as a run failure. Inside remote sandboxes, the git remote URL is configured with the Installation Access Token for authenticated pushing. diff --git a/docs/public/reference/user-configuration.mdx b/docs/public/reference/user-configuration.mdx index 5e0fe714e..8154900b5 100644 --- a/docs/public/reference/user-configuration.mdx +++ b/docs/public/reference/user-configuration.mdx @@ -355,8 +355,8 @@ email = "fabro-bot@company.com" | Key | Type / values | Default | Description | |---|---|---|---| -| `email` | string | "fabro@local" | Git author email for checkpoint commits. | -| `name` | string | "fabro" | Git author name for checkpoint commits. | +| `email` | string | resolved from the run's GitHub credential | Git author and committer email for every commit the run creates. When
unset, the run uses the credential's noreply address, else
`noreply@fabro.sh`. | +| `name` | string | resolved from the run's GitHub credential | Git author and committer name for every commit the run creates. When
unset, the run uses its GitHub App bot or PAT user, else `Fabro`. | ## `[run.pull_request]` diff --git a/lib/apps/fabro-cli/src/commands/run/events.rs b/lib/apps/fabro-cli/src/commands/run/events.rs index 2149bfdcf..ffebde99a 100644 --- a/lib/apps/fabro-cli/src/commands/run/events.rs +++ b/lib/apps/fabro-cli/src/commands/run/events.rs @@ -654,6 +654,18 @@ fn format_event_pretty_value(envelope: &serde_json::Value, styles: &Styles) -> O styles.dim.apply_to(&duration), )) } + "git.identity.resolved" => { + let name = prop_str_field(envelope, "name").unwrap_or("?"); + let email = prop_str_field(envelope, "email").unwrap_or("?"); + let source = prop_str_field(envelope, "source").unwrap_or("?"); + Some(format!( + "{} Git identity: {} <{}> {}", + styles.dim.apply_to(&ts), + name, + email, + styles.dim.apply_to(source), + )) + } "sandbox.create.progress" => { let code = envelope .pointer("/properties/progress/code") diff --git a/lib/apps/fabro-cli/tests/it/cmd/attach.rs b/lib/apps/fabro-cli/tests/it/cmd/attach.rs index 7f75eee71..537969ab4 100644 --- a/lib/apps/fabro-cli/tests/it/cmd/attach.rs +++ b/lib/apps/fabro-cli/tests/it/cmd/attach.rs @@ -1214,6 +1214,21 @@ fn attach_json_errors_without_prompting_for_human_input() { "run_id": "[ULID]", "ts": "[TIMESTAMP]" }, + { + "actor": { + "kind": "worker", + "run_id": "[ULID]" + }, + "event": "git.identity.resolved", + "id": "[EVENT_ID]", + "properties": { + "email": "noreply@fabro.sh", + "name": "Fabro", + "source": "default" + }, + "run_id": "[ULID]", + "ts": "[TIMESTAMP]" + }, { "actor": { "kind": "worker", diff --git a/lib/apps/fabro-cli/tests/it/cmd/dump.rs b/lib/apps/fabro-cli/tests/it/cmd/dump.rs index f56f590b2..4fb079cf9 100644 --- a/lib/apps/fabro-cli/tests/it/cmd/dump.rs +++ b/lib/apps/fabro-cli/tests/it/cmd/dump.rs @@ -261,9 +261,9 @@ fn dump_exports_completed_run_snapshot() { "); assert_snapshot!(dump_file_summary(&output_dir), @" - checkpoints/0017.json - checkpoints/0021.json - checkpoints/0025.json + checkpoints/0018.json + checkpoints/0022.json + checkpoints/0026.json events.jsonl graph.fabro run.json diff --git a/lib/apps/fabro-cli/tests/it/workflow/git_identity.rs b/lib/apps/fabro-cli/tests/it/workflow/git_identity.rs new file mode 100644 index 000000000..caddfd6f8 --- /dev/null +++ b/lib/apps/fabro-cli/tests/it/workflow/git_identity.rs @@ -0,0 +1,278 @@ +#![expect( + clippy::disallowed_methods, + reason = "integration test initializes an isolated git repository with the system git binary" +)] + +use std::path::Path; +use std::process::Command; + +use fabro_acp::test_support::fake_acp_agent_script; +use fabro_test::{TestContext, test_context}; +use fabro_types::{EventBody, GitIdentitySource}; + +use super::{find_run_dir, read_conclusion, run_events, run_state}; + +fn git(dir: &Path, args: &[&str]) -> String { + let output = Command::new("git") + .args(args) + .current_dir(dir) + .output() + .unwrap_or_else(|err| panic!("git {args:?} should run: {err}")); + assert!( + output.status.success(), + "git {args:?} failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8_lossy(&output.stdout).trim().to_string() +} + +fn init_repo_with_local_identity(dir: &Path) { + git(dir, &["init", "-q"]); + git(dir, &["config", "user.name", "Local Config"]); + git(dir, &["config", "user.email", "local@example.com"]); + std::fs::write(dir.join("README.md"), "seed\n").expect("seed file should write"); + git(dir, &["add", "README.md"]); + git(dir, &["commit", "-q", "-m", "seed"]); +} + +fn identity_of(dir: &Path, rev: &str) -> Vec { + git(dir, &["show", "-s", "--format=%an%n%ae%n%cn%n%ce", rev]) + .lines() + .map(str::to_string) + .collect() +} + +fn setup(context: &mut TestContext) { + context.write_home( + ".fabro/settings.toml", + "[server.auth]\nmethods = [\"dev-token\"]\n", + ); + context.isolated_server(); +} + +/// A host run with no GitHub credential and no explicit author commits as +/// the generic Fabro identity: a commit a script stage creates in the +/// checkout, and a commit it creates in a repository it initializes itself. The +/// checkout's own Git configuration and the host's inherited `GIT_*` variables +/// do not leak in, and the run does not touch the isolated HOME's Git +/// configuration. +#[test] +fn script_stage_commits_carry_the_run_identity() { + let mut context = test_context!(); + setup(&mut context); + init_repo_with_local_identity(&context.temp_dir); + let other_repo = context.temp_dir.join("other-repo"); + let home_gitconfig = context.home_dir.join(".gitconfig"); + let script = format!( + "set -e; printf work > work.txt && git add work.txt && git commit -q -m 'workflow commit' && \ + git init -q {other} && cd {other} && printf x > x.txt && git add x.txt && git commit -q -m 'other commit'", + other = other_repo.display() + ); + context.write_temp( + "identity.fabro", + format!( + r#"digraph Identity {{ + graph [goal="Commit as the run identity"] + start [shape=Mdiamond] + work [shape=parallelogram, script="{script}"] + exit [shape=Msquare] + start -> work -> exit +}}"# + ), + ); + + context + .run_cmd() + .env("GIT_AUTHOR_NAME", "Inherited Host") + .env("GIT_AUTHOR_EMAIL", "host@example.com") + .env("GIT_COMMITTER_NAME", "Inherited Host") + .env("GIT_COMMITTER_EMAIL", "host@example.com") + .args(["--auto-approve", "--environment", "local"]) + .arg(context.temp_dir.join("identity.fabro")) + .assert() + .success(); + + let run_dir = find_run_dir(&context); + assert_eq!(read_conclusion(&run_dir)["status"], "succeeded"); + + let expected = vec![ + "Fabro".to_string(), + "noreply@fabro.sh".to_string(), + "Fabro".to_string(), + "noreply@fabro.sh".to_string(), + ]; + // A host run without a clone has no managed run branch, so the checkout + // HEAD is the workflow's own commit. The engine checkpoint path is + // covered by the fabro-workflow git integration tests. + assert_eq!( + git(&context.temp_dir, &["log", "-1", "--format=%s"]), + "workflow commit" + ); + assert_eq!( + identity_of(&context.temp_dir, "HEAD"), + expected, + "workflow commit in the checkout" + ); + assert_eq!( + identity_of(&other_repo, "HEAD"), + expected, + "commit in a workflow-created repository" + ); + + // No configuration was written: the checkout keeps its local identity and + // the isolated HOME gained no Git configuration. + assert_eq!( + git(&context.temp_dir, &["config", "user.name"]), + "Local Config" + ); + assert_eq!( + git(&context.temp_dir, &["config", "user.email"]), + "local@example.com" + ); + assert!( + !home_gitconfig.exists(), + "the run must not write {}", + home_gitconfig.display() + ); + + // The resolved identity is recorded durably and in the event stream. + let state = run_state(&run_dir); + let identity = state + .git_identity + .expect("run state should record the identity"); + assert_eq!(identity.name, "Fabro"); + assert_eq!(identity.email, "noreply@fabro.sh"); + assert_eq!(identity.source, GitIdentitySource::Default); + assert!( + run_events(&run_dir) + .iter() + .any(|event| matches!(&event.event.body, EventBody::GitIdentityResolved(props) if props.identity == identity)), + "git.identity.resolved should be emitted" + ); +} + +/// `run.git.author` overrides the identity for every path, and a run whose +/// working directory has no Git origin still receives it. +#[test] +fn explicit_author_reaches_script_stages_without_a_git_origin() { + let mut context = test_context!(); + setup(&mut context); + let repo = context.temp_dir.join("fresh"); + let script = format!( + "set -e; git init -q {repo} && cd {repo} && printf x > x.txt && git add x.txt && git commit -q -m 'fresh commit'", + repo = repo.display() + ); + context.write_temp( + "explicit.fabro", + format!( + r#"digraph Explicit {{ + graph [goal="Commit as the explicit author"] + start [shape=Mdiamond] + work [shape=parallelogram, script="{script}"] + exit [shape=Msquare] + start -> work -> exit +}}"# + ), + ); + context.write_temp( + "workflow.toml", + r#"_version = 1 + +[workflow] +graph = "explicit.fabro" + +[run] +goal = "Commit as the explicit author" + +[run.git.author] +name = "Release Bot" +email = "release@example.com" + +[run.environment.env] +GIT_AUTHOR_NAME = "Run Env" +"#, + ); + + context + .run_cmd() + .args(["--auto-approve", "--environment", "local"]) + .arg(context.temp_dir.join("workflow.toml")) + .assert() + .success(); + + let run_dir = find_run_dir(&context); + assert_eq!(read_conclusion(&run_dir)["status"], "succeeded"); + assert_eq!(identity_of(&repo, "HEAD"), vec![ + "Release Bot".to_string(), + "release@example.com".to_string(), + "Release Bot".to_string(), + "release@example.com".to_string(), + ]); + let identity = run_state(&run_dir) + .git_identity + .expect("run state should record the identity"); + assert_eq!(identity.source, GitIdentitySource::Explicit); +} + +/// An ACP agent process is launched with the run identity in its environment. +#[test] +fn acp_agent_launch_env_carries_the_run_identity() { + let mut context = test_context!(); + setup(&mut context); + context.write_temp("fake_acp_agent.py", fake_acp_agent_script()); + let fake_agent = context.temp_dir.join("fake_acp_agent.py"); + let env_record = context.temp_dir.join("acp-env.json"); + let config = serde_json::json!({ + "type": "stdio", + "name": "fake", + "command": "python3", + "args": [fake_agent.to_string_lossy()], + "env": [ + {"name": "ACP_MODE", "value": "write_file"}, + {"name": "ACP_ENV_RECORD", "value": env_record.to_string_lossy()}, + { + "name": "ACP_ENV_RECORD_KEYS", + "value": "GIT_AUTHOR_NAME,GIT_AUTHOR_EMAIL,GIT_COMMITTER_NAME,GIT_COMMITTER_EMAIL", + }, + ], + }) + .to_string(); + let acp_config = format!("{config:?}"); + context.write_temp( + "acp_identity.fabro", + format!( + r#"digraph ACP {{ + graph [goal="Exercise ACP launch env"] + start [shape=Mdiamond] + work [type="agent", backend="acp", prompt="write hello.txt", acp.config={acp_config}] + exit [shape=Msquare] + start -> work -> exit +}}"# + ), + ); + git(&context.temp_dir, &["init", "-q"]); + + context + .run_cmd() + .env("GIT_AUTHOR_NAME", "Inherited Host") + .args(["--auto-approve", "--environment", "local"]) + .arg(context.temp_dir.join("acp_identity.fabro")) + .assert() + .success(); + + let run_dir = find_run_dir(&context); + assert_eq!(read_conclusion(&run_dir)["status"], "succeeded"); + let recorded: serde_json::Value = serde_json::from_str( + &std::fs::read_to_string(&env_record).expect("fake ACP agent should record its env"), + ) + .expect("record should be JSON"); + assert_eq!( + recorded, + serde_json::json!({ + "GIT_AUTHOR_NAME": "Fabro", + "GIT_AUTHOR_EMAIL": "noreply@fabro.sh", + "GIT_COMMITTER_NAME": "Fabro", + "GIT_COMMITTER_EMAIL": "noreply@fabro.sh", + }) + ); +} diff --git a/lib/apps/fabro-cli/tests/it/workflow/mod.rs b/lib/apps/fabro-cli/tests/it/workflow/mod.rs index ba180288c..155df6d23 100644 --- a/lib/apps/fabro-cli/tests/it/workflow/mod.rs +++ b/lib/apps/fabro-cli/tests/it/workflow/mod.rs @@ -12,6 +12,7 @@ mod command_routing; mod conditional_branching; mod dry_run_examples; mod full_stack; +mod git_identity; mod hooks; mod human_gate; pub(super) mod plugin; diff --git a/lib/components/fabro-checkpoint/src/author.rs b/lib/components/fabro-checkpoint/src/author.rs index 9546962b6..c381aa237 100644 --- a/lib/components/fabro-checkpoint/src/author.rs +++ b/lib/components/fabro-checkpoint/src/author.rs @@ -1,6 +1,7 @@ use std::fmt::Write; use fabro_config::GitAuthorLayer; +use fabro_types::GitIdentity; use fabro_types::settings::run::GitAuthorSettings; /// Resolved git author identity for checkpoint commits. @@ -13,8 +14,8 @@ pub struct GitAuthor { impl Default for GitAuthor { fn default() -> Self { Self { - name: "Fabro".into(), - email: "noreply@fabro.sh".into(), + name: GitIdentity::DEFAULT_NAME.into(), + email: GitIdentity::DEFAULT_EMAIL.into(), } } } @@ -61,3 +62,12 @@ impl From<&GitAuthorSettings> for GitAuthor { Self::from_options(value.name.clone(), value.email.clone()) } } + +impl From<&GitIdentity> for GitAuthor { + fn from(value: &GitIdentity) -> Self { + Self { + name: value.name.clone(), + email: value.email.clone(), + } + } +} diff --git a/lib/components/fabro-github/src/identity.rs b/lib/components/fabro-github/src/identity.rs new file mode 100644 index 000000000..ac53116b6 --- /dev/null +++ b/lib/components/fabro-github/src/identity.rs @@ -0,0 +1,328 @@ +//! GitHub account identity lookups for a run's Git author. +//! +//! A run's commits carry the identity of the credential the run uses: +//! the App's bot account (`[bot]`) or the authenticated user of a +//! personal access token. Both resolve to a stable GitHub.com noreply +//! address, `{id}+{login}@users.noreply.github.com`, so GitHub attributes +//! the commits to that account without a private email. + +use anyhow::{Context as _, bail}; +use serde::Deserialize; + +use crate::{GitHubAppCredentials, HttpClient, HttpMethod, github_headers, sign_app_jwt}; + +/// A GitHub account resolved for Git attribution. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct GitHubAccountIdentity { + /// The account login. App bot accounts carry the `[bot]` suffix. + pub login: String, + /// The numeric user id (a bot's *user* id, not the App id). + pub id: u64, +} + +impl GitHubAccountIdentity { + /// The GitHub.com noreply address GitHub attributes to this account. + #[must_use] + pub fn noreply_email(&self) -> String { + format!("{}+{}@users.noreply.github.com", self.id, self.login) + } +} + +#[derive(Deserialize)] +struct AccountResponse { + login: String, + id: u64, +} + +fn validated(account: &AccountResponse, what: &str) -> anyhow::Result { + let login = account.login.trim(); + if login.is_empty() { + bail!("GitHub returned an empty login for {what}"); + } + if login + .chars() + .any(|ch| ch.is_control() || ch.is_whitespace() || matches!(ch, '<' | '>' | '@')) + { + bail!("GitHub returned an invalid login for {what}"); + } + if account.id == 0 { + bail!("GitHub returned an invalid user id for {what}"); + } + Ok(GitHubAccountIdentity { + login: login.to_string(), + id: account.id, + }) +} + +/// Resolve the authenticated user of a personal access token via `GET /user`. +/// +/// Reads only the public login and id; no email scope is requested. +pub async fn lookup_token_identity( + client: &impl HttpClient, + token: &str, + base_url: &str, +) -> anyhow::Result { + let url = format!("{base_url}/user"); + let auth = format!("Bearer {token}"); + let resp = client + .request(HttpMethod::Get, &url, &github_headers(&auth), None) + .await + .context("Failed to fetch the GitHub token's user")?; + match resp.status { + 200 => {} + 401 => bail!("GitHub rejected the configured GITHUB_TOKEN while resolving its user"), + 403 => bail!("GitHub refused to identify the configured GITHUB_TOKEN's user (403)"), + status => bail!("Unexpected status {status} fetching the GitHub token's user"), + } + let account: AccountResponse = resp + .json() + .context("Failed to parse the GitHub token's user")?; + validated(&account, "the GitHub token's user") +} + +/// Resolve the bot account of a GitHub App. +/// +/// The App slug comes from the credentials when configured, otherwise from +/// `GET /app` with the App JWT. The bot account is then read from +/// `GET /users/{slug}[bot]`. That endpoint rejects App JWTs, so it is called +/// with `bearer` (an installation token or other API token) when one is +/// available and anonymously otherwise. +pub async fn lookup_app_bot_identity( + client: &impl HttpClient, + creds: &GitHubAppCredentials, + base_url: &str, + bearer: Option<&str>, +) -> anyhow::Result { + let slug = match creds.slug.as_deref().map(str::trim) { + Some(slug) if !slug.is_empty() => slug.to_string(), + _ => { + let jwt = sign_app_jwt(&creds.app_id, &creds.private_key_pem)?; + crate::get_authenticated_app(client, &jwt, base_url) + .await + .context("Failed to determine the GitHub App slug")? + .slug + } + }; + let login = format!("{slug}[bot]"); + let url = format!( + "{base_url}/users/{}", + login.replace('[', "%5B").replace(']', "%5D") + ); + let auth = bearer.map(|token| format!("Bearer {token}")); + let headers: Vec<(&str, &str)> = match auth.as_deref() { + Some(auth) => github_headers(auth).to_vec(), + None => vec![ + ("Accept", "application/vnd.github+json"), + ("User-Agent", "fabro"), + ], + }; + let resp = client + .request(HttpMethod::Get, &url, &headers, None) + .await + .with_context(|| format!("Failed to fetch the GitHub App bot account {login}"))?; + match resp.status { + 200 => {} + 404 => bail!( + "GitHub has no bot account for App slug {slug}; check server.integrations.github.slug" + ), + status => bail!("Unexpected status {status} fetching the GitHub App bot account {login}"), + } + let account: AccountResponse = resp + .json() + .with_context(|| format!("Failed to parse the GitHub App bot account {login}"))?; + let account = validated(&account, "the GitHub App bot account")?; + if account.login != login { + bail!( + "GitHub returned account {} for App slug {slug}; expected {login}", + account.login + ); + } + Ok(account) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::tests_mock::{MockHttpClient, test_rsa_key}; + + fn app(slug: Option<&str>) -> GitHubAppCredentials { + GitHubAppCredentials { + app_id: "12345".to_string(), + private_key_pem: test_rsa_key().to_string(), + slug: slug.map(str::to_string), + } + } + + #[test] + fn noreply_email_uses_id_plus_login() { + let identity = GitHubAccountIdentity { + login: "fabro-sh[bot]".to_string(), + id: 281_434_857, + }; + assert_eq!( + identity.noreply_email(), + "281434857+fabro-sh[bot]@users.noreply.github.com" + ); + } + + #[tokio::test] + async fn token_identity_reads_login_and_id_with_bearer_auth() { + let mock = MockHttpClient::new() + .on( + HttpMethod::Get, + "/user", + 200, + r#"{"login":"octocat","id":583231,"type":"User","email":null}"#, + ) + .with_req_header("Authorization", "Bearer ghp_secret"); + + let identity = lookup_token_identity(&mock, "ghp_secret", "") + .await + .unwrap(); + assert_eq!(identity, GitHubAccountIdentity { + login: "octocat".to_string(), + id: 583_231, + }); + assert_eq!(mock.request_count(), 1); + } + + #[tokio::test] + async fn token_identity_rejects_unauthorized_tokens() { + let mock = MockHttpClient::new().on(HttpMethod::Get, "/user", 401, "{}"); + let err = lookup_token_identity(&mock, "ghp_bad", "") + .await + .unwrap_err(); + assert!( + err.to_string() + .contains("rejected the configured GITHUB_TOKEN"), + "{err:#}" + ); + } + + #[tokio::test] + async fn token_identity_rejects_malformed_bodies() { + let mock = MockHttpClient::new().on(HttpMethod::Get, "/user", 200, r#"{"login":""}"#); + let err = lookup_token_identity(&mock, "ghp_x", "").await.unwrap_err(); + let chain = err.chain().map(ToString::to_string).collect::>(); + assert!( + chain + .iter() + .any(|message| message.contains("Failed to parse the GitHub token's user")), + "{chain:?}" + ); + + let mock = + MockHttpClient::new().on(HttpMethod::Get, "/user", 200, r#"{"login":" ","id":7}"#); + let err = lookup_token_identity(&mock, "ghp_x", "").await.unwrap_err(); + assert!(err.to_string().contains("empty login"), "{err:#}"); + + let mock = MockHttpClient::new().on( + HttpMethod::Get, + "/user", + 200, + r#"{"login":"evil\nname","id":7}"#, + ); + let err = lookup_token_identity(&mock, "ghp_x", "").await.unwrap_err(); + assert!(err.to_string().contains("invalid login"), "{err:#}"); + } + + #[tokio::test] + async fn token_identity_surfaces_transient_failures() { + let mock = MockHttpClient::new().on(HttpMethod::Get, "/user", 502, "bad gateway"); + let err = lookup_token_identity(&mock, "ghp_x", "").await.unwrap_err(); + assert!(err.to_string().contains("Unexpected status 502"), "{err:#}"); + } + + #[tokio::test] + async fn app_bot_identity_uses_configured_slug_and_bearer() { + let mock = MockHttpClient::new() + .on( + HttpMethod::Get, + "/users/my-app%5Bbot%5D", + 200, + r#"{"login":"my-app[bot]","id":4242,"type":"Bot"}"#, + ) + .with_req_header("Authorization", "Bearer ghs_installation"); + + let identity = + lookup_app_bot_identity(&mock, &app(Some("my-app")), "", Some("ghs_installation")) + .await + .unwrap(); + assert_eq!(identity.login, "my-app[bot]"); + assert_eq!(identity.id, 4242); + assert_eq!( + identity.noreply_email(), + "4242+my-app[bot]@users.noreply.github.com" + ); + // The configured slug skips the `/app` lookup entirely. + assert_eq!(mock.request_count(), 1); + } + + #[tokio::test] + async fn app_bot_identity_discovers_the_slug_when_not_configured() { + let mock = MockHttpClient::new() + .on( + HttpMethod::Get, + "/app", + 200, + r#"{"slug":"discovered-app","owner":{"login":"org"}}"#, + ) + .on( + HttpMethod::Get, + "/users/discovered-app%5Bbot%5D", + 200, + r#"{"login":"discovered-app[bot]","id":99}"#, + ); + + let identity = lookup_app_bot_identity(&mock, &app(None), "", None) + .await + .unwrap(); + assert_eq!(identity.login, "discovered-app[bot]"); + assert_eq!(mock.request_count(), 2); + } + + #[tokio::test] + async fn app_bot_identity_fails_when_the_bot_account_is_missing() { + let mock = MockHttpClient::new().on(HttpMethod::Get, "/users/nope%5Bbot%5D", 404, "{}"); + let err = lookup_app_bot_identity(&mock, &app(Some("nope")), "", None) + .await + .unwrap_err(); + assert!( + err.to_string().contains("no bot account for App slug nope"), + "{err:#}" + ); + } + + #[tokio::test] + async fn app_bot_identity_rejects_a_mismatched_login() { + let mock = MockHttpClient::new().on( + HttpMethod::Get, + "/users/my-app%5Bbot%5D", + 200, + r#"{"login":"someone-else","id":5}"#, + ); + let err = lookup_app_bot_identity(&mock, &app(Some("my-app")), "", None) + .await + .unwrap_err(); + assert!(err.to_string().contains("expected my-app[bot]"), "{err:#}"); + } + + #[tokio::test] + async fn app_bot_identity_preserves_the_app_lookup_error_chain() { + let mock = MockHttpClient::new().on(HttpMethod::Get, "/app", 401, ""); + let err = lookup_app_bot_identity(&mock, &app(None), "", None) + .await + .unwrap_err(); + let chain = err.chain().map(ToString::to_string).collect::>(); + assert!( + chain + .iter() + .any(|m| m.contains("Failed to determine the GitHub App slug")), + "{chain:?}" + ); + assert!( + chain.iter().any(|m| m.contains("authentication failed")), + "{chain:?}" + ); + } +} diff --git a/lib/components/fabro-github/src/lib.rs b/lib/components/fabro-github/src/lib.rs index 76ed229eb..9505b4e7c 100644 --- a/lib/components/fabro-github/src/lib.rs +++ b/lib/components/fabro-github/src/lib.rs @@ -12,6 +12,7 @@ use tokio::process::Command; use crate::token_source::SecretString; pub mod access; +pub mod identity; pub mod token_source; #[cfg(any(test, feature = "test-support"))] @@ -20,6 +21,7 @@ pub mod test_support; pub(crate) mod tests_mock; pub use access::GitHubRepositoryAccess; +pub use identity::GitHubAccountIdentity; pub const GITHUB_API_BASE_URL: &str = "https://api.github.com"; @@ -491,7 +493,7 @@ pub fn sign_app_jwt(app_id: &str, private_key_pem: &str) -> anyhow::Result [(&str, &str); 3] { +pub(crate) fn github_headers(auth: &str) -> [(&str, &str); 3] { [ ("Authorization", auth), ("Accept", "application/vnd.github+json"), diff --git a/lib/components/fabro-store/src/run_state.rs b/lib/components/fabro-store/src/run_state.rs index 8f0f641a0..5b61261a5 100644 --- a/lib/components/fabro-store/src/run_state.rs +++ b/lib/components/fabro-store/src/run_state.rs @@ -356,6 +356,9 @@ impl RunProjectionReducer for RunProjection { duration_ms: props.duration_ms, })); } + EventBody::GitIdentityResolved(props) => { + self.git_identity = Some(props.identity.clone()); + } EventBody::SandboxInitialized(props) => { let plan = sandbox_plan_from_projection_or_settings(self); self.sandbox = Some(RunSandbox::ready(plan, RunSandboxInstance { diff --git a/lib/components/fabro-workflow/src/event/convert.rs b/lib/components/fabro-workflow/src/event/convert.rs index 012f2485e..ced1dd4ae 100644 --- a/lib/components/fabro-workflow/src/event/convert.rs +++ b/lib/components/fabro-workflow/src/event/convert.rs @@ -738,6 +738,11 @@ fn event_body_from_event(event: &Event) -> EventBody { duration_ms: *duration_ms, }) } + Event::GitIdentityResolved { identity } => { + EventBody::GitIdentityResolved(fabro_types::GitIdentityResolvedProps { + identity: identity.clone(), + }) + } Event::SetupFailed { command, index, diff --git a/lib/components/fabro-workflow/src/event/events.rs b/lib/components/fabro-workflow/src/event/events.rs index c486953ee..67ab3ca45 100644 --- a/lib/components/fabro-workflow/src/event/events.rs +++ b/lib/components/fabro-workflow/src/event/events.rs @@ -564,6 +564,11 @@ pub enum Event { SetupCompleted { duration_ms: u64, }, + /// The run resolved the Git author/committer identity it uses for every + /// commit: engine checkpoints, metadata commits, and workflow commands. + GitIdentityResolved { + identity: ::fabro_types::GitIdentity, + }, SetupFailed { command: String, index: usize, @@ -1420,6 +1425,14 @@ impl Event { Self::SetupCompleted { duration_ms } => { info!(duration_ms, "Setup completed"); } + Self::GitIdentityResolved { identity } => { + info!( + name = %identity.name, + email = %identity.email, + source = %identity.source, + "Git identity resolved" + ); + } Self::SetupFailed { command, index, diff --git a/lib/components/fabro-workflow/src/event/names.rs b/lib/components/fabro-workflow/src/event/names.rs index a6ba1d4eb..6cf93a50b 100644 --- a/lib/components/fabro-workflow/src/event/names.rs +++ b/lib/components/fabro-workflow/src/event/names.rs @@ -78,6 +78,7 @@ pub fn event_name(event: &Event) -> Cow<'static, str> { Event::SetupCommandStarted { .. } => "setup.command.started", Event::SetupCommandCompleted { .. } => "setup.command.completed", Event::SetupCompleted { .. } => "setup.completed", + Event::GitIdentityResolved { .. } => "git.identity.resolved", Event::SetupFailed { .. } => "setup.failed", Event::StallWatchdogTimeout { .. } => "watchdog.timeout", Event::ArtifactCaptured { .. } => "artifact.captured", diff --git a/lib/components/fabro-workflow/src/git_identity.rs b/lib/components/fabro-workflow/src/git_identity.rs new file mode 100644 index 000000000..0b5afb9bc --- /dev/null +++ b/lib/components/fabro-workflow/src/git_identity.rs @@ -0,0 +1,329 @@ +//! One Git author and committer identity per run. +//! +//! The run resolves its identity once, after its GitHub credentials are +//! selected and before anything can commit, then uses it everywhere: engine +//! checkpoints and metadata commits read it through +//! [`RunOptions::git_author`](crate::run_options::RunOptions::git_author), +//! and every workflow command, prepare step, native agent shell tool, and ACP +//! agent launch receives it as the four `GIT_AUTHOR_*` / `GIT_COMMITTER_*` +//! variables so plain `git commit` inside the sandbox agrees with the engine. +//! +//! Resolution order: an explicit, complete `run.git.author`; the run's GitHub +//! App bot account; the authenticated user of the run's GitHub PAT; the +//! generic Fabro identity. A partial `run.git.author` overlays the fields it +//! supplies on whichever identity the credentials resolve to. Only the run's +//! selected credentials are consulted: a lookup failure for them is a setup +//! error, never a silent change of author. + +use std::collections::HashMap; +use std::sync::Arc; +use std::time::Duration; + +use anyhow::Context as _; +use fabro_github::token_source::InstallationTokenSource; +use fabro_github::{GitHubCredentials, identity}; +use fabro_types::settings::run::GitAuthorSettings; +use fabro_types::{GitIdentity, GitIdentitySource, WorkflowSettings}; +use tokio::time::timeout; + +use crate::error::Error; + +/// Environment variables Git reads for the author and committer. +pub const GIT_IDENTITY_ENV_KEYS: [&str; 4] = [ + "GIT_AUTHOR_NAME", + "GIT_AUTHOR_EMAIL", + "GIT_COMMITTER_NAME", + "GIT_COMMITTER_EMAIL", +]; + +/// Upper bound on one identity lookup against the GitHub API. +const LOOKUP_TIMEOUT: Duration = Duration::from_secs(30); + +/// The outcome of resolving a run's identity. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ResolvedGitIdentity { + pub identity: GitIdentity, + /// Set when the selected credentials were a standalone installation + /// token whose App bot account cannot be determined; the identity fell + /// back to the generic Fabro identity (plus any explicit fields). + pub warning: Option, +} + +/// The explicit `run.git.author` fields, trimmed; empty values count as unset. +fn explicit_fields(settings: &WorkflowSettings) -> (Option, Option) { + let author: Option<&GitAuthorSettings> = settings.run.git.author.as_ref(); + let field = |value: Option<&String>| { + value + .map(|value| value.trim()) + .filter(|value| !value.is_empty()) + .map(str::to_string) + }; + ( + field(author.and_then(|author| author.name.as_ref())), + field(author.and_then(|author| author.email.as_ref())), + ) +} + +/// Overlay explicit fields on a resolved identity. The source stays that of +/// the resolved identity unless both fields are explicit. +fn overlay(mut identity: GitIdentity, name: Option, email: Option) -> GitIdentity { + if name.is_some() && email.is_some() { + identity.source = GitIdentitySource::Explicit; + } + if let Some(name) = name { + identity.name = name; + } + if let Some(email) = email { + identity.email = email; + } + identity +} + +/// Resolve the run's Git identity from its settings and selected credentials. +/// +/// `github_token` is the run's managed token source, used only as the bearer +/// for the App bot-account lookup (that endpoint rejects App JWTs). +pub async fn resolve_git_identity( + settings: &WorkflowSettings, + credentials: Option<&GitHubCredentials>, + github_token: Option<&Arc>, +) -> Result { + let (name, email) = explicit_fields(settings); + if let (Some(name), Some(email)) = (name.clone(), email.clone()) { + return Ok(ResolvedGitIdentity { + identity: GitIdentity { + name, + email, + source: GitIdentitySource::Explicit, + }, + warning: None, + }); + } + + let (credential_identity, warning) = match credentials { + None => (GitIdentity::fabro_default(), None), + Some(GitHubCredentials::Installation(_)) => ( + GitIdentity::fabro_default(), + Some( + "The run's GitHub credential is a standalone installation token whose App bot \ + account cannot be determined; commits use the generic Fabro identity." + .to_string(), + ), + ), + Some(credentials) => ( + lookup_credential_identity(credentials, github_token) + .await + .map_err(|err| { + Error::engine_with_anyhow("Failed to resolve the run's Git identity", err) + })?, + None, + ), + }; + + Ok(ResolvedGitIdentity { + identity: overlay(credential_identity, name, email), + warning, + }) +} + +async fn lookup_credential_identity( + credentials: &GitHubCredentials, + github_token: Option<&Arc>, +) -> anyhow::Result { + let client = fabro_http::http_client() + .map_err(anyhow::Error::new) + .context("building HTTP client for GitHub identity lookup")?; + let base_url = fabro_github::github_api_base_url(); + let lookup = async { + match credentials { + GitHubCredentials::App(app) => { + let bearer = match github_token { + Some(source) => Some( + source + .resolve() + .await + .context("resolving the GitHub token for the App bot lookup")?, + ), + None => None, + }; + let account = identity::lookup_app_bot_identity( + &client, + app, + &base_url, + bearer.as_ref().map(|token| token.token.expose()), + ) + .await?; + Ok::<_, anyhow::Error>(GitIdentity { + email: account.noreply_email(), + name: account.login, + source: GitIdentitySource::GithubApp, + }) + } + GitHubCredentials::Pat(token) => { + let account = identity::lookup_token_identity(&client, token, &base_url).await?; + Ok(GitIdentity { + email: account.noreply_email(), + name: account.login, + source: GitIdentitySource::GithubPat, + }) + } + GitHubCredentials::Installation(_) => { + unreachable!("installation tokens never reach the credential lookup") + } + } + }; + timeout(LOOKUP_TIMEOUT, lookup) + .await + .context("GitHub identity lookup timed out")? +} + +/// The four Git environment variables for `identity`. +#[must_use] +pub fn git_identity_env(identity: &GitIdentity) -> [(&'static str, String); 4] { + [ + ("GIT_AUTHOR_NAME", identity.name.clone()), + ("GIT_AUTHOR_EMAIL", identity.email.clone()), + ("GIT_COMMITTER_NAME", identity.name.clone()), + ("GIT_COMMITTER_EMAIL", identity.email.clone()), + ] +} + +/// Set the identity variables on `env`, replacing any existing values so the +/// run's identity wins over inherited host variables and conflicting run or +/// step environment entries. +pub fn apply_git_identity_env(env: &mut HashMap, identity: &GitIdentity) { + for (key, value) in git_identity_env(identity) { + env.insert(key.to_string(), value); + } +} + +#[cfg(test)] +mod tests { + use fabro_types::settings::run::GitAuthorSettings; + + use super::*; + + fn settings(name: Option<&str>, email: Option<&str>) -> WorkflowSettings { + let mut settings = WorkflowSettings::default(); + settings.run.git.author = Some(GitAuthorSettings { + name: name.map(str::to_string), + email: email.map(str::to_string), + }); + settings + } + + fn installation() -> GitHubCredentials { + GitHubCredentials::Installation(fabro_github::InstallationToken { + token: "ghs_token".to_string(), + expires_at: chrono::Utc::now() + chrono::Duration::hours(1), + }) + } + + #[tokio::test] + async fn no_credentials_use_the_generic_identity_without_a_lookup() { + let resolved = resolve_git_identity(&WorkflowSettings::default(), None, None) + .await + .unwrap(); + assert_eq!(resolved.identity, GitIdentity::fabro_default()); + assert_eq!(resolved.identity.source, GitIdentitySource::Default); + assert!(resolved.warning.is_none()); + } + + #[tokio::test] + async fn complete_explicit_author_skips_credential_lookup() { + // A PAT lookup would need the network; a complete explicit author + // must never get that far. + let creds = GitHubCredentials::Pat("ghp_never_used".to_string()); + let resolved = resolve_git_identity( + &settings(Some("Release Bot"), Some("release@example.com")), + Some(&creds), + None, + ) + .await + .unwrap(); + assert_eq!(resolved.identity, GitIdentity { + name: "Release Bot".to_string(), + email: "release@example.com".to_string(), + source: GitIdentitySource::Explicit, + }); + } + + #[tokio::test] + async fn partial_explicit_author_overlays_the_resolved_identity() { + let resolved = resolve_git_identity(&settings(Some("Only Name"), None), None, None) + .await + .unwrap(); + assert_eq!(resolved.identity, GitIdentity { + name: "Only Name".to_string(), + email: GitIdentity::DEFAULT_EMAIL.to_string(), + source: GitIdentitySource::Default, + }); + + let resolved = resolve_git_identity(&settings(None, Some("only@example.com")), None, None) + .await + .unwrap(); + assert_eq!(resolved.identity.name, GitIdentity::DEFAULT_NAME); + assert_eq!(resolved.identity.email, "only@example.com"); + } + + #[tokio::test] + async fn blank_explicit_fields_count_as_unset() { + let resolved = resolve_git_identity(&settings(Some(" "), Some("")), None, None) + .await + .unwrap(); + assert_eq!(resolved.identity, GitIdentity::fabro_default()); + } + + #[tokio::test] + async fn standalone_installation_token_falls_back_with_a_warning() { + let resolved = + resolve_git_identity(&WorkflowSettings::default(), Some(&installation()), None) + .await + .unwrap(); + assert_eq!(resolved.identity, GitIdentity::fabro_default()); + let warning = resolved.warning.expect("fallback should warn"); + assert!( + warning.contains("standalone installation token"), + "{warning}" + ); + } + + #[tokio::test] + async fn standalone_installation_token_keeps_explicit_fields() { + let resolved = resolve_git_identity( + &settings(None, Some("pinned@example.com")), + Some(&installation()), + None, + ) + .await + .unwrap(); + assert_eq!(resolved.identity.name, GitIdentity::DEFAULT_NAME); + assert_eq!(resolved.identity.email, "pinned@example.com"); + assert_eq!(resolved.identity.source, GitIdentitySource::Default); + assert!(resolved.warning.is_some()); + } + + #[test] + fn identity_env_replaces_conflicting_entries() { + let identity = GitIdentity { + name: "fabro-bot[bot]".to_string(), + email: "7+fabro-bot[bot]@users.noreply.github.com".to_string(), + source: GitIdentitySource::GithubApp, + }; + let mut env = HashMap::from([ + ("GIT_AUTHOR_NAME".to_string(), "someone else".to_string()), + ( + "GIT_COMMITTER_EMAIL".to_string(), + "x@example.com".to_string(), + ), + ("KEEP".to_string(), "1".to_string()), + ]); + apply_git_identity_env(&mut env, &identity); + assert_eq!(env["GIT_AUTHOR_NAME"], "fabro-bot[bot]"); + assert_eq!(env["GIT_AUTHOR_EMAIL"], identity.email); + assert_eq!(env["GIT_COMMITTER_NAME"], "fabro-bot[bot]"); + assert_eq!(env["GIT_COMMITTER_EMAIL"], identity.email); + assert_eq!(env["KEEP"], "1"); + assert_eq!(env.len(), 5); + } +} diff --git a/lib/components/fabro-workflow/src/handler/manager_loop.rs b/lib/components/fabro-workflow/src/handler/manager_loop.rs index 92969ddeb..5b4988a78 100644 --- a/lib/components/fabro-workflow/src/handler/manager_loop.rs +++ b/lib/components/fabro-workflow/src/handler/manager_loop.rs @@ -211,6 +211,7 @@ impl Handler for SubWorkflowHandler { fork_source_ref: None, base_branch: None, display_base_sha: None, + git_identity: services.git_identity.clone(), git: None, }; @@ -230,6 +231,7 @@ impl Handler for SubWorkflowHandler { let interviewer = Arc::clone(&services.interviewer); let base_env = services.base_env.clone(); let github_token = services.github_token.clone(); + let git_identity = services.git_identity.clone(); let inputs = services.inputs.clone(); let dry_run = services.dry_run; let workflow_bundle = services.workflow_bundle.clone(); @@ -259,6 +261,7 @@ impl Handler for SubWorkflowHandler { interviewer, base_env, github_token, + git_identity, inputs, dry_run, workflow_path: child_workflow_path, diff --git a/lib/components/fabro-workflow/src/lib.rs b/lib/components/fabro-workflow/src/lib.rs index 7650cec63..58b1dfa01 100644 --- a/lib/components/fabro-workflow/src/lib.rs +++ b/lib/components/fabro-workflow/src/lib.rs @@ -295,6 +295,7 @@ pub mod event; pub mod file_resolver; pub mod git; pub(crate) mod git_bridge; +pub mod git_identity; pub(crate) mod graph; pub mod handler; mod hook_context; diff --git a/lib/components/fabro-workflow/src/lifecycle/git.rs b/lib/components/fabro-workflow/src/lifecycle/git.rs index 61095ae90..452197984 100644 --- a/lib/components/fabro-workflow/src/lifecycle/git.rs +++ b/lib/components/fabro-workflow/src/lifecycle/git.rs @@ -756,6 +756,7 @@ mod tests { fork_source_ref: None, base_branch: None, display_base_sha: None, + git_identity: None, git: Some(GitCheckpointOptions { base_sha: None, run_branch: None, diff --git a/lib/components/fabro-workflow/src/operations/start.rs b/lib/components/fabro-workflow/src/operations/start.rs index 8395d765f..e0c425845 100644 --- a/lib/components/fabro-workflow/src/operations/start.rs +++ b/lib/components/fabro-workflow/src/operations/start.rs @@ -919,6 +919,7 @@ impl RunSession { fork_source_ref: record.fork_source_ref.clone(), base_branch: record.base_branch().map(str::to_string), display_base_sha: None, + git_identity: None, git: self.git.clone(), }; diff --git a/lib/components/fabro-workflow/src/pipeline/execute/tests.rs b/lib/components/fabro-workflow/src/pipeline/execute/tests.rs index 5622d8304..23409afc2 100644 --- a/lib/components/fabro-workflow/src/pipeline/execute/tests.rs +++ b/lib/components/fabro-workflow/src/pipeline/execute/tests.rs @@ -115,6 +115,7 @@ fn test_run_options(run_dir: &Path, run_id: &str) -> RunOptions { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, workflow_slug: None, } } diff --git a/lib/components/fabro-workflow/src/pipeline/finalize.rs b/lib/components/fabro-workflow/src/pipeline/finalize.rs index ad87e3a8e..48ef1c6a1 100644 --- a/lib/components/fabro-workflow/src/pipeline/finalize.rs +++ b/lib/components/fabro-workflow/src/pipeline/finalize.rs @@ -617,6 +617,7 @@ mod tests { fork_source_ref: None, base_branch: None, display_base_sha: None, + git_identity: None, git: None, } } diff --git a/lib/components/fabro-workflow/src/pipeline/initialize.rs b/lib/components/fabro-workflow/src/pipeline/initialize.rs index 39fc772df..8c3e8610a 100644 --- a/lib/components/fabro-workflow/src/pipeline/initialize.rs +++ b/lib/components/fabro-workflow/src/pipeline/initialize.rs @@ -17,15 +17,13 @@ use fabro_static::EnvVars; use fabro_types::RunSandboxKind; use fabro_util::time::elapsed_ms; use fabro_vault::Vault; -use sandbox_driver::{CorrelationId, EventContext, Git as _}; +use sandbox_driver::{CorrelationId, EventContext}; use tokio::runtime::Handle; use tokio::sync::RwLock as AsyncRwLock; use super::types::{InitOptions, Initialized, LlmSpec, Persisted, SandboxEnvSpec}; use crate::error::Error; use crate::event::{DriverEventRecorder, Event, RunNoticeCode, RunNoticeLevel, SandboxLifecycle}; -use crate::git::GitAuthor; -use crate::git_bridge; use crate::handler::llm::{AgentAcpBackend, BackendRouter, PebbleBackend, routing}; use crate::handler::{HandlerRegistry, default_registry}; #[cfg(test)] @@ -39,6 +37,7 @@ use crate::services::{ use crate::stage_execution::{StageExecutionSeed, StageExecutionTracker}; use crate::steering_hub::SteeringHub; use crate::web_search::SearchSecrets; +use crate::{git_bridge, git_identity}; struct BuiltSandboxEnv { env: HashMap, @@ -75,20 +74,47 @@ fn git_setup_intent(run_options: &RunOptions) -> GitSetupIntent { } } -async fn configure_sandbox_git_identity( - sandbox: &RunSandbox, - author: &GitAuthor, -) -> Result<(), Error> { - let git = sandbox - .git() - .map_err(|err| Error::engine_with_source("Sandbox git identity setup failed", err))?; - let repo = sandbox.working_directory(); - for (key, value) in [("user.name", &author.name), ("user.email", &author.email)] { - git.config_set(repo, key, value) - .await - .map_err(|err| Error::engine_with_source("Sandbox git identity setup failed", err))?; +/// Resolve the run's Git identity once, before anything can commit. +/// +/// A resumed run reuses the identity it recorded at first initialization so +/// a token refresh or credential rotation never changes authorship mid-run; +/// runs recorded before identity tracking resolve on their next execution. +async fn resolve_run_git_identity( + options: &InitOptions, + is_resume: bool, + github_token: Option<&Arc>, +) -> Result { + if let Some(identity) = options.run_options.git_identity.clone() { + return Ok(identity); } - Ok(()) + if is_resume { + let recorded = options + .run_store + .state() + .await + .map_err(|err| Error::engine_with_anyhow("Failed to load run state", err))? + .git_identity; + if let Some(identity) = recorded { + return Ok(identity); + } + } + let resolved = git_identity::resolve_git_identity( + &options.run_options.settings, + options.run_options.github_app.as_ref(), + github_token, + ) + .await?; + if let Some(warning) = resolved.warning { + options.emitter.notice( + RunNoticeLevel::Warn, + RunNoticeCode::GitIdentityFallback, + warning, + ); + } + options.emitter.emit(&Event::GitIdentityResolved { + identity: resolved.identity.clone(), + }); + Ok(resolved.identity) } fn build_sandbox_env( @@ -547,9 +573,12 @@ pub async fn initialize( github_token, github_access: _, } = built_env; + let git_identity = resolve_run_git_identity(&options, is_resume, github_token.as_ref()).await?; + options.run_options.git_identity = Some(git_identity.clone()); let tool_env_provider = Arc::new(WorkflowToolEnvProvider { base_env: base_env.clone(), github_token: github_token.clone(), + git_identity: Some(git_identity.clone()), }); let github_token_refresh_managed = github_token .as_deref() @@ -633,11 +662,6 @@ pub async fn initialize( } } } - if sandbox.origin_url().is_some() { - let git_author = options.run_options.git_author(); - configure_sandbox_git_identity(sandbox.as_ref(), &git_author).await?; - } - if !options.lifecycle.setup_commands.is_empty() { options.emitter.emit(&Event::SetupStarted { command_count: options.lifecycle.setup_commands.len(), @@ -651,13 +675,14 @@ pub async fn initialize( }); let cmd_start = Instant::now(); let cancel_token = options.run_options.cancel_token.child_token(); - let step_env = (!setup.env.is_empty()).then_some(&setup.env); + let mut step_env = setup.env.clone(); + git_identity::apply_git_identity_env(&mut step_env, &git_identity); let result = sandbox .exec_command( command, options.lifecycle.setup_command_timeout_ms, None, - step_env, + Some(&step_env), Some(cancel_token.clone()), ) .await @@ -733,6 +758,7 @@ pub async fn initialize( interviewer: Arc::clone(&options.interviewer), base_env, github_token, + git_identity: Some(git_identity), inputs: options.run_options.settings.run.inputs.clone(), dry_run: options.dry_run, workflow_path: options.workflow_path.clone(), @@ -922,6 +948,7 @@ mod tests { fork_source_ref: None, base_branch: None, display_base_sha: None, + git_identity: None, git: None, } } @@ -1056,28 +1083,181 @@ mod tests { } #[tokio::test] - async fn configure_sandbox_git_identity_uses_run_author() { - let sandbox = fabro_sandbox::test_support::MockSandbox::linux(); - let author = GitAuthor::from_options( - Some("Fabro Bot".to_string()), - Some("fabro-bot@example.com".to_string()), + async fn initialize_resolves_the_generic_identity_without_credentials() { + let temp = tempfile::tempdir().unwrap(); + let run_dir = temp.path().join("run"); + std::fs::create_dir_all(&run_dir).unwrap(); + let (graph, source) = simple_graph(); + let persisted = test_persisted(graph, source, &run_dir); + let emitter = Arc::new(crate::event::Emitter::new(test_run_id())); + let seen = Arc::new(std::sync::Mutex::new(Vec::new())); + emitter.on_event({ + let seen = Arc::clone(&seen); + move |event| seen.lock().unwrap().push(event.clone()) + }); + + let run_store = memory_store().create_run(&test_run_id()).await.unwrap(); + let initialized = initialize( + persisted, + test_init_options( + run_store.into(), + emitter, + std::env::current_dir().unwrap(), + test_settings(&run_dir), + ), + ) + .await + .unwrap(); + + let expected = fabro_types::GitIdentity::fabro_default(); + assert_eq!(initialized.run_options.git_identity, Some(expected.clone())); + assert_eq!(initialized.engine.git_identity, Some(expected.clone())); + assert_eq!( + initialized.run_options.git_author(), + crate::git::GitAuthor::from(&expected) ); - - configure_sandbox_git_identity(&sandbox.sandbox(), &author) - .await - .expect("git identity should configure"); - - let commands = sandbox.driver().scripted_exec().commands(); - assert_eq!(commands.len(), 2, "{commands:#?}"); + let resolved = seen + .lock() + .unwrap() + .iter() + .find_map(|event| match &event.body { + fabro_types::EventBody::GitIdentityResolved(props) => Some(props.identity.clone()), + _ => None, + }) + .expect("initialize should record the resolved identity"); + assert_eq!(resolved, expected); assert!( - commands[0].contains("'config' '--local' '--' 'user.name' 'Fabro Bot'"), - "{}", - commands[0] + !seen + .lock() + .unwrap() + .iter() + .any(|event| event.event_name() == "run.notice"), + "the generic identity without credentials is not a fallback warning" + ); + } + + #[tokio::test] + async fn initialize_overlays_partial_explicit_author_on_the_generic_identity() { + let temp = tempfile::tempdir().unwrap(); + let run_dir = temp.path().join("run"); + std::fs::create_dir_all(&run_dir).unwrap(); + let (graph, source) = simple_graph(); + let mut settings = WorkflowSettings::default(); + settings.run.git.author = Some(fabro_types::settings::run::GitAuthorSettings { + name: Some("Release Bot".to_string()), + email: None, + }); + let persisted = test_persisted_run(graph, source, &run_dir, settings.clone(), None); + let emitter = Arc::new(crate::event::Emitter::new(test_run_id())); + let mut run_options = test_settings(&run_dir); + run_options.settings = settings; + + let run_store = memory_store().create_run(&test_run_id()).await.unwrap(); + let initialized = initialize( + persisted, + test_init_options( + run_store.into(), + emitter, + std::env::current_dir().unwrap(), + run_options, + ), + ) + .await + .unwrap(); + + assert_eq!( + initialized.run_options.git_identity, + Some(fabro_types::GitIdentity { + name: "Release Bot".to_string(), + email: fabro_types::GitIdentity::DEFAULT_EMAIL.to_string(), + source: fabro_types::GitIdentitySource::Default, + }) + ); + } + + #[tokio::test] + async fn initialize_warns_and_falls_back_for_a_standalone_installation_token() { + let temp = tempfile::tempdir().unwrap(); + let run_dir = temp.path().join("run"); + std::fs::create_dir_all(&run_dir).unwrap(); + let (graph, source) = simple_graph(); + let persisted = test_persisted(graph, source, &run_dir); + let emitter = Arc::new(crate::event::Emitter::new(test_run_id())); + let seen = Arc::new(std::sync::Mutex::new(Vec::new())); + emitter.on_event({ + let seen = Arc::clone(&seen); + move |event| seen.lock().unwrap().push(event.clone()) + }); + let mut run_options = test_settings(&run_dir); + run_options.github_app = Some(fabro_github::GitHubCredentials::Installation( + fabro_github::InstallationToken { + token: "ghs_token".to_string(), + expires_at: chrono::Utc::now() + chrono::Duration::hours(1), + }, + )); + + let run_store = memory_store().create_run(&test_run_id()).await.unwrap(); + let initialized = initialize( + persisted, + test_init_options( + run_store.into(), + emitter, + std::env::current_dir().unwrap(), + run_options, + ), + ) + .await + .unwrap(); + + assert_eq!( + initialized.run_options.git_identity, + Some(fabro_types::GitIdentity::fabro_default()) + ); + let notice = seen + .lock() + .unwrap() + .iter() + .find_map(|event| match &event.body { + fabro_types::EventBody::RunNotice(props) => Some(props.clone()), + _ => None, + }) + .expect("standalone installation token should warn"); + assert_eq!(notice.code, RunNoticeCode::GitIdentityFallback.to_string()); + assert_eq!(notice.level, RunNoticeLevel::Warn); + } + + /// The setup step's own env names a different author; the run's identity + /// must still win, and it must reach the shell even though the working + /// directory has no Git origin. + #[tokio::test] + async fn initialize_injects_the_git_identity_into_setup_commands() { + let setup = crate::run_options::SetupCommand { + command: format!( + "test \"$GIT_AUTHOR_NAME\" = {name} && test \"$GIT_AUTHOR_EMAIL\" = {email} && \ + test \"$GIT_COMMITTER_NAME\" = {name} && test \"$GIT_COMMITTER_EMAIL\" = {email}", + name = fabro_types::GitIdentity::DEFAULT_NAME, + email = fabro_types::GitIdentity::DEFAULT_EMAIL, + ), + env: HashMap::from([ + ("GIT_AUTHOR_NAME".to_string(), "step-author".to_string()), + ( + "GIT_COMMITTER_EMAIL".to_string(), + "step@example.com".to_string(), + ), + ]), + }; + + let (result, events) = initialize_with_setup_step(setup).await; + + assert!( + result.is_ok(), + "setup should see the run's Git identity: {:?}", + result.err() ); assert!( - commands[1].contains("'config' '--local' '--' 'user.email' 'fabro-bot@example.com'"), - "{}", - commands[1] + events + .iter() + .any(|event| event.event_name() == "setup.completed") ); } @@ -1261,6 +1441,7 @@ mod tests { let tool_env_provider = Arc::new(WorkflowToolEnvProvider { base_env: HashMap::new(), github_token: None, + git_identity: None, }); let (_registry, effective_dry_run) = build_registry( &LlmSpec { diff --git a/lib/components/fabro-workflow/src/run_metadata.rs b/lib/components/fabro-workflow/src/run_metadata.rs index d058cbaa7..92b30bc9c 100644 --- a/lib/components/fabro-workflow/src/run_metadata.rs +++ b/lib/components/fabro-workflow/src/run_metadata.rs @@ -733,6 +733,7 @@ mod tests { fork_source_ref: None, base_branch: None, display_base_sha: None, + git_identity: None, git: Some(GitCheckpointOptions { base_sha: None, run_branch: None, diff --git a/lib/components/fabro-workflow/src/run_options.rs b/lib/components/fabro-workflow/src/run_options.rs index 7c4ca98d6..5d53e5fa7 100644 --- a/lib/components/fabro-workflow/src/run_options.rs +++ b/lib/components/fabro-workflow/src/run_options.rs @@ -2,7 +2,7 @@ use std::collections::HashMap; use std::path::PathBuf; use fabro_types::settings::run::{RunCheckpointSettings, RunMode}; -use fabro_types::{ForkSourceRef, GitContext, RunId, WorkflowSettings}; +use fabro_types::{ForkSourceRef, GitContext, GitIdentity, RunId, WorkflowSettings}; use tokio_util::sync::CancellationToken; use crate::git::{GitAuthor, git_author_from_settings}; @@ -43,6 +43,10 @@ pub struct RunOptions { pub display_base_sha: Option, /// Git checkpoint options; `None` means checkpointing disabled. pub git: Option, + /// The identity resolved for this run's commits. Set by initialization + /// before any commit can be created; `None` only before that point, where + /// `git_author()` falls back to the submitted settings without a lookup. + pub git_identity: Option, } impl RunOptions { @@ -54,8 +58,11 @@ impl RunOptions { &self.settings.run.checkpoint } + /// The author and committer identity for commits this run creates. pub fn git_author(&self) -> GitAuthor { - git_author_from_settings(&self.settings) + self.git_identity + .as_ref() + .map_or_else(|| git_author_from_settings(&self.settings), GitAuthor::from) } pub fn artifact_glob_patterns(&self) -> &[String] { diff --git a/lib/components/fabro-workflow/src/services.rs b/lib/components/fabro-workflow/src/services.rs index d5f48a5c7..b66859bd1 100644 --- a/lib/components/fabro-workflow/src/services.rs +++ b/lib/components/fabro-workflow/src/services.rs @@ -10,12 +10,13 @@ use fabro_interview::Interviewer; use fabro_llm::credentials::CredentialProvider; use fabro_llm::lithos_catalog::Catalog; use fabro_sandbox::RunSandbox; -use fabro_types::{ManifestPath, RunId}; +use fabro_types::{GitIdentity, ManifestPath, RunId}; use lithos_llm::catalog::ProviderId; use pebble_coding_agent::tools::{ToolEnvProvider, ToolError}; use tokio_util::sync::CancellationToken; use crate::event::Emitter; +use crate::git_identity; use crate::handler::HandlerRegistry; use crate::interview_runtime::RunInterviewBlocker; use crate::run_metadata::{RunMetadataRuntime, RunMetadataWriterHandle}; @@ -239,6 +240,9 @@ pub struct EngineServices { pub base_env: HashMap, /// GitHub token source used to inject `GITHUB_TOKEN` at the point of use. pub github_token: Option>, + /// The run's resolved Git identity, injected as the `GIT_AUTHOR_*` / + /// `GIT_COMMITTER_*` variables into every stage environment. + pub git_identity: Option, /// Typed values from `[run.inputs]`, available to prompt templates. pub inputs: HashMap, /// When true, handlers should skip real execution and return simulated @@ -252,7 +256,12 @@ pub struct EngineServices { impl EngineServices { pub async fn env_for_stage(&self) -> anyhow::Result> { - resolve_workflow_env(&self.base_env, self.github_token.as_ref()).await + resolve_workflow_env( + &self.base_env, + self.github_token.as_ref(), + self.git_identity.as_ref(), + ) + .await } /// Test-only default: empty registry and cross-phase services. @@ -340,6 +349,7 @@ impl EngineServices { interviewer: Arc::new(fabro_interview::AutoApproveInterviewer::engine()), base_env: HashMap::new(), github_token: None, + git_identity: None, inputs: HashMap::new(), dry_run: false, workflow_path: None, @@ -351,13 +361,21 @@ impl EngineServices { pub struct WorkflowToolEnvProvider { pub base_env: HashMap, pub github_token: Option>, + /// The run's resolved Git identity; see [`EngineServices::git_identity`]. + pub git_identity: Option, } impl WorkflowToolEnvProvider { /// The environment tool processes run with right now: the configured - /// sandbox env plus a fresh `GITHUB_TOKEN` when the run has one. + /// sandbox env, a fresh `GITHUB_TOKEN` when the run has one, and the + /// run's Git identity. pub async fn resolve(&self) -> anyhow::Result> { - resolve_workflow_env(&self.base_env, self.github_token.as_ref()).await + resolve_workflow_env( + &self.base_env, + self.github_token.as_ref(), + self.git_identity.as_ref(), + ) + .await } } @@ -373,6 +391,7 @@ impl ToolEnvProvider for WorkflowToolEnvProvider { async fn resolve_workflow_env( base_env: &HashMap, github_token: Option<&Arc>, + identity: Option<&GitIdentity>, ) -> anyhow::Result> { let mut env = base_env.clone(); if let Some(source) = github_token { @@ -382,6 +401,11 @@ async fn resolve_workflow_env( resolved.token.expose().to_owned(), ); } + // Applied last: the run's identity wins over any `[run.environment]` + // entry of the same name, so `run.git.author` stays the one control. + if let Some(identity) = identity { + git_identity::apply_git_identity_env(&mut env, identity); + } Ok(env) } @@ -416,12 +440,46 @@ mod tests { let provider = WorkflowToolEnvProvider { base_env: HashMap::from([("FOO".to_string(), "bar".to_string())]), github_token: None, + git_identity: None, }; let env = provider.resolve().await.unwrap(); assert_eq!(env.get("FOO").map(String::as_str), Some("bar")); assert!(!env.contains_key("GITHUB_TOKEN")); + assert!(!env.contains_key("GIT_AUTHOR_NAME")); + } + + #[tokio::test] + async fn workflow_tool_env_provider_git_identity_wins_over_base_env() { + let provider = WorkflowToolEnvProvider { + base_env: HashMap::from([ + ("GIT_AUTHOR_NAME".to_string(), "from-run-env".to_string()), + ( + "GIT_COMMITTER_EMAIL".to_string(), + "run@example.com".to_string(), + ), + ]), + github_token: None, + git_identity: Some(fabro_types::GitIdentity { + name: "octocat".to_string(), + email: "1+octocat@users.noreply.github.com".to_string(), + source: fabro_types::GitIdentitySource::GithubPat, + }), + }; + + let env = provider.resolve().await.unwrap(); + + assert_eq!(env["GIT_AUTHOR_NAME"], "octocat"); + assert_eq!( + env["GIT_AUTHOR_EMAIL"], + "1+octocat@users.noreply.github.com" + ); + assert_eq!(env["GIT_COMMITTER_NAME"], "octocat"); + assert_eq!( + env["GIT_COMMITTER_EMAIL"], + "1+octocat@users.noreply.github.com" + ); } #[tokio::test] @@ -429,6 +487,7 @@ mod tests { let provider = WorkflowToolEnvProvider { base_env: HashMap::from([("FOO".to_string(), "bar".to_string())]), github_token: Some(InstallationTokenSource::pat("ghp_pat".to_string())), + git_identity: None, }; let env = provider.resolve().await.unwrap(); @@ -454,6 +513,7 @@ mod tests { "owner/repo", Arc::new(FailingMinter), )), + git_identity: None, }; let err = format!("{:#}", provider.resolve().await.unwrap_err()); diff --git a/lib/components/fabro-workflow/src/test_support.rs b/lib/components/fabro-workflow/src/test_support.rs index 93b6a9f48..3e1268e9b 100644 --- a/lib/components/fabro-workflow/src/test_support.rs +++ b/lib/components/fabro-workflow/src/test_support.rs @@ -286,6 +286,7 @@ async fn initialized( interviewer: Arc::new(AutoApproveInterviewer::engine()), base_env: options.env, github_token: None, + git_identity: run_options.git_identity.clone(), inputs: run_options.settings.run.inputs.clone(), dry_run: run_options.dry_run_enabled(), workflow_path: None, @@ -355,6 +356,33 @@ pub async fn run_graph_with_state( Ok((outcome, state)) } +/// Run a graph with a `[run.environment]`-style base env and no hooks. +pub async fn run_graph_with_env( + registry: HandlerRegistry, + emitter: Arc, + sandbox: Arc, + graph: &GvGraph, + run_options: &RunOptions, + env: HashMap, +) -> Result { + let initialized = initialized( + registry, + emitter, + sandbox, + graph, + run_options, + InitializedOptions { + hook_runner: None, + env, + checkpoint: None, + llm_source: None, + }, + ) + .await; + let executed = execute_and_emit_terminal(initialized).await; + executed.outcome +} + pub async fn run_graph_with_hooks( registry: HandlerRegistry, emitter: Arc, diff --git a/lib/components/fabro-workflow/tests/it/daytona_integration.rs b/lib/components/fabro-workflow/tests/it/daytona_integration.rs index ed5cec4e6..35c3ae143 100644 --- a/lib/components/fabro-workflow/tests/it/daytona_integration.rs +++ b/lib/components/fabro-workflow/tests/it/daytona_integration.rs @@ -581,6 +581,7 @@ async fn daytona_pipeline_artifact_offload_and_sync() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -769,6 +770,7 @@ async fn daytona_git_checkpoint_remote_emits_events() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: Some(GitCheckpointOptions { @@ -913,6 +915,7 @@ async fn daytona_git_checkpoint_with_shadow_branch() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: Some(GitCheckpointOptions { @@ -1071,6 +1074,7 @@ async fn daytona_asset_collection() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -1343,6 +1347,7 @@ async fn daytona_git_push_run_branch_to_origin() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: Some(GitCheckpointOptions { diff --git a/lib/components/fabro-workflow/tests/it/git_integration.rs b/lib/components/fabro-workflow/tests/it/git_integration.rs index ccf8fcc90..3a87b8be3 100644 --- a/lib/components/fabro-workflow/tests/it/git_integration.rs +++ b/lib/components/fabro-workflow/tests/it/git_integration.rs @@ -14,10 +14,12 @@ use fabro_types::{RunEvent, WorkflowSettings, fixtures}; use fabro_workflow::event::Emitter; use fabro_workflow::git; use fabro_workflow::handler::HandlerRegistry; +use fabro_workflow::handler::command::CommandHandler; use fabro_workflow::handler::exit::ExitHandler; use fabro_workflow::handler::start::StartHandler; +use fabro_workflow::outcome::StageOutcome; use fabro_workflow::run_options::{GitCheckpointOptions, RunOptions}; -use fabro_workflow::test_support::run_graph; +use fabro_workflow::test_support::{run_graph, run_graph_with_env}; use sandbox_driver::{ Capabilities, DirEntry, Exec, FileMetadata, Filesystem, PlatformInfo, SandboxId, SandboxStatus, }; @@ -169,6 +171,7 @@ fn test_run_options(run_dir: &Path) -> RunOptions { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, workflow_slug: None, } } @@ -589,3 +592,236 @@ async fn remote_prompt_demotion_stays_outside_checkout_and_survives_checkpoint() oversized_bytes ); } + +// --------------------------------------------------------------------------- +// One Git identity per run: engine checkpoints and workflow commands agree. +// --------------------------------------------------------------------------- + +fn git_stdout(repo_dir: &Path, args: &[&str]) -> String { + let output = Command::new("git") + .args(args) + .current_dir(repo_dir) + .output() + .unwrap_or_else(|err| panic!("git {args:?} should run: {err}")); + assert_success(&output, &format!("git {args:?}")); + String::from_utf8(output.stdout) + .expect("git output should be UTF-8") + .trim() + .to_string() +} + +/// `author name`, `author email`, `committer name`, `committer email`. +fn commit_identity(repo_dir: &Path, rev: &str) -> Vec { + git_stdout(repo_dir, &[ + "show", + "-s", + "--format=%an%n%ae%n%cn%n%ce", + rev, + ]) + .lines() + .map(str::to_string) + .collect() +} + +fn set_local_identity(repo_dir: &Path, name: &str, email: &str) { + for (key, value) in [("user.name", name), ("user.email", email)] { + let output = Command::new("git") + .args(["config", key, value]) + .current_dir(repo_dir) + .output() + .expect("git config should run"); + assert_success(&output, "git config"); + } +} + +fn command_node(id: &str, script: &str) -> Node { + let mut node = Node::new(id); + node.attrs.insert( + "shape".to_string(), + AttrValue::String("parallelogram".to_string()), + ); + node.attrs + .insert("script".to_string(), AttrValue::String(script.to_string())); + node +} + +fn identity_registry() -> HandlerRegistry { + let mut registry = make_registry(); + registry.register("command", Box::new(CommandHandler)); + registry +} + +/// The identity a workflow command sees is the run's, not the checkout's +/// local config, not an inherited `GIT_*` variable, and not a +/// `[run.environment]` entry. It reaches the primary checkout, a clone the +/// workflow creates, and a repository the workflow initializes, and the +/// engine's own checkpoint commit carries the same identity. +#[tokio::test] +async fn run_identity_governs_engine_and_workflow_commits_everywhere() { + let dir = tempfile::tempdir().unwrap(); + let repo_dir = dir.path().join("repo"); + init_repo(&repo_dir); + set_local_identity(&repo_dir, "Local Config", "local@example.com"); + let base_sha = git_stdout(&repo_dir, &["rev-parse", "HEAD"]); + + let identity = fabro_types::GitIdentity { + name: "fabro-sh[bot]".to_string(), + email: "281434857+fabro-sh[bot]@users.noreply.github.com".to_string(), + source: fabro_types::GitIdentitySource::GithubApp, + }; + let expected = vec![ + identity.name.clone(), + identity.email.clone(), + identity.name.clone(), + identity.email.clone(), + ]; + + let clone_dir = dir.path().join("clone"); + let fresh_dir = dir.path().join("fresh"); + let script = format!( + "set -e + printf work > work.txt && git add work.txt && git commit -q -m 'workflow commit' + git clone -q . {clone} && (cd {clone} && printf x > x.txt && git add x.txt && git commit -q -m 'clone commit') + git init -q {fresh} && (cd {fresh} && printf y > y.txt && git add y.txt && git commit -q -m 'fresh commit')", + clone = clone_dir.display(), + fresh = fresh_dir.display(), + ); + + let mut graph = simple_graph(); + graph + .nodes + .insert("work".to_string(), command_node("work", &script)); + graph.edges.clear(); + graph.edges.push(Edge::new("start", "work")); + graph.edges.push(Edge::new("work", "exit")); + + let run_tmp = tempfile::tempdir().unwrap(); + let mut run_options = test_run_options(run_tmp.path()); + run_options.git_identity = Some(identity.clone()); + run_options.git = Some(GitCheckpointOptions { + base_sha: Some(base_sha), + run_branch: None, + meta_branch: None, + }); + + // A `[run.environment]` entry and an inherited host variable both name a + // different author; the run identity must win over both. + let env = HashMap::from([ + ("GIT_AUTHOR_NAME".to_string(), "Run Env".to_string()), + ( + "GIT_COMMITTER_EMAIL".to_string(), + "run-env@example.com".to_string(), + ), + ]); + let outcome = run_graph_with_env( + identity_registry(), + Arc::new(Emitter::new(fixtures::RUN_2)), + local_env(&repo_dir).await, + &graph, + &run_options, + env, + ) + .await + .expect("workflow should complete"); + assert_eq!(outcome.status, StageOutcome::Succeeded, "{outcome:?}"); + + // The workflow's own commit in the primary checkout. + assert_eq!( + commit_identity(&repo_dir, "HEAD~1"), + expected, + "workflow commit in the primary checkout" + ); + assert_eq!( + git_stdout(&repo_dir, &["log", "-1", "--format=%s", "HEAD~1"]), + "workflow commit" + ); + // The engine's checkpoint commit on top of it. + assert_eq!( + commit_identity(&repo_dir, "HEAD"), + expected, + "engine checkpoint commit" + ); + assert!( + git_stdout(&repo_dir, &["log", "-1", "--format=%s", "HEAD"]).starts_with("fabro("), + "HEAD should be the checkpoint commit" + ); + // A clone the workflow created and a repository it initialized. + assert_eq!( + commit_identity(&clone_dir, "HEAD"), + expected, + "clone commit" + ); + assert_eq!( + commit_identity(&fresh_dir, "HEAD"), + expected, + "fresh repo commit" + ); + + // The checkout's own configuration is left alone. + assert_eq!( + git_stdout(&repo_dir, &["config", "user.name"]), + "Local Config" + ); + assert_eq!( + git_stdout(&repo_dir, &["config", "user.email"]), + "local@example.com" + ); +} + +/// Two runs with different identities in the same process do not leak into +/// each other: each run's commits carry only its own identity. +#[tokio::test] +async fn concurrent_runs_keep_their_own_identities() { + async fn run_with(name: &str, email: &str) -> (tempfile::TempDir, Vec) { + let dir = tempfile::tempdir().unwrap(); + let repo_dir = dir.path().join("repo"); + init_repo(&repo_dir); + let mut graph = simple_graph(); + graph.nodes.insert( + "work".to_string(), + command_node( + "work", + "for i in 1 2 3; do printf $i > f$i.txt; git add f$i.txt; git commit -q -m c$i; \ + sleep 0.05; done", + ), + ); + graph.edges.clear(); + graph.edges.push(Edge::new("start", "work")); + graph.edges.push(Edge::new("work", "exit")); + let run_tmp = tempfile::tempdir().unwrap(); + let mut run_options = test_run_options(run_tmp.path()); + run_options.run_id = fabro_types::RunId::new(); + run_options.git_identity = Some(fabro_types::GitIdentity { + name: name.to_string(), + email: email.to_string(), + source: fabro_types::GitIdentitySource::Explicit, + }); + run_graph( + identity_registry(), + Arc::new(Emitter::new(run_options.run_id)), + local_env(&repo_dir).await, + &graph, + &run_options, + ) + .await + .expect("workflow should complete"); + let identities = git_stdout(&repo_dir, &["log", "--format=%an <%ae> %cn <%ce>", "-3"]) + .lines() + .map(str::to_string) + .collect(); + (dir, identities) + } + + let (first, second) = tokio::join!( + run_with("Run One", "one@example.com"), + run_with("Run Two", "two@example.com"), + ); + assert_eq!(first.1, vec![ + "Run One Run One "; + 3 + ]); + assert_eq!(second.1, vec![ + "Run Two Run Two "; + 3 + ]); +} diff --git a/lib/components/fabro-workflow/tests/it/integration.rs b/lib/components/fabro-workflow/tests/it/integration.rs index 0109a8edc..641e259be 100644 --- a/lib/components/fabro-workflow/tests/it/integration.rs +++ b/lib/components/fabro-workflow/tests/it/integration.rs @@ -449,6 +449,7 @@ async fn end_to_end_linear_pipeline() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -582,6 +583,7 @@ async fn end_to_end_branching_pipeline() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -705,6 +707,7 @@ async fn end_to_end_human_gate_pipeline() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -804,6 +807,7 @@ async fn human_gate_interrupted_input_fails_closed_without_fail_route() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -948,6 +952,7 @@ async fn human_gate_timeout_routes_to_default_choice_when_unanswered() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -1063,6 +1068,7 @@ async fn human_gate_interrupted_input_routes_via_outcome_fail_condition() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -1497,6 +1503,7 @@ async fn goal_gate_routes_to_retry_target_on_failure() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -1620,6 +1627,7 @@ async fn goal_gate_routes_to_retry_target_when_present() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -2035,6 +2043,7 @@ async fn retry_on_failure_then_succeed() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -2110,6 +2119,7 @@ async fn pipeline_with_many_nodes() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -2519,6 +2529,7 @@ async fn smoke_test_with_mock_codergen_backend() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -2754,6 +2765,7 @@ capabilities = {{ text = true, tools = true, response_format = {{ json_object = github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -2889,6 +2901,7 @@ enabled = true github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -2985,6 +2998,7 @@ async fn end_to_end_parallel_fan_out_fan_in() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -3101,6 +3115,7 @@ async fn resume_from_checkpoint_completes_pipeline() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -3203,6 +3218,7 @@ async fn resume_from_checkpoint_preserves_goal_gate_outcomes() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -3246,6 +3262,7 @@ async fn graph_goal_in_context() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -3288,6 +3305,7 @@ async fn event_streaming_lifecycle() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -3368,6 +3386,7 @@ async fn context_flow_between_stages() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -3427,6 +3446,7 @@ async fn tool_handler_e2e() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -3498,6 +3518,7 @@ async fn auto_approve_interviewer_e2e() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -3538,6 +3559,7 @@ async fn codergen_without_backend_simulated() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -3646,6 +3668,7 @@ async fn branching_loop_back_on_failure() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -3732,6 +3755,7 @@ async fn human_gate_loops_back() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -3795,6 +3819,7 @@ async fn scenario_ship_a_feature() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -3883,6 +3908,7 @@ async fn scenario_parallel_expert_review() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -3973,6 +3999,7 @@ async fn scenario_node_retries_on_retry_status() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -4041,6 +4068,7 @@ async fn scenario_loop_restart_resets_context() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -4109,6 +4137,7 @@ async fn scenario_bug_triage_router() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -4174,6 +4203,7 @@ async fn scenario_crash_recovery() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -4287,6 +4317,7 @@ async fn manager_loop_stop_condition_satisfied_e2e() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -4372,6 +4403,7 @@ async fn manager_loop_max_cycles_exceeded_e2e() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -4518,6 +4550,7 @@ async fn conditional_branching_success_fail_paths() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -4577,6 +4610,7 @@ async fn edge_selection_condition_match_wins_over_weight() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -4630,6 +4664,7 @@ async fn edge_selection_weight_breaks_ties() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -4675,6 +4710,7 @@ async fn edge_selection_lexical_tiebreak() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -4739,6 +4775,7 @@ async fn context_updates_visible_across_nodes() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -4789,6 +4826,7 @@ async fn stylesheet_applies_model_override() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -4845,6 +4883,7 @@ async fn custom_handler_registration_and_execution() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -4924,6 +4963,7 @@ async fn integration_smoke_plan_implement_review_done() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -5016,6 +5056,7 @@ async fn manager_loop_runs_child_engine_e2e() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -5153,6 +5194,7 @@ async fn manager_loop_context_flows_e2e() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -5232,6 +5274,7 @@ async fn manager_loop_child_workflow_e2e() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -5343,6 +5386,7 @@ async fn import_e2e_through_engine() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -5522,6 +5566,7 @@ async fn fidelity_default_is_compact() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -5579,6 +5624,7 @@ async fn fidelity_graph_default_applied() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -5632,6 +5678,7 @@ async fn fidelity_node_overrides_graph_default() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -5691,6 +5738,7 @@ async fn fidelity_edge_overrides_node_and_graph() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -5740,6 +5788,7 @@ async fn fidelity_full_produces_empty_preamble() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -5799,6 +5848,7 @@ async fn fidelity_truncate_preamble_minimal() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -5871,6 +5921,7 @@ async fn fidelity_summary_low_mode() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -5938,6 +5989,7 @@ async fn fidelity_summary_medium_mode() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -6005,6 +6057,7 @@ async fn fidelity_summary_high_mode() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -6065,6 +6118,7 @@ async fn fidelity_full_sets_thread_id_in_context() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -6136,6 +6190,7 @@ async fn fidelity_full_nodes_share_thread_id() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -6217,6 +6272,7 @@ async fn fidelity_resume_degrades_full_to_summary_high() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -6314,6 +6370,7 @@ async fn fidelity_resume_degrade_only_affects_first_hop() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -6398,6 +6455,7 @@ async fn fidelity_resume_no_degrade_when_not_full() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -6440,6 +6498,7 @@ async fn fidelity_stored_in_checkpoint_context() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -6536,6 +6595,7 @@ async fn fidelity_precedence_multi_node_pipeline() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -6604,6 +6664,7 @@ async fn fidelity_compact_preamble_includes_completed_stages_and_context() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -6683,6 +6744,7 @@ async fn fidelity_summary_low_excludes_context_values_in_pipeline() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -6754,6 +6816,7 @@ async fn fidelity_summary_low_excludes_context_values_in_pipeline() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -6826,6 +6889,7 @@ async fn fidelity_thread_id_fallback_to_previous_node_in_pipeline() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -6880,6 +6944,7 @@ async fn fidelity_thread_id_from_node_class_in_pipeline() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -6937,6 +7002,7 @@ async fn fidelity_edge_thread_id_override_in_pipeline() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -6995,6 +7061,7 @@ async fn fidelity_full_without_explicit_thread_id_uses_previous_node() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -7063,6 +7130,7 @@ async fn fidelity_from_parsed_dot_pipeline() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -7112,6 +7180,7 @@ async fn fidelity_checkpoint_roundtrip_preserves_fidelity() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -7188,6 +7257,7 @@ async fn fidelity_node_thread_id_overrides_edge_thread_id_in_pipeline() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -7275,6 +7345,7 @@ async fn fidelity_resume_preserves_context_values_across_checkpoint() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -7513,6 +7584,7 @@ mod real_llm { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -7682,6 +7754,7 @@ mod real_llm { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -7805,6 +7878,7 @@ mod real_llm { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -7938,6 +8012,7 @@ mod real_llm { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -8039,6 +8114,7 @@ mod real_llm { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -8172,6 +8248,7 @@ async fn workflow_run_with_vault_only_openai_codex_builds_pr_body() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -8289,6 +8366,7 @@ async fn human_gate_freeform_only_routes_text() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -8423,6 +8501,7 @@ async fn human_gate_freeform_with_fixed_choice_match() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -8543,6 +8622,7 @@ async fn human_gate_freeform_fallback_on_unmatched_text() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -8674,6 +8754,7 @@ async fn human_gate_freeform_sets_allow_freeform_on_question() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -8783,6 +8864,7 @@ async fn human_gate_without_freeform_sets_allow_freeform_false() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -9088,6 +9170,7 @@ fn make_run_options(dir: &std::path::Path) -> RunOptions { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -10033,6 +10116,7 @@ async fn run_fidelity_prompt_pipeline(fidelity: &str) -> String { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -10152,6 +10236,7 @@ async fn run_parallel_fidelity_capture( github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -10405,6 +10490,7 @@ async fn large_context_values_are_offloaded_to_artifact_store() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -10503,6 +10589,7 @@ async fn artifact_pointers_rewritten_for_remote_sandbox() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -10593,6 +10680,7 @@ async fn downstream_local_execution_resolves_response_blob_refs_as_text() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -10673,6 +10761,7 @@ async fn downstream_remote_execution_resolves_response_blob_refs_as_text() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -10805,6 +10894,7 @@ async fn node_dir_uses_visit_count_on_revisit() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -10974,6 +11064,7 @@ async fn git_checkpoint_host_emits_events_and_diff_patch() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: Some(GitCheckpointOptions { @@ -11143,6 +11234,7 @@ async fn git_checkpoint_host_skips_metadata_branch_without_writer_prereqs() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: Some(GitCheckpointOptions { @@ -11329,6 +11421,7 @@ async fn parallel_shared_checkout_host_e2e() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: Some(GitCheckpointOptions { @@ -11583,6 +11676,7 @@ async fn git_checkpoint_host_skips_empty_diff_patch() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: Some(GitCheckpointOptions { @@ -11954,6 +12048,7 @@ async fn e2e_circuit_breaker_deterministic_self_loop() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -12001,6 +12096,7 @@ async fn e2e_circuit_breaker_custom_limit() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -12041,6 +12137,7 @@ async fn e2e_circuit_breaker_ignores_transient_failures() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -12088,6 +12185,7 @@ async fn e2e_circuit_breaker_different_reasons_separate_counters() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -12128,6 +12226,7 @@ async fn e2e_circuit_breaker_loop_restart() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -12191,6 +12290,7 @@ async fn e2e_failure_signature_persisted_in_context() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -12258,6 +12358,7 @@ async fn e2e_failure_signature_hint_overrides_reason_in_context() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -12319,6 +12420,7 @@ async fn e2e_signature_maps_persist_in_checkpoint() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -12450,6 +12552,7 @@ async fn e2e_circuit_breaker_emits_events_before_abort() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -12517,6 +12620,7 @@ async fn e2e_circuit_breaker_does_not_fire_below_limit() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -12616,6 +12720,7 @@ async fn e2e_circuit_breaker_multi_stage_impl_verify_cycle() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -12714,6 +12819,7 @@ async fn e2e_loop_restart_blocked_for_deterministic_failure() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -12754,6 +12860,7 @@ async fn e2e_loop_restart_blocked_for_structural_failure() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -12794,6 +12901,7 @@ async fn e2e_loop_restart_blocked_for_budget_exhausted_failure() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -12834,6 +12942,7 @@ async fn e2e_loop_restart_blocked_for_canceled_failure() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -12871,6 +12980,7 @@ async fn e2e_loop_restart_blocked_for_compilation_loop_failure() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -12912,6 +13022,7 @@ async fn e2e_loop_restart_allowed_for_transient_infra() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -13022,6 +13133,7 @@ async fn e2e_stall_watchdog_triggers_from_dot_parsed_pipeline() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -13078,6 +13190,7 @@ async fn e2e_stall_watchdog_kept_alive_by_handler_events() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -13124,6 +13237,7 @@ async fn e2e_stall_watchdog_disabled_with_zero_timeout() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -13190,6 +13304,7 @@ async fn e2e_stall_watchdog_with_explicit_timeout_override() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -13334,6 +13449,7 @@ async fn asset_collection_local_sandbox_success() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -13482,6 +13598,7 @@ async fn asset_collection_local_sandbox_symlink_working_directory() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -13582,6 +13699,7 @@ async fn asset_collection_local_sandbox_on_failure() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -13695,6 +13813,7 @@ async fn asset_collection_docker_sandbox() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, @@ -13767,6 +13886,7 @@ async fn wait_timer_e2e() { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, diff --git a/lib/components/fabro-workflow/tests/it/pebble_agent.rs b/lib/components/fabro-workflow/tests/it/pebble_agent.rs index 78642ab35..e7a359c97 100644 --- a/lib/components/fabro-workflow/tests/it/pebble_agent.rs +++ b/lib/components/fabro-workflow/tests/it/pebble_agent.rs @@ -213,6 +213,7 @@ fn run_options(run_dir: &Path, cancel_token: CancellationToken) -> RunOptions { github_app: None, base_branch: None, display_base_sha: None, + git_identity: None, pre_run_git: None, fork_source_ref: None, git: None, diff --git a/lib/foundation/fabro-api/tests/run_projection_round_trip.rs b/lib/foundation/fabro-api/tests/run_projection_round_trip.rs index e09a867a6..3970d262f 100644 --- a/lib/foundation/fabro-api/tests/run_projection_round_trip.rs +++ b/lib/foundation/fabro-api/tests/run_projection_round_trip.rs @@ -51,6 +51,11 @@ fn run_projection_round_trips_populated_projection() { }, "pull_request": null, "superseded_by": null, + "git_identity": { + "name": "fabro-sh[bot]", + "email": "123456+fabro-sh[bot]@users.noreply.github.com", + "source": "github_app" + }, "pending_interviews": { "q-1": { "question": { diff --git a/lib/foundation/fabro-config/src/layers/run.rs b/lib/foundation/fabro-config/src/layers/run.rs index c18b72015..0a63d032d 100644 --- a/lib/foundation/fabro-config/src/layers/run.rs +++ b/lib/foundation/fabro-config/src/layers/run.rs @@ -250,13 +250,22 @@ pub struct RunGitLayer { )] #[serde(deny_unknown_fields)] pub struct GitAuthorLayer { - /// Git author name for checkpoint commits. + /// Git author and committer name for every commit the run creates. When + /// unset, the run uses its GitHub App bot or PAT user, else `Fabro`. #[serde(default, skip_serializing_if = "Option::is_none")] - #[option(default = "\"fabro\"", value_type = "string")] + #[option( + default = "resolved from the run's GitHub credential", + value_type = "string" + )] pub name: Option, - /// Git author email for checkpoint commits. + /// Git author and committer email for every commit the run creates. When + /// unset, the run uses the credential's noreply address, else + /// `noreply@fabro.sh`. #[serde(default, skip_serializing_if = "Option::is_none")] - #[option(default = "\"fabro@local\"", value_type = "string")] + #[option( + default = "resolved from the run's GitHub credential", + value_type = "string" + )] pub email: Option, } diff --git a/lib/foundation/fabro-types/src/git_identity.rs b/lib/foundation/fabro-types/src/git_identity.rs new file mode 100644 index 000000000..8c7920aee --- /dev/null +++ b/lib/foundation/fabro-types/src/git_identity.rs @@ -0,0 +1,93 @@ +use serde::{Deserialize, Serialize}; + +/// The Git author and committer identity a run resolved once and uses for +/// every commit it creates: engine checkpoints, metadata commits, and any +/// `git commit` a workflow command or agent tool runs. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct GitIdentity { + pub name: String, + pub email: String, + pub source: GitIdentitySource, +} + +/// Where a run's Git identity came from. +#[derive( + Debug, + Clone, + Copy, + PartialEq, + Eq, + Hash, + Serialize, + Deserialize, + strum::Display, + strum::EnumString, + strum::IntoStaticStr, +)] +#[serde(rename_all = "snake_case")] +#[strum(serialize_all = "snake_case")] +pub enum GitIdentitySource { + /// `run.git.author` supplied both the name and the email. + Explicit, + /// The run's GitHub App bot account. + GithubApp, + /// The authenticated user of the run's GitHub personal access token. + GithubPat, + /// The generic Fabro identity: no usable credential identity. + Default, +} + +impl GitIdentity { + pub const DEFAULT_EMAIL: &'static str = "noreply@fabro.sh"; + pub const DEFAULT_NAME: &'static str = "Fabro"; + + /// The generic identity used when no credential identity is available. + #[must_use] + pub fn fabro_default() -> Self { + Self { + name: Self::DEFAULT_NAME.to_string(), + email: Self::DEFAULT_EMAIL.to_string(), + source: GitIdentitySource::Default, + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn source_serializes_as_snake_case() { + assert_eq!( + serde_json::to_value(GitIdentitySource::GithubApp).unwrap(), + serde_json::json!("github_app") + ); + assert_eq!(GitIdentitySource::GithubPat.to_string(), "github_pat"); + assert_eq!( + "explicit".parse::().unwrap(), + GitIdentitySource::Explicit + ); + } + + #[test] + fn identity_round_trips_through_json() { + let identity = GitIdentity { + name: "fabro-bot[bot]".to_string(), + email: "1+fabro-bot[bot]@users.noreply.github.com".to_string(), + source: GitIdentitySource::GithubApp, + }; + let value = serde_json::to_value(&identity).unwrap(); + assert_eq!( + value, + serde_json::json!({ + "name": "fabro-bot[bot]", + "email": "1+fabro-bot[bot]@users.noreply.github.com", + "source": "github_app", + }) + ); + assert_eq!( + serde_json::from_value::(value).unwrap(), + identity + ); + } +} diff --git a/lib/foundation/fabro-types/src/lib.rs b/lib/foundation/fabro-types/src/lib.rs index b4b26fe74..086028583 100644 --- a/lib/foundation/fabro-types/src/lib.rs +++ b/lib/foundation/fabro-types/src/lib.rs @@ -14,6 +14,7 @@ pub mod dense; pub mod diff; pub mod event_envelope; pub mod failure_signature; +pub mod git_identity; pub mod graph; mod id; mod input_scalar; @@ -73,6 +74,7 @@ pub use dense::{ServerSettings, UserSettings, WorkflowSettings}; pub use diff::{DiffStats, DiffSummary, RunDiff}; pub use event_envelope::EventEnvelope; pub use failure_signature::FailureSignature; +pub use git_identity::{GitIdentity, GitIdentitySource}; pub use graph::{ AttrValue, AttributeScope, ContextKeyAttr, Edge, Graph, KNOWN_HANDLER_TYPES, Node, OnFailure, ResolvedOnFailure, is_known_handler_type, is_llm_handler_type, shape_to_handler_type, diff --git a/lib/foundation/fabro-types/src/run_event/infra.rs b/lib/foundation/fabro-types/src/run_event/infra.rs index c8c1e7b96..dac6f6b70 100644 --- a/lib/foundation/fabro-types/src/run_event/infra.rs +++ b/lib/foundation/fabro-types/src/run_event/infra.rs @@ -1,7 +1,7 @@ use serde::{Deserialize, Serialize}; use super::ExecOutputTail; -use crate::{RunSandboxFailure, SandboxProviderKind}; +use crate::{GitIdentity, RunSandboxFailure, SandboxProviderKind}; #[derive( Debug, @@ -28,6 +28,7 @@ pub enum RunNoticeCode { CheckpointMetadataWriteFailed, DirtyWorktree, GitDiffFailed, + GitIdentityFallback, GitPushFailed, GithubTokenFailed, GithubTokenRefreshLimited, @@ -191,6 +192,14 @@ pub struct SetupCompletedProps { pub duration_ms: u64, } +/// The Git author/committer identity the run resolved for every commit it +/// creates, with the credential it was derived from. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct GitIdentityResolvedProps { + #[serde(flatten)] + pub identity: GitIdentity, +} + #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct SetupFailedProps { pub command: String, diff --git a/lib/foundation/fabro-types/src/run_event/mod.rs b/lib/foundation/fabro-types/src/run_event/mod.rs index 1d4ec2c32..10afc6346 100644 --- a/lib/foundation/fabro-types/src/run_event/mod.rs +++ b/lib/foundation/fabro-types/src/run_event/mod.rs @@ -252,6 +252,8 @@ pub enum EventBody { SetupCommandCompleted(SetupCommandCompletedProps), #[serde(rename = "setup.completed")] SetupCompleted(SetupCompletedProps), + #[serde(rename = "git.identity.resolved")] + GitIdentityResolved(GitIdentityResolvedProps), #[serde(rename = "setup.failed")] SetupFailed(SetupFailedProps), #[serde(rename = "watchdog.timeout")] @@ -518,6 +520,7 @@ impl EventBody { Self::SetupCommandStarted(_) => "setup.command.started", Self::SetupCommandCompleted(_) => "setup.command.completed", Self::SetupCompleted(_) => "setup.completed", + Self::GitIdentityResolved(_) => "git.identity.resolved", Self::SetupFailed(_) => "setup.failed", Self::StallWatchdogTimeout(_) => "watchdog.timeout", Self::ArtifactCaptured(_) => "artifact.captured", @@ -657,6 +660,7 @@ fn is_known_event_name(event: &str) -> bool { | "setup.command.completed" | "setup.completed" | "setup.failed" + | "git.identity.resolved" | "watchdog.timeout" | "artifact.captured" | "ssh.ready" diff --git a/lib/foundation/fabro-types/src/run_projection.rs b/lib/foundation/fabro-types/src/run_projection.rs index e3f256683..6b8613eb0 100644 --- a/lib/foundation/fabro-types/src/run_projection.rs +++ b/lib/foundation/fabro-types/src/run_projection.rs @@ -13,7 +13,7 @@ use strum::{Display, EnumString, IntoStaticStr}; use crate::run_event::{AgentSessionActivatedProps, StagePromptProps}; use crate::{ - AgentBackend, AgentMcpToolSummary, BilledTokenCounts, Checkpoint, Conclusion, + AgentBackend, AgentMcpToolSummary, BilledTokenCounts, Checkpoint, Conclusion, GitIdentity, InterviewQuestionRecord, InvalidTransition, ModelRef, ParallelBranchId, PullRequestCreation, PullRequestLink, RunApproval, RunControlAction, RunDiff, RunId, RunSandbox, RunSpec, RunStatus, RunTiming, StageCompletion, StageHandler, StageId, StageState, StageTiming, StartRecord, @@ -47,6 +47,10 @@ pub struct RunProjection { pub superseded_by: Option, #[serde(default, skip_serializing_if = "Option::is_none")] pub retried_from: Option, + /// The Git author/committer identity the run resolved for its commits. + /// Absent until the run's first initialization resolves it. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub git_identity: Option, pub pending_interviews: BTreeMap, stages: HashMap, } @@ -760,6 +764,7 @@ impl RunProjection { pull_request_creation: None, superseded_by: None, retried_from: None, + git_identity: None, pending_interviews: BTreeMap::new(), stages: HashMap::new(), } diff --git a/lib/packages/fabro-api-client/src/.openapi-generator/FILES b/lib/packages/fabro-api-client/src/.openapi-generator/FILES index e57ae9997..6ed3dc9de 100644 --- a/lib/packages/fabro-api-client/src/.openapi-generator/FILES +++ b/lib/packages/fabro-api-client/src/.openapi-generator/FILES @@ -158,6 +158,8 @@ models/fork-response.ts models/fork-source-ref.ts models/git-author-settings.ts models/git-context.ts +models/git-identity-source.ts +models/git-identity.ts models/git-run-target.ts models/github-integration-settings.ts models/github-integration-strategy.ts diff --git a/lib/packages/fabro-api-client/src/models/git-identity-source.ts b/lib/packages/fabro-api-client/src/models/git-identity-source.ts new file mode 100644 index 000000000..b3c2fc001 --- /dev/null +++ b/lib/packages/fabro-api-client/src/models/git-identity-source.ts @@ -0,0 +1,28 @@ +/* tslint:disable */ +/* eslint-disable */ +/** + * Fabro Run API + * HTTP API for managing Fabro workflow run executions. + * + * The version of the OpenAPI document: 0.2.0 + * + * + * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). + * https://openapi-generator.tech + * Do not edit the class manually. + */ + + + +/** + * Where a run\'s Git author/committer identity came from. + */ + +export const GitIdentitySource = { + EXPLICIT: 'explicit', + GITHUB_APP: 'github_app', + GITHUB_PAT: 'github_pat', + DEFAULT: 'default' +} as const; + +export type GitIdentitySource = typeof GitIdentitySource[keyof typeof GitIdentitySource]; diff --git a/lib/packages/fabro-api-client/src/models/git-identity.ts b/lib/packages/fabro-api-client/src/models/git-identity.ts new file mode 100644 index 000000000..2e20fd7b2 --- /dev/null +++ b/lib/packages/fabro-api-client/src/models/git-identity.ts @@ -0,0 +1,27 @@ +/* tslint:disable */ +/* eslint-disable */ +/** + * Fabro Run API + * HTTP API for managing Fabro workflow run executions. + * + * The version of the OpenAPI document: 0.2.0 + * + * + * NOTE: This class is auto generated by OpenAPI Generator (https://openapi-generator.tech). + * https://openapi-generator.tech + * Do not edit the class manually. + */ + + +// May contain unused imports in some cases +// @ts-ignore +import type { GitIdentitySource } from './git-identity-source'; + +/** + * The Git author and committer identity a run resolved once and uses for every commit it creates: engine checkpoints, metadata commits, and any commit a workflow command or agent tool runs. + */ +export interface GitIdentity { + 'name': string; + 'email': string; + 'source': GitIdentitySource; +} diff --git a/lib/packages/fabro-api-client/src/models/index.ts b/lib/packages/fabro-api-client/src/models/index.ts index d199bb683..9bc29da56 100644 --- a/lib/packages/fabro-api-client/src/models/index.ts +++ b/lib/packages/fabro-api-client/src/models/index.ts @@ -129,6 +129,8 @@ export * from './fork-response'; export * from './fork-source-ref'; export * from './git-author-settings'; export * from './git-context'; +export * from './git-identity'; +export * from './git-identity-source'; export * from './git-run-target'; export * from './github-integration-settings'; export * from './github-integration-strategy'; diff --git a/lib/packages/fabro-api-client/src/models/run-projection.ts b/lib/packages/fabro-api-client/src/models/run-projection.ts index 2f679a012..8496e8b71 100644 --- a/lib/packages/fabro-api-client/src/models/run-projection.ts +++ b/lib/packages/fabro-api-client/src/models/run-projection.ts @@ -21,6 +21,9 @@ import type { CheckpointRecord } from './checkpoint-record'; import type { Conclusion } from './conclusion'; // May contain unused imports in some cases // @ts-ignore +import type { GitIdentity } from './git-identity'; +// May contain unused imports in some cases +// @ts-ignore import type { PendingInterviewRecord } from './pending-interview-record'; // May contain unused imports in some cases // @ts-ignore @@ -83,6 +86,7 @@ export interface RunProjection { * Source run ID when this run was created by manual retry. */ 'retried_from'?: string | null; + 'git_identity'?: GitIdentity | null; 'pending_interviews': { [key: string]: PendingInterviewRecord; }; /** * Map from StageId (`node_id@visit`) to stage projection data.