dev tool indicator#1272
Merged
mantrakp04 merged 29 commits intodevfrom Apr 14, 2026
Merged
Conversation
- Updated package versions for '@supabase/*' libraries to 2.99.2 and '@supabase/ssr' to 0.9.0. - Added new devDependencies for 'rimraf' and 'framer-motion' in the pnpm-lock file. - Modified Next.js configuration to conditionally omit 'X-Frame-Options' in development mode for better integration with Stack Auth dev tools. - Refactored component exports in the template package to include tracking for dev tools. - Introduced new dev tool components and context for improved logging and state management. - Added styles for the dev tool indicator and panel, ensuring a consistent dark theme. - Implemented fetch interception to log API calls and user authentication events in the dev tool.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces a comprehensive developer tool feature for Stack Auth with AI chat capabilities, real-time API monitoring, component versioning, and configuration debugging. It adds AI SDK dependencies, implements a complete dev tool UI with multiple tabs, updates backend APIs for component metadata and feedback handling with production forwarding, enhances CSP headers for local development, and modifies the feedback pipeline. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
N2D4
reviewed
Mar 21, 2026
N2D4
requested changes
Mar 21, 2026
Contributor
N2D4
left a comment
N2D4
left a comment
There was a problem hiding this comment.
i like the indicator, some comments on how to implement it
- Updated package versions for '@supabase/*' libraries to 2.100.0 and added new dependencies for '@ai-sdk/react' and 'ai'. - Refactored API route to forward request body correctly in the backend. - Improved team switcher component to handle null team cases in URL mapping. - Introduced new AI tab in the dev tool for enhanced user interaction and logging. - Updated dev tool styles and components for better theme integration and user experience. - Added support for new features in the dev tool, including export functionality and improved state management.
Collaborator
Author
|
@greptile-ai review |
Contributor
Greptile SummaryThis PR introduces a localhost-only developer tools panel (a draggable trigger + tabbed panel built in vanilla DOM) that surfaces an overview, component version inspector, AI chat, console log, dashboard iframe, and support feedback form. It also makes the internal feedback endpoint accept unauthenticated submissions and adds
Confidence Score: 4/5Safe to merge with one resource-leak fix; all remaining concerns are minor or already flagged in prior review rounds. A single P1 remains: the window.resize listener in createTrigger accumulates across enable/disable cycles. Everything else — userPoll cleanup, console-tab unsub, panel resize handles — is correctly wired. Prior round concerns about circular-reference sanitization, header case-sensitivity, and the unauthenticated feedback endpoint have all been addressed or were intentional design choices. packages/template/src/dev-tool/dev-tool-core.ts — specifically the createTrigger function around line 432. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["mountDevTool(app)\n(client-app-impl.ts)"] --> B{isLocalhost?}
B -- No --> C[No-op]
B -- Yes --> D["createDevTool(app)\n(dev-tool-core.ts)"]
D --> E["createTrigger(togglePanel)\nregisters window.resize ⚠️ never removed"]
D --> F["addRequestListener\n(client-interface.ts)"]
D --> G["createStateStore\n(localStorage)"]
F --> H["requestListeners.forEach()\non every SDK fetch"]
H --> I["logStore.addApiLog()"]
I --> J[Console tab UI update]
D --> K["openPanel()\ncreatePanel(app, state, logStore)"]
K --> L["Tabs: overview · components · ai · console · docs · dashboard · support"]
L --> M["createOverviewTab\nsetInterval userPoll 3s ✅ cleanup registered"]
L --> N["createConsoleTab\nlogStore.subscribe ✅ unsub registered"]
L --> O["createAITab\nfetch /api/latest/ai/query/stream with Stack headers"]
O --> P["Local emulator backend\n/integrations/ai-proxy"]
P --> Q{STACK_OPENROUTER_API_KEY}
Q -- "FORWARD_TO_PRODUCTION" --> R["https://api.stack-auth.com\n/integrations/ai-proxy"]
Q -- real key --> S[OpenRouter API]
L --> T["createSupportTab\nPOST /api/latest/internal/feedback (no auth)"]
T --> U{STACK_FEEDBACK_MODE}
U -- "FORWARD_TO_PRODUCTION" --> V["https://api.stack-auth.com\n/internal/feedback"]
U -- email --> W[sendSupportFeedbackEmail]
Prompt To Fix All With AIThis is a comment left during a code review.
Path: packages/template/src/dev-tool/dev-tool-core.ts
Line: 432-440
Comment:
**`window.resize` listener leaks on every remount**
`createTrigger` attaches a `window.addEventListener('resize', …)` but never returns a teardown, and `createDevTool`'s cleanup function only removes the request listener, calls `panel?.cleanup()`, and removes the root DOM node. On every `StackDevTool.enable()`/`disable()`/`enable()` cycle (or HMR remount), a new listener is registered while the previous one stays alive — each stale listener still writes to `localStorage` on every resize event.
The fix is to capture the handler and return it from `createTrigger` as a cleanup, then call it from `createDevTool`:
```ts
// in createTrigger, before the return:
const onResize = () => { … };
window.addEventListener('resize', onResize);
// change return to include cleanup
return { el: btn, cleanup: () => window.removeEventListener('resize', onResize) };
```
Then in `createDevTool`:
```ts
return () => {
removeRequestListener();
trigger.cleanup(); // <-- add this
panel?.cleanup();
if (root.parentNode) root.parentNode.removeChild(root);
};
```
How can I resolve this? If you propose a fix, please make it concise.Reviews (5): Last reviewed commit: "Refactor Panel Closing Logic in Dev Tool..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a Stack Auth “dev tool indicator” overlay (panel + tabs) to the template/SDK packages to improve local development workflows, including embedded dashboard/docs, component previews, logging/export, support submission, and an AI helper. It also updates dependencies and adjusts the dashboard’s dev headers to support iframe embedding.
Changes:
- Introduces a new dev-tool UI (indicator + panel + tabs), wiring it into
StackProviderClientand adding styling/context/logging. - Adds AI SDK dependencies (
@ai-sdk/react,ai) acrosspackages/template,packages/react, andpackages/stack. - Adjusts dashboard security headers in development to allow iframe embedding (omits
X-Frame-Optionsin dev).
Reviewed changes
Copilot reviewed 30 out of 32 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Updates lockfile to reflect new deps (AI SDK, rolldown optional dep, etc.). |
| packages/template/src/react-dom.d.ts | Adds minimal react-dom typing for createPortal. |
| packages/template/src/providers/stack-provider-client.tsx | Renders DevToolEntry under the same translation context as Stack UI. |
| packages/template/src/index.ts | Simplifies/reformats exports for some components. |
| packages/template/src/dev-tool/tabs/support-tab.tsx | Adds Support tab (feedback form + feature requests iframe). |
| packages/template/src/dev-tool/tabs/overview-tab.tsx | Adds Overview tab (identity/actions/config/changelog). |
| packages/template/src/dev-tool/tabs/docs-tab.tsx | Adds Docs iframe tab. |
| packages/template/src/dev-tool/tabs/dashboard-tab.tsx | Adds Dashboard iframe tab with localhost-only embedding guard. |
| packages/template/src/dev-tool/tabs/console-tab.tsx | Adds Console tab (API/auth logs, export/share dialog, config view). |
| packages/template/src/dev-tool/tabs/components-tab.tsx | Adds Components tab (catalog + live previews + page iframes). |
| packages/template/src/dev-tool/tabs/ai-tab.tsx | Adds AI tab (chat UI + lightweight markdown renderer). |
| packages/template/src/dev-tool/index.tsx | Adds dev tool entrypoint, visibility logic, and global override commands. |
| packages/template/src/dev-tool/iframe-tab.tsx | Shared iframe wrapper with loading/error UX and sandboxing. |
| packages/template/src/dev-tool/hooks/use-dev-tool-state.tsx | Convenience re-export for useDevToolContext. |
| packages/template/src/dev-tool/hooks/use-component-registry.tsx | Adds preview context for component registry behavior. |
| packages/template/src/dev-tool/dev-tool-trigger.tsx | Adds floating “DEV” trigger pill. |
| packages/template/src/dev-tool/dev-tool-tab-bar.tsx | Adds reusable animated tab bar (bar + pill variants). |
| packages/template/src/dev-tool/dev-tool-styles.ts | Adds full CSS theme for dev tool indicator/panel/tabs. |
| packages/template/src/dev-tool/dev-tool-panel.tsx | Adds panel container (tabs, resizing, iframe reload, export dialog overlay). |
| packages/template/src/dev-tool/dev-tool-indicator.tsx | Adds portal-mounted indicator, fetch interception, auth event tracking, keyboard shortcut. |
| packages/template/src/dev-tool/dev-tool-context.tsx | Adds dev tool context + persisted state + global log store + URL helpers. |
| packages/template/src/dev-tool/component-catalog.tsx | Defines the built-in component catalog used by the Components tab. |
| packages/template/src/components-page/sign-in.tsx | Marks SignIn page as a client component. |
| packages/template/package.json | Adds AI SDK dependencies to the template package. |
| packages/template/package-template.json | Adds AI deps and adjusts peerDependencies macros for generated packages. |
| packages/stack/package.json | Adds AI SDK dependencies to @stackframe/stack. |
| packages/react/package.json | Adds AI SDK deps + adds react-dom to peer deps (and optional typing meta). |
| examples/docs-examples/src/app/team/[teamId]/page.tsx | Updates SelectedTeamSwitcher urlMap typing/behavior. |
| docs/src/components/stack-auth/stack-team-switcher.tsx | Adjusts docs example to handle `team |
| claude/CLAUDE-KNOWLEDGE.md | Adds internal notes about dev tool-related pitfalls/architecture. |
| apps/dashboard/next.config.mjs | Omits X-Frame-Options in dev to allow iframe embedding. |
| apps/backend/src/app/api/latest/ai/query/[mode]/route.ts | Strips projectId when forwarding AI requests 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.
![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: 9
🧹 Nitpick comments (10)
packages/template/src/dev-tool/iframe-tab.tsx (1)
73-80:onErroris unreliable for detecting iframe load failures.Many iframe loading failures (CSP blocks, X-Frame-Options denials, network issues) don't fire
onError. The iframe may display blank content without triggering the error state. Consider adding a timeout fallback or documenting this limitation.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/template/src/dev-tool/iframe-tab.tsx` around lines 73 - 80, The iframe's onError is unreliable; add a timeout fallback that starts whenever src changes (store an iframe ref like iframeRef) and clears on handleLoad and handleError; if the timeout elapses (e.g., 5–10s) check the iframeRef.current (try accessing contentDocument/contentWindow safely) and if it's empty or cross-origin-inaccessible treat it as a load failure and set the same error state used by handleError/loading, and also document this limitation in comments next to the iframe, referencing the iframe element, handleLoad, handleError, loading, src and title so future readers know why the timer exists.examples/docs-examples/src/app/team/[teamId]/page.tsx (1)
16-21: UseencodeURIComponent()for URL path segment.The defensive null check is appropriate given the underlying type-safety gap in
TeamSwitcher. However, the URL construction should use proper encoding for consistency with coding guidelines.♻️ Suggested fix
urlMap={(t) => { if (t == null) { throw new Error("SelectedTeamSwitcher urlMap expected a non-null team"); } - return `/team/${t.id}`; + return `/team/${encodeURIComponent(t.id)}`; }}As per coding guidelines: "Use
urlString`` orencodeURIComponent()` instead of normal string interpolation for URLs for consistency."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/docs-examples/src/app/team/`[teamId]/page.tsx around lines 16 - 21, The urlMap function constructs a raw path using string interpolation; update url creation in the urlMap arrow function to encode the team id (use encodeURIComponent(t.id) or the project's urlString helper) while keeping the existing null-check that throws if t is null—change the return for urlMap to produce an encoded path like /team/<encoded-id> so ids with unsafe characters are properly escaped.docs/src/components/stack-auth/stack-team-switcher.tsx (1)
235-235: Code example doesn't match actual implementation.The
generateCodeExample()at line 125 shows(team: { id: string }) => ...without null handling, but the actual implementation here accepts{ id: string } | null. Users copying the code example won't get the defensive null check.Additionally, consider using
encodeURIComponent(team.id)for URL consistency.♻️ Suggested fixes
- Update the code example generator (around line 125) to match:
if (props.urlMap) { - propsArray.push('urlMap={(team: { id: string }) => `/teams/${team.id}/dashboard`}'); + propsArray.push('urlMap={(team: { id: string } | null) => team ? `/teams/${team.id}/dashboard` : `/`}'); }
- Encode the URL path:
- urlMap={props.urlMap ? (team: { id: string } | null) => team ? `/teams/${team.id}/dashboard` : '/' : undefined} + urlMap={props.urlMap ? (team: { id: string } | null) => team ? `/teams/${encodeURIComponent(team.id)}/dashboard` : '/' : undefined}As per coding guidelines: "Use
urlString`` orencodeURIComponent()` instead of normal string interpolation for URLs."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/src/components/stack-auth/stack-team-switcher.tsx` at line 235, The code example produced by generateCodeExample() must match the actual urlMap prop signature used in stack-team-switcher.tsx: update generateCodeExample() to show the parameter as (team: { id: string } | null) => team ? `/teams/${encodeURIComponent(team.id)}/dashboard` : '/' (or equivalent using urlString/encodeURIComponent) so the example includes the null defensive check and encodes team.id; search for generateCodeExample and the urlMap prop to apply the change and replace plain string interpolation of team.id with encodeURIComponent(team.id) (or the project urlString helper) for URL safety.packages/template/src/dev-tool/dev-tool-indicator.tsx (1)
65-70: Use a monotonic clock forduration.These timings are currently derived from
Date.now(), so a system clock adjustment can make the dev-tool latency jump or even go negative. Keep the wall-clocktimestamp, but measure elapsed time withperformance.now().As per coding guidelines, don't use Date.now() for measuring elapsed (real) time, instead use performance.now().
Also applies to: 111-117
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/template/src/dev-tool/dev-tool-indicator.tsx` around lines 65 - 70, The code currently uses Date.now() (startTime) to compute request duration in the fetch wrapper (around the fetch call that uses originalFetch), which is susceptible to system clock changes; change it to record a monotonic start via performance.now() (e.g., startMonotonic = performance.now()) and compute duration as performance.now() - startMonotonic while keeping the wall-clock timestamp from Date.now() if you still need a timestamp; update the other analogous occurrence that computes elapsed time elsewhere in this file (the second startTime/duration pair) the same way to use performance.now() for elapsed measurements.packages/template/src/dev-tool/component-catalog.tsx (1)
31-33: Explain theanyescape hatch here or narrow it.
CatalogEntryis exported, soReact.ComponentType<any>becomes the registry contract for every previewed component. If heterogeneous props makeanyunavoidable, please add the required inline note about why it's safe and where prop misuse is still caught.As per coding guidelines, try to avoid the
anytype; whenever you need it, leave a comment explaining why you're using it and how you can be certain that errors would still be flagged.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/template/src/dev-tool/component-catalog.tsx` around lines 31 - 33, The exported type CatalogEntry currently uses React.ComponentType<any>, which leaks an any into the public registry contract; either make CatalogEntry generic (e.g., CatalogEntry<P = unknown> with component: React.ComponentType<P>) or switch to React.ComponentType<unknown> to avoid any, and update callers to provide the concrete prop type, or if heterogeneous props truly make any unavoidable add a concise inline comment on the CatalogEntry.type explaining why any is required, where runtime/compile-time prop validation still occurs (e.g., preview wrappers or story files), and add a TODO to tighten the type when components share a common prop shape; refer to the exported symbol CatalogEntry and its component field when making the change.packages/template/src/dev-tool/tabs/overview-tab.tsx (2)
131-131: Prefer explicit null check over non-null assertion.Per coding guidelines, prefer
?? throwErr(...)over non-null assertions. Althoughuseris checked in the JSX conditional, the async function scope doesn't carry that narrowing.Suggested fix
- await user!.signOut(); + if (user == null) { + throw new Error('Cannot sign out: no user is signed in'); + } + await user.signOut();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/template/src/dev-tool/tabs/overview-tab.tsx` at line 131, The non-null assertion on user in the async call await user!.signOut() should be replaced with an explicit null-check that throws if user is missing; update the sign-out call in the same scope (the function that calls user.signOut) to use the null-coalescing pattern (user ?? throwErr("user is undefined when attempting signOut")) before invoking signOut, or otherwise guard with an if and throw so the function no longer relies on the non-null assertion operator.
89-91: Avoidcatch (e: any)pattern.The coding guidelines prohibit using
any. For error handling, consider using a type guard or utility to safely extract the error message.Suggested approach
- } catch (e: any) { - setStatus({ type: 'error', message: e.message || 'Unknown error' }); + } catch (e) { + setStatus({ type: 'error', message: e instanceof Error ? e.message : 'Unknown error' }); }Apply the same pattern to the other catch blocks at lines 121-123 and 133-135.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/template/src/dev-tool/tabs/overview-tab.tsx` around lines 89 - 91, Replace the unsafe "catch (e: any)" usages with "catch (err: unknown)" and use a small type-guard/helper to extract a safe message before calling setStatus; for example, add a getErrorMessage(err: unknown): string utility that checks for err being an Error or having a message property and returns a string, then change the three catch blocks that call setStatus to use getErrorMessage(err) when creating the status ({ type: 'error', message: getErrorMessage(err) }); update all occurrences (the catch currently using setStatus at the top and the two other similar catch blocks) to use this helper.packages/template/src/dev-tool/dev-tool-context.tsx (2)
130-132: Document theanycast on globalThis.The coding guidelines require comments explaining
anyusage. This global attachment is for cross-module interop with the fetch interceptor.Suggested fix
// Expose globally so the fetch interceptor (which may be installed once) can // always reach the latest store even after HMR / remounts. if (typeof globalThis !== 'undefined') { + // Using `any` because globalThis doesn't have a typed slot for our store. + // This is safe as we control both the writer (here) and readers (dev-tool-indicator.tsx). (globalThis as any).__STACK_DEV_TOOL_LOG_STORE__ = globalLogStore; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/template/src/dev-tool/dev-tool-context.tsx` around lines 130 - 132, Add a short inline comment explaining the use of "any" on globalThis when assigning __STACK_DEV_TOOL_LOG_STORE__ to globalLogStore: state that the cast is intentional to attach a cross-module global used for the fetch interceptor/cross-module interop, and that TypeScript typing is intentionally bypassed here because globalThis has no typed property for this runtime-only debugging store; keep the comment adjacent to the cast that sets (globalThis as any).__STACK_DEV_TOOL_LOG_STORE__ = globalLogStore and mention the related symbol names (__STACK_DEV_TOOL_LOG_STORE__, globalLogStore, fetch interceptor) so future readers know why the any is necessary.
72-74: Add comment explaining why errors are silently ignored.Per coding guidelines, errors shouldn't be silently swallowed. For localStorage operations, silent failure is often acceptable (quota exceeded, private browsing, etc.), but the reasoning should be documented.
Suggested fix
} catch { - // ignore + // localStorage may be unavailable (private browsing, quota exceeded, etc.) + // Failing silently is acceptable here as state persistence is non-critical }Apply the same to the catch block at lines 83-85.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/template/src/dev-tool/dev-tool-context.tsx` around lines 72 - 74, The empty catch blocks in dev-tool-context (around the localStorage access in the code surrounding the anonymous catch { // ignore }) silently swallow errors; update both catch blocks (the one shown and the similar one later) to include a brief comment explaining why errors are intentionally ignored (e.g., localStorage may be unavailable in private mode or exceed quota, so failures are non-fatal for dev UX) and keep no-op behavior; ensure the comment references the localStorage/read or write operation performed (so future maintainers know the rationale) and leave behavior unchanged.packages/template/src/dev-tool/tabs/console-tab.tsx (1)
161-162: Avoidanyin map callback.The
anytype in(p: any) => p.idcan be replaced with a more specific type or type assertion that documents the expected shape.Suggested fix
if (key === 'oauthProviders' && Array.isArray(value)) { - lines.push(`${key}: ${value.map((p: any) => p.id).join(', ') || 'None'}`); + lines.push(`${key}: ${value.map((p: { id: string }) => p.id).join(', ') || 'None'}`); } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/template/src/dev-tool/tabs/console-tab.tsx` around lines 161 - 162, The map callback uses an unsafe any: replace (p: any) => p.id with a typed shape or assertion for the expected provider object (e.g., define or use an OAuthProvider type/interface with an id property and cast value to OAuthProvider[] before mapping, or change the callback to (p: OAuthProvider) => p.id) so the compiler knows the structure when mapping in the oauthProviders branch (the check key === 'oauthProviders' and the value.map call should use the new typed shape).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/template/package-template.json`:
- Around line 127-136: The peerDependencies JSON contains duplicated commas
because the marker entries ("//") and the following real dependency entries both
include commas; fix package-template.json by ensuring only one comma separates
items—remove the trailing comma from marker lines or the leading comma from the
subsequent dependency lines so each JSON property is properly comma-separated;
specifically update the peerDependencies block around the symbols
"@types/react", "//" marker entries, "@types/react-dom", "react-dom", "next",
and "react" so marker lines do not introduce extra leading/trailing commas and
the resulting JSON parses correctly.
In `@packages/template/src/dev-tool/dev-tool-indicator.tsx`:
- Around line 35-46: The fetch interceptor currently only reads init?.headers
and init?.method, causing fetch(new Request(...)) to lose the Request's headers
and method, and it measures durations with Date.now(); fix by first normalizing
the Request input: if input is a Request object, extract its headers and method
into local variables (merge into the existing headers and method variables),
then apply init overrides so init headers/method take precedence; use the
existing symbols (input, init, headers, method, isStackCall) to implement this
merge logic. Also replace all duration uses of Date.now() (start/end/duration
calculations) with performance.now() so elapsed time is measured with a
monotonic clock.
In `@packages/template/src/dev-tool/dev-tool-tab-bar.tsx`:
- Around line 25-42: The active-indicator measurement only runs on activeTab
changes; update the useEffect that reads barRef/current and sets setStyle so it
also installs a ResizeObserver on barRef.current to call the same measurement
logic whenever the tab bar resizes (recompute btn via
bar.querySelector(`[data-tab-id="${activeTab}"]`) and call setStyle with
offsetLeft/offsetWidth/offsetHeight/transition as before), ensure you
store/close the observer on cleanup, and keep the existing initialRef.current
handling so the initial transition behavior is preserved.
In `@packages/template/src/dev-tool/hooks/use-component-registry.tsx`:
- Around line 7-16: The DevToolComponentPreviewContext is created and a provider
(DevToolComponentPreviewProvider) is exported but there is no consumer hook; add
and export a hook (e.g., useDevToolComponentPreview) that calls
React.useContext(DevToolComponentPreviewContext) and returns the boolean value
so components can detect preview mode; ensure the hook handles missing provider
gracefully (return false or throw with a clear message) and reference
DevToolComponentPreviewContext and DevToolComponentPreviewProvider so callers
can locate the provider/hook pair.
In `@packages/template/src/dev-tool/iframe-tab.tsx`:
- Around line 37-40: The retry handler (retry useCallback) only toggles state
(setLoading/setError) and does not force the iframe to remount, so the iframe
src remains unchanged; modify the retry flow to force a remount by updating a
unique key (e.g., iframeKey state) or appending a cache-busting query param to
the src used by the iframe component (the element that renders the iframe) and
ensure retry increments/changes that key or param so React recreates the iframe;
keep setLoading/setError behavior and update the code paths in retry and the
iframe render to use the new iframeKey/src-with-param.
In `@packages/template/src/dev-tool/index.tsx`:
- Around line 19-25: The getOverride() path currently swallows errors and the
write paths (enable(), disable(), reset()) and global exposure of StackDevTool
are unsafe; update getOverride() to catch and return null only after logging or
wrapping the storage error (or better: implement a small typed storage helper
used by getOverride(), enable(), disable(), reset() that returns Result/throws
explicit errors), add try/catch around localStorage.setItem/removeItem with
clear error handling (e.g., log and fall back to in-memory state), and remove
the use of (window as any) by declaring StackDevTool on the global Window
interface (declare global { interface Window { StackDevTool: typeof StackDevTool
} }) so the tool is exposed type-safely; reference OVERRIDE_KEY, getOverride,
enable, disable, reset, and StackDevTool when making these changes.
In `@packages/template/src/dev-tool/tabs/ai-tab.tsx`:
- Around line 69-73: The onClick handler currently swallows errors from
navigator.clipboard.writeText which hides failures; update the promise catch to
surface feedback to the user instead of an empty catch: capture the error from
navigator.clipboard.writeText, log it (or send to a logger), and update UI state
(e.g., setCopied(false) and set a new setCopyError/setCopyFailed state or call
your existing toast/notification helper) so the component (ai-tab) can display a
friendly "copy failed" message; ensure you still clear any transient success
state (setCopied) and avoid unhandled promise rejections.
- Around line 225-226: You're using an unsafe cast (app as any) to access the
internal symbol; remove the cast and ensure the variable `app` is correctly
typed (e.g., as StackClientApp or a narrowed type that includes the
[stackAppInternalsSymbol] property) so you can call
`[stackAppInternalsSymbol]?.getConstructorOptions?.()` directly inside the
`useEffect`; alternatively add a small type guard that verifies `app && (app as
unknown)[stackAppInternalsSymbol]` then call `getConstructorOptions` — reference
`useEffect`, `app`, `stackAppInternalsSymbol`, and `getConstructorOptions` when
making the change.
In `@packages/template/src/dev-tool/tabs/support-tab.tsx`:
- Around line 34-44: The current useEffect early-exits on prefillApplied.current
which is never reset, so subsequent prefill props are ignored; change the effect
in support-tab.tsx (the useEffect that references prefillApplied,
setFeedbackType, setMessage, setStatus) to apply whenever the prefill prop
changes by removing or abandoning the prefillApplied flag and simply running the
setFeedbackType/setMessage/setStatus when prefill is truthy; ensure the effect
dependency remains [prefill] so each new prefill payload is applied.
---
Nitpick comments:
In `@docs/src/components/stack-auth/stack-team-switcher.tsx`:
- Line 235: The code example produced by generateCodeExample() must match the
actual urlMap prop signature used in stack-team-switcher.tsx: update
generateCodeExample() to show the parameter as (team: { id: string } | null) =>
team ? `/teams/${encodeURIComponent(team.id)}/dashboard` : '/' (or equivalent
using urlString/encodeURIComponent) so the example includes the null defensive
check and encodes team.id; search for generateCodeExample and the urlMap prop to
apply the change and replace plain string interpolation of team.id with
encodeURIComponent(team.id) (or the project urlString helper) for URL safety.
In `@examples/docs-examples/src/app/team/`[teamId]/page.tsx:
- Around line 16-21: The urlMap function constructs a raw path using string
interpolation; update url creation in the urlMap arrow function to encode the
team id (use encodeURIComponent(t.id) or the project's urlString helper) while
keeping the existing null-check that throws if t is null—change the return for
urlMap to produce an encoded path like /team/<encoded-id> so ids with unsafe
characters are properly escaped.
In `@packages/template/src/dev-tool/component-catalog.tsx`:
- Around line 31-33: The exported type CatalogEntry currently uses
React.ComponentType<any>, which leaks an any into the public registry contract;
either make CatalogEntry generic (e.g., CatalogEntry<P = unknown> with
component: React.ComponentType<P>) or switch to React.ComponentType<unknown> to
avoid any, and update callers to provide the concrete prop type, or if
heterogeneous props truly make any unavoidable add a concise inline comment on
the CatalogEntry.type explaining why any is required, where runtime/compile-time
prop validation still occurs (e.g., preview wrappers or story files), and add a
TODO to tighten the type when components share a common prop shape; refer to the
exported symbol CatalogEntry and its component field when making the change.
In `@packages/template/src/dev-tool/dev-tool-context.tsx`:
- Around line 130-132: Add a short inline comment explaining the use of "any" on
globalThis when assigning __STACK_DEV_TOOL_LOG_STORE__ to globalLogStore: state
that the cast is intentional to attach a cross-module global used for the fetch
interceptor/cross-module interop, and that TypeScript typing is intentionally
bypassed here because globalThis has no typed property for this runtime-only
debugging store; keep the comment adjacent to the cast that sets (globalThis as
any).__STACK_DEV_TOOL_LOG_STORE__ = globalLogStore and mention the related
symbol names (__STACK_DEV_TOOL_LOG_STORE__, globalLogStore, fetch interceptor)
so future readers know why the any is necessary.
- Around line 72-74: The empty catch blocks in dev-tool-context (around the
localStorage access in the code surrounding the anonymous catch { // ignore })
silently swallow errors; update both catch blocks (the one shown and the similar
one later) to include a brief comment explaining why errors are intentionally
ignored (e.g., localStorage may be unavailable in private mode or exceed quota,
so failures are non-fatal for dev UX) and keep no-op behavior; ensure the
comment references the localStorage/read or write operation performed (so future
maintainers know the rationale) and leave behavior unchanged.
In `@packages/template/src/dev-tool/dev-tool-indicator.tsx`:
- Around line 65-70: The code currently uses Date.now() (startTime) to compute
request duration in the fetch wrapper (around the fetch call that uses
originalFetch), which is susceptible to system clock changes; change it to
record a monotonic start via performance.now() (e.g., startMonotonic =
performance.now()) and compute duration as performance.now() - startMonotonic
while keeping the wall-clock timestamp from Date.now() if you still need a
timestamp; update the other analogous occurrence that computes elapsed time
elsewhere in this file (the second startTime/duration pair) the same way to use
performance.now() for elapsed measurements.
In `@packages/template/src/dev-tool/iframe-tab.tsx`:
- Around line 73-80: The iframe's onError is unreliable; add a timeout fallback
that starts whenever src changes (store an iframe ref like iframeRef) and clears
on handleLoad and handleError; if the timeout elapses (e.g., 5–10s) check the
iframeRef.current (try accessing contentDocument/contentWindow safely) and if
it's empty or cross-origin-inaccessible treat it as a load failure and set the
same error state used by handleError/loading, and also document this limitation
in comments next to the iframe, referencing the iframe element, handleLoad,
handleError, loading, src and title so future readers know why the timer exists.
In `@packages/template/src/dev-tool/tabs/console-tab.tsx`:
- Around line 161-162: The map callback uses an unsafe any: replace (p: any) =>
p.id with a typed shape or assertion for the expected provider object (e.g.,
define or use an OAuthProvider type/interface with an id property and cast value
to OAuthProvider[] before mapping, or change the callback to (p: OAuthProvider)
=> p.id) so the compiler knows the structure when mapping in the oauthProviders
branch (the check key === 'oauthProviders' and the value.map call should use the
new typed shape).
In `@packages/template/src/dev-tool/tabs/overview-tab.tsx`:
- Line 131: The non-null assertion on user in the async call await
user!.signOut() should be replaced with an explicit null-check that throws if
user is missing; update the sign-out call in the same scope (the function that
calls user.signOut) to use the null-coalescing pattern (user ?? throwErr("user
is undefined when attempting signOut")) before invoking signOut, or otherwise
guard with an if and throw so the function no longer relies on the non-null
assertion operator.
- Around line 89-91: Replace the unsafe "catch (e: any)" usages with "catch
(err: unknown)" and use a small type-guard/helper to extract a safe message
before calling setStatus; for example, add a getErrorMessage(err: unknown):
string utility that checks for err being an Error or having a message property
and returns a string, then change the three catch blocks that call setStatus to
use getErrorMessage(err) when creating the status ({ type: 'error', message:
getErrorMessage(err) }); update all occurrences (the catch currently using
setStatus at the top and the two other similar catch blocks) to use this helper.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ebf7daf6-ae8b-4d42-8a82-43789b5a9072
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (31)
apps/backend/src/app/api/latest/ai/query/[mode]/route.tsapps/dashboard/next.config.mjsclaude/CLAUDE-KNOWLEDGE.mddocs/src/components/stack-auth/stack-team-switcher.tsxexamples/docs-examples/src/app/team/[teamId]/page.tsxpackages/react/package.jsonpackages/stack/package.jsonpackages/template/package-template.jsonpackages/template/package.jsonpackages/template/src/components-page/sign-in.tsxpackages/template/src/dev-tool/component-catalog.tsxpackages/template/src/dev-tool/dev-tool-context.tsxpackages/template/src/dev-tool/dev-tool-indicator.tsxpackages/template/src/dev-tool/dev-tool-panel.tsxpackages/template/src/dev-tool/dev-tool-styles.tspackages/template/src/dev-tool/dev-tool-tab-bar.tsxpackages/template/src/dev-tool/dev-tool-trigger.tsxpackages/template/src/dev-tool/hooks/use-component-registry.tsxpackages/template/src/dev-tool/hooks/use-dev-tool-state.tsxpackages/template/src/dev-tool/iframe-tab.tsxpackages/template/src/dev-tool/index.tsxpackages/template/src/dev-tool/tabs/ai-tab.tsxpackages/template/src/dev-tool/tabs/components-tab.tsxpackages/template/src/dev-tool/tabs/console-tab.tsxpackages/template/src/dev-tool/tabs/dashboard-tab.tsxpackages/template/src/dev-tool/tabs/docs-tab.tsxpackages/template/src/dev-tool/tabs/overview-tab.tsxpackages/template/src/dev-tool/tabs/support-tab.tsxpackages/template/src/index.tspackages/template/src/providers/stack-provider-client.tsxpackages/template/src/react-dom.d.ts
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
- Updated the Next.js configuration to use a new environment variable for local emulator detection. - Simplified the URL mapping logic in the TeamPage component. - Cleaned up peer dependencies in package.json files for react and template packages. - Removed unused react-dom type definitions and adjusted exports in the template's index file. - Streamlined the StackProviderClient component by removing the TranslationProvider wrapper.
- Simplified URL mapping logic in the MyTeamSwitcher component to handle null cases more effectively. - Removed unnecessary 'use client' directive from the SignIn component. - Updated the deriveDashboardBaseUrl function to improve clarity and added a new resolveApiBaseUrl function for better API base URL resolution. - Enhanced console and overview tabs in the dev tool to provide more accurate environment information and improved changelog fetching logic. - Cleaned up changelog handling and improved error management in the ChangelogCard component.
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (7)
packages/template/src/dev-tool/tabs/overview-tab.tsx (3)
125-126: Potential issue with empty string segments in initials computation.If
displayNamecontains consecutive spaces or leading/trailing spaces,split(' ')may produce empty string elements. Accessings[0]on an empty string returnsundefined, which becomes an empty string when joined—functionally harmless but could be cleaner.✨ Suggested improvement
const initials = user - ? (user.displayName || user.primaryEmail || '?').split(' ').map((s: string) => s[0]).join('').slice(0, 2).toUpperCase() + ? (user.displayName || user.primaryEmail || '?').split(' ').filter(Boolean).map((s) => s[0]).join('').slice(0, 2).toUpperCase() : '?';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/template/src/dev-tool/tabs/overview-tab.tsx` around lines 125 - 126, The initials computation can produce empty segments when displayName has extra spaces; update the expression that builds initials (the ternary using user.displayName / user.primaryEmail) to trim the source string and split on one-or-more whitespace (or filter out empty strings) before mapping to s[0], e.g., call .trim() and .split(/\s+/) or add .split(' ').filter(Boolean) so the map over s[0] never sees empty strings and then slice/uppercase as before.
116-116: Avoid non-null assertion; add defensive null check.The coding guidelines prefer
?? throwErr(...)over non-null assertions. Although the button is only rendered whenuseris truthy, a race condition could occur between render and click if the user signs out elsewhere. Add a defensive check.🛡️ Proposed fix
const handleSignOut = async () => { setLoading(true); setStatus(null); try { + if (!user) { + setStatus({ type: 'error', message: 'No user to sign out' }); + setLoading(false); + return; + } await user.signOut(); setStatus({ type: 'success', message: 'Signed out' }); - } catch (e: any) { - setStatus({ type: 'error', message: e.message || 'Sign out failed' }); + } catch (e: unknown) { + setStatus({ type: 'error', message: e instanceof Error ? e.message : 'Sign out failed' }); } setLoading(false); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/template/src/dev-tool/tabs/overview-tab.tsx` at line 116, Replace the non-null assertion on the sign-out call to a defensive check: instead of using await user!.signOut(), ensure user is present before calling signOut by using a nullish-coalescing guard (e.g., user ?? throwErr('user is null on signOut')) or an early-return/if-check, so the call to user.signOut() only happens when user is defined; update the call site referencing user and signOut (and use the existing throwErr helper if available) to avoid a potential race-condition NPE.
70-72: Useunknowninstead ofanyfor caught errors.Per coding guidelines, avoid using
anywithout an explanatory comment. The caught error should be typed asunknownand checked properly before accessing.message.♻️ Proposed fix
- } catch (e: any) { - setStatus({ type: 'error', message: e.message || 'Unknown error' }); + } catch (e: unknown) { + setStatus({ type: 'error', message: e instanceof Error ? e.message : 'Unknown error' }); }This same pattern applies to lines 106-108 and 118-120.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/template/src/dev-tool/tabs/overview-tab.tsx` around lines 70 - 72, Replace the use of catch (e: any) with catch (error: unknown) in the try/catch blocks inside overview-tab.tsx (the handlers that call setStatus), and before reading a message validate the error: if error is an instance of Error use error.message, otherwise coerce to a safe string (e.g., String(error) or a fallback like 'Unknown error'); update all three occurrences (the catch near setStatus for error state and the other two similar catch blocks referenced) so you never access .message on an unknown-typed value.packages/template/src/dev-tool/dev-tool-context.tsx (2)
128-132: Add comment explaining theanycast on globalThis.Per coding guidelines,
anyusage should include a comment explaining why it's necessary. The global property is intentionally untyped to allow the fetch interceptor to reach the store across HMR boundaries.📝 Proposed fix
// Expose globally so the fetch interceptor (which may be installed once) can // always reach the latest store even after HMR / remounts. if (typeof globalThis !== 'undefined') { + // Using `any` because __STACK_DEV_TOOL_LOG_STORE__ is a dev-only global not part of the type system (globalThis as any).__STACK_DEV_TOOL_LOG_STORE__ = globalLogStore; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/template/src/dev-tool/dev-tool-context.tsx` around lines 128 - 132, Add an inline comment explaining the use of the any cast on globalThis where we assign (globalThis as any).__STACK_DEV_TOOL_LOG_STORE__ = globalLogStore; — state that the cast is intentional because we are adding an ad-hoc global property (__STACK_DEV_TOOL_LOG_STORE__) that cannot be typed safely across module/HMR boundaries and we need an untyped global so the fetch interceptor can access the latest globalLogStore across remounts/HMR; place the comment immediately adjacent to the (globalThis as any) cast to satisfy the coding guideline.
72-74: Consider adding a brief comment explaining why errors are silently ignored.While silently ignoring localStorage errors is reasonable (private browsing mode, quota exceeded, security restrictions), a brief comment explaining the intent would improve maintainability per the "fail loud" principle—making it clear this is intentional fallback behavior, not accidental error swallowing.
📝 Suggested improvement
} catch { - // ignore + // localStorage may be unavailable (private browsing, quota exceeded, etc.) + // Fall back to defaults gracefully }Same applies to
saveStateat lines 83-85.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/template/src/dev-tool/dev-tool-context.tsx` around lines 72 - 74, The catch blocks in loadState and saveState silently swallow errors—add a brief explanatory comment inside each catch explaining this is intentional fallback behavior (e.g., to handle private browsing, quota limits, or security restrictions when accessing localStorage) so maintainers know errors are expected and not accidentally ignored; update the catch in the loadState function and the catch in saveState to include that comment and, optionally, a TODO or link to docs if you want to revisit logging/telemetry later.packages/template/src/dev-tool/tabs/console-tab.tsx (2)
160-166: Add type annotation to avoidany.Per coding guidelines, avoid
anywithout an explanatory comment. The OAuth provider type should be narrowed for safer iteration.♻️ Proposed fix
for (const [key, value] of Object.entries(projectConfig.config)) { if (key === 'oauthProviders' && Array.isArray(value)) { - lines.push(`${key}: ${value.map((p: any) => p.id).join(', ') || 'None'}`); + // OAuth providers are typed with an `id` field in the project config + lines.push(`${key}: ${(value as Array<{ id: string }>).map((p) => p.id).join(', ') || 'None'}`); } else { lines.push(`${key}: ${JSON.stringify(value)}`); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/template/src/dev-tool/tabs/console-tab.tsx` around lines 160 - 166, The loop over projectConfig.config uses (p: any) which violates the no-any rule; change the oauthProviders branch to narrow the type (e.g., assert value as Array<OAuthProvider> or define an OAuthProvider interface with id: string and use (p: OAuthProvider)) and replace (p: any) with the concrete type; ensure the type (OAuthProvider) is imported or defined near the top and add a brief explanatory comment if a cast is necessary, keeping the rest of the iteration logic the same (symbols: projectConfig.config, oauthProviders, and the mapping callback).
169-174: Add comment explaining theanycast on window global.Per coding guidelines, any usage of
anyshould include a comment explaining why it's necessary and how you can be certain errors would still be flagged.📝 Suggested improvement
lines.push(`--- Environment ---`); + // __STACK_VERSION__ is injected at build time and not part of Window type lines.push(`SDK Version: ${typeof window !== 'undefined' ? (window as any).__STACK_VERSION__ || 'Unknown' : 'Unknown'}`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/template/src/dev-tool/tabs/console-tab.tsx` around lines 169 - 174, Add an inline comment next to the (window as any).__STACK_VERSION__ cast explaining why the any is necessary (this is a runtime-injected global not present in our TypeScript defs), how we ensure type safety (e.g., we surface errors by keeping the access narrow and checking existence at runtime, and prefer adding a proper global declaration in a .d.ts to avoid any in future), and reference the symbol __STACK_VERSION__ and the lines that push environment info in console-tab.tsx so reviewers know where the justification lives; optionally note the intended follow-up (add a global Window interface augmentation) to remove the any in a subsequent change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/template/src/dev-tool/dev-tool-context.tsx`:
- Around line 128-132: Add an inline comment explaining the use of the any cast
on globalThis where we assign (globalThis as any).__STACK_DEV_TOOL_LOG_STORE__ =
globalLogStore; — state that the cast is intentional because we are adding an
ad-hoc global property (__STACK_DEV_TOOL_LOG_STORE__) that cannot be typed
safely across module/HMR boundaries and we need an untyped global so the fetch
interceptor can access the latest globalLogStore across remounts/HMR; place the
comment immediately adjacent to the (globalThis as any) cast to satisfy the
coding guideline.
- Around line 72-74: The catch blocks in loadState and saveState silently
swallow errors—add a brief explanatory comment inside each catch explaining this
is intentional fallback behavior (e.g., to handle private browsing, quota
limits, or security restrictions when accessing localStorage) so maintainers
know errors are expected and not accidentally ignored; update the catch in the
loadState function and the catch in saveState to include that comment and,
optionally, a TODO or link to docs if you want to revisit logging/telemetry
later.
In `@packages/template/src/dev-tool/tabs/console-tab.tsx`:
- Around line 160-166: The loop over projectConfig.config uses (p: any) which
violates the no-any rule; change the oauthProviders branch to narrow the type
(e.g., assert value as Array<OAuthProvider> or define an OAuthProvider interface
with id: string and use (p: OAuthProvider)) and replace (p: any) with the
concrete type; ensure the type (OAuthProvider) is imported or defined near the
top and add a brief explanatory comment if a cast is necessary, keeping the rest
of the iteration logic the same (symbols: projectConfig.config, oauthProviders,
and the mapping callback).
- Around line 169-174: Add an inline comment next to the (window as
any).__STACK_VERSION__ cast explaining why the any is necessary (this is a
runtime-injected global not present in our TypeScript defs), how we ensure type
safety (e.g., we surface errors by keeping the access narrow and checking
existence at runtime, and prefer adding a proper global declaration in a .d.ts
to avoid any in future), and reference the symbol __STACK_VERSION__ and the
lines that push environment info in console-tab.tsx so reviewers know where the
justification lives; optionally note the intended follow-up (add a global Window
interface augmentation) to remove the any in a subsequent change.
In `@packages/template/src/dev-tool/tabs/overview-tab.tsx`:
- Around line 125-126: The initials computation can produce empty segments when
displayName has extra spaces; update the expression that builds initials (the
ternary using user.displayName / user.primaryEmail) to trim the source string
and split on one-or-more whitespace (or filter out empty strings) before mapping
to s[0], e.g., call .trim() and .split(/\s+/) or add .split(' ').filter(Boolean)
so the map over s[0] never sees empty strings and then slice/uppercase as
before.
- Line 116: Replace the non-null assertion on the sign-out call to a defensive
check: instead of using await user!.signOut(), ensure user is present before
calling signOut by using a nullish-coalescing guard (e.g., user ??
throwErr('user is null on signOut')) or an early-return/if-check, so the call to
user.signOut() only happens when user is defined; update the call site
referencing user and signOut (and use the existing throwErr helper if available)
to avoid a potential race-condition NPE.
- Around line 70-72: Replace the use of catch (e: any) with catch (error:
unknown) in the try/catch blocks inside overview-tab.tsx (the handlers that call
setStatus), and before reading a message validate the error: if error is an
instance of Error use error.message, otherwise coerce to a safe string (e.g.,
String(error) or a fallback like 'Unknown error'); update all three occurrences
(the catch near setStatus for error state and the other two similar catch blocks
referenced) so you never access .message on an unknown-typed value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 05d0f350-929d-490e-b2b3-20c56e59ecde
📒 Files selected for processing (9)
apps/backend/src/app/api/latest/ai/query/[mode]/route.tsapps/dashboard/next.config.mjsclaude/CLAUDE-KNOWLEDGE.mdpackages/react/package.jsonpackages/template/package-template.jsonpackages/template/src/dev-tool/dev-tool-context.tsxpackages/template/src/dev-tool/tabs/console-tab.tsxpackages/template/src/dev-tool/tabs/overview-tab.tsxpackages/template/src/providers/stack-provider-client.tsx
✅ Files skipped from review due to trivial changes (1)
- apps/backend/src/app/api/latest/ai/query/[mode]/route.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/template/src/providers/stack-provider-client.tsx
- apps/dashboard/next.config.mjs
- claude/CLAUDE-KNOWLEDGE.md
- packages/react/package.json
- Added STACK_FEEDBACK_MODE environment variable to control feedback forwarding to production. - Refactored feedback submission logic to streamline handling for both authenticated and unauthenticated users. - Introduced new internal API route for fetching the latest component versions. - Updated feedback form to include feedback type selection (feedback or bug report). - Improved email handling for support feedback submissions, including better context for user feedback. - Enhanced tests for feedback submission to cover various scenarios, including authenticated and unauthenticated requests.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (7)
packages/template/src/dev-tool/tabs/support-tab.tsx (1)
55-55: Add justification foranycast or use proper typing.Per coding guidelines,
anyusage requires a comment explaining why it's necessary and how errors would still be caught. Consider typing the internals symbol access or adding documentation.- const opts = (app as any)[stackAppInternalsSymbol]?.getConstructorOptions?.() ?? {}; + // Using any because stackAppInternalsSymbol is an internal API not exposed in public types. + // Type errors are mitigated by optional chaining and nullish coalescing. + const opts = (app as any)[stackAppInternalsSymbol]?.getConstructorOptions?.() ?? {};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/template/src/dev-tool/tabs/support-tab.tsx` at line 55, The cast to any when accessing the internals ((app as any)[stackAppInternalsSymbol]?.getConstructorOptions?.()) lacks justification; replace it by declaring a proper interface for the internals (with getConstructorOptions returning the appropriate options type) and assert app as that interface before indexing, or add a short comment explaining why a runtime-hidden symbol requires an escape hatch and how type-safety is still preserved (e.g., validating the return shape). Update references to stackAppInternalsSymbol and getConstructorOptions to use the new typed interface so the any cast is removed or clearly documented.apps/dashboard/src/components/feedback-form.tsx (1)
43-43: Consider typing the render props parameter.The
props: anybypasses type checking. Per coding guidelines,anyshould include a comment explaining the necessity.- stackFormFieldRender: (props: any) => ( + // Props type comes from SmartForm's internal render system + stackFormFieldRender: (props: any) => (🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/components/feedback-form.tsx` at line 43, The inline render prop parameter for stackFormFieldRender is currently typed as props: any which bypasses type checking; update the parameter to a specific type (e.g., the appropriate form-rendering prop interface used in your codebase like StackFormFieldProps or React.ComponentProps<typeof SomeFieldComponent>) so TypeScript can validate usage in stackFormFieldRender, and if a precise type is not available immediately, replace any with unknown or a narrow union and add a concise comment explaining why any is necessary and when a proper type will be introduced; locate the arrow function assigned to stackFormFieldRender to make this change.packages/template/src/dev-tool/tabs/overview-tab.tsx (2)
70-72: Avoidanytype for caught errors.Per coding guidelines, avoid the
anytype. Useunknownand narrow appropriately, or access common error properties safely.♻️ Suggested fix
- } catch (e: any) { - setStatus({ type: 'error', message: e.message || 'Unknown error' }); + } catch (e) { + setStatus({ type: 'error', message: e instanceof Error ? e.message : 'Unknown error' }); }Apply the same pattern at lines 106 and 118.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/template/src/dev-tool/tabs/overview-tab.tsx` around lines 70 - 72, Replace the use of catch (e: any) with catch (e: unknown) and safely narrow the error before reading message: inside the catch blocks in the component (the handler that calls setStatus) check if e is an instance of Error and use e.message, otherwise convert to a safe string (e.g., String(e) or 'Unknown error'); apply this same change to the other two catch blocks referenced (the ones at lines around 106 and 118) so all catches use unknown and proper narrowing before calling setStatus({ type: 'error', message: ... }).
115-117: Avoid non-null assertion onuser.Per coding guidelines, prefer
?? throwErr(...)over non-null assertions. Althoughuseris checked in the conditional rendering, explicit validation is safer.♻️ Suggested fix
const handleSignOut = async () => { setLoading(true); setStatus(null); try { - await user!.signOut(); + if (!user) { + setStatus({ type: 'error', message: 'No user to sign out' }); + setLoading(false); + return; + } + await user.signOut(); setStatus({ type: 'success', message: 'Signed out' });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/template/src/dev-tool/tabs/overview-tab.tsx` around lines 115 - 117, Replace the non-null assertion on user when calling signOut with an explicit guard using the project's throw helper (e.g., throwErr or a similar runtime-guard) so the code reads like: resolve user via "user ?? throwErr('user is required to sign out')" and then call user.signOut(); update the call site referring to user!.signOut and any related setStatus logic accordingly so the runtime will throw a clear error if user is null instead of relying on a non-null assertion.apps/backend/src/app/api/latest/internal/feedback/route.tsx (1)
49-56: Consider adding a timeout to the production fetch.The fetch to production lacks a timeout, which could cause requests to hang indefinitely if the production endpoint is slow or unresponsive. For a dev-tool feedback endpoint this is low risk, but adding an
AbortControllerwith a reasonable timeout (e.g., 10s) would improve reliability.♻️ Optional improvement
+ const controller = new AbortController(); + const timeoutId = setTimeout(() => controller.abort(), 10000); const prodResponse = await fetch("https://api.stack-auth.com/api/latest/internal/feedback", { method: "POST", headers: { "content-type": "application/json", "accept-encoding": "identity" }, body: JSON.stringify(body), + signal: controller.signal, }); + clearTimeout(timeoutId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/backend/src/app/api/latest/internal/feedback/route.tsx` around lines 49 - 56, The production fetch to "https://api.stack-auth.com/api/latest/internal/feedback" (producing prodResponse) needs an AbortController timeout to avoid hanging; create an AbortController, start a timeout (e.g., 10_000 ms) that calls controller.abort(), pass controller.signal into the fetch options, clear the timeout after the fetch completes, and catch AbortError to throw an appropriate StatusError (or map it to a 504) instead of letting the request hang; update the fetch call in route.tsx to use this controller and ensure you only apply it to the prod forwarding logic that sets prodResponse.apps/e2e/tests/backend/endpoints/api/v1/internal/feedback.test.ts (1)
90-109: Bug report test lacks email content verification.The "send bug reports with correct label" test verifies the response but doesn't assert that the email subject contains "[Bug Report]" as expected from the backend changes. Consider adding assertions similar to the other tests.
💡 Suggested enhancement
it("should send bug reports with correct label", async ({ expect }) => { const subject = "[Bug Report] bug@example.com"; + const recipientMailbox = createMailbox("team@stack-auth.com"); const response = await niceBackendFetch("/api/v1/internal/feedback", { // ... }); expect(response).toMatchInlineSnapshot(`...`); + + const emails = await waitForOutboxEmailWithStatus(subject, "sent"); + expect(emails[0]?.to).toMatchObject({ + type: "custom-emails", + emails: ["team@stack-auth.com"], + }); + + const messages = await recipientMailbox.waitForMessagesWithSubject(subject); + expect(messages).toHaveLength(1); + expect(messages[0]?.subject).toBe(subject); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/e2e/tests/backend/endpoints/api/v1/internal/feedback.test.ts` around lines 90 - 109, The test "should send bug reports with correct label" currently only asserts the HTTP response; update it to also verify the outgoing email subject includes "[Bug Report]". After calling niceBackendFetch("/api/v1/internal/feedback", ...), inspect the test mailer/mock used elsewhere (e.g., the captured email or lastSentEmail used by other feedback tests) and add an assertion that the email subject contains "[Bug Report]" (or that the subject equals the expected string) so the test ensures the backend applies the bug label to the email.packages/template/src/dev-tool/dev-tool-styles.ts (1)
6-34: Factor the theme tokens before they drift.Lines 6-34, 1999-2025, and 2633-2683 repeat the same light/dark variable sets in multiple places. Since this is already a
.tsmodule, generating those blocks from one token definition would be safer than hand-maintaining parallel copies.Also applies to: 1999-2025, 2633-2683
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/template/src/dev-tool/dev-tool-styles.ts` around lines 6 - 34, The theme variable blocks repeated for .stack-devtool should be factored into a single token definition in this module (e.g., export const devToolTokens or a getDevToolTokens() object) and then used to generate the CSS variable blocks for the dark and light variants instead of duplicating them; update the existing CSS generation logic that emits the .stack-devtool blocks (the current repeated variable list) to read from that single token source and replace the three duplicated blocks currently present (the block shown here plus the other two duplicates) so changes to a token propagate everywhere.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/backend/src/app/api/latest/internal/projects/crud.tsx`:
- Around line 19-24: Swap the authentication and authorization checks so you
validate auth.user before checking auth.project.id: first check if (!auth.user)
and throw new KnownErrors.UserAuthenticationRequired(), then check if
(auth.project.id !== "internal") and throw new
KnownErrors.ExpectedInternalProject(); ensure you call the error constructors
(use parentheses) and update the order where auth is inspected so
unauthenticated requests return UserAuthenticationRequired instead of
ExpectedInternalProject.
In `@apps/dashboard/src/components/feedback-form.tsx`:
- Line 67: The current fallback to an empty string for
headers["X-Stack-Publishable-Client-Key"] can silently produce malformed auth;
instead, call getPublicEnvVar("NEXT_PUBLIC_STACK_PUBLISHABLE_CLIENT_KEY") into a
local const, validate it (e.g., if (!key) throw new Error("Missing
NEXT_PUBLIC_STACK_PUBLISHABLE_CLIENT_KEY")), and only set
headers["X-Stack-Publishable-Client-Key"] = key when present; reference
getPublicEnvVar and headers["X-Stack-Publishable-Client-Key"] to locate and
replace the existing nullish-coalescing fallback with an explicit failure path.
In `@packages/template/src/dev-tool/dev-tool-styles.ts`:
- Around line 132-159: Add a prefers-reduced-motion override that disables the
listed animations and transitions; wrap a media query `@media`
(prefers-reduced-motion: reduce) { ... } and inside set animation: none
!important and transition: none !important for the animated selectors (e.g.
.stack-devtool .sdt-panel-enter, .stack-devtool .sdt-panel-exiting, any
pulse/shimmer/spinner/smooth-scroll/thinking classes used elsewhere) and ensure
keyframe-driven elements are covered so sdt-panel-enter/sdt-panel-exit keyframes
and the pulse/shimmer/spinner animations are effectively disabled for users who
prefer reduced motion.
- Around line 188-207: The .stack-devtool .sdt-tab rule removes the default
outline but doesn't provide an accessible focus replacement; add a visible focus
style for keyboard users by defining .sdt-tab:focus and/or
.sdt-tab:focus-visible (or both) to restore clear focus indication (e.g.,
outline or box-shadow and adjusted border-radius) while preserving existing
layout and transitions; update the CSS near the .stack-devtool .sdt-tab
declaration so keyboard focus on .sdt-tab shows a distinct, accessible ring
(using the same variables like --sdt-radius or --sdt-text-secondary as needed)
without reintroducing default browser outline artifacts.
In `@packages/template/src/dev-tool/tabs/components-tab.tsx`:
- Around line 243-246: The fetch to
`${apiBaseUrl}/api/latest/internal/component-versions` currently swallows errors
in .catch(() => {}); change the catch to accept the error (e) and log it (e.g.,
console.error or the repo logger) so failures are visible while preserving
current behavior (do not remove the stale check or the setVersions call);
reference the fetch call, the apiBaseUrl endpoint string, the setVersions(...)
call and the stale variable when making the change.
In `@packages/template/src/dev-tool/tabs/overview-tab.tsx`:
- Around line 509-512: The catch block that currently swallows errors when
fetching the npm registry (the promise chain ending with .catch(() => { if
(!stale) setResult({ status: "error" }); })) should capture the error and log it
before updating state; change the .catch to accept an error parameter and log it
(e.g., console.error or the existing logger) with context about the npm version
check, then preserve the existing stale check and setResult({ status: "error" })
inside the catch so failures are visible in logs while behavior remains the
same.
---
Nitpick comments:
In `@apps/backend/src/app/api/latest/internal/feedback/route.tsx`:
- Around line 49-56: The production fetch to
"https://api.stack-auth.com/api/latest/internal/feedback" (producing
prodResponse) needs an AbortController timeout to avoid hanging; create an
AbortController, start a timeout (e.g., 10_000 ms) that calls
controller.abort(), pass controller.signal into the fetch options, clear the
timeout after the fetch completes, and catch AbortError to throw an appropriate
StatusError (or map it to a 504) instead of letting the request hang; update the
fetch call in route.tsx to use this controller and ensure you only apply it to
the prod forwarding logic that sets prodResponse.
In `@apps/dashboard/src/components/feedback-form.tsx`:
- Line 43: The inline render prop parameter for stackFormFieldRender is
currently typed as props: any which bypasses type checking; update the parameter
to a specific type (e.g., the appropriate form-rendering prop interface used in
your codebase like StackFormFieldProps or React.ComponentProps<typeof
SomeFieldComponent>) so TypeScript can validate usage in stackFormFieldRender,
and if a precise type is not available immediately, replace any with unknown or
a narrow union and add a concise comment explaining why any is necessary and
when a proper type will be introduced; locate the arrow function assigned to
stackFormFieldRender to make this change.
In `@apps/e2e/tests/backend/endpoints/api/v1/internal/feedback.test.ts`:
- Around line 90-109: The test "should send bug reports with correct label"
currently only asserts the HTTP response; update it to also verify the outgoing
email subject includes "[Bug Report]". After calling
niceBackendFetch("/api/v1/internal/feedback", ...), inspect the test mailer/mock
used elsewhere (e.g., the captured email or lastSentEmail used by other feedback
tests) and add an assertion that the email subject contains "[Bug Report]" (or
that the subject equals the expected string) so the test ensures the backend
applies the bug label to the email.
In `@packages/template/src/dev-tool/dev-tool-styles.ts`:
- Around line 6-34: The theme variable blocks repeated for .stack-devtool should
be factored into a single token definition in this module (e.g., export const
devToolTokens or a getDevToolTokens() object) and then used to generate the CSS
variable blocks for the dark and light variants instead of duplicating them;
update the existing CSS generation logic that emits the .stack-devtool blocks
(the current repeated variable list) to read from that single token source and
replace the three duplicated blocks currently present (the block shown here plus
the other two duplicates) so changes to a token propagate everywhere.
In `@packages/template/src/dev-tool/tabs/overview-tab.tsx`:
- Around line 70-72: Replace the use of catch (e: any) with catch (e: unknown)
and safely narrow the error before reading message: inside the catch blocks in
the component (the handler that calls setStatus) check if e is an instance of
Error and use e.message, otherwise convert to a safe string (e.g., String(e) or
'Unknown error'); apply this same change to the other two catch blocks
referenced (the ones at lines around 106 and 118) so all catches use unknown and
proper narrowing before calling setStatus({ type: 'error', message: ... }).
- Around line 115-117: Replace the non-null assertion on user when calling
signOut with an explicit guard using the project's throw helper (e.g., throwErr
or a similar runtime-guard) so the code reads like: resolve user via "user ??
throwErr('user is required to sign out')" and then call user.signOut(); update
the call site referring to user!.signOut and any related setStatus logic
accordingly so the runtime will throw a clear error if user is null instead of
relying on a non-null assertion.
In `@packages/template/src/dev-tool/tabs/support-tab.tsx`:
- Line 55: The cast to any when accessing the internals ((app as
any)[stackAppInternalsSymbol]?.getConstructorOptions?.()) lacks justification;
replace it by declaring a proper interface for the internals (with
getConstructorOptions returning the appropriate options type) and assert app as
that interface before indexing, or add a short comment explaining why a
runtime-hidden symbol requires an escape hatch and how type-safety is still
preserved (e.g., validating the return shape). Update references to
stackAppInternalsSymbol and getConstructorOptions to use the new typed interface
so the any cast is removed or clearly documented.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 19105383-b6eb-40fe-acb7-81047a6126b4
📒 Files selected for processing (21)
apps/backend/.env.developmentapps/backend/src/app/api/latest/ai/query/[mode]/route.tsapps/backend/src/app/api/latest/internal/component-versions/route.tsapps/backend/src/app/api/latest/internal/feedback/route.tsxapps/backend/src/app/api/latest/internal/projects/crud.tsxapps/backend/src/lib/internal-feedback-emails.tsxapps/dashboard/next.config.mjsapps/dashboard/src/components/feedback-form.tsxapps/e2e/tests/backend/endpoints/api/v1/internal/feedback.test.tsexamples/demo/src/stack.tsxpackages/template/src/dev-tool/component-catalog.tsxpackages/template/src/dev-tool/dev-tool-indicator.tsxpackages/template/src/dev-tool/dev-tool-panel.tsxpackages/template/src/dev-tool/dev-tool-styles.tspackages/template/src/dev-tool/tabs/ai-tab.tsxpackages/template/src/dev-tool/tabs/components-tab.tsxpackages/template/src/dev-tool/tabs/overview-tab.tsxpackages/template/src/dev-tool/tabs/support-tab.tsxpackages/template/src/lib/stack-app/common.tspackages/template/src/lib/stack-app/index.tspackages/template/src/lib/stack-app/url-targets.ts
✅ Files skipped from review due to trivial changes (1)
- packages/template/src/dev-tool/component-catalog.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/backend/src/app/api/latest/ai/query/[mode]/route.ts
Collaborator
Author
|
@greptile-ai review |
- Updated the API route for component versions to use a record of changelogs instead of a single changelog string. - Refactored feedback test cases to include checks for forwarding mode, enhancing test coverage for authenticated and unauthenticated feedback submissions. - Cleaned up dev tool styles by removing unused CSS related to preview functionality. - Improved the handling of project ID checks in the CRUD operations for better clarity and error management.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (4)
packages/template/src/dev-tool/tabs/support-tab.tsx (1)
31-41:⚠️ Potential issue | 🟠 MajorReapply each new support prefill.
FeedbackFormstays mounted after tab switches, soprefillApplied.currentnever resets. A later “report this issue” flow in the same session will keep the stale message/type instead of the new payload.Proposed fix
- const prefillApplied = useRef(false); - // Apply prefill when it changes (e.g. navigating from share dialog) useEffect(() => { - if (prefill && !prefillApplied.current) { + if (prefill != null) { setFeedbackType(prefill.feedbackType); setMessage(prefill.message); setStatus("idle"); - prefillApplied.current = true; + setErrorMessage(""); } }, [prefill]);Also drop
useReffrom Line 4 once this gate is removed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/template/src/dev-tool/tabs/support-tab.tsx` around lines 31 - 41, The current guard using prefillApplied (prefillApplied = useRef(false)) prevents applying a new prefill after tab switches; remove the ref and simplify the useEffect so whenever prefill changes and is truthy you call setFeedbackType(prefill.feedbackType), setMessage(prefill.message), and setStatus("idle") (i.e., drop the prefillApplied.current check and the ref declaration), ensuring new prefill payloads always overwrite the form state.packages/template/src/dev-tool/dev-tool-styles.ts (2)
188-207:⚠️ Potential issue | 🟠 MajorRestore a visible keyboard focus ring for the main tabs.
This rule removes the default outline, but there is no
.sdt-tab:focus-visiblereplacement, so keyboard navigation across the top-level tabs is invisible.Proposed fix
.stack-devtool .sdt-tab { position: relative; z-index: 1; display: flex; @@ transition: color 0.15s ease; white-space: nowrap; outline: none; } + + .stack-devtool .sdt-tab:focus-visible { + outline: 2px solid var(--sdt-accent); + outline-offset: 2px; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/template/src/dev-tool/dev-tool-styles.ts` around lines 188 - 207, The .stack-devtool .sdt-tab rule removes the default outline making keyboard focus invisible; add a .sdt-tab:focus-visible rule that restores a visible focus indicator (e.g., outline or box-shadow using your design tokens such as --sdt-focus or --sdt-ring color), ensure it targets .stack-devtool .sdt-tab:focus-visible and provides sufficient contrast and a clear shape (rounded corners matching --sdt-radius) while keeping outline: none on the base selector.
132-159:⚠️ Potential issue | 🟠 MajorRespect
prefers-reduced-motionacross the dev-tool surface.Panel enter/exit, tab fades, pulse/shimmer/spinner effects, share-overlay transitions, smooth scrolling, and thinking dots still run even when the user opts out of motion. Add a reduced-motion override that disables animation/transition and resets smooth scrolling.
Proposed fix
+ `@media` (prefers-reduced-motion: reduce) { + .stack-devtool, + .stack-devtool *, + .stack-devtool *::before, + .stack-devtool *::after { + animation: none !important; + transition: none !important; + scroll-behavior: auto !important; + } + }Also applies to: 273-288, 591-604, 737-749, 1848-1854, 1981-1990, 2124-2130, 2285-2300
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/template/src/dev-tool/dev-tool-styles.ts` around lines 132 - 159, Add a global reduced-motion override using `@media` (prefers-reduced-motion: reduce) that targets the dev tool surface (.stack-devtool) and disables animations/transitions and smooth scrolling: set animation: none !important; transition: none !important; scroll-behavior: auto !important; and override key animation-using classes such as sdt-panel-enter, sdt-panel-exit, .sdt-panel-exiting plus UI elements like .sdt-tab, .sdt-pulse, .sdt-shimmer, .sdt-spinner, .sdt-share-overlay, and .sdt-thinking-dots to ensure no keyframe/animation runs when the user prefers reduced motion; place this rule near the existing keyframes (e.g., alongside sdt-panel-enter and sdt-panel-exit) so it covers the listed sections too.packages/template/src/dev-tool/tabs/components-tab.tsx (1)
239-245:⚠️ Potential issue | 🟠 MajorDon't leave version checks stuck in a silent loading state.
If this fetch rejects,
latestVersionsstaysnull, so every custom page with a version remains in"loading"forever and the UI never tells the user why version metadata disappeared. Surface an explicit error state here instead of.catch(() => {}).Based on learnings, "When building frontend code, always carefully deal with loading and error states. Be very explicit with these and 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 `@packages/template/src/dev-tool/tabs/components-tab.tsx` around lines 239 - 245, The fetch in useEffect (inside useEffect -> fetch(...).then(...).catch(() => {})) silently swallows errors leaving latestVersions null and pages stuck in "loading"; update the catch to set a clear error/loading state instead of an empty catch: call setVersions([]) or a new setVersionsError(true) (or both), mark the stale guard as appropriate, and log the error (e.g., console.error(err)). Modify the promise chain around fetch(...) -> .then((r)=>r.json()).then((data)=>{ if (!stale) setVersions(data.versions); }).catch((err)=>{ if (!stale) { setVersions([]); /* or setVersionsError(true) */ } console.error('Failed fetching component versions', err); }) so the UI can render a non-loading error/empty state rather than hang.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/template/src/dev-tool/tabs/components-tab.tsx`:
- Around line 177-185: The update banner currently interpolates numeric versions
using page.version and promptData.latestVersion which can be null/0 for
legacy/deprecated pages; modify the rendering logic in the components-tab
(around the isOutdated && promptData branch) to detect legacy targets (e.g.,
page.deprecated === true OR page.version == null OR promptData.latestVersion ===
0) and, for those cases, render a dedicated legacy-migration message instead of
"version {page.version}" / "version {promptData.latestVersion}"; keep the
existing numeric comparison text for normal pages where page.version and
promptData.latestVersion are valid numbers.
- Around line 325-340: The page list rows are interactive but use <div>, which
prevents keyboard focus/activation—change the row element rendering to a <button
type="button"> (preserving key={page.key}) and move the onClick={() =>
setSelectedKey(page.key)} onto that button; keep data-selected={selectedKey ===
page.key} and the same child structure/dot/label/badge logic (using isOutdated,
classificationBadgeClass, classificationLabel) and ensure any visual chrome is
reset in packages/template/src/dev-tool/dev-tool-styles.ts so the button looks
like the existing row.
In `@packages/template/src/lib/stack-app/url-targets.ts`:
- Around line 66-100: The metadata is inconsistent: fullPrompt currently states
"passkey button is NOT shown on sign-up" while versions[1] and versions[2]
require passkey sign-up support, which will make getPagePrompt("signUp") produce
stale output and duplicate upgrade work; update fullPrompt to describe passkey
sign-up when project.config.passkeyEnabled is true (e.g., "If
project.config.passkeyEnabled is true, show a 'Sign up with Passkey' button that
calls app.signUpWithPasskey() and include a divider between OAuth/passkey and
credential/magic-link sections") and remove or replace the contradictory clause
(the line referencing passkeys not shown on sign-up), ensuring fullPrompt,
versions (1 and 2), and any logic referenced by getPagePrompt("signUp") are
consistent.
- Around line 628-630: The check in getPagePrompt currently uses the `in`
operator which inspects the prototype chain and forces a type cast; replace it
with `Object.hasOwn(customPagePrompts, pageName)` and add a small type guard
function (e.g., isCustomPageKey(key: string): key is keyof typeof
customPagePrompts) so TypeScript narrows the type without a cast; then safely
access `customPagePrompts[pageName].versions` (and other properties) knowing the
key is an own property of `customPagePrompts`.
---
Duplicate comments:
In `@packages/template/src/dev-tool/dev-tool-styles.ts`:
- Around line 188-207: The .stack-devtool .sdt-tab rule removes the default
outline making keyboard focus invisible; add a .sdt-tab:focus-visible rule that
restores a visible focus indicator (e.g., outline or box-shadow using your
design tokens such as --sdt-focus or --sdt-ring color), ensure it targets
.stack-devtool .sdt-tab:focus-visible and provides sufficient contrast and a
clear shape (rounded corners matching --sdt-radius) while keeping outline: none
on the base selector.
- Around line 132-159: Add a global reduced-motion override using `@media`
(prefers-reduced-motion: reduce) that targets the dev tool surface
(.stack-devtool) and disables animations/transitions and smooth scrolling: set
animation: none !important; transition: none !important; scroll-behavior: auto
!important; and override key animation-using classes such as sdt-panel-enter,
sdt-panel-exit, .sdt-panel-exiting plus UI elements like .sdt-tab, .sdt-pulse,
.sdt-shimmer, .sdt-spinner, .sdt-share-overlay, and .sdt-thinking-dots to ensure
no keyframe/animation runs when the user prefers reduced motion; place this rule
near the existing keyframes (e.g., alongside sdt-panel-enter and sdt-panel-exit)
so it covers the listed sections too.
In `@packages/template/src/dev-tool/tabs/components-tab.tsx`:
- Around line 239-245: The fetch in useEffect (inside useEffect ->
fetch(...).then(...).catch(() => {})) silently swallows errors leaving
latestVersions null and pages stuck in "loading"; update the catch to set a
clear error/loading state instead of an empty catch: call setVersions([]) or a
new setVersionsError(true) (or both), mark the stale guard as appropriate, and
log the error (e.g., console.error(err)). Modify the promise chain around
fetch(...) -> .then((r)=>r.json()).then((data)=>{ if (!stale)
setVersions(data.versions); }).catch((err)=>{ if (!stale) { setVersions([]); /*
or setVersionsError(true) */ } console.error('Failed fetching component
versions', err); }) so the UI can render a non-loading error/empty state rather
than hang.
In `@packages/template/src/dev-tool/tabs/support-tab.tsx`:
- Around line 31-41: The current guard using prefillApplied (prefillApplied =
useRef(false)) prevents applying a new prefill after tab switches; remove the
ref and simplify the useEffect so whenever prefill changes and is truthy you
call setFeedbackType(prefill.feedbackType), setMessage(prefill.message), and
setStatus("idle") (i.e., drop the prefillApplied.current check and the ref
declaration), ensuring new prefill payloads always overwrite the form state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d9cc297d-1d59-4c90-84e5-85c2645baee0
📒 Files selected for processing (10)
apps/backend/src/app/api/latest/internal/component-versions/route.tsapps/e2e/tests/backend/endpoints/api/v1/internal/feedback.test.tspackages/react/package.jsonpackages/stack/package.jsonpackages/template/package-template.jsonpackages/template/package.jsonpackages/template/src/dev-tool/dev-tool-styles.tspackages/template/src/dev-tool/tabs/components-tab.tsxpackages/template/src/dev-tool/tabs/support-tab.tsxpackages/template/src/lib/stack-app/url-targets.ts
✅ Files skipped from review due to trivial changes (3)
- packages/template/package.json
- packages/react/package.json
- packages/stack/package.json
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/e2e/tests/backend/endpoints/api/v1/internal/feedback.test.ts
- Introduced new types and functions for managing trigger positions and placements. - Refactored the createTrigger function to utilize snapping logic for positioning. - Added clamping and resolution functions to ensure the trigger remains within viewport bounds. - Created unit tests for trigger snapping functionality to validate behavior across different scenarios.
N2D4
approved these changes
Apr 12, 2026
- Added support for forwarding requests to a production proxy in the AI proxy integration. - Improved error handling and response management for API requests. - Enhanced the dev tool UI with new styles for changelog content and inline markdown rendering. - Introduced new functions for parsing markdown images and appending inline markdown elements. - Updated the StackProviderClient to remove unnecessary DevToolMount component. This update aims to improve the functionality and user experience of the AI proxy and development tools.
- Improved the stringifyForDebug function to handle undefined, function, and symbol types more gracefully. - Introduced a new utility function, getLastItem, to simplify retrieval of the last item from an array. - Updated getCurrentAssistantMessage and appendTextDelta functions to utilize getLastItem for better readability. - Enhanced error handling in AI stream event processing. - Added tests for custom target version handling in URL targets, ensuring support for the latest documented versions and proper error throwing for unsupported versions. These changes aim to improve code clarity and robustness in the development tools and URL handling logic.
- Updated the createOverviewTab and createConsoleTab functions to return an object containing the element and a cleanup function, enhancing resource management. - Modified the createPanel function to handle the new return structure from tab functions, ensuring proper cleanup of resources. - Introduced a mountTab helper function to streamline the process of appending tab elements and managing cleanup functions. These changes improve the organization and maintainability of the dev tool's tab management logic.
- Updated the closePanel function to ensure proper cleanup of the panel element before setting it to null. - Improved the exit animation handling by moving the cleanup call before nullifying the panel reference. These changes enhance the reliability of the panel closing process in the development tool.
This was referenced Apr 15, 2026
Open
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.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Dependencies