Remove remaining internal Node repair spawns - #276
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 3 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe PR switches guarded repair to ChangesDashboard repair restart flow
Tmux hyperlink resolver
Node launch inventory checks
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/multiplexer/dashboard-control.ts (1)
562-571: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReminder: rebuild
dist/for this runtime behavior change.This changes aimux runtime repair behavior, so source-level validation alone is insufficient — run
yarn buildto updatedist/.As per coding guidelines: "For aimux runtime or CLI behavior changes, run
yarn buildto updatedist/; source-level validation withyarn vitestandyarn typecheckis not enough".🤖 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 `@src/multiplexer/dashboard-control.ts` around lines 562 - 571, Source-only changes to aimux runtime repair are not enough here; after updating the repair flow in dashboard-control and restartAimuxControlPlane-related behavior, rebuild the project so dist/ reflects the new runtime behavior. Run yarn build to regenerate the compiled output and verify the updated repair timeout/succeed/fail handling is included in the emitted artifacts.Source: Coding guidelines
🤖 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 `@src/multiplexer/dashboard-control.ts`:
- Around line 561-574: The repair timeout path in the dashboard control flow
still leaves restart work active, so later auto-repairs can re-enter while the
original Aimux restart is running. Update the logic around
restartAimuxControlPlane(), the timeout handler, and the fail/succeed flow so
timed-out repairs are gated by the runtime restart lock state or explicitly
cancelled before another attempt can start. Use the existing lock helpers and
the repairTimeout/restart promise handling in dashboard-control.ts to ensure a
second repair cannot proceed until the in-flight restart has settled.
---
Nitpick comments:
In `@src/multiplexer/dashboard-control.ts`:
- Around line 562-571: Source-only changes to aimux runtime repair are not
enough here; after updating the repair flow in dashboard-control and
restartAimuxControlPlane-related behavior, rebuild the project so dist/ reflects
the new runtime behavior. Run yarn build to regenerate the compiled output and
verify the updated repair timeout/succeed/fail handling is included in the
emitted artifacts.
🪄 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: 2e8cc346-9302-49eb-904e-65565e437b10
📒 Files selected for processing (4)
scripts/tmux-open-hyperlink.shsrc/multiplexer/dashboard-control.test.tssrc/multiplexer/dashboard-control.tssrc/one-shot-node-inventory.test.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/one-shot-node-inventory.test.ts (1)
14-20: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winBroaden the
project-restart-climatch.
/"restart",\s*"--project"/only catches one literal array shape, so a reintroduction using different quoting or a slightly different call form could slip past this guard. Matching both tokens more structurally would make the regression check less brittle.🤖 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 `@src/one-shot-node-inventory.test.ts` around lines 14 - 20, Broaden the `project-restart-cli` pattern in `runtimeNodeLaunchPatterns` so it matches the `restart` and `--project` tokens more structurally instead of relying on one exact quoted array shape. Update the regex in `src/one-shot-node-inventory.test.ts` to tolerate different quoting and spacing/call forms while still identifying the same restart CLI usage.
🤖 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 `@src/one-shot-node-inventory.test.ts`:
- Around line 14-20: Broaden the `project-restart-cli` pattern in
`runtimeNodeLaunchPatterns` so it matches the `restart` and `--project` tokens
more structurally instead of relying on one exact quoted array shape. Update the
regex in `src/one-shot-node-inventory.test.ts` to tolerate different quoting and
spacing/call forms while still identifying the same restart CLI usage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8bb0812e-b65a-4ecc-b10d-e5e605ee21a2
📒 Files selected for processing (5)
src/multiplexer/dashboard-control.test.tssrc/multiplexer/dashboard-control.tssrc/one-shot-node-inventory.test.tssrc/runtime-restart.test.tssrc/runtime-restart.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/multiplexer/dashboard-control.ts
- src/multiplexer/dashboard-control.test.ts
|
Sub-agent review finding fixed in 7d5f2d3: dashboard-initiated control-plane restarts are now abortable, the dashboard timeout aborts the in-process restart, and a regression test proves the global restart lock is released even when the owner PID remains alive. |
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)
src/runtime-restart.ts (1)
674-688: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winRegenerate
dist/for this runtime change.This alters
restartAimuxControlPlaneruntime behavior, so a source-only validation isn't sufficient—runyarn buildto updatedist/before merge.As per coding guidelines: "For aimux runtime or CLI behavior changes, run
yarn buildto updatedist/; source-level validation withyarn vitestandyarn typecheckis not enough".🤖 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 `@src/runtime-restart.ts` around lines 674 - 688, This change updates runtime behavior in restartAimuxControlPlane, so the compiled artifacts need to be regenerated as well. After making the source change, run yarn build to refresh dist/ for the affected runtime code, rather than relying only on source-level checks; use the restartAimuxControlPlane and restartAimuxControlPlaneUnlocked flow as the reference point when verifying the built output.Source: Coding guidelines
🤖 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 `@src/runtime-restart.ts`:
- Around line 682-687: The restart lock is being released too early in the
restart flow, allowing a second restart to overlap while the first one is still
tearing down. In runtime-restart.ts, update the logic around
restartAimuxControlPlaneUnlocked and raceRestartWithAbort so the lock is not
released until the restart work has fully settled, either by awaiting the
restart promise itself or by adding a bounded drain after abort before calling
releaseRuntimeRestartLock.
---
Outside diff comments:
In `@src/runtime-restart.ts`:
- Around line 674-688: This change updates runtime behavior in
restartAimuxControlPlane, so the compiled artifacts need to be regenerated as
well. After making the source change, run yarn build to refresh dist/ for the
affected runtime code, rather than relying only on source-level checks; use the
restartAimuxControlPlane and restartAimuxControlPlaneUnlocked flow as the
reference point when verifying the built output.
🪄 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: e519ec44-2518-47e2-9920-2e0b37b03aaf
📒 Files selected for processing (4)
src/multiplexer/dashboard-control.test.tssrc/multiplexer/dashboard-control.tssrc/runtime-restart.test.tssrc/runtime-restart.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/multiplexer/dashboard-control.ts
- src/multiplexer/dashboard-control.test.ts
1 participant
Summary
aimux restart --projectVerification
PATH="$HOME/.nvm/versions/node/v24.16.0/bin:$PATH" yarn typecheckPATH="$HOME/.nvm/versions/node/v24.16.0/bin:$PATH" yarn lintPATH="$HOME/.nvm/versions/node/v24.16.0/bin:$PATH" yarn vitest runPATH="$HOME/.nvm/versions/node/v24.16.0/bin:$PATH" yarn buildyarn typecheck && yarn lint && yarn testSummary by CodeRabbit
Bug Fixes
Tests