fix: disable stale bridge tool IDs from other pool slots#56
Merged
Conversation
A bridge slot reused by a later request stays connected under its previous tool schema, since OpenCode's server API has no endpoint to deregister an MCP server once added. getDisabledTools() also only snapshots OpenCode's built-in tool IDs once, before any bridge tools ever exist, so it can never know to disable them either. Without explicitly disabling every previously-registered bridge tool ID per turn, a stale, still-connected tool from an earlier turn's slot remains implicitly enabled and can be called by the model instead of (or alongside) the current turn's own tool - confirmed against a real live OpenCode server: a second tool-calling request reusing a different pool slot got back the *first* request's tool call (get_weather) instead of its own (calculate). - getToolBridgeState(): track every bridge tool ID ever registered across the pool's lifetime in a new knownToolIDs set. - registerToolBridge(): record each turn's tool IDs into it. Exported for direct unit testing. - buildToolsMap() (new, exported): explicitly disables every known bridge tool ID, then re-enables only the current turn's own IDs. runAgentTurn() now uses this instead of only ever setting (never clearing) tool IDs on the shared map. - index.test.js: unit tests covering slot isolation (a stale tool ID from an earlier turn must be explicitly disabled, not just absent) and the no-bridge passthrough case. Testing: - npm test - 140 passed (138 existing + 2 new) - npm run lint - clean - Manually verified against a live OpenCode server (not just mocks): two sequential tool-calling requests with different tools (get_weather, then calculate) each correctly returned their own tool_calls, with GET /mcp confirming both bridge slots ended up connected independently.
There was a problem hiding this comment.
Pull request overview
This PR fixes a tool-calling bug where OpenCode can surface (and allow calling) stale MCP bridge tool IDs from previously-used bridge pool slots by explicitly disabling all previously-known bridge tool IDs each turn and only re-enabling the current turn’s bridge tool IDs.
Changes:
- Track every bridge tool ID ever registered (
knownToolIDs) and use it to explicitly disable stale bridge tools per turn. - Introduce
buildToolsMap(baseTools, bridge)and updaterunAgentTurn()to use it instead of only ever addingtrueentries. - Add regression tests covering cross-slot stale tool disablement and the no-bridge case.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| index.js | Tracks known bridge tool IDs and builds a per-turn tools map that disables stale bridge tools across pool slots. |
| index.test.js | Adds regression coverage for bridge-slot isolation and buildToolsMap() behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+516
to
+520
| // so a slot reused for a later request stays connected under its old tool | ||
| // schema until it's next reused - see buildToolsMap() below for why this | ||
| // must be tracked and explicitly disabled per-turn, not just left out of the | ||
| // map. | ||
| knownToolIDs: new Set(), |
Comment on lines
+2255
to
+2257
| it("disables a previously-registered bridge tool ID from a different pool slot", async () => { | ||
| const { client } = createMcpMockClient() | ||
|
|
Comment on lines
+2291
to
+2292
| it("returns a plain copy of baseTools when there is no bridge for this turn", () => { | ||
| const baseTools = { bash: false, read: false } |
Comment on lines
691
to
+695
| }) | ||
|
|
||
| const toolIDs = bridgeTools.map((tool) => `${slotName}_${tool.name}`) | ||
| const bridgeState = getToolBridgeState() | ||
| for (const id of toolIDs) bridgeState.knownToolIDs.add(id) |
This was referenced Jul 5, 2026
KochC
added a commit
that referenced
this pull request
Jul 5, 2026
fix: address Copilot review follow-ups from #56; docs: add n8n integration guide
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #55. Fixes a bug where tool/function calling (#52/v1.7.0) can return the wrong tool call — a stale one from a previously-reused bridge pool slot — instead of the current turn's own tool.
The bug
OpenCode's server API has no endpoint to deregister an MCP server once added, so
registerToolBridge()'s pool ofpx_tools_Nslots keeps each slot connected under its previous tool schema until that slot is next reused.getDisabledTools()also only snapshots OpenCode's built-in tool IDs once, viaclient.tool.ids(), before any bridge tools ever exist — so it can never know aboutpx_tools_N_*IDs either, past or present.The result:
runAgentTurn()only ever set the current turn's own bridge tool IDs totrueon the sharedtoolsmap. A previous turn's still-connected tool was never explicitly set tofalse— it was simply absent from the map, which left it implicitly enabled rather than disabled. Once the pool has cycled through more than one slot, a later turn's session can see (and the model can call) an unrelated, stale tool left over from an earlier turn's slot.Confirmed against a live OpenCode server (not just mocks, per the caveat in #52's original PR description): two sequential tool-calling requests with different declared tools (
get_weather, thencalculate) — the second request's response contained the first request's tool call name instead of its own.The fix
getToolBridgeState(): added aknownToolIDsset tracking every bridge tool ID ever registered across the pool's lifetime, from any slot.registerToolBridge(): records each turn's tool IDs into it. Exported for direct unit testing.buildToolsMap(baseTools, bridge): explicitly disables every known bridge tool ID first, then re-enables only the current turn's own IDs.runAgentTurn()now uses this instead of a map that only ever gainedtrueentries and never explicitly cleared stale ones.Testing
npm test— 140 passed (138 existing + 2 new)npm run lint— cleantools: [get_weather]→ correctly got backfinish_reason: "tool_calls",function.name: "get_weather"tools: [calculate]→ correctly got backfunction.name: "calculate"(not the staleget_weather)GET /mcpconfirmed bothpx_tools_0andpx_tools_1ended up connected independently, as expectedRelated