Develop

[Diplow]

Summary by CodeRabbit

• New Features

  • Selectable LLM provider support (OpenRouter / Claude SDK) with sandboxed execution, Anthropic proxy, streaming, subagent creation, and map-centered AI context snapshots with single-query context retrieval.

• Documentation

  • New architecture and security guides for MCP, Claude SDK, Anthropic proxy, sandboxing, and operational guidance.

• Tests

  • Expanded suites for SDK streaming, repository adapters, queueing, context retrieval, and compatibility.

• Chores

  • Environment/config additions and dev script/package updates.

[coderabbitai]

Actionable comments posted: 9

♻️ Duplicate comments (3)

VERCEL_SANDBOX_INTEGRATION.md (1)

83-115: Use the correct token name: VERCEL_OIDC_TOKEN not VERCEL_TOKEN.

The documentation instructs users to set VERCEL_TOKEN, but the code in src/env.js (lines 26, 69) defines and exposes VERCEL_OIDC_TOKEN. This mismatch will cause sandbox initialization to fail when users follow this guide.

Apply this diff to correct the environment variable table:

 | Variable | Value | Description |
 |----------|-------|-------------|
 | `USE_SANDBOX` | `"true"` | Enable Vercel Sandbox (required for production) |
-| `VERCEL_TOKEN` | `<token>` | Vercel access token (get from vercel.com/account/tokens) |
+| `VERCEL_OIDC_TOKEN` | `<token>` | Vercel OIDC token (get from vercel.com/account/tokens or `vercel env pull`) |
 | `ANTHROPIC_API_KEY` | `sk-ant-...` | Anthropic API key (used by proxy, never exposed to sandbox) |

Also update the production setup instructions:

 1. Get Vercel token: https://vercel.com/account/tokens
 2. In Vercel project settings → Environment Variables, add:
    - `USE_SANDBOX=true`
-   - `VERCEL_TOKEN=<your-token>`
+   - `VERCEL_OIDC_TOKEN=<your-token>`
    - `ANTHROPIC_API_KEY=sk-ant-...`

And the "Next Steps Before Production" section:

-   - `VERCEL_TOKEN=<your-token>`
+   - `VERCEL_OIDC_TOKEN=<your-token>`

src/env.js (1)

20-27: Add conditional validation for provider-specific API keys.

The LLM_PROVIDER defaults to "openrouter", but both OPENROUTER_API_KEY and ANTHROPIC_API_KEY remain optional. This creates a runtime failure risk when the application attempts to use the selected provider without its required API key.

Add a .refine() validation to the createEnv call to enforce provider-specific keys:

     USE_QUEUE: z.enum(["true", "false"]).optional(),
   },
+  .refine((env) => {
+    // Skip validation in test environment
+    if (isTestEnv) return true;
+    
+    if (env.LLM_PROVIDER === "openrouter" && !env.OPENROUTER_API_KEY) {
+      throw new Error("OPENROUTER_API_KEY is required when LLM_PROVIDER is 'openrouter'");
+    }
+    
+    if (env.LLM_PROVIDER === "claude-agent-sdk" && !env.ANTHROPIC_API_KEY) {
+      throw new Error("ANTHROPIC_API_KEY is required when LLM_PROVIDER is 'claude-agent-sdk'");
+    }
+    
+    return true;
+  }, {
+    message: "Provider-specific API key validation failed"
+  }),
 
   /**

src/lib/domains/agentic/repositories/claude-agent-sdk.repository.ts (1)

149-153: Remove API key prefix from logs.

Line 151 logs a 15-character prefix of ANTHROPIC_API_KEY. This violates the security principle of never logging secret material, even partially.

Apply this diff:

       loggers.agentic('About to call SDK query', {
         anthropicBaseUrl: process.env.ANTHROPIC_BASE_URL,
-        anthropicApiKeyPrefix: process.env.ANTHROPIC_API_KEY?.substring(0, 15),
-        anthropicApiKeyLength: process.env.ANTHROPIC_API_KEY?.length
+        hasAnthropicApiKey: !!process.env.ANTHROPIC_API_KEY
+        // intentionally avoid logging key material
       })

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6483dc4 and ec7a93b.

📒 Files selected for processing (8)

• VERCEL_SANDBOX_INTEGRATION.md (1 hunks)
• docs/ANTHROPIC_PROXY_SECURITY.md (1 hunks)
• package.json (5 hunks)
• src/app/api/anthropic-proxy/[...path]/route.ts (1 hunks)
• src/env.js (2 hunks)
• src/lib/domains/agentic/repositories/_helpers/network-interceptor.ts (1 hunks)
• src/lib/domains/agentic/repositories/claude-agent-sdk-sandbox.repository.ts (1 hunks)
• src/lib/domains/agentic/repositories/claude-agent-sdk.repository.ts (1 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use absolute imports with the ~/ prefix instead of relative (./ or ../) imports

Files:

• src/lib/domains/agentic/repositories/_helpers/network-interceptor.ts
• src/lib/domains/agentic/repositories/claude-agent-sdk-sandbox.repository.ts
• src/env.js
• src/lib/domains/agentic/repositories/claude-agent-sdk.repository.ts
• src/app/api/anthropic-proxy/[...path]/route.ts

src/lib/domains/**

📄 CodeRabbit inference engine (CLAUDE.md)

Keep domain layer code under src/lib/domains following Domain-Driven Design boundaries

Files:

• src/lib/domains/agentic/repositories/_helpers/network-interceptor.ts
• src/lib/domains/agentic/repositories/claude-agent-sdk-sandbox.repository.ts
• src/lib/domains/agentic/repositories/claude-agent-sdk.repository.ts

🧠 Learnings (2)

📚 Learning: 2025-09-23T20:50:25.343Z

Learnt from: CR
Repo: Diplow/hexframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-23T20:50:25.343Z
Learning: Use pnpm scripts for quality checks: check:lint, typecheck, test, check:deadcode, check:architecture, check:ruleof6; do not rely on pnpm dev

Applied to files:

• package.json

📚 Learning: 2025-09-23T20:50:25.343Z

Learnt from: CR
Repo: Diplow/hexframe PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-09-23T20:50:25.343Z
Learning: Applies to src/app/map/** : Frontend/UI (Next.js App Router) code resides under src/app/map

Applied to files:

• src/app/api/anthropic-proxy/[...path]/route.ts

🧬 Code graph analysis (4)

src/lib/domains/agentic/repositories/_helpers/network-interceptor.ts (1)

src/lib/debug/debug-logger.ts (1)

• loggers (400-420)

src/lib/domains/agentic/repositories/claude-agent-sdk-sandbox.repository.ts (3)

src/lib/domains/agentic/repositories/index.ts (2)

• ClaudeAgentSDKSandboxRepository (13-13)
• ILLMRepository (8-8)

src/lib/debug/debug-logger.ts (1)

• loggers (400-420)

src/lib/domains/agentic/repositories/_helpers/sdk-helpers.ts (4)

• estimateUsage (29-43)
• extractSystemPrompt (3-8)
• buildPrompt (10-27)
• getClaudeModels (45-147)

src/lib/domains/agentic/repositories/claude-agent-sdk.repository.ts (4)

src/lib/debug/debug-logger.ts (1)

• loggers (400-420)

src/lib/domains/agentic/repositories/_helpers/network-interceptor.ts (1)

• installAnthropicNetworkInterceptor (22-80)

src/lib/domains/agentic/types/llm.types.ts (5)

• LLMGenerationParams (6-16)
• LLMResponse (18-30)
• LLMError (55-60)
• StreamChunk (32-35)
• ModelInfo (37-47)

src/lib/domains/agentic/repositories/_helpers/sdk-helpers.ts (4)

• extractSystemPrompt (3-8)
• buildPrompt (10-27)
• estimateUsage (29-43)
• getClaudeModels (45-147)

src/app/api/anthropic-proxy/[...path]/route.ts (3)

src/server/api/middleware/rate-limit.ts (1)

• rateLimits (123-171)

src/env.js (2)

• env (7-93)
• env (7-93)

src/lib/debug/debug-logger.ts (1)

• loggers (400-420)

🪛 GitHub Check: CodeQL

src/lib/domains/agentic/repositories/_helpers/network-interceptor.ts

[failure] 53-53: Incomplete URL substring sanitization
'api.anthropic.com' can be anywhere in the URL, and arbitrary hosts may come before or after it.

🪛 markdownlint-cli2 (0.18.1)

VERCEL_SANDBOX_INTEGRATION.md

13-13: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

---

164-164: No empty links

(MD042, no-empty-links)

docs/ANTHROPIC_PROXY_SECURITY.md

13-13: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

---

164-164: No empty links

(MD042, no-empty-links)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: Tests - Phase 2 (React Components)
• GitHub Check: Build
• GitHub Check: Tests - Phase 1 (Main Suite)
• GitHub Check: ESLint

🔇 Additional comments (4)

src/env.js (1)

64-70: LGTM: Runtime environment exposure is correct.

The new environment variables are properly exposed to the runtime environment with correct mappings.

src/lib/domains/agentic/repositories/claude-agent-sdk.repository.ts (3)

1-36: LGTM: Proper dynamic import strategy and safe helper function.

The comment explaining why query must be dynamically imported (to set env vars first) is helpful, and the extractDeltaText helper safely extracts delta text from SDK events.

---

218-328: LGTM: Streaming implementation correctly mirrors the generate method.

The streaming method properly:

• Uses includePartialMessages: true for real-time streaming
• Invokes onChunk callbacks with delta text
• Signals completion with isFinished: true
• Handles errors consistently with the non-streaming path

---

330-356: LGTM: Model info and error handling methods are well-structured.

The utility methods correctly:

• Retrieve model information from the static model database
• Check configuration status
• Create properly typed LLMError instances with provider metadata

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Fix Markdown fenced block language.

markdownlint is failing (MD040) because the architecture block lacks a language hint. Please declare it (e.g. text) so docs builds stay green.

-```
+```text
 …
-```
+```

🧰 Tools

🪛 markdownlint-cli2 (0.18.1)

13-13: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

🤖 Prompt for AI Agents

In docs/ANTHROPIC_PROXY_SECURITY.md around lines 13 to 35 the fenced code block
used to render the ASCII architecture diagram lacks a language hint which
triggers markdownlint MD040; update the opening fence to include a language tag
(e.g., change ``` to ```text) so the block is explicitly marked as text and
linting/doc builds pass.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Replace the placeholder PR link.

markdownlint (MD042) rejects the empty (#) link. Point it at the actual PR URL so the doc and lint both succeed.

-- [Implementation PR](#) <!-- Add PR link when merged -->
+- [Implementation PR](https://github.com/Diplow/hexframe/pull/152)

📝 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

	- [Anthropic SDK Base URL](https://github.com/anthropics/anthropic-sdk-typescript)
	- [Vercel Sandbox Docs](https://vercel.com/docs/vercel-sandbox)
	- [Implementation PR](#) <!-- Add PR link when merged -->
	- [Anthropic SDK Base URL](https://github.com/anthropics/anthropic-sdk-typescript)
	- [Vercel Sandbox Docs](https://vercel.com/docs/vercel-sandbox)
	- [Implementation PR](https://github.com/Diplow/hexframe/pull/152)

🧰 Tools

🪛 markdownlint-cli2 (0.18.1)

164-164: No empty links

(MD042, no-empty-links)

🤖 Prompt for AI Agents

In docs/ANTHROPIC_PROXY_SECURITY.md around lines 162 to 164, the Implementation
PR link is a placeholder "(#)" which triggers markdownlint MD042; replace that
placeholder with the actual pull request URL (absolute or repo-relative, e.g.
https://github.com/ORG/REPO/pull/NNN or ../pull/NNN) so the link is non-empty,
or if the PR URL is not yet available remove the link entirely and add a TODO
comment instead; ensure the final markdown uses a valid non-empty link target to
satisfy the linter.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Do not ship with a default proxy secret.

Falling back to 'change-me-in-production' makes the proxy callable by anyone who knows the default value. If INTERNAL_PROXY_SECRET is unset in prod, the protection is effectively gone. Fail fast instead so misconfiguration is visible.

-    const expectedAuth = env.INTERNAL_PROXY_SECRET ?? 'change-me-in-production'
+    const expectedAuth = env.INTERNAL_PROXY_SECRET
+    if (!expectedAuth) {
+      loggers.agentic.error('Anthropic proxy: INTERNAL_PROXY_SECRET missing')
+      return NextResponse.json(
+        { error: 'Proxy misconfigured: INTERNAL_PROXY_SECRET is not set' },
+        { status: 500 }
+      )
+    }

📝 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

	const expectedAuth = env.INTERNAL_PROXY_SECRET ?? 'change-me-in-production'
	// Validate client API key
	if (!clientApiKey || clientApiKey !== expectedAuth) {
	loggers.agentic.error('Anthropic proxy: Unauthorized request')
	return NextResponse.json(
	{ error: 'Unauthorized: Invalid API key' },
	{ status: 401 }
	)
	const expectedAuth = env.INTERNAL_PROXY_SECRET
	if (!expectedAuth) {
	loggers.agentic.error('Anthropic proxy: INTERNAL_PROXY_SECRET missing')
	return NextResponse.json(
	{ error: 'Proxy misconfigured: INTERNAL_PROXY_SECRET is not set' },
	{ status: 500 }
	)
	}
	// Validate client API key
	if (!clientApiKey || clientApiKey !== expectedAuth) {
	loggers.agentic.error('Anthropic proxy: Unauthorized request')
	return NextResponse.json(
	{ error: 'Unauthorized: Invalid API key' },
	{ status: 401 }
	)

🤖 Prompt for AI Agents

In src/app/api/anthropic-proxy/[...path]/route.ts around lines 60 to 68, remove
the fallback default 'change-me-in-production' and treat a missing
INTERNAL_PROXY_SECRET as a fatal misconfiguration: check if
env.INTERNAL_PROXY_SECRET is set at runtime (or preferably during app startup)
and if it's not, log an explicit error and respond with an error (HTTP 500) or
throw so the service fails fast; then validate clientApiKey strictly against the
real env.INTERNAL_PROXY_SECRET and return 401 only when a key is provided but
does not match.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

Verify host equality when intercepting URLs.

url.includes('api.anthropic.com') can match attacker-controlled hosts like https://api.anthropic.com.evil. Parse once and compare the hostname so only genuine Anthropic calls are rewritten (apply the same fix to the sandbox inline interceptor).

-    if (url.includes('api.anthropic.com')) {
-      // Extract the path from the Anthropic URL
-      // e.g., "https://api.anthropic.com/v1/messages" -> "/v1/messages"
-      const anthropicUrl = new URL(url)
+    if (url.startsWith('http')) {
+      const anthropicUrl = new URL(url)
+      if (anthropicUrl.hostname !== 'api.anthropic.com') {
+        return originalFetch(input, init)
+      }
+
+      // Extract the path from the Anthropic URL
+      // e.g., "https://api.anthropic.com/v1/messages" -> "/v1/messages"
       const apiPath = anthropicUrl.pathname + anthropicUrl.search

📝 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 (url.includes('api.anthropic.com')) {
	// Extract the path from the Anthropic URL
	// e.g., "https://api.anthropic.com/v1/messages" -> "/v1/messages"
	const anthropicUrl = new URL(url)
	const apiPath = anthropicUrl.pathname + anthropicUrl.search
	// Build proxy URL
	const proxyUrl = `${config.proxyBaseUrl}${apiPath}`
	if (url.startsWith('http')) {
	const anthropicUrl = new URL(url)
	if (anthropicUrl.hostname !== 'api.anthropic.com') {
	return originalFetch(input, init)
	}
	// Extract the path from the Anthropic URL
	// e.g., "https://api.anthropic.com/v1/messages" -> "/v1/messages"
	const apiPath = anthropicUrl.pathname + anthropicUrl.search
	// Build proxy URL
	const proxyUrl = `${config.proxyBaseUrl}${apiPath}`

🧰 Tools

🪛 GitHub Check: CodeQL

[failure] 53-53: Incomplete URL substring sanitization
'api.anthropic.com' can be anywhere in the URL, and arbitrary hosts may come before or after it.

🤖 Prompt for AI Agents

In src/lib/domains/agentic/repositories/_helpers/network-interceptor.ts around
lines 53 to 61, the code uses url.includes('api.anthropic.com') which can match
malicious hostnames; instead parse the URL once with new URL(url) and compare
the hostname (e.g., parsed.hostname.toLowerCase() === 'api.anthropic.com')
before rewriting, then build the proxy path from parsed.pathname+parsed.search;
apply the same hostname-compare change to the sandbox inline interceptor as
well.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify the security implications of permissionMode: 'bypassPermissions'.

Line 166 sets permissionMode: 'bypassPermissions', which allows MCP tools to execute without user permission prompts. While this enables smoother agentic workflows, it could pose security risks if untrusted tools are registered or if tool definitions are compromised.

Ensure that:

1. All MCP tools registered at /api/mcp are audited and trusted
2. The x-api-key authentication on the MCP server prevents unauthorized tool registration
3. Tool definitions include appropriate input validation and sandboxing

Run this script to verify MCP tool registration security:

---

🏁 Script executed:

#!/bin/bash
# Description: Check MCP server implementation for authentication and tool validation

# Find MCP server route implementation
fd -e ts -e tsx "mcp" --type f src/app/api | head -5

# Search for tool registration and authentication logic
rg -n -C5 "x-api-key|registerTool|tool.*registration" --type ts -g "**/*mcp*"

Length of output: 564

---

Now I need to verify the actual API implementation in the codebase:

Based on my research, I can now provide a comprehensive rewritten review comment. The key findings are:

1. Avoid bypassPermissions in production or on systems with sensitive data
2. Security researchers released analysis that there are multiple outstanding security issues with MCP, including prompt injection, tool permissions where combining tools can exfiltrate files, and lookalike tools can silently replace trusted ones
3. Authorization MUST be included in every HTTP request from client to server, even if they are part of the same logical session

The review comment conflates two separate security models: local Claude SDK permission modes and remote MCP server authentication. Let me provide the rewritten comment:

---

Avoid permissionMode: 'bypassPermissions' in production environments with untrusted or shared MCP tools.

Line 166 sets permissionMode: 'bypassPermissions', which should be avoided in production or on systems with sensitive data. This setting bypasses all tool execution approval prompts, so any MCP tool registered via the /api/mcp endpoint will execute without user confirmation.

Combined with known MCP vulnerabilities, this creates risks:

• Security researchers released analysis that there are multiple outstanding security issues with MCP, including prompt injection, tool permissions where combining tools can exfiltrate files, and lookalike tools can silently replace trusted ones
• If tools are dynamically registered or externally sourced, compromised tools execute silently

The x-api-key header in mcp-config.ts provides server authentication, but:

1. Authorization MUST be included in every HTTP request from client to server—verify the /api/mcp endpoint validates this on all requests
2. Authentication alone doesn't prevent prompt injection or tool combination attacks
3. If tools are user-configurable, they bypass the approval dialog

Recommendations:

• Use permissionMode: 'default' or 'plan' for production agentic workflows
• If bypassPermissions is necessary for development, restrict to sandboxed environments (e.g., Docker)
• Verify the /api/mcp endpoint enforces x-api-key validation and maintains a whitelisted tool registry
• Document which MCP tools are trusted and why bypassPermissions is acceptable in your use case

🤖 Prompt for AI Agents

In src/lib/domains/agentic/repositories/claude-agent-sdk.repository.ts around
lines 159-168 you set permissionMode: 'bypassPermissions' which disables tool
execution approval and is unsafe for production; change the permissionMode to
'default' or 'plan' for production agentic workflows, restrict any use of
'bypassPermissions' to isolated development/sandbox environments, ensure the
/api/mcp endpoint enforces x-api-key authentication on every request and
validates against a server-side whitelisted tool registry, and add documentation
noting which MCP tools are trusted and why any bypass is allowed.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

docs/CLAUDE_SDK_SECURITY_ARCHITECTURE.md (1)

463-463: Specify language for fenced code blocks to comply with MD040 linting.

The two ASCII diagrams lack language specifiers on their opening fence markers. Specify text or another appropriate language identifier.

Apply this diff:

- ┌─────────────────────────────────────────────────────────────────┐
+ ```text
+ ┌─────────────────────────────────────────────────────────────────┐
  │ User Request: "Create a new map tile"                          │
- ┌─────────────────────────────────────────────────────────────────┐

And similarly for line 537:

- ┌─────────────────────────────────────────────────────────────────┐
+ ```text
  │ User Request: "Create a new map tile"                          │

Also applies to: 537-537

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ec7a93b and bca8391.

📒 Files selected for processing (1)

• docs/CLAUDE_SDK_SECURITY_ARCHITECTURE.md (1 hunks)

🧰 Additional context used

🪛 LanguageTool

docs/CLAUDE_SDK_SECURITY_ARCHITECTURE.md

[grammar] ~247-~247: Ensure spelling is correct
Context: ...getCurrentUser - Get user profile and mappingId - getItemsForRootItem - Read entire hexagonal map hierarchy ...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

🪛 markdownlint-cli2 (0.18.1)

docs/CLAUDE_SDK_SECURITY_ARCHITECTURE.md

463-463: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

---

537-537: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: Build
• GitHub Check: Tests - Phase 2 (React Components)
• GitHub Check: ESLint
• GitHub Check: Tests - Phase 1 (Main Suite)

🔇 Additional comments (1)

docs/CLAUDE_SDK_SECURITY_ARCHITECTURE.md (1)

1-640: Comprehensive security documentation with clear threat modeling and defense-in-depth architecture.

This documentation provides thorough coverage of the Claude SDK integration security model. The three-layer proxy architecture is well-explained, threat modeling is systematic with honest residual risk assessment, and the security checklist is actionable for deployment.

[coderabbitai]

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/server/api/routers/agentic/agentic.ts (1)

118-125: Remove unnecessary type casts.

Lines 120 and 124 use as to cast types, but the zod schemas already validate and properly infer the correct types:

• messages: z.array(chatMessageSchema) already infers to ChatMessageContract[]
• compositionConfig: compositionConfigSchema.optional() already infers to CompositionConfig | undefined

The casts are redundant and mask the benefit of zod's type inference. Remove both casts on lines 120 and 124:

const response = await agenticService.generateResponse({
  mapContext,
  messages: input.messages,
  model: input.model,
  temperature: input.temperature,
  maxTokens: input.maxTokens,
  compositionConfig: input.compositionConfig
})

♻️ Duplicate comments (3)

src/lib/domains/iam/services/internal-api-key.service.ts (3)

36-40: Add input validation for userId and purpose parameters.

The function accepts userId and purpose without validating that they are non-empty strings. This could lead to corrupted data or unexpected database behavior.

Apply this diff to add validation:

 export async function getOrCreateInternalApiKey(
   userId: string,
   purpose: string,
   ttlMinutes?: number
 ): Promise<string> {
+  if (!userId?.trim() || !purpose?.trim()) {
+    throw new Error('userId and purpose must be non-empty strings')
+  }
+
   // Try to find existing active key

---

147-160: Timing attack vulnerability in key comparison.

Line 150 uses === to compare the decrypted key with the plaintext input, which is vulnerable to timing attacks. An attacker measuring response times could extract information about valid keys.

Use constant-time comparison:

+import { timingSafeEqual } from "crypto"

Update the comparison logic:

       try {
         const decrypted = decrypt(key.encryptedKey)
 
-        if (decrypted === plaintextKey) {
+        // Use constant-time comparison to prevent timing attacks
+        if (decrypted.length === plaintextKey.length && 
+            timingSafeEqual(Buffer.from(decrypted), Buffer.from(plaintextKey))) {
           // Update last used

---

170-207: Potential DoS and timing attack vulnerabilities in fallback path.

Lines 172-174 load all active keys without pagination or limits, which becomes expensive with many users and could be exploited for DoS attacks. Additionally, line 189 shares the timing attack vulnerability from the fast path.

Consider these mitigations:

1. Add pagination/limits:

   const allKeys = await db.query.internalApiKeys.findMany({
-    where: eq(internalApiKeys.isActive, true)
+    where: eq(internalApiKeys.isActive, true),
+    limit: 1000  // Prevent loading excessive records
   })

2. Apply timing-safe comparison to line 189 (same fix as the fast path)
3. Add rate limiting for validation attempts
4. Add observability to log when the fallback path is used

Ideally, require userId to avoid this expensive fallback entirely.

🧹 Nitpick comments (6)

docs/CLAUDE_SDK_SECURITY_ARCHITECTURE.md (4)

468-468: Specify language for ASCII art code blocks.

Both architecture diagrams are fenced code blocks without a language specifier. While ASCII art rendering is acceptable, add language identifier for better Markdown compliance and rendering consistency.

-```
+```text
 ┌─────────────────────────────────────────────────────────────────┐
 │ User Request: "Create a new map tile"                          │
 ...

Apply the same change to the second diagram starting at line 542.


Also applies to: 542-542

---

`371-446`: **Environment configuration guidance is thorough and well-structured.**

The `.env` vs `.env.local` vs Vercel distinction is clear. However, add explicit warning about Vercel OIDC token handling:


At line 436-446, consider clarifying that:
- `VERCEL_OIDC_TOKEN` is **not** available in local development (only in deployed Vercel environments)
- Using `vercel env pull` in development will pull production secrets—recommend against this
- For local sandbox testing, either deploy to preview or use non-sandbox mode

This is already mentioned in the Q&A (lines 638-639), but should be highlighted more prominently in the Configuration Files section for discoverability.

---

`590-612`: **Security checklist is practical but incomplete.**

The checklist covers critical items but is missing verification steps. Consider adding:
- [ ] Proxy secret rotation policy documented
- [ ] MCP token expiration verified in logs
- [ ] Sandbox network restrictions tested (allowlist validation)
- [ ] No hardcoded secrets in deployment configurations
- [ ] Audit logging for all proxy requests enabled


Also, the `[x]` checkbox on line 601 suggests the short-lived MCP token feature is already implemented—verify this is accurate and update the checkbox if this PR is introducing the feature (should then be `[ ]` or clarify it's a retrospective documentation of an existing feature).

---

`1-50`: **Consider adding disclaimer on documentation maintenance.**

This is a detailed, technical document that will require ongoing updates as the implementation evolves. Add a prominent note near the top (after the overview) reminding maintainers to:
1. Keep threat models synchronized with code changes
2. Re-review this doc whenever Claude SDK or Vercel Sandbox versions update
3. Track implementation gaps between documentation and actual code


Example:
```markdown
⚠️ **Maintenance Note:** This document describes the security architecture as of 2025-11-05. 
Security-related code changes must be reflected here within 2 weeks of merge. 
Review quarterly or when dependencies (Claude SDK, Vercel) update major versions.

src/server/api/routers/agentic/agentic.ts (2)

96-107: Extract agentic service creation to reduce duplication.

Lines 96-107 are similar to lines 185-196 and lines 240-248. Extract to a helper function.

Create a helper function:

function createAgenticServiceForRequest(
  mcpApiKey: string | undefined,
  eventBus: EventBusImpl,
  useQueue: boolean,
  userId: string
) {
  return createAgenticService({
    llmConfig: {
      openRouterApiKey: env.OPENROUTER_API_KEY ?? '',
      anthropicApiKey: env.ANTHROPIC_API_KEY ?? '',
      preferClaudeSDK: true,
      useSandbox: env.USE_SANDBOX === 'true',
      mcpApiKey
    },
    eventBus,
    useQueue,
    userId
  })
}

Then use it in all three locations.

---

210-222: Consider error handling for streaming callback.

The streaming callback (lines 219-221) pushes chunks to an array but doesn't handle potential errors during chunk processing. If an error occurs, it could leave the response in an inconsistent state.

Consider adding error handling:

       const response = await agenticService.generateStreamingResponse(
         {
           mapContext,
           messages: input.messages as ChatMessageContract[],
           model: input.model,
           temperature: input.temperature,
           maxTokens: input.maxTokens,
           compositionConfig: input.compositionConfig as CompositionConfig
         },
         (chunk) => {
+          try {
             chunks.push(chunk)
+          } catch (error) {
+            console.error('Error processing streaming chunk:', error)
+            // Consider propagating error or handling gracefully
+          }
         }
       )

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between bca8391 and 9eba7a5.

📒 Files selected for processing (3)

• docs/CLAUDE_SDK_SECURITY_ARCHITECTURE.md (1 hunks)
• src/lib/domains/iam/services/internal-api-key.service.ts (1 hunks)
• src/server/api/routers/agentic/agentic.ts (4 hunks)

🧰 Additional context used

📓 Path-based instructions (3)

**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use absolute imports with the ~/ prefix instead of relative (./ or ../) imports

Files:

• src/server/api/routers/agentic/agentic.ts
• src/lib/domains/iam/services/internal-api-key.service.ts

src/server/**

📄 CodeRabbit inference engine (CLAUDE.md)

Backend/API (tRPC and server logic) code resides under src/server

Files:

• src/server/api/routers/agentic/agentic.ts

src/lib/domains/**

📄 CodeRabbit inference engine (CLAUDE.md)

Keep domain layer code under src/lib/domains following Domain-Driven Design boundaries

Files:

• src/lib/domains/iam/services/internal-api-key.service.ts

🧬 Code graph analysis (2)

src/server/api/routers/agentic/agentic.ts (7)

src/server/api/trpc.ts (3)

• mappingServiceMiddleware (434-446)
• iamServiceMiddleware (448-459)
• protectedProcedure (247-262)

src/lib/domains/mapping/utils/context.ts (1)

• ContextStrategies (28-68)

src/lib/domains/iam/services/internal-api-key.service.ts (1)

• getOrCreateInternalApiKey (36-86)

src/lib/domains/agentic/services/agentic.factory.ts (1)

• createAgenticService (43-125)

src/env.js (2)

• env (7-93)
• env (7-93)

src/lib/domains/agentic/types/context.types.ts (2)

• ChatMessageContract (111-111)
• CompositionConfig (90-109)

src/lib/domains/agentic/types/contracts.ts (1)

• ChatMessageContract (11-19)

src/lib/domains/iam/services/internal-api-key.service.ts (3)

src/server/db/index.ts (2)

• schema (78-78)
• db (75-75)

src/server/db/schema/_tables/auth/internal-api-keys.ts (1)

• internalApiKeys (26-50)

src/lib/domains/iam/infrastructure/encryption.ts (2)

• decrypt (65-84)
• encrypt (42-58)

🪛 LanguageTool

docs/CLAUDE_SDK_SECURITY_ARCHITECTURE.md

[grammar] ~247-~247: Ensure spelling is correct
Context: ...getCurrentUser - Get user profile and mappingId - getItemsForRootItem - Read entire hexagonal map hierarchy ...

(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)

🪛 markdownlint-cli2 (0.18.1)

docs/CLAUDE_SDK_SECURITY_ARCHITECTURE.md

468-468: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

---

542-542: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)

• GitHub Check: Tests - Phase 1 (Main Suite)
• GitHub Check: Tests - Phase 2 (React Components)
• GitHub Check: Build

🔇 Additional comments (4)

docs/CLAUDE_SDK_SECURITY_ARCHITECTURE.md (4)

247-247: Field name styling in documentation.

Line 247 references mappingId as a field name. This is correctly camelCase for a field identifier, so the static analysis flag is a false positive. No change needed, but consider whether this field should be explicitly linked/referenced to the actual codebase type definitions for maintainability.

---

1-100: Comprehensive threat model coverage with appropriate risk assessment.

The security architecture document provides clear, actionable guidance on defense-in-depth mitigation. The three-layer proxy pattern (network interceptor + secure proxy + API key isolation) is well-architected, and the separation of development vs. production modes with explicit risk labeling is essential.

Key strengths:

• Explicit "UNSAFE" warning on development mode with clear threat vectors
• Short-lived MCP session tokens (10-min TTL) with auto-rotation significantly reduce exposure window
• Proxy pattern prevents real API key exposure to sandbox/AI-generated code
• Comprehensive environment configuration guidance

---

265-313: Acknowledge MCP token mitigation improvements with future roadmap clarity.

The documentation appropriately escalates the MCP key theft threat from indefinite exposure to a 10-minute window with auto-rotation, significantly reducing residual risk. The recommended future mitigations (operation allowlisting, transaction logging, anomaly detection, sandbox network restrictions) are well-prioritized and realistic.

However, ensure that:

1. The short-lived token implementation (marked as ✅ IMPLEMENTED) is actually deployed and tested
2. The "Recommended Future Mitigations" section is tracked as backlog items with clear ownership
3. The "Implementation Priority: Medium" assessment is validated against your threat model and risk tolerance

Consider adding a security.md or MAINTENANCE.md cross-reference for ongoing audit/review procedures.

---

190-313: Documentation does not match implementation—rewrite to describe actual architecture.

The documented threat mitigations are implemented but use a different architecture than described:

What the doc claims:

• "API key isolation via ANTHROPIC_API_KEY_ORIGINAL" ❌ (does not exist)
• "Sandbox mode in production prevents env access" ⚠️ (not clearly demonstrated)

What's actually implemented:

• ✅ Network interceptor (network-interceptor.ts) overrides global fetch and redirects to proxy
• ✅ Proxy validation (anthropic-proxy/route.ts) validates INTERNAL_PROXY_SECRET header
• ✅ MCP token TTL & rotation (internal-api-key.service.ts):
  • Supports ttlMinutes parameter for short-lived tokens
  • validateInternalApiKey() auto-deactivates expired keys
  • rotateInternalApiKey() creates new keys on each session
  • Purpose field distinguishes token types ('mcp', 'mcp-session')

The code implements stronger mitigations than documented—it has proper TTL management with automatic expiration and key rotation. Update the documentation to describe the actual internal-api-key.service pattern instead of the non-existent ANTHROPIC_API_KEY_ORIGINAL mechanism.

Likely an incorrect or invalid review comment.

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major

Add input validation for plaintextKey parameter.

The function should validate that plaintextKey is a non-empty string before performing database queries.

Apply this diff:

 export async function validateInternalApiKey(
   plaintextKey: string,
   userId?: string
 ): Promise<{ userId: string; purpose: string } | null> {
+  if (!plaintextKey?.trim()) {
+    return null
+  }
+
   const now = new Date()

📝 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

	export async function validateInternalApiKey(
	plaintextKey: string,
	userId?: string
	): Promise<{ userId: string; purpose: string } | null> {
	export async function validateInternalApiKey(
	plaintextKey: string,
	userId?: string
	): Promise<{ userId: string; purpose: string } | null> {
	if (!plaintextKey?.trim()) {
	return null
	}
	const now = new Date()

🤖 Prompt for AI Agents

In src/lib/domains/iam/services/internal-api-key.service.ts around lines 122 to
125, the function validateInternalApiKey does not validate its plaintextKey
input; add an early guard that ensures plaintextKey is a non-empty string (e.g.,
typeof plaintextKey === 'string' and plaintextKey.trim() !== ''), and if the
check fails return null immediately (or reject appropriately per existing
function conventions) to avoid performing any database queries with invalid
input.

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major

Extract MCP API key generation to reduce duplication.

Lines 88-93 are duplicated in generateStreamingResponse (lines 177-182). Extract this to a helper function.

Create a helper function:

async function getSessionMcpApiKey(userId: string | undefined): Promise<string | undefined> {
  if (!userId) return undefined
  
  const { getOrCreateInternalApiKey } = await import('~/lib/domains/iam')
  return getOrCreateInternalApiKey(userId, 'mcp-session', 10)
}

Then use it in both procedures:

-      const { getOrCreateInternalApiKey } = await import('~/lib/domains/iam')
-      const mcpApiKey = ctx.session?.userId
-        ? await getOrCreateInternalApiKey(ctx.session.userId, 'mcp-session', 10)
-        : undefined
+      const mcpApiKey = await getSessionMcpApiKey(ctx.session?.userId)

🤖 Prompt for AI Agents

In src/server/api/routers/agentic/agentic.ts around lines 88-93, the MCP API key
generation logic is duplicated (also present around lines 177-182); extract it
into a single async helper (e.g., getSessionMcpApiKey(userId?: string):
Promise<string | undefined>) that returns undefined for no userId, imports
getOrCreateInternalApiKey, and calls it with 'mcp-session' and TTL 10, then
replace both in-file occurrences with calls to that helper to eliminate
duplication and centralize behavior.

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (3)

src/lib/domains/iam/services/internal-api-key.service.ts (3)

138-143: Missing input validation for plaintextKey parameter.

The function doesn't validate that plaintextKey is a non-empty string before performing database queries. This was flagged in a previous review but remains unaddressed.

Apply this diff to add validation:

 export async function validateInternalApiKey(
   plaintextKey: string,
   userId?: string
 ): Promise<{ userId: string; purpose: string } | null> {
+  if (typeof plaintextKey !== 'string' || !plaintextKey.trim()) {
+    return null
+  }
+
   const now = new Date()

---

163-177: Timing attack vulnerability in key comparison.

Line 166 uses === to compare the decrypted key with the plaintext input, making the operation vulnerable to timing attacks. An attacker measuring response times could potentially extract information about valid keys. This critical issue was flagged in a previous review but remains unaddressed.

Apply this diff to use constant-time comparison:

+import { timingSafeEqual } from "crypto"

Then update the comparison at line 166:

       try {
         const decrypted = decrypt(key.encryptedKey)
 
-        if (decrypted === plaintextKey) {
+        // Use constant-time comparison to prevent timing attacks
+        if (
+          decrypted.length === plaintextKey.length &&
+          timingSafeEqual(Buffer.from(decrypted), Buffer.from(plaintextKey))
+        ) {
           // Update last used
           await db.update(internalApiKeys)

Note: The same fix must be applied to the fallback path at line 205.

---

186-223: Potential DoS and performance degradation in fallback path.

Lines 188-190 load all active keys from the database without pagination or limits, then attempt to decrypt each one. With many users, this operation becomes extremely expensive and could be exploited for DoS attacks. Additionally, line 205 shares the timing attack vulnerability from the fast path. These critical issues were flagged in a previous review but remain unaddressed.

Consider these mitigations:

1. Require userId parameter: Make userId mandatory to avoid the fallback path entirely
2. Add pagination/limits: If the fallback is necessary, add a reasonable limit:

   // Fallback: check all keys (for backwards compatibility or when userId not provided)
   // This is more expensive but ensures validation works even without userId hint
   const allKeys = await db.query.internalApiKeys.findMany({
-    where: eq(internalApiKeys.isActive, true)
+    where: eq(internalApiKeys.isActive, true),
+    limit: 1000  // Prevent loading excessive records
   })

3. Add rate limiting: Implement rate limiting for validation attempts to prevent brute-force and DoS attacks
4. Add observability: Log when the fallback path is used to monitor for abuse

Also apply the timing-safe comparison fix from the previous comment to line 205.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9eba7a5 and f261f9f.

📒 Files selected for processing (1)

• src/lib/domains/iam/services/internal-api-key.service.ts (1 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use absolute imports with the ~/ prefix instead of relative (./ or ../) imports

Files:

• src/lib/domains/iam/services/internal-api-key.service.ts

src/lib/domains/**

📄 CodeRabbit inference engine (CLAUDE.md)

Keep domain layer code under src/lib/domains following Domain-Driven Design boundaries

Files:

• src/lib/domains/iam/services/internal-api-key.service.ts

🧬 Code graph analysis (1)

src/lib/domains/iam/services/internal-api-key.service.ts (4)

src/server/db/index.ts (2)

• schema (78-78)
• db (75-75)

src/lib/domains/iam/index.ts (3)

• getOrCreateInternalApiKey (44-44)
• rotateInternalApiKey (45-45)
• validateInternalApiKey (46-46)

src/server/db/schema/_tables/auth/internal-api-keys.ts (1)

• internalApiKeys (26-50)

src/lib/domains/iam/infrastructure/encryption.ts (2)

• decrypt (65-84)
• encrypt (42-58)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: Build
• GitHub Check: Tests - Phase 2 (React Components)
• GitHub Check: Tests - Phase 1 (Main Suite)
• GitHub Check: ESLint

🔇 Additional comments (3)

src/lib/domains/iam/services/internal-api-key.service.ts (3)

1-7: LGTM!

The imports are correct and follow the coding guidelines using absolute imports with the ~/ prefix. The randomUUID import from crypto (line 3) properly addresses the previous concern about relying on global crypto.

---

36-94: LGTM!

The input validation (lines 42-47) properly addresses previous concerns using typeof checks and .trim(). The expiry handling logic (lines 60-64) correctly identifies and deactivates expired keys before creating a new one. The TTL support is well-implemented with optional expiresAt calculation.

---

105-127: LGTM!

The input validation (lines 110-115) properly addresses previous concerns. The rotation logic correctly deactivates old keys before creating a new one through getOrCreateInternalApiKey.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Misleading comment about key length.

The comment states "64 characters = 512 bits", but randomBytes(64) generates 64 bytes (512 bits), not 64 characters. After base64url encoding (line 22), the resulting key will be approximately 86 characters long.

Apply this diff to clarify:

-const KEY_LENGTH = 64 // 64 characters = 512 bits
+const KEY_LENGTH = 64 // 64 bytes = 512 bits (becomes ~86 chars after base64url encoding)

📝 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

	const KEY_LENGTH = 64 // 64 characters = 512 bits
	function generateApiKey(): string {
	return randomBytes(KEY_LENGTH).toString('base64url')
	}
	const KEY_LENGTH = 64 // 64 bytes = 512 bits (becomes ~86 chars after base64url encoding)
	function generateApiKey(): string {
	return randomBytes(KEY_LENGTH).toString('base64url')
	}

🤖 Prompt for AI Agents

In src/lib/domains/iam/services/internal-api-key.service.ts around lines 19 to
23, the comment is misleading: randomBytes(64) produces 64 bytes (512 bits) not
"64 characters", and after base64url encoding the output will be ~86 characters.
Update the comment to state that KEY_LENGTH is bytes (64 bytes = 512 bits) and
note that base64url encoding yields an ~86-character string, so callers should
treat the value as an encoded string of that length rather than 64 characters.

[coderabbitai]

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/server/api/routers/agentic/agentic.ts (1)

118-123: Update error message to reflect multi-provider support.

The error message only mentions OpenRouter, but the service configuration supports both OpenRouter and Anthropic (lines 106-112).

Update the message to match the one at line 205:

-          message: 'OpenRouter API key not configured. Please set OPENROUTER_API_KEY environment variable.',
+          message: 'API key not configured. Please set OPENROUTER_API_KEY or ANTHROPIC_API_KEY environment variable.',

♻️ Duplicate comments (2)

src/lib/domains/agentic/repositories/claude-agent-sdk.repository.ts (1)

169-169: Avoid permissionMode: 'bypassPermissions' in production environments with untrusted or shared MCP tools.

Lines 169 and 277 set permissionMode: 'bypassPermissions', which was flagged in previous reviews as a critical security issue. This bypasses all tool execution approval prompts, so any MCP tool registered via /api/mcp executes without user confirmation.

Combined with known MCP vulnerabilities, this creates risks:

• Security researchers released analysis documenting multiple MCP security issues including prompt injection, tool permission exploits where combining tools can exfiltrate files, and lookalike tools that can silently replace trusted ones
• If tools are dynamically registered or externally sourced, compromised tools execute silently

The x-api-key header provides server authentication, but:

1. Authorization must be validated on every HTTP request from client to server
2. Authentication alone doesn't prevent prompt injection or tool combination attacks
3. If tools are user-configurable, they bypass the approval dialog

Recommendations:

• Use permissionMode: 'default' or 'plan' for production agentic workflows
• If bypassPermissions is necessary for development, restrict to sandboxed environments
• Verify the /api/mcp endpoint enforces x-api-key validation and maintains a whitelisted tool registry
• Document which MCP tools are trusted and why bypassPermissions is acceptable in your use case

Apply this diff to use safer permission mode:

       model,
       systemPrompt,
       maxTurns: 10, // Allow multiple turns for tool use and agentic workflows
       mcpServers,
-      permissionMode: 'bypassPermissions' // Allow MCP tools without asking for permission
+      permissionMode: 'plan' // Review tool usage plan before execution
     }

Also applies to: 277-277

src/server/api/routers/agentic/agentic.ts (1)

97-102: Extract MCP API key generation to eliminate duplication.

The MCP API key generation logic here is duplicated at lines 181-186 in generateStreamingResponse. This was flagged in a previous review but appears unresolved.

Create a helper function:

async function getSessionMcpApiKey(userId: string | undefined): Promise<string | undefined> {
  if (!userId) return undefined
  
  const { getOrCreateInternalApiKey } = await import('~/lib/domains/iam')
  return getOrCreateInternalApiKey(userId, 'mcp-session', 10)
}

Then replace both occurrences:

-      const { getOrCreateInternalApiKey } = await import('~/lib/domains/iam')
-      const mcpApiKey = ctx.session?.userId
-        ? await getOrCreateInternalApiKey(ctx.session.userId, 'mcp-session', 10)
-        : undefined
+      const mcpApiKey = await getSessionMcpApiKey(ctx.session?.userId)

🧹 Nitpick comments (3)

src/lib/domains/agentic/repositories/_helpers/network-interceptor.ts (1)

74-81: Consider duplex option for streaming Request bodies.

Lines 74-81 preserve method and body correctly, but don't handle the duplex option required for streaming Request bodies in newer fetch implementations. While this may not be an issue for current usage, consider future-proofing.

If needed for streaming bodies, apply this diff:

     const originalRequest = input instanceof Request ? input : undefined
     const baseInit: RequestInit = {
       ...init,
       method: init?.method ?? originalRequest?.method,
       body:
         init?.body ??
-        (originalRequest?.body ? await originalRequest.clone().arrayBuffer() : undefined)
+        (originalRequest?.body ? await originalRequest.clone().arrayBuffer() : undefined),
+      duplex: init?.duplex ?? (originalRequest?.body ? 'half' : undefined)
     }

src/server/api/routers/agentic/agentic.ts (2)

129-129: Consider deriving Zod schema from the type to prevent drift.

The type cast is currently safe because the schema matches ChatMessageContract, but manual synchronization creates maintenance risk.

Consider using zod inference or creating the schema from the type:

import { z } from 'zod'
import type { ChatMessageContract } from '~/lib/domains/agentic'

// Derive the schema to stay in sync with the type
const chatMessageSchema = z.object({
  id: z.string(),
  type: z.enum(['user', 'assistant', 'system']),
  content: z.string(),
  metadata: z.object({
    timestamp: z.string().optional(),
    tileId: z.string().optional()
  }).optional()
}) satisfies z.ZodType<ChatMessageContract>

This ensures compile-time checking that the schema matches the type.

---

97-102: Consider adding automated cleanup for expired MCP API keys.

The short TTL (10 minutes) is excellent for limiting exposure, but expired keys will accumulate in the database.

Consider implementing a scheduled cleanup job (e.g., daily) to remove expired internalApiKeys records where expiresAt < NOW() AND isActive = false. This can be done via a cron job, database trigger, or background task to prevent table bloat.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f261f9f and aca80c1.

📒 Files selected for processing (4)

• src/lib/domains/agentic/repositories/_helpers/network-interceptor.ts (1 hunks)
• src/lib/domains/agentic/repositories/claude-agent-sdk-sandbox.repository.ts (1 hunks)
• src/lib/domains/agentic/repositories/claude-agent-sdk.repository.ts (1 hunks)
• src/server/api/routers/agentic/agentic.ts (4 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• src/lib/domains/agentic/repositories/claude-agent-sdk-sandbox.repository.ts

🧰 Additional context used

📓 Path-based instructions (3)

**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use absolute imports with the ~/ prefix instead of relative (./ or ../) imports

Files:

• src/server/api/routers/agentic/agentic.ts
• src/lib/domains/agentic/repositories/_helpers/network-interceptor.ts
• src/lib/domains/agentic/repositories/claude-agent-sdk.repository.ts

src/server/**

📄 CodeRabbit inference engine (CLAUDE.md)

Backend/API (tRPC and server logic) code resides under src/server

Files:

• src/server/api/routers/agentic/agentic.ts

src/lib/domains/**

📄 CodeRabbit inference engine (CLAUDE.md)

Keep domain layer code under src/lib/domains following Domain-Driven Design boundaries

Files:

• src/lib/domains/agentic/repositories/_helpers/network-interceptor.ts
• src/lib/domains/agentic/repositories/claude-agent-sdk.repository.ts

🧬 Code graph analysis (3)

src/server/api/routers/agentic/agentic.ts (8)

src/lib/domains/mapping/services/mapping.service.ts (1)

• MappingService (17-30)

src/lib/domains/mapping/utils/context.ts (1)

• ContextStrategies (28-68)

src/server/api/trpc.ts (2)

• mappingServiceMiddleware (434-446)
• iamServiceMiddleware (448-459)

src/lib/domains/iam/services/internal-api-key.service.ts (1)

• getOrCreateInternalApiKey (36-94)

src/lib/domains/agentic/services/agentic.factory.ts (1)

• createAgenticService (43-125)

src/env.js (2)

• env (7-93)
• env (7-93)

src/lib/domains/agentic/types/context.types.ts (2)

• ChatMessageContract (111-111)
• CompositionConfig (90-109)

src/lib/domains/agentic/types/contracts.ts (1)

• ChatMessageContract (11-19)

src/lib/domains/agentic/repositories/_helpers/network-interceptor.ts (1)

src/lib/debug/debug-logger.ts (1)

• loggers (400-420)

src/lib/domains/agentic/repositories/claude-agent-sdk.repository.ts (4)

src/lib/debug/debug-logger.ts (1)

• loggers (400-420)

src/lib/domains/agentic/repositories/_helpers/network-interceptor.ts (1)

• installAnthropicNetworkInterceptor (22-99)

src/lib/domains/agentic/types/llm.types.ts (5)

• LLMGenerationParams (6-16)
• LLMResponse (18-30)
• LLMError (55-60)
• StreamChunk (32-35)
• ModelInfo (37-47)

src/lib/domains/agentic/repositories/_helpers/sdk-helpers.ts (4)

• extractSystemPrompt (3-8)
• buildPrompt (10-27)
• estimateUsage (29-43)
• getClaudeModels (45-147)

🔇 Additional comments (10)

src/lib/domains/agentic/repositories/_helpers/network-interceptor.ts (1)

22-99: Well-structured security implementation.

The interceptor correctly addresses previous security concerns:

• Exact hostname comparison prevents malicious domain attacks (line 65)
• Request method/body preservation handles SDK calls properly (lines 74-81)
• Bypass mechanism prevents infinite loops (lines 42-44, 59-61)
• URL parsing errors are handled gracefully (lines 48-53)

src/lib/domains/agentic/repositories/claude-agent-sdk.repository.ts (5)

21-36: Type guard implementation is thorough.

The extractDeltaText helper safely navigates the event structure with comprehensive type checks, preventing runtime errors from malformed SDK events.

---

48-105: Proxy configuration correctly addresses past logging concerns.

The constructor properly:

• Validates INTERNAL_PROXY_SECRET when proxy is enabled (lines 53-55)
• Logs only boolean indicators, not secret prefixes (lines 69-73, 90-93) ✓
• Preserves original API key before overwriting (lines 62-64)
• Installs network interceptor to redirect SDK calls (lines 78-81)

---

158-171: Dynamic import after environment setup is correct.

Lines 158-160 dynamically import the SDK after setting environment variables, which is essential because the SDK reads ANTHROPIC_BASE_URL on module import. This pattern prevents import-time side effects from breaking proxy configuration.

---

186-192: Error handling covers all SDK error subtypes.

The error handling correctly catches error_during_execution, error_max_turns, and error_max_budget_usd subtypes, providing comprehensive coverage of SDK failure modes.

---

284-302: Streaming implementation correctly signals completion.

The streaming logic properly:

• Extracts delta text from stream events (lines 288-292)
• Handles result success/error cases (lines 293-301)
• Signals completion to the caller (line 305)

src/server/api/routers/agentic/agentic.ts (4)

15-26: LGTM! Duplication successfully eliminated.

The helper function correctly addresses the duplication flagged in the previous review. The strategy mapping logic is accurate and the function is properly reused in both procedures.

---

106-112: LGTM! LLM configuration is well-structured.

The configuration properly supports multiple providers with appropriate fallbacks and security considerations (short-lived MCP keys).

---

214-226: LGTM! Streaming implementation is correct.

The streaming response handling with callback accumulation is well-implemented, and the response structure matches the expected format.

---

244-252: LGTM! Configuration is consistent.

The LLM configuration matches the pattern established in other procedures, appropriately excluding mcpApiKey and useQueue for this read-only operation.

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major

Tighten the proxy loop-prevention check.

Line 59 uses pathname.includes('/api/anthropic-proxy'), which will match any path containing that substring anywhere (e.g., /foo/api/anthropic-proxy-test/bar). Use startsWith or exact matching to prevent false positives.

Apply this diff:

-    if (parsedUrl.pathname.includes('/api/anthropic-proxy')) {
+    if (parsedUrl.pathname.startsWith('/api/anthropic-proxy')) {
       return originalFetch(input, init)
     }

📝 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 (parsedUrl.pathname.includes('/api/anthropic-proxy')) {
	return originalFetch(input, init)
	}
	if (parsedUrl.pathname.startsWith('/api/anthropic-proxy')) {
	return originalFetch(input, init)
	}

🤖 Prompt for AI Agents

In src/lib/domains/agentic/repositories/_helpers/network-interceptor.ts around
lines 59 to 61, the proxy loop-prevention check uses
pathname.includes('/api/anthropic-proxy') which can match unintended paths;
replace it with a stricter match such as parsedUrl.pathname ===
'/api/anthropic-proxy' or parsedUrl.pathname.startsWith('/api/anthropic-proxy')
(prefer startsWith if you want to allow subpaths like
'/api/anthropic-proxy/...') so only the intended proxy endpoint triggers the
early return.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify if iamServiceMiddleware is necessary.

The iamServiceMiddleware is added but ctx.iamService is never used in these procedures. MCP API key generation uses a direct import instead (lines 99, 183).

Check if the IAM service should be used instead of the direct import, or if this middleware can be removed:

If IAMService does expose this method, consider using it through the context for better consistency:

const mcpApiKey = ctx.session?.userId
  ? await ctx.iamService.getOrCreateInternalApiKey(ctx.session.userId, 'mcp-session', 10)
  : undefined

Also applies to: 159-159

---

🏁 Script executed:

#!/bin/bash
# Check if IAMService exposes getOrCreateInternalApiKey method

ast-grep --pattern $'class IAMService {
  $$$
  getOrCreateInternalApiKey($$$) {
    $$$
  }
  $$$
}'

Length of output: 0

---

🏁 Script executed:

#!/bin/bash
# Search for IAMService class definition
rg -n "class IAMService" --type=ts --type=tsx

Length of output: 27

---

🏁 Script executed:

#!/bin/bash
# Search for getOrCreateInternalApiKey import
rg -n "getOrCreateInternalApiKey" --type=ts --type=tsx -B2 -A2

Length of output: 27

---

🏁 Script executed:

#!/bin/bash
# Check if ctx.iamService is used in agentic.ts
rg -n "ctx\.iamService" src/server/api/routers/agentic/agentic.ts

Length of output: 0

---

🏁 Script executed:

#!/bin/bash
# Search for IAMService class definition
rg -n "class IAMService" 

Length of output: 120

---

🏁 Script executed:

#!/bin/bash
# Search for getOrCreateInternalApiKey - all references
rg -n "getOrCreateInternalApiKey" -B2 -A2

Length of output: 2697

---

🏁 Script executed:

#!/bin/bash
# Check agentic.ts imports and getOrCreateInternalApiKey usage
cat src/server/api/routers/agentic/agentic.ts | head -150

Length of output: 5963

---

🏁 Script executed:

#!/bin/bash
# Search entire agentic.ts for any ctx.iamService usage
grep -n "ctx\.iamService\|iamService" src/server/api/routers/agentic/agentic.ts

Length of output: 251

---

🏁 Script executed:

#!/bin/bash
# Get full agentic.ts to check all procedures and context usage
wc -l src/server/api/routers/agentic/agentic.ts

Length of output: 45

---

🏁 Script executed:

#!/bin/bash
# Check generateStreamingResponse procedure around line 159
sed -n '150,200p' src/server/api/routers/agentic/agentic.ts

Length of output: 2116

---

Remove unnecessary iamServiceMiddleware from both procedures.

The middleware adds ctx.iamService to the context, but it's never referenced in either procedure. The MCP API key generation uses the standalone getOrCreateInternalApiKey() function (via dynamic import at lines 99 and 183), which doesn't require the middleware.

Remove .use(iamServiceMiddleware) from:

• Line 72 (generateResponse procedure)
• Line 159 (generateStreamingResponse procedure)

🤖 Prompt for AI Agents

In src/server/api/routers/agentic/agentic.ts around lines 72 and 159, remove the
unnecessary .use(iamServiceMiddleware) chaining from both the generateResponse
and generateStreamingResponse procedures since ctx.iamService is never used and
the MCP API key generation uses the standalone getOrCreateInternalApiKey() via
dynamic import; update the procedure chains to drop .use(iamServiceMiddleware)
so neither procedure applies that middleware anymore.

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

src/lib/domains/mapping/infrastructure/map-item/queries/specialized-queries.ts (1)

14-14: Unused type export: consider removing or documenting intended usage.

The FieldSelection type is exported but never used within this file. If it's intended for external consumption or future use, consider adding a JSDoc comment explaining its purpose. Otherwise, it may be dead code.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between aca80c1 and 05c53f2.

📒 Files selected for processing (2)

• src/lib/domains/mapping/infrastructure/map-item/queries/__tests__/context-for-center.test.ts (1 hunks)
• src/lib/domains/mapping/infrastructure/map-item/queries/specialized-queries.ts (3 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use absolute imports with the ~/ prefix instead of relative (./ or ../) imports

Files:

• src/lib/domains/mapping/infrastructure/map-item/queries/__tests__/context-for-center.test.ts
• src/lib/domains/mapping/infrastructure/map-item/queries/specialized-queries.ts

src/lib/domains/**

📄 CodeRabbit inference engine (CLAUDE.md)

Keep domain layer code under src/lib/domains following Domain-Driven Design boundaries

Files:

• src/lib/domains/mapping/infrastructure/map-item/queries/__tests__/context-for-center.test.ts
• src/lib/domains/mapping/infrastructure/map-item/queries/specialized-queries.ts

🧬 Code graph analysis (2)

src/lib/domains/mapping/infrastructure/map-item/queries/__tests__/context-for-center.test.ts (1)

src/lib/domains/mapping/services/__tests__/helpers/_test-utilities.ts (6)

• TestEnvironment (18-21)
• _cleanupDatabase (28-60)
• _createTestEnvironment (62-68)
• _createUniqueTestParams (120-133)
• _setupBasicMap (82-92)
• _createTestCoordinates (70-80)

src/lib/domains/mapping/infrastructure/map-item/queries/specialized-queries.ts (3)

src/lib/domains/mapping/infrastructure/map-item/types.ts (1)

• DbMapItemWithBase (24-27)

src/lib/domains/mapping/infrastructure/map-item/mappers.ts (1)

• pathToString (34-36)

src/server/db/schema/_tables/mapping/map-items.ts (1)

• mapItems (16-63)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: Build
• GitHub Check: Tests - Phase 2 (React Components)
• GitHub Check: Tests - Phase 1 (Main Suite)
• GitHub Check: ESLint

🔇 Additional comments (9)

src/lib/domains/mapping/infrastructure/map-item/queries/specialized-queries.ts (4)

218-426: Excellent optimization strategy with field-level control.

The three-query approach with progressive field reduction (full → preview → title-only) is well-designed for minimizing data transfer while preserving AI context needs. The composition-container exclusion logic correctly targets only actual composed children, not the container itself.

---

428-442: ✓ Bug fix confirmed: parent path now uses correct slice.

The path slicing bug flagged in the previous review has been properly fixed. Using slice(0, -1) correctly removes only the last segment, returning the immediate parent instead of the grandparent.

---

444-465: LGTM: Composed children filtering is precise.

The filter correctly identifies composed children by checking the prefix pattern and depth, excluding both the container itself and any non-composed siblings.

---

317-317: Empty string content handling is correctly implemented and validated across consumers.

The optimization is working as designed:

• Tests explicitly verify empty strings for nested items (lines 31, 81 in test file)
• Type definitions mark content as optional for children/grandchildren (agentic/types/contracts.ts)
• All serializers and strategies defensively check content before use: xml-serializer, structured-serializer, minimal.strategy, extended.strategy, and standard.strategy use conditional guards or fallback patterns
• No production crashes found; empty strings are properly handled throughout

src/lib/domains/mapping/infrastructure/map-item/queries/__tests__/context-for-center.test.ts (5)

1-18: LGTM: Test setup follows best practices.

Imports use absolute paths with ~/ prefix as per coding guidelines, and the beforeEach cleanup ensures test isolation.

---

21-142: Excellent test coverage for the parent path slicing fix.

These tests specifically validate that slice(0, -1) returns the immediate parent across various hierarchy depths. The test names clearly document the expected behavior and the bug that was fixed.

---

274-329: Good validation of composition-container exclusion.

This test correctly verifies that composition containers (paths ending with Direction.Center) are excluded from regular children, which is critical for the domain model's integrity.

---

713-814: Edge case well-covered: center path containing Direction.Center.

This test validates that when the center itself is a composed child (path contains 0), the query logic correctly distinguishes between:

• Regular children of the center (should be included)
• Composition containers under the center (should be excluded)

The comment at lines 802-804 references a potential bug pattern, but the implementation correctly uses precise patterns rather than overly broad wildcards.

---

816-923: Comprehensive full-context integration test.

This test validates all five context components together, ensuring they interact correctly and maintain proper parent-child-grandchild relationships. The assertions verify both structure and content.