Skip to content

Commit 534274c

Browse files
HazielMagallanesHazielMagallanes
committed
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 <contactame.haziel@gmail.com>
1 parent 80eb92a commit 534274c

3 files changed

Lines changed: 169 additions & 39 deletions

File tree

‎src/cli/client_adapter.c‎

Lines changed: 111 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -429,9 +429,16 @@ char *cbm_client_adapter_opencode(const char *binary_path) {
429429
sb_append(&sb, "// OpenCode already reaches every tool over MCP; this module adds the\n"
430430
"// context surfaces other clients get through hook configuration: graph\n"
431431
"// lookup after grep/glob, index-coverage notes after read, session-start\n"
432-
"// tier routing (carried on the first tool result of each session, since\n"
433-
"// OpenCode documents no context-output lifecycle hook), and reinjection\n"
434-
"// after compaction via the documented experimental surface.\n");
432+
"// tier routing (carried on the first tool result of each session), and\n"
433+
"// reinjection after compaction.\n"
434+
"//\n"
435+
"// Two runtimes load this module:\n"
436+
"// - OpenCode 1.18.x dispatches the hooks returned by server(ctx).\n"
437+
"// - OpenCode 2 ignores server and calls setup(ctx), which registers the\n"
438+
"// same behaviour on the context's tool and session hook domains.\n"
439+
"// setup returns early when those domains are absent, so older V2\n"
440+
"// preview loaders that accept the definition but expose no domains\n"
441+
"// are unaffected and keep using server().\n");
435442
sb_append(&sb, "import { spawn } from 'node:child_process';\n\n");
436443
sb_append(&sb, "const BIN = '");
437444
sb_append(&sb, bin);
@@ -461,20 +468,56 @@ char *cbm_client_adapter_opencode(const char *binary_path) {
461468
" });\n"
462469
"}\n\n");
463470

471+
/* OpenCode 2 tool results are structured (Tool.Result: content is the
472+
* string/parts surface the model reads, output is the tool's schema-typed
473+
* value). Append to content first so the context reaches the model even
474+
* when a tool fills both, then fall back to a string output, and finally
475+
* to content when no text surface exists at all. The caller assigns the
476+
* returned result onto event.result. */
477+
sb_append(&sb,
478+
"function appendResult(result, extra) {\n"
479+
" if (!result || typeof result !== 'object') return result;\n"
480+
" if (typeof result.content === 'string') {\n"
481+
" return { ...result, content: result.content + '\\n' + extra };\n"
482+
" }\n"
483+
" if (Array.isArray(result.content)) {\n"
484+
" const content = result.content.slice();\n"
485+
" for (let i = content.length - 1; i >= 0; i--) {\n"
486+
" if (content[i] && content[i].type === 'text') {\n"
487+
" content[i] = { ...content[i], text: content[i].text + '\\n' + extra };\n"
488+
" return { ...result, content };\n"
489+
" }\n"
490+
" }\n"
491+
" content.push({ type: 'text', text: extra });\n"
492+
" return { ...result, content };\n"
493+
" }\n"
494+
" if (typeof result.output === 'string') {\n"
495+
" return { ...result, output: result.output + '\\n' + extra };\n"
496+
" }\n"
497+
" if (result.content === undefined) {\n"
498+
" return { ...result, content: extra };\n"
499+
" }\n"
500+
" return result;\n"
501+
"}\n\n");
502+
503+
/* A host that both dispatches server() hooks and calls setup() with
504+
* domains would append twice; the server hooks yield once setup has
505+
* registered, and domain-less loaders stay on the server() path. */
506+
sb_append(&sb, "let setupRegistered = false;\n\n");
507+
464508
sb_append(
465509
&sb, "export default {\n"
466510
" id: 'codebase-memory-augment',\n"
467-
" // V2 config loader: requires id + setup|effect; no tool domain yet\n"
468-
" // at this point, so there is nothing to register here.\n"
469-
" setup() {},\n"
470-
" // Server runtime: reads default.server and dispatches the hooks it returns.\n"
511+
" // OpenCode 1.18.x server runtime: reads default.server and dispatches\n"
512+
" // the hooks it returns.\n"
471513
" server: async (ctx) => {\n"
472514
" const dir = ctx?.directory;\n"
473515
" const seen = new Set();\n"
474516
" const lifecycle = () =>\n"
475517
" augment({ hook_event_name: 'SessionStart', cwd: dir });\n"
476518
" return {\n"
477519
" 'tool.execute.after': async (input, output) => {\n"
520+
" if (setupRegistered) return;\n"
478521
" if (typeof output?.output !== 'string') return;\n"
479522
" const pieces = [];\n"
480523
" const sid = input?.sessionID;\n"
@@ -511,13 +554,74 @@ char *cbm_client_adapter_opencode(const char *binary_path) {
511554
" // Documented (experimental) compaction surface: output.context is the\n"
512555
" // mutable array of context strings for the rebuilt session.\n"
513556
" 'experimental.session.compacting': async (_input, output) => {\n"
557+
" if (setupRegistered) return;\n"
514558
" const note = await lifecycle();\n"
515559
" if (note && Array.isArray(output?.context)) {\n"
516560
" output.context.push(note);\n"
517561
" }\n"
518562
" },\n"
519563
" };\n"
520564
" },\n"
565+
" // OpenCode 2 runtime: default.server is ignored and setup(ctx) registers\n"
566+
" // the hooks on the context domains. The guard keeps older V2 preview\n"
567+
" // loaders (which expose no tool/session domains) from throwing and\n"
568+
" // leaves them on the server() path.\n"
569+
" async setup(ctx) {\n"
570+
" if (!ctx?.tool?.hook || !ctx?.session?.hook) return;\n"
571+
" const dir = ctx?.location?.directory;\n"
572+
" const seen = new Set();\n"
573+
" const pendingReinject = new Set();\n"
574+
" const lifecycle = () =>\n"
575+
" augment({ hook_event_name: 'SessionStart', cwd: dir });\n"
576+
" await ctx.tool.hook('execute.after', async (event) => {\n"
577+
" if (event?.status !== 'completed') return;\n"
578+
" const pieces = [];\n"
579+
" if (typeof event.sessionID === 'string' && !seen.has(event.sessionID)) {\n"
580+
" seen.add(event.sessionID);\n"
581+
" pieces.push(await lifecycle());\n"
582+
" }\n"
583+
" const args = event.input ?? {};\n"
584+
" const search =\n"
585+
" event?.tool === 'grep' ? 'Grep' : event?.tool === 'glob' ? 'Glob' : null;\n"
586+
" if (search) {\n"
587+
" pieces.push(await augment({\n"
588+
" hook_event_name: 'PreToolUse',\n"
589+
" tool_name: search,\n"
590+
" tool_input: args,\n"
591+
" cwd: dir,\n"
592+
" }));\n"
593+
" } else if (event?.tool === 'read') {\n"
594+
" const filePath = args.filePath ?? args.file_path ?? args.path;\n"
595+
" if (typeof filePath === 'string' && filePath) {\n"
596+
" pieces.push(await augment({\n"
597+
" hook_event_name: 'PostToolUse',\n"
598+
" tool_name: 'Read',\n"
599+
" tool_input: { file_path: filePath },\n"
600+
" cwd: dir,\n"
601+
" }));\n"
602+
" }\n"
603+
" }\n"
604+
" const extra = pieces.filter(Boolean).join('\\n');\n"
605+
" if (extra) {\n"
606+
" event.result = appendResult(event.result, extra);\n"
607+
" }\n"
608+
" });\n"
609+
" // OpenCode 2 compaction hooks: mark that session's next context\n"
610+
" // request and reinject the session-start note once, like the V1\n"
611+
" // surface did. Keyed by session so a compaction in one session\n"
612+
" // cannot steer another session's reinjection.\n"
613+
" await ctx.session.hook('compaction', (event) => {\n"
614+
" pendingReinject.add(event.sessionID);\n"
615+
" });\n"
616+
" await ctx.session.hook('context', async (event) => {\n"
617+
" if (!pendingReinject.delete(event.sessionID)) return;\n"
618+
" const note = await lifecycle();\n"
619+
" if (note && Array.isArray(event?.system)) {\n"
620+
" event.system.push({ type: 'text', text: note });\n"
621+
" }\n"
622+
" });\n"
623+
" setupRegistered = true;\n"
624+
" },\n"
521625
"};\n");
522626

523627
if (sb.failed) {

‎src/cli/client_adapter.h‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,16 @@ char *cbm_client_adapter_pi(const char *binary_path);
4848
* other clients get through their own native hook configuration. OpenCode has
4949
* no such configuration; a plugin module is its only extension point.
5050
*
51-
* NOTE for maintainers: this hooks `tool.execute.after`, whose ability to
52-
* modify a tool's output is NOT part of OpenCode's documented plugin contract
53-
* (only `tool.execute.before`'s argument mutation is). If OpenCode changes it,
54-
* augmentation stops silently — no error, no signal. That risk is accepted
55-
* deliberately and recorded here so it is not rediscovered as a mystery.
51+
* NOTE for maintainers: the module serves two loader generations from one
52+
* default export. OpenCode 1.18.x dispatches the hooks returned by
53+
* `server(ctx)`; its `tool.execute.after` ability to modify a tool's output is
54+
* NOT part of the documented plugin contract (only `tool.execute.before`'s
55+
* argument mutation is), so if that runtime changes, augmentation stops
56+
* silently — no error, no signal. That risk is accepted deliberately for the
57+
* server() path and recorded here so it is not rediscovered as a mystery.
58+
* OpenCode 2 ignores `server` and calls `setup(ctx)`, which registers
59+
* `execute.after`/`compaction`/`context` on the context domains and replaces
60+
* `event.result` through the documented hook surface.
5661
*
5762
* Returns NULL on allocation failure or when binary_path is NULL/empty. */
5863
char *cbm_client_adapter_opencode(const char *binary_path);

‎tests/test_agent_clients.c‎

Lines changed: 48 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -159,10 +159,10 @@ static char *agent_deep_same_name_json(size_t depth) {
159159

160160
TEST(agent_clients_registry_is_stable_and_callback_driven) {
161161
static const char *expected[] = {
162-
"qoder", "kimi", "gitlab-duo", "rovo-dev", "amp", "devin",
163-
"tabnine", "continue", "visual-studio", "trae", "roo-code", "amazon-q",
164-
"codebuddy", "ibm-bob-ide", "ibm-bob-shell", "pochi", "pi", "sourcegraph-cody",
165-
"omp",
162+
"qoder", "kimi", "gitlab-duo", "rovo-dev", "amp",
163+
"devin", "tabnine", "continue", "visual-studio", "trae",
164+
"roo-code", "amazon-q", "codebuddy", "ibm-bob-ide", "ibm-bob-shell",
165+
"pochi", "pi", "sourcegraph-cody", "omp",
166166
};
167167
static const uint32_t expected_capabilities[] = {
168168
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) {
866866

867867
TEST(agent_clients_new_standard_json_profiles_preserve_foreign_entries) {
868868
static const cbm_agent_client_id_t clients[] = {
869-
CBM_AGENT_CLIENT_CODEBUDDY,
870-
CBM_AGENT_CLIENT_IBM_BOB_IDE,
871-
CBM_AGENT_CLIENT_IBM_BOB_SHELL,
872-
CBM_AGENT_CLIENT_POCHI,
873-
CBM_AGENT_CLIENT_OMP,
869+
CBM_AGENT_CLIENT_CODEBUDDY, CBM_AGENT_CLIENT_IBM_BOB_IDE, CBM_AGENT_CLIENT_IBM_BOB_SHELL,
870+
CBM_AGENT_CLIENT_POCHI, CBM_AGENT_CLIENT_OMP,
874871
};
875872
for (size_t i = 0U; i < sizeof(clients) / sizeof(clients[0]); i++) {
876873
const char *foreign =
@@ -941,30 +938,29 @@ TEST(agent_clients_omp_resolves_to_injected_agent_dir_when_provided) {
941938
char resolved[512];
942939

943940
/* Default (no env) keeps the documented ~/.omp/agent path. */
944-
ASSERT_EQ(cbm_agent_client_resolve_path(CBM_AGENT_CLIENT_OMP, &options, resolved,
945-
sizeof(resolved)),
946-
0);
941+
ASSERT_EQ(
942+
cbm_agent_client_resolve_path(CBM_AGENT_CLIENT_OMP, &options, resolved, sizeof(resolved)),
943+
0);
947944
ASSERT_STR_EQ(resolved, "/home/tester/.omp/agent/mcp.json");
948945

949946
/* Named profile directory takes precedence over the home-relative default. */
950947
options.omp_agent_dir = "/home/tester/.omp/profiles/work/agent";
951-
ASSERT_EQ(cbm_agent_client_resolve_path(CBM_AGENT_CLIENT_OMP, &options, resolved,
952-
sizeof(resolved)),
953-
0);
948+
ASSERT_EQ(
949+
cbm_agent_client_resolve_path(CBM_AGENT_CLIENT_OMP, &options, resolved, sizeof(resolved)),
950+
0);
954951
ASSERT_STR_EQ(resolved, "/home/tester/.omp/profiles/work/agent/mcp.json");
955952

956953
/* PI_CODING_AGENT_DIR relocations also flow through the resolved option. */
957954
options.omp_agent_dir = "/srv/omp-shared/agent";
958-
ASSERT_EQ(cbm_agent_client_resolve_path(CBM_AGENT_CLIENT_OMP, &options, resolved,
959-
sizeof(resolved)),
960-
0);
955+
ASSERT_EQ(
956+
cbm_agent_client_resolve_path(CBM_AGENT_CLIENT_OMP, &options, resolved, sizeof(resolved)),
957+
0);
961958
ASSERT_STR_EQ(resolved, "/srv/omp-shared/agent/mcp.json");
962959
PASS();
963960
}
964961

965962
TEST(agent_clients_omp_profile_does_not_register_global_instructions_capability) {
966-
const cbm_agent_client_profile_t *profile =
967-
cbm_agent_client_by_id(CBM_AGENT_CLIENT_OMP);
963+
const cbm_agent_client_profile_t *profile = cbm_agent_client_by_id(CBM_AGENT_CLIENT_OMP);
968964
ASSERT_NOT_NULL(profile);
969965
ASSERT_EQ(profile->capabilities & CBM_AGENT_CAP_INSTRUCTIONS, 0U);
970966
ASSERT_NEQ(profile->capabilities & CBM_AGENT_CAP_MCP, 0U);
@@ -973,7 +969,6 @@ TEST(agent_clients_omp_profile_does_not_register_global_instructions_capability)
973969
PASS();
974970
}
975971

976-
977972
TEST(agent_clients_remove_only_canonical_and_missing_is_noop) {
978973
char *dir = NULL;
979974
char *path = agent_fixture("{\"keep\":true}\n", &dir);
@@ -1321,18 +1316,44 @@ TEST(client_adapter_opencode_covers_lifecycle_read_and_compaction) {
13211316
PASS();
13221317
}
13231318

1324-
/* #2077: OpenCode's V2 loader only reads the default export and needs an
1325-
* id plus a setup()/effect() function; the old named export had neither. */
1319+
/* #2077/#2089/#2204: OpenCode's loaders changed shape across releases. The
1320+
* 1.18.x server runtime dispatches the hooks returned by default.server; the
1321+
* 2.x runtime ignores server and calls setup(ctx), whose context provides the
1322+
* tool and session hook domains, so the same augmentation is registered
1323+
* there. Older V2 preview loaders expose no domains; setup returns early for
1324+
* them instead of throwing. */
13261325
TEST(client_adapter_opencode_exports_the_v2_default_definition_issue2077) {
13271326
char *js = cbm_client_adapter_opencode("/usr/local/bin/codebase-memory-mcp");
13281327
ASSERT_NOT_NULL(js);
13291328
ASSERT_NOT_NULL(strstr(js, "export default {"));
13301329
ASSERT_NOT_NULL(strstr(js, "id: 'codebase-memory-augment'"));
1331-
/* Hooks live under server(), which the server runtime reads; setup()
1332-
* stays empty since the V2 config loader has no tool domain yet. */
1333-
ASSERT_NOT_NULL(strstr(js, "setup() {}"));
1330+
/* OpenCode 1.18.x server runtime. */
13341331
ASSERT_NOT_NULL(strstr(js, "server: async (ctx) => {"));
1335-
ASSERT_NULL(strstr(js, "async setup(ctx) {"));
1332+
/* OpenCode 2 registers through the setup context; those domains exist on
1333+
* 2.x, so setup is no longer the empty placeholder #2089 pinned. */
1334+
ASSERT_NOT_NULL(strstr(js, "async setup(ctx) {"));
1335+
ASSERT_NOT_NULL(strstr(js, "ctx.tool.hook('execute.after'"));
1336+
ASSERT_NOT_NULL(strstr(js, "ctx.session.hook('compaction'"));
1337+
ASSERT_NOT_NULL(strstr(js, "ctx.session.hook('context'"));
1338+
ASSERT_NOT_NULL(strstr(js, "if (!ctx?.tool?.hook || !ctx?.session?.hook) return;"));
1339+
/* content is the surface the model reads; prefer it over output. */
1340+
const char *content_first = strstr(js, "if (typeof result.content === 'string') {");
1341+
const char *output_fallback = strstr(js, "if (typeof result.output === 'string') {");
1342+
ASSERT_NOT_NULL(content_first);
1343+
ASSERT_NOT_NULL(output_fallback);
1344+
ASSERT_TRUE(content_first < output_fallback);
1345+
ASSERT_NOT_NULL(strstr(js, "if (result.content === undefined) {"));
1346+
/* Reinjection is tracked per session; server() yields once setup() has
1347+
* registered, so a host dispatching both entries cannot append twice. */
1348+
ASSERT_NOT_NULL(strstr(js, "const pendingReinject = new Set();"));
1349+
ASSERT_NOT_NULL(strstr(js, "pendingReinject.add(event.sessionID);"));
1350+
ASSERT_NOT_NULL(strstr(js, "if (!pendingReinject.delete(event.sessionID)) return;"));
1351+
ASSERT_NOT_NULL(strstr(js, "let setupRegistered = false;"));
1352+
ASSERT_NOT_NULL(strstr(js, "if (setupRegistered) return;"));
1353+
ASSERT_NOT_NULL(strstr(js, "setupRegistered = true;"));
1354+
ASSERT_NOT_NULL(strstr(js, "event.result = appendResult(event.result, extra);"));
1355+
/* The empty setup placeholder and the old named export must both be gone. */
1356+
ASSERT_NULL(strstr(js, "setup() {}"));
13361357
ASSERT_NULL(strstr(js, "export const CodebaseMemory"));
13371358
free(js);
13381359
PASS();

0 commit comments

Comments
 (0)