Stash: prevent data-loss clobber on save + confirm before deleting gear

Audited the Stash — a private, NIP-44-encrypted, Amber-sensitive surface.

DATA-SAFETY FIX (the important one):
The stash is a single replaceable event whose encrypted content is the WHOLE
item list, so every add/update/remove rewrites the entire event. The mutations
built the new list from the `stash` closure value — but that's legitimately
`undefined` before the decrypt completes, and with Amber the read query is
gated / staleTime:Infinity, so it may never have been decrypted at mutation
time. Result: adding one item while `stash` was empty/undefined could publish a
list containing only that item and silently WIPE all existing gear.

Fix: mutations now call `fetchLatestStash()` — a fresh relay fetch + decrypt of
the current stash immediately before saving — and build the new list on top of
that. It falls back to React Query cache (never an empty list) on failure, and
throws rather than risk a clobber if it truly can't load. saveStash also writes
the just-published value straight into the cache to keep reads consistent.

The extra decrypt per mutation is an acceptable Amber tradeoff: a mutation is
already a deliberate write (encrypt + sign prompt), and data integrity beats
prompt count.

UX:
• Delete now asks for confirmation via an AlertDialog (item name + a note that
  public items get unlisted). Previously a single tap wiped an item outright —
  destructive and, with Amber, a surprise signer prompt.

The rest of StashPage (login gate, encryption-required state, distinct
wishlist/stash/filtered empty states, skeletons, rarity cards, grid/list) is
already excellent — left as-is.

Bumps to 1.2.23 (Android versionCode 10223 auto-derived).

Co-authored-by: shakespeare.diy <assistant@shakespeare.diy>

package.json

@@ -1,7 +1,7 @@
 {
   "name": "skate-spots",
   "private": true,
-  "version": "1.2.22",
+  "version": "1.2.23",
   "type": "module",
   "homepage": "https://skate-spots.com",
   "scripts": {

src/hooks/useStash.ts

@@ -113,6 +113,62 @@ export function useStash(options?: { enabled?: boolean }) {
     refetchOnWindowFocus: false,
   });
 
+  /**
+   * Re-fetch + decrypt the freshest stash from relays RIGHT BEFORE a mutation,
+   * rather than trusting the `stash` closure value.
+   *
+   * Why this matters (data-safety): the stash is one replaceable event whose
+   * content is the WHOLE list. Every add/update/remove rewrites that entire
+   * event. If we based the new list on a stale or not-yet-decrypted `stash`
+   * (which is legitimately `undefined` early, and — with Amber — may never have
+   * been decrypted because the query is gated / `staleTime: Infinity`), we'd
+   * publish a list missing everything the closure didn't know about and
+   * silently wipe the user's gear. So we always rebuild from a fresh decrypt.
+   *
+   * Falls back to the cached query data, then to an empty stash, only if the
+   * live fetch genuinely returns nothing.
+   */
+  const fetchLatestStash = useCallback(async (): Promise<Stash> => {
+    if (!user?.pubkey || !hasNip44) return { items: [], updatedAt: 0 };
+
+    let nip44Dec: typeof user.signer.nip44;
+    try { nip44Dec = user.signer?.nip44; } catch { nip44Dec = undefined; }
+    if (!nip44Dec) {
+      // Can't decrypt now — use whatever we already have cached to avoid clobber.
+      return (
+        queryClient.getQueryData<Stash>(['stash', user.pubkey]) ??
+        stash ?? { items: [], updatedAt: 0 }
+      );
+    }
+
+    try {
+      const events = await nostr.query([{
+        kinds: [STASH_KIND],
+        authors: [user.pubkey],
+        '#d': [STASH_D_TAG],
+        limit: 1,
+      }], { signal: AbortSignal.timeout(10000) });
+
+      if (events.length === 0) {
+        // No remote event: safe to start fresh only if we also have nothing cached.
+        return (
+          queryClient.getQueryData<Stash>(['stash', user.pubkey]) ??
+          stash ?? { items: [], updatedAt: 0 }
+        );
+      }
+
+      const decrypted = await nip44Dec.decrypt(user.pubkey, events[0].content);
+      return JSON.parse(decrypted) as Stash;
+    } catch (err) {
+      // On any failure, prefer cached data over an empty list so we never wipe.
+      console.error('[Stash] Failed to re-fetch latest stash before save:', err);
+      const cached = queryClient.getQueryData<Stash>(['stash', user.pubkey]) ?? stash;
+      if (cached) return cached;
+      // Nothing to fall back on — surface the error instead of risking a clobber.
+      throw err instanceof Error ? err : new Error('Could not load your stash to update it safely.');
+    }
+  }, [user, hasNip44, nostr, queryClient, stash]);
+
   // Save stash (encrypt and publish)
   const saveStash = useCallback(async (newStash: Stash) => {
     if (!user?.pubkey || !hasNip44) {
@@ -137,14 +193,15 @@ export function useStash(options?: { enabled?: boolean }) {
       ],
     });
 
-    // Invalidate cache
+    // Keep the cache in sync with exactly what we just published, then invalidate.
+    queryClient.setQueryData(['stash', user.pubkey], newStash);
     queryClient.invalidateQueries({ queryKey: ['stash', user.pubkey] });
-  }, [user, createEvent, queryClient]);
+  }, [user, hasNip44, createEvent, queryClient]);
 
   // Add item mutation
   const addItem = useMutation({
     mutationFn: async (item: Omit<StashItem, 'id' | 'addedAt'>) => {
-      const currentStash = stash || { items: [], updatedAt: 0 };
+      const currentStash = await fetchLatestStash();
       const newItem: StashItem = {
         ...item,
         id: crypto.randomUUID(),
@@ -167,7 +224,7 @@ export function useStash(options?: { enabled?: boolean }) {
   // Update item mutation
   const updateItem = useMutation({
     mutationFn: async (item: StashItem) => {
-      const currentStash = stash || { items: [], updatedAt: 0 };
+      const currentStash = await fetchLatestStash();
       const newStash: Stash = {
         items: currentStash.items.map(i => i.id === item.id ? item : i),
         updatedAt: Math.floor(Date.now() / 1000),
@@ -184,7 +241,7 @@ export function useStash(options?: { enabled?: boolean }) {
   // Remove item mutation
   const removeItem = useMutation({
     mutationFn: async (itemId: string) => {
-      const currentStash = stash || { items: [], updatedAt: 0 };
+      const currentStash = await fetchLatestStash();
       const newStash: Stash = {
         items: currentStash.items.filter(i => i.id !== itemId),
         updatedAt: Math.floor(Date.now() / 1000),

src/pages/StashPage.tsx

@@ -11,6 +11,16 @@ import { Textarea } from '@/components/ui/textarea';
 import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@/components/ui/select';
 import { Switch } from '@/components/ui/switch';
 import { Dialog, DialogContent, DialogHeader, DialogTitle, DialogTrigger, DialogFooter, DialogClose, DialogDescription } from '@/components/ui/dialog';
+import {
+  AlertDialog,
+  AlertDialogAction,
+  AlertDialogCancel,
+  AlertDialogContent,
+  AlertDialogDescription,
+  AlertDialogFooter,
+  AlertDialogHeader,
+  AlertDialogTitle,
+} from '@/components/ui/alert-dialog';
 import { Skeleton } from '@/components/ui/skeleton';
 import { Tooltip, TooltipContent, TooltipTrigger } from '@/components/ui/tooltip';
 import { LoginArea } from '@/components/auth/LoginArea';
@@ -638,8 +648,21 @@ export default function StashPage() {
     });
   };
 
-  // Remove: delete from stash, and clean up any public event.
+  // Remove: confirm first (deleting gear is destructive and, with Amber, costs a
+  // signer prompt), then delete from the encrypted stash and clean up any public
+  // event. `handleRemoveItem` just opens the confirm; `confirmRemoveItem` commits.
+  const [pendingDeleteId, setPendingDeleteId] = useState<string | null>(null);
+  const pendingDeleteItem = pendingDeleteId
+    ? items.find((i) => i.id === pendingDeleteId) ?? null
+    : null;
+
   const handleRemoveItem = (itemId: string) => {
+    setPendingDeleteId(itemId);
+  };
+
+  const confirmRemoveItem = () => {
+    if (!pendingDeleteId) return;
+    const itemId = pendingDeleteId;
     const target = items.find((i) => i.id === itemId);
     removeItem.mutate(itemId, {
       onSuccess: () => {
@@ -648,6 +671,7 @@ export default function StashPage() {
         }
       },
     });
+    setPendingDeleteId(null);
   };
   const [searchQuery, setSearchQuery] = useState('');
   const [filterCategory, setFilterCategory] = useState<string>('all');
@@ -1126,6 +1150,30 @@ export default function StashPage() {
       {listingItem && (
         <ListAtSpotDialog item={listingItem} open={!!listingItem} onOpenChange={(open) => !open && setListingItem(null)} onSelectSpot={handleSpotSelection} />
       )}
+
+      {/* Delete confirmation — deleting gear is destructive (and re-encrypts +
+          signs, so it costs an Amber prompt); confirm before committing. */}
+      <AlertDialog open={!!pendingDeleteId} onOpenChange={(open) => !open && setPendingDeleteId(null)}>
+        <AlertDialogContent>
+          <AlertDialogHeader>
+            <AlertDialogTitle>Remove from your stash?</AlertDialogTitle>
+            <AlertDialogDescription>
+              {pendingDeleteItem
+                ? <>“{pendingDeleteItem.title}” will be removed from your stash{pendingDeleteItem.visibility === 'public' ? ' and unlisted from your public showcase' : ''}. This can’t be undone.</>
+                : 'This item will be removed from your stash. This can’t be undone.'}
+            </AlertDialogDescription>
+          </AlertDialogHeader>
+          <AlertDialogFooter>
+            <AlertDialogCancel>Cancel</AlertDialogCancel>
+            <AlertDialogAction
+              onClick={confirmRemoveItem}
+              className="bg-red-600 hover:bg-red-700 text-white"
+            >
+              Remove
+            </AlertDialogAction>
+          </AlertDialogFooter>
+        </AlertDialogContent>
+      </AlertDialog>
     </Layout>
   );
 }