You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Adds experimental OpenCode 2 plan review support while preserving the OpenCode 1 entrypoint and behavior. The package deliberately keeps V1 on main and exposes the V2 { id, setup } adapter from exports["."]: OpenCode 1 checks ./server before main, so adding that subpath would make V1 load the incompatible V2 adapter. Every V2 API touch stays in server.ts so future beta churn remains isolated.
Both adapters now share the host-independent submit_plan execution path for backing-file edits, browser review, approval/denial formatting, persistence, and cancellation semantics. The V2 adapter registers the JSON Schema tool, resolves cwd from the active session, applies planning/tool visibility through the session context hook, preserves ordered system parts and their metadata within V2's single system message, and leaves stable session URL output to the shared server path. URL deduplication is scoped to one review invocation so repeated remote reviews on the fixed port remain discoverable. V2 has command templates but no native slash-command execution hook, so /plannotator-* commands are intentionally not replaced with model-mediated behavior.
Current V2 limitations are documented in the package README: next-16775 exposes neither a tool abort signal nor a plugin session-agent switch operation, so cancellation cannot immediately stop an active review and users must switch to build manually after approval. The adapter passes no synthetic cancellation signal, keeping its CLI child attached to the host instead of creating a detached process that cannot be cancelled. These gaps are called out rather than presented as full V1 parity.
The implementation pins @opencode-ai/plugin@0.0.0-next-16775 and adds its five-package dependency family to Bun's minimum-release-age exclusions. Stable @opencode-ai/plugin@1.18.13 cannot be used because its V2 context lacks the required session and tool domains; #1196 tracks moving to stable and removing those exclusions once the APIs ship.
Validation includes the full repository suite (2802 passed, 0 failed), repository typecheck, both plugin bundles, and a packed-tarball smoke against the real opencode2 v0.0.0-next-16775; the smoke serves a temporary npm registry so OpenCode performs the package installation and root-entrypoint resolution itself, and runs as a dedicated CI job.
TLDR: needs changes, all small. No supply-chain compromise (checked hard given today's npm worm), V1 preserved byte-faithfully, and in two places where this PR deviates from the guidance on #904, the PR is right and the guidance was stale. Two blocking items: a never-firing AbortSignal that orphans detached review processes, and a PR-body claim about a CI job that does not exist. Two disclosures needed: the bunfig age-gate widening and the deliberate exports-map shape.
Detail (AI review findings, skim as needed):
You were right and our issue guidance was stale, twice. At your pinned next-16770 the root export is genuinely { id, setup }; the { id?, server } shape we cited from the 2.0 branch is demoted to the legacy /v1 subpath, and the V2 loader schema requires a default export with required id, which server.ts matches exactly. Our notes on the abort signal and zod tool registration were likewise behind your pinned build; your documented limitations are honest against 16770. Second, your exports map ("." pointing at the V2 server with main at the V1 index) looks wrong and is the only arrangement that satisfies both loaders; the ./server boundary approved on the issue would have silently broken OpenCode 1, which checks ./server first. Good deviation. It needs a comment in package.json and a line in package-boundary.test.ts saying why, or a future contributor will "fix" it back to the approved-but-broken shape.
Blocking 1: the synthetic AbortSignal orphans processes.server.ts:190 passes new AbortController().signal that can never fire; cli-bridge.ts:347 treats any defined signal as reason to spawn detached, so quitting OpenCode 2 leaves a review server running for up to 96 hours holding its port, and detached means it no longer dies with the host. Pass undefined instead; that is strictly better until V2 exposes real cancellation.
Blocking 2: the PR body says the V2 smoke "now runs as a dedicated CI job." Zero workflow files changed; the fixture is real and excellent (temp registry, packed tarball, real opencode2 binary polling for the plugin id) but nothing runs it. Either add the job or correct the claim; the regression class this PR exists to fix currently has no automated guard.
Disclosure needed: bunfig.toml is the undisclosed thirteenth file. It widens the minimum-release-age excludes by five scopes, without which the install hard-fails, and the repo's own update-deps procedure requires documenting exactly this. Please: disclose it in the body, and strongly consider pinning stable @opencode-ai/plugin@1.18.13 for the published production dependency instead of a 2.4-hour-old nightly that next had already moved past at PR time; if the nightly is genuinely required for the V2 API, a tracking issue to remove the five excludes when a stable release covers it.
Verified clean: supply chain (all five nightlies published via npm OIDC by the same accounts as the stable line, registry signatures 107/107, zero lifecycle scripts, zero keyv/worm-adjacent packages in the entire closure, tarball is plain tsc output); V1 byte-preservation (executor is a line-for-line lift, commands.ts/cli-bridge.ts untouched so the #1183 and #1184 contracts hold by construction, four new executor tests); churn isolation real; the V2 single-system-part collapse correct; the agent-switch limitation verified honest against the V2 session domain; full suite 2803 pass / 0 fail; packing clean; merges fast-forward; no em dashes.
Nits for the same pass or follow-ups: resolveTargetAgent always returns undefined after a full lookup; ctx.agent.list() drops the directory query V1 passed; hooks register no cleanup; apps/opencode-plugin has no tsconfig so the new files are not CI-typechecked (a forced strict pass is clean, and surfaces four pre-existing V1 app.log shape errors worth a look someday). Separate follow-up for us, not you: plannotator uninstall and install.sh only know the V1 config key, so V2 configs will not be cleaned up; fine while V2 install is manual.
Genuinely strong first contribution. Fix the two blockers, add the two disclosures, and this merges.
Addressed in 7282b746 (with the CI blocker already fixed in f9cc77db, after the reviewed 4d4cdb3a revision):
Removed the synthetic AbortSignal end-to-end. The shared executor and review adapters now accept an absent host signal, and the V2 adapter passes none, so the CLI child is not detached. OpenCode 1 still passes its real tool cancellation signal unchanged.
The packed-package smoke is wired into .github/workflows/test.yml as the dedicated opencode-v2 job added in f9cc77db.
Documented the deliberate main/exports["."] compatibility boundary in package.json and package-boundary.test.ts, including why ./server must remain absent.
Documented the five OpenCode release-age exclusions in bunfig.toml and disclosed them in the PR body.
Checked stable @opencode-ai/plugin@1.18.13: its /v2/promise context lacks the session and tool domains this adapter requires, so it cannot replace next-16770. chore(opencode): move V2 adapter to the stable plugin API #1196 tracks moving to stable and removing all five exclusions once those APIs ship.
Validation after the changes: 107 OpenCode plugin tests pass, repository typecheck passes, the V1/V2 package build passes, and the newly packed tarball activates as plannotator under the exact opencode2 v0.0.0-next-16770 binary.
TLDR: the four original asks are resolved and verified. Two new small defects arrived with the fixes; both are precisely isolated and quick. Fix those and this merges.
Detail (AI review findings, skim as needed):
1. The new system-part test is config-sensitive. The reworked per-addition parts made the test's part count depend on the user's real Plannotator config: on a machine with pfmReminder: true the suite runs 106 pass / 1 fail, on a clean environment 107 / 0. Bisected definitively: reverting only server.{ts,test.ts} to the previously reviewed head on the same machine returns 5/0, so the sensitivity is PR-introduced (the old single-collapsed-part assertion was config-insensitive by construction). Your "107 tests pass" claim was honest, for the record; your environment just does not have the config that bites. Fix: isolate the test from host config (point the data dir or config at a fixture) so the assertion is deterministic everywhere, CI included.
2. One new strict type error at index.ts:554. Isolated by differential tsc: 15 pre-existing SDK-typing drift errors with your three modified files reverted, 16 at HEAD; the delta is exactly that line. One-line fix.
Also verified on this head: full suite 2802 pass with only the known config-sensitive failure above, builds and pack clean (10 files, exact expected names), zero merge conflicts, the "//" comment key in package.json produces no npm warnings, no em dashes.
One supply-chain note for the record, not a change request:dist/server.js correctly keeps @opencode-ai/plugin external, but dist/index.js bundles the /v1 runtime inline from the pinned nightly at publish time (confirmed via the build banner). The /v1 helper is byte-identical to stable 1.1.10's and both pin the same zod, so behavior is preserved; noting it so the tracking issue (#1196) migration also revisits what gets baked into the V1 bundle.
server.test.ts now points PLANNOTATOR_DATA_DIR at an empty temporary directory before dynamically importing the V2 adapter, then restores the environment and removes the fixture. I also ran that test with an external pfmReminder: true config; all 5 tests pass.
The V1 host now passes its known real context.abort signal into runPlanReview. The differential strict check is back to the 15 pre-existing SDK drift errors, with no error at index.ts:554.
Updated the plugin dependency, lockfile family, CI CLI, and both local OpenCode launchers to the registry-current matching next-16775 release. The latest API still has no tool abort signal or session-agent switch operation.
Clarified the README language: V2 supports command templates, but has no native slash-command execution hook.
Final validation: full suite 2802 passed / 0 failed, repository typecheck passed, workflow YAML parsed, both plugin bundles built, the package contained the expected 10 files, and the packed tarball installed and activated as plannotator under local opencode2 v0.0.0-next-16775 through the temporary npm registry.
TLDR: the type fix landed and is better than what we suggested. The test isolation fix cannot work as built, through no fault of the attempt: the config path is frozen at module load, so the env var only wins if your test file happens to load first. One small change and this merges.
Detail (AI review findings, skim as needed):
The test fix:packages/shared/config.ts evaluates the config path as a module-level constant the first time any file imports it, and sibling test files' static imports run in the same load phase, so setting PLANNOTATOR_DATA_DIR at your file's top level is defeated whenever any other file touches the config module first. Verified: your file alone passes 5/0, but paired with cli-bridge.test.ts it fails in both orderings, so it is presence-fragile rather than order-fragile. Your env restore is sound, for the record; nothing leaks. Two fixes that hold: mock the config module (mock.module("@plannotator/shared/config", ...)), or, simpler and truer to what the test protects, drop the exact part count and assert the invariant instead: both host parts survive with their cache and metadata, and some part starts with ## Plannotator. That version is immune to pfmReminder and to any installed improvement hook. We recommend the second.
The type fix: passing context.abort directly instead of the destructured variable is cleaner than the annotation we had in mind, keeps V1 behavior identical, and differential tsc confirms the error count returns exactly to baseline.
The pin bump 16770 to 16775 (not requested, re-audited anyway): clean and a net improvement. Every API surface the adapter touches is byte-identical between the two nightlies, all documented limitations remain accurate, same OIDC trusted publisher, zero lifecycle scripts, lockfile delta is exactly the five packages with no collateral movement this time, and 16775 is the current next tag so the pin is no longer superseded.
Note for merge timing: main has advanced tonight, so this now lands as a real merge rather than a fast-forward; still zero conflicts.
Addressed in 4e5ba14a using the recommended invariant-based test:
Removed the environment mutation, temporary directory, and dynamic import.
Preserved the assertions that both host system parts survive in order with their metadata and cache fields.
Replaced exact part counts and fixed indexes with an assertion that some system part starts with ## Plannotator.
Made the stripped-system assertion config-independent as well, so a PFM or improvement-hook part cannot become the next failure.
Reproduced the original failure first with cli-bridge.test.ts plus server.test.ts under pfmReminder: true (15 passed / 1 failed). After the change, the paired run passes 16/0 in both supplied file orderings, and the complete OpenCode plugin suite passes 107/0 under the same PFM-enabled config.
TLDR: merge as-is. All four asks resolved. The invariant-based test is deterministic in every arrangement that previously broke, and a mutation test proves it still catches the regression the old brittle assertion guarded.
Detail: full suite 2803 pass / 0 fail on the environment that reproduced the config sensitivity; the presence-fragility repro (paired with cli-bridge.test.ts, both orders) passes everywhere; reintroducing the pre-PR single-part collapse fails the new test under both real and clean configs, so the protection is structural, and ordering remains pinned. The one dimension no longer asserted is the exact part count, which was precisely the config-dependent quantity; correct trade. Single surgical commit, zero em dashes, merges onto current main with no conflicts.
Follow-ups filed mentally for the maintainer, none blocking this merge: the new opencode-v2 CI job hardcodes the 0.25.1 tarball filename and will silently break at the next version bump (derive it instead); apps/opencode-plugin has no tsconfig and sits outside the repo typecheck, which is how the earlier AbortSignal type regression reached review at all; #1196 already tracks the stable-API migration and bunfig exclusion removal.
Strong work across five rounds: proposal before code, correct deviations from stale guidance twice, a self-filed tracking issue, and every fix landing cleanly or better than asked.
Merged. Thank you for an outstanding first contribution: the dual-entrypoint design survived five review rounds intact, corrected our own stale guidance twice, and ships with V1 behavior byte-preserved. OpenCode 2 support is experimental per the README until the stable plugin API lands; #1196 tracks that migration. This will go out with the next release.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds experimental OpenCode 2 plan review support while preserving the OpenCode 1 entrypoint and behavior. The package deliberately keeps V1 on
mainand exposes the V2{ id, setup }adapter fromexports["."]: OpenCode 1 checks./serverbeforemain, so adding that subpath would make V1 load the incompatible V2 adapter. Every V2 API touch stays inserver.tsso future beta churn remains isolated.Both adapters now share the host-independent
submit_planexecution path for backing-file edits, browser review, approval/denial formatting, persistence, and cancellation semantics. The V2 adapter registers the JSON Schema tool, resolves cwd from the active session, applies planning/tool visibility through the session context hook, preserves ordered system parts and their metadata within V2's single system message, and leaves stable session URL output to the shared server path. URL deduplication is scoped to one review invocation so repeated remote reviews on the fixed port remain discoverable. V2 has command templates but no native slash-command execution hook, so/plannotator-*commands are intentionally not replaced with model-mediated behavior.Current V2 limitations are documented in the package README:
next-16775exposes neither a tool abort signal nor a plugin session-agent switch operation, so cancellation cannot immediately stop an active review and users must switch tobuildmanually after approval. The adapter passes no synthetic cancellation signal, keeping its CLI child attached to the host instead of creating a detached process that cannot be cancelled. These gaps are called out rather than presented as full V1 parity.The implementation pins
@opencode-ai/plugin@0.0.0-next-16775and adds its five-package dependency family to Bun's minimum-release-age exclusions. Stable@opencode-ai/plugin@1.18.13cannot be used because its V2 context lacks the requiredsessionandtooldomains; #1196 tracks moving to stable and removing those exclusions once the APIs ship.Validation includes the full repository suite (
2802passed,0failed), repository typecheck, both plugin bundles, and a packed-tarball smoke against the realopencode2 v0.0.0-next-16775; the smoke serves a temporary npm registry so OpenCode performs the package installation and root-entrypoint resolution itself, and runs as a dedicated CI job.Refs #904