From 45f1f41615c91104af2dc9980e3c6dde8ef25123 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 4 Oct 2026 11:12:19 +0000 Subject: [PATCH 1/4] Look up an incremental baseline by commit ancestry before branch names Search the 20 nearest commits behind HEAD in batches of 10, nearest first, and take the scan of the closest commit, reading every page of a batch so a nearer commit's scan on a later page is not missed. Fall back to the branch being scanned, then trunk, only when none of those commits was scanned. Co-authored-by: ibrahim --- src/incremental.rs | 223 ++++++++++++------- tests/cloud_commands_e2e/common/mod.rs | 15 +- tests/cloud_commands_e2e/scan_incremental.rs | 199 +++++++++++++++-- 3 files changed, 335 insertions(+), 102 deletions(-) diff --git a/src/incremental.rs b/src/incremental.rs index 1a50137..ee63c97 100644 --- a/src/incremental.rs +++ b/src/incremental.rs @@ -6,12 +6,12 @@ //! commits. This module works the diff out where the scan is run. //! //! Finding what to diff *against* is the same in both directions: ask the -//! server for the newest completed scan of this branch. If this branch has -//! never been scanned -- or the checkout names none, the detached HEAD a CI -//! checkout usually lands on -- take the scan of the nearest commit in this -//! checkout's recent history, whatever branch it was recorded under, and only -//! then the newest scan of trunk. What changed since the baseline can then be -//! measured two ways. +//! server for scans of the commits nearest behind HEAD in this checkout's +//! history, whatever branch they were recorded under, and take the one of the +//! closest commit. Only when none of those was scanned -- or there is no +//! history to walk -- fall back to the newest completed scan of this branch, +//! then of trunk. What changed since the baseline can then be measured two +//! ways. //! //! The first is the file checksums that scan uploaded, fetched and subtracted //! from this run's. It needs no git history, so it is the only one that works @@ -56,8 +56,10 @@ use std::collections::{BTreeSet, HashMap, VecDeque}; const SCAN_LOOKUP_PAGE_SIZE: u16 = 30; /// How many of this checkout's nearest commits a baseline is looked for among. -/// Must stay within the 50 the scan list's `sha` filter takes in one request. -const ANCESTOR_LOOKUP_COMMITS: usize = 10; +const ANCESTOR_LOOKUP_COMMITS: usize = 20; +/// How many of those commits one request asks about, nearest first. Must stay +/// within the 50 the scan list's `sha` filter takes in one request. +const ANCESTOR_LOOKUP_BATCH: usize = 10; /// The most scans per page the scan list returns, so one page normally holds /// every scan of those commits. const ANCESTOR_LOOKUP_PAGE_SIZE: u16 = 50; @@ -215,43 +217,49 @@ fn plan_diff( // Whether the lookup by commit answered, so the refusal below only claims // none of those commits was scanned when the server actually said so. let mut commits_searched = false; - let lookup = match &candidates { - Some(candidates) => { - // `baseline_branches` puts the branch being scanned first. - let (own, trunks) = candidates.split_at(usize::from(branch.is_some())); - let mut lookup = find_baseline(config, project_name, Some(own), checksums_usable); - // Between this branch's own history and trunk's newest scan, - // because the scan of a commit this one descends from differs by - // only what this history has done since. Trunk's newest also - // differs by whatever trunk did after this branch forked, and may - // not be this branch's trunk at all where that is not main or - // master. It also finds the earlier runs of a detached CI pipeline, - // which recorded no branch for a name to match. - if let (BaselineLookup::NotFound, Some(ancestry)) = (&lookup, &ancestry) { - lookup = match find_ancestor_baseline( - config, - project_name, - ancestry, - checksums_usable, - git_diff_refusal(sources).is_none(), - ) { - // A backend that cannot answer a list of commits must not - // cost the trunk lookup every earlier release made. If the - // endpoint itself is down, that lookup fails and says so. - BaselineLookup::LookupFailed => BaselineLookup::NotFound, - answered => { - commits_searched = true; - answered - } - }; + // Ancestry before any branch name, because the scan of a commit HEAD + // descends from differs by only what this history has done since. A + // branch's newest scan may be of a commit this checkout is behind or was + // rebased away from, and trunk's newest also differs by whatever trunk did + // after this branch forked. It also finds the earlier runs of a detached + // CI pipeline, which recorded no branch for a name to match. + let mut lookup = BaselineLookup::NotFound; + if let Some(ancestry) = &ancestry { + lookup = match find_ancestor_baseline( + config, + project_name, + ancestry, + checksums_usable, + git_diff_refusal(sources).is_none(), + ) { + // A backend that cannot answer a list of commits must not cost + // the branch lookups every earlier release made. If the endpoint + // itself is down, those fail too and say so. + BaselineLookup::LookupFailed => BaselineLookup::NotFound, + answered => { + commits_searched = true; + answered } - if lookup == BaselineLookup::NotFound { - lookup = find_baseline(config, project_name, Some(trunks), checksums_usable); + }; + } + if lookup == BaselineLookup::NotFound { + lookup = match &candidates { + Some(candidates) => { + // `baseline_branches` puts the branch being scanned first. Its + // own two walks finish before trunk is asked about, so a clean + // scan of this branch behind a page of dirty ones still beats + // trunk's. + let (own, trunks) = candidates.split_at(usize::from(branch.is_some())); + match find_baseline(config, project_name, Some(own), checksums_usable) { + BaselineLookup::NotFound => { + find_baseline(config, project_name, Some(trunks), checksums_usable) + } + found => found, + } } - lookup - } - None => find_baseline(config, project_name, None, checksums_usable), - }; + None => find_baseline(config, project_name, None, checksums_usable), + }; + } let baseline = match lookup { BaselineLookup::Found(scan) => scan, BaselineLookup::NotFound => { @@ -260,10 +268,10 @@ fn plan_diff( against, so there is nothing to compare this one to", match (&candidates, ancestry.as_ref().filter(|_| commits_searched)) { (Some(candidates), Some(ancestry)) => format!( - "on {}, nor of any of the {} most recent commits in this checkout's \ - history", - join_or(candidates), - ancestry.shas.len() + "of any of the {} most recent commits in this checkout's history, \ + nor on {},", + ancestry.shas.len(), + join_or(candidates) ), (Some(candidates), None) => format!("on {}", join_or(candidates)), (None, _) => "of its whole state".to_string(), @@ -453,6 +461,17 @@ impl BaselineScan { } } + /// Order among scans found by ancestry, lowest best: fewest commits back, + /// then one this run can diff by its checksums over one it cannot. + fn ancestor_rank(&self, checksums_usable: bool) -> (usize, bool) { + let readable_checksums = self.manifest_root.is_some() + && self.manifest_version.as_deref() == Some(MANIFEST_VERSION); + ( + self.commits_back.unwrap_or(usize::MAX), + !(checksums_usable && readable_checksums), + ) + } + /// How to refer to this scan in the one line the run prints. fn describe(&self) -> String { if let (Some(commits_back), Some(sha)) = (self.commits_back, &self.sha) { @@ -488,19 +507,22 @@ enum BaselineLookup { LookupFailed, } -/// The branches a baseline may come from, best first. +/// The branches a baseline may come from, best first, once no scan of a recent +/// commit was found. /// -/// The branch being scanned leads. Its last scan is the nearest ancestor of -/// this one that exists, so the diff against it is the smallest honest one and -/// the findings carried forward are this branch's own. A long-lived branch that -/// has diverged from trunk gets the biggest reduction: against trunk every file -/// it has touched since it forked is "changed", against its own last scan only -/// what moved since that scan is. +/// The branch being scanned leads. Its last scan is usually an ancestor of this +/// one further back than the commit lookup reaches, so the diff against it is +/// the smallest left and the findings carried forward are this branch's own. A +/// long-lived branch that has diverged from trunk gets the biggest reduction: +/// against trunk every file it has touched since it forked is "changed", +/// against its own last scan only what moved since that scan is. /// /// Trunk follows, because a branch on its first scan has no history of its own -/// and trunk is the line it descends from. *Other* branches never qualify: a -/// scan of someone else's feature branch is a baseline whose contents nobody -/// can predict, and the findings copied forward would be that branch's. +/// and trunk is the line it descends from. *Other* branches never qualify by +/// name: the newest scan of someone else's feature branch is a baseline whose +/// contents nobody can predict, and the findings copied forward would be that +/// branch's. The commit lookup may take one, because ancestry proves it covered +/// a tree this history produced. /// /// `origin/HEAD` records what the remote advertised as its default when this /// clone was made. It is absent from single-branch and `actions/checkout` @@ -770,10 +792,10 @@ impl Ancestry { /// all. The last is what every earlier run of a pipeline that checks out /// detached records, and a lookup keyed on names never finds them. /// -/// Nearest wins because it leaves the fewest files to rescan. The commits -/// travel in the request, so a page normally holds every candidate. Stops at -/// the first page holding any: the pages after it are for a backend that -/// ignored the filter and is listing the whole history. +/// Nearest wins because it leaves the fewest files to rescan. The commits are +/// asked about `ANCESTOR_LOOKUP_BATCH` at a time, nearest first, and the walk +/// stops at the first batch with any usable scan: every commit in a later batch +/// is at least as far back, so none of its scans could be closer. fn find_ancestor_baseline( config: &Config, project_name: &str, @@ -782,12 +804,47 @@ fn find_ancestor_baseline( git_diffable: bool, ) -> BaselineLookup { let url = config.get_url(); + for batch in ancestry.shas.chunks(ANCESTOR_LOOKUP_BATCH) { + match search_ancestor_batch( + &url, + project_name, + ancestry, + batch, + checksums_usable, + git_diffable, + ) { + BaselineLookup::NotFound => continue, + answered => return answered, + } + } + BaselineLookup::NotFound +} + +/// The nearest usable scan of any commit in `batch`. +/// +/// The server orders by recency, not by distance from HEAD, so a page full of +/// re-runs of one commit can push a nearer commit's scan onto the next page. +/// Pages are read until none is left, the budget runs out, or the best so far +/// is a scan of the batch's nearest commit, which nothing later can beat. +fn search_ancestor_batch( + url: &str, + project_name: &str, + ancestry: &Ancestry, + batch: &[String], + checksums_usable: bool, + git_diffable: bool, +) -> BaselineLookup { + let unbeatable = batch + .first() + .and_then(|sha| ancestry.commits_back.get(sha)) + .map(|&commits_back| (commits_back, !checksums_usable)); + let mut best: Option = None; for page in 1..=SCAN_LOOKUP_MAX_PAGES { let response = match api::query_baseline_scans_at_commits( - &url, + url, project_name, BLAST_ENGINE, - &ancestry.shas, + batch, // Only a checksum baseline may have been dirty, so without one // the server can drop what could never qualify. !checksums_usable, @@ -796,10 +853,9 @@ fn find_ancestor_baseline( ) { Ok(response) => response, Err(e) => { - crate::log::debug(&format!( - "Baseline lookup by commit failed, trying trunk instead: {e}" - )); - return BaselineLookup::LookupFailed; + crate::log::debug(&format!("Baseline lookup by commit failed: {e}")); + // A scan already read is still a scan of this history. + return best.map_or(BaselineLookup::LookupFailed, BaselineLookup::Found); } }; let scans = response.scans.unwrap_or_default(); @@ -809,7 +865,19 @@ fn find_ancestor_baseline( if let Some(scan) = nearest_ancestor_baseline(&scans, ancestry, checksums_usable, git_diffable) { - return BaselineLookup::Found(scan); + // Strictly better only: earlier pages are newer, and recency + // breaks a tie. + if best.as_ref().is_none_or(|kept| { + scan.ancestor_rank(checksums_usable) < kept.ancestor_rank(checksums_usable) + }) { + best = Some(scan); + } + } + if best + .as_ref() + .is_some_and(|kept| Some(kept.ancestor_rank(checksums_usable)) == unbeatable) + { + break; } if response .total_pages @@ -818,7 +886,7 @@ fn find_ancestor_baseline( break; } } - BaselineLookup::NotFound + best.map_or(BaselineLookup::NotFound, BaselineLookup::Found) } /// The scan on this page nearest behind HEAD that this run can diff from. @@ -843,14 +911,17 @@ fn nearest_ancestor_baseline( .filter_map(|(position, scan)| { let sha = scan.git_sha.as_deref()?; let commits_back = *ancestry.commits_back.get(sha)?; - let without_checksums = !(checksums_usable && has_readable_checksums(scan)); - Some(((commits_back, without_checksums, position), scan)) + let baseline = BaselineScan { + commits_back: Some(commits_back), + ..BaselineScan::from_response(scan) + }; + Some(( + (baseline.ancestor_rank(checksums_usable), position), + baseline, + )) }) .min_by_key(|(rank, _)| *rank) - .map(|((commits_back, _, _), scan)| BaselineScan { - commits_back: Some(commits_back), - ..BaselineScan::from_response(scan) - }) + .map(|(_, baseline)| baseline) } /// Whether `scan` may be diffed against, given what this run can diff with. @@ -1606,7 +1677,7 @@ mod tests { } #[test] - fn the_walk_stops_at_what_one_request_can_ask_about() { + fn the_walk_stops_at_the_commit_lookup_limit() { let dir = tempfile::tempdir().expect("tempdir"); let repo = Repository::init(dir.path()).expect("init"); let mut tip = commit_on(&repo, &[], "0"); diff --git a/tests/cloud_commands_e2e/common/mod.rs b/tests/cloud_commands_e2e/common/mod.rs index 1639164..5c209df 100644 --- a/tests/cloud_commands_e2e/common/mod.rs +++ b/tests/cloud_commands_e2e/common/mod.rs @@ -343,10 +343,11 @@ pub(crate) fn plan_step(plan: &[ExpectedRequest], label: &str) -> usize { /// Every baseline lookup a fixture repo makes when nothing turns up. /// -/// The branch being scanned first, whose last scan is the nearest baseline -/// there could be; then the scans of this checkout's recent commits on any -/// branch; then the two trunk names, since the fixture records no origin/HEAD. -/// Each branch lookup is two walks. The first takes a baseline of either kind, +/// The scans of this checkout's recent commits on any branch first, since the +/// nearest scanned ancestor is the best baseline there could be; the fixture's +/// history fits in one batch. Then the branch being scanned, then the two trunk +/// names, since the fixture records no origin/HEAD. Each branch lookup is two +/// walks. The first takes a baseline of either kind, /// so it cannot let the server drop scans that are not known-clean; the second /// asks for exactly those once no checksums have been found, so a clean scan /// behind a page budget's worth of dirty ones is still reachable. @@ -373,12 +374,12 @@ pub(crate) fn baseline_lookups_finding_nothing(project: &'static str) -> Vec>, scans: Vec) -> ExpectedRequ ) } +/// One page of the lookup by commit, for the cases that read more than one. +fn ancestor_lookup_page( + shas: Vec, + page: u16, + total_pages: u32, + scans: Vec, +) -> ExpectedRequest { + expected_request( + "look up a baseline scan of a recent commit", + move |request| { + assert_authenticated_request(request, Method::GET, "/api/v1/scans")?; + assert_query(request, "project", PROJECT)?; + assert_query(request, "page", &page.to_string())?; + assert_query(request, "page_size", "50")?; + assert_query(request, "sha", &shas.join(",")) + }, + json_response(json!({ + "status": "ok", + "page": page, + "total_pages": total_pages, + "scans": scans, + })), + ) +} + +/// The lookup by commit every git run makes first, answered with nothing, for +/// the cases about what happens once it falls back to branch names. +fn no_scanned_recent_commit() -> ExpectedRequest { + ancestor_lookup(None, vec![]) +} + /// One page of the baseline lookup, for the walk an old backend forces. fn baseline_lookup_page( branch: &'static str, @@ -208,6 +239,7 @@ fn the_baselines_stored_checksums_are_used_in_preference_to_a_git_diff() { let mut plan = vec![ verify_request(), scan_settings_request(PROJECT), + no_scanned_recent_commit(), baseline_lookup(FIXTURE_BRANCH, vec![scan]), checksum_download(manifest), start_upload(), @@ -326,6 +358,7 @@ fn a_dirty_worktree_is_scanned_incrementally_from_the_stored_checksums() { let mut plan = vec![ verify_request(), scan_settings_request(PROJECT), + no_scanned_recent_commit(), baseline_lookup(FIXTURE_BRANCH, vec![scan]), checksum_download(manifest), start_upload(), @@ -403,6 +436,7 @@ fn an_excluded_run_diffs_its_checksums_and_names_what_it_holds_back() { let mut plan = vec![ verify_request(), scan_settings_request(PROJECT), + no_scanned_recent_commit(), baseline_lookup(FIXTURE_BRANCH, vec![scan]), checksum_download(manifest), start_upload(), @@ -474,6 +508,7 @@ fn an_excluded_run_with_no_checksums_to_diff_scans_everything() { let mut plan = vec![ verify_request(), scan_settings_request(PROJECT), + no_scanned_recent_commit(), baseline_lookup(FIXTURE_BRANCH, vec![baseline_scan(&base_sha)]), start_upload(), expected_request( @@ -539,6 +574,7 @@ fn checksums_that_do_not_match_their_digest_fall_back_to_the_git_diff() { let mut plan = vec![ verify_request(), scan_settings_request(PROJECT), + no_scanned_recent_commit(), baseline_lookup(FIXTURE_BRANCH, vec![scan]), checksum_download(manifest), start_upload(), @@ -585,6 +621,7 @@ fn the_upload_carries_the_baseline_commit_and_the_files_that_changed_since_it() let mut plan = vec![ verify_request(), scan_settings_request(PROJECT), + no_scanned_recent_commit(), baseline_lookup(FIXTURE_BRANCH, vec![baseline_scan(&base_sha)]), start_upload(), expected_request( @@ -670,15 +707,15 @@ fn a_project_with_no_baseline_scan_uploads_without_a_diff() { assert_eq!(output.status.code(), Some(0), "{context}"); assert!( stdout.contains(&format!( - "has no completed scan on {FIXTURE_BRANCH}, main or master, nor of any of the 2 \ - most recent commits in this checkout's history that could be diffed against" + "has no completed scan of any of the 2 most recent commits in this checkout's \ + history, nor on {FIXTURE_BRANCH}, main or master, that could be diffed against" )), "{context}" ); } -/// The branch being scanned is asked about first, and its own last scan wins -/// over trunk's. +/// Once no recent commit turns out to have been scanned, the branch being +/// scanned is asked about next, and its own last scan wins over trunk's. /// /// A branch that has been scanned before has a nearer ancestor than trunk does: /// everything it changed before that scan was already analyzed, so diffing @@ -694,15 +731,18 @@ fn the_branch_being_scanned_is_preferred_over_trunk_and_falls_back_to_it() { let mut scan = baseline_scan(&base_sha); scan["branch"] = json!(baseline_branch); - // The branch being scanned is asked about first either way. When the - // baseline is trunk's, that lookup and the one by commit come back - // empty and the next one finds it -- so the plan is the assertion that - // the order is what it is. - let mut plan = vec![verify_request(), scan_settings_request(PROJECT)]; + // The lookup by commit comes first either way, then the branch being + // scanned. When the baseline is trunk's, those come back empty and the + // next one finds it -- so the plan is the assertion that the order is + // what it is. + let mut plan = vec![ + verify_request(), + scan_settings_request(PROJECT), + no_scanned_recent_commit(), + ]; if baseline_branch != FIXTURE_BRANCH { plan.push(baseline_lookup(FIXTURE_BRANCH, vec![])); plan.push(clean_baseline_lookup(FIXTURE_BRANCH, vec![])); - plan.push(ancestor_lookup(None, vec![])); } let expected_base = base_sha.clone(); plan.extend([ @@ -735,12 +775,12 @@ fn the_branch_being_scanned_is_preferred_over_trunk_and_falls_back_to_it() { } } -/// A branch with no scan of its own diffs against the scan of the commit it -/// forked from, whatever branch that scan names -- here a trunk called -/// `develop`, which the name lookup never asks about. A detached HEAD, which -/// names no branch to look up at all, gets the same answer. +/// The scan of a recent commit is asked about before any branch name, and +/// taken whatever branch it names -- here a trunk called `develop`, which the +/// name lookup never asks about. No branch lookup is made at all once it is +/// found, whether or not the checkout names a branch. #[test] -fn a_scan_of_a_recent_commit_is_the_baseline_when_the_branch_has_none() { +fn a_scan_of_a_recent_commit_is_the_baseline_before_any_branch_is_asked_about() { for detached in [false, true] { let project = git_project(); let base_sha = project.sha.clone(); @@ -753,10 +793,6 @@ fn a_scan_of_a_recent_commit_is_the_baseline_when_the_branch_has_none() { baseline_scan_with_checksums(&base_sha, &[("main.py", SOURCE_BODY)]); scan["branch"] = json!("develop"); let mut plan = vec![verify_request(), scan_settings_request(PROJECT)]; - if !detached { - plan.push(baseline_lookup(FIXTURE_BRANCH, vec![])); - plan.push(clean_baseline_lookup(FIXTURE_BRANCH, vec![])); - } plan.extend([ ancestor_lookup(Some(vec![head_sha.clone(), base_sha.clone()]), vec![scan]), checksum_download(manifest), @@ -806,6 +842,126 @@ fn a_scan_of_a_recent_commit_is_the_baseline_when_the_branch_has_none() { } } +/// Commits `count` new files on top of the fixture, one per commit, and +/// returns this checkout's history nearest first. +fn commit_steps(project: &GitProject, count: usize) -> Vec { + for step in 0..count { + let name = format!("step{step}.py"); + std::fs::write(project.path().join(&name), format!("print({step})\n")).expect("write step"); + run_git(project.path(), &["add", &name]); + run_git(project.path(), &["commit", "-m", &name]); + } + String::from_utf8(run_git(project.path(), &["rev-list", "HEAD"]).stdout) + .expect("UTF-8 SHAs") + .lines() + .map(str::to_string) + .collect() +} + +/// Upload asserting the checksum diff named [`BASELINE_SCAN`]. +fn checksum_diff_upload() -> ExpectedRequest { + expected_request( + "upload BLAST archive with the checksum diff", + |request| { + assert_authenticated_request( + request, + Method::PATCH, + "/api/v1/start-scan/transfer-123/", + )?; + assert_multipart_text_field(request, "incremental_base_scan_id", BASELINE_SCAN) + }, + json_response(json!({"scan_id": "blast-scan-123", "project_id": 91})), + ) +} + +/// Commits are asked about ten at a time, nearest first, so a scan of a commit +/// further back than the first ten is still found -- by a second request for +/// the next ten, and before any branch is asked about. +#[test] +fn a_scan_beyond_the_first_ten_commits_is_found_by_the_second_batch() { + let project = git_project(); + let history = commit_steps(&project, 11); + let base_sha = history.last().expect("fixture commit").clone(); + assert_eq!(history.len(), 12); + + let (scan, manifest) = baseline_scan_with_checksums(&base_sha, &[("main.py", SOURCE_BODY)]); + let mut plan = vec![ + verify_request(), + scan_settings_request(PROJECT), + ancestor_lookup(Some(history[..10].to_vec()), vec![]), + ancestor_lookup(Some(history[10..].to_vec()), vec![scan]), + checksum_download(manifest), + start_upload(), + checksum_diff_upload(), + ]; + plan.extend(scan_tail()); + + let api = ApiStub::start(plan); + let (mut command, _home) = cloud_command(&api, project.path()); + command.args(["scan", "blast", "--project-name", PROJECT]); + + let output = run_with_timeout(command, &api); + let transcript = api.assert_finished(); + let context = output_context(&output, &transcript); + let stdout = String::from_utf8_lossy(&output.stdout); + + assert_eq!(output.status.code(), Some(0), "{context}"); + assert!( + stdout.contains(&format!( + "Incremental scan: 11 files changed since the scan of ancestor commit {} (11 \ + commits back).", + &base_sha[..7] + )), + "{context}" + ); +} + +/// The server lists scans newest first, not nearest first, so the scan of the +/// closer commit can come back on a later page than one further back. The +/// batch is read to the end, and the closer commit wins. +#[test] +fn the_closest_commit_wins_even_when_its_scan_is_on_a_later_page() { + let project = git_project(); + let history = commit_steps(&project, 2); + let (near_sha, far_sha) = (history[1].clone(), history[2].clone()); + + let (mut far, _) = baseline_scan_with_checksums(&far_sha, &[("main.py", SOURCE_BODY)]); + far["id"] = json!("farther-scan"); + let (near, manifest) = baseline_scan_with_checksums( + &near_sha, + &[("main.py", SOURCE_BODY), ("step0.py", "print(0)\n")], + ); + let mut plan = vec![ + verify_request(), + scan_settings_request(PROJECT), + ancestor_lookup_page(history.clone(), 1, 2, vec![far]), + ancestor_lookup_page(history.clone(), 2, 2, vec![near]), + checksum_download(manifest), + start_upload(), + checksum_diff_upload(), + ]; + plan.extend(scan_tail()); + + let api = ApiStub::start(plan); + let (mut command, _home) = cloud_command(&api, project.path()); + command.args(["scan", "blast", "--project-name", PROJECT]); + + let output = run_with_timeout(command, &api); + let transcript = api.assert_finished(); + let context = output_context(&output, &transcript); + let stdout = String::from_utf8_lossy(&output.stdout); + + assert_eq!(output.status.code(), Some(0), "{context}"); + assert!( + stdout.contains(&format!( + "Incremental scan: 1 file changed since the scan of ancestor commit {} (1 commit \ + back).", + &near_sha[..7] + )), + "{context}" + ); +} + /// A detached HEAD has no branch lookup to make, and when none of its recent /// commits was scanned it still reaches trunk -- through a git diff here, which /// needs the two commits and no branch. @@ -896,6 +1052,7 @@ fn a_baseline_on_a_later_page_is_still_found() { let mut plan = vec![ verify_request(), scan_settings_request(PROJECT), + no_scanned_recent_commit(), baseline_lookup_page(FIXTURE_BRANCH, 1, 2, vec![unusable]), baseline_lookup_page(FIXTURE_BRANCH, 2, 2, vec![baseline_scan(&base_sha)]), start_upload(), @@ -936,6 +1093,7 @@ fn a_failed_lookup_is_not_reported_as_a_missing_baseline() { let mut plan = vec![ verify_request(), scan_settings_request(PROJECT), + no_scanned_recent_commit(), expected_request( "fail the baseline lookup", |request| assert_baseline_lookup_request(request, PROJECT, Some(FIXTURE_BRANCH), false), @@ -1097,6 +1255,7 @@ fn a_clean_baseline_is_found_even_when_dirty_scans_come_back_first() { let mut plan = vec![ verify_request(), scan_settings_request(PROJECT), + no_scanned_recent_commit(), baseline_lookup(FIXTURE_BRANCH, vec![dirty]), clean_baseline_lookup(FIXTURE_BRANCH, vec![baseline_scan(&base_sha)]), start_upload(), @@ -1194,6 +1353,7 @@ fn ignore_dirty_worktree_diffs_the_working_tree_instead_of_refusing() { let mut plan = vec![ verify_request(), scan_settings_request(PROJECT), + no_scanned_recent_commit(), baseline_lookup(FIXTURE_BRANCH, vec![baseline_scan(&base_sha)]), start_upload(), expected_request( @@ -1260,6 +1420,7 @@ fn a_dirty_worktree_with_no_stored_checksums_scans_everything() { let mut plan = vec![ verify_request(), scan_settings_request(PROJECT), + no_scanned_recent_commit(), baseline_lookup(FIXTURE_BRANCH, vec![baseline_scan(&base_sha)]), start_upload(), expected_request( From 10918406a0b461c81b4b00c01dc89502ffd4f26a Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 4 Oct 2026 11:19:16 +0000 Subject: [PATCH 2/4] Pin how a shallow clone's short history is looked up A shallow clone holds its boundary commit's parent ids but not the parents, so the commit lookup asks only about the commits the clone has, then falls back to the branch lookups. Covered with a real git clone --depth, and the no-baseline message no longer says 'the 1 most recent commits'. Co-authored-by: ibrahim --- src/incremental.rs | 50 +++++++++++- tests/cloud_commands_e2e/scan_incremental.rs | 83 ++++++++++++++++++++ 2 files changed, 129 insertions(+), 4 deletions(-) diff --git a/src/incremental.rs b/src/incremental.rs index ee63c97..9b19c7c 100644 --- a/src/incremental.rs +++ b/src/incremental.rs @@ -268,15 +268,22 @@ fn plan_diff( against, so there is nothing to compare this one to", match (&candidates, ancestry.as_ref().filter(|_| commits_searched)) { (Some(candidates), Some(ancestry)) => format!( - "of any of the {} most recent commits in this checkout's history, \ - nor on {},", - ancestry.shas.len(), + "{}, nor on {},", + match ancestry.shas.len() { + // Only when no parent is reachable: a shallow + // clone's depth, or a repository's first commit. + 1 => "of the only commit in this checkout's history".to_string(), + n => format!( + "of any of the {n} most recent commits in this checkout's \ + history" + ), + }, join_or(candidates) ), (Some(candidates), None) => format!("on {}", join_or(candidates)), (None, _) => "of its whole state".to_string(), } - )) + )); } BaselineLookup::LookupFailed => { return Err(format!( @@ -1648,6 +1655,41 @@ mod tests { assert_eq!(picked.commits_back, Some(2)); } + /// A shallow clone holds its boundary commit's parent ids but not the + /// parents, so the walk ends at the depth instead of reporting commits + /// nothing could diff against. + #[test] + fn a_shallow_clone_offers_only_the_commits_it_holds() { + let dir = tempfile::tempdir().expect("tempdir"); + let origin = dir.path().join("origin"); + let repo = Repository::init(&origin).expect("init"); + let mut tip = commit_on(&repo, &[], "0"); + for i in 1..5 { + tip = commit_on(&repo, &[tip], &i.to_string()); + } + repo.reference("refs/heads/main", tip, true, "test") + .expect("branch"); + repo.set_head("refs/heads/main").expect("HEAD"); + + for depth in [1, 2] { + let clone = dir.path().join(format!("depth-{depth}")); + let status = std::process::Command::new("git") + .args(["clone", "--quiet", "--depth", &depth.to_string()]) + .arg(format!("file://{}", origin.display())) + .arg(&clone) + .status() + .expect("run git clone"); + assert!(status.success(), "git clone --depth {depth} failed"); + let shallow = Repository::open(&clone).expect("open clone"); + assert!(shallow.is_shallow()); + + let ancestry = Ancestry::walk(&shallow, &tip.to_string()).expect("head in clone"); + + assert_eq!(ancestry.shas.len(), depth); + assert_eq!(ancestry.shas.first(), Some(&tip.to_string())); + } + } + #[test] fn a_head_this_clone_does_not_hold_offers_no_ancestry() { let l = lineage(); diff --git a/tests/cloud_commands_e2e/scan_incremental.rs b/tests/cloud_commands_e2e/scan_incremental.rs index bb79544..a9aabee 100644 --- a/tests/cloud_commands_e2e/scan_incremental.rs +++ b/tests/cloud_commands_e2e/scan_incremental.rs @@ -962,6 +962,89 @@ fn the_closest_commit_wins_even_when_its_scan_is_on_a_later_page() { ); } +/// A depth-1 clone, the `actions/checkout` default, holds only HEAD. The lookup +/// by commit asks about that one commit rather than history the clone does not +/// have, then falls back to the branch. The branch's scan is of a commit this +/// clone cannot reach, so only its stored checksums can diff against it; without +/// them the run scans everything and says why. +#[test] +fn a_shallow_clone_asks_about_the_commits_it_holds_then_falls_back_to_the_branch() { + for stored_checksums in [true, false] { + let project = git_project(); + let base_sha = project.sha.clone(); + let head_sha = second_commit(&project); + let clone = tempfile::TempDir::new().expect("create clone"); + run_git( + project.path(), + &[ + "clone", + "--quiet", + "--depth", + "1", + "--branch", + FIXTURE_BRANCH, + &format!("file://{}", project.path().display()), + &clone.path().display().to_string(), + ], + ); + + let mut plan = vec![ + verify_request(), + scan_settings_request(PROJECT), + ancestor_lookup(Some(vec![head_sha.clone()]), vec![]), + ]; + if stored_checksums { + let (scan, manifest) = + baseline_scan_with_checksums(&base_sha, &[("main.py", SOURCE_BODY)]); + plan.extend([ + baseline_lookup(FIXTURE_BRANCH, vec![scan]), + checksum_download(manifest), + start_upload(), + checksum_diff_upload(), + ]); + } else { + plan.extend([ + baseline_lookup(FIXTURE_BRANCH, vec![baseline_scan(&base_sha)]), + start_upload(), + expected_request( + "upload BLAST archive with no diff", + |request| { + assert_authenticated_request( + request, + Method::PATCH, + "/api/v1/start-scan/transfer-123/", + )?; + assert_no_multipart_field(request, "incremental_base_sha")?; + assert_no_multipart_field(request, "incremental_changed_files") + }, + json_response(json!({"scan_id": "blast-scan-123", "project_id": 91})), + ), + ]); + } + plan.extend(scan_tail()); + + let api = ApiStub::start(plan); + let (mut command, _home) = cloud_command(&api, clone.path()); + command.args(["scan", "blast", "--project-name", PROJECT]); + + let output = run_with_timeout(command, &api); + let transcript = api.assert_finished(); + let context = output_context(&output, &transcript); + let stdout = String::from_utf8_lossy(&output.stdout); + + assert_eq!(output.status.code(), Some(0), "{context}"); + let expected = if stored_checksums { + format!( + "Incremental scan: 2 files changed since the last scan of {FIXTURE_BRANCH} ({})", + &base_sha[..7] + ) + } else { + "A shallow clone cannot diff against it".to_string() + }; + assert!(stdout.contains(&expected), "{context}"); + } +} + /// A detached HEAD has no branch lookup to make, and when none of its recent /// commits was scanned it still reaches trunk -- through a git diff here, which /// needs the two commits and no branch. From 0df6b597263d5b1a510cba2112ff5608f0c90ee7 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sun, 4 Oct 2026 13:03:03 +0000 Subject: [PATCH 3/4] Prefer the first parent when a merge's parents tie for nearest baseline Both parents of a merge sit one commit back, so recency decided between them and the merged-in side could win. Its diff is everything the first line did since the fork, which can outgrow an incremental scan; the diff from the first parent is the merge itself. Rank first-parent ancestors ahead at equal distance, and compute the page walk's early-stop rank over the whole batch. Co-authored-by: ibrahim --- src/incremental.rs | 88 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 81 insertions(+), 7 deletions(-) diff --git a/src/incremental.rs b/src/incremental.rs index 9b19c7c..bee1afd 100644 --- a/src/incremental.rs +++ b/src/incremental.rs @@ -50,7 +50,7 @@ use crate::manifest::{Manifest, MANIFEST_VERSION}; use crate::scanners::blast::{classify_scan_status, ScanState}; use crate::utils::api::{self, ScanResponse}; use git2::Repository; -use std::collections::{BTreeSet, HashMap, VecDeque}; +use std::collections::{BTreeSet, HashMap, HashSet, VecDeque}; /// How many of the project's scans to read at a time, newest first. const SCAN_LOOKUP_PAGE_SIZE: u16 = 30; @@ -451,8 +451,15 @@ struct BaselineScan { /// Commits between its commit and this run's, when it was picked for /// being in this run's history rather than for the branch it names. commits_back: Option, + /// Whether its commit is on HEAD's first-parent line, when it was picked + /// for being in this run's history. + on_first_parent: bool, } +/// `BaselineScan::ancestor_rank`: commits back, off the first-parent line, +/// diffable only without checksums. Lowest is best. +type AncestorRank = (usize, bool, bool); + impl BaselineScan { fn from_response(scan: &ScanResponse) -> Self { BaselineScan { @@ -465,16 +472,24 @@ impl BaselineScan { .filter(|root| !root.is_empty()), manifest_version: scan.file_manifest_version.clone(), commits_back: None, + on_first_parent: false, } } /// Order among scans found by ancestry, lowest best: fewest commits back, - /// then one this run can diff by its checksums over one it cannot. - fn ancestor_rank(&self, checksums_usable: bool) -> (usize, bool) { + /// then one on HEAD's first-parent line, then one this run can diff by its + /// checksums over one it cannot. + /// + /// The first-parent line breaks the tie a merge makes, whose parents are + /// both one commit back. The diff from the first parent is the merge + /// itself; the diff from the other is everything the first line did since + /// the fork, which can outgrow an incremental scan. + fn ancestor_rank(&self, checksums_usable: bool) -> AncestorRank { let readable_checksums = self.manifest_root.is_some() && self.manifest_version.as_deref() == Some(MANIFEST_VERSION); ( self.commits_back.unwrap_or(usize::MAX), + !self.on_first_parent, !(checksums_usable && readable_checksums), ) } @@ -753,6 +768,8 @@ struct Ancestry { /// Nearest first, HEAD leading. shas: Vec, commits_back: HashMap, + /// Those of `shas` reached from HEAD through first parents alone. + first_parent: HashSet, } impl Ancestry { @@ -765,7 +782,18 @@ impl Ancestry { let mut ancestry = Ancestry { shas: vec![head.id().to_string()], commits_back: HashMap::from([(head.id().to_string(), 0)]), + first_parent: HashSet::from([head.id().to_string()]), }; + // A missing parent ends the line, which is where a shallow clone's + // history ends too. + let mut tip = head.clone(); + while ancestry.first_parent.len() < ANCESTOR_LOOKUP_COMMITS { + let Ok(parent) = tip.parent(0) else { + break; + }; + ancestry.first_parent.insert(parent.id().to_string()); + tip = parent; + } let mut queue = VecDeque::from([(head, 0)]); while let Some((commit, depth)) = queue.pop_front() { for parent_id in commit.parent_ids() { @@ -841,10 +869,18 @@ fn search_ancestor_batch( checksums_usable: bool, git_diffable: bool, ) -> BaselineLookup { - let unbeatable = batch - .first() - .and_then(|sha| ancestry.commits_back.get(sha)) - .map(|&commits_back| (commits_back, !checksums_usable)); + // Over the whole batch, not its first commit: that one can sit off the + // first-parent line at the same distance as one on it. + let unbeatable: Option = batch + .iter() + .filter_map(|sha| { + let commits_back = *ancestry.commits_back.get(sha)?; + Some((commits_back, !ancestry.first_parent.contains(sha))) + }) + .min() + .map(|(commits_back, off_first_parent)| { + (commits_back, off_first_parent, !checksums_usable) + }); let mut best: Option = None; for page in 1..=SCAN_LOOKUP_MAX_PAGES { let response = match api::query_baseline_scans_at_commits( @@ -920,6 +956,7 @@ fn nearest_ancestor_baseline( let commits_back = *ancestry.commits_back.get(sha)?; let baseline = BaselineScan { commits_back: Some(commits_back), + on_first_parent: ancestry.first_parent.contains(sha), ..BaselineScan::from_response(scan) }; Some(( @@ -1716,6 +1753,43 @@ mod tests { assert_eq!(back(&l.root), Some(2)); assert_eq!(back(&l.head), None); assert_eq!(ancestry.shas.first(), Some(&merge.to_string())); + + let on_first_parent = |sha: &str| ancestry.first_parent.contains(sha); + assert!(on_first_parent(&merge.to_string())); + assert!(on_first_parent(&l.near)); + assert!(on_first_parent(&l.root)); + assert!(!on_first_parent(&side_tip.to_string())); + assert!(!on_first_parent(&l.side)); + } + + /// A merge's parents are both one commit back. The diff from the first is + /// the merge; the diff from the other is everything the first line did + /// since the fork, so the first wins even when the other was scanned later. + #[test] + fn between_the_parents_of_a_merge_the_first_parent_wins() { + let l = lineage(); + let side_tip = commit_on(&l.repo, &[git2::Oid::from_str(&l.side).unwrap()], "tip.txt"); + let merge = commit_on( + &l.repo, + &[git2::Oid::from_str(&l.near).unwrap(), side_tip], + "merge.txt", + ); + let ancestry = Ancestry::walk(&l.repo, &merge.to_string()).expect("head in clone"); + // Newest first, so the merged-in side would win on recency. + let other_side = with_checksums(scan("feature", &side_tip.to_string())); + let scans = vec![other_side, scan("main", &l.near)]; + + for checksums_usable in [false, true] { + let picked = nearest_ancestor_baseline(&scans, &ancestry, checksums_usable, true) + .expect("a baseline"); + assert_eq!(picked.sha.as_deref(), Some(l.near.as_str())); + assert_eq!(picked.commits_back, Some(1)); + } + + // Still the baseline when the first parent was never scanned. + let picked = + nearest_ancestor_baseline(&scans[..1], &ancestry, true, true).expect("a baseline"); + assert_eq!(picked.sha, Some(side_tip.to_string())); } #[test] From a2a757112f76588ffa1867a158760d0e1b157cda Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 5 Oct 2026 08:03:27 +0000 Subject: [PATCH 4/4] Bump version to 1.16.2 Co-authored-by: ibrahim --- Cargo.lock | 2 +- Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6fca48f..6ebf8a3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -369,7 +369,7 @@ checksum = "773648b94d0e5d620f64f280777445740e61fe701025087ec8b57f45c791888b" [[package]] name = "corgea" -version = "1.16.1" +version = "1.16.2" dependencies = [ "chrono", "clap", diff --git a/Cargo.toml b/Cargo.toml index 55a82d3..a43f433 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "corgea" -version = "1.16.1" +version = "1.16.2" edition = "2021" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html