Claude/cpu guard script dn d8u - #134
Conversation
…eout Telegraf's long-polling uses getUpdates with timeout=50 (Telegram holds the connection for up to 50 seconds waiting for new updates). The callApi patch was wrapping ALL methods — including getUpdates — with globalThrottle, which has a hard 15-second race. This fired on every poll cycle, throwing 'rateLimiter: API call timed out after 15000ms', causing bot.launch() to reject and systemd to enter an infinite restart loop. Fix: add getUpdates (and Telegraf startup/lifecycle calls: getMe, deleteWebhook, setWebhook, getWebhookInfo, close, logOut) to the _MANAGED bypass set so they call the original callApi directly without the timeout wrapper. All 60 unit tests pass. Bot loads cleanly in CI mode. https://claude.ai/code/session_01WpEqAiDbpqnBywMjYXJHf4
…hod sets Two fixes: 1. index.js — ensureQaArtifacts(), persistQaProviderState(), ensureQaDirs(): All QA file writes are now wrapped in try/catch and treated as non-fatal. The bot service runs as the 'runewager' system user but qa/ is owned by root, so writeFileSync was throwing EACCES on every startup, which was caught by startBot()'s outer try/catch and called process.exit(1). 2. telegramSafe.js — callApi bypass list refactored (code review feedback): The inline _MANAGED set is replaced with three named, exported constants: - LONG_POLL_METHODS : getUpdates (must bypass 15 s timeout) - LIFECYCLE_METHODS : getMe, deleteWebhook, setWebhook, etc. - INDIVIDUALLY_PATCHED_METHODS : sendMessage, sendPhoto, etc. Combined into CALLAPI_BYPASS_METHODS used by the callApi patch. All sets are exported so test suites can assert bypass membership without requiring a live Telegraf instance. All 60 unit tests pass. Module loads cleanly in CI mode. https://claude.ai/code/session_01WpEqAiDbpqnBywMjYXJHf4
…e conflict Addresses code review feedback: index.js: - Added QA_FS_SUPPRESS constant (EACCES, EPERM, EROFS, EEXIST) — the set of expected error codes when qa/ is owned by root but the service runs as the runewager user. Defined once, shared by all three QA helpers. - ensureQaDirs: replace silent catch-all with per-directory loop; unexpected error codes (e.g. ENOENT, ENOMEM) are logged as warnings so they remain visible; permission codes are silently skipped. - persistQaProviderState: catch checks err.code — returns silently for permission errors, logs a warning for any other code (e.g. JSON failure). - ensureQaArtifacts: same pattern — unexpected errors are logged at warn level; permission errors (EACCES/EPERM/EROFS) are intentionally silent. telegramSafe.js: - Resolved merge conflict: kept the CALLAPI_BYPASS_METHODS module-level constants introduced in the previous commit; dropped the inline _MANAGED block that arrived from main during the merge. All 60 unit tests pass. https://claude.ai/code/session_01WpEqAiDbpqnBywMjYXJHf4
External code or tests could accidentally mutate the exported Sets (LONG_POLL_METHODS, LIFECYCLE_METHODS, etc.) which would silently alter runtime bypass behavior in callApi. Export frozen array snapshots instead — callers can still iterate and use .includes() for membership checks; the internal module-level Sets remain mutable for runtime use by the callApi patch. All 60 unit tests pass. https://claude.ai/code/session_01WpEqAiDbpqnBywMjYXJHf4
…=false, UMask=0022
ProtectSystem=strict makes the ENTIRE filesystem read-only (including /var),
requiring every directory Node.js touches to be explicitly listed in
ReadWritePaths. This caused silent crashes under systemd while manual
`node index.js` worked fine (no sandbox).
Changes:
- ProtectSystem=strict → full
/usr, /boot, /etc stay read-only; /var (where the app lives) is fully
writable. No more brittle per-directory ReadWritePaths maintenance.
- ProtectHome=true → false
Service runs as root; /root is root's home. Node/npm use /root for
internal caches and temp files — blocking it caused subtle startup
failures that only appeared under systemd sandboxing.
- UMask=0077 → 0022
0077 (owner-only) made log files mode 600, unreadable by log-tailing
utilities running as non-root. 0022 gives the standard 644/755.
- ReadWritePaths kept as belt-and-suspenders documentation; functionally
redundant under ProtectSystem=full but makes intent explicit and guards
against future tightening.
- Expanded inline comments explain each directive's rationale so the next
operator understands the trade-off before changing settings.
All 60 unit tests pass.
https://claude.ai/code/session_01WpEqAiDbpqnBywMjYXJHf4
…s catch blocks
EEXIST fix:
fs.mkdirSync with { recursive: true } never throws EEXIST for an existing
directory — it silently succeeds. The only way EEXIST reaches these catch
blocks is if a *file* exists at the expected directory path, which is a real
misconfiguration that will silently swallow all QA artifact writes. Removing
EEXIST from QA_FS_SUPPRESS lets that case surface as a logged warning.
Comment updated to explain the exclusion.
Non-Error guard fix:
JavaScript allows any value to be thrown, so a catch block's `e` is not
guaranteed to be an Error instance. All three QA fs catch blocks now use
optional chaining (e?.code, e?.message) and fall back to String(e) for the
message field, preventing secondary TypeErrors during error logging.
All 60 tests pass; node --check clean.
https://claude.ai/code/session_01WpEqAiDbpqnBywMjYXJHf4
Merge conflicts (5 hunks in index.js): - Keep HEAD QA_FS_SUPPRESS without EEXIST (correct: recursive mkdirSync never throws EEXIST for dirs, so EEXIST means misconfiguration) - Keep HEAD optional-chaining (e?.code / e?.message ?? String(e)) throughout Review comment — centralize QA_FS_SUPPRESS: - Added authoritative JSDoc explaining the set is the single source of suppression semantics for all QA fs helpers; adding a code here propagates to every caller automatically Review comment — stack trace behind debug flag: - All QA logEvent warn calls now spread stack: e?.stack when LOG_LEVEL=debug Review comment — independent artifact writes in ensureQaArtifacts: - Extracted tryWriteQaJson() helper: each artifact write is now guarded independently so one serialization/path failure cannot block the others - ensureQaArtifacts() restructured to call tryWriteQaJson per file Review comment — runewager.service hardening: - ProtectHome=false → ProtectHome=true; added /root/.npm and /root/.cache to ReadWritePaths so Node/npm caches work without blanket /root access - UMask=0022 → UMask=0027 (files 640, dirs 750): blocks world-readable artifacts while keeping group-read for log-tailing utilities https://claude.ai/code/session_01WpEqAiDbpqnBywMjYXJHf4
runewager.service: - UMask=0022 → UMask=0077; files now default to 600, dirs to 700 - Updated comment documenting per-call mode strategy and noting that systemd StandardOutput/StandardError logs are opened by systemd (not the process) and thus unaffected by UMask; group access via usermod -aG or setfacl is the documented mechanism for those files index.js — explicit mode on every writeFileSync / appendFileSync: - writeFileAtomic (temp file): mode 0o600 — sensitive data (runtime-state.json) - adminLog → adminEventsLogFile: mode 0o640 — log file, group-readable - appendBonusAdminLog → bonusAdminLogFile: mode 0o640 — log file, group-readable - writeQaLog daily log files: mode 0o640 — observability, group-readable - persistQaProviderState → qaProviderStateFile: mode 0o600 — private state - tryWriteQaJson → QA artifact files: mode 0o600 — private artifacts - sshv editor save → session.editorMode.filePath: mode 0o600 — private draft https://claude.ai/code/session_01WpEqAiDbpqnBywMjYXJHf4
…k loops Root cause: step 11 "God-Mode Heal" duplicated the same wait_for_health (20 attempts × 2s = 40s) + systemctl restart block FIVE times back-to-back, totalling up to ~3.5 minutes of wasted waiting even on a clean boot. Changes: - Extracted `_kill_port_squatters()` helper (lsof-first, SIGTERM→SIGKILL) - Step 9c: two-pass aggressive port clear with xargs SIGKILL fallback runs BEFORE systemd restart so the service never races a squatter - Step 11: replaced 5 duplicated heal blocks with a single 30s health probe (15×2s), one optional auto-recovery restart, and a final 20s probe (10×2s) - Telegram notification uses read_env_value() (no raw grep), log capped at 3.5k chars Result: cold-start completes in ~15-35s instead of 3-5 minutes.
… block - xargs -r portability: replaced with explicit empty-guard so BSD xargs works - _REPORT arithmetic expansion bug: $(( ... )) → $( ( ... ) ) — commands now actually execute and log content is captured correctly - _REPORT now wired into Telegram notification text payload - SYSTEMD_ACTIVE moved to after auto-recovery path so summary reflects post-heal state, not pre-restart stale value
When systemctl restart succeeded in step 10, the PID variable was never updated (the PID refresh was inside the || fallback block only). Step 11's auto-recovery path then called _kill_port_squatters with the old stale PID as the "safe" PID — so it killed the new bot process instead of protecting it. Fix: restructure step 10 as an if/else so PID is always refreshed after any restart path (systemctl or nohup fallback), before step 11 runs.
|
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 GuideThis PR hardens restart/health logic and file I/O security: it introduces an aggressive, reusable port-squatter killer around systemd restarts, simplifies and de-duplicates the post-start health check with a single auto-recovery path and Telegram report, and tightens filesystem behavior in index.js by setting explicit file modes, centralizing QA FS error suppression, and making QA artifact writes more robust and observable. Sequence diagram for restart and post-start health checksequenceDiagram
participant ProdRun as prod_run_sh
participant Systemd as systemd
participant NodeApp as node_index_js
participant Health as health_endpoint
participant Telegram as telegram_api
ProdRun->>ProdRun: read_env_value PORT
ProdRun->>ProdRun: _kill_port_squatters(PORT_PRESTART, PID)
alt systemd_available_and_service_file_exists
ProdRun->>Systemd: systemctl restart APP_NAME_service
Systemd-->>ProdRun: exit (success_or_failure)
else no_systemd_or_no_service_file
ProdRun->>NodeApp: kill old PID (if any)
ProdRun->>NodeApp: nohup node index_js
end
ProdRun->>ProdRun: sleep 3
ProdRun->>ProdRun: PID = get_bot_pid()
ProdRun->>ProdRun: HEALTH_URL = resolve_health_url()
ProdRun->>ProdRun: derive HEALTH_PORT from HEALTH_URL
ProdRun->>Health: wait_for_health(HEALTH_URL, 15, 2)
alt health_becomes_healthy_initial
Health-->>ProdRun: healthy
ProdRun->>ProdRun: HEALTH_STATUS = healthy
else health_unhealthy_initial
Health-->>ProdRun: timeout_or_failure
ProdRun->>ProdRun: HEALTH_STATUS = unhealthy
ProdRun->>ProdRun: check is_port_listening(HEALTH_PORT)
alt port_still_listening
ProdRun->>ProdRun: _kill_port_squatters(HEALTH_PORT, PID)
alt systemd_available_and_service_file_exists
ProdRun->>Systemd: systemctl restart APP_NAME_service
else no_systemd_or_no_service_file
ProdRun->>NodeApp: kill current PID (if any)
ProdRun->>NodeApp: nohup node index_js
end
ProdRun->>ProdRun: sleep 3
ProdRun->>ProdRun: PID = get_bot_pid()
else port_not_listening
ProdRun->>ProdRun: skip_additional_port_clear
end
ProdRun->>Health: wait_for_health(HEALTH_URL, 10, 2)
alt health_becomes_healthy_final
Health-->>ProdRun: healthy
ProdRun->>ProdRun: HEALTH_STATUS = healthy
else health_still_unhealthy
Health-->>ProdRun: timeout_or_failure
ProdRun->>ProdRun: HEALTH_STATUS = unhealthy
end
end
ProdRun->>ProdRun: PORT_STATUS = is_port_listening(HEALTH_PORT) ? listening : not_listening
ProdRun->>Systemd: systemctl is-active APP_NAME_service
Systemd-->>ProdRun: SYSTEMD_ACTIVE
ProdRun->>ProdRun: _ADMIN_IDS = read_env_value ADMIN_IDS
ProdRun->>ProdRun: _BOT_TOKEN = read_env_value TELEGRAM_BOT_TOKEN
alt telegram_reporting_enabled
ProdRun->>ProdRun: _REPORT = tail logs (50 total) and truncate
loop each_admin in _ADMIN_IDS
ProdRun->>Telegram: sendMessage(health_summary, PORT, PID, _REPORT)
Telegram-->>ProdRun: response_ignored
end
else telegram_reporting_disabled
ProdRun-->>ProdRun: skip_telegram_notification
end
ProdRun-->>ProdRun: print summary (SYSTEMD_ACTIVE, HEALTH_STATUS, PORT_STATUS)
Class diagram for QA filesystem helpers and secure file writesclassDiagram
class RunewagerApp {
<<module>>
}
class appendBonusAdminLog {
+appendBonusAdminLog(actorId, action, details)
}
class adminLog {
+adminLog(event, payload)
}
class writeFileAtomic {
+writeFileAtomic(filePath, content)
}
class saveJson {
+saveJson(filePath, data)
}
class QAConfig {
+QA_FS_SUPPRESS:Set~string~
}
class ensureQaDirs {
+ensureQaDirs()
}
class writeQaLog {
+writeQaLog(fileName, payload)
}
class persistQaProviderState {
+persistQaProviderState()
}
class tryWriteQaJson {
+tryWriteQaJson(filePath, payload, label)
}
class ensureQaArtifacts {
+ensureQaArtifacts()
}
class refreshQaProviderCooldowns {
+refreshQaProviderCooldowns()
}
class sshv_editor_save_handler {
+sshv_editor_save(ctx)
}
%% Relationships to module
RunewagerApp --> appendBonusAdminLog
RunewagerApp --> adminLog
RunewagerApp --> writeFileAtomic
RunewagerApp --> saveJson
RunewagerApp --> QAConfig
RunewagerApp --> ensureQaDirs
RunewagerApp --> writeQaLog
RunewagerApp --> persistQaProviderState
RunewagerApp --> tryWriteQaJson
RunewagerApp --> ensureQaArtifacts
RunewagerApp --> refreshQaProviderCooldowns
RunewagerApp --> sshv_editor_save_handler
%% QA helper interactions
QAConfig <.. ensureQaDirs : uses_QA_FS_SUPPRESS
QAConfig <.. persistQaProviderState : uses_QA_FS_SUPPRESS
QAConfig <.. tryWriteQaJson : uses_QA_FS_SUPPRESS
ensureQaArtifacts --> ensureQaDirs : calls
ensureQaArtifacts --> tryWriteQaJson : calls
ensureQaArtifacts --> persistQaProviderState : calls
refreshQaProviderCooldowns --> persistQaProviderState : calls
writeQaLog --> ensureQaDirs : uses_indirectly_via_getQaLogDirForToday
saveJson --> writeFileAtomic : calls
sshv_editor_save_handler --> adminLog : calls
%% File mode and encoding behaviors
appendBonusAdminLog : uses fs.appendFileSync(mode 0640, encoding utf8)
adminLog : uses fs.appendFileSync(mode 0640)
writeFileAtomic : uses fs.writeFileSync(tempFile, mode 0600)
persistQaProviderState : uses fs.writeFileSync(mode 0600)
tryWriteQaJson : uses fs.writeFileSync(mode 0600)
writeQaLog : uses fs.appendFileSync(mode 0640, encoding utf8)
sshv_editor_save_handler : uses fs.writeFileSync(mode 0600, encoding utf8)
Flow diagram for aggressive port-squatter clearing before restartflowchart TD
A_start["Start restart sequence"] --> B_read_port["Read PORT_PRESTART from env (default 3000)"]
B_read_port --> C_check_listen{"is_port_listening(PORT_PRESTART)?"}
C_check_listen -- "no" --> Z_continue["Continue to restart logic"]
C_check_listen -- "yes" --> D_log["say 'Port in use — aggressively clearing before restart'" ]
D_log --> E_kill_squatters["_kill_port_squatters(PORT_PRESTART, PID)"]
E_kill_squatters --> F_sleep1["sleep 1"]
F_sleep1 --> G_recheck{"is_port_listening(PORT_PRESTART)?"}
G_recheck -- "no" --> Z_continue
G_recheck -- "yes" --> H_warn["warn 'Port still occupied — forcing SIGKILL'" ]
H_warn --> I_has_lsof{"command -v lsof"}
I_has_lsof -- "yes" --> J_find_strays["_stray = lsof -t -iTCP:PORT_PRESTART -sTCP:LISTEN"]
J_find_strays --> K_kill9["xargs kill -9 on _stray (if any)"]
K_kill9 --> L_sleep2["sleep 1"]
I_has_lsof -- "no" --> L_sleep2
L_sleep2 --> Z_continue
%% _kill_port_squatters internal logic
subgraph S_kill_squatters["_kill_port_squatters(port, own_pid)"]
S1_check_lsof{"command -v lsof"}
S1_check_lsof -- "yes" --> S2_pids_lsof["pids = lsof -t -iTCP:port -sTCP:LISTEN | sort -u"]
S1_check_lsof -- "no" --> S3_pids_fallback["pids = port_listener_pids(port)"]
S2_pids_lsof --> S4_loop["for pid in pids"]
S3_pids_fallback --> S4_loop
S4_loop --> S5_skip_empty{"pid empty?"}
S5_skip_empty -- "yes" --> S4_loop
S5_skip_empty -- "no" --> S6_skip_own{"pid == own_pid?"}
S6_skip_own -- "yes" --> S4_loop
S6_skip_own -- "no" --> S7_warn["warn 'Pre-start: killing PID squatting port'" ]
S7_warn --> S8_term["kill -TERM pid"]
S8_term --> S9_sleep["sleep 1"]
S9_sleep --> S10_still_alive{"kill -0 pid?"}
S10_still_alive -- "yes" --> S11_kill9["kill -KILL pid"]
S10_still_alive -- "no" --> S4_loop
S11_kill9 --> S4_loop
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✨ 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 |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In prod-run.sh, the new systemctl restart block no longer falls back to the manual kill+nohup path when systemctl restart fails (you now do
|| trueinstead of|| { ... }), which means a broken systemd unit could leave the bot stopped; consider reintroducing a fallback start path on restart failure. - The Telegram notification message in prod-run.sh uses
parse_mode=Markdownand embeds raw log content in${_REPORT}and other fields without escaping Markdown control characters, which can break message rendering or truncation; it would be safer to escape or switch to MarkdownV2/HTML and sanitize accordingly.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In prod-run.sh, the new systemctl restart block no longer falls back to the manual kill+nohup path when systemctl restart fails (you now do `|| true` instead of `|| { ... }`), which means a broken systemd unit could leave the bot stopped; consider reintroducing a fallback start path on restart failure.
- The Telegram notification message in prod-run.sh uses `parse_mode=Markdown` and embeds raw log content in `${_REPORT}` and other fields without escaping Markdown control characters, which can break message rendering or truncation; it would be safer to escape or switch to MarkdownV2/HTML and sanitize accordingly.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Nitpicks 🔍
|
| else | ||
| [[ -n "$PID" ]] && kill "$PID" 2>/dev/null || true | ||
| nohup node "$PROJECT_DIR/index.js" >> "$MAIN_LOG" 2>> "$ERROR_LOG" < /dev/null & | ||
| disown |
There was a problem hiding this comment.
Suggestion: Because the script runs with set -e in a non-interactive shell, the bare disown call in the manual restart path will typically fail with "no job control" and exit status 1, causing the entire deployment script to abort on non-systemd systems immediately after starting the bot and before health checks and reporting run; this should be guarded (as in other parts of the script) so that a non-zero status from disown does not terminate the script. [logic error]
Severity Level: Major ⚠️
- ❌ prod-run.sh aborts on non-systemd hosts after restart.
- ⚠️ Post-start health checks never execute on affected systems.
- ⚠️ Telegram deploy health summary is never sent there.| disown | |
| disown || true |
Steps of Reproduction ✅
1. On a host without systemd (no `systemctl` in PATH), run `npm run prod` from
`/workspace/Runewager` which executes `./prod-run.sh` (entrypoint defined in
`package.json:10-15`).
2. Observe `prod-run.sh` starts with `set -euo pipefail` at `prod-run.sh:9`, meaning any
simple command returning non-zero (and not guarded with `|| true`) aborts the script.
3. During section "10) Safe restart" at `prod-run.sh:38-48`, because `command -v
systemctl` fails and `SERVICE_FILE` is irrelevant, the script takes the `else` branch at
`prod-run.sh:42-48`, runs `nohup node "$PROJECT_DIR/index.js" ... &` followed by the bare
`disown` at `prod-run.sh:47` (diff line 506).
4. In this non-interactive, no-job-control shell, `disown` exits with status 1 ("no job
control"); due to `set -e` this non-zero exit causes the entire `prod-run.sh` process to
terminate immediately, so the subsequent post-start health check and Telegram reporting
block at `prod-run.sh:49-79` never execute.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** prod-run.sh
**Line:** 506:506
**Comment:**
*Logic Error: Because the script runs with `set -e` in a non-interactive shell, the bare `disown` call in the manual restart path will typically fail with "no job control" and exit status 1, causing the entire deployment script to abort on non-systemd systems immediately after starting the bot and before health checks and reporting run; this should be guarded (as in other parts of the script) so that a non-zero status from `disown` does not terminate the script.
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. |
User description
Summary by Sourcery
Tighten deployment restart and health-check behavior while hardening filesystem permissions and QA logging for safer production operation.
Bug Fixes:
Enhancements:
CodeAnt-AI Description
Refresh bot PID after restart and avoid restart failures blocking startup
What Changed
Impact
✅ Fewer failed health checks after restart✅ Fewer infinite restart loops✅ Accurate post-start health targeting💡 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.