Skip to content

Implement comprehensive account deletion flow - #17

Open
PastaPastaPasta wants to merge 3 commits into
masterfrom
claude/implement-delete-account-yvcQF
Open

Implement comprehensive account deletion flow#17
PastaPastaPasta wants to merge 3 commits into
masterfrom
claude/implement-delete-account-yvcQF

Conversation

@PastaPastaPasta

@PastaPastaPasta PastaPastaPasta commented Jan 13, 2026

Copy link
Copy Markdown
Owner
  • Add registry-based AccountDeletionService that deletes all user documents across all Yappr contracts (main, profile, DM, block, hashtag, key backup)
  • Create multi-step delete account modal with warning, confirmation (type DELETE), progress tracking, and completion/error states
  • Add clear warnings about third-party indexers retaining historical copies
  • Update privacy policy to reflect that deletion IS now possible from Dash Platform, with caveats about external services
  • Clean up local storage after successful deletion

The service uses a future-proof registry pattern - new document types just need to be added to the registry array to be included in deletion.

Summary by CodeRabbit

  • New Features
    • Multi-step account deletion modal with typed confirmation, progress bar, per-item progress, and completion/error screens.
  • Documentation
    • Privacy page updated to explain data deletion, lists items removed (posts, likes, replies, profile, follows, DMs, etc.), and warns third parties may retain copies.
  • Chores
    • Private keys now stored via a secure storage flow during creation processes.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Jan 13, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

@PastaPastaPasta has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 13 minutes and 27 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 50c018ac21996fcd6396b410aef7a8e2dde370c5 and d4b6e7d.

📒 Files selected for processing (5)
  • app/privacy/page.tsx
  • app/settings/page.tsx
  • components/settings/delete-account-modal.tsx
  • hooks/use-delete-account-modal.ts
  • lib/services/account-deletion-service.ts
📝 Walkthrough

Walkthrough

Adds a modal-driven, multi-step account deletion UX backed by a new AccountDeletionService and a Zustand modal store; updates privacy page text; integrates the modal into settings; and replaces sessionStorage private-key writes with secure storage calls in test-create flows.

Changes

Cohort / File(s) Summary
Privacy Page
app/privacy/page.tsx
Replace "Data Cannot Be Deleted" section with a new "Data Deletion" section listing items removed on account deletion and warning about third-party retained copies.
Settings Page Integration
app/settings/page.tsx
Replace inline confirmation with modal flow: import and mount DeleteAccountModal, use useDeleteAccountModal, and call openDeleteAccountModal from handleDeleteAccount.
Delete Account Modal (UI)
components/settings/delete-account-modal.tsx
NEW: Client-side multi-step modal (warning → confirm → progress → complete/error) that fetches per-type document counts, requires typing "DELETE", displays live progress and per-type details, prevents closing during deletion, and calls service to delete.
Modal Store Hook
hooks/use-delete-account-modal.ts
NEW: Zustand store for modal state and actions (isOpen, step, progress, result, documentCounts, isLoadingCounts, open/close/reset and setters).
Account Deletion Service
lib/services/account-deletion-service.ts
NEW: AccountDeletionService and exported accountDeletionService singleton with registry-driven cross-contract deletion: counting, paginated queries, batched concurrent deletions (batch size 3, 500ms delay), progress callbacks, granular error aggregation, local cleanup, and exported types (DeletionProgress, DeletionResult, DocumentCounts, etc.).
Test Create Key Storage
app/test-create/page.tsx
Replace direct sessionStorage private-key writes with storePrivateKey(identityId, privateKey) secure storage calls in post/profile creation flows.
Package/Manifest
package.json
Manifest/packaging adjustments reflecting added modules/files.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant SettingsPage
    participant ModalStore as useDeleteAccountModal
    participant DeleteAccountModal
    participant DeletionService as accountDeletionService
    participant Contracts as Contracts/DB
    participant LocalStorage as Local Storage

    User->>SettingsPage: Click "Delete account"
    SettingsPage->>ModalStore: open()
    ModalStore-->>DeleteAccountModal: render (isOpen)

    DeleteAccountModal->>DeletionService: countUserDocuments(userId)
    DeletionService->>Contracts: query counts per contract/type
    Contracts-->>DeletionService: counts
    DeletionService-->>DeleteAccountModal: documentCounts
    DeleteAccountModal->>ModalStore: setDocumentCounts()

    User->>DeleteAccountModal: Confirm (type "DELETE")
    DeleteAccountModal->>ModalStore: setStep('progress')
    DeleteAccountModal->>DeletionService: deleteAccount(userId, onProgress)

    loop batched deletions
      DeletionService->>Contracts: deleteDocument(doc)
      Contracts-->>DeletionService: success / error
      DeletionService-->>DeleteAccountModal: onProgress(update)
    end

    DeletionService->>LocalStorage: clearLocalStorage(userId)
    LocalStorage-->>DeletionService: cleared
    DeletionService-->>DeleteAccountModal: final DeletionResult
    DeleteAccountModal->>ModalStore: setResult(), setStep('complete' or 'error')
    DeleteAccountModal->>User: show completion or error UI
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰
I hopped through counts and batches bright,
Typed DELETE by moonlit night,
Progress bars like carrots gleamed,
Keys tucked safe where secrets dreamed,
Farewell account — off I bound in light.

🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% 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 title 'Implement comprehensive account deletion flow' directly and accurately summarizes the main change: introducing a complete account deletion system with modal UI, service layer, and multi-step workflow.

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


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

@PastaPastaPasta

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jan 13, 2026

Copy link
Copy Markdown
✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

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

🤖 Fix all issues with AI agents
In @components/settings/delete-account-modal.tsx:
- Around line 116-119: handleCompleteLogout currently only calls handleClose and
router.push('/login'); update it to call the auth context logout() (e.g.,
logout()) to perform the full session teardown (setUser(null), clear identity,
invalidate caches, clear keys) before closing the modal and navigating; if
logout() is async, await it and handle errors, then call handleClose() and
router.push('/login') so the session is properly terminated prior to redirect.

In @lib/services/account-deletion-service.ts:
- Line 203: Replace the direct, casted SDK call to
sdk.documents.query(queryOptions as any) with the typed helper queryDocuments to
preserve typings and normalization: call queryDocuments(sdk, queryOptions)
instead of sdk.documents.query, then pass its result through mapToDocumentArray
to get the normalized documents array and update downstream code that consumes
`response` to use the returned normalized documents (replace references to
`response` with the new `documents` variable derived from mapToDocumentArray);
remove the unsafe `as any` cast and any SDK-specific response handling because
queryDocuments handles normalization.
- Around line 377-398: The clearLocalStorage method in
account-deletion-service.ts fails to remove the encrypted credentials; update
clearLocalStorage(userId) to also remove the 'yappr_encrypted_credentials' entry
(from lib/password-encrypted-storage.ts) when window is defined, and ensure any
secure-storage cleanup (e.g., clearPrivateKey) runs before/after removing that
item; additionally, audit app/test-create/page.tsx and replace any direct
sessionStorage use of 'yappr_pk' with the secure-storage API in
lib/secure-storage.ts so private keys are never written in plain text to
sessionStorage.
🧹 Nitpick comments (3)
components/settings/delete-account-modal.tsx (2)

24-37: close action is destructured but never used.

The close action is destructured from the store but handleClose calls reset() instead. Since reset() sets isOpen: false along with resetting all other state, this is correct behavior, but close can be removed from the destructuring to avoid confusion.

♻️ Suggested cleanup
   const {
     isOpen,
     step,
     progress,
     result,
     documentCounts,
     isLoadingCounts,
     setStep,
     setProgress,
     setResult,
     setDocumentCounts,
     setIsLoadingCounts,
     reset
-  } = useDeleteAccountModal()
+  } = useDeleteAccountModal()

Note: The destructuring is already correct - close is not included. No change needed.


507-545: Consider adding ARIA attributes for better accessibility.

The modal implementation is functionally solid but could benefit from accessibility enhancements for screen reader users:

  • Add role="dialog" and aria-modal="true" to the modal container
  • Add aria-labelledby pointing to the heading
  • Consider focus trapping to prevent keyboard navigation from escaping the modal
♿ Accessibility improvement
           <motion.div
             initial={{ opacity: 0, scale: 0.95 }}
             animate={{ opacity: 1, scale: 1 }}
             exit={{ opacity: 0, scale: 0.95 }}
             transition={{ duration: 0.2 }}
             className="fixed inset-0 flex items-center justify-center z-50 px-4 overflow-y-auto py-8"
           >
-            <div className="bg-white dark:bg-gray-900 rounded-2xl shadow-xl p-6 max-w-md w-full relative my-auto">
+            <div 
+              role="dialog"
+              aria-modal="true"
+              aria-labelledby="delete-account-title"
+              className="bg-white dark:bg-gray-900 rounded-2xl shadow-xl p-6 max-w-md w-full relative my-auto"
+            >

Then update headings in each step to include the id="delete-account-title" attribute.

lib/services/account-deletion-service.ts (1)

192-242: Pagination logic is sound, but consider adding a safety limit.

The while(true) pagination loop correctly breaks when:

  1. No documents are returned
  2. Fewer documents than QUERY_LIMIT are returned
  3. An error occurs

However, for defensive programming, consider adding a maximum iteration count to prevent infinite loops in edge cases (e.g., if the API returns exactly 100 documents repeatedly due to a bug).

🛡️ Optional safety limit
   private async queryUserDocuments(
     contractId: string,
     documentType: string,
     userId: string
   ): Promise<DocumentInfo[]> {
     const sdk = await getEvoSdk()
     const allDocuments: DocumentInfo[] = []
     let startAfter: string | undefined = undefined
+    const MAX_PAGES = 100 // Safety limit: 10,000 documents max

     // Paginate through all documents
-    while (true) {
+    for (let page = 0; page < MAX_PAGES; page++) {
       try {
         // ... existing code
       }
     }
+    
+    return allDocuments
   }
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3700d4f and 92ba29273f3c680f95971584f3542141eac54eac.

📒 Files selected for processing (5)
  • app/privacy/page.tsx
  • app/settings/page.tsx
  • components/settings/delete-account-modal.tsx
  • hooks/use-delete-account-modal.ts
  • lib/services/account-deletion-service.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: SDK initialization and connection management must be handled through lib/services/evo-sdk-service.ts
All Dash Platform write operations must be handled by lib/services/state-transition-service.ts
Query operations for reading documents must be handled by lib/services/document-service.ts
Username resolution must be handled via lib/services/dpns-service.ts and components/dpns/ components
When creating Dash Platform documents, only include content fields - ownership via $ownerId is automatic and should not be explicitly set
Private keys must be stored via biometric storage (lib/biometric-storage.ts) or session storage (lib/secure-storage.ts), never in plain text or local storage
Use Zustand store in lib/store.ts for state management
Handle DAPI Gateway timeouts by using short timeout for confirmation wait, assuming success if broadcast succeeded but wait times out, and updating UI immediately after broadcast
Use mock data from lib/mock-data.ts for development when not connected to Dash Platform
Use contexts/auth-context.tsx to manage user sessions
Post content must enforce 500 character limit

Files:

  • components/settings/delete-account-modal.tsx
  • app/privacy/page.tsx
  • app/settings/page.tsx
  • hooks/use-delete-account-modal.ts
  • lib/services/account-deletion-service.ts
**/*.tsx

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.tsx: Use Tailwind CSS with custom design system from tailwind.config.js for styling
Use Radix UI primitives from components/ui/ for UI components

Files:

  • components/settings/delete-account-modal.tsx
  • app/privacy/page.tsx
  • app/settings/page.tsx
🧠 Learnings (1)
📚 Learning: 2026-01-12T21:40:25.650Z
Learnt from: CR
Repo: PastaPastaPasta/yappr PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T21:40:25.650Z
Learning: Applies to **/*.{ts,tsx} : Use Zustand store in `lib/store.ts` for state management

Applied to files:

  • hooks/use-delete-account-modal.ts
🧬 Code graph analysis (3)
app/settings/page.tsx (2)
hooks/use-delete-account-modal.ts (1)
  • useDeleteAccountModal (34-52)
components/settings/delete-account-modal.tsx (1)
  • DeleteAccountModal (21-546)
hooks/use-delete-account-modal.ts (1)
lib/services/account-deletion-service.ts (3)
  • DeletionProgress (34-48)
  • DeletionResult (53-59)
  • DocumentCounts (64-66)
lib/services/account-deletion-service.ts (4)
lib/constants.ts (6)
  • YAPPR_CONTRACT_ID (6-6)
  • YAPPR_PROFILE_CONTRACT_ID (7-7)
  • YAPPR_DM_CONTRACT_ID (8-8)
  • YAPPR_BLOCK_CONTRACT_ID (9-9)
  • ENCRYPTED_KEY_BACKUP_CONTRACT_ID (11-11)
  • HASHTAG_CONTRACT_ID (12-12)
lib/secure-storage.ts (1)
  • clearPrivateKey (185-187)
lib/caches/block-cache.ts (1)
  • invalidateBlockCache (102-107)
lib/dash-platform-client.ts (1)
  • getDashPlatformClient (494-499)
🔇 Additional comments (7)
hooks/use-delete-account-modal.ts (1)

1-52: Clean Zustand store implementation for modal state management.

The store structure is well-organized with clear separation of state and actions. A few observations:

  1. The close() action only sets isOpen: false but doesn't reset other state. When the modal is reopened, it may show stale progress, result, or documentCounts from a previous session. Consider whether close() should also call reset() or if the current behavior (allowing handleClose in the modal to explicitly call reset()) is intentional.

  2. Per coding guidelines, state management should use lib/store.ts. However, a dedicated store for modal-specific state is a reasonable pattern for component isolation.

app/settings/page.tsx (1)

33-34: Clean modal integration following established patterns.

The integration correctly:

  • Imports and renders DeleteAccountModal at the top of the component tree
  • Uses the Zustand hook to access the open action
  • Delegates all modal logic to the modal component itself

This follows the same pattern as useDashPayContactsModal (line 210), maintaining consistency across the codebase.

Also applies to: 121-125, 532-532

app/privacy/page.tsx (1)

135-157: Privacy policy accurately reflects the new deletion capabilities.

The updated section:

  • Clearly communicates that deletion is now possible
  • Lists the specific data types that will be removed (matching the service implementation)
  • Appropriately warns users about third-party data retention

The amber warning box for third-party indexers is a good UX choice, ensuring users have realistic expectations.

lib/services/account-deletion-service.ts (4)

1-2: Good adherence to coding guidelines for SDK and service usage.

The service correctly uses getEvoSdk for SDK access and stateTransitionService for write operations, following the project's established patterns.


81-134: Registry-based architecture enables easy extensibility.

The registry pattern is well-designed:

  • Clear documentation about adding new document types
  • Human-readable service names for UI display
  • Comprehensive coverage of all Yappr contracts

This design makes it straightforward to add new document types when the platform expands.


409-428: Local storage cleanup occurs even on partial failures.

The condition on line 411 (result.totalDeleted > 0 || result.totalFailed === 0) means local storage is cleared:

  1. If any documents were deleted (even with failures)
  2. If there were zero failures and zero documents (empty account)

This seems intentional but verify this is the desired behavior. On partial failure, the user may retry deletion, but their session will already be cleared, potentially causing issues.


357-372: The stateTransitionService.deleteDocument method signature accepts exactly 4 parameters in the order: contractId, documentType, documentId, and ownerId. The code correctly passes doc.contractId, doc.documentType, doc.$id, and doc.$ownerId respectively, which aligns perfectly with the expected signature. No issues found.

Comment on lines +116 to +119
const handleCompleteLogout = () => {
handleClose()
router.push('/login')
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

rg -n -A 10 'function.*logout|const logout' contexts/auth-context.tsx

Repository: PastaPastaPasta/yappr

Length of output: 542


🏁 Script executed:

rg -n -A 25 'const logout = useCallback' contexts/auth-context.tsx

Repository: PastaPastaPasta/yappr

Length of output: 1030


🏁 Script executed:

rg -n -B 5 -A 15 'handleCompleteLogout' components/settings/delete-account-modal.tsx

Repository: PastaPastaPasta/yappr

Length of output: 1910


Call the logout() function from auth context to properly terminate the session.

handleCompleteLogout should invoke logout() from the auth context instead of only calling handleClose() and navigating to /login. The auth context's logout function performs critical cleanup including clearing user state (setUser(null)), clearing the DashPlatformClient identity, invalidating the block cache, and securely clearing private keys—all of which are necessary for proper session termination.

🤖 Prompt for AI Agents
In @components/settings/delete-account-modal.tsx around lines 116 - 119,
handleCompleteLogout currently only calls handleClose and router.push('/login');
update it to call the auth context logout() (e.g., logout()) to perform the full
session teardown (setUser(null), clear identity, invalidate caches, clear keys)
before closing the modal and navigating; if logout() is async, await it and
handle errors, then call handleClose() and router.push('/login') so the session
is properly terminated prior to redirect.

...(startAfter && { startAfter })
}

const response = await sdk.documents.query(queryOptions as any)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n lib/services/account-deletion-service.ts | sed -n '190,220p'

Repository: PastaPastaPasta/yappr

Length of output: 1228


🏁 Script executed:

# Get the full context of queryOptions construction
cat -n lib/services/account-deletion-service.ts | sed -n '1,210p'

Repository: PastaPastaPasta/yappr

Length of output: 7217


🏁 Script executed:

# Search for the queryOptions variable definition in the file
rg -A 5 -B 5 "queryOptions" lib/services/account-deletion-service.ts

Repository: PastaPastaPasta/yappr

Length of output: 722


🏁 Script executed:

# Check the @dashevo/evo-sdk package to understand the types
fd package.json | xargs grep -l "@dashevo/evo-sdk" | head -5

Repository: PastaPastaPasta/yappr

Length of output: 77


🏁 Script executed:

cat package.json | jq '.dependencies | select(. != null) | to_entries[] | select(.key | contains("dashevo")) | "\(.key): \(.value)"'

Repository: PastaPastaPasta/yappr

Length of output: 97


🏁 Script executed:

# Check the document-service.ts to see proper typing
cat -n lib/services/document-service.ts | head -100

Repository: PastaPastaPasta/yappr

Length of output: 3653


🏁 Script executed:

# Look for the query method signature in document-service
rg -A 10 "query" lib/services/document-service.ts | head -50

Repository: PastaPastaPasta/yappr

Length of output: 1556


🏁 Script executed:

# Check all imports from @dashevo/evo-sdk in the codebase
rg "from.*@dashevo/evo-sdk" --type ts --type tsx

Repository: PastaPastaPasta/yappr

Length of output: 92


🏁 Script executed:

# Check sdk-helpers to see how queryDocuments handles typing properly
cat -n lib/services/sdk-helpers.ts | head -150

Repository: PastaPastaPasta/yappr

Length of output: 5114


🏁 Script executed:

# Search for other direct sdk.documents.query calls to see if this is pattern
rg "sdk\.documents\.query" --type ts

Repository: PastaPastaPasta/yappr

Length of output: 4320


🏁 Script executed:

# Look at how account-deletion-service should be structured
rg "from.*document-service" lib/services/

Repository: PastaPastaPasta/yappr

Length of output: 1220


🏁 Script executed:

# Get the queryDocuments function signature from sdk-helpers
cat -n lib/services/sdk-helpers.ts | sed -n '200,250p'

Repository: PastaPastaPasta/yappr

Length of output: 1720


🏁 Script executed:

# Search for the full queryDocuments implementation
rg -A 15 "^export.*function queryDocuments" lib/services/sdk-helpers.ts

Repository: PastaPastaPasta/yappr

Length of output: 471


🏁 Script executed:

# Check QueryDocumentsOptions type definition
rg -B 5 "interface QueryDocumentsOptions" lib/services/sdk-helpers.ts

Repository: PastaPastaPasta/yappr

Length of output: 194


🏁 Script executed:

# See how other services properly use queryDocuments without the cast
rg -A 3 "queryDocuments\(sdk" lib/services/document-service.ts

Repository: PastaPastaPasta/yappr

Length of output: 235


🏁 Script executed:

# Check if account-deletion-service should extend BaseDocumentService
rg -B 5 "class AccountDeletionService" lib/services/account-deletion-service.ts

Repository: PastaPastaPasta/yappr

Length of output: 310


🏁 Script executed:

# Get the full QueryDocumentsOptions interface definition
rg -A 10 "export interface QueryDocumentsOptions" lib/services/sdk-helpers.ts

Repository: PastaPastaPasta/yappr

Length of output: 297


🏁 Script executed:

# Check if post-service.ts also uses as any and why
rg -B 5 -A 5 "as any" lib/services/post-service.ts | head -30

Repository: PastaPastaPasta/yappr

Length of output: 934


Use the queryDocuments helper for properly typed query operations.

The as any cast on line 203 bypasses type checking unnecessarily. The query object structure already matches the QueryDocumentsOptions interface from sdk-helpers.ts. Additionally, per coding guidelines, query operations should use the queryDocuments helper (as done in document-service.ts) to ensure consistent typing and handle response normalization.

Replace:

const response = await sdk.documents.query(queryOptions as any)

With:

const rawDocuments = await queryDocuments(sdk, queryOptions)
const documents = mapToDocumentArray(rawDocuments)

Then adjust response handling accordingly since queryDocuments already normalizes the SDK response.

🤖 Prompt for AI Agents
In @lib/services/account-deletion-service.ts at line 203, Replace the direct,
casted SDK call to sdk.documents.query(queryOptions as any) with the typed
helper queryDocuments to preserve typings and normalization: call
queryDocuments(sdk, queryOptions) instead of sdk.documents.query, then pass its
result through mapToDocumentArray to get the normalized documents array and
update downstream code that consumes `response` to use the returned normalized
documents (replace references to `response` with the new `documents` variable
derived from mapToDocumentArray); remove the unsafe `as any` cast and any
SDK-specific response handling because queryDocuments handles normalization.

Comment on lines +377 to +398
async clearLocalStorage(userId: string): Promise<void> {
// Clear session storage items
if (typeof window !== 'undefined') {
localStorage.removeItem('yappr_session')
sessionStorage.removeItem('yappr_dpns_username')
sessionStorage.removeItem('yappr_skip_dpns')
sessionStorage.removeItem('yappr_backup_prompt_shown')

// Clear private key from secure storage
const { clearPrivateKey } = await import('../secure-storage')
clearPrivateKey(userId)

// Clear block cache
const { invalidateBlockCache } = await import('../caches/block-cache')
invalidateBlockCache(userId)

// Clear DashPlatformClient identity
const { getDashPlatformClient } = await import('../dash-platform-client')
const dashClient = getDashPlatformClient()
dashClient.setIdentity('')
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find all localStorage and sessionStorage usage to verify cleanup is comprehensive
echo "=== localStorage usage ==="
rg -n "localStorage\.(setItem|getItem|removeItem)" --type ts --type tsx -g '!node_modules' | head -40

echo ""
echo "=== sessionStorage usage ==="
rg -n "sessionStorage\.(setItem|getItem|removeItem)" --type ts --type tsx -g '!node_modules' | head -40

Repository: PastaPastaPasta/yappr

Length of output: 194


🏁 Script executed:

#!/bin/bash
# Find all localStorage and sessionStorage usage without specifying file types
echo "=== localStorage usage ==="
rg "localStorage\.(setItem|getItem|removeItem)" -g '*.ts' -g '*.tsx' -n

echo ""
echo "=== sessionStorage usage ==="
rg "sessionStorage\.(setItem|getItem|removeItem)" -g '*.ts' -g '*.tsx' -n

Repository: PastaPastaPasta/yappr

Length of output: 4613


🏁 Script executed:

#!/bin/bash
# Check secure-storage.ts to understand proper private key handling
echo "=== secure-storage.ts overview ==="
cat -n lib/secure-storage.ts | head -50

echo ""
echo "=== 'yappr_pk' usage in test-create/page.tsx ==="
rg -B2 -A2 "yappr_pk" app/test-create/page.tsx

Repository: PastaPastaPasta/yappr

Length of output: 2249


🏁 Script executed:

#!/bin/bash
# Check biometric-storage.ts existence and implementation
echo "=== biometric-storage.ts ==="
if [ -f "lib/biometric-storage.ts" ]; then
  wc -l lib/biometric-storage.ts
  cat -n lib/biometric-storage.ts | head -60
else
  echo "File not found"
fi

echo ""
echo "=== Check what secure-storage stores and clears ==="
rg -A5 "storePrivateKey|clearPrivateKey" lib/secure-storage.ts

Repository: PastaPastaPasta/yappr

Length of output: 625


🏁 Script executed:

#!/bin/bash
# Get the full clearAllPrivateKeys implementation
echo "=== Full clearAllPrivateKeys implementation ==="
rg -A10 "clearAllPrivateKeys" lib/secure-storage.ts

echo ""
echo "=== Check what 'feed-tab' is used for ==="
rg -B2 -A2 "feed-tab" app/feed/page.tsx | head -30

echo ""
echo "=== Check app/test-create/page.tsx to understand yappr_pk context ==="
rg -B5 -A10 "yappr_pk" app/test-create/page.tsx | head -40

Repository: PastaPastaPasta/yappr

Length of output: 1945


🏁 Script executed:

#!/bin/bash
# Check the complete clearLocalStorage method and surrounding context
echo "=== Full clearLocalStorage method ==="
rg -B5 -A30 "async clearLocalStorage" lib/services/account-deletion-service.ts

echo ""
echo "=== Check if secureStorage.clearAll() exists and what it does ==="
rg -A8 "clearAll()" lib/secure-storage.ts

echo ""
echo "=== Check password-encrypted-storage STORAGE_KEY ==="
rg "STORAGE_KEY" lib/password-encrypted-storage.ts | head -5

Repository: PastaPastaPasta/yappr

Length of output: 1841


🏁 Script executed:

#!/bin/bash
# Check if there are other calls to clearLocalStorage or related cleanup
echo "=== Check where clearLocalStorage is called ==="
rg "clearLocalStorage" -B2 -A2

echo ""
echo "=== Check if encrypted credentials should be cleared on logout ==="
rg -B3 -A3 "yappr_encrypted_credentials" lib/password-encrypted-storage.ts

echo ""
echo "=== Check if there's any cleanup of 'feed-tab' elsewhere ==="
rg "feed-tab" -B2 -A2

Repository: PastaPastaPasta/yappr

Length of output: 1976


Add clearance of encrypted credentials storage during account cleanup.

The cleanup is incomplete. Beyond the keys currently being removed, the clearLocalStorage method must also clear yappr_encrypted_credentials (from lib/password-encrypted-storage.ts), which stores sensitive credential data and persists across sessions.

Additionally, audit and remediate the direct storage of yappr_pk in sessionStorage at app/test-create/page.tsx. Per coding guidelines, private keys must be stored via lib/secure-storage.ts, not plain text storage.

🤖 Prompt for AI Agents
In @lib/services/account-deletion-service.ts around lines 377 - 398, The
clearLocalStorage method in account-deletion-service.ts fails to remove the
encrypted credentials; update clearLocalStorage(userId) to also remove the
'yappr_encrypted_credentials' entry (from lib/password-encrypted-storage.ts)
when window is defined, and ensure any secure-storage cleanup (e.g.,
clearPrivateKey) runs before/after removing that item; additionally, audit
app/test-create/page.tsx and replace any direct sessionStorage use of 'yappr_pk'
with the secure-storage API in lib/secure-storage.ts so private keys are never
written in plain text to sessionStorage.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jan 13, 2026

Copy link
Copy Markdown

Deploying yappr with  Cloudflare Pages  Cloudflare Pages

Latest commit: d4b6e7d
Status:🚫  Build failed.

View logs

@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 (4)
app/test-create/page.tsx (1)

16-39: Minor: Consider adding input validation for the test page.

While the buttons are correctly disabled when privateKey or identityId are empty, there's no format validation before storing/using these values. For a test page this is acceptable, but adding basic validation (e.g., checking identityId is a valid Base58 string) could help catch user input errors earlier.

components/settings/delete-account-modal.tsx (2)

89-95: Minor: Simplify redundant conditional branches.

Lines 91-95 have two branches that both call setStep('error'). This can be simplified.

♻️ Suggested simplification
       if (deletionResult.success) {
         setStep('complete')
-      } else if (deletionResult.partialFailure) {
-        setStep('error')
       } else {
         setStep('error')
       }

514-552: Consider: Add focus trap for accessibility.

The modal lacks a focus trap, which could allow keyboard users to tab outside the modal while it's open. Consider using a focus trap library or Radix UI's Dialog primitive which handles this automatically.

lib/services/account-deletion-service.ts (1)

456-479: Consider: Keep deletion summary in sync with registry.

The human-readable summary is hardcoded separately from the registry. If new document types are added to the registry, this summary may become stale. Consider adding a comment noting this coupling, or deriving the summary from registry metadata.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 92ba29273f3c680f95971584f3542141eac54eac and 4672bd63c4b34d738f8c687303271481053ab419.

📒 Files selected for processing (3)
  • app/test-create/page.tsx
  • components/settings/delete-account-modal.tsx
  • lib/services/account-deletion-service.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: SDK initialization and connection management must be handled through lib/services/evo-sdk-service.ts
All Dash Platform write operations must be handled by lib/services/state-transition-service.ts
Query operations for reading documents must be handled by lib/services/document-service.ts
Username resolution must be handled via lib/services/dpns-service.ts and components/dpns/ components
When creating Dash Platform documents, only include content fields - ownership via $ownerId is automatic and should not be explicitly set
Private keys must be stored via biometric storage (lib/biometric-storage.ts) or session storage (lib/secure-storage.ts), never in plain text or local storage
Use Zustand store in lib/store.ts for state management
Handle DAPI Gateway timeouts by using short timeout for confirmation wait, assuming success if broadcast succeeded but wait times out, and updating UI immediately after broadcast
Use mock data from lib/mock-data.ts for development when not connected to Dash Platform
Use contexts/auth-context.tsx to manage user sessions
Post content must enforce 500 character limit

Files:

  • app/test-create/page.tsx
  • components/settings/delete-account-modal.tsx
  • lib/services/account-deletion-service.ts
**/*.tsx

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.tsx: Use Tailwind CSS with custom design system from tailwind.config.js for styling
Use Radix UI primitives from components/ui/ for UI components

Files:

  • app/test-create/page.tsx
  • components/settings/delete-account-modal.tsx
🧠 Learnings (3)
📚 Learning: 2026-01-12T21:40:25.650Z
Learnt from: CR
Repo: PastaPastaPasta/yappr PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T21:40:25.650Z
Learning: Applies to **/*.{ts,tsx} : Private keys must be stored via biometric storage (`lib/biometric-storage.ts`) or session storage (`lib/secure-storage.ts`), never in plain text or local storage

Applied to files:

  • app/test-create/page.tsx
  • lib/services/account-deletion-service.ts
📚 Learning: 2026-01-12T21:40:25.650Z
Learnt from: CR
Repo: PastaPastaPasta/yappr PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T21:40:25.650Z
Learning: Applies to **/*.{ts,tsx} : Use `contexts/auth-context.tsx` to manage user sessions

Applied to files:

  • components/settings/delete-account-modal.tsx
📚 Learning: 2026-01-12T21:40:25.650Z
Learnt from: CR
Repo: PastaPastaPasta/yappr PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T21:40:25.650Z
Learning: Applies to **/*.{ts,tsx} : Query operations for reading documents must be handled by `lib/services/document-service.ts`

Applied to files:

  • lib/services/account-deletion-service.ts
🧬 Code graph analysis (2)
app/test-create/page.tsx (3)
lib/secure-storage.ts (1)
  • storePrivateKey (177-179)
register-contract-with-nonce.js (2)
  • identityId (22-22)
  • privateKey (23-23)
test-dpns-resolve.js (1)
  • identityId (5-5)
components/settings/delete-account-modal.tsx (3)
contexts/auth-context.tsx (1)
  • useAuth (399-405)
hooks/use-delete-account-modal.ts (1)
  • useDeleteAccountModal (34-52)
lib/services/account-deletion-service.ts (2)
  • accountDeletionService (483-483)
  • DeletionProgress (35-49)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Cloudflare Pages
🔇 Additional comments (15)
app/test-create/page.tsx (1)

5-5: LGTM! Proper use of secure storage for private keys.

The change correctly replaces direct sessionStorage usage with storePrivateKey from the secure-storage API, aligning with the coding guideline that private keys must be stored via lib/secure-storage.ts. Based on learnings, this is the required pattern.

Also applies to: 21-22, 46-47

components/settings/delete-account-modal.tsx (7)

43-64: LGTM! Document counting effect is well-structured.

The effect correctly:

  • Guards against missing user identity
  • Prevents duplicate fetches with isLoadingCounts and documentCounts === null checks
  • Gracefully handles errors by setting empty counts and continuing
  • Includes all necessary dependencies

136-242: LGTM! Well-designed warning step with appropriate safeguards.

The warning step includes:

  • Clear visual hierarchy and iconography
  • Loading state for document counts
  • Comprehensive deletion summary
  • Important third-party indexer warning
  • Required consent checkbox before proceeding

244-295: LGTM! Strong confirmation step with explicit user action required.

The "type DELETE to confirm" pattern is a solid UX choice for destructive actions. Input attributes properly disable autocomplete/autocorrect.


297-364: LGTM! Progress step provides good feedback.

Nice touches:

  • Division by zero protected with Math.max(..., 1)
  • Real-time stats for deleted/failed documents
  • Clear "Do not close" warning

366-397: LGTM!

Clean completion state with clear call-to-action.


399-495: LGTM! Error handling provides good user options.

The error step appropriately:

  • Distinguishes partial vs. full failure
  • Limits error display to 5 items to prevent UI overflow
  • Offers sensible recovery options (retry or continue/cancel)

116-126: The logout() function from the auth context already handles navigation to /login at the end of its execution (line 273 in contexts/auth-context.tsx). Since handleCompleteLogout only navigates in the catch block for error cases, there is no double navigation risk—the success and error paths are correctly separated, with logout's internal navigation handling the success case and the error handler providing a fallback.

lib/services/account-deletion-service.ts (7)

82-135: LGTM! Well-designed registry pattern.

The registry is:

  • Comprehensive across all 6 Yappr contracts
  • Self-documenting with service names
  • Extensible with clear instructions for adding new types

Note: profile appears in both Main (line 95, "old contract") and Profile (line 103) contracts - this appears intentional for handling legacy data.


155-177: LGTM! Resilient document counting.

Error tolerance is appropriate here since counts are informational. The method correctly continues on failure rather than aborting.


183-237: LGTM! Robust pagination implementation.

The pagination handles:

  • Proper startAfter cursor management
  • Multiple fallback patterns for ID extraction (handles SDK inconsistencies)
  • Graceful failure on query errors (breaks loop, logs warning)

242-344: LGTM! Well-structured batch deletion with proper error handling.

Key strengths:

  • Promise.allSettled correctly handles partial failures
  • Batching (size 3) with 500ms delays helps avoid rate limiting
  • Granular progress updates enable responsive UI
  • Error capture handles both promise rejections and operation failures

348-364: LGTM! Proper use of state transition service.

Correctly delegates to stateTransitionService.deleteDocument per coding guidelines for Dash Platform write operations.


368-394: LGTM! Comprehensive local cleanup.

Properly clears:

  • Session/local storage entries
  • Private keys via clearPrivateKey (per secure storage guidelines)
  • Password-encrypted credentials
  • Cached data (block cache)
  • Platform client state

399-424: LGTM! Clean orchestration logic.

The condition at line 407 correctly handles edge cases:

  • totalDeleted > 0: Clear if anything was deleted (even with some failures)
  • totalFailed === 0: Clear if no failures (handles users with no documents)

Comment on lines +1 to +2
import { getEvoSdk } from './evo-sdk-service'
import { queryDocuments, identifierToBase58 } from './sdk-helpers'

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Check document-service.ts exports
rg -n 'export' lib/services/document-service.ts | head -20

Repository: PastaPastaPasta/yappr

Length of output: 188


🏁 Script executed:

# Check if sdk-helpers has queryDocuments
rg -n 'export.*queryDocuments' lib/services/sdk-helpers.ts

Repository: PastaPastaPasta/yappr

Length of output: 106


🏁 Script executed:

# Check the full account-deletion-service.ts file
wc -l lib/services/account-deletion-service.ts

Repository: PastaPastaPasta/yappr

Length of output: 109


🏁 Script executed:

# Check if document-service.ts has any query-related functions
rg -n 'query' lib/services/document-service.ts -i

Repository: PastaPastaPasta/yappr

Length of output: 710


🏁 Script executed:

# Check how account-deletion-service.ts uses queryDocuments
rg -B2 -A5 'queryDocuments' lib/services/account-deletion-service.ts

Repository: PastaPastaPasta/yappr

Length of output: 622


🏁 Script executed:

# Check if account-deletion-service.ts imports or uses BaseDocumentService
rg -n 'BaseDocumentService|document-service' lib/services/account-deletion-service.ts

Repository: PastaPastaPasta/yappr

Length of output: 47


Use lib/services/document-service.ts for query operations instead of importing directly from sdk-helpers.

The coding guidelines require all query operations for reading documents to be handled through lib/services/document-service.ts. This service currently imports queryDocuments directly from sdk-helpers, bypassing the BaseDocumentService abstraction layer which provides caching, error handling, and logging. Refactor to use BaseDocumentService to align with the architectural guidelines.

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

🧹 Nitpick comments (1)
lib/services/account-deletion-service.ts (1)

155-177: Consider lazy counting or caching for performance.

This method fetches all documents (paginated) just to count them. For a user with many posts/likes, this could result in dozens of API calls before deletion even begins.

If counting is only needed for UI display, consider:

  1. Using estimated counts or lazy loading
  2. Starting deletion without exact counts and updating progress dynamically
  3. Caching counts if the modal can be reopened

This isn't blocking since Dash Platform may not support COUNT queries, but worth noting for UX on accounts with large amounts of data.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4672bd63c4b34d738f8c687303271481053ab419 and 50c018ac21996fcd6396b410aef7a8e2dde370c5.

📒 Files selected for processing (1)
  • lib/services/account-deletion-service.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: SDK initialization and connection management must be handled through lib/services/evo-sdk-service.ts
All Dash Platform write operations must be handled by lib/services/state-transition-service.ts
Query operations for reading documents must be handled by lib/services/document-service.ts
Username resolution must be handled via lib/services/dpns-service.ts and components/dpns/ components
When creating Dash Platform documents, only include content fields - ownership via $ownerId is automatic and should not be explicitly set
Private keys must be stored via biometric storage (lib/biometric-storage.ts) or session storage (lib/secure-storage.ts), never in plain text or local storage
Use Zustand store in lib/store.ts for state management
Handle DAPI Gateway timeouts by using short timeout for confirmation wait, assuming success if broadcast succeeded but wait times out, and updating UI immediately after broadcast
Use mock data from lib/mock-data.ts for development when not connected to Dash Platform
Use contexts/auth-context.tsx to manage user sessions
Post content must enforce 500 character limit

Files:

  • lib/services/account-deletion-service.ts
🧠 Learnings (3)
📚 Learning: 2026-01-12T21:40:25.650Z
Learnt from: CR
Repo: PastaPastaPasta/yappr PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T21:40:25.650Z
Learning: Applies to **/*.{ts,tsx} : Query operations for reading documents must be handled by `lib/services/document-service.ts`

Applied to files:

  • lib/services/account-deletion-service.ts
📚 Learning: 2026-01-12T21:40:25.650Z
Learnt from: CR
Repo: PastaPastaPasta/yappr PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T21:40:25.650Z
Learning: Applies to **/*.{ts,tsx} : Private keys must be stored via biometric storage (`lib/biometric-storage.ts`) or session storage (`lib/secure-storage.ts`), never in plain text or local storage

Applied to files:

  • lib/services/account-deletion-service.ts
📚 Learning: 2026-01-12T21:40:25.650Z
Learnt from: CR
Repo: PastaPastaPasta/yappr PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-12T21:40:25.650Z
Learning: Applies to **/*.{ts,tsx} : SDK initialization and connection management must be handled through `lib/services/evo-sdk-service.ts`

Applied to files:

  • lib/services/account-deletion-service.ts
🧬 Code graph analysis (1)
lib/services/account-deletion-service.ts (6)
lib/constants.ts (6)
  • YAPPR_CONTRACT_ID (6-6)
  • YAPPR_PROFILE_CONTRACT_ID (7-7)
  • YAPPR_DM_CONTRACT_ID (8-8)
  • YAPPR_BLOCK_CONTRACT_ID (9-9)
  • ENCRYPTED_KEY_BACKUP_CONTRACT_ID (11-11)
  • HASHTAG_CONTRACT_ID (12-12)
lib/services/sdk-helpers.ts (2)
  • queryDocuments (200-228)
  • identifierToBase58 (27-96)
lib/secure-storage.ts (1)
  • clearPrivateKey (185-187)
lib/password-encrypted-storage.ts (1)
  • removeStoredCredential (165-177)
lib/caches/block-cache.ts (1)
  • invalidateBlockCache (102-107)
lib/dash-platform-client.ts (1)
  • getDashPlatformClient (494-499)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Cloudflare Pages
🔇 Additional comments (9)
lib/services/account-deletion-service.ts (9)

1-11: LGTM!

Imports correctly follow the coding guidelines: SDK access via evo-sdk-service, write operations via state-transition-service, and query operations via sdk-helpers.


35-49: LGTM!

The DeletionProgress interface provides comprehensive tracking including granular error details per document, which will be valuable for debugging failed deletions and user feedback.


82-135: LGTM!

The registry pattern provides good extensibility. The duplicate profile entry in both the main contract (line 95, marked as "old contract") and the unified profile contract is appropriate for migration scenarios.


184-238: LGTM!

The pagination logic is well-implemented. The startAfter: undefined on first iteration is correctly handled since queryDocuments only sets the parameter when it's defined. The ID normalization fallback chain (lines 211-212) appropriately handles various SDK response formats.


298-323: LGTM!

The batch processing with Promise.allSettled correctly handles mixed success/failure results. The error extraction logic properly distinguishes between rejected promises and fulfilled-but-unsuccessful deletions.


350-365: LGTM!

Correctly delegates to stateTransitionService for document deletion as per coding guidelines, with proper error handling.


400-425: LGTM!

The orchestration logic correctly handles edge cases. The condition on line 408 appropriately triggers cleanup when any documents were deleted or when there were no documents to delete (preventing orphaned local storage).


442-484: LGTM!

Helper methods are well-implemented. getAffectedDocumentTypes correctly deduplicates types, and getDeletionSummary provides clear categorization for user-facing warnings. The singleton export is appropriate for this stateless service.


370-395: The code correctly implements account deletion without violating security guidelines.

  1. Line 373 - yappr_session in localStorage: This is not a security violation. The codebase explicitly documents (line 158 in auth-context.tsx) that "private key is not saved, only used for login." The yappr_session stores only user profile data (identityId, dpnsUsername, balance) and timestamp. Private keys are properly stored via clearPrivateKey() at line 379-380, which uses secure-storage per the coding guidelines.

  2. Line 393 - setIdentity(''): Using an empty string is the correct pattern. This method is consistently used throughout the codebase (including in auth-context.tsx logout) to clear the identity state. There is no null variant or dedicated clear method; empty string is the intended behavior for clearing identity.

No changes needed.

Likely an incorrect or invalid review comment.

claude and others added 3 commits January 13, 2026 16:00
- Add registry-based AccountDeletionService that deletes all user documents
  across all Yappr contracts (main, profile, DM, block, hashtag, key backup)
- Create multi-step delete account modal with warning, confirmation (type
  DELETE), progress tracking, and completion/error states
- Add clear warnings about third-party indexers retaining historical copies
- Update privacy policy to reflect that deletion IS now possible from
  Dash Platform, with caveats about external services
- Clean up local storage after successful deletion

The service uses a future-proof registry pattern - new document types just
need to be added to the registry array to be included in deletion.
- Update handleCompleteLogout to properly call auth context logout() for full
  session teardown before navigating to login
- Replace direct SDK query with typed queryDocuments helper in account deletion
  service, using identifierToBase58 for proper identifier normalization
- Add cleanup of password-encrypted credentials during account deletion
- Replace direct sessionStorage use in test-create page with secure-storage API
  to avoid writing private keys in plain text

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use the proper abstraction layer for document queries instead of
calling sdk.documents.query directly. This aligns with the
architectural guidelines and provides consistent error handling.
@PastaPastaPasta
PastaPastaPasta force-pushed the claude/implement-delete-account-yvcQF branch from 50c018a to d4b6e7d Compare January 13, 2026 16:01
@thepastaclaw

thepastaclaw commented Jul 1, 2026

Copy link
Copy Markdown
Collaborator

🕓 Ready for review — 70 ahead in queue (commit d4b6e7d)
Queue position: 71/74 · 3 reviews active
ETA: start ~09:49 UTC · complete ~10:25 UTC (median 36m across 30 recent reviews; 3 slots)
Queued 40m ago · Last checked: 2026-07-21 19:20 UTC

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Preliminary review — Codex only

The account-deletion flow is not mergeable at the exact reviewed head. A missing module prevents compilation, while invalid queries and incomplete contract registries can leave documents behind yet report successful deletion. Partial-failure retry, completion cleanup, and the privacy policy also expose broken or misleading user workflows.

Validated blockers were found in the Codex precheck. Sonnet is deferred until a fresh Codex revalidation clears the blocker gate.

Review provenance

  • Codex reviewers: gpt-5.6-sol — general (failed), gpt-5.6-sol — general (completed)
  • Verifier: gpt-5.6-sol — verifier
  • Sonnet: not run (deferred by blocker gate)

🔴 7 blocking

🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `lib/services/account-deletion-service.ts`:
- [BLOCKING] lib/services/account-deletion-service.ts:383-384: Importing the removed credential-storage module breaks the build
  The referenced `lib/password-encrypted-storage.ts` module does not exist at this head. Commit b0150691 removed the unused subsystem before this PR, but the new client-side deletion service statically references its former API through a literal dynamic import. Next.js must resolve that module while bundling, so the settings route cannot compile.
- [BLOCKING] lib/services/account-deletion-service.ts:196-233: Failed enumeration is silently interpreted as having no documents
  Every registry entry is queried using `$ownerId` followed by `$createdAt`, but several target schemas do not declare that index shape. Unified and legacy profiles, avatars, block filters, block follows, and encrypted key backups have owner-only indexes; block uses `$ownerId` plus `blockedId`; and `listMember` has no owner index. Index-validation errors, transient DAPI failures, and later pagination failures are caught and converted into empty or truncated results. Those failures never increment `totalFailed`, so deletion can return `success: true`, clear credentials, and tell the user that all data was deleted. Use a registry-specific query shape and treat any enumeration failure as a failed deletion.
- [BLOCKING] lib/services/account-deletion-service.ts:109: DM invites and read receipts are omitted from deletion
  The direct-message service creates `conversationInvite`, `directMessage`, and `readReceipt` documents under `YAPPR_DM_CONTRACT_ID`, but the deletion registry includes only `directMessage`. User-owned invites—including optional sender public-key material—and read receipts therefore remain on Platform after the modal promises to remove all private data. Add both types with query shapes matching their respective indexes.
- [BLOCKING] lib/services/account-deletion-service.ts:114-120: Legacy moderation documents are registered under the wrong contract
  The enhanced block contract contains `block`, `blockFilter`, and `blockFollow`, but it has no `mute` document type. The main social contract contains the legacy `mute` and `block` types, yet neither is registered under `YAPPR_CONTRACT_ID`. The `mute` query against the block contract consequently fails, and legacy main-contract blocks and mutes are never deleted despite the promised block-list cleanup.
- [BLOCKING] lib/services/account-deletion-service.ts:408: Partial deletion clears the key required by the offered retry
  A partial result has both `totalDeleted > 0` and `totalFailed > 0`, so this condition clears the user's signing key before showing the error screen. The offered “Try Again” action then cannot delete the remaining documents because `stateTransitionService.deleteDocument` reports that no private key is available. Preserve credentials after partial failure; the existing “Continue Anyway” action can perform logout cleanup if the user chooses to abandon the retry.

In `components/settings/delete-account-modal.tsx`:
- [BLOCKING] components/settings/delete-account-modal.tsx:524-539: Completion can be dismissed without clearing auth context
  Successful deletion clears persisted credentials but does not set the auth context's `user` to null. The completion screen still allows both backdrop and close-button dismissal through `handleClose`, bypassing `handleCompleteLogout`. The protected settings route therefore remains satisfied by stale in-memory user state even though its signing credentials and Platform identity were cleared. Successful completion must only close through the auth-context logout path.

In `app/privacy/page.tsx`:
- [BLOCKING] app/privacy/page.tsx:147: Privacy policy incorrectly promises deletion of received messages
  Deletion queries only documents owned by the deleting identity. Direct-message documents are owned by their senders, so messages received by this user remain controlled by the sending identities and cannot be removed by this flow. The public privacy policy must distinguish sent, user-owned messages from received messages.

Comment on lines +383 to +384
const { removeStoredCredential } = await import('../password-encrypted-storage')
removeStoredCredential(userId)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔴 Blocking: Importing the removed credential-storage module breaks the build

The referenced lib/password-encrypted-storage.ts module does not exist at this head. Commit b015069 removed the unused subsystem before this PR, but the new client-side deletion service statically references its former API through a literal dynamic import. Next.js must resolve that module while bundling, so the settings route cannot compile.

source: ['codex']

Comment on lines +196 to +233
const documents = await queryDocuments(sdk, {
dataContractId: contractId,
documentTypeName: documentType,
where: [['$ownerId', '==', userId]],
orderBy: [['$createdAt', 'asc']],
limit: this.QUERY_LIMIT,
startAfter
})

if (documents.length === 0) {
break
}

// Add documents with metadata
for (const doc of documents) {
const docId = identifierToBase58(doc.$id) || (doc.$id as string) || (doc.id as string)
const ownerId = identifierToBase58(doc.$ownerId) || (doc.$ownerId as string) || (doc.ownerId as string) || userId

allDocuments.push({
$id: docId,
$ownerId: ownerId,
documentType,
contractId
})
}

// Check if we need to paginate
if (documents.length < this.QUERY_LIMIT) {
break
}

// Use last document ID for pagination
const lastDoc = documents[documents.length - 1]
startAfter = identifierToBase58(lastDoc.$id) || (lastDoc.$id as string) || (lastDoc.id as string)
} catch (error) {
// Some document types may fail to query (e.g., not registered)
console.warn(`Query failed for ${documentType} in contract ${contractId}:`, error)
break

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔴 Blocking: Failed enumeration is silently interpreted as having no documents

Every registry entry is queried using $ownerId followed by $createdAt, but several target schemas do not declare that index shape. Unified and legacy profiles, avatars, block filters, block follows, and encrypted key backups have owner-only indexes; block uses $ownerId plus blockedId; and listMember has no owner index. Index-validation errors, transient DAPI failures, and later pagination failures are caught and converted into empty or truncated results. Those failures never increment totalFailed, so deletion can return success: true, clear credentials, and tell the user that all data was deleted. Use a registry-specific query shape and treat any enumeration failure as a failed deletion.

source: ['codex']

// Direct messages contract
{
contractId: YAPPR_DM_CONTRACT_ID,
documentTypes: ['directMessage'],

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔴 Blocking: DM invites and read receipts are omitted from deletion

The direct-message service creates conversationInvite, directMessage, and readReceipt documents under YAPPR_DM_CONTRACT_ID, but the deletion registry includes only directMessage. User-owned invites—including optional sender public-key material—and read receipts therefore remain on Platform after the modal promises to remove all private data. Add both types with query shapes matching their respective indexes.

source: ['codex']

Comment on lines +114 to +120
contractId: YAPPR_BLOCK_CONTRACT_ID,
documentTypes: [
'block', // Blocked users
'blockFilter', // Bloom filter for blocks
'blockFollow', // Block follows
'mute', // Muted users
],

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔴 Blocking: Legacy moderation documents are registered under the wrong contract

The enhanced block contract contains block, blockFilter, and blockFollow, but it has no mute document type. The main social contract contains the legacy mute and block types, yet neither is registered under YAPPR_CONTRACT_ID. The mute query against the block contract consequently fails, and legacy main-contract blocks and mutes are never deleted despite the promised block-list cleanup.

source: ['codex']

const result = await this.deleteAllUserDocuments(userId, onProgress)

// If deletion was successful or partially successful, clear local storage
if (result.totalDeleted > 0 || result.totalFailed === 0) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔴 Blocking: Partial deletion clears the key required by the offered retry

A partial result has both totalDeleted > 0 and totalFailed > 0, so this condition clears the user's signing key before showing the error screen. The offered “Try Again” action then cannot delete the remaining documents because stateTransitionService.deleteDocument reports that no private key is available. Preserve credentials after partial failure; the existing “Continue Anyway” action can perform logout cleanup if the user chooses to abandon the retry.

Suggested change
if (result.totalDeleted > 0 || result.totalFailed === 0) {
if (result.success) {

source: ['codex']

Comment on lines +524 to +539
onClick={isDeleting ? undefined : handleClose}
/>

{/* Modal */}
<motion.div
initial={{ opacity: 0, scale: 0.95 }}
animate={{ opacity: 1, scale: 1 }}
exit={{ opacity: 0, scale: 0.95 }}
transition={{ duration: 0.2 }}
className="fixed inset-0 flex items-center justify-center z-50 px-4 overflow-y-auto py-8"
>
<div className="bg-white dark:bg-gray-900 rounded-2xl shadow-xl p-6 max-w-md w-full relative my-auto">
{/* Close button - hidden during deletion */}
{!isDeleting && step !== 'progress' && (
<button
onClick={handleClose}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔴 Blocking: Completion can be dismissed without clearing auth context

Successful deletion clears persisted credentials but does not set the auth context's user to null. The completion screen still allows both backdrop and close-button dismissal through handleClose, bypassing handleCompleteLogout. The protected settings route therefore remains satisfied by stale in-memory user state even though its signing credentials and Platform identity were cleared. Successful completion must only close through the auth-context logout path.

source: ['codex']

Comment thread app/privacy/page.tsx
<li>All your posts, likes, reposts, and replies are permanently removed</li>
<li>Your profile and avatar are deleted</li>
<li>Your follow lists, bookmarks, and blocks are removed</li>
<li>Your direct messages are deleted</li>

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🔴 Blocking: Privacy policy incorrectly promises deletion of received messages

Deletion queries only documents owned by the deleting identity. Direct-message documents are owned by their senders, so messages received by this user remain controlled by the sending identities and cannot be removed by this flow. The public privacy policy must distinguish sent, user-owned messages from received messages.

Suggested change
<li>Your direct messages are deleted</li>
<li>Your sent direct messages are deleted; messages sent to you remain controlled by their senders</li>

source: ['codex']

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.

3 participants