Skip to content

Commit cbbbb5c

Browse files
authored
fix(coding-agents): name the calling harness in every MCP registration (#3612)
* fix(coding-agents): name the calling harness in every MCP registration Claude Code and Codex launch the same dist/mcp-server.js, and only HINDSIGHT_MCP_HARNESS tells that server which one is calling. Four installers never set it — codex, cursor-cli, copilot-cli and grok-build — so their sessions fell back to "claude-code" for both the retain stamp and bank derivation: a Codex hindsight_ingest_document landed tagged harness:claude-code, indistinguishable from Claude Code's own writes. Every registration now names its own harness (claude-code included, so it no longer depends on the fallback). Codex's install also REPLACES an existing [mcp_servers.hindsight] block instead of skipping when one is present — appending only when absent made it install-once-only, so the harness-less block could never be repaired by re-running the installer, which is the upgrade path for anyone already hitting this. The guard sweeps INSTALLERS and greps whatever the install actually wrote for mcp-server.js: the harness that forgets is by construction the one nobody wrote a test for. Fixes #3603 * fix(coding-agents): require HINDSIGHT_MCP_HARNESS instead of guessing claude-code The fallback read as a safe convenience and was not. Every host launches the same mcp-server.js, so a registration that named no harness was silently served as Claude Code — which is what made #3603 invisible: the mis-stamped documents looked exactly like Claude Code's own. A wrong harness corrupts stored data (the harness:<id> stamp and the bank the session resolves); refusing to start is recoverable by re-running the installer, and the error says so. That turned up a second registration site the installer sweep could not see: the codebase survey builds INLINE MCP recipes for its claude-code and codex spawns (core/survey.ts), and neither named a harness — so a Codex-run survey already filed its findings as harness:claude-code in Claude Code's bank, and would now fail to start at all. Both recipes name their agent. Guarded by a source sweep modelled on the daemon-parity test: a module that points at mcp-server.js is registering it and must assign the variable. It matches an assignment rather than a mention on purpose — a bare-name search is satisfied by the comments this fix added next to each registration, which is exactly how survey.ts would have slipped through again.
1 parent 7ebf23d commit cbbbb5c

6 files changed

Lines changed: 241 additions & 41 deletions

File tree

hindsight-integrations/coding-agents/src/core/survey.test.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,9 @@ describe("startCodebaseSurvey", () => {
7676
const parsed = JSON.parse(mcpConfigJson);
7777
expect(parsed.mcpServers.hindsight.args).toEqual(["/x/mcp-server.js"]);
7878
expect(parsed.mcpServers.hindsight.env.HINDSIGHT_MCP_PROJECT_CWD).toBe("/repo");
79+
// #3603: mcp-server.js requires the harness — an inline recipe that omits it used to be served
80+
// as claude-code by default, and now refuses to start at all.
81+
expect(parsed.mcpServers.hindsight.env.HINDSIGHT_MCP_HARNESS).toBe("claude-code");
7982

8083
expect(options.cwd).toBe("/repo");
8184
expect(options.detached).toBe(true);
@@ -110,6 +113,9 @@ describe("startCodebaseSurvey", () => {
110113
expect(argv).toContain(SURVEY_PROMPT);
111114
expect(argv).toContain(`mcp_servers.hindsight.command="node"`);
112115
expect(argv).toContain(`mcp_servers.hindsight.args=["/x/mcp-server.js"]`);
116+
// Without this the survey's findings were stamped harness:claude-code and landed in Claude
117+
// Code's bank even though Codex ran the survey (#3603).
118+
expect(argv).toContain(`mcp_servers.hindsight.env.HINDSIGHT_MCP_HARNESS="codex"`);
113119
expect(argv).toContain(`mcp_servers.hindsight.env.HINDSIGHT_MCP_PROJECT_CWD="/repo"`);
114120
// No Claude-only flags leak into the codex recipe.
115121
expect(argv).not.toContain("--model");

hindsight-integrations/coding-agents/src/core/survey.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,11 @@ function buildSurveyPlan(
205205
hindsight: {
206206
command: "node",
207207
args: [opts.mcpServerPath],
208-
env: { HINDSIGHT_MCP_PROJECT_CWD: repoDir },
208+
// HINDSIGHT_MCP_HARNESS names the agent whose CLI this recipe drives — the survey's
209+
// ingests are its writes. mcp-server.js REQUIRES it (it used to default to
210+
// "claude-code", which is how the codex recipe below silently stamped its findings
211+
// harness:claude-code and wrote them to Claude Code's bank — #3603).
212+
env: { HINDSIGHT_MCP_PROJECT_CWD: repoDir, HINDSIGHT_MCP_HARNESS: "claude-code" },
209213
},
210214
},
211215
});
@@ -250,6 +254,8 @@ function buildSurveyPlan(
250254
`mcp_servers.hindsight.args=["${opts.mcpServerPath}"]`,
251255
"-c",
252256
`mcp_servers.hindsight.env.HINDSIGHT_MCP_PROJECT_CWD="${repoDir}"`,
257+
"-c",
258+
`mcp_servers.hindsight.env.HINDSIGHT_MCP_HARNESS="codex"`,
253259
SURVEY_PROMPT,
254260
],
255261
env,

hindsight-integrations/coding-agents/src/installer.test.ts

Lines changed: 76 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,13 @@
11
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
2-
import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync, existsSync } from "node:fs";
2+
import {
3+
mkdirSync,
4+
mkdtempSync,
5+
readdirSync,
6+
readFileSync,
7+
rmSync,
8+
writeFileSync,
9+
existsSync,
10+
} from "node:fs";
311
import { tmpdir } from "node:os";
412
import { dirname, join } from "node:path";
513
import { pathToFileURL } from "node:url";
@@ -119,6 +127,10 @@ describe("claude-code installer", () => {
119127
"--scope",
120128
"user",
121129
"hindsight",
130+
// Every host launches the same mcp-server.js, so the registration has to name its harness —
131+
// otherwise the server falls back to claude-code and mis-attributes the other hosts' writes.
132+
"--env",
133+
"HINDSIGHT_MCP_HARNESS=claude-code",
122134
"--",
123135
"node",
124136
join(ctx.dist, "mcp-server.js"),
@@ -179,6 +191,7 @@ describe("codex installer", () => {
179191
expect(toml).toContain("[features]\nhooks = true");
180192
expect(toml).toContain("[mcp_servers.hindsight]");
181193
expect(toml).toContain(join(ctx.dist, "mcp-server.js"));
194+
expect(toml).toContain('env = { HINDSIGHT_MCP_HARNESS = "codex" }');
182195
});
183196

184197
it("does NOT duplicate an existing [features] section (only appends mcp) and backs up the toml", () => {
@@ -205,6 +218,25 @@ describe("codex installer", () => {
205218
expect(toml).toContain("[mcp_servers.hindsight]");
206219
});
207220

221+
// Regression: Codex sessions ingested documents tagged `harness:claude-code`. Its registration
222+
// carried no HINDSIGHT_MCP_HARNESS, and install skipped whenever a block already existed — so
223+
// the stale, harness-less block survived every re-install and the shared mcp-server.js kept
224+
// falling back to claude-code for the bank AND the retain stamp.
225+
it("re-install REPLACES a harness-less mcp block instead of leaving it stale", () => {
226+
const ctx = makeCtx();
227+
mkdirSync(join(ctx.home, ".codex"), { recursive: true });
228+
writeFileSync(
229+
tomlPath(ctx),
230+
'[features]\nhooks = true\n\n[mcp_servers.hindsight]\ncommand = "node"\n' +
231+
`args = ["${join(ctx.dist, "mcp-server.js")}"]\n\n[ui]\ntheme = "dark"\n`
232+
);
233+
run(["install", "codex"], ctx);
234+
const toml = readFileSync(tomlPath(ctx), "utf8");
235+
expect(toml.match(/^\[mcp_servers\.hindsight\]/gm)).toHaveLength(1);
236+
expect(toml).toContain('env = { HINDSIGHT_MCP_HARNESS = "codex" }');
237+
expect(toml).toContain('[ui]\ntheme = "dark"'); // foreign sections survive the rewrite
238+
});
239+
208240
it("uninstall removes the mcp_servers.hindsight block and leaves the rest of the toml", () => {
209241
const ctx = makeCtx();
210242
run(["install", "codex"], ctx);
@@ -543,6 +575,7 @@ describe("cursor-cli installer", () => {
543575
expect(mcp.mcpServers.hindsight).toEqual({
544576
command: "node",
545577
args: [join(ctx.dist, "mcp-server.js")],
578+
env: { HINDSIGHT_MCP_HARNESS: "cursor-cli" },
546579
});
547580
});
548581

@@ -585,6 +618,7 @@ describe("grok-build installer", () => {
585618
expect(config).toContain(join(ctx.dist, "grok-stop-hook.js"));
586619
expect(config).toContain("[mcp_servers.hindsight]");
587620
expect(config).toContain(join(ctx.dist, "mcp-server.js"));
621+
expect(config).toContain('env = { HINDSIGHT_MCP_HARNESS = "grok-build" }');
588622
expect(existsSync(join(ctx.home, ".claude"))).toBe(false);
589623
});
590624

@@ -683,6 +717,47 @@ describe("run() CLI behavior", () => {
683717
});
684718
});
685719

720+
/**
721+
* Every host launches the SAME `dist/mcp-server.js`, so the command line cannot say who is calling
722+
* — only `HINDSIGHT_MCP_HARNESS` can. A registration that omits it silently inherits the server's
723+
* claude-code fallback, which is how a Codex `hindsight_ingest_document` was stored tagged
724+
* `harness:claude-code` on a machine running both (#3603).
725+
*
726+
* Swept over INSTALLERS rather than a hand-written list of config paths: the harness that gets this
727+
* wrong is by construction the one nobody wrote a test for, so the guard has to find the
728+
* registration itself — any file the install wrote that names mcp-server.js, wherever it landed.
729+
*/
730+
describe("MCP registrations name the calling harness", () => {
731+
/** Every file under `dir`, recursively. */
732+
function filesUnder(dir: string): string[] {
733+
return readdirSync(dir, { withFileTypes: true }).flatMap((e) =>
734+
e.isDirectory() ? filesUnder(join(dir, e.name)) : [join(dir, e.name)]
735+
);
736+
}
737+
738+
// These hosts have no MCP registration at all: they load our plugin/extension in-process
739+
// (src/kilo.ts, src/dsh.ts, src/prime-agent.ts, dist/index.js for opencode), and that entry
740+
// hands its own harness name straight to RuntimeCore.
741+
const IN_PROCESS = new Set(["opencode", "kilo", "prime-agent", "dsh"]);
742+
const MCP_HOSTS = INSTALLERS.map((i) => i.name).filter((n) => !IN_PROCESS.has(n));
743+
744+
it.each(MCP_HOSTS)("%s", (harness) => {
745+
const ctx = makeCtx();
746+
expect(run(["install", harness], ctx)).toBe(0);
747+
const registrations = [
748+
...filesUnder(ctx.home)
749+
.map((f) => readFileSync(f, "utf8"))
750+
// claude-code registers through the `claude` CLI instead of a file we write, so its
751+
// registration is the argv we handed the mock.
752+
.concat(ctx.claudeMcp.mock.calls.map((c) => c[0].join(" ")))
753+
.filter((text) => text.includes("mcp-server.js")),
754+
];
755+
expect(registrations.length).toBeGreaterThan(0);
756+
for (const text of registrations) expect(text).toContain(`HINDSIGHT_MCP_HARNESS`);
757+
for (const text of registrations) expect(text).toContain(harness);
758+
});
759+
});
760+
686761
/**
687762
* `all` is an explicit target rather than the default for a bare command: wiring every detected
688763
* agent rewrites a lot of a machine's config and should not happen by accident.

hindsight-integrations/coding-agents/src/installer.ts

Lines changed: 48 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,21 @@ const cmdHook = (dist: string, file: string, timeout: number) => ({
144144
hooks: [{ type: "command", command: `node "${join(dist, file)}"`, timeout }],
145145
});
146146

147+
/**
148+
* The MCP registration for a JSON-configured host.
149+
*
150+
* Every harness launches the SAME `dist/mcp-server.js`, so the command line alone cannot say who
151+
* is calling. `HINDSIGHT_MCP_HARNESS` is what the server reads to answer that; without it it falls
152+
* back to "claude-code", which is why a Codex `hindsight_ingest_document` landed tagged
153+
* `harness:claude-code` (and derived its bank as Claude Code's) on machines running both. Every
154+
* registration MUST name its own harness — pass it here, never rely on the fallback.
155+
*/
156+
const mcpServerEntry = (dist: string, harness: HookHarnessName | "cline-cli") => ({
157+
command: "node",
158+
args: [join(dist, "mcp-server.js")],
159+
env: { HINDSIGHT_MCP_HARNESS: harness },
160+
});
161+
147162
/** Install/uninstall consume the same lifecycle declaration as the runtime entrypoints. Keeping
148163
* event names and command files here would allow a host to run a different lifecycle than it installs. */
149164
function mergeHarnessHooks(
@@ -369,6 +384,8 @@ const claudeCode: HarnessInstaller = {
369384
"--scope",
370385
"user",
371386
"hindsight",
387+
"--env",
388+
"HINDSIGHT_MCP_HARNESS=claude-code",
372389
"--",
373390
"node",
374391
join(c.dist, "mcp-server.js"),
@@ -378,7 +395,7 @@ const claudeCode: HarnessInstaller = {
378395
} else {
379396
c.log?.(
380397
`claude-code: could not run \`claude mcp add\` — register the tools manually:\n` +
381-
` claude mcp add --scope user hindsight -- node "${join(c.dist, "mcp-server.js")}"`
398+
` claude mcp add --scope user hindsight --env HINDSIGHT_MCP_HARNESS=claude-code -- node "${join(c.dist, "mcp-server.js")}"`
382399
);
383400
}
384401
},
@@ -408,6 +425,17 @@ function defaultClaudeMcp(args: string[]): boolean {
408425
}
409426
}
410427

428+
/**
429+
* Our `[mcp_servers.hindsight]` table (plus any sub-table of it): the header line through to the
430+
* next table header or EOF. Shared by install — which REPLACES the block — and uninstall.
431+
*/
432+
const CODEX_MCP_BLOCK_RE = /^\[mcp_servers\.hindsight(?:\.[^\]]+)?\][^\n]*\n(?:(?!\[)[^\n]*\n?)*/gm;
433+
434+
/** Inline `env`, so CODEX_MCP_BLOCK_RE never has to straddle a `[mcp_servers.hindsight.env]` table. */
435+
const codexMcpBlock = (dist: string) =>
436+
`[mcp_servers.hindsight]\ncommand = "node"\nargs = [${JSON.stringify(join(dist, "mcp-server.js"))}]\n` +
437+
`env = { HINDSIGHT_MCP_HARNESS = "codex" }`;
438+
411439
const codex: HarnessInstaller = {
412440
name: "codex",
413441
detect: (c) => onPath("codex") || existsSync(join(c.home, ".codex")),
@@ -419,9 +447,13 @@ const codex: HarnessInstaller = {
419447
writeJson(hooksPath, cfg);
420448
c.log?.(`codex: hooks merged into ${hooksPath}`);
421449

422-
// config.toml: append-only, never rewrite (TOML round-tripping is not worth the risk).
450+
// config.toml: append-only for anything that is not ours (TOML round-tripping is not worth the
451+
// risk). Our OWN mcp block is stripped and rewritten instead of skipped when present: appending
452+
// only when absent made this install-once-only, so the harness-less registration that
453+
// attributed every Codex write to claude-code could never be repaired by re-running install.
423454
const tomlPath = join(c.home, ".codex", "config.toml");
424-
let toml = existsSync(tomlPath) ? readFileSync(tomlPath, "utf8") : "";
455+
const existing = existsSync(tomlPath) ? readFileSync(tomlPath, "utf8") : "";
456+
const toml = existing.replace(CODEX_MCP_BLOCK_RE, "");
425457
const additions: string[] = [];
426458
// Codex ≥ 0.145 deprecates `codex_hooks` for `[features].hooks`; accept either as "already
427459
// enabled", write the modern name for new installs.
@@ -434,18 +466,15 @@ const codex: HarnessInstaller = {
434466
additions.push("[features]\nhooks = true");
435467
}
436468
}
437-
if (!toml.includes("[mcp_servers.hindsight]")) {
438-
additions.push(
439-
`[mcp_servers.hindsight]\ncommand = "node"\nargs = ["${join(c.dist, "mcp-server.js")}"]`
440-
);
441-
}
442-
if (additions.length) {
469+
additions.push(codexMcpBlock(c.dist));
470+
const next = `${toml.replace(/\n*$/, "\n\n")}${additions.join("\n\n")}\n`;
471+
if (next !== existing) {
443472
if (existsSync(tomlPath) && !existsSync(`${tomlPath}.hindsight-backup`)) {
444473
copyFileSync(tomlPath, `${tomlPath}.hindsight-backup`);
445474
}
446475
mkdirSync(dirname(tomlPath), { recursive: true });
447-
writeFileSync(tomlPath, `${toml.replace(/\n*$/, "\n\n")}${additions.join("\n\n")}\n`);
448-
c.log?.(`codex: appended ${additions.length} section(s) to ${tomlPath}`);
476+
writeFileSync(tomlPath, next);
477+
c.log?.(`codex: wrote ${additions.length} section(s) to ${tomlPath}`);
449478
}
450479
installSkill(c, "codex", join(c.home, ".agents", "skills")); // agentskills-standard shared dir
451480
},
@@ -462,10 +491,7 @@ const codex: HarnessInstaller = {
462491
const tomlPath = join(c.home, ".codex", "config.toml");
463492
if (existsSync(tomlPath)) {
464493
const toml = readFileSync(tomlPath, "utf8");
465-
const cleaned = toml.replace(
466-
/\n?\[mcp_servers\.hindsight\]\ncommand = "node"\nargs = \[[^\]]*\]\n?/g,
467-
"\n"
468-
);
494+
const cleaned = toml.replace(CODEX_MCP_BLOCK_RE, "");
469495
if (cleaned !== toml) writeFileSync(tomlPath, cleaned);
470496
}
471497
c.log?.(
@@ -505,11 +531,7 @@ const antigravity: HarnessInstaller = {
505531
const mcp = readJson(mcpPath);
506532
mcp.mcpServers = {
507533
...(mcp.mcpServers ?? {}),
508-
hindsight: {
509-
command: "node",
510-
args: [join(c.dist, "mcp-server.js")],
511-
env: { HINDSIGHT_MCP_HARNESS: "antigravity-cli" },
512-
},
534+
hindsight: mcpServerEntry(c.dist, "antigravity-cli"),
513535
};
514536
writeJson(mcpPath, mcp);
515537
const settingsPath = join(c.home, ".gemini", "antigravity-cli", "settings.json");
@@ -915,11 +937,7 @@ const devin: HarnessInstaller = {
915937
const mcp = readJson(mcpPath);
916938
mcp.mcpServers = {
917939
...(mcp.mcpServers ?? {}),
918-
hindsight: {
919-
command: "node",
920-
args: [join(c.dist, "mcp-server.js")],
921-
env: { HINDSIGHT_MCP_HARNESS: "devin-cli" },
922-
},
940+
hindsight: mcpServerEntry(c.dist, "devin-cli"),
923941
};
924942
writeJson(mcpPath, mcp);
925943
c.log?.(`devin-cli: hooks merged into ${configPath}, MCP into ${mcpPath}`);
@@ -959,7 +977,7 @@ const cursor: HarnessInstaller = {
959977
const mcp = readJson(mcpPath);
960978
mcp.mcpServers = {
961979
...(mcp.mcpServers ?? {}),
962-
hindsight: { command: "node", args: [join(c.dist, "mcp-server.js")] },
980+
hindsight: mcpServerEntry(c.dist, "cursor-cli"),
963981
};
964982
writeJson(mcpPath, mcp);
965983
c.log?.(`cursor-cli: hooks merged into ${hooksPath}, MCP into ${mcpPath}`);
@@ -1001,7 +1019,7 @@ const copilot: HarnessInstaller = {
10011019
const mcp = readJson(mcpPath);
10021020
mcp.mcpServers = {
10031021
...(mcp.mcpServers ?? {}),
1004-
hindsight: { command: "node", args: [join(c.dist, "mcp-server.js")] },
1022+
hindsight: mcpServerEntry(c.dist, "copilot-cli"),
10051023
};
10061024
writeJson(mcpPath, mcp);
10071025
installSkill(c, "copilot-cli", join(c.home, ".copilot", "skills"));
@@ -1048,7 +1066,8 @@ const grok: HarnessInstaller = {
10481066
`[[hooks.SessionStart]]\n [[hooks.SessionStart.hooks]]\n type = \"command\"\n command = ${command("grok-sessionstart-hook.js")}\n timeout = 30\n\n` +
10491067
`[[hooks.UserPromptSubmit]]\n [[hooks.UserPromptSubmit.hooks]]\n type = \"command\"\n command = ${command("grok-hook.js")}\n timeout = 30\n\n` +
10501068
`[[hooks.Stop]]\n [[hooks.Stop.hooks]]\n type = \"command\"\n command = ${command("grok-stop-hook.js")}\n timeout = 60\n\n` +
1051-
`[mcp_servers.hindsight]\ncommand = \"node\"\nargs = [${tomlString(join(c.dist, "mcp-server.js"))}]\n${GROK_MARKER_END}\n`;
1069+
`[mcp_servers.hindsight]\ncommand = \"node\"\nargs = [${tomlString(join(c.dist, "mcp-server.js"))}]\n` +
1070+
`env = { HINDSIGHT_MCP_HARNESS = \"grok-build\" }\n${GROK_MARKER_END}\n`;
10521071
if (existsSync(path) && !existsSync(`${path}.hindsight-backup`))
10531072
copyFileSync(path, `${path}.hindsight-backup`);
10541073
mkdirSync(dirname(path), { recursive: true });
@@ -1099,11 +1118,7 @@ const cline: HarnessInstaller = {
10991118
const mcp = readJson(mcpPath);
11001119
mcp.mcpServers = {
11011120
...(mcp.mcpServers ?? {}),
1102-
hindsight: {
1103-
command: "node",
1104-
args: [join(c.dist, "mcp-server.js")],
1105-
env: { HINDSIGHT_MCP_HARNESS: "cline-cli" },
1106-
},
1121+
hindsight: mcpServerEntry(c.dist, "cline-cli"),
11071122
};
11081123
writeJson(mcpPath, mcp);
11091124
installSkill(c, "cline-cli", join(c.home, ".cline", "data", "settings", "skills"));

0 commit comments

Comments
 (0)