fix(server): decouple native Paseo tools from MCP injection - #4434
fix(server): decouple native Paseo tools from MCP injection#4434thomasvan wants to merge 1 commit into
Conversation
After getpaseo#4277, disabling MCP injection also disables the native Paseo tool catalog, even when the per-provider paseoTools policy is enabled. Deployments that serve MCP caller-scoped (daemon-wide injection off) lose native tools for their coordinator seats entirely. Gate native availability on mcp.enabled at startup and on both live field-change handlers, and keep injection controlling only the injected MCP server URL. The per-provider paseoTools policy remains the seat-level gate. Adds native-tools-gate.ts with the decision and its truth table.
|
| Filename | Overview |
|---|---|
| packages/server/src/server/bootstrap.ts | Decouples native catalogs from MCP injection, but enables the global OpenCode manifest without applying the provider-level policy. |
| packages/server/src/server/native-tools-gate.ts | Defines the intended MCP-enabled truth table as a small pure function. |
| packages/server/src/server/native-tools-gate.test.ts | Covers the helper truth table but not the startup and live bootstrap behavior that caused the regression. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[mcp.enabled] --> B[Native tools master gate]
C[mcp.injectIntoAgents] --> D[MCP base URL injection]
B --> E[AgentManager]
E --> F{Provider paseoTools policy}
F -->|enabled| G[Session-bound native catalog]
F -->|disabled| H[No executable session tools]
B --> I[Global OpenCode manifest]
I --> J[Tools advertised to all OpenCode sessions]
H --> K[Advertised invocation returns 403]
Reviews (1): Last reviewed commit: "fix(server): decouple native Paseo tools..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| // injection: caller-scoped deployments keep the catalog while injecting | ||
| // nothing (see native-tools-gate.ts). | ||
| agentManager.setPaseoToolsEnabled(isNativePaseoToolsEnabled(config.mcpEnabled)); | ||
| setAgentProviderToolsEnabled(isNativePaseoToolsEnabled(config.mcpEnabled)); |
There was a problem hiding this comment.
When MCP is enabled and an OpenCode worker has its paseoTools policy disabled, this installs the full daemon-wide manifest without applying that provider policy. OpenCode therefore advertises every native tool to the worker, but the session has no executable tools and each invocation returns 403. Apply the provider policy when exposing the manifest, or make catalog discovery session-scoped so disabled seats receive no catalog.
| test("mcp enabled + injection off keeps the native catalog available", () => { | ||
| expect(isNativePaseoToolsEnabled(true)).toBe(true); | ||
| }); | ||
|
|
||
| test("absent mcp.enabled defaults to enabled", () => { | ||
| expect(isNativePaseoToolsEnabled(undefined)).toBe(true); | ||
| }); | ||
|
|
||
| test("mcp disabled disables the native catalog regardless of injection", () => { | ||
| expect(isNativePaseoToolsEnabled(false)).toBe(false); | ||
| }); |
There was a problem hiding this comment.
These tests exercise only the extracted boolean helper, so they still pass if startup or either live configuration handler couples native tools back to MCP injection. The repository requires tests to use the same interface as callers and prove visible behavior. Before merging, add a bootstrap or daemon-level regression test that observes native catalog availability with injection disabled, including the relevant live configuration transition.
Rule Used: # Code Review Pattern Reference: Slop, Tests, Feat... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| import { CheckoutDiffManager } from "./checkout-diff-manager.js"; | ||
| import { ScheduleService } from "./schedule/service.js"; | ||
| import { DaemonConfigStore, type MutableDaemonConfig } from "./daemon-config-store.js"; | ||
| import { isNativePaseoToolsEnabled } from "./native-tools-gate.js"; |
There was a problem hiding this comment.
This new import uses a relative path, and the corresponding import in packages/server/src/server/native-tools-gate.test.ts:3 does the same. The server package guide requires imports to use the @server/* alias. This repository requirement must be satisfied before merging.
Context Used: packages/server/CLAUDE.md (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
Closing for now: this needs a daemon-level regression and real-provider evidence that native tools remain usable with MCP injection off at startup and after live config changes, while disabled provider policies still hide them. Please reopen with that evidence. |
2 participants
After #4277 landed per-provider control of the Paseo tool catalog, disabling MCP injection also disables the native catalog, even where the provider policy is enabled:
Why it matters. Deployments that serve MCP caller-scoped — each seat gets its own caller-scoped server, so daemon-wide injection is off — still need the native catalog for coordinator seats (and, via the per-provider
paseoToolspolicy, none for worker seats). Gating the catalog oninjectIntoAgentsremoves the whole channel for exactly those deployments; the per-provider policy from #4277 is a second gate on top of a switch that is already off.Fix. Gate native availability on
mcp.enabledat startup and on bothonFieldChangehandlers; injection keeps controlling only the injected MCP server URL (setMcpBaseUrl). The per-providerpaseoToolspolicy remains the seat-level gate. Addsnative-tools-gate.tsowning the decision, with its truth table (native-tools-gate.test.ts).Default behaviour is unchanged where MCP is enabled and injection is on; the change is only for injection-off deployments, whose native tools previously disappeared as a side effect of a flag about something else. Server typecheck and the new tests pass.