Claude/cpu guard script dn d8u - #130
Merged
Merged
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
|
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 · |
|
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 (3)
✨ 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 |
Reviewer's GuideRefactors Telegram callApi rate-limiter bypass logic into reusable, exported method sets, and makes QA artifact directory/file creation fault-tolerant by handling permission-related filesystem errors gracefully while logging unexpected failures. Sequence diagram for updated Telegram tg.callApi rate-limiter bypasssequenceDiagram
participant Caller
participant TelegrafBot
participant TgContext as TgContext_tg
participant GlobalThrottle
participant TelegramAPI
Caller->>TelegrafBot: invoke_telegraf_method
TelegrafBot->>TgContext: callApi(method, data, signal)
alt method in CALLAPI_BYPASS_METHODS
TgContext->>TelegramAPI: original_callApi(method, data, signal)
TelegramAPI-->>TgContext: response
TgContext-->>TelegrafBot: response
TelegrafBot-->>Caller: response
else method not in CALLAPI_BYPASS_METHODS
TgContext->>GlobalThrottle: schedule(original_callApi, method, data, signal)
GlobalThrottle->>TelegramAPI: original_callApi(method, data, signal)
TelegramAPI-->>GlobalThrottle: response
GlobalThrottle-->>TgContext: response
TgContext-->>TelegrafBot: response
TelegrafBot-->>Caller: response
end
Flow diagram for QA directory creation with suppressed filesystem errorsflowchart TD
A[start ensureQaDirs] --> B[dir in qaContextDir, qaStateDir, qaLogsDir]
B --> C["fs.mkdirSync(dir, recursive true)"]
C --> D{error thrown?}
D -- no --> E[continue to next dir]
D -- yes --> F{e.code in QA_FS_SUPPRESS?}
F -- yes --> E
F -- no --> G[logEvent warn ensureQaDirs: unexpected error]
G --> E
E --> H{more dirs?}
H -- yes --> B
H -- no --> I[end ensureQaDirs]
subgraph QA_FS_SUPPRESS_values
J[EACCES]
K[EPERM]
L[EROFS]
M[EEXIST]
end
Flow diagram for fault-tolerant QA artifact persistenceflowchart TD
A[start ensureQaArtifacts] --> B[try block]
B --> C[ensureQaDirs]
C --> D[write qaRepoInfoFile]
D --> E[write qaCapabilitiesFile]
E --> F[persistQaProviderState]
F --> G[end ensureQaArtifacts]
B -->|error e| H{e.code in QA_FS_SUPPRESS?}
H -- yes --> G
H -- no --> I[logEvent warn ensureQaArtifacts: unexpected error]
I --> G
subgraph persistQaProviderState_logic
J[start persistQaProviderState] --> K[try block]
K --> L[ensureQaDirs]
L --> M[write qaProviderStateFile]
M --> N[end persistQaProviderState]
K -->|error e| O{e.code in QA_FS_SUPPRESS?}
O -- yes --> N
O -- no --> P[logEvent warn persistQaProviderState: unexpected error]
P --> N
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
codeant-ai
Bot
added
the
size:L
![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:
- In the QA filesystem handling,
QA_FS_SUPPRESSis documented as covering permission-related cases but also includesEEXIST, which is not a permission error; consider either updating the comment to reflect this or droppingEEXISTfrom the suppression set so unexpectedEEXISTcases continue to be logged. - In the
fserror handlers you assumeehascodeandmessagefields; if there is any chance a non-Error value bubbles up, it may be safer to guard those accesses (e.g.,e && e.code) or fall back to a generic message to avoid secondary failures during logging.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the QA filesystem handling, `QA_FS_SUPPRESS` is documented as covering permission-related cases but also includes `EEXIST`, which is not a permission error; consider either updating the comment to reflect this or dropping `EEXIST` from the suppression set so unexpected `EEXIST` cases continue to be logged.
- In the `fs` error handlers you assume `e` has `code` and `message` fields; if there is any chance a non-Error value bubbles up, it may be safer to guard those accesses (e.g., `e && e.code`) or fall back to a generic message to avoid secondary failures during logging.
## Individual Comments
### Comment 1
<location path="index.js" line_range="4832-4833" />
<code_context>
writeFileAtomic(filePath, JSON.stringify(data, null, 2));
}
+// Expected error codes when the service user cannot write to qa/ (created as root).
+const QA_FS_SUPPRESS = new Set(['EACCES', 'EPERM', 'EROFS', 'EEXIST']);
+
function ensureQaDirs() {
</code_context>
<issue_to_address>
**issue (bug_risk):** Suppressing EEXIST for mkdir can hide a real configuration problem when a qa path is a file rather than a directory.
With `fs.mkdirSync`, `EEXIST` also covers the case where a file already exists at the target path (e.g. `qa/` or `qa/logs` is a file, not a directory). In that case we likely *do* want to surface an error, since QA artifacts will never be written and this will be hard to debug. Consider either removing `EEXIST` from `QA_FS_SUPPRESS`, or only suppressing it after verifying the existing path is a directory (e.g. via `fs.statSync`).
</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
+4832
to
+4833
| // Expected error codes when the service user cannot write to qa/ (created as root). | ||
| const QA_FS_SUPPRESS = new Set(['EACCES', 'EPERM', 'EROFS', 'EEXIST']); |
There was a problem hiding this comment.
issue (bug_risk): Suppressing EEXIST for mkdir can hide a real configuration problem when a qa path is a file rather than a directory.
With fs.mkdirSync, EEXIST also covers the case where a file already exists at the target path (e.g. qa/ or qa/logs is a file, not a directory). In that case we likely do want to surface an error, since QA artifacts will never be written and this will be hard to debug. Consider either removing EEXIST from QA_FS_SUPPRESS, or only suppressing it after verifying the existing path is a directory (e.g. via fs.statSync).
Nitpicks 🔍
|
|
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
Refine Telegram API rate-limiting bypass configuration and make QA artifact filesystem failures non-fatal while logging unexpected errors.
Enhancements:
CodeAnt-AI Description
Prevent Telegram long-poll timeouts and make QA artifact writes non-fatal
What Changed
Impact
✅ Stable long-polling without API call timeouts✅ Bot no longer crashes on startup when QA files are unwritable✅ Fewer systemd restart loops and permission-related startup failures💡 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.