Add pre-created pull request safe output mode - #53576
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ PR Code Quality Reviewer completed the code quality review.
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check.
|
|
✅ Ponytail Reviewer completed successfully!
|
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff
|
Comment MemoryNote This comment is managed by comment memory.It stores persistent context for this thread in the code block at the top of this comment.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Request changes
The pre-created PR flow has a couple of correctness holes that can produce the wrong branch ancestry and let the reused placeholder drift out of the requested draft state.
Blocking themes
- The activation step creates the placeholder branch from the activation checkout head, not from the eventual PR base branch, so runs targeting a different base can pre-create a PR with the wrong merge base.
- Reusing an allocated PR updates title/body/base only; it does not enforce the configured draft state, so a previously readied PR can stay non-draft across reruns.
🔎 Code quality review by PR Code Quality Reviewer · gpt54 · 14.2 AIC · ⌖ 7.09 AIC · ⊞ 7K
Comment /review to run again
| const workflowName = process.env.GH_AW_WORKFLOW_NAME || context.workflow || "Agentic workflow"; | ||
| const runUrl = `${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/actions/runs/${context.runId}`; | ||
| const branch = `gh-aw/pre-created/${context.runId}-${process.env.GITHUB_RUN_ATTEMPT || "1"}`; | ||
| const { stdout: checkoutHead } = await exec.getExecOutput("git", ["rev-parse", "HEAD"]); |
There was a problem hiding this comment.
This creates the placeholder branch and PR from whatever commit happened to be checked out in the activation job, but the activation checkout still uses the event default ref instead of the resolved PR base branch. On issue_comment/manual runs that target a different base branch, the pre-created branch will be forked from the wrong commit, so later updates either produce a nonsense diff or can’t be rebased onto the requested base cleanly.
💡 Why this blocks merge
The new mode promises that the eventual PR reuses the pre-created branch, so its starting point has to match the branch the real PR is targeting. Here you derive `headSha` from `git rev-parse HEAD` and create the synthetic commit/ref from that checkout, but nothing in this step aligns the activation checkout to `create-pull-request.base-branch` or the runtime-selected base. That means a workflow configured to open against `release/*` can pre-create a PR from `main`, and every subsequent push will inherit the wrong merge base.At minimum, pre-creation needs to resolve the effective base branch first and create the branch from that ref, or reject configurations where the base can differ from the activation checkout.
| if (existingPullRequest.state !== "open" || existingPullRequest.head.ref !== preCreatedBranch) { | ||
| throw new Error(`Pre-created pull request #${preCreatedPullRequestNumber} is not open on branch ${preCreatedBranch}`); | ||
| } | ||
| return githubClient.rest.pulls.update({ |
There was a problem hiding this comment.
If the pre-created PR is already marked ready for review, this update path can never recover because you omit draft on pulls.update(). The workflow will keep reusing the same PR number but silently leave it non-draft, which breaks the contract that pre-created PRs stay WIP until the agent is done.
💡 Why this blocks merge
`pulls.create()` respects the configured `draft` flag, but the pre-created path only updates `title`, `body`, and `base`. GitHub's update endpoint also accepts `draft`, so once a user or automation flips the placeholder PR out of draft, later workflow runs cannot restore the intended state. That is especially bad for reruns because the activation job always creates a placeholder titled “Work in progress”, but the finalization path no longer controls whether reviewers/merge automation see it as ready.Pass draft through on update as well, or explicitly detect and reject a pre-created PR whose draft state no longer matches the requested configuration.
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /tdd, /diagnosing-bugs, and /grill-with-docs — requesting changes on correctness and test-coverage gaps.
📋 Key Themes & Highlights
Key Themes
- Missing
draftpropagation: theupdatepath increate_or_update_pull_request.cjsnever passesdraft, so a workflow that signals a PR is ready will leave it permanently in draft state. - Retriable vs non-retriable errors: throwing a state-mismatch error inside
withRetryburns the retry budget on a non-transient condition. - Orphaned branches: if
pulls.createorchecks.createfails after the branch ref is created, the run leaves a dangling branch with no PR. - Unconstrained
maxvalidation:pre-create: truepasses validation whenmaxis unset, even though "unlimited PRs" contradicts a single pre-created PR. - Thin test coverage:
cancelledconclusion and the early-exit guard incomplete_pre_created_check_runare untested.
Positive Highlights
- ✅ Clean end-to-end lifecycle: activation → agent checkout → safe-outputs reuse → conclusion check.
- ✅ Good use of
withRetrywith descriptive labels; the retry config is consistently applied. - ✅ Solid validation surface in
validatePreCreatePullRequestwith a comprehensive test table. - ✅ Integration test covers all four generated job sections in one shot.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 61.5 AIC · ⌖ 11.7 AIC · ⊞ 7.8K
Comment /matt to run again
Comments that could not be inline-anchored
actions/setup/js/create_or_update_pull_request.cjs:220
[/tdd] draft field is missing from the update call — if the agent signals the PR is ready for review (draft: false), the pre-created draft PR stays as a draft permanently.
<details>
<summary>💡 Suggested fix</summary>
Add draft to the update payload:
return githubClient.rest.pulls.update({
owner: repoParts.owner,
repo: repoParts.repo,
pull_number: preCreatedPullRequestNumber,
title,
body,
base: baseBranch,
draft, // ← add this
});Add a test asserting th…
actions/setup/js/create_or_update_pull_request.cjs:211
[/diagnosing-bugs] Throwing inside withRetry when the PR is closed/merged causes the error to be retried unnecessarily before bubbling up. A state-mismatch is not a transient error; it should fail immediately without exhausting retries.
<details>
<summary>💡 Suggested approach</summary>
Fetch the PR outside the withRetry wrapper (or use a non-retriable error subclass), so the retry budget is preserved for actual transient failures (rate limits, network blips). Add a test covering this…
actions/setup/js/pre_create_pull_request.cjs:505
[/diagnosing-bugs] No cleanup if pulls.create or checks.create fails after the branch ref is already created — the run leaves an orphaned branch (gh-aw/pre-created/<runId>-<attempt>) with no associated PR.
<details>
<summary>💡 Suggested mitigation</summary>
Wrap pulls.create and checks.create in a try/catch that deletes the ref on failure:
try {
const { data: pullRequest } = await github.rest.pulls.create({ ... });
const { data: checkRun } = await github.rest.checks.…
</details>
<details><summary>pkg/workflow/create_pull_request.go:1133</summary>
**[/grill-with-docs]** When `config.Max` is `nil` (not explicitly set), `pre-create: true` passes validation — but `nil` typically means "unlimited" in this codebase. Allowing unlimited PRs with a single pre-created PR is logically contradictory and the behavior is undefined.
<details>
<summary>💡 Suggested fix</summary>
Treat `nil` the same as an explicit value other than `"1"`:
```go
if config.Max == nil {
return errors.New("safe-outputs.create-pull-request.pre-create requires max: 1")…
</details>
<details><summary>actions/setup/js/complete_pre_created_check_run.test.cjs:172</summary>
**[/tdd]** Missing test cases for `cancelled` conclusion and for the early-exit path when `GH_AW_PRE_CREATED_CHECK_RUN_ID` is absent.
<details>
<summary>💡 Suggested additions</summary>
```js
it("completes the check with 'cancelled' when a job was cancelled", async () => {
process.env.GH_AW_NEEDS = JSON.stringify({ agent: { result: "cancelled" } });
const { main } = await import("./complete_pre_created_check_run.cjs");
await main();
expect(global.github.rest.checks.update).toHaveBeenC…
</details>
There was a problem hiding this comment.
Review: Add pre-created pull request safe output mode
The overall approach is sound — pre-allocating a draft PR on activation makes the workflow run immediately visible. The implementation is clean and well-tested. I found one notable bug and a minor concern.
Bug: draft state not updated on pre-created PR
In create_or_update_pull_request.cjs, the draft parameter is destructured from options but never passed to pulls.update. This means the pre-created draft PR will never be promoted to a non-draft state when the agent finishes — it stays as a draft regardless of the user's draft: false config.
The GitHub REST API's PATCH /repos/{owner}/{repo}/pulls/{pull_number} does accept a draft field to change draft state.
Minor: No idempotency guard on repeated activation
In pre_create_pull_request.cjs, a new empty commit and branch are created every activation. On a re-run (GITHUB_RUN_ATTEMPT > 1) the branch name includes the attempt number so it's unique, but if the activation step itself retries within the same attempt, duplicate branches/PRs could be created. Consider checking whether the branch already exists before creating it.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 46.6 AIC · ⌖ 8.91 AIC · ⊞ 5.7K
| return githubClient.rest.pulls.update({ | ||
| owner: repoParts.owner, | ||
| repo: repoParts.repo, | ||
| pull_number: preCreatedPullRequestNumber, |
There was a problem hiding this comment.
The draft parameter is destructured from options but never passed to pulls.update. When the agent completes and calls createOrUpdatePullRequest, the pre-created draft PR will remain a draft regardless of the workflow config.
Fix:
return githubClient.rest.pulls.update({
owner: repoParts.owner,
repo: repoParts.repo,
pull_number: preCreatedPullRequestNumber,
title,
body,
base: baseBranch,
draft, // add this
});@copilot please address this.
There was a problem hiding this comment.
Ponytail review: over-engineering pass only.
net: -8 lines possible.
Generated by ✂️ Ponytail Reviewer for #53576 · auto · 62 AIC · ⌖ 5.11 AIC · ⊞ 7.3K
Comment /ponytail to run again
| remoteTarget: pushRemoteUrl || "origin", | ||
| remoteToken: headGitHubToken, | ||
| cwd: forkCwd, | ||
| allowExistingBranch: branchName === preCreatedBranch, |
There was a problem hiding this comment.
shrink: allowExistingBranch: branchName === preCreatedBranch repeated 3x (L1812, L2207, L2384) with identical logic. Compute once as const allowExistingBranch = branchName === preCreatedBranch; near L1751 and reuse.
| stepID, | ||
| )...) | ||
| } | ||
| appToken := fmt.Sprintf("${{ steps.%s.outputs.token }}", stepID) |
There was a problem hiding this comment.
yagni: app-token-mint-then-combine pattern duplicated almost verbatim between addActivationPreCreatePullRequestStep (L40-57) and buildConclusionPreCreatedCheckRunStep (L84-103). Extract a shared helper that mints and resolves the token, then call it from both.
There was a problem hiding this comment.
Pull request overview
Adds opt-in draft PR pre-creation so agent changes reuse a PR visible from workflow activation onward.
Changes:
- Creates and checks out a pre-allocated PR branch.
- Reuses the PR and completes its workflow check.
- Adds validation, schema, documentation, and tests.
Show a summary per file
| File | Description |
|---|---|
pkg/workflow/safe_outputs_handler_registry.go |
Passes pre-created PR identifiers to handlers. |
pkg/workflow/safe_output_handlers.go |
Adds check-write permissions. |
pkg/workflow/notify_comment.go |
Adds check completion to conclusion jobs. |
pkg/workflow/create_pull_request.go |
Defines and validates pre-create. |
pkg/workflow/compiler_yaml_checkout.go |
Checks out the allocated branch. |
pkg/workflow/compiler_validators.go |
Registers pre-create validation. |
pkg/workflow/compiler_safe_outputs_steps.go |
Preserves the branch in safe outputs. |
pkg/workflow/compiler_precreate_pull_request.go |
Generates activation and conclusion steps. |
pkg/workflow/compiler_precreate_pull_request_test.go |
Tests compiler behavior and validation. |
pkg/workflow/compiler_precreate_pull_request_integration_test.go |
Tests compiled workflow integration. |
pkg/workflow/compiler_activation_steps.go |
Adds activation checkout and PR creation. |
pkg/workflow/compiler_activation_permissions.go |
Grants activation write permissions. |
pkg/workflow/checkout_step_generator.go |
Applies generated checkout ref overrides. |
pkg/workflow/checkout_manager.go |
Adds default ref override state. |
pkg/parser/schemas/main_workflow_schema.json |
Adds the schema property. |
docs/src/content/docs/reference/safe-outputs-pull-requests.md |
Documents pre-created PRs. |
actions/setup/js/pre_create_pull_request.test.cjs |
Tests PR and check creation. |
actions/setup/js/pre_create_pull_request.cjs |
Creates the branch, PR, and check. |
actions/setup/js/create_pull_request.cjs |
Reuses the allocated branch and PR. |
actions/setup/js/create_pull_request_precreate.test.cjs |
Tests PR reuse. |
actions/setup/js/create_or_update_pull_request.cjs |
Selects PR creation or update. |
actions/setup/js/complete_pre_created_check_run.test.cjs |
Tests check conclusions. |
actions/setup/js/complete_pre_created_check_run.cjs |
Completes the linked check. |
.github/workflows/mcp-inspector.lock.yml |
Refreshes generated strict-mode metadata. |
.github/workflows/daily-team-evolution-insights.lock.yml |
Refreshes generated strict-mode metadata. |
Review details
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 25/25 changed files
- Comments generated: 5
- Review effort level: Balanced
| if isPreCreatePullRequestEnabled(data) { | ||
| checkoutManager := NewCheckoutManager(data.CheckoutConfigs) | ||
| if checkoutManager.HasAppAuth() { | ||
| ctx.steps = append(ctx.steps, checkoutManager.GenerateCheckoutAppTokenSteps(c, resolveCheckoutPermissions(data))...) | ||
| } | ||
| checkoutSteps = checkoutManager.GenerateDefaultCheckoutStep(c.trialMode, c.trialLogicalRepoSlug, c.getActionPin) | ||
| } else { | ||
| checkoutSteps = c.generateCheckoutGitHubFolderForActivation(data) |
| CustomToken: token, | ||
| })...) | ||
| } | ||
| steps = append(steps, c.buildConclusionPreCreatedCheckRunStep(data)...) |
| func validatePreCreatePullRequest(data *WorkflowData) error { | ||
| if !isPreCreatePullRequestEnabled(data) { | ||
| return nil | ||
| } |
| return githubClient.rest.pulls.update({ | ||
| owner: repoParts.owner, | ||
| repo: repoParts.repo, | ||
| pull_number: preCreatedPullRequestNumber, | ||
| title, | ||
| body, | ||
| base: baseBranch, | ||
| }); |
| team-reviewers: [platform-reviewers] # team slugs to request as reviewers | ||
| assignees: [user1] # assignees for the created PR and any fallback issue | ||
| draft: true # create as draft — enforced as policy (default: true) | ||
| pre-create: true # allocate a draft PR before the agent starts (default: false) |
|
@copilot Please address the blocking review feedback below, refresh the branch if needed, then run the
|
|
🎉 Excellent work on this PR! This feature addition for pre-created pull request safe outputs looks solid. What we are seeing:
PR readiness: Great job on the structured breakdown and permission auditing! 🚀
|
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
|
@copilot Please confirm the latest fixes are complete, refresh the branch if needed, and run the
|
Confirmed — no further edits were required, so I did not run Branch: HEAD is still Fixes still in place on HEAD: draft-state alignment via GraphQL after update, conclusion job running when Remaining items — confirmed: maintainer review approval, dismissal of the two stale |
|
@copilot make the test parallelizer agentic workflow use this feature . |
|
@copilot the pre-created PR is ALWAYS in draft mode, regardless of the draft field value. If draft:false, the PR is undrafted in the safe output phase. Add tests. |
|
@copilot Please address the latest maintainer feedback on this PR, then run the
|
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in That workflow runs every 2h and noops most runs, which would have left an empty draft PR per run, so I also made the conclusion step discard an unused allocation: when the pre-created PR is still open on its branch with Validation: |
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Confirmed — that is the implemented behavior, and tests now lock it in (
The test parallelizer workflow already uses Validation: |
PR TriageCategory: feature | Risk: high | Priority: medium (score 55)
Recommended action: defer — resolve CHANGES_REQUESTED before advancing
|
|
@copilot mark this field as experimental |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in
Validation: |
|
🎉 This pull request is included in a new release. Release: |
Adds an opt-in mode that allocates a draft pull request during activation, making the workflow run visible immediately and ensuring subsequent agent changes reuse that pull request.
Changes
Activation
Agent execution
Safe outputs
Conclusion
Configuration
pre-create.contents: write,pull-requests: write, andchecks: write.Run: https://github.com/github/gh-aw/actions/runs/32103414328> Generated by 👨🍳 PR Sous Chef · gpt54 · 14 AIC · ⌖ 7.84 AIC · ⊞ 8.8K · ◷
Caution
agentic threat detected
Threat detection flagged this output in warn mode. Manual review is REQUIRED before any follow-up automation.
Details
Potential security threats were detected in the agent output.
Review the workflow run logs for details.
Run: https://github.com/github/gh-aw/actions/runs/32109331559> Generated by 👨🍳 PR Sous Chef · gpt54 · 30 AIC · ⌖ 11.4 AIC · ⊞ 8.8K · ◷
Run: https://github.com/github/gh-aw/actions/runs/32123781702> Generated by 👨🍳 PR Sous Chef · gpt54 · 41.3 AIC · ⌖ 7.88 AIC · ⊞ 8.8K · ◷