mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-08-24 08:02:14 +03:00
fix(tests): drain compression/kiro/memory base-red mini-cluster from 2026-08-24 merges
Three stale unit tests went red across PRs #11301/#11303/#11304 because their mocks/fixtures predated intentional contract changes merged the same day: - compression-cli-rest-fallback-6571.test.ts: #10960 moved the CLI's MCP transport from the never-mounted /api/mcp/tools/call to the real Streamable HTTP endpoint /api/mcp/stream. The REST-fallback trigger in this test's fetch mock still targeted the retired path, so mcpCallTool() threw on an unmocked fetch instead of exercising the fallback. Updated the mock to intercept /api/mcp/stream. - memory-system-first-6135.test.ts: #11290/#11303 added a Claude-family reroute to the leading-system-message placement (Opus 5 rejects a system message spliced right after a plain-text assistant turn). The regression test used "anthropic" as its NON-Claude-specific example, which is now classified as Claude-family and no longer exercises the plain cache-safe splice path it targets. Switched the fixture provider to "openai". - kiro-auto-import-name-dedup-3615.test.ts: #10815/#11287 hardened findKiroConnectionByIdentity to require an account-level identifier (email/clientId) alongside a matching profileArn before trusting the match, since distinct Builder ID accounts can share a CodeWhisperer profile ARN. Extended findKiroConnectionByProfileArn (test-only helper) with an optional account-identity param, and added a companion test documenting the new ARN-alone-is-untrusted safety behavior. All three are genuine stale-test drift, not production bugs: the code changes were deliberate fixes from other reviewed PRs; only the tests' fixtures needed to catch up.
This commit is contained in:
@@ -439,13 +439,22 @@ type ProviderConnectionLike = {
|
||||
* whose stored `providerSpecificData.profileArn` matches the given ARN.
|
||||
* Returns null when profileArn is undefined/null or no match is found.
|
||||
*
|
||||
* #10815 hardened `findKiroConnectionByIdentity` to require an account-level
|
||||
* identifier (email or clientId) alongside a matching profileArn before
|
||||
* trusting the match — distinct Builder ID accounts (Google/GitHub social
|
||||
* login) can share the same CodeWhisperer profile ARN, and matching on ARN
|
||||
* alone let a second social login silently overwrite the first connection.
|
||||
* `email`/`clientId` here let a caller supply that account identifier; the
|
||||
* real `saveAndRespond()` call sites already do (see below).
|
||||
*
|
||||
* Exported for unit tests (#3615).
|
||||
*/
|
||||
export function findKiroConnectionByProfileArn(
|
||||
connections: ProviderConnectionLike[],
|
||||
profileArn: string | undefined
|
||||
profileArn: string | undefined,
|
||||
accountIdentity?: { email?: string | null; clientId?: string | null }
|
||||
): ProviderConnectionLike | null {
|
||||
return findKiroConnectionByIdentity(connections, { profileArn });
|
||||
return findKiroConnectionByIdentity(connections, { profileArn, ...accountIdentity });
|
||||
}
|
||||
|
||||
// ── Save to OmniRoute DB ──────────────────────────────────────────────────────
|
||||
|
||||
@@ -2,9 +2,15 @@ import test from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
|
||||
// Repro for #6571 — REST-fallback path of `omniroute compression` (hit only when
|
||||
// /api/mcp/tools/call is not mounted, i.e. mcpCall()'s 404/501 branch) uses the
|
||||
// the MCP surface is not mounted, i.e. mcpCall()'s 404/501 branch) uses the
|
||||
// nonexistent `engine` field instead of the canonical `defaultMode` field, and
|
||||
// the table renderer prints "[object Object]" for nested object cells.
|
||||
//
|
||||
// #10960 moved the MCP transport from the never-mounted `/api/mcp/tools/call`
|
||||
// to the real Streamable HTTP endpoint `/api/mcp/stream` (mcpClient.mjs ->
|
||||
// callMcpEndpoint()). The REST-fallback trigger in these mocks must match
|
||||
// that endpoint, not the retired one, or mcpCallTool() throws on an
|
||||
// unmocked fetch instead of exercising the fallback path this test targets.
|
||||
|
||||
type MockResponse = Pick<Response, "ok" | "status" | "headers" | "json" | "text">;
|
||||
|
||||
@@ -45,7 +51,7 @@ test("restCompressionStatus (via runCompressionStatus REST fallback) should surf
|
||||
const origFetch = globalThis.fetch;
|
||||
globalThis.fetch = (async (url: string | URL | Request) => {
|
||||
const u = String(url);
|
||||
if (u.includes("/api/mcp/tools/call")) return makeResp({ error: "not mounted" }, 404);
|
||||
if (u.includes("/api/mcp/stream")) return makeResp({ error: "not mounted" }, 404);
|
||||
if (u.includes("/api/settings/compression")) {
|
||||
// Canonical server payload — NOTE: field is `defaultMode`, there is no `engine` key.
|
||||
// src/lib/db/compression.ts COMPRESSION_MODES / GET route just returns getCompressionSettings().
|
||||
@@ -85,7 +91,7 @@ test("restSetEngine (via runCompressionEngineSet REST fallback) should PUT `defa
|
||||
const putBodies: Record<string, unknown>[] = [];
|
||||
globalThis.fetch = (async (url: string | URL | Request, init?: RequestInit) => {
|
||||
const u = String(url);
|
||||
if (u.includes("/api/mcp/tools/call")) return makeResp({ error: "not mounted" }, 404);
|
||||
if (u.includes("/api/mcp/stream")) return makeResp({ error: "not mounted" }, 404);
|
||||
if (u.includes("/api/settings/compression") && init?.method === "PUT") {
|
||||
const body = init?.body ? JSON.parse(String(init.body)) : {};
|
||||
putBodies.push(body);
|
||||
|
||||
@@ -113,12 +113,18 @@ test("derived name is never empty or null", () => {
|
||||
|
||||
const FAKE_PROFILE_ARN = "arn:aws:iam::123456789012:user/sso-user";
|
||||
|
||||
const FAKE_CLIENT_ID = "client-abc";
|
||||
|
||||
const fakeConnectionWithArn = {
|
||||
id: "conn-abc",
|
||||
provider: "kiro",
|
||||
authType: "oauth",
|
||||
email: null,
|
||||
providerSpecificData: { profileArn: FAKE_PROFILE_ARN, region: "us-east-1" },
|
||||
providerSpecificData: {
|
||||
profileArn: FAKE_PROFILE_ARN,
|
||||
region: "us-east-1",
|
||||
clientId: FAKE_CLIENT_ID,
|
||||
},
|
||||
};
|
||||
|
||||
const fakeConnectionNoArn = {
|
||||
@@ -129,13 +135,29 @@ const fakeConnectionNoArn = {
|
||||
providerSpecificData: { region: "us-east-1" },
|
||||
};
|
||||
|
||||
test("findKiroConnectionByProfileArn returns the matching connection", async () => {
|
||||
// The function should scan existing kiro connections and match by profileArn.
|
||||
test("findKiroConnectionByProfileArn returns the matching connection when an account identifier agrees", async () => {
|
||||
// #10815 — matching on profileArn alone is unsafe (distinct Builder ID
|
||||
// accounts can share a profile ARN), so the caller must also supply an
|
||||
// account-level identifier (email or clientId) that does not contradict
|
||||
// the stored connection, exactly like saveAndRespond()'s real call sites do.
|
||||
const result = await findKiroConnectionByProfileArn(
|
||||
[fakeConnectionWithArn, fakeConnectionNoArn],
|
||||
FAKE_PROFILE_ARN,
|
||||
{ clientId: FAKE_CLIENT_ID }
|
||||
);
|
||||
assert.deepEqual(result, fakeConnectionWithArn);
|
||||
});
|
||||
|
||||
test("findKiroConnectionByProfileArn returns null for a profileArn-only match with no account identifier (#10815)", async () => {
|
||||
// Guards the #10815 fix: two different Builder ID accounts (Google/GitHub
|
||||
// social login) can share the same CodeWhisperer profile ARN, so trusting
|
||||
// an ARN match without any account identifier would let a second social
|
||||
// login silently overwrite the first connection.
|
||||
const result = await findKiroConnectionByProfileArn(
|
||||
[fakeConnectionWithArn, fakeConnectionNoArn],
|
||||
FAKE_PROFILE_ARN
|
||||
);
|
||||
assert.deepEqual(result, fakeConnectionWithArn);
|
||||
assert.equal(result, null);
|
||||
});
|
||||
|
||||
test("findKiroConnectionByProfileArn returns null when no match exists", async () => {
|
||||
|
||||
@@ -111,7 +111,13 @@ describe("injectMemory system-must-be-first (#6135)", () => {
|
||||
|
||||
it("regression: a NON-flagged provider keeps the existing cache-safe placement", () => {
|
||||
const req = multiTurn();
|
||||
const out = injectMemory(req, [mem("dark mode")], "anthropic", { cacheSafe: true });
|
||||
// #11290/#11303 added a Claude-family-specific reroute to injectSystemFirst()
|
||||
// for the mid-array splice (a system message right after a plain-text
|
||||
// assistant turn is rejected by Claude Opus 5), so "anthropic" no longer
|
||||
// exercises the plain cache-safe splice path this test targets. Use a
|
||||
// provider outside both the strict-system-first set AND the Claude family
|
||||
// to keep testing the original (still-current) cache-safe behavior.
|
||||
const out = injectMemory(req, [mem("dark mode")], "openai", { cacheSafe: true });
|
||||
// Existing behavior: memory inserted just before the last user message (index 3).
|
||||
assert.equal(out.messages[3].role, "system");
|
||||
assert.ok(out.messages[3].content.includes("Memory context"));
|
||||
|
||||
Reference in New Issue
Block a user