From 9b0adabb173f0fa36ae7e51bddb8ebf74a9baf80 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Mon, 29 Jun 2026 00:13:44 -0300 Subject: [PATCH] =?UTF-8?q?fix(mcp):=20break=20tools.ts=20=E2=86=94=20tool?= =?UTF-8?q?Search.ts=20cycle=20(check:cycles=20red=20on=20release)=20(#528?= =?UTF-8?q?2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Break the tools.ts ↔ toolSearch.ts import cycle (check:cycles red on release) via a leaf toolDefinition.ts. Integrated into release/v3.8.40. --- CHANGELOG.md | 1 + open-sse/mcp-server/schemas/toolDefinition.ts | 31 +++++++++++++++++++ open-sse/mcp-server/schemas/toolSearch.ts | 2 +- open-sse/mcp-server/schemas/tools.ts | 26 +++------------- .../unit/mcp/tool-definition-reexport.test.ts | 23 ++++++++++++++ 5 files changed, 61 insertions(+), 22 deletions(-) create mode 100644 open-sse/mcp-server/schemas/toolDefinition.ts create mode 100644 tests/unit/mcp/tool-definition-reexport.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index d683801c9e..7cfd7c8a87 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ _In development β€” bullets added per PR; finalized at release._ ### πŸ”§ Bug Fixes +- **mcp:** break the `schemas/tools.ts ↔ schemas/toolSearch.ts` import cycle introduced when the `tool_search` defs (#5269) were extracted into their own module β€” `toolSearch.ts` imported `McpToolDefinition` from `tools.ts` while `tools.ts` imported `toolSearchTool` from `toolSearch.ts`, failing `check:cycles` on `release/v3.8.40`. The shared `AuditLevel` + `McpToolDefinition` types now live in a leaf `schemas/toolDefinition.ts` that both import; `tools.ts` re-exports them for backward compatibility. - **compression (analytics):** record attempted-but-no-op compression runs so Stacked is no longer invisible when it saves nothing. Previously a `compression_analytics` row was written only on a net-positive saving, so a Stacked (RTKβ†’Caveman) pipeline that ran on already-compact context produced no row β€” indistinguishable from "never dispatched" (`byMode.stacked.count` stayed flat while Ultra climbed). Such runs are now recorded with `skip_reason` and surfaced as a per-mode `skipped` count plus `totalSkipped`/`bySkipReason` in the analytics summary and the Mode Breakdown; the existing net-saving totals/averages are unchanged (skip rows are excluded from them) (#4268 β€” thanks @abdulkadirozyurt, @androw) - **cli (tray):** fix `omniroute server --tray` showing no tray on macOS/Linux with no error printed. The wired Unix tray path loaded `systray2` through an inline loader that called `require("module")` inside an ESM `.mjs` file (`"type":"module"`) β†’ `ReferenceError: require is not defined`, silently swallowed (regressed in v3.8.34); even if it had loaded, `systray2` isn't in `node_modules` (it's lazily installed into `~/.omniroute/runtime`). The loader now delegates to the runtime loader, the icon path (`icon.png`) is corrected, `isTemplateIcon` is `false` (the full-color icon rendered as a white square under macOS template mode), and tray start failures are surfaced to stderr instead of being swallowed (#4605 β€” thanks @ProgMEM-CC) - **agent-bridge (antigravity):** unwrap the cloudcode-pa `.request` envelope when converting Antigravity IDE requests. The real IDE sends `cloudcode-pa.googleapis.com/v1internal:generateContent` with the Gemini request nested under `.request` (`{ project, model, request: { contents, systemInstruction, generationConfig } }`), but the bridge read those fields at the top level β€” yielding an empty conversation, so prompts hung mid-execution. The legacy `/v1beta/models/:generateContent` top-level shape still works (#4294 β€” thanks @shabeer) diff --git a/open-sse/mcp-server/schemas/toolDefinition.ts b/open-sse/mcp-server/schemas/toolDefinition.ts new file mode 100644 index 0000000000..06e9c42fd8 --- /dev/null +++ b/open-sse/mcp-server/schemas/toolDefinition.ts @@ -0,0 +1,31 @@ +/** + * Leaf module for the shared MCP tool-definition types. + * + * Extracted out of `tools.ts` to break a dependency cycle: `tools.ts` imports the + * `toolSearchTool` value from `toolSearch.ts`, and `toolSearch.ts` needs the + * `McpToolDefinition` type. Keeping the type here (a leaf that only depends on zod) + * lets both files import it without forming a cycle. + */ + +import type { z } from "zod"; + +export type AuditLevel = "none" | "basic" | "full"; + +export interface McpToolDefinition { + /** Tool name (MCP identifier) */ + name: string; + /** Human-readable description for AI agents */ + description: string; + /** Zod schema for input validation */ + inputSchema: TInput; + /** Zod schema for output validation */ + outputSchema: TOutput; + /** Required API key scopes */ + scopes: readonly string[]; + /** Audit logging level */ + auditLevel: AuditLevel; + /** Phase: 1 = essential, 2 = advanced */ + phase: 1 | 2; + /** Source endpoints on OmniRoute that this tool wraps */ + sourceEndpoints: readonly string[]; +} diff --git a/open-sse/mcp-server/schemas/toolSearch.ts b/open-sse/mcp-server/schemas/toolSearch.ts index f13749a10f..34777668d9 100644 --- a/open-sse/mcp-server/schemas/toolSearch.ts +++ b/open-sse/mcp-server/schemas/toolSearch.ts @@ -1,5 +1,5 @@ import { z } from "zod"; -import type { McpToolDefinition } from "./tools.ts"; +import type { McpToolDefinition } from "./toolDefinition.ts"; export const toolSearchInput = z .object({ diff --git a/open-sse/mcp-server/schemas/tools.ts b/open-sse/mcp-server/schemas/tools.ts index f6a0db1abb..b7b0b84090 100644 --- a/open-sse/mcp-server/schemas/tools.ts +++ b/open-sse/mcp-server/schemas/tools.ts @@ -17,27 +17,11 @@ import { } from "../../../src/shared/constants/routingStrategies.ts"; // ============ Shared Types ============ - -export type AuditLevel = "none" | "basic" | "full"; - -export interface McpToolDefinition { - /** Tool name (MCP identifier) */ - name: string; - /** Human-readable description for AI agents */ - description: string; - /** Zod schema for input validation */ - inputSchema: TInput; - /** Zod schema for output validation */ - outputSchema: TOutput; - /** Required API key scopes */ - scopes: readonly string[]; - /** Audit logging level */ - auditLevel: AuditLevel; - /** Phase: 1 = essential, 2 = advanced */ - phase: 1 | 2; - /** Source endpoints on OmniRoute that this tool wraps */ - sourceEndpoints: readonly string[]; -} +// AuditLevel + McpToolDefinition live in the leaf ./toolDefinition.ts so that +// toolSearch.ts can import the type without forming a tools.ts ↔ toolSearch.ts cycle. +// Re-exported here for backward compatibility (many modules import them from ./tools.ts). +export type { AuditLevel, McpToolDefinition } from "./toolDefinition.ts"; +import type { McpToolDefinition } from "./toolDefinition.ts"; // ============ Phase 1: Essential Tools (8) ============ diff --git a/tests/unit/mcp/tool-definition-reexport.test.ts b/tests/unit/mcp/tool-definition-reexport.test.ts new file mode 100644 index 0000000000..2289f8f5ab --- /dev/null +++ b/tests/unit/mcp/tool-definition-reexport.test.ts @@ -0,0 +1,23 @@ +import { describe, it } from "node:test"; +import assert from "node:assert/strict"; + +// Regression guard for the tools.ts ↔ toolSearch.ts cycle fix: McpToolDefinition/AuditLevel +// live in the leaf ./toolDefinition.ts and must stay re-exported from ./tools.ts for the +// many modules that import them from there. A type-only check plus a runtime import of the +// leaf proves both the re-export path and that the leaf loads without pulling in tools.ts. +describe("MCP tool-definition leaf + re-export", () => { + it("toolDefinition.ts (leaf) imports without forming a cycle", async () => { + const leaf = await import( + "../../../open-sse/mcp-server/schemas/toolDefinition.ts" + ); + // It is a type-only module; the runtime namespace is empty but must load cleanly. + assert.ok(leaf, "leaf module loaded"); + }); + + it("tools.ts still exposes McpToolDefinition-shaped tool defs (re-export intact)", async () => { + const tools = await import("../../../open-sse/mcp-server/schemas/tools.ts"); + const def = (tools as { getHealthTool?: { name: string; scopes: readonly string[] } }) + .getHealthTool; + assert.ok(def && typeof def.name === "string" && Array.isArray(def.scopes)); + }); +});