Claude/cpu guard review fixes dn d8u - #117
Merged
Merged
Conversation
The ladder previously hardcoded 3 tiers (strike==1 / strike==2 / else), so MAX_STRIKES had no effect — termination always fired on the 3rd hit regardless of configuration. Fix: replace `strikes == 2` with `strikes < MAX_STRIKES` so the STOP/CONT pause repeats for strikes 2..(MAX_STRIKES-1) and termination only triggers at strike >= MAX_STRIKES. All log and Telegram messages now include the current strike / total (e.g. "STRIKE3/5") for clarity. Reported by CodeAnt AI review of PR #116. https://claude.ai/code/session_01WpEqAiDbpqnBywMjYXJHf4
rw_cpu_guard.sh — v3 upgrade:
- --forensics [N] / --forensics=N mode merged from CPU.sh:
vmstat (steal time), iostat (disk wait), 1-second ps tracer
(catches sub-8s spikes), proc-count fork-storm detector, 10s
top snapshots — all collected in /root/rw_cpu_forensics/
- Prints a structured summary after collection: top offenders,
peak proc count, steal-time samples, guard ACT log tail
- Safe to run alongside the daemon simultaneously (passive only)
- FORENSICS_DIR and FORENSICS_DURATION env overrides added
- Header updated to v3; all usage docs updated
runewager_redeploy.sh — new one-shot master deploy script:
1. Stop runewager systemd service
2. Kill anything holding port 3000 (SIGTERM → SIGKILL)
3. npm ci at low priority (nice+ionice)
4. Install/restart rw_cpu_guard via --install
5. Start runewager systemd service
6. Health-check bot at /health (waits up to 30 s)
7. Launch 5-min background forensics trace (non-blocking)
8. Final status summary (services, port, load, memory,
guard ACT log, bot journal tail)
- --dry-run and --skip-forensics flags
- Colour output; VPS path fallback; no tooltips touched
https://claude.ai/code/session_01WpEqAiDbpqnBywMjYXJHf4
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
Reviewer's GuideExtends rw_cpu_guard with a configurable multi-strike kill ladder and a new passive CPU forensics mode, and adds a dedicated runewager_redeploy.sh script that performs a safe, automated redeploy of the bot service with guard installation, health checks, and optional background forensics. Sequence diagram for runewager_redeploy one-shot nuclear redeploysequenceDiagram
actor Admin
participant RedeployScript as runewager_redeploy_sh
participant Systemd as systemd
participant BotService as runewager_service
participant Guard as rw_cpu_guard_sh
participant Health as bot_health_endpoint
participant Forensics as rw_cpu_forensics_trace
Admin->>RedeployScript: run with options (e.g. --skip-forensics, --dry-run)
RedeployScript->>RedeployScript: _check_root
RedeployScript->>RedeployScript: _check_app_dir
%% Step 1: Stop bot service
RedeployScript->>Systemd: systemctl is-active runewager
alt service active
RedeployScript->>Systemd: systemctl stop runewager
else service inactive
RedeployScript->>RedeployScript: log not running
end
%% Step 2: Kill port 3000
RedeployScript->>Systemd: lsof -t -i :3000
alt PIDs found
RedeployScript->>Systemd: kill -TERM PIDs on 3000
RedeployScript->>Systemd: kill -KILL remaining PIDs
else no PIDs
RedeployScript->>RedeployScript: log port free
end
%% Step 3: npm ci
RedeployScript->>RedeployScript: _npm_rebuild (cd app && npm ci)
%% Step 4: Install / restart guard
RedeployScript->>Guard: rw_cpu_guard_sh --install
Guard->>Systemd: install/enable/start rw_cpu_guard service
%% Step 5: Start bot
alt systemd unit exists
RedeployScript->>Systemd: systemctl start runewager
else no systemd unit
alt prod-run script exists
RedeployScript->>BotService: prod-run_sh
else no prod-run
RedeployScript->>Admin: report manual start needed
end
end
%% Step 6: Health check loop
loop until timeout or 200
RedeployScript->>Health: HTTP GET /health
alt status 200
Health-->>RedeployScript: 200 OK + JSON
RedeployScript->>RedeployScript: pretty-print health JSON
RedeployScript->>RedeployScript: break loop
else non 200 or no response
RedeployScript->>RedeployScript: sleep 3s
end
end
%% Step 7: Background forensics
alt SKIP_FORENSICS set
RedeployScript->>RedeployScript: skip forensics
else start forensics
RedeployScript->>Forensics: rw_cpu_guard_sh --forensics=300 &
Forensics->>Forensics: collect vmstat/iostat/ps/top/proc-count
Forensics->>Forensics: write logs under /root/rw_cpu_forensics
end
%% Step 8: Summary
RedeployScript->>Systemd: systemctl is-active runewager
RedeployScript->>Systemd: systemctl is-active rw_cpu_guard
RedeployScript->>Systemd: lsof -t -i :3000
RedeployScript->>Systemd: journalctl -u runewager -n 8
RedeployScript->>Guard: tail /var/log/rw_cpu_guard.log
RedeployScript-->>Admin: final status summary
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughTwo Bash scripts for Runewager bot infrastructure: a new runewager_redeploy.sh orchestrates a complete redeploy workflow with eight sequential steps including dependency rebuilds, service management, health checks, and forensics collection. The rw_cpu_guard.sh script extends with dedicated forensics mode for CPU data collection and enhanced strike/alert messaging. Changes
Sequence DiagramsequenceDiagram
participant Admin as Admin/Operator
participant Deploy as runewager_redeploy.sh
participant Systemd as systemd Service
participant Port as Port 3000
participant NPM as npm/Dependencies
participant Guard as rw_cpu_guard
participant Bot as Bot Service
participant Health as Health Endpoint
participant Monitor as Forensics Monitor
Admin->>Deploy: Execute (optionally with flags)
Deploy->>Systemd: Stop runewager service
Deploy->>Port: Detect & kill processes
Deploy->>NPM: npm ci (rebuild dependencies)
Deploy->>Guard: Install/restart rw_cpu_guard
Deploy->>Systemd: Start bot service<br/>(fallback to prod-run.sh)
Deploy->>Health: Poll /health endpoint<br/>(30s timeout)
Health-->>Deploy: JSON response or timeout
alt Not DRY-RUN and not skip-forensics
Deploy->>Monitor: Start 5-min forensics trace
Monitor->>Monitor: Collect vmstat, iostat, ps, top
end
Deploy->>Deploy: Generate status summary<br/>(service state, port usage,<br/>load/memory, logs)
Deploy-->>Admin: Print final status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
✨ 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 |
|
Caution Docstrings generation - FAILED An unexpected error occurred while creating a local commit: Not Found - https://docs.github.com/rest/git/refs#get-a-reference |
codeant-ai
Bot
added
the
size:XL
Sequence DiagramShows the redeploy script's high-level orchestration (stop service, rebuild, install guard, start service, start background forensics) and the rw_cpu_guard's core strike ladder logic that now honours MAX_STRIKES when responding to CPU hogs. sequenceDiagram
participant AdminScript as RedeployScript
participant System as systemd/OS
participant NPM as npm (rebuild)
participant GuardScript as rw_cpu_guard
participant Bot as Runewager
%% Redeploy happy-path (core steps)
AdminScript->>System: Stop runewager.service
AdminScript->>System: Kill processes on port 3000
AdminScript->>NPM: npm ci (low priority rebuild)
AdminScript->>GuardScript: rw_cpu_guard --install
GuardScript->>System: Install/enable rw_cpu_guard.service
AdminScript->>System: Start runewager.service
AdminScript->>Bot: Health-check (wait for 200)
AdminScript->>GuardScript: Launch background forensics (--forensics) & return
%% Guard strike ladder (core success path)
GuardScript->>GuardScript: Detect CPU hog -> apply throttle cgroup
GuardScript->>GuardScript: increment strikes (pid)
alt strikes == 1
GuardScript->>OS: renice + ionice (soft throttle)
else strikes < MAX_STRIKES
GuardScript->>OS: kill -STOP; sleep; kill -CONT
GuardScript->>AdminScript: send STOP/CONT alert (telegram)
else strikes >= MAX_STRIKES
GuardScript->>OS: kill -TERM; if alive kill -KILL
GuardScript->>AdminScript: send kill alert (telegram)
end
Generated by CodeAnt AI |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The forensics mode hard-codes output to /root/rw_cpu_forensics and /var/log/rw_cpu_forensics_deploy.log; consider failing fast with a clear message if these paths are not writable and/or allowing them to be overridden via environment variables so non-root or constrained environments behave predictably.
- The redeploy script assumes several external tools (lsof, curl, node/npm, systemctl, journalctl) are available; adding explicit preflight checks with clear error messages for missing critical dependencies (e.g. lsof and curl) would make failures easier to diagnose.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The forensics mode hard-codes output to /root/rw_cpu_forensics and /var/log/rw_cpu_forensics_deploy.log; consider failing fast with a clear message if these paths are not writable and/or allowing them to be overridden via environment variables so non-root or constrained environments behave predictably.
- The redeploy script assumes several external tools (lsof, curl, node/npm, systemctl, journalctl) are available; adding explicit preflight checks with clear error messages for missing critical dependencies (e.g. lsof and curl) would make failures easier to diagnose.
## Individual Comments
### Comment 1
<location path="scripts/rw_cpu_guard.sh" line_range="708-709" />
<code_context>
+
+ echo ""
+ echo "── Peak process count (fork-storm indicator) ───"
+ sort -k2 -rn "$proc_log" 2>/dev/null | head -5 \
+ | awk '{printf " %s procs=%s\n", $1, $2}' || echo " (no data)"
+
+ echo ""
</code_context>
<issue_to_address>
**issue (bug_risk):** Process-count summary is sorting/printing by the timestamp fields instead of the count.
Each `proc_log` line is `"<date> <time> <count>"`, so the count is in `$3`. Here you’re sorting on field 2 and printing `$2` as the count, which actually uses the time (`HH:MM:SS`). To show peak process count, this should sort and print field 3 (e.g., `sort -k3 -rn` and `procs=$3`), or you can change the log format so the count becomes the second field and update the `awk` accordingly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+708
to
+709
| sort -k2 -rn "$proc_log" 2>/dev/null | head -5 \ | ||
| | awk '{printf " %s procs=%s\n", $1, $2}' || echo " (no data)" |
There was a problem hiding this comment.
issue (bug_risk): Process-count summary is sorting/printing by the timestamp fields instead of the count.
Each proc_log line is "<date> <time> <count>", so the count is in $3. Here you’re sorting on field 2 and printing $2 as the count, which actually uses the time (HH:MM:SS). To show peak process count, this should sort and print field 3 (e.g., sort -k3 -rn and procs=$3), or you can change the log format so the count becomes the second field and update the awk accordingly.
Nitpicks 🔍
|
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
Comment on lines
+168
to
+174
| pushd "$app" >/dev/null | ||
| # Run at low priority so this install doesn't spike the box | ||
| nice -n 19 ionice -c 3 npm ci --prefer-offline 2>&1 \ | ||
| | tail -5 \ | ||
| | sed 's/^/ /' | ||
| ok "npm ci complete" | ||
| popd >/dev/null |
There was a problem hiding this comment.
Suggestion: The npm ci step always logs a successful completion and returns success even if the npm/ionice pipeline fails, which can mislead operators and cause the redeploy to proceed with a broken or incomplete dependency install; checking the pipeline exit code ensures failures are correctly reported and propagated. [logic error]
Severity Level: Major ⚠️
- ⚠️ Redeploy may proceed after failed dependency installation.
- ⚠️ Operators misled by false-success npm ci log message.
Suggested change
| pushd "$app" >/dev/null | |
| # Run at low priority so this install doesn't spike the box | |
| nice -n 19 ionice -c 3 npm ci --prefer-offline 2>&1 \ | |
| | tail -5 \ | |
| | sed 's/^/ /' | |
| ok "npm ci complete" | |
| popd >/dev/null | |
| pushd "$app" >/dev/null | |
| # Run at low priority so this install doesn't spike the box | |
| nice -n 19 ionice -c 3 npm ci --prefer-offline 2>&1 \ | |
| | tail -5 \ | |
| | sed 's/^/ /' | |
| local npm_status=$? | |
| if (( npm_status == 0 )); then | |
| ok "npm ci complete" | |
| else | |
| warn "npm ci failed with status ${npm_status}" | |
| fi | |
| popd >/dev/null | |
| return "$npm_status" |
Steps of Reproduction ✅
1. Introduce a realistic failure in the dependency install step, for example by
configuring the host without network access or by adding an invalid dependency so that
`npm ci` fails when run in the app directory used by `_npm_rebuild` at
`scripts/runewager_redeploy.sh:146-175`.
2. Execute `sudo ./scripts/runewager_redeploy.sh` so `main()` at `326-361` runs
`_npm_rebuild` at `351-353` after stopping the service and killing the port.
3. Inside `_npm_rebuild`, the pipeline `nice -n 19 ionice -c 3 npm ci --prefer-offline
2>&1 | tail -5 | sed 's/^/ /'` at `170-172` exits with a non-zero status due to the `npm
ci` error, but the function unconditionally prints `ok "npm ci complete"` at `173` and
then executes `popd` at `174`, returning success (exit code 0) from `_npm_rebuild`.
4. `main()` continues to `_install_guard`, `_start_bot_service`, and `_health_check` at
`351-356` without any indication that dependencies failed to rebuild, so operators reading
the redeploy output see a "npm ci complete" message even though the install actually
failed.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** scripts/runewager_redeploy.sh
**Line:** 168:174
**Comment:**
*Logic Error: The `npm ci` step always logs a successful completion and returns success even if the `npm`/`ionice` pipeline fails, which can mislead operators and cause the redeploy to proceed with a broken or incomplete dependency install; checking the pipeline exit code ensures failures are correctly reported and propagated.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
|
CodeAnt AI finished reviewing your PR. |
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.
User description
Summary by Sourcery
Add a CPU forensics mode to the rw_cpu_guard daemon and introduce a one-shot redeploy script for the Runewager bot.
New Features:
Enhancements:
CodeAnt-AI Description
Ensure guard respects configured strike limits and add passive CPU forensics plus a one-shot redeploy script
What Changed
Impact
✅ Fewer unexpected process kills during throttling✅ Clearer kill/throttle notifications (strike counts in logs and Telegram)✅ Easier post-deploy diagnostics with automated forensic traces and a redeploy workflow💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit
New Features
Chores