diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ca5b9ff1..be585cb68 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,14 @@ and adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### New Features + +- Two new hook entry points let a host agent tell CodeGraph which context is calling: `codegraph hooks pre-tool-use` tags each `codegraph_explore` call with the agent that made it, and `codegraph hooks post-compact` clears that context's already-sent record after a compaction. `codegraph install` wires them into Claude Code and Codex automatically — on Codex they are recorded as trusted as they are written, so it doesn't stop to ask you to review them — and already-sent tracking then follows each subagent and each compacted session instead of the connection they share. `codegraph uninstall` removes them, and hooks you added yourself are left alone either way. Other agents are unaffected — nothing changes for hosts that don't run them. + +### Fixes + +- `codegraph_explore` no longer tells a subagent that source was "already sent" when it went to a different agent. Harnesses like Claude Code route a subagent's tool calls over the same connection to CodeGraph as the main agent, so a fresh subagent could be handed a pointer to code its own context never received — and it would go read the file instead. Already-sent tracking is now kept per calling context. + ## [1.6.0] - 2026-08-26 diff --git a/__tests__/explore-cross-call-dedup.test.ts b/__tests__/explore-cross-call-dedup.test.ts index 608d85ce8..8b775b399 100644 --- a/__tests__/explore-cross-call-dedup.test.ts +++ b/__tests__/explore-cross-call-dedup.test.ts @@ -313,6 +313,51 @@ describe('a second call against a real index', () => { expect(await explore(QUERY, b)).toBe(firstForA); }, 180_000); + /** + * Per-caller bucketing (`sessionId`). + * + * One MCP connection is not one agent context: Claude Code subagents dispatch + * over the parent's connection, so an unbucketed record hands a subagent a + * pointer to source only the parent was ever sent — the exact "codegraph + * doesn't have it" shape that costs a Read. A host hook injects a distinct id + * per context; calls that carry none share the default bucket and behave + * exactly as they did before it existed. + */ + it('serves a second context in full — a subagent never got the first call', async () => { + const session = new ExploreSessionState(); + const firstForA = await explore(QUERY, session, { sessionId: 'A' }); + const firstForB = await explore(QUERY, session, { sessionId: 'B' }); + expect(firstForB).toBe(firstForA); + expect(firstForB).not.toContain(POINTER); + }, 180_000); + + it('still dedups the second call of ONE context', async () => { + const session = new ExploreSessionState(); + await explore(QUERY, session, { sessionId: 'A' }); + expect(await explore(QUERY, session, { sessionId: 'A' })).toContain(POINTER); + }, 180_000); + + it('does not dedup an identified call against the unidentified bucket', async () => { + const session = new ExploreSessionState(); + const unidentified = await explore(QUERY, session); + const identified = await explore(QUERY, session, { sessionId: 'A' }); + expect(identified).toBe(unidentified); + expect(identified).not.toContain(POINTER); + }, 180_000); + + it('clearSessionRecord drops one context\'s history and leaves its siblings', async () => { + const session = new ExploreSessionState(); + const firstForA = await explore(QUERY, session, { sessionId: 'A' }); + await explore(QUERY, session, { sessionId: 'B' }); + + expect(session.clearSessionRecord('A')).toBe(1); + + // A was reset: its next call is a first call again, byte for byte. + expect(await explore(QUERY, session, { sessionId: 'A' })).toBe(firstForA); + // B never lost anything. + expect(await explore(QUERY, session, { sessionId: 'B' })).toContain(POINTER); + }, 240_000); + it('is off entirely under CODEGRAPH_EXPLORE_DEDUP=0', async () => { const session = new ExploreSessionState(); const previous = process.env.CODEGRAPH_EXPLORE_DEDUP; diff --git a/__tests__/explore-session-reset.test.ts b/__tests__/explore-session-reset.test.ts new file mode 100644 index 000000000..303c3ce1f --- /dev/null +++ b/__tests__/explore-session-reset.test.ts @@ -0,0 +1,391 @@ +/** + * Resetting one caller's already-sent record. + * + * Bucketing the record per caller context (see explore-cross-call-dedup) fixes + * the subagent misfire, but a bucket still has to be forgettable: after a + * compact the agent's context no longer holds the source the server believes it + * was sent, so the next call gets pointers to text that is gone — the "codegraph + * doesn't have it" shape that costs a Read. The host knows when that happens and + * nothing else does, so the reset arrives from OUTSIDE the MCP conversation: + * + * 1. the daemon's control line — one JSON line on a fresh connection, one + * reply, no MCP session, and never a crash whatever the line says; + * 2. the `codegraph hooks` CLI entry points the host actually invokes, which + * must ALWAYS exit 0 and keep stdout clean, since stdout IS the hook + * protocol and a non-zero exit disrupts the agent. + */ +import { afterEach, describe, expect, it } from 'vitest'; +import { spawn } from 'child_process'; +import * as fs from 'fs'; +import * as net from 'net'; +import * as os from 'os'; +import * as path from 'path'; +import { Daemon, parseDaemonControlLine } from '../src/mcp/daemon'; +import { MCPSession } from '../src/mcp/session'; +import { getDaemonSocketCandidates } from '../src/mcp/daemon-paths'; +import type { MCPEngine } from '../src/mcp/engine'; +import type { JsonRpcTransport } from '../src/mcp/transport'; +import type { ExploreEmission } from '../src/mcp/explore-session-state'; + +const BIN = path.resolve(__dirname, '../dist/bin/codegraph.js'); + +const servers: net.Server[] = []; +const tmpDirs: string[] = []; +afterEach(async () => { + while (servers.length) { + const server = servers.pop()!; + await new Promise((resolve) => server.close(() => resolve())); + } + while (tmpDirs.length) { + try { fs.rmSync(tmpDirs.pop()!, { recursive: true, force: true }); } catch { /* best-effort */ } + } +}); + +function tmpProject(prefix: string): string { + const dir = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), prefix))); + tmpDirs.push(dir); + // `.codegraph/codegraph.db` is what makes a directory look indexed, which is + // what the hook needs before it goes looking for a daemon. + fs.mkdirSync(path.join(dir, '.codegraph'), { recursive: true }); + fs.writeFileSync(path.join(dir, '.codegraph', 'codegraph.db'), ''); + return dir; +} + +function emission(projectRoot: string): ExploreEmission { + return { + projectRoot, + query: 'q', + files: [{ path: 'a.ts', ranges: [{ start: 1, end: 10 }], bytes: 100 }], + sourceBytes: 100, + responseBytes: 400, + }; +} + +/** A transport that is never driven — these tests reach the session directly. */ +const idleTransport = (): JsonRpcTransport => ({ + start() { /* no messages in this test */ }, + stop() { /* nothing to tear down */ }, + send() { /* unused */ }, + notify() { /* unused */ }, + async request() { return {}; }, + sendResult() { /* unused */ }, + sendError() { /* unused */ }, +}); + +/** Send one line to a socket and resolve the first reply line carrying `ok`. */ +function control(socketPath: string, line: string): Promise | null> { + return new Promise((resolve) => { + const socket = net.createConnection(socketPath); + let buffered = ''; + const done = (value: Record | null): void => { + try { socket.destroy(); } catch { /* already gone */ } + resolve(value); + }; + socket.setEncoding('utf8'); + socket.on('connect', () => socket.write(`${line}\n`)); + socket.on('data', (chunk: string) => { + buffered += chunk; + for (const reply of buffered.split('\n')) { + try { + const parsed = JSON.parse(reply); + if (parsed && typeof parsed === 'object' && 'ok' in parsed) { done(parsed); return; } + } catch { /* the daemon hello, or a partial line */ } + } + }); + socket.on('error', () => done(null)); + socket.on('close', () => done(null)); + }); +} + +describe('parseDaemonControlLine', () => { + it('parses a control line, passing its fields through unvalidated', () => { + expect(parseDaemonControlLine('{"codegraph_control":1,"op":"clear-session-record","sessionId":"A"}')) + .toEqual({ op: 'clear-session-record', sessionId: 'A' }); + expect(parseDaemonControlLine('{"codegraph_control":1,"op":42}')) + .toEqual({ op: 42, sessionId: undefined }); + }); + + it('leaves a JSON-RPC first line alone — a client must not be hijacked', () => { + expect(parseDaemonControlLine('{"jsonrpc":"2.0","id":1,"method":"initialize"}')).toBeNull(); + expect(parseDaemonControlLine('{"codegraph_client":1,"pid":7}')).toBeNull(); + }); + + it('requires the exact marker', () => { + expect(parseDaemonControlLine('{"codegraph_control":true,"op":"x"}')).toBeNull(); + expect(parseDaemonControlLine('{"codegraph_control":2,"op":"x"}')).toBeNull(); + }); + + it('returns null for invalid / empty / non-object JSON', () => { + expect(parseDaemonControlLine('not json')).toBeNull(); + expect(parseDaemonControlLine('')).toBeNull(); + expect(parseDaemonControlLine('42')).toBeNull(); + expect(parseDaemonControlLine('null')).toBeNull(); + }); +}); + +describe('the daemon control line', () => { + /** + * A daemon serving two real sessions, reachable over a real socket. Only the + * engine is absent — the control path never touches it (that is half the + * point: a reset must not depend on an initialized index). + */ + async function serveDaemon(): Promise<{ + socketPath: string; + sessions: MCPSession[]; + clients: Set; + }> { + const root = tmpProject('cg-control-'); + // idleTimeoutMs 0 = never arm a real idle timer in a unit test. + const daemon = new Daemon(root, { idleTimeoutMs: 0 }) as unknown as { + handleConnection: (socket: net.Socket) => void; + clients: Set; + engine: { stop: () => void }; + }; + const engine = {} as MCPEngine; + const sessions = [new MCPSession(idleTransport(), engine), new MCPSession(idleTransport(), engine)]; + for (const session of sessions) daemon.clients.add(session); + // The engine the constructor built is never initialized here; stop it so no + // worker pool outlives the test. + try { daemon.engine.stop(); } catch { /* nothing started */ } + + const socketPath = getDaemonSocketCandidates(root)[0]!; + const server = net.createServer((socket) => daemon.handleConnection(socket)); + servers.push(server); + await new Promise((resolve) => server.listen(socketPath, () => resolve())); + return { socketPath, sessions, clients: daemon.clients }; + } + + it('clears the named caller everywhere and reports how much it dropped', async () => { + const { socketPath, sessions, clients } = await serveDaemon(); + // The caller knows its context id, not which connection carries it: bucket + // "A" exists on both sessions, "B" only on the second. + sessions[0]!.getExploreSessionState().record(emission('/repo/one'), 'A'); + sessions[1]!.getExploreSessionState().record(emission('/repo/two'), 'A'); + sessions[1]!.getExploreSessionState().record(emission('/repo/two'), 'B'); + + expect(await control(socketPath, '{"codegraph_control":1,"op":"clear-session-record","sessionId":"A"}')) + .toEqual({ ok: true, cleared: 2 }); + + expect(sessions[0]!.getExploreSessionState().view('A').projects).toEqual([]); + expect(sessions[1]!.getExploreSessionState().view('A').projects).toEqual([]); + // B was never named, so B still holds everything it was served. + expect(sessions[1]!.getExploreSessionState().view('B').projects[0]?.callCount).toBe(1); + // A control connection is a command, not a client — it must not register as + // one (that would make it count against the daemon's idle lifecycle). + expect(clients.size).toBe(2); + }); + + it('refuses an unknown op and a non-string id, and keeps serving after both', async () => { + const { socketPath, sessions } = await serveDaemon(); + sessions[0]!.getExploreSessionState().record(emission('/repo/one'), 'A'); + + expect(await control(socketPath, '{"codegraph_control":1,"op":"drop-everything","sessionId":"A"}')) + .toEqual({ ok: false }); + expect(await control(socketPath, '{"codegraph_control":1,"op":"clear-session-record","sessionId":{"nope":1}}')) + .toEqual({ ok: false }); + // Nothing was cleared by either, and the daemon still answers a real one. + expect(sessions[0]!.getExploreSessionState().view('A').projects[0]?.callCount).toBe(1); + expect(await control(socketPath, '{"codegraph_control":1,"op":"clear-session-record","sessionId":"A"}')) + .toEqual({ ok: true, cleared: 1 }); + }); + + it('reports zero for a caller nobody has a record for', async () => { + const { socketPath } = await serveDaemon(); + expect(await control(socketPath, '{"codegraph_control":1,"op":"clear-session-record","sessionId":"ghost"}')) + .toEqual({ ok: true, cleared: 0 }); + }); +}); + +/** + * The CLI hook entry points. Run against the BUILT binary, because the contract + * under test is a process contract — exit status and stdout bytes — not a + * function's return value. + */ +describe('codegraph hooks', () => { + /** + * Spawned ASYNC on purpose: the fake daemon below serves from this very event + * loop, and a synchronous spawn would block it — the hook would then time out + * against a server that never got a turn to answer. + */ + function runHook(args: string[], stdin: string, cwd?: string): Promise<{ status: number | null; stdout: string }> { + return new Promise((resolve) => { + const child = spawn(process.execPath, [BIN, 'hooks', ...args], { + cwd, + // Skip the daemon spawn and the wasm re-exec: a hook must resolve in one + // fast process, and the test asserts what that process alone printed. + env: { ...process.env, CODEGRAPH_NO_DAEMON: '1', CODEGRAPH_WASM_RELAUNCHED: '1' }, + }); + let stdout = ''; + child.stdout.setEncoding('utf-8'); + child.stdout.on('data', (chunk: string) => { stdout += chunk; }); + child.on('close', (status) => resolve({ status, stdout })); + child.stdin.on('error', () => { /* a hook may never read stdin */ }); + child.stdin.end(stdin); + }); + } + + describe('pre-tool-use', () => { + const payload = (over: Record): string => JSON.stringify({ + session_id: 'sess-1', + hook_event_name: 'PreToolUse', + tool_name: 'mcp__codegraph__codegraph_explore', + tool_input: { query: 'AuthService login', maxFiles: 8 }, + ...over, + }); + + it('stamps the main agent with the bare session id, keeping the original input', async () => { + const { status, stdout } = await runHook(['pre-tool-use'], payload({})); + expect(status).toBe(0); + expect(JSON.parse(stdout)).toEqual({ + hookSpecificOutput: { + hookEventName: 'PreToolUse', + updatedInput: { query: 'AuthService login', maxFiles: 8, sessionId: 'sess-1' }, + }, + }); + }); + + it('scopes a subagent to its session — the whole point of the hook', async () => { + const { status, stdout } = await runHook(['pre-tool-use'], payload({ agent_id: 'agent-42' })); + expect(status).toBe(0); + expect(JSON.parse(stdout)).toEqual({ + hookSpecificOutput: { + hookEventName: 'PreToolUse', + // Composite, not the bare agent id: a host that numbers agents only + // per-session would otherwise cross-bucket two sessions' subagents. + updatedInput: { query: 'AuthService login', maxFiles: 8, sessionId: 'sess-1:agent-42' }, + }, + }); + }); + + /** + * `permissionDecision` is opt-in per agent, declared by the wiring rather + * than sniffed from the payload. Codex requires it paired with the rewrite + * or discards the rewrite; Claude Code ACTS on it, so "allow" there would + * override a user who chose not to allowlist codegraph. The default has to + * be omission, and an unrecognized agent has to fall back to the default — + * a wrong guess in this direction is a privilege escalation. + */ + it('pairs permissionDecision with the rewrite for --agent codex', async () => { + const { status, stdout } = await runHook(['pre-tool-use', '--agent', 'codex'], payload({})); + expect(status).toBe(0); + expect(JSON.parse(stdout)).toEqual({ + hookSpecificOutput: { + hookEventName: 'PreToolUse', + permissionDecision: 'allow', + updatedInput: { query: 'AuthService login', maxFiles: 8, sessionId: 'sess-1' }, + }, + }); + }); + + it('omits permissionDecision by default and for any other agent', async () => { + for (const args of [ + ['pre-tool-use'], + ['pre-tool-use', '--agent', 'claude'], + ['pre-tool-use', '--agent', 'not-an-agent'], + ['pre-tool-use', '--agent', 'Codex'], + ]) { + const { status, stdout } = await runHook(args, payload({})); + expect(status, args.join(' ')).toBe(0); + expect(JSON.parse(stdout).hookSpecificOutput, args.join(' ')) + .not.toHaveProperty('permissionDecision'); + // Only that one field differs — the stamp is identical either way. + expect(JSON.parse(stdout).hookSpecificOutput.updatedInput, args.join(' ')) + .toEqual({ query: 'AuthService login', maxFiles: 8, sessionId: 'sess-1' }); + } + }); + + it('keeps the main context and its own subagent in different buckets', async () => { + const idFor = async (over: Record): Promise => + JSON.parse((await runHook(['pre-tool-use'], payload(over))).stdout) + .hookSpecificOutput.updatedInput.sessionId; + expect(await idFor({})).not.toBe(await idFor({ agent_id: 'agent-42' })); + // Same agent id under a different session is a different bucket too. + expect(await idFor({ agent_id: 'agent-42' })) + .not.toBe(await idFor({ session_id: 'sess-2', agent_id: 'agent-42' })); + }); + + it('says nothing about a tool that is not explore', async () => { + const { status, stdout } = await runHook(['pre-tool-use'], payload({ tool_name: 'Read' })); + expect(status).toBe(0); + expect(stdout).toBe(''); + }); + + it('exits 0 with empty stdout on junk, empty, or id-less input', async () => { + for (const input of ['not json', '', '{}', payload({ session_id: '', agent_id: '' })]) { + const { status, stdout } = await runHook(['pre-tool-use'], input); + expect(status, `input: ${input}`).toBe(0); + expect(stdout, `input: ${input}`).toBe(''); + } + }); + }); + + describe('post-compact', () => { + /** Stand in for the daemon: greet, capture one line, answer it. */ + async function fakeDaemon(root: string): Promise<{ received: string[] }> { + const received: string[] = []; + const server = net.createServer((socket) => { + socket.write('{"codegraph":"0.0.0","pid":1,"socketPath":"x","protocol":1}\n'); + let buffered = ''; + socket.setEncoding('utf8'); + socket.on('data', (chunk: string) => { + buffered += chunk; + const nl = buffered.indexOf('\n'); + if (nl === -1) return; + received.push(buffered.slice(0, nl)); + buffered = ''; + socket.end('{"ok":true,"cleared":1}\n'); + }); + socket.on('error', () => { /* client may vanish */ }); + }); + servers.push(server); + await new Promise((resolve) => server.listen(getDaemonSocketCandidates(root)[0]!, () => resolve())); + return { received }; + } + + it('relays the compacted context id to the project daemon', async () => { + const root = tmpProject('cg-postcompact-'); + const daemon = await fakeDaemon(root); + const { status } = await runHook(['post-compact'], JSON.stringify({ + session_id: 'sess-9', cwd: root, hook_event_name: 'PostCompact', trigger: 'auto', + })); + expect(status).toBe(0); + expect(JSON.parse(daemon.received[0]!)).toEqual({ + codegraph_control: 1, op: 'clear-session-record', sessionId: 'sess-9', + }); + }); + + it('relays a subagent id, and honours an explicit --session-id with no payload', async () => { + const root = tmpProject('cg-postcompact-agent-'); + const daemon = await fakeDaemon(root); + + expect((await runHook(['post-compact'], JSON.stringify({ + session_id: 'sess-9', agent_id: 'agent-42', cwd: root, + }))).status).toBe(0); + // The reset must spell the id exactly as the stamping hook did. + expect(JSON.parse(daemon.received[0]!).sessionId).toBe('sess-9:agent-42'); + + // No hook payload at all — the relay path (LLMMesh), which knows the id + // but has nothing on stdin to derive it from. + expect((await runHook(['post-compact', '--path', root, '--session-id', 'relayed-7'], '')).status).toBe(0); + expect(JSON.parse(daemon.received[1]!).sessionId).toBe('relayed-7'); + }); + + it('exits 0 when no daemon is listening, on an unindexed cwd, and on junk input', async () => { + const indexed = tmpProject('cg-postcompact-nodaemon-'); + const bare = fs.realpathSync(fs.mkdtempSync(path.join(os.tmpdir(), 'cg-postcompact-bare-'))); + tmpDirs.push(bare); + const payload = (cwd: string): string => JSON.stringify({ session_id: 'sess-9', cwd }); + + // Indexed but nothing is serving; nothing indexed at all; unparseable. + for (const [label, input] of [ + ['no daemon', payload(indexed)], + ['unindexed', payload(bare)], + ['junk', 'not json'], + ] as const) { + const { status, stdout } = await runHook(['post-compact'], input, bare); + expect(status, label).toBe(0); + expect(stdout, label).toBe(''); + } + }); + }); +}); diff --git a/__tests__/explore-session-state.test.ts b/__tests__/explore-session-state.test.ts index c0b71d9a6..348be65a4 100644 --- a/__tests__/explore-session-state.test.ts +++ b/__tests__/explore-session-state.test.ts @@ -30,6 +30,7 @@ import { ExploreSessionState, coalesceRanges, exploreProjectKey, + normalizeExploreSessionId, rangesCover, readExploreSessionView, viewForProject, @@ -158,6 +159,112 @@ describe('ExploreSessionState — the container', () => { }); }); +describe('per-caller buckets', () => { + /** + * The record is per MCP connection, but a connection can carry several agent + * contexts (Claude Code subagents dispatch over the parent's). A caller id + * buckets the history so no context is ever told it already holds source that + * went to a sibling — the failure that costs a Read. + */ + it('keeps two callers on one session from seeing each other\'s calls', () => { + const state = new ExploreSessionState(); + state.record(emission('/repo/a'), 'A'); + state.record(emission('/repo/a'), 'A'); + + expect(state.view('A').projects).toHaveLength(1); + expect(state.view('A').projects[0]!.callCount).toBe(2); + expect(state.callCount('/repo/a', 'A')).toBe(2); + // B and the unidentified bucket have been served nothing. + expect(state.view('B').projects).toEqual([]); + expect(state.view().projects).toEqual([]); + expect(state.callCount('/repo/a')).toBe(0); + }); + + it('files a call with no caller id under the default bucket, as before', () => { + const state = new ExploreSessionState(); + state.record(emission('/repo/a')); + expect(state.view().projects[0]?.callCount).toBe(1); + expect(state.forProject('/repo/a')?.callCount).toBe(1); + expect(state.view('A').projects).toEqual([]); + }); + + it('treats an unusable caller id as no id rather than as its own bucket', () => { + const state = new ExploreSessionState(); + state.record(emission('/repo/a'), 42 as unknown as string); + state.record(emission('/repo/a'), ' '); + expect(state.view().projects[0]?.callCount).toBe(2); + expect(state.snapshot()).toHaveLength(1); + }); + + it('normalizes a caller id to an opaque, bounded label', () => { + expect(normalizeExploreSessionId(' main:abc ')).toBe('main:abc'); + expect(normalizeExploreSessionId('')).toBeUndefined(); + expect(normalizeExploreSessionId(' ')).toBeUndefined(); + expect(normalizeExploreSessionId(undefined)).toBeUndefined(); + expect(normalizeExploreSessionId(42)).toBeUndefined(); + expect(normalizeExploreSessionId({ id: 'A' })).toBeUndefined(); + // Past the cap the id is hashed, not cut: a fixed-size label that is still + // the same one every time. + const long = normalizeExploreSessionId('x'.repeat(200)); + expect(long).toMatch(/^[0-9a-f]{64}$/); + expect(normalizeExploreSessionId('x'.repeat(200))).toBe(long); + }); + + it('keeps two over-long ids apart when they differ only past the cap', () => { + // Truncating would collapse these into ONE bucket — a record shared between + // two contexts, which is the very bug bucketing exists to fix. + expect(normalizeExploreSessionId('x'.repeat(200))) + .not.toBe(normalizeExploreSessionId('x'.repeat(129))); + expect(normalizeExploreSessionId(`${'x'.repeat(128)}A`)) + .not.toBe(normalizeExploreSessionId(`${'x'.repeat(128)}B`)); + }); + + it('serves an over-long id from its own bucket, never its 128-char prefix', () => { + const state = new ExploreSessionState(); + state.record(emission('/repo/a'), 'x'.repeat(200)); + expect(state.view('x'.repeat(200)).projects[0]?.callCount).toBe(1); + // A caller whose id IS the truncation must not be handed the other's record. + expect(state.view('x'.repeat(128)).projects).toEqual([]); + expect(state.snapshot()).toHaveLength(1); + }); + + it('clears one caller\'s record without touching another\'s', () => { + const state = new ExploreSessionState(); + state.record(emission('/repo/a'), 'A'); + state.record(emission('/repo/b'), 'A'); + state.record(emission('/repo/a'), 'B'); + + expect(state.clearSessionRecord('A')).toBe(2); + expect(state.view('A').projects).toEqual([]); + expect(state.view('B').projects[0]?.callCount).toBe(1); + expect(state.clearSessionRecord('A')).toBe(0); + }); + + it('never lets an unusable id clear the bucket everyone else shares', () => { + const state = new ExploreSessionState(); + state.record(emission('/repo/a')); + expect(state.clearSessionRecord('')).toBe(0); + expect(state.clearSessionRecord(' ')).toBe(0); + expect(state.clearSessionRecord(undefined as unknown as string)).toBe(0); + expect(state.view().projects[0]?.callCount).toBe(1); + }); + + it('bounds (caller, project) entries together — eviction spans buckets', () => { + // Accepted cost of one shared bound: a wide fan-out evicts the least + // recently active caller's record, which re-serves (safe) rather than + // pointing anyone at source they never received. + const state = new ExploreSessionState(); + for (let i = 0; i < EXPLORE_SESSION_LIMITS.MAX_PROJECTS; i++) { + state.record(emission('/repo/a'), `caller-${i}`); + } + expect(state.snapshot()).toHaveLength(EXPLORE_SESSION_LIMITS.MAX_PROJECTS); + state.record(emission('/repo/a'), 'caller-last'); + expect(state.snapshot()).toHaveLength(EXPLORE_SESSION_LIMITS.MAX_PROJECTS); + expect(state.view('caller-0').projects).toEqual([]); + expect(state.view('caller-last').projects[0]?.callCount).toBe(1); + }); +}); + describe('range bookkeeping', () => { it('merges overlapping and adjacent spans into one', () => { const { ranges, truncated } = coalesceRanges([ diff --git a/__tests__/installer-targets.test.ts b/__tests__/installer-targets.test.ts index 4ec3e5903..812a2bc15 100644 --- a/__tests__/installer-targets.test.ts +++ b/__tests__/installer-targets.test.ts @@ -22,7 +22,14 @@ import { parse as parseJsonc } from 'jsonc-parser'; import { ALL_TARGETS, getTarget, resolveTargetFlag } from '../src/installer/targets/registry'; import { uninstallTargets, refreshTargets } from '../src/installer'; import { upsertTomlTable, removeTomlTable, buildTomlTable } from '../src/installer/targets/toml'; -import { cleanupLegacyHooks, writePromptHookEntry, removePromptHookEntry } from '../src/installer/targets/claude'; +import { + cleanupLegacyHooks, + writePromptHookEntry, + removePromptHookEntry, + writeSessionHookEntries, + removeSessionHookEntries, +} from '../src/installer/targets/claude'; +import { codexHookTrustHash } from '../src/installer/targets/codex'; function mkTmpDir(label: string): string { return fs.mkdtempSync(path.join(os.tmpdir(), `cg-targets-${label}-`)); @@ -127,9 +134,14 @@ describe('Installer targets — contract', () => { // and verify the sibling survives. Skip for Codex (TOML) // and any target with no JSON config — they get covered // by their own dedicated tests below. - const paths = target.describePaths(location); + // + // Keyed on detect()'s configPath — the target's own answer for + // "where does my MCP entry live" — rather than scanning + // describePaths for a .json, which also turns up files that are + // written but hold no MCP config (codex's hooks.json). + const mcpConfig = target.detect(location).configPath; // Match .json or .jsonc — opencode prefers .jsonc. - const jsonPath = paths.find((p) => /\.jsonc?$/.test(p)); + const jsonPath = /\.jsonc?$/.test(mcpConfig) ? mcpConfig : undefined; if (!jsonPath) return; // Seed pre-existing config. @@ -957,7 +969,11 @@ describe('Installer targets — partial-state idempotency', () => { const afterInstall = fs.readFileSync(tomlPath, 'utf-8'); expect(afterInstall).toContain('command = "codegraph"'); expect(afterInstall).not.toContain('[[not-a-table]]'); - expect(afterInstall.endsWith(historyTables)).toBe(true); + // The array-of-tables survives verbatim. It no longer ends the file — the + // hooks' trust records append after it, which is what closes the last + // `[[history]]` element — but not a byte of it is rewritten or reordered. + expect(afterInstall).toContain(historyTables.trimEnd()); + expect(afterInstall.match(/\[\[history\]\]/g)).toHaveLength(2); const second = codex.install('global', { autoAllow: false }); expect(second.files.find((f) => f.path === tomlPath)?.action).toBe('unchanged'); @@ -967,6 +983,246 @@ describe('Installer targets — partial-state idempotency', () => { expect(fs.readFileSync(tomlPath, 'utf-8')).toBe(historyTables); }); + // ---- Codex per-context session hooks (hooks.json) ---- + // Codex reads hooks from a hooks.json in each config layer's .codex/ folder. + // Same two entries as the Claude target, spelled codex's way: PreToolUse with + // a regex over the tool name, and PostCompact with no matcher at all. Trust + // is held per ENTRY in config.toml's + // [hooks.state], which the installer must never write — adding entries leaves + // the user's already-trusted hooks trusted, and ours await their review. + const hooksPathFor = (loc: 'global' | 'local'): string => + path.join(loc === 'global' ? tmpHome : tmpCwd, '.codex', 'hooks.json'); + const hookCommandsFor = (file: string, event: string): string[] => { + const root = JSON.parse(fs.readFileSync(file, 'utf-8')); + return (root.hooks?.[event] ?? []).flatMap((g: any) => (g.hooks ?? []).map((h: any) => h.command)); + }; + + for (const location of ['global', 'local'] as const) { + it(`codex: install writes both session hooks to ${location} hooks.json`, () => { + const result = getTarget('codex')!.install(location, { autoAllow: false }); + const file = hooksPathFor(location); + expect(fs.existsSync(file)).toBe(true); + expect(result.files.find((f) => f.path === file)?.action).toBe('created'); + + const root = JSON.parse(fs.readFileSync(file, 'utf-8')); + const preToolUse = root.hooks.PreToolUse.find((g: any) => + (g.hooks ?? []).some((h: any) => h.command.includes('hooks pre-tool-use'))); + expect(preToolUse.matcher).toBe('codegraph_explore'); + expect(preToolUse.hooks[0].type).toBe('command'); + // --agent codex is what makes the hook pair permissionDecision with the + // rewrite; codex drops an unpaired one, and no other agent gets the field. + expect(preToolUse.hooks[0].command).toMatch(/hooks pre-tool-use --agent codex$/); + // PostCompact: codex fires it only when compaction succeeded, and every + // failure path returns before the history rewrite commits — so the skip + // on failure is correct, and PreCompact would clear a still-valid record. + expect(hookCommandsFor(file, 'PostCompact').some((c) => c.includes('hooks post-compact'))).toBe(true); + expect(root.hooks.PreCompact).toBeUndefined(); + + // The hooks.json write surfaces ONLY as the Created/Updated file line + // asserted above — no prose about it. Codex's own TUI is where untrusted + // hooks surface, so the install output stays a list of file actions. + expect((result.notes ?? []).some((n) => n.includes('hooks.json'))).toBe(false); + }); + } + + /** + * The trust hash reimplements a codex internal, so it is pinned against a + * REAL trusted pair — a hooks.json group and the `trusted_hash` codex itself + * wrote for it. If codex changes its normalization this test fails loudly + * here rather than silently leaving users with entries codex reports as + * needing review. + */ + it('codex: reproduces the trust hash codex itself writes', () => { + // Verbatim from a real ~/.codex/hooks.json + config.toml pair: a PreToolUse + // group matching `spawn_agent$` with one command handler and no timeout. + expect(codexHookTrustHash( + 'PreToolUse', + 'spawn_agent$', + '/home/atkins/.codex/hooks/subagent.py', + 'Validating subagent runtime profile', + )).toBe('sha256:dd9550baea8600a2d1a58f765a532ac503baf8c786cd4c0d3b76742874961733'); + expect(codexHookTrustHash( + 'PostToolUse', + 'spawn_agent$', + '/home/atkins/.codex/hooks/subagent.py', + 'Rendering subagent runtime profile', + )).toBe('sha256:e5c757d1ea8dbffed4b43dd9f8a994bcb60e927b30a60d8d488972e8eef62d0d'); + // A matcher-less group omits the key entirely rather than sending an empty + // string — TOML has no null, so codex's own serialization drops it. + expect(codexHookTrustHash('PostCompact', undefined, 'x')) + .not.toBe(codexHookTrustHash('PostCompact', '', 'x')); + }); + + it('codex: reports the hooks file as a plain file action, nothing more', () => { + const result = getTarget('codex')!.install('global', { autoAllow: false }); + // Pre-trusting is not narrated anywhere in the output — the hooks file + // reports exactly like every other file the installer touches. + expect(result.files.find((f) => f.path === hooksPathFor('global'))) + .toEqual({ path: hooksPathFor('global'), action: 'created' }); + expect((result.notes ?? []).some((n) => /trust/i.test(n))).toBe(false); + }); + + it('codex: records ONLY our two entries, with codex-shaped keys', () => { + getTarget('codex')!.install('global', { autoAllow: false }); + const tomlPath = path.join(tmpHome, '.codex', 'config.toml'); + const toml = fs.readFileSync(tomlPath, 'utf-8'); + const hooksFile = hooksPathFor('global'); + + // Key is :::. + for (const event of ['pre_tool_use', 'post_compact']) { + expect(toml).toContain(`[hooks.state."${hooksFile}:${event}:0:0"]`); + } + // Exactly two records, and each hash matches what our hooks.json says. + expect(toml.match(/trusted_hash/g)).toHaveLength(2); + const root = JSON.parse(fs.readFileSync(hooksFile, 'utf-8')); + const preCmd = root.hooks.PreToolUse[0].hooks[0].command; + expect(toml).toContain(codexHookTrustHash('PreToolUse', 'codegraph_explore', preCmd)); + expect(toml).toContain(codexHookTrustHash('PostCompact', undefined, root.hooks.PostCompact[0].hooks[0].command)); + }); + + it('codex: auto-trust never rewrites the user\'s own trust records', () => { + const tomlPath = path.join(tmpHome, '.codex', 'config.toml'); + fs.mkdirSync(path.dirname(tomlPath), { recursive: true }); + fs.writeFileSync(tomlPath, [ + '[hooks.state."/home/u/.codex/hooks.json:pre_tool_use:0:0"]', + 'trusted_hash = "sha256:theirs"', + 'enabled = true', + '', + ].join('\n')); + + getTarget('codex')!.install('global', { autoAllow: false }); + const toml = fs.readFileSync(tomlPath, 'utf-8'); + // Trust is per entry, so theirs is unreachable from ours — byte-for-byte. + expect(toml).toContain('[hooks.state."/home/u/.codex/hooks.json:pre_tool_use:0:0"]'); + expect(toml).toContain('trusted_hash = "sha256:theirs"'); + expect(toml).toContain('enabled = true'); + }); + + it('codex: re-trusting an upgraded launcher keeps the user\'s enabled flag', () => { + const codex = getTarget('codex')!; + codex.install('global', { autoAllow: false }); + const tomlPath = path.join(tmpHome, '.codex', 'config.toml'); + const key = `${hooksPathFor('global')}:pre_tool_use:0:0`; + + // The user disabled our hook in the Codex TUI, then upgraded — the launcher + // path is part of the hashed identity, so the hash must be refreshed + // WITHOUT re-enabling a hook they turned off. + let toml = fs.readFileSync(tomlPath, 'utf-8'); + toml = toml.replace(`[hooks.state."${key}"]`, `[hooks.state."${key}"]\nenabled = false`); + fs.writeFileSync(tomlPath, toml); + // Only the binary moves; the subcommand must survive or this stops being an + // upgrade and becomes an unrecognized hook. + const stale = fs.readFileSync(hooksPathFor('global'), 'utf-8') + .replace('"command": "codegraph hooks pre-tool-use', '"command": "/old/bin/codegraph hooks pre-tool-use'); + fs.writeFileSync(hooksPathFor('global'), stale); + const staleHash = codexHookTrustHash('PreToolUse', 'codegraph_explore', '/old/bin/codegraph hooks pre-tool-use --agent codex'); + + codex.install('global', { autoAllow: false }); + const after = fs.readFileSync(tomlPath, 'utf-8'); + expect(after).toContain('enabled = false'); + expect(after.match(/trusted_hash/g)).toHaveLength(2); + // Refreshed to the new launcher, in place — not left on the old one. + expect(after).not.toContain(staleHash); + expect(after).toContain(codexHookTrustHash('PreToolUse', 'codegraph_explore', 'codegraph hooks pre-tool-use --agent codex')); + }); + + it('codex: auto-trust is idempotent and uninstall removes only our records', () => { + const codex = getTarget('codex')!; + const tomlPath = path.join(tmpHome, '.codex', 'config.toml'); + fs.mkdirSync(path.dirname(tomlPath), { recursive: true }); + fs.writeFileSync(tomlPath, '[hooks.state."other:pre_tool_use:0:0"]\ntrusted_hash = "sha256:theirs"\n'); + + codex.install('global', { autoAllow: false }); + const first = fs.readFileSync(tomlPath, 'utf-8'); + codex.install('global', { autoAllow: false }); + expect(fs.readFileSync(tomlPath, 'utf-8')).toBe(first); + + codex.uninstall('global'); + const after = fs.readFileSync(tomlPath, 'utf-8'); + expect(after).toContain('other:pre_tool_use:0:0'); + expect(after).toContain('sha256:theirs'); + expect(after).not.toContain(hooksPathFor('global')); + }); + + it('codex: hooks install is idempotent (no duplicate, byte-identical re-run)', () => { + const codex = getTarget('codex')!; + const file = hooksPathFor('global'); + codex.install('global', { autoAllow: false }); + const first = fs.readFileSync(file, 'utf-8'); + + const second = codex.install('global', { autoAllow: false }); + expect(second.files.find((f) => f.path === file)?.action).toBe('unchanged'); + expect(fs.readFileSync(file, 'utf-8')).toBe(first); + expect(hookCommandsFor(file, 'PreToolUse')).toHaveLength(1); + expect(hookCommandsFor(file, 'PostCompact')).toHaveLength(1); + }); + + it('codex: install preserves the user\'s own hooks and unrelated top-level keys', () => { + const file = hooksPathFor('global'); + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync(file, JSON.stringify({ + description: 'my hooks', + hooks: { + PreToolUse: [{ matcher: '^Bash$', hooks: [{ type: 'command', command: 'my-guard' }] }], + SessionEnd: [{ hooks: [{ type: 'command', command: 'my-cleanup' }] }], + }, + }, null, 2) + '\n'); + + getTarget('codex')!.install('global', { autoAllow: false }); + const root = JSON.parse(fs.readFileSync(file, 'utf-8')); + expect(root.description).toBe('my hooks'); + expect(hookCommandsFor(file, 'SessionEnd')).toEqual(['my-cleanup']); + // Ours is APPENDED — codex keys a hook's trust record on its group index, + // so inserting ahead of the user's groups would renumber (and untrust) them. + expect(root.hooks.PreToolUse[0]).toEqual({ + matcher: '^Bash$', hooks: [{ type: 'command', command: 'my-guard' }], + }); + expect(root.hooks.PreToolUse).toHaveLength(2); + }); + + it('codex: uninstall removes exactly our entries and leaves the user\'s', () => { + const file = hooksPathFor('global'); + fs.mkdirSync(path.dirname(file), { recursive: true }); + fs.writeFileSync(file, JSON.stringify({ + description: 'my hooks', + hooks: { PreToolUse: [{ matcher: '^Bash$', hooks: [{ type: 'command', command: 'my-guard' }] }] }, + }, null, 2) + '\n'); + + const codex = getTarget('codex')!; + codex.install('global', { autoAllow: false }); + codex.uninstall('global'); + + const root = JSON.parse(fs.readFileSync(file, 'utf-8')); + expect(root.description).toBe('my hooks'); + expect(hookCommandsFor(file, 'PreToolUse')).toEqual(['my-guard']); + expect(root.hooks.PostCompact).toBeUndefined(); + }); + + it('codex: uninstall deletes a hooks.json that held only our entries', () => { + const codex = getTarget('codex')!; + const file = hooksPathFor('global'); + codex.install('global', { autoAllow: false }); + expect(fs.existsSync(file)).toBe(true); + + codex.uninstall('global'); + // We created it and nothing else ever lived there — no empty husk left. + expect(fs.existsSync(file)).toBe(false); + }); + + it('codex: leaves an unparseable hooks.json exactly as it is', () => { + const file = hooksPathFor('global'); + fs.mkdirSync(path.dirname(file), { recursive: true }); + const broken = '{ "hooks": { "PreToolUse": [ }}} not json'; + fs.writeFileSync(file, broken); + + const result = getTarget('codex')!.install('global', { autoAllow: false }); + // Never clobbered, never backed up and replaced: codex would then run our + // file instead of the config we failed to read. + expect(fs.readFileSync(file, 'utf-8')).toBe(broken); + expect(result.files.find((f) => f.path === file)?.action).toBe('unchanged'); + expect(fs.existsSync(`${file}.backup`)).toBe(false); + }); + it('claude: local install writes ./.mcp.json (project scope), not ./.claude.json', () => { const claude = getTarget('claude')!; const result = claude.install('local', { autoAllow: false }); @@ -1305,6 +1561,208 @@ describe('Installer targets — partial-state idempotency', () => { const stopCmds = (s.hooks?.Stop ?? []).flatMap((g: any) => (g.hooks ?? []).map((h: any) => h.command)); expect(stopCmds).toContain('codegraph sync-if-dirty'); }); + + // ---- Per-context session hooks (PreToolUse / PostCompact) ---- + // These are the delivery mechanism for per-context dedup: without them the + // already-sent record stays keyed per MCP connection and a subagent — + // dispatching over its parent's — is told source was "already sent" to a + // context that never received it. Written unconditionally by install (no + // opt-in flag), so the surgery has to be exact: the user's own hooks under + // the same events survive install AND uninstall. + const commandsFor = (s: any, event: string): string[] => + (s.hooks?.[event] ?? []).flatMap((g: any) => (g.hooks ?? []).map((h: any) => h.command)); + const readSettings = (loc: 'global' | 'local'): any => + JSON.parse(fs.readFileSync(path.join(loc === 'global' ? tmpHome : tmpCwd, '.claude', 'settings.json'), 'utf-8')); + + it('claude: install wires both session hooks under the right events', () => { + getTarget('claude')!.install('global', { autoAllow: true }); + const s = readSettings('global'); + + const preToolUse = s.hooks.PreToolUse.find((g: any) => + (g.hooks ?? []).some((h: any) => h.command.includes('hooks pre-tool-use'))); + // The MCP server name is the user's to rename, so the matcher keys on the + // tool SUFFIX rather than a fixed mcp__codegraph__ prefix. + expect(preToolUse.matcher).toBe('mcp__.*__codegraph_explore'); + expect(new RegExp(preToolUse.matcher).test('mcp__my-renamed-server__codegraph_explore')).toBe(true); + + expect(commandsFor(s, 'PostCompact').some((c: string) => c.includes('hooks post-compact'))).toBe(true); + // PostCompact fires once compaction has completed — PreCompact would run + // while the agent still holds the source, so it is deliberately not used, + // and neither is SessionStart (this event covers auto-compact explicitly). + expect(s.hooks.PreCompact).toBeUndefined(); + expect(s.hooks.SessionStart).toBeUndefined(); + + for (const event of ['PreToolUse', 'PostCompact']) { + expect(commandsFor(s, event).every((c: string) => c.includes('codegraph'))).toBe(true); + } + }); + + it('claude: session hooks install is idempotent (no duplicate, byte-identical re-run)', () => { + const claude = getTarget('claude')!; + const file = path.join(tmpHome, '.claude', 'settings.json'); + claude.install('global', { autoAllow: true }); + const first = fs.readFileSync(file, 'utf-8'); + expect(writeSessionHookEntries('global').action).toBe('unchanged'); + claude.install('global', { autoAllow: true }); + expect(fs.readFileSync(file, 'utf-8')).toBe(first); + + const s = JSON.parse(first); + expect(commandsFor(s, 'PreToolUse').filter((c: string) => c.includes('hooks pre-tool-use'))).toHaveLength(1); + expect(commandsFor(s, 'PostCompact').filter((c: string) => c.includes('hooks post-compact'))).toHaveLength(1); + }); + + // The user's PostCompact hook carries NO matcher — the shape that event uses — + // so this doubles as coverage that a matcher-less group survives our surgery. + const userHooks = (): Record => ({ + hooks: { + PreToolUse: [{ matcher: 'Bash', hooks: [{ type: 'command', command: 'my-bash-guard' }] }], + PostCompact: [{ hooks: [{ type: 'command', command: 'my-compact-note' }] }], + }, + }); + + it('claude: install preserves the user\'s own hooks under the same events', () => { + seedSettings('global', userHooks()); + getTarget('claude')!.install('global', { autoAllow: true }); + const s = readSettings('global'); + expect(commandsFor(s, 'PreToolUse')).toContain('my-bash-guard'); + expect(commandsFor(s, 'PostCompact')).toContain('my-compact-note'); + // The user's groups are untouched — ours is appended as its own. + expect(s.hooks.PreToolUse[0]).toEqual({ matcher: 'Bash', hooks: [{ type: 'command', command: 'my-bash-guard' }] }); + expect(s.hooks.PostCompact[0]).toEqual({ hooks: [{ type: 'command', command: 'my-compact-note' }] }); + }); + + it('claude: uninstall removes exactly our two entries and leaves the user\'s', () => { + seedSettings('global', userHooks()); + getTarget('claude')!.install('global', { autoAllow: true }); + getTarget('claude')!.uninstall('global'); + const s = readSettings('global'); + expect(commandsFor(s, 'PreToolUse')).toEqual(['my-bash-guard']); + // A matcher-less sibling group survives verbatim: the removal keys on the + // command, never on the group's matcher. + expect(s.hooks.PostCompact).toEqual([{ hooks: [{ type: 'command', command: 'my-compact-note' }] }]); + }); + + it('claude: uninstall reverses install, leaving no hook structures behind', () => { + const claude = getTarget('claude')!; + claude.install('global', { autoAllow: true }); + claude.uninstall('global'); + const s = readSettings('global'); + // Nothing else lived under these events, so the events — and `hooks` + // itself, which we created — are pruned rather than left as husks. + expect(s.hooks).toBeUndefined(); + }); + + it('claude: re-install re-points a session hook whose binary path went stale', () => { + seedSettings('global', { + hooks: { + PreToolUse: [{ + matcher: 'mcp__.*__codegraph_explore', + hooks: [{ type: 'command', command: '/gone/versions/v0.0.1/bin/codegraph hooks pre-tool-use' }], + }], + }, + }); + expect(writeSessionHookEntries('global').action).toBe('updated'); + const s = readSettings('global'); + // Rewritten in place, never duplicated: recognition is by the subcommand + // WITHOUT its flags, so both a path from an older install and a flag set + // that has since changed are ours to heal — the seed above carries neither + // the current path nor `--agent`, and comes back with both. + const pre = commandsFor(s, 'PreToolUse'); + expect(pre).toHaveLength(1); + expect(pre[0]).not.toContain('/gone/'); + expect(pre[0]).toContain('hooks pre-tool-use --agent claude'); + }); + + it('claude: session-hook removal leaves the prompt hook and legacy hooks alone', () => { + seedSettings('global', { + hooks: { + UserPromptSubmit: [{ hooks: [{ type: 'command', command: HOOK_CMD }] }], + Stop: [{ hooks: [{ type: 'command', command: 'codegraph sync-if-dirty' }] }], + }, + }); + writeSessionHookEntries('global'); + expect(removeSessionHookEntries('global').action).toBe('removed'); + const s = readSettings('global'); + expect(commandsFor(s, 'UserPromptSubmit')).toEqual([HOOK_CMD]); + expect(commandsFor(s, 'Stop')).toEqual(['codegraph sync-if-dirty']); + expect(s.hooks.PreToolUse).toBeUndefined(); + expect(s.hooks.PostCompact).toBeUndefined(); + }); + + it('claude: install never overwrites a non-array under a hook event', () => { + seedSettings('global', { hooks: { PreToolUse: { unexpected: 'user shape' } } }); + getTarget('claude')!.install('global', { autoAllow: true }); + const s = readSettings('global'); + expect(s.hooks.PreToolUse).toEqual({ unexpected: 'user shape' }); + // The other event still gets wired — one odd key doesn't block the install. + expect(commandsFor(s, 'PostCompact').some((c: string) => c.includes('hooks post-compact'))).toBe(true); + }); + + /** + * Fabricate the bundle layout `detectInstallMethod` recognizes — a vendored + * node and a launcher as siblings of `lib/` — and point argv[1] at the JS + * entry inside it, exactly as the real launcher does. + */ + function asBundledBinary(root: string, run: () => T): T { + const launcherName = process.platform === 'win32' ? 'codegraph.cmd' : 'codegraph'; + fs.mkdirSync(path.join(root, 'bin'), { recursive: true }); + fs.mkdirSync(path.join(root, 'lib', 'dist', 'bin'), { recursive: true }); + fs.writeFileSync(path.join(root, process.platform === 'win32' ? 'node.exe' : 'node'), ''); + fs.writeFileSync(path.join(root, 'bin', launcherName), ''); + const prev = process.argv[1]; + process.argv[1] = path.join(root, 'lib', 'dist', 'bin', 'codegraph.js'); + try { return run(); } finally { process.argv[1] = prev; } + } + + it('claude: prefers the absolute launcher over PATH when running from a bundle', () => { + const root = path.join(tmpCwd, 'bundle', 'versions', 'v1.2.3'); + const launcher = path.join(root, 'bin', process.platform === 'win32' ? 'codegraph.cmd' : 'codegraph'); + asBundledBinary(root, () => getTarget('claude')!.install('global', { autoAllow: true })); + const s = readSettings('global'); + expect(commandsFor(s, 'PreToolUse')).toEqual([`${launcher} hooks pre-tool-use --agent claude`]); + expect(commandsFor(s, 'PostCompact')).toEqual([`${launcher} hooks post-compact`]); + }); + + it('claude: quotes a launcher path containing spaces — hook commands are shell strings', () => { + const root = path.join(tmpCwd, 'My Tools', 'versions', 'v1.2.3'); + const launcher = path.join(root, 'bin', process.platform === 'win32' ? 'codegraph.cmd' : 'codegraph'); + asBundledBinary(root, () => writeSessionHookEntries('global')); + expect(commandsFor(readSettings('global'), 'PreToolUse')).toEqual([`"${launcher}" hooks pre-tool-use --agent claude`]); + }); + + it('claude: falls back to the PATH spelling when there is no installed bundle', () => { + // vitest's argv[1] is not a bundle layout — the source/npm/npx case. + getTarget('claude')!.install('global', { autoAllow: true }); + const expected = process.platform === 'win32' ? 'codegraph.cmd' : 'codegraph'; + expect(commandsFor(readSettings('global'), 'PreToolUse')).toEqual([`${expected} hooks pre-tool-use --agent claude`]); + }); + + it('session hooks reach Claude and Codex only — no other target writes them', () => { + // Codex's per-thread connections mean it never needs the stamp for + // ISOLATION, but the post-compact reset still has to name a bucket, so it + // wires hooks of its own (see the codex cases above). Every remaining + // target has no equivalent hook system, and none may quietly grow one. + const others = ALL_TARGETS.filter((t) => t.id !== 'claude' && t.id !== 'codex'); + for (const target of others) target.install('global', { autoAllow: true }); + + // Claude's own settings.json must not have been touched by any of them. + const settings = path.join(tmpHome, '.claude', 'settings.json'); + if (fs.existsSync(settings)) { + const s = JSON.parse(fs.readFileSync(settings, 'utf-8')); + expect(commandsFor(s, 'PreToolUse')).toEqual([]); + expect(commandsFor(s, 'PostCompact')).toEqual([]); + } + + // Nor may any of their OWN config files carry a codegraph hook command. + for (const target of others) { + for (const file of target.describePaths('global')) { + if (!fs.existsSync(file) || !/\.jsonc?$/.test(file)) continue; + const body = fs.readFileSync(file, 'utf-8'); + expect(body, `${target.id} → ${file}`).not.toContain('hooks pre-tool-use'); + expect(body, `${target.id} → ${file}`).not.toContain('hooks post-compact'); + } + } + }); }); describe('Installer targets — registry', () => { diff --git a/__tests__/mcp-tool-allowlist.test.ts b/__tests__/mcp-tool-allowlist.test.ts index 8d342134e..4c1a1c051 100644 --- a/__tests__/mcp-tool-allowlist.test.ts +++ b/__tests__/mcp-tool-allowlist.test.ts @@ -4,7 +4,7 @@ * Filtering happens in ListTools (getTools) and is enforced again on execute(). */ import { describe, it, expect, afterEach } from 'vitest'; -import { ToolHandler } from '../src/mcp/tools'; +import { ToolHandler, getStaticTools } from '../src/mcp/tools'; const ENV = 'CODEGRAPH_MCP_TOOLS'; @@ -61,3 +61,53 @@ describe('CODEGRAPH_MCP_TOOLS allowlist', () => { expect(res.content[0].text).not.toMatch(/disabled via CODEGRAPH_MCP_TOOLS/); }); }); + +/** + * CODEGRAPH_MCP_EXPLORE_SESSION_PARAM — whether explore's `sessionId` is DECLARED. + * + * A host hook's `updatedInput` reaches the server whether or not the property is + * in the schema, so the declaration is off by default: it would buy the hook + * path nothing while exposing a knob an agent could set by hand. It exists for a + * host that validates arguments against the schema, or a harness passing it as + * an explicit tool argument. The HANDLER is unaffected either way — the bucketing + * tests in explore-session-state / explore-cross-call-dedup drive it directly and + * never touch this flag. + */ +describe('CODEGRAPH_MCP_EXPLORE_SESSION_PARAM', () => { + const PARAM = 'CODEGRAPH_MCP_EXPLORE_SESSION_PARAM'; + const original = process.env[PARAM]; + afterEach(() => { + if (original === undefined) delete process.env[PARAM]; + else process.env[PARAM] = original; + }); + + const exploreProps = (defs: { name: string; inputSchema: { properties: Record } }[]) => + defs.find((t) => t.name === 'codegraph_explore')!.inputSchema.properties; + + it('does not declare sessionId by default', () => { + delete process.env[PARAM]; + expect(exploreProps(new ToolHandler(null).getTools())).not.toHaveProperty('sessionId'); + expect(exploreProps(getStaticTools())).not.toHaveProperty('sessionId'); + }); + + it('declares it when the flag is set, on both served surfaces', () => { + process.env[PARAM] = '1'; + for (const props of [exploreProps(new ToolHandler(null).getTools()), exploreProps(getStaticTools())]) { + expect(props.sessionId).toEqual({ + type: 'string', + description: 'Caller-context id, injected by the host\'s hooks — not set manually.', + }); + } + // The rest of the schema is untouched. + expect(exploreProps(getStaticTools())).toHaveProperty('query'); + expect(exploreProps(getStaticTools())).toHaveProperty('maxFiles'); + }); + + it('treats the OFF values and an empty setting as unset', () => { + for (const value of ['0', 'false', 'off', 'no', 'OFF', ' ', '']) { + process.env[PARAM] = value; + expect(exploreProps(getStaticTools()), `value: ${JSON.stringify(value)}`).not.toHaveProperty('sessionId'); + } + }); + +}); diff --git a/src/bin/codegraph.ts b/src/bin/codegraph.ts index 19038df1b..7dd8eb445 100644 --- a/src/bin/codegraph.ts +++ b/src/bin/codegraph.ts @@ -40,6 +40,7 @@ try { import { Command } from 'commander'; import * as path from 'path'; import * as fs from 'fs'; +import * as net from 'net'; import { getCodeGraphDir, isInitialized, unsafeIndexRootReason, findNearestCodeGraphRoot, planFrontload, hasStructuralKeyword, extractCodeTokens } from '../directory'; import { extractProseCandidates } from '../search/identifier-segments'; import { detectWorktreeIndexMismatch, worktreeMismatchWarning } from '../sync/worktree'; @@ -1473,6 +1474,230 @@ program } }); +/** + * codegraph hooks … (hidden) + * + * Host-agent hook entry points. A host (Claude Code today) invokes these with + * its hook payload on stdin; their whole job is to tell the server WHICH agent + * context a call belongs to, because one MCP connection carries several — a + * subagent dispatches over its parent's — and cross-call dedup must not tell a + * fresh context that source was "already sent" to a sibling. + * + * LOAD-BEARING, same contract as `prompt-hook`: a non-zero exit or a stray byte + * on stdout disrupts the host. Every failure path here exits 0 and prints + * nothing; stdout carries the hook protocol and nothing else, notes go to + * stderr. + */ + +/** The hook payload on stdin, or `{}` when there isn't one. Never rejects. */ +async function readHookPayload(): Promise> { + if (process.stdin.isTTY) return {}; // invoked by hand — nothing is coming + const raw = await new Promise((resolve) => { + let data = ''; + process.stdin.setEncoding('utf8'); + process.stdin.on('data', (c) => { data += c; }); + process.stdin.on('end', () => resolve(data)); + process.stdin.on('error', () => resolve(data)); + }); + try { + const parsed: unknown = JSON.parse(raw); + return parsed && typeof parsed === 'object' ? parsed as Record : {}; + } catch { + return {}; + } +} + +/** + * The caller-context id for a hook payload: the host session, plus the agent id + * when a subagent is calling. + * + * Composite rather than the agent id alone, because a bare `agent_id` assumes + * the host numbers its agents uniquely across ALL sessions — a host whose ids + * are only unique within a session would put two sessions' subagents in one + * bucket on a shared connection, which is the bug this whole mechanism exists + * to prevent. Scoping every agent id to its session removes the assumption, and + * a main context is never confused with a subagent of the same session: one is + * the session id alone, the other always carries the separator and a suffix. + * + * Both hooks derive through here — the injecting hook and the resetting hook + * MUST agree on the spelling or a reset clears a bucket nobody is filling. + */ +function hookSessionId(payload: Record): string | null { + const agentId = typeof payload.agent_id === 'string' ? payload.agent_id.trim() : ''; + const sessionId = typeof payload.session_id === 'string' ? payload.session_id.trim() : ''; + if (sessionId && agentId) return `${sessionId}:${agentId}`; + // A payload carrying only one of the two still identifies a context; only a + // payload with neither is unusable. + return sessionId || agentId || null; +} + +/** Cap on what a control reply may stream before we stop reading it. */ +const MAX_CONTROL_REPLY_BYTES = 64 * 1024; + +/** + * One-shot control round-trip against a daemon socket. The daemon greets every + * connection with its own hello, so the reply is the first line carrying an + * `ok` field. Resolves `null` on ANY failure — no listener, wrong socket, + * timeout, malformed reply — because the only callers are hooks that must + * degrade silently. + */ +function sendDaemonControl( + socketPath: string, + command: Record, + timeoutMs = 2_000, +): Promise<{ ok?: boolean; cleared?: number } | null> { + return new Promise((resolve) => { + let settled = false; + const done = (value: { ok?: boolean; cleared?: number } | null): void => { + if (settled) return; + settled = true; + clearTimeout(timer); + try { socket.destroy(); } catch { /* already gone */ } + resolve(value); + }; + const socket = net.createConnection(socketPath); + const timer = setTimeout(() => done(null), timeoutMs); + timer.unref?.(); + let buffered = ''; + socket.setEncoding('utf8'); + socket.on('connect', () => { socket.write(`${JSON.stringify(command)}\n`); }); + socket.on('data', (chunk: string) => { + buffered += chunk; + if (buffered.length > MAX_CONTROL_REPLY_BYTES) { done(null); return; } + let nl: number; + while ((nl = buffered.indexOf('\n')) !== -1) { + const line = buffered.slice(0, nl); + buffered = buffered.slice(nl + 1); + try { + const parsed: unknown = JSON.parse(line); + if (parsed && typeof parsed === 'object' && 'ok' in parsed) { + done(parsed as { ok?: boolean; cleared?: number }); + return; + } + } catch { /* the daemon hello, or noise before the reply */ } + } + }); + socket.on('error', () => done(null)); + socket.on('close', () => done(null)); + }); +} + +const hooks = program + .command('hooks', { hidden: true }) + .description('Host-agent hook entry points (payload on stdin)'); + +/** + * codegraph hooks post-compact (hidden) + * + * PostCompact hook: the context this caller had is gone, so the server's record + * of what it was already sent has to go with it — otherwise the next call is + * answered with pointers to source the compacted context no longer holds, and + * the agent Reads the file. + * + * Both Claude Code and codex wire this to their PostCompact event, which fires + * only once the compacted history has been committed. That is the one moment a + * reset is correct: a compaction that failed returns before rewriting anything, + * leaving the agent still holding the source the record describes, and a reset + * there would throw away a ledger that is still valid. Only `session_id` / + * `agent_id` and the payload cwd are read, so either host's payload shape works + * unchanged; `hook_event_name` and the rest are ignored. + */ +hooks + .command('post-compact') + .description('Drop this caller context\'s already-sent record after a compact') + .option('--path ', 'Project whose daemon to signal (default: the payload cwd)') + .option('--session-id ', 'Caller context id, when there is no hook payload to derive it from') + .action(async (options: { path?: string; sessionId?: string }) => { + try { + // An explicit --session-id means there is no hook payload to wait for — + // a relay caller's stdin may be a pipe that never closes. + const payload = options.sessionId?.trim() ? {} : await readHookPayload(); + const sessionId = options.sessionId?.trim() || hookSessionId(payload); + if (!sessionId) return; + + const cwd = options.path || (typeof payload.cwd === 'string' ? payload.cwd : '') || process.cwd(); + const found = findNearestCodeGraphRoot(cwd); + if (!found) return; // nothing indexed here — no daemon can be holding a record + // Realpath'd to match how the daemon keys its socket and lockfile: a + // symlinked cwd would otherwise resolve to a socket nobody is serving. + let root = found; + try { root = fs.realpathSync(found); } catch { /* keep the un-resolved path */ } + + const { getDaemonPidPath, getDaemonSocketCandidates, decodeLockInfo } = await import('../mcp/daemon-paths'); + // The lockfile is authoritative — a daemon that relocated past an + // unusable in-project filesystem is bound somewhere the candidate list + // only guesses at — but it may be stale or missing, so the candidates + // still follow it. + const fromLock = (() => { + try { return decodeLockInfo(fs.readFileSync(getDaemonPidPath(root), 'utf8'))?.socketPath ?? null; } + catch { return null; } + })(); + const candidates = [...new Set([fromLock, ...getDaemonSocketCandidates(root)].filter((c): c is string => !!c))]; + + for (const candidate of candidates) { + const reply = await sendDaemonControl(candidate, { + codegraph_control: 1, + op: 'clear-session-record', + sessionId, + }); + if (reply?.ok) return; + } + // No daemon answered. Nothing to reset — a fresh one starts with no record. + } catch { + // Degradable by contract: never surface an error to the host's hook pipeline. + } + }); + +/** + * codegraph hooks pre-tool-use (hidden) + * + * PreToolUse hook: stamps the calling context's id onto a codegraph_explore + * call so the server buckets its already-sent record per context instead of per + * connection. Only explore is touched; every other tool passes through + * untouched (no stdout at all, which the host reads as "no change"). + */ +hooks + .command('pre-tool-use') + .description('Stamp the calling context id onto a codegraph_explore call') + .option('--agent ', 'Agent whose hook protocol to emit for (an installer target name, e.g. codex)') + .action(async (options: { agent?: string }) => { + try { + const payload = await readHookPayload(); + const toolName = typeof payload.tool_name === 'string' ? payload.tool_name : ''; + if (!toolName.includes('codegraph_explore')) return; + const sessionId = hookSessionId(payload); + if (!sessionId) return; + const toolInput = payload.tool_input && typeof payload.tool_input === 'object' + ? payload.tool_input as Record + : {}; + // `permissionDecision` is emitted only for an agent that REQUIRES it. + // Codex treats it and `updatedInput` as mutually required and silently + // discards an unpaired rewrite, and its decision is inert for approvals + // — it gates the rewrite inside the hooks crate and never reaches core + // dispatch. Claude Code needs no pairing and ACTS on the field: "allow" + // auto-approves, overriding a user who deliberately did not allowlist + // codegraph (and explore's `projectPath` reaches any indexed project), + // while "ask" would force a prompt on every call even for users who did. + // Emitting nothing leaves Claude's own permission flow in charge, which + // is the correct output there. + // + // The agent is declared by the wiring that installed the hook, never + // sniffed from the payload: a guess that lands on "allow" is a privilege + // escalation, so any other --agent omits the field. + const pairsPermissionDecision = options.agent === 'codex'; + process.stdout.write(`${JSON.stringify({ + hookSpecificOutput: { + hookEventName: 'PreToolUse', + ...(pairsPermissionDecision ? { permissionDecision: 'allow' } : {}), + updatedInput: { ...toolInput, sessionId }, + }, + })}\n`); + } catch { + // Degradable by contract: an un-stamped call still works, it just dedups + // against the connection-wide bucket. + } + }); + /** * codegraph node [name] * diff --git a/src/installer/targets/claude.ts b/src/installer/targets/claude.ts index e95b0a35d..966221f89 100644 --- a/src/installer/targets/claude.ts +++ b/src/installer/targets/claude.ts @@ -28,13 +28,18 @@ import { WriteResult, } from './types'; import { + codegraphBinary, getCodeGraphPermissions, getMcpServerConfig, + isSessionHookCommand, jsonDeepEqual, + mergeHookEntries, + pruneHookCommands, readJsonFile, removeMarkedSection, writeJsonFile, upsertInstructionsEntry, + type HookEntry, } from './shared'; import { CODEGRAPH_SECTION_END, @@ -133,6 +138,14 @@ class ClaudeCodeTarget implements AgentTarget { if (removed.action === 'removed') files.push(removed); } + // 2d. Per-context session hooks. Unconditional: they are what tells the + // server WHICH agent context a call belongs to, and without them the + // cross-call dedup record stays keyed per MCP connection — so a subagent, + // which dispatches over its parent's, is told source was "already sent" to + // a context that never received it. Both hooks are inert to a user who + // never triggers them. + files.push(writeSessionHookEntries(loc)); + // 3. CLAUDE.md instructions — the short marker-fenced CodeGraph // block (#704). The MCP initialize instructions reach only the main // agent; CLAUDE.md is what Task-tool subagents (and non-MCP @@ -203,6 +216,10 @@ class ClaudeCodeTarget implements AgentTarget { const promptHookCleanup = removePromptHookEntry(loc); if (promptHookCleanup.action === 'removed') files.push(promptHookCleanup); + // 2d. Remove the per-context session hooks. + const sessionHookCleanup = removeSessionHookEntries(loc); + if (sessionHookCleanup.action === 'removed') files.push(sessionHookCleanup); + // 3. Instructions — strip the legacy CodeGraph block if present. files.push(removeInstructionsEntry(loc)); @@ -344,33 +361,7 @@ function removeHookCommandsMatching( return { path: file, action: 'unchanged' }; } - // Pass 1: drop matching command(s) from inside every matcher group. - let removedAny = false; - for (const event of Object.keys(hooks)) { - const groups = hooks[event]; - if (!Array.isArray(groups)) continue; - for (const group of groups) { - if (!group || !Array.isArray(group.hooks)) continue; - const before = group.hooks.length; - group.hooks = group.hooks.filter((h: any) => !match(h?.command)); - if (group.hooks.length !== before) removedAny = true; - } - } - - if (!removedAny) return { path: file, action: 'unchanged' }; - - // Pass 2: prune empty matcher groups, then events with no groups left, - // then an empty top-level `hooks`. Guarded by `removedAny` so we never - // restructure a settings.json that had no matching hooks. Sibling hooks - // (a different command in the group, or a different event) survive. - for (const event of Object.keys(hooks)) { - const groups = hooks[event]; - if (!Array.isArray(groups)) continue; - hooks[event] = groups.filter( - (g: any) => !(g && Array.isArray(g.hooks) && g.hooks.length === 0), - ); - if (hooks[event].length === 0) delete hooks[event]; - } + if (!pruneHookCommands(hooks, match)) return { path: file, action: 'unchanged' }; if (Object.keys(hooks).length === 0) delete settings.hooks; writeJsonFile(file, settings); @@ -395,6 +386,67 @@ export function removePromptHookEntry(loc: Location): WriteResult['files'][numbe return removeHookCommandsMatching(loc, isPromptHookCommand); } +/** + * The per-context session hooks, as Claude Code spells them. + * + * `PreToolUse` stamps the calling agent's id onto a `codegraph_explore` call so + * the server's already-sent record is bucketed per agent context rather than + * per MCP connection — without it a subagent, which dispatches over its + * parent's connection, is told source was "already sent" to a context that + * never received it. The matcher is a regex over the tool name, and the MCP + * server name is the user's to rename, so it matches the tool SUFFIX rather + * than a fixed `mcp__codegraph__` prefix. The command names its agent so the + * hook emits Claude's protocol and not another's — here that means NO + * `permissionDecision`, since Claude acts on the field and would auto-approve + * a call the user's own permission flow should decide. + * + * `PostCompact` fires once compaction has completed and before the resumed + * session's first turn, which is the only moment a reset is correct — + * `PreCompact` runs while the agent still holds the source. It covers manual + * `/compact` and auto-compact alike, and takes no matcher: events without + * matcher support omit the key rather than carrying an empty one. + */ +const SESSION_HOOKS: HookEntry[] = [ + { event: 'PreToolUse', matcher: 'mcp__.*__codegraph_explore', subcommand: 'hooks pre-tool-use --agent claude' }, + { event: 'PostCompact', subcommand: 'hooks post-compact' }, +]; + +/** + * Wire the per-context session hooks into Claude `settings.json`. + * + * Surgical: each entry is appended as its own matcher group, so the user's own + * hooks — including other groups under the same event — are untouched. An + * entry we already wrote is rewritten in place when the binary spelling has + * drifted (a moved bundle, a settings.json synced from another platform) and + * otherwise left alone, so a re-run on an unchanged file reports `unchanged` + * and the bytes never move. + */ +export function writeSessionHookEntries(loc: Location): WriteResult['files'][number] { + const file = settingsJsonPath(loc); + const created = !fs.existsSync(file); + const settings = readJsonFile(file); + + if (!settings.hooks || typeof settings.hooks !== 'object' || Array.isArray(settings.hooks)) { + settings.hooks = {}; + } + + const changed = mergeHookEntries(settings.hooks, SESSION_HOOKS, codegraphBinary()); + + if (!changed && !created) return { path: file, action: 'unchanged' }; + writeJsonFile(file, settings); + return { path: file, action: created ? 'created' : 'updated' }; +} + +/** + * Remove the per-context session hooks this installer writes. Reuses the + * command-level surgery `removeHookCommandsMatching` already does: a sibling + * hook sharing our event survives, and the structures we created are pruned + * only once they are empty. + */ +export function removeSessionHookEntries(loc: Location): WriteResult['files'][number] { + return removeHookCommandsMatching(loc, isSessionHookCommand); +} + export function writePermissionsEntry(loc: Location): WriteResult['files'][number] { const file = settingsJsonPath(loc); const settings = readJsonFile(file); diff --git a/src/installer/targets/codex.ts b/src/installer/targets/codex.ts index d5e361ce3..e29e27770 100644 --- a/src/installer/targets/codex.ts +++ b/src/installer/targets/codex.ts @@ -40,15 +40,22 @@ import { } from './types'; import { atomicWriteFileSync, + codegraphBinary, getMcpServerConfig, + isSessionHookCommand, + mergeHookEntries, + pruneHookCommands, removeMarkedSection, upsertInstructionsEntry, + writeJsonFile, + type HookEntry, } from './shared'; import { CODEGRAPH_SECTION_END, CODEGRAPH_SECTION_START, } from '../instructions-template'; -import { buildTomlTable, removeTomlTable, upsertTomlTable } from './toml'; +import { buildTomlTable, readTomlTableBody, removeTomlTable, upsertTomlTable } from './toml'; +import { createHash } from 'crypto'; const TOML_HEADER = 'mcp_servers.codegraph'; @@ -70,6 +77,16 @@ function instructionsPath(loc: Location): string { : path.join(process.cwd(), 'AGENTS.md'); } +/** + * Codex discovers hooks from a `hooks.json` in each config layer's `.codex/` + * folder — `~/.codex/hooks.json` for the user layer, `/.codex/hooks.json` + * for the project layer (`hooks_config_folder` in codex's `config/src/state.rs`) + * — which is the same global/local split the TOML config already uses. + */ +function hooksJsonPath(loc: Location): string { + return path.join(configDir(loc), 'hooks.json'); +} + /** * Project layers are "loaded but disabled when untrusted" (openai/codex * `loader/mod.rs`), so a local install can be written correctly and @@ -79,6 +96,272 @@ function trustNote(): string { return `Codex applies ${tomlConfigPath('local')} only in a project marked trusted — otherwise the layer is loaded but disabled. Trust this project in Codex to activate it.`; } +/** + * The per-context session hooks, as codex spells them. + * + * `PreToolUse`'s matcher is a regex tested against the tool name, so the bare + * name matches however the server ends up namespaced. It declares `--agent + * codex` because codex requires `permissionDecision` paired with the rewrite + * and drops an unpaired one — the hook emits that field for no other agent, + * and learns which one it serves from this flag rather than from the payload. + * + * `PostCompact` — codex gates it behind compaction SUCCESS in three of its + * four compaction paths, and in all four every failure path returns before + * the history rewrite commits, so a failed compaction leaves the agent's + * context intact. Skipping the reset there is exactly right; PreCompact would + * clear a still-valid record on every failed attempt. It carries no matcher, + * which codex reads as every trigger — manual and auto alike. + */ +const SESSION_HOOKS: HookEntry[] = [ + { event: 'PreToolUse', matcher: 'codegraph_explore', subcommand: 'hooks pre-tool-use --agent codex' }, + { event: 'PostCompact', subcommand: 'hooks post-compact' }, +]; + +/** + * Parse a `hooks.json`, or `null` when it is unusable. + * + * Deliberately NOT `readJsonFile`: that one backs a broken file up and returns + * `{}`, which here would mean writing our hooks over config we failed to read. + * Codex would then run our file instead of the user's, so an unparseable + * hooks.json is left exactly as it is. + */ +function readHooksJson(file: string): Record | null { + if (!fs.existsSync(file)) return {}; + try { + const parsed = JSON.parse(fs.readFileSync(file, 'utf-8')); + return parsed && typeof parsed === 'object' && !Array.isArray(parsed) ? parsed : null; + } catch { + return null; + } +} + +/** + * Wire the session hooks into this location's `hooks.json`, creating the file + * when it doesn't exist. Surgical: existing events, groups and unrelated + * top-level keys are preserved, ours is appended, and a re-run that finds + * everything already in place leaves the bytes untouched. + * + * Codex holds hook trust per ENTRY, in `[hooks.state]` inside config.toml: each + * handler carries a hash of its own normalized identity. Writing that state + * ourselves would stamp our hooks as reviewed and skip the review the user is + * entitled to, so the installer never touches it — and adding entries leaves + * every hook the user already trusted trusted. Nothing here announces that: + * codex's own TUI surfaces untrusted hooks on its next run, so the install + * reports this file the way it reports every other one, as a file action. + */ +export function writeSessionHookEntries(loc: Location): WriteResult['files'][number] { + const file = hooksJsonPath(loc); + const created = !fs.existsSync(file); + const root = readHooksJson(file); + if (!root) { + console.warn(` Warning: ${file} is not valid JSON — leaving it untouched.`); + console.warn(' Fix the file and re-run "codegraph install" to wire the CodeGraph hooks.'); + return { path: file, action: 'unchanged' }; + } + + if (!root.hooks || typeof root.hooks !== 'object' || Array.isArray(root.hooks)) { + root.hooks = {}; + } + const changed = mergeHookEntries(root.hooks, SESSION_HOOKS, codegraphBinary()); + if (!changed && !created) return { path: file, action: 'unchanged' }; + + writeJsonFile(file, root); + return { path: file, action: created ? 'created' : 'updated' }; +} + +/** + * Codex's snake_case label for each event we wire. It spells the event both in + * a hook's trust identity and in its `[hooks.state]` key. + */ +const HOOK_EVENT_KEY_LABEL: Record = { + PreToolUse: 'pre_tool_use', + PostToolUse: 'post_tool_use', + PostCompact: 'post_compact', +}; + +/** What codex normalizes an unset command-hook `timeout` to, in seconds. */ +const CODEX_DEFAULT_HOOK_TIMEOUT_SEC = 600; + +/** Recursively key-sorted copy — codex canonicalizes before hashing. */ +function canonicalJson(value: unknown): unknown { + if (Array.isArray(value)) return value.map(canonicalJson); + if (value && typeof value === 'object') { + const source = value as Record; + return Object.fromEntries(Object.keys(source).sort().map((k) => [k, canonicalJson(source[k])])); + } + return value; +} + +/** + * Reproduce the trust hash codex computes for one handler. + * + * Codex hashes a NORMALIZED identity rather than the source text, so that the + * same hook expressed in config.toml and in hooks.json converges: the event's + * snake_case label, the group's matcher, and the one handler with its defaults + * filled in — an unset `timeout` becoming {@link CODEX_DEFAULT_HOOK_TIMEOUT_SEC} + * and `async` defaulting to false. That is serialized to TOML (which drops unset + * optionals), converted to JSON, key-sorted at every level, and SHA-256'd. + * + * Pinned against real trusted entries in the fixture test — this reimplements + * another project's internal, so the test is what says it still agrees. If codex + * changes its normalization the hash simply stops matching, and codex reports + * the entry as needing review and asks in its own TUI, exactly as it does for a + * hook nobody has trusted. A stale hash can never read as trusted. + */ +export function codexHookTrustHash( + event: string, + matcher: string | undefined, + command: string, + statusMessage?: string, +): string { + const handler: Record = { + type: 'command', + command, + async: false, + timeout: CODEX_DEFAULT_HOOK_TIMEOUT_SEC, + }; + // Unset optionals are absent, not empty — see the matcher note below. Ours + // never carry a statusMessage; the parameter exists so the fixture test can + // hash a real-world handler that does. + if (statusMessage !== undefined) handler.statusMessage = statusMessage; + const identity: Record = { + event_name: HOOK_EVENT_KEY_LABEL[event] ?? event, + hooks: [handler], + }; + // An absent matcher is absent from the identity, not an empty string: TOML + // has no null, so codex's own serialization drops the key entirely. + if (matcher !== undefined) identity.matcher = matcher; + const canonical = JSON.stringify(canonicalJson(identity)); + return `sha256:${createHash('sha256').update(canonical).digest('hex')}`; +} + +/** + * Locate our handlers in a written hooks.json and describe each the way codex + * will: its `[hooks.state]` key and its current trust hash. + * + * The key carries the handler's POSITION (`:::`), + * so the indices are read back off the file we just wrote rather than assumed — + * ours may sit after any number of the user's groups. + */ +function ourHookTrustState(file: string, root: Record): Array<{ key: string; hash: string }> { + const found: Array<{ key: string; hash: string }> = []; + for (const { event, subcommand } of SESSION_HOOKS) { + const groups: any[] = Array.isArray(root.hooks?.[event]) ? root.hooks[event] : []; + const stable = subcommand.split(' --')[0] ?? subcommand; + groups.forEach((group, groupIndex) => { + const handlers: any[] = group && Array.isArray(group.hooks) ? group.hooks : []; + handlers.forEach((handler, handlerIndex) => { + const command = handler?.command; + if (typeof command !== 'string') return; + if (!isSessionHookCommand(command) || !command.includes(stable)) return; + found.push({ + key: `${file}:${HOOK_EVENT_KEY_LABEL[event] ?? event}:${groupIndex}:${handlerIndex}`, + // Hash the matcher AS WRITTEN, which is what codex will read back. + hash: codexHookTrustHash(event, typeof group.matcher === 'string' ? group.matcher : undefined, command), + }); + }); + }); + } + return found; +} + +/** + * Record our two hooks as trusted in this layer's config.toml. + * + * This is the one place the installer writes `[hooks.state]`, and it is not + * gated on a question. Codex's TUI review guards hooks that ARRIVE from + * somewhere the user didn't choose — a cloned repo's project hooks.json, say — + * and an installer the user just invoked, writing its own two entries pointing + * at its own binary, is not that threat; the Claude target installs its hooks + * without asking for the same reason. A local install's state also still sits + * behind codex's project-trust gate, so nothing runs in a project the user + * hasn't trusted anyway. + * + * Trust is held PER ENTRY, so what is written can only ever be the entries we + * wrote ourselves, keyed by their own position — the user's hooks are + * unreachable from here, and their trust records are never read, rewritten, or + * invalidated by ours. + * + * The launcher path is part of the hashed identity and changes with every + * version, so an existing record is UPDATED rather than skipped; only its + * `trusted_hash` line is replaced, keeping any `enabled` the user set in the + * codex TUI (re-enabling a hook they turned off is not ours to do). + * + * ponytail: a record is keyed by POSITION, so if our group's index ever shifts + * — a user inserting a group ahead of ours in the same file — the record for + * the old position is left behind. Harmless (its hash matches nothing at the + * new position, so codex asks rather than trusting) but untidy; prune it here + * if that ever stops being rare, taking care not to delete a record the user + * made for a hook of their own in the same file. + */ +function writeHookTrustState(loc: Location, entries: Array<{ key: string; hash: string }>): boolean { + const file = tomlConfigPath(loc); + let content = fs.existsSync(file) ? fs.readFileSync(file, 'utf-8') : ''; + let changed = false; + + for (const { key, hash } of entries) { + const header = `hooks.state.${JSON.stringify(key)}`; + const existing = readTomlTableBody(content, header) ?? []; + const body = [ + `trusted_hash = ${JSON.stringify(hash)}`, + ...existing.filter((line) => !/^\s*trusted_hash\s*=/.test(line)), + ].join('\n'); + const { content: next, action } = upsertTomlTable(content, header, `[${header}]\n${body}`); + if (action !== 'unchanged') changed = true; + content = next; + } + + if (changed) atomicWriteFileSync(file, content); + return changed; +} + +/** Drop the trust records for our own hooks, and nothing else. */ +function removeHookTrustState(loc: Location, entries: Array<{ key: string; hash: string }>): boolean { + const file = tomlConfigPath(loc); + if (!fs.existsSync(file)) return false; + let content = fs.readFileSync(file, 'utf-8'); + let changed = false; + for (const { key } of entries) { + const { content: next, action } = removeTomlTable(content, `hooks.state.${JSON.stringify(key)}`); + if (action === 'removed') changed = true; + content = next; + } + if (changed) atomicWriteFileSync(file, content.trimEnd() + '\n'); + return changed; +} + +/** + * Remove the session hooks this installer wrote. Leaves the user's own hooks, + * and any other top-level key, alone; deletes the file only when removing ours + * empties it completely — i.e. only when we were the ones who created it. + */ +export function removeSessionHookEntries(loc: Location): WriteResult['files'][number] { + const file = hooksJsonPath(loc); + if (!fs.existsSync(file)) return { path: file, action: 'not-found' }; + const root = readHooksJson(file); + if (!root || !root.hooks || typeof root.hooks !== 'object' || Array.isArray(root.hooks)) { + return { path: file, action: 'unchanged' }; + } + if (!pruneHookCommands(root.hooks, isSessionHookCommand)) { + return { path: file, action: 'unchanged' }; + } + if (Object.keys(root.hooks).length === 0) delete root.hooks; + + if (Object.keys(root).length === 0) { + try { fs.unlinkSync(file); } catch { /* ignore */ } + } else { + writeJsonFile(file, root); + } + return { path: file, action: 'removed' }; +} + +/** Our hooks as they now sit on disk, keyed and hashed the way codex will. */ +function hookTrustEntries(loc: Location): Array<{ key: string; hash: string }> { + const file = hooksJsonPath(loc); + const root = readHooksJson(file); + return root ? ourHookTrustState(file, root) : []; +} + class CodexTarget implements AgentTarget { readonly id = 'codex' as const; readonly displayName = 'Codex CLI'; @@ -107,7 +390,19 @@ class CodexTarget implements AgentTarget { install(loc: Location, _opts: InstallOptions): WriteResult { const files: WriteResult['files'] = []; - files.push(writeMcpEntry(loc)); + const mcp = writeMcpEntry(loc); + + // Per-context session hooks. They are what tells the server WHICH agent + // context a call belongs to; without them its already-sent record stays + // keyed per MCP connection. Their trust records go in unconditionally — + // see writeHookTrustState — and the write no-ops once they are right. + const hooks = writeSessionHookEntries(loc); + const trustChanged = writeHookTrustState(loc, hookTrustEntries(loc)); + + // The MCP entry and the trust records share config.toml, so it is reported + // once, as whichever write actually touched it. + files.push(trustChanged && mcp.action === 'unchanged' ? { path: mcp.path, action: 'updated' } : mcp); + files.push(hooks); // AGENTS.md gets the short marker-fenced CodeGraph block (#704): // subagents and non-MCP harnesses read AGENTS.md but never the MCP @@ -120,6 +415,10 @@ class CodexTarget implements AgentTarget { uninstall(loc: Location): WriteResult { const files: WriteResult['files'] = []; + // Read the keys off hooks.json BEFORE it is stripped — they are derived + // from the handlers' positions in it. + removeHookTrustState(loc, hookTrustEntries(loc)); + const tomlPath = tomlConfigPath(loc); if (fs.existsSync(tomlPath)) { const content = fs.readFileSync(tomlPath, 'utf-8'); @@ -138,6 +437,9 @@ class CodexTarget implements AgentTarget { files.push({ path: tomlPath, action: 'not-found' }); } + const hooks = removeSessionHookEntries(loc); + if (hooks.action === 'removed') files.push(hooks); + files.push(removeInstructionsEntry(loc)); return { files }; @@ -149,7 +451,7 @@ class CodexTarget implements AgentTarget { } describePaths(loc: Location): string[] { - return [tomlConfigPath(loc), instructionsPath(loc)]; + return [tomlConfigPath(loc), hooksJsonPath(loc), instructionsPath(loc)]; } } diff --git a/src/installer/targets/shared.ts b/src/installer/targets/shared.ts index 364f40427..95ccccc4a 100644 --- a/src/installer/targets/shared.ts +++ b/src/installer/targets/shared.ts @@ -15,6 +15,7 @@ import { CODEGRAPH_SECTION_START, CODEGRAPH_SECTION_END, } from '../instructions-template'; +import { detectInstallMethod } from '../../upgrade'; /** * The MCP-server config block codegraph injects. Same shape across @@ -48,6 +49,150 @@ export function getCodeGraphPermissions(): string[] { return ['mcp__codegraph__*']; } +/** + * The `codegraph` spelling to write into a hook command. + * + * Prefer the ABSOLUTE launcher — same discipline as Cursor's `--path`: resolve + * at install time what the agent would otherwise have to find at run time. A + * hook runs under whatever environment the host hands it, and Claude Code + * executes hooks through Git Bash on Windows, so a PATH lookup is the fragile + * half of the contract (#1466). + * + * Only a bundle install HAS an absolute launcher to point at; npm/npx put a + * shim on PATH and a source checkout has no installed binary at all, so those + * keep the PATH spelling. A path that has since moved is not a trap: hooks are + * recognized by their `hooks ` substring, so a re-install rewrites + * a stale one in place (see {@link mergeHookEntries}). + */ +export function codegraphBinary(): string { + const fallback = process.platform === 'win32' ? 'codegraph.cmd' : 'codegraph'; + try { + const method = detectInstallMethod({ + filename: process.argv[1] ?? '', + platform: process.platform, + cwd: process.cwd(), + }); + if (method.kind !== 'bundle' || !method.bundleRoot) return fallback; + const launcher = path.join(method.bundleRoot, 'bin', fallback); + if (!fs.existsSync(launcher)) return fallback; + // Hook commands are shell strings, so a launcher under "C:\Users\A B\…" + // has to survive word-splitting. + return /\s/.test(launcher) ? `"${launcher}"` : launcher; + } catch { + return fallback; + } +} + +/** One hook to wire: which event, an optional matcher, and our subcommand. */ +export interface HookEntry { + event: string; + matcher?: string; + subcommand: string; +} + +/** + * The `codegraph hooks` subcommands the installer wires into a host. Shared by + * every target: recognition has to be spelling-independent, so it keys on the + * subcommand rather than on the binary path or the host's event names. + */ +const SESSION_HOOK_SUBCOMMANDS = ['hooks pre-tool-use', 'hooks post-compact']; + +/** + * Recognizes a session hook this installer wrote, whatever binary spelling it + * was written with. The `codegraph`-scoped subcommand is the stable part — + * matching on it keeps an absolute path, a `.cmd`, or an `npx …` form all + * recognizable, and cannot collide with an unrelated user hook. + */ +export function isSessionHookCommand(command: unknown): boolean { + if (typeof command !== 'string' || !command.includes('codegraph')) return false; + return SESSION_HOOK_SUBCOMMANDS.some((s) => command.includes(s)); +} + +/** + * Merge hook entries into a host's event→matcher-groups map, in place. Returns + * whether anything actually changed, so the caller can leave a byte-identical + * file alone. + * + * Ours is always APPENDED as its own group rather than folded into a user's: + * hosts identify a hook by its position (codex keys its trust record on the + * group and handler index), so inserting ahead of existing groups renumbers + * them. An entry we already wrote is re-pointed at the binary we resolve now + * instead of being duplicated. + * + * Structure-only: every target reads and writes its own file, because their + * policies for a malformed one differ. + */ +export function mergeHookEntries( + hooks: Record, + entries: HookEntry[], + binary: string, +): boolean { + let changed = false; + for (const { event, matcher, subcommand } of entries) { + // A non-array under an event is not a shape any host reads, but it is the + // user's — overwriting it would destroy config we don't understand. + if (event in hooks && !Array.isArray(hooks[event])) continue; + const command = `${binary} ${subcommand}`; + // Recognize on the subcommand WITHOUT its flags: an entry whose flags have + // since changed is still the same entry, and must be re-pointed rather than + // joined by a second copy of itself. + const stable = subcommand.split(' --')[0]; + const groups: any[] = hooks[event] ?? []; + const ours = groups + .flatMap((g: any) => (g && Array.isArray(g.hooks) ? g.hooks : [])) + .filter((h: any) => isSessionHookCommand(h?.command) && h.command.includes(stable)); + if (ours.length > 0) { + for (const h of ours) { + if (h.command !== command) { h.command = command; changed = true; } + } + continue; + } + groups.push({ ...(matcher ? { matcher } : {}), hooks: [{ type: 'command', command }] }); + hooks[event] = groups; + changed = true; + } + return changed; +} + +/** + * Drop every hook command matching `match` from a host's event→groups map, in + * place, then prune what that emptied. Returns whether anything was removed. + * + * Surgical at the individual-command level: a sibling hook sharing a group (or + * an event) with ours survives. A group is pruned only once its `hooks` array + * is empty and an event only once it has no groups left — and none of that runs + * unless a command was actually removed, so a file with none of ours is left + * byte-for-byte untouched. The caller owns the now-possibly-empty `hooks` key + * itself, since where it hangs differs by host. + */ +export function pruneHookCommands( + hooks: Record, + match: (command: unknown) => boolean, +): boolean { + let removedAny = false; + for (const event of Object.keys(hooks)) { + const groups = hooks[event]; + if (!Array.isArray(groups)) continue; + for (const group of groups) { + if (!group || !Array.isArray(group.hooks)) continue; + const before = group.hooks.length; + group.hooks = group.hooks.filter((h: any) => !match(h?.command)); + if (group.hooks.length !== before) removedAny = true; + } + } + if (!removedAny) return false; + + for (const event of Object.keys(hooks)) { + const groups = hooks[event]; + if (!Array.isArray(groups)) continue; + hooks[event] = groups.filter( + (g: any) => !(g && Array.isArray(g.hooks) && g.hooks.length === 0), + ); + if (hooks[event].length === 0) delete hooks[event]; + } + return true; +} + /** * Read a JSON file, returning `{}` when missing or unparseable. * diff --git a/src/installer/targets/toml.ts b/src/installer/targets/toml.ts index 1dc086bf3..eb6245d02 100644 --- a/src/installer/targets/toml.ts +++ b/src/installer/targets/toml.ts @@ -102,6 +102,26 @@ export function upsertTomlTable( }; } +/** + * The body lines of an existing dotted-key table, or `null` when the table + * isn't there. Uses the same lexer-backed block scan as upsert/remove, so a + * bracket inside a string value can't be mistaken for the next table. + * + * Lets a caller rewrite ONE key of a table it does not own outright, keeping + * every sibling key the user or another tool put there. + */ +export function readTomlTableBody(fileContent: string, header: string): string[] | null { + const headerLine = `[${header}]`; + const headerIdx = findHeaderIndex(fileContent, headerLine); + if (headerIdx === -1) return null; + const blockEnd = findNextTableHeader(fileContent, headerIdx + headerLine.length); + return fileContent + .substring(headerIdx + headerLine.length, blockEnd) + .split('\n') + .map((line) => line.trimEnd()) + .filter((line) => line.length > 0); +} + /** * Remove a top-level dotted-key TOML table block. Returns the * possibly-empty new content + an action flag. diff --git a/src/mcp/daemon.ts b/src/mcp/daemon.ts index 500c48a8c..250be427c 100644 --- a/src/mcp/daemon.ts +++ b/src/mcp/daemon.ts @@ -367,6 +367,13 @@ export class Daemon { // timeout, a non-hello first line, an early close — yields null pids and we // fall back to the socket-close lifecycle exactly as before (#692). void readClientHello(socket).then((peers) => { + // A control command claims the connection: one line in, one line out, no + // session. It is deliberately never added to `clients`, so a control + // caller neither counts as a client nor holds the idle timer off. + if (peers.control) { + this.handleControl(socket, peers.control); + return; + } const transport = new SocketTransport(socket); const session = new MCPSession(transport, this.engine, { explicitProjectPath: this.projectRoot, @@ -383,6 +390,31 @@ export class Daemon { }); } + /** + * Answer a one-shot control connection. + * + * Total by design: an unknown op or a malformed field replies `{"ok":false}` + * instead of throwing, because a daemon that dies on a bad control line takes + * every connected session down with it. + * + * `clear-session-record` sweeps EVERY live session: the caller knows its own + * context id, never which connection carries it (subagents dispatch over the + * parent's). The id is an opaque bucket label — never logged, never echoed. + */ + private handleControl(socket: net.Socket, command: DaemonControlCommand): void { + let reply: { ok: boolean; cleared?: number } = { ok: false }; + if (command.op === 'clear-session-record' && typeof command.sessionId === 'string') { + let cleared = 0; + for (const session of [...this.clients]) { + try { + cleared += session.clearSessionRecord(command.sessionId); + } catch { /* one wedged session must not sink the sweep */ } + } + reply = { ok: true, cleared }; + } + try { socket.end(JSON.stringify(reply) + '\n'); } catch { /* peer already gone */ } + } + private dropClient(session: MCPSession): void { if (!this.clients.delete(session)) return; this.clientPeers.delete(session); @@ -772,6 +804,35 @@ export function parseClientHelloLine( return { pid: o.pid, hostPid: typeof o.hostPid === 'number' ? o.hostPid : null }; } +/** + * A one-shot control command, as it arrived on the wire. Fields stay `unknown` + * because they are unvalidated client data — {@link Daemon.handleControl} is + * the single place that decides whether a command is answerable. + */ +export interface DaemonControlCommand { + op: unknown; + sessionId: unknown; +} + +/** + * Read the first line of a connection as a control command, or `null` when it + * is not one. + * + * The `codegraph_control` marker is what CLAIMS the connection: without it the + * line belongs to a proxy hello or a direct MCP client and must reach the + * transport verbatim, so a junk first line stays MCP's problem exactly as it + * was. With it, the connection is a control caller and is answered as one even + * if the rest of the command is nonsense. + */ +export function parseDaemonControlLine(line: string): DaemonControlCommand | null { + let parsed: unknown; + try { parsed = JSON.parse(line); } catch { return null; } + if (!parsed || typeof parsed !== 'object') return null; + const o = parsed as Record; + if (o.codegraph_control !== 1) return null; + return { op: o.op, sessionId: o.sessionId }; +} + /** * A client's peer is dead when its proxy process is gone, or when its known * host process is gone. Unknown pid (no client-hello) is never "dead" on this @@ -787,6 +848,14 @@ export function peerIsDead( return false; } +/** Outcome of reading a connection's first line. */ +interface ClientHelloResult { + pid: number | null; + hostPid: number | null; + /** Set when that line claimed the connection as a one-shot control command. */ + control?: DaemonControlCommand; +} + /** * Read the optional client-hello line a proxy sends after the daemon hello. * Always resolves (never rejects) — fail-safe by design, since every connection @@ -796,15 +865,13 @@ export function peerIsDead( * Accumulates as Buffers and splits on the newline byte so a UTF-8 sequence * straddling a chunk boundary in the unshifted tail is never corrupted. */ -function readClientHello( - socket: net.Socket, -): Promise<{ pid: number | null; hostPid: number | null }> { +function readClientHello(socket: net.Socket): Promise { return new Promise((resolve) => { let chunks: Buffer[] = []; let total = 0; let settled = false; const finish = ( - peers: { pid: number | null; hostPid: number | null }, + peers: ClientHelloResult, putBack?: Buffer, ) => { if (settled) return; @@ -845,15 +912,24 @@ function readClientHello( else chunks = [all]; return; } - const peers = parseClientHelloLine(all.subarray(0, nl).toString('utf8')); + const line = all.subarray(0, nl).toString('utf8'); + const peers = parseClientHelloLine(line); if (peers) { const tail = all.subarray(nl + 1); finish(peers, tail.length > 0 ? tail : undefined); - } else { - // First line is not a client-hello (legacy/direct client) — hand the - // whole buffer back so the transport sees the message verbatim. - finish({ pid: null, hostPid: null }, all); + return; + } + const control = parseDaemonControlLine(line); + if (control) { + // One line in, one line out: nothing after it is application data, so + // the tail is dropped rather than handed to a transport that will + // never exist for this connection. + finish({ pid: null, hostPid: null, control }); + return; } + // First line is not a client-hello (legacy/direct client) — hand the + // whole buffer back so the transport sees the message verbatim. + finish({ pid: null, hostPid: null }, all); }; const onEnd = () => finish({ pid: null, hostPid: null }); // On timeout, hand back whatever partial bytes accumulated — discarding diff --git a/src/mcp/explore-dedup.ts b/src/mcp/explore-dedup.ts index 541945c7d..1fd9b60d8 100644 --- a/src/mcp/explore-dedup.ts +++ b/src/mcp/explore-dedup.ts @@ -80,6 +80,27 @@ export function exploreDedupEnabled(): boolean { return !OFF.has(raw.trim().toLowerCase()); } +/** + * Whether the explore schema ADVERTISES its `sessionId` parameter. Off by + * default: a host hook's `updatedInput` reaches the server whether or not the + * property is declared, so declaring it buys the hook path nothing and only + * exposes a knob an agent could set by hand. + * `CODEGRAPH_MCP_EXPLORE_SESSION_PARAM=1` declares it for a host that validates + * arguments against the schema, or a harness that passes it as an explicit tool + * argument. `MCP_`-prefixed like `CODEGRAPH_MCP_TOOLS`, because it shapes the + * MCP surface only — unlike the `CODEGRAPH_EXPLORE_*` family, which changes + * behaviour everywhere. + * + * The HANDLER accepts `sessionId` either way — this gates the advertisement + * alone, never the bucketing. Read per call (not memoized) so a test can toggle it. + */ +export function exploreSessionParamAdvertised(): boolean { + const raw = process.env.CODEGRAPH_MCP_EXPLORE_SESSION_PARAM; + if (raw === undefined) return false; + const value = raw.trim().toLowerCase(); + return value !== '' && !OFF.has(value); +} + /** * Identity of the bytes a call served for one file. * diff --git a/src/mcp/explore-session-state.ts b/src/mcp/explore-session-state.ts index 26439c03e..90d1c0137 100644 --- a/src/mcp/explore-session-state.ts +++ b/src/mcp/explore-session-state.ts @@ -16,7 +16,12 @@ * 1. **Per session, never persisted.** One instance is owned by an * {@link ../mcp/session.MCPSession} and dies with the socket. A new agent * session starts clean — dedup across sessions would suppress source the - * new agent has never seen. + * new agent has never seen. One MCP connection is not always one agent + * CONTEXT, though: Claude Code subagents dispatch over the parent's + * connection, so the record is additionally bucketed by an optional + * caller-supplied `sessionId` (see {@link normalizeExploreSessionId}) — + * without it a subagent is told source was "already sent" to a context + * that never received it, and it Reads the file. * 2. **Per project inside the session.** A session can query several projects * by `projectPath`, so state is keyed by the RESOLVED project root * (`cg.getProjectRoot()`), not by whatever path the agent typed. @@ -39,6 +44,7 @@ */ import * as path from 'path'; +import { createHash } from 'crypto'; /** * Property on a {@link ../mcp/tools.ToolResult} carrying what an explore call @@ -155,6 +161,42 @@ export function exploreProjectKey(projectRoot: string): string { : resolved; } +/** + * Longest caller-supplied session id used verbatim as a bucket label. The cap + * exists to bound what a client can make the server hold as a key; beyond it + * the id is HASHED, never truncated. + * + * Truncating would be a correctness hole, not a size trade: two distinct ids + * sharing a 128-char prefix would collapse into ONE bucket — a record shared + * between two contexts, which is exactly the bug this bucketing exists to fix, + * and the pointers it would hand the second context name source only the first + * received. A digest keeps distinct ids distinct at a fixed size. + */ +const MAX_SESSION_ID_CHARS = 128; + +/** Bucket shared by every caller that supplies no `sessionId`. */ +const DEFAULT_BUCKET = ''; + +/** + * Normalize a caller-supplied `sessionId` into a bucket label. + * + * CLIENT-CONTROLLED DATA: this value is a Map key and nothing else. It must + * never reach a response, a log line, or a path. Anything that is not a + * non-empty string collapses to `undefined` — the default bucket, which is + * byte-for-byte the behaviour of a host that injects no id at all. + * + * Deterministic in both branches: one id always lands in one bucket, and two + * ids that differ anywhere — including only past {@link MAX_SESSION_ID_CHARS} — + * land in different ones. + */ +export function normalizeExploreSessionId(raw: unknown): string | undefined { + if (typeof raw !== 'string') return undefined; + const trimmed = raw.trim(); + if (!trimmed) return undefined; + if (trimmed.length <= MAX_SESSION_ID_CHARS) return trimmed; + return createHash('sha256').update(trimmed).digest('hex'); +} + /** * Merge overlapping / adjacent spans into the smallest equivalent set, then cap * it. Adjacency (`next.start <= cur.end + 1`) counts as overlap: two ranges that @@ -195,12 +237,24 @@ export function rangesCover(ranges: ReadonlyArray, line: numbe } interface MutableProjectState { + /** Caller bucket this project's history belongs to; `''` = no `sessionId`. */ + bucket: string; projectRoot: string; callCount: number; responseBytes: number; calls: ExploreCallRecord[]; } +/** + * Map key for one (bucket, project) pair. A resolved project root can never + * contain NUL, so the separator keeps the pair unambiguous whatever a client + * spells as its session id. Bucket membership is still tested against the + * stored {@link MutableProjectState.bucket}, never by prefix-matching this key. + */ +function bucketedKey(bucket: string, projectKey: string): string { + return `${bucket}\u0000${projectKey}`; +} + /** * One MCP session's explore history. Created per session, thrown away with it. * @@ -210,17 +264,20 @@ interface MutableProjectState { * the tool-call path and a bookkeeping bug must never fail an explore. */ export class ExploreSessionState { - /** Insertion-ordered; a touched project is re-inserted, so the head is the LRU. */ + /** + * Keyed by (caller bucket, project). Insertion-ordered; a touched entry is + * re-inserted, so the head is the LRU. + */ private readonly projects = new Map(); /** - * File an emission. Returns the record as stored (with its session call - * index), or `null` if the emission was unusable. + * File an emission under the caller's bucket. Returns the record as stored + * (with its session call index), or `null` if the emission was unusable. */ - record(emission: ExploreEmission): ExploreCallRecord | null { + record(emission: ExploreEmission, sessionId?: string): ExploreCallRecord | null { if (!emission || typeof emission.projectRoot !== 'string' || !emission.projectRoot) return null; - const key = exploreProjectKey(emission.projectRoot); - const state = this.touch(key, emission.projectRoot); + const bucket = normalizeExploreSessionId(sessionId) ?? DEFAULT_BUCKET; + const state = this.touch(bucket, exploreProjectKey(emission.projectRoot), emission.projectRoot); state.callCount += 1; state.responseBytes += Math.max(0, emission.responseBytes || 0); @@ -240,18 +297,18 @@ export class ExploreSessionState { return record; } - /** Full state for one project, or `null` if it was never queried this session. */ - forProject(projectRoot: string): ExploreProjectState | null { - const state = this.projects.get(exploreProjectKey(projectRoot)); + /** Full state for one project in one bucket, or `null` if never queried. */ + forProject(projectRoot: string, sessionId?: string): ExploreProjectState | null { + const state = this.projects.get(this.keyFor(sessionId, projectRoot)); return state ? cloneProject(state) : null; } - /** Explore calls made this session against a project (including evicted ones). */ - callCount(projectRoot: string): number { - return this.projects.get(exploreProjectKey(projectRoot))?.callCount ?? 0; + /** Explore calls one bucket made against a project (including evicted ones). */ + callCount(projectRoot: string, sessionId?: string): number { + return this.projects.get(this.keyFor(sessionId, projectRoot))?.callCount ?? 0; } - /** Every project this session has queried, least-recently-used first. */ + /** Every project this session has queried, across all buckets, LRU first. */ snapshot(): ExploreProjectState[] { return [...this.projects.values()].map(cloneProject); } @@ -261,10 +318,15 @@ export class ExploreSessionState { * {@link EXPLORE_SESSION_LIMITS.MAX_VIEW_CALLS} calls per project: it crosses a * worker boundary on every explore, so it carries what a dedup/decay decision * needs and not the whole history. + * + * Scoped to ONE bucket: a caller may only be told it already holds source its + * own context was served. Everything a different `sessionId` (or none) was + * sent is invisible here. */ - view(): ExploreSessionView { + view(sessionId?: string): ExploreSessionView { + const bucket = normalizeExploreSessionId(sessionId) ?? DEFAULT_BUCKET; return { - projects: [...this.projects.values()].map((state) => ({ + projects: [...this.projects.values()].filter((state) => state.bucket === bucket).map((state) => ({ projectRoot: state.projectRoot, callCount: state.callCount, responseBytes: state.responseBytes, @@ -275,25 +337,58 @@ export class ExploreSessionState { }; } - /** Drop everything. Used by tests; a real session just goes away instead. */ + /** Drop everything, every bucket. Used by tests; a real session just goes away. */ clear(): void { this.projects.clear(); } /** - * Fetch a project's state, creating it if new, and mark it most-recently-used. - * Evicts the LRU project past the bound — dropping a project entirely (rather - * than its detail) is right here: a session that has moved on to four other - * repos is not about to re-ask the first one. + * Forget one caller's history and nothing else — the reset a host hook fires + * when that context is discarded (a compact, a finished subagent), after + * which its next call is served as a first call again. + * + * Returns how many project entries were dropped. An id that normalizes away + * clears NOTHING: the default bucket is shared by every caller that sends no + * `sessionId`, so a malformed id must never be able to wipe it. + */ + clearSessionRecord(sessionId: string): number { + const bucket = normalizeExploreSessionId(sessionId); + if (bucket === undefined) return 0; + let removed = 0; + for (const [key, state] of [...this.projects]) { + if (state.bucket !== bucket) continue; + this.projects.delete(key); + removed += 1; + } + return removed; + } + + /** Where one bucket's copy of a project is filed. */ + private keyFor(sessionId: string | undefined, projectRoot: string): string { + return bucketedKey(normalizeExploreSessionId(sessionId) ?? DEFAULT_BUCKET, exploreProjectKey(projectRoot)); + } + + /** + * Fetch a bucket's project state, creating it if new, and mark it + * most-recently-used. Evicts the LRU project past the bound — dropping a + * project entirely (rather than its detail) is right here: a session that has + * moved on to four other repos is not about to re-ask the first one. + * + * The bound spans buckets: {@link EXPLORE_SESSION_LIMITS.MAX_PROJECTS} caps + * (bucket, project) entries, not projects per bucket, so a wide subagent + * fan-out over one repo evicts the least recently active caller's record. That + * costs a re-serve for whoever was evicted — the safe direction (see the + * module header); it can never point a caller at source it did not receive. */ - private touch(key: string, projectRoot: string): MutableProjectState { + private touch(bucket: string, projectKey: string, projectRoot: string): MutableProjectState { + const key = bucketedKey(bucket, projectKey); const existing = this.projects.get(key); if (existing) { this.projects.delete(key); this.projects.set(key, existing); return existing; } - const created: MutableProjectState = { projectRoot, callCount: 0, responseBytes: 0, calls: [] }; + const created: MutableProjectState = { bucket, projectRoot, callCount: 0, responseBytes: 0, calls: [] }; this.projects.set(key, created); while (this.projects.size > EXPLORE_SESSION_LIMITS.MAX_PROJECTS) { const lru = this.projects.keys().next().value as string | undefined; diff --git a/src/mcp/session.ts b/src/mcp/session.ts index 1d5bd79c3..33d9e7288 100644 --- a/src/mcp/session.ts +++ b/src/mcp/session.ts @@ -159,6 +159,15 @@ export class MCPSession { return this.exploreSession; } + /** + * Forget one caller context's explore history (the daemon's control line + * relays a host hook's compact/subagent-teardown reset). Narrow on purpose: + * the daemon selects buckets, it never handles the state itself. + */ + clearSessionRecord(sessionId: string): number { + return this.exploreSession.clearSessionRecord(sessionId); + } + private async handleMessage(message: JsonRpcRequest | JsonRpcNotification): Promise { const isRequest = 'id' in message; switch (message.method) { diff --git a/src/mcp/tools.ts b/src/mcp/tools.ts index 5c23f675d..b23f2f1e5 100644 --- a/src/mcp/tools.ts +++ b/src/mcp/tools.ts @@ -47,6 +47,7 @@ import { EXPLORE_EMISSION_KEY, EXPLORE_SESSION_VIEW_ARG, ExploreSessionState, + normalizeExploreSessionId, readExploreSessionView, viewForProject, type ExploreEmission, @@ -57,6 +58,7 @@ import { EXPLORE_DEDUP, dedupeRange, exploreDedupEnabled, + exploreSessionParamAdvertised, fileFingerprint, formatBackReference, mergeRanges, @@ -1279,6 +1281,32 @@ function withRequiredProjectPath(defs: ToolDefinition[]): ToolDefinition[] { }); } +/** + * Declare explore's `sessionId` parameter, when the host needs it declared to + * let it through (see {@link exploreSessionParamAdvertised}). Applied where tool + * definitions are SERVED, not at module load, so the env var can be toggled for + * one server without rebuilding the static list. + */ +function withSessionIdParam(defs: ToolDefinition[]): ToolDefinition[] { + if (!exploreSessionParamAdvertised()) return defs; + return defs.map((tool) => { + if (tool.name !== 'codegraph_explore') return tool; + return { + ...tool, + inputSchema: { + ...tool.inputSchema, + properties: { + ...tool.inputSchema.properties, + sessionId: { + type: 'string', + description: 'Caller-context id, injected by the host\'s hooks — not set manually.', + }, + }, + }, + }; + }); +} + /** * Allowlist-filtered tool definitions WITHOUT an engine — the static surface the * proxy answers `tools/list` with before any project is open. Mirrors @@ -1288,10 +1316,10 @@ function withRequiredProjectPath(defs: ToolDefinition[]): ToolDefinition[] { export function getStaticTools(): ToolDefinition[] { const raw = process.env.CODEGRAPH_MCP_TOOLS; if (!raw || !raw.trim()) { - return tools.filter(t => DEFAULT_MCP_TOOLS.has(t.name.replace(/^codegraph_/, ''))); + return withSessionIdParam(tools.filter(t => DEFAULT_MCP_TOOLS.has(t.name.replace(/^codegraph_/, '')))); } const allow = new Set(raw.split(',').map(s => s.trim().replace(/^codegraph_/, '')).filter(Boolean)); - return allow.size ? tools.filter(t => allow.has(t.name.replace(/^codegraph_/, ''))) : tools; + return withSessionIdParam(allow.size ? tools.filter(t => allow.has(t.name.replace(/^codegraph_/, ''))) : tools); } /** @@ -1484,9 +1512,9 @@ export class ToolHandler { // No explicit allowlist → the default 4-tool surface (see // DEFAULT_MCP_TOOLS for the evidence). An allowlist replaces the // default entirely, so any defined tool can be re-enabled. - let visible = allow + let visible = withSessionIdParam(allow ? tools.filter(t => allow.has(t.name.replace(/^codegraph_/, ''))) - : tools.filter(t => DEFAULT_MCP_TOOLS.has(t.name.replace(/^codegraph_/, ''))); + : tools.filter(t => DEFAULT_MCP_TOOLS.has(t.name.replace(/^codegraph_/, '')))); // No default project loaded → no-root-index case (#993): a gateway server // started outside any repo, or a monorepo root whose indexes live in // sub-projects. With nothing to fall back to, EVERY call needs an explicit @@ -2023,14 +2051,20 @@ export class ToolHandler { // object down, on the ToolResult up — because either leg may cross a // structured-clone boundary into a worker, where a closure or a handler // field could not follow. - const dispatchArgs = this.withSessionView(toolName, args, sessionState); + // One MCP connection can carry several agent contexts (Claude Code + // subagents dispatch over the parent's), so the session record is bucketed + // by the caller id a host hook injects. Derived ONCE and threaded to both + // the read and the write side: a call must record into the same bucket it + // was answered from. CLIENT-CONTROLLED — an opaque bucket label only. + const exploreSessionId = normalizeExploreSessionId(args.sessionId); + const dispatchArgs = this.withSessionView(toolName, args, sessionState, exploreSessionId); const raw = (this.queryPool && this.queryPool.healthy && this.queryPool.ready) ? await this.queryPool.run(toolName, dispatchArgs) : await this.executeReadTool(toolName, dispatchArgs); // Record + STRIP before anything else touches the result: the emission is // internal bookkeeping and must never reach the client, whether or not a // caller passed session state. - const result = this.takeExploreEmission(raw, sessionState); + const result = this.takeExploreEmission(raw, sessionState, exploreSessionId); const withWorktree = this.withWorktreeNotice(result, args.projectPath as string | undefined); return this.withStalenessNotice(withWorktree, args.projectPath as string | undefined); } catch (err) { @@ -2066,6 +2100,7 @@ export class ToolHandler { toolName: string, args: Record, sessionState: ExploreSessionState | undefined, + sessionId?: string, ): Record { if (!(EXPLORE_SESSION_VIEW_ARG in args) && (!sessionState || toolName !== 'codegraph_explore')) { return args; @@ -2073,7 +2108,10 @@ export class ToolHandler { const copy = { ...args }; delete copy[EXPLORE_SESSION_VIEW_ARG]; if (sessionState && toolName === 'codegraph_explore') { - copy[EXPLORE_SESSION_VIEW_ARG] = sessionState.view(); + // Only this caller's bucket: what a sibling context was served is not + // this one's to be told it already holds. `sessionId` itself stays on the + // args — declared or not, dispatch ignores it. + copy[EXPLORE_SESSION_VIEW_ARG] = sessionState.view(sessionId); } return copy; } @@ -2091,13 +2129,14 @@ export class ToolHandler { private takeExploreEmission( result: ToolResult, sessionState: ExploreSessionState | undefined, + sessionId?: string, ): ToolResult { const emission = result?.[EXPLORE_EMISSION_KEY]; if (emission === undefined) return result; delete result[EXPLORE_EMISSION_KEY]; if (sessionState) { try { - sessionState.record(emission); + sessionState.record(emission, sessionId); } catch { /* bookkeeping only — never fail a served call */ } } return result;