fix(mcp): break tools.ts ↔ toolSearch.ts cycle (check:cycles red on release) (#5282)

Break the tools.ts ↔ toolSearch.ts import cycle (check:cycles red on release) via a leaf toolDefinition.ts. Integrated into release/v3.8.40.
This commit is contained in:
Diego Rodrigues de Sa e Souza
2026-06-29 00:13:44 -03:00
committed by GitHub
parent 57f81f60d5
commit 9b0adabb17
5 changed files with 61 additions and 22 deletions

View File

@@ -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/<model>:generateContent` top-level shape still works (#4294 — thanks @shabeer)

View File

@@ -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<TInput extends z.ZodTypeAny, TOutput extends z.ZodTypeAny> {
/** 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[];
}

View File

@@ -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({

View File

@@ -17,27 +17,11 @@ import {
} from "../../../src/shared/constants/routingStrategies.ts";
// ============ Shared Types ============
export type AuditLevel = "none" | "basic" | "full";
export interface McpToolDefinition<TInput extends z.ZodTypeAny, TOutput extends z.ZodTypeAny> {
/** 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) ============

View File

@@ -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));
});
});