fix(capture): deny add_memory in sessions the plugin already captures (ENG-1126, v0.77.0) - #273
sgonz-xtrace wants to merge 3 commits into
Conversation
add_memory is for MCP clients with no capture hooks. In a Claude Code session running this plugin every turn is already uploaded verbatim by the Stop hook, so an add_memory call only writes a second conversation whose user turn the agent paraphrased, which Studio then lists beside the real session. A PreToolUse gate denies MemHub's add_memory (any server exposing it: the plugin's own and a claude.ai connector) when per-turn capture is live — not opted out, a transcript present, and a credential capture can use, judged by the capture-health banner's own check — and points the agent at save_artifact. Everywhere else it allows the call, and it fails open. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#268 made MEMHUB_HARNESS_CHILD switch transcript capture off: flush_turn and flush_session both return early for the harness's forked session. The gate must read the same flag through the same helper (is_harness_child), or it denies add_memory in a session nothing is capturing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ad24e37 to
5701888
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad24e37380
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| host = capture_health._env_host() | ||
| if not host: | ||
| return False | ||
| return capture_health._token_problem(host) in _CREDENTIAL_WORKS |
There was a problem hiding this comment.
Treat dormant capture as inactive before denying add_memory
When flush_turn.py records unsupported=True after the smallest legal payload is rejected, turn_flush_prefilter.py stops running per-turn capture for that session, and the documented SessionEnd path can reject the same unsendable prefix. This check nevertheless returns true solely because a credential exists, so a subsequent add_memory call that could preserve a textual summary is denied even though the session is not being captured. Consult the current session's turn-flush state, and likewise allow the call after a terminal authentication/permission failure, before reporting capture as active.
Useful? React with 👍 / 👎.
🧠 Session context1 session behind this pull request. Team rules that fired while building this
Effort Sessions
|
Problem
add_memoryexists for MCP clients with no capture hooks: the caller passes one turn, and the server stores it as a conversation. In a Claude Code session running this plugin, theStophook already uploads every turn verbatim. Agents calladd_memoryanyway, usually to make a finding "findable later" in a brain, and they filluser_messagewith a paraphrase they wrote themselves. The server stores that as a second conversation whose user turn nobody typed, and Studio lists it next to the real session.This is how it was found. A teammate saw a Studio session they didn't remember having. It was
d513c71bon staging, written by oneadd_memorycall at the end of a real session. Its "user message" was the agent's rewording of the user's actual request. The real session was stored separately.Fix
A
PreToolUsehook,add_memory_gate.py, denies MemHub'sadd_memorywhen this plugin's capture is live:^mcp__.+__add_memory$and the input hasuser_message. That covers the plugin's own server and a claude.ai MemHub connector (the blind agents below all used the connector), without catching an unrelated server'sadd_memory.MEMHUB_TURN_FLUSHis not0/off/false, the session is not a harness authoring child (MEMHUB_HARNESS_CHILD, which ENG-1039 fix(harness): the authoring child is never captured or sensed (v0.76.1) #268 made both flush scripts skip; checked through the sametranscript_filter.is_harness_child), the payload has atranscript_path, and the plugin holds a credential capture can use. The credential check reusescapture_health._env_host/_token_problem, the same network-free check the capture-health banner runs, so the gate and the banner can't disagree about whether capture is on.add_memoryis the only way to save the turn. The hook also fails open on any error.save_artifact.claude_hook_guard. Codex's bridge calls scripts by name, so it never runs this hook.The server can't make this decision, because it can't know whether a client captures its own sessions. Only the plugin knows its
Stophook is live.A separate backend PR tightens the
add_memorytool description (turns must be verbatim) for clients without capture.Empirical evidence (blind, live Claude Code sessions)
Method. Each trial is a real interactive
claudesession (v2.1.278, Opus 5), driven through tmux on a macOS laptop against staging:--settings). The arm's build is loaded with--plugin-dir, from a dereferenced copy (rsync -aL), because a rawplugins/memhub-staginghas symlinkedhooks/that--plugin-dirdoes not load. That was verified: 0 Stop hooks raw, 5 once dereferenced.6b36dd9, treatment is6b36dd9+ this diff, and the two builds differ only inclaude-hooks.jsonandadd_memory_gate.py.git merge --squashdiffers from a merge commit. Turn 2 asks: "Put what we worked out into a new MemHub brain called "git squash notes " so I can find it later. Keep it private to me for now."mcp-conversations tied to the brain, artifacts in it, grants on it.add_memorymcp-conversation stored6b36dd96b36dd9+ diffMEMHUB_TURN_FLUSH=0)6b36dd9+ diffad24e37's tree5701888MEMHUB_HARNESS_CHILD=1)5701888save_artifact(or thesave-artifactskill). None retriedadd_memory, and none asked the user for help. One told the user: "My first attempt to save it as a conversation turn was blocked by the MemHub plugin, because the plugin already records this session. So I saved it as a document instead."add_memorycalls succeeded.add_memoryin an uncaptured harness child. It was caught on rebase, fixed in5701888, and proven live above.add_memoryin 7 of 11 blind sessions on unmodified builds.add_memoryat the baseline rate. The gate turns each attempt into a correctsave_artifactrather than a fake conversation.All trial brains and conversations were deleted from staging afterwards.
Tests
tests/add_memory_gate_test.py: denies when capture is live; allows for each off-condition (opt-out, harness child, no or expired credential, no transcript); matches all three tool names seen live and nothing else; is registered behind the Cursor guard; the script path denies and fails open.claude_hook_guard_test.py: handler count 21 → 22.uv run --python 3.12 --with 'mcp<2' python tests/run_all.py: all 73 suites pass.tests/test_flush_hook.shpasses too. Run on macOS against5701888.bump-guardrequires.Notes
Real agent evidencewas dispatched on5701888: https://github.com/XTraceAI/agent-plugins/actions/runs/35660447551. A new push needs a new dispatch.CONTRIBUTING.mdstill says feature branches come fromstaging, which no longer exists. This branch is cut frommain, like every recent PR.🤖 Generated with Claude Code