diff --git a/packages/compass-agent/src/cli.test.ts b/packages/compass-agent/src/cli.test.ts index 1f3aaf54..98a6bd9c 100644 --- a/packages/compass-agent/src/cli.test.ts +++ b/packages/compass-agent/src/cli.test.ts @@ -37,6 +37,7 @@ import { AGENT_SOCKET_PATH, authSeedPath, createSeedApiKeyResolver, + deriveLitellmMcpUrl, envFilePath, type MainDeps, main, @@ -181,6 +182,56 @@ describe("resolveRole", () => { }); }); +// The LiteLLM MCP URL is DERIVED from LITELLM_BASE_URL (RIG-2674), matching the +// wave's SOPS loader (secrets-env.nix): strip a trailing slash, strip a trailing +// /v1, append /mcp/. The trailing slash is load-bearing (LiteLLM 307-redirects +// /mcp and MCP clients don't re-POST across the redirect). +describe("deriveLitellmMcpUrl", () => { + test("derives /mcp/ from a /v1 base URL", () => { + expect( + deriveLitellmMcpUrl({ LITELLM_BASE_URL: "https://llm.example/v1" }), + ).toBe("https://llm.example/mcp/"); + }); + + test("strips a trailing slash before stripping /v1", () => { + // Non-vacuity: without the trailing-slash strip the /v1$ match misses and + // the result would be `…/v1//mcp/` — a broken double path. + expect( + deriveLitellmMcpUrl({ LITELLM_BASE_URL: "https://llm.example/v1/" }), + ).toBe("https://llm.example/mcp/"); + }); + + test("appends /mcp/ to a base with no /v1 suffix", () => { + expect( + deriveLitellmMcpUrl({ LITELLM_BASE_URL: "https://llm.example" }), + ).toBe("https://llm.example/mcp/"); + }); + + test("keeps the load-bearing trailing slash", () => { + // The whole point of the derivation: /mcp (no slash) 307-redirects and the + // MCP client drops the POST body. The result MUST end in a slash. + const url = deriveLitellmMcpUrl({ + LITELLM_BASE_URL: "https://llm.example/v1", + }); + expect(url?.endsWith("/mcp/")).toBe(true); + }); + + test("returns undefined when LITELLM_BASE_URL is unset (no gateway configured)", () => { + expect(deriveLitellmMcpUrl({})).toBeUndefined(); + }); + + test("treats an empty or whitespace-only base as unset", () => { + expect(deriveLitellmMcpUrl({ LITELLM_BASE_URL: "" })).toBeUndefined(); + expect(deriveLitellmMcpUrl({ LITELLM_BASE_URL: " " })).toBeUndefined(); + }); + + test("trims surrounding whitespace so a padded base still derives", () => { + expect( + deriveLitellmMcpUrl({ LITELLM_BASE_URL: " https://llm.example/v1 " }), + ).toBe("https://llm.example/mcp/"); + }); +}); + // The seed path is the frozen T5 placement: a 0600 `$HOME/.compass/auth-seed.json` // written by the Runner's materializer. describe("authSeedPath", () => { @@ -1379,6 +1430,8 @@ describe("main sources $HOME/.compass/env into process.env", () => { "COMPASS_MODEL", "OTEL_EXPORTER_OTLP_ENDPOINT", "COMPASS_FUTURE_VAR", + "LITELLM_BASE_URL", + "LITELLM_MCP_URL", ] as const; let savedEnv: Record = {}; beforeEach(() => { @@ -1461,6 +1514,76 @@ describe("main sources $HOME/.compass/env into process.env", () => { ); expect(process.env.COMPASS_FUTURE_VAR).toBeUndefined(); }); + + test("derives LITELLM_MCP_URL from a delivered LITELLM_BASE_URL (RIG-2674)", async () => { + const home = process.env.HOME as string; + // The keyring delivers only the base URL (+ API key). Without the derive, + // LITELLM_MCP_URL stays unset and the fleet mcp.json's ${LITELLM_MCP_URL} + // expands empty → the LiteLLM MCP server can't connect. Non-vacuity: with + // the derive removed this assertion reds (undefined, not the /mcp/ URL). + delete process.env.LITELLM_MCP_URL; + writeEnvFile(home, "LITELLM_BASE_URL=https://llm.example/v1\n"); + await main( + { HOME: home }, + deps( + fakeSession(), + fakeCarrier(emptyLog(), { control: emptyControlStream }), + ), + ); + expect(process.env.LITELLM_MCP_URL as string | undefined).toBe( + "https://llm.example/mcp/", + ); + }); + + test("LITELLM_MCP_URL is derived BEFORE the MCP manager connects (the load-bearing ordering)", async () => { + // The whole point of the fix is that the MCP connector — which reads + // ${LITELLM_MCP_URL} from process.env — sees the derived value. Asserting + // it only after main() resolves would pass even if a future refactor moved + // the derive AFTER the connect. Capture process.env AT the connect instant + // via the connectMcp seam (main calls it unconditionally, cli.ts:729, after + // loadMountedConfig and before session construction). Non-vacuity: move the + // derive block below the connect and this reds (undefined at capture) while + // the post-resolve assertion above would still pass. + const home = process.env.HOME as string; + delete process.env.LITELLM_MCP_URL; + writeEnvFile(home, "LITELLM_BASE_URL=https://llm.example/v1\n"); + let atConnect: string | undefined = "UNSET-SENTINEL"; + await main( + { HOME: home }, + { + ...deps( + fakeSession(), + fakeCarrier(emptyLog(), { control: emptyControlStream }), + ), + connectMcp: (_cwd, _mcp) => { + atConnect = process.env.LITELLM_MCP_URL; + return Promise.resolve({ + tools: [] as never, + disconnect: () => Promise.resolve(), + }); + }, + }, + ); + expect(atConnect).toBe("https://llm.example/mcp/"); + }); + + test("an explicitly-delivered LITELLM_MCP_URL wins over the derivation", async () => { + const home = process.env.HOME as string; + // If a deployer ever delivers the MCP URL directly (env file), the derive + // must not clobber it — same file-defines-it posture as the merge loop. + writeEnvFile( + home, + "LITELLM_BASE_URL=https://llm.example/v1\nLITELLM_MCP_URL=https://override.example/mcp/\n", + ); + await main( + { HOME: home }, + deps( + fakeSession(), + fakeCarrier(emptyLog(), { control: emptyControlStream }), + ), + ); + expect(process.env.LITELLM_MCP_URL).toBe("https://override.example/mcp/"); + }); }); // A Control stream that closes cleanly with no ops — the shortest complete run. diff --git a/packages/compass-agent/src/cli.ts b/packages/compass-agent/src/cli.ts index 08e5a237..e3c3a2f4 100644 --- a/packages/compass-agent/src/cli.ts +++ b/packages/compass-agent/src/cli.ts @@ -180,6 +180,33 @@ export function resolveRole( return raw ? raw : undefined; } +/** + * The LiteLLM MCP endpoint, DERIVED from the delivered `LITELLM_BASE_URL` + * (mirroring the wave's SOPS loader `secrets-env.nix`). Compass's keyring + * delivers `LITELLM_BASE_URL` + `LITELLM_API_KEY` but NOT the MCP URL — it is a + * derived var, not a stored secret — so the fleet `mcp.json`'s + * `${LITELLM_MCP_URL}` would otherwise expand empty and the LiteLLM MCP server + * would fail to connect. Deriving (rather than seeding a second secret) keeps a + * single source of truth and no base/MCP drift. + * + * The rule matches the loader byte-for-byte: strip one trailing `/`, strip a + * trailing `/v1`, append `/mcp/`. The trailing slash is LOAD-BEARING — LiteLLM + * 307-redirects `/mcp` and MCP clients do not re-POST across the redirect + * (`secrets-env.nix:27-29`). So `https://host/v1` → `https://host/mcp/`. + * + * Unset (or blank) base is a legitimate configuration: no LiteLLM gateway is + * configured, so nothing to derive — returns undefined and `main` leaves + * `LITELLM_MCP_URL` untouched. Same unset/trim semantics as `resolveModelSelector`. + */ +export function deriveLitellmMcpUrl( + env: Record, +): string | undefined { + const raw = env.LITELLM_BASE_URL?.trim(); + if (!raw) return undefined; + const base = raw.replace(/\/$/, "").replace(/\/v1$/, ""); + return `${base}/mcp/`; +} + /** One provider's credential in the seed file. Mirrors the SDK's `ApiKeyCredential`. */ interface SeedEntry { readonly type?: string; @@ -522,6 +549,18 @@ export async function main( process.env[key] = value; } + // Derive LITELLM_MCP_URL from the just-sourced LITELLM_BASE_URL (RIG-2674): + // Compass's keyring delivers the base URL + API key but not the derived MCP + // URL, so the fleet mcp.json's `${LITELLM_MCP_URL}` would expand empty and the + // LiteLLM MCP server would fail to connect. Derive it here — after the env + // merge, before the MCP connect below — reading from process.env (where the + // merge landed the base). An explicitly-delivered LITELLM_MCP_URL wins (same + // file-defines-it posture as the merge); we only fill the gap. + if (!process.env.LITELLM_MCP_URL) { + const mcpUrl = deriveLitellmMcpUrl(process.env); + if (mcpUrl) process.env.LITELLM_MCP_URL = mcpUrl; + } + // The identity overlay; undefined when unset or whitespace-only. What omits // the overlay in that case is the `persona ?` spread guard below (an absent // `systemPrompt` key), not any `||`/`??` subtlety — `resolvePersona` has