From fb5525fc2234a246dfdfc9e0d7ed73c824cbd8a6 Mon Sep 17 00:00:00 2001 From: Mark Jaquith Date: Fri, 11 Sep 2026 16:36:18 -0400 Subject: fix: opencode v2 lifecycle reporting (#3757) * fix: support opencode v2 lifecycle reporting * fix: repair Japanese docs * fix: ignore payload-less OpenCode events * fix: harden opencode v2 integration install and reporting Follow-ups on top of the V2 lifecycle support: - create `cli.json` when OpenCode has no V1 TUI preferences (`tui.json` or `kv.json`) to migrate, instead of only registering into an existing file - settle stalled socket attempts with a plain connect timer, and resend the latest lifecycle state after a failed delivery so the pane cannot get stuck - resolve the OpenCode state directory from `XDG_STATE_HOME` - remove the managed `herdr-opencode` directory on uninstall - document failed executions reporting `blocked`, keep the integration test environment independent of an inherited `XDG_STATE_HOME`, and simplify `reconcileBlockers` to reassign its map --------- Co-authored-by: Jonathan Liebig --- src/integration/actions.rs | 11 +- .../assets/opencode/herdr-agent-state.js | 11 +- .../assets/opencode/herdr-agent-state.test.ts | 10 + .../assets/opencode/herdr-tui-session.js | 243 +++++++++++++++++++-- .../assets/opencode/herdr-tui-session.test.ts | 224 ++++++++++++++++++- src/integration/assets/opencode/tui.js | 5 + src/integration/env.rs | 39 ++++ src/integration/mod.rs | 5 +- src/integration/opencode_config.rs | 154 +++++++++++-- src/integration/registry.rs | 12 + src/integration/targets.rs | 23 +- src/integration/tests.rs | 95 ++++++++ src/integration/types.rs | 1 + 13 files changed, 789 insertions(+), 44 deletions(-) create mode 100644 src/integration/assets/opencode/tui.js (limited to 'src/integration') diff --git a/src/integration/actions.rs b/src/integration/actions.rs index 24d1f9e3..e8b3ab63 100644 --- a/src/integration/actions.rs +++ b/src/integration/actions.rs @@ -144,7 +144,7 @@ fn install_target_inner(target: crate::api::schema::IntegrationTarget) -> io::Re } crate::api::schema::IntegrationTarget::Opencode => { let installed = install_opencode()?; - vec![ + let mut messages = vec![ format!( "installed opencode integration plugin to {}", installed.plugin_path.display() @@ -157,7 +157,14 @@ fn install_target_inner(target: crate::api::schema::IntegrationTarget) -> io::Re "ensured opencode tui plugin config at {}", installed.tui_config_path.display() ), - ] + ]; + if installed.cli_config_path.is_none() { + messages.push( + "to enable OpenCode V2, start opencode2 once, then reinstall this integration" + .to_string(), + ); + } + messages } crate::api::schema::IntegrationTarget::Kilo => { let installed = install_kilo()?; diff --git a/src/integration/assets/opencode/herdr-agent-state.js b/src/integration/assets/opencode/herdr-agent-state.js index 45032335..73a70534 100644 --- a/src/integration/assets/opencode/herdr-agent-state.js +++ b/src/integration/assets/opencode/herdr-agent-state.js @@ -2,7 +2,7 @@ // managed by herdr; reinstalling or updating the integration overwrites this file. // add custom hooks/plugins beside this file instead of editing it. // HERDR_INTEGRATION_ID=opencode -// HERDR_INTEGRATION_VERSION=11 +// HERDR_INTEGRATION_VERSION=12 import net from "node:net"; @@ -199,3 +199,12 @@ export const HerdrAgentStatePlugin = async () => { }, }; }; + +// V1 (1.18.29+) calls server(). V2 calls setup() instead. Its shared server +// cannot attribute sessions using its process environment: the pane-local TUI +// owns both selection and lifecycle reporting there, including remote servers. +export default { + id: "herdr.opencode", + server: HerdrAgentStatePlugin, + setup() {}, +}; diff --git a/src/integration/assets/opencode/herdr-agent-state.test.ts b/src/integration/assets/opencode/herdr-agent-state.test.ts index a3eea848..38c1f05c 100644 --- a/src/integration/assets/opencode/herdr-agent-state.test.ts +++ b/src/integration/assets/opencode/herdr-agent-state.test.ts @@ -227,6 +227,16 @@ function requestMethod(request: unknown): unknown { return isRecord(request) ? request.method : undefined; } +test("dual server entrypoint keeps V1 hooks and never reports from the V2 shared server", async () => { + const module = await import(`./herdr-agent-state.js?test=${++importCounter}`); + expect(module.default.server).toBe(module.HerdrAgentStatePlugin); + expect(await module.default.setup({})).toBeUndefined(); + expect(requests).toHaveLength(0); + const hooks = await module.default.server(); + await hooks["chat.message"]({ sessionID: "v1-root" }); + expect(requests.map(requestState)).toEqual(["working"]); +}); + function requestState(request: unknown): unknown { return requestParam(request, "state"); } diff --git a/src/integration/assets/opencode/herdr-tui-session.js b/src/integration/assets/opencode/herdr-tui-session.js index f4f08d57..4e5631ac 100644 --- a/src/integration/assets/opencode/herdr-tui-session.js +++ b/src/integration/assets/opencode/herdr-tui-session.js @@ -1,7 +1,7 @@ // installed by herdr // managed by herdr; reinstalling or updating the integration overwrites this file. // HERDR_INTEGRATION_ID=opencode-tui -// HERDR_INTEGRATION_VERSION=11 +// HERDR_INTEGRATION_VERSION=12 import net from "node:net"; @@ -10,11 +10,11 @@ const AGENT = "opencode"; const ROUTE_POLL_INTERVAL_MS = 100; const SELECTION_RETRY_DELAYS_MS = [100, 400, 1_000]; -function requestOnce(sessionID) { +function requestOnce(sessionID, state, seq, isCurrent = () => true) { const paneId = process.env.HERDR_PANE_ID; const socketPath = process.env.HERDR_SOCKET_PATH; if (!paneId || !socketPath) { - return Promise.resolve(); + return Promise.resolve(true); } const socketEndpoint = @@ -23,35 +23,50 @@ function requestOnce(sessionID) { id: `${SOURCE}:tui:${Date.now()}:${Math.floor(Math.random() * 1_000_000) .toString() .padStart(6, "0")}`, - method: "pane.report_agent_session", + method: state === undefined ? "pane.report_agent_session" : "pane.report_agent", params: { pane_id: paneId, source: SOURCE, agent: AGENT, agent_session_id: sessionID, - session_start_source: "select", + ...(state === undefined ? { session_start_source: "select" } : { state, seq }), }, }; return new Promise((resolve) => { + let settled = false; + let timer; + const settle = (delivered) => { + if (settled) return; + settled = true; + clearTimeout(timer); + client.destroy(); + resolve(delivered); + }; const client = net.createConnection(socketEndpoint, () => { + if (!isCurrent()) { + settle(false); + return; + } client.write(`${JSON.stringify(request)}\n`); }); - const finish = () => { - client.destroy(); - resolve(); - }; - client.setTimeout(500, finish); - client.on("data", finish); - client.on("error", finish); - client.on("end", finish); - client.on("close", resolve); + // A plain timer, not socket.setTimeout, so a connection that never finishes + // connecting still settles and cannot block later reports behind the queue. + timer = setTimeout(() => settle(false), 500); + timer.unref?.(); + client.on("data", () => settle(true)); + client.on("error", () => settle(false)); + client.on("end", () => settle(false)); + client.on("close", () => settle(false)); }); } export default { id: "herdr.opencode.session-selection", + // Keep this plain object dependency-free: V1 and V2 expose different SDK + // packages, but both loaders accept their own lifecycle entry on this object. + setup, tui: async (api) => { if ( process.env.HERDR_ENV !== "1" || @@ -111,3 +126,203 @@ export default { api.lifecycle.onDispose(() => clearInterval(routePoll)); }, }; + +function setup(api) { + if (process.env.HERDR_ENV !== "1" || !process.env.HERDR_SOCKET_PATH || !process.env.HERDR_PANE_ID) return; + + let disposed = false; + let selected; + let generation = 0; + let sequence = Date.now() * 1000; + let chain = Promise.resolve(); + let retryIndex = 0; + let nextSelectionAt = 0; + let state = "idle"; + let retryTimer; + const sessions = new Map(); + let blockers = new Map(); + // Event callbacks may precede cache updates. Retain each delta until the + // cache reflects it, so late hydration cannot undo a reply or lose an ask. + const blockerChanges = new Map(); + + function root(id) { + const seen = new Set(); + while (typeof id === "string" && !seen.has(id)) { + seen.add(id); + const session = api.data.session.get(id) ?? sessions.get(id); + if (!session) return; + if (!session.parentID) return id; + id = session.parentID; + } + } + + function current() { + const route = api.ui.router.current(); + return route.type === "session" ? root(route.sessionID) : undefined; + } + + // Selection and lifecycle use one queue. Recheck attribution at dispatch, + // not just when receiving the event, and reject A -> B -> A stale work too. + function enqueue(value) { + const sessionID = selected; + const revision = generation; + const isCurrent = () => !disposed && revision === generation && !!sessionID && current() === sessionID; + chain = chain.then(async () => { + if (!isCurrent()) return; + const delivered = await requestOnce(sessionID, value, value === undefined ? undefined : ++sequence, isCurrent); + if (!delivered) scheduleStateRetry(); + }).catch(() => {}); + } + + // A dropped report must not strand the pane on a stale state once the + // selection retry schedule has run out: resend the latest state until the + // socket accepts it or the selection is no longer current. + function scheduleStateRetry() { + if (disposed || retryTimer) return; + retryTimer = setTimeout(() => { + retryTimer = undefined; + publish(); + }, 500); + retryTimer.unref?.(); + } + + function publish() { + enqueue(blockers.size ? "blocked" : state); + } + + function changeBlocker(id, kind, requestID, present) { + if (typeof requestID !== "string") return; + const key = `${kind}:${requestID}`; + blockerChanges.set(key, { id, kind, present }); + if (present) blockers.set(key, id); + else blockers.delete(key); + } + + function reconcileBlockers() { + const next = new Map(); + const hydrated = new Set(); + const members = new Set([selected, ...api.data.session.family(selected), ...blockers.values()]); + for (const member of members) { + if (root(member) !== selected) continue; + for (const kind of ["permission", "form"]) { + const items = api.data.session[kind].list(member); + if (items === undefined) { + for (const [key, owner] of blockers) { + if (owner === member && key.startsWith(`${kind}:`)) next.set(key, owner); + } + continue; + } + hydrated.add(`${kind}:${member}`); + for (const item of items) next.set(`${kind}:${item.id}`, member); + } + } + for (const [key, change] of blockerChanges) { + if (hydrated.has(`${change.kind}:${change.id}`) && next.has(key) === change.present) { + blockerChanges.delete(key); + } else if (change.present) { + next.set(key, change.id); + } else { + next.delete(key); + } + } + const changed = (blockers.size > 0) !== (next.size > 0); + blockers = next; + return changed; + } + + function syncSelection() { + if (disposed) return; + const id = current(); + if (id !== selected) { + selected = id; + generation += 1; + retryIndex = 0; + nextSelectionAt = 0; + blockers.clear(); + blockerChanges.clear(); + if (id) { + state = api.data.session.status(id) === "running" ? "working" : "idle"; + } + } + if (!id) return; + const blockersChanged = reconcileBlockers(); + if (Date.now() < nextSelectionAt) { + if (blockersChanged) publish(); + return; + } + enqueue(undefined); + publish(); + const delay = SELECTION_RETRY_DELAYS_MS[retryIndex++]; + nextSelectionAt = delay === undefined ? Number.POSITIVE_INFINITY : Date.now() + delay; + } + + function receive({ details: event }) { + if (disposed) return; + const data = event.data; + if (data == null) return; + if (event.type === "session.created") { + sessions.set(data.sessionID, { id: data.sessionID, parentID: data.parentID }); + } + if (event.type === "session.deleted") { + const affected = data.sessionID === selected || [...blockers.values()].includes(data.sessionID); + sessions.delete(data.sessionID); + // Deletion is delivered after the cache can remove the session. Use + // stored ownership rather than looking up the deleted child's ancestry. + for (const [key, owner] of blockers) if (owner === data.sessionID) blockers.delete(key); + for (const [key, change] of blockerChanges) { + if (change.id === data.sessionID) blockerChanges.delete(key); + } + syncSelection(); + if (selected && affected) publish(); + return; + } + syncSelection(); + const id = event.type === "form.created" ? data.form.sessionID : data.sessionID; + if (!selected || root(id) !== selected) return; + switch (event.type) { + case "permission.asked": + changeBlocker(id, "permission", data.id, true); + break; + case "permission.replied": + changeBlocker(id, "permission", data.requestID, false); + break; + case "form.created": + changeBlocker(id, "form", data.form.id, true); + break; + case "form.replied": + case "form.cancelled": + changeBlocker(id, "form", data.id, false); + break; + case "session.execution.started": + if (id !== selected) return; + state = "working"; + break; + case "session.execution.succeeded": + case "session.execution.interrupted": + if (id !== selected) return; + state = "idle"; + break; + case "session.execution.failed": + if (id !== selected) return; + state = "blocked"; + break; + default: + return; + } + publish(); + } + + const unsubscribe = api.data.listen(receive); + syncSelection(); + const poll = setInterval(syncSelection, ROUTE_POLL_INTERVAL_MS); + return () => { + disposed = true; + generation += 1; + clearTimeout(retryTimer); + clearInterval(poll); + unsubscribe(); + sessions.clear(); + blockers.clear(); + blockerChanges.clear(); + }; +} diff --git a/src/integration/assets/opencode/herdr-tui-session.test.ts b/src/integration/assets/opencode/herdr-tui-session.test.ts index 1bcb1f81..dd031527 100644 --- a/src/integration/assets/opencode/herdr-tui-session.test.ts +++ b/src/integration/assets/opencode/herdr-tui-session.test.ts @@ -3,15 +3,25 @@ import { afterEach, beforeEach, expect, mock, test } from "bun:test"; const requests: unknown[] = []; const activeDisposers: Array<() => void> = []; const requestWaiters: Array<() => void> = []; +const stateWaiters: Array<() => void> = []; let importCounter = 0; +let holdConnections = false; +let failConnections = false; +const connections: Array<() => void> = []; mock.module("node:net", () => ({ default: { createConnection(_path: string, onConnect: () => void) { const handlers = new Map void>(); const client = { + destroyed: false, write(input: string) { - requests.push(JSON.parse(input.trim())); + if (client.destroyed) return; + const request = JSON.parse(input.trim()); + requests.push(request); + if (isRecord(request) && isRecord(request.params) && request.params.state !== undefined) { + stateWaiters.shift()?.(); + } requestWaiters.shift()?.(); queueMicrotask(() => client.emit("data")); }, @@ -19,12 +29,16 @@ mock.module("node:net", () => ({ on(event: string, handler: () => void) { handlers.set(event, handler); }, - destroy() {}, + destroy() { + client.destroyed = true; + }, emit(event: string) { handlers.get(event)?.(); }, }; - queueMicrotask(onConnect); + if (holdConnections) connections.push(onConnect); + else if (failConnections) queueMicrotask(() => client.emit("error")); + else queueMicrotask(onConnect); return client; }, }, @@ -33,6 +47,10 @@ mock.module("node:net", () => ({ beforeEach(() => { requests.length = 0; requestWaiters.length = 0; + stateWaiters.length = 0; + holdConnections = false; + failConnections = false; + connections.length = 0; process.env.HERDR_ENV = "1"; process.env.HERDR_SOCKET_PATH = "test.sock"; process.env.HERDR_PANE_ID = "test:p1"; @@ -93,6 +111,10 @@ function waitForNextRequest(): Promise { return new Promise((resolve) => requestWaiters.push(resolve)); } +function waitForStateReport(): Promise { + return new Promise((resolve) => stateWaiters.push(resolve)); +} + test("reports a root session when only the local route changes", async () => { const plugin = await loadPlugin(); const tui = fakeApi(); @@ -179,3 +201,199 @@ function requestParam(request: unknown, name: string): unknown { function isRecord(value: unknown): value is Record { return typeof value === "object" && value !== null; } + +function v2Api() { + const sessions = new Map([ + ["a", { id: "a" }], + ["b", { id: "b" }], + ["child", { id: "child", parentID: "a" }], + ]); + let route = { type: "session", sessionID: "a" }; + const listeners = new Set<(event: unknown) => void>(); + const permissions = new Map | undefined>(); + const forms = new Map | undefined>(); + return { + api: { + ui: { router: { current: () => route } }, + data: { + session: { + get: (id: string) => sessions.get(id), + family: () => [...sessions.keys()], + status: () => "idle", + permission: { list: (id: string) => permissions.get(id) }, + form: { list: (id: string) => forms.get(id) }, + }, + listen: (handler: (event: unknown) => void) => { + listeners.add(handler); + return () => listeners.delete(handler); + }, + }, + }, + select(sessionID: string) { route = { type: "session", sessionID }; }, + home() { route = { type: "home", sessionID: "" }; }, + emit(type: string, data?: object) { + for (const listener of listeners) listener({ details: { type, data } }); + }, + listeners, + sessions, + permissions, + forms, + }; +} + +const flushReports = () => new Promise((resolve) => setTimeout(resolve, 10)); +const states = () => requests.filter((r) => requestParam(r, "state") !== undefined) + .map((r) => requestParam(r, "state")); + +test("V2 ignores events without data", async () => { + const plugin = await loadPlugin(); + const tui = v2Api(); + const dispose = await plugin.setup(tui.api); + activeDisposers.push(dispose); + await flushReports(); + requests.length = 0; + expect(() => tui.emit("legacy.event")).not.toThrow(); + tui.emit("session.execution.started", { sessionID: "a" }); + await flushReports(); + expect(states()).toEqual(["working"]); +}); + +test("V2 completes and interrupts without legacy idle events", async () => { + for (const terminal of ["succeeded", "interrupted", "failed"]) { + const plugin = await loadPlugin(); + const tui = v2Api(); + const dispose = await plugin.setup(tui.api); + activeDisposers.push(dispose); + await flushReports(); + requests.length = 0; + tui.emit("session.execution.started", { sessionID: "a" }); + tui.emit(`session.execution.${terminal}`, { sessionID: "a" }); + await flushReports(); + expect(states()).toEqual(["working", terminal === "failed" ? "blocked" : "idle"]); + dispose(); + } +}); + +test("V2 aggregates root and child blockers and ignores other roots and child completion", async () => { + const plugin = await loadPlugin(); + const tui = v2Api(); + const dispose = await plugin.setup(tui.api); + activeDisposers.push(dispose); + await flushReports(); + requests.length = 0; + tui.emit("session.execution.started", { sessionID: "a" }); + tui.emit("permission.asked", { sessionID: "a", id: "permission-a" }); + tui.emit("form.created", { form: { sessionID: "child", id: "form-child" } }); + tui.emit("permission.replied", { sessionID: "a", requestID: "permission-a" }); + tui.emit("session.execution.succeeded", { sessionID: "child" }); + tui.emit("session.execution.started", { sessionID: "b" }); + tui.emit("permission.asked", { sessionID: "b", id: "other" }); + await flushReports(); + expect(states().at(-1)).toBe("blocked"); + expect(requests.every((r) => requestParam(r, "agent_session_id") === "a")).toBe(true); + tui.emit("form.cancelled", { sessionID: "child", id: "form-child" }); + tui.emit("session.execution.succeeded", { sessionID: "a" }); + await flushReports(); + expect(states().slice(-2)).toEqual(["working", "idle"]); +}); + +test("V2 discards queued reports after selection changes and stops on disposal", async () => { + const plugin = await loadPlugin(); + const tui = v2Api(); + const dispose = await plugin.setup(tui.api); + activeDisposers.push(dispose); + await flushReports(); + requests.length = 0; + tui.emit("session.execution.started", { sessionID: "a" }); + tui.select("b"); + tui.emit("session.execution.started", { sessionID: "b" }); + await flushReports(); + expect(requests.every((r) => requestParam(r, "agent_session_id") === "b")).toBe(true); + requests.length = 0; + tui.emit("session.execution.succeeded", { sessionID: "b" }); + tui.home(); + await flushReports(); + expect(requests).toHaveLength(0); + dispose(); + expect(tui.listeners.size).toBe(0); + tui.select("a"); + await new Promise((resolve) => setTimeout(resolve, 250)); + expect(requests).toHaveLength(0); +}); + +test("V2 reconciles late blocker hydration without reviving an already-replied request", async () => { + const plugin = await loadPlugin(); + const tui = v2Api(); + const dispose = await plugin.setup(tui.api); + activeDisposers.push(dispose); + await flushReports(); + tui.permissions.set("child", [{ id: "late" }]); + await waitForStateReport(); + expect(states().at(-1)).toBe("blocked"); + tui.emit("permission.replied", { sessionID: "child", requestID: "late" }); + await waitForStateReport(); + expect(states().at(-1)).toBe("idle"); + tui.permissions.set("child", []); + tui.forms.set("child", [{ id: "second" }]); + await waitForStateReport(); + expect(states().at(-1)).toBe("blocked"); + tui.sessions.delete("child"); + tui.emit("session.deleted", { sessionID: "child" }); + await flushReports(); + expect(states().at(-1)).toBe("idle"); +}); + +test("V2 never writes a delayed connection after disposal or a session switch", async () => { + for (const action of ["dispose", "switch"]) { + const plugin = await loadPlugin(); + const tui = v2Api(); + holdConnections = true; + requests.length = 0; + const dispose = await plugin.setup(tui.api); + activeDisposers.push(dispose); + await flushReports(); + expect(connections.length).toBeGreaterThan(0); + if (action === "dispose") dispose(); + else tui.select("b"); + holdConnections = false; + for (const connect of connections.splice(0)) connect(); + await flushReports(); + expect(requests).toHaveLength(0); + dispose(); + } +}); + +test("V2 settles a connection that never completes", async () => { + const plugin = await loadPlugin(); + const tui = v2Api(); + holdConnections = true; + const dispose = await plugin.setup(tui.api); + activeDisposers.push(dispose); + const started = Date.now(); + while (connections.length <= 1 && Date.now() - started < 2_000) { + await new Promise((resolve) => setTimeout(resolve, 25)); + } + expect(connections.length).toBeGreaterThan(1); + dispose(); +}); + +test("V2 resends the latest state after a failed delivery", async () => { + const plugin = await loadPlugin(); + const tui = v2Api(); + const dispose = await plugin.setup(tui.api); + activeDisposers.push(dispose); + await flushReports(); + // Exhaust the selection retry schedule so only the event report remains. + await new Promise((resolve) => setTimeout(resolve, 1_600)); + requests.length = 0; + tui.emit("session.execution.started", { sessionID: "a" }); + await flushReports(); + failConnections = true; + tui.emit("session.execution.succeeded", { sessionID: "a" }); + const resend = waitForStateReport(); + await new Promise((resolve) => setTimeout(resolve, 700)); + failConnections = false; + await resend; + expect(states().at(-1)).toBe("idle"); + dispose(); +}); diff --git a/src/integration/assets/opencode/tui.js b/src/integration/assets/opencode/tui.js new file mode 100644 index 00000000..cc1ffe6a --- /dev/null +++ b/src/integration/assets/opencode/tui.js @@ -0,0 +1,5 @@ +// installed by herdr +// HERDR_INTEGRATION_ID=opencode-tui-v2 +// HERDR_INTEGRATION_VERSION=12 +// V2 resolves the directory's tui entrypoint; V1 uses the original file. +export { default } from "../herdr-tui-session.js"; diff --git a/src/integration/env.rs b/src/integration/env.rs index c4ee1c34..48320c72 100644 --- a/src/integration/env.rs +++ b/src/integration/env.rs @@ -127,6 +127,14 @@ pub(crate) fn opencode_dir() -> io::Result { Ok(home_dir()?.join(".config/opencode")) } +pub(crate) fn opencode_state_dir() -> io::Result { + if let Some(value) = std::env::var_os("XDG_STATE_HOME").filter(|value| !value.is_empty()) { + return expand_tilde_path(PathBuf::from(value)).map(|path| path.join("opencode")); + } + + Ok(home_dir()?.join(".local/state/opencode")) +} + pub(crate) fn kilo_dir() -> io::Result { Ok(home_dir()?.join(".config/kilo")) } @@ -248,3 +256,34 @@ pub(crate) fn integration_env_lock() -> IntegrationEnvLock { appdata: std::env::var_os("APPDATA"), } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn opencode_state_dir_defaults_to_local_state() { + let _lock = integration_env_lock(); + let original = std::env::var_os("XDG_STATE_HOME"); + std::env::remove_var("XDG_STATE_HOME"); + let expected = home_dir().unwrap().join(".local/state/opencode"); + assert_eq!(opencode_state_dir().unwrap(), expected); + match original { + Some(value) => std::env::set_var("XDG_STATE_HOME", value), + None => std::env::remove_var("XDG_STATE_HOME"), + } + } + + #[test] + fn opencode_state_dir_honors_xdg_state_home() { + let _lock = integration_env_lock(); + let original = std::env::var_os("XDG_STATE_HOME"); + let xdg = std::env::temp_dir().join("herdr-xdg-state"); + std::env::set_var("XDG_STATE_HOME", &xdg); + assert_eq!(opencode_state_dir().unwrap(), xdg.join("opencode")); + match original { + Some(value) => std::env::set_var("XDG_STATE_HOME", value), + None => std::env::remove_var("XDG_STATE_HOME"), + } + } +} diff --git a/src/integration/mod.rs b/src/integration/mod.rs index 3bb8485c..d956c7bc 100644 --- a/src/integration/mod.rs +++ b/src/integration/mod.rs @@ -170,7 +170,10 @@ const OPENCODE_PLUGIN_ASSET: &str = include_str!("assets/opencode/herdr-agent-st const OPENCODE_TUI_PLUGIN_INSTALL_NAME: &str = "herdr-tui-session.js"; const OPENCODE_TUI_PLUGIN_SPEC: &str = "./herdr-tui-session.js"; const OPENCODE_TUI_PLUGIN_ASSET: &str = include_str!("assets/opencode/herdr-tui-session.js"); -const OPENCODE_INTEGRATION_VERSION: u32 = 11; +const OPENCODE_V2_TUI_PLUGIN_DIR: &str = "herdr-opencode"; +const OPENCODE_V2_TUI_PLUGIN_SPEC: &str = "./herdr-opencode"; +const OPENCODE_V2_TUI_PLUGIN_ASSET: &str = include_str!("assets/opencode/tui.js"); +const OPENCODE_INTEGRATION_VERSION: u32 = 12; const KILO_PLUGIN_INSTALL_NAME: &str = "herdr-agent-state.js"; const KILO_PLUGIN_ASSET: &str = include_str!("assets/kilo/herdr-agent-state.js"); const KILO_INTEGRATION_VERSION: u32 = 4; diff --git a/src/integration/opencode_config.rs b/src/integration/opencode_config.rs index 57431f93..2ab91e84 100644 --- a/src/integration/opencode_config.rs +++ b/src/integration/opencode_config.rs @@ -13,25 +13,53 @@ pub(crate) fn tui_config_path(config_dir: &Path) -> PathBuf { } pub(crate) fn validate_tui_plugin_config(config_dir: &Path) -> io::Result<()> { - let config_path = tui_config_path(config_dir); + validate_plugin_config(&tui_config_path(config_dir), "plugin")?; + validate_plugin_config(&config_dir.join("cli.json"), "plugins") +} + +fn validate_plugin_config(config_path: &Path, key: &str) -> io::Result<()> { if !config_path.is_file() { return Ok(()); } - let content = fs::read_to_string(&config_path)?; - let root = parse_root(&content, &config_path)?; - let object = root_object(&root, &config_path)?; + let content = fs::read_to_string(config_path)?; + let root = parse_root(&content, config_path)?; + let object = root_object(&root, config_path)?; if object - .get("plugin") + .get(key) .is_some_and(|property| property.array_value().is_none()) { - return Err(invalid_plugin_list(&config_path)); + return Err(invalid_plugin_list(config_path)); } Ok(()) } pub(crate) fn add_tui_plugin(config_dir: &Path, plugin_spec: &str) -> io::Result { - let config_path = tui_config_path(config_dir); + add_plugin(tui_config_path(config_dir), "plugin", plugin_spec) +} + +pub(crate) fn add_cli_plugin( + config_dir: &Path, + state_dir: &Path, + plugin_spec: &str, +) -> io::Result> { + let path = config_dir.join("cli.json"); + // OpenCode imports V1 TUI preferences (`tui.json`, `kv.json`) into cli.json on + // its first V2 start, but only while cli.json is absent. Defer registration + // while those sources still exist so we do not skip the migration; otherwise + // create cli.json ourselves, since OpenCode will never do it for a fresh V2 + // install with nothing to migrate. + if !path.is_file() && cli_migration_pending(config_dir, state_dir) { + return Ok(None); + } + add_plugin(path, "plugins", plugin_spec).map(Some) +} + +fn cli_migration_pending(config_dir: &Path, state_dir: &Path) -> bool { + config_dir.join("tui.json").is_file() || state_dir.join("kv.json").is_file() +} + +fn add_plugin(config_path: PathBuf, key: &str, plugin_spec: &str) -> io::Result { let content = if config_path.is_file() { fs::read_to_string(&config_path)? } else { @@ -40,7 +68,7 @@ pub(crate) fn add_tui_plugin(config_dir: &Path, plugin_spec: &str) -> io::Result let root = parse_root(&content, &config_path)?; let object = root_object(&root, &config_path)?; - match object.get("plugin") { + match object.get(key) { Some(property) => { let plugins = property .array_value() @@ -56,7 +84,7 @@ pub(crate) fn add_tui_plugin(config_dir: &Path, plugin_spec: &str) -> io::Result } None => { object.append( - "plugin", + key, CstInputValue::Array(vec![CstInputValue::String(plugin_spec.to_string())]), ); } @@ -67,20 +95,27 @@ pub(crate) fn add_tui_plugin(config_dir: &Path, plugin_spec: &str) -> io::Result } pub(crate) fn remove_tui_plugin(config_dir: &Path, plugin_spec: &str) -> io::Result { - let config_path = tui_config_path(config_dir); + remove_plugin(&tui_config_path(config_dir), "plugin", plugin_spec) +} + +pub(crate) fn remove_cli_plugin(config_dir: &Path, plugin_spec: &str) -> io::Result { + remove_plugin(&config_dir.join("cli.json"), "plugins", plugin_spec) +} + +fn remove_plugin(config_path: &Path, key: &str, plugin_spec: &str) -> io::Result { if !config_path.is_file() { return Ok(false); } - let content = fs::read_to_string(&config_path)?; - let root = parse_root(&content, &config_path)?; - let object = root_object(&root, &config_path)?; - let Some(property) = object.get("plugin") else { + let content = fs::read_to_string(config_path)?; + let root = parse_root(&content, config_path)?; + let object = root_object(&root, config_path)?; + let Some(property) = object.get(key) else { return Ok(false); }; let plugins = property .array_value() - .ok_or_else(|| invalid_plugin_list(&config_path))?; + .ok_or_else(|| invalid_plugin_list(config_path))?; let mut removed = false; for entry in plugins.elements() { if entry @@ -98,23 +133,30 @@ pub(crate) fn remove_tui_plugin(config_dir: &Path, plugin_spec: &str) -> io::Res property.remove(); } - fs::write(&config_path, root.to_string())?; + fs::write(config_path, root.to_string())?; Ok(true) } pub(crate) fn tui_plugin_is_configured(config_dir: &Path, plugin_spec: &str) -> bool { - let config_path = tui_config_path(config_dir); - let Ok(content) = fs::read_to_string(&config_path) else { + plugin_is_configured(&tui_config_path(config_dir), "plugin", plugin_spec) +} + +pub(crate) fn cli_plugin_is_configured(config_dir: &Path, plugin_spec: &str) -> bool { + plugin_is_configured(&config_dir.join("cli.json"), "plugins", plugin_spec) +} + +fn plugin_is_configured(config_path: &Path, key: &str, plugin_spec: &str) -> bool { + let Ok(content) = fs::read_to_string(config_path) else { return false; }; - let Ok(root) = parse_root(&content, &config_path) else { + let Ok(root) = parse_root(&content, config_path) else { return false; }; - let Ok(object) = root_object(&root, &config_path) else { + let Ok(object) = root_object(&root, config_path) else { return false; }; object - .get("plugin") + .get(key) .and_then(|property| property.array_value()) .is_some_and(|plugins| { plugins.elements().iter().any(|entry| { @@ -154,6 +196,7 @@ fn jsonc_parse_options() -> ParseOptions { fn plugin_entry_matches(entry: &Value, plugin_spec: &str) -> bool { entry.as_str() == Some(plugin_spec) + || entry.get("package").and_then(Value::as_str) == Some(plugin_spec) || entry .as_array() .and_then(|parts| parts.first()) @@ -295,4 +338,73 @@ mod tests { fs::remove_dir_all(dir).unwrap(); } + + #[test] + fn cli_registration_preserves_options_and_other_preferences() { + let dir = unique_dir(); + let state = unique_dir(); + let path = dir.join("cli.json"); + fs::write(&path, r#"{"theme":{"name":"catppuccin"},"plugins":[{"package":"./herdr-opencode","options":{"custom":true}},"example"]}"#).unwrap(); + add_cli_plugin(&dir, &state, "./herdr-opencode").unwrap(); + assert!(cli_plugin_is_configured(&dir, "./herdr-opencode")); + assert_eq!(parse_config(&path)["plugins"].as_array().unwrap().len(), 2); + assert_eq!(parse_config(&path)["plugins"][0]["options"]["custom"], true); + assert!(remove_cli_plugin(&dir, "./herdr-opencode").unwrap()); + assert_eq!(parse_config(&path)["plugins"], json!(["example"])); + assert_eq!(parse_config(&path)["theme"]["name"], "catppuccin"); + add_cli_plugin(&dir, &state, "./herdr-opencode").unwrap(); + add_cli_plugin(&dir, &state, "./herdr-opencode").unwrap(); + assert_eq!( + parse_config(&path)["plugins"], + json!(["example", "./herdr-opencode"]) + ); + fs::remove_dir_all(dir).unwrap(); + fs::remove_dir_all(state).unwrap(); + } + + #[test] + fn cli_registration_creates_missing_config_when_no_migration_pending() { + let dir = unique_dir(); + let state = unique_dir(); + let path = add_cli_plugin(&dir, &state, "./herdr-opencode") + .unwrap() + .expect("cli.json should be created when OpenCode has nothing to migrate"); + assert_eq!(path, dir.join("cli.json")); + assert_eq!( + parse_config(&path), + json!({ "plugins": ["./herdr-opencode"] }) + ); + assert!(cli_plugin_is_configured(&dir, "./herdr-opencode")); + fs::remove_dir_all(dir).unwrap(); + fs::remove_dir_all(state).unwrap(); + } + + #[test] + fn cli_registration_defers_while_migration_pending() { + let dir = unique_dir(); + let state = unique_dir(); + fs::write(dir.join("tui.json"), "{}").unwrap(); + assert!(add_cli_plugin(&dir, &state, "./herdr-opencode") + .unwrap() + .is_none()); + assert!(!dir.join("cli.json").exists()); + + fs::remove_file(dir.join("tui.json")).unwrap(); + fs::write(state.join("kv.json"), "{}").unwrap(); + assert!(add_cli_plugin(&dir, &state, "./herdr-opencode") + .unwrap() + .is_none()); + assert!(!dir.join("cli.json").exists()); + + fs::remove_dir_all(dir).unwrap(); + fs::remove_dir_all(state).unwrap(); + } + + #[test] + fn invalid_cli_plugin_list_fails_preflight() { + let dir = unique_dir(); + fs::write(dir.join("cli.json"), r#"{"plugins":{}}"#).unwrap(); + assert!(validate_tui_plugin_config(&dir).is_err()); + fs::remove_dir_all(dir).unwrap(); + } } diff --git a/src/integration/registry.rs b/src/integration/registry.rs index 46251970..f3bb8c04 100644 --- a/src/integration/registry.rs +++ b/src/integration/registry.rs @@ -427,6 +427,18 @@ fn opencode_tui_integration_is_valid(plugin_path: &Path, expected_version: u32) config_dir, super::OPENCODE_TUI_PLUGIN_SPEC, ) + && (!config_dir.join("cli.json").exists() + || (super::opencode_config::cli_plugin_is_configured( + config_dir, + super::OPENCODE_V2_TUI_PLUGIN_SPEC, + ) && fs::read_to_string( + config_dir + .join(super::OPENCODE_V2_TUI_PLUGIN_DIR) + .join("tui.js"), + ) + .ok() + .and_then(|content| parse_integration_version(&content)) + .is_some_and(|version| version >= expected_version))) } pub(crate) fn integration_status_at( diff --git a/src/integration/targets.rs b/src/integration/targets.rs index 0690fc90..d204c552 100644 --- a/src/integration/targets.rs +++ b/src/integration/targets.rs @@ -22,13 +22,14 @@ use super::config_edit::{ use super::env::{ antigravity_cli_dir, claude_dir, codex_dir, copilot_dir, cursor_dir, devin_dir, droid_dir, grok_dir, hermes_dir, hermes_plugin_dir, kilo_dir, kimi_dir, mastracode_dir, omp_extension_dir, - opencode_dir, pi_extension_dir, qodercli_dir, qwen_dir, + opencode_dir, opencode_state_dir, pi_extension_dir, qodercli_dir, qwen_dir, }; use super::file_ops::{ make_executable, remove_dir_all_if_exists, remove_file_if_exists, remove_legacy_bash_hook_file, }; use super::opencode_config::{ - add_tui_plugin, remove_tui_plugin, tui_config_path, validate_tui_plugin_config, + add_cli_plugin, add_tui_plugin, remove_cli_plugin, remove_tui_plugin, tui_config_path, + validate_tui_plugin_config, }; use super::types::{ AntigravityCliInstallPaths, AntigravityCliUninstallResult, ClaudeInstallPaths, @@ -468,11 +469,20 @@ pub(crate) fn install_opencode() -> io::Result { let tui_plugin_path = dir.join(OPENCODE_TUI_PLUGIN_INSTALL_NAME); fs::write(&tui_plugin_path, OPENCODE_TUI_PLUGIN_ASSET)?; let tui_config_path = add_tui_plugin(&dir, OPENCODE_TUI_PLUGIN_SPEC)?; + let v2_dir = dir.join(super::OPENCODE_V2_TUI_PLUGIN_DIR); + fs::create_dir_all(&v2_dir)?; + fs::write(v2_dir.join("tui.js"), super::OPENCODE_V2_TUI_PLUGIN_ASSET)?; + let cli_config_path = add_cli_plugin( + &dir, + &opencode_state_dir()?, + super::OPENCODE_V2_TUI_PLUGIN_SPEC, + )?; Ok(OpenCodeInstallPaths { plugin_path, tui_plugin_path, tui_config_path, + cli_config_path, }) } @@ -821,6 +831,15 @@ pub(crate) fn uninstall_opencode() -> io::Result { let plugin_path = dir.join("plugins").join(OPENCODE_PLUGIN_INSTALL_NAME); let tui_plugin_path = dir.join(OPENCODE_TUI_PLUGIN_INSTALL_NAME); let mut errors = Vec::new(); + remove_cli_plugin(&dir, super::OPENCODE_V2_TUI_PLUGIN_SPEC).unwrap_or_else(|err| { + errors.push(err.to_string()); + false + }); + let v2_dir = dir.join(super::OPENCODE_V2_TUI_PLUGIN_DIR); + remove_dir_all_if_exists(&v2_dir).unwrap_or_else(|err| { + errors.push(format!("failed to remove {}: {err}", v2_dir.display())); + false + }); let updated_tui_config = remove_tui_plugin(&dir, OPENCODE_TUI_PLUGIN_SPEC).unwrap_or_else(|err| { errors.push(err.to_string()); diff --git a/src/integration/tests.rs b/src/integration/tests.rs index b2210074..dc597d1d 100644 --- a/src/integration/tests.rs +++ b/src/integration/tests.rs @@ -134,6 +134,7 @@ fn clear_integration_path_env() { std::env::remove_var(COPILOT_HOME_ENV_VAR); std::env::remove_var(KIMI_CODE_HOME_ENV_VAR); std::env::remove_var("XDG_CONFIG_HOME"); + std::env::remove_var("XDG_STATE_HOME"); #[cfg(windows)] std::env::remove_var("APPDATA"); std::env::remove_var(QODERCLI_CONFIG_DIR_ENV_VAR); @@ -2301,11 +2302,105 @@ fn install_opencode_writes_server_and_tui_plugins() { let tui_config: Value = serde_json::from_str(&fs::read_to_string(&installed.tui_config_path).unwrap()).unwrap(); assert_eq!(tui_config["plugin"], json!([OPENCODE_TUI_PLUGIN_SPEC])); + let cli_config_path = installed + .cli_config_path + .expect("cli.json should be created when OpenCode has nothing to migrate"); + assert_eq!(cli_config_path, opencode_dir.join("cli.json")); + let cli_config: Value = + serde_json::from_str(&fs::read_to_string(&cli_config_path).unwrap()).unwrap(); + assert_eq!(cli_config["plugins"], json!([OPENCODE_V2_TUI_PLUGIN_SPEC])); std::env::remove_var("HOME"); let _ = fs::remove_dir_all(base); } +#[test] +fn opencode_install_defers_v2_registration_while_migration_pending() { + let _lock = integration_env_lock(); + let base = unique_base(); + let home = base.join("home"); + let opencode_dir = home.join(".config/opencode"); + fs::create_dir_all(&opencode_dir).unwrap(); + fs::write(opencode_dir.join("tui.json"), "{}").unwrap(); + std::env::set_var("HOME", &home); + + let installed = install_opencode().unwrap(); + + assert!(installed.cli_config_path.is_none()); + assert!(!opencode_dir.join("cli.json").exists()); + assert!(opencode_dir + .join(OPENCODE_V2_TUI_PLUGIN_DIR) + .join("tui.js") + .is_file()); + + std::env::remove_var("HOME"); + let _ = fs::remove_dir_all(base); +} + +#[test] +fn opencode_v2_install_status_and_uninstall_preserve_cli_preferences() { + let _lock = integration_env_lock(); + let base = unique_base(); + let home = base.join("home"); + let dir = home.join(".config/opencode"); + fs::create_dir_all(&dir).unwrap(); + std::env::set_var("HOME", &home); + let cli = dir.join("cli.json"); + fs::write( + &cli, + r#"{"theme":{"name":"catppuccin"},"plugins":["other"]}"#, + ) + .unwrap(); + let installed = install_opencode().unwrap(); + assert_eq!(installed.cli_config_path, Some(cli.clone())); + let status = || { + integration_status_at( + crate::api::schema::IntegrationTarget::Opencode, + installed.plugin_path.clone(), + OPENCODE_INTEGRATION_VERSION, + ) + .state + }; + assert_eq!(status(), IntegrationStatusKind::Current); + let entry = dir.join(OPENCODE_V2_TUI_PLUGIN_DIR).join("tui.js"); + assert_eq!( + fs::read_to_string(&entry).unwrap(), + OPENCODE_V2_TUI_PLUGIN_ASSET + ); + fs::remove_file(&entry).unwrap(); + assert_eq!(status(), IntegrationStatusKind::Outdated); + install_opencode().unwrap(); + super::opencode_config::remove_cli_plugin(&dir, OPENCODE_V2_TUI_PLUGIN_SPEC).unwrap(); + assert_eq!(status(), IntegrationStatusKind::Outdated); + install_opencode().unwrap(); + uninstall_opencode().unwrap(); + assert!(!entry.exists()); + assert_eq!( + serde_json::from_str::(&fs::read_to_string(cli).unwrap()).unwrap(), + json!({"theme":{"name":"catppuccin"},"plugins":["other"]}) + ); + std::env::remove_var("HOME"); + let _ = fs::remove_dir_all(base); +} + +#[test] +fn opencode_invalid_cli_config_does_not_overwrite_existing_plugins() { + let _lock = integration_env_lock(); + let base = unique_base(); + let home = base.join("home"); + let dir = home.join(".config/opencode"); + fs::create_dir_all(dir.join("plugins")).unwrap(); + std::env::set_var("HOME", &home); + let plugin = dir.join("plugins").join(OPENCODE_PLUGIN_INSTALL_NAME); + fs::write(&plugin, "previous integration").unwrap(); + fs::write(dir.join("cli.json"), r#"{"plugins":{}}"#).unwrap(); + assert!(install_opencode().is_err()); + assert_eq!(fs::read_to_string(plugin).unwrap(), "previous integration"); + assert!(!dir.join("tui.jsonc").exists()); + std::env::remove_var("HOME"); + let _ = fs::remove_dir_all(base); +} + #[test] fn opencode_status_requires_the_tui_plugin_and_config_entry() { let _lock = integration_env_lock(); diff --git a/src/integration/types.rs b/src/integration/types.rs index 2eaf7bf5..8169ab98 100644 --- a/src/integration/types.rs +++ b/src/integration/types.rs @@ -44,6 +44,7 @@ pub(crate) struct OpenCodeInstallPaths { pub plugin_path: PathBuf, pub tui_plugin_path: PathBuf, pub tui_config_path: PathBuf, + pub cli_config_path: Option, } #[derive(Debug)] -- cgit v1.2.3