From 941b5edbda3beb5144aaa9ae3418af445f24dd2a Mon Sep 17 00:00:00 2001 From: FND Date: Thu, 10 Sep 2026 09:13:21 +0200 Subject: [PATCH 1/2] fix(data-dir): resolve the data directory per call instead of at import --- .../server/codex-command-builders.test.ts | 80 +++++++- packages/server/codex-review.ts | 30 +-- packages/server/guide/guide-review.ts | 20 +- packages/server/tour/tour-review.ts | 20 +- packages/shared/improvement-hooks.test.ts | 173 ++++++++---------- packages/shared/improvement-hooks.ts | 32 ++-- 6 files changed, 200 insertions(+), 155 deletions(-) diff --git a/packages/server/codex-command-builders.test.ts b/packages/server/codex-command-builders.test.ts index f2fac83d2..d2fee120f 100644 --- a/packages/server/codex-command-builders.test.ts +++ b/packages/server/codex-command-builders.test.ts @@ -1,19 +1,51 @@ -import { describe, expect, test } from "bun:test"; +/** + * 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 } 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 } from "./codex-review"; import { buildGuideCodexCommand } from "./guide/guide-review"; import { buildTourCodexCommand } 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) { @@ -21,4 +53,34 @@ describe("Codex command builders", () => { 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); + }); }); diff --git a/packages/server/codex-review.ts b/packages/server/codex-review.ts index 2f40ac12a..b381fd807 100644 --- a/packages/server/codex-review.ts +++ b/packages/server/codex-review.ts @@ -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 { 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,24 @@ 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"); +} /** Ensure the schema file exists on disk and return its path. */ async function ensureSchemaFile(): Promise { - if (!schemaMaterialized) { - await mkdir(SCHEMA_DIR, { recursive: true }); - await writeFile(SCHEMA_FILE, CODEX_REVIEW_SCHEMA); - schemaMaterialized = true; + const schemaPath = getCodexReviewSchemaPath(); + // Existence is checked per resolved path: the data directory can change after + // import, so a process-wide "written once" flag would keep returning the old + // location without ever materializing the file there. + if (!existsSync(schemaPath)) { + await mkdir(getPlannotatorDataDir(), { recursive: true }); + await writeFile(schemaPath, CODEX_REVIEW_SCHEMA); } - return SCHEMA_FILE; + return schemaPath; } -export { SCHEMA_FILE as CODEX_REVIEW_SCHEMA_PATH }; - // --------------------------------------------------------------------------- // System prompt — copied verbatim from codex-rs/core/review_prompt.md // --------------------------------------------------------------------------- diff --git a/packages/server/guide/guide-review.ts b/packages/server/guide/guide-review.ts index 090bfb414..c587967bd 100644 --- a/packages/server/guide/guide-review.ts +++ b/packages/server/guide/guide-review.ts @@ -1,5 +1,6 @@ import { join } from "node:path"; import { tmpdir } from "node:os"; +import { existsSync } from "node:fs"; import { mkdir, writeFile, readFile, unlink } from "node:fs/promises"; import { getPlannotatorDataDir } from "@plannotator/shared/data-dir"; import { loadConfig, resolveCursorSandbox } from "../config"; @@ -475,17 +476,20 @@ 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"); +} async function ensureGuideSchemaFile(): Promise { - if (!guideSchemaMaterialized) { - await mkdir(GUIDE_SCHEMA_DIR, { recursive: true }); - await writeFile(GUIDE_SCHEMA_FILE, GUIDE_SCHEMA_JSON); - guideSchemaMaterialized = true; + const schemaPath = guideSchemaPath(); + // Checked per resolved path so a PLANNOTATOR_DATA_DIR change after import + // materializes the schema in the new location instead of reusing a flag. + if (!existsSync(schemaPath)) { + await mkdir(getPlannotatorDataDir(), { recursive: true }); + await writeFile(schemaPath, GUIDE_SCHEMA_JSON); } - return GUIDE_SCHEMA_FILE; + return schemaPath; } export function generateGuideOutputPath(): string { diff --git a/packages/server/tour/tour-review.ts b/packages/server/tour/tour-review.ts index 2422e891f..5651c302f 100644 --- a/packages/server/tour/tour-review.ts +++ b/packages/server/tour/tour-review.ts @@ -1,5 +1,6 @@ import { join } from "node:path"; import { tmpdir } from "node:os"; +import { existsSync } from "node:fs"; import { mkdir, writeFile, readFile, unlink } from "node:fs/promises"; import { getPlannotatorDataDir } from "@plannotator/shared/data-dir"; import type { DiffType } from "../vcs"; @@ -413,17 +414,20 @@ 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"); +} async function ensureTourSchemaFile(): Promise { - if (!tourSchemaMaterialized) { - await mkdir(TOUR_SCHEMA_DIR, { recursive: true }); - await writeFile(TOUR_SCHEMA_FILE, TOUR_SCHEMA_JSON); - tourSchemaMaterialized = true; + const schemaPath = tourSchemaPath(); + // Checked per resolved path so a PLANNOTATOR_DATA_DIR change after import + // materializes the schema in the new location instead of reusing a flag. + if (!existsSync(schemaPath)) { + await mkdir(getPlannotatorDataDir(), { recursive: true }); + await writeFile(schemaPath, TOUR_SCHEMA_JSON); } - return TOUR_SCHEMA_FILE; + return schemaPath; } export function generateTourOutputPath(): string { diff --git a/packages/shared/improvement-hooks.test.ts b/packages/shared/improvement-hooks.test.ts index 610c1c474..4e5b67e61 100644 --- a/packages/shared/improvement-hooks.test.ts +++ b/packages/shared/improvement-hooks.test.ts @@ -5,132 +5,103 @@ */ import { describe, expect, test, beforeEach, afterEach } from "bun:test"; -import { mkdirSync, writeFileSync, rmSync, existsSync } from "fs"; -import { join } from "path"; -import { tmpdir } from "os"; +import { mkdirSync, writeFileSync } from "node:fs"; +import { join, dirname } from "node:path"; -// We need to override the base dirs used by readImprovementHook. -// Since the module uses homedir() at import time, we mock it via -// a test harness that sets HOME to a temp directory. +// Imported statically and BEFORE any PLANNOTATOR_DATA_DIR is set: the module's +// import-time side effects used to freeze the data directory, so a later env +// change was silently ignored and every read kept hitting the old location. +import { getImprovementHookExpectedPath, readImprovementHook } from "./improvement-hooks"; + +import { createTestEnvironment } from "../../tests/helpers/environment"; + +const env = createTestEnvironment(["PLANNOTATOR_DATA_DIR"], "plannotator-improvement-hooks-"); -const TEST_HOME = join(tmpdir(), `improvement-hooks-test-${Date.now()}`); -const NEW_BASE = join(TEST_HOME, ".plannotator", "hooks"); -const LEGACY_BASE = join(TEST_HOME, ".plannotator"); const HOOK_RELATIVE = "compound/enterplanmode-improve-hook.txt"; -function setupTestHome() { - mkdirSync(join(NEW_BASE, "compound"), { recursive: true }); - mkdirSync(join(LEGACY_BASE, "compound"), { recursive: true }); -} +let dataDir = ""; -function cleanTestHome() { - if (existsSync(TEST_HOME)) { - rmSync(TEST_HOME, { recursive: true, force: true }); - } +function writeHook(filePath: string, content: string): void { + mkdirSync(dirname(filePath), { recursive: true }); + writeFileSync(filePath, content); } -// Since the module reads homedir() at import time, we need to -// re-import with HOME overridden. Use a helper that spawns a -// small inline script to test each scenario in isolation. -async function runScenario(setup: { - newPathContent?: string | null; - legacyPathContent?: string | null; -}): Promise<{ content: string; filePath: string } | null> { - setupTestHome(); - - const newPath = join(NEW_BASE, HOOK_RELATIVE); - const legacyPath = join(LEGACY_BASE, HOOK_RELATIVE); - - if (setup.newPathContent !== undefined && setup.newPathContent !== null) { - writeFileSync(newPath, setup.newPathContent); - } - if (setup.legacyPathContent !== undefined && setup.legacyPathContent !== null) { - writeFileSync(legacyPath, setup.legacyPathContent); - } - - // Run in a subprocess with HOME overridden so homedir() returns TEST_HOME - const proc = Bun.spawn( - [ - "bun", - "-e", - ` - import { readImprovementHook } from "./packages/shared/improvement-hooks"; - const result = readImprovementHook("enterplanmode-improve"); - console.log(JSON.stringify(result)); - `, - ], - { - // Exercise the fake HOME rather than inheriting the parent test sandbox. - env: { ...process.env, HOME: TEST_HOME, USERPROFILE: TEST_HOME, PLANNOTATOR_DATA_DIR: "" }, - cwd: join(import.meta.dir, "../.."), - stdout: "pipe", - stderr: "pipe", - }, - ); - - const stdout = await new Response(proc.stdout).text(); - const exitCode = await proc.exited; - - if (exitCode !== 0) { - const stderr = await new Response(proc.stderr).text(); - throw new Error(`Subprocess failed (exit ${exitCode}): ${stderr}`); - } - - const parsed = JSON.parse(stdout.trim()); - return parsed; -} +beforeEach(() => { + env.reset(); + dataDir = env.makeTempDir(); + process.env.PLANNOTATOR_DATA_DIR = dataDir; +}); + +afterEach(() => env.restore()); describe("readImprovementHook", () => { - beforeEach(setupTestHome); - afterEach(cleanTestHome); + test("returns content from new path when file exists", () => { + const newPath = join(dataDir, "hooks", HOOK_RELATIVE); + writeHook(newPath, "Focus on error handling"); - test("returns content from new path when file exists", async () => { - const result = await runScenario({ - newPathContent: "Focus on error handling", - }); + const result = readImprovementHook("enterplanmode-improve"); expect(result).not.toBeNull(); expect(result!.content).toBe("Focus on error handling"); - expect(result!.filePath).toContain(".plannotator/hooks/compound/"); + expect(result!.filePath).toBe(newPath); }); - test("new path wins over legacy path", async () => { - const result = await runScenario({ - newPathContent: "New instructions", - legacyPathContent: "Old instructions", - }); + test("new path wins over legacy path", () => { + writeHook(join(dataDir, "hooks", HOOK_RELATIVE), "New instructions"); + writeHook(join(dataDir, HOOK_RELATIVE), "Old instructions"); + + const result = readImprovementHook("enterplanmode-improve"); expect(result).not.toBeNull(); expect(result!.content).toBe("New instructions"); - expect(result!.filePath).toContain(".plannotator/hooks/compound/"); + expect(result!.filePath).toBe(join(dataDir, "hooks", HOOK_RELATIVE)); }); - test("falls back to legacy path when new path is absent", async () => { - const result = await runScenario({ - legacyPathContent: "Legacy instructions", - }); + test("falls back to legacy path when new path is absent", () => { + const legacyPath = join(dataDir, HOOK_RELATIVE); + writeHook(legacyPath, "Legacy instructions"); + + const result = readImprovementHook("enterplanmode-improve"); expect(result).not.toBeNull(); expect(result!.content).toBe("Legacy instructions"); - expect(result!.filePath).toContain(".plannotator/compound/"); - expect(result!.filePath).not.toContain(".plannotator/hooks/"); + expect(result!.filePath).toBe(legacyPath); }); - test("returns null when new path exists but is empty (no legacy fallback)", async () => { - const result = await runScenario({ - newPathContent: "", - legacyPathContent: "Legacy instructions", - }); - expect(result).toBeNull(); + test("returns null when new path exists but is empty (no legacy fallback)", () => { + writeHook(join(dataDir, "hooks", HOOK_RELATIVE), ""); + writeHook(join(dataDir, HOOK_RELATIVE), "Legacy instructions"); + + expect(readImprovementHook("enterplanmode-improve")).toBeNull(); }); - test("returns null when no files exist", async () => { - const result = await runScenario({}); - expect(result).toBeNull(); + test("returns null when no files exist", () => { + expect(readImprovementHook("enterplanmode-improve")).toBeNull(); }); - test("returns null when new path is whitespace-only (no legacy fallback)", async () => { - const result = await runScenario({ - newPathContent: " \n \n ", - legacyPathContent: "Legacy instructions", - }); - expect(result).toBeNull(); + test("returns null when new path is whitespace-only (no legacy fallback)", () => { + writeHook(join(dataDir, "hooks", HOOK_RELATIVE), " \n \n "); + writeHook(join(dataDir, HOOK_RELATIVE), "Legacy instructions"); + + expect(readImprovementHook("enterplanmode-improve")).toBeNull(); + }); +}); + +describe("data directory resolution", () => { + test("resolves PLANNOTATOR_DATA_DIR set after import", () => { + expect(getImprovementHookExpectedPath("enterplanmode-improve")).toBe( + join(dataDir, "hooks", HOOK_RELATIVE), + ); + }); + + test("follows a later change to PLANNOTATOR_DATA_DIR", () => { + expect(getImprovementHookExpectedPath("enterplanmode-improve")).toBe( + join(dataDir, "hooks", HOOK_RELATIVE), + ); + + const second = env.makeTempDir(); + process.env.PLANNOTATOR_DATA_DIR = second; + const secondHook = join(second, "hooks", HOOK_RELATIVE); + writeHook(secondHook, "Second location"); + + expect(getImprovementHookExpectedPath("enterplanmode-improve")).toBe(secondHook); + expect(readImprovementHook("enterplanmode-improve")!.filePath).toBe(secondHook); }); }); diff --git a/packages/shared/improvement-hooks.ts b/packages/shared/improvement-hooks.ts index 83a031430..308c8bb2a 100644 --- a/packages/shared/improvement-hooks.ts +++ b/packages/shared/improvement-hooks.ts @@ -21,21 +21,23 @@ import { join } from "path"; import { readFileSync, statSync } from "fs"; import { getPlannotatorDataDir } from "./data-dir"; -const DATA_DIR = getPlannotatorDataDir(); - -/** Hooks subdirectory (preferred location) */ -const HOOKS_BASE_DIR = join(DATA_DIR, "hooks"); - -/** Fallback: hooks placed directly in the data dir (pre-hooks-subdir layout) */ -const LEGACY_BASE_DIR = DATA_DIR; +/** + * Hooks subdirectory (preferred location). + * + * Resolved per call: PLANNOTATOR_DATA_DIR may be set after this module is + * imported, so a module-scope capture would freeze the pre-switch path. + */ +function hooksBaseDir(): string { + return join(getPlannotatorDataDir(), "hooks"); +} /** Maximum file size to read (50 KB) */ const MAX_FILE_SIZE = 50 * 1024; /** * Known improvement hook file paths, keyed by hook name. - * `path` is relative to HOOKS_BASE_DIR (~/.plannotator/hooks/). - * `legacyPath` is relative to LEGACY_BASE_DIR (~/.plannotator/). + * `path` is relative to the hooks base dir (~/.plannotator/hooks/). + * `legacyPath` is relative to the data dir itself (~/.plannotator/). */ const KNOWN_HOOKS = { "enterplanmode-improve": { @@ -51,7 +53,7 @@ export function getImprovementHookExpectedPath( ): string | null { const entry = KNOWN_HOOKS[hookName]; if (!entry) return null; - return join(HOOKS_BASE_DIR, entry.path); + return join(hooksBaseDir(), entry.path); } export interface ImprovementHookResult { @@ -92,9 +94,9 @@ function tryReadHookFile( * Read an improvement hook file by name. * * Lookup order: - * 1. New path (HOOKS_BASE_DIR + path). If it exists and validates, return it. + * 1. New path (hooks base dir + path). If it exists and validates, return it. * 2. If the new path exists but is invalid (empty, oversized, etc.), return null. - * 3. Only if the new path does not exist, try the legacy path (LEGACY_BASE_DIR + legacyPath). + * 3. Only if the new path does not exist, try the legacy path (data dir + legacyPath). */ export function readImprovementHook( hookName: ImprovementHookName, @@ -102,14 +104,14 @@ export function readImprovementHook( const entry = KNOWN_HOOKS[hookName]; if (!entry) return null; - const newPath = join(HOOKS_BASE_DIR, entry.path); + const newPath = join(hooksBaseDir(), entry.path); // New path exists — use it exclusively (even if invalid) if (fileExists(newPath)) { return tryReadHookFile(newPath, hookName); } - // New path absent — fall back to legacy path - const legacyFilePath = join(LEGACY_BASE_DIR, entry.legacyPath); + // New path absent — fall back to legacy path (directly in the data dir) + const legacyFilePath = join(getPlannotatorDataDir(), entry.legacyPath); return tryReadHookFile(legacyFilePath, hookName); } From 75326e3b3119c5aa1b3356d6dc25c00e64e6310b Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Thu, 10 Sep 2026 16:24:31 -0700 Subject: [PATCH 2/2] fix(shared): refresh agent schema files once per process, not once ever --- .../server/codex-command-builders.test.ts | 31 ++++++++++++++++--- packages/server/codex-review.ts | 19 +++++++----- packages/server/guide/guide-review.ts | 17 ++++++---- packages/server/tour/tour-review.ts | 17 ++++++---- 4 files changed, 61 insertions(+), 23 deletions(-) diff --git a/packages/server/codex-command-builders.test.ts b/packages/server/codex-command-builders.test.ts index d2fee120f..e962040ac 100644 --- a/packages/server/codex-command-builders.test.ts +++ b/packages/server/codex-command-builders.test.ts @@ -6,15 +6,15 @@ */ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { existsSync, readFileSync } from "node:fs"; +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 } from "./codex-review"; -import { buildGuideCodexCommand } from "./guide/guide-review"; -import { buildTourCodexCommand } from "./tour/tour-review"; +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"; @@ -83,4 +83,27 @@ describe("Codex command builders", () => { 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); + } + }); }); diff --git a/packages/server/codex-review.ts b/packages/server/codex-review.ts index b381fd807..556e16842 100644 --- a/packages/server/codex-review.ts +++ b/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"; @@ -93,15 +93,20 @@ export function getCodexReviewSchemaPath(): string { return join(getPlannotatorDataDir(), "codex-review-schema.json"); } -/** Ensure the schema file exists on disk and return its path. */ +/** Schema paths this process has already refreshed with its own schema. */ +const materializedSchemaPaths = new Set(); + +/** Ensure the schema file exists on disk with the current schema and return its path. */ async function ensureSchemaFile(): Promise { const schemaPath = getCodexReviewSchemaPath(); - // Existence is checked per resolved path: the data directory can change after - // import, so a process-wide "written once" flag would keep returning the old - // location without ever materializing the file there. - if (!existsSync(schemaPath)) { - await mkdir(getPlannotatorDataDir(), { recursive: true }); + // 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 schemaPath; } diff --git a/packages/server/guide/guide-review.ts b/packages/server/guide/guide-review.ts index c587967bd..8341a6330 100644 --- a/packages/server/guide/guide-review.ts +++ b/packages/server/guide/guide-review.ts @@ -1,6 +1,5 @@ -import { join } from "node:path"; +import { dirname, join } from "node:path"; import { tmpdir } from "node:os"; -import { existsSync } from "node:fs"; import { mkdir, writeFile, readFile, unlink } from "node:fs/promises"; import { getPlannotatorDataDir } from "@plannotator/shared/data-dir"; import { loadConfig, resolveCursorSandbox } from "../config"; @@ -481,13 +480,19 @@ function guideSchemaPath(): string { return join(getPlannotatorDataDir(), "guide-schema.json"); } +/** Schema paths this process has already refreshed with its own schema. */ +const materializedGuideSchemaPaths = new Set(); + async function ensureGuideSchemaFile(): Promise { const schemaPath = guideSchemaPath(); - // Checked per resolved path so a PLANNOTATOR_DATA_DIR change after import - // materializes the schema in the new location instead of reusing a flag. - if (!existsSync(schemaPath)) { - await mkdir(getPlannotatorDataDir(), { recursive: true }); + // 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 schemaPath; } diff --git a/packages/server/tour/tour-review.ts b/packages/server/tour/tour-review.ts index 5651c302f..eebd68e94 100644 --- a/packages/server/tour/tour-review.ts +++ b/packages/server/tour/tour-review.ts @@ -1,6 +1,5 @@ -import { join } from "node:path"; +import { dirname, join } from "node:path"; import { tmpdir } from "node:os"; -import { existsSync } from "node:fs"; import { mkdir, writeFile, readFile, unlink } from "node:fs/promises"; import { getPlannotatorDataDir } from "@plannotator/shared/data-dir"; import type { DiffType } from "../vcs"; @@ -419,13 +418,19 @@ function tourSchemaPath(): string { return join(getPlannotatorDataDir(), "tour-schema.json"); } +/** Schema paths this process has already refreshed with its own schema. */ +const materializedTourSchemaPaths = new Set(); + async function ensureTourSchemaFile(): Promise { const schemaPath = tourSchemaPath(); - // Checked per resolved path so a PLANNOTATOR_DATA_DIR change after import - // materializes the schema in the new location instead of reusing a flag. - if (!existsSync(schemaPath)) { - await mkdir(getPlannotatorDataDir(), { recursive: true }); + // 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 schemaPath; }