Repository navigation
refactor(retention): move log stripping to gateway - #3222
Merged
Merged
Conversation
Move the retention-based stripping of request/response payload fields (messages, content, reasoningContent, tools, toolChoice, toolResults, responsesApiData) from the worker into the gateway. Orgs with retentionLevel "none" no longer send those payloads through Redis at all. The gateway already resolves the organization (SWR-cached), so reading retentionLevel adds no query and the strip is a cheap object spread. The worker keeps the same strip as a safety net for logs whose retention level wasn't known at publish time. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VrcPpg8KCvom8H4FNU8cLN
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
github-actions
Bot
changed the title
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe change centralizes retention-sensitive log-field stripping, applies it before gateway queue publication and synchronous insertion, propagates organization retention settings through gateway endpoints, and simplifies worker retry persistence. ChangesLog retention enforcement
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant GatewayHandler
participant GatewayLogs
participant LogQueue
participant Worker
participant Database
GatewayHandler->>GatewayLogs: pass organization retentionLevel
GatewayLogs->>GatewayLogs: strip sensitive fields when retention is none
GatewayLogs->>LogQueue: publish processed log payload
LogQueue->>Worker: deliver queued log
Worker->>Database: insert queued log data on retry
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/gateway/src/lib/logs-retention.spec.ts (1)
9-15: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAvoid the dynamic mock import.
Use the string module specifier form of
vi.mock;import("@llmgateway/cache")violates the project’s no-dynamic-import rule.Proposed fix
-vi.mock(import("`@llmgateway/cache`"), async (importOriginal) => { +vi.mock("`@llmgateway/cache`", async (importOriginal) => {As per coding guidelines, “Always use top-level
import; never userequireor dynamic imports.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/gateway/src/lib/logs-retention.spec.ts` around lines 9 - 15, Replace the dynamic import passed to vi.mock in the cache mock setup with the string module specifier form, while preserving the existing importOriginal behavior and publishToQueue override.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/gateway/src/lib/logs.ts`:
- Around line 296-298: Make retention handling fail closed: in
apps/gateway/src/lib/logs.ts lines 296-298 and apps/worker/src/worker.ts lines
1765-1770, strip sensitive log fields unless retentionLevel explicitly equals
"retain", including omitted, null, or unresolved values. In
apps/gateway/src/lib/logs-retention.spec.ts lines 80-85, update the
unspecified-retention test to expect stripped fields.
In `@packages/db/src/log-retention.ts`:
- Around line 9-17: Extend RETENTION_SENSITIVE_LOG_FIELDS to include rawRequest,
rawResponse, upstreamRequest, and upstreamResponse so the retention
transformation strips these payload columns along with the existing sensitive
fields.
---
Nitpick comments:
In `@apps/gateway/src/lib/logs-retention.spec.ts`:
- Around line 9-15: Replace the dynamic import passed to vi.mock in the cache
mock setup with the string module specifier form, while preserving the existing
importOriginal behavior and publishToQueue override.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ba1fdb1b-23ea-4438-aa8d-9d50a2aafd4f
📒 Files selected for processing (6)
apps/gateway/src/chat/chat.tsapps/gateway/src/lib/logs-retention.spec.tsapps/gateway/src/lib/logs.tsapps/worker/src/worker.tspackages/db/src/index.tspackages/db/src/log-retention.ts
Comment on lines
+296
to
+298
| if (options?.retentionLevel === "none") { | ||
| logData = stripRetentionSensitiveLogFields(logData); | ||
| } |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Make unresolved retention fail closed. Omitted, null, or unresolved organization retention currently preserves payloads, contradicting the required default of "none".
apps/gateway/src/lib/logs.ts#L296-L298: strip unlessretentionLevel === "retain".apps/gateway/src/lib/logs-retention.spec.ts#L80-L85: change the unspecified-retention test to expect stripped fields.apps/worker/src/worker.ts#L1765-L1770: strip unless the lookup explicitly returns"retain".
Proposed fix
- if (options?.retentionLevel === "none") {
+ if (options?.retentionLevel !== "retain") {
logData = stripRetentionSensitiveLogFields(logData);
}
- if (retentionByOrg.get(data.organizationId) === "none") {
+ if (retentionByOrg.get(data.organizationId) !== "retain") {
return stripRetentionSensitiveLogFields(data);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (options?.retentionLevel === "none") { | |
| logData = stripRetentionSensitiveLogFields(logData); | |
| } | |
| if (options?.retentionLevel !== "retain") { | |
| logData = stripRetentionSensitiveLogFields(logData); | |
| } |
📍 Affects 3 files
apps/gateway/src/lib/logs.ts#L296-L298(this comment)apps/gateway/src/lib/logs-retention.spec.ts#L80-L85apps/worker/src/worker.ts#L1765-L1770
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/gateway/src/lib/logs.ts` around lines 296 - 298, Make retention handling
fail closed: in apps/gateway/src/lib/logs.ts lines 296-298 and
apps/worker/src/worker.ts lines 1765-1770, strip sensitive log fields unless
retentionLevel explicitly equals "retain", including omitted, null, or
unresolved values. In apps/gateway/src/lib/logs-retention.spec.ts lines 80-85,
update the unspecified-retention test to expect stripped fields.
Comment on lines
+9
to
+17
| export const RETENTION_SENSITIVE_LOG_FIELDS = [ | ||
| "messages", | ||
| "content", | ||
| "reasoningContent", | ||
| "tools", | ||
| "toolChoice", | ||
| "toolResults", | ||
| "responsesApiData", | ||
| ] as const; |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Strip the raw request/response payload columns too.
rawRequest, rawResponse, upstreamRequest, and upstreamResponse are spread through unchanged. Gateway publishing and worker persistence therefore retain these payloads for "none" organizations, regressing the previous metadata-only worker transformation.
Proposed fix
export const RETENTION_SENSITIVE_LOG_FIELDS = [
"messages",
"content",
"reasoningContent",
"tools",
"toolChoice",
"toolResults",
"responsesApiData",
+ "rawRequest",
+ "rawResponse",
+ "upstreamRequest",
+ "upstreamResponse",
] as const;
return {
...logData,
messages: null,
content: null,
reasoningContent: null,
tools: null,
toolChoice: null,
toolResults: null,
responsesApiData: null,
+ rawRequest: null,
+ rawResponse: null,
+ upstreamRequest: null,
+ upstreamResponse: null,
};Also applies to: 28-37
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/db/src/log-retention.ts` around lines 9 - 17, Extend
RETENTION_SENSITIVE_LOG_FIELDS to include rawRequest, rawResponse,
upstreamRequest, and upstreamResponse so the retention transformation strips
these payload columns along with the existing sensitive fields.
Remove the worker's per-batch org retention lookup and second payload strip. The gateway already strips request/response fields before publishing to the queue for non-retaining orgs, so the worker inserts queued rows as-is — no duplicate stripping and no Postgres round-trip per log batch. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VrcPpg8KCvom8H4FNU8cLN
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/worker/src/worker.ts`:
- Around line 1700-1710: The worker’s log insertion path must not assume all
queued Redis messages were produced by the new gateway. Update the flow around
logData and db.insert(log) to preserve a worker-side retention fallback for
legacy or pre-existing messages, or consume a versioned/enveloped record
carrying retention status while safely handling older entries; ensure
organizations with retentionLevel "none" cannot have sensitive payload fields
persisted during rolling deployment.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8b440885-9ee7-4fb4-963d-1ce3f39c9143
📒 Files selected for processing (2)
apps/worker/src/worker.tspackages/db/src/log-retention.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/db/src/log-retention.ts
Comment on lines
+1700
to
+1710
| // The gateway decides what to persist: it strips request/response payload | ||
| // fields before publishing for orgs that don't retain data, so the worker | ||
| // inserts the queued rows as-is with no per-batch org retention lookup. | ||
| const logData = message.map((i) => JSON.parse(i) as LogInsertData); | ||
| const organizationIds = Array.from( | ||
| new Set(logData.map((data) => data.organizationId)), | ||
| ); | ||
| const selectStart = Date.now(); | ||
| const retentionByOrg = | ||
| organizationIds.length > 0 | ||
| ? await getOrganizationRetentionLevels(organizationIds) | ||
| : new Map<string, "retain" | "none">(); | ||
| const selectMs = Date.now() - selectStart; | ||
|
|
||
| const processedLogData: ( | ||
| | LogInsertData | ||
| | Omit<LogInsertData, "messages" | "content"> | ||
| )[] = logData.map((data) => { | ||
| if (retentionByOrg.get(data.organizationId) === "none") { | ||
| const { | ||
| messages: _messages, | ||
| content: _content, | ||
| reasoningContent: _reasoningContent, | ||
| tools: _tools, | ||
| toolChoice: _toolChoice, | ||
| toolResults: _toolResults, | ||
| responsesApiData: _responsesApiData, | ||
| ...metadataOnly | ||
| } = data; | ||
| return metadataOnly; | ||
| } | ||
|
|
||
| return data; | ||
| }); | ||
|
|
||
| // Insert logs with retry logic | ||
| let lastError: Error | undefined; | ||
| for (let attempt = 0; attempt <= MAX_RETRIES; attempt++) { | ||
| try { | ||
| // Type assertion is safe here as both LogInsertData and its subset are compatible with the log insert schema | ||
| const insertStart = Date.now(); | ||
| await db.insert(log).values(processedLogData as LogInsertData[]); | ||
| await db.insert(log).values(logData); |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Do not bypass retention for pre-existing queued messages.
This assumes every Redis message was published by the new gateway. During a rolling deployment—or for a queue backlog created before this change—an org with retentionLevel: "none" can have unstripped prompts, completions, reasoning, or tool payloads inserted because the worker no longer has retention context or a safety net. Keep the worker fallback through migration, or version/envelope queued records with retention status and drain old queue entries before rollout.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/worker/src/worker.ts` around lines 1700 - 1710, The worker’s log
insertion path must not assume all queued Redis messages were produced by the
new gateway. Update the flow around logData and db.insert(log) to preserve a
worker-side retention fallback for legacy or pre-existing messages, or consume a
versioned/enveloped record carrying retention status while safely handling older
entries; ensure organizations with retentionLevel "none" cannot have sensitive
payload fields persisted during rolling deployment.
Extend the gateway's retention-based payload stripping to every logging path — embeddings, speech, OCR, moderations, images, and video jobs — via the same local insertLog wrapper used by chat completions. The Responses API stays gated to retaining orgs. Video job logs (inserted directly by the worker) reuse the shared strip helper. Add end-to-end tests asserting no request/response payload is persisted for a non-retaining org on each endpoint, plus unit tests for the shared stripRetentionSensitiveLogFields helper and the insertLog retention option. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VrcPpg8KCvom8H4FNU8cLN
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/db/src/log-retention.spec.ts (1)
51-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winEnsure every documented field is exercised by the fixture.
This loop only checks keys already present in
input. If a new field is added toRETENTION_SENSITIVE_LOG_FIELDSbut omitted frombaseLogData(), the test can still pass without verifying it. Add an assertion that every documented field exists in the fixture, or construct the fixture from the exported field list.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/db/src/log-retention.spec.ts` around lines 51 - 65, Update the test around stripRetentionSensitiveLogFields to verify every entry in RETENTION_SENSITIVE_LOG_FIELDS is present in the baseLogData fixture before checking stripped values. Ensure omitted documented fields fail the test, while preserving the existing assertions that sensitive fields become null and all other input fields remain unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/gateway/src/api.spec.ts`:
- Around line 3108-3111: Remove the unconditional log.content null assertion
from this non-retaining organization test, leaving the log.messages assertion as
the check that prompt data was stripped. Keep the test focused on behavior
actually exercised by logImageClientError.
- Around line 960-961: Make retention state explicit and isolated across the
affected tests: in apps/gateway/src/api.spec.ts lines 960-961, set the seeded
organization’s retentionLevel to "retain" within the test instead of relying on
seed state; in apps/gateway/src/ocr/ocr.spec.ts lines 17-21,
apps/gateway/src/speech/speech.spec.ts lines 86-89, and
apps/gateway/src/videos/videos.spec.ts lines 1229-1232, restore the shared
organization’s original retention level after each test assertion so later tests
are unaffected.
In `@apps/worker/src/services/video-jobs.ts`:
- Around line 1901-1911: The retention-scrubbing path around
stripRetentionSensitiveLogFields must also remove rawRequest, rawResponse,
upstreamRequest, and upstreamResponse before tables.log insertion when
retentionLevel is not "retain". Update the shared helper to null or omit these
payload fields consistently with its existing behavior, and extend its tests to
verify they are excluded while retained organizations still persist the original
logValues.
---
Nitpick comments:
In `@packages/db/src/log-retention.spec.ts`:
- Around line 51-65: Update the test around stripRetentionSensitiveLogFields to
verify every entry in RETENTION_SENSITIVE_LOG_FIELDS is present in the
baseLogData fixture before checking stripped values. Ensure omitted documented
fields fail the test, while preserving the existing assertions that sensitive
fields become null and all other input fields remain unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b8061e26-3b74-4b73-8855-a995abd3a90d
📒 Files selected for processing (13)
apps/gateway/src/api.spec.tsapps/gateway/src/embeddings/embeddings.spec.tsapps/gateway/src/embeddings/embeddings.tsapps/gateway/src/images/images.tsapps/gateway/src/lib/logs-retention.spec.tsapps/gateway/src/moderations/moderations.tsapps/gateway/src/ocr/ocr.spec.tsapps/gateway/src/ocr/ocr.tsapps/gateway/src/speech/speech.spec.tsapps/gateway/src/speech/speech.tsapps/gateway/src/videos/videos.spec.tsapps/worker/src/services/video-jobs.tspackages/db/src/log-retention.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/gateway/src/lib/logs-retention.spec.ts
Comment on lines
+960
to
+961
| test("/v1/chat/completions retains log payload when retention is enabled", async () => { | ||
| // The seeded org defaults to retentionLevel: "retain". |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Retention tests mutate the shared seeded org in place and never restore it. Every new retention test flips organization.retentionLevel to "none" on the shared org-id row, so each test's outcome depends on the harness reseeding that row between tests — and the one test that needs "retain" is ordered after a test that sets "none". Prefer setting the level explicitly per test (and restoring it), or seeding a separate non-retaining org.
apps/gateway/src/api.spec.ts#L960-L961: setretentionLevel: "retain"explicitly instead of relying on seed state;packages/db/src/schema.tsdefaults this column to"none"and the preceding test at Line 907 already set it to"none".apps/gateway/src/ocr/ocr.spec.ts#L17-L21: restore the level after the test (or move it last) — it currently runs first and leaks"none"into the remaining OCR tests.apps/gateway/src/speech/speech.spec.ts#L86-L89: restore the level after the assertion so later speech tests aren't affected.apps/gateway/src/videos/videos.spec.ts#L1229-L1232: restore the level after the assertion so later video tests aren't affected.
📍 Affects 4 files
apps/gateway/src/api.spec.ts#L960-L961(this comment)apps/gateway/src/ocr/ocr.spec.ts#L17-L21apps/gateway/src/speech/speech.spec.ts#L86-L89apps/gateway/src/videos/videos.spec.ts#L1229-L1232
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/gateway/src/api.spec.ts` around lines 960 - 961, Make retention state
explicit and isolated across the affected tests: in apps/gateway/src/api.spec.ts
lines 960-961, set the seeded organization’s retentionLevel to "retain" within
the test instead of relying on seed state; in apps/gateway/src/ocr/ocr.spec.ts
lines 17-21, apps/gateway/src/speech/speech.spec.ts lines 86-89, and
apps/gateway/src/videos/videos.spec.ts lines 1229-1232, restore the shared
organization’s original retention level after each test assertion so later tests
are unaffected.
Comment on lines
+3108
to
+3111
| // ...but the prompt is never persisted for a non-retaining org. | ||
| expect(log.messages).toBeNull(); | ||
| expect(log.content).toBeNull(); | ||
| }); |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
content is unconditionally null on this path, so that assertion proves nothing.
logImageClientError hardcodes content: null (Line 519), so only the messages assertion actually exercises the stripping. Asserting a field the producer never populates can create false confidence that retention is enforced here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/gateway/src/api.spec.ts` around lines 3108 - 3111, Remove the
unconditional log.content null assertion from this non-retaining organization
test, leaving the log.messages assertion as the check that prompt data was
stripped. Keep the test focused on behavior actually exercised by
logImageClientError.
Comment on lines
+1901
to
+1911
| }; | ||
|
|
||
| // Strip request/response payload fields for orgs that don't retain data, | ||
| // keeping the video log consistent with every other endpoint's policy. | ||
| await tx | ||
| .insert(tables.log) | ||
| .values( | ||
| organization?.retentionLevel === "retain" | ||
| ? logValues | ||
| : stripRetentionSensitiveLogFields(logValues), | ||
| ); |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Extend retention scrubbing to raw and upstream payloads.
logValues still populates rawRequest, rawResponse, upstreamRequest, and upstreamResponse, but stripRetentionSensitiveLogFields only nulls the explicitly listed fields. For organizations whose retention level is not "retain", these request/response payloads are therefore still persisted and may contain prompts or provider output. Extend the shared helper and its tests, or otherwise remove these fields before insertion.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/worker/src/services/video-jobs.ts` around lines 1901 - 1911, The
retention-scrubbing path around stripRetentionSensitiveLogFields must also
remove rawRequest, rawResponse, upstreamRequest, and upstreamResponse before
tables.log insertion when retentionLevel is not "retain". Update the shared
helper to null or omit these payload fields consistently with its existing
behavior, and extend its tests to verify they are excluded while retained
organizations still persist the original logValues.
Drop the unused options plumbing and Parameters<> indirection from the embeddings/speech/ocr/moderations insertLog wrappers — none of their call sites pass a second argument. The wrapper still exists to guarantee every log the handler writes carries the org retention level (so no call site can forget), just expressed with the plain LogInsertData type. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VrcPpg8KCvom8H4FNU8cLN
Drop the per-handler insertLog wrapper in embeddings/speech/ocr/
moderations and pass { retentionLevel } explicitly at each call site
instead. Same behavior — payloads are still stripped before publishing
for non-retaining orgs — with no import aliasing or local shadowing.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VrcPpg8KCvom8H4FNU8cLN
steebchen
changed the title
Strip request/response payload fields in insertLog unless the org is explicitly known to retain data. Omitted, null, or unresolved retention levels now default to stripping instead of persisting the payload — the worker no longer runs a fallback strip, so the gateway is the last chance to withhold payloads for a non-retaining (or unknown) org. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VrcPpg8KCvom8H4FNU8cLN
Document that createGatewayApiTestHarness re-seeds all test data (including the org's retentionLevel) in beforeEach, so tests may mutate shared org/project state without restoring it — isolation is guaranteed by the re-seed, and manual restore is redundant. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VrcPpg8KCvom8H4FNU8cLN
steebchen
enabled auto-merge
July 26, 2026 19:01
…ion-log-filter-h5oew1 # Conflicts: # apps/gateway/src/images/images.ts
steebchen
added this pull request to the merge queue
Jul 26, 2026
steebchen
added a commit
that referenced
this pull request
Jul 26, 2026
…ng-preservation-fzs9rr Resolves two conflicts with #3222 (retention log stripping moved from the worker to the gateway): - apps/worker/src/worker.ts: took main's side, dropping this branch's worker-side strip along with the now-unused org retention cache. - packages/db/src/log-retention.ts: this branch's worker strip also cleared the x-debug payloads (rawRequest/rawResponse/upstreamRequest/ upstreamResponse), which main's canonical helper did not cover, so taking main's side alone would have silently dropped that fix. Ported those four fields into the helper instead, which now applies them across every gateway surface rather than only the chat path. - apps/gateway/src/responses/responses.ts: formatting-only conflict from the prettier 3.6->3.9 bump; kept this branch's incomplete_details and reasoning fields. Co-Authored-By: Claude <noreply@anthropic.com>
This was referenced Jul 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Move the stripping of sensitive log fields (prompts, completions, reasoning, tools, etc.) for organizations with
retentionLevel: "none"out of the worker and into the gateway. The stripping already happened before this change — but only in the worker, after the full payload had been serialized and pushed through Redis. Doing it in the gateway means large request/response payloads never transit the log queue at all for non-retaining orgs, and the worker no longer needs a per-batch org retention lookup.Key Changes
packages/db/src/log-retention.ts: Defines theRETENTION_SENSITIVE_LOG_FIELDSconstant andstripRetentionSensitiveLogFields()helper (single source of truth for which payload fields get cleared)apps/gateway/src/lib/logs.ts):insertLog()accepts aretentionLeveloption and strips sensitive fields before publishing to the log queue (and before any synchronous insert) whenretentionLevel === "none"chat,embeddings,speech,ocr,moderations,images): capture the organization'sretentionLevelfrom the already-cached org lookup and pass it to everyinsertLog()call, including early-rejection and streaming pathsapps/worker/src/worker.ts): removed the per-batch org retention lookup and second strip — the gateway is now the sole decider of what gets persisted, so the worker inserts queued rows as-isapps/worker/src/services/video-jobs.ts): video job logs (inserted directly by the worker, not via the queue) reuse the shared strip helper for consistencystripRetentionSensitiveLogFields()and theinsertLog()retention option, plus end-to-end tests asserting no request/response payload is persisted for a non-retaining org across chat, embeddings, speech, OCR, moderations, images, and videoWhy
The gateway already resolves the organization (SWR-cached), so reading
retentionLevelthere adds no query — the strip is a cheap object spread. Moving the decision upstream keeps the insert-log hot path free of any penalty while ensuring privacy-conscious customers' payloads never reach Redis or Postgres in the first place.https://claude.ai/code/session_01VrcPpg8KCvom8H4FNU8cLN