diff --git a/CLAUDE.md b/CLAUDE.md index 33d0530..1c18186 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -73,7 +73,7 @@ Every `interactionCreate` branch runs through `runHandler(name, interaction, fn) ### Tickets (`services/tickets.js`, `models.js`) - `Ticket` schema has indexes on `{gmailThreadId}` (unique), `{status, lastActivity}`, `{senderEmail, status}`, `{discordThreadId}`. - **Discord-originated tickets** use `gmailThreadId` with prefix `discord-` / `discord-msg-` — skip the Gmail reply path entirely. -- `canRename(ticket)` enforces Discord's 2-rename/10-min per-channel limit via **two atomic `findOneAndUpdate` calls** (reset-if-expired, then increment-if-under-limit) — never a read-then-update. +- Renames route through `utils/renamer.js` (RENAMER_BOT secondary token). On 401/403/429 from the secondary, `services/channelQueue.js` falls back to the primary bot via `channel.setName`. `canRename()` is retained as an always-ok shim for back-compat. `Ticket.renameCount` / `Ticket.renameWindowStart` remain in the schema but are now unread/unwritten orphan fields. - `getOrCreateTicketCategory()` handles Discord's 50-channels-per-category ceiling by creating `" (Overflow N)"` categories; `cleanupEmptyOverflowCategory()` removes empties. The primary category is never deleted. - Scheduled jobs in `ready`: `checkAutoClose`, `checkAutoUnclaim`, `reconcileDeletedTicketChannels`, plus `services/staffNotifications.js#notifyAllStaffUnclaimed` and the pattern/surge/chat checkers. diff --git a/docs/api/DISCORD_API_VALIDATION.md b/docs/api/DISCORD_API_VALIDATION.md index 550d04d..38f2f64 100644 --- a/docs/api/DISCORD_API_VALIDATION.md +++ b/docs/api/DISCORD_API_VALIDATION.md @@ -245,12 +245,10 @@ Our code creates channels and renames them without explicit rate limit handling. **Discord Rate Limits:** - Channel creation: 50/day per guild -- Channel rename: 2 per 10 minutes per channel +- Channel rename: 2 per 10 minutes per channel **per bot token** **Our Protection:** -- ✅ We have rename rate limiting via `canRename()` function (2 renames per 10 minutes per channel) -- ✅ Tracks `rename_count` and `rename_window_start` on the ticket -- ✅ When limit is reached, skips rename and posts in the ticket: *Channel renamed too quickly. Try again \.* +- ✅ Renames route through `utils/renamer.js` (RENAMER_BOT secondary token). On 401/403/429 from the secondary, `services/channelQueue.js` falls back to the primary bot via `channel.setName`. `canRename()` is retained as an always-ok shim for back-compat. **Recommendation:** Current implementation is GOOD. No changes needed. diff --git a/docs/architecture/CRITICAL_FILES_AND_HOW_IT_WORKS.md b/docs/architecture/CRITICAL_FILES_AND_HOW_IT_WORKS.md index c53ff4f..00070cd 100644 --- a/docs/architecture/CRITICAL_FILES_AND_HOW_IT_WORKS.md +++ b/docs/architecture/CRITICAL_FILES_AND_HOW_IT_WORKS.md @@ -38,7 +38,7 @@ These are the files that give someone the fastest path to understanding the repo ### 8. [**services/tickets.js**](../services/tickets.js) - **Why:** Core ticket lifecycle and Discord channel/thread creation. -- **What you get:** Ticket numbers (`getNextTicketNumber`), channel naming and Discord rate limit handling (2 renames per 10 min), ticket limits and overflow category selection, rate limit for ticket creation per user, `createEmailTicketAsThread` / `createDiscordTicketAsThread`, auto-close/reminder/auto-unclaim jobs, and helpers like `updateTicketActivity`, `canRename`, `makeTicketName`. +- **What you get:** Ticket numbers (`getNextTicketNumber`), channel naming, ticket limits and overflow category selection, rate limit for ticket creation per user, `createEmailTicketAsThread` / `createDiscordTicketAsThread`, auto-close/reminder/auto-unclaim jobs, and helpers like `updateTicketActivity`, `canRename` (retained as an always-ok shim — see `utils/renamer.js` and `services/channelQueue.js` for actual rename handling and primary-bot fallback), `makeTicketName`. ### 9. [**handlers/buttons.js**](../handlers/buttons.js) - **Why:** Every button and ticket modal goes through here. @@ -167,7 +167,7 @@ Broccolini Bot is a **Node.js support-ticket bot** that connects **Gmail**, **Di ### Claim / Unclaim / Close - **Claim** (button or `/claim`) - Ticket is updated with `claimedBy` (user id or name). Channel may be renamed (respecting Discord’s 2 renames per 10 min). Claimed message is posted (template from CONFIG). + Ticket is updated with `claimedBy` (user id or name). Channel may be renamed via the secondary-bot path (`utils/renamer.js`), falling back to the primary bot on 401/403/429. Claimed message is posted (template from CONFIG). - **Unclaim** (button or `/unclaim`) `claimedBy` is cleared; channel rename and message as above. diff --git a/handlers/buttons.js b/handlers/buttons.js index 12c4b68..bfe3a9f 100644 --- a/handlers/buttons.js +++ b/handlers/buttons.js @@ -16,7 +16,7 @@ const { } = require('discord.js'); const { mongoose } = require('../db-connection'); const { CONFIG } = require('../config'); -const { canRename, makeTicketName, resolveCreatorNickname, minutesFromMs, getOrCreateTicketCategory, cleanupEmptyOverflowCategory, createDiscordTicketAsThread, checkTicketCreationRateLimit, getSenderLocal, toDiscordSafeName } = require('../services/tickets'); +const { makeTicketName, resolveCreatorNickname, getOrCreateTicketCategory, cleanupEmptyOverflowCategory, createDiscordTicketAsThread, checkTicketCreationRateLimit, getSenderLocal, toDiscordSafeName } = require('../services/tickets'); const { sendTicketClosedEmail } = require('../services/gmail'); const { getTicketActionRow } = require('../utils/ticketComponents'); const { sanitizeEmbedText, truncateEmbedDescription, truncateEmbedField, enforceEmbedLimit } = require('../utils'); @@ -26,6 +26,7 @@ const { runEscalation, runDeescalation } = require('./commands'); const { trackInteraction, trackError } = require('./analytics'); const { pendingCloses } = require('./pendingCloses'); const { increment } = require('../services/patternStore'); +const { logError } = require('../services/debugLog'); const Ticket = mongoose.model('Ticket'); const Transcript = mongoose.model('Transcript'); @@ -348,23 +349,11 @@ async function handleClaim(interaction, ticket) { const claimerEmoji = CONFIG.STAFF_EMOJIS.get(interaction.user.id) || CONFIG.CLAIMER_EMOJI_FALLBACK; const creatorNickname = await resolveCreatorNickname(guild, freshTicket); - const renameInfo = await canRename(freshTicket); - if (renameInfo.ok) { - const state = freshTicket.escalated ? 'escalated-claimed' : 'claimed'; - const newName = makeTicketName(state, freshTicket, creatorNickname, claimerEmoji); - enqueueRename(interaction.channel, newName).catch(() => {}); - } else { - const unlockAtMs = Date.now() + renameInfo.waitMs; - const unlockAtUnix = Math.floor(unlockAtMs / 1000); - await enqueueSend(interaction.channel, - `Channel renamed too quickly. Try again .` - ); - } + const state = freshTicket.escalated ? 'escalated-claimed' : 'claimed'; + const newName = makeTicketName(state, freshTicket, creatorNickname, claimerEmoji); + enqueueRename(interaction.channel, newName).catch(err => logError('rename', err).catch(() => {})); - const baseLabel = `Unclaim (${claimerLabel})`; - const label = renameInfo.ok - ? baseLabel - : `${baseLabel} – rename in ${minutesFromMs(renameInfo.waitMs)}m`; + const label = `Unclaim (${claimerLabel})`; btnClose .setCustomId('close_ticket') @@ -404,16 +393,7 @@ async function handleClaim(interaction, ticket) { const creatorNicknameUnclaim = await resolveCreatorNickname(guild, freshTicket); const unclaimState = (freshTicket.escalationTier ?? 0) >= 1 ? 'escalated' : 'unclaimed'; - const renameInfo = await canRename(freshTicket); - if (renameInfo.ok) { - enqueueRename(interaction.channel, makeTicketName(unclaimState, freshTicket, creatorNicknameUnclaim)).catch(() => {}); - } else { - const unlockAtMs = Date.now() + renameInfo.waitMs; - const unlockAtUnix = Math.floor(unlockAtMs / 1000); - await enqueueSend(interaction.channel, - `Channel renamed too quickly. Try again .` - ); - } + enqueueRename(interaction.channel, makeTicketName(unclaimState, freshTicket, creatorNicknameUnclaim)).catch(err => logError('rename', err).catch(() => {})); btnClose .setCustomId('close_ticket') @@ -731,8 +711,9 @@ async function handleTicketModal(interaction) { const actionRow = getTicketActionRow({ escalationTier: 0 }); enforceEmbedLimit([welcomeEmbed, infoEmbed, resourcesEmbed]); + let welcomeMsg; try { - const welcomeMsg = await enqueueSend(channel, { + welcomeMsg = await enqueueSend(channel, { content: `Hey There ${interaction.user} 🥦`, embeds: [welcomeEmbed, infoEmbed, resourcesEmbed], components: [actionRow] diff --git a/handlers/commands.js b/handlers/commands.js index 9a7b64d..5480c1d 100644 --- a/handlers/commands.js +++ b/handlers/commands.js @@ -13,14 +13,14 @@ const { const { mongoose } = require('../db-connection'); const { CONFIG, TICKET_TAGS } = require('../config'); const { getPriorityEmoji, getPriorityColor, replaceVariables, escapeRegex } = require('../utils'); -const { canRename, makeTicketName, resolveCreatorNickname, getSenderLocal, toDiscordSafeName, getOrCreateTicketCategory, createDiscordTicketAsThread, checkTicketCreationRateLimit } = require('../services/tickets'); +const { makeTicketName, resolveCreatorNickname, getSenderLocal, toDiscordSafeName, getOrCreateTicketCategory, createDiscordTicketAsThread, checkTicketCreationRateLimit } = require('../services/tickets'); const { sendTicketNotificationEmail } = require('../services/gmail'); const { getTicketActionRow } = require('../utils/ticketComponents'); const { getEmailRouting } = require('../services/guildSettings'); const { enqueueRename, enqueueMove, enqueueSend } = require('../services/channelQueue'); const { setNotifyDm } = require('../services/staffSettings'); const { trackInteraction, trackError, getAnalyticsSummary } = require('./analytics'); -const { logTicketEvent, logSecurity } = require('../services/debugLog'); +const { logTicketEvent, logSecurity, logError } = require('../services/debugLog'); const { handleAccountInfoCommand } = require('./accountinfo'); const { handleSetupCommand } = require('./setup'); const { pendingCloses } = require('./pendingCloses'); @@ -87,17 +87,8 @@ async function runEscalation(interaction, ticket, nextTier, reason) { if (ticket.game) increment(`staff_game_escalations:${interaction.user.id}`, ticket.game, 'week'); const creatorNickname = await resolveCreatorNickname(interaction.guild, ticket); - const renameInfo = await canRename(ticket); - if (renameInfo.ok) { - const newName = makeTicketName('escalated', ticket, creatorNickname); - enqueueRename(interaction.channel, newName).catch(() => {}); - } else { - const unlockAtMs = Date.now() + renameInfo.waitMs; - const unlockAtUnix = Math.floor(unlockAtMs / 1000); - await enqueueSend(interaction.channel, - `Channel renamed too quickly. Try again .` - ); - } + const newName = makeTicketName('escalated', ticket, creatorNickname); + enqueueRename(interaction.channel, newName).catch(err => logError('rename', err).catch(() => {})); if (!interaction.channel.isThread() && categoryId) { await enqueueMove(interaction.channel, categoryId); @@ -116,9 +107,11 @@ async function runEscalation(interaction, ticket, nextTier, reason) { const heyLine = creatorMention ? `Hey There ${creatorMention} 🥦` : 'Hey There 🥦'; - await enqueueSend(interaction.channel, - `${heyLine}\n**Getting the senior ${roleMention} for you.**` - ); + // Creator + role pings are intentional; still block @everyone/@here if somehow interpolated. + await enqueueSend(interaction.channel, { + content: `${heyLine}\n**Getting the senior ${roleMention} for you.**`, + allowedMentions: { parse: ['users', 'roles'] } + }); const escalationBody = CONFIG.ESCALATION_MESSAGE .replace(/\\n/g, '\n') @@ -198,16 +191,7 @@ async function runDeescalation(interaction, ticket) { const creatorNickname = await resolveCreatorNickname(interaction.guild, ticket); const state = newTier === 0 ? 'unclaimed' : 'escalated'; - const renameInfo = await canRename(ticket); - if (renameInfo.ok) { - enqueueRename(interaction.channel, makeTicketName(state, ticket, creatorNickname)).catch(() => {}); - } else { - const unlockAtMs = Date.now() + renameInfo.waitMs; - const unlockAtUnix = Math.floor(unlockAtMs / 1000); - await enqueueSend(interaction.channel, - `Channel renamed too quickly. Try again .` - ); - } + enqueueRename(interaction.channel, makeTicketName(state, ticket, creatorNickname)).catch(err => logError('rename', err).catch(() => {})); if (!interaction.channel.isThread()) { try { @@ -461,7 +445,7 @@ async function handleCommand(interaction) { SendMessages: true, ReadMessageHistory: true }); - await interaction.reply(`Added ${user} to this ticket.`); + await interaction.reply({ content: `Added ${user} to this ticket.`, allowedMentions: { parse: ['users'] } }); } catch (err) { console.error('Add user error:', err); await interaction.reply({ content: 'Failed to add user.', ephemeral: true }); @@ -479,7 +463,7 @@ async function handleCommand(interaction) { try { await interaction.channel.permissionOverwrites.delete(user.id); - await interaction.reply(`Removed ${user} from this ticket.`); + await interaction.reply({ content: `Removed ${user} from this ticket.`, allowedMentions: { parse: ['users'] } }); } catch (err) { console.error('Remove user error:', err); await interaction.reply({ content: 'Failed to remove user.', ephemeral: true }); @@ -511,15 +495,18 @@ async function handleCommand(interaction) { { $set: { claimedBy: claimerLabel } } ); - await interaction.reply( - `Ticket transferred to ${member} by ${interaction.user}.\nReason: ${reason}` - ); + // `reason` is staff-supplied freeform text; gate to user pings so @everyone in it can't mass-ping. + await interaction.reply({ + content: `Ticket transferred to ${member} by ${interaction.user}.\nReason: ${reason}`, + allowedMentions: { parse: ['users'] } + }); const logChan = await interaction.client.channels.fetch(CONFIG.LOG_CHAN).catch(() => null); if (logChan) { - await enqueueSend(logChan, - `Ticket ${interaction.channel} transferred from ${interaction.user.tag} to ${member.tag}.\nReason: ${reason}` - ); + await enqueueSend(logChan, { + content: `Ticket ${interaction.channel} transferred from ${interaction.user.tag} to ${member.tag}.\nReason: ${reason}`, + allowedMentions: { parse: ['users'] } + }); } } catch (err) { console.error('Transfer error:', err); @@ -784,7 +771,9 @@ async function handleCommand(interaction) { const content = replaceVariables(tag.content, context); await Tag.updateOne({ name }, { $inc: { useCount: 1 } }); - await interaction.reply(content); + // Tag bodies are staff-authored but may include variable substitutions from user/ticket data. + // Disable all mention parsing so a `@everyone` in a tag body never pings. + await interaction.reply({ content, allowedMentions: { parse: [] } }); } else if (subcommand === 'create') { diff --git a/services/channelQueue.js b/services/channelQueue.js index 855dd2e..e1a3cdf 100644 --- a/services/channelQueue.js +++ b/services/channelQueue.js @@ -6,7 +6,7 @@ * coalesce rapid successive calls so only the latest name is written. */ -const { logWarn } = require('../services/debugLog'); +const { logWarn, logError } = require('../services/debugLog'); const { renameChannel } = require('../utils/renamer'); // Per-channel: { chain: Promise, pendingName: string | null }. @@ -18,7 +18,22 @@ async function executeRename(channel, entry) { const currentName = entry.pendingName; if (currentName == null) return; try { - await renameChannel(channel.id, currentName); + try { + await renameChannel(channel.id, currentName); + } catch (err) { + // Secondary bot rate-limited (429), unauthorized (401), missing permission + // (403), or no token configured — fall back to the primary Discord.js client. + // Non-fallback errors rethrow so enqueueRename's catch can classify/log. + if (err && err.fallback === true && channel && typeof channel.setName === 'function') { + logWarn( + 'renameQueue', + `secondary-bot ${err.status ?? 'unavailable'}; falling back to primary channel=${channel.id}` + ).catch(() => {}); + await channel.setName(currentName); + } else { + throw err; + } + } } finally { // Clear only if no newer call arrived during the PATCH. If pendingName // has changed, leave it — the link queued by that newer call picks it up. @@ -42,6 +57,19 @@ function enqueueRename(channel, newName) { next.catch((err) => { logWarn('renameQueue', `Rename failed for ${channel.name}: ${err && err.message || err}`).catch(() => {}); + const status = err && err.status; + const msg = (err && err.message) || String(err); + if (status === 401 || status === 403) { + logError( + 'renameQueue:token/permission', + new Error(`secondary-bot ${status} channel=${channel.id} name=${channel.name}: ${msg}`) + ).catch(() => {}); + } else if (status === 429) { + logError( + 'renameQueue:secondary-bot ratelimited', + new Error(`429 channel=${channel.id} name=${channel.name}: ${msg}`) + ).catch(() => {}); + } }).finally(() => { if (renameChains.get(channel.id) === entry && entry.chain === next && entry.pendingName == null) { renameChains.delete(channel.id); @@ -50,8 +78,39 @@ function enqueueRename(channel, newName) { return next; } +// Shares renameChains so a move+rename pair on the same channel executes in +// call order. No coalescing: every move is a distinct chain link. function enqueueMove(channel, categoryId) { - return channel.setParent(categoryId, { lockPermissions: true }); + let entry = renameChains.get(channel.id); + if (!entry) { + entry = { chain: Promise.resolve(), pendingName: null }; + renameChains.set(channel.id, entry); + } + + const next = entry.chain.catch(() => {}).then(() => channel.setParent(categoryId, { lockPermissions: true })); + entry.chain = next; + + next.catch((err) => { + logWarn('moveQueue', `Move failed for ${channel.name}: ${err && err.message || err}`).catch(() => {}); + const status = err && err.status; + const msg = (err && err.message) || String(err); + if (status === 401 || status === 403) { + logError( + 'moveQueue:token/permission', + new Error(`${status} channel=${channel.id} categoryId=${categoryId}: ${msg}`) + ).catch(() => {}); + } else if (status === 429) { + logError( + 'moveQueue:ratelimited', + new Error(`429 channel=${channel.id} categoryId=${categoryId}: ${msg}`) + ).catch(() => {}); + } + }).finally(() => { + if (renameChains.get(channel.id) === entry && entry.chain === next && entry.pendingName == null) { + renameChains.delete(channel.id); + } + }); + return next; } // Per-channel promise chain for send ordering and to prevent interleaving. diff --git a/services/tickets.js b/services/tickets.js index 0c909c8..7c7aba2 100644 --- a/services/tickets.js +++ b/services/tickets.js @@ -25,10 +25,12 @@ async function getNextTicketNumber(senderEmail) { } // --- RENAME + NAMING --- -// Discord rate limit: 2 channel renames per 10 minutes per channel (see https://discord.com/developers/docs/topics/rate-limits). -// When limit is reached we skip the rename and post: "Channel renamed too quickly. Try again ." +// Renames flow through utils/renamer.js (RENAMER_BOT secondary token), +// which has its own Discord rate-limit bucket. We no longer gate on the +// primary bot's 2/10min per-channel budget here; 429s from the secondary +// bot surface via utils/renamer.js instead. -const RENAME_WINDOW_MS = 10 * 60 * 1000; // 10 minutes +const RENAME_WINDOW_MS = 10 * 60 * 1000; // 10 minutes (unused; kept for back-compat) const RENAME_LIMIT = 2; function getSenderLocal(senderEmail) { @@ -88,53 +90,10 @@ function makeTicketName(state, ticket, creatorNickname, claimerEmoji) { } } -async function canRename(ticket) { - const now = Date.now(); - const windowCutoff = new Date(now - RENAME_WINDOW_MS); - - // Atomic: reset the window if the stored start is older than the cutoff; count = 1. - const resetDoc = await Ticket.findOneAndUpdate( - { - gmailThreadId: ticket.gmailThreadId, - $or: [ - { renameWindowStart: { $lt: windowCutoff } }, - { renameWindowStart: null }, - { renameWindowStart: { $exists: false } } - ] - }, - { $set: { renameWindowStart: new Date(now), renameCount: 1 } }, - { new: true, projection: { renameCount: 1, renameWindowStart: 1 } } - ).lean(); - - if (resetDoc) { - ticket.renameWindowStart = resetDoc.renameWindowStart; - ticket.renameCount = resetDoc.renameCount; - return { ok: true, remaining: RENAME_LIMIT - resetDoc.renameCount, waitMs: 0 }; - } - - // Atomic: within window, only increment if count < limit. - const incDoc = await Ticket.findOneAndUpdate( - { - gmailThreadId: ticket.gmailThreadId, - renameCount: { $lt: RENAME_LIMIT } - }, - { $inc: { renameCount: 1 } }, - { new: true, projection: { renameCount: 1, renameWindowStart: 1 } } - ).lean(); - - if (incDoc) { - ticket.renameWindowStart = incDoc.renameWindowStart; - ticket.renameCount = incDoc.renameCount; - return { ok: true, remaining: RENAME_LIMIT - incDoc.renameCount, waitMs: 0 }; - } - - // At limit — read the window start to compute waitMs. - const fresh = await Ticket.findOne({ gmailThreadId: ticket.gmailThreadId }) - .select('renameWindowStart') - .lean(); - const windowStart = (fresh?.renameWindowStart && new Date(fresh.renameWindowStart).getTime()) || now; - const waitMs = Math.max(0, RENAME_WINDOW_MS - (now - windowStart)); - return { ok: false, remaining: 0, waitMs }; +// Retained for external callers (bOSScord, scripts). The gate now lives in +// the secondary bot's rate bucket; this helper no longer touches Mongo. +async function canRename(_ticket) { + return { ok: true, remaining: RENAME_LIMIT, waitMs: 0 }; } function minutesFromMs(ms) { diff --git a/utils/renamer.js b/utils/renamer.js index 1b2d250..006cacc 100644 --- a/utils/renamer.js +++ b/utils/renamer.js @@ -39,12 +39,30 @@ async function renameChannel(channelId, newName) { } if (res.status === 429) { - const retryAfter = (body && typeof body === 'object' && body.retry_after) || null; - logWarn('renamer', `429 rename channel=${channelId} retry_after=${retryAfter}`).catch(() => {}); - const err = new Error(`rename 429: retry_after=${retryAfter}`); + const retryAfterSec = (body && typeof body === 'object' && body.retry_after) || null; + const retryAfterMs = retryAfterSec != null ? Math.ceil(Number(retryAfterSec) * 1000) : null; + logWarn('renamer', `429 rename channel=${channelId} retry_after=${retryAfterSec}`).catch(() => {}); + + // Respect retry_after up to 2000ms; otherwise fail over immediately. + if (retryAfterMs != null && retryAfterMs > 0 && retryAfterMs <= 2000) { + await new Promise((resolve) => setTimeout(resolve, retryAfterMs)); + } + + const err = new Error(`rename 429: retry_after=${retryAfterSec}`); err.status = 429; - err.retryAfter = retryAfter; + err.retryAfter = retryAfterSec; err.body = body; + err.fallback = true; + throw err; + } + + if (res.status === 401 || res.status === 403) { + const bodyStr = typeof body === 'string' ? body : JSON.stringify(body); + logWarn('renamer', `${res.status} rename channel=${channelId} body=${bodyStr}`).catch(() => {}); + const err = new Error(`rename ${res.status}: ${bodyStr}`); + err.status = res.status; + err.body = body; + err.fallback = true; throw err; }