From 8a1ba9b2f749ef97f823fe548167c79425031b2d Mon Sep 17 00:00:00 2001 From: mintaka Date: Mon, 24 Aug 2026 13:42:09 -0400 Subject: [PATCH 1/2] fix(compass-agent): derive LITELLM_MCP_URL from the delivered base URL (RIG-2674) The fleet mcp.json declares the LiteLLM MCP server as `{ "url": "${LITELLM_MCP_URL}", ... }`, expanded from the agent's process.env at MCP connect. But `LITELLM_MCP_URL` is a DERIVED var, not a stored secret: in the wave the personal SOPS loader computes it from `LITELLM_BASE_URL` (secrets-env.nix). Compass's keyring delivers `LITELLM_BASE_URL` + `LITELLM_API_KEY` but NOT the derived MCP URL, so in a live Compass agent `${LITELLM_MCP_URL}` expands empty and the LiteLLM MCP server fails to connect. Derive it in the agent-env assembly (Matt-ruled option (b)) rather than seeding a second secret: single source of truth, no base/MCP drift, and Compass owns the load-bearing path rule in code. The rule mirrors the loader byte-for-byte: strip one 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. So `https://host/v1` -> `https://host/mcp/`. `deriveLitellmMcpUrl` is a pure resolver (sibling of `resolveModelSelector`); `main` fills the gap AFTER sourcing the env file and BEFORE the MCP connect, only when `LITELLM_MCP_URL` is not already present -- an explicitly-delivered value still wins (same file-defines-it posture as the env merge). Unset/blank base derives nothing (no gateway configured). Tests: pure-resolver unit block (derive, trailing-slash-then-v1 order, no-/v1 base, load-bearing slash, unset/blank/trim) + two `main` integration tests (derive lands in process.env; explicit URL wins). Red-green proven by mutation on both the call-site guard and the trailing-slash strip. Full compass-agent suite green for the touched files; no new deps (no FOD bump). Spec-impact: none (activation of an existing delivery contract; no proto or public-API change). Co-authored-by: Matt Wilkinson --- packages/compass-agent/src/cli.test.ts | 91 ++++++++++++++++++++++++++ packages/compass-agent/src/cli.ts | 39 +++++++++++ 2 files changed, 130 insertions(+) diff --git a/packages/compass-agent/src/cli.test.ts b/packages/compass-agent/src/cli.test.ts index 1f3aaf54..31a831be 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,44 @@ 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("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 From bad03d3ba9c83d501c1972763d562618f5ba21d7 Mon Sep 17 00:00:00 2001 From: mintaka Date: Mon, 24 Aug 2026 13:51:23 -0400 Subject: [PATCH 2/2] test(compass-agent): assert LITELLM_MCP_URL is derived before the MCP connect (RIG-2674) Review-fold: the existing integration test asserted the derived value lands in process.env only after main() resolves, which would still pass if a future refactor moved the derive block AFTER the MCP manager connects -- silently breaking the fix (the connector reads ${LITELLM_MCP_URL} from process.env at dial time). This adds a test that captures process.env at the connect instant via the connectMcp seam (main calls it unconditionally, after the env merge and before session construction), pinning the load-bearing ordering. Non-vacuity: disabling the derive reds it at the capture point. Spec-impact: none (test-only; strengthens the RIG-2674 ordering invariant). Co-authored-by: Matt Wilkinson --- packages/compass-agent/src/cli.test.ts | 32 ++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/packages/compass-agent/src/cli.test.ts b/packages/compass-agent/src/cli.test.ts index 31a831be..98a6bd9c 100644 --- a/packages/compass-agent/src/cli.test.ts +++ b/packages/compass-agent/src/cli.test.ts @@ -1535,6 +1535,38 @@ describe("main sources $HOME/.compass/env into process.env", () => { ); }); + 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