diff --git a/src/main/opencode/hook-plugin-opencode2-setup.test.ts b/src/main/opencode/hook-plugin-opencode2-setup.test.ts index 03e712820dd..e9e38334d3e 100644 --- a/src/main/opencode/hook-plugin-opencode2-setup.test.ts +++ b/src/main/opencode/hook-plugin-opencode2-setup.test.ts @@ -111,6 +111,61 @@ describe.each(['opencode', 'opencode2'] as const)('%s plugin on OpenCode 2', (ag await cleanup?.() }) + it('fails open when setup is probed without a usable context', async () => { + const module = await loadPluginModule( + agent === 'opencode2' + ? _internals.getOpenCode2PluginSource() + : _internals.getOpenCodePluginSource() + ) + // Why: OpenCode probes setup() during startup, and the setup API shape can + // drift between releases. A throw surfaces as a plugin failed error in the + // TUI, so every shape must resolve to a callable cleanup instead. + const contexts: unknown[] = [ + undefined, + {}, + { session: {} }, + { session: { hook: vi.fn() }, event: {} } + ] + for (const ctx of contexts) { + const cleanup = await module.default?.setup?.(ctx) + expect(cleanup).toBeTypeOf('function') + await cleanup?.() + } + }) + + it('disposes cleanly when the prompt hook returns nothing to dispose', async () => { + process.env.ORCA_PANE_KEY = 'tab-1:leaf-1' + const module = await loadPluginModule( + agent === 'opencode2' + ? _internals.getOpenCode2PluginSource() + : _internals.getOpenCodePluginSource() + ) + const cleanup = await module.default?.setup?.({ + session: { + get: async ({ sessionID }: { sessionID: string }) => ({ data: { id: sessionID } }), + hook: async () => undefined + }, + event: { + subscribe: async function* () {} + } + }) + expect(cleanup).toBeTypeOf('function') + await cleanup?.() + }) + + it('exposes a distinct plugin id per agent variant', async () => { + const module = await loadPluginModule( + agent === 'opencode2' + ? _internals.getOpenCode2PluginSource() + : _internals.getOpenCodePluginSource() + ) + // Why: both plugin files share one config dir, so distinct ids keep the + // loader from reporting a duplicate-id collision as a plugin failure. + expect(module.default?.id).toBe( + agent === 'opencode2' ? 'orca-opencode2-status' : 'orca-opencode-status' + ) + }) + it('subscribes through the OpenCode 2 setup API and disposes its registrations', async () => { process.env.ORCA_PANE_KEY = 'tab-1:leaf-1' const posts: unknown[] = [] diff --git a/src/main/opencode/hook-service.test.ts b/src/main/opencode/hook-service.test.ts index 46e75d2e8f8..277fcccf52b 100644 --- a/src/main/opencode/hook-service.test.ts +++ b/src/main/opencode/hook-service.test.ts @@ -22,6 +22,7 @@ const { getPathMock } = vi.hoisted(() => ({ import { OpenCodeHookService, + openCode2HookService, _internals, getOpenCodeFamilyPluginSource, getOpenCodePluginSource, @@ -92,7 +93,7 @@ describe('OpenCode hook plugin source', () => { const digest = (source: string): string => createHash('sha256').update(source).digest('hex') expect(digest(getOpenCodePluginSource())).toBe( - 'a43118afe856104629c968cc8adf43ced3ce85109977746e709bf4f391548fdf' + '609ae8b1fdf648e8023a1a55f2fb2038a44ca3a0d561021d2204814d48d1bb0b' ) expect( digest(getOpenCodeFamilyPluginSource('/hook/mimo-code', { emitSessionStart: false })) @@ -312,6 +313,31 @@ describe('OpenCodeHookService buildPtyEnv / clearPty round-trip', () => { expect(module.default?.setup).toBeTypeOf('function') }) + // Why: #22506 — both variants install side by side in one global plugins dir, and + // OpenCode 2 kills every plugin after the first that reuses an id ("Duplicate plugin + // ID"). Discovery sorts by path, so orca-opencode-status.js always wins and the + // opencode2 plugin never loads. Assert the installed files, not just the sources. + it('installs both family plugins into one config dir under distinct ids', async () => { + expect(new OpenCodeHookService().buildPtyEnv(daemonSessionId)).toEqual({}) + // #22440 Issue 2: the opencode2 variant must not shadow global config discovery either. + expect(openCode2HookService.buildPtyEnv(daemonSessionId)).toEqual({}) + + const pluginsDir = join(resolveOpenCodeConfigDirectory(), 'plugins') + const ids: string[] = [] + for (const fileName of ['orca-opencode-status.js', 'orca-opencode2-status.js']) { + // Why: a .mjs copy so Node parses the installed file as ESM without a package.json. + const modulePath = join(userDataDir, `installed-${fileName}-${Date.now()}.mjs`) + writeFileSync(modulePath, readFileSync(join(pluginsDir, fileName), 'utf8')) + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the assertions below validate the shape this names. + const module = (await import(pathToFileURL(modulePath).href)) as { + default?: { id?: unknown } + } + ids.push(String(module.default?.id)) + } + + expect(ids).toEqual(['orca-opencode-status', 'orca-opencode2-status']) + }) + it('clearPty leaves the shared OpenCode config dir off the teardown hot path', () => { const service = new OpenCodeHookService() service.buildPtyEnv(daemonSessionId) diff --git a/src/main/opencode/status-plugin-factory-source.ts b/src/main/opencode/status-plugin-factory-source.ts index fbd6d71a6d0..8348cc968eb 100644 --- a/src/main/opencode/status-plugin-factory-source.ts +++ b/src/main/opencode/status-plugin-factory-source.ts @@ -9,6 +9,10 @@ export function getStatusPluginFactorySource(options: { expectedAgent?: 'opencode' | 'opencode2' }): string[] { const expectedAgent = options.expectedAgent ?? (options.emitNextEvents ? 'opencode2' : 'opencode') + // Why: opencode and opencode2 share one config dir, so both plugin files load in + // either binary. Distinct ids keep the loader from reporting a duplicate-id + // collision as an 'orca-opencode-status' plugin failure. + const pluginID = expectedAgent === 'opencode2' ? 'orca-opencode2-status' : 'orca-opencode-status' return [ ...(options.emitNextEvents ? getOpenCode2EventNormalizationSource() : []), '// Why: accept the factory argument as an optional opaque parameter instead', @@ -285,7 +289,7 @@ export function getStatusPluginFactorySource(options: { '// export an object with server()"). `setup()` does not satisfy it. Keep the named', '// export so the factory-based loader still finds the same instance.', 'export default {', - ' id: "orca-opencode-status",', + ` id: "${pluginID}",`, ' server: OrcaOpenCodeStatusPlugin,', ...(options.emitNextEvents ? [' setup: setupOpenCode2Status,'] : []), '};', diff --git a/src/main/opencode2/status-plugin-setup-source.ts b/src/main/opencode2/status-plugin-setup-source.ts index 56648240fbd..b2576e7d294 100644 --- a/src/main/opencode2/status-plugin-setup-source.ts +++ b/src/main/opencode2/status-plugin-setup-source.ts @@ -9,67 +9,81 @@ export function getOpenCode2SetupSource(): string[] { const NON_SESSION_FORM_OWNERS = new Set(["global"]); async function setupOpenCode2Status(ctx) { - const controller = new AbortController(); - const client = { session: { get: (input, options) => ctx.session.get(input, options) } }; - const hooks = await OrcaOpenCodeStatusPlugin({ client }); - if (!hooks.event) return async () => {}; - const promptRegistration = await ctx.session.hook("prompt", async (properties) => { - await hooks.event({ event: { type: "session.next.prompt.admitted", properties } }); - }); - const consume = async () => { - for await (const input of ctx.event.subscribe({ signal: controller.signal })) { - if (controller.signal.aborted) break; - let type = input.type; - let properties = input.data; - if (type === "session.created") { - properties = { info: { ...properties, id: properties.sessionID } }; - } else if (type === "session.execution.started") { - type = "session.status"; - properties = { ...properties, status: { type: "busy" } }; - } else if (type === "session.execution.succeeded" || type === "session.execution.failed" || type === "session.execution.interrupted") { - type = "session.status"; - properties = { ...properties, status: { type: "idle" } }; - } else if (type === "permission.asked") { - properties = { ...properties, permission: properties.action, patterns: properties.resources }; - } else if (type === "form.created") { - const form = properties.form; - // Why: block on every form whose owner is a real session. "metadata" is - // optional in OpenCode's schema and its "kind" is a convention no - // producer is obliged to stamp, so an unknown shape must surface a - // blocker the user can clear rather than vanish while OpenCode waits. - if (!form || NON_SESSION_FORM_OWNERS.has(form.sessionID)) continue; - // A malformed form must not throw: that would kill the subscription. - const fields = Array.isArray(form.fields) ? form.fields : []; - type = "question.asked"; - properties = { - ...form, - questions: fields.map((field) => ({ - header: field.title || form.title, - question: field.description || field.title || form.title, - options: (field.options || []).map((option) => ({ label: option.label || option.value, description: option.description || "" })), - multiple: field.type === "multiselect", - })), - }; - } else if (type === "form.replied" || type === "form.cancelled") { - // A resolution for an ignored form is inert: the blocker key carries the - // form id, so it simply matches nothing. - type = type === "form.replied" ? "question.replied" : "question.rejected"; - properties = { ...properties, requestID: properties.id }; - } else if (type === "session.text.started" || type === "session.text.delta" || type === "session.text.ended") { - type = type.replace("session.", "session.next."); + const noop = async () => {}; + // Why: OpenCode may probe setup() with no context during startup, and the setup + // API shape can drift between releases. Never throw from setup — a throw surfaces + // as an 'orca-opencode-status' plugin failed error in the TUI, which is worse + // than silently running without status reporting. + try { + if (!ctx || typeof ctx.session?.hook !== "function" || typeof ctx.event?.subscribe !== "function") return noop; + const controller = new AbortController(); + const client = { session: { get: (input, options) => ctx.session.get(input, options) } }; + const hooks = await OrcaOpenCodeStatusPlugin({ client }); + if (!hooks || typeof hooks.event !== "function") return noop; + const promptRegistration = await ctx.session.hook("prompt", async (properties) => { + await hooks.event({ event: { type: "session.next.prompt.admitted", properties } }); + }); + const consume = async () => { + for await (const input of ctx.event.subscribe({ signal: controller.signal })) { + if (controller.signal.aborted) break; + let type = input.type; + let properties = input.data; + if (type === "session.created") { + properties = { info: { ...properties, id: properties.sessionID } }; + } else if (type === "session.execution.started") { + type = "session.status"; + properties = { ...properties, status: { type: "busy" } }; + } else if (type === "session.execution.succeeded" || type === "session.execution.failed" || type === "session.execution.interrupted") { + type = "session.status"; + properties = { ...properties, status: { type: "idle" } }; + } else if (type === "permission.asked") { + properties = { ...properties, permission: properties.action, patterns: properties.resources }; + } else if (type === "form.created") { + const form = properties.form; + // Why: block on every form whose owner is a real session. "metadata" is + // optional in OpenCode's schema and its "kind" is a convention no + // producer is obliged to stamp, so an unknown shape must surface a + // blocker the user can clear rather than vanish while OpenCode waits. + if (!form || NON_SESSION_FORM_OWNERS.has(form.sessionID)) continue; + // A malformed form must not throw: that would kill the subscription. + const fields = Array.isArray(form.fields) ? form.fields : []; + type = "question.asked"; + properties = { + ...form, + questions: fields.map((field) => ({ + header: field.title || form.title, + question: field.description || field.title || form.title, + options: (field.options || []).map((option) => ({ label: option.label || option.value, description: option.description || "" })), + multiple: field.type === "multiselect", + })), + }; + } else if (type === "form.replied" || type === "form.cancelled") { + // A resolution for an ignored form is inert: the blocker key carries the + // form id, so it simply matches nothing. + type = type === "form.replied" ? "question.replied" : "question.rejected"; + properties = { ...properties, requestID: properties.id }; + } else if (type === "session.text.started" || type === "session.text.delta" || type === "session.text.ended") { + type = type.replace("session.", "session.next."); + } + await hooks.event({ event: { type, properties } }); } - await hooks.event({ event: { type, properties } }); - } - }; - const consuming = consume().catch((error) => { - if (!controller.signal.aborted) console.warn("[orca-hook] event subscription failed:", error.message); - }); - return async () => { - controller.abort(); - await promptRegistration.dispose(); - await consuming; - await hooks.dispose(); - }; + }; + const consuming = consume().catch((error) => { + if (!controller.signal.aborted) console.warn("[orca-hook] event subscription failed:", error.message); + }); + return async () => { + try { + controller.abort(); + await promptRegistration?.dispose?.(); + await consuming; + await hooks.dispose?.(); + } catch { + // Why: cleanup runs during plugin unload; a throw here also fails the plugin. + } + }; + } catch { + return noop; + } } `.split('\n') }