diff --git a/README.md b/README.md index c4428e0675..48f73ec7e0 100644 --- a/README.md +++ b/README.md @@ -722,7 +722,7 @@ overwrite user-modified agents. | Codex CLI | Detected | `$CODEX_HOME/config.toml` | Managed `AGENTS.md` activation pointer, skill, three read-only agents; `SessionStart` + `SubagentStart`, `PreToolUse` graph lookup for `Bash` searches | | Gemini CLI | Detected | `.gemini/settings.json` | `GEMINI.md`, three explicit read/graph-tool subagents; `BeforeTool`, `AfterTool` `read_file` coverage, and `SessionStart` | | Zed | Detected | platform `settings.json` (JSONC) | `AGENTS.md` + shared skill | -| OpenCode | Detected | `$OPENCODE_CONFIG` or resolved global config | `AGENTS.md`, skill, three deny-by-default read-only agents; plugin adds grep/glob graph lookup, post-`read` coverage, first-tool-result session context, and post-compaction reinjection | +| OpenCode | Detected | `$OPENCODE_CONFIG` or resolved global config | `AGENTS.md`, skill, three deny-by-default read-only agents; plugin adds grep/glob graph lookup, post-`read` coverage, first-tool-result session context, and post-compaction reinjection on OpenCode 1.18.x | | Antigravity | Detected | `.gemini/config/mcp_config.json` | `.gemini/GEMINI.md` | | Aider | Detected | none | `CONVENTIONS.md` via `.aider.conf.yml` | | KiloCode | Detected | `.config/kilo/kilo.jsonc` | Rule + three graph-tool subagents with deny-by-default permissions | diff --git a/src/cli/client_adapter.c b/src/cli/client_adapter.c index cfd2719c78..f5747c4c80 100644 --- a/src/cli/client_adapter.c +++ b/src/cli/client_adapter.c @@ -427,9 +427,16 @@ char *cbm_client_adapter_opencode(const char *binary_path) { sb_append(&sb, "// OpenCode already reaches every tool over MCP; this module adds the\n" "// context surfaces other clients get through hook configuration: graph\n" "// lookup after grep/glob, index-coverage notes after read, session-start\n" - "// tier routing (carried on the first tool result of each session, since\n" - "// OpenCode documents no context-output lifecycle hook), and reinjection\n" - "// after compaction via the documented experimental surface.\n"); + "// tier routing (carried on the first tool result of each session), and\n" + "// on OpenCode 1.18.x reinjection after compaction.\n" + "//\n" + "// Two runtimes load this module:\n" + "// - OpenCode 1.18.x dispatches the hooks returned by server(ctx).\n" + "// - OpenCode 2 ignores server and calls setup(ctx), which registers the\n" + "// tool hook on the context's tool domain. The registration fails open,\n" + "// and the server() tool hook yields once it has registered. OpenCode 2\n" + "// gets no note after compaction: no compaction channel has been run on\n" + "// a real 2.x host yet.\n"); sb_append(&sb, "import { spawn } from 'node:child_process';\n\n"); sb_append(&sb, "const BIN = '"); sb_append(&sb, bin); @@ -481,64 +488,165 @@ char *cbm_client_adapter_opencode(const char *binary_path) { " });\n" "}\n\n"); + /* OpenCode 2 tool results are structured (Tool.Result: content is the + * string/parts surface the model reads, output is the tool's schema-typed + * value). Append to content first, then a string output; if neither text + * surface exists, leave the result untouched so the tool's own output is + * never hidden. A parts array gets a new text part only when it already + * holds a part: `content: []` is a valid result, and a part added there + * would be the note alone, so an empty array falls through to the string + * output (or the unchanged result). The caller assigns the returned result + * onto event.result. */ + sb_append(&sb, + "function appendResult(result, extra) {\n" + " if (!result || typeof result !== 'object') return result;\n" + " if (typeof result.content === 'string') {\n" + " return { ...result, content: result.content + '\\n' + extra };\n" + " }\n" + " if (Array.isArray(result.content) && result.content.length > 0) {\n" + " const content = result.content.slice();\n" + " for (let i = content.length - 1; i >= 0; i--) {\n" + " if (content[i] && content[i].type === 'text') {\n" + " content[i] = { ...content[i], text: content[i].text + '\\n' + extra };\n" + " return { ...result, content };\n" + " }\n" + " }\n" + " content.push({ type: 'text', text: extra });\n" + " return { ...result, content };\n" + " }\n" + " if (typeof result.output === 'string') {\n" + " return { ...result, output: result.output + '\\n' + extra };\n" + " }\n" + " return result;\n" + "}\n\n"); + + /* A host that dispatches both server() and setup() with a tool domain must + * not append the note twice: the server() tool hook yields once setup() + * registered the tool domain. */ + sb_append(&sb, "let toolHooksRegistered = false;\n\n"); + sb_append( - &sb, "export default {\n" - " id: 'codebase-memory-augment',\n" - " // V2 config loader: requires id + setup|effect; no tool domain yet\n" - " // at this point, so there is nothing to register here.\n" - " setup() {},\n" - " // Server runtime: reads default.server and dispatches the hooks it returns.\n" - " server: async (ctx) => {\n" - " const dir = ctx?.directory;\n" - " const seen = new Set();\n" - " const lifecycle = () =>\n" - " augment({ hook_event_name: 'SessionStart', cwd: dir });\n" - " return {\n" - " 'tool.execute.after': async (input, output) => {\n" - " if (typeof output?.output !== 'string') return;\n" - " const pieces = [];\n" - " const sid = input?.sessionID;\n" - " if (typeof sid === 'string' && !seen.has(sid)) {\n" - " seen.add(sid);\n" - " pieces.push(await lifecycle());\n" - " }\n" - " const args = input?.args ?? {};\n" - " const search =\n" - " input?.tool === 'grep' ? 'Grep' : input?.tool === 'glob' ? 'Glob' : null;\n" - " if (search) {\n" - " pieces.push(await augment({\n" - " hook_event_name: 'PreToolUse',\n" - " tool_name: search,\n" - " tool_input: args,\n" - " cwd: dir,\n" - " }));\n" - " } else if (input?.tool === 'read') {\n" - " const filePath = args.filePath ?? args.file_path ?? args.path;\n" - " if (typeof filePath === 'string' && filePath) {\n" - " pieces.push(await augment({\n" - " hook_event_name: 'PostToolUse',\n" - " tool_name: 'Read',\n" - " tool_input: { file_path: filePath },\n" - " cwd: dir,\n" - " }));\n" - " }\n" - " }\n" - " const extra = pieces.filter(Boolean).join('\\n');\n" - " if (extra) {\n" - " output.output += '\\n' + extra;\n" - " }\n" - " },\n" - " // Documented (experimental) compaction surface: output.context is the\n" - " // mutable array of context strings for the rebuilt session.\n" - " 'experimental.session.compacting': async (_input, output) => {\n" - " const note = await lifecycle();\n" - " if (note && Array.isArray(output?.context)) {\n" - " output.context.push(note);\n" - " }\n" - " },\n" - " };\n" - " },\n" - "};\n"); + &sb, + "export default {\n" + " id: 'codebase-memory-augment',\n" + " // OpenCode 1.18.x server runtime: reads default.server and dispatches\n" + " // the hooks it returns.\n" + " server: async (ctx) => {\n" + " const dir = ctx?.directory;\n" + " const seen = new Set();\n" + " const lifecycle = () =>\n" + " augment({ hook_event_name: 'SessionStart', cwd: dir });\n" + " return {\n" + " 'tool.execute.after': async (input, output) => {\n" + " if (toolHooksRegistered) return;\n" + " if (typeof output?.output !== 'string') return;\n" + " const pieces = [];\n" + " const sid = input?.sessionID;\n" + " if (typeof sid === 'string' && !seen.has(sid)) {\n" + " seen.add(sid);\n" + " pieces.push(await lifecycle());\n" + " }\n" + " const args = input?.args ?? {};\n" + " const search =\n" + " input?.tool === 'grep' ? 'Grep' : input?.tool === 'glob' ? 'Glob' : null;\n" + " if (search) {\n" + " pieces.push(await augment({\n" + " hook_event_name: 'PreToolUse',\n" + " tool_name: search,\n" + " tool_input: args,\n" + " cwd: dir,\n" + " }));\n" + " } else if (input?.tool === 'read') {\n" + " const filePath = args.filePath ?? args.file_path ?? args.path;\n" + " if (typeof filePath === 'string' && filePath) {\n" + " pieces.push(await augment({\n" + " hook_event_name: 'PostToolUse',\n" + " tool_name: 'Read',\n" + " tool_input: { file_path: filePath },\n" + " cwd: dir,\n" + " }));\n" + " }\n" + " }\n" + " const extra = pieces.filter(Boolean).join('\\n');\n" + " if (extra) {\n" + " output.output += '\\n' + extra;\n" + " }\n" + " },\n" + " // Documented (experimental) compaction surface: output.context is the\n" + " // mutable array of context strings for the rebuilt session.\n" + " 'experimental.session.compacting': async (_input, output) => {\n" + " const note = await lifecycle();\n" + " if (note && Array.isArray(output?.context)) {\n" + " output.context.push(note);\n" + " }\n" + " },\n" + " };\n" + " },\n" + " // OpenCode 2 runtime: default.server is ignored and setup(ctx) registers\n" + " // the tool hook on the context's tool domain. The registration fails\n" + " // open, so a missing domain cannot break the host.\n" + " async setup(ctx) {\n" + " const seen = new Set();\n" + " const dirs = new Map();\n" + " const dirFor = async (sessionID) => {\n" + " if (typeof sessionID !== 'string' || sessionID === '') return undefined;\n" + " if (dirs.has(sessionID)) return dirs.get(sessionID);\n" + " try {\n" + " const session = await ctx.session.get({ sessionID });\n" + " const dir = session?.location?.directory;\n" + " if (typeof dir === 'string' && dir !== '') {\n" + " dirs.set(sessionID, dir);\n" + " return dir;\n" + " }\n" + " } catch {}\n" + " return undefined;\n" + " };\n" + " const lifecycle = (dir) =>\n" + " augment({ hook_event_name: 'SessionStart', cwd: dir });\n" + " if (ctx?.tool?.hook) {\n" + " try {\n" + " await ctx.tool.hook('execute.after', async (event) => {\n" + " if (event?.status !== 'completed') return;\n" + " const dir = await dirFor(event?.sessionID);\n" + " if (!dir) return;\n" + " const pieces = [];\n" + " if (typeof event?.sessionID === 'string' && !seen.has(event.sessionID)) {\n" + " seen.add(event.sessionID);\n" + " pieces.push(await lifecycle(dir));\n" + " }\n" + " const args = event?.input ?? {};\n" + " const search =\n" + " event?.tool === 'grep' ? 'Grep' : event?.tool === 'glob' ? 'Glob' : null;\n" + " if (search) {\n" + " pieces.push(await augment({\n" + " hook_event_name: 'PreToolUse',\n" + " tool_name: search,\n" + " tool_input: args,\n" + " cwd: dir,\n" + " }));\n" + " } else if (event?.tool === 'read') {\n" + " const filePath = args.filePath ?? args.file_path ?? args.path;\n" + " if (typeof filePath === 'string' && filePath) {\n" + " pieces.push(await augment({\n" + " hook_event_name: 'PostToolUse',\n" + " tool_name: 'Read',\n" + " tool_input: { file_path: filePath },\n" + " cwd: dir,\n" + " }));\n" + " }\n" + " }\n" + " const extra = pieces.filter(Boolean).join('\\n');\n" + " if (extra) {\n" + " event.result = appendResult(event.result, extra);\n" + " }\n" + " });\n" + " toolHooksRegistered = true;\n" + " } catch {\n" + " // Fail open: the server() tool hook stays active.\n" + " }\n" + " }\n" + " },\n" + "};\n"); if (sb.failed) { free(sb.buf); diff --git a/src/cli/client_adapter.h b/src/cli/client_adapter.h index c1bfb652e6..1eec930658 100644 --- a/src/cli/client_adapter.h +++ b/src/cli/client_adapter.h @@ -48,11 +48,18 @@ char *cbm_client_adapter_pi(const char *binary_path); * other clients get through their own native hook configuration. OpenCode has * no such configuration; a plugin module is its only extension point. * - * NOTE for maintainers: this hooks `tool.execute.after`, whose ability to - * modify a tool's output is NOT part of OpenCode's documented plugin contract - * (only `tool.execute.before`'s argument mutation is). If OpenCode changes it, - * augmentation stops silently — no error, no signal. That risk is accepted - * deliberately and recorded here so it is not rediscovered as a mystery. + * NOTE for maintainers: the module serves two loader generations from one + * default export. OpenCode 1.18.x dispatches the hooks returned by + * `server(ctx)`; its `tool.execute.after` ability to modify a tool's output is + * NOT part of the documented plugin contract (only `tool.execute.before`'s + * argument mutation is), so if that runtime changes, augmentation stops + * silently — no error, no signal. That risk is accepted deliberately for the + * server() path and recorded here so it is not rediscovered as a mystery. + * OpenCode 2 ignores `server` and calls `setup(ctx)`, which registers + * `execute.after` on the context's tool domain and fails open. It registers no + * `compaction` hook: pushing a plain object into `event.messages` would fail, + * if at all, inside OpenCode's own compaction where the plugin cannot catch it, + * so a note after compaction needs a real 2.x run first. * * Returns NULL on allocation failure or when binary_path is NULL/empty. */ char *cbm_client_adapter_opencode(const char *binary_path); diff --git a/tests/test_agent_clients.c b/tests/test_agent_clients.c index 0d57723a07..c92005f9fb 100644 --- a/tests/test_agent_clients.c +++ b/tests/test_agent_clients.c @@ -1342,23 +1342,101 @@ TEST(client_adapter_opencode_covers_lifecycle_read_and_compaction) { PASS(); } -/* #2077: OpenCode's V2 loader only reads the default export and needs an - * id plus a setup()/effect() function; the old named export had neither. */ +/* #2077/#2089/#2204: the two OpenCode runtimes read different entries of the + * default export. 1.18.x dispatches the hooks returned by `server(ctx)`; 2.x + * ignores `server` and calls `setup(ctx)`, whose context provides the tool hook + * domain. The registration fails open, and the server() tool hook yields once + * it is registered. OpenCode 2 adds no note to a compaction (pinned by the + * compaction test below). */ TEST(client_adapter_opencode_exports_the_v2_default_definition_issue2077) { char *js = cbm_client_adapter_opencode("/usr/local/bin/codebase-memory-mcp"); ASSERT_NOT_NULL(js); ASSERT_NOT_NULL(strstr(js, "export default {")); ASSERT_NOT_NULL(strstr(js, "id: 'codebase-memory-augment'")); - /* Hooks live under server(), which the server runtime reads; setup() - * stays empty since the V2 config loader has no tool domain yet. */ - ASSERT_NOT_NULL(strstr(js, "setup() {}")); + /* OpenCode 1.18.x server runtime. */ ASSERT_NOT_NULL(strstr(js, "server: async (ctx) => {")); - ASSERT_NULL(strstr(js, "async setup(ctx) {")); + /* OpenCode 2 registers through the setup context; the domains exist on + * 2.x, so setup is no longer the empty placeholder #2089 pinned. */ + ASSERT_NOT_NULL(strstr(js, "async setup(ctx) {")); + ASSERT_NOT_NULL(strstr(js, "ctx.tool.hook('execute.after'")); + /* content is the surface the model reads; prefer it over output, and never + * synthesize content that would hide the tool's own result. */ + const char *content_first = strstr(js, "if (typeof result.content === 'string') {"); + const char *output_fallback = strstr(js, "if (typeof result.output === 'string') {"); + ASSERT_NOT_NULL(content_first); + ASSERT_NOT_NULL(output_fallback); + ASSERT_TRUE(content_first < output_fallback); + ASSERT_NULL(strstr(js, "if (result.content === undefined)")); + /* The V2 hooks read the session id with optional chaining (fail open). */ + ASSERT_NOT_NULL(strstr(js, "event?.sessionID")); + /* The plugin instance's location is not the session's location in a + * multi-project server: resolve each session's directory. */ + ASSERT_NOT_NULL(strstr(js, "const dir = await dirFor(")); + ASSERT_NOT_NULL(strstr(js, "ctx.session.get({ sessionID })")); + ASSERT_NOT_NULL(strstr(js, "dirs.has(sessionID)")); + ASSERT_NOT_NULL(strstr(js, "if (!dir) return;")); + /* No fallback to the plugin instance's location: that is not the session's + * location in a multi-project server. */ + ASSERT_NULL(strstr(js, "ctx?.location?.directory")); + ASSERT_NULL(strstr(js, "pendingReinject")); + /* The registration fails open; the server() tool hook yields once it is in. */ + ASSERT_NOT_NULL(strstr(js, "toolHooksRegistered = true;")); + ASSERT_NOT_NULL(strstr(js, "if (toolHooksRegistered) return;")); + ASSERT_NOT_NULL(strstr(js, "catch {")); + ASSERT_NOT_NULL(strstr(js, "event.result = appendResult(event.result, extra);")); + /* The empty setup placeholder and the old named export must both be gone. */ + ASSERT_NULL(strstr(js, "setup() {}")); ASSERT_NULL(strstr(js, "export const CodebaseMemory")); free(js); PASS(); } +/* #2204: OpenCode 2 gets no note after compaction until a compaction channel has + * run on a real 2.x host. A plain object pushed into `event.messages` would be + * rejected, if at all, inside OpenCode's own compaction, where the plugin's + * try/catch cannot reach it, so an untested channel could break compaction for + * every OpenCode 2 user. The note must not reach the system prompt either: not + * through `event.system`, and not through a session instruction entry. The + * 1.18.x server() path keeps its documented compaction surface. */ +TEST(client_adapter_opencode_v2_adds_no_note_to_the_compaction) { + char *js = cbm_client_adapter_opencode("/usr/local/bin/codebase-memory-mcp"); + ASSERT_NOT_NULL(js); + ASSERT_NULL(strstr(js, "ctx.session.hook(")); + ASSERT_NULL(strstr(js, "event.messages")); + ASSERT_NULL(strstr(js, "sessionHooksRegistered")); + ASSERT_NULL(strstr(js, ".system")); + ASSERT_NULL(strstr(js, "instructions")); + ASSERT_NULL(strstr(js, "entry.put")); + ASSERT_NOT_NULL(strstr(js, "'experimental.session.compacting'")); + free(js); + PASS(); +} + +/* #2204: Tool.Result allows `content: []`. appendResult adds a text part to a + * parts array only when the array already holds a part; an empty array falls + * through to the string `output` (or the unchanged result), so the note never + * becomes the whole content of a tool result. */ +TEST(client_adapter_opencode_v2_append_result_never_creates_note_only_content) { + char *js = cbm_client_adapter_opencode("/usr/local/bin/codebase-memory-mcp"); + ASSERT_NOT_NULL(js); + const char *parts = + strstr(js, "if (Array.isArray(result.content) && result.content.length > 0) {"); + const char *add_part = strstr(js, "content.push({ type: 'text', text: extra });"); + const char *output_fallback = strstr(js, "if (typeof result.output === 'string') {"); + ASSERT_NOT_NULL(parts); + ASSERT_NOT_NULL(add_part); + ASSERT_NOT_NULL(output_fallback); + /* The only part-adding push sits inside the non-empty branch; the string + * output fallback, where `content: []` lands, comes after it. */ + ASSERT_TRUE(parts < add_part); + ASSERT_TRUE(add_part < output_fallback); + ASSERT_NULL(strstr(add_part + 1, "content.push(")); + /* No unguarded array branch is left. */ + ASSERT_NULL(strstr(js, "if (Array.isArray(result.content)) {")); + free(js); + PASS(); +} + /* augment() is awaited inline inside 'tool.execute.after', so a hook-augment * that never exits would stall every grep/glob/read completion in the host * session until the agent is restarted. The generated plugin must bound the @@ -1431,6 +1509,8 @@ SUITE(agent_clients) { RUN_TEST(client_adapter_opencode_sends_the_required_hook_event); RUN_TEST(client_adapter_opencode_covers_lifecycle_read_and_compaction); RUN_TEST(client_adapter_opencode_exports_the_v2_default_definition_issue2077); + RUN_TEST(client_adapter_opencode_v2_adds_no_note_to_the_compaction); + RUN_TEST(client_adapter_opencode_v2_append_result_never_creates_note_only_content); RUN_TEST(client_adapter_opencode_bounds_a_hung_hook_augment_child); RUN_TEST(client_adapter_rejects_missing_binary_path); }