From 1ce374390fff3af9cd9cff116669460686a46d7d Mon Sep 17 00:00:00 2001 From: Jingyu Ma Date: Sun, 9 Aug 2026 10:46:07 -0700 Subject: [PATCH 1/6] fix(copilot): serve the session the user is in, not the one the bridge started in `/neatcontext:use` reported a context connected while `get_context` kept answering from a different one. No error anywhere: both halves were telling the truth about different files. The Copilot adapter derived a session by hashing `process.cwd()`, on the premise that every plugin process is handed the workspace. That premise is false. Copilot spawns the MCP bridge with the plugin installation directory and the slash-command CLI with the user's workspace, so the two halves hashed different paths and scoped to different selection files. Copilot does publish a session identity to both -- COPILOT_AGENT_SESSION_ID, and COPILOT_LOADER_PID for the host process -- so a session here is now a session, as on every other host, with the workspace digest kept as the fallback for a build that publishes neither. The mechanism for carrying a session across a process that outlives it already existed for two hosts and was copied into each. It is promoted into `shared/core/` and given `configureHostPid()`, so each adapter names its own host's pid instead of core knowing every host's variable -- those variables are inherited by child processes, and a shared list would let one host key on another. Codex keeps `process.ppid` unchanged. Where the two halves can still fail to agree, they say so: a drift warning when a live bridge is serving another session, an upgrade hint naming the connection a workspace had before selections were per session, and a warning when the host publishes no identity at all rather than degrading in silence. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../plugins/neatcontext/src/codex/session.mjs | 12 +- .../neatcontext/src/core/host-session.mjs | 117 +++-- package.json | 6 +- .../neatcontext/hooks/session-start.mjs | 4 + .../claude-code/neatcontext/hooks/stop.mjs | 4 + .../neatcontext/src/claude/session.mjs | 12 +- .../neatcontext/src/core/host-session.mjs | 110 +++-- plugins/copilot/neatcontext/README.md | 18 +- .../neatcontext/commands/disconnect.md | 2 +- .../neatcontext/src/copilot/mcp-bridge.mjs | 40 +- .../src/copilot/neatcontext-cli.mjs | 156 ++++++- .../neatcontext/src/copilot/session.mjs | 136 +++++- .../neatcontext/src/core/host-session.mjs | 345 +++++++++++++++ .../neatcontext/src/core/host-session.mjs | 345 +++++++++++++++ plugins/pi/neatcontext/package.json | 2 +- .../pi/neatcontext/src/core/host-session.mjs | 345 +++++++++++++++ shared/core/host-session.mjs | 345 +++++++++++++++ tests/copilot-plugin.test.mjs | 15 +- tests/copilot-session-drift.test.mjs | 418 ++++++++++++++++++ tests/host-session.test.mjs | 26 ++ tools/sync-context-core.mjs | 1 + 21 files changed, 2362 insertions(+), 97 deletions(-) create mode 100644 plugins/copilot/neatcontext/src/core/host-session.mjs create mode 100644 plugins/kimi-code/neatcontext/src/core/host-session.mjs create mode 100644 plugins/pi/neatcontext/src/core/host-session.mjs create mode 100644 shared/core/host-session.mjs create mode 100644 tests/copilot-session-drift.test.mjs diff --git a/codex-marketplace/plugins/neatcontext/src/codex/session.mjs b/codex-marketplace/plugins/neatcontext/src/codex/session.mjs index bfed07f..1da430c 100644 --- a/codex-marketplace/plugins/neatcontext/src/codex/session.mjs +++ b/codex-marketplace/plugins/neatcontext/src/codex/session.mjs @@ -13,7 +13,17 @@ // stale copy. See core/host-session.mjs. import { configureSessionId } from "../core/session.mjs"; -import { publishBridgeSession, resolveHostSessionId } from "../core/host-session.mjs"; +import { + configureHostPid, + publishBridgeSession, + resolveHostSessionId +} from "../core/host-session.mjs"; + +// Codex publishes no pid of its own, so the pointer file is keyed on +// `process.ppid` — which is Codex itself for the bridge and for the hooks and +// CLI it spawns. Said explicitly rather than left to the default so that a +// future reader knows it was considered. +configureHostPid(null); // 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. diff --git a/codex-marketplace/plugins/neatcontext/src/core/host-session.mjs b/codex-marketplace/plugins/neatcontext/src/core/host-session.mjs index 63bbe85..5e863d2 100644 --- a/codex-marketplace/plugins/neatcontext/src/core/host-session.mjs +++ b/codex-marketplace/plugins/neatcontext/src/core/host-session.mjs @@ -1,40 +1,42 @@ // Which session the host process is on *right now*. // // 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 -// changes underneath it — `/new` starts a new thread in the same process — and -// the bridge's copy of `CODEX_THREAD_ID` is whatever it was at spawn. The -// SessionStart hook and the skill-run CLI are spawned fresh and see the new one. +// rest of this plugin cannot see: the environment is a snapshot. Hosts spawn the +// MCP bridge once and keep it for the life of the window, but the session +// changes underneath it — starting a fresh conversation does not restart the +// server — and the bridge's copy of the id is whatever it was at spawn. The +// hooks and the slash-command CLI are spawned fresh and see the new one. // // So the two halves of the plugin end up reading and writing different files: -// `$neatcontext:use` writes the selection for the thread the user is in, the -// bridge keeps serving the thread the window started in, and nothing in either -// path can observe the disagreement. The user is told a context is connected and -// every later answer is grounded in a different one. +// `use` writes the selection for the session the user is in, the bridge keeps +// serving the session the window started in, and nothing in either path can +// observe the disagreement. The user is told a context is connected and every +// later answer is grounded in a different one. // // This module is the missing channel. One pointer file per host *process*, -// written by whichever short-lived process was just handed the current thread +// written by whichever short-lived process was just handed the current session // id, and re-read by the long-lived one on every message: // // ~/.neatcontext/plugin-hosts/.json { sessionId, source, updatedAt } // -// The host key has to be stable across a thread change and distinct per window, -// which rules out the thread id and the working directory both. What is left is -// the host process itself: the bridge and the SessionStart hook are spawned by -// Codex directly, so `process.ppid` is the same number in both. Two windows are -// two pids and never share a pointer. A process whose parent is not the host — -// the CLI runs inside Codex's shell tool — computes a key nothing else reads, -// and its writes are harmless and swept later. +// The host key has to be stable across a session change and distinct per window, +// which rules out the session id and the working directory both. What is left is +// the host process itself. How to name it is the one thing that differs per +// host, so each adapter registers its own answer with `configureHostPid()`: some +// hosts publish their own pid to every process they spawn, and a host that +// publishes nothing falls back to `process.ppid`, which is the host for any +// process it spawned directly. A process whose parent is not the host and whose +// host publishes no pid — a CLI running inside a shell tool — computes a key +// nothing else reads, and its writes are harmless and swept later. // // The bridge also publishes what it actually resolved: // // ~/.neatcontext/plugin-hosts/.bridge.json { pid, sessionId, updatedAt } // -// That is what lets `$neatcontext:use` check its own success against the process -// that will serve it, instead of against the file it just wrote itself. +// That is what lets `use` check its own success against the process that will +// serve it, instead of against the file it just wrote itself. -import { mkdir, readFile, readdir, rm, writeFile } from "node:fs/promises"; +import { mkdir, readFile, readdir, rename, rm, writeFile } from "node:fs/promises"; import path from "node:path"; import { neatContextHome } from "./storage-home.mjs"; @@ -60,14 +62,45 @@ function pidKey(value) { return /^[1-9][0-9]{0,9}$/.test(pid) ? `pid-${pid}` : null; } +// Where the host publishes its own process id, when it publishes one at all. +// +// Deliberately per adapter rather than a list of every host's variable checked +// in turn: those variables are inherited by child processes, so a host launched +// from another host's shell would key on the outer host and two different hosts +// could end up sharing one pointer file. Each adapter consults only the variable +// its own host sets. +// +// Process-global, because one process runs one host's adapter. A test that +// imported two adapters into the same process would have the second silently +// win; there is no such test, and there should not be one. +let hostPidProvider = null; + +export function configureHostPid(provider) { + if (provider !== null && typeof provider !== "function") { + throw new TypeError("The host pid provider must be a function or null."); + } + hostPidProvider = provider; +} + +function hostPid() { + if (!hostPidProvider) { + return null; + } + // An adapter reading a value the host never set must not take down the only + // path that can name this process's host. + try { + return hostPidProvider(); + } catch { + return null; + } +} + // The host process this process belongs to. // // NEATCONTEXT_HOST_KEY is the explicit form, for tests and for any host that can -// hand every one of its plugin processes the same identifier. Otherwise -// `process.ppid`: the bridge and the hooks are spawned by the Codex process -// directly, so both see the host's pid there. Codex publishes no pid of its own -// the way Claude Code publishes CLAUDE_PID, so a process spawned through a shell -// keys on the shell — a key nothing long-lived reads, which fails safe. +// hand every one of its plugin processes the same identifier. Otherwise the pid +// the adapter knows how to find, and failing that `process.ppid`, which is the +// host for a server or hook it spawned directly. export function hostKey() { const explicit = process.env.NEATCONTEXT_HOST_KEY; if (typeof explicit === "string") { @@ -77,7 +110,7 @@ export function hostKey() { ? trimmed : null; } - return pidKey(process.ppid); + return pidKey(hostPid()) ?? pidKey(process.ppid); } export function hostsDirectory() { @@ -113,12 +146,31 @@ async function readPointer(file) { } } +let writeCounter = 0; + +// Written to a temporary name and renamed into place, because these files are +// read by other processes while they are being written: the bridge re-reads the +// pointer on every message, and a reader that catches a half-written file falls +// back to the environment, which is the stale answer this file exists to +// correct. A rename is atomic, so a reader sees the old record or the new one. +// +// The temporary name carries this process's pid so that two processes writing +// the same pointer at once cannot collide on it, and stays in the same directory +// so the rename never crosses a volume. async function writePointer(file, value) { await mkdir(path.dirname(file), { recursive: true }); - await writeFile(file, `${JSON.stringify(value, null, 2)}\n`, { - encoding: "utf8", - mode: 0o600 - }); + writeCounter += 1; + const temporary = `${file}.${process.pid}.${writeCounter}.tmp`; + try { + await writeFile(temporary, `${JSON.stringify(value, null, 2)}\n`, { + encoding: "utf8", + mode: 0o600 + }); + await rename(temporary, file); + } catch (error) { + await rm(temporary, { force: true }).catch(() => undefined); + throw error; + } } export async function readHostPointer(key = hostKey()) { @@ -183,6 +235,11 @@ export async function resolveHostSessionId(envSessionId, { since = 0, key = host // rewriting. Age is not staleness here — a record from an hour ago is still what // this bridge is serving. Whether it is *running* is a question about its pid, // which the record carries. +// +// The cache is what this process last wrote, not what is on disk. Something that +// deleted the file from underneath it would not get it back until the session +// changes; nothing here does that, and defending against an outside `rm` would +// cost a stat on every message. let lastPublished = { id: undefined }; export async function publishBridgeSession(id, { key = hostKey(), now = Date.now() } = {}) { diff --git a/package.json b/package.json index 7994288..04ff8a8 100644 --- a/package.json +++ b/package.json @@ -12,11 +12,11 @@ "sync:evidence": "node tools/sync-conversation-evidence.mjs", "sync:context": "node tools/sync-context-core.mjs", "check:claude": "node --check plugins/claude-code/neatcontext/src/core/context-store.mjs && node --check plugins/claude-code/neatcontext/src/core/extension-bindings.mjs && node --check plugins/claude-code/neatcontext/src/core/extension-commands.mjs && node --check plugins/claude-code/neatcontext/src/core/extension-runtime.mjs && node --check plugins/claude-code/neatcontext/src/core/extensions.mjs && node --check plugins/claude-code/neatcontext/src/core/mcp-stdio-client.mjs && node --check plugins/claude-code/neatcontext/src/core/local-state.mjs && node --check plugins/claude-code/neatcontext/src/core/host-session.mjs && node --check plugins/claude-code/neatcontext/src/core/conversation-evidence.mjs && node --check plugins/claude-code/neatcontext/src/core/routing.mjs && node --check plugins/claude-code/neatcontext/src/core/routing-search.mjs && node --check plugins/claude-code/neatcontext/src/core/routing-candidates.mjs && node --check plugins/claude-code/neatcontext/src/core/session-state.mjs && node --check plugins/claude-code/neatcontext/src/core/selection.mjs && node --check plugins/claude-code/neatcontext/src/core/session.mjs && node --check plugins/claude-code/neatcontext/src/core/storage-home.mjs && node --check plugins/claude-code/neatcontext/src/claude/conversation-evidence.mjs && node --check plugins/claude-code/neatcontext/src/claude/mcp-bridge.mjs && node --check plugins/claude-code/neatcontext/src/claude/neatcontext-cli.mjs && node --check plugins/claude-code/neatcontext/src/claude/session.mjs && node --check plugins/claude-code/neatcontext/hooks/session-start.mjs && node --check plugins/claude-code/neatcontext/hooks/stop.mjs && node --check plugins/claude-code/neatcontext/hooks/pre-compact.mjs", - "check:kimi": "node --check plugins/kimi-code/neatcontext/src/core/context-store.mjs && node --check plugins/kimi-code/neatcontext/src/core/extension-bindings.mjs && node --check plugins/kimi-code/neatcontext/src/core/extension-commands.mjs && node --check plugins/kimi-code/neatcontext/src/core/extension-runtime.mjs && node --check plugins/kimi-code/neatcontext/src/core/extensions.mjs && node --check plugins/kimi-code/neatcontext/src/core/mcp-stdio-client.mjs && node --check plugins/kimi-code/neatcontext/src/core/local-state.mjs && node --check plugins/kimi-code/neatcontext/src/core/conversation-evidence.mjs && node --check plugins/kimi-code/neatcontext/src/core/routing.mjs && node --check plugins/kimi-code/neatcontext/src/core/routing-search.mjs && node --check plugins/kimi-code/neatcontext/src/core/routing-candidates.mjs && node --check plugins/kimi-code/neatcontext/src/core/selection.mjs && node --check plugins/kimi-code/neatcontext/src/core/session.mjs && node --check plugins/kimi-code/neatcontext/src/core/storage-home.mjs && node --check plugins/kimi-code/neatcontext/src/kimi/mcp-bridge.mjs && node --check plugins/kimi-code/neatcontext/src/kimi/neatcontext-cli.mjs && node --check plugins/kimi-code/neatcontext/src/kimi/session.mjs", - "check:copilot": "node --check plugins/copilot/neatcontext/src/core/context-store.mjs && node --check plugins/copilot/neatcontext/src/core/extension-bindings.mjs && node --check plugins/copilot/neatcontext/src/core/extension-commands.mjs && node --check plugins/copilot/neatcontext/src/core/extension-runtime.mjs && node --check plugins/copilot/neatcontext/src/core/extensions.mjs && node --check plugins/copilot/neatcontext/src/core/mcp-stdio-client.mjs && node --check plugins/copilot/neatcontext/src/core/local-state.mjs && node --check plugins/copilot/neatcontext/src/core/conversation-evidence.mjs && node --check plugins/copilot/neatcontext/src/core/routing.mjs && node --check plugins/copilot/neatcontext/src/core/routing-search.mjs && node --check plugins/copilot/neatcontext/src/core/routing-candidates.mjs && node --check plugins/copilot/neatcontext/src/core/selection.mjs && node --check plugins/copilot/neatcontext/src/core/session.mjs && node --check plugins/copilot/neatcontext/src/core/storage-home.mjs && node --check plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs && node --check plugins/copilot/neatcontext/src/copilot/neatcontext-cli.mjs && node --check plugins/copilot/neatcontext/src/copilot/session.mjs", + "check:kimi": "node --check plugins/kimi-code/neatcontext/src/core/context-store.mjs && node --check plugins/kimi-code/neatcontext/src/core/extension-bindings.mjs && node --check plugins/kimi-code/neatcontext/src/core/extension-commands.mjs && node --check plugins/kimi-code/neatcontext/src/core/extension-runtime.mjs && node --check plugins/kimi-code/neatcontext/src/core/extensions.mjs && node --check plugins/kimi-code/neatcontext/src/core/mcp-stdio-client.mjs && node --check plugins/kimi-code/neatcontext/src/core/local-state.mjs && node --check plugins/kimi-code/neatcontext/src/core/host-session.mjs && node --check plugins/kimi-code/neatcontext/src/core/conversation-evidence.mjs && node --check plugins/kimi-code/neatcontext/src/core/routing.mjs && node --check plugins/kimi-code/neatcontext/src/core/routing-search.mjs && node --check plugins/kimi-code/neatcontext/src/core/routing-candidates.mjs && node --check plugins/kimi-code/neatcontext/src/core/selection.mjs && node --check plugins/kimi-code/neatcontext/src/core/session.mjs && node --check plugins/kimi-code/neatcontext/src/core/storage-home.mjs && node --check plugins/kimi-code/neatcontext/src/kimi/mcp-bridge.mjs && node --check plugins/kimi-code/neatcontext/src/kimi/neatcontext-cli.mjs && node --check plugins/kimi-code/neatcontext/src/kimi/session.mjs", + "check:copilot": "node --check plugins/copilot/neatcontext/src/core/context-store.mjs && node --check plugins/copilot/neatcontext/src/core/extension-bindings.mjs && node --check plugins/copilot/neatcontext/src/core/extension-commands.mjs && node --check plugins/copilot/neatcontext/src/core/extension-runtime.mjs && node --check plugins/copilot/neatcontext/src/core/extensions.mjs && node --check plugins/copilot/neatcontext/src/core/mcp-stdio-client.mjs && node --check plugins/copilot/neatcontext/src/core/local-state.mjs && node --check plugins/copilot/neatcontext/src/core/host-session.mjs && node --check plugins/copilot/neatcontext/src/core/conversation-evidence.mjs && node --check plugins/copilot/neatcontext/src/core/routing.mjs && node --check plugins/copilot/neatcontext/src/core/routing-search.mjs && node --check plugins/copilot/neatcontext/src/core/routing-candidates.mjs && node --check plugins/copilot/neatcontext/src/core/selection.mjs && node --check plugins/copilot/neatcontext/src/core/session.mjs && node --check plugins/copilot/neatcontext/src/core/storage-home.mjs && node --check plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs && node --check plugins/copilot/neatcontext/src/copilot/neatcontext-cli.mjs && node --check plugins/copilot/neatcontext/src/copilot/session.mjs", "check:codex": "node --check codex-marketplace/plugins/neatcontext/src/core/context-store.mjs && node --check codex-marketplace/plugins/neatcontext/src/core/extension-bindings.mjs && node --check codex-marketplace/plugins/neatcontext/src/core/extension-commands.mjs && node --check codex-marketplace/plugins/neatcontext/src/core/extension-runtime.mjs && node --check codex-marketplace/plugins/neatcontext/src/core/extensions.mjs && node --check codex-marketplace/plugins/neatcontext/src/core/mcp-stdio-client.mjs && node --check codex-marketplace/plugins/neatcontext/src/core/local-state.mjs && node --check codex-marketplace/plugins/neatcontext/src/core/host-session.mjs && node --check codex-marketplace/plugins/neatcontext/src/core/conversation-evidence.mjs && node --check codex-marketplace/plugins/neatcontext/src/core/routing.mjs && node --check codex-marketplace/plugins/neatcontext/src/core/routing-search.mjs && node --check codex-marketplace/plugins/neatcontext/src/core/routing-candidates.mjs && node --check codex-marketplace/plugins/neatcontext/src/core/selection.mjs && node --check codex-marketplace/plugins/neatcontext/src/core/session.mjs && node --check codex-marketplace/plugins/neatcontext/src/core/storage-home.mjs && node --check codex-marketplace/plugins/neatcontext/src/codex/mcp-bridge.mjs && node --check codex-marketplace/plugins/neatcontext/src/codex/neatcontext-cli.mjs && node --check codex-marketplace/plugins/neatcontext/src/codex/session.mjs", "check:pi": "npm --prefix plugins/pi/neatcontext run check", - "check": "node tools/sync-conversation-evidence.mjs --check && node tools/sync-context-core.mjs --check && node --check shared/core/conversation-evidence.mjs && node --check shared/core/context-store.mjs && node --check shared/core/extension-bindings.mjs && node --check shared/core/extension-commands.mjs && node --check shared/core/extension-runtime.mjs && node --check shared/core/extensions.mjs && node --check shared/core/mcp-stdio-client.mjs && node --check shared/core/local-state.mjs && node --check shared/core/routing.mjs && node --check shared/core/routing-search.mjs && node --check shared/core/routing-candidates.mjs && node --check shared/core/selection.mjs && node --check shared/core/storage-home.mjs && npm run check:claude && npm run check:kimi && npm run check:copilot && npm run check:codex && npm run check:pi && node --check tools/sync-conversation-evidence.mjs && node --check tools/sync-context-core.mjs && node --check tools/e2e-no-nudge.mjs && node --check tools/e2e-extensions.mjs && node --check tools/e2e-commands.mjs && node --check tools/diff-coverage.mjs", + "check": "node tools/sync-conversation-evidence.mjs --check && node tools/sync-context-core.mjs --check && node --check shared/core/conversation-evidence.mjs && node --check shared/core/context-store.mjs && node --check shared/core/extension-bindings.mjs && node --check shared/core/extension-commands.mjs && node --check shared/core/extension-runtime.mjs && node --check shared/core/extensions.mjs && node --check shared/core/mcp-stdio-client.mjs && node --check shared/core/local-state.mjs && node --check shared/core/host-session.mjs && node --check shared/core/routing.mjs && node --check shared/core/routing-search.mjs && node --check shared/core/routing-candidates.mjs && node --check shared/core/selection.mjs && node --check shared/core/storage-home.mjs && npm run check:claude && npm run check:kimi && npm run check:copilot && npm run check:codex && npm run check:pi && node --check tools/sync-conversation-evidence.mjs && node --check tools/sync-context-core.mjs && node --check tools/e2e-no-nudge.mjs && node --check tools/e2e-extensions.mjs && node --check tools/e2e-commands.mjs && node --check tools/diff-coverage.mjs", "validate:plugin": "claude plugin validate . --strict && claude plugin validate plugins/claude-code/neatcontext --strict", "test": "node --test", "coverage": "node tools/diff-coverage.mjs", diff --git a/plugins/claude-code/neatcontext/hooks/session-start.mjs b/plugins/claude-code/neatcontext/hooks/session-start.mjs index 53b00ad..69ca94e 100644 --- a/plugins/claude-code/neatcontext/hooks/session-start.mjs +++ b/plugins/claude-code/neatcontext/hooks/session-start.mjs @@ -19,6 +19,10 @@ import { configureSessionId } from "../src/core/session.mjs"; import { readSelection } from "../src/core/local-state.mjs"; +// Registers which environment variable names this host's process, so `hostKey()` +// below resolves to the same pointer file the bridge and the slash commands use. +// The session provider it installs is overridden explicitly further down. +import "../src/claude/session.mjs"; import { hostKey, pruneHostPointers, diff --git a/plugins/claude-code/neatcontext/hooks/stop.mjs b/plugins/claude-code/neatcontext/hooks/stop.mjs index 16d8e61..05b0234 100644 --- a/plugins/claude-code/neatcontext/hooks/stop.mjs +++ b/plugins/claude-code/neatcontext/hooks/stop.mjs @@ -16,6 +16,10 @@ // user sees, and there is no longer anything worth showing them. import { configureSessionId } from "../src/core/session.mjs"; +// Registers which environment variable names this host's process, so the pointer +// written below lands on the file the bridge reads. The session provider it +// installs is overridden explicitly further down. +import "../src/claude/session.mjs"; import { writeHostPointer } from "../src/core/host-session.mjs"; import { updateRouting } from "../src/core/routing.mjs"; import { normalizeSaveState, rememberTranscriptPath } from "../src/core/session-state.mjs"; diff --git a/plugins/claude-code/neatcontext/src/claude/session.mjs b/plugins/claude-code/neatcontext/src/claude/session.mjs index d528baa..c88e7ad 100644 --- a/plugins/claude-code/neatcontext/src/claude/session.mjs +++ b/plugins/claude-code/neatcontext/src/claude/session.mjs @@ -13,7 +13,17 @@ // commands correct the stale copy. See core/host-session.mjs. import { configureSessionId } from "../core/session.mjs"; -import { publishBridgeSession, resolveHostSessionId } from "../core/host-session.mjs"; +import { + configureHostPid, + publishBridgeSession, + resolveHostSessionId +} from "../core/host-session.mjs"; + +// Claude Code sets CLAUDE_PID in the environment of every process it spawns — +// including the shell a slash command runs in, where `process.ppid` is the shell +// rather than the host. Without it the CLI would key on the shell and write a +// pointer the bridge never reads. +configureHostPid(() => process.env.CLAUDE_PID); // When this process started. A pointer older than this cannot be describing a // session change that happened after it, and is therefore not about this host. diff --git a/plugins/claude-code/neatcontext/src/core/host-session.mjs b/plugins/claude-code/neatcontext/src/core/host-session.mjs index d790f24..5e863d2 100644 --- a/plugins/claude-code/neatcontext/src/core/host-session.mjs +++ b/plugins/claude-code/neatcontext/src/core/host-session.mjs @@ -1,17 +1,17 @@ // Which session the host process is on *right now*. // // A host that identifies its sessions through the environment has a problem the -// rest of this plugin cannot see: the environment is a snapshot. Claude Code -// spawns the MCP bridge once and keeps it for the life of the window, but the -// session id changes underneath it — `/clear` starts a new session in the same -// process — and the bridge's copy of `CLAUDE_CODE_SESSION_ID` is whatever it was -// at spawn. The slash commands are spawned fresh and see the new one. +// rest of this plugin cannot see: the environment is a snapshot. Hosts spawn the +// MCP bridge once and keep it for the life of the window, but the session +// changes underneath it — starting a fresh conversation does not restart the +// server — and the bridge's copy of the id is whatever it was at spawn. The +// hooks and the slash-command CLI are spawned fresh and see the new one. // // So the two halves of the plugin end up reading and writing different files: -// `/neatcontext:use` writes the selection for the session the user is in, the -// bridge keeps serving the session the window started in, and nothing in either -// path can observe the disagreement. The user is told a context is connected and -// every later answer is grounded in a different one. +// `use` writes the selection for the session the user is in, the bridge keeps +// serving the session the window started in, and nothing in either path can +// observe the disagreement. The user is told a context is connected and every +// later answer is grounded in a different one. // // This module is the missing channel. One pointer file per host *process*, // written by whichever short-lived process was just handed the current session @@ -21,19 +21,22 @@ // // The host key has to be stable across a session change and distinct per window, // which rules out the session id and the working directory both. What is left is -// the host process itself: Claude Code publishes its pid as CLAUDE_PID to the -// processes it spawns, and the bridge is its direct child, so `process.ppid` is -// the same number seen from the other side. Two windows are two pids and never -// share a pointer. +// the host process itself. How to name it is the one thing that differs per +// host, so each adapter registers its own answer with `configureHostPid()`: some +// hosts publish their own pid to every process they spawn, and a host that +// publishes nothing falls back to `process.ppid`, which is the host for any +// process it spawned directly. A process whose parent is not the host and whose +// host publishes no pid — a CLI running inside a shell tool — computes a key +// nothing else reads, and its writes are harmless and swept later. // // The bridge also publishes what it actually resolved: // // ~/.neatcontext/plugin-hosts/.bridge.json { pid, sessionId, updatedAt } // -// That is what lets `/neatcontext:use` check its own success against the process -// that will serve it, instead of against the file it just wrote itself. +// That is what lets `use` check its own success against the process that will +// serve it, instead of against the file it just wrote itself. -import { mkdir, readFile, readdir, rm, writeFile } from "node:fs/promises"; +import { mkdir, readFile, readdir, rename, rm, writeFile } from "node:fs/promises"; import path from "node:path"; import { neatContextHome } from "./storage-home.mjs"; @@ -59,14 +62,45 @@ function pidKey(value) { return /^[1-9][0-9]{0,9}$/.test(pid) ? `pid-${pid}` : null; } +// Where the host publishes its own process id, when it publishes one at all. +// +// Deliberately per adapter rather than a list of every host's variable checked +// in turn: those variables are inherited by child processes, so a host launched +// from another host's shell would key on the outer host and two different hosts +// could end up sharing one pointer file. Each adapter consults only the variable +// its own host sets. +// +// Process-global, because one process runs one host's adapter. A test that +// imported two adapters into the same process would have the second silently +// win; there is no such test, and there should not be one. +let hostPidProvider = null; + +export function configureHostPid(provider) { + if (provider !== null && typeof provider !== "function") { + throw new TypeError("The host pid provider must be a function or null."); + } + hostPidProvider = provider; +} + +function hostPid() { + if (!hostPidProvider) { + return null; + } + // An adapter reading a value the host never set must not take down the only + // path that can name this process's host. + try { + return hostPidProvider(); + } catch { + return null; + } +} + // The host process this process belongs to. // // NEATCONTEXT_HOST_KEY is the explicit form, for tests and for any host that can -// hand every one of its plugin processes the same identifier. Otherwise: -// CLAUDE_PID, which Claude Code sets in the environment of the processes it -// spawns — including the shell a slash command runs in, where `process.ppid` is -// the shell rather than the host. `process.ppid` is the fallback for the bridge, -// which the host spawns directly. +// hand every one of its plugin processes the same identifier. Otherwise the pid +// the adapter knows how to find, and failing that `process.ppid`, which is the +// host for a server or hook it spawned directly. export function hostKey() { const explicit = process.env.NEATCONTEXT_HOST_KEY; if (typeof explicit === "string") { @@ -76,7 +110,7 @@ export function hostKey() { ? trimmed : null; } - return pidKey(process.env.CLAUDE_PID) ?? pidKey(process.ppid); + return pidKey(hostPid()) ?? pidKey(process.ppid); } export function hostsDirectory() { @@ -112,12 +146,31 @@ async function readPointer(file) { } } +let writeCounter = 0; + +// Written to a temporary name and renamed into place, because these files are +// read by other processes while they are being written: the bridge re-reads the +// pointer on every message, and a reader that catches a half-written file falls +// back to the environment, which is the stale answer this file exists to +// correct. A rename is atomic, so a reader sees the old record or the new one. +// +// The temporary name carries this process's pid so that two processes writing +// the same pointer at once cannot collide on it, and stays in the same directory +// so the rename never crosses a volume. async function writePointer(file, value) { await mkdir(path.dirname(file), { recursive: true }); - await writeFile(file, `${JSON.stringify(value, null, 2)}\n`, { - encoding: "utf8", - mode: 0o600 - }); + writeCounter += 1; + const temporary = `${file}.${process.pid}.${writeCounter}.tmp`; + try { + await writeFile(temporary, `${JSON.stringify(value, null, 2)}\n`, { + encoding: "utf8", + mode: 0o600 + }); + await rename(temporary, file); + } catch (error) { + await rm(temporary, { force: true }).catch(() => undefined); + throw error; + } } export async function readHostPointer(key = hostKey()) { @@ -182,6 +235,11 @@ export async function resolveHostSessionId(envSessionId, { since = 0, key = host // rewriting. Age is not staleness here — a record from an hour ago is still what // this bridge is serving. Whether it is *running* is a question about its pid, // which the record carries. +// +// The cache is what this process last wrote, not what is on disk. Something that +// deleted the file from underneath it would not get it back until the session +// changes; nothing here does that, and defending against an outside `rm` would +// cost a stat on every message. let lastPublished = { id: undefined }; export async function publishBridgeSession(id, { key = hostKey(), now = Date.now() } = {}) { diff --git a/plugins/copilot/neatcontext/README.md b/plugins/copilot/neatcontext/README.md index 71223e8..d924926 100644 --- a/plugins/copilot/neatcontext/README.md +++ b/plugins/copilot/neatcontext/README.md @@ -33,8 +33,8 @@ copilot plugin install neatcontext@neatcontext ## Commands - `/neatcontext:save [name]` — save reusable work from the visible conversation. -- `/neatcontext:use [name or number]` — connect or switch this workspace. -- `/neatcontext:disconnect` — disconnect only this workspace. +- `/neatcontext:use [name or number]` — connect or switch this session. +- `/neatcontext:disconnect` — disconnect only this session. - `/neatcontext:list` — list the contexts on this machine. - `/neatcontext:status` — show the selection and routing mode. - `/neatcontext:create` — create a context around an existing knowledge folder. @@ -47,9 +47,17 @@ copilot plugin install neatcontext@neatcontext - **Local Contexts.** The plugin stores Contexts on this machine. There is no NeatContext Desktop connection right now. -- **Selections are per workspace.** Copilot does not expose a session identity - to plugin processes, so a connected context belongs to the workspace folder: - every Copilot session opened in that folder shares it. +- **Selections are per session.** A connected context belongs to the Copilot + session that connected it, as on every other host. Where a Copilot build + publishes no session identity, the plugin falls back to the workspace folder, + every session opened there shares one selection, and `/neatcontext:status` + says so rather than leaving you to guess. +- **Upgrading from 0.3.3 or earlier.** Those releases scoped every Copilot + selection to the workspace folder. The first time you run a command after + upgrading, this session has no connection of its own yet, so + `/neatcontext:status` reports none and names the context that workspace used + to be connected to. Reconnect it once with `/neatcontext:use` and each session + keeps its own from then on. Nothing is deleted. - **Saving is explicit.** Run `/neatcontext:save` when the visible conversation contains durable work worth preserving. diff --git a/plugins/copilot/neatcontext/commands/disconnect.md b/plugins/copilot/neatcontext/commands/disconnect.md index 7ccc376..9a50585 100644 --- a/plugins/copilot/neatcontext/commands/disconnect.md +++ b/plugins/copilot/neatcontext/commands/disconnect.md @@ -9,4 +9,4 @@ Disconnect the context currently connected to this session. !`node "${CLAUDE_PLUGIN_ROOT}/src/copilot/neatcontext-cli.mjs" disconnect` Relay the result verbatim. Do not run a status command afterward. This affects -only the current workspace; other workspaces keep their own connections. +only the current session; other sessions keep their own connections. diff --git a/plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs b/plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs index bf332df..95fe625 100644 --- a/plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs +++ b/plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs @@ -1,6 +1,11 @@ // NeatContext plugin MCP server for GitHub Copilot. // // Behaviors kept from the Claude Code bridge: +// * this process outlives the session it was spawned in — a new session +// starts 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 previous session's context while the +// slash commands 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). @@ -10,7 +15,7 @@ // get_context instead of silently vanishing. import readline from "node:readline"; -import "./session.mjs"; +import { publishSessionId, refreshSessionId } from "./session.mjs"; import { readSelection } from "../core/local-state.mjs"; import { CONTEXT_MISSING_MESSAGE, @@ -165,11 +170,11 @@ A context is whatever its profile says it is. Do not assume a subject area for i Cite the exact file path of anything you rely on. When the profile and the knowledge folder do not cover the question, say so instead of answering from general knowledge.`; // Written to survive being wrong. These instructions are fixed at the -// handshake, but a context can be connected at any time afterwards — from this -// 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 connected at the moment this session started. That says nothing about now: a Context can be connected at any time, from this session or another window on this workspace. +// handshake, but a context can be connected at any time afterwards — by a +// slash command in this session, or by the session's routing. 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 connected at the moment this session started. That says nothing about now: a Context can be connected at any point later in this session. These instructions are fixed at the handshake and cannot be updated, so they are not evidence about the current state — and you must not tell the user nothing is connected on the strength of this text. @@ -469,23 +474,38 @@ function refusal(policy, target) { let started = false; let lastVersion = undefined; +// Re-resolve which session this process is serving, and publish the answer so a +// slash command can tell whether its write is the one this bridge will read. +async function syncSession() { + await refreshSessionId(); + await publishSessionId(); +} + // 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. async function currentVersion() { + // The session is part of it: a new session 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 session's extension tools. + const session = sessionId() ?? "none"; const mode = resolveMode(await readRouting(), sessionId()); const context = await activeContext(); // The extension signature is read from what the last resolve found, never by // starting anything: this runs on a timer, and a poll must not spawn a server. const extensions = extensionHost.signature(context?.record ?? null); if (context) { - return `${mode}/${context.missing ? "context:missing" : context.record.id}/${extensions}`; + return `${session}/${mode}/${context.missing ? "context:missing" : context.record.id}/${extensions}`; } - return `${mode}/none/${extensions}`; + return `${session}/${mode}/none/${extensions}`; } async function handleMessage(message) { const isNotification = message.id === undefined || message.id === null; + // Before anything reads a selection or a routing mode: which session 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. @@ -598,6 +618,10 @@ function startVersionWatch() { watching = true; setInterval(async () => { if (!started) return; + // The host does not send a message when the session changes underneath this + // process, so this tick is where that is noticed if nothing else asks first + // — and where the published answer stays fresh enough to be checked against. + await syncSession(); const version = await currentVersion(); if (version !== null && version !== lastVersion) { lastVersion = version; diff --git a/plugins/copilot/neatcontext/src/copilot/neatcontext-cli.mjs b/plugins/copilot/neatcontext/src/copilot/neatcontext-cli.mjs index f051f2b..4141f0e 100644 --- a/plugins/copilot/neatcontext/src/copilot/neatcontext-cli.mjs +++ b/plugins/copilot/neatcontext/src/copilot/neatcontext-cli.mjs @@ -20,8 +20,9 @@ // Exit code is always 0: the output is meant to be read, not branched on. import { readFile, rm } from "node:fs/promises"; -import "./session.mjs"; -import { clearSelection, readSelection } from "../core/local-state.mjs"; +import { sessionIdentityIsShared, unusableSessionOverride, workspaceSessionId } from "./session.mjs"; +import { clearSelection, readSelection, sessionSelectionFilePath } from "../core/local-state.mjs"; +import { awaitBridgeSession, readBridgeSession, writeHostPointer } from "../core/host-session.mjs"; import { createCapturedContext, createContext, @@ -67,6 +68,109 @@ function print(line = "") { process.stdout.write(`${line}\n`); } +// This process is spawned for one command and its environment is fresh, so the +// session it names is the session the user is actually in. The MCP bridge was +// spawned once, when the window opened, and cannot know that a new session +// replaced it. Recording it here is what lets the bridge read the selection this +// command is about to write. +// +// Every command records it, not only the ones that write a selection. Copilot +// ships no hooks, so this process is the only thing that can ever tell the +// bridge what session the window is on — and a read-only command is often the +// first one to run after a session changes, which makes it the earliest chance +// to heal the pointer. One small rename is worth that. +async function recordHostSession() { + return writeHostPointer(sessionId(), { source: "cli" }).catch(() => null); +} + +// Whether the two halves of the plugin can agree on a session at all. +// +// Every other warning here assumes they can and reports a disagreement. This one +// is about the case where there is no channel between them: a Copilot build that +// publishes neither a session id nor its own pid leaves the bridge and this +// process hashing their own working directories, which are not the same +// directory. Nothing downstream can detect that, so it is said once, here. +function hostIdentityWarning() { + if (sessionIdentityIsShared()) { + return null; + } + return ( + "Warning: this Copilot build publishes no session identity to plugin processes, " + + "so NeatContext cannot tell which session its MCP server is serving. A connected " + + "context may not reach it. Set NEATCONTEXT_SESSION_ID to any short identifier to " + + "pin one, and report your Copilot version at " + + "https://github.com/XTSoftwareLabs/neatcontext-plugins/issues." + ); +} + +// A selection saved by a release that scoped Copilot to the workspace folder. +// +// Those releases wrote `plugin-sessions/copilot-ws-.json`, which nothing +// reads once the host publishes a real session id. Saying so beats letting a +// working connection look like it disappeared — and it is only said when this +// workspace actually has one and nothing is connected now. +async function workspaceSelectionHint(connected) { + if (connected) { + return null; + } + const workspace = workspaceSessionId(); + if (sessionId() === workspace) { + return null; + } + try { + const saved = JSON.parse(await readFile(sessionSelectionFilePath(workspace), "utf8")); + const name = typeof saved?.contextName === "string" ? saved.contextName.trim() : ""; + if (!name) { + return null; + } + return ( + `An earlier version of this plugin connected "${name}" to this workspace rather ` + + "than to a session. Sessions each carry their own connection now, so reconnect " + + `it once with \`/neatcontext:use ${name}\`.` + ); + } catch { + return null; + } +} + +// Whether the bridge that will serve this session 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 session's file would leave the message true about the disk +// and false about the session. The bridge publishes what it resolved, so ask it. +// A bridge that publishes nothing (an older build, or none running) is not +// evidence of anything and gets no warning. +// +// This waits: a bridge that has not yet noticed the session change re-resolves +// within its watch interval, so the first command after a change can take about +// a second longer than the rest. Reporting drift that was about to resolve +// itself would be worse than the wait. +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 " + + "session and has not picked this one up, so `get_context` may keep returning the " + + "previous context. Restart Copilot 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 = {}; @@ -140,6 +244,19 @@ 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 session, this is a report about a selection it is not reading. + // The ids themselves are not printed — the user cannot act on a session uuid, + // and the fact of the disagreement is the whole message. + const bridge = await readBridgeSession().catch(() => null); + if (bridge && bridge.sessionId !== sessionId()) { + print( + "Warning: NeatContext's MCP server in this window is serving an earlier session, " + + "not this one. What follows is what this session selected; `get_context` may " + + "still answer from the other one. Restart Copilot to clear it." + ); + print(""); + } const routing = await readRouting(); const mode = resolveMode(routing, sessionId()); // Reported alongside the connection because the two together are the whole @@ -214,6 +331,11 @@ async function commandStatus(state) { "`/neatcontext:create`." : "No context is connected yet. Use `/neatcontext:use` to pick one." ); + const upgrade = await workspaceSelectionHint(connected); + if (upgrade) { + print(""); + print(upgrade); + } reportMode(); } @@ -384,6 +506,7 @@ async function commandUse(state, query) { "will be grounded in its domain profile and knowledge folder." ); await nudgeForDescription(target); + await printBridgeDrift(); } async function commandDisconnect(state) { @@ -398,6 +521,9 @@ async function commandDisconnect(state) { const name = connected?.name ?? remembered.contextName; print(`Disconnected the "${name}" context from this session.`); + // Same exposure as connecting: a bridge on another session clears nothing the + // session it is serving can see. + await printBridgeDrift(); } // A context with no routing description can only be routed to by name. @@ -586,6 +712,7 @@ 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}`); @@ -859,6 +986,31 @@ async function run() { const [command = "status", ...rest] = process.argv.slice(2); const { flags, query } = parseArgs(rest); + // Before anything reads a selection, because both of these say the session + // this command is about to act on may not be the one the user is in. They go + // to stderr so they cannot be mistaken for a command's result by whatever is + // relaying stdout verbatim. + const override = unusableSessionOverride(); + if (override) { + process.stderr.write( + `NEATCONTEXT_SESSION_ID is set to something that cannot name a session file ` + + `(${override.length} characters, letters, digits, dot, dash and underscore only), ` + + "so it is being ignored.\n" + ); + } + const identity = hostIdentityWarning(); + if (identity) { + process.stderr.write(`${identity}\n`); + } + + // Every invocation, not just the ones that change the selection. Copilot has + // no hook that runs when a session starts or ends, so this process is the only + // one that ever reports the current session to the long-lived bridge — and the + // report is worth making before a command writes a selection the bridge would + // otherwise never read, and after one, to heal a record something else wrote + // wrongly in between. + await recordHostSession(); + if (command === "create") { await commandCreate(flags); return; diff --git a/plugins/copilot/neatcontext/src/copilot/session.mjs b/plugins/copilot/neatcontext/src/copilot/session.mjs index d964e63..d707fb2 100644 --- a/plugins/copilot/neatcontext/src/copilot/session.mjs +++ b/plugins/copilot/neatcontext/src/copilot/session.mjs @@ -1,32 +1,60 @@ // GitHub Copilot host adapter for the reusable session-aware runtime. // -// Neither Copilot host hands this process a session id the way Claude Code -// does: Copilot CLI exposes no session identity to plugin processes, and the -// VS Code Agent Plugins preview does not document one for MCP servers. What -// both hosts do give every plugin process — the CLI the slash commands spawn -// and the MCP server alike — is the workspace as its working directory. +// This file used to say that Copilot exposes no session identity to plugin +// processes, and derived one by hashing `process.cwd()` instead — on the premise +// that every plugin process is given the workspace as its working directory. +// That premise is false. On Copilot CLI the MCP bridge is spawned with the +// *plugin installation* directory as its working directory while the CLI a slash +// command runs is spawned with the user's workspace, so the two halves of the +// plugin hashed different paths, read and wrote different selection files, and +// neither could observe the disagreement: `/neatcontext:use` reported success +// for a file `get_context` would never read. // -// So a "session" here is the workspace: one selected context per workspace, -// shared by every Copilot session opened in it. The id is a stable digest of -// the normalized workspace path, so the CLI and the MCP server agree on it -// without ever talking to each other. +// Copilot does publish a session identity, to the bridge and to the CLI alike: // -// NEATCONTEXT_SESSION_ID overrides the digest — for tests, and for any host -// that can inject a real per-session id into every plugin process. +// COPILOT_AGENT_SESSION_ID the session, distinct per window +// COPILOT_LOADER_PID the host process, which the bridge also sees as +// its own parent +// +// So a session here is a session, as on every other host, rather than a +// workspace. The workspace digest remains as the fallback for a Copilot build +// that publishes no session id, which keeps that case working exactly as it did +// — and no worse. +// +// Like Claude Code and Codex, the id is *resolved* rather than read: the bridge +// is spawned once and outlives the session it was spawned in, so it asks +// `refreshSessionId()` before it handles anything, which lets the pointer +// written by the freshly spawned CLI correct a stale copy. See +// core/host-session.mjs. +// +// NEATCONTEXT_SESSION_ID overrides everything — for tests, and for any host that +// can inject a real per-session id into every plugin process. // // CLAUDE_CODE_SESSION_ID is deliberately NOT consulted, even though a -// Claude-compat host might set it: a variable only some of this plugin's -// processes see is worse than none, because the CLI and the MCP server would -// scope to different sessions and the selection would silently split. (It -// also leaks into child shells when the user launches Copilot from inside a -// Claude Code session, which would hijack the scope the same way.) +// Claude-compat host might set it: it leaks into child shells when the user +// launches Copilot from inside a Claude Code session, which would scope this +// plugin to the outer host's session. import { createHash } from "node:crypto"; import path from "node:path"; import { configureSessionId } from "../core/session.mjs"; +import { + configureHostPid, + normalizeHostSessionId, + publishBridgeSession, + resolveHostSessionId +} from "../core/host-session.mjs"; +// When this process started. A pointer older than this cannot be describing a +// session change that happened after it, and is therefore not about this host. +const STARTED_AT = Date.now(); + +// A host-supplied id becomes a path segment (`plugin-sessions/.json`), so it +// is held to the same rule as one arriving from a pointer file. An unusable +// value falls through to the next source rather than being repaired: an id this +// plugin invented would not be the one the other half of it computes. function explicitId(value) { - return typeof value === "string" && value.trim().length > 0 ? value.trim() : null; + return normalizeHostSessionId(value); } export function workspaceSessionId(workspace = process.cwd()) { @@ -38,8 +66,80 @@ export function workspaceSessionId(workspace = process.cwd()) { return `copilot-ws-${digest}`; } +// What this process was started with. Never null: the workspace digest always +// answers, because a null session id would send every workspace to the one +// unscoped selection file they would then share. +export function environmentSessionId() { + return ( + explicitId(process.env.NEATCONTEXT_SESSION_ID) ?? + explicitId(process.env.COPILOT_AGENT_SESSION_ID) ?? + workspaceSessionId() + ); +} + +// Set, but not to something that can be a path segment. +// +// Worth saying out loud rather than falling through in silence: this used to +// accept any non-empty string, so a value with a `/` or a `:` in it that worked +// before now scopes the session somewhere else entirely. +export function unusableSessionOverride() { + const raw = process.env.NEATCONTEXT_SESSION_ID; + if (typeof raw !== "string" || raw.trim().length === 0) { + return null; + } + return explicitId(raw) ? null : raw.trim(); +} + +// Whether the bridge and this process can arrive at the same session at all. +// +// One of these has to hold, or the two halves are back to hashing their own +// working directories — which are not the same directory — with no channel +// between them. It is not a guess: each branch is a thing the host either +// publishes to both processes or does not. +export function sessionIdentityIsShared() { + const key = process.env.NEATCONTEXT_HOST_KEY; + return Boolean( + explicitId(process.env.NEATCONTEXT_SESSION_ID) ?? + explicitId(process.env.COPILOT_AGENT_SESSION_ID) ?? + (typeof key === "string" && key.trim().length > 0 ? key : null) ?? + // A host pid both halves can see is enough on its own: it names the + // pointer file they share, which is what carries the session across. + (/^[1-9][0-9]{0,9}$/.test(String(process.env.COPILOT_LOADER_PID ?? "").trim()) + ? "pid" + : null) + ); +} + +// Until something resolves it, the environment answers directly — which is what +// every process that is spawned per command wants, and what this file did before +// there was anything else to consult. +const UNRESOLVED = Symbol("unresolved"); +let resolved = UNRESOLVED; + export function copilotSessionId() { - return explicitId(process.env.NEATCONTEXT_SESSION_ID) ?? workspaceSessionId(); + return resolved === UNRESOLVED ? environmentSessionId() : resolved; +} + +// Re-resolves the session 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(environmentSessionId(), { 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(copilotSessionId() ?? null); } +// Copilot publishes the host's own pid to the processes it spawns, including the +// shell a slash command runs in — where `process.ppid` is that shell rather than +// the host. The bridge, spawned by the host directly, sees the same number as +// its parent, so both halves agree on which pointer file is theirs. +configureHostPid(() => process.env.COPILOT_LOADER_PID); configureSessionId(copilotSessionId); diff --git a/plugins/copilot/neatcontext/src/core/host-session.mjs b/plugins/copilot/neatcontext/src/core/host-session.mjs new file mode 100644 index 0000000..5e863d2 --- /dev/null +++ b/plugins/copilot/neatcontext/src/core/host-session.mjs @@ -0,0 +1,345 @@ +// Which session the host process is on *right now*. +// +// A host that identifies its sessions through the environment has a problem the +// rest of this plugin cannot see: the environment is a snapshot. Hosts spawn the +// MCP bridge once and keep it for the life of the window, but the session +// changes underneath it — starting a fresh conversation does not restart the +// server — and the bridge's copy of the id is whatever it was at spawn. The +// hooks and the slash-command CLI are spawned fresh and see the new one. +// +// So the two halves of the plugin end up reading and writing different files: +// `use` writes the selection for the session the user is in, the bridge keeps +// serving the session the window started in, and nothing in either path can +// observe the disagreement. The user is told a context is connected and every +// later answer is grounded in a different one. +// +// This module is the missing channel. One pointer file per host *process*, +// written by whichever short-lived process was just handed the current session +// id, and re-read by the long-lived one on every message: +// +// ~/.neatcontext/plugin-hosts/.json { sessionId, source, updatedAt } +// +// The host key has to be stable across a session change and distinct per window, +// which rules out the session id and the working directory both. What is left is +// the host process itself. How to name it is the one thing that differs per +// host, so each adapter registers its own answer with `configureHostPid()`: some +// hosts publish their own pid to every process they spawn, and a host that +// publishes nothing falls back to `process.ppid`, which is the host for any +// process it spawned directly. A process whose parent is not the host and whose +// host publishes no pid — a CLI running inside a shell tool — computes a key +// nothing else reads, and its writes are harmless and swept later. +// +// The bridge also publishes what it actually resolved: +// +// ~/.neatcontext/plugin-hosts/.bridge.json { pid, sessionId, updatedAt } +// +// That is what lets `use` check its own success against the process that will +// serve it, instead of against the file it just wrote itself. + +import { mkdir, readFile, readdir, rename, rm, writeFile } from "node:fs/promises"; +import path from "node:path"; +import { neatContextHome } from "./storage-home.mjs"; + +// A session id becomes a path segment (`plugin-sessions/.json`), and unlike +// the environment — which only the host can set — this one arrives from a file. +// Anything that could climb out of the 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 normalizeHostSessionId(value) { + if (typeof value !== "string") { + return null; + } + const id = value.trim(); + if (id === "." || id === ".." || !SAFE_SESSION_ID.test(id)) { + return null; + } + return id; +} + +function pidKey(value) { + const pid = typeof value === "string" ? value.trim() : String(value ?? ""); + return /^[1-9][0-9]{0,9}$/.test(pid) ? `pid-${pid}` : null; +} + +// Where the host publishes its own process id, when it publishes one at all. +// +// Deliberately per adapter rather than a list of every host's variable checked +// in turn: those variables are inherited by child processes, so a host launched +// from another host's shell would key on the outer host and two different hosts +// could end up sharing one pointer file. Each adapter consults only the variable +// its own host sets. +// +// Process-global, because one process runs one host's adapter. A test that +// imported two adapters into the same process would have the second silently +// win; there is no such test, and there should not be one. +let hostPidProvider = null; + +export function configureHostPid(provider) { + if (provider !== null && typeof provider !== "function") { + throw new TypeError("The host pid provider must be a function or null."); + } + hostPidProvider = provider; +} + +function hostPid() { + if (!hostPidProvider) { + return null; + } + // An adapter reading a value the host never set must not take down the only + // path that can name this process's host. + try { + return hostPidProvider(); + } catch { + return null; + } +} + +// The host process this process belongs to. +// +// NEATCONTEXT_HOST_KEY is the explicit form, for tests and for any host that can +// hand every one of its plugin processes the same identifier. Otherwise the pid +// the adapter knows how to find, and failing that `process.ppid`, which is the +// host for a server or hook it spawned directly. +export function hostKey() { + const explicit = process.env.NEATCONTEXT_HOST_KEY; + if (typeof explicit === "string") { + const trimmed = explicit.trim(); + return trimmed.length > 0 && trimmed !== "." && trimmed !== ".." && !trimmed.includes("/") && + !trimmed.includes("\\") + ? trimmed + : null; + } + return pidKey(hostPid()) ?? pidKey(process.ppid); +} + +export function hostsDirectory() { + return path.join(neatContextHome(), "plugin-hosts"); +} + +export function hostPointerPath(key) { + return path.join(hostsDirectory(), `${key}.json`); +} + +export function bridgePointerPath(key) { + return path.join(hostsDirectory(), `${key}.bridge.json`); +} + +async function readPointer(file) { + try { + const parsed = JSON.parse(await readFile(file, "utf8")); + const id = normalizeHostSessionId(parsed?.sessionId); + if (!id) { + return null; + } + const updatedAt = Date.parse(parsed?.updatedAt); + return { + sessionId: id, + source: typeof parsed?.source === "string" ? parsed.source : "unknown", + updatedAt: Number.isNaN(updatedAt) ? 0 : updatedAt, + pid: typeof parsed?.pid === "number" ? parsed.pid : null + }; + } catch { + // Missing, half-written, or hand-broken: the caller falls back to the + // environment, which is exactly the behavior that predates this file. + return null; + } +} + +let writeCounter = 0; + +// Written to a temporary name and renamed into place, because these files are +// read by other processes while they are being written: the bridge re-reads the +// pointer on every message, and a reader that catches a half-written file falls +// back to the environment, which is the stale answer this file exists to +// correct. A rename is atomic, so a reader sees the old record or the new one. +// +// The temporary name carries this process's pid so that two processes writing +// the same pointer at once cannot collide on it, and stays in the same directory +// so the rename never crosses a volume. +async function writePointer(file, value) { + await mkdir(path.dirname(file), { recursive: true }); + writeCounter += 1; + const temporary = `${file}.${process.pid}.${writeCounter}.tmp`; + try { + await writeFile(temporary, `${JSON.stringify(value, null, 2)}\n`, { + encoding: "utf8", + mode: 0o600 + }); + await rename(temporary, file); + } catch (error) { + await rm(temporary, { force: true }).catch(() => undefined); + throw error; + } +} + +export async function readHostPointer(key = hostKey()) { + return key ? readPointer(hostPointerPath(key)) : null; +} + +// Called by every process the host hands a session id to: the hooks, which are +// told on stdin, and the CLI, whose environment is fresh because it was spawned +// for this command. Last write wins — they are all reporting the same fact, and +// a hook runs at the end of every turn, so a wrong one cannot persist. +// +// Returns the id actually recorded, or null when there was nothing to record — +// no host key, or an id the host did not supply. +export async function writeHostPointer(rawId, { source = "unknown", key = hostKey() } = {}) { + const id = normalizeHostSessionId(rawId); + if (!id || !key) { + return null; + } + try { + await writePointer(hostPointerPath(key), { + sessionId: id, + source, + pid: process.pid, + updatedAt: new Date().toISOString() + }); + return id; + } catch { + // Recording this is never worth failing the command that triggered it: the + // caller keeps working against the environment, as it did before. + return null; + } +} + +// The session id to use, given what this process was started with. +// +// The pointer wins when the two disagree, because by construction it is the +// newer of the two: the environment was frozen when this process started and the +// pointer was written by a process that started later. +// +// The one case where that reasoning fails is a leftover pointer from a host that +// used to have this pid. It cannot describe a change that happened after this +// process started, so a *disagreeing* pointer older than this process is ignored. +// A real session change is always newer than the bridge that is reading it. +export async function resolveHostSessionId(envSessionId, { since = 0, key = hostKey() } = {}) { + const fallback = normalizeHostSessionId(envSessionId) ?? envSessionId ?? null; + const pointer = await readHostPointer(key); + if (!pointer || pointer.sessionId === envSessionId) { + return fallback; + } + if (since > 0 && pointer.updatedAt < since) { + return fallback; + } + return pointer.sessionId; +} + +// What the bridge resolved, published so a slash command can check its success +// against the process that will actually serve it rather than against its own +// write. +// +// Written when the answer changes and not on a timer: the bridge re-resolves +// several times a second and a file that says the same thing is not worth +// rewriting. Age is not staleness here — a record from an hour ago is still what +// this bridge is serving. Whether it is *running* is a question about its pid, +// which the record carries. +// +// The cache is what this process last wrote, not what is on disk. Something that +// deleted the file from underneath it would not get it back until the session +// changes; nothing here does that, and defending against an outside `rm` would +// cost a stat on every message. +let lastPublished = { id: undefined }; + +export async function publishBridgeSession(id, { key = hostKey(), now = Date.now() } = {}) { + if (!key) { + return false; + } + if (lastPublished.id === id) { + return false; + } + try { + await writePointer(bridgePointerPath(key), { + pid: process.pid, + sessionId: id ?? null, + updatedAt: new Date(now).toISOString() + }); + // Only once it is actually on disk: a write that failed has published + // nothing, and must not stop the next attempt. + lastPublished = { id }; + return true; + } catch { + return false; + } +} + +// Only a record left by a bridge that is still running says anything about what +// this session is being served right now. +export async function readBridgeSession(key = hostKey(), { alive = isProcessAlive } = {}) { + const record = key ? await readPointer(bridgePointerPath(key)) : null; + if (!record || record.pid === null || !alive(record.pid)) { + return null; + } + return record; +} + +// Waits for the bridge to be serving `id`, and reports what it found rather than +// deciding what to do about it: the caller is a command whose success message +// depends on the answer. +// +// `state` is "matched" (the bridge is on this session), "unknown" (no bridge is +// publishing, so there is nothing to check against — an older bridge, or none +// running at all), or "drifted". +export async function awaitBridgeSession( + id, + { timeoutMs = 3000, intervalMs = 100, key = hostKey() } = {} +) { + if (!id || !key) { + return { state: "unknown" }; + } + const deadline = Date.now() + timeoutMs; + let seen = null; + for (;;) { + seen = await readBridgeSession(key); + if (seen?.sessionId === id) { + return { state: "matched", seen }; + } + // Nothing is publishing: a bridge from before this existed, or none running. + // There is no answer coming, so waiting for one only delays the command. + if (!seen || Date.now() >= deadline) { + break; + } + await new Promise((resolve) => setTimeout(resolve, intervalMs)); + } + return seen ? { state: "drifted", seen } : { state: "unknown" }; +} + +// Pointers name a process, and processes end. Sweeping them keeps a machine that +// opens a lot of windows from accumulating a file per pid forever, and — more to +// the point — keeps a recycled pid from finding an ancient pointer waiting for +// it. Best effort: a failed sweep is not a failed command. +export async function pruneHostPointers({ alive = isProcessAlive } = {}) { + let entries; + try { + entries = await readdir(hostsDirectory()); + } catch { + return 0; + } + const mine = hostKey(); + let removed = 0; + for (const entry of entries) { + const key = entry.replace(/\.bridge\.json$|\.json$/, ""); + if (key === entry || key === mine) { + continue; + } + const pid = /^pid-([1-9][0-9]{0,9})$/.exec(key); + if (!pid || alive(Number(pid[1]))) { + continue; + } + await rm(path.join(hostsDirectory(), entry), { force: true }).catch(() => undefined); + removed += 1; + } + return removed; +} + +export function isProcessAlive(pid) { + try { + // Signal 0 checks for the process without touching it. EPERM means it exists + // and belongs to someone else, which still counts as alive. + process.kill(pid, 0); + return true; + } catch (error) { + return error?.code === "EPERM"; + } +} diff --git a/plugins/kimi-code/neatcontext/src/core/host-session.mjs b/plugins/kimi-code/neatcontext/src/core/host-session.mjs new file mode 100644 index 0000000..5e863d2 --- /dev/null +++ b/plugins/kimi-code/neatcontext/src/core/host-session.mjs @@ -0,0 +1,345 @@ +// Which session the host process is on *right now*. +// +// A host that identifies its sessions through the environment has a problem the +// rest of this plugin cannot see: the environment is a snapshot. Hosts spawn the +// MCP bridge once and keep it for the life of the window, but the session +// changes underneath it — starting a fresh conversation does not restart the +// server — and the bridge's copy of the id is whatever it was at spawn. The +// hooks and the slash-command CLI are spawned fresh and see the new one. +// +// So the two halves of the plugin end up reading and writing different files: +// `use` writes the selection for the session the user is in, the bridge keeps +// serving the session the window started in, and nothing in either path can +// observe the disagreement. The user is told a context is connected and every +// later answer is grounded in a different one. +// +// This module is the missing channel. One pointer file per host *process*, +// written by whichever short-lived process was just handed the current session +// id, and re-read by the long-lived one on every message: +// +// ~/.neatcontext/plugin-hosts/.json { sessionId, source, updatedAt } +// +// The host key has to be stable across a session change and distinct per window, +// which rules out the session id and the working directory both. What is left is +// the host process itself. How to name it is the one thing that differs per +// host, so each adapter registers its own answer with `configureHostPid()`: some +// hosts publish their own pid to every process they spawn, and a host that +// publishes nothing falls back to `process.ppid`, which is the host for any +// process it spawned directly. A process whose parent is not the host and whose +// host publishes no pid — a CLI running inside a shell tool — computes a key +// nothing else reads, and its writes are harmless and swept later. +// +// The bridge also publishes what it actually resolved: +// +// ~/.neatcontext/plugin-hosts/.bridge.json { pid, sessionId, updatedAt } +// +// That is what lets `use` check its own success against the process that will +// serve it, instead of against the file it just wrote itself. + +import { mkdir, readFile, readdir, rename, rm, writeFile } from "node:fs/promises"; +import path from "node:path"; +import { neatContextHome } from "./storage-home.mjs"; + +// A session id becomes a path segment (`plugin-sessions/.json`), and unlike +// the environment — which only the host can set — this one arrives from a file. +// Anything that could climb out of the 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 normalizeHostSessionId(value) { + if (typeof value !== "string") { + return null; + } + const id = value.trim(); + if (id === "." || id === ".." || !SAFE_SESSION_ID.test(id)) { + return null; + } + return id; +} + +function pidKey(value) { + const pid = typeof value === "string" ? value.trim() : String(value ?? ""); + return /^[1-9][0-9]{0,9}$/.test(pid) ? `pid-${pid}` : null; +} + +// Where the host publishes its own process id, when it publishes one at all. +// +// Deliberately per adapter rather than a list of every host's variable checked +// in turn: those variables are inherited by child processes, so a host launched +// from another host's shell would key on the outer host and two different hosts +// could end up sharing one pointer file. Each adapter consults only the variable +// its own host sets. +// +// Process-global, because one process runs one host's adapter. A test that +// imported two adapters into the same process would have the second silently +// win; there is no such test, and there should not be one. +let hostPidProvider = null; + +export function configureHostPid(provider) { + if (provider !== null && typeof provider !== "function") { + throw new TypeError("The host pid provider must be a function or null."); + } + hostPidProvider = provider; +} + +function hostPid() { + if (!hostPidProvider) { + return null; + } + // An adapter reading a value the host never set must not take down the only + // path that can name this process's host. + try { + return hostPidProvider(); + } catch { + return null; + } +} + +// The host process this process belongs to. +// +// NEATCONTEXT_HOST_KEY is the explicit form, for tests and for any host that can +// hand every one of its plugin processes the same identifier. Otherwise the pid +// the adapter knows how to find, and failing that `process.ppid`, which is the +// host for a server or hook it spawned directly. +export function hostKey() { + const explicit = process.env.NEATCONTEXT_HOST_KEY; + if (typeof explicit === "string") { + const trimmed = explicit.trim(); + return trimmed.length > 0 && trimmed !== "." && trimmed !== ".." && !trimmed.includes("/") && + !trimmed.includes("\\") + ? trimmed + : null; + } + return pidKey(hostPid()) ?? pidKey(process.ppid); +} + +export function hostsDirectory() { + return path.join(neatContextHome(), "plugin-hosts"); +} + +export function hostPointerPath(key) { + return path.join(hostsDirectory(), `${key}.json`); +} + +export function bridgePointerPath(key) { + return path.join(hostsDirectory(), `${key}.bridge.json`); +} + +async function readPointer(file) { + try { + const parsed = JSON.parse(await readFile(file, "utf8")); + const id = normalizeHostSessionId(parsed?.sessionId); + if (!id) { + return null; + } + const updatedAt = Date.parse(parsed?.updatedAt); + return { + sessionId: id, + source: typeof parsed?.source === "string" ? parsed.source : "unknown", + updatedAt: Number.isNaN(updatedAt) ? 0 : updatedAt, + pid: typeof parsed?.pid === "number" ? parsed.pid : null + }; + } catch { + // Missing, half-written, or hand-broken: the caller falls back to the + // environment, which is exactly the behavior that predates this file. + return null; + } +} + +let writeCounter = 0; + +// Written to a temporary name and renamed into place, because these files are +// read by other processes while they are being written: the bridge re-reads the +// pointer on every message, and a reader that catches a half-written file falls +// back to the environment, which is the stale answer this file exists to +// correct. A rename is atomic, so a reader sees the old record or the new one. +// +// The temporary name carries this process's pid so that two processes writing +// the same pointer at once cannot collide on it, and stays in the same directory +// so the rename never crosses a volume. +async function writePointer(file, value) { + await mkdir(path.dirname(file), { recursive: true }); + writeCounter += 1; + const temporary = `${file}.${process.pid}.${writeCounter}.tmp`; + try { + await writeFile(temporary, `${JSON.stringify(value, null, 2)}\n`, { + encoding: "utf8", + mode: 0o600 + }); + await rename(temporary, file); + } catch (error) { + await rm(temporary, { force: true }).catch(() => undefined); + throw error; + } +} + +export async function readHostPointer(key = hostKey()) { + return key ? readPointer(hostPointerPath(key)) : null; +} + +// Called by every process the host hands a session id to: the hooks, which are +// told on stdin, and the CLI, whose environment is fresh because it was spawned +// for this command. Last write wins — they are all reporting the same fact, and +// a hook runs at the end of every turn, so a wrong one cannot persist. +// +// Returns the id actually recorded, or null when there was nothing to record — +// no host key, or an id the host did not supply. +export async function writeHostPointer(rawId, { source = "unknown", key = hostKey() } = {}) { + const id = normalizeHostSessionId(rawId); + if (!id || !key) { + return null; + } + try { + await writePointer(hostPointerPath(key), { + sessionId: id, + source, + pid: process.pid, + updatedAt: new Date().toISOString() + }); + return id; + } catch { + // Recording this is never worth failing the command that triggered it: the + // caller keeps working against the environment, as it did before. + return null; + } +} + +// The session id to use, given what this process was started with. +// +// The pointer wins when the two disagree, because by construction it is the +// newer of the two: the environment was frozen when this process started and the +// pointer was written by a process that started later. +// +// The one case where that reasoning fails is a leftover pointer from a host that +// used to have this pid. It cannot describe a change that happened after this +// process started, so a *disagreeing* pointer older than this process is ignored. +// A real session change is always newer than the bridge that is reading it. +export async function resolveHostSessionId(envSessionId, { since = 0, key = hostKey() } = {}) { + const fallback = normalizeHostSessionId(envSessionId) ?? envSessionId ?? null; + const pointer = await readHostPointer(key); + if (!pointer || pointer.sessionId === envSessionId) { + return fallback; + } + if (since > 0 && pointer.updatedAt < since) { + return fallback; + } + return pointer.sessionId; +} + +// What the bridge resolved, published so a slash command can check its success +// against the process that will actually serve it rather than against its own +// write. +// +// Written when the answer changes and not on a timer: the bridge re-resolves +// several times a second and a file that says the same thing is not worth +// rewriting. Age is not staleness here — a record from an hour ago is still what +// this bridge is serving. Whether it is *running* is a question about its pid, +// which the record carries. +// +// The cache is what this process last wrote, not what is on disk. Something that +// deleted the file from underneath it would not get it back until the session +// changes; nothing here does that, and defending against an outside `rm` would +// cost a stat on every message. +let lastPublished = { id: undefined }; + +export async function publishBridgeSession(id, { key = hostKey(), now = Date.now() } = {}) { + if (!key) { + return false; + } + if (lastPublished.id === id) { + return false; + } + try { + await writePointer(bridgePointerPath(key), { + pid: process.pid, + sessionId: id ?? null, + updatedAt: new Date(now).toISOString() + }); + // Only once it is actually on disk: a write that failed has published + // nothing, and must not stop the next attempt. + lastPublished = { id }; + return true; + } catch { + return false; + } +} + +// Only a record left by a bridge that is still running says anything about what +// this session is being served right now. +export async function readBridgeSession(key = hostKey(), { alive = isProcessAlive } = {}) { + const record = key ? await readPointer(bridgePointerPath(key)) : null; + if (!record || record.pid === null || !alive(record.pid)) { + return null; + } + return record; +} + +// Waits for the bridge to be serving `id`, and reports what it found rather than +// deciding what to do about it: the caller is a command whose success message +// depends on the answer. +// +// `state` is "matched" (the bridge is on this session), "unknown" (no bridge is +// publishing, so there is nothing to check against — an older bridge, or none +// running at all), or "drifted". +export async function awaitBridgeSession( + id, + { timeoutMs = 3000, intervalMs = 100, key = hostKey() } = {} +) { + if (!id || !key) { + return { state: "unknown" }; + } + const deadline = Date.now() + timeoutMs; + let seen = null; + for (;;) { + seen = await readBridgeSession(key); + if (seen?.sessionId === id) { + return { state: "matched", seen }; + } + // Nothing is publishing: a bridge from before this existed, or none running. + // There is no answer coming, so waiting for one only delays the command. + if (!seen || Date.now() >= deadline) { + break; + } + await new Promise((resolve) => setTimeout(resolve, intervalMs)); + } + return seen ? { state: "drifted", seen } : { state: "unknown" }; +} + +// Pointers name a process, and processes end. Sweeping them keeps a machine that +// opens a lot of windows from accumulating a file per pid forever, and — more to +// the point — keeps a recycled pid from finding an ancient pointer waiting for +// it. Best effort: a failed sweep is not a failed command. +export async function pruneHostPointers({ alive = isProcessAlive } = {}) { + let entries; + try { + entries = await readdir(hostsDirectory()); + } catch { + return 0; + } + const mine = hostKey(); + let removed = 0; + for (const entry of entries) { + const key = entry.replace(/\.bridge\.json$|\.json$/, ""); + if (key === entry || key === mine) { + continue; + } + const pid = /^pid-([1-9][0-9]{0,9})$/.exec(key); + if (!pid || alive(Number(pid[1]))) { + continue; + } + await rm(path.join(hostsDirectory(), entry), { force: true }).catch(() => undefined); + removed += 1; + } + return removed; +} + +export function isProcessAlive(pid) { + try { + // Signal 0 checks for the process without touching it. EPERM means it exists + // and belongs to someone else, which still counts as alive. + process.kill(pid, 0); + return true; + } catch (error) { + return error?.code === "EPERM"; + } +} diff --git a/plugins/pi/neatcontext/package.json b/plugins/pi/neatcontext/package.json index 41ea3a9..d05915a 100644 --- a/plugins/pi/neatcontext/package.json +++ b/plugins/pi/neatcontext/package.json @@ -46,7 +46,7 @@ "access": "public" }, "scripts": { - "check": "node --check extensions/neatcontext.js && node --check src/pi/runtime.mjs && node --check src/pi/session.mjs && node --check src/core/context-store.mjs && node --check src/core/extension-bindings.mjs && node --check src/core/extension-commands.mjs && node --check src/core/extension-runtime.mjs && node --check src/core/extensions.mjs && node --check src/core/mcp-stdio-client.mjs && node --check src/core/local-state.mjs && node --check src/core/conversation-evidence.mjs && node --check src/core/routing.mjs && node --check src/core/routing-search.mjs && node --check src/core/routing-candidates.mjs && node --check src/core/selection.mjs && node --check src/core/session.mjs && node --check src/core/storage-home.mjs", + "check": "node --check extensions/neatcontext.js && node --check src/pi/runtime.mjs && node --check src/pi/session.mjs && node --check src/core/context-store.mjs && node --check src/core/extension-bindings.mjs && node --check src/core/extension-commands.mjs && node --check src/core/extension-runtime.mjs && node --check src/core/extensions.mjs && node --check src/core/mcp-stdio-client.mjs && node --check src/core/local-state.mjs && node --check src/core/host-session.mjs && node --check src/core/conversation-evidence.mjs && node --check src/core/routing.mjs && node --check src/core/routing-search.mjs && node --check src/core/routing-candidates.mjs && node --check src/core/selection.mjs && node --check src/core/session.mjs && node --check src/core/storage-home.mjs", "test": "node --test" } } diff --git a/plugins/pi/neatcontext/src/core/host-session.mjs b/plugins/pi/neatcontext/src/core/host-session.mjs new file mode 100644 index 0000000..5e863d2 --- /dev/null +++ b/plugins/pi/neatcontext/src/core/host-session.mjs @@ -0,0 +1,345 @@ +// Which session the host process is on *right now*. +// +// A host that identifies its sessions through the environment has a problem the +// rest of this plugin cannot see: the environment is a snapshot. Hosts spawn the +// MCP bridge once and keep it for the life of the window, but the session +// changes underneath it — starting a fresh conversation does not restart the +// server — and the bridge's copy of the id is whatever it was at spawn. The +// hooks and the slash-command CLI are spawned fresh and see the new one. +// +// So the two halves of the plugin end up reading and writing different files: +// `use` writes the selection for the session the user is in, the bridge keeps +// serving the session the window started in, and nothing in either path can +// observe the disagreement. The user is told a context is connected and every +// later answer is grounded in a different one. +// +// This module is the missing channel. One pointer file per host *process*, +// written by whichever short-lived process was just handed the current session +// id, and re-read by the long-lived one on every message: +// +// ~/.neatcontext/plugin-hosts/.json { sessionId, source, updatedAt } +// +// The host key has to be stable across a session change and distinct per window, +// which rules out the session id and the working directory both. What is left is +// the host process itself. How to name it is the one thing that differs per +// host, so each adapter registers its own answer with `configureHostPid()`: some +// hosts publish their own pid to every process they spawn, and a host that +// publishes nothing falls back to `process.ppid`, which is the host for any +// process it spawned directly. A process whose parent is not the host and whose +// host publishes no pid — a CLI running inside a shell tool — computes a key +// nothing else reads, and its writes are harmless and swept later. +// +// The bridge also publishes what it actually resolved: +// +// ~/.neatcontext/plugin-hosts/.bridge.json { pid, sessionId, updatedAt } +// +// That is what lets `use` check its own success against the process that will +// serve it, instead of against the file it just wrote itself. + +import { mkdir, readFile, readdir, rename, rm, writeFile } from "node:fs/promises"; +import path from "node:path"; +import { neatContextHome } from "./storage-home.mjs"; + +// A session id becomes a path segment (`plugin-sessions/.json`), and unlike +// the environment — which only the host can set — this one arrives from a file. +// Anything that could climb out of the 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 normalizeHostSessionId(value) { + if (typeof value !== "string") { + return null; + } + const id = value.trim(); + if (id === "." || id === ".." || !SAFE_SESSION_ID.test(id)) { + return null; + } + return id; +} + +function pidKey(value) { + const pid = typeof value === "string" ? value.trim() : String(value ?? ""); + return /^[1-9][0-9]{0,9}$/.test(pid) ? `pid-${pid}` : null; +} + +// Where the host publishes its own process id, when it publishes one at all. +// +// Deliberately per adapter rather than a list of every host's variable checked +// in turn: those variables are inherited by child processes, so a host launched +// from another host's shell would key on the outer host and two different hosts +// could end up sharing one pointer file. Each adapter consults only the variable +// its own host sets. +// +// Process-global, because one process runs one host's adapter. A test that +// imported two adapters into the same process would have the second silently +// win; there is no such test, and there should not be one. +let hostPidProvider = null; + +export function configureHostPid(provider) { + if (provider !== null && typeof provider !== "function") { + throw new TypeError("The host pid provider must be a function or null."); + } + hostPidProvider = provider; +} + +function hostPid() { + if (!hostPidProvider) { + return null; + } + // An adapter reading a value the host never set must not take down the only + // path that can name this process's host. + try { + return hostPidProvider(); + } catch { + return null; + } +} + +// The host process this process belongs to. +// +// NEATCONTEXT_HOST_KEY is the explicit form, for tests and for any host that can +// hand every one of its plugin processes the same identifier. Otherwise the pid +// the adapter knows how to find, and failing that `process.ppid`, which is the +// host for a server or hook it spawned directly. +export function hostKey() { + const explicit = process.env.NEATCONTEXT_HOST_KEY; + if (typeof explicit === "string") { + const trimmed = explicit.trim(); + return trimmed.length > 0 && trimmed !== "." && trimmed !== ".." && !trimmed.includes("/") && + !trimmed.includes("\\") + ? trimmed + : null; + } + return pidKey(hostPid()) ?? pidKey(process.ppid); +} + +export function hostsDirectory() { + return path.join(neatContextHome(), "plugin-hosts"); +} + +export function hostPointerPath(key) { + return path.join(hostsDirectory(), `${key}.json`); +} + +export function bridgePointerPath(key) { + return path.join(hostsDirectory(), `${key}.bridge.json`); +} + +async function readPointer(file) { + try { + const parsed = JSON.parse(await readFile(file, "utf8")); + const id = normalizeHostSessionId(parsed?.sessionId); + if (!id) { + return null; + } + const updatedAt = Date.parse(parsed?.updatedAt); + return { + sessionId: id, + source: typeof parsed?.source === "string" ? parsed.source : "unknown", + updatedAt: Number.isNaN(updatedAt) ? 0 : updatedAt, + pid: typeof parsed?.pid === "number" ? parsed.pid : null + }; + } catch { + // Missing, half-written, or hand-broken: the caller falls back to the + // environment, which is exactly the behavior that predates this file. + return null; + } +} + +let writeCounter = 0; + +// Written to a temporary name and renamed into place, because these files are +// read by other processes while they are being written: the bridge re-reads the +// pointer on every message, and a reader that catches a half-written file falls +// back to the environment, which is the stale answer this file exists to +// correct. A rename is atomic, so a reader sees the old record or the new one. +// +// The temporary name carries this process's pid so that two processes writing +// the same pointer at once cannot collide on it, and stays in the same directory +// so the rename never crosses a volume. +async function writePointer(file, value) { + await mkdir(path.dirname(file), { recursive: true }); + writeCounter += 1; + const temporary = `${file}.${process.pid}.${writeCounter}.tmp`; + try { + await writeFile(temporary, `${JSON.stringify(value, null, 2)}\n`, { + encoding: "utf8", + mode: 0o600 + }); + await rename(temporary, file); + } catch (error) { + await rm(temporary, { force: true }).catch(() => undefined); + throw error; + } +} + +export async function readHostPointer(key = hostKey()) { + return key ? readPointer(hostPointerPath(key)) : null; +} + +// Called by every process the host hands a session id to: the hooks, which are +// told on stdin, and the CLI, whose environment is fresh because it was spawned +// for this command. Last write wins — they are all reporting the same fact, and +// a hook runs at the end of every turn, so a wrong one cannot persist. +// +// Returns the id actually recorded, or null when there was nothing to record — +// no host key, or an id the host did not supply. +export async function writeHostPointer(rawId, { source = "unknown", key = hostKey() } = {}) { + const id = normalizeHostSessionId(rawId); + if (!id || !key) { + return null; + } + try { + await writePointer(hostPointerPath(key), { + sessionId: id, + source, + pid: process.pid, + updatedAt: new Date().toISOString() + }); + return id; + } catch { + // Recording this is never worth failing the command that triggered it: the + // caller keeps working against the environment, as it did before. + return null; + } +} + +// The session id to use, given what this process was started with. +// +// The pointer wins when the two disagree, because by construction it is the +// newer of the two: the environment was frozen when this process started and the +// pointer was written by a process that started later. +// +// The one case where that reasoning fails is a leftover pointer from a host that +// used to have this pid. It cannot describe a change that happened after this +// process started, so a *disagreeing* pointer older than this process is ignored. +// A real session change is always newer than the bridge that is reading it. +export async function resolveHostSessionId(envSessionId, { since = 0, key = hostKey() } = {}) { + const fallback = normalizeHostSessionId(envSessionId) ?? envSessionId ?? null; + const pointer = await readHostPointer(key); + if (!pointer || pointer.sessionId === envSessionId) { + return fallback; + } + if (since > 0 && pointer.updatedAt < since) { + return fallback; + } + return pointer.sessionId; +} + +// What the bridge resolved, published so a slash command can check its success +// against the process that will actually serve it rather than against its own +// write. +// +// Written when the answer changes and not on a timer: the bridge re-resolves +// several times a second and a file that says the same thing is not worth +// rewriting. Age is not staleness here — a record from an hour ago is still what +// this bridge is serving. Whether it is *running* is a question about its pid, +// which the record carries. +// +// The cache is what this process last wrote, not what is on disk. Something that +// deleted the file from underneath it would not get it back until the session +// changes; nothing here does that, and defending against an outside `rm` would +// cost a stat on every message. +let lastPublished = { id: undefined }; + +export async function publishBridgeSession(id, { key = hostKey(), now = Date.now() } = {}) { + if (!key) { + return false; + } + if (lastPublished.id === id) { + return false; + } + try { + await writePointer(bridgePointerPath(key), { + pid: process.pid, + sessionId: id ?? null, + updatedAt: new Date(now).toISOString() + }); + // Only once it is actually on disk: a write that failed has published + // nothing, and must not stop the next attempt. + lastPublished = { id }; + return true; + } catch { + return false; + } +} + +// Only a record left by a bridge that is still running says anything about what +// this session is being served right now. +export async function readBridgeSession(key = hostKey(), { alive = isProcessAlive } = {}) { + const record = key ? await readPointer(bridgePointerPath(key)) : null; + if (!record || record.pid === null || !alive(record.pid)) { + return null; + } + return record; +} + +// Waits for the bridge to be serving `id`, and reports what it found rather than +// deciding what to do about it: the caller is a command whose success message +// depends on the answer. +// +// `state` is "matched" (the bridge is on this session), "unknown" (no bridge is +// publishing, so there is nothing to check against — an older bridge, or none +// running at all), or "drifted". +export async function awaitBridgeSession( + id, + { timeoutMs = 3000, intervalMs = 100, key = hostKey() } = {} +) { + if (!id || !key) { + return { state: "unknown" }; + } + const deadline = Date.now() + timeoutMs; + let seen = null; + for (;;) { + seen = await readBridgeSession(key); + if (seen?.sessionId === id) { + return { state: "matched", seen }; + } + // Nothing is publishing: a bridge from before this existed, or none running. + // There is no answer coming, so waiting for one only delays the command. + if (!seen || Date.now() >= deadline) { + break; + } + await new Promise((resolve) => setTimeout(resolve, intervalMs)); + } + return seen ? { state: "drifted", seen } : { state: "unknown" }; +} + +// Pointers name a process, and processes end. Sweeping them keeps a machine that +// opens a lot of windows from accumulating a file per pid forever, and — more to +// the point — keeps a recycled pid from finding an ancient pointer waiting for +// it. Best effort: a failed sweep is not a failed command. +export async function pruneHostPointers({ alive = isProcessAlive } = {}) { + let entries; + try { + entries = await readdir(hostsDirectory()); + } catch { + return 0; + } + const mine = hostKey(); + let removed = 0; + for (const entry of entries) { + const key = entry.replace(/\.bridge\.json$|\.json$/, ""); + if (key === entry || key === mine) { + continue; + } + const pid = /^pid-([1-9][0-9]{0,9})$/.exec(key); + if (!pid || alive(Number(pid[1]))) { + continue; + } + await rm(path.join(hostsDirectory(), entry), { force: true }).catch(() => undefined); + removed += 1; + } + return removed; +} + +export function isProcessAlive(pid) { + try { + // Signal 0 checks for the process without touching it. EPERM means it exists + // and belongs to someone else, which still counts as alive. + process.kill(pid, 0); + return true; + } catch (error) { + return error?.code === "EPERM"; + } +} diff --git a/shared/core/host-session.mjs b/shared/core/host-session.mjs new file mode 100644 index 0000000..5e863d2 --- /dev/null +++ b/shared/core/host-session.mjs @@ -0,0 +1,345 @@ +// Which session the host process is on *right now*. +// +// A host that identifies its sessions through the environment has a problem the +// rest of this plugin cannot see: the environment is a snapshot. Hosts spawn the +// MCP bridge once and keep it for the life of the window, but the session +// changes underneath it — starting a fresh conversation does not restart the +// server — and the bridge's copy of the id is whatever it was at spawn. The +// hooks and the slash-command CLI are spawned fresh and see the new one. +// +// So the two halves of the plugin end up reading and writing different files: +// `use` writes the selection for the session the user is in, the bridge keeps +// serving the session the window started in, and nothing in either path can +// observe the disagreement. The user is told a context is connected and every +// later answer is grounded in a different one. +// +// This module is the missing channel. One pointer file per host *process*, +// written by whichever short-lived process was just handed the current session +// id, and re-read by the long-lived one on every message: +// +// ~/.neatcontext/plugin-hosts/.json { sessionId, source, updatedAt } +// +// The host key has to be stable across a session change and distinct per window, +// which rules out the session id and the working directory both. What is left is +// the host process itself. How to name it is the one thing that differs per +// host, so each adapter registers its own answer with `configureHostPid()`: some +// hosts publish their own pid to every process they spawn, and a host that +// publishes nothing falls back to `process.ppid`, which is the host for any +// process it spawned directly. A process whose parent is not the host and whose +// host publishes no pid — a CLI running inside a shell tool — computes a key +// nothing else reads, and its writes are harmless and swept later. +// +// The bridge also publishes what it actually resolved: +// +// ~/.neatcontext/plugin-hosts/.bridge.json { pid, sessionId, updatedAt } +// +// That is what lets `use` check its own success against the process that will +// serve it, instead of against the file it just wrote itself. + +import { mkdir, readFile, readdir, rename, rm, writeFile } from "node:fs/promises"; +import path from "node:path"; +import { neatContextHome } from "./storage-home.mjs"; + +// A session id becomes a path segment (`plugin-sessions/.json`), and unlike +// the environment — which only the host can set — this one arrives from a file. +// Anything that could climb out of the 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 normalizeHostSessionId(value) { + if (typeof value !== "string") { + return null; + } + const id = value.trim(); + if (id === "." || id === ".." || !SAFE_SESSION_ID.test(id)) { + return null; + } + return id; +} + +function pidKey(value) { + const pid = typeof value === "string" ? value.trim() : String(value ?? ""); + return /^[1-9][0-9]{0,9}$/.test(pid) ? `pid-${pid}` : null; +} + +// Where the host publishes its own process id, when it publishes one at all. +// +// Deliberately per adapter rather than a list of every host's variable checked +// in turn: those variables are inherited by child processes, so a host launched +// from another host's shell would key on the outer host and two different hosts +// could end up sharing one pointer file. Each adapter consults only the variable +// its own host sets. +// +// Process-global, because one process runs one host's adapter. A test that +// imported two adapters into the same process would have the second silently +// win; there is no such test, and there should not be one. +let hostPidProvider = null; + +export function configureHostPid(provider) { + if (provider !== null && typeof provider !== "function") { + throw new TypeError("The host pid provider must be a function or null."); + } + hostPidProvider = provider; +} + +function hostPid() { + if (!hostPidProvider) { + return null; + } + // An adapter reading a value the host never set must not take down the only + // path that can name this process's host. + try { + return hostPidProvider(); + } catch { + return null; + } +} + +// The host process this process belongs to. +// +// NEATCONTEXT_HOST_KEY is the explicit form, for tests and for any host that can +// hand every one of its plugin processes the same identifier. Otherwise the pid +// the adapter knows how to find, and failing that `process.ppid`, which is the +// host for a server or hook it spawned directly. +export function hostKey() { + const explicit = process.env.NEATCONTEXT_HOST_KEY; + if (typeof explicit === "string") { + const trimmed = explicit.trim(); + return trimmed.length > 0 && trimmed !== "." && trimmed !== ".." && !trimmed.includes("/") && + !trimmed.includes("\\") + ? trimmed + : null; + } + return pidKey(hostPid()) ?? pidKey(process.ppid); +} + +export function hostsDirectory() { + return path.join(neatContextHome(), "plugin-hosts"); +} + +export function hostPointerPath(key) { + return path.join(hostsDirectory(), `${key}.json`); +} + +export function bridgePointerPath(key) { + return path.join(hostsDirectory(), `${key}.bridge.json`); +} + +async function readPointer(file) { + try { + const parsed = JSON.parse(await readFile(file, "utf8")); + const id = normalizeHostSessionId(parsed?.sessionId); + if (!id) { + return null; + } + const updatedAt = Date.parse(parsed?.updatedAt); + return { + sessionId: id, + source: typeof parsed?.source === "string" ? parsed.source : "unknown", + updatedAt: Number.isNaN(updatedAt) ? 0 : updatedAt, + pid: typeof parsed?.pid === "number" ? parsed.pid : null + }; + } catch { + // Missing, half-written, or hand-broken: the caller falls back to the + // environment, which is exactly the behavior that predates this file. + return null; + } +} + +let writeCounter = 0; + +// Written to a temporary name and renamed into place, because these files are +// read by other processes while they are being written: the bridge re-reads the +// pointer on every message, and a reader that catches a half-written file falls +// back to the environment, which is the stale answer this file exists to +// correct. A rename is atomic, so a reader sees the old record or the new one. +// +// The temporary name carries this process's pid so that two processes writing +// the same pointer at once cannot collide on it, and stays in the same directory +// so the rename never crosses a volume. +async function writePointer(file, value) { + await mkdir(path.dirname(file), { recursive: true }); + writeCounter += 1; + const temporary = `${file}.${process.pid}.${writeCounter}.tmp`; + try { + await writeFile(temporary, `${JSON.stringify(value, null, 2)}\n`, { + encoding: "utf8", + mode: 0o600 + }); + await rename(temporary, file); + } catch (error) { + await rm(temporary, { force: true }).catch(() => undefined); + throw error; + } +} + +export async function readHostPointer(key = hostKey()) { + return key ? readPointer(hostPointerPath(key)) : null; +} + +// Called by every process the host hands a session id to: the hooks, which are +// told on stdin, and the CLI, whose environment is fresh because it was spawned +// for this command. Last write wins — they are all reporting the same fact, and +// a hook runs at the end of every turn, so a wrong one cannot persist. +// +// Returns the id actually recorded, or null when there was nothing to record — +// no host key, or an id the host did not supply. +export async function writeHostPointer(rawId, { source = "unknown", key = hostKey() } = {}) { + const id = normalizeHostSessionId(rawId); + if (!id || !key) { + return null; + } + try { + await writePointer(hostPointerPath(key), { + sessionId: id, + source, + pid: process.pid, + updatedAt: new Date().toISOString() + }); + return id; + } catch { + // Recording this is never worth failing the command that triggered it: the + // caller keeps working against the environment, as it did before. + return null; + } +} + +// The session id to use, given what this process was started with. +// +// The pointer wins when the two disagree, because by construction it is the +// newer of the two: the environment was frozen when this process started and the +// pointer was written by a process that started later. +// +// The one case where that reasoning fails is a leftover pointer from a host that +// used to have this pid. It cannot describe a change that happened after this +// process started, so a *disagreeing* pointer older than this process is ignored. +// A real session change is always newer than the bridge that is reading it. +export async function resolveHostSessionId(envSessionId, { since = 0, key = hostKey() } = {}) { + const fallback = normalizeHostSessionId(envSessionId) ?? envSessionId ?? null; + const pointer = await readHostPointer(key); + if (!pointer || pointer.sessionId === envSessionId) { + return fallback; + } + if (since > 0 && pointer.updatedAt < since) { + return fallback; + } + return pointer.sessionId; +} + +// What the bridge resolved, published so a slash command can check its success +// against the process that will actually serve it rather than against its own +// write. +// +// Written when the answer changes and not on a timer: the bridge re-resolves +// several times a second and a file that says the same thing is not worth +// rewriting. Age is not staleness here — a record from an hour ago is still what +// this bridge is serving. Whether it is *running* is a question about its pid, +// which the record carries. +// +// The cache is what this process last wrote, not what is on disk. Something that +// deleted the file from underneath it would not get it back until the session +// changes; nothing here does that, and defending against an outside `rm` would +// cost a stat on every message. +let lastPublished = { id: undefined }; + +export async function publishBridgeSession(id, { key = hostKey(), now = Date.now() } = {}) { + if (!key) { + return false; + } + if (lastPublished.id === id) { + return false; + } + try { + await writePointer(bridgePointerPath(key), { + pid: process.pid, + sessionId: id ?? null, + updatedAt: new Date(now).toISOString() + }); + // Only once it is actually on disk: a write that failed has published + // nothing, and must not stop the next attempt. + lastPublished = { id }; + return true; + } catch { + return false; + } +} + +// Only a record left by a bridge that is still running says anything about what +// this session is being served right now. +export async function readBridgeSession(key = hostKey(), { alive = isProcessAlive } = {}) { + const record = key ? await readPointer(bridgePointerPath(key)) : null; + if (!record || record.pid === null || !alive(record.pid)) { + return null; + } + return record; +} + +// Waits for the bridge to be serving `id`, and reports what it found rather than +// deciding what to do about it: the caller is a command whose success message +// depends on the answer. +// +// `state` is "matched" (the bridge is on this session), "unknown" (no bridge is +// publishing, so there is nothing to check against — an older bridge, or none +// running at all), or "drifted". +export async function awaitBridgeSession( + id, + { timeoutMs = 3000, intervalMs = 100, key = hostKey() } = {} +) { + if (!id || !key) { + return { state: "unknown" }; + } + const deadline = Date.now() + timeoutMs; + let seen = null; + for (;;) { + seen = await readBridgeSession(key); + if (seen?.sessionId === id) { + return { state: "matched", seen }; + } + // Nothing is publishing: a bridge from before this existed, or none running. + // There is no answer coming, so waiting for one only delays the command. + if (!seen || Date.now() >= deadline) { + break; + } + await new Promise((resolve) => setTimeout(resolve, intervalMs)); + } + return seen ? { state: "drifted", seen } : { state: "unknown" }; +} + +// Pointers name a process, and processes end. Sweeping them keeps a machine that +// opens a lot of windows from accumulating a file per pid forever, and — more to +// the point — keeps a recycled pid from finding an ancient pointer waiting for +// it. Best effort: a failed sweep is not a failed command. +export async function pruneHostPointers({ alive = isProcessAlive } = {}) { + let entries; + try { + entries = await readdir(hostsDirectory()); + } catch { + return 0; + } + const mine = hostKey(); + let removed = 0; + for (const entry of entries) { + const key = entry.replace(/\.bridge\.json$|\.json$/, ""); + if (key === entry || key === mine) { + continue; + } + const pid = /^pid-([1-9][0-9]{0,9})$/.exec(key); + if (!pid || alive(Number(pid[1]))) { + continue; + } + await rm(path.join(hostsDirectory(), entry), { force: true }).catch(() => undefined); + removed += 1; + } + return removed; +} + +export function isProcessAlive(pid) { + try { + // Signal 0 checks for the process without touching it. EPERM means it exists + // and belongs to someone else, which still counts as alive. + process.kill(pid, 0); + return true; + } catch (error) { + return error?.code === "EPERM"; + } +} diff --git a/tests/copilot-plugin.test.mjs b/tests/copilot-plugin.test.mjs index e7e7734..ec966d1 100644 --- a/tests/copilot-plugin.test.mjs +++ b/tests/copilot-plugin.test.mjs @@ -186,11 +186,24 @@ function toolCall(id, name, args = {}) { } // One isolated NeatContext home per test. +// +// The host variables are blanked rather than inherited: this suite runs *inside* +// a Copilot session on a developer machine, and a real COPILOT_AGENT_SESSION_ID +// leaking into every child would silently replace the identity under test. An +// empty value is "not set" everywhere it is read, so each test exercises the +// same fallbacks a clean host would. The host key is pinned per home so the +// pointer files of two tests running concurrently cannot collide. async function isolatedHome(prefix) { const directory = await mkdtemp(path.join(os.tmpdir(), prefix)); return { directory, - env: { NEATCONTEXT_HOME: directory } + env: { + NEATCONTEXT_HOME: directory, + NEATCONTEXT_SESSION_ID: "", + COPILOT_AGENT_SESSION_ID: "", + COPILOT_LOADER_PID: "", + NEATCONTEXT_HOST_KEY: `test-${path.basename(directory)}` + } }; } diff --git a/tests/copilot-session-drift.test.mjs b/tests/copilot-session-drift.test.mjs new file mode 100644 index 0000000..5ebebb5 --- /dev/null +++ b/tests/copilot-session-drift.test.mjs @@ -0,0 +1,418 @@ +// Regression tests for the reported failure on GitHub Copilot: `/neatcontext:use` +// reports a context connected while `get_context` keeps answering from a +// different one — with no error anywhere, because both halves are telling the +// truth about different files. +// +// Copilot's two plugin processes are not given the same working directory. The +// MCP bridge is spawned with the *plugin installation* directory; the CLI a +// slash command runs is spawned with the user's workspace. The adapter derived +// the session by hashing `process.cwd()`, so the two halves hashed different +// paths and scoped to different selection files. +// +// What is reproduced here is exactly that split, and nothing else: one bridge +// process with the plugin directory as its cwd, CLI processes with the workspace +// as theirs. Copilot ships no hooks, so the CLI is the only process that can +// ever tell the bridge what session the window is on. + +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)), + "..", + "plugins", + "copilot", + "neatcontext" +); +const copilot = path.join(plugin, "src", "copilot"); + +const HOST = "copilot-window"; +const OTHER_HOST = "copilot-window-2"; + +let home; +let workspace; +let otherWorkspace; +let hostsDirectory; + +// The environment Copilot hands a plugin process. `NEATCONTEXT_HOST_KEY` stands +// in for COPILOT_LOADER_PID so a test can say which window a process belongs to +// without depending on the real process tree. +function childEnv({ sessionId, host = HOST } = {}) { + return { + ...process.env, + // Empty is "not set" everywhere it is read, so a test that omits the session + // id exercises the workspace fallback even though this suite is itself run + // from a Copilot session whose id would otherwise be inherited. + COPILOT_AGENT_SESSION_ID: sessionId ?? "", + COPILOT_LOADER_PID: "", + NEATCONTEXT_SESSION_ID: "", + NEATCONTEXT_HOST_KEY: host, + NEATCONTEXT_HOME: home + }; +} + +before(async () => { + home = await mkdtemp(path.join(os.tmpdir(), "neatcontext-copilot-drift-")); + workspace = await mkdtemp(path.join(os.tmpdir(), "copilot-workspace-")); + otherWorkspace = await mkdtemp(path.join(os.tmpdir(), "copilot-workspace-b-")); + 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("../plugins/copilot/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 () => { + for (const directory of [home, workspace, otherWorkspace]) { + await rm(directory, { 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 }); +}); + +// A slash command. Copilot spawns it in the user's workspace — which is the +// directory the bridge is *not* given. +function cli(args, { sessionId, host = HOST, cwd = workspace } = {}) { + return new Promise((resolve) => { + const child = spawn( + process.execPath, + [path.join(copilot, "neatcontext-cli.mjs"), ...args], + { cwd, stdio: ["ignore", "pipe", "inherit"], env: childEnv({ sessionId, host }) } + ); + let out = ""; + child.stdout.on("data", (chunk) => (out += chunk)); + child.on("exit", () => resolve(out.trim())); + }); +} + +// A window: one bridge kept alive for its lifetime, spawned the way Copilot +// spawns it — with the plugin installation directory as its working directory, +// not the workspace. +function openWindow({ sessionId, host = HOST, cwd = plugin } = {}) { + const child = spawn(process.execPath, [path.join(copilot, "mcp-bridge.mjs")], { + cwd, + stdio: ["pipe", "pipe", "inherit"], + env: childEnv({ sessionId, host }) + }); + const waiters = new Map(); + 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); + } + }); + 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, + 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) + }; +} + +async function writeBridgeRecord(sessionId, { pid = process.pid, host = HOST } = {}) { + await mkdir(hostsDirectory, { recursive: true }); + await writeFile( + path.join(hostsDirectory, `${host}.bridge.json`), + JSON.stringify({ pid, sessionId, updatedAt: new Date().toISOString() }) + ); +} + +describe("a bridge and a slash command that were given different directories", () => { + it("serves the context the slash command just connected", async () => { + // The reported bug, reproduced by the cwd difference alone: before the fix + // this connected one selection file and read another, and reported success. + const window = openWindow({ sessionId: "copilot-session-a" }); + try { + await window.handshake(); + assert.match( + await cli(["use", "payment team"], { sessionId: "copilot-session-a" }), + /Connected the "payment team" context/ + ); + assert.match(await window.grounding(), /connected context: payment team/i); + } finally { + await window.close(); + } + }); + + it("agrees even when the host publishes no session id at all", async () => { + // An older or different Copilot build: the workspace digest is all there is, + // and the bridge's cwd digest is not the workspace's. The pointer the CLI + // writes is what closes the gap. + const window = openWindow({}); + try { + await window.handshake(); + await cli(["use", "Dokploy"]); + assert.match(await window.grounding(), /connected context: Dokploy/i); + } finally { + await window.close(); + } + }); + + it("stops serving a context once the session is disconnected", async () => { + const window = openWindow({ sessionId: "copilot-session-a" }); + try { + await window.handshake(); + await cli(["use", "payment team"], { sessionId: "copilot-session-a" }); + assert.match(await window.grounding(), /connected context: payment team/i); + + await cli(["disconnect"], { sessionId: "copilot-session-a" }); + const answer = await window.grounding(); + assert.doesNotMatch(answer, /connected context: payment team/i); + assert.match(answer, /No NeatContext Context is connected to this session/); + } finally { + await window.close(); + } + }); +}); + +describe("a session that is replaced under a running bridge", () => { + it("follows the session the user is in now", async () => { + const window = openWindow({ sessionId: "copilot-session-a" }); + try { + await window.handshake(); + await cli(["use", "payment team"], { sessionId: "copilot-session-a" }); + assert.match(await window.grounding(), /connected context: payment team/i); + + // A new session in the same window. Copilot does not restart the bridge, + // so its own environment still says session-a. + assert.match( + await cli(["use", "Dokploy"], { sessionId: "copilot-session-b" }), + /Connected the "Dokploy" context/ + ); + const answer = await window.grounding(); + assert.match(answer, /connected context: Dokploy/i); + assert.doesNotMatch(answer, /connected context: payment team/i); + } finally { + await window.close(); + } + }); +}); + +describe("two windows", () => { + it("do not share a selection", async () => { + const first = openWindow({ sessionId: "copilot-session-a", host: HOST }); + const second = openWindow({ sessionId: "copilot-session-b", host: OTHER_HOST }); + try { + await first.handshake(); + await second.handshake(); + await cli(["use", "payment team"], { sessionId: "copilot-session-a", host: HOST }); + + assert.match(await first.grounding(), /connected context: payment team/i); + const other = await second.grounding(); + assert.doesNotMatch(other, /connected context: payment team/i); + assert.match(other, /No NeatContext Context is connected to this session/); + } finally { + await first.close(); + await second.close(); + } + }); + + it("keep separate selections when neither publishes a session id", async () => { + const first = openWindow({ host: HOST }); + const second = openWindow({ host: OTHER_HOST }); + try { + await first.handshake(); + await second.handshake(); + await cli(["use", "payment team"], { host: HOST, cwd: workspace }); + await cli(["use", "Dokploy"], { host: OTHER_HOST, cwd: otherWorkspace }); + + assert.match(await first.grounding(), /connected context: payment team/i); + assert.match(await second.grounding(), /connected context: Dokploy/i); + } finally { + await first.close(); + await second.close(); + } + }); +}); + +describe("an explicit session id", () => { + it("still overrides everything the host publishes", async () => { + const env = { ...childEnv({ sessionId: "copilot-session-a" }), NEATCONTEXT_SESSION_ID: "pinned" }; + const run = (args, cwd) => + new Promise((resolve) => { + const child = spawn( + process.execPath, + [path.join(copilot, "neatcontext-cli.mjs"), ...args], + { cwd, stdio: ["ignore", "pipe", "inherit"], env } + ); + let out = ""; + child.stdout.on("data", (chunk) => (out += chunk)); + child.on("exit", () => resolve(out.trim())); + }); + + await run(["use", "payment team"], workspace); + // A different workspace and a different session id: the pin is what decides, + // so the selection is found again. + assert.match(await run(["status"], otherWorkspace), /Connected context: payment team/i); + const selection = JSON.parse( + await readFile(path.join(home, "plugin-sessions", "pinned.json"), "utf8") + ); + assert.ok(selection); + }); +}); + +describe("a host that publishes no identity at all", () => { + // Neither a session id nor a pid: the bridge and the CLI are back to hashing + // their own working directories with no channel between them. Nothing + // downstream can detect that, so what is pinned here is that the user is told. + function bare(args, cwd = workspace) { + return new Promise((resolve) => { + const env = { ...childEnv({}), NEATCONTEXT_HOST_KEY: "" }; + const child = spawn( + process.execPath, + [path.join(copilot, "neatcontext-cli.mjs"), ...args], + { cwd, stdio: ["ignore", "pipe", "pipe"], env } + ); + let stdout = ""; + let stderr = ""; + child.stdout.on("data", (chunk) => (stdout += chunk)); + child.stderr.on("data", (chunk) => (stderr += chunk)); + child.on("exit", () => resolve({ stdout: stdout.trim(), stderr: stderr.trim() })); + }); + } + + it("says so rather than failing silently", async () => { + const { stderr } = await bare(["status"]); + assert.match(stderr, /publishes no session identity/); + assert.match(stderr, /NEATCONTEXT_SESSION_ID/); + }); + + it("still scopes to the workspace, so one window keeps working", async () => { + await bare(["use", "payment team"]); + assert.match((await bare(["status"])).stdout, /Connected context: payment team/i); + assert.match((await bare(["status"], otherWorkspace)).stdout, /No context is connected/); + }); + + it("stays quiet as soon as the host publishes either one", async () => { + const withSession = await cli(["status"], { sessionId: "copilot-session-a" }); + assert.doesNotMatch(withSession, /publishes no session identity/); + // The host key stands in for COPILOT_LOADER_PID, which is enough on its own: + // it names the pointer file both halves share. + const withKey = await cli(["status"]); + assert.doesNotMatch(withKey, /publishes no session identity/); + }); +}); + +describe("a session id that cannot name a file", () => { + it("is ignored, and says so instead of scoping somewhere else in silence", async () => { + const run = (value) => + new Promise((resolve) => { + const child = spawn( + process.execPath, + [path.join(copilot, "neatcontext-cli.mjs"), "status"], + { + cwd: workspace, + stdio: ["ignore", "pipe", "pipe"], + env: { ...childEnv({}), NEATCONTEXT_SESSION_ID: value } + } + ); + let stderr = ""; + child.stderr.on("data", (chunk) => (stderr += chunk)); + child.on("exit", () => resolve(stderr.trim())); + }); + + assert.match(await run("teams/payments"), /cannot name a session file/); + assert.match(await run("../escape"), /cannot name a session file/); + // The value itself is not echoed back: it is the user's, and the length is + // enough to recognise which one they set. + assert.doesNotMatch(await run("teams/payments"), /teams\/payments/); + assert.equal(await run("payments-2"), ""); + }); +}); + +describe("upgrading from a release that scoped to the workspace", () => { + it("names the connection that workspace used to have", async () => { + // Exactly what an earlier release left behind: a selection filed under the + // workspace digest, with nothing under this session's id. + await cli(["use", "payment team"], { cwd: workspace }); + const status = await cli(["status"], { sessionId: "copilot-session-new" }); + assert.match(status, /No context is connected/); + assert.match(status, /An earlier version of this plugin connected "payment team"/); + assert.match(status, /\/neatcontext:use payment team/); + }); + + it("says nothing once this session has its own connection", async () => { + await cli(["use", "payment team"], { cwd: workspace }); + await cli(["use", "Dokploy"], { sessionId: "copilot-session-new" }); + const status = await cli(["status"], { sessionId: "copilot-session-new" }); + assert.doesNotMatch(status, /An earlier version of this plugin/); + }); + + it("says nothing when the workspace never had one", async () => { + const status = await cli(["status"], { sessionId: "copilot-session-new" }); + assert.match(status, /No context is connected/); + assert.doesNotMatch(status, /An earlier version of this plugin/); + }); +}); + +describe("telling the user when the bridge has not caught up", () => { + it("warns after use when a live bridge is serving another session", async () => { + await writeBridgeRecord("some-other-session"); + const output = await cli(["use", "payment team"], { sessionId: "copilot-session-a" }); + assert.match(output, /Connected the "payment team" context/); + assert.match(output, /still serving an earlier session/); + }); + + it("says nothing when the bridge agrees", async () => { + await writeBridgeRecord("copilot-session-a"); + const output = await cli(["use", "payment team"], { sessionId: "copilot-session-a" }); + assert.match(output, /Connected the "payment team" context/); + assert.doesNotMatch(output, /still serving an earlier session/); + }); + + it("says nothing when no bridge is publishing at all", async () => { + const output = await cli(["use", "payment team"], { sessionId: "copilot-session-a" }); + assert.match(output, /Connected the "payment team" context/); + assert.doesNotMatch(output, /still serving an earlier session/); + }); + + it("says nothing when the bridge that published it has exited", 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-session", { pid: 2147483647 }); + const output = await cli(["use", "payment team"], { sessionId: "copilot-session-a" }); + assert.doesNotMatch(output, /still serving an earlier session/); + }); +}); diff --git a/tests/host-session.test.mjs b/tests/host-session.test.mjs index 908010f..1f9d91e 100644 --- a/tests/host-session.test.mjs +++ b/tests/host-session.test.mjs @@ -9,6 +9,7 @@ import { after, before, beforeEach, describe, it } from "node:test"; import { awaitBridgeSession, bridgePointerPath, + configureHostPid, hostKey, hostPointerPath, hostsDirectory, @@ -51,6 +52,8 @@ after(async () => { beforeEach(async () => { process.env.NEATCONTEXT_HOME = directory; process.env.NEATCONTEXT_HOST_KEY = "host-under-test"; + // Core knows no host's variable by name; the adapter under test registers it. + configureHostPid(() => process.env.CLAUDE_PID); await rm(hostsDirectory(), { recursive: true, force: true }); }); @@ -100,6 +103,29 @@ describe("which host process this is", () => { process.env.CLAUDE_PID = "not-a-pid"; assert.equal(hostKey(), `pid-${process.ppid}`); }); + + it("ignores a host variable no adapter registered, so one host cannot key on another", () => { + delete process.env.NEATCONTEXT_HOST_KEY; + process.env.CLAUDE_PID = "48596"; + configureHostPid(null); + try { + assert.equal(hostKey(), `pid-${process.ppid}`); + } finally { + configureHostPid(() => process.env.CLAUDE_PID); + } + }); + + it("survives an adapter whose pid lookup throws", () => { + delete process.env.NEATCONTEXT_HOST_KEY; + configureHostPid(() => { + throw new Error("host went away"); + }); + try { + assert.equal(hostKey(), `pid-${process.ppid}`); + } finally { + configureHostPid(() => process.env.CLAUDE_PID); + } + }); }); describe("recording the session a host is on", () => { diff --git a/tools/sync-context-core.mjs b/tools/sync-context-core.mjs index 4ac01b7..5b32860 100644 --- a/tools/sync-context-core.mjs +++ b/tools/sync-context-core.mjs @@ -13,6 +13,7 @@ const files = [ "extension-commands.mjs", "extension-runtime.mjs", "extensions.mjs", + "host-session.mjs", "local-state.mjs", "mcp-stdio-client.mjs", "routing.mjs", From 08abf1d4e1c0c755cdd84b6fa54ba9aa1fe1ec33 Mon Sep 17 00:00:00 2001 From: Jingyu Ma Date: Sun, 9 Aug 2026 10:57:19 -0700 Subject: [PATCH 2/6] fix(copilot): close the gap between claiming a shared host key and having one Review follow-ups. `sessionIdentityIsShared()` accepted a NEATCONTEXT_HOST_KEY that `hostKey()` rejects, so `..` or a path-shaped value suppressed the warning while no pointer file was ever written -- claiming a channel that was never opened. The rule now lives in one exported `normalizeHostKey()` and both callers ask it. The upgrade hint is retired once the user acts on it. Left in place, the record behind it survived a deliberate `/neatcontext:disconnect` and went on telling them to reconnect what they had just disconnected. It is removed on the first connection made in a session of its own, and never when this session is the workspace fallback -- that file is the live selection then, not a leftover. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../neatcontext/src/core/host-session.mjs | 25 ++++++++--- .../neatcontext/src/core/host-session.mjs | 25 ++++++++--- plugins/copilot/neatcontext/README.md | 6 +-- .../src/copilot/neatcontext-cli.mjs | 18 ++++++++ .../neatcontext/src/copilot/session.mjs | 7 +++- .../neatcontext/src/core/host-session.mjs | 25 ++++++++--- .../neatcontext/src/core/host-session.mjs | 25 ++++++++--- .../pi/neatcontext/src/core/host-session.mjs | 25 ++++++++--- shared/core/host-session.mjs | 25 ++++++++--- tests/copilot-session-drift.test.mjs | 42 +++++++++++++++++++ 10 files changed, 188 insertions(+), 35 deletions(-) diff --git a/codex-marketplace/plugins/neatcontext/src/core/host-session.mjs b/codex-marketplace/plugins/neatcontext/src/core/host-session.mjs index 5e863d2..e576d78 100644 --- a/codex-marketplace/plugins/neatcontext/src/core/host-session.mjs +++ b/codex-marketplace/plugins/neatcontext/src/core/host-session.mjs @@ -95,6 +95,25 @@ function hostPid() { } } +// A host key names one directory entry, so it is held to the same rule as +// anything else that becomes a path segment. Exported because an adapter that +// wants to know whether a shared key is available has to ask the same question +// this does — a value accepted there and rejected here would claim a channel +// that was never opened. +export function normalizeHostKey(value) { + if (typeof value !== "string") { + return null; + } + const trimmed = value.trim(); + return trimmed.length > 0 && + trimmed !== "." && + trimmed !== ".." && + !trimmed.includes("/") && + !trimmed.includes("\\") + ? trimmed + : null; +} + // The host process this process belongs to. // // NEATCONTEXT_HOST_KEY is the explicit form, for tests and for any host that can @@ -104,11 +123,7 @@ function hostPid() { export function hostKey() { const explicit = process.env.NEATCONTEXT_HOST_KEY; if (typeof explicit === "string") { - const trimmed = explicit.trim(); - return trimmed.length > 0 && trimmed !== "." && trimmed !== ".." && !trimmed.includes("/") && - !trimmed.includes("\\") - ? trimmed - : null; + return normalizeHostKey(explicit); } return pidKey(hostPid()) ?? pidKey(process.ppid); } diff --git a/plugins/claude-code/neatcontext/src/core/host-session.mjs b/plugins/claude-code/neatcontext/src/core/host-session.mjs index 5e863d2..e576d78 100644 --- a/plugins/claude-code/neatcontext/src/core/host-session.mjs +++ b/plugins/claude-code/neatcontext/src/core/host-session.mjs @@ -95,6 +95,25 @@ function hostPid() { } } +// A host key names one directory entry, so it is held to the same rule as +// anything else that becomes a path segment. Exported because an adapter that +// wants to know whether a shared key is available has to ask the same question +// this does — a value accepted there and rejected here would claim a channel +// that was never opened. +export function normalizeHostKey(value) { + if (typeof value !== "string") { + return null; + } + const trimmed = value.trim(); + return trimmed.length > 0 && + trimmed !== "." && + trimmed !== ".." && + !trimmed.includes("/") && + !trimmed.includes("\\") + ? trimmed + : null; +} + // The host process this process belongs to. // // NEATCONTEXT_HOST_KEY is the explicit form, for tests and for any host that can @@ -104,11 +123,7 @@ function hostPid() { export function hostKey() { const explicit = process.env.NEATCONTEXT_HOST_KEY; if (typeof explicit === "string") { - const trimmed = explicit.trim(); - return trimmed.length > 0 && trimmed !== "." && trimmed !== ".." && !trimmed.includes("/") && - !trimmed.includes("\\") - ? trimmed - : null; + return normalizeHostKey(explicit); } return pidKey(hostPid()) ?? pidKey(process.ppid); } diff --git a/plugins/copilot/neatcontext/README.md b/plugins/copilot/neatcontext/README.md index d924926..72cb410 100644 --- a/plugins/copilot/neatcontext/README.md +++ b/plugins/copilot/neatcontext/README.md @@ -49,9 +49,9 @@ copilot plugin install neatcontext@neatcontext NeatContext Desktop connection right now. - **Selections are per session.** A connected context belongs to the Copilot session that connected it, as on every other host. Where a Copilot build - publishes no session identity, the plugin falls back to the workspace folder, - every session opened there shares one selection, and `/neatcontext:status` - says so rather than leaving you to guess. + publishes no session identity, the plugin falls back to the workspace folder + and every session opened there shares one selection. It says so when that + happens, rather than leaving you to guess. - **Upgrading from 0.3.3 or earlier.** Those releases scoped every Copilot selection to the workspace folder. The first time you run a command after upgrading, this session has no connection of its own yet, so diff --git a/plugins/copilot/neatcontext/src/copilot/neatcontext-cli.mjs b/plugins/copilot/neatcontext/src/copilot/neatcontext-cli.mjs index 4141f0e..b3c440b 100644 --- a/plugins/copilot/neatcontext/src/copilot/neatcontext-cli.mjs +++ b/plugins/copilot/neatcontext/src/copilot/neatcontext-cli.mjs @@ -133,6 +133,22 @@ async function workspaceSelectionHint(connected) { } } +// The hint above is a one-time thing, so the record behind it is retired the +// moment it has served its purpose: the first connection made in a session of +// its own is the user acting on it, whether or not they picked the same context. +// Left in place it would come back after every deliberate `/neatcontext:disconnect`, +// telling the user to reconnect the thing they just disconnected, forever. +// +// Never when this session *is* the workspace fallback — that file is the live +// selection then, not a leftover. +async function retireWorkspaceSelection() { + const workspace = workspaceSessionId(); + if (sessionId() === workspace) { + return; + } + await rm(sessionSelectionFilePath(workspace), { force: true }).catch(() => undefined); +} + // Whether the bridge that will serve this session has caught up with it. // // The success message below is written from the record this process just wrote, @@ -501,6 +517,7 @@ async function commandUse(state, query) { const target = resolution.context; const result = await applySelection(target); + await retireWorkspaceSelection(); print( `Connected the "${result.name}" context. Your next messages in this session ` + "will be grounded in its domain profile and knowledge folder." @@ -707,6 +724,7 @@ function printUpdatePreview(preview) { async function printSaveConnection(record) { const outcome = await connectAfterSave(record).catch(() => null); if (outcome?.connected) { + await retireWorkspaceSelection(); print(`Connected context: ${record.name}`); print( "This session had no context connected, so it is now grounded in the one it " + diff --git a/plugins/copilot/neatcontext/src/copilot/session.mjs b/plugins/copilot/neatcontext/src/copilot/session.mjs index d707fb2..7baa30e 100644 --- a/plugins/copilot/neatcontext/src/copilot/session.mjs +++ b/plugins/copilot/neatcontext/src/copilot/session.mjs @@ -40,6 +40,7 @@ import path from "node:path"; import { configureSessionId } from "../core/session.mjs"; import { configureHostPid, + normalizeHostKey, normalizeHostSessionId, publishBridgeSession, resolveHostSessionId @@ -97,11 +98,13 @@ export function unusableSessionOverride() { // between them. It is not a guess: each branch is a thing the host either // publishes to both processes or does not. export function sessionIdentityIsShared() { - const key = process.env.NEATCONTEXT_HOST_KEY; return Boolean( explicitId(process.env.NEATCONTEXT_SESSION_ID) ?? explicitId(process.env.COPILOT_AGENT_SESSION_ID) ?? - (typeof key === "string" && key.trim().length > 0 ? key : null) ?? + // The same rule `hostKey()` applies, asked through the same function: a + // key accepted here and rejected there would claim a channel that was + // never opened, which is the one thing this must not do. + normalizeHostKey(process.env.NEATCONTEXT_HOST_KEY) ?? // A host pid both halves can see is enough on its own: it names the // pointer file they share, which is what carries the session across. (/^[1-9][0-9]{0,9}$/.test(String(process.env.COPILOT_LOADER_PID ?? "").trim()) diff --git a/plugins/copilot/neatcontext/src/core/host-session.mjs b/plugins/copilot/neatcontext/src/core/host-session.mjs index 5e863d2..e576d78 100644 --- a/plugins/copilot/neatcontext/src/core/host-session.mjs +++ b/plugins/copilot/neatcontext/src/core/host-session.mjs @@ -95,6 +95,25 @@ function hostPid() { } } +// A host key names one directory entry, so it is held to the same rule as +// anything else that becomes a path segment. Exported because an adapter that +// wants to know whether a shared key is available has to ask the same question +// this does — a value accepted there and rejected here would claim a channel +// that was never opened. +export function normalizeHostKey(value) { + if (typeof value !== "string") { + return null; + } + const trimmed = value.trim(); + return trimmed.length > 0 && + trimmed !== "." && + trimmed !== ".." && + !trimmed.includes("/") && + !trimmed.includes("\\") + ? trimmed + : null; +} + // The host process this process belongs to. // // NEATCONTEXT_HOST_KEY is the explicit form, for tests and for any host that can @@ -104,11 +123,7 @@ function hostPid() { export function hostKey() { const explicit = process.env.NEATCONTEXT_HOST_KEY; if (typeof explicit === "string") { - const trimmed = explicit.trim(); - return trimmed.length > 0 && trimmed !== "." && trimmed !== ".." && !trimmed.includes("/") && - !trimmed.includes("\\") - ? trimmed - : null; + return normalizeHostKey(explicit); } return pidKey(hostPid()) ?? pidKey(process.ppid); } diff --git a/plugins/kimi-code/neatcontext/src/core/host-session.mjs b/plugins/kimi-code/neatcontext/src/core/host-session.mjs index 5e863d2..e576d78 100644 --- a/plugins/kimi-code/neatcontext/src/core/host-session.mjs +++ b/plugins/kimi-code/neatcontext/src/core/host-session.mjs @@ -95,6 +95,25 @@ function hostPid() { } } +// A host key names one directory entry, so it is held to the same rule as +// anything else that becomes a path segment. Exported because an adapter that +// wants to know whether a shared key is available has to ask the same question +// this does — a value accepted there and rejected here would claim a channel +// that was never opened. +export function normalizeHostKey(value) { + if (typeof value !== "string") { + return null; + } + const trimmed = value.trim(); + return trimmed.length > 0 && + trimmed !== "." && + trimmed !== ".." && + !trimmed.includes("/") && + !trimmed.includes("\\") + ? trimmed + : null; +} + // The host process this process belongs to. // // NEATCONTEXT_HOST_KEY is the explicit form, for tests and for any host that can @@ -104,11 +123,7 @@ function hostPid() { export function hostKey() { const explicit = process.env.NEATCONTEXT_HOST_KEY; if (typeof explicit === "string") { - const trimmed = explicit.trim(); - return trimmed.length > 0 && trimmed !== "." && trimmed !== ".." && !trimmed.includes("/") && - !trimmed.includes("\\") - ? trimmed - : null; + return normalizeHostKey(explicit); } return pidKey(hostPid()) ?? pidKey(process.ppid); } diff --git a/plugins/pi/neatcontext/src/core/host-session.mjs b/plugins/pi/neatcontext/src/core/host-session.mjs index 5e863d2..e576d78 100644 --- a/plugins/pi/neatcontext/src/core/host-session.mjs +++ b/plugins/pi/neatcontext/src/core/host-session.mjs @@ -95,6 +95,25 @@ function hostPid() { } } +// A host key names one directory entry, so it is held to the same rule as +// anything else that becomes a path segment. Exported because an adapter that +// wants to know whether a shared key is available has to ask the same question +// this does — a value accepted there and rejected here would claim a channel +// that was never opened. +export function normalizeHostKey(value) { + if (typeof value !== "string") { + return null; + } + const trimmed = value.trim(); + return trimmed.length > 0 && + trimmed !== "." && + trimmed !== ".." && + !trimmed.includes("/") && + !trimmed.includes("\\") + ? trimmed + : null; +} + // The host process this process belongs to. // // NEATCONTEXT_HOST_KEY is the explicit form, for tests and for any host that can @@ -104,11 +123,7 @@ function hostPid() { export function hostKey() { const explicit = process.env.NEATCONTEXT_HOST_KEY; if (typeof explicit === "string") { - const trimmed = explicit.trim(); - return trimmed.length > 0 && trimmed !== "." && trimmed !== ".." && !trimmed.includes("/") && - !trimmed.includes("\\") - ? trimmed - : null; + return normalizeHostKey(explicit); } return pidKey(hostPid()) ?? pidKey(process.ppid); } diff --git a/shared/core/host-session.mjs b/shared/core/host-session.mjs index 5e863d2..e576d78 100644 --- a/shared/core/host-session.mjs +++ b/shared/core/host-session.mjs @@ -95,6 +95,25 @@ function hostPid() { } } +// A host key names one directory entry, so it is held to the same rule as +// anything else that becomes a path segment. Exported because an adapter that +// wants to know whether a shared key is available has to ask the same question +// this does — a value accepted there and rejected here would claim a channel +// that was never opened. +export function normalizeHostKey(value) { + if (typeof value !== "string") { + return null; + } + const trimmed = value.trim(); + return trimmed.length > 0 && + trimmed !== "." && + trimmed !== ".." && + !trimmed.includes("/") && + !trimmed.includes("\\") + ? trimmed + : null; +} + // The host process this process belongs to. // // NEATCONTEXT_HOST_KEY is the explicit form, for tests and for any host that can @@ -104,11 +123,7 @@ function hostPid() { export function hostKey() { const explicit = process.env.NEATCONTEXT_HOST_KEY; if (typeof explicit === "string") { - const trimmed = explicit.trim(); - return trimmed.length > 0 && trimmed !== "." && trimmed !== ".." && !trimmed.includes("/") && - !trimmed.includes("\\") - ? trimmed - : null; + return normalizeHostKey(explicit); } return pidKey(hostPid()) ?? pidKey(process.ppid); } diff --git a/tests/copilot-session-drift.test.mjs b/tests/copilot-session-drift.test.mjs index 5ebebb5..74e6c6a 100644 --- a/tests/copilot-session-drift.test.mjs +++ b/tests/copilot-session-drift.test.mjs @@ -319,6 +319,29 @@ describe("a host that publishes no identity at all", () => { assert.match(stderr, /NEATCONTEXT_SESSION_ID/); }); + it("is not silenced by a host key that cannot name a file", async () => { + // `hostKey()` rejects these, so no pointer is written and the two halves are + // back to their own working directories. Accepting them here would claim a + // channel that was never opened. + for (const key of ["..", ".", "a/b", "a\\b", " "]) { + const { stderr } = await new Promise((resolve) => { + const child = spawn( + process.execPath, + [path.join(copilot, "neatcontext-cli.mjs"), "status"], + { + cwd: workspace, + stdio: ["ignore", "pipe", "pipe"], + env: { ...childEnv({}), NEATCONTEXT_HOST_KEY: key } + } + ); + let out = ""; + child.stderr.on("data", (chunk) => (out += chunk)); + child.on("exit", () => resolve({ stderr: out })); + }); + assert.match(stderr, /publishes no session identity/, `accepted ${JSON.stringify(key)}`); + } + }); + it("still scopes to the workspace, so one window keeps working", async () => { await bare(["use", "payment team"]); assert.match((await bare(["status"])).stdout, /Connected context: payment team/i); @@ -380,6 +403,25 @@ describe("upgrading from a release that scoped to the workspace", () => { assert.doesNotMatch(status, /An earlier version of this plugin/); }); + it("stops offering it once the user has acted on it, even after disconnecting", async () => { + await cli(["use", "payment team"], { cwd: workspace }); + // Acting on the hint is what retires it — this session has one of its own now. + await cli(["use", "Dokploy"], { sessionId: "copilot-session-new" }); + await cli(["disconnect"], { sessionId: "copilot-session-new" }); + const status = await cli(["status"], { sessionId: "copilot-session-new" }); + assert.match(status, /No context is connected/); + assert.doesNotMatch(status, /An earlier version of this plugin/); + }); + + it("says nothing when the workspace selection is the live one", async () => { + // No session id published, so this session *is* the workspace: the file is + // its own selection, not a leftover, and must be neither offered nor removed. + await cli(["use", "payment team"], { cwd: workspace }); + const status = await cli(["status"], { cwd: workspace }); + assert.match(status, /Connected context: payment team/i); + assert.doesNotMatch(status, /An earlier version of this plugin/); + }); + it("says nothing when the workspace never had one", async () => { const status = await cli(["status"], { sessionId: "copilot-session-new" }); assert.match(status, /No context is connected/); From 3e0f77ef34c66dc226d592314518b562f04d7882 Mon Sep 17 00:00:00 2001 From: Jingyu Ma Date: Sun, 9 Aug 2026 15:14:03 -0700 Subject: [PATCH 3/6] test(copilot): cover session identity failure paths Remove unreachable guards and exercise the remaining host identity, migration, drift, and atomic-write branches required by diff coverage. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../src/copilot/neatcontext-cli.mjs | 10 +---- tests/copilot-session-drift.test.mjs | 41 +++++++++++++++++++ tests/host-session.test.mjs | 21 ++++++++++ 3 files changed, 64 insertions(+), 8 deletions(-) diff --git a/plugins/copilot/neatcontext/src/copilot/neatcontext-cli.mjs b/plugins/copilot/neatcontext/src/copilot/neatcontext-cli.mjs index b3c440b..b78e8d9 100644 --- a/plugins/copilot/neatcontext/src/copilot/neatcontext-cli.mjs +++ b/plugins/copilot/neatcontext/src/copilot/neatcontext-cli.mjs @@ -109,10 +109,7 @@ function hostIdentityWarning() { // reads once the host publishes a real session id. Saying so beats letting a // working connection look like it disappeared — and it is only said when this // workspace actually has one and nothing is connected now. -async function workspaceSelectionHint(connected) { - if (connected) { - return null; - } +async function workspaceSelectionHint() { const workspace = workspaceSessionId(); if (sessionId() === workspace) { return null; @@ -164,9 +161,6 @@ async function retireWorkspaceSelection() { // itself would be worse than the wait. async function bridgeDriftWarning() { const id = sessionId(); - if (!id) { - return null; - } const { state } = await awaitBridgeSession(id); if (state !== "drifted") { return null; @@ -347,7 +341,7 @@ async function commandStatus(state) { "`/neatcontext:create`." : "No context is connected yet. Use `/neatcontext:use` to pick one." ); - const upgrade = await workspaceSelectionHint(connected); + const upgrade = await workspaceSelectionHint(); if (upgrade) { print(""); print(upgrade); diff --git a/tests/copilot-session-drift.test.mjs b/tests/copilot-session-drift.test.mjs index 74e6c6a..edeafff 100644 --- a/tests/copilot-session-drift.test.mjs +++ b/tests/copilot-session-drift.test.mjs @@ -23,6 +23,7 @@ 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"; +import { workspaceSessionId } from "../plugins/copilot/neatcontext/src/copilot/session.mjs"; const plugin = path.join( path.dirname(fileURLToPath(import.meta.url)), @@ -355,6 +356,26 @@ describe("a host that publishes no identity at all", () => { // it names the pointer file both halves share. const withKey = await cli(["status"]); assert.doesNotMatch(withKey, /publishes no session identity/); + + const withPid = await new Promise((resolve) => { + const child = spawn( + process.execPath, + [path.join(copilot, "neatcontext-cli.mjs"), "status"], + { + cwd: workspace, + stdio: ["ignore", "pipe", "pipe"], + env: { + ...childEnv({}), + NEATCONTEXT_HOST_KEY: "", + COPILOT_LOADER_PID: "48596" + } + } + ); + let stderr = ""; + child.stderr.on("data", (chunk) => (stderr += chunk)); + child.on("exit", () => resolve(stderr.trim())); + }); + assert.doesNotMatch(withPid, /publishes no session identity/); }); }); @@ -427,9 +448,29 @@ describe("upgrading from a release that scoped to the workspace", () => { assert.match(status, /No context is connected/); assert.doesNotMatch(status, /An earlier version of this plugin/); }); + + it("ignores a workspace selection without a context name", async () => { + const sessions = path.join(home, "plugin-sessions"); + await mkdir(sessions, { recursive: true }); + await writeFile( + path.join(sessions, `${workspaceSessionId(workspace)}.json`), + JSON.stringify({ contextName: " " }) + ); + + const status = await cli(["status"], { sessionId: "copilot-session-new" }); + assert.match(status, /No context is connected/); + assert.doesNotMatch(status, /An earlier version of this plugin/); + }); }); describe("telling the user when the bridge has not caught up", () => { + it("warns from status when a live bridge is serving another session", async () => { + await writeBridgeRecord("some-other-session"); + const output = await cli(["status"], { sessionId: "copilot-session-a" }); + assert.match(output, /serving an earlier session, not this one/); + assert.match(output, /get_context.*other one/); + }); + it("warns after use when a live bridge is serving another session", async () => { await writeBridgeRecord("some-other-session"); const output = await cli(["use", "payment team"], { sessionId: "copilot-session-a" }); diff --git a/tests/host-session.test.mjs b/tests/host-session.test.mjs index 1f9d91e..cf3790d 100644 --- a/tests/host-session.test.mjs +++ b/tests/host-session.test.mjs @@ -14,6 +14,7 @@ import { hostPointerPath, hostsDirectory, isProcessAlive, + normalizeHostKey, normalizeHostSessionId, publishBridgeSession, pruneHostPointers, @@ -80,6 +81,13 @@ describe("what counts as a session id", () => { }); describe("which host process this is", () => { + it("rejects an invalid pid provider", () => { + assert.throws( + () => configureHostPid("CLAUDE_PID"), + /host pid provider must be a function or null/ + ); + }); + it("takes the explicit key when the host or a test supplies one", () => { process.env.NEATCONTEXT_HOST_KEY = "window-7"; assert.equal(hostKey(), "window-7"); @@ -92,6 +100,12 @@ describe("which host process this is", () => { } }); + it("refuses a non-string explicit key", () => { + for (const value of [7, null, undefined, {}]) { + assert.equal(normalizeHostKey(value), null); + } + }); + it("falls back to the host pid, which a slash command reads from its environment", () => { delete process.env.NEATCONTEXT_HOST_KEY; process.env.CLAUDE_PID = "48596"; @@ -162,6 +176,13 @@ describe("recording the session a host is on", () => { assert.equal(await writeHostPointer("session-a"), null); assert.equal(await publishBridgeSession("session-a"), false); }); + + it("removes its temporary file when the atomic rename fails", async () => { + await mkdir(hostPointerPath(hostKey()), { recursive: true }); + + assert.equal(await writeHostPointer("session-a"), null); + assert.deepEqual(await readdir(hostsDirectory()), [`${hostKey()}.json`]); + }); }); describe("the session a long-lived process should serve", () => { From 49151be3708c224e9f271544336c736c5543647a Mon Sep 17 00:00:00 2001 From: Jingyu Ma Date: Sun, 9 Aug 2026 16:48:44 -0700 Subject: [PATCH 4/6] fix(copilot): use the shared agent session id Narrow the fix to the session identity verified on Copilot CLI. Keep workspace scoping as the fallback and cover command-to-MCP agreement across different working directories. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../plugins/neatcontext/src/codex/session.mjs | 12 +- .../neatcontext/src/core/host-session.mjs | 142 ++--- package.json | 6 +- .../neatcontext/hooks/session-start.mjs | 4 - .../claude-code/neatcontext/hooks/stop.mjs | 4 - .../neatcontext/src/claude/session.mjs | 12 +- .../neatcontext/src/core/host-session.mjs | 135 ++--- plugins/copilot/neatcontext/README.md | 16 +- .../neatcontext/commands/disconnect.md | 3 +- .../neatcontext/src/copilot/mcp-bridge.mjs | 40 +- .../src/copilot/neatcontext-cli.mjs | 168 +----- .../neatcontext/src/copilot/session.mjs | 132 +---- .../neatcontext/src/core/host-session.mjs | 360 ------------- .../neatcontext/src/core/host-session.mjs | 360 ------------- plugins/pi/neatcontext/package.json | 2 +- .../pi/neatcontext/src/core/host-session.mjs | 360 ------------- shared/core/host-session.mjs | 360 ------------- tests/copilot-plugin.test.mjs | 60 ++- tests/copilot-session-drift.test.mjs | 501 ------------------ tests/host-session.test.mjs | 47 -- tools/sync-context-core.mjs | 1 - 21 files changed, 151 insertions(+), 2574 deletions(-) delete mode 100644 plugins/copilot/neatcontext/src/core/host-session.mjs delete mode 100644 plugins/kimi-code/neatcontext/src/core/host-session.mjs delete mode 100644 plugins/pi/neatcontext/src/core/host-session.mjs delete mode 100644 shared/core/host-session.mjs delete mode 100644 tests/copilot-session-drift.test.mjs diff --git a/codex-marketplace/plugins/neatcontext/src/codex/session.mjs b/codex-marketplace/plugins/neatcontext/src/codex/session.mjs index 1da430c..bfed07f 100644 --- a/codex-marketplace/plugins/neatcontext/src/codex/session.mjs +++ b/codex-marketplace/plugins/neatcontext/src/codex/session.mjs @@ -13,17 +13,7 @@ // stale copy. See core/host-session.mjs. import { configureSessionId } from "../core/session.mjs"; -import { - configureHostPid, - publishBridgeSession, - resolveHostSessionId -} from "../core/host-session.mjs"; - -// Codex publishes no pid of its own, so the pointer file is keyed on -// `process.ppid` — which is Codex itself for the bridge and for the hooks and -// CLI it spawns. Said explicitly rather than left to the default so that a -// future reader knows it was considered. -configureHostPid(null); +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. diff --git a/codex-marketplace/plugins/neatcontext/src/core/host-session.mjs b/codex-marketplace/plugins/neatcontext/src/core/host-session.mjs index e576d78..63bbe85 100644 --- a/codex-marketplace/plugins/neatcontext/src/core/host-session.mjs +++ b/codex-marketplace/plugins/neatcontext/src/core/host-session.mjs @@ -1,42 +1,40 @@ // Which session the host process is on *right now*. // // A host that identifies its sessions through the environment has a problem the -// rest of this plugin cannot see: the environment is a snapshot. Hosts spawn the -// MCP bridge once and keep it for the life of the window, but the session -// changes underneath it — starting a fresh conversation does not restart the -// server — and the bridge's copy of the id is whatever it was at spawn. The -// hooks and the slash-command CLI are spawned fresh and see the new one. +// 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 +// changes underneath it — `/new` starts a new thread in the same process — and +// the bridge's copy of `CODEX_THREAD_ID` is whatever it was at spawn. The +// SessionStart hook and the skill-run CLI are spawned fresh and see the new one. // // So the two halves of the plugin end up reading and writing different files: -// `use` writes the selection for the session the user is in, the bridge keeps -// serving the session the window started in, and nothing in either path can -// observe the disagreement. The user is told a context is connected and every -// later answer is grounded in a different one. +// `$neatcontext:use` writes the selection for the thread the user is in, the +// bridge keeps serving the thread the window started in, and nothing in either +// path can observe the disagreement. The user is told a context is connected and +// every later answer is grounded in a different one. // // This module is the missing channel. One pointer file per host *process*, -// written by whichever short-lived process was just handed the current session +// written by whichever short-lived process was just handed the current thread // id, and re-read by the long-lived one on every message: // // ~/.neatcontext/plugin-hosts/.json { sessionId, source, updatedAt } // -// The host key has to be stable across a session change and distinct per window, -// which rules out the session id and the working directory both. What is left is -// the host process itself. How to name it is the one thing that differs per -// host, so each adapter registers its own answer with `configureHostPid()`: some -// hosts publish their own pid to every process they spawn, and a host that -// publishes nothing falls back to `process.ppid`, which is the host for any -// process it spawned directly. A process whose parent is not the host and whose -// host publishes no pid — a CLI running inside a shell tool — computes a key -// nothing else reads, and its writes are harmless and swept later. +// The host key has to be stable across a thread change and distinct per window, +// which rules out the thread id and the working directory both. What is left is +// the host process itself: the bridge and the SessionStart hook are spawned by +// Codex directly, so `process.ppid` is the same number in both. Two windows are +// two pids and never share a pointer. A process whose parent is not the host — +// the CLI runs inside Codex's shell tool — computes a key nothing else reads, +// and its writes are harmless and swept later. // // The bridge also publishes what it actually resolved: // // ~/.neatcontext/plugin-hosts/.bridge.json { pid, sessionId, updatedAt } // -// That is what lets `use` check its own success against the process that will -// serve it, instead of against the file it just wrote itself. +// That is what lets `$neatcontext:use` check its own success against the process +// that will serve it, instead of against the file it just wrote itself. -import { mkdir, readFile, readdir, rename, rm, writeFile } from "node:fs/promises"; +import { mkdir, readFile, readdir, rm, writeFile } from "node:fs/promises"; import path from "node:path"; import { neatContextHome } from "./storage-home.mjs"; @@ -62,70 +60,24 @@ function pidKey(value) { return /^[1-9][0-9]{0,9}$/.test(pid) ? `pid-${pid}` : null; } -// Where the host publishes its own process id, when it publishes one at all. -// -// Deliberately per adapter rather than a list of every host's variable checked -// in turn: those variables are inherited by child processes, so a host launched -// from another host's shell would key on the outer host and two different hosts -// could end up sharing one pointer file. Each adapter consults only the variable -// its own host sets. -// -// Process-global, because one process runs one host's adapter. A test that -// imported two adapters into the same process would have the second silently -// win; there is no such test, and there should not be one. -let hostPidProvider = null; - -export function configureHostPid(provider) { - if (provider !== null && typeof provider !== "function") { - throw new TypeError("The host pid provider must be a function or null."); - } - hostPidProvider = provider; -} - -function hostPid() { - if (!hostPidProvider) { - return null; - } - // An adapter reading a value the host never set must not take down the only - // path that can name this process's host. - try { - return hostPidProvider(); - } catch { - return null; - } -} - -// A host key names one directory entry, so it is held to the same rule as -// anything else that becomes a path segment. Exported because an adapter that -// wants to know whether a shared key is available has to ask the same question -// this does — a value accepted there and rejected here would claim a channel -// that was never opened. -export function normalizeHostKey(value) { - if (typeof value !== "string") { - return null; - } - const trimmed = value.trim(); - return trimmed.length > 0 && - trimmed !== "." && - trimmed !== ".." && - !trimmed.includes("/") && - !trimmed.includes("\\") - ? trimmed - : null; -} - // The host process this process belongs to. // // NEATCONTEXT_HOST_KEY is the explicit form, for tests and for any host that can -// hand every one of its plugin processes the same identifier. Otherwise the pid -// the adapter knows how to find, and failing that `process.ppid`, which is the -// host for a server or hook it spawned directly. +// hand every one of its plugin processes the same identifier. Otherwise +// `process.ppid`: the bridge and the hooks are spawned by the Codex process +// directly, so both see the host's pid there. Codex publishes no pid of its own +// the way Claude Code publishes CLAUDE_PID, so a process spawned through a shell +// keys on the shell — a key nothing long-lived reads, which fails safe. export function hostKey() { const explicit = process.env.NEATCONTEXT_HOST_KEY; if (typeof explicit === "string") { - return normalizeHostKey(explicit); + const trimmed = explicit.trim(); + return trimmed.length > 0 && trimmed !== "." && trimmed !== ".." && !trimmed.includes("/") && + !trimmed.includes("\\") + ? trimmed + : null; } - return pidKey(hostPid()) ?? pidKey(process.ppid); + return pidKey(process.ppid); } export function hostsDirectory() { @@ -161,31 +113,12 @@ async function readPointer(file) { } } -let writeCounter = 0; - -// Written to a temporary name and renamed into place, because these files are -// read by other processes while they are being written: the bridge re-reads the -// pointer on every message, and a reader that catches a half-written file falls -// back to the environment, which is the stale answer this file exists to -// correct. A rename is atomic, so a reader sees the old record or the new one. -// -// The temporary name carries this process's pid so that two processes writing -// the same pointer at once cannot collide on it, and stays in the same directory -// so the rename never crosses a volume. async function writePointer(file, value) { await mkdir(path.dirname(file), { recursive: true }); - writeCounter += 1; - const temporary = `${file}.${process.pid}.${writeCounter}.tmp`; - try { - await writeFile(temporary, `${JSON.stringify(value, null, 2)}\n`, { - encoding: "utf8", - mode: 0o600 - }); - await rename(temporary, file); - } catch (error) { - await rm(temporary, { force: true }).catch(() => undefined); - throw error; - } + await writeFile(file, `${JSON.stringify(value, null, 2)}\n`, { + encoding: "utf8", + mode: 0o600 + }); } export async function readHostPointer(key = hostKey()) { @@ -250,11 +183,6 @@ export async function resolveHostSessionId(envSessionId, { since = 0, key = host // rewriting. Age is not staleness here — a record from an hour ago is still what // this bridge is serving. Whether it is *running* is a question about its pid, // which the record carries. -// -// The cache is what this process last wrote, not what is on disk. Something that -// deleted the file from underneath it would not get it back until the session -// changes; nothing here does that, and defending against an outside `rm` would -// cost a stat on every message. let lastPublished = { id: undefined }; export async function publishBridgeSession(id, { key = hostKey(), now = Date.now() } = {}) { diff --git a/package.json b/package.json index 04ff8a8..7994288 100644 --- a/package.json +++ b/package.json @@ -12,11 +12,11 @@ "sync:evidence": "node tools/sync-conversation-evidence.mjs", "sync:context": "node tools/sync-context-core.mjs", "check:claude": "node --check plugins/claude-code/neatcontext/src/core/context-store.mjs && node --check plugins/claude-code/neatcontext/src/core/extension-bindings.mjs && node --check plugins/claude-code/neatcontext/src/core/extension-commands.mjs && node --check plugins/claude-code/neatcontext/src/core/extension-runtime.mjs && node --check plugins/claude-code/neatcontext/src/core/extensions.mjs && node --check plugins/claude-code/neatcontext/src/core/mcp-stdio-client.mjs && node --check plugins/claude-code/neatcontext/src/core/local-state.mjs && node --check plugins/claude-code/neatcontext/src/core/host-session.mjs && node --check plugins/claude-code/neatcontext/src/core/conversation-evidence.mjs && node --check plugins/claude-code/neatcontext/src/core/routing.mjs && node --check plugins/claude-code/neatcontext/src/core/routing-search.mjs && node --check plugins/claude-code/neatcontext/src/core/routing-candidates.mjs && node --check plugins/claude-code/neatcontext/src/core/session-state.mjs && node --check plugins/claude-code/neatcontext/src/core/selection.mjs && node --check plugins/claude-code/neatcontext/src/core/session.mjs && node --check plugins/claude-code/neatcontext/src/core/storage-home.mjs && node --check plugins/claude-code/neatcontext/src/claude/conversation-evidence.mjs && node --check plugins/claude-code/neatcontext/src/claude/mcp-bridge.mjs && node --check plugins/claude-code/neatcontext/src/claude/neatcontext-cli.mjs && node --check plugins/claude-code/neatcontext/src/claude/session.mjs && node --check plugins/claude-code/neatcontext/hooks/session-start.mjs && node --check plugins/claude-code/neatcontext/hooks/stop.mjs && node --check plugins/claude-code/neatcontext/hooks/pre-compact.mjs", - "check:kimi": "node --check plugins/kimi-code/neatcontext/src/core/context-store.mjs && node --check plugins/kimi-code/neatcontext/src/core/extension-bindings.mjs && node --check plugins/kimi-code/neatcontext/src/core/extension-commands.mjs && node --check plugins/kimi-code/neatcontext/src/core/extension-runtime.mjs && node --check plugins/kimi-code/neatcontext/src/core/extensions.mjs && node --check plugins/kimi-code/neatcontext/src/core/mcp-stdio-client.mjs && node --check plugins/kimi-code/neatcontext/src/core/local-state.mjs && node --check plugins/kimi-code/neatcontext/src/core/host-session.mjs && node --check plugins/kimi-code/neatcontext/src/core/conversation-evidence.mjs && node --check plugins/kimi-code/neatcontext/src/core/routing.mjs && node --check plugins/kimi-code/neatcontext/src/core/routing-search.mjs && node --check plugins/kimi-code/neatcontext/src/core/routing-candidates.mjs && node --check plugins/kimi-code/neatcontext/src/core/selection.mjs && node --check plugins/kimi-code/neatcontext/src/core/session.mjs && node --check plugins/kimi-code/neatcontext/src/core/storage-home.mjs && node --check plugins/kimi-code/neatcontext/src/kimi/mcp-bridge.mjs && node --check plugins/kimi-code/neatcontext/src/kimi/neatcontext-cli.mjs && node --check plugins/kimi-code/neatcontext/src/kimi/session.mjs", - "check:copilot": "node --check plugins/copilot/neatcontext/src/core/context-store.mjs && node --check plugins/copilot/neatcontext/src/core/extension-bindings.mjs && node --check plugins/copilot/neatcontext/src/core/extension-commands.mjs && node --check plugins/copilot/neatcontext/src/core/extension-runtime.mjs && node --check plugins/copilot/neatcontext/src/core/extensions.mjs && node --check plugins/copilot/neatcontext/src/core/mcp-stdio-client.mjs && node --check plugins/copilot/neatcontext/src/core/local-state.mjs && node --check plugins/copilot/neatcontext/src/core/host-session.mjs && node --check plugins/copilot/neatcontext/src/core/conversation-evidence.mjs && node --check plugins/copilot/neatcontext/src/core/routing.mjs && node --check plugins/copilot/neatcontext/src/core/routing-search.mjs && node --check plugins/copilot/neatcontext/src/core/routing-candidates.mjs && node --check plugins/copilot/neatcontext/src/core/selection.mjs && node --check plugins/copilot/neatcontext/src/core/session.mjs && node --check plugins/copilot/neatcontext/src/core/storage-home.mjs && node --check plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs && node --check plugins/copilot/neatcontext/src/copilot/neatcontext-cli.mjs && node --check plugins/copilot/neatcontext/src/copilot/session.mjs", + "check:kimi": "node --check plugins/kimi-code/neatcontext/src/core/context-store.mjs && node --check plugins/kimi-code/neatcontext/src/core/extension-bindings.mjs && node --check plugins/kimi-code/neatcontext/src/core/extension-commands.mjs && node --check plugins/kimi-code/neatcontext/src/core/extension-runtime.mjs && node --check plugins/kimi-code/neatcontext/src/core/extensions.mjs && node --check plugins/kimi-code/neatcontext/src/core/mcp-stdio-client.mjs && node --check plugins/kimi-code/neatcontext/src/core/local-state.mjs && node --check plugins/kimi-code/neatcontext/src/core/conversation-evidence.mjs && node --check plugins/kimi-code/neatcontext/src/core/routing.mjs && node --check plugins/kimi-code/neatcontext/src/core/routing-search.mjs && node --check plugins/kimi-code/neatcontext/src/core/routing-candidates.mjs && node --check plugins/kimi-code/neatcontext/src/core/selection.mjs && node --check plugins/kimi-code/neatcontext/src/core/session.mjs && node --check plugins/kimi-code/neatcontext/src/core/storage-home.mjs && node --check plugins/kimi-code/neatcontext/src/kimi/mcp-bridge.mjs && node --check plugins/kimi-code/neatcontext/src/kimi/neatcontext-cli.mjs && node --check plugins/kimi-code/neatcontext/src/kimi/session.mjs", + "check:copilot": "node --check plugins/copilot/neatcontext/src/core/context-store.mjs && node --check plugins/copilot/neatcontext/src/core/extension-bindings.mjs && node --check plugins/copilot/neatcontext/src/core/extension-commands.mjs && node --check plugins/copilot/neatcontext/src/core/extension-runtime.mjs && node --check plugins/copilot/neatcontext/src/core/extensions.mjs && node --check plugins/copilot/neatcontext/src/core/mcp-stdio-client.mjs && node --check plugins/copilot/neatcontext/src/core/local-state.mjs && node --check plugins/copilot/neatcontext/src/core/conversation-evidence.mjs && node --check plugins/copilot/neatcontext/src/core/routing.mjs && node --check plugins/copilot/neatcontext/src/core/routing-search.mjs && node --check plugins/copilot/neatcontext/src/core/routing-candidates.mjs && node --check plugins/copilot/neatcontext/src/core/selection.mjs && node --check plugins/copilot/neatcontext/src/core/session.mjs && node --check plugins/copilot/neatcontext/src/core/storage-home.mjs && node --check plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs && node --check plugins/copilot/neatcontext/src/copilot/neatcontext-cli.mjs && node --check plugins/copilot/neatcontext/src/copilot/session.mjs", "check:codex": "node --check codex-marketplace/plugins/neatcontext/src/core/context-store.mjs && node --check codex-marketplace/plugins/neatcontext/src/core/extension-bindings.mjs && node --check codex-marketplace/plugins/neatcontext/src/core/extension-commands.mjs && node --check codex-marketplace/plugins/neatcontext/src/core/extension-runtime.mjs && node --check codex-marketplace/plugins/neatcontext/src/core/extensions.mjs && node --check codex-marketplace/plugins/neatcontext/src/core/mcp-stdio-client.mjs && node --check codex-marketplace/plugins/neatcontext/src/core/local-state.mjs && node --check codex-marketplace/plugins/neatcontext/src/core/host-session.mjs && node --check codex-marketplace/plugins/neatcontext/src/core/conversation-evidence.mjs && node --check codex-marketplace/plugins/neatcontext/src/core/routing.mjs && node --check codex-marketplace/plugins/neatcontext/src/core/routing-search.mjs && node --check codex-marketplace/plugins/neatcontext/src/core/routing-candidates.mjs && node --check codex-marketplace/plugins/neatcontext/src/core/selection.mjs && node --check codex-marketplace/plugins/neatcontext/src/core/session.mjs && node --check codex-marketplace/plugins/neatcontext/src/core/storage-home.mjs && node --check codex-marketplace/plugins/neatcontext/src/codex/mcp-bridge.mjs && node --check codex-marketplace/plugins/neatcontext/src/codex/neatcontext-cli.mjs && node --check codex-marketplace/plugins/neatcontext/src/codex/session.mjs", "check:pi": "npm --prefix plugins/pi/neatcontext run check", - "check": "node tools/sync-conversation-evidence.mjs --check && node tools/sync-context-core.mjs --check && node --check shared/core/conversation-evidence.mjs && node --check shared/core/context-store.mjs && node --check shared/core/extension-bindings.mjs && node --check shared/core/extension-commands.mjs && node --check shared/core/extension-runtime.mjs && node --check shared/core/extensions.mjs && node --check shared/core/mcp-stdio-client.mjs && node --check shared/core/local-state.mjs && node --check shared/core/host-session.mjs && node --check shared/core/routing.mjs && node --check shared/core/routing-search.mjs && node --check shared/core/routing-candidates.mjs && node --check shared/core/selection.mjs && node --check shared/core/storage-home.mjs && npm run check:claude && npm run check:kimi && npm run check:copilot && npm run check:codex && npm run check:pi && node --check tools/sync-conversation-evidence.mjs && node --check tools/sync-context-core.mjs && node --check tools/e2e-no-nudge.mjs && node --check tools/e2e-extensions.mjs && node --check tools/e2e-commands.mjs && node --check tools/diff-coverage.mjs", + "check": "node tools/sync-conversation-evidence.mjs --check && node tools/sync-context-core.mjs --check && node --check shared/core/conversation-evidence.mjs && node --check shared/core/context-store.mjs && node --check shared/core/extension-bindings.mjs && node --check shared/core/extension-commands.mjs && node --check shared/core/extension-runtime.mjs && node --check shared/core/extensions.mjs && node --check shared/core/mcp-stdio-client.mjs && node --check shared/core/local-state.mjs && node --check shared/core/routing.mjs && node --check shared/core/routing-search.mjs && node --check shared/core/routing-candidates.mjs && node --check shared/core/selection.mjs && node --check shared/core/storage-home.mjs && npm run check:claude && npm run check:kimi && npm run check:copilot && npm run check:codex && npm run check:pi && node --check tools/sync-conversation-evidence.mjs && node --check tools/sync-context-core.mjs && node --check tools/e2e-no-nudge.mjs && node --check tools/e2e-extensions.mjs && node --check tools/e2e-commands.mjs && node --check tools/diff-coverage.mjs", "validate:plugin": "claude plugin validate . --strict && claude plugin validate plugins/claude-code/neatcontext --strict", "test": "node --test", "coverage": "node tools/diff-coverage.mjs", diff --git a/plugins/claude-code/neatcontext/hooks/session-start.mjs b/plugins/claude-code/neatcontext/hooks/session-start.mjs index 69ca94e..53b00ad 100644 --- a/plugins/claude-code/neatcontext/hooks/session-start.mjs +++ b/plugins/claude-code/neatcontext/hooks/session-start.mjs @@ -19,10 +19,6 @@ import { configureSessionId } from "../src/core/session.mjs"; import { readSelection } from "../src/core/local-state.mjs"; -// Registers which environment variable names this host's process, so `hostKey()` -// below resolves to the same pointer file the bridge and the slash commands use. -// The session provider it installs is overridden explicitly further down. -import "../src/claude/session.mjs"; import { hostKey, pruneHostPointers, diff --git a/plugins/claude-code/neatcontext/hooks/stop.mjs b/plugins/claude-code/neatcontext/hooks/stop.mjs index 05b0234..16d8e61 100644 --- a/plugins/claude-code/neatcontext/hooks/stop.mjs +++ b/plugins/claude-code/neatcontext/hooks/stop.mjs @@ -16,10 +16,6 @@ // user sees, and there is no longer anything worth showing them. import { configureSessionId } from "../src/core/session.mjs"; -// Registers which environment variable names this host's process, so the pointer -// written below lands on the file the bridge reads. The session provider it -// installs is overridden explicitly further down. -import "../src/claude/session.mjs"; import { writeHostPointer } from "../src/core/host-session.mjs"; import { updateRouting } from "../src/core/routing.mjs"; import { normalizeSaveState, rememberTranscriptPath } from "../src/core/session-state.mjs"; diff --git a/plugins/claude-code/neatcontext/src/claude/session.mjs b/plugins/claude-code/neatcontext/src/claude/session.mjs index c88e7ad..d528baa 100644 --- a/plugins/claude-code/neatcontext/src/claude/session.mjs +++ b/plugins/claude-code/neatcontext/src/claude/session.mjs @@ -13,17 +13,7 @@ // commands correct the stale copy. See core/host-session.mjs. import { configureSessionId } from "../core/session.mjs"; -import { - configureHostPid, - publishBridgeSession, - resolveHostSessionId -} from "../core/host-session.mjs"; - -// Claude Code sets CLAUDE_PID in the environment of every process it spawns — -// including the shell a slash command runs in, where `process.ppid` is the shell -// rather than the host. Without it the CLI would key on the shell and write a -// pointer the bridge never reads. -configureHostPid(() => process.env.CLAUDE_PID); +import { publishBridgeSession, resolveHostSessionId } from "../core/host-session.mjs"; // When this process started. A pointer older than this cannot be describing a // session change that happened after it, and is therefore not about this host. diff --git a/plugins/claude-code/neatcontext/src/core/host-session.mjs b/plugins/claude-code/neatcontext/src/core/host-session.mjs index e576d78..d790f24 100644 --- a/plugins/claude-code/neatcontext/src/core/host-session.mjs +++ b/plugins/claude-code/neatcontext/src/core/host-session.mjs @@ -1,17 +1,17 @@ // Which session the host process is on *right now*. // // A host that identifies its sessions through the environment has a problem the -// rest of this plugin cannot see: the environment is a snapshot. Hosts spawn the -// MCP bridge once and keep it for the life of the window, but the session -// changes underneath it — starting a fresh conversation does not restart the -// server — and the bridge's copy of the id is whatever it was at spawn. The -// hooks and the slash-command CLI are spawned fresh and see the new one. +// rest of this plugin cannot see: the environment is a snapshot. Claude Code +// spawns the MCP bridge once and keeps it for the life of the window, but the +// session id changes underneath it — `/clear` starts a new session in the same +// process — and the bridge's copy of `CLAUDE_CODE_SESSION_ID` is whatever it was +// at spawn. The slash commands are spawned fresh and see the new one. // // So the two halves of the plugin end up reading and writing different files: -// `use` writes the selection for the session the user is in, the bridge keeps -// serving the session the window started in, and nothing in either path can -// observe the disagreement. The user is told a context is connected and every -// later answer is grounded in a different one. +// `/neatcontext:use` writes the selection for the session the user is in, the +// bridge keeps serving the session the window started in, and nothing in either +// path can observe the disagreement. The user is told a context is connected and +// every later answer is grounded in a different one. // // This module is the missing channel. One pointer file per host *process*, // written by whichever short-lived process was just handed the current session @@ -21,22 +21,19 @@ // // The host key has to be stable across a session change and distinct per window, // which rules out the session id and the working directory both. What is left is -// the host process itself. How to name it is the one thing that differs per -// host, so each adapter registers its own answer with `configureHostPid()`: some -// hosts publish their own pid to every process they spawn, and a host that -// publishes nothing falls back to `process.ppid`, which is the host for any -// process it spawned directly. A process whose parent is not the host and whose -// host publishes no pid — a CLI running inside a shell tool — computes a key -// nothing else reads, and its writes are harmless and swept later. +// the host process itself: Claude Code publishes its pid as CLAUDE_PID to the +// processes it spawns, and the bridge is its direct child, so `process.ppid` is +// the same number seen from the other side. Two windows are two pids and never +// share a pointer. // // The bridge also publishes what it actually resolved: // // ~/.neatcontext/plugin-hosts/.bridge.json { pid, sessionId, updatedAt } // -// That is what lets `use` check its own success against the process that will -// serve it, instead of against the file it just wrote itself. +// That is what lets `/neatcontext:use` check its own success against the process +// that will serve it, instead of against the file it just wrote itself. -import { mkdir, readFile, readdir, rename, rm, writeFile } from "node:fs/promises"; +import { mkdir, readFile, readdir, rm, writeFile } from "node:fs/promises"; import path from "node:path"; import { neatContextHome } from "./storage-home.mjs"; @@ -62,70 +59,24 @@ function pidKey(value) { return /^[1-9][0-9]{0,9}$/.test(pid) ? `pid-${pid}` : null; } -// Where the host publishes its own process id, when it publishes one at all. -// -// Deliberately per adapter rather than a list of every host's variable checked -// in turn: those variables are inherited by child processes, so a host launched -// from another host's shell would key on the outer host and two different hosts -// could end up sharing one pointer file. Each adapter consults only the variable -// its own host sets. -// -// Process-global, because one process runs one host's adapter. A test that -// imported two adapters into the same process would have the second silently -// win; there is no such test, and there should not be one. -let hostPidProvider = null; - -export function configureHostPid(provider) { - if (provider !== null && typeof provider !== "function") { - throw new TypeError("The host pid provider must be a function or null."); - } - hostPidProvider = provider; -} - -function hostPid() { - if (!hostPidProvider) { - return null; - } - // An adapter reading a value the host never set must not take down the only - // path that can name this process's host. - try { - return hostPidProvider(); - } catch { - return null; - } -} - -// A host key names one directory entry, so it is held to the same rule as -// anything else that becomes a path segment. Exported because an adapter that -// wants to know whether a shared key is available has to ask the same question -// this does — a value accepted there and rejected here would claim a channel -// that was never opened. -export function normalizeHostKey(value) { - if (typeof value !== "string") { - return null; - } - const trimmed = value.trim(); - return trimmed.length > 0 && - trimmed !== "." && - trimmed !== ".." && - !trimmed.includes("/") && - !trimmed.includes("\\") - ? trimmed - : null; -} - // The host process this process belongs to. // // NEATCONTEXT_HOST_KEY is the explicit form, for tests and for any host that can -// hand every one of its plugin processes the same identifier. Otherwise the pid -// the adapter knows how to find, and failing that `process.ppid`, which is the -// host for a server or hook it spawned directly. +// hand every one of its plugin processes the same identifier. Otherwise: +// CLAUDE_PID, which Claude Code sets in the environment of the processes it +// spawns — including the shell a slash command runs in, where `process.ppid` is +// the shell rather than the host. `process.ppid` is the fallback for the bridge, +// which the host spawns directly. export function hostKey() { const explicit = process.env.NEATCONTEXT_HOST_KEY; if (typeof explicit === "string") { - return normalizeHostKey(explicit); + const trimmed = explicit.trim(); + return trimmed.length > 0 && trimmed !== "." && trimmed !== ".." && !trimmed.includes("/") && + !trimmed.includes("\\") + ? trimmed + : null; } - return pidKey(hostPid()) ?? pidKey(process.ppid); + return pidKey(process.env.CLAUDE_PID) ?? pidKey(process.ppid); } export function hostsDirectory() { @@ -161,31 +112,12 @@ async function readPointer(file) { } } -let writeCounter = 0; - -// Written to a temporary name and renamed into place, because these files are -// read by other processes while they are being written: the bridge re-reads the -// pointer on every message, and a reader that catches a half-written file falls -// back to the environment, which is the stale answer this file exists to -// correct. A rename is atomic, so a reader sees the old record or the new one. -// -// The temporary name carries this process's pid so that two processes writing -// the same pointer at once cannot collide on it, and stays in the same directory -// so the rename never crosses a volume. async function writePointer(file, value) { await mkdir(path.dirname(file), { recursive: true }); - writeCounter += 1; - const temporary = `${file}.${process.pid}.${writeCounter}.tmp`; - try { - await writeFile(temporary, `${JSON.stringify(value, null, 2)}\n`, { - encoding: "utf8", - mode: 0o600 - }); - await rename(temporary, file); - } catch (error) { - await rm(temporary, { force: true }).catch(() => undefined); - throw error; - } + await writeFile(file, `${JSON.stringify(value, null, 2)}\n`, { + encoding: "utf8", + mode: 0o600 + }); } export async function readHostPointer(key = hostKey()) { @@ -250,11 +182,6 @@ export async function resolveHostSessionId(envSessionId, { since = 0, key = host // rewriting. Age is not staleness here — a record from an hour ago is still what // this bridge is serving. Whether it is *running* is a question about its pid, // which the record carries. -// -// The cache is what this process last wrote, not what is on disk. Something that -// deleted the file from underneath it would not get it back until the session -// changes; nothing here does that, and defending against an outside `rm` would -// cost a stat on every message. let lastPublished = { id: undefined }; export async function publishBridgeSession(id, { key = hostKey(), now = Date.now() } = {}) { diff --git a/plugins/copilot/neatcontext/README.md b/plugins/copilot/neatcontext/README.md index 72cb410..fb050da 100644 --- a/plugins/copilot/neatcontext/README.md +++ b/plugins/copilot/neatcontext/README.md @@ -47,17 +47,11 @@ copilot plugin install neatcontext@neatcontext - **Local Contexts.** The plugin stores Contexts on this machine. There is no NeatContext Desktop connection right now. -- **Selections are per session.** A connected context belongs to the Copilot - session that connected it, as on every other host. Where a Copilot build - publishes no session identity, the plugin falls back to the workspace folder - and every session opened there shares one selection. It says so when that - happens, rather than leaving you to guess. -- **Upgrading from 0.3.3 or earlier.** Those releases scoped every Copilot - selection to the workspace folder. The first time you run a command after - upgrading, this session has no connection of its own yet, so - `/neatcontext:status` reports none and names the context that workspace used - to be connected to. Reconnect it once with `/neatcontext:use` and each session - keeps its own from then on. Nothing is deleted. +- **Selections are per session on Copilot CLI.** The command and MCP processes + use Copilot's shared session identity, so they agree even when they start in + different working directories. +- **Workspace fallback.** On a host that does not expose a session identity, + sessions opened in the same workspace share one selection. - **Saving is explicit.** Run `/neatcontext:save` when the visible conversation contains durable work worth preserving. diff --git a/plugins/copilot/neatcontext/commands/disconnect.md b/plugins/copilot/neatcontext/commands/disconnect.md index 9a50585..288327f 100644 --- a/plugins/copilot/neatcontext/commands/disconnect.md +++ b/plugins/copilot/neatcontext/commands/disconnect.md @@ -9,4 +9,5 @@ Disconnect the context currently connected to this session. !`node "${CLAUDE_PLUGIN_ROOT}/src/copilot/neatcontext-cli.mjs" disconnect` Relay the result verbatim. Do not run a status command afterward. This affects -only the current session; other sessions keep their own connections. +only the current session. On hosts without a session identity, the selection is +shared by sessions opened in the same workspace. diff --git a/plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs b/plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs index 95fe625..bf332df 100644 --- a/plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs +++ b/plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs @@ -1,11 +1,6 @@ // NeatContext plugin MCP server for GitHub Copilot. // // Behaviors kept from the Claude Code bridge: -// * this process outlives the session it was spawned in — a new session -// starts 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 previous session's context while the -// slash commands 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 +10,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, @@ -170,11 +165,11 @@ A context is whatever its profile says it is. Do not assume a subject area for i Cite the exact file path of anything you rely on. When the profile and the knowledge folder do not cover the question, say so instead of answering from general knowledge.`; // Written to survive being wrong. These instructions are fixed at the -// handshake, but a context can be connected at any time afterwards — by a -// slash command in this session, or by the session's routing. 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 connected at the moment this session started. That says nothing about now: a Context can be connected at any point later in this session. +// handshake, but a context can be connected at any time afterwards — from this +// 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 connected at the moment this session started. That says nothing about now: a Context can be connected at any time, from this session or another window on this workspace. These instructions are fixed at the handshake and cannot be updated, so they are not evidence about the current state — and you must not tell the user nothing is connected on the strength of this text. @@ -474,38 +469,23 @@ function refusal(policy, target) { let started = false; let lastVersion = undefined; -// Re-resolve which session this process is serving, and publish the answer so a -// slash command can tell whether its write is the one this bridge will read. -async function syncSession() { - await refreshSessionId(); - await publishSessionId(); -} - // 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. async function currentVersion() { - // The session is part of it: a new session 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 session's extension tools. - const session = sessionId() ?? "none"; const mode = resolveMode(await readRouting(), sessionId()); const context = await activeContext(); // The extension signature is read from what the last resolve found, never by // starting anything: this runs on a timer, and a poll must not spawn a server. const extensions = extensionHost.signature(context?.record ?? null); if (context) { - return `${session}/${mode}/${context.missing ? "context:missing" : context.record.id}/${extensions}`; + return `${mode}/${context.missing ? "context:missing" : context.record.id}/${extensions}`; } - return `${session}/${mode}/none/${extensions}`; + return `${mode}/none/${extensions}`; } async function handleMessage(message) { const isNotification = message.id === undefined || message.id === null; - // Before anything reads a selection or a routing mode: which session 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. @@ -618,10 +598,6 @@ function startVersionWatch() { watching = true; setInterval(async () => { if (!started) return; - // The host does not send a message when the session changes underneath this - // process, so this tick is where that is noticed if nothing else asks first - // — and where the published answer stays fresh enough to be checked against. - await syncSession(); const version = await currentVersion(); if (version !== null && version !== lastVersion) { lastVersion = version; diff --git a/plugins/copilot/neatcontext/src/copilot/neatcontext-cli.mjs b/plugins/copilot/neatcontext/src/copilot/neatcontext-cli.mjs index b78e8d9..f051f2b 100644 --- a/plugins/copilot/neatcontext/src/copilot/neatcontext-cli.mjs +++ b/plugins/copilot/neatcontext/src/copilot/neatcontext-cli.mjs @@ -20,9 +20,8 @@ // Exit code is always 0: the output is meant to be read, not branched on. import { readFile, rm } from "node:fs/promises"; -import { sessionIdentityIsShared, unusableSessionOverride, workspaceSessionId } from "./session.mjs"; -import { clearSelection, readSelection, sessionSelectionFilePath } from "../core/local-state.mjs"; -import { awaitBridgeSession, readBridgeSession, writeHostPointer } from "../core/host-session.mjs"; +import "./session.mjs"; +import { clearSelection, readSelection } from "../core/local-state.mjs"; import { createCapturedContext, createContext, @@ -68,119 +67,6 @@ function print(line = "") { process.stdout.write(`${line}\n`); } -// This process is spawned for one command and its environment is fresh, so the -// session it names is the session the user is actually in. The MCP bridge was -// spawned once, when the window opened, and cannot know that a new session -// replaced it. Recording it here is what lets the bridge read the selection this -// command is about to write. -// -// Every command records it, not only the ones that write a selection. Copilot -// ships no hooks, so this process is the only thing that can ever tell the -// bridge what session the window is on — and a read-only command is often the -// first one to run after a session changes, which makes it the earliest chance -// to heal the pointer. One small rename is worth that. -async function recordHostSession() { - return writeHostPointer(sessionId(), { source: "cli" }).catch(() => null); -} - -// Whether the two halves of the plugin can agree on a session at all. -// -// Every other warning here assumes they can and reports a disagreement. This one -// is about the case where there is no channel between them: a Copilot build that -// publishes neither a session id nor its own pid leaves the bridge and this -// process hashing their own working directories, which are not the same -// directory. Nothing downstream can detect that, so it is said once, here. -function hostIdentityWarning() { - if (sessionIdentityIsShared()) { - return null; - } - return ( - "Warning: this Copilot build publishes no session identity to plugin processes, " + - "so NeatContext cannot tell which session its MCP server is serving. A connected " + - "context may not reach it. Set NEATCONTEXT_SESSION_ID to any short identifier to " + - "pin one, and report your Copilot version at " + - "https://github.com/XTSoftwareLabs/neatcontext-plugins/issues." - ); -} - -// A selection saved by a release that scoped Copilot to the workspace folder. -// -// Those releases wrote `plugin-sessions/copilot-ws-.json`, which nothing -// reads once the host publishes a real session id. Saying so beats letting a -// working connection look like it disappeared — and it is only said when this -// workspace actually has one and nothing is connected now. -async function workspaceSelectionHint() { - const workspace = workspaceSessionId(); - if (sessionId() === workspace) { - return null; - } - try { - const saved = JSON.parse(await readFile(sessionSelectionFilePath(workspace), "utf8")); - const name = typeof saved?.contextName === "string" ? saved.contextName.trim() : ""; - if (!name) { - return null; - } - return ( - `An earlier version of this plugin connected "${name}" to this workspace rather ` + - "than to a session. Sessions each carry their own connection now, so reconnect " + - `it once with \`/neatcontext:use ${name}\`.` - ); - } catch { - return null; - } -} - -// The hint above is a one-time thing, so the record behind it is retired the -// moment it has served its purpose: the first connection made in a session of -// its own is the user acting on it, whether or not they picked the same context. -// Left in place it would come back after every deliberate `/neatcontext:disconnect`, -// telling the user to reconnect the thing they just disconnected, forever. -// -// Never when this session *is* the workspace fallback — that file is the live -// selection then, not a leftover. -async function retireWorkspaceSelection() { - const workspace = workspaceSessionId(); - if (sessionId() === workspace) { - return; - } - await rm(sessionSelectionFilePath(workspace), { force: true }).catch(() => undefined); -} - -// Whether the bridge that will serve this session 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 session's file would leave the message true about the disk -// and false about the session. The bridge publishes what it resolved, so ask it. -// A bridge that publishes nothing (an older build, or none running) is not -// evidence of anything and gets no warning. -// -// This waits: a bridge that has not yet noticed the session change re-resolves -// within its watch interval, so the first command after a change can take about -// a second longer than the rest. Reporting drift that was about to resolve -// itself would be worse than the wait. -async function bridgeDriftWarning() { - const id = sessionId(); - const { state } = await awaitBridgeSession(id); - if (state !== "drifted") { - return null; - } - return ( - "Warning: NeatContext's MCP server in this window is still serving an earlier " + - "session and has not picked this one up, so `get_context` may keep returning the " + - "previous context. Restart Copilot 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 = {}; @@ -254,19 +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 session, this is a report about a selection it is not reading. - // The ids themselves are not printed — the user cannot act on a session uuid, - // and the fact of the disagreement is the whole message. - const bridge = await readBridgeSession().catch(() => null); - if (bridge && bridge.sessionId !== sessionId()) { - print( - "Warning: NeatContext's MCP server in this window is serving an earlier session, " + - "not this one. What follows is what this session selected; `get_context` may " + - "still answer from the other one. Restart Copilot to clear it." - ); - print(""); - } const routing = await readRouting(); const mode = resolveMode(routing, sessionId()); // Reported alongside the connection because the two together are the whole @@ -341,11 +214,6 @@ async function commandStatus(state) { "`/neatcontext:create`." : "No context is connected yet. Use `/neatcontext:use` to pick one." ); - const upgrade = await workspaceSelectionHint(); - if (upgrade) { - print(""); - print(upgrade); - } reportMode(); } @@ -511,13 +379,11 @@ async function commandUse(state, query) { const target = resolution.context; const result = await applySelection(target); - await retireWorkspaceSelection(); print( `Connected the "${result.name}" context. Your next messages in this session ` + "will be grounded in its domain profile and knowledge folder." ); await nudgeForDescription(target); - await printBridgeDrift(); } async function commandDisconnect(state) { @@ -532,9 +398,6 @@ async function commandDisconnect(state) { const name = connected?.name ?? remembered.contextName; print(`Disconnected the "${name}" context from this session.`); - // Same exposure as connecting: a bridge on another session clears nothing the - // session it is serving can see. - await printBridgeDrift(); } // A context with no routing description can only be routed to by name. @@ -718,13 +581,11 @@ function printUpdatePreview(preview) { async function printSaveConnection(record) { const outcome = await connectAfterSave(record).catch(() => null); if (outcome?.connected) { - await retireWorkspaceSelection(); print(`Connected context: ${record.name}`); print( "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}`); @@ -998,31 +859,6 @@ async function run() { const [command = "status", ...rest] = process.argv.slice(2); const { flags, query } = parseArgs(rest); - // Before anything reads a selection, because both of these say the session - // this command is about to act on may not be the one the user is in. They go - // to stderr so they cannot be mistaken for a command's result by whatever is - // relaying stdout verbatim. - const override = unusableSessionOverride(); - if (override) { - process.stderr.write( - `NEATCONTEXT_SESSION_ID is set to something that cannot name a session file ` + - `(${override.length} characters, letters, digits, dot, dash and underscore only), ` + - "so it is being ignored.\n" - ); - } - const identity = hostIdentityWarning(); - if (identity) { - process.stderr.write(`${identity}\n`); - } - - // Every invocation, not just the ones that change the selection. Copilot has - // no hook that runs when a session starts or ends, so this process is the only - // one that ever reports the current session to the long-lived bridge — and the - // report is worth making before a command writes a selection the bridge would - // otherwise never read, and after one, to heal a record something else wrote - // wrongly in between. - await recordHostSession(); - if (command === "create") { await commandCreate(flags); return; diff --git a/plugins/copilot/neatcontext/src/copilot/session.mjs b/plugins/copilot/neatcontext/src/copilot/session.mjs index 7baa30e..e5c35ab 100644 --- a/plugins/copilot/neatcontext/src/copilot/session.mjs +++ b/plugins/copilot/neatcontext/src/copilot/session.mjs @@ -1,61 +1,29 @@ // GitHub Copilot host adapter for the reusable session-aware runtime. // -// This file used to say that Copilot exposes no session identity to plugin -// processes, and derived one by hashing `process.cwd()` instead — on the premise -// that every plugin process is given the workspace as its working directory. -// That premise is false. On Copilot CLI the MCP bridge is spawned with the -// *plugin installation* directory as its working directory while the CLI a slash -// command runs is spawned with the user's workspace, so the two halves of the -// plugin hashed different paths, read and wrote different selection files, and -// neither could observe the disagreement: `/neatcontext:use` reported success -// for a file `get_context` would never read. +// Copilot CLI exposes the current session to both the command process and the +// MCP server as COPILOT_AGENT_SESSION_ID. Prefer it so both halves select the +// same context even when the host starts them in different working directories. // -// Copilot does publish a session identity, to the bridge and to the CLI alike: +// The workspace digest remains the fallback for hosts that do not publish a +// session id. In that case all Copilot sessions opened in one workspace share +// the selection, as they did before. // -// COPILOT_AGENT_SESSION_ID the session, distinct per window -// COPILOT_LOADER_PID the host process, which the bridge also sees as -// its own parent -// -// So a session here is a session, as on every other host, rather than a -// workspace. The workspace digest remains as the fallback for a Copilot build -// that publishes no session id, which keeps that case working exactly as it did -// — and no worse. -// -// Like Claude Code and Codex, the id is *resolved* rather than read: the bridge -// is spawned once and outlives the session it was spawned in, so it asks -// `refreshSessionId()` before it handles anything, which lets the pointer -// written by the freshly spawned CLI correct a stale copy. See -// core/host-session.mjs. -// -// NEATCONTEXT_SESSION_ID overrides everything — for tests, and for any host that -// can inject a real per-session id into every plugin process. +// NEATCONTEXT_SESSION_ID overrides the digest — for tests, and for any host +// that can inject a real per-session id into every plugin process. // // CLAUDE_CODE_SESSION_ID is deliberately NOT consulted, even though a -// Claude-compat host might set it: it leaks into child shells when the user -// launches Copilot from inside a Claude Code session, which would scope this -// plugin to the outer host's session. +// Claude-compat host might set it: a variable only some of this plugin's +// processes see is worse than none, because the CLI and the MCP server would +// scope to different sessions and the selection would silently split. (It +// also leaks into child shells when the user launches Copilot from inside a +// Claude Code session, which would hijack the scope the same way.) import { createHash } from "node:crypto"; import path from "node:path"; import { configureSessionId } from "../core/session.mjs"; -import { - configureHostPid, - normalizeHostKey, - normalizeHostSessionId, - publishBridgeSession, - resolveHostSessionId -} from "../core/host-session.mjs"; -// When this process started. A pointer older than this cannot be describing a -// session change that happened after it, and is therefore not about this host. -const STARTED_AT = Date.now(); - -// A host-supplied id becomes a path segment (`plugin-sessions/.json`), so it -// is held to the same rule as one arriving from a pointer file. An unusable -// value falls through to the next source rather than being repaired: an id this -// plugin invented would not be the one the other half of it computes. function explicitId(value) { - return normalizeHostSessionId(value); + return typeof value === "string" && value.trim().length > 0 ? value.trim() : null; } export function workspaceSessionId(workspace = process.cwd()) { @@ -67,10 +35,7 @@ export function workspaceSessionId(workspace = process.cwd()) { return `copilot-ws-${digest}`; } -// What this process was started with. Never null: the workspace digest always -// answers, because a null session id would send every workspace to the one -// unscoped selection file they would then share. -export function environmentSessionId() { +export function copilotSessionId() { return ( explicitId(process.env.NEATCONTEXT_SESSION_ID) ?? explicitId(process.env.COPILOT_AGENT_SESSION_ID) ?? @@ -78,71 +43,4 @@ export function environmentSessionId() { ); } -// Set, but not to something that can be a path segment. -// -// Worth saying out loud rather than falling through in silence: this used to -// accept any non-empty string, so a value with a `/` or a `:` in it that worked -// before now scopes the session somewhere else entirely. -export function unusableSessionOverride() { - const raw = process.env.NEATCONTEXT_SESSION_ID; - if (typeof raw !== "string" || raw.trim().length === 0) { - return null; - } - return explicitId(raw) ? null : raw.trim(); -} - -// Whether the bridge and this process can arrive at the same session at all. -// -// One of these has to hold, or the two halves are back to hashing their own -// working directories — which are not the same directory — with no channel -// between them. It is not a guess: each branch is a thing the host either -// publishes to both processes or does not. -export function sessionIdentityIsShared() { - return Boolean( - explicitId(process.env.NEATCONTEXT_SESSION_ID) ?? - explicitId(process.env.COPILOT_AGENT_SESSION_ID) ?? - // The same rule `hostKey()` applies, asked through the same function: a - // key accepted here and rejected there would claim a channel that was - // never opened, which is the one thing this must not do. - normalizeHostKey(process.env.NEATCONTEXT_HOST_KEY) ?? - // A host pid both halves can see is enough on its own: it names the - // pointer file they share, which is what carries the session across. - (/^[1-9][0-9]{0,9}$/.test(String(process.env.COPILOT_LOADER_PID ?? "").trim()) - ? "pid" - : null) - ); -} - -// Until something resolves it, the environment answers directly — which is what -// every process that is spawned per command wants, and what this file did before -// there was anything else to consult. -const UNRESOLVED = Symbol("unresolved"); -let resolved = UNRESOLVED; - -export function copilotSessionId() { - return resolved === UNRESOLVED ? environmentSessionId() : resolved; -} - -// Re-resolves the session 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(environmentSessionId(), { 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(copilotSessionId() ?? null); -} - -// Copilot publishes the host's own pid to the processes it spawns, including the -// shell a slash command runs in — where `process.ppid` is that shell rather than -// the host. The bridge, spawned by the host directly, sees the same number as -// its parent, so both halves agree on which pointer file is theirs. -configureHostPid(() => process.env.COPILOT_LOADER_PID); configureSessionId(copilotSessionId); diff --git a/plugins/copilot/neatcontext/src/core/host-session.mjs b/plugins/copilot/neatcontext/src/core/host-session.mjs deleted file mode 100644 index e576d78..0000000 --- a/plugins/copilot/neatcontext/src/core/host-session.mjs +++ /dev/null @@ -1,360 +0,0 @@ -// Which session the host process is on *right now*. -// -// A host that identifies its sessions through the environment has a problem the -// rest of this plugin cannot see: the environment is a snapshot. Hosts spawn the -// MCP bridge once and keep it for the life of the window, but the session -// changes underneath it — starting a fresh conversation does not restart the -// server — and the bridge's copy of the id is whatever it was at spawn. The -// hooks and the slash-command CLI are spawned fresh and see the new one. -// -// So the two halves of the plugin end up reading and writing different files: -// `use` writes the selection for the session the user is in, the bridge keeps -// serving the session the window started in, and nothing in either path can -// observe the disagreement. The user is told a context is connected and every -// later answer is grounded in a different one. -// -// This module is the missing channel. One pointer file per host *process*, -// written by whichever short-lived process was just handed the current session -// id, and re-read by the long-lived one on every message: -// -// ~/.neatcontext/plugin-hosts/.json { sessionId, source, updatedAt } -// -// The host key has to be stable across a session change and distinct per window, -// which rules out the session id and the working directory both. What is left is -// the host process itself. How to name it is the one thing that differs per -// host, so each adapter registers its own answer with `configureHostPid()`: some -// hosts publish their own pid to every process they spawn, and a host that -// publishes nothing falls back to `process.ppid`, which is the host for any -// process it spawned directly. A process whose parent is not the host and whose -// host publishes no pid — a CLI running inside a shell tool — computes a key -// nothing else reads, and its writes are harmless and swept later. -// -// The bridge also publishes what it actually resolved: -// -// ~/.neatcontext/plugin-hosts/.bridge.json { pid, sessionId, updatedAt } -// -// That is what lets `use` check its own success against the process that will -// serve it, instead of against the file it just wrote itself. - -import { mkdir, readFile, readdir, rename, rm, writeFile } from "node:fs/promises"; -import path from "node:path"; -import { neatContextHome } from "./storage-home.mjs"; - -// A session id becomes a path segment (`plugin-sessions/.json`), and unlike -// the environment — which only the host can set — this one arrives from a file. -// Anything that could climb out of the 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 normalizeHostSessionId(value) { - if (typeof value !== "string") { - return null; - } - const id = value.trim(); - if (id === "." || id === ".." || !SAFE_SESSION_ID.test(id)) { - return null; - } - return id; -} - -function pidKey(value) { - const pid = typeof value === "string" ? value.trim() : String(value ?? ""); - return /^[1-9][0-9]{0,9}$/.test(pid) ? `pid-${pid}` : null; -} - -// Where the host publishes its own process id, when it publishes one at all. -// -// Deliberately per adapter rather than a list of every host's variable checked -// in turn: those variables are inherited by child processes, so a host launched -// from another host's shell would key on the outer host and two different hosts -// could end up sharing one pointer file. Each adapter consults only the variable -// its own host sets. -// -// Process-global, because one process runs one host's adapter. A test that -// imported two adapters into the same process would have the second silently -// win; there is no such test, and there should not be one. -let hostPidProvider = null; - -export function configureHostPid(provider) { - if (provider !== null && typeof provider !== "function") { - throw new TypeError("The host pid provider must be a function or null."); - } - hostPidProvider = provider; -} - -function hostPid() { - if (!hostPidProvider) { - return null; - } - // An adapter reading a value the host never set must not take down the only - // path that can name this process's host. - try { - return hostPidProvider(); - } catch { - return null; - } -} - -// A host key names one directory entry, so it is held to the same rule as -// anything else that becomes a path segment. Exported because an adapter that -// wants to know whether a shared key is available has to ask the same question -// this does — a value accepted there and rejected here would claim a channel -// that was never opened. -export function normalizeHostKey(value) { - if (typeof value !== "string") { - return null; - } - const trimmed = value.trim(); - return trimmed.length > 0 && - trimmed !== "." && - trimmed !== ".." && - !trimmed.includes("/") && - !trimmed.includes("\\") - ? trimmed - : null; -} - -// The host process this process belongs to. -// -// NEATCONTEXT_HOST_KEY is the explicit form, for tests and for any host that can -// hand every one of its plugin processes the same identifier. Otherwise the pid -// the adapter knows how to find, and failing that `process.ppid`, which is the -// host for a server or hook it spawned directly. -export function hostKey() { - const explicit = process.env.NEATCONTEXT_HOST_KEY; - if (typeof explicit === "string") { - return normalizeHostKey(explicit); - } - return pidKey(hostPid()) ?? pidKey(process.ppid); -} - -export function hostsDirectory() { - return path.join(neatContextHome(), "plugin-hosts"); -} - -export function hostPointerPath(key) { - return path.join(hostsDirectory(), `${key}.json`); -} - -export function bridgePointerPath(key) { - return path.join(hostsDirectory(), `${key}.bridge.json`); -} - -async function readPointer(file) { - try { - const parsed = JSON.parse(await readFile(file, "utf8")); - const id = normalizeHostSessionId(parsed?.sessionId); - if (!id) { - return null; - } - const updatedAt = Date.parse(parsed?.updatedAt); - return { - sessionId: id, - source: typeof parsed?.source === "string" ? parsed.source : "unknown", - updatedAt: Number.isNaN(updatedAt) ? 0 : updatedAt, - pid: typeof parsed?.pid === "number" ? parsed.pid : null - }; - } catch { - // Missing, half-written, or hand-broken: the caller falls back to the - // environment, which is exactly the behavior that predates this file. - return null; - } -} - -let writeCounter = 0; - -// Written to a temporary name and renamed into place, because these files are -// read by other processes while they are being written: the bridge re-reads the -// pointer on every message, and a reader that catches a half-written file falls -// back to the environment, which is the stale answer this file exists to -// correct. A rename is atomic, so a reader sees the old record or the new one. -// -// The temporary name carries this process's pid so that two processes writing -// the same pointer at once cannot collide on it, and stays in the same directory -// so the rename never crosses a volume. -async function writePointer(file, value) { - await mkdir(path.dirname(file), { recursive: true }); - writeCounter += 1; - const temporary = `${file}.${process.pid}.${writeCounter}.tmp`; - try { - await writeFile(temporary, `${JSON.stringify(value, null, 2)}\n`, { - encoding: "utf8", - mode: 0o600 - }); - await rename(temporary, file); - } catch (error) { - await rm(temporary, { force: true }).catch(() => undefined); - throw error; - } -} - -export async function readHostPointer(key = hostKey()) { - return key ? readPointer(hostPointerPath(key)) : null; -} - -// Called by every process the host hands a session id to: the hooks, which are -// told on stdin, and the CLI, whose environment is fresh because it was spawned -// for this command. Last write wins — they are all reporting the same fact, and -// a hook runs at the end of every turn, so a wrong one cannot persist. -// -// Returns the id actually recorded, or null when there was nothing to record — -// no host key, or an id the host did not supply. -export async function writeHostPointer(rawId, { source = "unknown", key = hostKey() } = {}) { - const id = normalizeHostSessionId(rawId); - if (!id || !key) { - return null; - } - try { - await writePointer(hostPointerPath(key), { - sessionId: id, - source, - pid: process.pid, - updatedAt: new Date().toISOString() - }); - return id; - } catch { - // Recording this is never worth failing the command that triggered it: the - // caller keeps working against the environment, as it did before. - return null; - } -} - -// The session id to use, given what this process was started with. -// -// The pointer wins when the two disagree, because by construction it is the -// newer of the two: the environment was frozen when this process started and the -// pointer was written by a process that started later. -// -// The one case where that reasoning fails is a leftover pointer from a host that -// used to have this pid. It cannot describe a change that happened after this -// process started, so a *disagreeing* pointer older than this process is ignored. -// A real session change is always newer than the bridge that is reading it. -export async function resolveHostSessionId(envSessionId, { since = 0, key = hostKey() } = {}) { - const fallback = normalizeHostSessionId(envSessionId) ?? envSessionId ?? null; - const pointer = await readHostPointer(key); - if (!pointer || pointer.sessionId === envSessionId) { - return fallback; - } - if (since > 0 && pointer.updatedAt < since) { - return fallback; - } - return pointer.sessionId; -} - -// What the bridge resolved, published so a slash command can check its success -// against the process that will actually serve it rather than against its own -// write. -// -// Written when the answer changes and not on a timer: the bridge re-resolves -// several times a second and a file that says the same thing is not worth -// rewriting. Age is not staleness here — a record from an hour ago is still what -// this bridge is serving. Whether it is *running* is a question about its pid, -// which the record carries. -// -// The cache is what this process last wrote, not what is on disk. Something that -// deleted the file from underneath it would not get it back until the session -// changes; nothing here does that, and defending against an outside `rm` would -// cost a stat on every message. -let lastPublished = { id: undefined }; - -export async function publishBridgeSession(id, { key = hostKey(), now = Date.now() } = {}) { - if (!key) { - return false; - } - if (lastPublished.id === id) { - return false; - } - try { - await writePointer(bridgePointerPath(key), { - pid: process.pid, - sessionId: id ?? null, - updatedAt: new Date(now).toISOString() - }); - // Only once it is actually on disk: a write that failed has published - // nothing, and must not stop the next attempt. - lastPublished = { id }; - return true; - } catch { - return false; - } -} - -// Only a record left by a bridge that is still running says anything about what -// this session is being served right now. -export async function readBridgeSession(key = hostKey(), { alive = isProcessAlive } = {}) { - const record = key ? await readPointer(bridgePointerPath(key)) : null; - if (!record || record.pid === null || !alive(record.pid)) { - return null; - } - return record; -} - -// Waits for the bridge to be serving `id`, and reports what it found rather than -// deciding what to do about it: the caller is a command whose success message -// depends on the answer. -// -// `state` is "matched" (the bridge is on this session), "unknown" (no bridge is -// publishing, so there is nothing to check against — an older bridge, or none -// running at all), or "drifted". -export async function awaitBridgeSession( - id, - { timeoutMs = 3000, intervalMs = 100, key = hostKey() } = {} -) { - if (!id || !key) { - return { state: "unknown" }; - } - const deadline = Date.now() + timeoutMs; - let seen = null; - for (;;) { - seen = await readBridgeSession(key); - if (seen?.sessionId === id) { - return { state: "matched", seen }; - } - // Nothing is publishing: a bridge from before this existed, or none running. - // There is no answer coming, so waiting for one only delays the command. - if (!seen || Date.now() >= deadline) { - break; - } - await new Promise((resolve) => setTimeout(resolve, intervalMs)); - } - return seen ? { state: "drifted", seen } : { state: "unknown" }; -} - -// Pointers name a process, and processes end. Sweeping them keeps a machine that -// opens a lot of windows from accumulating a file per pid forever, and — more to -// the point — keeps a recycled pid from finding an ancient pointer waiting for -// it. Best effort: a failed sweep is not a failed command. -export async function pruneHostPointers({ alive = isProcessAlive } = {}) { - let entries; - try { - entries = await readdir(hostsDirectory()); - } catch { - return 0; - } - const mine = hostKey(); - let removed = 0; - for (const entry of entries) { - const key = entry.replace(/\.bridge\.json$|\.json$/, ""); - if (key === entry || key === mine) { - continue; - } - const pid = /^pid-([1-9][0-9]{0,9})$/.exec(key); - if (!pid || alive(Number(pid[1]))) { - continue; - } - await rm(path.join(hostsDirectory(), entry), { force: true }).catch(() => undefined); - removed += 1; - } - return removed; -} - -export function isProcessAlive(pid) { - try { - // Signal 0 checks for the process without touching it. EPERM means it exists - // and belongs to someone else, which still counts as alive. - process.kill(pid, 0); - return true; - } catch (error) { - return error?.code === "EPERM"; - } -} diff --git a/plugins/kimi-code/neatcontext/src/core/host-session.mjs b/plugins/kimi-code/neatcontext/src/core/host-session.mjs deleted file mode 100644 index e576d78..0000000 --- a/plugins/kimi-code/neatcontext/src/core/host-session.mjs +++ /dev/null @@ -1,360 +0,0 @@ -// Which session the host process is on *right now*. -// -// A host that identifies its sessions through the environment has a problem the -// rest of this plugin cannot see: the environment is a snapshot. Hosts spawn the -// MCP bridge once and keep it for the life of the window, but the session -// changes underneath it — starting a fresh conversation does not restart the -// server — and the bridge's copy of the id is whatever it was at spawn. The -// hooks and the slash-command CLI are spawned fresh and see the new one. -// -// So the two halves of the plugin end up reading and writing different files: -// `use` writes the selection for the session the user is in, the bridge keeps -// serving the session the window started in, and nothing in either path can -// observe the disagreement. The user is told a context is connected and every -// later answer is grounded in a different one. -// -// This module is the missing channel. One pointer file per host *process*, -// written by whichever short-lived process was just handed the current session -// id, and re-read by the long-lived one on every message: -// -// ~/.neatcontext/plugin-hosts/.json { sessionId, source, updatedAt } -// -// The host key has to be stable across a session change and distinct per window, -// which rules out the session id and the working directory both. What is left is -// the host process itself. How to name it is the one thing that differs per -// host, so each adapter registers its own answer with `configureHostPid()`: some -// hosts publish their own pid to every process they spawn, and a host that -// publishes nothing falls back to `process.ppid`, which is the host for any -// process it spawned directly. A process whose parent is not the host and whose -// host publishes no pid — a CLI running inside a shell tool — computes a key -// nothing else reads, and its writes are harmless and swept later. -// -// The bridge also publishes what it actually resolved: -// -// ~/.neatcontext/plugin-hosts/.bridge.json { pid, sessionId, updatedAt } -// -// That is what lets `use` check its own success against the process that will -// serve it, instead of against the file it just wrote itself. - -import { mkdir, readFile, readdir, rename, rm, writeFile } from "node:fs/promises"; -import path from "node:path"; -import { neatContextHome } from "./storage-home.mjs"; - -// A session id becomes a path segment (`plugin-sessions/.json`), and unlike -// the environment — which only the host can set — this one arrives from a file. -// Anything that could climb out of the 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 normalizeHostSessionId(value) { - if (typeof value !== "string") { - return null; - } - const id = value.trim(); - if (id === "." || id === ".." || !SAFE_SESSION_ID.test(id)) { - return null; - } - return id; -} - -function pidKey(value) { - const pid = typeof value === "string" ? value.trim() : String(value ?? ""); - return /^[1-9][0-9]{0,9}$/.test(pid) ? `pid-${pid}` : null; -} - -// Where the host publishes its own process id, when it publishes one at all. -// -// Deliberately per adapter rather than a list of every host's variable checked -// in turn: those variables are inherited by child processes, so a host launched -// from another host's shell would key on the outer host and two different hosts -// could end up sharing one pointer file. Each adapter consults only the variable -// its own host sets. -// -// Process-global, because one process runs one host's adapter. A test that -// imported two adapters into the same process would have the second silently -// win; there is no such test, and there should not be one. -let hostPidProvider = null; - -export function configureHostPid(provider) { - if (provider !== null && typeof provider !== "function") { - throw new TypeError("The host pid provider must be a function or null."); - } - hostPidProvider = provider; -} - -function hostPid() { - if (!hostPidProvider) { - return null; - } - // An adapter reading a value the host never set must not take down the only - // path that can name this process's host. - try { - return hostPidProvider(); - } catch { - return null; - } -} - -// A host key names one directory entry, so it is held to the same rule as -// anything else that becomes a path segment. Exported because an adapter that -// wants to know whether a shared key is available has to ask the same question -// this does — a value accepted there and rejected here would claim a channel -// that was never opened. -export function normalizeHostKey(value) { - if (typeof value !== "string") { - return null; - } - const trimmed = value.trim(); - return trimmed.length > 0 && - trimmed !== "." && - trimmed !== ".." && - !trimmed.includes("/") && - !trimmed.includes("\\") - ? trimmed - : null; -} - -// The host process this process belongs to. -// -// NEATCONTEXT_HOST_KEY is the explicit form, for tests and for any host that can -// hand every one of its plugin processes the same identifier. Otherwise the pid -// the adapter knows how to find, and failing that `process.ppid`, which is the -// host for a server or hook it spawned directly. -export function hostKey() { - const explicit = process.env.NEATCONTEXT_HOST_KEY; - if (typeof explicit === "string") { - return normalizeHostKey(explicit); - } - return pidKey(hostPid()) ?? pidKey(process.ppid); -} - -export function hostsDirectory() { - return path.join(neatContextHome(), "plugin-hosts"); -} - -export function hostPointerPath(key) { - return path.join(hostsDirectory(), `${key}.json`); -} - -export function bridgePointerPath(key) { - return path.join(hostsDirectory(), `${key}.bridge.json`); -} - -async function readPointer(file) { - try { - const parsed = JSON.parse(await readFile(file, "utf8")); - const id = normalizeHostSessionId(parsed?.sessionId); - if (!id) { - return null; - } - const updatedAt = Date.parse(parsed?.updatedAt); - return { - sessionId: id, - source: typeof parsed?.source === "string" ? parsed.source : "unknown", - updatedAt: Number.isNaN(updatedAt) ? 0 : updatedAt, - pid: typeof parsed?.pid === "number" ? parsed.pid : null - }; - } catch { - // Missing, half-written, or hand-broken: the caller falls back to the - // environment, which is exactly the behavior that predates this file. - return null; - } -} - -let writeCounter = 0; - -// Written to a temporary name and renamed into place, because these files are -// read by other processes while they are being written: the bridge re-reads the -// pointer on every message, and a reader that catches a half-written file falls -// back to the environment, which is the stale answer this file exists to -// correct. A rename is atomic, so a reader sees the old record or the new one. -// -// The temporary name carries this process's pid so that two processes writing -// the same pointer at once cannot collide on it, and stays in the same directory -// so the rename never crosses a volume. -async function writePointer(file, value) { - await mkdir(path.dirname(file), { recursive: true }); - writeCounter += 1; - const temporary = `${file}.${process.pid}.${writeCounter}.tmp`; - try { - await writeFile(temporary, `${JSON.stringify(value, null, 2)}\n`, { - encoding: "utf8", - mode: 0o600 - }); - await rename(temporary, file); - } catch (error) { - await rm(temporary, { force: true }).catch(() => undefined); - throw error; - } -} - -export async function readHostPointer(key = hostKey()) { - return key ? readPointer(hostPointerPath(key)) : null; -} - -// Called by every process the host hands a session id to: the hooks, which are -// told on stdin, and the CLI, whose environment is fresh because it was spawned -// for this command. Last write wins — they are all reporting the same fact, and -// a hook runs at the end of every turn, so a wrong one cannot persist. -// -// Returns the id actually recorded, or null when there was nothing to record — -// no host key, or an id the host did not supply. -export async function writeHostPointer(rawId, { source = "unknown", key = hostKey() } = {}) { - const id = normalizeHostSessionId(rawId); - if (!id || !key) { - return null; - } - try { - await writePointer(hostPointerPath(key), { - sessionId: id, - source, - pid: process.pid, - updatedAt: new Date().toISOString() - }); - return id; - } catch { - // Recording this is never worth failing the command that triggered it: the - // caller keeps working against the environment, as it did before. - return null; - } -} - -// The session id to use, given what this process was started with. -// -// The pointer wins when the two disagree, because by construction it is the -// newer of the two: the environment was frozen when this process started and the -// pointer was written by a process that started later. -// -// The one case where that reasoning fails is a leftover pointer from a host that -// used to have this pid. It cannot describe a change that happened after this -// process started, so a *disagreeing* pointer older than this process is ignored. -// A real session change is always newer than the bridge that is reading it. -export async function resolveHostSessionId(envSessionId, { since = 0, key = hostKey() } = {}) { - const fallback = normalizeHostSessionId(envSessionId) ?? envSessionId ?? null; - const pointer = await readHostPointer(key); - if (!pointer || pointer.sessionId === envSessionId) { - return fallback; - } - if (since > 0 && pointer.updatedAt < since) { - return fallback; - } - return pointer.sessionId; -} - -// What the bridge resolved, published so a slash command can check its success -// against the process that will actually serve it rather than against its own -// write. -// -// Written when the answer changes and not on a timer: the bridge re-resolves -// several times a second and a file that says the same thing is not worth -// rewriting. Age is not staleness here — a record from an hour ago is still what -// this bridge is serving. Whether it is *running* is a question about its pid, -// which the record carries. -// -// The cache is what this process last wrote, not what is on disk. Something that -// deleted the file from underneath it would not get it back until the session -// changes; nothing here does that, and defending against an outside `rm` would -// cost a stat on every message. -let lastPublished = { id: undefined }; - -export async function publishBridgeSession(id, { key = hostKey(), now = Date.now() } = {}) { - if (!key) { - return false; - } - if (lastPublished.id === id) { - return false; - } - try { - await writePointer(bridgePointerPath(key), { - pid: process.pid, - sessionId: id ?? null, - updatedAt: new Date(now).toISOString() - }); - // Only once it is actually on disk: a write that failed has published - // nothing, and must not stop the next attempt. - lastPublished = { id }; - return true; - } catch { - return false; - } -} - -// Only a record left by a bridge that is still running says anything about what -// this session is being served right now. -export async function readBridgeSession(key = hostKey(), { alive = isProcessAlive } = {}) { - const record = key ? await readPointer(bridgePointerPath(key)) : null; - if (!record || record.pid === null || !alive(record.pid)) { - return null; - } - return record; -} - -// Waits for the bridge to be serving `id`, and reports what it found rather than -// deciding what to do about it: the caller is a command whose success message -// depends on the answer. -// -// `state` is "matched" (the bridge is on this session), "unknown" (no bridge is -// publishing, so there is nothing to check against — an older bridge, or none -// running at all), or "drifted". -export async function awaitBridgeSession( - id, - { timeoutMs = 3000, intervalMs = 100, key = hostKey() } = {} -) { - if (!id || !key) { - return { state: "unknown" }; - } - const deadline = Date.now() + timeoutMs; - let seen = null; - for (;;) { - seen = await readBridgeSession(key); - if (seen?.sessionId === id) { - return { state: "matched", seen }; - } - // Nothing is publishing: a bridge from before this existed, or none running. - // There is no answer coming, so waiting for one only delays the command. - if (!seen || Date.now() >= deadline) { - break; - } - await new Promise((resolve) => setTimeout(resolve, intervalMs)); - } - return seen ? { state: "drifted", seen } : { state: "unknown" }; -} - -// Pointers name a process, and processes end. Sweeping them keeps a machine that -// opens a lot of windows from accumulating a file per pid forever, and — more to -// the point — keeps a recycled pid from finding an ancient pointer waiting for -// it. Best effort: a failed sweep is not a failed command. -export async function pruneHostPointers({ alive = isProcessAlive } = {}) { - let entries; - try { - entries = await readdir(hostsDirectory()); - } catch { - return 0; - } - const mine = hostKey(); - let removed = 0; - for (const entry of entries) { - const key = entry.replace(/\.bridge\.json$|\.json$/, ""); - if (key === entry || key === mine) { - continue; - } - const pid = /^pid-([1-9][0-9]{0,9})$/.exec(key); - if (!pid || alive(Number(pid[1]))) { - continue; - } - await rm(path.join(hostsDirectory(), entry), { force: true }).catch(() => undefined); - removed += 1; - } - return removed; -} - -export function isProcessAlive(pid) { - try { - // Signal 0 checks for the process without touching it. EPERM means it exists - // and belongs to someone else, which still counts as alive. - process.kill(pid, 0); - return true; - } catch (error) { - return error?.code === "EPERM"; - } -} diff --git a/plugins/pi/neatcontext/package.json b/plugins/pi/neatcontext/package.json index d05915a..41ea3a9 100644 --- a/plugins/pi/neatcontext/package.json +++ b/plugins/pi/neatcontext/package.json @@ -46,7 +46,7 @@ "access": "public" }, "scripts": { - "check": "node --check extensions/neatcontext.js && node --check src/pi/runtime.mjs && node --check src/pi/session.mjs && node --check src/core/context-store.mjs && node --check src/core/extension-bindings.mjs && node --check src/core/extension-commands.mjs && node --check src/core/extension-runtime.mjs && node --check src/core/extensions.mjs && node --check src/core/mcp-stdio-client.mjs && node --check src/core/local-state.mjs && node --check src/core/host-session.mjs && node --check src/core/conversation-evidence.mjs && node --check src/core/routing.mjs && node --check src/core/routing-search.mjs && node --check src/core/routing-candidates.mjs && node --check src/core/selection.mjs && node --check src/core/session.mjs && node --check src/core/storage-home.mjs", + "check": "node --check extensions/neatcontext.js && node --check src/pi/runtime.mjs && node --check src/pi/session.mjs && node --check src/core/context-store.mjs && node --check src/core/extension-bindings.mjs && node --check src/core/extension-commands.mjs && node --check src/core/extension-runtime.mjs && node --check src/core/extensions.mjs && node --check src/core/mcp-stdio-client.mjs && node --check src/core/local-state.mjs && node --check src/core/conversation-evidence.mjs && node --check src/core/routing.mjs && node --check src/core/routing-search.mjs && node --check src/core/routing-candidates.mjs && node --check src/core/selection.mjs && node --check src/core/session.mjs && node --check src/core/storage-home.mjs", "test": "node --test" } } diff --git a/plugins/pi/neatcontext/src/core/host-session.mjs b/plugins/pi/neatcontext/src/core/host-session.mjs deleted file mode 100644 index e576d78..0000000 --- a/plugins/pi/neatcontext/src/core/host-session.mjs +++ /dev/null @@ -1,360 +0,0 @@ -// Which session the host process is on *right now*. -// -// A host that identifies its sessions through the environment has a problem the -// rest of this plugin cannot see: the environment is a snapshot. Hosts spawn the -// MCP bridge once and keep it for the life of the window, but the session -// changes underneath it — starting a fresh conversation does not restart the -// server — and the bridge's copy of the id is whatever it was at spawn. The -// hooks and the slash-command CLI are spawned fresh and see the new one. -// -// So the two halves of the plugin end up reading and writing different files: -// `use` writes the selection for the session the user is in, the bridge keeps -// serving the session the window started in, and nothing in either path can -// observe the disagreement. The user is told a context is connected and every -// later answer is grounded in a different one. -// -// This module is the missing channel. One pointer file per host *process*, -// written by whichever short-lived process was just handed the current session -// id, and re-read by the long-lived one on every message: -// -// ~/.neatcontext/plugin-hosts/.json { sessionId, source, updatedAt } -// -// The host key has to be stable across a session change and distinct per window, -// which rules out the session id and the working directory both. What is left is -// the host process itself. How to name it is the one thing that differs per -// host, so each adapter registers its own answer with `configureHostPid()`: some -// hosts publish their own pid to every process they spawn, and a host that -// publishes nothing falls back to `process.ppid`, which is the host for any -// process it spawned directly. A process whose parent is not the host and whose -// host publishes no pid — a CLI running inside a shell tool — computes a key -// nothing else reads, and its writes are harmless and swept later. -// -// The bridge also publishes what it actually resolved: -// -// ~/.neatcontext/plugin-hosts/.bridge.json { pid, sessionId, updatedAt } -// -// That is what lets `use` check its own success against the process that will -// serve it, instead of against the file it just wrote itself. - -import { mkdir, readFile, readdir, rename, rm, writeFile } from "node:fs/promises"; -import path from "node:path"; -import { neatContextHome } from "./storage-home.mjs"; - -// A session id becomes a path segment (`plugin-sessions/.json`), and unlike -// the environment — which only the host can set — this one arrives from a file. -// Anything that could climb out of the 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 normalizeHostSessionId(value) { - if (typeof value !== "string") { - return null; - } - const id = value.trim(); - if (id === "." || id === ".." || !SAFE_SESSION_ID.test(id)) { - return null; - } - return id; -} - -function pidKey(value) { - const pid = typeof value === "string" ? value.trim() : String(value ?? ""); - return /^[1-9][0-9]{0,9}$/.test(pid) ? `pid-${pid}` : null; -} - -// Where the host publishes its own process id, when it publishes one at all. -// -// Deliberately per adapter rather than a list of every host's variable checked -// in turn: those variables are inherited by child processes, so a host launched -// from another host's shell would key on the outer host and two different hosts -// could end up sharing one pointer file. Each adapter consults only the variable -// its own host sets. -// -// Process-global, because one process runs one host's adapter. A test that -// imported two adapters into the same process would have the second silently -// win; there is no such test, and there should not be one. -let hostPidProvider = null; - -export function configureHostPid(provider) { - if (provider !== null && typeof provider !== "function") { - throw new TypeError("The host pid provider must be a function or null."); - } - hostPidProvider = provider; -} - -function hostPid() { - if (!hostPidProvider) { - return null; - } - // An adapter reading a value the host never set must not take down the only - // path that can name this process's host. - try { - return hostPidProvider(); - } catch { - return null; - } -} - -// A host key names one directory entry, so it is held to the same rule as -// anything else that becomes a path segment. Exported because an adapter that -// wants to know whether a shared key is available has to ask the same question -// this does — a value accepted there and rejected here would claim a channel -// that was never opened. -export function normalizeHostKey(value) { - if (typeof value !== "string") { - return null; - } - const trimmed = value.trim(); - return trimmed.length > 0 && - trimmed !== "." && - trimmed !== ".." && - !trimmed.includes("/") && - !trimmed.includes("\\") - ? trimmed - : null; -} - -// The host process this process belongs to. -// -// NEATCONTEXT_HOST_KEY is the explicit form, for tests and for any host that can -// hand every one of its plugin processes the same identifier. Otherwise the pid -// the adapter knows how to find, and failing that `process.ppid`, which is the -// host for a server or hook it spawned directly. -export function hostKey() { - const explicit = process.env.NEATCONTEXT_HOST_KEY; - if (typeof explicit === "string") { - return normalizeHostKey(explicit); - } - return pidKey(hostPid()) ?? pidKey(process.ppid); -} - -export function hostsDirectory() { - return path.join(neatContextHome(), "plugin-hosts"); -} - -export function hostPointerPath(key) { - return path.join(hostsDirectory(), `${key}.json`); -} - -export function bridgePointerPath(key) { - return path.join(hostsDirectory(), `${key}.bridge.json`); -} - -async function readPointer(file) { - try { - const parsed = JSON.parse(await readFile(file, "utf8")); - const id = normalizeHostSessionId(parsed?.sessionId); - if (!id) { - return null; - } - const updatedAt = Date.parse(parsed?.updatedAt); - return { - sessionId: id, - source: typeof parsed?.source === "string" ? parsed.source : "unknown", - updatedAt: Number.isNaN(updatedAt) ? 0 : updatedAt, - pid: typeof parsed?.pid === "number" ? parsed.pid : null - }; - } catch { - // Missing, half-written, or hand-broken: the caller falls back to the - // environment, which is exactly the behavior that predates this file. - return null; - } -} - -let writeCounter = 0; - -// Written to a temporary name and renamed into place, because these files are -// read by other processes while they are being written: the bridge re-reads the -// pointer on every message, and a reader that catches a half-written file falls -// back to the environment, which is the stale answer this file exists to -// correct. A rename is atomic, so a reader sees the old record or the new one. -// -// The temporary name carries this process's pid so that two processes writing -// the same pointer at once cannot collide on it, and stays in the same directory -// so the rename never crosses a volume. -async function writePointer(file, value) { - await mkdir(path.dirname(file), { recursive: true }); - writeCounter += 1; - const temporary = `${file}.${process.pid}.${writeCounter}.tmp`; - try { - await writeFile(temporary, `${JSON.stringify(value, null, 2)}\n`, { - encoding: "utf8", - mode: 0o600 - }); - await rename(temporary, file); - } catch (error) { - await rm(temporary, { force: true }).catch(() => undefined); - throw error; - } -} - -export async function readHostPointer(key = hostKey()) { - return key ? readPointer(hostPointerPath(key)) : null; -} - -// Called by every process the host hands a session id to: the hooks, which are -// told on stdin, and the CLI, whose environment is fresh because it was spawned -// for this command. Last write wins — they are all reporting the same fact, and -// a hook runs at the end of every turn, so a wrong one cannot persist. -// -// Returns the id actually recorded, or null when there was nothing to record — -// no host key, or an id the host did not supply. -export async function writeHostPointer(rawId, { source = "unknown", key = hostKey() } = {}) { - const id = normalizeHostSessionId(rawId); - if (!id || !key) { - return null; - } - try { - await writePointer(hostPointerPath(key), { - sessionId: id, - source, - pid: process.pid, - updatedAt: new Date().toISOString() - }); - return id; - } catch { - // Recording this is never worth failing the command that triggered it: the - // caller keeps working against the environment, as it did before. - return null; - } -} - -// The session id to use, given what this process was started with. -// -// The pointer wins when the two disagree, because by construction it is the -// newer of the two: the environment was frozen when this process started and the -// pointer was written by a process that started later. -// -// The one case where that reasoning fails is a leftover pointer from a host that -// used to have this pid. It cannot describe a change that happened after this -// process started, so a *disagreeing* pointer older than this process is ignored. -// A real session change is always newer than the bridge that is reading it. -export async function resolveHostSessionId(envSessionId, { since = 0, key = hostKey() } = {}) { - const fallback = normalizeHostSessionId(envSessionId) ?? envSessionId ?? null; - const pointer = await readHostPointer(key); - if (!pointer || pointer.sessionId === envSessionId) { - return fallback; - } - if (since > 0 && pointer.updatedAt < since) { - return fallback; - } - return pointer.sessionId; -} - -// What the bridge resolved, published so a slash command can check its success -// against the process that will actually serve it rather than against its own -// write. -// -// Written when the answer changes and not on a timer: the bridge re-resolves -// several times a second and a file that says the same thing is not worth -// rewriting. Age is not staleness here — a record from an hour ago is still what -// this bridge is serving. Whether it is *running* is a question about its pid, -// which the record carries. -// -// The cache is what this process last wrote, not what is on disk. Something that -// deleted the file from underneath it would not get it back until the session -// changes; nothing here does that, and defending against an outside `rm` would -// cost a stat on every message. -let lastPublished = { id: undefined }; - -export async function publishBridgeSession(id, { key = hostKey(), now = Date.now() } = {}) { - if (!key) { - return false; - } - if (lastPublished.id === id) { - return false; - } - try { - await writePointer(bridgePointerPath(key), { - pid: process.pid, - sessionId: id ?? null, - updatedAt: new Date(now).toISOString() - }); - // Only once it is actually on disk: a write that failed has published - // nothing, and must not stop the next attempt. - lastPublished = { id }; - return true; - } catch { - return false; - } -} - -// Only a record left by a bridge that is still running says anything about what -// this session is being served right now. -export async function readBridgeSession(key = hostKey(), { alive = isProcessAlive } = {}) { - const record = key ? await readPointer(bridgePointerPath(key)) : null; - if (!record || record.pid === null || !alive(record.pid)) { - return null; - } - return record; -} - -// Waits for the bridge to be serving `id`, and reports what it found rather than -// deciding what to do about it: the caller is a command whose success message -// depends on the answer. -// -// `state` is "matched" (the bridge is on this session), "unknown" (no bridge is -// publishing, so there is nothing to check against — an older bridge, or none -// running at all), or "drifted". -export async function awaitBridgeSession( - id, - { timeoutMs = 3000, intervalMs = 100, key = hostKey() } = {} -) { - if (!id || !key) { - return { state: "unknown" }; - } - const deadline = Date.now() + timeoutMs; - let seen = null; - for (;;) { - seen = await readBridgeSession(key); - if (seen?.sessionId === id) { - return { state: "matched", seen }; - } - // Nothing is publishing: a bridge from before this existed, or none running. - // There is no answer coming, so waiting for one only delays the command. - if (!seen || Date.now() >= deadline) { - break; - } - await new Promise((resolve) => setTimeout(resolve, intervalMs)); - } - return seen ? { state: "drifted", seen } : { state: "unknown" }; -} - -// Pointers name a process, and processes end. Sweeping them keeps a machine that -// opens a lot of windows from accumulating a file per pid forever, and — more to -// the point — keeps a recycled pid from finding an ancient pointer waiting for -// it. Best effort: a failed sweep is not a failed command. -export async function pruneHostPointers({ alive = isProcessAlive } = {}) { - let entries; - try { - entries = await readdir(hostsDirectory()); - } catch { - return 0; - } - const mine = hostKey(); - let removed = 0; - for (const entry of entries) { - const key = entry.replace(/\.bridge\.json$|\.json$/, ""); - if (key === entry || key === mine) { - continue; - } - const pid = /^pid-([1-9][0-9]{0,9})$/.exec(key); - if (!pid || alive(Number(pid[1]))) { - continue; - } - await rm(path.join(hostsDirectory(), entry), { force: true }).catch(() => undefined); - removed += 1; - } - return removed; -} - -export function isProcessAlive(pid) { - try { - // Signal 0 checks for the process without touching it. EPERM means it exists - // and belongs to someone else, which still counts as alive. - process.kill(pid, 0); - return true; - } catch (error) { - return error?.code === "EPERM"; - } -} diff --git a/shared/core/host-session.mjs b/shared/core/host-session.mjs deleted file mode 100644 index e576d78..0000000 --- a/shared/core/host-session.mjs +++ /dev/null @@ -1,360 +0,0 @@ -// Which session the host process is on *right now*. -// -// A host that identifies its sessions through the environment has a problem the -// rest of this plugin cannot see: the environment is a snapshot. Hosts spawn the -// MCP bridge once and keep it for the life of the window, but the session -// changes underneath it — starting a fresh conversation does not restart the -// server — and the bridge's copy of the id is whatever it was at spawn. The -// hooks and the slash-command CLI are spawned fresh and see the new one. -// -// So the two halves of the plugin end up reading and writing different files: -// `use` writes the selection for the session the user is in, the bridge keeps -// serving the session the window started in, and nothing in either path can -// observe the disagreement. The user is told a context is connected and every -// later answer is grounded in a different one. -// -// This module is the missing channel. One pointer file per host *process*, -// written by whichever short-lived process was just handed the current session -// id, and re-read by the long-lived one on every message: -// -// ~/.neatcontext/plugin-hosts/.json { sessionId, source, updatedAt } -// -// The host key has to be stable across a session change and distinct per window, -// which rules out the session id and the working directory both. What is left is -// the host process itself. How to name it is the one thing that differs per -// host, so each adapter registers its own answer with `configureHostPid()`: some -// hosts publish their own pid to every process they spawn, and a host that -// publishes nothing falls back to `process.ppid`, which is the host for any -// process it spawned directly. A process whose parent is not the host and whose -// host publishes no pid — a CLI running inside a shell tool — computes a key -// nothing else reads, and its writes are harmless and swept later. -// -// The bridge also publishes what it actually resolved: -// -// ~/.neatcontext/plugin-hosts/.bridge.json { pid, sessionId, updatedAt } -// -// That is what lets `use` check its own success against the process that will -// serve it, instead of against the file it just wrote itself. - -import { mkdir, readFile, readdir, rename, rm, writeFile } from "node:fs/promises"; -import path from "node:path"; -import { neatContextHome } from "./storage-home.mjs"; - -// A session id becomes a path segment (`plugin-sessions/.json`), and unlike -// the environment — which only the host can set — this one arrives from a file. -// Anything that could climb out of the 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 normalizeHostSessionId(value) { - if (typeof value !== "string") { - return null; - } - const id = value.trim(); - if (id === "." || id === ".." || !SAFE_SESSION_ID.test(id)) { - return null; - } - return id; -} - -function pidKey(value) { - const pid = typeof value === "string" ? value.trim() : String(value ?? ""); - return /^[1-9][0-9]{0,9}$/.test(pid) ? `pid-${pid}` : null; -} - -// Where the host publishes its own process id, when it publishes one at all. -// -// Deliberately per adapter rather than a list of every host's variable checked -// in turn: those variables are inherited by child processes, so a host launched -// from another host's shell would key on the outer host and two different hosts -// could end up sharing one pointer file. Each adapter consults only the variable -// its own host sets. -// -// Process-global, because one process runs one host's adapter. A test that -// imported two adapters into the same process would have the second silently -// win; there is no such test, and there should not be one. -let hostPidProvider = null; - -export function configureHostPid(provider) { - if (provider !== null && typeof provider !== "function") { - throw new TypeError("The host pid provider must be a function or null."); - } - hostPidProvider = provider; -} - -function hostPid() { - if (!hostPidProvider) { - return null; - } - // An adapter reading a value the host never set must not take down the only - // path that can name this process's host. - try { - return hostPidProvider(); - } catch { - return null; - } -} - -// A host key names one directory entry, so it is held to the same rule as -// anything else that becomes a path segment. Exported because an adapter that -// wants to know whether a shared key is available has to ask the same question -// this does — a value accepted there and rejected here would claim a channel -// that was never opened. -export function normalizeHostKey(value) { - if (typeof value !== "string") { - return null; - } - const trimmed = value.trim(); - return trimmed.length > 0 && - trimmed !== "." && - trimmed !== ".." && - !trimmed.includes("/") && - !trimmed.includes("\\") - ? trimmed - : null; -} - -// The host process this process belongs to. -// -// NEATCONTEXT_HOST_KEY is the explicit form, for tests and for any host that can -// hand every one of its plugin processes the same identifier. Otherwise the pid -// the adapter knows how to find, and failing that `process.ppid`, which is the -// host for a server or hook it spawned directly. -export function hostKey() { - const explicit = process.env.NEATCONTEXT_HOST_KEY; - if (typeof explicit === "string") { - return normalizeHostKey(explicit); - } - return pidKey(hostPid()) ?? pidKey(process.ppid); -} - -export function hostsDirectory() { - return path.join(neatContextHome(), "plugin-hosts"); -} - -export function hostPointerPath(key) { - return path.join(hostsDirectory(), `${key}.json`); -} - -export function bridgePointerPath(key) { - return path.join(hostsDirectory(), `${key}.bridge.json`); -} - -async function readPointer(file) { - try { - const parsed = JSON.parse(await readFile(file, "utf8")); - const id = normalizeHostSessionId(parsed?.sessionId); - if (!id) { - return null; - } - const updatedAt = Date.parse(parsed?.updatedAt); - return { - sessionId: id, - source: typeof parsed?.source === "string" ? parsed.source : "unknown", - updatedAt: Number.isNaN(updatedAt) ? 0 : updatedAt, - pid: typeof parsed?.pid === "number" ? parsed.pid : null - }; - } catch { - // Missing, half-written, or hand-broken: the caller falls back to the - // environment, which is exactly the behavior that predates this file. - return null; - } -} - -let writeCounter = 0; - -// Written to a temporary name and renamed into place, because these files are -// read by other processes while they are being written: the bridge re-reads the -// pointer on every message, and a reader that catches a half-written file falls -// back to the environment, which is the stale answer this file exists to -// correct. A rename is atomic, so a reader sees the old record or the new one. -// -// The temporary name carries this process's pid so that two processes writing -// the same pointer at once cannot collide on it, and stays in the same directory -// so the rename never crosses a volume. -async function writePointer(file, value) { - await mkdir(path.dirname(file), { recursive: true }); - writeCounter += 1; - const temporary = `${file}.${process.pid}.${writeCounter}.tmp`; - try { - await writeFile(temporary, `${JSON.stringify(value, null, 2)}\n`, { - encoding: "utf8", - mode: 0o600 - }); - await rename(temporary, file); - } catch (error) { - await rm(temporary, { force: true }).catch(() => undefined); - throw error; - } -} - -export async function readHostPointer(key = hostKey()) { - return key ? readPointer(hostPointerPath(key)) : null; -} - -// Called by every process the host hands a session id to: the hooks, which are -// told on stdin, and the CLI, whose environment is fresh because it was spawned -// for this command. Last write wins — they are all reporting the same fact, and -// a hook runs at the end of every turn, so a wrong one cannot persist. -// -// Returns the id actually recorded, or null when there was nothing to record — -// no host key, or an id the host did not supply. -export async function writeHostPointer(rawId, { source = "unknown", key = hostKey() } = {}) { - const id = normalizeHostSessionId(rawId); - if (!id || !key) { - return null; - } - try { - await writePointer(hostPointerPath(key), { - sessionId: id, - source, - pid: process.pid, - updatedAt: new Date().toISOString() - }); - return id; - } catch { - // Recording this is never worth failing the command that triggered it: the - // caller keeps working against the environment, as it did before. - return null; - } -} - -// The session id to use, given what this process was started with. -// -// The pointer wins when the two disagree, because by construction it is the -// newer of the two: the environment was frozen when this process started and the -// pointer was written by a process that started later. -// -// The one case where that reasoning fails is a leftover pointer from a host that -// used to have this pid. It cannot describe a change that happened after this -// process started, so a *disagreeing* pointer older than this process is ignored. -// A real session change is always newer than the bridge that is reading it. -export async function resolveHostSessionId(envSessionId, { since = 0, key = hostKey() } = {}) { - const fallback = normalizeHostSessionId(envSessionId) ?? envSessionId ?? null; - const pointer = await readHostPointer(key); - if (!pointer || pointer.sessionId === envSessionId) { - return fallback; - } - if (since > 0 && pointer.updatedAt < since) { - return fallback; - } - return pointer.sessionId; -} - -// What the bridge resolved, published so a slash command can check its success -// against the process that will actually serve it rather than against its own -// write. -// -// Written when the answer changes and not on a timer: the bridge re-resolves -// several times a second and a file that says the same thing is not worth -// rewriting. Age is not staleness here — a record from an hour ago is still what -// this bridge is serving. Whether it is *running* is a question about its pid, -// which the record carries. -// -// The cache is what this process last wrote, not what is on disk. Something that -// deleted the file from underneath it would not get it back until the session -// changes; nothing here does that, and defending against an outside `rm` would -// cost a stat on every message. -let lastPublished = { id: undefined }; - -export async function publishBridgeSession(id, { key = hostKey(), now = Date.now() } = {}) { - if (!key) { - return false; - } - if (lastPublished.id === id) { - return false; - } - try { - await writePointer(bridgePointerPath(key), { - pid: process.pid, - sessionId: id ?? null, - updatedAt: new Date(now).toISOString() - }); - // Only once it is actually on disk: a write that failed has published - // nothing, and must not stop the next attempt. - lastPublished = { id }; - return true; - } catch { - return false; - } -} - -// Only a record left by a bridge that is still running says anything about what -// this session is being served right now. -export async function readBridgeSession(key = hostKey(), { alive = isProcessAlive } = {}) { - const record = key ? await readPointer(bridgePointerPath(key)) : null; - if (!record || record.pid === null || !alive(record.pid)) { - return null; - } - return record; -} - -// Waits for the bridge to be serving `id`, and reports what it found rather than -// deciding what to do about it: the caller is a command whose success message -// depends on the answer. -// -// `state` is "matched" (the bridge is on this session), "unknown" (no bridge is -// publishing, so there is nothing to check against — an older bridge, or none -// running at all), or "drifted". -export async function awaitBridgeSession( - id, - { timeoutMs = 3000, intervalMs = 100, key = hostKey() } = {} -) { - if (!id || !key) { - return { state: "unknown" }; - } - const deadline = Date.now() + timeoutMs; - let seen = null; - for (;;) { - seen = await readBridgeSession(key); - if (seen?.sessionId === id) { - return { state: "matched", seen }; - } - // Nothing is publishing: a bridge from before this existed, or none running. - // There is no answer coming, so waiting for one only delays the command. - if (!seen || Date.now() >= deadline) { - break; - } - await new Promise((resolve) => setTimeout(resolve, intervalMs)); - } - return seen ? { state: "drifted", seen } : { state: "unknown" }; -} - -// Pointers name a process, and processes end. Sweeping them keeps a machine that -// opens a lot of windows from accumulating a file per pid forever, and — more to -// the point — keeps a recycled pid from finding an ancient pointer waiting for -// it. Best effort: a failed sweep is not a failed command. -export async function pruneHostPointers({ alive = isProcessAlive } = {}) { - let entries; - try { - entries = await readdir(hostsDirectory()); - } catch { - return 0; - } - const mine = hostKey(); - let removed = 0; - for (const entry of entries) { - const key = entry.replace(/\.bridge\.json$|\.json$/, ""); - if (key === entry || key === mine) { - continue; - } - const pid = /^pid-([1-9][0-9]{0,9})$/.exec(key); - if (!pid || alive(Number(pid[1]))) { - continue; - } - await rm(path.join(hostsDirectory(), entry), { force: true }).catch(() => undefined); - removed += 1; - } - return removed; -} - -export function isProcessAlive(pid) { - try { - // Signal 0 checks for the process without touching it. EPERM means it exists - // and belongs to someone else, which still counts as alive. - process.kill(pid, 0); - return true; - } catch (error) { - return error?.code === "EPERM"; - } -} diff --git a/tests/copilot-plugin.test.mjs b/tests/copilot-plugin.test.mjs index ec966d1..18c1c59 100644 --- a/tests/copilot-plugin.test.mjs +++ b/tests/copilot-plugin.test.mjs @@ -2,7 +2,8 @@ // reused verbatim, commands ported, and a local Context adapter in src/copilot. // These tests pin the fork's contracts: what must stay byte-identical to // claude-code, how -// sessions scope to workspaces on hosts that expose no session identity, and +// sessions use the identity Copilot exposes, fall back to workspaces when a +// host exposes none, and // that nothing here runs on its own. import assert from "node:assert/strict"; @@ -186,23 +187,13 @@ function toolCall(id, name, args = {}) { } // One isolated NeatContext home per test. -// -// The host variables are blanked rather than inherited: this suite runs *inside* -// a Copilot session on a developer machine, and a real COPILOT_AGENT_SESSION_ID -// leaking into every child would silently replace the identity under test. An -// empty value is "not set" everywhere it is read, so each test exercises the -// same fallbacks a clean host would. The host key is pinned per home so the -// pointer files of two tests running concurrently cannot collide. async function isolatedHome(prefix) { const directory = await mkdtemp(path.join(os.tmpdir(), prefix)); return { directory, env: { NEATCONTEXT_HOME: directory, - NEATCONTEXT_SESSION_ID: "", - COPILOT_AGENT_SESSION_ID: "", - COPILOT_LOADER_PID: "", - NEATCONTEXT_HOST_KEY: `test-${path.basename(directory)}` + COPILOT_AGENT_SESSION_ID: "" } }; } @@ -571,7 +562,11 @@ test("Copilot sessions scope to the workspace when no session id is provided", a const workspaceB = await mkdtemp(path.join(os.tmpdir(), "copilot-ws-b-")); // An empty override is "not set": the runs below must fall through to the // workspace digest even when the test runner's own environment carries ids. - const wsEnv = { ...home.env, NEATCONTEXT_SESSION_ID: "" }; + const wsEnv = { + ...home.env, + NEATCONTEXT_SESSION_ID: "", + COPILOT_AGENT_SESSION_ID: "" + }; await createContext(home, "workspace scoped"); @@ -595,6 +590,45 @@ test("Copilot sessions scope to the workspace when no session id is provided", a assert.match(modeB.stdout, /auto \(the default\)/); }); +test("Copilot CLI and MCP bridge share the host session across working directories", async (t) => { + const home = await isolatedHome("neatcontext-copilot-session-"); + const commandWorkspace = await mkdtemp(path.join(os.tmpdir(), "copilot-command-ws-")); + const bridgeWorkspace = await mkdtemp(path.join(os.tmpdir(), "copilot-bridge-ws-")); + const sessions = []; + t.after(async () => { + await Promise.all(sessions.map((session) => session.close())); + await Promise.all( + [home.directory, commandWorkspace, bridgeWorkspace].map((directory) => + rm(directory, { recursive: true, force: true }) + ) + ); + }); + const env = { + ...home.env, + NEATCONTEXT_SESSION_ID: "", + COPILOT_AGENT_SESSION_ID: "copilot-agent-session-a" + }; + + await createContext(home, "shared session"); + const connect = await runNode(cli, ["use", "shared session"], { + env, + cwd: commandWorkspace + }); + assert.match(connect.stdout, /Connected the "shared session" context/); + + const session = rpcSession(env, { cwd: bridgeWorkspace }); + sessions.push(session); + await session.call(initialize(1)); + const grounded = await session.call(toolCall(2, "get_context")); + assert.match(grounded.result.content[0].text, /connected context: shared session/i); + + const otherSession = await runNode(cli, ["status"], { + env: { ...env, COPILOT_AGENT_SESSION_ID: "copilot-agent-session-b" }, + cwd: commandWorkspace + }); + assert.match(otherSession.stdout, /No context is connected yet/); +}); + test("Copilot MCP bridge serves Contexts and routing locally", async (t) => { const home = await isolatedHome("neatcontext-copilot-mcp-"); const sessions = []; diff --git a/tests/copilot-session-drift.test.mjs b/tests/copilot-session-drift.test.mjs deleted file mode 100644 index edeafff..0000000 --- a/tests/copilot-session-drift.test.mjs +++ /dev/null @@ -1,501 +0,0 @@ -// Regression tests for the reported failure on GitHub Copilot: `/neatcontext:use` -// reports a context connected while `get_context` keeps answering from a -// different one — with no error anywhere, because both halves are telling the -// truth about different files. -// -// Copilot's two plugin processes are not given the same working directory. The -// MCP bridge is spawned with the *plugin installation* directory; the CLI a -// slash command runs is spawned with the user's workspace. The adapter derived -// the session by hashing `process.cwd()`, so the two halves hashed different -// paths and scoped to different selection files. -// -// What is reproduced here is exactly that split, and nothing else: one bridge -// process with the plugin directory as its cwd, CLI processes with the workspace -// as theirs. Copilot ships no hooks, so the CLI is the only process that can -// ever tell the bridge what session the window is on. - -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"; -import { workspaceSessionId } from "../plugins/copilot/neatcontext/src/copilot/session.mjs"; - -const plugin = path.join( - path.dirname(fileURLToPath(import.meta.url)), - "..", - "plugins", - "copilot", - "neatcontext" -); -const copilot = path.join(plugin, "src", "copilot"); - -const HOST = "copilot-window"; -const OTHER_HOST = "copilot-window-2"; - -let home; -let workspace; -let otherWorkspace; -let hostsDirectory; - -// The environment Copilot hands a plugin process. `NEATCONTEXT_HOST_KEY` stands -// in for COPILOT_LOADER_PID so a test can say which window a process belongs to -// without depending on the real process tree. -function childEnv({ sessionId, host = HOST } = {}) { - return { - ...process.env, - // Empty is "not set" everywhere it is read, so a test that omits the session - // id exercises the workspace fallback even though this suite is itself run - // from a Copilot session whose id would otherwise be inherited. - COPILOT_AGENT_SESSION_ID: sessionId ?? "", - COPILOT_LOADER_PID: "", - NEATCONTEXT_SESSION_ID: "", - NEATCONTEXT_HOST_KEY: host, - NEATCONTEXT_HOME: home - }; -} - -before(async () => { - home = await mkdtemp(path.join(os.tmpdir(), "neatcontext-copilot-drift-")); - workspace = await mkdtemp(path.join(os.tmpdir(), "copilot-workspace-")); - otherWorkspace = await mkdtemp(path.join(os.tmpdir(), "copilot-workspace-b-")); - 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("../plugins/copilot/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 () => { - for (const directory of [home, workspace, otherWorkspace]) { - await rm(directory, { 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 }); -}); - -// A slash command. Copilot spawns it in the user's workspace — which is the -// directory the bridge is *not* given. -function cli(args, { sessionId, host = HOST, cwd = workspace } = {}) { - return new Promise((resolve) => { - const child = spawn( - process.execPath, - [path.join(copilot, "neatcontext-cli.mjs"), ...args], - { cwd, stdio: ["ignore", "pipe", "inherit"], env: childEnv({ sessionId, host }) } - ); - let out = ""; - child.stdout.on("data", (chunk) => (out += chunk)); - child.on("exit", () => resolve(out.trim())); - }); -} - -// A window: one bridge kept alive for its lifetime, spawned the way Copilot -// spawns it — with the plugin installation directory as its working directory, -// not the workspace. -function openWindow({ sessionId, host = HOST, cwd = plugin } = {}) { - const child = spawn(process.execPath, [path.join(copilot, "mcp-bridge.mjs")], { - cwd, - stdio: ["pipe", "pipe", "inherit"], - env: childEnv({ sessionId, host }) - }); - const waiters = new Map(); - 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); - } - }); - 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, - 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) - }; -} - -async function writeBridgeRecord(sessionId, { pid = process.pid, host = HOST } = {}) { - await mkdir(hostsDirectory, { recursive: true }); - await writeFile( - path.join(hostsDirectory, `${host}.bridge.json`), - JSON.stringify({ pid, sessionId, updatedAt: new Date().toISOString() }) - ); -} - -describe("a bridge and a slash command that were given different directories", () => { - it("serves the context the slash command just connected", async () => { - // The reported bug, reproduced by the cwd difference alone: before the fix - // this connected one selection file and read another, and reported success. - const window = openWindow({ sessionId: "copilot-session-a" }); - try { - await window.handshake(); - assert.match( - await cli(["use", "payment team"], { sessionId: "copilot-session-a" }), - /Connected the "payment team" context/ - ); - assert.match(await window.grounding(), /connected context: payment team/i); - } finally { - await window.close(); - } - }); - - it("agrees even when the host publishes no session id at all", async () => { - // An older or different Copilot build: the workspace digest is all there is, - // and the bridge's cwd digest is not the workspace's. The pointer the CLI - // writes is what closes the gap. - const window = openWindow({}); - try { - await window.handshake(); - await cli(["use", "Dokploy"]); - assert.match(await window.grounding(), /connected context: Dokploy/i); - } finally { - await window.close(); - } - }); - - it("stops serving a context once the session is disconnected", async () => { - const window = openWindow({ sessionId: "copilot-session-a" }); - try { - await window.handshake(); - await cli(["use", "payment team"], { sessionId: "copilot-session-a" }); - assert.match(await window.grounding(), /connected context: payment team/i); - - await cli(["disconnect"], { sessionId: "copilot-session-a" }); - const answer = await window.grounding(); - assert.doesNotMatch(answer, /connected context: payment team/i); - assert.match(answer, /No NeatContext Context is connected to this session/); - } finally { - await window.close(); - } - }); -}); - -describe("a session that is replaced under a running bridge", () => { - it("follows the session the user is in now", async () => { - const window = openWindow({ sessionId: "copilot-session-a" }); - try { - await window.handshake(); - await cli(["use", "payment team"], { sessionId: "copilot-session-a" }); - assert.match(await window.grounding(), /connected context: payment team/i); - - // A new session in the same window. Copilot does not restart the bridge, - // so its own environment still says session-a. - assert.match( - await cli(["use", "Dokploy"], { sessionId: "copilot-session-b" }), - /Connected the "Dokploy" context/ - ); - const answer = await window.grounding(); - assert.match(answer, /connected context: Dokploy/i); - assert.doesNotMatch(answer, /connected context: payment team/i); - } finally { - await window.close(); - } - }); -}); - -describe("two windows", () => { - it("do not share a selection", async () => { - const first = openWindow({ sessionId: "copilot-session-a", host: HOST }); - const second = openWindow({ sessionId: "copilot-session-b", host: OTHER_HOST }); - try { - await first.handshake(); - await second.handshake(); - await cli(["use", "payment team"], { sessionId: "copilot-session-a", host: HOST }); - - assert.match(await first.grounding(), /connected context: payment team/i); - const other = await second.grounding(); - assert.doesNotMatch(other, /connected context: payment team/i); - assert.match(other, /No NeatContext Context is connected to this session/); - } finally { - await first.close(); - await second.close(); - } - }); - - it("keep separate selections when neither publishes a session id", async () => { - const first = openWindow({ host: HOST }); - const second = openWindow({ host: OTHER_HOST }); - try { - await first.handshake(); - await second.handshake(); - await cli(["use", "payment team"], { host: HOST, cwd: workspace }); - await cli(["use", "Dokploy"], { host: OTHER_HOST, cwd: otherWorkspace }); - - assert.match(await first.grounding(), /connected context: payment team/i); - assert.match(await second.grounding(), /connected context: Dokploy/i); - } finally { - await first.close(); - await second.close(); - } - }); -}); - -describe("an explicit session id", () => { - it("still overrides everything the host publishes", async () => { - const env = { ...childEnv({ sessionId: "copilot-session-a" }), NEATCONTEXT_SESSION_ID: "pinned" }; - const run = (args, cwd) => - new Promise((resolve) => { - const child = spawn( - process.execPath, - [path.join(copilot, "neatcontext-cli.mjs"), ...args], - { cwd, stdio: ["ignore", "pipe", "inherit"], env } - ); - let out = ""; - child.stdout.on("data", (chunk) => (out += chunk)); - child.on("exit", () => resolve(out.trim())); - }); - - await run(["use", "payment team"], workspace); - // A different workspace and a different session id: the pin is what decides, - // so the selection is found again. - assert.match(await run(["status"], otherWorkspace), /Connected context: payment team/i); - const selection = JSON.parse( - await readFile(path.join(home, "plugin-sessions", "pinned.json"), "utf8") - ); - assert.ok(selection); - }); -}); - -describe("a host that publishes no identity at all", () => { - // Neither a session id nor a pid: the bridge and the CLI are back to hashing - // their own working directories with no channel between them. Nothing - // downstream can detect that, so what is pinned here is that the user is told. - function bare(args, cwd = workspace) { - return new Promise((resolve) => { - const env = { ...childEnv({}), NEATCONTEXT_HOST_KEY: "" }; - const child = spawn( - process.execPath, - [path.join(copilot, "neatcontext-cli.mjs"), ...args], - { cwd, stdio: ["ignore", "pipe", "pipe"], env } - ); - let stdout = ""; - let stderr = ""; - child.stdout.on("data", (chunk) => (stdout += chunk)); - child.stderr.on("data", (chunk) => (stderr += chunk)); - child.on("exit", () => resolve({ stdout: stdout.trim(), stderr: stderr.trim() })); - }); - } - - it("says so rather than failing silently", async () => { - const { stderr } = await bare(["status"]); - assert.match(stderr, /publishes no session identity/); - assert.match(stderr, /NEATCONTEXT_SESSION_ID/); - }); - - it("is not silenced by a host key that cannot name a file", async () => { - // `hostKey()` rejects these, so no pointer is written and the two halves are - // back to their own working directories. Accepting them here would claim a - // channel that was never opened. - for (const key of ["..", ".", "a/b", "a\\b", " "]) { - const { stderr } = await new Promise((resolve) => { - const child = spawn( - process.execPath, - [path.join(copilot, "neatcontext-cli.mjs"), "status"], - { - cwd: workspace, - stdio: ["ignore", "pipe", "pipe"], - env: { ...childEnv({}), NEATCONTEXT_HOST_KEY: key } - } - ); - let out = ""; - child.stderr.on("data", (chunk) => (out += chunk)); - child.on("exit", () => resolve({ stderr: out })); - }); - assert.match(stderr, /publishes no session identity/, `accepted ${JSON.stringify(key)}`); - } - }); - - it("still scopes to the workspace, so one window keeps working", async () => { - await bare(["use", "payment team"]); - assert.match((await bare(["status"])).stdout, /Connected context: payment team/i); - assert.match((await bare(["status"], otherWorkspace)).stdout, /No context is connected/); - }); - - it("stays quiet as soon as the host publishes either one", async () => { - const withSession = await cli(["status"], { sessionId: "copilot-session-a" }); - assert.doesNotMatch(withSession, /publishes no session identity/); - // The host key stands in for COPILOT_LOADER_PID, which is enough on its own: - // it names the pointer file both halves share. - const withKey = await cli(["status"]); - assert.doesNotMatch(withKey, /publishes no session identity/); - - const withPid = await new Promise((resolve) => { - const child = spawn( - process.execPath, - [path.join(copilot, "neatcontext-cli.mjs"), "status"], - { - cwd: workspace, - stdio: ["ignore", "pipe", "pipe"], - env: { - ...childEnv({}), - NEATCONTEXT_HOST_KEY: "", - COPILOT_LOADER_PID: "48596" - } - } - ); - let stderr = ""; - child.stderr.on("data", (chunk) => (stderr += chunk)); - child.on("exit", () => resolve(stderr.trim())); - }); - assert.doesNotMatch(withPid, /publishes no session identity/); - }); -}); - -describe("a session id that cannot name a file", () => { - it("is ignored, and says so instead of scoping somewhere else in silence", async () => { - const run = (value) => - new Promise((resolve) => { - const child = spawn( - process.execPath, - [path.join(copilot, "neatcontext-cli.mjs"), "status"], - { - cwd: workspace, - stdio: ["ignore", "pipe", "pipe"], - env: { ...childEnv({}), NEATCONTEXT_SESSION_ID: value } - } - ); - let stderr = ""; - child.stderr.on("data", (chunk) => (stderr += chunk)); - child.on("exit", () => resolve(stderr.trim())); - }); - - assert.match(await run("teams/payments"), /cannot name a session file/); - assert.match(await run("../escape"), /cannot name a session file/); - // The value itself is not echoed back: it is the user's, and the length is - // enough to recognise which one they set. - assert.doesNotMatch(await run("teams/payments"), /teams\/payments/); - assert.equal(await run("payments-2"), ""); - }); -}); - -describe("upgrading from a release that scoped to the workspace", () => { - it("names the connection that workspace used to have", async () => { - // Exactly what an earlier release left behind: a selection filed under the - // workspace digest, with nothing under this session's id. - await cli(["use", "payment team"], { cwd: workspace }); - const status = await cli(["status"], { sessionId: "copilot-session-new" }); - assert.match(status, /No context is connected/); - assert.match(status, /An earlier version of this plugin connected "payment team"/); - assert.match(status, /\/neatcontext:use payment team/); - }); - - it("says nothing once this session has its own connection", async () => { - await cli(["use", "payment team"], { cwd: workspace }); - await cli(["use", "Dokploy"], { sessionId: "copilot-session-new" }); - const status = await cli(["status"], { sessionId: "copilot-session-new" }); - assert.doesNotMatch(status, /An earlier version of this plugin/); - }); - - it("stops offering it once the user has acted on it, even after disconnecting", async () => { - await cli(["use", "payment team"], { cwd: workspace }); - // Acting on the hint is what retires it — this session has one of its own now. - await cli(["use", "Dokploy"], { sessionId: "copilot-session-new" }); - await cli(["disconnect"], { sessionId: "copilot-session-new" }); - const status = await cli(["status"], { sessionId: "copilot-session-new" }); - assert.match(status, /No context is connected/); - assert.doesNotMatch(status, /An earlier version of this plugin/); - }); - - it("says nothing when the workspace selection is the live one", async () => { - // No session id published, so this session *is* the workspace: the file is - // its own selection, not a leftover, and must be neither offered nor removed. - await cli(["use", "payment team"], { cwd: workspace }); - const status = await cli(["status"], { cwd: workspace }); - assert.match(status, /Connected context: payment team/i); - assert.doesNotMatch(status, /An earlier version of this plugin/); - }); - - it("says nothing when the workspace never had one", async () => { - const status = await cli(["status"], { sessionId: "copilot-session-new" }); - assert.match(status, /No context is connected/); - assert.doesNotMatch(status, /An earlier version of this plugin/); - }); - - it("ignores a workspace selection without a context name", async () => { - const sessions = path.join(home, "plugin-sessions"); - await mkdir(sessions, { recursive: true }); - await writeFile( - path.join(sessions, `${workspaceSessionId(workspace)}.json`), - JSON.stringify({ contextName: " " }) - ); - - const status = await cli(["status"], { sessionId: "copilot-session-new" }); - assert.match(status, /No context is connected/); - assert.doesNotMatch(status, /An earlier version of this plugin/); - }); -}); - -describe("telling the user when the bridge has not caught up", () => { - it("warns from status when a live bridge is serving another session", async () => { - await writeBridgeRecord("some-other-session"); - const output = await cli(["status"], { sessionId: "copilot-session-a" }); - assert.match(output, /serving an earlier session, not this one/); - assert.match(output, /get_context.*other one/); - }); - - it("warns after use when a live bridge is serving another session", async () => { - await writeBridgeRecord("some-other-session"); - const output = await cli(["use", "payment team"], { sessionId: "copilot-session-a" }); - assert.match(output, /Connected the "payment team" context/); - assert.match(output, /still serving an earlier session/); - }); - - it("says nothing when the bridge agrees", async () => { - await writeBridgeRecord("copilot-session-a"); - const output = await cli(["use", "payment team"], { sessionId: "copilot-session-a" }); - assert.match(output, /Connected the "payment team" context/); - assert.doesNotMatch(output, /still serving an earlier session/); - }); - - it("says nothing when no bridge is publishing at all", async () => { - const output = await cli(["use", "payment team"], { sessionId: "copilot-session-a" }); - assert.match(output, /Connected the "payment team" context/); - assert.doesNotMatch(output, /still serving an earlier session/); - }); - - it("says nothing when the bridge that published it has exited", 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-session", { pid: 2147483647 }); - const output = await cli(["use", "payment team"], { sessionId: "copilot-session-a" }); - assert.doesNotMatch(output, /still serving an earlier session/); - }); -}); diff --git a/tests/host-session.test.mjs b/tests/host-session.test.mjs index cf3790d..908010f 100644 --- a/tests/host-session.test.mjs +++ b/tests/host-session.test.mjs @@ -9,12 +9,10 @@ import { after, before, beforeEach, describe, it } from "node:test"; import { awaitBridgeSession, bridgePointerPath, - configureHostPid, hostKey, hostPointerPath, hostsDirectory, isProcessAlive, - normalizeHostKey, normalizeHostSessionId, publishBridgeSession, pruneHostPointers, @@ -53,8 +51,6 @@ after(async () => { beforeEach(async () => { process.env.NEATCONTEXT_HOME = directory; process.env.NEATCONTEXT_HOST_KEY = "host-under-test"; - // Core knows no host's variable by name; the adapter under test registers it. - configureHostPid(() => process.env.CLAUDE_PID); await rm(hostsDirectory(), { recursive: true, force: true }); }); @@ -81,13 +77,6 @@ describe("what counts as a session id", () => { }); describe("which host process this is", () => { - it("rejects an invalid pid provider", () => { - assert.throws( - () => configureHostPid("CLAUDE_PID"), - /host pid provider must be a function or null/ - ); - }); - it("takes the explicit key when the host or a test supplies one", () => { process.env.NEATCONTEXT_HOST_KEY = "window-7"; assert.equal(hostKey(), "window-7"); @@ -100,12 +89,6 @@ describe("which host process this is", () => { } }); - it("refuses a non-string explicit key", () => { - for (const value of [7, null, undefined, {}]) { - assert.equal(normalizeHostKey(value), null); - } - }); - it("falls back to the host pid, which a slash command reads from its environment", () => { delete process.env.NEATCONTEXT_HOST_KEY; process.env.CLAUDE_PID = "48596"; @@ -117,29 +100,6 @@ describe("which host process this is", () => { process.env.CLAUDE_PID = "not-a-pid"; assert.equal(hostKey(), `pid-${process.ppid}`); }); - - it("ignores a host variable no adapter registered, so one host cannot key on another", () => { - delete process.env.NEATCONTEXT_HOST_KEY; - process.env.CLAUDE_PID = "48596"; - configureHostPid(null); - try { - assert.equal(hostKey(), `pid-${process.ppid}`); - } finally { - configureHostPid(() => process.env.CLAUDE_PID); - } - }); - - it("survives an adapter whose pid lookup throws", () => { - delete process.env.NEATCONTEXT_HOST_KEY; - configureHostPid(() => { - throw new Error("host went away"); - }); - try { - assert.equal(hostKey(), `pid-${process.ppid}`); - } finally { - configureHostPid(() => process.env.CLAUDE_PID); - } - }); }); describe("recording the session a host is on", () => { @@ -176,13 +136,6 @@ describe("recording the session a host is on", () => { assert.equal(await writeHostPointer("session-a"), null); assert.equal(await publishBridgeSession("session-a"), false); }); - - it("removes its temporary file when the atomic rename fails", async () => { - await mkdir(hostPointerPath(hostKey()), { recursive: true }); - - assert.equal(await writeHostPointer("session-a"), null); - assert.deepEqual(await readdir(hostsDirectory()), [`${hostKey()}.json`]); - }); }); describe("the session a long-lived process should serve", () => { diff --git a/tools/sync-context-core.mjs b/tools/sync-context-core.mjs index 5b32860..4ac01b7 100644 --- a/tools/sync-context-core.mjs +++ b/tools/sync-context-core.mjs @@ -13,7 +13,6 @@ const files = [ "extension-commands.mjs", "extension-runtime.mjs", "extensions.mjs", - "host-session.mjs", "local-state.mjs", "mcp-stdio-client.mjs", "routing.mjs", From 449e861bac6a5207b467a5b8822866ebe774398f Mon Sep 17 00:00:00 2001 From: Jingyu Ma Date: Sun, 9 Aug 2026 21:26:21 -0700 Subject: [PATCH 5/6] fix(copilot): guide session-scope upgrades Validate host-provided session ids, surface existing workspace selections after upgrade, and document fallback and long-lived bridge limitations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- plugins/copilot/neatcontext/README.md | 9 +++- .../src/copilot/neatcontext-cli.mjs | 33 ++++++++++++++- .../neatcontext/src/copilot/session.mjs | 16 +++++-- tests/copilot-plugin.test.mjs | 42 +++++++++++++++++++ 4 files changed, 94 insertions(+), 6 deletions(-) diff --git a/plugins/copilot/neatcontext/README.md b/plugins/copilot/neatcontext/README.md index fb050da..9361ac6 100644 --- a/plugins/copilot/neatcontext/README.md +++ b/plugins/copilot/neatcontext/README.md @@ -51,7 +51,14 @@ copilot plugin install neatcontext@neatcontext use Copilot's shared session identity, so they agree even when they start in different working directories. - **Workspace fallback.** On a host that does not expose a session identity, - sessions opened in the same workspace share one selection. + sessions share one selection only when the host starts both plugin processes + in the same working directory. +- **Upgrading.** Existing workspace-scoped selections are preserved. If the + current session has no selection, `/neatcontext:status` names an available + workspace selection so it can be reconnected once. +- **Session replacement.** Session identity is captured when a plugin process + starts. If a host reuses an MCP process for another session, restart that host + before reconnecting a context. - **Saving is explicit.** Run `/neatcontext:save` when the visible conversation contains durable work worth preserving. diff --git a/plugins/copilot/neatcontext/src/copilot/neatcontext-cli.mjs b/plugins/copilot/neatcontext/src/copilot/neatcontext-cli.mjs index f051f2b..fadc671 100644 --- a/plugins/copilot/neatcontext/src/copilot/neatcontext-cli.mjs +++ b/plugins/copilot/neatcontext/src/copilot/neatcontext-cli.mjs @@ -20,8 +20,12 @@ // Exit code is always 0: the output is meant to be read, not branched on. import { readFile, rm } from "node:fs/promises"; -import "./session.mjs"; -import { clearSelection, readSelection } from "../core/local-state.mjs"; +import { workspaceSessionId } from "./session.mjs"; +import { + clearSelection, + readSelection, + sessionSelectionFilePath +} from "../core/local-state.mjs"; import { createCapturedContext, createContext, @@ -138,6 +142,26 @@ async function loadState() { return { contexts, selection, connected }; } +async function workspaceSelectionHint(state) { + const workspace = workspaceSessionId(); + if (sessionId() === workspace) { + return null; + } + try { + const saved = JSON.parse(await readFile(sessionSelectionFilePath(workspace), "utf8")); + const context = state.contexts.find((candidate) => candidate.id === saved?.contextId); + if (!context) { + return null; + } + return ( + `An earlier version connected "${context.name}" to this workspace. ` + + `Reconnect it for this session with \`/neatcontext:use ${context.name}\`.` + ); + } catch { + return null; + } +} + async function commandStatus(state) { const { connected, selection } = state; const routing = await readRouting(); @@ -214,6 +238,11 @@ async function commandStatus(state) { "`/neatcontext:create`." : "No context is connected yet. Use `/neatcontext:use` to pick one." ); + const upgrade = await workspaceSelectionHint(state); + if (upgrade) { + print(""); + print(upgrade); + } reportMode(); } diff --git a/plugins/copilot/neatcontext/src/copilot/session.mjs b/plugins/copilot/neatcontext/src/copilot/session.mjs index e5c35ab..4dbe813 100644 --- a/plugins/copilot/neatcontext/src/copilot/session.mjs +++ b/plugins/copilot/neatcontext/src/copilot/session.mjs @@ -8,8 +8,11 @@ // session id. In that case all Copilot sessions opened in one workspace share // the selection, as they did before. // -// NEATCONTEXT_SESSION_ID overrides the digest — for tests, and for any host -// that can inject a real per-session id into every plugin process. +// The host identity is captured when each process starts. This adapter does not +// try to detect a session replacement underneath a long-lived MCP server. +// +// NEATCONTEXT_SESSION_ID overrides both the host id and the digest — for tests, +// and for any host that can inject a real per-session id into every process. // // CLAUDE_CODE_SESSION_ID is deliberately NOT consulted, even though a // Claude-compat host might set it: a variable only some of this plugin's @@ -26,6 +29,13 @@ function explicitId(value) { return typeof value === "string" && value.trim().length > 0 ? value.trim() : null; } +const SAFE_HOST_SESSION_ID = /^[A-Za-z0-9][A-Za-z0-9._-]{0,199}$/; + +function hostSessionId(value) { + const id = explicitId(value); + return id && SAFE_HOST_SESSION_ID.test(id) ? id : null; +} + export function workspaceSessionId(workspace = process.cwd()) { const resolved = path.resolve(workspace); // Windows paths compare case-insensitively; two spellings of one folder must @@ -38,7 +48,7 @@ export function workspaceSessionId(workspace = process.cwd()) { export function copilotSessionId() { return ( explicitId(process.env.NEATCONTEXT_SESSION_ID) ?? - explicitId(process.env.COPILOT_AGENT_SESSION_ID) ?? + hostSessionId(process.env.COPILOT_AGENT_SESSION_ID) ?? workspaceSessionId() ); } diff --git a/tests/copilot-plugin.test.mjs b/tests/copilot-plugin.test.mjs index 18c1c59..3c6fbd7 100644 --- a/tests/copilot-plugin.test.mjs +++ b/tests/copilot-plugin.test.mjs @@ -581,6 +581,12 @@ test("Copilot sessions scope to the workspace when no session id is provided", a const sameWorkspace = await runNode(cli, ["status"], { env: wsEnv, cwd: workspaceA }); assert.match(sameWorkspace.stdout, /Connected context: workspace scoped/); + const unsafeHostId = await runNode(cli, ["status"], { + env: { ...wsEnv, COPILOT_AGENT_SESSION_ID: "../not-a-session" }, + cwd: workspaceA + }); + assert.match(unsafeHostId.stdout, /Connected context: workspace scoped/); + const otherWorkspace = await runNode(cli, ["status"], { env: wsEnv, cwd: workspaceB }); assert.match(otherWorkspace.stdout, /No context is connected yet/); @@ -629,6 +635,42 @@ test("Copilot CLI and MCP bridge share the host session across working directori assert.match(otherSession.stdout, /No context is connected yet/); }); +test("Copilot status offers an existing workspace selection after upgrading", async (t) => { + const home = await isolatedHome("neatcontext-copilot-upgrade-"); + const workspace = await mkdtemp(path.join(os.tmpdir(), "copilot-upgrade-ws-")); + t.after(async () => { + await Promise.all( + [home.directory, workspace].map((directory) => + rm(directory, { recursive: true, force: true }) + ) + ); + }); + const workspaceEnv = { + ...home.env, + NEATCONTEXT_SESSION_ID: "", + COPILOT_AGENT_SESSION_ID: "" + }; + const sessionEnv = { + ...workspaceEnv, + COPILOT_AGENT_SESSION_ID: "copilot-agent-session-new" + }; + + await createContext(home, "upgrade target"); + await runNode(cli, ["use", "upgrade target"], { env: workspaceEnv, cwd: workspace }); + + const status = await runNode(cli, ["status"], { env: sessionEnv, cwd: workspace }); + assert.match(status.stdout, /No context is connected yet/); + assert.match(status.stdout, /earlier version connected "upgrade target"/); + assert.match(status.stdout, /\/neatcontext:use upgrade target/); + + await runNode(cli, ["delete", "upgrade target", "--yes"], { + env: sessionEnv, + cwd: workspace + }); + const missing = await runNode(cli, ["status"], { env: sessionEnv, cwd: workspace }); + assert.doesNotMatch(missing.stdout, /earlier version connected/); +}); + test("Copilot MCP bridge serves Contexts and routing locally", async (t) => { const home = await isolatedHome("neatcontext-copilot-mcp-"); const sessions = []; From c4fcc26fbf5ebb5fa3a09aab12ce9e3b06f29994 Mon Sep 17 00:00:00 2001 From: Jingyu Ma Date: Sun, 9 Aug 2026 21:31:23 -0700 Subject: [PATCH 6/6] test(copilot): pin upgrade hint dismissal Clarify that the legacy workspace hint is per session and verify it disappears after the session reconnects. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- plugins/copilot/neatcontext/README.md | 2 +- tests/copilot-plugin.test.mjs | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/plugins/copilot/neatcontext/README.md b/plugins/copilot/neatcontext/README.md index 9361ac6..dd9c247 100644 --- a/plugins/copilot/neatcontext/README.md +++ b/plugins/copilot/neatcontext/README.md @@ -55,7 +55,7 @@ copilot plugin install neatcontext@neatcontext in the same working directory. - **Upgrading.** Existing workspace-scoped selections are preserved. If the current session has no selection, `/neatcontext:status` names an available - workspace selection so it can be reconnected once. + workspace selection so it can be reconnected for that session. - **Session replacement.** Session identity is captured when a plugin process starts. If a host reuses an MCP process for another session, restart that host before reconnecting a context. diff --git a/tests/copilot-plugin.test.mjs b/tests/copilot-plugin.test.mjs index 3c6fbd7..70600ce 100644 --- a/tests/copilot-plugin.test.mjs +++ b/tests/copilot-plugin.test.mjs @@ -663,6 +663,11 @@ test("Copilot status offers an existing workspace selection after upgrading", as assert.match(status.stdout, /earlier version connected "upgrade target"/); assert.match(status.stdout, /\/neatcontext:use upgrade target/); + await runNode(cli, ["use", "upgrade target"], { env: sessionEnv, cwd: workspace }); + const connected = await runNode(cli, ["status"], { env: sessionEnv, cwd: workspace }); + assert.match(connected.stdout, /Connected context: upgrade target/); + assert.doesNotMatch(connected.stdout, /earlier version connected/); + await runNode(cli, ["delete", "upgrade target", "--yes"], { env: sessionEnv, cwd: workspace