Skip to content

Handle side-repo checkouts in push_to_pull_request_branch by scoping git ops to target repo cwd#27894

Merged
pelikhan merged 10 commits intomainfrom
copilot/fix-push-to-pull-request-branch
Apr 23, 2026
Merged

Handle side-repo checkouts in push_to_pull_request_branch by scoping git ops to target repo cwd#27894
pelikhan merged 10 commits intomainfrom
copilot/fix-push-to-pull-request-branch

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 22, 2026

push_to_pull_request_branch was executing git commands from the workflow repo root, which breaks multi-repo workflows that checkout the target repo into a subdirectory (actions/checkout with path). This caused wrong branch detection and incremental patch generation against the wrong/local-missing branch context.

  • Repo checkout resolution for push handler

    • Added target checkout lookup in push_to_pull_request_branch using findRepoCheckout(repoResult.repo) when entry.repo is provided.
    • Stores resolved path on the entry (entry.repo_cwd) for downstream visibility.
    • Now fails fast with a clear error (including actions/checkout + path guidance) when an explicit entry.repo checkout cannot be found, instead of falling back to the workflow repo.
    • Added debug logging of the selected checkout folder location when an explicit target repo checkout is resolved.
  • Branch detection now uses target repo context

    • Switched branch detection from:
      • getCurrentBranch()
    • to:
      • getCurrentBranch(repoCwd)
    • This aligns behavior with create_pull_request and prevents reading branch state from the workflow repo.
  • Incremental patch/bundle generation now runs in target repo

    • Adds cwd to pushTransportOptions when a target checkout is resolved.
    • Adds repoSlug to pushTransportOptions in multi-repo mode to disambiguate generated patch/bundle filenames under /tmp/gh-aw/.
    • Ensures git fetch, branch existence checks, and incremental patch generation execute in the checked-out target repo, not process.cwd().
  • Coverage for multi-repo behavior

    • Added a focused test that creates a side-repo checkout in workspace, sets branch: "main", and verifies the handler detects the feature branch from that repo context.
    • Added a focused test verifying the explicit-repo missing-checkout error path and guidance text.
    • Expanded tests to simulate tracked-vs-local commits in a side-repo checkout and verify repoSlug-disambiguated patch naming.
    • Added a test assertion verifying the selected checkout folder location is logged.
let repoCwd = null;
if (entry.repo && entry.repo.trim()) {
  const repoSlug = repoResult.repo;
  const checkoutResult = findRepoCheckout(repoSlug);
  if (!checkoutResult.success) {
    return {
      content: [
        {
          type: "text",
          text: JSON.stringify({
            result: "error",
            error:
              `Repository checkout not found for ${repoSlug}. Ensure the repository is checked out in this workflow using actions/checkout. ` +
              "If checking out multiple repositories, use the 'path' input so the checkout can be located.",
          }),
        },
      ],
      isError: true,
    };
  }
  repoCwd = checkoutResult.path;
  entry.repo_cwd = repoCwd;
  server.debug(`Selected checkout folder for ${repoSlug}: ${repoCwd}`);
}

const detectedBranch = getCurrentBranch(repoCwd);

const pushTransportOptions = { mode: "incremental" };
if (repoCwd) {
  pushTransportOptions.cwd = repoCwd;
  pushTransportOptions.repoSlug = repoResult.repo;
}

Copilot AI and others added 2 commits April 22, 2026 20:54
Copilot AI changed the title [WIP] Fix push_to_pull_request_branch for multi-repo checkout support Handle side-repo checkouts in push_to_pull_request_branch by scoping git ops to target repo cwd Apr 22, 2026
Copilot AI requested a review from pelikhan April 22, 2026 20:59
@github-actions
Copy link
Copy Markdown
Contributor

Hey @Copilot 👋 — thanks for this fix to push_to_pull_request_branch! Scoping all git operations to the checked-out target repo's cwd is a meaningful correctness improvement for multi-repo workflows.

One thing worth noting: this repository doesn't have a CONTRIBUTING.md, so there are no formal contribution guidelines to evaluate against. The PR itself looks well-structured — it has a clear description, a focused diff (2 files, 48 lines), and the body mentions a focused test covering the new multi-repo branch detection path.

The only thing to double-check before requesting a full review: the PR is currently in draft state — flip it to "Ready for review" once you're satisfied with the changes.

Generated by Contribution Check · ● 831.6K ·

@pelikhan pelikhan marked this pull request as ready for review April 22, 2026 21:56
Copilot AI review requested due to automatic review settings April 22, 2026 21:56
@github-actions github-actions Bot mentioned this pull request Apr 22, 2026
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Improves push_to_pull_request_branch behavior in multi-repo workflows by scoping git branch detection and incremental patch/bundle generation to the checked-out target repo directory (when provided), and adds test coverage for that path.

Changes:

  • Resolve target repo checkout path and use it for branch detection (getCurrentBranch(repoCwd)).
  • Pass cwd into incremental patch/bundle generation options so git operations run in the target repo checkout.
  • Add a Vitest case that creates a side-repo checkout and verifies branch detection uses that repo’s context.
Show a summary per file
File Description
actions/setup/js/safe_outputs_handlers.cjs Adds repo checkout resolution and scopes branch detection + patch/bundle git operations via cwd.
actions/setup/js/safe_outputs_handlers.test.cjs Adds test validating branch detection in a side-repo checkout when repo is specified.

Copilot's findings

Tip

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

  • Files reviewed: 2/2 changed files
  • Comments generated: 2

Comment on lines +495 to +498
if (checkoutResult.success) {
repoCwd = checkoutResult.path;
entry.repo_cwd = repoCwd;
}
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When entry.repo is provided but findRepoCheckout(repoSlug) fails, the handler silently falls back to running git operations in the workflow repo (or workspace root). This can yield incorrect branch detection/patch generation and can also cause getCurrentBranch(repoCwd) to throw if the workflow cwd is not a git repo. Align behavior with createPullRequestHandler by returning a clear error (and guidance to use actions/checkout with path) when the target repo checkout cannot be found, rather than proceeding with repoCwd = null.

Suggested change
if (checkoutResult.success) {
repoCwd = checkoutResult.path;
entry.repo_cwd = repoCwd;
}
if (!checkoutResult.success) {
return {
content: [
{
type: "text",
text: JSON.stringify({
result: "error",
error: `Repository checkout not found for ${repoSlug}. Ensure the repository is checked out in this workflow using actions/checkout. If checking out multiple repositories, use the 'path' input so the checkout can be located.`,
}),
},
],
isError: true,
};
}
repoCwd = checkoutResult.path;
entry.repo_cwd = repoCwd;

Copilot uses AI. Check for mistakes.
// Build common options for both patch and bundle generation
const pushTransportOptions = { mode: "incremental" };
if (repoCwd) {
pushTransportOptions.cwd = repoCwd;
Copy link

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pushTransportOptions sets cwd for multi-repo, but it never sets repoSlug. Both generateGitPatch and generateGitBundle support options.repoSlug to disambiguate filenames in /tmp/gh-aw/ for multi-repo workflows; without it, two repos with the same branch name can overwrite each other’s patch/bundle files. Consider setting pushTransportOptions.repoSlug = repoResult.repo when operating in multi-repo mode (similar to createPullRequestHandler).

Suggested change
pushTransportOptions.cwd = repoCwd;
pushTransportOptions.cwd = repoCwd;
pushTransportOptions.repoSlug = repoResult.repo;

Copilot uses AI. Check for mistakes.
@github-actions
Copy link
Copy Markdown
Contributor

🧪 Test Quality Sentinel Report

Test Quality Score: 100/100

Excellent test quality

Metric Value
New/modified tests analyzed 1
✅ Design tests (behavioral contracts) 1 (100%)
⚠️ Implementation tests (low value) 0 (0%)
Tests with error/edge cases 1 (100%)
Duplicate test clusters 0
Test inflation detected No (ratio: 1.94:1 — 31 test lines / 16 production lines)
🚨 Coding-guideline violations None

Test Classification Details

Test File Classification Issues Detected
should detect branch from the checked out target repo when repo is provided actions/setup/js/safe_outputs_handlers.test.cjs ✅ Design None

Test Analysis

should detect branch from the checked out target repo when repo is provided

Classification: Design test — behavioral contract

What design invariant does this test enforce?
When a repo argument is provided to pushToPullRequestBranchHandler, the branch detection logic scopes its git operations to the checked-out target repository's working directory (not the workspace root). The test constructs a real local git repo, checks out a feature/test-change branch, then verifies that the handler detects feature/test-change — not whatever branch the workspace root is on.

What would break if deleted?
A regression in the repoCwd-scoped getCurrentBranch(repoCwd) call would go undetected. Specifically, if side-repo checkouts reverted to using the workspace root for branch detection, no test would catch it.

Strengths:

  • Uses real git operations via execSync — no mocking of internal business logic; only mockServer.debug (GitHub Actions runtime) is stubbed, which is an acceptable mock target.
  • Asserts on observable behavior: result.isError === true confirms the handler ran against the correct repo path.
  • Asserts on the debug log message "detecting actual working branch: feature/test-change" to verify the branch was resolved from the target repo's cwd.
  • Cleans up process.env.GITHUB_BASE_REF in a finally block — good hygiene.

Minor suggestion (non-blocking):
The two expect() calls lack descriptive failure messages. While not a hard guideline for JavaScript (unlike Go), adding .toBe(true, "handler should fail when no remote is reachable") would improve debuggability on future failures.


Language Support

Tests analyzed:

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

Verdict

Check passed. 0% of new tests are implementation tests (threshold: 30%). The single new test exercises the core behavioral change introduced in this PR — side-repo cwd scoping for branch detection — using real git operations.


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

🧪 Test quality analysis by Test Quality Sentinel · ● 443.6K ·

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Test Quality Sentinel: 100/100. Test quality is excellent — 0% of new tests are implementation tests (threshold: 30%). The single new test verifies the core behavioral contract of side-repo cwd scoping using real git operations.

@pelikhan
Copy link
Copy Markdown
Collaborator

@copilot review all comments

Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 22, 2026

@copilot review all comments

Addressed all actionable review comments in b7a93af: push_to_pull_request_branch now returns a clear error when an explicit repo checkout is missing, and it now sets repoSlug together with cwd in multi-repo mode. Added/updated tests to cover the missing-checkout behavior and verified targeted tests + validation.

@pelikhan
Copy link
Copy Markdown
Collaborator

@copilot add tests as you do a deep review

Copilot AI and others added 4 commits April 23, 2026 01:55
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/9cbfebfe-a9d7-42d9-9e47-accf291cc24b

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/9cbfebfe-a9d7-42d9-9e47-accf291cc24b

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/9cbfebfe-a9d7-42d9-9e47-accf291cc24b

Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Apr 23, 2026

@copilot add tests as you do a deep review

Implemented a deeper review with additional targeted tests in 9dde06d (plus supporting commits): added side-repo tracked/local commit setup, strengthened branch-detection assertions in target repo context, and verified repoSlug-disambiguated incremental patch filenames. Screenshot: N/A (no UI changes).

repoCwd = checkoutResult.path;
entry.repo_cwd = repoCwd;
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot log the select folder location

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the checkout-folder debug log in 2c81766 and covered it with a targeted test assertion. Screenshot: N/A (no UI changes).

Copilot AI requested a review from pelikhan April 23, 2026 02:31
@pelikhan pelikhan merged commit b730c14 into main Apr 23, 2026
@pelikhan pelikhan deleted the copilot/fix-push-to-pull-request-branch branch April 23, 2026 03:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

push_to_pull_request_branch does not support multi-repo (side-repo) checkout pattern

3 participants