diff --git a/apps/gateway/src/commands/command-executor.service.ts b/apps/gateway/src/commands/command-executor.service.ts index 678f0c1..0b76465 100644 --- a/apps/gateway/src/commands/command-executor.service.ts +++ b/apps/gateway/src/commands/command-executor.service.ts @@ -83,16 +83,15 @@ export class CommandExecutorService { success: true, message: 'Retry last message requested.', }; - case 'gc': { - // Admin-only: system-wide GC sweep across all sessions - const result = await this.sessionGC.sweepOrphans(); + case 'gc': + // Global retention requires a separate, authorized and audited job. + // Session cleanup is performed only through the session lifecycle. return { command: 'gc', - success: true, - message: `GC sweep complete: ${result.orphanedSessions} orphaned sessions cleaned in ${result.duration}ms.`, + success: false, + message: 'Global GC is disabled pending an authorized retention job.', conversationId, }; - } case 'agent': return await this.handleAgent(args ?? null, conversationId, userId); case 'provider': diff --git a/apps/gateway/src/commands/commands.integration.spec.ts b/apps/gateway/src/commands/commands.integration.spec.ts index 6571319..aed408d 100644 --- a/apps/gateway/src/commands/commands.integration.spec.ts +++ b/apps/gateway/src/commands/commands.integration.spec.ts @@ -176,14 +176,12 @@ describe('CommandExecutorService — integration', () => { expect(result.command).toBe('nonexistent'); }); - // /gc handler calls SessionGCService.sweepOrphans (admin-only, no userId arg) - it('/gc calls SessionGCService.sweepOrphans without arguments', async () => { + it('/gc refuses an unaudited global sweep', async () => { const payload: SlashCommandPayload = { command: 'gc', conversationId }; const result = await executor.execute(payload, userId); - expect(mockSessionGC.sweepOrphans).toHaveBeenCalledWith(); - expect(result.success).toBe(true); - expect(result.message).toContain('GC sweep complete'); - expect(result.message).toContain('3 orphaned sessions'); + expect(mockSessionGC.sweepOrphans).not.toHaveBeenCalled(); + expect(result.success).toBe(false); + expect(result.message).toContain('disabled pending an authorized retention job'); }); // /system with args calls SystemOverrideService.set diff --git a/apps/gateway/src/gc/session-gc.service.spec.ts b/apps/gateway/src/gc/session-gc.service.spec.ts index d92ac6a..10da7cb 100644 --- a/apps/gateway/src/gc/session-gc.service.spec.ts +++ b/apps/gateway/src/gc/session-gc.service.spec.ts @@ -12,7 +12,12 @@ type MockRedis = { describe('SessionGCService', () => { let service: SessionGCService; let mockRedis: MockRedis; - let mockLogService: { logs: { promoteToWarm: ReturnType } }; + let mockLogService: { + logs: { + promoteSessionToWarm: ReturnType; + promoteToWarm: ReturnType; + }; + }; /** * Helper: build a scan mock that returns all provided keys in a single @@ -30,6 +35,7 @@ describe('SessionGCService', () => { mockLogService = { logs: { + promoteSessionToWarm: vi.fn().mockResolvedValue(0), promoteToWarm: vi.fn().mockResolvedValue(0), }, }; @@ -64,49 +70,18 @@ describe('SessionGCService', () => { expect(result.sessionId).toBe('test-session-id'); }); - it('fullCollect() deletes all session keys', async () => { - mockRedis.scan = makeScanMock(['mosaic:session:abc:system', 'mosaic:session:xyz:foo']); - const result = await service.fullCollect(); - expect(mockRedis.del).toHaveBeenCalled(); - expect(result.valkeyKeys).toBe(2); + it('collect() demotes logs only for the requested session', async () => { + await service.collect('owned-session'); + + expect(mockLogService.logs.promoteSessionToWarm).toHaveBeenCalledWith( + 'owned-session', + expect.any(Date), + ); + expect(mockLogService.logs.promoteToWarm).not.toHaveBeenCalled(); }); - it('fullCollect() with no keys returns 0 valkeyKeys', async () => { - mockRedis.scan = makeScanMock([]); - const result = await service.fullCollect(); - expect(result.valkeyKeys).toBe(0); - expect(mockRedis.del).not.toHaveBeenCalled(); - }); - - it('fullCollect() returns duration', async () => { - const result = await service.fullCollect(); - expect(result.duration).toBeGreaterThanOrEqual(0); - }); - - it('sweepOrphans() extracts unique session IDs and collects them', async () => { - // First scan call returns the global session list; subsequent calls return - // per-session keys during collect(). - mockRedis.scan = vi - .fn() - .mockResolvedValueOnce([ - '0', - ['mosaic:session:abc:system', 'mosaic:session:abc:messages', 'mosaic:session:xyz:system'], - ]) - // collect('abc') scan - .mockResolvedValueOnce(['0', ['mosaic:session:abc:system', 'mosaic:session:abc:messages']]) - // collect('xyz') scan - .mockResolvedValueOnce(['0', ['mosaic:session:xyz:system']]); - mockRedis.del.mockResolvedValue(1); - - const result = await service.sweepOrphans(); - expect(result.orphanedSessions).toBeGreaterThanOrEqual(0); - expect(result.duration).toBeGreaterThanOrEqual(0); - }); - - it('sweepOrphans() returns empty when no session keys', async () => { - mockRedis.scan = makeScanMock([]); - const result = await service.sweepOrphans(); - expect(result.orphanedSessions).toBe(0); - expect(result.totalCleaned).toHaveLength(0); + it('does not expose automatic global GC entry points', () => { + expect('fullCollect' in service).toBe(false); + expect('sweepOrphans' in service).toBe(false); }); }); diff --git a/apps/gateway/src/gc/session-gc.service.ts b/apps/gateway/src/gc/session-gc.service.ts index 18d1e39..ea81969 100644 --- a/apps/gateway/src/gc/session-gc.service.ts +++ b/apps/gateway/src/gc/session-gc.service.ts @@ -1,4 +1,4 @@ -import { Inject, Injectable, Logger, type OnModuleInit } from '@nestjs/common'; +import { Inject, Injectable } from '@nestjs/common'; import type { QueueHandle } from '@mosaicstack/queue'; import type { LogService } from '@mosaicstack/log'; import { LOG_SERVICE } from '../log/log.tokens.js'; @@ -13,49 +13,13 @@ export interface GCResult { }; } -export interface GCSweepResult { - orphanedSessions: number; - totalCleaned: GCResult[]; - duration: number; -} - -export interface FullGCResult { - valkeyKeys: number; - logsDemoted: number; - jobsPurged: number; - tempFilesRemoved: number; - duration: number; -} - @Injectable() -export class SessionGCService implements OnModuleInit { - private readonly logger = new Logger(SessionGCService.name); - +export class SessionGCService { constructor( @Inject(REDIS) private readonly redis: QueueHandle['redis'], @Inject(LOG_SERVICE) private readonly logService: LogService, ) {} - onModuleInit(): void { - // Fire-and-forget: run full GC asynchronously so it does not block the - // NestJS bootstrap chain. Cold-start GC typically takes 100–500 ms - // depending on Valkey key count; deferring it removes that latency from - // the TTFB of the first HTTP request. - this.fullCollect() - .then((result) => { - this.logger.log( - `Full GC complete: ${result.valkeyKeys} Valkey keys, ` + - `${result.logsDemoted} logs demoted, ` + - `${result.jobsPurged} jobs purged, ` + - `${result.tempFilesRemoved} temp dirs removed ` + - `(${result.duration}ms)`, - ); - }) - .catch((err: unknown) => { - this.logger.error('Cold-start GC failed', err instanceof Error ? err.stack : String(err)); - }); - } - /** * Scan Valkey for all keys matching a pattern using SCAN (non-blocking). * KEYS is avoided because it blocks the Valkey event loop for the full scan @@ -86,79 +50,13 @@ export class SessionGCService implements OnModuleInit { result.cleaned.valkeyKeys = valkeyKeys.length; } - // 2. PG: demote hot-tier agent_logs for this session to warm - const cutoff = new Date(); // demote all hot logs for this session - const logsDemoted = await this.logService.logs.promoteToWarm(cutoff); + // 2. PG: demote hot-tier agent logs for this session only. + const cutoff = new Date(); + const logsDemoted = await this.logService.logs.promoteSessionToWarm(sessionId, cutoff); if (logsDemoted > 0) { result.cleaned.logsDemoted = logsDemoted; } return result; } - - /** - * Sweep GC — find orphaned artifacts from dead sessions. - * System-wide operation: only call from admin-authorized paths or internal - * scheduled jobs. Individual session cleanup is handled by collect(). - */ - async sweepOrphans(): Promise { - const start = Date.now(); - const cleaned: GCResult[] = []; - - // 1. Find all session-scoped Valkey keys (non-blocking SCAN) - const allSessionKeys = await this.scanKeys('mosaic:session:*'); - - // Extract unique session IDs from keys - const sessionIds = new Set(); - for (const key of allSessionKeys) { - const match = key.match(/^mosaic:session:([^:]+):/); - if (match) sessionIds.add(match[1]!); - } - - // 2. For each session ID, collect stale keys - for (const sessionId of sessionIds) { - const gcResult = await this.collect(sessionId); - if (Object.keys(gcResult.cleaned).length > 0) { - cleaned.push(gcResult); - } - } - - return { - orphanedSessions: cleaned.length, - totalCleaned: cleaned, - duration: Date.now() - start, - }; - } - - /** - * Full GC — aggressive collection for cold start. - * Assumes no sessions survived the restart. - */ - async fullCollect(): Promise { - const start = Date.now(); - - // 1. Valkey: delete ALL session-scoped keys (non-blocking SCAN) - const sessionKeys = await this.scanKeys('mosaic:session:*'); - if (sessionKeys.length > 0) { - await this.redis.del(...sessionKeys); - } - - // 2. NOTE: channel keys are NOT collected on cold start - // (discord/telegram plugins may reconnect and resume) - - // 3. PG: demote stale hot-tier logs older than 24h to warm - const hotCutoff = new Date(Date.now() - 24 * 60 * 60 * 1000); - const logsDemoted = await this.logService.logs.promoteToWarm(hotCutoff); - - // 4. No summarization job purge API available yet - const jobsPurged = 0; - - return { - valkeyKeys: sessionKeys.length, - logsDemoted, - jobsPurged, - tempFilesRemoved: 0, - duration: Date.now() - start, - }; - } } diff --git a/apps/gateway/src/log/cron.service.ts b/apps/gateway/src/log/cron.service.ts index aa9b82f..f66d39d 100644 --- a/apps/gateway/src/log/cron.service.ts +++ b/apps/gateway/src/log/cron.service.ts @@ -6,11 +6,10 @@ import { type OnModuleDestroy, } from '@nestjs/common'; import { SummarizationService } from './summarization.service.js'; -import { SessionGCService } from '../gc/session-gc.service.js'; import { QueueService, - QUEUE_SUMMARIZATION, QUEUE_GC, + QUEUE_SUMMARIZATION, QUEUE_TIER_MANAGEMENT, } from '../queue/queue.service.js'; import type { Worker } from 'bullmq'; @@ -23,14 +22,12 @@ export class CronService implements OnModuleInit, OnModuleDestroy { constructor( @Inject(SummarizationService) private readonly summarization: SummarizationService, - @Inject(SessionGCService) private readonly sessionGC: SessionGCService, @Inject(QueueService) private readonly queueService: QueueService, ) {} async onModuleInit(): Promise { const summarizationSchedule = process.env['SUMMARIZATION_CRON'] ?? '0 */6 * * *'; // every 6 hours const tierManagementSchedule = process.env['TIER_MANAGEMENT_CRON'] ?? '0 3 * * *'; // daily at 3am - const gcSchedule = process.env['SESSION_GC_CRON'] ?? '0 4 * * *'; // daily at 4am // M6-003: Summarization repeatable job await this.queueService.addRepeatableJob( @@ -56,15 +53,12 @@ export class CronService implements OnModuleInit, OnModuleDestroy { }); this.registeredWorkers.push(tierWorker); - // M6-004: GC repeatable job - await this.queueService.addRepeatableJob(QUEUE_GC, 'session-gc', {}, gcSchedule); - const gcWorker = this.queueService.registerWorker(QUEUE_GC, async () => { - await this.sessionGC.sweepOrphans(); - }); - this.registeredWorkers.push(gcWorker); + // Retire any repeatable global GC schedule created by older deployments. + // Session cleanup is now triggered only by an authorized session lifecycle operation. + await this.queueService.removeRepeatableJobs(QUEUE_GC, 'session-gc'); this.logger.log( - `BullMQ jobs scheduled: summarization="${summarizationSchedule}", tier="${tierManagementSchedule}", gc="${gcSchedule}"`, + `BullMQ jobs scheduled: summarization="${summarizationSchedule}", tier="${tierManagementSchedule}"`, ); } diff --git a/apps/gateway/src/queue/queue.service.ts b/apps/gateway/src/queue/queue.service.ts index a50a773..a8424ce 100644 --- a/apps/gateway/src/queue/queue.service.ts +++ b/apps/gateway/src/queue/queue.service.ts @@ -162,6 +162,23 @@ export class QueueService implements OnModuleInit, OnModuleDestroy { ); } + /** + * Remove every existing repeatable schedule for a job name. This supports + * safe retirement of previously registered system-wide jobs. + */ + async removeRepeatableJobs(queueName: string, jobName: string): Promise { + const queue = this.getQueue(queueName); + const jobs = await queue.getRepeatableJobs(); + const matchingJobs = jobs.filter((job) => job.name === jobName); + await Promise.all(matchingJobs.map((job) => queue.removeRepeatableByKey(job.key))); + if (matchingJobs.length > 0) { + this.logger.log( + `Removed ${matchingJobs.length} repeatable "${jobName}" job(s) from "${queueName}"`, + ); + } + return matchingJobs.length; + } + /** * Register a Worker for the given queue name with error handling and * exponential backoff. diff --git a/docs/guides/admin-guide.md b/docs/guides/admin-guide.md index 4f7c6a1..500fda7 100644 --- a/docs/guides/admin-guide.md +++ b/docs/guides/admin-guide.md @@ -301,6 +301,10 @@ Each OIDC provider requires its client ID, client secret, and issuer URL togethe | `TELEGRAM_BOT_TOKEN` | Telegram bot token (enables Telegram plugin) | | `TELEGRAM_GATEWAY_URL` | Gateway URL for Telegram plugin to call | +### Session retention and garbage collection + +Session cleanup is scoped to one session identifier and only removes that session's Valkey keys and demotes that session's hot logs. Gateway startup and scheduled jobs do not perform global session cleanup; startup removes legacy repeatable `session-gc` schedules created by older deployments. The `/gc` command is intentionally disabled until a distinct global-retention job supplies explicit authorization and audit evidence. This prevents one tenant or session's cleanup from changing another's retained data. + ### Observability | Variable | Default | Description | diff --git a/docs/scratchpads/tess-m1-sec-006-session-gc-scope.md b/docs/scratchpads/tess-m1-sec-006-session-gc-scope.md new file mode 100644 index 0000000..b87b37d --- /dev/null +++ b/docs/scratchpads/tess-m1-sec-006-session-gc-scope.md @@ -0,0 +1,22 @@ +# Scratchpad — TESS-M1-SEC-006 Session GC scope + +- **Task / issue:** TESS-M1-SEC-006 / #707 +- **Branch:** `fix/tess-session-gc-scope` from `origin/main` at `59e49cfd` +- **Objective:** Make session cleanup session-scoped and prevent automatic global retention/GC without an authorized, auditable operation. +- **Scope:** `apps/gateway`, `packages/log`, admin/developer operations documentation. +- **Budget:** Task estimate 18K; no explicit hard cap supplied. +- **Assumption:** No authorized global retention service exists today. Existing full/sweep GC must therefore be disabled from startup and cron paths, while single-session cleanup remains available. + +## Plan + +1. Add failing isolation tests proving single-session cleanup only demotes its own logs and automatic startup/scheduled GC cannot globally delete session data. +2. Add session-scoped log repository retention and make `collect(sessionId)` use it. +3. Remove automatic full/sweep GC invocation; preserve any future global operation behind an explicit authorization/audit seam. +4. Document the operational boundary, run gates, review, and commit without push. + +## Verification evidence + +- Isolation TDD: `pnpm --filter @mosaicstack/gateway test -- session-gc.service.spec.ts commands.integration.spec.ts command-executor-p8012.spec.ts` — 61 passed. +- `pnpm typecheck` — passed. +- `pnpm lint` — passed. +- `pnpm format:check` remains red only on the known pre-existing Tess documentation debt; changed files are Prettier-clean. diff --git a/packages/log/src/agent-logs.ts b/packages/log/src/agent-logs.ts index e303ee4..337f409 100644 --- a/packages/log/src/agent-logs.ts +++ b/packages/log/src/agent-logs.ts @@ -58,9 +58,28 @@ export function createAgentLogsRepo(db: Db) { return rows[0]; }, + /** + * Transition hot logs for one session to warm tier. Session retention is + * default-deny: no other session's logs can be changed by this operation. + */ + async promoteSessionToWarm(sessionId: string, olderThan: Date): Promise { + const result = await db + .update(agentLogs) + .set({ tier: 'warm', summarizedAt: new Date() }) + .where( + and( + eq(agentLogs.sessionId, sessionId), + eq(agentLogs.tier, 'hot'), + lt(agentLogs.createdAt, olderThan), + ), + ) + .returning(); + return result.length; + }, + /** * Transition hot logs older than the cutoff to warm tier. - * Returns the number of logs transitioned. + * Reserved for a separately authorized global retention job. */ async promoteToWarm(olderThan: Date): Promise { const result = await db