Claude/plan v3 deployment vp mpw#115
Merged
Merged
Conversation
…urity, 23 Block 1 fixes Menu lifecycle overhaul: - replyMenu() TTL auto-vanish via ttlMs parameter - clearStaleMenuIds() on startup (24h threshold) - mainMenuSentAt / adminMenuSentAt timestamps on user schema - sendHelpMenu() fixed: bare ctx.reply() → replyMenu() (panel stacking bug) - replaceCallbackPanel() fallback now tracks message ID in user state - Admin mode toggle double-fire fixed Feature upgrades: - User giveaway list: sendUserGiveawaysPage() 5/page + 2-min auto-vanish - Admin giveaway panel: activeGiveawaysKeyboard(page) 5/page pagination - pmenu_referral sub-menu with share deep-link button - Admin stats: active window indicator + Refresh button - Admin health panel: inline memory/errors/users/persist-age/uptime - Broadcast builder: Preview button sends to admin DM before mass send Block 1 security & logic fixes: - getStartAppLink(): encodeURIComponent + regex whitelist - referralShareHTML(): escapeHtml() on code param - unwrapTelegramUrl(): safe fallback, scheme whitelist, www.t.me support - getDiscordLink(): null when not configured (no hardcoded fallback) - evaluatePendingActionTimeout(): >= boundary, NaN guard, no createdAt mutation - getPlayLink(): legacy user.playMode fallback restored - Weighted winner pool: splice-after-pick guarantees termination - deploy_status + logs: exec() → execFile() (no shell spawn) - SSHV: rejects null bytes, backticks, $( and ${ before exec - Startup env warnings: ADMIN_IDS, TELEGRAM_CHANNEL_ID, TELEGRAM_GROUP_ID Centralized helpers: computeParticipantWeight(), getRealGiveaways(), isNewUserPromoEligible() Metrics: added runewager_menu_stale_recoveries, pending_actions_timed_out, uptime_seconds load_tooltips.sh: full rewrite — atomic write, --push/--pull flags, parameterized REPO_DIR, HTML-safe tooltips, --dry-run mode, permission checks, guarded git ops Tests: readdirSync wrapped in try/catch; isCatchAllRegexPattern expanded; extractCommandHandlerNames supports let/var/no-semicolon Infrastructure: pre-deploy-checks gate 3b (menu symbols), CHANGELOG.md created, package.json bumped to 3.0.0 All 60 tests pass. node --check clean. https://claude.ai/code/session_01VijmtzjN63WZJy5gYgJAKs
… during PR merges Restores 12 categories of codex changes that were incorrectly overwritten by "current" (main) when resolving merge conflicts across 6 merge commits. Security / TLS hardening (index.js): - Restore resolveTlsCertPathIfAllowed(): multi-directory fallback for TLS certs (PROJECT_DIR, certs/, /etc/ssl, /etc/letsencrypt) — was collapsed to PROJECT_DIR only - Restore isHealthTlsEnabled() to use resolveTlsCertPathIfAllowed() - Restore requestHealthPayload(): centralized HTTP/HTTPS health fetcher with dynamic protocol detection — was inlined with hardcoded https in two places - Update /health command to use requestHealthPayload() (shows protocol label) - Update pamenu_tools_health to use requestHealthPayload() - Update startHealthServer() TLS cert read to use resolveTlsCertPathIfAllowed() Command injection protection (index.js): - commandNeedsConfirmation(): restore pipe-to-bash/zsh detection, destructive-cmd-piped pattern, redirect pattern — main simplified to only catch "sh" - commandBlocked(): restore explicit pipe-to-shell blocker pattern Race condition fixes (index.js): - deleteEphemeralBonusPrompt(): restore runUserMutation guards for safe read-delete-write of ephemeralBonusMsgId/ChatId - sendEphemeralBonusPrompt(): restore guards (claimedPromo check, active promo lookup), per-user dynamic promo lookup via getActivePromoCodeForUser() (was hardcoded promoStore.code), "I Have Claimed" callback button, mutation-safe writes - Add promo_confirm_claimed_next callback handler (button was added, handler missing) Deploy/runtime hardening (deploy.sh, prod-run.sh, runewager.service): - deploy.sh: restore data/backups/ mkdir, chown -R APP_NAME, chmod 0750/0640/0600 permission hardening after npm install - prod-run.sh: restore chmod 0750 for data/data/backups/logs dirs, chmod 0640 for log+session files, chown -R to service user after dir creation - runewager.service: add UMask=0077 (prevents world-readable files from service) .gitignore: - Restore legacy root-level data file patterns: users.json, giveaways.json, promo.json, env.json, analytics*.json (pre-data/ directory structure) All 60 tests pass. node --check clean. https://claude.ai/code/session_01VijmtzjN63WZJy5gYgJAKs
- TDZ fix: move pendingActionsTimedOut/menuStaleRecoveries declarations
before evaluatePendingActionTimeout to eliminate temporal dead-zone risk
- Timeout boundary: revert < to <= so age exactly equal to 15m is NOT
expired — aligns with /testall check and unit test expectations
- /logs line count: clamp to [1, 200] (adds Math.max(1,...) lower bound
to reject negative/zero values from user input)
- Health panel: fix _errorRate.windowErrors → _errorRate.count (field
did not exist; .count is the correct field on _errorRate object)
- /logs fallback: add execFile('tail') fallback when journalctl errors
with no output (non-systemd systems); uses BOT_LOG_FILE env or default
- executeSshvCommand: fix comment — said "spawn" but code uses exec();
updated to accurately describe exec with shell:true (admin-only)
- load_tooltips.sh: remove || true that defeated diff --cached check,
causing .gitignore changes to never be committed on --push
All 60 tests pass. node --check clean.
https://claude.ai/code/session_01VijmtzjN63WZJy5gYgJAKs
## Helpful Tooltips (was Content Drops) - Rename all UI strings: Content Drops → Helpful Tooltips throughout - New settings panel: interval, silent mode, Link Channel/Group - Target group linking via forwarded message (saves name + ID) - Dashboard footer shows real group name + ID - "Show all Helpful Tooltips (N)" button with dynamic count - Inline button builder: [Label - URL] && [Label2 - URL2] syntax - Multiple buttons per row with &&, new line = new row - [Open Bot] shorthand for standard Open Bot button - Full URL + label validation before save - postTipToConfiguredTarget uses silentMode flag + parsed buttons - tipsStore extended with targetGroupTitle and silentMode fields ## Giveaway v3.0+ - Extracted buildGiveawayAnnouncementText + buildGiveawayAnnouncementKeyboard helpers - scheduleGiveawayRefresh: auto-refresh at 25%, 50%, 75% of duration + re-pin - scheduleGiveawayReminders overhauled: 10m, 5m, 1min, 30sec, 10→1 countdown - HTML results format: @handle, SC WON, (2x boost applied), DM tip - Full admin winner report per winner: TG handle, display name, RW username, prize, boost - "View Results in Group" deep-link button in admin report - Winner DMs include SC amount, boost status, RW username - DM failure tracking with summary count - giveawayPreflightCheck: validates group linked, warns on missing pin permission - gwizStart calls preflight before wizard begins ## Scripts - generate_tooltips.sh: extracts DEFAULT_TIPS_LIST from index.js → data/tooltips.json (atomic, idempotent) - add_tooltip.sh: appends placeholder tooltip entry, outputs new ID - deploy.sh: step 3b auto-runs generate_tooltips.sh before service start - prod-run.sh: step 6b auto-runs generate_tooltips.sh before bot launch ## /testall - Added Helpful Tooltips System checks (tipsStore shape, count, interval, target, parser) - Added Giveaway Extended checks (helpers defined, preflight defined) ## Docs - RUNEWAGER_FUNCTIONALITY_MAP.md: full v3.0+ sync with flowcharts All 60 tests pass. node --check clean. https://claude.ai/code/session_01VijmtzjN63WZJy5gYgJAKs
… linking - Group command guard middleware: bot.use() intercepts all commands in group/supergroup chats and redirects to DM with a deep-link button. Passthrough commands with own group logic: link, linkrunewager, giveaway, start_giveaway, admin. Suppresses handler execution for all others. - Onboarding progress bar: onboardingProgressBar(step) renders ●●○○○ Step N of 5 — Label. showOnboardingPrompt() prepends a Markdown progress header (auto-deletes after 8s) before each step-specific prompt. - Onboarding completion card: shown once (user.onboarding.completionCardShown flag) when user reaches the main menu after completing all 5 steps. Includes feature summary and Open Menu button. - Admin System Tools: added 🔗 Group Linking button to adminSystemToolsKeyboard() with admin_sys_group_linking action handler (renders group linking panel with back-to-system-tools navigation). - Schema: completionCardShown added to onboarding default + migration guard. - Map: RUNEWAGER_FUNCTIONALITY_MAP.md fully updated; todolist.md updated. - All 60 tests pass, node --check clean. https://claude.ai/code/session_01VijmtzjN63WZJy5gYgJAKs
…ates PR #112 review fixes (sourcery-ai): - R1: tips_cmd_import_batch now uses await_tip_import_batch pending type with dedicated JSON-array router handler; proper MarkdownV2 prompt - R2: generate_tooltips.sh fixes command substitution pollution; use RUNEWAGER_APP env var instead of process.argv[1] (undefined in node -) - R3: add_tooltip.sh fixes shell injection; TOOLTIP_TEXT passed via TOOLTIP_TEXT_ENV env var, heredoc uses <<'EOF', process.argv[2] for file - R4: extend catchAllCases array with (.|\n)*, (.|\n)+, (\.|[\s\S])*; add post-whitespace-strip forms to CATCH_ALL_CORES set - R5: extractCommandHandlerNames test now covers let/var declarations and no-semicolon forms (CMD_FOUR/eta, CMD_FIVE/theta) - R6: RUNEWAGER_FUNCTIONALITY_MAP.md typo "auto-deletes 8s" → "after 8s" Codebase audit duplicate removal: - A1: remove dead buildGiveawayAnnouncementText(giveaway, remainingStr) at ~12533; keep dynamic version at ~13795 - A2: remove simplified buildGiveawayAnnouncementKeyboard at ~13817 (wrong tgw_participants_ callback); restore full 5-row version - A3+A4: remove first duplicate bot.action registrations for admin_cat_system and admin_cat_support (identical bodies) All 60 tests pass, node --check clean, bash -n clean on both scripts. https://claude.ai/code/session_01VijmtzjN63WZJy5gYgJAKs
Resolved conflicts in favour of our branch (HEAD) for: - index.js: keep await_tip_import_batch router + handler, keep A1/A2 duplicate-function removal, keep Content Drops branding from main - add_tooltip.sh / generate_tooltips.sh: keep env-var injection fixes - test/smoke.test.js: keep extended catchAllCases + extractCommand fixtures - RUNEWAGER_FUNCTIONALITY_MAP.md / todolist.md: keep our updated content Incoming from origin/main: claude-pr-results-20260228_193756Z.md https://claude.ai/code/session_01VijmtzjN63WZJy5gYgJAKs
…ry bot start Every script that starts/restarts the bot now follows the same safe sequence: 1. git pull origin main (fetch + reset --hard) 2. generate_tooltips.sh (extracts DEFAULT_TIPS_LIST → data/tooltips.json) 3. kill any process blocking PORT (default 3000) 4. start/restart bot Changes per file: - prod-run.sh: add step 9c — free_port_if_conflicted() BEFORE step 10 restart (port-kill was already present in God-Mode Heal but fired after, not before) - deploy.sh: add step 3c — inline lsof/fuser port-kill before systemctl start - start.sh: add git fetch+reset, generate_tooltips, port-kill, stale-PID kill before bot launch; replace refuse-on-duplicate with kill-and-continue - dev-run.sh: add git fetch+reset (best-effort), generate_tooltips, port-kill before exec node - scripts/rollback.sh: add generate_tooltips after npm ci (refreshes from rolled-back index.js), add lsof/fuser port-kill before service start (no git pull — rollback intentionally targets an older commit) https://claude.ai/code/session_01VijmtzjN63WZJy5gYgJAKs
- Escape Markdown special chars in runewagerUsername to prevent parse failures - Add explicit smoke-test assertions for (.|)* and (.|)+ catch-all forms - Strip inline comments from .env PORT values in deploy.sh, dev-run.sh, start.sh (e.g. PORT=3000 # dev now correctly yields 3000) - Fix showOnboardingPrompt JSDoc: steps documented as 1–4 → 1–5 (matches impl) - Upgrade all port-block kill loops to SIGTERM-first then SIGKILL after 2s grace - Add path + shape validation to generate_tooltips.sh Node extraction block (validates RUNEWAGER_APP is absolute .js path; checks array literal size/shape) https://claude.ai/code/session_01VijmtzjN63WZJy5gYgJAKs
- Create docs/INDEX.md: exhaustive cross-reference of all 95 commands, 266 action handlers, and 50+ pending action types mapped to feature docs - Create docs/features/ with 15 per-feature .md files covering every bot subsystem (onboarding, menus, giveaway, bonus, promos, tooltips, referral, SSHV, deploy, user lookup, group linking, bug reports, announcements, misc) - Create docs/TODO_FUNCTIONALITY_UPGRADE.md with 14 open stale-menu and missing-handler items (walkthrough dead-end, clearOldMenus gaps, missing tip_view handler, language stub, broadcastFailedUsers cap) - Update RUNEWAGER_FUNCTIONALITY_MAP.md section 26 with full docs/ table and mandate for future Claude sessions to consult docs/INDEX.md first Future sessions: read docs/INDEX.md → feature .md → index.js (if needed) https://claude.ai/code/session_01VijmtzjN63WZJy5gYgJAKs
|
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 GuideImplements v3.1 deployment and functionality updates: adds group-command guard middleware and admin group linking entrypoint, enhances onboarding UX with progress/completion UI, introduces a safer batch tooltip import flow and hardens tooltip tooling/scripts, wires port-unblocking and tooltip regeneration into deploy/start/rollback scripts, and creates a structured docs/ feature documentation system plus audit/test fixes. Sequence diagram for group command guard middleware in v3.1sequenceDiagram
actor User
participant Telegram
participant BotMiddleware as Bot_Middleware
participant CommandHandlers as Command_Handlers
User->>Telegram: /menu in group chat
Telegram->>BotMiddleware: update (chat.type = group)
BotMiddleware->>BotMiddleware: detect command text
BotMiddleware->>BotMiddleware: extract rawCmd = menu
alt rawCmd in GROUP_PASSTHROUGH_COMMANDS
BotMiddleware->>CommandHandlers: next()
CommandHandlers-->>User: command-specific group handler
else non passthrough command
BotMiddleware->>BotMiddleware: build DM deep link using ctx.botInfo.username
BotMiddleware->>User: reply "This command works in DM" + Open DM button
BotMiddleware-->>CommandHandlers: do not call next()
end
User->>Telegram: taps Open DM button
Telegram->>CommandHandlers: starts DM chat with bot
CommandHandlers-->>User: normal DM onboarding/menu flow
Sequence diagram for onboarding progress bar and completion cardsequenceDiagram
actor User
participant Telegram
participant Bot as Bot_Onboarding
User->>Telegram: /start in DM
Telegram->>Bot: start update
Bot->>Bot: getUser(ctx)
Bot->>Bot: detect onboarding step (step = 2 for example)
Bot->>Bot: onboardingProgressBar(step)
Bot->>User: reply "Onboarding Progress" + dot bar (Markdown)
Bot->>Bot: schedule deleteMessage(progressMsg) in 8s
Bot->>User: send step 2 specific prompt
loop for each subsequent step (3..5)
User->>Telegram: responds to step prompt
Telegram->>Bot: message update
Bot->>Bot: advance onboarding.currentStep
Bot->>Bot: showOnboardingPrompt(ctx,user,step)
Bot->>User: progress bar header (auto delete) + next step prompt
end
User->>Telegram: completes final step 5
Telegram->>Bot: update (step 5 complete)
Bot->>Bot: mark user.onboarding.completedAt
alt completionCardShown is false
Bot->>Bot: set user.onboarding.completionCardShown = true
Bot->>User: reply onboarding completion card + Open Menu button
end
Bot->>User: sendPersistentUserMenu(ctx,user)
Sequence diagram for batch tooltip JSON import pending actionsequenceDiagram
actor Admin
participant Telegram
participant Bot as Bot_Tooltips
participant TipsStore as tipsStore
Admin->>Telegram: taps Import Tooltips (JSON) button
Telegram->>Bot: callback tips_cmd_import_batch
Bot->>Bot: clearPendingAction(user)
Bot->>Bot: set user.pendingAction = await_tip_import_batch
Bot->>Admin: reply instructions with JSON example (MarkdownV2)
Admin->>Telegram: sends JSON array message
Telegram->>Bot: message update
Bot->>Bot: detect pendingAction.type = await_tip_import_batch
Bot->>Bot: parse JSON text
alt invalid JSON or not array
Bot->>Admin: reply error (Invalid JSON)
else valid array
loop for each item in batch
Bot->>Bot: validate item.text and item.enabled
Bot->>TipsStore: push { id: nextTipId, text, enabled }
TipsStore->>TipsStore: increment nextTipId
end
Bot->>Bot: user.pendingAction = null
alt added.length == 0
Bot->>Admin: reply "No valid tooltip entries found"
else added.length > 0
Bot->>Bot: persistRuntimeState()
Bot->>Bot: saveHelpfulMessages()
Bot->>Admin: reply "Imported N tooltips, IDs: ..."
end
end
Updated class diagram for user onboarding, pending actions, and tooltip storeclassDiagram
class User {
number id
string username
string runewagerUsername
OnboardingState onboarding
WalkthroughState walkthrough
PendingAction pendingAction
Set~number~ giveawayJoinedIds
Set~string~ badges
number profileXP
}
class OnboardingState {
number currentStep
number startedAt
number completedAt
number[] stepTimestamps
boolean completionCardShown
}
class WalkthroughState {
number currentStep
Set~number~ completed
boolean started
}
class PendingAction {
string type
number createdAt
any data
}
class PendingActionType {
<<enumeration>>
await_tip_add_text
await_tip_edit_text
await_tip_import_batch
await_tip_settings_interval
await_tip_link_target
await_referral_code
await_runewager_username
await_bugreport
await_sshv_command
await_admin_whois
await_admin_resolvebug
}
class TipsStore {
Tip[] tips
boolean systemEnabled
number intervalHours
string targetGroup
string targetGroupTitle
boolean silentMode
number lastSentTipId
number nextTipId
}
class Tip {
number id
string text
boolean enabled
}
User --> OnboardingState : has
User --> WalkthroughState : has
User --> PendingAction : may_have
PendingAction --> PendingActionType : type_in
TipsStore --> Tip : contains
Flow diagram for docs feature documentation system introduced in this PRflowchart TD
subgraph Docs["docs/ directory"]
INDEX["docs/INDEX.md"]
TODO["docs/TODO_FUNCTIONALITY_UPGRADE.md"]
subgraph Features["docs/features/"]
F01["01-onboarding.md"]
F02["02-user-menu.md"]
F03["03-admin-menu.md"]
F04["04-giveaway.md"]
F05["05-bonus-30sc.md"]
F06["06-promos.md"]
F07["07-tooltips.md"]
F08["08-referral.md"]
F09["09-sshv.md"]
F10["10-deploy-ops.md"]
F11["11-user-lookup.md"]
F12["12-group-linking.md"]
F13["13-bug-reports.md"]
F14["14-announcements.md"]
F15["15-misc-commands.md"]
end
end
CODE[index.js and scripts/*]
QA["QA tests and audits"]
ClaudeSession["Claude coding session"]
ClaudeSession -->|must read first| INDEX
INDEX -->|links to feature docs| Features
Features -->|describe behavior of| CODE
CODE -->|changes require updates to| Features
CODE -->|changes require updates to| INDEX
CODE -->|issues recorded in| TODO
QA -->|verifies behavior vs docs| Features
QA -->|logs fixes in| TODO
INDEX -->|summarizes open items from| TODO
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a central docs INDEX and 16 feature docs; implements Phase work including a 35-step Walkthrough, onboarding progress and menu lifecycle management (clearOldMenus/lastMenuMsgId tracking), tooltip view/edit flows, improved broadcast-failure handling, and startup/deploy script integrations. (≤50 words) Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
codeant-ai
Bot
added
the
size:XXL
![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 3 issues, and left some high level feedback:
- The new
git fetch && reset --hard origin/maininstart.shanddev-run.shwill silently discard any local changes or non-mainbranches on every run; consider gating this behind an explicit flag/env var or a separateupdatescript so local/staging environments aren’t unexpectedly overwritten. - Port‑freeing logic (lsof/fuser, SIGTERM→SIGKILL, PORT parsing) is now duplicated across
start.sh,dev-run.sh,deploy.sh,scripts/rollback.sh, and partiallyprod-run.sh; extracting this into a single shared helper (or reusingfree_port_if_conflicted) would reduce drift and make future tweaks less error‑prone.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `git fetch && reset --hard origin/main` in `start.sh` and `dev-run.sh` will silently discard any local changes or non-`main` branches on every run; consider gating this behind an explicit flag/env var or a separate `update` script so local/staging environments aren’t unexpectedly overwritten.
- Port‑freeing logic (lsof/fuser, SIGTERM→SIGKILL, PORT parsing) is now duplicated across `start.sh`, `dev-run.sh`, `deploy.sh`, `scripts/rollback.sh`, and partially `prod-run.sh`; extracting this into a single shared helper (or reusing `free_port_if_conflicted`) would reduce drift and make future tweaks less error‑prone.
## Individual Comments
### Comment 1
<location path="add_tooltip.sh" line_range="47-56" />
<code_context>
# Append new entry and get new ID using Node.js
-NEW_ID=$(node - "$TOOLTIPS_FILE" <<EOF
+# Text and tmp path are passed via env vars to avoid shell injection on special characters
+NEW_ID=$(TOOLTIP_TEXT_ENV="$TOOLTIP_TEXT" TOOLTIP_TMP_FILE="$TMP_FILE" node - "$TOOLTIPS_FILE" <<'EOF'
const fs = require('fs');
-const file = process.argv[1];
+const file = process.argv[2];
+const text = process.env.TOOLTIP_TEXT_ENV;
+const tmpFile = process.env.TOOLTIP_TMP_FILE;
const list = JSON.parse(fs.readFileSync(file, 'utf8'));
const maxId = list.reduce((m, t) => Math.max(m, Number(t.id) || 0), 0);
const newId = maxId + 1;
-list.push({ id: newId, text: $(node -e "process.stdout.write(JSON.stringify('$TOOLTIP_TEXT'))"), enabled: true });
-fs.writeFileSync('${TMP_FILE}', JSON.stringify(list, null, 2));
+list.push({ id: newId, text, enabled: true });
+fs.writeFileSync(tmpFile, JSON.stringify(list, null, 2));
console.log(newId);
EOF
</code_context>
<issue_to_address>
**suggestion:** Consider basic type/shape validation before mutating tooltips.json in the Node helper
This script assumes `list` is an array of objects with `id` fields. If `tooltips.json` is valid JSON but not an array (or has non-numeric `id`s), `reduce`/`push` could throw or behave oddly. A small guard like `if (!Array.isArray(list)) throw new Error('Expected array');` and validating `id` as numeric before using it in `maxId` would make the script fail fast and avoid corrupting the file further.
```suggestion
const fs = require('fs');
const file = process.argv[2];
const text = process.env.TOOLTIP_TEXT_ENV;
const tmpFile = process.env.TOOLTIP_TMP_FILE;
// Parse and validate basic shape of tooltips.json
const list = JSON.parse(fs.readFileSync(file, 'utf8'));
if (!Array.isArray(list)) {
throw new Error('tooltips.json must contain a JSON array');
}
// Derive max numeric id, ignoring entries with non-numeric ids
const numericIds = list
.map((t) => (t && Object.prototype.hasOwnProperty.call(t, 'id') ? Number(t.id) : NaN))
.filter((id) => Number.isFinite(id));
const maxId = numericIds.length ? Math.max(...numericIds) : 0;
const newId = maxId + 1;
// Append new tooltip entry
list.push({ id: newId, text, enabled: true });
// Write updated list back to tmp file
fs.writeFileSync(tmpFile, JSON.stringify(list, null, 2));
console.log(newId);
```
</issue_to_address>
### Comment 2
<location path="docs/features/12-group-linking.md" line_range="20" />
<code_context>
+| Trigger | Callback / Command | Role |
+|---------|-------------------|------|
+| Button: Group Linking | `settings_group_linking_tools` | Admin |
+| (From admin settings tab) | `admin_cat_system` | Admin |
+
+---
</code_context>
<issue_to_address>
**issue:** Entry points table likely lists the wrong callback name for the group-linking tools entry point.
The second row’s callback should match the actual entry point used when opening group-linking tools from the admin settings tab (e.g., `settings_group_linking_tools` / `admin_sys_group_linking`), rather than the broader `admin_cat_system` callback, so the table accurately reflects how this feature is invoked.
</issue_to_address>
### Comment 3
<location path="docs/features/15-misc-commands.md" line_range="51" />
<code_context>
+
+---
+
+## Walkthrough Flow
+
+```
</code_context>
<issue_to_address>
**suggestion:** Walkthrough command `/walkthrough` is described but not listed in the User Commands table.
Since this section introduces `/walkthrough → interactive onboarding walkthrough`, it should also appear in the "User Commands" table for consistency and discoverability, or be explicitly noted there as disabled/hidden if that’s intentional.
Suggested implementation:
```
| `/scan_eligibility` | ~14225 | Check giveaway eligibility across users |
| `/pick_winner` | ~14329 | Manually run winner picker |
| `/gw_pause` | ~14154 | Pause a giveaway |
| `/gw_resume` | ~14175 | Resume a paused giveaway |
| `/testgiveaway` | ~13878 | Simulate a giveaway flow |
| `/walkthrough` | ~XXXXX | Interactive onboarding walkthrough |
---
## Walkthrough Flow
```
1. Replace `~XXXXX` with the appropriate message/line reference to be consistent with the rest of the table.
2. If `/walkthrough` is currently disabled/hidden, update the description to reflect that status (e.g., “Interactive onboarding walkthrough (currently disabled/hidden)”).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
|
||
| --- | ||
|
|
||
| ## Walkthrough Flow |
There was a problem hiding this comment.
suggestion: Walkthrough command /walkthrough is described but not listed in the User Commands table.
Since this section introduces /walkthrough → interactive onboarding walkthrough, it should also appear in the "User Commands" table for consistency and discoverability, or be explicitly noted there as disabled/hidden if that’s intentional.
Suggested implementation:
| `/scan_eligibility` | ~14225 | Check giveaway eligibility across users |
| `/pick_winner` | ~14329 | Manually run winner picker |
| `/gw_pause` | ~14154 | Pause a giveaway |
| `/gw_resume` | ~14175 | Resume a paused giveaway |
| `/testgiveaway` | ~13878 | Simulate a giveaway flow |
| `/walkthrough` | ~XXXXX | Interactive onboarding walkthrough |
---
## Walkthrough Flow
- Replace
~XXXXXwith the appropriate message/line reference to be consistent with the rest of the table. - If
/walkthroughis currently disabled/hidden, update the description to reflect that status (e.g., “Interactive onboarding walkthrough (currently disabled/hidden)”).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (20)
docs/features/05-bonus-30sc.md-29-41 (1)
29-41:⚠️ Potential issue | 🟡 MinorLabel fenced flow blocks with a language.
These blocks trigger markdownlint MD040.
🛠️ Proposed fix
-``` +```text pmenu_claim_bonus → bonus options menu └── w30_request_start → eligibility check + request menu @@ └── Request stored → admin notified-
+text
/wager30_admin → admin bonus manager
├── w30_admin_pick_approve_{userId} → approve request → SC credited
@@
└── w30_admin_reset_{userId} → reset bonus state for userAlso applies to: 47-57
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/features/05-bonus-30sc.md` around lines 29 - 41, Fenced flow blocks in docs/features/05-bonus-30sc.md are missing a language label which triggers markdownlint MD040; update each triple-backtick fence that contains the flow diagrams (e.g., the block containing pmenu_claim_bonus → bonus options menu and the block with /wager30_admin → admin bonus manager, as noted also at lines 47-57) to include a language identifier like "text" (e.g., replace ``` with ```text) so the blocks are properly labeled while keeping the diagram content unchanged.docs/features/06-promos.md-94-94 (1)
94-94:⚠️ Potential issue | 🟡 MinorFix callback ID mismatch for claim-bonus entry.
menu_claim_bonusconflicts with the earlierpmenu_claim_bonusnaming and can misdocument the wiring.🛠️ Proposed fix
-| `menu_claim_bonus` | Show eligible promos list | +| `pmenu_claim_bonus` | Show eligible promos list |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/features/06-promos.md` at line 94, The docs table has a callback ID mismatch: the promo entry uses pmenu_claim_bonus elsewhere but the row lists menu_claim_bonus; update the table row to use the correct callback ID (pmenu_claim_bonus) so it matches the implementation and other documentation for the claim-bonus flow, ensuring consistency between the promo wiring and the docs.docs/features/08-referral.md-28-40 (1)
28-40:⚠️ Potential issue | 🟡 MinorSpecify a language for the flow code fence.
This block triggers markdownlint MD040.
🛠️ Proposed fix
-``` +```text pmenu_referral → referral menu ├── ref_menu_code → show referral code (copyable) ├── ref_menu_share → share link (Telegram share button) └── ref_menu_how → explain boost mechanism @@ ├── Referrer gets 7-day boost ├── New user gets 7-day boost └── Both parties notified via DM</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/features/08-referral.mdaround lines 28 - 40, The fenced flow block in
docs/features/08-referral.md lacks a language tag and triggers markdownlint
MD040; update the opening code fence to include a language (e.g., change ``` toref_menu_how and the applyOnboardingReferralCode flow so the linter recognizes it as a code block with an explicit language.docs/features/10-deploy-ops.md-32-43 (1)
32-43:⚠️ Potential issue | 🟡 MinorAdd fenced code languages to satisfy markdownlint.
Both blocks are unlabeled and trigger MD040.
🛠️ Proposed fix
-``` +```text /deploy [source] └── deploy.sh called (source: github|bot|vps) @@ 7. Health check └── Telegram notify at each step-
+text
TestAll complete — X passed, Y warnings, Z failures.Also applies to: 70-72
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/features/10-deploy-ops.md` around lines 32 - 43, The markdown has unlabeled fenced code blocks that trigger MD040; add a language identifier (e.g., text) to each fenced code block containing the deployment steps starting with "/deploy [source]" and the test summary block beginning with "TestAll complete — X passed, Y warnings, Z failures." Update those fences to use ```text so both blocks (including the other occurrence at lines referenced as 70-72) are labeled and satisfy markdownlint.docs/features/11-user-lookup.md-31-50 (1)
31-50:⚠️ Potential issue | 🟡 MinorAdd language identifiers to fenced code blocks.
These blocks currently trigger markdownlint MD040 warnings.
🛠️ Proposed fix
-``` +```text /whois <id|username> └── Find user in userStore └── Show: userId, username, SC balance, bonus state, onboarding step, badges, pendingAction, last active-
+text
/bonusstatus
└── Show: bonus type, requested at, approved/denied,
SC amount, reason (if denied), weekly cooldown state-``` +```text /refreshuser <userId> └── Merge missing schema fields from DEFAULT_USER └── Confirm: "Schema refreshed for user X"</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/features/11-user-lookup.mdaround lines 31 - 50, The fenced code blocks
for the command outlines (/whois, /bonusstatus, /refreshuser) lack language
identifiers and trigger markdownlint MD040; update each triple-backtick fence to
include a language (e.g., use ```text) for the blocks containing the /whois,
/bonusstatus, and /refreshuser examples so the linter stops complaining and the
blocks render consistently.</details> </blockquote></details> <details> <summary>docs/features/11-user-lookup.md-19-19 (1)</summary><blockquote> `19-19`: _⚠️ Potential issue_ | _🟡 Minor_ **Escape pipe in `/whois` usage to preserve table structure.** The `|` inside `<id|username>` is parsed as a column delimiter. <details> <summary>🛠️ Proposed fix</summary> ```diff -| `/whois <id|username>` | Lookup user | Admin | +| `/whois <id\|username>` | Lookup user | Admin | ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@docs/features/11-user-lookup.md` at line 19, The table cell containing `/whois <id|username>` is breaking the Markdown table because the pipe is treated as a column delimiter; update that cell by escaping the pipe or using the HTML entity so it is treated as literal (e.g., change the `/whois <id|username>` occurrence to use an escaped pipe or `|` such as `/whois <id\|username>` or `/whois <id|username>`), ensuring the table structure remains intact. ``` </details> </blockquote></details> <details> <summary>docs/features/13-bug-reports.md-29-36 (1)</summary><blockquote> `29-36`: _⚠️ Potential issue_ | _🟡 Minor_ **Add fence languages to the flow blocks.** Both fenced blocks should declare a language (e.g., `text`) to satisfy markdown lint and improve editor tooling. <details> <summary>Suggested doc fix</summary> ```diff -``` +```text help_open_bugreport or menu_bugreport └── pendingAction: await_bugreport └── User types bug description (text input) └── Report saved to bugReports[] with timestamp + userId └── Admin notified via DM └── User receives: "Thank you for your report!" ``` @@ -``` +```text pamenu_bug_reports → list open reports └── admin_cmd_viewbugs → recent reports with userId + text └── admin_cmd_resolvebug_prompt → pendingAction: await_admin_resolvebug └── Admin types reportId → mark as resolved └── admin_cmd_exportbugs → export all reports as text ``` ``` </details> Also applies to: 42-48 <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/features/13-bug-reports.mdaround lines 29 - 36, The fenced code blocks
in the flow documentation lack a declared language, which trips markdown linting
and editor tooling; update both blocks by adding a language marker (e.g.,
text) to the opening fences for the flow that includes help_open_bugreport and
menu_bugreport, and the block that lists pamenu_bug_reports, admin_cmd_viewbugs,
admin_cmd_resolvebug_prompt, and admin_cmd_exportbugs so the fences read like
text ...to satisfy linters and improve editor rendering.</details> </blockquote></details> <details> <summary>docs/features/04-giveaway.md-30-54 (1)</summary><blockquote> `30-54`: _⚠️ Potential issue_ | _🟡 Minor_ **Add a language tag to the wizard flow code fence.** Use `text` (or another appropriate language) on the fenced block for markdownlint/tooling compatibility. <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/features/04-giveaway.mdaround lines 30 - 54, The fenced wizard flow
block containing pamenu_start_giveaway and the gwiz_* step keys lacks a language
tag; update the opening fence fromtotext (or another appropriate
language) so markdownlint and tooling recognize it—e.g., locate the block with
pamenu_start_giveaway and
gwiz_title_skip/gwiz_await_title/gwizScKeyboard/gwiz_winners_* etc. and add the
language tag to the code fence.</details> </blockquote></details> <details> <summary>docs/features/04-giveaway.md-22-23 (1)</summary><blockquote> `22-23`: _⚠️ Potential issue_ | _🟡 Minor_ **Correct Join button role to `User`.** The “Join (in group)” action is user-facing; this row currently marks it as Admin and conflicts with the Join Flow section. <details> <summary>Suggested doc fix</summary> ```diff -| Button: Join (in group) | `gwiz_start_here` | Admin | +| Button: Join (in group) | `gwiz_start_here` | User | ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@docs/features/04-giveaway.md` around lines 22 - 23, Update the table row that lists "Button: Join (in group)" with the identifier `gwiz_start_here` to mark its role as User (not Admin) so it matches the Join Flow; similarly verify the `/join` row remains marked User and ensure both entries consistently reflect the user-facing action. ``` </details> </blockquote></details> <details> <summary>docs/features/14-announcements.md-26-36 (1)</summary><blockquote> `26-36`: _⚠️ Potential issue_ | _🟡 Minor_ **Add a language tag to the broadcast flow fence.** Specify a fence language (e.g., `text`) for markdown lint/tooling compatibility. <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/features/14-announcements.mdaround lines 26 - 36, The markdown code
fence for the broadcast flow diagram doesn't include a language tag; update the
opening triple backticks to include a language such as "text" (e.g., ```text) so
tooling/linting recognizes it; apply this change to the fence that contains the
admin_cmd_announce_start flow (references: admin_cmd_announce_start,
broadcastFailedUsers[], /broadcast_retry, /broadcast_failed).</details> </blockquote></details> <details> <summary>docs/features/09-sshv.md-26-34 (1)</summary><blockquote> `26-34`: _⚠️ Potential issue_ | _🟡 Minor_ **Add language identifiers to both flow code fences.** Use `text` for these blocks to satisfy markdown lint and improve rendering/tool support. Also applies to: 40-46 <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/features/09-sshv.mdaround lines 26 - 34, The code fence blocks showing
the SSHV flow lack language identifiers; update both triple-backtick fences that
contain the flow diagram (the block that begins with "/sshv →
renderSshvConsole(ctx) ..." and the other similar block referenced in the
comment) to use the text language identifier by changingtotext so
tools/markdownlint render them correctly; ensure this is applied to the flow
involving renderSshvConsole, sshv_run_prompt, sshv_confirm_run and
sshv_cancel_run.</details> </blockquote></details> <details> <summary>docs/features/02-user-menu.md-32-33 (1)</summary><blockquote> `32-33`: _⚠️ Potential issue_ | _🟡 Minor_ **Fix the Promos row column semantics.** This row currently places callback names in the “Purpose” column and a description in “Callback Data,” unlike the rest of the table. <details> <summary>Suggested doc fix</summary> ```diff -| 🎁 Promos | (opens promo list) | `menu_claim_bonus` or `pmenu_claim_bonus` | +| 🎁 Promos | `menu_claim_bonus` or `pmenu_claim_bonus` | Opens promo list | ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@docs/features/02-user-menu.md` around lines 32 - 33, The Promos table row has its cells swapped: move the descriptive text "opens promo list" into the Purpose column and place the callback identifiers `menu_claim_bonus` or `pmenu_claim_bonus` into the Callback Data column so the row matches the other entries (i.e., change the Promos row from "| 🎁 Promos | (opens promo list) | `menu_claim_bonus` or `pmenu_claim_bonus` |" to have Purpose = "opens promo list" and Callback Data = "`menu_claim_bonus` or `pmenu_claim_bonus`"). ``` </details> </blockquote></details> <details> <summary>docs/features/13-bug-reports.md-54-61 (1)</summary><blockquote> `54-61`: _⚠️ Potential issue_ | _🟡 Minor_ **Fix callback table schema (rows currently have 4 cells, header has 3).** Markdown renderers will drop/misplace trailing data in this table. Add an explicit action/effect column in the header (or remove the extra cell in rows). <details> <summary>Suggested doc fix</summary> ```diff -| Callback | Handler Line | Role | -|----------|-------------|------| -| `help_open_bugreport` | ~7577 | Set `await_bugreport` | User | -| `menu_bugreport` | ~8609 | Set `await_bugreport` | User | -| `pamenu_bug_reports` | ~7954 | View open reports | Admin | -| `admin_cmd_viewbugs` | ~9251 | List recent reports | Admin | -| `admin_cmd_resolvebug_prompt` | ~9257 | Set `await_admin_resolvebug` | Admin | -| `admin_cmd_exportbugs` | ~9265 | Export all reports | Admin | +| Callback | Handler Line | Effect | Role | +|----------|--------------|--------|------| +| `help_open_bugreport` | ~7577 | Set `await_bugreport` | User | +| `menu_bugreport` | ~8609 | Set `await_bugreport` | User | +| `pamenu_bug_reports` | ~7954 | View open reports | Admin | +| `admin_cmd_viewbugs` | ~9251 | List recent reports | Admin | +| `admin_cmd_resolvebug_prompt` | ~9257 | Set `await_admin_resolvebug` | Admin | +| `admin_cmd_exportbugs` | ~9265 | Export all reports | Admin | ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@docs/features/13-bug-reports.md` around lines 54 - 61, The table header has three columns but the rows contain four cells (an extra action/effect column), causing Markdown rendering issues; update the table in docs/features/13-bug-reports.md so the header explicitly includes the fourth column (e.g., add an "Action/Effect" or "Action" column) to match rows for callbacks like `help_open_bugreport`, `menu_bugreport`, `pamenu_bug_reports`, `admin_cmd_viewbugs`, `admin_cmd_resolvebug_prompt`, and `admin_cmd_exportbugs`, ensuring each row cell aligns with the header and renders correctly. ``` </details> </blockquote></details> <details> <summary>docs/features/04-giveaway.md-127-128 (1)</summary><blockquote> `127-128`: _⚠️ Potential issue_ | _🟡 Minor_ **Update line number references in documentation.** The functions are valid and actively used in index.js, but the documented line numbers are inaccurate. `buildGiveawayAnnouncementText` is defined at line 13765 (not 13795), and `buildGiveawayAnnouncementKeyboard` is defined at line 12531 (not 12561). Update the documentation to reflect the correct definition locations. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@docs/features/04-giveaway.md` around lines 127 - 128, Update the inaccurate line-number references in the docs for the two giveaway helper functions: change the documented line number for buildGiveawayAnnouncementText from 13795 to 13765, and change the documented line number for buildGiveawayAnnouncementKeyboard from 12561 to 12531 so the docs point to the correct definition locations. ``` </details> </blockquote></details> <details> <summary>docs/features/01-onboarding.md-90-90 (1)</summary><blockquote> `90-90`: _⚠️ Potential issue_ | _🟡 Minor_ **Use `pendingAction` terminology consistently.** Line 90 says `pending_action` timeout, but the documented state key elsewhere is `user.pendingAction`. Align wording to avoid confusion in implementation/debug flows. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@docs/features/01-onboarding.md` at line 90, The documentation uses inconsistent terminology: replace the `pending_action` mention with the project's canonical key `pendingAction` (and reference `user.pendingAction` where applicable) so wording matches code; update the sentence that currently reads "`pending_action` timeout (15 min) applies to all text input steps." to use `pendingAction` (or explicitly `user.pendingAction`) and indicate the 15-minute timeout applies to that key. ``` </details> </blockquote></details> <details> <summary>docs/features/07-tooltips.md-47-92 (1)</summary><blockquote> `47-92`: _⚠️ Potential issue_ | _🟡 Minor_ **Add language identifiers to fenced flow blocks.** Flow blocks at Line 47, Line 64, Line 76, and Line 86 should specify a language (e.g., `text`) to satisfy markdown linting and keep rendering/tooling consistent. <details> <summary>Suggested patch</summary> ```diff -``` +```text tips_cmd_add └── pendingAction: await_tip_add_text └── User sends tooltip text (HTML/plain text) └── Text parsed (body + optional button rows) └── Tip saved → tipsDashboard refreshed ``` @@ -``` +```text tips_cmd_edit → tipSelectKeyboard('tip_edit_select') @@ -``` +```text tips_cmd_remove → tipSelectKeyboard('tip_remove') @@ -``` +```text tips_cmd_import_batch @@ ``` ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/features/07-tooltips.mdaround lines 47 - 92, The fenced flow blocks
for tips_cmd_add, tips_cmd_edit, tips_cmd_remove, and tips_cmd_import_batch lack
a language identifier, triggering markdown linting errors; update each
triple-backtick fence that wraps the flows for symbols tips_cmd_add,
tips_cmd_edit → tipSelectKeyboard('tip_edit_select'), tips_cmd_remove →
tipSelectKeyboard('tip_remove'), and tips_cmd_import_batch to include a language
token (e.g., ```text) at the opening fence so the blocks render and lint
correctly.</details> </blockquote></details> <details> <summary>docs/features/15-misc-commands.md-53-59 (1)</summary><blockquote> `53-59`: _⚠️ Potential issue_ | _🟡 Minor_ **Add fenced-code language for the walkthrough block.** Line 53 should specify a language (e.g., `text`) to satisfy markdownlint MD040. <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/features/15-misc-commands.mdaround lines 53 - 59, The fenced code
block for the walkthrough commands is missing a language tag causing
markdownlint MD040; update the block header fromto a language-tagged fence such astext (i.e., change the opening fence that contains the/walkthrough → interactive onboarding walkthroughblock to include a language liketext)
so the code block is language-specified.</details> </blockquote></details> <details> <summary>docs/INDEX.md-3-3 (1)</summary><blockquote> `3-3`: _⚠️ Potential issue_ | _🟡 Minor_ **Avoid source-of-truth conflict with `RUNEWAGER_FUNCTIONALITY_MAP.md`.** Line 3 describes this index as primary for all sessions, which conflicts with the repo rule that `RUNEWAGER_FUNCTIONALITY_MAP.md` is authoritative. Reword this line and link to the map explicitly. Based on learnings: Applies to `RUNEWAGER_FUNCTIONALITY_MAP.md` : Always treat `RUNEWAGER_FUNCTIONALITY_MAP.md` as the authoritative source of truth before and after code changes. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@docs/INDEX.md` at line 3, Rephrase the sentence in docs/INDEX.md that currently calls this index "the primary navigation index for all Claude sessions" to remove source-of-truth language and instead reference RUNEWAGER_FUNCTIONALITY_MAP.md as authoritative; update the text to say this index is a supplemental navigation aid and add an explicit link to RUNEWAGER_FUNCTIONALITY_MAP.md (the authoritative map) so readers are directed to consult RUNEWAGER_FUNCTIONALITY_MAP.md before and after making changes. ``` </details> </blockquote></details> <details> <summary>start.sh-82-86 (1)</summary><blockquote> `82-86`: _⚠️ Potential issue_ | _🟡 Minor_ **Handle all stale bot PIDs, not only the first match.** Using `head -1` can leave additional stale `node .../index.js` processes alive. Iterate all matching PIDs before launch. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@start.sh` around lines 82 - 86, The current startup uses pgrep -f "node.*${PROJECT_DIR}/index\.js" and assigns only the first PID to _OLD_PID (via head -1) which leaves other stale processes running; change the logic in start.sh to collect all matching PIDs (remove the head -1) and iterate over them, logging and killing each PID (use the existing echo/kil l pattern for each PID and preserve the 2>/dev/null || true behavior), ensuring the loop targets every PID returned by pgrep -f "node.*${PROJECT_DIR}/index\.js" rather than just the first. ``` </details> </blockquote></details> <details> <summary>docs/features/12-group-linking.md-26-35 (1)</summary><blockquote> `26-35`: _⚠️ Potential issue_ | _🟡 Minor_ **Add a language identifier to the fenced flow block.** The code fence starting on Line 26 is missing a language tag (MD040). <details> <summary>Suggested fix</summary> ```diff -``` +```text settings_group_linking_tools → groupLinkingToolsKeyboard ├── group_link_start → pendingAction: await_register_chat_forward │ └── Admin forwards any message from target group/channel │ └── Chat ID extracted → group added to linkedGroups[] ├── group_link_view → list all linked groups with IDs ├── group_link_remove_menu → show removal picker │ └── group_link_remove_{chatId} → remove from linkedGroups[] └── group_link_test_permissions → check bot perms in each group ``` ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/features/12-group-linking.mdaround lines 26 - 35, The fenced flow
block starting at the diagram lacks a language tag; update the opening fence to
include a language identifier (for example use ```text) so the block becomes a
proper fenced code block. Edit the block that documents
settings_group_linking_tools → groupLinkingToolsKeyboard (the diagram containing
group_link_start, group_link_view, group_link_remove_menu,
group_link_test_permissions) and change the opening triple backticks to include
the language identifier while leaving the block contents unchanged.</details> </blockquote></details> </blockquote></details> <details> <summary>🧹 Nitpick comments (7)</summary><blockquote> <details> <summary>docs/TODO_FUNCTIONALITY_UPGRADE.md (1)</summary><blockquote> `3-5`: **Use full timestamps for log maintenance metadata.** Since this document is maintained session-by-session, using an ISO timestamp (date+time+zone) for `Last updated` will make the operation log auditable and less ambiguous. <details> <summary>📝 Suggested tweak</summary> ```diff -> **Last updated:** 2026-02-28 +> **Last updated:** 2026-02-28T23:59:59Z ``` </details> As per coding guidelines: "`**/*.md`: Log significant operations in markdown format with timestamps and descriptive messages". <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@docs/TODO_FUNCTIONALITY_UPGRADE.md` around lines 3 - 5, Update the "Last updated" metadata entry in the document to use a full ISO 8601 timestamp (date, time, and timezone) instead of just the date; locate the "Last updated:" line (the metadata key) and replace the existing value with an ISO timestamp like YYYY-MM-DDTHH:MM:SS±HH:MM to make the log auditable and unambiguous per the markdown logging guideline. ``` </details> </blockquote></details> <details> <summary>docs/features/03-admin-menu.md (1)</summary><blockquote> `106-111`: **Consider documenting stale-menu recovery dependencies here as well.** This section would be more complete if it also referenced startup stale-ID cleanup (`clearStaleMenuIds`) and the menu timestamp fields used for stale detection (`mainMenuSentAt`, `adminMenuSentAt`). Based on learnings: Add mainMenuSentAt and adminMenuSentAt timestamps to user schema for stale menu detection; implement clearStaleMenuIds() at bot startup to null out transient and 24h-stale menu IDs. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@docs/features/03-admin-menu.md` around lines 106 - 111, Add documentation and references for stale-menu recovery by listing the startup cleanup function clearStaleMenuIds and the user schema timestamp fields mainMenuSentAt and adminMenuSentAt under the Dependencies section; describe that mainMenuSentAt/adminMenuSentAt are used for stale detection and that clearStaleMenuIds is run at bot startup to null out transient menu IDs and any IDs older than 24 hours so stale menus are recoverable. ``` </details> </blockquote></details> <details> <summary>docs/features/02-user-menu.md (1)</summary><blockquote> `97-102`: **Consider adding stale-menu lifecycle dependencies in this section.** Including startup stale cleanup (`clearStaleMenuIds`) and schema timestamps (`mainMenuSentAt`, `adminMenuSentAt`) would make this menu doc more complete. Based on learnings: Add mainMenuSentAt and adminMenuSentAt timestamps to user schema for stale menu detection; implement clearStaleMenuIds() at bot startup to null out transient and 24h-stale menu IDs. <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@docs/features/02-user-menu.md` around lines 97 - 102, Add startup stale-menu lifecycle details: document the clearStaleMenuIds() dependency alongside sendPersistentUserMenu(ctx, user) and clearOldMenus(ctx, user), and state that the user schema must include mainMenuSentAt and adminMenuSentAt timestamp fields for stale detection; describe clearStaleMenuIds() behavior (runs at bot startup, nulls transient menu IDs and clears IDs older than 24h) and reference where sendPersistentUserMenu and clearOldMenus use those timestamps for deciding whether to resend or delete menus. ``` </details> </blockquote></details> <details> <summary>todolist.md (1)</summary><blockquote> `237-242`: **Clarify the “Deferred to v3.1” section label.** At Line 237, the heading says deferred, but Line 238, Line 239, and Line 241 are already marked complete. Consider renaming to avoid status ambiguity (for example, “v3.1 status / carried items”). <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@todolist.md` around lines 237 - 242, The "Deferred to v3.1" heading is ambiguous because several checklist items beneath it are already completed; update the heading text to something like "v3.1 status / carried items" or "v3.1 (carried/partial completed items)" to reflect mixed statuses, and ensure any referencing documentation uses the new heading; search for the exact string "Deferred to v3.1" in this file and replace it, and verify related symbols mentioned nearby (onboardingProgressBar(), completionCardShown, adminSystemToolsKeyboard(), admin_sys_group_linking, userStore) remain unchanged. ``` </details> </blockquote></details> <details> <summary>docs/features/01-onboarding.md (1)</summary><blockquote> `1-98`: **Add timestamped operation entries for onboarding doc updates.** Please add a brief dated operations/change-log section (timestamp + action summary) to match repo markdown logging expectations. As per coding guidelines: "`**/*.md`: Log significant operations in markdown format with timestamps and descriptive messages". <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@docs/features/01-onboarding.md` around lines 1 - 98, Add a small "Operations / Change Log" section to the onboarding doc (the Feature: Onboarding & Account Setup with ID "onboarding") containing timestamped entries tracking updates; append a heading like "## Change Log / Operations" at the end and add bullet lines using ISO 8601 dates and brief action summaries (e.g. "2026-03-01 — Added change-log section", "2026-02-15 — Updated referral handling notes"), ensuring each entry is a single-line markdown bullet and follows the repo convention for "*.md" logs. ``` </details> </blockquote></details> <details> <summary>docs/features/07-tooltips.md (1)</summary><blockquote> `1-188`: **Add a timestamped “operations log” section for this feature doc.** This file captures significant behavior and admin workflows but does not log operations with timestamps. Add a short change-log block (date + concise action entries) near the end for ongoing updates. As per coding guidelines: "`**/*.md`: Log significant operations in markdown format with timestamps and descriptive messages". <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@docs/features/07-tooltips.md` around lines 1 - 188, Add an "Operations Log" markdown section near the end of the doc that records timestamped changelog entries (ISO date + short description) for significant operational changes; include at least one starter entry (date + "Initial operations log added"), and instruct maintainers to append entries for changes affecting tipsTimer, tipsStore, postTipToConfiguredTarget, parseTipText, batch imports or settings changes; format entries consistently (e.g., "2026-03-01: Added operations log — <brief note>") and ensure the section follows the project's markdown logging guideline for "**/*.md" files. ``` </details> </blockquote></details> <details> <summary>index.js (1)</summary><blockquote> `10973-11004`: **Batch import should validate tooltip button syntax like add/edit flows.** This path currently skips `parseTooltipButtons` validation, so malformed button definitions get imported and degrade silently at send-time. <details> <summary>♻️ Suggested hardening</summary> ```diff const added = []; for (const item of batch) { if (!item || typeof item.text !== 'string' || !item.text.trim()) continue; - const newTip = { id: tipsStore.nextTipId, text: item.text.trim(), enabled: item.enabled !== false }; + const textValue = item.text.trim(); + const parsedButtons = parseTooltipButtons(textValue); + if (parsedButtons.error) continue; + const newTip = { id: tipsStore.nextTipId, text: textValue, enabled: item.enabled !== false }; tipsStore.tips.push(newTip); tipsStore.nextTipId += 1; added.push(newTip.id); } ``` </details> <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@index.js` around lines 10973 - 11004, The batch import path for action.type 'await_tip_import_batch' currently skips the same tooltip button validation used by add/edit flows; update the loop that builds newTip to call parseTooltipButtons(item.buttons) (or the same validator used by the add/edit handlers) and skip or reject items whose parse throws or returns invalid results, ensuring newTip.buttons uses the parsed/normalized value; preserve the existing behavior of skipping items with empty text, collect and report invalid item indices (or a count) back to the user, then only call persistRuntimeState() and saveHelpfulMessages() if at least one valid tip was added so imported tooltips have the same button syntax guarantees as tools created via add/edit. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@deploy.sh:
- Around line 243-249: The current shutdown loop kills every PID in _BLOCKING
regardless of what binary they're running; limit signals to only the Runewager
process by checking the process command line (e.g., contains "index.js" or the
project name "Runewager") before sending SIGTERM/SIGKILL. Modify the loops that
iterate _BLOCKING to inspect each PID’s cmdline (via /proc//cmdline or
pgrep -f/pgrep pattern matching) and only call kill on PIDs whose command
matches "index.js" or "Runewager"; keep the two-stage SIGTERM then SIGKILL flow
and still swallow errors (|| true) for missing/vanished PIDs.In
@dev-run.sh:
- Around line 44-49: The current cleanup loop unconditionally kills every PID in
_DEV_PIDS, which may terminate unrelated services; update the loops that iterate
over _DEV_PIDS to first verify each PID belongs to our dev server before sending
signals: for each _p in $_DEV_PIDS check the process command line (e.g., via
/proc/$_p/cmdline orps -p $_p -o args=) and only issue kill and kill -9 when
the args match the expected Runewager dev server invocation (e.g., contain
"node" and the repo's index.js or repo-specific path); leave non-matching PIDs
untouched. Ensure the checks are applied in both the graceful kill loop and the
subsequent SIGKILL loop so DEV_PORT and _DEV_PIDS logic only targets
Runewager-owned processes.- Around line 24-26: The script dev-run.sh currently runs git -C "$ROOT_DIR"
reset --hard origin/main unconditionally; change it so the hard reset is only
executed after explicit opt-in: add flag parsing for a new option (e.g.,
--force-reset or env var FORCE_RESET) or an interactive confirmation prompt, and
wrap the git -C "$ROOT_DIR" reset --hard origin/main call so it runs only when
the flag is present or the user confirms; if not confirmed, skip the reset and
proceed (or exit), and ensure the warning message remains clear (e.g., keep the
existing echo) so destructive operations are not performed silently.In
@generate_tooltips.sh:
- Around line 37-43: The current validation of RUNEWAGER_APP fails if APP_DIR
(or RUNEWAGER_DIR) is relative, causing fallback placeholder JSON; fix by
normalizing APP_DIR to an absolute path before setting RUNEWAGER_APP and
performing the strict regex check: resolve APP_DIR (e.g., with realpath/readlink
-f or equivalent) and then build RUNEWAGER_APP="$APP_DIR/index.js", so the
validator in the node snippet (appFile, the /^/[^\0]+.js$/ check and
RUNEWAGER_APP usage) receives an absolute path and avoids the destructive
fallback.In
@index.js:
- Around line 272-277: The group-guard is suppressing the /join handler because
'join' is missing from GROUP_PASSTHROUGH_COMMANDS; update the Set declared as
GROUP_PASSTHROUGH_COMMANDS to include the string 'join' so the guard will
passthrough and allow bot.command('join', ...) to run in group contexts.- Around line 5791-5812: Move the assignment to
user.onboarding.completionCardShown so it only happens after ctx.reply succeeds:
call await ctx.reply(...) (using the existing rwName, Markup.inlineKeyboard and
'to_main_menu' callback) inside a try block and set
user.onboarding.completionCardShown = true after the await returns (or in the
successful path), and catch/log errors without mutating completionCardShown;
ensure the Markdown escaping of rwName remains unchanged and preserve the
current inline keyboard structure.In
@prod-run.sh:
- Around line 387-390: The current prestart port cleanup may kill any process on
PORT_PRESTART; update free_port_if_conflicted (and its callers like the block
using is_port_listening, PORT_PRESTART and optional PID) to only act on PIDs
that are owned by this Runewager project by checking the process command/args
(e.g., match "node" with the project-specific entry point like "index.js" or
other unique path fragment) before sending signals; if a PID arg is supplied use
it but still validate its command, and if no matching Runewager process is
found, skip killing and log a safe message instead.In
@RUNEWAGER_FUNCTIONALITY_MAP.md:
- Line 405: Update the conflicting guidance so RUNEWAGER_FUNCTIONALITY_MAP.md
remains authoritative: replace or revise the sentence referencing
docs/INDEX.mdat the current Line 405 so it instructs readers to consult
RUNEWAGER_FUNCTIONALITY_MAP.md first (the "map-first" contract referenced around
Lines 396-399), then thedocs/INDEX.mdand per-feature.mdfiles as
secondary resources; ensure the wording explicitly enforces "map-first" startup
and verification steps and remove any language that mandatesdocs/INDEX.mdas
the initial source of truth.In
@scripts/rollback.sh:
- Around line 93-97: The tooltip refresh uses
TOOLTIP_SCRIPT="${PROJECT_DIR}/generate_tooltips.sh" which points to the wrong
location; update TOOLTIP_SCRIPT to reference the actual script under scripts
(e.g. "${PROJECT_DIR}/scripts/generate_tooltips.sh") so the -x check and
subsequent bash invocation in the rollback flow use the correct file (affecting
TOOLTIP_SCRIPT, PROJECT_DIR and the generate_tooltips.sh invocation).- Around line 105-116: The current rollback uses a stale snapshot in
_RB_BLOCKING and then sleeps before sending SIGKILL, risking PID reuse; update
the logic in the block that defines and uses _RB_BLOCKING (the lsof/fuser
resolution and the two kill loops) to re-resolve live PIDs or gate each
candidate with a kill -0 check before sending SIGKILL: after the sleep, either
re-run the lsof/fuser resolution into a fresh variable or loop over the original
list but call kill -0 "$pid" (or verify the process still matches the original
port) and only then send kill -9; ensure each destructive command checks exit
codes and falls back safely (log and skip) rather than blindly killing.In
@start.sh:
- Around line 71-76: The current cleanup kills every PID in _PIDS regardless of
ownership; change it to only target Runewager-owned processes by filtering _PIDS
to those whose command line contains "${PROJECT_DIR}/index.js" before sending
signals. Implement: build a filtered list (e.g., OWN_PIDS) by checking each
PID's cmdline (via /proc//cmdline or pgrep -f with the pattern
"${PROJECT_DIR}/index.js"), then iterate over OWN_PIDS for the SIGTERM and
SIGKILL loops instead of _PIDS; keep the existing sleep/delay logic and ensure
non-matching PIDs are skipped safely.- Around line 47-49: The script currently always runs git reset --hard
origin/main (using PROJECT_DIR) which can erase local work; change it to require
an explicit opt-in: wrap the git fetch/reset block in a conditional that checks
an environment variable (e.g., FORCE_RESET=true) or, if running interactively,
prompt the user with read -p to confirm before running git -C "$PROJECT_DIR"
reset --hard origin/main; if not confirmed or the env flag is not set, skip the
hard reset and fall back to pulling or using the local copy and keep the
existing echo "⚠️ git pull failed — starting with local copy" path. Ensure
CI/non-interactive runs can opt in by exporting the env flag and preserve
PROJECT_DIR usage.In
@test/smoke.test.js:
- Around line 226-227: Update the catch-all regex detection and tests so
isCatchAllRegexPattern recognizes non-greedy and non-capturing variants: expand
the predicate in isCatchAllRegexPattern to match (.+), (.), (?:.) and the
non-greedy anchored form ^(?:.?)$ (in addition to existing variants), and add
corresponding assertions in the smoke.test.js test matrix (include the
'^(?:.?)$' entry and variants next to existing '(.|\n)', '(.|\n)+', '(.|)',
'(.|)+' cases) so the function classifies these patterns as catch-all.
Minor comments:
In@docs/features/01-onboarding.md:
- Line 90: The documentation uses inconsistent terminology: replace the
pending_actionmention with the project's canonical keypendingAction(and
referenceuser.pendingActionwhere applicable) so wording matches code; update
the sentence that currently reads "pending_actiontimeout (15 min) applies to
all text input steps." to usependingAction(or explicitly
user.pendingAction) and indicate the 15-minute timeout applies to that key.In
@docs/features/02-user-menu.md:
- Around line 32-33: The Promos table row has its cells swapped: move the
descriptive text "opens promo list" into the Purpose column and place the
callback identifiersmenu_claim_bonusorpmenu_claim_bonusinto the Callback
Data column so the row matches the other entries (i.e., change the Promos row
from "| 🎁 Promos | (opens promo list) |menu_claim_bonusor
pmenu_claim_bonus|" to have Purpose = "opens promo list" and Callback Data =
"menu_claim_bonusorpmenu_claim_bonus").In
@docs/features/04-giveaway.md:
- Around line 30-54: The fenced wizard flow block containing
pamenu_start_giveaway and the gwiz_* step keys lacks a language tag; update the
opening fence fromtotext (or another appropriate language) so
markdownlint and tooling recognize it—e.g., locate the block with
pamenu_start_giveaway and
gwiz_title_skip/gwiz_await_title/gwizScKeyboard/gwiz_winners_* etc. and add the
language tag to the code fence.- Around line 22-23: Update the table row that lists "Button: Join (in group)"
with the identifiergwiz_start_hereto mark its role as User (not Admin) so it
matches the Join Flow; similarly verify the/joinrow remains marked User and
ensure both entries consistently reflect the user-facing action.- Around line 127-128: Update the inaccurate line-number references in the docs
for the two giveaway helper functions: change the documented line number for
buildGiveawayAnnouncementText from 13795 to 13765, and change the documented
line number for buildGiveawayAnnouncementKeyboard from 12561 to 12531 so the
docs point to the correct definition locations.In
@docs/features/05-bonus-30sc.md:
- Around line 29-41: Fenced flow blocks in docs/features/05-bonus-30sc.md are
missing a language label which triggers markdownlint MD040; update each
triple-backtick fence that contains the flow diagrams (e.g., the block
containing pmenu_claim_bonus → bonus options menu and the block with
/wager30_admin → admin bonus manager, as noted also at lines 47-57) to include a
language identifier like "text" (e.g., replacewithtext) so the blocks
are properly labeled while keeping the diagram content unchanged.In
@docs/features/06-promos.md:
- Line 94: The docs table has a callback ID mismatch: the promo entry uses
pmenu_claim_bonus elsewhere but the row lists menu_claim_bonus; update the table
row to use the correct callback ID (pmenu_claim_bonus) so it matches the
implementation and other documentation for the claim-bonus flow, ensuring
consistency between the promo wiring and the docs.In
@docs/features/07-tooltips.md:
- Around line 47-92: The fenced flow blocks for tips_cmd_add, tips_cmd_edit,
tips_cmd_remove, and tips_cmd_import_batch lack a language identifier,
triggering markdown linting errors; update each triple-backtick fence that wraps
the flows for symbols tips_cmd_add, tips_cmd_edit →
tipSelectKeyboard('tip_edit_select'), tips_cmd_remove →
tipSelectKeyboard('tip_remove'), and tips_cmd_import_batch to include a language
token (e.g., ```text) at the opening fence so the blocks render and lint
correctly.In
@docs/features/08-referral.md:
- Around line 28-40: The fenced flow block in docs/features/08-referral.md lacks
a language tag and triggers markdownlint MD040; update the opening code fence to
include a language (e.g., changetotext) for the flow containing
pmenu_referral, ref_menu_code, ref_menu_share, ref_menu_how and the
applyOnboardingReferralCode flow so the linter recognizes it as a code block
with an explicit language.In
@docs/features/09-sshv.md:
- Around line 26-34: The code fence blocks showing the SSHV flow lack language
identifiers; update both triple-backtick fences that contain the flow diagram
(the block that begins with "/sshv → renderSshvConsole(ctx) ..." and the other
similar block referenced in the comment) to use the text language identifier by
changingtotext so tools/markdownlint render them correctly; ensure this
is applied to the flow involving renderSshvConsole, sshv_run_prompt,
sshv_confirm_run and sshv_cancel_run.In
@docs/features/10-deploy-ops.md:
- Around line 32-43: The markdown has unlabeled fenced code blocks that trigger
MD040; add a language identifier (e.g., text) to each fenced code block
containing the deployment steps starting with "/deploy [source]" and the test
summary block beginning with "TestAll complete — X passed, Y warnings, Z
failures." Update those fences to use ```text so both blocks (including the
other occurrence at lines referenced as 70-72) are labeled and satisfy
markdownlint.In
@docs/features/11-user-lookup.md:
- Around line 31-50: The fenced code blocks for the command outlines (/whois,
/bonusstatus, /refreshuser) lack language identifiers and trigger markdownlint
MD040; update each triple-backtick fence to include a language (e.g., useexamples so the linter stops complaining and the blocks render consistently. - Line 19: The table cell containing `/whois <id|username>` is breaking the Markdown table because the pipe is treated as a column delimiter; update that cell by escaping the pipe or using the HTML entity so it is treated as literal (e.g., change the `/whois <id|username>` occurrence to use an escaped pipe or `|` such as `/whois <id\|username>` or `/whois <id|username>`), ensuring the table structure remains intact. In `@docs/features/12-group-linking.md`: - Around line 26-35: The fenced flow block starting at the diagram lacks a language tag; update the opening fence to include a language identifier (for example use ```text) so the block becomes a proper fenced code block. Edit the block that documents settings_group_linking_tools → groupLinkingToolsKeyboard (the diagram containing group_link_start, group_link_view, group_link_remove_menu, group_link_test_permissions) and change the opening triple backticks to include the language identifier while leaving the block contents unchanged. In `@docs/features/13-bug-reports.md`: - Around line 29-36: The fenced code blocks in the flow documentation lack a declared language, which trips markdown linting and editor tooling; update both blocks by adding a language marker (e.g., `text`) to the opening fences for the flow that includes help_open_bugreport and menu_bugreport, and the block that lists pamenu_bug_reports, admin_cmd_viewbugs, admin_cmd_resolvebug_prompt, and admin_cmd_exportbugs so the fences read like ```text ... ``` to satisfy linters and improve editor rendering. - Around line 54-61: The table header has three columns but the rows contain four cells (an extra action/effect column), causing Markdown rendering issues; update the table in docs/features/13-bug-reports.md so the header explicitly includes the fourth column (e.g., add an "Action/Effect" or "Action" column) to match rows for callbacks like `help_open_bugreport`, `menu_bugreport`, `pamenu_bug_reports`, `admin_cmd_viewbugs`, `admin_cmd_resolvebug_prompt`, and `admin_cmd_exportbugs`, ensuring each row cell aligns with the header and renders correctly. In `@docs/features/14-announcements.md`: - Around line 26-36: The markdown code fence for the broadcast flow diagram doesn't include a language tag; update the opening triple backticks to include a language such as "text" (e.g., ```text) so tooling/linting recognizes it; apply this change to the fence that contains the admin_cmd_announce_start flow (references: admin_cmd_announce_start, broadcastFailedUsers[], /broadcast_retry, /broadcast_failed). In `@docs/features/15-misc-commands.md`: - Around line 53-59: The fenced code block for the walkthrough commands is missing a language tag causing markdownlint MD040; update the block header from ``` to a language-tagged fence such as ```text (i.e., change the opening fence that contains the `/walkthrough → interactive onboarding walkthrough` block to include a language like `text`) so the code block is language-specified. In `@docs/INDEX.md`: - Line 3: Rephrase the sentence in docs/INDEX.md that currently calls this index "the primary navigation index for all Claude sessions" to remove source-of-truth language and instead reference RUNEWAGER_FUNCTIONALITY_MAP.md as authoritative; update the text to say this index is a supplemental navigation aid and add an explicit link to RUNEWAGER_FUNCTIONALITY_MAP.md (the authoritative map) so readers are directed to consult RUNEWAGER_FUNCTIONALITY_MAP.md before and after making changes. In `@start.sh`: - Around line 82-86: The current startup uses pgrep -f "node.*${PROJECT_DIR}/index\.js" and assigns only the first PID to _OLD_PID (via head -1) which leaves other stale processes running; change the logic in start.sh to collect all matching PIDs (remove the head -1) and iterate over them, logging and killing each PID (use the existing echo/kil l pattern for each PID and preserve the 2>/dev/null || true behavior), ensuring the loop targets every PID returned by pgrep -f "node.*${PROJECT_DIR}/index\.js" rather than just the first. --- Nitpick comments: In `@docs/features/01-onboarding.md`: - Around line 1-98: Add a small "Operations / Change Log" section to the onboarding doc (the Feature: Onboarding & Account Setup with ID "onboarding") containing timestamped entries tracking updates; append a heading like "## Change Log / Operations" at the end and add bullet lines using ISO 8601 dates and brief action summaries (e.g. "2026-03-01 — Added change-log section", "2026-02-15 — Updated referral handling notes"), ensuring each entry is a single-line markdown bullet and follows the repo convention for "*.md" logs. In `@docs/features/02-user-menu.md`: - Around line 97-102: Add startup stale-menu lifecycle details: document the clearStaleMenuIds() dependency alongside sendPersistentUserMenu(ctx, user) and clearOldMenus(ctx, user), and state that the user schema must include mainMenuSentAt and adminMenuSentAt timestamp fields for stale detection; describe clearStaleMenuIds() behavior (runs at bot startup, nulls transient menu IDs and clears IDs older than 24h) and reference where sendPersistentUserMenu and clearOldMenus use those timestamps for deciding whether to resend or delete menus. In `@docs/features/03-admin-menu.md`: - Around line 106-111: Add documentation and references for stale-menu recovery by listing the startup cleanup function clearStaleMenuIds and the user schema timestamp fields mainMenuSentAt and adminMenuSentAt under the Dependencies section; describe that mainMenuSentAt/adminMenuSentAt are used for stale detection and that clearStaleMenuIds is run at bot startup to null out transient menu IDs and any IDs older than 24 hours so stale menus are recoverable. In `@docs/features/07-tooltips.md`: - Around line 1-188: Add an "Operations Log" markdown section near the end of the doc that records timestamped changelog entries (ISO date + short description) for significant operational changes; include at least one starter entry (date + "Initial operations log added"), and instruct maintainers to append entries for changes affecting tipsTimer, tipsStore, postTipToConfiguredTarget, parseTipText, batch imports or settings changes; format entries consistently (e.g., "2026-03-01: Added operations log — <brief note>") and ensure the section follows the project's markdown logging guideline for "**/*.md" files. In `@docs/TODO_FUNCTIONALITY_UPGRADE.md`: - Around line 3-5: Update the "Last updated" metadata entry in the document to use a full ISO 8601 timestamp (date, time, and timezone) instead of just the date; locate the "Last updated:" line (the metadata key) and replace the existing value with an ISO timestamp like YYYY-MM-DDTHH:MM:SS±HH:MM to make the log auditable and unambiguous per the markdown logging guideline. In `@index.js`: - Around line 10973-11004: The batch import path for action.type 'await_tip_import_batch' currently skips the same tooltip button validation used by add/edit flows; update the loop that builds newTip to call parseTooltipButtons(item.buttons) (or the same validator used by the add/edit handlers) and skip or reject items whose parse throws or returns invalid results, ensuring newTip.buttons uses the parsed/normalized value; preserve the existing behavior of skipping items with empty text, collect and report invalid item indices (or a count) back to the user, then only call persistRuntimeState() and saveHelpfulMessages() if at least one valid tip was added so imported tooltips have the same button syntax guarantees as tools created via add/edit. In `@todolist.md`: - Around line 237-242: The "Deferred to v3.1" heading is ambiguous because several checklist items beneath it are already completed; update the heading text to something like "v3.1 status / carried items" or "v3.1 (carried/partial completed items)" to reflect mixed statuses, and ensure any referencing documentation uses the new heading; search for the exact string "Deferred to v3.1" in this file and replace it, and verify related symbols mentioned nearby (onboardingProgressBar(), completionCardShown, adminSystemToolsKeyboard(), admin_sys_group_linking, userStore) remain unchanged.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
RUNEWAGER_FUNCTIONALITY_MAP.mdadd_tooltip.shdeploy.shdev-run.shdocs/INDEX.mddocs/TODO_FUNCTIONALITY_UPGRADE.mddocs/features/01-onboarding.mddocs/features/02-user-menu.mddocs/features/03-admin-menu.mddocs/features/04-giveaway.mddocs/features/05-bonus-30sc.mddocs/features/06-promos.mddocs/features/07-tooltips.mddocs/features/08-referral.mddocs/features/09-sshv.mddocs/features/10-deploy-ops.mddocs/features/11-user-lookup.mddocs/features/12-group-linking.mddocs/features/13-bug-reports.mddocs/features/14-announcements.mddocs/features/15-misc-commands.mdgenerate_tooltips.shindex.jsprod-run.shscripts/rollback.shstart.shtest/smoke.test.jstodolist.md
Comment on lines
+243
to
+249
| if [[ -n "$_BLOCKING" ]]; then | ||
| say "Port $DEPLOY_PORT blocked — sending SIGTERM then SIGKILL…" | ||
| for _pid in $_BLOCKING; do kill "$_pid" 2>/dev/null || true; done | ||
| sleep 2 | ||
| for _pid in $_BLOCKING; do | ||
| kill -9 "$_pid" 2>/dev/null || true | ||
| done |
There was a problem hiding this comment.
Scope port blocker termination to Runewager process only.
Lines 245-248 kill every PID bound to DEPLOY_PORT, which can stop unrelated services on the host. Filter by command line (project index.js) before signaling.
Based on learnings: Applies to **/{deploy,ci}.{yml,yaml,sh} : Scope pkill to specific processes (e.g., 'Runewager/index.js') to avoid killing unrelated Node processes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@deploy.sh` around lines 243 - 249, The current shutdown loop kills every PID
in _BLOCKING regardless of what binary they're running; limit signals to only
the Runewager process by checking the process command line (e.g., contains
"index.js" or the project name "Runewager") before sending SIGTERM/SIGKILL.
Modify the loops that iterate _BLOCKING to inspect each PID’s cmdline (via
/proc/<pid>/cmdline or pgrep -f/pgrep pattern matching) and only call kill on
PIDs whose command matches "index.js" or "Runewager"; keep the two-stage SIGTERM
then SIGKILL flow and still swallow errors (|| true) for missing/vanished PIDs.
Comment on lines
+272
to
+277
| const GROUP_PASSTHROUGH_COMMANDS = new Set([ | ||
| 'link', 'linkrunewager', // handled: group username inline confirm | ||
| 'giveaway', // handled: shows active giveaways for the group | ||
| 'start_giveaway', // handled: admin can start giveaway from group context | ||
| 'admin', // handled: brief DM dashboard link | ||
| ]); |
There was a problem hiding this comment.
/join is unintentionally blocked in groups by the new guard.
/join has explicit group logic, but it is not included in GROUP_PASSTHROUGH_COMMANDS, so Line 302 suppresses it before bot.command('join', ...) can run.
🐛 Proposed fix
const GROUP_PASSTHROUGH_COMMANDS = new Set([
'link', 'linkrunewager', // handled: group username inline confirm
'giveaway', // handled: shows active giveaways for the group
+ 'join', // handled: joins active giveaway from group context
'start_giveaway', // handled: admin can start giveaway from group context
'admin', // handled: brief DM dashboard link
]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@index.js` around lines 272 - 277, The group-guard is suppressing the /join
handler because 'join' is missing from GROUP_PASSTHROUGH_COMMANDS; update the
Set declared as GROUP_PASSTHROUGH_COMMANDS to include the string 'join' so the
guard will passthrough and allow bot.command('join', ...) to run in group
contexts.
Comment on lines
+93
to
+97
| TOOLTIP_SCRIPT="${PROJECT_DIR}/generate_tooltips.sh" | ||
| if [[ -x "$TOOLTIP_SCRIPT" ]]; then | ||
| say "Refreshing tooltips from rolled-back code..." | ||
| RUNEWAGER_DIR="$PROJECT_DIR" bash "$TOOLTIP_SCRIPT" >/dev/null 2>&1 \ | ||
| || warn "generate_tooltips.sh failed (non-fatal)" |
There was a problem hiding this comment.
Tooltip refresh points to the wrong script path.
This targets ${PROJECT_DIR}/generate_tooltips.sh, but the generator is in scripts/; the refresh step may be skipped unintentionally.
🛠️ Proposed fix
-TOOLTIP_SCRIPT="${PROJECT_DIR}/generate_tooltips.sh"
+TOOLTIP_SCRIPT="${PROJECT_DIR}/scripts/generate_tooltips.sh"
if [[ -x "$TOOLTIP_SCRIPT" ]]; then
say "Refreshing tooltips from rolled-back code..."
RUNEWAGER_DIR="$PROJECT_DIR" bash "$TOOLTIP_SCRIPT" >/dev/null 2>&1 \
|| warn "generate_tooltips.sh failed (non-fatal)"
fi📝 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.
Suggested change
| TOOLTIP_SCRIPT="${PROJECT_DIR}/generate_tooltips.sh" | |
| if [[ -x "$TOOLTIP_SCRIPT" ]]; then | |
| say "Refreshing tooltips from rolled-back code..." | |
| RUNEWAGER_DIR="$PROJECT_DIR" bash "$TOOLTIP_SCRIPT" >/dev/null 2>&1 \ | |
| || warn "generate_tooltips.sh failed (non-fatal)" | |
| TOOLTIP_SCRIPT="${PROJECT_DIR}/scripts/generate_tooltips.sh" | |
| if [[ -x "$TOOLTIP_SCRIPT" ]]; then | |
| say "Refreshing tooltips from rolled-back code..." | |
| RUNEWAGER_DIR="$PROJECT_DIR" bash "$TOOLTIP_SCRIPT" >/dev/null 2>&1 \ | |
| || warn "generate_tooltips.sh failed (non-fatal)" | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/rollback.sh` around lines 93 - 97, The tooltip refresh uses
TOOLTIP_SCRIPT="${PROJECT_DIR}/generate_tooltips.sh" which points to the wrong
location; update TOOLTIP_SCRIPT to reference the actual script under scripts
(e.g. "${PROJECT_DIR}/scripts/generate_tooltips.sh") so the -x check and
subsequent bash invocation in the rollback flow use the correct file (affecting
TOOLTIP_SCRIPT, PROJECT_DIR and the generate_tooltips.sh invocation).
Comment on lines
+71
to
+76
| if [[ -n "${_PIDS:-}" ]]; then | ||
| echo "⚠️ Port $BOT_PORT blocked — sending SIGTERM then SIGKILL..." | ||
| for _pid in $_PIDS; do kill "$_pid" 2>/dev/null || true; done | ||
| sleep 2 | ||
| for _pid in $_PIDS; do kill -9 "$_pid" 2>/dev/null || true; done | ||
| sleep 1 |
There was a problem hiding this comment.
Limit port cleanup to Runewager-owned PIDs.
Current logic kills every process listening on BOT_PORT. That is risky on shared hosts/dev machines. Filter by process args containing ${PROJECT_DIR}/index.js.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@start.sh` around lines 71 - 76, The current cleanup kills every PID in _PIDS
regardless of ownership; change it to only target Runewager-owned processes by
filtering _PIDS to those whose command line contains "${PROJECT_DIR}/index.js"
before sending signals. Implement: build a filtered list (e.g., OWN_PIDS) by
checking each PID's cmdline (via /proc/<pid>/cmdline or pgrep -f with the
pattern "${PROJECT_DIR}/index.js"), then iterate over OWN_PIDS for the SIGTERM
and SIGKILL loops instead of _PIDS; keep the existing sleep/delay logic and
ensure non-matching PIDs are skipped safely.
Comment on lines
+226
to
+227
| '(.|\n)*', '(.|\n)+', // literal newline form (pre-strip) | ||
| '(.|)*', '(.|)+', // post-whitespace-strip form of the above |
There was a problem hiding this comment.
Catch-all detection is still missing the non-greedy ^(?:.*?)$ form.
isCatchAllRegexPattern doesn’t currently classify that variant as catch-all, and the test matrix doesn’t assert it.
🛠️ Proposed fix
const CATCH_ALL_CORES = new Set([
'.*', '.+', '(?:.*)', '(?:.+)',
+ '(?:.*?)',
'(.*)', '(.+)', '(.+)?',
'(.|\n)*', '(.|\n)+', // literal newline form (pre-strip)
'(.|)*', '(.|)+', // post-whitespace-strip form of the above
'(\\.|[\\s\\S])*',
]); const catchAllCases = [
'.*', '^.*$', '.+', '^.+$', '(.*)', '(.+)', '(?:.*)', '(?:.+)', '^(?:.*)$', '(.+)?',
+ '^(?:.*?)$',
'(.|\n)*', '(.|\n)+', '(\\.|[\\s\\S])*',
// Post-whitespace-strip forms (isCatchAllRegexPattern strips \s before comparing)
'(.|)*', '(.|)+',
];Based on learnings: "Applies to **/.test.js : Expand isCatchAllRegexPattern() to detect (.+), (.), (?:.), and ^(?:.?)$ patterns".
Also applies to: 449-454
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/smoke.test.js` around lines 226 - 227, Update the catch-all regex
detection and tests so isCatchAllRegexPattern recognizes non-greedy and
non-capturing variants: expand the predicate in isCatchAllRegexPattern to match
(.+), (.*), (?:.*) and the non-greedy anchored form ^(?:.*?)$ (in addition to
existing variants), and add corresponding assertions in the smoke.test.js test
matrix (include the '^(?:.*?)$' entry and variants next to existing '(.|\n)*',
'(.|\n)+', '(.|)*', '(.|)+' cases) so the function classifies these patterns as
catch-all.
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| # --------------------------------------------------------- | ||
| # 3c) Kill anything blocking the bot port before starting | ||
| # --------------------------------------------------------- | ||
| DEPLOY_PORT="$(grep -E '^PORT=' "$PROJECT_DIR/.env" 2>/dev/null | head -1 | cut -d= -f2 | cut -d'#' -f1 | tr -d '"' | tr -d "'" | tr -d ' ' | tr -d $'\r')" |
There was a problem hiding this comment.
Suggestion: The PORT parsing in the new "3c) Kill anything blocking the bot port" block runs a failing grep/cut pipeline under set -euo pipefail without a trailing || true, so if .env is missing or lacks a PORT= line the non-zero exit status will abort the entire deploy script instead of falling back to the default port. [logic error]
Severity Level: Major ⚠️
- ❌ Deploy script aborts if .env missing or PORT unset.
- ⚠️ Prevents new code deployment until configuration corrected.
Suggested change
| DEPLOY_PORT="$(grep -E '^PORT=' "$PROJECT_DIR/.env" 2>/dev/null | head -1 | cut -d= -f2 | cut -d'#' -f1 | tr -d '"' | tr -d "'" | tr -d ' ' | tr -d $'\r')" | |
| DEPLOY_PORT="$(grep -E '^PORT=' "$PROJECT_DIR/.env" 2>/dev/null | head -1 | cut -d= -f2 | cut -d'#' -f1 | tr -d '"' | tr -d "'" | tr -d ' ' | tr -d $'\r' || true)" |
Steps of Reproduction ✅
1. Ensure the VPS has the PR version of `deploy.sh` deployed at
`/var/www/html/Runewager/deploy.sh` (verified in `/workspace/Runewager/deploy.sh`, lines
24–38 set `set -euo pipefail` and `PROJECT_DIR`).
2. Remove or rename the `.env` file in the project directory so that `$PROJECT_DIR/.env`
does not exist, or create a `.env` file without any `PORT=` line (the deploy script does
not guard this access with `-f` at line 235, unlike BOT_TOKEN/ADMIN_IDS at lines 46–50
which explicitly check `-f "$PROJECT_DIR/.env"`).
3. Run the deploy script, e.g. `bash /var/www/html/Runewager/deploy.sh vps`; execution
proceeds through steps 0–3b successfully, then reaches the "3c) Kill anything blocking the
bot port before starting" block at lines 232–251.
4. At line 235, `DEPLOY_PORT="$(grep -E '^PORT=' "$PROJECT_DIR/.env" 2>/dev/null | head -1
| cut ... | tr -d $'\r')"` executes: since `.env` is missing (grep exit 2) or has no
`PORT=` (grep exit 1), the pipeline exits non‑zero; with `set -euo pipefail`, this
non‑zero status in a simple assignment causes the script to trigger the ERR trap and abort
the deploy before `DEPLOY_PORT` can fall back to the default
`DEPLOY_PORT="${DEPLOY_PORT:-3000}"` at line 236.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** deploy.sh
**Line:** 235:235
**Comment:**
*Logic Error: The PORT parsing in the new "3c) Kill anything blocking the bot port" block runs a failing grep/cut pipeline under `set -euo pipefail` without a trailing `|| true`, so if `.env` is missing or lacks a `PORT=` line the non-zero exit status will abort the entire deploy script instead of falling back to the default port.
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.| fi | ||
|
|
||
| # Kill anything blocking port 3000 (or PORT from .env) | ||
| DEV_PORT=$(grep -E '^PORT=' "$ROOT_DIR/.env" 2>/dev/null | head -1 | cut -d= -f2 | cut -d'#' -f1 | tr -d '"' | tr -d "'" | tr -d ' ' || true) |
There was a problem hiding this comment.
Suggestion: The dev-run PORT extraction does not strip a trailing carriage return from .env values, so on CRLF .env files the computed port (e.g. 3000\r) can cause lsof/fuser lookups to fail silently and leave the old process running on the intended port. [possible bug]
Severity Level: Major ⚠️
- ❌ dev-run.sh may not free configured port on Windows.
- ⚠️ Developers see EADDRINUSE errors starting local server.
Suggested change
| DEV_PORT=$(grep -E '^PORT=' "$ROOT_DIR/.env" 2>/dev/null | head -1 | cut -d= -f2 | cut -d'#' -f1 | tr -d '"' | tr -d "'" | tr -d ' ' || true) | |
| DEV_PORT=$(grep -E '^PORT=' "$ROOT_DIR/.env" 2>/dev/null | head -1 | cut -d= -f2 | cut -d'#' -f1 | tr -d '"' | tr -d "'" | tr -d ' ' | tr -d '\r' || true) |
Steps of Reproduction ✅
1. In `/workspace/Runewager/dev-run.sh:36-50`, note that `DEV_PORT` is parsed from
`$ROOT_DIR/.env` and only quotes and spaces are stripped (`tr -d '"'`, `tr -d "'"`, `tr -d
' '`), but no carriage return is removed.
2. On a Windows or WSL development machine, create `$ROOT_DIR/.env` with CRLF endings so
the line storing the port is `PORT=3000\r\n` (for example, by editing `.env` in a Windows
editor and saving with default CRLF line endings).
3. Start any long‑running process bound to TCP port 3000 (e.g. an old `node index.js`
instance from a previous run or `python -m http.server 3000`) so that `lsof`/`fuser`
should find a listener on port `3000`.
4. Run `./dev-run.sh` from the repo root: `DEV_PORT` becomes `3000\r`, so the subsequent
`lsof -ti :"$DEV_PORT"` or `fuser -n tcp "$DEV_PORT"` at `dev-run.sh:39-42` do not match
the existing listener; `_DEV_PIDS` remains empty, no processes are killed, and when `exec
node index.js` at `dev-run.sh:54` runs, Node fails to bind with `EADDRINUSE` because the
old process is still holding port 3000.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** dev-run.sh
**Line:** 37:37
**Comment:**
*Possible Bug: The dev-run PORT extraction does not strip a trailing carriage return from `.env` values, so on CRLF `.env` files the computed port (e.g. `3000\r`) can cause `lsof`/`fuser` lookups to fail silently and leave the old process running on the intended port.
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.| # ── Kill anything blocking port before restart ──────────────────────────────── | ||
| _RB_BLOCKING="" | ||
| if command -v lsof >/dev/null 2>&1; then | ||
| _RB_BLOCKING="$(lsof -ti :"$PORT" 2>/dev/null || true)" |
There was a problem hiding this comment.
Suggestion: The rollback script's new "Kill anything blocking port before restart" block uses lsof -ti :"$PORT" which returns all processes with any TCP connection on that port (both listeners and clients), so in environments where other applications connect to the bot's port this logic can kill unrelated client processes instead of only the service actually blocking the port; align the lsof invocation with the production runner's helper by restricting it to listening sockets only. [logic error]
Severity Level: Major ⚠️
- ⚠️ Rollback can kill unrelated local clients using bot port.
- ⚠️ Monitoring or health-check agents may terminate during rollback.
- ⚠️ Operational safety inconsistent with prod-run listener-only helper.
Suggested change
| _RB_BLOCKING="$(lsof -ti :"$PORT" 2>/dev/null || true)" | |
| # Restrict to TCP listeners on the port to avoid killing unrelated client processes | |
| _RB_BLOCKING="$(lsof -t -iTCP:"$PORT" -sTCP:LISTEN 2>/dev/null || true)" |
Steps of Reproduction ✅
1. Start the Runewager service normally so it listens on PORT (default 3000) as defined at
`scripts/rollback.sh:23-25` where `SERVICE_NAME="runewager"` and `PORT="${PORT:-3000}"`.
2. On the same host, open a long-lived HTTP client connection to the bot (for example, a
monitoring script or `curl` loop hitting `http://127.0.0.1:$PORT/health`) so that this
client process maintains an active TCP connection to port `$PORT`.
3. In another shell on the same host, execute the rollback script `scripts/rollback.sh` so
that control flows down to the "Kill anything blocking port before restart" block at
`scripts/rollback.sh:104-117`, which runs `lsof -ti :"$PORT"` to collect all PIDs with any
TCP usage of that port (both listeners and clients).
4. Observe that the script then sends SIGTERM and SIGKILL to every PID in `_RB_BLOCKING`
at `scripts/rollback.sh:111-115`, terminating not only the `runewager` listener but also
the unrelated long-lived client process connected to the port; this contrasts with the
production helper in `prod-run.sh:141` which already restricts `lsof` to `-iTCP:"${port}"
-sTCP:LISTEN`.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** scripts/rollback.sh
**Line:** 107:107
**Comment:**
*Logic Error: The rollback script's new "Kill anything blocking port before restart" block uses `lsof -ti :"$PORT"` which returns all processes with any TCP connection on that port (both listeners and clients), so in environments where other applications connect to the bot's port this logic can kill unrelated client processes instead of only the service actually blocking the port; align the `lsof` invocation with the production runner's helper by restricting it to listening sockets only.
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. |
… path validation - index.js: keep MarkdownV2 escaping for runewagerUsername (security fix) - generate_tooltips.sh: keep path validation (absolute .js, no traversal) - start.sh / dev-run.sh / scripts/rollback.sh: keep SIGTERM→SIGKILL two-step - start.sh: keep inline comment stripping in PORT parse - test/smoke.test.js: keep (.|)* and (.|)+ catch-all cases https://claude.ai/code/session_01VijmtzjN63WZJy5gYgJAKs
PR review comment fixes: - add_tooltip.sh: validate list is Array, filter non-finite IDs before computing maxId; throw fast on malformed tooltips.json - docs/12-group-linking.md: fix entry points table — admin System Tools uses admin_sys_group_linking, not admin_cat_system - docs/15-misc-commands.md: remove⚠️ walkthrough dead-end note (resolved) T-01 — Walkthrough system hardened: - sendWalkthroughStep(): clearOldMenus() before every step send - Back button disabled on step 1 (first step) - Next replaced with Finish button on last step (step 35) - walk_done on last step: sets started=false, fires walkthrough_completed analytics, answerCbQuery with success toast, returns to main menu - New doc: docs/features/16-walkthrough.md T-02 — clearOldMenus() added to 5 missing locations: - sendOnboardingReferralPrompt() - renderSshvConsole() - renderGroupLinkingTools() - tips_cmd_edit handler - tips_cmd_remove handler (sendGiveawayListPage already used replyMenu() — no change needed) T-03 — Tooltip view flow implemented: - tipsDashboardKeyboard(): "👁 View Tooltip" button added (row 4) - tips_cmd_view handler: clearOldMenus + tipSelectKeyboard('tip_view') - tip_view_{id} handler: preview card with Prev/Next tip navigation, Edit / Toggle / Delete buttons, Back to List, Admin Menu T-15 — Broadcast failure logging made reliable: - Removed 500-item cap from broadcastFailedUsers push and load - Every failure logged via adminLog('broadcast_failure', ...) to data/admin-events.log — no silent drops - /broadcast_failed: chunked output (30/chunk, up to 90 inline); overflow note points to log file - High-failure-rate warning: >20% failure rate sends⚠️ DM to ADMIN_IDS Merge conflicts resolved (PRs #112-114): - index.js: keep MarkdownV2 escaping for runewagerUsername - generate_tooltips.sh: keep path validation (absolute .js, no traversal) - start.sh/dev-run.sh/rollback.sh: keep SIGTERM→SIGKILL two-step - test/smoke.test.js: keep (.|)* and (.|)+ catch-all test cases Tests: 60/60 pass | node --check: clean | bash -n: all scripts OK https://claude.ai/code/session_01VijmtzjN63WZJy5gYgJAKs
There was a problem hiding this comment.
Actionable comments posted: 12
♻️ Duplicate comments (2)
RUNEWAGER_FUNCTIONALITY_MAP.md (1)
405-405:⚠️ Potential issue | 🟠 MajorResolve map-first contract conflict (still present).
Line 405 instructs docs-index-first, but Lines 396–399 require map-first. This contradiction must be removed to keep process rules unambiguous.
Based on learnings: "Always treat RUNEWAGER_FUNCTIONALITY_MAP.md as the authoritative source of truth before and after code changes" and "Begin every coding session by reading RUNEWAGER_FUNCTIONALITY_MAP.md; end with full verification pass".Suggested patch
-- 2026-03-01: Created `docs/` feature documentation system — 15 per-feature `.md` files, central `docs/INDEX.md` with full callback + pending-action cross-reference, and `docs/TODO_FUNCTIONALITY_UPGRADE.md` tracking 14 open upgrade/stale-menu items. Future Claude sessions must consult `docs/INDEX.md` first, then the relevant feature `.md`, before reading `index.js`. +- 2026-03-01: Created `docs/` feature documentation system — 15 per-feature `.md` files, central `docs/INDEX.md` with full callback + pending-action cross-reference, and `docs/TODO_FUNCTIONALITY_UPGRADE.md` tracking 14 open upgrade/stale-menu items. Future Claude sessions must consult `RUNEWAGER_FUNCTIONALITY_MAP.md` first, then `docs/INDEX.md` and the relevant feature `.md`, before reading `index.js`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@RUNEWAGER_FUNCTIONALITY_MAP.md` at line 405, The document RUNEWAGER_FUNCTIONALITY_MAP.md contains a conflicting instruction: one place tells readers to consult docs/INDEX.md first while another mandates treating RUNEWAGER_FUNCTIONALITY_MAP.md as authoritative and to start/end sessions by reading it; fix this by removing or editing the docs-first sentence so the file consistently enforces the "map-first" rule (i.e., update the sentence that references docs/INDEX.md to instruct readers to consult RUNEWAGER_FUNCTIONALITY_MAP.md first and to perform a full verification pass at the end), ensuring all wording in RUNEWAGER_FUNCTIONALITY_MAP.md unambiguously states the authoritative map-first workflow.index.js (1)
5795-5816:⚠️ Potential issue | 🟠 MajorSet
completionCardShownonly after successful send.This is still set before
ctx.reply(...)resolves, so a send failure can permanently suppress the one-time completion card for that user.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@index.js` around lines 5795 - 5816, The flag user.onboarding.completionCardShown is being set before the send, so a failed ctx.reply will still suppress future sends; change the flow in the block that builds the message (uses ctx.reply and Markup.inlineKeyboard with callback 'to_main_menu') to await ctx.reply inside a try/await and only set user.onboarding.completionCardShown = true after the await resolves successfully (and persist the user update if you currently persist elsewhere); if ctx.reply throws, leave the flag unchanged so the card can be retried.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/features/12-group-linking.md`:
- Around line 26-35: The fenced flow block starting with
"settings_group_linking_tools → groupLinkingToolsKeyboard" lacks a language tag
(triggering markdownlint MD040); update the fenced code block by adding an
explicit language identifier (e.g., text) after the opening backticks so the
block becomes "```text" to satisfy the linter while leaving the content of the
flow (settings_group_linking_tools, groupLinkingToolsKeyboard, group_link_start,
pendingAction: await_register_chat_forward, etc.) unchanged.
- Line 90: Replace the placeholder "(⚠️ audit flag)" on the `clearOldMenus(ctx,
user)` entry with a concrete behavior description to match other feature docs:
state that `clearOldMenus(ctx, user)` deletes previous menus before sending the
new group linking UI (since T-02 was resolved and `renderGroupLinkingTools()`
already calls it); mirror the concise style used in
`02-user-menu.md`/`16-walkthrough.md` and remove the audit-note placeholder.
In `@docs/features/14-announcements.md`:
- Around line 26-36: The fenced code block that begins with
"admin_cmd_announce_start → announcement builder UI" is missing a language tag
(MD040); update the opening fence from ``` to a language-specified fence such as
```text (or ```bash/```md as appropriate) so the block is flagged correctly by
the linter—edit the block containing "admin_cmd_announce_start → announcement
builder UI" and add the chosen language immediately after the opening backticks.
- Around line 60-63: The broadcastFailedUsers[] array is currently unbounded;
modify all places that push entries into broadcastFailedUsers (the push logic
that records broadcast failures) to enforce a 500-item cap by removing oldest
entries (e.g., shift()) when length >= 500 before pushing the new entry, and
likewise when restoring state from disk (the load/restore routine that
repopulates broadcastFailedUsers) trim the saved array to the most recent 500
entries before assigning it to broadcastFailedUsers so memory stays bounded.
In `@docs/features/15-misc-commands.md`:
- Around line 53-59: The fenced code block that documents the walkthrough flow
(the block starting with "/walkthrough → 35-step interactive walkthrough" and
containing "menu_walkthrough", "walk_back", "walk_next", "walk_done") is missing
a language tag; update the opening fence to include a language (for example use
```text) so the block is properly annotated (replace ``` with ```text).
In `@docs/features/16-walkthrough.md`:
- Around line 59-64: The snippet uses non-JavaScript terminology: replace the
pseudotype `bool` used for user.walkthrough.started with the proper JS type name
`boolean` (or reword the snippet as pseudocode); update the example object
(user.walkthrough with properties currentStep, completed, started) so started is
described as a boolean (or add a clarifying comment that the snippet is
pseudocode) to avoid using `bool`.
- Around line 26-41: The fenced code block starting at the walkthrough diagram
should include a language tag to satisfy MD040; update the block delimiter from
``` to ```text (or another appropriate language) so the block is marked as code.
Locate the block that documents sendWalkthroughStep and the walk_next /
walk_back / walk_done flow and change its opening fence to include "text" (or
similar) while leaving the content and closing fence unchanged.
In `@docs/INDEX.md`:
- Line 6: The two occurrences of the index.js size reference are inconsistent
(one shows "`index.js`: ~15,050 lines" and the other shows a different count);
pick a single canonical value or a small range and update both occurrences so
they match exactly (replace the string "`index.js`: ~15,050 lines" and the
matching reference at the later paragraph to the chosen value), ensuring the two
instances of the "**Bot version:** 3.0.0 | `index.js`: ..." line are identical.
In `@index.js`:
- Around line 1222-1223: The broadcastFailedUsers array is being extended
without limit (broadcastFailedUsers.push(...raw.broadcastFailedUsers)), causing
potential memory bloat; when loading state (restore/append paths) and when
recording failures, cap additions so the array never exceeds 500 entries by only
pushing up to (500 - broadcastFailedUsers.length) items and when loading
raw.broadcastFailedUsers slice to at most the first (500) entries before
merging; update both occurrences (the push call and any other places that append
to broadcastFailedUsers such as the other noted block around lines ~14992-14995)
to enforce this 500-entry cap.
- Around line 12242-12248: The menu-tracking fails for image steps because
sendPhoto is not returning the sent message, so `sent` is undefined and
`user.lastMenuMsgId/lastMenuChatId` never update; fix by making sendPhoto (the
function handling `step.image`) return the sent message object (the same shape
that `ctx.reply` returns) or, if it already uses the Telegram API call, await
and return that API result; then the existing code that reads `sent.message_id`
and sets `user.lastMenuMsgId`/`user.lastMenuChatId` (and uses
`getContextChatId(ctx)` as fallback) will work for image steps as well. Ensure
the returned object includes message_id and chat.id.
- Around line 14900-14905: The failure-rate alert is using the cumulative
broadcastFailedUsers array instead of only this run’s failures, causing the
percentage to drift; change the code to compute failed using the current run's
failure collection (e.g., a run-scoped array like broadcastFailedUsersThisRun or
by filtering broadcastFailedUsers by currentRunId/timestamp) and use that
failedThisRun together with the current run's sent (sentThisRun) to compute
attempted and the failure ratio before calling logEvent/alert; update references
to failed, attempted and the alert condition accordingly (symbols:
broadcastFailedUsers, failed, sent, attempted, logEvent).
In `@RUNEWAGER_FUNCTIONALITY_MAP.md`:
- Line 406: The doc currently contradicts itself about broadcast-failure
retention (mentions both a removed 500-item cap and earlier safety caps); update
RUNEWAGER_FUNCTIONALITY_MAP.md to state the single, authoritative policy: remove
the 500-item cap mention and clearly document that failures are persisted via
adminLog(), surfaced through the /broadcast_failed endpoint paginated in chunks
of 30 with an overflow note, and that a >20% failure rate triggers admin DMs —
or alternatively, if you intend to keep a cap, state the exact cap value and
eviction policy instead of saying "caps exist"; ensure the text references
adminLog(), /broadcast_failed, the 30-item pagination, the 500-item cap (if
retained) and the >20% threshold so there is no ambiguity.
---
Duplicate comments:
In `@index.js`:
- Around line 5795-5816: The flag user.onboarding.completionCardShown is being
set before the send, so a failed ctx.reply will still suppress future sends;
change the flow in the block that builds the message (uses ctx.reply and
Markup.inlineKeyboard with callback 'to_main_menu') to await ctx.reply inside a
try/await and only set user.onboarding.completionCardShown = true after the
await resolves successfully (and persist the user update if you currently
persist elsewhere); if ctx.reply throws, leave the flag unchanged so the card
can be retried.
In `@RUNEWAGER_FUNCTIONALITY_MAP.md`:
- Line 405: The document RUNEWAGER_FUNCTIONALITY_MAP.md contains a conflicting
instruction: one place tells readers to consult docs/INDEX.md first while
another mandates treating RUNEWAGER_FUNCTIONALITY_MAP.md as authoritative and to
start/end sessions by reading it; fix this by removing or editing the docs-first
sentence so the file consistently enforces the "map-first" rule (i.e., update
the sentence that references docs/INDEX.md to instruct readers to consult
RUNEWAGER_FUNCTIONALITY_MAP.md first and to perform a full verification pass at
the end), ensuring all wording in RUNEWAGER_FUNCTIONALITY_MAP.md unambiguously
states the authoritative map-first workflow.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
RUNEWAGER_FUNCTIONALITY_MAP.mdadd_tooltip.shdocs/INDEX.mddocs/TODO_FUNCTIONALITY_UPGRADE.mddocs/features/07-tooltips.mddocs/features/12-group-linking.mddocs/features/14-announcements.mddocs/features/15-misc-commands.mddocs/features/16-walkthrough.mdindex.js
🚧 Files skipped from review as they are similar to previous changes (3)
- add_tooltip.sh
- docs/TODO_FUNCTIONALITY_UPGRADE.md
- docs/features/07-tooltips.md
Comment on lines
+26
to
+35
| ``` | ||
| settings_group_linking_tools → groupLinkingToolsKeyboard | ||
| ├── group_link_start → pendingAction: await_register_chat_forward | ||
| │ └── Admin forwards any message from target group/channel | ||
| │ └── Chat ID extracted → group added to linkedGroups[] | ||
| ├── group_link_view → list all linked groups with IDs | ||
| ├── group_link_remove_menu → show removal picker | ||
| │ └── group_link_remove_{chatId} → remove from linkedGroups[] | ||
| └── group_link_test_permissions → check bot perms in each group | ||
| ``` |
There was a problem hiding this comment.
Add a language tag to the fenced flow block.
Line 26 uses a fenced code block without a language, which triggers markdownlint MD040.
Suggested patch
-```
+```text
settings_group_linking_tools → groupLinkingToolsKeyboard
├── group_link_start → pendingAction: await_register_chat_forward
│ └── Admin forwards any message from target group/channel
│ └── Chat ID extracted → group added to linkedGroups[]
├── group_link_view → list all linked groups with IDs
├── group_link_remove_menu → show removal picker
│ └── group_link_remove_{chatId} → remove from linkedGroups[]
└── group_link_test_permissions → check bot perms in each group</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.21.0)</summary>
[warning] 26-26: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @docs/features/12-group-linking.md around lines 26 - 35, The fenced flow
block starting with "settings_group_linking_tools → groupLinkingToolsKeyboard"
lacks a language tag (triggering markdownlint MD040); update the fenced code
block by adding an explicit language identifier (e.g., text) after the opening
backticks so the block becomes "```text" to satisfy the linter while leaving the
content of the flow (settings_group_linking_tools, groupLinkingToolsKeyboard,
group_link_start, pendingAction: await_register_chat_forward, etc.) unchanged.
</details>
<!-- fingerprinting:phantom:triton:hawk -->
<!-- This is an auto-generated comment by CodeRabbit -->
| ## Dependencies | ||
|
|
||
| - `renderGroupLinkingTools(ctx, user)` — sends group linking UI | ||
| - `clearOldMenus(ctx, user)` — should precede menu send (⚠️ audit flag) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat docs/features/12-group-linking.md | head -100 | tail -20Repository: gamblecodezcom/Runewager
Length of output: 577
🏁 Script executed:
rg "clearOldMenus" -B 2 -A 2Repository: gamblecodezcom/Runewager
Length of output: 18651
🏁 Script executed:
find . -name "index.js" -type f | head -20Repository: gamblecodezcom/Runewager
Length of output: 78
Update documentation to reflect implemented behavior instead of "audit flag" placeholder.
Line 90's wording "(clearOldMenus(ctx, user) is already implemented and actively called in renderGroupLinkingTools(). The TODO item T-02 was resolved on 2026-03-01. Update the entry to match the style of other feature docs (e.g., 02-user-menu.md, 16-walkthrough.md) to describe actual behavior: clearOldMenus(ctx, user) — deletes previous menus before sending new group linking UI.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/features/12-group-linking.md` at line 90, Replace the placeholder "(⚠️
audit flag)" on the `clearOldMenus(ctx, user)` entry with a concrete behavior
description to match other feature docs: state that `clearOldMenus(ctx, user)`
deletes previous menus before sending the new group linking UI (since T-02 was
resolved and `renderGroupLinkingTools()` already calls it); mirror the concise
style used in `02-user-menu.md`/`16-walkthrough.md` and remove the audit-note
placeholder.
Comment on lines
+26
to
+36
| ``` | ||
| admin_cmd_announce_start → announcement builder UI | ||
| ├── Compose text/media | ||
| ├── Add inline buttons (optional) | ||
| ├── 👁 Preview → send preview to admin DM | ||
| └── ✅ Send → broadcast to all users | ||
| └── One message per user (with setImmediate yield between) | ||
| └── Failed users tracked in broadcastFailedUsers[] | ||
| └── /broadcast_retry → retry failed | ||
| └── /broadcast_failed → view failed list | ||
| ``` |
There was a problem hiding this comment.
Specify a fenced-code language for the broadcast flow block.
Line 26 starts a fenced block without a language (MD040).
Suggested patch
-```
+```text
admin_cmd_announce_start → announcement builder UI
├── Compose text/media
├── Add inline buttons (optional)
├── 👁 Preview → send preview to admin DM
└── ✅ Send → broadcast to all users
└── One message per user (with setImmediate yield between)
└── Failed users tracked in broadcastFailedUsers[]
└── /broadcast_retry → retry failed
└── /broadcast_failed → view failed list</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/features/14-announcements.md` around lines 26 - 36, The fenced code
block that begins with "admin_cmd_announce_start → announcement builder UI" is
missing a language tag (MD040); update the opening fence from ``` to a
language-specified fence such as ```text (or ```bash/```md as appropriate) so
the block is flagged correctly by the linter—edit the block containing
"admin_cmd_announce_start → announcement builder UI" and add the chosen language
immediately after the opening backticks.
Comment on lines
+60
to
+63
| - `broadcastFailedUsers[]` — **no cap** — every failure is recorded (unbounded in memory; also written to `data/admin-events.log` via `adminLog('broadcast_failure', ...)`). | ||
| - `/broadcast_retry <message>` — retries all entries in `broadcastFailedUsers[]` with the provided message text. | ||
| - `/broadcast_failed` — shows failed users in chunks of 30 (up to 90 shown inline; full list in `data/admin-events.log`). Also shows a high-failure-rate ⚠️ warning if >20% of users failed. | ||
| - **High failure warning:** after weekly boost reminders, if >20% of attempted users failed, all `ADMIN_IDS` receive an alert DM with retry instructions. |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n -C3 'broadcastFailedUsers|broadcast_failed|broadcast_retry|adminLog\(.+broadcast_failure' index.jsRepository: gamblecodezcom/Runewager
Length of output: 5234
broadcastFailedUsers[] is unbounded and should be capped at 500 items.
The implementation (line 14892) pushes entries without any cap check, and on load (line 1221–1222) all saved entries are restored without limit. This can cause memory bloat over time. Add a 500-item cap when pushing to broadcastFailedUsers[] and on state load.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/features/14-announcements.md` around lines 60 - 63, The
broadcastFailedUsers[] array is currently unbounded; modify all places that push
entries into broadcastFailedUsers (the push logic that records broadcast
failures) to enforce a 500-item cap by removing oldest entries (e.g., shift())
when length >= 500 before pushing the new entry, and likewise when restoring
state from disk (the load/restore routine that repopulates broadcastFailedUsers)
trim the saved array to the most recent 500 entries before assigning it to
broadcastFailedUsers so memory stays bounded.
Comment on lines
+53
to
+59
| ``` | ||
| /walkthrough → 35-step interactive walkthrough | ||
| └── menu_walkthrough → start/resume walkthrough | ||
| ├── walk_back → previous step (disabled on step 1) | ||
| ├── walk_next → next step (disabled on last step) | ||
| └── walk_done → mark step complete; on last step: finish + return to menu | ||
| ``` |
There was a problem hiding this comment.
Add a language to the walkthrough flow fenced block.
Line 53 starts a fenced block without a language (MD040).
Suggested patch
-```
+```text
/walkthrough → 35-step interactive walkthrough
└── menu_walkthrough → start/resume walkthrough
├── walk_back → previous step (disabled on step 1)
├── walk_next → next step (disabled on last step)
└── walk_done → mark step complete; on last step: finish + return to menu</details>
<details>
<summary>🧰 Tools</summary>
<details>
<summary>🪛 markdownlint-cli2 (0.21.0)</summary>
[warning] 53-53: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
</details>
</details>
<details>
<summary>🤖 Prompt for AI Agents</summary>
Verify each finding against the current code and only fix it if needed.
In @docs/features/15-misc-commands.md around lines 53 - 59, The fenced code
block that documents the walkthrough flow (the block starting with "/walkthrough
→ 35-step interactive walkthrough" and containing "menu_walkthrough",
"walk_back", "walk_next", "walk_done") is missing a language tag; update the
opening fence to include a language (for example use text) so the block is properly annotated (replace with ```text).
</details>
<!-- fingerprinting:phantom:triton:hawk -->
<!-- This is an auto-generated comment by CodeRabbit -->
| > **Purpose:** This is the primary navigation index for all Claude sessions. Before modifying any feature, read the relevant `.md` file here. After any change, update both the feature `.md` and this index. | ||
|
|
||
| **Last updated:** 2026-03-01 | ||
| **Bot version:** 3.0.0 | `index.js`: ~15,050 lines | Commands: 95 | Action handlers: 270+ |
There was a problem hiding this comment.
Keep index.js size reference consistent in one source.
Line 6 and Line 174 list different approximate line counts (~15,050 vs ~14,960). Use one value/range to avoid documentation drift.
Also applies to: 174-174
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/INDEX.md` at line 6, The two occurrences of the index.js size reference
are inconsistent (one shows "`index.js`: ~15,050 lines" and the other shows a
different count); pick a single canonical value or a small range and update both
occurrences so they match exactly (replace the string "`index.js`: ~15,050
lines" and the matching reference at the later paragraph to the chosen value),
ensuring the two instances of the "**Bot version:** 3.0.0 | `index.js`: ..."
line are identical.
Comment on lines
+1222
to
1223
| broadcastFailedUsers.push(...raw.broadcastFailedUsers); | ||
| } |
There was a problem hiding this comment.
Reintroduce the broadcastFailedUsers hard cap on both restore and append paths.
broadcastFailedUsers is now unbounded when loading state and when recording failures, which can cause gradual memory bloat in long-running processes.
🛠️ Proposed fix
+const BROADCAST_FAILED_USERS_CAP = 500;
+
+function pushBroadcastFailure(entry) {
+ if (!entry || typeof entry !== 'object') return;
+ broadcastFailedUsers.push(entry);
+ if (broadcastFailedUsers.length > BROADCAST_FAILED_USERS_CAP) {
+ broadcastFailedUsers.splice(0, broadcastFailedUsers.length - BROADCAST_FAILED_USERS_CAP);
+ }
+}
if (Array.isArray(raw.broadcastFailedUsers)) {
- broadcastFailedUsers.push(...raw.broadcastFailedUsers);
+ broadcastFailedUsers.push(...raw.broadcastFailedUsers.slice(-BROADCAST_FAILED_USERS_CAP));
}
- broadcastFailedUsers.push({ userId: user.id, lastError: e.message, failedAt: now });
+ pushBroadcastFailure({ userId: user.id, lastError: e.message, failedAt: now });As per coding guidelines Cap broadcastFailedUsers at 500 on push and on load to prevent memory bloat.
Also applies to: 14992-14995
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@index.js` around lines 1222 - 1223, The broadcastFailedUsers array is being
extended without limit (broadcastFailedUsers.push(...raw.broadcastFailedUsers)),
causing potential memory bloat; when loading state (restore/append paths) and
when recording failures, cap additions so the array never exceeds 500 entries by
only pushing up to (500 - broadcastFailedUsers.length) items and when loading
raw.broadcastFailedUsers slice to at most the first (500) entries before
merging; update both occurrences (the push call and any other places that append
to broadcastFailedUsers such as the other noted block around lines ~14992-14995)
to enforce this 500-entry cap.
Comment on lines
+12242
to
12248
| const sent = await (step.image | ||
| ? sendPhoto(ctx, step.image, header, nav) | ||
| : ctx.reply(header, nav)); | ||
| if (sent && sent.message_id) { | ||
| user.lastMenuMsgId = sent.message_id; | ||
| user.lastMenuChatId = sent.chat ? sent.chat.id : getContextChatId(ctx); | ||
| } |
There was a problem hiding this comment.
Walkthrough menu tracking is skipped for image steps because sendPhoto returns no message object.
For steps with step.image, sent becomes undefined, so lastMenuMsgId/lastMenuChatId are never updated. This breaks the cleanup chain for subsequent transitions.
🛠️ Proposed fix
async function sendPhoto(ctx, imgSrc, caption, extra = {}) {
try {
const isUrl = imgSrc.startsWith('https://') || imgSrc.startsWith('http://');
const safeImgPath = isUrl ? null : validateSafePath(imgSrc, IMG_DIR);
const source = isUrl ? imgSrc : { source: fs.createReadStream(safeImgPath) };
- await ctx.replyWithPhoto(source, { caption, ...extra });
+ return await ctx.replyWithPhoto(source, { caption, ...extra });
} catch (e) {
logEvent('warn', 'sendPhoto failed, falling back to text', { imgSrc, error: e.message });
- await ctx.reply(caption, extra);
+ return await ctx.reply(caption, extra);
}
}📝 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.
Suggested change
| const sent = await (step.image | |
| ? sendPhoto(ctx, step.image, header, nav) | |
| : ctx.reply(header, nav)); | |
| if (sent && sent.message_id) { | |
| user.lastMenuMsgId = sent.message_id; | |
| user.lastMenuChatId = sent.chat ? sent.chat.id : getContextChatId(ctx); | |
| } | |
| async function sendPhoto(ctx, imgSrc, caption, extra = {}) { | |
| try { | |
| const isUrl = imgSrc.startsWith('https://') || imgSrc.startsWith('http://'); | |
| const safeImgPath = isUrl ? null : validateSafePath(imgSrc, IMG_DIR); | |
| const source = isUrl ? imgSrc : { source: fs.createReadStream(safeImgPath) }; | |
| return await ctx.replyWithPhoto(source, { caption, ...extra }); | |
| } catch (e) { | |
| logEvent('warn', 'sendPhoto failed, falling back to text', { imgSrc, error: e.message }); | |
| return await ctx.reply(caption, extra); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@index.js` around lines 12242 - 12248, The menu-tracking fails for image steps
because sendPhoto is not returning the sent message, so `sent` is undefined and
`user.lastMenuMsgId/lastMenuChatId` never update; fix by making sendPhoto (the
function handling `step.image`) return the sent message object (the same shape
that `ctx.reply` returns) or, if it already uses the Telegram API call, await
and return that API result; then the existing code that reads `sent.message_id`
and sets `user.lastMenuMsgId`/`user.lastMenuChatId` (and uses
`getContextChatId(ctx)` as fallback) will work for image steps as well. Ensure
the returned object includes message_id and chat.id.
Comment on lines
+14900
to
+14905
| const failed = broadcastFailedUsers.length; | ||
| logEvent('info', 'Weekly boost reminder sent', { sent, failed }); | ||
| // Warn admins if more than 20% of attempted users failed | ||
| const attempted = sent + failed; | ||
| if (attempted > 0 && failed / attempted > 0.20) { | ||
| const adminIds = (process.env.ADMIN_IDS || '').split(',').map((id) => id.trim()).filter(Boolean); |
There was a problem hiding this comment.
Weekly boost failure-rate alert uses historical backlog instead of this run’s failures.
failed is derived from broadcastFailedUsers.length, so alert percentages drift upward over time even when current sends are healthy.
🛠️ Proposed fix
async function runWeeklyBoostReminder() {
- const now = Date.now(); const WEEK_MS_26 = 7 * 24 * 60 * 60 * 1000; let sent = 0;
+ const now = Date.now(); const WEEK_MS_26 = 7 * 24 * 60 * 60 * 1000; let sent = 0; let failedThisRun = 0;
for (const [, user] of userStore) {
@@
} catch (e) {
user.unreachable = true;
pushBroadcastFailure({ userId: user.id, lastError: e.message, failedAt: now });
+ failedThisRun += 1;
// Log each failure to admin-events.log so no entry is ever silently dropped
adminLog('broadcast_failure', { userId: user.id, error: e.message });
}
@@
- const failed = broadcastFailedUsers.length;
+ const failed = failedThisRun;
logEvent('info', 'Weekly boost reminder sent', { sent, failed });
// Warn admins if more than 20% of attempted users failed
const attempted = sent + failed;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@index.js` around lines 14900 - 14905, The failure-rate alert is using the
cumulative broadcastFailedUsers array instead of only this run’s failures,
causing the percentage to drift; change the code to compute failed using the
current run's failure collection (e.g., a run-scoped array like
broadcastFailedUsersThisRun or by filtering broadcastFailedUsers by
currentRunId/timestamp) and use that failedThisRun together with the current
run's sent (sentThisRun) to compute attempted and the failure ratio before
calling logEvent/alert; update references to failed, attempted and the alert
condition accordingly (symbols: broadcastFailedUsers, failed, sent, attempted,
logEvent).
| - 2026-02-27: Hardened SSHV Run prompt flow so admin text in private DM executes against active SSHV sessions if pending state desynchronizes. | ||
| - 2026-02-28: v3.1 — added group command guard middleware (`GROUP_PASSTHROUGH_COMMANDS` + `bot.use` interceptor); added `onboardingProgressBar()` and progress header on each onboarding step prompt (auto-deletes after 8s); added one-time onboarding completion card (tracked via `user.onboarding.completionCardShown`); added `🔗 Group Linking` to Admin System Tools keyboard (`admin_sys_group_linking` callback with back-to-system-tools navigation). | ||
| - 2026-03-01: Created `docs/` feature documentation system — 15 per-feature `.md` files, central `docs/INDEX.md` with full callback + pending-action cross-reference, and `docs/TODO_FUNCTIONALITY_UPGRADE.md` tracking 14 open upgrade/stale-menu items. Future Claude sessions must consult `docs/INDEX.md` first, then the relevant feature `.md`, before reading `index.js`. | ||
| - 2026-03-01: Phase implementation — resolved T-01/T-02/T-03/T-15 from TODO list. (1) Walkthrough: `sendWalkthroughStep()` upgraded with `clearOldMenus()`, Back disabled on step 1, Finish on last step, `walk_done` on last step returns to main menu. New doc: `16-walkthrough.md`. (2) Menu stacking: `clearOldMenus()` added to `sendOnboardingReferralPrompt`, `renderSshvConsole`, `renderGroupLinkingTools`, `tips_cmd_edit`, `tips_cmd_remove`. (3) Tooltip view: `tips_cmd_view` selector + `tip_view_{id}` handler with Prev/Next/Edit/Toggle/Delete/Back/AdminMenu — "👁 View Tooltip" button added to dashboard. (4) Broadcast failures: 500-item cap removed; all failures logged via `adminLog()`; `/broadcast_failed` shows chunks of 30 with overflow note; >20% failure rate triggers admin DM warnings. PR comments fixed: `add_tooltip.sh` array validation hardened; `docs/12-group-linking.md` entry-point callback corrected to `admin_sys_group_linking`; merge conflicts (PRs #112-114) resolved keeping SIGTERM→SIGKILL safety improvements. 60/60 tests pass. |
There was a problem hiding this comment.
Fix broadcast-failure retention contradiction in the authoritative map.
Line 406 says the 500-item cap was removed, but the earlier safety section says failure tracking caps exist to bound memory growth. Keep one consistent statement, or explicitly document current retention policy.
As per coding guidelines: "RUNEWAGER_FUNCTIONALITY_MAP.md: Always treat RUNEWAGER_FUNCTIONALITY_MAP.md as the authoritative source of truth before and after code changes".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@RUNEWAGER_FUNCTIONALITY_MAP.md` at line 406, The doc currently contradicts
itself about broadcast-failure retention (mentions both a removed 500-item cap
and earlier safety caps); update RUNEWAGER_FUNCTIONALITY_MAP.md to state the
single, authoritative policy: remove the 500-item cap mention and clearly
document that failures are persisted via adminLog(), surfaced through the
/broadcast_failed endpoint paginated in chunks of 30 with an overflow note, and
that a >20% failure rate triggers admin DMs — or alternatively, if you intend to
keep a cap, state the exact cap value and eviction policy instead of saying
"caps exist"; ensure the text references adminLog(), /broadcast_failed, the
30-item pagination, the 500-item cap (if retained) and the >20% threshold so
there is no ambiguity.
Three changes addressing remaining PR #115 review feedback: 1. generate_tooltips.sh — skip guard to preserve runtime tips - Skip regeneration if data/tooltips.json already has entries. - Prevents admin-added tooltips (broadcast every 4h via bot) from being overwritten on every restart or deploy. - Add --force flag to override the guard when intentional reset is needed. - --dry-run still works regardless of the guard. 2. scripts/helpers/free_port.sh — shared SIGTERM→SIGKILL helper - Extracts duplicated port-freeing logic (lsof/fuser, SIGTERM→SIGKILL) from start.sh, dev-run.sh, deploy.sh, and scripts/rollback.sh into a single reusable helper (sourceable or callable directly). - Reduces drift: future tweaks to kill strategy happen in one place. 3. start.sh / dev-run.sh — RUNEWAGER_AUTO_UPDATE gate - git fetch + reset --hard origin/main is now conditional on RUNEWAGER_AUTO_UPDATE env var (default 1 in prod start.sh, default 0 in dev-run.sh). - Prevents silent discard of local/staging uncommitted changes. - Documented in .env.example. https://claude.ai/code/session_01VijmtzjN63WZJy5gYgJAKs
There was a problem hiding this comment.
♻️ Duplicate comments (4)
generate_tooltips.sh (1)
20-22:⚠️ Potential issue | 🟠 MajorNormalize
APP_DIRbefore strict absolute-path validation.Line 21 allows a relative
RUNEWAGER_DIR, but Lines 61-63 require an absolute.jspath. That mismatch can trigger fallback placeholder writes instead of extracting real tooltips.🛠️ Proposed fix
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -APP_DIR="${RUNEWAGER_DIR:-$SCRIPT_DIR}" +RAW_APP_DIR="${RUNEWAGER_DIR:-$SCRIPT_DIR}" +APP_DIR="$(cd "$RAW_APP_DIR" 2>/dev/null && pwd)" || error "Invalid RUNEWAGER_DIR: $RAW_APP_DIR" DATA_DIR="$APP_DIR/data" TOOLTIPS_FILE="$DATA_DIR/tooltips.json" TMP_FILE="$TOOLTIPS_FILE.tmp.$$"Also applies to: 57-63
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@generate_tooltips.sh` around lines 20 - 22, APP_DIR may be left relative from RUNEWAGER_DIR which later causes the absolute-path validation for .js files to fail; normalize APP_DIR to an absolute path immediately after its assignment (similar to how SCRIPT_DIR is computed) so subsequent checks that validate .js paths (the logic around APP_DIR, DATA_DIR and the .js extraction block) operate on an absolute path; update any later uses that check for absolute .js file paths to use the normalized APP_DIR value so the tooltip extraction code path runs correctly.scripts/helpers/free_port.sh (1)
19-24:⚠️ Potential issue | 🟠 MajorRe-resolve blockers before
SIGKILLto avoid PID-reuse kills.Line 23 sends
kill -9using the original_pidslist after a delay. If a PID is reused in that window, an unrelated process can be terminated.🛠️ Proposed hardening
if [[ -n "$_pids" ]]; then echo "[free_port] Port $port blocked — sending SIGTERM then SIGKILL..." for _p in $_pids; do kill "$_p" 2>/dev/null || true; done sleep 2 - for _p in $_pids; do kill -9 "$_p" 2>/dev/null || true; done + local _still_pids="" + if command -v lsof >/dev/null 2>&1; then + _still_pids="$(lsof -ti :"$port" 2>/dev/null || true)" + elif command -v fuser >/dev/null 2>&1; then + _still_pids="$(fuser -n tcp "$port" 2>/dev/null | tr ' ' '\n' | sed '/^$/d' || true)" + fi + for _p in $_still_pids; do + kill -0 "$_p" 2>/dev/null && kill -9 "$_p" 2>/dev/null || true + done sleep 1 fiAs per coding guidelines: "
**/*.sh: ... confirm destructive operations; chain commands and check exit codes".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/helpers/free_port.sh` around lines 19 - 24, The current logic uses the original _pids list for the final SIGKILL which risks killing a different process if PIDs were recycled; update the sequence in scripts/helpers/free_port.sh so that after the graceful kill and sleep you re-resolve which PIDs are still holding the target port (e.g., re-run the same discovery used initially, using the port variable) and only SIGKILL those current PIDs instead of the old _pids; ensure each destructive command (the SIGTERM loop, the re-resolution command, and the SIGKILL loop) is chained/guarded and checks exit status (or ignores errors explicitly) to follow the shell guideline for confirming destructive operations while referencing the existing _pids and _p variables to locate the code to change.dev-run.sh (1)
22-30:⚠️ Potential issue | 🟠 Major
RUNEWAGER_AUTO_UPDATEfrom.envis not actually applied.Line 23 says
.envcan control this, but Line 24 only reads inherited environment. In./dev-run.sh,.envvalue is currently ignored.🛠️ Proposed fix
# Optionally pull latest code (off by default in dev to preserve local changes) # Set RUNEWAGER_AUTO_UPDATE=1 in .env or environment to enable -if [ "${RUNEWAGER_AUTO_UPDATE:-0}" = "1" ]; then +AUTO_UPDATE_FROM_ENV_FILE=$(grep -E '^RUNEWAGER_AUTO_UPDATE=' "$ROOT_DIR/.env" 2>/dev/null | head -1 | cut -d= -f2 | cut -d'#' -f1 | tr -d '"' | tr -d "'" | tr -d ' ' || true) +AUTO_UPDATE="${RUNEWAGER_AUTO_UPDATE:-${AUTO_UPDATE_FROM_ENV_FILE:-0}}" +if [ "$AUTO_UPDATE" = "1" ]; then echo "[dev-run] Pulling latest code from origin main..." git -C "$ROOT_DIR" fetch origin main 2>&1 \ && git -C "$ROOT_DIR" reset --hard origin/main 2>&1 \ || echo "[dev-run] WARN: git pull failed — starting with local copy" elseAs per coding guidelines: "
**/*.sh: ... confirm destructive operations; chain commands and check exit codes".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev-run.sh` around lines 22 - 30, The script currently checks RUNEWAGER_AUTO_UPDATE but never loads the .env file; update dev-run.sh to load environment variables from the repository .env (using $ROOT_DIR to locate it) before the if [ "${RUNEWAGER_AUTO_UPDATE:-0}" = "1" ] check so the .env value is honored; ensure the loader exports variables into the shell environment (so RUNEWAGER_AUTO_UPDATE becomes available), validate the loaded value compared against "1" in the existing condition, and keep the existing safety patterns (confirm destructive operations, chain git commands and check exit codes) around the git fetch/reset block.start.sh (1)
44-52:⚠️ Potential issue | 🟠 Major
.envcannot currently disable auto-update beforegit reset --hard.The comments state
.envcontrols this, but Line 48 reads only process env with default1. Running./start.shcan still hard-reset even when.envsetsRUNEWAGER_AUTO_UPDATE=0.🛠️ Proposed fix
############################################### # Optionally pull latest code before starting # Set RUNEWAGER_AUTO_UPDATE=0 in .env to disable # (useful in dev/staging to avoid overwriting local changes) ############################################### -if [[ "${RUNEWAGER_AUTO_UPDATE:-1}" == "1" ]]; then +AUTO_UPDATE_FROM_ENV_FILE="$(grep -E '^RUNEWAGER_AUTO_UPDATE=' "$PROJECT_DIR/.env" 2>/dev/null | head -1 | cut -d= -f2 | cut -d'#' -f1 | tr -d '"' | tr -d "'" | tr -d ' ' || true)" +AUTO_UPDATE="${RUNEWAGER_AUTO_UPDATE:-${AUTO_UPDATE_FROM_ENV_FILE:-1}}" +if [[ "$AUTO_UPDATE" == "1" ]]; then echo "📥 Pulling latest code from origin main..." git -C "$PROJECT_DIR" fetch origin main 2>&1 \ && git -C "$PROJECT_DIR" reset --hard origin/main 2>&1 \ || echo "⚠️ git pull failed — starting with local copy" elseAs per coding guidelines: "
**/*.sh: ... confirm destructive operations; chain commands and check exit codes".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@start.sh` around lines 44 - 52, The auto-update check currently reads RUNEWAGER_AUTO_UPDATE from the environment but may run before .env is sourced and uses a chained git command that can still execute git reset --hard; fix by sourcing the .env (if present) before the check, then make the update guard explicit (if [[ "${RUNEWAGER_AUTO_UPDATE:-1}" == "1" ]]; then ... fi) and run git -C "$PROJECT_DIR" fetch origin main && git -C "$PROJECT_DIR" reset --hard origin/main inside that block (do not rely on a single chained line with a trailing ||), and add a confirmation or safer guard before the destructive git reset --hard (or skip reset when RUNEWAGER_AUTO_UPDATE != "1"), referencing RUNEWAGER_AUTO_UPDATE, start.sh, PROJECT_DIR, and the git reset --hard command to locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@dev-run.sh`:
- Around line 22-30: The script currently checks RUNEWAGER_AUTO_UPDATE but never
loads the .env file; update dev-run.sh to load environment variables from the
repository .env (using $ROOT_DIR to locate it) before the if [
"${RUNEWAGER_AUTO_UPDATE:-0}" = "1" ] check so the .env value is honored; ensure
the loader exports variables into the shell environment (so
RUNEWAGER_AUTO_UPDATE becomes available), validate the loaded value compared
against "1" in the existing condition, and keep the existing safety patterns
(confirm destructive operations, chain git commands and check exit codes) around
the git fetch/reset block.
In `@generate_tooltips.sh`:
- Around line 20-22: APP_DIR may be left relative from RUNEWAGER_DIR which later
causes the absolute-path validation for .js files to fail; normalize APP_DIR to
an absolute path immediately after its assignment (similar to how SCRIPT_DIR is
computed) so subsequent checks that validate .js paths (the logic around
APP_DIR, DATA_DIR and the .js extraction block) operate on an absolute path;
update any later uses that check for absolute .js file paths to use the
normalized APP_DIR value so the tooltip extraction code path runs correctly.
In `@scripts/helpers/free_port.sh`:
- Around line 19-24: The current logic uses the original _pids list for the
final SIGKILL which risks killing a different process if PIDs were recycled;
update the sequence in scripts/helpers/free_port.sh so that after the graceful
kill and sleep you re-resolve which PIDs are still holding the target port
(e.g., re-run the same discovery used initially, using the port variable) and
only SIGKILL those current PIDs instead of the old _pids; ensure each
destructive command (the SIGTERM loop, the re-resolution command, and the
SIGKILL loop) is chained/guarded and checks exit status (or ignores errors
explicitly) to follow the shell guideline for confirming destructive operations
while referencing the existing _pids and _p variables to locate the code to
change.
In `@start.sh`:
- Around line 44-52: The auto-update check currently reads RUNEWAGER_AUTO_UPDATE
from the environment but may run before .env is sourced and uses a chained git
command that can still execute git reset --hard; fix by sourcing the .env (if
present) before the check, then make the update guard explicit (if [[
"${RUNEWAGER_AUTO_UPDATE:-1}" == "1" ]]; then ... fi) and run git -C
"$PROJECT_DIR" fetch origin main && git -C "$PROJECT_DIR" reset --hard
origin/main inside that block (do not rely on a single chained line with a
trailing ||), and add a confirmation or safer guard before the destructive git
reset --hard (or skip reset when RUNEWAGER_AUTO_UPDATE != "1"), referencing
RUNEWAGER_AUTO_UPDATE, start.sh, PROJECT_DIR, and the git reset --hard command
to locate the change.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.env.exampledeploy.shdev-run.shgenerate_tooltips.shscripts/helpers/free_port.shscripts/rollback.shstart.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- deploy.sh
1. generate_tooltips.sh — normalize APP_DIR to absolute path Move helper function definitions (info/warn/error) before variable assignments so error() is available at init time. Normalize RUNEWAGER_DIR → APP_DIR via cd+pwd immediately after assignment so the Node.js absolute-path validation (requires '/'-prefixed path) never fails when a caller passes a relative RUNEWAGER_DIR. 2. scripts/helpers/free_port.sh — re-query port before SIGKILL Extract discovery into _query_port_pids() helper. After the SIGTERM grace period, re-query the port for survivors and only SIGKILL PIDs that are still listening — guards against killing an unrelated process that reused a PID during the 2 s sleep window. 3. dev-run.sh — read RUNEWAGER_AUTO_UPDATE from .env as fallback Parse RUNEWAGER_AUTO_UPDATE from .env before the auto-update guard so the flag works even when .env values have not been exported into the calling shell. Use explicit if/else instead of chained && || for the destructive git reset --hard command. 4. start.sh — same .env-read fix + explicit if/else for git reset Same pattern as dev-run.sh: resolve RUNEWAGER_AUTO_UPDATE from env then .env (default 1 for prod), replace the chained git &&/|| with an explicit if/else block. https://claude.ai/code/session_01VijmtzjN63WZJy5gYgJAKs
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
dev-run.sh (1)
47-49: Port cleanup delegation looks good; consider non-fatal handling for consistency.Delegating to
free_port.shaddresses the previous concern about killing unrelated processes. The DEV_PORT parsing mirrors the auto-update extraction pattern nicely.One minor inconsistency: the tooltip script (lines 40-44) treats failures as non-fatal with
|| echo WARN, butfree_port.shfailure would abort the script due toset -e. If the helper can fail for benign reasons (e.g., no process on port), consider making it non-fatal:-bash "$ROOT_DIR/scripts/helpers/free_port.sh" "$DEV_PORT" +bash "$ROOT_DIR/scripts/helpers/free_port.sh" "$DEV_PORT" \ + || echo "[dev-run] WARN: free_port.sh failed (non-fatal)"That said, if the helper is designed to exit 0 on "nothing to free," this may not be needed.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev-run.sh` around lines 47 - 49, The free_port.sh invocation can abort the whole script under set -e if it exits non-zero for benign reasons; update the call that uses DEV_PORT and free_port.sh so failures are treated non-fatal (e.g., append a safe fallback like "|| true" or capture the exit and emit a warning) to mirror the tooltip handling; locate the DEV_PORT variable and the bash "$ROOT_DIR/scripts/helpers/free_port.sh" "$DEV_PORT" invocation and change it to handle non-zero exits gracefully (or ensure free_port.sh itself returns 0 for benign cases).generate_tooltips.sh (1)
37-40: Reject unknown CLI flags instead of silently ignoring them.A typo like
--frocecurrently does nothing and can lead to unexpected behavior. Fail fast with acasedefault branch.Suggested parser update
-for arg in "$@"; do - [[ "$arg" == "--dry-run" ]] && DRY_RUN=true - [[ "$arg" == "--force" ]] && FORCE=true -done +for arg in "$@"; do + case "$arg" in + --dry-run) DRY_RUN=true ;; + --force) FORCE=true ;; + *) error "Unknown option: $arg. Usage: ./generate_tooltips.sh [--dry-run] [--force]" ;; + esac +done🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@generate_tooltips.sh` around lines 37 - 40, The current argument loop in generate_tooltips.sh silently ignores unknown flags; replace the simple [[ ... ]] checks with a proper case-based parser over "$@" (or inside the existing for arg in "$@"; do ... done) that explicitly handles "--dry-run" (set DRY_RUN=true) and "--force" (set FORCE=true) and includes a default *) branch which prints an error like "Unknown flag: $arg" to stderr and exits non-zero, ensuring typos such as "--froce" cause the script to fail fast.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@generate_tooltips.sh`:
- Around line 49-50: The inline Node.js invocations that interpolate shell
variables (e.g., the _EXISTING assignment using node -e and '$TOOLTIPS_FILE')
are fragile for paths with quotes; update these to pass TOOLTIPS_FILE and
TMP_FILE via environment variables or argv instead of embedding them in
single-quoted JS code. Modify the _EXISTING computation and the other node -e
uses to read process.env or process.argv for the file path (referencing
TOOLTIPS_FILE, TMP_FILE and the node -e invocations) and ensure error
handling/output remains the same.
---
Nitpick comments:
In `@dev-run.sh`:
- Around line 47-49: The free_port.sh invocation can abort the whole script
under set -e if it exits non-zero for benign reasons; update the call that uses
DEV_PORT and free_port.sh so failures are treated non-fatal (e.g., append a safe
fallback like "|| true" or capture the exit and emit a warning) to mirror the
tooltip handling; locate the DEV_PORT variable and the bash
"$ROOT_DIR/scripts/helpers/free_port.sh" "$DEV_PORT" invocation and change it to
handle non-zero exits gracefully (or ensure free_port.sh itself returns 0 for
benign cases).
In `@generate_tooltips.sh`:
- Around line 37-40: The current argument loop in generate_tooltips.sh silently
ignores unknown flags; replace the simple [[ ... ]] checks with a proper
case-based parser over "$@" (or inside the existing for arg in "$@"; do ...
done) that explicitly handles "--dry-run" (set DRY_RUN=true) and "--force" (set
FORCE=true) and includes a default *) branch which prints an error like "Unknown
flag: $arg" to stderr and exits non-zero, ensuring typos such as "--froce" cause
the script to fail fast.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
dev-run.shgenerate_tooltips.shscripts/helpers/free_port.shstart.sh
🚧 Files skipped from review as they are similar to previous changes (2)
- start.sh
- scripts/helpers/free_port.sh
1. generate_tooltips.sh — case-based arg parser Replace the for-loop [[]] checks with a case statement that rejects unknown flags (e.g. --froce typo) with a clear error and non-zero exit. 2. generate_tooltips.sh — env-var file paths in Node.js invocations The three inline node -e calls that interpolated $TOOLTIPS_FILE / $TMP_FILE directly into single-quoted JS strings were fragile for paths containing quotes or special characters. All three now pass the path via a dedicated env var (TOOLTIPS_FILE_PATH or VALIDATE_FILE) and read process.env inside the script, matching the existing RUNEWAGER_APP pattern. 3. dev-run.sh — non-fatal free_port.sh invocation free_port.sh can exit non-zero on benign errors (no lsof/fuser, race after SIGTERM) which would abort dev-run.sh under set -eu. Added || echo WARN fallback to mirror the same non-fatal pattern used for the tooltip script invocation directly above. https://claude.ai/code/session_01VijmtzjN63WZJy5gYgJAKs
There was a problem hiding this comment.
♻️ Duplicate comments (1)
dev-run.sh (1)
27-30:⚠️ Potential issue | 🟠 MajorGuard hard reset behind an explicit force-reset confirmation.
Line 29 still executes a destructive
reset --hardwhen auto-update is enabled, which can silently discard local work. Keep auto-update, but require a separate explicit force-reset opt-in and default tomerge --ff-only.Suggested change
if [ "$_AUTO_UPDATE" = "1" ]; then echo "[dev-run] Pulling latest code from origin main..." - if git -C "$ROOT_DIR" fetch origin main 2>&1 && git -C "$ROOT_DIR" reset --hard origin/main 2>&1; then - echo "[dev-run] Code updated to $(git -C "$ROOT_DIR" rev-parse --short HEAD)" + if git -C "$ROOT_DIR" fetch origin main 2>&1; then + if [ "${DEV_RUN_ALLOW_HARD_RESET:-0}" = "1" ]; then + git -C "$ROOT_DIR" reset --hard origin/main 2>&1 \ + || echo "[dev-run] WARN: hard reset failed — starting with local copy" + else + git -C "$ROOT_DIR" merge --ff-only origin/main 2>&1 \ + || echo "[dev-run] WARN: non-fast-forward; set DEV_RUN_ALLOW_HARD_RESET=1 to force reset" + fi + echo "[dev-run] Code updated to $(git -C "$ROOT_DIR" rev-parse --short HEAD)" else - echo "[dev-run] WARN: git pull failed — starting with local copy" + echo "[dev-run] WARN: git fetch failed — starting with local copy" fi elseAs per coding guidelines:
**/*.shscripts must “confirm destructive operations”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@dev-run.sh` around lines 27 - 30, The script currently performs a destructive git -C "$ROOT_DIR" reset --hard origin/main whenever _AUTO_UPDATE=1; change this so auto-update defaults to a non-destructive fast-forward path (run git -C "$ROOT_DIR" fetch origin main and git -C "$ROOT_DIR" merge --ff-only origin/main) and only perform git -C "$ROOT_DIR" reset --hard origin/main when an explicit opt-in variable (e.g. _AUTO_UPDATE_FORCE=1) is set or when an interactive confirmation is given; update the _AUTO_UPDATE branch logic to check _AUTO_UPDATE_FORCE before calling reset --hard and otherwise use merge --ff-only, and ensure any destructive action requires the explicit opt-in or a user confirmation to satisfy the “confirm destructive operations” guideline.
🧹 Nitpick comments (1)
generate_tooltips.sh (1)
102-115: Atomic write pattern is correctly implemented; consider adding a cleanup trap.The temp file → validate → move pattern follows coding guidelines. The
VALIDATE_FILEenv var usage correctly addresses the previous shell interpolation concern.One minor gap: if the script is interrupted (e.g., SIGINT) between line 103 and 113, the temp file may remain. Consider adding a trap for cleanup.
♻️ Optional: Add cleanup trap
TMP_FILE="$TOOLTIPS_FILE.tmp.$$" +trap 'rm -f "$TMP_FILE"' EXIT DRY_RUN=false🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@generate_tooltips.sh` around lines 102 - 115, Add a shell trap to remove the temporary file on script exit or interruption so TMP_FILE doesn't linger: register a trap that runs rm -f "$TMP_FILE" (and any other temporary artifacts) on EXIT, SIGINT, and SIGTERM near the top of the section that writes/validates the temp file (the block that uses VALIDATE_FILE, TMP_FILE, and mv to TOOLTIPS_FILE); ensure the trap is safe to call if TMP_FILE is unset and is removed before the successful mv to TOOLTIPS_FILE or on any early exit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@dev-run.sh`:
- Around line 27-30: The script currently performs a destructive git -C
"$ROOT_DIR" reset --hard origin/main whenever _AUTO_UPDATE=1; change this so
auto-update defaults to a non-destructive fast-forward path (run git -C
"$ROOT_DIR" fetch origin main and git -C "$ROOT_DIR" merge --ff-only
origin/main) and only perform git -C "$ROOT_DIR" reset --hard origin/main when
an explicit opt-in variable (e.g. _AUTO_UPDATE_FORCE=1) is set or when an
interactive confirmation is given; update the _AUTO_UPDATE branch logic to check
_AUTO_UPDATE_FORCE before calling reset --hard and otherwise use merge
--ff-only, and ensure any destructive action requires the explicit opt-in or a
user confirmation to satisfy the “confirm destructive operations” guideline.
---
Nitpick comments:
In `@generate_tooltips.sh`:
- Around line 102-115: Add a shell trap to remove the temporary file on script
exit or interruption so TMP_FILE doesn't linger: register a trap that runs rm -f
"$TMP_FILE" (and any other temporary artifacts) on EXIT, SIGINT, and SIGTERM
near the top of the section that writes/validates the temp file (the block that
uses VALIDATE_FILE, TMP_FILE, and mv to TOOLTIPS_FILE); ensure the trap is safe
to call if TMP_FILE is unset and is removed before the successful mv to
TOOLTIPS_FILE or on any early exit.
1. generate_tooltips.sh — trap for TMP_FILE cleanup Register 'trap rm -f TMP_FILE EXIT INT TERM' immediately before the atomic write section so the temp file is always removed on any exit (error, signal, or normal completion). After a successful mv the path no longer exists, so the trap is a safe no-op on the happy path. 2. dev-run.sh — default to merge --ff-only; gate reset --hard behind opt-in Auto-update now runs 'git fetch + merge --ff-only' (non-destructive). 'git reset --hard origin/main' is only executed when RUNEWAGER_FORCE_RESET=1 is set in the environment or .env, satisfying the "confirm destructive operations" guideline. Fast-forward failure emits a clear warning pointing the user to RUNEWAGER_FORCE_RESET. Documented in .env.example. https://claude.ai/code/session_01VijmtzjN63WZJy5gYgJAKs
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
Introduce v3.1 operational and documentation updates, including stricter group command handling, enhanced onboarding UX, safer tooltip tooling, and more robust deploy/run scripts with port management and tooltip regeneration.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation:
Tests:
Chores:
CodeAnt-AI Description
Redirect group commands to DM, show onboarding progress and completion, add admin Group Linking and safer tooltip import/refresh
What Changed
Impact
✅ Clearer DM guidance for group users✅ Shorter onboarding with visible progress and a welcoming completion card✅ Fewer deploy-start failures due to blocked ports and stale tooltips💡 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
Bug Fixes
Refactors
Documentation