Skip to content

Improve Bonk to read triggering comments and act on fixup requests#13147

Merged
penalosa merged 3 commits intomainfrom
penalosa/bonk-fixup-improvements
Mar 31, 2026
Merged

Improve Bonk to read triggering comments and act on fixup requests#13147
penalosa merged 3 commits intomainfrom
penalosa/bonk-fixup-improvements

Conversation

@penalosa
Copy link
Copy Markdown
Contributor

@penalosa penalosa commented Mar 31, 2026

Bonk was ignoring the /bonk comment that triggered it and defaulting to a generic PR review instead of acting on the specific request (e.g. "fix the formatting and commit").

Root cause: the on-demand workflow (bonk.yml) never injected the triggering comment body into the agent's context — unlike bonk-pr-review.yml which constructs and passes a prompt.

Changes:

  1. bonk.yml — new "Build prompt with triggering comment" step that injects github.event.comment.body (via env var to avoid injection) into the prompt: parameter passed to the ask-bonk action.

  2. bonk.md (on-demand agent) — restructured to prioritize the triggering comment:

    • New non-negotiable rule: "Triggering comment is the task" — read it first, before reading the PR
    • New non-negotiable rule: "No re-reviewing on fixup requests"
    • Implementation workflow now starts from the triggering comment and gathers only the context needed
    • New negative examples matching the exact failure mode observed on [vitest-pool-workers] Fix dynamic import() in entrypoint and DO handlers #13056
  3. bonk_reviewer.md (auto-review) — raised the LGTM threshold with an explicit "not actionable" list (style, lint-level issues, speculative perf, lockfile changes, etc.)

Open with Devin

Bonk was ignoring the triggering comment when invoked via /bonk, defaulting
to a generic PR review instead of acting on the specific request. Three changes:

1. Inject the triggering comment body into Bonk's context via the workflow
   prompt parameter, so Bonk actually sees what was asked.

2. Restructure the agent prompt to prioritize the triggering comment as the
   task definition, with new non-negotiable rules and a task-first
   implementation workflow.

3. Raise the auto-reviewer's LGTM threshold with an explicit not-actionable
   list to reduce noise on new PRs.
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Mar 31, 2026

⚠️ No Changeset found

Latest commit: 916d68b

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-project-automation github-project-automation bot moved this to Untriaged in workers-sdk Mar 31, 2026
@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk bot commented Mar 31, 2026

LGTM

github run

@penalosa penalosa marked this pull request as ready for review March 31, 2026 16:00
@penalosa penalosa requested a review from a team as a code owner March 31, 2026 16:00
@penalosa penalosa requested a review from emily-shen March 31, 2026 16:00
@workers-devprod
Copy link
Copy Markdown
Contributor

workers-devprod commented Mar 31, 2026

Codeowners approval required for this PR:

  • ✅ @cloudflare/wrangler
Show detailed file reviewers

Copy link
Copy Markdown
Contributor

@ask-bonk ask-bonk bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good. The comment body is correctly passed through an environment variable (COMMENT_BODY) to avoid script injection, which is the right pattern. The heredoc delimiter EOF is used both as the output boundary and appears literally in the echo — if a comment body ever contained a line that is exactly EOF on its own it would truncate the output early, but that's an edge case that's unlikely to matter in practice and is consistent with how the review workflow handles its prompt.

Two small notes (non-blocking):

  1. PR description mentions bonk_reviewer.md changes that aren't in the diff — the description says the LGTM threshold was raised with an explicit "not actionable" list, but no changes to .github/bonk_reviewer.md are included. Might be worth either adding those changes or updating the description.

  2. Line 49github.event.comment.user.login and github.event.comment.html_url are interpolated directly into the run: script via ${{ }} rather than being passed through env vars like COMMENT_BODY is. These are lower risk (login is alphanumeric, html_url is a GitHub URL), but for consistency with the injection-prevention approach used for the comment body, they could also be passed via env vars.

The agent instruction changes in bonk.md are well-structured — the "triggering comment is the task" rule and the reordered implementation workflow should fix the observed failure mode where Bonk ignored the /bonk comment and fell back to a generic review.

@ask-bonk
Copy link
Copy Markdown
Contributor

ask-bonk bot commented Mar 31, 2026

I've reviewed and approved PR #13147. Here's a summary:

Approval rationale: The core change is sound — the workflow now correctly passes the triggering comment body into the agent's prompt, and the agent instructions are restructured to prioritize acting on that comment rather than falling back to a generic review.

Two non-blocking observations I flagged:

  1. The PR description mentions changes to bonk_reviewer.md (raising the LGTM threshold) that aren't actually in the diff — either those changes are missing or the description needs updating.

  2. github.event.comment.user.login and github.event.comment.html_url are interpolated directly via ${{ }} in the run: block, while comment.body is correctly routed through an env var. These are low-risk fields, but using env vars for all user-controlled inputs would be more consistent with the injection-prevention approach.

github run

devin-ai-integration[bot]

This comment was marked as resolved.

@github-project-automation github-project-automation bot moved this from Untriaged to Approved in workers-sdk Mar 31, 2026
Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new bot commented Mar 31, 2026

create-cloudflare

npm i https://pkg.pr.new/create-cloudflare@13147

@cloudflare/kv-asset-handler

npm i https://pkg.pr.new/@cloudflare/kv-asset-handler@13147

miniflare

npm i https://pkg.pr.new/miniflare@13147

@cloudflare/pages-shared

npm i https://pkg.pr.new/@cloudflare/pages-shared@13147

@cloudflare/unenv-preset

npm i https://pkg.pr.new/@cloudflare/unenv-preset@13147

@cloudflare/vite-plugin

npm i https://pkg.pr.new/@cloudflare/vite-plugin@13147

@cloudflare/vitest-pool-workers

npm i https://pkg.pr.new/@cloudflare/vitest-pool-workers@13147

@cloudflare/workers-editor-shared

npm i https://pkg.pr.new/@cloudflare/workers-editor-shared@13147

wrangler

npm i https://pkg.pr.new/wrangler@13147

commit: 916d68b

@penalosa penalosa added the skip-pr-description-validation Skip validation of the required PR description format label Mar 31, 2026
@penalosa penalosa merged commit f31a6f4 into main Mar 31, 2026
54 of 56 checks passed
@penalosa penalosa deleted the penalosa/bonk-fixup-improvements branch March 31, 2026 16:13
@github-project-automation github-project-automation bot moved this from Approved to Done in workers-sdk Mar 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changeset-required skip-pr-description-validation Skip validation of the required PR description format

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants