From 534274c1601034b5eca97ec286c0346389076a8f Mon Sep 17 00:00:00 2001 From: HazielMagallanes Date: Mon, 28 Sep 2026 20:45:46 -0300 Subject: [PATCH] fix(cli): emit OpenCode plugin hooks for the V2 setup context - keep the `server(ctx)` entry that OpenCode 1.18.x dispatches hooks from - fill `setup(ctx)` with the V2 hook registrations (execute.after, compaction, context) plus the domain guard for older V2 preview loaders, which stay on the server() path - prefer `content` over `output` in appendResult and fall back to a string `output`, so the note still lands when no content surface exists - key post-compaction reinjection by session id and yield in the server() hooks once setup() has registered, preventing double appends - update the #2089 emission test in place to state the 2.x contract Fixes #2204 Refs #2077 #2089 Signed-off-by: HazielMagallanes --- src/cli/client_adapter.c | 118 ++++++++++++++++++++++++++++++++++--- src/cli/client_adapter.h | 15 +++-- tests/test_agent_clients.c | 75 ++++++++++++++--------- 3 files changed, 169 insertions(+), 39 deletions(-) diff --git a/src/cli/client_adapter.c b/src/cli/client_adapter.c index 68feb61027..39eeb33b90 100644 --- a/src/cli/client_adapter.c +++ b/src/cli/client_adapter.c @@ -429,9 +429,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" + "// 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" + "// same behaviour on the context's tool and session hook domains.\n" + "// setup returns early when those domains are absent, so older V2\n" + "// preview loaders that accept the definition but expose no domains\n" + "// are unaffected and keep using server().\n"); sb_append(&sb, "import { spawn } from 'node:child_process';\n\n"); sb_append(&sb, "const BIN = '"); sb_append(&sb, bin); @@ -461,13 +468,48 @@ 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 so the context reaches the model even + * when a tool fills both, then fall back to a string output, and finally + * to content when no text surface exists at all. 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)) {\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" + " if (result.content === undefined) {\n" + " return { ...result, content: extra };\n" + " }\n" + " return result;\n" + "}\n\n"); + + /* A host that both dispatches server() hooks and calls setup() with + * domains would append twice; the server hooks yield once setup has + * registered, and domain-less loaders stay on the server() path. */ + sb_append(&sb, "let setupRegistered = 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" + " // 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" @@ -475,6 +517,7 @@ char *cbm_client_adapter_opencode(const char *binary_path) { " augment({ hook_event_name: 'SessionStart', cwd: dir });\n" " return {\n" " 'tool.execute.after': async (input, output) => {\n" + " if (setupRegistered) return;\n" " if (typeof output?.output !== 'string') return;\n" " const pieces = [];\n" " const sid = input?.sessionID;\n" @@ -511,6 +554,7 @@ char *cbm_client_adapter_opencode(const char *binary_path) { " // 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" + " if (setupRegistered) return;\n" " const note = await lifecycle();\n" " if (note && Array.isArray(output?.context)) {\n" " output.context.push(note);\n" @@ -518,6 +562,66 @@ char *cbm_client_adapter_opencode(const char *binary_path) { " },\n" " };\n" " },\n" + " // OpenCode 2 runtime: default.server is ignored and setup(ctx) registers\n" + " // the hooks on the context domains. The guard keeps older V2 preview\n" + " // loaders (which expose no tool/session domains) from throwing and\n" + " // leaves them on the server() path.\n" + " async setup(ctx) {\n" + " if (!ctx?.tool?.hook || !ctx?.session?.hook) return;\n" + " const dir = ctx?.location?.directory;\n" + " const seen = new Set();\n" + " const pendingReinject = new Set();\n" + " const lifecycle = () =>\n" + " augment({ hook_event_name: 'SessionStart', cwd: dir });\n" + " await ctx.tool.hook('execute.after', async (event) => {\n" + " if (event?.status !== 'completed') return;\n" + " const pieces = [];\n" + " if (typeof event.sessionID === 'string' && !seen.has(event.sessionID)) {\n" + " seen.add(event.sessionID);\n" + " pieces.push(await lifecycle());\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" + " // OpenCode 2 compaction hooks: mark that session's next context\n" + " // request and reinject the session-start note once, like the V1\n" + " // surface did. Keyed by session so a compaction in one session\n" + " // cannot steer another session's reinjection.\n" + " await ctx.session.hook('compaction', (event) => {\n" + " pendingReinject.add(event.sessionID);\n" + " });\n" + " await ctx.session.hook('context', async (event) => {\n" + " if (!pendingReinject.delete(event.sessionID)) return;\n" + " const note = await lifecycle();\n" + " if (note && Array.isArray(event?.system)) {\n" + " event.system.push({ type: 'text', text: note });\n" + " }\n" + " });\n" + " setupRegistered = true;\n" + " },\n" "};\n"); if (sb.failed) { diff --git a/src/cli/client_adapter.h b/src/cli/client_adapter.h index c1bfb652e6..be32edc384 100644 --- a/src/cli/client_adapter.h +++ b/src/cli/client_adapter.h @@ -48,11 +48,16 @@ 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`/`compaction`/`context` on the context domains and replaces + * `event.result` through the documented hook surface. * * 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 df8515e73c..4f1ffee49f 100644 --- a/tests/test_agent_clients.c +++ b/tests/test_agent_clients.c @@ -159,10 +159,10 @@ static char *agent_deep_same_name_json(size_t depth) { TEST(agent_clients_registry_is_stable_and_callback_driven) { static const char *expected[] = { - "qoder", "kimi", "gitlab-duo", "rovo-dev", "amp", "devin", - "tabnine", "continue", "visual-studio", "trae", "roo-code", "amazon-q", - "codebuddy", "ibm-bob-ide", "ibm-bob-shell", "pochi", "pi", "sourcegraph-cody", - "omp", + "qoder", "kimi", "gitlab-duo", "rovo-dev", "amp", + "devin", "tabnine", "continue", "visual-studio", "trae", + "roo-code", "amazon-q", "codebuddy", "ibm-bob-ide", "ibm-bob-shell", + "pochi", "pi", "sourcegraph-cody", "omp", }; static const uint32_t expected_capabilities[] = { CBM_AGENT_CAP_MCP | CBM_AGENT_CAP_SKILL | CBM_AGENT_CAP_AGENT | CBM_AGENT_CAP_HOOK, @@ -866,11 +866,8 @@ TEST(agent_clients_json_schemas_are_exact_and_policy_neutral) { TEST(agent_clients_new_standard_json_profiles_preserve_foreign_entries) { static const cbm_agent_client_id_t clients[] = { - CBM_AGENT_CLIENT_CODEBUDDY, - CBM_AGENT_CLIENT_IBM_BOB_IDE, - CBM_AGENT_CLIENT_IBM_BOB_SHELL, - CBM_AGENT_CLIENT_POCHI, - CBM_AGENT_CLIENT_OMP, + CBM_AGENT_CLIENT_CODEBUDDY, CBM_AGENT_CLIENT_IBM_BOB_IDE, CBM_AGENT_CLIENT_IBM_BOB_SHELL, + CBM_AGENT_CLIENT_POCHI, CBM_AGENT_CLIENT_OMP, }; for (size_t i = 0U; i < sizeof(clients) / sizeof(clients[0]); i++) { const char *foreign = @@ -941,30 +938,29 @@ TEST(agent_clients_omp_resolves_to_injected_agent_dir_when_provided) { char resolved[512]; /* Default (no env) keeps the documented ~/.omp/agent path. */ - ASSERT_EQ(cbm_agent_client_resolve_path(CBM_AGENT_CLIENT_OMP, &options, resolved, - sizeof(resolved)), - 0); + ASSERT_EQ( + cbm_agent_client_resolve_path(CBM_AGENT_CLIENT_OMP, &options, resolved, sizeof(resolved)), + 0); ASSERT_STR_EQ(resolved, "/home/tester/.omp/agent/mcp.json"); /* Named profile directory takes precedence over the home-relative default. */ options.omp_agent_dir = "/home/tester/.omp/profiles/work/agent"; - ASSERT_EQ(cbm_agent_client_resolve_path(CBM_AGENT_CLIENT_OMP, &options, resolved, - sizeof(resolved)), - 0); + ASSERT_EQ( + cbm_agent_client_resolve_path(CBM_AGENT_CLIENT_OMP, &options, resolved, sizeof(resolved)), + 0); ASSERT_STR_EQ(resolved, "/home/tester/.omp/profiles/work/agent/mcp.json"); /* PI_CODING_AGENT_DIR relocations also flow through the resolved option. */ options.omp_agent_dir = "/srv/omp-shared/agent"; - ASSERT_EQ(cbm_agent_client_resolve_path(CBM_AGENT_CLIENT_OMP, &options, resolved, - sizeof(resolved)), - 0); + ASSERT_EQ( + cbm_agent_client_resolve_path(CBM_AGENT_CLIENT_OMP, &options, resolved, sizeof(resolved)), + 0); ASSERT_STR_EQ(resolved, "/srv/omp-shared/agent/mcp.json"); PASS(); } TEST(agent_clients_omp_profile_does_not_register_global_instructions_capability) { - const cbm_agent_client_profile_t *profile = - cbm_agent_client_by_id(CBM_AGENT_CLIENT_OMP); + const cbm_agent_client_profile_t *profile = cbm_agent_client_by_id(CBM_AGENT_CLIENT_OMP); ASSERT_NOT_NULL(profile); ASSERT_EQ(profile->capabilities & CBM_AGENT_CAP_INSTRUCTIONS, 0U); ASSERT_NEQ(profile->capabilities & CBM_AGENT_CAP_MCP, 0U); @@ -973,7 +969,6 @@ TEST(agent_clients_omp_profile_does_not_register_global_instructions_capability) PASS(); } - TEST(agent_clients_remove_only_canonical_and_missing_is_noop) { char *dir = NULL; char *path = agent_fixture("{\"keep\":true}\n", &dir); @@ -1321,18 +1316,44 @@ 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: OpenCode's loaders changed shape across releases. The + * 1.18.x server runtime dispatches the hooks returned by default.server; the + * 2.x runtime ignores server and calls setup(ctx), whose context provides the + * tool and session hook domains, so the same augmentation is registered + * there. Older V2 preview loaders expose no domains; setup returns early for + * them instead of throwing. */ 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; those 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'")); + ASSERT_NOT_NULL(strstr(js, "ctx.session.hook('compaction'")); + ASSERT_NOT_NULL(strstr(js, "ctx.session.hook('context'")); + ASSERT_NOT_NULL(strstr(js, "if (!ctx?.tool?.hook || !ctx?.session?.hook) return;")); + /* content is the surface the model reads; prefer it over output. */ + 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_NOT_NULL(strstr(js, "if (result.content === undefined) {")); + /* Reinjection is tracked per session; server() yields once setup() has + * registered, so a host dispatching both entries cannot append twice. */ + ASSERT_NOT_NULL(strstr(js, "const pendingReinject = new Set();")); + ASSERT_NOT_NULL(strstr(js, "pendingReinject.add(event.sessionID);")); + ASSERT_NOT_NULL(strstr(js, "if (!pendingReinject.delete(event.sessionID)) return;")); + ASSERT_NOT_NULL(strstr(js, "let setupRegistered = false;")); + ASSERT_NOT_NULL(strstr(js, "if (setupRegistered) return;")); + ASSERT_NOT_NULL(strstr(js, "setupRegistered = true;")); + 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();