release: promote release/0.5.x to main for v0.5.7

CHANGELOG.md

@@ -5,6 +5,27 @@ All notable changes to the Fovea project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [0.5.7] - 2026-06-30
+
+The 0.5.7 patch is the second of three audit-driven hardening releases — the frontend slice — fixing data-loss and stale-cache defects in the annotation UI ([#190](https://github.com/parafovea/fovea/pull/190)). Nothing is breaking.
+
+### Fixed
+
+#### Auto-Save No Longer Drops Edits
+
+- A comment-only edit in the video summary editor was never auto-saved: the auto-save change-detection keyed on the summary body alone, so editing only the comment never armed the debounce and the edit was lost on dialog dismiss or navigation. (The 0.5.6 line's predecessor fix folded the comment into the comparison snapshot but did not make the debounce *fire* on it — a passing test masked the gap.) `useAutoSave` now keys change detection on the serialized comparison snapshot's VALUE, so an edit to any compared field — including a sibling field such as the comment — schedules a save (`annotation-tool/src/hooks/data/useAutoSave.ts`, `VideoSummaryEditor.tsx`).
+- Dismissing the summary dialog with Escape or a click outside bypassed the save-on-close flow, dropping edits made in the last second. The dialog now routes Escape/backdrop dismiss through the same `forceSave` flush as the Done button (`annotation-tool/src/components/video/VideoSummaryDialog.tsx`).
+- After a transient save failure, the in-progress guard was released immediately even though a retry was scheduled, allowing a second save to run concurrently with the retry (duplicate writes / last-writer-wins). The guard is now held through the backoff and released only on a terminal outcome (`annotation-tool/src/hooks/data/useAutoSave.ts`).
+- The session-expiry emergency save was a no-op stub that logged but saved nothing. It now flushes every mounted editor's pending edits through a registry of their `forceSave` callbacks (`annotation-tool/src/hooks/auth/useEmergencySave.ts`, `annotation-tool/src/hooks/data/autoSaveRegistry.ts`).
+
+#### Ontology Type Deletion Persists Again
+
+- Deleting an entity, role, event, or relation type from a persona stopped taking effect after 0.5.6: the client deleted by PUTting the ontology with the type omitted, but 0.5.6 changed the ontology write to merge by id, so the omitted type was kept and re-appeared on the next refetch. These deletions now call the dedicated DELETE endpoint (which also cleans up gloss references, world assignments, and annotations) (`annotation-tool/src/store/queries/usePersonas.ts`).
+
+#### Stale Caches Refreshed After Mutations
+
+- Creating and sharing a persona did not refresh the Sent Shares panel; a self-role change in a project or group left the list's own-role field stale; the summary save/generate/delete mutations skipped the batch summaries-lookup cache; and deleting an ontology type (or a persona) left annotation lists and world panels showing entries that no longer exist server-side. Each of these mutations now invalidates the additional query keys it affects (`annotation-tool/src/store/queries/{usePersonas,useProjects,useGroups,useSummaries}.ts`).
+
 ## [0.5.6] - 2026-06-30
 
 The 0.5.6 patch is the first of three audit-driven hardening releases — the backend slice — fixing a batch of authorization, data-integrity, idempotency, and concurrency defects surfaced by a code audit ([#188](https://github.com/parafovea/fovea/pull/188)). Nothing is breaking; the API additively gains `409` conflict responses on duplicate creates and the resource-fork now produces correctly-scoped resources.

annotation-tool/package.json

@@ -1,7 +1,7 @@
 {
   "name": "@fovea/annotation-tool",
   "private": true,
-  "version": "0.5.6",
+  "version": "0.5.7",
   "type": "module",
   "scripts": {
     "dev": "vite",

annotation-tool/src/components/video/VideoSummaryDialog.tsx

@@ -77,7 +77,7 @@ export default function VideoSummaryDialog({
   }, [open, initialPersonaId, videoId, timestampCapture])
 
   return (
-    <Dialog open={open && !timestampCapture} onOpenChange={(isOpen) => { if (!isOpen && !timestampCapture) onClose() }}>
+    <Dialog open={open && !timestampCapture} onOpenChange={(isOpen) => { if (!isOpen && !timestampCapture) void handleDone() }}>
       <DialogContent className="sm:max-w-2xl min-h-[60vh] max-h-[80vh]">
         <DialogHeader>
           <DialogTitle>Edit Video Summary</DialogTitle>

annotation-tool/src/hooks/auth/useEmergencySave.ts

@@ -12,6 +12,7 @@ import { useQueryClient } from '@tanstack/react-query'
 
 import { logWarning, logCritical } from '@services/errorLogging'
 import { withSpan } from '@telemetry/tracing'
+import { flushAllAutoSaves } from '../data/autoSaveRegistry'
 
 /**
  * Result of an emergency save operation.
@@ -60,14 +61,19 @@ export function useEmergencySave(): EmergencySaveState {
       const results: EmergencySaveResult = { saved: 0, errors: [] }
 
       try {
-        // Get all mutation cache entries that might have pending data
-        // For now, just log that we attempted emergency save
-        // In a full implementation, we'd iterate through pending mutations
         logWarning('Emergency save triggered', {
           component: 'useEmergencySave',
           mutationCacheSize: queryClient.getMutationCache().getAll().length,
         })
 
+        // Flush every mounted editor's pending edits by invoking the auto-save
+        // forceSave callbacks they registered. Best-effort: a save may still
+        // fail (e.g. the session is already gone on a hard 401), which is
+        // recorded in errors rather than swallowed.
+        const flushed = await flushAllAutoSaves()
+        results.saved = flushed.saved
+        results.errors.push(...flushed.errors.map((e) => e.message))
+
         span.setAttribute('save_attempted', true)
       } catch (error) {
         results.errors.push((error as Error).message)

annotation-tool/src/hooks/data/autoSaveRegistry.ts

@@ -0,0 +1,44 @@
+/**
+ * Process-wide registry of active auto-save flush callbacks.
+ *
+ * Each mounted `useAutoSave` registers its `forceSave` here so a global handler
+ * (the session-expiry emergency save) can flush every editor's pending edits at
+ * once, without each editor having to subscribe to the global event itself.
+ *
+ * @module
+ */
+
+type FlushFn = () => Promise<void>
+
+const flushCallbacks = new Set<FlushFn>()
+
+/**
+ * Register an auto-save flush callback. Returns an unregister function to call
+ * on unmount.
+ *
+ * @param flush - the editor's `forceSave`
+ * @returns a function that removes the callback from the registry
+ */
+export function registerAutoSaveFlush(flush: FlushFn): () => void {
+  flushCallbacks.add(flush)
+  return () => {
+    flushCallbacks.delete(flush)
+  }
+}
+
+/**
+ * Flush every registered auto-save concurrently. Resolves with the count of
+ * callbacks that flushed without throwing and the errors from those that did,
+ * so a caller (e.g. emergency save) can report a real outcome rather than a
+ * no-op. Never rejects.
+ *
+ * @returns the number flushed successfully and any errors encountered
+ */
+export async function flushAllAutoSaves(): Promise<{ saved: number; errors: Error[] }> {
+  const callbacks = Array.from(flushCallbacks)
+  const results = await Promise.allSettled(callbacks.map((flush) => flush()))
+  const errors = results
+    .filter((r): r is PromiseRejectedResult => r.status === 'rejected')
+    .map((r) => (r.reason instanceof Error ? r.reason : new Error(String(r.reason))))
+  return { saved: results.length - errors.length, errors }
+}

annotation-tool/src/hooks/data/useAutoSave.test.ts

@@ -222,6 +222,54 @@ describe('useAutoSave', () => {
     })
   })
 
+  describe('change detection', () => {
+    // Regression guard for the comment-autosave bug: a field folded into
+    // getComparisonSnapshot but living OUTSIDE `data` (e.g. a sibling comment)
+    // must still schedule a save when it changes. The debounce effect keys on
+    // the serialized snapshot, not on `data`, so a snapshot-only change re-arms
+    // it. Fails on the prior code, which keyed the effect on `data` alone.
+    it('schedules a save when only a getComparisonSnapshot field changes (not `data`)', async () => {
+      vi.useFakeTimers()
+      try {
+        const onSave = vi.fn().mockResolvedValue(undefined)
+        // Stable `data` reference across renders, so only the comment changes.
+        const data = { summary: ['unchanged'] }
+
+        const { rerender } = renderHook(
+          ({ comment }: { comment: string }) =>
+            useAutoSave({
+              data,
+              isEnabled: true,
+              onSave,
+              debounceMs: 100,
+              periodicMs: 0,
+              entityType: 'summary',
+              getComparisonSnapshot: (d) => ({ d, comment }),
+            }),
+          { initialProps: { comment: 'first' } },
+        )
+
+        // Flush the initial mount save so it cannot mask the change below.
+        await act(async () => {
+          await vi.advanceTimersByTimeAsync(150)
+        })
+        onSave.mockClear()
+
+        // Change ONLY the comment; `data` is the same object reference.
+        rerender({ comment: 'edited' })
+        await act(async () => {
+          await vi.advanceTimersByTimeAsync(150)
+        })
+
+        expect(onSave).toHaveBeenCalledTimes(1)
+        // And it persists with the latest comment in the saved snapshot.
+        expect(onSave).toHaveBeenLastCalledWith(data)
+      } finally {
+        vi.useRealTimers()
+      }
+    })
+  })
+
   describe('error handling', () => {
     it('sets error status when save fails after all retries', async () => {
       const error = new Error('Save failed')

annotation-tool/src/hooks/data/useAutoSave.ts

@@ -10,6 +10,7 @@
 import { useState, useEffect, useRef, useCallback } from 'react'
 import { logWarning, logCritical } from '@services/errorLogging'
 import { withSpan } from '@telemetry/tracing'
+import { registerAutoSaveFlush } from './autoSaveRegistry'
 
 /**
  * Status of the auto-save operation.
@@ -148,21 +149,28 @@ export function useAutoSave<T>({
 
   // Serialize `data` to the string used for change detection. When a caller
   // supplies getComparisonSnapshot, serialize the snapshot it derives (which
-  // strips server-managed fields the editor never writes); otherwise fall
-  // back to serializing the whole `data`. Held in a ref for the same reason
-  // performSave is (see performSaveRef below): the caller's
-  // getComparisonSnapshot closure can change identity on every render, and we
-  // must not let that churn re-arm the debounce/periodic effects.
+  // strips server-managed fields the editor never writes, and can fold in a
+  // sibling field such as a comment); otherwise fall back to serializing the
+  // whole `data`. The closure is held in a ref and updated DURING render (not in
+  // an effect) so the change key computed below reflects the latest snapshot
+  // this same render — otherwise an edit to a getComparisonSnapshot-only field
+  // (e.g. a comment that lives outside `data`) would lag a render and the
+  // debounce effect, keyed on the change key's string VALUE, would never re-run.
   const getComparisonSnapshotRef = useRef(getComparisonSnapshot)
-  useEffect(() => {
-    getComparisonSnapshotRef.current = getComparisonSnapshot
-  }, [getComparisonSnapshot])
+  getComparisonSnapshotRef.current = getComparisonSnapshot
 
   const serialize = useCallback((value: T): string => {
     const snapshot = getComparisonSnapshotRef.current
     return JSON.stringify(snapshot ? snapshot(value) : value)
   }, [])
 
+  // Value-based change key: the serialized comparison snapshot of the current
+  // data. Because it is a string compared by VALUE, using it as the debounce
+  // effect's dependency re-arms a save on any change to a compared field
+  // (including getComparisonSnapshot-only fields) without churning every render
+  // when the snapshot closure's identity changes.
+  const changeKey = serialize(data)
+
   // Keep the data ref pointed at the latest `data` seen on render. Done during
   // render (not in an effect) so a forceSave fired from the same event handler
   // that just re-rendered the parent reads the committed value rather than the
@@ -196,6 +204,12 @@ export function useAutoSave<T>({
       setSaveStatus(attempt > 0 ? 'retrying' : 'saving')
       setRetryCount(attempt)
 
+      // When a retry is scheduled below, the in-progress guard must stay held
+      // through the backoff delay so a debounce/periodic tick cannot start a
+      // second save concurrently with the pending retry (duplicate writes /
+      // last-writer-wins). The retry timeout releases and re-runs.
+      let retryScheduled = false
+
       try {
         await withSpan(
           `${entityType}-autosave`,
@@ -234,13 +248,16 @@ export function useAutoSave<T>({
         } else if (attempt < maxRetries - 1) {
           // Exponential backoff: 1s, 2s, 4s
           const delay = Math.pow(2, attempt) * 1000
+          retryScheduled = true
           logWarning(`${entityType} save failed, retrying`, {
             entityId,
             retryCount: attempt + 1,
             error: err.message,
           })
 
           setTimeout(() => {
+            // Release and re-run synchronously: performSave re-acquires the guard
+            // before its first await, so no concurrent save can slip into the gap.
             saveInProgressRef.current = false
             performSave(attempt + 1, force, dataOverride)
           }, delay)
@@ -253,7 +270,9 @@ export function useAutoSave<T>({
           })
         }
       } finally {
-        if (attempt === 0 || attempt >= maxRetries - 1) {
+        // Release the guard on a terminal outcome (success or final failure),
+        // but NOT when a retry is pending — the retry timeout owns the release.
+        if (!retryScheduled) {
           saveInProgressRef.current = false
         }
       }
@@ -287,13 +306,23 @@ export function useAutoSave<T>({
     await performSaveRef.current(0, true, dataOverride)
   }, [])
 
-  // Debounced save on data change. Deps deliberately exclude
-  // performSave (held via ref above) — see the ref comment.
+  // Register this editor's flush so a global handler (emergency save on session
+  // expiry) can persist its pending edits. forceSave is stable, so this runs
+  // once; the cleanup deregisters on unmount.
+  useEffect(() => {
+    if (!isEnabled) return
+    return registerAutoSaveFlush(() => forceSave())
+  }, [isEnabled, forceSave])
+
+  // Debounced save on change. Keyed on `changeKey` (the serialized comparison
+  // snapshot) rather than `data` so an edit to ANY compared field — including a
+  // getComparisonSnapshot-only field such as a comment that lives outside `data`
+  // — re-arms the save. Deps deliberately exclude performSave (held via ref) —
+  // see the ref comment.
   useEffect(() => {
     if (!isEnabled) return
 
-    const serialized = serialize(data)
-    if (serialized !== lastSavedDataRef.current) {
+    if (changeKey !== lastSavedDataRef.current) {
       setPendingChanges(true)
     }
 
@@ -310,7 +339,7 @@ export function useAutoSave<T>({
         clearTimeout(debounceTimerRef.current)
       }
     }
-  }, [data, isEnabled, debounceMs, serialize])
+  }, [changeKey, isEnabled, debounceMs])
 
   // Periodic backup save. performSave intentionally NOT in deps for
   // the same reason — periodicTimer should NOT re-arm every render.

annotation-tool/src/store/queries/useGroups.ts

@@ -267,6 +267,9 @@ export function useUpdateGroupMember() {
     onSuccess: (_, { groupId }) => {
       queryClient.invalidateQueries({ queryKey: groupKeys.detail(groupId) })
       queryClient.invalidateQueries({ queryKey: groupKeys.members(groupId) })
+      // The list carries each group's own-role field, which a self-role change
+      // makes stale; refresh it (the add/remove paths already do).
+      queryClient.invalidateQueries({ queryKey: groupKeys.lists() })
       // Membership/role changes alter the caller's own permissions; refresh the
       // client ability mirror so the UI reflects them without a staleTime lag.
       queryClient.invalidateQueries({ queryKey: abilityKeys.all })

annotation-tool/src/store/queries/usePersonas.test.tsx

@@ -281,6 +281,38 @@ describe('usePersonas hooks', () => {
 
         await waitFor(() => expect(result.current.isSuccess).toBe(true))
       })
+
+      // Regression guard: type deletion must hit the explicit DELETE endpoint,
+      // not a whole-ontology PUT that merely omits the type. The server's
+      // ontology PUT merges by id, so an omission would be a no-op (the type
+      // would be kept and re-seeded on the next refetch).
+      it('calls the DELETE endpoint and not a whole-ontology PUT', async () => {
+        let deleteHit = false
+        let putHit = false
+        server.use(
+          http.delete('/api/personas/:personaId/ontology/:category/:typeId', () => {
+            deleteHit = true
+            return HttpResponse.json({ message: 'deleted', glossReferencesConverted: 0, annotationsDeleted: 0, worldAssignmentsRemoved: 0 })
+          }),
+          http.put('/api/personas/:personaId/ontology', () => {
+            putHit = true
+            return HttpResponse.json({})
+          })
+        )
+        const ontologyWithEntity = {
+          ...emptyOntology,
+          entities: [{ id: 'entity-1', name: 'Vehicle', definition: 'x', wikidataId: null, parentId: null }],
+        }
+        const { result } = renderHook(() => useDeleteEntityFromPersona(), {
+          wrapper: createWrapper({ [ontologyCacheKey]: ontologyWithEntity }),
+        })
+
+        result.current.mutate({ personaId: 'persona-1', entityId: 'entity-1' })
+        await waitFor(() => expect(result.current.isSuccess).toBe(true))
+
+        expect(deleteHit).toBe(true)
+        expect(putHit).toBe(false)
+      })
     })
   })