diff --git a/docs/public/api-reference/fabro-api.yaml b/docs/public/api-reference/fabro-api.yaml index 0c8a8b7d4..fc19316bf 100644 --- a/docs/public/api-reference/fabro-api.yaml +++ b/docs/public/api-reference/fabro-api.yaml @@ -9318,7 +9318,10 @@ components: folder: "#/components/schemas/FolderRunTarget" GitRunTarget: - description: Public github.com repository target. + description: >- + Public github.com repository target. The branch names the attached + working branch. An optional tag selects a release at worker start, and + an optional exact SHA is authoritative when both are present. type: object additionalProperties: false required: @@ -9335,14 +9338,23 @@ components: example: acme/my-app branch: type: string - description: Required branch name, preserved exactly. + description: Required attached working branch name, preserved exactly. example: feature/foo + tag: + type: string + minLength: 1 + description: >- + Optional bare tag name. Prefixes such as `refs/tags/` and `tags/` + are rejected. Without `sha`, the worker resolves this tag when the + sandbox starts and fails if it is unavailable. + example: v1.2.3 sha: type: string pattern: "^[0-9A-Fa-f]{40}$" description: >- Optional exact commit. The server lowercase-normalizes its syntax - but does not resolve it or prove branch ancestry. + but does not resolve it, prove branch ancestry, or prove that it + matches an accompanying tag. When present, this exact commit wins. NoneRunTarget: description: >- diff --git a/docs/public/execution/environments.mdx b/docs/public/execution/environments.mdx index 2cd2f5508..d592b2a76 100644 --- a/docs/public/execution/environments.mdx +++ b/docs/public/execution/environments.mdx @@ -270,7 +270,7 @@ memory = "4GB" mode = "block" ``` -Docker and Daytona are clone-based providers. When a run has a GitHub origin, Fabro clones it into the provider workspace with a history depth of 100. Set `[run.clone] enabled = false` to start a manifest-backed run with an empty workspace. Set `[run.clone] depth = 0` to clone full history. A version-backed run intent can instead submit the explicit `{ "kind": "none" }` target, which forces an empty provider workspace regardless of the workflow's clone setting. The `none` target is not supported by Local environments, while the Local-only `folder` target is rejected by Docker and Daytona. Docker and Daytona ignore `cwd`; use the provider-owned workspace layout and `run.working_dir` for repository-relative commands. +Docker and Daytona are clone-based providers. When a run has a GitHub origin, Fabro clones it into the provider workspace with a history depth of 100. Set `[run.clone] enabled = false` to start a manifest-backed run with an empty workspace. Set `[run.clone] depth = 0` to clone full history. A version-backed run intent can instead submit the explicit `{ "kind": "none" }` target, which forces an empty provider workspace regardless of the workflow's clone setting. Its Git target may select a branch, an optional bare tag, an optional exact commit SHA, or both tag and SHA. Both providers attach the selected revision to the target's working branch; an exact SHA wins over a tag, and unavailable tags or commits fail without branch fallback. The `none` target is not supported by Local environments, while the Local-only `folder` target is rejected by Docker and Daytona. Docker and Daytona ignore `cwd`; use the provider-owned workspace layout and `run.working_dir` for repository-relative commands. The image must provide `/bin/bash`; Fabro evaluates every sandbox command with it and has no `sh` fallback. Commands run in a **non-login** shell, so login profiles (`/etc/profile.d/*.sh`, `~/.bash_profile`, and `nvm`/`rbenv`/`sdkman` initializers) are not sourced — put anything they set into the Dockerfile's `ENV` instead. Fabro verifies Bash during initialization and again on resume, and fails with remediation rather than reporting the sandbox ready. diff --git a/docs/public/integrations/github.mdx b/docs/public/integrations/github.mdx index 9b6347b95..4533f416f 100644 --- a/docs/public/integrations/github.mdx +++ b/docs/public/integrations/github.mdx @@ -227,18 +227,29 @@ When a workflow runs in a remote sandbox (Daytona or Docker), Fabro clones the c For public repositories, the clone works without credentials. The token is still generated because it's needed for pushing checkpoints. -#### Exact commits for run intents +#### Git targets for run intents -The `RunIntent` create body names a required Git branch and may also pin a full -40-character commit SHA. Creating the run validates and lowercase-normalizes -the SHA, but it does not contact GitHub, resolve the commit, or prove that the -commit belongs to the submitted branch. +The `RunIntent` create body always names a GitHub repository and a working +branch. It may also select a bare tag, pin a full 40-character commit SHA, or +include both: -At sandbox setup, Docker fetches the submitted commit directly and Daytona -receives it as `commit_id`; the submitted branch remains the working branch. -If the exact commit is unavailable, setup fails. Fabro never substitutes the -branch's newer HEAD. When the request omits `sha`, the sandbox resolves the -branch at materialization time instead. +| Target fields | Revision selected when the worker starts | +|---|---| +| `branch` | The branch HEAD | +| `branch` + `sha` | The exact commit | +| `branch` + `tag` | The tag's peeled commit | +| `branch` + `tag` + `sha` | The exact commit; the tag remains part of the run's identity | + +`branch` is always the attached branch inside the sandbox. `tag` is a bare tag +name such as `v1.2.3`; `refs/tags/v1.2.3` and `tags/v1.2.3` are rejected. An +unpinned tag is resolved when the worker starts, so moving a tag before that +point changes the selected commit. + +Creating the run validates the selectors and lowercase-normalizes `sha`, but +does not contact GitHub or prove ancestry. An exact SHA is authoritative: +Fabro does not prove it belongs to the branch or matches the accompanying tag. +If a requested tag or exact commit is unavailable, sandbox setup fails without +falling back to a same-named branch or the branch's newer HEAD. ### GITHUB_TOKEN injection diff --git a/lib/apps/fabro-server/src/run_manifest.rs b/lib/apps/fabro-server/src/run_manifest.rs index 30cac5b0d..809dd34df 100644 --- a/lib/apps/fabro-server/src/run_manifest.rs +++ b/lib/apps/fabro-server/src/run_manifest.rs @@ -935,6 +935,7 @@ fn preflight_sandbox_spec( run_id: None, clone_origin_url, clone_branch, + clone_tag: None, clone_commit_sha: None, } } @@ -947,6 +948,7 @@ fn preflight_sandbox_spec( run_id: None, clone_origin_url, clone_branch, + clone_tag: None, clone_commit_sha: None, api_key: daytona_api_key, } diff --git a/lib/apps/fabro-server/src/server/handler/lifecycle.rs b/lib/apps/fabro-server/src/server/handler/lifecycle.rs index fe4020aba..02a81084b 100644 --- a/lib/apps/fabro-server/src/server/handler/lifecycle.rs +++ b/lib/apps/fabro-server/src/server/handler/lifecycle.rs @@ -809,12 +809,12 @@ async fn batch_archive_runs( State(state): State>, Json(request): Json, ) -> Response { - batch_run_archive_action( + Box::pin(batch_run_archive_action( state, Principal::User(user), request, ArchiveAction::Archive, - ) + )) .await } @@ -823,12 +823,12 @@ async fn batch_unarchive_runs( State(state): State>, Json(request): Json, ) -> Response { - batch_run_archive_action( + Box::pin(batch_run_archive_action( state, Principal::User(user), request, ArchiveAction::Unarchive, - ) + )) .await } diff --git a/lib/apps/fabro-server/src/server/handler/runs.rs b/lib/apps/fabro-server/src/server/handler/runs.rs index 852a569db..42b7854ae 100644 --- a/lib/apps/fabro-server/src/server/handler/runs.rs +++ b/lib/apps/fabro-server/src/server/handler/runs.rs @@ -1091,7 +1091,7 @@ async fn validate_intent_environment( SandboxProviderKind::Daytona => image.docker.is_some(), }; let (target_incompatible, detail) = match target { - RunTarget::Git { .. } => ( + RunTarget::Git(_) => ( provider == SandboxProviderKind::Local || !settings.run.clone.enabled, "Git targets require a compatible clone-enabled Docker or Daytona environment", ), diff --git a/lib/apps/fabro-server/src/server/handler/sessions.rs b/lib/apps/fabro-server/src/server/handler/sessions.rs index 7225fca3d..f5f1877b3 100644 --- a/lib/apps/fabro-server/src/server/handler/sessions.rs +++ b/lib/apps/fabro-server/src/server/handler/sessions.rs @@ -1169,7 +1169,10 @@ async fn drive_agent_session( result = &mut process => { while let Ok(event) = receiver.try_recv() { record_turn_output(output, &event); - persist_agent_event(run_store, run_id, session_id, turn_id, event, sender).await?; + Box::pin(persist_agent_event( + run_store, run_id, session_id, turn_id, event, sender, + )) + .await?; } return Ok(result); } @@ -1177,7 +1180,10 @@ async fn drive_agent_session( match event { Ok(event) => { record_turn_output(output, &event); - persist_agent_event(run_store, run_id, session_id, turn_id, event, sender).await?; + Box::pin(persist_agent_event( + run_store, run_id, session_id, turn_id, event, sender, + )) + .await?; } Err(RecvError::Lagged(_) | RecvError::Closed) => {} } diff --git a/lib/apps/fabro-server/src/server/tests.rs b/lib/apps/fabro-server/src/server/tests.rs index f5618ca2b..68c01bdf1 100644 --- a/lib/apps/fabro-server/src/server/tests.rs +++ b/lib/apps/fabro-server/src/server/tests.rs @@ -3598,7 +3598,7 @@ async fn store_workflow_version( } #[tokio::test] -async fn post_runs_run_intent_creates_submitted_version_backed_git_target_without_starting() { +async fn post_runs_run_intent_persists_tagged_exact_git_target_without_starting() { let state = test_app_state(); let app = crate::test_support::build_test_router(Arc::clone(&state)); let workflow_version_id = store_workflow_version( @@ -3638,6 +3638,7 @@ docker = "workflow-owned:latest" "kind": "git", "repo": "fabro-sh/fabro", "branch": "feature/run-intent", + "tag": "v1.2.3", "sha": submitted_sha }, "args": { @@ -3671,11 +3672,12 @@ docker = "workflow-owned:latest" ); assert_eq!( projection.spec.target, - Some(fabro_types::RunTarget::Git { + Some(fabro_types::RunTarget::Git(fabro_types::GitRunTarget { repo: "fabro-sh/fabro".to_string(), branch: "feature/run-intent".to_string(), + tag: Some("v1.2.3".to_string()), sha: Some("abcdef0123456789abcdef0123456789abcdef01".to_string()), - }) + })) ); assert_eq!( projection diff --git a/lib/components/fabro-agent/tests/it/docker_shell.rs b/lib/components/fabro-agent/tests/it/docker_shell.rs index 19f0f00f8..83ad3ac0a 100644 --- a/lib/components/fabro-agent/tests/it/docker_shell.rs +++ b/lib/components/fabro-agent/tests/it/docker_shell.rs @@ -29,6 +29,7 @@ async fn shell_reports_real_docker_process_outcome() { None, None, None, + None, ) else { return; }; diff --git a/lib/components/fabro-sandbox/src/clone_source.rs b/lib/components/fabro-sandbox/src/clone_source.rs index 7c787b366..f9667f6bc 100644 --- a/lib/components/fabro-sandbox/src/clone_source.rs +++ b/lib/components/fabro-sandbox/src/clone_source.rs @@ -8,6 +8,7 @@ pub(crate) enum CloneDecision { GitHub { origin_url: String, branch: Option, + tag: Option, commit_sha: Option, }, } @@ -82,17 +83,87 @@ pub(crate) fn exact_repository_init_command(clone_url: &str, checkout_path: &str ) } -/// Fetch a single admitted commit with the same history depth a branch clone +/// A revision the checkout is pinned to instead of the branch's current HEAD. +/// +/// The working branch names the checkout the run works on; it never constrains +/// which revision is fetched. No layer proves branch/revision ancestry, and an +/// unavailable revision fails without falling back to branch HEAD. +#[derive(Debug, Clone, PartialEq, Eq)] +pub(crate) enum PinnedRevision { + /// An exact commit SHA, already normalized by + /// [`normalize_exact_commit_sha`]. + Commit(String), + /// A bare tag name, fetched as `refs/tags/` so a same-named branch is + /// never consulted. + Tag(String), +} + +impl PinnedRevision { + /// An exact commit is authoritative over a tag; the tag stays on the run + /// target as durable identity but does not drive the checkout. + pub(crate) fn from_selectors(tag: Option<&str>, commit_sha: Option<&str>) -> Option { + match (commit_sha, tag) { + (Some(sha), _) => Some(Self::Commit(sha.to_string())), + (None, Some(tag)) => Some(Self::Tag(tag.to_string())), + (None, None) => None, + } + } + + /// Human-readable prefix for error messages. + pub(crate) fn label(&self) -> &'static str { + match self { + Self::Commit(_) => "Exact commit checkout", + Self::Tag(_) => "Tag checkout", + } + } + + /// The refspec handed to `git fetch`. + pub(crate) fn fetch_refspec(&self) -> String { + match self { + Self::Commit(sha) => sha.clone(), + Self::Tag(tag) => tag_ref(tag), + } + } + + /// The commit HEAD must resolve to after checkout, when one is known. + pub(crate) fn expected_sha(&self) -> Option<&str> { + match self { + Self::Commit(sha) => Some(sha), + Self::Tag(_) => None, + } + } + + /// Validate the `rev-parse HEAD` output of a pinned checkout and return the + /// resolved commit ID. + pub(crate) fn verify_head(&self, output: &str) -> crate::Result { + let actual_sha = verify_resolved_head(output)?; + if self + .expected_sha() + .is_some_and(|expected| expected != actual_sha) + { + return Err(crate::Error::message( + "Exact checkout HEAD did not match the requested commit", + )); + } + Ok(actual_sha) + } +} + +/// Fully-qualified ref for a bare tag name. +pub(crate) fn tag_ref(tag: &str) -> String { + format!("refs/tags/{tag}") +} + +/// Fetch a single pinned refspec with the same history depth a branch clone /// gets, so both paths can reach the same number of parent commits. /// -/// The fetch names the commit directly rather than the branch. No layer proves -/// that the submitted commit belongs to the submitted branch: the branch names -/// the working branch, while a fetchable exact commit is checked out as-is. +/// The fetch names the revision directly rather than the branch, and +/// `--no-tags` keeps unrelated tags from being pulled alongside it. #[cfg(any(feature = "docker", test))] -pub(crate) fn exact_fetch_command( +pub(crate) fn pinned_fetch_command( checkout_path: &str, fetch_source: &str, - commit_sha: &str, + refspec: &str, depth: Option, ) -> String { let depth_arg = depth_argument(depth); @@ -100,7 +171,7 @@ pub(crate) fn exact_fetch_command( "{git} -C {} fetch{depth_arg} --no-tags {} -- {}", sandbox::shell_quote(checkout_path), sandbox::shell_quote(fetch_source), - sandbox::shell_quote(commit_sha), + sandbox::shell_quote(refspec), git = sandbox::GIT, ) } @@ -131,7 +202,8 @@ pub(crate) fn exact_branch_checkout_command( ) } -/// Print the current HEAD commit and nothing else, for [`verify_exact_head`]. +/// Print the current HEAD commit and nothing else, for +/// [`PinnedRevision::verify_head`]. pub(crate) fn exact_head_revision_command(checkout_path: &str) -> String { format!( "{git} -C {path} rev-parse HEAD", @@ -141,7 +213,8 @@ pub(crate) fn exact_head_revision_command(checkout_path: &str) -> String { } /// Check out the admitted branch and print the resulting HEAD in one shell -/// command; stdout is the `rev-parse HEAD` output for [`verify_exact_head`]. +/// command; stdout is the `rev-parse HEAD` output for +/// [`PinnedRevision::verify_head`]. #[cfg(any(feature = "docker", test))] pub(crate) fn exact_checkout_verify_command( checkout_path: &str, @@ -155,17 +228,17 @@ pub(crate) fn exact_checkout_verify_command( ) } -pub(crate) fn verify_exact_head(output: &str, expected_sha: &str) -> crate::Result<()> { - let actual_sha = output.trim(); - let actual_sha = normalize_exact_commit_sha(actual_sha).map_err(|err| { - crate::Error::context("Exact checkout produced an invalid HEAD commit ID", err) - })?; - if actual_sha != expected_sha { - return Err(crate::Error::message( - "Exact checkout HEAD did not match the requested commit", - )); - } - Ok(()) +/// The peeled commit behind whatever `git fetch` just wrote to `FETCH_HEAD`; +/// a commit peels to itself, an annotated tag to the commit it points at. +#[cfg(any(feature = "docker", test))] +pub(crate) const FETCH_HEAD_COMMIT: &str = "FETCH_HEAD^{commit}"; + +/// Validate that a `rev-parse HEAD` output is a single commit ID and return it +/// normalized. +pub(crate) fn verify_resolved_head(output: &str) -> crate::Result { + normalize_exact_commit_sha(output.trim()).map_err(|err| { + crate::Error::context("Pinned checkout produced an invalid HEAD commit ID", err) + }) } fn trim_root(root: &str) -> &str { @@ -194,31 +267,39 @@ pub(crate) fn decide_clone( skip_clone: bool, clone_origin_url: Option<&str>, clone_branch: Option<&str>, + clone_tag: Option<&str>, clone_commit_sha: Option<&str>, ) -> crate::Result { + if clone_tag.is_some_and(|tag| tag.trim().is_empty()) { + return Err(crate::Error::message( + "Tag checkout requires a non-empty tag", + )); + } + let tag = clone_tag.map(str::to_string); let commit_sha = clone_commit_sha .map(normalize_exact_commit_sha) .transpose()?; - if commit_sha.is_some() { + if let Some(pin) = PinnedRevision::from_selectors(tag.as_deref(), commit_sha.as_deref()) { + let selector = pin.label(); if skip_clone { - return Err(crate::Error::message( - "Exact commit checkout requires cloning to be enabled", - )); + return Err(crate::Error::message(format!( + "{selector} requires cloning to be enabled" + ))); } if clone_origin_url.is_none_or(|url| url.trim().is_empty()) { - return Err(crate::Error::message( - "Exact commit checkout requires a repository origin", - )); + return Err(crate::Error::message(format!( + "{selector} requires a repository origin" + ))); } // The branch names the checkout the run works on; it is not used to // constrain which commits may be fetched. No layer proves branch/SHA // ancestry, and an unavailable exact commit fails without falling back // to branch HEAD. if clone_branch.is_none_or(|branch| branch.trim().is_empty()) { - return Err(crate::Error::message( - "Exact commit checkout requires a repository branch", - )); + return Err(crate::Error::message(format!( + "{selector} requires a repository branch" + ))); } } @@ -246,6 +327,7 @@ pub(crate) fn decide_clone( branch: clone_branch .filter(|branch| !branch.trim().is_empty()) .map(str::to_string), + tag, commit_sha, }) } @@ -267,7 +349,7 @@ pub(crate) fn repo_cloned_for_record( clone_origin_url: Option<&str>, ) -> Option { Some(matches!( - decide_clone(skip_clone, clone_origin_url, None, None).ok()?, + decide_clone(skip_clone, clone_origin_url, None, None, None).ok()?, CloneDecision::GitHub { .. } )) } @@ -307,17 +389,8 @@ mod tests { String::from_utf8(output.stdout).expect("git output should be UTF-8") } - #[expect( - clippy::disallowed_methods, - reason = "hermetic command-builder proof intentionally runs local Bash synchronously" - )] fn run_shell(cwd: &Path, command: &str) -> String { - let output = isolated_command(Command::new("/bin/bash").current_dir(cwd).args([ - "--noprofile", - "--norc", - "-c", - command, - ])); + let output = run_shell_output(cwd, command); assert!( output.status.success(), "command failed: {}", @@ -326,6 +399,19 @@ mod tests { String::from_utf8(output.stdout).expect("command output should be UTF-8") } + #[expect( + clippy::disallowed_methods, + reason = "hermetic command-builder proof intentionally runs local Bash synchronously" + )] + fn run_shell_output(cwd: &Path, command: &str) -> Output { + isolated_command(Command::new("/bin/bash").current_dir(cwd).args([ + "--noprofile", + "--norc", + "-c", + command, + ])) + } + #[test] fn skip_clone_overrides_present_origin() { assert_eq!( @@ -334,6 +420,7 @@ mod tests { Some("https://gitlab.com/acme/widgets.git"), Some("main"), None, + None, ) .unwrap(), CloneDecision::EmptyWorkspace { @@ -345,7 +432,7 @@ mod tests { #[test] fn missing_origin_creates_empty_workspace() { assert_eq!( - decide_clone(false, None, None, None).unwrap(), + decide_clone(false, None, None, None, None).unwrap(), CloneDecision::EmptyWorkspace { reason: EmptyWorkspaceReason::MissingOrigin, } @@ -360,16 +447,63 @@ mod tests { Some("git@github.com:acme/widgets.git"), Some("feature/work"), None, + None, ) .unwrap(), CloneDecision::GitHub { origin_url: "https://github.com/acme/widgets".to_string(), branch: Some("feature/work".to_string()), + tag: None, commit_sha: None, } ); } + #[test] + fn tag_clone_keeps_working_branch_and_bare_tag_distinct() { + assert_eq!( + decide_clone( + false, + Some("https://github.com/acme/widgets"), + Some("release"), + Some("v1.2.3"), + None, + ) + .unwrap(), + CloneDecision::GitHub { + origin_url: "https://github.com/acme/widgets".to_string(), + branch: Some("release".to_string()), + tag: Some("v1.2.3".to_string()), + commit_sha: None, + } + ); + } + + #[test] + fn pinned_revision_prefers_exact_commit_and_qualifies_tags() { + let sha = "0123456789abcdef0123456789abcdef01234567"; + assert_eq!(PinnedRevision::from_selectors(None, None), None); + let tag = PinnedRevision::from_selectors(Some("release/v1"), None).unwrap(); + assert_eq!(tag.fetch_refspec(), "refs/tags/release/v1"); + assert_eq!(tag.expected_sha(), None); + let commit = PinnedRevision::from_selectors(Some("release/v1"), Some(sha)).unwrap(); + assert_eq!(commit.fetch_refspec(), sha); + assert_eq!(commit.expected_sha(), Some(sha)); + assert_eq!( + pinned_fetch_command( + "/repos/acme/widgets", + "origin", + &tag.fetch_refspec(), + Some(10) + ), + "git -c maintenance.auto=0 -c gc.auto=0 -C /repos/acme/widgets fetch --depth 10 --no-tags origin -- refs/tags/release/v1" + ); + assert_eq!( + exact_checkout_verify_command("/repos/acme/widgets", "release", FETCH_HEAD_COMMIT), + "git -c maintenance.auto=0 -c gc.auto=0 -C /repos/acme/widgets checkout -B release FETCH_HEAD'^{commit}' && git -c maintenance.auto=0 -c gc.auto=0 -C /repos/acme/widgets rev-parse HEAD" + ); + } + #[test] fn non_github_origin_fails_without_skip_clone() { let error = decide_clone( @@ -377,6 +511,7 @@ mod tests { Some("https://gitlab.com/acme/widgets.git"), None, None, + None, ) .expect_err("non-GitHub origins should fail"); assert!(error.to_string().contains("GitHub repository origins only")); @@ -392,12 +527,14 @@ mod tests { false, Some("https://github.com/acme/widgets"), Some("moving-branch"), + Some("release"), Some(lowercase), ) .unwrap(), CloneDecision::GitHub { origin_url: "https://github.com/acme/widgets".to_string(), branch: Some("moving-branch".to_string()), + tag: Some("release".to_string()), commit_sha: Some(lowercase.to_string()), } ); @@ -406,12 +543,14 @@ mod tests { false, Some("https://github.com/acme/widgets"), Some("main"), + None, Some(uppercase), ) .unwrap(), CloneDecision::GitHub { origin_url: "https://github.com/acme/widgets".to_string(), branch: Some("main".to_string()), + tag: None, commit_sha: Some(uppercase.to_ascii_lowercase()), } ); @@ -432,6 +571,7 @@ mod tests { false, Some("https://github.com/acme/widgets"), None, + None, Some(sha), ) .expect_err("invalid exact commit SHA should fail"); @@ -443,33 +583,50 @@ mod tests { } #[test] - fn exact_checkout_requires_clone_origin_and_branch() { + fn pinned_checkout_requires_clone_origin_and_branch() { let sha = "0123456789abcdef0123456789abcdef01234567"; - let skip_error = decide_clone( - true, + for (tag, commit_sha) in [(None, Some(sha)), (Some("v1"), None)] { + let skip_error = decide_clone( + true, + Some("https://github.com/acme/widgets"), + Some("main"), + tag, + commit_sha, + ) + .expect_err("pinned checkout with skip-clone should fail"); + assert!(skip_error.to_string().contains("requires cloning")); + + for origin in [None, Some(""), Some(" ")] { + let error = decide_clone(false, origin, Some("main"), tag, commit_sha) + .expect_err("pinned checkout without an origin should fail"); + assert!(error.to_string().contains("requires a repository origin")); + } + + for branch in [None, Some(""), Some(" ")] { + let error = decide_clone( + false, + Some("https://github.com/acme/widgets"), + branch, + tag, + commit_sha, + ) + .expect_err("pinned checkout without a branch should fail"); + assert!(error.to_string().contains("requires a repository branch")); + } + } + } + + #[test] + fn tag_checkout_rejects_empty_tag() { + let empty_tag = decide_clone( + false, Some("https://github.com/acme/widgets"), Some("main"), - Some(sha), + Some(""), + None, ) - .expect_err("exact checkout with skip-clone should fail"); - assert!(skip_error.to_string().contains("requires cloning")); - - for origin in [None, Some(""), Some(" ")] { - let error = decide_clone(false, origin, Some("main"), Some(sha)) - .expect_err("exact checkout without an origin should fail"); - assert!(error.to_string().contains("requires a repository origin")); - } - - for branch in [None, Some(""), Some(" ")] { - let error = decide_clone( - false, - Some("https://github.com/acme/widgets"), - branch, - Some(sha), - ) - .expect_err("exact checkout without a branch should fail"); - assert!(error.to_string().contains("requires a repository branch")); - } + .expect_err("empty tags should fail"); + assert!(empty_tag.to_string().contains("non-empty tag")); } #[test] @@ -479,7 +636,7 @@ mod tests { "https://token@example.com/acme/widgets.git?x=a b", "/repos/acme's widgets", ); - let fetch = exact_fetch_command( + let fetch = pinned_fetch_command( "/repos/acme's widgets", "https://token@example.com/acme/widgets.git?x=a b", sha, @@ -503,9 +660,9 @@ mod tests { } #[test] - fn exact_fetch_omits_depth_for_full_history() { + fn pinned_fetch_omits_depth_for_full_history() { assert_eq!( - exact_fetch_command( + pinned_fetch_command( "/repos/acme/widgets", "origin", "0123456789abcdef0123456789abcdef01234567", @@ -518,15 +675,18 @@ mod tests { #[test] fn exact_checkout_verification_rejects_invalid_or_mismatched_head() { let expected = "0123456789abcdef0123456789abcdef01234567"; - verify_exact_head("0123456789ABCDEF0123456789ABCDEF01234567\n", expected) + let pin = PinnedRevision::Commit(expected.to_string()); + pin.verify_head("0123456789ABCDEF0123456789ABCDEF01234567\n") .expect("uppercase command output should normalize"); - let invalid = verify_exact_head("fatal: not a revision", expected) + let invalid = pin + .verify_head("fatal: not a revision") .expect_err("non-SHA output should fail verification"); assert!(invalid.to_string().contains("invalid HEAD commit ID")); assert!(!invalid.to_string().contains("fatal: not a revision")); - let mismatched = verify_exact_head("1123456789abcdef0123456789abcdef01234567", expected) + let mismatched = pin + .verify_head("1123456789abcdef0123456789abcdef01234567") .expect_err("mismatched SHA should fail verification"); assert!(mismatched.to_string().contains("did not match")); } @@ -536,7 +696,7 @@ mod tests { clippy::disallowed_methods, reason = "hermetic Git proof uses isolated synchronous temp-repository I/O" )] - fn exact_checkout_fetches_admitted_commit_after_branch_advances() { + fn exact_checkout_fetches_admitted_commit_after_branch_and_tag_advance() { let temp = tempfile::tempdir().expect("tempdir"); let remote = temp.path().join("remote.git"); let source = temp.path().join("source"); @@ -561,10 +721,14 @@ mod tests { ]); run_git(&source, &["push", "-u", "origin", "main"]); let admitted_sha = run_git(&source, &["rev-parse", "HEAD"]).trim().to_string(); + run_git(&source, &["tag", "release"]); + run_git(&source, &["push", "origin", "refs/tags/release"]); fs::write(source.join("revision.txt"), "B\n").expect("write commit B"); run_git(&source, &["commit", "-am", "commit B"]); run_git(&source, &["push", "origin", "main"]); + run_git(&source, &["tag", "-f", "release"]); + run_git(&source, &["push", "--force", "origin", "refs/tags/release"]); let advanced_sha = run_git(&source, &["rev-parse", "HEAD"]).trim().to_string(); assert_ne!(admitted_sha, advanced_sha); @@ -576,11 +740,11 @@ mod tests { ); run_shell( temp.path(), - &exact_fetch_command(checkout_path, remote_path, &admitted_sha, Some(10)), + &pinned_fetch_command(checkout_path, remote_path, &admitted_sha, Some(10)), ); let checked_out_sha = run_shell( temp.path(), - &exact_checkout_verify_command(checkout_path, "main", "FETCH_HEAD"), + &exact_checkout_verify_command(checkout_path, "main", FETCH_HEAD_COMMIT), ); assert_eq!(checked_out_sha.trim(), admitted_sha); @@ -609,6 +773,79 @@ mod tests { ); } + #[test] + #[expect( + clippy::disallowed_methods, + reason = "hermetic Git proof uses isolated synchronous temp-repository I/O" + )] + fn tag_checkout_peels_lightweight_and_annotated_tags_without_branch_fallback() { + let temp = tempfile::tempdir().expect("tempdir"); + let remote = temp.path().join("remote.git"); + let source = temp.path().join("source"); + fs::create_dir(&source).expect("source directory"); + + run_git(temp.path(), &[ + "init", + "--bare", + remote.to_str().expect("UTF-8 remote path"), + ]); + run_git(&source, &["init"]); + fs::write(source.join("revision.txt"), "release\n").expect("write release commit"); + run_git(&source, &["add", "revision.txt"]); + run_git(&source, &["commit", "-m", "release"]); + run_git(&source, &["branch", "-M", "main"]); + let release_sha = run_git(&source, &["rev-parse", "HEAD"]).trim().to_string(); + run_git(&source, &["tag", "lightweight"]); + run_git(&source, &["tag", "-a", "annotated", "-m", "annotated"]); + run_git(&source, &[ + "remote", + "add", + "origin", + remote.to_str().expect("UTF-8 remote path"), + ]); + run_git(&source, &["push", "origin", "main", "--tags"]); + + let remote_path = remote.to_str().expect("UTF-8 remote path"); + for tag in ["lightweight", "annotated"] { + let checkout = temp.path().join(format!("checkout-{tag}")); + let checkout_path = checkout.to_str().expect("UTF-8 checkout path"); + run_shell( + temp.path(), + &exact_repository_init_command(remote_path, checkout_path), + ); + run_shell( + temp.path(), + &pinned_fetch_command(checkout_path, "origin", &tag_ref(tag), Some(10)), + ); + let head = run_shell( + temp.path(), + &exact_checkout_verify_command(checkout_path, "release-work", FETCH_HEAD_COMMIT), + ); + + assert_eq!(verify_resolved_head(&head).unwrap(), release_sha); + assert_eq!( + run_git(&checkout, &["symbolic-ref", "HEAD"]).trim(), + "refs/heads/release-work" + ); + } + + let missing = temp.path().join("missing"); + let missing_path = missing.to_str().expect("UTF-8 checkout path"); + run_shell( + temp.path(), + &exact_repository_init_command(remote_path, missing_path), + ); + let output = run_shell_output( + temp.path(), + &pinned_fetch_command(missing_path, "origin", &tag_ref("main"), Some(10)), + ); + assert!( + !output.status.success(), + "a branch must not satisfy a tag fetch" + ); + assert!(!missing.join("revision.txt").exists()); + } + #[test] fn github_layout_maps_ssh_origin_to_repos_checkout_and_workspace_link() { let layout = github_repo_layout( diff --git a/lib/components/fabro-sandbox/src/daytona/mod.rs b/lib/components/fabro-sandbox/src/daytona/mod.rs index 8eda8b64f..b3cbea53d 100644 --- a/lib/components/fabro-sandbox/src/daytona/mod.rs +++ b/lib/components/fabro-sandbox/src/daytona/mod.rs @@ -27,7 +27,7 @@ use tokio::task::JoinHandle; use tokio::{fs, time}; use tokio_util::sync::CancellationToken; -use crate::clone_source::{self, CloneDecision, EmptyWorkspaceReason}; +use crate::clone_source::{self, CloneDecision, EmptyWorkspaceReason, PinnedRevision}; use crate::git_retry::{self, CredentialContext, GitRetryReason}; use crate::push_credentials::{self, PushCredentialState}; use crate::redact::redact_auth_url; @@ -109,6 +109,16 @@ const DAYTONA_STATE_CHANGE_POLL_INTERVAL: Duration = Duration::from_secs(1); /// leaked by a dead worker. An explicit `0` disables auto-stop entirely. const DEFAULT_AUTO_STOP_INTERVAL_MINUTES: i32 = 120; +/// The ref Daytona's native clone checks out. A pinned tag is fetched by its +/// fully-qualified ref so a same-named branch is never consulted; with an +/// exact commit, `commit_id` drives the checkout and the branch is only a name. +fn git_clone_selector(branch: Option<&str>, pin: Option<&PinnedRevision>) -> Option { + match pin { + Some(PinnedRevision::Tag(tag)) => Some(clone_source::tag_ref(tag)), + Some(PinnedRevision::Commit(_)) | None => branch.map(str::to_string), + } +} + pub(crate) fn daytona_not_found(err: &DaytonaError) -> bool { matches!(err, DaytonaError::NotFound { .. }) || err.status_code() == Some(404) } @@ -464,6 +474,7 @@ pub struct DaytonaSandbox { /// Explicit branch to clone. When set, overrides the branch detected by /// the submitted run spec. clone_branch: Option, + clone_tag: Option, clone_commit_sha: Option, } @@ -478,14 +489,16 @@ impl DaytonaSandbox { run_id: Option, clone_origin_url: Option, clone_branch: Option, + clone_tag: Option, clone_commit_sha: Option, api_key: Option, ) -> crate::Result { - if clone_commit_sha.is_some() { + if clone_tag.is_some() || clone_commit_sha.is_some() { clone_source::decide_clone( config.skip_clone, clone_origin_url.as_deref(), clone_branch.as_deref(), + clone_tag.as_deref(), clone_commit_sha.as_deref(), )?; } @@ -512,6 +525,7 @@ impl DaytonaSandbox { run_id, clone_origin_url, clone_branch, + clone_tag, clone_commit_sha, }) } @@ -565,6 +579,7 @@ impl DaytonaSandbox { run_id: None, clone_origin_url, clone_branch, + clone_tag: None, clone_commit_sha: None, }) } @@ -650,25 +665,29 @@ impl DaytonaSandbox { self.fail_init(init_start, err) } - /// Point the admitted branch at the exact commit and verify the resulting - /// HEAD. + /// Point the admitted branch at the pinned revision and verify the + /// resulting HEAD. /// /// Daytona's native clone honors `commit_id`, but leaves the workspace on - /// whatever ref its own checkout produced. Re-pointing the branch keeps the - /// admitted branch name readable back out of the workspace, matching what - /// the Docker provider produces for the same inputs. - async fn attach_exact_commit_branch( + /// whatever ref its own checkout produced (a detached tag, or the exact + /// commit). Re-pointing the branch keeps the admitted branch name readable + /// back out of the workspace, matching what the Docker provider produces + /// for the same inputs. + async fn attach_pinned_branch( process_svc: &daytona_sdk::ProcessService, checkout_path: &str, branch: &str, - expected_sha: &str, + pin: &PinnedRevision, deadline: time::Instant, ) -> crate::Result<()> { + // An exact commit is named directly; a tag clone is already sitting on + // the tag, so peel whatever HEAD points at to its commit. + let revision = pin.expected_sha().unwrap_or("HEAD^{commit}"); Self::run_required_post_clone_command( process_svc, - &clone_source::exact_branch_checkout_command(checkout_path, branch, expected_sha), + &clone_source::exact_branch_checkout_command(checkout_path, branch, revision), "/", - "git checkout exact commit", + "git checkout pinned revision", deadline, ) .await?; @@ -676,11 +695,12 @@ impl DaytonaSandbox { process_svc, &clone_source::exact_head_revision_command(checkout_path), "/", - "git rev-parse HEAD after exact checkout", + "git rev-parse HEAD after pinned checkout", deadline, ) .await?; - clone_source::verify_exact_head(&head, expected_sha) + pin.verify_head(&head)?; + Ok(()) } /// Execute one post-clone command under the shared setup deadline. @@ -1521,6 +1541,7 @@ impl Sandbox for DaytonaSandbox { self.config.skip_clone, self.clone_origin_url.as_deref(), self.clone_branch.as_deref(), + self.clone_tag.as_deref(), self.clone_commit_sha.as_deref(), ) .map_err(|e| self.fail_init(init_start, e))?; @@ -1549,6 +1570,7 @@ impl Sandbox for DaytonaSandbox { CloneDecision::GitHub { origin_url, branch, + tag, commit_sha, } => { let layout = @@ -1647,6 +1669,8 @@ impl Sandbox for DaytonaSandbox { self.fail_init(init_start, err) })?; + let pin = PinnedRevision::from_selectors(tag.as_deref(), commit_sha.as_deref()); + let clone_selector = git_clone_selector(branch.as_deref(), pin.as_ref()); let clone_plan = git_retry::RetryPlan::clone_default(None); let clone_result = git_retry::retry_git_operation( SandboxProviderKind::Daytona, @@ -1657,7 +1681,7 @@ impl Sandbox for DaytonaSandbox { let origin = origin_url.as_str(); let target = layout.primary_repo_path.as_str(); let options = GitCloneOptions { - branch: branch.clone(), + branch: clone_selector.clone(), commit_id: commit_sha.clone(), username: username.clone(), password: password.clone(), @@ -1702,21 +1726,22 @@ impl Sandbox for DaytonaSandbox { } }; - if let Some(expected_sha) = commit_sha.as_deref() { + if let Some(pin) = &pin { let Some(branch) = branch.as_deref().filter(|branch| !branch.trim().is_empty()) else { - let err = crate::Error::message( - "Exact commit checkout requires a repository branch", - ); + let err = crate::Error::message(format!( + "{} requires a repository branch", + pin.label() + )); return Err(self .fail_clone_initialization(sandbox, &origin_url, init_start, err) .await); }; - if let Err(err) = Self::attach_exact_commit_branch( + if let Err(err) = Self::attach_pinned_branch( &process_svc, &layout.primary_repo_path, branch, - expected_sha, + pin, post_clone_deadline, ) .await @@ -3193,6 +3218,25 @@ mod tests { use super::*; use crate::sandbox::BASH_PROBE_MARKER; + #[test] + fn daytona_clone_selector_uses_fully_qualified_tag_unless_sha_is_exact() { + let sha = "0123456789abcdef0123456789abcdef01234567"; + let tag = PinnedRevision::from_selectors(Some("v1.2.3"), None); + assert_eq!( + git_clone_selector(Some("release-work"), tag.as_ref()).as_deref(), + Some("refs/tags/v1.2.3") + ); + let commit = PinnedRevision::from_selectors(Some("v1.2.3"), Some(sha)); + assert_eq!( + git_clone_selector(Some("release-work"), commit.as_ref()).as_deref(), + Some("release-work") + ); + assert_eq!( + git_clone_selector(Some("release-work"), None).as_deref(), + Some("release-work") + ); + } + #[tokio::test] async fn invalid_exact_sha_fails_before_daytona_client_construction() { let error = DaytonaSandbox::new( @@ -3201,6 +3245,7 @@ mod tests { None, Some("https://github.com/acme/widgets".to_string()), Some("main".to_string()), + None, Some("not-a-sha".to_string()), Some("dtn_not_used".to_string()), ) @@ -3220,6 +3265,7 @@ mod tests { None, Some("https://github.com/acme/widgets".to_string()), None, + None, Some("0123456789abcdef0123456789abcdef01234567".to_string()), Some("dtn_not_used".to_string()), ) @@ -3547,6 +3593,7 @@ mod tests { run_id: None, clone_origin_url: None, clone_branch: None, + clone_tag: None, clone_commit_sha: None, } } @@ -3726,6 +3773,7 @@ mod tests { None, None, None, + None, Some("dtn_test".to_string()), ) .await @@ -3765,6 +3813,7 @@ mod tests { None, None, None, + None, Some("dtn_test".to_string()), ) .await @@ -4093,6 +4142,7 @@ mod tests { None, None, None, + None, Some("dtn_test".to_string()), ) .await diff --git a/lib/components/fabro-sandbox/src/docker.rs b/lib/components/fabro-sandbox/src/docker.rs index af69d31e0..8e3abbce7 100644 --- a/lib/components/fabro-sandbox/src/docker.rs +++ b/lib/components/fabro-sandbox/src/docker.rs @@ -156,6 +156,7 @@ pub struct DockerSandbox { run_id: Option, clone_origin_url: Option, clone_branch: Option, + clone_tag: Option, clone_commit_sha: Option, container_id: OnceCell, repo_cloned: OnceCell, @@ -187,13 +188,15 @@ impl DockerSandbox { run_id: Option, clone_origin_url: Option, clone_branch: Option, + clone_tag: Option, clone_commit_sha: Option, ) -> crate::Result { - if clone_commit_sha.is_some() { + if clone_tag.is_some() || clone_commit_sha.is_some() { clone_source::decide_clone( config.skip_clone, clone_origin_url.as_deref(), clone_branch.as_deref(), + clone_tag.as_deref(), clone_commit_sha.as_deref(), )?; } @@ -205,6 +208,7 @@ impl DockerSandbox { run_id, clone_origin_url, clone_branch, + clone_tag, clone_commit_sha, ) } @@ -216,6 +220,7 @@ impl DockerSandbox { run_id: Option, clone_origin_url: Option, clone_branch: Option, + clone_tag: Option, clone_commit_sha: Option, ) -> crate::Result { let push_credentials = PushCredentialState::new(push_credentials::build_token_source( @@ -229,6 +234,7 @@ impl DockerSandbox { run_id, clone_origin_url, clone_branch, + clone_tag, clone_commit_sha, container_id: OnceCell::new(), repo_cloned: OnceCell::new(), @@ -256,6 +262,7 @@ impl DockerSandbox { clone_origin_url.clone(), clone_branch, None, + None, )?; sandbox.validate_managed_container(container_id).await?; sandbox @@ -903,6 +910,7 @@ impl DockerSandbox { &self, origin_url: String, branch: Option, + tag: Option, commit_sha: Option, ) -> crate::Result<()> { self.verify_git_available().await?; @@ -965,13 +973,15 @@ impl DockerSandbox { } let clone_deadline = time::Instant::now() + GIT_CLONE_TIMEOUT; - if let Some(expected_sha) = commit_sha.as_deref() { - // `decide_clone` already rejects an exact commit without a branch; - // re-check here so the checkout can never silently drop the branch - // name callers read back out of the workspace. + if let Some(pin) = + clone_source::PinnedRevision::from_selectors(tag.as_deref(), commit_sha.as_deref()) + { + // `decide_clone` already rejects a pinned revision without a + // branch; re-check here so the checkout can never silently drop the + // branch name callers read back out of the workspace. let Some(branch) = branch.as_deref().filter(|branch| !branch.trim().is_empty()) else { let error = - crate::Error::message("Exact commit checkout requires a repository branch"); + crate::Error::message(format!("{} requires a repository branch", pin.label())); return Err(self.report_clone_failure(&origin_url, error)); }; @@ -980,7 +990,7 @@ impl DockerSandbox { if let Err(error) = self .run_exact_local_git_command( &init_command, - "initialize Docker exact repository checkout", + "initialize Docker pinned repository checkout", clone_deadline, auth_url.as_ref(), ) @@ -989,18 +999,18 @@ impl DockerSandbox { return Err(self.report_clone_failure(&origin_url, error)); } - let fetch_command = clone_source::exact_fetch_command( + let fetch_command = clone_source::pinned_fetch_command( &layout.primary_repo_path, "origin", - expected_sha, + &pin.fetch_refspec(), self.config.clone_depth, ); if let Err(failure) = self .retry_git_transfer( &fetch_command, "fetch", - "Docker exact fetch", - "git fetch exact commit", + "Docker pinned fetch", + "git fetch pinned revision", clone_deadline, clone_credential_context, auth_url.as_ref(), @@ -1013,12 +1023,12 @@ impl DockerSandbox { let checkout_command = clone_source::exact_checkout_verify_command( &layout.primary_repo_path, branch, - "FETCH_HEAD", + clone_source::FETCH_HEAD_COMMIT, ); let head = match self .run_exact_local_git_command( &checkout_command, - "git checkout exact commit", + "git checkout pinned revision", clone_deadline, auth_url.as_ref(), ) @@ -1027,7 +1037,7 @@ impl DockerSandbox { Ok(result) => result, Err(error) => return Err(self.report_clone_failure(&origin_url, error)), }; - if let Err(error) = clone_source::verify_exact_head(&head.stdout, expected_sha) { + if let Err(error) = pin.verify_head(&head.stdout) { return Err(self.report_clone_failure(&origin_url, error)); } } else { @@ -1826,6 +1836,7 @@ impl Sandbox for DockerSandbox { self.config.skip_clone, self.clone_origin_url.as_deref(), self.clone_branch.as_deref(), + self.clone_tag.as_deref(), self.clone_commit_sha.as_deref(), ) .map_err(|e| self.fail_init(init_start, e))?; @@ -1847,9 +1858,13 @@ impl Sandbox for DockerSandbox { CloneDecision::GitHub { origin_url, branch, + tag, commit_sha, } => { - if let Err(e) = self.clone_github_repo(origin_url, branch, commit_sha).await { + if let Err(e) = self + .clone_github_repo(origin_url, branch, tag, commit_sha) + .await + { return Err(self.fail_init(init_start, e)); } } @@ -2668,6 +2683,7 @@ mod tests { None, Some("https://github.com/acme/widgets".to_string()), Some("main".to_string()), + None, Some("not-a-sha".to_string()), ) .err() @@ -2685,6 +2701,7 @@ mod tests { None, Some("https://github.com/acme/widgets".to_string()), None, + None, Some("0123456789abcdef0123456789abcdef01234567".to_string()), ) .err() @@ -3072,6 +3089,7 @@ mod tests { None, None, None, + None, ) .expect("test sandbox should build"); sandbox diff --git a/lib/components/fabro-sandbox/src/provider/daytona.rs b/lib/components/fabro-sandbox/src/provider/daytona.rs index 45b212181..b43a8e8d5 100644 --- a/lib/components/fabro-sandbox/src/provider/daytona.rs +++ b/lib/components/fabro-sandbox/src/provider/daytona.rs @@ -130,6 +130,7 @@ impl SandboxProvider for DaytonaSandboxProvider { clone_origin_url, clone_branch, None, + None, Some(api_key), ) .await?; diff --git a/lib/components/fabro-sandbox/src/provider/docker.rs b/lib/components/fabro-sandbox/src/provider/docker.rs index 37337eaed..008b3b537 100644 --- a/lib/components/fabro-sandbox/src/provider/docker.rs +++ b/lib/components/fabro-sandbox/src/provider/docker.rs @@ -105,6 +105,7 @@ impl SandboxProvider for DockerSandboxProvider { clone_origin_url, clone_branch, None, + None, )?; sandbox.initialize().await?; let container_id = sandbox.container_identifier()?.to_string(); diff --git a/lib/components/fabro-sandbox/src/sandbox_spec.rs b/lib/components/fabro-sandbox/src/sandbox_spec.rs index 06565eb95..6c8c2ec64 100644 --- a/lib/components/fabro-sandbox/src/sandbox_spec.rs +++ b/lib/components/fabro-sandbox/src/sandbox_spec.rs @@ -32,6 +32,7 @@ pub enum SandboxSpec { run_id: Option, clone_origin_url: Option, clone_branch: Option, + clone_tag: Option, clone_commit_sha: Option, }, #[cfg(feature = "daytona")] @@ -41,6 +42,7 @@ pub enum SandboxSpec { run_id: Option, clone_origin_url: Option, clone_branch: Option, + clone_tag: Option, clone_commit_sha: Option, api_key: Option, }, @@ -204,6 +206,7 @@ impl SandboxSpec { run_id, clone_origin_url, clone_branch, + clone_tag, clone_commit_sha, } => { let mut sandbox = DockerSandbox::new( @@ -212,6 +215,7 @@ impl SandboxSpec { *run_id, clone_origin_url.clone(), clone_branch.clone(), + clone_tag.clone(), clone_commit_sha.clone(), ) .context("Failed to create Docker sandbox")?; @@ -227,6 +231,7 @@ impl SandboxSpec { run_id, clone_origin_url, clone_branch, + clone_tag, clone_commit_sha, api_key, } => { @@ -236,6 +241,7 @@ impl SandboxSpec { *run_id, clone_origin_url.clone(), clone_branch.clone(), + clone_tag.clone(), clone_commit_sha.clone(), api_key.clone(), ) @@ -282,6 +288,7 @@ mod tests { run_id: None, clone_origin_url: Some("git@github.com:brynary/rack-test.git".to_string()), clone_branch: Some("main".to_string()), + clone_tag: None, clone_commit_sha: None, }; let mut sandbox = MockSandbox::linux(); @@ -320,6 +327,7 @@ mod tests { run_id: None, clone_origin_url: Some("https://github.com/acme/widgets".to_string()), clone_branch: Some("main".to_string()), + clone_tag: None, clone_commit_sha: Some("not-a-sha".to_string()), }; @@ -349,6 +357,7 @@ mod tests { run_id: None, clone_origin_url: Some("https://gitlab.com/acme/widgets".to_string()), clone_branch: None, + clone_tag: None, clone_commit_sha: None, }; let mut sandbox = MockSandbox::linux(); diff --git a/lib/components/fabro-sandbox/tests/daytona_streaming_live.rs b/lib/components/fabro-sandbox/tests/daytona_streaming_live.rs index 9fdaf8fb8..dc5e2e7bf 100644 --- a/lib/components/fabro-sandbox/tests/daytona_streaming_live.rs +++ b/lib/components/fabro-sandbox/tests/daytona_streaming_live.rs @@ -38,6 +38,7 @@ mod daytona_streaming_live { None, None, None, + None, ) .await?, ); @@ -76,6 +77,7 @@ mod daytona_streaming_live { None, None, None, + None, ) .await?; sandbox.initialize().await?; @@ -182,6 +184,7 @@ mod daytona_streaming_live { None, None, None, + None, ) .await?; @@ -232,6 +235,7 @@ mod daytona_streaming_live { None, None, None, + None, ) .await?; @@ -300,6 +304,7 @@ mod daytona_streaming_live { None, None, None, + None, ) .await?; diff --git a/lib/components/fabro-sandbox/tests/docker_streaming.rs b/lib/components/fabro-sandbox/tests/docker_streaming.rs index 1dfca5555..0ee733f38 100644 --- a/lib/components/fabro-sandbox/tests/docker_streaming.rs +++ b/lib/components/fabro-sandbox/tests/docker_streaming.rs @@ -41,6 +41,7 @@ async fn streaming_timeout_terminates_docker_exec_before_returning() { None, None, None, + None, ) .expect("docker sandbox should construct"); sandbox @@ -114,6 +115,7 @@ async fn streaming_command_receives_exact_stdin_and_eof() { None, None, None, + None, ) .expect("docker sandbox should construct"); sandbox @@ -176,6 +178,7 @@ async fn cloned_docker_sandbox_uses_repos_checkout_and_workspace_symlink() { Some("https://github.com/brynary/rack-test".to_string()), None, None, + None, ) .expect("docker sandbox should construct"); sandbox @@ -243,6 +246,7 @@ async fn docker_runs_clean_bash_through_both_command_paths() { None, None, None, + None, ) .expect("docker sandbox should construct"); sandbox @@ -337,6 +341,7 @@ async fn docker_glob_matches_patterns_containing_a_path_separator() { None, None, None, + None, ) .expect("docker sandbox should construct"); sandbox @@ -422,6 +427,7 @@ async fn docker_runtime_directory_is_private_and_outside_workspace() { None, None, None, + None, ) .expect("docker sandbox should construct"); sandbox diff --git a/lib/components/fabro-workflow/src/operations/create.rs b/lib/components/fabro-workflow/src/operations/create.rs index 79141999b..5e76d17e8 100644 --- a/lib/components/fabro-workflow/src/operations/create.rs +++ b/lib/components/fabro-workflow/src/operations/create.rs @@ -478,7 +478,7 @@ pub async fn persist_create_run( let (source_directory, git) = match target.as_ref() { Some(RunTarget::None {}) => (None, None), Some(RunTarget::Folder { path }) => (Some(path.clone()), git), - Some(RunTarget::Git { .. }) | None => (Some(source_directory), git), + Some(RunTarget::Git(_)) | None => (Some(source_directory), git), }; let persisted_run_dir = run_dir.clone(); let persisted = spawn_blocking(move || { diff --git a/lib/components/fabro-workflow/src/operations/fork.rs b/lib/components/fabro-workflow/src/operations/fork.rs index f7eb00bcc..0327ba1bb 100644 --- a/lib/components/fabro-workflow/src/operations/fork.rs +++ b/lib/components/fabro-workflow/src/operations/fork.rs @@ -407,11 +407,12 @@ mod tests { source_directory: Some("/client/source".to_string()), workflow_slug: Some("fork-source".to_string()), workflow_version_id: Some(workflow_version_id), - target: Some(fabro_types::RunTarget::Git { + target: Some(fabro_types::RunTarget::Git(fabro_types::GitRunTarget { repo: "example/repo".to_string(), branch: "main".to_string(), + tag: None, sha: None, - }), + })), automation: None, provenance: test_support::test_run_provenance(), manifest_blob: None, @@ -500,11 +501,12 @@ mod tests { ); assert_eq!( forked_state.spec.target, - Some(fabro_types::RunTarget::Git { + Some(fabro_types::RunTarget::Git(fabro_types::GitRunTarget { repo: "example/repo".to_string(), branch: "main".to_string(), + tag: None, sha: None, - }) + })) ); assert_eq!( forked_state.spec.fork_source_ref.unwrap().source_run_id, diff --git a/lib/components/fabro-workflow/src/operations/retry.rs b/lib/components/fabro-workflow/src/operations/retry.rs index 51990d671..fd386bb36 100644 --- a/lib/components/fabro-workflow/src/operations/retry.rs +++ b/lib/components/fabro-workflow/src/operations/retry.rs @@ -170,11 +170,12 @@ mod tests { } fn run_target() -> RunTarget { - RunTarget::Git { + RunTarget::Git(fabro_types::GitRunTarget { repo: "fabro-sh/fabro".to_string(), branch: "main".to_string(), + tag: None, sha: Some("abcdef0123456789abcdef0123456789abcdef01".to_string()), - } + }) } async fn append_created( diff --git a/lib/components/fabro-workflow/src/operations/start.rs b/lib/components/fabro-workflow/src/operations/start.rs index 600cbee1f..20edc4800 100644 --- a/lib/components/fabro-workflow/src/operations/start.rs +++ b/lib/components/fabro-workflow/src/operations/start.rs @@ -16,6 +16,8 @@ use fabro_sandbox::from_environment::{ }; use fabro_sandbox::{DockerSandboxOptions, SandboxSpec}; use fabro_static::EnvVars; +#[cfg(test)] +use fabro_types::GitRunTarget; use fabro_types::settings::run::{ ApprovalMode, McpServerSettings as ResolvedMcpServerSettings, PullRequestSettings, ResolvedGithubIntegration, ResolvedMcpEntry, RunMode, RunNamespace as ResolvedRunSettings, @@ -470,7 +472,7 @@ impl RunSession { } let sandbox = match sandbox_provider { SandboxProviderKind::Local => match record.target.as_ref() { - Some(target @ (RunTarget::Git { .. } | RunTarget::None {})) => { + Some(target @ (RunTarget::Git(_) | RunTarget::None {})) => { return Err(Error::engine(format!( "persisted {} run targets require a clone-based sandbox provider", target.kind_name() @@ -502,6 +504,7 @@ impl RunSession { run_id: Some(record.run_id), clone_origin_url: clone_source.origin_url, clone_branch: clone_source.branch, + clone_tag: clone_source.tag, clone_commit_sha: clone_source.commit_sha, } } @@ -517,6 +520,7 @@ impl RunSession { run_id: Some(record.run_id), clone_origin_url: clone_source.origin_url, clone_branch: clone_source.branch, + clone_tag: clone_source.tag, clone_commit_sha: clone_source.commit_sha, api_key, } @@ -599,6 +603,7 @@ impl RunSession { struct CloneSourceForRun { origin_url: Option, branch: Option, + tag: Option, commit_sha: Option, /// The target asked for an empty workspace, so the provider must not /// clone even when it would otherwise inherit an origin. @@ -652,6 +657,7 @@ fn clone_source_for_run(record: &RunSpec) -> Result { return Ok(CloneSourceForRun { origin_url: record.repo_origin_url().map(str::to_string), branch: record.base_branch().map(str::to_string), + tag: None, commit_sha: None, skip_clone: false, }); @@ -668,22 +674,25 @@ fn clone_source_for_run(record: &RunSpec) -> Result { "persisted Git run target has an invalid repository slug" } TargetValidationError::Branch => "persisted Git run target has an invalid branch", + TargetValidationError::Tag => "persisted Git run target has an invalid tag", TargetValidationError::Sha => "persisted Git run target has an invalid SHA", }) })?; // A target with no Git projection (`none` or `folder`) supplies no clone // source. Folder targets only reach the Local provider, where `skip_clone` // is unused. - Ok(match validated.git { - Some(git) => CloneSourceForRun { + Ok(match (validated.target, validated.git) { + (RunTarget::Git(target), Some(git)) => CloneSourceForRun { origin_url: Some(git.origin_url), - branch: Some(git.branch), + branch: Some(target.branch), + tag: target.tag, commit_sha: git.sha, skip_clone: false, }, - None => CloneSourceForRun { + _ => CloneSourceForRun { origin_url: None, branch: None, + tag: None, commit_sha: None, skip_clone: true, }, @@ -3175,11 +3184,12 @@ reasoning = false let mut spec = test_support::test_run_spec(); let submitted_sha = "ABCDEF0123456789ABCDEF0123456789ABCDEF01"; let normalized_sha = "abcdef0123456789abcdef0123456789abcdef01"; - spec.target = Some(RunTarget::Git { + spec.target = Some(RunTarget::Git(GitRunTarget { repo: "fabro-sh/fabro".to_string(), branch: "feature/run-intent".to_string(), + tag: Some("v1.2.3".to_string()), sha: Some(submitted_sha.to_string()), - }); + })); spec.git = Some(fabro_types::GitContext { origin_url: "https://github.com/fabro-sh/fabro".to_string(), branch: "feature/run-intent".to_string(), @@ -3194,17 +3204,19 @@ reasoning = false Some("https://github.com/fabro-sh/fabro") ); assert_eq!(source.branch.as_deref(), Some("feature/run-intent")); + assert_eq!(source.tag.as_deref(), Some("v1.2.3")); assert_eq!(source.commit_sha.as_deref(), Some(normalized_sha)); } #[test] fn clone_commit_persisted_git_target_without_sha_keeps_branch_unpinned() { let mut spec = test_support::test_run_spec(); - spec.target = Some(RunTarget::Git { + spec.target = Some(RunTarget::Git(GitRunTarget { repo: "fabro-sh/fabro".to_string(), branch: "feature/run-intent".to_string(), + tag: None, sha: None, - }); + })); spec.git = Some(fabro_types::GitContext { origin_url: "https://github.com/fabro-sh/fabro".to_string(), branch: "feature/run-intent".to_string(), @@ -3218,14 +3230,32 @@ reasoning = false assert_eq!(source.commit_sha, None); } + #[test] + fn clone_source_preserves_unpinned_tag_separately_from_working_branch() { + let mut spec = test_support::test_run_spec(); + spec.target = Some(RunTarget::Git(GitRunTarget { + repo: "fabro-sh/fabro".to_string(), + branch: "release-work".to_string(), + tag: Some("v1.2.3".to_string()), + sha: None, + })); + + let source = clone_source_for_run(&spec).unwrap(); + + assert_eq!(source.branch.as_deref(), Some("release-work")); + assert_eq!(source.tag.as_deref(), Some("v1.2.3")); + assert_eq!(source.commit_sha, None); + } + #[test] fn clone_commit_persisted_git_target_is_authoritative_over_projection() { let mut spec = test_support::test_run_spec(); - spec.target = Some(RunTarget::Git { + spec.target = Some(RunTarget::Git(GitRunTarget { repo: "fabro-sh/fabro".to_string(), branch: "main".to_string(), + tag: None, sha: None, - }); + })); // A drifted (or absent) projection never feeds the clone source: the // validated target alone does. spec.git = Some(fabro_types::GitContext { diff --git a/lib/components/fabro-workflow/tests/it/daytona_integration.rs b/lib/components/fabro-workflow/tests/it/daytona_integration.rs index 313befccb..d183e74c0 100644 --- a/lib/components/fabro-workflow/tests/it/daytona_integration.rs +++ b/lib/components/fabro-workflow/tests/it/daytona_integration.rs @@ -208,6 +208,7 @@ async fn create_env_with_github_app( None, None, None, + None, ) .await .expect("Failed to create Daytona client — is DAYTONA_API_KEY set?") @@ -413,7 +414,7 @@ async fn daytona_snapshot_sandbox() { }; let creds = load_github_app_credentials(); - let env = DaytonaSandbox::new(config, Some(creds), None, None, None, None, None) + let env = DaytonaSandbox::new(config, Some(creds), None, None, None, None, None, None) .await .expect("Failed to create Daytona client — is DAYTONA_API_KEY set?"); env.initialize().await.unwrap(); @@ -1618,7 +1619,7 @@ async fn daytona_computer_use_browser_screenshot() { skip_clone: true, ..DaytonaConfig::default() }; - let env = DaytonaSandbox::new(config, None, None, None, None, None, None) + let env = DaytonaSandbox::new(config, None, None, None, None, None, None, None) .await .expect("DAYTONA_API_KEY must be set"); env.initialize().await.unwrap(); @@ -1766,7 +1767,7 @@ async fn daytona_playwright_mcp_sandbox_transport() { skip_clone: true, ..DaytonaConfig::default() }; - let sandbox = DaytonaSandbox::new(config, None, None, None, None, None, None) + let sandbox = DaytonaSandbox::new(config, None, None, None, None, None, None, None) .await .expect("DAYTONA_API_KEY must be set"); sandbox.initialize().await.unwrap(); diff --git a/lib/components/fabro-workflow/tests/it/integration.rs b/lib/components/fabro-workflow/tests/it/integration.rs index 2f6602cf5..64fb3d91b 100644 --- a/lib/components/fabro-workflow/tests/it/integration.rs +++ b/lib/components/fabro-workflow/tests/it/integration.rs @@ -13737,7 +13737,7 @@ async fn asset_collection_docker_sandbox() { ..Default::default() }; let sandbox: Arc = Arc::new( - fabro_agent::DockerSandbox::new(config, None, None, None, None, None) + fabro_agent::DockerSandbox::new(config, None, None, None, None, None, None) .expect("Docker not available"), ); sandbox.initialize().await.expect("Docker init failed"); diff --git a/lib/foundation/fabro-api/build.rs b/lib/foundation/fabro-api/build.rs index 7a79fbfb2..852cd87fb 100644 --- a/lib/foundation/fabro-api/build.rs +++ b/lib/foundation/fabro-api/build.rs @@ -739,6 +739,7 @@ fn main() { ("RunIntent", "fabro_types::RunIntent", &[]), ("RunIntentArgs", "fabro_types::RunIntentArgs", &[]), ("RunTarget", "fabro_types::RunTarget", &[]), + ("GitRunTarget", "fabro_types::GitRunTarget", &[]), ("WorkflowPath", "fabro_types::WorkflowPath", &[]), ("WorkflowVersionId", "fabro_types::WorkflowVersionId", &[]), ("BlobHash", "fabro_types::BlobHash", &[]), diff --git a/lib/foundation/fabro-api/src/lib.rs b/lib/foundation/fabro-api/src/lib.rs index 59a72d5e6..34c16b569 100644 --- a/lib/foundation/fabro-api/src/lib.rs +++ b/lib/foundation/fabro-api/src/lib.rs @@ -48,7 +48,7 @@ pub mod types { AgentToolsAvailableProps, AskFabro, AuthMethod, AutomationRef, BilledTokenCounts, BlobHash, CommandTermination, Conclusion, ContentPart, CreateVariableRequest, DiffStats, DiffSummary, DirtyStatus, EventEnvelope, ExecOutputTail, FailureCategory, FailureDetail, - FailureSignature, GitContext, IdpIdentity, IntegrationConnectionKind, + FailureSignature, GitContext, GitRunTarget, IdpIdentity, IntegrationConnectionKind, IntegrationConnectionState, IntegrationConnectionStatus, IntegrationProvider, IntegrationStatus, InterviewOption, InterviewQuestionRecord, LlmOutputKind, McpServerDraft as CreateMcpServerRequest, McpServerProjection, diff --git a/lib/foundation/fabro-api/tests/run_intent_round_trip.rs b/lib/foundation/fabro-api/tests/run_intent_round_trip.rs index 62d49b24b..475d12141 100644 --- a/lib/foundation/fabro-api/tests/run_intent_round_trip.rs +++ b/lib/foundation/fabro-api/tests/run_intent_round_trip.rs @@ -2,7 +2,7 @@ use std::any::{TypeId, type_name}; use std::collections::HashMap; use fabro_api::types::{RunIntent as ApiRunIntent, RunIntentArgs as ApiRunIntentArgs}; -use fabro_types::{RunIntent, RunIntentArgs, RunTarget, test_support}; +use fabro_types::{GitRunTarget, RunIntent, RunIntentArgs, RunTarget, test_support}; use serde_json::json; #[test] @@ -10,17 +10,19 @@ fn run_intent_schemas_reuse_canonical_types() { assert_same_type::(); assert_same_type::(); assert_same_type::(); + assert_same_type::(); } #[test] fn run_intent_round_trips_the_openapi_shape() { let intent = RunIntent { workflow_version_id: test_support::test_workflow_version_id(), - target: RunTarget::Git { + target: RunTarget::Git(GitRunTarget { repo: "fabro-sh/fabro".to_string(), branch: "feature/run-intent".to_string(), + tag: Some("v1.2.3".to_string()), sha: Some("abcdef0123456789abcdef0123456789abcdef01".to_string()), - }, + }), args: RunIntentArgs { model: Some("gpt-5.6-sol".to_string()), provider: Some("openai".to_string()), diff --git a/lib/foundation/fabro-types/src/lib.rs b/lib/foundation/fabro-types/src/lib.rs index 704c2e9e9..9848a3560 100644 --- a/lib/foundation/fabro-types/src/lib.rs +++ b/lib/foundation/fabro-types/src/lib.rs @@ -131,7 +131,7 @@ pub use run_event::{ pub use run_failure::RunFailure; pub use run_id::{RunId, fixtures}; pub use run_intent::{ - RunIntent, RunIntentArgs, RunTarget, TargetValidationError, ValidatedRunTarget, + GitRunTarget, RunIntent, RunIntentArgs, RunTarget, TargetValidationError, ValidatedRunTarget, }; pub use run_projection::{ ActivatedSkill, AgentControlState, CheckpointRecord, McpServerProjection, McpServerStatus, diff --git a/lib/foundation/fabro-types/src/run_intent.rs b/lib/foundation/fabro-types/src/run_intent.rs index 867160e3d..1deb67931 100644 --- a/lib/foundation/fabro-types/src/run_intent.rs +++ b/lib/foundation/fabro-types/src/run_intent.rs @@ -45,16 +45,39 @@ pub struct RunIntentArgs { #[serde(tag = "kind", rename_all = "snake_case", deny_unknown_fields)] #[strum(serialize_all = "snake_case")] pub enum RunTarget { - Git { - repo: String, - branch: String, - #[serde(default, skip_serializing_if = "Option::is_none")] - sha: Option, - }, + Git(GitRunTarget), None {}, - Folder { - path: String, - }, + Folder { path: String }, +} + +/// A Git-backed run target. +/// +/// `branch` is always the attached working branch. When present, `tag` names +/// the requested release identity. An exact `sha` is authoritative over both +/// selectors while preserving the tag in durable state. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct GitRunTarget { + pub repo: String, + pub branch: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub tag: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub sha: Option, +} + +/// A bare branch or tag name: not `HEAD`, not a `refs/` or `tags/` selector, +/// not a commit SHA, and otherwise a valid GitHub ref selector. +/// +/// The selector grammar is checked on the bare name so its leading-character +/// rules apply to the name itself, not to a prefixed selector that would mask +/// them. +fn is_bare_ref_name(name: &str) -> bool { + name != "HEAD" + && !name.starts_with("tags/") + && !name.starts_with("refs/") + && repository::normalize_git_commit_sha(name).is_none() + && repository::is_valid_github_ref_selector(name) } impl RunTarget { @@ -71,21 +94,20 @@ impl RunTarget { /// filesystem validation and canonicalization during provider admission. pub fn validate(self) -> Result { match self { - Self::Git { repo, branch, sha } => { + Self::Git(GitRunTarget { + repo, + branch, + tag, + sha, + }) => { let slug = GitHubRepositorySlug::try_new(&repo) .ok_or(TargetValidationError::Repository)?; - // The selector grammar is checked on the bare branch name so - // its leading-character rules apply to the branch itself, not - // to a `heads/`-prefixed selector that would mask them. - if branch == "HEAD" - || branch.starts_with("heads/") - || branch.starts_with("tags/") - || branch.starts_with("refs/") - || repository::normalize_git_commit_sha(&branch).is_some() - || !repository::is_valid_github_ref_selector(&branch) - { + if !is_bare_ref_name(&branch) || branch.starts_with("heads/") { return Err(TargetValidationError::Branch); } + if tag.as_deref().is_some_and(|tag| !is_bare_ref_name(tag)) { + return Err(TargetValidationError::Tag); + } let sha = sha .map(|sha| { repository::normalize_git_commit_sha(&sha).ok_or(TargetValidationError::Sha) @@ -98,7 +120,12 @@ impl RunTarget { dirty: DirtyStatus::Clean, }; Ok(ValidatedRunTarget { - target: Self::Git { repo, branch, sha }, + target: Self::Git(GitRunTarget { + repo, + branch, + tag, + sha, + }), git: Some(git), }) } @@ -129,6 +156,8 @@ pub enum TargetValidationError { Repository, #[error("target branch must be a non-empty branch name, not a ref or commit selector")] Branch, + #[error("target tag must be a non-empty bare tag name, not a ref or commit selector")] + Tag, #[error("target SHA must be exactly 40 ASCII hexadecimal characters")] Sha, } diff --git a/lib/foundation/fabro-types/tests/run_event_serde.rs b/lib/foundation/fabro-types/tests/run_event_serde.rs index f90b0f70a..0e11baca7 100644 --- a/lib/foundation/fabro-types/tests/run_event_serde.rs +++ b/lib/foundation/fabro-types/tests/run_event_serde.rs @@ -7,7 +7,9 @@ use fabro_types::run_event::{RunSessionTurnFailedCode, RunSessionTurnFailedProps use fabro_types::settings::InterpString; use fabro_types::settings::run::RunGoal; use fabro_types::test_support::{test_run_provenance, test_workflow_version_id}; -use fabro_types::{AutomationRef, EventBody, RunTarget, TurnId, WorkflowSettings, fixtures}; +use fabro_types::{ + AutomationRef, EventBody, GitRunTarget, RunTarget, TurnId, WorkflowSettings, fixtures, +}; fn templated_settings() -> WorkflowSettings { let mut settings = WorkflowSettings::default(); @@ -26,11 +28,12 @@ fn run_created_props_round_trip_templated_settings() { source_directory: Some("/Users/client/project".to_string()), workflow_slug: Some("demo".to_string()), workflow_version_id: Some(test_workflow_version_id()), - target: Some(RunTarget::Git { + target: Some(RunTarget::Git(GitRunTarget { repo: "fabro-sh/fabro".to_string(), branch: "main".to_string(), + tag: None, sha: None, - }), + })), automation: Some(AutomationRef { id: "nightly".to_string(), name: Some("Nightly".to_string()), diff --git a/lib/foundation/fabro-types/tests/run_intent.rs b/lib/foundation/fabro-types/tests/run_intent.rs index 6fcf5cd82..57b2e3c0d 100644 --- a/lib/foundation/fabro-types/tests/run_intent.rs +++ b/lib/foundation/fabro-types/tests/run_intent.rs @@ -1,7 +1,7 @@ use std::collections::HashMap; use fabro_types::{ - RunIntent, RunIntentArgs, RunTarget, WorkflowVersionId, normalize_git_commit_sha, + GitRunTarget, RunIntent, RunIntentArgs, RunTarget, WorkflowVersionId, normalize_git_commit_sha, }; use serde_json::json; @@ -14,11 +14,12 @@ fn version_id() -> WorkflowVersionId { fn intent() -> RunIntent { RunIntent { workflow_version_id: version_id(), - target: RunTarget::Git { + target: RunTarget::Git(GitRunTarget { repo: "fabro-sh/fabro".to_string(), branch: "feature/run-intent".to_string(), + tag: Some("v1.2.3".to_string()), sha: Some("ABCDEF0123456789ABCDEF0123456789ABCDEF01".to_string()), - }, + }), args: RunIntentArgs { model: Some("gpt-5.6".to_string()), provider: Some("openai".to_string()), @@ -43,6 +44,7 @@ fn run_intent_round_trips_the_strict_git_shape() { assert_eq!(value["target"]["kind"], "git"); assert_eq!(value["target"]["repo"], "fabro-sh/fabro"); assert_eq!(value["target"]["branch"], "feature/run-intent"); + assert_eq!(value["target"]["tag"], "v1.2.3"); assert_eq!( serde_json::from_value::(value).expect("intent should deserialize"), intent @@ -158,11 +160,12 @@ fn git_commit_sha_normalization_is_exact_and_pure() { #[test] fn target_validation_normalizes_sha_without_network_resolution() { - let validated = RunTarget::Git { + let validated = RunTarget::Git(GitRunTarget { repo: "fabro-sh/fabro".to_string(), branch: "feature/run-intent".to_string(), + tag: Some("release/v1".to_string()), sha: Some("ABCDEF0123456789ABCDEF0123456789ABCDEF01".to_string()), - } + }) .validate() .unwrap(); let git = validated @@ -174,11 +177,15 @@ fn target_validation_normalizes_sha_without_network_resolution() { Some("abcdef0123456789abcdef0123456789abcdef01") ); assert_eq!(git.origin_url, "https://github.com/fabro-sh/fabro"); - assert_eq!(validated.target, RunTarget::Git { - repo: "fabro-sh/fabro".to_string(), - branch: "feature/run-intent".to_string(), - sha: Some("abcdef0123456789abcdef0123456789abcdef01".to_string()), - }); + assert_eq!( + validated.target, + RunTarget::Git(GitRunTarget { + repo: "fabro-sh/fabro".to_string(), + branch: "feature/run-intent".to_string(), + tag: Some("release/v1".to_string()), + sha: Some("abcdef0123456789abcdef0123456789abcdef01".to_string()), + }) + ); } #[test] @@ -204,17 +211,18 @@ fn run_intent_folder_target_is_preserved_for_provider_admission() { fn target_validation_rejects_invalid_grammar() { use fabro_types::TargetValidationError; - let validate = |repo: &str, branch: &str, sha: Option<&str>| { - RunTarget::Git { + let validate = |repo: &str, branch: &str, tag: Option<&str>, sha: Option<&str>| { + RunTarget::Git(GitRunTarget { repo: repo.to_string(), branch: branch.to_string(), + tag: tag.map(str::to_string), sha: sha.map(str::to_string), - } + }) .validate() }; assert_eq!( - validate("not-a-slug", "main", None).unwrap_err(), + validate("not-a-slug", "main", None, None).unwrap_err(), TargetValidationError::Repository ); for branch in [ @@ -229,13 +237,66 @@ fn target_validation_rejects_invalid_grammar() { "bad..branch", ] { assert_eq!( - validate("fabro-sh/fabro", branch, None).unwrap_err(), + validate("fabro-sh/fabro", branch, None, None).unwrap_err(), TargetValidationError::Branch, "{branch:?}" ); } + for tag in [ + "", + "HEAD", + "-v1", + ".v1", + "tags/v1", + "refs/tags/v1", + "abcdef0123456789abcdef0123456789abcdef01", + "bad..tag", + ] { + assert_eq!( + validate("fabro-sh/fabro", "main", Some(tag), None).unwrap_err(), + TargetValidationError::Tag, + "{tag:?}" + ); + } assert_eq!( - validate("fabro-sh/fabro", "main", Some("short")).unwrap_err(), + validate("fabro-sh/fabro", "main", None, Some("short")).unwrap_err(), TargetValidationError::Sha ); } + +#[test] +fn git_target_round_trips_all_branch_tag_sha_states() { + for target in [ + GitRunTarget { + repo: "fabro-sh/fabro".to_string(), + branch: "main".to_string(), + tag: None, + sha: None, + }, + GitRunTarget { + repo: "fabro-sh/fabro".to_string(), + branch: "main".to_string(), + tag: None, + sha: Some("abcdef0123456789abcdef0123456789abcdef01".to_string()), + }, + GitRunTarget { + repo: "fabro-sh/fabro".to_string(), + branch: "release".to_string(), + tag: Some("v1.2.3".to_string()), + sha: None, + }, + GitRunTarget { + repo: "fabro-sh/fabro".to_string(), + branch: "release".to_string(), + tag: Some("v1.2.3".to_string()), + sha: Some("abcdef0123456789abcdef0123456789abcdef01".to_string()), + }, + ] { + let run_target = RunTarget::Git(target); + let value = serde_json::to_value(&run_target).expect("target should serialize"); + assert_eq!( + serde_json::from_value::(value).expect("target should deserialize"), + run_target + ); + } +} diff --git a/lib/foundation/fabro-types/tests/run_spec_serde.rs b/lib/foundation/fabro-types/tests/run_spec_serde.rs index c2519deaa..f052acd9a 100644 --- a/lib/foundation/fabro-types/tests/run_spec_serde.rs +++ b/lib/foundation/fabro-types/tests/run_spec_serde.rs @@ -5,7 +5,7 @@ use fabro_types::run::{DirtyStatus, ForkSourceRef, GitContext, RunSpec}; use fabro_types::settings::InterpString; use fabro_types::settings::run::RunGoal; use fabro_types::test_support::{test_run_provenance, test_workflow_version_id}; -use fabro_types::{AutomationRef, RunTarget, WorkflowSettings, fixtures}; +use fabro_types::{AutomationRef, GitRunTarget, RunTarget, WorkflowSettings, fixtures}; fn templated_settings() -> WorkflowSettings { let mut settings = WorkflowSettings::default(); @@ -22,11 +22,12 @@ fn run_spec_round_trips_templated_settings() { graph_source: None, workflow_slug: Some("demo".to_string()), workflow_version_id: Some(test_workflow_version_id()), - target: Some(RunTarget::Git { + target: Some(RunTarget::Git(GitRunTarget { repo: "fabro-sh/fabro".to_string(), branch: "main".to_string(), + tag: None, sha: Some("abc123".to_string()), - }), + })), automation: Some(AutomationRef { id: "nightly".to_string(), name: Some("Nightly".to_string()), diff --git a/lib/packages/fabro-api-client/src/models/git-run-target.ts b/lib/packages/fabro-api-client/src/models/git-run-target.ts index 4805feff4..504db9687 100644 --- a/lib/packages/fabro-api-client/src/models/git-run-target.ts +++ b/lib/packages/fabro-api-client/src/models/git-run-target.ts @@ -15,7 +15,7 @@ /** - * Public github.com repository target. + * Public github.com repository target. The branch names the attached working branch. An optional tag selects a release at worker start, and an optional exact SHA is authoritative when both are present. */ export interface GitRunTarget { 'kind': GitRunTargetKindEnum; @@ -24,11 +24,15 @@ export interface GitRunTarget { */ 'repo': string; /** - * Required branch name, preserved exactly. + * Required attached working branch name, preserved exactly. */ 'branch': string; /** - * Optional exact commit. The server lowercase-normalizes its syntax but does not resolve it or prove branch ancestry. + * Optional bare tag name. Prefixes such as `refs/tags/` and `tags/` are rejected. Without `sha`, the worker resolves this tag when the sandbox starts and fails if it is unavailable. + */ + 'tag'?: string; + /** + * Optional exact commit. The server lowercase-normalizes its syntax but does not resolve it, prove branch ancestry, or prove that it matches an accompanying tag. When present, this exact commit wins. */ 'sha'?: string; }