Add fallback PR flow for diverged push-to-pull-request-branch and make it opt-out#27220
Add fallback PR flow for diverged push-to-pull-request-branch and make it opt-out#27220
push-to-pull-request-branch and make it opt-out#27220Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/267bf5c1-a299-43c5-be20-17cdb0ab2a0c Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/267bf5c1-a299-43c5-be20-17cdb0ab2a0c Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/37d80ed2-4179-4462-ac6e-bbe23dc36ad5 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…orward failures Agent-Logs-Url: https://github.com/github/gh-aw/sessions/9f0ad012-0f23-4022-ad35-c955d90fc310 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/9f0ad012-0f23-4022-ad35-c955d90fc310 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a safer runtime fallback for push-to-pull-request-branch when pushes fail due to diverged/non-fast-forward branches, defaulting to opening a fallback PR (opt-out via config), and updates permissions + docs + UI surfaces to reflect the new behavior.
Changes:
- Add
fallback-as-pull-requestconfig (defaulttrue) and incorporate it into permission computation forpush-to-pull-request-branch. - Implement non-fast-forward push fallback in the JS handler by pushing a new fallback branch and opening a PR targeting the original PR branch.
- Update schema/docs and safe-output summaries/comments to distinguish fallback-to-issue vs fallback-to-pull-request.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/safe_outputs_permissions_test.go | Updates/extends permission expectation tests for new fallback flag behavior. |
| pkg/workflow/safe_outputs_permissions.go | Gates pull-requests: write requirement on fallback-as-pull-request for push-to-PR-branch. |
| pkg/workflow/push_to_pull_request_branch_test.go | Adds compilation test to ensure disabling fallback avoids requesting pull-requests: write. |
| pkg/workflow/push_to_pull_request_branch.go | Adds FallbackAsPullRequest field and parses fallback-as-pull-request from frontmatter. |
| pkg/parser/schemas/main_workflow_schema.json | Documents and defaults the new fallback-as-pull-request boolean in schema. |
| docs/src/content/docs/reference/safe-outputs-pull-requests.md | Documents default fallback PR behavior and opt-out semantics. |
| docs/src/content/docs/reference/frontmatter-full.md | Adds full frontmatter reference entry for fallback-as-pull-request. |
| actions/setup/js/types/handler-factory.d.ts | Adds fallback_as_pull_request to handler config typing. |
| actions/setup/js/safe_output_summary.test.cjs | Adds test coverage for fallback PR summary rendering. |
| actions/setup/js/safe_output_summary.cjs | Adds logic to display fallback PR vs fallback issue messaging in summaries. |
| actions/setup/js/safe_output_handler_manager.test.cjs | Adds test for fallback PR annotation prepended to subsequent comments. |
| actions/setup/js/safe_output_handler_manager.cjs | Tracks fallback target type (issue vs PR) and updates add_comment annotation text accordingly. |
| actions/setup/js/push_to_pull_request_branch.test.cjs | Adds tests for default fallback PR creation and opt-out behavior. |
| actions/setup/js/push_to_pull_request_branch.cjs | Implements non-fast-forward fallback PR creation and returns structured fallback metadata. |
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_output_summary.cjs:46
- The generated summary section title is hard-coded to "Fallback Issue" whenever
fallback_usedis true, even when the fallback target is a pull request. This makes the step summary misleading for the new non-fast-forward fallback PR flow. Please compute the section title based on the effective fallback target type (issue vs pull request).
// Add message details
const sectionTitle = isFallback ? `### ${displayType} — Fallback Issue\n\n` : `### ${displayType}\n\n`;
summary += sectionTitle;
- Files reviewed: 13/14 changed files
- Comments generated: 3
| // Detect fallback outcomes for code-push types | ||
| const isFallback = success && result && result.fallback_used === true; | ||
| const fallbackType = isFallback && (result.pull_request_url || result.pull_request_number != null) ? "pull_request" : "issue"; | ||
|
|
||
| // Choose emoji and status based on success and fallback | ||
| const emoji = isFallback ? "⚠️" : success ? "✅" : "❌"; | ||
| const status = isFallback ? "Fallback Issue Created" : success ? "Success" : "Failed"; | ||
| const status = isFallback ? (fallbackType === "pull_request" ? "Fallback Pull Request Created" : "Fallback Issue Created") : success ? "Success" : "Failed"; |
There was a problem hiding this comment.
fallbackType is inferred from the presence of pull_request_url/pull_request_number, but the handler now returns an explicit fallback_type field. Relying on inference can misclassify if the result shape changes (e.g., only number present) and also diverges from the structured metadata contract described in the PR. Prefer result.fallback_type when present, and fall back to inference only for backward compatibility with older issue-fallback results.
This issue also appears on line 44 of the same file.
| // Track when a code-push operation falls back to creating a review issue instead. | ||
| // When set, subsequent add_comment messages will receive a correction note prepended | ||
| // to their body so the posted comment accurately reflects the actual outcome. |
There was a problem hiding this comment.
The comment above codePushFallbackInfo still says the fallback is always a review issue, but the code now also tracks fallback pull requests. Please update the comment to reflect both fallback target types so it stays accurate for future maintenance.
| // Track when a code-push operation falls back to creating a review issue instead. | |
| // When set, subsequent add_comment messages will receive a correction note prepended | |
| // to their body so the posted comment accurately reflects the actual outcome. | |
| // Track when a code-push operation falls back to creating an issue or pull request instead. | |
| // When set, subsequent add_comment messages will receive a correction note prepended | |
| // to their body so the posted comment accurately reflects the actual fallback target. |
| AllowedFiles []string `yaml:"allowed-files,omitempty"` // Strict allowlist of glob patterns for files eligible for push. Checked independently of protected-files; both checks must pass. | ||
| ExcludedFiles []string `yaml:"excluded-files,omitempty"` // List of glob patterns for files to exclude from the patch using git :(exclude) pathspecs. Matching files are stripped by git at generation time and will not appear in the commit or be subject to allowed-files or protected-files checks. | ||
| PatchFormat string `yaml:"patch-format,omitempty"` // Transport format for packaging changes: "am" (default, uses git format-patch) or "bundle" (uses git bundle, preserves merge topology and per-commit metadata). | ||
| FallbackAsPullRequest *bool `yaml:"fallback-as-pull-request,omitempty"` // When true (default), creates a fallback pull request if direct push fails due to diverged/non-fast-forward branch. When false, fallback is disabled and pull-requests: write is not requested. | ||
| AllowWorkflows bool `yaml:"allow-workflows,omitempty"` // When true, adds workflows: write to the GitHub App token. Requires safe-outputs.github-app to be configured. | ||
| } |
There was a problem hiding this comment.
fallback-as-pull-request is parsed into PushToPullRequestBranchConfig, but it is not currently propagated into the generated handler config for push_to_pull_request_branch (see pkg/workflow/compiler_safe_outputs_handlers.go where only existing fields are emitted). As a result, setting this flag to false will reduce requested permissions (no pull-requests: write) but the JS handler will still default fallback_as_pull_request to true and attempt to create a fallback PR, likely failing at runtime due to missing permissions. Please add this field to the handler config JSON (e.g., fallback_as_pull_request) and add/adjust a compilation test to assert it is present when set to false.
See below for a potential fix:
ManifestFilesPolicy *string `yaml:"protected-files,omitempty"` // Controls protected-file protection: "blocked" (default) hard-blocks, "allowed" permits all changes, "fallback-to-issue" creates a review issue instead of pushing.
ProtectedFilesExclude []string `yaml:"-"` // Files/prefixes to exclude from the default protected list (from object-form protected-files.exclude). Not sourced from YAML directly; populated during parsing.
AllowedFiles []string `yaml:"allowed-files,omitempty"` // Strict allowlist of glob patterns for files eligible for push. Checked independently of protected-files; both checks must pass.
ExcludedFiles []string `yaml:"excluded-files,omitempty"` // List of glob patterns for files to exclude from the patch using git :(exclude) pathspecs. Matching files are stripped by git at generation time and will not appear in the commit or be subject to allowed-files or protected-files checks.
PatchFormat string `yaml:"patch-format,omitempty"` // Transport format for packaging changes: "am" (default, uses git format-patch) or "bundle" (uses git bundle, preserves merge topology and per-commit metadata).
FallbackAsPullRequest *bool `yaml:"fallback-as-pull-request,omitempty" json:"fallback_as_pull_request,omitempty"` // When true (default), creates a fallback pull request if direct push fails due to diverged/non-fast-forward branch. When false, fallback is disabled and pull-requests: write is not requested.
AllowWorkflows bool `yaml:"allow-workflows,omitempty"` // When true, adds workflows: write to the GitHub App token. Requires safe-outputs.github-app to be configured.
|
@copilot review all comments |
…propagation Agent-Logs-Url: https://github.com/github/gh-aw/sessions/f35b039c-a302-4ce9-a90d-a41ec5468b27 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/f35b039c-a302-4ce9-a90d-a41ec5468b27 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Reviewed all comments and pushed fixes in 1983795 and e1ce37a. Addressed: (1) explicit fallback_type precedence in safe_output_summary, (2) stale fallback comments in safe_output_handler_manager, and (3) propagation of fallback-as-pull-request into compiled push_to_pull_request_branch handler config with test coverage. 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 keep working |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/892e3d78-093f-4f10-9bd4-3d91ae48774e Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/892e3d78-093f-4f10-9bd4-3d91ae48774e Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/892e3d78-093f-4f10-9bd4-3d91ae48774e Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Implemented additional follow-up changes in a766692, 3bd003c, and 6db0ad6: hardened handler-config assertions by parsing JSON from the compiled lock workflow, added explicit fallback_as_pull_request=true coverage, and polished naming/guard logic per review feedback. UI changes: none (screenshot N/A). 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 |
Reviewed all comments again and confirmed the follow-ups are already in the branch: explicit fallback_type precedence/guard, updated fallback target comment text, and fallback_as_pull_request propagation + compile test coverage (including true/false assertions) in commits a766692, 3bd003c, and 6db0ad6. UI changes: none (screenshot N/A). |
|
@copilot merge main and recompile |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Merged origin/main, resolved conflicts, and recompiled workflows in 733f53a. Also ran targeted push-to-pull-request-branch workflow tests after the merge. UI changes: none (screenshot N/A). 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:
|
When
push-to-pull-request-branchhit non-fast-forward/diverged branch errors, the safe-output pipeline hard-failed and cancelled downstream outputs. This change adds a fallback path that opens a new PR against the original PR branch, and introduces a config switch to disable that behavior.Runtime fallback behavior
push_to_pull_request_branchnow detects non-fast-forward push failures and, by default, creates a fallback PR (head: <fallback-branch>,base: <original-pr-branch>).fallback_used,fallback_type: "pull_request", fallback PR number/url) for downstream handling.Config + permission model
safe-outputs.push-to-pull-request-branch.fallback-as-pull-request(defaulttrue).pull-requests: writeon that flag:true(default): requirespull-requests: write.false: no fallback PR creation; nopull-requests: writerequirement for this handler.Safe-output footer/comment/summary updates
Schema + docs updates
fallback-as-pull-requestto workflow schema and frontmatter reference.