Repository navigation
fix(claude): fail cost scans when source roots are unreadable - #4617
Conversation
Inventory treated an unreadable root, a directory the enumerator could not open, and a failed file stat as if the sources did not exist. The scan then published a low report and replaced the cache and report memo with it. Throw instead, so the previous cache and memo stay in place until the sources are readable again.
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
clawsweeper
Bot
added
clawsweeper:intake:bug
|
Codex review: needs real behavior proof before merge. What this changesThis PR makes Claude source-inventory failures abort cost scans before incomplete results replace cached reports. Example: Two Claude transcripts contribute 10 and 5 input tokens, then the second transcript's directory becomes unreadable.
Review scores
ProductKind: Bug fix · Worth it: Yes · Fix scope: Complete Merge readiness⛔ Blocked before merge - 3 items remain Keep open: this PR addresses a source-reproducible cost-cache integrity bug that current main still contains. Priority: P2 Before merge
Findings
Tests
Agent review detailsHow this fits togetherThe shared Claude and Vertex cost scanner inventories local transcript roots, reconciles cached usage rows, and supplies reports to the menu, spend dashboard, and CLI. flowchart TD
A[Claude transcript roots] --> B[Source inventory]
B --> C[Cache reconciliation]
C --> D[Report and memo]
D --> E[Menu dashboard and CLI]
B --> F[Filesystem failure]
F --> G[Abort before cache writes]
Technical reviewBest possible solution: Keep the inventory-before-write boundary and surface an actionable source-access error through the existing cost-report callers. Do we have a high-confidence way to reproduce the issue? Current-main source shows how an unreadable directory produces an incomplete inventory that removes cached rows; the author also reports failing base runs for both permission scenarios, but this read-only review did not execute them. Is this the best way to solve the issue? Aborting incomplete inventory at its existing shared owner is a narrow root-cause repair that preserves missing-path churn, report-window isolation, and best-effort artifact stamps. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against 6480977b47f6. Provenance checked
TestingProof path: in-process harness. SecurityNone. EvidenceAcceptance criteria:
What I checked:
Likely related people:
Review metrics
LabelsLabel changes:
Label justifications:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment Reviewed October 18, 2026, 3:21 PM ET / 19:21 UTC. |
Refs steipete#4617. Co-authored-by: Dhruva Sahrawat <21278423+dhruva77@users.noreply.github.com>
# Conflicts: # CHANGELOG.md
|
Merged in 25ecff1, thanks @dhruva77! Claude cost scans now keep existing history when a source folder can't be read, instead of treating it as empty. We kept your fail-closed handling, folded a duplicate map into one stamp dictionary, and applied the same check to the new Cowork file roots. It will ship in the next release. |
2 participants
Summary
The Claude cost scanner builds a source inventory before it touches the cache. Three failures in that inventory currently look the same as "no files here":
FileManager.fileExists(atPath:)returnsfalseboth for a root that is missing and for a root it cannot stat. One example is a parent directory without search permission.FileManager.enumerator(at:...)runs without anerrorHandler. A subdirectory that can't be read (EACCES) is skipped silently, and so is every transcript under it.CostUsageClaudeFileStamp.read(at:)returnsnilfor anyfstatatfailure, so the inventory drops the file.Because the inventory is incomplete, the scan treats those files as deleted. It removes their rows from the cache, saves the smaller cache, and stores the low report in the report memo. The menu and spend views then show totals that are too low, and nothing reports an error. Once the files are readable again, the next refresh has to reparse them all.
Concrete scenario:
~/.claude/projects/<project>/contains a subdirectory that isn't readable, for example after a permissions change, a restore or sync tool, or a directory created by another user. That project's spend disappears from the Claude cost report. The report still says "updated just now", and no error is shown.May be related: #3209. That issue reports the spend chart not updating with no error message, which matches this failure mode. I haven't confirmed the cause there.
Fix
The changes are confined to
inventoryClaudeRootsandCostUsageClaudeFileStamp:statdecides whether a root exists.ENOENTandENOTDIRstill mean "missing, skip". Any other error is thrown.errorHandler. Files that vanish mid-walk are ignored as normal churn. Any other enumeration error stops the walk and is thrown. If the enumerator can't be created for a root that still exists, that is thrown too.CostUsageClaudeFileStamp.readIfPresent(at:). It returnsnilfor a missing file and throws for otherfstataterrors. The existingread(at:)is unchanged for its artifact-stamp callers.Inventory runs before the cache artifact or memo is read or written, so a throw leaves the last valid cache and the completed-report memo untouched. The error reaches the caller like any other scan failure. After the sources become readable, the next refresh hits the existing memo without parsing anything.
Missing roots and files that disappear during a scan behave exactly as before. Callers outside this path, the cache format and the memo format are unchanged. No CHANGELOG entry is included, to avoid conflicts with other open PRs; I'm happy to add one.
Tests
New parameterized test in
CostUsageClaudeReportContextTests:unreadable source inventory fails without replacing the cache or report memo. It covers two cases:000;000.Each case:
The test is disabled when running as root, because root ignores directory permissions.
The same test against
mainfails in both cases. With an unreadable subdirectory,mainreturns a 10-token report instead of 15, raises no error, and rewrites both the cache and the memo. With an unreadable root, it also returns successfully and raises no error.Notes
The fix is small and isolated: about 50 production lines across two files in the Claude cost scanner, plus one focused test. It doesn't touch the daily-report aggregation hunk changed by #4588, so the two should rebase cleanly in either order.