-
Notifications
You must be signed in to change notification settings - Fork 476
fix: honour workflow_dispatch + aw_context in checkout_pr_branch.cjs #49451
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
Changes from all commits
4b7ffcc
ad53ee9
9a144a2
6c7c9be
8351382
835680b
acc53f9
c3e98e7
518d38b
d038257
53c9f32
f9d3001
2fbe65e
70a721d
17a11ce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,83 @@ | ||
| --- | ||
| private: true | ||
| emoji: "🧪" | ||
| name: Smoke Checkout PR Dispatch | ||
| description: Integration test validating that workflow_dispatch events with aw_context.item_type == 'pull_request' correctly check out the PR branch | ||
| on: | ||
| slash_command: | ||
| name: smoke-checkout-pr-dispatch | ||
| strategy: centralized | ||
| events: [issues, issue_comment, pull_request, pull_request_comment] | ||
| workflow_dispatch: | ||
| pull_request: | ||
| types: [labeled] | ||
| names: ["smoke-checkout-pr-dispatch"] | ||
| status-comment: true | ||
| permissions: | ||
| contents: read | ||
| pull-requests: read | ||
| engine: copilot | ||
| strict: true | ||
| network: | ||
| allowed: | ||
| - defaults | ||
| imports: | ||
| - shared/otlp.md | ||
| tools: | ||
| bash: | ||
| - "git status" | ||
| - "git log *" | ||
| - "git branch *" | ||
| - "git remote *" | ||
| - "echo *" | ||
| safe-outputs: | ||
| allowed-domains: [default-safe-outputs] | ||
| add-comment: | ||
| hide-older-comments: true | ||
| max: 1 | ||
| messages: | ||
| footer: "> 🧪 *checkout_pr_branch dispatch smoke test by [{workflow_name}]({run_url})*{ai_credits_suffix}{history_link}" | ||
| run-started: "🧪 [{workflow_name}]({run_url}) is validating workflow_dispatch PR branch checkout..." | ||
| run-success: "✅ [{workflow_name}]({run_url}) successfully validated workflow_dispatch PR branch checkout." | ||
| run-failure: "[{workflow_name}]({run_url}) failed to validate workflow_dispatch PR branch checkout: {status}" | ||
| timeout-minutes: 10 | ||
| features: | ||
| gh-aw-detection: false | ||
| --- | ||
|
|
||
| # Smoke Test: workflow_dispatch + aw_context PR Branch Checkout | ||
|
|
||
| This workflow validates the `checkout_pr_branch.cjs` behaviour when triggered via `workflow_dispatch` | ||
| with an `aw_context` input whose `item_type` is `"pull_request"`. The setup step should detect the | ||
| context, fetch `refs/pull/<N>/head`, and check out the PR branch before the agent runs. | ||
|
|
||
| It also exercises the same checkout path when triggered directly by a pull-request event or | ||
| `slash_command`, ensuring both code paths are exercised. | ||
|
|
||
| ## Context | ||
|
|
||
| - Event: `${{ github.event_name }}` | ||
| - `aw_context` input (if present): `${{ github.event.inputs.aw_context }}` | ||
|
|
||
| ## Test Requirements | ||
|
|
||
| Run each check and mark as ✅ pass or ❌ fail: | ||
|
|
||
| 1. **Git status**: Run `git status` and confirm the workspace is in a clean, initialised state. | ||
| 2. **Branch check**: Run `git branch --show-current` and record the checked-out branch name. | ||
| 3. **Remote check**: Run `git remote -v` and confirm a remote named `origin` is present. | ||
| 4. **Log check**: Run `git log --oneline -3` and confirm at least one commit is visible. | ||
| 5. **Not default branch**: When triggered via `workflow_dispatch` with a PR `aw_context`, or via | ||
| a PR event, the current branch **must not** be `main` — a different branch name confirms the | ||
| PR checkout ran successfully. | ||
|
|
||
| ## Output | ||
|
|
||
| Add a comment summarising the checkout validation results: | ||
|
|
||
| - Event name and, if `${{ github.event_name }}` is `workflow_dispatch`, the `item_number` extracted | ||
| from `aw_context` (parse `${{ github.event.inputs.aw_context }}` to find it) | ||
| - Current branch name (from `git branch --show-current`) | ||
| - Last 3 commits (from `git log --oneline -3`) | ||
| - Whether the workspace is on a branch other than `main` | ||
| - Overall status: PASS or FAIL with a brief explanation |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -554,6 +554,146 @@ If the pull request is still open, verify that: | |
| }); | ||
| }); | ||
|
|
||
| describe("workflow_dispatch events with aw_context", () => { | ||
| beforeEach(() => { | ||
| mockContext.eventName = "workflow_dispatch"; | ||
| mockContext.payload = { | ||
| repository: { fork: false }, | ||
| inputs: { | ||
| aw_context: JSON.stringify({ item_type: "pull_request", item_number: 123 }), | ||
| }, | ||
| }; | ||
| }); | ||
|
|
||
| it("should checkout PR using git fetch refs/pull when aw_context has item_type pull_request", async () => { | ||
| await runScript(); | ||
|
|
||
| expect(mockCore.info).toHaveBeenCalledWith("Detected workflow_dispatch event for PR #123 via aw_context, will fetch PR ref"); | ||
| expect(mockCore.info).toHaveBeenCalledWith("Event: workflow_dispatch"); | ||
| 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"]); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [/tdd] The happy-path test asserts that 💡 Suggested additional assertionexpect(mockOctokit.rest.pulls.get).toHaveBeenCalledWith(
expect.objectContaining({ pull_number: 123 })
);(Adjust to whatever @copilot please address this.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added in the latest commit — the happy-path test now asserts the expect(mockGithub.rest.pulls.get).toHaveBeenCalledWith(
expect.objectContaining({ pull_number: 123 })
);This pins the contract between the |
||
| expect(mockExec.exec).toHaveBeenCalledWith("git", ["checkout", "-B", "feature-branch", "origin/pr-head"]); | ||
|
|
||
| // fetchPRDetails must be called with the correct PR number to resolve head ref / commit count | ||
| expect(mockGithub.rest.pulls.get).toHaveBeenCalledWith(expect.objectContaining({ pull_number: 123 })); | ||
|
|
||
| expect(mockCore.setOutput).toHaveBeenCalledWith("checkout_pr_success", "true"); | ||
| expect(mockCore.setFailed).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("should skip checkout when aw_context item_type is not pull_request", async () => { | ||
| mockContext.payload.inputs.aw_context = JSON.stringify({ item_type: "issue", item_number: 42 }); | ||
|
|
||
| await runScript(); | ||
|
|
||
| expect(mockCore.info).toHaveBeenCalledWith("No pull request context available, skipping checkout"); | ||
| expect(mockExec.exec).not.toHaveBeenCalled(); | ||
| expect(mockCore.setFailed).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("should skip checkout when workflow_dispatch has no aw_context input", async () => { | ||
| mockContext.payload.inputs = {}; | ||
|
|
||
| await runScript(); | ||
|
|
||
| expect(mockCore.info).toHaveBeenCalledWith("No pull request context available, skipping checkout"); | ||
| expect(mockExec.exec).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("should skip checkout when workflow_dispatch has no inputs at all", async () => { | ||
| mockContext.payload.inputs = undefined; | ||
|
|
||
| await runScript(); | ||
|
|
||
| expect(mockCore.info).toHaveBeenCalledWith("No pull request context available, skipping checkout"); | ||
| expect(mockExec.exec).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("should warn and skip checkout when aw_context is invalid JSON", async () => { | ||
| mockContext.payload.inputs.aw_context = "not-valid-json{"; | ||
|
|
||
| await runScript(); | ||
|
|
||
| expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Failed to parse aw_context:")); | ||
| expect(mockCore.info).toHaveBeenCalledWith("No pull request context available, skipping checkout"); | ||
| expect(mockExec.exec).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("should skip checkout when aw_context pull_request has no item_number", async () => { | ||
| mockContext.payload.inputs.aw_context = JSON.stringify({ item_type: "pull_request" }); | ||
|
|
||
| await runScript(); | ||
|
|
||
| expect(mockCore.info).toHaveBeenCalledWith("No pull request context available, skipping checkout"); | ||
| expect(mockExec.exec).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("should skip checkout when aw_context item_number is a non-numeric string", async () => { | ||
| mockContext.payload.inputs.aw_context = JSON.stringify({ item_type: "pull_request", item_number: "abc" }); | ||
|
|
||
| await runScript(); | ||
|
|
||
| expect(mockCore.info).toHaveBeenCalledWith("No pull request context available, skipping checkout"); | ||
| expect(mockExec.exec).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("should skip checkout when aw_context item_number is zero", async () => { | ||
| mockContext.payload.inputs.aw_context = JSON.stringify({ item_type: "pull_request", item_number: 0 }); | ||
|
|
||
| await runScript(); | ||
|
|
||
| expect(mockCore.info).toHaveBeenCalledWith("No pull request context available, skipping checkout"); | ||
| expect(mockExec.exec).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("should skip checkout when aw_context item_number is a non-integer float", async () => { | ||
| mockContext.payload.inputs.aw_context = JSON.stringify({ item_type: "pull_request", item_number: 1.5 }); | ||
|
|
||
| await runScript(); | ||
|
|
||
| expect(mockCore.info).toHaveBeenCalledWith("No pull request context available, skipping checkout"); | ||
| expect(mockExec.exec).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("should checkout PR when aw_context repo matches current repository", async () => { | ||
| mockContext.payload.inputs.aw_context = JSON.stringify({ | ||
| item_type: "pull_request", | ||
| item_number: 123, | ||
| repo: "test-owner/test-repo", | ||
| }); | ||
|
|
||
| await runScript(); | ||
|
|
||
| expect(mockCore.info).toHaveBeenCalledWith("Detected workflow_dispatch event for PR #123 via aw_context, will fetch PR ref"); | ||
| expect(mockExec.exec).toHaveBeenCalledWith("git", ["fetch", "origin", "+refs/pull/123/head:refs/remotes/origin/pr-head", "--depth=2"]); | ||
| expect(mockCore.warning).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("should warn and skip checkout when aw_context repo does not match current repository", async () => { | ||
| mockContext.payload.inputs.aw_context = JSON.stringify({ | ||
| item_type: "pull_request", | ||
| item_number: 123, | ||
| repo: "other-owner/other-repo", | ||
| }); | ||
|
|
||
| await runScript(); | ||
|
|
||
| expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("Cross-repository workflow_dispatch is not supported")); | ||
| expect(mockCore.warning).toHaveBeenCalledWith(expect.stringContaining("other-owner/other-repo")); | ||
| expect(mockCore.info).toHaveBeenCalledWith("No pull request context available, skipping checkout"); | ||
| expect(mockExec.exec).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("should set output to true on successful workflow_dispatch PR checkout", async () => { | ||
| await runScript(); | ||
|
|
||
| expect(mockCore.setOutput).toHaveBeenCalledWith("checkout_pr_success", "true"); | ||
| expect(mockCore.setFailed).not.toHaveBeenCalled(); | ||
| }); | ||
| }); | ||
|
|
||
| describe("different event types", () => { | ||
| it("should handle pull_request_target event", async () => { | ||
| mockContext.eventName = "pull_request_target"; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[/diagnosing-bugs]
awContext.item_numberpasses any truthy value — a string"abc"or float would silently proceed togit fetch refs/pull/abc/headand fail late with a confusing git error.💡 Suggested fix
This also ensures
pullRequest.numberis always a plain integer downstream, consistent with how thepull_requestandissue_commentbranches populate it.@copilot please address this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in the latest commit. The truthy check is replaced with proper integer validation:
Three additional test cases were added covering
item_number: "abc",item_number: 0, anditem_number: 1.5— all correctly skip checkout. 67/67 tests pass.