• pnpm-lock.yaml: Language not supported

apps/backend/src/lib/ai/tools/docs.ts-26-34 (1)

26-34: ⚠️ Potential issue | 🟠 Major

Add a timeout to this outbound call.

This fetch currently has no timeout; transient network stalls can hold backend execution indefinitely. Add an AbortController with a timeout (e.g., 120 seconds, consistent with ai-query-handlers.ts) and pass it as the signal option. The existing error handling at lines 51-54 will map timeout failures to the fallback message appropriately.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/src/lib/ai/tools/docs.ts` around lines 26 - 34, The outbound
fetch that sets const res = await fetch(..., body: JSON.stringify(action)) lacks
a timeout; create an AbortController, set a 120000ms timer that calls
controller.abort(), and pass controller.signal to the fetch options so the
request cancels on timeout (clear the timer after fetch completes); mirror the
120s timeout used in ai-query-handlers.ts and let the existing error handling
map abort errors to the fallback message.

apps/backend/src/lib/ai/tools/docs.ts-53-56 (1)

53-56: ⚠️ Potential issue | 🟠 Major

Replace the blanket catch block with explicit error handling.

The catch (err) on line 53 swallows all exceptions indiscriminately, violating the coding guideline to NEVER try-catch-all. Handle expected transport errors (fetch failures, JSON parsing errors) explicitly and let other errors propagate for visibility into unexpected defects.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/src/lib/ai/tools/docs.ts` around lines 53 - 56, The current
catch (err) in the docs transport code blindly swallows all errors; change it to
explicitly detect and handle expected transport-related errors (e.g.,
network/fetch failures, JSON.parse/SyntaxError, TypeError from missing fields)
by checking err instanceof FetchError / SyntaxError / TypeError or by inspecting
err.name/message, call captureError("docs-tools-transport-error", err) for those
cases and return the friendly "Stack Auth docs tools are temporarily
unavailable..." message; for any other unexpected error, rethrow it so it
surfaces (do not return or swallow it). Ensure you update the catch block around
the code that uses captureError so only transport/parsing errors are handled and
all other exceptions propagate.

apps/internal-tool/.env.development-5-6 (1)

5-6: ⚠️ Potential issue | 🟠 Major

Use NEXT_PUBLIC_STACK_ prefix for these public env vars.
NEXT_PUBLIC_SPACETIMEDB_HOST and NEXT_PUBLIC_SPACETIMEDB_DB_NAME break the repo’s env naming rule. Rename to NEXT_PUBLIC_STACK_... and align all references.

Suggested rename

-NEXT_PUBLIC_SPACETIMEDB_HOST=ws://localhost:${NEXT_PUBLIC_STACK_PORT_PREFIX:-81}39
-NEXT_PUBLIC_SPACETIMEDB_DB_NAME=stack-auth-llm
+NEXT_PUBLIC_STACK_SPACETIMEDB_HOST=ws://localhost:${NEXT_PUBLIC_STACK_PORT_PREFIX:-81}39
+NEXT_PUBLIC_STACK_SPACETIMEDB_DB_NAME=stack-auth-llm

As per coding guidelines {.env*,**/*.{ts,tsx}}: All environment variables should be prefixed with STACK_ (or NEXT_PUBLIC_STACK_ if public) to ensure Turborepo picks up changes and improves readability.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/internal-tool/.env.development` around lines 5 - 6, Rename the public
environment variables NEXT_PUBLIC_SPACETIMEDB_HOST and
NEXT_PUBLIC_SPACETIMEDB_DB_NAME to use the required NEXT_PUBLIC_STACK_ prefix
(e.g., NEXT_PUBLIC_STACK_SPACETIMEDB_HOST and
NEXT_PUBLIC_STACK_SPACETIMEDB_DB_NAME) and preserve their values (keep the
ws://localhost:${NEXT_PUBLIC_STACK_PORT_PREFIX:-81}39 expression). Update every
reference to these symbols across .env* files and in all TS/TSX code (search for
NEXT_PUBLIC_SPACETIMEDB_HOST and NEXT_PUBLIC_SPACETIMEDB_DB_NAME and replace
with the new NEXT_PUBLIC_STACK_* names) so imports/uses (process.env or
next/config) remain consistent.

apps/internal-tool/.env-8-9 (1)

8-9: ⚠️ Potential issue | 🟠 Major

Rename public SpacetimeDB env keys to follow repo prefix convention.
NEXT_PUBLIC_SPACETIMEDB_HOST and NEXT_PUBLIC_SPACETIMEDB_DB_NAME violate the enforced env naming convention and can break Turborepo env change detection consistency. Please rename these to NEXT_PUBLIC_STACK_... and update consumers accordingly.

Suggested rename

-NEXT_PUBLIC_SPACETIMEDB_HOST=REPLACE_ME
-NEXT_PUBLIC_SPACETIMEDB_DB_NAME=REPLACE_ME
+NEXT_PUBLIC_STACK_SPACETIMEDB_HOST=REPLACE_ME
+NEXT_PUBLIC_STACK_SPACETIMEDB_DB_NAME=REPLACE_ME

As per coding guidelines {.env*,**/*.{ts,tsx}}: All environment variables should be prefixed with STACK_ (or NEXT_PUBLIC_STACK_ if public) to ensure Turborepo picks up changes and improves readability.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/internal-tool/.env` around lines 8 - 9, Rename the two public env keys
to use the repo prefix and update all consumers: change
NEXT_PUBLIC_SPACETIMEDB_HOST -> NEXT_PUBLIC_STACK_SPACETIMEDB_HOST and
NEXT_PUBLIC_SPACETIMEDB_DB_NAME -> NEXT_PUBLIC_STACK_SPACETIMEDB_DB_NAME in the
.env file, then update every code reference that reads the old names (e.g.,
process.env.NEXT_PUBLIC_SPACETIMEDB_HOST,
process.env.NEXT_PUBLIC_SPACETIMEDB_DB_NAME, import.meta.env or any Next.js
runtime/config usages) to the new keys, and also update any examples/docs/CI
vars that mention them.

.github/workflows/db-migration-backwards-compatibility.yaml-141-143 (1)

141-143: ⚠️ Potential issue | 🟠 Major

Guard internal-tool .env copy in base-branch scenarios.

Line 142 and Line 339 can fail when the checked-out base branch does not yet contain apps/internal-tool/.env.development, which makes migration-compat CI fail for the wrong reason.

💡 Suggested hardening

-      - name: Create .env.test.local file for apps/internal-tool
-        run: cp apps/internal-tool/.env.development apps/internal-tool/.env.test.local
+      - name: Create .env.test.local file for apps/internal-tool
+        run: |
+          if [ -f apps/internal-tool/.env.development ]; then
+            cp apps/internal-tool/.env.development apps/internal-tool/.env.test.local
+          else
+            echo "apps/internal-tool/.env.development not found; skipping."
+          fi

Also applies to: 338-340

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/db-migration-backwards-compatibility.yaml around lines 141
- 143, The workflow step named "Create .env.test.local file for
apps/internal-tool" attempts to copy apps/internal-tool/.env.development
unconditionally which fails if the base branch lacks that file; change the step
to check for the source file's existence before copying (e.g., test for the file
and only run cp when present) and apply the same guarded logic to the other
identical step later in the file so both occurrences no longer cause a hard
failure in base-branch scenarios.

apps/backend/src/lib/ai/prompts.ts-155-160 (1)

155-160: ⚠️ Potential issue | 🟠 Major

Conflicting prompt rules can cancel the verified-QA fast path.

Line 156 permits answering directly from verified QA, but Line 186 and Line 269 still require docs retrieval for every question. This contradiction can make behavior inconsistent and ignore the new priority order.

🧭 Suggested wording alignment

-**CRITICAL**: Keep responses SHORT and concise. ALWAYS use the available tools to pull relevant documentation for every question. There should almost never be a question where you don't retrieve relevant docs.
+**CRITICAL**: Keep responses SHORT and concise. Use the available tools to pull relevant documentation unless there is an exact/near-exact Human-Verified Q&A match.

@@
-2. For every question, use the available tools to retrieve the most relevant documentation sections
+2. For every question without an exact/near-exact Human-Verified Q&A match, use the available tools to retrieve the most relevant documentation sections

@@
-This is not optional - retrieve relevant documentation for every question.
+This is not optional - retrieve relevant documentation for every question that is not covered by an exact/near-exact Human-Verified Q&A match.

apps/backend/src/lib/ai/verified-qa.ts-19-45 (1)

19-45: ⚠️ Potential issue | 🟠 Major

Bound the verified-QA payload size to avoid request-path latency/context blowups.

Line 19 iterates the full table and Line 34 injects every published pair into the prompt. As the table grows, this will increase per-request latency/cost and can exceed model context limits.

Please cap the number of entries (and/or total characters/tokens) included in formatted, and prefer a deterministic subset strategy (e.g., most recent N).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/src/lib/ai/verified-qa.ts` around lines 19 - 45, The current
loop over conn.db.mcpCallLog.iter() collects all publishedToQa rows into pairs
and injects every entry into formatted, which can grow unbounded; change the
logic to include only a deterministic, bounded subset (e.g., the most recent N
entries or stop when total characters/tokens exceed a limit). Specifically, when
iterating conn.db.mcpCallLog.iter(), accumulate entries into pairs but break
once you reach a configured maxCount (e.g., MAX_VERIFIED_QA = N) or a maxChars
threshold, preferring deterministic selection like newest-by-timestamp or
iterating in reverse, and then build formatted from that bounded pairs array;
ensure any selection uses fields like publishedToQa,
humanCorrectedQuestion/humanCorrectedAnswer and preserve the existing output
ordering and rules.

apps/internal-tool/src/components/AddManualQa.tsx-92-92 (1)

92-92: ⚠️ Potential issue | 🟠 Major

Avoid voiding save promises in click handlers.

Line 92 and Line 102 currently void the async call. Replace this with the project-standard async utility (runAsynchronously / runAsynchronouslyWithAlert) so promise lifecycle/error flow is handled consistently.

As per coding guidelines, "NEVER try-catch-all, NEVER void a promise, and NEVER use .catch(console.error)or similar. UserunAsynchronouslyorrunAsynchronouslyWithAlert instead for error handling with loading indicators."

Also applies to: 102-102

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/internal-tool/src/components/AddManualQa.tsx` at line 92, The click
handlers in AddManualQa.tsx are voiding the async promise from handleSave
(onClick={() => void handleSave(false)} and the similar call at line 102);
replace these with the project async helper to preserve promise lifecycle and
error/loading handling: call runAsynchronously or runAsynchronouslyWithAlert
(per context) and pass the handleSave invocation (e.g., () => handleSave(false)
/ () => handleSave(true)) so the utility manages errors and loading indicators;
update both occurrences referencing the handleSave function and ensure you
import the appropriate runAsynchronously* helper if not already imported.

apps/backend/src/app/api/latest/internal/mcp-review/update-correction/route.ts-42-49 (1)

42-49: ⚠️ Potential issue | 🟠 Major

Avoid storing reviewer email in persisted correction metadata.

Line 48 falls back to user.primary_email, which adds unnecessary PII to the review log. Prefer a stable non-PII identifier.

🧹 Proposed fix

     await conn.reducers.updateHumanCorrection({
       token,
       correlationId: body.correlationId,
       correctedQuestion: body.correctedQuestion,
       correctedAnswer: body.correctedAnswer,
       publish: body.publish,
-      reviewedBy: user.display_name ?? user.primary_email ?? user.id,
+      reviewedBy: user.id,
     });

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@apps/backend/src/app/api/latest/internal/mcp-review/update-correction/route.ts`
around lines 42 - 49, The call to conn.reducers.updateHumanCorrection is
currently populating reviewedBy with user.display_name ?? user.primary_email ??
user.id which can persist PII (user.primary_email); change it to use a stable
non-PII identifier (for example user.id or an internal reviewerId) and remove
the email fallback. Update the payload sent to updateHumanCorrection to set
reviewedBy (or reviewerId if you prefer) to user.id (or user.display_name only
if confirmed non-PII) and, if necessary, adjust the updateHumanCorrection
reducer/signature to accept and persist that non-PII identifier instead of any
email.

apps/internal-tool/scripts/spacetime-token.mjs-15-15 (1)

15-15: ⚠️ Potential issue | 🟠 Major

Do not fall back to a static token during inject.

Line 15 defaults to "change-me". That can accidentally publish a known token. Fail fast when STACK_MCP_LOG_TOKEN is missing.

🔒 Proposed fix

 if (action === "inject") {
-  const token = process.env.STACK_MCP_LOG_TOKEN || "change-me";
+  const token = process.env.STACK_MCP_LOG_TOKEN;
+  if (token == null || token === "") {
+    console.error("STACK_MCP_LOG_TOKEN is required for inject.");
+    process.exit(1);
+  }
   if (existsSync(BACKUP)) {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/internal-tool/scripts/spacetime-token.mjs` at line 15, The constant
token currently falls back to a static string ("change-me") which is unsafe;
change the logic around the token declaration (the token constant that reads
process.env.STACK_MCP_LOG_TOKEN) to fail fast when the env var is missing by
throwing an Error or exiting with a non-zero status instead of defaulting to
"change-me" (do this where the token is defined and used, e.g., the token
constant in spacetime-token.mjs and any inject-related call sites).

apps/backend/src/app/api/latest/internal/mcp-review/delete/route.ts-7-8 (1)

7-8: ⚠️ Potential issue | 🟠 Major

Route path is outside the documented API resource hierarchy.

/api/latest/internal/mcp-review/delete does not match the required resource organization. Please move this under a documented resource namespace (or explicitly document an allowed internal exception).

As per coding guidelines: apps/backend/src/app/api/latest/**/*.{ts,tsx}: API routes should follow RESTful design patterns organized by resource type: Auth endpoints at /api/latest/auth/*, user management at /api/latest/users/*, team management at /api/latest/teams/*, and OAuth providers at /api/latest/oauth-providers/*.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/src/app/api/latest/internal/mcp-review/delete/route.ts` around
lines 7 - 8, The route exported as POST via createSmartRouteHandler in this file
uses the path /api/latest/internal/mcp-review/delete which violates the
documented API hierarchy; move this route into a documented resource namespace
(for example under /api/latest/mcp-reviews/ or another appropriate resource like
/api/latest/reviews/) by relocating the file into that resource folder and
updating any references/imports accordingly, ensure the exported symbol POST and
createSmartRouteHandler usage remain unchanged, and update any tests or callers
that reference the old path to the new resource path.

apps/backend/src/app/api/latest/integrations/ai-proxy/[[...path]]/route.ts-22-23 (1)

22-23: ⚠️ Potential issue | 🟠 Major

Use a monotonic clock for the logging duration.

startedAt = Date.now() can jump with wall-clock changes, which skews the latency that observeAndLog records. Since this endpoint feeds analytics, switch the elapsed-time path to performance.now() consistently, both in route.ts line 23 and in the duration calculations within ai-proxy-handlers.ts.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/src/app/api/latest/integrations/ai-proxy/`[[...path]]/route.ts
around lines 22 - 23, startedAt is using Date.now() which is non‑monotonic;
change the route.ts variable declaration (startedAt) to use performance.now()
and update any duration calculations in ai-proxy-handlers.ts (e.g., inside
observeAndLog) to compute elapsed as performance.now() - startedAt so all
latency logging uses the monotonic clock; keep correlationId and other logic
unchanged and ensure types/imports for performance are available where you
update startedAt and duration math.

apps/internal-tool/src/app/app-client.tsx-67-69 (1)

67-69: ⚠️ Potential issue | 🟠 Major

Require isAiChatReviewer === true here.

!metadata?.isAiChatReviewer approves any truthy JSON value. A string like "false" would pass this check (accessing a property on a string returns undefined, making !undefined true), allowing access to internal logs without proper authorization. The backend already implements explicit validation in mcp-review routes—mirror that pattern by removing the cast and checking the property's type.

Suggested fix

-    const metadata = user.clientReadOnlyMetadata as Record<string, unknown> | null;
-    if (!metadata?.isAiChatReviewer) {
+    const metadata = user.clientReadOnlyMetadata;
+    const isAiChatReviewer =
+      metadata != null &&
+      typeof metadata === "object" &&
+      "isAiChatReviewer" in metadata &&
+      metadata.isAiChatReviewer === true;
+    if (!isAiChatReviewer) {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/internal-tool/src/app/app-client.tsx` around lines 67 - 69, The current
guard inside the NODE_ENV check treats any truthy value as authorized because it
uses a cast and truthiness; replace that with an explicit boolean check against
the actual metadata object: read user.clientReadOnlyMetadata into metadata (no
cast) and require that metadata is a non-null object and
metadata.isAiChatReviewer === true before allowing access. Update the
conditional around the metadata variable (the block that currently uses
!metadata?.isAiChatReviewer) to explicitly verify typeof metadata === "object"
&& metadata !== null && metadata.isAiChatReviewer === true so only a real
boolean true permits access.

apps/internal-tool/src/app/app-client.tsx-192-200 (1)

192-200: ⚠️ Potential issue | 🟠 Major

Don't swallow these mutation failures.

Each callback converts the mutation into fire-and-forget with .catch(() => {}), preventing child components from handling errors. Return the promise from an async callback instead.

Suggested fix pattern

-                  onMarkReviewed={(correlationId) => {
-                    getApi()
-                      .then(api => api.markReviewed({ correlationId }))
-                      .catch(() => { /* errors are surfaced by UI state */ });
-                  }}
+                  onMarkReviewed={async (correlationId) => {
+                    const api = await getApi();
+                    await api.markReviewed({ correlationId });
+                  }}

Also applies to: 213-221

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/internal-tool/src/app/app-client.tsx` around lines 192 - 200, The
callbacks onSaveCorrection and onMarkReviewed currently call
getApi().then(...).catch(() => {}) which swallows mutation failures; change each
to return the promise so callers can handle errors: have onSaveCorrection return
getApi().then(api => api.updateCorrection({ correlationId, correctedQuestion,
correctedAnswer, publish })) and have onMarkReviewed return getApi().then(api =>
api.markReviewed({ correlationId })); remove the empty .catch handlers (and
apply the same change to the similar callbacks around the 213-221 block) so
errors propagate to the UI.

apps/internal-tool/src/components/CallLogList.tsx-143-154 (1)

143-154: ⚠️ Potential issue | 🟠 Major

Make sorting and row selection keyboard-accessible.

Line 146 binds sorting to a plain <th>, and Line 259 binds selection to a plain <tr>. Neither element is focusable or keyboard-operable, so keyboard users cannot sort the table or open a call from this screen. Please move these interactions onto real buttons (or add equivalent focus/keyboard handling and ARIA semantics).

Also applies to: 257-263

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/internal-tool/src/components/CallLogList.tsx` around lines 143 - 154,
The SortHeader currently attaches onClick to a non-focusable <th>; change
SortHeader to render a native <button> inside the <th> (keep the visual layout)
and move the click to that button so handleSort(field) is invoked from a
focusable control, include an accessible label for screen readers and preserve
the sort indicator. Likewise, for row selection (the clickable <tr> logic at
lines ~257-263), move the interaction into a focusable element inside each row
(e.g., a button or link that calls the existing row-open handler) or add
tabindex, role="button", and key handlers (Enter/Space) plus aria-label on the
existing control so keyboard users can activate rows; ensure existing handler
names (handleSort, SortHeader, and the row open handler) are reused and no
behavior is lost.

apps/backend/src/lib/ai/qa-reviewer.ts-133-158 (1)

133-158: ⚠️ Potential issue | 🟠 Major

Validate the full QA payload before building update.

After the partial guard, Line 142 trusts fields that were never checked. If the model omits improvementSuggestions or returns malformed flags entries, qaImprovementSuggestions/qaFlagsJson can be built from invalid data and the reducer call fails at runtime instead of falling back cleanly.

Proposed fix

+    const isObject = (value: unknown): value is Record<string, unknown> =>
+      value != null && typeof value === "object";
+
+    const isFlag = (
+      value: unknown,
+    ): value is { type: string, severity: "low" | "medium" | "high" | "critical", explanation: string } => {
+      if (!isObject(value)) return false;
+      return (
+        typeof value.type === "string" &&
+        (value.severity === "low" ||
+          value.severity === "medium" ||
+          value.severity === "high" ||
+          value.severity === "critical") &&
+        typeof value.explanation === "string"
+      );
+    };
+
     if (
       typeof raw.needsHumanReview !== "boolean" ||
       typeof raw.answerCorrect !== "boolean" ||
       typeof raw.answerRelevant !== "boolean" ||
       !Array.isArray(raw.flags) ||
+      !raw.flags.every(isFlag) ||
+      typeof raw.improvementSuggestions !== "string" ||
       typeof raw.overallScore !== "number"
     ) {
       throw new Error(`Invalid QA review response shape: ${JSON.stringify(raw).slice(0, 200)}`);
     }

As per coding guidelines "Validate all assumptions either through the type system (preferred), assertions, or tests. Ideally, use at least two out of three validation methods."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/src/lib/ai/qa-reviewer.ts` around lines 133 - 158, The current
guard validates only a subset of fields then builds update using unchecked
properties; enhance validation for parsed.flags and
parsed.improvementSuggestions before creating update: assert
parsed.improvementSuggestions is a string (or default to ""), validate
parsed.flags is an array of objects where each item has string properties type,
severity, explanation (filter out or normalize invalid entries), then set
qaFlagsJson = JSON.stringify(sanitizedFlags) and qaImprovementSuggestions =
parsed.improvementSuggestions || "" when constructing update so malformed or
missing fields cannot cause runtime errors (refer to raw, parsed, parsed.flags,
parsed.improvementSuggestions, update, qaFlagsJson, qaImprovementSuggestions).

apps/backend/src/lib/ai/openrouter-usage.ts-72-94 (1)

72-94: ⚠️ Potential issue | 🟠 Major

Parse SSE events by block, not by individual data: lines.

This loop only recognizes \n\n boundaries and then tries to JSON.parse each data: line separately. Valid SSE streams can use CRLF separators, split one event across multiple data: lines, or end without a trailing blank line. In all of those cases the usage event is skipped, so the analytics added by this PR will undercount tokens/cost.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/src/lib/ai/openrouter-usage.ts` around lines 72 - 94, The SSE
parser must treat each SSE event as a block (joining multiple "data:" lines and
handling CRLF) and also process a final event that may lack a trailing blank
line: normalize newlines (replace "\r\n" with "\n") as you append
decoder.decode(value, {stream:true}) into buffer, detect event boundaries by
"\n\n" on the normalized buffer, for each event block collect all lines starting
with "data:" and join their contents with "\n" into a single JSON string before
JSON.parse and calling mergeUsageFromEvent(acc,...), skip empty/"[DONE]" events
as before; after the reader loop finishes also parse any remaining buffer as a
final event block so partial/no trailing blank-line events are not lost (use
isUsageEmpty(acc) unchanged).

apps/internal-tool/spacetimedb/src/index.ts-3-5 (1)

3-5: ⚠️ Potential issue | 🟠 Major

Fail fast if the publish-time token was not injected.

If this module is published with EXPECTED_LOG_TOKEN still set to the checked-in placeholder, every write reducer ends up protected by a predictable public string. That turns a deployment mistake into a real write-auth bypass.

Proposed fix

-const EXPECTED_LOG_TOKEN = '__SPACETIMEDB_LOG_TOKEN__';
+const TOKEN_PLACEHOLDER = '__SPACETIMEDB_LOG_TOKEN__';
+const EXPECTED_LOG_TOKEN = TOKEN_PLACEHOLDER;
+
+if (EXPECTED_LOG_TOKEN === TOKEN_PLACEHOLDER) {
+  throw new Error("STACK_MCP_LOG_TOKEN was not injected before publishing the SpacetimeDB module");
+}

As per coding guidelines "Validate all assumptions either through the type system (preferred), assertions, or tests. Ideally, use at least two out of three validation methods."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/internal-tool/spacetimedb/src/index.ts` around lines 3 - 5, The
published placeholder EXPECTED_LOG_TOKEN ('__SPACETIMEDB_LOG_TOKEN__') must
never remain in a release; add a runtime assertion at module init to fail fast
if EXPECTED_LOG_TOKEN === '__SPACETIMEDB_LOG_TOKEN__' (throw an Error with a
clear message including EXPECTED_LOG_TOKEN) so any publish without injection
aborts startup; place this check near the top of index.ts where
EXPECTED_LOG_TOKEN is defined (use a plain throw or Node assert) and add/adjust
unit/integration tests to cover the failure path to satisfy the "at least two
validations" guideline.

apps/internal-tool/spacetimedb/src/index.ts-7-44 (1)

7-44: ⚠️ Potential issue | 🟠 Major

Add .unique() constraint to correlationId field and switch mutation reducers to keyed lookup.

The mcpCallLog table uses correlationId as the functional lookup key across all mutation reducers (lines 136, 169, 197, 260), but the schema has no uniqueness constraint. Each reducer iterates the entire table with .iter() and compares row.correlationId === args.correlationId, causing O(n) performance and risking duplicate collision behavior.

Mark correlationId: t.string().unique() in the schema, then switch these reducers to use ctx.db.mcpCallLog.correlationId.get(args.correlationId) for efficient keyed access.

Also applies to: 118-267

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/internal-tool/spacetimedb/src/index.ts` around lines 7 - 44, The
mcpCallLog schema must mark correlationId as unique and the reducers that
currently scan the table should use keyed lookups: update the mcpCallLog table
definition to change correlationId to t.string().unique() (in the table(...)
block) and in each mutation reducer that currently does table.iter() +
row.correlationId === args.correlationId (the reducers referencing
correlationId) replace that scan with
ctx.db.mcpCallLog.correlationId.get(args.correlationId) to fetch the single row
for O(1) access and remove the full-table iteration.

apps/backend/src/lib/ai/qa-reviewer.ts-173-183 (1)

173-183: ⚠️ Potential issue | 🟠 Major

Don't use .catch() to swallow promise rejections—let the error propagate to the caller.

The .catch() handler here violates the coding guideline: "NEVER use .catch(console.error) or similar. Use runAsynchronously or runAsynchronouslyWithAlert instead." This swallows the rejection, preventing proper error flow to the caller's async wrapper (runAsynchronouslyAndWaitUntil), which owns the retry/reporting path.

Use try/catch with re-throw instead. The error will still be logged via runAsynchronously() in the caller, but the promise rejection will flow correctly through the async chain.

Proposed fix

-  await conn.reducers.updateMcpQaReview({
-    token,
-    correlationId: entry.correlationId,
-    qaReviewModelId: REVIEW_MODEL_ID,
-    ...update,
-  }).catch((err: unknown) => {
-    captureError("qa-reviewer", err instanceof Error ? err : new Error(String(err)));
-  });
+  try {
+    await conn.reducers.updateMcpQaReview({
+      token,
+      correlationId: entry.correlationId,
+      qaReviewModelId: REVIEW_MODEL_ID,
+      ...update,
+    });
+  } catch (err) {
+    captureError("qa-reviewer", err instanceof Error ? err : new Error(String(err)));
+    throw err;
+  }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/src/lib/ai/qa-reviewer.ts` around lines 173 - 183, The current
use of .catch() on conn.reducers.updateMcpQaReview swallows promise rejections;
change this to an explicit try/catch around the await call in the function that
calls getConnection(), i.e., wrap the await
conn.reducers.updateMcpQaReview({...}) in try { ... } catch (err) {
captureError("qa-reviewer", err instanceof Error ? err : new
Error(String(err))); throw err; } so the error is still logged but re-thrown to
let the caller (runAsynchronouslyAndWaitUntil / runAsynchronously) handle
retries/reporting; keep the existing token, correlationId and update payload
unchanged and preserve the early return when getConnection() is falsy.

apps/backend/src/lib/ai/ai-query-handlers.ts-170-173 (1)

170-173: ⚠️ Potential issue | 🟠 Major

Log the triggering prompt instead of the first user turn.

messages.find(...) always picks the oldest user message, so multi-turn MCP logs/reviews will be attached to stale context. Use the latest user turn here, or reuse mcpCallMetadata.userPrompt, which already represents the prompt that triggered this call.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/src/lib/ai/ai-query-handlers.ts` around lines 170 - 173, The
current code picks the oldest user message via messages.find(...) resulting in
stale context; change it to use the latest user turn or directly reuse
mcpCallMetadata.userPrompt as the triggering prompt. Replace the logic that
computes firstUserMessage and question so that question is set from
mcpCallMetadata.userPrompt (or from the last element in messages with role ===
"user") and ensure you reference the existing variables messages,
firstUserMessage, question, and mcpCallMetadata.userPrompt when updating the
computation.

apps/backend/src/lib/ai/ai-query-handlers.ts-90-98 (1)

90-98: ⚠️ Potential issue | 🟠 Major

Validate toolResult.output before assigning to ContentBlock.result.

The as Json cast bypasses the type system without validating the actual value. Since buildStepsJson calls JSON.stringify directly on toolResult.output without validation, non-JSON-serializable values (BigInt, Symbol, undefined, functions, cyclic objects) will cause runtime failures in the logging path. Use isJsonSerializable to validate before storing:

result: (toolResult?.output && isJsonSerializable(toolResult.output) ? toolResult.output : null)

Aligns with coding guidelines: "Do NOT use as/any/type casts to bypass the type system" and "Validate all assumptions either through the type system (preferred), assertions, or tests."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/src/lib/ai/ai-query-handlers.ts` around lines 90 - 98, The code
is casting toolResult.output to Json and may store non-serializable values into
the ContentBlock.result; replace that cast with a runtime validation using
isJsonSerializable before assigning result. In the blocks.push call (the object
with type "tool-call", toolName/toolCallId/args/argsText), use
resultsByCallId.get(toolCall.toolCallId) to obtain toolResult and set result to
toolResult.output only if toolResult?.output exists and
isJsonSerializable(toolResult.output); otherwise set result to null, and remove
the "as Json" cast; this mirrors the validation used by buildStepsJson and
prevents runtime JSON.stringify failures.

apps/dashboard/src/lib/ai-dashboard/shared-prompt.ts-10-13 (1)

10-13: ⚠️ Potential issue | 🟡 Minor

Narrow DashboardMessage.role to a literal union instead of string.

The current type role: string weakens compile-time validation. All actual usages in the codebase construct messages with only "user" and "assistant" roles. Update to role: "user" | "assistant" to enforce this through the type system and prevent invalid role values from being introduced. This also aligns with similar message types elsewhere in the codebase (e.g., ai-conversations.ts).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/dashboard/src/lib/ai-dashboard/shared-prompt.ts` around lines 10 - 13,
Update the DashboardMessage type to narrow the role field from string to the
literal union "user" | "assistant": change the role property in the
DashboardMessage type definition so it reads role: "user" | "assistant" to match
usages and other message types (e.g., ai-conversations.ts) and ensure
compile-time validation prevents invalid roles from being created.

apps/internal-tool/src/utils.ts-7-8 (1)

7-8: ⚠️ Potential issue | 🟡 Minor

Remove the as cast and use direct property access or Reflect.get().

After the type narrowing with typeof, null check, and in operator on line 7, TypeScript should allow accessing the property without a cast. The suggested refactor using Reflect.get(ts, "__timestamp_micros_since_unix_epoch__") is a valid approach that avoids the cast while remaining fully type-safe. The return value will be unknown and is properly validated on line 9–10.

Suggested refactor

  if (typeof ts === "object" && ts !== null && "__timestamp_micros_since_unix_epoch__" in ts) {
-    const micros = (ts as Record<string, unknown>).__timestamp_micros_since_unix_epoch__;
+    const micros = Reflect.get(ts, "__timestamp_micros_since_unix_epoch__");

This aligns with the coding guideline: **/*.ts{,x}: Do NOT use as/any/type casts to bypass the type system unless explicitly asked by the user.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/internal-tool/src/utils.ts` around lines 7 - 8, After the type-narrowing
that checks typeof ts === "object", ts !== null and the "in" operator for
"__timestamp_micros_since_unix_epoch__", remove the unnecessary cast "(ts as
Record<string, unknown>)" and instead access the property in a type-safe way,
e.g. const micros = Reflect.get(ts, "__timestamp_micros_since_unix_epoch__") or
const micros = (ts as any)["__timestamp_micros_since_unix_epoch__"] replaced by
Reflect.get to avoid casts; keep the subsequent validation of micros unchanged
so it remains unknown-typed and validated on lines that follow.

apps/internal-tool/src/utils.ts-17-19 (1)

17-19: ⚠️ Potential issue | 🟡 Minor

Guard against non-finite numbers before creating Date.

When ts is NaN or Infinity, new Date(ts) creates an Invalid Date object silently, which breaks downstream sorting and filtering operations. Since the function already validates the bigint type (line 9-11) and throws for unsupported types (line 20), add a finite number check for consistency.

Suggested guard

   if (typeof ts === "number") {
+    if (!Number.isFinite(ts)) {
+      throw new TypeError(`Expected finite timestamp number, got ${ts}`);
+    }
     return new Date(ts);
   }

This aligns with the coding guideline: "Validate all assumptions either through the type system (preferred), assertions, or tests."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/internal-tool/src/utils.ts` around lines 17 - 19, In the typeof ts ===
"number" branch of the timestamp-to-Date conversion function, validate that ts
is a finite number (e.g., via Number.isFinite) before calling new Date(ts); if
it's not finite (NaN, Infinity), throw the same kind of unsupported/invalid
timestamp error used elsewhere so callers get a consistent failure mode—this
keeps behavior aligned with the existing bigint check in the function and
prevents creating Invalid Date objects during sorting/filtering.

apps/internal-tool/src/app/app-client.tsx-141-144 (1)

141-144: ⚠️ Potential issue | 🟡 Minor

Reset the usage selection in this tab handler.

This button clears selectedRow, not selectedUsageRow, so reopening the Usage tab keeps the previous usage detail pane selected.

Suggested fix

               onClick={() => {
                 setTab("usage");
-                setSelectedRow(null);
+                setSelectedUsageRow(null);
               }}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/internal-tool/src/app/app-client.tsx` around lines 141 - 144, The
onClick handler that switches to the "usage" tab calls setSelectedRow(null) but
does not clear the usage-specific selection, so the prior usage detail remains;
update the handler used when setting tab to "usage" (the onClick block invoking
setTab("usage")) to also call setSelectedUsageRow(null) to reset usage selection
(in addition to or instead of setSelectedRow) so reopening the Usage tab shows
no previously selected usage.

apps/backend/src/app/api/latest/internal/mcp-review/add-manual/route.ts-15-19 (1)

15-19: ⚠️ Potential issue | 🟡 Minor

Reject blank questions and answers at the API boundary.

defined() only guarantees the fields are present; "" and whitespace-only payloads still reach addManualQa(). That lets a bad client create empty knowledge entries or publish blank answers. Add a trim/non-empty check in the schema or a handler assertion before calling the reducer.

As per coding guidelines, "Validate all assumptions either through the type system (preferred), assertions, or tests. Ideally, use at least two out of three validation methods."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/src/app/api/latest/internal/mcp-review/add-manual/route.ts`
around lines 15 - 19, The request body schema currently uses defined() which
allows empty or whitespace-only strings; update the yup schema for question and
answer to enforce non-empty trimmed content (e.g., use
yupString().trim().min(1).defined() or .required()) and additionally add a
defensive assertion before calling addManualQa() that ensures
question.trim().length > 0 and answer.trim().length > 0 so blank payloads are
rejected at the API boundary; reference the body schema block
(yupObject/yupString) and the call site addManualQa() when making these changes.

apps/backend/src/lib/ai/mcp-logger.ts-48-59 (1)

48-59: ⚠️ Potential issue | 🟡 Minor

STACK_MCP_LOG_TOKEN throws if not configured.

Line 54 uses getEnvVariable("STACK_MCP_LOG_TOKEN") without a default, which will throw if the token is not set. This could cause unexpected failures when SpacetimeDB is configured but the log token isn't. Consider either providing a default or handling this case explicitly.

🛡️ Proposed fix: Early return if token not configured

 export async function logMcpCall(entry: McpLogEntry): Promise<void> {
   const conn = await getConnection();
   if (!conn) {
     return;
   }

-  const token = getEnvVariable("STACK_MCP_LOG_TOKEN");
+  const token = getEnvVariable("STACK_MCP_LOG_TOKEN", "");
+  if (!token) {
+    return;
+  }
+
   await conn.reducers.logMcpCall({
     token,
     ...entry,
   });
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/src/lib/ai/mcp-logger.ts` around lines 48 - 59, The call to
getEnvVariable("STACK_MCP_LOG_TOKEN") inside logMcpCall can throw if the env var
is missing; change logMcpCall to handle a missing token by calling
getEnvVariable with a safe accessor or catching the absence and doing an early
return (or no-op) before invoking conn.reducers.logMcpCall; specifically, in
function logMcpCall use a non-throwing lookup for STACK_MCP_LOG_TOKEN (or wrap
it in a try/catch) and if token is falsy return immediately so that
getConnection and conn.reducers.logMcpCall are not invoked without a valid
token.

apps/backend/src/lib/ai/ai-proxy-handlers.ts-97-122 (1)

97-122: ⚠️ Potential issue | 🟡 Minor

Use performance.now() for duration measurement instead of Date.now().

Lines 116 and 135 use Date.now() - startedAt for duration calculation. Per coding guidelines, performance.now() should be used for measuring elapsed real time as it's more precise and not subject to system clock adjustments. Additionally, the caller in apps/backend/src/app/api/latest/integrations/ai-proxy/[[...path]]/route.ts at line 23 also initializes startedAt with Date.now() and needs to be updated to use performance.now() as well.

🔧 Proposed fix

In route.ts line 23, change:

-const startedAt = Date.now();
+const startedAt = performance.now();

In ai-proxy-handlers.ts lines 116 and 135, change:

-durationMs: BigInt(Date.now() - startedAt),
+durationMs: BigInt(Math.round(performance.now() - startedAt)),

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/src/lib/ai/ai-proxy-handlers.ts` around lines 97 - 122, Change
elapsed-time measurements to use high-resolution timers: replace Date.now()
usage with performance.now() when computing duration in observeAndLog (the two
places that compute BigInt(Date.now() - startedAt) for building the log row and
scheduling the log in the streaming and non‑streaming branches), and also update
the caller that sets startedAt in the ai-proxy route handler (the initialization
in route.ts) to use performance.now() instead of Date.now(); keep all other
variables and the BigInt conversion intact so buildProxyLogRow, scheduleLog, and
scanSseForUsage behavior is unchanged.

apps/backend/src/app/api/latest/ai/query/[mode]/route.ts-73-73 (1)

73-73: ⚠️ Potential issue | 🟡 Minor

Use performance.now() instead of Date.now() for elapsed time measurement.

The startedAt value is used to calculate duration for logging. As per coding guidelines, Date.now() should not be used for measuring elapsed real time since it can be affected by system clock adjustments. Use performance.now() instead for monotonic timing.

Proposed fix

-    const startedAt = Date.now();
+    const startedAt = performance.now();

Note: Ensure the downstream logging code that calculates duration also uses performance.now() for the end time.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/src/app/api/latest/ai/query/`[mode]/route.ts at line 73, The
timing measurement uses Date.now() (const startedAt) which is susceptible to
system clock changes; replace Date.now() with performance.now() when setting
startedAt in the route handler and update any downstream duration/end-time
calculations that currently use Date.now() (or compute Date-based differences)
to use performance.now() so the logged duration is monotonic and accurate.

apps/internal-tool/src/hooks/useSpacetimeDB.ts-7-8 (1)

7-8: ⚠️ Potential issue | 🟡 Minor

Environment variables should use the NEXT_PUBLIC_STACK_ prefix.

As per coding guidelines, all environment variables should be prefixed with STACK_ (or NEXT_PUBLIC_STACK_ if public) to ensure Turborepo picks up changes and improves readability.

Suggested fix

-const rawHost = process.env.NEXT_PUBLIC_SPACETIMEDB_HOST;
-const rawDbName = process.env.NEXT_PUBLIC_SPACETIMEDB_DB_NAME;
+const rawHost = process.env.NEXT_PUBLIC_STACK_SPACETIMEDB_HOST;
+const rawDbName = process.env.NEXT_PUBLIC_STACK_SPACETIMEDB_DB_NAME;

Update corresponding .env files and the resolveEnv error messages accordingly.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/internal-tool/src/hooks/useSpacetimeDB.ts` around lines 7 - 8, The code
is reading environment vars rawHost and rawDbName from NEXT_PUBLIC_SPACETIMEDB_*
which must be renamed to the STACK-prefixed public names; update the references
in useSpacetimeDB.ts to read NEXT_PUBLIC_STACK_SPACETIMEDB_HOST and
NEXT_PUBLIC_STACK_SPACETIMEDB_DB_NAME (update any helper like resolveEnv or
error messages that mention the old names), and also update your .env files to
use those new keys so Turborepo picks up changes and the error
messages/reporting reflect the new variable names.

apps/backend/src/lib/ai/ai-query-handlers.ts-122-123 (1)

122-123: ⚠️ Potential issue | 🟡 Minor

Use a monotonic clock for these latency metrics.

These durations are part of the analytics path, so Date.now() can skew them on wall-clock adjustments. Measure elapsed time with performance.now() end-to-end and convert explicitly when persisting the value.

As per coding guidelines, "Don't use Date.now() for measuring elapsed real time. Instead use performance.now()"

Also applies to: 143-144, 186-186

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/src/lib/ai/ai-query-handlers.ts` around lines 122 - 123, Replace
wall-clock timing with a monotonic clock: capture startedAt using
performance.now() (instead of Date.now()) and compute durationMs by taking the
difference from performance.now(), converting to integer milliseconds (e.g.
Math.round or Math.floor) and wrapping with BigInt for persistence; update the
three places that set durationMs (the current occurrence where durationMs:
BigInt(Date.now() - startedAt) and the other two occurrences referenced) to use
BigInt(Math.round(performance.now() - startedAt)) and ensure any startedAt
declarations are initialized from performance.now().

apps/backend/src/app/api/latest/ai/query/[mode]/route.ts (1)

99-101: Avoid casting request messages to ModelMessage[].

The messages as ModelMessage[] assertion bypasses the last type boundary before we hand request data to the AI SDK. Prefer making requestBodySchema / the route input type produce ModelMessage[] directly, or add an explicit converter/assertion here so incompatible content parts fail loudly instead of slipping into streamText / generateText. As per coding guidelines, "Do NOT use as/any/type casts to bypass the type system unless explicitly asked by the user. Most places where you would use type casts don't actually need them."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/src/app/api/latest/ai/query/`[mode]/route.ts around lines 99 -
101, The code is unsafely casting incoming messages into ModelMessage[] when
building cachedMessages; replace the cast with proper validation/conversion so
invalid shapes fail early. Update the route's input schema (requestBodySchema)
or the route handler to parse/validate the incoming messages into ModelMessage
objects (or map/convert each message into ModelMessage) before using them, and
then construct cachedMessages = [systemMessage, ...validatedMessages]; ensure
ModeContext (ctx) receives the validated array so downstream functions like
streamText/generateText only get well-typed ModelMessage instances rather than
relying on an "as" cast.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

apps/internal-tool/src/components/KnowledgeBase.tsx (2)

126-127: Minor accessibility gaps in ConfirmDialog.

The modal has no role="dialog"/aria-modal, no Escape-key handler, and no focus management, so keyboard users can't easily dismiss it and screen readers won't announce it. For an internal tool this is low priority, but adding a keydown listener for Escape and an aria-modal="true" + role="dialog" would be a cheap win.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/internal-tool/src/components/KnowledgeBase.tsx` around lines 126 - 127,
The modal wrapper currently lacks accessibility attributes and an Escape-key
handler; update the ConfirmDialog (the outer div that uses onCancel) to include
role="dialog" and aria-modal="true", add a ref to the dialog container and call
focus() on mount (or focus the first focusable element) to set initial focus,
and register a keydown listener (via useEffect) that calls the existing onCancel
when Escape is pressed; ensure you also stopPropagation on inner content
(existing onClick stop) and clean up the event listener on unmount.

---

274-285: Consider dropping the "Edit" confirmation.

Unlike publish/unpublish/delete, opening the inline editor is a non-destructive action (the user can still cancel without saving). Requiring a confirm dialog here adds friction without a safety benefit — consider wiring the Edit button directly to onStartEdit() and keeping ConfirmDialog only for publish/unpublish/delete.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/internal-tool/src/components/KnowledgeBase.tsx` around lines 274 - 285,
Remove the confirmation step for edit actions by deleting the conditional block
that renders ConfirmDialog when pending === "edit" and instead invoke
onStartEdit() directly from the Edit button handler; update any handlers that
previously setPending("edit") to call onStartEdit() (and clear pending state if
needed via setPending(null) inside the Edit button flow), leaving ConfirmDialog
usage intact only for "publish"/"unpublish"/"delete" flows (symbols to change:
pending, ConfirmDialog, setPending, onStartEdit).

apps/internal-tool/src/lib/mcp-review-api.ts (1)

67-67: Use the project URL helper for this new endpoint.

This newly added fetch URL uses normal string interpolation. Please switch it to the repo’s URL helper pattern, or otherwise encode dynamic URL pieces consistently. As per coding guidelines, use urlString`` or encodeURIComponent()` instead of normal string interpolation for URLs, for consistency even if it's not strictly necessary.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/internal-tool/src/lib/mcp-review-api.ts` at line 67, The fetch call
assigned to `res` currently uses a plain template literal for the endpoint;
replace it with the repo URL helper so dynamic URL pieces are encoded
consistently—use the `urlString` template tag (e.g.,
urlString`/api/latest/internal/spacetimedb-enroll-reviewer`) or, if you must
manually build the path, wrap any dynamic segments with `encodeURIComponent()`;
update the `fetch` invocation that references
`API_URL`/`spacetimedb-enroll-reviewer` to use that helper so URL encoding and
project conventions are preserved.

apps/backend/src/lib/ai/mcp-logger.ts (1)

55-57: Encode URL path components before constructing SpacetimeDB endpoints.

dbName comes from env and reducer is a dynamic helper argument; encode them before placing them in the path.

Suggested fix

-  const dbName = getEnvVariable("STACK_SPACETIMEDB_DB_NAME");
-  const res = await fetch(`${base}/v1/database/${dbName}/call/${reducer}`, {
+  const dbName = encodeURIComponent(getEnvVariable("STACK_SPACETIMEDB_DB_NAME"));
+  const reducerName = encodeURIComponent(reducer);
+  const res = await fetch(`${base}/v1/database/${dbName}/call/${reducerName}`, {

-  const dbName = getEnvVariable("STACK_SPACETIMEDB_DB_NAME");
-  const res = await fetch(`${base}/v1/database/${dbName}/sql`, {
+  const dbName = encodeURIComponent(getEnvVariable("STACK_SPACETIMEDB_DB_NAME"));
+  const res = await fetch(`${base}/v1/database/${dbName}/sql`, {

As per coding guidelines, “Use urlString`` or encodeURIComponent() instead of normal string interpolation for URLs, for consistency even if it's not strictly necessary”.

Also applies to: 96-98

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/src/lib/ai/mcp-logger.ts` around lines 55 - 57, The URL path is
constructed with unescaped values dbName and reducer in the fetch call inside
mcp-logger (e.g., the const dbName = getEnvVariable(...) and the POST to
`${base}/v1/database/${dbName}/call/${reducer}`); encode both path components
before interpolation (use encodeURIComponent() or your project's urlString``
helper) so the SpacetimeDB endpoint is safe; apply the same change to the other
fetch usage around lines 96-98 that builds a similar path.

apps/backend/src/lib/ai/ai-query-handlers.ts (1)

91-98: Validate or normalize tool outputs instead of casting to Json.

toolResult?.output is not proven JSON-safe here; the cast can hide values that later fail response/log serialization. Normalize through an existing JSON utility or add a small serializer boundary before putting it into ContentBlock.

As per coding guidelines, “Do NOT use as/any/type casts or anything else like that to bypass the type system.”

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/backend/src/lib/ai/ai-query-handlers.ts` around lines 91 - 98, The code
is unsafely casting toolResult?.output to Json when building the "tool-call"
ContentBlock; replace the cast with a real serialization/validation boundary:
locate the array push that creates the block (the object with type: "tool-call",
toolName, toolCallId, argsText, result) and run toolResult?.output through an
existing JSON sanitizer/serializer utility (or JSON.stringify then parse back
via a safeDeserialize helper) to ensure the value is valid JSON or normalized
(and handle/omit non-serializable values or fallback to null), then assign that
sanitized value to result instead of using (toolResult?.output as Json).

apps/internal-tool/src/components/Usage.tsx (1)

226-229: Don’t silently drop malformed tool-usage data.

If requestedToolsJson is invalid, the dashboard currently hides that row from tool analytics with no signal. Surface an “invalid tools JSON” bucket or route the parse through a shared safe parser with explicit error handling.

As per coding guidelines, “NEVER try-catch-all” and “NEVER just silently use fallback values when you don't know how to fix type errors.”

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@apps/internal-tool/src/components/Usage.tsx` around lines 226 - 229, The
current try/catch around JSON.parse(requestedToolsJson) swallows parse errors
and hides rows; change it to use an explicit safe parse and surface malformed
entries by catching the error into a named variable and incrementing a dedicated
bucket (e.g. toolCounts.set("invalid_tools_json", ...)) or calling the shared
safe parser (e.g. parseJsonSafe) that returns an error result; also log the
parse error with context (requestedToolsJson and record id) instead of an empty
catch. Update the block that references requestedToolsJson and toolCounts so
malformed JSON increments the "invalid_tools_json" counter and the error is
reported rather than silently ignored.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes