doctor: repair auto-memory junction + OS-aware env-contract checks (HARNESS-040) - #576
Merged
Merged
Conversation
…cks (HARNESS-040) Wire `dotf doctor` to detect and repair the per-machine setup drift the no-CI model relies on it to catch. - Add `checkAutoMemoryLink`: verify the Claude auto-memory dir is a vault link so /handoff lands in the single sink, FAIL/--fix-repair when missing, SKIP when no vault source, and WARN (never destroy) when it is a real diverged dir. - memlink: add read-only `Status`; share the cross-OS Claude project-key encoding (`ClaudeProjectKey`/`ClaudeMemoryTarget` map / \ : -> -, fixing the wrong target on Windows) and delete the local `encodeProjectPath`; widen `isLink` to detect junctions (`ModeIrregular`, not `ModeSymlink`, on Go 1.26). - Select the env-contract OS dialect from `GOOS` instead of a hardcoded "linux", ending the false-positive PATH drift the session-start banner showed on Windows; `expandHome` now resolves `$env:USERPROFILE` for the windows dialect. Defers the Hive venv repair (#574) and createLink cmd-quoting robustness (#575). Closes #551
📝 WalkthroughWalkthroughThe PR adds auto-memory link repair to ChangesDoctor repair flow
Sequence Diagram(s)sequenceDiagram
participant "doctor.Run" as doctorRun
participant "checkAutoMemoryLink" as checkAutoMemoryLink
participant "memlink.Status" as memlinkStatus
participant "memlink.Ensure" as memlinkEnsure
doctorRun->>checkAutoMemoryLink: run in the non-quick sweep
checkAutoMemoryLink->>memlinkStatus: classify the vault target
memlinkStatus-->>checkAutoMemoryLink: StateLinked / StateNoSource / StateRealDir / StateRepairable
alt StateRepairable and --fix
checkAutoMemoryLink->>memlinkEnsure: create the link
memlinkEnsure-->>checkAutoMemoryLink: success or failure
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
cli/internal/doctor/checks_contract_os_test.go (1)
12-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winWrap the dialect cases in
t.Runsubtests.
TestContractOSiterates a map but doesn't uset.Run, so a failure reports inside one combined test and map iteration order is non-deterministic. Table-drivent.Runsubtests give per-case isolation and naming.♻️ Table-driven with t.Run
- for goos, want := range map[string]string{ - "windows": "windows", - "linux": "linux", - "darwin": "linux", - "": "linux", - } { - if got := contractOS(&System{GOOS: goos}); got != want { - t.Errorf("contractOS(GOOS=%q) = %q, want %q", goos, got, want) - } - } + for goos, want := range map[string]string{ + "windows": "windows", + "linux": "linux", + "darwin": "linux", + "": "linux", + } { + goos, want := goos, want + t.Run(goos, func(t *testing.T) { + if got := contractOS(&System{GOOS: goos}); got != want { + t.Errorf("contractOS(GOOS=%q) = %q, want %q", goos, got, want) + } + }) + }As per coding guidelines: "Use table-driven tests with t.Run in Go" for
**/*_test.go.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cli/internal/doctor/checks_contract_os_test.go` around lines 12 - 23, `TestContractOS` currently loops over a map without per-case subtests, so update it to use table-driven `t.Run` cases for each GOOS value. Keep the existing assertions in `contractOS` but move each case into a named subtest so failures are isolated and readable, and use stable case definitions instead of relying on map iteration order.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cli/internal/memlink/memlink.go`:
- Around line 99-107: ClaudeProjectKey currently normalizes only slashes,
backslashes, and colons, but Claude Code also replaces dots with hyphens, so
update the replacement logic in ClaudeProjectKey to include "." as a mapped
separator as well. Keep the change localized to the strings.NewReplacer call in
memlink.go so paths like .config or v1.2 resolve to the same per-project key
Claude uses.
---
Nitpick comments:
In `@cli/internal/doctor/checks_contract_os_test.go`:
- Around line 12-23: `TestContractOS` currently loops over a map without
per-case subtests, so update it to use table-driven `t.Run` cases for each GOOS
value. Keep the existing assertions in `contractOS` but move each case into a
named subtest so failures are isolated and readable, and use stable case
definitions instead of relying on map iteration order.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c93e2061-1682-4597-96cf-1a0e4f0c3eaf
📒 Files selected for processing (18)
cli/internal/doctor/checks_automemory.gocli/internal/doctor/checks_automemory_test.gocli/internal/doctor/checks_contract.gocli/internal/doctor/checks_contract_os_test.gocli/internal/doctor/contract.gocli/internal/doctor/doctor.gocli/internal/doctor/fs.gocli/internal/mem/session_start_adapter.gocli/internal/mem/session_start_detect.gocli/internal/mem/session_start_injectors.gocli/internal/mem/session_start_injectors_test.gocli/internal/memlink/memlink.gocli/internal/memlink/memlink_test.godocs/lessons.mdspecs/HARNESS-040-doctor-fix-drift-repair/features.jsonspecs/HARNESS-040-doctor-fix-drift-repair/proposal.mdspecs/HARNESS-040-doctor-fix-drift-repair/tasks.mdspecs/HARNESS-040-doctor-fix-drift-repair/verification.md
💤 Files with no reviewable changes (2)
- cli/internal/mem/session_start_injectors_test.go
- cli/internal/mem/session_start_injectors.go
Comment on lines
+99
to
+107
| // ClaudeProjectKey encodes a working directory into Claude Code's per-project key | ||
| // — the directory name under ~/.claude/projects. Claude maps every path separator | ||
| // AND the Windows drive colon to '-', so `/home/me/proj` and `C:\Users\me\proj` | ||
| // become `-home-me-proj` and `C--Users-me-proj`. The retired shell twin mapped | ||
| // only '/', silently producing the wrong key — and thus the wrong junction target | ||
| // — on Windows (the root cause of the unlinked auto-memory dir, #551). | ||
| func ClaudeProjectKey(cwd string) string { | ||
| return strings.NewReplacer("/", "-", `\`, "-", ":", "-").Replace(cwd) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major
🧩 Analysis chain
🌐 Web query:
Does Claude Code encode ~/.claude/projects directory keys by replacing dots (.) with hyphens in addition to path separators?
💡 Result:
Yes, Claude Code encodes project directory paths stored in ~/.claude/projects/ by replacing dots (.) with hyphens (-), in addition to replacing path separators (such as / and ), underscores, and other special characters [1][2][3][4]. This encoding behavior is part of how Claude Code generates unique storage keys for projects [5][6]. Because the encoding process is lossy—meaning it collapses different characters (like dots, slashes, and dashes) into the same hyphen character—it can create directory name collisions where distinct project paths map to the same folder in ~/.claude/projects/ [1][7][5][6]. Users have reported that this behavior can lead to cross-project session contamination, where different projects inadvertently share memory, conventions, and session history [5][6].
Citations:
- 1: Path encoding collision causes session loss on directory reorganization anthropics/claude-code#21085
- 2: fix: decode dots and dashes in encoded project paths vtemian/claude-notes#14
- 3: fix(hooks): encode dots and the drive colon in worktree transcript resolution Yeachan-Heo/oh-my-claudecode#3300
- 4: slopus/happy@dff0eb1
- 5: [BUG] Project memory directory naming can collide when paths differ only by / vs - anthropics/claude-code#29471
- 6: [BUG] Lossy path normalization causes cross-project session contamination anthropics/claude-code#30244
- 7: [BUG] Lossy project directory path encoding makes decoding impossible anthropics/claude-code#9221
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant file and nearby tests/usages.
git ls-files 'cli/internal/memlink/*' 'cli/**/memlink*' | sed 's#^`#FILE` #'
printf '\n---\n'
ast-grep outline cli/internal/memlink/memlink.go --view expanded || true
printf '\n---\n'
sed -n '1,180p' cli/internal/memlink/memlink.go
printf '\n---\n'
rg -n "ClaudeProjectKey|projects/|\\.claude" cli -SRepository: mlorentedev/dotfiles
Length of output: 14036
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the relevant file and nearby tests/usages.
git ls-files 'cli/internal/memlink/*' 'cli/**/memlink*' | sed 's#^`#FILE` #'
printf '\n---\n'
ast-grep outline cli/internal/memlink/memlink.go --view expanded || true
printf '\n---\n'
sed -n '1,180p' cli/internal/memlink/memlink.go
printf '\n---\n'
rg -n "ClaudeProjectKey|projects/|\\.claude" cli -SRepository: mlorentedev/dotfiles
Length of output: 14036
Add . to ClaudeProjectKey
Claude Code also turns dots into hyphens, so paths like .config or v1.2 still map to the wrong ~/.claude/projects/<key> directory unless this replacer includes . too.
🛠️ Possible fix
- return strings.NewReplacer("/", "-", `\`, "-", ":", "-").Replace(cwd)
+ return strings.NewReplacer("/", "-", `\`, "-", ":", "-", ".", "-").Replace(cwd)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // ClaudeProjectKey encodes a working directory into Claude Code's per-project key | |
| // — the directory name under ~/.claude/projects. Claude maps every path separator | |
| // AND the Windows drive colon to '-', so `/home/me/proj` and `C:\Users\me\proj` | |
| // become `-home-me-proj` and `C--Users-me-proj`. The retired shell twin mapped | |
| // only '/', silently producing the wrong key — and thus the wrong junction target | |
| // — on Windows (the root cause of the unlinked auto-memory dir, #551). | |
| func ClaudeProjectKey(cwd string) string { | |
| return strings.NewReplacer("/", "-", `\`, "-", ":", "-").Replace(cwd) | |
| } | |
| // ClaudeProjectKey encodes a working directory into Claude Code's per-project key | |
| // — the directory name under ~/.claude/projects. Claude maps every path separator | |
| // AND the Windows drive colon to '-', so `/home/me/proj` and `C:\Users\me\proj` | |
| // become `-home-me-proj` and `C--Users-me-proj`. The retired shell twin mapped | |
| // only '/', silently producing the wrong key — and thus the wrong junction target | |
| // — on Windows (the root cause of the unlinked auto-memory dir, `#551`). | |
| func ClaudeProjectKey(cwd string) string { | |
| return strings.NewReplacer("/", "-", `\`, "-", ":", "-", ".", "-").Replace(cwd) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@cli/internal/memlink/memlink.go` around lines 99 - 107, ClaudeProjectKey
currently normalizes only slashes, backslashes, and colons, but Claude Code also
replaces dots with hyphens, so update the replacement logic in ClaudeProjectKey
to include "." as a mapped separator as well. Keep the change localized to the
strings.NewReplacer call in memlink.go so paths like .config or v1.2 resolve to
the same per-project key Claude uses.
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.
Wires
dotf doctorto detect and repair two classes of per-machine setup drift the no-CI model relies on it to catch. Scoped to parts 1 + 2 of #551; the cross-language Hive-venv repair and amemlinkrobustness gap are split out (see Deferred).What changed
Part 1 — auto-memory ↔ vault junction. New
checkAutoMemoryLink: verifies the Claude auto-memory dir is a link into the knowledge vault (so/handofflands the continuity block in the single sink, not in.claude). It consumes the mergedmemlinkprimitive, so doctor and the session-start adapter compute an identical target on every OS.--fix, idempotent — mirrorscheckVaultHooks.--fix—memlinkrefuses to overwrite real data by contract.Two latent Windows bugs fixed in
memlink(the root cause of the never-created junction here):-(C:\Users\me\proj→C--Users-me-proj); the ported encoder mapped only/. Now a single cross-OS encoding (ClaudeProjectKey/ClaudeMemoryTarget) shared by both callers; the localmem.encodeProjectPathis deleted.mklink /Jjunction surfaces viaos.LstatasModeIrregular, notModeSymlink, on Go 1.26 (verified empirically) —isLinkwidened accordingly.Part 2 — OS-aware env-contract checks.
checkContractEnvVars/checkContractPathselected thelinuxdialect unconditionally, so on Windows they checked Linux PATH entries/defaults and emitted false-positive drift every session. They now pick the dialect fromSystem.GOOS;expandHomealso resolves$env:USERPROFILE. Linux output is unchanged.Test plan
go test ./internal/doctor/ ./internal/mem/ ./internal/memlink/→ green (run on a Windows host, so the Windows branches are exercised natively).TestCheckAutoMemoryLink(FAIL/SKIP/WARN/--fix+idempotent),TestContractOS,TestCheckContractPath_Dialects,TestCheckContractEnvVars_WindowsDialect,TestClaudeProjectKey/TestClaudeMemoryTarget/TestStatus.(2 checks, all ok)), anddotf doctorWARNs on the real diverged auto-memory dir without touching its files.TestEmbeddedTemplatesMatchVaultfailures in the module are pre-existing (tracked by Re-vendor vault templates into the CLI (TestEmbeddedTemplatesMatchVault drift) #461), confirmed by re-running them with this branch stashed.Deferred (ticketed)
doctor --fixrepairs the Hive MCP Python venv (cross-language).memlink.createLinkrobustness for path components with barecmddelimiters.Spec
specs/HARNESS-040-doctor-fix-drift-repair/(proposal + tasks + verification + features.json).Closes #551
Summary by CodeRabbit
New Features
Bug Fixes