fix(mem): own the Claude project-key encoding in Go so the Windows twins can't drift - #739
Conversation
…ins can't drift Two PowerShell sites re-implemented the auto-memory junction key and deleted the drive ':' instead of mapping it to '-', producing C-Users-... where Claude Code and memlink.ClaudeProjectKey use C--Users-.... setup-windows.ps1 deposited junctions Claude never reads; knowledge-crystallize.ps1 warned "No MEMORY.md found" for every project. vault-maintenance-weekly.ps1 also passed the POSIX --all, which bound positionally to $ProjectDir and left [switch]$All false, so the weekly task never fanned out. - add `dotf mem project-key <path>` exposing memlink.ClaudeProjectKey as the one encoding source - setup-windows.ps1 + knowledge-crystallize.ps1 resolve the key via a shared utils.ps1 helper (dotf-first, corrected pure fallback); fix the decoder regex - vault-maintenance-weekly.ps1: --all -> -All - Pester guard binds the PS encoder to Go's known-good keys Closes #689
|
Warning Review limit reached
Next review available in: 45 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds ChangesWindows project-key alignment
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 3
🧹 Nitpick comments (1)
cli/internal/cmd/mem_test.go (1)
61-78: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRefactor to use a standard table-driven test with a slice of structs.
Using a map for test cases introduces non-deterministic execution order, and running them in the same scope makes it harder to isolate failures. Consider using a slice of structs with
t.Runfor better readability and standard Go testing practices.♻️ Proposed refactor
func TestMemProjectKey(t *testing.T) { - for in, want := range map[string]string{ - `C:\Users\me\p`: "C--Users-me-p", - "/home/me/p": "-home-me-p", - } { - cmd := newMemCmd() - var out bytes.Buffer - cmd.SetArgs([]string{"project-key", in}) - cmd.SetOut(&out) - cmd.SetErr(io.Discard) - if err := cmd.Execute(); err != nil { - t.Fatalf("project-key %q: unexpected error %v", in, err) - } - if got := strings.TrimSpace(out.String()); got != want { - t.Errorf("project-key %q = %q, want %q", in, got, want) - } - } + tests := []struct { + name string + in string + want string + }{ + {"windows_path", `C:\Users\me\p`, "C--Users-me-p"}, + {"unix_path", "/home/me/p", "-home-me-p"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + cmd := newMemCmd() + var out bytes.Buffer + cmd.SetArgs([]string{"project-key", tc.in}) + cmd.SetOut(&out) + cmd.SetErr(io.Discard) + if err := cmd.Execute(); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got := strings.TrimSpace(out.String()); got != tc.want { + t.Errorf("got %q, want %q", got, tc.want) + } + }) + } }🤖 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/cmd/mem_test.go` around lines 61 - 78, Refactor TestMemProjectKey to define cases as a slice of structs containing input and expected output, then iterate with t.Run using a descriptive case name. Create and configure a fresh command and output buffer inside each subtest so failures remain isolated while preserving the existing assertions.
🤖 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 `@specs/BUG-031-windows-project-key/proposal.md`:
- Around line 49-56: The fallback encoding described in the proposal omits the
slash replacement used by ClaudeProjectKey. Update the fallback example to
replace /, \, and : consistently, or reference Get-ClaudeProjectKeyEncoded as
the single source of truth; keep the dotf-based path unchanged.
In `@specs/BUG-031-windows-project-key/tasks.md`:
- Around line 40-46: Resolve the contradiction in the “Machine-readable
features” section: either add the omitted features.json artifact or update the
“See sibling features.json” reference to its actual canonical location. Keep the
surrounding statement consistent with the chosen artifact location and omission
policy.
In `@specs/BUG-031-windows-project-key/verification.md`:
- Around line 26-39: Update the PSScriptAnalyzer scope statement in the
verification checklist to match the five files listed in the test status, or
explicitly distinguish the two CI-linted files from the three additionally
analyzed files. Ensure the verification record is unambiguous without changing
the reported results.
---
Nitpick comments:
In `@cli/internal/cmd/mem_test.go`:
- Around line 61-78: Refactor TestMemProjectKey to define cases as a slice of
structs containing input and expected output, then iterate with t.Run using a
descriptive case name. Create and configure a fresh command and output buffer
inside each subtest so failures remain isolated while preserving the existing
assertions.
🪄 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: ba5666a6-78e1-45ce-bd5d-811a8e788772
📒 Files selected for processing (11)
cli/.gitignorecli/internal/cmd/mem.gocli/internal/cmd/mem_test.goscripts/knowledge-crystallize.ps1scripts/utils.ps1scripts/vault-maintenance-weekly.ps1setup-windows.ps1specs/BUG-031-windows-project-key/proposal.mdspecs/BUG-031-windows-project-key/tasks.mdspecs/BUG-031-windows-project-key/verification.mdtests/claude-project-key.Tests.ps1
| - **Go owns the datum.** New `dotf mem project-key <path>` subcommand prints | ||
| `memlink.ClaudeProjectKey(path)`. The encoding now has one implementation. | ||
| - **PowerShell calls Go, with a corrected fallback.** `setup-windows.ps1` and | ||
| `knowledge-crystallize.ps1` obtain the key via `dotf mem project-key`, guarded on | ||
| `dotf` being on PATH (the existing `if (Get-Command dotf ...)` pattern at | ||
| `setup-windows.ps1:605/619/645`). When `dotf` is absent the inline fallback uses | ||
| the **corrected** encoding `.Replace('\','-').Replace(':','-')` (matches Go), so | ||
| bootstrap never hard-depends on the CLI yet never re-emits the buggy key. |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== proposal excerpt ==\n'
sed -n '40,70p' specs/BUG-031-windows-project-key/proposal.md
printf '\n== find ClaudeProjectKey ==\n'
rg -n "ClaudeProjectKey|project-key|Replace\\(" -S .Repository: mlorentedev/dotfiles
Length of output: 18247
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the encoder and the Windows scripts mentioned in the proposal.
fd -a "ClaudeProjectKey" .
fd -a "setup-windows.ps1" .
fd -a "knowledge-crystallize.ps1" .
fd -a "proposal.md" specs/BUG-031-windows-project-key
printf '\n== encoder references ==\n'
rg -n "ClaudeProjectKey|Replace\\('/'|Replace\\('\\\\'|Replace\\(':'" -S . --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**'
printf '\n== proposal lines ==\n'
sed -n '45,60p' specs/BUG-031-windows-project-key/proposal.mdRepository: mlorentedev/dotfiles
Length of output: 10442
Make the fallback example include / too. ClaudeProjectKey replaces /, \, and :; the proposal’s fallback snippet only shows \ and :. Either add the / replacement or point this line at Get-ClaudeProjectKeyEncoded so it matches the single source of truth.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~54-~54: Possible missing comma found.
Context: ...ndows.ps1:605/619/645). When dotf` is absent the inline fallback uses the **correc...
(AI_HYDRA_LEO_MISSING_COMMA)
🤖 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 `@specs/BUG-031-windows-project-key/proposal.md` around lines 49 - 56, The
fallback encoding described in the proposal omits the slash replacement used by
ClaudeProjectKey. Update the fallback example to replace /, \, and :
consistently, or reference Get-ClaudeProjectKeyEncoded as the single source of
truth; keep the dotf-based path unchanged.
| > `features.json` omitted, matching the current precedent (BUG-029, BUG-030 ship | ||
| > proposal/tasks/verification only). Acceptance criteria map to Go/Pester tests | ||
| > named in `verification.md`. | ||
|
|
||
| ## Machine-readable features | ||
|
|
||
| See sibling `features.json`. Pass-state is harness-gated (agent may not set `passing`). |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove or correct the stale features.json reference.
This section says features.json is intentionally omitted, then instructs readers to “See sibling features.json.” Either include the artifact or replace the reference with the canonical location; otherwise the spec is internally contradictory.
🤖 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 `@specs/BUG-031-windows-project-key/tasks.md` around lines 40 - 46, Resolve the
contradiction in the “Machine-readable features” section: either add the omitted
features.json artifact or update the “See sibling features.json” reference to
its actual canonical location. Keep the surrounding statement consistent with
the chosen artifact location and omission policy.
| - [x] `go build`/`vet`/`test ./...` clean; PSScriptAnalyzer clean on the two | ||
| CI-linted changed files; every changed `.ps1` ASCII-only. | ||
|
|
||
| ## Test status | ||
|
|
||
| - Go: `go build ./... && go vet ./... && go test ./...` -> all packages `ok` | ||
| (cli/internal/cmd, memlink, doctor, env, mem, ... green). `TestMemProjectKey` | ||
| passes. | ||
| - Pester (Windows, this box): `Invoke-Pester tests/claude-project-key.Tests.ps1` | ||
| -> `Tests Passed: 10, Failed: 0`. The dotf cross-check ran green against a | ||
| PR-built `dotf` on PATH. | ||
| - PSScriptAnalyzer (repo settings, Error+Warning): PASS on `setup-windows.ps1`, | ||
| `scripts/knowledge-crystallize.ps1`, `scripts/utils.ps1`, | ||
| `scripts/vault-maintenance-weekly.ps1`, `tests/claude-project-key.Tests.ps1`. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Make the PSScriptAnalyzer scope consistent.
Lines 26-27 claim only two CI-linted changed files, but lines 37-39 list five analyzed files. Replace “two” with the actual scope or explain the distinction so the verification record is unambiguous.
🤖 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 `@specs/BUG-031-windows-project-key/verification.md` around lines 26 - 39,
Update the PSScriptAnalyzer scope statement in the verification checklist to
match the five files listed in the test status, or explicitly distinguish the
two CI-linted files from the three additionally analyzed files. Ensure the
verification record is unambiguous without changing the reported results.
…oding Two characterization tests in knowledge-crystallize-ps1.bats asserted the buggy behavior this PR removes: one required a local `Get-EncodedPath` function (deleted in favor of the shared utils.ps1 helper), the other grepped for the colon-deleting `Replace ':' ''` pattern that produced the wrong single-dash key. They locked the #689 bug in place, so the fix flipped them red. Re-point both at the corrected reality: the script sources utils.ps1 and resolves the key via Get-ClaudeProjectKey (no local encoder), and the drive colon is mapped to '-' (double-dash key), never deleted.
BUG-031's correct fix flipped two bats characterization tests red because they asserted the old buggy behavior (a Get-EncodedPath function + the colon-deleting Replace pattern). A *.ps1-only grep missed them. Record the rule: grep every test extension for a removed symbol/pattern, and invert any test that fingerprints a bug when you kill it.
1 participant
Why
The Claude auto-memory junction key encoding drifted between the Go layer and its Windows PowerShell twins.
memlink.ClaudeProjectKey(the #551 fix) maps/,\and the drive:each to-(soC:\Users\me\p->C--Users-me-p), but two PowerShell sites still mapped only\and deleted the:(producingC-Users-me-p) -- a key Claude never reads:setup-windows.ps1built junctions at the wrong path and printed a misleadingLinked auto-memory.knowledge-crystallize.ps1resolved a nonexistent path and warnedNo MEMORY.md foundfor every project.Separately,
vault-maintenance-weekly.ps1invoked the crystallizer with the POSIX--all, which binds positionally to$ProjectDirand leaves[switch]$Allfalse -- so the Sunday scheduled task has never processed a project.Root cause of the encoding half: one datum duplicated across Go + 2 PowerShell sites, which drifted when #551 fixed only Go. Per ADR-020 (Go owns tooling logic) + SSOT, Go now owns the encoding and the twins call it.
What
dotf mem project-key <path>exposesmemlink.ClaudeProjectKeyas the single encoding source (TestMemProjectKey).scripts/utils.ps1:Get-ClaudeProjectKey(dotf-first, corrected fallback) +Get-ClaudeProjectKeyEncoded(pure, for the decoder's hot scan -- no per-directory subprocess).setup-windows.ps1+knowledge-crystallize.ps1call the helper; deleted the buggy localGet-EncodedPath; fixedGet-DecodedPathto the double-dash key (^([A-Za-z])--).vault-maintenance-weekly.ps1:--all->-All.tests/claude-project-key.Tests.ps1, 10/10): binds the PS encoder to Go's known-good keys; the dotf cross-check probes support at discovery time and skips (not fails) against a released dotf that predates the subcommand (CI: integration container tests the released dotf, not the PR's built binary #734).Verification
go build/vet/test ./...green;TestMemProjectKeypasses..ps1ASCII-only.Notes / follow-ups (ticketed, not inlined)
.ps1(this PR'sutils.ps1+ the test aren't covered) and ASCII-only isn't enforced (9 pre-existing em-dashes insetup-windows.ps1pass).docs/audits/codebase-audit-2026-07-06.md, which is absent from the repo.cli/.gitignore: addedbin/so a manualgo build -o bincan't be committed.Spec:
specs/BUG-031-windows-project-key/.Closes #689.
Summary by CodeRabbit
New Features
dotf mem project-key <path>to generate Claude project memory keys consistently.Bug Fixes
Tests