Skip to content

Install repository OpenCode dependencies up front and accept mentions after opening punctuation - #231

Merged
danlapid merged 2 commits into
mainfrom
v0.5.2-fixes
Sep 28, 2026
Merged

danlapid merged 2 commits into
mainfrom
v0.5.2-fixes

Conversation

@danlapid

@danlapid danlapid commented Sep 28, 2026 •

Copy link
Copy Markdown
Collaborator

Two fixes found after v0.5.1. Two independent commits.

1. Install repository OpenCode dependencies before starting OpenCode
OpenCode installs each config directory's package.json dependencies in the background at startup, so a repository tool under .opencode/ that imports @opencode-ai/plugin could load first and fail the run with Cannot find module '@opencode-ai/plugin'; a retry on the same build succeeded. Before the main run, and before specialists start in their worktree at the PR head, Bonk now runs bun install in every OpenCode config directory (the checkout's .opencode/, the global config directory, OPENCODE_CONFIG_DIR) that has a package.json but no node_modules. It uses --frozen-lockfile when a bun lockfile exists and --no-save otherwise, so no lockfile is left in the checkout. It is bounded by a timeout, and a failure is only a warning.

2. Accept mentions after opening punctuation
The whole-word check required the start of the comment or whitespace before a mention, so Please review (@ask-bonk) started the workflow but the action skipped it. A mention may now also follow (, [, {, ", ' or <. Path and word characters and backticks before it still disqualify it, so .github/workflows/bonk.yml, a/bonk and `/bonk` do not match. The action's check and the prompt extraction change together.

278 tests pass; typecheck, lint and build are clean after each commit.

🤖 Generated with Claude Code

OpenCode installs the dependencies of each config directory with a
package.json in the background at startup. A repository whose
.opencode/ tools import @opencode-ai/plugin could have a tool load
before that install finished, failing the run with "Cannot find module
'@opencode-ai/plugin'"; a retry on the same build then succeeded.

Before the main run, and before specialists start in their worktree at
the pull request head, run bun install in every OpenCode config
directory (the checkout's .opencode/, the global config directory and
OPENCODE_CONFIG_DIR) that has a package.json but no node_modules. It
uses --frozen-lockfile when a bun lockfile exists and --no-save
otherwise, so no lockfile is left in the checkout. It is bounded by a
timeout, and a failure is only a warning since OpenCode still tries its
own install.
The whole-word mention check required the start of the comment or whitespace before a mention, so "Please review (@ask-bonk)" started the workflow but the action then skipped it. A mention may now also follow ( [ { " ' <. Path and word characters and backticks before it still disqualify it, so .github/workflows/bonk.yml, a/bonk and `/bonk` do not match.
@danlapid danlapid changed the title Install repository OpenCode dependencies before starting OpenCode Install repository OpenCode dependencies up front and accept mentions after opening punctuation Sep 28, 2026
@ask-bonk

ask-bonk Bot commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Review: 1 finding (1 warning; 1 note).

I checked all seven specialist findings against the code and kept two, both in github/script/context.ts:

  • Warning, line 218: the 2-minute install timeout doesn't actually stop the call. It kills only the bun process, but the code also waits for stderr to close. Scripts that bun started during the install keep stderr open after bun is killed. If one of them runs long, the install step never returns and the run hangs until the job timeout, with nothing logged.
  • Info, line 457: the comment says code spans don't trigger a run, but only a backtick right before the mention (`/bonk`) stops it. A mention after a space inside a code span still triggers Bonk. The same goes for the pattern in action.yml.

I dropped the other five findings and recorded a reason for each in the review file. tests#1, the unrestored console.log spy, can't affect other tests: it's in the last group of tests in the file, and the earlier groups restore their mocks. correctness#2 (installing when no specialists run) only does earlier the same install the main run would do anyway.

Other findings

  • [INFO] github/script/context.ts:457: The comment says code spans don't trigger a run. The regex only rejects a mention that comes directly after a backtick (`/bonk`). A mention after whitespace inside a code span still matches, for example Run `npx foo /bonk review` locally. The same goes for the bash pattern in action.yml. Consider narrowing the wording to "a mention wrapped directly in backticks".
Specialist findings not posted (5)
  • correctness#2 [SUGGESTION] github/script/specialists.ts:1138: When selection.selected is empty but selection.skipped is not (every specialist was carried forward), cwd falls back to process.cwd() and runPool launches nothing. The install still runs, th… — When no specialist is selected, cwd is the checkout, and the main run installs the same .opencode/ anyway. Installing earlier only moves that cost; later calls skip it because node_modules exists.
  • tests#1 [WARNING] test/github-script.spec.ts:1777: vi.spyOn(console, "log") is never restored. Vitest isn't configured with restoreMocks (vitest.config.mts only sets cloudflareTest), so console.log stays silenced for any test that runs after t… — This is the last describe in the file, and the earlier describes call vi.restoreAllMocks() in beforeEach/afterEach, so the unrestored spy can't affect other tests.
  • tests#2 [SUGGESTION] test/github-script.spec.ts:1757: Only bun.lock is tested as the lockfile that triggers --frozen-lockfile. The bun.lockb branch in installOpenCodeDependencies is never exercised, so a typo in that filename would go unnoticed. … — A coverage nit in a test file, below warning.
  • tests#3 [SUGGESTION] test/github-script.spec.ts:1739: The openCodeConfigDirs test passes both XDG_CONFIG_HOME and OPENCODE_CONFIG_DIR, so three paths go untested: the homedir()/.config fallback when XDG_CONFIG_HOME is unset, leaving out `OPENCO… — A coverage nit in a test file, below warning.
  • tests#4 [SUGGESTION] github/script/context.ts:210: bunInstall has no tests. That leaves untested the flag choice (--frozen-lockfile vs --no-save, which is what keeps a new lockfile out of a write-mode checkout), the error thrown on a non-zero ex… — bunInstall calls Bun.spawn, which the Workers Vitest pool can't run. The flag choice is a single line.

Reviewed commit: 83f35bd3 · github run

Comment thread github/script/context.ts
});
const timer = setTimeout(() => proc.kill("SIGKILL"), INSTALL_TIMEOUT_MS);
try {
const [exitCode, stderr] = await Promise.all([

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[WARNING] The 2-minute timeout doesn't actually cap this call. proc.kill("SIGKILL") kills only the bun process, but Promise.all also waits for new Response(proc.stderr).text(). That promise resolves only after every process holding the stderr pipe closes it. Lifecycle scripts that bun started (the .opencode/package.json root postinstall, or dependency scripts) inherit the pipe and keep running after bun is killed. If one of them runs long or stays in the background, installOpenCodeDependencies never returns, and the run hangs until the job timeout without logging anything. Await proc.exited first. Then read stderr with a short bound, for example by racing it against a timer or cancelling the reader after exit.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

🚀 Deploying Preview to Cloudflare 🚀

Preview Deployments by commit

Status Deployment URL Commit Updated (UTC) See this deployment's details
  • Build: Success ✅

View logs ↗
No Preview URL. Enable ↗ 83f35bd 2026-09-28T16:54:06.756Z View logs ↗

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
ask-bonk 83f35bd Sep 28 2026, 04:54 PM

@danlapid
danlapid merged commit 2c20fe3 into main Sep 28, 2026
5 checks passed
@danlapid
danlapid deleted the v0.5.2-fixes branch September 28, 2026 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant