[cueweb] Job/Layer/Frame context-menu parity + frame log viewer enhancements#2426
Open
ramonfigueiredo wants to merge 40 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.
…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.
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 build/runtime environment flags, ~35 new Next.js API routes (frame/job/layer RPC proxies plus log access), secure filesystem frame preview API, preview/color utilities, context-menu wiring, user color persistence, custom-event dialogs (job/layer/frame/send-to-group/comments), frame log tail/follow/search/copy-line interactions, Monitor Cue page with selection and bulk actions, and sandbox Blender tooling for rendering previewable test jobs. ChangesBackend Configuration & API Routes
Preview & Image Utilities
Actions, Dispatchers & Contracts
Table Rendering & Context Menus
Dialog Components
Frame Log Viewer
Monitor Cue Page
Sandbox Blender Integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![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: 16
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (7)
sandbox/load_test_jobs.py-821-823 (1)
821-823:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFrame-count validation is missing in both CLIs. Non-positive frame counts propagate into invalid frame ranges/render behavior.
sandbox/load_test_jobs.py#L821-L823: enforceargs.frame_count >= 1incmd_blenderand fail fast with a clear CLI error.sandbox/render_blender_demo.py#L68-L69: enforceargs.frames >= 1immediately after parsing arguments.🤖 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 `@sandbox/load_test_jobs.py` around lines 821 - 823, Validate frame count right after parsing: in sandbox/load_test_jobs.py inside cmd_blender ensure args.frame_count >= 1 and exit with a clear CLI error if not (use the nframes/args.frame_count variable and fail fast before using discover_blender/output_root); similarly in sandbox/render_blender_demo.py check args.frames >= 1 immediately after argument parsing and raise/exit with a descriptive message if the value is < 1 so invalid frame ranges never propagate into rendering logic.cueweb/app/api/job/action/staggerframes/route.ts-33-33 (1)
33-33:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReject non-finite stagger values (
NaN/Infinity) at the route boundary.
typeof ... === 'number'still accepts non-finite values and can leak invalid payloads upstream.Proposed fix
- if (!jsonBody?.job || typeof jsonBody.range !== 'string' || typeof jsonBody.stagger !== 'number') { + if (!jsonBody?.job || typeof jsonBody.range !== 'string' || typeof jsonBody.stagger !== 'number' || !Number.isFinite(jsonBody.stagger)) { return NextResponse.json({ error: 'Invalid request body: job, range, numeric stagger required' }, { 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/staggerframes/route.ts` at line 33, Update the request validation to reject non-finite numeric values: in the route handler where you check jsonBody (the condition currently using "typeof jsonBody.stagger === 'number'"), change that check to use Number.isFinite(jsonBody.stagger) (e.g., if (!jsonBody?.job || typeof jsonBody.range !== 'string' || !Number.isFinite(jsonBody.stagger)) ...) so NaN/Infinity are treated as invalid and the route returns the rejection early.cueweb/app/utils/user_colors.ts-64-69 (1)
64-69:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winValidate parsed localStorage shape before returning.
At Line 68,
JSON.parseoutput is cast without runtime checks. If storage is corrupted (e.g.null/array), consumers can crash when indexing the map.Suggested fix
export function readUserColors(): Record<string, string> { if (typeof window === "undefined") return {}; try { const raw = window.localStorage.getItem(USER_COLORS_KEY); - return raw ? (JSON.parse(raw) as Record<string, string>) : {}; + const parsed = raw ? JSON.parse(raw) : {}; + if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return {}; + return Object.fromEntries( + Object.entries(parsed).filter( + ([k, v]) => typeof k === "string" && typeof v === "string", + ), + ) as Record<string, string>; } catch { 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/utils/user_colors.ts` around lines 64 - 69, The readUserColors function currently casts JSON.parse output to Record<string,string> without runtime validation; update it to validate the parsed value from window.localStorage.getItem(USER_COLORS_KEY) before returning: after JSON.parse(raw) ensure the result is a non-null object (typeof === "object" && !Array.isArray(result)), iterate its own keys and confirm each value is a string (filter/transform to a safe Record<string,string>), and return that sanitized map or {} on any mismatch or parse error; keep the try/catch but replace the unchecked cast with this runtime shape check to avoid crashes in consumers.cueweb/components/ui/dependency-wizard-dialog.tsx-313-314 (1)
313-314:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard
initialTypeat runtime before setting wizard state.At Line 313,
detail.initialTypeis trusted directly. Since this comes from aCustomEvent, malformed payloads can push an invalid type and break downstream lookups.Suggested fix
- if (detail.initialType) setDependType(detail.initialType); + if (detail.initialType && detail.initialType in TYPE_CONFIG) { + setDependType(detail.initialType as DependType); + }🤖 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/dependency-wizard-dialog.tsx` around lines 313 - 314, Guard against invalid values in the CustomEvent payload before calling setDependType: add a runtime check that detail.initialType is one of the expected dependency types (e.g., matches the DependencyType enum or an allowedTypes array) and only call setDependType(detail.initialType) when that check passes; otherwise skip it or call setDependType with a safe default. Implement the check as a small helper like isValidDependType(value) and use it around the existing setDependType/detail.initialType usage so malformed CustomEvent payloads cannot break downstream lookups.cueweb/components/ui/job-comments-dialog.tsx-96-99 (1)
96-99:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGuard event payload before dereferencing
detail.job.At Line 97, malformed events can throw before the dialog opens. Add a null-safe read and early return.
Suggested fix
function handler(e: Event) { - const j = (e as CustomEvent<OpenJobCommentsDetail>).detail.job; + const j = (e as CustomEvent<OpenJobCommentsDetail>).detail?.job; + if (!j) return; setJob(j);🤖 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-comments-dialog.tsx` around lines 96 - 99, In handler, guard the event payload before dereferencing detail.job: cast e to CustomEvent<OpenJobCommentsDetail> and check that event.detail and event.detail.job are non-null, returning early if missing; only then call setJob(j) and setSelectedId(null). Update the handler function (referencing handler, OpenJobCommentsDetail, setJob, and setSelectedId) to perform a null-safe read and early return to avoid exceptions from malformed events.cueweb/app/globals.css-151-153 (1)
151-153:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse theme-aware hover color for the copy glyph.
The hover color is hardcoded to
#ffffff, which can become low-contrast in light mode. Prefer a tokenized foreground color so the icon remains visible across themes.Suggested patch
.cueweb-copy-line-glyph:hover::before { - color: `#ffffff`; + color: hsl(var(--foreground)); }🤖 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/globals.css` around lines 151 - 153, The hover rule for .cueweb-copy-line-glyph:hover::before uses a hardcoded color (`#ffffff`); change it to use a theme token CSS variable so the icon adapts to light/dark themes, e.g. replace color: `#ffffff` with color: var(--foreground, `#ffffff`) (or your app's established token like --vscode-editor-foreground) and ensure that variable is defined for both light and dark themes so the glyph maintains sufficient contrast across themes.cueweb/components/ui/frame-range-selector.tsx-192-198 (1)
192-198:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKill is gated by
jobeven though the action does not require it.The button is disabled by
!job, butkillFrames(...)is driven by selected frames and username/reason. This blocks valid kills in job-less contexts.Suggested patch
<Button size="xs" variant="outline" className="text-destructive hover:text-destructive" onClick={doKill} - disabled={!hasSelection || !job} + disabled={!hasSelection} > Kill </Button>🤖 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/frame-range-selector.tsx` around lines 192 - 198, The Kill button is incorrectly disabled when there is no job because its disabled prop checks !job even though doKill calls killFrames(...) using selected frames and username/reason only; remove the job gating by changing the Button's disabled condition to rely on hasSelection (e.g., disabled={!hasSelection}) and also inspect the doKill function to ensure it does not reference job — if it does, refactor doKill to call killFrames(selectedFrames, username, reason) (or otherwise use the selection/credentials) so kills work in job-less contexts.
🧹 Nitpick comments (1)
cueweb/app/api/job/action/markdoneframes/route.ts (1)
35-36: ⚡ Quick winUnify
cueweb/app/api/job/action/*request-validation scope to preserve API contract parity.
These three handlers apply route-specific deep payload checks, but sibling job action routes intentionally only gate onjsonBody.joband let backend validation/errors flow throughhandleRoute.
cueweb/app/api/job/action/markdoneframes/route.ts#L35-L36: removereqshape/type checks at route layer; require onlyjsonBody.job.cueweb/app/api/job/action/setmaxgpus/route.ts#L33-L34: remove route-layervaltype validation; require onlyjsonBody.job.cueweb/app/api/job/action/setmingpus/route.ts#L33-L34: remove route-layervaltype validation; require onlyjsonBody.job.Based on learnings, this path’s established contract is shallow route validation with malformed payloads handled downstream.
🤖 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/markdoneframes/route.ts` around lines 35 - 36, Remove the deep route-layer payload shape/type checks so the route handlers only validate presence of jsonBody.job and let backend validation flow through handleRoute; specifically, in markdoneframes/route.ts remove the jsonBody.req and typeof jsonBody.req !== 'object' check and only gate on jsonBody.job (keeping the existing NextResponse.json error response), and in setmaxgpus/route.ts and setmingpus/route.ts remove the route-layer typeof val/val shape checks so they likewise only check jsonBody.job; keep all other logic (jsonBody extraction, NextResponse.json usage, and subsequent calls to handleRoute) unchanged.Source: Learnings
🤖 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 30-31: Wrap the await request.json() call in a try/catch in both
getdepends route handlers so invalid JSON returns a 400 response instead of
throwing; replace the current stringify/parse round-trip (const body =
JSON.stringify(await request.json()); const jsonBody = JSON.parse(body);) with a
guarded single parse: try { const jsonBody = await request.json(); } catch (err)
{ return new Response("Invalid JSON", { status: 400 }); } and apply the same
pattern to the other getdepends handler, removing the redundant
JSON.stringify/JSON.parse usage.
In `@cueweb/app/api/frame/action/markaswaiting/route.ts`:
- Around line 30-31: The current parse flow using JSON.stringify(await
request.json()) and JSON.parse(body) can throw on malformed JSON and embeds
status codes in the JSON body instead of using HTTP response status; update the
route handler to safely read and parse the request using await request.text()
and a try/catch around JSON.parse to return a 400 Response on parse errors (with
a clear error message), and whenever you forward an upstream response or build a
Response from jsonBody, use the status/statusCode found in the parsed object
(e.g., jsonBody.status || jsonBody.statusCode) as the HTTP Response status
instead of placing it inside the JSON payload; apply the same safe-parse +
status-propagation fix for the other occurrence that uses body/jsonBody at the
second location.
In `@cueweb/app/api/frame/preview/route.ts`:
- Around line 31-40: The MIME map in this route (constant MIME) currently
includes "svg" => "image/svg+xml", which allows SVGs to be served from the
/api/frame/preview endpoint; remove or exclude the "svg" entry from MIME (and
any checks that rely on MIME["svg"]) so SVGs are treated as unsupported for this
route (update any validation logic that looks up MIME for file extensions in
route handler code to reject/return 415 for "svg" inputs).
- Around line 42-47: The allowedRoots() helper currently returns an empty list
as allow-all when CUEWEB_PREVIEW_ROOTS is unset; change enforcement to
default-deny by treating an empty/absent env as no roots allowed and fail early.
In the preview route where image paths are validated (the code that checks
absolute path and prefix), resolve both the target path and each configured root
with fs.realpath (or fs.promises.realpath) and verify the resolved target path
startsWith a resolved root to prevent symlink escapes; if no configured roots
exist or no root matches, reject the request. Update references to
allowedRoots(), the code that does the absolute/prefix check, and any readFile
calls to perform realpath validation before reading.
In `@cueweb/app/api/layer/action/getoutputpaths/route.ts`:
- Around line 31-41: Wrap the request.json() call in a try/catch inside the
route handler (the block that currently does const body = JSON.stringify(await
request.json()); const jsonBody = JSON.parse(body);) to detect malformed JSON
and return NextResponse.json({ error: 'Invalid request body' }, { status: 400 })
instead of letting a 500 escape; then when forwarding the response from
handleRoute(method, endpoint, body, true) use the Response object's HTTP status
(response.status) for NextResponse.json's options rather than any status field
inside response.json() (i.e., always call NextResponse.json({ data/error: ... },
{ status: response.status })); apply the same try/catch parse guard and
response.status propagation fix to the markdone route as well so neither route
relies on responseData.status.
- Around line 31-35: The current parsing around await request.json() and
JSON.parse can throw and return a 500; wrap the parse in a try/catch (or
validate await request.json() result) and return NextResponse.json({ error:
'Invalid request body' }, { status: 400 }) on SyntaxError or missing
jsonBody.layer, referencing the jsonBody variable and the request.json() call in
this route handler; additionally, when forwarding responses do not put status
inside the JSON body—use NextResponse.json(forwardedBody, { status:
forwardedResponse.status }) (i.e., replace returning { ..., status: res.status }
with using NextResponse.json(..., { status: res.status })) so the HTTP status is
set correctly.
In `@cueweb/app/api/layer/action/markdone/route.ts`:
- Around line 29-33: The code currently does JSON stringify/parse which can
throw and doesn't propagate HTTP status properly; update the route handler to
safely parse the request body by calling await request.json() inside a try/catch
(avoid the intermediate JSON.stringify/parse using the body and jsonBody
pattern), return a 400 via NextResponse.json(..., { status: 400 }) when parsing
fails or when required fields (like jsonBody.layer) are missing, and ensure any
other responses use NextResponse.json(responseBody, { status: <code> }) instead
of embedding a status field in the JSON body; reference request.json(), the
body/jsonBody variables, and NextResponse.json to locate where to change.
In `@cueweb/app/frames/`[frame-name]/page.tsx:
- Around line 412-430: The current useEffect rebuilds a decoration per line via
model.getLineCount() and ed.deltaDecorations (in the effect watching
logContentVersion/editorMounted), causing O(N) work; change it to only create
decorations for the visible viewport (use editorRef.current.getVisibleRanges()
or monaco editor API) plus a small buffer, and update
copyGlyphDecorationsRef.current via deltaDecorations for that limited range;
attach the logic to scroll/visible-range change events (rather than every
content tick) and keep fallback behavior to rely on line-number click copy if
decorations are not present. Ensure you update the same symbols (useEffect,
editorRef, monacoRef, copyGlyphDecorationsRef, deltaDecorations) so the effect
only computes decorations for visible lines instead of model.getLineCount().
- Around line 385-401: The polling effect calling loadNewerLogMessages() can
overlap with the scroll-triggered loading code and cause duplicate appends or
counter drift; add a shared in-flight guard (e.g. an isLoadingRef or mutex) used
by both the useEffect ticker and the scroll handler so both check-and-set before
invoking loadNewerLogMessages (or any scroll-based loader) and clear it on
completion or error; ensure the guard is cleaned up in the effect's cleanup and
that scroll-based logic and functions like loadNewerLogMessages and
scrollToVeryBottom honor the guard to avoid concurrent/preemptive calls.
In `@cueweb/components/ui/frame-preview-panel.tsx`:
- Around line 52-83: The resolve() async can finish out-of-order and overwrite
state for a later selection; add a monotonic request id (or use an
AbortController) stored in a ref (e.g., resolveRequestIdRef) that you increment
at the start of resolve() and capture locally, then after each await (before
calling setCandidates/setWebCandidates/setStatus) verify the captured id matches
the current ref value and abort early if it does not; apply the same pattern to
the other frame-resolution handler at lines ~85–95 so only the most recent async
resolution updates component state.
In `@cueweb/components/ui/frame-range-selector.tsx`:
- Around line 152-162: The element currently uses role="slider" but lacks
keyboard interaction; either add keyboard support or remove slider
semantics—choose one: (A) Implement keyboard handlers: make the div focusable
(add tabIndex={0}), manage focus via stripRef, add an onKeyDown handler that
adjusts the selection incrementally (e.g., left/right or Home/End keys) by
calling the same update logic used by onPointerDown/onPointerMove (create helper
functions like adjustSelectionBy(delta) or setSelectionIndex(idx) to reuse
existing state), and keep aria-valuemin/aria-valuemax/aria-valuenow updated; or
(B) Remove role="slider" and related slider ARIA attributes and use a neutral
role (or none) so it does not promise keyboard operability—modify the element
where role="slider", aria-valuemin, aria-valuemax, aria-valuenow, and tabIndex
are set accordingly. Ensure to reference stripRef, onPointerDown, onPointerMove,
and the aria attributes when making changes.
In `@cueweb/components/ui/job-comments-dialog.tsx`:
- Around line 125-149: The handleSave and handleDelete flows currently await
addJobComment / saveJobComment / deleteJobComment without catch blocks, causing
unhandled rejections and silent UI state changes; wrap the
mutation+post-mutation steps inside a try block and add a catch that (a) logs
the error and surfaces a user-facing message (e.g., via an existing
notification/toast or setError) and (b) prevents running post-success actions
(refresh, startNew, setDirty) on failure; keep the finally to call
setSubmitting(false). Update handleSave and handleDelete to reference the same
functions (addJobComment, saveJobComment, deleteJobComment, refresh, startNew,
setDirty) so failures are handled explicitly and users get feedback.
In `@cueweb/components/ui/job-details-inline.tsx`:
- Around line 280-285: The inline job-details view currently mounts
LayerExtraDialogs and FrameExtraDialogs but does not mount
DependencyWizardDialog, so the "Dependency Wizard..." context-menu action has no
handler; add a mounted <DependencyWizardDialog job={job} /> (or equivalent props
used by the page version) into cueweb/components/ui/job-details-inline.tsx
alongside LayerExtraDialogs/FrameExtraDialogs so the same event-driven dialog
instance receives the event, and repeat the same addition for the frames inline
area currently rendering FrameExtraDialogs to ensure both layer and frame
context-menus can open the wizard.
In `@cueweb/components/ui/job-extra-dialogs.tsx`:
- Around line 78-93: In apply(), after parsing value to Number (const n =
Number(value)), add an extra validation branch that, for the integer-only fields
("minGpus", "maxGpus", "maxRetries"), ensures n is an integer (e.g.
Number.isInteger(n)); if not, call toastWarning with a clear message like
`${SCALAR_LABEL[field]} must be a non-negative integer.` and return; leave the
existing non-negative/finite check for other fields and keep the existing
selection of setter functions (setJobMinGpus, setJobMaxGpus, setJobMaxRetries,
setJobMinCores, setJobMaxCores) and the rest of apply() unchanged. Ensure the
same integer validation is applied at the other similar location noted (the
second apply use around the other occurrence).
- Around line 269-286: The apply() handler builds the addRenderPartition payload
using Number(...) and Math methods without validating results, which can produce
NaN and lead to nulls in the JSON; before calling addRenderPartition, validate
and normalize numeric inputs (cores, memGb, gpus, gpuMemGb) by parsing them to
finite numbers, fallback to safe defaults (e.g., 0 or 1 as appropriate), then
apply Math.max/Math.round; update the payload construction in apply() to use
these validated variables and keep existing early checks (host.trim(), setBusy,
toastWarning) unchanged so malformed numeric input is prevented.
In `@sandbox/load_test_jobs.py`:
- Around line 872-876: Replace the racy immediate lookup using
_find_job(short_name) with a wait-for-search call: call
_wait_for_jobs([short_name]) and take the first result (i.e.
_wait_for_jobs([short_name])[0]) before resolving the "beauty" layer; then
continue using next(l for l in job.getLayers() if l.name() == "beauty") and call
layer.registerOutputPath(output_spec) as before. Apply the same replacement in
the Blender submission path that currently queries the job immediately (use
_wait_for_jobs([short_name])[0] instead of _find_job(short_name]) so layer
lookup becomes deterministic.
---
Minor comments:
In `@cueweb/app/api/job/action/staggerframes/route.ts`:
- Line 33: Update the request validation to reject non-finite numeric values: in
the route handler where you check jsonBody (the condition currently using
"typeof jsonBody.stagger === 'number'"), change that check to use
Number.isFinite(jsonBody.stagger) (e.g., if (!jsonBody?.job || typeof
jsonBody.range !== 'string' || !Number.isFinite(jsonBody.stagger)) ...) so
NaN/Infinity are treated as invalid and the route returns the rejection early.
In `@cueweb/app/globals.css`:
- Around line 151-153: The hover rule for .cueweb-copy-line-glyph:hover::before
uses a hardcoded color (`#ffffff`); change it to use a theme token CSS variable so
the icon adapts to light/dark themes, e.g. replace color: `#ffffff` with color:
var(--foreground, `#ffffff`) (or your app's established token like
--vscode-editor-foreground) and ensure that variable is defined for both light
and dark themes so the glyph maintains sufficient contrast across themes.
In `@cueweb/app/utils/user_colors.ts`:
- Around line 64-69: The readUserColors function currently casts JSON.parse
output to Record<string,string> without runtime validation; update it to
validate the parsed value from window.localStorage.getItem(USER_COLORS_KEY)
before returning: after JSON.parse(raw) ensure the result is a non-null object
(typeof === "object" && !Array.isArray(result)), iterate its own keys and
confirm each value is a string (filter/transform to a safe
Record<string,string>), and return that sanitized map or {} on any mismatch or
parse error; keep the try/catch but replace the unchecked cast with this runtime
shape check to avoid crashes in consumers.
In `@cueweb/components/ui/dependency-wizard-dialog.tsx`:
- Around line 313-314: Guard against invalid values in the CustomEvent payload
before calling setDependType: add a runtime check that detail.initialType is one
of the expected dependency types (e.g., matches the DependencyType enum or an
allowedTypes array) and only call setDependType(detail.initialType) when that
check passes; otherwise skip it or call setDependType with a safe default.
Implement the check as a small helper like isValidDependType(value) and use it
around the existing setDependType/detail.initialType usage so malformed
CustomEvent payloads cannot break downstream lookups.
In `@cueweb/components/ui/frame-range-selector.tsx`:
- Around line 192-198: The Kill button is incorrectly disabled when there is no
job because its disabled prop checks !job even though doKill calls
killFrames(...) using selected frames and username/reason only; remove the job
gating by changing the Button's disabled condition to rely on hasSelection
(e.g., disabled={!hasSelection}) and also inspect the doKill function to ensure
it does not reference job — if it does, refactor doKill to call
killFrames(selectedFrames, username, reason) (or otherwise use the
selection/credentials) so kills work in job-less contexts.
In `@cueweb/components/ui/job-comments-dialog.tsx`:
- Around line 96-99: In handler, guard the event payload before dereferencing
detail.job: cast e to CustomEvent<OpenJobCommentsDetail> and check that
event.detail and event.detail.job are non-null, returning early if missing; only
then call setJob(j) and setSelectedId(null). Update the handler function
(referencing handler, OpenJobCommentsDetail, setJob, and setSelectedId) to
perform a null-safe read and early return to avoid exceptions from malformed
events.
In `@sandbox/load_test_jobs.py`:
- Around line 821-823: Validate frame count right after parsing: in
sandbox/load_test_jobs.py inside cmd_blender ensure args.frame_count >= 1 and
exit with a clear CLI error if not (use the nframes/args.frame_count variable
and fail fast before using discover_blender/output_root); similarly in
sandbox/render_blender_demo.py check args.frames >= 1 immediately after argument
parsing and raise/exit with a descriptive message if the value is < 1 so invalid
frame ranges never propagate into rendering logic.
---
Nitpick comments:
In `@cueweb/app/api/job/action/markdoneframes/route.ts`:
- Around line 35-36: Remove the deep route-layer payload shape/type checks so
the route handlers only validate presence of jsonBody.job and let backend
validation flow through handleRoute; specifically, in markdoneframes/route.ts
remove the jsonBody.req and typeof jsonBody.req !== 'object' check and only gate
on jsonBody.job (keeping the existing NextResponse.json error response), and in
setmaxgpus/route.ts and setmingpus/route.ts remove the route-layer typeof
val/val shape checks so they likewise only check jsonBody.job; keep all other
logic (jsonBody extraction, NextResponse.json usage, and subsequent calls to
handleRoute) 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: 0102dc69-aea2-41d6-823c-c24e4137f0a7
📒 Files selected for processing (53)
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/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-comments-dialog.tsxcueweb/components/ui/job-details-inline.tsxcueweb/components/ui/job-extra-dialogs.tsxcueweb/components/ui/layer-extra-dialogs.tsxcueweb/components/ui/toast-host.tsxcueweb/components/ui/toolbar.tsxdocker-compose.ymlsandbox/README.mdsandbox/load_test_jobs.pysandbox/render_blender_demo.py
…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: 4
🤖 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 359-366: In selectRange, when the stored anchor
(lastSelectedRef.current) is no longer visible (anchorIdx < 0) you currently add
toId to the selection but do not update the anchor; update
lastSelectedRef.current = toId after calling setSelected so future Shift-clicks
use the new anchor. This modifies the fallback path in selectRange to both
select the clicked id and re-anchor lastSelectedRef.current to toId.
- Around line 287-317: The load(...) function can return out-of-order results;
fix it by giving each invocation a monotonic id (e.g. a ref like latestLoadId)
that you increment just before calling getJobs, capture the id in the local
scope, and only call setJobs(...) and setNow(...) (and setJobs(prev => prev ??
[]) in the catch) if the captured id matches the current latestLoadId; update
both the success and error paths in load to check this id so slower earlier
requests cannot overwrite newer data (no changes needed to useEffect beyond
continuing to call load with selectedShows/autoRefresh/REFRESH_MS).
In `@cueweb/components/ui/send-to-group-dialog.tsx`:
- Around line 46-63: The async load function can have its results applied to
state after the dialog has moved to a different Job; protect against stale
responses by adding a request-scoped cancellation/guard (e.g., a requestId
counter in a ref or an AbortController) that you increment/create when load
starts and capture inside the async work, then only call setGroups,
setSelectedId, setLoading, or handleError if the captured request id matches the
latest one; update the load function (and any other async loaders with the same
pattern) to check the guard before every state update after awaits and to
clear/mark the request finished in finally so stale responses are ignored.
- Around line 77-89: The apply() function currently uses try/finally so thrown
errors from reparentJobs are swallowed; change it to try/catch/finally in
apply(), catch exceptions from reparentJobs, log the error
(console.error(error)) and surface a visible error to the user (invoke an
existing UI error handler such as setError(error.message) if available, or fall
back to window.alert) so the user gets feedback; keep setBusy(false) in finally
and preserve the existing success path (setOpen(false) and
window.dispatchEvent("cueweb:refresh-now")).
🪄 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: 9b30f799-db37-4059-83e8-ab8f601aa671
📒 Files selected for processing (5)
cueweb/app/monitor-cue/page.tsxcueweb/app/utils/action_utils.tscueweb/components/ui/context_menus/action-context-menu.tsxcueweb/components/ui/job-booking-bar.tsxcueweb/components/ui/send-to-group-dialog.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- cueweb/app/utils/action_utils.ts
- cueweb/components/ui/context_menus/action-context-menu.tsx
…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.
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.
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
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
cueweb/app/api/frame/action/getdepends/route.ts (1)
26-28: 💤 Low valueRedundant method checks in App Router POST handlers. In Next.js App Router, exporting a named
POSTfunction means only POST requests reach that handler—other methods receive an automatic 405 from the framework. The explicit check is dead code.
cueweb/app/api/frame/action/getdepends/route.ts#L26-L28: remove theif (method !== 'POST')block and inline"POST"in thehandleRoutecall.cueweb/app/api/layer/action/getoutputpaths/route.ts#L27-L29: apply the same removal.🤖 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/frame/action/getdepends/route.ts` around lines 26 - 28, Remove the redundant method checks in both Next.js App Router POST handlers since exporting a named POST function means only POST requests reach the handler and other methods are automatically rejected by the framework. In cueweb/app/api/frame/action/getdepends/route.ts at lines 26-28, delete the entire if (method !== 'POST') block and directly inline "POST" in the handleRoute call. Apply the identical removal in cueweb/app/api/layer/action/getoutputpaths/route.ts at lines 27-29, deleting the method check and inlining "POST" in the corresponding handleRoute call.
🤖 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/frame/action/getdepends/route.ts`:
- Around line 26-28: Remove the redundant method checks in both Next.js App
Router POST handlers since exporting a named POST function means only POST
requests reach the handler and other methods are automatically rejected by the
framework. In cueweb/app/api/frame/action/getdepends/route.ts at lines 26-28,
delete the entire if (method !== 'POST') block and directly inline "POST" in the
handleRoute call. Apply the identical removal in
cueweb/app/api/layer/action/getoutputpaths/route.ts at lines 27-29, deleting the
method check and inlining "POST" in the corresponding handleRoute call.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 136ec2ad-be96-409f-a9e8-407f92d9f87a
📒 Files selected for processing (20)
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/preview_utils.tscueweb/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.tsxcueweb/components/ui/send-to-group-dialog.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 (16)
- cueweb/app/api/layer/action/setminmemory/route.ts
- cueweb/app/api/layer/action/settags/route.ts
- cueweb/app/utils/user_colors.ts
- cueweb/components/ui/frame-preview-panel.tsx
- sandbox/render_blender_demo.py
- cueweb/app/api/layer/action/markdone/route.ts
- cueweb/components/ui/send-to-group-dialog.tsx
- cueweb/app/utils/preview_utils.ts
- sandbox/load_test_jobs.py
- cueweb/app/api/frame/action/markaswaiting/route.ts
- cueweb/app/monitor-cue/page.tsx
- cueweb/app/api/layer/action/getdepends/route.ts
- cueweb/components/ui/job-comments-dialog.tsx
- cueweb/components/ui/job-extra-dialogs.tsx
- cueweb/components/ui/frame-range-selector.tsx
- cueweb/app/frames/[frame-name]/page.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.
Collaborator
Author
|
Ready for review! |
Collaborator
Author
|
Note: The CueWeb docs will be updated on a new PR or later in this PR. |
- 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
…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.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cueweb/app/api/getlogversions/route.ts (1)
83-92:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPath traversal allows directory enumeration.
The
filenameparameter is passed togetLogVersionswithout path validation. An attacker can probe arbitrary directories to learn file names, sizes, and modification times—useful for reconnaissance before exploiting the more severe arbitrary-read ingetlog.Apply the same allow-list validation pattern recommended for the
getlogroute.🔒 Proposed fix
+const ALLOWED_LOG_ROOTS = (process.env.CUEWEB_LOG_ROOTS ?? "").split(",").map(s => s.trim()).filter(Boolean); + export async function GET(request: Request) { - // Validate the method, only allow GET - if (request.method !== "GET") { - return NextResponse.json({ error: "Method Not Allowed" }, { status: 405 }); - } - const { searchParams } = new URL(request.url); const filename = searchParams.get("filename"); // Validate the filename parameter if (!filename) { return NextResponse.json({ error: "Filename is required" }, { status: 400 }); } + const resolved = path.resolve(filename); + if (ALLOWED_LOG_ROOTS.length > 0 && !ALLOWED_LOG_ROOTS.some(root => resolved.startsWith(root + path.sep))) { + return NextResponse.json({ error: "Access denied" }, { status: 403 }); + } + try { - const versions = await getLogVersions(filename); + const versions = await getLogVersions(resolved);🤖 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/getlogversions/route.ts` around lines 83 - 92, The filename parameter in the getlogversions route is susceptible to path traversal attacks that enable directory enumeration. Apply the same allow-list validation pattern used in the getlog route to the filename parameter before it is passed to getLogVersions. This validation should prevent directory traversal attempts and only allow access to whitelisted file names or directories, ensuring that attackers cannot probe arbitrary directories to enumerate files.
🤖 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/getlog/route.ts`:
- Around line 53-64: The filePath parameter obtained from the query string is
passed directly to fs.stat and fs.readFile without any validation, creating a
path traversal vulnerability. Implement path validation by resolving the
filePath to an absolute path using the path module and verify that the resolved
path starts with an allowed log directory prefix. Only proceed with the fs.stat
and fs.readFile operations if the resolved path is within the allowed directory,
otherwise return an appropriate error response. This pattern should match the
validation approach mentioned in the PR objectives for the preview route.
---
Outside diff comments:
In `@cueweb/app/api/getlogversions/route.ts`:
- Around line 83-92: The filename parameter in the getlogversions route is
susceptible to path traversal attacks that enable directory enumeration. Apply
the same allow-list validation pattern used in the getlog route to the filename
parameter before it is passed to getLogVersions. This validation should prevent
directory traversal attempts and only allow access to whitelisted file names or
directories, ensuring that attackers cannot probe arbitrary directories to
enumerate files.
🪄 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: b364de4d-782f-4346-80c3-b257f9b6aec9
📒 Files selected for processing (4)
cueweb/app/api/getlog/route.tscueweb/app/api/getlogversions/route.tscueweb/app/frames/[frame-name]/page.tsxcueweb/components/ui/frame-log-search.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- cueweb/app/frames/[frame-name]/page.tsx
- 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.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
cueweb/components/ui/job-details-inline.tsx (1)
311-316:⚠️ Potential issue | 🟠 MajorMount
DependencyWizardDialogto handle layer/frame wizard actions.This is the same issue flagged in the previous review. The inline view mounts
LayerExtraDialogsandFrameExtraDialogsbut notDependencyWizardDialog, so the "Dependency Wizard..." context-menu action will have no handler in this surface.🔧 Proposed fix (from previous review)
Import and mount
DependencyWizardDialogalongside the other dialog components:import { JobDependencyGraph } from "./job-dependency-graph"; +import { DependencyWizardDialog } from "./dependency-wizard-dialog"; import { FrameExtraDialogs } from "./frame-extra-dialogs";<LayerExtraDialogs job={job} /> + <DependencyWizardDialog />Apply the same addition near line 378 where
FrameExtraDialogsis mounted.🤖 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 311 - 316, The job-details-inline view is missing the DependencyWizardDialog component mount, which prevents the "Dependency Wizard..." context-menu action from functioning. Import DependencyWizardDialog if not already imported, then mount it alongside the existing LayerExtraDialogs component (around line 311-316) and also add it near line 378 where FrameExtraDialogs is mounted, ensuring both surfaces have the dialog handler available for layer/frame wizard actions.
🤖 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/attributes-panel.tsx`:
- Around line 51-78: The CopyButton function has a setTimeout that is not
cleaned up when the component unmounts, which can cause a React warning and
memory leak. To fix this, use a useRef hook to track whether the component is
still mounted and check this ref before calling setCopied(false) in the timeout
callback, or alternatively store the timeout ID in a ref and clear it with a
useEffect cleanup function that fires on component unmount. Ensure that the
mounted state is properly tracked by setting it to true on mount and false on
unmount.
In `@cueweb/components/ui/job-details-inline.tsx`:
- Around line 357-360: Remove the unnecessary arrow function wrapper and type
cast from the frame table's onRowClick handler. The SimpleDataTable component
already passes the extracted row data directly to the callback, so the row
parameter in onRowClick receives the Frame object directly without needing a
cast. Replace the current onRowClick={(row) => handleFrameClick(row as Frame)}
with onRowClick={handleFrameClick} to pass the handler function directly and
match the pattern used in the layer table.
---
Duplicate comments:
In `@cueweb/components/ui/job-details-inline.tsx`:
- Around line 311-316: The job-details-inline view is missing the
DependencyWizardDialog component mount, which prevents the "Dependency
Wizard..." context-menu action from functioning. Import DependencyWizardDialog
if not already imported, then mount it alongside the existing LayerExtraDialogs
component (around line 311-316) and also add it near line 378 where
FrameExtraDialogs is mounted, ensuring both surfaces have the dialog handler
available for layer/frame wizard actions.
🪄 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: 7120fec1-58c6-4518-ab33-849c419ce506
📒 Files selected for processing (2)
cueweb/components/ui/attributes-panel.tsxcueweb/components/ui/job-details-inline.tsx
…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.
- getlog: canonicalize the requested path with realpath and enforce the shared CUEWEB_LOG_ROOTS allow-list (same pattern as stuck-frames/ lastline), plus null-byte / non-absolute rejection. Prevents reading arbitrary server-accessible files via crafted paths. - attributes-panel: clear the CopyButton "copied" reset timer on unmount so it can't fire setState on an unmounted component. - job-details-inline: pass handleFrameClick directly to the frame table's onRowClick (SimpleDataTable already hands back row.original), dropping the redundant `row as Frame` cast.
DiegoTavares
approved these changes
Jun 19, 2026
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
2 participants
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] Job/Layer/Frame context-menu parity + frame log viewer enhancements
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.
Follow tail mode:
New API routes
Frame:
Job:
Layer:
Config
Dockerfile + docker-compose.yml:
Sandbox
load_test_jobs.py:
blendersubcommand renders a real image sequence and registers the layer output path so the frame preview has real framesLLM usage disclosure
Parts of this solution's implementation were developed with assistance from Claude Opus.