fix(shared): resolve data-dir paths per call instead of capturing at import

packages/server/codex-command-builders.test.ts

@@ -1,24 +1,109 @@
-import { describe, expect, test } from "bun:test";
-import { buildCodexCommand } from "./codex-review";
-import { buildGuideCodexCommand } from "./guide/guide-review";
-import { buildTourCodexCommand } from "./tour/tour-review";
+/**
+ * Schema materialization for the Codex output-schema flag, across all three
+ * review surfaces.
+ *
+ * Run: bun test packages/server/codex-command-builders.test.ts
+ */
+
+import { afterEach, beforeEach, describe, expect, test } from "bun:test";
+import { existsSync, readFileSync, writeFileSync } from "node:fs";
+import { join } from "node:path";
+
+// Imported statically and BEFORE any PLANNOTATOR_DATA_DIR is set: these modules
+// used to capture the data directory at import, so changing the env var later
+// left every derived path (and the materialized schema) in the old location.
+import { buildCodexCommand, CODEX_REVIEW_SCHEMA } from "./codex-review";
+import { buildGuideCodexCommand, GUIDE_SCHEMA_JSON } from "./guide/guide-review";
+import { buildTourCodexCommand, TOUR_SCHEMA_JSON } from "./tour/tour-review";
+
+import { createTestEnvironment } from "../../tests/helpers/environment";
+
+const env = createTestEnvironment(["PLANNOTATOR_DATA_DIR"], "plannotator-codex-builders-");
+
+const OPTIONS = {
+  cwd: "/tmp/project",
+  outputPath: "/tmp/output.json",
+  prompt: "Review these changes.",
+};
+
+beforeEach(() => {
+  env.reset();
+  process.env.PLANNOTATOR_DATA_DIR = env.makeTempDir();
+});
+
+afterEach(() => env.restore());
+
+/** The path passed to `--output-schema`. */
+function schemaPath(command: string[]): string {
+  const index = command.indexOf("--output-schema");
+  expect(index).toBeGreaterThanOrEqual(0);
+  return command[index + 1]!;
+}
 
 describe("Codex command builders", () => {
   test("use the current automatic approval flag for every review surface", async () => {
-    const options = {
-      cwd: "/tmp/project",
-      outputPath: "/tmp/output.json",
-      prompt: "Review these changes.",
-    };
     const commands = await Promise.all([
-      buildCodexCommand(options),
-      buildGuideCodexCommand(options),
-      buildTourCodexCommand(options),
+      buildCodexCommand(OPTIONS),
+      buildGuideCodexCommand(OPTIONS),
+      buildTourCodexCommand(OPTIONS),
     ]);
 
     for (const command of commands) {
       expect(command).toContain("--approve-for-me");
       expect(command).not.toContain("--full-auto");
     }
   });
+
+  test("materialize each schema under PLANNOTATOR_DATA_DIR set after import", async () => {
+    const dataDir = process.env.PLANNOTATOR_DATA_DIR!;
+    const surfaces = [
+      [await buildCodexCommand(OPTIONS), "codex-review-schema.json"],
+      [await buildGuideCodexCommand(OPTIONS), "guide-schema.json"],
+      [await buildTourCodexCommand(OPTIONS), "tour-schema.json"],
+    ] as const;
+
+    for (const [command, fileName] of surfaces) {
+      const expected = join(dataDir, fileName);
+      expect(schemaPath(command)).toBe(expected);
+      expect(existsSync(expected)).toBe(true);
+      expect(readFileSync(expected, "utf-8")).toContain('"type":"object"');
+    }
+  });
+
+  test("re-materialize the schema after PLANNOTATOR_DATA_DIR changes", async () => {
+    const first = process.env.PLANNOTATOR_DATA_DIR!;
+    expect(schemaPath(await buildCodexCommand(OPTIONS))).toBe(
+      join(first, "codex-review-schema.json"),
+    );
+
+    const second = env.makeTempDir();
+    process.env.PLANNOTATOR_DATA_DIR = second;
+    const schema = schemaPath(await buildCodexCommand(OPTIONS));
+
+    expect(schema).toBe(join(second, "codex-review-schema.json"));
+    expect(existsSync(schema)).toBe(true);
+  });
+
+  test("overwrite a stale schema file left by an older binary", async () => {
+    // A schema written by an older version persists in the data dir forever
+    // (nothing prunes it). An existence check would keep serving those stale
+    // bytes; every process must refresh the file with its own schema once.
+    const dataDir = process.env.PLANNOTATOR_DATA_DIR!;
+    const stale = '{"stale":"written by an older binary"}';
+    const surfaces = [
+      [buildCodexCommand, "codex-review-schema.json", CODEX_REVIEW_SCHEMA],
+      [buildGuideCodexCommand, "guide-schema.json", GUIDE_SCHEMA_JSON],
+      [buildTourCodexCommand, "tour-schema.json", TOUR_SCHEMA_JSON],
+    ] as const;
+
+    for (const [build, fileName, currentSchema] of surfaces) {
+      const path = join(dataDir, fileName);
+      writeFileSync(path, stale);
+
+      const command = await build(OPTIONS);
+
+      expect(schemaPath(command)).toBe(path);
+      expect(readFileSync(path, "utf-8")).toBe(currentSchema);
+    }
+  });
 });

packages/server/codex-review.ts

@@ -5,7 +5,7 @@
  * The review server (review.ts) calls into this module via the agent-jobs callbacks.
  */
 
-import { join } from "node:path";
+import { dirname, join } from "node:path";
 import { tmpdir } from "node:os";
 import { appendFile, mkdir, unlink, writeFile, readFile } from "node:fs/promises";
 import { existsSync } from "node:fs";
@@ -21,19 +21,19 @@ import { classifyFindingPlacement } from "@plannotator/shared/external-annotatio
 // Debug log — only active when PLANNOTATOR_DEBUG is set
 // ---------------------------------------------------------------------------
 
-const DATA_DIR = getPlannotatorDataDir();
 const DEBUG_ENABLED = !!process.env.PLANNOTATOR_DEBUG;
-const DEBUG_LOG_PATH = join(DATA_DIR, "codex-review-debug.log");
 
 async function debugLog(label: string, data?: unknown): Promise<void> {
   if (!DEBUG_ENABLED) return;
   try {
-    await mkdir(DATA_DIR, { recursive: true });
+    // Resolved per call: PLANNOTATOR_DATA_DIR may change after import.
+    const dataDir = getPlannotatorDataDir();
+    await mkdir(dataDir, { recursive: true });
     const timestamp = new Date().toISOString();
     const line = data !== undefined
       ? `[${timestamp}] ${label}: ${typeof data === "string" ? data : JSON.stringify(data, null, 2)}\n`
       : `[${timestamp}] ${label}\n`;
-    await appendFile(DEBUG_LOG_PATH, line);
+    await appendFile(join(dataDir, "codex-review-debug.log"), line);
   } catch { /* never fail the main flow */ }
 }
 
@@ -88,22 +88,29 @@ export const CODEX_REVIEW_SCHEMA = JSON.stringify({
   additionalProperties: false,
 });
 
-const SCHEMA_DIR = DATA_DIR;
-const SCHEMA_FILE = join(SCHEMA_DIR, "codex-review-schema.json");
-let schemaMaterialized = false;
+/** Resolve the materialized schema path for the current data directory. */
+export function getCodexReviewSchemaPath(): string {
+  return join(getPlannotatorDataDir(), "codex-review-schema.json");
+}
+
+/** Schema paths this process has already refreshed with its own schema. */
+const materializedSchemaPaths = new Set<string>();
 
-/** Ensure the schema file exists on disk and return its path. */
+/** Ensure the schema file exists on disk with the current schema and return its path. */
 async function ensureSchemaFile(): Promise<string> {
-  if (!schemaMaterialized) {
-    await mkdir(SCHEMA_DIR, { recursive: true });
-    await writeFile(SCHEMA_FILE, CODEX_REVIEW_SCHEMA);
-    schemaMaterialized = true;
+  const schemaPath = getCodexReviewSchemaPath();
+  // Guarded per resolved path, not per process and not by file existence: the
+  // data directory can change after import (so a process-wide flag would keep
+  // returning the old location), and a stale file left by an older binary must
+  // be overwritten once per process so Codex always gets the current schema.
+  if (!materializedSchemaPaths.has(schemaPath)) {
+    await mkdir(dirname(schemaPath), { recursive: true });
+    await writeFile(schemaPath, CODEX_REVIEW_SCHEMA);
+    materializedSchemaPaths.add(schemaPath);
   }
-  return SCHEMA_FILE;
+  return schemaPath;
 }
 
-export { SCHEMA_FILE as CODEX_REVIEW_SCHEMA_PATH };
-
 // ---------------------------------------------------------------------------
 // System prompt — copied verbatim from codex-rs/core/review_prompt.md
 // ---------------------------------------------------------------------------

packages/server/guide/guide-review.ts

@@ -1,4 +1,4 @@
-import { join } from "node:path";
+import { dirname, join } from "node:path";
 import { tmpdir } from "node:os";
 import { mkdir, writeFile, readFile, unlink } from "node:fs/promises";
 import { getPlannotatorDataDir } from "@plannotator/shared/data-dir";
@@ -475,17 +475,26 @@ export function buildGuideClaudeCommand(prompt: string, model: string = "sonnet"
   };
 }
 
-const GUIDE_SCHEMA_DIR = getPlannotatorDataDir();
-const GUIDE_SCHEMA_FILE = join(GUIDE_SCHEMA_DIR, "guide-schema.json");
-let guideSchemaMaterialized = false;
+/** Materialized schema path under the current data directory. */
+function guideSchemaPath(): string {
+  return join(getPlannotatorDataDir(), "guide-schema.json");
+}
+
+/** Schema paths this process has already refreshed with its own schema. */
+const materializedGuideSchemaPaths = new Set<string>();
 
 async function ensureGuideSchemaFile(): Promise<string> {
-  if (!guideSchemaMaterialized) {
-    await mkdir(GUIDE_SCHEMA_DIR, { recursive: true });
-    await writeFile(GUIDE_SCHEMA_FILE, GUIDE_SCHEMA_JSON);
-    guideSchemaMaterialized = true;
+  const schemaPath = guideSchemaPath();
+  // Guarded per resolved path, not per process and not by file existence: a
+  // PLANNOTATOR_DATA_DIR change after import materializes the schema in the
+  // new location, and a stale file left by an older binary is overwritten
+  // once per process so the agent always gets the current schema.
+  if (!materializedGuideSchemaPaths.has(schemaPath)) {
+    await mkdir(dirname(schemaPath), { recursive: true });
+    await writeFile(schemaPath, GUIDE_SCHEMA_JSON);
+    materializedGuideSchemaPaths.add(schemaPath);
   }
-  return GUIDE_SCHEMA_FILE;
+  return schemaPath;
 }
 
 export function generateGuideOutputPath(): string {

packages/server/tour/tour-review.ts

@@ -1,4 +1,4 @@
-import { join } from "node:path";
+import { dirname, join } from "node:path";
 import { tmpdir } from "node:os";
 import { mkdir, writeFile, readFile, unlink } from "node:fs/promises";
 import { getPlannotatorDataDir } from "@plannotator/shared/data-dir";
@@ -413,17 +413,26 @@ export function buildTourClaudeCommand(prompt: string, model: string = "sonnet",
   };
 }
 
-const TOUR_SCHEMA_DIR = getPlannotatorDataDir();
-const TOUR_SCHEMA_FILE = join(TOUR_SCHEMA_DIR, "tour-schema.json");
-let tourSchemaMaterialized = false;
+/** Materialized schema path under the current data directory. */
+function tourSchemaPath(): string {
+  return join(getPlannotatorDataDir(), "tour-schema.json");
+}
+
+/** Schema paths this process has already refreshed with its own schema. */
+const materializedTourSchemaPaths = new Set<string>();
 
 async function ensureTourSchemaFile(): Promise<string> {
-  if (!tourSchemaMaterialized) {
-    await mkdir(TOUR_SCHEMA_DIR, { recursive: true });
-    await writeFile(TOUR_SCHEMA_FILE, TOUR_SCHEMA_JSON);
-    tourSchemaMaterialized = true;
+  const schemaPath = tourSchemaPath();
+  // Guarded per resolved path, not per process and not by file existence: a
+  // PLANNOTATOR_DATA_DIR change after import materializes the schema in the
+  // new location, and a stale file left by an older binary is overwritten
+  // once per process so the agent always gets the current schema.
+  if (!materializedTourSchemaPaths.has(schemaPath)) {
+    await mkdir(dirname(schemaPath), { recursive: true });
+    await writeFile(schemaPath, TOUR_SCHEMA_JSON);
+    materializedTourSchemaPaths.add(schemaPath);
   }
-  return TOUR_SCHEMA_FILE;
+  return schemaPath;
 }
 
 export function generateTourOutputPath(): string {