fix(cleanup): address PR #10 CodeRabbit review findings

components/AccountForm.tsx

@@ -38,8 +38,11 @@ const AccountForm: React.FC<Props> = ({
 
   const sanitizeNote = (value: string) =>
     value
-      .replace(/\b(?=(?:[\s\S]*\d){13,19})(?:\d[ -]?){13,19}\b/g, "[redacted]")
-      .replace(/\b(cvv|cvc)\s*:?\s*\d{3,4}\b/gi, "[redacted]");
+      .replace(/\b(cvv|cvc)\s*:?\s*\d{3,4}\b/gi, "[redacted]")
+      .replace(/\d[ -]?(?:\d[ -]?){11,17}\d/g, (run) => {
+        const digits = run.replace(/\D/g, "");
+        return digits.length >= 13 && digits.length <= 19 ? "[redacted]" : run;
+      });
 
   const [confirmationModal, setConfirmationModal] = useState<{
     isOpen: boolean;

components/GoogleDrivePicker.tsx

@@ -5,6 +5,10 @@ import {
 } from "@googleworkspace/drive-picker-react";
 import { logger } from "../src/lib/application/logger";
 
+const maskFileId = (fileId: string): string => {
+  return fileId.length > 4 ? `***${fileId.slice(-4)}` : "***";
+};
+
 interface GoogleDrivePickerProps {
   onPicked: (fileId: string) => void;
   onCancel?: () => void;
@@ -30,21 +34,6 @@ export const GoogleDrivePicker: React.FC<GoogleDrivePickerProps> = ({
   const apiKey = import.meta.env.VITE_GOOGLE_API_KEY;
   const appId = import.meta.env.VITE_GOOGLE_APP_ID;
 
-  if (!clientId || !apiKey) {
-    logger.error(
-      "GoogleDrivePicker: Missing VITE_GOOGLE_CLIENT_ID or VITE_GOOGLE_API_KEY",
-    );
-  }
-  if (!appId) {
-    logger.warn(
-      "GoogleDrivePicker: Missing VITE_GOOGLE_APP_ID - this may cause integration issues",
-    );
-  }
-
-  const maskFileId = (fileId: string): string => {
-    return fileId.length > 4 ? `***${fileId.slice(-4)}` : "***";
-  };
-
   const handlePicked = (e: CustomEvent) => {
     const data = e.detail;
     if (data.docs && data.docs.length > 0) {
@@ -124,10 +113,6 @@ export const useGoogleDrivePicker = () => {
     });
   };
 
-  const maskFileId = (fileId: string): string => {
-    return fileId.length > 4 ? `***${fileId.slice(-4)}` : "***";
-  };
-
   const handlePicked = (e: CustomEvent) => {
     const data = e.detail;
     if (data.docs && data.docs.length > 0) {

docs/PLAN.md

@@ -4,7 +4,7 @@
 
 Replace the 2,879-line `DataProvider.tsx` monolith with focused **Zustand stores** (state) and **application commands** (logic). Data flows in one direction:
 
-```
+```text
 Sheets ──→ DataProvider ──→ Stores ──→ Components
                 │               │
                 ↓               ↓
@@ -33,7 +33,7 @@ Sheets ──→ DataProvider ──→ Stores ──→ Components
 
 ## Architecture
 
-```
+```text
 src/
 ├── lib/
 │   ├── domain/            ← Pure functions (balance.engine, currency)
@@ -50,7 +50,7 @@ src/
 
 ### Data flow after full migration
 
-```
+```text
 User action (click "Save")
   → Command (submitTransaction in commands.ts)
     → Domain logic (computeAccountTransactionAmount / computeBudgetConsumption / computeSavingsMovement)
@@ -79,15 +79,15 @@ Each domain follows the same pattern:
 | 6 | Pockets | Medium | — | savePocket, deletePocket | AssetsPage |
 | 7 | Accounts | Medium | — | saveAccount, deleteAccount | AccountCard, AccountForm, AccountPage |
 | 8 | Transactions | Very High | submit, delete, batchDelete | batchEdit (309 lines), bulkImport (120 lines) | History, Dashboard, Charts |
-| 9 | Privacy | Medium | — | vault commands (6 handlers) | Auth, Profile |
+| 9 | Mask Mode | Medium | — | mask-mode toggles (replaces vault handlers) | Auth, Profile |
 | 10 | Sync | Very High | — | syncData (464 lines) | DataProvider itself |
 | 11 | Cleanup | — | — | — | Remove DataProvider, DataContext, dead code |
 
 ## Commit Plan
 
 ### Phase 1: Foundation (1 commit)
 
-```
+```text
 Commit 1: DataProvider pipes loaded data into Zustand stores
   - After loadData completes, call:
     useFinanceStore.getState().setAccounts(data.accounts)
@@ -99,7 +99,7 @@ Commit 1: DataProvider pipes loaded data into Zustand stores
 
 ### Phase 2: Simple Domains (8 small commits)
 
-```
+```text
 Commit 2: Extract Category commands + store wiring
   - Create saveCategory, deleteCategory in commands.ts
   - Switch CategoryManager from useData() to useFinanceStore()
@@ -131,7 +131,7 @@ Commit 9: Verify all simple domains migrated
 
 ### Phase 3: Transactions Domain (3-4 commits, split into sub-tasks)
 
-```
+```text
 Commit 10: Extract batchEditTransaction to command
   - 309-line handler in DataProvider → commands.ts
   - Split into smaller sub-tasks
@@ -144,10 +144,10 @@ Commit 12: Switch History page components to useFinanceStore()
 Commit 13: Switch Dashboard + Charts to useFinanceStore()
 ```
 
-### Phase 4: Privacy + Sync (2-3 commits)
+### Phase 4: Mask Mode + Sync (2-3 commits)
 
-```
-Commit 14: Create privacy commands (vault enable/disable/lock/unlock)
+```text
+Commit 14: Create mask-mode commands (toggle setMaskMode, mask helpers)
   - Switch Auth and Profile components
 
 Commit 15: Extract syncData to sync command
@@ -159,7 +159,7 @@ Commit 16: Final sync — DataProvider becomes thin
 
 ### Phase 5: Cleanup (1-2 commits)
 
-```
+```text
 Commit 17: Remove DataContext
   - All components now use stores directly
   - Delete context/DataContext.tsx

docs/adrs/001-layered-architecture-refactor.md

@@ -1,6 +1,5 @@
 # ADR-001: Layered Architecture Refactor
 
-**Status:** Accepted (in progress)  
 **Date:** 2026-06-01  
 **Deciders:** Edwin  
 
@@ -266,3 +265,7 @@ Pages become thin — they call commands (or use Zustand hooks directly for read
 - `src/lib/domain/balance.engine.ts` — extracted balance computation
 - `src/lib/domain/currency.ts` — extracted currency conversion
 - `src/lib/domain/__tests__/` — 22 tests for domain functions
+
+---
+
+**Status:** Accepted (in progress)

docs/adrs/002-drop-vault-data-minimization.md

@@ -1,6 +1,5 @@
 # Drop the vault: data minimization for a personal finance tracker
 
-**Status:** Accepted
 **Date:** 2026-06-04
 **Deciders:** Edwin
 
@@ -49,3 +48,7 @@ The only sensitive data the app will hold after this change is whatever the user
 - `pages/ProfilePage.tsx` — vault UI deleted; export `_note` warning removed
 - `layouts/MainLayout.tsx` — vault unlock modal deleted
 - `helpers/useAppInit.tsx` — vault-specific init paths deleted
+
+---
+
+**Status:** Accepted

layouts/MainLayout.tsx

@@ -244,6 +244,7 @@ const MainLayout: React.FC = () => {
           )}
 
           <button
+            type="button"
             onClick={() => setMaskMode(!maskMode)}
             aria-pressed={maskMode}
             className="w-full flex items-center justify-between p-3 rounded-xl bg-gray-900/50 hover:bg-gray-800/50 border border-gray-800/50 transition-all group"
@@ -302,6 +303,7 @@ const MainLayout: React.FC = () => {
               </div>
             )}
             <button
+              type="button"
               onClick={() => setMaskMode(!maskMode)}
               aria-label="Toggle mask mode"
               aria-pressed={maskMode}

opencode.json

@@ -44,7 +44,7 @@
         "edit": "allow",
         "glob": "allow",
         "grep": "allow",
-        "bash": "allow"
+        "bash": "deny"
       }
     },
     "commands-splitter": {
@@ -63,7 +63,7 @@
         "edit": "allow",
         "glob": "allow",
         "grep": "allow",
-        "bash": "allow"
+        "bash": "deny"
       }
     },
     "page-migrator": {
@@ -82,7 +82,7 @@
         "edit": "allow",
         "glob": "allow",
         "grep": "allow",
-        "bash": "allow"
+        "bash": "deny"
       }
     }
   }

src/lib/domain/__tests__/crypto.test.ts → services/__tests__/crypto.test.ts

@@ -4,7 +4,7 @@ import {
 	verifyPassword,
 	isLegacyHash,
 	PBKDF2_ITERATIONS,
-} from "../../../../services/crypto.services";
+} from "../crypto.services";
 
 describe("crypto.services", () => {
 	describe("hashPassword", () => {

src/lib/application/commands/__tests__/sync.test.ts

@@ -2,7 +2,7 @@ import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
 import * as StorageService from "../../../../../services/storage.services";
 import * as SheetService from "../../../../../services/sheets.services";
 import { useSyncStore } from "../../../../stores/sync.store";
-import { syncData } from "../sync";
+import { syncData, resetAndSync } from "../sync";
 
 vi.mock("../../../../../services/storage.services");
 vi.mock("../../../../../services/sheets.services");
@@ -94,6 +94,7 @@ describe("resetAndSync 401 handling", () => {
 
   afterEach(() => {
     vi.useRealTimers();
+    vi.unstubAllGlobals();
   });
 
   it("cleans up isSyncing and preserves keep-list on 401 from loadFromGoogleSheets", async () => {
@@ -114,7 +115,6 @@ describe("resetAndSync 401 handling", () => {
     const updateProfile = vi.fn();
     const loginWithGoogle = vi.fn();
 
-    const { resetAndSync } = await import("../sync");
     await resetAndSync({ ...baseProfile } as any, updateProfile, loginWithGoogle);
 
     const state = useSyncStore.getState();

src/lib/application/commands/accounts.ts

@@ -24,10 +24,9 @@ export async function saveAccount(
     ? [...existingAccounts, accountWithUser]
     : existingAccounts.map((a) => (a.id === acc.id ? accountWithUser : a));
 
-  await StorageService.saveAccounts(updated);
-
+  const newTxs: Transaction[] = [];
   if (isNew && accountWithUser.balance !== 0) {
-    const openingTx: Transaction = {
+    newTxs.push({
       id: crypto.randomUUID(),
       userId: accountWithUser.userId,
       accountId: accountWithUser.id,
@@ -38,19 +37,14 @@ export async function saveAccount(
       date: new Date().toLocaleDateString("en-CA"),
       createdAt: new Date().toISOString(),
       updatedAt: new Date().toISOString(),
-    };
-    const updatedTxs = [openingTx, ...existingTransactions];
-    await StorageService.saveTransactions(updatedTxs);
-    store.setTransactions(updatedTxs);
+    });
   }
 
-  store.setAccounts(updated);
-
   if (!isNew) {
     const oldAcc = existingAccounts.find((a) => a.id === acc.id);
     if (oldAcc && oldAcc.balance !== accountWithUser.balance) {
       const diff = accountWithUser.balance - oldAcc.balance;
-      const adjustmentTx: Transaction = {
+      newTxs.push({
         id: crypto.randomUUID(),
         userId: accountWithUser.userId,
         accountId: accountWithUser.id,
@@ -62,11 +56,24 @@ export async function saveAccount(
         createdAt: new Date().toISOString(),
         updatedAt: new Date().toISOString(),
         note: `Manually changed balance from ${oldAcc.balance} to ${accountWithUser.balance}`,
-      };
-      const updatedTxs = [adjustmentTx, ...existingTransactions];
+      });
+    }
+  }
+
+  if (newTxs.length > 0) {
+    const updatedTxs = [...newTxs, ...existingTransactions];
+    await StorageService.saveAccounts(updated);
+    try {
       await StorageService.saveTransactions(updatedTxs);
       store.setTransactions(updatedTxs);
+      store.setAccounts(updated);
+    } catch (error) {
+      await StorageService.saveAccounts(existingAccounts);
+      throw error;
     }
+  } else {
+    await StorageService.saveAccounts(updated);
+    store.setAccounts(updated);
   }
 
   showToast("Account saved", "success");

src/lib/application/commands/migration.ts

@@ -1,6 +1,3 @@
-// TEMPORARY: One-time v1 -> v2 schema cleanup orchestrator. See CONTEXT.md and
-// docs/adrs/002-drop-vault-data-minimization.md for removal criterion.
-
 import * as StorageService from "../../../../services/storage.services";
 import * as SheetService from "../../../../services/sheets.services";
 import { useFinanceStore } from "../../../stores/finance.store";

src/lib/application/commands/transactions.ts

@@ -96,7 +96,6 @@ export async function submitTransaction(
       : null;
   if (partnerLeg) applyDeltas(partnerLeg, 1);
 
-  // Build updated transaction list
   let updatedTransactions: Transaction[];
   if (isEdit) {
     updatedTransactions = store.transactions.map((t: Transaction) =>
@@ -143,19 +142,16 @@ export async function submitTransaction(
     return p;
   });
 
-  // Update store
   store.setTransactions(updatedTransactions);
   if (accountUpdates.size > 0) store.setAccounts(updatedAccounts);
   if (potUpdates.size > 0) store.setPots(updatedPots);
   if (pocketUpdates.size > 0) store.setPockets(updatedPockets);
 
-  // Persist to local storage
   StorageService.saveTransactions(updatedTransactions);
   if (accountUpdates.size > 0) StorageService.saveAccounts(updatedAccounts);
   if (potUpdates.size > 0) StorageService.savePots(updatedPots);
   if (pocketUpdates.size > 0) StorageService.savePockets(updatedPockets);
 
-  // Cloud sync
   if (isCloudEnabled) {
     if (isEdit) {
       await SheetService.updateOne("Transactions", txWithUser.id, txWithUser);
@@ -175,7 +171,6 @@ export async function submitTransaction(
     }
   }
 
-  // Handle subscriptions
   if (newSubscription && !isEdit) {
     const sub: Subscription = {
       ...newSubscription,
@@ -196,7 +191,6 @@ export async function submitTransaction(
     if (isCloudEnabled) await SheetService.insertOne("Subscriptions", sub);
   }
 
-  // Handle link to existing subscription (advance next payment date)
   if (txWithUser.subscriptionId && !newSubscription && subscriptions) {
     const sub = subscriptions.find((s: Subscription) => s.id === txWithUser.subscriptionId);
     if (sub) {
@@ -539,7 +533,6 @@ export async function batchEditTransactions(
     finalUpdatesMap.set(id, thisUpdates);
     affectedTransactionIds.add(id);
 
-    // Handle partner leg
     if (
       originalTx.linkedTransactionId &&
       (nullifiedFields.includes("potId") ||
@@ -553,7 +546,6 @@ export async function batchEditTransactions(
       const partner = transactions.find((t) => t.id === originalTx.linkedTransactionId);
       if (partner) {
         const partnerUpdates: any = { ...cleanUpdates };
-        // Swap account/ pocket linkage fields for the partner side
         if (cleanUpdates.accountId !== undefined) {
           partnerUpdates.toAccountId = cleanUpdates.accountId;
         }
@@ -566,8 +558,7 @@ export async function batchEditTransactions(
         if (cleanUpdates.toSavingPocketId !== undefined) {
           partnerUpdates.savingPocketId = cleanUpdates.toSavingPocketId;
         }
-        // Sync shared transfer fields to keep both legs consistent
-        const sharedFields = ["amount", "currency", "date", "fee", "feeType", "notes", "shopName"] as const;
+        const sharedFields = ["amount", "currency", "date", "fee", "feeType", "note", "shopName"] as const;
         for (const field of sharedFields) {
           if (cleanUpdates[field as keyof typeof cleanUpdates] !== undefined) {
             partnerUpdates[field] = cleanUpdates[field as keyof typeof cleanUpdates];
@@ -597,7 +588,6 @@ export async function batchEditTransactions(
     }
   }
 
-  // Recalculate balance/pot/pocket impacts
   const potUpdates = new Map<string, number>();
   const pocketUpdates = new Map<string, number>();
   const accountUpdates = new Map<string, number>();