fix(mcp): keep explore's already-sent tracking per calling context - #1621
Draft
AtkinsChang wants to merge 4 commits into
Draft
fix(mcp): keep explore's already-sent tracking per calling context#1621AtkinsChang wants to merge 4 commits into
AtkinsChang wants to merge 4 commits into
Conversation
AtkinsChang
force-pushed
the
fix/1620-explore-dedup-session-buckets
branch
5 times, most recently
from
August 26, 2026 14:29
80d7988 to
a7754af
Compare
Author
|
@colbymchenry Before I polish this further: are you okay with the general approach here? Since an MCP connection can span multiple contexts (main/subagents, and across compaction), this PR:
So CodeGraph relies on the harness to define context lifecycle rather than trying to infer it. Is this ownership model acceptable, or would you prefer a different approach? |
`codegraph_explore`'s cross-call dedup keyed its record per MCP connection, but one connection carries several agent contexts — harnesses route a subagent's calls over its parent's — so a fresh subagent was told source was "already sent" to a context that never received it, and it Read the file. The record is now bucketed by an optional caller-supplied `sessionId` a host hook injects. It is Map-key-only client data: trimmed, never logged, echoed, or used in a path; anything unusable collapses to the default bucket, which is byte-for-byte today's behaviour for hosts that inject nothing. Past 128 chars an id is hashed rather than cut — truncation would collapse two ids sharing a prefix into one shared record, reintroducing the bug this fixes. The LRU bound spans buckets, so a wide fan-out evicts the least recently active caller — a re-serve, the safe direction. The schema does NOT advertise the parameter. A host hook's replacement input reaches the server whether or not the property is declared, so declaring it buys the hook path nothing while exposing a knob an agent could set by hand. `CODEGRAPH_MCP_EXPLORE_SESSION_PARAM=1` declares it where a host validates arguments against the schema, or a harness passes it explicitly; the handler accepts it either way.
…ssion A bucket has to be forgettable: after a compact the agent no longer holds the source the server believes it was sent, so its next explore comes back as pointers to text that is gone — the shape that costs a Read. Only the host knows when that happens, so the reset arrives from outside the MCP conversation. The daemon now answers a one-shot control line: a connection whose first line carries the `codegraph_control` marker is a command, never a session, so it is never registered as a client and never holds the idle timer. It sweeps every live session for the named bucket and replies with how much it dropped; an unknown op or a non-string id is refused rather than thrown, since a daemon that dies on a bad control line takes every connected session with it. A malformed line or a client hello still reaches the transport verbatim. A hidden `codegraph hooks` namespace carries the two entry points a host invokes: `pre-tool-use` stamps the calling context id onto a `codegraph_explore` call, `post-compact` relays the reset to the project's daemon. Both follow `prompt-hook`'s contract — always exit 0, stdout carries the hook protocol and nothing else. The context id is the host session plus the agent id when a subagent is calling. A bare agent id would assume the host numbers its agents uniquely across all sessions; scoping each to its session drops that assumption, and both hooks derive through the same function so a reset always names the bucket the stamp filled. `pre-tool-use --agent codex` additionally pairs the rewrite with `permissionDecision`, which codex requires or it silently discards the rewrite, and which is inert for its approvals. No other agent gets the field: Claude Code acts on it, so "allow" would override a user who chose not to allowlist codegraph. The agent is named by the wiring that installed the hook, never inferred from the payload, and an unrecognized name omits the field.
Per-caller bucketing only engages if something injects the caller id, so without the hooks installed no real user gets the fix — the record stays keyed per MCP connection and a subagent is still told source was "already sent". The Claude target now merges two entries into the settings file for the install's scope: PreToolUse, matched on the tool suffix `mcp__.*__codegraph_explore` because the MCP server name is the user's to rename, and PostCompact, which fires once compaction has completed and before the resumed session's first turn — the only moment a reset is correct, since PreCompact would run while the agent still holds the source. PostCompact covers manual and automatic compaction alike and supports no matcher, so that key is omitted rather than written empty. The PreToolUse command names its agent, so the hook emits Claude's protocol rather than another's — which here means no `permissionDecision`: Claude acts on that field, and "allow" would auto-approve a call the user's own permission flow should decide. The command prefers the absolute launcher, resolved through the existing install-method detection, rather than trusting PATH: hooks run under whatever environment the host hands them, and Claude executes them through Git Bash on Windows. Entries are recognized by their `hooks <subcommand>` substring with flags excluded, so an entry whose path or flags have drifted is rewritten in place instead of joined by a second copy of itself. The structure surgery — launcher resolution, the merge, and the prune — lives in the shared target helpers rather than in this target, since it is the host's event names and file that vary, not the shape of the edit. Each target keeps its own read/write policy, which is where they genuinely differ. Surgical throughout: our entries go in as their own groups, appended so a host that identifies a hook by position never renumbers the user's; uninstall drops exactly those two commands and prunes only structures left empty; a matcher-less sibling group survives both directions; a non-array a user put under one of these events is never overwritten; and a re-run on an unchanged file leaves the bytes alone. Claude only — Codex is immune by construction and Cursor/opencode have no equivalent hook system.
Codex carries the same problem the Claude wiring solved: without a hook to stamp the calling context, the server's already-sent record stays keyed per MCP connection. Its per-thread connections mean it never needs the stamp for isolation, but the post-compact reset still has to name a bucket. Codex discovers hooks from a `hooks.json` in each config layer's `.codex/` folder, which maps onto the global/local split the TOML config already uses. Two entries go in: PreToolUse, whose matcher is a regex over the tool name, and PostCompact, which codex gates behind compaction SUCCESS in three of its four compaction paths. That gate is what we want: in all four paths every failure returns before the history rewrite commits, so a failed compaction leaves the agent's context — and the record describing it — intact, and skipping the reset is exactly right. PreCompact would instead clear a still-valid record on every failed attempt. This matches how the Claude target wires the same reset. The PreToolUse command declares `--agent codex`, which is what makes the hook pair `permissionDecision` with its rewrite. Codex requires that pair or it silently drops the rewrite; no other agent receives the field. Both entries are recorded as trusted in `[hooks.state]` as they are written, so codex does not stop to ask about them. Its review flow guards hooks that arrive from somewhere the user didn't choose; an installer they just invoked, writing its own two entries pointing at its own binary, is not that threat, and a local install's state still sits behind codex's project-trust gate. Trust is held per ENTRY — a hash of each handler's normalized identity — so this can only ever record the two entries the installer wrote; the user's hooks are unreachable from there and their records are never read or rewritten. The hash reimplements a codex internal and is pinned by a fixture test against real entries codex wrote; if it ever stops agreeing, the entry simply reads as needing review, which is what happens today. An unparseable hooks.json is left exactly as it is rather than backed up and replaced: codex would otherwise run our file in place of config we failed to read. The contract suite's "sibling MCP server" case keyed off the first .json in describePaths as a proxy for "this target's MCP config". hooks.json is written but holds no MCP entry, so that proxy now asks detect() for the config path directly. Its "Claude-only" hooks case becomes "Claude and Codex only", and now also checks the remaining targets' own config files rather than only Claude's. The array-of-tables case no longer requires the user's `[[history]]` tables to END the file, since the trust records append after them — it requires them to survive byte-for-byte, which is the actual guarantee.
AtkinsChang
force-pushed
the
fix/1620-explore-dedup-session-buckets
branch
from
August 27, 2026 05:11
a7754af to
e4be07c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1620
Problem
The explore already-sent record (the source of "Already sent earlier in this conversation" back-references) is scoped to the MCP connection. Harnesses like Claude Code route a subagent's tool calls over the same connection as the main agent, and a compaction empties the context while the connection lives on — in both cases the agent is handed pointers to source its current context never held.
What this does (4 commits)
fix(mcp)— per-context bucketing. The session record is bucketed by an optional, host-injectedsessionIdoncodegraph_explore. No id → the shared default bucket, byte-for-byte today's behavior (goldens unchanged). Ids are opaque Map keys (never logged/echoed/pathed); over-long ids are sha256-hashed rather than truncated so distinct ids can never collide into one bucket. The schema doesn't advertise the parameter by default;CODEGRAPH_MCP_EXPLORE_SESSION_PARAM=1declares it for hosts that validate args (hook-injected fields reach the server either way — verified on Claude Code 2.1.246).feat(mcp)— the reset path. A one-shot control line on the daemon socket ({"codegraph_control":1,"op":"clear-session-record","sessionId":…}) clears one caller's buckets across live sessions; it can never clear the shared default bucket. Two always-exit-0 hook entry points drive it:codegraph hooks pre-tool-usestampssessionId(<session_id>for the main agent,<session_id>:<agent_id>for a subagent) onto explore calls via the host's PreToolUseupdatedInput;codegraph hooks post-compactreads the compact hook payload and sends the control line.--agent codexadditionally pairspermissionDecision:"allow"with the rewrite — codex requires the pair (and treats the decision as approval-inert) while Claude Code would act on it, so it is emitted only where the wiring explicitly declares codex.feat(installer)— Claude Code wiring.codegraph installadds two hooks to Claude settings:PreToolUse(matchermcp__.*__codegraph_explore) →hooks pre-tool-use --agent claude, andPostCompact→hooks post-compact. Surgical merge, idempotent, uninstall reverses; user hooks untouched.feat(installer)— Codex wiring. Same two entries into~/.codex/hooks.json(global) /./.codex/hooks.json(local), with the samePostCompactevent (codex fires it only on successful compaction, and every failure path in all four compaction implementations returns before the history rewrite commits — a failed compaction leaves the agent's context intact, so skipping the reset there is correct). Entries are appended, never reordered — codex's hook-trust key includes the group index, so reordering would silently un-trust existing user hooks.[hooks.state]is never written: codex asks the user to review & trust the new entries on next run.Compatibility