Skip to content

Commit 17b72bb

Browse files
authored
fix: materialize changed-since base without runner origin access (#8139)
1 parent df637ff commit 17b72bb

5 files changed

Lines changed: 68 additions & 34 deletions

File tree

src/core/runner/offload_changed_since.rs

Lines changed: 5 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ use std::process::Command;
33

44
use crate::core::error::{Error, Result};
55

6-
use super::origin_refs::{advertised_origin_refs_for_commit, best_advertised_ref};
76
use super::RunnerWorkspaceSyncMode;
87

98
#[derive(Debug, Clone, PartialEq, Eq)]
@@ -64,15 +63,14 @@ pub fn prepare_git_lab_offload_changed_since(
6463

6564
let resolved_base = resolve_changed_since_base(source_path, &git_ref)?;
6665
ensure_local_merge_base(source_path, &git_ref)?;
67-
let git_fetch_refs = advertised_origin_ref_for_commit(source_path, &resolved_base)?
68-
.into_iter()
69-
.collect();
70-
7166
Ok(LabOffloadChangedSincePreflight {
7267
args: rewrite_changed_since_ref(args, &resolved_base),
7368
requested_ref: Some(git_ref),
7469
resolved_base: Some(resolved_base),
75-
git_fetch_refs,
70+
// The final Git materialization verifies this exact commit. Do not
71+
// probe origin here: controller bundles carry it when Lab cannot read
72+
// a private remote.
73+
git_fetch_refs: Vec::new(),
7674
})
7775
}
7876

@@ -152,18 +150,6 @@ fn resolve_changed_since_base(path: &Path, git_ref: &str) -> Result<String> {
152150
)
153151
}
154152

155-
fn advertised_origin_ref_for_commit(path: &Path, commit: &str) -> Result<Option<String>> {
156-
let refs = advertised_origin_refs_for_commit(
157-
path,
158-
commit,
159-
"changed_since",
160-
"Lab offload could not inspect origin refs for changed-since base materialization",
161-
commit.to_string(),
162-
vec!["Run with --placement local to execute the changed-since command locally while investigating remote ref availability.".to_string()],
163-
)?;
164-
Ok(best_advertised_ref(refs))
165-
}
166-
167153
fn ensure_local_merge_base(path: &Path, git_ref: &str) -> Result<()> {
168154
let output = Command::new("git")
169155
.args(["merge-base", git_ref, "HEAD"])
@@ -360,7 +346,7 @@ mod tests {
360346

361347
assert_eq!(preflight.resolved_base.as_deref(), Some(base_sha.as_str()));
362348
assert_eq!(preflight.requested_ref.as_deref(), Some("base"));
363-
assert_eq!(preflight.git_fetch_refs, vec!["refs/heads/base"]);
349+
assert!(preflight.git_fetch_refs.is_empty());
364350
assert_eq!(
365351
preflight.args,
366352
vec![

src/core/runner/workspace/git.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ pub(super) fn materialize_git_from_controller_bundle(
186186
head,
187187
branch,
188188
remote_url,
189+
changed_since_base,
189190
&sha256,
190191
allow_dirty_lab_workspace,
191192
);
@@ -450,6 +451,7 @@ pub(crate) fn git_bundle_install_command(
450451
head: &str,
451452
branch: Option<&str>,
452453
remote_url: &str,
454+
changed_since_base: Option<&str>,
453455
expected_sha256: &str,
454456
allow_dirty_lab_workspace: bool,
455457
) -> String {
@@ -483,6 +485,7 @@ pub(crate) fn git_bundle_install_command(
483485
.op(WorkspaceMaterializationOperation::VerifyGitBaseline {
484486
remote_url: remote_url.to_string(),
485487
head: head.to_string(),
488+
changed_since_base: changed_since_base.map(str::to_string),
486489
})
487490
.restore_owner()
488491
.command()
@@ -511,6 +514,7 @@ pub(super) fn materialize_git_command(
511514
.op(WorkspaceMaterializationOperation::VerifyGitBaseline {
512515
remote_url: remote_url.to_string(),
513516
head: head.to_string(),
517+
changed_since_base: changed_since_base.map(str::to_string),
514518
})
515519
.restore_owner()
516520
.command()

src/core/runner/workspace/materializer.rs

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ pub(super) enum WorkspaceMaterializationOperation {
2727
VerifyGitBaseline {
2828
remote_url: String,
2929
head: String,
30+
changed_since_base: Option<String>,
3031
},
3132
SyncGitCheckout {
3233
remote_url: String,
@@ -157,9 +158,16 @@ impl WorkspaceMaterializationOperation {
157158
Self::GuardCleanGitWorkspace { allow_dirty } => {
158159
dirty_git_workspace_guard("$dest", *allow_dirty)
159160
}
160-
Self::VerifyGitBaseline { remote_url, head } => {
161-
verify_git_baseline_command("$dest", remote_url, head)
162-
}
161+
Self::VerifyGitBaseline {
162+
remote_url,
163+
head,
164+
changed_since_base,
165+
} => verify_git_baseline_command(
166+
"$dest",
167+
remote_url,
168+
head,
169+
changed_since_base.as_deref(),
170+
),
163171
Self::SyncGitCheckout {
164172
remote_url,
165173
head,
@@ -205,16 +213,28 @@ impl WorkspaceMaterializationOperation {
205213
}
206214
}
207215

208-
fn verify_git_baseline_command(dest: &str, remote_url: &str, head: &str) -> String {
216+
fn verify_git_baseline_command(
217+
dest: &str,
218+
remote_url: &str,
219+
head: &str,
220+
changed_since_base: Option<&str>,
221+
) -> String {
209222
let status = format!(
210223
"git -C {dest} status --porcelain=v1 2>/dev/null | while IFS= read -r line; do path=${{line#???}}; if [ \"$path\" = .homeboy ] || [ \"${{path#.homeboy/}}\" != \"$path\" ]; then :; else printf '%s\\n' \"$line\"; fi; done || true",
211224
dest = dest,
212225
);
226+
let changed_since = changed_since_base.map_or_else(String::new, |base| {
227+
format!(
228+
" && git -C {dest} rev-parse --verify -q {base}^{{commit}} >/dev/null && test \"$(git -C {dest} merge-base {base} HEAD)\" = {base}",
229+
base = shell::quote_arg(base),
230+
)
231+
});
213232
format!(
214-
"test -d {dest}/.git && test \"$(git -C {dest} rev-parse HEAD)\" = {head} && test \"$(git -C {dest} config --get remote.origin.url)\" = {remote_url} && test -z \"$({status})\"",
233+
"test -d {dest}/.git && test \"$(git -C {dest} rev-parse HEAD)\" = {head} && test \"$(git -C {dest} config --get remote.origin.url)\" = {remote_url}{changed_since} && test -z \"$({status})\"",
215234
dest = dest,
216235
head = shell::quote_arg(head),
217236
remote_url = shell::quote_arg(remote_url),
237+
changed_since = changed_since,
218238
status = status,
219239
)
220240
}

src/core/runner/workspace/tests/git.rs

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use crate::core::runner::workspace::types::{RunnerWorkspaceSyncMode, RunnerWorks
1212
use crate::core::runner::workspace::util::git_output;
1313

1414
#[test]
15-
fn controller_routed_git_sync_materializes_private_unavailable_remote_with_changed_since_base() {
15+
fn changed_since_retry_route_materializes_private_unavailable_origin_from_bundle() {
1616
crate::test_support::with_isolated_home(|_| {
1717
let origin = tempfile::tempdir().expect("origin tempdir");
1818
let author = tempfile::tempdir().expect("author tempdir");
@@ -96,6 +96,28 @@ fn controller_routed_git_sync_materializes_private_unavailable_remote_with_chang
9696
.exists(),
9797
"the fixture must retain a commit graph after its objects are removed"
9898
);
99+
crate::core::runner::create(
100+
&format!(
101+
r#"{{"id":"lab-local-git-bundle","kind":"local","workspace_root":"{}"}}"#,
102+
runner_root.path().display()
103+
),
104+
false,
105+
)
106+
.expect("create runner");
107+
108+
let args = vec![
109+
"homeboy".to_string(),
110+
"agent-task".to_string(),
111+
"retry".to_string(),
112+
"--changed-since".to_string(),
113+
base.clone(),
114+
];
115+
let changed_since =
116+
crate::core::runner::prepare_git_lab_offload_changed_since(&args, source.path())
117+
.expect("preflight must resolve the local base without probing private origin");
118+
assert_eq!(changed_since.resolved_base.as_deref(), Some(base.as_str()));
119+
assert!(changed_since.git_fetch_refs.is_empty());
120+
99121
remove_local_packs(source.path());
100122
assert!(
101123
git_without_lazy_fetch(
@@ -113,14 +135,6 @@ fn controller_routed_git_sync_materializes_private_unavailable_remote_with_chang
113135
.is_err(),
114136
"the fixture must leave unrelated promisor objects absent locally"
115137
);
116-
crate::core::runner::create(
117-
&format!(
118-
r#"{{"id":"lab-local-git-bundle","kind":"local","workspace_root":"{}"}}"#,
119-
runner_root.path().display()
120-
),
121-
false,
122-
)
123-
.expect("create runner");
124138

125139
let sync_result = sync_workspace(
126140
"lab-local-git-bundle",
@@ -130,7 +144,7 @@ fn controller_routed_git_sync_materializes_private_unavailable_remote_with_chang
130144
// The runner first attempts its normal clone. The inaccessible
131145
// origin then exercises the controller bundle fallback.
132146
controller_routed_git: false,
133-
changed_since_base: Some(base.clone()),
147+
changed_since_base: changed_since.resolved_base,
134148
git_fetch_refs: Vec::new(),
135149
snapshot_includes: Vec::new(),
136150
allow_dirty_lab_workspace: false,
@@ -189,6 +203,10 @@ fn controller_routed_git_sync_materializes_private_unavailable_remote_with_chang
189203
git_output(remote, &["merge-base", &base, "HEAD"]).unwrap(),
190204
base
191205
);
206+
assert_eq!(git_output(remote, &["rev-parse", "HEAD"]).unwrap(), head);
207+
assert!(git_output(remote, &["status", "--porcelain=v1"])
208+
.expect("read final checkout status")
209+
.is_empty());
192210
assert!(
193211
!git_output(
194212
source.path(),
@@ -599,13 +617,16 @@ fn git_bundle_materialization_disables_lazy_fetches() {
599617
"abc123",
600618
None,
601619
"https://github.example.invalid/example-org/private-source.git",
620+
Some("def456"),
602621
"0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
603622
false,
604623
);
605624

606625
assert!(command.contains("export GIT_NO_LAZY_FETCH=1"));
607626
assert!(command.contains("shasum -a 256"));
608627
assert!(command.contains("trap 'rm -rf"));
628+
assert!(command.contains("rev-parse --verify -q def456^{commit}"));
629+
assert!(command.contains("merge-base def456 HEAD"));
609630
}
610631

611632
#[test]
@@ -616,6 +637,7 @@ fn git_bundle_materialization_rejects_digest_mismatch_before_clone() {
616637
"abc123",
617638
None,
618639
"https://github.example.invalid/example-org/private-source.git",
640+
None,
619641
"0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef",
620642
false,
621643
);

src/core/runner/workspace/tests/materializer.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ fn workspace_materializer_builds_git_bundle_checkout_command() {
5353
.op(WorkspaceMaterializationOperation::VerifyGitBaseline {
5454
remote_url: "https://github.com/Extra-Chill/homeboy.git".to_string(),
5555
head: "abc123".to_string(),
56+
changed_since_base: None,
5657
})
5758
.restore_owner()
5859
.command();
@@ -87,6 +88,7 @@ fn workspace_materializer_builds_direct_git_checkout_command() {
8788
.op(WorkspaceMaterializationOperation::VerifyGitBaseline {
8889
remote_url: "https://github.com/Extra-Chill/homeboy.git".to_string(),
8990
head: "abc123".to_string(),
91+
changed_since_base: Some("origin/main".to_string()),
9092
})
9193
.restore_owner()
9294
.command();

0 commit comments

Comments
 (0)