Repository navigation
fix(harness): confine per-session state to one injectable state dir - #977
Merged
Merged
Conversation
Every deferred and injected tool in session f6341932 answered "engrams hook bridge unavailable": the hook bridge's round trip found no socket, because the session's own `/workspace/.engrams/hook.sock` had been unlinked while its harness kept a listener on the dead inode. The suite did it. `run_engine` unlinks and rebinds the hook socket at a FIXED absolute path, and `write_hook_settings` / `write_mcp_config` write beside it, but only the socket was overridable and `test_cli` left the override `None`. On a laptop or a CI runner `/workspace/.engrams` does not exist, so the bind fails and the tests pass degraded — invisible. Inside a dogfooding session that directory is the LIVE session's state, so `just check` unlinked its sockets (twice: once to clear a "stale" bind, once through `SockGuard::drop`), repointed its claude-settings.json at the nextest binary, and overwrote its MCP config and Bash cwd tracker. Ordinary built-ins kept working, which is why the session otherwise looked healthy. Partial overrides were the bug, so there is now ONE root. `engram_harness_sdk::state::StateDir` owns the directory and the name of every file in it; both harnesses derive all of their per-session paths from `Cli::state_dir`, which has no fallback. Production keeps the fixed in-VM default, and `test_cli` injects a fresh temp dir per test — as a required struct field, so a new test cannot silently reach production. This retires four `Option` seams in the claude harness (hook_sock_path, mcp_sock_path, session_id_file, mode_stamp_file) and three in codex, plus `mode_stamp::MODE_STAMP_FILE`. `ENGRAM_STATE_DIR` carries the root to each re-invoked bridge child, so the Bash cwd tracker stays coherent across the process boundary. The regression test runs an engine turn and asserts all seven per-session files land in the injected root and that the root holds nothing else — a new state file has to be named in `StateDir`. Also fixed, found while running the suite in a live session: both fake-claude fakes recorded one invocation per line, but argv carries `--append-system-prompt`, whose ambient value is multi-line in a real session, so the line count and the last-line read were wrong. And the codex tests shared /tmp/thread-id and /tmp/parked-calls.json across the whole suite. The hazard landed in #381 (ADR 0054, the fixed-path bind), was half-covered by #400 (a socket seam for the one hook-firing test), and was widened by #681 (ADR 0089, mcp.sock + mcp-config.json). It became a routine session-killer once papercut (#689) put a sync tool in every manifest. Co-Authored-By: Claude <noreply@anthropic.com>
The dev Process backend has no guest, so every sandbox it spawns on a host shared the harness's default state directory — the fixed in-VM `/workspace/.engrams`. Two dev sandboxes therefore fought over one hook socket (B's bind unlinks A's), and a dev stack running inside an engrams session aimed both at that session's own live state. On a laptop, where the path does not exist at all, the binds simply failed and deferred tools never worked. Root it in the sandbox dir instead — the same directory snapshot/restore carries — leaving an explicit `ENGRAM_STATE_DIR` untouched. Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
Author
|
✅ engrams review — complete. 0 findings posted. · View details |
nikhilunni
approved these changes
Aug 3, 2026
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
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.
Why
In session
f6341932, everypapercutcall came back as a denial:That string has exactly one source: the
PreToolUsehook bridge, when itsround trip to the harness main process returns nothing
(
crates/engram-harness-claude/src/main.rs, theNonearm). The socketenv var is always set at spawn, so the round trip fails only when the
socket FILE is gone — and something inside that session had deleted it.
That something was the harness's own test suite.
run_engineunlinks andrebinds its hook socket at a FIXED absolute path, and
write_hook_settings/
write_mcp_configwrite to fixed absolute paths beside it:test_cli()left the socket overrideNone, so 14 of the 15run_enginetests bound the production path. On a laptop or a CI runner
/workspace/.engramsdoes not exist, the bind fails, and the tests passdegraded — invisible. Inside a dogfooding session that directory is the
LIVE session's state, so
just check(orcargo nextest run -p engram-harness-claude) did this to the session running it:remove_file("/workspace/.engrams/hook.sock")— the live harness keepsits listener on an unlinked inode, so the hook can never reach it again.
SockGuard::dropunlinks the path once more whenthe test's engine returns. Same for
mcp.sock.claude-settings.jsonrewritten to point the hook at the nextestbinary,
mcp-config.jsonrewritten or deleted,bash-cwdoverwritten.From then on every deferred and injected tool in that session —
papercut,AskUserQuestion,ExitPlanMode,browser_view— answers "engrams hookbridge unavailable". Ordinary built-ins are unaffected (the bridge allows
them locally), which is why the session otherwise looked healthy.
When: #381 (
b0d2bd97, 2026-06-23, ADR 0054) introduced the fixed-pathbind inside a test-reachable path. #400 (
1d0a30ef, same day) added thehook_sock_pathseam — but only for the one test that fires hooks, whichis why the hazard stayed hidden. #681 (
05b2f241, 2026-07-15, ADR 0089)added
mcp.sockandmcp-config.jsonin the same shape. The symptombecame routine once
papercut(#689) shipped in every session's manifest:the next tool call after a test run fails. The tests knew, obliquely — the
shared fake claude omits
session_idfrom itsinitline with the comment"which would touch /workspace".
What
One root, one seam.
engram_harness_sdk::state::StateDirowns the statedirectory and the name of every file in it; both harnesses derive all of
their per-session paths from
Cli::state_dir, which has no fallback:/workspace/.engrams).directory across every sandbox on the host, now roots it in the sandbox
dir via
ENGRAM_STATE_DIR.test_cli()injects a fresh temp dir per test. It is a required field ofa struct literal, so a new test cannot silently fall back to production.
This retires four
Optiontest seams in the claude harness and three incodex (
hook_sock_path,mcp_sock_path,session_id_file,mode_stamp_file,thread_id_file,parked_calls_file) — partialoverrides were the bug.
Also here, both noticed while fixing the above:
carries
--append-system-prompt, whose ambient value is multi-line in areal session — the line count and the last-line read were both wrong (a
suite-only failure in any session with a system prompt set). They now
fold newlines out of the record.
/tmp/thread-idand/tmp/parked-calls.jsonacrossthe suite; each test now has its own root.
Verification
cargo nextest run -p engram-harness-claude -p engram-harness-sdk: 103passed, and the live session's
/workspace/.engramscame outbyte-identical with the same socket inodes — the suite no longer touches
it at all. Before this change the same run would have unlinked the socket
under the session executing it.
every_per_session_file_lands_in_the_injected_state_dirruns an engineturn and asserts all seven per-session files appear in the injected root,
and that the root holds nothing else — a new state file has to be named
in
StateDir(and there) rather than written to a path of its own.cargo fmt --check,cargo hakari verify,cargo clippy --workspace --all-targetsclean.engram-harness-codextests cannot run in this session (its fakeapp-server needs
jq, absent from the dev image — the same three testsfail identically on
main); the codex lane in CI covers them.🤖 Generated with Claude Code