From 065ceb3e30c0925447625b03cd0dafb0407043a4 Mon Sep 17 00:00:00 2001 From: tanglearncode Date: Mon, 10 Aug 2026 06:04:41 +0800 Subject: [PATCH] fix(routing): let a session grounded in nothing reach the context it needs Nothing connected is the case routing exists for, and it was the one case that could not route. Two independent faults, both found driving the real Copilot bridge against a real store: a question that plainly belonged to a saved incident context came back as an offer to run /neatcontext:use. A mode nobody chose was written down. Until now the resolved mode was persisted on every routing write - deriving a card, logging a decision, noting a refusal - so a file recorded whatever default the build that last touched it happened to ship. "ask" was that default until the shortlist learned to ask on its own, so every machine that had ever saved a context had "ask" written into plugin-routing.json, and #77's new default reached none of them. Reinstalling did not help either: that file lives in ~/.neatcontext and outlives any one install. Schema 2 fixes both halves. An unchosen mode now stays out of the file entirely, so a future change of default still reaches this machine, and an "ask" in a schema 1 file is read as what it was - a default, not a choice - and dropped. "manual" was never a default, so it always stands. A machine where "ask" genuinely was chosen loses it once; one `/neatcontext:mode ask --global` puts it back, recorded under a schema where a stored mode means somebody asked for it. The nothing-connected text argued against routing. It led with a slash command, which is the first and most imperative thing the model reads, and it answered "what now?" before the menu below it ever got a turn. It now leads with the route the session can take itself, and falls back to the commands only where they are genuinely the way forward: manual mode, or a store with nothing in it. Codex keeps its no-polling rule verbatim. The menu's rules split on whether anything is connected. Every guard there - "clearly belongs", "not on a follow-up", "stands on its own" - exists to make leaving a context cost something. A session grounded in nothing has nowhere to leave from, and the same sentences read as reasons to do nothing at all. Tests drive the migration through all five core copies, since one ~/.neatcontext serves every host a user has installed, and each host's own harness covers its manual-mode and empty-store fallbacks. --- .../neatcontext/src/codex/mcp-bridge.mjs | 39 ++- .../plugins/neatcontext/src/core/routing.mjs | 87 +++++- codex-marketplace/tests/codex-plugin.test.mjs | 64 +++++ .../neatcontext/src/claude/mcp-bridge.mjs | 42 ++- .../neatcontext/src/core/routing.mjs | 87 +++++- .../neatcontext/src/copilot/mcp-bridge.mjs | 43 ++- .../copilot/neatcontext/src/core/routing.mjs | 87 +++++- .../neatcontext/src/core/routing.mjs | 87 +++++- .../neatcontext/src/kimi/mcp-bridge.mjs | 42 ++- plugins/pi/neatcontext/src/core/routing.mjs | 87 +++++- plugins/pi/neatcontext/src/pi/runtime.mjs | 53 +++- .../pi/neatcontext/tests/pi-runtime.test.mjs | 19 ++ shared/core/routing.mjs | 87 +++++- tests/copilot-plugin.test.mjs | 5 +- tests/kimi-plugin.test.mjs | 51 ++++ tests/routing-confidence.test.mjs | 4 +- tests/routing-shortlist.test.mjs | 20 +- tests/routing-unconnected.test.mjs | 257 ++++++++++++++++++ tests/routing.test.mjs | 42 +++ 19 files changed, 1104 insertions(+), 99 deletions(-) create mode 100644 tests/routing-unconnected.test.mjs diff --git a/codex-marketplace/plugins/neatcontext/src/codex/mcp-bridge.mjs b/codex-marketplace/plugins/neatcontext/src/codex/mcp-bridge.mjs index 4e23637..7040214 100644 --- a/codex-marketplace/plugins/neatcontext/src/codex/mcp-bridge.mjs +++ b/codex-marketplace/plugins/neatcontext/src/codex/mcp-bridge.mjs @@ -82,11 +82,36 @@ const GET_CONTEXT_TOOL = { // there is nothing to connect. const NOTHING_CONNECTED_HEAD = "No NeatContext Context is selected for this thread."; +// The manual-mode version, and the fallback whenever no menu follows. Routing +// is off here, so a command the user types is genuinely the only way forward. const NOTHING_CONNECTED = `${NOTHING_CONNECTED_HEAD} Continue normal work without NeatContext grounding. Do not retry ` + "get_context until use_context succeeds or the user explicitly asks to refresh NeatContext " + "state. Connect one with `$neatcontext:use`."; +// What to say instead when routing is on and there are Contexts to route to. +// +// Leading with `$neatcontext:use` in that situation is what made routing look +// broken: this text is the first and most imperative thing the model reads, and +// it answered "what now?" with a command for the user to type before the menu +// below ever got a turn. The no-polling rule these carry is the same one the +// original text established — a Context that does not exist is not worth asking +// about twice — and selecting one the menu already names is not polling. +const NOTHING_CONNECTED_ROUTABLE = + `${NOTHING_CONNECTED_HEAD} There are Contexts on this machine, listed below with what each ` + + "one is for. When one of them covers what the user asked, select it with use_context and " + + "then call get_context once — do not ask the user to run a command to select a Context you " + + "can already name. When none of them covers it, continue normal work without NeatContext " + + "grounding and do not retry get_context until use_context succeeds or the user explicitly " + + "asks to refresh NeatContext state."; + +const NOTHING_CONNECTED_ASK = + `${NOTHING_CONNECTED_HEAD} There are Contexts on this machine, listed below with what each ` + + "one is for. Routing is in ask mode, so name the one that covers what the user asked and ask " + + "whether to select it rather than selecting first. When none of them covers it, continue " + + "normal work without NeatContext grounding and do not retry get_context until use_context " + + "succeeds or the user explicitly asks to refresh NeatContext state."; + const NOTHING_EXISTS = `${NOTHING_CONNECTED_HEAD} There are none on this machine yet. Continue normal work without ` + "NeatContext grounding and do not retry get_context. Save the work in this conversation as " + @@ -97,7 +122,7 @@ const CONNECTION_RULE = `## Connecting a context, in Codex Contexts are connected from this session and nowhere else: the \`use_context\` tool, or \`$neatcontext:use \` run by the user. \`$neatcontext:disconnect\` disconnects the current one from this session. New ones are made from here too: \`$neatcontext:save\` turns the work in this conversation into one, and \`$neatcontext:create\` builds one around a folder of documents the user already has. -There is no Desktop connection right now. Contexts are stored by this plugin. When the connected context is the wrong one, or none is connected, name the one you need and offer to switch to it here.`; +There is no Desktop connection right now. Contexts are stored by this plugin. When the connected context is the wrong one, or none is connected, name the one you need and select it here with \`use_context\` — or offer to, when the routing rules above say to ask first.`; // The two tools that let a session change what it is grounded in. They are the // plugin's whole routing mechanism: there is no model in any process here, so @@ -202,7 +227,17 @@ async function listAllContexts() { // they have none. async function nothingConnectedText() { const { contexts } = await listAllContexts().catch(() => ({ contexts: [] })); - return contexts.length === 0 ? NOTHING_EXISTS : NOTHING_CONNECTED; + if (contexts.length === 0) { + return NOTHING_EXISTS; + } + // The mode decides whether a menu is about to follow this text, and therefore + // whether pointing at a command is the honest answer or the one that breaks + // routing. + const mode = resolveMode(await readRouting().catch(() => ({ sessions: {} })), sessionId()); + if (mode === "manual") { + return NOTHING_CONNECTED; + } + return mode === "ask" ? NOTHING_CONNECTED_ASK : NOTHING_CONNECTED_ROUTABLE; } // The selected context, or null when nothing is selected. A selection diff --git a/codex-marketplace/plugins/neatcontext/src/core/routing.mjs b/codex-marketplace/plugins/neatcontext/src/core/routing.mjs index bf5675e..5c8da96 100644 --- a/codex-marketplace/plugins/neatcontext/src/core/routing.mjs +++ b/codex-marketplace/plugins/neatcontext/src/core/routing.mjs @@ -41,7 +41,9 @@ export const MODES = ["auto", "ask", "manual"]; // Asking every time then costs a question per turn and buys nothing. export const DEFAULT_MODE = "auto"; -const SCHEMA = 1; +// 2 marks the file as one where a stored mode means somebody chose it. See +// `chosenMode` for what schema 1 got wrong and why it cannot be read literally. +const SCHEMA = 2; const MAX_USE_WHEN = 240; const MAX_ALIASES = 12; const MAX_DECISIONS = 100; @@ -137,13 +139,40 @@ export async function readRouting() { return { schema: SCHEMA, declines, - mode: MODES.includes(parsed?.mode) ? parsed.mode : DEFAULT_MODE, + mode: chosenMode(parsed), cards, sessions: typeof parsed?.sessions === "object" && parsed.sessions !== null ? parsed.sessions : {}, decisions: Array.isArray(parsed?.decisions) ? parsed.decisions : [] }; } +// The mode the user actually chose, or null when nobody has. +// +// Null is not a synonym for the default, and the difference is the whole point. +// Until schema 2 the *resolved* mode was written back on every routing write — +// deriving a card, logging a decision, noting a refusal — so a file ended up +// stating whatever the default happened to be in the build that last touched +// it. "ask" was that default until the shortlist learned to ask on its own, so +// every machine that had ever saved a context had "ask" written down, and +// changing the default to "auto" reached none of them. Routing looked switched +// off on exactly the machines that used it most, and reinstalling did not help: +// this file lives in ~/.neatcontext and outlives any one plugin install. +// +// So an "ask" in a pre-schema-2 file is not evidence of a choice, and is +// dropped. A machine where it genuinely was one loses it once, and a single +// `/neatcontext:mode ask --global` puts it back — written under schema 2 this +// time, where a stored mode means somebody asked for it. "manual" is never a +// default, so it was always deliberate and always stands. +function chosenMode(parsed) { + if (!MODES.includes(parsed?.mode)) { + return null; + } + if (parsed.schema !== SCHEMA && parsed.mode === "ask") { + return null; + } + return parsed.mode; +} + async function writeRouting(state) { // Sessions accumulate forever otherwise — one per host window, ever. const sessions = Object.entries(state.sessions) @@ -151,11 +180,16 @@ async function writeRouting(state) { .slice(0, MAX_SESSIONS); const file = routingFilePath(); await mkdir(path.dirname(file), { recursive: true }); + const { mode, ...rest } = state; await writeFile( file, `${JSON.stringify( { - ...state, + ...rest, + // Written down only when somebody chose it. An unchosen mode stays out + // of the file entirely, so this machine keeps following the default + // rather than pinning whichever one this build happens to ship. + ...(MODES.includes(mode) ? { mode } : {}), sessions: Object.fromEntries(sessions), declines: pruneDeclines(state.declines, Date.now()), decisions: state.decisions.slice(-MAX_DECISIONS) @@ -232,7 +266,12 @@ export function isCardStale(card, source) { // auto, another window writing code wants to be left alone. export function resolveMode(state, id) { const session = id ? state.sessions[id] : null; - return MODES.includes(session?.mode) ? session.mode : state.mode; + if (MODES.includes(session?.mode)) { + return session.mode; + } + // `state.mode` is null on a machine where nobody has set one, which is what + // lets the default below actually apply. + return MODES.includes(state.mode) ? state.mode : DEFAULT_MODE; } export function setMode(mode, { global: isGlobal = false, id = sessionId() } = {}) { @@ -422,23 +461,43 @@ export function renderMenu(entries, { connectedId, mode } = {}) { lines.push(`- **${entry.name}**${marker} — ${describe(entry)}`); } lines.push(""); - lines.push(...routingInstructions(mode)); + lines.push(...routingInstructions(mode, Boolean(connectedId))); return lines.join("\n"); } // Shared with the shortlist below, because a shortlist is still a menu: the // same model still decides, still asks first in ask mode, and still must not // route on a follow-up. Only the number of things it chooses between differs. -function routingInstructions(mode) { +// +// Split on whether anything is connected, because the two situations are not +// the same move. Switching means leaving somewhere, and every guard here — +// "clearly belongs", "not on a follow-up", "stands on its own" — exists to make +// leaving cost something. A session grounded in nothing has nowhere to leave +// from: the same guards read as reasons to do nothing at all, which is how a +// question that plainly belonged to a saved context ended up answered from +// general knowledge with a slash command offered as consolation. +function routingInstructions(mode, connected) { return [ - mode === "auto" - ? "Routing is on (auto). When the user's request clearly belongs to one of the other contexts above, switch to it with the `use_context` tool, then call `get_context` and answer from what it returns. Say in one line that you switched, and which context you are now on. When two contexts are both plausible, do not guess — name them and ask which one." - : "Routing is on (ask). When the user's request clearly belongs to one of the other contexts above, say so and ask before switching — never switch first. If they agree, call `use_context`, then `get_context`, and answer from what it returns.", - "Do not route on follow-ups, short replies, or anything that continues the current topic — a switch needs a request that stands on its own and plainly belongs elsewhere. If the user declines a switch, drop it and do not raise that context again this session.", + connected ? switchInstruction(mode) : connectInstruction(mode), + connected + ? "Do not route on follow-ups, short replies, or anything that continues the current topic — a switch needs a request that stands on its own and plainly belongs elsewhere. If the user declines a switch, drop it and do not raise that context again this session." + : "There is no current topic to continue and nothing to leave, so connecting the context a request belongs to is the expected move rather than an interruption. If the user declines one, drop it and do not raise that context again this session.", "When the user corrects a wrong route, pass what they called it as `alias` to `use_context` so the same words route correctly next time." ]; } +function switchInstruction(mode) { + return mode === "auto" + ? "Routing is on (auto). When the user's request clearly belongs to one of the other contexts above, switch to it with the `use_context` tool, then call `get_context` and answer from what it returns. Say in one line that you switched, and which context you are now on. When two contexts are both plausible, do not guess — name them and ask which one." + : "Routing is on (ask). When the user's request clearly belongs to one of the other contexts above, say so and ask before switching — never switch first. If they agree, call `use_context`, then `get_context`, and answer from what it returns."; +} + +function connectInstruction(mode) { + return mode === "auto" + ? "Routing is on (auto), and this session is grounded in nothing yet. When the user's request belongs to one of the contexts above, connect it with the `use_context` tool, then call `get_context` and answer from what it returns. Do that yourself — do not ask the user to run a command to connect a context you can already name. Say in one line which context you connected. When two contexts are both plausible, do not guess — name them and ask which one." + : "Routing is on (ask), and this session is grounded in nothing yet. When the user's request belongs to one of the contexts above, name it and ask whether to connect it — never connect first. If they agree, call `use_context`, then `get_context`, and answer from what it returns."; +} + // The same menu, cut down to what the request actually reached. // // Two things change against the full list. It is short, so each entry can @@ -461,13 +520,15 @@ export function renderShortlist(entries, { connectedId, mode, decision } = {}) { } lines.push(""); lines.push( - "These are the contexts on this machine whose own description matched the request, best first. Others exist and did not match — that is a reason to stay where you are, not to reach for the closest one here." + connectedId + ? "These are the contexts on this machine whose own description matched the request, best first. Others exist and did not match — that is a reason to stay where you are, not to reach for the closest one here." + : "These are the contexts on this machine whose own description matched the request, best first. Others exist and did not match — so if none of these covers the request, say the store does not have it rather than reaching for the closest one here." ); const tie = tieNote(decision); if (tie) { lines.push(tie); } - lines.push(...routingInstructions(mode)); + lines.push(...routingInstructions(mode, Boolean(connectedId))); return lines.join("\n"); } @@ -486,7 +547,7 @@ function tieNote(decision) { return ( `${names} match the request about equally well, so which one is right is not something to ` + "decide on the user's behalf. Name them, say in one line what each covers, and ask which — " + - "in auto mode too. Switch only once they have answered." + "in auto mode too. Call `use_context` only once they have answered." ); } diff --git a/codex-marketplace/tests/codex-plugin.test.mjs b/codex-marketplace/tests/codex-plugin.test.mjs index 6cd1653..68d13fa 100644 --- a/codex-marketplace/tests/codex-plugin.test.mjs +++ b/codex-marketplace/tests/codex-plugin.test.mjs @@ -451,3 +451,67 @@ test("Codex narrows the routing menu to the request", async () => { await session.close(); } }); + +// A thread with nothing selected is the case routing exists for. Leading with +// `$neatcontext:use` there is what made routing look broken: it is the first +// thing the model reads and it answers "what now?" before the menu below it +// gets a turn. The no-polling rule has to survive the rewording. +test("Codex tells an unselected thread to select a Context itself", async () => { + const home = await mkdtemp(path.join(os.tmpdir(), "neatcontext-codex-unselected-")); + const env = { NEATCONTEXT_HOME: home, CODEX_THREAD_ID: "unselected-thread" }; + const capturePath = path.join(home, "refunds.json"); + await writeFile( + capturePath, + JSON.stringify({ + schema: 1, + name: "Refunds", + profile: + "# Refunds\n\n## Purpose\nRefunds and chargebacks.\n\n## What to do\nAnswer.\n\n" + + "## What to avoid\nGuessing.\n\n## Behavior\nBe concise.", + routingDescription: "refunds and chargebacks", + knowledge: [{ path: "session-summary.md", content: "# Refunds\n\nrefunds" }] + }), + "utf8" + ); + assert.equal((await runNode(cli, ["save", "--from", capturePath, "--consume"], { env })).code, 0); + // Saving connects an unconnected session, so step back off it to reach the + // state this test is about. + assert.equal((await runNode(cli, ["disconnect"], { env })).code, 0); + + const session = rpcSession(env); + try { + await session.call({ + jsonrpc: "2.0", + id: 1, + method: "initialize", + params: { protocolVersion: "2025-11-25", capabilities: {}, clientInfo: { name: "t", version: "1" } } + }); + const routable = ( + await session.call({ + jsonrpc: "2.0", + id: 2, + method: "tools/call", + params: { name: "get_context", arguments: { query: "refunds" } } + }) + ).result.content[0].text; + assert.match(routable, /select it with use_context/); + assert.match(routable, /do not ask the user to run a command/); + assert.match(routable, /do not retry get_context/, "the no-polling rule must survive"); + + // Manual mode publishes no menu, so there is nothing to select from and the + // command really is the only way forward. + assert.equal((await runNode(cli, ["mode", "manual"], { env })).code, 0); + const manual = ( + await session.call({ + jsonrpc: "2.0", + id: 3, + method: "tools/call", + params: { name: "get_context", arguments: { query: "refunds" } } + }) + ).result.content[0].text; + assert.match(manual, /Connect one with `\$neatcontext:use`/); + assert.doesNotMatch(manual, /## Contexts/); + } finally { + await session.close(); + } +}); diff --git a/plugins/claude-code/neatcontext/src/claude/mcp-bridge.mjs b/plugins/claude-code/neatcontext/src/claude/mcp-bridge.mjs index 6fad367..18a2e99 100644 --- a/plugins/claude-code/neatcontext/src/claude/mcp-bridge.mjs +++ b/plugins/claude-code/neatcontext/src/claude/mcp-bridge.mjs @@ -77,11 +77,37 @@ const GET_CONTEXT_TOOL = { // there is nothing to connect. const NOTHING_CONNECTED_HEAD = "No NeatContext Context is connected to this session."; +// The manual-mode version, and the fallback whenever no menu follows. Routing +// is off here, so a command the user types is genuinely the only way forward. const NOTHING_CONNECTED = `${NOTHING_CONNECTED_HEAD} Connect one with \`/neatcontext:use\`, save this conversation as ` + "a new one with `/neatcontext:save`, or create one from a folder of documents with " + "`/neatcontext:create`. Until then, do not answer from general knowledge."; +// What to say instead when routing is on and there are contexts to route to. +// +// Leading with `/neatcontext:use` in that situation is what made routing look +// broken. This text is the first and most imperative thing the model reads, and +// it answered "what now?" with a command for the user to type before the menu +// below ever got a turn — so a question that plainly belonged to a saved +// context came back as an offer to go and connect one by hand. The menu is +// still what carries the mode-specific rules; these two only have to stop +// contradicting it. +const NOTHING_CONNECTED_ROUTABLE = + `${NOTHING_CONNECTED_HEAD} There are contexts on this machine, listed below with what each ` + + "one is for. Connect the one this request belongs to with `use_context`, then call " + + "`get_context` again and answer from what it returns — do not ask the user to run a command " + + "to connect a context you can already name. If none of them covers the request, say so and " + + "offer `/neatcontext:save` to make one out of this conversation. Until then, do not answer " + + "from general knowledge."; + +const NOTHING_CONNECTED_ASK = + `${NOTHING_CONNECTED_HEAD} There are contexts on this machine, listed below with what each ` + + "one is for. Routing is in ask mode, so name the one this request belongs to and ask whether " + + "to connect it rather than connecting first. If none of them covers the request, say so and " + + "offer `/neatcontext:save` to make one out of this conversation. Until then, do not answer " + + "from general knowledge."; + const NOTHING_EXISTS = `${NOTHING_CONNECTED_HEAD} There are none on this machine yet, so \`/neatcontext:use\` has ` + "nothing to list. Save the work in this conversation as the first one with " + @@ -94,7 +120,7 @@ const CONNECTION_RULE = `## Connecting a context, in Claude Code Contexts are connected from this session and nowhere else: the \`use_context\` tool, or \`/neatcontext:use \` run by the user. \`/neatcontext:disconnect\` disconnects the current one from this session. New ones are made from here too: \`/neatcontext:save\` turns the work in this conversation into one, and \`/neatcontext:create\` builds one around a folder of documents the user already has. -There is no Desktop connection right now. Contexts are stored by this plugin. When the connected context is the wrong one, or none is connected, name the one you need and offer to switch to it here.`; +There is no Desktop connection right now. Contexts are stored by this plugin. When the connected context is the wrong one, or none is connected, name the one you need and connect it here with \`use_context\` — or offer to, when the routing rules above say to ask first.`; // The two tools that let a session change what it is grounded in. They are the // plugin's whole routing mechanism: there is no model in any process here, so @@ -181,7 +207,7 @@ These instructions are fixed at the handshake and cannot be updated, so they are When the user asks anything that depends on their own domain, documents, tools, or team conventions, call the get_context tool and let its answer decide: - If it returns a Context, ground your answer in it and cite what you used. -- Only if it reports that nothing is connected, say so, and offer the way forward it names — connecting an existing context with /neatcontext:use, saving this conversation as a new one with /neatcontext:save, or building one from a folder of documents with /neatcontext:create. Which of those actually applies depends on what exists right now, so relay what the tool says rather than guessing from this text.`; +- If it reports that nothing is connected, it also lists the contexts that exist and says what to do about them — which may be to connect one yourself with the use_context tool, to ask the user first, or to tell them to run a command. Do what that answer says. It knows the current state and this text does not, so never substitute a slash command of your own for the route it offers.`; function writeLine(message) { process.stdout.write(`${JSON.stringify(message)}\n`); @@ -202,7 +228,17 @@ async function listAllContexts() { // they have none. async function nothingConnectedText() { const { contexts } = await listAllContexts().catch(() => ({ contexts: [] })); - return contexts.length === 0 ? NOTHING_EXISTS : NOTHING_CONNECTED; + if (contexts.length === 0) { + return NOTHING_EXISTS; + } + // The mode decides whether a menu is about to follow this text, and therefore + // whether pointing at a slash command is the honest answer or the one that + // breaks routing. + const mode = resolveMode(await readRouting().catch(() => ({ sessions: {} })), sessionId()); + if (mode === "manual") { + return NOTHING_CONNECTED; + } + return mode === "ask" ? NOTHING_CONNECTED_ASK : NOTHING_CONNECTED_ROUTABLE; } // The selected context, or null when nothing is selected. A selection diff --git a/plugins/claude-code/neatcontext/src/core/routing.mjs b/plugins/claude-code/neatcontext/src/core/routing.mjs index bf5675e..5c8da96 100644 --- a/plugins/claude-code/neatcontext/src/core/routing.mjs +++ b/plugins/claude-code/neatcontext/src/core/routing.mjs @@ -41,7 +41,9 @@ export const MODES = ["auto", "ask", "manual"]; // Asking every time then costs a question per turn and buys nothing. export const DEFAULT_MODE = "auto"; -const SCHEMA = 1; +// 2 marks the file as one where a stored mode means somebody chose it. See +// `chosenMode` for what schema 1 got wrong and why it cannot be read literally. +const SCHEMA = 2; const MAX_USE_WHEN = 240; const MAX_ALIASES = 12; const MAX_DECISIONS = 100; @@ -137,13 +139,40 @@ export async function readRouting() { return { schema: SCHEMA, declines, - mode: MODES.includes(parsed?.mode) ? parsed.mode : DEFAULT_MODE, + mode: chosenMode(parsed), cards, sessions: typeof parsed?.sessions === "object" && parsed.sessions !== null ? parsed.sessions : {}, decisions: Array.isArray(parsed?.decisions) ? parsed.decisions : [] }; } +// The mode the user actually chose, or null when nobody has. +// +// Null is not a synonym for the default, and the difference is the whole point. +// Until schema 2 the *resolved* mode was written back on every routing write — +// deriving a card, logging a decision, noting a refusal — so a file ended up +// stating whatever the default happened to be in the build that last touched +// it. "ask" was that default until the shortlist learned to ask on its own, so +// every machine that had ever saved a context had "ask" written down, and +// changing the default to "auto" reached none of them. Routing looked switched +// off on exactly the machines that used it most, and reinstalling did not help: +// this file lives in ~/.neatcontext and outlives any one plugin install. +// +// So an "ask" in a pre-schema-2 file is not evidence of a choice, and is +// dropped. A machine where it genuinely was one loses it once, and a single +// `/neatcontext:mode ask --global` puts it back — written under schema 2 this +// time, where a stored mode means somebody asked for it. "manual" is never a +// default, so it was always deliberate and always stands. +function chosenMode(parsed) { + if (!MODES.includes(parsed?.mode)) { + return null; + } + if (parsed.schema !== SCHEMA && parsed.mode === "ask") { + return null; + } + return parsed.mode; +} + async function writeRouting(state) { // Sessions accumulate forever otherwise — one per host window, ever. const sessions = Object.entries(state.sessions) @@ -151,11 +180,16 @@ async function writeRouting(state) { .slice(0, MAX_SESSIONS); const file = routingFilePath(); await mkdir(path.dirname(file), { recursive: true }); + const { mode, ...rest } = state; await writeFile( file, `${JSON.stringify( { - ...state, + ...rest, + // Written down only when somebody chose it. An unchosen mode stays out + // of the file entirely, so this machine keeps following the default + // rather than pinning whichever one this build happens to ship. + ...(MODES.includes(mode) ? { mode } : {}), sessions: Object.fromEntries(sessions), declines: pruneDeclines(state.declines, Date.now()), decisions: state.decisions.slice(-MAX_DECISIONS) @@ -232,7 +266,12 @@ export function isCardStale(card, source) { // auto, another window writing code wants to be left alone. export function resolveMode(state, id) { const session = id ? state.sessions[id] : null; - return MODES.includes(session?.mode) ? session.mode : state.mode; + if (MODES.includes(session?.mode)) { + return session.mode; + } + // `state.mode` is null on a machine where nobody has set one, which is what + // lets the default below actually apply. + return MODES.includes(state.mode) ? state.mode : DEFAULT_MODE; } export function setMode(mode, { global: isGlobal = false, id = sessionId() } = {}) { @@ -422,23 +461,43 @@ export function renderMenu(entries, { connectedId, mode } = {}) { lines.push(`- **${entry.name}**${marker} — ${describe(entry)}`); } lines.push(""); - lines.push(...routingInstructions(mode)); + lines.push(...routingInstructions(mode, Boolean(connectedId))); return lines.join("\n"); } // Shared with the shortlist below, because a shortlist is still a menu: the // same model still decides, still asks first in ask mode, and still must not // route on a follow-up. Only the number of things it chooses between differs. -function routingInstructions(mode) { +// +// Split on whether anything is connected, because the two situations are not +// the same move. Switching means leaving somewhere, and every guard here — +// "clearly belongs", "not on a follow-up", "stands on its own" — exists to make +// leaving cost something. A session grounded in nothing has nowhere to leave +// from: the same guards read as reasons to do nothing at all, which is how a +// question that plainly belonged to a saved context ended up answered from +// general knowledge with a slash command offered as consolation. +function routingInstructions(mode, connected) { return [ - mode === "auto" - ? "Routing is on (auto). When the user's request clearly belongs to one of the other contexts above, switch to it with the `use_context` tool, then call `get_context` and answer from what it returns. Say in one line that you switched, and which context you are now on. When two contexts are both plausible, do not guess — name them and ask which one." - : "Routing is on (ask). When the user's request clearly belongs to one of the other contexts above, say so and ask before switching — never switch first. If they agree, call `use_context`, then `get_context`, and answer from what it returns.", - "Do not route on follow-ups, short replies, or anything that continues the current topic — a switch needs a request that stands on its own and plainly belongs elsewhere. If the user declines a switch, drop it and do not raise that context again this session.", + connected ? switchInstruction(mode) : connectInstruction(mode), + connected + ? "Do not route on follow-ups, short replies, or anything that continues the current topic — a switch needs a request that stands on its own and plainly belongs elsewhere. If the user declines a switch, drop it and do not raise that context again this session." + : "There is no current topic to continue and nothing to leave, so connecting the context a request belongs to is the expected move rather than an interruption. If the user declines one, drop it and do not raise that context again this session.", "When the user corrects a wrong route, pass what they called it as `alias` to `use_context` so the same words route correctly next time." ]; } +function switchInstruction(mode) { + return mode === "auto" + ? "Routing is on (auto). When the user's request clearly belongs to one of the other contexts above, switch to it with the `use_context` tool, then call `get_context` and answer from what it returns. Say in one line that you switched, and which context you are now on. When two contexts are both plausible, do not guess — name them and ask which one." + : "Routing is on (ask). When the user's request clearly belongs to one of the other contexts above, say so and ask before switching — never switch first. If they agree, call `use_context`, then `get_context`, and answer from what it returns."; +} + +function connectInstruction(mode) { + return mode === "auto" + ? "Routing is on (auto), and this session is grounded in nothing yet. When the user's request belongs to one of the contexts above, connect it with the `use_context` tool, then call `get_context` and answer from what it returns. Do that yourself — do not ask the user to run a command to connect a context you can already name. Say in one line which context you connected. When two contexts are both plausible, do not guess — name them and ask which one." + : "Routing is on (ask), and this session is grounded in nothing yet. When the user's request belongs to one of the contexts above, name it and ask whether to connect it — never connect first. If they agree, call `use_context`, then `get_context`, and answer from what it returns."; +} + // The same menu, cut down to what the request actually reached. // // Two things change against the full list. It is short, so each entry can @@ -461,13 +520,15 @@ export function renderShortlist(entries, { connectedId, mode, decision } = {}) { } lines.push(""); lines.push( - "These are the contexts on this machine whose own description matched the request, best first. Others exist and did not match — that is a reason to stay where you are, not to reach for the closest one here." + connectedId + ? "These are the contexts on this machine whose own description matched the request, best first. Others exist and did not match — that is a reason to stay where you are, not to reach for the closest one here." + : "These are the contexts on this machine whose own description matched the request, best first. Others exist and did not match — so if none of these covers the request, say the store does not have it rather than reaching for the closest one here." ); const tie = tieNote(decision); if (tie) { lines.push(tie); } - lines.push(...routingInstructions(mode)); + lines.push(...routingInstructions(mode, Boolean(connectedId))); return lines.join("\n"); } @@ -486,7 +547,7 @@ function tieNote(decision) { return ( `${names} match the request about equally well, so which one is right is not something to ` + "decide on the user's behalf. Name them, say in one line what each covers, and ask which — " + - "in auto mode too. Switch only once they have answered." + "in auto mode too. Call `use_context` only once they have answered." ); } diff --git a/plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs b/plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs index bf332df..5eb1551 100644 --- a/plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs +++ b/plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs @@ -72,11 +72,37 @@ const GET_CONTEXT_TOOL = { // there is nothing to connect. const NOTHING_CONNECTED_HEAD = "No NeatContext Context is connected to this session."; +// The manual-mode version, and the fallback whenever no menu follows. Routing +// is off here, so a command the user types is genuinely the only way forward. const NOTHING_CONNECTED = `${NOTHING_CONNECTED_HEAD} Connect one with \`/neatcontext:use\`, save this conversation as ` + "a new one with `/neatcontext:save`, or create one from a folder of documents with " + "`/neatcontext:create`. Until then, do not answer from general knowledge."; +// What to say instead when routing is on and there are contexts to route to. +// +// Leading with `/neatcontext:use` in that situation is what made routing look +// broken. This text is the first and most imperative thing the model reads, and +// it answered "what now?" with a command for the user to type before the menu +// below ever got a turn — so a question that plainly belonged to a saved +// context came back as an offer to go and connect one by hand. The menu is +// still what carries the mode-specific rules; these two only have to stop +// contradicting it. +const NOTHING_CONNECTED_ROUTABLE = + `${NOTHING_CONNECTED_HEAD} There are contexts on this machine, listed below with what each ` + + "one is for. Connect the one this request belongs to with `use_context`, then call " + + "`get_context` again and answer from what it returns — do not ask the user to run a command " + + "to connect a context you can already name. If none of them covers the request, say so and " + + "offer `/neatcontext:save` to make one out of this conversation. Until then, do not answer " + + "from general knowledge."; + +const NOTHING_CONNECTED_ASK = + `${NOTHING_CONNECTED_HEAD} There are contexts on this machine, listed below with what each ` + + "one is for. Routing is in ask mode, so name the one this request belongs to and ask whether " + + "to connect it rather than connecting first. If none of them covers the request, say so and " + + "offer `/neatcontext:save` to make one out of this conversation. Until then, do not answer " + + "from general knowledge."; + const NOTHING_EXISTS = `${NOTHING_CONNECTED_HEAD} There are none on this machine yet, so \`/neatcontext:use\` has ` + "nothing to list. Save the work in this conversation as the first one with " + @@ -89,7 +115,7 @@ const CONNECTION_RULE = `## Connecting a context, in GitHub Copilot Contexts are connected from this session and nowhere else: the \`use_context\` tool, or \`/neatcontext:use \` run by the user. \`/neatcontext:disconnect\` disconnects the current one from this session. New ones are made from here too: \`/neatcontext:save\` turns the work in this conversation into one, and \`/neatcontext:create\` builds one around a folder of documents the user already has. -There is no Desktop connection right now. Contexts are stored by this plugin. When the connected context is the wrong one, or none is connected, name the one you need and offer to switch to it here.`; +There is no Desktop connection right now. Contexts are stored by this plugin. When the connected context is the wrong one, or none is connected, name the one you need and connect it here with \`use_context\` — or offer to, when the routing rules above say to ask first.`; // The two tools that let a session change what it is grounded in. They are the // plugin's whole routing mechanism: there is no model in any process here, so @@ -176,7 +202,7 @@ These instructions are fixed at the handshake and cannot be updated, so they are When the user asks anything that depends on their own domain, documents, tools, or team conventions, call the get_context tool and let its answer decide: - If it returns a Context, ground your answer in it and cite what you used. -- Only if it reports that nothing is connected, say so, and offer the way forward it names — connecting an existing context with /neatcontext:use, saving this conversation as a new one with /neatcontext:save, or building one from a folder of documents with /neatcontext:create. Which of those actually applies depends on what exists right now, so relay what the tool says rather than guessing from this text.`; +- If it reports that nothing is connected, it also lists the contexts that exist and says what to do about them — which may be to connect one yourself with the use_context tool, to ask the user first, or to tell them to run a command. Do what that answer says. It knows the current state and this text does not, so never substitute a slash command of your own for the route it offers.`; function writeLine(message) { process.stdout.write(`${JSON.stringify(message)}\n`); @@ -194,10 +220,19 @@ async function listAllContexts() { // Resolved per call, never fixed at startup: the user can create or save the // first context mid-session, and the next get_context has to stop telling them -// they have none. +// they have none. The mode is read here for the same reason — it decides +// whether a menu is about to follow this text, and therefore whether pointing +// at a slash command is the honest answer or the one that breaks routing. async function nothingConnectedText() { const { contexts } = await listAllContexts().catch(() => ({ contexts: [] })); - return contexts.length === 0 ? NOTHING_EXISTS : NOTHING_CONNECTED; + if (contexts.length === 0) { + return NOTHING_EXISTS; + } + const mode = resolveMode(await readRouting().catch(() => ({ sessions: {} })), sessionId()); + if (mode === "manual") { + return NOTHING_CONNECTED; + } + return mode === "ask" ? NOTHING_CONNECTED_ASK : NOTHING_CONNECTED_ROUTABLE; } // The selected context, or null when nothing is selected. A selection diff --git a/plugins/copilot/neatcontext/src/core/routing.mjs b/plugins/copilot/neatcontext/src/core/routing.mjs index bf5675e..5c8da96 100644 --- a/plugins/copilot/neatcontext/src/core/routing.mjs +++ b/plugins/copilot/neatcontext/src/core/routing.mjs @@ -41,7 +41,9 @@ export const MODES = ["auto", "ask", "manual"]; // Asking every time then costs a question per turn and buys nothing. export const DEFAULT_MODE = "auto"; -const SCHEMA = 1; +// 2 marks the file as one where a stored mode means somebody chose it. See +// `chosenMode` for what schema 1 got wrong and why it cannot be read literally. +const SCHEMA = 2; const MAX_USE_WHEN = 240; const MAX_ALIASES = 12; const MAX_DECISIONS = 100; @@ -137,13 +139,40 @@ export async function readRouting() { return { schema: SCHEMA, declines, - mode: MODES.includes(parsed?.mode) ? parsed.mode : DEFAULT_MODE, + mode: chosenMode(parsed), cards, sessions: typeof parsed?.sessions === "object" && parsed.sessions !== null ? parsed.sessions : {}, decisions: Array.isArray(parsed?.decisions) ? parsed.decisions : [] }; } +// The mode the user actually chose, or null when nobody has. +// +// Null is not a synonym for the default, and the difference is the whole point. +// Until schema 2 the *resolved* mode was written back on every routing write — +// deriving a card, logging a decision, noting a refusal — so a file ended up +// stating whatever the default happened to be in the build that last touched +// it. "ask" was that default until the shortlist learned to ask on its own, so +// every machine that had ever saved a context had "ask" written down, and +// changing the default to "auto" reached none of them. Routing looked switched +// off on exactly the machines that used it most, and reinstalling did not help: +// this file lives in ~/.neatcontext and outlives any one plugin install. +// +// So an "ask" in a pre-schema-2 file is not evidence of a choice, and is +// dropped. A machine where it genuinely was one loses it once, and a single +// `/neatcontext:mode ask --global` puts it back — written under schema 2 this +// time, where a stored mode means somebody asked for it. "manual" is never a +// default, so it was always deliberate and always stands. +function chosenMode(parsed) { + if (!MODES.includes(parsed?.mode)) { + return null; + } + if (parsed.schema !== SCHEMA && parsed.mode === "ask") { + return null; + } + return parsed.mode; +} + async function writeRouting(state) { // Sessions accumulate forever otherwise — one per host window, ever. const sessions = Object.entries(state.sessions) @@ -151,11 +180,16 @@ async function writeRouting(state) { .slice(0, MAX_SESSIONS); const file = routingFilePath(); await mkdir(path.dirname(file), { recursive: true }); + const { mode, ...rest } = state; await writeFile( file, `${JSON.stringify( { - ...state, + ...rest, + // Written down only when somebody chose it. An unchosen mode stays out + // of the file entirely, so this machine keeps following the default + // rather than pinning whichever one this build happens to ship. + ...(MODES.includes(mode) ? { mode } : {}), sessions: Object.fromEntries(sessions), declines: pruneDeclines(state.declines, Date.now()), decisions: state.decisions.slice(-MAX_DECISIONS) @@ -232,7 +266,12 @@ export function isCardStale(card, source) { // auto, another window writing code wants to be left alone. export function resolveMode(state, id) { const session = id ? state.sessions[id] : null; - return MODES.includes(session?.mode) ? session.mode : state.mode; + if (MODES.includes(session?.mode)) { + return session.mode; + } + // `state.mode` is null on a machine where nobody has set one, which is what + // lets the default below actually apply. + return MODES.includes(state.mode) ? state.mode : DEFAULT_MODE; } export function setMode(mode, { global: isGlobal = false, id = sessionId() } = {}) { @@ -422,23 +461,43 @@ export function renderMenu(entries, { connectedId, mode } = {}) { lines.push(`- **${entry.name}**${marker} — ${describe(entry)}`); } lines.push(""); - lines.push(...routingInstructions(mode)); + lines.push(...routingInstructions(mode, Boolean(connectedId))); return lines.join("\n"); } // Shared with the shortlist below, because a shortlist is still a menu: the // same model still decides, still asks first in ask mode, and still must not // route on a follow-up. Only the number of things it chooses between differs. -function routingInstructions(mode) { +// +// Split on whether anything is connected, because the two situations are not +// the same move. Switching means leaving somewhere, and every guard here — +// "clearly belongs", "not on a follow-up", "stands on its own" — exists to make +// leaving cost something. A session grounded in nothing has nowhere to leave +// from: the same guards read as reasons to do nothing at all, which is how a +// question that plainly belonged to a saved context ended up answered from +// general knowledge with a slash command offered as consolation. +function routingInstructions(mode, connected) { return [ - mode === "auto" - ? "Routing is on (auto). When the user's request clearly belongs to one of the other contexts above, switch to it with the `use_context` tool, then call `get_context` and answer from what it returns. Say in one line that you switched, and which context you are now on. When two contexts are both plausible, do not guess — name them and ask which one." - : "Routing is on (ask). When the user's request clearly belongs to one of the other contexts above, say so and ask before switching — never switch first. If they agree, call `use_context`, then `get_context`, and answer from what it returns.", - "Do not route on follow-ups, short replies, or anything that continues the current topic — a switch needs a request that stands on its own and plainly belongs elsewhere. If the user declines a switch, drop it and do not raise that context again this session.", + connected ? switchInstruction(mode) : connectInstruction(mode), + connected + ? "Do not route on follow-ups, short replies, or anything that continues the current topic — a switch needs a request that stands on its own and plainly belongs elsewhere. If the user declines a switch, drop it and do not raise that context again this session." + : "There is no current topic to continue and nothing to leave, so connecting the context a request belongs to is the expected move rather than an interruption. If the user declines one, drop it and do not raise that context again this session.", "When the user corrects a wrong route, pass what they called it as `alias` to `use_context` so the same words route correctly next time." ]; } +function switchInstruction(mode) { + return mode === "auto" + ? "Routing is on (auto). When the user's request clearly belongs to one of the other contexts above, switch to it with the `use_context` tool, then call `get_context` and answer from what it returns. Say in one line that you switched, and which context you are now on. When two contexts are both plausible, do not guess — name them and ask which one." + : "Routing is on (ask). When the user's request clearly belongs to one of the other contexts above, say so and ask before switching — never switch first. If they agree, call `use_context`, then `get_context`, and answer from what it returns."; +} + +function connectInstruction(mode) { + return mode === "auto" + ? "Routing is on (auto), and this session is grounded in nothing yet. When the user's request belongs to one of the contexts above, connect it with the `use_context` tool, then call `get_context` and answer from what it returns. Do that yourself — do not ask the user to run a command to connect a context you can already name. Say in one line which context you connected. When two contexts are both plausible, do not guess — name them and ask which one." + : "Routing is on (ask), and this session is grounded in nothing yet. When the user's request belongs to one of the contexts above, name it and ask whether to connect it — never connect first. If they agree, call `use_context`, then `get_context`, and answer from what it returns."; +} + // The same menu, cut down to what the request actually reached. // // Two things change against the full list. It is short, so each entry can @@ -461,13 +520,15 @@ export function renderShortlist(entries, { connectedId, mode, decision } = {}) { } lines.push(""); lines.push( - "These are the contexts on this machine whose own description matched the request, best first. Others exist and did not match — that is a reason to stay where you are, not to reach for the closest one here." + connectedId + ? "These are the contexts on this machine whose own description matched the request, best first. Others exist and did not match — that is a reason to stay where you are, not to reach for the closest one here." + : "These are the contexts on this machine whose own description matched the request, best first. Others exist and did not match — so if none of these covers the request, say the store does not have it rather than reaching for the closest one here." ); const tie = tieNote(decision); if (tie) { lines.push(tie); } - lines.push(...routingInstructions(mode)); + lines.push(...routingInstructions(mode, Boolean(connectedId))); return lines.join("\n"); } @@ -486,7 +547,7 @@ function tieNote(decision) { return ( `${names} match the request about equally well, so which one is right is not something to ` + "decide on the user's behalf. Name them, say in one line what each covers, and ask which — " + - "in auto mode too. Switch only once they have answered." + "in auto mode too. Call `use_context` only once they have answered." ); } diff --git a/plugins/kimi-code/neatcontext/src/core/routing.mjs b/plugins/kimi-code/neatcontext/src/core/routing.mjs index bf5675e..5c8da96 100644 --- a/plugins/kimi-code/neatcontext/src/core/routing.mjs +++ b/plugins/kimi-code/neatcontext/src/core/routing.mjs @@ -41,7 +41,9 @@ export const MODES = ["auto", "ask", "manual"]; // Asking every time then costs a question per turn and buys nothing. export const DEFAULT_MODE = "auto"; -const SCHEMA = 1; +// 2 marks the file as one where a stored mode means somebody chose it. See +// `chosenMode` for what schema 1 got wrong and why it cannot be read literally. +const SCHEMA = 2; const MAX_USE_WHEN = 240; const MAX_ALIASES = 12; const MAX_DECISIONS = 100; @@ -137,13 +139,40 @@ export async function readRouting() { return { schema: SCHEMA, declines, - mode: MODES.includes(parsed?.mode) ? parsed.mode : DEFAULT_MODE, + mode: chosenMode(parsed), cards, sessions: typeof parsed?.sessions === "object" && parsed.sessions !== null ? parsed.sessions : {}, decisions: Array.isArray(parsed?.decisions) ? parsed.decisions : [] }; } +// The mode the user actually chose, or null when nobody has. +// +// Null is not a synonym for the default, and the difference is the whole point. +// Until schema 2 the *resolved* mode was written back on every routing write — +// deriving a card, logging a decision, noting a refusal — so a file ended up +// stating whatever the default happened to be in the build that last touched +// it. "ask" was that default until the shortlist learned to ask on its own, so +// every machine that had ever saved a context had "ask" written down, and +// changing the default to "auto" reached none of them. Routing looked switched +// off on exactly the machines that used it most, and reinstalling did not help: +// this file lives in ~/.neatcontext and outlives any one plugin install. +// +// So an "ask" in a pre-schema-2 file is not evidence of a choice, and is +// dropped. A machine where it genuinely was one loses it once, and a single +// `/neatcontext:mode ask --global` puts it back — written under schema 2 this +// time, where a stored mode means somebody asked for it. "manual" is never a +// default, so it was always deliberate and always stands. +function chosenMode(parsed) { + if (!MODES.includes(parsed?.mode)) { + return null; + } + if (parsed.schema !== SCHEMA && parsed.mode === "ask") { + return null; + } + return parsed.mode; +} + async function writeRouting(state) { // Sessions accumulate forever otherwise — one per host window, ever. const sessions = Object.entries(state.sessions) @@ -151,11 +180,16 @@ async function writeRouting(state) { .slice(0, MAX_SESSIONS); const file = routingFilePath(); await mkdir(path.dirname(file), { recursive: true }); + const { mode, ...rest } = state; await writeFile( file, `${JSON.stringify( { - ...state, + ...rest, + // Written down only when somebody chose it. An unchosen mode stays out + // of the file entirely, so this machine keeps following the default + // rather than pinning whichever one this build happens to ship. + ...(MODES.includes(mode) ? { mode } : {}), sessions: Object.fromEntries(sessions), declines: pruneDeclines(state.declines, Date.now()), decisions: state.decisions.slice(-MAX_DECISIONS) @@ -232,7 +266,12 @@ export function isCardStale(card, source) { // auto, another window writing code wants to be left alone. export function resolveMode(state, id) { const session = id ? state.sessions[id] : null; - return MODES.includes(session?.mode) ? session.mode : state.mode; + if (MODES.includes(session?.mode)) { + return session.mode; + } + // `state.mode` is null on a machine where nobody has set one, which is what + // lets the default below actually apply. + return MODES.includes(state.mode) ? state.mode : DEFAULT_MODE; } export function setMode(mode, { global: isGlobal = false, id = sessionId() } = {}) { @@ -422,23 +461,43 @@ export function renderMenu(entries, { connectedId, mode } = {}) { lines.push(`- **${entry.name}**${marker} — ${describe(entry)}`); } lines.push(""); - lines.push(...routingInstructions(mode)); + lines.push(...routingInstructions(mode, Boolean(connectedId))); return lines.join("\n"); } // Shared with the shortlist below, because a shortlist is still a menu: the // same model still decides, still asks first in ask mode, and still must not // route on a follow-up. Only the number of things it chooses between differs. -function routingInstructions(mode) { +// +// Split on whether anything is connected, because the two situations are not +// the same move. Switching means leaving somewhere, and every guard here — +// "clearly belongs", "not on a follow-up", "stands on its own" — exists to make +// leaving cost something. A session grounded in nothing has nowhere to leave +// from: the same guards read as reasons to do nothing at all, which is how a +// question that plainly belonged to a saved context ended up answered from +// general knowledge with a slash command offered as consolation. +function routingInstructions(mode, connected) { return [ - mode === "auto" - ? "Routing is on (auto). When the user's request clearly belongs to one of the other contexts above, switch to it with the `use_context` tool, then call `get_context` and answer from what it returns. Say in one line that you switched, and which context you are now on. When two contexts are both plausible, do not guess — name them and ask which one." - : "Routing is on (ask). When the user's request clearly belongs to one of the other contexts above, say so and ask before switching — never switch first. If they agree, call `use_context`, then `get_context`, and answer from what it returns.", - "Do not route on follow-ups, short replies, or anything that continues the current topic — a switch needs a request that stands on its own and plainly belongs elsewhere. If the user declines a switch, drop it and do not raise that context again this session.", + connected ? switchInstruction(mode) : connectInstruction(mode), + connected + ? "Do not route on follow-ups, short replies, or anything that continues the current topic — a switch needs a request that stands on its own and plainly belongs elsewhere. If the user declines a switch, drop it and do not raise that context again this session." + : "There is no current topic to continue and nothing to leave, so connecting the context a request belongs to is the expected move rather than an interruption. If the user declines one, drop it and do not raise that context again this session.", "When the user corrects a wrong route, pass what they called it as `alias` to `use_context` so the same words route correctly next time." ]; } +function switchInstruction(mode) { + return mode === "auto" + ? "Routing is on (auto). When the user's request clearly belongs to one of the other contexts above, switch to it with the `use_context` tool, then call `get_context` and answer from what it returns. Say in one line that you switched, and which context you are now on. When two contexts are both plausible, do not guess — name them and ask which one." + : "Routing is on (ask). When the user's request clearly belongs to one of the other contexts above, say so and ask before switching — never switch first. If they agree, call `use_context`, then `get_context`, and answer from what it returns."; +} + +function connectInstruction(mode) { + return mode === "auto" + ? "Routing is on (auto), and this session is grounded in nothing yet. When the user's request belongs to one of the contexts above, connect it with the `use_context` tool, then call `get_context` and answer from what it returns. Do that yourself — do not ask the user to run a command to connect a context you can already name. Say in one line which context you connected. When two contexts are both plausible, do not guess — name them and ask which one." + : "Routing is on (ask), and this session is grounded in nothing yet. When the user's request belongs to one of the contexts above, name it and ask whether to connect it — never connect first. If they agree, call `use_context`, then `get_context`, and answer from what it returns."; +} + // The same menu, cut down to what the request actually reached. // // Two things change against the full list. It is short, so each entry can @@ -461,13 +520,15 @@ export function renderShortlist(entries, { connectedId, mode, decision } = {}) { } lines.push(""); lines.push( - "These are the contexts on this machine whose own description matched the request, best first. Others exist and did not match — that is a reason to stay where you are, not to reach for the closest one here." + connectedId + ? "These are the contexts on this machine whose own description matched the request, best first. Others exist and did not match — that is a reason to stay where you are, not to reach for the closest one here." + : "These are the contexts on this machine whose own description matched the request, best first. Others exist and did not match — so if none of these covers the request, say the store does not have it rather than reaching for the closest one here." ); const tie = tieNote(decision); if (tie) { lines.push(tie); } - lines.push(...routingInstructions(mode)); + lines.push(...routingInstructions(mode, Boolean(connectedId))); return lines.join("\n"); } @@ -486,7 +547,7 @@ function tieNote(decision) { return ( `${names} match the request about equally well, so which one is right is not something to ` + "decide on the user's behalf. Name them, say in one line what each covers, and ask which — " + - "in auto mode too. Switch only once they have answered." + "in auto mode too. Call `use_context` only once they have answered." ); } diff --git a/plugins/kimi-code/neatcontext/src/kimi/mcp-bridge.mjs b/plugins/kimi-code/neatcontext/src/kimi/mcp-bridge.mjs index 45ec472..445410e 100644 --- a/plugins/kimi-code/neatcontext/src/kimi/mcp-bridge.mjs +++ b/plugins/kimi-code/neatcontext/src/kimi/mcp-bridge.mjs @@ -90,11 +90,37 @@ const GET_CONTEXT_TOOL = { // there is nothing to connect. const NOTHING_CONNECTED_HEAD = "No NeatContext Context is connected to this session."; +// The manual-mode version, and the fallback whenever no menu follows. Routing +// is off here, so a command the user types is genuinely the only way forward. const NOTHING_CONNECTED = `${NOTHING_CONNECTED_HEAD} Connect one with \`/neatcontext:use\`, save this conversation as ` + "a new one with `/neatcontext:save`, or create one from a folder of documents with " + "`/neatcontext:create`. Until then, do not answer from general knowledge."; +// What to say instead when routing is on and there are contexts to route to. +// +// Leading with `/neatcontext:use` in that situation is what made routing look +// broken. This text is the first and most imperative thing the model reads, and +// it answered "what now?" with a command for the user to type before the menu +// below ever got a turn — so a question that plainly belonged to a saved +// context came back as an offer to go and connect one by hand. The menu is +// still what carries the mode-specific rules; these two only have to stop +// contradicting it. +const NOTHING_CONNECTED_ROUTABLE = + `${NOTHING_CONNECTED_HEAD} There are contexts on this machine, listed below with what each ` + + "one is for. Connect the one this request belongs to with `use_context`, then call " + + "`get_context` again and answer from what it returns — do not ask the user to run a command " + + "to connect a context you can already name. If none of them covers the request, say so and " + + "offer `/neatcontext:save` to make one out of this conversation. Until then, do not answer " + + "from general knowledge."; + +const NOTHING_CONNECTED_ASK = + `${NOTHING_CONNECTED_HEAD} There are contexts on this machine, listed below with what each ` + + "one is for. Routing is in ask mode, so name the one this request belongs to and ask whether " + + "to connect it rather than connecting first. If none of them covers the request, say so and " + + "offer `/neatcontext:save` to make one out of this conversation. Until then, do not answer " + + "from general knowledge."; + const NOTHING_EXISTS = `${NOTHING_CONNECTED_HEAD} There are none on this machine yet, so \`/neatcontext:use\` has ` + "nothing to list. Save the work in this conversation as the first one with " + @@ -107,7 +133,7 @@ const CONNECTION_RULE = `## Connecting a context, in Kimi Code Contexts are connected from this session and nowhere else: the \`use_context\` tool, or \`/neatcontext:use \` run by the user. \`/neatcontext:disconnect\` disconnects the current one from this session. New ones are made from here too: \`/neatcontext:save\` turns the work in this conversation into one, and \`/neatcontext:create\` builds one around a folder of documents the user already has. -There is no Desktop connection right now. Contexts are stored by this plugin. When the connected context is the wrong one, or none is connected, name the one you need and offer to switch to it here.`; +There is no Desktop connection right now. Contexts are stored by this plugin. When the connected context is the wrong one, or none is connected, name the one you need and connect it here with \`use_context\` — or offer to, when the routing rules above say to ask first.`; // The two tools that let a session change what it is grounded in. They are the // plugin's whole routing mechanism: there is no model in any process here, so @@ -194,7 +220,7 @@ These instructions are fixed at the handshake and cannot be updated, so they are When the user asks anything that depends on their own domain, documents, tools, or team conventions, call the get_context tool and let its answer decide: - If it returns a Context, ground your answer in it and cite what you used. -- Only if it reports that nothing is connected, say so, and offer the way forward it names — connecting an existing context with /neatcontext:use, saving this conversation as a new one with /neatcontext:save, or building one from a folder of documents with /neatcontext:create. Which of those actually applies depends on what exists right now, so relay what the tool says rather than guessing from this text.`; +- If it reports that nothing is connected, it also lists the contexts that exist and says what to do about them — which may be to connect one yourself with the use_context tool, to ask the user first, or to tell them to run a command. Do what that answer says. It knows the current state and this text does not, so never substitute a slash command of your own for the route it offers.`; const BINDING_RULE = "NeatContext has not yet bound its MCP bridge to this Kimi Code session. Call " + @@ -254,7 +280,17 @@ async function listAllContexts() { // they have none. async function nothingConnectedText() { const { contexts } = await listAllContexts().catch(() => ({ contexts: [] })); - return contexts.length === 0 ? NOTHING_EXISTS : NOTHING_CONNECTED; + if (contexts.length === 0) { + return NOTHING_EXISTS; + } + // The mode decides whether a menu is about to follow this text, and therefore + // whether pointing at a slash command is the honest answer or the one that + // breaks routing. + const mode = resolveMode(await readRouting().catch(() => ({ sessions: {} })), sessionId()); + if (mode === "manual") { + return NOTHING_CONNECTED; + } + return mode === "ask" ? NOTHING_CONNECTED_ASK : NOTHING_CONNECTED_ROUTABLE; } // The selected context, or null when nothing is selected. A selection diff --git a/plugins/pi/neatcontext/src/core/routing.mjs b/plugins/pi/neatcontext/src/core/routing.mjs index bf5675e..5c8da96 100644 --- a/plugins/pi/neatcontext/src/core/routing.mjs +++ b/plugins/pi/neatcontext/src/core/routing.mjs @@ -41,7 +41,9 @@ export const MODES = ["auto", "ask", "manual"]; // Asking every time then costs a question per turn and buys nothing. export const DEFAULT_MODE = "auto"; -const SCHEMA = 1; +// 2 marks the file as one where a stored mode means somebody chose it. See +// `chosenMode` for what schema 1 got wrong and why it cannot be read literally. +const SCHEMA = 2; const MAX_USE_WHEN = 240; const MAX_ALIASES = 12; const MAX_DECISIONS = 100; @@ -137,13 +139,40 @@ export async function readRouting() { return { schema: SCHEMA, declines, - mode: MODES.includes(parsed?.mode) ? parsed.mode : DEFAULT_MODE, + mode: chosenMode(parsed), cards, sessions: typeof parsed?.sessions === "object" && parsed.sessions !== null ? parsed.sessions : {}, decisions: Array.isArray(parsed?.decisions) ? parsed.decisions : [] }; } +// The mode the user actually chose, or null when nobody has. +// +// Null is not a synonym for the default, and the difference is the whole point. +// Until schema 2 the *resolved* mode was written back on every routing write — +// deriving a card, logging a decision, noting a refusal — so a file ended up +// stating whatever the default happened to be in the build that last touched +// it. "ask" was that default until the shortlist learned to ask on its own, so +// every machine that had ever saved a context had "ask" written down, and +// changing the default to "auto" reached none of them. Routing looked switched +// off on exactly the machines that used it most, and reinstalling did not help: +// this file lives in ~/.neatcontext and outlives any one plugin install. +// +// So an "ask" in a pre-schema-2 file is not evidence of a choice, and is +// dropped. A machine where it genuinely was one loses it once, and a single +// `/neatcontext:mode ask --global` puts it back — written under schema 2 this +// time, where a stored mode means somebody asked for it. "manual" is never a +// default, so it was always deliberate and always stands. +function chosenMode(parsed) { + if (!MODES.includes(parsed?.mode)) { + return null; + } + if (parsed.schema !== SCHEMA && parsed.mode === "ask") { + return null; + } + return parsed.mode; +} + async function writeRouting(state) { // Sessions accumulate forever otherwise — one per host window, ever. const sessions = Object.entries(state.sessions) @@ -151,11 +180,16 @@ async function writeRouting(state) { .slice(0, MAX_SESSIONS); const file = routingFilePath(); await mkdir(path.dirname(file), { recursive: true }); + const { mode, ...rest } = state; await writeFile( file, `${JSON.stringify( { - ...state, + ...rest, + // Written down only when somebody chose it. An unchosen mode stays out + // of the file entirely, so this machine keeps following the default + // rather than pinning whichever one this build happens to ship. + ...(MODES.includes(mode) ? { mode } : {}), sessions: Object.fromEntries(sessions), declines: pruneDeclines(state.declines, Date.now()), decisions: state.decisions.slice(-MAX_DECISIONS) @@ -232,7 +266,12 @@ export function isCardStale(card, source) { // auto, another window writing code wants to be left alone. export function resolveMode(state, id) { const session = id ? state.sessions[id] : null; - return MODES.includes(session?.mode) ? session.mode : state.mode; + if (MODES.includes(session?.mode)) { + return session.mode; + } + // `state.mode` is null on a machine where nobody has set one, which is what + // lets the default below actually apply. + return MODES.includes(state.mode) ? state.mode : DEFAULT_MODE; } export function setMode(mode, { global: isGlobal = false, id = sessionId() } = {}) { @@ -422,23 +461,43 @@ export function renderMenu(entries, { connectedId, mode } = {}) { lines.push(`- **${entry.name}**${marker} — ${describe(entry)}`); } lines.push(""); - lines.push(...routingInstructions(mode)); + lines.push(...routingInstructions(mode, Boolean(connectedId))); return lines.join("\n"); } // Shared with the shortlist below, because a shortlist is still a menu: the // same model still decides, still asks first in ask mode, and still must not // route on a follow-up. Only the number of things it chooses between differs. -function routingInstructions(mode) { +// +// Split on whether anything is connected, because the two situations are not +// the same move. Switching means leaving somewhere, and every guard here — +// "clearly belongs", "not on a follow-up", "stands on its own" — exists to make +// leaving cost something. A session grounded in nothing has nowhere to leave +// from: the same guards read as reasons to do nothing at all, which is how a +// question that plainly belonged to a saved context ended up answered from +// general knowledge with a slash command offered as consolation. +function routingInstructions(mode, connected) { return [ - mode === "auto" - ? "Routing is on (auto). When the user's request clearly belongs to one of the other contexts above, switch to it with the `use_context` tool, then call `get_context` and answer from what it returns. Say in one line that you switched, and which context you are now on. When two contexts are both plausible, do not guess — name them and ask which one." - : "Routing is on (ask). When the user's request clearly belongs to one of the other contexts above, say so and ask before switching — never switch first. If they agree, call `use_context`, then `get_context`, and answer from what it returns.", - "Do not route on follow-ups, short replies, or anything that continues the current topic — a switch needs a request that stands on its own and plainly belongs elsewhere. If the user declines a switch, drop it and do not raise that context again this session.", + connected ? switchInstruction(mode) : connectInstruction(mode), + connected + ? "Do not route on follow-ups, short replies, or anything that continues the current topic — a switch needs a request that stands on its own and plainly belongs elsewhere. If the user declines a switch, drop it and do not raise that context again this session." + : "There is no current topic to continue and nothing to leave, so connecting the context a request belongs to is the expected move rather than an interruption. If the user declines one, drop it and do not raise that context again this session.", "When the user corrects a wrong route, pass what they called it as `alias` to `use_context` so the same words route correctly next time." ]; } +function switchInstruction(mode) { + return mode === "auto" + ? "Routing is on (auto). When the user's request clearly belongs to one of the other contexts above, switch to it with the `use_context` tool, then call `get_context` and answer from what it returns. Say in one line that you switched, and which context you are now on. When two contexts are both plausible, do not guess — name them and ask which one." + : "Routing is on (ask). When the user's request clearly belongs to one of the other contexts above, say so and ask before switching — never switch first. If they agree, call `use_context`, then `get_context`, and answer from what it returns."; +} + +function connectInstruction(mode) { + return mode === "auto" + ? "Routing is on (auto), and this session is grounded in nothing yet. When the user's request belongs to one of the contexts above, connect it with the `use_context` tool, then call `get_context` and answer from what it returns. Do that yourself — do not ask the user to run a command to connect a context you can already name. Say in one line which context you connected. When two contexts are both plausible, do not guess — name them and ask which one." + : "Routing is on (ask), and this session is grounded in nothing yet. When the user's request belongs to one of the contexts above, name it and ask whether to connect it — never connect first. If they agree, call `use_context`, then `get_context`, and answer from what it returns."; +} + // The same menu, cut down to what the request actually reached. // // Two things change against the full list. It is short, so each entry can @@ -461,13 +520,15 @@ export function renderShortlist(entries, { connectedId, mode, decision } = {}) { } lines.push(""); lines.push( - "These are the contexts on this machine whose own description matched the request, best first. Others exist and did not match — that is a reason to stay where you are, not to reach for the closest one here." + connectedId + ? "These are the contexts on this machine whose own description matched the request, best first. Others exist and did not match — that is a reason to stay where you are, not to reach for the closest one here." + : "These are the contexts on this machine whose own description matched the request, best first. Others exist and did not match — so if none of these covers the request, say the store does not have it rather than reaching for the closest one here." ); const tie = tieNote(decision); if (tie) { lines.push(tie); } - lines.push(...routingInstructions(mode)); + lines.push(...routingInstructions(mode, Boolean(connectedId))); return lines.join("\n"); } @@ -486,7 +547,7 @@ function tieNote(decision) { return ( `${names} match the request about equally well, so which one is right is not something to ` + "decide on the user's behalf. Name them, say in one line what each covers, and ask which — " + - "in auto mode too. Switch only once they have answered." + "in auto mode too. Call `use_context` only once they have answered." ); } diff --git a/plugins/pi/neatcontext/src/pi/runtime.mjs b/plugins/pi/neatcontext/src/pi/runtime.mjs index 74706fb..049218f 100644 --- a/plugins/pi/neatcontext/src/pi/runtime.mjs +++ b/plugins/pi/neatcontext/src/pi/runtime.mjs @@ -73,19 +73,62 @@ import { export const PLUGIN_VERSION = "0.1.0"; -// The one thing to say when a session has nothing to ground in. It is -// deliberately about what to do in the current session. +// What to say when a session has nothing to ground in. It is deliberately about +// what to do in the current session. +// +// This one is for when routing cannot help: manual mode, or a store with +// nothing in it yet. A command the user types is then genuinely the only way +// forward. export const NOTHING_CONNECTED = "No NeatContext Context is connected to this session. Connect one with " + "`/neatcontext-use`, save this conversation with `/neatcontext-save`, or create one " + "with `/neatcontext-create`. Until then, do not answer from general knowledge."; +// What to say instead when routing is on and there are contexts to route to. +// +// Leading with `/neatcontext-use` in that situation is what made routing look +// broken. This text is the first and most imperative thing the model reads, and +// it answered "what now?" with a command for the user to type before the menu +// below ever got a turn — so a question that plainly belonged to a saved +// context came back as an offer to go and connect one by hand. The menu is +// still what carries the mode-specific rules; these two only have to stop +// contradicting it. +export const NOTHING_CONNECTED_ROUTABLE = + "No NeatContext Context is connected to this session. There are contexts on this machine, " + + "listed below with what each one is for. Connect the one this request belongs to with " + + "`use_context`, then call `get_context` again and answer from what it returns — do not ask " + + "the user to run a command to connect a context you can already name. If none of them covers " + + "the request, say so and offer `/neatcontext-save` to make one out of this conversation. " + + "Until then, do not answer from general knowledge."; + +export const NOTHING_CONNECTED_ASK = + "No NeatContext Context is connected to this session. There are contexts on this machine, " + + "listed below with what each one is for. Routing is in ask mode, so name the one this request " + + "belongs to and ask whether to connect it rather than connecting first. If none of them " + + "covers the request, say so and offer `/neatcontext-save` to make one out of this " + + "conversation. Until then, do not answer from general knowledge."; + +// The mode decides whether a menu is about to follow this text, and therefore +// whether pointing at a slash command is the honest answer or the one that +// breaks routing. +export async function nothingConnectedText() { + const { contexts } = await listAllContexts().catch(() => ({ contexts: [] })); + if (contexts.length === 0) { + return NOTHING_CONNECTED; + } + const mode = resolveMode(await readRouting().catch(() => ({ sessions: {} })), sessionId()); + if (mode === "manual") { + return NOTHING_CONNECTED; + } + return mode === "ask" ? NOTHING_CONNECTED_ASK : NOTHING_CONNECTED_ROUTABLE; +} + // How connecting works in pi. const CONNECTION_RULE = `## Connecting a context, in pi Contexts are connected from this session and nowhere else: the \`use_context\` tool, or \`/neatcontext-use \` run by the user. \`/neatcontext-disconnect\` disconnects the current one from this session. \`/neatcontext-save\` saves the current conversation and \`/neatcontext-create\` makes one from a knowledge folder. -There is no Desktop connection right now. Contexts are stored by this plugin. When the connected context is the wrong one, or none is connected, name the one you need and offer to switch to it here.`; +There is no Desktop connection right now. Contexts are stored by this plugin. When the connected context is the wrong one, or none is connected, name the one you need and connect it here with \`use_context\` — or offer to, when the routing rules above say to ask first.`; const CONTEXT_INSTRUCTIONS = `This session can be grounded in a NeatContext Context: one domain profile and local knowledge stored on this machine. @@ -102,7 +145,7 @@ const NO_CONTEXT_INSTRUCTIONS = `No NeatContext Context is connected to this ses When the user asks anything that depends on their own domain, documents, tools, or team conventions, call the get_context tool and let its answer decide: - If it returns a Context, ground your answer in it and cite what you used. -- Only if it reports that nothing is connected, say so, and tell them to connect one with /neatcontext-use, save this conversation with /neatcontext-save, or create one with /neatcontext-create.`; +- If it reports that nothing is connected, it also lists the contexts that exist and says what to do about them — which may be to connect one yourself with the use_context tool, to ask the user first, or to tell them to run a command. Do what that answer says, rather than substituting a slash command of your own for the route it offers.`; // --- which source serves this session ---------------------------------------- @@ -320,7 +363,7 @@ export async function getContext(query) { return `${parts.join("\n\n")}\n\n${await pluginNotes(query)}`; } await resolveExtensions(null); - return `${NOTHING_CONNECTED}\n\n${await pluginNotes(query)}`; + return `${await nothingConnectedText()}\n\n${await pluginNotes(query)}`; } // --- routing tools ------------------------------------------------------------ diff --git a/plugins/pi/neatcontext/tests/pi-runtime.test.mjs b/plugins/pi/neatcontext/tests/pi-runtime.test.mjs index 9d384fa..3d8bb16 100644 --- a/plugins/pi/neatcontext/tests/pi-runtime.test.mjs +++ b/plugins/pi/neatcontext/tests/pi-runtime.test.mjs @@ -112,6 +112,25 @@ describe("local Contexts", () => { assert.doesNotMatch(context, /\$neatcontext:/); }); + // With contexts to route to, a slash command is the wrong lead: it answers + // "what now?" before the menu below it gets a turn, and the model acts on it. + it("tells an ungrounded session to connect a context itself", async () => { + await createOrders(); + const context = await runtime.getContext("order fulfillment"); + assert.match(context, /Connect the one this request belongs to with `use_context`/); + assert.match(context, /do not ask the user to run a command/); + }); + + // Manual mode publishes no menu, so there is nothing for the session to + // connect from and the command really is the only way forward. + it("falls back to the commands when routing is off", async () => { + await createOrders(); + await runtime.commandMode("manual"); + const context = await runtime.getContext("order fulfillment"); + assert.match(context, /Connect one with `\/neatcontext-use`/); + assert.doesNotMatch(context, /## Contexts available on this machine/); + }); + it("refuses a knowledge folder that is not there", async () => { const result = await runtime.createContext({ name: "Ghost", diff --git a/shared/core/routing.mjs b/shared/core/routing.mjs index bf5675e..5c8da96 100644 --- a/shared/core/routing.mjs +++ b/shared/core/routing.mjs @@ -41,7 +41,9 @@ export const MODES = ["auto", "ask", "manual"]; // Asking every time then costs a question per turn and buys nothing. export const DEFAULT_MODE = "auto"; -const SCHEMA = 1; +// 2 marks the file as one where a stored mode means somebody chose it. See +// `chosenMode` for what schema 1 got wrong and why it cannot be read literally. +const SCHEMA = 2; const MAX_USE_WHEN = 240; const MAX_ALIASES = 12; const MAX_DECISIONS = 100; @@ -137,13 +139,40 @@ export async function readRouting() { return { schema: SCHEMA, declines, - mode: MODES.includes(parsed?.mode) ? parsed.mode : DEFAULT_MODE, + mode: chosenMode(parsed), cards, sessions: typeof parsed?.sessions === "object" && parsed.sessions !== null ? parsed.sessions : {}, decisions: Array.isArray(parsed?.decisions) ? parsed.decisions : [] }; } +// The mode the user actually chose, or null when nobody has. +// +// Null is not a synonym for the default, and the difference is the whole point. +// Until schema 2 the *resolved* mode was written back on every routing write — +// deriving a card, logging a decision, noting a refusal — so a file ended up +// stating whatever the default happened to be in the build that last touched +// it. "ask" was that default until the shortlist learned to ask on its own, so +// every machine that had ever saved a context had "ask" written down, and +// changing the default to "auto" reached none of them. Routing looked switched +// off on exactly the machines that used it most, and reinstalling did not help: +// this file lives in ~/.neatcontext and outlives any one plugin install. +// +// So an "ask" in a pre-schema-2 file is not evidence of a choice, and is +// dropped. A machine where it genuinely was one loses it once, and a single +// `/neatcontext:mode ask --global` puts it back — written under schema 2 this +// time, where a stored mode means somebody asked for it. "manual" is never a +// default, so it was always deliberate and always stands. +function chosenMode(parsed) { + if (!MODES.includes(parsed?.mode)) { + return null; + } + if (parsed.schema !== SCHEMA && parsed.mode === "ask") { + return null; + } + return parsed.mode; +} + async function writeRouting(state) { // Sessions accumulate forever otherwise — one per host window, ever. const sessions = Object.entries(state.sessions) @@ -151,11 +180,16 @@ async function writeRouting(state) { .slice(0, MAX_SESSIONS); const file = routingFilePath(); await mkdir(path.dirname(file), { recursive: true }); + const { mode, ...rest } = state; await writeFile( file, `${JSON.stringify( { - ...state, + ...rest, + // Written down only when somebody chose it. An unchosen mode stays out + // of the file entirely, so this machine keeps following the default + // rather than pinning whichever one this build happens to ship. + ...(MODES.includes(mode) ? { mode } : {}), sessions: Object.fromEntries(sessions), declines: pruneDeclines(state.declines, Date.now()), decisions: state.decisions.slice(-MAX_DECISIONS) @@ -232,7 +266,12 @@ export function isCardStale(card, source) { // auto, another window writing code wants to be left alone. export function resolveMode(state, id) { const session = id ? state.sessions[id] : null; - return MODES.includes(session?.mode) ? session.mode : state.mode; + if (MODES.includes(session?.mode)) { + return session.mode; + } + // `state.mode` is null on a machine where nobody has set one, which is what + // lets the default below actually apply. + return MODES.includes(state.mode) ? state.mode : DEFAULT_MODE; } export function setMode(mode, { global: isGlobal = false, id = sessionId() } = {}) { @@ -422,23 +461,43 @@ export function renderMenu(entries, { connectedId, mode } = {}) { lines.push(`- **${entry.name}**${marker} — ${describe(entry)}`); } lines.push(""); - lines.push(...routingInstructions(mode)); + lines.push(...routingInstructions(mode, Boolean(connectedId))); return lines.join("\n"); } // Shared with the shortlist below, because a shortlist is still a menu: the // same model still decides, still asks first in ask mode, and still must not // route on a follow-up. Only the number of things it chooses between differs. -function routingInstructions(mode) { +// +// Split on whether anything is connected, because the two situations are not +// the same move. Switching means leaving somewhere, and every guard here — +// "clearly belongs", "not on a follow-up", "stands on its own" — exists to make +// leaving cost something. A session grounded in nothing has nowhere to leave +// from: the same guards read as reasons to do nothing at all, which is how a +// question that plainly belonged to a saved context ended up answered from +// general knowledge with a slash command offered as consolation. +function routingInstructions(mode, connected) { return [ - mode === "auto" - ? "Routing is on (auto). When the user's request clearly belongs to one of the other contexts above, switch to it with the `use_context` tool, then call `get_context` and answer from what it returns. Say in one line that you switched, and which context you are now on. When two contexts are both plausible, do not guess — name them and ask which one." - : "Routing is on (ask). When the user's request clearly belongs to one of the other contexts above, say so and ask before switching — never switch first. If they agree, call `use_context`, then `get_context`, and answer from what it returns.", - "Do not route on follow-ups, short replies, or anything that continues the current topic — a switch needs a request that stands on its own and plainly belongs elsewhere. If the user declines a switch, drop it and do not raise that context again this session.", + connected ? switchInstruction(mode) : connectInstruction(mode), + connected + ? "Do not route on follow-ups, short replies, or anything that continues the current topic — a switch needs a request that stands on its own and plainly belongs elsewhere. If the user declines a switch, drop it and do not raise that context again this session." + : "There is no current topic to continue and nothing to leave, so connecting the context a request belongs to is the expected move rather than an interruption. If the user declines one, drop it and do not raise that context again this session.", "When the user corrects a wrong route, pass what they called it as `alias` to `use_context` so the same words route correctly next time." ]; } +function switchInstruction(mode) { + return mode === "auto" + ? "Routing is on (auto). When the user's request clearly belongs to one of the other contexts above, switch to it with the `use_context` tool, then call `get_context` and answer from what it returns. Say in one line that you switched, and which context you are now on. When two contexts are both plausible, do not guess — name them and ask which one." + : "Routing is on (ask). When the user's request clearly belongs to one of the other contexts above, say so and ask before switching — never switch first. If they agree, call `use_context`, then `get_context`, and answer from what it returns."; +} + +function connectInstruction(mode) { + return mode === "auto" + ? "Routing is on (auto), and this session is grounded in nothing yet. When the user's request belongs to one of the contexts above, connect it with the `use_context` tool, then call `get_context` and answer from what it returns. Do that yourself — do not ask the user to run a command to connect a context you can already name. Say in one line which context you connected. When two contexts are both plausible, do not guess — name them and ask which one." + : "Routing is on (ask), and this session is grounded in nothing yet. When the user's request belongs to one of the contexts above, name it and ask whether to connect it — never connect first. If they agree, call `use_context`, then `get_context`, and answer from what it returns."; +} + // The same menu, cut down to what the request actually reached. // // Two things change against the full list. It is short, so each entry can @@ -461,13 +520,15 @@ export function renderShortlist(entries, { connectedId, mode, decision } = {}) { } lines.push(""); lines.push( - "These are the contexts on this machine whose own description matched the request, best first. Others exist and did not match — that is a reason to stay where you are, not to reach for the closest one here." + connectedId + ? "These are the contexts on this machine whose own description matched the request, best first. Others exist and did not match — that is a reason to stay where you are, not to reach for the closest one here." + : "These are the contexts on this machine whose own description matched the request, best first. Others exist and did not match — so if none of these covers the request, say the store does not have it rather than reaching for the closest one here." ); const tie = tieNote(decision); if (tie) { lines.push(tie); } - lines.push(...routingInstructions(mode)); + lines.push(...routingInstructions(mode, Boolean(connectedId))); return lines.join("\n"); } @@ -486,7 +547,7 @@ function tieNote(decision) { return ( `${names} match the request about equally well, so which one is right is not something to ` + "decide on the user's behalf. Name them, say in one line what each covers, and ask which — " + - "in auto mode too. Switch only once they have answered." + "in auto mode too. Call `use_context` only once they have answered." ); } diff --git a/tests/copilot-plugin.test.mjs b/tests/copilot-plugin.test.mjs index e7e7734..534857d 100644 --- a/tests/copilot-plugin.test.mjs +++ b/tests/copilot-plugin.test.mjs @@ -610,8 +610,9 @@ test("Copilot MCP bridge serves Contexts and routing locally", async (t) => { const empty = await session.call(toolCall(3, "get_context")); assert.equal(empty.result.isError, false); assert.match(empty.result.content[0].text, /No NeatContext Context is connected/); - // A context exists here, so `use` is a real option and leads. - assert.match(empty.result.content[0].text, /\/neatcontext:use/); + // A context exists and routing is on, so the answer leads with the route this + // session can take itself rather than with a command for the user to type. + assert.match(empty.result.content[0].text, /Connect the one this request belongs to/); // Ask mode refuses an unrequested switch. It is set explicitly rather than // assumed, so this keeps testing ask mode whatever the default becomes. diff --git a/tests/kimi-plugin.test.mjs b/tests/kimi-plugin.test.mjs index 732e0f6..e30d025 100644 --- a/tests/kimi-plugin.test.mjs +++ b/tests/kimi-plugin.test.mjs @@ -588,4 +588,55 @@ test("Kimi narrows the routing menu to the request", async (t) => { ); assert.match(unmatched.result.content[0].text, /## Contexts available on this machine/); assert.match(unmatched.result.content[0].text, /Docker container/); + + // Nothing is connected in this session, which is the case routing exists for. + // Leading with a slash command there is what made routing look broken: it is + // the first thing the model reads and it answers "what now?" before the menu + // below it gets a turn. + assert.match(narrowed, /Connect the one this request belongs to with `use_context`/); + assert.match(narrowed, /do not ask the user to run a command/); +}); + +// Manual mode publishes no menu, so there is nothing for the session to connect +// from and the command really is the only way forward. +test("Kimi falls back to the commands when routing is off", async (t) => { + const home = await localHome("neatcontext-kimi-manual-"); + const sessions = []; + t.after(async () => { + await Promise.all(sessions.map((session) => session.close())); + await rm(home.directory, { recursive: true, force: true }); + }); + + await createLocalContext(home, "kimi-manual", "Refunds", "refunds and chargebacks"); + await runNode(cli, ["--session-id", "kimi-manual", "mode", "manual"], { env: home.env }); + + const session = rpcSession(home.env); + sessions.push(session); + await session.call(initialize(1)); + await session.call(toolCall(2, "bind_session", { session_id: "kimi-manual" })); + + const text = (await session.call(toolCall(3, "get_context", { query: "refunds" }))).result + .content[0].text; + assert.match(text, /Connect one with `\/neatcontext:use`/); + assert.doesNotMatch(text, /## Contexts/); +}); + +// The cold start: bound, routing on, but nothing saved yet. `use` has nothing +// to list, so the answer has to lead with save rather than with the menu. +test("Kimi empty store points at save", async (t) => { + const home = await localHome("neatcontext-kimi-empty-"); + const sessions = []; + t.after(async () => { + await Promise.all(sessions.map((session) => session.close())); + await rm(home.directory, { recursive: true, force: true }); + }); + + const session = rpcSession(home.env); + sessions.push(session); + await session.call(initialize(1)); + await session.call(toolCall(2, "bind_session", { session_id: "kimi-empty" })); + + const text = (await session.call(toolCall(3, "get_context"))).result.content[0].text; + assert.match(text, /nothing to list/); + assert.match(text, /\/neatcontext:save/); }); diff --git a/tests/routing-confidence.test.mjs b/tests/routing-confidence.test.mjs index 8f68b59..69bedf9 100644 --- a/tests/routing-confidence.test.mjs +++ b/tests/routing-confidence.test.mjs @@ -173,7 +173,9 @@ describe("renderShortlist with a decision", () => { it("overrides auto explicitly, since auto is where the mistake would go unseen", () => { const text = routing.renderShortlist(entries, { mode: "auto", decision: assess(entries) }); assert.match(text, /in auto mode too/); - assert.match(text, /Switch only once they have answered/); + // Worded for both situations: a near-tie is a question whether the session + // would be switching contexts or connecting its first one. + assert.match(text, /Call `use_context` only once they have answered/); }); it("says nothing about ties when there is a clear leader", () => { diff --git a/tests/routing-shortlist.test.mjs b/tests/routing-shortlist.test.mjs index 613156e..c3e024a 100644 --- a/tests/routing-shortlist.test.mjs +++ b/tests/routing-shortlist.test.mjs @@ -140,12 +140,30 @@ describe("renderShortlist", () => { }); it("carries the same routing rules as the full menu", () => { - const text = routing.renderShortlist(entries, { mode: "ask" }); + const text = routing.renderShortlist(entries, { mode: "ask", connectedId: "b" }); assert.match(text, /Routing is on \(ask\)/); assert.match(text, /Do not route on follow-ups/); assert.match(text, /pass what they called it as `alias`/); }); + // The follow-up guard exists to make *leaving* a context cost something. With + // nothing connected there is nowhere to leave from, and the same sentence + // reads as a reason to do nothing at all — which is how a request that + // plainly belonged to a saved context got answered from general knowledge. + it("drops the follow-up guard when the session is grounded in nothing", () => { + const text = routing.renderShortlist(entries, { mode: "auto" }); + assert.doesNotMatch(text, /Do not route on follow-ups/); + assert.match(text, /nothing to leave/); + assert.match(text, /connect it with the `use_context` tool/); + assert.match(text, /do not ask the user to run a command/); + }); + + it("still asks first in ask mode when nothing is connected", () => { + const text = routing.renderShortlist(entries, { mode: "ask" }); + assert.match(text, /Routing is on \(ask\)/); + assert.match(text, /never connect first/); + }); + it("uses the auto wording in auto mode", () => { assert.match(routing.renderShortlist(entries, { mode: "auto" }), /Routing is on \(auto\)/); }); diff --git a/tests/routing-unconnected.test.mjs b/tests/routing-unconnected.test.mjs new file mode 100644 index 0000000..4d57330 --- /dev/null +++ b/tests/routing-unconnected.test.mjs @@ -0,0 +1,257 @@ +// A session grounded in nothing is the case routing exists for, and it was the +// one case that could not route. +// +// Two independent faults, both reproduced from a real GitHub Copilot session +// where a question about a saved incident context came back as an offer to run +// /neatcontext:use: +// +// 1. Every machine that had ever saved a context had "ask" written into +// plugin-routing.json by a build whose default was ask. Auto became the +// default in #77 and reached none of them, and reinstalling the plugin +// does not help — that file lives in ~/.neatcontext and outlives any one +// install. +// 2. The nothing-connected text led with a slash command, which is the first +// and most imperative thing the model reads. It answered "what now?" +// before the routing menu below it ever got a turn. + +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 path from "node:path"; +import readline from "node:readline"; +import { fileURLToPath } from "node:url"; +import { after, before, beforeEach, describe, it } from "node:test"; + +const root = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); +const plugin = path.join(root, "plugins", "copilot", "neatcontext"); +const bridgeFile = path.join(plugin, "src", "copilot", "mcp-bridge.mjs"); + +let home; +let docs; + +before(async () => { + home = await mkdtemp(path.join(os.tmpdir(), "neatcontext-unconnected-test-")); + docs = path.join(home, "docs"); + await mkdir(docs, { recursive: true }); + await writeFile(path.join(docs, "incident.md"), "# Incident\n"); + process.env.NEATCONTEXT_HOME = home; +}); + +after(async () => { + await rm(home, { recursive: true, force: true }); +}); + +beforeEach(async () => { + await rm(path.join(home, "contexts"), { recursive: true, force: true }); + await rm(path.join(home, "plugin-routing.json"), { force: true }); + await rm(path.join(home, "plugin-sessions"), { recursive: true, force: true }); +}); + +// Every host ships its own copy of the core, kept byte-identical by +// `npm run sync:context`. The migration has to hold in all five: a user with +// Copilot and Claude Code installed has one ~/.neatcontext between them, and a +// copy that still read the baked-in "ask" would drag the other back. +const HOSTS = [ + ["Claude Code", "plugins/claude-code/neatcontext/src/core/routing.mjs"], + ["GitHub Copilot", "plugins/copilot/neatcontext/src/core/routing.mjs"], + ["Kimi Code", "plugins/kimi-code/neatcontext/src/core/routing.mjs"], + ["pi", "plugins/pi/neatcontext/src/core/routing.mjs"], + ["Codex", "codex-marketplace/plugins/neatcontext/src/core/routing.mjs"] +]; + +const cores = await Promise.all( + HOSTS.map(async ([name, file]) => [name, await import(`../${file}`)]) +); + +const routing = await import("../plugins/copilot/neatcontext/src/core/routing.mjs"); +const store = await import("../plugins/copilot/neatcontext/src/core/context-store.mjs"); + +const routingFile = () => path.join(home, "plugin-routing.json"); +const readRoutingFile = async () => JSON.parse(await readFile(routingFile(), "utf8")); +const writeRoutingFile = (value) => writeFile(routingFile(), `${JSON.stringify(value, null, 2)}\n`, "utf8"); + +async function create(name, useWhen) { + const { record, profileText } = await store.createContext({ + name, + knowledgeFolder: docs, + profile: `# ${name}\n\n## Purpose\n${useWhen}` + }); + await routing.putCard(record.id, { useWhen, source: profileText }); + return record; +} + +function bridge(sessionId = "unconnected-bridge") { + const child = spawn(process.execPath, [bridgeFile], { + env: { ...process.env, NEATCONTEXT_SESSION_ID: sessionId, NEATCONTEXT_HOME: home }, + stdio: ["pipe", "pipe", "inherit"], + windowsHide: true + }); + const waiters = new Map(); + readline.createInterface({ input: child.stdout }).on("line", (line) => { + const message = JSON.parse(line); + const waiter = waiters.get(message.id); + if (waiter) { + waiters.delete(message.id); + waiter(message); + } + }); + let nextId = 0; + const send = (method, params) => + new Promise((resolve) => { + nextId += 1; + waiters.set(nextId, resolve); + child.stdin.write( + `${JSON.stringify({ jsonrpc: "2.0", id: nextId, method, ...(params ? { params } : {}) })}\n` + ); + }); + return { + send, + close: async () => { + child.stdin.end(); + if (child.exitCode === null) await new Promise((resolve) => child.once("exit", resolve)); + } + }; +} + +for (const [host, core] of cores) { + describe(`a mode nobody chose (${host})`, () => { + it("does not get written down, so a later default can still reach this machine", async () => { + // Any routing write at all — deriving a card is the common one. + await create("Incident", "checkout 5xx"); + const file = await readRoutingFile(); + assert.equal( + Object.hasOwn(file, "mode"), + false, + "an unchosen mode must stay out of the file entirely" + ); + assert.equal(core.resolveMode(await core.readRouting(), "any-session"), core.DEFAULT_MODE); + }); + + it("is dropped from a schema 1 file, where it recorded the build's default", async () => { + await writeRoutingFile({ schema: 1, mode: "ask", cards: {}, sessions: {}, decisions: [] }); + const state = await core.readRouting(); + assert.equal(state.mode, null, "a schema 1 ask is not evidence of a choice"); + assert.equal(core.resolveMode(state, "any-session"), "auto"); + }); + + it("keeps manual from a schema 1 file, which was never anyone's default", async () => { + await writeRoutingFile({ schema: 1, mode: "manual", cards: {}, sessions: {}, decisions: [] }); + assert.equal(core.resolveMode(await core.readRouting(), "any-session"), "manual"); + }); + }); + + describe(`a mode somebody chose (${host})`, () => { + it("survives, and is written down so it is never mistaken for a default again", async () => { + await core.setMode("ask", { global: true, id: "chooser" }); + assert.equal((await readRoutingFile()).mode, "ask"); + assert.equal((await readRoutingFile()).schema, 2); + + // The migration must not run twice and undo the choice it just recorded. + await create("Incident", "checkout 5xx"); + assert.equal(core.resolveMode(await core.readRouting(), "chooser"), "ask"); + assert.equal((await readRoutingFile()).mode, "ask"); + }); + + it("still scopes to one session when that is what was asked for", async () => { + await core.setMode("manual", { id: "one-window" }); + const state = await core.readRouting(); + assert.equal(core.resolveMode(state, "one-window"), "manual"); + assert.equal(core.resolveMode(state, "another-window"), core.DEFAULT_MODE); + }); + }); +} + +describe("get_context with nothing connected", () => { + const ask = (session, query) => + session + .send("tools/call", { name: "get_context", arguments: query ? { query } : {} }) + .then((response) => response.result.content[0].text); + + it("routes itself rather than handing back a command to type", async () => { + await create("Incident", "checkout-api 5xx from pgbouncer pool exhaustion"); + await create("Queue lag", "order-events partition lag"); + const session = bridge("routes-itself"); + try { + await session.send("initialize", { protocolVersion: "2025-11-25" }); + const text = await ask(session, "why is checkout-api throwing 5xx?"); + + assert.match(text, /Connect the one this request belongs to with `use_context`/); + assert.match(text, /do not ask the user to run a command/); + // The regression itself: the old text opened by telling the model to send + // the user to /neatcontext:use, and that is what it acted on. + assert.doesNotMatch( + text.split("## Contexts")[0], + /\/neatcontext:use/, + "the lead paragraph must not answer 'what now?' with a slash command" + ); + assert.match(text, /Incident/); + } finally { + await session.close(); + } + }); + + it("is reachable on a machine upgraded from a pre-#77 routing file", async () => { + await create("Incident", "checkout-api 5xx from pgbouncer pool exhaustion"); + const state = await readRoutingFile(); + await writeRoutingFile({ ...state, schema: 1, mode: "ask" }); + + const session = bridge("upgraded-machine"); + try { + await session.send("initialize", { protocolVersion: "2025-11-25" }); + assert.match(await ask(session, "checkout-api 5xx"), /Routing is on \(auto\)/); + + // And the switch it was told to make actually goes through, unprompted, + // which is what the baked-in ask was refusing. + const used = await session.send("tools/call", { + name: "use_context", + arguments: { context: "Incident", reason: "checkout 5xx" } + }); + assert.equal(used.result.isError, false); + assert.match(used.result.content[0].text, /Switched this session to "Incident"/); + } finally { + await session.close(); + } + }); + + it("asks first when the user has genuinely chosen ask", async () => { + await create("Incident", "checkout-api 5xx from pgbouncer pool exhaustion"); + await routing.setMode("ask", { global: true, id: "deliberate-asker" }); + const session = bridge("deliberate-asker"); + try { + await session.send("initialize", { protocolVersion: "2025-11-25" }); + const text = await ask(session, "checkout-api 5xx"); + assert.match(text, /Routing is in ask mode/); + assert.match(text, /ask whether to connect it rather than connecting first/); + assert.doesNotMatch(text, /Connect the one this request belongs to with `use_context`/); + } finally { + await session.close(); + } + }); + + it("falls back to the commands in manual mode, where there is no menu to follow", async () => { + await create("Incident", "checkout-api 5xx from pgbouncer pool exhaustion"); + await routing.setMode("manual", { global: true, id: "manual-user" }); + const session = bridge("manual-user"); + try { + await session.send("initialize", { protocolVersion: "2025-11-25" }); + const text = await ask(session, "checkout-api 5xx"); + assert.match(text, /Connect one with `\/neatcontext:use`/); + assert.doesNotMatch(text, /## Contexts/); + } finally { + await session.close(); + } + }); + + it("still leads with save when the store is empty", async () => { + const session = bridge("empty-store"); + try { + await session.send("initialize", { protocolVersion: "2025-11-25" }); + const text = await ask(session, "checkout-api 5xx"); + assert.match(text, /nothing to list/); + assert.match(text, /\/neatcontext:save/); + } finally { + await session.close(); + } + }); +}); diff --git a/tests/routing.test.mjs b/tests/routing.test.mjs index f334f23..4a799a3 100644 --- a/tests/routing.test.mjs +++ b/tests/routing.test.mjs @@ -260,6 +260,48 @@ describe("session routing", () => { } }); + // Nothing connected is the case routing exists for, so the answer must lead + // with the route the session can take itself. Leading with a slash command + // there is what made routing look broken: it is the first thing the model + // reads, and it answers "what now?" before the menu below gets a turn. + it("tells an ungrounded session to connect a context itself", async () => { + await create("Payments", "payment failures"); + await create("Orders", "order fulfillment"); + const session = bridge("ungrounded-window"); + try { + await session.send("initialize", { protocolVersion: "2025-11-25" }); + const answer = await session.send("tools/call", { + name: "get_context", + arguments: { query: "why are payments failing?" } + }); + const text = answer.result.content[0].text; + assert.match(text, /Connect the one this request belongs to with `use_context`/); + assert.match(text, /do not ask the user to run a command/); + } finally { + await session.close(); + } + }); + + // Manual mode publishes no menu, so there is nothing to connect from and the + // command really is the only way forward. + it("falls back to the commands when routing is off", async () => { + await create("Payments", "payment failures"); + await cli(["mode", "manual"], "manual-window"); + const session = bridge("manual-window"); + try { + await session.send("initialize", { protocolVersion: "2025-11-25" }); + const answer = await session.send("tools/call", { + name: "get_context", + arguments: { query: "why are payments failing?" } + }); + const text = answer.result.content[0].text; + assert.match(text, /Connect one with `\/neatcontext:use`/); + assert.doesNotMatch(text, /## Contexts/); + } finally { + await session.close(); + } + }); + it("previews a Context without changing the selection", async () => { await create("Payments", "payment failures"); await create("Orders", "order fulfillment");