diff --git a/open-sse/handlers/chatCore.ts b/open-sse/handlers/chatCore.ts index f9935c0588..bbe358da63 100644 --- a/open-sse/handlers/chatCore.ts +++ b/open-sse/handlers/chatCore.ts @@ -77,7 +77,11 @@ import { isStripReasoningRequested, } from "./chatCore/headers.ts"; import { markCodexScopeRateLimited } from "./chatCore/codexFailover.ts"; -import { getCodexClientSessionId, isCodexOriginatedHeaders, isClaudeCodeOriginatedHeaders } from "../config/codexIdentity.ts"; +import { + getCodexClientSessionId, + isCodexOriginatedHeaders, + isClaudeCodeOriginatedHeaders, +} from "../config/codexIdentity.ts"; import { noteCodexTurnStateProvenance, readCodexTurnStateHeader, @@ -123,6 +127,7 @@ import { extractMemoryTextFromResponse, extractMemoryTextFromRequestBody, resolveMemoryOwnerId, + shouldExtractMemory, } from "./chatCore/memoryExtraction.ts"; import { CORS_HEADERS } from "../utils/cors.ts"; import { checkResourcePressureGuard } from "../utils/resourcePressure.ts"; @@ -359,6 +364,7 @@ import { assertExclusiveConnectionLeaseFence } from "@/lib/db/exclusiveConnectio import { deleteSessionAccountAffinity } from "@/lib/db/sessionAccountAffinity"; import { getCacheControlSettings } from "@/lib/cacheControlSettings"; import { guardrailRegistry } from "@/lib/guardrails"; +import type { VideoBridgeLogRedactionEntry } from "@/lib/guardrails/videoBridge"; import { shouldPreserveCacheControl, resolveConnectionCacheOverride, @@ -478,6 +484,15 @@ type ChatCoreExecutorResult = ReturnType & { _accountSemaphoreRelease?: () => void; }; +/** + * #12150 P1b: shape of handleChatCore's optional `videoBridgeLog` param — see + * its destructure default below. `handleChatCore`'s own params object has no + * type annotation (pre-existing convention for this god-function), so this + * alias is applied via a local cast at each read site instead of widening + * the whole destructure to a typed object. + */ +type VideoBridgeLogParam = { observed: boolean; redaction: VideoBridgeLogRedactionEntry[] } | null; + /** * Core chat handler - shared between SSE and Worker * Returns { success, response, status, error } for caller to handle fallback @@ -528,8 +543,22 @@ export async function handleChatCore({ skipResourcePressureGuard = false, reasoningTransportFallback = "drop", managedLease = null, + // #12150 P1b: additive, optional video-bridge log/Memory shadow — shape is + // VideoBridgeLogParam (defined near the top of this file). Built once in chat.ts from + // preCallGuardrails.results (video-bridge guardrail meta) and threaded here + // through executeChatWithBreaker. `undefined` for every non-video request, + // so this parameter changes nothing on the byte-identical default path. + // `observed` gates durable Memory extraction (surface 3); `redaction` is + // applied to a CLONE of `body` at the persistAttemptLogs sink (surface 1) — + // the model-bound `body` itself is never touched. + videoBridgeLog = undefined, }) { let { provider, model, extendedContext } = modelInfo; + // #12150 P1b: true iff the video-bridge guardrail rendered >=1 transcript + // cue into a replaced part of this request. Gates request-derived Memory + // extraction (chatCore/memoryExtraction.ts::shouldExtractMemory). + const videoBridgeObserved: boolean = + (videoBridgeLog as VideoBridgeLogParam | undefined)?.observed === true; const resilienceSettings = resolveResilienceSettings(cachedSettings); if (!skipResourcePressureGuard) { try { @@ -1062,6 +1091,9 @@ export async function handleChatCore({ // client explicitly sent x-omniroute-session-id. The raw header remains a // fallback for any caller that somehow bypassed conversationId resolution. sessionTag: conversationId || explicitSessionIdHeader, + // #12150 P1b surface 1: undefined for every non-video request (byte-identical + // to before this param existed) — see applyVideoBridgeLogRedaction. + videoBridgeLogRedaction: (videoBridgeLog as VideoBridgeLogParam | undefined)?.redaction, }); // Primary path: merge client model id + alias target so config on either key applies; resolved @@ -5132,9 +5164,28 @@ export async function handleChatCore({ ); if (memoryOwnerId && memorySettings?.enabled && memorySettings.maxTokens > 0) { - const requestMemoryText = extractMemoryTextFromRequestBody(body as Record); - if (requestMemoryText) { - extractFacts(requestMemoryText, memoryOwnerId, pipelineSessionId); + // #12150 P1b surface 3: a video-bridge-observed request's request-derived + // text is a flattened transcript description, not user-authored + // conversation — never persist it into durable Memory. The + // response-derived extraction just below (the model's own reply) is + // unaffected — out of scope for this gap-closure task. + if ( + shouldExtractMemory({ + enabled: memorySettings.enabled, + maxTokens: memorySettings.maxTokens, + memoryOwnerId, + videoBridgeObserved, + }) + ) { + const requestMemoryText = extractMemoryTextFromRequestBody(body as Record); + if (requestMemoryText) { + extractFacts(requestMemoryText, memoryOwnerId, pipelineSessionId); + } + } else if (videoBridgeObserved) { + log?.debug?.( + "MEMORY", + "Skipping request-derived memory extraction: video-bridge transcript observed" + ); } const memoryText = extractMemoryTextFromResponse(memoryExtractionResponse); @@ -5761,9 +5812,26 @@ export async function handleChatCore({ memorySettings.maxTokens > 0 && streamStatus === 200 ) { - const requestMemoryText = extractMemoryTextFromRequestBody(body as Record); - if (requestMemoryText) { - extractFacts(requestMemoryText, memoryOwnerId, pipelineSessionId); + // #12150 P1b surface 3: see the matching non-streaming gate above — + // suppresses only the request-derived extraction for an observed + // request; the streamed-response extraction just below is unaffected. + if ( + shouldExtractMemory({ + enabled: memorySettings.enabled, + maxTokens: memorySettings.maxTokens, + memoryOwnerId, + videoBridgeObserved, + }) + ) { + const requestMemoryText = extractMemoryTextFromRequestBody(body as Record); + if (requestMemoryText) { + extractFacts(requestMemoryText, memoryOwnerId, pipelineSessionId); + } + } else if (videoBridgeObserved) { + log?.debug?.( + "MEMORY", + "Skipping request-derived memory extraction: video-bridge transcript observed" + ); } const streamedMemoryText = extractMemoryTextFromResponse( diff --git a/open-sse/handlers/chatCore/attemptLogging.ts b/open-sse/handlers/chatCore/attemptLogging.ts index 25d8033480..e2effde2ac 100644 --- a/open-sse/handlers/chatCore/attemptLogging.ts +++ b/open-sse/handlers/chatCore/attemptLogging.ts @@ -15,11 +15,76 @@ import { logAuditEvent } from "@/lib/compliance"; import { emit } from "@/lib/events/eventBus"; import type { RequestCompletedPayload, RequestFailedPayload } from "@/lib/events/types"; import { saveCallLog } from "@/lib/usageDb"; +import type { VideoBridgeLogRedactionEntry } from "@/lib/guardrails/videoBridge"; import { FORMATS } from "../../translator/formats.ts"; import { takeEarlyKeepaliveBytes } from "../../utils/earlyKeepaliveByteBuffer.ts"; import { cloneBoundedChatLogPayload, truncateForLog } from "./logTruncation.ts"; import { attachLogMeta } from "./cacheUsageMeta.ts"; +/** + * Apply the video-bridge redaction shadow (P1a's `meta.videoBridgeLogRedaction`, + * threaded here via `PersistAttemptLogsContext.videoBridgeLogRedaction`) to a + * CLONE of `body` before it is serialized into the persisted call log (#12150 + * surface 1). + * + * `body` itself is NEVER mutated: by the time an attempt is logged, this same + * `body` reference has already been sent upstream (the model path), so + * mutating it here would be both unsafe and pointless. Only the containers on + * the path to each redacted part are cloned (container array -> message -> + * content array -> part); every sibling message/part keeps referencing the + * original objects. Returns `body` unchanged (same reference, no allocation) + * when there is nothing to redact, so the common non-video path is + * byte-identical to before this function existed. + */ +export function applyVideoBridgeLogRedaction( + body: unknown, + redaction: VideoBridgeLogRedactionEntry[] | null | undefined +): unknown { + if (!redaction || redaction.length === 0) return body; + if (!body || typeof body !== "object") return body; + + const source = body as Record; + const clone: Record = { ...source }; + const clonedContainers = new Map(); + const clonedMessages = new Map>(); + + for (const entry of redaction) { + const { container, messageIndex, partIndex, redactedText } = entry; + const originalContainer = source[container]; + if (!Array.isArray(originalContainer)) continue; + + let containerClone = clonedContainers.get(container); + if (!containerClone) { + containerClone = [...originalContainer]; + clonedContainers.set(container, containerClone); + clone[container] = containerClone; + } + + const originalMessage = originalContainer[messageIndex]; + if (!originalMessage || typeof originalMessage !== "object") continue; + const originalContent = (originalMessage as Record).content; + if (!Array.isArray(originalContent)) continue; + + const messageKey = `${container}:${messageIndex}`; + let messageClone = clonedMessages.get(messageKey); + if (!messageClone) { + messageClone = { + ...(originalMessage as Record), + content: [...originalContent], + }; + clonedMessages.set(messageKey, messageClone); + containerClone[messageIndex] = messageClone; + } + + const contentClone = messageClone.content as unknown[]; + const originalPart = originalContent[partIndex]; + if (!originalPart || typeof originalPart !== "object") continue; + contentClone[partIndex] = { ...(originalPart as Record), text: redactedText }; + } + + return clone; +} + /** * Extract the OpenAI Responses API response id this attempt produced, so it * can be indexed for OmniRoute-native `previous_response_id` continuation @@ -89,6 +154,15 @@ export type PersistAttemptLogsContext = { * explicitly present (never synthesized from skillRequestId) — persisted as call_logs.session_tag * for per-session cost attribution. */ sessionTag?: string | null; + /** + * #12150 P1b: video-bridge structured-redaction shadow (P1a's + * `meta.videoBridgeLogRedaction`), threaded from chat.ts's + * `preCallGuardrails.results` down through handleChatCore. When present, + * `applyVideoBridgeLogRedaction` swaps each mapped part's text for the + * placeholder in the CLONE that gets persisted — `body` itself (the model + * path) is never touched. Omitted/empty for every non-video request. + */ + videoBridgeLogRedaction?: VideoBridgeLogRedactionEntry[]; }; function toConnectionId(value: unknown): string | null { @@ -204,6 +278,7 @@ export function persistAttemptLogs(args: PersistAttemptLogsArgs, ctx: PersistAtt correlationId, modelPinned, sessionTag, + videoBridgeLogRedaction, } = ctx; const initialConnectionId = toConnectionId(connectionId); const finalConnectionId = toConnectionId(credentials?.connectionId) || initialConnectionId; @@ -287,10 +362,15 @@ export function persistAttemptLogs(args: PersistAttemptLogsArgs, ctx: PersistAtt duration: Date.now() - startTime, tokens: tokens || {}, requestBody: cloneBoundedChatLogPayload( - attachLogMeta(truncateForLog(body as Record), { - ...accountRotationMeta, - claudePromptCache: claudeCacheMeta, - }) + attachLogMeta( + truncateForLog( + applyVideoBridgeLogRedaction(body, videoBridgeLogRedaction) as Record + ), + { + ...accountRotationMeta, + claudePromptCache: claudeCacheMeta, + } + ) ), responseBody: cloneBoundedChatLogPayload( attachLogMeta(truncateForLog(responseBody as Record), { diff --git a/open-sse/handlers/chatCore/memoryExtraction.ts b/open-sse/handlers/chatCore/memoryExtraction.ts index 7ca6f66da6..281e0b4b00 100644 --- a/open-sse/handlers/chatCore/memoryExtraction.ts +++ b/open-sse/handlers/chatCore/memoryExtraction.ts @@ -129,3 +129,30 @@ export function resolveMemoryOwnerId(apiKeyInfo: Record | null) } return null; } + +/** + * Pure decision for whether the request-derived text should be extracted into + * durable Memory (#12150 P1b, surface 3). Extracted from chatCore.ts's inline + * `memoryOwnerId && memorySettings?.enabled && memorySettings.maxTokens > 0` + * check (unchanged) plus one new condition: a video-bridge-observed request's + * request-derived text is a flattened transcript description, not + * user-authored conversation, so it must never be persisted as a "memory + * fact". `videoBridgeObserved` is optional and defaults to falsy, so every + * existing non-video caller (which never passes it) keeps today's exact + * behavior. Governs ONLY the request-derived extractFacts call — the + * response-derived one (the model's own reply) is out of scope and + * unaffected by this function. + */ +export function shouldExtractMemory(input: { + enabled: boolean | null | undefined; + maxTokens: number | null | undefined; + memoryOwnerId: string | null | undefined; + videoBridgeObserved?: boolean | null; +}): boolean { + const { enabled, maxTokens, memoryOwnerId, videoBridgeObserved } = input; + if (!memoryOwnerId) return false; + if (!enabled) return false; + if (!(typeof maxTokens === "number" && maxTokens > 0)) return false; + if (videoBridgeObserved) return false; + return true; +} diff --git a/src/sse/handlers/chat.ts b/src/sse/handlers/chat.ts index 18a2760519..ea7dcae05b 100644 --- a/src/sse/handlers/chat.ts +++ b/src/sse/handlers/chat.ts @@ -102,6 +102,7 @@ import { withConversationId, } from "./chatHelpers"; import { buildModalityBridgeHeader } from "@/lib/guardrails/modalityBridge/bridgeStats"; +import type { VideoBridgeLogRedactionEntry } from "@/lib/guardrails/videoBridge"; import { resolveConversationId } from "@omniroute/open-sse/services/conversationTracker.ts"; import { isAntigravityMissingProjectError, @@ -308,6 +309,34 @@ function intersectAllowedConnectionIds(primary: unknown, secondary: unknown): st return first || second || null; } +/** Shape of the videoBridgeLog param threaded to executeChatWithBreaker -> handleChatCore (#12150 P1b). */ +type VideoBridgeLog = { observed: boolean; redaction: VideoBridgeLogRedactionEntry[] }; + +/** + * #12150 P1b: derive the video-bridge log/Memory shadow from + * preCallGuardrails.results. Returns undefined when the video-bridge + * guardrail did not run (disabled, no video parts) or ran but rendered no + * transcript cue (ordinary video, or the request was blocked/failed before + * meta was set) — so every non-video request threads `undefined` through the + * dispatch chain, byte-identical to before this param existed. + * + * `results` is typed as a structural subset of GuardrailExecutionResult + * (src/lib/guardrails/base.ts), the same "no type dependency on the + * guardrail core" pattern already used by buildModalityBridgeHeader + * (modalityBridge/bridgeStats.ts). + */ +function deriveVideoBridgeLog( + results: Array<{ guardrail: string; meta?: Record | null }> +): VideoBridgeLog | undefined { + const entry = results.find((r) => r.guardrail === "video-bridge"); + const meta = entry?.meta; + if (!meta || typeof meta.videoBridgeObserved !== "boolean") return undefined; + const redaction = Array.isArray(meta.videoBridgeLogRedaction) + ? (meta.videoBridgeLogRedaction as VideoBridgeLogRedactionEntry[]) + : []; + return { observed: meta.videoBridgeObserved, redaction }; +} + function isManagedComboUnsupported( combo: ComboLike, settings: Record, @@ -741,6 +770,10 @@ async function handleChatImplementation( // guardrail transformed the payload (describe path) — stamped on the main // success exits below via withModalityBridgeHeader(). const modalityBridgeHeader = buildModalityBridgeHeader(preCallGuardrails.results); + // #12150 P1b: video-bridge log/Memory shadow — undefined on every + // non-video request. Threaded through handleSingleModelChat's + // runtimeOptions -> executeChatWithBreaker -> handleChatCore. + const videoBridgeLog = deriveVideoBridgeLog(preCallGuardrails.results); telemetry.endPhase(); // Agentic conversation tracking (X-ConversationId): resolved once per @@ -1110,6 +1143,7 @@ async function handleChatImplementation( reasoningIntent, reasoningRequestTags: requestRoutingTags.tags, managedLease, + videoBridgeLog, // #7360 follow-up: without this, a target dispatch abandoned by // targetTimeoutRunner.ts's per-target timeout (comboTargetTimeoutMs) // never learns it was abandoned — it only watches the ORIGINAL @@ -1180,6 +1214,7 @@ async function handleChatImplementation( forceLiveComboTest: isComboLiveTest, conversationId, managedLease, + videoBridgeLog, }, combo.strategy, true @@ -1273,6 +1308,7 @@ async function handleChatImplementation( reasoningIntent, reasoningRequestTags: requestRoutingTags.tags, managedLease, + videoBridgeLog, }, null, false @@ -1321,6 +1357,8 @@ async function handleSingleModelChat( reasoningRequestTags?: string[]; reasoningTransportFallback?: "skip" | "drop"; managedLease?: ManagedLeaseDispatchContext | null; + /** #12150 P1b: video-bridge log/Memory shadow — undefined on every non-video request. */ + videoBridgeLog?: VideoBridgeLog; /** * Per-target abort signal from combo.ts's targetTimeoutRunner * (comboTargetTimeoutMs) — see the #7360 follow-up comment at the @@ -1398,6 +1436,7 @@ async function handleSingleModelChat( redirectCombo.config?.reasoningTransportFallback === "skip" ? "skip" : "drop", conversationId: runtimeOptions?.conversationId ?? null, managedLease: runtimeOptions.managedLease ?? null, + videoBridgeLog: runtimeOptions.videoBridgeLog, // #7360 follow-up — see the primary handleSingleModel closure above. modelAbortSignal: target?.modelAbortSignal ?? null, }, @@ -1877,6 +1916,7 @@ async function handleSingleModelChat( sessionAffinityKey: runtimeOptions.sessionAffinityKey ?? null, reasoningTransportFallback: runtimeOptions.reasoningTransportFallback ?? "drop", managedLease: runtimeOptions.managedLease ?? null, + videoBridgeLog: runtimeOptions.videoBridgeLog, }, runtimeOptions ); diff --git a/src/sse/handlers/chatHelpers.ts b/src/sse/handlers/chatHelpers.ts index 958d934573..0f014f8e86 100644 --- a/src/sse/handlers/chatHelpers.ts +++ b/src/sse/handlers/chatHelpers.ts @@ -430,6 +430,10 @@ export async function executeChatWithBreaker({ reasoningTransportFallback = "drop", sessionAffinityKey = null, managedLease = null, + // #12150 P1b: additive, optional video-bridge log/Memory shadow — undefined + // for every non-video request. Passed straight through to handleChatCore; + // see its own destructure default for the shape and consumers. + videoBridgeLog = undefined, }: ExecuteChatWithBreakerOptions): Promise { let tlsFingerprintUsed = false; const normalizedTrafficType: TrafficType = @@ -489,6 +493,7 @@ export async function executeChatWithBreaker({ sessionAffinityKey, reasoningTransportFallback, managedLease, + videoBridgeLog, skipResourcePressureGuard: true, onCredentialsRefreshed: async (newCreds: any) => { await updateProviderCredentials(credentials.connectionId, { diff --git a/tests/unit/video-bridge-log-redaction.test.ts b/tests/unit/video-bridge-log-redaction.test.ts new file mode 100644 index 0000000000..2c32ead1ba --- /dev/null +++ b/tests/unit/video-bridge-log-redaction.test.ts @@ -0,0 +1,159 @@ +// tests/unit/video-bridge-log-redaction.test.ts +// P1b of #12150 (Video Bridge transcript retention) — surface 1 (call-log sink). +// Exercises the real persistAttemptLogs serialization (same harness pattern as +// tests/unit/chatcore-attempt-logging.test.ts): a real temp DB, a poll for the +// async saveCallLog write, and assertions on the persisted requestBody. +// +// Proves: when PersistAttemptLogsContext carries a videoBridgeLogRedaction map +// (P1a's per-part structured-redaction shadow), the PERSISTED requestBody has +// the transcript text swapped for the placeholder — while a control call +// WITHOUT the map (the byte-identical non-video path) keeps the original text, +// and the caller's own `body` object is never mutated in the process (the +// model already received the untouched original earlier in the request +// lifecycle; this call must not reach back and change it). +import { test, before, after } from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +const testDataDir = fs.mkdtempSync(path.join(os.tmpdir(), "omni-video-log-redaction-test-")); +process.env.DATA_DIR = testDataDir; + +const coreDb = await import("../../src/lib/db/core.ts"); +const { getCallLogById } = await import("../../src/lib/usage/callLogs.ts"); +const { persistAttemptLogs } = await import("../../open-sse/handlers/chatCore/attemptLogging.ts"); + +const SECRET = "secret words"; +const PLACEHOLDER_TEXT = + "[Video 1]: A person talks. transcript[00:00-00:02]: [redacted-video-transcript]"; + +function videoBody() { + return { + model: "openai/gpt-x", + messages: [ + { role: "system", content: "sys" }, + { + role: "user", + content: [ + { type: "text", text: "look at this video" }, + { + type: "text", + text: `[Video 1]: A person talks. transcript[00:00-00:02]: ${SECRET}`, + }, + ], + }, + ], + }; +} + +function baseCtx(overrides: Record = {}) { + return { + provider: "openai", + connectionId: "conn-1", + model: "gpt-x", + skillRequestId: "skill-1", + detailedLoggingEnabled: false, + reqLogger: null, + pendingRequestId: "REPLACE", + clientRawRequest: { endpoint: "/v1/chat/completions" }, + requestedModel: "gpt-x-requested", + credentials: { connectionId: "cred-conn" }, + startTime: Date.now(), + body: videoBody(), + sourceFormat: "openai", + targetFormat: "openai", + comboName: null, + comboStepId: null, + comboExecutionKey: null, + tokensCompressed: 0, + apiKeyInfo: { id: "key-1", name: "Key One" }, + noLogEnabled: false, + ...overrides, + } as Parameters[1]; +} + +async function pollForCallLog(id: string, tries = 120) { + for (let i = 0; i < tries; i++) { + const row = await getCallLogById(id); + if (row) return row as Record; + await new Promise((r) => setTimeout(r, 20)); + } + return null; +} + +function persistedPartText(requestBody: unknown): string { + const record = requestBody as { + messages?: Array<{ content?: Array<{ text?: string }> }>; + }; + return record?.messages?.[1]?.content?.[1]?.text ?? ""; +} + +before(async () => { + await coreDb.ensureDbInitialized(); +}); + +after(() => { + coreDb.resetDbInstance(); + fs.rmSync(testDataDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); +}); + +test("persisted requestBody carries the placeholder and never the raw transcript when a redaction map is present", async () => { + const id = "video-redacted-1"; + persistAttemptLogs( + { status: 200, tokens: { input: 1, output: 2 } }, + baseCtx({ + pendingRequestId: id, + videoBridgeLogRedaction: [ + { container: "messages", messageIndex: 1, partIndex: 1, redactedText: PLACEHOLDER_TEXT }, + ], + }) + ); + const row = await pollForCallLog(id); + assert.ok(row, "call log row should be persisted"); + const persistedText = persistedPartText(row.requestBody); + assert.equal(persistedText, PLACEHOLDER_TEXT); + assert.ok(!persistedText.includes(SECRET), "persisted log must not contain the raw transcript"); + assert.equal( + JSON.stringify(row.requestBody).includes(SECRET), + false, + "raw transcript must not appear anywhere in the persisted requestBody" + ); +}); + +test("control: without a redaction map the persisted requestBody keeps the original text (model path untouched)", async () => { + const id = "video-control-1"; + persistAttemptLogs( + { status: 200, tokens: { input: 1, output: 2 } }, + baseCtx({ pendingRequestId: id }) + ); + const row = await pollForCallLog(id); + assert.ok(row); + const persistedText = persistedPartText(row.requestBody); + assert.ok( + persistedText.includes(SECRET), + "control call (no redaction map) must keep the raw transcript text" + ); +}); + +test("the caller's body object is never mutated by the redaction", async () => { + const id = "video-nomutate-1"; + const body = videoBody(); + const snapshotBefore = JSON.parse(JSON.stringify(body)); + persistAttemptLogs( + { status: 200 }, + baseCtx({ + pendingRequestId: id, + body, + videoBridgeLogRedaction: [ + { container: "messages", messageIndex: 1, partIndex: 1, redactedText: PLACEHOLDER_TEXT }, + ], + }) + ); + await pollForCallLog(id); + assert.deepEqual( + body, + snapshotBefore, + "ctx.body must be byte-identical after persistAttemptLogs runs" + ); +}); diff --git a/tests/unit/video-bridge-memory-suppression.test.ts b/tests/unit/video-bridge-memory-suppression.test.ts new file mode 100644 index 0000000000..317f315ff3 --- /dev/null +++ b/tests/unit/video-bridge-memory-suppression.test.ts @@ -0,0 +1,170 @@ +// tests/unit/video-bridge-memory-suppression.test.ts +// P1b of #12150 (Video Bridge transcript retention) — surface 3 (Memory sink). +// +// chatCore.ts gates its two extractFacts(requestMemoryText, ...) call sites +// (non-streaming ~L5134, streaming ~L5758) on an inline +// `memoryOwnerId && memorySettings?.enabled && memorySettings.maxTokens > 0` +// check. This adds a fourth condition — the request must not be a +// video-bridge-observed one — extracted to a pure, exported decision function +// so it is unit-testable without invoking the handleChatCore monolith (same +// god-file-decomposition convention as chatCore/attemptLogging.ts, +// chatCore/nonStreamingUsageStats.ts, etc.). +// +// Only the REQUEST-derived extractFacts call is gated (per the design doc, +// "surface 3: gate extractFacts on !videoBridgeObserved for request-derived +// text") — the response-derived extractFacts call (the model's own reply) is +// out of scope and untouched. +import test from "node:test"; +import assert from "node:assert/strict"; + +import { + shouldExtractMemory, + extractMemoryTextFromRequestBody, +} from "../../open-sse/handlers/chatCore/memoryExtraction.ts"; + +// ─── shouldExtractMemory: pure decision table ────────────────────────────── + +test("shouldExtractMemory: videoBridgeObserved=true skips extraction even when memory is otherwise enabled", () => { + assert.equal( + shouldExtractMemory({ + enabled: true, + maxTokens: 2000, + memoryOwnerId: "key-1", + videoBridgeObserved: true, + }), + false + ); +}); + +test("shouldExtractMemory: videoBridgeObserved=false extracts when memory is enabled (unaffected non-video path)", () => { + assert.equal( + shouldExtractMemory({ + enabled: true, + maxTokens: 2000, + memoryOwnerId: "key-1", + videoBridgeObserved: false, + }), + true + ); +}); + +test("shouldExtractMemory: videoBridgeObserved omitted (undefined) behaves like false — additive param default", () => { + assert.equal( + shouldExtractMemory({ + enabled: true, + maxTokens: 2000, + memoryOwnerId: "key-1", + }), + true + ); +}); + +test("shouldExtractMemory: still false when memory disabled, regardless of videoBridgeObserved", () => { + assert.equal( + shouldExtractMemory({ + enabled: false, + maxTokens: 2000, + memoryOwnerId: "key-1", + videoBridgeObserved: false, + }), + false + ); +}); + +test("shouldExtractMemory: still false when maxTokens <= 0, regardless of videoBridgeObserved", () => { + assert.equal( + shouldExtractMemory({ + enabled: true, + maxTokens: 0, + memoryOwnerId: "key-1", + videoBridgeObserved: false, + }), + false + ); +}); + +test("shouldExtractMemory: still false when memoryOwnerId is null, regardless of videoBridgeObserved", () => { + assert.equal( + shouldExtractMemory({ + enabled: true, + maxTokens: 2000, + memoryOwnerId: null, + videoBridgeObserved: false, + }), + false + ); +}); + +// ─── Integration stub: wire the real decision + the real request-text ───── +// extractor together against a stubbed extractFacts, mirroring the exact +// shape of the two chatCore.ts call sites (only the DB-writing extractFacts +// is stubbed — everything else is the real exported implementation). + +function runRequestMemoryExtractionStub(params: { + memoryOwnerId: string | null; + memorySettings: { enabled: boolean; maxTokens: number }; + videoBridgeObserved: boolean; + body: Record; + pipelineSessionId: string; + extractFactsSpy: (text: string, ownerId: string, sessionId: string) => void; +}): void { + const { + memoryOwnerId, + memorySettings, + videoBridgeObserved, + body, + pipelineSessionId, + extractFactsSpy, + } = params; + if (memoryOwnerId && memorySettings?.enabled && memorySettings.maxTokens > 0) { + if ( + shouldExtractMemory({ + enabled: memorySettings.enabled, + maxTokens: memorySettings.maxTokens, + memoryOwnerId, + videoBridgeObserved, + }) + ) { + const requestMemoryText = extractMemoryTextFromRequestBody(body); + if (requestMemoryText) { + extractFactsSpy(requestMemoryText, memoryOwnerId, pipelineSessionId); + } + } + } +} + +const flattenedVideoBody = { + messages: [ + { + role: "user", + content: "[Video 1]: A person talks. transcript[00:00-00:02]: secret words", + }, + ], +}; + +test("integration stub: zero extractFacts calls for a video-bridge-observed request", () => { + const calls: Array<[string, string, string]> = []; + runRequestMemoryExtractionStub({ + memoryOwnerId: "key-1", + memorySettings: { enabled: true, maxTokens: 2000 }, + videoBridgeObserved: true, + body: flattenedVideoBody, + pipelineSessionId: "session-1", + extractFactsSpy: (text, ownerId, sessionId) => calls.push([text, ownerId, sessionId]), + }); + assert.equal(calls.length, 0, "extractFacts must not be called when videoBridgeObserved=true"); +}); + +test("integration stub: extractFacts IS called for the same body when video-bridge was not observed", () => { + const calls: Array<[string, string, string]> = []; + runRequestMemoryExtractionStub({ + memoryOwnerId: "key-1", + memorySettings: { enabled: true, maxTokens: 2000 }, + videoBridgeObserved: false, + body: flattenedVideoBody, + pipelineSessionId: "session-1", + extractFactsSpy: (text, ownerId, sessionId) => calls.push([text, ownerId, sessionId]), + }); + assert.equal(calls.length, 1, "extractFacts must run on the ordinary (non-video) path"); + assert.match(calls[0][0], /secret words/); +});