test(chatcore): tests for telemetry/memory-skills/semantic-cache + wire 2 into stryker (QG v2 Fase 9 T5 Fase 3) (#4222)

Integrated into release/v3.8.29 (QG v2 Fase 9 T5 Fase 3 — last 3 chatCore leaf tests + stryker batch i).

.github/workflows/nightly-mutation.yml

@@ -24,10 +24,11 @@ jobs:
     #     2 batches (d=heavy, e=light). After #4204 (D7b) merged, the reset-aware quota pair
     #     quotaScoring/quotaStrategies was added as batch f. A covering-test audit then added the
     #     6 chatCore/* leaves with direct unit coverage as batch g. A follow-up then wrote dedicated
-    #     unit tests for 6 more leaves and added them as batch h; the last 3 chatCore leaves
-    #     (memorySkillsInjection/semanticCache/telemetryHelpers) remain a follow-up — see
-    #     _mutate_godfiles_excluded_comment in stryker.conf.json.
-    # 8 PARALLEL batches, each overriding the mutate set via `--mutate` (Stryker 9 CLI:
+    #     unit tests for 6 more leaves and added them as batch h. A final follow-up added dedicated
+    #     tests (no mock.module — fetch-override + crafted inputs + temp-DATA_DIR) for telemetryHelpers
+    #     + memorySkillsInjection as batch i (14/15 chatCore leaves). semanticCache stays out (its
+    #     cache-HIT block needs a fixture) — see _mutate_godfiles_excluded_comment in stryker.conf.json.
+    # 9 PARALLEL batches, each overriding the mutate set via `--mutate` (Stryker 9 CLI:
     # `-m, --mutate <comma-list>`; the conf's `mutate[]` remains the local-run default/union).
     # Each batch targets <180min; full coverage every night in parallel (~180min wall-clock).
     # The `incremental` cache (per-batch key) makes runs after the first cold one much cheaper,
@@ -54,6 +55,8 @@ jobs:
             mutate: "open-sse/handlers/chatCore/comboContextCache.ts,open-sse/handlers/chatCore/idempotency.ts,open-sse/handlers/chatCore/passthroughHelpers.ts,open-sse/handlers/chatCore/responseHeaders.ts,open-sse/handlers/chatCore/sanitization.ts,open-sse/handlers/chatCore/upstreamTimeouts.ts"
           - name: h
             mutate: "open-sse/handlers/chatCore/headers.ts,open-sse/handlers/chatCore/logTruncation.ts,open-sse/handlers/chatCore/memoryExtraction.ts,open-sse/handlers/chatCore/nonStreamingSse.ts,open-sse/handlers/chatCore/passthroughToolNames.ts,open-sse/handlers/chatCore/executorHelpers.ts"
+          - name: i
+            mutate: "open-sse/handlers/chatCore/telemetryHelpers.ts,open-sse/handlers/chatCore/memorySkillsInjection.ts"
     timeout-minutes: 180
     steps:
       - uses: actions/checkout@v6

stryker.conf.json

@@ -73,10 +73,12 @@
       "tests/unit/chatcore-imports-cleanly.test.ts",
       "tests/unit/chatcore-log-truncation.test.ts",
       "tests/unit/chatcore-memory-extraction.test.ts",
+      "tests/unit/chatcore-memory-skills-injection.test.ts",
       "tests/unit/chatcore-non-streaming-sse.test.ts",
       "tests/unit/chatcore-passthrough-tool-names.test.ts",
       "tests/unit/chatcore-sanitization.test.ts",
       "tests/unit/chatcore-strip-stale-headers.test.ts",
+      "tests/unit/chatcore-telemetry-helpers.test.ts",
       "tests/unit/chatcore-translation-paths.test.ts",
       "tests/unit/check-error-helper.test.ts",
       "tests/unit/check-route-guard-membership.test.ts",
@@ -214,15 +216,20 @@
     "-log-truncation, -memory-extraction, -non-streaming-sse, -passthrough-tool-names,",
     "-executor-helpers — wired into tap.testFiles above) and added those leaves as batch h",
     "(headers/logTruncation/memoryExtraction/nonStreamingSse/passthroughToolNames/executorHelpers).",
+    "A later follow-up added dedicated tests (NO mock.module — unavailable under the tap-runner; used",
+    "fetch-override + crafted inputs + temp-DATA_DIR) for telemetryHelpers (both fns, all branches) and",
+    "memorySkillsInjection (getSkillsProviderForFormat fully + injectMemoryAndSkills guards/empty-DB",
+    "path) and added them as batch i. 14 of 15 chatCore leaves now mutated.",
     "",
     "STILL EXCLUDED (follow-ups, NOT in `mutate` yet):",
     "  - combo.ts + chatCore.ts barrels: their handleComboChat/handleChatCore CORES were not",
     "    split (out of scope — Fase 3 ChatCoreContext refactor). The barrels are now thin-ish",
     "    but still large; keep excluded until the cores are split.",
-    "  - the last 3 chatCore/* leaves (memorySkillsInjection/semanticCache/telemetryHelpers): no",
-    "    dedicated unit test yet (memorySkillsInjection has 6 imports, semanticCache 7, telemetryHelpers",
-    "    uses fetch) — they need mocked deps. Add each once it gains a dedicated unit test (or after",
-    "    a Stryker --dryRunOnly confirms the perTest map covers it).",
+    "  - the LAST chatCore/* leaf, semanticCache.ts: it HAS a dedicated test (chatcore-semantic-cache,",
+    "    covers the guard-false + cache-MISS paths) but the cache-HIT block — the bulk of the logic —",
+    "    needs a populated-cache fixture, so most of its mutants would survive (no clean kill). Kept out",
+    "    of `mutate` until a HIT-path fixture test exists; the test runs in the normal test:unit suite",
+    "    as a regression guard.",
     "See project memory: Quality Gate v2 / Fase 9 (project-combo-split)."
   ],
   "mutate": [
@@ -253,7 +260,9 @@
     "open-sse/handlers/chatCore/memoryExtraction.ts",
     "open-sse/handlers/chatCore/nonStreamingSse.ts",
     "open-sse/handlers/chatCore/passthroughToolNames.ts",
-    "open-sse/handlers/chatCore/executorHelpers.ts"
+    "open-sse/handlers/chatCore/executorHelpers.ts",
+    "open-sse/handlers/chatCore/telemetryHelpers.ts",
+    "open-sse/handlers/chatCore/memorySkillsInjection.ts"
   ],
   "_ignorePatterns_comment": [
     "ignorePatterns = files NOT copied into the Stryker sandbox. It does NOT scope",

tests/unit/chatcore-memory-skills-injection.test.ts

@@ -0,0 +1,128 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+
+// Isolated DATA_DIR set BEFORE importing anything that touches the DB
+// (injectMemoryAndSkills -> getMemorySettings / retrieveMemories / injectSkills).
+const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-mem-skills-"));
+process.env.DATA_DIR = TEST_DATA_DIR;
+
+const { getSkillsProviderForFormat, injectMemoryAndSkills } = await import(
+  "../../open-sse/handlers/chatCore/memorySkillsInjection.ts"
+);
+const { FORMATS } = await import("../../open-sse/translator/formats.ts");
+const core = await import("../../src/lib/db/core.ts");
+
+test.after(() => {
+  core.resetDbInstance();
+  fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+});
+
+// ─── getSkillsProviderForFormat (pure switch) ────────────────────────────────
+
+test("getSkillsProviderForFormat maps CLAUDE -> anthropic", () => {
+  assert.equal(getSkillsProviderForFormat(FORMATS.CLAUDE), "anthropic");
+});
+
+test("getSkillsProviderForFormat maps GEMINI -> google", () => {
+  assert.equal(getSkillsProviderForFormat(FORMATS.GEMINI), "google");
+});
+
+test("getSkillsProviderForFormat maps OPENAI and any unknown format -> openai (default)", () => {
+  assert.equal(getSkillsProviderForFormat(FORMATS.OPENAI), "openai");
+  // any other / unknown format falls through to the default branch
+  assert.equal(getSkillsProviderForFormat("gemini-cli"), "openai");
+  assert.equal(getSkillsProviderForFormat("codex"), "openai");
+  assert.equal(getSkillsProviderForFormat("totally-unknown"), "openai");
+  assert.equal(getSkillsProviderForFormat(""), "openai");
+});
+
+// ─── injectMemoryAndSkills ───────────────────────────────────────────────────
+
+test("injectMemoryAndSkills with memoryOwnerId=null skips both branches and returns the body unchanged", async () => {
+  // memoryOwnerId is null -> memorySettings stays null -> the memory guard is false
+  // (no getMemorySettings/retrieveMemories) AND the skills guard (memorySettings?.skillsEnabled)
+  // is false. The body is returned verbatim with memorySettings=null.
+  const body: Record<string, unknown> = {
+    model: "gpt-4o",
+    messages: [{ role: "user", content: "hello world" }],
+  };
+
+  const result = await injectMemoryAndSkills({
+    body,
+    memoryOwnerId: null,
+    provider: "openai",
+    effectiveModel: "gpt-4o",
+    sourceFormat: FORMATS.OPENAI,
+    targetFormat: FORMATS.OPENAI,
+    backgroundReason: null,
+    log: null,
+  });
+
+  assert.equal(result.memorySettings, null, "memorySettings is null when no owner is provided");
+  // body is returned as-is (same reference, no injection happened)
+  assert.equal(result.body, body);
+  assert.deepEqual(result.body.messages, [{ role: "user", content: "hello world" }]);
+  assert.equal("tools" in result.body, false, "no skills were injected");
+});
+
+test("injectMemoryAndSkills with an empty DB resolves settings, finds nothing to inject, returns body unchanged", async () => {
+  // memoryOwnerId is set -> getMemorySettings() resolves DB defaults (enabled, skillsEnabled).
+  // The body has NO `messages` array (only `input`), so shouldInjectMemory() returns false and
+  // the memory-retrieval branch is skipped. The skills branch runs injectSkills(), but the
+  // empty DB registry has no skills, so mergedTools.length == existingTools.length and the body
+  // is NOT cloned/mutated. This exercises the realistic "nothing to inject" path end-to-end.
+  const log = {
+    debug: (..._args: unknown[]) => {
+      /* swallow */
+    },
+  };
+  const body: Record<string, unknown> = {
+    model: "gpt-4o",
+    input: [{ role: "user", content: "no messages array here" }],
+  };
+
+  const result = await injectMemoryAndSkills({
+    body,
+    memoryOwnerId: "owner-empty-db",
+    provider: "openai",
+    effectiveModel: "gpt-4o",
+    sourceFormat: FORMATS.OPENAI,
+    targetFormat: FORMATS.OPENAI,
+    backgroundReason: null,
+    log,
+  });
+
+  // memorySettings was resolved (defaults) — it is a real object, not null.
+  assert.ok(result.memorySettings, "memorySettings resolved from DB defaults");
+  assert.equal(result.memorySettings.enabled, true);
+  assert.equal(result.memorySettings.skillsEnabled, true);
+  // No skills in the empty registry -> body returned unchanged (same reference).
+  assert.equal(result.body, body);
+  assert.equal("tools" in result.body, false, "no skills injected from an empty registry");
+});
+
+test("injectMemoryAndSkills resolves cleanly for a CLAUDE-format body with no owner (provider-mapping path)", async () => {
+  // Characterizes the no-owner short-circuit for a non-OpenAI source format. Nothing is
+  // injected; the function just returns the body untouched and memorySettings=null.
+  const body: Record<string, unknown> = {
+    model: "claude-3-5-sonnet",
+    messages: [{ role: "user", content: [{ type: "text", text: "hi" }] }],
+  };
+
+  const result = await injectMemoryAndSkills({
+    body,
+    memoryOwnerId: null,
+    provider: "claude",
+    effectiveModel: "claude-3-5-sonnet",
+    sourceFormat: FORMATS.CLAUDE,
+    targetFormat: FORMATS.CLAUDE,
+    backgroundReason: "background-task",
+    log: null,
+  });
+
+  assert.equal(result.memorySettings, null);
+  assert.equal(result.body, body);
+});

tests/unit/chatcore-semantic-cache.test.ts

@@ -0,0 +1,127 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+
+// Isolated DATA_DIR set BEFORE importing anything that touches the DB
+// (checkSemanticCache -> getCachedResponse reads the semantic_cache SQLite table).
+const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-sem-cache-"));
+process.env.DATA_DIR = TEST_DATA_DIR;
+
+const { checkSemanticCache } = await import("../../open-sse/handlers/chatCore/semanticCache.ts");
+const core = await import("../../src/lib/db/core.ts");
+
+test.after(() => {
+  core.resetDbInstance();
+  fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+});
+
+// A reusable persistAttemptLogs spy + base args. The functions below should NEVER be
+// invoked on the guard-false / cache-miss paths (those only run on a HIT).
+function makeBaseArgs(overrides: Record<string, unknown> = {}) {
+  const persistCalls: unknown[] = [];
+  const args = {
+    semanticCacheEnabled: false,
+    body: { model: "gpt-4o", messages: [{ role: "user", content: "hi" }], temperature: 0 },
+    clientRawRequest: { headers: {} },
+    model: "gpt-4o",
+    provider: "openai",
+    stream: false,
+    reqLogger: {
+      logConvertedResponse: () => {
+        throw new Error("logConvertedResponse should not run on guard-false / miss paths");
+      },
+    },
+    effectiveServiceTier: undefined,
+    connectionId: null as string | null,
+    startTime: Date.now(),
+    log: {
+      debug: () => {
+        throw new Error("log.debug should only fire on a cache HIT");
+      },
+    },
+    persistAttemptLogs: (a: unknown) => {
+      persistCalls.push(a);
+    },
+    apiKeyId: null as string | null,
+    ...overrides,
+  };
+  return { args, persistCalls };
+}
+
+// ─── checkSemanticCache ──────────────────────────────────────────────────────
+
+test("checkSemanticCache returns null when semanticCacheEnabled is false (outer guard short-circuit)", async () => {
+  const { args, persistCalls } = makeBaseArgs({ semanticCacheEnabled: false });
+
+  const result = await checkSemanticCache(args as Parameters<typeof checkSemanticCache>[0]);
+
+  assert.equal(result, null, "disabled cache -> no-op null result");
+  assert.equal(persistCalls.length, 0, "no logging side effects when the guard is false");
+});
+
+test("checkSemanticCache returns null when enabled but the body is NOT cacheable (temperature != 0)", async () => {
+  // isCacheableForRead requires temperature === 0. A non-zero temperature makes the guard
+  // false even with semanticCacheEnabled=true, so it short-circuits to the null no-op.
+  const { args, persistCalls } = makeBaseArgs({
+    semanticCacheEnabled: true,
+    body: { model: "gpt-4o", messages: [{ role: "user", content: "hi" }], temperature: 0.7 },
+  });
+
+  const result = await checkSemanticCache(args as Parameters<typeof checkSemanticCache>[0]);
+
+  assert.equal(result, null, "non-cacheable body -> guard false -> null");
+  assert.equal(persistCalls.length, 0);
+});
+
+test("checkSemanticCache returns null when the x-omniroute-no-cache header forces a bypass", async () => {
+  // The no-cache header makes isCacheableForRead return false even with temperature:0.
+  const { args } = makeBaseArgs({
+    semanticCacheEnabled: true,
+    body: { model: "gpt-4o", messages: [{ role: "user", content: "hi" }], temperature: 0 },
+    clientRawRequest: { headers: { "x-omniroute-no-cache": "true" } },
+  });
+
+  const result = await checkSemanticCache(args as Parameters<typeof checkSemanticCache>[0]);
+
+  assert.equal(result, null, "explicit no-cache header bypasses the cache read");
+});
+
+test("checkSemanticCache returns null on a cache MISS (enabled + cacheable body + empty cache)", async () => {
+  // semanticCacheEnabled=true AND temperature:0 (cacheable) -> enters the guard -> builds a
+  // signature -> getCachedResponse() finds nothing in the empty (fresh DATA_DIR) cache ->
+  // the `if (cached)` block is skipped -> the function falls through to `return null`.
+  const { args, persistCalls } = makeBaseArgs({
+    semanticCacheEnabled: true,
+    body: {
+      model: "gpt-4o",
+      messages: [{ role: "user", content: "a unique miss query " + Date.now() }],
+      temperature: 0,
+    },
+  });
+
+  const result = await checkSemanticCache(args as Parameters<typeof checkSemanticCache>[0]);
+
+  assert.equal(result, null, "empty cache -> MISS -> null (no HIT-path side effects)");
+  assert.equal(persistCalls.length, 0, "persistAttemptLogs only runs on a HIT");
+});
+
+test("checkSemanticCache MISS also works for the Responses-API `input` body shape", async () => {
+  // generateSignature falls back to body.input when body.messages is absent; the empty cache
+  // still yields a MISS, so the result is null.
+  const { args, persistCalls } = makeBaseArgs({
+    semanticCacheEnabled: true,
+    body: {
+      model: "gpt-4o",
+      input: [{ role: "user", content: "responses api miss " + Date.now() }],
+      temperature: 0,
+    },
+    stream: true,
+  });
+
+  const result = await checkSemanticCache(args as Parameters<typeof checkSemanticCache>[0]);
+
+  assert.equal(result, null);
+  assert.equal(persistCalls.length, 0);
+});

tests/unit/chatcore-telemetry-helpers.test.ts

@@ -0,0 +1,173 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+
+// Isolated DATA_DIR set BEFORE importing anything that may touch the DB
+// (maybeSyncClaudeExtraUsageState -> fetchLiveProviderLimits -> getProviderConnectionById).
+const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-telemetry-"));
+process.env.DATA_DIR = TEST_DATA_DIR;
+
+const { forwardDashboardEventToLiveWs, maybeSyncClaudeExtraUsageState } = await import(
+  "../../open-sse/handlers/chatCore/telemetryHelpers.ts"
+);
+const core = await import("../../src/lib/db/core.ts");
+
+const originalFetch = globalThis.fetch;
+const originalLiveWsPort = process.env.LIVE_WS_PORT;
+
+test.afterEach(() => {
+  globalThis.fetch = originalFetch;
+  if (originalLiveWsPort === undefined) {
+    delete process.env.LIVE_WS_PORT;
+  } else {
+    process.env.LIVE_WS_PORT = originalLiveWsPort;
+  }
+});
+
+test.after(() => {
+  globalThis.fetch = originalFetch;
+  core.resetDbInstance();
+  fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+});
+
+// ─── forwardDashboardEventToLiveWs ───────────────────────────────────────────
+
+test("forwardDashboardEventToLiveWs POSTs event+payload+timestamp as JSON to the default port", async () => {
+  delete process.env.LIVE_WS_PORT;
+  let capturedUrl: string | undefined;
+  let capturedInit: RequestInit | undefined;
+  globalThis.fetch = (async (url: string, init: RequestInit) => {
+    capturedUrl = url;
+    capturedInit = init;
+    return new Response("ok", { status: 200 });
+  }) as typeof fetch;
+
+  const before = Date.now();
+  await forwardDashboardEventToLiveWs("my-event", { foo: "bar" });
+  const after = Date.now();
+
+  // Default port is 20129 when LIVE_WS_PORT is unset.
+  assert.equal(capturedUrl, "http://127.0.0.1:20129/__omniroute_event");
+  assert.equal(capturedInit?.method, "POST");
+  assert.equal(
+    (capturedInit?.headers as Record<string, string>)["content-type"],
+    "application/json"
+  );
+  assert.ok(capturedInit?.signal, "an AbortSignal is attached for the 1.5s timeout");
+
+  const parsed = JSON.parse(capturedInit?.body as string);
+  assert.equal(parsed.event, "my-event");
+  assert.deepEqual(parsed.payload, { foo: "bar" });
+  assert.equal(typeof parsed.timestamp, "number");
+  assert.ok(
+    parsed.timestamp >= before && parsed.timestamp <= after,
+    "timestamp is Date.now() captured at call time"
+  );
+});
+
+test("forwardDashboardEventToLiveWs honors LIVE_WS_PORT override", async () => {
+  process.env.LIVE_WS_PORT = "31337";
+  let capturedUrl: string | undefined;
+  globalThis.fetch = (async (url: string) => {
+    capturedUrl = url;
+    return new Response("ok", { status: 200 });
+  }) as typeof fetch;
+
+  await forwardDashboardEventToLiveWs("e", null);
+
+  assert.equal(capturedUrl, "http://127.0.0.1:31337/__omniroute_event");
+});
+
+test("forwardDashboardEventToLiveWs swallows fetch rejection and still resolves", async () => {
+  globalThis.fetch = (async () => {
+    throw new Error("sidecar down");
+  }) as typeof fetch;
+
+  // Must not throw — best-effort sidecar bridge; the catch swallows.
+  await assert.doesNotReject(forwardDashboardEventToLiveWs("e", { a: 1 }));
+});
+
+// ─── maybeSyncClaudeExtraUsageState ──────────────────────────────────────────
+
+test("maybeSyncClaudeExtraUsageState returns early when connectionId is falsy (no fetch)", async () => {
+  let fetchCalled = false;
+  globalThis.fetch = (async () => {
+    fetchCalled = true;
+    return new Response("", { status: 200 });
+  }) as typeof fetch;
+
+  await maybeSyncClaudeExtraUsageState({
+    provider: "claude",
+    connectionId: null,
+    providerSpecificData: {},
+    log: null,
+  });
+
+  assert.equal(fetchCalled, false, "guard short-circuits before any network/DB work");
+});
+
+test("maybeSyncClaudeExtraUsageState returns early for non-claude provider (block disabled)", async () => {
+  let fetchCalled = false;
+  globalThis.fetch = (async () => {
+    fetchCalled = true;
+    return new Response("", { status: 200 });
+  }) as typeof fetch;
+
+  await maybeSyncClaudeExtraUsageState({
+    provider: "openai",
+    connectionId: "some-conn",
+    providerSpecificData: {},
+    log: null,
+  });
+
+  assert.equal(fetchCalled, false, "isClaudeExtraUsageBlockEnabled is false for non-claude");
+});
+
+test("maybeSyncClaudeExtraUsageState returns early for claude with blockExtraUsage:false", async () => {
+  let fetchCalled = false;
+  globalThis.fetch = (async () => {
+    fetchCalled = true;
+    return new Response("", { status: 200 });
+  }) as typeof fetch;
+
+  await maybeSyncClaudeExtraUsageState({
+    provider: "claude",
+    connectionId: "some-conn",
+    providerSpecificData: { blockExtraUsage: false },
+    log: null,
+  });
+
+  assert.equal(fetchCalled, false, "explicit blockExtraUsage:false disables the block");
+});
+
+test("maybeSyncClaudeExtraUsageState enters the try for claude+enabled, swallows the error, and logs via log.debug", async () => {
+  // provider=claude, providerSpecificData={} (blockExtraUsage !== false), connectionId set
+  // -> passes the guard -> calls the REAL fetchLiveProviderLimits("bogus-conn") which
+  // looks the connection up in the (empty) DB, finds nothing, throws "Connection not found",
+  // and the function's internal try/catch swallows it while logging to log.debug.
+  const calls: unknown[][] = [];
+  const log = {
+    debug: (...args: unknown[]) => {
+      calls.push(args);
+    },
+  };
+
+  await assert.doesNotReject(
+    maybeSyncClaudeExtraUsageState({
+      provider: "claude",
+      connectionId: "bogus-conn-id",
+      providerSpecificData: {},
+      log,
+    })
+  );
+
+  assert.equal(calls.length, 1, "the swallowed error path logs exactly once");
+  assert.equal(calls[0][0], "CLAUDE_USAGE");
+  assert.match(
+    String(calls[0][1]),
+    /Failed to sync Claude extra-usage state:/,
+    "logs the sync-failure message with the underlying error text"
+  );
+});