Add menu lifecycle, pending-action timeout, referral onboarding, group-linking tools, Discord/link helpers, and tests - #102
Merged
gamblecodezcom merged 2 commits intoFeb 27, 2026
Conversation
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThe PR consolidates launcher and routing logic by removing the Changes
Possibly related PRs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 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 |
codeant-ai
Bot
added
the
size:XL
Nitpicks 🔍
|
|
CodeAnt AI finished reviewing your PR. |
Owner
Author
|
@sourcery-ai review |
…for-runewager-bot
|
CodeAnt AI is running Incremental review Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
gamblecodezcom
deleted the
codex/perform-full-audit-and-implementation-for-runewager-bot
branch
February 27, 2026 12:58
Owner
Author
|
@sourcery-ai review |
Reviewer's GuideImplements settings-aware play launch helpers, simplifies pending-action timeout evaluation, updates referral share copy, introduces a unified play-launch button for menus/quick commands, tightens catch-all regex detection in smoke tests, adjusts privacy-mode env parsing for backward compatibility, and updates tests/docs to reflect the new behavior and environment contracts. Sequence diagram for settings-aware play launch button behaviorsequenceDiagram
actor User
participant TelegramClient
participant Bot
participant Helpers
User->>TelegramClient: Tap Play button in menu
TelegramClient->>Bot: callback menu_main/play (or quick command)
Bot->>Bot: load user from store
Bot->>Helpers: playLaunchButton(user, textBrowser, textMini, route)
Helpers->>Helpers: inBrowser = user.settings.playMode == browser
alt User in browser mode
Helpers-->>Bot: Markup.button.url(textBrowser, getBrowserLink(route))
else User in miniapp mode or unset
Helpers->>Helpers: getWebAppLink(route)
Helpers-->>Bot: Markup.button.webApp(textMini, webAppUrl)
end
Bot-->>TelegramClient: reply("🎮 Open Runewager", inlineKeyboard[playLaunchButton, BackToMain])
TelegramClient-->>User: Render keyboard with browser or mini-app Play button
Class diagram for play link and launch helper structureclassDiagram
class User {
String id
Settings settings
}
class Settings {
String playMode // browser or miniapp
}
class IndexHelpers {
+getBrowserLink(route)
+getWebAppLink(route)
+getStartAppLink(route)
+getPlayLink(user, route)
+playLaunchButton(user, textBrowser, textMini, route)
+miniAppPlayButton(user, route)
}
class MarkupHelpers {
+button_url(text, url)
+button_webApp(text, url)
}
User --> Settings : has
IndexHelpers ..> User : reads settings.playMode
IndexHelpers ..> Settings : uses playMode
IndexHelpers ..> MarkupHelpers : creates buttons
IndexHelpers : getBrowserLink(route)
IndexHelpers : getWebAppLink(route)
IndexHelpers : getStartAppLink(route)
IndexHelpers : getPlayLink(user, route)
IndexHelpers : playLaunchButton(user, textBrowser, textMini, route)
IndexHelpers : miniAppPlayButton(user, route)
IndexHelpers ..> IndexHelpers : getPlayLink calls
IndexHelpers ..> IndexHelpers : playLaunchButton calls getBrowserLink
IndexHelpers ..> IndexHelpers : playLaunchButton calls getWebAppLink
IndexHelpers ..> IndexHelpers : miniAppPlayButton calls playLaunchButton
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
codeant-ai
Bot
added
size:M
|
CodeAnt AI Incremental review completed. |
![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 7 issues, and left some high level feedback:
- The change to
referralShareHTMLto interpolatecodedirectly rather than viaescapeHtmlreintroduces an HTML injection surface in Telegram messages (e.g., codes containing</>), so consider restoring escaping or otherwise constraining/validating referral codes before rendering them into HTML. - The simplification of
evaluatePendingActionTimeoutremoves the normalization/repair logic for non‑numericcreatedAtvalues, which means a corrupted or stringified timestamp will now always be treated as expired; consider re-adding a small normalization step so badcreatedAtvalues are healed rather than silently expiring the pending action. - The new
isCatchAllRegexPatternimplementation now only recognizes a very narrow set of literal patterns (.*,^.*$,.+,^.+$); if any existing catch-all callbacks use grouped or slightly more complex forms, they will stop being detected—either expand this helper to cover those shapes or assert in tests that the codebase only uses the supported literal forms.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The change to `referralShareHTML` to interpolate `code` directly rather than via `escapeHtml` reintroduces an HTML injection surface in Telegram messages (e.g., codes containing `<`/`>`), so consider restoring escaping or otherwise constraining/validating referral codes before rendering them into HTML.
- The simplification of `evaluatePendingActionTimeout` removes the normalization/repair logic for non‑numeric `createdAt` values, which means a corrupted or stringified timestamp will now always be treated as expired; consider re-adding a small normalization step so bad `createdAt` values are healed rather than silently expiring the pending action.
- The new `isCatchAllRegexPattern` implementation now only recognizes a very narrow set of literal patterns (`.*`, `^.*$`, `.+`, `^.+$`); if any existing catch-all callbacks use grouped or slightly more complex forms, they will stop being detected—either expand this helper to cover those shapes or assert in tests that the codebase only uses the supported literal forms.
## Individual Comments
### Comment 1
<location path="index.js" line_range="74" />
<code_context>
- const button = getPlayButton(user);
- if (button.url) return Markup.button.url(button.text, button.url);
- return Markup.button.webApp(button.text, button.web_app.url);
+ const mode = user && user.settings ? user.settings.playMode : 'miniapp';
+ return mode === 'browser' ? getBrowserLink(route) : getStartAppLink(route);
}
</code_context>
<issue_to_address>
**question (bug_risk):** Consider preserving the previous `user.playMode` fallback and normalization logic for play mode selection.
The previous implementation used `user.playMode` as a fallback when `user.settings` was missing and normalized values to `'browser'` or `'miniapp'`. The new code only reads `user.settings.playMode` and otherwise defaults to `'miniapp'`, changing behavior for users relying on `user.playMode` or non-standard values. If this behavior change isn’t deliberate, please restore the earlier fallback/normalization while keeping the simplified structure.
</issue_to_address>
### Comment 2
<location path="index.js" line_range="519-521" />
<code_context>
- user.pendingAction.createdAt = safeNow;
- return { hadPending: true, expired: false, expiredType: null };
- }
+ if (!user.pendingAction.createdAt) user.pendingAction.createdAt = now;
- if ((safeNow - normalizedCreatedAt) <= PENDING_ACTION_TIMEOUT_MS) {
+ if ((now - Number(user.pendingAction.createdAt || 0)) <= PENDING_ACTION_TIMEOUT_MS) {
return { hadPending: true, expired: false, expiredType: null };
}
</code_context>
<issue_to_address>
**issue (bug_risk):** The simplified timeout calculation can misbehave when `createdAt` is non‑numeric or `now` is not a finite number.
Previously we normalized `now`/`createdAt` and treated non‑finite `createdAt` as "no timeout yet". The new code only sets `createdAt` when falsy and then does `now - Number(user.pendingAction.createdAt || 0)`. For any truthy, non‑numeric `createdAt` (e.g., object, non‑numeric string), `Number(...)` becomes `NaN`, so the subtraction and `<=` comparison are `NaN` and the action is always considered expired. A non‑numeric `now` has the same effect. Please add back a minimal normalization/`Number.isFinite` check to avoid accidental early expiry when data is malformed.
</issue_to_address>
### Comment 3
<location path="index.js" line_range="4747" />
<code_context>
Daily giveaways, instant rewards, drops, and a secret new-user bonus waiting inside the bot.
-Use my code <b>${escapeHtml(code)}</b> when you start — we BOTH get a <b>2× boost on all giveaways for 7 days</b>.
+Use my code <b>${code}</b> when you start — we BOTH get a <b>2× boost on all giveaways for 7 days</b>.
👉 <a href="https://t.me/RunewagerBot">Tap here to join Runewager</a>
</code_context>
<issue_to_address>
**🚨 issue (security):** Removing `escapeHtml` on `code` introduces an HTML injection / formatting risk in the referral message.
The previous `escapeHtml(code)` call prevented codes with `<`, `>`, `&`, or quotes from breaking the markup or injecting tags. Now `code` is interpolated directly into HTML, so a malicious or malformed value could alter the rendered content. Unless `code` is strictly validated to only allow safe characters, this should continue to be HTML-escaped here.
</issue_to_address>
### Comment 4
<location path="index.js" line_range="3803" />
<code_context>
/** Keyboard for the new persistent USER MAIN MENU */
function userMainMenuKeyboard(isAdminUser, user = null) {
+ const browserMode = user && user.settings && user.settings.playMode === 'browser';
const rows = [
- [playButtonMarkup(user)],
</code_context>
<issue_to_address>
**nitpick:** `browserMode` is computed but not used in `userMainMenuKeyboard`.
`browserMode` is declared but never used in `userMainMenuKeyboard`. If it’s no longer needed after moving this logic into `playLaunchButton`, remove it; if it’s meant to control the keyboard layout, hook it into that branching logic instead.
</issue_to_address>
### Comment 5
<location path="test/smoke.test.js" line_range="555-564" />
<code_context>
+test('persistent user menu play button is settings-aware and supports browser mode', () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Play button smoke test doesn’t fully exercise all callsites and routes of the new playLaunchButton helper
The new helper is also used by the quick‑play callback and on other routes that aren’t covered here. To make this a stronger guard on the new behavior, please:
- Assert that the quick‑play callback (`menu_qc_play`) uses `playLaunchButton` or `miniAppPlayButton`, and
- If there’s route‑specific behavior (e.g. `profile`, `claim`), add a small smoke test that those routes are passed into `playLaunchButton`/`miniAppPlayButton` as expected.
Suggested implementation:
```javascript
test('persistent user menu play button is settings-aware and supports browser mode', () => {
const source = fs.readFileSync(path.resolve(__dirname, '..', 'index.js'), 'utf8');
// Ensure quick-play callback is wired through the new helper(s)
assert.match(
source,
/menu_qc_play[\s\S]*?(playLaunchButton|miniAppPlayButton)/,
'menu_qc_play should delegate to playLaunchButton or miniAppPlayButton'
);
// Ensure profile route is wired through the new helper(s)
assert.match(
source,
/['"`]profile['"`][\s\S]*?(playLaunchButton|miniAppPlayButton)/,
'profile route should delegate to playLaunchButton or miniAppPlayButton'
);
// Ensure claim route is wired through the new helper(s)
assert.match(
source,
/['"`]claim['"`][\s\S]*?(playLaunchButton|miniAppPlayButton)/,
'claim route should delegate to playLaunchButton or miniAppPlayButton'
);
```
- If the `menu_qc_play`, `profile`, or `claim` behaviors live in a different entry file (not `index.js`), adjust the `fs.readFileSync` target accordingly or add an additional `source` read for that file.
- If the code structure separates route configuration from the helpers (e.g., routes in one module, handlers in another), you may want to tighten the regexes to match the actual wiring (e.g., `menu_qc_play:.*playLaunchButton` or `router\.get\('/profile',.*playLaunchButton`), once you confirm the concrete callsites in the codebase.
</issue_to_address>
### Comment 6
<location path="test/unit.test.js" line_range="452" />
<code_context>
-test('browser mode returns external runewager affiliate link only for play', () => {
+test('browser mode returns external gamble-codez link only', () => {
const user = { settings: { playMode: 'browser' } };
const out = getPlayLink(user, 'play');
</code_context>
<issue_to_address>
**suggestion (testing):** Play link unit tests miss edge cases for default/unknown playMode and route behavior
To better cover this logic, consider adding:
- Tests for when `user` is null or lacks `settings.playMode`, asserting the default `miniapp` behavior.
- A table-driven test over all supported routes in browser mode (play, profile, claim, affiliate) plus an unknown route to confirm the intended fallback.
This will clarify the routing contract and reduce risk of regressions around play‑mode handling.
</issue_to_address>
### Comment 7
<location path="test/unit.test.js" line_range="506-515" />
<code_context>
-function escapeHtml(text) {
- return String(text || '')
- .replace(/&/g, '&')
</code_context>
<issue_to_address>
**question (testing):** Referral share HTML behavior is no longer explicitly specified by tests
The old `referralShareHTML` test that verified HTML-sensitive referral codes were escaped has been removed, and the new implementation now interpolates `code` directly into HTML without any explicit behavior defined by tests.
Please add a focused test that asserts the intended behavior (e.g., that `<img>` or other HTML-like codes are either preserved as raw HTML or escaped). This will make the security/UX choice here explicit and prevent future regressions in how referral codes are rendered.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| const button = getPlayButton(user); | ||
| if (button.url) return Markup.button.url(button.text, button.url); | ||
| return Markup.button.webApp(button.text, button.web_app.url); | ||
| const mode = user && user.settings ? user.settings.playMode : 'miniapp'; |
There was a problem hiding this comment.
question (bug_risk): Consider preserving the previous user.playMode fallback and normalization logic for play mode selection.
The previous implementation used user.playMode as a fallback when user.settings was missing and normalized values to 'browser' or 'miniapp'. The new code only reads user.settings.playMode and otherwise defaults to 'miniapp', changing behavior for users relying on user.playMode or non-standard values. If this behavior change isn’t deliberate, please restore the earlier fallback/normalization while keeping the simplified structure.
Comment on lines
+519
to
+521
| if (!user.pendingAction.createdAt) user.pendingAction.createdAt = now; | ||
|
|
||
| if ((safeNow - normalizedCreatedAt) <= PENDING_ACTION_TIMEOUT_MS) { | ||
| if ((now - Number(user.pendingAction.createdAt || 0)) <= PENDING_ACTION_TIMEOUT_MS) { |
There was a problem hiding this comment.
issue (bug_risk): The simplified timeout calculation can misbehave when createdAt is non‑numeric or now is not a finite number.
Previously we normalized now/createdAt and treated non‑finite createdAt as "no timeout yet". The new code only sets createdAt when falsy and then does now - Number(user.pendingAction.createdAt || 0). For any truthy, non‑numeric createdAt (e.g., object, non‑numeric string), Number(...) becomes NaN, so the subtraction and <= comparison are NaN and the action is always considered expired. A non‑numeric now has the same effect. Please add back a minimal normalization/Number.isFinite check to avoid accidental early expiry when data is malformed.
| Daily giveaways, instant rewards, drops, and a secret new-user bonus waiting inside the bot. | ||
|
|
||
| Use my code <b>${escapeHtml(code)}</b> when you start — we BOTH get a <b>2× boost on all giveaways for 7 days</b>. | ||
| Use my code <b>${code}</b> when you start — we BOTH get a <b>2× boost on all giveaways for 7 days</b>. |
There was a problem hiding this comment.
🚨 issue (security): Removing escapeHtml on code introduces an HTML injection / formatting risk in the referral message.
The previous escapeHtml(code) call prevented codes with <, >, &, or quotes from breaking the markup or injecting tags. Now code is interpolated directly into HTML, so a malicious or malformed value could alter the rendered content. Unless code is strictly validated to only allow safe characters, this should continue to be HTML-escaped here.
|
|
||
| /** Keyboard for the new persistent USER MAIN MENU */ | ||
| function userMainMenuKeyboard(isAdminUser, user = null) { | ||
| const browserMode = user && user.settings && user.settings.playMode === 'browser'; |
There was a problem hiding this comment.
nitpick: browserMode is computed but not used in userMainMenuKeyboard.
browserMode is declared but never used in userMainMenuKeyboard. If it’s no longer needed after moving this logic into playLaunchButton, remove it; if it’s meant to control the keyboard layout, hook it into that branching logic instead.
Comment on lines
+555
to
+564
| test('persistent user menu play button is settings-aware and supports browser mode', () => { | ||
| const source = fs.readFileSync(path.resolve(__dirname, '..', 'index.js'), 'utf8'); | ||
| assert.ok(source.includes('function getPlayButton('), 'Expected unified getPlayButton helper'); | ||
| const helperStart = source.indexOf('function getPlayButton('); | ||
| const helperBlock = source.slice(helperStart, helperStart + 650); | ||
| assert.ok(helperBlock.includes("url: 'https://runewager.com/r/GambleCodez'"), 'Expected browser mode external URL'); | ||
| assert.ok(helperBlock.includes("web_app: { url: 'https://t.me/RuneWager_bot/Play' }"), 'Expected mini app web_app launch'); | ||
| assert.ok(!helperBlock.includes('startapp='), 'Expected no startapp launch in getPlayButton helper'); | ||
| }); | ||
|
|
||
| test('all menu play buttons use unified getPlayButton path with no hardcoded mixed mode', () => { | ||
| const source = fs.readFileSync(path.resolve(__dirname, '..', 'index.js'), 'utf8'); | ||
| assert.ok(source.includes('function playButtonMarkup('), 'Expected helper to convert getPlayButton to Markup button'); | ||
|
|
||
| const playButtonUsageCount = (source.match(/\[playButtonMarkup\(user\)\]/g) || []).length; | ||
| assert.ok(playButtonUsageCount >= 2, 'Expected both main and persistent menus to use unified play button helper'); | ||
|
|
||
| const quickStart = source.indexOf("bot.action('menu_qc_play'"); | ||
| assert.ok(quickStart >= 0, 'Expected quick-play callback'); | ||
| const quickBlock = source.slice(quickStart, quickStart + 350); | ||
| assert.ok(quickBlock.includes('playButtonMarkup(user)'), 'Expected quick-play to use unified play button helper'); | ||
| }); | ||
|
|
||
| test('settings play mode toggle persists, clears old menus, and re-renders settings', () => { | ||
| const source = fs.readFileSync(path.resolve(__dirname, '..', 'index.js'), 'utf8'); | ||
| const start = source.indexOf("bot.action('settings_toggle_playmode'"); | ||
| assert.ok(start >= 0, 'Expected settings_toggle_playmode callback'); | ||
| assert.ok(source.includes('function playLaunchButton('), 'Expected shared playLaunchButton helper'); | ||
| const helperStart = source.indexOf('function playLaunchButton('); | ||
| const helperBlock = source.slice(helperStart, helperStart + 700); | ||
| assert.ok(helperBlock.includes('Markup.button.url'), 'Expected browser-mode URL launch button'); | ||
| assert.ok(helperBlock.includes('Markup.button.webApp'), 'Expected mini-app webApp launch button'); | ||
|
|
||
| const start = source.indexOf('function userMainMenuKeyboard('); | ||
| assert.ok(start >= 0, 'Expected userMainMenuKeyboard'); |
There was a problem hiding this comment.
suggestion (testing): Play button smoke test doesn’t fully exercise all callsites and routes of the new playLaunchButton helper
The new helper is also used by the quick‑play callback and on other routes that aren’t covered here. To make this a stronger guard on the new behavior, please:
- Assert that the quick‑play callback (
menu_qc_play) usesplayLaunchButtonorminiAppPlayButton, and - If there’s route‑specific behavior (e.g.
profile,claim), add a small smoke test that those routes are passed intoplayLaunchButton/miniAppPlayButtonas expected.
Suggested implementation:
test('persistent user menu play button is settings-aware and supports browser mode', () => {
const source = fs.readFileSync(path.resolve(__dirname, '..', 'index.js'), 'utf8');
// Ensure quick-play callback is wired through the new helper(s)
assert.match(
source,
/menu_qc_play[\s\S]*?(playLaunchButton|miniAppPlayButton)/,
'menu_qc_play should delegate to playLaunchButton or miniAppPlayButton'
);
// Ensure profile route is wired through the new helper(s)
assert.match(
source,
/['"`]profile['"`][\s\S]*?(playLaunchButton|miniAppPlayButton)/,
'profile route should delegate to playLaunchButton or miniAppPlayButton'
);
// Ensure claim route is wired through the new helper(s)
assert.match(
source,
/['"`]claim['"`][\s\S]*?(playLaunchButton|miniAppPlayButton)/,
'claim route should delegate to playLaunchButton or miniAppPlayButton'
);- If the
menu_qc_play,profile, orclaimbehaviors live in a different entry file (notindex.js), adjust thefs.readFileSynctarget accordingly or add an additionalsourceread for that file. - If the code structure separates route configuration from the helpers (e.g., routes in one module, handlers in another), you may want to tighten the regexes to match the actual wiring (e.g.,
menu_qc_play:.*playLaunchButtonorrouter\.get\('/profile',.*playLaunchButton), once you confirm the concrete callsites in the codebase.
|
|
||
|
|
||
| test('browser mode returns external runewager affiliate link only for play', () => { | ||
| test('browser mode returns external gamble-codez link only', () => { |
There was a problem hiding this comment.
suggestion (testing): Play link unit tests miss edge cases for default/unknown playMode and route behavior
To better cover this logic, consider adding:
- Tests for when
useris null or lackssettings.playMode, asserting the defaultminiappbehavior. - A table-driven test over all supported routes in browser mode (play, profile, claim, affiliate) plus an unknown route to confirm the intended fallback.
This will clarify the routing contract and reduce risk of regressions around play‑mode handling.
There was a problem hiding this comment.
Hey - I've found 7 issues, and left some high level feedback:
- The change to
referralShareHTML(and removal ofescapeHtml) means referral codes are now interpolated directly into HTML without escaping, which allows malicious codes to inject arbitrary markup into messages; consider restoring HTML escaping for the referral code before embedding it. evaluatePendingActionTimeoutnow assumes a numericnowand only seedscreatedAtwhen it is falsy; ifcreatedAtis a non-numeric truthy value,Number(createdAt)becomesNaNand the timeout logic will treat it as immediately expired—consider reintroducing basic numeric validation for bothnowandcreatedAtto keep behavior predictable and resilient to corrupt state.getPlayLinkno longer falls back touser.playModeand instead defaults to'miniapp'whenuser.settings.playModeis missing, which will silently change behavior for any legacy users stored withplayModeat the top level; if that legacy shape still exists in production data, you may want to preserve the old fallback logic during the transition.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The change to `referralShareHTML` (and removal of `escapeHtml`) means referral codes are now interpolated directly into HTML without escaping, which allows malicious codes to inject arbitrary markup into messages; consider restoring HTML escaping for the referral code before embedding it.
- `evaluatePendingActionTimeout` now assumes a numeric `now` and only seeds `createdAt` when it is falsy; if `createdAt` is a non-numeric truthy value, `Number(createdAt)` becomes `NaN` and the timeout logic will treat it as immediately expired—consider reintroducing basic numeric validation for both `now` and `createdAt` to keep behavior predictable and resilient to corrupt state.
- `getPlayLink` no longer falls back to `user.playMode` and instead defaults to `'miniapp'` when `user.settings.playMode` is missing, which will silently change behavior for any legacy users stored with `playMode` at the top level; if that legacy shape still exists in production data, you may want to preserve the old fallback logic during the transition.
## Individual Comments
### Comment 1
<location path="index.js" line_range="73-74" />
<code_context>
}
function getPlayLink(user, route = 'play') {
- return getPlayMode(user) === 'browser' ? getBrowserLink(route) : getStartAppLink(route);
-}
-
-function getPlayButton(user = null) {
- if (getPlayMode(user) === 'browser') {
- return {
- text: '🔵 Play & Win',
- url: 'https://runewager.com/r/GambleCodez',
- };
- }
- return {
- text: '🔵 Play & Win',
- web_app: { url: 'https://t.me/RuneWager_bot/Play' },
- };
-}
-
-function playButtonMarkup(user = null) {
- const button = getPlayButton(user);
- if (button.url) return Markup.button.url(button.text, button.url);
- return Markup.button.webApp(button.text, button.web_app.url);
+ const mode = user && user.settings ? user.settings.playMode : 'miniapp';
+ return mode === 'browser' ? getBrowserLink(route) : getStartAppLink(route);
}
</code_context>
<issue_to_address>
**issue:** Revised play-mode resolution ignores legacy/user-level `playMode` and may change behavior for existing users.
Previously `getPlayMode` fell back to `user.playMode` when `user.settings.playMode` was missing; the new logic only checks `user.settings.playMode` (or defaults to `'miniapp'`). To preserve existing behavior for records that still use the legacy field, consider:
```js
const mode = user && user.settings
? (user.settings.playMode ?? user.playMode)
: (user && user.playMode) || 'miniapp';
```
If `user.playMode` is guaranteed to be unused in production, this change is fine—but it would be good to confirm that assumption against existing data first.
</issue_to_address>
### Comment 2
<location path="index.js" line_range="519-521" />
<code_context>
- user.pendingAction.createdAt = safeNow;
- return { hadPending: true, expired: false, expiredType: null };
- }
+ if (!user.pendingAction.createdAt) user.pendingAction.createdAt = now;
- if ((safeNow - normalizedCreatedAt) <= PENDING_ACTION_TIMEOUT_MS) {
+ if ((now - Number(user.pendingAction.createdAt || 0)) <= PENDING_ACTION_TIMEOUT_MS) {
return { hadPending: true, expired: false, expiredType: null };
}
</code_context>
<issue_to_address>
**issue (bug_risk):** The simplified timeout logic assumes `now` and `createdAt` are always numeric and may misbehave if non-numeric values are passed.
Previously, both `now` and `createdAt` were normalized with `Number()` and validated with `Number.isFinite`, falling back to `Date.now()` when invalid. The new version:
- Uses `now` directly in arithmetic without validation.
- Coerces `createdAt` with `Number()` but doesn’t handle `NaN` specially.
If `now` or `createdAt` are non-numeric (e.g., unexpected types), `Number(createdAt)` may be `NaN`, making the comparison always `false` and incorrectly treating actions as expired. To retain robustness, consider reintroducing lightweight validation:
```js
const nowMs = Number(now);
const safeNow = Number.isFinite(nowMs) ? nowMs : Date.now();
if (!user.pendingAction.createdAt) {
user.pendingAction.createdAt = safeNow;
}
const createdMs = Number(user.pendingAction.createdAt);
if (!Number.isFinite(createdMs)) {
user.pendingAction.createdAt = safeNow;
return { hadPending: true, expired: false, expiredType: null };
}
if ((safeNow - createdMs) <= PENDING_ACTION_TIMEOUT_MS) {
// ...
}
```
</issue_to_address>
### Comment 3
<location path="test/smoke.test.js" line_range="210-212" />
<code_context>
* Supports `.*`, `^.*$`, and `.+` with optional grouping/anchors.
*/
function isCatchAllRegexPattern(patternSource) {
- let compact = String(patternSource || '').replace(/\s+/g, '');
- compact = compact.replace(/^\^/, '').replace(/\$$/, '');
- while (compact.startsWith('(') && compact.endsWith(')')) compact = compact.slice(1, -1);
- compact = compact.replace(/^\?:/, '');
-
- if (['.*', '.+', '.*?', '.+?'].includes(compact)) return true;
- if (/^\(\?:?\.?\*\)$/.test(String(patternSource || '').replace(/\s+/g, ''))) return true;
- return false;
+ const compact = String(patternSource || '').replace(/\s+/g, '');
+ return compact === '.*' || compact === '^.*$' || compact === '.+' || compact === '^.+$';
}
</code_context>
<issue_to_address>
**issue:** Catch-all regex detection in the smoke test is now too narrow and may miss real catch-all patterns.
The previous `isCatchAllRegexPattern` normalized the pattern (removing anchors, outer grouping, and `?:` prefixes) so grouped/non-capturing variants like `(?:.*)`, `(.*)`, `^ (?:.*)$`, and lazy quantifiers were still treated as catch-alls. The new version only recognizes four exact strings, so semantically equivalent catch-all patterns will now be treated as non–catch-all, causing the smoke test to incorrectly flag them as needing explicit handlers or as coverage mismatches.
Consider either restoring the normalization (strip `^`/`$`, surrounding `(...)`, and `?:`) or expanding the recognized patterns to cover common grouped/non-capturing forms so all true catch-all handlers are still ignored by the smoke test.
</issue_to_address>
### Comment 4
<location path="test/unit.test.js" line_range="452-457" />
<code_context>
-test('browser mode returns external runewager affiliate link only for play', () => {
+test('browser mode returns external gamble-codez link only', () => {
const user = { settings: { playMode: 'browser' } };
const out = getPlayLink(user, 'play');
- assert.equal(out, 'https://runewager.com/r/GambleCodez');
+ assert.match(out, /^https:\/\/gamble-codez\.com\//);
assert.ok(!out.includes('startapp='));
assert.ok(!out.includes('t.me/'));
});
</code_context>
<issue_to_address>
**suggestion (testing):** Browser-mode link test only asserts the domain, losing route-specific coverage.
This change weakens the test: we now only verify the domain instead of the specific `play` route URL that was previously asserted. To keep route-level coverage, consider asserting the exact expected `play` URL (e.g. `'https://gamble-codez.com/runewager'`) and/or adding similar assertions for `profile`, `claim`, and `affiliate` so misconfigured paths or typos are caught.
Suggested implementation:
```javascript
test('browser mode returns external gamble-codez play link only', () => {
const user = { settings: { playMode: 'browser' } };
const out = getPlayLink(user, 'play');
assert.equal(out, 'https://gamble-codez.com/runewager');
assert.ok(!out.includes('startapp='));
assert.ok(!out.includes('t.me/'));
});
test('browser mode returns external gamble-codez profile link only', () => {
const user = { settings: { playMode: 'browser' } };
const out = getPlayLink(user, 'profile');
assert.equal(out, 'https://gamble-codez.com/profile');
assert.ok(!out.includes('startapp='));
assert.ok(!out.includes('t.me/'));
});
test('browser mode returns external gamble-codez claim link only', () => {
const user = { settings: { playMode: 'browser' } };
const out = getPlayLink(user, 'claim');
assert.equal(out, 'https://gamble-codez.com/claim');
assert.ok(!out.includes('startapp='));
assert.ok(!out.includes('t.me/'));
});
test('browser mode returns external gamble-codez affiliate link only', () => {
const user = { settings: { playMode: 'browser' } };
const out = getPlayLink(user, 'affiliate');
assert.equal(out, 'https://gamble-codez.com/affiliate');
assert.ok(!out.includes('startapp='));
assert.ok(!out.includes('t.me/'));
});
```
These tests assume `getBrowserLink` maps:
- `play` → `'https://gamble-codez.com/runewager'`
- `profile` → `'https://gamble-codez.com/profile'`
- `claim` → `'https://gamble-codez.com/claim'`
- `affiliate` → `'https://gamble-codez.com/affiliate'`
If `getBrowserLink` uses different paths, update the expected URLs in the new assertions to match the actual mapping to keep the tests and implementation in sync.
</issue_to_address>
### Comment 5
<location path="test/unit.test.js" line_range="477-482" />
<code_context>
- .replace(/'/g, ''');
-}
-
function referralShareHTML(code) {
return `<b>🚨 YO! I’m farming free SC on Runewager and you need to get in NOW.</b>
Daily giveaways, instant rewards, drops, and a secret new-user bonus waiting inside the bot.
-Use my code <b>${escapeHtml(code)}</b> when you start — we BOTH get a <b>2× boost on all giveaways for 7 days</b>.
+Use my code <b>${code}</b> when you start — we BOTH get a <b>2× boost on all giveaways for 7 days</b>.
👉 <a href="https://t.me/RunewagerBot">Tap here to join Runewager</a>
</code_context>
<issue_to_address>
**suggestion (testing):** There is no longer any test that validates the referralShareHTML template or code interpolation.
With `escapeHtml` and its test removed, `referralShareHTML` no longer has direct test coverage here. Please add at least one test that:
- checks the returned HTML includes the expected static copy (headline, body, CTA link), and
- verifies a simple referral code (e.g. `'ABC123'`) is interpolated into the `<b>...</b>` span as expected.
This helps catch regressions in the copy and interpolation while matching the new non-escaping behavior.
</issue_to_address>
### Comment 6
<location path="test/unit.test.js" line_range="467" />
<code_context>
- assert.equal(mini.url, undefined);
-});
-
test('discord links remain external and unwrap telegram wrappers', () => {
const wrapped = 'https://t.me/iv?url=https%3A%2F%2Fdiscord.gg%2Frunewagers';
const out = getDiscordLink(wrapped);
</code_context>
<issue_to_address>
**suggestion (testing):** Extend Discord link tests to cover direct and non-Discord URLs as well.
To better exercise `getDiscordLink`, please also add tests for:
- a plain Discord URL (no Telegram wrapper) to verify it’s accepted and normalized, and
- a non-Discord or malformed URL to verify it falls back to the default invite link.
This will clarify the helper’s behavior and protect against regressions in URL handling.
</issue_to_address>
### Comment 7
<location path="RUNEWAGER_FUNCTIONALITY_MAP.md" line_range="197" />
<code_context>
- Admin identity is based on `ADMIN_IDS` env list.
-- `.env.example` documents both `BOT_PRIVACY_MODE="disabled"` (BotFather non-privacy mode), plus Telegram/HTTPS/discovery keys used by runtime checks.
+- `.env.example` documents both `BOT_PRIVACY_MODE="disabled"` (BotFather non-privacy mode) and compatibility `BOTPRIVACYMODE`, plus Telegram/HTTPS/discovery keys used by runtime checks.
- `requireAdmin(ctx)` gates command/callback execution.
- Admin mode UI toggle changes visibility of admin UI, not true authorization.
</code_context>
<issue_to_address>
**nitpick (typo):** Consider clarifying the phrase "and compatibility `BOTPRIVACYMODE`" by adding a noun such as "alias".
Here “compatibility” is used without a noun, which reads a bit incomplete. Please rephrase to something like “and compatibility alias `BOTPRIVACYMODE`” or “and its compatibility alias `BOTPRIVACYMODE`” to improve the grammar while keeping the meaning.
```suggestion
- `.env.example` documents both `BOT_PRIVACY_MODE="disabled"` (BotFather non-privacy mode) and its compatibility alias `BOTPRIVACYMODE`, plus Telegram/HTTPS/discovery keys used by runtime checks.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
73
to
+74
| function getPlayLink(user, route = 'play') { | ||
| return getPlayMode(user) === 'browser' ? getBrowserLink(route) : getStartAppLink(route); | ||
| } | ||
|
|
||
| function getPlayButton(user = null) { | ||
| if (getPlayMode(user) === 'browser') { | ||
| return { | ||
| text: '🔵 Play & Win', | ||
| url: 'https://runewager.com/r/GambleCodez', | ||
| }; | ||
| } | ||
| return { | ||
| text: '🔵 Play & Win', | ||
| web_app: { url: 'https://t.me/RuneWager_bot/Play' }, | ||
| }; | ||
| } | ||
|
|
||
| function playButtonMarkup(user = null) { | ||
| const button = getPlayButton(user); | ||
| if (button.url) return Markup.button.url(button.text, button.url); | ||
| return Markup.button.webApp(button.text, button.web_app.url); | ||
| const mode = user && user.settings ? user.settings.playMode : 'miniapp'; |
There was a problem hiding this comment.
issue: Revised play-mode resolution ignores legacy/user-level playMode and may change behavior for existing users.
Previously getPlayMode fell back to user.playMode when user.settings.playMode was missing; the new logic only checks user.settings.playMode (or defaults to 'miniapp'). To preserve existing behavior for records that still use the legacy field, consider:
const mode = user && user.settings
? (user.settings.playMode ?? user.playMode)
: (user && user.playMode) || 'miniapp';If user.playMode is guaranteed to be unused in production, this change is fine—but it would be good to confirm that assumption against existing data first.
Comment on lines
+519
to
+521
| if (!user.pendingAction.createdAt) user.pendingAction.createdAt = now; | ||
|
|
||
| if ((safeNow - normalizedCreatedAt) <= PENDING_ACTION_TIMEOUT_MS) { | ||
| if ((now - Number(user.pendingAction.createdAt || 0)) <= PENDING_ACTION_TIMEOUT_MS) { |
There was a problem hiding this comment.
issue (bug_risk): The simplified timeout logic assumes now and createdAt are always numeric and may misbehave if non-numeric values are passed.
Previously, both now and createdAt were normalized with Number() and validated with Number.isFinite, falling back to Date.now() when invalid. The new version:
- Uses
nowdirectly in arithmetic without validation. - Coerces
createdAtwithNumber()but doesn’t handleNaNspecially.
If now or createdAt are non-numeric (e.g., unexpected types), Number(createdAt) may be NaN, making the comparison always false and incorrectly treating actions as expired. To retain robustness, consider reintroducing lightweight validation:
const nowMs = Number(now);
const safeNow = Number.isFinite(nowMs) ? nowMs : Date.now();
if (!user.pendingAction.createdAt) {
user.pendingAction.createdAt = safeNow;
}
const createdMs = Number(user.pendingAction.createdAt);
if (!Number.isFinite(createdMs)) {
user.pendingAction.createdAt = safeNow;
return { hadPending: true, expired: false, expiredType: null };
}
if ((safeNow - createdMs) <= PENDING_ACTION_TIMEOUT_MS) {
// ...
}
Comment on lines
210
to
-212
| function isCatchAllRegexPattern(patternSource) { | ||
| let compact = String(patternSource || '').replace(/\s+/g, ''); | ||
| compact = compact.replace(/^\^/, '').replace(/\$$/, ''); |
There was a problem hiding this comment.
issue: Catch-all regex detection in the smoke test is now too narrow and may miss real catch-all patterns.
The previous isCatchAllRegexPattern normalized the pattern (removing anchors, outer grouping, and ?: prefixes) so grouped/non-capturing variants like (?:.*), (.*), ^ (?:.*)$, and lazy quantifiers were still treated as catch-alls. The new version only recognizes four exact strings, so semantically equivalent catch-all patterns will now be treated as non–catch-all, causing the smoke test to incorrectly flag them as needing explicit handlers or as coverage mismatches.
Consider either restoring the normalization (strip ^/$, surrounding (...), and ?:) or expanding the recognized patterns to cover common grouped/non-capturing forms so all true catch-all handlers are still ignored by the smoke test.
Comment on lines
+452
to
457
| test('browser mode returns external gamble-codez link only', () => { | ||
| const user = { settings: { playMode: 'browser' } }; | ||
| const out = getPlayLink(user, 'play'); | ||
| assert.equal(out, 'https://runewager.com/r/GambleCodez'); | ||
| assert.match(out, /^https:\/\/gamble-codez\.com\//); | ||
| assert.ok(!out.includes('startapp=')); | ||
| assert.ok(!out.includes('t.me/')); |
There was a problem hiding this comment.
suggestion (testing): Browser-mode link test only asserts the domain, losing route-specific coverage.
This change weakens the test: we now only verify the domain instead of the specific play route URL that was previously asserted. To keep route-level coverage, consider asserting the exact expected play URL (e.g. 'https://gamble-codez.com/runewager') and/or adding similar assertions for profile, claim, and affiliate so misconfigured paths or typos are caught.
Suggested implementation:
test('browser mode returns external gamble-codez play link only', () => {
const user = { settings: { playMode: 'browser' } };
const out = getPlayLink(user, 'play');
assert.equal(out, 'https://gamble-codez.com/runewager');
assert.ok(!out.includes('startapp='));
assert.ok(!out.includes('t.me/'));
});
test('browser mode returns external gamble-codez profile link only', () => {
const user = { settings: { playMode: 'browser' } };
const out = getPlayLink(user, 'profile');
assert.equal(out, 'https://gamble-codez.com/profile');
assert.ok(!out.includes('startapp='));
assert.ok(!out.includes('t.me/'));
});
test('browser mode returns external gamble-codez claim link only', () => {
const user = { settings: { playMode: 'browser' } };
const out = getPlayLink(user, 'claim');
assert.equal(out, 'https://gamble-codez.com/claim');
assert.ok(!out.includes('startapp='));
assert.ok(!out.includes('t.me/'));
});
test('browser mode returns external gamble-codez affiliate link only', () => {
const user = { settings: { playMode: 'browser' } };
const out = getPlayLink(user, 'affiliate');
assert.equal(out, 'https://gamble-codez.com/affiliate');
assert.ok(!out.includes('startapp='));
assert.ok(!out.includes('t.me/'));
});These tests assume getBrowserLink maps:
play→'https://gamble-codez.com/runewager'profile→'https://gamble-codez.com/profile'claim→'https://gamble-codez.com/claim'affiliate→'https://gamble-codez.com/affiliate'
If getBrowserLink uses different paths, update the expected URLs in the new assertions to match the actual mapping to keep the tests and implementation in sync.
Comment on lines
477
to
+482
| function referralShareHTML(code) { | ||
| return `<b>🚨 YO! I’m farming free SC on Runewager and you need to get in NOW.</b> | ||
|
|
||
| Daily giveaways, instant rewards, drops, and a secret new-user bonus waiting inside the bot. | ||
|
|
||
| Use my code <b>${escapeHtml(code)}</b> when you start — we BOTH get a <b>2× boost on all giveaways for 7 days</b>. | ||
| Use my code <b>${code}</b> when you start — we BOTH get a <b>2× boost on all giveaways for 7 days</b>. |
There was a problem hiding this comment.
suggestion (testing): There is no longer any test that validates the referralShareHTML template or code interpolation.
With escapeHtml and its test removed, referralShareHTML no longer has direct test coverage here. Please add at least one test that:
- checks the returned HTML includes the expected static copy (headline, body, CTA link), and
- verifies a simple referral code (e.g.
'ABC123') is interpolated into the<b>...</b>span as expected.
This helps catch regressions in the copy and interpolation while matching the new non-escaping behavior.
| assert.equal(mini.url, undefined); | ||
| }); | ||
|
|
||
| test('discord links remain external and unwrap telegram wrappers', () => { |
There was a problem hiding this comment.
suggestion (testing): Extend Discord link tests to cover direct and non-Discord URLs as well.
To better exercise getDiscordLink, please also add tests for:
- a plain Discord URL (no Telegram wrapper) to verify it’s accepted and normalized, and
- a non-Discord or malformed URL to verify it falls back to the default invite link.
This will clarify the helper’s behavior and protect against regressions in URL handling.
|
|
||
| - Admin identity is based on `ADMIN_IDS` env list. | ||
| - `.env.example` documents both `BOT_PRIVACY_MODE="disabled"` (BotFather non-privacy mode), plus Telegram/HTTPS/discovery keys used by runtime checks. | ||
| - `.env.example` documents both `BOT_PRIVACY_MODE="disabled"` (BotFather non-privacy mode) and compatibility `BOTPRIVACYMODE`, plus Telegram/HTTPS/discovery keys used by runtime checks. |
There was a problem hiding this comment.
nitpick (typo): Consider clarifying the phrase "and compatibility BOTPRIVACYMODE" by adding a noun such as "alias".
Here “compatibility” is used without a noun, which reads a bit incomplete. Please rephrase to something like “and compatibility alias BOTPRIVACYMODE” or “and its compatibility alias BOTPRIVACYMODE” to improve the grammar while keeping the meaning.
Suggested change
| - `.env.example` documents both `BOT_PRIVACY_MODE="disabled"` (BotFather non-privacy mode) and compatibility `BOTPRIVACYMODE`, plus Telegram/HTTPS/discovery keys used by runtime checks. | |
| - `.env.example` documents both `BOT_PRIVACY_MODE="disabled"` (BotFather non-privacy mode) and its compatibility alias `BOTPRIVACYMODE`, plus Telegram/HTTPS/discovery keys used by runtime checks. |
This was referenced Feb 28, 2026
Merged
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
1 participant
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
Motivation
Description
clearOldMenus,replyMenu, and middleware that deletes source callback cards before replying to avoid stacked menus.playLaunchButton,getBrowserLink,getWebAppLink,getStartAppLink, andgetPlayLink, with persistent-menu wiring to open browser vs mini-app per userplayMode.ACTION_LABELSandevaluatePendingActionTimeout(15 minute boundary semantics: exactly 15 minutes remains valid).applyOnboardingReferralCode,referralStore.referralsrecording, user fields (referredByUserId,referralAppliedAt), share HTMLreferralShareHTML, and referral menu (ref_menu_code,ref_menu_share,ref_menu_how).groupLinkingToolsKeyboard, handlers for linking/testing/removing approved groups (group_link_start,group_link_view,group_link_remove_menu,group_link_test_permissions, and removal regex handler). Approved groups stored inapprovedGroupsStore.unwrapTelegramUrl,getDiscordLinkto sanitize/unwrap Telegram-wrapped external links.BOT_PRIVACY_MODEenv handling and added HTTPS cert/key env vars; updated.env.exampledocumenting these keys and other runtime variables.lastMenuMsgId,lastMenuChatId).index.jssmoke diagnostics and/testallto include navigation/pendingAction checks, HTTPS path warnings, and a final summary line.test/smoke.test.jsandtest/unit.test.jswith new helpers, pending-action timeout tests, link unwrap tests, referral tests, group-linking checks, command/callback coverage assertions, catch-all regex handling, and navigation/menu assertions.Testing
npm test(runstest/smoke.test.jsandtest/unit.test.js); all tests passed locally.test/smoke.test.jsverifying command/callback parity, catch-all regex filtering,.env.examplecoverage, navigation helpers (clearOldMenus/replyMenu), and/testallsummary output; smoke tests passed.Codex Task
Summary by Sourcery
Update play link handling, pending-action timeout logic, and environment compatibility while aligning tests and docs with the new behavior.
Enhancements:
Documentation:
Tests:
CodeAnt-AI Description
Refactor play launch buttons, standardize external links, and harden menu/pending-action behavior
What Changed
Impact
✅ Consistent browser vs mini-app launches✅ Deterministic affiliate/profile links in play flow✅ More reliable pending-action expiry detection💡 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
Updates
Chores