fix(bundle): fetch prerequisite commits by SHA instead of broad deepen#39466
Merged
Conversation
The safe_outputs bundle-application path could hang while "iteratively
deepening origin/<base>" when updating a PR branch with a long, complex
history. The deepen gate asked whether each bundle prerequisite was an
ancestor of origin/<base>; for push-to-PR-branch updates the prerequisite
lives on the PR branch and is never an ancestor of base, so deepening base
could never satisfy it and escalated toward a full --unshallow that timed
out.
Rework ensureFullHistoryForBundle:
- Primary path now fetches the exact prerequisite commit SHAs (declared by
git bundle verify) directly from origin via `git fetch origin <sha>...`,
which GitHub honors and which brings precisely the needed objects
regardless of which branch they live on.
- Gate changed from base-branch ancestry (merge-base --is-ancestor) to
object presence (git cat-file -e <sha>^{commit}), which is what bundle
apply actually requires.
- Fallback deepen now steps by a small fixed increment (5 commits) up to a
bounded iteration cap, re-checking presence each step, so a single fetch
cannot pull a huge slice of monorepo history. --unshallow remains a last
resort only.
Update tests to cover the direct-SHA primary path and the deepen-by-5
fallback.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes bundle-application hangs in shallow clones by switching prerequisite satisfaction from “is ancestor of origin/” checks + broad deepening to a targeted “object exists locally” gate with a primary fetch-by-SHA strategy, plus a bounded deepen fallback.
Changes:
- Reworked
ensureFullHistoryForBundleto fetch missing bundle prerequisite commit SHAs directly fromoriginand gate ongit cat-file -e <sha>^{commit}. - Added a bounded fallback deepen strategy using a small fixed step (
--deepen=5) with an iteration cap before last-resort--unshallow. - Updated unit/integration tests to cover the direct-SHA path and the new fallback deepen behavior.
Show a summary per file
| File | Description |
|---|---|
| actions/setup/js/git_helpers.cjs | Implements direct prerequisite SHA fetch, presence-based gating, and bounded deepen fallback. |
| actions/setup/js/git_helpers.test.cjs | Updates tests to validate direct SHA fetch and fixed-step deepen fallback. |
| actions/setup/js/create_pull_request.test.cjs | Adjusts create PR bundle shallow-checkout tests to assert direct SHA prerequisite fetch and no deepen/unshallow. |
| actions/setup/js/push_to_pull_request_branch.test.cjs | Adjusts push-to-PR bundle application test to assert direct SHA fetch and no deepen. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 4/4 changed files
- Comments generated: 1
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
|
✅ smoke-ci: safeoutputs CLI comment + comment-memory run (27583773725)
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(bundle): fetch prerequisite commits by SHA instead of broad deepen
Summary
Replaces the iterative-deepen-only strategy for resolving bundle prerequisites with a targeted three-tier approach. The primary path now fetches the exact prerequisite SHA directly from origin (
git fetch origin <sha>), falling back to fixed-step iterative deepening only if that fails, and resorting to--unshallowas a last option. This is more efficient and reliable — especially in shallow clones where the prerequisite object already exists remotely and a broad deepen would waste network and time.Alongside the fetch strategy change, the missing-object check was corrected from an ancestry probe (
merge-base --is-ancestor) to a plain object-presence probe (cat-file -e), matching its actual purpose: confirming the object exists in the local repo, not that it sits on a specific branch.What changed
actions/setup/js/git_helpers.cjs— core logic (high impact)BUNDLE_DEEPEN_STEPSarray--unshallowfindMissingAncestorsusingmerge-base --is-ancestorfindMissingObjectsusingcat-file -eBUNDLE_DEEPEN_STEP=5,BUNDLE_DEEPEN_MAX_ITERATIONS=200The rename from
findMissingAncestors→findMissingObjectsreflects the corrected semantics: the function answers "does this object exist locally?", not "is this commit an ancestor of HEAD?".actions/setup/js/git_helpers.test.cjs— unit tests (medium impact)merge-base --is-ancestormocks withcat-file -emocks throughout.actions/setup/js/create_pull_request.test.cjs— integration test (medium impact)--deepenor--unshallowinvolved.beforeEachmock updated frommerge-base --is-ancestortocat-file -ereturning success by default.actions/setup/js/push_to_pull_request_branch.test.cjs— shallow-checkout test (medium impact)prereqSha/prereqFetchedtracking variables.git merge-base --is-ancestorstub replaced withgit cat-file -estub whose exit code is gated on whether a direct fetch has already occurred.mockExec.execimplementation now setsprereqFetched = truewhen it observesgit fetch origin <prereqSha>.--deepencall was made.Why this matters
In shallow checkouts, iterating
--deepento hunt for a prerequisite SHA is a best-effort heuristic — it may overshoot (fetching far more history than needed) or fail entirely if the prerequisite is on a different branch. Fetching by SHA directly is deterministic, minimal, and works regardless of branch topology. The test coverage changes confirm the happy path no longer touches--deepenat all.Risk
Low. No public API or workflow interface changes. The three-tier fallback ensures behaviour degrades gracefully to the previous deepen strategy if the direct fetch fails for any reason. Breaking: false across all changed files.