Skip to content

[Bug]: Stale content DB cleanup deletes live-but-idle sessions (WAL mtime only, no liveness check) #1024

Description

@EvolveAegis

[Bug]: Stale content DB cleanup deletes live-but-idle sessions (WAL mtime only, no liveness check)

Description

cleanupStaleContentDBs in src/store.ts:222 is the startup cleanup that runs on every new session via server.ts:729. When it finds a per-project *.db with a non-empty -wal sidecar, it marks the DB for deletion based purely on WAL mtime:

// src/store.ts:244
if (walStat.size > 0 && (Date.now() - walStat.mtimeMs) > 3600_000) {
  shouldClean = true;
}

One hour of WAL idle is treated as "owner is dead" and the DB + WAL + SHM are unlinked, with no check that the owning process is actually alive. A session that's simply been idle for more than an hour (long think, slow tool, user stepped away) loses its content index.

The liveness probe this needs already exists in the same file. isProcessAlive at src/store.ts:207 wraps process.kill(pid, 0), and the sibling cleanupStaleDBs at src/store.ts:179 uses exactly that probe before deleting PID-based DBs (line 190: process.kill(pid, 0), unlink only on ESRCH). The content-DB cleanup just never calls it.

To Reproduce

git clone https://github.com/mksglu/context-mode
cd context-mode && npm install
git checkout 252e74b7  # verified on main at this commit

cat > tests/cm11-poc.test.ts <<'EOF'
import { describe, it, expect } from "vitest";
import { mkdtempSync, writeFileSync, existsSync, utimesSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { cleanupStaleContentDBs } from "../src/store";

describe("CM-11: WAL mtime staleness deletes a DB with no liveness check", () => {
  it("non-empty WAL backdated 2h is deleted although the owner could be alive", () => {
    const dir = mkdtempSync(join(tmpdir(), "cm11-"));
    const dbPath = join(dir, "proj-x.db");
    const walPath = dbPath + "-wal";
    writeFileSync(dbPath, Buffer.alloc(4096, 1));
    writeFileSync(walPath, Buffer.alloc(8192, 2));           // synthetic non-empty WAL
    const twoHoursAgo = new Date(Date.now() - 2 * 3600_000);
    utimesSync(dbPath, twoHoursAgo, twoHoursAgo);            // age < maxAgeDays(14) -> NOT stale by age
    utimesSync(walPath, twoHoursAgo, twoHoursAgo);           // but WAL idle > 1h

    const cleaned = cleanupStaleContentDBs(dir, 14);
    console.log(`cleaned=${cleaned}; db exists after: ${existsSync(dbPath)}`);
    expect(cleaned).toBe(1);
    expect(existsSync(dbPath)).toBe(false);                  // deleted with no PID check
  });

  it("fresh WAL mtime is NOT deleted (contrast)", () => {
    const dir = mkdtempSync(join(tmpdir(), "cm11b-"));
    const dbPath = join(dir, "proj-y.db");
    const walPath = dbPath + "-wal";
    writeFileSync(dbPath, Buffer.alloc(4096, 1));
    writeFileSync(walPath, Buffer.alloc(8192, 2));           // synthetic
    const cleaned = cleanupStaleContentDBs(dir, 14);
    console.log(`cleaned=${cleaned}; db exists: ${existsSync(dbPath)}`);
    expect(cleaned).toBe(0);
    expect(existsSync(dbPath)).toBe(true);
  });
});
EOF

npx vitest run tests/cm11-poc.test.ts --reporter=verbose
# cleaned=1; db exists after: false   <- a live-but-idle owner is indistinguishable, so it gets deleted
# cleaned=0; db exists: true           <- only mtime gates the delete; no PID is ever probed

Expected Behavior

Before unlinking a DB with a non-empty WAL, the cleanup should confirm the owning process is actually dead. The block comment at src/store.ts:234 already says it intends to extract a PID from the WAL header — that PID should be passed to isProcessAlive, and the DB kept when the owner is alive (or when no PID can be determined). A live session must never have its content index deleted out from under it.

Actual Behavior

Deletion is driven by WAL mtime alone. Any per-project content DB whose WAL hasn't been touched in the last hour is removed on the next session's startup (server.ts:729), regardless of whether the owning session is alive but idle.

Additional Context

  • Reachable on every new session: server.ts:726-734 calls cleanupStaleContentDBs(contentDir, 14) at startup, and again on the legacy shared dir with maxAgeDays=0.
  • The liveness pattern is already in-repo: cleanupStaleDBs (src/store.ts:179-201) does process.kill(pid, 0) and only unlinks on ESRCH; isProcessAlive (src/store.ts:207) is defined but never called from the content-DB path.
  • Real-world trigger: two sessions on the same project — session A idle for >1h with an unflushed WAL, session B starts up and A's content DB is deleted. The synthetic PoC above exercises the deletion branch in isolation.
  • Verified against 252e74b7 on main.
  • Happy to open a PR against next if that's useful.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions