Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughRefactors AI integrations into a unified OpenRouter-based backend flow: adds model selection, system prompts, tool factories (MCP, SQL, email tools), new AI query route with stream/generate modes and production-forwarding, removes adapter-based chat modules, and updates client-side chat adapters and interfaces accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client/Dashboard
participant Backend as Backend API
participant OpenRouter as OpenRouter
participant Docs as Docs MCP
participant Analytics as ClickHouse
Client->>Backend: POST /api/latest/ai/query/[mode] (messages, tools, systemPrompt, quality, speed)
Backend->>Backend: validate request & auth
Backend->>Backend: selectModel(quality, speed, isAuthenticated)
Backend->>Docs: request docs tools (if requested)
Docs-->>Backend: docs tool/context
Backend->>Analytics: prepare SQL client (if sql-query requested)
Backend->>OpenRouter: forward model + messages + tools
OpenRouter-->>Backend: AI response (text + tool-call blocks)
alt mode == "stream"
Backend-->>Client: stream response (passthrough)
else mode == "generate"
Backend->>Backend: assemble content blocks (text, tool calls, results)
Backend-->>Client: JSON { content: [...] }
end
sequenceDiagram
participant Client as Client
participant Backend as Backend API
participant Prod as Production API (FORWARD_TO_PRODUCTION)
participant OpenRouter as OpenRouter
Client->>Backend: POST /api/latest/ai/query/[mode]
Backend->>Backend: read STACK_OPENROUTER_API_KEY
alt STACK_OPENROUTER_API_KEY == "FORWARD_TO_PRODUCTION"
Backend->>Prod: forwardToProduction(headers, mode, body)
Prod->>OpenRouter: complete AI orchestration
OpenRouter-->>Prod: AI response
Prod-->>Backend: proxied response
Backend-->>Client: passthrough response
else
Backend->>OpenRouter: local model selection + query
OpenRouter-->>Backend: AI response
Backend-->>Client: formatted local response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Greptile SummaryThis PR consolidates all AI functionality into a single unified backend endpoint ( Key changes:
Issues found:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client as Client (Dashboard/Docs)
participant DashboardAPI as Dashboard/Docs API
participant UnifiedEndpoint as Backend AI Endpoint
participant Forward as Production Forwarder
participant OpenRouter as OpenRouter API
participant Tools as AI Tools
Client->>DashboardAPI: POST /api/email-ai or /api/ai-search
DashboardAPI->>DashboardAPI: Authenticate user & verify project access
DashboardAPI->>UnifiedEndpoint: POST /api/latest/ai/query/[mode]
Note over DashboardAPI,UnifiedEndpoint: Include auth headers (x-stack-*)
UnifiedEndpoint->>UnifiedEndpoint: Validate tool names
UnifiedEndpoint->>UnifiedEndpoint: Get STACK_OPENROUTER_API_KEY
alt API key is "FORWARD_TO_PRODUCTION"
UnifiedEndpoint->>Forward: Forward request
Forward->>Forward: Filter x-stack-* headers
Forward->>UnifiedEndpoint: POST to api.stack-auth.com
UnifiedEndpoint-->>DashboardAPI: Return production response
else API key is valid
UnifiedEndpoint->>UnifiedEndpoint: Select model (quality/speed/auth)
UnifiedEndpoint->>UnifiedEndpoint: Get system prompt by ID
UnifiedEndpoint->>Tools: Load requested tools
alt mode == "stream"
UnifiedEndpoint->>OpenRouter: streamText()
OpenRouter-->>UnifiedEndpoint: Stream response
UnifiedEndpoint-->>DashboardAPI: Pipe stream
DashboardAPI-->>Client: Stream UI updates
else mode == "generate"
UnifiedEndpoint->>OpenRouter: generateText()
OpenRouter->>Tools: Execute tool calls
Tools-->>OpenRouter: Tool results
OpenRouter-->>UnifiedEndpoint: Complete response
UnifiedEndpoint->>UnifiedEndpoint: Collect text + tool calls
UnifiedEndpoint-->>DashboardAPI: JSON response
DashboardAPI->>DashboardAPI: Sanitize generated code
DashboardAPI-->>Client: Return result
end
end
Last reviewed commit: c6caf88 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (18)
docs/src/components/chat/ai-chat.tsx-605-614 (1)
605-614:⚠️ Potential issue | 🟠 Major
astype casts onpart.inputandpart.outputviolate the no-cast guideline.
DynamicToolUIPart.inputandDynamicToolUIPart.outputare typed asunknownby the SDK — correct per its design, since tool schemas are not known at compile time. However, casting them withasbypasses the type system without any runtime validation, which violates the coding guideline.Two options, in order of preference:
- Pass
unknowndown and validate insideToolCallDisplay— widenToolCallDisplay's prop types to acceptunknown, then use runtime narrowing there.- Narrow here with a type guard before constructing the prop object.
♻️ Option 2: runtime-narrow before passing
- <ToolCallDisplay - key={index} - toolCall={{ - toolName: part.toolName, - args: part.input as { id?: string, search_query?: string }, - result: part.output as { content?: { text: string }[], text?: string } | undefined, - }} - /> + <ToolCallDisplay + key={index} + toolCall={{ + toolName: part.toolName, + args: (typeof part.input === 'object' && part.input !== null) + ? (part.input as { id?: string, search_query?: string }) + : {}, + result: (typeof part.output === 'object' && part.output !== null) + ? (part.output as { content?: { text: string }[], text?: string }) + : undefined, + }} + />Note: even option 2 still retains
asafter the guard, which the guideline discourages. The cleanest solution is option 1 — changeToolCallDisplay's prop types tounknownand do the checks once inside it. As per coding guidelines: "Do NOT useas/any/type casts or anything else to bypass the type system."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/src/components/chat/ai-chat.tsx` around lines 605 - 614, The current map over toolInvocations casts DynamicToolUIPart.input and output with "as", which violates the no-cast rule; instead, change ToolCallDisplay's props to accept unknown for args/result (update ToolCallDisplay prop types to args: unknown and result: unknown) and move runtime validation/narrowing into ToolCallDisplay (use type guards/shape checks there) so you can pass toolCall={{ toolName: part.toolName, args: part.input, result: part.output }} without any "as" casts; alternatively, if you must narrow here, implement explicit type guard functions for DynamicToolUIPart.input/output and only construct the prop object after those guards succeed.apps/backend/src/route-handlers/smart-response.tsx-116-118 (1)
116-118:⚠️ Potential issue | 🟠 MajorPreserve duplicate header values when copying from
Response.The current code overwrites multi-value headers. Since
headersis aMap<string, string[]>(), successive calls toheaders.set(key, [value])for the same header name will replace the entire array instead of accumulating values. This truncates headers likeSet-Cookiethat appear multiple times inobj.body.headers.entries().Use conditional logic to append to existing header arrays:
for (const [key, value] of obj.body.headers.entries()) { - headers.set(key.toLowerCase(), [value]); + const normalizedKey = key.toLowerCase(); + const existing = headers.get(normalizedKey); + if (existing == null) { + headers.set(normalizedKey, [value]); + } else { + existing.push(value); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/route-handlers/smart-response.tsx` around lines 116 - 118, The loop copying headers from obj.body.headers.entries() currently calls headers.set(key.toLowerCase(), [value]) which overwrites existing arrays and loses duplicate header values (e.g., Set-Cookie); update the logic in smart-response.tsx where headers is populated so it checks headers.get(keyLower) first and, if present, appends the new value to that array, otherwise sets a new array with the value—use the same key normalization (key.toLowerCase()) and preserve ordering of values.apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-templates/[templateId]/page-client.tsx-28-28 (1)
28-28:⚠️ Potential issue | 🟠 MajorUse generic syntax for
useParams()instead of type casting.Line 28 uses
as { projectId: string }to cast the result. Replace with the proper generic syntax:useParams<{ projectId: string }>(). This validates at compile-time without bypassing the type system and aligns with Next.js App Router conventions and the coding guideline against unnecessary type casts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/email-templates/[templateId]/page-client.tsx at line 28, Replace the type assertion on the useParams call with the generic form to get proper compile-time typing: locate the line where useParams() is cast using "as { projectId: string }" (the const { projectId } = useParams() statement) and change it to use the generic type parameter form so the hook returns the correctly typed params instead of bypassing the type system.apps/dashboard/src/components/vibe-coding/chat-adapters.ts-62-63 (1)
62-63:⚠️ Potential issue | 🟠 MajorFail fast on malformed AI responses instead of casting and defaulting to
[].Line 62 bypasses type safety via
as, and Line 63 silently masks response-shape errors by returning empty content. This can hide backend regressions and produce misleading "empty AI response" behavior.🔧 Proposed fix
+type AiResponse = { content: ChatContent }; + +function assertAiResponse(value: unknown): asserts value is AiResponse { + if (typeof value !== "object" || value == null || !("content" in value) || !Array.isArray(value.content)) { + throw new Error("Invalid AI response shape: expected { content: ChatContent }"); + } +} + -const result = await response.json() as { content?: ChatContent }; -const content: ChatContent = Array.isArray(result.content) ? result.content : []; +const result: unknown = await response.json(); +assertAiResponse(result); +const content = result.content;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/vibe-coding/chat-adapters.ts` around lines 62 - 63, The code currently force-casts response.json() to { content?: ChatContent } and defaults malformed content to [] (variables: response, result, content, ChatContent), which hides invalid AI responses; replace the cast-and-default with explicit validation: parse await response.json() into a temporary any, verify that result.content exists and is an array of expected items (or otherwise matches ChatContent shape), and if not throw a descriptive error (include response.status and a short serialized body snippet) so callers fail fast instead of receiving an empty array.apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-themes/[themeId]/page-client.tsx-22-22 (1)
22-22:⚠️ Potential issue | 🟠 MajorReplace route-param cast with the generic useParams API.
Line 22 uses
asto forceprojectIdtyping. Instead, useuseParams<{ projectId: string }>()to get proper type safety without a cast. This aligns with Next.js App Router's intended API and eliminates the type bypass that violates coding guidelines.const { projectId } = useParams<{ projectId: string }>();This pattern should be corrected consistently across the codebase (also appears in email-templates and email-drafts files).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/email-themes/[themeId]/page-client.tsx at line 22, Replace the forced type cast on useParams by switching to the generic useParams API: locate the line where useParams is called and projectId is extracted (symbol: useParams and variable projectId) and change the pattern from "useParams() as { projectId: string }" to using the generic form useParams<{ projectId: string }>() so TypeScript infers projectId correctly; apply the same replacement in the related files that use the same pattern (email-templates and email-drafts) to keep typing consistent across the codebase.apps/backend/src/lib/ai/prompts.ts-337-337 (1)
337-337:⚠️ Potential issue | 🟠 MajorDraft prompt still instructs the wrong tool name.
email-assistant-drafttells the model to callcreateEmailTemplate, but this flow now usescreateEmailDraft. This can produce invalid tool calls and fail request validation.Suggested fix
-- YOU MUST WRITE A FULL REACT COMPONENT WHEN CALLING THE createEmailTemplate TOOL. +- YOU MUST WRITE A FULL REACT COMPONENT WHEN CALLING THE createEmailDraft TOOL.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/lib/ai/prompts.ts` at line 337, The prompt template named email-assistant-draft still instructs the model to call createEmailTemplate; update that instruction to call createEmailDraft instead (and adjust any example tool call snippets in the same template to match the createEmailDraft tool signature and payload keys), so generated tool calls validate correctly; search for the string "createEmailTemplate" inside the email-assistant-draft prompt and replace it with "createEmailDraft" and verify the surrounding example JSON matches the actual createEmailDraft parameters.apps/backend/src/lib/ai/tools/docs.ts-12-14 (1)
12-14:⚠️ Potential issue | 🟠 MajorDo not silently route unknown environments to production MCP.
Right now, any non-
developmentenvironment (includingtest) is treated as production. This can accidentally hit production infra from CI/staging. Handleproductionexplicitly and throw on unexpected values.Suggested fix
export async function createDocsTools() { - const mcpUrl = - getNodeEnvironment() === "development" - ? new URL("/api/internal/mcp", "http://localhost:8104") - : new URL("/api/internal/mcp", "https://mcp.stack-auth.com"); + const nodeEnv = getNodeEnvironment(); + const mcpUrl = + nodeEnv === "development" || nodeEnv === "test" + ? new URL("/api/internal/mcp", "http://localhost:8104") + : nodeEnv === "production" + ? new URL("/api/internal/mcp", "https://mcp.stack-auth.com") + : (() => { + throw new Error(`Unsupported NODE_ENV for docs MCP client: ${nodeEnv}`); + })();As per coding guidelines, "Fail early, fail loud. Fail fast with an error instead of silently continuing."
🤖 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 12 - 14, The code currently treats any non-development environment as production when constructing the MCP URL; update the logic around getNodeEnvironment() so it explicitly handles "development" (use new URL("/api/internal/mcp", "http://localhost:8104")) and "production" (use new URL("/api/internal/mcp", "https://mcp.stack-auth.com")) and throws an error for any other unexpected environment value (do not silently fallback), referencing the existing getNodeEnvironment() call and the new URL("/api/internal/mcp", ...) construction so the change is applied in the same assignment site.docs/src/app/api/chat/route.ts-34-47 (1)
34-47:⚠️ Potential issue | 🟠 MajorAdd a timeout guard to the upstream fetch.
This backend call has no timeout, so network stalls can keep the route open indefinitely and degrade capacity under failure conditions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/src/app/api/chat/route.ts` around lines 34 - 47, The fetch to `${backendBaseUrl}/api/latest/ai/query/stream` (creating backendResponse) lacks a timeout guard; wrap the request in an AbortController, pass controller.signal to the fetch call used with requestHeaders and body (the same payload: quality/speed/systemPrompt/tools/messages), start a setTimeout to call controller.abort() after a sensible timeout (e.g., 10–30s), and clear the timer on success; handle the abort by catching the thrown AbortError and returning the existing errorResponse fallback so stalled requests are terminated and the route doesn't hang.apps/dashboard/src/app/api/email-ai/route.ts-79-91 (1)
79-91:⚠️ Potential issue | 🟠 MajorAdd a timeout to the backend generate call.
Without an abort/timeout, upstream stalls can hang this API request and tie up server resources.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/api/email-ai/route.ts` around lines 79 - 91, The fetch to `${backendBaseUrl}/api/latest/ai/query/generate` (producing backendResponse) lacks an abort/timeout and can hang; update the route handler in route.ts to wrap the fetch with an AbortController: create a controller, pass controller.signal to fetch, start a timer (e.g., setTimeout) that calls controller.abort() after a configurable timeout (e.g., 10–30s), and clear the timer after fetch completes; also catch AbortError (or check controller.signal.aborted) and return/throw a descriptive 504 Gateway Timeout response instead of leaving the request hanging.docs/src/app/api/chat/route.ts-7-9 (1)
7-9:⚠️ Potential issue | 🟠 MajorDo not silently default docs AI traffic to production.
If
NEXT_PUBLIC_STACK_API_URLis missing, this route sends chat payloads to production by default. Treat missing config as an explicit failure instead of silently rerouting data.As per coding guidelines "Fail early, fail loud. Fail fast with an error instead of silently continuing."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/src/app/api/chat/route.ts` around lines 7 - 9, The current assignment of backendBaseUrl uses a silent default to "https://api.stack-auth.com" when process.env.NEXT_PUBLIC_STACK_API_URL is missing; change this to fail loudly by validating process.env.NEXT_PUBLIC_STACK_API_URL at startup or inside the route and throwing a clear error (or returning a 500) if it's undefined, so update the logic around the backendBaseUrl constant to require NEXT_PUBLIC_STACK_API_URL and surface an explicit error message instead of silently falling back to production.apps/backend/src/lib/ai/tools/create-email-draft.ts-25-43 (1)
25-43:⚠️ Potential issue | 🟠 MajorPrompt example conflicts with the required email structure.
The description requires
<Html>,<Head />,<Preview />,<Tailwind>,<Body>, and<Container>, but the example only shows<Container>. This inconsistency will drive invalid generations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/lib/ai/tools/create-email-draft.ts` around lines 25 - 43, The prompt/example in create-email-draft is inconsistent: the required email structure mandates using <Html>, <Head />, <Preview />, <Tailwind>, <Body>, and <Container> but the example only renders <Container>, which will produce invalid drafts; update the example/template (e.g., the EmailTemplate or example return value) to include the full hierarchy—wrap content in <Html><Tailwind><Head /><Body><Preview /> and then <Container>—and ensure only inlineable Tailwind utilities are used (no hover:, focus:, media queries, dark:, group-hover:, etc.) and that <Head /> is placed inside <Tailwind> so style injection works.apps/backend/src/lib/ai/tools/sql-query.ts-14-18 (1)
14-18:⚠️ Potential issue | 🟠 MajorEnforce SQL constraints in code, not only in prompt text.
The description says “only SELECT” and “include LIMIT”, but
executeruns any input query string. Add explicit guards beforeclient.queryto enforce that contract.Suggested guard
execute: async ({ query }: { query: string }) => { + const normalized = query.trim().toLowerCase(); + const isSelectLike = + normalized.startsWith("select") || normalized.startsWith("with"); + const hasLimit = /\blimit\s+\d+\b/i.test(query); + if (!isSelectLike || !hasLimit) { + return { + success: false as const, + error: "Only SELECT/WITH...SELECT queries with an explicit LIMIT are allowed", + }; + } + const client = getClickhouseExternalClient(); return await client.query({Also applies to: 21-34
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/lib/ai/tools/sql-query.ts` around lines 14 - 18, Add runtime guards to enforce the schema's SQL constraints (only SELECT and must include LIMIT) before calling client.query: in the execute function in sql-query.ts validate the provided query string (from the input matching inputSchema) by trimming whitespace, checking that it begins with "SELECT" (case-insensitive) and that it contains a LIMIT clause (case-insensitive) and reject/throw an error for any query that fails these checks (also consider disallowing multiple statements or trailing semicolons). Ensure the check runs immediately before the call to client.query so the contract described in inputSchema.describe is enforced in code.apps/backend/.env.development-60-60 (1)
60-60:⚠️ Potential issue | 🟠 MajorAvoid making production forwarding the default in committed dev env.
STACK_OPENROUTER_API_KEY=FORWARD_TO_PRODUCTIONcauses local AI traffic to hit production by default, which is risky for privacy/cost and weakens local isolation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/.env.development` at line 60, The dev env currently sets STACK_OPENROUTER_API_KEY=FORWARD_TO_PRODUCTION which forwards local AI traffic to production; change the STACK_OPENROUTER_API_KEY value in the development env to an empty value or a non-production placeholder (e.g. leave blank or use a local/test key) and add a short inline comment warning not to forward to production so committed dev envs never default to production credentials.apps/backend/src/lib/ai/tools/create-email-template.ts-25-54 (1)
25-54:⚠️ Potential issue | 🟠 MajorExample template violates the hierarchy rules defined above it.
You require
<Html>/<Head>/<Preview>/<Tailwind>/<Body>/<Container>, but the example does not follow that structure. Models usually follow examples over prose constraints.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/lib/ai/tools/create-email-template.ts` around lines 25 - 54, The example EmailTemplate violates the required email component hierarchy: wrap the template return in <Html> with <Tailwind> (and render <Head /> inside <Tailwind>), then <Preview />, <Body>, and finally <Container> containing your content; update the function EmailTemplate (and its export EmailTemplate.PreviewVariables) to follow this Html -> Tailwind(Head) -> Preview -> Body -> Container structure and remove any non-inlineable Tailwind utilities (hover:, focus:, dark:, media queries, group-*, etc.) so only inlineable utilities are used; ensure Subject and NotificationCategory remain inside the Container as shown.apps/dashboard/src/app/api/email-ai/route.ts-50-53 (1)
50-53:⚠️ Potential issue | 🟠 MajorDo not forward client-selected
systemPromptandtoolsin an admin-scoped request.These fields are user-controlled and are forwarded unchanged while using admin access headers. Enforce a server-side allowlist here to keep this endpoint constrained to intended email tooling.
Also applies to: 59-59, 89-89
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/api/email-ai/route.ts` around lines 50 - 53, The current handler reads client-controlled fields payload.systemPrompt and payload.tools and forwards them in an admin-scoped request; instead enforce a server-side allowlist and never forward raw client values. Replace use of payload.systemPrompt and payload.tools when building the admin request (the code that issues the admin-scoped fetch) with validated/allowed values: derive allowedSystemPrompt and allowedTools on the server (e.g., lookup by payload.projectId or use a fixed server-side map), validate payload.tools against that allowlist and discard or replace invalid entries, then send only allowedSystemPrompt and allowedTools in the admin request; keep payload.projectId (and other safe fields) as before and remove any direct forwarding of payload.systemPrompt or payload.tools.apps/dashboard/src/app/api/ai-search/route.ts-68-70 (1)
68-70:⚠️ Potential issue | 🟠 MajorRetry without admin auth only for auth failures.
Line 68-70 retries on any non-OK response. This should be constrained to
401/403; otherwise backend errors are retried unnecessarily and may hide root causes.💡 Suggested fix
- if (!backendResponse.ok && useAdminAuth) { + if (useAdminAuth && (backendResponse.status === 401 || backendResponse.status === 403)) { backendResponse = await makeRequest(false); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/api/ai-search/route.ts` around lines 68 - 70, The current retry logic retries any non-OK response when useAdminAuth is true; change it so it only retries when the backendResponse status indicates an auth failure (401 or 403). After the initial request, inspect backendResponse.status and only call makeRequest(false) to overwrite backendResponse if status === 401 || status === 403; leave other non-OK responses untouched so we don't mask backend errors. Use the existing variables backendResponse, useAdminAuth, and makeRequest to implement this conditional retry.apps/backend/src/lib/ai/schema.ts-16-18 (1)
16-18:⚠️ Potential issue | 🟠 Major
systemPromptallows an unsupported value that is not implemented in prompt resolution.Line 16-18 includes
"edit-dashboard", but the backend prompt ID union inapps/backend/src/lib/ai/prompts.tsdoes not include it. This lets invalid prompt IDs pass schema validation and fail later in runtime behavior.💡 Suggested fix
systemPrompt: yupString().oneOf([ "command-center-ask-ai", "docs-ask-ai", "wysiwyg-edit", "email-wysiwyg-editor", "email-assistant-template", "email-assistant-theme", "email-assistant-draft", "create-dashboard", - "edit-dashboard", "run-query", ]).defined(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/lib/ai/schema.ts` around lines 16 - 18, The schema's systemPrompt allowed values include "edit-dashboard", which is not present in the backend prompt ID union in prompts.ts; update them to match by either removing "edit-dashboard" from the systemPrompt enum in apps/backend/src/lib/ai/schema.ts or adding a corresponding prompt ID and handler in apps/backend/src/lib/ai/prompts.ts (e.g., add "edit-dashboard" to the Prompt ID union and implement its resolution), and ensure the two sources remain in sync so only supported prompt IDs pass validation.apps/backend/src/app/api/latest/internal/wysiwyg-edit/route.tsx-171-176 (1)
171-176:⚠️ Potential issue | 🟠 MajorDo not silently return original source when production response has no text block.
Line 175 falls back to
source_codeif the forwarded response has no valid text content. That turns malformed upstream responses into false-success 200s.💡 Suggested fix
- const updatedSource = stripCodeFences(textBlock?.text?.trim() ?? source_code); + const text = textBlock?.text?.trim(); + if (text == null || text === "") { + throw new StatusError( + StatusError.BadGateway, + "Production AI response did not contain a text content block." + ); + } + const updatedSource = stripCodeFences(text);As per coding guidelines, "Fail early, fail loud. Fail fast with an error instead of silently continuing."
🤖 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/wysiwyg-edit/route.tsx` around lines 171 - 176, The code silently falls back to source_code when prodResult has no valid text block; instead detect when textBlock is missing (check prodResult.content and textBlock from the existing variables prodResult and textBlock) and fail fast—throw an error or return an error response (e.g., throw new Error or return a 5xx/4xx NextResponse) rather than setting updatedSource to source_code; update the logic around updatedSource/stripCodeFences to only run when textBlock?.text exists and ensure callers surface the error instead of returning a 200 with the original source_code.
🟡 Minor comments (3)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-drafts/[draftId]/page-client.tsx-19-19 (1)
19-19:⚠️ Potential issue | 🟡 MinorReplace
aswith runtime validation; use?? throwErr()for defensive programming.Line 19 uses a type cast that bypasses type safety. The codebase explicitly requires defensive programming with proper runtime validation. Replace the cast with explicit type checking and fail loudly if the expected shape is violated, consistent with the established
?? throwErr(...)pattern used throughout the codebase.🔧 Proposed fix
-const { projectId } = useParams() as { projectId: string }; +const params = useParams<{ projectId?: string | string[] }>(); +const projectIdParam = params.projectId; +if (typeof projectIdParam !== "string") { + throw new Error("Expected route param `projectId` to be a single string"); +} +const projectId = projectIdParam;Per coding guidelines: avoid
ascasts and prefer?? throwErr(...)with explicit assumptions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/email-drafts/[draftId]/page-client.tsx at line 19, Replace the unsafe type assertion on useParams with runtime validation: stop using "useParams() as { projectId: string }" and instead read params from useParams(), then assert projectId exists at runtime and call throwErr(...) if it does not; e.g., get params via useParams(), extract params.projectId, and use "?? throwErr('Missing projectId')" to fail loudly (reference symbols: useParams, projectId variable, throwErr).apps/backend/src/lib/ai/tools/create-email-theme.ts-20-21 (1)
20-21:⚠️ Potential issue | 🟡 MinorTheme tool instructions conflict with the renderer props contract.
This says
EmailThememust take onlychildren, but the theme prompt contract includes optionalunsubscribeLink. Aligning these instructions will reduce inconsistent model outputs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/lib/ai/tools/create-email-theme.ts` around lines 20 - 21, The EmailTheme implementation must be adjusted so its component signature accepts only a single prop, children (React node), and must not reference or accept unsubscribeLink; update the EmailTheme function/component in create-email-theme.ts to remove any unsubscribeLink prop usage and only render children, and ensure imports are limited to "@react-email/components" (remove any other package imports); alternatively, if the renderer contract should include unsubscribeLink, update the prompt/contract instead so both the EmailTheme component and renderer agree on the same props (but do not leave mismatched props between EmailTheme and the renderer).apps/backend/src/lib/ai/tools/create-email-template.ts-21-22 (1)
21-22:⚠️ Potential issue | 🟡 MinorComplete the truncated
PreviewVariablesinstruction.The requirement text ends mid-instruction (
EmailTemplate.PreviewVariables = { ...), which leaves the expected output format ambiguous.
🧹 Nitpick comments (4)
apps/dashboard/src/components/vibe-coding/chat-adapters.ts (1)
15-19: Prefer an ES6Mapfor context lookup.
CONTEXT_MAPis currently a record-like object; the project guideline prefersMapfor key/value lookup tables.♻️ Proposed refactor
-const CONTEXT_MAP = { - "email-theme": { systemPrompt: "email-assistant-theme", tools: ["create-email-theme"] }, - "email-template": { systemPrompt: "email-assistant-template", tools: ["create-email-template"] }, - "email-draft": { systemPrompt: "email-assistant-draft", tools: ["create-email-draft"] }, -} as const; +const CONTEXT_MAP = new Map([ + ["email-theme", { systemPrompt: "email-assistant-theme", tools: ["create-email-theme"] }], + ["email-template", { systemPrompt: "email-assistant-template", tools: ["create-email-template"] }], + ["email-draft", { systemPrompt: "email-assistant-draft", tools: ["create-email-draft"] }], +] as const); ... -const { systemPrompt, tools } = CONTEXT_MAP[contextType]; +const contextConfig = CONTEXT_MAP.get(contextType); +if (contextConfig == null) { + throw new Error(`Unsupported context type: ${contextType}`); +} +const { systemPrompt, tools } = contextConfig;As per coding guidelines: "Use ES6 maps instead of records wherever you can."
Also applies to: 44-44
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/vibe-coding/chat-adapters.ts` around lines 15 - 19, CONTEXT_MAP is defined as a plain object but the project guideline requires using an ES6 Map for lookup tables; replace the object literal with a Map instance (e.g., new Map([...])) typed to the same shape (keys -> { systemPrompt, tools }) and populate it with the three entries ("email-theme", "email-template", "email-draft"); then update all call sites (the usage around the existing reference CONTEXT_MAP at line ~44) to use CONTEXT_MAP.get(key) instead of bracket/index access and handle the possible undefined return. Keep the symbol name CONTEXT_MAP and preserve the same value shapes for compatibility.apps/backend/src/app/api/latest/internal/ai-chat/[threadId]/route.tsx (1)
19-21: Constrainmessageat the API boundary instead of usingyupMixed().Persisting fully unconstrained payloads makes thread data shape unstable and brittle for consumers. Define a concrete message schema here.
As per coding guidelines "Any assumption you make should either be validated through the type system (preferred), assertions, or tests. Optimally, two out of three."
🤖 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/ai-chat/`[threadId]/route.tsx around lines 19 - 21, The schema uses yupMixed() for the incoming message which leaves the API boundary unconstrained; replace the loose type with a concrete Yup schema for body.message (e.g., an object with required string fields like id, role, content, and optional metadata) by updating body: yupObject({ message: yupMixed().defined() }) to a detailed schema using yup.string(), yup.object(), and .required()/.defined() as appropriate so the route handler (route.tsx) validates message shape before persisting or passing into functions that expect specific fields.apps/dashboard/src/app/api/email-ai/route.ts (1)
87-87: Resolve the auth-header TODO before merge.The inline uncertainty on a privileged header (
x-stack-admin-access-token) should be finalized and documented to avoid auth regressions.I can draft a short issue with acceptance criteria for this TODO if you want.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/api/email-ai/route.ts` at line 87, Decide and remove the inline "TODO" by either (A) keeping the header "x-stack-admin-access-token" but sourcing its value securely (use a vetted secret/config or server-side session token instead of a transient client value) and adding a clear comment documenting why this header is required and its expected lifecycle, or (B) removing the header entirely if privileged admin access is not needed and replacing it with the proper auth mechanism (e.g., Authorization: Bearer via getServerSession or next-auth). Update the request construction in route.ts where "x-stack-admin-access-token": accessToken is set (and the variable accessToken) so it uses the approved secret/session source, include a short inline comment describing the approved auth flow and any acceptance criteria, and ensure no privileged token is exposed to the client.apps/e2e/tests/backend/endpoints/api/v1/ai-model-config.ts (1)
1-4: Replace TODO-based sync with an enforceable check.This duplicated matrix will drift unless sync is automated. Add a test or check that fails when backend model mapping changes.
I can draft a lightweight sync-check approach for this file if useful.
As per coding guidelines "ALWAYS add new E2E tests when you change the API or SDK interface. Generally, err on the side of creating too many tests."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/e2e/tests/backend/endpoints/api/v1/ai-model-config.ts` around lines 1 - 4, The duplicated model mapping in apps/e2e/tests/backend/endpoints/api/v1/ai-model-config.ts must be automatically validated against the canonical mapping in apps/backend/src/lib/ai/models.ts; add an E2E test that retrieves the backend's authoritative mapping (either by calling a narrow HTTP endpoint you expose like GET /api/v1/internal/ai-model-mapping or by reading a build-time JSON artifact emitted by the backend) and assert strict equality with the local matrix constant in this file (the duplicated matrix); make the test fail the CI when they differ and run it with the existing E2E suite so any changes to the backend mapping require updating the duplicated matrix or the source mapping simultaneously.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/backend/src/app/api/latest/ai/query/`[mode]/route.ts:
- Around line 78-84: Wrap the generateText call in an abort/timeout guard:
create an AbortController, start a timer (e.g., REQUEST_TIMEOUT_MS or a sensible
default like 30_000 ms) that calls controller.abort() and rejects when fired,
pass controller.signal into generateText (or race generateText against a timeout
promise if generateText does not accept a signal), and clear the timer after
generateText resolves. Update the generate path where generateText({ model,
system: systemPrompt, messages, tools: toolsArg, stopWhen:
stepCountIs(stepLimit) }) is invoked to propagate an appropriate timeout/abort
error so the request is terminated deterministically instead of hanging. Ensure
the abort/timeout cleanup (clearTimeout) runs in both success and error paths.
In `@apps/backend/src/lib/ai/forward.ts`:
- Around line 19-23: The production forward fetch call currently blocks without
an abort; update the forward logic in forward.ts to use an AbortController:
create a controller, pass controller.signal to fetch(productionUrl, { method:
"POST", headers: forwardHeaders, body: JSON.stringify(body), signal }), start a
timer (e.g., 10s or configurable) that calls controller.abort() on timeout, and
clear that timer when fetch resolves or rejects so you don't leak timers; ensure
any abort errors are handled where forward() is called.
In `@apps/dashboard/src/app/api/ai-search/route.ts`:
- Around line 51-64: The fetch to `${backendBaseUrl}/api/latest/ai/query/stream`
is unbounded and can hang; modify the route handler to create an
AbortController, pass controller.signal into the fetch call, and set a timeout
(e.g., via setTimeout) to call controller.abort() after a reasonable duration;
ensure you clear the timeout after fetch completes and handle the aborted/error
case so the route returns an appropriate error response. Use the existing
variables (requestHeaders, modelMessages, withAuth, tools) and keep the POST
body and headers the same while only adding the AbortController and timeout
logic surrounding the fetch invocation.
In `@docs/package.json`:
- Around line 21-23: Update the `@openrouter/ai-sdk-provider` dependency in
package.json from "0.7.5" to "2.2.3" so it matches the backend and satisfies the
ai@^6.0.0 peer dependency; locate the dependency entry
"@openrouter/ai-sdk-provider" and change its version string to "2.2.3", then run
your package manager install to refresh lockfiles.
---
Major comments:
In `@apps/backend/.env.development`:
- Line 60: The dev env currently sets
STACK_OPENROUTER_API_KEY=FORWARD_TO_PRODUCTION which forwards local AI traffic
to production; change the STACK_OPENROUTER_API_KEY value in the development env
to an empty value or a non-production placeholder (e.g. leave blank or use a
local/test key) and add a short inline comment warning not to forward to
production so committed dev envs never default to production credentials.
In `@apps/backend/src/app/api/latest/internal/wysiwyg-edit/route.tsx`:
- Around line 171-176: The code silently falls back to source_code when
prodResult has no valid text block; instead detect when textBlock is missing
(check prodResult.content and textBlock from the existing variables prodResult
and textBlock) and fail fast—throw an error or return an error response (e.g.,
throw new Error or return a 5xx/4xx NextResponse) rather than setting
updatedSource to source_code; update the logic around
updatedSource/stripCodeFences to only run when textBlock?.text exists and ensure
callers surface the error instead of returning a 200 with the original
source_code.
In `@apps/backend/src/lib/ai/prompts.ts`:
- Line 337: The prompt template named email-assistant-draft still instructs the
model to call createEmailTemplate; update that instruction to call
createEmailDraft instead (and adjust any example tool call snippets in the same
template to match the createEmailDraft tool signature and payload keys), so
generated tool calls validate correctly; search for the string
"createEmailTemplate" inside the email-assistant-draft prompt and replace it
with "createEmailDraft" and verify the surrounding example JSON matches the
actual createEmailDraft parameters.
In `@apps/backend/src/lib/ai/schema.ts`:
- Around line 16-18: The schema's systemPrompt allowed values include
"edit-dashboard", which is not present in the backend prompt ID union in
prompts.ts; update them to match by either removing "edit-dashboard" from the
systemPrompt enum in apps/backend/src/lib/ai/schema.ts or adding a corresponding
prompt ID and handler in apps/backend/src/lib/ai/prompts.ts (e.g., add
"edit-dashboard" to the Prompt ID union and implement its resolution), and
ensure the two sources remain in sync so only supported prompt IDs pass
validation.
In `@apps/backend/src/lib/ai/tools/create-email-draft.ts`:
- Around line 25-43: The prompt/example in create-email-draft is inconsistent:
the required email structure mandates using <Html>, <Head />, <Preview />,
<Tailwind>, <Body>, and <Container> but the example only renders <Container>,
which will produce invalid drafts; update the example/template (e.g., the
EmailTemplate or example return value) to include the full hierarchy—wrap
content in <Html><Tailwind><Head /><Body><Preview /> and then <Container>—and
ensure only inlineable Tailwind utilities are used (no hover:, focus:, media
queries, dark:, group-hover:, etc.) and that <Head /> is placed inside
<Tailwind> so style injection works.
In `@apps/backend/src/lib/ai/tools/create-email-template.ts`:
- Around line 25-54: The example EmailTemplate violates the required email
component hierarchy: wrap the template return in <Html> with <Tailwind> (and
render <Head /> inside <Tailwind>), then <Preview />, <Body>, and finally
<Container> containing your content; update the function EmailTemplate (and its
export EmailTemplate.PreviewVariables) to follow this Html -> Tailwind(Head) ->
Preview -> Body -> Container structure and remove any non-inlineable Tailwind
utilities (hover:, focus:, dark:, media queries, group-*, etc.) so only
inlineable utilities are used; ensure Subject and NotificationCategory remain
inside the Container as shown.
In `@apps/backend/src/lib/ai/tools/docs.ts`:
- Around line 12-14: The code currently treats any non-development environment
as production when constructing the MCP URL; update the logic around
getNodeEnvironment() so it explicitly handles "development" (use new
URL("/api/internal/mcp", "http://localhost:8104")) and "production" (use new
URL("/api/internal/mcp", "https://mcp.stack-auth.com")) and throws an error for
any other unexpected environment value (do not silently fallback), referencing
the existing getNodeEnvironment() call and the new URL("/api/internal/mcp", ...)
construction so the change is applied in the same assignment site.
In `@apps/backend/src/lib/ai/tools/sql-query.ts`:
- Around line 14-18: Add runtime guards to enforce the schema's SQL constraints
(only SELECT and must include LIMIT) before calling client.query: in the execute
function in sql-query.ts validate the provided query string (from the input
matching inputSchema) by trimming whitespace, checking that it begins with
"SELECT" (case-insensitive) and that it contains a LIMIT clause
(case-insensitive) and reject/throw an error for any query that fails these
checks (also consider disallowing multiple statements or trailing semicolons).
Ensure the check runs immediately before the call to client.query so the
contract described in inputSchema.describe is enforced in code.
In `@apps/backend/src/route-handlers/smart-response.tsx`:
- Around line 116-118: The loop copying headers from obj.body.headers.entries()
currently calls headers.set(key.toLowerCase(), [value]) which overwrites
existing arrays and loses duplicate header values (e.g., Set-Cookie); update the
logic in smart-response.tsx where headers is populated so it checks
headers.get(keyLower) first and, if present, appends the new value to that
array, otherwise sets a new array with the value—use the same key normalization
(key.toLowerCase()) and preserve ordering of values.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/email-templates/[templateId]/page-client.tsx:
- Line 28: Replace the type assertion on the useParams call with the generic
form to get proper compile-time typing: locate the line where useParams() is
cast using "as { projectId: string }" (the const { projectId } = useParams()
statement) and change it to use the generic type parameter form so the hook
returns the correctly typed params instead of bypassing the type system.
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/email-themes/[themeId]/page-client.tsx:
- Line 22: Replace the forced type cast on useParams by switching to the generic
useParams API: locate the line where useParams is called and projectId is
extracted (symbol: useParams and variable projectId) and change the pattern from
"useParams() as { projectId: string }" to using the generic form useParams<{
projectId: string }>() so TypeScript infers projectId correctly; apply the same
replacement in the related files that use the same pattern (email-templates and
email-drafts) to keep typing consistent across the codebase.
In `@apps/dashboard/src/app/api/ai-search/route.ts`:
- Around line 68-70: The current retry logic retries any non-OK response when
useAdminAuth is true; change it so it only retries when the backendResponse
status indicates an auth failure (401 or 403). After the initial request,
inspect backendResponse.status and only call makeRequest(false) to overwrite
backendResponse if status === 401 || status === 403; leave other non-OK
responses untouched so we don't mask backend errors. Use the existing variables
backendResponse, useAdminAuth, and makeRequest to implement this conditional
retry.
In `@apps/dashboard/src/app/api/email-ai/route.ts`:
- Around line 79-91: The fetch to
`${backendBaseUrl}/api/latest/ai/query/generate` (producing backendResponse)
lacks an abort/timeout and can hang; update the route handler in route.ts to
wrap the fetch with an AbortController: create a controller, pass
controller.signal to fetch, start a timer (e.g., setTimeout) that calls
controller.abort() after a configurable timeout (e.g., 10–30s), and clear the
timer after fetch completes; also catch AbortError (or check
controller.signal.aborted) and return/throw a descriptive 504 Gateway Timeout
response instead of leaving the request hanging.
- Around line 50-53: The current handler reads client-controlled fields
payload.systemPrompt and payload.tools and forwards them in an admin-scoped
request; instead enforce a server-side allowlist and never forward raw client
values. Replace use of payload.systemPrompt and payload.tools when building the
admin request (the code that issues the admin-scoped fetch) with
validated/allowed values: derive allowedSystemPrompt and allowedTools on the
server (e.g., lookup by payload.projectId or use a fixed server-side map),
validate payload.tools against that allowlist and discard or replace invalid
entries, then send only allowedSystemPrompt and allowedTools in the admin
request; keep payload.projectId (and other safe fields) as before and remove any
direct forwarding of payload.systemPrompt or payload.tools.
In `@apps/dashboard/src/components/vibe-coding/chat-adapters.ts`:
- Around line 62-63: The code currently force-casts response.json() to {
content?: ChatContent } and defaults malformed content to [] (variables:
response, result, content, ChatContent), which hides invalid AI responses;
replace the cast-and-default with explicit validation: parse await
response.json() into a temporary any, verify that result.content exists and is
an array of expected items (or otherwise matches ChatContent shape), and if not
throw a descriptive error (include response.status and a short serialized body
snippet) so callers fail fast instead of receiving an empty array.
In `@docs/src/app/api/chat/route.ts`:
- Around line 34-47: The fetch to `${backendBaseUrl}/api/latest/ai/query/stream`
(creating backendResponse) lacks a timeout guard; wrap the request in an
AbortController, pass controller.signal to the fetch call used with
requestHeaders and body (the same payload:
quality/speed/systemPrompt/tools/messages), start a setTimeout to call
controller.abort() after a sensible timeout (e.g., 10–30s), and clear the timer
on success; handle the abort by catching the thrown AbortError and returning the
existing errorResponse fallback so stalled requests are terminated and the route
doesn't hang.
- Around line 7-9: The current assignment of backendBaseUrl uses a silent
default to "https://api.stack-auth.com" when
process.env.NEXT_PUBLIC_STACK_API_URL is missing; change this to fail loudly by
validating process.env.NEXT_PUBLIC_STACK_API_URL at startup or inside the route
and throwing a clear error (or returning a 500) if it's undefined, so update the
logic around the backendBaseUrl constant to require NEXT_PUBLIC_STACK_API_URL
and surface an explicit error message instead of silently falling back to
production.
In `@docs/src/components/chat/ai-chat.tsx`:
- Around line 605-614: The current map over toolInvocations casts
DynamicToolUIPart.input and output with "as", which violates the no-cast rule;
instead, change ToolCallDisplay's props to accept unknown for args/result
(update ToolCallDisplay prop types to args: unknown and result: unknown) and
move runtime validation/narrowing into ToolCallDisplay (use type guards/shape
checks there) so you can pass toolCall={{ toolName: part.toolName, args:
part.input, result: part.output }} without any "as" casts; alternatively, if you
must narrow here, implement explicit type guard functions for
DynamicToolUIPart.input/output and only construct the prop object after those
guards succeed.
---
Minor comments:
In `@apps/backend/src/lib/ai/tools/create-email-theme.ts`:
- Around line 20-21: The EmailTheme implementation must be adjusted so its
component signature accepts only a single prop, children (React node), and must
not reference or accept unsubscribeLink; update the EmailTheme
function/component in create-email-theme.ts to remove any unsubscribeLink prop
usage and only render children, and ensure imports are limited to
"@react-email/components" (remove any other package imports); alternatively, if
the renderer contract should include unsubscribeLink, update the prompt/contract
instead so both the EmailTheme component and renderer agree on the same props
(but do not leave mismatched props between EmailTheme and the renderer).
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/email-drafts/[draftId]/page-client.tsx:
- Line 19: Replace the unsafe type assertion on useParams with runtime
validation: stop using "useParams() as { projectId: string }" and instead read
params from useParams(), then assert projectId exists at runtime and call
throwErr(...) if it does not; e.g., get params via useParams(), extract
params.projectId, and use "?? throwErr('Missing projectId')" to fail loudly
(reference symbols: useParams, projectId variable, throwErr).
---
Nitpick comments:
In `@apps/backend/src/app/api/latest/internal/ai-chat/`[threadId]/route.tsx:
- Around line 19-21: The schema uses yupMixed() for the incoming message which
leaves the API boundary unconstrained; replace the loose type with a concrete
Yup schema for body.message (e.g., an object with required string fields like
id, role, content, and optional metadata) by updating body: yupObject({ message:
yupMixed().defined() }) to a detailed schema using yup.string(), yup.object(),
and .required()/.defined() as appropriate so the route handler (route.tsx)
validates message shape before persisting or passing into functions that expect
specific fields.
In `@apps/dashboard/src/app/api/email-ai/route.ts`:
- Line 87: Decide and remove the inline "TODO" by either (A) keeping the header
"x-stack-admin-access-token" but sourcing its value securely (use a vetted
secret/config or server-side session token instead of a transient client value)
and adding a clear comment documenting why this header is required and its
expected lifecycle, or (B) removing the header entirely if privileged admin
access is not needed and replacing it with the proper auth mechanism (e.g.,
Authorization: Bearer via getServerSession or next-auth). Update the request
construction in route.ts where "x-stack-admin-access-token": accessToken is set
(and the variable accessToken) so it uses the approved secret/session source,
include a short inline comment describing the approved auth flow and any
acceptance criteria, and ensure no privileged token is exposed to the client.
In `@apps/dashboard/src/components/vibe-coding/chat-adapters.ts`:
- Around line 15-19: CONTEXT_MAP is defined as a plain object but the project
guideline requires using an ES6 Map for lookup tables; replace the object
literal with a Map instance (e.g., new Map([...])) typed to the same shape (keys
-> { systemPrompt, tools }) and populate it with the three entries
("email-theme", "email-template", "email-draft"); then update all call sites
(the usage around the existing reference CONTEXT_MAP at line ~44) to use
CONTEXT_MAP.get(key) instead of bracket/index access and handle the possible
undefined return. Keep the symbol name CONTEXT_MAP and preserve the same value
shapes for compatibility.
In `@apps/e2e/tests/backend/endpoints/api/v1/ai-model-config.ts`:
- Around line 1-4: The duplicated model mapping in
apps/e2e/tests/backend/endpoints/api/v1/ai-model-config.ts must be automatically
validated against the canonical mapping in apps/backend/src/lib/ai/models.ts;
add an E2E test that retrieves the backend's authoritative mapping (either by
calling a narrow HTTP endpoint you expose like GET
/api/v1/internal/ai-model-mapping or by reading a build-time JSON artifact
emitted by the backend) and assert strict equality with the local matrix
constant in this file (the duplicated matrix); make the test fail the CI when
they differ and run it with the existing E2E suite so any changes to the backend
mapping require updating the duplicated matrix or the source mapping
simultaneously.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (39)
apps/backend/.env.developmentapps/backend/package.jsonapps/backend/src/app/api/latest/ai/query/[mode]/route.tsapps/backend/src/app/api/latest/internal/ai-chat/[threadId]/route.tsxapps/backend/src/app/api/latest/internal/wysiwyg-edit/route.tsxapps/backend/src/lib/ai-chat/adapter-registry.tsapps/backend/src/lib/ai-chat/email-draft-adapter.tsapps/backend/src/lib/ai-chat/email-template-adapter.tsapps/backend/src/lib/ai-chat/email-theme-adapter.tsapps/backend/src/lib/ai/forward.tsapps/backend/src/lib/ai/models.tsapps/backend/src/lib/ai/prompts.tsapps/backend/src/lib/ai/schema.tsapps/backend/src/lib/ai/tools/create-dashboard.tsapps/backend/src/lib/ai/tools/create-email-draft.tsapps/backend/src/lib/ai/tools/create-email-template.tsapps/backend/src/lib/ai/tools/create-email-theme.tsapps/backend/src/lib/ai/tools/docs.tsapps/backend/src/lib/ai/tools/index.tsapps/backend/src/lib/ai/tools/sql-query.tsapps/backend/src/route-handlers/smart-response.tsxapps/backend/src/route-handlers/smart-route-handler.tsxapps/dashboard/.env.developmentapps/dashboard/package.jsonapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-drafts/[draftId]/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-templates/[templateId]/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-themes/[themeId]/page-client.tsxapps/dashboard/src/app/api/ai-search/route.tsapps/dashboard/src/app/api/email-ai/route.tsapps/dashboard/src/components/vibe-coding/chat-adapters.tsapps/dashboard/src/components/vibe-coding/draft-tool-components.tsxapps/e2e/tests/backend/endpoints/api/v1/ai-model-config.tsapps/e2e/tests/backend/endpoints/api/v1/ai-query.test.tsdocs/package.jsondocs/src/app/api/chat/route.tsdocs/src/components/chat/ai-chat.tsxpackages/stack-shared/src/interface/admin-interface.tspackages/template/src/lib/stack-app/apps/implementations/admin-app-impl.tspackages/template/src/lib/stack-app/apps/interfaces/admin-app.ts
💤 Files with no reviewable changes (9)
- apps/dashboard/.env.development
- packages/template/src/lib/stack-app/apps/interfaces/admin-app.ts
- apps/backend/src/lib/ai-chat/email-template-adapter.ts
- apps/dashboard/package.json
- apps/backend/src/lib/ai-chat/adapter-registry.ts
- apps/backend/src/lib/ai-chat/email-draft-adapter.ts
- packages/stack-shared/src/interface/admin-interface.ts
- packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts
- apps/backend/src/lib/ai-chat/email-theme-adapter.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/dashboard/src/app/api/email-ai/route.ts (1)
79-81: UseURLconstructor or tagged template for URL buildingPlain template-literal string interpolation for URLs violates the project's URL construction guideline. As per coding guidelines: "Use
urlString``orencodeURIComponent()instead of normal string interpolation for URLs for consistency."✏️ Proposed fix
- const backendResponse = await fetch( - `${backendBaseUrl}/api/latest/ai/query/generate`, + const backendResponse = await fetch( + new URL("/api/latest/ai/query/generate", backendBaseUrl).toString(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/api/email-ai/route.ts` around lines 79 - 81, Replace the plain template literal URL used when calling fetch for backendResponse with a proper URL construction method: build the request URL using the URL constructor (new URL('/api/latest/ai/query/generate', backendBaseUrl)) or the project's urlString tagged template instead of `${backendBaseUrl}/api/latest/ai/query/generate`; update the fetch call in apps/dashboard/src/app/api/email-ai/route.ts (the backendResponse fetch) to use that constructed URL so it follows the project's URL construction guideline.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/dashboard/src/app/api/email-ai/route.ts`:
- Around line 67-72: Replace the coercive check using !accessToken with an
explicit null/undefined check so empty-string tokens are not treated as missing;
update the conditional that uses hasProjectAccess and accessToken (the if that
currently reads "!hasProjectAccess || !accessToken") to use accessToken == null
instead (i.e., check hasProjectAccess and accessToken == null) so only
null/undefined accessToken values are considered unauthorized.
- Around line 50-59: Replace the naked type cast of req.json() and add runtime
validation for the request payload: do not use "as" to assume shape — parse the
body with a schema validator (e.g., zod) or manual guards and validate required
fields projectId, systemPrompt, tools, and messages before destructuring; if
validation fails return a 400 with a clear error. Update the code around the
payload assignment and the destructuring of { projectId, systemPrompt, tools,
messages, quality, speed } so that projectId, systemPrompt, tools (array), and
messages (array/expected shape) are asserted by the schema/guards and default
values for quality and speed are applied only after successful validation.
- Around line 101-109: The code assumes result.content is an array and directly
calls result.content.map, which can throw if the backend returns an unexpected
shape; replace the unchecked cast and guard the value before mapping: parse
backendResponse.json() into a safe local (e.g., const resultRaw = await
backendResponse.json()), validate that resultRaw?.content is an Array (or coerce
to an empty array), then build sanitized.content by mapping over that validated
array and applying sanitizeGeneratedCode(item.args.content) only for items with
type "tool-call" and a string args.content; update references to result and
sanitized accordingly so the function never calls .map on undefined and avoids
using as-casts.
---
Nitpick comments:
In `@apps/dashboard/src/app/api/email-ai/route.ts`:
- Around line 79-81: Replace the plain template literal URL used when calling
fetch for backendResponse with a proper URL construction method: build the
request URL using the URL constructor (new URL('/api/latest/ai/query/generate',
backendBaseUrl)) or the project's urlString tagged template instead of
`${backendBaseUrl}/api/latest/ai/query/generate`; update the fetch call in
apps/dashboard/src/app/api/email-ai/route.ts (the backendResponse fetch) to use
that constructed URL so it follows the project's URL construction guideline.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/package.json (2)
20-59:⚠️ Potential issue | 🔴 CriticalUpdate
pnpm-lock.yaml— all CI pipelines are currently broken.Every pipeline (
E2E API Tests,Dev Environment Test,Run setup tests,Prisma migration sync,Lint & build) fails with:
pnpm install --frozen-lockfile failed: 1 dependencies were removed:@openrouter/ai-sdk-provider@0.7.5The lockfile was not regenerated after removing
@openrouter/ai-sdk-provider. Runpnpm install(without--frozen-lockfile) from the repo root and commit the updatedpnpm-lock.yaml.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/package.json` around lines 20 - 59, The CI failures are caused by a stale pnpm-lock.yaml after removing the dependency `@openrouter/ai-sdk-provider`@0.7.5; fix by running pnpm install from the repository root (no --frozen-lockfile) to regenerate pnpm-lock.yaml, verify the lockfile now reflects the current dependencies (matching the dependencies block in package.json), and commit the updated pnpm-lock.yaml so pipelines using pnpm install --frozen-lockfile succeed.
58-58:⚠️ Potential issue | 🔴 CriticalUpgrade
zodto at least^3.25.76or preferably^4.1.8.The current version
^3.23.8is below the minimum supported range forai@^6.0.0. The AI SDK v6 declares zod as a peer dependency with the compatible range^3.25.76 || ^4.1.8. While zod v3.25.76+ would resolve the immediate incompatibility, upgrading to^4.1.8is the recommended path per the AI SDK migration guidance and avoids TypeScript performance issues associated with zod v3.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/package.json` at line 58, Update the declared dependency "zod" in package.json from "^3.23.8" to at least "^3.25.76" (preferably "^4.1.8") to satisfy the ai@^6.0.0 peerDependency; change the "zod" version string in the dependencies block and then run your package manager (npm/yarn/pnpm) to install and update lockfiles so the new version is resolved.
♻️ Duplicate comments (1)
apps/backend/src/app/api/latest/ai/query/[mode]/route.ts (1)
78-86:⚠️ Potential issue | 🔴 Critical
abortSignalis never passed togenerateText— the timeout is a no-op.The
AbortControllerandsetTimeoutare set up correctly, butcontroller.signalis never forwarded togenerateText. When the timeout fires,controller.abort()runs in a vacuum and the request continues to hang indefinitely.This was flagged in a previous review and marked as implemented, but the fix is incomplete.
🐛 Proposed fix
const result = await generateText({ model, system: systemPrompt, messages, tools: toolsArg, stopWhen: stepCountIs(stepLimit), + abortSignal: controller.signal, }).finally(() => clearTimeout(timeoutId));🤖 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 78 - 86, The AbortController is created but its signal is never forwarded to generateText, so timeouts don't cancel the request; update the generateText invocation in route.ts to pass the controller.signal (e.g., abortSignal: controller.signal or signal: controller.signal depending on generateText's parameter name) and ensure generateText/its downstream fetch logic respects that signal so controller.abort() actually cancels the operation while keeping the existing clearTimeout in the finally block.
🧹 Nitpick comments (4)
apps/backend/src/app/api/latest/ai/query/[mode]/route.ts (2)
100-106: Boolean check onstep.textsilently drops empty-string text blocks.
if (step.text)is falsy for"", which means an empty string from a step is silently skipped rather than explicitly handled. Per coding guidelines, prefer explicit null/undefined checks over boolean coercion.✏️ Proposed fix
- if (step.text) { + if (step.text != null && step.text !== "") {🤖 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 100 - 106, Replace the boolean coercion check that drops empty strings in the result.steps loop: instead of using if (step.text) in the forEach over result.steps (the block that pushes to contentBlocks), explicitly check for null/undefined (e.g., step.text != null or step.text !== undefined && step.text !== null) so empty-string text blocks are preserved and only null/undefined are skipped.
120-120:as Jsoncast on tool result output.
(toolResult?.output ?? null) as Jsonis another type cast. IftoolResult.outputcan genuinely be any shape, consider widening thecontentBlockstype to acceptunknownfor the result field, or validate the shape. As per coding guidelines: "Do NOT useas/any/type casts."🤖 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 120, The current cast "(toolResult?.output ?? null) as Json" should be removed; instead update the type of the contentBlocks result field to accept unknown (or a validated narrower type) and ensure toolResult.output is validated/mapped before assignment—modify the contentBlocks type definition and the code that builds contentBlocks (where toolResult and its output are used) to either validate/transform toolResult.output into the expected shape or assign it as unknown/null without using "as", e.g., change the result property type to unknown | null and add a runtime check/mapper for toolResult.output in the route handler that constructs contentBlocks.apps/dashboard/src/app/api/email-ai/route.ts (2)
93-94: UseurlStringtagged template instead of plain string interpolation for URLs.As per coding guidelines: "Use
urlStringorencodeURIComponent()instead of normal string interpolation for URLs for consistency."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/api/email-ai/route.ts` around lines 93 - 94, Replace the plain string interpolation when calling fetch with the urlString tagged template to build the URL; specifically change the fetch invocation that uses backendBaseUrl and the path "/api/latest/ai/query/generate" (the code creating backendResponse) to use urlString so the final request URL is constructed via urlString`${backendBaseUrl}/api/latest/ai/query/generate` (or equivalent) for proper encoding/consistency.
33-38: HTML entity decode order:&must be decoded last.Currently
&is decoded last on Line 38, which is correct. However, the comment on Line 31 mentions&&→&&as a motivating example. If the order were ever shuffled, decoding&first would corrupt sequences like&lt;→<→<. Worth adding a brief inline note that the order is intentional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/app/api/email-ai/route.ts` around lines 33 - 38, The HTML-entity decoding chain in route.ts (the sequence of .replace calls on result) must keep & decoded last to avoid corrupting sequences like &lt; → < → <; update the .replace chain in the same block (the code that mutates result) to ensure .replace(/&/g, "&") remains the final replacement and add a concise inline comment above that block (near the result = result.replace(...)) stating the intentional order (e.g., "& must be decoded last to avoid double-decoding sequences like &lt;") so future edits preserve the correct decoding order.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/backend/src/app/api/latest/ai/query/`[mode]/route.ts:
- Around line 64-71: The stream path currently calls streamText(...) without a
timeout/abort guard and can hang; mirror the generate path by creating an
AbortController with a timer that aborts after the configured timeout, pass its
signal into streamText (e.g., add a signal parameter when calling streamText),
and ensure you clear the timeout and handle/propagate the abort error (and close
the response stream) when the controller fires; reference the streamText call
and the generate-path AbortController/timer logic to implement the same pattern.
- Around line 51-54: Remove the unnecessary "as" type casts on quality, speed,
systemPromptId, toolNames (and messages) — the requestBodySchema validated by
createSmartRouteHandler already guarantees these shapes; instead let TypeScript
infer types from the validated body (or tighten the handler's typed request
body) and rely on validateToolNames() for per-tool runtime checks; update the
code around the variables quality, speed, systemPromptId, toolNames (and
messages) to drop the casts and use the validated values directly.
In `@apps/dashboard/src/app/api/email-ai/route.ts`:
- Around line 115-117: The code currently assigns contentArr by silently falling
back to [] when (result as Record<string,unknown>)?.content is not an array;
instead, after calling backendResponse.json() and obtaining result, explicitly
validate that (result as Record<string,unknown>)?.content is an Array and if not
throw an Error (or return a 500 with a clear message) including the unexpected
shape and the raw result to aid debugging; update the logic around result,
contentArr and the final returned payload so that contentArr is only set from a
validated array and any other shape causes an immediate surfaced error rather
than returning { content: [] }.
---
Outside diff comments:
In `@docs/package.json`:
- Around line 20-59: The CI failures are caused by a stale pnpm-lock.yaml after
removing the dependency `@openrouter/ai-sdk-provider`@0.7.5; fix by running pnpm
install from the repository root (no --frozen-lockfile) to regenerate
pnpm-lock.yaml, verify the lockfile now reflects the current dependencies
(matching the dependencies block in package.json), and commit the updated
pnpm-lock.yaml so pipelines using pnpm install --frozen-lockfile succeed.
- Line 58: Update the declared dependency "zod" in package.json from "^3.23.8"
to at least "^3.25.76" (preferably "^4.1.8") to satisfy the ai@^6.0.0
peerDependency; change the "zod" version string in the dependencies block and
then run your package manager (npm/yarn/pnpm) to install and update lockfiles so
the new version is resolved.
---
Duplicate comments:
In `@apps/backend/src/app/api/latest/ai/query/`[mode]/route.ts:
- Around line 78-86: The AbortController is created but its signal is never
forwarded to generateText, so timeouts don't cancel the request; update the
generateText invocation in route.ts to pass the controller.signal (e.g.,
abortSignal: controller.signal or signal: controller.signal depending on
generateText's parameter name) and ensure generateText/its downstream fetch
logic respects that signal so controller.abort() actually cancels the operation
while keeping the existing clearTimeout in the finally block.
---
Nitpick comments:
In `@apps/backend/src/app/api/latest/ai/query/`[mode]/route.ts:
- Around line 100-106: Replace the boolean coercion check that drops empty
strings in the result.steps loop: instead of using if (step.text) in the forEach
over result.steps (the block that pushes to contentBlocks), explicitly check for
null/undefined (e.g., step.text != null or step.text !== undefined && step.text
!== null) so empty-string text blocks are preserved and only null/undefined are
skipped.
- Line 120: The current cast "(toolResult?.output ?? null) as Json" should be
removed; instead update the type of the contentBlocks result field to accept
unknown (or a validated narrower type) and ensure toolResult.output is
validated/mapped before assignment—modify the contentBlocks type definition and
the code that builds contentBlocks (where toolResult and its output are used) to
either validate/transform toolResult.output into the expected shape or assign it
as unknown/null without using "as", e.g., change the result property type to
unknown | null and add a runtime check/mapper for toolResult.output in the route
handler that constructs contentBlocks.
In `@apps/dashboard/src/app/api/email-ai/route.ts`:
- Around line 93-94: Replace the plain string interpolation when calling fetch
with the urlString tagged template to build the URL; specifically change the
fetch invocation that uses backendBaseUrl and the path
"/api/latest/ai/query/generate" (the code creating backendResponse) to use
urlString so the final request URL is constructed via
urlString`${backendBaseUrl}/api/latest/ai/query/generate` (or equivalent) for
proper encoding/consistency.
- Around line 33-38: The HTML-entity decoding chain in route.ts (the sequence of
.replace calls on result) must keep & decoded last to avoid corrupting
sequences like &lt; → < → <; update the .replace chain in the same block
(the code that mutates result) to ensure .replace(/&/g, "&") remains the
final replacement and add a concise inline comment above that block (near the
result = result.replace(...)) stating the intentional order (e.g., "& must
be decoded last to avoid double-decoding sequences like &lt;") so future
edits preserve the correct decoding order.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/backend/src/app/api/latest/ai/query/[mode]/route.tsapps/dashboard/src/app/api/email-ai/route.tsdocs/package.json
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/stack-shared/src/interface/admin-interface.ts (1)
478-524: Loose typing forquality,speed, androleweakens compile-time safety.The parameters
quality,speed, androleare typed asstring, but they clearly have a small set of valid values (e.g.,"smartest","fast","generate","stream","user","assistant", etc.). Using plainstringhere means callers can pass arbitrary strings without any compile-time feedback.Consider using union types (or importing the allowed values from the backend schema) to provide type safety:
async sendAiQuery(options: { systemPrompt: string, tools: string[], - messages: Array<{ role: string, content: unknown }>, - quality?: string, - speed?: string, + messages: Array<{ role: "user" | "assistant" | "system", content: unknown }>, + quality?: "smartest" | "fast" | "cheap", + speed?: "fast" | "slow", mode: "stream", }): Promise<Response>;(Adjust the union values to match the actual backend-accepted values.)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stack-shared/src/interface/admin-interface.ts` around lines 478 - 524, The sendAiQuery overloads use plain string for options.quality, options.speed and the messages[].role which weakens type safety; change those to concrete union types (or import existing enums/types from the backend schema) and update the signatures for sendAiQuery and the messages array accordingly (e.g., define types like Quality = "smartest" | "..." , Speed = "fast" | "..." , Role = "user" | "assistant" | "system" or reuse existing exported types) so callers get compile-time checks—update every occurrence in the overloads and the implementation signature (sendAiQuery, messages: Array<{ role: Role, content: unknown }>) and adjust defaulting logic to still accept the correct union values.packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts (1)
693-696:as anybypasses overload type safety — consider a narrower cast.The
as anyworks around TypeScript's inability to call an overloaded function's implementation signature externally. However, per coding guidelines,as anyshould be avoided in favor of narrower alternatives. A targeted function-type cast would preserve some type checking on the argument and return types:♻️ Suggested narrower cast
- // any: overload implementation signature is intentionally broader than individual overloads - return (this._interface.sendAiQuery as any)(options); + // Cast to the implementation signature shape because TypeScript's overload resolution + // does not expose the implementation signature to external callers. + return (this._interface.sendAiQuery as (options: typeof options) => Promise<{ content: ChatContent } | Response>)(options);As per coding guidelines, "Do NOT use
as/any/type casts or anything else to bypass the type system unless you specifically asked the user about it."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts` around lines 693 - 696, The call currently uses a blanket cast "(this._interface.sendAiQuery as any)(options)" which bypasses type safety; replace it with a narrower function-type cast or an explicit typed wrapper so TypeScript can check the argument and return shapes (e.g., cast this._interface.sendAiQuery to a function type that accepts the concrete options type and returns Promise<{ content: ChatContent } | Response>), or alternatively expose an implementation-signature on the interface so you can call this._interface.sendAiQuery(options) without casting; target the symbol this._interface.sendAiQuery and use the concrete options type and the return union { content: ChatContent } | Response rather than any.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/email-drafts/[draftId]/page-client.tsx:
- Line 165: The createChatAdapter call currently passes draftId in the wrong
position and includes an extra thread ID parameter; update the invocation to
match the new signature by removing draftId and making the second argument the
contextType string ("email-draft"), so call createChatAdapter(stackAdminApp,
"email-draft", handleToolUpdate, () => currentCode) instead of the current
ordering.
In `@apps/dashboard/src/components/vibe-coding/chat-adapters.ts`:
- Line 86: Replace the silent fallback at the ChatContent assignment so
malformed AI responses raise an explicit error: instead of defaulting const
content: ChatContent = Array.isArray(result.content) ? result.content : [];
validate result.content and throw a descriptive Error (including the unexpected
value) when it is not an array. Update the code path that uses content (the
variable named content and any surrounding parsing logic in chat-adapters.ts) to
rely on this validation so downstream consumers (e.g., the function handling AI
responses) never receive a silently coerced empty array.
---
Nitpick comments:
In `@packages/stack-shared/src/interface/admin-interface.ts`:
- Around line 478-524: The sendAiQuery overloads use plain string for
options.quality, options.speed and the messages[].role which weakens type
safety; change those to concrete union types (or import existing enums/types
from the backend schema) and update the signatures for sendAiQuery and the
messages array accordingly (e.g., define types like Quality = "smartest" | "..."
, Speed = "fast" | "..." , Role = "user" | "assistant" | "system" or reuse
existing exported types) so callers get compile-time checks—update every
occurrence in the overloads and the implementation signature (sendAiQuery,
messages: Array<{ role: Role, content: unknown }>) and adjust defaulting logic
to still accept the correct union values.
In `@packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts`:
- Around line 693-696: The call currently uses a blanket cast
"(this._interface.sendAiQuery as any)(options)" which bypasses type safety;
replace it with a narrower function-type cast or an explicit typed wrapper so
TypeScript can check the argument and return shapes (e.g., cast
this._interface.sendAiQuery to a function type that accepts the concrete options
type and returns Promise<{ content: ChatContent } | Response>), or alternatively
expose an implementation-signature on the interface so you can call
this._interface.sendAiQuery(options) without casting; target the symbol
this._interface.sendAiQuery and use the concrete options type and the return
union { content: ChatContent } | Response rather than any.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (8)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-drafts/[draftId]/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-templates/[templateId]/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-themes/[themeId]/page-client.tsxapps/dashboard/src/app/api/ai-search/route.tsapps/dashboard/src/components/vibe-coding/chat-adapters.tspackages/stack-shared/src/interface/admin-interface.tspackages/template/src/lib/stack-app/apps/implementations/admin-app-impl.tspackages/template/src/lib/stack-app/apps/interfaces/admin-app.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-templates/[templateId]/page-client.tsx
- apps/dashboard/src/app/api/ai-search/route.ts
- apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-themes/[themeId]/page-client.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts`:
- Around line 670-695: Replace the bare "as any" on this._interface.sendAiQuery
with a narrow cast to the exact overloaded function type and add a one-line
explanatory comment: state that TypeScript cannot directly delegate to another
overloaded implementation signature so we cast through unknown to the explicit
overloaded function type to satisfy the implementation signature, and reassure
readers that the original overloads on sendAiQuery enforce call-site type
safety; target the symbols this._interface.sendAiQuery and the class's
sendAiQuery implementation when making this change.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-drafts/[draftId]/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-templates/[templateId]/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-themes/[themeId]/page-client.tsxpackages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-themes/[themeId]/page-client.tsx
- apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-drafts/[draftId]/page-client.tsx
- apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-templates/[templateId]/page-client.tsx
N2D4
requested changes
Feb 27, 2026
Comment on lines
+32
to
+39
| const apiKey = getEnvVariable("STACK_OPENROUTER_API_KEY", ""); | ||
|
|
||
| if (apiKey === "") { | ||
| throw new StatusError( | ||
| StatusError.InternalServerError, | ||
| "OpenRouter API key is not configured. Please set STACK_OPENROUTER_API_KEY environment variable." | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
this should be a StackAssertionError, or better even, just not specify a fallback
Suggested change
| const apiKey = getEnvVariable("STACK_OPENROUTER_API_KEY", ""); | |
| if (apiKey === "") { | |
| throw new StatusError( | |
| StatusError.InternalServerError, | |
| "OpenRouter API key is not configured. Please set STACK_OPENROUTER_API_KEY environment variable." | |
| ); | |
| } | |
| const apiKey = getEnvVariable("STACK_OPENROUTER_API_KEY"); |
Collaborator
Author
There was a problem hiding this comment.
applied the change
Comment on lines
+109
to
+185
| @@ -197,44 +155,45 @@ ${html_context.slice(0, 500)} | |||
| Please update the source code to change "${old_text}" to "${new_text}" at the specified location. Return ONLY the complete updated source code. | |||
| `; | |||
|
|
|||
| // Model is configurable via env var; no default to surface missing config errors | |||
| const modelName = getEnvVariable("STACK_AI_MODEL"); | |||
| if (apiKey === "FORWARD_TO_PRODUCTION") { | |||
| const prodResponse = await forwardToProduction(fullReq.headers, "generate", { | |||
| quality: "smart", | |||
| speed: "fast", | |||
| systemPrompt: "wysiwyg-edit", | |||
| tools: [], | |||
| messages: [{ role: "user", content: userPrompt }], | |||
| }); | |||
|
|
|||
| if (!prodResponse.ok) { | |||
| throw new StatusError(prodResponse.status, `Production AI request failed: ${prodResponse.status}`); | |||
| } | |||
|
|
|||
| const prodResult = await prodResponse.json() as { content?: Array<{ type: string, text?: string }> }; | |||
| const textBlock = Array.isArray(prodResult.content) | |||
| ? prodResult.content.find((b) => b.type === "text" && b.text) | |||
| : undefined; | |||
| const updatedSource = stripCodeFences(textBlock?.text?.trim() ?? source_code); | |||
|
|
|||
| if (!openai) { | |||
| // This shouldn't happen since we check isMockMode above, but guard anyway | |||
| throw new Error("OpenAI client not initialized - STACK_OPENROUTER_API_KEY may be missing"); | |||
| return { | |||
| statusCode: 200, | |||
| bodyType: "json", | |||
| body: { updated_source: updatedSource }, | |||
| }; | |||
| } | |||
|
|
|||
| // Create abort controller for timeout | |||
| const model = selectModel("smart", "fast", true); | |||
|
|
|||
Contributor
There was a problem hiding this comment.
huge code duplication. the goal of the unified AI endpoint is to remove the other AI endpoints entirely (including this one), and do the logic in the frontend instead
Collaborator
Author
There was a problem hiding this comment.
removed this endpoint entirely
Comment on lines
5
to
6
| export const PATCH = createSmartRouteHandler({ | ||
| metadata: { |
Contributor
There was a problem hiding this comment.
why is this file still required?
Collaborator
Author
There was a problem hiding this comment.
this file is being used to save a chat message (specifically for the email themes, templates, and drafts).
Comment on lines
+26
to
+31
| const response = await adminApp.sendAiQuery({ | ||
| systemPrompt: "command-center-ask-ai", | ||
| tools, | ||
| stopWhen: tools ? stepCountIs(5) : undefined, | ||
| messages: modelMessages, | ||
| quality: "smart", | ||
| speed: "fast", |
Contributor
There was a problem hiding this comment.
also here, this endpoint should be removed, instead we should just call the new AI endpoint from the frontend
Collaborator
Author
There was a problem hiding this comment.
removed the endpoint, we are now calling the new ai endpoint from the frontend.
Comment on lines
+15
to
+54
| /** | ||
| * Sanitizes AI-generated JSX/TSX code before it is applied to the email renderer. | ||
| * | ||
| * Handles four common model output issues: | ||
| * 1. Markdown code fences (```tsx ... ```) wrapping the output despite instructions | ||
| * 2. HTML-encoded angle brackets (<Component> instead of <Component>) | ||
| * 3. Bare & in JSX text content (invalid JSX; must be & or {"&"}) | ||
| * 4. Semicolons used as property separators in JS object literals instead of commas | ||
| * (the AI confuses TypeScript interface syntax with JS object syntax). | ||
| * TypeScript also accepts commas in interfaces/types, so replacing ; → , is always safe. | ||
| */ | ||
| function sanitizeGeneratedCode(code: string): string { | ||
| let result = code.trim(); | ||
|
|
||
| if (result.startsWith("```")) { | ||
| const lines = result.split("\n"); | ||
| lines.shift(); | ||
| if (lines[lines.length - 1]?.trim() === "```") { | ||
| lines.pop(); | ||
| } | ||
| result = lines.join("\n").trim(); | ||
| } | ||
|
|
||
| result = result | ||
| .replace(/</g, "<") | ||
| .replace(/>/g, ">") | ||
| .replace(/"/g, '"') | ||
| .replace(/'/g, "'") | ||
| .replace(/&/g, "&"); | ||
|
|
||
| result = result.replace(/;(\s*\n\s*[A-Za-z_$][\w$]*\s*:)/g, ",$1"); | ||
|
|
||
| return result; | ||
| } | ||
|
|
||
| const CONTEXT_MAP = { | ||
| "email-theme": { systemPrompt: "email-assistant-theme", tools: ["create-email-theme"] }, | ||
| "email-template": { systemPrompt: "email-assistant-template", tools: ["create-email-template"] }, | ||
| "email-draft": { systemPrompt: "email-assistant-draft", tools: ["create-email-draft"] }, | ||
| } as const; |
Contributor
There was a problem hiding this comment.
this function seems dangerous, let's talk about better options we have to achieve this
Comment on lines
+20
to
+46
| if (projectId != null && publishableClientKey != null) { | ||
| requestHeaders["x-stack-access-type"] = "client"; | ||
| requestHeaders["x-stack-project-id"] = projectId; | ||
| requestHeaders["x-stack-publishable-client-key"] = publishableClientKey; | ||
| } | ||
|
|
||
| return result.toDataStreamResponse({ | ||
| getErrorMessage, | ||
| }); | ||
| } catch (error) { | ||
| console.error('Chat API Error:', error); | ||
| return new Response( | ||
| JSON.stringify({ | ||
| error: 'Failed to process chat request', | ||
| details: getErrorMessage(error), | ||
| const errorResponse = new Response( | ||
| JSON.stringify({ | ||
| error: "Documentation service temporarily unavailable", | ||
| details: "Our documentation service is currently unreachable. Please try again in a moment, or visit https://docs.stack-auth.com directly for help.", | ||
| }), | ||
| { status: 503, headers: { "content-type": "application/json" } } | ||
| ); | ||
|
|
||
| const backendResponse = await fetch( | ||
| `${backendBaseUrl}/api/latest/ai/query/stream`, | ||
| { | ||
| method: "POST", | ||
| headers: requestHeaders, | ||
| body: JSON.stringify({ | ||
| quality: "smart", | ||
| speed: "fast", | ||
| systemPrompt: "docs-ask-ai", | ||
| tools: ["docs"], | ||
| messages: modelMessages, | ||
| }), | ||
| { | ||
| status: 500, | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| } | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
as with the other AI endpoints, these calls should be on the frontend
Collaborator
Author
There was a problem hiding this comment.
removed the endpoint
Comment on lines
100
to
108
| @@ -113,6 +106,22 @@ export type StackAdminApp<HasTokenStore extends boolean = boolean, ProjectId ext | |||
| domPath: Array<{ tagName: string, index: number }>, | |||
| htmlContext: string, | |||
| }): Promise<{ updatedSource: string }>, | |||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What has been done in this PR:
We now use a single endpoint throughout the codebase that makes the call to openrouter. Specifically, email drafts, email templates, email themes, wysiwyg, cmd centre ai search and docs ai, all use this unified ai endpoint. All the tools are defined in the backend, all the prompts exist in the backend. This is roughly how the endpoint works.
→ if not, return 400 error
→ if empty, return 500 error
→ forward the entire request to production API
→ return whatever production returns
→ call streamText and pipe the response back as a live stream
→ call generateText and wait for full completion
→ collect all text blocks and tool calls from every step and return
How to review this PR:
This PR will be easier to review if we look at the different folders that were affected.
under packages - We added streaming functionality, and made renaming changes
under docs there are three files that have changed
under backend
under dashboard
Summary by CodeRabbit
New Features
Refactor
Chores