Claude/plan v3 deployment vp mpw#113
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
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
Reviewer's GuideAdds group command guard middleware and onboarding UX improvements, introduces a safer batch tooltip import flow, promotes group linking to a top-level admin system tool, and hardens deployment/dev/rollback scripts by auto-pulling code, refreshing tooltips, and force-freeing the bot port; also updates documentation/tests and removes duplicate/dead code for giveaways and admin categories. Sequence diagram for group command guard middleware in group chatssequenceDiagram
actor User
participant Telegram
participant BotMiddleware as Bot_GroupGuard_Middleware
participant BotCommandRouter as Bot_Command_Router
User->>Telegram: /menu in group chat
Telegram->>BotMiddleware: Update (message with text "/menu")
BotMiddleware->>BotMiddleware: Detect chatType = group/supergroup
BotMiddleware->>BotMiddleware: Extract rawCmd = "menu"
BotMiddleware->>BotMiddleware: Check GROUP_PASSTHROUGH_COMMANDS
alt Command NOT in passthrough set
BotMiddleware->>BotMiddleware: Build DM deep-link URL
BotMiddleware->>Telegram: reply("This command works in DM", inline Open DM button)
BotMiddleware-->>BotCommandRouter: Do NOT call next() (command suppressed)
else Command in passthrough set (e.g. /link)
BotMiddleware-->>BotCommandRouter: next()
BotCommandRouter->>BotCommandRouter: Route to link/giveaway/admin handler
end
Sequence diagram for batch tooltip import pendingAction flowsequenceDiagram
actor Admin
participant Telegram
participant BotCallback as Bot_Callback_Handler
participant BotTextRouter as Bot_Text_Router
participant TipsStore as Tips_Store
Admin->>Telegram: Tap "📥 Batch Import" (tips_cmd_import_batch)
Telegram->>BotCallback: CallbackQuery tips_cmd_import_batch
BotCallback->>BotCallback: clearPendingAction(user)
BotCallback->>BotCallback: user.pendingAction = { type: await_tip_import_batch, createdAt }
BotCallback->>Telegram: reply("Paste JSON array...", MarkdownV2)
Admin->>Telegram: Send JSON array of tooltip objects
Telegram->>BotTextRouter: Message with JSON text
BotTextRouter->>BotTextRouter: Load user.pendingAction
alt action.type == await_tip_import_batch
BotTextRouter->>BotTextRouter: Validate non-empty text
BotTextRouter->>BotTextRouter: JSON.parse(text)
alt Parsed value is array
loop For each item in batch
BotTextRouter->>BotTextRouter: Validate item.text string
BotTextRouter->>TipsStore: Push { id: nextTipId, text, enabled }
TipsStore->>TipsStore: Increment nextTipId
end
BotTextRouter->>BotTextRouter: user.pendingAction = null
BotTextRouter->>TipsStore: persistRuntimeState()
BotTextRouter->>TipsStore: saveHelpfulMessages()
BotTextRouter->>Telegram: reply("Imported N tooltip(s). IDs: ...")
else Invalid JSON or non-array
BotTextRouter->>Telegram: reply("❌ Invalid JSON... must be array")
end
else Other pendingAction types
BotTextRouter->>BotTextRouter: Route to other handlers
end
Class diagram for user onboarding and tooltip batch import structuresclassDiagram
class User {
String id
OnboardingState onboarding
PendingAction pendingAction
Set~String~ badges
Set~Number~ giveawayJoinedIds
Number miniAppLastSyncAt
Number profileXP
}
class OnboardingState {
Number currentStep
Number startedAt
Number completedAt
Array~Number~ stepTimestamps
Boolean completionCardShown
}
class PendingAction {
String type
Number createdAt
}
class TipsStore {
Array~Tip~ tips
Number nextTipId
persistRuntimeState()
saveHelpfulMessages()
}
class Tip {
Number id
String text
Boolean enabled
}
class GroupCommandGuard {
Set~String~ GROUP_PASSTHROUGH_COMMANDS
useMiddleware(ctx, next)
}
class OnboardingUX {
onboardingProgressBar(step)
showOnboardingPrompt(ctx, user, step)
}
User --> OnboardingState : has
User --> PendingAction : may_have
TipsStore --> Tip : contains
GroupCommandGuard ..> User : reads
OnboardingUX ..> User : reads_and_updates
class PendingActionTypes {
<<enumeration>>
await_tip_add_text
await_tip_import_batch
await_tip_edit_text
await_tip_settings_interval
await_tip_amount
}
PendingAction --> PendingActionTypes : type_value_from
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughThis PR introduces group command guard middleware that restricts certain commands to DMs, adds onboarding progress indicators and completion cards, implements admin group linking features, refactors tooltip generation scripts to use environment variables, and adds pre-startup port cleanup across deployment and development scripts. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant GroupChat as Group Chat
participant Bot
participant BotDM as Bot DM
User->>GroupChat: Sends /admin command
GroupChat->>Bot: Command intercepted
Bot->>Bot: Check if command in<br/>GROUP_PASSTHROUGH_COMMANDS
alt Command NOT in passthrough list
Bot->>GroupChat: Reply with DM prompt<br/>(includes DM link)
Bot->>Bot: Suppress group execution
User->>BotDM: Opens DM from link
BotDM->>User: Executes command in DM
else Command IS in passthrough list
Bot->>GroupChat: Execute command in group
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The
showOnboardingPromptJSDoc still documentsstepas 1–4 whileonboardingProgressBarand the completion card treat onboarding as 5 steps; updating the comments and any related copy to consistently describe the 1–5 range would avoid confusion for future maintainers. - The port-unblocking logic (lsof/fuser +
kill -9) is now duplicated in several scripts (start.sh,dev-run.sh,deploy.sh,rollback.sh,prod-run.sh); consider extracting a common helper or at least standardizing the behavior (signal, logging) in one place to reduce drift and maintenance overhead.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `showOnboardingPrompt` JSDoc still documents `step` as 1–4 while `onboardingProgressBar` and the completion card treat onboarding as 5 steps; updating the comments and any related copy to consistently describe the 1–5 range would avoid confusion for future maintainers.
- The port-unblocking logic (lsof/fuser + `kill -9`) is now duplicated in several scripts (`start.sh`, `dev-run.sh`, `deploy.sh`, `rollback.sh`, `prod-run.sh`); consider extracting a common helper or at least standardizing the behavior (signal, logging) in one place to reduce drift and maintenance overhead.
## Individual Comments
### Comment 1
<location path="index.js" line_range="5790-5797" />
<code_context>
// ── Onboarding complete — show persistent user main menu ─────────────────────
+
+ // Show a one-time completion card the first time the user reaches the main menu
+ if (!user.onboarding.completionCardShown) {
+ user.onboarding.completionCardShown = true;
+ const rwName = user.runewagerUsername ? `*${user.runewagerUsername}*` : 'your account';
+ await ctx.reply(
+ `🎉 *You're all set!*\n\n`
+ + `●●●●● Onboarding complete!\n\n`
+ + `Welcome to Runewager, ${rwName}! Here's what you can do:\n`
+ + `• 🎮 Play — launch the Runewager Mini App\n`
+ + `• 🎁 Promos — claim exclusive SC bonuses\n`
</code_context>
<issue_to_address>
**issue (bug_risk):** Escape or sanitize `runewagerUsername` before interpolating into Markdown to avoid rendering or parse issues.
Because this reply uses `parse_mode: 'Markdown'` and directly interpolates `rwName`, any Markdown-reserved characters in `runewagerUsername` (e.g. `_`, `*`, `[`, `]`) can break formatting or cause Telegram to reject the message. Please either escape the username with the existing Markdown escaper (like `escapeMarkdownV2`) or switch this message to HTML and sanitize the value there.
</issue_to_address>
### Comment 2
<location path="test/smoke.test.js" line_range="227" />
<code_context>
'(.*)', '(.+)', '(.+)?',
- '(.|\n)*', '(.|\n)+',
+ '(.|\n)*', '(.|\n)+', // literal newline form (pre-strip)
+ '(.|)*', '(.|)+', // post-whitespace-strip form of the above
'(\\.|[\\s\\S])*',
]);
</code_context>
<issue_to_address>
**suggestion (testing):** Add explicit tests for the new stripped `(.|)*` / `(.|)+` catch-all forms
The new stripped variants `(.|)*` and `(.|)+` are now in `CATCH_ALL_CORES`, but tests only cover the pre-strip forms `(.|\n)*` and `(.|\n)+`. To protect the whitespace-stripping behavior, please add assertions (e.g., in `catch-all detection recognizes supported regex forms` or a small new test) that `isCatchAllRegexPattern('(.|)*')` and `isCatchAllRegexPattern('(.|)+')` both return `true`.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| // Show a one-time completion card the first time the user reaches the main menu | ||
| if (!user.onboarding.completionCardShown) { | ||
| user.onboarding.completionCardShown = true; | ||
| const rwName = user.runewagerUsername ? `*${user.runewagerUsername}*` : 'your account'; | ||
| await ctx.reply( | ||
| `🎉 *You're all set!*\n\n` | ||
| + `●●●●● Onboarding complete!\n\n` | ||
| + `Welcome to Runewager, ${rwName}! Here's what you can do:\n` |
There was a problem hiding this comment.
issue (bug_risk): Escape or sanitize runewagerUsername before interpolating into Markdown to avoid rendering or parse issues.
Because this reply uses parse_mode: 'Markdown' and directly interpolates rwName, any Markdown-reserved characters in runewagerUsername (e.g. _, *, [, ]) can break formatting or cause Telegram to reject the message. Please either escape the username with the existing Markdown escaper (like escapeMarkdownV2) or switch this message to HTML and sanitize the value there.
| '(.*)', '(.+)', '(.+)?', | ||
| '(.|\n)*', '(.|\n)+', | ||
| '(.|\n)*', '(.|\n)+', // literal newline form (pre-strip) | ||
| '(.|)*', '(.|)+', // post-whitespace-strip form of the above |
There was a problem hiding this comment.
suggestion (testing): Add explicit tests for the new stripped (.|)* / (.|)+ catch-all forms
The new stripped variants (.|)* and (.|)+ are now in CATCH_ALL_CORES, but tests only cover the pre-strip forms (.|\n)* and (.|\n)+. To protect the whitespace-stripping behavior, please add assertions (e.g., in catch-all detection recognizes supported regex forms or a small new test) that isCatchAllRegexPattern('(.|)*') and isCatchAllRegexPattern('(.|)+') both return true.
Nitpicks 🔍
|
| # --------------------------------------------------------- | ||
| # 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 | tr -d '"' | tr -d "'" | tr -d $'\r')" |
There was a problem hiding this comment.
Suggestion: When PORT in .env includes an inline comment (e.g., PORT=3000 # dev), the current parsing stores the entire 3000 # dev string in DEPLOY_PORT, so the lsof/fuser call cannot interpret it as a valid port and the script silently fails to kill the blocking process, potentially leaving the deploy unable to start the bot on the intended port. [logic error]
Severity Level: Major ⚠️
- ❌ Deploy script may miss processes blocking configured bot port.
- ⚠️ Bot restart via /deploy can fail to bind port.
- ⚠️ Admin may see deploy complete while bot unreachable.| DEPLOY_PORT="$(grep -E '^PORT=' "$PROJECT_DIR/.env" 2>/dev/null | head -1 | cut -d= -f2 | tr -d '"' | tr -d "'" | tr -d $'\r')" | |
| DEPLOY_PORT="$(grep -E '^PORT=' "$PROJECT_DIR/.env" 2>/dev/null \ | |
| | head -1 \ | |
| | cut -d= -f2- \ | |
| | sed 's/[[:space:]]*#.*$//' \ | |
| | tr -d '"' \ | |
| | tr -d "'" \ | |
| | tr -d $'\r')" |
Steps of Reproduction ✅
1. On the VPS, edit the environment file `/workspace/Runewager/.env` (same directory
`PROJECT_DIR` used by `deploy.sh` at `/workspace/Runewager/deploy.sh:26`) and set the port
with an inline comment, e.g. `PORT=3000 # production port`.
2. Ensure a Node/bot process is already listening on TCP port 3000 (e.g. via a previous
manual `node index.js` run); this is what the guard at
`/workspace/Runewager/deploy.sh:233-249` is meant to detect and kill.
3. Trigger a deploy using the bot's `/deploy` command, which (per
`/workspace/Runewager/index.js:6855-6873` from Grep) spawns `deploy.sh` to handle git,
dependencies, and service restart.
4. During step "3c) Kill anything blocking the bot port before starting" at
`/workspace/Runewager/deploy.sh:233-249`, the current code on lines 235-236 parses
`DEPLOY_PORT` using `cut -d= -f2`, yielding the literal string `3000 # production port`;
`lsof -ti :"$DEPLOY_PORT"` and `fuser -n tcp "$DEPLOY_PORT"` then fail to match the real
port so `_BLOCKING` stays empty, no PIDs are killed, and the subsequent `systemctl start
"${APP_NAME}.service"` at line 257 can fail to bind to port 3000, leaving the new bot
service not running on the intended port.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** deploy.sh
**Line:** 235:235
**Comment:**
*Logic Error: When `PORT` in `.env` includes an inline comment (e.g., `PORT=3000 # dev`), the current parsing stores the entire `3000 # dev` string in `DEPLOY_PORT`, so the lsof/fuser call cannot interpret it as a valid port and the script silently fails to kill the blocking process, potentially leaving the deploy unable to start the bot 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.| 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 | tr -d '"' | tr -d "'" || true) |
There was a problem hiding this comment.
Suggestion: Similar to the deploy script, if PORT in .env includes an inline comment (e.g., PORT=3000 # dev), the current parsing leaves the comment in DEV_PORT, so the lsof/fuser invocation cannot parse it as a valid port and the script may fail to kill the process blocking the dev server port, causing confusing startup failures. [logic error]
Severity Level: Major ⚠️
- ❌ dev-run.sh fails to kill process blocking dev port.
- ⚠️ Local dev `node index.js` may fail with EADDRINUSE.
- ⚠️ Developers must manually diagnose and free blocked ports.| DEV_PORT=$(grep -E '^PORT=' "$ROOT_DIR/.env" 2>/dev/null | head -1 | cut -d= -f2 | tr -d '"' | tr -d "'" || true) | |
| DEV_PORT=$(grep -E '^PORT=' "$ROOT_DIR/.env" 2>/dev/null \ | |
| | head -1 \ | |
| | cut -d= -f2- \ | |
| | sed 's/[[:space:]]*#.*$//' \ | |
| | tr -d '"' \ | |
| | tr -d "'" || true) |
Steps of Reproduction ✅
1. In the project root `/workspace/Runewager`, create or edit `.env` so the port line
includes an inline comment, e.g. `PORT=3000 # dev`, matching the pattern read by
`dev-run.sh:37` (`grep -E '^PORT=' "$ROOT_DIR/.env"`).
2. Start a process that listens on TCP port 3000, e.g. by running `node index.js` once and
leaving it running, so that port 3000 is already in use before running the dev script.
3. Run the dev helper script `./dev-run.sh`; after the Node version check at
`dev-run.sh:15-20`, the script parses the port at `dev-run.sh:37-38`, assigning
`DEV_PORT="3000 # dev"` because `cut -d= -f2` returns the entire `3000 # dev` value
without stripping the comment.
4. The script then attempts to kill processes on that port via `lsof`/`fuser` at
`dev-run.sh:39-42`, but because `DEV_PORT` contains `3000 # dev`, the arguments to
`lsof`/`fuser` are malformed and no process on TCP port 3000 is actually killed; the
subsequent `exec node index.js` at `dev-run.sh:52` may then fail to bind with an
`EADDRINUSE` error even though `dev-run.sh` claimed to free the port.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** dev-run.sh
**Line:** 37:37
**Comment:**
*Logic Error: Similar to the deploy script, if `PORT` in `.env` includes an inline comment (e.g., `PORT=3000 # dev`), the current parsing leaves the comment in `DEV_PORT`, so the lsof/fuser invocation cannot parse it as a valid port and the script may fail to kill the process blocking the dev server port, causing confusing startup failures.
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 3000 (or PORT) | ||
| ############################################### | ||
| BOT_PORT="$(grep -E '^PORT=' "$PROJECT_DIR/.env" 2>/dev/null | head -1 | cut -d= -f2 | tr -d '"' | tr -d "'" | tr -d $'\r')" | ||
| BOT_PORT="${BOT_PORT:-3000}" | ||
| if command -v lsof >/dev/null 2>&1; then | ||
| _PIDS="$(lsof -ti :"$BOT_PORT" 2>/dev/null || true)" | ||
| elif command -v fuser >/dev/null 2>&1; then | ||
| _PIDS="$(fuser -n tcp "$BOT_PORT" 2>/dev/null | tr ' ' '\n' | sed '/^$/d' || true)" | ||
| fi | ||
| if [[ -n "${_PIDS:-}" ]]; then | ||
| echo "⚠️ Port $BOT_PORT blocked — killing before start..." |
There was a problem hiding this comment.
Suggestion: The start script currently kills every process listening on the configured port (default 3000) using kill -9, regardless of whether it belongs to this bot or some unrelated service, which can unexpectedly terminate other applications using that port; it should restrict termination to processes actually running this project's index.js. [logic error]
Severity Level: Critical 🚨
- ❌ start.sh can SIGKILL unrelated services on PORT/3000.
- ⚠️ Local dev tools on port 3000 abruptly terminated.
- ⚠️ Shared VPS may kill non-Runewager apps unintentionally.| # Kill anything blocking port 3000 (or PORT) | |
| ############################################### | |
| BOT_PORT="$(grep -E '^PORT=' "$PROJECT_DIR/.env" 2>/dev/null | head -1 | cut -d= -f2 | tr -d '"' | tr -d "'" | tr -d $'\r')" | |
| BOT_PORT="${BOT_PORT:-3000}" | |
| if command -v lsof >/dev/null 2>&1; then | |
| _PIDS="$(lsof -ti :"$BOT_PORT" 2>/dev/null || true)" | |
| elif command -v fuser >/dev/null 2>&1; then | |
| _PIDS="$(fuser -n tcp "$BOT_PORT" 2>/dev/null | tr ' ' '\n' | sed '/^$/d' || true)" | |
| fi | |
| if [[ -n "${_PIDS:-}" ]]; then | |
| echo "⚠️ Port $BOT_PORT blocked — killing before start..." | |
| # Kill any stale bot process on port 3000 (or PORT) | |
| ############################################### | |
| BOT_PORT="$(grep -E '^PORT=' "$PROJECT_DIR/.env" 2>/dev/null | head -1 | cut -d= -f2 | tr -d '"' | tr -d "'" | tr -d $'\r')" | |
| BOT_PORT="${BOT_PORT:-3000}" | |
| _PIDS="" | |
| if command -v lsof >/dev/null 2>&1; then | |
| # Limit to node processes, then filter to this project's index.js | |
| _CANDIDATES="$(lsof -ti :"$BOT_PORT" -a -c node 2>/dev/null || true)" | |
| for _pid in $_CANDIDATES; do | |
| if ps -o args= -p "$_pid" 2>/dev/null | grep -q "$PROJECT_DIR/index.js"; then | |
| _PIDS+=" ${_pid}" | |
| fi | |
| done | |
| elif command -v fuser >/dev/null 2>&1; then | |
| # fuser has no easy command filter; rely on later ps/grep filter | |
| _CANDIDATES="$(fuser -n tcp "$BOT_PORT" 2>/dev/null | tr ' ' '\n' | sed '/^$/d' || true)" | |
| for _pid in $_CANDIDATES; do | |
| if ps -o args= -p "$_pid" 2>/dev/null | grep -q "$PROJECT_DIR/index.js"; then | |
| _PIDS+=" ${_pid}" | |
| fi | |
| done | |
| fi | |
| if [[ -n "${_PIDS:-}" ]]; then | |
| echo "⚠️ Port $BOT_PORT blocked by stale bot process — killing before start..." |
Steps of Reproduction ✅
1. On a host with the repo checked out at `/workspace/Runewager`, ensure `.env` exists
with `PORT=3000` or omit `PORT` so the default `3000` is used (start.sh reads this at
`start.sh:64`).
2. Start any non-Runewager process listening on TCP port 3000, for example a local dev
server, then verify it is bound to port 3000 (this external process is not referenced in
the repo but only needs to listen on the configured port).
3. From the project root, execute the entry script `/workspace/Runewager/start.sh`
(top-level script at `start.sh:1`, designed to be run directly as the hardened start
script per its header comments).
4. During execution, the block at `start.sh:61-75` runs: it resolves `BOT_PORT`, discovers
the listener PIDs via `lsof` or `fuser`, and unconditionally executes `kill -9` on each
PID; the previously started unrelated service on port 3000 is forcibly terminated even
though it is not a Runewager `index.js` process.Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** start.sh
**Line:** 62:72
**Comment:**
*Logic Error: The start script currently kills every process listening on the configured port (default 3000) using `kill -9`, regardless of whether it belongs to this bot or some unrelated service, which can unexpectedly terminate other applications using that port; it should restrict termination to processes actually running this project's `index.js`.
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. |
- 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
User description
Summary by Sourcery
Add group command guard middleware, enhance onboarding UX, and improve deployment/startup robustness while addressing PR review and audit issues.
New Features:
Enhancements:
Build:
Deployment:
Documentation:
Tests:
Chores:
CodeAnt-AI Description
Redirect group commands to DM, add onboarding progress header and one-time completion card
What Changed
Impact
✅ Fewer group-command misfires in public chats✅ Clearer onboarding progress and a single welcome card after completion✅ Faster, more reliable restarts when the bot port is already in use💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit
New Features
Chores