Skip to content

fix(opencode): add V2 setup() to generated status plugin - #20458

Closed
donniesilalahi wants to merge 2 commits into
stablyai:mainfrom
donniesilalahi:opencode-v2-plugin-setup
Closed

donniesilalahi wants to merge 2 commits into
stablyai:mainfrom
donniesilalahi:opencode-v2-plugin-setup

Conversation

@donniesilalahi

Copy link
Copy Markdown

Problem

After migrating OpenCode V1 → V2, the generated orca-opencode-status.js no longer loads. The V2 plugin loader requires the default export to carry id plus setup() (or effect()); our file only exports { id, server }, so every OpenCode V2 session logs:

failed to load plugin … Plugin must export a default definition with an id and an effect or setup function.
(Missing key at ["default"]["effect"], Missing key at ["default"]["setup"])

Result: Orca receives no session busy/idle/message status from any OpenCode V2 agent (opencode plugin list shows the plugin as - instead of orca-opencode-status).

Fix

src/main/opencode/status-plugin-factory-source.ts — the generated module's default export is now { id, setup, server }:

  • setup(ctx) (V2 entrypoint) instantiates the existing V1 factory with a thin client facade (ctx.session.get adapted to the V1 { data } shape; empty list fallback so ancestry walks fail open), pumps ctx.event.subscribe() into the unchanged V1 event handler in order, and returns an abort + dispose() cleanup.
  • server() is untouched, so V1 keeps working from the same file.
  • Bridge for a real V2 event rename: question.asked/replied/rejected no longer exist (now form.created/replied/cancelled, confirmed against the V2 binary's event strings). setup() normalizes form.* to the question.* shapes the attention path expects; without this the dashboard never learns a question tool is waiting. All other consumed events (session.idle/status/created/error, message.*, permission.*) still exist in V2 and are forwarded as-is.

Tests

src/main/opencode/hook-plugin-module-contract.test.ts — existing V1 contract tests unchanged (they still pass: server is kept), plus:

  • default export exposes callable setup() alongside server(),
  • setup() over a mock V2 event stream posts SessionBusy through the hook endpoint,
  • form.created bridges to an AskUserQuestion post.

Verified locally against the assembled plugin: one stream produced SessionStart → SessionBusy → AskUserQuestion → SessionIdle (trailing idle is disposal publishing its final state), and opencode plugin list reports the plugin active.

Notes

  • The generated file uses require("fs") for the endpoint-file read; that still resolves under V2 (Bun provides require in ESM plugin scope) — no change needed there.
  • No changes to the hook payload contract; Orca server side is untouched.

@pullfrog pullfrog Bot left a comment

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.

Caution

src/main/opencode/status-plugin-factory-source.ts no longer parses: the new trailing array element was appended without a comma, so every test that imports hook-service.ts errors during transform before it can run. The V2 setup() approach itself looks sound once that is fixed.

Reviewed changes

  • V2 setup() entrypoint — the generated default export now carries { id, setup, server }; setup(ctx) instantiates the existing V1 factory behind a client facade and pumps ctx.event.subscribe() into the unchanged event handler.
  • form.* bridge — normalizeV2Event maps V2 form.created/replied/cancelled onto the question.asked/replied/rejected shapes the attention path already consumes.
  • Contract tests — new cases assert setup() is callable, drives SessionBusy, and posts AskUserQuestion for form.created.

⚠️ The pinned generated-source digest in hook-service.test.ts was not updated

Even after the missing comma is added, the keeps generated plugin bytes stable across the module split test fails: the generated source changed, so the pinned SHA-256 no longer matches. The author needs to recompute both pins.

Technical details
# Update the generated-source digest pins

## Affected sites
- `src/main/opencode/hook-service.test.ts:77` — pins `getOpenCodePluginSource()` to `d14859a36c88…`.
- `src/main/opencode/hook-service.test.ts:80-82` — pins `getOpenCodeFamilyPluginSource('/hook/mimo-code', { emitSessionStart: false })`; the factory source is shared, so this digest almost certainly changes too (the test never reached it because the first assertion aborts).

## Required outcome
- Recompute both `sha256` pins against the corrected generated source.
- Verified value for the primary source, after temporarily adding the missing comma: `51eb33931d60a4d74b83cf1153a159fd252fa445cef313ca1050bfc59f5297dc`.

## Suggested approach (optional)
`pnpm test src/main/opencode/hook-service.test.ts` prints the received digest on failure; copy both received values into the assertions.

## Open questions for the human
- Is the double trailing `''` element intentional (two tagged-template newlines) or was a single trailing newline intended? Either is fine, but confirm the generated file's trailing whitespace is what the loader should see.

Pullfrog  | Fix all ➔ | Fix 👍s ➔ | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

Comment on lines 323 to +324
''
''

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.

The pre-existing '' on line 323 was the last array element, so it had no trailing comma. Appending a second '' without adding that comma leaves two adjacent string literals, and the module stops parsing — oxlint reports Expected ',' or ']' but found 'string' at line 324, and pnpm test src/main/opencode/hook-service.test.ts fails at transform. Add the comma.

Suggested change
''
''
'',
''

@coderabbitai

coderabbitai Bot commented Sep 13, 2026 •

Copy link
Copy Markdown
Contributor

Review Change StackReview Change Stack

📝 Walkthrough

Walkthrough

The plugin contract now includes an optional V2 setup() entrypoint and PluginHooksCleanup type. The status factory keeps the V1 server() entrypoint and adds setup(). The adapter provides a V1-style client facade, normalizes V2 session and form events, forwards them to the existing plugin, and returns cleanup logic. Tests cover entrypoint exposure, session status handling, and form question handling.

Priority: ⬇️ Low

Merge Risk: 🔴 Critical · up to 1a23d

The updated OpenCode status plugin source contains a small syntax mistake that prevents the file from compiling, so the application build and the new plugin support cannot run at all until the stray empty string is removed.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description clearly explains the problem, implementation, compatibility behavior, event bridge, and tests. It does not follow the required template and omits the mandatory Linked Issue, Visual Pro… Use the repository template headings. Add a linked issue after “Fixes #”. Add “Visual Proof” with N/A and a reason if there is no UI change. Complete the Testing, AI Disclosure, Review, Agent skill upstream boundary, Notes, and Checklist se…
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the main change: adding V2 setup() support to the generated OpenCode status plugin.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Description check

Explanation

The description clearly explains the problem, implementation, compatibility behavior, event bridge, and tests. It does not follow the required template and omits the mandatory Linked Issue, Visual Proof or an N/A explanation, checklist items, and other required sections.

Resolution

Use the repository template headings. Add a linked issue after “Fixes #”. Add “Visual Proof” with N/A and a reason if there is no UI change. Complete the Testing, AI Disclosure, Review, Agent skill upstream boundary, Notes, and Checklist sections, including platform coverage and command results.

  • Fix all pre-merge checks with AI

Warning

Some tools did not complete. Review the errors below.

🔧 Biome (2.5.10)
src/main/opencode/status-plugin-factory-source.ts

File contains syntax errors that prevent linting: Line 324: expected , but instead found ''


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

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.

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: d966829e-1900-4d8a-81fe-742e123b0c21

📥 Commits

Reviewing files that changed from the base of the PR and between 9a12ccd and 1a23d3c.

📒 Files selected for processing (2)
  • src/main/opencode/hook-plugin-module-contract.test.ts
  • src/main/opencode/status-plugin-factory-source.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

' server: OrcaOpenCodeStatusPlugin,',
'};',
''
''

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.

🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win

Remove the duplicate empty string element.

Line 323 is already the final array element. Line 324 adds an adjacent string literal without a comma. TypeScript cannot parse this file.

Proposed fix
     '};',
     ''
-    ''
   ]
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
''
🧰 Tools
🪛 Biome (2.5.10)

[error] 324-324: expected , but instead found ''

(parse)

Source: Linters/SAST tools

@oliver-mee

Copy link
Copy Markdown
Contributor

This approach matches what 2.0.3 GA actually requires. I verified the loader schema firsthand today: a default export with id plus a setup() function loads clean, and ctx.event.subscribe({ signal }) with AbortController cleanup is the current subscription API, so the wrapper shape here should pass validation where the v1-only module fails.

One data point from the consumer side: I ran a minimal equivalent (setup plus subscribe pump, no facade) against 2.0.3 and it loads with zero errors across service restarts, including hot-reload on file change. The facade direction in this PR is strictly better since it keeps the full v1 feature set instead of reimplementing a subset.

Happy to test a preview build with the generated output on 2.0.3 if that helps move it along.

@nwparker

nwparker commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

Superseded by the generated V2 plugin and Form bridge now on main, including merged #24662. Credit to @donniesilalahi for the original setup/event-stream and question-attention work.

Verified with installed OpenCode 2.0.16: the pre-fix generated module fails the real loader; the current module loads. A genuine question tool creates a Form, the plugin sends busy and question details, and OpenCode consumes the actual reply before reporting idle and a successful session. Cancelling a second genuine Form returns HTTP 204 and produces the mapped idle hook before process cleanup.

The cancellation suite exited unsuccessfully because its extra check expected terminal session fields that never appeared. Cancellation’s durable session outcome remains unproved; the observed Form bridge is sufficient for this PR’s scope. Broader permission and child-session behavior remains tracked in #20410. Closing this older implementation because its loader and question-attention fixes are covered by merged code.

@nwparker nwparker closed this Oct 4, 2026
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.

3 participants