From 4608f1f719af35b4e708563a1a3b0778b595cd18 Mon Sep 17 00:00:00 2001 From: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> Date: Fri, 4 Sep 2026 03:47:42 -0300 Subject: [PATCH] feat(db): fail-closed previous_response_id continuation for redacted video turns (#12150 P2b) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A continuation turn resolves previous_response_id back to the stored client snapshot (responsesContinuationStore.ts). Since #12528 that snapshot has its video transcript cues structurally redacted to [redacted-video-transcript] before storage, so rehydrating it would forward the placeholder upstream as if it were genuine history. Mark such rows with a new call_logs.video_content_removed column (migration 173, set from videoBridgeObserved via the call-log write path) and make resolvePreviousResponseState fail closed on a marked row — the client resends full history, exactly like a real previous_response_not_found. Default 0, so every non-video request and continuation is byte-identical to before. TDD: RED→GREEN for the store refusal, the saveCallLog persistence, and the persistAttemptLogs ctx→row marker wiring. Refs #12150, #12430 (items 1-2). --- open-sse/handlers/chatCore.ts | 4 ++ open-sse/handlers/chatCore/attemptLogging.ts | 11 +++ .../173_call_logs_video_content_removed.sql | 16 +++++ src/lib/db/responsesContinuationStore.ts | 14 +++- src/lib/usage/callLogs.ts | 11 ++- .../unit/responses-continuation-store.test.ts | 68 ++++++++++++++++++- tests/unit/save-call-log-persistence.test.ts | 67 ++++++++++++++++++ tests/unit/video-bridge-log-redaction.test.ts | 35 ++++++++++ 8 files changed, 219 insertions(+), 7 deletions(-) create mode 100644 src/lib/db/migrations/173_call_logs_video_content_removed.sql diff --git a/open-sse/handlers/chatCore.ts b/open-sse/handlers/chatCore.ts index 622e934084..1b99f46cca 100644 --- a/open-sse/handlers/chatCore.ts +++ b/open-sse/handlers/chatCore.ts @@ -1097,6 +1097,10 @@ export async function handleChatCore({ // #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, + // #12150 P2 surface 2: mark the persisted call_logs row so + // resolvePreviousResponseState refuses to rehydrate a snapshot whose video + // transcript was redacted. false for every non-video request. + videoContentRemoved: videoBridgeObserved, }); // Primary path: merge client model id + alias target so config on either key applies; resolved diff --git a/open-sse/handlers/chatCore/attemptLogging.ts b/open-sse/handlers/chatCore/attemptLogging.ts index 5ae0876f77..3192ddbd9b 100644 --- a/open-sse/handlers/chatCore/attemptLogging.ts +++ b/open-sse/handlers/chatCore/attemptLogging.ts @@ -250,6 +250,15 @@ export type PersistAttemptLogsContext = { * path) is never touched. Omitted/empty for every non-video request. */ videoBridgeLogRedaction?: VideoBridgeLogRedactionEntry[]; + /** + * #12150 P2 surface 2: true when the video-bridge guardrail observed and + * rewrote video parts on this request, so the persisted client snapshot had + * its transcript cues structurally redacted (videoBridgeObserved in + * chatCore.ts). Written to the `call_logs.video_content_removed` marker so + * `resolvePreviousResponseState` refuses to rehydrate this row as continuation + * history. Omitted/false for every non-video request. + */ + videoContentRemoved?: boolean; }; function toConnectionId(value: unknown): string | null { @@ -368,6 +377,7 @@ export function persistAttemptLogs(args: PersistAttemptLogsArgs, ctx: PersistAtt modelPinned, sessionTag, videoBridgeLogRedaction, + videoContentRemoved, } = ctx; const initialConnectionId = toConnectionId(connectionId); const finalConnectionId = toConnectionId(credentials?.connectionId) || initialConnectionId; @@ -499,6 +509,7 @@ export function persistAttemptLogs(args: PersistAttemptLogsArgs, ctx: PersistAtt modelPinned: modelPinned || false, sessionTag: sessionTag || null, responseId: extractResponsesId(sourceFormat, clientResponse), + videoContentRemoved: videoContentRemoved || false, }).catch(() => {}); // Emit the terminal request-lifecycle event to the live dashboard bus. `request.started` diff --git a/src/lib/db/migrations/173_call_logs_video_content_removed.sql b/src/lib/db/migrations/173_call_logs_video_content_removed.sql new file mode 100644 index 0000000000..f6c244e323 --- /dev/null +++ b/src/lib/db/migrations/173_call_logs_video_content_removed.sql @@ -0,0 +1,16 @@ +-- 173: mark call-log rows whose persisted client-request snapshot had its +-- video transcript content structurally redacted (#12150 P2 surface 2). +-- +-- Set to 1 by the call-log write path when the video-bridge guardrail observed +-- and rewrote video parts on this request (see videoBridgeObserved in +-- open-sse/handlers/chatCore.ts). resolvePreviousResponseState +-- (src/lib/db/responsesContinuationStore.ts) refuses to rehydrate a row so +-- marked: the stored snapshot carries [redacted-video-transcript] placeholders +-- in place of the client's real cues, so reconstructing a continuation off it +-- would forward the placeholder text upstream as if it were real history. +-- Failing closed makes the client resend full history instead, exactly like a +-- real previous_response_not_found. +-- +-- Default 0 (NOT NULL): every existing and non-video row is "nothing removed". + +ALTER TABLE call_logs ADD COLUMN video_content_removed INTEGER NOT NULL DEFAULT 0; diff --git a/src/lib/db/responsesContinuationStore.ts b/src/lib/db/responsesContinuationStore.ts index dce175dc92..90d96e8f6b 100644 --- a/src/lib/db/responsesContinuationStore.ts +++ b/src/lib/db/responsesContinuationStore.ts @@ -66,17 +66,27 @@ export function resolvePreviousResponseState( const db = getDbInstance(); const row = db .prepare( - `SELECT artifact_relpath, api_key_id FROM call_logs + `SELECT artifact_relpath, api_key_id, video_content_removed FROM call_logs WHERE response_id = ? AND detail_state = 'ready' ORDER BY timestamp DESC LIMIT 1` ) - .get(responseId) as { artifact_relpath: string | null; api_key_id: string | null } | undefined; + .get(responseId) as + | { artifact_relpath: string | null; api_key_id: string | null; video_content_removed: number } + | undefined; if (!row || !row.artifact_relpath) return null; // Tenant isolation: a response id is only ever handed back to the API key // that created it. A stored row with no api_key_id at all (no-log/legacy) // can never be resolved by any key -- fail closed rather than guess. if (!apiKeyId || row.api_key_id !== apiKeyId) return null; + // #12150 P2 surface 2: the persisted clientRawRequest snapshot on this row had + // its video transcript cues structurally redacted to [redacted-video-transcript] + // before storage (videoBridgeSnapshotRedaction, marker written by the call-log + // path). The stored input therefore no longer carries the client's real cue + // text -- reconstructing a continuation off it would forward the placeholder + // upstream as if it were genuine history. Fail closed so the client resends + // full history, exactly like a real previous_response_not_found. + if (row.video_content_removed === 1) return null; const { artifact, state } = readCallArtifact(row.artifact_relpath); if (state !== "ready" || !artifact?.pipeline) return null; diff --git a/src/lib/usage/callLogs.ts b/src/lib/usage/callLogs.ts index 5f0e3a03fc..3e1ed314de 100644 --- a/src/lib/usage/callLogs.ts +++ b/src/lib/usage/callLogs.ts @@ -522,6 +522,11 @@ async function saveCallLogOperation(entry: any): Promise { // this row's artifact for OmniRoute-native continuation. See // src/lib/db/responsesContinuationStore.ts. responseId: typeof entry.responseId === "string" ? entry.responseId : null, + // #12150 P2 surface 2: 1 when this request's persisted client snapshot had + // its video transcript cues structurally redacted, so + // resolvePreviousResponseState refuses to rehydrate it as continuation + // history. See src/lib/db/responsesContinuationStore.ts. + videoContentRemoved: entry.videoContentRemoved ? 1 : 0, }; const requestSummary = noLogEnabled @@ -570,7 +575,8 @@ async function saveCallLogOperation(entry: any): Promise { combo_name, combo_step_id, combo_execution_key, error_summary, detail_state, artifact_relpath, artifact_size_bytes, artifact_sha256, has_request_body, has_response_body, has_pipeline_details, request_summary, - correlation_id, model_pinned, session_tag, response_id, error_type + correlation_id, model_pinned, session_tag, response_id, error_type, + video_content_removed ) VALUES ( @id, @timestamp, @method, @path, @status, @model, @requestedModel, @provider, @@ -581,7 +587,8 @@ async function saveCallLogOperation(entry: any): Promise { @comboName, @comboStepId, @comboExecutionKey, @errorSummary, @detailState, @artifactRelPath, @artifactSizeBytes, @artifactSha256, @hasRequestBody, @hasResponseBody, @hasPipelineDetails, @requestSummary, - @correlationId, @modelPinned, @sessionTag, @responseId, @errorType + @correlationId, @modelPinned, @sessionTag, @responseId, @errorType, + @videoContentRemoved ) ` ).run({ diff --git a/tests/unit/responses-continuation-store.test.ts b/tests/unit/responses-continuation-store.test.ts index 0b0bce17c3..6d8cb66c5f 100644 --- a/tests/unit/responses-continuation-store.test.ts +++ b/tests/unit/responses-continuation-store.test.ts @@ -27,13 +27,15 @@ function insertCallLog(row: { apiKeyId: string | null; detailState: string; artifactRelPath: string | null; + videoContentRemoved?: 0 | 1; }) { const db = core.getDbInstance(); db.prepare( `INSERT INTO call_logs (id, timestamp, method, path, status, model, provider, account, duration, - tokens_in, tokens_out, api_key_id, detail_state, artifact_relpath, response_id) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)` + tokens_in, tokens_out, api_key_id, detail_state, artifact_relpath, response_id, + video_content_removed) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)` ).run( row.id, new Date().toISOString(), @@ -49,7 +51,8 @@ function insertCallLog(row: { row.apiKeyId, row.detailState, row.artifactRelPath, - row.responseId + row.responseId, + row.videoContentRemoved ?? 0 ); } @@ -398,6 +401,65 @@ test("resolvePreviousResponseState fails closed on an empty output array even wi assert.equal(store.resolvePreviousResponseState("resp_gen-empty-output", "key-1"), null); }); +test("resolvePreviousResponseState fails closed when the row had video content removed (#12150 P2)", () => { + // #12150 P2 surface 2: the persisted clientRawRequest snapshot had its video + // transcript cues structurally redacted to [redacted-video-transcript] before + // storage (videoBridgeSnapshotRedaction). The stored input therefore no longer + // carries the client's real cue text -- reconstructing a continuation off it + // would forward the placeholder upstream as if it were genuine history. When the + // owning row is marked video_content_removed=1 this must fail closed (return + // null) so the client resends full history, exactly like previous_response_not_found, + // even though the artifact itself is otherwise a perfectly resolvable 'ready' row. + insertCallLog({ + id: "log-video-removed", + responseId: "resp_video_removed", + apiKeyId: "key-1", + detailState: "ready", + artifactRelPath: "2026-01-01/log-video-removed.json", + videoContentRemoved: 1, + }); + writeArtifact("2026-01-01/log-video-removed.json", { + clientRawRequest: { + body: { + input: [{ type: "message", role: "user", content: "[redacted-video-transcript]" }], + }, + }, + providerRequest: { body: { input: [] } }, + clientResponse: { + id: "resp_video_removed", + output: [{ type: "message", role: "assistant", content: "hello" }], + }, + }); + + assert.equal(store.resolvePreviousResponseState("resp_video_removed", "key-1"), null); +}); + +test("resolvePreviousResponseState still resolves a normal row (video_content_removed=0)", () => { + // Guard the fail-closed above does not over-fire: an ordinary row (the default + // 0) resolves exactly as before. + insertCallLog({ + id: "log-video-notremoved", + responseId: "resp_video_notremoved", + apiKeyId: "key-1", + detailState: "ready", + artifactRelPath: "2026-01-01/log-video-notremoved.json", + videoContentRemoved: 0, + }); + writeArtifact("2026-01-01/log-video-notremoved.json", { + clientRawRequest: { body: { input: [{ type: "message", role: "user", content: "hi" }] } }, + providerRequest: { body: { input: [{ type: "message", role: "user", content: "hi" }] } }, + clientResponse: { + id: "resp_video_notremoved", + output: [{ type: "message", role: "assistant", content: "hello" }], + }, + }); + + assert.deepEqual(store.resolvePreviousResponseState("resp_video_notremoved", "key-1"), { + input: [{ type: "message", role: "user", content: "hi" }], + output: [{ type: "message", role: "assistant", content: "hello" }], + }); +}); + test("resolvePreviousResponseState returns null when detail logging was never captured for this row", () => { insertCallLog({ id: "log-5", diff --git a/tests/unit/save-call-log-persistence.test.ts b/tests/unit/save-call-log-persistence.test.ts index 8d59ad18d6..7eb0cef2e5 100644 --- a/tests/unit/save-call-log-persistence.test.ts +++ b/tests/unit/save-call-log-persistence.test.ts @@ -152,6 +152,73 @@ test("saveCallLog persists modelPinned=false as 0", async () => { db.prepare("DELETE FROM call_logs WHERE id = ?").run(testId); }); +test("call_logs table has video_content_removed column", () => { + const db = getDbInstance(); + const columns = db.prepare("PRAGMA table_info(call_logs)").all() as { name: string }[]; + const colNames = columns.map((c) => c.name); + assert.ok( + colNames.includes("video_content_removed"), + "call_logs should have video_content_removed column" + ); +}); + +test("saveCallLog persists videoContentRemoved=true as 1 (#12150 P2)", async () => { + const db = getDbInstance(); + const testId = `test-videoremoved-${Date.now()}`; + + await saveCallLog({ + id: testId, + method: "POST", + path: "/v1/responses", + status: 200, + model: "video-model", + provider: "test-provider", + duration: 500, + tokens: { in: 10, out: 5 }, + videoContentRemoved: true, + }); + + const row = db + .prepare("SELECT id, video_content_removed FROM call_logs WHERE id = ?") + .get(testId) as Record; + assert.ok(row, "row should exist"); + assert.equal( + row.video_content_removed, + 1, + "video_content_removed should be 1 when videoContentRemoved=true" + ); + + db.prepare("DELETE FROM call_logs WHERE id = ?").run(testId); +}); + +test("saveCallLog defaults video_content_removed to 0 when absent (#12150 P2)", async () => { + const db = getDbInstance(); + const testId = `test-novideoremoved-${Date.now()}`; + + await saveCallLog({ + id: testId, + method: "POST", + path: "/v1/chat/completions", + status: 200, + model: "normal-model", + provider: "test-provider", + duration: 500, + tokens: { in: 10, out: 5 }, + }); + + const row = db + .prepare("SELECT id, video_content_removed FROM call_logs WHERE id = ?") + .get(testId) as Record; + assert.ok(row, "row should exist"); + assert.equal( + row.video_content_removed, + 0, + "video_content_removed should default to 0 when not provided" + ); + + db.prepare("DELETE FROM call_logs WHERE id = ?").run(testId); +}); + test("getCallLogs returns modelPinned boolean", async () => { const db = getDbInstance(); const testId = `test-pinned-roundtrip-${Date.now()}`; diff --git a/tests/unit/video-bridge-log-redaction.test.ts b/tests/unit/video-bridge-log-redaction.test.ts index 268102d246..d828cb31a4 100644 --- a/tests/unit/video-bridge-log-redaction.test.ts +++ b/tests/unit/video-bridge-log-redaction.test.ts @@ -149,6 +149,41 @@ test("persisted requestBody carries the placeholder and never the raw transcript ); }); +test("#12150 P2 surface 2: persistAttemptLogs marks the call_logs row video_content_removed=1 when ctx.videoContentRemoved is true", async () => { + // The continuation fail-closed (resolvePreviousResponseState) depends on this + // marker being written for any request whose stored client snapshot had its + // video transcript redacted. This proves the ctx.videoContentRemoved signal + // reaches the persisted row; the row is the exact thing the continuation store + // reads back. + const id = "video-marker-1"; + persistAttemptLogs( + { status: 200, tokens: { input: 1, output: 2 } }, + baseCtx({ pendingRequestId: id, videoContentRemoved: true }) + ); + const row = await pollForCallLog(id); + assert.ok(row, "call log row should be persisted"); + const marker = coreDb + .getDbInstance() + .prepare("SELECT video_content_removed FROM call_logs WHERE id = ?") + .get(id) as { video_content_removed: number }; + assert.equal(marker.video_content_removed, 1); +}); + +test("#12150 P2 surface 2: the marker defaults to 0 for an ordinary (non-video) request", async () => { + const id = "video-marker-control-1"; + persistAttemptLogs( + { status: 200, tokens: { input: 1, output: 2 } }, + baseCtx({ pendingRequestId: id }) + ); + const row = await pollForCallLog(id); + assert.ok(row); + const marker = coreDb + .getDbInstance() + .prepare("SELECT video_content_removed FROM call_logs WHERE id = ?") + .get(id) as { video_content_removed: number }; + assert.equal(marker.video_content_removed, 0); +}); + test("control: without a redaction map the persisted requestBody keeps the original text (model path untouched)", async () => { const id = "video-control-1"; persistAttemptLogs(