docs: comprehensive documentation update for phases 28-37#419
Conversation
- DEFERRED.md: add deferred items from phases 28-37 (observability, upload pipeline, code quality, infrastructure hardening) - FEATURES.md: add batch upload, batch download, media preview, streaming playback, Web Worker encryption, structured logging; update E2E test suites (14 web, 11 SDK, load tests); fix test coverage mapping - README.md: update features list, project structure, tech stack, and documentation links to reflect current state - FILESYSTEM_SPECIFICATION.md: new document covering naming rules, case handling, reserved names, size limits, folder depth, versioning constraints, FUSE mount specifics, and known cross-platform gaps Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: a5f4c4dacebd
WalkthroughThis PR updates planning and project documentation: expands deferred milestone scope, restructures deferred items, revises the feature/E2E matrix, extends the README and project layout, adds a cross-platform filesystem specification, appends roadmap phases, and updates codebase concerns and testing matrices. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
Implemented in Phase 37 (encrypt.worker.ts + EncryptionWorkerService). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: bd705e5bbe92
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
README.md (2)
11-11:⚠️ Potential issue | 🟠 MajorAuth identity statement is too strong and currently inaccurate.
“Same vault regardless of login method” implies automatic cross-method identity convergence. In practice, cross-method sharing requires explicit auth-method linking. Please narrow this wording.
Based on learnings, “There is no automatic convergence — cross-method identity sharing requires explicit user-initiated linking via the Settings page.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 11, The sentence claiming “the same user always gets the same vault regardless of login method” is too strong and inaccurate; update the README text containing that phrase to clarify that there is no automatic cross-method convergence and that cross-method identity sharing requires explicit user-initiated linking (e.g., via the Settings page), replacing or rewording the clause to reflect explicit linking is required.
50-50:⚠️ Potential issue | 🟠 MajorZero-knowledge architecture text conflicts with current server crypto-material stance.
Line 50 says the API “stores encrypted keys,” which conflicts with the current documented zero-crypto-material server model. Please update this sentence to reflect the post-migration state precisely.
Based on learnings, “As of Phase 20 … the server stores ZERO crypto material by design.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 50, Update the README sentence that currently reads "stores encrypted keys" under the "Zero-knowledge relay" description to reflect the post-migration design: replace that phrase with a clear statement such as "stores ZERO crypto material by design (as of Phase 20)" and ensure the surrounding sentence still conveys that the client encrypts locally and the API only routes IPFS/IPNS operations without holding any crypto material; keep the existing link to ARCHITECTURE.md intact for full details.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/FILESYSTEM_SPECIFICATION.md`:
- Line 150: Update the sentence that says "staging IPFS gateway" to reflect that
file content is fetched via Kubo RPC API endpoints (not an IPFS gateway); e.g.,
change wording in the rationale paragraph that describes fetching on open() and
caching so it states "fetched from IPFS via Kubo RPC endpoints" and add a short
clause clarifying "the IPFS gateway is not used" while keeping the 120‑second
timeout rationale and the note that subsequent read() calls are served from
cache.
- Line 13: Reword the sentence beginning "The strictest platform constraint
wins." to state it as the target invariant (CipherBox aims to enforce
Windows-compatible filename rules across platforms) and immediately follow it
with a concise "Current deviations" subsection that enumerates the documented
exceptions (e.g., case-sensitive web duplicates, permitted Windows-invalid
characters in some contexts) and their remedial tracking status; update the
enforcement/constraint checklist language elsewhere (the sections referencing
case-sensitive web duplicates and Windows-invalid characters) to clearly mark
them as known deviations from the target invariant rather than contradictory
enforcement requirements so readers can distinguish the intended invariant from
current gaps.
- Around line 44-46: Update the document to reference the actual helper function
names (is_macos_reserved_name, is_windows_reserved_name,
is_system_reserved_name) instead of the incorrect
helpers.rs:is_platform_special() / is_windows_special(), and explicitly mark
each pattern row as “implemented” when covered by those functions or
“planned/superset” when not yet implemented; also update the “Cross-platform
filter” table header and the entries referenced in the later section (lines
~65-76) to use the same corrected symbol names and implementation status so the
docs match the codebase.
In `@README.md`:
- Line 64: Update the README table row that currently reads "Web Crypto API
(AES-256-GCM/CTR, ECIES secp256k1)" to clearly separate native Web Crypto usage
from library implementations: replace the cell content to mention Web Crypto API
for AES-256-GCM/CTR and HKDF-SHA256 and explicitly note eciesjs for ECIES
secp256k1 (or use the concise form "Web Crypto API (AES-256-GCM, AES-256-CTR,
HKDF-SHA256) + eciesjs"); edit the table row string that contains "Web Crypto
API (AES-256-GCM/CTR, ECIES secp256k1)" accordingly.
---
Outside diff comments:
In `@README.md`:
- Line 11: The sentence claiming “the same user always gets the same vault
regardless of login method” is too strong and inaccurate; update the README text
containing that phrase to clarify that there is no automatic cross-method
convergence and that cross-method identity sharing requires explicit
user-initiated linking (e.g., via the Settings page), replacing or rewording the
clause to reflect explicit linking is required.
- Line 50: Update the README sentence that currently reads "stores encrypted
keys" under the "Zero-knowledge relay" description to reflect the post-migration
design: replace that phrase with a clear statement such as "stores ZERO crypto
material by design (as of Phase 20)" and ensure the surrounding sentence still
conveys that the client encrypts locally and the API only routes IPFS/IPNS
operations without holding any crypto material; keep the existing link to
ARCHITECTURE.md intact for full details.
🪄 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: 8dff0d4c-a99f-4773-9b67-3839c35b1cbf
📒 Files selected for processing (4)
.planning/DEFERRED.md.planning/FEATURES.mdREADME.mddocs/FILESYSTEM_SPECIFICATION.md
- Remove "Error tracking service (Sentry)" — implemented as Grafana Faro in Phase 30 (apps/web/src/lib/faro.ts); add to Implemented table - Remove "Session replay" — anti-feature in a zero-knowledge product, never in scope - Remove "Production Kubo deployment hardening" — no production environment exists; premature to list Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 757e613e56ac
FUSE callbacks are thin plumbing between OS and Rust SDK — unit tests would require mocking the host OS, which is unreliable. Desktop E2E suite covers these paths at the appropriate integration level. Update CONCERNS.md to mark as won't-fix with rationale. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 42ca1ee01769
Legacy vault v1 columns no longer exist — staging was reset and the full schema migration baseline does not include them. Nothing to drop. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: f7582e9de5ef
Systematic verification of every item against the codebase found: - Desktop FUSE CTR streaming: implemented in crates/fuse/src/operations.rs - Linux FUSE mount: implemented in crates/fuse/src/platform/linux.rs - Desktop SDK adoption: not applicable, desktop uses Rust FUSE by design - Batch upload secondary pin warning: pin:secondaryFailed emitted in P37 - Per-file IPNS conflict detection: implemented via per-file IPNS in P12.6 - User-configurable bin retention: RECYCLE_BIN_RETENTION_DAYS env var Also updated stale notes on desktop device approval polling and platform code signing to reflect actual partial implementation state. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 03a675cdbd2a
The env var RECYCLE_BIN_RETENTION_DAYS is operator-level config, not an end-user setting. The deferred item is about per-user control over their own retention period. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 114f5db59741
Faro ships errors/warnings to Grafana Cloud with privacy scrubbing. The original "Datadog, Loki" framing is outdated -- Faro was the chosen solution, implemented in Phase 30. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 818957bd593e
DEFERRED.md: - Remove DTS circular dependency -- devDep in crypto, production dep in core; no runtime circular, this is by design - Remove useFileOperations.ts decomposition -- 515 lines with 3 cohesive callbacks is appropriate size - Update notes on remaining items with current line counts - Move remote log shipping to implemented -- Grafana Faro ROADMAP.md: - Add Phase 38: retire folder.service.ts and bin.service.ts - Add Phase 39: user-configurable vault parameters - Add phases 36-39 to progress table Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 89dba2895b55
The @cipherbox/crypto devDependency on @cipherbox/core exists only for one test file importing deriveRegistryIpnsKeypair. Should have been cleaned up during Phase 19.1 restructuring. Refactor to use hardcoded test vectors instead. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 81b16535c9ef
User-configurable vault parameters now covers: - Bin retention period (currently hardcoded 30 days) - Delete behavior (soft delete to bin vs hard delete) - Versioning defaults (max versions, cooldown period) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 114c8ed1ad9e
Planned features: bin retention period, delete behavior (soft/hard), max versions per file, version cooldown period. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 445d7b2087c0
- FILESYSTEM_SPECIFICATION.md: add Linux desktop as 3rd platform with libfuse3 backend and NFC-normalized case-sensitive lookups - README.md: add Linux to desktop platforms and tech stack - ARCHITECTURE.md: update TEE from "Nitro fallback planned" to Phala CVM; fix frozen specs path to .planning/milestones/m0/ - DATABASE_EVOLUTION_PROTOCOL.md: add 6 missing migrations from phases 15-27, add share_invites and pin_migrations tables, update to 14 tables - CLAUDE.md: fix 00-Preliminary-R&D paths to .planning/milestones/m0/, update TEE description, fix republish interval from 3h to 6h Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 0c601c4cffe3
macOS: Finder + Terminal. Linux: file manager + terminal. Windows: Explorer + PowerShell + Git Bash. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: ccdc3e408477
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: a2d274ef6f18
Large media files use AES-256-CTR for both encryption and decryption, not just playback. Updated encryption modes table, rationale sections. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: db2b21f29821
Entire-Checkpoint: 295a09934929
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
README.md (1)
64-64:⚠️ Potential issue | 🟡 MinorSeparate Web Crypto primitives from ECIES library usage in the Crypto row.
Line 64 currently implies ECIES is provided by Web Crypto API. In this repo, ECIES secp256k1 is provided by
eciesjs, while Web Crypto is used for AES/HKDF.#!/bin/bash # Verify docs vs implementation references for ECIES and Web Crypto usage. rg -n -C2 'Web Crypto API|ECIES|eciesjs' README.md apps packages .github || trueBased on learnings: “Use Web Crypto API for AES-256-GCM, AES-256-CTR, and HKDF-SHA256. Use eciesjs for ECIES secp256k1 key wrapping.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 64, The README's Crypto table conflates Web Crypto primitives with the ECIES implementation; update the Crypto row so it clearly separates Web Crypto API usage (AES-256-GCM, AES-256-CTR, HKDF-SHA256) from ECIES (implemented via the eciesjs library, secp256k1). Edit the line that currently reads "Web Crypto API (AES-256-GCM/CTR, ECIES secp256k1)" to instead list Web Crypto primitives and explicitly state "ECIES (secp256k1) via eciesjs" so readers can map implementation to libraries correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.planning/codebase/CONCERNS.md:
- Around line 225-229: Update the outage impact wording in the `@phala/dstack-sdk`
section to state IPNS records expire after 48 hours (not 72); change the
"72-hour TTL" text to "48-hour TTL" so it matches the TEE signer configuration
constant TEE_RECORD_LIFETIME_MS and the actual signer behavior.
In @.planning/DEFERRED.md:
- Line 3: Update the `Last updated` metadata in .planning/DEFERRED.md to a
non-future audit date (change "Last updated: 2026-03-31" to "Last updated:
2026-03-30") and ensure any matching footer/header occurrences of the `Last
updated` line are changed to the same date so the document's metadata is
consistent with the PR creation timestamp.
---
Duplicate comments:
In `@README.md`:
- Line 64: The README's Crypto table conflates Web Crypto primitives with the
ECIES implementation; update the Crypto row so it clearly separates Web Crypto
API usage (AES-256-GCM, AES-256-CTR, HKDF-SHA256) from ECIES (implemented via
the eciesjs library, secp256k1). Edit the line that currently reads "Web Crypto
API (AES-256-GCM/CTR, ECIES secp256k1)" to instead list Web Crypto primitives
and explicitly state "ECIES (secp256k1) via eciesjs" so readers can map
implementation to libraries correctly.
🪄 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: ff767fc5-7b6e-4c44-8936-c6a814519c4c
📒 Files selected for processing (10)
.claude/claude.md.planning/DEFERRED.md.planning/FEATURES.md.planning/ROADMAP.md.planning/codebase/CONCERNS.md.planning/codebase/TESTING.mdREADME.mddocs/ARCHITECTURE.mddocs/DATABASE_EVOLUTION_PROTOCOL.mddocs/FILESYSTEM_SPECIFICATION.md
✅ Files skipped from review due to trivial changes (4)
- docs/DATABASE_EVOLUTION_PROTOCOL.md
- .planning/ROADMAP.md
- docs/FILESYSTEM_SPECIFICATION.md
- .planning/codebase/TESTING.md
🚧 Files skipped from review as they are similar to previous changes (1)
- .planning/FEATURES.md
- FILESYSTEM_SPECIFICATION.md: reword design principle as target invariant with known gaps, fix "IPFS gateway" to "Kubo RPC API" - README.md: separate Web Crypto API from eciesjs in crypto table - CONCERNS.md: fix IPNS record TTL from 72h to 48h Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 2fe8397de199
Web app is thin wrappers around SDK -- 14 Playwright E2E suites cover all critical flows. SDK/SDK-core have comprehensive unit tests where the actual logic lives. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Entire-Checkpoint: c9d49186dcb0
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Line 21: README claims a hard cap of "up to 10 versions" but the metadata
contract in packages/core/src/file/types.ts defines versions?: VersionEntry[]
with no max; either soften the README or enforce the cap. Fix by either (A)
updating README.md to remove or soften the "up to 10 versions" language and
reference the actual metadata shape (versions?: VersionEntry[]) or (B) implement
and document enforcement where versions are created/validated (e.g., add a
maxVersions constant and enforce it in the code paths that append to
VersionEntry arrays such as the file versioning/commit handlers and any
validation functions that construct VersionEntry[]), and update the types or add
runtime validation to reflect the max; ensure you reference versions?:
VersionEntry[] and the module packages/core/src/file/types.ts when making the
change.
- Line 23: The README claim about "30-day soft-delete" is not reflected in code:
inspect packages/core/src/bin/types.ts (the DeletedRecord type / deletedAt
field) and either update README to remove or qualify the 30-day retention
language, or implement an enforcement mechanism (add an expiresAt/ttl field to
the type and a cleanup job that permanently deletes records after 30 days, or
point README to the service that performs TTL cleanup). Make the change
consistent by referencing the DeletedRecord/deletedAt symbol in types.ts and
ensure the README text mentions the exact enforcement point (code path or
background job) or is reworded to avoid implying automatic enforcement.
🪄 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: e3683d5e-753e-40e7-bab5-440465c5c4cc
📒 Files selected for processing (3)
.planning/codebase/CONCERNS.mdREADME.mddocs/FILESYSTEM_SPECIFICATION.md
✅ Files skipped from review due to trivial changes (1)
- docs/FILESYSTEM_SPECIFICATION.md
🚧 Files skipped from review as they are similar to previous changes (1)
- .planning/codebase/CONCERNS.md
Summary
Comprehensive documentation audit and update covering all changes from phases 28-37 plus roadmap planning for phases 38-39.
New documentation
Updated documentation
00-Preliminary-R&D/paths, TEE description, republish intervalPlanning updates
Test plan
apps/api/src/migrations/tests/🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation