Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/e9136023-f723-4e26-a90b-a615b40f6d6f Co-authored-by: mrjf <180956+mrjf@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Automatically switches push_to_pull_request_branch from patch transport to bundle transport when the incremental range contains merge commits, to avoid git am --3way failures on long-running PR branches.
Changes:
- Added merge-commit detection in
pushToPullRequestBranchHandlerand auto-fallback to bundle unlesspatch_formatwas explicitly set. - Introduced
hasMergeCommitsInRange()ingit_helpers.cjsusinggit rev-list --merges --count. - Added tests covering auto-switch behavior and explicit
patch_format: am, plus a changeset entry documenting the behavior.
Show a summary per file
| File | Description |
|---|---|
actions/setup/js/safe_outputs_handlers.test.cjs |
Adds regression tests for merge-commit auto-switching and explicit am override behavior. |
actions/setup/js/safe_outputs_handlers.cjs |
Implements merge-commit detection and conditional auto-switch from patch to bundle transport. |
actions/setup/js/git_helpers.cjs |
Adds a helper to detect whether a ref range contains merge commits. |
.changeset/patch-auto-bundle-on-merge-commits.md |
Documents the new auto-fallback behavior for release notes. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (2)
actions/setup/js/safe_outputs_handlers.cjs:591
- Switching to
bundlehere bypasses theexcluded_filesfiltering that this handler only applies on the patch path (pushPatchOptions.excludedFiles).generateGitBundlehas no equivalent exclusion support, so a branch with merge commits can end up pushing files that the workflow explicitly configured to omit. The auto-switch should be disabled whenexcluded_filesis set, or bundle generation needs matching exclusion logic first.
if (!useBundle && !patchFormatExplicit && entry.branch) {
const hasMerges = hasMergeCommitsInRange(`refs/remotes/origin/${entry.branch}`, entry.branch, { cwd: repoCwd || undefined });
if (hasMerges) {
server.debug(`push_to_pull_request_branch: detected merge commit(s) in incremental range origin/${entry.branch}..${entry.branch}; auto-switching to bundle transport (set patch-format: am to override).`);
useBundle = true;
actions/setup/js/safe_outputs_handlers.cjs:591
- Auto-switching to bundle here also skips the protected-file / allowed-files enforcement in the actual push step. That code inspects
patchContent, but bundle pushes have no patch content, socheckFileProtection("")treats the change as touching no files (actions/setup/js/push_to_pull_request_branch.cjs:247-265andactions/setup/js/manifest_file_helpers.cjs:202-229). As a result, any branch with a merge commit can bypass those safeguards entirely once this fallback fires.
if (!useBundle && !patchFormatExplicit && entry.branch) {
const hasMerges = hasMergeCommitsInRange(`refs/remotes/origin/${entry.branch}`, entry.branch, { cwd: repoCwd || undefined });
if (hasMerges) {
server.debug(`push_to_pull_request_branch: detected merge commit(s) in incremental range origin/${entry.branch}..${entry.branch}; auto-switching to bundle transport (set patch-format: am to override).`);
useBundle = true;
- Files reviewed: 4/4 changed files
- Comments generated: 1
| if (!useBundle && !patchFormatExplicit && entry.branch) { | ||
| const hasMerges = hasMergeCommitsInRange(`refs/remotes/origin/${entry.branch}`, entry.branch, { cwd: repoCwd || undefined }); | ||
| if (hasMerges) { | ||
| server.debug(`push_to_pull_request_branch: detected merge commit(s) in incremental range origin/${entry.branch}..${entry.branch}; auto-switching to bundle transport (set patch-format: am to override).`); | ||
| useBundle = true; |
|
@copilot review all comments |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/89949bd9-a843-4d2e-b39f-771b1552fd9c Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Addressed in 63532bd: the bundle code path now computes the incremental net |
Long-running PR branches that merge their base branch locally produce patches
git am --3waycannot apply, blocking every subsequent push (e.g. autoloop ontsessebe). Thebundletransport already handles this but is opt-in.Changes
safe_outputs_handlers.cjs— InpushToPullRequestBranchHandler, whenpatch-formatis not explicitly configured andorigin/<branch>..<branch>contains ≥1 merge commit, flipuseBundle = true. Explicitpatch-format: amis still honored. The bundle code path now also computes the incremental netdiff_sizeviacomputeIncrementalDiffSize(baseCommit..branch)and records it on the safe-output entry, so the downstream push step validatesmax_patch_sizeagainst the actual net change rather than the on-disk bundle artifact (which on long-running branches accumulates packed git objects and per-commit metadata, and can exceed the limit even when each iteration only changes a few KB).git_helpers.cjs— NewhasMergeCommitsInRange(baseRef, headRef, { cwd })wrappinggit rev-list --merges --count. Best-effort: returnsfalseon any git error so detection failures fall through to existing behavior. Uses local refs only (no extra fetch).safe_outputs_handlers.test.cjs: (a) merge-commit scenario produces a bundle, emits the auto-switch debug line, and records a non-negativediff_sizeon the entry; (b) explicitpatch_format: amis respected even with merges in range.diff_sizerecording for bundle transport.Detection logic
Notes for review
push_to_pull_request_branch(incremental).create_pull_request(full mode) is not auto-switched here — that path rarely produces merge commits since the branch is typically new, and the range computation is more involved. Can extend if needed.refs/remotes/origin/<branch>..<branch>against the local checkout, identical to what the patch generator uses in incremental mode.amvia explicitpatch-format: amif they want the prior failure mode (e.g. to surface a configuration mistake loudly rather than silently switching transports).diff_sizerecording for bundle transport applies whether the bundle was selected explicitly viapatch-format: bundleor auto-selected by the new merge-commit detection — so existing bundle users also benefit from accuratemax_patch_sizevalidation.