refactor: split and dedup oversized source files in fuse, sdk-core, api, and web#538
Merged
Conversation
Entire-Checkpoint: a4db893efa17
Entire-Checkpoint: fdcc3336003d
# Conflicts: # .planning/STATE.md
…ib.rs Task 1 of plan 55-01. Move NETWORK_TIMEOUT/block_with_timeout into runtime.rs; pending/event types + spawn_metadata_refresh into events.rs; publish queue/ coordinator + next_file_publish_sequence + resolve helpers into publish.rs. lib.rs keeps cfg-gated module decls + re-exports so the public crate surface is unchanged. cargo build + cargo test -p cipherbox-fuse pass (64/64); winfsp feature build is Windows-only (CI-gated).
- Move encrypt_metadata_to_json, merge_folder_children, spawn_metadata_publish, spawn_bin_entry_publish, spawn_file_meta_reencrypt, and private helpers ReencryptOutcome/resolve_and_fetch_file_meta to metadata.rs - Move CipherBoxFS struct, inherent impl, uuid_from_ino, and mount_point to fs.rs - Move T-45-08 merge tests into metadata.rs mod tests - Add cfg-gated pub mod metadata/fs declarations and re-exports to lib.rs - No visibility changes; no public API changes
- Create crates/fuse/src/replay.rs: replay_for_vault (pub), resolve_folder_key, resolve_folder_key_cached, fetch_merge_publish_parent, publish_child_folder_metadata, replay_mkdir_entry, replay_upload_entry, decrypt_journal_name (pub(crate)), plus all replay tests (T-45-06, T-45-07, REQ-4, REQ-5, T-45-05, F2) - Slim lib.rs from ~2128 to ~74 LoC production declarations + re-exports; handler_harness_tests and durability_characterization_tests stay per RESEARCH Pitfall 3 - Fix crate::NETWORK_TIMEOUT → crate::runtime::NETWORK_TIMEOUT in read_ops.rs - Update durability_characterization_tests to use crate::replay::decrypt_journal_name (Rule 3 fix: decrypt_journal_name moved to replay.rs as pub(crate))
- Extract fetchAndDecryptMetadata + loadFolderMetadata into load.ts - Extract 4 pure transforms into metadata-ops.ts (rename/delete/addFilePointer/move) - Extract IPNS record build + batch-publish fns into registration.ts - Reduce index.ts to ~20 LoC barrel re-exporting all sibling modules - registration.ts imports fetchAndDecryptMetadata directly from ./load (not via barrel) - All 211 sdk-core tests pass; public export surface unchanged (D-05)
- Create ipns-record.codec.ts with IpnsRecordFields interface + 3 standalone fns - parseIpnsRecordBytes, parseCachedRecord, withCachedPublicKey take Logger as param - Remove the 3 private methods from IpnsService; replace calls with imported fns - IpnsService retains @Injectable, constructor, and all orchestration unchanged - No api:generate (no DTO/endpoint change); 893 API tests pass
- Create details/DetailsPrimitives.tsx: CopyableValue, DetailRow, formatDateWithTime - Create details/VersionHistory.tsx: preserves void folderKey lint suppression (Pitfall 7) - Create details/FileDetails.tsx: file metadata display with version history - Create details/FolderDetails.tsx: folder IPNS/key/timestamp display - Slim DetailsDialog.tsx to container: retains both cross-guarded useEffects (Pitfall 4) - CSS import stays in container only; barrel (index.ts) untouched - Container props/signature byte-identical; 63 web tests pass; tsc --noEmit clean
… bin-publish helper
- crates/fuse/src/write_ops.rs -> write_ops/mod.rs behind preserved pub(crate) mod implementation facade
- Handler bodies split into implementation/{file_data,delete,mkdir,rename}.rs
- Shared ~50-line bin-publish tail deduped into publish_bin_entry_on_delete helper in delete.rs
- crate::write_ops::implementation::handle_* caller paths unchanged
- All 64 cipherbox-fuse tests pass
…mplete_auth_setup tail - load_vault_settings moved from auth.rs to vault.rs (pub(crate), body byte-identical, ECIES unwrap preserved) - auth.rs call site updated to super::vault::load_vault_settings - complete_auth_setup mount/device-registry/teardown tail factored into private post_auth_finalize helper - complete_auth_setup pub(crate) signature unchanged; debug.rs import path stable - cargo build -p cipherbox-desktop passes
- Create crates/fuse/src/content_ops.rs with fetch_and_decrypt_content_async and publish_file_metadata under cfg any(fuse,winfsp) - A2 scope narrowing: fetch_and_decrypt_file_content stays in each operations.rs because macOS uses 3s private NETWORK_TIMEOUT while Windows uses crate:: block_with_timeout (10s); different timeouts are intentional for sync FUSE path - operations.rs (macOS) and platform/windows/operations.rs both re-export the two shared async helpers from crate::content_ops - Add pub mod content_ops cfg-gated any(fuse,winfsp) to lib.rs - cargo test -p cipherbox-fuse: 64 passed - Windows winfsp side verified by inspection and CI (cannot compile on macOS)
- Create crates/fuse/src/poll.rs with PollResult enum (any fuse,winfsp) and poll_filepointer_resolution (fuse-only, pub(crate)); read_ops.rs imports them - Remove duplicate PollResult + poll_filepointer_resolution from read_ops.rs - Add spawn_content_prefetch_fuse helper in read_ops.rs to dedupe 3x identical prefetch-spawn blocks (handle_open read path + 2x handle_read sites) - Create platform/windows/content_fetch.rs with spawn_content_prefetch (winfsp-only) deduping 2x prefetch closure in platform/windows/read_ops.rs - Declare mod content_fetch in platform/windows/mod.rs - handle_release stays in read_ops.rs (CR-04/D-04 journal-fsync-before-ack) - cargo test -p cipherbox-fuse: 64 passed - Windows winfsp side verified by inspection and CI (cannot compile on macOS)
…ulate.rs - Normalize macOS and Windows prepopulate blocks into single shared fn - Use cipherbox_core re-export paths (work on both fuse + winfsp) - Use get_unresolved_file_pointers_for_parent throughout (more precise) - Both mount_filesystem fns now call prepopulate::prepopulate_filesystem - A3 deviation: blocks were NOT byte-identical; normalized to match arms + re-export paths
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
WalkthroughPhase 55 completes a large-scale pure structural refactor across Rust ( ChangesPhase 55 Planning and Governance Documentation
Rust Fuse Refactor
Desktop Tauri, API, SDK, and Web Refactors
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
github-actions
Bot
added
release:web:refactor
Contributor
Release Preview
Cascade Details
|
Greptile SummaryThis PR is a large-scale source-file decomposition ("Phase 55") that splits several oversized Rust and TypeScript source files into cohesive sibling modules while explicitly preserving all public surfaces. No behavioral changes are intended; the work is pure extraction and deduplication.
Confidence Score: 5/5Safe to merge — all changes are structural extractions with no behavioral differences; public surfaces are frozen and verified against the original. Every module split was verified to be byte-equivalent or semantically equivalent to the removed code: the IPNS codec is a direct move of three private methods with an explicit logger parameter; the Rust module decomposition preserves all cfg gates and re-exports; the prepopulate helper normalizes the macOS all-inodes variant to the more precise scoped call (correct given sequential population order); the barrel pattern in folder/index.ts is intact; key-zeroization invariants are maintained. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
subgraph Before["Before (monolithic)"]
LIB["crates/fuse/src/lib.rs\n(3276 LoC)"]
WO["write_ops.rs\n(1132 LoC)"]
IDX["folder/index.ts\n(615 LoC)"]
SVC["ipns.service.ts\n(private methods)"]
DD["DetailsDialog.tsx\n(500 LoC)"]
end
subgraph After["After (decomposed)"]
LIB2["lib.rs (~74 LoC)\ncrate root + re-exports"]
RT["runtime.rs"]
EV["events.rs"]
PUB["publish.rs"]
META["metadata.rs"]
FS["fs.rs"]
REP["replay.rs"]
WOM["write_ops/mod.rs"]
DEL["implementation/delete.rs"]
IDX2["folder/index.ts (barrel)"]
LOAD["load.ts"]
MOPS["metadata-ops.ts"]
REG["registration.ts"]
CODEC["ipns-record.codec.ts"]
SVC2["ipns.service.ts (imports codec)"]
DD2["DetailsDialog.tsx (shell)"]
FDT["FileDetails.tsx"]
PP["fuse/prepopulate.rs\n(macOS+Windows shared)"]
CO["content_ops.rs\n(shared async crypto)"]
end
LIB --> LIB2
LIB --> RT & EV & PUB & META & FS & REP
WO --> WOM & DEL
IDX --> IDX2
IDX --> LOAD & MOPS & REG
SVC --> CODEC & SVC2
DD --> DD2 & FDT
LIB --> PP & CO
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
subgraph Before["Before (monolithic)"]
LIB["crates/fuse/src/lib.rs\n(3276 LoC)"]
WO["write_ops.rs\n(1132 LoC)"]
IDX["folder/index.ts\n(615 LoC)"]
SVC["ipns.service.ts\n(private methods)"]
DD["DetailsDialog.tsx\n(500 LoC)"]
end
subgraph After["After (decomposed)"]
LIB2["lib.rs (~74 LoC)\ncrate root + re-exports"]
RT["runtime.rs"]
EV["events.rs"]
PUB["publish.rs"]
META["metadata.rs"]
FS["fs.rs"]
REP["replay.rs"]
WOM["write_ops/mod.rs"]
DEL["implementation/delete.rs"]
IDX2["folder/index.ts (barrel)"]
LOAD["load.ts"]
MOPS["metadata-ops.ts"]
REG["registration.ts"]
CODEC["ipns-record.codec.ts"]
SVC2["ipns.service.ts (imports codec)"]
DD2["DetailsDialog.tsx (shell)"]
FDT["FileDetails.tsx"]
PP["fuse/prepopulate.rs\n(macOS+Windows shared)"]
CO["content_ops.rs\n(shared async crypto)"]
end
LIB --> LIB2
LIB --> RT & EV & PUB & META & FS & REP
WO --> WOM & DEL
IDX --> IDX2
IDX --> LOAD & MOPS & REG
SVC --> CODEC & SVC2
DD --> DD2 & FDT
LIB --> PP & CO
Reviews (5): Last reviewed commit: "chore(ci): re-trigger CI after release-t..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (3)
.planning/phases/55-large-source-file-refactor/55-01-SUMMARY.md (1)
68-77: 💤 Low valueFix missing language specifier in code block (MD040).
Line 68 starts a code block without a language identifier. This triggers the Biome
MD040lint (fenced-code-language).🔧 Proposed fix
-``` +``` mod cache, constants, error, file_handle, helpers, inode, journal_helpers(Change the opening fence to specify the language, e.g.,
rust` ortext` depending on intent.)🤖 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 @.planning/phases/55-large-source-file-refactor/55-01-SUMMARY.md around lines 68 - 77, The markdown code block containing the module declarations starting with "mod cache, constants, error, file_handle..." is missing a language specifier after the opening fence, which triggers the Biome MD040 lint rule. Add a language identifier to the opening code fence by changing the opening ``` to ```rust (or ```text if more appropriate) to specify the language of the code block and satisfy the fenced-code-language requirement.apps/desktop/src-tauri/src/fuse/prepopulate.rs (1)
104-106: 💤 Low valueConsider logging a warning if root folder key length is invalid.
If
root_folder_key.try_into()fails (line 104-105), FilePointer resolution is silently skipped with no diagnostic output. While this should never happen given that keys are validated upstream, adding a warning would aid debugging in edge cases.if !root_unresolved.is_empty() { log::info!("Resolving {} root FilePointer(s)...", root_unresolved.len()); let root_key_arr: Result<[u8; 32], _> = root_folder_key.try_into(); - if let Ok(fk) = root_key_arr { + match root_key_arr { + Ok(fk) => { let fk = Zeroizing::new(fk); for (fp_ino, fp_ipns) in &root_unresolved { // ... existing resolution logic ... } + } + Err(_) => { + log::warn!( + "Root folder key length {} (expected 32), skipping FilePointer resolution", + root_folder_key.len() + ); + } } }The same pattern applies at lines 228-230 for subfolder keys.
🤖 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 `@apps/desktop/src-tauri/src/fuse/prepopulate.rs` around lines 104 - 106, Add warning log messages when the try_into() conversions fail for root folder and subfolder keys. When root_folder_key.try_into() fails in the if let Ok(fk) block around line 104-105, log a warning indicating that the root folder key length is invalid before silently skipping FilePointer resolution. Apply the same pattern to the subfolder key validation around lines 228-230 where a similar try_into() call is made. This ensures diagnostic output is available for debugging if these edge cases occur, even though keys should be validated upstream.crates/fuse/src/replay.rs (1)
848-851: 💤 Low valueVerify unused
_public_keyparameter.The
_public_keyparameter is passed toreplay_upload_entrybut never used. If this is intentional (for API consistency with other replay functions or future use), consider adding a brief doc comment explaining why. If it's dead code, it should be removed to avoid confusion.🤖 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 `@crates/fuse/src/replay.rs` around lines 848 - 851, The `replay_upload_entry` function has an unused `_public_key` parameter. Either add a doc comment to the function explaining why this parameter is intentionally included despite being unused (such as for API consistency or future use), or remove the parameter entirely if it is truly dead code. The underscore prefix indicates intentional non-use, but clarification is needed for maintainability.
🤖 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 `@apps/api/src/ipns/ipns-record.codec.ts`:
- Line 19: Remove trailing commas from the specified lines (19, 55, 65, 72, and
87) in the ipns-record.codec.ts file to comply with the project's Prettier
configuration. These trailing commas appear in function parameters, method
arguments, and function calls throughout the file, and removing them will
resolve the CI pipeline failures related to Prettier formatting violations.
In `@apps/api/src/ipns/ipns.service.ts`:
- Around line 25-29: The import statement from './ipns-record.codec' containing
parseIpnsRecordBytes, parseCachedRecord, and withCachedPublicKey is currently
formatted across multiple lines but the linter expects it on a single line.
Reformat this import statement to be a single line, keeping all the imported
symbols together on one line while maintaining the same import source.
In `@crates/fuse/src/content_ops.rs`:
- Around line 114-175: The issue is that expected_sequence_number is hardcoded
to None in the IpnsPublishRequest, but it should be set to current_seq when not
is_first_publish to prevent concurrent devices from overwriting each other.
Additionally, when a Conflict result is returned from the publish_ipns call, the
code still calls coordinator.record_publish which updates the local cache,
treating the conflict as a successful publish. To fix this, change
expected_sequence_number from None to current_seq in the IpnsPublishRequest
construction, and modify the match statement on the PublishResult to return an
error when Conflict occurs instead of just logging a warning, ensuring
coordinator.record_publish is only called in the Success case.
In `@crates/fuse/src/events.rs`:
- Around line 77-109: The metadata refresh operation in the async block spawned
by rt.spawn can hang indefinitely if the resolve_ipns or fetch_content calls
don't complete, blocking future refreshes for the same IPNS name. Wrap the
entire async operation (the block containing resolve_ipns, fetch_content, and
decrypt_metadata_from_ipfs_public calls) with tokio::time::timeout using a
NETWORK_TIMEOUT constant to enforce a maximum execution time. If the timeout is
exceeded, the error handling should catch the timeout and send a
PendingRefresh::Failure message to unblock future refresh attempts for the same
IPNS name.
In `@crates/fuse/src/fs.rs`:
- Around line 412-421: The loop in this section breaks when spawned reaches
MAX_CONCURRENT_FP_RESOLVES, causing remaining unresolved file pointers in the
iterator to be dropped without being tracked for later processing. To fix this,
ensure that any unresolved file pointers that exceed the concurrent limit are
preserved in a pending queue or similar persistence mechanism so they can be
scheduled for resolution in the next refresh cycle. Modify the loop logic to
collect or queue the remaining unresolved items that weren't spawned due to
hitting MAX_CONCURRENT_FP_RESOLVES, rather than simply breaking and discarding
them.
- Around line 269-289: The pruned CIDs are being unpinned unconditionally for
all upload completions, but stale uploads should not trigger unpins since newer
writes may still reference those CIDs. Wrap the for loop that iterates over
result.pruned_cids and spawns the unpin_content calls with a generation guard
check that verifies inode.write_generation == result.write_generation before
proceeding with the unpin operation, similar to how the inode update and content
cache update are protected.
- Around line 215-223: The code uses `.ok()` on the result of
`cipherbox_crypto::wrap_key(key, &self.public_key)`, which silently converts
errors to None, allowing the function to continue without the encrypted IPNS
key. Instead of using `.ok()` followed by `.map()`, replace this error handling
to propagate the error up the call stack. The wrapping operation in the else-if
branch where `file_ipns_private_key` is Some should return an error result if
key wrapping fails, rather than defaulting to None and continuing execution.
In `@crates/fuse/src/metadata.rs`:
- Around line 279-348: When the bin IPNS record is not found (first publish
scenario), the code creates empty metadata but later uses unwrap_or(0) to get
the sequence, then publishes with expected_sequence_number set to "0", which
causes conflicts. To fix this, detect when the record does not exist (capture
the not-found condition from the earlier error handling) and pass
expected_sequence_number as None for first-publish semantics instead of
Some("0"). Additionally, in the PublishResult::Conflict arm, the code should not
silently return success after a conflict is detected; instead, it should
propagate the conflict as an error or handle the retry logic appropriately,
rather than treating it as a successful outcome.
In `@crates/fuse/src/publish.rs`:
- Around line 21-23: The sequence increment operation in the map closure using
`seq + 1` will overflow when the sequence value reaches u64::MAX, causing panics
in debug mode or undefined behavior in release mode. Replace the simple addition
operator with Rust's checked arithmetic method for u64 to safely handle the
increment and properly return an error when overflow would occur. This involves
using a method that returns an Option type instead of performing unchecked
arithmetic on the seq value inside the map function.
In `@packages/sdk-core/src/folder/load.ts`:
- Around line 20-34: The `fetchAndDecryptMetadata` function lacks error handling
for potentially throwing operations on the decoding and parsing of encrypted
metadata. Wrap the operations that decode the encrypted bytes
(TextDecoder.decode), parse the JSON (JSON.parse), and decrypt the folder
metadata (decryptFolderMetadata) in a try-catch block. In the catch block, log
the error with clear context about which operation failed, then either transform
the error with additional context or re-throw it to be handled upstream.
---
Nitpick comments:
In @.planning/phases/55-large-source-file-refactor/55-01-SUMMARY.md:
- Around line 68-77: The markdown code block containing the module declarations
starting with "mod cache, constants, error, file_handle..." is missing a
language specifier after the opening fence, which triggers the Biome MD040 lint
rule. Add a language identifier to the opening code fence by changing the
opening ``` to ```rust (or ```text if more appropriate) to specify the language
of the code block and satisfy the fenced-code-language requirement.
In `@apps/desktop/src-tauri/src/fuse/prepopulate.rs`:
- Around line 104-106: Add warning log messages when the try_into() conversions
fail for root folder and subfolder keys. When root_folder_key.try_into() fails
in the if let Ok(fk) block around line 104-105, log a warning indicating that
the root folder key length is invalid before silently skipping FilePointer
resolution. Apply the same pattern to the subfolder key validation around lines
228-230 where a similar try_into() call is made. This ensures diagnostic output
is available for debugging if these edge cases occur, even though keys should be
validated upstream.
In `@crates/fuse/src/replay.rs`:
- Around line 848-851: The `replay_upload_entry` function has an unused
`_public_key` parameter. Either add a doc comment to the function explaining why
this parameter is intentionally included despite being unused (such as for API
consistency or future use), or remove the parameter entirely if it is truly dead
code. The underscore prefix indicates intentional non-use, but clarification is
needed for maintainability.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 60dbd7ab-985a-48e7-b924-b8f07a9663f7
📒 Files selected for processing (55)
.planning/ROADMAP.md.planning/STATE.md.planning/phases/55-large-source-file-refactor/55-01-PLAN.md.planning/phases/55-large-source-file-refactor/55-01-SUMMARY.md.planning/phases/55-large-source-file-refactor/55-02-PLAN.md.planning/phases/55-large-source-file-refactor/55-02-SUMMARY.md.planning/phases/55-large-source-file-refactor/55-03-PLAN.md.planning/phases/55-large-source-file-refactor/55-03-SUMMARY.md.planning/phases/55-large-source-file-refactor/55-04-PLAN.md.planning/phases/55-large-source-file-refactor/55-04-SUMMARY.md.planning/phases/55-large-source-file-refactor/55-CONTEXT.md.planning/phases/55-large-source-file-refactor/55-DISCUSSION-LOG.md.planning/phases/55-large-source-file-refactor/55-PATTERNS.md.planning/phases/55-large-source-file-refactor/55-RESEARCH.md.planning/phases/55-large-source-file-refactor/55-SECURITY.md.planning/phases/55-large-source-file-refactor/55-VALIDATION.md.planning/phases/55-large-source-file-refactor/55-VERIFICATION.md.planning/todos/pending/2026-06-21-zeroize-fuse-metadata-publish-key-params.mdapps/api/src/ipns/ipns-record.codec.tsapps/api/src/ipns/ipns.service.tsapps/desktop/src-tauri/src/commands/auth.rsapps/desktop/src-tauri/src/commands/vault.rsapps/desktop/src-tauri/src/fuse/mod.rsapps/desktop/src-tauri/src/fuse/prepopulate.rsapps/desktop/src-tauri/src/fuse/windows/mod.rsapps/web/src/components/file-browser/DetailsDialog.tsxapps/web/src/components/file-browser/details/DetailsPrimitives.tsxapps/web/src/components/file-browser/details/FileDetails.tsxapps/web/src/components/file-browser/details/FolderDetails.tsxapps/web/src/components/file-browser/details/VersionHistory.tsxcrates/fuse/src/content_ops.rscrates/fuse/src/events.rscrates/fuse/src/fs.rscrates/fuse/src/lib.rscrates/fuse/src/metadata.rscrates/fuse/src/operations.rscrates/fuse/src/platform/windows/content_fetch.rscrates/fuse/src/platform/windows/mod.rscrates/fuse/src/platform/windows/operations.rscrates/fuse/src/platform/windows/read_ops.rscrates/fuse/src/poll.rscrates/fuse/src/publish.rscrates/fuse/src/read_ops.rscrates/fuse/src/replay.rscrates/fuse/src/runtime.rscrates/fuse/src/write_ops.rscrates/fuse/src/write_ops/implementation/delete.rscrates/fuse/src/write_ops/implementation/file_data.rscrates/fuse/src/write_ops/implementation/mkdir.rscrates/fuse/src/write_ops/implementation/rename.rscrates/fuse/src/write_ops/mod.rspackages/sdk-core/src/folder/index.tspackages/sdk-core/src/folder/load.tspackages/sdk-core/src/folder/metadata-ops.tspackages/sdk-core/src/folder/registration.ts
💤 Files with no reviewable changes (1)
- crates/fuse/src/write_ops.rs
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (37.91%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #538 +/- ##
==========================================
+ Coverage 64.75% 68.10% +3.35%
==========================================
Files 143 175 +32
Lines 11115 19396 +8281
Branches 1246 1298 +52
==========================================
+ Hits 7197 13209 +6012
- Misses 3675 5942 +2267
- Partials 243 245 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Owner
Author
|
@coderabbitai full review please |
|
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
packages/sdk-core/src/__tests__/folder.test.ts (1)
607-632: ⚡ Quick winAdd an early-wrap failure zeroization test for
createSubfolder.Current coverage checks zeroization only when TEE wrapping fails. Please also cover failure in the initial user-key wrap path so this sensitive-data cleanup contract is fully guarded.
As per coding guidelines for
**/*.test.ts, tests should emphasize meaningful edge-case coverage rather than nominal-path counts.🤖 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 `@packages/sdk-core/src/__tests__/folder.test.ts` around lines 607 - 632, The test `zeros key material and rethrows if TEE wrapping fails` currently only covers zeroization when the third wrapKey call (TEE wrap) fails. Add a new test case to cover the scenario where the initial user-key wrap fails (either the first or second wrapKey call for wrapping the private key or folder key for the user public key). This new test should verify that both ipnsPriv and folderKey buffers are properly zeroed when createSubfolder encounters an error during the early wrapping phase, ensuring the sensitive-data cleanup contract is fully covered across all error paths in the function.Source: Coding guidelines
apps/web/src/components/file-browser/details/FolderDetails.tsx (1)
74-77: ⚡ Quick winUse
encryptedIpnsPrivateKeyterminology in this TSX surface.To keep naming consistent without changing the upstream
FolderEntrycontract, alias once locally and render via the alias.As per coding guidelines: “Use consistent terminology: `encryptedIpnsPrivateKey` (not `encrypted_ipns_key`, `ipns_key_encrypted`).”Suggested refactor
export function FolderDetails({ item, @@ }) { + const encryptedIpnsPrivateKey = item.ipnsPrivateKeyEncrypted; + return ( @@ <DetailRow label="IPNS Private Key"> <span className="details-value details-value--redacted"> - {item.ipnsPrivateKeyEncrypted.slice(0, 16)}...{item.ipnsPrivateKeyEncrypted.slice(-8)}{' '} + {encryptedIpnsPrivateKey.slice(0, 16)}...{encryptedIpnsPrivateKey.slice(-8)}{' '} (ECIES-wrapped) </span> </DetailRow>🤖 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 `@apps/web/src/components/file-browser/details/FolderDetails.tsx` around lines 74 - 77, The DetailRow component in FolderDetails uses terminology `ipnsPrivateKeyEncrypted` but should use the consistent terminology `encryptedIpnsPrivateKey` when rendering. Since the upstream FolderEntry contract cannot be changed, create a local const alias within the component that maps `item.ipnsPrivateKeyEncrypted` to `encryptedIpnsPrivateKey`, then update the slice() method calls in the DetailRow to use this new alias variable instead of directly accessing the item property.Source: Coding guidelines
crates/fuse/src/fs.rs (1)
541-545: 💤 Low valueConsider graceful handling when home directory is unavailable.
mount_point()usesexpect()which will panic if the home directory cannot be determined. While rare, this could occur in containerized or unusual environments.♻️ Optional: Return Result instead of panicking
#[cfg(any(feature = "fuse", feature = "winfsp"))] -pub fn mount_point() -> PathBuf { - dirs::home_dir() - .expect("Could not determine home directory") - .join("CipherBox") +pub fn mount_point() -> Result<PathBuf, String> { + dirs::home_dir() + .map(|h| h.join("CipherBox")) + .ok_or_else(|| "Could not determine home directory".to_string()) }🤖 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 `@crates/fuse/src/fs.rs` around lines 541 - 545, The mount_point() function currently uses .expect() which will panic if dirs::home_dir() returns None, causing a crash in environments where the home directory is unavailable. Modify the function to return Result<PathBuf, Error> instead of PathBuf, and replace the .expect() call with the ? operator to propagate the error gracefully to the caller, allowing proper error handling without panicking.
🤖 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 `@apps/web/src/components/file-browser/details/DetailsPrimitives.tsx`:
- Around line 19-33: The setCopied(true) call is executed regardless of whether
the copy operation succeeds or fails because it is placed outside the try-catch
block. Move the setCopied(true) call and the subsequent setTimeout for
timeoutRef.current into the try block, after the
navigator.clipboard.writeText(value) call succeeds. This ensures the "copied"
success state is only shown when the clipboard operation actually completes
successfully.
In `@apps/web/src/components/file-browser/details/VersionHistory.tsx`:
- Around line 36-37: In the VersionHistory component where the privateKey is
retrieved from the vault, instead of silently returning when the privateKey is
unavailable, surface a user-visible error message to the user. Replace the early
return statement with a call to show an error notification or toast message that
informs the user that the vault key is unavailable and prevents the download
action from proceeding. This ensures users understand why their download request
failed rather than appearing as a broken feature.
In `@crates/fuse/src/write_ops/implementation/file_data.rs`:
- Around line 153-164: Before allocating a new inode with
fs.inodes.allocate_ino(), add a check to verify that no child with the name
name_str already exists under the parent directory. This check should be
inserted after the parent_exists validation but before the inode allocation. If
a child with the same name already exists under parent, reply with an error
using libc::EEXIST to reject the duplicate name and return early, preventing the
creation of duplicate directory entries.
- Around line 105-123: The offset and data length are not validated before
calling handle.write_at, allowing negative offsets to wrap into huge sizes and
risking overflow when computing new_end. Validate the offset parameter before
calling handle.write_at to reject invalid (negative or out-of-range) values, and
replace the unchecked addition (offset as u64 + data.len() as u64) with
checked_add to safely detect overflow conditions. Return an appropriate error
response if validation fails, ensuring no mutations occur for invalid offsets.
In `@crates/fuse/src/write_ops/implementation/mkdir.rs`:
- Around line 27-58: The mkdir function checks that the parent exists as a
folder but does not verify whether a child with the same name already exists
under that parent before allocating a new inode. After the parent_exists check
succeeds, add a check to see if the fs.inodes collection already contains a
child entry with name_str under the parent inode, and if it does, call
reply.error(libc::EEXIST) and return early to prevent creating duplicate
directory names in the same parent directory.
In `@packages/sdk-core/src/folder/registration.ts`:
- Around line 61-65: The key-wrapping operations that compute
`ipnsPrivateKeyEncrypted` and `folderKeyEncrypted` using the `wrapKey` function
are currently positioned before the try-catch block. If either `wrapKey` call
fails, the exception won't be caught and sensitive buffers won't be properly
zeroed in the catch block. Move both key-wrapping statements (the
`ipnsPrivateKeyEncrypted` and `folderKeyEncrypted` assignments that call
`wrapKey` with `ipnsKeypair.privateKey` and `folderKey`) into the try block so
that any errors during encryption are properly handled and sensitive data is
cleaned up via the existing zeroization logic in the catch block.
---
Nitpick comments:
In `@apps/web/src/components/file-browser/details/FolderDetails.tsx`:
- Around line 74-77: The DetailRow component in FolderDetails uses terminology
`ipnsPrivateKeyEncrypted` but should use the consistent terminology
`encryptedIpnsPrivateKey` when rendering. Since the upstream FolderEntry
contract cannot be changed, create a local const alias within the component that
maps `item.ipnsPrivateKeyEncrypted` to `encryptedIpnsPrivateKey`, then update
the slice() method calls in the DetailRow to use this new alias variable instead
of directly accessing the item property.
In `@crates/fuse/src/fs.rs`:
- Around line 541-545: The mount_point() function currently uses .expect() which
will panic if dirs::home_dir() returns None, causing a crash in environments
where the home directory is unavailable. Modify the function to return
Result<PathBuf, Error> instead of PathBuf, and replace the .expect() call with
the ? operator to propagate the error gracefully to the caller, allowing proper
error handling without panicking.
In `@packages/sdk-core/src/__tests__/folder.test.ts`:
- Around line 607-632: The test `zeros key material and rethrows if TEE wrapping
fails` currently only covers zeroization when the third wrapKey call (TEE wrap)
fails. Add a new test case to cover the scenario where the initial user-key wrap
fails (either the first or second wrapKey call for wrapping the private key or
folder key for the user public key). This new test should verify that both
ipnsPriv and folderKey buffers are properly zeroed when createSubfolder
encounters an error during the early wrapping phase, ensuring the sensitive-data
cleanup contract is fully covered across all error paths in the function.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 849db75b-3716-496e-850a-1c59b94744d9
📒 Files selected for processing (57)
.planning/ROADMAP.md.planning/STATE.md.planning/phases/55-large-source-file-refactor/55-01-PLAN.md.planning/phases/55-large-source-file-refactor/55-01-SUMMARY.md.planning/phases/55-large-source-file-refactor/55-02-PLAN.md.planning/phases/55-large-source-file-refactor/55-02-SUMMARY.md.planning/phases/55-large-source-file-refactor/55-03-PLAN.md.planning/phases/55-large-source-file-refactor/55-03-SUMMARY.md.planning/phases/55-large-source-file-refactor/55-04-PLAN.md.planning/phases/55-large-source-file-refactor/55-04-SUMMARY.md.planning/phases/55-large-source-file-refactor/55-CONTEXT.md.planning/phases/55-large-source-file-refactor/55-DISCUSSION-LOG.md.planning/phases/55-large-source-file-refactor/55-PATTERNS.md.planning/phases/55-large-source-file-refactor/55-RESEARCH.md.planning/phases/55-large-source-file-refactor/55-SECURITY.md.planning/phases/55-large-source-file-refactor/55-VALIDATION.md.planning/phases/55-large-source-file-refactor/55-VERIFICATION.md.planning/todos/pending/2026-06-21-fuse-ipns-robustness-findings-from-pr538-review.md.planning/todos/pending/2026-06-21-zeroize-fuse-metadata-publish-key-params.mdapps/api/src/ipns/ipns-record.codec.tsapps/api/src/ipns/ipns.service.tsapps/desktop/src-tauri/src/commands/auth.rsapps/desktop/src-tauri/src/commands/vault.rsapps/desktop/src-tauri/src/fuse/mod.rsapps/desktop/src-tauri/src/fuse/prepopulate.rsapps/desktop/src-tauri/src/fuse/windows/mod.rsapps/web/src/components/file-browser/DetailsDialog.tsxapps/web/src/components/file-browser/details/DetailsPrimitives.tsxapps/web/src/components/file-browser/details/FileDetails.tsxapps/web/src/components/file-browser/details/FolderDetails.tsxapps/web/src/components/file-browser/details/VersionHistory.tsxcrates/fuse/src/content_ops.rscrates/fuse/src/events.rscrates/fuse/src/fs.rscrates/fuse/src/lib.rscrates/fuse/src/metadata.rscrates/fuse/src/operations.rscrates/fuse/src/platform/windows/content_fetch.rscrates/fuse/src/platform/windows/mod.rscrates/fuse/src/platform/windows/operations.rscrates/fuse/src/platform/windows/read_ops.rscrates/fuse/src/poll.rscrates/fuse/src/publish.rscrates/fuse/src/read_ops.rscrates/fuse/src/replay.rscrates/fuse/src/runtime.rscrates/fuse/src/write_ops.rscrates/fuse/src/write_ops/implementation/delete.rscrates/fuse/src/write_ops/implementation/file_data.rscrates/fuse/src/write_ops/implementation/mkdir.rscrates/fuse/src/write_ops/implementation/rename.rscrates/fuse/src/write_ops/mod.rspackages/sdk-core/src/__tests__/folder.test.tspackages/sdk-core/src/folder/index.tspackages/sdk-core/src/folder/load.tspackages/sdk-core/src/folder/metadata-ops.tspackages/sdk-core/src/folder/registration.ts
💤 Files with no reviewable changes (1)
- crates/fuse/src/write_ops.rs
Capture 6 pre-existing correctness/security findings surfaced by CodeRabbit's second pass on the phase-55 refactor PR. Each verified byte-identical to origin/main (code only moved, not introduced); deferred per HARD-06 to a hardening pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Entire-Checkpoint: 4225b9b49181
The release-target recompute commit was pushed by github-actions[bot] using the default token, which does not trigger workflow runs. This empty commit re-runs CI on the corrected release-please-config.json (stale pins resolved). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Entire-Checkpoint: 9e72f45cf5c5
FSM1
added a commit
that referenced
this pull request
Jun 21, 2026
* docs: file resolved hardening todos and add deferred-findings phases 56-58 Bookkeeping (verified against live code via parallel review agents): - File #5 IPNS S1/S2/S3 review to completed/ (bulk shipped in PR #529); residue tracked by phase 58 todos. - File #10 large-file refactor survey to completed/ (Tier-1/2 shipped in PR #538); re-capture the 14 open Tier-3 items. - Fold the per-file-IPNS-conflict todo into the PR #538 robustness todo (one unified site post-#538) and file to completed/. - Trim the metadata zeroize todo to its one remaining helper (spawn_metadata_publish). New phases via gsd-tools phase.add (HARD-07..09): - 56 FUSE & IPNS Durability Hardening — PR #538 deferred findings. - 57 API CID/Provider Hardening & Module Dedup. - 58 IPNS Signature-Verify Coverage — Phase 51 / PR #529 S1/S2 residue. Update ROADMAP (sections, progress table, execution order), REQUIREMENTS (HARD-07..09 + traceability), and STATE body. STATE frontmatter progress counts left unreconciled (pre-existing drift, todo #6). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Entire-Checkpoint: f7d4c3a60cd5 * docs(56): capture phase context Entire-Checkpoint: bd91ecff0d20 * docs(state): record phase 56 context session Entire-Checkpoint: e93a1c24b15f --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 55 — Large Source-File Refactor
Splits and dedups the Tier-1 + Tier-2 oversized source files into cohesive modules with the public surface frozen (requirement HARD-06). No
pnpm api:generate, consumers compile untouched, existing test suites stay green.What changed
Wave 1
crates/fuse/src/lib.rs(3276 LoC) decomposed into 6 sibling modules:runtime,events,publish,metadata,fs,replay. lib.rs is now a ~74-LoC crate root of cfg-gated module decls + re-exports (the two integration test modules deliberately stay in lib.rs).packages/sdk-core/src/folder/index.tsintoload.ts/metadata-ops.ts/registration.ts; extractedapps/api/src/ipns/ipns-record.codec.tsfromipns.service.ts; splitDetailsDialog.tsxintodetails/{VersionHistory,FileDetails,FolderDetails,DetailsPrimitives}.tsx.Wave 2
crates/fuse/src/write_ops.rs(1132 LoC) →write_ops/directory module behind the existingpub(crate) mod implementationfacade; deduped the bin-publish tail shared by unlink + rmdir. Movedload_vault_settingstocommands/vault.rsand factored thecomplete_auth_setuptail into a private finalize helper.Wave 3
content_ops.rs(crypto/IPNS helpers),content_fetch.rs(winfsp prefetch),poll.rs(PollResult+poll_filepointer_resolution), and sharedfuse/prepopulate.rs. The macOS sync-FUSE 3s read timeout was intentionally preserved (the sync wrapper was NOT hoisted into the 10scontent_opspath).handle_releasestays inread_ops.rs(durability invariant CR-04/D-04).Verification
cargo build -p cipherbox-fuse+cargo test -p cipherbox-fuse— 64 tests pass;cargo build -p cipherbox-desktopclean.@cipherbox/sdk-core211,@cipherbox/api893,@cipherbox/web63 +tsc --noEmitclean.publish_file_metadatamove.IpnsService,DetailsDialogprops, fuse crate re-exports) — unchanged. Noapi:generate(confirmed:packages/api-client/untouched).Cargo Check & Test (Windows)CI gate.Notes
#17(large-file refactor candidate survey) remains an open captured todo.maindoes not require signatures.🤖 Generated with Claude Code
Summary by CodeRabbit