diff --git a/apps/opencode-plugin/README.md b/apps/opencode-plugin/README.md index 6206cdd0..20fcab34 100644 --- a/apps/opencode-plugin/README.md +++ b/apps/opencode-plugin/README.md @@ -40,12 +40,13 @@ Install OpenCode 2 from npm's `next` tag, then add Plannotator to the V2 `plugin Restart OpenCode 2 and verify that `plannotator` appears in `opencode2 plugin list`. -OpenCode 2 support is experimental while its plugin API is in beta. The core `submit_plan` review flow works, but the current API has these limitations: +OpenCode 2 support is experimental while its plugin API is in beta. The core `submit_plan` review flow works everywhere. Two newer capabilities depend on which plugin API your OpenCode build ships with, and Plannotator detects both at runtime rather than requiring a particular channel: -- OpenCode 2 does not expose a native slash-command execution hook. Its command definitions expand to model prompts, so `/plannotator-review`, `/plannotator-annotate`, and `/plannotator-last` remain OpenCode 1-only instead of silently becoming model-mediated commands. -- V2 tool execution does not expose an abort signal. Cancelling a turn cannot yet stop a running review server or CLI child immediately. -- The V2 plugin context cannot switch the active session agent. Agent switching selected in the review UI is ignored with a server-log warning; switch to `build` manually after approval before implementation. -- The V2 plugin context has no TUI toast/log API, so remote session URLs are written to the server output rather than shown as a toast. +- **Slash commands.** Native command execution landed upstream in `@opencode-ai/plugin` (anomalyco/opencode issue #2185, PR #44765) and currently ships on the `beta` and `dev` dist-tags; the `next` and `latest` tags still carry the older API. Capability is detected from the command draft OpenCode hands the plugin, not from the plugin API's shape: `ctx.command.transform` exists on both generations, and only the newer draft has `add`. On a host that has it, Plannotator registers `/plannotator-review`, `/plannotator-annotate`, and `/plannotator-last` itself and runs the same machinery OpenCode 1 uses, so your raw arguments reach the CLI unchanged and nothing is routed through the model. On an older host it registers nothing and the commands run from their markdown definitions, which ask the agent to run the `plannotator` CLI and relay its output; that path works but costs a model turn and depends on the agent following the instruction. +- **Command precedence.** OpenCode activates its own config-command loader after package plugins, and the last definition to claim a name wins, so the markdown stubs the installer writes to `~/.config/opencode/commands` would otherwise shadow the native definitions on every normal install. Plannotator re-registers the three names shortly after startup so its own definitions are the ones that run. If that reclaim cannot run, the stubs keep the names and the commands still work through the model-mediated fallback. +- **Agent switching.** `ctx.session.switchAgent` arrived with the same plugin API generation. On a host that exposes it, an agent switch chosen in the review UI is applied to the session. On an older host the plan is still approved and a warning is written to the server log; switch to `build` manually before implementation. +- **Abort signal.** V2 tool execution still exposes no abort signal. Cancelling a turn cannot stop a running review server or CLI child immediately. +- **TUI toasts.** OpenCode 2 has a TUI plugin entry point, but it is separate from the server plugin Plannotator registers, so session URLs are written to the server output rather than shown as a toast. Remote sessions should read the URL from the OpenCode log. ### OpenCode 1 @@ -68,7 +69,7 @@ Restart OpenCode. By default, the `submit_plan` tool is available to OpenCode's ## Workflow Modes -The examples below use the OpenCode 1 config shape. OpenCode 2 places the same option keys under the plugin entry's `options` object shown above. In V2, `manual` intentionally registers no tool and native slash-command handlers are unavailable, so it currently leaves the integration inactive. +The examples below use the OpenCode 1 config shape. OpenCode 2 places the same option keys under the plugin entry's `options` object shown above. In V2, `manual` registers no tool, so it leaves only the slash commands: useful on a host with native command execution, inactive on one without it. - **`plan-agent`** (default): `submit_plan` is available to OpenCode's built-in `plan` agent plus any extra agents listed in `planningAgents`. This keeps Plannotator integrated with OpenCode plan mode without nudging `build` to call it. - **`manual`**: `submit_plan` is not registered. Use `/plannotator-last`, `/plannotator-annotate`, and `/plannotator-review` when you want Plannotator. diff --git a/apps/opencode-plugin/agent-switch.ts b/apps/opencode-plugin/agent-switch.ts index 6812819f..e0c525ba 100644 --- a/apps/opencode-plugin/agent-switch.ts +++ b/apps/opencode-plugin/agent-switch.ts @@ -1,3 +1,5 @@ +import { supportsSwitchAgent, type V2ContextLike } from "./v2-client"; + export interface OpenCodeAgentLike { name?: string; } @@ -71,3 +73,44 @@ export async function resolveValidatedTargetAgent(input: { warnAgentUnavailable(input.client, targetAgent, input.delivery ?? "feedback"); return undefined; } + +/** + * OpenCode 2 agent switch. + * + * `ctx.session.switchAgent` arrived with the same plugin-API generation as + * native command execution, so it is duck-typed rather than imported: on a host + * without it the plan is still approved and the caller is told the switch was + * skipped. Returns the agent actually switched to, or undefined when the + * session's agent was left alone. + */ +export async function switchV2SessionAgent(input: { + ctx: V2ContextLike; + sessionID: string; + requestedAgent?: string; + getAgents: () => Promise; + warn?: (message: string) => void; +}): Promise { + const warn = input.warn ?? ((message: string) => console.error(message)); + const targetAgent = resolveTargetAgent(input.requestedAgent); + if (!targetAgent) return undefined; + + const available = (await input.getAgents()).some((agent) => agent.name === targetAgent); + if (!available) { + warn(`[Plannotator] Configured OpenCode agent "${targetAgent}" is not available; approving the plan without switching agents.`); + return undefined; + } + + if (!supportsSwitchAgent(input.ctx)) { + warn("[Plannotator] This OpenCode 2 host does not expose agent switching to plugins; approving the plan without switching agents."); + return undefined; + } + + try { + await input.ctx.session!.switchAgent!({ sessionID: input.sessionID, agent: targetAgent }); + } catch (error) { + warn(`[Plannotator] Could not switch the OpenCode session to "${targetAgent}": ${error instanceof Error ? error.message : String(error)}`); + return undefined; + } + + return targetAgent; +} diff --git a/apps/opencode-plugin/command-interception.test.ts b/apps/opencode-plugin/command-interception.test.ts new file mode 100644 index 00000000..c0a1b7d1 --- /dev/null +++ b/apps/opencode-plugin/command-interception.test.ts @@ -0,0 +1,91 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { createTestEnvironment } from "../../tests/helpers/environment"; +import PlannotatorPlugin from "./index"; + +/** + * OpenCode 1 slash-command interception. + * + * The V1 plugin clears `output.parts` IN PLACE before anything reaches the + * model. Now that the shared markdown stubs carry real instructions ("run the + * plannotator CLI and relay stdout", for OpenCode 2 hosts on the stale + * channels), a regression here would leak those instructions to the OpenCode 1 + * model and re-open the #713 class: OpenCode resolves prompt parts over + * " " and auto-attaches any file path it finds, which on a + * large file blows the context before the annotation UI even opens. + * + * Interception lives on the always-built plugin object; `shouldRegisterSubmitPlan` + * only gates `plugin.tool`, so `workflow: "manual"` must intercept too. + */ + +const envKeys = ["PLANNOTATOR_BIN", "PLANNOTATOR_DATA_DIR"] as const; +const environment = createTestEnvironment(envKeys, "plannotator-oc1-intercept-"); + +afterEach(() => environment.restore()); + +const COMMANDS = ["plannotator-review", "plannotator-annotate", "plannotator-last"] as const; + +function makeClient() { + return { + app: { + log: async () => ({}), + agents: async () => ({ data: [] }), + }, + config: { get: async () => ({ data: {} }) }, + session: { + messages: async () => ({ data: [] }), + prompt: async () => ({}), + }, + }; +} + +async function interceptionHandler(options: Record) { + const plugin = await PlannotatorPlugin( + { client: makeClient(), directory: "/project" } as never, + // "cli" keeps the embedded server out of the test; the CLI spawn then fails + // fast against the bogus PLANNOTATOR_BIN below and is swallowed by + // handleCliCommand's own catch. + { runtime: "cli", ...options } as never, + ); + return (plugin as Record)["command.execute.before"] as ( + input: Record, + output: { parts: unknown[] }, + ) => Promise; +} + +describe("OpenCode 1 command interception", () => { + for (const workflow of ["plan-agent", "manual"] as const) { + for (const command of COMMANDS) { + test(`${workflow}: /${command} empties output.parts before the model sees it`, async () => { + environment.reset(); + process.env.PLANNOTATOR_BIN = "/nonexistent/plannotator-interception-test"; + process.env.PLANNOTATOR_DATA_DIR = environment.makeTempDir(); + + const handler = await interceptionHandler({ workflow }); + const parts = [{ type: "text", text: "run the plannotator CLI and relay stdout" }]; + const output = { parts }; + + await handler( + { command, sessionID: "session-1", arguments: "" }, + output, + ); + + expect(parts.length).toBe(0); + // Mutated in place, never reassigned: the caller holds this exact array + // and ignores anything assigned to output.parts. + expect(output.parts).toBe(parts); + }); + } + } + + test("an unrelated command keeps its parts untouched", async () => { + environment.reset(); + process.env.PLANNOTATOR_BIN = "/nonexistent/plannotator-interception-test"; + process.env.PLANNOTATOR_DATA_DIR = environment.makeTempDir(); + + const handler = await interceptionHandler({ workflow: "plan-agent" }); + const output = { parts: [{ type: "text", text: "someone else's command" }] }; + await handler({ command: "other-command", sessionID: "session-1", arguments: "" }, output); + + expect(output.parts.length).toBe(1); + }); +}); diff --git a/apps/opencode-plugin/commands/plannotator-annotate.md b/apps/opencode-plugin/commands/plannotator-annotate.md index ae16d4a2..901a51b4 100644 --- a/apps/opencode-plugin/commands/plannotator-annotate.md +++ b/apps/opencode-plugin/commands/plannotator-annotate.md @@ -1,3 +1,9 @@ --- description: Open interactive annotation UI for a file, folder, or URL --- + +Run `plannotator annotate $ARGUMENTS` with Bash, in the foreground, and wait for it to finish. + +Relay its stdout to the user. If annotations come back, address them now. If the command reports that the arguments could not be resolved to a file, URL, or folder, work out which target the user meant and re-run it with that concrete path. + +Do not ask the user to run the command themselves. diff --git a/apps/opencode-plugin/commands/plannotator-last.md b/apps/opencode-plugin/commands/plannotator-last.md index 8001428b..61554ffe 100644 --- a/apps/opencode-plugin/commands/plannotator-last.md +++ b/apps/opencode-plugin/commands/plannotator-last.md @@ -1,3 +1,9 @@ --- description: Annotate the last assistant message --- + +Run `plannotator last $ARGUMENTS` with Bash, in the foreground, and wait for it to finish. Send no message before running it: the command targets the latest rendered assistant response, so a preamble becomes the thing being annotated. + +Relay its stdout to the user and carry any returned feedback into your next response. An approval can still carry notes; treat those as guidance, not a change request. + +Do not ask the user to run the command themselves. diff --git a/apps/opencode-plugin/commands/plannotator-review.md b/apps/opencode-plugin/commands/plannotator-review.md index 7253f9de..c21f58e9 100644 --- a/apps/opencode-plugin/commands/plannotator-review.md +++ b/apps/opencode-plugin/commands/plannotator-review.md @@ -1,3 +1,9 @@ --- description: Open interactive code review for current changes or a PR URL; pass --git or --gitbutler to force that provider --- + +Run `plannotator review $ARGUMENTS` with Bash, in the foreground, and wait for it to finish. + +Relay its stdout to the user. If it returns feedback or annotations, address them now. If it returns an approval, say the review passed and continue. + +Do not ask the user to run the command themselves. diff --git a/apps/opencode-plugin/fixtures/v2-installed-smoke.ts b/apps/opencode-plugin/fixtures/v2-installed-smoke.ts index 05324d43..d98f2375 100644 --- a/apps/opencode-plugin/fixtures/v2-installed-smoke.ts +++ b/apps/opencode-plugin/fixtures/v2-installed-smoke.ts @@ -1,7 +1,8 @@ -import { mkdirSync, mkdtempSync, readFileSync, rmSync } from "node:fs"; +import { copyFileSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import path from "node:path"; import { createServer } from "node:net"; +import { NATIVE_COMMANDS } from "../native-commands"; const [opencodeBin, pluginTarball] = Bun.argv.slice(2); if (!opencodeBin || !pluginTarball) { @@ -36,6 +37,22 @@ mkdirSync(path.join(root, "config"), { recursive: true }); mkdirSync(path.join(root, "data"), { recursive: true }); mkdirSync(path.join(root, "cache"), { recursive: true }); +// Reproduce a NORMAL install: scripts/install.sh (and the package postinstall) +// write the three markdown stubs into ~/.config/opencode/commands, which is +// exactly where OpenCode 2's own ConfigCommandPlugin scans. Without them the +// sandbox is a shape no user has, and the contest between the config-loaded +// stubs and the plugin's native definitions never happens here. +const stubsDir = path.join(root, "config", "opencode", "commands"); +mkdirSync(stubsDir, { recursive: true }); +const stubSource = path.join(import.meta.dir, "..", "commands"); +for (const entry of readdirSync(stubSource)) { + if (entry.endsWith(".md")) copyFileSync(path.join(stubSource, entry), path.join(stubsDir, entry)); +} + +// Set when the host is known to ship the post-#44765 command API (the `beta` / +// `dev` channels). CI runs a `next` build, where the stubs legitimately win. +const expectNativeCommands = process.env.PLANNOTATOR_SMOKE_EXPECT_NATIVE === "1"; + const env = { ...process.env, XDG_CONFIG_HOME: path.join(root, "config"), @@ -111,6 +128,7 @@ let failed = false; try { await waitForHealthyServer(url); const plugins = await waitForPlugin(url); + await checkCommands(url); console.log(JSON.stringify(plugins)); } catch (error) { failed = true; @@ -203,10 +221,32 @@ async function waitForPlugin(url: string): Promise { if (!httpResponse.ok) { throw new Error(`OpenCode plugin API returned ${httpResponse.status}: ${lastOutput}`); } - const response = JSON.parse(lastOutput) as { data?: Array<{ id?: string } | string> }; - if (response.data?.some((plugin) => + const response = JSON.parse(lastOutput) as { + data?: Array< + | { + id?: string; + status?: string; + error?: string; + state?: { status?: string; error?: string }; + } + | string + >; + }; + const entry = response.data?.find((plugin) => typeof plugin === "string" ? plugin === "plannotator" : plugin.id === "plannotator" - )) { + ); + if (entry) { + // A plugin whose setup threw still LISTS here. Without this check the + // smoke passed on a plugin that took the whole command registration down + // with it, which is the exact failure a wrong capability probe produces. + // Plugin.Info carries status/error at the TOP level; `state` is read as a + // fallback only, so this keeps working whichever shape the host serves. + const info = typeof entry === "string" ? undefined : entry; + const status = info?.status ?? info?.state?.status; + if (status === "failed") { + const error = info?.error ?? info?.state?.error ?? "no error reported"; + throw new Error(`Plannotator activated as failed in OpenCode 2: ${error}`); + } console.error(`plannotator activated after ${elapsed()}`); return response; } @@ -225,6 +265,50 @@ async function waitForPlugin(url: string): Promise { ); } +/** + * Assert the three slash commands resolve, and report WHICH definition owns + * each name. + * + * Ownership is readable from the description: the plugin's native definitions + * and the markdown stubs' frontmatter deliberately differ (pinned by + * native-commands.test.ts), so a stub description means the config-loaded + * command won the name. On a `next` host that is correct and expected; on a + * host with the post-#44765 command API it is the shadowing bug, which is what + * PLANNOTATOR_SMOKE_EXPECT_NATIVE makes fatal. + */ +async function checkCommands(url: string): Promise { + const response = await fetch(`${url}/api/command`, { + headers: { ...authHeaders(), "x-opencode-directory": encodeURIComponent(process.cwd()) }, + signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS), + }); + const body = await response.text(); + if (!response.ok) throw new Error(`OpenCode command API returned ${response.status}: ${body}`); + + const commands = (JSON.parse(body) as { data?: Array<{ name?: string; description?: string }> }).data ?? []; + const missing: string[] = []; + const shadowed: string[] = []; + for (const command of NATIVE_COMMANDS) { + const found = commands.find((entry) => entry.name === command.name); + if (!found) { + missing.push(command.name); + continue; + } + const native = found.description === command.description; + if (!native) shadowed.push(command.name); + console.error(`/${command.name}: ${native ? "plugin definition" : "markdown stub"}`); + } + + if (missing.length > 0) { + throw new Error(`OpenCode 2 resolved no command for: ${missing.join(", ")}. Full list: ${body.slice(0, 800)}`); + } + if (expectNativeCommands && shadowed.length > 0) { + throw new Error( + `The markdown stubs shadowed the plugin's native definitions for: ${shadowed.join(", ")}. ` + + "The command names must be reclaimed after OpenCode's ConfigCommandPlugin activates.", + ); + } +} + // A stuck teardown used to turn a failing smoke into a multi-minute CI wall-clock burn that // hid the diagnostics behind the job timeout. Every step here is bounded and escalates. async function shutdown(): Promise { diff --git a/apps/opencode-plugin/index.ts b/apps/opencode-plugin/index.ts index 75601ebf..95b5205f 100644 --- a/apps/opencode-plugin/index.ts +++ b/apps/opencode-plugin/index.ts @@ -76,6 +76,16 @@ function readBundledHtml(filename: string): string { return readFileSync(resolveBundledHtmlPath(filename), "utf-8"); } +/** Best-effort warm of the sync cache. Never throws, on any failure. */ +function preloadBundledHtml(filename: string, assign: (html: string) => void): void { + try { + readFile(resolveBundledHtmlPath(filename), "utf-8").then(assign).catch(() => {}); + } catch { + // The asset is not on disk. The lazy getters raise a clear error if and + // when a code path actually needs it. + } +} + function getPlanHtml(): string { if (!_planHtml) _planHtml = readBundledHtml("plannotator.html"); return _planHtml; @@ -234,9 +244,15 @@ async function runPlanReview(input: { const PlannotatorPlugin: Plugin = async (ctx, rawOptions?: PlannotatorOpenCodeOptions) => { const workflowOptions = normalizeWorkflowOptions(rawOptions); - // Preload HTML in background — populates the sync cache before first use - readFile(resolveBundledHtmlPath("plannotator.html"), "utf-8").then(h => { _planHtml = h; }).catch(() => {}); - readFile(resolveBundledHtmlPath("review-editor.html"), "utf-8").then(h => { _reviewHtml = h; }).catch(() => {}); + // Preload HTML in background: populates the sync cache before first use. + // `resolveBundledHtmlPath` THROWS when the asset is absent, and it runs + // synchronously here, outside the .catch that was meant to absorb exactly + // that. An unbuilt checkout (or a partial install) therefore took down plugin + // construction itself, before any code path that needs the HTML. A missing + // asset must only fail the feature that reads it, which is what the lazy + // getters already do. + preloadBundledHtml("plannotator.html", (html) => { _planHtml = html; }); + preloadBundledHtml("review-editor.html", (html) => { _reviewHtml = html; }); let cachedAgents: any[] | null = null; diff --git a/apps/opencode-plugin/native-commands.test.ts b/apps/opencode-plugin/native-commands.test.ts new file mode 100644 index 00000000..65f6755c --- /dev/null +++ b/apps/opencode-plugin/native-commands.test.ts @@ -0,0 +1,475 @@ +import { describe, expect, mock, test } from "bun:test"; +import { readFileSync } from "node:fs"; +import path from "node:path"; +import { + NATIVE_COMMANDS, + reclaimNativeCommands, + registerNativeCommands, + type CliCommandRequest, +} from "./native-commands"; +import { createV2BridgeClient, normalizeAgentList, readListPayload, toBridgeMessages } from "./v2-client"; +import { switchV2SessionAgent } from "./agent-switch"; + +const STUB_DIR = path.join(import.meta.dir, "commands"); + +/** The pre-#44765 draft: `transform` exists, `add` does not. */ +function legacyDraft() { + return { list: () => [], get: () => undefined, update: () => {}, remove: () => {} }; +} + +function makeDeps(overrides: Record = {}) { + const runCommand = mock(async (_request: CliCommandRequest) => {}); + const added: Array<{ name: string; description?: string; execute: Function }> = []; + const transform = mock(async (apply: (draft: { add: (d: any) => void }) => void) => { + apply({ add: (definition) => added.push(definition) }); + return { dispose: async () => {} }; + }); + + const ctx: any = { + // No list/reload here on purpose: the reclaim loop then exits before its + // first wait, so these tests never schedule a timer. + command: { transform }, + session: { get: async () => ({ location: { directory: "/project" } }) }, + location: { directory: "/fallback" }, + ...overrides, + }; + + return { + ctx, + added, + transform, + runCommand, + deps: { + ctx, + getAgents: async () => [], + getBridgeContext: async () => ({ sharingEnabled: true }), + runCommand, + }, + }; +} + +/** + * A faithful stand-in for OpenCode's command state: transforms are appended and + * REPLAYED in registration order, and `add` is a `Map.set`, so the last + * transform to add a name wins (core/src/state.ts, core/src/command.ts). + */ +function makeCommandHost() { + const committed = new Map(); + const transforms: Array<(draft: any) => void> = []; + const materialize = () => { + committed.clear(); + for (const transform of transforms) transform({ add: (d: any) => committed.set(d.name, d) }); + }; + return { + committed, + domain: { + transform: async (apply: (draft: any) => void) => { + transforms.push(apply); + materialize(); + return { dispose: async () => {} }; + }, + list: async () => ({ + location: {}, + data: [...committed.values()].map(({ name, description }) => ({ name, description })), + }), + reload: async () => { materialize(); }, + }, + /** Stand-in for OpenCode's ConfigCommandPlugin, which activates after us. */ + addConfigStubs: () => { + transforms.push((draft: any) => { + for (const command of NATIVE_COMMANDS) { + draft.add({ name: command.name, description: "from the markdown stub", execute: async () => {} }); + } + }); + materialize(); + }, + }; +} + +describe("OpenCode 2 native command registration", () => { + test("registers nothing when the host has no command domain", async () => { + const { deps } = makeDeps({ command: undefined }); + expect(await registerNativeCommands(deps)).toBe(false); + }); + + test("registers nothing on a pre-#44765 draft that has no add", async () => { + // The real old-host shape. `ctx.command.transform` EXISTS on `next` and + // `latest`; only the draft tells the truth. Calling a missing `add` here + // would throw inside the batched reload flush and abort it before commit, + // taking every command registration down with it. + let applied = false; + const { deps } = makeDeps({ + command: { + transform: async (apply: (draft: any) => void) => { + applied = true; + apply(legacyDraft()); + return { dispose: async () => {} }; + }, + }, + }); + + expect(await registerNativeCommands(deps)).toBe(false); + expect(applied).toBe(true); + }); + + test("registers exactly the three Plannotator commands when the draft supports add", async () => { + const { deps, added } = makeDeps(); + expect(await registerNativeCommands(deps)).toBe(true); + // Command names are the user-visible slash commands and are deliberately + // frozen: they must match the OpenCode 1 stubs so both hosts agree. + expect(added.map((command) => command.name)).toEqual([ + "plannotator-review", + "plannotator-annotate", + "plannotator-last", + ]); + for (const command of added) expect(command.execute).toBeInstanceOf(Function); + }); + + test("each execute runs the CLI path with the raw argument tail", async () => { + const { deps, added, runCommand } = makeDeps(); + await registerNativeCommands(deps); + + const annotate = added.find((command) => command.name === "plannotator-annotate")!; + await annotate.execute({ + sessionID: "session-9", + prompt: { text: "notes.md --gate --json" }, + delivery: "steer", + }); + + expect(runCommand).toHaveBeenCalledTimes(1); + const request = runCommand.mock.calls[0]![0]!; + expect(request.command).toBe("plannotator-annotate"); + expect(request.sessionId).toBe("session-9"); + // Raw pass-through: flags must reach the CLI's own argument resolution + // unparsed, exactly as OpenCode 1 forwards `input.arguments`. + expect(request.rawArgs).toBe("notes.md --gate --json"); + expect(request.cwd).toBe("/project"); + }); + + test("an argument-less invocation still runs with an empty tail", async () => { + const { deps, added, runCommand } = makeDeps(); + await registerNativeCommands(deps); + + const review = added.find((command) => command.name === "plannotator-review")!; + await review.execute({ sessionID: "session-1" }); + + expect(runCommand.mock.calls[0]![0]!.rawArgs).toBe(""); + }); + + test("falls back to the plugin location when the session has no directory", async () => { + const { deps, added, runCommand } = makeDeps({ + session: { get: async () => { throw new Error("no session"); } }, + }); + await registerNativeCommands(deps); + await added[0]!.execute({ sessionID: "session-1", prompt: { text: "" } }); + + expect(runCommand.mock.calls[0]![0]!.cwd).toBe("/fallback"); + }); + + test("a failing command is reported, not rethrown into OpenCode", async () => { + const failing = mock(async () => { throw new Error("boom"); }); + const { deps, added } = makeDeps(); + const errors: unknown[] = []; + const originalError = console.error; + console.error = (...args: unknown[]) => { errors.push(args[0]); }; + try { + await registerNativeCommands({ ...deps, runCommand: failing }); + await added[0]!.execute({ sessionID: "session-1", prompt: { text: "" } }); + } finally { + console.error = originalError; + } + expect(errors.some((line) => String(line).includes("boom"))).toBe(true); + }); +}); + +describe("reclaiming the command names from the config-loaded stubs", () => { + // OpenCode activates its own ConfigCommandPlugin AFTER package plugins, and + // it replays the installed markdown stubs into the same name-keyed map, so a + // setup-time registration is always overwritten on a normal install. + test("re-registers after the config stubs shadow the native definitions", async () => { + const host = makeCommandHost(); + const { deps } = makeDeps({ command: host.domain }); + const apply = () => registerNativeCommands(deps).then(() => {}); + + await apply(); + expect(host.committed.get("plannotator-review")?.description).toBe(NATIVE_COMMANDS[0]!.description); + + host.addConfigStubs(); + expect(host.committed.get("plannotator-review")?.description).toBe("from the markdown stub"); + + await reclaimNativeCommands({ + ctx: deps.ctx, + apply, + isSupported: () => true, + wait: async () => {}, + }); + + for (const command of NATIVE_COMMANDS) { + expect(host.committed.get(command.name)?.description).toBe(command.description); + } + }); + + test("a reload after the reclaim keeps the native definitions", async () => { + // Config only ever calls reload() afterwards; replay order is stable, so + // winning once must mean winning permanently. + const host = makeCommandHost(); + const { deps } = makeDeps({ command: host.domain }); + const apply = () => registerNativeCommands(deps).then(() => {}); + + await apply(); + host.addConfigStubs(); + await reclaimNativeCommands({ ctx: deps.ctx, apply, isSupported: () => true, wait: async () => {} }); + await host.domain.reload(); + + expect(host.committed.get("plannotator-review")?.description).toBe(NATIVE_COMMANDS[0]!.description); + }); + + test("stops re-registering once ownership outlives a reclaim", async () => { + const host = makeCommandHost(); + const { deps } = makeDeps({ command: host.domain }); + let applies = 0; + const apply = async () => { applies += 1; await registerNativeCommands(deps); }; + + await apply(); + host.addConfigStubs(); + applies = 0; + await reclaimNativeCommands({ ctx: deps.ctx, apply, isSupported: () => true, wait: async () => {} }); + + // One reclaim, then the next tick confirms ownership and the loop exits + // instead of piling on a transform per tick. + expect(applies).toBe(1); + }); + + test("keeps ticking while the draft probe has not run yet", async () => { + // The probe flag only flips when the transform REPLAYS, which under boot + // batching is at the flush after every plugin has loaded, and Plannotator + // loads before the post-group config plugins. An early tick that reads + // false must skip, not end the loop, or the reclaim is inert in exactly + // the shape production has. + const host = makeCommandHost(); + const { deps } = makeDeps({ command: host.domain }); + const apply = () => registerNativeCommands(deps).then(() => {}); + + await apply(); + host.addConfigStubs(); + + let ticks = 0; + await reclaimNativeCommands({ + ctx: deps.ctx, + apply, + // False on the first tick, true from the second: the host flushed. + isSupported: () => ticks > 1, + wait: async () => { ticks += 1; }, + }); + + expect(host.committed.get("plannotator-review")?.description).toBe(NATIVE_COMMANDS[0]!.description); + }); + + test("does nothing on a host without list or reload, and never on an unsupported draft", async () => { + const apply = mock(async () => {}); + await reclaimNativeCommands({ + ctx: { command: { transform: async () => ({}) } }, + apply, + isSupported: () => true, + wait: async () => {}, + }); + + const host = makeCommandHost(); + await reclaimNativeCommands({ + ctx: { command: host.domain }, + apply, + isSupported: () => false, + wait: async () => {}, + }); + + expect(apply).not.toHaveBeenCalled(); + }); + + test("a throwing list read ends the reclaim instead of looping", async () => { + const apply = mock(async () => {}); + await reclaimNativeCommands({ + ctx: { + command: { + transform: async () => ({}), + list: async () => { throw new Error("no service"); }, + reload: async () => {}, + }, + }, + apply, + isSupported: () => true, + wait: async () => {}, + }); + + expect(apply).not.toHaveBeenCalled(); + }); +}); + +describe("V2 list shapes", () => { + test("reads an agent list as a bare array or a { data } envelope", () => { + const entries = [{ id: "plan", mode: "primary", hidden: false }]; + expect(normalizeAgentList(entries)).toEqual([ + { name: "plan", description: undefined, mode: "primary", hidden: false }, + ]); + expect(normalizeAgentList({ location: {}, data: entries })).toEqual(normalizeAgentList(entries)); + }); + + test("unusable responses degrade to an empty list instead of throwing", () => { + expect(normalizeAgentList(undefined)).toEqual([]); + expect(normalizeAgentList({ data: "nope" })).toEqual([]); + expect(normalizeAgentList([{ mode: "primary" }])).toEqual([]); + expect(readListPayload({ data: [{ description: "nameless" }] })).toEqual([]); + }); +}); + +describe("V2 agent switching", () => { + test("switches the session agent when the host exposes switchAgent", async () => { + const switchAgent = mock(async (_input: { sessionID: string; agent: string }) => {}); + const result = await switchV2SessionAgent({ + ctx: { session: { switchAgent } }, + sessionID: "session-1", + requestedAgent: "build", + getAgents: async () => [{ name: "build" }], + warn: () => {}, + }); + + expect(switchAgent).toHaveBeenCalledWith({ sessionID: "session-1", agent: "build" }); + expect(result).toBe("build"); + }); + + test("warns and leaves the agent alone when the host has no switchAgent", async () => { + const warnings: string[] = []; + const result = await switchV2SessionAgent({ + ctx: { session: {} }, + sessionID: "session-1", + requestedAgent: "build", + getAgents: async () => [{ name: "build" }], + warn: (message) => warnings.push(message), + }); + + expect(result).toBeUndefined(); + expect(warnings).toHaveLength(1); + }); + + test("a failing switch does not fail the approval", async () => { + const warnings: string[] = []; + const result = await switchV2SessionAgent({ + ctx: { session: { switchAgent: async () => { throw new Error("busy"); } } }, + sessionID: "session-1", + requestedAgent: "build", + getAgents: async () => [{ name: "build" }], + warn: (message) => warnings.push(message), + }); + + expect(result).toBeUndefined(); + expect(warnings.some((line) => line.includes("busy"))).toBe(true); + }); + + test("an unavailable or disabled agent never reaches switchAgent", async () => { + const switchAgent = mock(async () => {}); + expect(await switchV2SessionAgent({ + ctx: { session: { switchAgent } }, + sessionID: "session-1", + requestedAgent: "ghost", + getAgents: async () => [{ name: "build" }], + warn: () => {}, + })).toBeUndefined(); + expect(await switchV2SessionAgent({ + ctx: { session: { switchAgent } }, + sessionID: "session-1", + requestedAgent: "disabled", + getAgents: async () => [{ name: "build" }], + warn: () => {}, + })).toBeUndefined(); + expect(switchAgent).not.toHaveBeenCalled(); + }); +}); + +describe("V2 feedback delivery", () => { + function makeBridge(switchAgent: (input: { sessionID: string; agent: string }) => Promise) { + const prompt = mock(async (_input: unknown) => ({})); + const warnings: string[] = []; + const client = createV2BridgeClient({ + ctx: { session: { prompt, switchAgent } }, + getAgents: async () => [], + warn: (message) => warnings.push(message), + }); + return { client, prompt, warnings }; + } + + test("a failing switchAgent still delivers the feedback", async () => { + // Same guarantee the approval path gives: the reviewer's words must not be + // lost because the session refused to change agent. + const { client, prompt, warnings } = makeBridge(async () => { throw new Error("busy"); }); + + await client.session.prompt({ + path: { id: "session-1" }, + body: { agent: "build", parts: [{ type: "text", text: "please fix" }] }, + }); + + expect(prompt).toHaveBeenCalledTimes(1); + expect(prompt.mock.calls[0]![0]).toMatchObject({ sessionID: "session-1", text: "please fix" }); + expect(warnings.some((line) => line.includes("busy"))).toBe(true); + }); + + test("feedback is queued, never steered into a running turn", async () => { + // The invocation's own delivery was chosen at admission; a review comes + // back minutes later, when a steer would land mid-turn. + const { client, prompt } = makeBridge(async () => {}); + + await client.session.prompt({ + path: { id: "session-1" }, + body: { parts: [{ type: "text", text: "LGTM" }] }, + }); + + expect(prompt.mock.calls[0]![0]).toMatchObject({ delivery: "queue" }); + }); +}); + +describe("V2 session context translation", () => { + // `/plannotator-last` reads assistant text out of the session. V2 messages + // are flat (`{ id, type, content }`) where V1 nested them under info/parts; + // getRecentAssistantMessages reads the V1 shape. + test("maps flat V2 messages into the nested shape the bridge reads", () => { + const mapped = toBridgeMessages([ + { id: "m1", type: "assistant", time: { created: 5 }, content: [{ type: "text", text: "hi" }] }, + ]) as Array<{ info: { id: string; role: string; time: { created: number } }; parts: unknown[] }>; + + expect(mapped[0]!.info).toEqual({ id: "m1", role: "assistant", time: { created: 5 } }); + expect(mapped[0]!.parts).toEqual([{ type: "text", text: "hi" }]); + }); + + test("a non-array context yields no messages", () => { + expect(toBridgeMessages(undefined)).toEqual([]); + }); +}); + +describe("shared command stubs", () => { + function readStub(name: string): { frontmatter: string; body: string } { + const source = readFileSync(path.join(STUB_DIR, `${name}.md`), "utf-8"); + const match = /^---\n([\s\S]*?)\n---\n([\s\S]*)$/.exec(source); + if (!match) throw new Error(`${name}.md has no frontmatter`); + return { frontmatter: match[1]!, body: match[2]! }; + } + + for (const command of NATIVE_COMMANDS) { + // OpenCode 1 evaluates a command template's shell interpolation BEFORE the + // V1 plugin's command.execute.before hook can clear the parts, so a `!` + // backtick in these shared stubs would launch a second Plannotator session + // on every OC1 invocation. Permanently pinned. + test(`${command.name}.md carries no shell interpolation`, () => { + const { body } = readStub(command.name); + expect(body).not.toContain("!`"); + // The model-mediated fallback needs the argument tail to reach the CLI. + expect(body).toContain("$ARGUMENTS"); + }); + + // The reclaim tells our definition from the config-loaded stub by reading + // the description back out of ctx.command.list(). Identical descriptions + // would make that check always report ownership and silently disable it. + test(`${command.name} native description differs from the stub frontmatter`, () => { + const { frontmatter } = readStub(command.name); + expect(frontmatter).toContain("description:"); + expect(frontmatter).not.toContain(command.description); + }); + } +}); diff --git a/apps/opencode-plugin/native-commands.ts b/apps/opencode-plugin/native-commands.ts new file mode 100644 index 00000000..db478ef5 --- /dev/null +++ b/apps/opencode-plugin/native-commands.ts @@ -0,0 +1,277 @@ +/** + * Native slash commands for OpenCode 2. + * + * OpenCode's V2 plugin API gained command EXECUTION in anomalyco/opencode + * PR #44765 (issue #2185): the command draft grew an `add({ name, description, + * execute })` method whose callback fully owns the invocation, so nothing + * reaches the model unless it says so. That shape currently ships only on the + * `beta` and `dev` dist-tags of `@opencode-ai/plugin`; `next` and `latest` + * still carry a draft of `{ list, get, update, remove }` with no `add`. + * + * `ctx.command.transform` therefore proves NOTHING: it exists on both. The only + * honest probe is the draft handed to the callback, which is what this module + * checks. On an older host it adds nothing and the markdown command stubs stay + * the (model-mediated) fallback. + * + * Execution reuses the exact V1 machinery, `handleCliCommand`, over a + * translation client, so the two hosts cannot drift. + */ + +import { handleCliCommand, type OpenCodeBridgeAgent, type OpenCodeBridgeContext } from "./cli-bridge"; +import { + createV2BridgeClient, + readListPayload, + type V2CommandDraft, + type V2CommandInvocation, + type V2ContextLike, +} from "./v2-client"; + +/** + * Descriptions are deliberately NOT copies of the markdown stubs' frontmatter. + * They are the provenance signal the reclaim below reads back out of + * `ctx.command.list()` to tell our definition from the config-loaded stub, and + * `native-commands.test.ts` pins that they stay distinct. + */ +export const NATIVE_COMMANDS: ReadonlyArray<{ name: string; description: string }> = [ + { + name: "plannotator-review", + description: + "Open the Plannotator code review UI for current changes or a PR URL; pass --git or --gitbutler to force that provider", + }, + { + name: "plannotator-annotate", + description: "Open the Plannotator annotation UI for a file, folder, or URL", + }, + { + name: "plannotator-last", + description: "Annotate the last assistant message in Plannotator", + }, +]; + +/** + * Delay BEFORE each ownership re-check, in milliseconds. The loop awaits these + * one after another, so the ticks land at roughly 0.3s, 1.5s, 5.5s and 15.5s + * after setup. + * + * Four bounded ticks, then it stops for good. Plugin activation and the config + * command scan both finish well inside that window; a host slower than that + * keeps the fallback, which still works. + */ +const RECLAIM_SCHEDULE_MS = [300, 1_200, 4_000, 10_000] as const; + +export interface CliCommandRequest { + command: string; + client: unknown; + sessionId?: string; + rawArgs: string; + cwd?: string; + bridge?: OpenCodeBridgeContext; +} + +export interface NativeCommandDeps { + ctx: V2ContextLike; + getAgents: () => Promise; + getBridgeContext: () => Promise; + /** + * Injection seam for tests only; production always uses `handleCliCommand`. + * Bun's `mock.module` is process-global and cannot be unset, so a module mock + * of `cli-bridge` here would leak into every other suite. + */ + runCommand?: (request: CliCommandRequest) => Promise; + /** Test seam for the reclaim schedule; production uses real timers. */ + wait?: (ms: number) => Promise; +} + +/** Resolve the invocation's working directory, session location first. */ +async function resolveDirectory(ctx: V2ContextLike, sessionID: string): Promise { + try { + const session = await ctx.session?.get?.({ sessionID }); + const directory = session?.location?.directory; + if (typeof directory === "string" && directory) return directory; + } catch { + // Fall through to the plugin location, then the process cwd. + } + return ctx.location?.directory || process.cwd(); +} + +export async function runNativeCommand( + command: string, + invocation: V2CommandInvocation, + deps: NativeCommandDeps, +): Promise { + const sessionID = invocation.sessionID; + // The raw argument tail, exactly as OpenCode 1 forwards it. The CLI's own + // tolerant argument resolution takes it from here: nothing is parsed or + // rewritten on the way through. + const rawArgs = typeof invocation.prompt?.text === "string" ? invocation.prompt.text : ""; + const client = createV2BridgeClient({ ctx: deps.ctx, getAgents: deps.getAgents }); + + const run = deps.runCommand ?? ((request: CliCommandRequest) => handleCliCommand(request as never)); + await run({ + command, + client, + sessionId: sessionID, + rawArgs, + cwd: await resolveDirectory(deps.ctx, sessionID), + bridge: await deps.getBridgeContext(), + }); +} + +function defaultWait(ms: number): Promise { + return new Promise((resolve) => { + const timer = setTimeout(resolve, ms); + // Never hold the host process open for a background reconciliation. + (timer as { unref?: () => void }).unref?.(); + }); +} + +/** + * Do our definitions currently own all three names? + * + * `ctx.command.list()` returns the MATERIALIZED command map, so a description + * that is not ours means another transform (in practice OpenCode's own + * ConfigCommandPlugin, replaying the installed markdown stubs) added the name + * after us and won. + */ +async function ownsNativeCommands(ctx: V2ContextLike): Promise { + const list = await ctx.command?.list?.(); + const commands = readListPayload(list); + return NATIVE_COMMANDS.every((command) => commands.some((entry) => + entry.name === command.name && entry.description === command.description)); +} + +/** + * Take the three names back from the config-loaded markdown stubs. + * + * Mechanism, verified against anomalyco/opencode `origin/v2`: + * - Command definitions live in a name-keyed Map and `draft.add` is a + * `Map.set` (`packages/core/src/command.ts`), so the LAST transform to add a + * name wins. + * - Transforms replay in registration order: `transforms = [...transforms, + * transform]`, and `materialize` walks that array + * (`packages/core/src/state.ts`). + * - Activation order is `pre` -> packages -> `post`, and OpenCode's own + * ConfigCommandPlugin, which scans `~/.config/opencode/{command,commands}/ + * **\/*.md`, sits in `post` (`packages/core/src/plugin/internal.ts:265-269`, + * `packages/core/src/plugin/supervisor.ts`). + * So a setup-time transform ALWAYS replays before config's, and the stubs the + * installer writes shadow the native definitions on every normal install. The + * fix is to register the same transform once more after activation settles, so + * ours is last in the replay order; from then on it stays last, because config + * only ever calls `reload()` and never re-registers. + * + * The explicit `reload()` is belt and braces, not a requirement: each plugin's + * effect runs inside `State.batch` (`packages/core/src/plugin.ts`), but the + * batch clears its active flag before flushing, so a registration arriving + * after that takes the direct path and materializes on its own. Calling + * `reload()` anyway costs one recompute and removes any dependence on that + * ordering detail holding in a future OpenCode. + * + * Deliberately not driven by `ctx.event.subscribe()`: upstream #44788 reports + * that stream as unreliable on some V2 nightlies, and `command.list()` is a + * direct read of committed state with no bus involved. + * + * Failure mode: if this never gets to run, or the host has no `list`/`reload`, + * or a future OpenCode reorders activation, the markdown stubs keep winning and + * the three commands still work through their model-mediated fallback bodies. + * Degraded, never broken. + */ +export async function reclaimNativeCommands(input: { + ctx: V2ContextLike; + apply: () => Promise; + isSupported: () => boolean; + wait?: (ms: number) => Promise; +}): Promise { + const wait = input.wait ?? defaultWait; + const list = input.ctx.command?.list; + const reload = input.ctx.command?.reload; + if (typeof list !== "function" || typeof reload !== "function") return; + + let reclaimed = false; + for (const delay of RECLAIM_SCHEDULE_MS) { + await wait(delay); + // The draft probe only runs when the transform REPLAYS, which under boot + // batching is at the flush after every plugin has loaded. Plannotator loads + // before the post-group config plugins, so an early tick can legitimately + // see this false: skip the tick, never end the loop, or the reclaim would + // be inert in exactly the shape production has. + if (!input.isSupported()) continue; + + let owned: boolean; + try { + owned = await ownsNativeCommands(input.ctx); + } catch { + return; + } + // Owning the names on an early tick can simply mean config has not loaded + // yet, so ownership alone is not an exit condition: only ownership that + // outlives a reclaim is. + if (owned) { + if (reclaimed) return; + continue; + } + + try { + await input.apply(); + await reload(); + reclaimed = true; + } catch { + return; + } + } +} + +/** + * Register the three Plannotator commands when the host's draft supports it. + * + * Returns whether the draft accepted them. Note the callback may not have run + * by the time `transform` resolves: during boot the host coalesces transforms + * into one batched reload, so the honest answer arrives a tick later. The + * reclaim loop re-reads the same flag rather than trusting this snapshot. + */ +export async function registerNativeCommands(deps: NativeCommandDeps): Promise { + const transform = deps.ctx.command?.transform; + if (typeof transform !== "function") return false; + + let supported = false; + const apply = async () => { + await transform((draft: V2CommandDraft) => { + // The ONLY honest capability probe: `transform` exists on hosts whose + // draft is `{ list, get, update, remove }`, where `add` is undefined and + // calling it would throw inside the batched reload flush, aborting it + // before commit and taking every command registration down with it. + if (typeof draft?.add !== "function") return; + supported = true; + for (const command of NATIVE_COMMANDS) { + draft.add({ + name: command.name, + description: command.description, + execute: async (invocation) => { + try { + await runNativeCommand(command.name, invocation, deps); + } catch (error) { + // handleCliCommand already logs and swallows everything except a + // prompt-delivery failure. Report that one and stop: rethrowing + // would surface an OpenCode command execution error for feedback + // the reviewer has already given. + console.error( + `[Plannotator] /${command.name} failed: ${error instanceof Error ? error.message : String(error)}`, + ); + } + }, + }); + } + }); + }; + + await apply(); + + void reclaimNativeCommands({ + ctx: deps.ctx, + apply, + isSupported: () => supported, + wait: deps.wait, + }); + + return supported; +} diff --git a/apps/opencode-plugin/server.test.ts b/apps/opencode-plugin/server.test.ts index f34c2832..9bd92e05 100644 --- a/apps/opencode-plugin/server.test.ts +++ b/apps/opencode-plugin/server.test.ts @@ -21,6 +21,13 @@ type SessionContextHook = (event: { function createContext( options: Record = {}, agents: Array<{ id: string; description?: string; mode: string; hidden: boolean }> = [], + hostOverrides: { + // Pre-#44765 hosts DO expose command.transform, but hand the callback a + // draft with no `add`. The adapter must then register nothing, throw + // nothing, and behave exactly as it did before. + command?: { transform: (apply: (draft: any) => void) => Promise }; + agentListShape?: "envelope" | "array"; + } = {}, ) { let toolDefinition: Record | undefined; let sessionContextHook: SessionContextHook | undefined; @@ -29,8 +36,11 @@ function createContext( return { context: { options, + ...(hostOverrides.command ? { command: hostOverrides.command } : {}), agent: { - list: async () => ({ location: { directory: "/project" }, data: agents }), + list: async () => (hostOverrides.agentListShape === "array" + ? agents + : { location: { directory: "/project" }, data: agents }), transform: async () => ({ dispose: async () => {} }), }, session: { @@ -200,6 +210,106 @@ describe("OpenCode V2 server plugin", () => { expect(event.tools.submit_plan).toBeUndefined(); }); + test("registers the slash commands only on a host that exposes the command API", async () => { + const registered: string[] = []; + const withCommands = createContext({}, [], { + command: { + transform: async (apply) => { + apply({ add: (definition: { name: string }) => registered.push(definition.name) }); + return { dispose: async () => {} }; + }, + }, + }); + await serverPlugin.setup(withCommands.context as never); + expect(registered).toEqual([ + "plannotator-review", + "plannotator-annotate", + "plannotator-last", + ]); + + // No command domain: nothing registered, and the pre-existing submit_plan + // contract is untouched. + const withoutCommands = createContext(); + await serverPlugin.setup(withoutCommands.context as never); + expect(withoutCommands.getToolDefinition()?.name).toBe("submit_plan"); + }); + + test("a pre-#44765 command draft registers nothing and does not fail setup", async () => { + // The real `next` / `latest` shape: transform exists, the draft is + // { list, get, update, remove }. Touching `add` here would throw inside the + // host's batched reload flush and abort it before commit. + let applied = false; + const testContext = createContext({}, [], { + command: { + transform: async (apply) => { + applied = true; + apply({ list: () => [], get: () => undefined, update: () => {}, remove: () => {} }); + return { dispose: async () => {} }; + }, + }, + }); + + await serverPlugin.setup(testContext.context as never); + expect(applied).toBe(true); + expect(testContext.getToolDefinition()?.name).toBe("submit_plan"); + }); + + test("a rejecting command transform never fails plugin setup", async () => { + // A slash command has a working markdown fallback; the whole Plannotator + // integration going down for it would not. + const testContext = createContext({}, [], { + command: { transform: async () => { throw new Error("command domain unavailable"); } }, + }); + const originalError = console.error; + console.error = () => {}; + try { + await serverPlugin.setup(testContext.context as never); + } finally { + console.error = originalError; + } + expect(testContext.getToolDefinition()?.name).toBe("submit_plan"); + }); + + test("registers slash commands even when submit_plan is disabled", async () => { + // `workflow: "manual"` returns early before the tool registration, which is + // exactly the mode that depends on the slash commands existing. + const registered: string[] = []; + const testContext = createContext({ workflow: "manual" }, [], { + command: { + transform: async (apply) => { + apply({ add: (definition: { name: string }) => registered.push(definition.name) }); + return { dispose: async () => {} }; + }, + }, + }); + await serverPlugin.setup(testContext.context as never); + + expect(registered).toHaveLength(3); + expect(testContext.getToolDefinition()).toBeUndefined(); + }); + + test("reads a bare-array agent list, so subagent gating still applies", async () => { + // Newer plugin hosts answer agent.list() with an array rather than the + // `{ data }` envelope; reading `.data` blindly emptied the list and let + // subagents keep submit_plan. + delete process.env.PLANNOTATOR_ALLOW_SUBAGENTS; + const testContext = createContext( + { workflow: "all-agents" }, + [{ id: "researcher", mode: "subagent", hidden: false }], + { agentListShape: "array" }, + ); + await serverPlugin.setup(testContext.context as never); + const event = { + agent: "researcher", + system: [{ type: "text" as const, text: "Base system prompt" }], + messages: [], + tools: { submit_plan: { description: "Submit", input: {} } }, + }; + + await testContext.getSessionContextHook()?.(event); + expect(event.tools.submit_plan).toBeUndefined(); + }); + test("generic reminder composes into the existing part instead of pushing a second one", async () => { process.env.PLANNOTATOR_ALLOW_SUBAGENTS = "1"; const testContext = createContext( diff --git a/apps/opencode-plugin/server.ts b/apps/opencode-plugin/server.ts index eb660dee..13f3c531 100644 --- a/apps/opencode-plugin/server.ts +++ b/apps/opencode-plugin/server.ts @@ -22,7 +22,9 @@ import { type OpenCodeBridgeContext, type OpenCodePlanReviewResult, } from "./cli-bridge"; -import { resolveTargetAgent } from "./agent-switch"; +import { switchV2SessionAgent } from "./agent-switch"; +import { registerNativeCommands } from "./native-commands"; +import { normalizeAgentList, type V2ContextLike } from "./v2-client"; import { executeSubmitPlan } from "./submit-plan-executor"; import type { PlanEdit } from "./plan-edits"; import { getPlanningPrompt } from "./planning-prompt"; @@ -63,19 +65,38 @@ const serverPlugin = { const getAgents = async (): Promise => { if (cachedAgents) return cachedAgents; try { - const response = await ctx.agent.list(); - cachedAgents = response.data.map((agent) => ({ - name: agent.id, - description: agent.description, - mode: agent.mode, - hidden: agent.hidden, - })); + // The documented success shape is the `{ location, data }` envelope. + // `normalizeAgentList` also accepts a bare array because reading + // `.data` off anything else throws into this catch, where the failure + // is invisible: an empty agent list silently disables subagent gating + // and agent-switch validation rather than reporting anything. + cachedAgents = normalizeAgentList(await ctx.agent.list()); } catch { cachedAgents = []; } return cachedAgents; }; + // The pinned `@opencode-ai/plugin` types predate the command-execution API + // (PR #44765), so the context is re-viewed through a duck-typed shape. Every + // capability behind it is probed before use. + const v2 = ctx as unknown as V2ContextLike; + + // Native slash commands are registered before the submit_plan early return + // below, so `workflow: "manual"`, which registers no tool, still gets them. + // Wrapped because a transform rejection must never fail plugin setup: the + // whole Plannotator integration would go down for a slash command that has + // a working markdown fallback. + try { + await registerNativeCommands({ + ctx: v2, + getAgents, + getBridgeContext: () => getBridgeContext(getAgents), + }); + } catch (error) { + console.error(`[Plannotator] Could not register the OpenCode 2 slash commands: ${error instanceof Error ? error.message : String(error)}`); + } + if (shouldModifyPrompts(workflowOptions)) { await ctx.session.hook("context", async (event) => { if ( @@ -194,18 +215,16 @@ const serverPlugin = { directory, bridge, }), - resolveTargetAgent: async ({ requestedAgent }) => { - const targetAgent = resolveTargetAgent(requestedAgent); - if (!targetAgent) return undefined; - const available = (await getAgents()).some((agent) => agent.name === targetAgent); - if (!available) { - console.error(`[Plannotator] Configured OpenCode agent "${targetAgent}" is not available; approving the plan without switching agents.`); - return undefined; - } - // The current OpenCode 2 API exposes no session-agent switch operation to plugins. - console.error("[Plannotator] OpenCode 2 does not currently expose agent switching to plugins; approving the plan without switching agents."); - return undefined; - }, + resolveTargetAgent: async ({ requestedAgent }) => await switchV2SessionAgent({ + ctx: v2, + sessionID: toolContext.sessionID, + requestedAgent, + getAgents, + }), + // The switch above is the whole handoff on V2. Its session.prompt + // has no `noReply` equivalent, so an injected approval note would + // start a model turn the reviewer never asked for; the submit_plan + // tool result already carries the approval text. sendApprovalHandoff: async () => {}, }); diff --git a/apps/opencode-plugin/v2-client.ts b/apps/opencode-plugin/v2-client.ts new file mode 100644 index 00000000..0fecde9c --- /dev/null +++ b/apps/opencode-plugin/v2-client.ts @@ -0,0 +1,251 @@ +/** + * Duck-typed adapters over the OpenCode 2 plugin context. + * + * The V2 plugin API is still pre-release: the published `next` and `latest` + * dist-tags of `@opencode-ai/plugin` carry an older context shape than the + * `beta` / `dev` nightlies. Nothing here may import the plugin package at + * runtime or assume a domain exists: every capability is probed before use so + * the adapter degrades to today's behavior on an older host. + */ + +import type { OpenCodeBridgeAgent } from "./cli-bridge"; + +/** The subset of the V2 session domain this plugin touches. */ +export interface V2SessionDomain { + get?: (input: { sessionID: string }) => Promise<{ location?: { directory?: string } }>; + prompt?: (input: { sessionID: string; text: string; delivery?: unknown }) => Promise; + switchAgent?: (input: { sessionID: string; agent: string }) => Promise; + context?: (input: { sessionID: string }) => Promise; +} + +/** + * The subset of the V2 command domain this plugin touches. + * + * `transform` exists on every V2 host and says nothing about capability: the + * pre-#44765 draft is `{ list, get, update, remove }`. Only the draft handed to + * the callback can answer that, which is why nothing here treats the presence + * of `transform` as support. + */ +export interface V2CommandDomain { + transform?: (apply: (draft: V2CommandDraft) => void) => Promise | unknown; + list?: (input?: unknown) => Promise; + reload?: () => Promise; +} + +export interface V2ContextLike { + agent?: { list?: (input?: unknown) => Promise }; + session?: V2SessionDomain; + command?: V2CommandDomain; + location?: { directory?: string }; +} + +export interface V2CommandInvocation { + sessionID: string; + prompt?: { text?: string }; + /** + * The admission mode OpenCode chose for the invocation. Carried for + * completeness and deliberately NOT reused when feedback comes back: see + * `FEEDBACK_DELIVERY`. + */ + delivery?: unknown; +} + +export interface V2CommandDefinition { + name: string; + description?: string; + execute: (input: V2CommandInvocation) => Promise; +} + +/** + * Post-#44765 draft. `add` is optional in the type because an older host hands + * the callback a draft without it; every call site must probe before using it. + */ +export interface V2CommandDraft { + add?: (definition: V2CommandDefinition) => void; +} + +export interface V2CommandListEntry { + name: string; + description?: string; +} + +/** The V1-shaped client `cli-bridge` consumes. */ +export interface V2BridgeClient { + app: { + log: (entry: { level: "info" | "error"; message: string }) => void; + agents: () => Promise<{ data: OpenCodeBridgeAgent[] }>; + }; + // Widened to `unknown` on purpose: these are handed to `cli-bridge`, whose + // client interface declares the same operations with `unknown` parameters. + session: { + messages: (input: unknown) => Promise<{ data: unknown[] }>; + prompt: (input: unknown) => Promise; + }; +} + +function isRecord(value: unknown): value is Record { + return !!value && typeof value === "object"; +} + +/** + * Unwrap a list response that may or may not be enveloped. + * + * The generated client types every `list` as `{ location, data }`, and that is + * what the documented success shape is. Reading `.data` unconditionally throws + * on anything else and the throw lands in a caller's catch, where it degrades + * silently rather than loudly, so both shapes are accepted here instead. + */ +function readEntries(response: unknown): unknown[] { + if (Array.isArray(response)) return response; + if (isRecord(response) && Array.isArray(response.data)) return response.data; + return []; +} + +/** Read `ctx.command.list()` into name/description pairs, envelope or not. */ +export function readListPayload(response: unknown): V2CommandListEntry[] { + const entries: V2CommandListEntry[] = []; + for (const entry of readEntries(response)) { + if (!isRecord(entry) || typeof entry.name !== "string") continue; + entries.push({ + name: entry.name, + description: typeof entry.description === "string" ? entry.description : undefined, + }); + } + return entries; +} + +/** Read an agent list, envelope or bare array, without ever throwing. */ +export function normalizeAgentList(response: unknown): OpenCodeBridgeAgent[] { + const entries = readEntries(response); + + const agents: OpenCodeBridgeAgent[] = []; + for (const entry of entries) { + if (!isRecord(entry)) continue; + const name = typeof entry.id === "string" + ? entry.id + : typeof entry.name === "string" ? entry.name : undefined; + if (!name) continue; + agents.push({ + name, + description: typeof entry.description === "string" ? entry.description : undefined, + mode: typeof entry.mode === "string" ? entry.mode : undefined, + hidden: entry.hidden === true, + }); + } + return agents; +} + +/** True when this host's session domain can switch the active agent. */ +export function supportsSwitchAgent(ctx: V2ContextLike): boolean { + return typeof ctx.session?.switchAgent === "function"; +} + +// There is deliberately no `supportsNativeCommands(ctx)`. `ctx.command.transform` +// exists on hosts whose draft predates PR #44765 and has no `add`, so any probe +// from the context alone reports a false positive; the draft itself is the only +// witness. See `native-commands.ts`. + +/** + * Translate `ctx.session.context()` output into the message shape + * `getRecentAssistantMessages` reads. V2 messages are flat + * (`{ id, type, time, content }`); V1 nested them under `info` / `parts`. + */ +export function toBridgeMessages(context: unknown): unknown[] { + if (!Array.isArray(context)) return []; + return context.filter(isRecord).map((message) => ({ + info: { + id: typeof message.id === "string" ? message.id : undefined, + role: typeof message.type === "string" ? message.type : undefined, + time: isRecord(message.time) ? { created: message.time.created } : undefined, + }, + parts: Array.isArray(message.content) ? message.content : [], + })); +} + +function joinTextParts(parts: unknown[]): string { + return parts + .filter((part): part is { type: string; text: string } => + isRecord(part) && part.type === "text" && typeof part.text === "string") + .map((part) => part.text) + .join("\n"); +} + +/** Read the session id out of the V1-shaped `{ path: { id } }` request. */ +function readSessionId(request: unknown): string | undefined { + if (!isRecord(request) || !isRecord(request.path)) return undefined; + return typeof request.path.id === "string" ? request.path.id : undefined; +} + +/** + * How Plannotator feedback is admitted to the session. + * + * A command invocation carries its own delivery, but that value was chosen when + * the user pressed enter, and a review comes back minutes later: replaying a + * "steer" then would land the feedback in the middle of whatever turn is + * running now. "queue" is the safe choice for a late arrival. Upstream's own + * default is "steer" (`packages/core/src/session/prompt.ts`), so this is set + * explicitly rather than omitted. + */ +const FEEDBACK_DELIVERY = "queue"; + +/** + * Build the V1-shaped client `handleCliCommand` and `resolveValidatedTargetAgent` + * expect, backed by the V2 context. Delivering feedback goes through + * `ctx.session.prompt`, the direct path, rather than a synthetic-event + * injection, which is unreliable on some V2 nightlies (upstream #44788). + * + * There is deliberately no `tui` domain: the V2 server-plugin context exposes + * none, and every toast call site in `cli-bridge` is best-effort. + */ +export function createV2BridgeClient(input: { + ctx: V2ContextLike; + getAgents: () => Promise; + /** Best-effort warning sink; defaults to stderr. */ + warn?: (message: string) => void; +}): V2BridgeClient { + const warn = input.warn ?? ((message: string) => console.error(message)); + const loggedUrls = new Set(); + return { + app: { + agents: async () => ({ data: await input.getAgents() }), + log: ({ message }) => { + const url = /https?:\/\/\S+/.exec(message)?.[0]; + if (url && loggedUrls.has(url)) return; + if (url) loggedUrls.add(url); + console.error(message); + }, + }, + session: { + messages: async (request) => { + const sessionID = readSessionId(request); + if (!sessionID) return { data: [] }; + const context = await input.ctx.session?.context?.({ sessionID }); + return { data: toBridgeMessages(context) }; + }, + prompt: async (request) => { + const sessionID = readSessionId(request); + if (!sessionID) throw new Error("Plannotator feedback has no OpenCode session to deliver to."); + const body = isRecord(request) && isRecord(request.body) ? request.body : {}; + const agent = typeof body.agent === "string" ? body.agent : undefined; + if (agent && typeof input.ctx.session?.switchAgent === "function") { + // A failed switch must never cost the reviewer their feedback: the + // same guarantee `switchV2SessionAgent` gives the approval path. + try { + await input.ctx.session.switchAgent({ sessionID, agent }); + } catch (error) { + warn(`[Plannotator] Could not switch the OpenCode session to "${agent}": ${error instanceof Error ? error.message : String(error)}`); + } + } + const prompt = input.ctx.session?.prompt; + if (typeof prompt !== "function") { + throw new Error("OpenCode 2 host exposes no session.prompt; cannot deliver Plannotator feedback."); + } + return await prompt({ + sessionID, + text: joinTextParts(Array.isArray(body.parts) ? body.parts : []), + delivery: FEEDBACK_DELIVERY, + }); + }, + }, + }; +} diff --git a/scripts/opencode2-native-commands-smoke.sh b/scripts/opencode2-native-commands-smoke.sh new file mode 100755 index 00000000..7b221f31 --- /dev/null +++ b/scripts/opencode2-native-commands-smoke.sh @@ -0,0 +1,53 @@ +#!/usr/bin/env bash +# Verify the OpenCode 2 native slash commands against a host that actually has +# the post-#44765 command API. +# +# CI cannot do this: .github/workflows/test.yml pins @opencode-ai/cli to a +# `next` build, and `next` still ships the older command draft (no `add`), so +# the CI leg can only prove the fallback path. The command API currently lives +# on the `beta` and `dev` dist-tags, which move daily and are not something to +# pin a required check to. So this is a script a human runs before a release. +# +# Usage: +# scripts/opencode2-native-commands-smoke.sh [dist-tag] # default: dev +# +# What it proves: +# 1. The plugin activates without status:"failed". +# 2. All three slash commands resolve. +# 3. They resolve to the PLUGIN's definitions, not the markdown stubs the +# fixture installs into the sandbox config dir exactly as install.sh does. +# (3) is the shadowing check and is fatal here because of +# PLANNOTATOR_SMOKE_EXPECT_NATIVE=1. +# +# What it does NOT prove: that /plannotator-review opens the UI without a model +# turn. Run that by hand in the TUI against the same build. + +set -euo pipefail + +tag="${1:-dev}" +repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +work="$(mktemp -d)" +trap 'rm -rf "$work"' EXIT + +echo "==> installing @opencode-ai/cli@$tag into $work" +cd "$work" +npm init -y >/dev/null 2>&1 +npm install --no-audit --no-fund "@opencode-ai/cli@$tag" >/dev/null +opencode_bin="$work/node_modules/.bin/opencode" +if [ ! -x "$opencode_bin" ]; then + echo "No opencode binary at $opencode_bin" >&2 + exit 1 +fi +"$opencode_bin" --version + +echo "==> building and packing the plugin" +cd "$repo_root" +bun run build:opencode +cd "$repo_root/apps/opencode-plugin" +bun pm pack --filename "$work/plannotator-opencode.tgz" >/dev/null + +echo "==> running the smoke with native commands required" +PLANNOTATOR_SMOKE_EXPECT_NATIVE=1 \ + bun run --cwd "$repo_root/apps/opencode-plugin" smoke:v2 -- \ + "$opencode_bin" \ + "$work/plannotator-opencode.tgz"