From 1761a7ffd22e8c128d117ceb334288d40899b2d9 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Wed, 3 Jun 2026 08:45:48 -0700 Subject: [PATCH] FN-5940: stop logging no-op task:moved activity rows Prevent same-column task move noise from bloating the activity log. - skip task:moved activity logging and event emission when a task stays in the same column - add a one-time TaskStore cleanup migration that deletes existing same-column task:moved rows - cover listener, moveTask, polling replication, and migration behavior with focused core tests - document the no-op cleanup invariant and manual VACUUM guidance for reclaimed SQLite space Files changed: docs/storage.md | 8 ++ packages/core/src/__tests__/activity-log-no-op-moved.test.ts | 122 +++++++++++++++++++++ packages/core/src/__tests__/no-op-moved-cleanup-migration.test.ts | 108 ++++++++++++++++++ packages/core/src/store.ts | 51 ++++++++- 4 files changed, 286 insertions(+), 3 deletions(-) Fusion-Task-Id: FN-5940 Fusion-Task-Lineage: f34cf8d0-3639-44d5-8a90-8ba461636eaa --- docs/storage.md | 8 ++ .../activity-log-no-op-moved.test.ts | 122 ++++++++++++++++++ .../no-op-moved-cleanup-migration.test.ts | 108 ++++++++++++++++ packages/core/src/store.ts | 51 +++++++- 4 files changed, 286 insertions(+), 3 deletions(-) create mode 100644 packages/core/src/__tests__/activity-log-no-op-moved.test.ts create mode 100644 packages/core/src/__tests__/no-op-moved-cleanup-migration.test.ts diff --git a/docs/storage.md b/docs/storage.md index 492fadc239..55e98d9700 100644 --- a/docs/storage.md +++ b/docs/storage.md @@ -22,6 +22,14 @@ - Archived-task snapshot behavior (`taskToArchiveEntry` / `archiveTask`) is unchanged in spirit: archive payloads still embed a capped agent-log snapshot, now sourced from the JSONL file instead of `fusion.db`. - Retention is now independent from SQLite operational-log pruning. `settings.agentLogFileRetentionDays` controls age-based pruning of JSONL entries for soft-deleted and archived tasks only. Default: `0` (disabled). +### Activity-log no-op `task:moved` cleanup (FN-5940) + +- `TaskStore` now defends the invariant that `activityLog` never records a `task:moved` row when `metadata.from === metadata.to`. +- Defense is layered: the `task:moved` listener skips same-column transitions, and source emitters skip no-op `archived -> archived` / same-column polling re-emits before subscribers see them. +- Existing junk rows are removed by a one-time init migration guarded by `__meta.noOpTaskMovedActivityCleanupVersion = "1"`. +- The cleanup deletes only rows matching `type = 'task:moved'` where `json_extract(metadata, '$.from') = json_extract(metadata, '$.to')`; legitimate distinct-column moves are preserved. +- The migration does **not** run `VACUUM` automatically. After the delete lands on a large disk-backed DB, run `fn db --vacuum` manually to reclaim the freed space from the SQLite file. + ### Dashboard delete-event handling (FN-5135) - Dashboard clients treat any SSE payload with `deletedAt != null` (`task:created`, `task:updated`, `task:moved`, `task:merged`) as a delete-equivalent and remove/suppress that task locally. diff --git a/packages/core/src/__tests__/activity-log-no-op-moved.test.ts b/packages/core/src/__tests__/activity-log-no-op-moved.test.ts new file mode 100644 index 0000000000..81eb11f0fc --- /dev/null +++ b/packages/core/src/__tests__/activity-log-no-op-moved.test.ts @@ -0,0 +1,122 @@ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +import { rm } from "node:fs/promises"; + +import { TaskStore } from "../store.js"; +import { createTaskStoreTestHarness, makeTmpDir } from "./store-test-helpers.js"; + +describe("activity log task:moved no-op guard", () => { + const harness = createTaskStoreTestHarness(); + + beforeEach(async () => { + await harness.beforeEach(); + }); + + afterEach(async () => { + await harness.afterEach(); + }); + + it("does not record same-column task:moved emits and still records distinct moves", async () => { + const store = harness.store(); + const task = await harness.createTestTask(); + + (store as any).emit("task:moved", { task, from: "archived", to: "archived", source: "engine" }); + expect(await store.getActivityLog({ type: "task:moved" })).toEqual([]); + + (store as any).emit("task:moved", { task, from: "triage", to: "todo", source: "engine" }); + + const activity = await store.getActivityLog({ type: "task:moved" }); + expect(activity).toHaveLength(1); + expect(activity[0]).toMatchObject({ + type: "task:moved", + taskId: task.id, + metadata: { from: "triage", to: "todo" }, + }); + }); + + it("does not record activity for same-column moveTask calls", async () => { + const store = harness.store(); + const task = await harness.createTestTask(); + + await store.moveTask(task.id, "triage"); + + expect(await store.getActivityLog({ type: "task:moved" })).toEqual([]); + }); + + it("records legitimate moveTask transitions exactly once", async () => { + const store = harness.store(); + const task = await harness.createTestTask(); + + await store.moveTask(task.id, "todo"); + + expect(await store.getActivityLog({ type: "task:moved" })).toEqual([ + expect.objectContaining({ + taskId: task.id, + metadata: { from: "triage", to: "todo" }, + }), + ]); + }); + + it("does not emit or record archived-to-archived polling replication no-ops", async () => { + const rootDir = makeTmpDir(); + const globalDir = makeTmpDir(); + const writer = new TaskStore(rootDir, globalDir); + const observer = new TaskStore(rootDir, globalDir); + + try { + await writer.init(); + await observer.init(); + + const task = await writer.createTask({ column: "done", description: "archive me" }); + const archived = await writer.archiveTask(task.id, false); + const movedEvents: Array<{ from: string; to: string }> = []; + observer.on("task:moved", ({ from, to }) => movedEvents.push({ from, to })); + (observer as any).taskCache.set(archived.id, { ...archived }); + (observer as any).lastKnownModified = 0; + + await (observer as any).checkForChanges(); + + expect(movedEvents).toEqual([]); + expect(await observer.getActivityLog({ type: "task:moved" })).toEqual([ + expect.objectContaining({ + taskId: task.id, + metadata: { from: "done", to: "archived" }, + }), + ]); + } finally { + writer.close(); + observer.close(); + await rm(rootDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 50 }); + await rm(globalDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 50 }); + } + }); + + it("does not emit or record same-column polling observations", async () => { + const rootDir = makeTmpDir(); + const globalDir = makeTmpDir(); + const writer = new TaskStore(rootDir, globalDir); + const observer = new TaskStore(rootDir, globalDir); + + try { + await writer.init(); + await observer.init(); + + const task = await writer.createTask({ column: "todo", description: "same-column poll" }); + const movedEvents: Array<{ from: string; to: string }> = []; + observer.on("task:moved", ({ from, to }) => movedEvents.push({ from, to })); + (observer as any).taskCache.set(task.id, { ...task }); + (observer as any).lastKnownModified = 0; + + await writer.updateTask(task.id, { title: "still todo" }); + await (observer as any).checkForChanges(); + + expect(movedEvents).toEqual([]); + expect(await observer.getActivityLog({ type: "task:moved" })).toEqual([]); + } finally { + writer.close(); + observer.close(); + await rm(rootDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 50 }); + await rm(globalDir, { recursive: true, force: true, maxRetries: 5, retryDelay: 50 }); + } + }); +}); diff --git a/packages/core/src/__tests__/no-op-moved-cleanup-migration.test.ts b/packages/core/src/__tests__/no-op-moved-cleanup-migration.test.ts new file mode 100644 index 0000000000..d8cca1d6f3 --- /dev/null +++ b/packages/core/src/__tests__/no-op-moved-cleanup-migration.test.ts @@ -0,0 +1,108 @@ +import { afterEach, beforeEach, describe, expect, it } from "vitest"; + +import { createTaskStoreTestHarness } from "./store-test-helpers.js"; + +describe("no-op task:moved activity cleanup migration", () => { + const harness = createTaskStoreTestHarness(); + + beforeEach(async () => { + await harness.beforeEach(); + await harness.reopenDiskBackedStore(); + }); + + afterEach(async () => { + await harness.afterEach(); + }); + + it("deletes only no-op task:moved rows once and leaves later rows untouched", async () => { + const store = harness.store(); + const db = store.getDatabase(); + const task = await harness.createTestTask(); + const insert = db.prepare( + `INSERT INTO activityLog (id, timestamp, type, taskId, taskTitle, details, metadata) + VALUES (?, ?, ?, ?, ?, ?, ?)`, + ); + + insert.run( + "noop-1", + "2026-06-03T00:00:01.000Z", + "task:moved", + task.id, + task.title ?? null, + "noop archived", + JSON.stringify({ from: "archived", to: "archived" }), + ); + insert.run( + "noop-2", + "2026-06-03T00:00:02.000Z", + "task:moved", + task.id, + task.title ?? null, + "noop todo", + JSON.stringify({ from: "todo", to: "todo" }), + ); + insert.run( + "move-1", + "2026-06-03T00:00:03.000Z", + "task:moved", + task.id, + task.title ?? null, + "real move", + JSON.stringify({ from: "triage", to: "todo" }), + ); + insert.run( + "created-1", + "2026-06-03T00:00:04.000Z", + "task:created", + task.id, + task.title ?? null, + "created", + null, + ); + db.prepare("DELETE FROM __meta WHERE key = ?").run("noOpTaskMovedActivityCleanupVersion"); + + await harness.reopenDiskBackedStore(); + + const migratedDb = harness.store().getDatabase(); + const movedRows = migratedDb.prepare( + "SELECT id, metadata FROM activityLog WHERE type = 'task:moved' ORDER BY id", + ).all() as Array<{ id: string; metadata: string | null }>; + const migrationRow = migratedDb + .prepare("SELECT value FROM __meta WHERE key = ?") + .get("noOpTaskMovedActivityCleanupVersion") as { value: string } | undefined; + + expect(movedRows).toEqual([ + { + id: "move-1", + metadata: JSON.stringify({ from: "triage", to: "todo" }), + }, + ]); + const createdRows = migratedDb.prepare( + "SELECT id FROM activityLog WHERE type = 'task:created' ORDER BY id", + ).all() as Array<{ id: string }>; + expect(createdRows.map((row) => row.id)).toContain("created-1"); + expect(migrationRow?.value).toBe("1"); + + migratedDb.prepare("DELETE FROM activityLog WHERE id = ?").run("move-1"); + migratedDb.prepare( + `INSERT INTO activityLog (id, timestamp, type, taskId, taskTitle, details, metadata) + VALUES (?, ?, 'task:moved', ?, ?, ?, ?)`, + ).run( + "noop-after", + "2026-06-03T00:00:05.000Z", + task.id, + task.title ?? null, + "post-migration noop", + JSON.stringify({ from: "archived", to: "archived" }), + ); + + await harness.reopenDiskBackedStore(); + + const reopenedDb = harness.store().getDatabase(); + const postReopenRows = reopenedDb.prepare( + "SELECT id FROM activityLog WHERE type = 'task:moved' ORDER BY id", + ).all() as Array<{ id: string }>; + + expect(postReopenRows).toEqual([{ id: "noop-after" }]); + }); +}); diff --git a/packages/core/src/store.ts b/packages/core/src/store.ts index aa291da4bf..9c3b1f6434 100644 --- a/packages/core/src/store.ts +++ b/packages/core/src/store.ts @@ -1413,6 +1413,7 @@ export class TaskStore extends EventEmitter { } await this.migrateActiveArchivedTasksToArchiveDb(); await this.migrateAgentLogEntriesToFilesOnce(); + await this.cleanupNoOpTaskMovedActivityRowsOnce(); if (this.db.getSchemaVersion() < SCHEMA_VERSION) { this.db.init(); } @@ -2607,6 +2608,7 @@ export class TaskStore extends EventEmitter { // Task moved this.on("task:moved", (data) => { if (this.suppressActivityLogForPollingEmit) return; + if (data.from === data.to) return; this.recordActivityFromListener( { type: "task:moved", @@ -5686,7 +5688,9 @@ export class TaskStore extends EventEmitter { if (this.isWatching) this.taskCache.set(id, { ...task }); - this.emit("task:moved", { task, from: fromColumn, to: toColumn, source: moveSource }); + if (fromColumn !== toColumn) { + this.emit("task:moved", { task, from: fromColumn, to: toColumn, source: moveSource }); + } return task; } @@ -8634,9 +8638,12 @@ export class TaskStore extends EventEmitter { if (archivedSet.has(id)) { // Task moved to archive — emit task:moved (matching what // archiveTask emits in-process) so other subscribers can react. - // Activity-log listeners skip this emit; the originating + // Skip already-archived cache entries to avoid no-op emits. + // Activity-log listeners skip polling emits; the originating // TaskStore instance wrote the row in-process. - this.emit("task:moved", { task: cached, from: cached.column, to: "archived" as Column, source: "engine" }); + if (cached.column !== "archived") { + this.emit("task:moved", { task: cached, from: cached.column, to: "archived" as Column, source: "engine" }); + } } else { // Polling replicas only mirror the originating delete signal. // Do not record run-audit here; the writer already owns that row. @@ -10290,6 +10297,44 @@ export class TaskStore extends EventEmitter { this.db.bumpLastModified(); } + private async cleanupNoOpTaskMovedActivityRowsOnce(): Promise { + const migrationKey = "noOpTaskMovedActivityCleanupVersion"; + const migrationVersion = "1"; + const row = this.db.prepare("SELECT value FROM __meta WHERE key = ?").get(migrationKey) as + | { value: string } + | undefined; + + if (row?.value === migrationVersion) { + return; + } + + const hasTable = + this.db.prepare("SELECT 1 FROM sqlite_master WHERE type = 'table' AND name = 'activityLog' LIMIT 1").get() !== + undefined; + const markDone = () => { + this.db.prepare(` + INSERT INTO __meta (key, value) VALUES (?, ?) + ON CONFLICT(key) DO UPDATE SET value = excluded.value + `).run(migrationKey, migrationVersion); + }; + + if (!hasTable) { + markDone(); + this.db.bumpLastModified(); + return; + } + + this.db.transactionImmediate(() => { + this.db.prepare(` + DELETE FROM activityLog + WHERE type = 'task:moved' + AND json_extract(metadata, '$.from') = json_extract(metadata, '$.to') + `).run(); + markDone(); + this.db.bumpLastModified(); + }); + } + // ── Archive Cleanup Methods ───────────────────────────────────────── /**