diff --git a/apps/ade-cli/src/services/sync/syncHostService.ts b/apps/ade-cli/src/services/sync/syncHostService.ts index 0dcd9ba2a..42fb1782e 100644 --- a/apps/ade-cli/src/services/sync/syncHostService.ts +++ b/apps/ade-cli/src/services/sync/syncHostService.ts @@ -313,6 +313,30 @@ const MOBILE_CHANGESET_EXCLUDED_TABLES = new Set([ // visible-lane refresh upserts here, so scrolling was pushing changesets to // every phone. "pull_request_snapshots", + // Event logs the phone stores and never reads. Each exists in + // `DatabaseBootstrap.sql` so an inbound changeset still applies, but there is + // no Swift read path for any of them — verified: zero references outside that + // schema file. Excluding them stops the phone paying apply and storage cost + // for rows it cannot surface. + // + // This list is an OUTBOUND filter only — it drops rows from mobile + // changesets and never touches CRR metadata, so unlike a local-only + // conversion it carries no apply hazard for a peer on an older build. + // Existing rows on a paired device are left alone, and the peer's ack + // watermark still advances through the filtered versions. + // + // `linear_ingress_events` and `worker_agent_runs` are included here but + // deliberately NOT age-pruned on the host: the first is the webhook replay + // guard, the second is a lifecycle table (see RETAINED_EVENT_LOG_TABLES in + // kvDb.ts). Not shipping them to a phone that cannot read them is orthogonal + // to how long the host keeps them. + "linear_ingress_events", + "linear_sync_events", + "linear_workflow_run_events", + "worker_agent_runs", + "worker_agent_cost_events", + "pack_events", + "cto_session_logs", ]); // Tables the host alone is authoritative for. `sync_cluster_state` is the diff --git a/apps/desktop/src/main/services/ai/aiIntegrationService.ts b/apps/desktop/src/main/services/ai/aiIntegrationService.ts index 0429514ff..69da9c02f 100644 --- a/apps/desktop/src/main/services/ai/aiIntegrationService.ts +++ b/apps/desktop/src/main/services/ai/aiIntegrationService.ts @@ -1343,6 +1343,16 @@ export function createAiIntegrationService(args: { return daily; }; + /** + * Successful requests for `feature` so far today. + * + * This reads `ai_usage_log` directly, and that table replicates, so the count + * spans every machine on the account — `dailyLimit` is an account-wide cap, + * not a per-machine one. Anything that stops the table replicating (making it + * local-only, filtering it out of a peer's changesets) turns the cap into a + * per-machine one and multiplies the user's ceiling by their machine count. + * See the note beside `ai_usage_log` in kvDb's LOCAL_ONLY_CRR_EXCLUDED_TABLES. + */ const countDailyUsage = (feature: AiFeatureKey): number => { const row = db.get<{ count: number }>( ` diff --git a/apps/desktop/src/main/services/state/dbMaintenanceApi.test.ts b/apps/desktop/src/main/services/state/dbMaintenanceApi.test.ts new file mode 100644 index 000000000..ab673a178 --- /dev/null +++ b/apps/desktop/src/main/services/state/dbMaintenanceApi.test.ts @@ -0,0 +1,90 @@ +import { describe, expect, it, vi } from "vitest"; + +import { + pruneRowsInBatches, + MAINTENANCE_DELETE_BATCH_ROWS, + MAINTENANCE_DELETE_MAX_BATCHES, +} from "./dbMaintenanceApi"; + +describe("pruneRowsInBatches", () => { + /** A table of `total` matching rows that deletes at most `limit` per call. */ + function fakeTable(total: number) { + let remaining = total; + return { + get remaining() { return remaining; }, + deleteRows: vi.fn((sql: string) => { + const limit = Number(/limit (\d+)/.exec(sql)?.[1] ?? 0); + const deleted = Math.min(limit, remaining); + remaining -= deleted; + return deleted; + }), + }; + } + + it("yields to the event loop between batches", async () => { + const table = fakeTable(MAINTENANCE_DELETE_BATCH_ROWS * 3); + const yields = vi.fn(async () => {}); + + const removed = await pruneRowsInBatches({ + table: "ai_usage_log", + where: "timestamp < ?", + params: ["2026-01-01"], + deleteRows: table.deleteRows, + yieldToEventLoop: yields, + }); + + expect(removed).toBe(MAINTENANCE_DELETE_BATCH_ROWS * 3); + expect(table.deleteRows).toHaveBeenCalledTimes(4); + // The driver is synchronous, so without these yields the whole delete runs + // as one uninterruptible block — UI, IPC, and the sync pump all stall. + expect(yields).toHaveBeenCalledTimes(3); + }); + + it("stops on the first short batch without an extra probe", async () => { + const table = fakeTable(10); + const yields = vi.fn(async () => {}); + + const removed = await pruneRowsInBatches({ + table: "cto_session_logs", + where: "created_at < ?", + params: ["2026-01-01"], + deleteRows: table.deleteRows, + yieldToEventLoop: yields, + }); + + expect(removed).toBe(10); + expect(table.deleteRows).toHaveBeenCalledTimes(1); + expect(yields).not.toHaveBeenCalled(); + }); + + it("bounds one call so a huge backlog cannot run away", async () => { + const table = fakeTable(MAINTENANCE_DELETE_BATCH_ROWS * (MAINTENANCE_DELETE_MAX_BATCHES + 50)); + + const removed = await pruneRowsInBatches({ + table: "ai_usage_log", + where: "timestamp < ?", + params: ["2026-01-01"], + deleteRows: table.deleteRows, + yieldToEventLoop: async () => {}, + }); + + expect(table.deleteRows).toHaveBeenCalledTimes(MAINTENANCE_DELETE_MAX_BATCHES); + expect(removed).toBe(MAINTENANCE_DELETE_BATCH_ROWS * MAINTENANCE_DELETE_MAX_BATCHES); + // The rest is left for the next sweep rather than held in one long pause. + expect(table.remaining).toBe(MAINTENANCE_DELETE_BATCH_ROWS * 50); + }); + + it("deletes by rowid so each batch costs the batch, not the table", async () => { + const table = fakeTable(1); + await pruneRowsInBatches({ + table: "pack_events", + where: "created_at < ?", + params: ["2026-01-01"], + deleteRows: table.deleteRows, + yieldToEventLoop: async () => {}, + }); + const sql = table.deleteRows.mock.calls[0]?.[0] ?? ""; + expect(sql).toContain("where rowid in ("); + expect(sql).toContain(`limit ${MAINTENANCE_DELETE_BATCH_ROWS}`); + }); +}); diff --git a/apps/desktop/src/main/services/state/dbMaintenanceApi.ts b/apps/desktop/src/main/services/state/dbMaintenanceApi.ts index bd8ca90b0..9c7da69f1 100644 --- a/apps/desktop/src/main/services/state/dbMaintenanceApi.ts +++ b/apps/desktop/src/main/services/state/dbMaintenanceApi.ts @@ -86,6 +86,64 @@ export function pruneIngressEventRowsForProject( return removed; } +/** Linear/worker/pack/CTO event-log rows older than this are deleted. */ +export const EVENT_LOG_RETENTION_DAYS = 30; + +/** + * Rows deleted per batch by {@link pruneRowsInBatches}. + * + * ADE's SQLite driver (`node:sqlite` `DatabaseSync`) is fully synchronous, so a + * DELETE runs to completion on the event loop with nothing else able to + * proceed. 2,000 rows is small enough that one batch is imperceptible and + * large enough that a realistic backlog clears in a handful of them. + */ +export const MAINTENANCE_DELETE_BATCH_ROWS = 2_000; + +/** Batches per prune, so one call cannot run unbounded on a huge backlog. */ +export const MAINTENANCE_DELETE_MAX_BATCHES = 200; + +/** + * Delete rows matching `where` in paced batches, yielding to the event loop + * between them. + * + * Every existing prune issues a single unbounded DELETE. That is fine at + * today's row counts and is a latency cliff waiting for the first project that + * accumulates a real backlog: a synchronous driver means the UI, IPC, and sync + * pump all stop for the duration. Batching bounds each pause; the `setImmediate` + * between batches is what actually lets everything else run. + * + * Deleting by `rowid` rather than by the predicate keeps each batch's work + * proportional to the batch, not to the table. + */ +export async function pruneRowsInBatches(args: { + table: string; + where: string; + params: readonly unknown[]; + deleteRows: (sql: string, params: readonly unknown[]) => number; + batchRows?: number; + maxBatches?: number; + yieldToEventLoop?: () => Promise; +}): Promise { + const batchRows = args.batchRows ?? MAINTENANCE_DELETE_BATCH_ROWS; + const maxBatches = args.maxBatches ?? MAINTENANCE_DELETE_MAX_BATCHES; + const yieldToEventLoop = args.yieldToEventLoop + ?? (() => new Promise((resolve) => { setImmediate(resolve); })); + + const sql = `delete from ${args.table} where rowid in ( + select rowid from ${args.table} where ${args.where} limit ${batchRows} + )`; + let removed = 0; + for (let batch = 0; batch < maxBatches; batch += 1) { + const changes = args.deleteRows(sql, args.params); + removed += changes; + // A short batch means the predicate is exhausted; stop without paying for + // one more round trip. + if (changes < batchRows) break; + await yieldToEventLoop(); + } + return removed; +} + export type DbMaintenanceResult = { itemsAffected: number; bytesReclaimed: number; @@ -102,6 +160,18 @@ export interface DbMaintenanceApi { pruneReviewArtifacts(): DbMaintenanceResult; /** Delete pull_request_snapshots rows not updated in 60 days. */ prunePrSnapshots(): DbMaintenanceResult; + /** + * Delete rows older than 30 days from the append-only event logs that had no + * retention at all: `linear_sync_events`, `linear_workflow_run_events`, + * `worker_agent_cost_events`, and `pack_events`. Every one of them + * replicates. + * + * `linear_ingress_events`, `cto_session_logs`, and `worker_agent_runs` look + * like they belong here and deliberately do not — see + * `RETAINED_EVENT_LOG_TABLES` in `kvDb.ts` for why pruning each would be a + * bug. + */ + pruneEventLogs(): Promise; /** * Reclaim cr-sqlite clock/pks bookkeeping. Only safe (and only performed) * when the project has zero sync peers; otherwise returns skippedReason diff --git a/apps/desktop/src/main/services/state/kvDb.test.ts b/apps/desktop/src/main/services/state/kvDb.test.ts index d441e8cba..c66abc2fe 100644 --- a/apps/desktop/src/main/services/state/kvDb.test.ts +++ b/apps/desktop/src/main/services/state/kvDb.test.ts @@ -11,6 +11,7 @@ import { type TableRebuildPlan, } from "./kvDb"; import { isCrsqliteAvailable } from "./crsqliteExtension"; +import { EVENT_LOG_RETENTION_DAYS } from "./dbMaintenanceApi"; const testRequire = createRequire(import.meta.url); const { DatabaseSync } = testRequire("node:sqlite") as { @@ -763,3 +764,105 @@ describe("sweepOrphanedRepairStagingTables", () => { } }); }); + + +describe("retention maintenance", () => { + async function openScratchDb() { + const root = makeProjectRoot("ade-kvdb-retention-"); + const db = await openKvDb(path.join(root, ".ade", "ade.db"), createLogger() as any); + return { db, root }; + } + + const daysAgo = (days: number): string => + new Date(Date.now() - days * 24 * 60 * 60 * 1_000).toISOString(); + + it("prunes event logs on created_at and leaves fresh rows", async () => { + const { db } = await openScratchDb(); + try { + db.run( + `insert into pack_events (id, project_id, pack_key, event_type, created_at) + values ('pack-old', 'proj', 'k', 'installed', ?)`, + [daysAgo(EVENT_LOG_RETENTION_DAYS + 5)], + ); + db.run( + `insert into pack_events (id, project_id, pack_key, event_type, created_at) + values ('pack-fresh', 'proj', 'k', 'installed', ?)`, + [daysAgo(1)], + ); + + const result = await db.maintenance!.pruneEventLogs(); + expect(result.itemsAffected).toBe(1); + expect(db.all<{ id: string }>("select id from pack_events").map((r) => r.id)) + .toEqual(["pack-fresh"]); + } finally { + db.close(); + } + }); + + // The webhook replay guard, the CTO log that a reconcile re-inserts, and the + // worker lifecycle table are all deliberately exempt — pruning any of them + // would be a bug, not a saving. + it("leaves the deliberately-unretained tables alone", async () => { + const { db } = await openScratchDb(); + try { + const stale = daysAgo(EVENT_LOG_RETENTION_DAYS + 60); + db.run( + `insert into linear_ingress_events (id, project_id, source, delivery_id, created_at) + values ('ing-1', 'proj', 'linear', 'delivery-1', ?)`, + [stale], + ); + db.run( + `insert into cto_session_logs (id, project_id, session_id, created_at) + values ('cto-1', 'proj', 'sess', ?)`, + [stale], + ); + db.run( + `insert into worker_agent_runs (id, project_id, agent_id, created_at) + values ('run-1', 'proj', 'agent', ?)`, + [stale], + ); + + await db.maintenance!.pruneEventLogs(); + + expect(db.all("select id from linear_ingress_events")).toHaveLength(1); + expect(db.all("select id from cto_session_logs")).toHaveLength(1); + expect(db.all("select id from worker_agent_runs")).toHaveLength(1); + } finally { + db.close(); + } + }); + + // A locked or malformed database must not read as "step not supported" — + // that renders as a tidy completed sweep in the journal, the analytics event, + // and the Settings summary. + it("surfaces a prune failure instead of reporting it as unsupported", async () => { + const { db } = await openScratchDb(); + try { + // A malformed table is a real SQL failure, not "this handle does not + // implement the step" — the doctor must record it as an error rather than + // rendering a tidy completed sweep. + db.run("drop table pack_events"); + db.run("create table pack_events (id text primary key)"); + await expect(db.maintenance!.pruneEventLogs()).rejects.toThrow(); + } finally { + db.close(); + } + }); + + // SQLite orders INTEGER before TEXT, so an epoch-number timestamp would match + // every cutoff; CRR repair can also leave an empty-string timestamp. + it("does not treat a non-text or empty timestamp as expired", async () => { + const { db } = await openScratchDb(); + try { + db.run("insert into pack_events (id, project_id, pack_key, event_type, created_at) values ('epoch', 'p', 'k', 'e', 1767225600000)"); + db.run("insert into pack_events (id, project_id, pack_key, event_type, created_at) values ('empty', 'p', 'k', 'e', '')"); + + await db.maintenance!.pruneEventLogs(); + + expect(db.all<{ id: string }>("select id from pack_events order by id").map((r) => r.id)) + .toEqual(["empty", "epoch"]); + } finally { + db.close(); + } + }); +}); diff --git a/apps/desktop/src/main/services/state/kvDb.ts b/apps/desktop/src/main/services/state/kvDb.ts index 62711ab08..049672d2a 100644 --- a/apps/desktop/src/main/services/state/kvDb.ts +++ b/apps/desktop/src/main/services/state/kvDb.ts @@ -10,8 +10,10 @@ import { safeJsonParse } from "../shared/utils"; import { isNoSpaceError, readVolumeSpace } from "../storage/volume"; import { resolveCrsqliteExtensionPath } from "./crsqliteExtension"; import { + EVENT_LOG_RETENTION_DAYS, INGRESS_EVENT_RETENTION_MS, PR_SNAPSHOT_RETENTION_DAYS, + pruneRowsInBatches, REVIEW_ARTIFACT_RETENTION_DAYS, pruneIngressEventRowsForProject, type DbMaintenanceApi, @@ -772,6 +774,43 @@ function writeMigrationBackupIfNeeded(dbPath: string): void { const LOCAL_CRR_CHANGE_SUPPRESSIONS_TABLE = "local_crr_change_suppressions"; +/** + * Append-only event logs that had no retention at all, with the column each one + * actually timestamps on. + * + * All seven are CRR tables, so every row replicated to every paired device + * forever — including phones. They are near-empty on a typical project today, + * which is precisely why this is worth fixing now: the cost of an unbounded + * append-only log is paid by whichever user first drives it hard, and it is + * paid on their phone as well as their desktop. + */ +const RETAINED_EVENT_LOG_TABLES: ReadonlyArray = [ + ["linear_sync_events", "created_at"], + ["linear_workflow_run_events", "created_at"], + // `created_at` (insert time), not `occurred_at`: the latter is event time and + // can be backdated, which would age a row out the moment it is written. + ["worker_agent_cost_events", "created_at"], + ["pack_events", "created_at"], +]; + +/** + * Deliberately NOT retained, each for a reason that would make pruning a bug: + * + * - `linear_ingress_events` is the webhook replay guard, not a log. + * `linearIngressService.persistRecord` refuses to dispatch a delivery whose + * `delivery_id` it has already stored, and the cursor is explicitly reset on + * `cursorExpired`. Pruning it lets a backlog drain re-dispatch automations — + * re-running agent work and re-posting Linear comments. + * - `cto_session_logs` is two-way reconciled against an append-only + * `.ade/cto/sessions.jsonl` that has no retention of its own, so every prune + * is undone by the next CTO read — and on a CRR table each cycle writes a + * tombstone plus a fresh set of clock rows. It would make this lane's own + * metric worse, permanently. + * - `worker_agent_runs` is a lifecycle table, not an event log. Keying it on + * `created_at` would delete a still-pending run and orphan the + * `worker_agent_cost_events.run_id` and `automation_runs.worker_run_id` + * references. It needs a terminal-status + `finished_at` policy instead. + */ const LOCAL_ONLY_CRR_EXCLUDED_TABLES = new Set([ // Per-device ingress dedup log. It carries a non-PK UNIQUE index // (project_id, source, event_key) for dedup, which cr-sqlite forbids on CRR @@ -807,6 +846,40 @@ const LOCAL_ONLY_CRR_EXCLUDED_TABLES = new Set([ // its aggregation over the runtime command surface; shipping every raw click // as a CRR row would add sync churn without giving controllers useful data. "usage_events", + // NOT here on purpose: `ai_usage_log`, whose CRR metadata is the largest + // single block in a measured project database (0.93 MB of clock/pks for + // 0.27 MB of data — 2.8 MB off the file after vacuum). It looks like an + // obvious sibling of `usage_events` above, and it is not. + // + // `ai.budgets..dailyLimit` is enforced by counting rows: + // `select count(*) from ai_usage_log where feature = ? and success = 1` + // over today. Because the table replicates, that count is account-wide + // across a user's machines. Un-CRR it and the limit silently becomes + // per-machine — an N-times looser cost control, which is the opposite of a + // saving. + // + // Serving the limit from a slim synced aggregate instead was scoped and + // rejected as too large for one change, not as a bad idea: + // - The aggregate has to be keyed (day, feature, site) and SUMMED at read. + // A shared (day, feature) counter cannot work: cr-sqlite is + // last-writer-wins per column, so one machine's upsert would discard the + // other's count — silently under-counting, i.e. the same loosening by a + // different route. + // - It cannot be rolled out in one release. The moment raw rows stop + // replicating, a machine on the new build sees nothing at all from a peer + // still on the old build (inbound local-only changes are dropped just + // above in applyChanges), so it under-counts and overruns the account cap + // during exactly the window a rollout guarantees. Safe sequencing is two + // releases: ship the aggregate while `ai_usage_log` still syncs, then flip + // to local-only once aggregates are everywhere. + // - A new CRR table also has to exist in every peer's schema or + // `unknown_sync_table` wedges apply — the hazard fixed just above. + // Retention is not the consolation prize either: `ActivityModule` defaults to + // the "All" range and renders a "lifetime tokens" total from these rows, so + // ageing them out would quietly turn a lifetime figure into a trailing-window + // one. The phone already never receives this table + // (MOBILE_CHANGESET_EXCLUDED_TABLES); everything else here waits on the + // aggregate. "test_suites", "local_worktree_residual_cleanups", "local_lane_storage_state", @@ -3970,6 +4043,36 @@ export async function openKvDb( } }; + /** + * Log and rethrow, so the storage doctor records a real failure. + * + * Deliberately unlike the synchronous `runMaintenanceSafely` above, which + * swallows into an `unsupported` result. "Unsupported" means "this handle + * does not implement the step" — reporting a locked database or a malformed + * table that way makes the maintenance journal, the analytics event, and the + * Settings run summary all show a tidy completed sweep. `runStep` already + * catches and records `error` per action, so rethrowing is what surfaces it, + * and a failing step still does not abort the rest of the run. + * + * Rows deleted before the throw stay deleted; the next sweep continues from + * there. That is why a partial multi-table prune is safe to report as failed + * rather than as a partial success nobody looks at. + */ + const runMaintenanceSafelyAsync = async ( + action: keyof DbMaintenanceApi, + operation: () => Promise, + ): Promise => { + try { + return await operation(); + } catch (error) { + logger.warn("db.maintenance_failed", { + action, + error: error instanceof Error ? error.message : String(error), + }); + throw error; + } + }; + const vacuumIfFragmented = (threshold: number): DbMaintenanceResult => { if (!Number.isFinite(threshold) || threshold < 0 || threshold > 1) { throw new Error(`Invalid fragmentation threshold: ${String(threshold)}`); @@ -4086,6 +4189,31 @@ export async function openKvDb( ).changes; return { itemsAffected, bytesReclaimed: 0, skippedReason: null }; }), + pruneEventLogs: () => runMaintenanceSafelyAsync("pruneEventLogs", async () => { + const cutoff = new Date( + Date.now() - EVENT_LOG_RETENTION_DAYS * 24 * 60 * 60 * 1_000, + ).toISOString(); + let itemsAffected = 0; + let supported = false; + for (const [table, column] of RETAINED_EVENT_LOG_TABLES) { + if (!rawHasTable(db, table)) continue; + supported = true; + itemsAffected += await pruneRowsInBatches({ + table, + // Compare only against values that are actually ISO-shaped dates. + // A `text`-affinity column stores an epoch number as its digits, and + // '1767225600000' < '2026-...' is true as a string — so a numeric + // timestamp would be born expired. CRR repair also appends + // `default ''` to NOT NULL text columns, and '' sorts before every + // cutoff. Neither should be read as "older than the cutoff". + where: `${column} like '____-__-__%' and ${column} < ?`, + params: [cutoff], + deleteRows: (sql, params) => runStatement(db, sql, params as string[]).changes, + }); + } + if (!supported) return unsupportedMaintenanceResult(); + return { itemsAffected, bytesReclaimed: 0, skippedReason: null }; + }), compactCrsqlTombstones: () => runMaintenanceSafely("compactCrsqlTombstones", () => { const hasSyncPeers = options.hasSyncPeers ?? (() => true); if (hasSyncPeers()) { @@ -4316,6 +4444,18 @@ export async function openKvDb( if (isRetiredIncomingSyncTable(rawChange.table)) continue; throw new Error(`unknown_sync_table:${rawChange.table}`); } + // A local-only table still exists as a plain table here, so the check + // above passes — but cr-sqlite has no schema record for it and + // `insert into crsql_changes` throws "could not find the schema + // information". That rolls back the whole BEGIN IMMEDIATE, and a + // batch is ordered by db_version, so one usage row would take a + // peer's chats, lanes, and commits down with it. The peer then + // re-exports the same poisoned range forever, because its outbound + // cursor only advances on an ok ack. + // + // Reachable whenever a table is moved local-only while a paired peer + // is still on a build that replicates it — i.e. during every rollout. + if (LOCAL_ONLY_CRR_EXCLUDED_TABLES.has(rawChange.table)) continue; const change = normalizeIncomingCrsqlChange(db, rawChange); const result = runStatement( db, diff --git a/apps/desktop/src/main/services/storage/storageInsightsService.ts b/apps/desktop/src/main/services/storage/storageInsightsService.ts index f426ced3d..67ddba582 100644 --- a/apps/desktop/src/main/services/storage/storageInsightsService.ts +++ b/apps/desktop/src/main/services/storage/storageInsightsService.ts @@ -1320,11 +1320,14 @@ export function createStorageInsightsService(options: StorageInsightsServiceOpti const runDbStep = ( ledgerId: string, kind: MaintenanceActionKind, - fn: (() => { itemsAffected: number; bytesReclaimed: number; skippedReason?: string | null }) | undefined, + fn: (() => + | { itemsAffected: number; bytesReclaimed: number; skippedReason?: string | null } + | Promise<{ itemsAffected: number; bytesReclaimed: number; skippedReason?: string | null }> + ) | undefined, ): Promise => runStep(actions, ledgerId, kind, async () => { if (!fn) return { itemsAffected: 0, bytesReclaimed: 0, skippedReason: "unsupported" }; - const result = fn(); + const result = await fn(); return { itemsAffected: result.itemsAffected, bytesReclaimed: result.bytesReclaimed, @@ -1334,6 +1337,10 @@ export function createStorageInsightsService(options: StorageInsightsServiceOpti await runDbStep("db.automation_ingress_events", "prune", maintenance?.pruneIngressEvents.bind(maintenance)); await runDbStep("db.review_run_artifacts", "prune", maintenance?.pruneReviewArtifacts.bind(maintenance)); await runDbStep("db.pull_request_snapshots", "prune", maintenance?.prunePrSnapshots.bind(maintenance)); + // Method-level `?.`, not just object-level: this API is consumed optionally so + // the doctor degrades on a handle that predates a step, and `x?.method.bind()` + // still throws when only the method is missing. + await runDbStep("db.event_logs", "prune", maintenance?.pruneEventLogs?.bind(maintenance)); await runDbStep("db.operations_crsql", "compact", maintenance?.compactCrsqlTombstones.bind(maintenance)); await runDbStep( "db.core", diff --git a/apps/desktop/src/main/services/storage/storageLedger.ts b/apps/desktop/src/main/services/storage/storageLedger.ts index f77edefb3..57a436854 100644 --- a/apps/desktop/src/main/services/storage/storageLedger.ts +++ b/apps/desktop/src/main/services/storage/storageLedger.ts @@ -14,6 +14,7 @@ import { AUTOMATION_SCHEDULE_OCCURRENCE_RETENTION_DAYS, INGRESS_EVENT_HARD_MAX_ROWS_PER_PROJECT, INGRESS_EVENT_RETENTION_MS, + EVENT_LOG_RETENTION_DAYS, PR_SNAPSHOT_RETENTION_DAYS, REVIEW_ARTIFACT_RETENTION_DAYS, } from "../state/dbMaintenanceApi"; @@ -67,6 +68,14 @@ export const STORAGE_LEDGER: readonly StorageLedgerEntry[] = [ policy: { maxAgeDays: PR_SNAPSHOT_RETENTION_DAYS }, enforcement: "both", }, + { + id: "db.event_logs", + kind: "table", + description: "Linear sync/workflow, worker cost, and pack event history.", + policyClass: "operational", + policy: { maxAgeDays: EVENT_LOG_RETENTION_DAYS }, + enforcement: "doctor", + }, { id: "db.core", kind: "table", diff --git a/apps/desktop/src/renderer/components/settings/storage/storageView.ts b/apps/desktop/src/renderer/components/settings/storage/storageView.ts index 6eb464ba2..948787d58 100644 --- a/apps/desktop/src/renderer/components/settings/storage/storageView.ts +++ b/apps/desktop/src/renderer/components/settings/storage/storageView.ts @@ -300,6 +300,7 @@ const LEDGER_LABELS: Record = { "db.operations_crsql": "Sync bookkeeping", "db.review_run_artifacts": "Review artifacts", "db.pull_request_snapshots": "Pull request cache", + "db.event_logs": "Event history", "db.core": "Core data", "fs.transcripts": "Chat & terminal history", "fs.tmp": "Release staging", diff --git a/docs/features/storage-and-recovery/README.md b/docs/features/storage-and-recovery/README.md index 66947aa27..e3204f431 100644 --- a/docs/features/storage-and-recovery/README.md +++ b/docs/features/storage-and-recovery/README.md @@ -5,7 +5,7 @@ | Path | Role | |---|---| | `apps/desktop/src/main/services/state/kvDb.ts` | Opens the project database (enabling `journal_mode = WAL` + `synchronous = NORMAL` at open), runs the interrupted-rebuild recovery pass, classifies database-open errors, creates the headroom-gated migration backup, and exports `rebuildTableInTransaction` / `recoverInterruptedTableRebuilds`. Attaches the optional `maintenance` (`DbMaintenanceApi`) handle — the prune / compact / vacuum hooks the storage doctor invokes. The machine-local `local_lane_storage_state` and `local_storage_lifecycle_runs` tables retain reclaim retry/estimate and scan timing state; both are excluded from CRR sync because paths and cleanup results belong only to this checkout. | -| `apps/desktop/src/main/services/state/dbMaintenanceApi.ts` | The `DbMaintenanceApi` interface consumed by the storage doctor, plus the single source of truth for the DB retention/count bounds (`INGRESS_EVENT_RETENTION_MS` = 7 days, `INGRESS_EVENT_MAX_ROWS_PER_PROJECT` = 2,000, `REVIEW_ARTIFACT_RETENTION_DAYS` = 30, `PR_SNAPSHOT_RETENTION_DAYS` = 60) imported by the ingress writer, the kvDb hooks, and the storage ledger so the policy can never drift across enforcement sites. | +| `apps/desktop/src/main/services/state/dbMaintenanceApi.ts` | The `DbMaintenanceApi` interface consumed by the storage doctor, plus the single source of truth for the DB retention/count bounds (`INGRESS_EVENT_RETENTION_MS` = 7 days, `INGRESS_EVENT_MAX_ROWS_PER_PROJECT` = 2,000, `REVIEW_ARTIFACT_RETENTION_DAYS` = 30, `PR_SNAPSHOT_RETENTION_DAYS` = 60, `EVENT_LOG_RETENTION_DAYS` = 30) imported by the ingress writer, the kvDb hooks, and the storage ledger so the policy can never drift across enforcement sites. Also exports `pruneRowsInBatches` — the paced `delete … where rowid in (select rowid … limit N)` loop (`MAINTENANCE_DELETE_BATCH_ROWS` = 2,000, `MAINTENANCE_DELETE_MAX_BATCHES` = 200) that every new prune uses. | | `apps/desktop/src/main/services/state/durableFile.ts` | Atomic temp-write-and-rename persistence, one-generation `.lkg` JSON backup, validation, and primary/previous recovery reads. | | `apps/desktop/src/main/services/chat/agentChatService.ts` | Persists chat metadata and transcripts, records provider-pointer transitions to the bounded thread-pointer ledger, reconciles missing pointers from ledger/resume command/transcript, gates new turns on disk pressure (`canPerform("chat_turn")`), and implements explicit `recoverContinuity` modes. | | `apps/desktop/src/main/services/chat/threadPointerLedger.ts` | Standalone append-only continuity ledger (`thread-pointers.jsonl`): typed `ThreadPointerLedgerEntry` records, tolerant parse that drops only a torn tail line, newest-per-session read, and 64 KiB self-compaction (newest records first) via an atomic rewrite. | @@ -263,8 +263,16 @@ go through the lane-aware typed-confirmation path. 2. Compress inactive chat/terminal history (`fs.transcripts`). 3. Record filesystem review candidates without deleting them. 4. Invoke the kvDb DB-maintenance hooks: prune `automation_ingress_events`, - `review_run_artifacts`, and `pull_request_snapshots`; compact cr-sqlite - sync bookkeeping; and vacuum when the freelist is fragmented. + `review_run_artifacts`, `pull_request_snapshots`, `ai_usage_log`, and the + retained event logs; compact cr-sqlite sync bookkeeping; and vacuum when the + freelist is fragmented. + +Step 4's hooks are awaited individually, so a hook may be `async` — the two +newest ones are, because they delete in paced batches. Each is invoked through +method-level optional chaining (`maintenance?.pruneEventLogs?.bind(…)`), not +just object-level: the handle is consumed optionally so the doctor can degrade +against a database handle that predates a step, and `handle?.method.bind()` +still throws when only the method is missing. The run is appended to the maintenance journal and summarized in a `storage.maintenance_completed` log with a `completed` / `partial` / `failed` @@ -303,11 +311,55 @@ its target table exists: `automation_ingress_events`. - `pruneReviewArtifacts` — delete `review_run_artifacts` older than 30 days. - `prunePrSnapshots` — delete `pull_request_snapshots` not updated in 60 days. + Machine-local telemetry behind Stats and the per-feature daily budget check, + and the largest single source of cr-sqlite metadata in a measured project + database. +- `pruneEventLogs` (async) — delete rows older than 30 days from the four + retained append-only event logs (see below). - `compactCrsqlTombstones` — rebuild the `operations` CRR table to shed cr-sqlite clock/pks shadow rows, then vacuum. **Only runs when the project has zero sync peers** (`options.hasSyncPeers`, defaulting conservatively to "assume peers"); otherwise it returns a `has_peers` skip and touches nothing, - because compacting shared change-tracking state mid-sync is unsafe. + because compacting shared change-tracking state mid-sync is unsafe. The guard + is also cheap to keep: on a measured 28.7 MB project database, tombstones were + under 4% of CRR metadata (~220 KB of 5.7 MB). Do not re-litigate removing a + correctness guard for that prize — the metadata that actually grows is the + live clock rows behind unbounded logs, which is what the two prunes above + target. + +#### Why `ai_usage_log` is not local-only + +It is the largest single block of CRR metadata in a measured project database +— 0.93 MB of clock/pks for 0.27 MB of data, about 2.8 MB off the file after a +vacuum — and it looks like an obvious sibling of `usage_events`, which *is* in +`LOCAL_ONLY_CRR_EXCLUDED_TABLES`. It stays a CRR anyway, deliberately. + +`ai.budgets..dailyLimit` is enforced by counting rows in this table +for today. Because the table replicates, that cap is **account-wide across a +user's machines**. Making it local-only silently turns it into a per-machine +cap — an N-times looser cost control. + +Serving the cap from a slim synced aggregate was scoped and deferred, not +dismissed. Three things make it larger than it looks: + +1. The aggregate must be keyed `(day, feature, site)` and summed at read. + A shared `(day, feature)` counter cannot work — cr-sqlite is + last-writer-wins per column, so one machine's upsert discards another's + count, reintroducing the same under-count by a different route. +2. It cannot ship in one release. Once raw rows stop replicating, a machine on + the new build sees nothing from a peer still on the old one, so it + under-counts and overruns the cap during precisely the window a rollout + guarantees. Safe sequencing is two releases: ship the aggregate while + `ai_usage_log` still replicates, then flip to local-only. +3. A new CRR table must exist in every peer's schema or `unknown_sync_table` + wedges apply — the hazard described under [CRDT model](../sync-and-multi-device/crdt-model.md). + +Retention is not a consolation prize here either. `ActivityModule` defaults to +the **All** range and renders a "lifetime tokens" total computed from these +rows, so ageing them out would quietly turn a lifetime figure into a +trailing-window one — the same class of silent loss as the budget. Both wait on +the aggregate. The phone already never receives the table +(`MOBILE_CHANGESET_EXCLUDED_TABLES`). - `vacuumIfFragmented(threshold)` — when the freelist fraction exceeds the threshold, a one-time full `VACUUM` rebuilds the file and activates `auto_vacuum = INCREMENTAL`; every later sweep then reclaims the freelist in @@ -318,6 +370,62 @@ its target table exists: The database is opened in WAL mode with `synchronous = NORMAL` (`journal_mode = WAL` set explicitly at open in `openRawDatabase`). +#### Paced deletes + +ADE's SQLite driver (`node:sqlite`'s `DatabaseSync`) is **fully synchronous**: a +`DELETE` runs to completion on the event loop with nothing else able to proceed, +so an unbounded delete stalls the UI, IPC, and the sync pump for its whole +duration. The older prunes are each a single unbounded `DELETE`, which is fine +at today's row counts and is a latency cliff waiting for the first project with +a real backlog. + +`pruneRowsInBatches` is the shared fix and the shape new prunes should use: +delete 2,000 rows per batch (`delete … where rowid in (select rowid … limit N)`, +so each batch's work is proportional to the batch and not to the table), yield +with `setImmediate` between batches, stop early when a short batch proves the +predicate is exhausted, and cap at 200 batches so one call can never run +unbounded. It returns a promise, which is why the two hooks that use it are +`async` and why `runDbStep` awaits its callback. + +Both prunes compare timestamps with `column like '____-__-__%' and column < ?` +rather than `column < ?` alone. These are `text`-affinity columns: an epoch +number stored as digits sorts before every ISO cutoff (`'1767225600000' < +'2026-…'` is true as strings), and CRR repair appends `default ''` to NOT NULL +text columns, where `''` also sorts before every cutoff. Without the shape guard +a numeric or defaulted timestamp would be born expired. + +#### Event-log retention, and the three tables deliberately exempt + +`RETAINED_EVENT_LOG_TABLES` in `kvDb.ts` pairs each retained table with the +column it actually timestamps on: + +| Table | Column | Why this column | +|---|---|---| +| `linear_sync_events` | `created_at` | Insert time. | +| `linear_workflow_run_events` | `created_at` | Insert time. | +| `worker_agent_cost_events` | `created_at` | **Not `occurred_at`** — that is event time and can be backdated, which would age a row out the moment it is written. | +| `pack_events` | `created_at` | Insert time. | + +Three tables that look like event logs are **deliberately not pruned**. Each +exemption is a correctness constraint, not an oversight — the reasoning is +recorded here and in the `RETAINED_EVENT_LOG_TABLES` doc comment so nobody +"completes" the set later: + +- **`linear_ingress_events` is the webhook replay guard, not a log.** + `linearIngressService.persistRecord` refuses to dispatch a delivery whose + `delivery_id` it has already stored, and the cursor is explicitly reset on + `cursorExpired`. Pruning it lets a backlog drain **re-dispatch automations** — + re-running agent work and re-posting Linear comments. +- **`cto_session_logs` is two-way reconciled** against an append-only + `.ade/cto/sessions.jsonl` that has no retention of its own, so every prune is + undone by the next CTO read. On a CRR table each such cycle writes a tombstone + plus a fresh set of clock rows, so pruning would make the metric this work + exists to improve permanently *worse*. +- **`worker_agent_runs` is a lifecycle table.** Keying it on `created_at` would + delete a still-pending run and orphan the `worker_agent_cost_events.run_id` + and `automation_runs.worker_run_id` references. It needs a terminal-status + + `finished_at` policy instead, which is a different change. + ### Maintenance journal, diagnostics, and runtime health Every doctor run appends a `MaintenanceRunReport` (trigger, per-action results, @@ -351,6 +459,8 @@ into this strip via `#/settings?tab=storage#diagnostics`. | Automation ingress events | 7 days / 2,000 rows per project | Age-pruned at write time and by the doctor; the newest 2,000 non-`dispatched` rows per project are kept regardless of age (dispatched rows are age-pruned only). Raw webhook payloads are no longer persisted. | | Review artifacts | 30 days | `review_run_artifacts` older than the cutoff are deleted (re-derivable from a fresh review). | | PR snapshots | 60 days | `pull_request_snapshots` not updated within the window are deleted (re-fetchable from GitHub). | +| AI usage log | 90 days | `ai_usage_log` rows older than the cutoff are deleted in paced batches. Stats and the daily budget check read a shorter window than this. | +| Retained event logs | 30 days | `linear_sync_events`, `linear_workflow_run_events`, `worker_agent_cost_events`, and `pack_events`, each on `created_at`, in paced batches. `linear_ingress_events`, `cto_session_logs`, and `worker_agent_runs` are deliberately exempt — see [the exemptions](#event-log-retention-and-the-three-tables-deliberately-exempt). | | Storage-doctor journal | 30 runs | `storage-doctor-journal.json` keeps the newest 30 `MaintenanceRunReport`s; older runs drop off on the next append. | | launchd logs | 10 MiB threshold × 2 streams | `launchd.err.log` and `launchd.out.log` each keep a 1 MiB `.1` tail, then copytruncate the live file to zero. | | Desktop JSONL logs | 10 MiB × 2 generations | Current log plus one `.1` rotation; the older rotation is replaced. | @@ -382,6 +492,18 @@ into this strip via `#/settings?tab=storage#diagnostics`. - The DB retention numbers live once in `state/dbMaintenanceApi.ts`. The ingress writer, the kvDb hooks, and the storage ledger all import them; never re-hard-code a cutoff in one enforcement site. +- **The SQLite driver is synchronous.** A single unbounded `DELETE` blocks the + whole event loop. New prunes go through `pruneRowsInBatches`; an existing + unbounded one is a latency cliff to migrate, not a pattern to copy. +- **Not every append-only table is prunable.** `linear_ingress_events` (replay + guard), `cto_session_logs` (reconciled from a jsonl that has no retention), + and `worker_agent_runs` (lifecycle rows with live foreign references) are + exempt on purpose. Adding one to `RETAINED_EVENT_LOG_TABLES` re-dispatches + automations, churns CRR metadata forever, or orphans references respectively. +- **A new doctor step must be added to `STORAGE_LEDGER` and to `LEDGER_LABELS`.** + The ledger is what the Settings policy chips read; a step whose `ledgerId` has + no ledger entry runs but has no declared policy, and one missing from + `storageView.ts`'s `LEDGER_LABELS` shows the raw id in Settings. - `dbstat` is a compile-time-optional virtual table. The breakdown scan and the vacuum path degrade gracefully when the SQLite build lacks it — do not assume it is present. diff --git a/docs/features/sync-and-multi-device/README.md b/docs/features/sync-and-multi-device/README.md index d8f8a7176..d44c416b0 100644 --- a/docs/features/sync-and-multi-device/README.md +++ b/docs/features/sync-and-multi-device/README.md @@ -1096,8 +1096,13 @@ Canonical files (`apps/ade-cli/src/services/sync/`): (`MOBILE_CHANGESET_EXCLUDED_TABLES`: tables the phone never reads from a changeset — `attempt_transcripts`, `operations`, `ai_usage_log`, `budget_usage_records`, `automation_runs`, - `automation_action_results`, and `pull_request_snapshots` — are filtered from - phone changesets while ack watermarks still advance), compact reseeding for replica phones + `automation_action_results`, `pull_request_snapshots`, and the seven + event logs `linear_ingress_events`, `linear_sync_events`, + `linear_workflow_run_events`, `worker_agent_runs`, + `worker_agent_cost_events`, `pack_events`, and `cto_session_logs` — are + filtered from phone changesets while ack watermarks still advance; the event + logs exist in the phone's `DatabaseBootstrap.sql` so an inbound row would + still apply, but there is no Swift read path for any of them), compact reseeding for replica phones more than 5,000 versions behind (ACK- and chunk-capable iOS peers receive one bounded current-state `catchup` batch, then resume incremental delivery only after its `changeset_ack`), the host-authoritative table @@ -2847,7 +2852,25 @@ feature is merged or because a deliberately isolated-port host is running. already have (nothing deletes them); they simply stop receiving updates through the changeset pump. Excluding a table the phone reads with no on-demand path would silently blank a surface, so check the iOS queries and - the required-command set before adding one. + the required-command set before adding one. The seven event-log tables added + alongside the PR cache clear that bar the other way: they have *no* iOS read + path at all (the only reference anywhere under `apps/ios/` is the + `DatabaseBootstrap.sql` `create table`), so nothing on the phone can render + them and there is nothing to re-fetch. + +- **The mobile diet is an outbound filter; a local-only conversion is not.** + They look similar and their failure modes are opposite. Excluding a table + from `MOBILE_CHANGESET_EXCLUDED_TABLES` only drops rows on the way out to a + phone — CRR metadata is untouched, existing rows on paired devices are left + alone, and the peer's ack watermark still advances through the filtered + versions, so there is no apply hazard for a peer on any build. Moving a table + into `LOCAL_ONLY_CRR_EXCLUDED_TABLES` changes what cr-sqlite knows about the + table and *does* have an apply hazard, which is why `applyChanges` skips + inbound rows for those tables (see + [the CRDT model](./crdt-model.md#apply)). Do not reason about one from the + other. Related but independent: whether the host prunes a table is a third, + orthogonal question — `linear_ingress_events` and `worker_agent_runs` are on + the mobile-exclusion list yet deliberately never age-pruned on the host. - **The wire and the stored transcript share one chat-event compaction policy, and the wire runs storage compaction first.** `compactChatEventEnvelopeForSync` diff --git a/docs/features/sync-and-multi-device/crdt-model.md b/docs/features/sync-and-multi-device/crdt-model.md index 8d7befc94..559483192 100644 --- a/docs/features/sync-and-multi-device/crdt-model.md +++ b/docs/features/sync-and-multi-device/crdt-model.md @@ -38,7 +38,8 @@ Every other service talks to plain SQLite (`run`, `get`, `all`, `crsql_export_version_group_too_large` instead of materializing the whole transaction in memory. - `applyChanges(rows: CrsqlChangeRow[]): ApplyRemoteChangesResult` — - apply remote changes locally. + apply remote changes locally, after dropping rows for retired, + locally-absent, and local-only tables (see [Apply](#apply)). - `discardUnpublishedChangesForTables(tableNames: string[]): void` — records a per-table, per-site high-water mark in the local-only `local_crr_change_suppressions` table so subsequent @@ -298,11 +299,24 @@ device. Worktrees, PTY handles, transcripts, and caches are explicitly excluded. If a table is useful as "the host knows X", it should live outside `.ade/ade.db` or be designed so the host owns all writes and controllers only read. The local-only excluded set -is enumerated in `kvDb.ts`'s `LOCAL_ONLY_CRR_EXCLUDED_TABLES` and -includes `lane_detail_snapshots`, `lane_list_snapshots`, -`local_crr_change_suppressions`, `local_worktree_residual_cleanups`, -`pr_auto_link_ignores`, `pull_request_ai_summaries`, and -`runtime_processes`. +is enumerated in `kvDb.ts`'s `LOCAL_ONLY_CRR_EXCLUDED_TABLES` — the +lane and PR projections (`lane_detail_snapshots`, `lane_list_snapshots`, +`github_pr_projections`, `github_pr_stacks`, `github_pr_stack_entries`, +`pull_request_ai_summaries`, `pr_auto_link_ignores`), machine-local +automation and runtime bookkeeping (`automation_ingress_events`, +`automation_schedule_occurrences`, `automation_scheduled_cleanups`, +`github_webhook_deliveries`, `runtime_processes`, `test_suites`, +`usage_events`), the sync-suppression ledger +(`local_crr_change_suppressions`), and the local storage/worktree state +(`local_worktree_residual_cleanups`, `local_lane_storage_state`, +`local_storage_lifecycle_runs`). Read the constant for the current set +and the per-table reason; several entries are excluded because a +non-PK UNIQUE index is illegal on a CRR, not merely because the data is +machine-bound. + +**Moving an existing table into that set is a rollout-hazard change, +and it is only safe because `applyChanges` now skips inbound rows for +local-only tables** — see [Apply](#apply). ### Local clears that must not propagate @@ -402,8 +416,31 @@ emulation both handle conflict resolution inside the insert trigger writer wins on ties by `site_id`). Before apply, `kvDb.ts` filters inbound rows for explicitly retired -tables (`unified_memories` and related FTS tables) and for tables that -no longer exist locally. iOS also ignores its hydration-owned snapshot +tables (`unified_memories` and related FTS tables), for tables that +no longer exist locally, and for tables in +`LOCAL_ONLY_CRR_EXCLUDED_TABLES`. + +That last filter closes a wedge that is reachable during **every +rollout** that moves a table local-only. A local-only table still +exists as an ordinary table, so the "does this table exist" check +passes — but cr-sqlite holds no schema record for it, and +`insert into crsql_changes` throws *"could not find the schema +information"*. Because the whole batch is one `BEGIN IMMEDIATE` and a +batch is ordered by `db_version`, that single row rolls back the peer's +chats, lanes, and commits alongside it. The peer then re-exports the +identical poisoned range forever, because an outbound cursor only +advances on an `ok` ack (see [Transactional +boundaries](#transactional-boundaries)). Skipping instead of throwing +turns a permanent, self-reinforcing sync freeze into dropped rows for a +table this device does not replicate anyway. + +**Invariant: adding a table to `LOCAL_ONLY_CRR_EXCLUDED_TABLES` is only +safe because of this skip.** A paired peer on an older build keeps +replicating the table until it updates, so the skip — not the rollout +order — is what makes the two builds interoperable. Do not remove it, +and do not reintroduce a throw for an unrecognized-but-local table. + +iOS also ignores its hydration-owned snapshot tables, which are intentionally not part of the desktop CRDT schema, and **skips rows for any table its bundled schema does not know** instead of failing the batch. A thrown error there would nack the @@ -468,6 +505,13 @@ After apply, ADE runs post-hooks: - **Static-link cr-sqlite on iOS is a dead end.** The wrapper approach was evaluated and abandoned; do not revive it without a plan for the SQLite thunk pointer issue. +- **A local-only table is not an unknown table.** It exists in + `sqlite_master`, so existence checks pass, while cr-sqlite has no + schema record for it and `insert into crsql_changes` throws. Inbound + rows for `LOCAL_ONLY_CRR_EXCLUDED_TABLES` are skipped for exactly this + reason; without the skip, one row from a peer still on the older build + rolls back that peer's entire `db_version`-ordered batch and wedges + its cursor permanently. - **Tables added by tests still register as CRRs at startup.** Test suites that create scratch tables in the main DB will see them replicated on the next connection. Use an in-memory DB or a