From 6e0fde860c798f2f76afe9f4a8a6988958617d62 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Mon, 13 Jul 2026 13:10:53 -0700 Subject: [PATCH] FN-7949: fix deleted planning-mode session resurrecting after in-flight generation completes Adds a bounded-TTL delete tombstone to AiSessionStore so a straggling post-delete generation write can never resurrect a session the user explicitly deleted. - AiSessionStore now records a 10-minute delete tombstone (id -> deletion timestamp) in delete(), deleteByIdAndType(), and bulk cleanup paths (cleanupOld/cleanupStaleSessions/emitDeletedSessions). - upsert() checks the tombstone first and drops (no-ops) any write for a tombstoned id without touching SQLite or emitting ai_session:updated, fixing the root cause once in the shared store rather than per-producer (planning.ts, subtask-breakdown.ts, mission-interview.ts, milestone-slice-interview.ts). - Tombstone entries are pruned lazily on check and piggyback on the existing cleanupStaleSessions() cadence so the in-memory map cannot grow unbounded. - Adds a changeset (patch) documenting the user-facing fix. - Updates docs/architecture.md and docs/storage.md with the new "AI session delete tombstones" behavior. - Adds regression tests covering the tombstone guard in ai-session-store.test.ts and routes-planning.test.ts. Files changed: .changeset/fn-7949-ai-session-delete-tombstone.md | 7 + docs/architecture.md | 2 +- docs/storage.md | 12 +- packages/dashboard/src/__tests__/ai-session-store.test.ts | 145 +++++++++++++++ packages/dashboard/src/__tests__/routes-planning.test.ts | 200 ++++++++++++++++++++- packages/dashboard/src/ai-session-store.ts | 83 +++++++++ 6 files changed, 446 insertions(+), 3 deletions(-) Fusion-Task-Id: FN-7949 Fusion-Task-Lineage: 8e509dae-0cc5-46cd-9c4b-9048cfda56d3 Co-authored-by: Fusion (runfusion.ai) --- .../fn-7949-ai-session-delete-tombstone.md | 7 + docs/architecture.md | 2 +- docs/storage.md | 12 +- .../src/__tests__/ai-session-store.test.ts | 145 +++++++++++++ .../src/__tests__/routes-planning.test.ts | 200 +++++++++++++++++- packages/dashboard/src/ai-session-store.ts | 83 ++++++++ 6 files changed, 446 insertions(+), 3 deletions(-) create mode 100644 .changeset/fn-7949-ai-session-delete-tombstone.md diff --git a/.changeset/fn-7949-ai-session-delete-tombstone.md b/.changeset/fn-7949-ai-session-delete-tombstone.md new file mode 100644 index 0000000000..4ccf1b80fd --- /dev/null +++ b/.changeset/fn-7949-ai-session-delete-tombstone.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Fix a deleted Planning Mode session silently reappearing after an in-flight generation finishes. +category: fix +dev: AiSessionStore now records a bounded-TTL delete tombstone (10 min) in `delete()`/`deleteByIdAndType()`/bulk cleanup paths; `upsert()` drops writes for tombstoned ids without emitting `ai_session:updated`. Fixes the root cause in the shared store rather than in `planning.ts`/`subtask-breakdown.ts`/`mission-interview.ts`/`milestone-slice-interview.ts`, so all AiSessionType producers are protected. diff --git a/docs/architecture.md b/docs/architecture.md index f33387245e..0123227428 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -176,7 +176,7 @@ Concrete references: - JSON helpers: `toJson`, `toJsonNullable`, `fromJson` - Core schema tables include: `tasks`, `config`, `workflow_steps`, `activityLog`, `archivedTasks`, `automations`, `agents`, `agentHeartbeats`, approval tables (`approval_requests`, `approval_request_audit_events`), `task_documents`, `task_document_revisions`, mission hierarchy tables (`missions`, `milestones`, `slices`, `mission_features`, `mission_events`), goals table (`goals`), plugin/routine tables (`plugins`, `routines`), roadmap tables (`roadmaps`, `roadmap_milestones`, `roadmap_features`), insight tables (`project_insights`, `project_insight_runs`), research tables (`research_runs`, `research_exports`, `research_run_events`), eval tables (`eval_runs`, `eval_task_results`, `eval_run_events`), todo tables (`todo_lists`, `todo_items`), `__meta` - Migration-created tables include: `ai_sessions`, `messages`, `agentRatings`, `chat_sessions`, `chat_messages`, `runAuditEvents`, `mission_contract_assertions`, `mission_feature_assertions`, `mission_validator_runs`, `mission_validator_failures`, `mission_fix_feature_lineage` - - `ai_sessions.status` lifecycle includes `draft` (pre-start planning session), then `generating`, `awaiting_input`, terminal `complete` / `error` + - `ai_sessions.status` lifecycle includes `draft` (pre-start planning session), then `generating`, `awaiting_input`, terminal `complete` / `error`; deletion is final within a bounded tombstone window that blocks a straggling post-delete write from resurrecting the row (FN-7949) — see `docs/storage.md` "AI session delete tombstones" - **Roadmap feature ownership**: roadmap contracts, ordering/handoff helpers, persistence, routes, and dashboard UI live in `plugins/fusion-plugin-roadmap` (package `@fusion-plugin-examples/roadmap`, plugin id `fusion-plugin-roadmap`) rather than dashboard/core ownership. - **CentralCore**: `packages/core/src/central-core.ts` - Global project registry, health, central activity feed, global concurrency diff --git a/docs/storage.md b/docs/storage.md index 8a245f5ca4..764c910a11 100644 --- a/docs/storage.md +++ b/docs/storage.md @@ -470,7 +470,7 @@ The `tasks.cumulativeActiveMs` and `tasks.executionCompletedAt` columns are the | `project_insight_run_events` | Append-only per-run lifecycle trail (`seq`, `type`, `message`, optional `status`/`classification`/`metadata`) used by cancel/retry/timeout auditing and API inspection. | | `todo_lists` | Project-scoped todo list metadata (`projectId`, title, created/updated timestamps). | | `todo_items` | Todo list items (`listId` FK) with completion state, completion timestamp, and deterministic `sortOrder`. | -| `ai_sessions` *(migration-created)* | Persisted AI interactive sessions (planning/interview/subtask) with status and conversation history. | +| `ai_sessions` *(migration-created)* | Persisted AI interactive sessions (planning/interview/subtask) with status and conversation history. Deletion is final within a bounded tombstone window (FN-7949) — see below. | | `messages` *(migration-created)* | Inter-agent/user message mailbox storage. | | `agentRatings` *(migration-created)* | Agent performance ratings (1-5), optional reviewer metadata, and run/task attribution. | | `chat_sessions` *(migration-created)* | Chat session metadata (agent/project/model/status/title timestamps). | @@ -493,6 +493,16 @@ The `tasks.cumulativeActiveMs` and `tasks.executionCompletedAt` columns are the | `eval_task_results` | Per-task eval outcomes linked to runs (`runId` FK cascade), including durable task snapshots and structured score payloads. `categoryScores[]` stores canonical per-category fields (`category`, `deterministicScore`, `aiScore`, `finalScore`, `weight`, `band`, `rationale`, `evidence[]`), plus `overallScore` derived from category finals. Also stores deterministic/AI signal payloads, summary rationale, structured follow-up suggestions (`suggestionId`, `dedupeKey`, recommendation, lifecycle state, suppression fields, optional `createdTaskId` linkage), and a bounded `TaskEvaluationEvidenceBundle` (fixed source-order groups, capped entry counts, max 500-char excerpts with truncation marker) embedded in result metadata for backward-compatible persistence. | | `eval_run_events` | Append-only eval run event trail (`runId` FK cascade, ordered by `seq`) for orchestration/debug auditing and downstream API/UI drill-down. | +### AI session delete tombstones (FN-7949) + +/* +FNXC:AiSessionStore 2026-07-13-00:00: Deleting a Planning Mode session while its background generation was still in flight let the session silently reappear moments later. Root cause: `runGenerationWithTimeout`'s `Promise.race` (in `planning.ts`, and the equivalent wrappers in `subtask-breakdown.ts`/`mission-interview.ts`/`milestone-slice-interview.ts`) only stops the *caller* from awaiting the in-flight `session.agent.session.prompt()` call — it does not cancel it. A straggling `persistSession(...)`-style `upsert()` call landing after the row was deleted would silently re-INSERT it and re-broadcast `ai_session:updated`. +*/ + +`AiSessionStore.delete()` / `deleteByIdAndType()` / the bulk `cleanupOld()` / `cleanupStaleSessions()` paths now record a delete tombstone (`id -> deletion timestamp`) alongside removing the row. `AiSessionStore.upsert()` checks that tombstone first: a write for an id deleted within the last `DELETE_TOMBSTONE_TTL_MS` (10 minutes — generously longer than any realistic straggling generation write) is dropped without touching SQLite and without emitting `ai_session:updated`. This closes the resurrection race for every `AiSessionType` producer that shares the store (`planning`, `subtask`, `mission_interview`, `milestone_interview`, `slice_interview`), not just the originally reported Planning Mode case. + +A normal delete with no in-flight generation, and a genuinely new session that reuses a brand-new distinct id, are both unaffected — the guard only applies to writes for the *exact* id that was just deleted. Tombstone entries are pruned lazily (on tombstone check) and piggyback pruning on the existing `cleanupStaleSessions()` cadence, so the in-memory tombstone map cannot grow unbounded on a long-running server. See `packages/dashboard/src/ai-session-store.ts` (`upsert()`, `isTombstoned()`, `pruneExpiredTombstones()`). + ### Central SQLite Tables Inventory (`packages/core/src/central-db.ts`) | Table | Purpose | diff --git a/packages/dashboard/src/__tests__/ai-session-store.test.ts b/packages/dashboard/src/__tests__/ai-session-store.test.ts index 01444dfb4d..06787d7356 100644 --- a/packages/dashboard/src/__tests__/ai-session-store.test.ts +++ b/packages/dashboard/src/__tests__/ai-session-store.test.ts @@ -6,6 +6,7 @@ import { join } from "node:path"; import { Database } from "@fusion/core"; import { AiSessionStore, + DELETE_TOMBSTONE_TTL_MS, SESSION_CLEANUP_DEFAULT_MAX_AGE_MS, type AiSessionRow, type AiSessionStatus, @@ -581,4 +582,148 @@ describe("AiSessionStore", () => { updatedAt: expect.any(String), }); }); + + // ── FN-7949: delete tombstone guard ──────────────────────────────────── + + describe("delete tombstone guard (FN-7949)", () => { + it("upsert() does not resurrect a session id deleted via delete()", () => { + const row = makeRow("S-tomb", "awaiting_input"); + store.upsert(row); + expect(store.get("S-tomb")).not.toBeNull(); + + store.delete("S-tomb"); + expect(store.get("S-tomb")).toBeNull(); + + // Simulate a straggling persistSession() write landing after delete — + // this is the exact FN-7949 race: an abandoned generation resolves and + // calls upsert() with the same id after the user already deleted it. + store.upsert(makeRow("S-tomb", "complete")); + + expect(store.get("S-tomb")).toBeNull(); + }); + + it("upsert() does not emit ai_session:updated for a tombstoned id", () => { + const row = makeRow("S-tomb-evt", "awaiting_input"); + store.upsert(row); + store.delete("S-tomb-evt"); + + const onUpdated = vi.fn(); + store.on("ai_session:updated", onUpdated); + + store.upsert(makeRow("S-tomb-evt", "complete")); + + expect(onUpdated).not.toHaveBeenCalled(); + }); + + it("upsert() logs a diagnostics warning when dropping a tombstoned write", () => { + const diagnostics = captureDiagnostics(); + store.upsert(makeRow("S-tomb-log", "awaiting_input")); + store.delete("S-tomb-log"); + + store.upsert(makeRow("S-tomb-log", "complete")); + + expect(diagnostics).toContainEqual( + expect.objectContaining({ + level: "warn", + scope: "ai-session-store", + message: "Dropped upsert for tombstoned (deleted) session", + context: expect.objectContaining({ + sessionId: "S-tomb-log", + operation: "upsert-tombstoned", + }), + }), + ); + }); + + it("delete-then-brand-new-session-with-a-different-id is entirely unaffected", () => { + store.upsert(makeRow("S-old", "complete")); + store.delete("S-old"); + + store.upsert(makeRow("S-new", "awaiting_input")); + + expect(store.get("S-old")).toBeNull(); + expect(store.get("S-new")).not.toBeNull(); + expect(store.get("S-new")?.status).toBe("awaiting_input"); + }); + + it("double-delete of the same id is idempotent and does not throw", () => { + store.upsert(makeRow("S-double", "complete")); + + expect(() => store.delete("S-double")).not.toThrow(); + expect(() => store.delete("S-double")).not.toThrow(); + + store.upsert(makeRow("S-double", "complete")); + expect(store.get("S-double")).toBeNull(); + }); + + it("deleteByIdAndType() also tombstones and blocks resurrection", () => { + store.upsert(makeRow("S-typed", "awaiting_input")); + + const removed = store.deleteByIdAndType("S-typed", "planning"); + expect(removed).toBe(true); + expect(store.get("S-typed")).toBeNull(); + + store.upsert(makeRow("S-typed", "complete")); + expect(store.get("S-typed")).toBeNull(); + }); + + it("deleteByIdAndType() double-delete is idempotent (second call returns false, no throw)", () => { + store.upsert(makeRow("S-typed-2", "awaiting_input")); + + expect(store.deleteByIdAndType("S-typed-2", "planning")).toBe(true); + expect(() => store.deleteByIdAndType("S-typed-2", "planning")).not.toThrow(); + expect(store.deleteByIdAndType("S-typed-2", "planning")).toBe(false); + }); + + it("tombstone expires after DELETE_TOMBSTONE_TTL_MS, allowing a later upsert for the same id", () => { + vi.useFakeTimers(); + try { + store.upsert(makeRow("S-ttl", "awaiting_input")); + store.delete("S-ttl"); + expect(store.get("S-ttl")).toBeNull(); + + // Still within the TTL window — dropped. + store.upsert(makeRow("S-ttl", "complete")); + expect(store.get("S-ttl")).toBeNull(); + + // Advance past the TTL window — the tombstone must expire so a + // legitimate (if practically unexpected) id-reuse upsert lands. + vi.advanceTimersByTime(DELETE_TOMBSTONE_TTL_MS + 1_000); + + store.upsert(makeRow("S-ttl", "complete")); + expect(store.get("S-ttl")).not.toBeNull(); + expect(store.get("S-ttl")?.status).toBe("complete"); + } finally { + vi.useRealTimers(); + } + }); + + it("cleanupOld()-deleted ids are protected by the same tombstone guard", () => { + seedSession({ id: "S-bulk-old", status: "complete", ageMs: 2 * 60 * 60 * 1000 }); + + const removed = store.cleanupOld(60 * 60 * 1000); + expect(removed).toBe(1); + expect(store.get("S-bulk-old")).toBeNull(); + + // A straggling write for the bulk-cleaned-up id must still be dropped. + store.upsert(makeRow("S-bulk-old", "complete")); + expect(store.get("S-bulk-old")).toBeNull(); + }); + + it("cleanupStaleSessions()-deleted ids (terminal and orphaned) are protected by the tombstone guard", () => { + seedSession({ id: "S-bulk-terminal", status: "complete", ageMs: 8 * 24 * 60 * 60 * 1000 }); + seedSession({ id: "S-bulk-orphaned", status: "awaiting_input", ageMs: 8 * 24 * 60 * 60 * 1000 }); + + const summary = store.cleanupStaleSessions(); + expect(summary.totalDeleted).toBe(2); + expect(store.get("S-bulk-terminal")).toBeNull(); + expect(store.get("S-bulk-orphaned")).toBeNull(); + + store.upsert(makeRow("S-bulk-terminal", "complete")); + store.upsert(makeRow("S-bulk-orphaned", "awaiting_input")); + + expect(store.get("S-bulk-terminal")).toBeNull(); + expect(store.get("S-bulk-orphaned")).toBeNull(); + }); + }); }); diff --git a/packages/dashboard/src/__tests__/routes-planning.test.ts b/packages/dashboard/src/__tests__/routes-planning.test.ts index 4dd53cf6ea..0b3292f920 100644 --- a/packages/dashboard/src/__tests__/routes-planning.test.ts +++ b/packages/dashboard/src/__tests__/routes-planning.test.ts @@ -5,6 +5,7 @@ import express from "express"; import http from "node:http"; import { EventEmitter } from "node:events"; import { existsSync, mkdtempSync, mkdirSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { execFileSync } from "node:child_process"; @@ -38,7 +39,7 @@ import { import * as planningModule from "../planning.js"; import { __resetSubtaskBreakdownState, subtaskStreamManager } from "../subtask-breakdown.js"; import * as subtaskBreakdownModule from "../subtask-breakdown.js"; -import { SESSION_CLEANUP_DEFAULT_MAX_AGE_MS, type AiSessionRow } from "../ai-session-store.js"; +import { AiSessionStore, SESSION_CLEANUP_DEFAULT_MAX_AGE_MS, type AiSessionRow } from "../ai-session-store.js"; import * as usageModule from "../usage.js"; import * as claudeCliProbeModule from "../claude-cli-probe.js"; import * as droidCliProbeModule from "../droid-cli-probe.js"; @@ -4007,6 +4008,203 @@ describe("DELETE /api/ai-sessions/cleanup", () => { }); }); +// ── FN-7949: delete-mid-generation resurrection race ────────────────────── +// Reproduces the exact reported bug: deleting a Planning Mode session while +// its background generation is still in flight must not let a straggling +// write resurrect it. Uses a REAL AiSessionStore (backed by a temp SQLite +// db) wired the same way server.ts wires it — via setAiSessionStore() for +// planning.ts's internal persistSession() calls AND via the routes.ts +// aiSessionStore option for the DELETE endpoint — so the tombstone guard +// added to AiSessionStore.upsert() is exercised end-to-end, not mocked out. +describe("DELETE /api/ai-sessions/:id mid-generation (FN-7949)", () => { + let store: TaskStore; + let db: InstanceType; + let realAiSessionStore: AiSessionStore; + let tmpRoot: string; + + function buildApp() { + const app = express(); + app.use(express.json()); + app.use("/api", createApiRoutes(store, { aiSessionStore: realAiSessionStore as any })); + return app; + } + + /** + * Deferred-prompt mock agent: the FIRST prompt() call (triggered by + * POST /api/planning/start) resolves immediately with a question, so the + * session reaches `awaiting_input` normally. The SECOND prompt() call + * (triggered by POST /api/planning/respond) does not resolve until the + * test explicitly calls `resolveSecondPrompt()` — modeling the abandoned + * `session.agent.session.prompt()` call that `runGenerationWithTimeout`'s + * `Promise.race` only stops *awaiting*, not actually cancels. + */ + function setupDeferredRespondAgent() { + const messages: Array<{ role: string; content: string }> = []; + let callIndex = 0; + let resolveSecondPrompt: (() => void) | undefined; + const secondPromptStarted = new Promise((resolveStarted) => { + const mockAgent = { + session: { + state: { messages }, + prompt: vi.fn(async (msg: string) => { + messages.push({ role: "user", content: msg }); + if (callIndex === 0) { + callIndex++; + messages.push({ + role: "assistant", + content: JSON.stringify({ + type: "question", + data: { id: "q-scope", type: "text", question: "What is the scope?" }, + }), + }); + return; + } + callIndex++; + await new Promise((resolve) => { + resolveSecondPrompt = resolve; + resolveStarted(); + }); + messages.push({ + role: "assistant", + content: JSON.stringify({ + type: "complete", + data: { title: "Late-landing plan", description: "Should never be seen", suggestedSize: "S" }, + }), + }); + }), + dispose: vi.fn(), + }, + }; + __setCreateFnAgent(async () => mockAgent); + }); + + return { + secondPromptStarted, + resolveSecondPrompt: () => resolveSecondPrompt?.(), + }; + } + + beforeEach(() => { + store = createMockStore(); + __resetPlanningState(); + tmpRoot = mkdtempSync(join(tmpdir(), "kb-fn7949-ai-session-")); + db = new Database(join(tmpRoot, ".fusion")); + db.init(); + realAiSessionStore = new AiSessionStore(db as any); + setAiSessionStore(realAiSessionStore); + }); + + afterEach(async () => { + __setCreateFnAgent(undefined as any); + __resetPlanningState(); + realAiSessionStore.stopScheduledCleanup(); + try { + db.close(); + } catch { + // no-op + } + await rm(tmpRoot, { recursive: true, force: true }); + }); + + it("does not resurrect a session deleted while a generation is still in flight", async () => { + const { secondPromptStarted, resolveSecondPrompt } = setupDeferredRespondAgent(); + + const startRes = await REQUEST( + buildApp(), + "POST", + "/api/planning/start", + JSON.stringify({ initialPlan: "Deleted mid-generation planning session" }), + { "Content-Type": "application/json" }, + ); + expect(startRes.status).toBe(201); + const sessionId = startRes.body.sessionId as string; + expect(realAiSessionStore.get(sessionId)).not.toBeNull(); + expect(realAiSessionStore.get(sessionId)?.status).toBe("awaiting_input"); + + // Fire the respond call WITHOUT awaiting it — its underlying prompt() + // call is deferred and will not resolve until we explicitly release it + // below, simulating the straggling in-flight generation from FN-7949. + const respondPromise = REQUEST( + buildApp(), + "POST", + "/api/planning/respond", + JSON.stringify({ sessionId, responses: { "q-scope": "medium" } }), + { "Content-Type": "application/json" }, + ); + + // Wait for the second prompt() call to actually start (i.e. respond's + // synchronous persistSession(session, "generating") has already run and + // execution is now parked awaiting the deferred prompt). + await secondPromptStarted; + + // Delete the session while that generation is still in flight — the + // exact FN-7949 reproduction step. + const deleteRes = await REQUEST(buildApp(), "DELETE", `/api/ai-sessions/${sessionId}`); + expect(deleteRes.status).toBe(200); + expect(deleteRes.body).toEqual({ ok: true }); + expect(realAiSessionStore.get(sessionId)).toBeNull(); + + const updatedEventIds: string[] = []; + realAiSessionStore.on("ai_session:updated", (summary) => updatedEventIds.push(summary.id)); + + // Let the abandoned prompt() call resolve — the straggling write lands + // after the session was already deleted. + resolveSecondPrompt(); + await respondPromise.catch(() => { + // The request may resolve or reject depending on how far the aborted + // in-memory session state got; either is fine — what matters is the + // store-level assertion below. + }); + + // The deleted session must not have been resurrected, and no + // ai_session:updated event should have fired for it. + expect(realAiSessionStore.get(sessionId)).toBeNull(); + expect(updatedEventIds).not.toContain(sessionId); + }); + + it("a normal delete with no in-flight generation continues to work exactly as before", async () => { + setupPlanningMockAgentForFn7949(); + + const startRes = await REQUEST( + buildApp(), + "POST", + "/api/planning/start", + JSON.stringify({ initialPlan: "Ordinary planning session, deleted cleanly" }), + { "Content-Type": "application/json" }, + ); + expect(startRes.status).toBe(201); + const sessionId = startRes.body.sessionId as string; + expect(realAiSessionStore.get(sessionId)).not.toBeNull(); + + const deleteRes = await REQUEST(buildApp(), "DELETE", `/api/ai-sessions/${sessionId}`); + + expect(deleteRes.status).toBe(200); + expect(deleteRes.body).toEqual({ ok: true }); + expect(realAiSessionStore.get(sessionId)).toBeNull(); + }); + + function setupPlanningMockAgentForFn7949() { + const messages: Array<{ role: string; content: string }> = []; + const mockAgent = { + session: { + state: { messages }, + prompt: vi.fn(async (msg: string) => { + messages.push({ role: "user", content: msg }); + messages.push({ + role: "assistant", + content: JSON.stringify({ + type: "question", + data: { id: "q-scope", type: "text", question: "What is the scope?" }, + }), + }); + }), + dispose: vi.fn(), + }, + }; + __setCreateFnAgent(async () => mockAgent); + } +}); + describe("POST /api/ai-sessions/:id/ping", () => { let store: TaskStore; diff --git a/packages/dashboard/src/ai-session-store.ts b/packages/dashboard/src/ai-session-store.ts index 6fdf659c78..28413200bb 100644 --- a/packages/dashboard/src/ai-session-store.ts +++ b/packages/dashboard/src/ai-session-store.ts @@ -81,6 +81,32 @@ export const SESSION_CLEANUP_DEFAULT_MAX_AGE_MS = 7 * 24 * 60 * 60 * 1000; /** Default scheduled interval for stale session cleanup runs (6 hours). */ export const SESSION_CLEANUP_INTERVAL_MS = 6 * 60 * 60 * 1000; +/** + * FNXC:AiSessionStore 2026-07-13-00:00: + * FN-7949 — deleting a Planning Mode session while its background generation + * is still in flight let the session silently reappear. Root cause: + * `runGenerationWithTimeout` (planning.ts) and the equivalent wrappers in + * subtask-breakdown.ts/mission-interview.ts/milestone-slice-interview.ts use + * `Promise.race([operation(...), abortPromise])` to abort generation — that + * only stops the *caller* from awaiting `operation`, it does NOT cancel the + * underlying `session.agent.session.prompt()` call. If the session is deleted + * while that promise is still pending, the abandoned call later resolves and + * calls `persistSession(...)` -> `upsert()`, which used to unconditionally + * re-INSERT the row and re-emit `ai_session:updated`, resurrecting a session + * the user explicitly deleted. + * + * Fix: `AiSessionStore` remembers deleted ids in a bounded-TTL tombstone map. + * `upsert()` drops (no-ops) any write for an id tombstoned within the TTL + * window, so a straggling write can never resurrect a deleted session. This + * lives here — the single shared store — rather than being duplicated in each + * producer, so the invariant holds for every AiSessionType (planning, subtask, + * mission_interview, milestone_interview, slice_interview) without forking + * the fix per-producer. 10 minutes is generously longer than any realistic + * straggling generation write (session ids are UUIDs, never legitimately + * reused), so id-reuse racing past the TTL is not an expected production path. + */ +export const DELETE_TOMBSTONE_TTL_MS = 10 * 60 * 1000; + export interface AiSessionCleanupSummary { terminalDeleted: number; orphanedDeleted: number; @@ -96,6 +122,12 @@ export class AiSessionStore extends EventEmitter { private thinkingTimers = new Map>(); /** Interval used for periodic stale-session cleanup. */ private cleanupTimer: ReturnType | undefined; + /** + * FN-7949 delete tombstones: id -> deletion timestamp (ms since epoch). + * Consulted by `upsert()` to drop straggling writes for ids deleted within + * `DELETE_TOMBSTONE_TTL_MS`. See the FNXC:AiSessionStore comment above. + */ + private deletedIds = new Map(); constructor(private db: Database) { super(); @@ -108,6 +140,18 @@ export class AiSessionStore extends EventEmitter { * Emits `ai_session:updated` after writing. */ upsert(session: AiSessionRow): void { + // FNXC:AiSessionStore 2026-07-13-00:00: FN-7949 tombstone guard — drop any + // upsert for an id that was deleted within the TTL window. This is what + // actually prevents a straggling post-delete generation write (see the + // constant-level FNXC comment) from resurrecting a deleted session. + if (this.isTombstoned(session.id)) { + diagnostics.warn("Dropped upsert for tombstoned (deleted) session", { + sessionId: session.id, + operation: "upsert-tombstoned", + }); + return; + } + const now = new Date().toISOString(); // FNXC:PlanningMode 2026-07-02-00:00: Planning checkpoints persist pending summaries inside inputPayload, so every session upsert must refresh inputPayload on existing rows instead of treating it as create-only draft metadata. const thinking = trimThinking(session.thinkingOutput); @@ -623,6 +667,7 @@ export class AiSessionStore extends EventEmitter { delete(id: string): void { this.clearThinkingTimer(id); this.db.prepare("DELETE FROM ai_sessions WHERE id = ?").run(id); + this.deletedIds.set(id, Date.now()); this.emit("ai_session:deleted", id); } @@ -642,6 +687,7 @@ export class AiSessionStore extends EventEmitter { const removed = Number(result.changes ?? 0) > 0; if (removed) { + this.deletedIds.set(id, Date.now()); this.emit("ai_session:deleted", id); } return removed; @@ -721,6 +767,9 @@ export class AiSessionStore extends EventEmitter { * - Orphaned active sessions (`generating`, `awaiting_input`) are deleted directly. */ cleanupStaleSessions(maxAgeMs = SESSION_CLEANUP_DEFAULT_MAX_AGE_MS): AiSessionCleanupSummary { + // FN-7949: piggyback tombstone-map pruning on the existing cleanup cadence. + this.pruneExpiredTombstones(); + const terminalDeleted = this.cleanupOld(maxAgeMs); const cutoff = new Date(Date.now() - maxAgeMs).toISOString(); @@ -796,12 +845,46 @@ export class AiSessionStore extends EventEmitter { // ── Internal ──────────────────────────────────────────────────────── private emitDeletedSessions(rows: Array<{ id: string }>): void { + const now = Date.now(); for (const { id } of rows) { this.clearThinkingTimer(id); + this.deletedIds.set(id, now); this.emit("ai_session:deleted", id); } } + /** + * Returns true when `id` was deleted within the tombstone TTL window. + * Lazily prunes the specific entry when it has expired so the map does not + * hold expired entries indefinitely for ids that are never re-upserted. + */ + private isTombstoned(id: string): boolean { + const deletedAt = this.deletedIds.get(id); + if (deletedAt === undefined) return false; + if (Date.now() - deletedAt < DELETE_TOMBSTONE_TTL_MS) return true; + this.deletedIds.delete(id); + return false; + } + + /** + * Prune expired tombstone entries. Piggybacks on the existing scheduled + * cleanup cadence (`startScheduledCleanup`/`cleanupStaleSessions`) so the + * `deletedIds` map cannot grow unbounded over a long-running server + * process. Safe to call at any time; also invoked lazily via + * `isTombstoned` for individually-checked ids. + */ + private pruneExpiredTombstones(): number { + const now = Date.now(); + let pruned = 0; + for (const [id, deletedAt] of this.deletedIds) { + if (now - deletedAt >= DELETE_TOMBSTONE_TTL_MS) { + this.deletedIds.delete(id); + pruned++; + } + } + return pruned; + } + private writeThinking(sessionId: string, thinkingOutput: string): void { const now = new Date().toISOString(); this.db