Skip to content

feat(web): user-configurable vault parameters#423

Merged
FSM1 merged 21 commits into
mainfrom
docs/phase-39-context
Mar 31, 2026
Merged

feat(web): user-configurable vault parameters#423
FSM1 merged 21 commits into
mainfrom
docs/phase-39-context

Conversation

@FSM1

@FSM1 FSM1 commented Mar 31, 2026

Copy link
Copy Markdown
Owner

Summary

  • Added VaultSettings type to @cipherbox/core with defaults matching current hardcoded values (30-day retention, 10 max versions, 15min cooldown, soft-delete)
  • Added deriveVaultSettingsIpnsKeypair HKDF derivation to @cipherbox/crypto
  • Created vault settings Zustand store and IPNS load/save service (follows BYO-IPFS config pattern)
  • Wired vault settings into all consumers: delete behavior toggle, configurable max versions + version cooldown, bin retention from vault metadata
  • Added "Vault" tab to Settings page with number inputs, presets, and radio group for delete behavior
  • Deprecated server-side RECYCLE_BIN_RETENTION_DAYS env var — client controls retention via encrypted vault metadata
  • Net: +922 lines across 18 files (includes 143-line test suite)

Changes

Core types & crypto (packages/core, packages/crypto)

  • VaultSettings type, DEFAULT_VAULT_SETTINGS, validateVaultSettings()
  • deriveVaultSettingsIpnsKeypair() with domain-separated HKDF
  • 143-line unit test suite for validation, defaults, and edge cases

Web app integration (apps/web)

  • vault-settings.store.ts — Zustand store for settings state
  • vault-settings.service.ts — IPNS load/save with ECIES encryption
  • useAuth.ts — parallel load of BYO config + vault settings on login
  • useFolderMutations.ts — delete behavior toggle (soft-delete to bin vs permanent)
  • file-metadata.service.ts — configurable MAX_VERSIONS_PER_FILE and VERSION_COOLDOWN_MS
  • useBin.ts — retention days from vault settings store
  • VaultTab.tsx — new Settings tab with 4 setting sections
  • clear-user-stores.ts — clears vault settings on logout

Test plan

  • pnpm typecheck passes
  • pnpm --filter @cipherbox/core test passes (vault settings tests)
  • Settings > Vault tab renders with default values
  • Changing retention days saves and persists across page reload
  • Toggling delete behavior to "permanent" shows confirmation dialog on delete
  • Version cooldown and max versions settings take effect on file updates
  • Existing vaults with no settings field auto-populate defaults (no migration needed)

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • New "Vault" tab in Settings to view/edit encrypted vault parameters (retention, delete behavior, max versions, cooldown). Settings load at login, persist securely, and apply app-wide (deletion, retention, versioning). Save/reset with success/error UI.
  • Documentation

    • Phase roadmap, research, validation, planning, and discussion docs added describing scope, UX, and acceptance criteria.
  • Tests

    • Unit and IPNS/key-derivation tests covering validation and deterministic persistence.
  • Chores

    • Pinned a web dependency version.

FSM1 and others added 10 commits March 31, 2026 02:57
Entire-Checkpoint: 3b7cc299735c
4 plans in 2 waves:
- Wave 1: Core types/HKDF (01) + Zustand store/IPNS service (02)
- Wave 2: Consumer integration (03) + Settings UI (04)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 440542a415c9
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…rivation

Define VaultSettings type with recycleBinRetentionDays, deleteBehavior,
maxVersionsPerFile, and versionCooldownMinutes fields. Add DEFAULT_VAULT_SETTINGS
matching current hardcoded behavior and validateVaultSettings for safe parsing.
Add deriveVaultSettingsIpnsKeypair with domain-separated HKDF info string.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: c3c2c69bd4d3
Create useVaultSettingsStore for reactive vault settings state management
and vault-settings.service.ts for loading/saving encrypted settings via IPNS.
Follows established BYO-IPFS config pattern: ECIES encrypt, IPFS upload, IPNS publish.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 7e24a0d83d65
Load vault settings from encrypted IPNS on login (parallel with BYO
config), clear on logout via clearAllUserStores. Replace hardcoded
values: useFolderMutations reads deleteBehavior for soft/hard delete,
file-metadata.service reads maxVersionsPerFile and versionCooldownMinutes
from store, useBin reads recycleBinRetentionDays from vault settings
store instead of API config endpoint.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Create VaultTab component with form controls for recycle bin retention
(1-365 days), delete behavior (bin/permanent radio), max versions per
file (0-100), and version cooldown (0-1440 minutes). Add VAULT tab to
Settings page alongside existing LINKED METHODS, SECURITY, STORAGE tabs
with full keyboard navigation and ARIA roles.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Mar 31, 2026

Copy link
Copy Markdown

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Adds encrypted, user-configurable VaultSettings (schema, defaults, validator), deterministic HKDF-based IPNS key derivation, IPFS/IPNS-backed load/save with ECIES, a Zustand store, web integration (auth load, bin/delete/versioning consumers), Settings UI (Vault tab), tests, and Phase 39 planning/state docs.

Changes

Cohort / File(s) Summary
Planning & Phase State
\.planning/ROADMAP.md, \.planning/STATE.md
Marked Phase 39 items complete and advanced project state to Phase 39; updated progress counters and metadata.
Phase Docs
\.planning/phases/39-user-configurable-vault-parameters/...
Added context, research, plans, validation, summaries, and discussion log for Phase 39.
Core — Types & Validation
packages/core/src/vault/types.ts, packages/core/src/vault/settings.ts, packages/core/src/vault/index.ts, packages/core/src/index.ts
Introduced VaultSettings type, DEFAULT_VAULT_SETTINGS, and validateVaultSettings() with clamping/normalization; re-exported via core barrels.
Core — Tests
packages/core/src/__tests__/vault-settings.test.ts
Added unit tests for defaults, clamping, normalization, invalid inputs, and version enforcement.
Crypto — IPNS Derivation
packages/crypto/src/vault/derive-ipns.ts, packages/crypto/src/vault/index.ts, packages/crypto/src/index.ts
Added deterministic HKDF-based deriveVaultSettingsIpnsKeypair() with domain-separated info and strict 32-byte key validation; re-exported from crypto barrels.
Crypto — Tests
packages/crypto/src/__tests__/vault-settings-ipns.test.ts
Added tests for deterministic derivation, domain separation, key-size validation, and IPNS name format.
Web — Store & Persistence Service
apps/web/src/stores/vault-settings.store.ts, apps/web/src/services/vault-settings.service.ts
Added useVaultSettingsStore (Zustand) and loadVaultSettings / saveVaultSettings implementing IPNS/IPFS + ECIES persistence with a 10s timeout/fallback and monotonic IPNS publish.
Web — Hook & Consumer Integration
apps/web/src/hooks/useAuth.ts, apps/web/src/lib/clear-user-stores.ts, apps/web/src/hooks/useBin.ts, apps/web/src/hooks/useFolderMutations.ts, apps/web/src/services/file-metadata.service.ts, apps/web/src/stores/bin.store.ts
Load vault settings during auth (parallel with BYO config), clear store on logout, switch bin retention source to vault settings store, make delete flow respect deleteBehavior, replace hardcoded versioning constants with store-driven getters, and remove retentionDays from bin store.
Web — Settings UI & Styles
apps/web/src/components/settings/VaultTab.tsx, apps/web/src/routes/SettingsPage.tsx, apps/web/src/App.css, apps/web/src/styles/settings.css
Added VaultTab UI (form inputs, save/reset, validation, save feedback), wired new 'vault' tab into SettingsPage, and added CSS classes for the tab UI and feedback.
Misc — Exports & Package Pins
packages/core/src/index.ts, packages/crypto/src/index.ts, packages/core/src/vault/index.ts, packages/crypto/src/vault/index.ts, apps/web/package.json, packages/sdk-core/package.json
Updated barrels to re-export new types/functions and pinned axios to 1.13.2 in two package.json files.

Sequence Diagram(s)

sequenceDiagram
    participant User as User (Login)
    participant Auth as useAuth Hook
    participant Crypto as Crypto (deriveVaultSettingsIpnsKeypair)
    participant IPNS as IPNS Resolver
    participant IPFS as IPFS Gateway
    participant ECIES as ECIES unwrapKey
    participant Store as VaultSettingsStore
    participant Bin as Bin Logic

    User->>Auth: Login
    Auth->>Crypto: deriveVaultSettingsIpnsKeypair(userPrivateKey)
    Crypto-->>Auth: { ipnsName, publicKey, privateKey }
    Auth->>IPNS: resolve(ipnsName) (10s timeout)
    IPNS-->>Auth: CID or error
    Auth->>IPFS: fetch(CID)
    IPFS-->>Auth: encrypted blob or error
    Auth->>ECIES: unwrapKey(encrypted, userPrivateKey)
    ECIES-->>Auth: plaintext JSON
    Auth->>Auth: validateVaultSettings(JSON)
    Auth->>Store: setSettings(validated)
    Auth->>Bin: update retention from store
Loading
sequenceDiagram
    participant User as User (Settings UI)
    participant VaultTab as VaultTab
    participant Validator as validateVaultSettings
    participant Crypto as Crypto (deriveVaultSettingsIpnsKeypair)
    participant ECIES as ECIES wrapKey
    participant IPFS as IPFS(addToIpfs)
    participant IPNS as IPNS Publisher
    participant Store as VaultSettingsStore

    User->>VaultTab: Click [SAVE SETTINGS]
    VaultTab->>Validator: validateVaultSettings(form)
    Validator-->>VaultTab: validated settings
    VaultTab->>Crypto: deriveVaultSettingsIpnsKeypair(userPrivateKey)
    Crypto-->>VaultTab: { ipnsName, ... }
    VaultTab->>ECIES: wrapKey(JSON, userPublicKey)
    ECIES-->>VaultTab: encrypted bytes
    VaultTab->>IPFS: addToIpfs(encrypted bytes)
    IPFS-->>VaultTab: CID
    VaultTab->>IPNS: publish(ipnsName, CID, sequence)
    IPNS-->>VaultTab: publish result (success/fail)
    VaultTab->>Store: setSettings(validated)
    Store-->>User: success feedback
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

Possibly related PRs

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 72.73% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'feat(web): user-configurable vault parameters' clearly and concisely summarizes the main change—adding configurable vault settings to the web app, which is the primary objective across all modified files.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch docs/phase-39-context

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces user-configurable vault parameters (retention, delete behavior, and file versioning controls) stored as encrypted, client-owned IPNS metadata, and wires those settings through the web app so behavior is driven by vault metadata rather than hardcoded defaults / server config.

Changes:

  • Added VaultSettings to @cipherbox/core with defaults + runtime validation/clamping.
  • Added deterministic HKDF-derived IPNS keypair derivation for vault settings in @cipherbox/crypto.
  • Implemented web app load/save via IPNS (service + Zustand store) and integrated settings into bin retention, delete behavior, and versioning logic; added a new Settings “Vault” tab UI.

Reviewed changes

Copilot reviewed 29 out of 29 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
packages/crypto/src/vault/derive-ipns.ts Adds deriveVaultSettingsIpnsKeypair() using domain-separated HKDF info.
packages/crypto/src/vault/index.ts Re-exports the new derivation function.
packages/crypto/src/index.ts Exposes the new derivation function from the package root.
packages/core/src/vault/types.ts Introduces VaultSettings type.
packages/core/src/vault/settings.ts Adds defaults + validateVaultSettings() sanitization/clamping.
packages/core/src/vault/index.ts Re-exports vault settings helpers/types.
packages/core/src/index.ts Exposes vault settings helpers/types from @cipherbox/core.
packages/core/src/tests/vault-settings.test.ts Unit tests for defaults + validation behavior.
apps/web/src/stores/vault-settings.store.ts New Zustand store for vault settings state.
apps/web/src/services/vault-settings.service.ts New IPNS-backed load/save service (ECIES encrypt → IPFS → IPNS).
apps/web/src/hooks/useAuth.ts Loads vault settings in parallel with BYO config on login and updates bin retention.
apps/web/src/hooks/useFolderMutations.ts Routes delete operations through user-selected delete behavior.
apps/web/src/services/file-metadata.service.ts Reads versioning limits/cooldown from vault settings store.
apps/web/src/hooks/useBin.ts Reads retention days from vault settings store for UI logic.
apps/web/src/routes/SettingsPage.tsx Adds “VAULT” tab to Settings page.
apps/web/src/components/settings/VaultTab.tsx New settings UI for vault retention/delete/versioning parameters.
apps/web/src/App.css Styles for the new Vault settings UI.
apps/web/src/lib/clear-user-stores.ts Clears vault settings store on logout.
.planning/ROADMAP.md + .planning/phases/39-* Planning/research/validation artifacts for Phase 39.

Comment thread packages/core/src/vault/settings.ts Outdated
Comment thread apps/web/src/services/vault-settings.service.ts
Comment thread apps/web/src/hooks/useBin.ts
Comment thread apps/web/src/components/settings/VaultTab.tsx
Comment thread apps/web/src/components/settings/VaultTab.tsx
Comment thread packages/crypto/src/vault/derive-ipns.ts
FSM1 and others added 3 commits March 31, 2026 05:23
- Allow 0 retention days in validation and UI (D-03: 0 disables retention)
- Add TEE enrollment for vault settings IPNS republishing
- Use vault settings store as single source of truth for purge retention
- Clean up setTimeout on VaultTab unmount to prevent stale state updates
- Add deriveVaultSettingsIpnsKeypair unit tests (determinism, domain separation, validation)
- Fix pre-existing @cipherbox/core circular dep in vault-ipns tests (use test vectors)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 122e71adcdbe
Brings in phase 38 completion and resolves conflicts:
- ROADMAP.md: keep phase 39 plans marked complete
- useBin.ts: use SDK purgeExpired with vault settings store retention
- vault-ipns.test.ts: keep main's inline comments on test vectors
- STATE.md: restore phase 39 progress

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 20695bcb7dfb
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: f94fde17108a

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 31 out of 32 changed files in this pull request and generated 4 comments.

Comment thread packages/core/src/__tests__/vault-settings.test.ts Outdated
Comment thread packages/core/src/vault/types.ts Outdated
Comment thread apps/web/src/hooks/useAuth.ts
Comment thread .planning/phases/39-user-configurable-vault-parameters/39-04-SUMMARY.md Outdated
The test expected clamp(0) => 1 but we now allow 0 (D-03: 0 disables
retention). Updated to expect 0 and added negative-value test case.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 8a8ce8c8b238

@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: 2

🧹 Nitpick comments (4)
apps/web/src/hooks/useBin.ts (1)

23-23: Consider removing redundant bin store retention sync.

Per context snippet 3, useAuth.ts:298 still calls useBinStore.getState().setRetentionDays(vaultSettings.recycleBinRetentionDays). Since useBin.ts now reads directly from useVaultSettingsStore, this write to useBinStore.retentionDays is orphaned—no consumer reads it.

The redundant write is harmless but creates a stale data path. Consider removing the setRetentionDays call in useAuth.ts and potentially deprecating useBinStore.retentionDays entirely in a follow-up cleanup.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/hooks/useBin.ts` at line 23, The code now reads recycle bin
retention directly via retentionDays = useVaultSettingsStore((s) =>
s.settings.recycleBinRetentionDays) making the assignment
useBinStore.getState().setRetentionDays(...) in useAuth.ts redundant; remove the
call to
useBinStore.getState().setRetentionDays(vaultSettings.recycleBinRetentionDays)
from useAuth.ts (or gate it behind a deprecated flag) and, if safe, plan to
deprecate/remove useBinStore.retentionDays to avoid the stale write path—ensure
any consumers reference retentionDays from useVaultSettingsStore instead of
useBinStore.
apps/web/src/services/file-metadata.service.ts (1)

245-253: Refresh JSDoc wording for cooldown source.

Line 252 now uses settings-driven cooldown, but the nearby docs still mention VERSION_COOLDOWN_MS. Consider updating that wording to reflect user-configurable cooldown from vault settings.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/services/file-metadata.service.ts` around lines 245 - 253,
Update the JSDoc/comment around shouldCreateVersion to stop referencing the
hard-coded VERSION_COOLDOWN_MS and instead state that the cooldown is
configurable via vault settings and obtained through getVersionCooldownMs().
Mention that shouldCreateVersion checks currentMetadata.versions (using the
newest version's timestamp) and compares Date.now() against the settings-driven
cooldown returned by getVersionCooldownMs().
packages/core/src/vault/settings.ts (1)

35-39: Avoid duplicated default literals in validator fallbacks.

Using inline 30/10/15/'bin' here can silently drift from DEFAULT_VAULT_SETTINGS later.

♻️ Suggested refactor
-  const recycleBinRetentionDays = clamp(toNumber(raw.recycleBinRetentionDays, 30), 0, 365);
+  const recycleBinRetentionDays = clamp(
+    toNumber(raw.recycleBinRetentionDays, DEFAULT_VAULT_SETTINGS.recycleBinRetentionDays),
+    0,
+    365
+  );
   const deleteBehavior =
-    raw.deleteBehavior === 'bin' || raw.deleteBehavior === 'permanent' ? raw.deleteBehavior : 'bin';
-  const maxVersionsPerFile = clamp(toNumber(raw.maxVersionsPerFile, 10), 0, 100);
-  const versionCooldownMinutes = clamp(toNumber(raw.versionCooldownMinutes, 15), 0, 1440);
+    raw.deleteBehavior === 'bin' || raw.deleteBehavior === 'permanent'
+      ? raw.deleteBehavior
+      : DEFAULT_VAULT_SETTINGS.deleteBehavior;
+  const maxVersionsPerFile = clamp(
+    toNumber(raw.maxVersionsPerFile, DEFAULT_VAULT_SETTINGS.maxVersionsPerFile),
+    0,
+    100
+  );
+  const versionCooldownMinutes = clamp(
+    toNumber(raw.versionCooldownMinutes, DEFAULT_VAULT_SETTINGS.versionCooldownMinutes),
+    0,
+    1440
+  );

Also applies to: 54-57

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/core/src/vault/settings.ts` around lines 35 - 39, The validators in
settings.ts use hard-coded default literals (e.g., 30, 10, 15, 'bin') which can
drift from the canonical DEFAULT_VAULT_SETTINGS; update the code to read
defaults from DEFAULT_VAULT_SETTINGS (e.g.,
DEFAULT_VAULT_SETTINGS.recycleBinRetentionDays, .maxVersionsPerFile,
.versionCooldownMinutes, .deleteBehavior) when calling toNumber/clamp and when
validating raw.deleteBehavior so the fallback values come from that single
source of truth (adjust any occurrences around the consts
recycleBinRetentionDays, deleteBehavior, maxVersionsPerFile,
versionCooldownMinutes and the similar block at 54-57).
apps/web/src/services/vault-settings.service.ts (1)

40-41: Consider clearing derived IPNS keypair after use.

For defense-in-depth, the Ed25519 keypair.privateKey could be cleared after use in both loadVaultSettings and saveVaultSettings. While this is a derived key (not the user's master key), zeroing ephemeral crypto material reduces the attack surface.

🛡️ Optional: Clear derived keypair in loadVaultSettings
 export async function loadVaultSettings(userPrivateKey: Uint8Array): Promise<VaultSettings> {
   const inner = async (): Promise<VaultSettings> => {
     const keypair = await deriveVaultSettingsIpnsKeypair(userPrivateKey);
-
-    const resolved = await resolveIpnsRecord(keypair.ipnsName);
-    if (!resolved?.cid) return { ...DEFAULT_VAULT_SETTINGS };
+    try {
+      const resolved = await resolveIpnsRecord(keypair.ipnsName);
+      if (!resolved?.cid) return { ...DEFAULT_VAULT_SETTINGS };
 
-    const encrypted = await fetchFromIpfs(resolved.cid);
-    const plaintext = await unwrapKey(encrypted, userPrivateKey);
-    let parsed: unknown;
-    try {
-      const json = new TextDecoder().decode(plaintext);
-      parsed = JSON.parse(json);
+      const encrypted = await fetchFromIpfs(resolved.cid);
+      const plaintext = await unwrapKey(encrypted, userPrivateKey);
+      let parsed: unknown;
+      try {
+        const json = new TextDecoder().decode(plaintext);
+        parsed = JSON.parse(json);
+      } finally {
+        clearBytes(plaintext);
+      }
+
+      return validateVaultSettings(parsed);
     } finally {
-      clearBytes(plaintext);
+      clearBytes(keypair.privateKey);
     }
-
-    return validateVaultSettings(parsed);
   };
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/services/vault-settings.service.ts` around lines 40 - 41, The
derived Ed25519 keypair returned by deriveVaultSettingsIpnsKeypair should be
explicitly cleared after use to reduce exposure: in both loadVaultSettings and
saveVaultSettings, wrap the usage of keypair (the variable from
deriveVaultSettingsIpnsKeypair) in a try/finally and in the finally overwrite
keypair.privateKey (and any other sensitive buffer fields) with zeros or set to
null to ensure it’s cleared even on errors; reference the
deriveVaultSettingsIpnsKeypair call and keypair.privateKey and add the cleanup
in both functions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/core/src/__tests__/vault-settings.test.ts`:
- Around line 52-55: The test currently asserts validateVaultSettings({
recycleBinRetentionDays: 0 }) should clamp to 1, but per Phase 39 semantics zero
is valid (disable/immediate purge); update the test in the it block ('should
clamp recycleBinRetentionDays below 1 to 1') to instead expect
result.recycleBinRetentionDays toBe(0) (or rename the test to reflect that 0 is
allowed) and ensure the assertion references validateVaultSettings so it aligns
with the intended behavior.

In `@packages/core/src/vault/types.ts`:
- Around line 76-77: Update the JSDoc for the recycleBinRetentionDays field to
reflect the actual allowed range (0-365) instead of 1-365; locate the comment
above the recycleBinRetentionDays property in the types.ts declaration and
change the text "default: 30, range: 1-365" to "default: 30, range: 0-365" so
consumers are not misled about the valid edge-case value 0.

---

Nitpick comments:
In `@apps/web/src/hooks/useBin.ts`:
- Line 23: The code now reads recycle bin retention directly via retentionDays =
useVaultSettingsStore((s) => s.settings.recycleBinRetentionDays) making the
assignment useBinStore.getState().setRetentionDays(...) in useAuth.ts redundant;
remove the call to
useBinStore.getState().setRetentionDays(vaultSettings.recycleBinRetentionDays)
from useAuth.ts (or gate it behind a deprecated flag) and, if safe, plan to
deprecate/remove useBinStore.retentionDays to avoid the stale write path—ensure
any consumers reference retentionDays from useVaultSettingsStore instead of
useBinStore.

In `@apps/web/src/services/file-metadata.service.ts`:
- Around line 245-253: Update the JSDoc/comment around shouldCreateVersion to
stop referencing the hard-coded VERSION_COOLDOWN_MS and instead state that the
cooldown is configurable via vault settings and obtained through
getVersionCooldownMs(). Mention that shouldCreateVersion checks
currentMetadata.versions (using the newest version's timestamp) and compares
Date.now() against the settings-driven cooldown returned by
getVersionCooldownMs().

In `@apps/web/src/services/vault-settings.service.ts`:
- Around line 40-41: The derived Ed25519 keypair returned by
deriveVaultSettingsIpnsKeypair should be explicitly cleared after use to reduce
exposure: in both loadVaultSettings and saveVaultSettings, wrap the usage of
keypair (the variable from deriveVaultSettingsIpnsKeypair) in a try/finally and
in the finally overwrite keypair.privateKey (and any other sensitive buffer
fields) with zeros or set to null to ensure it’s cleared even on errors;
reference the deriveVaultSettingsIpnsKeypair call and keypair.privateKey and add
the cleanup in both functions.

In `@packages/core/src/vault/settings.ts`:
- Around line 35-39: The validators in settings.ts use hard-coded default
literals (e.g., 30, 10, 15, 'bin') which can drift from the canonical
DEFAULT_VAULT_SETTINGS; update the code to read defaults from
DEFAULT_VAULT_SETTINGS (e.g., DEFAULT_VAULT_SETTINGS.recycleBinRetentionDays,
.maxVersionsPerFile, .versionCooldownMinutes, .deleteBehavior) when calling
toNumber/clamp and when validating raw.deleteBehavior so the fallback values
come from that single source of truth (adjust any occurrences around the consts
recycleBinRetentionDays, deleteBehavior, maxVersionsPerFile,
versionCooldownMinutes and the similar block at 54-57).
🪄 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: bcc53d51-ac33-4147-8682-244e70e92068

📥 Commits

Reviewing files that changed from the base of the PR and between 96455b3 and 7aa4069.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml, !**/pnpm-lock.yaml
📒 Files selected for processing (31)
  • .planning/ROADMAP.md
  • .planning/STATE.md
  • .planning/phases/39-user-configurable-vault-parameters/39-01-PLAN.md
  • .planning/phases/39-user-configurable-vault-parameters/39-02-PLAN.md
  • .planning/phases/39-user-configurable-vault-parameters/39-03-PLAN.md
  • .planning/phases/39-user-configurable-vault-parameters/39-03-SUMMARY.md
  • .planning/phases/39-user-configurable-vault-parameters/39-04-PLAN.md
  • .planning/phases/39-user-configurable-vault-parameters/39-04-SUMMARY.md
  • .planning/phases/39-user-configurable-vault-parameters/39-CONTEXT.md
  • .planning/phases/39-user-configurable-vault-parameters/39-DISCUSSION-LOG.md
  • .planning/phases/39-user-configurable-vault-parameters/39-RESEARCH.md
  • .planning/phases/39-user-configurable-vault-parameters/39-VALIDATION.md
  • apps/web/src/App.css
  • apps/web/src/components/settings/VaultTab.tsx
  • apps/web/src/hooks/useAuth.ts
  • apps/web/src/hooks/useBin.ts
  • apps/web/src/hooks/useFolderMutations.ts
  • apps/web/src/lib/clear-user-stores.ts
  • apps/web/src/routes/SettingsPage.tsx
  • apps/web/src/services/file-metadata.service.ts
  • apps/web/src/services/vault-settings.service.ts
  • apps/web/src/stores/vault-settings.store.ts
  • packages/core/src/__tests__/vault-settings.test.ts
  • packages/core/src/index.ts
  • packages/core/src/vault/index.ts
  • packages/core/src/vault/settings.ts
  • packages/core/src/vault/types.ts
  • packages/crypto/src/__tests__/vault-settings-ipns.test.ts
  • packages/crypto/src/index.ts
  • packages/crypto/src/vault/derive-ipns.ts
  • packages/crypto/src/vault/index.ts

Comment thread packages/core/src/__tests__/vault-settings.test.ts Outdated
Comment thread packages/core/src/vault/types.ts Outdated
@codecov

codecov Bot commented Mar 31, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 62.27%. Comparing base (96455b3) to head (9a33ff2).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #423      +/-   ##
==========================================
+ Coverage   62.01%   62.27%   +0.26%     
==========================================
  Files         133      134       +1     
  Lines       10012    10082      +70     
  Branches     1042     1050       +8     
==========================================
+ Hits         6209     6279      +70     
  Misses       3587     3587              
  Partials      216      216              
Flag Coverage Δ
api 84.92% <100.00%> (+0.18%) ⬆️
api-client 84.92% <100.00%> (+0.18%) ⬆️
core 84.92% <100.00%> (+0.18%) ⬆️
crypto 84.92% <100.00%> (+0.18%) ⬆️
sdk 84.92% <100.00%> (+0.18%) ⬆️
sdk-core 84.92% <100.00%> (+0.18%) ⬆️

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

Files with missing lines Coverage Δ
packages/core/src/vault/settings.ts 100.00% <100.00%> (ø)
packages/crypto/src/vault/derive-ipns.ts 57.30% <100.00%> (+14.87%) ⬆️
🚀 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.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 2dc891623b62
FSM1 and others added 2 commits March 31, 2026 12:41
useBin now reads retention from useVaultSettingsStore, making
bin store's retentionDays field write-only dead code. Removed the
field, setter, and both callers (useAuth, VaultTab).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: ba54d251a580

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 35 out of 36 changed files in this pull request and generated 4 comments.

Comment thread apps/web/src/hooks/useAuth.ts
Comment thread apps/web/src/components/settings/VaultTab.tsx
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: f862659678db
FSM1 and others added 2 commits March 31, 2026 13:59
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 37bdd802b935
Replace hard-coded literals (30, 10, 15, 'bin') in validateVaultSettings
with references to DEFAULT_VAULT_SETTINGS. Also update shouldCreateVersion
JSDoc to reference configurable cooldown instead of removed constant.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: e15e26ca9481

@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: 1

🧹 Nitpick comments (1)
apps/web/src/services/file-metadata.service.ts (1)

398-400: Rename prunedVersions to reflect retained entries.

slice(0, maxVer) keeps versions; current name reads as the opposite and adds cognitive friction.

✏️ Suggested rename for clarity
-  const prunedVersions = newVersions.slice(0, maxVer);
-  const prunedCids = newVersions.slice(maxVer).map((v) => v.cid);
+  const retainedVersions = newVersions.slice(0, maxVer);
+  const prunedCids = newVersions.slice(maxVer).map((v) => v.cid);
...
-    versions: prunedVersions.length > 0 ? prunedVersions : undefined,
+    versions: retainedVersions.length > 0 ? retainedVersions : undefined,
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/web/src/services/file-metadata.service.ts` around lines 398 - 400, The
variable prunedVersions is misnamed because newVersions.slice(0, maxVer)
actually selects the retained entries; rename prunedVersions to retainedVersions
(or keptVersions) and update all usages accordingly; keep prunedCids as the ones
removed (newVersions.slice(maxVer).map((v) => v.cid)); ensure you change the
declaration and any subsequent references to the new identifier and leave
getMaxVersionsPerFile(), newVersions, maxVer, and prunedCids unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/core/src/vault/settings.ts`:
- Around line 31-62: validateVaultSettings currently coerces any object into the
v1 shape and can overwrite newer schemas; update validateVaultSettings to first
check raw.version and if it's present and not 'v1' return the input unchanged
(preserve unknown schema) instead of normalizing fields. Locate
validateVaultSettings and use DEFAULT_VAULT_SETTINGS, and the field names
recycleBinRetentionDays, deleteBehavior, maxVersionsPerFile,
versionCooldownMinutes to ensure the early-return happens before any
clamping/coercion logic so unknown versions are not lost.

---

Nitpick comments:
In `@apps/web/src/services/file-metadata.service.ts`:
- Around line 398-400: The variable prunedVersions is misnamed because
newVersions.slice(0, maxVer) actually selects the retained entries; rename
prunedVersions to retainedVersions (or keptVersions) and update all usages
accordingly; keep prunedCids as the ones removed
(newVersions.slice(maxVer).map((v) => v.cid)); ensure you change the declaration
and any subsequent references to the new identifier and leave
getMaxVersionsPerFile(), newVersions, maxVer, and prunedCids unchanged.
🪄 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: 1972eef6-2582-4103-ab94-e7594f2166da

📥 Commits

Reviewing files that changed from the base of the PR and between bdb0d70 and 8b12f44.

📒 Files selected for processing (2)
  • apps/web/src/services/file-metadata.service.ts
  • packages/core/src/vault/settings.ts

Comment thread packages/core/src/vault/settings.ts
…able

- validateVaultSettings now returns defaults for unrecognized version
  strings instead of silently coercing future schemas to v1
- Rename prunedVersions to retainedVersions in restoreVersion since
  the variable holds kept entries, not removed ones

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 74a72014c69a
@FSM1 FSM1 enabled auto-merge (squash) March 31, 2026 12:20
@FSM1 FSM1 merged commit fa7b443 into main Mar 31, 2026
25 checks passed
@FSM1 FSM1 deleted the docs/phase-39-context branch March 31, 2026 12:27
FSM1 added a commit that referenced this pull request Apr 1, 2026
Add stub summaries for phases that shipped outside the GSD workflow
so project statistics accurately reflect completion state.

- Phase 38 (PR #422): 4 plan summaries + context status update
- Phase 39 (PR #423): 2 plan summaries + context status update

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: b901b387076f
FSM1 added a commit that referenced this pull request Apr 1, 2026
Add stub summaries for phases that shipped outside the GSD workflow
so project statistics accurately reflect completion state.

- Phase 38 (PR #422): 4 plan summaries + context status update
- Phase 39 (PR #423): 2 plan summaries + context status update


Entire-Checkpoint: b901b387076f

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants