[cueweb] Add Monitor Cue page (CueCommander parity)#2423
Open
ramonfigueiredo wants to merge 38 commits into
Open
[cueweb] Add Monitor Cue page (CueCommander parity)#2423ramonfigueiredo wants to merge 38 commits into
ramonfigueiredo wants to merge 38 commits into
Conversation
…parity) Wire the existing CueCommander "Services" menu item to a new /services page mirroring CueGUI's "Facility Service Defaults" tab (ServiceDialog): a left pane listing facility default services and a right-pane edit form. (In CueGUI the Services tab is the Facility Service Defaults tab, so the page lives at /services where the sidebar item already points.) - Form fields match CueGUI: Name, Threadable, Min/Max Threads (100 = 1 thread), Min Memory MB, Min Gpu Memory MB, Timeout, Timeout LLU, OOM Increase MB, predefined Tags (two-column order) plus a Custom Tags toggle. Memory converts MB<->KB (x1024); threads stored as cores*100. - Save validates (name length/charset, non-negative numbers, min <= max threads when max > 0, OOM > 0, tag charset) then shows a facility-wide confirmation before Create (new) or Update (existing). New / Del in the left pane (Del confirms first). - Proxy routes under app/api/service/: getdefaultservices, create, update, delete. Service type + getDefaultServices in get_utils; create/update/ deleteService in action_utils.
Add a /stuck-frames route listing frames in RUNNING state longer than a configurable threshold, with per-row Retry and Kill actions. - Threshold slider (1-48h, default 8h) persisted to localStorage; filtering is client-side so the slider is instant. - Server-side aggregation route /api/stuck-frames lists unfinished jobs (GetJobs) and fans out GetFrames per job filtered to RUNNING (FrameState 2), so the browser makes a single request. - Retry/Kill reuse retryFrames/killFrames; kill records the signed-in user in the reason. Polls every 30s.
Replace the running-time MVP with CueGUI's StuckFramePlugin behavior: - Detection predicate (LLU / % stuck / avg-completion / runtime / >500s, %stuck<1.1) applied live client-side. - Multi-service filter bar (catch-all + per-service filters) with % Run Since LLU, Min LLU, % Avg Completion, Total Runtime, Exclude Keywords, Enable; CueGUI service defaults; persisted to localStorage. - Job-grouped table: Name, Comment, Frame, Host, LLU, Runtime, % Stuck, Average, Last Line (lazy rqlog tail). - Frame menu: Tail/View/Last Log, Retry/Eat/Kill, Log + Log-and-X, Frame Not Stuck, Add/Exclude Job, Core Up, View Host. Job menu: View Comments, Job Not Stuck, Add/Exclude Job, Core Up. Auto-refresh, Notification, Refresh, Clear. - Aggregation route attaches service/avgFrameSec/layer via GetLayers; add /api/stuck-frames/lastline and /api/layer/action/setmincores.
Extend /hosts to match CueGUI's Monitor Hosts window: - Full column set with Swap/Physical/GPU/Temp red/green bars, Load %, GPU + Temp Free columns, comment icon, and row coloring by hardware/ lock state (new SimpleDataTable getRowClassName hook). - Filter bar: name/regex + Filter Allocation/HardwareState/LockState/OS multi-selects, Auto-refresh, Refresh, Clear. - Context menu: Comments, View Procs, Lock/Unlock, Edit Tags, Rename Tag, Change Allocation, Reboot, Reboot when idle, Delete Host, Set/Clear Repair State (Take Ownership shown disabled). - New dialogs: host Comments (view/add), Rename Tag, Change Allocation, Delete confirm. - Bottom Proc monitor panel (View Procs) with View Job / Unbook / Kill / Unbook and Kill. - Routes: host renametag/setallocation/delete/sethardwarestate/addcomment, proc kill/unbookone, proc getprocs. Extended Host/Proc types + actions.
Add the /monitor-cue route (previously a dead sidebar link) replicating CueGUI's Monitor Cue window: - "Shows" multi-select (All Shows / Clear / per-show, persisted) drives a show-grouped job table. - Full column set: Run, Cores, Gpus, Wait, Depend, Total, Min, Max, Min G, Max G, Pri, ETA, MaxRss, MaxGpuMem, Age, Progress (frame-state colored bar). Row coloring by paused/dead/depend/waiting. - Toolbar: Eat / Retry / Pause / Unpause / Kill (confirm) on selected jobs, Refresh + Auto-refresh (5s), Expand/Collapse All, Select search + Clr + selectMine. - Reuses the existing JobContextMenu (+ job action dialogs) for the per-job right-click menu and JobProgressBar for Progress.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds Monitor Cue job monitoring, preview and dialog plumbing, frame log improvements, API route coverage for frame/job/layer actions, and sandbox Blender preview tooling. ChangesCueWeb monitoring, APIs, dialogs, and preview system
Sandbox Blender preview rendering and tooling
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/monitor-cue/page.tsx`:
- Around line 134-137: The persistShows callback currently calls
window.localStorage.setItem directly which can throw (QuotaExceededError) and
break selection; wrap the localStorage.setItem(SELECTED_SHOWS_KEY,
JSON.stringify(names)) call in a try/catch inside persistShows (keep the
existing setSelectedShows(names) call), and in the catch log a concise warning
(e.g., console.warn) with the error so the UI still updates even if storage
fails.
- Line 399: The context-menu open callback currently forces a cast ({ original:
j } as unknown as Row<Job>) and the Unmonitor flow uses
rowSelection/setRowSelection that are no-ops, causing stale selected state; fix
by (1) removing the unsafe cast: either change JobContextMenu and helpers (e.g.,
any usages that expect Row<Job>) to accept a minimal shape like { original: Job
} or construct a proper Row<Job>-compatible object for contextMenuHandleOpen so
code reads row.original without a cast, and (2) wire the real rowSelection and
setRowSelection through monitor-cue/page.tsx into the context menu and into
unmonitorJobGivenRow so Unmonitor updates the page’s selected state (ensure
setRowSelection mutates the same selection state used by the table). Use the
symbols contextMenuHandleOpen, JobContextMenu, unmonitorJobGivenRow,
rowSelection, and setRowSelection to locate and update the code.
- Around line 442-456: The JobContextMenu selection props are stubbed out which
prevents unmonitorJobGivenRow from updating the page-local selected Set<string>;
wire the component to the page's actual selection state by passing the page's
selected state and setter into JobContextMenu (replace rowSelection={{}} with
rowSelection={selected} and replace setRowSelection={() => {}} with
setRowSelection={setSelected} or an equivalent wrapper) so unmonitorJobGivenRow
can remove the id from selected and the "{selected.size} selected" count updates
correctly; alternatively, ensure unmonitorJobGivenRow directly updates the
page's selected via setSelected when removing an id.
🪄 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: 1b6150cb-9bfc-40ed-a8a1-bcebbd780ced
📒 Files selected for processing (1)
cueweb/app/monitor-cue/page.tsx
…cements Bring the Cuetopia tables to CueGUI parity across the Job, Layer and Frame right-click menus, add the supporting REST-gateway routes and dialogs, and extend the frame log viewer (search, follow/tail, line numbers, per-line copy, preview). Also adds a Blender render demo to the sandbox. 1) Job menu - [x] Set User Color paints the whole row with CueGUI's 15 default swatches plus a bright palette (app/utils/user_colors.ts); - [x] Change Comments in a new tab to Comments popup (job-comments-dialog.tsx); - [x] New auto-eat column; - [x] Set Min/Max Cores - [x] Set Min/Max GPUs - [x] Max Retries - [x] Use Local Cores - [x] Reorder - [x] Stagger via job-extra-dialogs.tsx; - [x] Show Progress Bar. This option shows a configurable command. Layer menu (layer-extra-dialogs.tsx + routes) - [x] View Layer - [x] View Dependencies - [x] Dependency Wizard (LAYER_ON_LAYER) - [x] Mark done - [x] Reorder - [x] Stagger - [x] Properties (min cores/memory/gpu-memory, threadable, tags) - [x] Eat and Mark done - [x] View Processes. 2) Frame menu (frame-extra-dialogs.tsx + routes) - [x] View Host - [x] View Dependencies - [x] Dependency Wizard (FRAME_ON_FRAME) - [x] Drop depends - [x] Mark as waiting - [x] Mark done - [x] Filter Selected Layers - [x] Reorder - [x] Preview All (external viewer configurable) - [x] Eat and Mark done - [x] View Processes. - [x] Frame range selector (frame-range-selector.tsx): drag / shift-click to select a contiguous range and Retry / Eat / Kill it. 3) Frame log viewer (app/frames/[frame-name]/page.tsx) - [x] Search bar with highlight, n/total counter, Enter/Shift+Enter navigation, case + regex toggles (frame-log-search.tsx). Follow tail mode: - [x] Auto-scroll on new lines - [x] Pause on scroll-up - [x] Jump to bottom - [x] "Tail Log" action opens it by default (last 200 lines, 1s poll). - [x] Absolute line numbers + hover/click per-line copy with toast. - [x] Frame preview thumbnail viewer (frame-preview-panel.tsx + /api/frame/preview + preview_utils.ts) 4) Fixes - [x] Correct gateway package on the layer/frame createdepend* routes (/layer.* and /frame.* -> /job.LayerInterface and /job.FrameInterface); depend creation now succeeds. - [x] Mount DependencyWizardDialog once per page (was opening twice on Monitor Jobs); Add an initialType open option. New API routes Frame: - [x] getdepends - [x] dropdepends - [x] markaswaiting - [x] preview Job: - [x] addrenderpart - [x] markdoneframes - [x] reorderframes - [x] staggerframes - [x] setmingpus - [x] setmaxgpus Layer: - [x] getdepends - [x] getoutputpaths - [x] markdone - [x] reorderframes - [x] staggerframes - [x] setmincores - [x] setminmemory - [x] setmingpumemory - [x] settags - [x] setthreadable Config Dockerfile + docker-compose.yml: - [x] NEXT_PUBLIC_CUEPROGBAR_COMMAND - [x] NEXT_PUBLIC_PREVIEW_COMMAND - [x] NEXT_PUBLIC_PREVIEW_URL build args + runtime env. Sandbox load_test_jobs.py: - [x] New `blender` subcommand renders a real image sequence and registers the layer output path so the frame preview has real frames - [x] Cross-platform Blender discovery (macOS/Windows/Linux) - [x] Standalone render_blender_demo.py wrapper - [x] README updated.
…election Bring the CueCommander Monitor Cue table and its right-click menu closer to CueGUI/CueCommander. Right-click job menu - Add "Send To Group..." (Monitor Cue only) with a group picker dialog (send-to-group-dialog.tsx) that reparents the job via reparentJobs. - Merge "Auto-Eat On/Off" into a single state-aware toggle: "Enable auto eating" / "Disable auto eating". - Rename "Unbook..." to "Unbook Frames..." (CueGUI naming). - Move "Set Priority..." after the cores/gpus setters (CueGUI order). - Monitor Cue only (hidden on Cuetopia/Monitor Jobs): Unmonitor, Set User Color / Clear User Color, Use Local Cores, and the resource/priority/unbook setters (Set Min/Max Cores, Set Minimum/Maximum Cores, Set Minimum/Maximum Gpus, Set Priority, Unbook Frames) - matching where CueGUI exposes them. - Toolbar action buttons now show icons (eat/retry/pause/unpause/kill). Table - Comment + auto-eat columns matching Monitor Jobs (amber sticky-note, yellow Pacman), and a "Readable Age" column. - Booking Bar column (job-booking-bar.tsx) mirroring CueGUI's JobBookingBarDelegate: running/waiting bar with full-height min (cyan) and max (red) core markers. - Sortable columns (asc/desc) with header arrows. - Show/hide + reorder columns via a Columns dropdown, plus a "Filter jobs..." search box, both at the top-right of the table (persisted to localStorage). - Select-all header checkbox; Shift+click range selection of rows. - CueGUI row tint: blue=paused, red=dead, yellow=high maxRss, green=waiting, purple=all-depend. Removed the white-on-hover row highlight. - "Select:" (name/regex) box now selects matching jobs live as you type. Fixes - Kill failed on Monitor Cue in no-auth mode: fall back to UNKNOWN_USER (not "") so the username-required kill request validates. - Mount JobExtraDialogs / JobCommentsDialog / SendToGroupDialog on the page so every job-menu action (Set Min/Max Cores, Reorder, Comments, etc.) works.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cueweb/app/monitor-cue/page.tsx (1)
307-317:⚠️ Potential issue | 🟠 Major | ⚡ Quick winListen for
cueweb:refresh-nowso Send-To-Group updates immediately.
cueweb/components/ui/send-to-group-dialog.tsxdispatches this event after a successful move, but this page never subscribes, so the table can stay stale (notably when auto-refresh is off).Suggested fix
React.useEffect(() => { let cancelled = false; const isCancelled = () => cancelled; load(selectedShows, isCancelled); @@ }, [selectedShows, autoRefresh, load]); + React.useEffect(() => { + const onRefreshNow = () => { + void load(selectedShows); + }; + window.addEventListener("cueweb:refresh-now", onRefreshNow); + return () => window.removeEventListener("cueweb:refresh-now", onRefreshNow); + }, [load, selectedShows]);🤖 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/monitor-cue/page.tsx` around lines 307 - 317, The page's useEffect currently only triggers periodic or selection-based loads and doesn't subscribe to the "cueweb:refresh-now" DOM event, so add a listener inside the same React.useEffect that calls the existing load(selectedShows, isCancelled) when the event fires (use the same isCancelled closure), and remove the listener on cleanup; ensure the listener is registered regardless of autoRefresh so Send-To-Group dispatches cause an immediate reload, and keep the existing interval/cleanup logic (symbols: load, selectedShows, isCancelled, autoRefresh, REFRESH_MS).
🧹 Nitpick comments (1)
cueweb/app/jobs/columns.tsx (1)
230-232: ⚡ Quick winUse a shared event-name constant for job comments open.
This emitter uses a hardcoded
"cueweb:open-job-comments"string while the listener side is constant-based. Please centralize the event name in one exported constant to prevent silent breakage from future string drift.🤖 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 230 - 232, Replace the hardcoded "cueweb:open-job-comments" string with a shared exported constant: add an exported constant (e.g. export const OPEN_JOB_COMMENTS_EVENT = "cueweb:open-job-comments") to your shared events/constants module, import that constant where the listener is defined and where the emitter is defined (the window.dispatchEvent(new CustomEvent(...)) call in columns.tsx), and use the constant in the CustomEvent constructor and the listener registration so both sides reference the same symbol.
🤖 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/frame/action/getdepends/route.ts`:
- Around line 39-40: Return the upstream HTTP status in the response object
instead of embedding it in the JSON body: where you currently do "if
(!response.ok) return NextResponse.json({ error: responseData.error, status:
response.status }); return NextResponse.json({ data: responseData.data, status:
responseData.status });" change both calls to set the HTTP status via
NextResponse.json(..., { status: response.status }) and remove the "status"
field from the JSON body (e.g., return NextResponse.json({ error:
responseData.error }, { status: response.status }) for errors and return
NextResponse.json({ data: responseData.data }, { status: response.status }) for
success). Apply the same edit pattern to the corresponding code in markaswaiting
(the identical NextResponse.json usage).
In `@cueweb/app/api/frame/preview/route.ts`:
- Around line 89-94: The error responses currently include the server filesystem
path via the variable `normalized`; remove `path: normalized` from the client
JSON payloads and return only a generic error message (or a generated/request
correlation id) instead. Update each NextResponse.json call that currently
returns `{ error: "...", path: normalized }` to return `{ error: "File not
found" }`, `{ error: "Permission denied" }`, or `{ error: "Could not read image"
}` (or `{ error: "...", requestId }` if you add a correlation id generated with
crypto.randomUUID() or taken from an incoming header like `x-request-id`). Keep
logging of `normalized` to the server logs (e.g., console.error or your existing
logger) so the path remains available for diagnostics but is not exposed to the
client.
- Around line 58-67: Require configured preview roots and replace the brittle
normalized.includes("..") check with path.resolve/path.relative validation: if
allowedRoots() returns an empty array return a 403 (fail closed); for each root
compute resolvedRoot = path.resolve(root) and resolvedTarget =
path.resolve(target) and use const rel = path.relative(resolvedRoot,
resolvedTarget) to allow the request only when rel === "" or
(!rel.startsWith("..") && !path.isAbsolute(rel)); remove the
normalized.includes("..") logic and the previous equality/startsWith comparison,
and keep using NextResponse.json to return the same 403 error messages when
validation fails.
In `@cueweb/app/api/job/action/setmaxgpus/route.ts`:
- Around line 37-39: For each proxy route
(cueweb/app/api/job/action/setmaxgpus/route.ts 37-39,
cueweb/app/api/job/action/setmingpus/route.ts 37-39,
cueweb/app/api/job/action/staggerframes/route.ts 37-39) guard the upstream body
parsing by wrapping the await response.json() call in a try/catch; on success
use the parsed body as before, but on failure fall back to a generic object
(e.g. { error: 'Upstream returned non-JSON or empty body' }) so you can still
forward the upstream HTTP status and a meaningful error, and ensure the
subsequent NextResponse.json(...) calls use that fallback when parsing fails.
In `@cueweb/app/api/layer/action/getdepends/route.ts`:
- Around line 30-40: For both sites, guard parsing of the incoming JSON by
wrapping await request.json() in a try/catch and return NextResponse.json({
error: 'Invalid JSON' }, { status: 400 }) on parse error; then validate the
parsed object (e.g., ensure parsed.layer exists) before continuing. Call
handleRoute(method, endpoint, body, true) as before, then when mapping the
upstream response use the upstream HTTP status as the actual response status: on
error return NextResponse.json({ error: responseData.error }, { status:
response.status }) and on success return NextResponse.json({ data:
responseData.data }, { status: response.status }). Apply these exact changes in
the two files: cueweb/app/api/layer/action/getdepends/route.ts (lines 30-40) and
cueweb/app/api/layer/action/getoutputpaths/route.ts (lines 31-42).
In `@cueweb/app/api/layer/action/markdone/route.ts`:
- Around line 38-39: The wrapper currently places HTTP status inside the JSON
body, causing NextResponse to always return 200; update both responses to pass
the status as the second argument to NextResponse.json instead of only in the
body. Concretely, replace the error return with NextResponse.json({ error:
responseData.error }, { status: response.status }) and the success return with
NextResponse.json({ data: responseData.data }, { status: responseData.status ??
response.status }) so the HTTP status is preserved while keeping body fields
clean (use symbols: response, responseData, NextResponse.json).
- Around line 29-33: The request JSON parsing can throw and should return a 400
instead of bubbling to a 500: wrap the parsing logic around request.json() and
JSON.parse in a try/catch (the block that currently does const body =
JSON.stringify(await request.json()); const jsonBody = JSON.parse(body);), and
on any error return NextResponse.json({ error: 'Invalid request body' }, {
status: 400 }); then continue to validate that jsonBody is an object and has
.layer as before.
In `@cueweb/app/api/layer/action/setminmemory/route.ts`:
- Around line 33-35: The request-body validation currently allows negative and
fractional memory values; update the conditional that checks jsonBody.memory to
also ensure it's an integer and non-negative (e.g., replace the memory check
with typeof jsonBody.memory === 'number' && Number.isFinite(jsonBody.memory) &&
Number.isInteger(jsonBody.memory) && jsonBody.memory >= 0) so the handler
rejects values like -1 or 1.5 and returns the same 400 response when the check
fails.
In `@cueweb/app/api/layer/action/settags/route.ts`:
- Around line 33-35: The request body check only verifies that jsonBody.tags is
an array but not that every element is a string; update the validation in the
route (the jsonBody check around jsonBody?.layer and
Array.isArray(jsonBody.tags)) to also assert tags.every(t => typeof t ===
'string') and return the same 400 NextResponse when that check fails so the
payload matches the declared tags: string[] contract.
In `@cueweb/app/frames/`[frame-name]/page.tsx:
- Around line 412-430: The current useEffect rebuilds a decoration for every
line (using model.getLineCount, monaco.Range and ed.deltaDecorations) on each
logContentVersion change, which is O(total lines); instead, only create/update
glyph decorations for the currently visible lines and update them incrementally:
move creation out of the heavy effect, register handlers using
editor.onDidScrollChange and editor.onDidChangeModelContent to compute
editor.getVisibleRanges(), build decorations only for those visible line numbers
(use copyGlyphDecorationsRef and ed.deltaDecorations to replace the small set),
and on model content edits expand/contract the visible-range decorations rather
than rebuilding for model.getLineCount; keep existing identifiers (useEffect,
editorRef, monacoRef, copyGlyphDecorationsRef, deltaDecorations) and remove the
full-model loop so updates are proportional to viewport size.
- Around line 385-401: The follow-mode polling can start overlapping tick
executions (the setInterval in the useEffect) causing concurrent calls to
loadNewerLogMessages() and duplicate appends; modify tick to serialize
executions by introducing an in-progress guard: add a local boolean like
isTickRunning (closed over in the effect) and at the start of tick return early
if cancelled or isTickRunning or no editorRef, set isTickRunning = true before
awaiting loadNewerLogMessages(), and in a finally block set isTickRunning =
false (while still honoring cancelled checks before calling
scrollToVeryBottom()); keep the existing cancelled and clearInterval logic and
the same dependencies but ensure only one loadNewerLogMessages() runs at a time.
In `@cueweb/app/utils/user_colors.ts`:
- Around line 106-119: The readableTextColor function uses a fixed luminance
cutoff which can choose a lower-contrast foreground; update
readableTextColor(hex: string) to compute the pixel luminance as currently done,
then compute WCAG contrast ratios against black and white using
(Llighter+0.05)/(Ldarker+0.05) where Lblack=0 and Lwhite=1, compare the two
contrast ratios and return "`#000000`" or "`#ffffff`" whichever yields the higher
contrast (keep the existing invalid-input behavior of returning "inherit"). This
replaces the hard-coded luminance > 0.45 decision in readableTextColor with a
contrast-ratio comparison to ensure the chosen foreground actually maximizes
legibility.
In `@cueweb/components/ui/frame-extra-dialogs.tsx`:
- Around line 183-200: The confirm() function currently lets errors from
dropFramesDepends, markFramesAsWaiting, markdoneFrames, and eatAndMarkdoneFrames
bubble as unhandled rejections; add a catch block to the existing try/finally
that calls handleError(err) (and optionally shows a user toast) before
rethrowing or returning, ensuring setBusy(false) still runs in finally and
maintaining existing checks that call toastWarning and setOpen(false) on
success; reference the confirm function and use handleError to surface any
thrown errors from those async calls while preserving setBusy and setOpen
behavior.
In `@cueweb/components/ui/frame-preview-panel.tsx`:
- Around line 52-83: The resolve(Frame) async handler can race: capture a
per-request sequence id (e.g., requestIdRef via useRef<number>) and increment it
at the start of resolve, storing the current id in a local const (currentId);
after each await (after getLayersForJob, fetchLayerOutputPaths, and before any
setState calls including setCandidates, setWebCandidates, setWebIdx, setStatus
and in the catch block) check that requestIdRef.current === currentId and return
early if it differs so stale completions cannot overwrite newer previews;
implement the same check for all completion paths within resolve (including the
catch) and for any other similar async preview handlers to ensure out-of-order
responses are ignored.
In `@cueweb/components/ui/job-comments-dialog.tsx`:
- Around line 96-99: The handler function currently assumes the event is a
well-formed CustomEvent and directly accesses (e as
CustomEvent<OpenJobCommentsDetail>).detail.job; modify handler to first guard
that e is a CustomEvent with a truthy detail and a detail.job before
dereferencing: cast to CustomEvent only after checking 'detail' exists (or check
'if ("detail" in e && e instanceof CustomEvent && e.detail && e.detail.job)')
and only then call setJob(j) and setSelectedId(null); if the guard fails, return
early (optionally log a warning) so a malformed event cannot throw inside
handler.
In `@cueweb/components/ui/job-extra-dialogs.tsx`:
- Around line 269-286: The apply function builds an addRenderPartition payload
using Number(...) and Math.* directly, which can produce NaN for empty/invalid
inputs; before calling addRenderPartition (in apply) parse and validate each
numeric input (cores, memGb, gpus, gpuMemGb) with Number(...) or parseFloat,
check isFinite/!isNaN and apply safe defaults or show toastWarning and return on
invalid input, then compute threads = Math.max(1, parsedCores), maxCores =
Math.max(1, parsedCores), maxMemory = Math.round(parsedMemGb * KB_PER_GB)
(ensure parsedMemGb >= 0), maxGpus = Math.max(0, parsedGpus), maxGpuMemory =
Math.round(parsedGpuMemGb * KB_PER_GB) (ensure >= 0); update apply to
validate/normalize these values before building the payload and only call
addRenderPartition when all numeric fields are valid.
In `@sandbox/load_test_jobs.py`:
- Around line 874-876: The code does a one-shot lookup with
_find_job(short_name) which can race Cuebot visibility; instead, poll until the
job is resolvable before accessing layers and calling layer.registerOutputPath.
Replace the direct _find_job call with the existing polling helper used
elsewhere in the repo (reuse the project's "wait/until job resolvable" helper)
to repeatedly call _find_job(short_name) with a short delay and timeout, then
after the helper returns a non-None job proceed to find the "beauty" layer via
job.getLayers() and call layer.registerOutputPath(output_spec).
---
Outside diff comments:
In `@cueweb/app/monitor-cue/page.tsx`:
- Around line 307-317: The page's useEffect currently only triggers periodic or
selection-based loads and doesn't subscribe to the "cueweb:refresh-now" DOM
event, so add a listener inside the same React.useEffect that calls the existing
load(selectedShows, isCancelled) when the event fires (use the same isCancelled
closure), and remove the listener on cleanup; ensure the listener is registered
regardless of autoRefresh so Send-To-Group dispatches cause an immediate reload,
and keep the existing interval/cleanup logic (symbols: load, selectedShows,
isCancelled, autoRefresh, REFRESH_MS).
---
Nitpick comments:
In `@cueweb/app/jobs/columns.tsx`:
- Around line 230-232: Replace the hardcoded "cueweb:open-job-comments" string
with a shared exported constant: add an exported constant (e.g. export const
OPEN_JOB_COMMENTS_EVENT = "cueweb:open-job-comments") to your shared
events/constants module, import that constant where the listener is defined and
where the emitter is defined (the window.dispatchEvent(new CustomEvent(...))
call in columns.tsx), and use the constant in the CustomEvent constructor and
the listener registration so both sides reference the same symbol.
🪄 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: 181510c9-49b2-41cb-b635-c27932d85733
📒 Files selected for processing (56)
cueweb/Dockerfilecueweb/app/api/frame/action/createdependonframe/route.tscueweb/app/api/frame/action/createdependonjob/route.tscueweb/app/api/frame/action/createdependonlayer/route.tscueweb/app/api/frame/action/dropdepends/route.tscueweb/app/api/frame/action/getdepends/route.tscueweb/app/api/frame/action/markaswaiting/route.tscueweb/app/api/frame/preview/route.tscueweb/app/api/job/action/addrenderpart/route.tscueweb/app/api/job/action/markdoneframes/route.tscueweb/app/api/job/action/reorderframes/route.tscueweb/app/api/job/action/setmaxgpus/route.tscueweb/app/api/job/action/setmingpus/route.tscueweb/app/api/job/action/staggerframes/route.tscueweb/app/api/layer/action/createdependonframe/route.tscueweb/app/api/layer/action/createdependonjob/route.tscueweb/app/api/layer/action/createdependonlayer/route.tscueweb/app/api/layer/action/createframebyframedepend/route.tscueweb/app/api/layer/action/getdepends/route.tscueweb/app/api/layer/action/getoutputpaths/route.tscueweb/app/api/layer/action/markdone/route.tscueweb/app/api/layer/action/reorderframes/route.tscueweb/app/api/layer/action/setmincores/route.tscueweb/app/api/layer/action/setmingpumemory/route.tscueweb/app/api/layer/action/setminmemory/route.tscueweb/app/api/layer/action/settags/route.tscueweb/app/api/layer/action/setthreadable/route.tscueweb/app/api/layer/action/staggerframes/route.tscueweb/app/frames/[frame-name]/page.tsxcueweb/app/frames/frame-columns.tsxcueweb/app/globals.csscueweb/app/jobs/[job-name]/page.tsxcueweb/app/jobs/columns.tsxcueweb/app/jobs/data-table.tsxcueweb/app/monitor-cue/page.tsxcueweb/app/utils/action_utils.tscueweb/app/utils/preview_utils.tscueweb/app/utils/user_colors.tscueweb/components/ui/context_menus/action-context-menu.tsxcueweb/components/ui/dependency-wizard-dialog.tsxcueweb/components/ui/frame-extra-dialogs.tsxcueweb/components/ui/frame-log-search.tsxcueweb/components/ui/frame-preview-panel.tsxcueweb/components/ui/frame-range-selector.tsxcueweb/components/ui/job-booking-bar.tsxcueweb/components/ui/job-comments-dialog.tsxcueweb/components/ui/job-details-inline.tsxcueweb/components/ui/job-extra-dialogs.tsxcueweb/components/ui/layer-extra-dialogs.tsxcueweb/components/ui/send-to-group-dialog.tsxcueweb/components/ui/toast-host.tsxcueweb/components/ui/toolbar.tsxdocker-compose.ymlsandbox/README.mdsandbox/load_test_jobs.pysandbox/render_blender_demo.py
✅ Files skipped from review due to trivial changes (3)
- cueweb/components/ui/toolbar.tsx
- cueweb/components/ui/toast-host.tsx
- sandbox/README.md
…ltered selection
Fixes from code review on the Monitor Hosts work.
Host action routes
- addcomment, delete, renametag, setallocation, sethardwarestate: return handleRoute(...) directly instead of re-reading + re-wrapping its response. handleRoute already returns the final {data}/{error} NextResponse, so the extra unwrap was redundant and flattened error/status propagation.
- sethardwarestate: validate `state` against the full host.HardwareState enum (UP, DOWN, REBOOTING, REBOOT_WHEN_IDLE, REPAIR) and 400 on anything else, instead of only checking it's a string.
Change Allocation dialog (host-monitor-dialogs.tsx)
- Reset allocs/allocId/busy when the dialog reopens so a fast OK before getAllocations() resolves can't move the new hosts to the previous allocation (OK stays disabled until fresh data loads).
Proc monitor panel
- Add a request-sequence token so out-of-order proc loads can't clobber a newer host scope (quick A -> B switch no longer leaves the table on A).
Frame range selector
- Add a keyboard-accessible "Select range" (From #/To #) numeric input so keyboard/AT users can build a contiguous multi-frame selection without the pointer-only strip; feeds the same Retry/Eat/Kill buttons.
SimpleDataTable
- Feed the frame range selector the table's filtered row model so the strip mirrors the "Filter frames..." text filter, not just the state chips - range actions can no longer target rows the user filtered out.
…and filter persistence
Bring the /hosts page closer to CueGUI parity with ownership actions, a dedicated comments column, a full-featured comments dialog, tag improvements, and shareable filters.
Ownership
- Add app/api/host/action/takeownership -> /host.OwnerInterface/TakeOwnership ({ owner: { name }, host }).
- Implement takeHostOwnership() and takeOwnershipGivenRow().
- Add HostTakeOwnershipDialog to claim selected hosts for the current NextAuth user and refresh the rows.
- Enable "Take Ownership" only for NIMBY_LOCKED hosts (matching CueGUI's canTakeOwnership behavior); otherwise show it disabled.
Comments column
- Add a dedicated sortable comments column with StickyNote header and amber comment indicator cells.
- Remove the inline comment icon from the Name column.
Host comments dialog
- Replace the basic add/list/delete UI with a CueGUI-style dialog featuring:
- Subject / User / Date table with row selection.
- Sanitized markdown preview (react-markdown + rehype-sanitize).
- New, Save, Delete, and Close actions.
- Editing allowed only for comments authored by the current user; other comments are read-only.
- Save through /api/comment/action/save.
- Per-comment delete support; removing the last comment clears the row indicator.
- Add predefined comment macros stored in localStorage:
- Macro picker dropdown.
- Add / Edit / Delete macro dialogs.
- Consistent author information derived from the NextAuth session.
- Update deleteJobComment() to return performAction()'s success boolean.
Rename Tag refinements
- Restrict renameHostTag() to selected hosts that actually contain the old tag.
- Skip unrelated hosts and warn when no matching hosts are found.
- Populate the tag selector with the union of tags across all selected hosts.
Filter persistence
- Persist Allocation, HardwareState, LockState, OS filters, and name search in the URL query string (?q, &alloc, &hw, &lock, &os).
- Initialize filter state from URL parameters and update them as filters change.
- Clear URL parameters when filters are reset.
- Wrap the page in a Suspense boundary to support useSearchParams while preserving static prerendering.
These changes bring Monitor Hosts closer to CueGUI behavior and improve usability, discoverability, and shareability of host views.
…ng them Address review feedback on the Facility Service Defaults page: - get_utils.ts: getDefaultServices now throws on a non-array response (mirroring getHosts) instead of collapsing a failed load into []. A backend outage reaches the page's catch/error state rather than falsely rendering "No services defined". - service-defaults-form.tsx: require non-negative integers for the numeric fields (Min/Max Threads, Min Memory, Min GPU Memory, Timeout, Timeout LLU, OOM Increase). They map to integer-backed proto fields and CueGUI uses integer spin boxes, so fractional input like 1.5 is now rejected up front. - service-defaults-form.tsx / services/page.tsx: throw from the confirm handlers when createService / updateService / deleteService return false, so ConfirmDialog keeps the modal open for retry instead of dismissing as if the action had succeeded. The error toast is still surfaced by the helpers.
…e load failures Address review feedback on the Stuck Frames endpoint and helper: - route.ts: cap the per-job GetFrames/GetLayers fan-out with a fixed-size worker pool (MAX_CONCURRENT_JOBS = 16) instead of Promise.all over every unfinished job, so a large farm no longer fires hundreds/thousands of concurrent gateway calls on each 30s poll. - route.ts: paginate GetFrames via getRunningFrames(), looping pages until a short page arrives (bounded by MAX_FRAME_PAGES) instead of fetching only page 1. Jobs with more than MAX_FRAMES_PER_JOB running frames are no longer silently truncated, so Retry/Kill stay available for every row. - get_utils.ts: getStuckFrames now throws on a non-array response (mirroring getHosts) instead of collapsing a failed fetch into []. A backend outage reaches the page's catch/error path rather than rendering the empty state.
- page: key job grouping, hide, Core Up and comment links by jobId instead of jobName, so two jobs sharing a name aren't merged or acted on together. - page / action_utils: retryFrames / eatFrames / killFrames now return a success boolean, and act() only hides the frame + reloads when the action succeeded (a failed Retry/Eat/Kill no longer removes the frame from view). - stuck-frame-filters: ignore non-finite numeric input so a NaN can't poison filter thresholds or persisted state; don't add an empty service filter when all services are taken (and disable the add button in that case).
- host-monitor-dialogs: handleSave now captures the add/save result and skips the refresh + optimistic hasComment dispatch when the action fails, so a rejected comment no longer updates the UI as if it succeeded. - action_utils: saveJobComment returns performAction's success boolean (was void) so the save path can be gated; existing callers that ignore it are unaffected.
- page: auto-refresh notification now fires only when stuck frames are actually present. load() returns the fetched data so the interval applies the same detection + hidden filters and notifies with the count, instead of firing on every scan. - stuck-frame-filters: the service dropdown only offers the row's own service plus unused ones, preventing duplicate service-specific filters. - stuck-frame-filters: add aria-label to the icon-only add/remove filter buttons for screen-reader support.
API routes - frame/preview: harden the path check with path.resolve + path.relative boundary validation (replacing the brittle "..".includes test), and stop echoing the resolved server path in error responses (log it server-side instead). Document the optional CUEWEB_PREVIEW_ROOTS allow-list in .env.example. - frame/getdepends, frame/markaswaiting, layer/getdepends, layer/getoutputpaths, layer/markdone: return the upstream HTTP status via the response init instead of embedding it in the JSON body, so failures aren't reported as HTTP 200. - layer/getoutputpaths, layer/markdone: guard request.json() parsing and return 400 on malformed JSON. - layer/setminmemory: require a non-negative integer (KB). - layer/settags: validate every tags element is a string. UI - monitor-cue: wrap the show-selection localStorage write in try/catch so a quota error can't break selection. - frames viewer: serialize follow-mode polling so a slow fetch can't overlap and duplicate appended log chunks; decorate only visible lines with the copy glyph (bounded by viewport instead of O(total loaded lines)). - frame-preview-panel: add a request-sequence token so a stale resolve() can't overwrite a newer frame preview. - frame-range-selector: make the ARIA slider focusable and keyboard-operable (arrows / Shift+arrows / Home / End / Escape) with aria-valuetext. - job-comments-dialog: guard the custom-event payload before dereferencing. - job-extra-dialogs: validate cores/memory/GPU inputs before building the addRenderPartition payload so NaN can't reach the API. - user_colors: pick readable text color by WCAG contrast ratio rather than a fixed luminance cutoff. sandbox - load_test_jobs, render_blender_demo: poll via _wait_for_jobs after launch instead of a one-shot _find_job, so registerOutputPath isn't skipped on a transient post-submit lookup race.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/frame/preview/route.ts`:
- Line 61: The security issue is that path.resolve() performs lexical path
resolution without following symlinks, but fs.readFile() does follow symlinks.
This allows a symlink under an allowed preview root to point outside the root
while still passing the containment check using path.relative(). Replace the
path.resolve(target) call with fs.realpathSync(target) to obtain the canonical
path that follows symlinks. Apply the same change to all other locations where
normalized paths are created for containment checking (the comment indicates
this applies at lines 66-70 and 87), and ensure the containment validation
compares canonical realpath values for both the root and target paths to
properly prevent symlink-based directory traversal attacks.
🪄 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: 3951142d-ff6e-4c45-9c1f-593a89f234cd
📒 Files selected for processing (18)
cueweb/.env.examplecueweb/app/api/frame/action/getdepends/route.tscueweb/app/api/frame/action/markaswaiting/route.tscueweb/app/api/frame/preview/route.tscueweb/app/api/layer/action/getdepends/route.tscueweb/app/api/layer/action/getoutputpaths/route.tscueweb/app/api/layer/action/markdone/route.tscueweb/app/api/layer/action/setminmemory/route.tscueweb/app/api/layer/action/settags/route.tscueweb/app/frames/[frame-name]/page.tsxcueweb/app/monitor-cue/page.tsxcueweb/app/utils/user_colors.tscueweb/components/ui/frame-preview-panel.tsxcueweb/components/ui/frame-range-selector.tsxcueweb/components/ui/job-comments-dialog.tsxcueweb/components/ui/job-extra-dialogs.tsxsandbox/load_test_jobs.pysandbox/render_blender_demo.py
✅ Files skipped from review due to trivial changes (1)
- cueweb/.env.example
🚧 Files skipped from review as they are similar to previous changes (12)
- cueweb/app/api/frame/action/markaswaiting/route.ts
- cueweb/app/api/layer/action/markdone/route.ts
- cueweb/app/api/layer/action/getoutputpaths/route.ts
- cueweb/components/ui/frame-preview-panel.tsx
- cueweb/app/api/layer/action/settags/route.ts
- sandbox/load_test_jobs.py
- cueweb/app/api/layer/action/getdepends/route.ts
- cueweb/app/api/frame/action/getdepends/route.ts
- cueweb/components/ui/job-extra-dialogs.tsx
- cueweb/app/utils/user_colors.ts
- sandbox/render_blender_demo.py
- cueweb/app/monitor-cue/page.tsx
path.resolve is lexical but fs.readFile follows symlinks, so a symlink inside an allowed root could resolve outside it and still pass the containment check. - Resolve the target and configured roots with fs.realpath; use the canonical path for the boundary check, format check, and read. - Treat unresolvable roots as non-matching (fail closed). - Missing/permission errors now surface at realpath with the same responses.
- lastline route: canonicalize the requested path with realpath (replacing the brittle ".." substring check), re-check the .rqlog extension on the real file, and restrict reads to an optional CUEWEB_LOG_ROOTS allow-list when set (unset = unrestricted, best-effort). Documented in .env.example. - setmincores route: reject non-finite (NaN/Infinity) and negative cores; keep fractional values allowed since the proto field is a float. - page: guard the auto-refresh interval with an in-flight flag so a slow backend can't pile up overlapping scans. - page: add an aria-label to the comments icon button for screen readers.
Collaborator
Author
|
Note: The CueWeb docs will be updated on a new PR or later in this PR. |
API routes - frame/preview: block SVG (drop from web-renderable set + MIME) so it's never served same-origin as executable content; canonicalize the path with realpath before the root check (defeats symlink escape) and stop leaking the resolved path in error responses. Document optional CUEWEB_PREVIEW_ROOTS in .env.example. - frame/getdepends, frame/markaswaiting, layer/getdepends, layer/getoutputpaths, layer/markdone: guard request.json() parsing (400 on malformed JSON) and return the upstream HTTP status via the response init instead of the body. UI - frames viewer: serialize newer-log appends with a shared in-flight guard so the follow-mode poll and scroll loader can't double-append; decorate only visible lines with the copy glyph (bounded by viewport, not O(total lines)). - frame-preview-panel: request-sequence token so a stale resolve() can't overwrite a newer frame preview. - frame-range-selector: make the ARIA slider focusable and keyboard-operable (arrows / Shift+arrows / Home / End / Escape) with aria-valuetext. - monitor-cue: request-id token so a slower load() can't overwrite newer table data; re-anchor Shift-selection when the previous anchor is no longer visible. - job-extra-dialogs: integer-only validation for GPU/retry scalar fields and the render-partition cores/GPU inputs (per int32 proto); cores stay fractional. - send-to-group-dialog: staleness token so a stale group load can't apply to a newer job context. sandbox - load_test_jobs, render_blender_demo: poll via _wait_for_jobs after launch instead of a one-shot _find_job, so registerOutputPath isn't skipped on a post-submit lookup race.
…://github.com/ramonfigueiredo/OpenCue into cueweb/job-layer-frame-actions-and-log-viewer
…-and-log-viewer # Conflicts: # cueweb/app/api/frame/preview/route.ts # cueweb/app/api/layer/action/getdepends/route.ts # cueweb/components/ui/frame-range-selector.tsx # cueweb/components/ui/job-extra-dialogs.tsx
- setmincores route: validate layer is an object with a non-empty string id (reject { layer: {} }) alongside the existing non-negative cores check.
- stuck-frames route: guard gateway payloads with Array.isArray before spreading/iterating (frames, jobs, layers), so a malformed upstream response degrades to empty instead of 500-ing the whole route.
- page: surface an explicit error state instead of rendering "no stuck frames" when a load fails with no data; key the log-export buckets by jobId (not jobName) so same-named jobs aren't merged; gate Core Up close/refresh on all layer updates succeeding.
- User guide: Add user-facing Facility Service Defaults section with screenshots - Reference: Add technical entry (routes, RPCs, MB/KB and units, validation) - Developer guide: Add CueCommander-parity architecture section - Other guides: Add overview entry - Add CueCommander Facility Services screenshots (light + dark variants)
…roup, View Filters, Tasks)
Add a right-click menu on Monitor Cue show rows, porting the CueGUI CueCommander Monitor Cue show menu.
- Menu (monitor-cue-show-menu.tsx): Show Properties / Service Properties / Group Properties / Task Properties / View Filters / Create Group, plus Change Cuewho / View Cuewho. Implemented items open their dialogs; the pending ones (Service Properties, Cuewho) surface a "not implemented yet" toast.
- Group Properties + Create Group (group-properties-dialog, create-group-dialog, shared group-form): edit/create the root group's defaults via GroupInterface setters; toggle fields with CueGUI's sentinel/clamp semantics; Department is a dropdown from GetDepartmentNames. Proxies: group/action/{createsubgroup,update}.
- View Filters (view-filters-dialog): three-panel editor (filters / matchers / actions) with full Filter/Matcher/Action RPC coverage, all enums, and per-action-type value editing (incl. memory GB <-> KB). Proxies: show/getfilters, filter/{getmatchers,getactions,mutate}.
- Task Properties (task-properties-dialog): department selector, managed toggle, managed cores, and the task list with Set Min Cores / Clear Adjustment / Delete. Proxies: show/getdepartments, department/gettasks, task/mutate.
- Show Properties: toast "No changes to save." instead of closing silently.
- get_utils / action_utils: Group/Filter/Matcher/Action/Department/Task types, read helpers, and group/filter/task mutate helpers.
- rest_gateway: register the previously-omitted DepartmentInterface handler so the department dropdown and tasks reach Cuebot through the gateway.
# Conflicts: # cueweb/app/utils/action_utils.ts
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (2)
cueweb/app/utils/get_utils.ts (2)
452-470: ⚡ Quick winFix Content-Type header to match JSON body.
The function sends a JSON body via
JSON.stringify({})but declaresContent-Type: application/x-www-form-urlencoded. This is misleading; the correct header for JSON isapplication/json. While Next.js API routes likely auto-parse the body regardless, the mismatch harms code clarity.✏️ Suggested fix
const response = await fetch(`${base}/api/department/getdepartmentnames`, { method: "POST", - headers: { "Content-Type": "application/x-www-form-urlencoded" }, + headers: { "Content-Type": "application/json" }, body: JSON.stringify({}), });Note: This same Content-Type mismatch exists in
accessGetApi(not changed in this PR), so a broader refactor to fix all occurrences would be ideal.🤖 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/get_utils.ts` around lines 452 - 470, In the getDepartmentNames function, the fetch request sends a JSON body via JSON.stringify({}) but declares an incorrect Content-Type header of application/x-www-form-urlencoded. Change the Content-Type header value to application/json to correctly match the JSON body format being transmitted. This fix ensures the header accurately describes the request content type and improves code clarity.
367-372: Both callers ofgetShowRootGroupexplicitly handle the null case, so the fallback doesn't cause the masked behavior described in the original review.The function's logic to find a root group is sound, and both calling sites (
group-properties-dialog.tsxandcreate-group-dialog.tsx) checkif (!root)before proceeding. The fallback togroups[0]is defensive—comments elsewhere ingroup-tree.tsx(lines 172–173, 190–191) document the assumption that "a show always has a root." The backend has a dedicatedGetRootGroupRPC endpoint, reinforcing this expectation.However, if a root group truly goes missing in the data, silently returning the first group instead of null could obscure the problem. Consider logging a warning or explicitly handling the case where no proper root is found, to make data integrity issues more visible during debugging.
🤖 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/get_utils.ts` around lines 367 - 372, In the getShowRootGroup function, add a warning log when the fallback to groups[0] is triggered (i.e., when no root group with level 0 or no parentId is found). This warning should be logged before returning groups[0], so that any data integrity issues where a proper root group is missing become visible during debugging instead of being silently masked by the defensive fallback logic.
🤖 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/department/gettasks/route.ts`:
- Around line 29-31: The JSON parsing on line 29 when calling request.json() can
throw an error if the JSON is malformed, which bypasses your validation and
returns a 500 error instead of the intended 400 error. Wrap the JSON parsing
logic (the JSON.stringify(await request.json()) and JSON.parse(body) calls) in a
try-catch block, and in the catch block return a 400 status code with an
appropriate error message for invalid JSON input before proceeding to the
validation checks on line 31.
In `@cueweb/app/api/filter/getactions/route.ts`:
- Around line 29-31: The await request.json() call on line 29 can throw an
exception when the request body contains invalid JSON, which will result in an
unhandled 500 error instead of the expected 400 status code for input validation
errors. Wrap the JSON parsing logic (the request.json() and JSON.parse(body)
calls) in a try-catch block, and when a parsing error is caught, return a 400
status response with an appropriate error message to maintain API contract
consistency where input validation errors always return 400, not 500.
In `@cueweb/app/api/filter/getmatchers/route.ts`:
- Around line 29-31: The JSON parsing logic in the getmatchers route handler may
throw an exception if the request body contains malformed JSON, which results in
a 500 error instead of the appropriate 400 error for invalid input. Wrap the
JSON parsing operations (the await request.json() call and subsequent
JSON.parse() on line 29-30) in a try-catch block before the body-shape
validation check on line 31, and return a 400 response when a JSON parsing error
is caught, so that malformed JSON is handled gracefully as a client error rather
than an unhandled server error.
In `@cueweb/app/api/filter/mutate/route.ts`:
- Line 70: The current return statement in the NextResponse.json call returns
raw RPC objects from responseData.data when present, which may not include a
proper success flag, causing action_utils.filterMutate to incorrectly evaluate
the result. Modify the return statement to ensure a stable success flag is
always included in the response data by explicitly checking the response.status
code (typically 2xx for success) and always including success: boolean in the
returned data structure, rather than relying on the fallback pattern that can
expose incomplete RPC objects.
In `@cueweb/app/api/group/action/createsubgroup/route.ts`:
- Line 52: The response returned by the `createSubGroup` route handler is
missing the `success` field that callers expect, which can cause the success
status to be misread as failure. Modify the return statement to include both the
`data` and a `success` field in the response object, where `success` should be
derived from the response status (typically checking if the status is in the 200
range).
In `@cueweb/app/api/group/action/update/route.ts`:
- Around line 74-77: The loop that iterates through Object.keys(changes)
silently skips unknown keys by continuing when a setter is not found in the
SETTERS dictionary. Instead of silently ignoring these unknown keys with the
continue statement, throw an error or reject the request when an unknown key is
encountered. This will prevent the endpoint from returning success while
silently dropping invalid changes, which helps catch contract regressions and
user-edit failures. Replace the continue statement in the setter lookup logic
with appropriate error handling that explicitly rejects the request when an
unsupported changes key is provided.
In `@cueweb/app/api/show/getdepartments/route.ts`:
- Around line 29-31: The JSON parsing operation on line 29 where
`JSON.stringify(await request.json())` is called can throw an exception if the
request body contains malformed JSON, which will result in an unhandled 500
error instead of a proper 400 client error. Wrap the JSON parsing logic (the
lines where body and jsonBody are assigned) in a try-catch block, and in the
catch block, return a 400 status response with an appropriate error message.
This ensures malformed JSON requests are handled gracefully as client errors
before the validation check on line 31.
In `@cueweb/app/api/task/mutate/route.ts`:
- Line 58: The NextResponse.json return statement in the task mutate route
handler is forwarding the raw RPC response data which may not include a success
field, causing taskMutate to incorrectly derive failure from missing
result.success. Modify the response construction to ensure the success field is
always explicitly included in the returned data by merging responseData.data
with a success property, rather than using a fallback that only applies when
data is undefined. Ensure that when responseData.data exists, it is combined
with or explicitly includes a success field based on the response status.
In `@cueweb/components/ui/create-group-dialog.tsx`:
- Around line 91-100: The updateGroup function call is awaited but its return
value is not checked before showing success feedback. Capture the result of the
updateGroup call and verify it returns true before proceeding with the
toastSuccess, window.dispatchEvent, and setOpen calls. If updateGroup returns
false or fails, display an appropriate error message instead of success feedback
and keep the dialog open so the user can address the issue.
In `@cueweb/components/ui/group-form.tsx`:
- Around line 54-57: The GPU fields in group-form.tsx have `min: 1` set while
their newDefault is `0`, which prevents saving valid zero values because
computeGroupChanges clamps enabled values to the minimum threshold. Change the
`min` property from 1 to 0 for the defaultJobMinGpus, defaultJobMaxGpus, and
maxGpus field definitions around lines 54-57 to allow zero as a valid value.
Additionally, check and apply the same fix to similar GPU field definitions
around lines 103-109 that have the same constraint issue.
In `@cueweb/components/ui/group-properties-dialog.tsx`:
- Around line 67-80: The getShowRootGroup() async call needs to be guarded
against stale responses that can overwrite group state after a different show
becomes active. Capture the current detail.show.id before initiating the
getShowRootGroup() promise, and in the .then() handler after successfully
retrieving the root group, verify that the captured show ID still matches the
current detail.show.id before calling setGroup() and setState(). If the IDs do
not match, skip the state update to prevent the stale response from overwriting
state for the active show.
In `@cueweb/components/ui/task-properties-dialog.tsx`:
- Around line 72-87: The loadDepartments and loadTasks functions lack
stale-response protection, allowing slower previous requests to overwrite
current state after user interactions. Add request token or
AbortController-based cancellation to both functions: maintain a reference to
track the latest request for each function, cancel any previous in-flight
requests when a new one is initiated, and verify responses belong to the current
request before updating state. This ensures stale responses from cancelled
requests are ignored and the correct department context is always used for
subsequent operations.
In `@cueweb/components/ui/view-filters-dialog.tsx`:
- Around line 129-170: The component initiates multiple overlapping async
requests (loadFilters, getShowGroups, loadMatchersActions) without protection
against out-of-order responses, causing stale results to be applied when users
rapidly switch between shows or filters. Implement request cancellation using
AbortController: in the useEffect handling OPEN_VIEW_FILTERS_EVENT, create an
AbortController and pass its signal to getShowFilters and getShowGroups calls,
aborting any previous pending requests when a new show is opened. Similarly, in
the useEffect that calls loadMatchersActions based on the selected filter,
implement AbortController to cancel prior getFilterMatchers and getFilterActions
requests when the selection changes. Ensure cancelled requests are caught and
ignored to prevent race conditions where stale data overwrites the current
filter context.
- Around line 409-412: The Input component uses defaultValue which creates an
uncontrolled input that doesn't rebind when m.input changes after a refresh or
rollback. Convert this to a controlled component by replacing
defaultValue={m.input} with value={m.input}, and add an onChange handler to
track intermediate input changes while the onBlur handler commits the final
value. This same fix should be applied to all other instances at lines 557-560
and 567-595 that have the same pattern of uncontrolled inputs with optimistic
updates.
---
Nitpick comments:
In `@cueweb/app/utils/get_utils.ts`:
- Around line 452-470: In the getDepartmentNames function, the fetch request
sends a JSON body via JSON.stringify({}) but declares an incorrect Content-Type
header of application/x-www-form-urlencoded. Change the Content-Type header
value to application/json to correctly match the JSON body format being
transmitted. This fix ensures the header accurately describes the request
content type and improves code clarity.
- Around line 367-372: In the getShowRootGroup function, add a warning log when
the fallback to groups[0] is triggered (i.e., when no root group with level 0 or
no parentId is found). This warning should be logged before returning groups[0],
so that any data integrity issues where a proper root group is missing become
visible during debugging instead of being silently masked by the defensive
fallback logic.
🪄 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: e3eb6ce7-827e-4011-8f35-f572ad932bab
📒 Files selected for processing (21)
cueweb/app/api/department/getdepartmentnames/route.tscueweb/app/api/department/gettasks/route.tscueweb/app/api/filter/getactions/route.tscueweb/app/api/filter/getmatchers/route.tscueweb/app/api/filter/mutate/route.tscueweb/app/api/group/action/createsubgroup/route.tscueweb/app/api/group/action/update/route.tscueweb/app/api/show/getdepartments/route.tscueweb/app/api/show/getfilters/route.tscueweb/app/api/task/mutate/route.tscueweb/app/monitor-cue/page.tsxcueweb/app/utils/action_utils.tscueweb/app/utils/get_utils.tscueweb/components/ui/create-group-dialog.tsxcueweb/components/ui/group-form.tsxcueweb/components/ui/group-properties-dialog.tsxcueweb/components/ui/monitor-cue-show-menu.tsxcueweb/components/ui/show-properties-dialog.tsxcueweb/components/ui/task-properties-dialog.tsxcueweb/components/ui/view-filters-dialog.tsxrest_gateway/opencue_gateway/main.go
✅ Files skipped from review due to trivial changes (3)
- rest_gateway/opencue_gateway/main.go
- cueweb/app/api/department/getdepartmentnames/route.ts
- cueweb/app/api/show/getfilters/route.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- cueweb/app/monitor-cue/page.tsx
…errides) Render the CueGUI Monitor Cue group hierarchy and add the show-scoped Service Properties dialog. - Monitor Cue is now a group tree (monitor-cue/page.tsx): per show it loads getShowGroups + getGroupJobs and renders show -> nested folders -> jobs. Jobs are placed by group id (a job's group name is not unique - root and a subgroup can share the show name), with rolled-up folder stats, per-folder collapse, and depth indentation. - Group folder right-click menu: Group Properties (edits that group), Create Group (subgroup under it), Delete Group (confirm). Generalize the Group Properties / Create Group dialogs to take a target/parent Group (the show-row menu still targets the root). New group/action/delete proxy + deleteGroup helper; the tree refreshes on cueweb:groups-changed and the poll. - Service Properties (service-properties-dialog.tsx): two-pane editor for a show's service overrides (New / Del + the shared service form). Save creates a new override (CreateServiceOverride) or updates the selected one (ServiceOverrideInterface.Update), with no facility-wide confirmation. Make service-defaults-form reusable via onPersist + confirm props (the Facility Service Defaults page is unchanged). Proxies: show/getserviceoverrides and consolidated serviceoverride/mutate; ServiceOverride type + helpers. - Wire both into the Monitor Cue show menu (replacing the placeholders).
Address code review on the Monitor Cue show-menu features (View Filters, Task Properties, Group Properties, Create Group). API routes: - Return 400 instead of 500 on malformed JSON in the read proxies (getfilters, getdepartments, getserviceoverrides, gettasks, filter/getactions, filter/getmatchers) by guarding request.json() with try/catch. - group/action/update: reject unknown change keys with 400 rather than silently dropping them, so a renamed setter can't report success while applying nothing. Dialogs: - create-group: check the post-create updateGroup result and downgrade the success toast to a warning when follow-up properties fail to apply. - group-properties, task-properties, view-filters: guard async loads with per-request generation tokens so a slow response for a previously selected show/department/filter can't overwrite current state. - view-filters: remount the uncontrolled matcher/action value editors on reload (keyed by a reload nonce) so they rebind to server state after a refresh or a failed-commit rollback.
Three enhancements to the frame log viewer.
Log version dropdown polish (getlogversions + frames page):
- /api/getlogversions now returns { name, size, mtime } per version (fs.stat, replacing the wc -l shell check), sorted newest-first by mtime.
- The version dropdown renders three columns -- name, timestamp, size (MB) -- with a "Latest" badge on the most recent; the closed control stays compact.
Log download button (new /api/getlog + frames page):
- /api/getlog streams the selected log file as text/plain with Content-Disposition: attachment; filename="<frame>.log" (filename derived and sanitized server-side). Reads via fs (no shell). Requires a signed-in session when an auth provider is configured; open in the sandbox.
- A Download button next to the version dropdown downloads the current version.
"Next error" jumper (frame-log-search):
- A button that drives a fixed case-insensitive regex (error|exception|traceback) through the existing Monaco search pipeline: first press reveals the first match, subsequent presses cycle via the same next/prev logic, reusing the n/total counter and highlight decorations.
- job-details-inline: single-click a frame row to load it into the Attributes panel (type "frame") and highlight the row; double-click still opens the log viewer. Selection is idempotent (no flicker on double-click) and clears when the job/filtered layer changes or the frame disappears on a poll. Jobs and layers already populated the panel; this fills in frames. - attributes-panel: add copy-key and copy-value icon buttons to every attribute row (clipboard API, momentary check confirmation, warning toast on failure) for all entity types.
…actions-and-log-viewer # Conflicts: # cueweb/components/ui/frame-range-selector.tsx
…-and-log-viewer # Conflicts: # cueweb/app/monitor-cue/page.tsx # cueweb/app/utils/action_utils.ts # cueweb/app/utils/get_utils.ts
…s-and-log-viewer # Conflicts: # cueweb/.env.example # cueweb/app/api/layer/action/setmincores/route.ts
The generic Attributes side-panel previously populated only from Monitor Jobs (jobs/layers/frames). Wire setAttributeSelection on every other view that has a selectable entity, matching CueGUI's AttributesPlugin (Job, Layer, Host) and extending it with Frame: - Monitor Hosts: click a host row -> Host attributes (+ row highlight). - Host detail: load the host into the panel on view/refresh. - Job detail: load the job on open; click a layer row -> Layer attributes, single-click a frame row -> Frame attributes (double-click still opens the log); both highlighted. - Monitor Cue: click a job row in the group tree -> Job attributes, alongside the existing multi-select. - Stuck Frames: click a frame row -> Frame attributes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Issues
Main issue:
Issues related to this PR:
Summarize your change.
[cueweb] Add Monitor Cue page (CueCommander parity)
Add the /monitor-cue route (previously a dead sidebar link) replicating CueGUI's Monitor Cue window:
[cueweb] Monitor Cue: CueGUI parity for menu, columns, booking bar, selection
Bring the CueCommander Monitor Cue table and its right-click menu closer to CueGUI/CueCommander.
Right-click job menu
Table
Fixes
LLM usage disclosure
Parts of this solution's implementation were developed with assistance from Claude Opus.