fix(claude): bind memory from the host, ending the MEMORY.md index clobber (ADR-0004) - #81
Open
dlovell wants to merge 1 commit into
Open
fix(claude): bind memory from the host, ending the MEMORY.md index clobber (ADR-0004)#81dlovell wants to merge 1 commit into
dlovell wants to merge 1 commit into
Conversation
…obber (ADR-0004) setup-claude's per-entry copytree overwrote the container's MEMORY.md index with the host copy on every entry, orphaning container-written memories (on disk, absent from every session's context — observed live: project_agent_workflow_rubric.md). Memory now follows the transcript pattern: bind-mounted from the host, never copied. - docker-compose.yml: bind ~/.claude/memory (global) and a per-project store keyed by the MAIN checkout path (shared across all worktree containers and host sessions in the main checkout), nested inside the DEV_CLAUDE_LOGS transcript bind. - dev/devcontainer: DEV_CLAUDE_MEMORY / DEV_CLAUDE_GLOBAL_MEMORY exports; ensure_claude_memory pre-creates sources and migrates legacy $DEV_CLAUDE_LOGS/memory before the nested bind would shadow it. - lib/claude-memory.sh: index union by link target (container line wins a collision), index-line synthesis for orphaned memory files, one-shot self-healing migration that cannot resurrect deleted memories. - setup-claude.py: copy paths demoted to a mountless fallback — samefile no-op when the bind is present; otherwise a non-clobbering merge holding the invariant that every memory file has an index line. - tests/test-claude-memory.sh: drift guard at both layers — a container-written index line survives a reseed; orphans get index lines; the samefile guard keeps the bind path copy-free. Skills (executable instructions, unlike inert memory facts) are deliberately NOT bound; ADR-0004 records the options and recommends the ro-bind + git-tracked project tier route for a follow-up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01REnm9VWsgaYGQuL9E36s3r
dlovell
added a commit
that referenced
this pull request
Aug 2, 2026
…rule Status -> Accepted. Annotations compose (tool: x; test: y), Decision 2's mutation record is named (unguarded) by the taxonomy's own terms, and the decision-time hand counts are kept but reconciled against the re-runnable baseline posted on #89: a recorded 119-line comment sweep (was a curated 58), "2 of 25" verified under the strict criterion, and three mutation spot-checks that stayed green through the full suite. CLAUDE.md lands the rule in the same PR: the invariant header adopts the vocabulary (legacy dash and bare test citations defined), Conventions gains the guard rule, and the ADR list gains 0005 (0004 reserved by #81). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019cdwFBF6jioKa465vQVGyg
This was referenced Aug 3, 2026
Contributor
Author
|
Heads-up on an interaction this PR doesn't currently reference. This PR adds two more binds under
#115 is the Separately, #113 makes the ancestor walk bind-aware, which this PR needs: the per-project memory bind's first ancestor is the transcript bind mount point, so without #113 the walk would chown it. #113 also has the deeper protection (a path under a bind, not just the mount point itself), which the Worth either landing #115's fix first, or noting the interaction in this PR's body so the next reader sees it. Also flagging one premise in this PR's compose comment that has no guard and no citation: "compose orders nested binds by target depth". That is a claim about the engine rather than about a file, so nothing in the tree can catch it drifting — see #116, which is about exactly this class (facts about the merged mount graph with no reader). |
This was referenced Aug 4, 2026
dlovell
added a commit
that referenced
this pull request
Aug 4, 2026
Review of #120 found two ways the guard could be evaded, both verified here by mutation against the pushed head (`bash tests/run-all` -> exit 0 with each applied, i.e. both were genuinely green): 1. `projects/<name>/host-mounts.txt` is a SECOND committed bind channel. `lib/host-mounts.sh` turns those list files into a generated override that lands in the same `services.app.volumes`, so a committed `~/.claude/evil:/home/vscode/.claude/evil` — a bind under the very volume #115's `chown -R` is rooted at — was invisible to a guard that parsed compose files only. The previous revision's own comment named this blind spot for compose.override.yml and left the identical one in the same overlay directory, one file over. host_mount_mounts() now reads them, mirroring read_list()'s comment/blank handling and host-mounts.sh:25's leading-~ expansion, with a synthetic-line assertion so the parser cannot be green by reading nothing. `host-mounts.local.txt` stays out of scope: gitignored per-developer state, not a repo fact. 2. The reader discarded mount options, so ADR-0001's other recorded half — docs/adr/0001:46, "the store is still reachable READ-ONLY via the existing .claude-host mount (host ~/.claude, which includes credentials/)" — could not be checked by anything. Dropping `:ro` makes the host credential store shared-writable from every container, a regression squarely inside the ADR this suite claims to enforce, and it was green. Options are now a fourth column and both `.claude-host*` mounts are pinned `ro`. Also from review: the cross-check gated on `docker compose version` while using `config --format=json` (which arrived ~v2.24), so an older CLI fed empty output to json.load and `set -e` killed the suite mid-run — before the assertion written for exactly that case could be reached. Now it distinguishes the two outcomes that differ in meaning: no docker CLI SKIPs, docker present but unrenderable FAILs, because a cross-check that silently stops running is the failure mode being guarded against. The JSON parse is tolerant so it reports rather than aborts. Nits: tmpfs targets were split before interpolation, inconsistent with the interpolate-then-split lesson the same file documents; the `unguarded` annotation now carries ADR-0005's mandatory colon-form reason; pyyaml is noted as this suite's first non-stdlib dependency (fail-closed, CI has it). 12 -> 14 assertions, and the CLAUDE.md invariant no longer overstates what the reader covers: it names both channels and the options column. Refs #116, #115, #81 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MMRkcVZwXpwBqjMY4bSehH
dlovell
pushed a commit
that referenced
this pull request
Aug 4, 2026
…h time
Review round 2 on this PR found the guard still evadable through two committed
channels, both verified green with a `~/.claude` bind injected:
`defaults/compose.override.yml` (a real overlay — DEV_PROJECT_DIR falls back to
it) and `nix/base/compose.nix-base.yml` (appended by dc() on the nix route).
That is the third leak from the same hand-maintained list: rev 1 read
docker-compose.yml only, review found host-mounts.txt, review found these. Each
fix extended the list by hand and the list leaked again. So this one does not
extend it — `committed_compose_files` derives from `git ls-files`, and a
compose file added anywhere in the repo is scanned the day it lands. The
anti-vacuity assertions name the members whose absence caused a real gap, so a
regex that stops matching them cannot pass as "nothing to scan".
Also from review, each verified by mutation:
- host_mount_mounts split on `:` BEFORE interpolating — the exact hazard
compose_mounts documents, reintroduced in the fix for the previous channel
gap. Fixing that exposed a second, deeper error in both readers: an unset
`${VAR:-default}` was expanded to a stub path rather than to DEFAULT, which
is what compose does. `/home/vscode/.clau${X:-de}/two` really mounts under
~/.claude and the stub put it under /home/vscode/.clau/stub/x/, matching no
prefix. Both readers now model compose's actual substitution.
- a colon-free host-mounts line is forwarded verbatim and read by compose as an
anonymous volume at that path; the parser skipped it as malformed.
- the "docker present but unrenderable -> FAIL" branch was unreachable:
pipefail carried docker's status out of the command substitution and set -e
killed the suite at the assignment, before the assertion written for that
case. The previous revision's comment claimed this was fixed; the tolerant
JSON parse only covered exit-0-with-garbage.
The same "fails with no diagnostic" shape turned up twice more while fixing it
and is handled the same way: `git ls-files` failing now yields a clean FAIL
naming the cause instead of aborting at the assignment, and a malformed compose
file names itself instead of raising a bare traceback.
CLAUDE.md's invariant said the graph has TWO committed channels, which was
false; it now says the list is derived and why. The nesting rule was also
stated unqualified while the guard reads only the base file's volumes — binds
under overlay-declared volumes are the same #115 hazard and are now recorded
`unguarded:` rather than implied covered.
14 -> 19 assertions. Mutation record extended to nine, with a note on method:
measure in a fresh copy per mutation, because `cp -a` of this worktree carries
a .git pointing at another worktree's gitdir, so `git checkout` does not
restore and mutations accumulate.
Refs #116, #115, #81
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MMRkcVZwXpwBqjMY4bSehH
dlovell
pushed a commit
that referenced
this pull request
Aug 4, 2026
…p model
Review round 3 found three more evasions, each verified green on the head SHA
that CI had already passed. All three are the same class the two prior rounds
found: the reader models compose, and the model was narrower than compose.
1. ADR-0001 was enforced BY NAME. The check keyed on the substring
"credential" appearing in a path, so a writable
`${HOME}/.claude:/home/vscode/.claude-rw` — the host store, which contains
.credentials.json — passed every assertion simply by choosing a target that
did not say "credential". The rule the ADR actually states is about the
STORE, so the check is now keyed on the mount SOURCE: nothing whose source
is the host home or under it may be writable, wherever it lands and whatever
it is called. One declared exception, ADR-0001's own: the transcript
carve-out, which must be writable and addresses a per-container leaf. The
check also now reads the DERIVED file set rather than the base file alone,
because an overlay could mount the store at a target outside ~/.claude and
evade both this and the coverage assertion.
2. The reader read `services.app` only, while `dc up -d` starts every service
in the merged config. A sidecar service in a project overlay bind-mounting
the host store under ~/.claude passed 19/19. It now reads every service, and
also top-level named volumes declared with `driver_opts: {o: bind, device:
...}` — a bind in volume clothing, and another way the store can be mounted
without the word "bind" appearing.
3. The interpolation model still diverged from compose on `${VAR-def}`
(colon-less default) and unbraced `$VAR`; both render under ~/.claude for
real compose while the reader left them literal. That is the third instance
of this class, one spelling over from the previous fix — and the model had
by then been copied into both readers, so each fix had to be applied twice.
There is now exactly ONE interpolation definition, shared.
Also: CLAUDE.md described the derivation as `*compose*.y*ml`, which reads as
substring-anywhere while the code anchors `compose` at a path-component start
(`foo-compose.yml` would not be scanned); the invariant now says what the regex
does. The header's "what is asserted" list and mutation count had drifted from
the file and are rewritten, and the method note no longer asserts a cause for
the earlier miscounts that does not reproduce.
Mutation record 9 -> 12 entries, all measured one-copy-per-mutation.
Refs #116, #115, #81
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MMRkcVZwXpwBqjMY4bSehH
dlovell
added a commit
that referenced
this pull request
Aug 6, 2026
…ch-commit history (#122) Three commits, one story: what an audit is required to leave behind, and what the auditor is able to read. ## 1. `810dbad` — ADR-0006 amendment: committing an audit report is optional Reverses the "audits get a baseline" requirement. Closure is untouched — every surviving shape is still filed, landed, or recorded as an accepted `unguarded:` invariant — but the report itself is no longer mandatory. Why: the report is the second copy and the weaker one. A finding's durable form is an issue, a landed guard, or an `unguarded:` invariant, and all three live where the next reader already looks; a committed report restates that in prose nothing keeps true. It is the restate-over-derive shape ADR-0005 rejects, applied to the auditor's own output. A mandatory artifact also taxes the run that was already easiest to skip — the failure mode is not a missing file, it is a skipped audit. The cost is accepted explicitly rather than waved off: with no guaranteed prior report there is no guaranteed baseline, so measurements may not form a time series and a checked-and-dropped list may be re-litigated. Recorded in `unguarded:` form with a revisit trigger. Existing reports and the directory stay. ## 2. `934e999` — the auditor reads branch commits; the vocabulary is derived, not named **Branch history as an input.** `main` is squash-merged, so a hunk fixed three times on a branch lands as one clean commit and the fix-the-fix chain is unreachable from `git log`. Both audit agents excluded that history — `pr-reviewer` reviews the net `base...head` diff and deliberately anchors its history probe to `origin/main`, and the auditor is repo-scoped — so nobody read it. This PR's own subject demonstrates the loss. #90, which created these agents, contains: ``` 1254335 fix(agents): repair the log command; drop the ADR-0005 hard dependency ab9e77d fix(agents): anchor the repeat-offender log to origin/main; tidy audit ``` Two corrections to the same three lines of prose. On `main` that is one commit, `ba15520`. `gh pr view <n> --json commits` still returns the pre-squash sequence after merge, so this needs no pre-merge window. Scoped to PRs the other probes already implicated, so it stays one API call per candidate rather than per PR in the window. The squash fact is written as a check, not an assertion, so a change of merge strategy is noticed instead of assumed. Also adds "a set of merged PRs" to the scope list, making a retrospective over recent reviews a first-class scope. **Vocabulary drift.** #100 (`04e9eee`) typed the invariant ledger into the ADR-0005 vocabulary and touched `CLAUDE.md` only. `structural-auditor.md` reads that ledger and still named `(—)` in two places — a marker with zero remaining instances — so an agent following the instruction searched for a token that no longer exists and would have skipped every accepted-risk invariant in the repo. Silent under-coverage, in the agent whose job is finding unguarded couplings. Fixed by removing the copy rather than guarding it: the cross-section bullet names the categories and defers to the ledger's own header for the markers. A legacy untyped citation is now itself a finding, matching "type it when you touch it". **`pr-reviewer`** gains a note that the `origin/main` anchor also puts the branch's own commits out of scope, and that this is a handoff to the auditor. Without it the exclusion reads as an oversight and the next editor removes the anchor, restoring the self-trigger `ab9e77de` fixed. ## 3. `0d35e4c` — stop enumerating the audit reports by name The amendment named the two files in `docs/audits/` as "the existing reports". #112 commits a third under the now-optional rule, so that list goes stale on merge — a second copy of a directory listing, which is the shape the amendment itself argues against. De-enumerated in all three places with the reason stated inline. ## Verification - `pre-commit run --files` on all three files: clean. The `check-gitignore-agents` probe passes, confirming the agent files can actually be staged (ADR-0003). - The `gh pr view` command was **run as written** before being embedded. The two commands this file has shipped previously were both wrong on first landing; embedding a third unverified one would have repeated the exact failure this PR documents. - No test covers `.claude/agents/`, which is the point of the issue below. ## Related - #121 — the finding this work produced, filed rather than fixed here: `.claude/agents/*.md` ships shell commands and ledger vocabulary with no guard, and both halves have now broken once. This PR fixes the two symptoms; the mechanism is open. Deliberately **not** `Closes` — and #121 argues accept-with-a-reason is a legitimate disposal. - Merge-tested against all seven open PRs. Clean against `main`, #112, #113, #110, #109, #108. - **#117** conflicts with this branch only (it is clean against `main`): it rewrites the "Guards follow ADR-0005" bullet while `810dbad` extends the "A structural audit…" bullet directly beneath it. Adjacent lines, independent content — whoever merges second keeps both. - **#81** already conflicts with `main` independently of this branch, but note it edits the *same sentence* `810dbad` does: it replaces "ADR-0004 is reserved by #81" with the real entry while this adds ADR-0006. Both additions must survive the resolution; taking either side wholesale drops an ADR from the index. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_011KsnnuPRJW5sUPppm3RSRo --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: t <t@t>
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.
Defect (observed live, not hypothetical)
setup-claude.pymirrored memory host→container withcopytree(..., dirs_exist_ok=True)on every entry (setup_clauderuns unconditionally in theuppath). Individual memory files rarely collide —MEMORY.mdcollides always, so container-written index edits were destroyed on the next entry while the memory files they pointed at survived. Verified in a running container:project_agent_workflow_rubric.md(written+indexed Aug 1) is on disk with no index line; the containerMEMORY.mdmtime is the host file's Jul 20 mtime (copy2preserves it). Orphaned memories are invisible to every session — no issue was open for this (checked), which is itself the failure mode this PR fixes.Fix: treat memory like transcripts — bind, don't copy (ADR-0004)
~/.claude/memory(global) and a per-project store at~/.claude/projects/<main-checkout-key>/memory, nested inside theDEV_CLAUDE_LOGStranscript bind. Deleting the copy removes the defect class entirely; memory now survivesclean/resetand is shared across all worktree containers and host sessions in the main checkout.lib/claude-memory.sh, run host-side before compose mounts): the container project dir is already a bind, so the nested mount would silently shadow existing pre-ADR-0004 memories.migrate_project_memorymerges them into the shared store first —MEMORY.mdunioned by link target (container line wins), merged files drained so deleted memories can't resurrect, interrupted runs self-heal. Index-line synthesis (from frontmatter) restores the every memory file has an index line invariant — including repairing the currently-orphaned rubric memory on firstdevcontainer upafter this lands.copy_global_memory/ thesetup_project_settingsmemory branch remain as the mountless fallback (compose run withoutdev/devcontainer). They nowmerge_memory: samefile no-op when the bind is present; otherwise union — deliberately not the half-fix "skip MEMORY.md if it exists", which reproduces the orphan bug mirrored (a new host memory arriving with no index line).tests/test-claude-memory.shpins at both layers that a container-written index line survives a reseed, orphans get synthesized index lines, and the samefile guard keeps the bind path copy-free. New DEV vars added to the harness clean-slate list..claude/skills/project tier, mirroring ADR-0003) for a follow-up decision.Design decisions a reviewer may want to weigh in on
Test plan
bash tests/run-all— 23 suites green (including newtest-claude-memory.sh, 16 assertions)docker compose configrenders both binds at the right nesting🤖 Generated with Claude Code
https://claude.ai/code/session_01REnm9VWsgaYGQuL9E36s3r