Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ Example:
$help
$ping
$usage
$stats
```

## AI Command Usage
Expand Down
21 changes: 21 additions & 0 deletions src/commands/ai/askai.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { openRouter } from "../../utils/ai.ts";
import { addToContext, getContext } from "../../utils/context.ts";
import { pretty } from "../../utils/pretty.ts";
import { sanitizeForPrompt } from "../../utils/sanitize.ts";
import { recordUsage } from "../../utils/stats.ts";
import { canUseAI, formatTimeLeft, setUsage } from "../../utils/usage.ts";

export let CURRENT_MODEL_INDEX = 0;
Expand All @@ -20,6 +21,9 @@ export default {
async execute({ message, args, ctx }: CommandCallbackOpts) {
if (message.author.bot) return;

if ((message as any)._processedByAskai) return;
Comment on lines 23 to +24

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The double-reply prevention mechanism mutates the message object directly using (message as any), which can cause memory leaks in long-running bots. The flag _processedByAskai is never cleaned up, and this approach bypasses TypeScript's type safety. Additionally, there's a potential race condition if multiple handlers process the same message concurrently.

Confidence: 4/5

Suggested Fix

Consider using a Set to track processed message IDs with automatic cleanup:

Suggested change
if ((message as any)._processedByAskai) return;
// Add at module level (outside execute function)
const processedMessages = new Set<string>();
if (processedMessages.has(message.id)) return;
processedMessages.add(message.id);
setTimeout(() => processedMessages.delete(message.id), 60000); // Cleanup after 1 minute

This approach:

  • Prevents memory leaks by cleaning up old entries
  • Maintains type safety without any casts
  • Better handles race conditions
  • Uses message IDs (which are unique) instead of mutating objects
Prompt for AI

Copy this prompt to your AI IDE to fix this issue locally:

In src/commands/ai/askai.ts around line 23, the double-reply prevention mechanism uses (message as any)._processedByAskai which can cause memory leaks and bypasses type safety; replace this with a module-level Set<string> called processedMessages that tracks message.id values, add a check using processedMessages.has(message.id), then processedMessages.add(message.id), and include a setTimeout to delete the message ID after 60 seconds for automatic cleanup.

📍 This suggestion applies to lines 23-24

(message as any)._processedByAskai = true;

const { imageUrl, mimeType } = getAttachmentData(message);
const question = parseQuestion(args, !!imageUrl);

Expand Down Expand Up @@ -79,10 +83,27 @@ export default {
await message.reply({
content: pretty(result.text),
});

/// token usage
const tokensUsedByModel = result.usage?.totalTokens ?? 0;
if (tokensUsedByModel > 0) {
await setUsage(userId, tokensUsedByModel);
}

// stats

const profile = {
username: message.author.username,
displayName: message.author.displayName,
avatar: message.author.displayAvatarURL({
extension: "png",
size: 256,
}),
};

recordUsage(userId, profile, tokensUsedByModel).catch((err) => {
Comment on lines +95 to +104

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The stats recording operation lacks error handling. If recordUsage fails (e.g., file system error, JSON corruption), it will cause the entire command to fail even though the AI response was already successfully sent to the user. This creates a poor user experience where they see an error despite their request being processed correctly.

Confidence: 5/5

Suggested Fix
Suggested change
const profile = {
username: message.author.username,
displayName: message.author.displayName,
avatar: message.author.displayAvatarURL({
extension: "png",
size: 256,
}),
};
recordUsage(userId, profile, tokensUsedByModel).catch((err) => {
const profile = {
username: message.author.username,
displayName: message.author.displayName,
avatar: message.author.displayAvatarURL({
extension: "png",
size: 256,
}),
};
try {
await recordUsage(userId, profile, tokensUsedByModel);
} catch (error) {
console.error("Failed to record usage stats:", error);
}

Wrap the stats recording in a try-catch block to prevent non-critical stats failures from affecting the user experience. The error is logged for debugging but doesn't interrupt the command flow since the primary operation (AI response) has already succeeded.

Prompt for AI

Copy this prompt to your AI IDE to fix this issue locally:

In src/commands/ai/askai.ts around line 95, the stats recording code (lines 95-104) lacks error handling which can cause the entire command to fail if recordUsage throws an error; wrap the profile creation and recordUsage call in a try-catch block that logs the error with console.error but doesn't throw, ensuring that stats recording failures don't affect the user experience since the AI response has already been successfully sent.

📍 This suggestion applies to lines 95-104

console.error("[Stats] Failed to record usage:", err);
});
} else {
await message.reply(
"Sorry, I encountered an issue processing your request right now.",
Expand Down
64 changes: 64 additions & 0 deletions src/commands/ai/stats.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import { AttachmentBuilder } from "discord.js";
import type { CommandCallbackOpts } from "../../types/command.ts";
import { getUserStats } from "../../utils/stats.ts";
import { renderStatsCard } from "../../visuals/statsCard.ts";

export default {
name: "stats",
description: "Show your AI usage statistics with heatmap",
aliases: ["mystats", "usage-stats"],
async execute({ message }: CommandCallbackOpts) {
if (message.author.bot) return;

try {
const target = message.mentions?.users?.first() || message.author;
const userId = target.id;

const stats = await getUserStats(userId);

if (!stats.hasData) {
const who =
target.id === message.author.id
? "You have"
: `${target.username} has`;

await message.reply(
`${who} no AI usage recorded yet. Use \`$ai\` or \`,\` with a question to start tracking your stats.`,
);
return;
}

const member = message.guild
? await message.guild.members.fetch(target.id).catch(() => null)
: null;

const displayName =
member?.displayName || stats.displayName || target.username;
const avatarUrl =
target.displayAvatarURL({ extension: "png", size: 256 }) ||
stats.avatar ||
null;

const buffer = await renderStatsCard({
displayName,
handle: `@${target.username}`,
avatarUrl,
brand: "Rael",
heatmap: stats.heatmap,
lifetimeTokens: stats.lifetimeTokens,
peakDayTokens: stats.peakDayTokens,
currentStreak: stats.currentStreak,
longestStreak: stats.longestStreak,
});
Comment on lines +42 to +52

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The renderStatsCard function is called without validating its return value before passing it to AttachmentBuilder. If renderStatsCard returns null, undefined, or an invalid buffer (e.g., due to image rendering failure), the AttachmentBuilder constructor will throw an error that bypasses the catch block's user-friendly error message, potentially exposing internal error details to the user.

Confidence: 4/5

Suggested Fix
Suggested change
const buffer = await renderStatsCard({
displayName,
handle: `@${target.username}`,
avatarUrl,
brand: "Rael",
heatmap: stats.heatmap,
lifetimeTokens: stats.lifetimeTokens,
peakDayTokens: stats.peakDayTokens,
currentStreak: stats.currentStreak,
longestStreak: stats.longestStreak,
});
const buffer = await renderStatsCard({
displayName,
handle: `@${target.username}`,
avatarUrl,
brand: "Rael",
heatmap: stats.heatmap,
lifetimeTokens: stats.lifetimeTokens,
peakDayTokens: stats.peakDayTokens,
currentStreak: stats.currentStreak,
longestStreak: stats.longestStreak,
});
if (!buffer || buffer.length === 0) {
throw new Error("Failed to generate stats card buffer");
}

Add validation after the renderStatsCard call to ensure a valid buffer is returned. This ensures the error is caught by the existing try-catch block and the user receives the friendly error message "Could not generate your stats card right now." instead of a raw error.

Prompt for AI

Copy this prompt to your AI IDE to fix this issue locally:

In src/commands/ai/stats.ts around line 42, after the renderStatsCard call completes and before creating the AttachmentBuilder, add a validation check to ensure the buffer is valid; add an if statement checking if (!buffer || buffer.length === 0) and throw new Error("Failed to generate stats card buffer") to ensure rendering failures are caught by the existing try-catch block and provide the user-friendly error message instead of exposing internal errors.

📍 This suggestion applies to lines 42-52


const attachment = new AttachmentBuilder(buffer, {
name: `stats-${target.id}.png`,
});

await message.reply({ files: [attachment] });
} catch (err) {
console.error("Stats command error:", err);
await message.reply("Could not generate your stats card right now.");
}
},
};
203 changes: 203 additions & 0 deletions src/utils/stats.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,203 @@
import fs from "fs/promises";
import path from "path";
import { fileURLToPath } from "url";

const __filename = fileURLToPath(import.meta.url);
const __dirname = path.dirname(__filename);

const DATA_DIR = path.join(__dirname, "..", "..", "data");
const STATS_FILE = path.join(DATA_DIR, "stats.json");

interface DailyRecord {
[date: string]: number;
}

interface UserRecord {
username: string;
displayName: string;
avatar: string;
lifetimeTokens: number;
peakDayTokens: number;
daily: DailyRecord;
}

interface StatsStore {
users: Record<string, UserRecord>;
}

let store: StatsStore = { users: {} };
let isLoaded = false;
let writeQueued = false;
let isWriting = false;

async function loadStore(): Promise<void> {
if (isLoaded) return;

try {
await fs.mkdir(DATA_DIR, { recursive: true });
const raw = await fs.readFile(STATS_FILE, "utf-8");
const parsed = JSON.parse(raw);
store = parsed?.users ? parsed : { users: {} };
Comment on lines +39 to +40

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The loadStore() function parses untrusted JSON data without validation. If the stats.json file is corrupted or maliciously modified, JSON.parse(raw) can throw an error that's silently caught, or worse, the parsed?.users check is insufficient to validate the structure. This could lead to runtime errors when the code assumes store.users[userId] is a UserRecord but it's actually malformed data.

Confidence: 5/5

Suggested Fix
Suggested change
const parsed = JSON.parse(raw);
store = parsed?.users ? parsed : { users: {} };
const raw = await fs.readFile(STATS_FILE, "utf-8");
const parsed = JSON.parse(raw);
// Validate structure before assignment
if (parsed && typeof parsed === 'object' && parsed.users && typeof parsed.users === 'object') {
store = parsed as StatsStore;
} else {
store = { users: {} };
}

Add proper validation to ensure the parsed JSON matches the expected StatsStore structure. This prevents runtime errors when accessing user records and protects against corrupted or malicious data.

Prompt for AI

Copy this prompt to your AI IDE to fix this issue locally:

In src/utils/stats.ts around line 39, the JSON.parse result is assigned to store without proper validation; after parsing, add a validation check to ensure parsed is an object with a users property that is also an object, using if (parsed && typeof parsed === 'object' && parsed.users && typeof parsed.users === 'object') before assigning it to store, otherwise default to { users: {} } to prevent runtime errors from corrupted or malicious data.

📍 This suggestion applies to lines 39-40

} catch {
store = { users: {} };
}
isLoaded = true;
}

async function saveStore(): Promise<void> {
if (isWriting) {
writeQueued = true;
return;
}

isWriting = true;
try {
await fs.mkdir(DATA_DIR, { recursive: true });
await fs.writeFile(STATS_FILE, JSON.stringify(store), "utf-8");
} catch (err) {
console.error("Failed to save stats.json:", err);
} finally {
isWriting = false;
if (writeQueued) {
writeQueued = false;
await saveStore();
}
}
}
Comment on lines +47 to +66

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The saveStore() function has a critical race condition vulnerability. If multiple concurrent calls occur, the writeQueued flag mechanism can lose data. When isWriting is true and a second call sets writeQueued = true, a third concurrent call will also set writeQueued = true (overwriting the flag), but only ONE recursive save will occur after the current write completes. This means the third call's data changes will be lost.

Confidence: 5/5

Suggested Fix
Suggested change
async function saveStore(): Promise<void> {
if (isWriting) {
writeQueued = true;
return;
}
isWriting = true;
try {
await fs.mkdir(DATA_DIR, { recursive: true });
await fs.writeFile(STATS_FILE, JSON.stringify(store), "utf-8");
} catch (err) {
console.error("Failed to save stats.json:", err);
} finally {
isWriting = false;
if (writeQueued) {
writeQueued = false;
await saveStore();
}
}
}
async function saveStore(): Promise<void> {
if (isWriting) {
writeQueued = true;
return;
}
isWriting = true;
writeQueued = false;
try {
await fs.mkdir(DATA_DIR, { recursive: true });
await fs.writeFile(STATS_FILE, JSON.stringify(store, null, 2), "utf-8");
} catch (err) {
console.error("Failed to save stats.json:", err);
} finally {
isWriting = false;
if (writeQueued) {
await saveStore();
}

Move writeQueued = false to the beginning of the write operation (after setting isWriting = true). This ensures that any new calls during the write will properly queue another save, preventing data loss from concurrent modifications.

Prompt for AI

Copy this prompt to your AI IDE to fix this issue locally:

In src/utils/stats.ts around line 47-66, the saveStore function has a race condition where multiple concurrent calls can lose data because the writeQueued flag is reset in the finally block instead of at the start of the write operation; move the writeQueued = false statement to line 53 (right after isWriting = true) to ensure that any calls during the write operation will properly queue another save, preventing data loss from concurrent modifications.

📍 This suggestion applies to lines 47-66

Comment on lines +47 to +66

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The saveStore() function has a critical race condition vulnerability. If multiple concurrent calls to recordUsage() occur (which is likely in a Discord bot with multiple users), the writeQueued flag approach doesn't prevent data loss. When isWriting is true and a second call sets writeQueued = true, a third concurrent call will also set writeQueued = true, but only ONE queued write will execute after the current write completes. This means the third call's data modifications to the in-memory store object will be lost.

Confidence: 5/5

Suggested Fix

The current queuing mechanism is fundamentally flawed for handling multiple concurrent writes. Consider implementing a proper write queue using an array or using a debounced write approach with a minimum delay. However, this requires significant refactoring beyond a simple line-range fix.
Recommended approach:

  1. Implement a write queue (array) instead of a boolean flag
  2. Use a mutex/lock library like async-mutex to serialize writes
  3. Or implement a debounced write with a timer that batches multiple updates
    This is a critical architectural issue that needs careful redesign to prevent data loss in production.
Prompt for AI

Copy this prompt to your AI IDE to fix this issue locally:

In src/utils/stats.ts around line 47, the saveStore() function has a race condition where multiple concurrent calls can cause data loss because the writeQueued boolean flag only tracks one pending write; replace the writeQueued boolean with a proper write queue mechanism, either by using an array to queue pending writes or by implementing a debounced write approach with a timer, or by using a mutex library like async-mutex to serialize all write operations and ensure no data modifications are lost during concurrent access.

📍 This suggestion applies to lines 47-66


function getDayKey(date = new Date()): string {
return date.toISOString().slice(0, 10);
}

export async function recordUsage(
userId: string,
profile: { username?: string; displayName?: string; avatar?: string },
tokens: number,
): Promise<void> {
if (!userId) return;

await loadStore();

if (!store.users[userId]) {
store.users[userId] = {
username: "",
displayName: "",
avatar: "",
lifetimeTokens: 0,
peakDayTokens: 0,
daily: {},
};
}

const user = store.users[userId];

if (profile.username) user.username = profile.username;
if (profile.displayName) user.displayName = profile.displayName;
if (profile.avatar) user.avatar = profile.avatar;

const safeTokens = Math.max(0, Math.floor(tokens));
if (safeTokens > 0) {
const today = getDayKey();
user.daily[today] = (user.daily[today] || 0) + safeTokens;
user.lifetimeTokens += safeTokens;

if (user.daily[today] > user.peakDayTokens) {
user.peakDayTokens = user.daily[today];
}
}

await saveStore();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue as Comment #1 in askai.ts - the saveStore() call at the end of recordUsage lacks error handling. If the save fails (disk full, permission error, etc.), it will crash the entire operation even though the in-memory stats were successfully updated. This is a non-critical operation that should not prevent the function from completing.

Confidence: 5/5

Suggested Fix
Suggested change
await saveStore();
await saveStore().catch((error) => {
console.error("Failed to save stats after recording usage:", error);
});

Wrap the saveStore() call in a .catch() handler to log errors without throwing. This ensures that save failures don't crash the calling code, since the in-memory stats have already been updated successfully.

Prompt for AI

Copy this prompt to your AI IDE to fix this issue locally:

In src/utils/stats.ts around line 109, the saveStore call lacks error handling which could crash the recordUsage function if the save fails; add a .catch() handler to log the error without throwing, ensuring that save failures don't impact the calling code since the in-memory stats have already been updated successfully.

}

function computeStreaks(daily: DailyRecord = {}) {
const activeDays = new Set(
Object.keys(daily).filter((d) => daily[d] && daily[d] > 0),
);
if (activeDays.size === 0) return { currentStreak: 0, longestStreak: 0 };

// Current streak
let currentStreak = 0;
const cursor = new Date();
if (!activeDays.has(getDayKey(cursor))) {
cursor.setUTCDate(cursor.getUTCDate() - 1);
}
while (activeDays.has(getDayKey(cursor))) {
currentStreak++;
cursor.setUTCDate(cursor.getUTCDate() - 1);
}
Comment on lines +120 to +127

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current streak calculation has a logic error. It creates a new Date() object which uses the local system time, but then calls getDayKey(cursor) which converts to ISO string (UTC). This can cause off-by-one errors in streak calculation depending on the server's timezone. For example, if the server is in PST and it's 11 PM on Jan 1st PST, new Date() is Jan 1st locally but toISOString() returns Jan 2nd UTC, breaking the streak logic.

Confidence: 5/5

Suggested Fix
Suggested change
const cursor = new Date();
if (!activeDays.has(getDayKey(cursor))) {
cursor.setUTCDate(cursor.getUTCDate() - 1);
}
while (activeDays.has(getDayKey(cursor))) {
currentStreak++;
cursor.setUTCDate(cursor.getUTCDate() - 1);
}
// Current streak
let currentStreak = 0;
const cursor = new Date();
cursor.setUTCHours(0, 0, 0, 0); // Normalize to UTC midnight
if (!activeDays.has(getDayKey(cursor))) {
cursor.setUTCDate(cursor.getUTCDate() - 1);
}
while (activeDays.has(getDayKey(cursor))) {
currentStreak++;
cursor.setUTCDate(cursor.getUTCDate() - 1);
}

Normalize the cursor date to UTC midnight before starting the streak calculation. This ensures consistency between the date used for checking and the date keys stored in the daily records.

Prompt for AI

Copy this prompt to your AI IDE to fix this issue locally:

In src/utils/stats.ts around line 120, the current streak calculation creates a Date object that may have timezone inconsistencies with the getDayKey() function which uses UTC; after creating the cursor Date object on line 120, add cursor.setUTCHours(0, 0, 0, 0) to normalize it to UTC midnight before the streak calculation begins, ensuring the date comparisons are consistent with how dates are stored in the daily records.

📍 This suggestion applies to lines 120-127


// Longest streak
const sorted = [...activeDays].sort();
let longestStreak = 1;
let run = 1;

for (let i = 1; i < sorted.length; i++) {
const prev = new Date(`${sorted[i - 1]}T00:00:00Z`);
const curr = new Date(`${sorted[i]}T00:00:00Z`);
const diff = Math.round((curr.getTime() - prev.getTime()) / 86400000);
run = diff === 1 ? run + 1 : 1;
if (run > longestStreak) longestStreak = run;
}

return { currentStreak, longestStreak };
}

export async function getUserStats(userId: string) {
await loadStore();
const user = store.users[userId];

if (!user) {
return {
displayName: "",
username: "",
avatar: "",
lifetimeTokens: 0,
peakDayTokens: 0,
currentStreak: 0,
longestStreak: 0,
heatmap: { columns: [], max: 0 },
hasData: false,
};
}

const { currentStreak, longestStreak } = computeStreaks(user.daily);

// Build 30-week heatmap
const weeks = 30;
const today = new Date();
const end = new Date(
Date.UTC(today.getUTCFullYear(), today.getUTCMonth(), today.getUTCDate()),
);
end.setUTCDate(end.getUTCDate() + (6 - end.getUTCDay()));

const totalDays = weeks * 7;
const start = new Date(end);
start.setUTCDate(start.getUTCDate() - (totalDays - 1));

const columns: number[][] = [];
let max = 0;
const cursor = new Date(start);

for (let w = 0; w < weeks; w++) {
const week: number[] = [];
for (let d = 0; d < 7; d++) {
const value = user.daily[getDayKey(cursor)] || 0;
if (value > max) max = value;
week.push(value);
cursor.setUTCDate(cursor.getUTCDate() + 1);
}
columns.push(week);
}

return {
displayName: user.displayName,
username: user.username,
avatar: user.avatar,
lifetimeTokens: user.lifetimeTokens,
peakDayTokens: user.peakDayTokens,
currentStreak,
longestStreak,
heatmap: { columns, max },
hasData: user.lifetimeTokens > 0,
};
}
2 changes: 1 addition & 1 deletion src/utils/usage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ async function loadUsage(): Promise<UsageData> {
async function saveUsage(data: UsageData): Promise<void> {
usageCache = data;
await fs.mkdir(path.dirname(USAGE_FILE), { recursive: true });
await fs.writeFile(USAGE_FILE, JSON.stringify(data, null, 2), "utf-8");
await fs.writeFile(USAGE_FILE, JSON.stringify(data), "utf-8");
}

export async function getUsage(userId: string): Promise<number> {
Expand Down
Loading
Loading