Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 123 additions & 0 deletions packages/compass-agent/src/cli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {
AGENT_SOCKET_PATH,
authSeedPath,
createSeedApiKeyResolver,
deriveLitellmMcpUrl,
envFilePath,
type MainDeps,
main,
Expand Down Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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<string, string | undefined> = {};
beforeEach(() => {
Expand Down Expand Up @@ -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.
Expand Down
39 changes: 39 additions & 0 deletions packages/compass-agent/src/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, string | undefined>,
): 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;
Expand Down Expand Up @@ -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
Expand Down
Loading