Closed
Conversation
This reverts commit 937db90.
Co-authored-by: Konsti Wohlwend <n2d4xc@gmail.com>
…er/route.ts Co-authored-by: Konsti Wohlwend <n2d4xc@gmail.com>
…er/route.ts Co-authored-by: Konsti Wohlwend <n2d4xc@gmail.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughReworks backend AI surface and tools, adds model selection, central prompts and validation, introduces unified /api/latest/ai/query route (stream/generate) with optional production forwarding, removes legacy chat/WYSIWYG adapters, and ships a new Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client App
participant Route as /api/latest/ai/query
participant Forward as Production Forwarder
participant Model as Model Selector
participant Tools as Tools Provider
participant ModelExec as Model Provider
Client->>Route: POST {mode, quality, speed, systemPrompt, tools, messages}
Route->>Route: validate request & tool names
alt FORWARD_TO_PRODUCTION
Route->>Forward: POST proxied body + x-stack-* headers
Forward-->>Route: production Response
Route-->>Client: proxied Response
else Local handling
Route->>Model: selectModel(quality,speed,isAuth)
Model-->>Route: model instance
Route->>Tools: getTools(toolNames, context)
Tools-->>Route: toolSet
Route->>ModelExec: invoke (streamText/generateText) with model & tools
ModelExec-->>Route: text / structured content (tool-calls)
Route->>Tools: execute any tool calls (if present)
Tools-->>Route: tool results
Route-->>Client: stream or JSON response with text + toolCall blocks
end
sequenceDiagram
participant User as User
participant UI as Dashboard Editor
participant Chat as Assistant Chat
participant Backend as Backend AI (/api/latest/ai/query)
participant Sandbox as DashboardSandbox (iframe)
User->>UI: Request dashboard generation/edit
UI->>Chat: send messages
Chat->>Backend: generateDashboardCode(messages, context)
Backend->>Backend: build messages (types, routes, currentSource)
Backend-->>Chat: {content, toolCall(updateDashboard)}
Chat->>UI: display content and tool UI
UI->>Sandbox: render runtimeCode in iframe
Sandbox->>UI: postMessage events (widget edits, grid state)
User->>UI: save dashboard → PUT config (customDashboards)
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
🧪 Generate unit tests (beta)
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces custom dashboards in the dashboard app and migrates multiple “AI chat / WYSIWYG” flows to a unified backend AI query endpoint (stream + generate), alongside packaging a reusable dashboard-ui-components library.
Changes:
- Add custom dashboard config schema/defaults and dashboard UI pages (list + detail) to create/manage dashboards stored in config.
- Introduce a unified backend AI endpoint (
/api/latest/ai/query/{stream|generate}) with tool plumbing (docs MCP, sql-query, email tools, update-dashboard) and forward-to-production support. - Extract and publish a
@stackframe/dashboard-ui-componentspackage and wire dashboard UI to consume it.
Reviewed changes
Copilot reviewed 108 out of 114 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
pnpm-workspace.yaml |
Adds pnpm minimumReleaseAge exclude for @ai-sdk/anthropic. |
packages/template/src/lib/stack-app/apps/interfaces/admin-app.ts |
Removes legacy admin chat + WYSIWYG methods from the template admin app interface. |
packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts |
Removes implementation passthroughs for legacy admin chat + WYSIWYG calls. |
packages/stack-shared/src/utils/react.tsx |
Enhances useRefState to support lazy initialization. |
packages/stack-shared/src/schema-fields.ts |
Adds customDashboardsSchema. |
packages/stack-shared/src/config/schema.ts |
Adds customDashboards to config schema + defaults. |
packages/stack-shared/src/config/schema-fuzzer.test.ts |
Extends config fuzzer to cover customDashboards. |
packages/dashboard-ui-components/* |
New reusable dashboard design/grid/chart components package with build tooling and scripts. |
docs/src/components/chat/ai-chat.tsx |
Switches docs chat to DefaultChatTransport against unified AI endpoint. |
docs/src/app/api/chat/route.ts |
Deletes old docs chat API route implementation. |
docs/package.json |
Upgrades AI SDK deps (ai@^6, @ai-sdk/react@^3). |
apps/e2e/tests/backend/.../wysiwyg-edit.test.ts |
Migrates WYSIWYG tests to unified AI endpoint. |
apps/e2e/tests/backend/.../ai-query.test.ts |
Adds E2E coverage for unified AI endpoint validation/auth/tools/system prompts. |
apps/dashboard/src/lib/api-headers.ts |
Adds helper to build Stack Auth headers for frontend → backend calls. |
apps/dashboard/src/lib/ai-dashboard/* |
Adds shared prompt/context building and zod contracts for dashboard codegen. |
apps/dashboard/src/components/vibe-coding/* |
Adds dashboard-specific tool UI + chat adapters; adds WYSIWYG edit helper via unified endpoint. |
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/dashboards/* |
Adds dashboards list + detail routes for custom dashboards. |
apps/backend/src/app/api/latest/ai/query/[mode]/route.ts |
Adds unified AI endpoint implementation (stream/generate) and forwarding behavior. |
apps/backend/src/lib/ai/* |
Adds AI request schema, model selection, tools registry, MCP docs tool, SQL tool, dashboard tool. |
apps/backend/src/route-handlers/smart-response.tsx |
Adds bodyType: "response" support for returning raw Response objects. |
apps/backend/.env.development |
Changes default OpenRouter handling to forward to production. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| system: systemPrompt, | ||
| messages: messages as ModelMessage[], | ||
| tools: toolsArg, | ||
| stopWhen: stepCountIs(stepLimit), |
There was a problem hiding this comment.
The generate-mode timeout is currently ineffective: an AbortController is created and scheduled to abort after 120s, but its signal is never passed to generateText. This means long-running AI requests may run indefinitely and ignore the intended timeout. Pass abortSignal: controller.signal to generateText (or use an existing timeout helper) so the request is actually aborted on timeout.
Suggested change
| stopWhen: stepCountIs(stepLimit), | |
| stopWhen: stepCountIs(stepLimit), | |
| abortSignal: controller.signal, |
Comment on lines
+85
to
+92
| <DesignListItemRow | ||
| key={dashboard.id} | ||
| icon={ChartBarIcon} | ||
| title={dashboard.displayName} | ||
| size="lg" | ||
| onClick={() => router.push(`dashboards/${dashboard.id}`)} | ||
| buttons={[ | ||
| { |
There was a problem hiding this comment.
router.push is being called with a relative path (e.g. dashboards/${id}). Next.js navigation expects an absolute path, and this can resolve incorrectly (e.g. duplicating dashboards/ in the URL) depending on the current route. Use an absolute route like /projects/${projectId}/dashboards/${id} (or otherwise ensure the path starts with /).
Comment on lines
+65
to
+70
| function useDashboardId(): string { | ||
| const pathname = usePathname(); | ||
| const parts = pathname.split("/"); | ||
| const dashboardsIdx = parts.indexOf("dashboards"); | ||
| return parts[dashboardsIdx + 1] ?? throwErr("Dashboard ID not found in path"); | ||
| } |
There was a problem hiding this comment.
useDashboardId doesn’t handle the case where the pathname does not contain the dashboards segment: parts.indexOf("dashboards") will return -1 and the function will return parts[0] instead of throwing. Add an explicit check for dashboardsIdx === -1 (and for dashboardsIdx + 1 being in range) to avoid silently using an incorrect dashboardId.
Comment on lines
+20
to
+52
| /** | ||
| * Sanitizes AI-generated JSX/TSX code before it is applied to the 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; |
There was a problem hiding this comment.
The comment for sanitizeGeneratedCode says it handles “Bare & in JSX text content”, but the implementation doesn’t escape/encode bare & (it actually decodes & back to &). This is misleading and can reintroduce invalid JSX. Either implement the described escaping behavior or update the comment to match what the function actually does.
|
|
||
| STACK_OPENROUTER_API_KEY=mock-openrouter-api-key | ||
|
|
||
| STACK_OPENROUTER_API_KEY=FORWARD_TO_PRODUCTION |
There was a problem hiding this comment.
Setting STACK_OPENROUTER_API_KEY=FORWARD_TO_PRODUCTION in the default dev env causes local /api/latest/ai/query/* requests to be forwarded to production. This can have unintended operational/security impacts (local data or test traffic hitting prod, harder debugging, surprising costs). Consider keeping the default as a local/mock key and requiring an explicit opt-in env var to enable production forwarding.
Suggested change
| STACK_OPENROUTER_API_KEY=FORWARD_TO_PRODUCTION | |
| STACK_OPENROUTER_API_KEY=mock_openrouter_api_key |
![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: 2
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/widget-playground/page-client.tsx (2)
252-258:⚠️ Potential issue | 🟠 MajorMutating module-level
widgetsarray won't trigger React re-render.Pushing to the
widgetsarray at module scope doesn't cause any component to re-render, so the newly saved widget won't appear in the grid until the page is refreshed. If the intent is for the widget to appear immediately, consider liftingwidgetsto React state or using a ref with a forced re-render.🤖 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]/widget-playground/page-client.tsx around lines 252 - 258, The code is mutating a module-level widgets array (widgets) with widgets.push(...) which won't trigger React updates; change widgets to React state inside the component (e.g., const [widgets, setWidgets] = useState<WidgetType[]>(initialWidgets)) and replace the push call in the save flow (where deserializeWidget, generateUuid, compilationResult are used) with setWidgets(prev => [...prev, await deserializeWidget({...})]) so the UI re-renders immediately; remove the module-scope mutation and ensure any code that reads widgets uses the state variable instead.
244-259:⚠️ Potential issue | 🟠 MajorWrap async
onClickhandlers withrunAsynchronouslyorrunAsynchronouslyWithAlert.Both Button onClick handlers are async but not wrapped with proper error handling. If
compileWidgetSourceordeserializeWidgetthrows, the error will be an unhandled promise rejection.As per coding guidelines: "Use
runAsynchronouslyorrunAsynchronouslyWithAlertinstead for error handling."Suggested fix
- <Button onClick={async () => { - const result = await compileWidgetSource(source); - setCompilationResult(result); + <Button onClick={() => runAsynchronouslyWithAlert(async () => { + const result = await compileWidgetSource(source); + setCompilationResult(result); }}>Compile</Button> {compilationResult?.status === "ok" && ( <> <textarea style={{ fontFamily: "monospace", width: '100%', height: '35%' }} value={compilationResult.data} readOnly /> - <Button onClick={async () => { - widgets.push(await deserializeWidget({ + <Button onClick={() => runAsynchronouslyWithAlert(async () => { + const widget = await deserializeWidget({ id: generateUuid(), version: 1, sourceJs: compilationResult.data, compilationResult: Result.ok(compilationResult.data), - })); + }); + widgets.push(widget); alert("Widget saved"); - }}>Save as widget</Button> + })}>Save as widget</Button>Note: You'll need to import
runAsynchronouslyWithAlertfrom@stackframe/stack-shared/dist/utils/promises.🤖 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]/widget-playground/page-client.tsx around lines 244 - 259, The two async onClick handlers that call compileWidgetSource and deserializeWidget must be wrapped with runAsynchronouslyWithAlert to avoid unhandled promise rejections; replace the inline async arrow functions for the Compile Button and the Save as widget Button by invoking runAsynchronouslyWithAlert(() => { ... }) and move the existing await logic inside that callback, keeping calls to setCompilationResult, widgets.push, generateUuid and Result.ok intact; also add the import for runAsynchronouslyWithAlert from `@stackframe/stack-shared/dist/utils/promises` at the top of the file.packages/dashboard-ui-components/src/components/cursor-blast-effect.tsx (1)
205-292:⚠️ Potential issue | 🟠 MajorAvoid
dangerouslySetInnerHTMLfor static CSS.This turns a plain
<style>block into a security-linted injection sink for no gain. Rendering the CSS as a normal child string keeps the same behavior and avoids the current Biome error.🔒 Proposed fix
- <style dangerouslySetInnerHTML={{ __html: ` + <style>{` .cursor-blast-ring { position: absolute; inset: 0; border-radius: 999px; border: 2px solid hsl(197 98% 67% / 0.9); box-shadow: 0 0 22px hsl(191 100% 72% / 0.6), inset 0 0 12px hsl(204 100% 77% / 0.65); animation: blast-ring 560ms cubic-bezier(0.16, 1, 0.3, 1) forwards; } .cursor-blast-core { position: absolute; left: 50%; top: 50%; width: 10px; height: 10px; border-radius: 999px; transform: translate(-50%, -50%); background: hsl(196 100% 85%); box-shadow: 0 0 26px hsl(193 100% 72% / 0.9), 0 0 10px hsl(201 100% 85% / 0.9); animation: blast-core 420ms ease-out forwards; } .cursor-blast-shard-wrap { position: absolute; left: 50%; top: 50%; width: 0; height: 0; } .cursor-blast-shard { position: absolute; left: 0; top: -1.5px; width: 12px; height: 3px; border-radius: 999px; background: linear-gradient(90deg, hsl(190 100% 84%), hsl(197 98% 67%)); box-shadow: 0 0 12px hsl(195 100% 70% / 0.8); animation: blast-shard 680ms cubic-bezier(0.22, 1, 0.36, 1) forwards; } `@keyframes` blast-ring { 0% { transform: scale(0.2); opacity: 0.95; } 100% { transform: scale(1.6); opacity: 0; } } `@keyframes` blast-core { 0% { transform: translate(-50%, -50%) scale(0.5); opacity: 1; } 100% { transform: translate(-50%, -50%) scale(2.2); opacity: 0; } } `@keyframes` blast-shard { 0% { transform: translateX(0) scaleX(0.7); opacity: 1; } 100% { transform: translateX(46px) scaleX(1.1); opacity: 0; } } `@media` (prefers-reduced-motion: reduce) { .cursor-blast-ring, .cursor-blast-core, .cursor-blast-shard { animation-duration: 1ms; } } - ` }} /> + `}</style>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/dashboard-ui-components/src/components/cursor-blast-effect.tsx` around lines 205 - 292, The inline <style> element in the CursorBlastEffect component currently uses dangerouslySetInnerHTML which creates an unnecessary injection sink; replace the dangerouslySetInnerHTML usage in the JSX style element with a normal child string (e.g. <style>{`...css...`}</style>) containing the same CSS (all .cursor-blast-* rules and `@keyframes`) so the CSS is rendered as a plain string child instead of via dangerouslySetInnerHTML, leaving class names and media query logic unchanged and ensuring no other logic in the component (CursorBlastEffect) depends on innerHTML.apps/dashboard/src/components/design-components/list.tsx (1)
127-149:⚠️ Potential issue | 🟠 MajorPrevent row activation when trailing actions are used.
Making the outer row clickable means clicks and keyboard events from
ListItemButtonsnow bubble up and also invoke the row handler. That will cause action buttons/menus to trigger the row action as well. Please stop propagation from the action area or make only the non-action content region clickable.Also applies to: 155-180
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/design-components/list.tsx` around lines 127 - 149, The row is fully clickable (interactiveProps spread on the outer div), so clicks/keyboard events from the action area (buttons rendered by ListItemButtons) bubble and trigger the row onClick; fix by preventing propagation from the action area: add event handlers to the buttons container (where ListItemButtons is rendered) or inside the ListItemButtons root to call e.stopPropagation() for onClick and e.stopPropagation() for onKeyDown/onKeyUp (to cover keyboard activation), or alternatively move interactiveProps/onClick to the main content wrapper (the div with "flex-1 min-w-0") so only non-action content is clickable; reference ListItemButtons, interactiveProps, onClick, buttons, and the outer div when making the change.apps/backend/src/app/api/latest/internal/ai-chat/[threadId]/route.tsx (1)
69-69:⚠️ Potential issue | 🟡 MinorUnsafe type cast from Prisma JsonValue.
The cast
as object[]assumes all stored message contents are objects. If a malformed message was saved (e.g., a primitive or array), this cast would silently pass but could cause runtime errors in consuming code.🛡️ Suggested defensive approach
- const messages = dbMessages.map((message) => message.content) as object[]; + const messages = dbMessages.map((message) => { + // Content should be an object; filter out malformed entries + if (typeof message.content !== 'object' || message.content === null) { + console.warn(`Malformed message content in thread ${params.threadId}`); + return null; + } + return message.content; + }).filter((m): m is object => m !== null);🤖 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 at line 69, The code unsafely casts Prisma JsonValue to object[] via the variable messages from dbMessages.map(...), so replace the direct cast with a safe validation step: iterate dbMessages (the mapped element used in route.tsx), perform a runtime type-guard for the expected message shape (e.g., typeof === "object" && !Array.isArray(...) and check required fields) and either transform to the correct typed shape, filter out or throw/log malformed entries, and return a properly typed array (not `as object[]`) so downstream consumers of messages get validated objects or a clear error/logging path.apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/email-themes/[themeId]/page-client.tsx (1)
39-52:⚠️ Potential issue | 🟠 MajorGuard against stale WYSIWYG responses overwriting newer code.
This callback captures
currentCode, awaits a network round-trip, and then unconditionally callssetCurrentCode. If two commits are in flight, an older response can resolve last and clobber newer edits.🛡️ Proposed fix
-import { useCallback, useEffect, useState } from "react"; +import { useCallback, useEffect, useRef, useState } from "react"; ... + const wysiwygRequestIdRef = useRef(0); ... const handleWysiwygEditCommit: OnWysiwygEditCommit = useCallback(async (data) => { + const requestId = ++wysiwygRequestIdRef.current; + const sourceCode = currentCode; const result = await applyWysiwygEdit(backendBaseUrl, { currentUser, sourceType: 'theme', - sourceCode: currentCode, + sourceCode, oldText: data.oldText, newText: data.newText, metadata: data.metadata, domPath: data.domPath, htmlContext: data.htmlContext, }); - setCurrentCode(result.updatedSource); + if (wysiwygRequestIdRef.current === requestId) { + setCurrentCode(result.updatedSource); + } return result.updatedSource; }, [backendBaseUrl, currentCode, currentUser]);🤖 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 around lines 39 - 52, The handleWysiwygEditCommit callback can let an older async response clobber newer edits; fix it by capturing a snapshot of currentCode at the start (e.g., const baseCode = currentCode) before calling applyWysiwygEdit, then after the await compare the latest currentCode (read from state or a ref) to baseCode and only call setCurrentCode(result.updatedSource) if they match (or otherwise implement a merge/ignore strategy); update references in the function (handleWysiwygEditCommit, applyWysiwygEdit, setCurrentCode, currentCode) accordingly so stale responses do not overwrite newer code.
🟡 Minor comments (13)
apps/backend/src/route-handlers/smart-response.tsx-112-121 (1)
112-121:⚠️ Potential issue | 🟡 MinorVariable naming should reflect Response body types, not just ArrayBuffer.
The variable
arrayBufferBodycan holdReadableStream<Uint8Array> | null(from Response.body),Uint8Array(from TextEncoder), orArrayBuffer. WhileResponseconstructor accepts all these as validBodyInittypes, the name is misleading. Consider renaming toresponseBodyfor clarity across all cases.🤖 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 112 - 121, The variable name arrayBufferBody is misleading because in the "response" case (and where it's later used) it can be ReadableStream<Uint8Array>, Uint8Array, or ArrayBuffer; rename arrayBufferBody to responseBody across the function to reflect BodyInit semantics, update the assignment in the case "response" block (where isResponseBody(obj.body) is checked and obj.body.body assigned), and update all subsequent references/usages (including where headers are set and the Response/BodyInit is passed) to use responseBody instead of arrayBufferBody.apps/dashboard/src/components/commands/create-dashboard/dashboard-sandbox-host.tsx-541-543 (1)
541-543:⚠️ Potential issue | 🟡 MinorSilent fallback to localhost may mask configuration issues.
Per coding guidelines, avoid silently using fallback values. If
NEXT_PUBLIC_STACK_API_URLis missing in production, this will silently connect to localhost, causing confusing failures.🛡️ Proposed fix: throw on missing env var
const baseUrl = useMemo(() => { - return getPublicEnvVar("NEXT_PUBLIC_STACK_API_URL") ?? 'http://localhost:8102'; + const url = getPublicEnvVar("NEXT_PUBLIC_STACK_API_URL"); + if (!url) { + if (process.env.NODE_ENV === "development") { + return 'http://localhost:8102'; + } + throw new Error("NEXT_PUBLIC_STACK_API_URL environment variable is required"); + } + return url; }, []);As per coding guidelines: "NEVER silently use fallback values when type errors occur. If a state should never happen due to higher-level logic, either update the types or throw an error."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/commands/create-dashboard/dashboard-sandbox-host.tsx` around lines 541 - 543, Replace the silent fallback in the useMemo that computes baseUrl: instead of defaulting to 'http://localhost:8102' when getPublicEnvVar("NEXT_PUBLIC_STACK_API_URL") returns undefined, validate the result and throw a clear error (including the env var name) when the value is missing in non-development environments so misconfiguration surfaces immediately; keep a guarded dev-only fallback only if NODE_ENV === 'development' (otherwise throw). Update the useMemo that defines baseUrl and reference getPublicEnvVar("NEXT_PUBLIC_STACK_API_URL") and baseUrl in the error message so reviewers can easily find and verify the change.apps/dashboard/src/components/commands/create-dashboard/dashboard-sandbox-host.tsx-556-590 (1)
556-590:⚠️ Potential issue | 🟡 MinorRace condition: messages may be lost if iframe isn't ready.
These
useEffecthooks post messages to the iframe immediately when props change. However, if the iframe'scontentWindowhasn't finished loading (e.g., Babel/dependencies still loading), messages sent during this window are lost. The iframe emits'stack-ai-dashboard-ready'when ready, but these effects don't wait for it.This could cause the initial theme, chat state, or grid state to not be applied.
♻️ Suggested approach: track iframe readiness
Add a state to track when the iframe signals it's ready:
const [iframeReady, setIframeReady] = useState(false); // In the message handler effect, set ready state: if (type === "stack-ai-dashboard-ready") { setIframeReady(true); return; }Then guard the postMessage effects:
useEffect(() => { - if (iframeRef.current?.contentWindow) { + if (iframeReady && iframeRef.current?.contentWindow) { iframeRef.current.contentWindow.postMessage({ type: 'stack-theme-change', theme: resolvedTheme, }, '*'); } - }, [resolvedTheme]); + }, [resolvedTheme, iframeReady]);Apply similarly to the other postMessage effects. Also consider sending all current state when
iframeReadybecomestrue.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/commands/create-dashboard/dashboard-sandbox-host.tsx` around lines 556 - 590, The useEffect hooks sending postMessage to iframeRef.current.contentWindow (for resolvedTheme, isChatOpen, layoutEditing, savedGridState) can lose messages if the iframe isn't loaded; add a boolean state (e.g., iframeReady via useState) that you set to true when the message handler sees type === 'stack-ai-dashboard-ready', and change each postMessage effect to early-return unless iframeReady is true (or trigger a single send-all effect when iframeReady flips true to send theme, chatOpen, editing, serializedGrid together). Ensure you reference iframeRef, the existing message handler that inspects 'stack-ai-dashboard-ready', and the state variables resolvedTheme, isChatOpen, layoutEditing, savedGridState when implementing the guard.apps/dashboard/src/components/design-components/subpage-header.tsx-13-38 (1)
13-38:⚠️ Potential issue | 🟡 MinorAdd SubpageHeader to the dashboard Playground.
SubpageHeader is missing from the Playground component showcase. Per the coding guidelines, all design components added or modified in the dashboard must be showcased in the Playground page. Add an entry to the COMPONENT_LIST in
apps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/playground/page-client.tsxand implement the corresponding preview and controls sections following the pattern of existing components.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/design-components/subpage-header.tsx` around lines 13 - 38, Add SubpageHeader to the Playground by updating the COMPONENT_LIST in page-client.tsx: import SubpageHeader and add a new component entry (e.g., id: 'SubpageHeader') with a preview function that renders <SubpageHeader title={title} onBack={handleBack} actions={actionsPreview}/> and a controls function that exposes props for title (string), actions (toggle to show an example action button), and a back handler (use a noop or action logger). Follow the pattern used by other entries in COMPONENT_LIST for preview and controls implementations, reusing the same UI controls utilities and state handling so playground toggles update the rendered preview. Ensure the new entry appears in the exported COMPONENT_LIST so the Playground shows the SubpageHeader.packages/dashboard-ui-components/scripts/copy-iife.ts-16-21 (1)
16-21:⚠️ Potential issue | 🟡 MinorKeep the renamed bundle and sourcemap filename in sync.
This copies the JS to
dashboard-ui-components.iife.jsand the map todashboard-ui-components.iife.js.map, but the bundle you copied was originally built asdashboard-ui-components.global.js. Unless you also rewrite itssourceMappingURLfooter, DevTools will still look for the old map name and miss the copied file.Suggested fix
-fs.copyFileSync(src, dest); +const bundle = fs + .readFileSync(src, 'utf8') + .replace( + /\/\/# sourceMappingURL=dashboard-ui-components\.global\.js\.map$/, + '//# sourceMappingURL=dashboard-ui-components.iife.js.map', + ); +fs.writeFileSync(dest, bundle); console.log('[copy-iife] Copied IIFE bundle to', dest);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/dashboard-ui-components/scripts/copy-iife.ts` around lines 16 - 21, The copied bundle and sourcemap filenames are out of sync: after copying src -> dest and srcMap -> destMap (variables src, dest, srcMap, destMap), update the bundle's sourceMappingURL footer in the copied dest file to point at the new destMap filename (or alternatively copy the map using the original map filename used inside the bundle). Open dest, replace the existing sourceMappingURL comment to reference path basename(destMap), and save so DevTools finds the copied sourcemap.packages/dashboard-ui-components/src/components/skeleton.tsx-10-12 (1)
10-12:⚠️ Potential issue | 🟡 MinorRespect reduced-motion for the default pulse animation.
animate-pulseruns unconditionally, so reduced-motion users still get animated loading placeholders. Gate it withmotion-safe:or explicitly disable it withmotion-reduce:animate-none.♿ Proposed fix
className={cn( - "animate-pulse rounded-md bg-black/[0.06] dark:bg-white/[0.06]", + "motion-safe:animate-pulse motion-reduce:animate-none rounded-md bg-black/[0.06] dark:bg-white/[0.06]", className )}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/dashboard-ui-components/src/components/skeleton.tsx` around lines 10 - 12, The Skeleton component's className currently applies "animate-pulse" unconditionally; update the class string used in the Skeleton component (where cn is called with the default "animate-pulse rounded-md bg-black/[0.06] dark:bg-white/[0.06]" and the className prop) to respect reduced motion by using motion-safe:animate-pulse and motion-reduce:animate-none instead of plain animate-pulse so users with reduced-motion preferences won't see the animation.packages/dashboard-ui-components/src/components/chart-tooltip.tsx-63-67 (1)
63-67:⚠️ Potential issue | 🟡 MinorPreserve falsy-but-valid tooltip labels.
Lines 63 and 146 treat
0and""as “missing” labels. Tooltip labels areReactNodes, so those are valid values and should not fall back. Use explicit null checks and??here.🔎 Suggested fix
- if (!value) { + if (value == null) { return null; } @@ - {itemConfig?.label || item.name} + {itemConfig?.label ?? item.name}Also applies to: 145-146
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/dashboard-ui-components/src/components/chart-tooltip.tsx` around lines 63 - 67, The tooltip code treats falsy-but-valid ReactNode labels like 0 or "" as missing; update the checks in the ChartTooltip component (the block that currently does `if (!value) return null;` and the similar logic around the other label at lines ~145-146) to only treat null/undefined as absent (use explicit null checks e.g., value == null) and use the nullish coalescing operator (??) for any fallback text so that 0 and empty string are preserved as valid labels; ensure you update both occurrences (the early return and the fallback rendering) and keep existing className handling (labelClassName / cn) intact.packages/dashboard-ui-components/src/components/chart-legend.tsx-37-43 (1)
37-43:⚠️ Potential issue | 🟡 MinorUse a stable React key for legend items.
Line 43 keys each pill by
item.value, but multiple series can legitimately share the same visible label. That causes React to reuse the wrong legend item when payload order changes. Prefer a key derived fromdataKeywith an index fallback.🧩 Suggested fix
- {payload.map((item) => { + {payload.map((item, index) => { const key = `${nameKey || item.dataKey || "value"}`; const itemConfig = getPayloadConfigFromPayload(config, item, key); return ( <div - key={item.value} + key={String(item.dataKey ?? item.value ?? index)} className={cn(🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/dashboard-ui-components/src/components/chart-legend.tsx` around lines 37 - 43, The legend uses unstable keys (key={item.value}) inside payload.map which can collide when multiple series share the same visible label; update the payload.map callback (add the index parameter) and build a stable key using the series dataKey (item.dataKey) or the computed key (the existing key variable/nameKey fallback) with an index fallback (e.g., `${item.dataKey ?? key}-${index}`) so each legend item rendered by the component (where getPayloadConfigFromPayload and nameKey are used) has a unique, stable React key.apps/backend/src/lib/ai/prompts.ts-180-182 (1)
180-182:⚠️ Potential issue | 🟡 MinorMinor typo in prompt text.
Line 181 contains "Product to help with related Stack Auth topics" which appears to be a typo. Should likely be "Proceed to help" or "Offer to help".
✏️ Proposed fix
- Avoid saying things are "not possible" or "impossible", instead say that you don't know - Ask clarifying questions to better understand the user's needs -- Product to help with related Stack Auth topics that might be useful +- Offer to help with related Stack Auth topics that might be useful - Provide the best information you can based on your knowledge, but acknowledge limitations🤖 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` around lines 180 - 182, There's a minor typo in the prompt text: replace the string "Product to help with related Stack Auth topics that might be useful" with a corrected phrase like "Offer to help with related Stack Auth topics that might be useful" (locate the exact prompt entry containing "Product to help with related Stack Auth topics" in the prompts file and update it accordingly).apps/dashboard/src/lib/api-headers.ts-14-19 (1)
14-19:⚠️ Potential issue | 🟡 MinorSilent fallback for missing publishable client key.
Using
?? ""for the publishable client key violates the coding guideline to fail early rather than silently continue with fallback values. An empty key will likely cause confusing authentication failures downstream.As per coding guidelines: "NEVER silently use fallback values when type errors occur... Fail early, fail loud."
🛡️ Proposed fix
+import { throwErr } from "@stackframe/stack-shared/dist/utils/errors"; + export async function buildStackAuthHeaders( currentUser?: CurrentUser | null, ): Promise<Record<string, string>> { const accessToken = currentUser ? await currentUser.getAccessToken() : null; return { "x-stack-access-type": "client", "x-stack-project-id": "internal", - "x-stack-publishable-client-key": getPublicEnvVar("NEXT_PUBLIC_STACK_PUBLISHABLE_CLIENT_KEY") ?? "", + "x-stack-publishable-client-key": getPublicEnvVar("NEXT_PUBLIC_STACK_PUBLISHABLE_CLIENT_KEY") ?? throwErr("NEXT_PUBLIC_STACK_PUBLISHABLE_CLIENT_KEY is not set"), ...(accessToken ? { "x-stack-access-token": accessToken } : {}), }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/lib/api-headers.ts` around lines 14 - 19, The header builder silently falls back to an empty publishable key; instead fail early when getPublicEnvVar("NEXT_PUBLIC_STACK_PUBLISHABLE_CLIENT_KEY") returns null/undefined. Update the function that returns these headers to validate the result of getPublicEnvVar and throw a clear Error (or assert) if the publishable client key is missing, rather than using ?? ""; reference the "x-stack-publishable-client-key" header and the getPublicEnvVar call so the missing-key check is performed before returning the headers (leave the conditional spread for accessToken unchanged).packages/dashboard-ui-components/src/components/grid-layout/resize-handle.tsx-16-18 (1)
16-18:⚠️ Potential issue | 🟡 MinorReject the
(0, 0)handle too.The current guard accepts a center handle, but that does not map to any edge and turns the middle of the widget into a dead resize hotspot.
As per coding guidelines, "Fail early, fail loud. Fail fast with an error instead of silently continuing."🛡️ Suggested change
- if (![ -1, 0, 1 ].includes(x) || ![ -1, 0, 1 ].includes(y)) { + if ((x === 0 && y === 0) || ![ -1, 0, 1 ].includes(x) || ![ -1, 0, 1 ].includes(y)) { throw new StackAssertionError(`Invalid resize handle coordinates, must be -1, 0, or 1: ${x}, ${y}`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/dashboard-ui-components/src/components/grid-layout/resize-handle.tsx` around lines 16 - 18, The coordinate validation currently allows the center handle (0,0) which is invalid; update the guard in the resize handle validation (the block that throws StackAssertionError) to also reject the case where x === 0 && y === 0 so a center handle triggers the same StackAssertionError; locate the validation in resize-handle.tsx (the function or component that checks x and y) and add the extra condition to fail fast when both coordinates are zero.packages/dashboard-ui-components/src/components/grid-layout/resize-handle.tsx-83-91 (1)
83-91:⚠️ Potential issue | 🟡 MinorOnly start resizing on the primary mouse button.
Right- and middle-clicks currently enter the drag path too, which suppresses the browser gesture and can resize accidentally.
🖱️ Suggested change
onMouseDown={(event) => { + if (event.button !== 0) return; dragBaseCoordinates.set([event.clientX, event.clientY]); window.addEventListener('mouseup', () => { dragBaseCoordinates.set(null);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/dashboard-ui-components/src/components/grid-layout/resize-handle.tsx` around lines 83 - 91, The onMouseDown handler starts a resize for any mouse button; change it to only begin on the primary button by checking event.button === 0 before setting dragBaseCoordinates, adding the window.addEventListener('mouseup', ...) or any cleanup and calling event.preventDefault()/event.stopPropagation() only when event.button === 0; update the handler where dragBaseCoordinates.set is used (the onMouseDown inline function) so right- and middle-clicks do nothing.apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/dashboards/page-client.tsx-169-171 (1)
169-171:⚠️ Potential issue | 🟡 MinorTrim the dashboard name before validating it.
A value like
" "passes.min(1)today and creates a blank-looking dashboard entry.🧹 Suggested change
- name: yup.string().defined().min(1, "Name is required"), + name: yup.string().trim().defined().min(1, "Name is required"),🤖 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]/dashboards/page-client.tsx around lines 169 - 171, The schema currently allows whitespace-only names because .min(1) runs on the raw value; update the formSchema's name rule (the yup.object entry for "name") to trim input before validating (e.g., use yup.string().trim() or a .transform(v => v?.trim()) prior to .min(1)/.defined()) so strings like " " are reduced to "" and will fail the minimum-length validation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 99805349-4f95-4b76-9f38-e53ff1787308
⛔ Files ignored due to path filters (4)
.cursor/debug-9f43b8.logis excluded by!**/*.logapps/dashboard/public/dashboard-ui-components.iife.js.mapis excluded by!**/*.mapapps/dashboard/src/generated/bundled-type-definitions.tsis excluded by!**/generated/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (110)
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/public/dashboard-ui-components.iife.jsapps/dashboard/scripts/bundle-type-definitions.tsapps/dashboard/src/app/(main)/(protected)/(outside-dashboard)/playground/page-client.tsxapps/dashboard/src/app/(main)/(protected)/layout-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/dashboards/[dashboardId]/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/dashboards/[dashboardId]/page.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/dashboards/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/dashboards/page.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/design-language/page-client.tsxapps/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/(main)/(protected)/projects/[projectId]/payments/products/[productId]/edit/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/payments/products/new/page-client.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/sidebar-layout.tsxapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/widget-playground/page-client.tsxapps/dashboard/src/app/api/ai-search/route.tsapps/dashboard/src/components/assistant-ui/thread.tsxapps/dashboard/src/components/cmdk-commands.tsxapps/dashboard/src/components/cmdk-search.tsxapps/dashboard/src/components/commands/ask-ai.tsxapps/dashboard/src/components/commands/create-dashboard/create-dashboard-preview.tsxapps/dashboard/src/components/commands/create-dashboard/dashboard-sandbox-host.tsxapps/dashboard/src/components/design-components/editable-grid.tsxapps/dashboard/src/components/design-components/index.tsapps/dashboard/src/components/design-components/list.tsxapps/dashboard/src/components/design-components/menu.tsxapps/dashboard/src/components/design-components/subpage-header.tsxapps/dashboard/src/components/editable-input.tsxapps/dashboard/src/components/stack-companion.tsxapps/dashboard/src/components/vibe-coding/assistant-chat.tsxapps/dashboard/src/components/vibe-coding/chat-adapters.tsapps/dashboard/src/components/vibe-coding/dashboard-tool-components.tsxapps/dashboard/src/components/vibe-coding/draft-tool-components.tsxapps/dashboard/src/components/vibe-coding/index.tsapps/dashboard/src/lib/ai-dashboard/contracts.tsapps/dashboard/src/lib/ai-dashboard/shared-prompt.tsapps/dashboard/src/lib/api-headers.tsapps/e2e/tests/backend/endpoints/api/v1/ai-model-config.tsapps/e2e/tests/backend/endpoints/api/v1/ai-query.test.tsapps/e2e/tests/backend/endpoints/api/v1/internal/wysiwyg-edit.test.tsdocs/package.jsondocs/src/app/api/chat/route.tsdocs/src/components/chat/ai-chat.tsxpackages/dashboard-ui-components/.eslintrc.cjspackages/dashboard-ui-components/package.jsonpackages/dashboard-ui-components/scripts/build-iife.tspackages/dashboard-ui-components/scripts/copy-iife.tspackages/dashboard-ui-components/src/components/alert.tsxpackages/dashboard-ui-components/src/components/badge.tsxpackages/dashboard-ui-components/src/components/button.tsxpackages/dashboard-ui-components/src/components/card.tsxpackages/dashboard-ui-components/src/components/chart-card.tsxpackages/dashboard-ui-components/src/components/chart-container.tsxpackages/dashboard-ui-components/src/components/chart-legend.tsxpackages/dashboard-ui-components/src/components/chart-theme.tsxpackages/dashboard-ui-components/src/components/chart-tooltip.tsxpackages/dashboard-ui-components/src/components/cursor-blast-effect.tsxpackages/dashboard-ui-components/src/components/edit-mode.tsxpackages/dashboard-ui-components/src/components/empty-state.tsxpackages/dashboard-ui-components/src/components/grid-layout/draggable.tsxpackages/dashboard-ui-components/src/components/grid-layout/grid-logic.tspackages/dashboard-ui-components/src/components/grid-layout/grid.tsxpackages/dashboard-ui-components/src/components/grid-layout/heading-widget.tsxpackages/dashboard-ui-components/src/components/grid-layout/index.tspackages/dashboard-ui-components/src/components/grid-layout/resize-handle.tsxpackages/dashboard-ui-components/src/components/grid-layout/types.tsxpackages/dashboard-ui-components/src/components/input.tsxpackages/dashboard-ui-components/src/components/metric-card.tsxpackages/dashboard-ui-components/src/components/pill-toggle.tsxpackages/dashboard-ui-components/src/components/progress-bar.tsxpackages/dashboard-ui-components/src/components/separator.tsxpackages/dashboard-ui-components/src/components/skeleton.tsxpackages/dashboard-ui-components/src/components/table.tsxpackages/dashboard-ui-components/src/components/tabs.tsxpackages/dashboard-ui-components/src/index.tspackages/dashboard-ui-components/tsconfig.jsonpackages/dashboard-ui-components/tsdown.config.tspackages/stack-shared/src/config/schema-fuzzer.test.tspackages/stack-shared/src/config/schema.tspackages/stack-shared/src/interface/admin-interface.tspackages/stack-shared/src/schema-fields.tspackages/stack-shared/src/utils/react.tsxpackages/template/src/lib/stack-app/apps/implementations/admin-app-impl.tspackages/template/src/lib/stack-app/apps/interfaces/admin-app.tspnpm-workspace.yaml
💤 Files with no reviewable changes (11)
- apps/backend/src/lib/ai-chat/adapter-registry.ts
- apps/dashboard/src/components/design-components/index.ts
- apps/backend/src/lib/ai-chat/email-theme-adapter.ts
- packages/template/src/lib/stack-app/apps/implementations/admin-app-impl.ts
- apps/backend/src/lib/ai-chat/email-template-adapter.ts
- apps/backend/src/lib/ai-chat/email-draft-adapter.ts
- packages/template/src/lib/stack-app/apps/interfaces/admin-app.ts
- apps/backend/src/app/api/latest/internal/wysiwyg-edit/route.tsx
- packages/stack-shared/src/interface/admin-interface.ts
- apps/dashboard/src/app/api/ai-search/route.ts
- docs/src/app/api/chat/route.ts
Comment on lines
+303
to
+312
| <DashboardSandboxHost | ||
| artifact={artifact} | ||
| onBack={handleBack} | ||
| onEditToggle={handleEditToggle} | ||
| onNavigate={handleNavigate} | ||
| onWidgetEditRequest={handleWidgetEditRequest} | ||
| onWidgetAddRequest={handleWidgetAddRequest} | ||
| onGridStateChange={handleGridStateChange} | ||
| savedGridState={gridState} | ||
| isChatOpen={isChatOpen} |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "DashboardSandboxHost contract:"
fd 'dashboard-sandbox-host.tsx' . -x sed -n '1,260p' {}
echo
echo "All references to savedGridState:"
rg -n -C3 '\bsavedGridState\b'Repository: stack-auth/stack-auth
Length of output: 19688
Pass the persisted savedGridState instead of the live gridState to this prop.
Line 311 incorrectly passes gridState (the current live state) to the savedGridState prop, but this prop is designed to receive the persisted baseline state. The component uses this baseline to establish the save/discard anchor in the sandbox iframe (sent via 'dashboard-saved-grid-state' message). Additionally, line 166 compares gridState against savedGridState to detect unsaved changes; passing the same value to both would break this detection entirely.
🤖 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]/dashboards/[dashboardId]/page-client.tsx
around lines 303 - 312, The DashboardSandboxHost call is passing the live
gridState to the savedGridState prop which breaks unsaved-change detection;
update the prop to pass the persisted baseline value (the variable that holds
the persisted/saved grid state, e.g., persistedSavedGridState or
savedGridStateFromStore) instead of gridState so that savedGridState !==
gridState when edits occur; modify the JSX prop savedGridState={gridState} to
savedGridState={persistedSavedGridState} (or the actual persisted variable name
used in this module) and keep all other handlers (handleBack, handleEditToggle,
handleNavigate, handleWidgetEditRequest, handleWidgetAddRequest,
handleGridStateChange, isChatOpen) unchanged.
Comment on lines
+1
to
+8
| import * as fs from 'node:fs'; | ||
| import * as path from 'node:path'; | ||
|
|
||
| const src = path.resolve(__dirname, '../dist/dashboard-ui-components.global.js'); | ||
| const srcMap = src + '.map'; | ||
| const destDir = path.resolve(__dirname, '../../../apps/dashboard/public'); | ||
| const dest = path.join(destDir, 'dashboard-ui-components.iife.js'); | ||
| const destMap = dest + '.map'; |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== dashboard-ui-components package.json =="
fd '^package\.json$' packages/dashboard-ui-components --exec sed -n '1,220p' {}
echo
echo "== script / module-format references =="
rg -n '"type"\s*:|"copy-iife"|"build-iife"|tsx|ts-node|node --loader|node --import' packages/dashboard-ui-components -g 'package.json'Repository: stack-auth/stack-auth
Length of output: 2307
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== tsconfig.json in packages/dashboard-ui-components =="
fd '^tsconfig\.json$' packages/dashboard-ui-components --exec cat {}
echo
echo "== Root tsconfig for reference =="
cat tsconfig.json | head -40
echo
echo "== Check if copy-iife.ts exists and its full content =="
cat packages/dashboard-ui-components/scripts/copy-iife.tsRepository: stack-auth/stack-auth
Length of output: 734
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Full content of copy-iife.ts =="
cat packages/dashboard-ui-components/scripts/copy-iife.ts
echo
echo "== Check if there's a tsconfig for scripts directory =="
find packages/dashboard-ui-components/scripts -name 'tsconfig*.json' -exec cat {} \;Repository: stack-auth/stack-auth
Length of output: 889
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check if the built IIFE bundle exists and has sourcemap reference =="
if [ -f "packages/dashboard-ui-components/dist/dashboard-ui-components.global.js" ]; then
echo "IIFE file exists, checking last 5 lines for sourcemap reference:"
tail -5 packages/dashboard-ui-components/dist/dashboard-ui-components.global.js
else
echo "IIFE file not found at expected location"
fi
echo
echo "== Check node_modules for tsx and version =="
fd '^package\.json$' node_modules/tsx --exec cat {}Repository: stack-auth/stack-auth
Length of output: 1848
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check for __dirname usage elsewhere in the codebase to see pattern ===="
rg -n '__dirname' --type ts --type tsx packages/dashboard-ui-components/scripts
echo
echo "== Check for import.meta.url usage pattern in scripts ===="
rg -n 'import.meta.url' --type ts --type tsx packages/dashboard-ui-components/scriptsRepository: stack-auth/stack-auth
Length of output: 184
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Check for __dirname usage in scripts ===="
rg -n '__dirname' packages/dashboard-ui-components/scripts/
echo
echo "== Check for import.meta.url usage in scripts ===="
rg -n 'import.meta.url' packages/dashboard-ui-components/scripts/
echo
echo "== Check build-iife.ts for comparison ===="
cat packages/dashboard-ui-components/scripts/build-iife.ts | head -30Repository: stack-auth/stack-auth
Length of output: 536
Use import.meta.url instead of __dirname for ESM compatibility.
The TypeScript configuration ("module": "ES2020") targets ES modules, where __dirname is not natively available. This script relies on tsx's implicit polyfilling of Node.js globals, which is undocumented and fragile. The standard approach is to derive the directory explicitly using import.meta.url.
Additionally, this pattern affects other scripts in the directory (e.g., build-iife.ts line 56), suggesting a systematic issue.
Suggested fix
import * as fs from 'node:fs';
import * as path from 'node:path';
+import { fileURLToPath } from 'node:url';
-const src = path.resolve(__dirname, '../dist/dashboard-ui-components.global.js');
+const currentDir = path.dirname(fileURLToPath(import.meta.url));
+const src = path.resolve(currentDir, '../dist/dashboard-ui-components.global.js');
const srcMap = src + '.map';
-const destDir = path.resolve(__dirname, '../../../apps/dashboard/public');
+const destDir = path.resolve(currentDir, '../../../apps/dashboard/public');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| import * as fs from 'node:fs'; | |
| import * as path from 'node:path'; | |
| const src = path.resolve(__dirname, '../dist/dashboard-ui-components.global.js'); | |
| const srcMap = src + '.map'; | |
| const destDir = path.resolve(__dirname, '../../../apps/dashboard/public'); | |
| const dest = path.join(destDir, 'dashboard-ui-components.iife.js'); | |
| const destMap = dest + '.map'; | |
| import * as fs from 'node:fs'; | |
| import * as path from 'node:path'; | |
| import { fileURLToPath } from 'node:url'; | |
| const currentDir = path.dirname(fileURLToPath(import.meta.url)); | |
| const src = path.resolve(currentDir, '../dist/dashboard-ui-components.global.js'); | |
| const srcMap = src + '.map'; | |
| const destDir = path.resolve(currentDir, '../../../apps/dashboard/public'); | |
| const dest = path.join(destDir, 'dashboard-ui-components.iife.js'); | |
| const destMap = dest + '.map'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/dashboard-ui-components/scripts/copy-iife.ts` around lines 1 - 8,
The script uses __dirname to build paths (variables src, srcMap, destDir, dest,
destMap) which breaks under ESM; replace that with an explicit ESM-safe
directory derived from import.meta.url by importing fileURLToPath from 'url' and
computing const __dirname = path.dirname(fileURLToPath(import.meta.url)), then
use that __dirname to resolve src and dest paths; apply the same change to the
other script mentioned (build-iife.ts) so all path computations use
import.meta.url instead of the non-ESM __dirname global.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
| body: JSON.stringify(body), | ||
| }); | ||
|
|
||
| const json = await response.json() as { content?: ChatContent }; |
There was a problem hiding this comment.
HTTP error responses (4xx/5xx) from the AI endpoint are silently swallowed, returning empty array instead of throwing an error
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/dashboard-ui-components/src/components/card.tsx (1)
95-105:⚠️ Potential issue | 🟡 MinorPropagate the effective
glassmorphicvalue to descendants.
DesignCategoryTabsand other children useuseGlassmorphicDefault()to read this context, so keeping the provider hardcoded totruemeans<DesignCard glassmorphic={false}>still makes nested components default back to glassmorphic styling. Pass the resolvedglassmorphicvalue through the provider instead.♻️ Proposed fix
- <DesignCardNestingContext.Provider value={true}> + <DesignCardNestingContext.Provider value={glassmorphic}>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/dashboard-ui-components/src/components/card.tsx` around lines 95 - 105, The provider currently forces nested components to always read glassmorphic=true; resolve and pass the effective glassmorphic boolean (the local variable glassmorphic computed from glassmorphicProp) into DesignCardNestingContext.Provider instead of the hardcoded true so descendants (e.g., DesignCategoryTabs and any components using useGlassmorphicDefault()) inherit the card's actual glassmorphic setting.
♻️ Duplicate comments (1)
apps/dashboard/src/app/(main)/(protected)/projects/[projectId]/dashboards/[dashboardId]/page-client.tsx (1)
283-291:⚠️ Potential issue | 🔴 CriticalPass the persisted grid baseline, not the live grid state.
Line 291 is still wiring
savedGridState={gridState}. That makes the sandbox’s saved baseline equal to the live state, so its unsaved-change detection for layout edits never has a stable anchor. This prop needs the persistedsavedGridStatestate instead.🤖 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]/dashboards/[dashboardId]/page-client.tsx around lines 283 - 291, The DashboardSandboxHost is being passed the live gridState instead of the persisted baseline, so change the savedGridState prop from savedGridState={gridState} to use the persisted state variable savedGridState (i.e., savedGridState={savedGridState}) so the sandbox compares edits against the stable persisted baseline; update the prop on the DashboardSandboxHost invocation and ensure the savedGridState state variable is the persisted value used elsewhere for saving/loading.
🤖 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]/dashboards/[dashboardId]/page-client.tsx:
- Around line 75-76: The code silently falls back to "" for the AI backend URL;
change the backendBaseUrl initialization (where
getPublicEnvVar("NEXT_PUBLIC_STACK_API_URL") is used alongside currentUser) to
fail fast by throwing a clear error when the env var is missing or empty (e.g.,
replace the nullish fallback with an immediate throw specifying "Missing
NEXT_PUBLIC_STACK_API_URL" or use your project's throwErr/assert helper), so any
misconfiguration surfaces immediately instead of leading to downstream chat
failures.
- Around line 37-47: The extractGridStateFromSource function currently swallows
JSON parse errors and returns null; change it to surface malformed payloads by
throwing a descriptive error (including GRID_STATE_PREFIX and an excerpt of the
corrupted json) or returning an explicit error object instead of null so callers
can render an error state; update the try/catch around JSON.parse in
extractGridStateFromSource to include context (tsxSource slice, start index, and
the parse error) and rethrow or return that error payload so corruption is not
silently lost.
- Around line 81-108: The current logic waits for hasEverExisted before
redirecting, causing a blank page for direct visits to an invalid dashboard; fix
by adding an explicit "config loaded but dashboard missing" path: detect when
config.customDashboards has finished loading (e.g., add or derive an
isConfigLoaded boolean from config or from the presence of
config.apps.installed), and in a useEffect (or extend the existing one that
references hasEverExisted and dashboard) immediately call
router.replace(`/projects/${projectId}/dashboards`) when isConfigLoaded is true
and dashboard is undefined; keep the existing hasEverExisted behavior for
preservation of edit-flow, but ensure the new check prevents silent null returns
by redirecting as soon as config load completes and the dashboard key is
missing.
In
`@apps/dashboard/src/components/commands/create-dashboard/dashboard-sandbox-host.tsx`:
- Around line 229-235: The sandbox is initializing window.__chatOpen and
window.__editPanelOpen but the host mapping wires isChatOpen to the edit panel
field; fix this by wiring the isChatOpen prop to window.__chatOpen and the edit
panel prop (e.g., isEditPanelOpen or similar) to window.__editPanelOpen in both
the initialization block (window.__chatOpen = ...) and the host-to-iframe
message mapping (where chatOpen / editPanelOpen are sent), or rename the prop to
match the intended field if the boolean actually represents the edit panel;
ensure the same corrected mapping is applied in the other occurrence noted (the
second block around the later initialization).
- Around line 579-582: When the iframe is recreated (srcDoc recalculated via
getSandboxDocument and assigned to srcDoc), current runtime state (theme,
edit/selecting flags, chat open, savedGridStateRef) must be resent to the new
document on iframe load; add an onLoad handler for the iframe that reads current
values (resolvedTheme or initialThemeRef.current,
isChatOpen/initialChatOpenRef.current, edit/select flags stored in state/refs)
and either (a) regenerate/send an updated srcDoc by calling getSandboxDocument
with those current values before mounting, or (b) postMessage the current state
into the iframe window so the sandbox bootstraps into the correct mode. Update
code around getSandboxDocument/srcDoc, and implement the iframe onLoad handler
to call window.postMessage or invoke a sandbox-ready callback so the iframe's
__layoutEditing, __selectingForEdit, theme, and grid state are set immediately
after the document is replaced.
- Around line 570-572: The baseUrl useMemo currently silently falls back to
'http://localhost:8102' when getPublicEnvVar("NEXT_PUBLIC_STACK_API_URL") is
missing; change it to fail fast by reading
getPublicEnvVar("NEXT_PUBLIC_STACK_API_URL") inside the useMemo and if it is
null/undefined then throw a clear Error in non-development environments, but
allow the explicit local fallback only when process.env.NODE_ENV ===
'development' (so keep the dev-only behavior), referencing the baseUrl constant
and getPublicEnvVar in dashboard-sandbox-host.tsx to locate and update the
logic.
- Around line 43-53: When the dashboard-ui-components IIFE fails to load, turn
the current silent log into a propagated init failure by updating the
script.onerror handler (the one that currently logs "Failed to load
dashboard-ui-components IIFE bundle") to dispatch the existing error signal used
by the UI: create and dispatch a CustomEvent('stack-ai-dashboard-error', {
detail: e }) (and optionally set a flag such as window.__depsReady = false)
instead of only console.error so that waitForDeps() and the error UI receive a
rejected init and show the error; ensure the handler references the same script
loading code that sets window.dispatchEvent(new Event('deps-ready')) so the
error flows through the existing error UI.
In `@packages/dashboard-ui-components/src/components/grid-layout/grid-logic.ts`:
- Around line 142-161: fromSerialized currently trusts serializedAny fields and
can throw opaque errors; validate the versioned payload shape before mapping by
asserting serializedAny.nonEmptyElements is an array and each element has
expected keys (instance, x, y, width, height), serializedAny.varHeights is an
array of entries with numeric y and instances array, and that
serializedAny.width and serializedAny.fixedHeight have the right types for
version 1; if any check fails, throw a StackAssertionError with the serialized
payload for context. Locate the checks in fromSerialized (WidgetInstanceGrid)
around the switch case for version 1 and perform these defensive assertions
before calling .map() or new WidgetInstanceGrid; then use
deserializeWidgetInstance only after validating the element/instance shapes so
runtime map errors are avoided and failures are explicit.
- Around line 816-821: The current loop over this.elements() treats synthetic
placeholder rectangles (created by _withEmptyElements()) as real elements,
causing canAddVarHeight() to return false incorrectly; update the loop in the
function that checks insertion at y (the block iterating "for (const element of
this.elements())") to skip those synthetic empty elements by detecting the
placeholder flag added by _withEmptyElements (e.g., element.isEmpty or
element.isPlaceholder or similar) — either filter this.elements() first or add a
continue when a placeholder is detected so only real elements contribute to the
overlap check used by canAddVarHeight().
- Around line 84-99: The layout can divide by zero when width < elemWidth inside
WidgetInstanceGrid.fromWidgetInstances: before computing nonEmptyElements,
validate and normalize the column count by computing const cols = Math.max(1,
Math.floor(width / elemWidth)) (or fallback to 1 column when width < elemWidth)
and use cols in place of Math.floor(width / elemWidth) when calculating y (and
any other place that depends on columns); this ensures fromWidgetInstances never
produces Infinity row coordinates when height is "auto".
- Around line 712-734: The current bare catch blocks swallow all errors during
resize logic; update both catches to only handle the expected
StackAssertionError and rethrow any other errors: in the outer try/catch and the
inner try/catch, change to catch (err) and if (!(err instanceof
StackAssertionError)) throw err, otherwise proceed to call
clampElementResize(x,y,requestedDelta) and then attempt withResizedElement as
before; reference WidgetInstanceGrid, clampElementResize, withResizedElement and
StackAssertionError so only validation failures are suppressed and real bugs
bubble up.
In `@packages/dashboard-ui-components/src/components/grid-layout/grid.tsx`:
- Around line 47-49: The grid-state-change CustomEvent must be emitted on every
local mutation so the parent iframe receives updates; ensure
dispatchGridStateChange (which calls window.dispatchEvent with serializedGrid:
grid.serialize()) is invoked after any code paths that mutate gridRef —
specifically after reorder, delete, and widget settings update handlers (the
functions that currently modify gridRef but do not call
dispatchGridStateChange); reuse the dispatchGridStateChange helper in those
delete/settings/reorder mutation locations so every local change triggers the
same serialized event.
---
Outside diff comments:
In `@packages/dashboard-ui-components/src/components/card.tsx`:
- Around line 95-105: The provider currently forces nested components to always
read glassmorphic=true; resolve and pass the effective glassmorphic boolean (the
local variable glassmorphic computed from glassmorphicProp) into
DesignCardNestingContext.Provider instead of the hardcoded true so descendants
(e.g., DesignCategoryTabs and any components using useGlassmorphicDefault())
inherit the card's actual glassmorphic setting.
---
Duplicate comments:
In
`@apps/dashboard/src/app/`(main)/(protected)/projects/[projectId]/dashboards/[dashboardId]/page-client.tsx:
- Around line 283-291: The DashboardSandboxHost is being passed the live
gridState instead of the persisted baseline, so change the savedGridState prop
from savedGridState={gridState} to use the persisted state variable
savedGridState (i.e., savedGridState={savedGridState}) so the sandbox compares
edits against the stable persisted baseline; update the prop on the
DashboardSandboxHost invocation and ensure the savedGridState state variable is
the persisted value used elsewhere for saving/loading.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d5dd07ed-f46d-4127-b27b-237eb6071fb5
⛔ Files ignored due to path filters (2)
apps/dashboard/public/dashboard-ui-components.iife.js.mapis excluded by!**/*.mapapps/dashboard/src/generated/bundled-type-definitions.tsis excluded by!**/generated/**
📒 Files selected for processing (12)
apps/backend/src/lib/ai/prompts.tsapps/dashboard/public/dashboard-ui-components.iife.jsapps/dashboard/src/app/(main)/(protected)/projects/[projectId]/dashboards/[dashboardId]/page-client.tsxapps/dashboard/src/components/assistant-ui/thread.tsxapps/dashboard/src/components/commands/create-dashboard/dashboard-sandbox-host.tsxapps/dashboard/src/components/vibe-coding/assistant-chat.tsxpackages/dashboard-ui-components/src/components/card.tsxpackages/dashboard-ui-components/src/components/grid-layout/draggable.tsxpackages/dashboard-ui-components/src/components/grid-layout/grid-logic.tspackages/dashboard-ui-components/src/components/grid-layout/grid.tsxpackages/dashboard-ui-components/src/components/grid-layout/heading-widget.tsxpackages/dashboard-ui-components/src/components/table.tsx
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/backend/src/lib/ai/prompts.ts
- packages/dashboard-ui-components/src/components/grid-layout/draggable.tsx
- apps/dashboard/src/components/vibe-coding/assistant-chat.tsx
- packages/dashboard-ui-components/src/components/table.tsx
- packages/dashboard-ui-components/src/components/grid-layout/heading-widget.tsx
Comment on lines
+37
to
+47
| function extractGridStateFromSource(tsxSource: string): unknown | null { | ||
| const idx = tsxSource.indexOf(GRID_STATE_PREFIX); | ||
| if (idx === -1) return null; | ||
| const lineStart = idx + GRID_STATE_PREFIX.length; | ||
| const lineEnd = tsxSource.indexOf("\n", lineStart); | ||
| const jsonStr = tsxSource.slice(lineStart, lineEnd === -1 ? undefined : lineEnd).trim(); | ||
| try { | ||
| return JSON.parse(jsonStr); | ||
| } catch { | ||
| return null; | ||
| } |
Contributor
There was a problem hiding this comment.
Don't silently drop corrupted grid metadata.
If the persisted // __GRID_STATE__: payload is malformed, Lines 43-46 quietly turn that into null. That hides data corruption, resets the saved-layout baseline, and the next save can overwrite the bad source without surfacing the real problem. Throw or render an explicit error state here instead of treating it as “no grid state”.
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/dashboard/src/app/`(main)/(protected)/projects/[projectId]/dashboards/[dashboardId]/page-client.tsx
around lines 37 - 47, The extractGridStateFromSource function currently swallows
JSON parse errors and returns null; change it to surface malformed payloads by
throwing a descriptive error (including GRID_STATE_PREFIX and an excerpt of the
corrupted json) or returning an explicit error object instead of null so callers
can render an error state; update the try/catch around JSON.parse in
extractGridStateFromSource to include context (tsxSource slice, start index, and
the parse error) and rethrow or return that error payload so corruption is not
silently lost.
Comment on lines
+75
to
+76
| const currentUser = useUser({ or: "redirect" }); | ||
| const backendBaseUrl = getPublicEnvVar("NEXT_PUBLIC_STACK_API_URL") ?? ""; |
Contributor
There was a problem hiding this comment.
Fail fast when the AI backend URL is missing.
Line 76 falls back to "" for NEXT_PUBLIC_STACK_API_URL. That converts a config error into later chat failures against the wrong base URL, which is much harder to diagnose. This should throw immediately or show a visible misconfiguration error.
As per coding guidelines "NEVER silently use fallback values when type errors occur. If a state should never happen due to higher-level logic, either update the types or throw. Prefer ?? throwErr(...) over non-null assertions." and "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/dashboard/src/app/`(main)/(protected)/projects/[projectId]/dashboards/[dashboardId]/page-client.tsx
around lines 75 - 76, The code silently falls back to "" for the AI backend URL;
change the backendBaseUrl initialization (where
getPublicEnvVar("NEXT_PUBLIC_STACK_API_URL") is used alongside currentUser) to
fail fast by throwing a clear error when the env var is missing or empty (e.g.,
replace the nullish fallback with an immediate throw specifying "Missing
NEXT_PUBLIC_STACK_API_URL" or use your project's throwErr/assert helper), so any
misconfiguration surfaces immediately instead of leading to downstream chat
failures.
Comment on lines
+81
to
+108
| const [hasEverExisted, setHasEverExisted] = useState(false); | ||
|
|
||
| const enabledAppIds = useMemo(() => | ||
| typedEntries(config.apps.installed) | ||
| .filter(([appId, appConfig]) => appConfig?.enabled && appId in ALL_APPS) | ||
| .map(([appId]) => appId as AppId), | ||
| [config.apps.installed] | ||
| ); | ||
|
|
||
| const dashboard = config.customDashboards[dashboardId] as | ||
| | typeof config.customDashboards[string] | ||
| | undefined; | ||
|
|
||
| useEffect(() => { | ||
| if (dashboard) { | ||
| setHasEverExisted(true); | ||
| } | ||
| }, [dashboard]); | ||
|
|
||
| useEffect(() => { | ||
| if (hasEverExisted && !dashboard) { | ||
| router.replace(`/projects/${projectId}/dashboards`); | ||
| } | ||
| }, [hasEverExisted, dashboard, router, projectId]); | ||
|
|
||
| if (!dashboard) { | ||
| return null; | ||
| } |
Contributor
There was a problem hiding this comment.
Invalid dashboard IDs can get stuck on a blank page.
The redirect only runs after hasEverExisted becomes true. If someone lands directly on /dashboards/<bad-id>, dashboard stays falsy forever and Lines 106-107 keep returning null with no redirect or error UI. This needs an explicit “config finished loading but dashboard is missing” path.
Based on learnings: "When building frontend code, always carefully deal with loading and error states. Be very explicit with these. Make sure errors are NEVER just silently swallowed."
🤖 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]/dashboards/[dashboardId]/page-client.tsx
around lines 81 - 108, The current logic waits for hasEverExisted before
redirecting, causing a blank page for direct visits to an invalid dashboard; fix
by adding an explicit "config loaded but dashboard missing" path: detect when
config.customDashboards has finished loading (e.g., add or derive an
isConfigLoaded boolean from config or from the presence of
config.apps.installed), and in a useEffect (or extend the existing one that
references hasEverExisted and dashboard) immediately call
router.replace(`/projects/${projectId}/dashboards`) when isConfigLoaded is true
and dashboard is undefined; keep the existing hasEverExisted behavior for
preservation of edit-flow, but ensure the new check prevents silent null returns
by redirecting as soon as config load completes and the dashboard key is
missing.
Comment on lines
+43
to
+53
| // Load local IIFE for dashboard-ui-components (after globals are set) | ||
| const script = document.createElement('script'); | ||
| script.src = '${dashboardUrl}/dashboard-ui-components.iife.js'; | ||
| script.onload = () => { | ||
| window.__depsReady = true; | ||
| window.dispatchEvent(new Event('deps-ready')); | ||
| }; | ||
| script.onerror = (e) => { | ||
| console.error('Failed to load dashboard-ui-components IIFE bundle', e); | ||
| }; | ||
| document.head.appendChild(script); |
Contributor
There was a problem hiding this comment.
Surface dev bundle load failures instead of hanging forever.
If dashboard-ui-components.iife.js fails to load in development, script.onerror only logs and waitForDeps() keeps waiting forever. The iframe then stays blank and never emits stack-ai-dashboard-error. Please turn the error path into a rejected init signal so it flows through the existing error UI.
Also applies to: 318-323
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/dashboard/src/components/commands/create-dashboard/dashboard-sandbox-host.tsx`
around lines 43 - 53, When the dashboard-ui-components IIFE fails to load, turn
the current silent log into a propagated init failure by updating the
script.onerror handler (the one that currently logs "Failed to load
dashboard-ui-components IIFE bundle") to dispatch the existing error signal used
by the UI: create and dispatch a CustomEvent('stack-ai-dashboard-error', {
detail: e }) (and optionally set a flag such as window.__depsReady = false)
instead of only console.error so that waitForDeps() and the error UI receive a
rejected init and show the error; ensure the handler references the same script
loading code that sets window.dispatchEvent(new Event('deps-ready')) so the
error flows through the existing error UI.
Comment on lines
+229
to
+235
| // Controls visibility flag — only true in the full dashboard viewer (not cmd+K preview) | ||
| window.__showControls = ${showControls}; | ||
| window.__chatOpen = false; | ||
| window.__editPanelOpen = false; | ||
| window.__layoutEditing = false; | ||
| window.__selectingForEdit = false; | ||
| window.__savedGridState = ${savedGridState != null ? JSON.stringify(savedGridState) : 'null'}; |
Contributor
There was a problem hiding this comment.
isChatOpen is wired to the wrong sandbox field.
The sandbox initializes window.__chatOpen to false, and later the host still sends chatOpen: false while mapping isChatOpen to editPanelOpen. That means the iframe's chat state never reflects the prop, so any layout or behavior keyed off __chatOpen will stay wrong. If this boolean is actually the edit panel state, the prop name should be updated instead.
Also applies to: 594-600
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/dashboard/src/components/commands/create-dashboard/dashboard-sandbox-host.tsx`
around lines 229 - 235, The sandbox is initializing window.__chatOpen and
window.__editPanelOpen but the host mapping wires isChatOpen to the edit panel
field; fix this by wiring the isChatOpen prop to window.__chatOpen and the edit
panel prop (e.g., isEditPanelOpen or similar) to window.__editPanelOpen in both
the initialization block (window.__chatOpen = ...) and the host-to-iframe
message mapping (where chatOpen / editPanelOpen are sent), or rename the prop to
match the intended field if the boolean actually represents the edit panel;
ensure the same corrected mapping is applied in the other occurrence noted (the
second block around the later initialization).
Comment on lines
+142
to
+161
| public static fromSerialized(widgets: Widget<any, any>[], serialized: Json): WidgetInstanceGrid { | ||
| if (typeof serialized !== "object" || serialized === null) { | ||
| throw new StackAssertionError(`WidgetInstanceGrid serialization is not an object or is null!`, { serialized }); | ||
| } | ||
| if (!("className" in serialized) || typeof serialized.className !== "string" || serialized.className !== "WidgetInstanceGrid") { | ||
| throw new StackAssertionError(`WidgetInstanceGrid serialization is not a WidgetInstanceGrid!`, { serialized }); | ||
| } | ||
|
|
||
| const serializedAny = serialized as any; | ||
| switch (serializedAny.version) { | ||
| case 1: { | ||
| const nonEmptyElements: GridElement[] = serializedAny.nonEmptyElements.map((element: any) => ({ | ||
| instance: element.instance ? deserializeWidgetInstance(widgets, element.instance) : null, | ||
| x: element.x, | ||
| y: element.y, | ||
| width: element.width, | ||
| height: element.height, | ||
| })); | ||
| const varHeights: Map<number, WidgetInstance[]> = new Map(serializedAny.varHeights.map((entry: any) => [entry.y, entry.instances.map((serialized: any) => deserializeWidgetInstance(widgets, serialized))])); | ||
| return new WidgetInstanceGrid(nonEmptyElements, varHeights, serializedAny.width, serializedAny.fixedHeight); |
Contributor
There was a problem hiding this comment.
Validate the versioned payload shape before mapping it.
After checking className, this assumes nonEmptyElements, varHeights, width, and fixedHeight already have the expected types. A malformed saved dashboard will currently fail with generic runtime errors inside .map() or construct nonsense data instead of failing with a targeted assertion.
As per coding guidelines, "Any assumption you make should either be validated through the type system (preferred), assertions, or tests."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/dashboard-ui-components/src/components/grid-layout/grid-logic.ts`
around lines 142 - 161, fromSerialized currently trusts serializedAny fields and
can throw opaque errors; validate the versioned payload shape before mapping by
asserting serializedAny.nonEmptyElements is an array and each element has
expected keys (instance, x, y, width, height), serializedAny.varHeights is an
array of entries with numeric y and instances array, and that
serializedAny.width and serializedAny.fixedHeight have the right types for
version 1; if any check fails, throw a StackAssertionError with the serialized
payload for context. Locate the checks in fromSerialized (WidgetInstanceGrid)
around the switch case for version 1 and perform these defensive assertions
before calling .map() or new WidgetInstanceGrid; then use
deserializeWidgetInstance only after validating the element/instance shapes so
runtime map errors are avoided and failures are explicit.
Comment on lines
+321
to
+341
| public canSwap(x1: number, y1: number, x2: number, y2: number) { | ||
| const elementsToSwap = [this.getElementAt(x1, y1), this.getElementAt(x2, y2)]; | ||
| return elementsToSwap[0].instance !== null || elementsToSwap[1].instance !== null; | ||
| } | ||
|
|
||
| public withSwappedElements(x1: number, y1: number, x2: number, y2: number) { | ||
| if (!this.canSwap(x1, y1, x2, y2)) { | ||
| throw new StackAssertionError(`Cannot swap elements at ${x1}, ${y1} and ${x2}, ${y2}`); | ||
| } | ||
|
|
||
| const elementsToSwap = [this.getElementAt(x1, y1), this.getElementAt(x2, y2)]; | ||
| const newElements = [...this.elements()].map((element) => { | ||
| if (element.x === elementsToSwap[0].x && element.y === elementsToSwap[0].y) { | ||
| return { ...element, instance: elementsToSwap[1].instance }; | ||
| } | ||
| if (element.x === elementsToSwap[1].x && element.y === elementsToSwap[1].y) { | ||
| return { ...element, instance: elementsToSwap[0].instance }; | ||
| } | ||
| return element; | ||
| }); | ||
| return new WidgetInstanceGrid(newElements.filter((element) => element.instance !== null), this._varHeights, this.width, this._fixedHeight); |
Contributor
There was a problem hiding this comment.
canSwap() is not actually checking whether the widgets fit.
This currently returns true whenever at least one slot is occupied. grid.tsx uses that as the “new locations are big enough” gate, so swaps into undersized slots are still allowed even when the destination violates a widget’s min size or calculateMinSize() contract.
Comment on lines
+712
to
+734
| try { | ||
| // Expand the grid height if needed to accommodate pushed elements | ||
| let newFixedHeight = this._fixedHeight; | ||
| if (newFixedHeight !== "auto") { | ||
| for (const el of newElements) { | ||
| if (el.y + el.height > newFixedHeight) { | ||
| newFixedHeight = el.y + el.height; | ||
| } | ||
| } | ||
| } | ||
| const newGrid = new WidgetInstanceGrid(newElements, this._varHeights, this.width, newFixedHeight); | ||
| return { grid: newGrid, achievedDelta, blocked: { top: blockedTop, left: blockedLeft, right: blockedRight, bottom: blockedBottom } }; | ||
| } catch { | ||
| const clamped = this.clampElementResize(x, y, requestedDelta); | ||
| if (clamped.top === 0 && clamped.left === 0 && clamped.bottom === 0 && clamped.right === 0) { | ||
| return { grid: this, achievedDelta: clamped, blocked: { top: blockedTop, left: blockedLeft, right: blockedRight, bottom: blockedBottom } }; | ||
| } | ||
| try { | ||
| return { grid: this.withResizedElement(x, y, clamped), achievedDelta: clamped, blocked: { top: blockedTop, left: blockedLeft, right: blockedRight, bottom: blockedBottom } }; | ||
| } catch { | ||
| return { grid: this, achievedDelta: { top: 0, left: 0, bottom: 0, right: 0 }, blocked: { top: blockedTop, left: blockedLeft, right: blockedRight, bottom: blockedBottom } }; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "grid-logic.ts" | head -20Repository: stack-auth/stack-auth
Length of output: 140
🏁 Script executed:
# Check the grid-logic.ts file structure and locate the lines in question
fd "grid-logic.ts" --exec wc -l {} \;Repository: stack-auth/stack-auth
Length of output: 144
🏁 Script executed:
# Examine the actual file around the problematic lines
fd "grid-logic.ts" --exec sed -n '700,740p' {} \;Repository: stack-auth/stack-auth
Length of output: 1965
🏁 Script executed:
# Search for StackAssertionError definition
rg "StackAssertionError" -t ts -t tsx -B 2 -A 2 | head -50Repository: stack-auth/stack-auth
Length of output: 92
🏁 Script executed:
# Find the withResizedElement method to understand what it does
rg "withResizedElement" -t ts -t tsx -B 5 -A 10 | head -100Repository: stack-auth/stack-auth
Length of output: 92
🏁 Script executed:
# Find clampElementResize method
rg "clampElementResize" -t ts -t tsx -B 3 -A 8 | head -80Repository: stack-auth/stack-auth
Length of output: 92
🏁 Script executed:
# Search for StackAssertionError without file type restriction
rg "StackAssertionError" -B 2 -A 2 | head -60Repository: stack-auth/stack-auth
Length of output: 5801
🏁 Script executed:
# Look at the imports and beginning of grid-logic.ts
head -50 packages/dashboard-ui-components/src/components/grid-layout/grid-logic.tsRepository: stack-auth/stack-auth
Length of output: 1970
🏁 Script executed:
# Search for withResizedElement implementation
rg "withResizedElement" -B 3 -A 12 | head -100Repository: stack-auth/stack-auth
Length of output: 11763
🏁 Script executed:
# Find clampElementResize implementation
rg "clampElementResize" -B 2 -A 8 | head -80Repository: stack-auth/stack-auth
Length of output: 10049
Catch only expected assertion errors; rethrow anything else.
Both bare catch {} blocks swallow all errors, converting any bug in the resize/collision logic into a silent no-op. This violates the no-catch-all rule and the fail-fast principle. The WidgetInstanceGrid constructor and withResizedElement only throw StackAssertionError for expected validation failures, so catch only those and rethrow everything else to surface unexpected errors.
Suggested fix
- } catch {
+ } catch (error) {
+ if (!(error instanceof StackAssertionError)) {
+ throw error;
+ }
const clamped = this.clampElementResize(x, y, requestedDelta);
if (clamped.top === 0 && clamped.left === 0 && clamped.bottom === 0 && clamped.right === 0) {
return { grid: this, achievedDelta: clamped, blocked: { top: blockedTop, left: blockedLeft, right: blockedRight, bottom: blockedBottom } };
}
try {
return { grid: this.withResizedElement(x, y, clamped), achievedDelta: clamped, blocked: { top: blockedTop, left: blockedLeft, right: blockedRight, bottom: blockedBottom } };
- } catch {
+ } catch (error) {
+ if (!(error instanceof StackAssertionError)) {
+ throw error;
+ }
return { grid: this, achievedDelta: { top: 0, left: 0, bottom: 0, right: 0 }, blocked: { top: blockedTop, left: blockedLeft, right: blockedRight, bottom: blockedBottom } };
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| try { | |
| // Expand the grid height if needed to accommodate pushed elements | |
| let newFixedHeight = this._fixedHeight; | |
| if (newFixedHeight !== "auto") { | |
| for (const el of newElements) { | |
| if (el.y + el.height > newFixedHeight) { | |
| newFixedHeight = el.y + el.height; | |
| } | |
| } | |
| } | |
| const newGrid = new WidgetInstanceGrid(newElements, this._varHeights, this.width, newFixedHeight); | |
| return { grid: newGrid, achievedDelta, blocked: { top: blockedTop, left: blockedLeft, right: blockedRight, bottom: blockedBottom } }; | |
| } catch { | |
| const clamped = this.clampElementResize(x, y, requestedDelta); | |
| if (clamped.top === 0 && clamped.left === 0 && clamped.bottom === 0 && clamped.right === 0) { | |
| return { grid: this, achievedDelta: clamped, blocked: { top: blockedTop, left: blockedLeft, right: blockedRight, bottom: blockedBottom } }; | |
| } | |
| try { | |
| return { grid: this.withResizedElement(x, y, clamped), achievedDelta: clamped, blocked: { top: blockedTop, left: blockedLeft, right: blockedRight, bottom: blockedBottom } }; | |
| } catch { | |
| return { grid: this, achievedDelta: { top: 0, left: 0, bottom: 0, right: 0 }, blocked: { top: blockedTop, left: blockedLeft, right: blockedRight, bottom: blockedBottom } }; | |
| } | |
| } | |
| try { | |
| // Expand the grid height if needed to accommodate pushed elements | |
| let newFixedHeight = this._fixedHeight; | |
| if (newFixedHeight !== "auto") { | |
| for (const el of newElements) { | |
| if (el.y + el.height > newFixedHeight) { | |
| newFixedHeight = el.y + el.height; | |
| } | |
| } | |
| } | |
| const newGrid = new WidgetInstanceGrid(newElements, this._varHeights, this.width, newFixedHeight); | |
| return { grid: newGrid, achievedDelta, blocked: { top: blockedTop, left: blockedLeft, right: blockedRight, bottom: blockedBottom } }; | |
| } catch (error) { | |
| if (!(error instanceof StackAssertionError)) { | |
| throw error; | |
| } | |
| const clamped = this.clampElementResize(x, y, requestedDelta); | |
| if (clamped.top === 0 && clamped.left === 0 && clamped.bottom === 0 && clamped.right === 0) { | |
| return { grid: this, achievedDelta: clamped, blocked: { top: blockedTop, left: blockedLeft, right: blockedRight, bottom: blockedBottom } }; | |
| } | |
| try { | |
| return { grid: this.withResizedElement(x, y, clamped), achievedDelta: clamped, blocked: { top: blockedTop, left: blockedLeft, right: blockedRight, bottom: blockedBottom } }; | |
| } catch (error) { | |
| if (!(error instanceof StackAssertionError)) { | |
| throw error; | |
| } | |
| return { grid: this, achievedDelta: { top: 0, left: 0, bottom: 0, right: 0 }, blocked: { top: blockedTop, left: blockedLeft, right: blockedRight, bottom: blockedBottom } }; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/dashboard-ui-components/src/components/grid-layout/grid-logic.ts`
around lines 712 - 734, The current bare catch blocks swallow all errors during
resize logic; update both catches to only handle the expected
StackAssertionError and rethrow any other errors: in the outer try/catch and the
inner try/catch, change to catch (err) and if (!(err instanceof
StackAssertionError)) throw err, otherwise proceed to call
clampElementResize(x,y,requestedDelta) and then attempt withResizedElement as
before; reference WidgetInstanceGrid, clampElementResize, withResizedElement and
StackAssertionError so only validation failures are suppressed and real bugs
bubble up.
Comment on lines
+816
to
+821
| for (const element of this.elements()) { | ||
| if (element.y < y && element.y + element.height > y) { | ||
| result = false; | ||
| break; | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Ignore synthetic empty slots when deciding whether a var-height section can be inserted.
this.elements() includes the placeholder rectangles produced by _withEmptyElements(). Those empty rectangles can span multiple rows, so this returns false even when only blank space crosses y. Since grid.tsx gates slot rendering on canAddVarHeight(), valid insertion points disappear in sparse layouts.
Suggested fix
- for (const element of this.elements()) {
+ for (const element of this._nonEmptyElements) {
if (element.y < y && element.y + element.height > y) {
result = false;
break;
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/dashboard-ui-components/src/components/grid-layout/grid-logic.ts`
around lines 816 - 821, The current loop over this.elements() treats synthetic
placeholder rectangles (created by _withEmptyElements()) as real elements,
causing canAddVarHeight() to return false incorrectly; update the loop in the
function that checks insertion at y (the block iterating "for (const element of
this.elements())") to skip those synthetic empty elements by detecting the
placeholder flag added by _withEmptyElements (e.g., element.isEmpty or
element.isPlaceholder or similar) — either filter this.elements() first or add a
continue when a placeholder is detected so only real elements contribute to the
overlap check used by canAddVarHeight().
Comment on lines
+47
to
+49
| const dispatchGridStateChange = useCallback((grid: WidgetInstanceGrid) => { | ||
| window.dispatchEvent(new CustomEvent('grid-state-change', { detail: { serializedGrid: grid.serialize() } })); | ||
| }, []); |
Contributor
There was a problem hiding this comment.
Emit grid-state-change on every local grid mutation.
apps/dashboard/src/components/commands/create-dashboard/dashboard-sandbox-host.tsx:299-311 only forwards grid updates when this event fires. Reordering var-height widgets, deleting widgets, and updating widget settings here mutate gridRef without dispatching the event, so the parent iframe can persist stale dashboard state after those edits.
Suggested consolidation
+ const commitGrid = useCallback((nextGrid: WidgetInstanceGrid) => {
+ props.gridRef.set(nextGrid);
+ dispatchGridStateChange(nextGrid);
+ }, [props.gridRef, dispatchGridStateChange]);
+
const dispatchGridStateChange = useCallback((grid: WidgetInstanceGrid) => {
window.dispatchEvent(new CustomEvent('grid-state-change', { detail: { serializedGrid: grid.serialize() } }));
}, []);- props.gridRef.set(props.gridRef.current.withMovedVarHeightToInstance(activeInstanceId as string, overLocation[1], overLocation[0]));
+ commitGrid(props.gridRef.current.withMovedVarHeightToInstance(activeInstanceId as string, overLocation[1], overLocation[0]));Apply the same helper to the delete/settings paths as well.
Also applies to: 154-166, 215-221, 412-418
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/dashboard-ui-components/src/components/grid-layout/grid.tsx` around
lines 47 - 49, The grid-state-change CustomEvent must be emitted on every local
mutation so the parent iframe receives updates; ensure dispatchGridStateChange
(which calls window.dispatchEvent with serializedGrid: grid.serialize()) is
invoked after any code paths that mutate gridRef — specifically after reorder,
delete, and widget settings update handlers (the functions that currently modify
gridRef but do not call dispatchGridStateChange); reuse the
dispatchGridStateChange helper in those delete/settings/reorder mutation
locations so every local change triggers the same serialized event.
| } else { | ||
| const controller = new AbortController(); | ||
| const timeoutId = setTimeout(() => controller.abort(), 120_000); | ||
| const result = await generateText({ |
There was a problem hiding this comment.
AbortController timeout is created but its signal is never passed to generateText, making the 120-second timeout completely ineffective.
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.
Custom Dashboards and unified ai
Summary by CodeRabbit