Fix safe output bundle fetch for checked-out PR branches

[Copilot]

Bug Fix

What was the bug?

safe_outputs could fail during create_pull_request when the bundle target branch was already checked out. Git rejects direct fetches into the checked-out branch ref:

git fetch bundle refs/heads/autoloop/perf-comparison:refs/heads/autoloop/perf-comparison

How did you fix it?

• Bundle application

  • Fetch bundle commits into a temporary ref under refs/bundles/...
  • Update the target branch with git update-ref
  • Checkout and hard-reset the branch after updating the ref

• Regression coverage

  • Updated bundle transport tests to assert the handler no longer fetches directly into refs/heads/<branch>
  • Added coverage for branches like autoloop/perf-comparison

git fetch bundle refs/heads/<source>:refs/bundles/create-pr-<branch>
git update-ref refs/heads/<branch> refs/bundles/create-pr-<branch>
git checkout <branch>
git reset --hard

[github-actions]

Hey @copilot-swe-agent 👋 — thanks for tackling this bundle fetch bug! The fix is technically solid: using a temporary refs/bundles/... ref and then updating the branch with git update-ref is the right approach for checked-out branches, and it's great to see regression test coverage included.

However, this PR was opened directly by a coding agent on behalf of an external contributor, which doesn't match the contribution model for this project. Direct PRs from non-core team members are not accepted — even well-crafted ones from agents.

Here's what to do instead:

• Open a GitHub Issue describing the bug and your proposed fix as a detailed agentic plan. Include the root cause analysis, the refs/bundles/... approach, and the test strategy you already have.
• A core team member will review the plan and have their own agent implement and open the PR.

If you'd like to prepare a ready-to-use plan for the issue, your agent can generate it with this prompt:

Write a detailed agentic implementation plan for a GitHub Issue describing the following bug fix:

Bug: `safe_outputs` fails during `create_pull_request` when the bundle target branch is already checked out, because git rejects direct fetches into a checked-out branch ref.

Proposed fix:
- In `actions/setup/js/create_pull_request.cjs`, fetch bundle commits into a temporary ref under `refs/bundles/create-pr-<branch>` instead of directly into `refs/heads/<branch>`.
- Use `git update-ref refs/heads/<branch> refs/bundles/create-pr-<branch>` to update the target branch.
- Follow up with `git checkout <branch> && git reset --hard`.

Regression tests:
- Update `actions/setup/js/create_pull_request.test.cjs` to assert the handler no longer fetches directly into `refs/heads/<branch>`.
- Add a test case for branches with slashes like `autoloop/perf-comparison`.

Format the output as a GitHub issue body with sections: Background, Root Cause, Proposed Solution, Test Plan.

Generated by Contribution Check · ● 7.9M · ◷

[pelikhan]

@copilot add integration test

[Copilot]

@copilot add integration test

Added a real git integration test for the checked-out target branch bundle case in 3810fa7, with follow-up cleanup in 083ed99.

[Copilot]

Pull request overview

Fixes bundle application for create-pull-request flows where the target branch may already be checked out.

Changes:

• Adds applyBundleToBranch to fetch bundles into a temporary ref before updating/checking out the target branch.
• Updates bundle transport unit assertions for temporary refs and update-ref.
• Adds a real-git integration test for checked-out branch bundle application.

Show a summary per file

File	Description
actions/setup/js/create_pull_request.cjs	Refactors bundle application through a temporary ref and updates fallback instructions.
actions/setup/js/create_pull_request.test.cjs	Updates bundle fetch expectations and adds branch-with-slash regression coverage.
actions/setup/js/create_pull_request_bundle_integration.test.cjs	Adds integration coverage for applying bundles when the target branch is checked out.

Copilot's findings

Tip

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

• Files reviewed: 3/3 changed files
• Comments generated: 1

[Copilot]

Updated in 5513624 to use the temporary-ref/update-ref/reset flow in the fallback issue instructions instead of fetching directly into the branch ref.

[github-actions]

🧪 Test Quality Sentinel Report

Test Quality Score: 100/100

✅ Excellent test quality

Metric	Value
New/modified tests analyzed	5 (2 new, 3 updated)
✅ Design tests (behavioral contracts)	5 (100%)
⚠️ Implementation tests (low value)	0 (0%)
Tests with error/edge cases	5 (100%)
Duplicate test clusters	0
Test inflation detected	No
🚨 Coding-guideline violations	None

---

Test Classification Details

View all 5 tests

Test	File	Classification	Notes
"should not fetch a bundle directly into the target branch" (new)	create_pull_request.test.cjs	✅ Design	Verifies negative (old arg pattern) + positive (temp-ref approach + update-ref + reset)
"applies a bundle when the target branch is currently checked out" (new)	create_pull_request_bundle_integration.test.cjs	✅ Design	Real git ops; proves direct fetch fails, then asserts fix yields correct HEAD/file contents and cleans up temp ref
Updated "fetches bundle..." test	create_pull_request.test.cjs	✅ Design	Assertion args updated to match new 3-step git protocol (fetch→temp ref, update-ref, reset)
Updated fallback error test	create_pull_request.test.cjs	✅ Design	Error-path coverage preserved; uses .toThrow() equivalent mock; updated temp-ref target
Updated list-heads fallback test	create_pull_request.test.cjs	✅ Design	Covers the main→<branch> fallback mapping with updated temp-ref arg

---

Flagged Tests — Requires Review

None. All tests are well-formed with behavioral assertions.

---

Highlights

New integration test is a standout. create_pull_request_bundle_integration.test.cjs is a genuine regression guard: it first proves the old approach fails (git fetch into a checked-out branch exits non-zero with "refusing to fetch into branch"), then calls applyBundleToBranch() and asserts the working tree HEAD matches the bundle tip, file contents are correct, and the temporary refs/bundles/ ref was cleaned up. Deleting this test would allow the original bug to silently regress.

Mocking is appropriate. Unit tests spy on global.exec (the GitHub Actions exec API), which is external I/O — a legitimate mock target. No internal business-logic functions are mocked.

Test inflation check. The integration file adds 115 lines vs 59 lines added to the production file — a ratio of 1.95:1, just under the 2:1 threshold. Given the setup boilerplate required for real git repos, this is expected and acceptable.

---

Language Support

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

---

Verdict

✅ Check passed. 0% of new tests are implementation tests (threshold: 30%). Both new tests enforce genuine behavioral contracts — one as a focused unit regression guard, one as a real-git integration proof.

---

📖 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:

• §25858342021

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

[github-actions]

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

[github-actions]

[/diagnose] The temp ref name sanitises / → -, so branches foo/bar and foo-bar both map to refs/bundles/create-pr-foo-bar. If two concurrent workflow runs target these two branches on the same runner they will overwrite each other's temp ref mid-flight.

Consider including a short random or timestamp suffix to make the ref unique per invocation:

const bundleTempRef = `refs/bundles/create-pr-${branchName.replace(/[^a-zA-Z0-9-]/g, "-")}-${Date.now()}`;

[pelikhan]

@copilot use crypto short suffix

[Copilot]

Implemented in 3f18c9f with a short crypto-random suffix for bundle temp refs; fe85167 adds cleanup verification for the generated ref.

[github-actions]

[/diagnose] git reset --hard (without a ref) discards any staged/unstaged changes on the currently checked-out branch. After git checkout <branch> has already updated the working tree to the new HEAD, the reset is a no-op in the clean CI case — but if for some reason the branch had local uncommitted changes before applyBundleToBranch was called, this silently destroys them.

A comment explaining the intent would help:

// Ensure the working tree matches the new HEAD in case checkout left any index/worktree drift
await execApi.exec("git", ["reset", "--hard"]);

If the goal is strictly to update the working tree to match the new ref, consider git reset --hard HEAD (explicit) or git read-tree -u -m HEAD so the intent is unambiguous.

[github-actions]

[/tdd] The integration test covers the happy path well (temp ref cleaned up, HEAD at expected commit, file contents correct). The error path — what happens when applyBundleToBranch throws mid-way (e.g. update-ref fails after the fetch) — isn't exercised. The finally cleanup block is the most likely place for a latent ref leak.

Consider adding a test that injects a failure after the fetch step and asserts:

1. The refs/bundles/... ref is deleted
2. The target branch ref is not left in a partial state

it("cleans up the temp ref when update-ref fails", async () => {
  // inject error on update-ref, then assert temp ref is gone
});

[pelikhan]

@copilot review all review comments

[Copilot]

@copilot review all review comments

Reviewed all review comments and addressed the remaining actionable ones in afadfdd and fcd7012. Targeted Vitest coverage passes for the create_pull_request bundle tests.