fix(dashboard): prevent supervisor queue poll from overwriting just-saved order - #656
Conversation
…aved order Add save-in-flight flag and 5-second grace period so the 30-second polling loop skips queue fetches immediately after a save, preventing the snap-back race condition. Also improve drag-and-drop with directional drop indicators (top/bottom border) and guard _specs_dir against None project_dir in multi-project mode. Closes #632
|
Warning Review limit reached
Next review available in: 105 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
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 |
|
xsovad06
left a comment
There was a problem hiding this comment.
PR Summary
Single-commit PR by xsovad06 fixing three related issues: (1) a race condition where the supervisor queue polling loop overwrites just-saved reorder data, (2) drag-and-drop UX improvement with directional drop indicators, and (3) a crash in _specs_dir() when project_dir is None in multi-project mode. 3 files changed (+65/-7), 1 commit.
Findings
No blocking or actionable findings. The changes are well-scoped and correct.
The insertion index arithmetic in queueDrop (insertIdx adjustment for _dragIdx < targetIdx) correctly compensates for the splice-then-insert offset. The _queueSaveInFlight flag with finally cleanup and the 5-second _queueLastSaved grace period together form a two-layer guard against the poll-overwrite race. All five queue-mutating paths (saveQueue, addToQueue, removeFromQueue, clearQueue, and moveQueueItem/queueDrop via saveQueue) correctly set _queueLastSaved.
The _specs_dir None guard provides defense-in-depth alongside the existing guard in dependency_graph.py:84.
Verdict
Approve. Clean, focused fix with good test coverage.
What's Done Well
-
The two-layer poll guard (
_queueSaveInFlight+_queueLastSavedgrace period) is a robust pattern. The flag catches in-flight saves, and the timestamp catches the window between save completion and the next poll cycle. Either alone would have gaps. -
The spec_service None guard is placed at exactly the right level (
_specs_diritself), making every downstream caller safe without requiring changes in each one. The 5 new tests cover all public entry points. -
The drag-and-drop
_dropPositionlogic with midpoint calculation gives users clear visual feedback about where the drop will land, which was missing before. The border cleanup inqueueDragEndandqueueDragLeavecovers all exit paths.
(Posted as comment: GitHub does not allow self-reviews with formal approval/rejection state.)



Summary
_specs_dir()againstNoneproject dir to prevent crashes in multi-project mode.Closes #632
Changes
Queue poll-overwrite guard (
supervisor.html):_queueSaveInFlightflag (blocks poll during in-flight saves) and_queueLastSavedtimestamp (5-second grace period after any queue mutation).saveQueue,addToQueue,removeFromQueue,clearQueue, and implicitly viasaveQueueformoveQueueItem/queueDrop.Drag-and-drop polish (
supervisor.html):queueDragOvercalculates cursor position relative to element midpoint and highlights top or bottom border accordingly.queueDropinserts above or below the target based on the last drag-over position.borderTopColorandborderBottomColor.spec_service None guard (
spec_service.py):_specs_dir()returnsNonewhen bothproject_dirarg andget_project_dir()areNone.find_spec_file,_iter_all_specs) handleNonegracefully.dependency_graph.pyalready had its own guard at line 84 (unchanged).Review guidance
_queueLastSavedis set in ALL queue-mutating paths, not justsaveQueue.insertIdxadjustment when_dragIdx < targetIdx) handles the splice-then-insert offset correctly.Test plan
TestSpecsDirNoneGuardcovering_specs_dir,find_spec_file,read_spec,list_all_specs, andlist_pending_specswithNoneproject dirmake checkpasses (5896 passed, 1 pre-existing env-dependent failure intest_scheduler.py)