refactor: replace cleanup service with focused temp role service and fix daily streaks

src/commands/admin/cleanup.ts

@@ -1,73 +0,0 @@
-import { createCommand } from "@lib/utils";
-import { SlashCommandBuilder, PermissionFlagsBits } from "discord.js";
-import { lootdropService } from "@/modules/economy/lootdrop.service";
-import { createBaseEmbed } from "@lib/embeds";
-
-export const cleanup = createCommand({
-    data: new SlashCommandBuilder()
-        .setName("cleanup")
-        .setDescription("Manually trigger cleanup tasks")
-        .setDefaultMemberPermissions(PermissionFlagsBits.Administrator)
-        .addStringOption(option =>
-            option.setName("type")
-                .setDescription("The type of cleanup to perform")
-                .setRequired(true)
-                .addChoices(
-                    { name: 'Lootdrops', value: 'lootdrops' },
-                    { name: 'Timers (Expired)', value: 'timers' },
-                    { name: 'Quests (Old Completed)', value: 'quests' },
-                    { name: 'All', value: 'all' }
-                )
-        )
-        .addBooleanOption(option =>
-            option.setName("include_claimed")
-                .setDescription("Whether to cleanup claimed lootdrops as well (only for lootdrops/all)")
-                .setRequired(false)
-        ),
-    execute: async (interaction) => {
-        await interaction.deferReply({ ephemeral: true });
-
-        const type = interaction.options.getString("type", true);
-        const includeClaimed = interaction.options.getBoolean("include_claimed") || false;
-
-        try {
-            let stats = {
-                lootdrops: 0,
-                timers: 0,
-                quests: 0
-            };
-
-            const runLootdrops = type === 'lootdrops' || type === 'all';
-            const runTimers = type === 'timers' || type === 'all';
-            const runQuests = type === 'quests' || type === 'all';
-
-            const messages: string[] = [];
-
-            if (runLootdrops) {
-                stats.lootdrops = await lootdropService.cleanupExpiredLootdrops(includeClaimed);
-                messages.push(`- **Lootdrops**: ${stats.lootdrops} removed ${includeClaimed ? "(including claimed)" : ""}`);
-            }
-
-            if (runTimers) {
-                // Import dynamically to avoid circular deps if any, or just standard import
-                const { cleanupService } = await import("@/modules/system/cleanup.service");
-                stats.timers = await cleanupService.cleanupTimers();
-                messages.push(`- **Timers**: ${stats.timers} expired timers processing/removed`);
-            }
-
-            if (runQuests) {
-                const { cleanupService } = await import("@/modules/system/cleanup.service");
-                stats.quests = await cleanupService.cleanupQuests();
-                messages.push(`- **Quests**: ${stats.quests} archived/removed`);
-            }
-
-            const embed = createBaseEmbed("Cleanup Complete")
-                .setDescription(`successfully executed cleanup for **${type}**.\n\n${messages.join("\n")}`);
-
-            await interaction.editReply({ embeds: [embed] });
-        } catch (error) {
-            console.error("Cleanup failed:", error);
-            await interaction.editReply({ content: "❌ An error occurred while performing cleanup." });
-        }
-    }
-});

src/lib/config.ts

@@ -69,12 +69,7 @@ export interface GameConfigType {
             autoTimeoutThreshold?: number;
         };
     };
-    system: {
+    system: Record<string, any>;
-        cleanup: {
-            intervalMs: number;
-            questArchiveDays: number;
-        };
-    };
 }
 
 // Initial default config state
@@ -167,17 +162,7 @@ const configSchema = z.object({
             dmOnWarn: true
         }
     }),
-    system: z.object({
+    system: z.record(z.string(), z.any()).default({}),
-        cleanup: z.object({
-            intervalMs: z.number().default(24 * 60 * 60 * 1000), // Daily
-            questArchiveDays: z.number().default(30)
-        })
-    }).default({
-        cleanup: {
-            intervalMs: 24 * 60 * 60 * 1000,
-            questArchiveDays: 30
-        }
-    })
 });
 
 export function reloadConfig() {

src/modules/economy/economy.service.test.ts

@@ -209,6 +209,15 @@ describe("economyService", () => {
             expect(result.streak).toBe(1);
         });
 
+        it("should preserve streak if cooldown is missing but user has a streak", async () => {
+            mockFindFirst
+                .mockResolvedValueOnce(undefined) // No cooldown
+                .mockResolvedValueOnce({ id: 1n, dailyStreak: 10 });
+
+            const result = await economyService.claimDaily("1");
+            expect(result.streak).toBe(11);
+        });
+
         it("should prevent weekly bonus exploit by resetting streak", async () => {
             // Mock user at streak 7.
             // Mock time as 24h + 1m after expiry.

src/modules/economy/economy.service.ts

@@ -68,8 +68,6 @@ export const economyService = {
     claimDaily: async (userId: string, tx?: Transaction) => {
         return await withTransaction(async (txFn) => {
             const now = new Date();
-            const startOfDay = new Date(now);
-            startOfDay.setHours(0, 0, 0, 0);
 
             // Check cooldown
             const cooldown = await txFn.query.userTimers.findFirst({
@@ -90,17 +88,23 @@ export const economyService = {
             });
 
             if (!user) {
-                throw new Error("User not found"); // This might be system error because user should exist if authenticated, but keeping simple for now
+                throw new Error("User not found");
             }
 
             let streak = (user.dailyStreak || 0) + 1;
 
-            // If previous cooldown exists and expired more than 24h ago (meaning >48h since last claim), reduce streak by one for each day passed minimum 1
+            // Check if streak should be reset due to missing a day
             if (cooldown) {
                 const timeSinceReady = now.getTime() - cooldown.expiresAt.getTime();
+                // If more than 24h passed since it became ready, they missed a full calendar day
                 if (timeSinceReady > 24 * 60 * 60 * 1000) {
                     streak = 1;
                 }
+            } else if ((user.dailyStreak || 0) > 0) {
+                // If no cooldown record exists but user has a streak,
+                // we'll allow one "free" increment to restore the timer state.
+                // This prevents unfair resets if timers were cleared/lost.
+                streak = (user.dailyStreak || 0) + 1;
             } else {
                 streak = 1;
             }

src/modules/system/cleanup.service.test.ts

@@ -1,111 +0,0 @@
-import { describe, it, expect, mock, beforeEach, spyOn } from "bun:test";
-import { cleanupService } from "./cleanup.service";
-import { lootdrops, userTimers, userQuests } from "@/db/schema";
-import { config } from "@/lib/config";
-
-const mockDelete = mock();
-const mockReturning = mock();
-const mockWhere = mock();
-const mockFindMany = mock();
-
-mockDelete.mockReturnValue({ where: mockWhere });
-mockWhere.mockReturnValue({ returning: mockReturning });
-
-// Mock DrizzleClient
-mock.module("@/lib/DrizzleClient", () => ({
-    DrizzleClient: {
-        delete: mockDelete,
-        query: {
-            userTimers: {
-                findMany: mockFindMany
-            }
-        }
-    }
-}));
-
-// Mock AuroraClient
-mock.module("@/lib/BotClient", () => ({
-    AuroraClient: {
-        guilds: {
-            fetch: mock().mockResolvedValue({
-                members: {
-                    fetch: mock().mockResolvedValue({
-                        user: { tag: "TestUser#1234" },
-                        roles: { remove: mock().mockResolvedValue({}) }
-                    })
-                }
-            })
-        }
-    }
-}));
-
-// Mock Config
-mock.module("@/lib/config", () => ({
-    config: {
-        system: {
-            cleanup: {
-                intervalMs: 86400000,
-                questArchiveDays: 30
-            }
-        }
-    }
-}));
-
-describe("cleanupService", () => {
-    beforeEach(() => {
-        mockDelete.mockClear();
-        mockWhere.mockClear();
-        mockReturning.mockClear();
-        mockFindMany.mockClear();
-    });
-
-    it("cleanupLootdrops should delete expired unclaimed lootdrops", async () => {
-        mockReturning.mockResolvedValue([{ id: "msg1" }, { id: "msg2" }]);
-
-        const count = await cleanupService.cleanupLootdrops();
-
-        expect(count).toBe(2);
-        expect(mockDelete).toHaveBeenCalledWith(lootdrops);
-    });
-
-    it("cleanupTimers should delete expired timers and handle roles", async () => {
-        // Mock findMany for expired ACCESS timers
-        mockFindMany.mockResolvedValue([
-            { userId: 123n, type: 'ACCESS', key: 'role_456', metadata: { roleId: '456' } }
-        ]);
-
-        // Mock returning for bulk delete
-        mockReturning.mockResolvedValue([{ userId: 789n }]); // One other timer
-
-        const count = await cleanupService.cleanupTimers();
-
-        // 1 from findMany + 1 from bulk delete (simplified mock behavior)
-        expect(count).toBe(2);
-        expect(mockDelete).toHaveBeenCalledWith(userTimers);
-    });
-
-    it("cleanupQuests should delete old completed quests", async () => {
-        mockReturning.mockResolvedValue([{ userId: 123n }]);
-
-        const count = await cleanupService.cleanupQuests();
-
-        expect(count).toBe(1);
-        expect(mockDelete).toHaveBeenCalledWith(userQuests);
-    });
-
-    it("runAll should run all cleanup tasks and log stats", async () => {
-        const spyLootdrops = spyOn(cleanupService, 'cleanupLootdrops').mockResolvedValue(1);
-        const spyTimers = spyOn(cleanupService, 'cleanupTimers').mockResolvedValue(2);
-        const spyQuests = spyOn(cleanupService, 'cleanupQuests').mockResolvedValue(3);
-
-        await cleanupService.runAll();
-
-        expect(spyLootdrops).toHaveBeenCalled();
-        expect(spyTimers).toHaveBeenCalled();
-        expect(spyQuests).toHaveBeenCalled();
-
-        spyLootdrops.mockRestore();
-        spyTimers.mockRestore();
-        spyQuests.mockRestore();
-    });
-});

src/modules/system/cleanup.service.ts

@@ -1,119 +0,0 @@
-import { lootdrops, userTimers, userQuests } from "@/db/schema";
-import { eq, and, lt, isNull, sql } from "drizzle-orm";
-import { DrizzleClient } from "@/lib/DrizzleClient";
-import { AuroraClient } from "@/lib/BotClient";
-import { env } from "@/lib/env";
-import { config } from "@/lib/config";
-import { TimerType } from "@/lib/constants";
-
-export const cleanupService = {
-    /**
-     * Runs all cleanup tasks
-     */
-    runAll: async () => {
-        console.log("🧹 Starting system cleanup...");
-        const stats = {
-            lootdrops: 0,
-            timers: 0,
-            quests: 0
-        };
-
-        try {
-            stats.lootdrops = await cleanupService.cleanupLootdrops();
-            stats.timers = await cleanupService.cleanupTimers();
-            stats.quests = await cleanupService.cleanupQuests();
-
-            console.log(`✅ Cleanup finished. Stats:
-- Lootdrops: ${stats.lootdrops} removed
-- Timers: ${stats.timers} removed
-- Quests: ${stats.quests} cleaned`);
-        } catch (error) {
-            console.error("❌ Error during cleanup:", error);
-        }
-    },
-
-    /**
-     * Deletes unclaimed expired lootdrops
-     */
-    cleanupLootdrops: async (): Promise<number> => {
-        const now = new Date();
-        const result = await DrizzleClient.delete(lootdrops)
-            .where(and(
-                lt(lootdrops.expiresAt, now),
-                isNull(lootdrops.claimedBy)
-            ))
-            .returning({ id: lootdrops.messageId });
-
-        return result.length;
-    },
-
-    /**
-     * Cleans up expired user timers and handles side effects (like role removal)
-     */
-    cleanupTimers: async (): Promise<number> => {
-        const now = new Date();
-        let deletedCount = 0;
-
-        // 1. Specific handling for ACCESS timers (revoking roles etc)
-        // This is migrated from scheduler.ts
-        const expiredAccess = await DrizzleClient.query.userTimers.findMany({
-            where: and(
-                eq(userTimers.type, TimerType.ACCESS),
-                lt(userTimers.expiresAt, now)
-            )
-        });
-
-        for (const timer of expiredAccess) {
-            const meta = timer.metadata as any;
-            const userIdStr = timer.userId.toString();
-
-            if (timer.key.startsWith('role_')) {
-                try {
-                    const roleId = meta?.roleId || timer.key.replace('role_', '');
-                    const guildId = env.DISCORD_GUILD_ID;
-
-                    if (guildId) {
-                        const guild = await AuroraClient.guilds.fetch(guildId);
-                        const member = await guild.members.fetch(userIdStr);
-                        await member.roles.remove(roleId);
-                        console.log(`👋 Removed temporary role ${roleId} from ${member.user.tag}`);
-                    }
-                } catch (err) {
-                    console.error(`Failed to remove role for user ${userIdStr}:`, err);
-                }
-            }
-
-            // Delete specifically this one
-            await DrizzleClient.delete(userTimers)
-                .where(and(
-                    eq(userTimers.userId, timer.userId),
-                    eq(userTimers.type, timer.type),
-                    eq(userTimers.key, timer.key)
-                ));
-            deletedCount++;
-        }
-
-        // 2. Bulk delete all other expired timers (COOLDOWN, EFFECT, etc)
-        const result = await DrizzleClient.delete(userTimers)
-            .where(lt(userTimers.expiresAt, now))
-            .returning({ userId: userTimers.userId });
-
-        deletedCount += result.length;
-        return deletedCount;
-    },
-
-    /**
-     * Deletes or archives old completed quests
-     */
-    cleanupQuests: async (): Promise<number> => {
-        const archiveDays = config.system.cleanup.questArchiveDays;
-        const threshold = new Date();
-        threshold.setDate(threshold.getDate() - archiveDays);
-
-        const result = await DrizzleClient.delete(userQuests)
-            .where(lt(userQuests.completedAt, threshold))
-            .returning({ userId: userQuests.userId });
-
-        return result.length;
-    }
-};

src/modules/system/scheduler.ts

@@ -1,32 +1,21 @@
-import { cleanupService } from "./cleanup.service";
+import { temporaryRoleService } from "./temp-role.service";
-import { config } from "@/lib/config";
 
-/**
- * The Scheduler responsible for periodic tasks and system maintenance.
- */
 export const schedulerService = {
     start: () => {
         console.log("🕒 Scheduler started: Maintenance loops initialized.");
 
-        // 1. High-frequency timer cleanup (every 60s)
+        // 1. Temporary Role Revocation (every 60s)
-        // This handles role revocations and cooldown expirations
         setInterval(() => {
-            cleanupService.cleanupTimers();
+            temporaryRoleService.processExpiredRoles();
         }, 60 * 1000);
 
-        // 2. Scheduled system cleanup (configurable, default daily)
+        // 2. Terminal Update Loop (every 60s)
-        // This handles lootdrops, quests, etc.
-        setInterval(() => {
-            cleanupService.runAll();
-        }, config.system.cleanup.intervalMs);
-
-        // 3. Terminal Update Loop (every 60s)
         const { terminalService } = require("@/modules/terminal/terminal.service");
         setInterval(() => {
             terminalService.update();
         }, 60 * 1000);
 
-        // Run an initial cleanup on start for good measure
+        // Run an initial check on start
-        cleanupService.runAll();
+        temporaryRoleService.processExpiredRoles();
     }
 };

src/modules/system/temp-role.service.test.ts

@@ -0,0 +1,114 @@
+import { describe, it, expect, mock, beforeEach } from "bun:test";
+import { temporaryRoleService } from "./temp-role.service";
+import { userTimers } from "@/db/schema";
+import { TimerType } from "@/lib/constants";
+
+const mockDelete = mock();
+const mockWhere = mock();
+const mockFindMany = mock();
+
+mockDelete.mockReturnValue({ where: mockWhere });
+
+// Mock DrizzleClient
+mock.module("@/lib/DrizzleClient", () => ({
+    DrizzleClient: {
+        delete: mockDelete,
+        query: {
+            userTimers: {
+                findMany: mockFindMany
+            }
+        }
+    }
+}));
+
+// Mock AuroraClient
+const mockRemoveRole = mock();
+const mockFetchMember = mock();
+const mockFetchGuild = mock();
+
+mock.module("@/lib/BotClient", () => ({
+    AuroraClient: {
+        guilds: {
+            fetch: mockFetchGuild
+        }
+    }
+}));
+
+mock.module("@/lib/env", () => ({
+    env: {
+        DISCORD_GUILD_ID: "guild123"
+    }
+}));
+
+describe("temporaryRoleService", () => {
+    beforeEach(() => {
+        mockDelete.mockClear();
+        mockWhere.mockClear();
+        mockFindMany.mockClear();
+        mockRemoveRole.mockClear();
+        mockFetchMember.mockClear();
+        mockFetchGuild.mockClear();
+
+        mockFetchGuild.mockResolvedValue({
+            members: {
+                fetch: mockFetchMember
+            }
+        });
+
+        mockFetchMember.mockResolvedValue({
+            user: { tag: "TestUser#1234" },
+            roles: { remove: mockRemoveRole }
+        });
+    });
+
+    it("should revoke expired roles and delete timers", async () => {
+        // Mock findMany to return an expired role timer
+        mockFindMany.mockResolvedValue([
+            {
+                userId: 123n,
+                type: TimerType.ACCESS,
+                key: 'role_456',
+                expiresAt: new Date(Date.now() - 1000),
+                metadata: { roleId: '456' }
+            }
+        ]);
+
+        const count = await temporaryRoleService.processExpiredRoles();
+
+        expect(count).toBe(1);
+        expect(mockFetchGuild).toHaveBeenCalledWith("guild123");
+        expect(mockFetchMember).toHaveBeenCalledWith("123");
+        expect(mockRemoveRole).toHaveBeenCalledWith("456");
+        expect(mockDelete).toHaveBeenCalledWith(userTimers);
+    });
+
+    it("should still delete the timer even if member is not found", async () => {
+        mockFindMany.mockResolvedValue([
+            {
+                userId: 999n,
+                type: TimerType.ACCESS,
+                key: 'role_789',
+                expiresAt: new Date(Date.now() - 1000),
+                metadata: {}
+            }
+        ]);
+
+        // Mock member fetch failure
+        mockFetchMember.mockRejectedValue(new Error("Member not found"));
+
+        const count = await temporaryRoleService.processExpiredRoles();
+
+        expect(count).toBe(1);
+        expect(mockRemoveRole).not.toHaveBeenCalled();
+        expect(mockDelete).toHaveBeenCalledWith(userTimers);
+    });
+
+    it("should return 0 if no expired timers exist", async () => {
+        mockFindMany.mockResolvedValue([]);
+
+        const count = await temporaryRoleService.processExpiredRoles();
+
+        expect(count).toBe(0);
+        expect(mockDelete).not.toHaveBeenCalled();
+    });
+});

src/modules/system/temp-role.service.ts

@@ -0,0 +1,67 @@
+import { userTimers } from "@/db/schema";
+import { eq, and, lt } from "drizzle-orm";
+import { DrizzleClient } from "@/lib/DrizzleClient";
+import { AuroraClient } from "@/lib/BotClient";
+import { env } from "@/lib/env";
+import { TimerType } from "@/lib/constants";
+
+export const temporaryRoleService = {
+    /**
+     * Checks for and revokes expired temporary roles.
+     * This is intended to run as a high-frequency maintenance task.
+     */
+    processExpiredRoles: async (): Promise<number> => {
+        const now = new Date();
+        let revokedCount = 0;
+
+        // Find all expired ACCESS (temporary role) timers
+        const expiredTimers = await DrizzleClient.query.userTimers.findMany({
+            where: and(
+                eq(userTimers.type, TimerType.ACCESS),
+                lt(userTimers.expiresAt, now)
+            )
+        });
+
+        if (expiredTimers.length === 0) return 0;
+
+        for (const timer of expiredTimers) {
+            const userIdStr = timer.userId.toString();
+            const meta = timer.metadata as any;
+
+            // We only handle keys that indicate role management
+            if (timer.key.startsWith('role_')) {
+                try {
+                    const roleId = meta?.roleId || timer.key.replace('role_', '');
+                    const guildId = env.DISCORD_GUILD_ID;
+
+                    if (guildId) {
+                        const guild = await AuroraClient.guilds.fetch(guildId);
+                        const member = await guild.members.fetch(userIdStr).catch(() => null);
+
+                        if (member) {
+                            await member.roles.remove(roleId);
+                            console.log(`👋 Temporary role ${roleId} revoked from ${member.user.tag} (Expired)`);
+                        } else {
+                            console.log(`⚠️ Could not find member ${userIdStr} to revoke role ${roleId}.`);
+                        }
+                    }
+                } catch (err) {
+                    console.error(`❌ Failed to revoke role for user ${userIdStr}:`, err);
+                }
+            }
+
+            // Always delete the timer record after trying to revoke (or if it's not a role key)
+            // to prevent repeated failed attempts.
+            await DrizzleClient.delete(userTimers)
+                .where(and(
+                    eq(userTimers.userId, timer.userId),
+                    eq(userTimers.type, timer.type),
+                    eq(userTimers.key, timer.key)
+                ));
+
+            revokedCount++;
+        }
+
+        return revokedCount;
+    }
+};