Skip to content

feat: web rotation UX and durable anti-rollback client state#587

Merged
FSM1 merged 103 commits into
mainfrom
feat/web-integration-rotation-ux-and-durable-client-state
Jul 2, 2026
Merged

feat: web rotation UX and durable anti-rollback client state#587
FSM1 merged 103 commits into
mainfrom
feat/web-integration-rotation-ux-and-durable-client-state

Conversation

@FSM1

@FSM1 FSM1 commented Jul 2, 2026

Copy link
Copy Markdown
Owner

Phase 68 — Web Integration: Rotation UX and Durable Client State

The web app now uses rotateReadFromNode for every revocation-triggering mutation, persists a durable IndexedDB generation + sequence high-water that survives page reload, and reconciles folderTree before any rotation publish.

What's in here (12 plans)

SDK (packages/sdk, packages/sdk-core)

  • Durable {nodeId → highestGeneration} / {nodeId → highestSeq} monotonic-max state machine behind an injected HighWaterStore seam; enforceResolved throws GenerationRegressionError / SequenceRegressionError fail-closed on any regression, with a versionFloor gate for cold-device first contact (ROT-07, SC#1/SC#4)
  • executeLazyRotation and the addShareKeys/reWrapForRecipients per-mutation fan-out are deleted; delete/move/rename call rotateReadFromNode on scope exit (SC#2)
  • Reconcile-before-publish: any sequenceNumber mismatch defers with ReconcileStaleError — never publishes against superseded state (SC#3, closes the silent-missed-revocation class from #489/#494)
  • reconcileFolderSequence routes every mutation resolve through enforceResolved, making the fail-closed gate live in real navigation/mutation flows (68-11 gap closure)
  • rotateReadFromNode returns the root's rotated {readKey, generation, sequenceNumber} and performScopeExitRotation writes it back into folderTree, so a same-session second mutation self-heals instead of deferring until reload (68-12 gap closure)
  • Owner reconcile: reMintGrantsRootedAt re-wraps grants for surviving recipients and deletes (never re-mints) revoked ones

Web (apps/web)

  • IndexedDB-backed HighWaterStore adapter with in-memory degradation (D-08), wired as the rotationHighWater singleton in useAuth
  • Durable rotation job checkpoints (cipherbox-rotation-jobs, metadata-only — never key bytes) with resume-after-reload, plus navigator.locks multi-tab leader election with a designed-safe fallback (D-09)
  • Rotation status badge (idle / root-cut / tail-walk / resuming; aria-live="polite", non-interactive) and the mutation-failure UX classifier: bounded defer retries (~5 attempts / ~30s) then terminal fail-closed toast; revoked co-writer gets a terminal no-action toast (D-01–D-06, WRITE-03)

API (apps/api)

  • PATCH /shares/:shareId/grant — owner-only update of readDescriptorRef/rootGeneration after rotation (D-10/D-11), with regenerated api-client

Tests

  • rotation-durability.spec.ts + rotation-ux.spec.ts web-e2e specs: real-browser reload durability proof, relay-regression rejection via captured/replayed IPNS record bytes, badge lifecycle, toast contract (run on the push-to-main web-e2e gate)
  • SDK/API unit coverage for all of the above (high-water 20 cases, client rotation 24, updateGrant route)

Ship-pass results

  • Verification: 14/14 must-haves (re-verified after 68-11/68-12 gap closure)
  • Security: 41/41 threats closed (68-SECURITY.md)
  • Nyquist validation: 0 gaps
  • SDK E2E gate: 12/12 active tests green across all 5 live suites (11 quarantined v1 suites remain describe.skip from #578)
  • Ship fixes on top of the phase: a zeroization-order bug in enumerateMoveDescendants (depth-2+ move descendants misclassified unreadable) fixed with a red-green regression pair, plus dead-code removal orphaned by the rotation cutover

Deferred (todos in .planning/todos/pending/)

  • WRITE-03 Refresh-access path has no live production trigger (pre-existing Phase 65/66 seam)
  • Web vitest is not run in CI; ipns.service.test.ts broken locally (pre-existing, fails on main too)
  • Retire dead SDK share scaffolding (ShareCallbacks, addShareKeysFn) + owner-reconcile double-fetch

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added rotation progress and resume indicators in the app header, plus actionable toast buttons for key workflows like refresh and retry.
    • Improved sharing and sync flows with live server-backed data and better grant handling.
  • Bug Fixes

    • Hardened sync and mutation flows to reject stale data, prevent unsafe rollbacks, and recover more reliably after reloads or interrupted rotations.
    • Removed outdated share-key fan-out behavior for a cleaner, more consistent experience.

FSM1 added 30 commits July 1, 2026 16:26
Entire-Checkpoint: 5d783d7577ae
Entire-Checkpoint: 2ebaa765350e
Entire-Checkpoint: 413c22b4ce10
Entire-Checkpoint: e35021f660ee
Entire-Checkpoint: 1ced91067630
… SDK, web-e2e coverage)

Entire-Checkpoint: 50ef03eff3dd
…machine

- monotonic-max generation and seq floors over an injected HighWaterStore
- seedFromGrant owner-vouched first-contact seed (only raises, never lowers)
- restart-persistence: a fresh instance over the same backing store observes prior floors
- malformed stored values (negative, fractional, NaN, non-numeric) treated as absent
- createRotationHighWater exposes monotonic-max generation and seq floors
  over an injected HighWaterStore seam, no in-instance cache
- seedFromGrant raises the generation floor only when higher (owner-vouched)
- V5 fail-closed validation: malformed stored values treated as absent
- below-floor seq/generation throws distinguishable regression errors, no bump
- non-regressing values bump both floors and resolve normally
- cold-device first-contact versionFloor gate (below rejects, at-or-above seeds)
- GenerationRegressionError and SequenceRegressionError are instanceof-distinguishable
- Add UpdateGrantDto with hex-validated readDescriptorRef and numeric-string rootGeneration
- Add PATCH :shareId/grant controller route mirroring updateShareItemName idiom
- Add SharesService.updateGrant with sharerId ownership check, 403 for non-owner, 404 for unknown share
- Extend Notification type with action?: label onClick field
- Render action button in NotificationToast between message and x dismiss
- Skip auto-dismiss for terminal error toasts carrying an action
- Add notification-action-btn CSS for muted default / green hover styling
…port

- GenerationRegressionError and SequenceRegressionError, instanceof-distinguishable
- enforceResolved rejects seq/generation regressions, applies cold-device
  versionFloor gate on first contact, bumps monotonic-max otherwise
- pure pass/throw pre-unseal gate, never emits an AAD/unseal parameter
- exported from the SDK barrel for the web IndexedDB adapter (68-06)
- Regenerate openapi.json, generated shares client, and updateGrantDto model via pnpm api:generate
- Add useRotationStore with idle/root-cut/tail-walk/resuming status
- Presentation-only; drivers wired in 68-08 call the setters
…achine

- documents the RED->GREEN TDD cycle for createRotationHighWater and enforceResolved
- records coverage mapping for ROT-07 deliverables
- Add RotationStatusBadge non-interactive status pill component
- Mount badge in header-right before UserMenu
- Add rotation-status-badge styles reusing header-search-btn tokens
Add SUMMARY.md documenting the notification action affordance and
rotation status badge primitives built for the 68-UI-SPEC contract.
Rewire the stub fetch functions in share.service.ts to call the generated
sharesControllerGetReceivedShares/sharesControllerGetSentShares endpoints,
and extend the ReceivedShare/SentShare store types with readDescriptorRef,
rootGeneration (parsed to number with an isFinite fail-closed guard), and
rootNodeId. This is the D-07 data path ROT-07s durable generation floor
seeds from.
Remove the useAuth.ts shareCallbacks config (getCoveringShares/addShareKeys)
and no-op the useSharedNavigation.ts addShareKeysFn seed callback now that
the web addShareKeys fan-out function is deleted (SC#2 / D-12). Both call
sites always threw the Phase-68-deferred stub, so this changes no observed
behavior while retiring the legacy per-mutation key-wrap wiring. The SDK's
own ShareCallbacks/SharedFolderState.addShareKeysFn seams in packages/sdk
are untouched.
@github-actions github-actions Bot added release:sdk:feat Minor version bump (new feature) for sdk release:api:feat Minor version bump (new feature) for api release:web:feat Minor version bump (new feature) for web release:sdk-core:feat Minor version bump (new feature) for sdk-core release:tee-worker:fix Patch version bump (bug fix) for tee-worker release:cipherbox-fuse:fix Patch version bump (bug fix) for cipherbox-fuse release:cipherbox-sdk:fix Patch version bump (bug fix) for cipherbox-sdk release:desktop:fix Patch version bump (bug fix) for desktop labels Jul 2, 2026
@github-actions

github-actions Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Release Preview

Package Bump Label Source
api minor release:api:feat Direct (feat commit)
cipherbox-fuse patch release:cipherbox-fuse:fix Cascade (api minor)
cipherbox-sdk patch release:cipherbox-sdk:fix Cascade (api minor)
desktop minor release:desktop:fix Cascade (api minor)
sdk minor release:sdk:feat Direct (feat commit)
sdk-core minor release:sdk-core:feat Direct (feat commit)
tee-worker patch release:tee-worker:fix Cascade (sdk-core minor)
web minor release:web:feat Direct (feat commit)

Cascade Details

  • sdk-core minor -> tee-worker patch (direct dependency)
  • api minor -> cipherbox-fuse patch (direct dependency)
  • api minor -> cipherbox-sdk patch (direct dependency)
  • api minor -> desktop patch (direct dependency)

@codecov

codecov Bot commented Jul 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.33333% with 26 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.79%. Comparing base (ab209a9) to head (c3c215d).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
packages/sdk/src/client.ts 90.20% 23 Missing and 1 partial ⚠️
apps/api/src/shares/shares.controller.ts 60.00% 0 Missing and 2 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             main     #587       +/-   ##
===========================================
+ Coverage   65.64%   83.79%   +18.14%     
===========================================
  Files         151      117       -34     
  Lines       12351     8498     -3853     
  Branches     1390     1457       +67     
===========================================
- Hits         8108     7121      -987     
+ Misses       4009     1139     -2870     
- Partials      234      238        +4     
Flag Coverage Δ
api 83.79% <93.33%> (+0.40%) ⬆️
api-client 83.79% <93.33%> (+0.40%) ⬆️
core 83.79% <93.33%> (+0.40%) ⬆️
crypto 83.79% <93.33%> (+0.40%) ⬆️
desktop ?
sdk 83.79% <93.33%> (+0.40%) ⬆️
sdk-core 83.79% <93.33%> (+0.40%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
apps/api/src/shares/shares.service.ts 97.43% <100.00%> (+0.42%) ⬆️
packages/sdk-core/src/rotation/engine.ts 82.99% <100.00%> (+0.25%) ⬆️
packages/sdk/src/share/owner-reconcile.ts 100.00% <100.00%> (ø)
packages/sdk/src/state/rotation-high-water.ts 100.00% <100.00%> (ø)
apps/api/src/shares/shares.controller.ts 70.31% <60.00%> (-0.88%) ⬇️
packages/sdk/src/client.ts 82.02% <90.20%> (+1.26%) ⬆️

... and 36 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@greptile-apps

greptile-apps Bot commented Jul 2, 2026

Copy link
Copy Markdown

Greptile Summary

This phase wires the SDK's scope-exit rotation (rotateReadFromNode) into every revocation-triggering mutation (rename, move, delete, bin), adds a durable IndexedDB-backed HighWaterStore for anti-rollback generation/seq floors that survive page reload, and introduces reconcile-before-publish as a pre-mutation guard against stale-state publishes.

  • SDK (packages/sdk, packages/sdk-core): reconcileFolderSequence re-resolves the IPNS sequence before every publish and gates it through the injected rotationHighWater.enforceResolved (fail-closed on unverified records); performScopeExitRotation drives rotateReadFromNode on scope exit and refreshes the in-memory folderTree with the root's new key/generation/seq so same-session second mutations self-heal; dead executeLazyRotation/addShareKeys/reWrapForRecipients fan-out is removed.
  • Web (apps/web): rotation-state.service.ts supplies the IndexedDB-backed HighWaterStore with D-08 in-memory degradation fallback; rotation-driver.service.ts maps SDK lifecycle signals onto durable job checkpoints, the rotation badge store, and navigator.locks multi-tab leader election; useMutationFailureUx classifies fail-closed SDK errors into bounded-backoff retry, regression-rejected, and write-descriptor-stale UX paths with toast actions.
  • API (apps/api): New PATCH /shares/:shareId/grant endpoint with pessimistic row lock and anti-rollback BigInt comparison for rootGeneration.

Confidence Score: 4/5

Safe to merge with one structural UX gap in the rotation-mutation coupling worth addressing before users with active grants hit transient rotation failures

The anti-rollback enforcement, IDB adapter, reconcile-before-publish, and owner-reconcile transport are all well-structured. The main concern is that scope-exit rotation errors propagate to the calling hook after folder:updated has already been emitted — a rotation-side IPNS failure causes the hook to surface an error for a mutation that already landed on the network and is visible in the UI, creating a conflicting signal for the user.

packages/sdk/src/client.ts — the ordering of folder:updated emission vs. performScopeExitRotation in renameItem, deleteItem, and deleteToBin

Important Files Changed

Filename Overview
packages/sdk/src/client.ts Adds reconcile-before-publish, scope-exit rotation, and folderTree self-heal; rotation errors propagate after folder:updated emission, creating misleading failure UX for callers
packages/sdk/src/state/rotation-high-water.ts New durable anti-rollback state machine with monotonic-max generation and seq floors, V5 fail-closed validation, and cold-device versionFloor gate; well-tested and correctly structured
apps/web/src/hooks/useMutationFailureUx.ts Fail-closed error classifier with bounded retry; dispatchDeferExhausted and dispatchWriteDescriptorStale correctly dismiss originating toasts; Syncing toast ID not captured so it cannot be dismissed on successful retry
apps/web/src/services/rotation-state.service.ts IndexedDB-backed HighWaterStore adapter with correct D-08 in-memory degradation fallback; idbPut performs max-preserving read-modify-write in a single transaction for multi-tab safety
apps/web/src/services/rotation-driver.service.ts Durable job checkpoint + badge lifecycle + multi-tab leader election; IDB connections opened per-call (deferred cleanup acknowledged in todos); resumeInterruptedRotation correctly marks badge without replaying the walk
apps/web/src/services/owner-reconcile.service.ts Thin transport wrapper for owner-reconcile; correctly paginates all sent grants and includes a hasSdkClient guard after the async fetch
apps/api/src/shares/shares.service.ts Adds updateGrant with pessimistic row lock and anti-rollback BigInt comparison; correct owner-only guard; rootGeneration NOT NULL constraint ensures no null-coercion path
packages/sdk-core/src/rotation/engine.ts rotateReadFromNode now returns RotateReadResult on fresh rotation and undefined on resume/skip, enabling same-session folderTree self-heal in the SDK client

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Hook as useFolderMutations
    participant UX as runWithFailureUx
    participant SDK as CipherBoxClient
    participant HW as RotationHighWater IDB
    participant IPNS as IPNS/API
    participant Store as FolderStore + UI

    Hook->>UX: "runWithFailureUx(() => renameItem(...))"
    UX->>SDK: renameItem(...)
    SDK->>IPNS: resolveIpnsRecord reconcile check
    IPNS-->>SDK: sequenceNumber signatureVerified
    SDK->>HW: enforceResolved seq generation versionFloor
    HW-->>SDK: pass or throw SequenceRegressionError
    SDK->>IPNS: updateFolderMetadataAndPublish
    IPNS-->>SDK: newSequenceNumber
    SDK->>Store: emitter.emit folder:updated UI updates here
    SDK->>SDK: performScopeExitRotation
    SDK->>IPNS: rotateReadFromNode root cut plus tail walk
    IPNS-->>SDK: RotateReadResult readKey generation seq
    SDK->>SDK: folderTree.set new readKey seq gen
    SDK-->>UX: resolved
    UX-->>Hook: resolved

    alt ReconcileStaleError before publish
        UX->>UX: retry with backoff max 5 attempts 30s
        UX->>Hook: dispatchDeferExhausted toast on exhaustion
    else SequenceRegressionError or GenerationRegressionError
        UX->>Hook: Stale data from server rejected toast
    else CannotWriteUntilRefetchError
        UX->>Hook: Refresh access toast with action
    end
Loading
%%{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"}}}%%
sequenceDiagram
    participant Hook as useFolderMutations
    participant UX as runWithFailureUx
    participant SDK as CipherBoxClient
    participant HW as RotationHighWater IDB
    participant IPNS as IPNS/API
    participant Store as FolderStore + UI

    Hook->>UX: "runWithFailureUx(() => renameItem(...))"
    UX->>SDK: renameItem(...)
    SDK->>IPNS: resolveIpnsRecord reconcile check
    IPNS-->>SDK: sequenceNumber signatureVerified
    SDK->>HW: enforceResolved seq generation versionFloor
    HW-->>SDK: pass or throw SequenceRegressionError
    SDK->>IPNS: updateFolderMetadataAndPublish
    IPNS-->>SDK: newSequenceNumber
    SDK->>Store: emitter.emit folder:updated UI updates here
    SDK->>SDK: performScopeExitRotation
    SDK->>IPNS: rotateReadFromNode root cut plus tail walk
    IPNS-->>SDK: RotateReadResult readKey generation seq
    SDK->>SDK: folderTree.set new readKey seq gen
    SDK-->>UX: resolved
    UX-->>Hook: resolved

    alt ReconcileStaleError before publish
        UX->>UX: retry with backoff max 5 attempts 30s
        UX->>Hook: dispatchDeferExhausted toast on exhaustion
    else SequenceRegressionError or GenerationRegressionError
        UX->>Hook: Stale data from server rejected toast
    else CannotWriteUntilRefetchError
        UX->>Hook: Refresh access toast with action
    end
Loading

Comments Outside Diff (1)

  1. packages/sdk/src/client.ts, line 1307-1331 (link)

    P1 Rotation error after folder:updated emission surfaces as a false mutation failure

    folder:updated is emitted (and the React store updated) before performScopeExitRotation is awaited. If the rotation then throws — a network error, IPNS CAS conflict, or any unclassified error — the error propagates through withLock to runWithFailureUx and into the calling hook. But the UI has already reflected the mutation as complete (the rename/delete already appears done). The hook's error path fires anyway, potentially dispatching a misleading toast (e.g. dispatchRegressionRejected() would show "Stale data from server rejected." for what is actually a rotation-side IPNS failure). The same pattern appears in deleteItem (which also emits folder:updated before calling performScopeExitRotation).

    Rotation errors should be surfaced with a dedicated notice ("Access revocation in progress — will resume on next action") rather than propagating as a full mutation failure, since the primary publish already landed on the network and the UI already reflects it.

Reviews (5): Last reviewed commit: "fix(web): dismiss the refresh-access toa..." | Re-trigger Greptile

Comment thread apps/web/src/services/rotation-driver.service.ts
Comment thread apps/web/src/services/owner-reconcile.service.ts
Comment thread apps/web/src/hooks/useMutationFailureUx.ts Outdated
Comment thread apps/web/src/hooks/useFileOperations.ts Outdated
…e stub

Greptile PR findings: a persistent reconcile failure stacked a new
Retry toast per click - the originating toast is now dismissed before
the re-run via the notification id addNotification now returns; the
not-implemented update-file stub threw inside runWithFailureUx, firing
the degraded-cache notice for an operation that never reaches the
rotation path.

Entire-Checkpoint: 64095f48c9ee
Comment thread apps/api/src/shares/shares.service.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/sdk-core/src/rotation/engine.ts (1)

890-967: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

Dirty-resume republish result is silently dropped, leaving caller's folderTree stale.

On the dirty-resume path (rootResult.skipped === true, isDirty === true), when frontier.length > 0 the code seeds parentTracking for the root and, once all frontier children finish, republishes the root via updateFolderMetadataAndPublish (line ~1042/1110) — this bumps the root's on-chain IPNS sequenceNumber. However, the final return (line 1192) unconditionally returns undefined whenever rootResult.skipped is true, regardless of whether this dirty-resume walk actually republished the root with a new sequence number.

Since performScopeExitRotation (packages/sdk/src/client.ts) only refreshes its in-memory folderTree entry when rotationResult is truthy, this specific case leaves the client's cached sequenceNumber stale after a real publish occurred — a subsequent same-session mutation on this folder will hit reconcileFolderSequence's ReconcileStaleError and defer permanently until a full reload. This is exactly the class of bug "ROT-07 Gap 2" fixed for the normal path, but the dirty-resume-with-pending-children case isn't covered.

🛠️ Sketch: surface the republished sequence on the dirty-resume path too
   if (rootResult.skipped) {
     // Resume path: root already committed in a prior run.
     const { isDirty, frontier } = await verifySubtreeClean(rootNodeIpnsName, rootReadKey, ctx);
     if (!isDirty) {
       jobRecord.status = 'complete';
       if (jobRecord.persistCallback) await jobRecord.persistCallback(jobRecord);
       return undefined;
     }
     ...
+    // Track whether this dirty-resume walk republishes the root so the final
+    // return can report the up-to-date sequenceNumber even without a fresh key.
   }
   ...
-  if (rootResult.skipped) {
-    return undefined;
-  }
+  if (rootResult.skipped) {
+    return dirtyResumeRootRepublish
+      ? { readKey: rootReadKey, generation: rootNode.generation, sequenceNumber: dirtyResumeRootRepublish.newSequenceNumber }
+      : undefined;
+  }

Also applies to: 1180-1199

🤖 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/rotation/engine.ts` around lines 890 - 967, The
dirty-resume path in rotateReadFromNode is dropping the result of a real
republish, so the caller never learns the updated IPNS sequence. In the branch
where rootResult.skipped is true and frontier.length > 0, make sure the code
captures the sequence returned by the root
republish/updateFolderMetadataAndPublish flow and returns a truthy rotation
result instead of always falling through to undefined. Use the existing symbols
rotateReadFromNode, parentTracking, and updateFolderMetadataAndPublish to locate
the dirty-resume republish path, and only return undefined when no republish
actually occurred.
🧹 Nitpick comments (4)
packages/sdk/src/__tests__/rotation-high-water.test.ts (1)

1-271: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Add coverage for invalid live parameters passed to enforceResolved/bumpGeneration/bumpSeq.

Existing tests cover malformed stored values (Lines 136-183) but not malformed live params (e.g., generation: NaN or negative seq) passed directly into enforceResolved. Given NaN comparisons never throw in JS, this is the exact class of input that could silently bypass the regression gate — see the companion comment on rotation-high-water.ts.

As per path instructions, **/*.test.ts review should "Focus on test coverage, edge cases, and test quality."

🤖 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/src/__tests__/rotation-high-water.test.ts` around lines 1 - 271,
Add tests in createRotationHighWater’s enforceResolved / bumpGeneration /
bumpSeq coverage for invalid live inputs such as NaN, negative, and non-integer
values. The current suite only checks malformed stored values, so extend the
existing describe blocks in rotation-high-water.test.ts to assert these APIs
fail closed or reject invalid parameters before any floor is updated. Use the
same public symbols already referenced here—createRotationHighWater,
GenerationRegressionError, and SequenceRegressionError—to keep the new cases
aligned with the existing regression-gate behavior.

Source: Path instructions

tests/web-e2e/tests/rotation-durability.spec.ts (1)

55-56: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Avoid pinning the rotation-state IndexedDB version in this spec

indexedDB.open('cipherbox-rotation-state', 1) duplicates the service’s DB version and will start throwing VersionError if that version changes. Open without a version argument, or share the service constant.

🤖 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 `@tests/web-e2e/tests/rotation-durability.spec.ts` around lines 55 - 56, The
rotation-state IndexedDB open call in the rotation-durability spec is pinning a
schema version that can drift from the service. Update the IndexedDB access in
the spec around the req open/error handling to avoid hardcoding version 1,
either by opening the database without a version argument or by reusing the same
version constant used by the service’s rotation-state database.
apps/web/src/services/rotation-driver.service.ts (1)

59-101: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

IndexedDB connections opened per call, never closed.

Every putJobCheckpoint/deleteJobCheckpoint/getAllJobCheckpoints call opens a fresh openJobDB() connection that is never closed, accumulating open handles over a session and risking a blocked upgrade if JOB_DB_VERSION is ever bumped while old connections remain open. Consider caching a single connection promise at module scope and reusing it.

🤖 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/services/rotation-driver.service.ts` around lines 59 - 101, The
IndexedDB helpers are opening a new connection on every call and never closing
it, which can leave stale handles around. Update the openJobDB,
putJobCheckpoint, deleteJobCheckpoint, and getAllJobCheckpoints flow to reuse a
single module-scoped connection promise or cached IDBDatabase instance instead
of reopening per operation. Ensure the shared connection is created once, reused
by all checkpoint functions, and only closed intentionally if you add explicit
teardown logic.
apps/web/src/services/rotation-state.service.ts (1)

42-111: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Cache the IndexedDB connection instead of reopening per get/put.

idbGet/idbPut each call openRotationDB() independently and never close the resulting connection. Since this backs enforceResolved, called on every rotation-participating resolveIpnsRecord (potentially every navigation), each resolve can open 1-2 fresh IDBDatabase connections that are never released — real overhead on a hot path, and a future DB_VERSION bump would risk onblocked since old connections stay open indefinitely. Cache a single connection-open promise at module scope and reuse it across get/put calls.

🔧 Proposed fix: cache the open connection
-function openRotationDB(): Promise<IDBDatabase> {
-  return new Promise((resolve, reject) => {
-    const request = indexedDB.open(DB_NAME, DB_VERSION);
-    request.onupgradeneeded = () => { ... };
-    request.onsuccess = () => resolve(request.result);
-    request.onerror = () => reject(request.error);
-  });
-}
+let dbPromise: Promise<IDBDatabase> | null = null;
+function openRotationDB(): Promise<IDBDatabase> {
+  if (!dbPromise) {
+    dbPromise = new Promise((resolve, reject) => {
+      const request = indexedDB.open(DB_NAME, DB_VERSION);
+      request.onupgradeneeded = () => { ... };
+      request.onsuccess = () => resolve(request.result);
+      request.onerror = () => {
+        dbPromise = null;
+        reject(request.error);
+      };
+    });
+  }
+  return dbPromise;
+}
🤖 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/services/rotation-state.service.ts` around lines 42 - 111, The
IndexedDB helpers in rotation-state.service are reopening the database for every
idbGet/idbPut call and never releasing those connections. Update openRotationDB
to cache a single module-scope Promise<IDBDatabase> (and reuse it from idbGet
and idbPut) so repeated resolveIpnsRecord/enforceResolved calls share one open
connection instead of creating new ones. Keep the existing openRotationDB,
idbGet, and idbPut behavior the same aside from reusing the cached connection,
and ensure the cache is initialized once and reused for all reads/writes.
🤖 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/hooks/useMutationFailureUx.ts`:
- Around line 80-100: The defer-exhaustion retry path is dropping the original
options object, so the retried mutation in
dispatchDeferExhausted/retryAfterExhaustion falls back to default
runWithFailureUx behavior. Thread opts through both helpers and pass it into the
retried runWithFailureUx call so any original behavior flags like
refreshWriteAccess are preserved when the user clicks Retry.

In `@apps/web/src/services/owner-reconcile.service.ts`:
- Around line 44-70: `decodeSentGrants` currently only fetches the first page
from `sharesControllerGetSentShares`, so reconcile can miss later sent grants.
Update `decodeSentGrants` in `owner-reconcile.service.ts` to loop through all
pages using the API client’s `limit` and `offset` (similar to `share.service.ts`
pagination), accumulating and decoding every share before returning. Keep the
existing per-share decode/error handling intact so a bad row still doesn’t abort
the full sweep.

In `@packages/sdk/src/client.ts`:
- Around line 617-642: The rotation refresh block in client.ts leaves sensitive
key material alive because `rotationResult.readKey` is copied into `folderTree`
but never wiped. In the `rotationResult` handling near `folderTree.set(...)`,
zero the original `rotationResult.readKey` buffer after creating the defensive
copy, while keeping the existing `oldFolderKey.fill(0)` cleanup intact. Use the
existing `rotationResult`, `folderTree`, and `oldFolderKey` flow as the anchor
for the fix.
- Around line 663-730: The MAX_NODES guard in enumerateMoveDescendants leaves
queued readKey buffers uncleared when the loop stops early. Update the cleanup
in enumerateMoveDescendants so any remaining queue entries are drained and their
readKey Uint8Array values are filled with zeros before returning, while
preserving the existing per-entry finally zeroing for dequeued items.
- Around line 515-554: The generation passed into reconcileFolderSequence is
currently read from folderTree instead of the freshly resolved IPNS record, so
the rotationHighWater check is using cached local state. Update the
resolveIpnsRecord flow and the reconcileFolderSequence path to carry the
resolved generation through this method, and use that value in enforceResolved;
if a fallback to folderTree is still needed, make it explicit in the contract
for resolveIpnsRecord and reconcileFolderSequence.

In `@packages/sdk/src/state/rotation-high-water.ts`:
- Around line 150-168: `enforceResolved` is comparing untrusted `generation` and
`seq` inputs without validating them first, so malformed values like NaN can
bypass the regression checks. Update the `enforceResolved` method in
`rotation-high-water` to validate the live inputs before any floor comparisons
or `bumpFloor` calls, using the same floor-value rules enforced by
`isValidFloorValue`/`readFloor`. Reject invalid `generation`, `seq`, and
`versionFloor` values up front so the `GenerationRegressionError` and
`SequenceRegressionError` paths remain fail-closed.

---

Outside diff comments:
In `@packages/sdk-core/src/rotation/engine.ts`:
- Around line 890-967: The dirty-resume path in rotateReadFromNode is dropping
the result of a real republish, so the caller never learns the updated IPNS
sequence. In the branch where rootResult.skipped is true and frontier.length >
0, make sure the code captures the sequence returned by the root
republish/updateFolderMetadataAndPublish flow and returns a truthy rotation
result instead of always falling through to undefined. Use the existing symbols
rotateReadFromNode, parentTracking, and updateFolderMetadataAndPublish to locate
the dirty-resume republish path, and only return undefined when no republish
actually occurred.

---

Nitpick comments:
In `@apps/web/src/services/rotation-driver.service.ts`:
- Around line 59-101: The IndexedDB helpers are opening a new connection on
every call and never closing it, which can leave stale handles around. Update
the openJobDB, putJobCheckpoint, deleteJobCheckpoint, and getAllJobCheckpoints
flow to reuse a single module-scoped connection promise or cached IDBDatabase
instance instead of reopening per operation. Ensure the shared connection is
created once, reused by all checkpoint functions, and only closed intentionally
if you add explicit teardown logic.

In `@apps/web/src/services/rotation-state.service.ts`:
- Around line 42-111: The IndexedDB helpers in rotation-state.service are
reopening the database for every idbGet/idbPut call and never releasing those
connections. Update openRotationDB to cache a single module-scope
Promise<IDBDatabase> (and reuse it from idbGet and idbPut) so repeated
resolveIpnsRecord/enforceResolved calls share one open connection instead of
creating new ones. Keep the existing openRotationDB, idbGet, and idbPut behavior
the same aside from reusing the cached connection, and ensure the cache is
initialized once and reused for all reads/writes.

In `@packages/sdk/src/__tests__/rotation-high-water.test.ts`:
- Around line 1-271: Add tests in createRotationHighWater’s enforceResolved /
bumpGeneration / bumpSeq coverage for invalid live inputs such as NaN, negative,
and non-integer values. The current suite only checks malformed stored values,
so extend the existing describe blocks in rotation-high-water.test.ts to assert
these APIs fail closed or reject invalid parameters before any floor is updated.
Use the same public symbols already referenced here—createRotationHighWater,
GenerationRegressionError, and SequenceRegressionError—to keep the new cases
aligned with the existing regression-gate behavior.

In `@tests/web-e2e/tests/rotation-durability.spec.ts`:
- Around line 55-56: The rotation-state IndexedDB open call in the
rotation-durability spec is pinning a schema version that can drift from the
service. Update the IndexedDB access in the spec around the req open/error
handling to avoid hardcoding version 1, either by opening the database without a
version argument or by reusing the same version constant used by the service’s
rotation-state database.
🪄 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: b685af9c-6515-449e-abe7-3b1627a304de

📥 Commits

Reviewing files that changed from the base of the PR and between 9fa5f5b and c2fb041.

⛔ Files ignored due to path filters (4)
  • packages/api-client/openapi.json is excluded by !packages/api-client/**
  • packages/api-client/src/generated/shares/shares.ts is excluded by !**/generated/**, !**/generated/**, !packages/api-client/**
  • packages/api-client/src/models/index.ts is excluded by !packages/api-client/**
  • packages/api-client/src/models/updateGrantDto.ts is excluded by !packages/api-client/**
📒 Files selected for processing (83)
  • .planning/REQUIREMENTS.md
  • .planning/ROADMAP.md
  • .planning/STATE.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-01-PLAN.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-01-SUMMARY.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-02-PLAN.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-02-SUMMARY.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-03-PLAN.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-03-SUMMARY.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-04-PLAN.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-04-SUMMARY.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-05-PLAN.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-05-SUMMARY.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-06-PLAN.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-06-SUMMARY.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-07-PLAN.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-07-SUMMARY.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-08-PLAN.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-08-SUMMARY.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-09-PLAN.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-09-SUMMARY.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-10-PLAN.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-10-SUMMARY.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-11-PLAN.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-11-SUMMARY.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-12-PLAN.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-12-SUMMARY.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-CONTEXT.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-DISCUSSION-LOG.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-LEARNINGS.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-PATTERNS.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-RESEARCH.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-SECURITY.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-UI-SPEC.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-VALIDATION.md
  • .planning/phases/68-web-integration-rotation-ux-and-durable-client-state/68-VERIFICATION.md
  • .planning/todos/pending/2026-07-02-retire-dead-sdk-share-scaffolding.md
  • .planning/todos/pending/2026-07-02-rotation-hardening-followups-from-pr-review.md
  • .planning/todos/pending/2026-07-02-web-vitest-not-in-ci-and-ipns-service-test-broken.md
  • .planning/todos/pending/2026-07-02-write03-refresh-access-path-has-no-live-trigger.md
  • apps/api/src/shares/dto/update-grant.dto.ts
  • apps/api/src/shares/shares.controller.spec.ts
  • apps/api/src/shares/shares.controller.ts
  • apps/api/src/shares/shares.service.spec.ts
  • apps/api/src/shares/shares.service.ts
  • apps/web/src/components/NotificationToast.tsx
  • apps/web/src/components/file-browser/useFileBrowserActions.ts
  • apps/web/src/components/layout/AppHeader.tsx
  • apps/web/src/components/layout/RotationStatusBadge.tsx
  • apps/web/src/hooks/useAuth.ts
  • apps/web/src/hooks/useFileOperations.ts
  • apps/web/src/hooks/useFolderMutations.ts
  • apps/web/src/hooks/useMutationFailureUx.ts
  • apps/web/src/hooks/useSharedNavigation.ts
  • apps/web/src/index.css
  • apps/web/src/lib/multi-tab-lock.ts
  • apps/web/src/services/ipns.service.ts
  • apps/web/src/services/owner-reconcile.service.ts
  • apps/web/src/services/rotation-driver.service.ts
  • apps/web/src/services/rotation-state.service.ts
  • apps/web/src/services/share.service.ts
  • apps/web/src/stores/notification.store.ts
  • apps/web/src/stores/rotation.store.ts
  • apps/web/src/stores/share.store.ts
  • apps/web/src/styles/layout.css
  • packages/sdk-core/src/__tests__/rotation/engine.test.ts
  • packages/sdk-core/src/index.ts
  • packages/sdk-core/src/rotation/engine.ts
  • packages/sdk-core/src/rotation/index.ts
  • packages/sdk/src/__tests__/client-move-enumerate.test.ts
  • packages/sdk/src/__tests__/client-rotation.test.ts
  • packages/sdk/src/__tests__/client.test.ts
  • packages/sdk/src/__tests__/collect-subtree-ipns-names.test.ts
  • packages/sdk/src/__tests__/owner-reconcile.test.ts
  • packages/sdk/src/__tests__/rotation-high-water.test.ts
  • packages/sdk/src/client.ts
  • packages/sdk/src/index.ts
  • packages/sdk/src/share/index.ts
  • packages/sdk/src/share/owner-reconcile.ts
  • packages/sdk/src/state/rotation-high-water.ts
  • packages/sdk/src/types.ts
  • tests/web-e2e/tests/rotation-durability.spec.ts
  • tests/web-e2e/tests/rotation-ux.spec.ts

Comment thread apps/web/src/hooks/useMutationFailureUx.ts Outdated
Comment thread apps/web/src/services/owner-reconcile.service.ts
Comment thread packages/sdk/src/client.ts
Comment thread packages/sdk/src/client.ts
Comment thread packages/sdk/src/client.ts
Comment thread packages/sdk/src/state/rotation-high-water.ts
Second-pass PR review fixes for PR 587:

- rotation-high-water: enforceResolved now rejects NaN/negative/fractional
  live generation/seq inputs fail-closed instead of letting NaN comparisons
  silently pass the regression gate; bumpFloor never persists a malformed
  candidate; malformed cold-device versionFloor rejects rather than
  disabling the first-contact gate. Covered by 5 new unit tests.
- useMutationFailureUx: thread the original opts through the
  defer-exhaustion manual Retry so a subsequent stale-write failure still
  offers the refreshWriteAccess path.
- owner-reconcile: paginate the sent-grants fetch so grants beyond the
  first page are no longer silently unreconciled.
- enumerateMoveDescendants: zero readKeys left queued when the MAX_NODES
  bound cuts the walk short.
- rotation-durability e2e spec: open the rotation-state IndexedDB without
  a pinned version so the probe survives a future DB_VERSION bump.
- todo: record the two deferred review findings — reconcile-gate cached
  generation source and the dirty-resume republish result drop.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Entire-Checkpoint: 9f0ec39529e4
@FSM1

FSM1 commented Jul 2, 2026

Copy link
Copy Markdown
Owner Author

Dispositions for the CodeRabbit review-body findings that have no inline thread:

  • engine.ts dirty-resume republish result dropped (outside diff, Major): real — same stale-seq class as ROT-07 Gap 2, but on the resume path. Too deep in the engine for a ship-time patch; captured as item 6 in .planning/todos/pending/2026-07-02-rotation-hardening-followups-from-pr-review.md.
  • rotation-high-water.test.ts invalid live-param coverage (nitpick): added in 11caee2 alongside the enforceResolved input-validation fix — 5 new tests.
  • rotation-durability.spec.ts pinned IndexedDB version (nitpick): fixed in 11caee2 — the probe now opens without a version argument.
  • IndexedDB connection caching in rotation-state / rotation-driver services (nitpicks): already deferred as item 3 of the same rotation-hardening todo from the first review round.

Comment thread packages/sdk/src/client.ts Outdated
Symmetric with the read-path guard in the web ipns.service:
reconcileFolderSequence now throws before enforceResolved when the
resolved record is not signature-verified, so a relay returning a forged
inflated seq can never bump the durable floor and permanently wedge
future mutations behind SequenceRegressionError. Regression test added.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Entire-Checkpoint: 46fe4bb06d13
Comment thread apps/web/src/hooks/useMutationFailureUx.ts
Same anti-stacking rule already applied to the defer-exhausted Retry:
the follow-up refresh/retry path dispatches its own notice, so drop the
originating toast before re-running instead of stacking one per attempt.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Entire-Checkpoint: 456303bfd721
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release:api:feat Minor version bump (new feature) for api release:cipherbox-fuse:fix Patch version bump (bug fix) for cipherbox-fuse release:cipherbox-sdk:fix Patch version bump (bug fix) for cipherbox-sdk release:desktop:fix Patch version bump (bug fix) for desktop release:sdk:feat Minor version bump (new feature) for sdk release:sdk-core:feat Minor version bump (new feature) for sdk-core release:tee-worker:fix Patch version bump (bug fix) for tee-worker release:web:feat Minor version bump (new feature) for web

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant