fix: support Claude Code metadata user_id JSON format#939
Conversation
📝 WalkthroughWalkthrough引入集中化的 Claude 元数据 user_id 处理模块,支持 legacy 与 JSON 两种格式;在代理转发与会话守护处改为基于上下文注入 metadata.user_id,并新增/更新大量单元测试覆盖相关逻辑与边界情况。 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the proxy's compatibility with Claude Code clients by introducing robust support for evolving Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request is a solid improvement, adding support for the new JSON-based metadata.user_id format from Claude Code clients while maintaining backward compatibility. The refactoring to centralize user_id handling into the new metadata-user-id.ts module is well-executed and enhances maintainability. The change to populate user_id before session extraction is a key fix for session consistency. The new regression tests are comprehensive and cover the new functionality well. I have a couple of suggestions in src/lib/claude-code/metadata-user-id.ts to improve consistency in handling session IDs by ensuring they are always trimmed.
| const parsedObj = parsed as Record<string, unknown>; | ||
| const sessionId = | ||
| typeof parsedObj.session_id === "string" && parsedObj.session_id.trim().length > 0 | ||
| ? parsedObj.session_id |
There was a problem hiding this comment.
The current logic correctly validates that the session_id is not just whitespace, but it returns the original untrimmed string. This could lead to issues downstream if other parts of the system expect a trimmed session ID. It's better to return the trimmed version of the session ID for consistency.
| ? parsedObj.session_id | |
| ? parsedObj.session_id.trim() |
| const [, deviceId, sessionId] = legacyMatch; | ||
| if (!sessionId) { | ||
| return emptyParseResult(); | ||
| } | ||
|
|
||
| return { | ||
| sessionId, | ||
| format: "legacy", | ||
| deviceId: deviceId || null, | ||
| accountUuid: null, | ||
| }; |
There was a problem hiding this comment.
In the legacy format parsing, the extracted sessionId is not trimmed or checked for being only whitespace. This is inconsistent with the JSON format parsing. To ensure consistency and prevent invalid session IDs, the extracted sessionId from the legacy format should be trimmed and validated to be non-empty.
const [, deviceId, rawSessionId] = legacyMatch;
const sessionId = rawSessionId?.trim();
if (!sessionId) {
return emptyParseResult();
}
return {
sessionId,
format: "legacy",
deviceId: deviceId || null,
accountUuid: null,
};
🧪 测试结果
总体结果: ✅ 所有测试通过 |
| if (existingMetadata?.user_id !== undefined && existingMetadata?.user_id !== null) { | ||
| return message; | ||
| } |
There was a problem hiding this comment.
Empty-string
user_id bypasses injection but also fails extraction
The guard preserves any non-null/non-undefined user_id, including "". If a client sends metadata: { user_id: "" }, the injection is skipped here, but parseClaudeMetadataUserId("") returns { sessionId: null, … }, so extractClientSessionId later returns null for that field. The session then falls back to metadata.session_id or generates a fresh ID — producing a mismatch between what is in the outbound user_id (still "") and the session assigned to the request.
Consider tightening the guard to also skip falsy strings:
| if (existingMetadata?.user_id !== undefined && existingMetadata?.user_id !== null) { | |
| return message; | |
| } | |
| if (existingMetadata?.user_id !== undefined && existingMetadata?.user_id !== null && existingMetadata?.user_id !== "") { | |
| return message; | |
| } |
Alternatively, pass the empty-string value through parseClaudeMetadataUserId and only preserve it when it yields a valid session ID.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/claude-code/metadata-user-id.ts
Line: 131-133
Comment:
**Empty-string `user_id` bypasses injection but also fails extraction**
The guard preserves any non-`null`/non-`undefined` `user_id`, including `""`. If a client sends `metadata: { user_id: "" }`, the injection is skipped here, but `parseClaudeMetadataUserId("")` returns `{ sessionId: null, … }`, so `extractClientSessionId` later returns `null` for that field. The session then falls back to `metadata.session_id` or generates a fresh ID — producing a mismatch between what is in the outbound `user_id` (still `""`) and the session assigned to the request.
Consider tightening the guard to also skip falsy strings:
```suggestion
if (existingMetadata?.user_id !== undefined && existingMetadata?.user_id !== null && existingMetadata?.user_id !== "") {
return message;
}
```
Alternatively, pass the empty-string value through `parseClaudeMetadataUserId` and only preserve it when it yields a valid session ID.
How can I resolve this? If you propose a fix, please make it concise.| ) { | ||
| const completionSessionId = | ||
| SessionManager.extractClientSessionId(requestMessage, null, session.userAgent) ?? | ||
| SessionManager.generateSessionId(); | ||
| const completedMessage = injectClaudeMetadataUserIdWithContext(requestMessage, { | ||
| keyId, | ||
| sessionId: completionSessionId, | ||
| userAgent: session.userAgent, | ||
| }); | ||
|
|
||
| if (completedMessage !== requestMessage) { | ||
| session.request.message = completedMessage; | ||
| } | ||
| } | ||
|
|
||
| const warmupMaybeIntercepted = | ||
| session.isWarmupRequest() && |
There was a problem hiding this comment.
Completion block runs for warmup requests too
The claudeMetadataCompletionEnabled block precedes the warmupMaybeIntercepted check, so warmup requests that reach this point (authenticated, Claude-format, non-Codex) also get metadata.user_id injected and session.request.message mutated. In most cases this is harmless because warmup interception happens immediately afterward, but it performs unnecessary crypto work and mutates a message that will never be forwarded.
Consider placing the completion block after the warmupMaybeIntercepted calculation and skipping it when the request will be intercepted:
if (
claudeMetadataCompletionEnabled &&
!warmupMaybeIntercepted && // ← add this guard
session.originalFormat === "claude" &&
!isCodexRequest
) {
// ... injection logic
}Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/v1/_lib/proxy/session-guard.ts
Line: 86-102
Comment:
**Completion block runs for warmup requests too**
The `claudeMetadataCompletionEnabled` block precedes the `warmupMaybeIntercepted` check, so warmup requests that reach this point (authenticated, Claude-format, non-Codex) also get `metadata.user_id` injected and `session.request.message` mutated. In most cases this is harmless because warmup interception happens immediately afterward, but it performs unnecessary crypto work and mutates a message that will never be forwarded.
Consider placing the completion block after the `warmupMaybeIntercepted` calculation and skipping it when the request will be intercepted:
```typescript
if (
claudeMetadataCompletionEnabled &&
!warmupMaybeIntercepted && // ← add this guard
session.originalFormat === "claude" &&
!isCodexRequest
) {
// ... injection logic
}
```
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/lib/claude-code/metadata-user-id.test.ts (1)
2-9: 改用@/别名导入源文件。这里在
src/内新增测试时仍然使用了相对路径,后续移动目录时会更脆弱,也不符合仓库约定。参考修改
import { describe, expect, test } from "vitest"; import { buildClaudeMetadataDeviceId, buildClaudeMetadataUserId, CLAUDE_CODE_METADATA_USER_ID_JSON_SWITCH_VERSION, injectClaudeMetadataUserIdWithContext, parseClaudeMetadataUserId, resolveClaudeMetadataUserIdFormat, -} from "./metadata-user-id"; +} from "@/lib/claude-code/metadata-user-id";As per coding guidelines "Use path alias
@/to reference files in./src/directory".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/claude-code/metadata-user-id.test.ts` around lines 2 - 9, The test file uses relative imports for the module under test; update the import statement to use the project path alias "@/..." instead of the relative path so it remains stable when files move. Locate the import in metadata-user-id.test.ts that brings in buildClaudeMetadataDeviceId, buildClaudeMetadataUserId, CLAUDE_CODE_METADATA_USER_ID_JSON_SWITCH_VERSION, injectClaudeMetadataUserIdWithContext, parseClaudeMetadataUserId, and resolveClaudeMetadataUserIdFormat and change the module specifier to the corresponding "@/..." path that points to the same metadata-user-id module.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/v1/_lib/proxy/session-guard.ts`:
- Around line 82-99: The current code calls SessionManager.generateSessionId()
and injects it back into Claude requests when extractClientSessionId() returns
null, which causes later logic (SessionManager.getOrCreateSessionId) to treat
the request as if the client provided a session and breaks reuse/sticky-routing;
change the flow so you only inject a client session id when
extractClientSessionId(...) actually returns a non-null value (i.e., use the
extracted id directly) and do NOT generate-and-write a new session id into the
request body; apply the same fix for the duplicate block referenced around lines
109-120 (functions to look for: claudeMetadataCompletionEnabled,
session.originalFormat, isCodexRequest, SessionManager.extractClientSessionId,
SessionManager.generateSessionId, injectClaudeMetadataUserIdWithContext, and
SessionManager.getOrCreateSessionId).
In `@src/lib/claude-code/metadata-user-id.ts`:
- Around line 126-133: 当前逻辑只判断 existingMetadata?.user_id 是否为 null/undefined
并直接返回,导致空字符串或仅空白的 user_id 被视为“已存在”而不会补全;在检查 message.metadata.user_id(现有变量
existingMetadata)时,把字符串类型的值进行 trim 并验证长度 > 0 才视为有效:如果 typeof
existingMetadata.user_id === "string" 则用 existingMetadata.user_id.trim().length
> 0 才返回 message,否则当为 null/undefined 或空/只空白字符串时继续后续补全流程(此改动影响到 ProxySessionGuard
和 ProxyForwarder 的补全行为)。
---
Nitpick comments:
In `@src/lib/claude-code/metadata-user-id.test.ts`:
- Around line 2-9: The test file uses relative imports for the module under
test; update the import statement to use the project path alias "@/..." instead
of the relative path so it remains stable when files move. Locate the import in
metadata-user-id.test.ts that brings in buildClaudeMetadataDeviceId,
buildClaudeMetadataUserId, CLAUDE_CODE_METADATA_USER_ID_JSON_SWITCH_VERSION,
injectClaudeMetadataUserIdWithContext, parseClaudeMetadataUserId, and
resolveClaudeMetadataUserIdFormat and change the module specifier to the
corresponding "@/..." path that points to the same metadata-user-id module.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 05a3c775-974b-478a-817d-6ec324d85962
📒 Files selected for processing (10)
src/app/v1/_lib/proxy/forwarder.tssrc/app/v1/_lib/proxy/session-guard.tssrc/lib/claude-code/metadata-user-id.test.tssrc/lib/claude-code/metadata-user-id.tssrc/lib/session-manager.tstests/unit/lib/session-manager-helpers.test.tstests/unit/proxy/client-detector.test.tstests/unit/proxy/client-guard.test.tstests/unit/proxy/metadata-injection.test.tstests/unit/proxy/session-guard-warmup-intercept.test.ts
There was a problem hiding this comment.
Code Review Summary
This PR introduces proper backward compatibility for Claude Code client metadata.user_id formats. The implementation is well-structured with centralized parsing/building logic, comprehensive test coverage, and correct error handling. The approach of pre-completing metadata before session extraction correctly solves the stated session-reuse problem.
PR Size: L
- Lines changed: 611 (548 additions, 63 deletions)
- Files changed: 10
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 1 |
Low Priority Notes
- [SIMPLIFY-READABILITY] Redundant guard check in forwarder.ts (Confidence: 60 - Below threshold)
The existingMetadata reconstruction and user_id existence check in injectClaudeMetadataUserId (lines 370-378) duplicates logic already present in injectClaudeMetadataUserIdWithContext. While redundant, this provides defense-in-depth and does not cause any runtime issues. Consider removing in a future cleanup pass if desired, but not blocking.
Validation Performed
- Verified empty-string
user_idhandling is intentional (test confirms preservation behavior) - Verified JSON parse failure catch is intentional fallback to legacy format
- Verified type safety across all new functions
- Verified test coverage for legacy format, JSON format, and edge cases
- Checked for CLAUDE.md compliance (no emojis, proper conventions)
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean (intentional fallback pattern)
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Comprehensive (117 new test lines, integration tests added)
- Code clarity - Good
Automated review by Claude AI
| if (parsed && typeof parsed === "object" && !Array.isArray(parsed)) { | ||
| const parsedObj = parsed as Record<string, unknown>; | ||
| const sessionId = | ||
| typeof parsedObj.session_id === "string" && parsedObj.session_id.trim().length > 0 |
There was a problem hiding this comment.
[MEDIUM] [LOGIC-BUG] JSON session_id 校验使用 trim,但返回值未 trim
Location: src/lib/claude-code/metadata-user-id.ts:71-84
Evidence:
const sessionId =
typeof parsedObj.session_id === "string" && parsedObj.session_id.trim().length > 0
? parsedObj.session_id
: null;Why this is a problem: 这里用 trim().length > 0 判断非空,但返回的 sessionId 仍包含可能的前后空白。若客户端发送 "session_id": " sess_123 ",会把空白带入 Session ID,导致会话复用/并发统计/Redis key 命名出现非预期差异。
Suggested fix:
const sessionId =
typeof parsedObj.session_id === "string" ? parsedObj.session_id.trim() : null;
if (sessionId) {
return {
sessionId,
format: "json",
deviceId: typeof parsedObj.device_id === "string" ? parsedObj.device_id : null,
accountUuid: typeof parsedObj.account_uuid === "string" ? parsedObj.account_uuid : null,
};
}There was a problem hiding this comment.
Code Review Summary
This PR adds a Claude Code metadata.user_id helper and updates session extraction/completion to support both legacy and JSON-string formats. The overall change is reasonably contained, but there is a parser normalization edge case that can lead to inconsistent Session IDs.
PR Size: L
- Lines changed: 611
- Files changed: 10
- Split suggestion:
- PR A: add
metadata-user-idhelper +SessionManager.extractClientSessionIdcompatibility + unit tests - PR B: wire completion into
ProxySessionGuard/forwarder + warmup regression tests - PR C: update client-detector/client-guard/metadata-injection tests
- PR A: add
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 1 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
- None
High Priority Issues (Should Fix)
- src/lib/claude-code/metadata-user-id.ts:71-84 — JSON
session_idvalidation uses.trim()but returns the untrimmed string (can propagate whitespace into Session IDs). (Confidence: 85)
Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling
- Type safety
- Documentation accuracy
- Test coverage
- Code clarity
Automated review by Codex AI
There was a problem hiding this comment.
Reviewed PR #939 and applied size/L.
Posted 1 inline review comment:
- [MEDIUM] [LOGIC-BUG]
src/lib/claude-code/metadata-user-id.ts(JSONsession_idis checked with.trim()but the returned value is not trimmed)
Submitted the PR review summary via gh pr review.
🧪 测试结果
总体结果: ✅ 所有测试通过 |
| if (extractedClaudeSessionId) { | ||
| const completedMessage = injectClaudeMetadataUserIdWithContext(requestMessage, { | ||
| keyId, | ||
| sessionId: extractedClaudeSessionId, | ||
| userAgent: session.userAgent, | ||
| }); | ||
|
|
||
| if (completedMessage !== requestMessage) { | ||
| session.request.message = completedMessage; | ||
| } |
There was a problem hiding this comment.
Pre-extraction injection can leave
user_id out of sync with the final assigned session
When a client sends metadata.session_id but no metadata.user_id, the pre-extraction injection fires here (lines 98–107) and writes user_id with extractedClaudeSessionId. After that, hasUsableClaudeMetadataUserId returns true, so the post-assignment injection block (lines 126–144) becomes a no-op.
If getOrCreateSessionId returns a different sessionId than extractedClaudeSessionId (e.g. the hint was rejected and a new session ID was allocated), the outbound message carries a user_id whose embedded session ID no longer matches session.sessionId. The forwarder-level injection also becomes a no-op for the same reason, so there is no recovery path.
The test at tests/unit/proxy/session-guard-warmup-intercept.test.ts:965 actually demonstrates this: the mock returns "session_assigned" while the user_id assertion checks for sess_legacy_seed — confirming the two values can diverge.
A safe fix is to allow the post-assignment injection to force-overwrite the user_id when the pre-extraction injection wrote a different session ID:
// After session.setSessionId(sessionId)
if (
claudeMetadataCompletionEnabled &&
!warmupMaybeIntercepted &&
session.originalFormat === "claude" &&
!isCodexRequest
) {
// Always overwrite here — we now know the canonical sessionId
const completedMessage = {
...(session.request.message as Record<string, unknown>),
metadata: {
...((session.request.message as Record<string, unknown>).metadata as Record<string, unknown> ?? {}),
user_id: buildClaudeMetadataUserId({ keyId, sessionId, userAgent: session.userAgent }),
},
};
session.request.message = completedMessage;
}Alternatively, skip the pre-extraction injection for the metadata.session_id-only case (i.e. when the client has no user_id) and let the post-assignment block handle injection exclusively; the second extraction would still find the session via metadata.session_id path B.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/v1/_lib/proxy/session-guard.ts
Line: 98-107
Comment:
**Pre-extraction injection can leave `user_id` out of sync with the final assigned session**
When a client sends `metadata.session_id` but no `metadata.user_id`, the pre-extraction injection fires here (lines 98–107) and writes `user_id` with `extractedClaudeSessionId`. After that, `hasUsableClaudeMetadataUserId` returns `true`, so the **post-assignment injection block (lines 126–144) becomes a no-op**.
If `getOrCreateSessionId` returns a *different* `sessionId` than `extractedClaudeSessionId` (e.g. the hint was rejected and a new session ID was allocated), the outbound message carries a `user_id` whose embedded session ID no longer matches `session.sessionId`. The forwarder-level injection also becomes a no-op for the same reason, so there is no recovery path.
The test at `tests/unit/proxy/session-guard-warmup-intercept.test.ts:965` actually demonstrates this: the mock returns `"session_assigned"` while the `user_id` assertion checks for `sess_legacy_seed` — confirming the two values can diverge.
A safe fix is to allow the post-assignment injection to force-overwrite the `user_id` when the pre-extraction injection wrote a different session ID:
```typescript
// After session.setSessionId(sessionId)
if (
claudeMetadataCompletionEnabled &&
!warmupMaybeIntercepted &&
session.originalFormat === "claude" &&
!isCodexRequest
) {
// Always overwrite here — we now know the canonical sessionId
const completedMessage = {
...(session.request.message as Record<string, unknown>),
metadata: {
...((session.request.message as Record<string, unknown>).metadata as Record<string, unknown> ?? {}),
user_id: buildClaudeMetadataUserId({ keyId, sessionId, userAgent: session.userAgent }),
},
};
session.request.message = completedMessage;
}
```
Alternatively, skip the pre-extraction injection for the `metadata.session_id`-only case (i.e. when the client has no `user_id`) and let the post-assignment block handle injection exclusively; the second extraction would still find the session via `metadata.session_id` path B.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/v1/_lib/proxy/session-guard.ts (1)
90-121:⚠️ Potential issue | 🟠 Major不要把提取到的客户端 session 预写回
metadata.user_id。Line 98 先把
extractedClaudeSessionId写回请求后,Line 132 的二次补全会被injectClaudeMetadataUserIdWithContext()的“已有user_id就直接返回”逻辑短路。只要getOrCreateSessionId()最终返回了不同的服务端sessionId,请求里的metadata.user_id就会继续指向旧 session,而内部复用/路由用的是新 session;forwarder 之后也会因为already_exists不再纠正它。参考修改
+ let messageForSessionExtraction = requestMessage; const extractedClaudeSessionId = claudeMetadataCompletionEnabled && !warmupMaybeIntercepted && session.originalFormat === "claude" && !isCodexRequest ? SessionManager.extractClientSessionId(requestMessage, null, session.userAgent) : null; if (extractedClaudeSessionId) { const completedMessage = injectClaudeMetadataUserIdWithContext(requestMessage, { keyId, sessionId: extractedClaudeSessionId, userAgent: session.userAgent, }); if (completedMessage !== requestMessage) { - session.request.message = completedMessage; + messageForSessionExtraction = completedMessage; } } // 1. 尝试从客户端提取 session_id(兼容 metadata.user_id / metadata.session_id) const clientSessionId = SessionManager.extractClientSessionId( - session.request.message, + messageForSessionExtraction, session.headers, session.userAgent );Also applies to: 126-144
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/v1/_lib/proxy/session-guard.ts` around lines 90 - 121, The code currently writes extractedClaudeSessionId back into session.request.message via injectClaudeMetadataUserIdWithContext before SessionManager.getOrCreateSessionId runs, which can short-circuit later metadata injection; instead stop pre-writing extractedClaudeSessionId into session.request.message (remove the injectClaudeMetadataUserIdWithContext call at that point) and only pass the extractedClaudeSessionId as an input to SessionManager.getOrCreateSessionId (keep calling SessionManager.extractClientSessionId to obtain clientSessionId and pass extractedClaudeSessionId/context through the sessionId resolution flow), then after await SessionManager.getOrCreateSessionId(keyId, messages, clientSessionId) compare the returned sessionId with the original extractedClaudeSessionId and only call injectClaudeMetadataUserIdWithContext to update session.request.message (and thus metadata.user_id) if the returned sessionId differs or if you need to align metadata.user_id with the resolved sessionId.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@src/app/v1/_lib/proxy/session-guard.ts`:
- Around line 90-121: The code currently writes extractedClaudeSessionId back
into session.request.message via injectClaudeMetadataUserIdWithContext before
SessionManager.getOrCreateSessionId runs, which can short-circuit later metadata
injection; instead stop pre-writing extractedClaudeSessionId into
session.request.message (remove the injectClaudeMetadataUserIdWithContext call
at that point) and only pass the extractedClaudeSessionId as an input to
SessionManager.getOrCreateSessionId (keep calling
SessionManager.extractClientSessionId to obtain clientSessionId and pass
extractedClaudeSessionId/context through the sessionId resolution flow), then
after await SessionManager.getOrCreateSessionId(keyId, messages,
clientSessionId) compare the returned sessionId with the original
extractedClaudeSessionId and only call injectClaudeMetadataUserIdWithContext to
update session.request.message (and thus metadata.user_id) if the returned
sessionId differs or if you need to align metadata.user_id with the resolved
sessionId.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f76605f2-66b8-4680-bf03-04b3b9e1f238
📒 Files selected for processing (6)
src/app/v1/_lib/proxy/forwarder.tssrc/app/v1/_lib/proxy/session-guard.tssrc/lib/claude-code/metadata-user-id.test.tssrc/lib/claude-code/metadata-user-id.tstests/unit/proxy/metadata-injection.test.tstests/unit/proxy/session-guard-warmup-intercept.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/claude-code/metadata-user-id.test.ts
Summary
Add support for both legacy string and JSON-string
metadata.user_idformats used by different versions of Claude Code clients. This ensures session extraction, client detection, and guard behavior work correctly across all client versions.Problem
Claude Code v2.1.78+ changed the
metadata.user_idformat from a string pattern (user_{hash}_account__session_{sessionId}) to a JSON string ({"device_id":"...","account_uuid":"","session_id":"..."}). Without this fix:Follow-up to: #729 (original metadata.user_id injection feature)
Solution
New module (
src/lib/claude-code/metadata-user-id.ts): Centralized handling for:user_idstringsuser_idin the appropriate format based on Claude Code versionPre-session extraction completion: Complete
metadata.user_idinjection before session extraction so downstream components (session reuse, routing) consume the same session IDComprehensive test coverage: Regression tests for session extraction, injection, client detection, and guard behavior across both formats
Changes
Core Changes
src/lib/claude-code/metadata-user-id.ts(+150): New module for parsing/buildingmetadata.user_idin legacy or JSON formatsrc/app/v1/_lib/proxy/session-guard.ts(+23/-1): Add metadata completion before session extractionsrc/app/v1/_lib/proxy/forwarder.ts(+9/-24): Delegate injection logic to new modulesrc/lib/session-manager.ts(+9/-16): Use new parser for extracting session ID fromuser_idTest Coverage
src/lib/claude-code/metadata-user-id.test.ts(+117): Unit tests for parsing and building both formatstests/unit/lib/session-manager-helpers.test.ts(+46): Session extraction tests for both formatstests/unit/proxy/client-detector.test.ts(+33/-7): Client detection with both format typestests/unit/proxy/client-guard.test.ts(+24/-3): Guard behavior with both format typestests/unit/proxy/metadata-injection.test.ts(+54/-12): Injection tests for version-specific formatstests/unit/proxy/session-guard-warmup-intercept.test.ts(+83): Session guard completion testsTechnical Details
metadata.user_idFormatuser_{device_id}_account__session_{session_id}{"device_id":"...","account_uuid":"","session_id":"..."}Test Plan
bun run typecheck- TypeScript type check passesbun run build- Production build succeedsbun run test- Unit tests passbun run lint- Lint check passesDescription enhanced by Claude AI
Greptile Summary
This PR adds dual-format support for
metadata.user_idin Claude Code requests, handling both the legacy string pattern (user_{hash}_account__session_{id}) used by clients older than v2.1.78 and the new JSON-string format ({"device_id":"...","session_id":"..."}) introduced in v2.1.78+. The change centralises all parsing/building logic in a newsrc/lib/claude-code/metadata-user-id.tsmodule and movesuser_idinjection earlier in the pipeline (before session extraction) so session routing consistently usesmetadata.user_idas the primary source.Key changes:
metadata-user-id.tsmodule withparseClaudeMetadataUserId,buildClaudeMetadataUserId,resolveClaudeMetadataUserIdFormat,hasUsableClaudeMetadataUserId, andinjectClaudeMetadataUserIdWithContext— all well-testedsession-guard.tsadds a pre-extraction injection step (lines 98–107) that writesuser_idbefore the mainextractClientSessionIdcall, followed by a post-assignment injection step (lines 126–144) that writes the final assigned session IDuser_id;injectClaudeMetadataUserIdWithContextthen short-circuits on the already-present value, so the post-assignment injection becomes a no-op. IfgetOrCreateSessionIdreturns a different ID than the client's hint, the outbounduser_idwill carry a session ID that no longer matchessession.sessionId. The new test atsession-guard-warmup-intercept.test.ts:965inadvertently demonstrates this: the mock returns"session_assigned"while the assertion checks forsess_legacy_seedsession-manager.tsandforwarder.tscleanly delegate to the new module with no other concernsConfidence Score: 3/5
Important Files Changed
Sequence Diagram
sequenceDiagram participant C as Client participant SG as SessionGuard participant SM as SessionManager participant MUI as metadata-user-id.ts participant FW as Forwarder C->>SG: POST /v1/messages (metadata.session_id or metadata.user_id) SG->>SM: extractClientSessionId(requestMessage, null, ua)<br/>(pre-extraction pass) SM->>MUI: parseClaudeMetadataUserId(user_id) MUI-->>SM: { sessionId, format } SM-->>SG: extractedClaudeSessionId alt extractedClaudeSessionId != null SG->>MUI: injectClaudeMetadataUserIdWithContext(msg, extractedClaudeSessionId) note over SG,MUI: user_id frozen with client-supplied session hint MUI-->>SG: mutated message (user_id written) SG->>SG: session.request.message = mutated end SG->>SM: extractClientSessionId(session.request.message, headers, ua)<br/>(main extraction) SM-->>SG: clientSessionId SG->>SM: getOrCreateSessionId(keyId, messages, clientSessionId) SM-->>SG: sessionId (may differ from clientSessionId!) SG->>SG: session.setSessionId(sessionId) SG->>MUI: injectClaudeMetadataUserIdWithContext(msg, sessionId) note over SG,MUI: hasUsableClaudeMetadataUserId check:<br/>if user_id already set → returns early (no-op!) MUI-->>SG: message (possibly unchanged) SG->>FW: forward request FW->>MUI: applyClaudeMetadataUserIdInjectionWithAudit<br/>(also no-op if user_id already present) FW->>C: proxied responseComments Outside Diff (1)
src/app/v1/_lib/proxy/forwarder.ts, line 365-373 (link)The
existingMetadatareconstruction anduser_idexistence check on these lines are now performed twice: once here and again insideinjectClaudeMetadataUserIdWithContext. Since the delegate already short-circuits whenuser_idis present (and whenkeyId/sessionIdare missing), this wrapper-level check is no longer needed and can be removed to keep the function thin:Prompt To Fix With AI
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Prompt To Fix All With AI
Last reviewed commit: "fix: address metadat..."