fix: honour workflow_dispatch + aw_context in checkout_pr_branch.cjs - #49451
Conversation
When a workflow_dispatch event includes an aw_context input with item_type="pull_request" and item_number, the compiled workflow's "Checkout PR branch" step if: guard correctly matches. However, the script had no handler for this event, falling straight to "No pull request context available, skipping checkout" with exit 0. Add a workflow_dispatch handler after the issue_comment handler: parse aw_context, and when item_type is "pull_request" with a valid item_number, construct a minimal PR object. The existing fetchPRDetails + refs/pull/N/head checkout path then proceeds as normal. Also update the module JSDoc to document the new workflow_dispatch case and add 7 new test cases covering all branches of the new handler. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
✅ Test Quality Sentinel completed test quality analysis. Warning threat detection engine error DetailsThe threat detection engine failed to produce results. Review the workflow run logs for details. |
|
✅ PR Code Quality Reviewer completed the code quality review. Warning threat detection engine error DetailsThe threat detection engine failed to produce results. Review the workflow run logs for details. |
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅ Warning threat detection engine error DetailsThe threat detection engine failed to produce results. Review the workflow run logs for details. |
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. No ADR enforcement needed: PR #49451 does not have the 'implementation' label and has 0 new lines of code in business logic directories (threshold: 100). |
There was a problem hiding this comment.
Pull request overview
Adds PR-branch checkout support for workflow_dispatch events carrying aw_context.
Changes:
- Parses dispatch context and resolves the PR number.
- Adds dispatch checkout and edge-case tests.
- Documents the supported event type.
Show a summary per file
| File | Description |
|---|---|
actions/setup/js/checkout_pr_branch.cjs |
Handles PR context from workflow dispatches. |
actions/setup/js/checkout_pr_branch.test.cjs |
Tests dispatch checkout behavior and malformed inputs. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Suppressed comments (1)
actions/setup/js/checkout_pr_branch.cjs:212
- The generated
if:guard checks onlyitem_type, so a dispatch with a missing or zeroitem_numberstill runs this step. This truthiness check then leavespullRequestunset, and lines 219–222 report success without checking anything out—the same silent wrong-ref failure this change is intended to fix. Validate a positive integer and fail the step when PR context is invalid.
if (awContext.item_type === "pull_request" && awContext.item_number) {
pullRequest = {
number: awContext.item_number,
state: "open",
};
core.info(`Detected workflow_dispatch event for PR #${pullRequest.number} via aw_context, will fetch PR ref`);
}
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
| const awContext = JSON.parse(awContextStr); | ||
| if (awContext.item_type === "pull_request" && awContext.item_number) { | ||
| pullRequest = { | ||
| number: awContext.item_number, |
There was a problem hiding this comment.
Addressed in a previous commit (acc53f9): the handler now compares awContext.repo against context.repo.owner/context.repo.repo before constructing pullRequest. A mismatch emits core.warning and skips checkout. This thread is outdated; the code it referenced no longer exists in its original form.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Warning
threat detection engine error
The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.
Details
The threat detection engine failed to produce results.
Review the workflow run logs for details.
The fix is correct and well-tested. The workflow_dispatch handler is properly guarded, error handling is non-fatal, and all branches are covered by tests. No blocking issues found.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · sonnet46 · 13.5 AIC · ⊞ 5.3K
|
Warning threat detection engine error DetailsThe threat detection engine failed to produce results. Review the workflow run logs for details. 🧪 Test Quality Sentinel Report
📊 Metrics (7 tests)
🧪 Test Breakdown (7 tests, new describe block "workflow_dispatch events with aw_context")
|
There was a problem hiding this comment.
✅ Test Quality Sentinel: 71/100 — Acceptable. 6 of 7 tests are design-focused with strong edge-case coverage (14% implementation ratio, threshold: 30%). While test inflation (3.4:1) and one duplicate assertion were noted, the behavioral contract is well-protected. Approved.
There was a problem hiding this comment.
Warning
threat detection engine error
The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.
Details
The threat detection engine failed to produce results.
Review the workflow run logs for details.
Skills-Based Review 🧠
Applied /diagnosing-bugs and /tdd — requesting changes on two targeted issues.
📋 Key Themes & Highlights
Issues
item_numbertype safety (checkout_pr_branch.cjsline 203) — truthy check allows non-integer values that will break the downstream git ref. ANumber.isIntegerguard is cheap and prevents a late, opaque failure.- Missing
fetchPRDetailsassertion in the happy-path test (checkout_pr_branch.test.cjsline 576) — the test verifies the git commands but does not assert the API call that fetches full PR details, leaving that contract unverified.
Positive Highlights
- ✅ Root cause fixed in the right place — mirrors the existing
if:guard in the compiled workflow - ✅ Defensive JSON parsing with
core.warninginstead of silent skip - ✅ 7 test cases covering all notable branches including edge cases (invalid JSON, missing inputs, wrong item_type, missing item_number)
- ✅ JSDoc updated to document the fourth supported event type
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · sonnet46 · 30.1 AIC · ⊞ 7K
Comment /matt to run again
| // Handle workflow_dispatch events with aw_context pointing to a PR | ||
| if (!pullRequest && eventName === "workflow_dispatch") { | ||
| const awContextStr = context.payload.inputs?.aw_context; | ||
| if (awContextStr) { |
There was a problem hiding this comment.
[/diagnosing-bugs] awContext.item_number passes any truthy value — a string "abc" or float would silently proceed to git fetch refs/pull/abc/head and fail late with a confusing git error.
💡 Suggested fix
const prNumber = Number(awContext.item_number);
if (awContext.item_type === "pull_request" && Number.isInteger(prNumber) && prNumber > 0) {
pullRequest = { number: prNumber, state: "open" };This also ensures pullRequest.number is always a plain integer downstream, consistent with how the pull_request and issue_comment branches populate it.
@copilot please address this.
There was a problem hiding this comment.
Fixed in the latest commit. The truthy check is replaced with proper integer validation:
const prNumber = Number(awContext.item_number);
if (awContext.item_type === "pull_request" && Number.isInteger(prNumber) && prNumber > 0) {
pullRequest = { number: prNumber, state: "open" };Three additional test cases were added covering item_number: "abc", item_number: 0, and item_number: 1.5 — all correctly skip checkout. 67/67 tests pass.
| expect(mockCore.info).toHaveBeenCalledWith("Pull Request #123"); | ||
|
|
||
| // workflow_dispatch uses git fetch refs/pull + checkout (not the fast pull_request path) | ||
| expect(mockExec.exec).toHaveBeenCalledWith("git", ["fetch", "origin", "+refs/pull/123/head:refs/remotes/origin/pr-head", "--depth=2"]); |
There was a problem hiding this comment.
[/tdd] The happy-path test asserts that fetchPRDetails is called indirectly via the git fetch/checkout sequence, but there is no test asserting that fetchPRDetails is actually invoked with the correct PR number (123). If the implementation ever switches to a direct API call instead, this test would not catch the regression.
💡 Suggested additional assertion
expect(mockOctokit.rest.pulls.get).toHaveBeenCalledWith(
expect.objectContaining({ pull_number: 123 })
);(Adjust to whatever fetchPRDetails mock call is used elsewhere in the test file.) This pins the contract between the workflow_dispatch path and the shared checkout logic.
@copilot please address this.
There was a problem hiding this comment.
Added in the latest commit — the happy-path test now asserts the fetchPRDetails contract:
expect(mockGithub.rest.pulls.get).toHaveBeenCalledWith(
expect.objectContaining({ pull_number: 123 })
);This pins the contract between the workflow_dispatch path and the shared API-fetch/checkout logic. All 67 tests pass.
There was a problem hiding this comment.
Warning
threat detection engine error
The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.
Details
The threat detection engine failed to produce results.
Review the workflow run logs for details.
Verdict: non-blocking, request minor hardening
The workflow_dispatch handler mirrors the compiler's if: guard correctly and preserves the assertTrustedCheckoutRuntime() gate. Two non-blocking robustness issues flagged inline: unvalidated item_number type/range, and hardcoded state: "open" that bypasses the closed-PR fast path (masked at runtime by the existing API re-check fallback, so not merge-blocking).
Review notes
- Trust boundary: confirmed the new branch still flows through
assertTrustedCheckoutRuntime()before any git operation — no bypass introduced. - Test coverage is solid for the main branches (missing input, invalid JSON, wrong item_type, missing item_number) but doesn't cover a non-numeric/string
item_numberoritem_number: 0. state: "open"inaccuracy is soft-mitigated by theisNowClosedre-check viagithub.rest.pulls.getin the catch block, so a closed PR referenced viaaw_contextwill still be handled gracefully, just via a slower/less direct path than theisClosedfast check.
🔎 Code quality review by PR Code Quality Reviewer · auto · 44.6 AIC · ⊞ 7.8K
Comment /review to run again
| if (!pullRequest && eventName === "workflow_dispatch") { | ||
| const awContextStr = context.payload.inputs?.aw_context; | ||
| if (awContextStr) { | ||
| try { | ||
| const awContext = JSON.parse(awContextStr); | ||
| if (awContext.item_type === "pull_request" && awContext.item_number) { |
There was a problem hiding this comment.
item_number from the untrusted aw_context JSON payload is used unvalidated as a truthy/number check, letting malformed values silently change behavior.
There was a problem hiding this comment.
Fixed in the latest commit — item_number is now validated as a positive integer before the pullRequest object is constructed. See the sibling thread reply for the exact change and new test coverage.
| pullRequest = { | ||
| number: awContext.item_number, | ||
| state: "open", |
There was a problem hiding this comment.
Hardcoding state: "open" bypasses the closed-PR fast-path check even when the referenced PR is actually closed.
There was a problem hiding this comment.
The state: "open" stub is intentional for the workflow_dispatch path: the dispatch payload carries no PR state, so the value is used only for the informational fast-path log at line 242. The actual PR state is retrieved from the GitHub API by fetchPRDetails before any git operation, so a closed PR is handled correctly. This thread is outdated (the surrounding code has changed since it was raised).
|
Warning threat detection engine error DetailsThe threat detection engine failed to produce results. Review the workflow run logs for details. @copilot please address the remaining review feedback, refresh the branch if GitHub allows it, and run the Current maintainer-facing blockers:
Please summarize which comments are fully addressed, whether any unresolved review thread IDs can now be resolved, and what still needs maintainer action.
|
…w_context.repo mismatches Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Addressed in commit The
Two new test cases cover the match and mismatch paths; all 64 tests pass. |
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Final
Actions taken this pass:
Still needed: CI re-trigger (close/reopen PR or push from a maintainer) before merge. |
|
Caution agentic threat detected DetailsPotential security threats were detected in the agent output. Review the workflow run logs for details. @copilot quick triage for this PR:
|
Final
Actions taken this pass: No file changes — local validation confirmed clean, no code fixes needed. |
|
@copilot quick triage for this PR:
|
The UNSTABLE state is from in-progress checks, not any code failure. Confirmed:
No code changes needed. PR is ready for merge once the current check resolves green. No other maintainer action required. |
|
@copilot quick triage for this PR:
|
UNSTABLE is from the in-progress PR Sous Chef check run (30705809447), not a code failure. Both prior No code changes needed. PR is ready to merge — the only action remaining is a maintainer merge. |
|
@copilot quick triage for this PR:
|
UNSTABLE is from the current in-progress agent check run (30707880672). All three prior Maintainer merge is the only remaining action. |
|
@copilot Please triage PR #49451 (fix: honour workflow_dispatch + aw_context in checkout_pr_branch.cjs). Refresh the branch if GitHub allows it, address any outstanding review feedback or CI instability, and run the
|
Final
Actions taken this pass: No file changes — local validation confirmed clean, no code fixes needed. |
PR Triage
workflow_dispatch checkout silently no-ops leaving stale ref; large diff, 2 approvals w/ 1 dismissed stale review.
|
The
Checkout PR branchstep'sif:guard correctly matchesworkflow_dispatchevents whereaw_context.item_type == 'pull_request', but the script it runs had no handler for this case — falling through tocore.info("No pull request context available, skipping checkout")and exiting 0. The workspace stayed on whatever ref the dispatch used, while the job showed green.Changes
checkout_pr_branch.cjs— adds aworkflow_dispatchhandler after the existingissue_commentblock: parsescontext.payload.inputs.aw_context, and whenitem_type === "pull_request"with a validitem_number, constructs a minimal PR object so the existingfetchPRDetails+refs/pull/N/headcheckout path proceeds normally. Invalid JSON emitscore.warninginstead of silently skipping.checkout_pr_branch.test.cjs— adds 7 test cases covering: successful checkout, non-PRitem_type, missingaw_context, missinginputs, invalid JSON, missingitem_number, and output correctness.workflow_dispatchas a fourth supported event type.Warning
threat detection engine error
The threat detection engine encountered an error and could not complete analysis. This is a tooling failure, not a security finding.
Details
The threat detection engine failed to produce results.
Review the workflow run logs for details.
run_url: https://github.com/github/gh-aw/actions/runs/30683195966
Run: https://github.com/github/gh-aw/actions/runs/30689325959
Run: https://github.com/github/gh-aw/actions/runs/30700817659
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 details: https://github.com/github/gh-aw/actions/runs/30702262954
Branch refresh requested by pr-sous-chef for run https://github.com/github/gh-aw/actions/runs/30709866559