Repository navigation
feat(plugin): OpenCode V2 support and reproducible bundling (#38, #43) - #44
Conversation
OpenCode V2 reads a default export with an `id` and `setup()`; the published package only exported a V1 function, which produced `PluginModule.LoadError` on load (issue #38). The build also inlined `@opencode-ai/plugin`, zod, and the SDK into a 0.55 MB bundle because the `--external:<name>` flag is a bun no-op (issue #43). - `skill-creator.ts`: one shared tool registry (zod shapes) drives both the V1 `server()` hooks and the V2 `id`/`setup()` entrypoint; V2 derives JSON Schema via `zod.toJSONSchema`, returns `{ content }`, and returns a cleanup function that stops active review servers. `setup()` overrides the eval conflict-guard enumerator with `ctx.skill.list`, so the guard now works on V2 instead of silently no-oping on the removed `opencode debug skill`. - `lib/run-eval.ts`: the conflict guard takes an injectable enumerator and aborts with an actionable error when enumeration is unavailable, rather than warn-and-continue into a contaminated eval. - `lib/review-server.ts`: stop SIGTERMing whatever holds the requested port; bind without killing other processes and fail loudly on EADDRINUSE. - `scripts/build.mjs`: use the working `--external=<name>` form for both SDK packages; the bundle now externalizes both and inlines no node_modules. - `bin/opencode-skill-creator.js`: migrate the installer to the V2 `plugins` key while preserving any legacy `plugin` array and object-form entries. - `package.json`: keep `@opencode-ai/plugin` as a required peer at the V2 documented floor (>= 1.18.29) and add `@opencode/plugin` as an optional peer; track `package-lock.json` for reproducible installs. - Workflows: `npm install` -> `npm ci`, stage the lockfile with the automatic version bump, and add a PR CI job using the existing npm + Bun suites. - Tests: real V2 runtime load + tool execution from the packed tarball, real V1 >= 1.18.29 floor load, installer key migration, eval subprocess harness, conflict-guard injection, review preflight, port-preservation, and pack manifest checks. Co-authored-by: sogeisetsu <47493432+sogeisetsu@users.noreply.github.com>
…03 gates
Stage 1 correction 1 (still no PR). Addresses lead inspection findings.
Changed-state correctness:
- Replace the module-global `activeSkillEnumerator` and `activeServers` map with
a per-instance object created in each V2 `setup()`. Two locations in one
process now keep independent enumerators, project roots and review servers,
and one instance's cleanup never closes another's servers. V1 keeps its
process-wide instance (cwd root, `opencode debug skill`) unchanged.
- V2 resolves the eval project root from `ctx.location.directory` (falling back
through `findProjectRoot`) instead of `process.cwd`, and `setup()` fails
loudly when the location directory is missing.
- `createV2SkillEnumerator` now scopes `ctx.skill.list({ location })` to the
instance's project root.
Test isolation (root cause of the stray global skill):
- Earlier `setup()`/`server()` unit tests ran without isolating HOME/XDG, so the
process-wide bundled-skill install wrote `~/.config/opencode/skills`. Wrap all
startup calls in `withPrivateHome`, disable the registry auto-update check,
and add a regression asserting the real config is byte-for-byte unchanged.
Tests:
- Real V2 runtime: add D-02 conflict end-to-end (installed-skill collision makes
`skill_eval` abort) and a clean-state eval against the isolated synthetic
skill with a controlled fake child.
- runEval harness: feed the captured real v2 `tool_use` event through the actual
parser; cover chunk boundaries, final buffer without newline, agent/model argv,
and cwd/PWD isolation. Drop the parser-logic-duplication constant test.
`skill_serve_review` always spawned `open <url>`, so automated tests and library callers opened the user's browser (many localhost review tabs) and never closed them. - `serveReview` already accepted `openBrowser`; make it authoritative and add `OPENCODE_SKILL_CREATOR_OPEN_BROWSER=0|false` as a process-level override. The interactive default stays true. Add an injectable `openBrowserImpl` launcher so tests can assert invocation without spawning anything. - `skill_serve_review` gains an explicit `openBrowser` argument (default true) and passes it through; tool description documents the opt-out. - Tests: set `OPENCODE_SKILL_CREATOR_OPEN_BROWSER=0` process-wide in the node suite, pass `openBrowser: false` in every automated review call, and add `test/browser-open.test.ts` proving no launcher invocation when disabled (and one when enabled) with no real browser. All suites tear down servers/temp dirs in `finally`. - Record the durable preference in the task worktree AGENTS.md: automated tests never open the browser; interactive QA reuses one Ego TaskSpace and closes only task-created pages; workers report cleanup.
🏹 Robin✅ Review done. I flagged 7 things worth a look. Push fixes whenever you like, then comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
🏹 Robin
Heads up: this is a point-in-time review. Push fixes freely, then comment
/robinwhenever you want another pass.
Summary
This change adds a CI workflow, migrates the installer and docs from the V1 plugin key to the V2 plugins key, replaces the destructive lsof | SIGTERM port cleanup with a loud EADDRINUSE error, adds a browser-open opt-out (option + env var), and splits the plugin into V1/V2 entrypoints with per-instance state and a shared tool registry. The refactor is clean, well-commented, and backed by new tests. The main concerns are an unhandled async spawn error in the browser launcher, a fresh-install gap for V1 users in the installer, and a few smaller consistency/test-hygiene issues.
Robin — the Robin Hood of code review. Free for every PR.
…l, harden test teardown Robin round 1 accepted fixes (PR #44). - review-server.ts: attach an `error` handler to the spawned launcher. A missing binary (headless Linux, `open` absent) emits an asynchronous 'error' event that the surrounding try/catch cannot catch; without a listener it crashed the process. The launcher now forwards async failures via an `onError` callback and the review server logs an actionable warning with the manual URL instead of dying. Interactive default behavior is unchanged. - test/browser-open.test.ts: capture `stop` outside the try and always await it in `finally`, with env restoration and workspace removal nested so they run even if `stop` throws. Adds a regression that points PATH at an empty dir, exercises the real `spawn("open", …)` ENOENT path, and asserts the process survives, the server still answers 200, and the manual URL is reported. No browser is launched. - README.md / plugin/README.md: state that the automatic installer targets OpenCode V2 only, and add an explicit supported-V1 (>= 1.18.29) manual singular `plugin` setup with restart. Removes the claim that the same automatic install works on the V1 floor; the installer is not changed to write both keys. Verification: build OK (113 KB, no inlined node_modules); npm test 46/46 (real V1 1.18.29 floor + real V2 runtime); test:ts 82/82; listeners 40->40; global config hashes unchanged; no browser opened.
|
/robin Round 1 triage complete. Fixed in 97a3e15 (on top of the reviewed 6db0a3f):
Rejected with factual evidence (unchanged): F3 (lock pins 1.18.33 ≥ floor; dev-range churn), F5 (no tool reads the context; V1/V2 contexts are different APIs), F6 (build manifest intentionally includes Verification on 97a3e15:
|
🏹 Robin✅ Review done. I flagged 9 things worth a look. Push fixes whenever you like, then comment |
There was a problem hiding this comment.
🏹 Robin
Heads up: this is a point-in-time review. Push fixes freely, then comment
/robinwhenever you want another pass.
Summary
This is a well-structured change: it splits the plugin into V1 (server()) and V2 (setup()) entrypoints from a single bundle, moves the installer to the V2 plugins key while preserving legacy plugin entries, replaces the dangerous lsof-based port killing with a loud EADDRINUSE error, adds a browser-open opt-out with a test seam, and makes the skill-conflict guard fail loudly instead of silently skipping. Comments and the new tests are generally good. The main gaps are that CI never verifies the committed dist/ matches a fresh build, the conflict guard now aborts evals whenever skill enumeration is unavailable, and the release process may ship these breaking changes as a patch bump.
Robin — the Robin Hood of code review. Free for every PR.
… clarify V1 install docs Robin round 2 accepted fixes (PR #44). - Version 0.3.0 (was 0.2.25): this line drops the function default export that OpenCode V1 < 1.18.29 required and the automatic installer now writes the V2 `plugins` key, so it is a compatibility-breaking change rather than a patch. Set in package.json; the lockfile is updated coherently (version-only, no dependency churn) and dist metadata is regenerated through `npm run build`. This is explicit version selection for the normal gated merge/automatic publisher — no manual npm publish, tag, or GitHub release. - README.md / plugin/README.md: document that older V1 (< 1.18.29) releases must pin the older 0.2.x line, and separate the V1 manual singular-`plugin` setup from the V2 installer's preservation of a pre-existing legacy `plugin` array. The earlier "do not add both keys" line now explains the upgrade exception and warns users to keep only the key their runtime uses. Verification: build OK (113 KB, 0 inlined deps); version coherent across package.json / package-lock.json / dist/package.json; npm pack dry-run ships 0.3.0; npm test 46/46 (real V1 1.18.29 floor + real V2 runtime); test:ts 82/82; listeners 40->40; global config unchanged; no browser opened.
|
/robin Round 2 triage complete. Fixed in 79791b1 (on top of 97a3e15):
Rejected with factual evidence (unchanged): R1 (sourceHash gate runs before rebuild; Verification on 79791b1:
|
🏹 Robin✅ Review done. I flagged 8 things worth a look. Push fixes whenever you like, then comment |
There was a problem hiding this comment.
🏹 Robin
Heads up: this is a point-in-time review. Push fixes freely, then comment
/robinwhenever you want another pass.
Summary
This change adds a CI workflow, migrates the installer to OpenCode V2's plugins key while preserving any legacy plugin array, replaces the dangerous lsof -ti | SIGTERM port-killing with a fail-loud bind, adds a browser-open opt-out with proper async error handling, and refactors the plugin into per-instance state with dual V1/V2 entrypoints. The refactor is well-structured, the docs are updated consistently with the behavior change, and the new browser-open tests cover the important paths. The main gaps are a CI step that does not actually verify the committed dist/ matches a fresh build, plus a few smaller consistency and test-isolation issues.
Robin — the Robin Hood of code review. Free for every PR.
1 participant
Summary
Adds OpenCode V2 plugin support and fixes the reproducible-bundling problem. Two linked issues:
PluginModule.LoadError: Plugin must export a default definition with an id and an effect or setup function.plugin/dist/skill-creator.jsrebuilds inline@opencode-ai/plugin+zod(~12k lines), swamping small PR diffs.What changed
V2 plugin entrypoint (#38)
plugin/skill-creator.tsnow exports a default definition withid+setup()alongside the existing V1server(). One shared tool registry (zod shapes) drives both entrypoints; V2 returns{ content }results and derives JSON Schema viazod.toJSONSchema, registering the full publicskill_*tool surface (14 tools).setup()installs actx.skill.listconflict-guard enumerator so the eval guard works on V2 instead of silently no-oping on the removedopencode debug skill. It resolves the eval/project root fromctx.location.directoryand returns a cleanup function that stops the review servers it started.Eval conflict guard
plugin/lib/run-eval.ts: the guard takes an injectable enumerator and aborts with an actionable error when enumeration is unavailable, rather than warn-and-continue into a contaminated eval.Build correctness (#43)
plugin/scripts/build.mjs:--external:<name>→ the working--external=<name>form for both SDK packages; the bundle now externalizes them and inlines nonode_modules. Dist drops ~0.55 MB → ~112 KB.plugin/package.json: required peer@opencode-ai/plugin >= 1.18.29, optional peer@opencode/plugin ^2.0.0, and a trackedpackage-lock.jsonfor reproducible installs.plugin/bin/opencode-skill-creator.js: installer writes the V2pluginskey while preserving any legacypluginarray and object entries.Safety / hygiene
plugin/lib/review-server.ts: no longer SIGTERMs whatever holds the requested port; binds without killing other processes and fails loudly onEADDRINUSE.skill_serve_reviewgains an explicitopenBrowserargument (defaulttrue) with anOPENCODE_SKILL_CREATOR_OPEN_BROWSER=0|falseprocess override, so automated callers/tests never open the user's browser.Workflows / tests / docs
.github/workflows/publish.yml:npm install→npm ci; stage the lockfile with the automatic version bump..github/workflows/ci.yml(new): PR/main CI on Node 22 + Bun —npm ci,npm test,npm run test:ts,npm run build,npm pack.>= 1.18.29floor, D-02 installed-skill conflict + clean state, eval subprocess harness, review preflight, busy-port preservation, per-instance isolation, and pack-manifest checks.README.md/plugin/README.md: compatibility table,pluginskey, and the V1/V2 floor.Testing
npm test— 46 pass / 0 fail (with a provisioned V1 floor binary; 40 + 1 skip without).npm run test:ts— 81 pass / 0 fail.npm run build— dist imports only@opencode-ai/plugin(external); 0 inlinednode_modules.PluginModule.LoadError), 14skill_*tools registered,skill_parse/skill_validateexecute from the packed tarball.opencode-ai@1.18.29: object entrypoint loads and lists the bundled skill.deepseek-v4.1-flash): positive trigger 1 / negative trigger 0,summary{total:2, passed:2, run_errors:0}.Limitations
ctx.skill.listconflict/clean-state path is proven end-to-end in the real runtime; the enumerator unit test is supporting wiring coverage.Credit
Builds on the V2 port proposed by @sogeisetsu in the fork (
sogeisetsu/feat/opencode-v2-support), adopted with corrections and attribution. Thanks for the detailed #38 report.Co-authored-by: sogeisetsu 47493432+sogeisetsu@users.noreply.github.com