Claude/cpu guard script dn d8u - #121
Conversation
|
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 GuideAdds a dedicated HTTP endpoint service with secure Autofix webhook handling and admin notifications, introduces a robust Telegram API rate-limiting layer used by both the bot and the new backend, and hardens deployment/CPU-guard scripts for better service coordination and diagnostics. Sequence diagram for Autofix webhook handling in the new HTTP endpoint servicesequenceDiagram
participant AutofixService
participant RunewagerEndpoint as runewager_endpoint
participant verifySignature
participant sendAdmin
participant TelegramAPI
AutofixService->>RunewagerEndpoint: POST /autofix/webhook (body, signature)
RunewagerEndpoint->>RunewagerEndpoint: assign requestId
RunewagerEndpoint->>verifySignature: verifySignature(rawBody, signature)
verifySignature-->>RunewagerEndpoint: isValid
alt signature invalid
RunewagerEndpoint->>RunewagerEndpoint: logger.error(autofix.webhook, Signature mismatch)
RunewagerEndpoint-->>AutofixService: 401 { ok: false, error: Invalid signature }
else signature valid
RunewagerEndpoint->>RunewagerEndpoint: parse payload, determine eventName
RunewagerEndpoint->>RunewagerEndpoint: logger.event(autofix.webhook, Autofix event received)
RunewagerEndpoint->>sendAdmin: sendAdmin("Autofix [requestId]: eventName")
sendAdmin->>TelegramAPI: HTTPS POST /bot<TG_TOKEN>/sendMessage
TelegramAPI-->>sendAdmin: 200 OK (per admin chat)
sendAdmin-->>RunewagerEndpoint: Promise resolved
RunewagerEndpoint-->>AutofixService: 200 { ok: true, received: true }
end
Note over RunewagerEndpoint,TelegramAPI: On errors, endpoint logs and uses sendAdmin to notify admins, then returns 500
Sequence diagram for Telegram API calls through telegramSafe and rateLimitersequenceDiagram
actor User
participant TelegramClient as telegram_user
participant TelegramAPI
participant TelegrafBot as telegraf_bot
participant telegramSafe
participant rateLimiter
User->>TelegramClient: send command/message
TelegramClient->>TelegramAPI: deliver update
TelegramAPI-->>TelegrafBot: webhook/polling update
TelegrafBot->>TelegrafBot: middleware handles ctx
TelegrafBot->>TelegrafBot: ctx.reply(text)
TelegrafBot->>telegramSafe: patched bot.telegram.sendMessage(chatId, text, extra)
telegramSafe->>rateLimiter: enqueue(chatId, fn)
rateLimiter->>rateLimiter: apply per-chat queue and globalThrottle
rateLimiter->>TelegramAPI: execute fn() → sendMessage(chatId, text, extra)
TelegramAPI-->>rateLimiter: API response
rateLimiter-->>telegramSafe: result
telegramSafe-->>TelegrafBot: Promise resolved
TelegrafBot-->>TelegramClient: message appears in chat
Class diagram for new rateLimiter, telegramSafe, and backend modulesclassDiagram
class rateLimiter {
<<module>>
-Map~string, Promise~any~~ _chatQueues
-Promise~void~ _globalTail
-number _globalLast
-GLOBAL_GAP_MS : number
-CHAT_GAP_MS : number
-_delay(ms number) Promise~void~
-_globalSlot(fn Function) Promise~any~
+enqueue(chatId string, fn Function) Promise~any~
+globalThrottle(fn Function) Promise~any~
+stats() RateLimiterStats
}
class RateLimiterStats {
+pendingQueues : number
+globalLastMs : number
}
class telegramSafe {
<<module>>
-_bot : Telegraf
-_inited : boolean
-_withRetry(fn Function, maxRetries number) Promise~any~
-_assertInited() void
+init(bot Telegraf) void
+sendMessage(chatId string, text string, options object) Promise~Message~
+editMessageText(chatId string, messageId number, text string, options object) Promise~Message_or_boolean~
+answerCallbackQuery(callbackId string, options object) Promise~boolean~
+sendPhoto(chatId string, photo any, options object) Promise~Message~
+sendDocument(chatId string, document any, options object) Promise~Message~
}
class backend_js {
<<module>>
-PORT : number
-AUTOFIX_SECRET : string
-TG_TOKEN : string
-ADMIN_CHAT_IDS : string[]
-LOG_DIR : string
-LOG_FILE : string
-ERR_FILE : string
-_logStream : WriteStream
-_errStream : WriteStream
+sendAdmin(msg string) Promise~void~
+verifySignature(rawBody Buffer, signature string) boolean
}
class logger {
<<object>>
+info(eventType string, msg string, extra object) void
+error(eventType string, msg string, extra object) void
+event(eventType string, msg string, extra object) void
-_entry(level string, eventType string, msg string, extra object) object
-_write(streams WriteStream[], obj object) void
}
class express_app {
<<object>>
+get(path string, handler Function) void
+post(path string, handler Function) void
+use(middleware Function) void
+listen(port number, host string, callback Function) Server
}
class Server {
+close(callback Function) void
}
telegramSafe --> rateLimiter : uses
backend_js --> logger : uses
backend_js --> express_app : configures routes
backend_js --> Server : creates via app.listen
backend_js --> sendAdmin : exports function
backend_js --> verifySignature : exports function
express_app --> logger : logs in handlers
rateLimiter --> RateLimiterStats : returns
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. 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughIntroduces a new backend HTTP service (backend.js) running on port 3001 that accepts webhooks, validates signatures via HMAC-SHA256, logs events, broadcasts admin notifications through Telegram, and exposes health-check endpoints. Includes rate-limiting infrastructure (rateLimiter.js, telegramSafe.js) to throttle Telegram API calls, along with configuration, systemd unit, and deployment script updates. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Backend as backend.js
participant TelegramAPI as Telegram API
participant Admin as Admin Chat
Client->>Backend: POST /autofix/webhook<br/>(payload + signature)
activate Backend
Backend->>Backend: Capture raw body
Backend->>Backend: Verify HMAC-SHA256<br/>signature
alt Signature Invalid
Backend->>Client: 401 Unauthorized
Backend->>TelegramAPI: sendAdmin(crash alert)
TelegramAPI->>Admin: Error notification
else Signature Valid
Backend->>Backend: Parse payload<br/>Assign requestId<br/>Log event
Backend->>TelegramAPI: sendAdmin(webhook received)
TelegramAPI->>Admin: Admin notification
Backend->>Client: 200 OK
end
deactivate Backend
sequenceDiagram
participant Caller
participant telegramSafe as telegramSafe.js
participant RateLimiter as rateLimiter.js
participant TelegramAPI as Telegram API
Caller->>telegramSafe: sendMessage(chatId, text)
activate telegramSafe
telegramSafe->>RateLimiter: enqueue(chatId, fn)
activate RateLimiter
RateLimiter->>RateLimiter: Per-chat queue + Global throttle
RateLimiter->>TelegramAPI: Call when ready
activate TelegramAPI
alt 429 Rate Limited
TelegramAPI-->>RateLimiter: retry_after
RateLimiter->>RateLimiter: Exponential backoff
RateLimiter->>TelegramAPI: Retry
end
TelegramAPI-->>RateLimiter: Success
deactivate TelegramAPI
RateLimiter-->>telegramSafe: Result
deactivate RateLimiter
telegramSafe-->>Caller: Resolved promise
deactivate telegramSafe
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Sequence DiagramThe PR adds a companion HTTP endpoint that accepts Autofix webhooks, verifies HMAC signatures, logs the event, and broadcasts a short admin message via a shared, rate-limited Telegram wrapper. It also patches the running bot to route all Telegram calls through the same rate limiter. sequenceDiagram
participant Autofix
participant Endpoint as runewager-endpoint
participant Logger
participant RateLimiter as telegramSafe/rateLimiter
participant Telegram as TelegramAPI
Autofix->>Endpoint: POST /autofix/webhook (raw body + signature)
Endpoint->>Endpoint: Verify HMAC signature
Endpoint->>Logger: Log received autofix event
Endpoint->>RateLimiter: sendAdmin("Autofix [id]: <event>") (enqueue)
RateLimiter->>Telegram: Throttle + (retry on 429/transient) → sendMessage
Telegram-->>RateLimiter: 200 OK
RateLimiter-->>Endpoint: sendAdmin resolved
Endpoint-->>Autofix: 200 OK (received)
Generated by CodeAnt AI |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In telegramSafe.answerCallbackQuery you always call _bot.telegram.answerCbQuery(), but init() supports both answerCbQuery and answerCallbackQuery; consider using whichever method actually exists to avoid a runtime error if only the alias is present.
- rateLimiter.stats() is currently unused; wiring it into /health/full (or another diagnostic endpoint) would make the new rate limiter observable in production and help debug queueing behavior.
- backend.js sendAdmin() talks directly to the Telegram HTTP API and bypasses the new rate-limiting layer; if these broadcasts can be frequent, consider routing them through telegramSafe to avoid surprising rate-limit interactions.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In telegramSafe.answerCallbackQuery you always call _bot.telegram.answerCbQuery(), but init() supports both answerCbQuery and answerCallbackQuery; consider using whichever method actually exists to avoid a runtime error if only the alias is present.
- rateLimiter.stats() is currently unused; wiring it into /health/full (or another diagnostic endpoint) would make the new rate limiter observable in production and help debug queueing behavior.
- backend.js sendAdmin() talks directly to the Telegram HTTP API and bypasses the new rate-limiting layer; if these broadcasts can be frequent, consider routing them through telegramSafe to avoid surprising rate-limit interactions.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Nitpicks 🔍
|
…t failure rw_cpu_guard.sh: - CSV header: cmd → args to match the ps field now being recorded runewager_redeploy.sh: - Extract _show_payload() helper: captures formatter output into a variable, then falls back to raw only when output is empty/non-zero; fixes the silent-failure bug where sed's exit code masked python3/jq failures so the || fallback never triggered - Replace three duplicated 'echo | head -20 | sed' blocks with a single call to _show_payload(), keeping the python3 → jq → raw cascade https://claude.ai/code/session_01WpEqAiDbpqnBywMjYXJHf4
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (1)
runewager-endpoint.service (1)
38-40: Plan log rotation for append-only service logs.
backend.log/backend-error.logcan grow indefinitely and eventually impact disk availability. Consider journald output or a guaranteed logrotate policy for these files.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@runewager-endpoint.service` around lines 38 - 40, The service currently uses append-only files (StandardOutput=append:/var/www/html/Runewager/logs/backend.log and StandardError=append:/var/www/html/Runewager/logs/backend-error.log) which can grow indefinitely; either switch the unit to use the journal (e.g., StandardOutput=journal and StandardError=journal) so systemd/journald handles rotation, or keep the files but add a guaranteed logrotate policy for /var/www/html/Runewager/logs/backend.log and backend-error.log and ensure the unit’s ReadWritePaths remains correct; update the unit’s StandardOutput/StandardError lines or create a logrotate config that rotates, compresses, and sets max age/size for those filenames and reload systemd/logrotate accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend.js`:
- Around line 186-189: Currently the webhook verification is fail-open when
AUTOFIX_SECRET is unset; change the logic in the same handler that references
AUTOFIX_SECRET and verifySignature so requests are rejected unless a secret is
configured and signature verification succeeds: if AUTOFIX_SECRET is falsy, log
an error via logger (e.g., logger.error with context like 'autofix.webhook' and
requestId) and return res.status(401).json({ ok: false, error: 'Webhook secret
not configured', requestId }) instead of accepting the request, and keep the
existing verifySignature(rawBody, sig) check when the secret is present to
return 401 on signature mismatch.
- Around line 72-89: sendAdmin currently issues Telegram https.request calls
with no timeout, allowing a stalled socket to hang processing; update sendAdmin
to set a per-request timeout (e.g., 5s) on each request and ensure the timeout
handler aborts/destroys the request and resolves the Promise so Promise.all
cannot hang. Specifically, inside the ADMIN_CHAT_IDS.map callback that creates
the req via https.request, call req.setTimeout or attach a 'socket' listener to
call socket.setTimeout, and in the timeout callback call
req.destroy()/req.abort() and resolve(); also ensure existing 'error' and
response handlers still resolve to avoid unhandled rejections. Reference
symbols: sendAdmin, ADMIN_CHAT_IDS, TG_TOKEN, https.request,
req.setTimeout/req.destroy (or socket.setTimeout).
- Line 40: The unguarded filesystem call around LOG_DIR can throw and crash
startup; wrap the check-and-create logic that uses fs.existsSync and
fs.mkdirSync for LOG_DIR in a try/catch block so errors are handled gracefully
(log the error via your logger and continue or exit cleanly as appropriate),
ensuring the catch covers both existence check and recursive mkdir so
permission/disk errors don't bubble up uncaught.
In `@rateLimiter.js`:
- Around line 49-56: The chain kept in _globalTail can be stalled by a
never-resolving fn(), so add a timeout guard inside the async callback passed to
_globalTail: define a timeout constant (e.g., GLOBAL_CALL_TIMEOUT_MS) and race
the fn() promise against a timeout promise that rejects after that duration,
then use the raced result to call resolve(...) or reject(...). Update the async
block that currently does "resolve(await fn()) / catch(err) reject(err)" to
instead await Promise.race([fn(), timeoutReject]) (or equivalent) so a hung fn()
will reject and allow the _globalTail chain to continue; keep existing wait
logic that uses _globalLast and GLOBAL_GAP_MS and ensure _globalLast is still
updated before running the guarded call.
In `@scripts/runewager_redeploy.sh`:
- Around line 125-139: The loop that stops legacy services (loop variable
_legacy) currently swallows failures with "systemctl stop ... || true" and never
verifies the unit stopped; change the non-dry-run branch in the for loop to call
"systemctl stop \"$ _legacy\"" without "|| true", then immediately verify with
"systemctl is-active --quiet \"$ _legacy\"" (optionally retry/wait a few
seconds) and if it remains active log an error via warn including the unit name
and BOT_PORT and either exit non-zero or surface the failure so the redeploy
fails-safe; keep the dry-run behavior unchanged and reuse the existing warn
function for messages.
In `@scripts/rw_cpu_guard.sh`:
- Line 681: The script currently writes the CSV header directly to "$trace"
which can leave consumers reading a partially-initialized file; change to atomic
initialization by creating a temp file (e.g., via mktemp), write the header
"timestamp,pid,ppid,cpu%,mem%,args" into that temp file, ensure the write is
flushed/validated (optional fsync or close), then move (mv) the temp file to
"$trace" to replace/create it atomically; reference the "$trace" variable and
the current echo that writes the header when making this change.
In `@telegramSafe.js`:
- Around line 186-242: These exported wrappers are double-wrapping
already-patched methods (after init) causing double-throttling/retries; remove
the outer _withRetry and enqueue calls in sendMessage, editMessageText,
answerCallbackQuery, sendPhoto, and sendDocument so they simply _assertInited()
and directly return the underlying _bot.telegram.sendMessage / editMessageText /
answerCbQuery / sendPhoto / sendDocument call (for answerCallbackQuery, keep
only globalThrottle if the patched method does not handle the 10s callback
deadline), ensuring you reference the existing functions by name (sendMessage,
editMessageText, answerCallbackQuery, sendPhoto, sendDocument) and the patched
targets (_bot.telegram.sendMessage, _bot.telegram.editMessageText,
_bot.telegram.answerCbQuery, _bot.telegram.sendPhoto,
_bot.telegram.sendDocument) when making the change.
- Around line 105-110: The code currently sets _inited = true before init(bot)
completes, which can leave the module marked initialized if init throws; change
the sequence in the init(bot) flow so you validate/patch the provided bot and
assign _bot (and any derived tg) and only then set _inited = true; wrap the
validation/patch logic (the parts that use bot, _bot, tg and any patching
functions) in a try/catch so that on error you leave _inited false and rethrow
the error (or return failure), ensuring _inited is flipped only after successful
initialization.
- Around line 133-136: The wrapper for tg.answerCallbackQuery narrows its
signature to (callbackQueryId, extra) and can drop the optional text parameter;
change the reassignment so it forwards all arguments variadically (e.g., use
...args) to the bound _answerCallbackQuery and through
_withRetry/globalThrottle, preserving the original call shape and ensuring
tg.answerCbQuery behavior is unchanged; update the wrapper around
tg.answerCallbackQuery (which creates _answerCallbackQuery) to accept and pass
through all args.
---
Nitpick comments:
In `@runewager-endpoint.service`:
- Around line 38-40: The service currently uses append-only files
(StandardOutput=append:/var/www/html/Runewager/logs/backend.log and
StandardError=append:/var/www/html/Runewager/logs/backend-error.log) which can
grow indefinitely; either switch the unit to use the journal (e.g.,
StandardOutput=journal and StandardError=journal) so systemd/journald handles
rotation, or keep the files but add a guaranteed logrotate policy for
/var/www/html/Runewager/logs/backend.log and backend-error.log and ensure the
unit’s ReadWritePaths remains correct; update the unit’s
StandardOutput/StandardError lines or create a logrotate config that rotates,
compresses, and sets max age/size for those filenames and reload
systemd/logrotate accordingly.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.env.examplebackend.jsindex.jspackage.jsonrateLimiter.jsrunewager-endpoint.servicescripts/runewager_redeploy.shscripts/rw_cpu_guard.shtelegramSafe.js
| if (AUTOFIX_SECRET && !verifySignature(rawBody, sig)) { | ||
| logger.error('autofix.webhook', 'Signature mismatch — request rejected', { requestId }); | ||
| return res.status(401).json({ ok: false, error: 'Invalid signature', requestId }); | ||
| } |
There was a problem hiding this comment.
Webhook verification is currently fail-open when secret is unset
If AUTOFIX_SECRET is missing, unsigned requests are accepted. That creates an easy misconfiguration path to unauthenticated webhook traffic.
Suggested fix
- if (AUTOFIX_SECRET && !verifySignature(rawBody, sig)) {
+ if (!AUTOFIX_SECRET) {
+ logger.error('autofix.webhook', 'AUTOFIX_SECRET is not configured', { requestId });
+ return res.status(503).json({ ok: false, error: 'Webhook secret not configured', requestId });
+ }
+ if (!verifySignature(rawBody, sig)) {
logger.error('autofix.webhook', 'Signature mismatch — request rejected', { requestId });
return res.status(401).json({ ok: false, error: 'Invalid signature', requestId });
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (AUTOFIX_SECRET && !verifySignature(rawBody, sig)) { | |
| logger.error('autofix.webhook', 'Signature mismatch — request rejected', { requestId }); | |
| return res.status(401).json({ ok: false, error: 'Invalid signature', requestId }); | |
| } | |
| if (!AUTOFIX_SECRET) { | |
| logger.error('autofix.webhook', 'AUTOFIX_SECRET is not configured', { requestId }); | |
| return res.status(503).json({ ok: false, error: 'Webhook secret not configured', requestId }); | |
| } | |
| if (!verifySignature(rawBody, sig)) { | |
| logger.error('autofix.webhook', 'Signature mismatch — request rejected', { requestId }); | |
| return res.status(401).json({ ok: false, error: 'Invalid signature', requestId }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@backend.js` around lines 186 - 189, Currently the webhook verification is
fail-open when AUTOFIX_SECRET is unset; change the logic in the same handler
that references AUTOFIX_SECRET and verifySignature so requests are rejected
unless a secret is configured and signature verification succeeds: if
AUTOFIX_SECRET is falsy, log an error via logger (e.g., logger.error with
context like 'autofix.webhook' and requestId) and return res.status(401).json({
ok: false, error: 'Webhook secret not configured', requestId }) instead of
accepting the request, and keep the existing verifySignature(rawBody, sig) check
when the secret is present to return 401 on signature mismatch.
|
|
||
| // ─── Config ────────────────────────────────────────────────────────────────── | ||
|
|
||
| const PORT = parseInt(process.env.ENDPOINT_PORT ?? '3001', 10); | ||
| const AUTOFIX_SECRET = process.env.AUTOFIX_SECRET ?? ''; | ||
| const TG_TOKEN = process.env.TELEGRAM_BOT_TOKEN ?? process.env.BOT_TOKEN ?? ''; | ||
| const ADMIN_CHAT_IDS = (process.env.ADMIN_CHAT_IDS ?? '') | ||
| .split(',').map(s => s.trim()).filter(Boolean); | ||
|
|
||
| // ─── Structured Logging ────────────────────────────────────────────────────── | ||
|
|
||
| const LOG_DIR = path.join(__dirname, 'logs'); | ||
| const LOG_FILE = path.join(LOG_DIR, 'backend.log'); | ||
| const ERR_FILE = path.join(LOG_DIR, 'backend-error.log'); | ||
|
|
||
| if (!fs.existsSync(LOG_DIR)) fs.mkdirSync(LOG_DIR, { recursive: true }); | ||
|
|
||
| const _logStream = fs.createWriteStream(LOG_FILE, { flags: 'a' }); | ||
| const _errStream = fs.createWriteStream(ERR_FILE, { flags: 'a' }); | ||
|
|
||
| const logger = { | ||
| _entry(level, eventType, msg, extra) { | ||
| return { ts: new Date().toISOString(), level, eventType, msg, ...extra }; | ||
| }, | ||
| _write(streams, obj) { | ||
| const line = JSON.stringify(obj) + '\n'; | ||
| for (const s of streams) s.write(line); | ||
| }, | ||
| info(eventType, msg, extra = {}) { | ||
| this._write([_logStream], this._entry('info', eventType, msg, extra)); | ||
| }, | ||
| error(eventType, msg, extra = {}) { | ||
| this._write([_logStream, _errStream], this._entry('error', eventType, msg, extra)); | ||
| }, | ||
| event(eventType, msg, extra = {}) { | ||
| this._write([_logStream], this._entry('event', eventType, msg, extra)); | ||
| }, | ||
| }; | ||
|
|
||
| // ─── Telegram Admin Broadcast ───────────────────────────────────────────────── | ||
|
|
||
| /** | ||
| * Send a plain-text message to every chat ID listed in ADMIN_CHAT_IDS. | ||
| * Failures per-chat are swallowed so one bad ID never blocks the others. | ||
| * @param {string} msg | ||
| * @returns {Promise<void>} | ||
| */ | ||
| function sendAdmin(msg) { | ||
| if (!TG_TOKEN || ADMIN_CHAT_IDS.length === 0) return Promise.resolve(); | ||
|
|
||
| return Promise.all(ADMIN_CHAT_IDS.map(chatId => new Promise(resolve => { | ||
| const body = JSON.stringify({ chat_id: chatId, text: String(msg) }); | ||
| const req = https.request({ | ||
| hostname: 'api.telegram.org', | ||
| path: `/bot${TG_TOKEN}/sendMessage`, | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| 'Content-Length': Buffer.byteLength(body), | ||
| }, | ||
| }, resolve); | ||
| req.on('error', () => resolve()); | ||
| req.write(body); | ||
| req.end(); |
There was a problem hiding this comment.
Suggestion: Admin notifications sent from this new HTTP service call the Telegram Bot API directly via https.request and bypass the shared global rate limiter, so a burst of Autofix webhooks can easily exceed Telegram's request limits and cause 429 errors that drop admin alerts; routing these calls through the existing rateLimiter's globalThrottle ensures they respect the configured global gap and integrate with the central rate-limiting strategy. [possible bug]
Severity Level: Major ⚠️
- ⚠️ Admin Autofix alerts may be rate-limited and dropped.
- ⚠️ Endpoint bypasses shared Telegram global rate limiter design.
- ⚠️ Crash/error notifications from backend may not reach admins.| // ─── Config ────────────────────────────────────────────────────────────────── | |
| const PORT = parseInt(process.env.ENDPOINT_PORT ?? '3001', 10); | |
| const AUTOFIX_SECRET = process.env.AUTOFIX_SECRET ?? ''; | |
| const TG_TOKEN = process.env.TELEGRAM_BOT_TOKEN ?? process.env.BOT_TOKEN ?? ''; | |
| const ADMIN_CHAT_IDS = (process.env.ADMIN_CHAT_IDS ?? '') | |
| .split(',').map(s => s.trim()).filter(Boolean); | |
| // ─── Structured Logging ────────────────────────────────────────────────────── | |
| const LOG_DIR = path.join(__dirname, 'logs'); | |
| const LOG_FILE = path.join(LOG_DIR, 'backend.log'); | |
| const ERR_FILE = path.join(LOG_DIR, 'backend-error.log'); | |
| if (!fs.existsSync(LOG_DIR)) fs.mkdirSync(LOG_DIR, { recursive: true }); | |
| const _logStream = fs.createWriteStream(LOG_FILE, { flags: 'a' }); | |
| const _errStream = fs.createWriteStream(ERR_FILE, { flags: 'a' }); | |
| const logger = { | |
| _entry(level, eventType, msg, extra) { | |
| return { ts: new Date().toISOString(), level, eventType, msg, ...extra }; | |
| }, | |
| _write(streams, obj) { | |
| const line = JSON.stringify(obj) + '\n'; | |
| for (const s of streams) s.write(line); | |
| }, | |
| info(eventType, msg, extra = {}) { | |
| this._write([_logStream], this._entry('info', eventType, msg, extra)); | |
| }, | |
| error(eventType, msg, extra = {}) { | |
| this._write([_logStream, _errStream], this._entry('error', eventType, msg, extra)); | |
| }, | |
| event(eventType, msg, extra = {}) { | |
| this._write([_logStream], this._entry('event', eventType, msg, extra)); | |
| }, | |
| }; | |
| // ─── Telegram Admin Broadcast ───────────────────────────────────────────────── | |
| /** | |
| * Send a plain-text message to every chat ID listed in ADMIN_CHAT_IDS. | |
| * Failures per-chat are swallowed so one bad ID never blocks the others. | |
| * @param {string} msg | |
| * @returns {Promise<void>} | |
| */ | |
| function sendAdmin(msg) { | |
| if (!TG_TOKEN || ADMIN_CHAT_IDS.length === 0) return Promise.resolve(); | |
| return Promise.all(ADMIN_CHAT_IDS.map(chatId => new Promise(resolve => { | |
| const body = JSON.stringify({ chat_id: chatId, text: String(msg) }); | |
| const req = https.request({ | |
| hostname: 'api.telegram.org', | |
| path: `/bot${TG_TOKEN}/sendMessage`, | |
| method: 'POST', | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| 'Content-Length': Buffer.byteLength(body), | |
| }, | |
| }, resolve); | |
| req.on('error', () => resolve()); | |
| req.write(body); | |
| req.end(); | |
| const { globalThrottle } = require('./rateLimiter'); | |
| // ─── Config ────────────────────────────────────────────────────────────────── | |
| const PORT = parseInt(process.env.ENDPOINT_PORT ?? '3001', 10); | |
| const AUTOFIX_SECRET = process.env.AUTOFIX_SECRET ?? ''; | |
| const TG_TOKEN = process.env.TELEGRAM_BOT_TOKEN ?? process.env.BOT_TOKEN ?? ''; | |
| const ADMIN_CHAT_IDS = (process.env.ADMIN_CHAT_IDS ?? '') | |
| .split(',').map(s => s.trim()).filter(Boolean); | |
| // ─── Structured Logging ────────────────────────────────────────────────────── | |
| const LOG_DIR = path.join(__dirname, 'logs'); | |
| const LOG_FILE = path.join(LOG_DIR, 'backend.log'); | |
| const ERR_FILE = path.join(LOG_DIR, 'backend-error.log'); | |
| if (!fs.existsSync(LOG_DIR)) fs.mkdirSync(LOG_DIR, { recursive: true }); | |
| const _logStream = fs.createWriteStream(LOG_FILE, { flags: 'a' }); | |
| const _errStream = fs.createWriteStream(ERR_FILE, { flags: 'a' }); | |
| const logger = { | |
| _entry(level, eventType, msg, extra) { | |
| return { ts: new Date().toISOString(), level, eventType, msg, ...extra }; | |
| }, | |
| _write(streams, obj) { | |
| const line = JSON.stringify(obj) + '\n'; | |
| for (const s of streams) s.write(line); | |
| }, | |
| info(eventType, msg, extra = {}) { | |
| this._write([_logStream], this._entry('info', eventType, msg, extra)); | |
| }, | |
| error(eventType, msg, extra = {}) { | |
| this._write([_logStream, _errStream], this._entry('error', eventType, msg, extra)); | |
| }, | |
| event(eventType, msg, extra = {}) { | |
| this._write([_logStream], this._entry('event', eventType, msg, extra)); | |
| }, | |
| }; | |
| // ─── Telegram Admin Broadcast ───────────────────────────────────────────────── | |
| /** | |
| * Send a plain-text message to every chat ID listed in ADMIN_CHAT_IDS. | |
| * Failures per-chat are swallowed so one bad ID never blocks the others. | |
| * @param {string} msg | |
| * @returns {Promise<void>} | |
| */ | |
| function sendAdmin(msg) { | |
| if (!TG_TOKEN || ADMIN_CHAT_IDS.length === 0) return Promise.resolve(); | |
| const sendOne = (chatId) => | |
| globalThrottle(() => new Promise((resolve) => { | |
| const body = JSON.stringify({ chat_id: chatId, text: String(msg) }); | |
| const req = https.request({ | |
| hostname: 'api.telegram.org', | |
| path: `/bot${TG_TOKEN}/sendMessage`, | |
| method: 'POST', | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| 'Content-Length': Buffer.byteLength(body), | |
| }, | |
| }, () => resolve()); | |
| req.on('error', () => resolve()); | |
| req.write(body); | |
| req.end(); | |
| })); | |
| return Promise.all(ADMIN_CHAT_IDS.map(sendOne)).then(() => {}); |
Steps of Reproduction ✅
1. Start the new HTTP endpoint service by running `npm run endpoint` (script defined in
`package.json:12`, which executes `node backend.js` and loads `backend.js` as the main
module).
2. Ensure admin notifications are enabled by setting `TELEGRAM_BOT_TOKEN` (or `BOT_TOKEN`)
and `ADMIN_CHAT_IDS` in the environment (see `.env.example:2-3` referencing `backend.js
sendAdmin()` and `ENDPOINT_PORT`), so `TG_TOKEN` and `ADMIN_CHAT_IDS` in
`backend.js:28-32` are non-empty and `sendAdmin()` will send Telegram messages.
3. Trigger multiple Autofix webhooks in quick succession against the endpoint's webhook
handler at `backend.js:180-206` by issuing many `POST /autofix/webhook` HTTP requests
(each successful signature check leads to `await sendAdmin(\`🤖 Autofix [${requestId}]:
${eventName}\`)` at `backend.js:197`).
4. Observe that each webhook causes `sendAdmin()` at `backend.js:72-89` to fan out one
`https.request` call per admin chat in parallel, without any reference to
`rateLimiter.globalThrottle` (`rateLimiter.js:18-19,117-119`), meaning these Telegram API
calls bypass the global 35ms gap described in `rateLimiter.js:18-19`; combined with normal
bot traffic routed through `telegramSafe.js` (`telegramSafe.js:10-19`) this unthrottled
stream can exceed Telegram's documented ~30 requests/second per bot, leading to 429
responses and dropped admin alerts under realistic bursty Autofix usage.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** backend.js
**Line:** 25:88
**Comment:**
*Possible Bug: Admin notifications sent from this new HTTP service call the Telegram Bot API directly via `https.request` and bypass the shared global rate limiter, so a burst of Autofix webhooks can easily exceed Telegram's request limits and cause 429 errors that drop admin alerts; routing these calls through the existing `rateLimiter`'s `globalThrottle` ensures they respect the configured global gap and integrate with the central rate-limiting strategy.
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.| # Stop legacy service names so they can't restart and steal port ${BOT_PORT} | ||
| # while this redeploy is in progress. Failures are non-fatal. | ||
| # Respects --dry-run: only warns, does not stop, when DRY_RUN=1. | ||
| local _legacy | ||
| for _legacy in "runewager-bot" "runewager_bot" "runewager-endpoint"; do | ||
| if [[ "$_legacy" != "$BOT_SERVICE" ]] \ | ||
| && systemctl is-active --quiet "$_legacy" 2>/dev/null; then | ||
| if (( DRY_RUN )); then | ||
| warn "[DRY-RUN] would stop legacy service '${_legacy}'" | ||
| else | ||
| warn "Legacy service '${_legacy}' is running — stopping to prevent port conflict" | ||
| systemctl stop "$_legacy" 2>/dev/null || true | ||
| fi | ||
| fi | ||
| done |
There was a problem hiding this comment.
Suggestion: The redeploy script now stops the runewager-endpoint systemd service as if it were a legacy bot unit, but never starts it again, so every bot redeploy will silently take down the new HTTP companion service (backend.js on port 3001) and leave it offline until manually restarted. [logic error]
Severity Level: Critical 🚨
- ❌ Companion HTTP endpoint backend.js stopped on every redeploy.
- ❌ Autofix webhook `/autofix/webhook` in `backend.js` unreachable.
- ❌ Health endpoints `/health` and `/health/full` return connection errors.
- ⚠️ Admin Telegram broadcasts from backend.js silently stop working.| # Stop legacy service names so they can't restart and steal port ${BOT_PORT} | |
| # while this redeploy is in progress. Failures are non-fatal. | |
| # Respects --dry-run: only warns, does not stop, when DRY_RUN=1. | |
| local _legacy | |
| for _legacy in "runewager-bot" "runewager_bot" "runewager-endpoint"; do | |
| if [[ "$_legacy" != "$BOT_SERVICE" ]] \ | |
| && systemctl is-active --quiet "$_legacy" 2>/dev/null; then | |
| if (( DRY_RUN )); then | |
| warn "[DRY-RUN] would stop legacy service '${_legacy}'" | |
| else | |
| warn "Legacy service '${_legacy}' is running — stopping to prevent port conflict" | |
| systemctl stop "$_legacy" 2>/dev/null || true | |
| fi | |
| fi | |
| done | |
| # Stop legacy service names so they can't restart and steal port ${BOT_PORT} | |
| # while this redeploy is in progress. Failures are non-fatal. | |
| # Respects --dry-run: only warns, does not stop, when DRY_RUN=1. | |
| local _legacy | |
| for _legacy in "runewager-bot" "runewager_bot"; do | |
| if [[ "$_legacy" != "$BOT_SERVICE" ]] \ | |
| && systemctl is-active --quiet "$_legacy" 2>/dev/null; then | |
| if (( DRY_RUN )); then | |
| warn "[DRY-RUN] would stop legacy service '${_legacy}'" | |
| else | |
| warn "Legacy service '${_legacy}' is running — stopping to prevent port conflict" | |
| systemctl stop "$_legacy" 2>/dev/null || true | |
| fi | |
| fi | |
| done |
Steps of Reproduction ✅
1. Install and enable the companion service described in `runewager-endpoint.service`
(`/workspace/Runewager/runewager-endpoint.service:1-13`), so that both `runewager.service`
(main bot) and `runewager-endpoint.service` (backend.js on port 3001) are active under
systemd.
2. On that host, run `sudo ./scripts/runewager_redeploy.sh` without `--dry-run`; `main()`
at `scripts/runewager_redeploy.sh:396-432` (approx.) invokes `_stop_bot_service` at
`scripts/runewager_redeploy.sh:422`.
3. Inside `_stop_bot_service` (`scripts/runewager_redeploy.sh:117-139`), the loop at line
129 iterates over `"runewager-bot" "runewager_bot" "runewager-endpoint"`; for
`_legacy="runewager-endpoint"` and `BOT_SERVICE="runewager"`, the condition passes and
`systemctl stop "runewager-endpoint"` is executed (lines 129-137), stopping the HTTP
companion even though it listens on a different port (3001 per
`runewager-endpoint.service:5`).
4. The subsequent `_start_bot_service()` function
(`scripts/runewager_redeploy.sh:173-205`) starts only `${BOT_SERVICE}` (`runewager`) and
never calls `systemctl start runewager-endpoint`; as a result, after the redeploy script
exits, `runewager-endpoint` remains inactive, leaving `backend.js` and its `/health` and
`/autofix/webhook` endpoints down until a manual `systemctl start runewager-endpoint` is
run.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** scripts/runewager_redeploy.sh
**Line:** 125:139
**Comment:**
*Logic Error: The redeploy script now stops the `runewager-endpoint` systemd service as if it were a legacy bot unit, but never starts it again, so every bot redeploy will silently take down the new HTTP companion service (backend.js on port 3001) and leave it offline until manually restarted.
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.| * @param {object} [options] | ||
| * @returns {Promise<boolean>} | ||
| */ | ||
| function answerCallbackQuery(callbackId, options = {}) { | ||
| _assertInited(); | ||
| return _withRetry(() => | ||
| globalThrottle(() => _bot.telegram.answerCbQuery(callbackId, options)) |
There was a problem hiding this comment.
Suggestion: The exported answerCallbackQuery helper passes the entire options object as the second argument to answerCbQuery, which Telegraf interprets as the text parameter, so callers providing an options object (e.g. { text, showAlert }) will end up sending '[object Object]' or the wrong fields to Telegram instead of the intended message and flags. [logic error]
Severity Level: Major ⚠️
- ⚠️ Exported helper sends malformed callback texts to Telegram.
- ⚠️ Future modules using helper mis-acknowledge inline button presses.
- ⚠️ Currently unused helper; bug latent in shared API surface.| * @param {object} [options] | |
| * @returns {Promise<boolean>} | |
| */ | |
| function answerCallbackQuery(callbackId, options = {}) { | |
| _assertInited(); | |
| return _withRetry(() => | |
| globalThrottle(() => _bot.telegram.answerCbQuery(callbackId, options)) | |
| * @param {{ text?: string, showAlert?: boolean, extra?: object }} [options] | |
| * @returns {Promise<boolean>} | |
| */ | |
| function answerCallbackQuery(callbackId, options = {}) { | |
| _assertInited(); | |
| const { text, showAlert, extra } = options || {}; | |
| return _withRetry(() => | |
| globalThrottle(() => _bot.telegram.answerCbQuery(callbackId, text, showAlert, extra)) |
Steps of Reproduction ✅
1. Start the main bot process defined in `/workspace/Runewager/index.js`, which creates
the Telegraf instance and calls `telegramSafe.init(bot)` at `index.js:228-235`, ensuring
`_bot` is set inside `telegramSafe.js`.
2. In the same Node.js process (or from a new module), require the safe wrapper module as
documented in `telegramSafe.js:5-7` with `const tg = require('./telegramSafe');`.
3. Invoke the exported helper in `telegramSafe.js:208-220` as intended for options usage,
for example:
`tg.answerCallbackQuery('some-callback-id', { text: 'Done', showAlert: true });`
4. The call enters `function answerCallbackQuery(callbackId, options = {})` at
`telegramSafe.js:215`, which currently calls `_bot.telegram.answerCbQuery(callbackId,
options)` at `telegramSafe.js:218`. Because Telegraf's `answerCbQuery` interprets its
second argument as the `text` string (per the comment at `telegramSafe.js:125` and
Telegraf's API), the entire `{ text: 'Done', showAlert: true }` object is coerced to the
string `"[object Object]"`, and no proper `showAlert` flag or structured extra options are
passed. The resulting callback response sent to Telegram therefore has malformed text and
ignores the intended options.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** telegramSafe.js
**Line:** 212:218
**Comment:**
*Logic Error: The exported `answerCallbackQuery` helper passes the entire `options` object as the second argument to `answerCbQuery`, which Telegraf interprets as the text parameter, so callers providing an options object (e.g. `{ text, showAlert }`) will end up sending `'[object Object]'` or the wrong fields to Telegram instead of the intended message and flags.
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. |
backend.js — new companion Express server (7 required features):
1. POST /autofix/webhook — HMAC-SHA256 signature verification, try/catch
error boundary; sendAdmin() called on both success and error
2. GET /autofix/test — generates + immediately verifies a signed test
payload; returns {ok, signatureValid, timestamp, requestId}
3. sendAdmin(msg) — broadcasts plain-text to all ADMIN_CHAT_IDS via
Telegram Bot API using Node built-in https; per-chat failures swallowed
4. Structured logger — logger.info/error/event write NDJSON lines with
{ts, level, eventType, msg, ...extra} to logs/backend.log +
logs/backend-error.log; errors also go to the error log
5. requestId middleware — crypto.randomBytes(5).toString('hex') attached to
req.id on every request; included in all log entries and JSON responses
6. GET /health/full — uptimeSec, nodeVersion, memoryMB, cpuLoad,
autofixWebhookRegistered, adminChatIdsConfigured, service name
7. Graceful shutdown — SIGTERM/SIGINT close HTTP server cleanly; 10 s
force-exit watchdog; uncaughtException sends admin alert before exit
runewager-endpoint.service — renamed from runewager-bot.service:
- Description: Runewager Endpoint — Autofix webhook and admin bridge
- ExecStart: node backend.js; logs to logs/backend{,-error}.log
- After=runewager.service so main bot starts first
package.json:
- Add express ^4.21.2 dependency
- Add "endpoint" and "endpoint:check" npm scripts
.env.example:
- Document ADMIN_CHAT_IDS, AUTOFIX_SECRET, ENDPOINT_PORT
scripts/runewager_redeploy.sh:
- Hoist _show_payload() above _health_check() so it is defined once and
reusable by any step (not redefined on every loop iteration)
- Use bash array for formatter args: _show_payload "$text" python3 -m json.tool
eliminates word-splitting bugs for multi-word formatters
- Legacy service stop block now respects --dry-run: prints warn instead of
calling systemctl stop when DRY_RUN=1
- Add runewager-endpoint to legacy service list so it is also stopped during
a redeploy of the main runewager service
https://claude.ai/code/session_01WpEqAiDbpqnBywMjYXJHf4
…29 errors
rateLimiter.js (new):
- Per-chat serialized queue — 1100ms spacing between messages to same chatId
- Global rate chain — GLOBAL_GAP_MS=35ms between ANY two API calls (~28/sec)
well under Telegram's hard 30/sec global limit
- enqueue(chatId, fn) — per-chat + global throttle; caller gets result
immediately when fn() settles; next call in same chat waits CHAT_GAP_MS
- globalThrottle(fn) — global-only throttle for time-critical calls
- stats() — exposes pending queue count for /health/full observability
- Never breaks chain: .catch(()=>{}) guards on both global and per-chat tails
telegramSafe.js (new):
- init(bot) patches bot.telegram in place — one call covers ALL paths:
ctx.reply() → bot.telegram.sendMessage ✓
ctx.replyWithPhoto() → bot.telegram.sendPhoto ✓
ctx.editMessageText() → bot.telegram.editMessageText ✓
ctx.answerCbQuery() → bot.telegram.answerCbQuery ✓
bot.telegram.*() → directly patched ✓
ctx.telegram.*() → same object ref, patched ✓
- answerCbQuery uses globalThrottle only (NOT per-chat queue) to respect
the hard 10-second callback-query deadline
- _withRetry(fn, maxRetries=3): 429 → respects retry_after + 250ms margin;
transient network errors (ETIMEDOUT/ECONNRESET) → exponential backoff
- Explicit exports (sendMessage, editMessageText, answerCallbackQuery,
sendPhoto, sendDocument) for non-context callers (e.g. backend.js)
- init() is idempotent — safe to call multiple times
index.js:
- require('./telegramSafe') at top-of-file
- telegramSafe.init(bot) called immediately after new Telegraf(BOT_TOKEN),
BEFORE any bot.use() middleware — guarantees patch is always active
Zero changes to the 558 ctx.reply(), 287 ctx.answerCbQuery(), or 44 direct
bot.telegram.* call sites — the patch is transparent to all existing code.
Bot behavior is unchanged; only throttling and 429 resilience are added.
https://claude.ai/code/session_01WpEqAiDbpqnBywMjYXJHf4
8c6e068 to
45921e6
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (8)
backend.js (3)
72-89:⚠️ Potential issue | 🔴 CriticalAdd request timeout in
sendAdminto avoid hangs.Without a timeout, a stuck socket can block webhook handling or crash-path notification indefinitely.
Suggested patch
- const req = https.request({ + const req = https.request({ hostname: 'api.telegram.org', path: `/bot${TG_TOKEN}/sendMessage`, method: 'POST', headers: { 'Content-Type': 'application/json', 'Content-Length': Buffer.byteLength(body), }, - }, resolve); + }, (resp) => { + resp.resume(); + resolve(); + }); + req.setTimeout(5000, () => req.destroy(new Error('Telegram request timeout'))); req.on('error', () => resolve()); req.write(body); req.end();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend.js` around lines 72 - 89, The sendAdmin helper can hang because HTTPS requests have no timeout; modify the sendAdmin function so each outgoing request (the req created in ADMIN_CHAT_IDS.map) has a timeout set (e.g., req.setTimeout or a manual timer) and that the timeout handler aborts the request and resolves the per-chat Promise to avoid blocking Promise.all; ensure you also attach the timeout handler alongside the existing req.on('error') handler so sockets that never respond are cleaned up and the overall sendAdmin Promise always settles.
186-189:⚠️ Potential issue | 🟠 MajorFail closed when
AUTOFIX_SECRETis missing.Current check accepts unsigned webhook traffic when the secret is unset, which weakens authentication guarantees.
Suggested patch
- if (AUTOFIX_SECRET && !verifySignature(rawBody, sig)) { + if (!AUTOFIX_SECRET) { + logger.error('autofix.webhook', 'AUTOFIX_SECRET is not configured', { requestId }); + return res.status(401).json({ ok: false, error: 'Webhook secret not configured', requestId }); + } + if (!verifySignature(rawBody, sig)) { logger.error('autofix.webhook', 'Signature mismatch — request rejected', { requestId }); return res.status(401).json({ ok: false, error: 'Invalid signature', requestId }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend.js` around lines 186 - 189, The current webhook auth accepts requests when AUTOFIX_SECRET is unset; update the check so missing or invalid secrets both reject the request: in backend.js adjust the logic around AUTOFIX_SECRET and verifySignature(rawBody, sig) (the block that currently logs via logger.error('autofix.webhook', ...) and returns res.status(401).json(...)) to first verify AUTOFIX_SECRET is present and, if absent, log an error and return 401, and otherwise call verifySignature and reject on failure—ensure you reference AUTOFIX_SECRET, verifySignature(rawBody, sig), logger.error, and the res.status(401) response so unsigned traffic is never accepted when the secret is unset.
40-40:⚠️ Potential issue | 🟠 MajorWrap log directory initialization in
try/catch.Line 40 can throw on permission/disk failures and abort startup before service readiness is reported.
As per coding guidelines: `**/*.js` — “Wrap directory operations (readdirSync, etc.) in try/catch blocks for robust error handling.”Suggested patch
-if (!fs.existsSync(LOG_DIR)) fs.mkdirSync(LOG_DIR, { recursive: true }); +try { + if (!fs.existsSync(LOG_DIR)) fs.mkdirSync(LOG_DIR, { recursive: true }); +} catch (err) { + console.error(`[backend] Failed to initialize log directory: ${err.message}`); + process.exit(1); +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend.js` at line 40, Wrap the LOG_DIR initialization (the fs.existsSync(LOG_DIR) / fs.mkdirSync(LOG_DIR, { recursive: true }) calls) in a try/catch so filesystem permission or disk errors do not abort startup; in the catch block log the error with context (include LOG_DIR and the caught error) using the existing logger (e.g., processLogger.error) or console.error if no logger is available, and fail gracefully (do not rethrow) so the service can continue starting or handle the missing log dir at runtime.telegramSafe.js (3)
133-136:⚠️ Potential issue | 🟡 MinorForward all optional args in
answerCallbackQueryalias wrapper.Line 135 narrows args to
(callbackQueryId, extra), which can drop/shift optional parameters.Suggested patch
- tg.answerCallbackQuery = (callbackQueryId, extra) => - _withRetry(() => globalThrottle(() => _answerCallbackQuery(callbackQueryId, extra))); + tg.answerCallbackQuery = (callbackQueryId, ...args) => + _withRetry(() => globalThrottle(() => _answerCallbackQuery(callbackQueryId, ...args)));For Telegraf v4 Telegram class methods, what is the exact parameter signature for answerCbQuery/answerCallbackQuery (including optional text and extra args)?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@telegramSafe.js` around lines 133 - 136, The wrapper for tg.answerCallbackQuery currently hard-codes two parameters and can drop optional args; change the alias assignment so tg.answerCallbackQuery = (...args) => _withRetry(() => globalThrottle(() => _answerCallbackQuery(...args))); locate the assignment where tg.answerCallbackQuery is rebound (the block comparing tg.answerCallbackQuery !== tg.answerCbQuery that creates _answerCallbackQuery and uses _withRetry/globalThrottle) and replace the fixed-arg wrapper with a rest-args forwarder to preserve optional text and extra parameters.
105-110:⚠️ Potential issue | 🟠 MajorSet
_initedonly after successful initialization.Line 106 flips
_initedbefore bot validation/patching; if init throws, the module can stay permanently “initialized” but unusable.Suggested patch
function init(bot) { if (_inited) return; - _inited = true; - _bot = bot; - + if (!bot || !bot.telegram) { + throw new Error('[telegramSafe] init(bot) requires a valid Telegraf instance'); + } const tg = bot.telegram; + // ...apply patches... + _bot = bot; + _inited = true;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@telegramSafe.js` around lines 105 - 110, Do not set _inited to true until initialization completes successfully: move the assignment of _inited = true (and any state that marks module ready) to after bot validation/patching completes without error; ensure _bot is assigned only when validation succeeds and wrap the initialization steps (validation/patching of the passed-in bot/telegram) so any thrown error prevents flipping _inited. Reference symbols: _inited, _bot, bot, tg.
186-242:⚠️ Potential issue | 🟠 MajorAvoid double retry/throttle in exported wrappers.
These wrappers call
_bot.telegram.*methods that are already patched ininit, so wrapping again adds extra queueing/retry latency and can self-throttle unnecessarily.Suggested patch
function sendMessage(chatId, text, options = {}) { _assertInited(); - return _withRetry(() => enqueue(chatId, () => _bot.telegram.sendMessage(chatId, text, options))); + return _bot.telegram.sendMessage(chatId, text, options); } @@ function editMessageText(chatId, messageId, text, options = {}) { _assertInited(); - return _withRetry(() => - enqueue(chatId, () => - _bot.telegram.editMessageText(chatId, messageId, undefined, text, options) - ) - ); + return _bot.telegram.editMessageText(chatId, messageId, undefined, text, options); } @@ function answerCallbackQuery(callbackId, options = {}) { _assertInited(); - return _withRetry(() => - globalThrottle(() => _bot.telegram.answerCbQuery(callbackId, options)) - ); + return _bot.telegram.answerCbQuery(callbackId, options); } @@ function sendPhoto(chatId, photo, options = {}) { _assertInited(); - return _withRetry(() => enqueue(chatId, () => _bot.telegram.sendPhoto(chatId, photo, options))); + return _bot.telegram.sendPhoto(chatId, photo, options); } @@ function sendDocument(chatId, document, options = {}) { _assertInited(); - return _withRetry(() => enqueue(chatId, () => _bot.telegram.sendDocument(chatId, document, options))); + return _bot.telegram.sendDocument(chatId, document, options); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@telegramSafe.js` around lines 186 - 242, The exported wrapper functions (sendMessage, editMessageText, answerCallbackQuery, sendPhoto, sendDocument) are applying _withRetry/enqueue/globalThrottle on top of _bot.telegram methods that were already patched in init, causing double retry/throttle and extra latency; update each function to keep the _assertInited() call but remove the outer _withRetry/enqueue/globalThrottle wrappers and simply return the direct call to the patched method (e.g., return _bot.telegram.sendMessage(chatId, text, options) in sendMessage; return _bot.telegram.editMessageText(...) in editMessageText; return _bot.telegram.answerCbQuery(callbackId, options) in answerCallbackQuery; return _bot.telegram.sendPhoto(chatId, photo, options) in sendPhoto; and return _bot.telegram.sendDocument(chatId, document, options) in sendDocument), preserving signatures and options.scripts/runewager_redeploy.sh (1)
139-146:⚠️ Potential issue | 🟠 MajorVerify legacy service stop actually succeeded before continuing.
Line 145 still suppresses stop failures (
|| true) and does not check if the unit is still active, so conflicts can persist silently.Based on learnings: Applies to `**/*.sh` — “Start, stop, restart bot processes; verify bot is running and capture startup errors.”Suggested patch
- systemctl stop "$_legacy" 2>/dev/null || true + if ! systemctl stop "$_legacy" 2>/dev/null; then + warn "Failed to stop legacy service '${_legacy}'" + fi + if systemctl is-active --quiet "$_legacy" 2>/dev/null; then + fail "Legacy service '${_legacy}' is still active after stop" + return 1 + fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/runewager_redeploy.sh` around lines 139 - 146, The stop logic for legacy units suppresses failures and doesn't verify the unit stopped; update the block that handles $_legacy so that when not in DRY_RUN you remove the unconditional "|| true", call systemctl stop "$_legacy", then poll systemctl is-active "$_legacy" (or use systemctl --wait stop) with a short retry loop/timeout to confirm the unit becomes inactive; if it remains active log an error via warn and exit non-zero (or return failure) to prevent continuing with a port conflict. Ensure DRY_RUN still only logs the intended action and does not attempt the stop.rateLimiter.js (1)
49-57:⚠️ Potential issue | 🔴 CriticalAdd a timeout guard in
_globalSlotto prevent global queue deadlock.If
fn()never settles at Line 54,_globalTailstalls permanently and all outbound Telegram calls block.Suggested patch
const GLOBAL_GAP_MS = 35; // ≤28 req/sec globally (Telegram hard limit: 30/sec) const CHAT_GAP_MS = 1100; // 1 msg/sec per chat (Telegram hard limit: 1/sec) +const API_CALL_TIMEOUT_MS = 15000; @@ - try { resolve(await fn()); } + try { + resolve(await Promise.race([ + Promise.resolve().then(fn), + _delay(API_CALL_TIMEOUT_MS).then(() => { + throw new Error(`rateLimiter: API call timed out after ${API_CALL_TIMEOUT_MS}ms`); + }), + ])); + } catch (err) { reject(err); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@rateLimiter.js` around lines 49 - 57, The global chain can deadlock if fn() never settles; update the _globalTail handler (the .then(async () => { ... }) block that uses _globalLast, GLOBAL_GAP_MS, _delay and calls resolve(await fn())) to race fn() against a timeout: create a short constant (e.g. GLOBAL_CALL_TIMEOUT_MS), start a timeout Promise that rejects with a descriptive Error after that duration, and use Promise.race([fn(), timeoutPromise]) instead of await fn(); ensure you clear/ignore the timeout when fn() settles and forward rejection to reject(...) so the per-call resultP sees the error while the outer chain remains unblocked (the existing .catch(() => {}) on _globalTail already prevents chain-breaks).
🧹 Nitpick comments (1)
.env.example (1)
2-4: ReorderADMIN_CHAT_IDSaboveADMIN_IDSto satisfy dotenv-linter.This avoids the
UnorderedKeywarning and keeps.env.examplelint-clean.♻️ Suggested diff
-ADMIN_IDS=YOUR_TELEGRAM_USER_ID # ADMIN_CHAT_IDS: comma-separated Telegram chat IDs for backend.js sendAdmin() broadcasts. # Can be the same as ADMIN_IDS. Used by runewager-endpoint for Autofix alerts. ADMIN_CHAT_IDS=YOUR_TELEGRAM_USER_ID +ADMIN_IDS=YOUR_TELEGRAM_USER_ID🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.env.example around lines 2 - 4, Move the ADMIN_CHAT_IDS entry above the ADMIN_IDS entry in the .env.example so dotenv-linter's alphabetical ordering is satisfied; specifically reorder the two keys so ADMIN_CHAT_IDS appears before ADMIN_IDS (preserving their values/descriptions) to eliminate the UnorderedKey warning.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@index.js`:
- Around line 229-234: The init currently only wraps a few specific methods so
calls like ctx.telegram.callApi(...) and many other Telegram methods
(deleteMessage, getMe, getChatMember, getChat, unpinChatMessage, pinChatMessage,
setMyCommands, setChatMenuButton, etc.) bypass your rate limiter; update
telegramSafe.init to also wrap the Telegram.prototype.callApi method (the
underlying transport used by most API calls) or wrap all remaining Telegram
class methods so every direct bot.telegram.* and ctx.telegram.* call is routed
through the global rate limiter/queue, ensuring callApi, sendMessage,
editMessageText, answerCbQuery, answerCallbackQuery and the listed methods are
proxied through the rate-limiter; alternatively, if some methods must remain
unwrapped, add clear documentation in the init call explaining which methods are
intentionally excluded and why.
In `@scripts/runewager_redeploy.sh`:
- Around line 126-127: The for-loop that treats services as legacy conflicts
incorrectly includes "runewager-endpoint" (the loop variable _legacy in the line
with values "runewager-bot" "runewager_bot" "runewager-endpoint"), causing the
script to stop the endpoint even though the redeploy only restarts runewager;
remove "runewager-endpoint" from that list so only true legacy/conflicting
services ("runewager-bot" and "runewager_bot") are stopped, leaving the endpoint
running.
---
Duplicate comments:
In `@backend.js`:
- Around line 72-89: The sendAdmin helper can hang because HTTPS requests have
no timeout; modify the sendAdmin function so each outgoing request (the req
created in ADMIN_CHAT_IDS.map) has a timeout set (e.g., req.setTimeout or a
manual timer) and that the timeout handler aborts the request and resolves the
per-chat Promise to avoid blocking Promise.all; ensure you also attach the
timeout handler alongside the existing req.on('error') handler so sockets that
never respond are cleaned up and the overall sendAdmin Promise always settles.
- Around line 186-189: The current webhook auth accepts requests when
AUTOFIX_SECRET is unset; update the check so missing or invalid secrets both
reject the request: in backend.js adjust the logic around AUTOFIX_SECRET and
verifySignature(rawBody, sig) (the block that currently logs via
logger.error('autofix.webhook', ...) and returns res.status(401).json(...)) to
first verify AUTOFIX_SECRET is present and, if absent, log an error and return
401, and otherwise call verifySignature and reject on failure—ensure you
reference AUTOFIX_SECRET, verifySignature(rawBody, sig), logger.error, and the
res.status(401) response so unsigned traffic is never accepted when the secret
is unset.
- Line 40: Wrap the LOG_DIR initialization (the fs.existsSync(LOG_DIR) /
fs.mkdirSync(LOG_DIR, { recursive: true }) calls) in a try/catch so filesystem
permission or disk errors do not abort startup; in the catch block log the error
with context (include LOG_DIR and the caught error) using the existing logger
(e.g., processLogger.error) or console.error if no logger is available, and fail
gracefully (do not rethrow) so the service can continue starting or handle the
missing log dir at runtime.
In `@rateLimiter.js`:
- Around line 49-57: The global chain can deadlock if fn() never settles; update
the _globalTail handler (the .then(async () => { ... }) block that uses
_globalLast, GLOBAL_GAP_MS, _delay and calls resolve(await fn())) to race fn()
against a timeout: create a short constant (e.g. GLOBAL_CALL_TIMEOUT_MS), start
a timeout Promise that rejects with a descriptive Error after that duration, and
use Promise.race([fn(), timeoutPromise]) instead of await fn(); ensure you
clear/ignore the timeout when fn() settles and forward rejection to reject(...)
so the per-call resultP sees the error while the outer chain remains unblocked
(the existing .catch(() => {}) on _globalTail already prevents chain-breaks).
In `@scripts/runewager_redeploy.sh`:
- Around line 139-146: The stop logic for legacy units suppresses failures and
doesn't verify the unit stopped; update the block that handles $_legacy so that
when not in DRY_RUN you remove the unconditional "|| true", call systemctl stop
"$_legacy", then poll systemctl is-active "$_legacy" (or use systemctl --wait
stop) with a short retry loop/timeout to confirm the unit becomes inactive; if
it remains active log an error via warn and exit non-zero (or return failure) to
prevent continuing with a port conflict. Ensure DRY_RUN still only logs the
intended action and does not attempt the stop.
In `@telegramSafe.js`:
- Around line 133-136: The wrapper for tg.answerCallbackQuery currently
hard-codes two parameters and can drop optional args; change the alias
assignment so tg.answerCallbackQuery = (...args) => _withRetry(() =>
globalThrottle(() => _answerCallbackQuery(...args))); locate the assignment
where tg.answerCallbackQuery is rebound (the block comparing
tg.answerCallbackQuery !== tg.answerCbQuery that creates _answerCallbackQuery
and uses _withRetry/globalThrottle) and replace the fixed-arg wrapper with a
rest-args forwarder to preserve optional text and extra parameters.
- Around line 105-110: Do not set _inited to true until initialization completes
successfully: move the assignment of _inited = true (and any state that marks
module ready) to after bot validation/patching completes without error; ensure
_bot is assigned only when validation succeeds and wrap the initialization steps
(validation/patching of the passed-in bot/telegram) so any thrown error prevents
flipping _inited. Reference symbols: _inited, _bot, bot, tg.
- Around line 186-242: The exported wrapper functions (sendMessage,
editMessageText, answerCallbackQuery, sendPhoto, sendDocument) are applying
_withRetry/enqueue/globalThrottle on top of _bot.telegram methods that were
already patched in init, causing double retry/throttle and extra latency; update
each function to keep the _assertInited() call but remove the outer
_withRetry/enqueue/globalThrottle wrappers and simply return the direct call to
the patched method (e.g., return _bot.telegram.sendMessage(chatId, text,
options) in sendMessage; return _bot.telegram.editMessageText(...) in
editMessageText; return _bot.telegram.answerCbQuery(callbackId, options) in
answerCallbackQuery; return _bot.telegram.sendPhoto(chatId, photo, options) in
sendPhoto; and return _bot.telegram.sendDocument(chatId, document, options) in
sendDocument), preserving signatures and options.
---
Nitpick comments:
In @.env.example:
- Around line 2-4: Move the ADMIN_CHAT_IDS entry above the ADMIN_IDS entry in
the .env.example so dotenv-linter's alphabetical ordering is satisfied;
specifically reorder the two keys so ADMIN_CHAT_IDS appears before ADMIN_IDS
(preserving their values/descriptions) to eliminate the UnorderedKey warning.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.env.examplebackend.jsindex.jspackage.jsonrateLimiter.jsrunewager-endpoint.servicescripts/runewager_redeploy.shtelegramSafe.js
scripts/rw_cpu_guard.sh:
- Atomic trace CSV header init: mktemp → write → mv instead of direct truncate
telegramSafe.js:
- init(): validate bot.telegram before patching; set _inited=true only after
all patching succeeds (prevents permanently-broken "initialized" state)
- answerCallbackQuery patch in init(): use variadic ...args to preserve text
parameter (was narrowed to (callbackQueryId, extra), dropping text)
- Exported answerCallbackQuery(): destructure {text, showAlert, extra} from
options object and forward as positional args to answerCbQuery — fixes
[object Object] text bug; prefer answerCbQuery, fall back to answerCallbackQuery
rateLimiter.js:
- _globalSlot(): wrap fn() in Promise.race against 15s timeout so a hung
call never stalls the entire global queue chain indefinitely
scripts/runewager_redeploy.sh:
- Remove runewager-endpoint from legacy stop loop (it's a companion on port
3001, not a legacy bot conflict); add _restart_companion_services() step
called after _start_bot_service so the endpoint is always restarted
- Add runewager-endpoint to _summary() service state check
- Legacy stop loop: replace || true with explicit post-stop is-active check;
call fail() if unit is still running after systemctl stop
backend.js:
- Use ADMIN_IDS env var (same as bot's admin list) instead of ADMIN_CHAT_IDS
- Route sendAdmin() through globalThrottle (rateLimiter.js) to enforce the
35ms global gap and avoid unthrottled 429 bursts
- Wire rateLimiter.stats() into GET /health/full response for observability
https://claude.ai/code/session_01WpEqAiDbpqnBywMjYXJHf4
…nd hardening telegramSafe.js: - Exported helpers (sendMessage, editMessageText, answerCallbackQuery, sendPhoto, sendDocument) now call _bot.telegram.* directly instead of re-wrapping with _withRetry/enqueue/globalThrottle — methods are already patched in init() so double-wrapping caused extra retry/throttle latency - answerCallbackQuery helper drops outer _withRetry/globalThrottle for same reason - Patch tg.callApi as a safety net for all methods not individually patched (deleteMessage, getChatMember, setMyCommands, pinChatMessage, etc.) using a managed-methods Set to skip pass-through for methods already handled above, preventing double-wrapping when individually-patched methods call callApi internally backend.js: - sendAdmin: add 10s req.setTimeout so a hung socket never blocks Promise.all - POST /autofix/webhook: fail closed — reject (401) when AUTOFIX_SECRET is missing, not just when signature mismatches; unsigned traffic always rejected - Log directory creation wrapped in try/catch so startup survives permission/disk errors instead of aborting before service readiness is reported - Use ADMIN_IDS env var (bot's existing admin list) — ADMIN_CHAT_IDS removed .env.example: - Remove redundant ADMIN_CHAT_IDS entry; backend.js now reads ADMIN_IDS directly https://claude.ai/code/session_01WpEqAiDbpqnBywMjYXJHf4
User description
Summary by Sourcery
Introduce a new HTTP companion endpoint service and centralised Telegram API rate limiting, while tightening deployment/health tooling and telemetry.
New Features:
Enhancements:
Build:
Deployment:
CodeAnt-AI Description
Add companion HTTP endpoint, central Telegram rate-limiter, and safer Telegram API wrappers
What Changed
Impact
✅ Fewer Telegram rate-limit (429) failures and automatic retries✅ Fewer redeploy port-conflict failures during rollout✅ Clearer Autofix webhook and health diagnostics💡 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