mirror of
https://github.com/thedotmack/claude-mem.git
synced 2026-09-20 04:23:02 +08:00
* fix(mcp): cross-platform Node launcher for plugin .mcp.json (closes #2792) plugin/.mcp.json used `command: "sh"` with an `sh -c` discovery prelude, which Claude Code cannot spawn on Windows when Git's usr/bin is not on PATH — so the mcp-search tools never registered (#2792/#2790/#2714/#2461). Replace it with a pure-Node `command: "node"` + `node -e <payload>` launcher emitted by buildMcpNodeLauncher() in hook-shell-template.ts. The payload mirrors the POSIX prelude's plugin-root discovery order ($CLAUDE_PLUGIN_ROOT/$PLUGIN_ROOT, extra candidates, mtime-sorted cache roots, then the marketplace dir), spawns the resolved mcp-server.cjs with inherited stdio, forwards SIGTERM/SIGINT/SIGHUP, and propagates the child's exit code/signal. No shell dependency. The generator remains the single source of truth: build-hooks.js verifies plugin/.mcp.json args[1] matches buildShellCommand({host:'mcp'}), and the plugin-distribution tests are updated to assert the Node payload. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(mcp): address review — exit 1 on signal re-raise failure, drop unused trailingCommand Greptile review of #2807: - Signal-kill fallback now exits 1 (not 0) when re-raising the child's terminating signal throws, so an unexpected kill is no longer reported to the host as clean success. Regenerated plugin/.mcp.json to match. - trailingCommand is now optional and intentionally ignored by the mcp Node launcher (spawn target derives from requireFile). Documented in buildMcpNodeLauncher, dropped the meaningless shell tokens from the mcp call sites, and made shell hosts fail loud if trailingCommand is missing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Alex Newman
+3
-3
@@ -2,10 +2,10 @@
|
||||
"mcpServers": {
|
||||
"mcp-search": {
|
||||
"type": "stdio",
|
||||
"command": "sh",
|
||||
"command": "node",
|
||||
"args": [
|
||||
"-c",
|
||||
"_C=\"${CLAUDE_CONFIG_DIR:-$HOME/.claude}\"; _E=\"${CLAUDE_PLUGIN_ROOT:-${PLUGIN_ROOT:-}}\"; _P=$({ [ -n \"$_E\" ] && printf '%s\\n' \"$_E\"; printf '%s\\n' \"$PWD/plugin\" \"$PWD\"; ls -dt \"$HOME/.codex/plugins/cache/claude-mem-local/claude-mem\"/[0-9]*/ \"$HOME/.codex/plugins/cache/thedotmack/claude-mem\"/[0-9]*/ \"$_C/plugins/cache/thedotmack/claude-mem\"/[0-9]*/ 2>/dev/null; printf '%s\\n' \"$_C/plugins/marketplaces/thedotmack/plugin\"; } | while IFS= read -r _R; do [ -d \"$_R/plugin/scripts\" ] && _Q=\"$_R/plugin\" || _Q=\"$_R\"; [ -f \"$_Q/scripts/mcp-server.cjs\" ] && { printf '%s\\n' \"$_Q\"; break; }; done); [ -n \"$_P\" ] || { echo \"claude-mem: mcp server not found\" >&2; exit 1; }; exec node \"$_P/scripts/mcp-server.cjs\""
|
||||
"-e",
|
||||
"const f=require('fs'),p=require('path'),o=require('os'),c=require('child_process');const h=o.homedir();const C=process.env.CLAUDE_CONFIG_DIR||p.join(h,'.claude');const E=process.env.CLAUDE_PLUGIN_ROOT||process.env.PLUGIN_ROOT||'';const d=process.cwd();const L=x=>{try{return f.readdirSync(x).filter(n=>/^\\d/.test(n)).map(n=>p.join(x,n)).filter(z=>{try{return f.statSync(z).isDirectory()}catch{return false}}).sort((a,b)=>f.statSync(b).mtimeMs-f.statSync(a).mtimeMs)}catch{return[]}};const K=[E,p.join(d,\"plugin\"),d,...L(p.join(h,\".codex/plugins/cache/claude-mem-local/claude-mem\")),...L(p.join(h,\".codex/plugins/cache/thedotmack/claude-mem\")),...L(p.join(C,\"plugins/cache/thedotmack/claude-mem\")),p.join(C,\"plugins/marketplaces/thedotmack/plugin\")].filter(Boolean);let R=null;for(const k of K){const r=f.existsSync(p.join(k,'plugin','scripts'))?p.join(k,'plugin'):k;if(f.existsSync(p.join(r,'scripts',\"mcp-server.cjs\"))){R=r;break}}if(!R){process.stderr.write(\"claude-mem: mcp server not found\\n\");process.exit(1)}const ch=c.spawn(process.execPath,[p.join(R,'scripts',\"mcp-server.cjs\")],{stdio:'inherit'});for(const s of ['SIGTERM','SIGINT','SIGHUP'])process.on(s,()=>{try{ch.kill(s)}catch{}});ch.on('exit',(code,sig)=>{if(sig){process.removeAllListeners(sig);try{process.kill(process.pid,sig)}catch{process.exit(1)}}else process.exit(code==null?0:code)})"
|
||||
]
|
||||
}
|
||||
}
|
||||
|
||||
@@ -106,8 +106,9 @@ function shellTemplateManifest(buildShellCommand) {
|
||||
'plugin/.mcp.json': {
|
||||
kind: 'mcp',
|
||||
command: buildShellCommand({
|
||||
// The mcp Node launcher derives its spawn target from requireFile, so
|
||||
// no trailingCommand is needed (it is ignored for this host).
|
||||
host: 'mcp', requireFile: 'mcp-server.cjs',
|
||||
trailingCommand: ['exec', 'node', '"$_P/scripts/mcp-server.cjs"'],
|
||||
notFoundMessage: 'claude-mem: mcp server not found',
|
||||
mcpExtraCandidates: ['$PWD/plugin', '$PWD'],
|
||||
mcpExtraCacheRoots: [
|
||||
|
||||
@@ -33,9 +33,11 @@ export interface ShellTemplateOptions {
|
||||
/**
|
||||
* Trailing command tokens run after `_P` resolves. Tokens are emitted
|
||||
* verbatim (callers pass already-quoted `"$_P/scripts/X"` forms), matching
|
||||
* the hand-authored files.
|
||||
* the hand-authored files. Required for every shell host; the `mcp` host
|
||||
* ignores it (the Node launcher derives its spawn target from `requireFile`),
|
||||
* so mcp callers may omit it.
|
||||
*/
|
||||
trailingCommand: string[];
|
||||
trailingCommand?: string[];
|
||||
/** Extra env exports prepended to the trailing command (e.g. CLAUDE_MEM_CODEX_HOOK=1). */
|
||||
extraEnv?: Record<string, string>;
|
||||
/** Optional trailing JSON echoed after the command (e.g. SessionStart continue marker). */
|
||||
@@ -126,12 +128,88 @@ function candidateBlock(options: ShellTemplateOptions): string {
|
||||
const CYGPATH_CLAUSE =
|
||||
`command -v cygpath >/dev/null 2>&1 && { _W=$(cygpath -w "$_P" 2>/dev/null); [ -n "$_W" ] && _P="$_W"; };`;
|
||||
|
||||
/**
|
||||
* Translate a shell-token candidate (`$PWD`, `$PWD/x`, `$HOME/x`, `$_C/x`) into
|
||||
* an equivalent Node path expression for the cross-platform MCP launcher.
|
||||
* `d` = process.cwd(), `h` = os.homedir(), `C` = resolved CLAUDE_CONFIG_DIR.
|
||||
*/
|
||||
function shTokenToNode(token: string): string {
|
||||
if (token === '$PWD') return 'd';
|
||||
const map: Array<[string, string]> = [
|
||||
['$PWD/', 'd'],
|
||||
['$HOME/', 'h'],
|
||||
['$_C/', 'C'],
|
||||
];
|
||||
for (const [prefix, base] of map) {
|
||||
if (token.startsWith(prefix)) {
|
||||
return `p.join(${base},${JSON.stringify(token.slice(prefix.length))})`;
|
||||
}
|
||||
}
|
||||
// Literal fallback (no known shell base) — embed as-is.
|
||||
return JSON.stringify(token);
|
||||
}
|
||||
|
||||
/**
|
||||
* Cross-platform MCP launcher (issues #2792, #2790, #2714, #2461). The plugin
|
||||
* `.mcp.json` previously used `command: "sh"`, which Claude Code cannot spawn on
|
||||
* Windows when Git's `usr/bin` is not on PATH, so the search tools never
|
||||
* registered. This emits the `node -e` payload (`.mcp.json` args[1]) that does
|
||||
* the same plugin-root discovery in pure Node — no shell dependency — then
|
||||
* spawns the resolved server and forwards signals. The candidate order mirrors
|
||||
* the POSIX prelude's: $CLAUDE_PLUGIN_ROOT/$PLUGIN_ROOT, mcpExtraCandidates,
|
||||
* mtime-sorted cache roots, then the marketplace install dir.
|
||||
*
|
||||
* Only `requireFile`, `notFoundMessage`, and the mcp* candidate fields are
|
||||
* consumed. `trailingCommand`, `extraEnv`, `trailingJson`, and the cygpath
|
||||
* clause are intentionally ignored for this host — the spawn target is derived
|
||||
* solely from `requireFile`, and the Node launcher needs no shell scaffolding.
|
||||
*/
|
||||
function buildMcpNodeLauncher(options: ShellTemplateOptions): string {
|
||||
const candidates = (options.mcpExtraCandidates ?? []).map(shTokenToNode);
|
||||
const cacheRoots = [
|
||||
...(options.mcpExtraCacheRoots ?? []),
|
||||
'$_C/plugins/cache/thedotmack/claude-mem',
|
||||
].map(shTokenToNode);
|
||||
const marketplace = shTokenToNode('$_C/plugins/marketplaces/thedotmack/plugin');
|
||||
const require = JSON.stringify(options.requireFile);
|
||||
const notFound = JSON.stringify(`${options.notFoundMessage}\n`);
|
||||
|
||||
const kParts = [
|
||||
'E',
|
||||
...candidates,
|
||||
...cacheRoots.map((root) => `...L(${root})`),
|
||||
marketplace,
|
||||
].join(',');
|
||||
|
||||
return (
|
||||
`const f=require('fs'),p=require('path'),o=require('os'),c=require('child_process');` +
|
||||
`const h=o.homedir();` +
|
||||
`const C=process.env.CLAUDE_CONFIG_DIR||p.join(h,'.claude');` +
|
||||
`const E=process.env.CLAUDE_PLUGIN_ROOT||process.env.PLUGIN_ROOT||'';` +
|
||||
`const d=process.cwd();` +
|
||||
`const L=x=>{try{return f.readdirSync(x).filter(n=>/^\\d/.test(n)).map(n=>p.join(x,n)).filter(z=>{try{return f.statSync(z).isDirectory()}catch{return false}}).sort((a,b)=>f.statSync(b).mtimeMs-f.statSync(a).mtimeMs)}catch{return[]}};` +
|
||||
`const K=[${kParts}].filter(Boolean);` +
|
||||
`let R=null;` +
|
||||
`for(const k of K){const r=f.existsSync(p.join(k,'plugin','scripts'))?p.join(k,'plugin'):k;if(f.existsSync(p.join(r,'scripts',${require}))){R=r;break}}` +
|
||||
`if(!R){process.stderr.write(${notFound});process.exit(1)}` +
|
||||
`const ch=c.spawn(process.execPath,[p.join(R,'scripts',${require})],{stdio:'inherit'});` +
|
||||
`for(const s of ['SIGTERM','SIGINT','SIGHUP'])process.on(s,()=>{try{ch.kill(s)}catch{}});` +
|
||||
`ch.on('exit',(code,sig)=>{if(sig){process.removeAllListeners(sig);try{process.kill(process.pid,sig)}catch{process.exit(1)}}else process.exit(code==null?0:code)})`
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Build the full single-line shell command string for a Rule A site.
|
||||
* The output is byte-compatible with the hand-authored command strings in
|
||||
* the host-managed config files.
|
||||
*/
|
||||
export function buildShellCommand(options: ShellTemplateOptions): string {
|
||||
// MCP uses a cross-platform Node launcher instead of an `sh -c` prelude so it
|
||||
// spawns on Windows without Git Bash (#2792/#2790/#2714/#2461).
|
||||
if (options.host === 'mcp') {
|
||||
return buildMcpNodeLauncher(options);
|
||||
}
|
||||
|
||||
const parts: string[] = [];
|
||||
|
||||
// The PATH prelude is pushed verbatim (including any trailing space). `parts`
|
||||
@@ -146,12 +224,9 @@ export function buildShellCommand(options: ShellTemplateOptions): string {
|
||||
parts.push(candidateBlock(options));
|
||||
parts.push(`[ -n "$_P" ] || { echo "${options.notFoundMessage}" >&2; exit 1; };`);
|
||||
|
||||
// cygpath conversion: claude-code + codex-cli only. MCP runs under `sh -c`
|
||||
// which already understands POSIX paths, so no conversion (matches current
|
||||
// plugin/.mcp.json).
|
||||
if (options.host !== 'mcp') {
|
||||
parts.push(CYGPATH_CLAUSE);
|
||||
}
|
||||
// cygpath conversion: claude-code + codex-cli. MCP returned early above (it
|
||||
// uses the Node launcher), so every host reaching here needs the clause.
|
||||
parts.push(CYGPATH_CLAUSE);
|
||||
|
||||
const envPrefix = options.extraEnv
|
||||
? Object.entries(options.extraEnv)
|
||||
@@ -159,6 +234,11 @@ export function buildShellCommand(options: ShellTemplateOptions): string {
|
||||
.join('')
|
||||
: '';
|
||||
|
||||
// Shell hosts always run a trailing command; fail loud rather than emit a
|
||||
// launcher that silently resolves `_P` and then does nothing.
|
||||
if (!options.trailingCommand) {
|
||||
throw new Error(`buildShellCommand: host '${options.host}' requires trailingCommand`);
|
||||
}
|
||||
let command = `${envPrefix}${options.trailingCommand.join(' ')}`;
|
||||
if (options.trailingJson) {
|
||||
command += `; echo '${JSON.stringify(options.trailingJson)}'`;
|
||||
|
||||
@@ -129,21 +129,24 @@ describe('Plugin Distribution - hooks.json Integrity', () => {
|
||||
});
|
||||
|
||||
describe('Plugin Distribution - Startup Root Resolution', () => {
|
||||
it('MCP startup commands should have config-dir based non-empty fallbacks', () => {
|
||||
for (const relativePath of ['plugin/.mcp.json']) {
|
||||
const command = mcpStartupCommandFrom(relativePath);
|
||||
it('MCP startup command resolves the plugin root cross-platform (#2792)', () => {
|
||||
// The launcher is now a cross-platform `node -e` payload (no `sh`), so it
|
||||
// spawns on Windows without Git Bash. It must still resolve the plugin root
|
||||
// with config-dir + env fallbacks and try cache roots before marketplaces.
|
||||
const command = mcpStartupCommandFrom('plugin/.mcp.json');
|
||||
|
||||
expect(command).toContain('${CLAUDE_CONFIG_DIR:-$HOME/.claude}');
|
||||
expect(command).toContain('_E="${CLAUDE_PLUGIN_ROOT:-${PLUGIN_ROOT:-}}"');
|
||||
expect(command).toContain('while IFS= read -r _R');
|
||||
expect(command).toContain('$_C/plugins/marketplaces/thedotmack/plugin');
|
||||
expect(command).toContain('$_C/plugins/cache/thedotmack/claude-mem');
|
||||
expect(command).toContain('[ -f "$_Q/scripts/mcp-server.cjs" ]');
|
||||
expect(command).not.toContain('"/scripts/mcp-server.cjs"');
|
||||
expect(command.indexOf('$_C/plugins/cache/thedotmack/claude-mem')).toBeLessThan(
|
||||
command.indexOf('$_C/plugins/marketplaces/thedotmack/plugin')
|
||||
);
|
||||
}
|
||||
expect(command).toContain('CLAUDE_CONFIG_DIR');
|
||||
expect(command).toContain('.claude');
|
||||
expect(command).toContain('CLAUDE_PLUGIN_ROOT');
|
||||
expect(command).toContain('PLUGIN_ROOT');
|
||||
expect(command).toContain('plugins/marketplaces/thedotmack/plugin');
|
||||
expect(command).toContain('plugins/cache/thedotmack/claude-mem');
|
||||
expect(command).toContain('mcp-server.cjs');
|
||||
// No bare absolute "/scripts/..." path leaks through.
|
||||
expect(command).not.toContain('"/scripts/mcp-server.cjs"');
|
||||
expect(command.indexOf('plugins/cache/thedotmack/claude-mem')).toBeLessThan(
|
||||
command.indexOf('plugins/marketplaces/thedotmack/plugin')
|
||||
);
|
||||
});
|
||||
|
||||
it('Codex hook commands should have config-dir based non-empty fallbacks', () => {
|
||||
@@ -273,8 +276,9 @@ const RULE_A_EXPECTATIONS: Record<string, Record<string, string>> = {
|
||||
};
|
||||
|
||||
const MCP_EXPECTED = buildShellCommand({
|
||||
// The mcp Node launcher derives its spawn target from requireFile; it ignores
|
||||
// trailingCommand, so none is passed (see buildMcpNodeLauncher).
|
||||
host: 'mcp', requireFile: 'mcp-server.cjs',
|
||||
trailingCommand: ['exec', 'node', '"$_P/scripts/mcp-server.cjs"'],
|
||||
notFoundMessage: 'claude-mem: mcp server not found',
|
||||
mcpExtraCandidates: ['$PWD/plugin', '$PWD'],
|
||||
mcpExtraCacheRoots: [
|
||||
@@ -308,14 +312,16 @@ describe('Spawn-Contract Templating - Rule A generator parity', () => {
|
||||
// The placeholder may appear only inside the _E="${CLAUDE_PLUGIN_ROOT:-...}"
|
||||
// expansion, never as a bare `${CLAUDE_PLUGIN_ROOT}` token that would reach
|
||||
// the binary unsubstituted.
|
||||
const all = [
|
||||
...Object.values(RULE_A_EXPECTATIONS).flatMap((c) => Object.values(c)),
|
||||
MCP_EXPECTED,
|
||||
];
|
||||
for (const command of all) {
|
||||
const shCommands = Object.values(RULE_A_EXPECTATIONS).flatMap((c) => Object.values(c));
|
||||
for (const command of shCommands) {
|
||||
expect(command).not.toMatch(/\$\{CLAUDE_PLUGIN_ROOT\}(?!:-)/);
|
||||
expect(command).toContain('_E="${CLAUDE_PLUGIN_ROOT:-${PLUGIN_ROOT:-}}"');
|
||||
}
|
||||
// The MCP node launcher reads env vars directly — it has no `${...}` shell
|
||||
// tokens at all, so a raw placeholder can never reach the binary.
|
||||
expect(MCP_EXPECTED).not.toContain('${CLAUDE_PLUGIN_ROOT}');
|
||||
expect(MCP_EXPECTED).toContain('process.env.CLAUDE_PLUGIN_ROOT');
|
||||
expect(MCP_EXPECTED).toContain('process.env.PLUGIN_ROOT');
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user