feat(opencode): register ctx tools natively via plugin (#574)#597
Conversation
|
@omercnet Do we need CONTEXT_MODE_IDLE_TIMEOUT_MS logic still? Can we should revert that completely? @ousamabenyounes That's death code i guess. |
c5dc414 to
0087f4f
Compare
|
Addressed the concern in the latest push: OpenCode/Kilo no longer use idle-shutdown as a mitigation because plugin-native tools remove the redundant MCP child entirely. I also added an upgrade-path cleanup so removes any legacy block from existing opencode/kilo configs, with a regression test proving other MCP servers are preserved. |
|
Correction / expanded answer: addressed the concern in the latest push. OpenCode/Kilo no longer use idle-shutdown as a mitigation because plugin-native tools remove the redundant MCP child entirely. I also added an upgrade-path cleanup: |
Why you don't removed CONTEXT_MODE_IDLE_TIMEOUT_MS logic? |
0087f4f to
424bbce
Compare
|
Handled in the latest push: removed the What changed:
Validation after removal:
|
|
I’m not fully comfortable merging this yet. Given the regression we had from the previous PR, and the fact that this PR also initially left obvious tech debt behind until it was pointed out, I think we should do a deeper review before merging. There is a lot of AI-generated work and desicion and forward and involved here, and we should be extra careful not to leave dead code, legacy logic, or unused mitigation paths in the codebase! |
424bbce to
e1b15e1
Compare
|
Addressed the code-review findings in the latest push:
Validation after this push:
|
e1b15e1 to
f040f35
Compare
|
@mksglu please reconsider - this approach eliminates the need for a process entirely |
Sure. Please give me time I'm gonna deeply review |
Empirical E2E verification — red/green delta, real OpenCode + real LLM sessionTL;DRDoes this PR actually fix something? Yes.
The user-visible architectural win: a plugin-only OpenCode config becomes self-sufficient. The plugin loads, registers SetupPlugin actually loads in opencode session
Red / Green via real LLM session (same model, same prompt)Prompt: Without PR #597 ( 10 tools, all OpenCode built-ins. Zero With PR #597 ( 11 Direct factory invocation (deterministic — no LLM variance)To eliminate LLM variance, a Node script imports Same script against API surface cross-check vs upstream sst/opencode @ dev
export function tool<Args extends z.ZodRawShape>(input: {
description: string
args: Args
execute(args: z.infer<z.ZodObject<Args>>, context: ToolContext): Promise<ToolResult>
})
export type ToolResult = string | {
title?: string; output: string; metadata?: { [key: string]: any }; attachments?: ToolAttachment[]
}PR #597's type NativeToolDefinition = {
description: string;
args: Record<string, unknown>;
execute: (args, ctx) => Promise<string | { title?: string; output: string; metadata?: Record<string, unknown> }>;
};Structural match. Each of the 5 hook signatures was line-matched against
Full test suite — no regressionPR #597 branch: Baseline PR #597 adds 3 new test files ( What was NOT verified
Branch hygiene noteThe PR head ( Net verdict from this empirical pass
Reopen courtesy noteReopening so these test artifacts stay attached to a live PR thread (easier to navigate than a closed-PR comment). No merge intent from my side — @mksglu your deep-review call still owns this. I'm posting the empirical results so the conversation can rest on observed behaviour rather than inferred behaviour. |
81e85c6 to
9e5451c
Compare
|
adding upgrade path |
4b8d344 to
2219d67
Compare
|
Added the upgrade-scenario handling you asked about:
Regression tests added for all three cases. |
|
The plumbing is verified end-to-end on this branch (
Two small nits before merge — both ~10min fixes: 1.
|
Move OpenCode/Kilo from plugin+MCP dual registration to plugin-native ctx_* tools. The plugin now imports the shared server tool registry without starting stdio, exposes all 11 ctx_* tools via the OpenCode/Kilo tool map, and uses AsyncLocalStorage to pass project/session context into existing handlers without a process.env race. Upgrade safety for existing users: - configureAllHooks removes only legacy mcp.context-mode while preserving other MCP servers. - doctor warns when a legacy mcp.context-mode block remains and points to context-mode upgrade. - stale legacy OpenCode/Kilo MCP children suppress ctx_* registration and become no-op rather than exposing duplicate tools. Safety cleanup: - Remove CONTEXT_MODE_IDLE_TIMEOUT_MS entirely; plugin-native tools remove the need for timer-driven MCP death, and timer shutdown was unsafe for hosts that keep registered tool handles. - Guard process-wide exception handlers so importing server.js for native tools does not alter OpenCode/Kilo host crash semantics. - Scope CONTEXT_MODE_EMBEDDED_PLUGIN_TOOLS to the dynamic import and restore it so child commands do not inherit the internal guard. Tests cover native tool registration, native ctx_stats execution, host side-effect leakage, native session attribution, legacy MCP config cleanup, doctor warning, and stale MCP no-op predicate. Refs: mksglu#574, mksglu#565, mksglu#592
2219d67 to
d35978b
Compare
|
Addressed both nits in the latest push:
Validation:
|
OpenCode's plugin tool registry (refs/platforms/opencode/packages/ opencode/src/tool/registry.ts:127) uses the Zod schema only as a boolean type guard via .safeParse(u).success — it passes RAW args to def.execute(). Our ctx_batch_execute / ctx_search handlers rely on z.preprocess(coerceCommandsArray | coerceJsonArray, …) to coerce JSON-string args back into arrays and to fill defaults. PR #574 / #597 wired ctx_* tools natively via the plugin tool map and called registered.handler(args ?? {}) directly, bypassing the MCP SDK's safeParseAsync wrapper. Result: when the LLM delivered commands as a JSON-stringified array or omitted them entirely, the handler crashed with "commands.map is not a function" instead of either coercing the value or producing an actionable validation error. Fix: run inputSchema.parse(args) inside buildNativeTools before invoking the handler — same contract as the MCP framework (server/mcp.js safeParseAsync line 174). Validation failures now surface as "Invalid arguments for <tool>: <zod error>" rather than opaque TypeErrors downstream. TDD slices (tests/opencode-plugin.test.ts): - baseline well-formed args still work - JSON-stringified commands array is coerced - bare-string commands are lifted to {label,command} - missing commands raises a clear "Invalid arguments" error - JSON-stringified queries on ctx_search are coerced
…cy MCP child (#623) When OpenCode/Kilo users upgrade from v1.0.136 to v1.0.137+ without running `context-mode upgrade`, their opencode.json keeps the legacy `mcp.context-mode` block alongside `plugin: ["context-mode"]`. PR #574/#597 intentionally suppresses ctx_* registration on that legacy MCP child to avoid duplicate tools — but did so silently, so any MCP client inspecting the child via tools/list sees an empty list with no signal. Add emitSuppressionDiagnostic() — one-shot stderr line at first suppressed registerTool() call. Explains the exact reason, links #623, and points to `context-mode upgrade` as the fix. Zero behavior change for tools; pure observability. Plugin-native tools continue to serve OpenCode/Kilo unchanged. Empirical reproduction matrix (probe MCP child with crafted opencode.json, OPENCODE=1, CONTEXT_MODE_PLATFORM=opencode): scenario | v1.0.136 | v1.0.137+ ----------------------+----------+---------- plugin-only | 11 | 11 stale-mcp-only | 11 | 11 plugin+stale-mcp | 11 | 0 <-- silent before, diagnosed now no-config | 11 | 11 Generic MCP hosts (non-OpenCode/Kilo) never trigger suppression — 14 adapters unaffected. Existing infrastructure (configureAllHooks auto- removal + doctor warning) already in place since v1.0.139; this commit closes the observability gap on the silent path.
Summary
Implements #574 for OpenCode/KiloCode: the plugin now registers all 11
ctx_*tools natively through the TypeScript plugintoolmap, so the shipped OpenCode/Kilo configs no longer need a redundant local MCP child.This keeps the stdio MCP server intact for the other adapters (Claude Code, Codex, Cursor, Gemini CLI, VS Code Copilot, JetBrains Copilot, Qwen Code, Kiro, Antigravity, OMP, Pi, Zed). Only OpenCode/Kilo change to plugin-only because they have a native tool API.
What changed
src/server.tsserverplusREGISTERED_CTX_TOOLSby wrappingserver.registerTool(...).CONTEXT_MODE_EMBEDDED_PLUGIN_TOOLS=1guard so plugin imports can load the tool registry without starting stdio transport.withProjectDirOverride()so OpenCode/Kilo tool-context directories flow into existing handlers without a globalprocess.envrace.main()do not leak/tmp/cm-fs-preload-*snippets.src/adapters/opencode/plugin.tstool: { ctx_* }map from the shared server registry.z.object({...})schemas to OpenCode/Kilo raw zod arg shapes.{content:[{type:"text",text}]}into OpenCode{title, output}tool results.configs/opencode/opencode.json/configs/kilo/kilo.jsonmcpblocks.plugin: ["context-mode"].Docs
docs/platform-support.mdmarks OpenCode as plugin-native ctx_* tools.Tests
tests/opencode-plugin.test.tsasserts all 11 ctx_* tools are registered natively andctx_statsexecutes through the plugin tool map.tests/adapters/opencode-idle-config.test.tsasserts OpenCode/Kilo configs are plugin-only.Verification
Results:
Risk notes
main()but still registers the exact same handlers.process.env.CONTEXT_MODE_PROJECT_DIRoverride would introduce under concurrent tool calls.Refs: #574, #565, #592
Update: legacy config cleanup
After maintainer feedback about idle-shutdown being death-code for MCP hosts, the PR now also cleans existing OpenCode/Kilo configs during
context-mode upgrade:configureAllHooks()removes only the legacymcp.context-modeblock while preserving any other MCP servers in the user's config. This ensures existing users don't keep spawning the old redundant MCP child after upgrading to plugin-native tools.Regression test:
configureAllHooks removes legacy context-mode MCP block for plugin-only mode (#574).Update: idle-timeout feature removed
After maintainer feedback, this PR now removes the
CONTEXT_MODE_IDLE_TIMEOUT_MSfeature entirely. OpenCode/Kilo no longer need it because plugin-native tools eliminate the redundant MCP child; other hosts were harmed by clean timer-driven MCP exits.CONTEXT_MODE_STARTUP_SWEEPremains because it only runs at MCP child boot and does not kill the active child on a timer.