From 11a37b31a298e1368c1d710d3815f05457f08bbb Mon Sep 17 00:00:00 2001 From: tanglearncode Date: Sat, 22 Aug 2026 07:02:17 +0800 Subject: [PATCH] fix(codex): show the context the session actually routed to MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `use_context` connected a context and `get_context` served it, and then `$neatcontext:status` and `$neatcontext:list`, run a second later, reported that nothing was connected at all. The two halves were reading different files. Codex exports CODEX_THREAD_ID to the processes it starts through its shell tool — the CLI a skill runs — and to nothing else. The MCP bridge is started with a scrubbed environment that no CODEX_* variable survives into, its MCP client advertises no `roots` and answers `roots/list` with an empty list, and its parent process is not the one the SessionStart hook is spawned from, so the pointer file keyed on the host process never joined them either. The bridge wrote the machine-wide selection; the CLI read the per-thread one; neither could see the disagreement. So the Codex adapter stops claiming a session identity it cannot give every one of its processes: one selection, one routing mode, one set of declines, shared by the bridge, the hook, and the skills. That is coarser than per-thread — two Codex windows share it — but it is what this host exposes, and a shared answer that is true beats a private one only half the plugin can see. The Copilot adapter refuses the same trade for the same reason. NEATCONTEXT_SESSION_ID is the way back for any host that can inject one id everywhere. The thread-drift machinery built on the old assumption goes with it: the pointer writes, the bridge's published session, and the drift warnings that could never fire. `pruneHostPointers` stays, to sweep the files older versions left behind. Co-Authored-By: Claude Opus 5 --- codex-marketplace/README.md | 34 +- .../plugins/neatcontext/.mcp.json | 3 - .../neatcontext/hooks/session-start.mjs | 44 +-- .../neatcontext/skills/disconnect/SKILL.md | 5 +- .../skills/disconnect/agents/openai.yaml | 4 +- .../plugins/neatcontext/skills/list/SKILL.md | 2 +- .../plugins/neatcontext/skills/mode/SKILL.md | 2 +- .../neatcontext/skills/status/SKILL.md | 2 +- .../plugins/neatcontext/skills/use/SKILL.md | 2 +- .../neatcontext/src/codex/mcp-bridge.mjs | 47 +-- .../neatcontext/src/codex/neatcontext-cli.mjs | 75 +--- .../plugins/neatcontext/src/codex/session.mjs | 86 +++-- .../neatcontext/src/core/host-session.mjs | 6 + codex-marketplace/tests/codex-plugin.test.mjs | 42 ++- tests/codex-host-session.test.mjs | 15 +- tests/codex-session-drift.test.mjs | 348 ------------------ tests/codex-session-scope.test.mjs | 333 +++++++++++++++++ 17 files changed, 482 insertions(+), 568 deletions(-) delete mode 100644 tests/codex-session-drift.test.mjs create mode 100644 tests/codex-session-scope.test.mjs diff --git a/codex-marketplace/README.md b/codex-marketplace/README.md index 09b18ed..f4f45c3 100644 --- a/codex-marketplace/README.md +++ b/codex-marketplace/README.md @@ -72,14 +72,14 @@ saved. Your next messages will use its domain profile and knowledge folder. The saved context keeps the investigation approach, system knowledge, findings, and verified resolution—not the raw conversation. -This thread had nothing connected, so saving also connected it — the work it -just wrote up is the work it is still doing. A thread that already has a context -connected keeps it, even when you save under a new name; use `$neatcontext:use` -when you actually want to switch. +Nothing was connected, so saving also connected what it just wrote — the work it +wrote up is the work it is still doing. When a context is already connected it +stays connected, even if you save under a new name; use `$neatcontext:use` when +you actually want to switch. After more work on the same subject, invoke `$neatcontext:save` again. With no -name it updates the context this thread is now connected to, previewing the -merged result and asking before applying it. +name it updates the connected context, previewing the merged result and asking +before applying it. When a similar issue appears later, connect the saved context in a new Codex thread with `$neatcontext:use`. NeatContext can also route you to the right @@ -125,15 +125,18 @@ later. ### `$neatcontext:use [name or number]` -Connect a context to the current Codex thread. +Connect a context to Codex. -Invoke the skill without a name to see the available choices. Each Codex thread -keeps its own connected context. +Invoke the skill without a name to see the available choices. One context is +connected at a time, and every part of the plugin sees the same one — the +skills you invoke, the routing the model does for itself, and the grounding it +loads. Codex does not tell the plugin's MCP server which thread it is serving, +so the connection is shared by the Codex threads and windows on this machine +rather than kept per thread; switch it whenever the subject changes. ### `$neatcontext:disconnect` -Disconnect the context from the current Codex thread. Other threads keep their -own connections, and the context itself is not deleted. +Disconnect the connected context. The context itself is not deleted. ### `$neatcontext:list` @@ -141,7 +144,7 @@ List all contexts you can connect. ### `$neatcontext:status` -Show the context connected to the current thread and the current routing mode. +Show the connected context and the current routing mode. It also reports problems such as missing context files or knowledge folders. @@ -206,15 +209,16 @@ is deleted with it. ### `$neatcontext:mode [auto|ask|manual]` -Choose how the current thread switches between contexts: +Choose how Codex switches between contexts: - `auto` — switch on a clear match and tell you; ask when the choice is unclear; this is the default - `ask` — ask before every switch, clear match or not - `manual` — switch only when you invoke `$neatcontext:use` -Invoke `$neatcontext:mode` without an argument to show the current mode. Add -`--global` to set the default for new threads: +Invoke `$neatcontext:mode` without an argument to show the current mode. Like +the connected context, the mode is shared by the Codex sessions on this +machine. `--global` sets the same default for every host that uses NeatContext: ```text $neatcontext:mode auto --global diff --git a/codex-marketplace/plugins/neatcontext/.mcp.json b/codex-marketplace/plugins/neatcontext/.mcp.json index 3c9c3e0..fa39369 100644 --- a/codex-marketplace/plugins/neatcontext/.mcp.json +++ b/codex-marketplace/plugins/neatcontext/.mcp.json @@ -5,9 +5,6 @@ "args": [ "./src/codex/mcp-bridge.mjs" ], - "env_vars": [ - "CODEX_THREAD_ID" - ], "cwd": "." } } diff --git a/codex-marketplace/plugins/neatcontext/hooks/session-start.mjs b/codex-marketplace/plugins/neatcontext/hooks/session-start.mjs index 7209db9..204da1f 100644 --- a/codex-marketplace/plugins/neatcontext/hooks/session-start.mjs +++ b/codex-marketplace/plugins/neatcontext/hooks/session-start.mjs @@ -2,22 +2,22 @@ // Profiles and knowledge are deliberately not injected here; get_context loads // only the selected context after routing has chosen it. // -// This hook is also the only moment anything in the plugin learns that `/new` -// happened. Codex starts a new thread inside the same process and does not -// restart the MCP server, so the bridge's `CODEX_THREAD_ID` still names the -// thread that just ended. The id delivered on stdin here is the current one, -// so it is recorded for the long-lived bridge to re-read — see -// src/core/host-session.mjs. +// The thread id Codex delivers on stdin is deliberately not used to scope +// anything. It would scope this hook and the skill-run CLI to a thread the MCP +// bridge cannot name, and the menu printed here would then describe a selection +// the bridge is not serving. See src/codex/session.mjs for what Codex does and +// does not expose. import { readSelection } from "../src/core/local-state.mjs"; -import { configureSessionId } from "../src/core/session.mjs"; -import { pruneHostPointers, writeHostPointer } from "../src/core/host-session.mjs"; +import "../src/codex/session.mjs"; +import { pruneHostPointers } from "../src/core/host-session.mjs"; import { menuEntries, readRouting, renderMenu, resolveMode } from "../src/core/routing.mjs"; +import { sessionId } from "../src/core/session.mjs"; import { listAllContexts } from "../src/core/selection.mjs"; async function readInput() { @@ -28,18 +28,14 @@ async function readInput() { return raw.trim().length > 0 ? JSON.parse(raw) : {}; } -const input = await readInput(); -const threadId = - typeof input.session_id === "string" && input.session_id.trim().length > 0 - ? input.session_id.trim() - : process.env.CODEX_THREAD_ID; +// Read and discard: Codex writes the hook payload to stdin and this hook has +// nothing left to take from it, but a reader that never drains it leaves the +// host writing into a pipe nobody empties. +await readInput().catch(() => ({})); -configureSessionId(() => threadId); - -// Tell the long-lived bridge which thread this host process is on now. Silent -// on failure: recording this must never delay or break the start of a thread. -// Startup is also the natural moment to sweep pointers whose host is gone. -await writeHostPointer(threadId, { source: "session-start" }).catch(() => undefined); +// Earlier versions of this plugin left one pointer file per host process behind. +// Nothing writes them now; sweeping the ones whose process is gone is what +// clears them off machines that ran those versions. await pruneHostPointers().catch(() => undefined); const [{ contexts }, state, selection] = await Promise.all([ @@ -47,7 +43,7 @@ const [{ contexts }, state, selection] = await Promise.all([ readRouting(), readSelection().catch(() => null) ]); -const mode = resolveMode(state, threadId); +const mode = resolveMode(state, sessionId()); const selected = selection?.available === false ? null : selection; const menu = renderMenu(menuEntries(contexts, state), { connectedId: selected?.contextId ?? null, @@ -55,15 +51,15 @@ const menu = renderMenu(menuEntries(contexts, state), { }); const groundingGuidance = selected - ? `The "${selected.contextName}" context is selected for this thread. For a request in its scope, call \`get_context\` only if its result is not already present since the latest context switch or compaction; otherwise reuse the existing result. Do not call \`get_context\` merely to check connection status.` + ? `The "${selected.contextName}" context is connected. For a request in its scope, call \`get_context\` only if its result is not already present since the latest context switch or compaction; otherwise reuse the existing result. Do not call \`get_context\` merely to check connection status.` : contexts.length > 0 - ? "No NeatContext context is selected for this thread. Do not call `get_context` to check connection status. Follow the routing menu, and load grounding only after `use_context` succeeds." + ? "No NeatContext context is connected. Do not call `get_context` to check connection status. Follow the routing menu, and load grounding only after `use_context` succeeds." : "No NeatContext contexts are currently available. Do not call `get_context`. Continue normal work without NeatContext grounding unless the user asks to create or import a context."; const guidance = [ - "NeatContext is installed for this Codex thread.", + "NeatContext is installed for this Codex session.", groundingGuidance, - "Connect or switch contexts inside this thread with `use_context` or the explicit `$neatcontext:use` skill. Disconnect the current context with `$neatcontext:disconnect`. There is no Desktop connection right now.", + "Connect or switch contexts from here with `use_context` or the explicit `$neatcontext:use` skill. Disconnect the current context with `$neatcontext:disconnect`. There is no Desktop connection right now.", menu, "Use `$neatcontext:save` to preserve durable work from the visible conversation. Never parse Codex transcript files for that workflow." ] diff --git a/codex-marketplace/plugins/neatcontext/skills/disconnect/SKILL.md b/codex-marketplace/plugins/neatcontext/skills/disconnect/SKILL.md index 4af7d19..941de5f 100644 --- a/codex-marketplace/plugins/neatcontext/skills/disconnect/SKILL.md +++ b/codex-marketplace/plugins/neatcontext/skills/disconnect/SKILL.md @@ -1,6 +1,6 @@ --- name: disconnect -description: Disconnect the NeatContext Context from the current Codex thread. Use when the user explicitly invokes this skill or asks to disconnect, detach, clear, or stop using the connected context. +description: Disconnect the connected NeatContext Context from Codex. Use when the user explicitly invokes this skill or asks to disconnect, detach, clear, or stop using the connected context. --- # Disconnect context @@ -12,5 +12,4 @@ file. Run: node "/src/codex/neatcontext-cli.mjs" disconnect ``` -Relay the output verbatim. Do not run a redundant status check. The command -affects only the current Codex thread. +Relay the output verbatim. Do not run a redundant status check. diff --git a/codex-marketplace/plugins/neatcontext/skills/disconnect/agents/openai.yaml b/codex-marketplace/plugins/neatcontext/skills/disconnect/agents/openai.yaml index 18af91a..142e335 100644 --- a/codex-marketplace/plugins/neatcontext/skills/disconnect/agents/openai.yaml +++ b/codex-marketplace/plugins/neatcontext/skills/disconnect/agents/openai.yaml @@ -1,7 +1,7 @@ interface: display_name: "Disconnect Context" - short_description: "Disconnect this Codex thread from its context" - default_prompt: "Use $neatcontext:disconnect to disconnect the context from this thread." + short_description: "Disconnect the connected context" + default_prompt: "Use $neatcontext:disconnect to disconnect the connected context." policy: allow_implicit_invocation: false diff --git a/codex-marketplace/plugins/neatcontext/skills/list/SKILL.md b/codex-marketplace/plugins/neatcontext/skills/list/SKILL.md index f2ae916..68b969e 100644 --- a/codex-marketplace/plugins/neatcontext/skills/list/SKILL.md +++ b/codex-marketplace/plugins/neatcontext/skills/list/SKILL.md @@ -1,6 +1,6 @@ --- name: list -description: List the local NeatContext Contexts available to the current Codex thread. Use when the user asks what contexts exist, what can be connected, or explicitly invokes this skill. +description: List the local NeatContext Contexts available to Codex. Use when the user asks what contexts exist, what can be connected, or explicitly invokes this skill. --- # List contexts diff --git a/codex-marketplace/plugins/neatcontext/skills/mode/SKILL.md b/codex-marketplace/plugins/neatcontext/skills/mode/SKILL.md index d30fcbc..be5426a 100644 --- a/codex-marketplace/plugins/neatcontext/skills/mode/SKILL.md +++ b/codex-marketplace/plugins/neatcontext/skills/mode/SKILL.md @@ -1,6 +1,6 @@ --- name: mode -description: Show or set NeatContext routing to auto, ask, or manual for the current Codex thread, with an optional global default for new threads. Use only when the user explicitly invokes this skill or clearly asks to change routing behavior. +description: Show or set NeatContext routing to auto, ask, or manual for Codex, with an optional default shared with every other NeatContext host. Use only when the user explicitly invokes this skill or clearly asks to change routing behavior. --- # Routing mode diff --git a/codex-marketplace/plugins/neatcontext/skills/status/SKILL.md b/codex-marketplace/plugins/neatcontext/skills/status/SKILL.md index 69cd89c..33346a6 100644 --- a/codex-marketplace/plugins/neatcontext/skills/status/SKILL.md +++ b/codex-marketplace/plugins/neatcontext/skills/status/SKILL.md @@ -1,6 +1,6 @@ --- name: status -description: Report the NeatContext context and routing mode active in the current Codex thread, including missing-file or stale-routing warnings. Use when the user asks which context is connected or explicitly invokes this skill. +description: Report the NeatContext context and routing mode active in Codex, including missing-file or stale-routing warnings. Use when the user asks which context is connected or explicitly invokes this skill. --- # Context status diff --git a/codex-marketplace/plugins/neatcontext/skills/use/SKILL.md b/codex-marketplace/plugins/neatcontext/skills/use/SKILL.md index a8eb4b6..a0ae282 100644 --- a/codex-marketplace/plugins/neatcontext/skills/use/SKILL.md +++ b/codex-marketplace/plugins/neatcontext/skills/use/SKILL.md @@ -1,6 +1,6 @@ --- name: use -description: Connect or switch this Codex thread to a local NeatContext Context by name or list number. Use only when the user explicitly invokes this skill, names a context to connect, or agrees to a routing suggestion. +description: Connect or switch Codex to a local NeatContext Context by name or list number. Use only when the user explicitly invokes this skill, names a context to connect, or agrees to a routing suggestion. --- # Use context diff --git a/codex-marketplace/plugins/neatcontext/src/codex/mcp-bridge.mjs b/codex-marketplace/plugins/neatcontext/src/codex/mcp-bridge.mjs index 2bf332b..c319b53 100644 --- a/codex-marketplace/plugins/neatcontext/src/codex/mcp-bridge.mjs +++ b/codex-marketplace/plugins/neatcontext/src/codex/mcp-bridge.mjs @@ -1,11 +1,12 @@ // NeatContext plugin MCP server for Codex. // +// Codex starts this process with an environment that carries no thread id and +// no way to derive one, so the selection it reads is the one every other part +// of the plugin reads — see src/codex/session.mjs. Nothing here tries to +// discover which thread it is serving: a bridge that scoped itself to a thread +// would answer from a file the skills cannot see. +// // Behaviors kept from the Claude Code bridge: -// * this process outlives the thread it was spawned in — /new starts a new -// one without restarting it — so the host session is re-resolved before -// every message rather than read once from the environment. Without that, -// the bridge goes on serving the pre-/new thread's context while the -// SessionStart hook and the skills write the new one's. // * initialize advertises tools.listChanged, and we poll the selected // context so the host refreshes its tool list when the user runs // $neatcontext:use (or the session routes itself). @@ -15,7 +16,7 @@ // get_context instead of silently vanishing. import readline from "node:readline"; -import { publishSessionId, refreshSessionId } from "./session.mjs"; +import "./session.mjs"; import { readSelection } from "../core/local-state.mjs"; import { CONTEXT_MISSING_MESSAGE, @@ -47,8 +48,8 @@ const GET_CONTEXT_TOOL = { title: "Get Context", description: "Load the domain profile and local knowledge pointers for the NeatContext Context " + - "already selected for this thread. Do not call merely to discover whether a Context " + - "is selected.", + "already connected in this Codex session. Do not call merely to discover whether a " + + "Context is connected.", inputSchema: { type: "object", properties: { @@ -80,7 +81,7 @@ const GET_CONTEXT_TOOL = { // are both locked. `$neatcontext:save` is the one that always opens: it builds // the first context out of the conversation already happening. So it leads when // there is nothing to connect. -const NOTHING_CONNECTED_HEAD = "No NeatContext Context is selected for this thread."; +const NOTHING_CONNECTED_HEAD = "No NeatContext Context is connected."; // The manual-mode version, and the fallback whenever no menu follows. Routing // is off here, so a command the user types is genuinely the only way forward. @@ -189,7 +190,7 @@ const ROUTING_TOOLS = new Map([ // get_context instead, which is re-read on every call and refreshed live by // tools/list_changed. These instructions do one job: get get_context called at // the right moments. -const CONTEXT_INSTRUCTIONS = `This Codex thread has a selected NeatContext Context: one domain profile and local knowledge stored on this machine. +const CONTEXT_INSTRUCTIONS = `This Codex session has a connected NeatContext Context: one domain profile and local knowledge stored on this machine. For a request in that Context's scope, call get_context only when its current result is not already present since the latest context switch or compaction; otherwise reuse the existing result. Never call get_context merely to check connection status. Read the profile in full when grounding is loaded. @@ -202,7 +203,7 @@ Cite the exact file path of anything you rely on. When the profile and the knowl // session or from another window on the same workspace. So this must never // state "nothing is connected" as a settled fact; it defers the current state // to get_context, which is the only thing that stays true. -const NO_CONTEXT_INSTRUCTIONS = `No NeatContext Context was selected when this thread started. A Context can be selected later with use_context. +const NO_CONTEXT_INSTRUCTIONS = `No NeatContext Context was connected when this session started. A Context can be connected later with use_context. Do not call get_context merely to check connection status. Continue normal work without NeatContext grounding until use_context succeeds or the user explicitly asks to refresh NeatContext state. @@ -514,17 +515,10 @@ let lastVersion = undefined; // What the host's tool list depends on. Switching between contexts has to // change this; so does the routing mode, because leaving manual has to make the // routing tools appear without waiting for a restart. -// Re-resolve which thread this process is serving, and publish the answer so a -// skill-run command can tell whether its write is the one this bridge will read. -async function syncSession() { - await refreshSessionId(); - await publishSessionId(); -} - async function currentVersion() { - // The session is part of it: `/new` changes what this process is grounded in - // without changing anything the selection or the mode can report, and the - // host has to be told to drop the previous thread's extension tools. + // The session is part of it for hosts that scope by one. Codex does not give + // this process a session to scope by, so here it is a constant and the + // selection below is what moves. const session = sessionId() ?? "none"; const mode = resolveMode(await readRouting(), sessionId()); const context = await activeContext(); @@ -539,11 +533,6 @@ async function currentVersion() { async function handleMessage(message) { const isNotification = message.id === undefined || message.id === null; - // Before anything reads a selection or a routing mode: which thread this - // host is on may have changed since the last message, and every one of those - // is per session. - await syncSession(); - // Routing tools decide which context serves the session next, so they are // answered before that choice is read. if (message.method === "tools/call" && ROUTING_TOOLS.has(message.params?.name)) { @@ -655,10 +644,8 @@ function startVersionWatch() { watching = true; setInterval(async () => { if (!started) return; - // The host does not send a message when the user runs `/new`, so this tick - // is where a thread change is noticed if nothing else asks first — and - // where the published answer stays fresh enough to be checked against. - await syncSession(); + // The host does not send a message when a skill connects a context, so this + // tick is where that is noticed if nothing else asks first. const version = await currentVersion(); if (version !== null && version !== lastVersion) { lastVersion = version; diff --git a/codex-marketplace/plugins/neatcontext/src/codex/neatcontext-cli.mjs b/codex-marketplace/plugins/neatcontext/src/codex/neatcontext-cli.mjs index 0878a94..abbf01e 100644 --- a/codex-marketplace/plugins/neatcontext/src/codex/neatcontext-cli.mjs +++ b/codex-marketplace/plugins/neatcontext/src/codex/neatcontext-cli.mjs @@ -22,7 +22,6 @@ import { readFile, rm } from "node:fs/promises"; import "./session.mjs"; import { clearSelection, readSelection } from "../core/local-state.mjs"; -import { awaitBridgeSession, readBridgeSession, writeHostPointer } from "../core/host-session.mjs"; import { createCapturedContext, createContext, @@ -68,49 +67,6 @@ function print(line = "") { process.stdout.write(`${line}\n`); } -// This process is spawned for one command and its environment is fresh, so the -// thread it names is the thread the user is actually in. The MCP bridge was -// spawned once, when the window opened, and cannot know that `/new` gave it a -// new one. Recording it here is what lets the bridge read the selection this -// command is about to write. -async function recordHostSession() { - return writeHostPointer(sessionId(), { source: "cli" }).catch(() => null); -} - -// Whether the bridge that will serve this thread has caught up with it. -// -// The success message below is written from the record this process just wrote, -// which is the one thing that cannot detect the failure this guards: a bridge -// reading a different thread's file would leave the message true about the disk -// and false about the thread. The bridge publishes what it resolved, so ask it. -// A bridge that publishes nothing (an older build, none running, or a bridge -// whose host key this shell-spawned process cannot compute) is not evidence of -// anything and gets no warning. -async function bridgeDriftWarning() { - const id = sessionId(); - if (!id) { - return null; - } - const { state } = await awaitBridgeSession(id); - if (state !== "drifted") { - return null; - } - return ( - "Warning: NeatContext's MCP server in this window is still serving an earlier " + - "thread and has not picked this one up, so `get_context` may keep returning the " + - "previous context. Restart Codex to clear it, and report it at " + - "https://github.com/XTSoftwareLabs/neatcontext-plugins/issues." - ); -} - -async function printBridgeDrift() { - const warning = await bridgeDriftWarning(); - if (warning) { - print(""); - print(warning); - } -} - // `--name value`, `--name=value`, and bare `--flag` booleans. function parseArgs(argv) { const flags = {}; @@ -184,18 +140,6 @@ async function loadState() { async function commandStatus(state) { const { connected, selection } = state; - // First, because it changes what everything below is about: if the bridge is - // on another thread, this is a report about a selection it is not reading. - const bridge = await readBridgeSession().catch(() => null); - if (bridge && sessionId() && bridge.sessionId !== sessionId()) { - print( - "Warning: NeatContext's MCP server in this window is serving an earlier thread " + - `(${bridge.sessionId}), not this one (${sessionId()}). What follows is what this ` + - "thread selected; `get_context` may still answer from the other one. Restart " + - "Codex to clear it." - ); - print(""); - } const routing = await readRouting(); const mode = resolveMode(routing, sessionId()); // Reported alongside the connection because the two together are the whole @@ -440,7 +384,6 @@ async function commandUse(state, query) { "will be grounded in its domain profile and knowledge folder." ); await nudgeForDescription(target); - await printBridgeDrift(); } async function commandDisconnect(state) { @@ -454,10 +397,7 @@ async function commandDisconnect(state) { await disconnectSelection(); const name = connected?.name ?? remembered.contextName; - print(`Disconnected the "${name}" context from this thread.`); - // Same exposure as connecting: a bridge on another thread clears nothing the - // thread it is serving can see. - await printBridgeDrift(); + print(`Disconnected the "${name}" context.`); } // A context with no routing description can only be routed to by name. @@ -544,10 +484,7 @@ async function commandMode(query, flags) { : `Context routing is now ${wanted} for this session.` ); if (wanted === "auto") { - print( - "In auto mode this session switches context on its own, and tells you when it does. " + - "Other Codex threads keep theirs." - ); + print("In auto mode Codex switches context on its own, and tells you when it does."); } } @@ -646,7 +583,6 @@ async function printSaveConnection(record) { "This session had no context connected, so it is now grounded in the one it " + "just saved. Your next messages will use its domain profile and knowledge folder." ); - await printBridgeDrift(); return; } print(`Use command: $neatcontext:use ${record.name}`); @@ -925,13 +861,6 @@ async function run() { return; } - // Before the commands that change what this thread is grounded in: the write - // is worthless if the bridge is still reading another thread's file, and this - // is the process that knows which thread that is. - if (command === "use" || command === "disconnect") { - await recordHostSession(); - } - const state = await loadState(); if (command === "status") { diff --git a/codex-marketplace/plugins/neatcontext/src/codex/session.mjs b/codex-marketplace/plugins/neatcontext/src/codex/session.mjs index bfed07f..780b209 100644 --- a/codex-marketplace/plugins/neatcontext/src/codex/session.mjs +++ b/codex-marketplace/plugins/neatcontext/src/codex/session.mjs @@ -1,53 +1,49 @@ // Codex host adapter for the reusable session-aware runtime. // -// `CODEX_THREAD_ID` is right for any process Codex spawns per event — the -// SessionStart hook, the CLI a skill runs. It is right for the MCP bridge too, -// for exactly as long as the thread it was spawned in lasts: `/new` starts a -// new thread inside the same host process, and the bridge's environment still -// names the old one. Reading it there is how a bridge ends up serving one -// thread's context to another. +// Codex does not hand this plugin a session identity that all of its processes +// can see, so this adapter deliberately claims none. // -// So the id is *resolved* rather than read: short-lived processes keep the -// environment, and the bridge asks `refreshSessionId()` before it handles -// anything, which lets the pointer written by the SessionStart hook correct the -// stale copy. See core/host-session.mjs. +// `CODEX_THREAD_ID` looks like the answer and is not. Codex exports it to the +// processes it starts through its shell tool — which is the CLI a skill runs, +// and nothing else. The MCP bridge is started with a scrubbed environment: the +// platform basics, plus whatever `.mcp.json` sets literally. No `CODEX_*` +// variable reaches it, and there is nowhere else for it to look. Codex's MCP +// client advertises no `roots` capability and answers `roots/list` with an +// empty list; a plugin server's `cwd` has to point inside the plugin, so it +// cannot even name the workspace it is serving; and hooks are spawned from a +// different parent process than the bridge, so a pointer file keyed on the +// host process never joins the two halves either. +// +// Scoping on a value only half the plugin can read is worse than not scoping at +// all. It splits the selection in silence: `use_context` writes the thread's +// file from the bridge, `$neatcontext:status` reads the machine's file from the +// CLI, and the user is told nothing is connected one line after being told a +// context was. The Copilot adapter refuses the same trade for the same reason. +// +// So one scope, shared by the bridge, the hook, and the CLI: one selection, one +// routing mode, one set of declines. Two Codex windows on a machine share them +// too, which is coarser than this plugin would like — but it is what the host +// currently exposes, and a shared answer that is true beats a private one that +// only one half of the plugin can see. +// +// NEATCONTEXT_SESSION_ID is the way back. A host that can inject one id into +// every one of its plugin processes — and any test that wants two sessions — +// gets per-session scoping again through it. import { configureSessionId } from "../core/session.mjs"; -import { publishBridgeSession, resolveHostSessionId } from "../core/host-session.mjs"; - -// When this process started. A pointer older than this cannot be describing a -// thread change that happened after it, and is therefore not about this host. -const STARTED_AT = Date.now(); - -function environmentThreadId() { - return process.env.CODEX_THREAD_ID; -} - -// Until something resolves it, the environment answers directly — which is what -// every process that is spawned per event wants, and what this file did before -// there was anything else to consult. -const UNRESOLVED = Symbol("unresolved"); -let resolved = UNRESOLVED; - -export function codexThreadId() { - return resolved === UNRESOLVED ? environmentThreadId() : resolved; -} - -// Re-resolves the thread this host process is on now. -// -// Synchronous everywhere else on purpose: `sessionId()` is called from inside -// path joins all over the runtime, and every one of them would have to become -// async to await this. The bridge serializes its messages, so refreshing once -// at the top of each is enough for all of them to agree. -export async function refreshSessionId() { - resolved = await resolveHostSessionId(environmentThreadId(), { since: STARTED_AT }); - return resolved; -} -// Publishes what this process resolved, so `$neatcontext:use` can verify its own -// success against the bridge instead of against the file it just wrote. -export async function publishSessionId() { - await publishBridgeSession(codexThreadId() ?? null); +// A session id becomes a path segment (`plugin-sessions/.json`), so anything +// that could climb out of that directory, or name the directory itself, is not +// a session id. +const SAFE_SESSION_ID = /^[A-Za-z0-9][A-Za-z0-9._-]{0,199}$/; + +export function codexSessionId() { + const explicit = process.env.NEATCONTEXT_SESSION_ID; + if (typeof explicit !== "string") { + return null; + } + const id = explicit.trim(); + return SAFE_SESSION_ID.test(id) ? id : null; } -configureSessionId(codexThreadId); +configureSessionId(codexSessionId); diff --git a/codex-marketplace/plugins/neatcontext/src/core/host-session.mjs b/codex-marketplace/plugins/neatcontext/src/core/host-session.mjs index 63bbe85..034b05c 100644 --- a/codex-marketplace/plugins/neatcontext/src/core/host-session.mjs +++ b/codex-marketplace/plugins/neatcontext/src/core/host-session.mjs @@ -1,5 +1,11 @@ // Which session the host process is on *right now*. // +// The Codex plugin no longer scopes anything by thread — Codex spawns its hooks +// and its MCP server from different parents, so the pointer below never joined +// the two halves, and src/codex/session.mjs explains what replaced it. This copy +// is kept in parity with the Claude Code one, which does use it, and `pruneHostPointers` +// still runs from the Codex SessionStart hook to clear the files older versions left. +// // A host that identifies its sessions through the environment has a problem the // rest of this plugin cannot see: the environment is a snapshot. Codex spawns // the MCP bridge once and keeps it for the life of the window, but the thread diff --git a/codex-marketplace/tests/codex-plugin.test.mjs b/codex-marketplace/tests/codex-plugin.test.mjs index 68d13fa..8df3077 100644 --- a/codex-marketplace/tests/codex-plugin.test.mjs +++ b/codex-marketplace/tests/codex-plugin.test.mjs @@ -106,8 +106,11 @@ test("marketplace and plugin manifests describe an isolated Codex package", asyn const mcp = JSON.parse(await readFile(path.join(pluginRoot, ".mcp.json"), "utf8")); assert.deepEqual(mcp.mcpServers.neatcontext.args, ["./src/codex/mcp-bridge.mjs"]); - assert.deepEqual(mcp.mcpServers.neatcontext.env_vars, ["CODEX_THREAD_ID"]); assert.equal(mcp.mcpServers.neatcontext.cwd, "."); + // Nothing is forwarded into the bridge: Codex scrubs the MCP environment, so + // a declared passthrough would only suggest the bridge knows which thread it + // is serving. It does not — see src/codex/session.mjs. + assert.equal(mcp.mcpServers.neatcontext.env_vars, undefined); }); test("all namespaced workflows are real skills without scaffold placeholders", async () => { @@ -137,24 +140,33 @@ test("all namespaced workflows are real skills without scaffold placeholders", a } }); -test("Codex CLI isolates routing by CODEX_THREAD_ID", async () => { +test("Codex routing settings are shared, not split by CODEX_THREAD_ID", async () => { + // CODEX_THREAD_ID reaches the CLI and never the MCP bridge, so scoping on it + // would leave the two halves enforcing different modes. It is ignored: what + // one Codex process sets, the next one reads. const home = await mkdtemp(path.join(os.tmpdir(), "neatcontext-codex-routing-")); const env = { NEATCONTEXT_HOME: home, CODEX_THREAD_ID: "thread-a" }; - const changed = await runNode(cli, ["mode", "auto"], { env }); + const changed = await runNode(cli, ["mode", "manual"], { env }); assert.equal(changed.code, 0); - assert.match(changed.stdout, /Other Codex threads keep theirs/); + assert.match(changed.stdout, /Context routing is now manual everywhere/); const current = await runNode(cli, ["mode"], { env }); - assert.match(current.stdout, /Context routing is auto \(this session\)/); + assert.match(current.stdout, /Context routing is manual/); const other = await runNode(cli, ["mode"], { env: { ...env, CODEX_THREAD_ID: "thread-b" } }); - assert.match(other.stdout, /Context routing is auto \(the default\)/); + assert.match(other.stdout, /Context routing is manual/); + + const back = await runNode(cli, ["mode", "auto"], { + env: { ...env, CODEX_THREAD_ID: "thread-b" } + }); + assert.match(back.stdout, /In auto mode Codex switches context on its own/); + assert.match((await runNode(cli, ["mode"], { env })).stdout, /Context routing is auto/); }); test("Codex saves conversation provenance without touching a transcript", async () => { @@ -227,10 +239,7 @@ test("Codex saves conversation provenance without touching a transcript", async assert.match(connected.stdout, /Connected the "Codex smoke context" context/); const disconnected = await runNode(cli, ["disconnect"], { env }); - assert.match( - disconnected.stdout, - /Disconnected the "Codex smoke context" context from this thread/ - ); + assert.match(disconnected.stdout, /Disconnected the "Codex smoke context" context/); const status = await runNode(cli, ["status"], { env }); assert.match(status.stdout, /No context is connected yet/); }); @@ -322,12 +331,13 @@ test("selected contexts advertise one-shot grounding guidance", async () => { }), "utf8" ); - // Saved from another thread on purpose: a save connects the thread it ran in, - // and this test needs "selected-thread" to start with nothing selected. + // A save connects a session that had nothing connected, and this test needs + // to start with nothing connected — so the save runs under a session id of + // its own, which is the one thing that scopes a Codex process separately. assert.equal( ( await runNode(cli, ["save", "--from", capturePath, "--consume"], { - env: { ...env, CODEX_THREAD_ID: "authoring-thread" } + env: { ...env, NEATCONTEXT_SESSION_ID: "authoring-session" } }) ).code, 0 @@ -346,14 +356,14 @@ test("selected contexts advertise one-shot grounding guidance", async () => { assert.equal(unselectedHookResult.code, 0); const unselectedHook = JSON.parse(unselectedHookResult.stdout).hookSpecificOutput.additionalContext; - assert.match(unselectedHook, /No NeatContext context is selected/); + assert.match(unselectedHook, /No NeatContext context is connected/); assert.match(unselectedHook, /load grounding only after `use_context` succeeds/); assert.equal((await runNode(cli, ["use", "Selected smoke context"], { env })).code, 0); const hookResult = await runNode(hook, [], { env, input: hookInput }); assert.equal(hookResult.code, 0); const hookOutput = JSON.parse(hookResult.stdout).hookSpecificOutput.additionalContext; - assert.match(hookOutput, /"Selected smoke context" context is selected/); + assert.match(hookOutput, /"Selected smoke context" context is connected/); assert.match(hookOutput, /otherwise reuse the existing result/); assert.match(hookOutput, /Do not call `get_context` merely/); @@ -371,7 +381,7 @@ test("selected contexts advertise one-shot grounding guidance", async () => { const listed = await rpc.call({ jsonrpc: "2.0", id: 2, method: "tools/list", params: {} }); const getContext = listed.result.tools.find((tool) => tool.name === "get_context"); assert.equal(getContext.annotations.readOnlyHint, true); - assert.match(getContext.description, /already selected for this thread/); + assert.match(getContext.description, /already connected in this Codex session/); assert.match(getContext.description, /Do not call merely/); } finally { await rpc.close(); diff --git a/tests/codex-host-session.test.mjs b/tests/codex-host-session.test.mjs index 0eac81b..f54eee2 100644 --- a/tests/codex-host-session.test.mjs +++ b/tests/codex-host-session.test.mjs @@ -1,8 +1,13 @@ -// The record that tells a long-lived Codex plugin process which thread its host -// is on now. Exercised directly here; the behavior it buys is in -// codex-session-drift.test.mjs. This is the Codex copy of the module — same -// mechanism as the Claude Code one, but its host key has no CLAUDE_PID -// equivalent to consult, only the explicit key and the parent process. +// The record that tells a long-lived plugin process which session its host is on +// now. This is the Codex copy of the module — same mechanism as the Claude Code +// one, but its host key has no CLAUDE_PID equivalent to consult, only the +// explicit key and the parent process. +// +// Codex itself no longer routes through it: its hooks and its MCP server are +// spawned from different parents, so the two never shared a key, and +// tests/codex-session-scope.test.mjs holds the plugin to one scope instead. What +// the Codex bundle still calls is `pruneHostPointers`, which clears the files +// older versions of the plugin left behind. import assert from "node:assert/strict"; import { mkdtemp, mkdir, readdir, rm, writeFile } from "node:fs/promises"; diff --git a/tests/codex-session-drift.test.mjs b/tests/codex-session-drift.test.mjs deleted file mode 100644 index 2b9f78b..0000000 --- a/tests/codex-session-drift.test.mjs +++ /dev/null @@ -1,348 +0,0 @@ -// Regression tests for thread drift in the Codex plugin: after `/new`, -// `$neatcontext:use` reports the new context connected while `get_context` -// keeps answering from the previous thread's one. -// -// The host process outlives the thread. Codex starts a new thread inside the -// same window and does not restart the MCP server, so the bridge keeps the -// `CODEX_THREAD_ID` it was spawned with while the SessionStart hook and the -// skill-run CLI see the new one. The two halves then read and write different -// files, and nothing in either path can notice. -// -// What is simulated here is exactly that and nothing else: one long-lived -// bridge process, CLI commands spawned per command, and the SessionStart hook -// Codex runs when the thread changes underneath them. - -import assert from "node:assert/strict"; -import { spawn } from "node:child_process"; -import { mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; -import os from "node:os"; -import readline from "node:readline"; -import path from "node:path"; -import { fileURLToPath } from "node:url"; -import { after, before, beforeEach, describe, it } from "node:test"; -import { closeSession } from "./process-helpers.mjs"; - -const plugin = path.join( - path.dirname(fileURLToPath(import.meta.url)), - "..", - "codex-marketplace", - "plugins", - "neatcontext" -); -const codex = path.join(plugin, "src", "codex"); - -const HOST = "codex-window"; -const OTHER_HOST = "codex-window-2"; - -let home; -let hostsDirectory; - -const childEnv = (threadId, host = HOST) => ({ - ...process.env, - CODEX_THREAD_ID: threadId, - // Stands in for the host process id the plugin keys on, so the test controls - // which "window" each child belongs to. - NEATCONTEXT_HOST_KEY: host, - NEATCONTEXT_HOME: home -}); - -before(async () => { - home = await mkdtemp(path.join(os.tmpdir(), "neatcontext-codex-drift-")); - hostsDirectory = path.join(home, "plugin-hosts"); - const docs = path.join(home, "docs"); - await mkdir(docs, { recursive: true }); - await writeFile(path.join(docs, "payments.md"), "# Payments\n"); - // The contexts a thread could connect. On disk once, like a user's: what the - // tests vary is which thread is connected to them, never the store itself. - process.env.NEATCONTEXT_HOME = home; - const store = await import( - "../codex-marketplace/plugins/neatcontext/src/core/context-store.mjs" - ); - for (const name of ["payment team", "Dokploy"]) { - await store.createContext({ - name, - knowledgeFolder: docs, - profile: `# ${name}\n\n## Purpose\nQuestions about ${name}.` - }); - } -}); -after(async () => { - await rm(home, { recursive: true, force: true }); -}); -beforeEach(async () => { - await rm(hostsDirectory, { recursive: true, force: true }); - await rm(path.join(home, "plugin-sessions"), { recursive: true, force: true }); - await rm(path.join(home, "plugin-selection.json"), { force: true }); - await rm(path.join(home, "plugin-routing.json"), { force: true }); -}); - -// A skill-run command: spawned per invocation, with the environment of the -// thread the user is in right now. -function cli(threadId, ...args) { - return new Promise((resolve) => { - const child = spawn(process.execPath, [path.join(codex, "neatcontext-cli.mjs"), ...args], { - stdio: ["ignore", "pipe", "inherit"], - env: childEnv(threadId) - }); - let out = ""; - child.stdout.on("data", (chunk) => (out += chunk)); - child.on("exit", () => resolve(out.trim())); - }); -} - -// What Codex runs when a thread starts — including the new one `/new` creates -// in the window that is already open. -function sessionStart(threadId, source = "clear", host = HOST) { - return new Promise((resolve) => { - const child = spawn(process.execPath, [path.join(plugin, "hooks", "session-start.mjs")], { - stdio: ["pipe", "pipe", "inherit"], - env: childEnv(threadId, host) - }); - let out = ""; - child.stdout.on("data", (chunk) => (out += chunk)); - child.on("exit", () => resolve(out.trim())); - child.stdin.end( - JSON.stringify({ session_id: threadId, source, hook_event_name: "SessionStart" }) - ); - }); -} - -// A window: one bridge process kept alive across threads, as Codex keeps it. -// `threadId` is what it was spawned with, and is never updated afterwards — -// that is the whole point. -function openWindow(threadId, host = HOST) { - const child = spawn(process.execPath, [path.join(codex, "mcp-bridge.mjs")], { - stdio: ["pipe", "pipe", "inherit"], - env: childEnv(threadId, host) - }); - const waiters = new Map(); - const notifications = []; - readline.createInterface({ input: child.stdout }).on("line", (line) => { - if (!line.trim()) return; - const message = JSON.parse(line); - if (message.id != null && waiters.has(message.id)) { - waiters.get(message.id)(message); - waiters.delete(message.id); - } else { - notifications.push(message); - } - }); - let nextId = 1; - const send = (method, params) => - new Promise((resolve) => { - const id = nextId++; - waiters.set(id, resolve); - child.stdin.write( - `${JSON.stringify({ jsonrpc: "2.0", id, method, ...(params ? { params } : {}) })}\n` - ); - }); - return { - pid: child.pid, - notifications, - send, - async handshake() { - const response = await send("initialize", { - protocolVersion: "2025-06-18", - capabilities: {}, - clientInfo: { name: "test", version: "1" } - }); - child.stdin.write( - `${JSON.stringify({ jsonrpc: "2.0", method: "notifications/initialized" })}\n` - ); - return response; - }, - grounding: async () => - (await send("tools/call", { name: "get_context", arguments: {} })).result.content[0].text, - close: () => closeSession(child) - }; -} - -const readJson = async (file) => JSON.parse(await readFile(file, "utf8")); - -async function writeBridgeRecord(threadId, { pid = process.pid, host = HOST } = {}) { - await mkdir(hostsDirectory, { recursive: true }); - await writeFile( - path.join(hostsDirectory, `${host}.bridge.json`), - JSON.stringify({ pid, sessionId: threadId, updatedAt: new Date().toISOString() }) - ); -} - -async function waitFor(predicate, { timeoutMs = 6000 } = {}) { - const deadline = Date.now() + timeoutMs; - for (;;) { - if (await predicate()) return true; - if (Date.now() >= deadline) return false; - await new Promise((resolve) => setTimeout(resolve, 50)); - } -} - -describe("a thread that is replaced under a running Codex bridge", () => { - it("stops serving the previous thread's context", async () => { - const window = openWindow("thread-a"); - try { - await window.handshake(); - await cli("thread-a", "use", "payment", "team"); - assert.match(await window.grounding(), /connected context: payment team/); - - // `/new`: a new thread, the same window, the same bridge process. - await sessionStart("thread-b"); - - const answer = await window.grounding(); - // "payment team" still appears in the routing menu, as one of the contexts - // this thread could connect. What must be gone is it being served. - assert.doesNotMatch(answer, /connected context: payment team/); - assert.match(answer, /No NeatContext Context is selected for this thread/); - } finally { - await window.close(); - } - }); - - it("serves the context the new thread connects", async () => { - const window = openWindow("thread-a"); - try { - await window.handshake(); - await cli("thread-a", "use", "payment", "team"); - await sessionStart("thread-b"); - - assert.match(await cli("thread-b", "use", "Dokploy"), /Connected the "Dokploy" context/); - assert.match(await window.grounding(), /connected context: Dokploy/); - } finally { - await window.close(); - } - }); - - it("tells the host its tool list changed, without being asked anything", async () => { - const window = openWindow("thread-a"); - try { - await window.handshake(); - await cli("thread-a", "use", "payment", "team"); - await window.grounding(); - window.notifications.length = 0; - - await sessionStart("thread-b"); - - const changed = await waitFor(async () => - window.notifications.some( - (message) => message.method === "notifications/tools/list_changed" - ) - ); - assert.ok(changed, "the bridge never announced that its tool list had changed"); - } finally { - await window.close(); - } - }); - - it("leaves the other windows on the machine alone", async () => { - const first = openWindow("thread-a", HOST); - const second = openWindow("other-thread", OTHER_HOST); - try { - await first.handshake(); - await second.handshake(); - await cli("thread-a", "use", "payment", "team"); - await cli("other-thread", "use", "Dokploy"); - - await sessionStart("thread-b", "clear", HOST); - - assert.match(await first.grounding(), /No NeatContext Context is selected/); - assert.match(await second.grounding(), /connected context: Dokploy/); - } finally { - await first.close(); - await second.close(); - } - }); - - it("still emits its routing guidance after recording the thread", async () => { - await cli("thread-a", "use", "payment", "team"); - - const output = await sessionStart("thread-b"); - const { hookSpecificOutput } = JSON.parse(output); - assert.equal(hookSpecificOutput.hookEventName, "SessionStart"); - // The new thread inherits no selection, and the guidance must say so. - assert.match(hookSpecificOutput.additionalContext, /No NeatContext context is selected/); - - const pointer = await readJson(path.join(hostsDirectory, `${HOST}.json`)); - assert.equal(pointer.sessionId, "thread-b"); - assert.equal(pointer.source, "session-start"); - }); -}); - -describe("a Codex bridge deciding which thread it is on", () => { - it("publishes what it resolved, so a skill-run command can check it", async () => { - const window = openWindow("thread-a"); - try { - await window.handshake(); - const record = await readJson(path.join(hostsDirectory, `${HOST}.bridge.json`)); - assert.equal(record.sessionId, "thread-a"); - assert.equal(record.pid, window.pid); - } finally { - await window.close(); - } - }); - - it("ignores a pointer left behind by an earlier host with the same key", async () => { - // A pid is reused eventually. A record older than this process cannot be - // describing a change that happened after it started. - await mkdir(hostsDirectory, { recursive: true }); - await writeFile( - path.join(hostsDirectory, `${HOST}.json`), - JSON.stringify({ - sessionId: "long-gone-thread", - source: "session-start", - updatedAt: "2001-01-01T00:00:00.000Z" - }) - ); - await cli("thread-a", "use", "payment", "team"); - - const window = openWindow("thread-a"); - try { - await window.handshake(); - assert.match(await window.grounding(), /connected context: payment team/); - } finally { - await window.close(); - } - }); -}); - -describe("what $neatcontext:use claims", () => { - it("warns when the bridge has not picked this thread up", async () => { - // A bridge stuck on another thread: the selection lands on disk and the - // process that serves the thread never reads it. - await writeBridgeRecord("some-other-thread"); - - const output = await cli("thread-a", "use", "payment", "team"); - assert.match(output, /Connected the "payment team" context/); - assert.match(output, /still serving an earlier thread/); - }); - - it("stays quiet when the bridge is on the same thread", async () => { - await writeBridgeRecord("thread-a"); - - const output = await cli("thread-a", "use", "payment", "team"); - assert.match(output, /Connected the "payment team" context/); - assert.doesNotMatch(output, /still serving an earlier thread/); - }); - - it("stays quiet when no bridge is publishing anything to check against", async () => { - const output = await cli("thread-a", "use", "payment", "team"); - assert.match(output, /Connected the "payment team" context/); - assert.doesNotMatch(output, /still serving an earlier thread/); - }); - - it("reports the drift from $neatcontext:status too", async () => { - await cli("thread-a", "use", "payment", "team"); - await writeBridgeRecord("some-other-thread"); - - const status = await cli("thread-a", "status"); - assert.match(status, /serving an earlier thread \(some-other-thread\), not this one/); - assert.match(status, /Connected context: payment team/); - }); - - it("ignores a record from a bridge that is no longer running", async () => { - // Above Linux's pid ceiling and not a multiple of four, which Windows pids - // are: no platform can have handed this one out. - await writeBridgeRecord("some-other-thread", { pid: 2147483647 }); - - const output = await cli("thread-a", "use", "payment", "team"); - assert.doesNotMatch(output, /still serving an earlier thread/); - }); -}); diff --git a/tests/codex-session-scope.test.mjs b/tests/codex-session-scope.test.mjs new file mode 100644 index 0000000..66c7445 --- /dev/null +++ b/tests/codex-session-scope.test.mjs @@ -0,0 +1,333 @@ +// One scope for every Codex process, and what happens when the host tries to +// split it. +// +// Codex hands its plugin processes very different things. A skill runs through +// the shell tool, which exports CODEX_THREAD_ID. The SessionStart hook is given +// the thread id on stdin. The MCP bridge gets neither: Codex starts it with a +// scrubbed environment, no CODEX_* variable survives into it, its MCP client +// offers no `roots`, and its parent process is not the one the hook was spawned +// from — so no pointer file keyed on the host process joins the two halves. +// +// Scope the selection on any of that and the plugin splits in silence: +// `use_context` connects a context in the bridge and `$neatcontext:status`, run +// a second later, reports that nothing is connected. These tests spawn the +// three process kinds the way Codex spawns them — different environments, +// different host keys — and hold them to one answer. + +import assert from "node:assert/strict"; +import { spawn } from "node:child_process"; +import { mkdir, mkdtemp, readdir, rm, writeFile } from "node:fs/promises"; +import os from "node:os"; +import readline from "node:readline"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { after, before, beforeEach, describe, it } from "node:test"; +import { closeSession } from "./process-helpers.mjs"; + +const plugin = path.join( + path.dirname(fileURLToPath(import.meta.url)), + "..", + "codex-marketplace", + "plugins", + "neatcontext" +); +const codex = path.join(plugin, "src", "codex"); + +// The host keys stand in for what `process.ppid` resolves to in each process. +// They differ on purpose: on Codex they really are different processes, and a +// plugin that only works when they agree does not work. +const BRIDGE_HOST = "codex-window"; +const HOOK_HOST = "codex-hook-runner"; +const SHELL_HOST = "codex-shell"; +const THREAD = "01a02647-61ec-76a1-9571-ccfb40c1b415"; + +let home; +let hostsDirectory; + +function childEnv({ thread = null, host, sessionId = null } = {}) { + const env = { ...process.env, NEATCONTEXT_HOME: home, NEATCONTEXT_HOST_KEY: host }; + delete env.CODEX_THREAD_ID; + delete env.NEATCONTEXT_SESSION_ID; + if (thread) env.CODEX_THREAD_ID = thread; + if (sessionId) env.NEATCONTEXT_SESSION_ID = sessionId; + return env; +} + +before(async () => { + home = await mkdtemp(path.join(os.tmpdir(), "neatcontext-codex-scope-")); + hostsDirectory = path.join(home, "plugin-hosts"); + const docs = path.join(home, "docs"); + await mkdir(docs, { recursive: true }); + await writeFile(path.join(docs, "payments.md"), "# Payments\n"); + process.env.NEATCONTEXT_HOME = home; + const store = await import( + "../codex-marketplace/plugins/neatcontext/src/core/context-store.mjs" + ); + for (const name of ["payment team", "Dokploy"]) { + await store.createContext({ + name, + knowledgeFolder: docs, + profile: `# ${name}\n\n## Purpose\nQuestions about ${name}.` + }); + } +}); +after(async () => { + await rm(home, { recursive: true, force: true }); +}); +beforeEach(async () => { + await rm(hostsDirectory, { recursive: true, force: true }); + await rm(path.join(home, "plugin-sessions"), { recursive: true, force: true }); + await rm(path.join(home, "plugin-selection.json"), { force: true }); + await rm(path.join(home, "plugin-routing.json"), { force: true }); +}); + +// A skill: Codex runs it through the shell tool, so it is the one process that +// is handed the thread id. +function cli(...args) { + const options = typeof args.at(-1) === "object" ? args.pop() : {}; + return new Promise((resolve) => { + const child = spawn(process.execPath, [path.join(codex, "neatcontext-cli.mjs"), ...args], { + stdio: ["ignore", "pipe", "inherit"], + env: childEnv({ thread: THREAD, host: SHELL_HOST, ...options }) + }); + let out = ""; + child.stdout.on("data", (chunk) => (out += chunk)); + child.on("exit", () => resolve(out.trim())); + }); +} + +// What Codex runs when a thread starts, resumes, or compacts. It is told the +// thread id on stdin and spawned from its own parent. +function sessionStart(threadId, source = "startup") { + return new Promise((resolve) => { + const child = spawn(process.execPath, [path.join(plugin, "hooks", "session-start.mjs")], { + stdio: ["pipe", "pipe", "inherit"], + env: childEnv({ host: HOOK_HOST }) + }); + let out = ""; + child.stdout.on("data", (chunk) => (out += chunk)); + child.on("exit", () => resolve(out.trim())); + child.stdin.end( + JSON.stringify({ session_id: threadId, source, hook_event_name: "SessionStart" }) + ); + }); +} + +// The MCP bridge: one per window, started with nothing that names a thread. +function openWindow({ sessionId = null } = {}) { + const child = spawn(process.execPath, [path.join(codex, "mcp-bridge.mjs")], { + stdio: ["pipe", "pipe", "inherit"], + env: childEnv({ host: BRIDGE_HOST, sessionId }) + }); + const waiters = new Map(); + const notifications = []; + readline.createInterface({ input: child.stdout }).on("line", (line) => { + if (!line.trim()) return; + const message = JSON.parse(line); + if (message.id != null && waiters.has(message.id)) { + waiters.get(message.id)(message); + waiters.delete(message.id); + } else { + notifications.push(message); + } + }); + let nextId = 1; + const send = (method, params) => + new Promise((resolve) => { + const id = nextId++; + waiters.set(id, resolve); + child.stdin.write( + `${JSON.stringify({ jsonrpc: "2.0", id, method, ...(params ? { params } : {}) })}\n` + ); + }); + return { + pid: child.pid, + notifications, + async handshake() { + const response = await send("initialize", { + protocolVersion: "2025-06-18", + capabilities: {}, + clientInfo: { name: "test", version: "1" } + }); + child.stdin.write( + `${JSON.stringify({ jsonrpc: "2.0", method: "notifications/initialized" })}\n` + ); + return response; + }, + call: async (name, args = {}) => + (await send("tools/call", { name, arguments: args })).result.content[0].text, + grounding: async () => + (await send("tools/call", { name: "get_context", arguments: {} })).result.content[0].text, + close: () => closeSession(child) + }; +} + +async function waitFor(predicate, { timeoutMs = 6000 } = {}) { + const deadline = Date.now() + timeoutMs; + for (;;) { + if (await predicate()) return true; + if (Date.now() >= deadline) return false; + await new Promise((resolve) => setTimeout(resolve, 50)); + } +} + +describe("a context the session routes itself to", () => { + it("is the context the skills report", async () => { + const window = openWindow(); + try { + await window.handshake(); + assert.match( + await window.call("use_context", { context: "payment team", reason: "test" }), + /Switched this session to "payment team"/ + ); + + // The bug this file exists for: connected in the bridge, invisible to + // every command the user can run. + assert.match(await cli("status"), /Connected context: payment team/); + assert.match(await cli("list"), /payment team\s+\(connected\)/); + } finally { + await window.close(); + } + }); + + it("is still the one the skills report after the thread changes", async () => { + const window = openWindow(); + try { + await window.handshake(); + await window.call("use_context", { context: "payment team", reason: "test" }); + + // `/new`: a new thread id, delivered to the hook and to every later + // skill. Nothing about it can move the selection out from under the + // bridge, because nothing is scoped to it. + await sessionStart("01a0300f-0000-7000-8000-00000000beef", "clear"); + + assert.match(await window.grounding(), /connected context: payment team/); + assert.match( + await cli("status", { thread: "01a0300f-0000-7000-8000-00000000beef" }), + /Connected context: payment team/ + ); + } finally { + await window.close(); + } + }); +}); + +describe("a context a skill connects", () => { + it("is what the bridge serves", async () => { + const window = openWindow(); + try { + await window.handshake(); + assert.match(await cli("use", "Dokploy"), /Connected the "Dokploy" context/); + assert.match(await window.grounding(), /connected context: Dokploy/); + + assert.match(await cli("disconnect"), /Disconnected the "Dokploy" context/); + assert.match(await window.grounding(), /No NeatContext Context is connected/); + } finally { + await window.close(); + } + }); + + it("makes the bridge tell the host its tool list changed", async () => { + const window = openWindow(); + try { + await window.handshake(); + await window.grounding(); + window.notifications.length = 0; + + await cli("use", "payment", "team"); + + const announced = await waitFor(() => + window.notifications.some( + (message) => message.method === "notifications/tools/list_changed" + ) + ); + assert.ok(announced, "the bridge never announced that its tool list had changed"); + } finally { + await window.close(); + } + }); + + it("is named by the routing menu the hook re-injects", async () => { + await cli("use", "payment", "team"); + + const { hookSpecificOutput } = JSON.parse(await sessionStart(THREAD)); + assert.equal(hookSpecificOutput.hookEventName, "SessionStart"); + assert.match(hookSpecificOutput.additionalContext, /The "payment team" context is connected/); + assert.doesNotMatch(hookSpecificOutput.additionalContext, /No NeatContext context is connected/); + }); +}); + +describe("the routing mode a skill sets", () => { + it("is the mode the bridge enforces", async () => { + assert.match(await cli("mode", "manual"), /Context routing is now manual/); + + const window = openWindow(); + try { + await window.handshake(); + assert.match( + await window.call("use_context", { context: "payment team", reason: "test" }), + /Context routing is off \(manual mode\)/ + ); + assert.match(await cli("status"), /No context is connected yet/); + } finally { + await window.close(); + } + }); +}); + +describe("a pointer file left by an older version of this plugin", () => { + it("does not re-scope the bridge onto a thread the skills cannot see", async () => { + await mkdir(hostsDirectory, { recursive: true }); + await writeFile( + path.join(hostsDirectory, `${BRIDGE_HOST}.json`), + JSON.stringify({ + sessionId: THREAD, + source: "session-start", + updatedAt: new Date().toISOString() + }) + ); + + const window = openWindow(); + try { + await window.handshake(); + await window.call("use_context", { context: "payment team", reason: "test" }); + assert.match(await cli("status"), /Connected context: payment team/); + // Nothing was written per thread, so nothing can be read per thread. + assert.deepEqual(await readdir(path.join(home, "plugin-sessions")).catch(() => []), []); + } finally { + await window.close(); + } + }); + + it("is swept by the hook once its process is gone", async () => { + await mkdir(hostsDirectory, { recursive: true }); + // Above Linux's pid ceiling and not a multiple of four, which Windows pids + // are: no platform can have handed this one out. + await writeFile(path.join(hostsDirectory, "pid-2147483647.json"), JSON.stringify({})); + + await sessionStart(THREAD); + + assert.deepEqual(await readdir(hostsDirectory).catch(() => []), []); + }); +}); + +describe("a host that can name a session in every one of its processes", () => { + it("gets its selection scoped to that session", async () => { + const session = "explicit-session"; + const window = openWindow({ sessionId: session }); + try { + await window.handshake(); + await window.call("use_context", { context: "payment team", reason: "test" }); + + assert.match( + await cli("status", { sessionId: session }), + /Connected context: payment team/ + ); + assert.deepEqual(await readdir(path.join(home, "plugin-sessions")), [`${session}.json`]); + // A different session of that host keeps its own. + assert.match(await cli("status", { sessionId: "another-session" }), /No context is connected/); + } finally { + await window.close(); + } + }); +});