feat: add close-older-pull-requests to create-pull-request safe output#36702
Conversation
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>
close-older-pull-requests to create-pull-request safe output
There was a problem hiding this comment.
Pull request overview
Adds a new close-older-pull-requests option to the create-pull-request safe-output handler to automatically close older open PRs when a new PR is created, reducing accumulation of low-signal placeholder/WIP PRs. The PR also wires the new config through the Go compiler and documents the new frontmatter fields.
Changes:
- Add
close-older-pull-requests+close-older-keyfields tocreate-pull-requestconfig, compile them into handler JSON, and adjust permissions. - Implement
close_older_pull_requests.cjs(+ tests) and invoke it fromcreate_pull_request.cjsafter successful PR creation. - Document the new settings and enable them for
chaos-pr-bundle-fuzzer.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/safe_outputs_config.go | Emits close_older_pull_requests + close_older_key into handler config JSON. |
| pkg/workflow/safe_output_handlers.go | Adds issues: write permission when close-older PR closure is enabled. |
| pkg/workflow/create_pull_request.go | Adds config fields and parsing for close-older-pull-requests and close-older-key. |
| docs/src/content/docs/reference/frontmatter-full.md | Documents close-older-pull-requests and close-older-key under create-pull-request. |
| actions/setup/js/create_pull_request.cjs | Calls closeOlderPullRequests after creating a PR; reads caller workflow ID + close-key config. |
| actions/setup/js/close_older_pull_requests.cjs | New runtime helper to search/comment/close older PRs (shares core logic with close-older entities). |
| actions/setup/js/close_older_pull_requests.test.cjs | Unit tests for search/filter/close behavior and caps. |
| .github/workflows/chaos-pr-bundle-fuzzer.md | Enables close-older-pull-requests: true to prevent backlog growth. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 8/8 changed files
- Comments generated: 3
| // Close older pull requests if enabled (best-effort: errors are logged but do not fail the workflow) | ||
| if (closeOlderPullRequestsEnabled) { | ||
| if (workflowId || rawCloseOlderKey) { | ||
| const searchKey = rawCloseOlderKey ? `gh-aw-close-key: ${rawCloseOlderKey}` : `workflow-id: ${workflowId}`; | ||
| core.info(`Attempting to close older pull requests for ${repoParts.owner}/${repoParts.repo}#${pullRequest.number} using ${searchKey}`); | ||
| try { | ||
| const closedPRs = await closeOlderPullRequests(github, repoParts.owner, repoParts.repo, workflowId, { number: pullRequest.number, html_url: pullRequest.html_url }, workflowName, runUrl, callerWorkflowId, rawCloseOlderKey); | ||
| if (closedPRs.length > 0) { |
| // isCloseOlderPullRequestsEnabled returns true when close-older-pull-requests is | ||
| // configured and not explicitly set to false or a GitHub Actions expression. | ||
| // Used for compile-time permission calculation. |
| # Optional explicit deduplication key for close-older matching. When set, a `<!-- | ||
| # gh-aw-close-key: <value> -->` marker is embedded in the PR body and used as the | ||
| # primary key for searching and filtering older pull requests instead of the | ||
| # workflow-id markers. This gives deterministic isolation across caller workflows | ||
| # and is stable across workflow renames. The value is normalized to identifier | ||
| # style (lowercase alphanumeric, dashes, underscores). |
|
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #36702 does not have the 'implementation' label and has only 20 new lines of code in business logic directories (≤100 threshold). |
|
🧪 Test Quality Sentinel completed test quality analysis. |
🧪 Test Quality Sentinel Report✅ Test Quality Score: 81/100 — Excellent
📊 Metrics & Test Classification (16 tests analyzed)
Test Classification Details
Language SupportTests analyzed:
Verdict
📖 Understanding Test ClassificationsDesign Tests (High Value) verify what the system does:
Implementation Tests (Low Value) verify how the system does it:
Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators. References: §26909970993
|
![github-actions[bot]](https://github.com/_gh/avatars/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /diagnose, /tdd, and /zoom-out — clean implementation that closely mirrors close_older_issues; flagging one misleading log string and a small test coverage gap.
📋 Key Themes & Highlights
Key Themes
- Log message inconsistency:
workflow-id:in the info log doesn't match the actualgh-aw-workflow-id:marker format — misleading during debugging - Test coverage gap:
closeOlderPullRequestsintegration tests don't exercise thecloseOlderKeyforwarding path end-to-end - Permission over-grant note:
isCloseOlderPullRequestsEnabledconservatively grantsissues: writefor any non-false expression value, including unresolved GA expressions — intentional but worth documenting
Positive Highlights
- ✅ Strong unit test coverage for
searchOlderPullRequestsacross all meaningful filter variants - ✅ Clean delegation to
closeOlderEntitiesvia closure — avoids duplicating the rate-limit / error-continue loop - ✅ Best-effort error handling in
create_pull_request.cjs— failures log a warning but don't abort the workflow - ✅ Permission grant correctly scoped to
issues: write(comment posting via issues API) rather thanpull-requests: write - ✅ Documentation in
frontmatter-full.mdis thorough and accurate
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 1.5M
| // Close older pull requests if enabled (best-effort: errors are logged but do not fail the workflow) | ||
| if (closeOlderPullRequestsEnabled) { | ||
| if (workflowId || rawCloseOlderKey) { | ||
| const searchKey = rawCloseOlderKey ? `gh-aw-close-key: ${rawCloseOlderKey}` : `workflow-id: ${workflowId}`; |
There was a problem hiding this comment.
[/diagnose] Log message uses wrong marker name — workflow-id: should be gh-aw-workflow-id: to match the actual HTML comment marker embedded in PR bodies.
💡 Suggested fix
// Before
const searchKey = rawCloseOlderKey ? `gh-aw-close-key: ${rawCloseOlderKey}` : `workflow-id: ${workflowId}`;
// After
const searchKey = rawCloseOlderKey ? `gh-aw-close-key: ${rawCloseOlderKey}` : `gh-aw-workflow-id: ${workflowId}`;This is a log-only string so it has no functional impact, but it is misleading when debugging why close-older didn't match — the log message would show a different key format than what's actually searched for.
| expect(global.core.info).toHaveBeenCalledWith("✓ No older pull requests found to close - operation complete"); | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
[/tdd] The closeOlderPullRequests integration suite doesn't include a test for the closeOlderKey forwarding path. The unit-level coverage in searchOlderPullRequests is good, but there's no test verifying that when closeOlderKey is passed to closeOlderPullRequests it reaches the search call with the correct query.
💡 Suggested test case to add
it('should forward closeOlderKey to search when provided', async () => {
mockGithub.rest.search.issuesAndPullRequests.mockResolvedValue({
data: { items: [] },
});
const newPR = { number: 200, html_url: 'https://github.com/owner/repo/pull/200' };
await closeOlderPullRequests(
mockGithub, 'owner', 'repo', 'some-workflow', newPR,
'My Workflow', 'https://github.com/owner/repo/actions/runs/1',
undefined, 'my-close-key'
);
expect(mockGithub.rest.search.issuesAndPullRequests).toHaveBeenCalledWith({
q: 'repo:owner/repo is:pr is:open "gh-aw-close-key: my-close-key" in:body',
per_page: 50,
});
});This guards against a regression where the closure over closeOlderKey in closeOlderPullRequests stops forwarding the value.
| } | ||
|
|
||
| // CreatePullRequestsConfig holds configuration for creating GitHub pull requests from agent output | ||
| type CreatePullRequestsConfig struct { |
There was a problem hiding this comment.
[/zoom-out] isCloseOlderPullRequestsEnabled treats any non-"false"/non-"0" string value (including GitHub Actions expressions like ${{ inputs.enable_close }}) as "enabled" at compile time. This means workflows using a templatable value always compile with issues: write granted, even when the runtime expression evaluates to false.
This is consistent with how other templatable fields (e.g. draft, auto-merge) are handled elsewhere in the codebase — so no change needed if over-granting is intentional/acceptable. Worth calling out explicitly in the comment so future maintainers understand the tradeoff.
|
@copilot merge main and recompile |
…ody; add schema entry for close-older-pull-requests Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done. Here's what was addressed: Marker embedding fixed (
Doc comment corrected ( Docs now accurate ( Schema fix ( Main merged at |
WIP/placeholder PRs were accumulating unboundedly —
chaos-pr-bundle-fuzzeralone had 10+ open PRs at 0% merge rate, generating reviewer noise with no signal value.Adds a
close-older-pull-requestsoption tocreate-pull-request(analogous to the existingclose-older-issuesoncreate-issue) that auto-closes older open PRs from the same workflow when a new one is created.New runtime handler
close_older_pull_requests.cjs— mirrorsclose_older_issues.cjs; usesis:prsearch qualifier andpulls.update(state: "closed")to close; comments viaissues.createComment(GitHub uses issues endpoint for PR comments); capped at 10 closures per runGo config wiring
CloseOlderPullRequests *stringandCloseOlderKey stringtoCreatePullRequestsConfig"close-older-pull-requests"registered inBoolFieldsso YAMLtrue/falseis parsed correctlysafe_outputs_config.go: emitsclose_older_pull_requests+close_older_keyinto handler configsafe_output_handlers.go: addsissues: writepermission whenclose-older-pull-requestsis enabled withfallback-as-issue: false(already included whentrue)Integration
create_pull_request.cjsreads config flags, readsGH_AW_CALLER_WORKFLOW_IDfor reusable-workflow isolation, and callscloseOlderPullRequestsafter PR creation (best-effort; errors logged, not fatal)Usage
Applied immediately
chaos-pr-bundle-fuzzer.md: enabledclose-older-pull-requests: trueto stop backlog accumulationfrontmatter-full.md: documented new fields