Repository navigation
Conversation
When the Claude PreToolUse hook is cancelled at its 5s timeout, mem_save reaches the MCP server without a session_id and the fallback refuses because two live sessions share the project and directory. SessionStart (startup, resume, clear, fork) and each successful PreToolUse confirmation now record host process -> session under the data dir, keyed by PID plus process start time so a reused PID never matches. The MCP server resolves an omitted session_id through its parent's binding when the bound session, or its live :resume:N continuation, is in the write's project. Otherwise the existing fail-closed ambiguity refusal applies. Explicit session IDs and non-Claude clients are unchanged. Refs Gentleman-Programming#1693
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
…hook Measured on GitHub Actions (Claude Code 2.1.304, Windows, macOS and Ubuntu, npm and native installs): shell-form hooks run through sh -c or Git Bash, so on Ubuntu (dash) and Windows a bash script's $PPID and a shell-form executable's os.Getppid() are an intermediate shell, not the Claude process. Exec-form hooks (args set, no shell, Claude Code 2.1.139+) are direct children of Claude on every platform tested. Publish the binding from a dedicated exec-form SessionStart hook using os.Getppid(), drop ENGRAM_HOST_PID from session-start.sh, and stop publishing from the shell-form PreToolUse hook, which is left unchanged so older Claude Code versions keep working. Evidence: https://github.com/doblas/engram-1693-probe Refs Gentleman-Programming#1693
Require Claude Code 2.1.295 hook-failure semantics so a failed or cancelled identity rewrite cannot fall through to a stale host binding. Keep hook matching, command and timeout unchanged. Document the host requirement and advance paired plugin metadata. Refs Gentleman-Programming#1693
Keep absent bindings on the existing fallback path, but propagate verified host-binding failures before any save, summary, prompt or passive-capture write can be attributed to another live session. Cover unchanged fallback and explicit-session behavior alongside no-write rejection. Refs Gentleman-Programming#1693
Merge canonical main 9795575 into PR1954 while keeping both claude-bind-session and resolve-hook-project. Preserve truthful usage and isolate native hook test bindings lazily so configuration-free subprocesses need no temporary directory. Keep test processes away from legacy developer database recovery without changing runtime behavior. Refs Gentleman-Programming#1693. Risk: item 3 (shared hook contracts); item 5 (integration).
258e7a3
🔗 Linked Issue
Closes #1693
Lifecycle Evidence
dnlrsls.onFailuresetting has not been exercised by the maintainer.🏷️ PR Type
type:bug— Bug fixtype:feature— New featuretype:question— Question requiring tracked worktype:docs— Documentation onlytype:refactor— Code refactoring (no behavior change)type:chore— Maintenance, dependencies, toolingtype:breaking-change— Breaking change📝 Summary
SessionStarthook ("args": ["hook", "claude-bind-session"], on startup, resume, clear and fork) records host process → session under<data dir>/host-sessions/<pid>.json. The record is keyed by PID plus process start time, so a reused PID never matches. Exec form means no shell, so the hook's parent is the Claude process on Linux, macOS and Windows. This was measured; see Notes.session_id,engram mcpfirst looks up its parent's binding. It uses the binding only when the bound session, or its live:resume:Ncontinuation, belongs to the write's project.session_idhandling is unchanged.PreToolUsehook setsonFailure: "block". Supported hook-failure protection requires Claude Code ≥ 2.1.295 and plugin ≥ 0.1.16. A failed SessionStart refresh may retain an old binding; the PID registry alone is not write-time freshness authority.📂 Changes
internal/hostbind/*/proc), on other Unix systems (ps -o lstart) and on Windows (GetProcessTimes)cmd/engram/hook_bind.goengram hook claude-bind-sessioncommand (host =os.Getppid(), prints nothing, always exits 0)cmd/engram/hook_claude.goplugin/claude-code/hooks/hooks.jsoninternal/mcp/mcp.goplugin/claude-code/.claude-plugin/plugin.json,.claude-plugin/marketplace.jsondocs/PLUGINS.md*_test.go/clearrebinding, resume continuations, explicitsession_id, unusable bindings → refusal; bind uses the parent PID; PreToolUse does not publish; hooks.json exec-form entry🧪 Test Plan
Maintainer follow-up (
864210fa,a28142d4)onFailure; focused configuration checks: 4 PASS. Existing hook-command compatibility selection: 8 PASS.go test ./plugin -count=1 -timeout=10m -v: 77 PASS / 1 FAIL / 3 top-level SKIP, plus 2 nested skips. The sole failure (TestClaudeHookResponseValidation/jq16=false/valid_project, CRLF versus LF) was reproduced once on pristine4eeed4a6with byte-identical helper/test files. It remains unfixed and disclosed; this is not an all-green suite or CI waiver.go test ./internal/mcp -count=1 -timeout=10m -v: 402 PASS / 1 FAIL / 1 SKIP. Sole failure: generated contract byte comparison after Windows checkout converted LF to CRLF. Local LF-only normalization preserved the exact committed blob;go test ./internal/mcp -run '^TestAgentContractArtifactCurrent- [x] Focused regression:go test ./internal/hostbind ./internal/mcp` — okgo test ./cmd/engram— ok exceptTestHookSessionConfirmationLatency/*/registration_timeout, which also fails intermittently on a cleanorigin/main(2 of 3 runs on the branch, 1 of 3 onmain, the sameunexpected endpoint: /failure, also in the Codex variant this PR doesn't touch)go veton the touched packages;go test ./plugin/...and./internal/setuppass;make deadcode-checkpasses;GOOS=windows go build ./...andGOOS=darwin go build ./...passengram, and a fake Messages API.host-sessions/<pid>.jsonwas written with the PID and start time of theclaudeprocess that is the MCP server's parent, and with the session's ID. Bindings of exited hosts are pruned./clearis covered too: one headless process gets prompt,/clearand prompt over--input-format stream-json. On all 6 combinations the exec-form SessionStart (source=clear, new session ID) is a direct child of the MCP server's parent, and the MCP server is not restarted (run 39384361663).✅ Contributor Checklist
Closes #1693).type:*label:type:bug.Co-Authored-Bytrailers in commits💬 Notes for Reviewers
-count=1 -v` then passed. No generated JSON change is committed; the full suite was not rerun or relabeled green.
864210fa: 24 cases per version, then 12 exact normalized compatibility comparisons + 12 intended no-write refusals. Only generated sync/review metadata and verified physical config-source checkout roots were normalized. Session paths and persisted effects remained exact. Comparator fixture defects were corrected under explicit authorization using saved reports, without rerunning Go or changing product expectations.864210fa; its receipt does not cover the resolver commit. Resolver ASSESS: native medium, escalated high/shared contract item 3,reviewDue=false; required independent verification completed.a28142d4: all six required checks passed, as did lint, Windows setup/cloud-wrapper checks, policy helpers and Obsidian build (functional CI run 39543766702). The non-required issue-auto-assignment job failed with onlyUnable to load or run trusted issue assignment helper.; no specific cause was exposed and no rerun or workflow change was made here. Older label/size runs were cancelled and superseded by successful runs. Performance Ratchet was skipped; CodeRabbit's success does not imply a technical review of this draft.review-353d702609064f0c, targetsha256:b1b3eb3355b2ff8a288c2ec49955b0cafaad6fecb16a74a14726884734c1d3b5). Its informationalR3-prune-publication-racewarning is non-blocking later work, not an open correction. This is separate from the earlier R4 receipt.Original contributor verification (before this follow-up; historical)
go test ./internal/hostbind ./internal/mcp— okgo test ./cmd/engram— ok exceptTestHookSessionConfirmationLatency/*/registration_timeout, which also fails intermittently on a cleanorigin/main(2 of 3 runs on the branch, 1 of 3 onmain, the sameunexpected endpoint: /failure, also in the Codex variant this PR doesn't touch)go veton the touched packages;go test ./plugin/...and./internal/setuppass;make deadcode-checkpasses;GOOS=windows go build ./...andGOOS=darwin go build ./...passengram, and a fake Messages API.host-sessions/<pid>.jsonwas written with the PID and start time of theclaudeprocess that is the MCP server's parent, and with the session's ID. Bindings of exited hosts are pruned./clearis covered too: one headless process gets prompt,/clearand prompt over--input-format stream-json. On all 6 combinations the exec-form SessionStart (source=clear, new session ID) is a direct child of the MCP server's parent, and the MCP server is not restarted (run 39384361663).✅ Contributor Checklist
Closes #1693).type:*label:type:bug.Co-Authored-Bytrailers in commits💬 Notes for Reviewers
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores