[codex] fix trash and runtime sync churn#27
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
![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 introduces configurable trash retention and size/count caps to the TrashJanitor to prevent disk exhaustion in high-churn environments, updating the default retention period from 30 to 7 days. It also refactors the FileSyncActor to skip excluded subdirectories during full-sync explosions and adds local-agent VM images and derived caches to the claudeAppSupport exclude patterns. Feedback on these changes highlights a potential data loss issue where a failed directory usage check could lead to premature bucket deletion, and suggests refining the file count logic to ignore empty subdirectories so they do not block cleanup.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| let usage = directoryUsage(url) ?? (bytes: 0, files: 0) | ||
| buckets.append(Bucket( | ||
| url: url, | ||
| name: name, | ||
| mtime: mtime, | ||
| size: usage.bytes, | ||
| fileCount: usage.files | ||
| )) |
There was a problem hiding this comment.
If directoryUsage returns nil (e.g., due to transient I/O or permission errors), treating it as (bytes: 0, files: 0) will set bucket.fileCount to 0. This causes isEmptyAndSettled to evaluate to true, leading to the premature deletion of the entire bucket and potential data loss. Skipping the bucket when usage cannot be determined is a much safer approach.
guard let usage = directoryUsage(url) else {
logger.warning("TrashJanitor: failed to get usage for \(url.path)", category: "trash")
continue
}
buckets.append(Bucket(
url: url,
name: name,
mtime: mtime,
size: usage.bytes,
fileCount: usage.files
))| private func directoryUsage(_ url: URL) -> (bytes: Int64, files: Int)? { | ||
| guard let it = fm.enumerator(at: url, | ||
| includingPropertiesForKeys: [.totalFileSizeKey], | ||
| options: [.skipsHiddenFiles]) else { | ||
| options: []) else { | ||
| return nil | ||
| } | ||
| var total: Int64 = 0 | ||
| var files = 0 | ||
| for case let fileURL as URL in it { | ||
| let values = try? fileURL.resourceValues(forKeys: [.totalFileSizeKey]) | ||
| if let bytes = values?.totalFileSize { | ||
| total += Int64(bytes) | ||
| } | ||
| files += 1 | ||
| } | ||
| return total | ||
| return (total, files) | ||
| } |
There was a problem hiding this comment.
The fm.enumerator returns all descendants, including directory entries. If a bucket contains empty subdirectories, files will be greater than 0, which prevents the bucket from being recognized as empty and cleaned up. Only counting regular files ensures that empty directory structures are correctly swept.
private func directoryUsage(_ url: URL) -> (bytes: Int64, files: Int)? {
guard let it = fm.enumerator(at: url,
includingPropertiesForKeys: [.totalFileSizeKey, .isDirectoryKey],
options: []) else {
return nil
}
var total: Int64 = 0
var files = 0
for case let fileURL as URL in it {
let values = try? fileURL.resourceValues(forKeys: [.totalFileSizeKey, .isDirectoryKey])
if values?.isDirectory == true {
continue
}
if let bytes = values?.totalFileSize {
total += Int64(bytes)
}
files += 1
}
return (total, files)
}
ComBba
changed the title
1 participant
Summary
Fixes #26.
This adds the missing guardrails around rsync trash growth and prevents high-churn Claude Desktop and Codex runtime artifacts from entering sync in the first place.
Root Cause
Code inspection showed four compounding issues:
RsyncCommandBuildercreates a fresh~/.claudesync/trash/<job-id>/bucket for every rsync job that uses--backup-dir.TrashJanitoronly removed UUID buckets older than the retention window, with no size cap, bucket-count cap, or empty-bucket cleanup.FileSyncActorexplodes top-level full syncs into immediate subdirectory jobs before rsync filtering, so existing target excludes such asLocal Storage/did not prevent those subdirs from being queued. The logs confirmed the same path forvm_bundles/,Code Cache/, andDawnWebGPUCache/.codexConfigwas treated as a realtime target but only excluded*.log,cache/, and*.tmp, so active Codex sessions, SQLite/WAL state, package caches, helper runtimes, and temp directories could repeatedly trigger realtime syncs.Runtime/log evidence from this machine before the fix:
~/.claudesync/logs/claudesync.log*contained 10,422--backup-dir=rsync invocations.vm_bundlesrsync entries.vm_bundleswas 37G, with large VM image files includingrootfs.img,rootfs.img.zst,sessiondata.img, and hidden.rootfs.img.*variants.~/.codexwas 1.3G despite the PRD expectation that the Codex config target stays under 10MB; most of that was runtime churn (sessions471M,packages389M,.tmp187M,logs_2.sqlite136M, plus WAL/SQLite/helper directories).~/.claudesync/preferences.jsonhad notrashRetentionDays, so the app used the old compiled default.Changes
vm_bundles/,*.img,*.raw,*.zst,Code Cache/, andDawnWebGPUCache/.codexConfig:sessions/,history.jsonl,session_index.jsonl,shell_snapshots/,sqlite/,*.sqlite*,packages/,.tmp/,node_repl/,ambient-suggestions/,attachments/,computer-use/,process_manager/,memories/, and plugin install staging.0disabling each cap.TrashJanitorto remove settled empty buckets and purge oldest settled buckets when size/count caps are exceeded..rootfs.img.*backup payloads are included in cap enforcement.Validation
CLAUDESYNC_DISABLE_SINGLE_INSTANCE=1 xcodebuild -scheme ClaudeSync -configuration Debug -destination 'platform=macOS' -only-testing:ClaudeSyncTests/TrashJanitorTests testCLAUDESYNC_DISABLE_SINGLE_INSTANCE=1 xcodebuild -scheme ClaudeSync -configuration Debug -destination 'platform=macOS' -only-testing:ClaudeSyncTests/SyncTargetTests -only-testing:ClaudeSyncTests/FileSyncActorTests/testFullSyncExplode_skipsCodexRuntimeSubdirs_v1_3_5 testCLAUDESYNC_DISABLE_SINGLE_INSTANCE=1 xcodebuild -quiet -scheme ClaudeSync -configuration Debug -destination 'platform=macOS' testFull suite passed after the final follow-up patch.