Fix bundle transport in shallow checkouts

[Copilot]

Bug Fix

What was the bug?

Bundle transport can fail before ref updates in fetch-depth: 1 checkouts because git fetch <bundle> rejects bundles whose prerequisite commits are missing locally. This affects both create_pull_request and push_to_pull_request_branch.

How did you fix it?

• Shared shallow-checkout guard

  • Added ensureFullHistoryForBundle() to detect shallow repositories before bundle fetch.

  • Runs:

    git fetch --unshallow origin

    only when git rev-parse --is-shallow-repository reports true.

  • If shallow status cannot be determined, logs a warning and skips unshallowing instead of failing bundle application early.

• Bundle application paths

  • Call the guard before git fetch <bundle> in:
    • create_pull_request
    • push_to_pull_request_branch

• Regression coverage

  • Added focused tests for shallow bundle application and verified unshallow happens before bundle fetch.
  • Added fallback coverage for shallow-status detection failures.

[Copilot]

Pull request overview

Fixes failures when applying git bundles in shallow (fetch-depth: 1) checkouts by unshallowing the repository before git fetch <bundle> so bundle prerequisite commits are available locally.

Changes:

• Added ensureFullHistoryForBundle() in git_helpers.cjs to detect shallow repos and run git fetch --unshallow origin only when needed.
• Invoked the shallow-history guard before bundle fetch in both create_pull_request and push_to_pull_request_branch.
• Added regression tests to validate unshallow happens before bundle fetch (and only when shallow).

Show a summary per file

File	Description
actions/setup/js/git_helpers.cjs	Adds ensureFullHistoryForBundle() helper to unshallow before bundle fetch when required.
actions/setup/js/git_helpers.test.cjs	Adds unit tests covering shallow vs non-shallow behavior for the new helper.
actions/setup/js/create_pull_request.cjs	Calls the new helper before fetching from a bundle during PR creation.
actions/setup/js/create_pull_request.test.cjs	Adds regression test asserting unshallow occurs before bundle fetch in shallow checkouts.
actions/setup/js/push_to_pull_request_branch.cjs	Calls the new helper before fetching from a bundle when pushing to an existing PR branch.
actions/setup/js/push_to_pull_request_branch.test.cjs	Updates test to simulate shallow repo and assert unshallow precedes bundle fetch.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 6/6 changed files
• Comments generated: 0

[github-actions]

Skills-Based Review 🧠

Applied /diagnose and /tdd — this is a bug fix with regression tests, the classic pairing.

Key Themes

• Missing error resilience in the guard: ensureFullHistoryForBundle does not catch a failure from git rev-parse --is-shallow-repository. If that command fails for any environmental reason, the error bubbles up to the outer try/catch and surfaces as a generic bundle-apply failure, masking the real cause.
• Dead mock branch in the new test: The cmd.startsWith("git ls-remote") condition in the create_pull_request bundle test is never true (cmd is "git"), so the intent is invisible and the mock is slightly misleading.

Positive Highlights

• ✅ Correct seam for the regression test — the fix is tested at the call-site level (integration-style), not just unit-tested in isolation.
• ✅ Ordering assertion — both tests verify that unshallow happens before the bundle fetch using call-index comparison. That's exactly the right assertion for this bug class.
• ✅ Shared helper — extracting ensureFullHistoryForBundle into git_helpers.cjs keeps both call sites DRY and testable independently.
• ✅ push_to_pull_request_branch passes auth env — the options including gitAuthEnv are forwarded correctly.

Verdict

No blocking issues. The two items above are worth a quick follow-up (error resilience, dead mock branch) but neither is a correctness bug in the happy path.

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · ● 7.4M

[github-actions]

[/diagnose] The getExecOutput call has no error handling for when git rev-parse --is-shallow-repository fails (e.g., unusual git environment). An error here propagates out and is caught by the outer bundle-fetch try/catch, silently aborting the PR creation with { success: false } — which looks like a bundle error, not a shallow-check error.

Consider treating any failure as non-shallow and logging a warning:

try {
  ({ stdout } = await execApi.getExecOutput(
    'git', ['rev-parse', '--is-shallow-repository'], options
  ));
} catch {
  core.warning('Could not determine shallow status; skipping unshallow');
  return;
}

Adding a test for this error path would lock in the fallback behaviour.

[github-actions]

[/tdd] This mock branch is dead code. cmd is "git" (just the executable), so cmd.startsWith("git ls-remote") is always false. The catch-all return covers ls-remote calls, but the intent is invisible.

Fix to match by args:

if (cmd === 'git' && args && args[0] === 'ls-remote') {
  return Promise.resolve({ exitCode: 0, stdout: '', stderr: '' });
}

The pattern used in the existing tests at lines ~1848 (cmdStr.includes('ls-remote --heads origin')) is an alternative. Either way, the dead branch should be removed so the mock accurately documents its contract.

[github-actions]

🧪 Test Quality Sentinel Report

Test Quality Score: 80/100

✅ Excellent — all new tests enforce behavioral contracts.

Metric	Value
New/modified tests analyzed	4
✅ Design tests (behavioral contracts)	4 (100%)
⚠️ Implementation tests (low value)	0 (0%)
Tests with error/edge cases	4 (100%)
Duplicate test clusters	0
Test inflation detected	Yes (create_pull_request.test.cjs: 127 test lines added vs. 3 production lines)
🚨 Coding-guideline violations	None

---

Test Classification Details

View all 4 tests

Test	File	Classification	Notes
"should unshallow before fetching a bundle"	create_pull_request.test.cjs	✅ Design	Verifies unshallow happens before bundle fetch (ordering contract)
"should fetch full history when repository is shallow"	git_helpers.test.cjs	✅ Design	Verifies ensureFullHistoryForBundle calls fetch --unshallow on shallow repo
"should not fetch full history when repository is not shallow"	git_helpers.test.cjs	✅ Design	Edge case: no exec call when repo is not shallow
Modified bundle push test	push_to_pull_request_branch.test.cjs	✅ Design	Extended to assert unshallow precedes bundle fetch (ordering contract)

---

Test Inflation Note

create_pull_request.test.cjs added 127 lines vs. 3 production lines added to create_pull_request.cjs. The inflation is driven by a large beforeEach/afterEach setup block (mocking global.core, global.github, global.exec, etc.), not by low-value test duplication. The actual test body is compact and behavioral. While the ratio triggers the inflation penalty (-10 pts), the setup complexity is justified by the integration-style test exercising the full main() handler.

push_to_pull_request_branch.test.cjs added ~16 lines vs. ~5 production lines. The extra lines reorganize mockResolvedValueOnce chaining into a named mockImplementation and add ordering assertions — quality improvements, not inflation.

---

Mocking Assessment

All mocking targets are external I/O or GitHub Actions runtime globals (global.core, global.github, global.context, global.exec), which is the accepted pattern in this codebase. The pushSignedCommits spy is on an external module boundary (git operations). No internal business-logic functions are mocked without a behavioral output assertion. ✅

---

Language Support

Tests analyzed:

• 🟨 JavaScript (*.test.cjs): 4 tests (vitest)
• 🐹 Go (*_test.go): 0 tests changed

---

Verdict

✅ Check passed. 0% of new tests are implementation tests (threshold: 30%). Test quality score: 80/100.

---

📖 Understanding Test Classifications

Design Tests (High Value) verify what the system does:

• Assert on observable outputs, return values, or state changes
• Cover error paths and boundary conditions
• Would catch a behavioral regression if deleted
• Remain valid even after internal refactoring

Implementation Tests (Low Value) verify how the system does it:

• Assert on internal function calls (mocking internals)
• Only test the happy path with typical inputs
• Break during legitimate refactoring even when behavior is correct
• Give false assurance: they pass even when the system is wrong

Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators.

References: §25707268158

🧪 Test quality analysis by Test Quality Sentinel · ● 6.6M · ◷

[github-actions]

✅ Test Quality Sentinel: 80/100. Test quality is acceptable — 0% of new tests are implementation tests (threshold: 30%).

[pelikhan]

@copilot review all comments

[Copilot]

@copilot review all comments

Reviewed all comments and addressed the actionable items in ce4c4d6: shallow-status detection failures now warn and skip unshallow, the dead ls-remote mock branch is fixed, and fallback coverage was added. Targeted tests and formatting checks pass.