test: verify visible automatic bonk wrapper (updated)#361
test: verify visible automatic bonk wrapper (updated)#361geoquant wants to merge 8 commits intocloudflare:mainfrom
Conversation
|
/bonk review Triggered automatically for this PR. |
There was a problem hiding this comment.
Review Summary
This PR refactors the Bonk automation workflow to provide automatic /bonk review triggers on PRs. This is a well-structured change that improves the developer experience by eliminating the manual /bonk comment step.
Changes Overview
1. bonk-check.yml - Major refactor:
- Switches from
issue_comment/pull_request/pull_request_reviewtriggers topull_request_target(for automatic triggering on PR open/update/ready) - Adds
workflow_dispatchfor manual testing capability - Introduces proper concurrency control with
cancel-in-progress: true - Posts a visible automatic
/bonk reviewcomment (with<!-- auto-bonk -->marker to prevent duplicates) - Dispatches the
bonk.ymlworkflow via GitHub API
2. bonk.yml - Adds workflow_dispatch support:
- New
workflow_dispatchtrigger with PR metadata inputs - Conditional logic to skip setup steps (pnpm, Node, deps) for auto-dispatched runs
- New "Run automatic Bonk review" job for workflow_dispatch events
3. bigbonk.yml - Cleanup:
- Removes unused
pull_request_reviewtrigger
4. Test file:
AUTO_BONK_SMOKE_TEST_4.md- Simple smoke test file as described
Correctness & Security
pull_request_targetusage: Appropriately used here since the workflow needs write permissions (PR comments, workflow dispatch) and only operates on PR metadata, not untrusted codeactions: writepermission: Required for thegh apicall to dispatch thebonk.ymlworkflow - this is the minimal required permission- Concurrency handling: Properly uses
cancel-in-progress: trueinbonk-check.ymlto prevent redundant runs - Draft PR handling: Correctly skips automatic bonk for draft PRs
Suggestions
-
Line 115-121 in
bonk-check.yml: The summary step uses${{ steps.pr.outputs.number }}directly in the shell command. Consider quoting to handle any edge cases, though GitHub Actions automatically handles numeric outputs safely. -
The
AUTO_BONK_SMOKE_TEST_4.mdfile should be removed before merge since it's only for testing the workflow.
Overall, this is a clean implementation that improves the PR review workflow. The logic is sound and the security considerations are appropriately addressed.
commit: |
Docs PreviewCommit: |
Fourth smoke-test PR for the automatic Bonk workflow.
This PR targets
mainso the smoke test reflects the real protected-branch flow after thebonk-checkcontext/concurrency polish in PR #357.Expected behavior for this smoke test:
/bonk reviewcomment appears on the PR/bonkmain