From 3ca8dc1db1efab00b07b254ae7dc4fbe28f21af5 Mon Sep 17 00:00:00 2001 From: Svilen Stefanov Date: Fri, 9 Oct 2026 23:06:11 +0200 Subject: [PATCH] feat: catch up the review base from the nearest saved ancestor With no saved base for the merge base and no usable baseline committed there, a review analyzed the merge base from scratch, even when a saved analysis of a commit a few steps below it was sitting in the artifact store. find-ancestor-base.sh walks the merge base's first-parent history up to 100 commits, deepening the shallow checkout, then pages through the artifact listing newest first, keeping base analyses for this configuration that a run on the repository's own code produced (the same provenance rule as fetch-state.sh). It stops at the first page holding a walked commit (50 pages at most; one or two calls in practice, and only on a run that would otherwise analyze from scratch). The nearest hit seeds an incremental catch-up to the merge base, which is then published under the merge base's own name. Order: exact artifact, committed at the merge base, nearest ancestor, full. Seeding keeps the merge base's own .codeboardingignore and health configuration. Reported as base_analysis_method=incremental, the reason naming the ancestor and the commits caught up. A saved analysis of the merge base under another configuration makes a full run's reason "incompatible"; anything else is "no usable analysis was available". There is no separate "too far behind" reason: proving it took compare API calls that only changed the wording. Sync uses the same lookup when the branch has no usable committed baseline, so the first sync after the setup pull request merges catches up from the base that pull request's review saved. FORCE_FULL is now lowercased with tr, which also runs on the bash 3.2 macOS ships. Co-Authored-By: Claude Opus 5.5 --- action.yml | 9 + docs/COMMIT_STRATEGY.md | 20 +- scripts/action/analyze.sh | 82 +++++++- scripts/action/find-ancestor-base.sh | 81 ++++++++ tests/test_action_state.py | 291 +++++++++++++++++++++++++++ 5 files changed, 470 insertions(+), 13 deletions(-) create mode 100755 scripts/action/find-ancestor-base.sh diff --git a/action.yml b/action.yml index 7ca8012..3dbd9a6 100644 --- a/action.yml +++ b/action.yml @@ -451,6 +451,13 @@ runs: CHECKOUT_DIR: ${{ github.workspace }}/.codeboarding-target STAGE_DIR: ${{ runner.temp }}/cb-state/${{ github.action }}/out FORCE_FULL: ${{ inputs.force_full }} + CFG_HASH: ${{ steps.state.outputs.cfg_hash }} + # Lets a branch without a usable committed baseline catch up from a saved analysis. + ANCESTOR_LOOKUP: ${{ github.server_url == 'https://github.com' && steps.state.outputs.cfg_hash != '' }} + GIT_TOKEN: ${{ inputs.github_token }} + REPOSITORY: ${{ github.repository }} + GH_HOST: ${{ github.server_url }} + GITHUB_SERVER_URL: ${{ github.server_url }} DEPTH_CAP: ${{ inputs.depth_cap }} MODEL: ${{ inputs.model }} AGENT_MODEL_INPUT: ${{ inputs.agent_model }} @@ -539,6 +546,8 @@ runs: BASE_FETCH_SECONDS: ${{ steps.fetch_base.outputs.seconds }} WARMSTART_DIR: ${{ runner.temp }}/cb-state/${{ github.action }}/warmstart RENEW_BASE: ${{ steps.fetch_base.outputs.renew }} + # Lets a branch without a usable committed baseline catch up from a saved analysis. + ANCESTOR_LOOKUP: ${{ github.server_url == 'https://github.com' && steps.state.outputs.cfg_hash != '' }} # For rewriting the progress comment while a base is built from scratch. PROGRESS_HEADER: ${{ steps.guard.outputs.comment_id }} BASE_REF: ${{ steps.guard.outputs.base_ref }} diff --git a/docs/COMMIT_STRATEGY.md b/docs/COMMIT_STRATEGY.md index 9f9fdcf..52407b2 100644 --- a/docs/COMMIT_STRATEGY.md +++ b/docs/COMMIT_STRATEGY.md @@ -72,7 +72,7 @@ never fires. | `base_sha` | string | the base branch tip when the event fired — *not* what was compared against | | `kind` | string | always `review`, so a reader can tell this artifact from a base or warm-start bundle | | `analysed_files_changed` | string | analysed files whose content hash differs between base and head; `unknown` when the analyses cannot say | -| `base_analysis_method` | string | how this run obtained the analysis of the merge base: `reused` (the merge base already had one: its saved artifact, or a committed baseline with nothing to catch up), `incremental` (an earlier commit's analysis updated to the merge base) or `full` (analyzed from scratch in this run). Whichever, the base graph is the merge base's own analysis | +| `base_analysis_method` | string | how this run obtained the analysis of the merge base: `reused` (the merge base already had one: its saved artifact, or a committed baseline with nothing to catch up), `incremental` (an earlier commit's analysis, committed or saved, updated to the merge base) or `full` (analyzed from scratch in this run). Whichever, the base graph is the merge base's own analysis | | `base_analysis_reason` | string | the method in words, e.g. `updated the analysis of 9f8e7d6 to a1b2c3d, 4 commits caught up`; where an incremental run started and how far it caught up are detail here, present when known | | `base_seconds` | string | wall time spent obtaining the base, the artifact lookup included | | `head_seconds` | string | wall time of the head analysis | @@ -107,15 +107,29 @@ them: |---|---| | the published `codeboarding-base--` artifact with a compatible depth cap | none | | no usable artifact — check out the merge base, seed from a compatible baseline committed there, catch up | one incremental, full if Core requires it | -| no compatible committed baseline either | full analysis directly, at the configured `depth_cap` | +| no compatible committed baseline either: the nearest `codeboarding-base--` artifact among the merge base's last 100 first-parent ancestors | one incremental from that commit to the merge base | +| none within 100 commits either | full analysis directly, at the configured `depth_cap` | A trusted run that computed the base publishes it, so the next pull request -forking from that commit gets the first row. The review metadata reports the +forking from that commit gets the first row; that includes a base caught up from +an ancestor. The review metadata reports the row as `base_analysis_method` (`reused`, `incremental` or `full`) with a `base_analysis_reason`, and the review comment repeats both under the diagram, with measured times. While a base is computed, the progress comment says so in two steps, with the elapsed time and the reason. +The ancestor lookup walks the merge base's first-parent history, deepening the +shallow checkout to 101 commits, then pages through the repository's artifacts +newest first, keeping those named for this configuration and produced by a run on +the repository's own code. It stops at the first page holding one of the walked +commits (usually the first; at most 50 pages) and takes the nearest commit seen. +The merge base's own `.codeboardingignore` and health configuration replace the +seed's. A base caught up this way reports `incremental`, its reason naming the +ancestor and the commits caught up. Sync uses the same lookup when the branch has +no usable committed baseline, so the first sync after the setup pull request +merges catches up from the base that pull request's review saved, instead of +analyzing from scratch. + The configuration hash includes `depth_cap`. The workflow input controls depth for both fresh and fallback analyses; stored legacy depth values never override it. diff --git a/scripts/action/analyze.sh b/scripts/action/analyze.sh index a7048e5..25a9b5e 100755 --- a/scripts/action/analyze.sh +++ b/scripts/action/analyze.sh @@ -124,14 +124,24 @@ analyze_sync() { rm -rf "$work" seed_state "$CHECKOUT_DIR" "$state" - if [ "${FORCE_FULL,,}" = true ] || [ "$(depth_cap_from "$state/analysis.json")" != "$DEPTH_CAP" ]; then - full "$CHECKOUT_DIR" "$state" "$DEPTH_CAP" - else - incremental "$CHECKOUT_DIR" "$state" - if [ "$REQUIRES_FULL" = true ]; then - full "$CHECKOUT_DIR" "$state" "$DEPTH_CAP" + REQUIRES_FULL=true + if [ "$(printf '%s' "${FORCE_FULL:-false}" | tr '[:upper:]' '[:lower:]')" != true ]; then + if [ "$(depth_cap_from "$state/analysis.json")" = "$DEPTH_CAP" ]; then + incremental "$CHECKOUT_DIR" "$state" + fi + # A branch with no usable committed baseline, such as the first sync after the + # setup pull request merged, catches up from that pull request's saved base. + if [ "$REQUIRES_FULL" = true ] && + seed_from_ancestor "${REPOSITORY:-}" "$(git -C "$CHECKOUT_DIR" rev-parse HEAD 2>/dev/null || true)" "$state" true "$CHECKOUT_DIR"; then + incremental "$CHECKOUT_DIR" "$state" + [ "$REQUIRES_FULL" = true ] || + echo "::notice::Caught up from the saved analysis of $ANCESTOR_SHA instead of analyzing from scratch." fi fi + unset GIT_TOKEN + if [ "$REQUIRES_FULL" = true ]; then + full "$CHECKOUT_DIR" "$state" "$DEPTH_CAP" + fi # Sync already computes the graph every review of this branch compares against, # so publish it instead of making the first pull request recompute it. stage "$state" base @@ -140,8 +150,9 @@ analyze_sync() { } # How far below the merge base this run looks for the commit a saved analysis -# describes. Past it, a catch-up count is reported as unknown. -CATCHUP_BOUND=100 +# describes, and for a saved ancestor to catch up from. Past it, a catch-up count +# is unknown and no ancestor is used. It is also the fetch depth. +CATCHUP_BOUND="${CATCHUP_BOUND:-100}" # A depth above 1 also deepens a commit the shallow checkout already holds. fetch_commit() { @@ -209,6 +220,46 @@ catchup_count() { echo "$count" } +# Replaces $3 with the saved analysis of the nearest first-parent ancestor of $2 +# (or of $2 itself when $4 is true), keeping the user-authored configuration of +# checkout $5. Sets ANCESTOR_SHA, or ANCESTOR_REASON=incompatible when the only +# candidate was saved under another configuration or depth. +ANCESTOR_SHA="" ANCESTOR_REASON="" +seed_from_ancestor() { + local repository="$1" tip="$2" state="$3" include_tip="$4" config_from="$5" found dest="$RUNNER_TEMP/codeboarding-ancestor" + ANCESTOR_SHA="" ANCESTOR_REASON="" + [ "${ANCESTOR_LOOKUP:-false}" = true ] && [ -n "${CFG_HASH:-}" ] && [ -n "${REPOSITORY:-}" ] && [ -n "$tip" ] || return 1 + fetch_commit "$repository" "$tip" "$(( CATCHUP_BOUND + 1 ))" || true + found="$(GH_TOKEN="${GIT_TOKEN:-}" GH_ENTERPRISE_TOKEN="${GIT_TOKEN:-}" TIP_SHA="$tip" DEST="$dest" \ + INCLUDE_TIP="$include_tip" CATCHUP_BOUND="$CATCHUP_BOUND" \ + "$ACTION_PATH/scripts/action/find-ancestor-base.sh" || true)" + ANCESTOR_SHA="$(awk -F= '$1 == "ancestor_sha" {print $2; exit}' <<< "$found")" + if [ -z "$ANCESTOR_SHA" ]; then + ! grep -qx 'other_cfg_at_tip=true' <<< "$found" || ANCESTOR_REASON=incompatible + return 1 + fi + if [ "$(depth_cap_from "$dest/analysis.json")" != "$DEPTH_CAP" ]; then + ANCESTOR_SHA="" ANCESTOR_REASON=incompatible + return 1 + fi + rm -rf "$state" + cp -a "$dest" "$state" + keep_user_config "$config_from" "$state" +} +# What the user writes under .codeboarding/ belongs to the commit being analysed, +# not to whichever analysis seeded it. +USER_CONFIG=(.codeboardingignore health/health_config.json health/.healthignore) +keep_user_config() { + local checkout="$1" state="$2" file + for file in "${USER_CONFIG[@]}"; do + rm -f "${state:?}/$file" + [ ! -f "$checkout/.codeboarding/$file" ] || { + mkdir -p "$(dirname "$state/$file")" + cp "$checkout/.codeboarding/$file" "$state/$file" + } + done +} + # Rewrites the sticky progress comment while the base is built from scratch. A # fork's read-only token makes every call fail, which costs nothing. PROGRESS_PID="" @@ -291,9 +342,20 @@ analyze_review() { elif [ -f "$base_state/analysis.json" ]; then full_cause=incompatible fi + # Nothing at the merge base to grow from: catch up from the nearest saved + # ancestor, and publish the result under the merge base's own name below. + if [ "$REQUIRES_FULL" = true ] && seed_from_ancestor "$REVIEW_BASE_REPO" "$REVIEW_BASE_SHA" "$base_state" false "$base_checkout"; then + incremental "$base_checkout" "$base_state" + if [ "$REQUIRES_FULL" = true ]; then + full_cause=incompatible + else + base_method=incremental base_from_sha="$ANCESTOR_SHA" + catchup_commits="$(catchup_count "$ANCESTOR_SHA" "$REVIEW_BASE_SHA")" + fi + fi if [ "$REQUIRES_FULL" = true ]; then base_method=full - full_cause="${full_cause:-no_baseline}" + full_cause="${full_cause:-${ANCESTOR_REASON:-no_baseline}}" trap progress_stop EXIT progress_start "$base_started" full "$base_checkout" "$base_state" "$DEPTH_CAP" @@ -365,7 +427,7 @@ base_reason() { else reason="updated the analysis of ${from:0:7} to $base" case "$count" in - '') ;; + '' | 0) ;; 1) reason="$reason, 1 commit caught up" ;; *) reason="$reason, $count commits caught up" ;; esac diff --git a/scripts/action/find-ancestor-base.sh b/scripts/action/find-ancestor-base.sh new file mode 100755 index 0000000..e3cf119 --- /dev/null +++ b/scripts/action/find-ancestor-base.sh @@ -0,0 +1,81 @@ +#!/usr/bin/env bash +# Finds the nearest first-parent ancestor of TIP_SHA with a saved base analysis +# under this configuration and downloads it into DEST, so a run without an exact +# base can catch up from it instead of analyzing from scratch. Best effort. +# +# Prints key=value lines on stdout and everything else on stderr: +# ancestor_sha the commit whose analysis is now in DEST, or empty +# other_cfg_at_tip true when TIP_SHA has a saved analysis under another configuration +# +# Needs the tip's first-parent history in CHECKOUT_DIR, at least CATCHUP_BOUND deep. +set -euo pipefail +: "${REPOSITORY:?}" "${CFG_HASH:?}" "${TIP_SHA:?}" "${CHECKOUT_DIR:?}" "${DEST:?}" +BOUND="${CATCHUP_BOUND:-100}" +rm -rf "$DEST" + +api() { gh api -H 'Accept: application/vnd.github+json' "$@"; } +export GH_HOST="${GH_HOST:-github.com}" +GH_HOST="${GH_HOST#*://}" + +ancestor="" other_cfg=false +report() { + printf 'ancestor_sha=%s\nother_cfg_at_tip=%s\n' "$ancestor" "$other_cfg" +} +trap report EXIT + +# The merge base's first-parent history, nearest first. Distance 0 is the tip +# itself, which a review already looked up by its exact name; sync has no such +# lookup and passes INCLUDE_TIP=true. +walk="$(git -C "$CHECKOUT_DIR" rev-list --first-parent --max-count=$(( BOUND + 1 )) "$TIP_SHA" 2>/dev/null || true)" +[ "${INCLUDE_TIP:-false}" = true ] || walk="$(tail -n +2 <<< "$walk")" +nearest() { + local commit + for commit in $walk; do + if grep -qx "$commit" <<< "$saved"; then + echo "$commit" + return 0 + fi + done +} + +# The artifact listing, newest first. Its name filter is exact, so a prefix needs +# the pages themselves; paging stops at the first page holding a saved ancestor. +# Bases are published as their commits are synced or reviewed, so the nearest one +# is nearly always the newest: one or two calls in practice, MAX_PAGES (50, 5,000 +# artifacts) at worst, against GITHUB_TOKEN's 1,000 requests an hour per +# repository, and only on a run that would otherwise analyze from scratch. The +# provenance rule is fetch-state.sh's: only artifacts from a run on this +# repository's own code, since a fork's workflow can upload under any name and +# its bytes would reach a pickle loader. +trusted='.artifacts[]? + | select(.expired == false) + | select(.workflow_run != null) + | select(.workflow_run.head_repository_id == .workflow_run.repository_id) + | select(.name | startswith("codeboarding-base-")) + | .name' +prefix="codeboarding-base-$CFG_HASH-" +saved="" page=1 +while [ "$page" -le "${MAX_PAGES:-50}" ]; do + if ! listing="$(api "repos/$REPOSITORY/actions/artifacts?per_page=100&page=$page" 2>/dev/null)"; then + echo "::warning::Could not list artifacts in $REPOSITORY; not looking for an older saved analysis." >&2 + exit 0 + fi + names="$(jq -r "$trusted" <<< "$listing" 2>/dev/null || true)" + saved="$saved$(grep "^$prefix" <<< "$names" | cut -c$(( ${#prefix} + 1 ))- || true)"$'\n' + if grep "^codeboarding-base-.*-$TIP_SHA\$" <<< "$names" | grep -vq "^$prefix"; then + other_cfg=true + fi + ancestor="$(nearest)" + [ -z "$ancestor" ] || break + returned="$(jq -r '.artifacts | length' <<< "$listing" 2>/dev/null || echo 0)" + [ "${returned:-0}" -eq 100 ] || break + page=$(( page + 1 )) +done + +[ -n "$ancestor" ] || exit 0 + +# Downloaded by its exact name, through the same checks as every other lookup. +if ! GITHUB_OUTPUT="" ARTIFACT_NAME="$prefix$ancestor" DEST="$DEST" \ + "$(dirname "$0")/fetch-state.sh" >&2 || [ ! -f "$DEST/analysis.json" ]; then + ancestor="" +fi diff --git a/tests/test_action_state.py b/tests/test_action_state.py index de351fd..70d07ed 100644 --- a/tests/test_action_state.py +++ b/tests/test_action_state.py @@ -654,6 +654,297 @@ def test_analysis_is_staged_for_publication(self) -> None: self.assertFalse((self.stage_dir / "base").exists(), "a published base needs no republishing") +GH_STUB = """#!/usr/bin/env python3 +\"\"\"gh stand-in: serves an artifact listing and one zip from a JSON config.\"\"\" +import json, os, sys +from urllib.parse import parse_qs, urlparse + +config = json.load(open(os.environ["CB_GH_CONFIG"])) +with open(os.environ["CB_GH_LOG"], "a") as log: + log.write(" ".join(sys.argv[1:]) + "\\n") +path = next(a for a in sys.argv[1:] if a.startswith("repos/")) +url = urlparse(path) +query = parse_qs(url.query) +if url.path.endswith("/zip"): + sys.stdout.buffer.write(open(config["zip"], "rb").read()) +elif url.path.endswith("/actions/artifacts"): + artifacts = config["artifacts"] + if "name" in query: + artifacts = [a for a in artifacts if a["name"] == query["name"][0]] + page = int(query.get("page", ["1"])[0]) + if "name" not in query and "pages" in config: + pages = config["pages"] + print(json.dumps({"artifacts": pages[page - 1] if page <= len(pages) else []})) + else: + print(json.dumps({"artifacts": artifacts if page == 1 else []})) +""" + + +class AncestorSeedTests(unittest.TestCase): + """With no saved base for the merge base and nothing committed there, a review + catches up from the nearest saved ancestor on the base branch's first-parent + history instead of analyzing the merge base from scratch.""" + + def setUp(self) -> None: + import zipfile + + self.temp_dir = tempfile.TemporaryDirectory() + self.root = Path(self.temp_dir.name) + self.bin_dir = self.root / "bin" + self.bin_dir.mkdir() + for name, body in (("codeboarding", ENGINE_STUB), ("gh", GH_STUB)): + (self.bin_dir / name).write_text(body, encoding="utf-8") + (self.bin_dir / name).chmod(0o755) + self.engine_log = self.root / "engine.log" + self.engine_log.write_text("", encoding="utf-8") + self.gh_log = self.root / "gh.log" + self.gh_log.write_text("", encoding="utf-8") + self.gh_config = self.root / "gh.json" + self.output = self.root / "github-output" + self.runner_temp = self.root / "runner" + self.runner_temp.mkdir() + self.stage_dir = self.root / "state" / "out" + self.origin = self.root / "origin" + self.origin.mkdir() + self.bundle = self.root / "bundle.zip" + with zipfile.ZipFile(self.bundle, "w") as archive: + archive.writestr("analysis.json", json.dumps({"metadata": {"depth_cap": 2}, "components": []})) + archive.writestr("static_analysis.pkl", "pickle") + archive.writestr("metadata.json", json.dumps({"kind": "base", "merge_base_sha": "ancestor"})) + + def tearDown(self) -> None: + self.temp_dir.cleanup() + + def _history(self, commits: int) -> list[str]: + """A base branch of `commits` code commits, oldest first, in origin/ (the checkout).""" + git = ["git", "-C", str(self.origin), "-c", "user.name=T", "-c", "user.email=t@example.com"] + subprocess.run([*git, "init", "-q"], check=True) + shas = [] + for index in range(commits): + (self.origin / f"file{index}.py").write_text("pass\n", encoding="utf-8") + subprocess.run([*git, "add", "-A"], check=True) + subprocess.run([*git, "-c", "commit.gpgsign=false", "commit", "-q", "-m", f"c{index}"], check=True) + shas.append(subprocess.check_output([*git, "rev-parse", "HEAD"], text=True).strip()) + return shas + + @staticmethod + def _artifact(name: str, *, fork: bool = False) -> dict: + return { + "id": abs(hash(name)) % 100000, + "name": name, + "expired": False, + "created_at": "2026-10-01T00:00:00Z", + "expires_at": "2027-01-01T00:00:00Z", + "workflow_run": {"id": 1, "repository_id": 1, "head_repository_id": 2 if fork else 1}, + } + + def _serve(self, artifacts: list[dict], pages: list | None = None) -> None: + config = {"artifacts": artifacts, "zip": str(self.bundle)} + if pages is not None: + config["pages"] = pages + self.gh_config.write_text(json.dumps(config), encoding="utf-8") + + def _analyze(self, checkout: Path, **extra: str) -> dict[str, str]: + self.output.write_text("", encoding="utf-8") + result = subprocess.run( + [str(ANALYZE)], + env={ + "PATH": f"{self.bin_dir}:{os.environ['PATH']}", + "GITHUB_OUTPUT": str(self.output), + "RUNNER_TEMP": str(self.runner_temp), + "CB_ENGINE_LOG": str(self.engine_log), + "CB_GH_CONFIG": str(self.gh_config), + "CB_GH_LOG": str(self.gh_log), + "ACTION_PATH": str(ROOT), + "ANALYSIS_KIND": "review", + "CHECKOUT_DIR": str(checkout), + "REVIEW_HEAD_SHA": "head-sha", + "REVIEW_BASE_REPO": "origin", + "REPOSITORY": "owner/repo", + "GITHUB_SERVER_URL": f"file://{self.root}", + "PR_NUMBER": "42", + "ENGINE_VERSION": "0.14.5", + "CFG_HASH": "cfg", + "ANCESTOR_LOOKUP": "true", + "BASE_DIR": str(self.root / "state" / "base"), + "WARMSTART_DIR": str(self.root / "state" / "warmstart"), + "STAGE_DIR": str(self.stage_dir), + "DEPTH_CAP": "2", + **extra, + }, + capture_output=True, + text=True, + check=False, + ) + self.assertEqual(result.returncode, 0, result.stderr or result.stdout) + values: dict[str, str] = {} + for line in self.output.read_text(encoding="utf-8").splitlines(): + key, _, value = line.partition("=") + values[key] = value + return values + + def _modes(self) -> list[str]: + return [json.loads(line)["mode"] for line in self.engine_log.read_text().splitlines()] + + def _assert_full(self, values: dict[str, str], reason: str = "no usable analysis was available") -> None: + self.assertEqual(values["base_analysis_method"], "full") + self.assertEqual(values["base_analysis_reason"], reason) + + def test_it_catches_up_from_the_nearest_saved_ancestor(self) -> None: + shas = self._history(5) + merge_base = shas[-1] + # Two saved ancestors: the nearer one wins. + self._serve( + [self._artifact(f"codeboarding-base-cfg-{shas[0]}"), self._artifact(f"codeboarding-base-cfg-{shas[2]}")] + ) + + values = self._analyze(self.origin, REVIEW_BASE_SHA=merge_base) + + self.assertEqual(values["base_analysis_method"], "incremental") + self.assertEqual( + values["base_analysis_reason"], + f"updated the analysis of {shas[2][:7]} to {merge_base[:7]}, 2 commits caught up", + ) + self.assertEqual(self._modes(), ["incremental", "incremental"]) + # Published under the merge base's own name, so the next review hits it exactly. + self.assertEqual(values["publish_base"], "true") + staged = json.loads((self.stage_dir / "base" / "metadata.json").read_text()) + self.assertEqual(staged["merge_base_sha"], merge_base) + + def test_an_ancestor_beyond_the_bound_is_not_used(self) -> None: + shas = self._history(5) + self._serve([self._artifact(f"codeboarding-base-cfg-{shas[0]}")]) + + values = self._analyze(self.origin, REVIEW_BASE_SHA=shas[-1], CATCHUP_BOUND="2") + + self._assert_full(values) + self.assertEqual(self._modes(), ["full", "incremental"]) + self.assertNotIn("/zip", self.gh_log.read_text()) + + def test_a_saved_analysis_off_this_history_is_not_used(self) -> None: + shas = self._history(2) + self._serve([self._artifact("codeboarding-base-cfg-" + "e" * 40)]) + + values = self._analyze(self.origin, REVIEW_BASE_SHA=shas[-1]) + + self._assert_full(values) + + def test_a_busy_artifact_store_is_paged_until_a_saved_ancestor_appears(self) -> None: + shas = self._history(3) + noise = [self._artifact(f"codeboarding-review-{i}-1") for i in range(100)] + found = self._artifact(f"codeboarding-base-cfg-{shas[0]}") + later = [self._artifact(f"codeboarding-warmstart-cfg-pr{i}") for i in range(100)] + self._serve([found], pages=[noise] * 11 + [noise[:99] + [found], later]) + + values = self._analyze(self.origin, REVIEW_BASE_SHA=shas[-1]) + + self.assertEqual(values["base_analysis_method"], "incremental") + self.assertIn(f"updated the analysis of {shas[0][:7]} ", values["base_analysis_reason"]) + listings = [ + line + for line in self.gh_log.read_text().splitlines() + if "per_page=100&page=" in line and "name=" not in line + ] + self.assertEqual(len(listings), 12, "paging stops at the page holding the ancestor") + + def test_a_failed_deepen_falls_back_to_a_full_analysis(self) -> None: + # The deepen fails, so the shallow checkout walks no ancestor at all. + shas = self._history(4) + bare = self.root / "origin.git" + subprocess.run(["git", "clone", "-q", "--bare", str(self.origin), str(bare)], check=True) + checkout = self.root / "shallow" + subprocess.run(["git", "clone", "-q", "--depth=1", f"file://{bare}", str(checkout)], check=True) + self._serve([self._artifact(f"codeboarding-base-cfg-{shas[0]}")]) + + values = self._analyze(checkout, REVIEW_BASE_SHA=shas[-1], GITHUB_SERVER_URL=f"file://{self.root}/missing") + + self._assert_full(values) + + def test_the_merge_bases_own_configuration_survives_the_seed(self) -> None: + shas = self._history(2) + (self.origin / ".codeboarding").mkdir() + (self.origin / ".codeboarding" / ".codeboardingignore").write_text("docs/\n", encoding="utf-8") + git = ["git", "-C", str(self.origin), "-c", "user.name=T", "-c", "user.email=t@example.com"] + subprocess.run([*git, "add", "-A"], check=True) + subprocess.run([*git, "-c", "commit.gpgsign=false", "commit", "-q", "-m", "ignore docs"], check=True) + merge_base = subprocess.check_output([*git, "rev-parse", "HEAD"], text=True).strip() + import zipfile + + with zipfile.ZipFile(self.bundle, "a") as archive: + archive.writestr(".codeboardingignore", "stale/\n") + archive.writestr("health/health_config.json", "{}") + self._serve([self._artifact(f"codeboarding-base-cfg-{shas[0]}")]) + + values = self._analyze(self.origin, REVIEW_BASE_SHA=merge_base) + + self.assertEqual(values["base_analysis_method"], "incremental") + staged = self.stage_dir / "base" + self.assertEqual((staged / ".codeboardingignore").read_text(), "docs/\n") + self.assertFalse((staged / "health" / "health_config.json").exists(), "the merge base has none") + + def test_an_ancestor_saved_by_a_run_on_forked_code_is_never_read(self) -> None: + shas = self._history(3) + self._serve([self._artifact(f"codeboarding-base-cfg-{shas[0]}", fork=True)]) + + values = self._analyze(self.origin, REVIEW_BASE_SHA=shas[-1]) + + self._assert_full(values) + self.assertNotIn("/zip", self.gh_log.read_text(), "a fork's bundle was downloaded") + + def test_another_configuration_saved_at_the_merge_base_is_incompatible(self) -> None: + shas = self._history(2) + self._serve([self._artifact(f"codeboarding-base-othercfg-{shas[-1]}")]) + + values = self._analyze(self.origin, REVIEW_BASE_SHA=shas[-1]) + + self._assert_full(values, "the existing analysis was incompatible or could not be updated incrementally") + + def test_a_shallow_checkout_is_deepened_to_find_the_ancestor(self) -> None: + shas = self._history(4) + bare = self.root / "origin.git" + subprocess.run(["git", "clone", "-q", "--bare", str(self.origin), str(bare)], check=True) + subprocess.run(["git", "-C", str(bare), "config", "uploadpack.allowAnySHA1InWant", "true"], check=True) + checkout = self.root / "shallow" + subprocess.run(["git", "clone", "-q", "--depth=1", f"file://{bare}", str(checkout)], check=True) + self._serve([self._artifact(f"codeboarding-base-cfg-{shas[0]}")]) + + values = self._analyze(checkout, REVIEW_BASE_SHA=shas[-1]) + + self.assertEqual( + values["base_analysis_reason"], + f"updated the analysis of {shas[0][:7]} to {shas[-1][:7]}, 3 commits caught up", + ) + + def test_without_the_lookup_nothing_is_listed(self) -> None: + # GHES has no artifact store, so the action turns the lookup off there. + shas = self._history(2) + self._serve([self._artifact(f"codeboarding-base-cfg-{shas[0]}")]) + + values = self._analyze(self.origin, REVIEW_BASE_SHA=shas[-1], ANCESTOR_LOOKUP="false") + + self.assertEqual(values["base_analysis_method"], "full") + self.assertEqual(self.gh_log.read_text(), "") + + def test_a_first_sync_catches_up_from_the_setup_reviews_base(self) -> None: + # Merging the setup pull request leaves no committed baseline, but its + # preview review saved the base at its merge base, the new tip's parent. + shas = self._history(3) + self._serve([self._artifact(f"codeboarding-base-cfg-{shas[1]}")]) + + self._analyze(self.origin, ANALYSIS_KIND="sync", FORCE_FULL="false") + + self.assertEqual(self._modes(), ["incremental"]) + + def test_a_forced_sync_never_seeds(self) -> None: + shas = self._history(2) + self._serve([self._artifact(f"codeboarding-base-cfg-{shas[0]}")]) + + self._analyze(self.origin, ANALYSIS_KIND="sync", FORCE_FULL="True") + + self.assertEqual(self._modes(), ["full"]) + self.assertEqual(self.gh_log.read_text(), "") + + class ReviewArtifactTests(unittest.TestCase): """The artifact is the only channel a reader outside the run can use: cache entries have no download API, so whatever the webview needs must ship here."""