Make bundle mode the default for safe-output patch packaging#30626
Make bundle mode the default for safe-output patch packaging#30626
Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/51d88a30-2bd7-4a03-a6e1-c913384c7013 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/51d88a30-2bd7-4a03-a6e1-c913384c7013 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This pull request switches the default safe-outputs transport for packaging changes from am (format-patch) to bundle (git bundle) for create_pull_request and push_to_pull_request_branch, updating schema/docs and adjusting handler tests to match the new default.
Changes:
- Change JS safe-outputs handlers to default
patch_formatto"bundle"for both create PR and push-to-PR-branch flows. - Update workflow compiler comments plus JSON schema defaults/descriptions to reflect
"bundle"as the default. - Update reference docs/frontmatter examples and JS tests to align with the new default behavior.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/push_to_pull_request_branch.go | Updates inline documentation/comments to reflect bundle as default patch transport. |
| pkg/workflow/create_pull_request.go | Updates inline documentation/comments to reflect bundle as default patch transport. |
| pkg/parser/schemas/main_workflow_schema.json | Changes schema defaults/descriptions for patch-format to default to bundle. |
| docs/src/content/docs/reference/safe-outputs-pull-requests.md | Updates reference docs default/description for patch-format. |
| docs/src/content/docs/reference/frontmatter-full.md | Updates reference examples and wording to show bundle as the default. |
| actions/setup/js/safe_outputs_handlers.test.cjs | Updates tests to expect bundle as the default transport and bundle-shaped responses. |
| actions/setup/js/safe_outputs_handlers.cjs | Switches default patch_format from am to bundle for both handlers. |
| actions/setup/js/push_to_pull_request_branch.cjs | Updates a comment to reflect am is now non-default apply path. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
actions/setup/js/safe_outputs_handlers.cjs:573
- push_to_pull_request_branch now defaults patch_format to "bundle", but the downstream push/apply logic’s file-safety checks are currently patch-text based. With bundle transport, patchContent is empty while the operation is treated as non-empty, so allowed-files/protected-files enforcement (and excluded-files behavior) may not run as intended. Consider adding bundle-aware validation (e.g., derive changed paths/diff after fetching the bundle to a temp ref) or rejecting bundle when protections/exclusions cannot be enforced equivalently.
// Determine transport format: "bundle" (default) uses git bundle (preserves merge topology),
// "am" uses git format-patch / git am (good for linear histories).
// Use ?? (nullish coalescing) so an empty-string resolved value is preserved and
// rejected below rather than silently falling back to "bundle".
// Track whether the user explicitly set patch_format so we can auto-fall-back
// to bundle transport when merge commits are detected (since `git am` cannot
// apply merge commits). When the user explicitly chose a format, respect it.
const patchFormatExplicit = pushConfig["patch_format"] !== undefined || config["patch_format"] !== undefined;
const pushPatchFormat = pushConfig["patch_format"] ?? config["patch_format"] ?? "bundle";
const validPushPatchFormats = ["am", "bundle"];
- Files reviewed: 8/8 changed files
- Comments generated: 1
| // Determine transport format: "bundle" (default) uses git bundle (preserves merge topology), | ||
| // "am" uses git format-patch / git am (good for linear histories). | ||
| // Use ?? (nullish coalescing) so an empty-string resolved value is preserved and | ||
| // rejected below rather than silently falling back to "am". | ||
| const patchFormat = prConfig["patch_format"] ?? config["patch_format"] ?? "am"; | ||
| // rejected below rather than silently falling back to "bundle". | ||
| const patchFormat = prConfig["patch_format"] ?? config["patch_format"] ?? "bundle"; | ||
| const validPatchFormats = ["am", "bundle"]; |
There was a problem hiding this comment.
@copilot always generate a patch so that you can apply the allowed-files/protected-files protection, but use the bundle for applying the changes.
There was a problem hiding this comment.
Implemented in 68f366d (plus clarification follow-up 5596dbb): bundle mode now always generates and records a patch (patch_path) for allowed-files/protected-files/excluded-files enforcement, while bundle remains the apply-time transport. Also updated apply-time handlers/tests to require and validate patch content even when bundle_path is used. No UI changes in this PR, so no screenshot applies.
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/9e569345-e6bd-4e8d-aa51-17398e0b2006 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/9e569345-e6bd-4e8d-aa51-17398e0b2006 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
@copilot review all comments |
Summary
amtobundlefor:create_pull_requestpush_to_pull_request_branchpatch-format: ambehavior supportedallowed-files/protected-files/excluded-filesenforcement continues to run on patch contentValidation
make build && make fmtnpm run test:js -- safe_outputs_handlers.test.cjs push_to_pull_request_branch.test.cjs create_pull_request.test.cjs(fromactions/setup/js)make agent-report-progresssecret_scanningon changed filesparallel_validationrun completed with: