Skip to content

feat(cms-130): integrate dashboard with real Studio runtime data#26

Merged
iipanda merged 17 commits into
mainfrom
feat/cms-130-dashboard-real-data
Apr 6, 2026
Merged

feat(cms-130): integrate dashboard with real Studio runtime data#26
iipanda merged 17 commits into
mainfrom
feat/cms-130-dashboard-real-data

Conversation

@iipanda
Copy link
Copy Markdown
Collaborator

@iipanda iipanda commented Apr 5, 2026

Summary

Replaces all mock data in the Studio dashboard with real API-backed data from existing MVP contracts (schema list, content list, session, capabilities). Removes fabricated widgets that have no truthful data source in MVP.

  • Stat cards now show real document, published, and draft counts derived from GET /api/v1/content pagination totals
  • Content Types card lists schema types from GET /api/v1/schema with per-type document counts and published/draft progress bars
  • Recent Documents replaces the fabricated "Recent Activity" feed with the 5 most recently updated documents from the content API
  • Removed: "Editors online" card (live collaboration is non-MVP), weekly growth/today-updated trends (no temporal aggregation API)
  • Added: Loading spinner, empty state with schema CTA, error state with message, forbidden state for 401/403
  • Welcome message uses authenticated session email instead of mock user name

New files

File Purpose
packages/studio/src/lib/content-list-api.ts Studio API client for GET /api/v1/content with filter/sort/pagination support
packages/studio/src/lib/content-list-api.test.ts 8 tests covering query params, headers, error codes, and edge cases
packages/studio/src/lib/dashboard-data.ts Extracted pure async dashboard data loader (testable without DOM)
packages/studio/src/lib/dashboard-data.test.ts 9 tests covering loaded, empty, error, forbidden, type cap, and draft states

Acceptance criteria

  1. Dashboard summary cards and lists backed by real Studio/runtime data — done
  2. Unsupported widgets removed or replaced with truthful content — done (editors online, activity feed, trends)
  3. Dashboard handles loading, empty, error, and forbidden states without mock fallback — done
  4. Role-aware interaction constraints enforced — done (capabilities context gates schema CTA in empty state)
  5. No new public contracts introduced — done (consumes existing APIs only)

Dependencies

  • CMS-48 (internal Studio route surfaces)
  • CMS-129 (session, env context, role-aware nav)

Test plan

  • bun run check passes (build + typecheck)
  • bun run format:check passes
  • 248 studio tests pass (231 existing + 17 new, 0 failures)
  • New tests cover: real-data state, empty state, error state (401/403/500/generic), type cap at 5, recent document fields
  • No studio-review impact (no dashboard fixtures exist in review app)
  • Manual verification: start dev server, confirm dashboard loads real data from backend

Summary by CodeRabbit

  • New Features

    • Live dashboard with totals, per-type stats (capped), recent documents, and loading/forbidden/error states
    • Admin auth UX: email/password sign-in, SSO initiation, return-to handling, and redirect for unauthenticated sessions
    • New capability flag to control content-creation visibility
  • New Components

    • Content-list API client, dashboard data loader, login API client, Login page and runtime /login route
    • Review API stubs for session, content listing, and environments
  • Tests

    • Tests added for content-list API, dashboard loader, and login/SSO flows
  • Chores

    • Demo seed now ensures demo RBAC grant

Replace all mock data in the Studio dashboard with real API-backed data
from existing MVP contracts. The dashboard now fetches schema types from
GET /api/v1/schema and content stats from GET /api/v1/content to display
truthful document counts, published/draft ratios, and per-type breakdowns.

- Add content-list-api client for paginated content queries with filtering
- Extract dashboard data loading into testable pure async module
- Replace fabricated stat cards with real document/published/draft counts
- Replace mock content types list with schema API + per-type content counts
- Replace fabricated recent activity feed with real recent documents list
- Remove non-MVP "Editors online" widget and temporal trend data
- Add loading, empty, error, and forbidden state handling
- Use session context for personalized welcome message
- Add 17 new tests covering all dashboard states and API client behavior
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 5, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
mdcms-studio-review Ready Ready Preview, Comment Apr 6, 2026 1:05am

Request Review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 5, 2026

📝 Walkthrough

Walkthrough

Adds login and content-list client APIs and tests, a dashboard-data loader and tests, server SSO provider route and demo RBAC grant logic, an API-driven admin dashboard and login UI, capability/mount-info plumbing, and related test adjustments.

Changes

Cohort / File(s) Summary
Content List API
packages/studio/src/lib/content-list-api.ts, packages/studio/src/lib/content-list-api.test.ts
New content-list API client with list(query?): builds /api/v1/content requests, applies studio auth and project/environment headers, parses/normalizes pagination, handles malformed payloads and non-OK responses via RuntimeError. Tests cover query strings, headers, pagination, malformed responses, and HTTP error passthrough.
Login API & Tests
packages/studio/src/lib/login-api.ts, packages/studio/src/lib/login-api.test.ts
New createLoginApi exposing login and getSsoProviders; handles success/401/429/network errors and SSO providers parsing. Tests validate request shapes, outcome mapping (success, invalid_credentials, throttled, error), retry extraction, and SSO provider fallback behavior.
Dashboard Loader & Tests
packages/studio/src/lib/dashboard-data.ts, packages/studio/src/lib/dashboard-data.test.ts
Adds loadDashboardData that concurrently fetches schema entries and aggregated content counts, computes totals/drafts, per-type stats (limit 5), recent documents (limit 5), and maps errors to forbidden/error. Tests verify aggregations, limits, recent docs shape, and error mappings.
Admin Dashboard Page & Layout
packages/studio/src/lib/runtime-ui/app/admin/page.tsx, packages/studio/src/lib/runtime-ui/app/admin/layout.tsx, packages/studio/src/lib/runtime-ui/app/admin/mount-info-context.tsx
Replaces mock dashboard with API-driven fetching via loadDashboardData, adds loading/forbidden/error branches and unmount cancellation guards, renders contentTypes and recentDocuments, enforces session redirect to /admin/login, and extends mount-info with auth (default { mode: "cookie" }).
Login Page & Routing
packages/studio/src/lib/runtime-ui/app/admin/login-page.tsx, packages/studio/src/lib/remote-studio-app.tsx
Adds client-side LoginPage with email/password form, SSO initiation, return-to handling, and registers /login runtime route; integrates session and mount info providers for the unauthenticated flow.
Capabilities Context & Tests
packages/studio/src/lib/runtime-ui/app/admin/capabilities-context.tsx, packages/studio/src/lib/runtime-ui/app/admin/settings-page.test.tsx, packages/studio/src/lib/remote-studio-app.test.ts
Introduces canCreateContent boolean to AdminCapabilitiesValue and default capabilities; updates test fixtures to include the new flag.
Auth Service & Server Changes
apps/server/src/lib/auth.ts, apps/server/src/bin/demo-seed-api-key.ts, apps/server/src/lib/collaboration-auth.test.ts
Adds AuthService.listSsoProviders() implementation and GET /api/v1/auth/sso/providers route; provider name formatting and aggregation; demo seed script ensures demo RBAC grant insertion; test stub updated to include new method.
Studio Review API & Fixtures
apps/studio-review/.../api/v1/auth/session/route.ts, apps/studio-review/.../api/v1/content/route.ts, apps/studio-review/.../api/v1/environments/route.ts, apps/studio-review/review/content-documents.ts
Adds review API routes for session, content listing (with filters/pagination/sort), environments, and a helper to list review content documents for scenarios.
Misc (types/exports/imports)
packages/studio/src/lib/content-list-api.ts, packages/studio/src/lib/login-api.ts, packages/studio/src/lib/runtime-ui/...
Exports new API types and factories; updates StudioMountInfo to include auth with default; adjusts imports and minor test fixtures.

Sequence Diagram

sequenceDiagram
    actor User
    participant Dashboard as "Dashboard Page"
    participant Loader as "loadDashboardData"
    participant SchemaApi as "Schema API"
    participant ContentApi as "Content List API"
    participant HTTP as "HTTP Layer"

    User->>Dashboard: open /admin
    Dashboard->>Dashboard: set state = loading
    Dashboard->>Loader: loadDashboardData(schemaApi, contentApi)

    par Fetch schema types
        Loader->>SchemaApi: list()
        SchemaApi->>HTTP: GET /api/v1/schema
        HTTP-->>SchemaApi: schema entries
        SchemaApi-->>Loader: types
    and Fetch totals & recent
        Loader->>ContentApi: list({ limit: 0 })
        ContentApi->>HTTP: GET /api/v1/content?limit=0
        HTTP-->>ContentApi: pagination total
        ContentApi-->>Loader: total

        Loader->>ContentApi: list({ published: true, limit: 0 })
        ContentApi->>HTTP: GET /api/v1/content?published=true&limit=0
        HTTP-->>ContentApi: published total
        ContentApi-->>Loader: published

        Loader->>ContentApi: list({ sort: "updatedAt", order: "desc", limit: 5 })
        ContentApi->>HTTP: GET /api/v1/content?sort=updatedAt&order=desc&limit=5
        HTTP-->>ContentApi: recent docs
        ContentApi-->>Loader: recent documents
    and Per-type stats
        Loader->>ContentApi: list({ type: T, limit: 0 }) for each type (up to 5)
        ContentApi->>HTTP: GET /api/v1/content?type=T&limit=0
        HTTP-->>ContentApi: type totals
        ContentApi-->>Loader: per-type totals
    end

    Loader->>Loader: compute draft counts & aggregate
    Loader-->>Dashboard: DashboardLoadResult (loaded|forbidden|error)
    Dashboard->>User: render appropriate UI
Loading

Estimated Code Review Effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly Related PRs

Poem

🐇
I hopped through tests and fetches bright,
Counts and types now wake to light,
No mock remains, the dashboard sings,
Recent docs and stats take wings,
A joyful hop for devs tonight!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: integrating the dashboard with real Studio runtime data instead of mock data, matching the core objective and changeset focus.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/cms-130-dashboard-real-data

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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/studio/src/lib/dashboard-data.ts`:
- Around line 48-50: The calculation of draftDocuments in dashboard-data.ts can
yield negative values when publishedResult.pagination.total >
totalResult.pagination.total; update the logic that sets draftDocuments
(currently using totalDocuments and publishedDocuments) to clamp it to zero
(e.g., use Math.max(0, totalDocuments - publishedDocuments)) and optionally add
a single-line debug/warn log when the clamp occurs to aid observability; adjust
any consumers expecting negatives if necessary.

In `@packages/studio/src/lib/runtime-ui/app/admin/page.tsx`:
- Around line 39-54: The formatRelativeTime function should guard against
invalid date strings by validating the created Date before using getTime(); in
formatRelativeTime(dateStr: string) check if Number.isNaN(date.getTime()) (or
use isNaN(date.getTime())) right after const date = new Date(dateStr) and return
a safe fallback (e.g., an empty string or "Invalid date") instead of continuing
to compute diffs so the UI won't display "Invalid Date"; update only
formatRelativeTime to perform this validation and return the fallback when the
date is invalid.
🪄 Autofix (Beta)

✅ Autofix completed


ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 97930784-7aa9-4f33-b3b4-08b39a594eec

📥 Commits

Reviewing files that changed from the base of the PR and between 4cb8285 and fd045f4.

📒 Files selected for processing (5)
  • packages/studio/src/lib/content-list-api.test.ts
  • packages/studio/src/lib/content-list-api.ts
  • packages/studio/src/lib/dashboard-data.test.ts
  • packages/studio/src/lib/dashboard-data.ts
  • packages/studio/src/lib/runtime-ui/app/admin/page.tsx

Comment thread packages/studio/src/lib/dashboard-data.ts Outdated
Comment thread packages/studio/src/lib/runtime-ui/app/admin/page.tsx
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 5, 2026

Note

Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.

Fixes Applied Successfully

Fixed 2 file(s) based on 2 unresolved review comments.

Files modified:

  • packages/studio/src/lib/dashboard-data.ts
  • packages/studio/src/lib/runtime-ui/app/admin/page.tsx

Commit: ee947fefdcfb6dffc7510e72bbe249c68c66894d

The changes have been pushed to the feat/cms-130-dashboard-real-data branch.

Time taken: 3m 29s

Fixed 2 file(s) based on 2 unresolved review comments.

Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 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/studio/src/lib/dashboard-data.ts`:
- Around line 40-46: The top-level Promise.all currently includes
schemaApi.list(), so a 403 from schemaApi.list() rejects the whole await and
prevents contentApi.list() results from being returned; change this by removing
schemaApi.list() from the Promise.all and call schemaApi.list() separately
inside a try/catch (or run it with .catch) so that if it throws a 403 you assign
schemaTypes = [] (or degrade to an empty array) while still awaiting the three
contentApi.list() calls (totalResult, publishedResult, recentResult) in
Promise.all; apply the same pattern for the later schemaApi.list() usage around
the second block (lines 94-101) so schema access failures don’t block
content-backed widgets.
- Around line 57-68: In the async mapping callback (the typesToShow.map handler)
clamp the per-type published count before returning: compute totalCount from
typeTotal.pagination.total and publishedCount from
typePublished.pagination.total, then set publishedCount =
Math.min(publishedCount, totalCount) so the returned object (type, directory,
localized, totalCount, publishedCount) never reports published > total; update
the publishedCount field in that return object accordingly.

In `@packages/studio/src/lib/runtime-ui/app/admin/page.tsx`:
- Around line 183-186: The UI is using data.contentTypes.length (which is the
truncated first-five list returned by loadDashboardData()) to render the
"detail" count; update the loader to return the full schema count (e.g., add a
field like totalContentTypes or contentTypeCount in loadDashboardData()) and
then change the page component to use that new field instead of
data.contentTypes.length when rendering the detail text; locate references to
loadDashboardData() and the data object and replace the length-based count with
the new totalContentTypes/contentTypeCount property, preserving the
pluralization logic.
- Around line 73-89: When starting a new fetch for a changed mount (when
project, environment, and apiBaseUrl are present), reset the dashboard state to
loading immediately and ensure the in-flight promise is cancellable: call
setState({ status: "loading" }) right before creating config/authOpts and
invoking loadDashboardData, keep the existing cancelled flag logic and cleanup
(set cancelled = true in the effect cleanup) so loadDashboardData's .then only
calls setState(result) when not cancelled; reference functions/variables:
project, environment, apiBaseUrl, setState, loadDashboardData, cancelled,
createStudioSchemaRouteApi, createStudioContentListApi.
🪄 Autofix (Beta)

✅ Autofix completed


ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: b2873928-2ee1-46c3-96ac-8df225b66b25

📥 Commits

Reviewing files that changed from the base of the PR and between fd045f4 and ee947fe.

📒 Files selected for processing (2)
  • packages/studio/src/lib/dashboard-data.ts
  • packages/studio/src/lib/runtime-ui/app/admin/page.tsx

Comment thread packages/studio/src/lib/dashboard-data.ts
Comment thread packages/studio/src/lib/dashboard-data.ts Outdated
Comment thread packages/studio/src/lib/runtime-ui/app/admin/page.tsx Outdated
Comment thread packages/studio/src/lib/runtime-ui/app/admin/page.tsx
…rd widgets

Schema API errors (e.g. 403) no longer reject the entire dashboard load.
schemaApi.list() is called with .catch() so it degrades to an empty array
while the three content API calls still resolve. This lets content stat
cards and the recent documents list render even when the caller lacks
schema:read permission.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
packages/studio/src/lib/dashboard-data.ts (1)

42-48: Schema errors are silently swallowed, including unexpected failures.

The .catch(() => []) on line 44 degrades gracefully on any error, but this includes 500s, network timeouts, and other unexpected failures that might warrant logging or different handling. Consider logging non-403 errors before degrading:

💡 Optional: Add observability for unexpected schema failures
-        schemaApi.list().catch((): SchemaRegistryEntry[] => []),
+        schemaApi.list().catch((error: unknown): SchemaRegistryEntry[] => {
+          const statusCode =
+            error && typeof error === "object" && "statusCode" in error
+              ? (error as { statusCode: number }).statusCode
+              : undefined;
+          if (statusCode !== 403) {
+            console.warn("Schema list failed, degrading to empty:", error);
+          }
+          return [];
+        }),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/studio/src/lib/dashboard-data.ts` around lines 42 - 48, The current
.catch((): SchemaRegistryEntry[] => []) on schemaApi.list() swallows all errors;
change it to inspect the caught error in the catch handler for schemaApi.list():
if the error represents a 403/forbidden (e.g., err?.status === 403) return an
empty SchemaRegistryEntry[] as before, otherwise log the error (e.g., via
console.error or the project logger) with context "schemaApi.list error" and
then return [] (or rethrow if your policy prefers). Target the schemaApi.list()
call and the schemaTypes variable to implement this conditional logging
behavior.
🤖 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/studio/src/lib/dashboard-data.ts`:
- Around line 42-48: The current .catch((): SchemaRegistryEntry[] => []) on
schemaApi.list() swallows all errors; change it to inspect the caught error in
the catch handler for schemaApi.list(): if the error represents a 403/forbidden
(e.g., err?.status === 403) return an empty SchemaRegistryEntry[] as before,
otherwise log the error (e.g., via console.error or the project logger) with
context "schemaApi.list error" and then return [] (or rethrow if your policy
prefers). Target the schemaApi.list() call and the schemaTypes variable to
implement this conditional logging behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 09a2d11f-a07a-42a8-a7b9-defb6678d71f

📥 Commits

Reviewing files that changed from the base of the PR and between ee947fe and f53b624.

📒 Files selected for processing (2)
  • packages/studio/src/lib/dashboard-data.test.ts
  • packages/studio/src/lib/dashboard-data.ts
✅ Files skipped from review due to trivial changes (1)
  • packages/studio/src/lib/dashboard-data.test.ts

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 5, 2026

Note

Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.

Fixes Applied Successfully

Fixed 2 file(s) based on 4 unresolved review comments.

Files modified:

  • packages/studio/src/lib/dashboard-data.ts
  • packages/studio/src/lib/runtime-ui/app/admin/page.tsx

Commit: 69b531213761951e0741b701559de0b102d6786c

The changes have been pushed to the feat/cms-130-dashboard-real-data branch.

Time taken: 3m 10s

Fixed 2 file(s) based on 4 unresolved review comments.

Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (4)
packages/studio/src/lib/runtime-ui/app/admin/page.tsx (3)

87-91: Unhandled promise rejection if loadDashboardData throws synchronously.

While unlikely given the implementation, .then() without .catch() means any unexpected rejection goes unhandled. Consider adding error handling for robustness.

💡 Add catch handler
     loadDashboardData(schemaApi, contentApi).then((result) => {
       if (!cancelled) {
         setState(result);
       }
-    });
+    }).catch((err) => {
+      if (!cancelled) {
+        console.error("Dashboard load failed unexpectedly:", err);
+        setState({ status: "error", message: "An unexpected error occurred." });
+      }
+    });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/studio/src/lib/runtime-ui/app/admin/page.tsx` around lines 87 - 91,
The call to loadDashboardData(...).then(...) can throw synchronously or reject
and currently has no rejection handler; update the code that invokes
loadDashboardData (the promise chain that calls setState when !cancelled) to
handle errors—either append a .catch(...) to the promise chain or wrap the call
in an async IIFE with try/catch—and ensure any caught error is handled/logged
and does not leave an unhandled promise rejection (refer to loadDashboardData,
setState and the cancelled flag to preserve existing cancellation behavior).

1-1: Consider removing @ts-nocheck or documenting why it's needed.

Disabling TypeScript checking for the entire file masks potential type errors. If specific lines require suppression, prefer targeted // @ts-expect-error`` comments with explanations.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/studio/src/lib/runtime-ui/app/admin/page.tsx` at line 1, Remove the
top-level "// `@ts-nocheck`" directive in this file and either fix the underlying
TypeScript errors or replace it with focused, line-specific suppressions (e.g.,
"// `@ts-expect-error` — explain why" directly above the problematic expression).
Search for any remaining type errors reported by the TS compiler in this file
(page.tsx) and address them by adding proper types, refining props/state types,
or adding the targeted suppression comments only where necessary, documenting
each "// `@ts-expect-error`" to justify why the check is suppressed.

59-62: Edge case: emails without @ return undefined from split.

If email somehow lacks an @ character, split("@")[0] returns the entire string (not undefined), so the fallback ?? email is never reached. This is fine for the current behavior, but the fallback is misleading.

💡 Minor clarity improvement
 function deriveUserLabel(email: string): string {
-  const local = email.split("@")[0] ?? email;
+  const local = email.split("@")[0];
   return local.charAt(0).toUpperCase() + local.slice(1);
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/studio/src/lib/runtime-ui/app/admin/page.tsx` around lines 59 - 62,
The deriveUserLabel function's fallback is misleading and doesn't handle empty
emails safely; change the logic to first coerce email to a string, take the part
before '@', and handle an empty local part before capitalizing. For example, in
deriveUserLabel replace the current local assignment with something like: const
local = (email || '').split('@')[0]; then if local is empty return '' (or a
desired default) otherwise return local.charAt(0).toUpperCase() +
local.slice(1); this keeps behavior for emails without '@' and avoids calling
charAt on an empty string.
packages/studio/src/lib/dashboard-data.ts (1)

43-49: Schema error handling swallows all errors, not just 403s.

The .catch((): SchemaRegistryEntry[] => []) on line 45 silently converts any schema API failure (network errors, 500s, etc.) into an empty array. While graceful degradation for 403 was the stated goal, this also masks transient failures where a retry might succeed. Consider logging non-403 errors for observability.

💡 Optional: Log unexpected schema errors
       await Promise.all([
-        schemaApi.list().catch((): SchemaRegistryEntry[] => []),
+        schemaApi.list().catch((err: unknown): SchemaRegistryEntry[] => {
+          const code =
+            err && typeof err === "object" && "statusCode" in err
+              ? (err as { statusCode: number }).statusCode
+              : undefined;
+          if (code !== 403) {
+            console.warn("Schema API error (degrading to empty):", err);
+          }
+          return [];
+        }),
         contentApi.list({ limit: 1 }),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/studio/src/lib/dashboard-data.ts` around lines 43 - 49, The current
catch on schemaApi.list() swallows all errors; change it to only swallow 403s:
call schemaApi.list().catch((err): SchemaRegistryEntry[] => { if (err?.status
=== 403 || err?.response?.status === 403) return [];
console.error("schemaApi.list error:", err); throw err; }) so schemaTypes is []
on forbidden but other errors are logged and rethrown (so Promise.all fails and
transient errors aren't masked). Ensure you update the call site where
schemaTypes is assigned and keep the SchemaRegistryEntry type reference.
🤖 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/studio/src/lib/dashboard-data.ts`:
- Around line 43-49: The current catch on schemaApi.list() swallows all errors;
change it to only swallow 403s: call schemaApi.list().catch((err):
SchemaRegistryEntry[] => { if (err?.status === 403 || err?.response?.status ===
403) return []; console.error("schemaApi.list error:", err); throw err; }) so
schemaTypes is [] on forbidden but other errors are logged and rethrown (so
Promise.all fails and transient errors aren't masked). Ensure you update the
call site where schemaTypes is assigned and keep the SchemaRegistryEntry type
reference.

In `@packages/studio/src/lib/runtime-ui/app/admin/page.tsx`:
- Around line 87-91: The call to loadDashboardData(...).then(...) can throw
synchronously or reject and currently has no rejection handler; update the code
that invokes loadDashboardData (the promise chain that calls setState when
!cancelled) to handle errors—either append a .catch(...) to the promise chain or
wrap the call in an async IIFE with try/catch—and ensure any caught error is
handled/logged and does not leave an unhandled promise rejection (refer to
loadDashboardData, setState and the cancelled flag to preserve existing
cancellation behavior).
- Line 1: Remove the top-level "// `@ts-nocheck`" directive in this file and
either fix the underlying TypeScript errors or replace it with focused,
line-specific suppressions (e.g., "// `@ts-expect-error` — explain why" directly
above the problematic expression). Search for any remaining type errors reported
by the TS compiler in this file (page.tsx) and address them by adding proper
types, refining props/state types, or adding the targeted suppression comments
only where necessary, documenting each "// `@ts-expect-error`" to justify why the
check is suppressed.
- Around line 59-62: The deriveUserLabel function's fallback is misleading and
doesn't handle empty emails safely; change the logic to first coerce email to a
string, take the part before '@', and handle an empty local part before
capitalizing. For example, in deriveUserLabel replace the current local
assignment with something like: const local = (email || '').split('@')[0]; then
if local is empty return '' (or a desired default) otherwise return
local.charAt(0).toUpperCase() + local.slice(1); this keeps behavior for emails
without '@' and avoids calling charAt on an empty string.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 95a57c11-20c8-4ff3-bf03-fa4a0e9a365e

📥 Commits

Reviewing files that changed from the base of the PR and between f53b624 and 69b5312.

📒 Files selected for processing (2)
  • packages/studio/src/lib/dashboard-data.ts
  • packages/studio/src/lib/runtime-ui/app/admin/page.tsx

- Schema .catch() now only swallows 401/403 and rethrows other errors
  (500s, network failures) so transient issues are not silently masked
- Add .catch() to loadDashboardData promise chain in page.tsx to prevent
  unhandled rejections from unexpected throws
- Harden deriveUserLabel to safely handle empty email strings
- Add tests for schema 500 and network error propagation
…ookie

The dashboard was hardcoding { auth: { mode: "cookie" } } when creating
API clients, but the dev/review app uses token auth. API calls failed
with 401/403 because the Bearer token was never sent.

Fix: expose auth on StudioMountInfo context so child pages use the
actual auth mode provided by the host. The layout now passes
context.auth through to the mount info provider.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 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/studio/src/lib/dashboard-data.ts`:
- Around line 60-67: The top-level publishedDocuments value is not clamped and
can exceed totalDocuments; update the logic around the
publishedDocuments/draftDocuments calculation to normalize publishedDocuments
before returning (e.g., clamp publishedDocuments to the range [0,
totalDocuments] using publishedDocuments and totalDocuments), then recompute
draftDocuments from the clamped published value (currently compute
draftDocuments = Math.max(0, totalDocuments - publishedDocuments) after
clamping) and keep the existing warning log for inconsistencies; adjust any
return/object that exposes publishedDocuments so it uses the clamped
publishedDocuments variable.

In `@packages/studio/src/lib/runtime-ui/app/admin/page.tsx`:
- Around line 44-56: The relative-time code calculates diff from date and then
returns "Just now" when seconds < 60, but negative diffs (future timestamps)
cause incorrect "Just now" outputs; before computing seconds/minutes/hours,
check if diff < 0 and return a stable fallback (for consistency use the same
formatted fallback used at the end: date.toLocaleDateString("en-US", { month:
"short", day: "numeric" })); update the logic around the variables now, diff,
date (the branch that currently returns "Just now") so future timestamps are
handled immediately and the rest of the time-ago branches remain unchanged.
- Around line 263-272: Add a content-write flag to the admin capability model
and gate the CTA: extend AdminCapabilitiesValue to include a boolean like
canCreateContent, populate it by extracting response.capabilities.content.write
from the CurrentPrincipalCapabilitiesResponse in the layout's capability
initialization where capabilities are built (the code around the layout init
that currently reads other response.capabilities), and then wrap/gate the "New
Document" Button/Link render in
packages/studio/src/lib/runtime-ui/app/admin/page.tsx so it only renders when
canCreateContent is true; ensure the flag name is exported where
AdminCapabilitiesValue is used so the page can read it.
🪄 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: c928dab0-24a8-4029-a0d0-668eff6ca329

📥 Commits

Reviewing files that changed from the base of the PR and between 69b5312 and c6f7f7e.

📒 Files selected for processing (3)
  • packages/studio/src/lib/dashboard-data.test.ts
  • packages/studio/src/lib/dashboard-data.ts
  • packages/studio/src/lib/runtime-ui/app/admin/page.tsx
✅ Files skipped from review due to trivial changes (1)
  • packages/studio/src/lib/dashboard-data.test.ts

Comment thread packages/studio/src/lib/dashboard-data.ts Outdated
Comment thread packages/studio/src/lib/runtime-ui/app/admin/page.tsx
Comment thread packages/studio/src/lib/runtime-ui/app/admin/page.tsx Outdated
iipanda added 5 commits April 6, 2026 02:09
Add public GET /api/v1/auth/sso/providers endpoint that returns the list
of configured OIDC and SAML providers, with friendly display names.
Adds createLoginApi factory with login() and getSsoProviders() methods,
with injectable fetcher for testing and typed LoginResult discriminated union.
…cking dashboard

The demo user has no RBAC grants for content:read, so the content API
returns 403 for session-based requests. Previously this triggered the
full "Access denied" page even though the user IS authenticated and
the layout loads fine.

Now both schema and content API 403/401 errors degrade to empty results
(zero counts, no types, no recent docs) instead of blocking the entire
dashboard. The "forbidden" state is reserved for truly unexpected auth
failures that escape the permission catch. This matches the expected
behavior: a logged-in user with limited permissions sees a dashboard
with whatever data they're allowed to access.
…ent access

The demo seed created a user and API key but never inserted an RBAC
grant. Session-based requests (cookie auth from Studio embed) went
through RBAC evaluation with no grants, so content:read and schema:read
were denied with 403.

Add ensureDemoRbacGrant to the seed script that gives the demo user
the editor role at project scope for marketing-site. This is idempotent
and skips if a non-revoked grant already exists.

After this fix, re-run compose:dev (or just the db-migrate/seed step)
to apply the grant, then reload /admin.
…seed

Three changes based on feedback:

1. Remove "empty" status — zero documents is a valid loaded state that
   shows the normal dashboard UI with 0 counts and empty lists, not a
   special "No content yet" page. The dashboard always renders cards.

2. Permission errors (401/403) show the "Access denied" state honestly
   instead of silently degrading to empty data that hides the real
   problem from the user.

3. Demo seed RBAC grant changed from editor to viewer — the dashboard
   only needs content:read and schema:read, not write permissions.
@iipanda
Copy link
Copy Markdown
Collaborator Author

iipanda commented Apr 6, 2026

@coderabbitai resume

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 6, 2026

✅ Actions performed

Reviews resumed.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (2)
packages/studio/src/lib/runtime-ui/app/admin/page.tsx (2)

233-242: ⚠️ Potential issue | 🟠 Major

Gate "New Document" CTA on content write capability.

This button renders for all users regardless of permissions. Read-only users (viewers) will see an action they cannot perform. The backend exposes content.write capability that should control document creation visibility.

Consider adding a canCreateContent flag to AdminCapabilitiesValue (extracted from response.capabilities.content.write in layout.tsx) and conditionally rendering this button.

💡 Suggested approach
+import { useAdminCapabilities } from "./capabilities-context.js";

 export default function DashboardPage() {
+  const { canCreateContent } = useAdminCapabilities();
   // ... existing code ...

   {/* Quick Actions */}
   <div className="flex flex-wrap gap-3">
+    {canCreateContent && (
       <Button
         variant="default"
         asChild
         className="bg-accent hover:bg-accent-hover text-white"
       >
         <Link href="/admin/content">
           <Plus className="mr-2 h-4 w-4" />
           New Document
         </Link>
       </Button>
+    )}
   </div>
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/studio/src/lib/runtime-ui/app/admin/page.tsx` around lines 233 -
242, The "New Document" Button in admin/page.tsx is shown to all users; add a
capability check so only users with content write permission see it. Update
AdminCapabilitiesValue (in layout.tsx) to include a canCreateContent boolean
derived from response.capabilities.content.write, pass that capability into the
admin page props/context, and wrap the Button (rendering Link/Plus) in a
conditional that checks canCreateContent before rendering. Ensure the symbol
names to change are AdminCapabilitiesValue, canCreateContent,
response.capabilities.content.write, and the Button/Link/Plus block in
admin/page.tsx.

44-50: ⚠️ Potential issue | 🟡 Minor

Handle future timestamps before the relative-time branches.

Negative diff values (future updatedAt) cause seconds, minutes, hours, and days to all be negative. The condition seconds < 60 at Line 50 will be true for any negative value, incorrectly returning "Just now" even for timestamps far in the future.

🛡️ Proposed fix
   const now = new Date();
   const diff = now.getTime() - date.getTime();
+  if (diff < 0) {
+    return date.toLocaleDateString("en-US", { month: "short", day: "numeric" });
+  }
   const seconds = Math.floor(diff / 1000);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/studio/src/lib/runtime-ui/app/admin/page.tsx` around lines 44 - 50,
The relative-time logic uses diff/seconds/minutes/hours/days and currently
checks seconds < 60 first, which incorrectly returns "Just now" for future
timestamps; add a guard at the top (check if diff < 0 or seconds < 0) in the
same function/block that computes diff/seconds (referencing diff, seconds,
minutes, hours, days) and return an appropriate string for future dates (e.g.,
"In the future" or format as "in X minutes/hours") before any of the existing
relative-time branches so negative values don't fall through to the "Just now"
case.
🧹 Nitpick comments (1)
packages/studio/src/lib/login-api.ts (1)

66-75: Sanitize retryAfterSeconds before returning throttled state.

Current parsing accepts any numeric value from the payload. Clamping to a positive finite integer avoids bad retry behavior from malformed server responses.

Proposed patch
       if (response.status === 429) {
         let retryAfterSeconds = 5;
         try {
           const payload = await response.json();
           if (
             isRecord(payload) &&
             isRecord(payload.details) &&
             typeof payload.details.retryAfterSeconds === "number"
           ) {
-            retryAfterSeconds = payload.details.retryAfterSeconds;
+            const candidate = payload.details.retryAfterSeconds;
+            if (Number.isFinite(candidate) && candidate > 0) {
+              retryAfterSeconds = Math.ceil(candidate);
+            }
           }
         } catch {
           /* use default */
         }
         return { outcome: "throttled", retryAfterSeconds };
       }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/studio/src/lib/login-api.ts` around lines 66 - 75, The code
currently trusts payload.details.retryAfterSeconds directly; change the parsing
so retryAfterSeconds is sanitized to a positive finite integer before use: when
payload.details.retryAfterSeconds is a number, validate with Number.isFinite and
coerce via Math.floor (or equivalent), then clamp to a minimum of 1 (or keep the
existing default 5 if invalid) so malformed or negative/NaN/infinite values
cannot be returned; update the logic around the retryAfterSeconds variable in
this parsing block (referencing retryAfterSeconds,
payload.details.retryAfterSeconds and isRecord) to perform this validation and
fallback to the original default when invalid.
🤖 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/studio/src/lib/login-api.ts`:
- Around line 93-95: The code currently returns payload.data as SsoProvider[]
without validating each element; update the logic where isRecord(payload) is
checked to filter and validate each item at runtime (e.g., create or use an
isSsoProvider(item) type guard) and return only items that pass that guard
instead of an unchecked cast. Ensure you reference payload.data and SsoProvider
in the implementation, use Array.isArray(payload.data) as the initial check,
then payload.data.filter(isSsoProvider) and return the filtered array so callers
receive a bona fide SsoProvider[].

In `@packages/studio/src/lib/runtime-ui/app/admin/login-page.tsx`:
- Around line 93-110: The fetch in handleSsoClick uses redirect: "manual", which
prevents the Location header from being exposed so
response.headers.get("location") always returns null; remove the redirect:
"manual" option so the browser follows the SSO redirect automatically (or
alternatively change the API to return the redirect URL in the JSON body and
read it from response.json()), updating handleSsoClick to either let fetch
perform the redirect or parse the returned URL from the response body and assign
window.location.href accordingly.

---

Duplicate comments:
In `@packages/studio/src/lib/runtime-ui/app/admin/page.tsx`:
- Around line 233-242: The "New Document" Button in admin/page.tsx is shown to
all users; add a capability check so only users with content write permission
see it. Update AdminCapabilitiesValue (in layout.tsx) to include a
canCreateContent boolean derived from response.capabilities.content.write, pass
that capability into the admin page props/context, and wrap the Button
(rendering Link/Plus) in a conditional that checks canCreateContent before
rendering. Ensure the symbol names to change are AdminCapabilitiesValue,
canCreateContent, response.capabilities.content.write, and the Button/Link/Plus
block in admin/page.tsx.
- Around line 44-50: The relative-time logic uses
diff/seconds/minutes/hours/days and currently checks seconds < 60 first, which
incorrectly returns "Just now" for future timestamps; add a guard at the top
(check if diff < 0 or seconds < 0) in the same function/block that computes
diff/seconds (referencing diff, seconds, minutes, hours, days) and return an
appropriate string for future dates (e.g., "In the future" or format as "in X
minutes/hours") before any of the existing relative-time branches so negative
values don't fall through to the "Just now" case.

---

Nitpick comments:
In `@packages/studio/src/lib/login-api.ts`:
- Around line 66-75: The code currently trusts payload.details.retryAfterSeconds
directly; change the parsing so retryAfterSeconds is sanitized to a positive
finite integer before use: when payload.details.retryAfterSeconds is a number,
validate with Number.isFinite and coerce via Math.floor (or equivalent), then
clamp to a minimum of 1 (or keep the existing default 5 if invalid) so malformed
or negative/NaN/infinite values cannot be returned; update the logic around the
retryAfterSeconds variable in this parsing block (referencing retryAfterSeconds,
payload.details.retryAfterSeconds and isRecord) to perform this validation and
fallback to the original default when invalid.
🪄 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: e39ef765-a5dd-4411-9edb-66656a8efe62

📥 Commits

Reviewing files that changed from the base of the PR and between 69b5312 and cfd34df.

📒 Files selected for processing (12)
  • apps/server/src/bin/demo-seed-api-key.ts
  • apps/server/src/lib/auth.ts
  • apps/server/src/lib/collaboration-auth.test.ts
  • packages/studio/src/lib/dashboard-data.test.ts
  • packages/studio/src/lib/dashboard-data.ts
  • packages/studio/src/lib/login-api.test.ts
  • packages/studio/src/lib/login-api.ts
  • packages/studio/src/lib/remote-studio-app.tsx
  • packages/studio/src/lib/runtime-ui/app/admin/layout.tsx
  • packages/studio/src/lib/runtime-ui/app/admin/login-page.tsx
  • packages/studio/src/lib/runtime-ui/app/admin/mount-info-context.tsx
  • packages/studio/src/lib/runtime-ui/app/admin/page.tsx
✅ Files skipped from review due to trivial changes (1)
  • apps/server/src/bin/demo-seed-api-key.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/studio/src/lib/dashboard-data.test.ts
  • packages/studio/src/lib/dashboard-data.ts

Comment thread packages/studio/src/lib/login-api.ts
…rect fix, write-gated CTA

1. getSsoProviders: validate each element with isSsoProvider type guard
   instead of unchecked cast to SsoProvider[]

2. SSO redirect: server now returns redirect URL in JSON body alongside
   the 302 Location header; client reads body.url instead of relying on
   the opaque redirect response's inaccessible Location header

3. "New Document" button gated by canCreateContent capability — viewers
   who lack content:write no longer see the write CTA. Added
   canCreateContent to AdminCapabilitiesValue, wired from
   response.capabilities.content.write in the layout.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (2)
packages/studio/src/lib/runtime-ui/app/admin/capabilities-context.tsx (1)

7-7: Missing useCanCreateContent() convenience hook.

The existing capabilities (canReadSchema, canManageUsers, canManageSettings) each have a corresponding convenience hook (lines 40-49). For consistency, add a useCanCreateContent() hook:

export function useCanCreateContent(): boolean {
  return useAdminCapabilities().canCreateContent;
}

This maintains the established pattern and allows consumers to import the specific hook rather than destructuring from useAdminCapabilities().

Also applies to: 14-14

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/studio/src/lib/runtime-ui/app/admin/capabilities-context.tsx` at
line 7, Add a new convenience hook that returns the canCreateContent capability
to match the existing pattern: implement export function useCanCreateContent() {
return useAdminCapabilities().canCreateContent; } so consumers can call
useCanCreateContent() instead of destructuring useAdminCapabilities(); reference
the existing useAdminCapabilities() hook and the canCreateContent property when
adding this new export.
packages/studio/src/lib/runtime-ui/app/admin/login-page.tsx (1)

74-77: URL construction for success redirect may be fragile.

The expression returnTo.replace("/admin", "") only replaces the first occurrence. If returnTo is /admin/content/admin-docs, this produces /content/admin-docs (correct), but the logic assumes returnTo always starts with /admin. Since useReturnTo() already enforces this, consider using a more explicit approach:

-        window.location.href = basePath
-          ? `${basePath}${returnTo.replace("/admin", "")}`
-          : returnTo;
+        const relativePath = returnTo.startsWith("/admin")
+          ? returnTo.slice("/admin".length) || "/"
+          : returnTo;
+        window.location.href = basePath ? `${basePath}${relativePath}` : returnTo;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/studio/src/lib/runtime-ui/app/admin/login-page.tsx` around lines 74
- 77, The redirect builds the URL using returnTo.replace("/admin", "") which can
accidentally remove interior occurrences; instead explicitly strip only a
leading "/admin" prefix (e.g., check returnTo.startsWith("/admin") and use
returnTo.slice("/admin".length) otherwise leave it) and then concatenate with
basePath (ensuring you don't introduce double slashes). Update the case
"success" block that sets window.location.href to use this explicit
prefix-removal logic (referencing basePath, returnTo, and the case "success"
branch) so the redirect is robust.
🤖 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/studio/src/lib/runtime-ui/app/admin/layout.tsx`:
- Around line 222-233: The current auth gate in useEffect is too aggressive
because it redirects on sessionState.status === "error"; change the redirect
logic so router.replace(`/admin/login?returnTo=${returnTo}`) only runs when
sessionState.status === "unauthenticated" and do not redirect for "error";
instead surface an inline error state when sessionState.status === "error"
(e.g., render an error banner or set an error flag) so authenticated users
aren't sent to the login page on transient failures—update the useEffect
conditional that references sessionState.status, pathname, router and adjust
downstream rendering to handle the "error" state inline rather than calling
router.replace.

In `@packages/studio/src/lib/runtime-ui/app/admin/login-page.tsx`:
- Line 1: Remove the top-level "@ts-nocheck" and fix the TypeScript issues in
this authentication page: explicitly type the LoginPage component (e.g.,
React.FC or appropriate props interface), type the handleSubmit parameter as
React.FormEvent<HTMLFormElement>, add interfaces for credentials and the login
API response (e.g., LoginCredentials, LoginResponse) used by any
fetchLogin/authenticate calls, strongly type local state
(username/password/error) and API results, and replace any "any"/unknown usages
with safe parsing/guards and explicit return types so the file compiles without
disabling type checking.
- Around line 42-61: The useEffect that calls
createLoginApi(...).getSsoProviders() currently omits error handling so
rejections leave ssoLoading true; wrap the async call with proper .catch() or
try/catch (e.g., change the void api.getSsoProviders().then(...) to include
.catch(err => { if (!cancelled) { setSsoLoading(false); setSsoProviders([]); /*
log or report err */ } })) and ensure both success and failure paths clear
setSsoLoading(false) and respect the cancelled flag; reference createLoginApi,
api.getSsoProviders, setSsoProviders, setSsoLoading and the enclosing useEffect
when making the change.
- Around line 93-118: The SSO flow in handleSsoClick currently reads redirect
URL from the JSON body but silently does nothing if redirectUrl is null; update
handleSsoClick to set an error state (call setError) when no redirectUrl is
returned or the response body is not as expected, and also set an error for
non-OK responses or JSON parse failure so the user receives visible feedback
instead of nothing; locate the fetch handler inside handleSsoClick and add
appropriate setError(...) calls in the branch where redirectUrl is null and in
the response error/parse failure branches.

---

Nitpick comments:
In `@packages/studio/src/lib/runtime-ui/app/admin/capabilities-context.tsx`:
- Line 7: Add a new convenience hook that returns the canCreateContent
capability to match the existing pattern: implement export function
useCanCreateContent() { return useAdminCapabilities().canCreateContent; } so
consumers can call useCanCreateContent() instead of destructuring
useAdminCapabilities(); reference the existing useAdminCapabilities() hook and
the canCreateContent property when adding this new export.

In `@packages/studio/src/lib/runtime-ui/app/admin/login-page.tsx`:
- Around line 74-77: The redirect builds the URL using
returnTo.replace("/admin", "") which can accidentally remove interior
occurrences; instead explicitly strip only a leading "/admin" prefix (e.g.,
check returnTo.startsWith("/admin") and use returnTo.slice("/admin".length)
otherwise leave it) and then concatenate with basePath (ensuring you don't
introduce double slashes). Update the case "success" block that sets
window.location.href to use this explicit prefix-removal logic (referencing
basePath, returnTo, and the case "success" branch) so the redirect is robust.
🪄 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: 119d3cff-6d45-41d8-9cf7-f5242e706008

📥 Commits

Reviewing files that changed from the base of the PR and between cfd34df and 9ede07a.

📒 Files selected for processing (8)
  • apps/server/src/lib/auth.ts
  • packages/studio/src/lib/login-api.ts
  • packages/studio/src/lib/remote-studio-app.test.ts
  • packages/studio/src/lib/runtime-ui/app/admin/capabilities-context.tsx
  • packages/studio/src/lib/runtime-ui/app/admin/layout.tsx
  • packages/studio/src/lib/runtime-ui/app/admin/login-page.tsx
  • packages/studio/src/lib/runtime-ui/app/admin/page.tsx
  • packages/studio/src/lib/runtime-ui/app/admin/settings-page.test.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
  • apps/server/src/lib/auth.ts
  • packages/studio/src/lib/runtime-ui/app/admin/page.tsx

Comment thread packages/studio/src/lib/runtime-ui/app/admin/layout.tsx Outdated
@@ -0,0 +1,217 @@
// @ts-nocheck
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Avoid @ts-nocheck on security-sensitive code.

Disabling TypeScript checking on an authentication page is risky—type errors in credential handling, API calls, or redirect logic could lead to security issues or runtime failures. Consider removing this directive and addressing any type errors explicitly.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/studio/src/lib/runtime-ui/app/admin/login-page.tsx` at line 1,
Remove the top-level "@ts-nocheck" and fix the TypeScript issues in this
authentication page: explicitly type the LoginPage component (e.g., React.FC or
appropriate props interface), type the handleSubmit parameter as
React.FormEvent<HTMLFormElement>, add interfaces for credentials and the login
API response (e.g., LoginCredentials, LoginResponse) used by any
fetchLogin/authenticate calls, strongly type local state
(username/password/error) and API results, and replace any "any"/unknown usages
with safe parsing/guards and explicit return types so the file compiles without
disabling type checking.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll address this in follow-up PR

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@iipanda, understood! Would you like me to open a GitHub issue to track the removal of @ts-nocheck and adding proper TypeScript types to login-page.tsx? That way it won't get lost before the follow-up PR.

Comment thread packages/studio/src/lib/runtime-ui/app/admin/login-page.tsx
Comment thread packages/studio/src/lib/runtime-ui/app/admin/login-page.tsx
The dashboard and layout changes introduced new API dependencies that
the studio-review app did not mock:

- GET /api/v1/auth/session — returns a static review session so the
  layout auth gate does not redirect to login
- GET /api/v1/environments — returns a single staging environment for
  the header environment selector
- GET /api/v1/content — content list endpoint with type/published
  filtering, sorting, and pagination for dashboard stat cards and
  recent documents

Also exports listReviewContentDocuments from the review content
infrastructure to support the list route.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/studio-review/app/review-api/`[scenario]/api/v1/content/route.ts:
- Around line 13-19: The parsing logic for limit and offset treats 0 as falsy
due to the use of "||", so a request with limit=0 gets coerced to the default 20
before clamping; update the parsing to explicitly detect invalid parses (NaN)
instead of using "||": read the raw string from url.searchParams.get("limit") /
"offset", call parseInt(..., 10), check Number.isNaN(parsed) (or isNaN(parsed))
and only then substitute the default (20 for limit, 0 for offset), then apply
Math.max/Math.min to clamp; change the code around the limit and offset
calculations (the variables limit and offset) accordingly so that parsed 0 is
preserved and clamped correctly.
🪄 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: a0cec114-4871-4f74-ae27-474a46f82d86

📥 Commits

Reviewing files that changed from the base of the PR and between 9ede07a and e9572a9.

📒 Files selected for processing (4)
  • apps/studio-review/app/review-api/[scenario]/api/v1/auth/session/route.ts
  • apps/studio-review/app/review-api/[scenario]/api/v1/content/route.ts
  • apps/studio-review/app/review-api/[scenario]/api/v1/environments/route.ts
  • apps/studio-review/review/content-documents.ts

… capabilities

- Layout auth gate no longer redirects on session error — only on
  unauthenticated. Transient session failures show an inline error
  instead of kicking authenticated users to the login page.
- SSO providers useEffect now has .catch() so a rejection clears
  ssoLoading instead of leaving it stuck true.
- SSO redirect shows error when redirectUrl is null instead of
  silently doing nothing.
- Add useCanCreateContent convenience hook to match the pattern of
  useCanReadSchema, useCanManageUsers, useCanManageSettings.
- Replace naive returnTo.replace("/admin", "") with prefix-safe
  stripAdminPrefix() that only strips a leading /admin prefix.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/studio/src/lib/runtime-ui/app/admin/layout.tsx (1)

1-1: ⚠️ Potential issue | 🟠 Major

Avoid @ts-nocheck in the admin layout/auth gate.

Line 1 disables compile-time checks in a security-sensitive integration point (session + capabilities + redirect behavior). Please re-enable type checking and address surfaced errors.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/studio/src/lib/runtime-ui/app/admin/layout.tsx` at line 1, Remove
the top-line "// `@ts-nocheck`" and re-enable type checking, then fix the type
errors in the admin layout/auth gate: add explicit types to the exported layout
component (e.g., AdminLayout or the default export) and its props, type any
async auth helper functions (e.g., getServerSession or verifySession) to return
Promise<Session | null> using Session from "next-auth" (or your auth lib),
import and use the correct redirect/response types from "next/navigation" (or
NextResponse) for redirecting, and tighten any capability/user role types (e.g.,
UserRole, Capability) used in gate checks so the conditional logic compiles; run
tsc and address each surfaced error by adding proper imports and explicit return
types rather than using any or suppressing checks.
♻️ Duplicate comments (2)
packages/studio/src/lib/runtime-ui/app/admin/login-page.tsx (2)

117-127: ⚠️ Potential issue | 🟠 Major

Do not redirect SSO on non-OK responses.

Line 117 currently redirects based on body.url even when the response status is not successful. Gate redirect behavior behind response.ok first.

Proposed fix
     })
       .then(async (response) => {
+        if (!response.ok) {
+          setError("SSO sign-in failed. Please try again.");
+          return;
+        }
         const body = await response.json().catch(() => undefined);
         const redirectUrl =
           body && typeof body === "object" && typeof body.url === "string"
             ? body.url
             : null;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/studio/src/lib/runtime-ui/app/admin/login-page.tsx` around lines 117
- 127, The redirect logic in the .then(async (response) => { ... }) block should
be gated by response.ok; update the handler in login-page.tsx so you only
parse/redirect when response.ok is true and body.url is a string (use
response.ok && body && typeof body.url === "string"), otherwise call setError
(the existing setError) with a failure message (e.g., SSO provider returned an
error or no redirect) and do not set window.location.href; ensure you still
await response.json() only when needed or handle a failed json parse without
redirecting.

1-1: ⚠️ Potential issue | 🟠 Major

Remove @ts-nocheck from the login surface.

Line 1 disables type safety across credential and redirect logic on an auth page. This should be removed and remaining type errors fixed explicitly.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/studio/src/lib/runtime-ui/app/admin/login-page.tsx` at line 1,
Remove the top-line "@ts-nocheck" from login-page.tsx and explicitly fix all
resulting type errors: add proper types to the LoginPage component (e.g.,
React.FC or explicit props), strongly type the credentials state/object
(username/email, password), type form handlers such as handleSubmit and onChange
as React.FormEvent/ChangeEvent, and ensure redirect/redirectUrl handling is
typed as string | undefined (or URL) with safe null checks before use; also
annotate any async auth calls/response values so their shapes are known. Target
symbols: LoginPage, credentials state, handleSubmit/onChange, and any
redirect/redirectUrl or auth response variables; update imports/types as needed
to eliminate remaining TypeScript errors.
🤖 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/studio/src/lib/runtime-ui/app/admin/layout.tsx`:
- Around line 234-240: The loading branch currently checks both
sessionState.status === "loading" and typeof window !== "undefined", which
prevents rendering the loading UI in non-browser render paths; remove the typeof
window !== "undefined" guard so that the conditional is simply if
(sessionState.status === "loading") and always returns the loading JSX (the divs
with "Loading..."), keeping the existing markup and classes intact.

In `@packages/studio/src/lib/runtime-ui/app/admin/login-page.tsx`:
- Around line 19-24: The current helpers use startsWith("/admin") which
incorrectly treats paths like "/administrator" as admin; update both helpers
(the returnTo check and stripAdminPrefix) to use a boundary-safe condition:
treat a path as admin only if path === "/admin" || path.startsWith("/admin/").
For stripAdminPrefix, when path === "/admin" return "/" (or another canonical
root) instead of an empty string, otherwise return path.slice("/admin".length)
so "/admin/foo" becomes "/foo"; apply the same boundary check where deciding
whether to return the original or the admin default.

---

Outside diff comments:
In `@packages/studio/src/lib/runtime-ui/app/admin/layout.tsx`:
- Line 1: Remove the top-line "// `@ts-nocheck`" and re-enable type checking, then
fix the type errors in the admin layout/auth gate: add explicit types to the
exported layout component (e.g., AdminLayout or the default export) and its
props, type any async auth helper functions (e.g., getServerSession or
verifySession) to return Promise<Session | null> using Session from "next-auth"
(or your auth lib), import and use the correct redirect/response types from
"next/navigation" (or NextResponse) for redirecting, and tighten any
capability/user role types (e.g., UserRole, Capability) used in gate checks so
the conditional logic compiles; run tsc and address each surfaced error by
adding proper imports and explicit return types rather than using any or
suppressing checks.

---

Duplicate comments:
In `@packages/studio/src/lib/runtime-ui/app/admin/login-page.tsx`:
- Around line 117-127: The redirect logic in the .then(async (response) => { ...
}) block should be gated by response.ok; update the handler in login-page.tsx so
you only parse/redirect when response.ok is true and body.url is a string (use
response.ok && body && typeof body.url === "string"), otherwise call setError
(the existing setError) with a failure message (e.g., SSO provider returned an
error or no redirect) and do not set window.location.href; ensure you still
await response.json() only when needed or handle a failed json parse without
redirecting.
- Line 1: Remove the top-line "@ts-nocheck" from login-page.tsx and explicitly
fix all resulting type errors: add proper types to the LoginPage component
(e.g., React.FC or explicit props), strongly type the credentials state/object
(username/email, password), type form handlers such as handleSubmit and onChange
as React.FormEvent/ChangeEvent, and ensure redirect/redirectUrl handling is
typed as string | undefined (or URL) with safe null checks before use; also
annotate any async auth calls/response values so their shapes are known. Target
symbols: LoginPage, credentials state, handleSubmit/onChange, and any
redirect/redirectUrl or auth response variables; update imports/types as needed
to eliminate remaining TypeScript errors.
🪄 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: 7b065472-d27f-4691-b4e0-00e329338d3c

📥 Commits

Reviewing files that changed from the base of the PR and between e9572a9 and 19e073f.

📒 Files selected for processing (3)
  • packages/studio/src/lib/runtime-ui/app/admin/capabilities-context.tsx
  • packages/studio/src/lib/runtime-ui/app/admin/layout.tsx
  • packages/studio/src/lib/runtime-ui/app/admin/login-page.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • packages/studio/src/lib/runtime-ui/app/admin/capabilities-context.tsx

Comment on lines +234 to +240
if (sessionState.status === "loading" && typeof window !== "undefined") {
return (
<div className="flex min-h-screen items-center justify-center bg-background">
<div className="text-foreground-muted text-sm">Loading...</div>
</div>
);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Remove the window guard from the loading state branch.

Line 234 should render the loading state whenever session status is "loading". The extra typeof window condition can bypass the guard in non-browser render paths.

Proposed fix
-  if (sessionState.status === "loading" && typeof window !== "undefined") {
+  if (sessionState.status === "loading") {
     return (
       <div className="flex min-h-screen items-center justify-center bg-background">
         <div className="text-foreground-muted text-sm">Loading...</div>
       </div>
     );
   }
📝 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
if (sessionState.status === "loading" && typeof window !== "undefined") {
return (
<div className="flex min-h-screen items-center justify-center bg-background">
<div className="text-foreground-muted text-sm">Loading...</div>
</div>
);
}
if (sessionState.status === "loading") {
return (
<div className="flex min-h-screen items-center justify-center bg-background">
<div className="text-foreground-muted text-sm">Loading...</div>
</div>
);
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/studio/src/lib/runtime-ui/app/admin/layout.tsx` around lines 234 -
240, The loading branch currently checks both sessionState.status === "loading"
and typeof window !== "undefined", which prevents rendering the loading UI in
non-browser render paths; remove the typeof window !== "undefined" guard so that
the conditional is simply if (sessionState.status === "loading") and always
returns the loading JSX (the divs with "Loading..."), keeping the existing
markup and classes intact.

Comment on lines +19 to +24
return returnTo.startsWith("/admin") ? returnTo : "/admin";
}

function stripAdminPrefix(path: string): string {
return path.startsWith("/admin") ? path.slice("/admin".length) : path;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Harden /admin path boundary checks for returnTo.

The current startsWith("/admin") logic treats paths like /administrator as valid admin paths and stripAdminPrefix then corrupts the target. Use a boundary-safe check (/admin or /admin/...) in both helpers.

Proposed fix
+function isAdminPath(path: string): boolean {
+  return path === "/admin" || path.startsWith("/admin/");
+}
+
 function useReturnTo(): string {
   if (typeof window === "undefined") return "/admin";
@@
-  return returnTo.startsWith("/admin") ? returnTo : "/admin";
+  return isAdminPath(returnTo) ? returnTo : "/admin";
 }
 
 function stripAdminPrefix(path: string): string {
-  return path.startsWith("/admin") ? path.slice("/admin".length) : path;
+  return isAdminPath(path) ? path.slice("/admin".length) : path;
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/studio/src/lib/runtime-ui/app/admin/login-page.tsx` around lines 19
- 24, The current helpers use startsWith("/admin") which incorrectly treats
paths like "/administrator" as admin; update both helpers (the returnTo check
and stripAdminPrefix) to use a boundary-safe condition: treat a path as admin
only if path === "/admin" || path.startsWith("/admin/"). For stripAdminPrefix,
when path === "/admin" return "/" (or another canonical root) instead of an
empty string, otherwise return path.slice("/admin".length) so "/admin/foo"
becomes "/foo"; apply the same boundary check where deciding whether to return
the original or the admin default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant