Skip to content

fix(cli): register OpenCode 2 hooks via the setup context #2385

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
HazielMagallanes wants to merge 1 commit into
base: main
Choose a base branch
from
+169 −39
Open

fix(cli): register OpenCode 2 hooks via the setup context #2385

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 111 additions & 7 deletions src/cli/client_adapter.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -461,20 +468,56 @@ 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"
" const lifecycle = () =>\n"
" 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"
Expand Down Expand Up @@ -511,13 +554,74 @@ 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"
" }\n"
" },\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) {
Expand Down
15 changes: 10 additions & 5 deletions src/cli/client_adapter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
75 changes: 48 additions & 27 deletions tests/test_agent_clients.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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();
Expand Down
Loading