fix(mcp): translate oauth into the shape Zoo Code validates for .roo/mcp.json

docs/reference/file-formats.md

@@ -1598,7 +1598,7 @@ You can control which individual tools from an MCP server are enabled or disable
 
 > **Kiro note:** Kiro MCP servers are written under `mcpServers` in `.kiro/settings/mcp.json` (project) and `~/.kiro/settings/mcp.json` (global). Kiro supports `disabledTools` natively and Rulesync preserves it on generate and import. Kiro does not expose a corresponding per-server `enabledTools` allowlist, so that field is omitted for Kiro targets. The two Rulesync-only authoring keys are translated onto the field names Kiro actually reads: `kiroAutoApprove` becomes `autoApprove` (tools run without a confirmation prompt) and `kiroAutoBlock` becomes `disabledTools` (tools hidden from the agent). A server that already spells `autoApprove` or `disabledTools` natively keeps working — the two lists are merged rather than one overwriting the other. On import, `autoApprove` is lifted back into `kiroAutoApprove`; `disabledTools` stays as-is because it is already a canonical Rulesync field with the same meaning, so `kiroAutoBlock` has no import counterpart. That makes `kiroAutoBlock` a redundant spelling of `disabledTools`: a Kiro config imported after a generate comes back as canonical `disabledTools`, which then also reaches the other targets that support it. Prefer authoring `disabledTools` directly, which makes that scope explicit from the start.
 
-> **Roo Code / Zoo Code note:** Both targets write `.roo/mcp.json` through the same adapter, and their per-server MCP schema is **denylist-only**: `disabledTools` clears each named tool's `enabledForPrompt` (the tool stops being offered to the model), and there is no corresponding `enabledTools` allowlist, so that field is omitted for these targets. The server config is emitted verbatim, so `disabledTools` round-trips as itself. Earlier Rulesync versions stripped it before the adapter saw it, which meant a filter you had written into `.roo/mcp.json` by hand — or through Zoo Code's own tool toggles, which write the same key — was deleted on the next generate.
+> **Roo Code / Zoo Code note:** Both targets write `.roo/mcp.json` through the same adapter, and their per-server MCP schema is **denylist-only**: `disabledTools` clears each named tool's `enabledForPrompt` (the tool stops being offered to the model), and there is no corresponding `enabledTools` allowlist, so that field is omitted for these targets. The server config is emitted verbatim, so `disabledTools` round-trips as itself. Earlier Rulesync versions stripped it before the adapter saw it, which meant a filter you had written into `.roo/mcp.json` by hand — or through Zoo Code's own tool toggles, which write the same key — was deleted on the next generate. Canonical `oauth` is translated into the shape Zoo Code validates ([Zoo-Code-Org/Zoo-Code#2129](https://github.com/Zoo-Code-Org/Zoo-Code/pull/2129)), because one entry Zoo Code rejects makes it load none of the servers in the project `.roo/mcp.json`: it is kept only on a `"type": "http"` server (written as `streamable-http`), and dropped with a warning from stdio, sse and untyped `url` servers (Zoo Code reads `type`, not `transport`, and requires an explicit `type` for url servers); a block without a non-empty `clientId` is dropped with a warning; `clientId`, `scope`, `redirectUri` (a URL) and `resource` (`"auto"`, `false` or a URL) pass through, and a value that does not fit is dropped with a warning; a canonical `scopes` list is joined into `scope` when no `scope` is given; Claude Code's `callbackPort` becomes `redirectUri: "http://127.0.0.1:<port>/callback"` (an explicit `redirectUri` wins), and any other key such as `authServerMetadataUrl` is dropped with a warning. On import, a `redirectUri` of exactly the form `http://127.0.0.1:<port>/callback` is read back as `callbackPort`; any other, including a `localhost` one, is kept as written so the redirect URI registered with the OAuth client never changes. The same rules apply to the `roo` target, since Roo Code never read `oauth`.
 
 > **deepagents note:** MCP servers are written to `.deepagents/.mcp.json` (project) / `~/.deepagents/.mcp.json` (global). Several translations apply, because dcode **drops an individual server it cannot validate** — the rest of the file still loads, so a mistake here is silent rather than loud. **Transports:** dcode accepts only `stdio`, `sse` and `http` (plus the aliases `streamable_http` / `streamable-http` → `http`), so canonical `local` is written as `stdio` and `streamable-http` as `http`; canonical `ws` has no counterpart and the server is skipped with a warning at generate time, where you can still see it. Whether the value lands under `type` or `transport` follows whichever key you authored — dcode reads the two interchangeably, and with neither set it infers `http` from a `url` and `stdio` otherwise. On import, both spellings of the `streamable_http` alias come back as canonical `http`. **Tool filters:** canonical `enabledTools` becomes `allowedTools` (dcode never reads `enabledTools`, and it ignores unknown keys silently, so forwarding the canonical name would be a no-op), while `disabledTools` carries its own name; each entry is a tool name or an `fnmatch` glob, and import lifts `allowedTools` back. Upstream rejects a server that sets **both** filters and rejects an **empty** list, and each case is resolved the way that does not hand the model more tools than your canonical config allows. Setting both is valid canonically — other targets apply the two lists independently — but has no form here, so the **server is skipped** with a warning rather than written without filters, which would leave it running with every tool including the denied ones. An empty `enabledTools` likewise **skips the server**, since an allowlist of nothing means no tools at all and dropping the key would publish all of them. An empty `disabledTools` is the one genuine no-op, so only that key is dropped (with a warning) and the server is still written. **Server shapes:** dcode validates each entry into FastMCP 4's typed server models, so three canonical shapes are normalized as well. An `httpUrl` is written as `url` (dcode never reads the `httpUrl` alias and would take such a server for stdio) with the transport pinned to `http` when none is set, since `httpUrl` names a streamable HTTP endpoint (an authored `type` or `transport` is kept); when both are set, `url` wins and `httpUrl` is dropped. An array `command` is split into its first element and an `args` list that the rest is prepended to, because FastMCP accepts only a string `command`; an empty array leaves nothing to spawn, so that server is skipped with a warning (on a remote server the empty `command` is simply dropped). A server with `disabled: true` is **skipped** with a warning, because dcode has no per-server `disabled` key — its only off switch is the server viewer's `[mcp].disabled_servers` toggle in `config.toml` — and writing the key would leave the server running; `disabled: false` is simply dropped. **OAuth:** since deepagents-code 0.1.87, dcode reads a per-server `oauth` block only as a custom Slack OAuth app — `{ "client_id": …, "client_secret": … }` (secret optional, both non-empty strings) on a `slack.com` / `*.slack.com` server with `auth: "oauth"` — and skips any other server that sets `oauth`. On such a Slack server the canonical `clientId` is written as `client_id` (a native `client_id` wins), `client_secret` passes through, and every other key (`callbackPort`, `authServerMetadataUrl`, …) is dropped with a warning; on every other server, or when no usable client id remains, `oauth` is left out with a warning so the server still loads. On import, `client_id` comes back as `clientId`. See the [MCP tools docs](https://docs.langchain.com/oss/deepagents/code/mcp-tools).
 

src/features/mcp/roo-mcp.test.ts

@@ -8,6 +8,7 @@ import {
 } from "../../constants/rulesync-paths.js";
 import { setupTestDirectory } from "../../test-utils/test-directories.js";
 import { ensureDir, writeFileContent } from "../../utils/file.js";
+import { fallbackLogger } from "../../utils/logger.js";
 import { RooMcp } from "./roo-mcp.js";
 import { RulesyncMcp } from "./rulesync-mcp.js";
 
@@ -401,6 +402,234 @@ describe("RooMcp", () => {
     });
   });
 
+  describe("fromRulesyncMcp oauth", () => {
+    const generate = (mcpServers: Record<string, unknown>) => {
+      const rulesyncMcp = new RulesyncMcp({
+        relativeDirPath: RULESYNC_RELATIVE_DIR_PATH,
+        relativeFilePath: "mcp.json",
+        fileContent: JSON.stringify({ mcpServers }),
+      });
+      return JSON.parse(RooMcp.fromRulesyncMcp({ rulesyncMcp }).getFileContent()).mcpServers;
+    };
+
+    it("should map callbackPort to a loopback redirectUri on an http server", () => {
+      const warnSpy = vi.spyOn(fallbackLogger, "warn").mockImplementation(() => {});
+      const servers = generate({
+        remote: {
+          type: "http",
+          url: "https://example.com/mcp",
+          oauth: { clientId: "abc", scope: "read", callbackPort: 8080 },
+        },
+      });
+
+      expect(servers.remote.oauth).toEqual({
+        clientId: "abc",
+        scope: "read",
+        redirectUri: "http://127.0.0.1:8080/callback",
+      });
+      expect(warnSpy).not.toHaveBeenCalled();
+    });
+
+    it("should keep an explicit redirectUri and resource", () => {
+      const servers = generate({
+        remote: {
+          type: "http",
+          url: "https://example.com/mcp",
+          oauth: {
+            clientId: "abc",
+            callbackPort: 8080,
+            redirectUri: "http://localhost:9000/callback",
+            resource: false,
+          },
+        },
+      });
+
+      expect(servers.remote.oauth).toEqual({
+        clientId: "abc",
+        redirectUri: "http://localhost:9000/callback",
+        resource: false,
+      });
+    });
+
+    it("should drop oauth keys Zoo Code does not read, with a warning", () => {
+      const warnSpy = vi.spyOn(fallbackLogger, "warn").mockImplementation(() => {});
+      const servers = generate({
+        remote: {
+          type: "http",
+          url: "https://example.com/mcp",
+          oauth: { clientId: "abc", authServerMetadataUrl: "https://auth.example.com/meta" },
+        },
+      });
+
+      expect(servers.remote.oauth).toEqual({ clientId: "abc" });
+      expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("oauth.authServerMetadataUrl"));
+    });
+
+    it("should drop a redirectUri or resource that is not a URL, with a warning", () => {
+      const warnSpy = vi.spyOn(fallbackLogger, "warn").mockImplementation(() => {});
+      const servers = generate({
+        remote: {
+          type: "http",
+          url: "https://example.com/mcp",
+          oauth: { clientId: "abc", redirectUri: "not a url", resource: "my-api" },
+        },
+      });
+
+      expect(servers.remote.oauth).toEqual({ clientId: "abc" });
+      expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("oauth.redirectUri"));
+      expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("oauth.resource"));
+    });
+
+    it('should keep resource "auto" and a URL resource', () => {
+      const servers = generate({
+        auto: {
+          type: "http",
+          url: "https://a.example.com/mcp",
+          oauth: { clientId: "a", resource: "auto" },
+        },
+        url: {
+          type: "http",
+          url: "https://b.example.com/mcp",
+          oauth: { clientId: "b", resource: "https://b.example.com" },
+        },
+      });
+
+      expect(servers.auto.oauth).toEqual({ clientId: "a", resource: "auto" });
+      expect(servers.url.oauth).toEqual({ clientId: "b", resource: "https://b.example.com" });
+    });
+
+    it("should join a canonical scopes list into scope unless scope is set", () => {
+      const warnSpy = vi.spyOn(fallbackLogger, "warn").mockImplementation(() => {});
+      const servers = generate({
+        joined: {
+          type: "http",
+          url: "https://a.example.com/mcp",
+          oauth: { clientId: "a", scopes: ["read", "write"] },
+        },
+        explicit: {
+          type: "http",
+          url: "https://b.example.com/mcp",
+          oauth: { clientId: "b", scope: "admin", scopes: ["read"] },
+        },
+      });
+
+      expect(servers.joined.oauth).toEqual({ clientId: "a", scope: "read write" });
+      expect(servers.explicit.oauth).toEqual({ clientId: "b", scope: "admin" });
+      expect(warnSpy).not.toHaveBeenCalled();
+    });
+
+    it("should drop the oauth block when clientId is missing", () => {
+      const warnSpy = vi.spyOn(fallbackLogger, "warn").mockImplementation(() => {});
+      const servers = generate({
+        remote: {
+          type: "http",
+          url: "https://example.com/mcp",
+          oauth: { callbackPort: 8080 },
+        },
+      });
+
+      expect(servers.remote).not.toHaveProperty("oauth");
+      expect(servers.remote.type).toBe("streamable-http");
+      expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("oauth.clientId"));
+    });
+
+    it("should drop oauth from stdio, sse and untyped url servers", () => {
+      const warnSpy = vi.spyOn(fallbackLogger, "warn").mockImplementation(() => {});
+      const servers = generate({
+        local: { command: "node", args: ["server.js"], oauth: { clientId: "abc" } },
+        events: { type: "sse", url: "https://example.com/sse", oauth: { clientId: "abc" } },
+        transportOnly: {
+          transport: "http",
+          url: "https://example.com/mcp",
+          oauth: { clientId: "abc" },
+        },
+      });
+
+      expect(servers.local).not.toHaveProperty("oauth");
+      expect(servers.events).not.toHaveProperty("oauth");
+      expect(servers.transportOnly).not.toHaveProperty("oauth");
+      expect(warnSpy).toHaveBeenCalledTimes(3);
+    });
+  });
+
+  describe("toRulesyncMcp oauth", () => {
+    const importServers = (mcpServers: Record<string, unknown>) => {
+      const rooMcp = new RooMcp({
+        relativeDirPath: ".roo",
+        relativeFilePath: "mcp.json",
+        fileContent: JSON.stringify({ mcpServers }),
+      });
+      return JSON.parse(rooMcp.toRulesyncMcp().getFileContent()).mcpServers;
+    };
+
+    it("should map a loopback /callback redirectUri back to callbackPort", () => {
+      const servers = importServers({
+        remote: {
+          type: "streamable-http",
+          url: "https://example.com/mcp",
+          oauth: { clientId: "abc", redirectUri: "http://127.0.0.1:8080/callback" },
+        },
+      });
+
+      expect(servers.remote.oauth).toEqual({ clientId: "abc", callbackPort: 8080 });
+    });
+
+    it("should keep any other redirectUri as written", () => {
+      const servers = importServers({
+        remote: {
+          type: "streamable-http",
+          url: "https://example.com/mcp",
+          oauth: { clientId: "abc", redirectUri: "http://127.0.0.1:8080/auth", resource: "auto" },
+        },
+      });
+
+      expect(servers.remote.oauth).toEqual({
+        clientId: "abc",
+        redirectUri: "http://127.0.0.1:8080/auth",
+        resource: "auto",
+      });
+    });
+
+    it("should keep a localhost redirectUri as written", () => {
+      const servers = importServers({
+        remote: {
+          type: "streamable-http",
+          url: "https://example.com/mcp",
+          oauth: { clientId: "abc", redirectUri: "http://localhost:8080/callback" },
+        },
+      });
+
+      expect(servers.remote.oauth).toEqual({
+        clientId: "abc",
+        redirectUri: "http://localhost:8080/callback",
+      });
+    });
+
+    it("should round-trip callbackPort through generate and import", () => {
+      const rulesyncMcp = new RulesyncMcp({
+        relativeDirPath: RULESYNC_RELATIVE_DIR_PATH,
+        relativeFilePath: "mcp.json",
+        fileContent: JSON.stringify({
+          mcpServers: {
+            remote: {
+              type: "http",
+              url: "https://example.com/mcp",
+              oauth: { clientId: "abc", callbackPort: 8080 },
+            },
+          },
+        }),
+      });
+      const rooMcp = RooMcp.fromRulesyncMcp({ rulesyncMcp });
+      const reimported = JSON.parse(rooMcp.toRulesyncMcp().getFileContent());
+
+      expect(reimported.mcpServers.remote).toEqual({
+        type: "http",
+        url: "https://example.com/mcp",
+        oauth: { clientId: "abc", callbackPort: 8080 },
+      });
+    });
+  });
+
   describe("toRulesyncMcp", () => {
     it("should convert to RulesyncMcp", () => {
       const mcpContent = {