fix(lora): keep the save outcome, honour the umask and sweep stale downloads

Leaving Integrations drops the CivitAI key draft only when a key was
typed, so the last Save's outcome survives and Sources still marks a
refused source. The key status line is rewritten only when it changes.

Cached CivitAI images are opened like a plain save, so they keep the
umask's mode, and each import removes .pm-*.part files older than an
hour from the image cache. A legacy scan stopped because its page left
ends cancelled in Activity, and staging is awaited to the end even if
the handler is cancelled again.

TESTING.md

@@ -2664,8 +2664,9 @@ Recorded deviations (plan D60–D70, D74, D75, D79–D82 renumbered D92–D102,
   A `p.small#civitai-key-status[role=status]` (it also describes the field) says "A key
   is saved." / "No key saved." / "The new key replaces the saved one when you save." /
   "The saved key is removed when you save."; pressing "Clear saved key" keeps the
-  keyboard on the key field. A typed key is dropped when the section is left (the field
-  comes back empty). The field is `autocomplete=new-password`, so browsers never offer it
+  keyboard on the key field, and the line is rewritten only when its text changes. A typed
+  key is dropped when the section is left (the field comes back empty); leaving without one
+  keeps the last Save's outcome, so Sources still marks a refused source. The field is `autocomplete=new-password`, so browsers never offer it
   as a login; the concept's
   prototype warning is dropped. The LoRA rows save through the one Save row:
   `POST /lora/enable` with `expected_revision` (a config.json changed elsewhere is
@@ -3147,9 +3148,12 @@ imports changed meanwhile), `blocked-by-pairs`, `nothing-to-import`, `backup-fai
 whatever happened after. The tests change real files behind a held download
 (`test_files_changed_while_staging_fail_the_job`, removed and added files too). The
 replace binds the category, not one parameter per prompt, so SQLite's variable limit
-never caps a collection. The legacy `POST /lora/scan` runs the same stages under its SSE
+never caps a collection. Cached CivitAI images are written to a `.pm-*.part` file in the
+same folder and renamed into place, with the umask's mode as a plain save gives; each import
+first removes `.pm-*.part` files older than an hour from the image cache and its LoRA
+folders (`TestStaleDownloadSweep`). The legacy `POST /lora/scan` runs the same stages under its SSE
 event shape; a page that closes its connection stops staging before its next LoRA, and the
-scan keeps its records hold until staging has stopped
+scan keeps its records hold until staging has stopped, then ends `cancelled` in Activity
 (`test_a_disconnected_legacy_scan_stops_and_holds_until_it_stopped`) (`test_legacy_scan_events_unchanged_on_success`), holds
 the registry as "LoRA import (admin page)" and takes no safety backup. `POST /lora/enable`
 accepts the optional `expected_revision`. LoraManager detection also searches every

py/api/lora_import_routes.py

@@ -135,6 +135,7 @@ class LoraImportRoutesMixin:
         from ..config import IntegrationConfig
 
         cache_dir = await self._run_in_executor(lora_utils.get_lora_image_cache_dir)
+        await self._run_in_executor(lora_utils.sweep_stale_downloads, cache_dir)
         loop = asyncio.get_running_loop()
 
         def report(index, record, staged):

py/api/lora_integration.py

@@ -73,6 +73,13 @@ def _closed(request):
     return closed
 
 
+def _stopped(error) -> bool:
+    """Whether a legacy scan ended because its page closed the connection."""
+    from .. import lora_import
+
+    return isinstance(error, (ConnectionError, lora_import.ImportCancelled))
+
+
 class LoraIntegrationMixin:
     """Mixin providing LoraManager detection, scanning, and trigger word endpoints."""
 
@@ -357,8 +364,13 @@ class LoraIntegrationMixin:
                     await self._lora_scan_stream(
                         allowed, send_progress, disconnected=_closed(request)
                     )
+                except asyncio.CancelledError:
+                    # The server cancelled the handler when the page left.
+                    job.finish("cancelled")
+                    raise
                 except Exception as e:
-                    job.finish("failed")
+                    # A page that left stopped the scan; nothing failed (R2).
+                    job.finish("cancelled" if _stopped(e) else "failed")
                     await self._send_scan_failure(e, send_progress)
                 with contextlib.suppress(ConnectionError, RuntimeError):
                     await response.write_eof()
@@ -375,14 +387,13 @@ class LoraIntegrationMixin:
         import) are warnings in the admin page's own words; anything else is
         logged with its traceback.
         """
-        from .. import lora_import
         from .lora_import_routes import (
             EmptyReplacement,
             PairDependencyError,
             PlanChanged,
         )
 
-        if isinstance(error, (ConnectionError, lora_import.ImportCancelled)):
+        if _stopped(error):
             self.logger.info("LoRA scan stopped: the page closed its connection")
             return
         if isinstance(error, PlanChanged):
@@ -498,8 +509,18 @@ class LoraIntegrationMixin:
                 await report(*events.get_nowait())
         finally:
             stop.set()
-            with contextlib.suppress(Exception):
-                await asyncio.shield(staging)
+            # Staging must have stopped before the records hold is released,
+            # even when the handler is cancelled again while it waits (R3).
+            interrupted = False
+            while not staging.done():
+                try:
+                    await asyncio.shield(staging)
+                except asyncio.CancelledError:
+                    interrupted = True
+                except Exception:
+                    pass
+            if interrupted:
+                raise asyncio.CancelledError
         staged = await staging
         if not await self.lora_files_unchanged(lm_path, reviewed):
             raise LoraFilesChanged(_FILES_CHANGED)

py/lora_utils.py

@@ -524,21 +524,35 @@ def _download_one(url: str, local_path: Path, api_key: str) -> Optional[str]:
         return None
 
 
+_PART_PREFIX = ".pm-"
+_PART_SUFFIX = ".part"
+# A temp download older than this was left by a killed process, not a live one
+# (each download has a 10 s timeout).
+STALE_PART_SECONDS = 3600
+
+
 def _save_atomically(image, local_path: Path) -> None:
     """Write ``image`` as JPEG so ``local_path`` only ever names a whole file.
 
-    The image goes to a temp file in the same folder, which is closed before
-    ``os.replace`` puts it in place (Windows cannot replace an open file); a
-    failure removes the temp file. A reader (another import's cache check)
-    never sees a half-written image.
+    The image goes to a ``.pm-*.part`` file in the same folder, which is
+    closed before ``os.replace`` puts it in place (Windows cannot replace an
+    open file); a failure removes the temp file. A reader (another import's
+    cache check) never sees a half-written image. The temp file is opened
+    like ``img.save`` opens a file, so the image keeps the umask's mode.
     """
-    import tempfile
+    import secrets
 
-    fd, temp = tempfile.mkstemp(
-        dir=str(local_path.parent), prefix=".pm-", suffix=".part"
-    )
+    while True:
+        temp = local_path.parent / (
+            f"{_PART_PREFIX}{secrets.token_hex(8)}{_PART_SUFFIX}"
+        )
+        try:
+            handle = open(temp, "xb")
+        except FileExistsError:
+            continue
+        break
     try:
-        with os.fdopen(fd, "wb") as handle:
+        with handle:
             image.save(handle, "JPEG", quality=85)
         os.replace(temp, local_path)
     except BaseException:
@@ -549,6 +563,33 @@ def _save_atomically(image, local_path: Path) -> None:
         raise
 
 
+def sweep_stale_downloads(cache_dir: Path) -> None:
+    """Remove ``.pm-*.part`` files a killed download left in the image cache.
+
+    Only that exact pattern, only in ``cache_dir`` and its LoRA folders, and
+    only once older than ``STALE_PART_SECONDS``; a file that cannot be read
+    or removed is left for a later import.
+    """
+    import time
+
+    cutoff = time.time() - STALE_PART_SECONDS
+    try:
+        folders = [cache_dir] + [p for p in cache_dir.iterdir() if p.is_dir()]
+    except OSError:
+        return
+    for folder in folders:
+        try:
+            candidates = list(folder.glob(f"{_PART_PREFIX}*{_PART_SUFFIX}"))
+        except OSError:
+            continue
+        for path in candidates:
+            try:
+                if path.is_file() and path.stat().st_mtime < cutoff:
+                    os.remove(path)
+            except OSError:
+                logger.debug(f"Could not remove stale download {path}")
+
+
 def _civitai_cache_plan(
     metadata: Dict, metadata_path: Path, cache_dir: Path
 ) -> Tuple[Path, List[str], List[Tuple[str, Path]]]:

tests/browser/system-integrations-concept.spec.mjs

@@ -71,7 +71,6 @@ function measureIntegrations(mainSelector) {
         // The concept's `.small` text (its Save row status); production's row notes use the same class.
         small: css(layout.querySelector('#settings-status'), 'fontSize', 'color'),
         note: note && css(note, 'fontSize', 'color'),
-        // Whether each select shows its whole chosen option (its text within the box, less the native arrow).
         // Whether each select is as wide as the browser lays out a copy holding only its chosen option, i.e. it shows
         // that option whole (the browser's own select sizing, arrow included).
         selectsFit: [...layout.querySelectorAll('.setting-row > select')].map(select => {
@@ -178,6 +177,16 @@ for (const theme of THEMES) for (const [width, height] of SIZES) {
     });
 }
 
+// D106 below the concept's smallest size: on a 320px phone both selects still show their whole option, without page
+// scroll (S10 re-review N9).
+for (const theme of THEMES) test(`integrations selects fit at 320 wide ${theme}`, async ({ page, request }) => {
+    await page.setViewportSize({ width: 320, height: 700 });
+    await fakeLoraManager(request);
+    const production = await productionIntegrations(page, theme);
+    expect(production.selectsFit).toEqual([true, true]);
+    expect(production.pageScroll).toBe(false);
+});
+
 test('integrations comparison detects mutations', async ({ page, request }) => {
     await page.setViewportSize({ width: 1600, height: 1100 });
     await fakeLoraManager(request);

tests/browser/workspace-system-integrations.spec.mjs

@@ -218,6 +218,18 @@ test('key never prefilled; blank keeps; Clear saved key clears; saved through Sa
     await expect(field).toHaveValue('');
     await expect(key.locator('p.small')).toHaveText('A key is saved.');
     expect(statusBodies.join('')).not.toContain('sk-test-key');
+    // The announced line is rewritten only when its text changes, not on every edit elsewhere (S10 re-review N8).
+    await page.evaluate(() => {
+        window.__keyNoteWrites = 0;
+        new MutationObserver(records => { window.__keyNoteWrites += records.length; })
+            .observe(document.querySelector('#civitai-key-status'), { childList: true, characterData: true, subtree: true });
+    });
+    const triggers = settingOf(page, 'Automatically include trigger words').locator('input');
+    await triggers.check();
+    await expect(status(page)).toHaveText('Unsaved changes.');
+    await triggers.uncheck();
+    await expect(status(page)).toHaveText('No unsaved changes.');
+    expect(await page.evaluate(() => window.__keyNoteWrites)).toBe(0);
     // A blank field keeps the saved key: nothing to save.
     await field.fill('   ');
     await expect(status(page)).toHaveText('No unsaved changes.');
@@ -285,6 +297,29 @@ test('a save where only the LoRA part is written says so; the next save needs no
         expect((await serverJson(request, '/prompt_manager/settings')).sources.configured.map(source => source.path)).toEqual(['output']);
     });
 
+test('a partial save made from Integrations keeps its outcome when Sources opens (S10 re-review R1)',
+    async ({ page, request }) => {
+        await lora(request);
+        await expect(await request.post('/__test__/system/missing-source', { data: { inside: true } })).toBeOK();
+        await openIntegrations(page);
+        const nav = page.locator('#workspace-main .section-nav');
+        const sourceRow = title => layoutOf(page).locator('#source-list .operation').filter({ has: page.locator('h3', { hasText: title }) });
+        await nav.getByRole('button', { name: 'Sources & scans', exact: true }).click();
+        await expect(sourceRow('unplugged')).toBeVisible();
+        await expect(await request.post('/__test__/system/unplug')).toBeOK();
+        await sourceRow('ComfyUI input').getByRole('button', { name: 'Remove', exact: true }).click();
+        // Back in Integrations, a LoRA change, then Save from there.
+        await nav.getByRole('button', { name: 'Integrations', exact: true }).click();
+        await settingOf(page, 'Automatically include trigger words').locator('input').check();
+        await save(page).click();
+        const reason = "output/unplugged can't be used: Gallery root does not exist. Remove it or reconnect it, then save.";
+        await expect(status(page)).toHaveText(reason);
+        // Leaving Integrations (no key was typed) keeps the outcome: Sources shows the reason on the refused row.
+        await nav.getByRole('button', { name: 'Sources & scans', exact: true }).click();
+        await expect(sourceRow('unplugged')).toHaveAttribute('aria-invalid', 'true');
+        await expect(status(page)).toHaveText(reason);
+    });
+
 test('Review import shows replacement, blocked items and network effects with the backup box; Start opens the job (D107)',
     async ({ page, request }) => {
         await lora(request, { loras: 3, civitai: true, previous: 2 });

tests/test_lora_import.py

@@ -13,6 +13,7 @@ import sqlite3
 import sys
 import tempfile
 import threading
+import time
 import unittest
 from pathlib import Path
 from unittest.mock import patch
@@ -531,6 +532,9 @@ class LoraImportRouteTests(LoraImportAPITestCase):
                 break
             await asyncio.sleep(0.01)
         self.assertIsNone(self.api._jobs.active("lora_import"))
+        # R2 (re-review): the user left and nothing failed, so Activity says so.
+        job = next(j for j in self.api._jobs.list() if j.kind == "lora_import")
+        self.assertEqual(job.state, "cancelled")
         # Staging stopped at the next LoRA: no download runs after the hold.
         served = len(self.requests)
         self.assertLess(served, 4)
@@ -538,6 +542,21 @@ class LoraImportRouteTests(LoraImportAPITestCase):
         self.assertEqual(len(self.requests), served)
         self.assertEqual(_snapshot(self.api.db), before)
 
+    async def test_an_import_sweeps_stale_temp_downloads_from_the_cache(self):
+        # N7 (re-review): a download killed mid-write leaves a .pm-*.part file;
+        # the next import removes it once it is an hour old.
+        self._two_loras()
+        stale = self.cache_dir / "old_lora" / ".pm-dead.part"
+        stale.parent.mkdir(parents=True)
+        stale.write_bytes(b"x")
+        stamp = time.time() - 2 * 3600
+        os.utime(stale, (stamp, stamp))
+
+        resp = await self.client.request("POST", SCAN)
+        await resp.text()
+
+        self.assertFalse(stale.exists())
+
     async def test_legacy_scan_events_unchanged_on_success(self):
         self._two_loras()
         (self.loras / "broken.metadata.json").write_text("{{{")

tests/test_lora_utils.py

@@ -12,6 +12,7 @@ import shutil
 import sys
 import tempfile
 import threading
+import time
 import types
 import unittest
 import urllib.parse
@@ -576,6 +577,61 @@ class TestDownloadIsAtomic(unittest.TestCase):
         self.assertIsNone(result)
         self.assertEqual(list(self.tmpdir.iterdir()), [])
 
+    @unittest.skipIf(os.name == "nt", "POSIX file modes")
+    def test_the_image_gets_the_permissions_a_plain_save_gives(self):
+        # S10 re-review N7: the cached JPEG honours the umask like img.save did.
+        previous = os.umask(0o022)
+        self.addCleanup(os.umask, previous)
+        opener = _FakeOpener(_FakeResponse(_png_bytes()))
+        with patch.object(lora_utils, "_open_url", opener):
+            lora_utils._download_one(
+                "https://civitai.com/api/download/1", self.target, ""
+            )
+        self.assertEqual(self.target.stat().st_mode & 0o777, 0o644)
+
+
+class TestStaleDownloadSweep(unittest.TestCase):
+    """S10 re-review N7: a killed download's ``.pm-*.part`` is swept later."""
+
+    def setUp(self):
+        self.cache = Path(tempfile.mkdtemp())
+        self.addCleanup(shutil.rmtree, self.cache, True)
+        self.lora = self.cache / "my_lora"
+        self.lora.mkdir()
+
+    def _file(self, path, age_seconds):
+        path.write_bytes(b"x")
+        stamp = time.time() - age_seconds
+        os.utime(path, (stamp, stamp))
+        return path
+
+    def test_only_old_temp_downloads_are_removed(self):
+        old = self._file(self.lora / ".pm-abc123.part", 2 * 3600)
+        old_top = self._file(self.cache / ".pm-top.part", 2 * 3600)
+        fresh = self._file(self.lora / ".pm-new.part", 60)
+        image = self._file(self.lora / "abc.jpg", 2 * 3600)
+        other = self._file(self.lora / "keep.part", 2 * 3600)
+        prefixed = self._file(self.lora / ".pm-keep.jpg", 2 * 3600)
+        deep = self.lora / "nested"
+        deep.mkdir()
+        nested = self._file(deep / ".pm-deep.part", 2 * 3600)
+
+        lora_utils.sweep_stale_downloads(self.cache)
+
+        self.assertFalse(old.exists())
+        self.assertFalse(old_top.exists())
+        for kept in (fresh, image, other, prefixed, nested):
+            self.assertTrue(kept.exists(), kept.name)
+
+    def test_a_missing_cache_is_fine(self):
+        lora_utils.sweep_stale_downloads(self.cache / "absent")
+
+    def test_a_file_that_cannot_be_removed_is_left(self):
+        old = self._file(self.lora / ".pm-abc.part", 2 * 3600)
+        with patch.object(lora_utils.os, "remove", side_effect=PermissionError):
+            lora_utils.sweep_stale_downloads(self.cache)
+        self.assertTrue(old.exists())
+
 
 class _FakeHTTPSHandler(urllib.request.HTTPSHandler):
     """Serves canned responses through a real urllib opener, recording requests.

web/js/workspace/system-integrations.mjs

@@ -414,7 +414,9 @@ export function mountIntegrations(container, { api, activity, scope, drafts, cli
         pathInput.value = values.lora_path || values.lora_detected_path || '';
         setNote(triggers, cacheText(values));
         keyInput.disabled = unread;
-        keyNote.textContent = keyText(values, changes) ?? '';
+        // A live region: rewritten only when it says something new, so it is not re-announced on every edit.
+        const note = keyText(values, changes) ?? '';
+        if (keyNote.textContent !== note) keyNote.textContent = note;
         // D114: "Clear saved key" is offered only while a saved key would stay.
         if (values.lora_has_key === true && changes.clear_civitai_api_key !== true) key.children[0].append(keyActions);
         else {
@@ -434,8 +436,11 @@ export function mountIntegrations(container, { api, activity, scope, drafts, cli
     };
     scope.cleanup(drafts.subscribe(repaint));
     // A typed key is held only while its field shows it: leaving the section drops it, since the field comes back
-    // empty and a hidden key draft could not be seen or discarded (S10 review N4).
-    scope.cleanup(() => drafts.set('host', 'civitai_api_key', undefined));
+    // empty and a hidden key draft could not be seen or discarded (S10 review N4). Only a typed key: any set clears
+    // the last Save's outcome, which Sources still needs (S10 re-review R1).
+    scope.cleanup(() => {
+        if (drafts.changes('host').civitai_api_key !== undefined) drafts.set('host', 'civitai_api_key', undefined);
+    });
     repaint();
     return [comfy, surface];
 }