Fix restart zombie PID handling and document local install flow - #200
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
More reviews will be available in 10 minutes and 6 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?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 credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. 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, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds zombie-process detection to ChangesZombie Process Liveness Fix
Local Release Install Documentation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 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)
Comment |
![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
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)
117-131:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
psunavailability causes false negatives for live processes.If
execFileSync("ps", ...)throws becausepsis not installed (e.g., minimal Alpine containers, some embedded environments), the catch block returnsfalse, incorrectly reporting an alive process as dead. This would causewaitForPidExitto return immediately without waiting, potentially racing with process cleanup.Consider isolating the
psfailure from "process not alive" by catching and handling theexecFileSyncerror separately:Proposed fix to handle ps unavailability gracefully
function defaultIsPidAlive(pid: number): boolean { try { process.kill(pid, 0); - if (process.platform !== "win32") { - const state = execFileSync("ps", ["-o", "stat=", "-p", String(pid)], { - encoding: "utf8", - stdio: ["ignore", "pipe", "ignore"], - }).trim(); - if (state.startsWith("Z")) return false; - } - return true; } catch { return false; } + if (process.platform !== "win32") { + try { + const state = execFileSync("ps", ["-o", "stat=", "-p", String(pid)], { + encoding: "utf8", + stdio: ["ignore", "pipe", "ignore"], + }).trim(); + if (state.startsWith("Z")) return false; + } catch { + // ps unavailable or failed; fall through to treat as alive + } + } + return true; }🤖 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 117 - 131, The defaultIsPidAlive function conflates two different failure scenarios: when the process is truly not alive and when the ps command is unavailable. When execFileSync throws an exception (e.g., ps command not found), the outer catch block returns false, incorrectly marking alive processes as dead. To fix this, wrap only the execFileSync call in a separate try-catch block that gracefully handles the error by returning true (since process.kill(pid, 0) already succeeded), and reserve the outer catch block only for when process.kill itself throws an exception, indicating the process is actually not alive. This way, unavailability of ps does not cause false negatives for live processes.
♻️ Duplicate comments (2)
README.md (1)
87-89:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSame hardcoded
darwin-arm64platform issue in "Build from source" section.Line 89 has the same platform hardcoding concern as the earlier "frozen local build" example. Users on non-ARM64 systems will not be able to copy this command as-is.
🤖 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 `@README.md` around lines 87 - 89, The scripts/install.sh command on line 89 contains a hardcoded platform reference aimux-darwin-arm64.tar.gz which assumes ARM64 architecture and will fail for users on other platforms. Replace the hardcoded platform string with a variable or dynamic reference that detects the current system architecture and selects the appropriate binary file (such as aimux-darwin-x86_64.tar.gz for Intel systems). Consider using a pattern that matches the output from the yarn release:asset command or documenting platform-specific variations for different architectures.AGENTS.md (1)
102-103:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSame hardcoded
darwin-arm64platform issue in AGENTS.md code block.The bash example in the added code block (lines 102–103) has the same platform-hardcoding concern as README.md. Users on non-Apple-Silicon systems will not be able to copy this command verbatim.
🤖 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 `@AGENTS.md` around lines 102 - 103, The bash example in AGENTS.md has a hardcoded platform identifier `darwin-arm64` in the file path `release/aimux-darwin-arm64.tar.gz`, which will not work for users on different platforms or architectures. Replace the hardcoded platform string in the tar.gz filename with a variable placeholder or instruction that allows users to dynamically specify their appropriate platform (such as darwin-arm64, darwin-x64, linux-x64, etc.) so the command is usable across different systems.
🤖 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 `@README.md`:
- Around line 66-67: The install example in the README hardcodes darwin-arm64
platform identifier, which leaves users on other platforms (Intel macOS, Linux,
Windows) without clear guidance on which tarball to use. Replace the hardcoded
darwin-arm64 example with either: a documentation section listing all supported
platform identifiers (darwin-x64, linux-x64, linux-arm64, win32-x64,
darwin-arm64) with brief descriptions of which systems they target, or add
instructions showing how to determine the correct platform identifier for the
user's system before running the scripts/install.sh command. Ensure the example
or guidance makes it clear how to substitute the correct platform identifier for
the AIMUX_RELEASE_VERSION and scripts/install.sh lines.
---
Outside diff comments:
In `@src/runtime-restart.ts`:
- Around line 117-131: The defaultIsPidAlive function conflates two different
failure scenarios: when the process is truly not alive and when the ps command
is unavailable. When execFileSync throws an exception (e.g., ps command not
found), the outer catch block returns false, incorrectly marking alive processes
as dead. To fix this, wrap only the execFileSync call in a separate try-catch
block that gracefully handles the error by returning true (since
process.kill(pid, 0) already succeeded), and reserve the outer catch block only
for when process.kill itself throws an exception, indicating the process is
actually not alive. This way, unavailability of ps does not cause false
negatives for live processes.
---
Duplicate comments:
In `@AGENTS.md`:
- Around line 102-103: The bash example in AGENTS.md has a hardcoded platform
identifier `darwin-arm64` in the file path `release/aimux-darwin-arm64.tar.gz`,
which will not work for users on different platforms or architectures. Replace
the hardcoded platform string in the tar.gz filename with a variable placeholder
or instruction that allows users to dynamically specify their appropriate
platform (such as darwin-arm64, darwin-x64, linux-x64, etc.) so the command is
usable across different systems.
In `@README.md`:
- Around line 87-89: The scripts/install.sh command on line 89 contains a
hardcoded platform reference aimux-darwin-arm64.tar.gz which assumes ARM64
architecture and will fail for users on other platforms. Replace the hardcoded
platform string with a variable or dynamic reference that detects the current
system architecture and selects the appropriate binary file (such as
aimux-darwin-x86_64.tar.gz for Intel systems). Consider using a pattern that
matches the output from the yarn release:asset command or documenting
platform-specific variations for different architectures.
🪄 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: 26c53c29-a2a4-4ce6-885b-03f1fe2d4126
📒 Files selected for processing (3)
AGENTS.mdREADME.mdsrc/runtime-restart.ts
1 participant
Summary
aimuxuses the installed bundle behind~/.local/bin/aimuxyarn linkguidance with the local release install flowVerification
aimux restart --jsonandaimux doctor versionsSummary by CodeRabbit
Documentation
Improvements