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) <noreply@runfusion.ai>
This commit is contained in:
gsxdsm
7
.changeset/fn-7949-ai-session-delete-tombstone.md
Normal file
7
.changeset/fn-7949-ai-session-delete-tombstone.md
Normal file
@@ -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.
|
||||
@@ -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
|
||||
|
||||
@@ -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 |
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<typeof Database>;
|
||||
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<void>((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<void>((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;
|
||||
|
||||
|
||||
@@ -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<AiSessionStoreEvents> {
|
||||
private thinkingTimers = new Map<string, ReturnType<typeof setTimeout>>();
|
||||
/** Interval used for periodic stale-session cleanup. */
|
||||
private cleanupTimer: ReturnType<typeof setInterval> | 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<string, number>();
|
||||
|
||||
constructor(private db: Database) {
|
||||
super();
|
||||
@@ -108,6 +140,18 @@ export class AiSessionStore extends EventEmitter<AiSessionStoreEvents> {
|
||||
* 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<AiSessionStoreEvents> {
|
||||
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<AiSessionStoreEvents> {
|
||||
|
||||
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<AiSessionStoreEvents> {
|
||||
* - 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<AiSessionStoreEvents> {
|
||||
// ── 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
|
||||
|
||||
Reference in New Issue
Block a user