Add gated merge-pull-request safe-output with policy-driven merge enforcement#27193
Open
Add gated merge-pull-request safe-output with policy-driven merge enforcement#27193
merge-pull-request safe-output with policy-driven merge enforcement#27193Conversation
…ding Agent-Logs-Url: https://github.com/github/gh-aw/sessions/31a07f1a-cfce-42d7-9fb2-5db24724d231 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/31a07f1a-cfce-42d7-9fb2-5db24724d231 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Copilot created this pull request from a session on behalf of
pelikhan
April 19, 2026 13:44
View session
pelikhan
reviewed
Apr 19, 2026
| */ | ||
| async function getPullRequestWithMergeability(githubClient, owner, repo, pullNumber) { | ||
| let pr = null; | ||
| for (let i = 0; i < 3; i++) { |
Contributor
Author
There was a problem hiding this comment.
Updated in 355be65: switched mergeability retrieval to withRetry(...) with an explicit mergeability-pending retry condition and improved fallback error chaining.
Collaborator
|
@copilot must enforce that the base branch is not a protected branch or the default branch |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/2b89cfe4-e6fd-4edf-ac73-d8ad50a84640 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/2b89cfe4-e6fd-4edf-ac73-d8ad50a84640 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Implemented in 355be65: added extensive logging throughout 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:
|
Collaborator
|
@copilot use withRetry for every graphql call |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/b1124194-bc78-4fea-a403-2bc920b62c05 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/b1124194-bc78-4fea-a403-2bc920b62c05 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Implemented in a194986: 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:
|
Collaborator
|
@copilot use w3c-spec-writer to update safe-outputs specification |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/88afb2b1-7be3-42a1-be22-0271d7ec255e Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Implemented in d4e226e: all GraphQL calls in 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:
|
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/32fbbe57-499c-444c-8898-4a778723de9f Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Implemented in d9ccab5: updated the W3C-style safe-outputs specification with a new 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:
|
Collaborator
|
@copilot add tests to enforce spec |
Contributor
🧪 Test Quality Sentinel ReportTest Quality Score: 85/100✅ Excellent test quality
Test Classification Details
Analysis Notes✅
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
✅ Test Quality Sentinel: 85/100. Test quality is acceptable — 0% of new tests are implementation tests (threshold: 30%). The documentation contract test is well-structured with proper build tags, descriptive assertion messages, and no mock libraries. Consider adding a unit test for parseMergePullRequestConfig in a follow-up to cover the parsing logic.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new policy-gated safe-outputs.merge-pull-request capability that can merge PRs only when repository/CI/review/label/branch/file gates pass, integrating with the existing safe-outputs compiler + runtime handler pipeline.
Changes:
- Introduces
merge_pull_requestas a new safe-output/tool type (Go config + JSON schemas + JS tool catalogs + TS types). - Adds compiler/runtime validation and wiring (permissions mapping, state mapping, handler registry/config emission).
- Implements the JS runtime handler with merge gating logic and refactors shared check-run selection into
check_runs_helpers.cjs.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/safe_outputs_validation_config.go | Adds runtime field/type validation config for merge_pull_request payloads. |
| pkg/workflow/safe_outputs_validation.go | Adds compiler-time validation for merge-pull-request glob fields. |
| pkg/workflow/safe_outputs_state.go | Maps MergePullRequest safe-output config into the internal state key mapping. |
| pkg/workflow/safe_outputs_specification_merge_pull_request_test.go | Adds a spec-enforcement unit test ensuring the merge type is documented with key statements. |
| pkg/workflow/safe_outputs_permissions.go | Adds required permissions computation for merge safe-output (contents:write, pull-requests:write) and key-to-config wiring. |
| pkg/workflow/safe_outputs_config.go | Wires merge-pull-request extraction into frontmatter parsing flow. |
| pkg/workflow/merge_pull_request.go | Introduces MergePullRequestConfig and compiler parsing for merge-pull-request configuration. |
| pkg/workflow/js/safe_outputs_tools.json | Adds the merge_pull_request tool definition to the workflow JS tool catalog. |
| pkg/workflow/compiler_types.go | Extends SafeOutputsConfig with MergePullRequest. |
| pkg/workflow/compiler_safe_outputs_handlers.go | Registers merge_pull_request in the handler registry and emits handler config. |
| pkg/workflow/compiler.go | Calls merge-pull-request config validation during workflow compilation. |
| pkg/parser/schemas/main_workflow_schema.json | Extends the main workflow schema with safe-outputs.merge-pull-request. |
| docs/src/content/docs/reference/safe-outputs-specification.md | Updates spec version/date and documents merge_pull_request semantics/permissions/history. |
| actions/setup/js/types/safe-outputs-config.d.ts | Adds TS type for merge-pull-request safe-output config. |
| actions/setup/js/safe_outputs_tools.json | Adds the merge_pull_request tool definition to the action setup tool catalog. |
| actions/setup/js/safe_output_handler_manager.cjs | Registers the JS handler module for merge_pull_request. |
| actions/setup/js/merge_pull_request.cjs | Implements merge gating + merge execution handler with retries and detailed logging. |
| actions/setup/js/check_skip_if_check_failing.cjs | Refactors check-run filtering to reuse the new helper. |
| actions/setup/js/check_runs_helpers.cjs | Adds shared helpers for selecting latest relevant check runs and computing failing checks. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 18/19 changed files
- Comments generated: 6
Comment on lines
+8
to
+15
| type MergePullRequestConfig struct { | ||
| BaseSafeOutputConfig `yaml:",inline"` | ||
| RequiredLabels []string `yaml:"required-labels,omitempty"` // Labels that must be present on the PR | ||
| AllowedLabels []string `yaml:"allowed-labels,omitempty"` // Glob patterns; at least one PR label must match when configured | ||
| AllowedBranches []string `yaml:"allowed-branches,omitempty"` // Glob patterns for source branch names | ||
| AllowedFiles []string `yaml:"allowed-files,omitempty"` // Glob patterns; all changed files must match when configured | ||
| ProtectedFiles []string `yaml:"protected-files,omitempty"` // Glob patterns; any match blocks merge | ||
| } |
There was a problem hiding this comment.
MergePullRequestConfig doesn’t embed SafeOutputTargetConfig, so users can’t configure target-repo / allowed-repos for cross-repository merges (despite docs/tooling describing cross-repo support). Consider embedding SafeOutputTargetConfig (and parsing it) to align with other PR-focused safe outputs (e.g., close-pull-request/update-pull-request).
| AddStringSlice("allowed_labels", c.AllowedLabels). | ||
| AddStringSlice("allowed_branches", c.AllowedBranches). | ||
| AddStringSlice("allowed_files", c.AllowedFiles). | ||
| AddStringSlice("protected_files", c.ProtectedFiles). |
There was a problem hiding this comment.
The merge_pull_request handler config builder doesn’t pass target-repo / allowed_repos through to the JS handler. As a result, resolveTargetRepoConfig(config) can’t enforce the intended cross-repo allowlist, and merges will always use env/context defaults. Wire c.TargetRepoSlug and c.AllowedRepos into this builder (and add the fields to the config struct if needed).
Suggested change
| AddStringSlice("protected_files", c.ProtectedFiles). | |
| AddStringSlice("protected_files", c.ProtectedFiles). | |
| AddIfNotEmpty("target-repo", c.TargetRepoSlug). | |
| AddStringSlice("allowed_repos", c.AllowedRepos). |
Comment on lines
+7019
to
+7091
| "merge-pull-request": { | ||
| "oneOf": [ | ||
| { | ||
| "type": "null", | ||
| "description": "Enable pull request merge with default policy configuration" | ||
| }, | ||
| { | ||
| "type": "object", | ||
| "description": "Configuration for controlled pull request merges. The merge is blocked unless all configured gates pass.", | ||
| "properties": { | ||
| "max": { | ||
| "description": "Maximum number of pull request merges to perform per run (default: 1). Supports integer or GitHub Actions expression (e.g. '${{ inputs.max }}').", | ||
| "oneOf": [ | ||
| { | ||
| "type": "integer", | ||
| "minimum": 1, | ||
| "maximum": 10, | ||
| "default": 1 | ||
| }, | ||
| { | ||
| "type": "string", | ||
| "pattern": "^\\$\\{\\{.*\\}\\}$", | ||
| "description": "GitHub Actions expression that resolves to an integer at runtime" | ||
| } | ||
| ] | ||
| }, | ||
| "required-labels": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "string" | ||
| }, | ||
| "description": "List of labels that must all be present on the pull request before merge is allowed." | ||
| }, | ||
| "allowed-labels": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "string" | ||
| }, | ||
| "description": "Glob patterns for pull request labels. At least one existing PR label must match one of these patterns when configured." | ||
| }, | ||
| "allowed-branches": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "string" | ||
| }, | ||
| "description": "Glob patterns for allowed source branch names (pull request head ref)." | ||
| }, | ||
| "allowed-files": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "string" | ||
| }, | ||
| "description": "Exclusive allowlist of file path glob patterns. When configured, every changed file in the pull request must match at least one pattern." | ||
| }, | ||
| "protected-files": { | ||
| "type": "array", | ||
| "items": { | ||
| "type": "string" | ||
| }, | ||
| "description": "Glob patterns for protected files. If any changed file matches, merge is blocked. This check takes precedence over allowed-files." | ||
| }, | ||
| "github-token": { | ||
| "$ref": "#/$defs/github_token", | ||
| "description": "GitHub token to use for this specific output type. Overrides global github-token if specified." | ||
| }, | ||
| "staged": { | ||
| "type": "boolean", | ||
| "description": "If true, evaluate merge gates and emit preview results without executing the merge API call.", | ||
| "examples": [true, false] | ||
| } | ||
| }, | ||
| "additionalProperties": false | ||
| } |
There was a problem hiding this comment.
Schema for safe-outputs.merge-pull-request is missing cross-repo fields (e.g., target-repo, allowed-repos) that are used by other safe-output types and are referenced by the spec/docs for this type. Either add these properties here (and implement them in the Go config + handler wiring) or update the documentation to remove cross-repo claims for merge-pull-request.
Comment on lines
+2816
to
+2817
| - `target-repo`: Cross-repository target | ||
| - `allowed-repos`: Cross-repository allowlist |
There was a problem hiding this comment.
The spec lists target-repo / allowed-repos as merge_pull_request configuration parameters, but the compiler schema and Go config for merge-pull-request currently don’t expose these fields. Please either implement/wire these options end-to-end or remove them from the spec to avoid documenting unsupported configuration.
Suggested change
| - `target-repo`: Cross-repository target | |
| - `allowed-repos`: Cross-repository allowlist |
|
|
||
| if err := validateSimpleGlobList("allowed-labels", c.AllowedLabels); err != nil { | ||
| return err | ||
| } |
There was a problem hiding this comment.
Compiler validation currently checks allowed-labels for empty entries but doesn’t validate required-labels. Since required labels are treated as literal label names, an empty/whitespace entry will silently make merges impossible to satisfy. Consider adding the same non-empty validation for required-labels.
Suggested change
| } | |
| } | |
| if err := validateSimpleGlobList("required-labels", c.RequiredLabels); err != nil { | |
| return err | |
| } |
| }; | ||
| } | ||
|
|
||
| module.exports = { main }; |
There was a problem hiding this comment.
This new handler introduces substantial merge gating logic (reviews, checks, labels, branch and file constraints, idempotency, staged mode), but there are no accompanying unit tests in actions/setup/js for merge_pull_request. Adding tests for a few key pass/fail scenarios (e.g., required labels missing, protected-files match, required checks failing, already-merged idempotency) would help prevent regressions.
Suggested change
| module.exports = { main }; | |
| module.exports = { | |
| main, | |
| __testables: { | |
| compilePathGlobs, | |
| listChangedFiles, | |
| resolveContextPullNumber, | |
| }, | |
| }; |
Generated by Design Decision Gate workflow to document the architectural decision to add merge-pull-request as a safe-output type with policy-driven gate enforcement integrated into the existing compiler+runtime model. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
Commit pushed:
|
Contributor
🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (343 new lines in AI has analyzed the PR diff and generated a draft ADR to help you get started: 📄 Draft ADR: The draft captures the following key decisions identified from the diff:
What to do next
Once an ADR is linked in the PR body, this gate will re-run and verify the implementation matches the decision. Why ADRs Matter
ADRs create a searchable, permanent record of why the codebase looks the way it does. Future contributors (and your future self) will thank you. 📋 Michael Nygard ADR Format ReferenceAn ADR must contain these four sections to be considered complete:
All ADRs are stored in
References: §24632957089
|
Collaborator
|
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/dfc96d3b-31d1-421b-8c18-997d46b780dc Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Implemented in b996f24: 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:
|
Collaborator
|
@copilot allowed labels do not support globs allowed-labels: ["release/", "automerge/"] |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/8e341bdf-224f-4ff1-b265-5f2cdc3f0355 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Collaborator
|
@copilot merge to default branch is not allowed and should be refused. Update specification and add tests. |
Collaborator
|
@copilot ensure temporary ids are handled. |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/8e341bdf-224f-4ff1-b265-5f2cdc3f0355 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Collaborator
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/8e341bdf-224f-4ff1-b265-5f2cdc3f0355 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Implemented in 80d176b (with core behavior change in f663f8b): 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:
|
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/619e2266-55dc-4e30-8ee9-dd5121c508a7 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/619e2266-55dc-4e30-8ee9-dd5121c508a7 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/619e2266-55dc-4e30-8ee9-dd5121c508a7 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Contributor
Author
Implemented in 47b7e59: updated the safe-outputs specification to explicitly state merge_pull_request refuses merges to the repository default branch, and added tests enforcing that requirement (spec enforcement plus runtime default-branch detection coverage). No UI changes (no screenshot applicable).
Implemented in 47b7e59 and 7f6709d: merge_pull_request now handles temporary IDs for pull_request_number via resolved temporary-id mappings, including validation/error handling, and includes dedicated unit tests for resolved and unresolved temporary IDs. No UI changes (no screenshot applicable). 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:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR introduces a new
safe-outputs.merge-pull-requestcapability that only merges PRs when repository, review, CI, branch, label, and file-scope gates pass. It extends the existing safe-output model (compiler + runtime) without introducing a separate execution path.Frontmatter + compiler integration
merge-pull-requesttoSafeOutputsConfigand extraction flow.required-labelsallowed-labelsallowed-branchesallowed-filesprotected-filesrequired-labelsandallowed-labels.Schema + tool contract updates
main_workflow_schema.jsonto supportsafe-outputs.merge-pull-request.merge_pull_requesttool schema to both safe-output tool catalogs:actions/setup/js/safe_outputs_tools.jsonpkg/workflow/js/safe_outputs_tools.jsonmerge_pull_requestpayload fields.allowed-labelssemantics in schema/docs to exact label names (not glob patterns).pull_request_numbersupport for temporary-ID based resolution in merge flow semantics.Runtime handler + gating logic
actions/setup/js/merge_pull_request.cjsand registered it in the safe-output handler manager.allowed-files/protected-filesfile gating (protected match blocks)withRetry(...)withRetry(...)allowed-labelsuses exact label matching (no glob matching).pull_request_numberusing resolved safe-output temporary-ID mappings, with explicit unresolved-ID error paths.Shared logic reuse
check_runs_helpers.cjs.check_skip_if_check_failing.cjsto consume the shared helper instead of duplicating logic.Permissions + state plumbing
contents:write+pull-requests:writepath).Specification updates
docs/src/content/docs/reference/safe-outputs-specification.md) to include a formalmerge_pull_requesttype section.allowed-labelsdocumentation to exact-match semantics.pull_request_number.Spec enforcement tests
pkg/workflow/safe_outputs_specification_merge_pull_request_test.goto enforce that the spec includes themerge_pull_requesttype section and required policy/permission statements.Runtime tests
actions/setup/js/merge_pull_request.test.cjswith targeted coverage for branch sanitization, protected base-branch policy detection, unsafe base-branch rejection behavior, and exact allowed-label matching behavior (including edge cases).pull_request_number(resolved and unresolved cases).Compiler validation tests
pkg/workflow/safe_outputs_validation_merge_pull_request_test.goto enforce non-empty validation behavior forrequired-labelsandallowed-labels.