-
Notifications
You must be signed in to change notification settings - Fork 395
Support allowed-branches enforcement for create-pull-request safe output
#33610
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
dsyme
merged 11 commits into
main
from
copilot/support-allowed-branches-create-pull-request
May 20, 2026
Merged
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
8019020
Initial plan
Copilot 6aff5dc
Support create-pull-request allowed-branches
Copilot bf173cf
Add allowed-branches to schema and references
Copilot adbe8a9
Document allowed-branches constraint in PR tool schema
Copilot bfc98bd
Merge branch 'main' of https://github.com/github/gh-aw into copilot/s…
dsyme 84a22b0
fix tests
dsyme b9a5500
Merge branch 'main' into copilot/support-allowed-branches-create-pull…
dsyme e19809a
code review
dsyme f52dfaf
docs(adr): add draft ADR-33610 for allowed-branches enforcement
github-actions[bot] a0d9014
Merge branch 'copilot/support-allowed-branches-create-pull-request' o…
dsyme e44c718
code review
dsyme File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
89 changes: 89 additions & 0 deletions
89
docs/adr/33610-allowed-branches-enforcement-for-create-pull-request.md
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| # ADR-33610: Collector-Time `allowed-branches` Enforcement for `create-pull-request` Safe Output | ||
|
|
||
| **Date**: 2026-05-20 | ||
| **Status**: Draft | ||
| **Deciders**: Unknown (draft generated from PR #33610 diff) | ||
|
|
||
| --- | ||
|
|
||
| ## Part 1 — Narrative (Human-Friendly) | ||
|
|
||
| ### Context | ||
|
|
||
| The `create-pull-request` safe output already supports `allowed-base-branches` so authors can restrict which base branches an agent may target at runtime, but there is no symmetric control over the **source** branch the PR is opened from. Some repositories enforce source-branch naming conventions (e.g. `feature/*`, `release/*`) and want those conventions to be enforced even when an agent generates the PR. Branch policy violations should also surface as soon as possible — ideally when the agent records the safe output through the MCP tool — so the agent gets actionable, immediate feedback rather than a late failure during the apply step. | ||
|
|
||
| ### Decision | ||
|
|
||
| We will add an `allowed-branches` field to `safe-outputs.create-pull-request` that accepts an array of glob patterns (or a GitHub Actions expression resolving to a comma-separated list, consistent with `allowed-base-branches`). We will enforce the policy **at safe-output collection time** inside `safe_outputs_handlers.cjs`: when the agent emits a `create_pull_request` intent, the effective branch (agent-provided `branch`, or the resolved current checkout branch when omitted) MUST match one of the configured patterns, otherwise the handler returns an MCP error and the safe output is not recorded. The compiler (`pkg/workflow`) is extended to plumb the field as `allowed_branches` into the runtime handler config and to treat it as a templatable expression-array field alongside `labels`, `allowed-repos`, and `allowed-base-branches`. | ||
|
|
||
| ### Alternatives Considered | ||
|
|
||
| #### Alternative 1: Enforce only at apply-time in the PR-creation workflow step | ||
|
|
||
| The branch policy could be checked only in the post-agent step that actually opens the PR. This was rejected because the agent would not see the rejection until much later in the pipeline (after the entire turn has completed), which wastes work and obscures the cause. Collector-time enforcement returns a structured MCP error the agent can react to inside the same turn. | ||
|
|
||
| #### Alternative 2: Reuse `allowed-base-branches` for source branches | ||
|
|
||
| We considered overloading the existing `allowed-base-branches` field with a separate boolean to also constrain source branches. This was rejected because base and source branches have different semantics (target vs origin) and conflating them would make the config harder to reason about and hide what is being constrained. | ||
|
|
||
| #### Alternative 3: Use regex patterns instead of glob patterns | ||
|
|
||
| A regex field would be more expressive. This was rejected for consistency: every other branch/path constraint in safe-outputs (`allowed-base-branches`, `allowed-files`, `excluded-files`, `protected-files`) uses glob syntax via the existing `globPatternToRegex` helper. A regex outlier would force workflow authors to learn two pattern dialects. | ||
|
|
||
| ### Consequences | ||
|
|
||
| #### Positive | ||
| - Repository-level source-branch naming conventions can be enforced by workflow configuration rather than by relying on the agent's prompt obedience. | ||
| - Symmetric with `allowed-base-branches`, so the mental model is "two parallel branch policies (source + base)" and authors do not have to learn a new shape. | ||
| - Errors are surfaced inside the agent's turn (as an MCP error containing the configured patterns), enabling self-correction without a full pipeline failure. | ||
| - Expression-array support means the allow-list can be parameterized by `workflow_call` inputs (e.g. `${{ inputs['allowed-branches'] }}`). | ||
|
|
||
| #### Negative | ||
| - Workflow authors now have two distinct branch policy fields (`allowed-branches` and `allowed-base-branches`) and must understand the difference. Confusing the two would silently fail to constrain the intended branch. | ||
| - The runtime copy of `safe_outputs_handlers.cjs` and the compiler config plumbing must stay in sync; drift between the two would result in collector-time enforcement silently disappearing. This is partially mitigated by the new `TestSafeOutputsToolsJSONInSync` test, but the handler code itself is not similarly guarded. | ||
| - Collector-time enforcement runs in the agent's environment using its inferred branch; an agent that omits `branch` and runs from an unexpected checkout could still pass the policy if that checkout happens to match — the policy is only as strong as the branch resolution it sees. | ||
|
|
||
| #### Neutral | ||
| - Two copies of `safe_outputs_tools.json` (`actions/setup/js/` and `pkg/workflow/js/`) must be updated together when the tool description changes; a new test enforces tool-name parity between them. | ||
| - The `branch` tool-input description now references the workflow configuration, so generated MCP tool schemas vary slightly depending on which safe-output features are enabled. | ||
|
|
||
| --- | ||
|
|
||
| ## Part 2 — Normative Specification (RFC 2119) | ||
|
|
||
| > The key words **MUST**, **MUST NOT**, **REQUIRED**, **SHALL**, **SHALL NOT**, **SHOULD**, **SHOULD NOT**, **RECOMMENDED**, **MAY**, and **OPTIONAL** in this section are to be interpreted as described in [RFC 2119](https://www.rfc-editor.org/rfc/rfc2119). | ||
|
|
||
| ### Configuration Surface | ||
|
|
||
| 1. The `safe-outputs.create-pull-request` config **MUST** accept an optional `allowed-branches` field. | ||
| 2. The `allowed-branches` field **MUST** accept either (a) an array of strings, or (b) a single GitHub Actions expression string of the form `${{ ... }}` that resolves to a comma-separated list. | ||
| 3. Implementations **MUST** treat each entry as a glob pattern; bare strings without `*` **MUST** be matched as exact branch names. | ||
| 4. Implementations **MUST NOT** reject configurations where `allowed-branches` is absent or empty — in that case no branch policy is applied. | ||
|
|
||
| ### Collector-Time Enforcement | ||
|
|
||
| 1. When `allowed-branches` is non-empty, the `create_pull_request` MCP handler **MUST** resolve the effective branch from (in order): the agent-supplied `branch` field if present and not equal to the configured base, otherwise the branch detected from the current checkout (`GITHUB_HEAD_REF` / `GITHUB_REF_NAME`). | ||
| 2. The handler **MUST** match the resolved branch against the configured patterns using the shared `globPatternToRegex` helper in `pathMode: true, caseSensitive: true` mode. | ||
| 3. If the resolved branch does not match any configured pattern, the handler **MUST** return a structured MCP error with `isError: true`, `result: "error"`, and an `error` message that includes the rejected branch and the configured patterns. | ||
| 4. On rejection, the handler **MUST NOT** call `appendSafeOutput` — the PR intent **MUST NOT** be recorded. | ||
| 5. The handler **MUST** apply `allowed-branches` enforcement before any intent-probe validation (`validateCreatePullRequestIntent`), so policy violations short-circuit cheaper-to-detect errors. | ||
|
|
||
| ### Compiler Plumbing | ||
|
|
||
| 1. `CreatePullRequestsConfig` **MUST** expose `AllowedBranches []string` with YAML tag `allowed-branches,omitempty`. | ||
| 2. The list of templatable expression-array fields for `create-pull-request` **MUST** include `allowed-branches` alongside `labels`, `allowed-repos`, and `allowed-base-branches`. | ||
| 3. The handler config builder **MUST** emit `allowed_branches` via `AddTemplatableStringSlice` when `AllowedBranches` is non-empty, and **MUST NOT** emit the key when it is empty. | ||
| 4. The JSON-schema in `pkg/parser/schemas/main_workflow_schema.json` **MUST** describe `allowed-branches` with a `oneOf` of either an array of strings or an expression string matching `^\$\{\{.*\}\}$`. | ||
|
|
||
| ### Tooling Description Parity | ||
|
|
||
| 1. The `branch` parameter description in both `actions/setup/js/safe_outputs_tools.json` and `pkg/workflow/js/safe_outputs_tools.json` **MUST** state that the branch must be provided and match `allowed-branches` when the workflow configures that field. | ||
| 2. The compiler-embedded and runtime copies of `safe_outputs_tools.json` **MUST** declare the same tool names in the same order, as verified by `TestSafeOutputsToolsJSONInSync`. | ||
|
|
||
| ### Conformance | ||
|
|
||
| An implementation is considered conformant with this ADR if it satisfies every **MUST** and **MUST NOT** requirement above. In particular, an implementation that accepts the `allowed-branches` field but does not reject non-matching branches at MCP collection time is **non-conformant**. | ||
|
|
||
| --- | ||
|
|
||
| *This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/26187066394) workflow. The PR author must review, complete (especially the Deciders field), and finalize this document before the PR can merge.* |
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
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.