diff --git a/plugins/codex/scripts/lib/state.mjs b/plugins/codex/scripts/lib/state.mjs index 2da23498f..af65c568b 100644 --- a/plugins/codex/scripts/lib/state.mjs +++ b/plugins/codex/scripts/lib/state.mjs @@ -6,7 +6,8 @@ import path from "node:path"; import { resolveWorkspaceRoot } from "./workspace.mjs"; const STATE_VERSION = 1; -const PLUGIN_DATA_ENV = "CLAUDE_PLUGIN_DATA"; +export const PLUGIN_DATA_ENV = "CODEX_COMPANION_PLUGIN_DATA"; +const HOST_PLUGIN_DATA_ENV = "CLAUDE_PLUGIN_DATA"; const FALLBACK_STATE_ROOT_DIR = path.join(os.tmpdir(), "codex-companion"); const STATE_FILE_NAME = "state.json"; const JOBS_DIR_NAME = "jobs"; @@ -38,7 +39,11 @@ export function resolveStateDir(cwd) { const slugSource = path.basename(workspaceRoot) || "workspace"; const slug = slugSource.replace(/[^a-zA-Z0-9._-]+/g, "-").replace(/^-+|-+$/g, "") || "workspace"; const hash = createHash("sha256").update(canonicalWorkspaceRoot).digest("hex").slice(0, 16); - const pluginDataDir = process.env[PLUGIN_DATA_ENV]; + // CLAUDE_PLUGIN_DATA is scoped to this plugin while its hook runs, but the + // SessionStart env file is shared by every plugin. Persist that scoped value + // under a Codex-owned name so a sibling hook cannot redirect our later jobs. + // Keep the host variable as a fallback for direct and pre-upgrade callers. + const pluginDataDir = process.env[PLUGIN_DATA_ENV] || process.env[HOST_PLUGIN_DATA_ENV]; const stateRoot = pluginDataDir ? path.join(pluginDataDir, "state") : FALLBACK_STATE_ROOT_DIR; return path.join(stateRoot, `${slug}-${hash}`); } diff --git a/plugins/codex/scripts/session-lifecycle-hook.mjs b/plugins/codex/scripts/session-lifecycle-hook.mjs index 778571e6c..8ef95b2c5 100644 --- a/plugins/codex/scripts/session-lifecycle-hook.mjs +++ b/plugins/codex/scripts/session-lifecycle-hook.mjs @@ -13,12 +13,12 @@ import { sendBrokerShutdown, teardownBrokerSession } from "./lib/broker-lifecycle.mjs"; -import { loadState, resolveStateFile, saveState } from "./lib/state.mjs"; +import { loadState, PLUGIN_DATA_ENV, resolveStateFile, saveState } from "./lib/state.mjs"; import { TRANSCRIPT_PATH_ENV } from "./lib/claude-session-transfer.mjs"; import { resolveWorkspaceRoot } from "./lib/workspace.mjs"; export const SESSION_ID_ENV = "CODEX_COMPANION_SESSION_ID"; -const PLUGIN_DATA_ENV = "CLAUDE_PLUGIN_DATA"; +const HOST_PLUGIN_DATA_ENV = "CLAUDE_PLUGIN_DATA"; function readHookInput() { const raw = fs.readFileSync(0, "utf8").trim(); @@ -77,7 +77,7 @@ function cleanupSessionJobs(cwd, sessionId) { function handleSessionStart(input) { appendEnvVar(SESSION_ID_ENV, input.session_id); appendEnvVar(TRANSCRIPT_PATH_ENV, input.transcript_path); - appendEnvVar(PLUGIN_DATA_ENV, process.env[PLUGIN_DATA_ENV]); + appendEnvVar(PLUGIN_DATA_ENV, process.env[HOST_PLUGIN_DATA_ENV]); } async function handleSessionEnd(input) { diff --git a/tests/runtime.test.mjs b/tests/runtime.test.mjs index 8f276835b..a366b3093 100644 --- a/tests/runtime.test.mjs +++ b/tests/runtime.test.mjs @@ -669,7 +669,7 @@ test("task --resume-last ignores running tasks from other Claude sessions", () = assert.match(resume.stderr, /No previous Codex task thread was found for this repository\./); }); -test("session start hook exports the Claude session id, transcript path, and plugin data dir", () => { +test("session start hook exports the Claude session id, transcript path, and a Codex-owned plugin data variable", () => { const repo = makeTempDir(); const envFile = path.join(makeTempDir(), "claude-env.sh"); fs.writeFileSync(envFile, "", "utf8"); @@ -694,7 +694,7 @@ test("session start hook exports the Claude session id, transcript path, and plu assert.equal(result.status, 0, result.stderr); assert.equal( fs.readFileSync(envFile, "utf8"), - `export CODEX_COMPANION_SESSION_ID='sess-current'\nexport CODEX_COMPANION_TRANSCRIPT_PATH='${transcriptPath}'\nexport CLAUDE_PLUGIN_DATA='${pluginDataDir}'\n` + `export CODEX_COMPANION_SESSION_ID='sess-current'\nexport CODEX_COMPANION_TRANSCRIPT_PATH='${transcriptPath}'\nexport CODEX_COMPANION_PLUGIN_DATA='${pluginDataDir}'\n` ); }); diff --git a/tests/state.test.mjs b/tests/state.test.mjs index 0f8f57cea..600c75e3f 100644 --- a/tests/state.test.mjs +++ b/tests/state.test.mjs @@ -5,21 +5,47 @@ import test from "node:test"; import assert from "node:assert/strict"; import { makeTempDir } from "./helpers.mjs"; -import { resolveJobFile, resolveJobLogFile, resolveStateDir, resolveStateFile, saveState } from "../plugins/codex/scripts/lib/state.mjs"; +import { + resolveJobFile, + resolveJobLogFile, + resolveStateDir, + resolveStateFile, + saveState +} from "../plugins/codex/scripts/lib/state.mjs"; test("resolveStateDir uses a temp-backed per-workspace directory", () => { const workspace = makeTempDir(); - const stateDir = resolveStateDir(workspace); + const previousCodexPluginDataDir = process.env.CODEX_COMPANION_PLUGIN_DATA; + const previousPluginDataDir = process.env.CLAUDE_PLUGIN_DATA; + delete process.env.CODEX_COMPANION_PLUGIN_DATA; + delete process.env.CLAUDE_PLUGIN_DATA; + + try { + const stateDir = resolveStateDir(workspace); - assert.equal(stateDir.startsWith(os.tmpdir()), true); - assert.match(path.basename(stateDir), /.+-[a-f0-9]{16}$/); - assert.match(stateDir, new RegExp(`^${os.tmpdir().replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}`)); + assert.equal(stateDir.startsWith(os.tmpdir()), true); + assert.match(path.basename(stateDir), /.+-[a-f0-9]{16}$/); + assert.match(stateDir, new RegExp(`^${os.tmpdir().replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}`)); + } finally { + if (previousCodexPluginDataDir == null) { + delete process.env.CODEX_COMPANION_PLUGIN_DATA; + } else { + process.env.CODEX_COMPANION_PLUGIN_DATA = previousCodexPluginDataDir; + } + if (previousPluginDataDir == null) { + delete process.env.CLAUDE_PLUGIN_DATA; + } else { + process.env.CLAUDE_PLUGIN_DATA = previousPluginDataDir; + } + } }); -test("resolveStateDir uses CLAUDE_PLUGIN_DATA when it is provided", () => { +test("resolveStateDir falls back to CLAUDE_PLUGIN_DATA when it is provided", () => { const workspace = makeTempDir(); const pluginDataDir = makeTempDir(); + const previousCodexPluginDataDir = process.env.CODEX_COMPANION_PLUGIN_DATA; const previousPluginDataDir = process.env.CLAUDE_PLUGIN_DATA; + delete process.env.CODEX_COMPANION_PLUGIN_DATA; process.env.CLAUDE_PLUGIN_DATA = pluginDataDir; try { @@ -32,6 +58,94 @@ test("resolveStateDir uses CLAUDE_PLUGIN_DATA when it is provided", () => { new RegExp(`^${path.join(pluginDataDir, "state").replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}`) ); } finally { + if (previousCodexPluginDataDir == null) { + delete process.env.CODEX_COMPANION_PLUGIN_DATA; + } else { + process.env.CODEX_COMPANION_PLUGIN_DATA = previousCodexPluginDataDir; + } + if (previousPluginDataDir == null) { + delete process.env.CLAUDE_PLUGIN_DATA; + } else { + process.env.CLAUDE_PLUGIN_DATA = previousPluginDataDir; + } + } +}); + +test("resolveStateDir prefers the Codex plugin data dir over another plugin's host-scoped value", () => { + const workspace = makeTempDir(); + const codexPluginDataDir = makeTempDir(); + const siblingPluginDataDir = makeTempDir(); + const previousCodexPluginDataDir = process.env.CODEX_COMPANION_PLUGIN_DATA; + const previousPluginDataDir = process.env.CLAUDE_PLUGIN_DATA; + process.env.CODEX_COMPANION_PLUGIN_DATA = codexPluginDataDir; + process.env.CLAUDE_PLUGIN_DATA = siblingPluginDataDir; + + try { + const stateDir = resolveStateDir(workspace); + + assert.equal(stateDir.startsWith(path.join(codexPluginDataDir, "state")), true); + assert.equal(stateDir.startsWith(path.join(siblingPluginDataDir, "state")), false); + } finally { + if (previousCodexPluginDataDir == null) { + delete process.env.CODEX_COMPANION_PLUGIN_DATA; + } else { + process.env.CODEX_COMPANION_PLUGIN_DATA = previousCodexPluginDataDir; + } + if (previousPluginDataDir == null) { + delete process.env.CLAUDE_PLUGIN_DATA; + } else { + process.env.CLAUDE_PLUGIN_DATA = previousPluginDataDir; + } + } +}); + +test("saveState does not prune another plugin's job when the host-scoped data dir was overwritten", () => { + const workspace = makeTempDir(); + const codexPluginDataDir = makeTempDir(); + const siblingPluginDataDir = makeTempDir(); + const previousCodexPluginDataDir = process.env.CODEX_COMPANION_PLUGIN_DATA; + const previousPluginDataDir = process.env.CLAUDE_PLUGIN_DATA; + process.env.CODEX_COMPANION_PLUGIN_DATA = siblingPluginDataDir; + process.env.CLAUDE_PLUGIN_DATA = siblingPluginDataDir; + + try { + const siblingStateFile = resolveStateFile(workspace); + const siblingJobFile = resolveJobFile(workspace, "sibling-job"); + const siblingLogFile = resolveJobLogFile(workspace, "sibling-job"); + const siblingState = { + version: 1, + config: { stopReviewGate: false }, + jobs: [ + { + id: "sibling-job", + status: "completed", + logFile: siblingLogFile, + createdAt: "2026-08-25T00:00:00.000Z", + updatedAt: "2026-08-25T00:00:00.000Z" + } + ] + }; + fs.writeFileSync(siblingJobFile, '{"owner":"sibling"}\n', "utf8"); + fs.writeFileSync(siblingLogFile, "sibling output\n", "utf8"); + fs.writeFileSync(siblingStateFile, `${JSON.stringify(siblingState, null, 2)}\n`, "utf8"); + + process.env.CODEX_COMPANION_PLUGIN_DATA = codexPluginDataDir; + saveState(workspace, { + version: 1, + config: { stopReviewGate: false }, + jobs: [] + }); + + assert.deepEqual(JSON.parse(fs.readFileSync(siblingStateFile, "utf8")), siblingState); + assert.equal(fs.readFileSync(siblingJobFile, "utf8"), '{"owner":"sibling"}\n'); + assert.equal(fs.readFileSync(siblingLogFile, "utf8"), "sibling output\n"); + assert.equal(fs.existsSync(resolveStateFile(workspace)), true); + } finally { + if (previousCodexPluginDataDir == null) { + delete process.env.CODEX_COMPANION_PLUGIN_DATA; + } else { + process.env.CODEX_COMPANION_PLUGIN_DATA = previousCodexPluginDataDir; + } if (previousPluginDataDir == null) { delete process.env.CLAUDE_PLUGIN_DATA; } else {