[cueweb/docs/sandbox/images] Complete Professional UI/UX Foundations milestone: CueGUI parity, monitoring UX overhaul, responsive/mobile support, major CueWeb usability improvements, and introduce optional Role-Based Access Control (RBAC) / Admin platform

[ramonfigueiredo]

Related Issues

Main issue:

• [cueweb] Finalize CueWeb with Full CueGUI Functionality and Professional UI #2016

Issues related to this PR:

• [cueweb] Professional UI/UX Foundations: Add a global application header with navigation bar and user menu #2356
• [cueweb] Professional UI/UX Foundations: Add a collapsible sidebar with primary navigation #2357
• [cueweb] Professional UI/UX Foundations: Breadcrumb navigation with context path #2358
• [cueweb] Professional UI/UX Foundations: Status bar with system metrics #2359
• [cueweb] Professional UI/UX Foundations: Professional empty-state components #2360
• [cueweb] Professional UI/UX Foundations: Loading skeletons for tables and detail panes #2361
• [cueweb] Professional UI/UX Foundations: Modal dialog redesign with shadcn Dialog modern styles #2362
• [cueweb] Professional UI/UX Foundations: Tabbed interface for multi-view detail pages #2363
• [cueweb] Professional UI/UX Foundations: Dashboard landing page with widget grid #2364
• [cueweb] Professional UI/UX Foundations: Toolbar redesign with grouped action buttons and icon support #2365
• [cueweb] Professional UI/UX Foundations: Keyboard shortcuts overlay (? to toggle) #2366
• [cueweb] Professional UI/UX Foundations: Toast notification system for action feedback #2367
• [cueweb] Add CueGUI parity improvements for tables, context menus, keyboard shortcuts, and load-test tooling #2368

Summarize your change.

Professional UI/UX Foundations for CueWeb are now fully implemented, establishing a complete application redesign, reusable UI system, responsive/mobile support, and a CueGUI-aligned monitoring experience across Jobs, Layers, and Frames.

Application layout, navigation, and dashboard

• Add persistent global AppHeader with CueGUI-style navigation, theme toggle, authenticated user session menu, and responsive mobile hamburger entrypoint
• Add collapsible AppSidebar with grouped navigation, persistent state, active route highlighting, and responsive behavior
• Add mobile navigation drawer using Radix Sheet, mirroring all sidebar groups with scrollable navigation and sticky title bar
• Restructure application layout so sidebar owns the full left column and eliminates header overlap
• Add breadcrumb navigation for all detail views with truncation and hover tooltips
• Add fixed IDE-style bottom status bar showing gateway health, last refresh state, and build version
• Add CueWeb Dashboard with dynamic charts, widget-based layout, and independently loading stat/chart components. Dashboard with information about "Number of Active Jobs by State (Running, Paused, ...)", "Number of Hosts (up and down)", "Number of Recent Failures", and "Number of Show" and the following charts: 1) Frame state breakdown, 2) Top jobs by running frames, 3) Hosts by state, 4) Jobs per show, and 5) Job age distribution.
• Default NEXT_PUBLIC_URL to same-origin relative paths, enabling LAN access and reverse-proxy compatibility without rebuild

Core UI system and reusable primitives

• Add EmptyState component and replace all placeholder empty views across jobs, search, layers, and logs
• Add Skeleton loading primitives and apply consistent shimmer loading states across tables and detail panes
• Standardize all modal dialogs on a unified header/body/footer layout with consistent spacing and accessibility behavior
• Add global themed toast system using react-toastify and remove remaining alert-based UI feedback
• Improve dark mode consistency, switch visibility, and overall accessibility across controls and overlays
• Introduce reusable widget/card architecture with isolated loading, error, and refresh behaviour

Monitoring and CueGUI parity

• Introduce tabbed job detail page with Overview, Layers, Frames, Comments, and Dependencies, with URL-synced tab state
• Extend Jobs, Layers, and Frames tables with CueGUI-aligned columns (state, timing, memory, eligibility, progress)
• Add client-side column visibility toggles, reorderable columns, and reset-to-default behavior
• Add substring filtering for all monitor tables with automatic pagination reset
• Restore inline layer pagination and remove legacy popup-only patterns
• Add inline Job Details panel with synchronized Layers and Frames views
• Improve Attributes panel synchronization with selected entities
• Wrap Jobs, Layers, and Frames tables in overflow-x-auto for horizontal swipeability on mobile devices
• Remove redundant inline layer-name links from Frames table for cleaner navigation consistency

Context menus, actions, and productivity enhancements

• Rewrite Job, Layer, and Frame context menus to match CueGUI action structure, including expanded action sets, separators, clipboard utilities, and improved accessibility
• Add per-row "⋮" Actions button as the leftmost cell in Jobs, Layers, and Frames, exposing full context menus for touch/mobile users
• Wire Frame "View Log" and "Tail Log" menu items to the same log viewer used by row double-click navigation, with validation guards for unstarted frames
• Add Copy Layer Name, Copy Frame Name, and Copy Log Path actions
• Implement shared clipboard helper with navigator.clipboard support and fallback for insecure contexts and iOS Safari quirks
• Add external log editor integration via NEXT_PUBLIC_LOG_EDITOR_URL (supports vscode://, vscode-insiders://, subl://, txmt://, idea://)
• Detect missing external editor handlers via blur/visibility heuristics and surface warning toasts

Toolbar and keyboard shortcuts

• Refactor toolbar into grouped action sections (monitor, filter, refresh) with icons and visual separators
• Add responsive overflow handling for smaller viewports using dropdown fallback
• Introduce global keyboard shortcut system:
  • ? opens shortcuts overlay
  • / focuses search
  • r refreshes monitored data
  • t toggles theme
• Add clickable shortcuts overlay with shared keyboard/action logic and mobile-friendly responsive sizing

Notifications and subscription system

• Improve job subscription bell to function without requiring desktop notification permission
• Always emit in-app completion notifications
• Enable desktop notifications only when permission is granted
• Handle runtime permission changes without reload
• Add cross-tab synchronization for subscriptions and notification state

Load testing and developer tooling (OpenCue/sandbox/load_test_jobs.py)

• Replace simple load test script with a structured CLI supporting multiple job generation strategies (simple, wide, deep, chain, fan-out, fan-in, diamond, mixed)
• Add batching, pause/resume behaviour, dry-run mode, uniqueness guarantees, and dependency graph generation
• Improve robustness of job lookup and failure handling in stress scenarios

CueWeb status indicators

• Gateway status shows real-time health (/api/health polling, response latency, online/offline state)
• Last refresh shows continuously updated relative timing for Jobs table refresh cycles

Documentation and integration

• Refresh CueWeb documentation (README and all CueWeb docs) to reflect the new application layout, dashboard, responsive/mobile behaviour, row actions, horizontal table scrolling, keyboard shortcuts, status bar, clipboard fallback, external editor integration, and LAN access improvements
• Document all new UI primitives and interaction patterns for future extension
• Merge upstream changes and resolve integration conflicts across UI and monitoring layers

[cueweb] Add RBAC + Admin UI with Bootstrap Admin and pluggable groups resolvers

• Local username/password provider with argon2id, rate-limited /login, and a first-launch bootstrap admin (24-char random password, written to /data/.cueweb-bootstrap mode 0600, printed once to the container log, forced rotation on first sign-in via /login/change-password).
• SQLite policy store at /data/cueweb-rbac.db (better-sqlite3, WAL + foreign keys, forward-only migrations bundled as TS so Next.js ships them). Tables: users, groups, user_groups, roles, role_permissions, group_roles, user_roles, admins, audit_log.
• Built-in roles seeded on every start: site-admin (*), operator, viewer. Custom roles supported. Permission catalog covers jobs, layers, frames, hosts, shows, and the CueCommander menu.
• Pluggable groups resolvers selected by CUEWEB_GROUPS_RESOLVER: okta (groups claim from ID token), ldap (memberOf via ldapjs), none (open-source default). Direct user_roles survive a resync. Google + GitHub sign-ins land in the users table as imported so admins can attach roles via the UI even without external groups.
• Enforcement at three layers: edge middleware for /admin/* and /api/admin/*, requireFeature() / requireAdmin() route helpers, and useFeature() / useIsAdmin() / client hooks. CueCommander menu and the new Admin header link gate accordingly.
• Admin UI at /admin with tabs for Overview, Users, Groups, Roles, Permissions, Admins, and an Audit log with CSV export. Every mutation through /api/admin/* writes a before/after audit entry.
• Self-service /api/me/password endpoint (not under /api/admin/* so the admin-only middleware does not block it) drives the forced password-change page.
• Sandbox mode preserved: empty NEXT_PUBLIC_AUTH_PROVIDER short-circuits middleware and hooks to "allow", CueWeb behaves like before this layer existed, and no providers are registered.
• Login + change-password pages get a theme toggle (sun/moon) matching the global header pattern.
• Dockerfile installs python+g++ as a virtual .build-deps group for the native modules (better-sqlite3, argon2), declares /data as a volume, and exposes CUEWEB_GROUPS_RESOLVER. docker-compose adds a cueweb-data named volume.
• Tests cover migrations + seeding, bootstrap admin (forced pw change + 0600 perms), Okta resolver (group sync preserves direct grants), and rate-limit window behavior.
• Documentation: README RBAC section + all CueWeb docs under docs/ (reference, user guide, other guide, quick start, tutorial, concepts, developer guide, deployment) updated with the new providers, env vars, bootstrap flow, Admin UI tour, Kubernetes PVC, and bootstrap-password recovery.

Summary by CodeRabbit

• New Features
  • Overhauled CueWeb UI: persistent theme-aware header, collapsible persisted sidebar, dockable Attributes panel, inline job details, and fixed bottom status bar with gateway health and last-refresh timestamps.
  • Expanded auth & RBAC: local/Okta/LDAP/Google/GitHub support, admin UI for users/roles/groups, and role-based access controls.
  • Notifications & shortcuts: job-completion toast/optional desktop notifications, keyboard shortcuts overlay with persisted notify toggle.
  • Dashboard: new widgets and charts (jobs, hosts, frame states, top jobs, job age).

[coderabbitai]

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (2)

cueweb/components/ui/search-dropdown.tsx (2)

110-110: ⚡ Quick win

Use a stable key instead of index.

Using index as the key in a virtualized list that changes (e.g., when filtered) is an anti-pattern. React may reuse components incorrectly during reconciliation. Use a stable identifier like job.name for more predictable rendering.

🔑 Proposed fix

           <div
-              key={index}
+              key={job.name}
               role="button"
               tabIndex={0}

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cueweb/components/ui/search-dropdown.tsx` at line 110, Replace the unstable
key={index} used in the virtualized list render inside the SearchDropdown
component with a stable identifier from the item (e.g., use job.name or another
unique field on the job object). Update the element that currently sets
key={index} to instead set key={job.name} (or key={job.id} if available) so
React can reconcile filtered/changed lists correctly; ensure the referenced
symbol is the same item variable used in the map/rowRenderer where key is
assigned.

---

104-106: ⚡ Quick win

Optimize the added-job lookup with a memoized Set.

The tableData.some() call runs a linear scan for every rendered item. With a large tableData, this can cause performance issues even in a virtualized list. Create a Set of added job names outside the item renderer for O(1) lookups.

⚡ Proposed optimization

Add this useMemo hook after line 46 (after listRef declaration):

 const listRef = React.useRef<FixedSizeList | null>(null);
+
+const addedJobNames = React.useMemo(
+  () => new Set(tableData.map((job) => job.name)),
+  [tableData],
+);

Then replace the isJobAdded calculation:

-          const isJobAdded = tableData.some(
-            (existingJob: Job) => existingJob.name === job.name,
-          );
+          const isJobAdded = addedJobNames.has(job.name);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cueweb/components/ui/search-dropdown.tsx` around lines 104 - 106, Create a
memoized Set of added job names to avoid repeated linear scans: use React's
useMemo to build a Set from tableData (e.g., new Set(tableData.map(j =>
j.name))) declared near listRef (after its declaration) and then replace the
per-item some() call that computes isJobAdded with a constant-time lookup using
set.has(job.name). Update any references to the old variable name (isJobAdded)
to use the new lookup and ensure the memo depends on tableData so it rebuilds
when the list changes.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cueweb/components/ui/app-header.tsx`:
- Around line 153-158: The input's onKeyDown currently calls e.stopPropagation()
for every key, which prevents dropdown keyboard handling; update the onKeyDown
handler (the input with value={query} and setQuery) to conditionally stop
propagation only for keys that should not affect the parent menu (e.g., regular
character keys and Enter) and allow navigation/escape keys (Escape, ArrowUp,
ArrowDown, ArrowLeft, ArrowRight, Tab if desired) to bubble so the menu's
keyboard controls continue to work.

In `@cueweb/components/ui/app-sidebar.tsx`:
- Around line 364-376: The facility list uses custom role="radiogroup" and
role="radio" on buttons but lacks full radio keyboard semantics; update the
facilities.map rendering (the button that calls setFacility) to implement
complete radio behavior: either render native <input type="radio"> elements for
each facility bound to the facility state (or wrap a visually styled label
around such an input), or add the missing keyboard handlers on the button
(manage tabIndex, onKeyDown to handle
ArrowLeft/ArrowRight/ArrowUp/ArrowDown/Home/End to move focus and call
setFacility, ensure aria-checked is provided and focus is placed correctly).
Make these changes in the component that contains facilities.map and setFacility
so keyboard navigation and screen-reader semantics match an actual radio group.

In `@cueweb/components/ui/attributes-panel.tsx`:
- Around line 110-123: The toggle button in attributes-panel (the button using
setOpen and reading open to render ChevronDown/ChevronRight and label) is
missing an accessibility state; add aria-expanded={open} to that button so
assistive technologies announce the open/closed state of the attribute tree node
(keep the existing onClick, style, and className behavior and place
aria-expanded alongside them).

---

Nitpick comments:
In `@cueweb/components/ui/search-dropdown.tsx`:
- Line 110: Replace the unstable key={index} used in the virtualized list render
inside the SearchDropdown component with a stable identifier from the item
(e.g., use job.name or another unique field on the job object). Update the
element that currently sets key={index} to instead set key={job.name} (or
key={job.id} if available) so React can reconcile filtered/changed lists
correctly; ensure the referenced symbol is the same item variable used in the
map/rowRenderer where key is assigned.
- Around line 104-106: Create a memoized Set of added job names to avoid
repeated linear scans: use React's useMemo to build a Set from tableData (e.g.,
new Set(tableData.map(j => j.name))) declared near listRef (after its
declaration) and then replace the per-item some() call that computes isJobAdded
with a constant-time lookup using set.has(job.name). Update any references to
the old variable name (isJobAdded) to use the new lookup and ensure the memo
depends on tableData so it rebuilds when the list changes.

🪄 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: 1a22e861-2b8f-436c-b294-ceea1a5ce6d9

📥 Commits

Reviewing files that changed from the base of the PR and between bb6bd61 and 69d8092.

⛔ Files ignored due to path filters (43)

• cueweb/app/favicon.ico is excluded by !**/*.ico
• cueweb/app/icon.png is excluded by !**/*.png
• cueweb/app/icondark.png is excluded by !**/*.png
• cueweb/app/iconlight.png is excluded by !**/*.png
• cueweb/interface_screenshots/column _visibility_dropdown.png is excluded by !**/*.png
• cueweb/interface_screenshots/column _visibility_dropdown_dark.png is excluded by !**/*.png
• cueweb/interface_screenshots/frame.png is excluded by !**/*.png
• cueweb/interface_screenshots/frame_context_menu_open.png is excluded by !**/*.png
• cueweb/interface_screenshots/frame_context_menu_open_and_success_notification.png is excluded by !**/*.png
• cueweb/interface_screenshots/frame_context_menu_open_dark.png is excluded by !**/*.png
• cueweb/interface_screenshots/frame_dark.png is excluded by !**/*.png
• cueweb/interface_screenshots/job_context_menu_open.png is excluded by !**/*.png
• cueweb/interface_screenshots/job_context_menu_open_and_success_notification.png is excluded by !**/*.png
• cueweb/interface_screenshots/job_context_menu_open_dark.png is excluded by !**/*.png
• cueweb/interface_screenshots/job_data_table_filtering.png is excluded by !**/*.png
• cueweb/interface_screenshots/job_data_table_filtering_dark.png is excluded by !**/*.png
• cueweb/interface_screenshots/job_search_functionality.png is excluded by !**/*.png
• cueweb/interface_screenshots/job_search_functionality_dark.png is excluded by !**/*.png
• cueweb/interface_screenshots/layer_context_menu_open.png is excluded by !**/*.png
• cueweb/interface_screenshots/layer_context_menu_open_and_success_notification.png is excluded by !**/*.png
• cueweb/interface_screenshots/layer_context_menu_open_dark.png is excluded by !**/*.png
• cueweb/interface_screenshots/layersframes.png is excluded by !**/*.png
• cueweb/interface_screenshots/layersframes_dark.png is excluded by !**/*.png
• cueweb/interface_screenshots/login.png is excluded by !**/*.png
• cueweb/interface_screenshots/mainpage.png is excluded by !**/*.png
• cueweb/interface_screenshots/mainpage_dark.png is excluded by !**/*.png
• cueweb/interface_screenshots/un-monitoring_selected_jobs-after.png is excluded by !**/*.png
• cueweb/interface_screenshots/un-monitoring_selected_jobs-before.png is excluded by !**/*.png
• cueweb/public/opencue-icon-black.png is excluded by !**/*.png
• cueweb/public/opencue-icon-white.png is excluded by !**/*.png
• cueweb/public/opencue_logo_small.png is excluded by !**/*.png
• images/opencue-horizontal-black.png is excluded by !**/*.png
• images/opencue-horizontal-black.svg is excluded by !**/*.svg
• images/opencue-horizontal-white.png is excluded by !**/*.png
• images/opencue-horizontal-white.svg is excluded by !**/*.svg
• images/opencue-icon-black.png is excluded by !**/*.png
• images/opencue-icon-black.svg is excluded by !**/*.svg
• images/opencue-icon-white.png is excluded by !**/*.png
• images/opencue-icon-white.svg is excluded by !**/*.svg
• images/opencue-stacked-black.png is excluded by !**/*.png
• images/opencue-stacked-black.svg is excluded by !**/*.svg
• images/opencue-stacked-white.png is excluded by !**/*.png
• images/opencue-stacked-white.svg is excluded by !**/*.svg

📒 Files selected for processing (28)

• cueweb/README.md
• cueweb/app/frames/[frame-name]/page.tsx
• cueweb/app/jobs/[job-name]/comments/page.tsx
• cueweb/app/jobs/data-table.tsx
• cueweb/app/layout.tsx
• cueweb/app/providers/session-provider.tsx
• cueweb/app/utils/help_menu.ts
• cueweb/app/utils/menus.ts
• cueweb/app/utils/use_attribute_selection.ts
• cueweb/app/utils/use_attributes_panel.ts
• cueweb/app/utils/use_cuebot_facility.ts
• cueweb/app/utils/use_disable_job_interaction.ts
• cueweb/app/utils/use_menu_registry.ts
• cueweb/components/ui/app-header.tsx
• cueweb/components/ui/app-sidebar.tsx
• cueweb/components/ui/attributes-panel.tsx
• cueweb/components/ui/context_menus/action-context-menu.tsx
• cueweb/components/ui/cuewebicon.tsx
• cueweb/components/ui/read-only-banner.tsx
• cueweb/components/ui/search-dropdown.tsx
• docs/_docs/concepts/cueweb-rest-gateway.md
• docs/_docs/developer-guide/cueweb-development.md
• docs/_docs/getting-started/deploying-cueweb.md
• docs/_docs/other-guides/cueweb.md
• docs/_docs/quick-starts/quick-start-cueweb.md
• docs/_docs/reference/cueweb.md
• docs/_docs/tutorials/cueweb-tutorial.md
• docs/_docs/user-guides/cueweb-user-guide.md

💤 Files with no reviewable changes (2)

• cueweb/app/frames/[frame-name]/page.tsx
• cueweb/app/jobs/[job-name]/comments/page.tsx

[coderabbitai]

Actionable comments posted: 20

Caution

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

⚠️ Outside diff range comments (2)

docs/_docs/quick-starts/quick-start-cueweb.md (1)

132-133: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update the example Next.js version in startup output.

The quick-start snippet still shows Next.js 14.2.32, but this PR/repo is on Next.js 15.x. Keeping this current avoids confusion during setup validation.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/_docs/quick-starts/quick-start-cueweb.md` around lines 132 - 133, The
example startup output shows "Next.js 14.2.32" which is outdated; update the
snippet in quick-start-cueweb.md to reflect the current major release (replace
"Next.js 14.2.32" with "Next.js 15.x" or the exact 15.y.z used by this repo) so
the startup display matches the project's Next.js version.

cueweb/app/frames/[frame-name]/page.tsx (1)

102-113: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Reset log counters when the selected log is missing or empty.

When getLogLineCount() returns -1 or 0, totalNumLogLines keeps the previous value, so the header can show stale line counts for the new version.

Proposed fix

   const fetchInitialLogs = async () => {
     try {
       const totalLines = await getLogLineCount();
       if (totalLines == -1) {
         setLogStatus("missing");
+        setTotalNumLogLines(-1);
+        setNumberOfLinesLoaded(0);
         return;
       }
       if (totalLines === 0) {
         setLogStatus("empty");
+        setTotalNumLogLines(0);
         setNumberOfLinesLoaded(0);
         return;
       }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cueweb/app/frames/`[frame-name]/page.tsx around lines 102 - 113, When
getLogLineCount() returns -1 or 0 the component currently leaves
totalNumLogLines stale; inside fetchInitialLogs update state to clear counts:
call setTotalNumLogLines(0) in both the missing (-1) and empty (0) branches, and
also ensure setNumberOfLinesLoaded(0) is called for the missing branch (it
already is for empty). This uses the existing functions fetchInitialLogs,
getLogLineCount, setLogStatus, setNumberOfLinesLoaded, and setTotalNumLogLines
to ensure the header no longer shows stale line counts.

🧹 Nitpick comments (3)

cueweb/app/utils/layers_frames_utils.ts (1)

78-83: ⚡ Quick win

Apply consistent edge-case handling to secondsToHHHMM.

The secondsToHHMMSS function now clamps negatives and floors fractional inputs (lines 67-72), but secondsToHHHMM doesn't apply the same pattern. For consistency and to prevent fractional second bleed, consider applying the same Math.max(0, Math.floor(sec)) pattern here.

♻️ Proposed fix for consistency

 export const secondsToHHHMM = (sec: number): string => {
+  const total = Math.max(0, Math.floor(sec));
-  const hours = Math.floor(sec / 3600).toString().padStart(3, "0");
-  const minutes = Math.floor((sec % 3600) / 60).toString().padStart(2, "0");
+  const hours = Math.floor(total / 3600).toString().padStart(3, "0");
+  const minutes = Math.floor((total % 3600) / 60).toString().padStart(2, "0");

   return `${hours}:${minutes}`;
 };

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cueweb/app/utils/layers_frames_utils.ts` around lines 78 - 83, The
secondsToHHHMM function doesn't clamp negative values or floor fractional inputs
like secondsToHHMMSS does; update secondsToHHHMM to first normalize the input
with const total = Math.max(0, Math.floor(sec)) (or equivalent) and then compute
hours and minutes from total, keeping the same padStart formatting, so negative
or fractional seconds are handled consistently; target the secondsToHHHMM
function for this change.

cueweb/components/ui/context_menus/useContextMenu.ts (1)

29-43: ⚡ Quick win

Consider a discriminated union for MenuItem to avoid dummy values.

The current design requires label, onClick, and isActive to be provided even when separator: true, forcing consumers to pass unused dummy values. A discriminated union would make the API clearer and prevent invalid states:

-export interface MenuItem {
-  label: string;
-  onClick(row: Row<any> | null): any;
-  isActive: boolean;
-  component?: React.ReactNode;
-  separator?: boolean;
-}
+export type MenuItem =
+  | {
+      separator?: false;
+      label: string;
+      onClick(row: Row<any> | null): any;
+      isActive: boolean;
+      component?: React.ReactNode;
+    }
+  | {
+      separator: true;
+    };

This eliminates the need for dummy values and makes the intent explicit at compile time.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cueweb/components/ui/context_menus/useContextMenu.ts` around lines 29 - 43,
MenuItem currently requires label/onClick/isActive even when separator is true,
forcing dummy values; change MenuItem into a discriminated union (e.g., a
Separator variant with separator: true and only the fields needed, and an
Action/Item variant with separator?: false and label/onClick/isActive/component)
and update all consumers (hooks, renderers, and types referencing MenuItem such
as useContextMenu, any menu rendering components, and handlers) to handle both
variants by switching on the separator discriminator; this removes the need for
dummy values and enforces correct shapes at compile time.

cueweb/components/ui/job-details-inline.tsx (1)

111-147: ⚡ Quick win

Use a stable dependency for the polling effect.

Line 146 includes job in the dependency array, so object identity churn can reset the interval and trigger extra immediate fetches for the same selected job. Depend on job?.id only.

♻️ Proposed fix

-  React.useEffect(() => {
-    if (!job) return;
+  React.useEffect(() => {
+    if (!job) return;
+    const selectedJob = job;
@@
-        const [nextLayers, nextFrames] = await Promise.all([
-          getLayersForJob(job),
-          getFramesForJob(job),
+        const [nextLayers, nextFrames] = await Promise.all([
+          getLayersForJob(selectedJob),
+          getFramesForJob(selectedJob),
         ]);
@@
-  }, [job?.id, job]);
+  }, [job?.id]);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cueweb/components/ui/job-details-inline.tsx` around lines 111 - 147, The
polling useEffect is subscribed to both job and job?.id causing identity churn
to reset the interval; update the dependency array of the React.useEffect (the
effect that defines load, calls getLayersForJob/getFramesForJob and starts the
setInterval) to only depend on job?.id (remove job) so the interval isn’t
recreated when the job object identity changes.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cueweb/app/api/health/route.ts`:
- Around line 94-123: The AbortController timeout (timer) is only cleared on the
success path; ensure the timer is always cleared by moving clearTimeout(timer)
into a finally block that runs after the try/catch around the fetch; locate the
controller/timer setup (const controller = new AbortController(); const timer =
setTimeout(...)), the fetch call that uses controller.signal, and add a finally
that calls clearTimeout(timer) and optionally controller.abort()/cleanup so the
timeout cannot linger on error/abort paths.

In `@cueweb/app/api/host/gethosts/route.ts`:
- Around line 31-35: The current code calls await request.json() without
guarding parse errors which throws on malformed JSON and surfaces as a 500; wrap
the request.json() call in a try/catch inside the route handler (the code that
declares body/jsonBody) and on any SyntaxError or parsing failure return
NextResponse.json({ error: 'Invalid request body' }, { status: 400 }); replace
the stringify/parse dance (body/jsonBody) with the safely-parsed object from
request.json() and keep the existing typeof/object check on jsonBody before
proceeding.
- Around line 40-42: The handler currently always returns HTTP 200 because both
NextResponse.json calls omit the second `{ status: ... }` option; update the two
calls in route.ts to pass the upstream status so the proxied response codes are
preserved (use `response.status` for the error branch and the successful branch
as appropriate), i.e. when returning the error payload built from `responseData`
pass `{ status: response.status }` and when returning `{ data: hosts }` pass the
upstream status (e.g., `{ status: response.status }`) so the client receives the
correct HTTP status; change the two NextResponse.json invocations that reference
`response`, `responseData`, and `hosts` accordingly.

In `@cueweb/app/api/job/action/dropdepends/route.ts`:
- Around line 34-35: Remove the redundant JSON stringify/parse round-trip in
route.ts: replace the current two-step (const body = JSON.stringify(await
request.json()); const jsonBody = JSON.parse(body);) with a single parse (const
jsonBody = await request.json();) and then only call JSON.stringify(jsonBody) at
the call site when passing data into handleRoute (or keep it as an object if
handleRoute accepts objects); ensure any validation runs against jsonBody before
stringifying.

In `@cueweb/app/api/job/action/setautoeat/route.ts`:
- Around line 27-31: Wrap the request.json() call in a try/catch to handle
malformed JSON and return a 400 using NextResponse.json instead of letting it
throw a 500; specifically, around the code that assigns body/jsonBody (the await
request.json() -> const body = JSON.stringify(await request.json()); const
jsonBody = JSON.parse(body);) catch the error from request.json() or JSON.parse
and return NextResponse.json({ error: 'Invalid JSON payload' }, { status: 400 })
so the route handler (route.ts) treats client-side malformed JSON as a bad
request.
- Around line 35-36: The route currently returns NextResponse.json without
passing an HTTP status, causing all responses to be 200; update the two return
sites that call NextResponse.json (the error branch checking response.ok and the
success branch returning responseData) to pass the appropriate status via the
options object (e.g., use response.status for the upstream fetch error and
responseData.status for the success payload) so the route preserves the correct
HTTP status codes.

In `@cueweb/app/api/job/action/setmaxretries/route.ts`:
- Around line 27-28: Remove the redundant serialize-then-parse round-trip by
calling await request.json() once and using that object for validation and
further processing; where handleRoute expects a string, call JSON.stringify on
that parsed object at the call site instead of creating intermediate variables
like body and jsonBody via JSON.stringify(...) followed by JSON.parse(...).
Update the code in setmaxretries/route.ts to use the parsed payload (from
request.json()) directly for validation and only stringify when passing to
handleRoute.

In `@cueweb/app/api/job/action/setpriority/route.ts`:
- Around line 27-36: Wrap the request body parsing in a try/catch and validate
without serializing/deserializing twice: call await request.json() inside a try
block, validate that the result is an object with job and numeric val, and
return NextResponse.json({ error: 'Invalid request body (...)' }, { status: 400
}) for malformed or invalid JSON; then call handleRoute(method, endpoint, body,
true) as before but when handling the upstream response use response.status for
the HTTP status on the NextResponse (i.e. return NextResponse.json({ error:
responseData.error }, { status: response.status }) for !response.ok and return
NextResponse.json({ data: responseData.data }, { status: response.status }) on
success) so the actual HTTP status is propagated. Reference: body, jsonBody,
handleRoute, response, responseData, NextResponse.json.

In `@cueweb/app/api/show/getshows/route.ts`:
- Line 45: The non-OK branch in route.ts returns NextResponse.json({ error:
responseData.error, status: response.status }) but does not set the HTTP status
code, causing a 200 response; update the error path (the check on response.ok)
to return a NextResponse with the same upstream HTTP status (response.status) —
e.g., use NextResponse.json(..., { status: response.status }) and propagate any
relevant headers from the upstream response if needed so clients receive the
correct error status instead of 200.

In `@cueweb/app/jobs/`[job-name]/page.tsx:
- Around line 145-177: The effect that loads tab data uses a one-time firstLoad
computed outside the async runFetch and a module-level loadedTabs ref that is
never reset when job changes, causing flicker on polling and stale cache across
job switches; fix by resetting loadedTabs.current (and clearing related state
like layers/frames/comments and loading flags) whenever the job id changes, and
compute firstLoad inside runFetch (or read loadedTabs.current at the moment of
fetch) so each poll uses the current cached state; update the useEffect
containing runFetch/loadedTabs/tab/job to clear per-job cache on job change and
to compute firstLoad dynamically before setting per-tab loading states.

In `@cueweb/app/jobs/data-table.tsx`:
- Around line 356-361: The effect that calls addUsersJobs() uses
state.loadFinished inside addUsersJobs but doesn't include it in its dependency
array, causing stale behavior; update all useEffect hooks that invoke
addUsersJobs (the one with dependencies [state.username, state.autoloadMine] and
the other effect at the second call) to include state.loadFinished in their
dependency arrays (e.g., [state.username, state.autoloadMine,
state.loadFinished]) so addUsersJobs runs when loadFinished changes and remove
any stale assumptions while preserving existing setInitialLoading behavior.

In `@cueweb/app/utils/action_utils.ts`:
- Around line 417-435: The prompt handlers setPriorityGivenRow and
setMaxRetriesGivenRow currently use Number.parseInt which accepts partial inputs
like "10abc"; replace that loose parse with strict whole-string validation:
first test raw with a regex that matches an optional leading minus and digits
for priority (/^\s*-?\d+\s*$/) and a non-negative integer for retries
(/^\s*\d+\s*$/), show the existing toastWarning on failure, then call
Number.parseInt only after the regex passes; keep the existing early-return for
raw === null and then invoke setJobPriority(job, value) and the corresponding
update for retries (e.g., setJobMaxRetries or whatever setter is used) with the
parsed integer.

In `@cueweb/components/dashboard/jobs-per-show-chart.tsx`:
- Around line 42-63: The polling useEffect (function load used inside
React.useEffect) can overlap requests; add an in-flight guard and per-tick
AbortController to prevent stacked fetches and cancel previous ticks: inside
load (the async function that calls getJobs) introduce a local/in-module boolean
like isFetching (or track via a ref) to return early if a fetch is already in
progress, and create a new AbortController each tick passed to getJobs (or to
the underlying fetch) so you can call controller.abort() in the effect cleanup;
ensure you still set cancelled and clearInterval, and only call setJobs/setError
when not cancelled and the fetch was not aborted. Reference: React.useEffect,
load, getJobs, setJobs, setError, REFRESH_MS.

In `@cueweb/components/ui/context_menus/base-context-menu.tsx`:
- Around line 41-52: The menuMaxHeight computation wrongly enforces a 240px
minimum via Math.max(remainingBelow, 240) which can cause overflow when clicking
near the bottom; update the calculation used in menuMaxHeight (and adjust how
remainingBelow is used) to remove the 240px minimum so the cap respects the
actual available space (e.g., use remainingBelow directly, optionally clamped to
>= 0), keep the VIEWPORT_MARGIN_PX and contextMenuState.position.y usage intact,
and rely on the existing overflowY: 'auto' behavior to handle internal
scrolling.

In `@cueweb/components/ui/jobs-searchbox.tsx`:
- Around line 109-115: The Enter key handler submits even during IME
composition; update handleKeyDown to bail out when an IME composition is in
progress by checking the composition flag (use event.isComposing or
event.nativeEvent.isComposing) before treating Enter as submit. In handleKeyDown
(the useCallback that reads event.key === "Enter"), add an early return if
event.isComposing is truthy, then proceed with the existing preventDefault and
onSubmit(searchQuery) logic.

In `@cueweb/components/ui/simple-data-table.tsx`:
- Around line 159-161: The current computation of columnOrderStorageKey can
collide with columnVisibilityStorageKey when the latter doesn't end with
"columnVisibility"; update the logic in the component so that if
columnVisibilityStorageKey endsWith("columnVisibility") you replace that suffix
to produce columnOrderStorageKey, otherwise derive a distinct key (e.g., append
or prepend a clear suffix like "-columnOrder") to avoid overwriting; modify the
identifier handling around columnVisibilityStorageKey/columnOrderStorageKey to
use this conditional endsWith-based branch so order and visibility persistence
keys are always unique.

In `@cueweb/components/ui/theme-toggle.tsx`:
- Around line 41-46: The pre-mount placeholder returned in theme-toggle.tsx
inside the if (!mounted) block renders a focusable <Button>; make it
non-interactive by adding the disabled prop and tabIndex={-1} to that
placeholder Button (the component named Button in the theme-toggle.tsx file) so
it cannot receive keyboard focus during hydration.

In `@cueweb/README.md`:
- Line 348: The "Keyboard shortcuts" heading currently jumps depth and triggers
markdownlint; locate the heading text "Keyboard shortcuts" and change its
markdown level/style to match the surrounding headings (use the same ATX hash
count or Setext underline style used nearby) so it does not decrease/increase
depth unexpectedly and conforms to MD001/MD003; ensure spacing and casing match
the other section headings for consistency.

In `@docs/_docs/other-guides/cueweb.md`:
- Line 77: Update the inaccurate sentence "Menu items are disabled if the job
has finished, and the context menu is always rendered on-screen." to reflect
current UI logic: indicate that while destructive or editing actions are
disabled for finished jobs, some non-destructive/read-only or view-only context
menu actions remain available and the context menu is still rendered on-screen;
replace the original line with a concise phrasing that mentions
"destructive/edit actions disabled, non-destructive/view actions permitted" and
keeps the note about on-screen rendering.

In `@sandbox/load_test_jobs.py`:
- Around line 897-904: The _normalize_argv function currently promotes any token
present in KNOWN_SUBCOMMANDS as the command even if it is a flag value; change
the logic to only treat a positional token (i.e., a token that does not look
like a flag) as a candidate command. Concretely, in the loop over raw (variables
cmd, raw, rest, KNOWN_SUBCOMMANDS), skip tokens that start with '-' (flags) and
skip tokens that are immediate values of flags (e.g., tokens following a flag
without '='), and only set cmd when you encounter the first non-flag token that
is in KNOWN_SUBCOMMANDS; fall back to "simple" if none found. This ensures flags
like "--prefix chain" won’t mis-detect "chain" as a subcommand.

---

Outside diff comments:
In `@cueweb/app/frames/`[frame-name]/page.tsx:
- Around line 102-113: When getLogLineCount() returns -1 or 0 the component
currently leaves totalNumLogLines stale; inside fetchInitialLogs update state to
clear counts: call setTotalNumLogLines(0) in both the missing (-1) and empty (0)
branches, and also ensure setNumberOfLinesLoaded(0) is called for the missing
branch (it already is for empty). This uses the existing functions
fetchInitialLogs, getLogLineCount, setLogStatus, setNumberOfLinesLoaded, and
setTotalNumLogLines to ensure the header no longer shows stale line counts.

In `@docs/_docs/quick-starts/quick-start-cueweb.md`:
- Around line 132-133: The example startup output shows "Next.js 14.2.32" which
is outdated; update the snippet in quick-start-cueweb.md to reflect the current
major release (replace "Next.js 14.2.32" with "Next.js 15.x" or the exact 15.y.z
used by this repo) so the startup display matches the project's Next.js version.

---

Nitpick comments:
In `@cueweb/app/utils/layers_frames_utils.ts`:
- Around line 78-83: The secondsToHHHMM function doesn't clamp negative values
or floor fractional inputs like secondsToHHMMSS does; update secondsToHHHMM to
first normalize the input with const total = Math.max(0, Math.floor(sec)) (or
equivalent) and then compute hours and minutes from total, keeping the same
padStart formatting, so negative or fractional seconds are handled consistently;
target the secondsToHHHMM function for this change.

In `@cueweb/components/ui/context_menus/useContextMenu.ts`:
- Around line 29-43: MenuItem currently requires label/onClick/isActive even
when separator is true, forcing dummy values; change MenuItem into a
discriminated union (e.g., a Separator variant with separator: true and only the
fields needed, and an Action/Item variant with separator?: false and
label/onClick/isActive/component) and update all consumers (hooks, renderers,
and types referencing MenuItem such as useContextMenu, any menu rendering
components, and handlers) to handle both variants by switching on the separator
discriminator; this removes the need for dummy values and enforces correct
shapes at compile time.

In `@cueweb/components/ui/job-details-inline.tsx`:
- Around line 111-147: The polling useEffect is subscribed to both job and
job?.id causing identity churn to reset the interval; update the dependency
array of the React.useEffect (the effect that defines load, calls
getLayersForJob/getFramesForJob and starts the setInterval) to only depend on
job?.id (remove job) so the interval isn’t recreated when the job object
identity changes.

🪄 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: d12641e1-5c5c-478a-aa5b-28ba0abf3095

📥 Commits

Reviewing files that changed from the base of the PR and between 69d8092 and 88c0eae.

⛔ Files ignored due to path filters (1)

• cueweb/package-lock.json is excluded by !**/package-lock.json

📒 Files selected for processing (76)

• cueweb/Dockerfile
• cueweb/README.md
• cueweb/app/api/health/route.ts
• cueweb/app/api/host/gethosts/route.ts
• cueweb/app/api/job/action/dropdepends/route.ts
• cueweb/app/api/job/action/setautoeat/route.ts
• cueweb/app/api/job/action/setmaxretries/route.ts
• cueweb/app/api/job/action/setpriority/route.ts
• cueweb/app/api/show/getshows/route.ts
• cueweb/app/dashboard/page.tsx
• cueweb/app/frames/[frame-name]/page.tsx
• cueweb/app/frames/frame-columns.tsx
• cueweb/app/globals.css
• cueweb/app/jobs/[job-name]/comments/page.tsx
• cueweb/app/jobs/[job-name]/page.tsx
• cueweb/app/jobs/columns.tsx
• cueweb/app/jobs/data-table.tsx
• cueweb/app/jobs/index.css
• cueweb/app/layers/layer-columns.tsx
• cueweb/app/layout.tsx
• cueweb/app/login/ldap/page.tsx
• cueweb/app/page.tsx
• cueweb/app/providers/job-subscription-poller.tsx
• cueweb/app/utils/action_utils.ts
• cueweb/app/utils/get_utils.ts
• cueweb/app/utils/layer_progress_utils.ts
• cueweb/app/utils/layers_frames_utils.ts
• cueweb/app/utils/use_media_query.ts
• cueweb/app/utils/use_shortcut_notifications.ts
• cueweb/components/dashboard/active-jobs-widget.tsx
• cueweb/components/dashboard/chart-card.tsx
• cueweb/components/dashboard/chart-register.ts
• cueweb/components/dashboard/frame-state-chart.tsx
• cueweb/components/dashboard/hosts-state-chart.tsx
• cueweb/components/dashboard/hosts-widget.tsx
• cueweb/components/dashboard/job-age-chart.tsx
• cueweb/components/dashboard/jobs-per-show-chart.tsx
• cueweb/components/dashboard/recent-failures-widget.tsx
• cueweb/components/dashboard/shows-widget.tsx
• cueweb/components/dashboard/top-jobs-chart.tsx
• cueweb/components/dashboard/widget-card.tsx
• cueweb/components/ui/app-header.tsx
• cueweb/components/ui/app-sidebar.tsx
• cueweb/components/ui/attributes-panel.tsx
• cueweb/components/ui/breadcrumbs.tsx
• cueweb/components/ui/context_menus/action-context-menu.tsx
• cueweb/components/ui/context_menus/base-context-menu.tsx
• cueweb/components/ui/context_menus/useContextMenu.ts
• cueweb/components/ui/dialog.tsx
• cueweb/components/ui/empty-state.tsx
• cueweb/components/ui/frames-layers-popup.tsx
• cueweb/components/ui/job-details-inline.tsx
• cueweb/components/ui/jobs-searchbox.tsx
• cueweb/components/ui/layer-progress-bar.tsx
• cueweb/components/ui/shortcuts-overlay.tsx
• cueweb/components/ui/simple-data-table.tsx
• cueweb/components/ui/skeleton.tsx
• cueweb/components/ui/status-bar.tsx
• cueweb/components/ui/status.tsx
• cueweb/components/ui/subscribe-bell.tsx
• cueweb/components/ui/switch.tsx
• cueweb/components/ui/tabs.tsx
• cueweb/components/ui/theme-toggle.tsx
• cueweb/components/ui/toast-host.tsx
• cueweb/components/ui/toolbar.tsx
• cueweb/next.config.js
• cueweb/package.json
• docs/_docs/concepts/cueweb-rest-gateway.md
• docs/_docs/developer-guide/cueweb-development.md
• docs/_docs/getting-started/deploying-cueweb.md
• docs/_docs/other-guides/cueweb.md
• docs/_docs/quick-starts/quick-start-cueweb.md
• docs/_docs/reference/cueweb.md
• docs/_docs/tutorials/cueweb-tutorial.md
• docs/_docs/user-guides/cueweb-user-guide.md
• sandbox/load_test_jobs.py

💤 Files with no reviewable changes (1)

• cueweb/app/page.tsx

✅ Files skipped from review due to trivial changes (5)

• cueweb/components/dashboard/active-jobs-widget.tsx
• cueweb/app/dashboard/page.tsx
• cueweb/app/globals.css
• docs/_docs/concepts/cueweb-rest-gateway.md
• docs/_docs/reference/cueweb.md

[coderabbitai]

Actionable comments posted: 3

Caution

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

⚠️ Outside diff range comments (2)

cueweb/app/api/job/action/setmaxretries/route.ts (1)

35-36: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Return the correct HTTP status code to callers (no implicit 200s).

In cueweb/app/api/job/action/setmaxretries/route.ts (lines 35-36), both NextResponse.json(...) calls omit the second { status: ... } argument, so Next.js defaults the HTTP response to 200 even when the upstream response.ok is false.

Proposed fix

-  if (!response.ok) return NextResponse.json({ error: responseData.error, status: response.status });
-  return NextResponse.json({ data: responseData.data, status: responseData.status });
+  if (!response.ok) {
+    return NextResponse.json(
+      { error: responseData.error, status: response.status },
+      { status: response.status },
+    );
+  }
+  return NextResponse.json(
+    { data: responseData.data, status: responseData.status ?? response.status },
+    { status: response.status },
+  );

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cueweb/app/api/job/action/setmaxretries/route.ts` around lines 35 - 36, The
two NextResponse.json calls are not specifying an HTTP status so Next.js
defaults to 200; update both calls (the error branch where response.ok is false
and the success branch) to pass the status option as the second argument to
NextResponse.json (use response.status or responseData.status as appropriate) so
the HTTP status returned to callers matches the upstream response; locate the
calls near the response.ok check in route.ts and add the { status: ... } options
to each NextResponse.json invocation.

cueweb/app/api/job/action/dropdepends/route.ts (1)

48-49: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Preserve upstream HTTP status in dropdepends route responses.

Both NextResponse.json returns at cueweb/app/api/job/action/dropdepends/route.ts lines 48-49 omit { status }, so they default to HTTP 200 even when handleRoute returns a non-200 response.

Proposed fix

-  if (!response.ok) return NextResponse.json({ error: responseData.error, status: response.status });
-  return NextResponse.json({ data: responseData.data, status: responseData.status });
+  if (!response.ok) {
+    return NextResponse.json(
+      { error: responseData.error, status: response.status },
+      { status: response.status },
+    );
+  }
+  return NextResponse.json(
+    { data: responseData.data, status: responseData.status ?? response.status },
+    { status: response.status },
+  );

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cueweb/app/api/job/action/dropdepends/route.ts` around lines 48 - 49, The
route currently returns JSON bodies but doesn't set the HTTP status on
NextResponse.json, so clients always get 200; update the two NextResponse.json
calls in cueweb/app/api/job/action/dropdepends/route.ts to pass the upstream
HTTP status: for the error branch use response.status (e.g.,
NextResponse.json(..., { status: response.status })) and for the success branch
use the upstream status (responseData.status if available or fall back to
response.status) so the HTTP response matches what handleRoute/response
produced.

🧹 Nitpick comments (2)

docs/_docs/developer-guide/cueweb-development.md (1)

194-194: 💤 Low value

Consider locale consistency for "afterwards".

Line 194 uses "afterwards" (British English). For consistency with American English conventions commonly used in technical documentation, consider "afterward".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/_docs/developer-guide/cueweb-development.md` at line 194, Update the
phrasing in the documentation snippet describing SubscribeBell to use American
English consistency: replace the word "afterwards" with "afterward" in the
sentence that says the OS-level permission is requested afterwards via an
inlined requestNotificationPermission() helper (the description referencing
SubscribeBell, components/ui/subscribe-bell.tsx, useJobSubscriptions and
requestNotificationPermission()).

docs/_docs/concepts/cueweb-rest-gateway.md (1)

282-282: 💤 Low value

Consider locale consistency for "afterwards".

Line 282 uses "afterwards" (British English). For consistency with American English conventions, consider "afterward".

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/_docs/concepts/cueweb-rest-gateway.md` at line 282, Replace the British
English word "afterwards" with the American English "afterward" in the docs
sentence that currently contains "afterwards" (search for that exact token in
the cueweb-rest-gateway.md content) so the document uses consistent American
English spelling.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cueweb/app/api/job/action/dropdepends/route.ts`:
- Line 34: The route currently calls await request.json() without error
handling; wrap that call in a try/catch inside the handler in
cueweb/app/api/job/action/dropdepends/route.ts so malformed JSON returns a
client error { status: 400 } (consistent with host/gethosts and
job/getcomments). In the try block assign to a typed variable (use any or a type
assertion for jsonBody) so subsequent accesses jsonBody.job and jsonBody.target
remain type-safe, and in the catch return { status: 400 } immediately.

In `@cueweb/app/api/job/action/setmaxretries/route.ts`:
- Line 27: Wrap the await request.json() call (the const jsonBody = await
request.json() line) in a try/catch and on JSON parse errors return a 400
response instead of letting the error bubble to a 500; specifically catch
parsing/SyntaxError (or any error from request.json()) and return a Response
with status 400 and a short error payload/message consistent with the other job
action routes (see setpriority/setautoeat for format) so callers get a clear
"malformed JSON" 400 response.

In `@sandbox/README.md`:
- Line 289: Update the README code span to remove the internal trailing space
that triggers MD038: change the inline code snippet currently shown as `+ ` to
either "`+`" with the space moved outside the backticks (e.g., `+` followed by a
literal space in the sentence) or to a description like "`+` (plus sign)" so the
code span contains no trailing whitespace; the surrounding flag reference
`--print-names` should remain unchanged.

---

Outside diff comments:
In `@cueweb/app/api/job/action/dropdepends/route.ts`:
- Around line 48-49: The route currently returns JSON bodies but doesn't set the
HTTP status on NextResponse.json, so clients always get 200; update the two
NextResponse.json calls in cueweb/app/api/job/action/dropdepends/route.ts to
pass the upstream HTTP status: for the error branch use response.status (e.g.,
NextResponse.json(..., { status: response.status })) and for the success branch
use the upstream status (responseData.status if available or fall back to
response.status) so the HTTP response matches what handleRoute/response
produced.

In `@cueweb/app/api/job/action/setmaxretries/route.ts`:
- Around line 35-36: The two NextResponse.json calls are not specifying an HTTP
status so Next.js defaults to 200; update both calls (the error branch where
response.ok is false and the success branch) to pass the status option as the
second argument to NextResponse.json (use response.status or responseData.status
as appropriate) so the HTTP status returned to callers matches the upstream
response; locate the calls near the response.ok check in route.ts and add the {
status: ... } options to each NextResponse.json invocation.

---

Nitpick comments:
In `@docs/_docs/concepts/cueweb-rest-gateway.md`:
- Line 282: Replace the British English word "afterwards" with the American
English "afterward" in the docs sentence that currently contains "afterwards"
(search for that exact token in the cueweb-rest-gateway.md content) so the
document uses consistent American English spelling.

In `@docs/_docs/developer-guide/cueweb-development.md`:
- Line 194: Update the phrasing in the documentation snippet describing
SubscribeBell to use American English consistency: replace the word "afterwards"
with "afterward" in the sentence that says the OS-level permission is requested
afterwards via an inlined requestNotificationPermission() helper (the
description referencing SubscribeBell, components/ui/subscribe-bell.tsx,
useJobSubscriptions and requestNotificationPermission()).

🪄 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: 4e822f5e-0178-42ab-9a78-38eeb1fec5db

📥 Commits

Reviewing files that changed from the base of the PR and between 88c0eae and 1c8d234.

📒 Files selected for processing (25)

• cueweb/README.md
• cueweb/app/api/health/route.ts
• cueweb/app/api/host/gethosts/route.ts
• cueweb/app/api/job/action/dropdepends/route.ts
• cueweb/app/api/job/action/setautoeat/route.ts
• cueweb/app/api/job/action/setmaxretries/route.ts
• cueweb/app/api/job/action/setpriority/route.ts
• cueweb/app/api/show/getshows/route.ts
• cueweb/app/jobs/[job-name]/page.tsx
• cueweb/app/jobs/data-table.tsx
• cueweb/app/utils/action_utils.ts
• cueweb/components/dashboard/jobs-per-show-chart.tsx
• cueweb/components/ui/context_menus/base-context-menu.tsx
• cueweb/components/ui/jobs-searchbox.tsx
• cueweb/components/ui/simple-data-table.tsx
• cueweb/components/ui/theme-toggle.tsx
• docs/_docs/concepts/cueweb-rest-gateway.md
• docs/_docs/developer-guide/cueweb-development.md
• docs/_docs/other-guides/cueweb.md
• docs/_docs/quick-starts/quick-start-cueweb.md
• docs/_docs/reference/cueweb.md
• docs/_docs/tutorials/cueweb-tutorial.md
• docs/_docs/user-guides/cueweb-user-guide.md
• sandbox/README.md
• sandbox/load_test_jobs.py

✅ Files skipped from review due to trivial changes (1)

• docs/_docs/other-guides/cueweb.md

[coderabbitai]

🧹 Nitpick comments (1)

cueweb/app/api/job/action/setmaxretries/route.ts (1)

35-43: ⚡ Quick win

Consider validating max_retries is finite and non-negative.

The typeof jsonBody.max_retries !== 'number' check allows NaN, Infinity, and negative values (all are type number in JavaScript). Adding finite and range checks would prevent invalid values from reaching the backend.

🛡️ Proposed validation enhancement

  if (
    !jsonBody
    || typeof jsonBody !== 'object'
    || Array.isArray(jsonBody)
    || !jsonBody.job
    || typeof jsonBody.max_retries !== 'number'
+   || !Number.isFinite(jsonBody.max_retries)
+   || jsonBody.max_retries < 0
  ) {
-   return NextResponse.json({ error: 'Invalid request body (need {job, max_retries:number})' }, { status: 400 });
+   return NextResponse.json({ error: 'Invalid request body (need {job, max_retries:non-negative finite number})' }, { status: 400 });
  }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cueweb/app/api/job/action/setmaxretries/route.ts` around lines 35 - 43, The
current request-body validation in route.ts only checks typeof
jsonBody.max_retries === 'number' which still permits NaN, Infinity and
negatives; update the validation around jsonBody.max_retries (in the if that
returns NextResponse.json(...)) to ensure the value is finite and non-negative
by using Number.isFinite(jsonBody.max_retries) (or Number.isFinite + >= 0)
before accepting it; adjust the conditional to reject when max_retries is not
finite or is < 0 so only valid numeric retry counts reach the handler.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@cueweb/app/api/job/action/setmaxretries/route.ts`:
- Around line 35-43: The current request-body validation in route.ts only checks
typeof jsonBody.max_retries === 'number' which still permits NaN, Infinity and
negatives; update the validation around jsonBody.max_retries (in the if that
returns NextResponse.json(...)) to ensure the value is finite and non-negative
by using Number.isFinite(jsonBody.max_retries) (or Number.isFinite + >= 0)
before accepting it; adjust the conditional to reject when max_retries is not
finite or is < 0 so only valid numeric retry counts reach the handler.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e83a4d79-65b3-4f9a-a200-d636d9c20bb0

📥 Commits

Reviewing files that changed from the base of the PR and between 1c8d234 and df32e90.

📒 Files selected for processing (3)

• cueweb/app/api/job/action/dropdepends/route.ts
• cueweb/app/api/job/action/setmaxretries/route.ts
• sandbox/README.md

✅ Files skipped from review due to trivial changes (1)

• sandbox/README.md

[ramonfigueiredo]

CueWeb Dashboard

CueWeb > Cuetopia > Monitor Jobs

CueWeb Light mode

Search job option: 1) Type job name + Enter or Regex

Search job option: 2) Pick jobs from a list (live results, click to add)

Job Details (Right click on a job > View Job Details)

CueWeb Dark Mode

Menus

Menu: Dashboard

Menu: File > Disable Job Interaction

Menu: Cuebot Facility

Menu: Cuetopia > Monitor Jobs

Menu: CueCommander (Many options/pages)

Menu: Other (Attributes Window and Shortcuts)

Menu: Help (Online User Guide, Make a Suggestion, Report a Bug)

Left-side Menu

Cuetopia > Monitor Jobs: Menus after right-clicking a job/layer/frame

Menu after right-clicking a job

Menu after right-clicking a layer

Menu after right-clicking a frame

Frame Log Page

Shortcuts (Press ? or Menu: Others > Show Shortcuts)

Page: Job Comments Management (CRUD)

CueWeb status indicators

Add fixed IDE-style bottom status bar showing gateway health, last refresh state, and build version

• Gateway status shows real-time health (/api/health polling, response latency, online/offline state)
• Last refresh shows continuously updated relative timing for Jobs table refresh cycles

---

Optional Role-Based Access Control (RBAC) / Admin platform

Managing users and roles (Admin UI tutorial)

This walkthrough assumes the deployment was started with NEXT_PUBLIC_AUTH_PROVIDER=local (the simplest path - no external IdP needed). If you used Okta, Google, GitHub, or LDAP, the user lifecycle works the same way; you just sign in through that provider's button instead.

Find the bootstrap password

The first time CueWeb runs with local enabled it prints a one-time bootstrap admin password to the container log. Find it with:

docker compose logs cueweb --tail 20

Look for a banner that reads CueWeb bootstrap admin created. The password is also saved to /data/.cueweb-bootstrap inside the container (mode 0600).

Sign in and rotate the password

1. Open /login. The form shows Username + Password fields.
2. Sign in as admin with the bootstrap password. CueWeb redirects to Set a new password.
3. Enter the bootstrap password as Current password, then a new password (12+ characters) twice. Click Update password.

You land on the Jobs dashboard. An Admin button now appears in the top-right of the header.

Create your first user

1. Click Admin -> Users tab.
2. Click Create local user, fill in the form (username + display name + email + temporary 12+ character password), and submit.
3. The new user appears in the table with a must change pw chip - they'll be forced to rotate the password on first sign-in, just like the bootstrap admin.

Assign a role

By default a brand-new local user has no roles, so they cannot do anything beyond sign in.

1. In the same Users tab, find the new user's row.
2. In the Direct roles column, pick operator (or viewer) from the + attach... dropdown.
3. The role appears as a chip next to their name; CueWeb refreshes effective permissions on the user's next API call.

Use groups to scale assignments

Use Groups when you need to manage many users at once - for example, all operators in a show.

1. Open the Groups tab and click Create local group, e.g. gr-render-operators.
2. In the new group's Roles column, attach operator.
3. (Future improvement: a "Users in this group" tab. For now, group membership is best driven from Okta or LDAP via the groups resolver - any user in the matching directory group gets operator automatically on next sign-in.)

Add a second admin

1. Open the Admins tab.
2. Pick Username (or Email / External ID) and type the username of an existing user from the Users tab.
3. Click Add admin. They can now reach /admin themselves.

CueWeb refuses to remove the last admin row so the policy store always has a recovery path.

Reset a forgotten password (local users)

If a local user loses their password, an admin can reset it:

1. Open Users, find the row.
2. Click Reset password, supply a fresh temporary password (12+ characters).
3. The user's next sign-in lands on Set a new password automatically.

Look at the audit log

1. Open the Audit log tab.
2. Filter by action (e.g. user.create) or actor (e.g. admin).
3. Expand a row's "before/after" to see the JSON change. Click Export current view to CSV to share with security or compliance.

---

Sign in

Set a new password

CueWeb administration

• Overview

• Users

• Groups

• Roles

• Permissions

• Admins

• Audit log

[coderabbitai]

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)

cueweb/app/jobs/columns.tsx (1)

334-346: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Use nullish coalescing in state sort fallback.

Line 345 uses ||, so the "Failing" rank (0) is discarded as falsy and sorted as fallback 5. This breaks state ordering.

🛠️ Suggested fix

-      return (stateOrder[stateA] || 5) - (stateOrder[stateB] || 5);
+      return (stateOrder[stateA] ?? 5) - (stateOrder[stateB] ?? 5);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cueweb/app/jobs/columns.tsx` around lines 334 - 346, The state sorting
function (sortingFn) currently falls back using || which treats the rank 0 for
"Failing" as falsy; update the fallback to use nullish coalescing (??) when
looking up stateOrder for stateA and stateB so a defined 0 value is preserved
(e.g., replace (stateOrder[stateA] || 5) and (stateOrder[stateB] || 5) with
nullish coalescing), leaving getState and the stateOrder mapping unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cueweb/app/utils/action_utils.ts`:
- Around line 458-467: The copyTextToClipboard function currently returns
navigator.clipboard.writeText(text) directly so a rejection prevents the legacy
document.execCommand("copy") fallback from running; change copyTextToClipboard
to await or attach a .catch on navigator.clipboard.writeText(text) (inside the
feature-detection branch) and on rejection fall through to the existing legacy
path (do not return immediately on success without handling errors), ensuring
the function still attempts the document-based fallback when clipboard.writeText
fails; update function copyTextToClipboard to catch or await the promise from
navigator.clipboard.writeText and only short-circuit on success while allowing
the fallback to run on error.

In `@cueweb/README.md`:
- Line 110: The README contains a broken fragment link to
`#external-editor-integration` because there is no corresponding heading; either
add a heading titled "External editor integration" (exact text) somewhere
appropriate in the file or update the link text at the sentence starting with "A
new **View Log on \<editor\>** item..." to point to the actual existing heading
ID; search for any existing heading about external editor integration and align
the anchor (or add the missing heading) so the fragment resolves.

---

Outside diff comments:
In `@cueweb/app/jobs/columns.tsx`:
- Around line 334-346: The state sorting function (sortingFn) currently falls
back using || which treats the rank 0 for "Failing" as falsy; update the
fallback to use nullish coalescing (??) when looking up stateOrder for stateA
and stateB so a defined 0 value is preserved (e.g., replace (stateOrder[stateA]
|| 5) and (stateOrder[stateB] || 5) with nullish coalescing), leaving getState
and the stateOrder mapping unchanged.

🪄 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: cf58d505-db51-4046-be8e-04dd6ea8dda2

📥 Commits

Reviewing files that changed from the base of the PR and between df32e90 and 4f87999.

📒 Files selected for processing (25)

• cueweb/Dockerfile
• cueweb/README.md
• cueweb/app/frames/frame-columns.tsx
• cueweb/app/jobs/columns.tsx
• cueweb/app/jobs/data-table.tsx
• cueweb/app/layers/layer-columns.tsx
• cueweb/app/layout.tsx
• cueweb/app/utils/action_utils.ts
• cueweb/app/utils/api_utils.ts
• cueweb/components/ui/app-header.tsx
• cueweb/components/ui/context_menus/action-context-menu.tsx
• cueweb/components/ui/mobile-nav-sheet.tsx
• cueweb/components/ui/row-actions-cell.tsx
• cueweb/components/ui/sheet.tsx
• cueweb/components/ui/shortcuts-overlay.tsx
• cueweb/components/ui/simple-data-table.tsx
• docker-compose.yml
• docs/_docs/concepts/cueweb-rest-gateway.md
• docs/_docs/developer-guide/cueweb-development.md
• docs/_docs/getting-started/deploying-cueweb.md
• docs/_docs/other-guides/cueweb.md
• docs/_docs/quick-starts/quick-start-cueweb.md
• docs/_docs/reference/cueweb.md
• docs/_docs/tutorials/cueweb-tutorial.md
• docs/_docs/user-guides/cueweb-user-guide.md

✅ Files skipped from review due to trivial changes (6)

• cueweb/components/ui/row-actions-cell.tsx
• docs/_docs/developer-guide/cueweb-development.md
• docs/_docs/getting-started/deploying-cueweb.md
• docs/_docs/reference/cueweb.md
• docs/_docs/user-guides/cueweb-user-guide.md
• docs/_docs/concepts/cueweb-rest-gateway.md

[coderabbitai]

♻️ Duplicate comments (1)

cueweb/app/utils/action_utils.ts (1)

458-467: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

navigator.clipboard.writeText() rejection bypasses legacy fallback.

When writeText() rejects (e.g., permission denied, document lost focus), the promise is returned directly and the execCommand fallback at line 469+ never executes. Wrap in try/catch so failures fall through to the legacy path.

Proposed fix

   if (
     typeof navigator !== "undefined"
     && navigator.clipboard
     && typeof window !== "undefined"
     && window.isSecureContext
   ) {
-    return navigator.clipboard.writeText(text);
+    try {
+      await navigator.clipboard.writeText(text);
+      return;
+    } catch {
+      // Fall through to legacy fallback.
+    }
   }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cueweb/app/utils/action_utils.ts` around lines 458 - 467, The modern
clipboard branch in copyTextToClipboard returns
navigator.clipboard.writeText(text) directly, which prevents falling back to the
legacy execCommand path if writeText rejects; wrap the await/return of
navigator.clipboard.writeText(text) in a try/catch inside copyTextToClipboard
(call navigator.clipboard.writeText(text) and await it), and on catch simply
allow the function to continue (do not rethrow) so the existing legacy
execCommand fallback runs; refer to the copyTextToClipboard function and its
navigator.clipboard.writeText usage to make this change.

🧹 Nitpick comments (2)

cueweb/components/ui/row-actions-cell.tsx (1)

59-64: ⚡ Quick win

Expose explicit menu-trigger semantics on the actions button.

Adding aria-haspopup="menu" helps assistive tech announce the button as opening a menu.

♿ Suggested tweak

     <button
       type="button"
       onClick={handleClick}
+      aria-haspopup="menu"
       aria-label={label}
       title={label}
       className="inline-flex h-7 w-7 items-center justify-center rounded text-foreground/60 transition-colors hover:bg-foreground/10 hover:text-foreground focus:outline-none focus:ring-2 focus:ring-ring"
     >

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cueweb/components/ui/row-actions-cell.tsx` around lines 59 - 64, The actions
button JSX (button with onClick={handleClick}, aria-label={label},
title={label}) lacks the explicit menu-trigger semantic; add
aria-haspopup="menu" to that button element so assistive technologies announce
it as opening a menu, keeping the existing attributes (onClick/aria-label/title)
unchanged and preserving className and event handler (handleClick).

cueweb/app/layers/layer-columns.tsx (1)

212-214: ⚡ Quick win

Use raw timestamp as accessor value for deterministic date sorting.

Keep the accessor numeric and move formatting to cell so sorting stays stable regardless of display format.

🛠️ Suggested refactor

   {
     id: "eligible",
-    accessorFn: (row) => (row.eligibleTime ? convertUnixToHumanReadableDate(row.eligibleTime) : ""),
+    accessorFn: (row) => row.eligibleTime ?? 0,
+    cell: ({ row }) =>
+      row.original.eligibleTime
+        ? convertUnixToHumanReadableDate(row.original.eligibleTime)
+        : "",
     header: ({ column }) => renderHeader("Eligible", column),
   },

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cueweb/app/layers/layer-columns.tsx` around lines 212 - 214, The accessor for
the "eligible" column currently returns a formatted string which breaks numeric
sorting; change the accessorFn on the column with id "eligible" to return the
raw numeric timestamp (e.g. row.eligibleTime or null/undefined) and implement a
cell renderer that calls convertUnixToHumanReadableDate on that timestamp for
display; keep header as renderHeader("Eligible", column) and ensure the cell
handles empty/undefined timestamps the same way as the previous conditional
formatting.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Duplicate comments:
In `@cueweb/app/utils/action_utils.ts`:
- Around line 458-467: The modern clipboard branch in copyTextToClipboard
returns navigator.clipboard.writeText(text) directly, which prevents falling
back to the legacy execCommand path if writeText rejects; wrap the await/return
of navigator.clipboard.writeText(text) in a try/catch inside copyTextToClipboard
(call navigator.clipboard.writeText(text) and await it), and on catch simply
allow the function to continue (do not rethrow) so the existing legacy
execCommand fallback runs; refer to the copyTextToClipboard function and its
navigator.clipboard.writeText usage to make this change.

---

Nitpick comments:
In `@cueweb/app/layers/layer-columns.tsx`:
- Around line 212-214: The accessor for the "eligible" column currently returns
a formatted string which breaks numeric sorting; change the accessorFn on the
column with id "eligible" to return the raw numeric timestamp (e.g.
row.eligibleTime or null/undefined) and implement a cell renderer that calls
convertUnixToHumanReadableDate on that timestamp for display; keep header as
renderHeader("Eligible", column) and ensure the cell handles empty/undefined
timestamps the same way as the previous conditional formatting.

In `@cueweb/components/ui/row-actions-cell.tsx`:
- Around line 59-64: The actions button JSX (button with onClick={handleClick},
aria-label={label}, title={label}) lacks the explicit menu-trigger semantic; add
aria-haspopup="menu" to that button element so assistive technologies announce
it as opening a menu, keeping the existing attributes (onClick/aria-label/title)
unchanged and preserving className and event handler (handleClick).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 20059789-514d-4771-82a3-49a31c10f37b

📥 Commits

Reviewing files that changed from the base of the PR and between df32e90 and 4f87999.

📒 Files selected for processing (25)

• cueweb/Dockerfile
• cueweb/README.md
• cueweb/app/frames/frame-columns.tsx
• cueweb/app/jobs/columns.tsx
• cueweb/app/jobs/data-table.tsx
• cueweb/app/layers/layer-columns.tsx
• cueweb/app/layout.tsx
• cueweb/app/utils/action_utils.ts
• cueweb/app/utils/api_utils.ts
• cueweb/components/ui/app-header.tsx
• cueweb/components/ui/context_menus/action-context-menu.tsx
• cueweb/components/ui/mobile-nav-sheet.tsx
• cueweb/components/ui/row-actions-cell.tsx
• cueweb/components/ui/sheet.tsx
• cueweb/components/ui/shortcuts-overlay.tsx
• cueweb/components/ui/simple-data-table.tsx
• docker-compose.yml
• docs/_docs/concepts/cueweb-rest-gateway.md
• docs/_docs/developer-guide/cueweb-development.md
• docs/_docs/getting-started/deploying-cueweb.md
• docs/_docs/other-guides/cueweb.md
• docs/_docs/quick-starts/quick-start-cueweb.md
• docs/_docs/reference/cueweb.md
• docs/_docs/tutorials/cueweb-tutorial.md
• docs/_docs/user-guides/cueweb-user-guide.md

✅ Files skipped from review due to trivial changes (6)

• docs/_docs/reference/cueweb.md
• docs/_docs/quick-starts/quick-start-cueweb.md
• docs/_docs/developer-guide/cueweb-development.md
• docs/_docs/concepts/cueweb-rest-gateway.md
• docs/_docs/user-guides/cueweb-user-guide.md
• docs/_docs/other-guides/cueweb.md

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@cueweb/app/jobs/columns.tsx`:
- Around line 339-340: The code forwards a username from table.options.meta into
JobCommentIndicator (via the username variable), exposing PII in URLs; remove
the username prop usage and stop passing username from table.options.meta to
JobCommentIndicator, instead have JobCommentIndicator obtain the current user
identity from the authenticated session/context (e.g., session hook or auth
provider) and construct comment URLs/server calls without embedding the username
in query strings; update JobCommentIndicator's props/signature to drop the
username param and update any internal URL-building logic to use session-derived
identity.

In `@cueweb/README.md`:
- Line 371: The README's Actions bullet lists "Copy Log Directory" but the
implemented context menu exposes only "Copy Log Path" and "Copy Frame Name";
update the README to match the code by removing "Copy Log Directory" from the
Actions line (or alternatively implement a corresponding "Copy Log Directory"
menu item in the context-menu code to match the docs). Ensure the Actions
sentence containing "View Job Details", "Copy Job / Layer / Frame Name", "Copy
Log Path / Log Directory", "View Log + Tail Log", and "View Log on <editor>" is
edited so the exact menu item names ("Copy Log Path", "Copy Frame Name", and
optionally "Copy Log Directory") reflect the actual context-menu implementation.

🪄 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: b7e22311-539a-4cdb-a434-93638cf19135

📥 Commits

Reviewing files that changed from the base of the PR and between 16829ff and 7e6c62e.

📒 Files selected for processing (10)

• cueweb/README.md
• cueweb/app/jobs/columns.tsx
• cueweb/components/ui/context_menus/action-context-menu.tsx
• docs/_docs/concepts/cueweb-rest-gateway.md
• docs/_docs/developer-guide/cueweb-development.md
• docs/_docs/other-guides/cueweb.md
• docs/_docs/quick-starts/quick-start-cueweb.md
• docs/_docs/reference/cueweb.md
• docs/_docs/tutorials/cueweb-tutorial.md
• docs/_docs/user-guides/cueweb-user-guide.md

✅ Files skipped from review due to trivial changes (2)

• docs/_docs/user-guides/cueweb-user-guide.md
• docs/_docs/other-guides/cueweb.md

[coderabbitai]

Caution

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

⚠️ Outside diff range comments (1)

cueweb/components/ui/context_menus/action-context-menu.tsx (1)

461-462: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Hardcoded "black" icon color breaks dark mode contrast.

The Retry icons use color={active ? "black" : "gray"}, which renders poorly against dark backgrounds. Use undefined (or "currentColor") for the active state so the icon inherits the theme's text color.

🎨 Suggested fix

-    { label: "Retry", onClick: retryLayerFramesGivenRow, isActive: active, component: <TbReload className="mr-1" size={14} color={active ? "black" : "gray"} /> },
+    { label: "Retry", onClick: retryLayerFramesGivenRow, isActive: active, component: <TbReload className="mr-1" size={14} color={active ? undefined : "gray"} /> },

Apply the same fix in FrameContextMenu at line 630:

-    { label: "Retry", onClick: retryFrameGivenRow, isActive: active, component: <TbReload className="mr-1" size={14} color={active ? "black" : "gray"} /> },
+    { label: "Retry", onClick: retryFrameGivenRow, isActive: active, component: <TbReload className="mr-1" size={14} color={active ? undefined : "gray"} /> },

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cueweb/components/ui/context_menus/action-context-menu.tsx` around lines 461
- 462, The icon color is hardcoded to "black" for active state which breaks dark
mode; update the menu item entries that use TbReload (the ones with onClick
handlers retryLayerFramesGivenRow and retryLayerDeadFramesGivenRow) to pass
color={undefined} (or "currentColor") when active so the icon inherits theme
text color instead of forcing "black"; apply the same change in the
FrameContextMenu occurrence referenced (same TbReload usage) so both places
follow the theme.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@cueweb/components/ui/context_menus/action-context-menu.tsx`:
- Around line 461-462: The icon color is hardcoded to "black" for active state
which breaks dark mode; update the menu item entries that use TbReload (the ones
with onClick handlers retryLayerFramesGivenRow and retryLayerDeadFramesGivenRow)
to pass color={undefined} (or "currentColor") when active so the icon inherits
theme text color instead of forcing "black"; apply the same change in the
FrameContextMenu occurrence referenced (same TbReload usage) so both places
follow the theme.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 16326dc7-277d-421e-afeb-d6c8f8355040

📥 Commits

Reviewing files that changed from the base of the PR and between 7e6c62e and 9635c05.

📒 Files selected for processing (8)

• cueweb/README.md
• cueweb/app/jobs/columns.tsx
• cueweb/app/jobs/data-table.tsx
• cueweb/components/ui/context_menus/action-context-menu.tsx
• docs/_docs/concepts/cueweb-rest-gateway.md
• docs/_docs/developer-guide/cueweb-development.md
• docs/_docs/other-guides/cueweb.md
• docs/_docs/quick-starts/quick-start-cueweb.md

💤 Files with no reviewable changes (1)

• cueweb/app/jobs/data-table.tsx

✅ Files skipped from review due to trivial changes (5)

• docs/_docs/quick-starts/quick-start-cueweb.md
• docs/_docs/developer-guide/cueweb-development.md
• cueweb/README.md
• docs/_docs/concepts/cueweb-rest-gateway.md
• docs/_docs/other-guides/cueweb.md

[coderabbitai]

Actionable comments posted: 11

Note

Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.

🟡 Minor comments (12)

cueweb/app/api/admin/audit/route.ts-40-42 (1)

40-42: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Escape LIKE special characters in user input.

The actor parameter is user-controlled and inserted into a LIKE pattern without escaping the special characters % and _. While this doesn't allow SQL injection (the query uses parameterization), it does allow users to inject LIKE wildcards that could match unintended records or bypass filtering.

🛡️ Proposed fix to escape LIKE wildcards

Add a helper to escape LIKE special characters:

function escapeLike(input: string): string {
  return input.replace(/[%_]/g, '\\$&');
}

Then apply it before building the pattern:

  if (actor) {
    clauses.push("actor_label LIKE ?");
-    args.push(`%${actor}%`);
+    args.push(`%${escapeLike(actor)}%`);
  }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cueweb/app/api/admin/audit/route.ts` around lines 40 - 42, The actor value is
inserted into the SQL LIKE pattern without escaping '%' and '_' allowing users
to inject wildcard matches; add an escape helper (e.g., escapeLike) that
replaces /[%_]/ with escaped versions, call it on actor before constructing the
pattern, and then push the escaped pattern (like `%${escapedActor}%`) into args
where clauses.push("actor_label LIKE ?") is used so the LIKE treats literal
characters instead of wildcards.

cueweb/app/admin/layout.tsx-7-7 (1)

7-7: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Remove unused import.

The Link import from next/link is not used in this file.

🧹 Proposed fix

-import Link from "next/link";

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cueweb/app/admin/layout.tsx` at line 7, Remove the unused Link import: delete
the import of Link from "next/link" in the file (the symbol Link in layout.tsx)
so the module only imports what is actually used; ensure no other references to
Link remain in the file after removal.

cueweb/app/api/admin/users/[id]/password/route.ts-50-56 (1)

50-56: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Require password to be a string before validation.

Line 50 coerces arbitrary JSON values into strings. That can silently accept malformed payloads and set unintended passwords.

Suggested fix

-  const password = String(body.password ?? "");
+  if (typeof body.password !== "string") {
+    return NextResponse.json({ error: "invalid_password_type" }, { status: 400 });
+  }
+  const password = body.password;
   if (password.length < MIN_LEN) {

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cueweb/app/api/admin/users/`[id]/password/route.ts around lines 50 - 56, The
handler currently coerces arbitrary JSON into a string via const password =
String(body.password ?? "") which can accept non-string payloads; instead, first
verify that body.password is a string (e.g., typeof body.password === "string")
and return the 400 NextResponse.json error if it is missing or not a string,
then perform the MIN_LEN check (using the existing MIN_LEN symbol) and only
thereafter use the validated string for further processing; update the code
paths that reference password to rely on this validated string rather than blind
coercion.

cueweb/app/api/admin/groups/[id]/route.ts-25-27 (1)

25-27: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Tighten ID validation to integers only.

Line 25 and Line 26 accept any finite numeric value. Route IDs should be positive integers to avoid accepting malformed path params.

Suggested fix

   const { id } = await params;
   const groupId = Number(id);
-  if (!Number.isFinite(groupId)) {
+  if (!Number.isInteger(groupId) || groupId <= 0) {
     return NextResponse.json({ error: "invalid_id" }, { status: 400 });
   }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cueweb/app/api/admin/groups/`[id]/route.ts around lines 25 - 27, The current
validation converts id to Number (groupId) and only checks Number.isFinite,
which allows non-integer and non-positive values; change the check in the
handler that computes groupId to require a positive integer by validating
Number.isInteger(groupId) && groupId > 0 and return the same NextResponse.json({
error: "invalid_id" }, { status: 400 }) when that fails; update the validation
around the variables id and groupId in route.ts so only positive integers are
accepted.

cueweb/app/admin/groups/page.tsx-69-85 (1)

69-85: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add error handling for role attach/detach operations.

Both attachRole and detachRole silently ignore failures. Users won't know if the operation failed.

Proposed fix

   async function attachRole(groupId: number, roleName: string) {
     if (!roleName) return;
-    await fetch(`/api/admin/groups/${groupId}/roles`, {
+    const res = await fetch(`/api/admin/groups/${groupId}/roles`, {
       method: "POST",
       headers: { "content-type": "application/json" },
       body: JSON.stringify({ roleName }),
     });
+    if (!res.ok) {
+      const b = await res.json().catch(() => ({}));
+      alert(b.error || "Failed to attach role.");
+    }
     refresh();
   }

   async function detachRole(groupId: number, roleName: string) {
     const sp = new URLSearchParams({ roleName });
-    await fetch(`/api/admin/groups/${groupId}/roles?${sp}`, {
+    const res = await fetch(`/api/admin/groups/${groupId}/roles?${sp}`, {
       method: "DELETE",
     });
+    if (!res.ok) {
+      const b = await res.json().catch(() => ({}));
+      alert(b.error || "Failed to detach role.");
+    }
     refresh();
   }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cueweb/app/admin/groups/page.tsx` around lines 69 - 85, attachRole and
detachRole currently ignore network/server failures; update both functions
(attachRole, detachRole) to handle errors by checking the fetch response
(response.ok) and treating non-OK as failures: await response, parse error
text/json if present, show a user-visible error (e.g. alert or toast) instead of
always calling refresh, and wrap the fetch in try/catch to surface network
exceptions similarly; only call refresh() on success and ensure the error path
logs the error (console.error) and notifies the user.

cueweb/app/admin/admins/page.tsx-25-28 (1)

25-28: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add error handling for the initial fetch.

If the fetch fails (network error or non-OK response), calling .json() on the response may throw or return unexpected data, leaving admins in an inconsistent state.

Proposed fix

   const refresh = React.useCallback(async () => {
-    const r = await fetch("/api/admin/admins").then((r) => r.json());
-    setAdmins(r.admins ?? []);
+    try {
+      const res = await fetch("/api/admin/admins");
+      if (!res.ok) throw new Error("Failed to fetch admins");
+      const data = await res.json();
+      setAdmins(data.admins ?? []);
+    } catch {
+      setError("Failed to load admins.");
+    }
   }, []);

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cueweb/app/admin/admins/page.tsx` around lines 25 - 28, The refresh
function's fetch currently calls .json() unguarded and can leave admins in an
inconsistent state on network or non-OK responses; wrap the fetch and parsing in
try/catch, check response.ok before calling response.json(), and on any failure
log the error (e.g., console.error) and setAdmins([]) or keep previous state as
appropriate; update the refresh callback (the async function that calls
fetch("/api/admin/admins") and setAdmins) to handle non-OK responses and
exceptions so admins is deterministically reset or preserved.

cueweb/app/api/admin/users/[id]/roles/route.ts-73-98 (1)

73-98: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

DELETE handler skips user existence check.

The GET and POST handlers verify the user exists before proceeding, but DELETE only validates the id format and role existence. If the user doesn't exist, detachRoleFromUser is still called and an audit entry is recorded for a non-existent user.

Proposed fix

   const userId = Number(id);
   if (!Number.isFinite(userId)) {
     return NextResponse.json({ error: "invalid_id" }, { status: 400 });
   }
+  if (!findUserById(userId)) {
+    return NextResponse.json({ error: "not_found" }, { status: 404 });
+  }
   const roleName = req.nextUrl.searchParams.get("roleName") ?? "";

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cueweb/app/api/admin/users/`[id]/roles/route.ts around lines 73 - 98, The
DELETE handler must verify the target user exists before calling
detachRoleFromUser and recording an audit. After parsing userId (from params)
and resolving role (using findRoleByName), look up the user using the same
helper used in GET/POST (e.g., findUserById(userId) or the project's getUserById
helper); if the lookup returns null/undefined return NextResponse.json({ error:
"user_not_found" }, { status: 404 }) and do not call detachRoleFromUser or
appendAudit; only perform detachRoleFromUser(userId, role.id) and
appendAudit(...) when the user lookup succeeds. Ensure you use the existing
symbols requireAdmin, findRoleByName, detachRoleFromUser, appendAudit, and
NextResponse to implement this check.

cueweb/README.md-281-281 (1)

281-281: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Align supported provider list across the README.

Line 281 and Line 297 currently document only local/okta/ldap, but the same README elsewhere and this PR’s implementation still include google/github. Please keep one consistent provider set to avoid misconfiguration.

Suggested fix

-The CueWeb project utilizes the [NextAuth.js](https://next-auth.js.org/) library for authentication. Supported providers are **Okta**, **LDAP**, and a built-in **local** username/password provider that ships with CueWeb.
+The CueWeb project utilizes the [NextAuth.js](https://next-auth.js.org/) library for authentication. Supported providers are **local**, **Okta**, **Google**, **GitHub**, and **LDAP**.

-To enable an authentication provider, list it in `NEXT_PUBLIC_AUTH_PROVIDER`. Supported values are `local`, `okta`, and `ldap`, comma-separated (e.g. `NEXT_PUBLIC_AUTH_PROVIDER=local,okta`).
+To enable an authentication provider, list it in `NEXT_PUBLIC_AUTH_PROVIDER`. Supported values are `local`, `okta`, `google`, `github`, and `ldap`, comma-separated (e.g. `NEXT_PUBLIC_AUTH_PROVIDER=local,okta`).

Also applies to: 297-297

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cueweb/README.md` at line 281, The README lists supported auth providers
inconsistently; update the provider list mentions at the spots referencing
NEXT_PUBLIC_AUTH_PROVIDER (currently lines describing "local/okta/ldap") to
match the actual implementation and other README references by including google
and github (so the documented comma-separated identifiers include local, okta,
ldap, google, github), or alternatively remove google/github everywhere if you
intend to drop them; ensure all README occurrences that enumerate providers use
the exact same set of identifiers (NEXT_PUBLIC_AUTH_PROVIDER values) so the docs
and implementation are consistent.

docs/_docs/developer-guide/cueweb-development.md-341-341 (1)

341-341: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add a language identifier to this fenced code block.

Line 341 opens a fence without a language, which triggers markdownlint MD040.

Suggested fix

-```
+```text
 cueweb/
 ├── instrumentation.ts                # Next.js server-startup hook (edge-safe)
 ...

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @docs/_docs/developer-guide/cueweb-development.md at line 341, The fenced
code block that begins with "cueweb/" is missing a language identifier which
triggers markdownlint MD040; add an explicit language tag (for example "text" or
"bash") immediately after the opening backticks for that block so it becomes
text (or bash) to suppress MD040 and ensure proper syntax highlighting for
the directory tree.

</details>

</blockquote></details>
<details>
<summary>cueweb/package.json-39-39 (1)</summary><blockquote>

`39-39`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_

**Avoid dual type sources for `better-sqlite3` (remove `@types/better-sqlite3` from `cueweb/package.json`).**

`better-sqlite3@11.5.0` publishes its own TypeScript declarations (`types: index.d.ts`), so the separate `@types/better-sqlite3` dependency adds avoidable drift/conflict risk.

```json
    "better-sqlite3": "^11.5.0",
    "`@types/better-sqlite3`": "^7.6.11",
```

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cueweb/package.json` at line 39, Remove the duplicate external typings by
deleting the "`@types/better-sqlite3`" dependency from package.json because
better-sqlite3@11.5.0 ships its own TypeScript declarations; update the
dependencies block to keep only "better-sqlite3": "^11.5.0", and run your
package manager (npm/yarn/pnpm) to reinstall/lockfile update so the dev
dependency is removed from node_modules and lockfile.
```

</details>

</blockquote></details>
<details>
<summary>docs/_docs/getting-started/deploying-cueweb.md-499-503 (1)</summary><blockquote>

`499-503`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_

**Fix invalid Compose command in CueWeb bootstrap reset step.**

`docker compose down cueweb` is invalid because `docker compose down` can’t take a service name; use a cueweb-scoped shutdown instead (e.g., `docker compose stop cueweb && docker compose rm -f cueweb`) before `docker volume rm <stack>_cueweb-data`, since the same Compose project also defines other services (db/kafka/etc.) and `docker compose down` would unnecessarily stop the whole stack.

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/_docs/getting-started/deploying-cueweb.md` around lines 499 - 503, The
current reset sequence uses an invalid and overly broad command `docker compose
down cueweb`; replace it with a cueweb-scoped shutdown that only stops and
removes the cueweb service: run `docker compose stop cueweb && docker compose rm
-f cueweb` (or equivalent) before `docker volume rm <stack>_cueweb-data`, then
bring cueweb back up with `docker compose up -d cueweb && docker compose logs
cueweb --tail 20`; update the documented commands in the CueWeb bootstrap reset
step accordingly so only the cueweb service is stopped/removed rather than the
whole Compose stack.
```

</details>

</blockquote></details>
<details>
<summary>docs/_docs/user-guides/cueweb-user-guide.md-121-129 (1)</summary><blockquote>

`121-129`: _⚠️ Potential issue_ | _🟡 Minor_ | _⚡ Quick win_

**Add a language identifier to the fenced code block.**

This block will fail markdown linting (`MD040`) as written. Please tag it explicitly (for example `text`).

 

<details>
<summary>Suggested doc fix</summary>

```diff
-   ```
+   ```text
    ========================================================================
    CueWeb bootstrap admin created (first-launch flow).
        username : admin
        password : <random 24-character string>
    Credentials were also written to /data/.cueweb-bootstrap (mode 0600). The
    password will be required to change on first login.
    ========================================================================
    ```
```
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @docs/_docs/user-guides/cueweb-user-guide.md around lines 121 - 129, The
fenced code block using triple backticks in the CueWeb bootstrap admin example
is missing a language identifier and triggers markdown lint rule MD040; update
the opening fence (the triple-backtick block shown around the "CueWeb bootstrap
admin created (first-launch flow)" message) to include an explicit language tag
such as text (e.g., change totext) so the code block is properly
identified for the linter and documentation rendering.

</details>

</blockquote></details>

</blockquote></details>

<details>
<summary>🧹 Nitpick comments (5)</summary><blockquote>

<details>
<summary>cueweb/lib/rbac/identity.ts (1)</summary><blockquote>

`38-42`: _⚡ Quick win_

**Consider stronger typing for OAuth identity parameters.**

The `any` types on the parameters reduce type safety and make it harder to catch errors at compile time. Consider defining explicit interfaces for the expected OAuth profile structures (Google, GitHub) to improve maintainability and IDE support.




<details>
<summary>♻️ Suggested type definitions</summary>

Define provider-specific interfaces at the top of the file:

```typescript
interface GoogleProfile {
  sub?: string;
  email?: string;
  name?: string;
}

interface GitHubProfile {
  id?: number | string;
  login?: string;
  email?: string;
  name?: string;
}

interface OAuthAccount {
  provider: string;
  providerAccountId?: string;
}
```

Then update the function signature:

```diff
-export function upsertProviderIdentity(args: {
-  account: any;
-  profile: any;
-  user: any;
-}): number | null {
+export function upsertProviderIdentity(args: {
+  account: OAuthAccount;
+  profile: GoogleProfile | GitHubProfile;
+  user: any;
+}): number | null {
```

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cueweb/lib/rbac/identity.ts` around lines 38 - 42, Replace the loose any
types on upsertProviderIdentity's args with explicit interfaces: define
provider-specific interfaces (e.g., GoogleProfile, GitHubProfile) and a generic
OAuthAccount (with provider and providerAccountId) and use them in the function
signature (args: { account: OAuthAccount; profile: GoogleProfile |
GitHubProfile; user: /* your User type */ }). Update any internal property
accesses in upsertProviderIdentity to reflect the new typed fields (provider,
providerAccountId, sub/id, email, name, login) so the compiler can validate
usage and improve IDE hints.
```

</details>

</blockquote></details>
<details>
<summary>cueweb/lib/rbac/local_provider.ts (1)</summary><blockquote>

`66-72`: **IP-based rate limiting has known limitations but is acceptable.**

The rate-limiting key is derived from `x-forwarded-for` or `x-real-ip` headers, which can be spoofed or absent. This is standard practice for IP-based rate limiting in proxied environments, though it has inherent limitations:

- Attackers can strip headers to share the "unknown" rate-limit bucket
- Multiple legitimate users behind the same NAT/proxy share one bucket

These are acceptable tradeoffs for brute-force protection. Consider documenting this behavior if rate-limiting becomes a pain point for legitimate users.

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cueweb/lib/rbac/local_provider.ts` around lines 66 - 72, The rate-limit key
currently derives from headers stored in the local variable ip and used to form
key = `local:${ip}`, which can be spoofed or absent; update the code to also
fall back to Express's req.ip (or equivalent request property) if
x-forwarded-for/x-real-ip are missing, normalize/truncate the extracted IP
string consistently, and add an inline comment above the ip/key logic
documenting the known limitations (header spoofing, shared NAT buckets, and the
"unknown" fallback) so future maintainers understand the trade-offs for this
brute-force protection.
```

</details>

</blockquote></details>
<details>
<summary>cueweb/components/rbac/use_roles.ts (1)</summary><blockquote>

`54-60`: _💤 Low value_

**Consider wrapping `authEnabled()` check in `useMemo`.**

`authEnabled()` is called directly during render without memoization. If this function is cheap (just reads env vars), this is fine. However, for consistency with the other hooks and to ensure stability, consider memoizing it.



<details>
<summary>Suggested change</summary>

```diff
 export function useIsAdmin(): boolean {
   const { data } = useSession();
-  // Sandbox mode: hide the Admin shortcut (there is no auth, so the
-  // admin UI is unreachable from the header by design).
-  if (!authEnabled()) return false;
-  return !!data?.user?.isAdmin;
+  return useMemo(() => {
+    // Sandbox mode: hide the Admin shortcut (there is no auth, so the
+    // admin UI is unreachable from the header by design).
+    if (!authEnabled()) return false;
+    return !!data?.user?.isAdmin;
+  }, [data?.user?.isAdmin]);
 }
```
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cueweb/components/rbac/use_roles.ts` around lines 54 - 60, The authEnabled()
call in useIsAdmin is invoked directly during render; wrap it in a React useMemo
to stabilize the value like other hooks: import useMemo, compute const
isAuthEnabled = useMemo(() => authEnabled(), []), replace the direct
authEnabled() check with if (!isAuthEnabled) return false, and keep the final
return as !!data?.user?.isAdmin to preserve behavior.
```

</details>

</blockquote></details>
<details>
<summary>cueweb/app/admin/users/page.tsx (1)</summary><blockquote>

`31-44`: _💤 Low value_

**Consider debouncing the search input.**

The current implementation triggers a fetch on every keystroke because `useEffect` depends on `refresh`, which depends on `q`. For an admin page with typically small datasets this is acceptable, but debouncing would reduce unnecessary API calls.

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cueweb/app/admin/users/page.tsx` around lines 31 - 44, The search currently
triggers refresh on every keystroke because refresh depends on q; debounce the
calls to the API by wrapping the query-triggering logic in a debounced function
(or debouncing updates to q) so fetch(`/api/admin/users?...`) is only called
after the user stops typing for a short delay. Update the React component to
create a debounced version of refresh (or a new debounced fetchUsers helper)
using useRef or a debounce utility (e.g., lodash.debounce), call that debounced
function from the effect or input change handler instead of calling refresh
directly, and ensure you cancel/cleanup the debounce on unmount; keep setUsers
and setRoles usage inside the debounced fetch logic so state updates remain the
same.
```

</details>

</blockquote></details>
<details>
<summary>cueweb/app/login/change-password/page.tsx (1)</summary><blockquote>

`36-40`: _💤 Low value_

**Form renders during session loading, causing a flash before redirect.**

When `status === "loading"`, the full form is rendered. If the user is actually unauthenticated, they'll see the password form briefly before being redirected to `/login`. Consider returning early with a loading indicator.


<details>
<summary>Suggested improvement</summary>

```diff
   React.useEffect(() => {
     if (status === "unauthenticated") {
       router.replace("/login");
     }
   }, [router, status]);

+  if (status === "loading" || status === "unauthenticated") {
+    return (
+      <div className="flex items-center justify-center h-screen">
+        <span className="text-sm text-muted-foreground">Loading…</span>
+      </div>
+    );
+  }
+
   async function submit(e: React.FormEvent) {
```
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cueweb/app/login/change-password/page.tsx` around lines 36 - 40, The
component currently renders the password form while session status is "loading",
causing a flash before redirect; modify the render logic to return early: if
status === "loading" render a loading indicator (or null) and do not render the
form, and if status === "unauthenticated" perform router.replace("/login") (as
already in React.useEffect) and return null immediately to avoid rendering the
form; update the component's top-level render/return to check the status
variable first (before JSX for the form) so the form only mounts when status ===
"authenticated".
```

</details>

</blockquote></details>

</blockquote></details>

<details>
<summary>🤖 Prompt for all review comments with AI agents</summary>

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @cueweb/app/admin/roles/page.tsx:

• Around line 60-69: The PATCH failure path currently continues and clears the
  draft and triggers refresh; modify the handler so that when res.ok is false you
  return early after alerting (do not call setEditing to remove editing[roleId]
  and do not call refresh), and only clear the draft (setEditing removing roleId)
  and call refresh when the PATCH succeeds; locate the failure branch that checks
  res.ok, the setEditing((e) => { const { [roleId]: _, ...rest } = e; return rest;
  }) call and the refresh() call and move/guard the latter two to execute only on
  success.

In @cueweb/app/api/admin/groups/[id]/roles/route.ts:

• Around line 82-87: DELETE currently skips validating the group and may return
  ok for invalid ids; call findGroupById(groupId) at the start of the DELETE
  handler (same check as GET/POST), return NextResponse.json({ error:
  "group_not_found" }, { status: 404 }) if not found, and only then call
  detachRoleFromGroup(...) and appendAudit(...). Ensure you perform the role
  lookup with findRoleByName(roleName) after validating the group and avoid
  writing audit entries for non-existent groups.

In @cueweb/app/api/admin/groups/[id]/route.ts:

• Around line 42-49: The delete and audit are separate DB writes (deleteGroup
  then appendAudit) which can leave the DB inconsistent; wrap both operations in a
  single database transaction or use a single atomic operation so either both
  succeed or both roll back. Locate deleteGroup and appendAudit in route.ts and
  refactor to run inside the same transaction context (or implement a new helper
  like deleteGroupWithAudit that accepts the transaction), perform the group
  deletion and insert the audit record within that transaction, and ensure errors
  roll back and propagate to the client.

In @cueweb/app/api/admin/users/[id]/password/route.ts:

• Around line 59-66: The password reset (setUserPassword) and audit insertion
  (appendAudit) must be performed atomically so a failed audit doesn't leave the
  password changed; update the handler to run both operations inside a single
  database transaction or a new transactional helper (e.g., create a function like
  resetPasswordWithAudit) that calls setUserPassword(userId, hash, true) and
  appendAudit({actorId: gate.userId, actorLabel: String(gate.userId), action:
  "user.password_reset", target: user:${userId}}) within the same transaction
  and rolls back if either write fails, or otherwise ensure the audit write is
  retried/compensated if your DB lacks transactions.

In @cueweb/lib/rbac/config.ts:

• Around line 35-49: rawList currently returns any trimmed string from
  NEXT_PUBLIC_AUTH_PROVIDER which lets typos like "locla" count as an enabled
  provider; update rawList to normalize and then filter values against a hardcoded
  allowlist (e.g., ["local","okta","ldap"]) so only known providers are returned,
  make authEnabled() rely on the filtered rawList() length, and ensure
  hasProvider(name) checks inclusion against that filtered list so only valid
  provider names are considered.

In @cueweb/lib/rbac/db/dal.ts:

• Around line 70-110: The upsertUser function currently does a check-then-insert
  (findUserByExternalId then INSERT) which races; change it to a single atomic
  upsert inside a transaction using SQLite's INSERT ... ON CONFLICT(external_id)
  DO UPDATE to ensure only one statement handles insert-or-update. Modify
  upsertUser to getDb(), begin a transaction, execute a single prepared statement
  like INSERT INTO users (...) VALUES (...) ON CONFLICT(external_id) DO UPDATE SET
  username=@username, email=@email, display_name=@displayName, source=@source,
  updated_at=strftime('%s','now') (and update password_hash/must_change_password
  as needed), then SELECT id FROM users WHERE external_id=@externalId to return
  the id, and commit; keep references to upsertUser, findUserByExternalId (remove
  its pre-check), getDb and use param names from the current run object.
• Around line 467-468: The audit payload serialization must be made safe so
  JSON.stringify on input.before and input.after cannot throw (e.g., due to
  circular refs or BigInt); replace the direct JSON.stringify calls for
  input.before and input.after with a safe serializer (or wrap each stringify in
  try/catch) that returns null or a fallback string on error (or converts BigInt
  to string and handles cycles), ensuring the audit insert in
  cueweb/lib/rbac/db/dal.ts never crashes the mutation path when serializing
  input.before/input.after.

In @cueweb/lib/rbac/db/index.ts:

• Around line 56-72: The code caches _db before running runMigrations and
  seedBuiltinRoles, which can leave _db pointing to a partially initialized DB on
  error; change initialization so you create the Database instance (const db = new
  Database(...)) but do NOT assign _db or _initialized until after
  runMigrations(db) and seedBuiltinRoles(db) complete successfully; implement this
  by using a local variable (e.g., db) to run the setup, then on success set _db =
  db and _initialized = true, and wrap the migration/seed steps in a try/catch
  that closes db (db.close() or equivalent) and rethrows on failure to avoid
  leaking a bad cached handle.

In @cueweb/lib/rbac/rate_limit.ts:

• Around line 31-60: Store currently retains expired Window entries
  indefinitely; update recordFailedAttempt and isBlocked to evict expired entries
  from the Map when they are observed (i.e., if w.resetAt <= now then
  store.delete(key) before creating a fresh window or returning not-blocked), and
  add a lightweight periodic scavenger (e.g., setInterval) that iterates the store
  and deletes entries whose resetAt <= Date.now() to prevent unbounded memory
  growth; reference the store Map and the functions recordFailedAttempt and
  isBlocked so changes are applied where keys are read/created and implement the
  periodic cleanup in the same module.

In @cueweb/lib/rbac/resolvers/ldap.ts:

• Around line 70-77: Move the client.bind(...) call inside the same try/finally
  that calls client.unbind() so unbind runs even if bind fails: wrap the bind
  promise (the existing await new Promise((resolve, reject) =>
  client.bind(...))) together with the subsequent client.search logic inside the
  try block, and keep client.unbind() in the finally block; modify the code around
  client.bind, client.search and the try/finally so any rejection from client.bind
  still triggers the finally path and calls client.unbind().

In @docs/_docs/user-guides/cueweb-user-guide.md:

• Line 133: Update the sentence that advises removing the cueweb-data volume
  to include an explicit, prominent warning that deletion is destructive and will
  erase all persisted RBAC, authentication, and application data causing
  irreversible loss and possible production lockout; reference the cueweb-data
  volume and the "first-launch" flow so readers know this will reset the system
  and advise taking backups or performing this only in non-production/test
  environments or with administrator approval.

---

Minor comments:
In @cueweb/app/admin/admins/page.tsx:

• Around line 25-28: The refresh function's fetch currently calls .json()
  unguarded and can leave admins in an inconsistent state on network or non-OK
  responses; wrap the fetch and parsing in try/catch, check response.ok before
  calling response.json(), and on any failure log the error (e.g., console.error)
  and setAdmins([]) or keep previous state as appropriate; update the refresh
  callback (the async function that calls fetch("/api/admin/admins") and
  setAdmins) to handle non-OK responses and exceptions so admins is
  deterministically reset or preserved.

In @cueweb/app/admin/groups/page.tsx:

• Around line 69-85: attachRole and detachRole currently ignore network/server
  failures; update both functions (attachRole, detachRole) to handle errors by
  checking the fetch response (response.ok) and treating non-OK as failures: await
  response, parse error text/json if present, show a user-visible error (e.g.
  alert or toast) instead of always calling refresh, and wrap the fetch in
  try/catch to surface network exceptions similarly; only call refresh() on
  success and ensure the error path logs the error (console.error) and notifies
  the user.

In @cueweb/app/admin/layout.tsx:

• Line 7: Remove the unused Link import: delete the import of Link from
  "next/link" in the file (the symbol Link in layout.tsx) so the module only
  imports what is actually used; ensure no other references to Link remain in the
  file after removal.

In @cueweb/app/api/admin/audit/route.ts:

• Around line 40-42: The actor value is inserted into the SQL LIKE pattern
  without escaping '%' and '' allowing users to inject wildcard matches; add an
  escape helper (e.g., escapeLike) that replaces /[%]/ with escaped versions,
  call it on actor before constructing the pattern, and then push the escaped
  pattern (like %${escapedActor}%) into args where clauses.push("actor_label
  LIKE ?") is used so the LIKE treats literal characters instead of wildcards.

In @cueweb/app/api/admin/groups/[id]/route.ts:

• Around line 25-27: The current validation converts id to Number (groupId) and
  only checks Number.isFinite, which allows non-integer and non-positive values;
  change the check in the handler that computes groupId to require a positive
  integer by validating Number.isInteger(groupId) && groupId > 0 and return the
  same NextResponse.json({ error: "invalid_id" }, { status: 400 }) when that
  fails; update the validation around the variables id and groupId in route.ts so
  only positive integers are accepted.

In @cueweb/app/api/admin/users/[id]/password/route.ts:

• Around line 50-56: The handler currently coerces arbitrary JSON into a string
  via const password = String(body.password ?? "") which can accept non-string
  payloads; instead, first verify that body.password is a string (e.g., typeof
  body.password === "string") and return the 400 NextResponse.json error if it is
  missing or not a string, then perform the MIN_LEN check (using the existing
  MIN_LEN symbol) and only thereafter use the validated string for further
  processing; update the code paths that reference password to rely on this
  validated string rather than blind coercion.

In @cueweb/app/api/admin/users/[id]/roles/route.ts:

• Around line 73-98: The DELETE handler must verify the target user exists
  before calling detachRoleFromUser and recording an audit. After parsing userId
  (from params) and resolving role (using findRoleByName), look up the user using
  the same helper used in GET/POST (e.g., findUserById(userId) or the project's
  getUserById helper); if the lookup returns null/undefined return
  NextResponse.json({ error: "user_not_found" }, { status: 404 }) and do not call
  detachRoleFromUser or appendAudit; only perform detachRoleFromUser(userId,
  role.id) and appendAudit(...) when the user lookup succeeds. Ensure you use the
  existing symbols requireAdmin, findRoleByName, detachRoleFromUser, appendAudit,
  and NextResponse to implement this check.

In @cueweb/package.json:

• Line 39: Remove the duplicate external typings by deleting the
  "@types/better-sqlite3" dependency from package.json because
  better-sqlite3@11.5.0 ships its own TypeScript declarations; update the
  dependencies block to keep only "better-sqlite3": "^11.5.0", and run your
  package manager (npm/yarn/pnpm) to reinstall/lockfile update so the dev
  dependency is removed from node_modules and lockfile.

In @cueweb/README.md:

• Line 281: The README lists supported auth providers inconsistently; update the
  provider list mentions at the spots referencing NEXT_PUBLIC_AUTH_PROVIDER
  (currently lines describing "local/okta/ldap") to match the actual
  implementation and other README references by including google and github (so
  the documented comma-separated identifiers include local, okta, ldap, google,
  github), or alternatively remove google/github everywhere if you intend to drop
  them; ensure all README occurrences that enumerate providers use the exact same
  set of identifiers (NEXT_PUBLIC_AUTH_PROVIDER values) so the docs and
  implementation are consistent.

In @docs/_docs/developer-guide/cueweb-development.md:

• Line 341: The fenced code block that begins with "cueweb/" is missing a
  language identifier which triggers markdownlint MD040; add an explicit language
  tag (for example "text" or "bash") immediately after the opening backticks for
  that block so it becomes text (or bash) to suppress MD040 and ensure
  proper syntax highlighting for the directory tree.

In @docs/_docs/getting-started/deploying-cueweb.md:

• Around line 499-503: The current reset sequence uses an invalid and overly
  broad command docker compose down cueweb; replace it with a cueweb-scoped
  shutdown that only stops and removes the cueweb service: run docker compose stop cueweb && docker compose rm -f cueweb (or equivalent) before docker volume rm <stack>_cueweb-data, then bring cueweb back up with docker compose up -d cueweb && docker compose logs cueweb --tail 20; update the documented
  commands in the CueWeb bootstrap reset step accordingly so only the cueweb
  service is stopped/removed rather than the whole Compose stack.

In @docs/_docs/user-guides/cueweb-user-guide.md:

• Around line 121-129: The fenced code block using triple backticks in the
  CueWeb bootstrap admin example is missing a language identifier and triggers
  markdown lint rule MD040; update the opening fence (the triple-backtick block
  shown around the "CueWeb bootstrap admin created (first-launch flow)" message)
  to include an explicit language tag such as text (e.g., change totext)
  so the code block is properly identified for the linter and documentation
  rendering.

---

Nitpick comments:
In @cueweb/app/admin/users/page.tsx:

• Around line 31-44: The search currently triggers refresh on every keystroke
  because refresh depends on q; debounce the calls to the API by wrapping the
  query-triggering logic in a debounced function (or debouncing updates to q) so
  fetch(/api/admin/users?...) is only called after the user stops typing for a
  short delay. Update the React component to create a debounced version of refresh
  (or a new debounced fetchUsers helper) using useRef or a debounce utility (e.g.,
  lodash.debounce), call that debounced function from the effect or input change
  handler instead of calling refresh directly, and ensure you cancel/cleanup the
  debounce on unmount; keep setUsers and setRoles usage inside the debounced fetch
  logic so state updates remain the same.

In @cueweb/app/login/change-password/page.tsx:

• Around line 36-40: The component currently renders the password form while
  session status is "loading", causing a flash before redirect; modify the render
  logic to return early: if status === "loading" render a loading indicator (or
  null) and do not render the form, and if status === "unauthenticated" perform
  router.replace("/login") (as already in React.useEffect) and return null
  immediately to avoid rendering the form; update the component's top-level
  render/return to check the status variable first (before JSX for the form) so
  the form only mounts when status === "authenticated".

In @cueweb/components/rbac/use_roles.ts:

• Around line 54-60: The authEnabled() call in useIsAdmin is invoked directly
  during render; wrap it in a React useMemo to stabilize the value like other
  hooks: import useMemo, compute const isAuthEnabled = useMemo(() =>
  authEnabled(), []), replace the direct authEnabled() check with if
  (!isAuthEnabled) return false, and keep the final return as
  !!data?.user?.isAdmin to preserve behavior.

In @cueweb/lib/rbac/identity.ts:

• Around line 38-42: Replace the loose any types on upsertProviderIdentity's
  args with explicit interfaces: define provider-specific interfaces (e.g.,
  GoogleProfile, GitHubProfile) and a generic OAuthAccount (with provider and
  providerAccountId) and use them in the function signature (args: { account:
  OAuthAccount; profile: GoogleProfile | GitHubProfile; user: /* your User type */
  }). Update any internal property accesses in upsertProviderIdentity to reflect
  the new typed fields (provider, providerAccountId, sub/id, email, name, login)
  so the compiler can validate usage and improve IDE hints.

In @cueweb/lib/rbac/local_provider.ts:

• Around line 66-72: The rate-limit key currently derives from headers stored in
  the local variable ip and used to form key = local:${ip}, which can be spoofed
  or absent; update the code to also fall back to Express's req.ip (or equivalent
  request property) if x-forwarded-for/x-real-ip are missing, normalize/truncate
  the extracted IP string consistently, and add an inline comment above the ip/key
  logic documenting the known limitations (header spoofing, shared NAT buckets,
  and the "unknown" fallback) so future maintainers understand the trade-offs for
  this brute-force protection.

</details>

<details>
<summary>🪄 Autofix (Beta)</summary>

Fix all unresolved CodeRabbit comments on this PR:

- [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended)
- [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes

</details>

---

<details>
<summary>ℹ️ Review info</summary>

<details>
<summary>⚙️ Run configuration</summary>

**Configuration used**: defaults

**Review profile**: CHILL

**Plan**: Pro

**Run ID**: `2a0b5169-20b2-4c22-8408-a9b17c5ea5b3`

</details>

<details>
<summary>📥 Commits</summary>

Reviewing files that changed from the base of the PR and between 9635c05780d25f445dbf8673ad5f2f6247393a88 and 558023e8db27f1278faede1e3f5763ed39e27666.

</details>

<details>
<summary>⛔ Files ignored due to path filters (1)</summary>

* `cueweb/package-lock.json` is excluded by `!**/package-lock.json`

</details>

<details>
<summary>📒 Files selected for processing (74)</summary>

* `cueweb/Dockerfile`
* `cueweb/README.md`
* `cueweb/__tests__/__mocks__/server-only.ts`
* `cueweb/__tests__/rbac/bootstrap.test.ts`
* `cueweb/__tests__/rbac/db.test.ts`
* `cueweb/__tests__/rbac/rate_limit.test.ts`
* `cueweb/__tests__/rbac/resolvers.test.ts`
* `cueweb/app/admin/admins/page.tsx`
* `cueweb/app/admin/audit/page.tsx`
* `cueweb/app/admin/groups/page.tsx`
* `cueweb/app/admin/layout.tsx`
* `cueweb/app/admin/page.tsx`
* `cueweb/app/admin/permissions/page.tsx`
* `cueweb/app/admin/roles/page.tsx`
* `cueweb/app/admin/tabs.tsx`
* `cueweb/app/admin/users/page.tsx`
* `cueweb/app/api/admin/admins/[userId]/route.ts`
* `cueweb/app/api/admin/admins/route.ts`
* `cueweb/app/api/admin/audit/route.ts`
* `cueweb/app/api/admin/groups/[id]/roles/route.ts`
* `cueweb/app/api/admin/groups/[id]/route.ts`
* `cueweb/app/api/admin/groups/route.ts`
* `cueweb/app/api/admin/permissions/route.ts`
* `cueweb/app/api/admin/roles/[id]/route.ts`
* `cueweb/app/api/admin/roles/route.ts`
* `cueweb/app/api/admin/users/[id]/password/route.ts`
* `cueweb/app/api/admin/users/[id]/roles/route.ts`
* `cueweb/app/api/admin/users/[id]/route.ts`
* `cueweb/app/api/admin/users/route.ts`
* `cueweb/app/api/me/password/route.ts`
* `cueweb/app/login/change-password/page.tsx`
* `cueweb/app/login/page.tsx`
* `cueweb/app/utils/menus.ts`
* `cueweb/components/rbac/index.ts`
* `cueweb/components/rbac/require_feature.tsx`
* `cueweb/components/rbac/use_roles.ts`
* `cueweb/components/ui/app-header.tsx`
* `cueweb/instrumentation.node.ts`
* `cueweb/instrumentation.ts`
* `cueweb/jest.config.js`
* `cueweb/lib/auth.ts`
* `cueweb/lib/rbac/bootstrap.ts`
* `cueweb/lib/rbac/config.ts`
* `cueweb/lib/rbac/db/dal.ts`
* `cueweb/lib/rbac/db/index.ts`
* `cueweb/lib/rbac/db/migrations.ts`
* `cueweb/lib/rbac/db/migrations/0001_initial.sql`
* `cueweb/lib/rbac/db/migrations_data.ts`
* `cueweb/lib/rbac/db/types.ts`
* `cueweb/lib/rbac/identity.ts`
* `cueweb/lib/rbac/local_provider.ts`
* `cueweb/lib/rbac/permissions.ts`
* `cueweb/lib/rbac/rate_limit.ts`
* `cueweb/lib/rbac/require_feature.ts`
* `cueweb/lib/rbac/resolvers/index.ts`
* `cueweb/lib/rbac/resolvers/ldap.ts`
* `cueweb/lib/rbac/resolvers/none.ts`
* `cueweb/lib/rbac/resolvers/okta.ts`
* `cueweb/lib/rbac/resolvers/types.ts`
* `cueweb/lib/rbac/roles.ts`
* `cueweb/lib/rbac/seed.ts`
* `cueweb/middleware.ts`
* `cueweb/next.config.js`
* `cueweb/package.json`
* `cueweb/types/next-auth.d.ts`
* `docker-compose.yml`
* `docs/_docs/concepts/cueweb-rest-gateway.md`
* `docs/_docs/developer-guide/cueweb-development.md`
* `docs/_docs/getting-started/deploying-cueweb.md`
* `docs/_docs/other-guides/cueweb.md`
* `docs/_docs/quick-starts/quick-start-cueweb.md`
* `docs/_docs/reference/cueweb.md`
* `docs/_docs/tutorials/cueweb-tutorial.md`
* `docs/_docs/user-guides/cueweb-user-guide.md`

</details>

<details>
<summary>✅ Files skipped from review due to trivial changes (6)</summary>

* cueweb/jest.config.js
* cueweb/__tests__/__mocks__/server-only.ts
* cueweb/app/api/admin/permissions/route.ts
* cueweb/instrumentation.ts
* cueweb/lib/rbac/resolvers/index.ts
* cueweb/types/next-auth.d.ts

</details>

</details>

<!-- This is an auto-generated comment by CodeRabbit for review status -->

[ramonfigueiredo]

@DiegoTavares / @lithorus
Ready for review!

[coderabbitai]

🧹 Nitpick comments (2)

cueweb/app/api/admin/groups/[id]/roles/route.ts (1)

60-68: 💤 Low value

Consider wrapping role attach/detach + audit in runInTransaction for consistency.

The other admin routes (groups/[id]/route.ts, users/[id]/password/route.ts) now use runInTransaction() to ensure the mutation and audit persist atomically. While these operations are lower-risk (the role change succeeds even if audit fails), wrapping them maintains consistent audit guarantees across all admin endpoints.

♻️ Proposed refactor

+import {
+  appendAudit,
+  attachRoleToGroup,
+  detachRoleFromGroup,
+  findGroupById,
+  findRoleByName,
+  listRolesForGroup,
+  runInTransaction,
+} from "`@/lib/rbac/db/dal`";
...
-  attachRoleToGroup(groupId, role.id);
-  appendAudit({
-    actorId: gate.userId,
-    actorLabel: String(gate.userId),
-    action: "group.role_attach",
-    target: `group:${groupId}`,
-    after: { roleName: role.name },
-  });
+  runInTransaction(() => {
+    attachRoleToGroup(groupId, role.id);
+    appendAudit({
+      actorId: gate.userId,
+      actorLabel: String(gate.userId),
+      action: "group.role_attach",
+      target: `group:${groupId}`,
+      after: { roleName: role.name },
+    });
+  });
...
-  detachRoleFromGroup(groupId, role.id);
-  appendAudit({
-    actorId: gate.userId,
-    actorLabel: String(gate.userId),
-    action: "group.role_detach",
-    target: `group:${groupId}`,
-    before: { roleName: role.name },
-  });
+  runInTransaction(() => {
+    detachRoleFromGroup(groupId, role.id);
+    appendAudit({
+      actorId: gate.userId,
+      actorLabel: String(gate.userId),
+      action: "group.role_detach",
+      target: `group:${groupId}`,
+      before: { roleName: role.name },
+    });
+  });

Also applies to: 89-97

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cueweb/app/api/admin/groups/`[id]/roles/route.ts around lines 60 - 68, Wrap
the role attach/detach and their corresponding audit calls in runInTransaction
so the mutation and appendAudit are atomic: replace the standalone calls to
attachRoleToGroup(groupId, role.id) and appendAudit(...) (and similarly the
detachRoleFromGroup + appendAudit block around lines handling role removal) with
a single runInTransaction(async () => { await attachRoleToGroup(...); await
appendAudit(...); }) (and analogous for detach) ensuring you await both
operations inside the transaction and return the NextResponse.json({ ok: true })
only after the transaction completes.

cueweb/app/admin/roles/page.tsx (1)

72-80: 💤 Low value

Return early on delete failure for consistency.

The same pattern fixed in savePerms should apply here: refresh() is called unconditionally at line 79, even when the delete fails. This causes an unnecessary UI refresh after a failed operation.

Suggested fix

   async function deleteRole(roleId: number) {
     if (!confirm("Delete this role?")) return;
     const res = await fetch(`/api/admin/roles/${roleId}`, { method: "DELETE" });
     if (!res.ok) {
       const b = await res.json().catch(() => ({}));
       alert(b.error || "Failed.");
+      return;
     }
-    refresh();
+    await refresh();
   }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cueweb/app/admin/roles/page.tsx` around lines 72 - 80, In deleteRole, when
the DELETE response is not ok you should return early instead of calling
refresh() unconditionally; after parsing the error body and showing
alert(b.error || "Failed."), add a return so that refresh() only runs on
success—refer to the deleteRole function and the refresh() call for the exact
locations to change, mirroring the savePerms early-return pattern.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@cueweb/app/admin/roles/page.tsx`:
- Around line 72-80: In deleteRole, when the DELETE response is not ok you
should return early instead of calling refresh() unconditionally; after parsing
the error body and showing alert(b.error || "Failed."), add a return so that
refresh() only runs on success—refer to the deleteRole function and the
refresh() call for the exact locations to change, mirroring the savePerms
early-return pattern.

In `@cueweb/app/api/admin/groups/`[id]/roles/route.ts:
- Around line 60-68: Wrap the role attach/detach and their corresponding audit
calls in runInTransaction so the mutation and appendAudit are atomic: replace
the standalone calls to attachRoleToGroup(groupId, role.id) and appendAudit(...)
(and similarly the detachRoleFromGroup + appendAudit block around lines handling
role removal) with a single runInTransaction(async () => { await
attachRoleToGroup(...); await appendAudit(...); }) (and analogous for detach)
ensuring you await both operations inside the transaction and return the
NextResponse.json({ ok: true }) only after the transaction completes.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 8d97e3f9-d4ff-49b5-90ad-7bf6a6426837

📥 Commits

Reviewing files that changed from the base of the PR and between 558023e and 91ccdda.

📒 Files selected for processing (15)

• cueweb/README.md
• cueweb/app/admin/roles/page.tsx
• cueweb/app/api/admin/groups/[id]/roles/route.ts
• cueweb/app/api/admin/groups/[id]/route.ts
• cueweb/app/api/admin/users/[id]/password/route.ts
• cueweb/lib/rbac/config.ts
• cueweb/lib/rbac/db/dal.ts
• cueweb/lib/rbac/db/index.ts
• cueweb/lib/rbac/rate_limit.ts
• cueweb/lib/rbac/resolvers/ldap.ts
• docs/_docs/concepts/cueweb-rest-gateway.md
• docs/_docs/getting-started/deploying-cueweb.md
• docs/_docs/quick-starts/quick-start-cueweb.md
• docs/_docs/reference/cueweb.md
• docs/_docs/user-guides/cueweb-user-guide.md

✅ Files skipped from review due to trivial changes (1)

• cueweb/README.md