**Actionable comments posted: 9**

🧹 Nitpick comments (2)

src/lib/mcp/workspace-cache.ts (1)
`16-17`: **This invalidation is only local to the current process.** Because the cache lives in module memory, deleting the key here only helps the instance that handled the write. If these routes scale horizontally, other instances can keep serving stale workspace data until the TTL expires.

🤖 Prompt for AI Agents
``` Verify each finding against the current code and only fix it if needed. In `@src/lib/mcp/workspace-cache.ts` around lines 16 - 17, invalidateWorkspaceCache only deletes the key from the local in-memory stateCache, so other process instances will still serve stale data; update the invalidation to publish a distributed invalidation event (or delete the key from the shared cache) so all instances clear their local stateCache. Modify invalidateWorkspaceCache (and add a companion subscriber handler if needed) to both remove workspaceId from stateCache and publish an "invalidate-workspace" message via your app's bus (e.g., Redis pub/sub or your existing message broker), and ensure any process subscribing to that topic clears stateCache.delete(workspaceId) when it receives the message. ```

src/lib/ai/workers/workspace-worker.ts (1)
`461-461`: **Extract the cache-bust into one shared post-append helper.** The same “append succeeded → invalidate cache → broadcast” sequence is now repeated in every mutation branch here, and the OCR/audio persist steps repeat it too. Centralizing that flow will make future mutation paths much less likely to miss the MCP cache invalidation again. Also applies to: 560-560, 688-688, 848-848, 975-975, 1081-1081, 1225-1225, 1360-1360, 1469-1469, 1551-1551, 1614-1614

🤖 Prompt for AI Agents
``` Verify each finding against the current code and only fix it if needed. In `@src/lib/ai/workers/workspace-worker.ts` at line 461, Multiple mutation branches repeat the exact "append succeeded → invalidateWorkspaceCache(params.workspaceId) → broadcast" sequence; extract that sequence into a single helper (e.g., handlePostAppend or postAppendSuccess) and call it from every mutation/OCR/audio persist branch. Specifically: create a helper that accepts the workspaceId and any broadcast payload/context, calls invalidateWorkspaceCache(workspaceId) and then performs the existing broadcast logic, replace each inline invalidateWorkspaceCache(...) + broadcast block with a call to that helper (references: invalidateWorkspaceCache and the existing broadcast call site), and update all repeated locations (the ones you noted and any similar branches) to use the new helper so cache-bust + broadcast are centralized. ```

🤖 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/api/mcp-keys/route.ts`: - Around line 15-42: The count-and-insert must be made atomic to prevent race conditions: wrap the check and insert in a single transaction and acquire a per-user lock (e.g., pg_advisory_xact_lock on userId) or use SELECT ... FOR UPDATE on a user row before re-counting; then re-check the active key count against MAX_KEYS_PER_USER and only perform the insert into apiKeys if below the limit. Modify the code around db, MAX_KEYS_PER_USER, and apiKeys in route.ts so the logic that reads the count and the db.insert(...) are executed inside db.transaction (or a DB-side function) with the advisory lock held to guarantee serialized checks and prevent concurrent inserts. In `@src/app/api/mcp/route.ts`: - Around line 298-327: The query string is still ReDoS-prone because arbitrary user regexes are compiled in new RegExp(query, "i") inside the search flow (see variables query, regex and function getCachedState); change the behavior to treat query as a literal substring by default: introduce/usage an escape utility (e.g., escapeRegex) to escape regex metacharacters and replace new RegExp(query, "i") with new RegExp(escapeRegex(query), "i") unless the client explicitly requests regex mode, or alternatively validate/compile user regexes with a safe engine before use; ensure MAX_REGEX_LENGTH still applies and update any client flags/validation around explicit regex mode to avoid silently allowing untrusted patterns. - Around line 71-75: The audio branch currently only reads data.transcript and summary, but persisted audio uses segments (see persist-result.ts); update the audio handling (case "audio" in extractText / route.ts) to fallback to flattening item.data.segments (e.g., join each segment.text in order) when data.transcript is missing, then include summary as before—i.e., build text from transcript || joined segments || null and combine with summary via the existing [..].filter(Boolean).join("\n\n") logic so search/read will include the full transcript content. - Around line 592-605: The handler currently only accepts "tools/list" and "tools/call" and rejects all other MCP messages, breaking the required MCP initialization flow; update the dispatch in the route handler (the block that examines method and calls server.request using ListToolsRequestSchema and CallToolRequestSchema) to properly handle the MCP lifecycle: accept and process "initialize" (validate via the appropriate InitializeRequest schema or server.request) and respond with the required initialize result, accept the "notifications/initialized" notification (no response) and record session state (e.g., set an initialized flag keyed to the incoming session/request context) before allowing "tools/list" and "tools/call", or replace this manual routing by delegating the entire HTTP transport to the SDK's StreamableHTTPServerTransport so it manages the init/notification lifecycle for you; ensure you reference and update the same dispatch logic around server.request, method, ListToolsRequestSchema, and CallToolRequestSchema to gate tool RPCs on a successful initialization. In `@src/components/auth/AccountModal.tsx`: - Around line 361-389: The snippet builders (functions/consts snippet and claudeCodeSnippet and the computed strings cursorGlobal, cursorProject, vscode) currently prepend a "// " comment so the copied payload is not valid JSON; remove the leading comment prefix and return only the JSON document string (e.g., start with "{\n \"mcpServers\": { ... } }") for both snippet(...) and claudeCodeSnippet, and ensure the copy action that uses cursorGlobal/cursorProject/vscode writes the pure JSON string unchanged so .json files remain valid. - Around line 138-147: fetchKeys currently assigns setKeys(data.keys || []) even when the HTTP response failed; change it to check res.ok before updating state so a 401/500 response does not clobber the existing keys. Specifically, after awaiting fetch("/api/mcp-keys") and before calling setKeys, verify res.ok and only parse/update state (setKeys(data.keys || [])) on success; on non-ok responses call toast.error (or keep existing toast/error handling) and do not call setKeys so the previous list remains; ensure setIsLoading(false) still runs in finally and consider logging the response status/message for debugging. In `@src/lib/base-url.ts`: - Around line 1-19: The current getBaseURL returns PRODUCTION_URL when NEXT_PUBLIC_APP_URL is missing or malformed, which can leak user secrets to the live site; update getBaseURL to validate NEXT_PUBLIC_APP_URL and fail fast: read envUrl in getBaseURL, if envUrl exists and is not a localhost/127.0.0.1 address return the trimmed value, otherwise if NODE_ENV === "production" allow falling back to PRODUCTION_URL, but in non-production (preview/self-hosted) throw a clear Error indicating NEXT_PUBLIC_APP_URL is missing or invalid so callers (e.g., MCP endpoint builders) won't silently point at PRODUCTION_URL; reference the getBaseURL function and PRODUCTION_URL constant when making this change. In `@src/lib/mcp/workspace-cache.ts`: - Around line 4-5: The in-memory Map stateCache and TTL constant CACHE_TTL_MS allow expired AgentState objects to accumulate; add a bounded pruning strategy: implement a periodic cleanup (e.g., setInterval) that iterates stateCache and deletes entries whose expiresAt < Date.now(), and also enforce a max size (evict oldest or least-recently-used entries) when stateCache.length exceeds a capped value; update any put/get code that sets expiresAt so accesses refresh LRU metadata if using LRU eviction. Use the symbols stateCache and CACHE_TTL_MS and add a small helper like pruneStateCache invoked on interval to perform the removals. - Around line 11-12: The current code unconditionally caches the result of loadWorkspaceState, which may be the loader's empty fallback on read errors; update the logic around loadWorkspaceState(workspaceId) so you only call stateCache.set(workspaceId, { state, expiresAt: Date.now() + CACHE_TTL_MS }) when the returned state is a real workspace (e.g., not an empty fallback): detect this by checking for expected content (for example that state is truthy and has required keys or non-empty collections) and skip caching when the state appears empty/fallback; keep references to loadWorkspaceState, stateCache, workspaceId, and CACHE_TTL_MS so the guard is implemented at the same call site. --- Nitpick comments: In `@src/lib/ai/workers/workspace-worker.ts`: - Line 461: Multiple mutation branches repeat the exact "append succeeded → invalidateWorkspaceCache(params.workspaceId) → broadcast" sequence; extract that sequence into a single helper (e.g., handlePostAppend or postAppendSuccess) and call it from every mutation/OCR/audio persist branch. Specifically: create a helper that accepts the workspaceId and any broadcast payload/context, calls invalidateWorkspaceCache(workspaceId) and then performs the existing broadcast logic, replace each inline invalidateWorkspaceCache(...) + broadcast block with a call to that helper (references: invalidateWorkspaceCache and the existing broadcast call site), and update all repeated locations (the ones you noted and any similar branches) to use the new helper so cache-bust + broadcast are centralized. In `@src/lib/mcp/workspace-cache.ts`: - Around line 16-17: invalidateWorkspaceCache only deletes the key from the local in-memory stateCache, so other process instances will still serve stale data; update the invalidation to publish a distributed invalidation event (or delete the key from the shared cache) so all instances clear their local stateCache. Modify invalidateWorkspaceCache (and add a companion subscriber handler if needed) to both remove workspaceId from stateCache and publish an "invalidate-workspace" message via your app's bus (e.g., Redis pub/sub or your existing message broker), and ensure any process subscribing to that topic clears stateCache.delete(workspaceId) when it receives the message. ```

🪄 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**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `c61cec41-65d4-4867-9707-79104d4ffc79`

📥 Commits

Reviewing files that changed from the base of the PR and between 65068fd90f1af0d35f74325b299dd4388594503f and b794f7c814b41f0f28f2cb02238188fa4c1ac7db.

📒 Files selected for processing (16)

* `.gitignore` * `drizzle/0006_add_api_keys.sql` * `drizzle/meta/_journal.json` * `package.json` * `src/app/api/mcp-keys/[id]/route.ts` * `src/app/api/mcp-keys/route.ts` * `src/app/api/mcp/route.ts` * `src/app/api/workspaces/[id]/events/route.ts` * `src/components/auth/AccountModal.tsx` * `src/components/ui/table.tsx` * `src/lib/ai/workers/workspace-worker.ts` * `src/lib/base-url.ts` * `src/lib/db/schema.ts` * `src/lib/mcp/workspace-cache.ts` * `src/workflows/audio-transcribe/steps/persist-result.ts` * `src/workflows/ocr-dispatch/steps/persist-results.ts`