fix(backup): drop the settings-pin workaround, upstream fixed the cause (#2656)

This modal carried two workarounds for #2716: `onSuccess` deliberately did not
    invalidate `['settings']`, and a query-cache subscription pinned the entry to
    the pre-restore copy for as long as the result panel was up. Both existed
    because SettingsPage's debounced auto-save diffed its `localSettings` form
    state against the live cache, so any refetch of a restored settings row --
    this modal's, a window refocus, a reconnect, or any of the ~30 other observers
    of the key -- read as an edit and PATCHed the pre-restore values back over the
    restore about 500 ms later.

    `43cb216a` on dev fixed that. The page now keeps a server baseline and
    reconciles a moved snapshot field by field: an untouched field adopts the
    server's value instead of overwriting it. The restore no longer needs an
    exception, and maziggy explicitly invited dropping it.

    A commit on top rather than a rebase-drop of `21bb5afc`: later commits touch
    this file, and the workaround was right when it was written. This says so.

    The reload on close stays -- it was never one of the two workarounds. Its
    stated reason was, though, and it was the #2716 bug, so it is restated for
    what it actually buys: invalidating `['settings']` only resyncs what reads
    that query, and the interface language, currency and auth toggles are read on
    boot.

    Tests: "never invalidates the settings query" inverts; the pin test and its
    control go with the pin. The reload pair stays. 28 -> 26 tests in this file.

frontend/src/__tests__/components/GitHubRestoreModal.test.tsx

@@ -6,7 +6,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest';
 import { screen, waitFor } from '@testing-library/react';
 import userEvent from '@testing-library/user-event';
 import { delay, http, HttpResponse } from 'msw';
-import { QueryClient, useQuery, useQueryClient } from '@tanstack/react-query';
+import { QueryClient } from '@tanstack/react-query';
 import { render } from '../utils';
 import { server } from '../mocks/server';
 import { GitHubRestoreModal } from '../../components/GitHubRestoreModal';
@@ -538,14 +538,13 @@ describe('GitHubRestoreModal', () => {
     expect(onClose).toHaveBeenCalled();
   });
 
-  // Regression guard for the settings clobber found in manual testing (#2656).
-  // SettingsPage — which renders this modal — holds a `localSettings` copy of the
-  // form state and a debounced effect that PATCHes it back whenever the server
-  // copy differs. Refetching ['settings'] here therefore made the page overwrite
-  // the restore with the pre-restore values, silently, ~500 ms later. 75 of the
-  // ~80 keys in a real backup sit in that save payload, so a restore reported as
-  // "77 restored, 0 failed" left almost nothing behind.
-  describe('settings restore must not be undone by SettingsPage', () => {
+  // A settings restore has to reach the rest of the app. It used to be the
+  // opposite problem: SettingsPage's debounced auto-save wrote its pre-restore
+  // form state back over the restore whenever ['settings'] refetched, so this
+  // modal skipped that invalidation and pinned the cache instead. #2716 fixed
+  // the page — it now reconciles a moved server snapshot field by field — and
+  // the workaround came out with this commit.
+  describe('a settings restore reaches the rest of the app', () => {
     /** Runs a restore returning `results`, leaving the modal on its summary. */
     async function restoreWith(results: Record<string, unknown>, onClose = vi.fn()) {
       server.use(
@@ -586,29 +585,13 @@ describe('GitHubRestoreModal', () => {
       };
     }
 
-    /**
-     * Stands in for SettingsPage: hands the test the provider's QueryClient and
-     * keeps an observer on ['settings'] for as long as the modal is mounted, so
-     * the entry survives the test client's `gcTime: 0`. Disabled, because these
-     * tests write the cache directly rather than fetching it.
-     */
-    function makeSettingsProbe(capture: (client: QueryClient) => void) {
-      return function SettingsProbe() {
-        capture(useQueryClient());
-        useQuery({ queryKey: ['settings'], queryFn: async () => null, enabled: false });
-        return null;
-      };
-    }
-
-    it('never invalidates the settings query', async () => {
+    it('invalidates the settings query alongside the other rewritten caches', async () => {
       const invalidate = vi.spyOn(QueryClient.prototype, 'invalidateQueries');
       await restoreWith({ settings: { restored: 77, skipped: 3, failed: 0, notes: [] } });
 
       const keys = invalidate.mock.calls.map((c) => JSON.stringify(c[0]?.queryKey));
-      // The caches this restore genuinely rewrites are still refreshed...
       expect(keys).toContain(JSON.stringify(['spools']));
-      // ...but ['settings'] must not be, or the page writes the old values back.
-      expect(keys).not.toContain(JSON.stringify(['settings']));
+      expect(keys).toContain(JSON.stringify(['settings']));
       invalidate.mockRestore();
     });
 
@@ -623,101 +606,15 @@ describe('GitHubRestoreModal', () => {
         await userEvent.click(closeButtons[closeButtons.length - 1]);
 
         expect(loc.reload).toHaveBeenCalled();
-        // Closing in place would leave SettingsPage mounted and armed.
+        // Invalidating ['settings'] only resyncs what reads that query. The
+        // interface language and the auth state do not, so closing in place
+        // would leave both showing their pre-restore values.
         expect(onClose).not.toHaveBeenCalled();
       } finally {
         loc.restore();
       }
     });
 
-    // Not invalidating ['settings'] only closes the refetch *this* modal caused.
-    // The query still refetches on window focus and on reconnect — both default
-    // to true, and ['settings'] has other always-mounted observers — and a
-    // refetch landing while the result panel is up puts the restored values in
-    // the cache next to SettingsPage's pre-restore `localSettings`, which is
-    // precisely the state its debounced effect writes back. So the cache is
-    // pinned to the copy the page already agrees with until the exit reload.
-    it('pins the settings cache against a background refetch landing under the result panel', async () => {
-      const preRestore = { currency: 'EUR' };
-      let client: QueryClient | null = null;
-      const Probe = makeSettingsProbe((c) => {
-        client = c;
-      });
-
-      server.use(
-        http.post('/api/v1/github-backup/restore', () =>
-          HttpResponse.json({
-            success: true,
-            message: 'Restored 77 item(s) from aaa1111',
-            log_id: 7,
-            ref: mockPreview.ref,
-            results: { settings: { restored: 77, skipped: 3, failed: 0, notes: [] } },
-          })
-        )
-      );
-      render(
-        <>
-          <Probe />
-          <GitHubRestoreModal onClose={vi.fn()} />
-        </>
-      );
-      // The copy SettingsPage's form state was built from.
-      client!.setQueryData(['settings'], preRestore);
-
-      const checkboxes = await waitFor(() => screen.getAllByRole('checkbox') as HTMLInputElement[]);
-      await userEvent.click(checkboxes[2]);
-      await userEvent.click(screen.getByRole('button', { name: /Restore$/ }));
-      await waitFor(() => screen.getByText('Restore from backup?'));
-      const confirmButtons = screen.getAllByRole('button', { name: /Restore$/ });
-      await userEvent.click(confirmButtons[confirmButtons.length - 1]);
-      await waitFor(() => screen.getByText('Restored 77 item(s) from aaa1111'));
-
-      // A focus/reconnect refetch lands the restored server values.
-      client!.setQueryData(['settings'], { currency: 'USD' });
-
-      // Pinned back, so SettingsPage sees no divergence and saves nothing.
-      expect(client!.getQueryData(['settings'])).toEqual(preRestore);
-    });
-
-    it('leaves the settings cache alone when settings were not restored', async () => {
-      let client: QueryClient | null = null;
-      const Probe = makeSettingsProbe((c) => {
-        client = c;
-      });
-      server.use(
-        http.post('/api/v1/github-backup/restore', () =>
-          HttpResponse.json({
-            success: true,
-            message: 'Restored 4 item(s) from aaa1111',
-            log_id: 8,
-            ref: mockPreview.ref,
-            results: { spools: { restored: 4, skipped: 0, failed: 0, notes: [] } },
-          })
-        )
-      );
-      render(
-        <>
-          <Probe />
-          <GitHubRestoreModal onClose={vi.fn()} />
-        </>
-      );
-      client!.setQueryData(['settings'], { currency: 'EUR' });
-
-      const checkboxes = await waitFor(() => screen.getAllByRole('checkbox') as HTMLInputElement[]);
-      await userEvent.click(checkboxes[1]);
-      await userEvent.click(screen.getByRole('button', { name: /Restore$/ }));
-      await waitFor(() => screen.getByText('Restore from backup?'));
-      const confirmButtons = screen.getAllByRole('button', { name: /Restore$/ });
-      await userEvent.click(confirmButtons[confirmButtons.length - 1]);
-      await waitFor(() => screen.getByText('Restored 4 item(s) from aaa1111'));
-
-      const fresh = { currency: 'USD' };
-      client!.setQueryData(['settings'], fresh);
-      // No settings were touched, so there is nothing to protect and normal
-      // refetching must keep working.
-      expect(client!.getQueryData(['settings'])).toEqual(fresh);
-    });
-
     it('closes normally when settings were not part of the restore', async () => {
       const loc = stubReload();
       try {

frontend/src/components/GitHubRestoreModal.tsx

@@ -1,4 +1,4 @@
-import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
+import { useCallback, useEffect, useMemo, useState } from 'react';
 import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query';
 import { useTranslation } from 'react-i18next';
 import {
@@ -81,10 +81,6 @@ export function GitHubRestoreModal({ onClose }: GitHubRestoreModalProps) {
   const [overwriteExisting, setOverwriteExisting] = useState(false);
   const [showConfirm, setShowConfirm] = useState(false);
   const [result, setResult] = useState<GitHubRestoreResponse | null>(null);
-  // The ['settings'] cache entry as it stood when the restore returned — i.e.
-  // the copy SettingsPage's form state was built from. Used to pin the cache
-  // while a settings restore result is on screen; see the effect below.
-  const pinnedSettings = useRef<unknown>(undefined);
 
   const commitsQuery = useQuery({
     queryKey: ['github-backup-commits'],
@@ -158,32 +154,33 @@ export function GitHubRestoreModal({ onClose }: GitHubRestoreModalProps) {
       // above a message saying nothing had been restored. Only a real success
       // gets the panel; a failure keeps the form and shows the red block below.
       if (data.success) {
-        pinnedSettings.current = queryClient.getQueryData(['settings']);
         setResult(data);
-        // A restore rewrites rows these caches hold.
+        // A restore rewrites rows these caches hold. ['settings'] is one of
+        // them: until #2716 was fixed on dev, invalidating it made
+        // SettingsPage's debounced auto-save write the pre-restore form state
+        // straight back over the restore, so this modal skipped it and pinned
+        // the cache instead. That page now reconciles a moved server snapshot
+        // field by field, so the restore no longer needs an exception.
         queryClient.invalidateQueries({ queryKey: ['spools'] });
         queryClient.invalidateQueries({ queryKey: ['archives'] });
+        queryClient.invalidateQueries({ queryKey: ['settings'] });
       }
       // A failure that got as far as resolving the commit still writes a log row
       // (status "failed"), so refresh the history and status either way.
       queryClient.invalidateQueries({ queryKey: ['github-backup-logs'] });
       queryClient.invalidateQueries({ queryKey: ['github-backup-status'] });
-      // ['settings'] is deliberately NOT invalidated. SettingsPage — which
-      // renders this modal — keeps a `localSettings` copy of the form state and
-      // a debounced effect that PATCHes it back whenever the server copy
-      // differs. Refetching here makes that effect write the pre-restore values
-      // over everything we just restored, 500 ms later and silently: 75 of the
-      // ~80 keys in a backup are in its save payload. A reload is the only safe
-      // resync, so `closeModal` below forces one instead.
     },
     onError: () => setShowConfirm(false),
   });
 
   const isRestoring = restoreMutation.isPending;
 
-  // Leaving this modal mounted after a settings restore lets SettingsPage
-  // overwrite what was restored (see the onSuccess comment), so every exit path
-  // reloads rather than just closing.
+  // A settings restore rewrites rows the whole app reads, and not all of them
+  // through a query this modal can invalidate. The interface language is applied
+  // by i18n.changeLanguage, called only from the SettingsPage dropdown and the
+  // appliance-locale bootstrap; the auth state comes from AuthProvider's
+  // mount-time getAuthStatus, not from ['settings'] at all. So every exit path
+  // after a settings restore reloads rather than just closing.
   const settingsRestored = Boolean(result && 'settings' in result.results);
   const closeModal = useCallback(() => {
     if (settingsRestored) {
@@ -193,37 +190,6 @@ export function GitHubRestoreModal({ onClose }: GitHubRestoreModalProps) {
     onClose();
   }, [settingsRestored, onClose]);
 
-  // Not invalidating ['settings'] above keeps the page from reverting a restore
-  // *we* triggered a refetch for, but it is not enough on its own: the query
-  // still refetches on window focus and on reconnect (both default to true, and
-  // ['settings'] has other always-mounted observers that trigger it), and any
-  // such refetch lands the restored values in the cache while SettingsPage's
-  // `localSettings` still holds the pre-restore ones — which is exactly the
-  // state its debounced effect PATCHes back. Tabbing away from the result panel
-  // for a minute and back was enough to lose the restore.
-  //
-  // The refetch cannot be prevented from here, so pin the cache instead: while
-  // the result panel is up, any fresh ['settings'] payload is written straight
-  // back to the copy the page already agrees with, so `hasChanges` stays false
-  // and nothing is saved over the restore. Every exit path reloads (above), and
-  // that reload is what resyncs the page for real.
-  useEffect(() => {
-    if (!settingsRestored) return;
-    const pinned = pinnedSettings.current;
-    if (pinned === undefined) return;
-    // Compared by value, not identity: react-query's structural sharing stores a
-    // copy rather than the object handed to setQueryData, so an identity check
-    // would never match its own write and would recurse until the stack blew.
-    const pinnedJson = JSON.stringify(pinned);
-    return queryClient.getQueryCache().subscribe((event) => {
-      if (event.type !== 'updated' || event.action.type !== 'success') return;
-      const key = event.query.queryKey;
-      if (!Array.isArray(key) || key.length !== 1 || key[0] !== 'settings') return;
-      if (JSON.stringify(event.query.state.data) === pinnedJson) return;
-      queryClient.setQueryData(['settings'], pinned);
-    });
-  }, [settingsRestored, queryClient]);
-
   // Close on Escape, except while a restore is in flight.
   useEffect(() => {
     const handleKeyDown = (e: KeyboardEvent) => {

static/index.html

@@ -26,8 +26,8 @@
 
     <!-- Splash screens for iOS -->
     <link rel="apple-touch-startup-image" href="/img/android-chrome-512x512.png" />
-    <script type="module" crossorigin src="/assets/index-B67xFyee.js"></script>
-    <link rel="stylesheet" crossorigin href="/assets/index-B3jj6-fz.css">
+    <script type="module" crossorigin src="/assets/index-CHCEEMgx.js"></script>
+    <link rel="stylesheet" crossorigin href="/assets/index-C_6BSgrK.css">
   </head>
   <body>
     <div id="root"></div>