fix(storage): honour MAGIC_CONTEXT_TEST_DATA_DIR in the shared storage resolver

packages/plugin/src/features/magic-context/storage-db.ts

@@ -5,13 +5,11 @@ import {
     type Dirent,
     existsSync,
     mkdirSync,
-    mkdtempSync,
     readdirSync,
     readFileSync,
     statSync,
     unlinkSync,
 } from "node:fs";
-import { tmpdir } from "node:os";
 import { basename, dirname, join } from "node:path";
 import { bootQuietRemainingMs, scheduleAfterBootQuiet } from "../../plugin/boot-quiet";
 import {
@@ -158,79 +156,14 @@ export function resolveDatabasePath(dbPathOverride?: string): { dbDir: string; d
     if (dbPathOverride) {
         return { dbDir: dirname(dbPathOverride), dbPath: dbPathOverride };
     }
-    // Test-isolation guard. Under the test runner the preload
-    // (bunfig.toml `[test] preload`) sets MAGIC_CONTEXT_TEST_DATA_DIR to a
-    // throwaway temp dir AND XDG_DATA_HOME to the same dir. Tests that manage
-    // their OWN XDG_DATA_HOME (per-test temp dirs) keep working — we honor XDG
-    // below via getMagicContextStorageDir(). The guard fires ONLY when
-    // XDG_DATA_HOME is UNSET: that is the dangerous window, because
-    // getMagicContextStorageDir() would otherwise fall back to the REAL
-    // ~/.local/share and a bare openDatabase() would run migrations on the
-    // user's production DB. Some tests delete XDG_DATA_HOME to exercise
-    // path-fallback behavior (2026-06-01 incident: a dormant test migrated the
-    // live DB to v26 and fail-closed every running v25 binary); in that window
-    // we resolve into the dedicated test dir instead of the real path. No test
-    // mutates MAGIC_CONTEXT_TEST_DATA_DIR, so the guard cannot be defeated. It
-    // is never set in production.
-    const testDataDir = process.env.MAGIC_CONTEXT_TEST_DATA_DIR;
-    if (testDataDir && !process.env.XDG_DATA_HOME) {
-        const dbDir = join(testDataDir, "cortexkit", "magic-context");
-        return { dbDir, dbPath: join(dbDir, "context.db") };
-    }
-    // CWD-INDEPENDENT TEST BACKSTOP. The MAGIC_CONTEXT_TEST_DATA_DIR / XDG guard
-    // above only fires when the bunfig `[test] preload` ran — which depends on
-    // `bun test`'s CWD having a bunfig with `[test] preload`. A `bun test` from a
-    // dir WITHOUT that wiring (monorepo root, a package missing its bunfig, or a
-    // brand-new package) recursively runs every *.test.ts with NO preload, so a
-    // bare openDatabase() would resolve to the user's REAL shared DB and run
-    // migrations on it. That is exactly how the live DB was migrated to v41 by a
-    // worktree whose LATEST was 41 (a re-run of the 2026-06-01 v26 incident).
-    //
-    // Bun sets NODE_ENV=test for EVERY `bun test` regardless of CWD/bunfig (and
-    // it is never "test" in the plugin runtime — production never sets it). So if
-    // we are under the test runner with neither the test data dir nor an explicit
-    // override, we MUST NOT touch real storage: redirect into a throwaway temp dir
-    // so the live DB is physically unreachable. This makes it structurally
-    // impossible for ANY test, from ANY CWD, to read or migrate production data.
-    // Fire ONLY when XDG_DATA_HOME is unset: that is the dangerous window where
-    // getMagicContextStorageDir() below would otherwise resolve to the REAL
-    // ~/.local/share shared DB. When a test sets its own XDG_DATA_HOME (a
-    // per-test temp dir, e.g. to exercise path fallbacks or share a DB across
-    // helper calls), getMagicContextStorageDir() already points inside that
-    // controlled dir — honor it, do not override.
-    if (process.env.NODE_ENV === "test" && !process.env.XDG_DATA_HOME) {
-        // Memoized per-process so repeated openDatabase() calls in the same
-        // unisolated test resolve to the SAME path (openDatabase caches by path;
-        // a fresh temp dir per call would defeat the cache and hand back
-        // different DB handles).
-        const dbDir = getTestBackstopDbDir();
-        if (!testBackstopWarned) {
-            testBackstopWarned = true;
-            log(
-                "[magic-context] TEST BACKSTOP: NODE_ENV=test with no MAGIC_CONTEXT_TEST_DATA_DIR " +
-                    `— redirecting DB to a throwaway temp dir (${dbDir}) so no test can touch the ` +
-                    "user's real shared database. Wire `[test] preload` in this package's bunfig.toml.",
-            );
-        }
-        return { dbDir, dbPath: join(dbDir, "context.db") };
-    }
+    // Test-isolation guards (MAGIC_CONTEXT_TEST_DATA_DIR + the CWD-independent
+    // NODE_ENV backstop) both live in getMagicContextStorageDir(), so this
+    // resolver and every direct caller of that helper are covered by one
+    // implementation. See its doc comment for the incident history.
     const dbDir = getMagicContextStorageDir();
     return { dbDir, dbPath: join(dbDir, "context.db") };
 }
 
-let testBackstopDbDir: string | null = null;
-let testBackstopWarned = false;
-function getTestBackstopDbDir(): string {
-    if (!testBackstopDbDir) {
-        testBackstopDbDir = join(
-            mkdtempSync(join(tmpdir(), "mc-test-db-backstop-")),
-            "cortexkit",
-            "magic-context",
-        );
-    }
-    return testBackstopDbDir;
-}
-
 export function getDatabasePath(db: Database): string | null {
     return pathByDatabase.get(db) ?? null;
 }

packages/plugin/src/shared/data-path.test.ts

@@ -20,6 +20,8 @@ const savedEnv = {
     XDG_DATA_HOME: process.env.XDG_DATA_HOME,
     LOCALAPPDATA: process.env.LOCALAPPDATA,
     MAGIC_CONTEXT_LOG_PATH: process.env.MAGIC_CONTEXT_LOG_PATH,
+    MAGIC_CONTEXT_TEST_DATA_DIR: process.env.MAGIC_CONTEXT_TEST_DATA_DIR,
+    NODE_ENV: process.env.NODE_ENV,
 };
 
 describe("data-path", () => {
@@ -36,14 +38,14 @@ describe("data-path", () => {
     });
 
     afterEach(() => {
-        if (savedEnv.XDG_CACHE_HOME !== undefined)
-            process.env.XDG_CACHE_HOME = savedEnv.XDG_CACHE_HOME;
-        if (savedEnv.XDG_DATA_HOME !== undefined)
-            process.env.XDG_DATA_HOME = savedEnv.XDG_DATA_HOME;
-        if (savedEnv.LOCALAPPDATA !== undefined) process.env.LOCALAPPDATA = savedEnv.LOCALAPPDATA;
-        if (savedEnv.MAGIC_CONTEXT_LOG_PATH !== undefined)
-            process.env.MAGIC_CONTEXT_LOG_PATH = savedEnv.MAGIC_CONTEXT_LOG_PATH;
-        else delete process.env.MAGIC_CONTEXT_LOG_PATH;
+        // Restore-or-delete every var this suite touches. Several tests lift a
+        // guard (NODE_ENV, MAGIC_CONTEXT_TEST_DATA_DIR) to assert production
+        // shape, so a restore that skips the unset case would leak state into
+        // the next test and make results order-dependent.
+        for (const [key, value] of Object.entries(savedEnv)) {
+            if (value !== undefined) process.env[key] = value;
+            else delete process.env[key];
+        }
     });
 
     test("getCacheDir falls back to <homedir>/.cache when XDG_CACHE_HOME is unset (all platforms)", () => {
@@ -89,8 +91,62 @@ describe("data-path", () => {
         // Cross-harness shared path: both OpenCode and Pi plugins read/write here,
         // unlike the legacy opencode/storage/plugin/magic-context location which
         // was OpenCode-specific. See ARCHITECTURE_DECISIONS memory for rationale.
+        // Production shape, so both test-isolation guards (the preload's data
+        // dir and the NODE_ENV backstop bun sets for every `bun test`) are
+        // lifted for the duration of this assertion.
+        const savedTestDir = process.env.MAGIC_CONTEXT_TEST_DATA_DIR;
+        const savedNodeEnv = process.env.NODE_ENV;
+        delete process.env.MAGIC_CONTEXT_TEST_DATA_DIR;
+        delete process.env.NODE_ENV;
+        try {
+            expect(getMagicContextStorageDir()).toBe(
+                path.join(os.homedir(), ".local", "share", "cortexkit", "magic-context"),
+            );
+        } finally {
+            if (savedTestDir !== undefined) process.env.MAGIC_CONTEXT_TEST_DATA_DIR = savedTestDir;
+            if (savedNodeEnv !== undefined) process.env.NODE_ENV = savedNodeEnv;
+        }
+    });
+
+    test("getMagicContextStorageDir backstops to a temp dir under NODE_ENV=test with no guard set", () => {
+        // CWD-independent backstop: a `bun test` from a dir whose bunfig has no
+        // `[test] preload` runs every suite with neither guard env var set. The
+        // DB resolver used to own this branch, so direct callers of this helper
+        // (the CLI doctors' own PRAGMA integrity_check) still reached the real
+        // shared DB. Memoized, so repeated calls must agree — openDatabase()
+        // caches by path.
+        const savedTestDir = process.env.MAGIC_CONTEXT_TEST_DATA_DIR;
+        delete process.env.MAGIC_CONTEXT_TEST_DATA_DIR;
+        process.env.NODE_ENV = "test";
+        try {
+            const resolved = getMagicContextStorageDir();
+            expect(resolved).not.toContain(path.join(os.homedir(), ".local", "share"));
+            expect(resolved.endsWith(path.join("cortexkit", "magic-context"))).toBe(true);
+            expect(getMagicContextStorageDir()).toBe(resolved);
+        } finally {
+            if (savedTestDir !== undefined) process.env.MAGIC_CONTEXT_TEST_DATA_DIR = savedTestDir;
+        }
+    });
+
+    test("getMagicContextStorageDir honors MAGIC_CONTEXT_TEST_DATA_DIR when XDG_DATA_HOME is unset", () => {
+        // The hole this closes: a test that deletes XDG_DATA_HOME to exercise
+        // path fallbacks used to resolve to the user's REAL shared storage,
+        // because bun caches os.homedir() and a mutated process.env.HOME cannot
+        // move getDataDir(). Callers that build their own context.db path (the
+        // CLI doctors) then ran integrity checks against production data.
+        process.env.MAGIC_CONTEXT_TEST_DATA_DIR = "/tmp/mc-test-isolation";
         expect(getMagicContextStorageDir()).toBe(
-            path.join(os.homedir(), ".local", "share", "cortexkit", "magic-context"),
+            path.join("/tmp/mc-test-isolation", "cortexkit", "magic-context"),
+        );
+    });
+
+    test("getMagicContextStorageDir prefers XDG_DATA_HOME over MAGIC_CONTEXT_TEST_DATA_DIR", () => {
+        // A test managing its own per-test data home is already controlled, and
+        // several suites depend on that dir being honored.
+        process.env.MAGIC_CONTEXT_TEST_DATA_DIR = "/tmp/mc-test-isolation";
+        process.env.XDG_DATA_HOME = "/tmp/custom-data";
+        expect(getMagicContextStorageDir()).toBe(
+            path.join("/tmp/custom-data", "cortexkit", "magic-context"),
         );
     });
 

packages/plugin/src/shared/data-path.ts

@@ -1,4 +1,4 @@
-import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs";
+import { existsSync, mkdirSync, mkdtempSync, readFileSync, writeFileSync } from "node:fs";
 import * as os from "node:os";
 import * as path from "node:path";
 import { getHarness, type HarnessId } from "./harness";
@@ -167,11 +167,76 @@ export function getOpenCodeStorageDir(): string {
  *   - Future cross-harness session migration
  *
  * Layout: <XDG_DATA_HOME>/cortexkit/magic-context/
+ *
+ * TEST-ISOLATION GUARD. `openDatabase()` has been guarded in
+ * `resolveDatabasePath()` since the 2026-06-01 (v26) and 2026-06-19 (v41)
+ * incidents, in which unisolated tests migrated the user's REAL shared DB.
+ * Every OTHER caller bypassed that guard — notably the CLI doctors, which
+ * build `join(getMagicContextStorageDir(), "context.db")` themselves and run
+ * `PRAGMA integrity_check` against it. A test that deletes XDG_DATA_HOME to
+ * exercise path fallbacks cannot restore isolation by setting
+ * `process.env.HOME`: bun caches `os.homedir()` at startup, so `getDataDir()`
+ * still resolves to the real home. MAGIC_CONTEXT_TEST_DATA_DIR — set by
+ * `packages/plugin/test-preload.ts`, and restored by any test that touches it
+ * — is honored here, so the preload's "cannot be defeated" guarantee holds for
+ * every caller, not just `openDatabase()`. It is never set in production.
+ *
+ * CWD-INDEPENDENT BACKSTOP. The preload only runs when `bun test`'s CWD has a
+ * bunfig wiring `[test] preload`. A run from a dir WITHOUT that wiring (a
+ * package missing its bunfig, or a brand-new one) executes every *.test.ts with
+ * NO preload, and this resolver would hand back the REAL shared path — which is
+ * how the live DB reached v41. Bun sets NODE_ENV=test for EVERY `bun test`
+ * regardless of CWD, and production never sets it, so that window redirects to
+ * a memoized throwaway dir instead. It lives here rather than in
+ * `resolveDatabasePath()` so direct callers (the CLI doctors' own
+ * `PRAGMA integrity_check`, announcements, the models.dev cache) are covered
+ * too — they never go through the DB resolver.
+ *
+ * XDG_DATA_HOME still wins over both: a test that manages its own data home is
+ * already controlled, and production has no test dir set at all.
  */
 export function getMagicContextStorageDir(): string {
+    if (!process.env.XDG_DATA_HOME) {
+        const testDataDir = process.env.MAGIC_CONTEXT_TEST_DATA_DIR;
+        if (testDataDir) {
+            return path.join(testDataDir, "cortexkit", "magic-context");
+        }
+        if (process.env.NODE_ENV === "test") {
+            return getTestBackstopStorageDir();
+        }
+    }
     return path.join(getDataDir(), "cortexkit", "magic-context");
 }
 
+let testBackstopStorageDir: string | null = null;
+let testBackstopWarned = false;
+
+/**
+ * Memoized per process so repeated calls in the same unisolated test resolve to
+ * the SAME path — `openDatabase()` caches by path, and a fresh temp dir per call
+ * would defeat that cache and hand back different DB handles.
+ */
+function getTestBackstopStorageDir(): string {
+    if (!testBackstopStorageDir) {
+        testBackstopStorageDir = path.join(
+            mkdtempSync(path.join(os.tmpdir(), "mc-test-db-backstop-")),
+            "cortexkit",
+            "magic-context",
+        );
+    }
+    if (!testBackstopWarned) {
+        testBackstopWarned = true;
+        // Deliberately console, not the logger: logger.ts imports this module.
+        console.warn(
+            "[magic-context] TEST BACKSTOP: NODE_ENV=test with no MAGIC_CONTEXT_TEST_DATA_DIR " +
+                `— redirecting storage to a throwaway temp dir (${testBackstopStorageDir}) so no ` +
+                "test can touch the user's real shared database. Wire `[test] preload` in this " +
+                "package's bunfig.toml.",
+        );
+    }
+    return testBackstopStorageDir;
+}
+
 /**
  * Legacy magic-context storage directory used by the OpenCode plugin before the
  * shared cortexkit path. Used only for one-time migration of existing data into