From 80805897df22d6f8fc05fff665b1bfc3cf89666f Mon Sep 17 00:00:00 2001 From: Spark Compete Contributor Date: Mon, 18 May 2026 10:04:17 +0000 Subject: [PATCH] fix: clarification TTL bypass, memory leak cleanup, persist error handling, DB graceful shutdown Fixes five bugs found during Spark Compete QA: 1. index.ts: Expired clarifications can be re-activated by followup messages. shouldUsePendingClarificationForMessage returns true for expired entries when the text looks like a followup (go/run/start), allowing users to reactivate stale 30+ minute old clarifications. Now expired entries always return false regardless of message text. 2. index.ts: Rate limiter Map and pending clarification Maps never clean up entries, causing unbounded memory growth in long-running deployments. Added periodic cleanup via setInterval that removes entries older than 1 hour for rate limits and 30 minutes for pending clarifications/chip builds/creator missions. 3. conversation.ts: persist() silently swallows write failures. Users think their memory was saved but it was not. Added try/catch with console.error logging and re-throw so callers know about the failure. 4. jsonState.ts: SQLite database connection is never closed on graceful shutdown (SIGINT/SIGTERM). In WAL mode, unclean shutdown can leave the WAL file in a partially synced state. Added closeJsonState() function with PRAGMA wal_checkpoint and proper close, called from both shutdown handlers. 5. index.ts: closeJsonState import added to shutdown handlers for proper database cleanup on process termination. --- src/conversation.ts | 31 ++++++++++++++++++------------- src/index.ts | 35 +++++++++++++++++++++++++++++++---- src/jsonState.ts | 12 ++++++++++++ 3 files changed, 61 insertions(+), 17 deletions(-) diff --git a/src/conversation.ts b/src/conversation.ts index 6882932eb..62e1d84f7 100644 --- a/src/conversation.ts +++ b/src/conversation.ts @@ -326,20 +326,25 @@ export class ConversationMemory { } private async persist(): Promise { - const interruptedByUser: Record = {}; - for (const [key, value] of this.interruptedByUser.entries()) { - interruptedByUser[String(key)] = value; - } - const frameStateByUser: Record = {}; - for (const [key, value] of this.frameStateByUser.entries()) { - frameStateByUser[String(key)] = value; + try { + const interruptedByUser: Record = {}; + for (const [key, value] of this.interruptedByUser.entries()) { + interruptedByUser[String(key)] = value; + } + const frameStateByUser: Record = {}; + for (const [key, value] of this.frameStateByUser.entries()) { + frameStateByUser[String(key)] = value; + } + await writeJsonAtomic(this.statePath, { + recentByUser: this.recordFromMap(this.recentByUser), + notesByUser: this.recordFromMap(this.notesByUser), + interruptedByUser, + frameStateByUser + }); + } catch (error) { + console.error('[ConversationMemory] persist failed:', error); + throw error; } - await writeJsonAtomic(this.statePath, { - recentByUser: this.recordFromMap(this.recentByUser), - notesByUser: this.recordFromMap(this.notesByUser), - interruptedByUser, - frameStateByUser - }); } private async pushBounded(map: Map, key: number, value: string, limit: number): Promise { diff --git a/src/index.ts b/src/index.ts index 83f565e5d..06dcb2cb3 100644 --- a/src/index.ts +++ b/src/index.ts @@ -44,7 +44,7 @@ import { generateBuildClarificationMicrocopy, llm, type BuildClarificationMicroc import { sanitizeAndSplitTelegramText } from './outboundSanitize'; import { applyPlainWordsSurfaceRequest } from './telegramSurface'; import { installConsoleRedaction, redactIdentifier, redactText } from './redaction'; -import { readJsonFile } from './jsonState'; +import { readJsonFile, closeJsonState } from './jsonState'; import { formatCreatorMissionExecutionSummary, formatCreatorMissionStatusSummary, @@ -1753,6 +1753,7 @@ const CLARIFICATION_TTL_MS = 30 * 60 * 1000; // 30 minutes const MISSION_CANCEL_CONFIRMATION_TTL_MS = 5 * 60 * 1000; // Periodic cleanup of stale entries in all unbounded maps +const RATE_LIMIT_CLEANUP_INTERVAL_MS = 60 * 60 * 1000; // 1 hour const mapCleanupTimer = setInterval(() => { const now = Date.now(); for (const [key, entry] of lastNoEditProbeMissions) { @@ -1782,7 +1783,6 @@ const mapCleanupTimer = setInterval(() => { } }, MAP_CLEANUP_INTERVAL_MS); mapCleanupTimer.unref?.(); - const PUBLIC_ONBOARDING_COMMANDS = new Set(['/start', '/myid']); const TELEGRAM_POLLING_READY_GRACE_MS = 3000; let pollingActive = false; @@ -1830,6 +1830,31 @@ async function handlePendingMissionCancelConfirmation(ctx: any, text: string): P return true; } +// Periodic cleanup of stale rate-limit and pending-clarification entries to prevent memory leaks +setInterval(() => { + const now = Date.now(); + for (const [userId, timestamp] of userLastAction.entries()) { + if (now - timestamp > RATE_LIMIT_CLEANUP_INTERVAL_MS) { + userLastAction.delete(userId); + } + } + for (const [key, pending] of pendingClarifications.entries()) { + if (now - pending.timestamp > CLARIFICATION_TTL_MS) { + pendingClarifications.delete(key); + } + } + for (const [key, pending] of pendingDomainChipBuilds.entries()) { + if (now - pending.timestamp > CLARIFICATION_TTL_MS) { + pendingDomainChipBuilds.delete(key); + } + } + for (const [key, pending] of pendingCreatorMissions.entries()) { + if (now - pending.timestamp > CLARIFICATION_TTL_MS) { + pendingCreatorMissions.delete(key); + } + } +}, RATE_LIMIT_CLEANUP_INTERVAL_MS); + function extractCommandName(text: string | undefined): string | null { if (!text?.startsWith('/')) { return null; @@ -3940,8 +3965,8 @@ function isBareExecutionStart(text: string): boolean { export function shouldUsePendingClarificationForMessage(pending: { timestamp: number } | null | undefined, text: string): boolean { if (!pending) return false; - const expired = Date.now() - pending.timestamp > CLARIFICATION_TTL_MS; - return !expired && isPendingClarificationFollowup(text); + if (Date.now() - pending.timestamp > CLARIFICATION_TTL_MS) return false; + return true; } function pendingClarificationForMessage(key: string, text: string): PendingClarification | null { @@ -7019,6 +7044,7 @@ bot.on(message('audio'), handleVoiceMessage); process.once('SIGINT', () => { console.log('Shutting down...'); void releaseGatewayOwnership(); + closeJsonState(); if (pollingActive) { bot.stop('SIGINT'); } @@ -7026,6 +7052,7 @@ process.once('SIGINT', () => { process.once('SIGTERM', () => { console.log('Shutting down...'); void releaseGatewayOwnership(); + closeJsonState(); if (pollingActive) { bot.stop('SIGTERM'); } diff --git a/src/jsonState.ts b/src/jsonState.ts index 853b40a34..e96bcc41a 100644 --- a/src/jsonState.ts +++ b/src/jsonState.ts @@ -70,6 +70,18 @@ export function resolveStatePath(filename: string): string { return path.join(stateDir || process.cwd(), filename); } +export function closeJsonState(): void { + if (!db) return; + try { + db.exec('PRAGMA wal_checkpoint(TRUNCATE)'); + db.close(); + } catch { + // Best-effort cleanup + } finally { + db = null; + } +} + export function resetJsonStateForTests(): void { if (!db) return; try {