Repository navigation
fix(discover): match Claude project dirs case-insensitively on Windows - #2931
Conversation
|
Following up on the verification note in the description: I ran the CLAUDE.md pre-commit gate in a rust:1 container. fmt --check came back clean, clippy has no warnings, and cargo test --all passes, including the new encoding and dir_matches_filter tests. |
kingpy-bot
left a comment
There was a problem hiding this comment.
Independently verified this on head fa156a7f3d9418ef5f0258196f3e7e5f1e40ae3a. The colon normalization matches Claude Code’s Windows project-directory shape, and keeping case-insensitive matching behind cfg!(windows) preserves the existing Unix semantics. The existing sanitizer already covers spaces, so the additional spaced-path report follows the same encoding path. In a clean Rust 1.91 container, cargo fmt --all --check, cargo clippy --all-targets -- -D warnings, and the full cargo test suite all pass (2,485 passed, 8 ignored).
|
I did a full code review of this and the logic is correct and complete — both the colon encoding (verified against Claude Code's real folder format, e.g. But there's one gap I can't close and I want to flag it honestly: no one has run this end-to-end on real Windows yet. The author verified the slug format against real folders but had no Rust toolchain on Windows, the unit tests only exercise the pure Could a Windows user run this quick check? In a real project that has Claude Code history: # 1. Build this branch
cargo build --release
# 2. From a project dir you've used with Claude Code:
# Before this PR, `rtk discover` reports "Scanned: 0 sessions" (issue #2919)
.\target\release\rtk discover
# Expected AFTER this PR: it finds your sessions (Scanned: N > 0).
# Bonus: confirm it works with an uppercase-vs-lowercase drive-letter folder,
# and that `rtk discover -p <name>` matches case-insensitively.The fix looks right on paper (fixes #2919 at both the encode and match sites) — just want one real-Windows confirmation before it's merged, since the whole point is Windows-only behavior that our CI/dev environments don't actually exercise. |
|
Ran this on a real Windows machine, not Docker or WSL. Base branch (5d32d07, before this fix): Also tested the case-insensitive One honest caveat: this machine had no Rust toolchain until today, so I installed rustup fresh with the GNU target since there's no Visual Studio Build Tools here. Shouldn't affect what's being tested since cfg!(windows) is true either way, but flagging it since you were specific about wanting real execution rather than a read of the code. |
`rtk discover -p <filter>` matches the filter against project directory names with a case-sensitive `contains`, so on Windows it can come back empty even when the project is there. NTFS is case-insensitive and Claude Code builds the folder name from whatever cwd casing it was launched with, so one project can end up under both `E--proj` and `e--proj` while `std::env::current_dir()` hands back an uppercase drive letter. Compare case-insensitively under `cfg!(windows)`. Unix behavior is unchanged. Refs rtk-ai#2919
fa156a7 to
d2d5054
Compare
|
#2952 covers the colon half of this and closed #2919, so I rebased onto develop and dropped that part. What is left is the other half of what I originally hit: Diff is down to One caveat on verification: cargo is blocked by an Application Control policy on this machine and Docker would not come up, so I could not run the full suite this time. I compiled the function and the new assertions with rustc directly and they pass, and the current develop path does fail the case-mismatched filter. rustfmt clean. Relying on CI for the rest. Fine by me to close this if it is not worth carrying now that #2919 is done. |
…ough the walk The `_impl` tests pin the predicate but nothing reaches the `cfg!(windows)` wrapper, so a wrapper that silently stayed case-sensitive passed the suite on every runner. Add a `discover_sessions_in_projects_dir` test with a case-mismatched filter that expects a match on Windows and none elsewhere, and say in the `-p` help of `discover` and `learn` that the match is case-insensitive on Windows. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Brings the branch onto the current develop so CI's semgrep baseline is the develop tip rather than the July merge-base; no source changes beyond the merge itself.
KuSh
left a comment
There was a problem hiding this comment.
Round 1 of 3 — APPROVED, follow-ups filed
Claim: rtk discover -p <filter> and the default current-project filter match the ~/.claude/projects/ directory name case-insensitively on Windows, with Unix behaviour unchanged.
Scope: accept (round 1, frozen)
Ran: discover × 9 forms (default cwd, -p upper/lower/mixed, empty, non-ASCII, --all) and learn × 4 forms, against a 6-project history tree (mixed-case siblings E--PROJECT-app/e--PROJECT-app, …-rtk/…-RTK, one non-UTF-8 name), before/after on the merge-base 6f0b0ca, the head and the head merged onto develop c529013; the Windows branch executed on Linux by forcing the cfg!(windows) flag in a probe build; invariants: non-default targets | under the hook (rtk rewrite "rtk discover -p X" returns it unchanged) | split streams | every decision (both casing directions, the non-match, empty filter, non-UTF-8 name); mutation: 3 mutants of the matcher; type-check of cfg(windows) via cargo check --target x86_64-pc-windows-gnu --all-targets. Savings: n/a, no bash output is filtered by this change.
Matrix: executed this round
CI @925f9be5: all 11 checks green (run 36357961505: fmt, clippy, semgrep, Security Scan, benchmark, doc review, test presence, tests on ubuntu, macos and windows, CLA). On the author's head d2d5054 CI had never run (the August run expired with 0 jobs and could not be re-run). On the fixup head 5a6c151, 9 of 10 jobs passed including the Windows test; semgrep failed on a remove_dir_all inside a git_cmd.rs test that develop added on 2026-09-14, because the job's baseline was this PR's July merge-base — hence the develop merge.
Blocks merge (0)
Checked and correct — no need to re-verify
- Linux before/after is byte-identical on all 13 rows: Unix semantics are unchanged as claimed.
- With the Windows branch active,
-p e--projectgoes from 0 sessions to bothE--PROJECT-appande--PROJECT-app; the default cwd filter also picks up a case-mismatched sibling folder;learn -p e--projectgoes from "No Claude Code sessions found" to scanning them. Exit codes 0 throughout on both sides. - Empty filter, non-ASCII filter and a non-UTF-8 directory name behave identically before and after.
- ASCII-only lowering is sufficient: the encoder replaces every non-ASCII char with
-, so no folder name carries one. - Both callers (
discover::run,learn::run) reach the change throughdiscover_sessions_in_projects_dir; there is no other reader of~/.claude/projects/. cargo fmt --check,cargo clippy --all-targets(0 warnings) andcargo test --allpass on the head and on the head merged onto develop; the merge is clean.- Mutating the
_implto ignore its flag failstest_dir_matches_filter_case_insensitive_windows, so the predicate is pinned. Forcing the wrapper to case-sensitive passed the whole suite on every platform, which is what the fixup's test closes: with the wrapper forced the other way it fails (left: 1, right: 0).
Filed as follow-ups
- #4313
rtk discover: the example command shown for a bucket changes between runs on the same history — pre-existing, outside this PR's hunks; filed
Your questions
- "Fine by me to close this if it is not worth carrying now that #2919 is done." → keep it: #2952 fixed the colon only, and the case half is real — the #2919 reporter's own tree has
E--PROJECT-…ande--PROJECT-…side by side with an uppercasecurrent_dir(), which still finds nothing on develop. - "Happy to follow up on anything clippy/fmt flags." → nothing flagged, both clean on the head and after the merge.
Next
fixup 5a6c151 pushed, then develop merged as 925f9be so CI's semgrep baseline is current: adds a discover_sessions_in_projects_dir test with a case-mismatched filter (a match on Windows, none elsewhere — the only thing that exercises the cfg!(windows) wrapper, which no test reached before), and says in the -p help of discover and learn that the match is case-insensitive on Windows, and rewords the helper's doc comment so the drive-letter casing reads as the shell's choice rather than an OS guarantee — approving once CI is green on it.
Rounds: 1/3. Threads: 0 open, 0 resolved this round.
Fixes #2919
rtk discover(andrtk learn) always reportedScanned: 0 sessionsin default current-project mode on Windows. Two independent defects insrc/discover/provider.rs:1. The drive-letter colon was never sanitized.
encode_project_path()kept:, producing filters likeE:-PROJECT-app— but:is not a legal character in Windows directory names, so the filter could never match any real folder under~/.claude/projects/. Claude Code encodesE:\PROJECT\appasE--PROJECT-app. Verified against a real machine: for cwdJ:\Z\GitHub\Corner, Claude Code's history folder isJ--Z-GitHub-Corner; the previous encoding computesJ:-Z-GitHub-Corner, the new one matches exactly. The existingtest_encode_project_path_windowsexpectation encoded the impossible form and was updated.2. Case-sensitive matching (the mismatch reported in the issue). Claude Code encodes the folder from whatever cwd casing the session was launched with, while
std::env::current_dir()reports an uppercase drive letter; the same project can even produce sibling folders (E--projande--proj, as in the issue). NTFS is case-insensitive, so the directory-name match is now case-insensitive on Windows only — Unix behavior is unchanged. This also makes-p <name>case-insensitive on Windows, consistent with the platform's path semantics.Both fixes flow through
discover_sessions(), sortk learn's current-project mode is fixed too.Tests: updated the Windows encoding test, added a lowercase-drive encoding test, and unit tests for the new
dir_matches_filterhelper covering both casing directions plus a non-match. The helper takes an explicit case-insensitivity flag so the Windows semantics are exercised on any CI platform.Note on verification: developed on a Windows machine without a local Rust toolchain, so the encoding behavior was verified against real Claude Code history folders (above) and the suite is left to CI. Happy to follow up on anything clippy/fmt flags.