fix(cache): absolutize local sources before computing the index cache key - #105
Conversation
… key `resolve_cache_dir` hashed the raw source string the CLI/MCP received after a purely lexical normalize, while `from_path` recorded `std::path::absolute` in the manifest. Every `csp search` run with the default `.` therefore shared one cache key across all repos, forcing a full rebuild on each switch, and the same repo reached via different spellings got separate entries. `normalize_source` now makes local paths absolute (`std::path::absolute`, no filesystem access — the same call the MCP in-memory key and the manifest use) before path-normalizing, so `.`, `./r/../r`, and `/abs/r` share one leaf and the keyed form equals the manifest `sourceId`. Git URLs stay verbatim. Mirrors upstream `cache_key`'s `Path.resolve()`; ADR-0002 already specified this. Entries keyed from the old relative form become unreachable and are cleaned by `clear index` / `clear orphans`. Closes #100
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 11 |
| Duplication | 2 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the cache key source identity normalization logic in crates/csp/src/indexing/cache.rs to make local paths absolute against the current directory using std::path::absolute before normalizing them. This ensures that relative paths (like .) and their absolute counterparts resolve to the same cache key, preventing collisions when searching different repositories. Corresponding documentation has been updated in semble.md, and comprehensive unit tests have been added to verify the new behavior. I have no feedback to provide as the implementation is correct and well-tested.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR makes local disk-cache identities absolute before lexical normalization, adds coverage for equivalent local path spellings, and documents the corrected cache model.
Confidence Score: 4/5The PR should not merge until all supported SCP-style Git remotes remain independent of the caller’s current directory. The local-path fix works for the tested forms, but the new normalization regresses cache reuse for accepted Files Needing Attention: crates/csp/src/indexing/cache.rs, .please/docs/references/semble.md
|
| Filename | Overview |
|---|---|
| crates/csp/src/indexing/cache.rs | Absolutizes local cache-key sources and adds tests, but misclassifies supported non-git SCP-style remotes as local paths. |
| .please/docs/references/semble.md | Documents the cache-key fix, including an inaccurate claim that the normalized key source always equals the manifest source identity. |
Prompt To Fix All With AI
### Issue 1
crates/csp/src/indexing/cache.rs:150
**SCP remotes become local paths**
Non-`git` SCP-style remotes such as `deploy@host:org/repo.git` are recognized as Git URLs by `is_git_url`, but this check treats them as local paths because it only exempts strings beginning with `git@`. On Unix, `std::path::absolute` then prefixes the current directory, so using the same supported remote from different directories creates separate cache entries and forces redundant index builds. Use the shared Git URL classifier here so every accepted remote syntax remains verbatim.
### Issue 2
.please/docs/references/semble.md:299-300
**Documented identities can differ**
The documentation says the normalized cache-key source equals the manifest `sourceId`, but `normalize_source` also collapses lexical `.` and `..` components while `from_path` stores only the output of `std::path::absolute`. For inputs such as `.` or `./sub/../sub`, those strings differ. This gives maintainers an inaccurate cache invariant, so the statement should be corrected or the normalization paths aligned.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Reviews (1): Last reviewed commit: "fix(cache): absolutize local sources bef..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 2 files
Architecture diagram
sequenceDiagram
participant CLI as CLI/MCP/SDK
participant Cache as cache.rs
participant FS as Filesystem
participant Manifest as Manifest store
participant Index as Index cache store
Note over CLI,Index: Local source normalization flow
CLI->>Cache: resolve_cache_dir(source, contentType)
Cache->>Cache: is_url_scheme(source)?
alt URL or scp-style git source
Cache-->>CLI: Return key from verbatim source
else Local path
Cache->>FS: std::path::absolute(source)
FS-->>Cache: Absolute path
Cache->>Cache: normalize_posix(abs_path)
Cache->>Index: Compute sha256 key from normalized path
Index-->>Cache: Cache leaf path
Cache-->>CLI: Return cache directory
CLI->>Manifest: Record sourceId = absolute normalized path
end
Note over CLI,Index: In-memory MCP key path
MCP->>Cache: resolve_cache_dir(".")
Cache->>FS: std::path::absolute(".")
FS-->>Cache: /current/working/dir
Cache->>Cache: normalize_posix(/current/working/dir)
Cache-->>MCP: Same cache leaf as manifest sourceId
Note over CLI,Index: Different repos with default "."
CLI->>Cache: resolve_cache_dir(".") in repo-a
Cache->>FS: std::path::absolute(".")
FS-->>Cache: /repos/repo-a
Cache->>Index: Key from /repos/repo-a
CLI->>Cache: resolve_cache_dir(".") in repo-b
Cache->>FS: std::path::absolute(".")
FS-->>Cache: /repos/repo-b
Cache->>Index: Key from /repos/repo-b
Note over Index: Two separate cache entries<br/>(previously one shared entry)
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…s sweepable - Route `IndexCache::compute_key` through `cache::normalize_source` so `.`, `./r/../r`, and `/abs/r` name one in-memory session entry as well as one on-disk cache key (no duplicate `Arc<CspIndex>` per spelling, `evict` can't miss). - Fold `\` into `/` only on Windows: on Unix a backslash is an ordinary filename byte, so `/repos/a\b` and `/repos/a/b` must keep distinct keys and `..\y` must not synthesize a `..` segment. - `source_is_gone` now requires both the recorded `sourceId` and its lexically normalized form to be `NotFound`: `from_path` records `std::path::absolute`, which keeps `..`, so deleting only an intermediate directory must not make a still-present source look orphaned.
…ache key `normalize_source` exempted only `git@…` prefixes, so a non-`git` scp remote such as `deploy@host:org/repo.git` — which `is_git_url` accepts and `load_or_build_index` clones — was absolutized against the caller's cwd and keyed differently from every directory. Route the check through the shared classifier, and correct the semble drift note that claimed the keyed form equals the manifest `sourceId`.
|
1 participant
Summary
resolve_cache_dirhashed the raw source string the CLI/MCP received after a purely lexicalnormalize_posix, whilefrom_pathrecordedstd::path::absolutein the manifest. Everycsp searchrun with the default.therefore shared one cache key across all repos, which forced a full rebuild on each switch and seeded the incremental-reuse path from the other repo's manifest. The same repo reached through different spellings got separate entries.normalize_sourceincrates/csp/src/indexing/cache.rsnow makes local paths absolute withstd::path::absolutebefore path-normalizing. That call touches no filesystem and is the same one the MCP in-memory key and the manifest already use..,./r/../r, and/abs/rnow share one cache leaf, and the keyed form equals the manifestsourceId. Git URLs stay verbatim.This mirrors upstream semble
cache_key'sPath.resolve(). ADR-0002 already specified "normalized absolute path for local sources", so this is an implementation drift fix and needs no ADR change. Entries keyed from the old relative form become unreachable;clear indexandclear orphansclean them up. A drift note is added to.please/docs/references/semble.md.Related issue
Closes #100
Related: #87 / #102 (clear orphans), where this was found.
Checklist
cargo test --workspace— 333 lib + 26 CLI pass)cargo fmt,cargo clippy --all-targets --all-features -- -D warnings).please/docs/references/semble.md)BREAKING CHANGE:note is includedTests
Three new unit tests: relative and absolute forms share a key (including a
./sub/../subdetour), two relative names key by their absolute forms without colliding, and git URLs are not absolutized.End-to-end verification
Ran the built binary against a throwaway
HOME:.produce two cache entries (previously one).../repo-areuses repo-a's entry, so no third entry appears.csp clear orphansreclaims exactly that entry.Summary by cubic
Local source cache keys now use absolute, normalized paths instead of raw relative spellings, so searches from different repositories using
.no longer collide and equivalent path spellings reuse one entry. MCP session keys use the same normalizer, while every remote accepted byis_git_urlremains verbatim.source_is_gonenow requires both the recordedsourceIdand its normalized path to be missing, preventingclear orphansfrom deleting a live source reached through...clear indexorclear orphansreclaims them..collides across repos) #100.Written for commit 221a6db. Summary will update on new commits.