From aa8915115361b09b5780b28790dbd458f0af202c Mon Sep 17 00:00:00 2001 From: Marc Liu Date: Mon, 3 Aug 2026 08:26:09 -0400 Subject: [PATCH 1/4] feat(daemon): add retention sweeps + incremental auto_vacuum [#2339] Extend the daily job_queue.cleanup handler with configurable retention sweeps for terminal external/github events (+deliveries), mcp_audit_log, and space_goal_events, plus incremental_vacuum(500) page reclamation. - Retention deletion is OFF by default (HYPERNEO_RETENTION_ENABLED=1); each TTL is independently configurable. Only terminal-state rows are pruned; in-flight events are always kept so an active pipeline never loses work. - Enable PRAGMA auto_vacuum = INCREMENTAL on fresh DBs; convert existing auto_vacuum=NONE DBs via opt-in migration 169 (HYPERNEO_DB_VACUUM_MIGRATION), since a full VACUUM on a multi-GB DB is a long operation to schedule deliberately. incremental_vacuum then shrinks the file as pages are freed by retention + normal deletes. sdk_messages retention is intentionally deferred (out of scope for this pass; flagged for a separate policy decision). --- packages/daemon/src/app.ts | 2 +- .../src/lib/job-handlers/cleanup.handler.ts | 37 +- .../daemon/src/lib/job-handlers/retention.ts | 233 +++++++++++ packages/daemon/src/storage/database-core.ts | 18 +- .../daemon/src/storage/schema/migrations.ts | 50 +++ .../job-handlers/cleanup-handler.test.ts | 16 +- .../2-handlers/job-handlers/retention.test.ts | 388 ++++++++++++++++++ .../storage/database-core.test.ts | 11 + .../storage/migrations/migration-169_test.ts | 101 +++++ 9 files changed, 843 insertions(+), 13 deletions(-) create mode 100644 packages/daemon/src/lib/job-handlers/retention.ts create mode 100644 packages/daemon/tests/unit/2-handlers/job-handlers/retention.test.ts create mode 100644 packages/daemon/tests/unit/4-space-storage/storage/migrations/migration-169_test.ts diff --git a/packages/daemon/src/app.ts b/packages/daemon/src/app.ts index 3d5a782360..bd28ac2ce8 100644 --- a/packages/daemon/src/app.ts +++ b/packages/daemon/src/app.ts @@ -921,7 +921,7 @@ export async function createDaemonApp(options: CreateDaemonAppOptions): Promise< SKILL_VALIDATE, createSkillValidateHandler(skillsManager, db.appMcpServers) ); - jobProcessor.register(JOB_QUEUE_CLEANUP, createCleanupHandler(jobQueue)); + jobProcessor.register(JOB_QUEUE_CLEANUP, createCleanupHandler(jobQueue, db.getDatabase())); jobProcessor.register( MEMORY_CONSOLIDATION, createMemoryConsolidationHandler(db.agentMemory, jobQueue) diff --git a/packages/daemon/src/lib/job-handlers/cleanup.handler.ts b/packages/daemon/src/lib/job-handlers/cleanup.handler.ts index 2ebf8c0b1a..472ad98ef3 100644 --- a/packages/daemon/src/lib/job-handlers/cleanup.handler.ts +++ b/packages/daemon/src/lib/job-handlers/cleanup.handler.ts @@ -1,13 +1,44 @@ +import type { Database as BunDatabase } from 'bun:sqlite'; import type { Job, JobQueueRepository } from '../../storage/repositories/job-queue-repository'; import { JOB_QUEUE_CLEANUP } from '../job-queue-constants'; +import { Logger } from '../logger'; +import { + loadRetentionConfig, + type RetentionConfig, + type RetentionStats, + runRetention, +} from './retention'; const DEFAULT_MAX_AGE_MS = 7 * 24 * 60 * 60 * 1000; // 7 days const NEXT_RUN_DELAY_MS = 24 * 60 * 60 * 1000; // 24 hours -export function createCleanupHandler(jobQueue: JobQueueRepository) { - return async (_job: Job): Promise<{ deletedJobs: number; nextRunAt: number }> => { +const logger = new Logger('Cleanup'); + +/** + * Build the daily cleanup job handler. + * + * Two responsibilities, both self-perpetuating on a 24h cadence: + * 1. Reap terminal job_queue rows older than 7 days (pre-existing). + * 2. Run retention sweeps (events / audit / goal events) + reclaim freed pages + * via incremental_vacuum. Retention deletion is OFF by default; see + * `loadRetentionConfig`. `retentionConfig` is optional and mainly for tests — + * production reads env vars per run so operators can tune TTLs live. + */ +export function createCleanupHandler( + jobQueue: JobQueueRepository, + db: BunDatabase, + retentionConfig?: RetentionConfig +) { + return async ( + _job: Job + ): Promise<{ deletedJobs: number; retention: RetentionStats; nextRunAt: number }> => { const deletedJobs = jobQueue.cleanup(Date.now() - DEFAULT_MAX_AGE_MS); + const retention = runRetention(db, retentionConfig ?? loadRetentionConfig()); + logger.info( + `retention sweep: external=${retention.externalEvents} deliveries=${retention.deliveries} github=${retention.githubEvents} mcpAudit=${retention.mcpAudit} goalEvents=${retention.goalEvents} vacuumedPages=${retention.vacuumedPages}` + ); + const nextRunAt = Date.now() + NEXT_RUN_DELAY_MS; // Self-schedule: only enqueue next cleanup if none is already pending @@ -16,6 +47,6 @@ export function createCleanupHandler(jobQueue: JobQueueRepository) { jobQueue.enqueue({ queue: JOB_QUEUE_CLEANUP, payload: {}, runAt: nextRunAt }); } - return { deletedJobs, nextRunAt }; + return { deletedJobs, retention, nextRunAt }; }; } diff --git a/packages/daemon/src/lib/job-handlers/retention.ts b/packages/daemon/src/lib/job-handlers/retention.ts new file mode 100644 index 0000000000..ed646e1b26 --- /dev/null +++ b/packages/daemon/src/lib/job-handlers/retention.ts @@ -0,0 +1,233 @@ +/** + * Retention sweeps for append-heavy / event-log tables. + * + * The daemon DB grows monotonically: external/github events, the MCP audit log + * and space goal events are written continuously and never reaped. Over time the + * file outgrows the OS page cache and scans hit cold disk. This module prunes + * rows that have reached a terminal state and are older than a configurable TTL, + * then reclaims the freed pages via `incremental_vacuum`. + * + * Policy: + * - Deletion is OFF by default (`HYPERNEO_RETENTION_ENABLED=1` to activate) and + * each TTL is independently configurable. Default windows only matter once + * enabled — they are intentionally conservative. + * - Only terminal-state event rows are pruned; in-flight states (published / + * routed / received / pending) are always kept so an active pipeline never + * loses work. + * - `incremental_vacuum(500)` runs every cycle regardless of `enabled`: it is + * space maintenance, not deletion, and reclaims pages freed by any delete + * (including the pre-existing 7-day job_queue and worktree reapers). It is a + * no-op unless the DB is in incremental-vacuum mode (see migration 169 / + * DatabaseCore's fresh-DB pragma). + */ + +import type { Database as BunDatabase } from 'bun:sqlite'; + +const DAY_MS = 24 * 60 * 60 * 1000; + +export interface RetentionConfig { + /** Master switch for deletion sweeps. Vacuum still runs when disabled. */ + enabled: boolean; + /** TTL (days) for terminal external + github events and their deliveries. */ + eventsDays: number; + /** TTL (days) for mcp_audit_log rows. */ + mcpAuditDays: number; + /** TTL (days) for space_goal_events rows. */ + goalEventsDays: number; + /** Max pages reclaimed per incremental_vacuum. 0 disables vacuum. */ + vacuumPages: number; +} + +export interface RetentionStats { + externalEvents: number; + deliveries: number; + githubEvents: number; + mcpAudit: number; + goalEvents: number; + vacuumedPages: number; +} + +function emptyStats(): RetentionStats { + return { + externalEvents: 0, + deliveries: 0, + githubEvents: 0, + mcpAudit: 0, + goalEvents: 0, + vacuumedPages: 0, + }; +} + +function readEnv(name: string): string | undefined { + return process.env[`HYPERNEO_RETENTION_${name}`]; +} + +function envBool(value: string | undefined): boolean { + return value === '1' || value === 'true'; +} + +function envInt(value: string | undefined, fallback: number): number { + if (value === undefined || value === '') return fallback; + const parsed = Number(value); + if (!Number.isFinite(parsed) || parsed < 0) return fallback; + return Math.floor(parsed); +} + +/** + * Read retention configuration from `HYPERNEO_RETENTION_*` env vars. + * + * Read per-run (not cached at startup) so operators can adjust TTLs without a + * daemon restart. Defaults are conservative and only take effect once enabled. + */ +export function loadRetentionConfig(): RetentionConfig { + return { + enabled: envBool(readEnv('ENABLED')), + eventsDays: envInt(readEnv('EVENTS_DAYS'), 14), + mcpAuditDays: envInt(readEnv('MCP_AUDIT_DAYS'), 30), + goalEventsDays: envInt(readEnv('GOAL_EVENTS_DAYS'), 60), + vacuumPages: envInt(readEnv('VACUUM_PAGES'), 500), + }; +} + +// Terminal (resolved) states for the external-event pipeline. `published` / +// `routed` are in-flight (may still be delivered) and are never pruned. +const EXTERNAL_EVENT_TERMINAL_STATES = [ + 'delivered', + 'delivery_failed', + 'failed', + 'ignored', + 'ambiguous', +] as const; + +// Deliveries: `pending` is in-flight; only resolved deliveries are pruned. +const DELIVERY_TERMINAL_STATES = ['delivered', 'failed'] as const; + +// GitHub events: `received` / `routed` are in-flight; the rest are resolved. +const GITHUB_EVENT_TERMINAL_STATES = [ + 'processed', + 'ignored', + 'ambiguous', + 'delivered', + 'failed', +] as const; + +function tableExists(db: BunDatabase, name: string): boolean { + return !!db.prepare(`SELECT name FROM sqlite_master WHERE type = 'table' AND name = ?`).get(name); +} + +function placeholders(count: number): string { + return Array.from({ length: count }, () => '?').join(','); +} + +/** + * Count the matching rows, then delete them, returning the pre-delete count. + * + * We count first because `DELETE ... .changes` includes rows removed by FK + * CASCADE (e.g. a pruned external event pulls its deliveries), which would + * inflate the per-table stat. A SELECT COUNT is accurate and cheap (the age + * columns are indexed). The `whereClause` may carry `?` placeholders bound by + * `params`, used identically for the count and the delete. + */ +function prune( + db: BunDatabase, + table: string, + whereClause: string, + params: (string | number)[] +): number { + const count = ( + db.prepare(`SELECT COUNT(*) AS c FROM ${table} WHERE ${whereClause}`).get(...params) as { + c: number; + } + ).c; + if (count > 0) { + db.prepare(`DELETE FROM ${table} WHERE ${whereClause}`).run(...params); + } + return count; +} + +/** Current page count of the database file. */ +function pageCount(db: BunDatabase): number { + const row = db.prepare('PRAGMA page_count').get() as { page_count?: number } | null; + return Number(row?.page_count ?? 0); +} + +/** + * Reclaim up to `maxPages` free pages from the end of the file. + * + * Returns the number of pages freed (page_count delta). A no-op — returns 0 — + * when the DB is not in incremental-vacuum mode (auto_vacuum != INCREMENTAL), + * which covers pre-migration existing databases. + */ +export function incrementalVacuum(db: BunDatabase, maxPages: number): number { + if (maxPages <= 0) return 0; + const mode = db.prepare('PRAGMA auto_vacuum').get() as { auto_vacuum?: number } | null; + if (Number(mode?.auto_vacuum ?? 0) !== 2) return 0; // 2 == INCREMENTAL + + const before = pageCount(db); + // Pragma arguments cannot be bound; maxPages is a validated non-negative int. + db.exec(`PRAGMA incremental_vacuum(${maxPages})`); + const after = pageCount(db); + const freed = before - after; + return freed > 0 ? freed : 0; +} + +/** + * Run retention sweeps for the configured tables, then reclaim freed pages. + * + * Each table is guarded by `tableExists` so this is safe to run against a DB + * that hasn't yet had a given table created (e.g. minimal test schemas, or a + * fresh DB mid-bootstrap). When `config.enabled` is false, no rows are deleted + * but `incremental_vacuum` still runs. + */ +export function runRetention(db: BunDatabase, config: RetentionConfig): RetentionStats { + const stats = emptyStats(); + + if (config.enabled) { + const now = Date.now(); + const eventsCutoff = now - config.eventsDays * DAY_MS; + const mcpCutoff = now - config.mcpAuditDays * DAY_MS; + const goalCutoff = now - config.goalEventsDays * DAY_MS; + + // Prune resolved deliveries BEFORE their events: the independent sweep reaps + // old resolved deliveries under a kept (in-flight) event, and runs with an + // accurate count. The event delete below then cascades any remaining + // deliveries of pruned events (uncounted, by design). + if (tableExists(db, 'space_external_event_deliveries')) { + stats.deliveries = prune( + db, + 'space_external_event_deliveries', + `state IN (${placeholders(DELIVERY_TERMINAL_STATES.length)}) AND updated_at < ?`, + [...DELIVERY_TERMINAL_STATES, eventsCutoff] + ); + } + + if (tableExists(db, 'space_external_events')) { + stats.externalEvents = prune( + db, + 'space_external_events', + `state IN (${placeholders(EXTERNAL_EVENT_TERMINAL_STATES.length)}) AND updated_at < ?`, + [...EXTERNAL_EVENT_TERMINAL_STATES, eventsCutoff] + ); + } + + if (tableExists(db, 'space_github_events')) { + stats.githubEvents = prune( + db, + 'space_github_events', + `state IN (${placeholders(GITHUB_EVENT_TERMINAL_STATES.length)}) AND updated_at < ?`, + [...GITHUB_EVENT_TERMINAL_STATES, eventsCutoff] + ); + } + + if (tableExists(db, 'mcp_audit_log')) { + stats.mcpAudit = prune(db, 'mcp_audit_log', 'timestamp < ?', [mcpCutoff]); + } + + if (tableExists(db, 'space_goal_events')) { + stats.goalEvents = prune(db, 'space_goal_events', 'created_at < ?', [goalCutoff]); + } + } + + stats.vacuumedPages = incrementalVacuum(db, config.vacuumPages); + return stats; +} diff --git a/packages/daemon/src/storage/database-core.ts b/packages/daemon/src/storage/database-core.ts index 6ef897cadd..0275ba9297 100644 --- a/packages/daemon/src/storage/database-core.ts +++ b/packages/daemon/src/storage/database-core.ts @@ -44,9 +44,25 @@ export class DatabaseCore { mkdirSync(dir, { recursive: true }); } - // Open database + // Open database. Detect a fresh file (didn't exist before opening) so we can + // set auto_vacuum = INCREMENTAL before any table is created — the pragma is + // only effective on an empty DB. + const dbFileExisted = existsSync(this.dbPath); this.db = new BunDatabase(this.dbPath); + // Enable incremental auto-vacuum on FRESH databases only. auto_vacuum is a + // header flag that only takes effect when set before the first table is + // created; on an existing DB the flip is a silent no-op until a full VACUUM. + // Existing (auto_vacuum = NONE) databases are converted by the opt-in + // migration 169 (HYPERNEO_DB_VACUUM_MIGRATION) — VACUUM on a multi-GB DB is a + // long, disk-intensive operation to schedule deliberately. Once INCREMENTAL, + // the daily cleanup job runs incremental_vacuum(500) to reclaim pages freed + // by retention sweeps and normal deletes, so the file shrinks over time + // instead of growing monotonically. + if (!dbFileExisted) { + this.db.exec('PRAGMA auto_vacuum = INCREMENTAL'); + } + // Enable WAL mode for better concurrency and crash recovery // WAL mode provides: // - Better performance for concurrent reads/writes diff --git a/packages/daemon/src/storage/schema/migrations.ts b/packages/daemon/src/storage/schema/migrations.ts index 8223a07b0e..36321e32de 100644 --- a/packages/daemon/src/storage/schema/migrations.ts +++ b/packages/daemon/src/storage/schema/migrations.ts @@ -793,6 +793,20 @@ export function runMigrations(db: BunDatabase, createBackup: () => void): void { // lookup. (The live-query task-scope nodeExecStmt already drives off // idx_node_executions_run and is unaffected.) run(migrationMarkerKey(168), () => runMigration168(db)); + + // Migration 169: Convert existing auto_vacuum = NONE databases to INCREMENTAL + // so the daily cleanup job's incremental_vacuum(500) can reclaim pages freed + // by retention sweeps and normal deletes. Fresh databases already get + // INCREMENTAL at creation (DatabaseCore), so this no-ops for them. + // + // GATED behind HYPERNEO_DB_VACUUM_MIGRATION: a full VACUUM rebuilds the entire + // file (temporarily doubling disk usage), so on a multi-GB production DB it is + // a long, disk-intensive operation that must be scheduled deliberately (e.g. + // during a maintenance window). When the flag is unset the migration is not + // registered and not marked, so setting the flag + restarting runs it once. + if (envFlag(process.env.HYPERNEO_DB_VACUUM_MIGRATION)) { + run(migrationMarkerKey(169), () => runMigration169(db)); + } } function migrationMarkerKey(version: number): string { @@ -11477,3 +11491,39 @@ export function runMigration168(db: BunDatabase): void { ON node_executions(agent_session_id) `); } + +/** + * Read the database's `auto_vacuum` mode from its header. + * 0 = NONE (SQLite default), 1 = FULL, 2 = INCREMENTAL. + */ +function readAutoVacuumMode(db: BunDatabase): number { + const row = db.prepare('PRAGMA auto_vacuum').get() as { auto_vacuum?: number } | undefined; + return Number(row?.auto_vacuum ?? 0); +} + +/** + * Migration 169: Convert auto_vacuum = NONE to INCREMENTAL via a full VACUUM. + * + * Fresh databases are created with auto_vacuum = INCREMENTAL by DatabaseCore, so + * this is a no-op for them (mode is already 2). FULL (1) is left untouched — it + * is not a target mode and changing it is out of scope. For a NONE database, + * setting the pragma records the desired mode in the header and the subsequent + * VACUUM rebuilds the file in incremental mode. After conversion, freed pages + * land on the free-list and incremental_vacuum(500) (daily cleanup job) reclaims + * them so the file shrinks instead of growing monotonically. + * + * See registration site: gated behind HYPERNEO_DB_VACUUM_MIGRATION. + */ +export function runMigration169(db: BunDatabase): void { + const mode = readAutoVacuumMode(db); + if (mode === 2) return; // already INCREMENTAL + if (mode === 1) return; // FULL — leave untouched + // mode === 0 (NONE) → rebuild into incremental mode. + db.exec('PRAGMA auto_vacuum = INCREMENTAL'); + db.exec('VACUUM'); +} + +/** Truthy check for an opt-in `HYPERNEO_*` env flag. */ +function envFlag(value: string | undefined): boolean { + return value === '1' || value === 'true'; +} diff --git a/packages/daemon/tests/unit/2-handlers/job-handlers/cleanup-handler.test.ts b/packages/daemon/tests/unit/2-handlers/job-handlers/cleanup-handler.test.ts index ce0f11a91c..40d2b5f894 100644 --- a/packages/daemon/tests/unit/2-handlers/job-handlers/cleanup-handler.test.ts +++ b/packages/daemon/tests/unit/2-handlers/job-handlers/cleanup-handler.test.ts @@ -1,9 +1,9 @@ -import { describe, it, expect, beforeEach, afterEach } from 'bun:test'; import { Database } from 'bun:sqlite'; -import { JobQueueRepository } from '../../../../src/storage/repositories/job-queue-repository'; +import { afterEach, beforeEach, describe, expect, it } from 'bun:test'; import { createCleanupHandler } from '../../../../src/lib/job-handlers/cleanup.handler'; import { JOB_QUEUE_CLEANUP } from '../../../../src/lib/job-queue-constants'; import type { Job } from '../../../../src/storage/repositories/job-queue-repository'; +import { JobQueueRepository } from '../../../../src/storage/repositories/job-queue-repository'; function createTestDb(): Database { const db = new Database(':memory:'); @@ -77,7 +77,7 @@ describe('createCleanupHandler', () => { VALUES ('recent-completed', 'some.queue', 'completed', '{}', 0, 3, 0, ${recentTime}, ${recentTime}, ${recentTime}) `); - const handler = createCleanupHandler(jobQueue); + const handler = createCleanupHandler(jobQueue, db); const result = await handler(fakeJob); expect(result.deletedJobs).toBe(1); @@ -96,7 +96,7 @@ describe('createCleanupHandler', () => { VALUES ('old-dead', 'some.queue', 'dead', '{}', 0, 3, 3, ${eightDaysAgo}, ${eightDaysAgo}, ${eightDaysAgo}) `); - const handler = createCleanupHandler(jobQueue); + const handler = createCleanupHandler(jobQueue, db); const result = await handler(fakeJob); expect(result.deletedJobs).toBe(1); @@ -108,7 +108,7 @@ describe('createCleanupHandler', () => { }); it('self-schedules the next cleanup job ~24 hours from now', async () => { - const handler = createCleanupHandler(jobQueue); + const handler = createCleanupHandler(jobQueue, db); const before = Date.now(); const result = await handler(fakeJob); const after = Date.now(); @@ -128,7 +128,7 @@ describe('createCleanupHandler', () => { // Pre-enqueue a pending cleanup job jobQueue.enqueue({ queue: JOB_QUEUE_CLEANUP, payload: {}, runAt: Date.now() + 1000 }); - const handler = createCleanupHandler(jobQueue); + const handler = createCleanupHandler(jobQueue, db); await handler(fakeJob); const pending = jobQueue.listJobs({ queue: JOB_QUEUE_CLEANUP, status: 'pending', limit: 10 }); @@ -145,7 +145,7 @@ describe('createCleanupHandler', () => { VALUES ('old-failed', 'some.queue', 'failed', '{}', 0, 3, 1, ${eightDaysAgo}, ${eightDaysAgo}, ${eightDaysAgo}) `); - const handler = createCleanupHandler(jobQueue); + const handler = createCleanupHandler(jobQueue, db); const result = await handler(fakeJob); expect(result.deletedJobs).toBe(1); @@ -160,7 +160,7 @@ describe('createCleanupHandler', () => { VALUES ('recent', 'some.queue', 'completed', '{}', 0, 3, 0, ${recentTime}, ${recentTime}, ${recentTime}) `); - const handler = createCleanupHandler(jobQueue); + const handler = createCleanupHandler(jobQueue, db); const result = await handler(fakeJob); expect(result.deletedJobs).toBe(0); diff --git a/packages/daemon/tests/unit/2-handlers/job-handlers/retention.test.ts b/packages/daemon/tests/unit/2-handlers/job-handlers/retention.test.ts new file mode 100644 index 0000000000..25cd07b9db --- /dev/null +++ b/packages/daemon/tests/unit/2-handlers/job-handlers/retention.test.ts @@ -0,0 +1,388 @@ +import { Database } from 'bun:sqlite'; +import { afterEach, beforeEach, describe, expect, it } from 'bun:test'; +import { + incrementalVacuum, + loadRetentionConfig, + runRetention, +} from '../../../../src/lib/job-handlers/retention'; + +const DAY = 24 * 60 * 60 * 1000; + +/** + * Minimal schema for the retention-target tables. Mirrors the production DDL + * (CHECK constraints, FKs, epoch-ms timestamp columns) closely enough to + * exercise the delete queries. `spaces` exists so the external_events FK resolves. + */ +function createRetentionSchema(db: Database): void { + db.exec('PRAGMA foreign_keys = ON'); + db.exec(`CREATE TABLE spaces (id TEXT PRIMARY KEY)`); + db.exec(`INSERT INTO spaces (id) VALUES ('sp1')`); + + db.exec(` + CREATE TABLE space_external_events ( + id TEXT PRIMARY KEY, + space_id TEXT NOT NULL, + source TEXT NOT NULL, + topic TEXT NOT NULL, + dedupe_key TEXT NOT NULL, + occurred_at INTEGER NOT NULL, + ingested_at INTEGER NOT NULL, + summary TEXT NOT NULL DEFAULT '', + payload_json TEXT NOT NULL DEFAULT '{}', + state TEXT NOT NULL DEFAULT 'published' + CHECK(state IN ('published','routed','delivered','delivery_failed','failed','ignored','ambiguous')), + created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL, + UNIQUE(space_id, source, dedupe_key), + FOREIGN KEY (space_id) REFERENCES spaces(id) ON DELETE CASCADE + ) + `); + + db.exec(` + CREATE TABLE space_external_event_deliveries ( + event_id TEXT NOT NULL, + delivery_key TEXT NOT NULL, + workflow_run_id TEXT NOT NULL, + task_id TEXT NOT NULL, + node_id TEXT NOT NULL, + agent_name TEXT NOT NULL, + state TEXT NOT NULL DEFAULT 'pending' CHECK(state IN ('pending','delivered','failed')), + failure_reason TEXT, + delivered_at INTEGER, + updated_at INTEGER NOT NULL, + PRIMARY KEY(event_id, delivery_key), + FOREIGN KEY (event_id) REFERENCES space_external_events(id) ON DELETE CASCADE + ) + `); + + db.exec(` + CREATE TABLE space_github_events ( + id TEXT PRIMARY KEY, + space_id TEXT NOT NULL, + task_id TEXT, + source TEXT NOT NULL CHECK(source IN ('webhook','polling')), + delivery_id TEXT NOT NULL, + event_type TEXT NOT NULL, + action TEXT NOT NULL, + repo_owner TEXT NOT NULL, + repo_name TEXT NOT NULL, + pr_number INTEGER NOT NULL, + pr_url TEXT NOT NULL, + actor TEXT NOT NULL, + actor_type TEXT NOT NULL, + dedupe_key TEXT NOT NULL, + raw_payload TEXT NOT NULL DEFAULT '', + state TEXT NOT NULL DEFAULT 'received' + CHECK(state IN ('received','processed','ignored','ambiguous','routed','delivered','failed')), + occurred_at INTEGER NOT NULL, + created_at INTEGER NOT NULL, + updated_at INTEGER NOT NULL, + UNIQUE(space_id, dedupe_key) + ) + `); + + db.exec(` + CREATE TABLE mcp_audit_log ( + id TEXT PRIMARY KEY, + timestamp INTEGER NOT NULL, + tool_name TEXT NOT NULL + ) + `); + + db.exec(` + CREATE TABLE space_goal_events ( + id TEXT PRIMARY KEY, + space_id TEXT NOT NULL, + goal_id TEXT NOT NULL, + event_type TEXT NOT NULL, + source TEXT NOT NULL, + created_at INTEGER NOT NULL + ) + `); +} + +function insertExternalEvent(db: Database, id: string, state: string, ageDays: number): void { + const now = Date.now(); + db.exec( + `INSERT INTO space_external_events (id, space_id, source, topic, dedupe_key, occurred_at, ingested_at, state, created_at, updated_at) + VALUES (?, 'sp1', 'github', 'topic', ?, ?, ?, ?, ?, ?)`, + [id, id, now, now, state, now, now - ageDays * DAY] + ); +} + +function insertDelivery( + db: Database, + eventId: string, + key: string, + state: string, + ageDays: number +): void { + const now = Date.now(); + db.exec( + `INSERT INTO space_external_event_deliveries (event_id, delivery_key, workflow_run_id, task_id, node_id, agent_name, state, updated_at) + VALUES (?, ?, 'run', 'task', 'node', 'agent', ?, ?)`, + [eventId, key, state, now - ageDays * DAY] + ); +} + +function insertGithubEvent(db: Database, id: string, state: string, ageDays: number): void { + const now = Date.now(); + db.exec( + `INSERT INTO space_github_events (id, space_id, source, delivery_id, event_type, action, repo_owner, repo_name, pr_number, pr_url, actor, actor_type, dedupe_key, state, occurred_at, created_at, updated_at) + VALUES (?, 'sp1', 'polling', ?, 'push', 'opened', 'o', 'r', 1, 'url', 'a', 'user', ?, ?, ?, ?, ?)`, + [id, id, id, state, now, now, now - ageDays * DAY] + ); +} + +function insertAudit(db: Database, id: string, ageDays: number): void { + db.exec(`INSERT INTO mcp_audit_log (id, timestamp, tool_name) VALUES (?, ?, 't')`, [ + id, + Date.now() - ageDays * DAY, + ]); +} + +function insertGoalEvent(db: Database, id: string, ageDays: number): void { + db.exec( + `INSERT INTO space_goal_events (id, space_id, goal_id, event_type, source, created_at) VALUES (?, 'sp1', 'g1', 'created', 'system', ?)`, + [id, Date.now() - ageDays * DAY] + ); +} + +function count(db: Database, table: string): number { + return (db.prepare(`SELECT COUNT(*) as c FROM ${table}`).get() as { c: number }).c; +} + +describe('retention', () => { + let db: Database; + + beforeEach(() => { + db = new Database(':memory:'); + createRetentionSchema(db); + }); + + afterEach(() => { + db.close(); + }); + + describe('runRetention (enabled)', () => { + const config = { + enabled: true, + eventsDays: 14, + mcpAuditDays: 30, + goalEventsDays: 60, + vacuumPages: 0, // disable vacuum in these pure-deletion tests + }; + + it('prunes old terminal external events, keeps recent + in-flight', () => { + insertExternalEvent(db, 'old-delivered', 'delivered', 20); // pruned + insertExternalEvent(db, 'old-routed', 'routed', 20); // in-flight: kept + insertExternalEvent(db, 'new-delivered', 'delivered', 1); // recent: kept + insertExternalEvent(db, 'new-published', 'published', 1); // in-flight: kept + + const stats = runRetention(db, config); + + expect(stats.externalEvents).toBe(1); + expect(count(db, 'space_external_events')).toBe(3); + expect( + db.prepare(`SELECT id FROM space_external_events WHERE id = 'old-delivered'`).get() + ).toBeNull(); + }); + + it('cascades delivery rows when their terminal event is pruned', () => { + insertExternalEvent(db, 'old-delivered', 'delivered', 20); + insertDelivery(db, 'old-delivered', 'd1', 'delivered', 20); // cascade-deleted + insertExternalEvent(db, 'kept-routed', 'routed', 1); + insertDelivery(db, 'kept-routed', 'd2', 'delivered', 1); // kept (recent) + + const stats = runRetention(db, config); + + // d1 is gone via FK cascade (not counted in the independent delivery stat); + // the recent d2 under the kept event remains. + expect(stats.externalEvents).toBe(1); + expect(count(db, 'space_external_event_deliveries')).toBe(1); + }); + + it('independently prunes old terminal deliveries under a kept event', () => { + // Kept (in-flight routed) event, but with an old resolved delivery. + insertExternalEvent(db, 'routed', 'routed', 1); + insertDelivery(db, 'routed', 'old-d', 'delivered', 20); // pruned independently + insertDelivery(db, 'routed', 'new-d', 'delivered', 1); // kept + + const stats = runRetention(db, config); + + expect(stats.deliveries).toBe(1); + expect(count(db, 'space_external_event_deliveries')).toBe(1); + }); + + it('prunes old terminal github events, keeps recent + in-flight', () => { + insertGithubEvent(db, 'old-processed', 'processed', 20); // pruned + insertGithubEvent(db, 'old-received', 'received', 20); // in-flight: kept + insertGithubEvent(db, 'new-processed', 'processed', 1); // recent: kept + + const stats = runRetention(db, config); + + expect(stats.githubEvents).toBe(1); + expect(count(db, 'space_github_events')).toBe(2); + }); + + it('prunes old mcp_audit_log by its own TTL', () => { + insertAudit(db, 'old', 40); // > 30d: pruned + insertAudit(db, 'recent', 5); // kept + + const stats = runRetention(db, config); + + expect(stats.mcpAudit).toBe(1); + expect(count(db, 'mcp_audit_log')).toBe(1); + }); + + it('prunes old space_goal_events by its own TTL', () => { + insertGoalEvent(db, 'old', 90); // > 60d: pruned + insertGoalEvent(db, 'recent', 10); // kept + + const stats = runRetention(db, config); + + expect(stats.goalEvents).toBe(1); + expect(count(db, 'space_goal_events')).toBe(1); + }); + }); + + describe('runRetention (disabled)', () => { + it('deletes nothing when enabled=false', () => { + insertExternalEvent(db, 'old-delivered', 'delivered', 200); + insertGithubEvent(db, 'old-processed', 'processed', 200); + insertAudit(db, 'old', 200); + insertGoalEvent(db, 'old', 200); + + const stats = runRetention(db, { + enabled: false, + eventsDays: 14, + mcpAuditDays: 30, + goalEventsDays: 60, + vacuumPages: 0, + }); + + expect(stats.externalEvents).toBe(0); + expect(stats.githubEvents).toBe(0); + expect(stats.mcpAudit).toBe(0); + expect(stats.goalEvents).toBe(0); + expect(count(db, 'space_external_events')).toBe(1); + }); + }); + + describe('missing tables', () => { + it('skips absent tables without error', () => { + const minimal = new Database(':memory:'); + minimal.exec('PRAGMA foreign_keys = ON'); + // Only one of the target tables exists. + minimal.exec(`CREATE TABLE mcp_audit_log (id TEXT PRIMARY KEY, timestamp INTEGER NOT NULL)`); + minimal.exec(`INSERT INTO mcp_audit_log (id, timestamp) VALUES ('x', 0)`); + + const stats = runRetention(minimal, { + enabled: true, + eventsDays: 14, + mcpAuditDays: 30, + goalEventsDays: 60, + vacuumPages: 0, + }); + + expect(stats.mcpAudit).toBe(1); + expect(stats.externalEvents).toBe(0); // table absent → 0, no throw + minimal.close(); + }); + }); + + describe('incrementalVacuum', () => { + it('is a no-op (returns 0) on a non-incremental DB', () => { + // Fresh in-memory DB defaults to auto_vacuum = NONE (0). + const none = new Database(':memory:'); + none.exec('CREATE TABLE t (id INTEGER PRIMARY KEY, blob TEXT)'); + none.exec(`INSERT INTO t (blob) VALUES ('x')`); + expect(incrementalVacuum(none, 500)).toBe(0); + none.close(); + }); + + it('reclaims freed pages on an incremental DB', () => { + const inc = new Database(':memory:'); + // Must be set before any table is created to take effect. + inc.exec('PRAGMA auto_vacuum = INCREMENTAL'); + inc.exec('CREATE TABLE t (id INTEGER PRIMARY KEY, blob TEXT)'); + for (let i = 0; i < 3000; i++) { + inc.prepare('INSERT INTO t (blob) VALUES (?)').run('x'.repeat(3000)); + } + inc.exec('DELETE FROM t WHERE id > 100'); + + const beforePages = (inc.prepare('PRAGMA page_count').get() as { page_count: number }) + .page_count; + const freed = incrementalVacuum(inc, 500); + const afterPages = (inc.prepare('PRAGMA page_count').get() as { page_count: number }) + .page_count; + + expect(freed).toBeGreaterThan(0); + expect(freed).toBeLessThanOrEqual(500); + expect(afterPages).toBeLessThan(beforePages); + inc.close(); + }); + + it('returns 0 when vacuumPages <= 0', () => { + const inc = new Database(':memory:'); + inc.exec('PRAGMA auto_vacuum = INCREMENTAL'); + inc.exec('CREATE TABLE t (id INTEGER PRIMARY KEY)'); + expect(incrementalVacuum(inc, 0)).toBe(0); + inc.close(); + }); + }); + + describe('loadRetentionConfig', () => { + const keys = [ + 'HYPERNEO_RETENTION_ENABLED', + 'HYPERNEO_RETENTION_EVENTS_DAYS', + 'HYPERNEO_RETENTION_MCP_AUDIT_DAYS', + 'HYPERNEO_RETENTION_GOAL_EVENTS_DAYS', + 'HYPERNEO_RETENTION_VACUUM_PAGES', + ]; + const original = keys.map((k) => process.env[k]); + + afterEach(() => { + keys.forEach((k, i) => { + if (original[i] === undefined) delete process.env[k]; + else process.env[k] = original[i]; + }); + }); + + it('defaults to disabled with conservative TTLs', () => { + for (const k of keys) delete process.env[k]; + const cfg = loadRetentionConfig(); + expect(cfg.enabled).toBe(false); + expect(cfg.eventsDays).toBe(14); + expect(cfg.mcpAuditDays).toBe(30); + expect(cfg.goalEventsDays).toBe(60); + expect(cfg.vacuumPages).toBe(500); + }); + + it('parses enabled flags and overrides', () => { + process.env.HYPERNEO_RETENTION_ENABLED = '1'; + process.env.HYPERNEO_RETENTION_EVENTS_DAYS = '7'; + process.env.HYPERNEO_RETENTION_MCP_AUDIT_DAYS = '10'; + process.env.HYPERNEO_RETENTION_GOAL_EVENTS_DAYS = '90'; + process.env.HYPERNEO_RETENTION_VACUUM_PAGES = '250'; + const cfg = loadRetentionConfig(); + expect(cfg).toEqual({ + enabled: true, + eventsDays: 7, + mcpAuditDays: 10, + goalEventsDays: 90, + vacuumPages: 250, + }); + }); + + it('falls back to defaults on invalid input', () => { + process.env.HYPERNEO_RETENTION_ENABLED = 'yes'; // not 1/true → false + process.env.HYPERNEO_RETENTION_EVENTS_DAYS = 'not-a-number'; + process.env.HYPERNEO_RETENTION_VACUUM_PAGES = '-5'; + const cfg = loadRetentionConfig(); + expect(cfg.enabled).toBe(false); + expect(cfg.eventsDays).toBe(14); // default + expect(cfg.vacuumPages).toBe(500); // default + }); + }); +}); diff --git a/packages/daemon/tests/unit/4-space-storage/storage/database-core.test.ts b/packages/daemon/tests/unit/4-space-storage/storage/database-core.test.ts index 686d1a167e..62e8675422 100644 --- a/packages/daemon/tests/unit/4-space-storage/storage/database-core.test.ts +++ b/packages/daemon/tests/unit/4-space-storage/storage/database-core.test.ts @@ -104,6 +104,17 @@ describe('DatabaseCore', () => { expect(result.foreign_keys).toBe(1); }); + it('should set auto_vacuum = INCREMENTAL on a fresh database', async () => { + // dbPath did not exist before initialize(), so the DB is created fresh and + // auto_vacuum is set before any table is created. + dbCore = new DatabaseCore(dbPath); + await dbCore.initialize(); + + const db = dbCore.getDb(); + const result = db.prepare('PRAGMA auto_vacuum').get() as { auto_vacuum: number }; + expect(result.auto_vacuum).toBe(2); // 2 == INCREMENTAL + }); + it('should create database tables', async () => { dbCore = new DatabaseCore(dbPath); await dbCore.initialize(); diff --git a/packages/daemon/tests/unit/4-space-storage/storage/migrations/migration-169_test.ts b/packages/daemon/tests/unit/4-space-storage/storage/migrations/migration-169_test.ts new file mode 100644 index 0000000000..32c223cf64 --- /dev/null +++ b/packages/daemon/tests/unit/4-space-storage/storage/migrations/migration-169_test.ts @@ -0,0 +1,101 @@ +/** + * Migration 169 Tests — Convert auto_vacuum NONE → INCREMENTAL. + * + * Covers: + * - Converts a populated auto_vacuum = NONE database to INCREMENTAL (2) + * - After conversion, incremental_vacuum reclaims freed pages (the whole point) + * - No-op when already INCREMENTAL (fresh DBs created by DatabaseCore) + * - Leaves FULL (1) mode untouched + * - Idempotent: running twice is a no-op after the first pass + */ + +import { Database as BunDatabase } from 'bun:sqlite'; +import { describe, expect, test } from 'bun:test'; +import { runMigration169 } from '../../../../../src/storage/schema/migrations'; + +function autoVacuum(db: BunDatabase): number { + return (db.prepare('PRAGMA auto_vacuum').get() as { auto_vacuum: number }).auto_vacuum; +} + +function pageCount(db: BunDatabase): number { + return (db.prepare('PRAGMA page_count').get() as { page_count: number }).page_count; +} + +/** Populate a single table with enough rows to span many pages. */ +function populate(db: BunDatabase): void { + db.exec('CREATE TABLE t (id INTEGER PRIMARY KEY, blob TEXT)'); + const insert = db.prepare('INSERT INTO t (blob) VALUES (?)'); + for (let i = 0; i < 3000; i++) insert.run('x'.repeat(3000)); +} + +describe('Migration 169: convert auto_vacuum NONE → INCREMENTAL', () => { + test('converts a populated NONE database to INCREMENTAL', () => { + const db = new BunDatabase(':memory:'); + // Fresh in-memory DB defaults to auto_vacuum = NONE (0). + populate(db); + expect(autoVacuum(db)).toBe(0); + + runMigration169(db); + + expect(autoVacuum(db)).toBe(2); // INCREMENTAL + db.close(); + }); + + test('after conversion, incremental_vacuum reclaims freed pages', () => { + const db = new BunDatabase(':memory:'); + populate(db); + expect(autoVacuum(db)).toBe(0); + + runMigration169(db); + expect(autoVacuum(db)).toBe(2); + + // Free a large contiguous range, then reclaim — only possible because the + // rebuild put the DB in incremental mode with a pointer-map. + db.exec('DELETE FROM t WHERE id > 100'); + const freeBefore = (db.prepare('PRAGMA freelist_count').get() as { freelist_count: number }) + .freelist_count; + const pagesBefore = pageCount(db); + expect(freeBefore).toBeGreaterThan(0); + + db.exec('PRAGMA incremental_vacuum(500)'); + expect(pageCount(db)).toBeLessThan(pagesBefore); + db.close(); + }); + + test('is a no-op when already INCREMENTAL', () => { + const db = new BunDatabase(':memory:'); + db.exec('PRAGMA auto_vacuum = INCREMENTAL'); + populate(db); + expect(autoVacuum(db)).toBe(2); + + runMigration169(db); // should not throw or alter mode + + expect(autoVacuum(db)).toBe(2); + db.close(); + }); + + test('leaves FULL (1) mode untouched', () => { + const db = new BunDatabase(':memory:'); + db.exec('PRAGMA auto_vacuum = FULL'); + populate(db); + expect(autoVacuum(db)).toBe(1); + + runMigration169(db); + + expect(autoVacuum(db)).toBe(1); // unchanged + db.close(); + }); + + test('is idempotent — running twice yields the same mode', () => { + const db = new BunDatabase(':memory:'); + populate(db); + expect(autoVacuum(db)).toBe(0); + + runMigration169(db); + expect(autoVacuum(db)).toBe(2); + + runMigration169(db); // second run is a no-op (already INCREMENTAL) + expect(autoVacuum(db)).toBe(2); + db.close(); + }); +}); From 3b98c487a185a7b3e07643cb8f763d1622fb551b Mon Sep 17 00:00:00 2001 From: Marc Liu Date: Mon, 3 Aug 2026 08:29:50 -0400 Subject: [PATCH 2/4] =?UTF-8?q?chore(daemon):=20renumber=20vacuum=20migrat?= =?UTF-8?q?ion=20169=20=E2=86=92=20170=20(dev=20shipped=20M169=20in=20#234?= =?UTF-8?q?6)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../daemon/src/lib/job-handlers/retention.ts | 2 +- packages/daemon/src/storage/database-core.ts | 2 +- .../daemon/src/storage/schema/migrations.ts | 11 +++++++---- ...ation-169_test.ts => migration-170_test.ts} | 18 +++++++++--------- 4 files changed, 18 insertions(+), 15 deletions(-) rename packages/daemon/tests/unit/4-space-storage/storage/migrations/{migration-169_test.ts => migration-170_test.ts} (87%) diff --git a/packages/daemon/src/lib/job-handlers/retention.ts b/packages/daemon/src/lib/job-handlers/retention.ts index ed646e1b26..2666824e1c 100644 --- a/packages/daemon/src/lib/job-handlers/retention.ts +++ b/packages/daemon/src/lib/job-handlers/retention.ts @@ -17,7 +17,7 @@ * - `incremental_vacuum(500)` runs every cycle regardless of `enabled`: it is * space maintenance, not deletion, and reclaims pages freed by any delete * (including the pre-existing 7-day job_queue and worktree reapers). It is a - * no-op unless the DB is in incremental-vacuum mode (see migration 169 / + * no-op unless the DB is in incremental-vacuum mode (see migration 170 / * DatabaseCore's fresh-DB pragma). */ diff --git a/packages/daemon/src/storage/database-core.ts b/packages/daemon/src/storage/database-core.ts index 0275ba9297..cdb018a7f6 100644 --- a/packages/daemon/src/storage/database-core.ts +++ b/packages/daemon/src/storage/database-core.ts @@ -54,7 +54,7 @@ export class DatabaseCore { // header flag that only takes effect when set before the first table is // created; on an existing DB the flip is a silent no-op until a full VACUUM. // Existing (auto_vacuum = NONE) databases are converted by the opt-in - // migration 169 (HYPERNEO_DB_VACUUM_MIGRATION) — VACUUM on a multi-GB DB is a + // migration 170 (HYPERNEO_DB_VACUUM_MIGRATION) — VACUUM on a multi-GB DB is a // long, disk-intensive operation to schedule deliberately. Once INCREMENTAL, // the daily cleanup job runs incremental_vacuum(500) to reclaim pages freed // by retention sweeps and normal deletes, so the file shrinks over time diff --git a/packages/daemon/src/storage/schema/migrations.ts b/packages/daemon/src/storage/schema/migrations.ts index 36321e32de..7e067fefca 100644 --- a/packages/daemon/src/storage/schema/migrations.ts +++ b/packages/daemon/src/storage/schema/migrations.ts @@ -794,7 +794,7 @@ export function runMigrations(db: BunDatabase, createBackup: () => void): void { // idx_node_executions_run and is unaffected.) run(migrationMarkerKey(168), () => runMigration168(db)); - // Migration 169: Convert existing auto_vacuum = NONE databases to INCREMENTAL + // Migration 170: Convert existing auto_vacuum = NONE databases to INCREMENTAL // so the daily cleanup job's incremental_vacuum(500) can reclaim pages freed // by retention sweeps and normal deletes. Fresh databases already get // INCREMENTAL at creation (DatabaseCore), so this no-ops for them. @@ -804,8 +804,11 @@ export function runMigrations(db: BunDatabase, createBackup: () => void): void { // a long, disk-intensive operation that must be scheduled deliberately (e.g. // during a maintenance window). When the flag is unset the migration is not // registered and not marked, so setting the flag + restarting runs it once. + // + // (Originally authored as M169 on this branch; renumbered to 170 because dev + // shipped an unrelated M169 in #2346.) if (envFlag(process.env.HYPERNEO_DB_VACUUM_MIGRATION)) { - run(migrationMarkerKey(169), () => runMigration169(db)); + run(migrationMarkerKey(170), () => runMigration170(db)); } } @@ -11502,7 +11505,7 @@ function readAutoVacuumMode(db: BunDatabase): number { } /** - * Migration 169: Convert auto_vacuum = NONE to INCREMENTAL via a full VACUUM. + * Migration 170: Convert auto_vacuum = NONE to INCREMENTAL via a full VACUUM. * * Fresh databases are created with auto_vacuum = INCREMENTAL by DatabaseCore, so * this is a no-op for them (mode is already 2). FULL (1) is left untouched — it @@ -11514,7 +11517,7 @@ function readAutoVacuumMode(db: BunDatabase): number { * * See registration site: gated behind HYPERNEO_DB_VACUUM_MIGRATION. */ -export function runMigration169(db: BunDatabase): void { +export function runMigration170(db: BunDatabase): void { const mode = readAutoVacuumMode(db); if (mode === 2) return; // already INCREMENTAL if (mode === 1) return; // FULL — leave untouched diff --git a/packages/daemon/tests/unit/4-space-storage/storage/migrations/migration-169_test.ts b/packages/daemon/tests/unit/4-space-storage/storage/migrations/migration-170_test.ts similarity index 87% rename from packages/daemon/tests/unit/4-space-storage/storage/migrations/migration-169_test.ts rename to packages/daemon/tests/unit/4-space-storage/storage/migrations/migration-170_test.ts index 32c223cf64..a45b3751ba 100644 --- a/packages/daemon/tests/unit/4-space-storage/storage/migrations/migration-169_test.ts +++ b/packages/daemon/tests/unit/4-space-storage/storage/migrations/migration-170_test.ts @@ -1,5 +1,5 @@ /** - * Migration 169 Tests — Convert auto_vacuum NONE → INCREMENTAL. + * Migration 170 Tests — Convert auto_vacuum NONE → INCREMENTAL. * * Covers: * - Converts a populated auto_vacuum = NONE database to INCREMENTAL (2) @@ -11,7 +11,7 @@ import { Database as BunDatabase } from 'bun:sqlite'; import { describe, expect, test } from 'bun:test'; -import { runMigration169 } from '../../../../../src/storage/schema/migrations'; +import { runMigration170 } from '../../../../../src/storage/schema/migrations'; function autoVacuum(db: BunDatabase): number { return (db.prepare('PRAGMA auto_vacuum').get() as { auto_vacuum: number }).auto_vacuum; @@ -28,14 +28,14 @@ function populate(db: BunDatabase): void { for (let i = 0; i < 3000; i++) insert.run('x'.repeat(3000)); } -describe('Migration 169: convert auto_vacuum NONE → INCREMENTAL', () => { +describe('Migration 170: convert auto_vacuum NONE → INCREMENTAL', () => { test('converts a populated NONE database to INCREMENTAL', () => { const db = new BunDatabase(':memory:'); // Fresh in-memory DB defaults to auto_vacuum = NONE (0). populate(db); expect(autoVacuum(db)).toBe(0); - runMigration169(db); + runMigration170(db); expect(autoVacuum(db)).toBe(2); // INCREMENTAL db.close(); @@ -46,7 +46,7 @@ describe('Migration 169: convert auto_vacuum NONE → INCREMENTAL', () => { populate(db); expect(autoVacuum(db)).toBe(0); - runMigration169(db); + runMigration170(db); expect(autoVacuum(db)).toBe(2); // Free a large contiguous range, then reclaim — only possible because the @@ -68,7 +68,7 @@ describe('Migration 169: convert auto_vacuum NONE → INCREMENTAL', () => { populate(db); expect(autoVacuum(db)).toBe(2); - runMigration169(db); // should not throw or alter mode + runMigration170(db); // should not throw or alter mode expect(autoVacuum(db)).toBe(2); db.close(); @@ -80,7 +80,7 @@ describe('Migration 169: convert auto_vacuum NONE → INCREMENTAL', () => { populate(db); expect(autoVacuum(db)).toBe(1); - runMigration169(db); + runMigration170(db); expect(autoVacuum(db)).toBe(1); // unchanged db.close(); @@ -91,10 +91,10 @@ describe('Migration 169: convert auto_vacuum NONE → INCREMENTAL', () => { populate(db); expect(autoVacuum(db)).toBe(0); - runMigration169(db); + runMigration170(db); expect(autoVacuum(db)).toBe(2); - runMigration169(db); // second run is a no-op (already INCREMENTAL) + runMigration170(db); // second run is a no-op (already INCREMENTAL) expect(autoVacuum(db)).toBe(2); db.close(); }); From 5fd659479482a57854064cee8a54d6d735ebbe4c Mon Sep 17 00:00:00 2001 From: Marc Liu Date: Mon, 3 Aug 2026 08:51:01 -0400 Subject: [PATCH 3/4] =?UTF-8?q?fix(daemon):=20address=20review=20=E2=80=94?= =?UTF-8?q?=20retention=20indexes=20(M171),=20trim=20dead=20enum,=20VACUUM?= =?UTF-8?q?=20boot-safety=20[#2339]?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - P2: add covering indexes (mcp_audit_log(timestamp), space_github_events(state, updated_at), space_goal_events(created_at)) via migration 171 so the retention sweep is an indexed range scan, not a full scan under a write lock; correct the prune() doc comment. - P3: trim EXTERNAL_EVENT_TERMINAL_STATES to delivered/failed/ignored — M124 collapsed the enum and migrated delivery_failed/ambiguous away. - Non-blocking: wrap the M170 VACUUM registration in try/catch so a failed VACUUM (e.g. <2x free disk) can't take the daemon down; left unmarked so it retries next boot. --- .../daemon/src/lib/job-handlers/retention.ts | 24 ++++---- .../daemon/src/storage/schema/migrations.ts | 52 +++++++++++++++- .../storage/migrations/migration-171_test.ts | 60 +++++++++++++++++++ 3 files changed, 123 insertions(+), 13 deletions(-) create mode 100644 packages/daemon/tests/unit/4-space-storage/storage/migrations/migration-171_test.ts diff --git a/packages/daemon/src/lib/job-handlers/retention.ts b/packages/daemon/src/lib/job-handlers/retention.ts index 2666824e1c..1221860c3f 100644 --- a/packages/daemon/src/lib/job-handlers/retention.ts +++ b/packages/daemon/src/lib/job-handlers/retention.ts @@ -89,15 +89,12 @@ export function loadRetentionConfig(): RetentionConfig { }; } -// Terminal (resolved) states for the external-event pipeline. `published` / -// `routed` are in-flight (may still be delivered) and are never pruned. -const EXTERNAL_EVENT_TERMINAL_STATES = [ - 'delivered', - 'delivery_failed', - 'failed', - 'ignored', - 'ambiguous', -] as const; +// Terminal (resolved) states for the external-event pipeline. `published` is +// in-flight (may still be delivered) and is never pruned. Migration 124 +// collapsed this enum to ('published','delivered','failed','ignored') and +// migrated the legacy `routed`/`delivery_failed`/`ambiguous` values away, so +// only these three terminal states can exist. +const EXTERNAL_EVENT_TERMINAL_STATES = ['delivered', 'failed', 'ignored'] as const; // Deliveries: `pending` is in-flight; only resolved deliveries are pruned. const DELIVERY_TERMINAL_STATES = ['delivered', 'failed'] as const; @@ -124,9 +121,12 @@ function placeholders(count: number): string { * * We count first because `DELETE ... .changes` includes rows removed by FK * CASCADE (e.g. a pruned external event pulls its deliveries), which would - * inflate the per-table stat. A SELECT COUNT is accurate and cheap (the age - * columns are indexed). The `whereClause` may carry `?` placeholders bound by - * `params`, used identically for the count and the delete. + * inflate the per-table stat. Each target table has a covering index for its + * retention predicate — `(state, updated_at)` on external events + deliveries, + * and the migration-171 indexes on github events / mcp_audit_log / goal events + * — so both the COUNT and DELETE are indexed range scans, not full table scans. + * The `whereClause` may carry `?` placeholders bound by `params`, used + * identically for the count and the delete. */ function prune( db: BunDatabase, diff --git a/packages/daemon/src/storage/schema/migrations.ts b/packages/daemon/src/storage/schema/migrations.ts index 8c2d9a1d5a..1b391bee1a 100644 --- a/packages/daemon/src/storage/schema/migrations.ts +++ b/packages/daemon/src/storage/schema/migrations.ts @@ -10,6 +10,7 @@ */ import type { Database as BunDatabase } from 'bun:sqlite'; +import { Logger } from '../../lib/logger'; import { runMigration94 as runMigration94External } from './m94-backfill-workflow-templates'; import { runMigration106 as runMigration106External } from './m106-backfill-agent-templates'; import { RESERVED_SPACE_AGENT_HANDLES, slugify, validateSlug } from '../../lib/space/slug'; @@ -24,6 +25,8 @@ import { createEvolutionTables } from './evolution'; import { createLongHorizonAgentTables } from './long-horizon-agents'; import { migrateLegacyLongHorizonAgentData } from '../../lib/space/agents/legacy-long-horizon-migration'; +const migrationLogger = new Logger('Migrations'); + /** * Run all database migrations * @@ -819,8 +822,27 @@ export function runMigrations(db: BunDatabase, createBackup: () => void): void { // (Originally authored as M169 on this branch; renumbered to 170 because dev // shipped an unrelated M169 in #2346.) if (envFlag(process.env.HYPERNEO_DB_VACUUM_MIGRATION)) { - run(migrationMarkerKey(170), () => runMigration170(db)); + // VACUUM can fail at runtime (e.g. <2× free disk for the rebuild). Catch so + // a mis-set flag can't take the daemon down: on failure the migration is NOT + // marked (runMarkedMigration skips markMigration when migration() throws), so + // the next boot retries it once the condition clears. The flag is opt-in, so + // the owner removing it always recovers. + try { + run(migrationMarkerKey(170), () => runMigration170(db)); + } catch (err) { + migrationLogger.error( + '[migration 170] auto_vacuum VACUUM failed — skipped (not marked, will retry next boot):', + err + ); + } } + + // Migration 171: Covering indexes for the retention sweep predicates so the + // daily prune (COUNT + DELETE by state/age) is an indexed range scan, not a + // full table scan under a write lock — mcp_audit_log in particular is one of + // the largest tables. external events + deliveries already carry + // (state, updated_at); this fills the gap for the other three. + run(migrationMarkerKey(171), () => runMigration171(db)); } function migrationMarkerKey(version: number): string { @@ -11582,6 +11604,34 @@ export function runMigration170(db: BunDatabase): void { db.exec('VACUUM'); } +/** + * Migration 171: Covering indexes for the retention sweep predicates. + * + * The daily retention prune filters each table by state + age. Without a leading + * index on the age/state column the COUNT-then-DELETE is a full table scan under + * a write lock — worst on `mcp_audit_log` (one row per MCP tool call, one of the + * largest tables on the 15GB DB). These indexes make both passes indexed range + * scans. `space_external_events` / deliveries already carry `(state, updated_at)` + * (the latter via M165), so they are not touched here. + * + * Idempotent (`IF NOT EXISTS`); runs on fresh and existing databases. + */ +export function runMigration171(db: BunDatabase): void { + if (tableExists(db, 'mcp_audit_log')) { + db.exec(`CREATE INDEX IF NOT EXISTS idx_mcp_audit_log_timestamp ON mcp_audit_log(timestamp)`); + } + if (tableExists(db, 'space_github_events')) { + db.exec( + `CREATE INDEX IF NOT EXISTS idx_space_github_events_state_updated ON space_github_events(state, updated_at)` + ); + } + if (tableExists(db, 'space_goal_events')) { + db.exec( + `CREATE INDEX IF NOT EXISTS idx_space_goal_events_created ON space_goal_events(created_at)` + ); + } +} + /** Truthy check for an opt-in `HYPERNEO_*` env flag. */ function envFlag(value: string | undefined): boolean { return value === '1' || value === 'true'; diff --git a/packages/daemon/tests/unit/4-space-storage/storage/migrations/migration-171_test.ts b/packages/daemon/tests/unit/4-space-storage/storage/migrations/migration-171_test.ts new file mode 100644 index 0000000000..3bade3f29c --- /dev/null +++ b/packages/daemon/tests/unit/4-space-storage/storage/migrations/migration-171_test.ts @@ -0,0 +1,60 @@ +/** + * Migration 171 Tests — Retention covering indexes. + * + * Covers: + * - Creates the three covering indexes the retention sweep needs + * - Idempotent (running twice / with pre-existing indexes is a no-op) + * - Skips absent tables without error + */ + +import { describe, test, expect } from 'bun:test'; +import { Database as BunDatabase } from 'bun:sqlite'; +import { runMigration171 } from '../../../../../src/storage/schema/migrations'; + +function indexExists(db: BunDatabase, name: string): boolean { + return !!db.prepare(`SELECT name FROM sqlite_master WHERE type = 'index' AND name = ?`).get(name); +} + +function createTables(db: BunDatabase): void { + db.exec(`CREATE TABLE mcp_audit_log (id TEXT PRIMARY KEY, timestamp INTEGER NOT NULL)`); + db.exec( + `CREATE TABLE space_github_events (id TEXT PRIMARY KEY, state TEXT NOT NULL, updated_at INTEGER NOT NULL)` + ); + db.exec(`CREATE TABLE space_goal_events (id TEXT PRIMARY KEY, created_at INTEGER NOT NULL)`); +} + +describe('Migration 171: retention covering indexes', () => { + test('creates the three covering indexes', () => { + const db = new BunDatabase(':memory:'); + createTables(db); + + runMigration171(db); + + expect(indexExists(db, 'idx_mcp_audit_log_timestamp')).toBe(true); + expect(indexExists(db, 'idx_space_github_events_state_updated')).toBe(true); + expect(indexExists(db, 'idx_space_goal_events_created')).toBe(true); + db.close(); + }); + + test('is idempotent — running twice does not error', () => { + const db = new BunDatabase(':memory:'); + createTables(db); + + runMigration171(db); + expect(() => runMigration171(db)).not.toThrow(); + + expect(indexExists(db, 'idx_mcp_audit_log_timestamp')).toBe(true); + db.close(); + }); + + test('skips absent tables without error', () => { + const db = new BunDatabase(':memory:'); + // Only one of the three tables exists. + db.exec(`CREATE TABLE mcp_audit_log (id TEXT PRIMARY KEY, timestamp INTEGER NOT NULL)`); + + expect(() => runMigration171(db)).not.toThrow(); + expect(indexExists(db, 'idx_mcp_audit_log_timestamp')).toBe(true); + expect(indexExists(db, 'idx_space_github_events_state_updated')).toBe(false); + db.close(); + }); +}); From 69a42f496ba33ffc49d3bf404a9e3dc8eca5dffa Mon Sep 17 00:00:00 2001 From: Marc Liu Date: Mon, 3 Aug 2026 08:59:53 -0400 Subject: [PATCH 4/4] fix(daemon): add M171 indexes to space-test-db helper for schema parity [#2339] CI's DB schema parity check (check-db-schema-parity) compares the fully-migrated production schema against tests/unit/helpers/space-test-db.ts. Migration 171 added covering indexes to production; mirror mcp_audit_log(timestamp) and space_goal_events(created_at) here so parity passes. (space_github_events isn't in the helper, so its index needs no mirror.) --- packages/daemon/tests/unit/helpers/space-test-db.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/daemon/tests/unit/helpers/space-test-db.ts b/packages/daemon/tests/unit/helpers/space-test-db.ts index 5564643b0d..0ef6a2e25b 100644 --- a/packages/daemon/tests/unit/helpers/space-test-db.ts +++ b/packages/daemon/tests/unit/helpers/space-test-db.ts @@ -807,6 +807,7 @@ export function createSpaceTables(db: BunDatabase): void { db.exec( `CREATE INDEX IF NOT EXISTS idx_mcp_audit_log_session ON mcp_audit_log (session_id, timestamp)` ); + db.exec(`CREATE INDEX IF NOT EXISTS idx_mcp_audit_log_timestamp ON mcp_audit_log (timestamp)`); // Task schedules (migration 124) db.exec(` @@ -879,4 +880,7 @@ export function createSpaceTables(db: BunDatabase): void { db.exec( `CREATE INDEX IF NOT EXISTS idx_space_goal_events_source_task ON space_goal_events(source_task_id, created_at DESC)` ); + db.exec( + `CREATE INDEX IF NOT EXISTS idx_space_goal_events_created ON space_goal_events(created_at)` + ); }