# System Review — Drive · Storage · Network A critical pass over the design captured in [`docs/storage/`](./storage/), [`docs/drive/`](./drive/), [`docs/network/`](./network/), and [`docs/drive-storage-network.md`](./drive-storage-network.md). Identifies concrete flaws, proposes fixes, and aligns the system with the in-conversation requirements: bitpacker-encoded chunk-state bitsets, batched mesh requests, dehydrated drive templates, CDN fallback with reversed-ID URL layout, per-file policy flags, and lock-icon UX. The review is structured as 16 numbered findings, each with: **diagnosis** (the gap), **severity**, and **fix** (the concrete change). Where the fix has a corresponding new document, that doc is referenced and contains the formal spec. --- ## Severity legend - **Blocker** — without this, a real deployment will fail. Must land before any drive/network code ships. - **Major** — produces incorrect behavior, wasted bandwidth, or UX gaps that users will notice. Land early. - **Minor** — design hygiene, future-proofing, or convention. Land opportunistically. --- ## §1 — Chunk request granularity is wasteful — **Major** **Diagnosis.** The current spec (`drive-mesh-port`, `network-plugin-content-delivery`) describes `cd:request` as one bus event per chunk. A 1,000-chunk file with 800 missing chunks generates 800 envelopes, 800 round-trips, 800 routing decisions. Even with the 13-byte protocol header, the framing overhead is ~16 KiB of pure noise per file — and the worst case (every peer requesting the same 800 chunks) saturates the gossip layer. **Fix.** Define a unified `ChunkRequestEnvelope` with three forms: 1. `{ kind: 'whole' }` — request everything the holder has (replaces empty-array ambiguity). 2. `{ kind: 'specific'; chunkIds: number[] }` — sparse selection (use when ≤ 64 chunks). 3. `{ kind: 'bitset'; bytes: Uint8Array }` — dense selection (use when > 64 chunks; bytes encoded via `@suaveplan/bitpacker-compressed` for sparse runs). The selection threshold (64) matches the 8-byte break-even where `bitpacker-compressed` outperforms a `chunkIds` array. Spec'd in: [`drive/mesh-protocol.md`](./drive/mesh-protocol.md). --- ## §2 — Bitset advertisements not standardized to bitpacker — **Major** **Diagnosis.** `MESH-GOSSIP-01` says "gzipped bitset advertisements" but does not pin a schema or codec. With 10 bitpacker packages already shipped (`@suaveplan/bitpacker`, `bitpacker-compressed`, `bitpacker-network`, `bitpacker-schema`, `bitpacker-streaming`, `bitpacker-strategies`, `bitpacker-delta`, `bitpacker-game`, `bitpacker-validation`, `bitpacker-crypto`), every mesh implementer would otherwise pick a different encoding and fragment the protocol. **Fix.** Pin the canonical advertisement format: - Codec: `@suaveplan/bitpacker-compressed` for the bitset payload (RLE + gzip — typical 800-chunk presence map encodes to <100 bytes when most chunks are present). - Schema: `@suaveplan/bitpacker-schema` for envelope validation. - Wire transport: `@suaveplan/bitpacker-network` for the on-wire framing. Spec'd in: [`drive/mesh-protocol.md`](./drive/mesh-protocol.md) §3. --- ## §3 — No "dehydrated drive" concept — **Blocker** **Diagnosis.** The current docs assume drives are populated by writes. They have no concept of "here is the structure of the drive (folders, files, sizes, hashes) — go fetch the bytes from peers/CDN as needed." This is a real production scenario: a game ships with a drive template describing 2 GB of assets across 5,000 files; the user starts the game and the drive lazily populates from mesh peers, falling back to a CDN, while showing per-file progress in the UI. **Fix.** Introduce `DriveTemplate` — a server-signed, schema-versioned config containing: - Folder structure (recursive `FolderTemplate`). - Per-file `FileTemplate` stubs with `fileId`, `size`, `chunkCount`, `checksum`, and policy flags. - Optional inline-wrapped `fileKey` for immediate decryption, OR a `keyAuthority` peer ID for retroactive grant. - Server signature over canonical serialization, with `expiresAt` and a 16-byte `nonce` for replay protection. `Drive.hydrateFrom(template)` instantiates the DirIndex and FileLocationIndex from the template; per-file bytes arrive over the mesh/CDN later. Spec'd in: [`drive/dehydrated-drives.md`](./drive/dehydrated-drives.md). --- ## §4 — CDN fallback URL convention unspecified — **Major** **Diagnosis.** `drive-transport-https` is described as "server fallback when WebRTC isn't available" but no URL layout is defined. Without a convention, every deployment writes its own — and the chunks plugin can't fetch from a CDN it doesn't know how to address. **Fix.** Adopt the **reversed-ID URL scheme** for CDN-backed chunk pools: ```text ////_.chunk ``` Where `reversed = fileId.split('').reverse().join('')` and `chunkIdPadded` is the chunk ID zero-padded to 9 digits. Example with `fileId = "abcdef1234567890..."`: ```text /0/9/8/abcdef1234567890.../000000001.chunk ``` Reversing avoids hot directories when fileIds share prefixes (sequential timestamps, `f-` prefixes, etc.). Three directory levels × 16-character hex alphabet = 4,096 leaf directories — distributes 1 M files at ~250 per directory. `DriveTemplate.cdnFallback` carries `{ baseUrl, levels, padding }` so the layout is declarative, not hardcoded. Spec'd in: [`drive/dehydrated-drives.md`](./drive/dehydrated-drives.md) §4. --- ## §5 — Per-file policy flags missing — **Blocker** **Diagnosis.** The drive layer has no per-file controls for the three policies users care about: - **autoDownload** — should this file be eagerly fetched, or wait until first read? - **meshAllowed** — may we request this from peers, or must we hit the server only? - **cdnAllowed** — may we fall back to the CDN, or only the mesh / only local? Today these are global drive settings or implicit. The user's brief calls for them to be per-file, signed in the template, and enforced at runtime. **Fix.** Add `PerFilePolicy` to `FileLocation` and `FileTemplate`: ```ts interface PerFilePolicy { autoDownload: boolean; meshAllowed: boolean; cdnAllowed: boolean; retention?: 'permanent' | 'lru' | { maxAgeMs: number }; } ``` The drive enforces these in the read middleware: - `autoDownload=true` triggers a fetch on `Drive.hydrateFrom()` not on first read. - `meshAllowed=false` skips the `cd:request` bus emit. - `cdnAllowed=false` skips the `drive-transport-https` fallback. - All-disabled means the file is **local-only** — `read()` throws `DRIVE.UNAVAILABLE` if the bytes aren't present. Spec'd in: [`drive/dehydrated-drives.md`](./drive/dehydrated-drives.md) §6. --- ## §6 — No `getKeyStatus()` / no lock-icon UX contract — **Major** **Diagnosis.** The user's brief calls for visual lock states (locked, unlocked-with-key, unencrypted) per file. The drive surface has no method exposing this — the chunks plugin knows about encryption, the sharing plugin knows about KEKs, but `Drive.list()` returns DirEntries that don't aggregate the answer. **Fix.** Add `Drive.getKeyStatus(path): KeyStatus` returning: ```ts type KeyStatus = 'unencrypted' | 'encrypted-locked' | 'encrypted-unlocked'; ``` And include it on every `DirEntry` returned by `list()`: ```ts interface DirEntry { // ... existing fields ... keyStatus?: KeyStatus; // present for type === 'file' presence?: ChunkPresence; // see §7 } ``` UX maps directly: 🔒 for `encrypted-locked`, 🔓 for `encrypted-unlocked`, no icon for `unencrypted`. Spec'd in: [`drive/architecture.md`](./drive/architecture.md) (will be updated). --- ## §7 — `Drive.list()` lacks presence/progress data — **Major** **Diagnosis.** The user wants `███░░░░░░░ 33%` next to file listings. Currently `Drive.list()` returns DirEntries with `size` and `mtime` but no `chunksPresent / chunksTotal`. The mesh plugin tracks this internally but doesn't expose it. **Fix.** Add `ChunkPresence` to every file DirEntry: ```ts interface ChunkPresence { chunksPresent: number; // count of chunks locally available chunksTotal: number; // total chunks in the file sourceHints: SourceHint[]; // ordered: where missing chunks could come from } type SourceHint = 'local' | 'mesh' | 'cdn'; ``` Renderable as a progress bar in any UI. Updated lazily — list() is allowed to return a snapshot from the last gossip cycle, not a fresh probe. Spec'd in: [`drive/architecture.md`](./drive/architecture.md) update. --- ## §8 — Retroactive key distribution undefined — **Major** **Diagnosis.** The user's brief: "optional decryption keys could be included as part of the signed config, or would have to be retro-actively added to the drive for files to be unlocked." The current sharing plugin handles partition KEKs but not per-file `fileKey` retro-grants from arbitrary peers. **Fix.** Two distribution paths, both first-class: 1. **Inline grant** (`FileTemplate.encryption.keyDistribution.kind === 'inline'`) — the wrapped `fileKey` is in the signed template. Drive can decrypt immediately on `hydrateFrom()`. 2. **Retroactive grant** (`FileTemplate.encryption.keyDistribution.kind === 'retroactive'`) — template names a `keyAuthority` (peer pubkey). The peer issues a `KeyGrantEnvelope` whenever access is granted: ```ts interface KeyGrantEnvelope extends MessageEnvelope { type: 'key:grant'; payload: { fileId: string; wrappedKey: Uint8Array; // wrapped to recipient X25519 pubkey issuer: string; // peer pubkey issuerSignature: Uint8Array; // Ed25519 over (fileId, wrappedKey, recipient, expiresAt) expiresAt: number; }; } ``` On receipt, the drive validates the issuer signature against the template's `keyAuthority`, unwraps, and stores the `fileKey` in its KeyRing. The lock icon flips from 🔒 to 🔓. Spec'd in: [`drive/dehydrated-drives.md`](./drive/dehydrated-drives.md) §7. --- ## §9 — No cache retention strategy — **Major** **Diagnosis.** A drive that auto-downloads everything will fill the disk forever. The current docs don't address GC, LRU, or retention windows. Users in dehydrated-drive scenarios (game assets, media libraries) need predictable storage budgets. **Fix.** Add three retention modes to `PerFilePolicy.retention`: - `'permanent'` — never evict (default for explicit user-saved files). - `'lru'` — drive maintains a global byte budget; LRU evicts chunks (not whole files; preserves manifest+index for re-download). - `{ maxAgeMs }` — chunks older than N ms eligible for eviction. Plus `DriveOptions.maxLocalCacheBytes` for the global budget. Eviction is at chunk granularity; the file remains visible (with reduced `chunksPresent`) and re-fetches on read. Spec'd in: [`drive/dehydrated-drives.md`](./drive/dehydrated-drives.md) §6. --- ## §10 — Mesh advertisement trust unbounded — **Major** **Diagnosis.** `MESH-GOSSIP-01` says peers advertise their chunk inventory. A malicious or buggy peer can advertise chunks it doesn't have, wasting our request budget. The current spec has `MESH-SCORE-01` (bad-sig sets score to `-Infinity`), but only after we've already paid the round-trip. **Fix.** Probabilistic verification + adaptive trust: - On a peer's first N requests for a given file, accept their advertisements as truth. - After the first M responses, compute their "advertised-but-missing" rate. If > 5%, demote to "unverified" — their advertisements are still listened to, but probabilistically skipped (P(skip) ∝ miss rate). - Bad sig immediately drops to `-Infinity` (existing `MESH-SCORE-01`). - Trust score per `(peerId, fileId)` so a peer good for one file doesn't get blanket trust. Spec'd in: [`drive/mesh-protocol.md`](./drive/mesh-protocol.md) §6. --- ## §11 — Chunk response doesn't piggyback presence — **Minor** **Diagnosis.** The current `cd:response` returns chunk bytes only. But the responding peer is in the perfect position to tell us its full presence bitset for this file — saving us a separate `cd:advertise` round-trip in many cases. **Fix.** Add `presence: Uint8Array` (bitpacker-compressed bitset) to every `cd:response` payload. Receiver updates its peer-index for that `(peerId, fileId)` for free. Spec'd in: [`drive/mesh-protocol.md`](./drive/mesh-protocol.md) §4. --- ## §12 — CDN write-side authentication unspecified — **Major** **Diagnosis.** The CDN URL convention (§4) covers reads. Writes are unspecified — anyone with the URL would otherwise be able to upload chunks, poisoning the cache. The current `drive-transport-https` proposal doesn't address this. **Fix.** CDN writes use **server-signed upload tokens**, not direct PUT: - The drive POSTs a chunk to `/upload` with a `CdnUploadToken` (server-signed, 60-second expiry). - Token contains `(fileId, chunkId, chunkHash, uploaderPubkey)`. CDN validates the token, validates the chunk against the hash, and stores at the reversed-ID URL. - Ed25519 sig from the chunk's writer is preserved end-to-end; readers verify it when fetching. CDNs that don't want to validate signatures can decline writes (read-only CDN); the drive falls back to peer storage in that case. Spec'd in: [`drive/dehydrated-drives.md`](./drive/dehydrated-drives.md) §5. --- ## §13 — Drive template lacks replay protection — **Major** **Diagnosis.** A `DriveTemplate` signed once is valid forever unless its `expiresAt` is hit. An attacker could re-issue an old (less-restrictive) template they captured. There's no nonce or sequence number making each issuance unique. **Fix.** Required `nonce: Uint8Array(16)` and `issuedAt: number` on every template. Signature covers both. Drives reject templates with: - `nonce` already seen (16-byte LRU of recent nonces, ~1 KiB cost). - `issuedAt` more than 24h in the past. - `expiresAt` in the past (existing). Spec'd in: [`drive/dehydrated-drives.md`](./drive/dehydrated-drives.md) §3. --- ## §14 — `kind: 'inline'` files miss signature scaffolding — **Minor** **Diagnosis.** The `FileLocation.storageHints` `kind: 'inline'` case (small files <1 KiB stored directly in the FileLocationIndex) skips the chunks-plugin pipeline. That means no per-chunk Ed25519 sig and no manifest sig. For internal indexes and tiny configs this is fine, but if an inline file is shared, the integrity story breaks. **Fix.** Inline files must still carry a signature: `{ kind: 'inline'; bytes: Uint8Array; sig: Uint8Array }` where `sig = Ed25519-sign(driveSecretKey, fileId ‖ bytes)`. Verify on read. The cost is +64 bytes per inline file; trivial. Spec'd in: [`drive/index-and-paths.md`](./drive/index-and-paths.md) (will be updated). --- ## §15 — No "stuck" state surface — **Minor** **Diagnosis.** When a chunk is unavailable from local + mesh + CDN, the drive read currently throws `DRIVE.UNAVAILABLE`. But the UI wants to know *before* the user clicks: "this file is partially-stuck — 12 chunks unfetchable." Today there's no signal. **Fix.** Add `ChunkPresence.stuckChunks: number` (chunks tried-and-failed across all configured sources within the last N minutes). Reset on retry-success or peer-population. UI can show ⚠ next to a file with stuckChunks > 0. Spec'd in: [`drive/architecture.md`](./drive/architecture.md) update. --- ## §16 — Folder template recursion unbounded — **Minor** **Diagnosis.** `FolderTemplate.children: (FolderTemplate | FileTemplate)[]` is recursive with no depth cap. A maliciously-crafted template could blow the stack on validation. **Fix.** Cap recursion at depth 32 (conservative; real filesystems rarely exceed 16). Validate during template parsing; reject deeper. The depth cap is in `DriveTemplateSchema` (Zod). Spec'd in: [`drive/dehydrated-drives.md`](./drive/dehydrated-drives.md) §3. --- ## Summary by severity | Severity | Count | Findings | |---|---|---| | **Blocker** | 2 | §3 (dehydrated drive), §5 (per-file policy flags) | | **Major** | 9 | §1, §2, §4, §6, §7, §8, §9, §10, §12, §13 | | **Minor** | 5 | §11, §14, §15, §16 | The two **Blockers** are about features the user explicitly asked for and the system does not yet have. They produce two new docs: - [`drive/dehydrated-drives.md`](./drive/dehydrated-drives.md) — `DriveTemplate`, policy flags, CDN convention, key distribution. - [`drive/mesh-protocol.md`](./drive/mesh-protocol.md) — chunk request envelope forms, bitset advertisements, response piggyback. The **Major** items either land inside those two docs or as additive sections to existing docs (`drive/architecture.md` gains `getKeyStatus`/`getPresence`; `drive/index-and-paths.md` gets the inline-sig fix). The **Minor** items are hardening; they ride on the same change set without their own structural impact. --- ## Implementation order 1. **Land `enterprise-types-cross-cutting`** — already archived 2026-05-02. ✓ 2. **Resolve `enterprise-drive` vs `drive-port`** — already SUPERSEDED 2026-05-02. ✓ 3. **Author new spec changes** for the two new docs: - `drive-mesh-protocol-v1` — formalize the request/response/advertise envelopes (§1, §2, §10, §11). - `drive-template-v1` — formalize `DriveTemplate`, policy flags, key distribution, CDN convention (§3, §4, §5, §8, §9, §12, §13, §16). 4. **Update existing capability specs** — `openspec/specs/drive/spec.md` adds `getKeyStatus` / `getPresence` requirements (§6, §7); `openspec/specs/network/spec.md` references the new mesh-protocol spec (§1, §2, §10, §11). 5. **Land `drive-port` + `drive-plugin-chunks-port` + `drive-mesh-port` + `drive-transport-https-port`** in dependency order, each conforming to the new specs. 6. **Land `network-plugin-content-delivery`** consuming the mesh-protocol envelopes. 7. **Port the discovery + transport plugins** from `main`. --- ## What this review does NOT touch Three areas where the existing design is sound and needs no change: - **Storage capability matrix.** Five flags (`partialReads`, `partialWrites`, `nativeStreams`, `atomicWrites`, `contentAddressable`) cover the access patterns; no new flag needed for any finding here. - **Tier model & failover.** Tier 0/1/2 + four hosting models + sibling-pocket structure already accommodates the user's mesh/online-player scenarios. The mesh-protocol changes plug in without altering topology. - **Cryptographic primitives.** AEAD + Ed25519 + BLAKE3 + X25519 sealed-box is sufficient. No new primitives proposed. The stack as documented is a sound foundation. The findings above harden it for the production scenarios the user described — they are sharp edges to round, not load-bearing rewrites.