From 90507a8a82d2cdaed3f4cc4abd0fddba8cec1e08 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza <8016841+diegosouzapw@users.noreply.github.com> Date: Thu, 18 Jun 2026 22:36:14 -0300 Subject: [PATCH] test(chatcore): tests for telemetry/memory-skills/semantic-cache + wire 2 into stryker (QG v2 Fase 9 T5 Fase 3) (#4222) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 | 11 +- stryker.conf.json | 19 +- .../chatcore-memory-skills-injection.test.ts | 128 +++++++++++++ tests/unit/chatcore-semantic-cache.test.ts | 127 +++++++++++++ tests/unit/chatcore-telemetry-helpers.test.ts | 173 ++++++++++++++++++ 5 files changed, 449 insertions(+), 9 deletions(-) create mode 100644 tests/unit/chatcore-memory-skills-injection.test.ts create mode 100644 tests/unit/chatcore-semantic-cache.test.ts create mode 100644 tests/unit/chatcore-telemetry-helpers.test.ts diff --git a/.github/workflows/nightly-mutation.yml b/.github/workflows/nightly-mutation.yml index cb7f520f43..92a301cb4c 100644 --- a/.github/workflows/nightly-mutation.yml +++ b/.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 `; 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 diff --git a/stryker.conf.json b/stryker.conf.json index 2faf253376..b7de9f5ff3 100644 --- a/stryker.conf.json +++ b/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", diff --git a/tests/unit/chatcore-memory-skills-injection.test.ts b/tests/unit/chatcore-memory-skills-injection.test.ts new file mode 100644 index 0000000000..54aff6bf9a --- /dev/null +++ b/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 = { + 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 = { + 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 = { + 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); +}); diff --git a/tests/unit/chatcore-semantic-cache.test.ts b/tests/unit/chatcore-semantic-cache.test.ts new file mode 100644 index 0000000000..7e9c99166d --- /dev/null +++ b/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 = {}) { + 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[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[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[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[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[0]); + + assert.equal(result, null); + assert.equal(persistCalls.length, 0); +}); diff --git a/tests/unit/chatcore-telemetry-helpers.test.ts b/tests/unit/chatcore-telemetry-helpers.test.ts new file mode 100644 index 0000000000..a155da8f7c --- /dev/null +++ b/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)["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" + ); +});