mirror of
https://github.com/willmiao/ComfyUI-Lora-Manager.git
synced 2026-09-27 05:54:08 -03:00
feat(sidecars): surface storage location and cover excluded models in migration
After migrating to centralized sidecar storage users had no indication where their files went, and portable-mode installs silently placed the sidecar root inside the plugin folder where a reinstall or git clean would delete it. - Migration now also covers models excluded from the library view and returns the resolved sidecar root in its result payload - get_settings exposes the resolved sidecar root, whether it is the default, and whether it lives inside the installation folder - New POST /api/lm/sidecars/open-location endpoint opens (or copies) the sidecar storage folder - Settings UI always shows the effective storage path with an open-folder button, and warns when the root is inside the installation folder (portable-mode hazard) - Migration confirmation shows the destination; on completion a result dialog summarizes moved/skipped/conflict counts with the storage location and an open-folder action - Ignore /sidecars/ at the repository root so portable-mode sidecars are never committed Refs #1045
This commit is contained in:
@@ -116,6 +116,7 @@ const appendMigrationModal = () => {
|
||||
modal.innerHTML = `
|
||||
<h2 data-role="title"></h2>
|
||||
<p data-role="message"></p>
|
||||
<p data-role="destination" style="display:none"></p>
|
||||
<button data-action="confirm-sidecar-migration"></button>
|
||||
<button data-action="cancel-sidecar-migration"></button>`;
|
||||
document.body.appendChild(modal);
|
||||
@@ -225,8 +226,30 @@ describe('SettingsManager sidecar storage', () => {
|
||||
expect(modal.classList.contains('show')).toBe(false);
|
||||
});
|
||||
|
||||
it('shows a deferred notice and skips migration when the user cancels', async () => {
|
||||
it('names the resolved destination in the confirm dialog', async () => {
|
||||
const manager = createManager();
|
||||
const { select } = appendSidecarControls();
|
||||
const modal = appendMigrationModal();
|
||||
state.global.settings = {
|
||||
sidecar_storage_mode: 'alongside',
|
||||
sidecar_storage_root: '/data/sidecars',
|
||||
};
|
||||
manager._loadedSidecarStorageMode = 'alongside';
|
||||
select.value = 'centralized';
|
||||
mockFetchOk();
|
||||
|
||||
const changePromise = manager.handleSidecarStorageModeChange();
|
||||
await vi.waitFor(() => expect(modal.classList.contains('show')).toBe(true));
|
||||
|
||||
const destination = modal.querySelector('[data-role="destination"]');
|
||||
expect(destination.textContent).toContain('/data/sidecars');
|
||||
expect(destination.style.display).toBe('block');
|
||||
|
||||
modal.querySelector('[data-action="cancel-sidecar-migration"]').click();
|
||||
await changePromise;
|
||||
});
|
||||
|
||||
it('shows a deferred notice and skips migration when the user cancels', async () => { const manager = createManager();
|
||||
const { select } = appendSidecarControls();
|
||||
const modal = appendMigrationModal();
|
||||
state.global.settings = { sidecar_storage_mode: 'centralized' };
|
||||
@@ -282,8 +305,7 @@ describe('SettingsManager sidecar storage', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('handleSidecarStoragePathChange', () => {
|
||||
it('offers root relocation when the path changes in centralized mode', async () => {
|
||||
describe('handleSidecarStoragePathChange', () => { it('offers root relocation when the path changes in centralized mode', async () => {
|
||||
const manager = createManager();
|
||||
const { pathInput } = appendSidecarControls();
|
||||
const modal = appendMigrationModal();
|
||||
@@ -337,4 +359,153 @@ describe('SettingsManager sidecar storage', () => {
|
||||
expect(showToast).toHaveBeenCalledWith('settings.sidecarStorage.migrationDeferred', {}, 'info');
|
||||
});
|
||||
});
|
||||
|
||||
describe('renderSidecarStorageInfo', () => {
|
||||
const appendStorageInfoElements = () => {
|
||||
const resolved = document.createElement('code');
|
||||
resolved.id = 'sidecarStorageResolvedPath';
|
||||
const warning = document.createElement('div');
|
||||
warning.id = 'sidecarStorageRepoWarning';
|
||||
warning.style.display = 'none';
|
||||
document.body.append(resolved, warning);
|
||||
return { resolved, warning };
|
||||
};
|
||||
|
||||
it('shows the resolved root and the repo warning when inside the install folder', () => {
|
||||
const manager = createManager();
|
||||
const { resolved, warning } = appendStorageInfoElements();
|
||||
state.global.settings = {
|
||||
sidecar_storage_root: '/repo/ComfyUI-Lora-Manager/sidecars',
|
||||
sidecar_storage_root_in_repo: true,
|
||||
};
|
||||
|
||||
manager.renderSidecarStorageInfo();
|
||||
|
||||
expect(resolved.textContent).toBe('/repo/ComfyUI-Lora-Manager/sidecars');
|
||||
expect(warning.style.display).toBe('block');
|
||||
});
|
||||
|
||||
it('hides the repo warning when the root lives outside the install folder', () => {
|
||||
const manager = createManager();
|
||||
const { resolved, warning } = appendStorageInfoElements();
|
||||
state.global.settings = {
|
||||
sidecar_storage_root: '/data/sidecars',
|
||||
sidecar_storage_root_in_repo: false,
|
||||
};
|
||||
|
||||
manager.renderSidecarStorageInfo();
|
||||
|
||||
expect(resolved.textContent).toBe('/data/sidecars');
|
||||
expect(warning.style.display).toBe('none');
|
||||
});
|
||||
});
|
||||
|
||||
describe('openSidecarStorageLocation', () => {
|
||||
it('posts to the open-location endpoint', async () => {
|
||||
const manager = createManager();
|
||||
mockFetchOk({ success: true });
|
||||
|
||||
await manager.openSidecarStorageLocation();
|
||||
|
||||
expect(global.fetch).toHaveBeenCalledWith('/api/lm/sidecars/open-location', { method: 'POST' });
|
||||
expect(showToast).toHaveBeenCalledWith('settings.sidecarStorage.openLocationSuccess', {}, 'success');
|
||||
});
|
||||
|
||||
it('copies the path to the clipboard in clipboard mode', async () => {
|
||||
const manager = createManager();
|
||||
mockFetchOk({ success: true, mode: 'clipboard', path: '/data/sidecars' });
|
||||
const writeText = vi.fn().mockResolvedValue();
|
||||
Object.defineProperty(navigator, 'clipboard', {
|
||||
value: { writeText },
|
||||
configurable: true,
|
||||
});
|
||||
|
||||
await manager.openSidecarStorageLocation();
|
||||
|
||||
expect(writeText).toHaveBeenCalledWith('/data/sidecars');
|
||||
expect(showToast).toHaveBeenCalledWith(
|
||||
'settings.sidecarStorage.openLocationCopied',
|
||||
{ path: '/data/sidecars' },
|
||||
'success'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('showSidecarMigrationResult', () => {
|
||||
const appendResultModal = () => {
|
||||
const modal = document.createElement('div');
|
||||
modal.id = 'sidecarMigrationResultModal';
|
||||
modal.innerHTML = `
|
||||
<h2 data-role="title"></h2>
|
||||
<p data-role="message"></p>
|
||||
<p data-role="destination" style="display:none"></p>
|
||||
<button data-action="open-sidecar-location" style="display:none"></button>
|
||||
<button data-action="close-sidecar-result"></button>`;
|
||||
document.body.appendChild(modal);
|
||||
return modal;
|
||||
};
|
||||
|
||||
it('renders counters and location, reloads only when closed', async () => {
|
||||
const manager = createManager();
|
||||
const modal = appendResultModal();
|
||||
mockFetchOk({ success: true });
|
||||
|
||||
manager.showSidecarMigrationResult({
|
||||
success: true,
|
||||
direction: 'to_centralized',
|
||||
moved: 12,
|
||||
models_moved: 5,
|
||||
skipped: 1,
|
||||
conflicts: 2,
|
||||
error_count: 0,
|
||||
sidecar_root: '/data/sidecars',
|
||||
});
|
||||
|
||||
expect(modal.classList.contains('show')).toBe(true);
|
||||
expect(modal.querySelector('[data-role="message"]').textContent).toContain('12');
|
||||
expect(modal.querySelector('[data-role="destination"]').textContent).toContain('/data/sidecars');
|
||||
expect(modal.querySelector('[data-action="open-sidecar-location"]').style.display).not.toBe('none');
|
||||
expect(resetAndReload).not.toHaveBeenCalled();
|
||||
|
||||
// "Open Folder" keeps the result modal open.
|
||||
modal.querySelector('[data-action="open-sidecar-location"]').click();
|
||||
await vi.waitFor(() => expect(global.fetch).toHaveBeenCalledWith(
|
||||
'/api/lm/sidecars/open-location',
|
||||
{ method: 'POST' }
|
||||
));
|
||||
expect(modal.classList.contains('show')).toBe(true);
|
||||
|
||||
modal.querySelector('[data-action="close-sidecar-result"]').click();
|
||||
expect(modal.classList.contains('show')).toBe(false);
|
||||
expect(resetAndReload).toHaveBeenCalledWith(true);
|
||||
});
|
||||
|
||||
it('hides the location row and open button when migrating back alongside', () => {
|
||||
const manager = createManager();
|
||||
const modal = appendResultModal();
|
||||
|
||||
manager.showSidecarMigrationResult({
|
||||
success: true,
|
||||
direction: 'to_alongside',
|
||||
moved: 3,
|
||||
models_moved: 3,
|
||||
skipped: 0,
|
||||
conflicts: 0,
|
||||
error_count: 0,
|
||||
sidecar_root: '/data/sidecars',
|
||||
});
|
||||
|
||||
expect(modal.querySelector('[data-role="destination"]').style.display).toBe('none');
|
||||
expect(modal.querySelector('[data-action="open-sidecar-location"]').style.display).toBe('none');
|
||||
});
|
||||
|
||||
it('falls back to toast plus reload when the modal is absent', () => {
|
||||
const manager = createManager();
|
||||
|
||||
manager.showSidecarMigrationResult({ success: true, direction: 'to_centralized' });
|
||||
|
||||
expect(showToast).toHaveBeenCalledWith('settings.sidecarStorage.migrateSuccess', {}, 'success');
|
||||
expect(resetAndReload).toHaveBeenCalledWith(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -31,6 +31,9 @@
|
||||
'language': 'en',
|
||||
'llm_api_key_set': False,
|
||||
'other_models_paths_available': False,
|
||||
'sidecar_storage_root': '/sidecars',
|
||||
'sidecar_storage_root_in_repo': False,
|
||||
'sidecar_storage_root_is_default': True,
|
||||
'standalone_mode': False,
|
||||
'theme': 'dark',
|
||||
}),
|
||||
|
||||
@@ -117,8 +117,20 @@ class TestSettingsHandlerSnapshots:
|
||||
"""Snapshot tests for SettingsHandler responses."""
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_settings_response_format(self, snapshot: SnapshotAssertion):
|
||||
async def test_get_settings_response_format(
|
||||
self, snapshot: SnapshotAssertion, monkeypatch: pytest.MonkeyPatch
|
||||
):
|
||||
"""Verify get_settings response format matches snapshot."""
|
||||
# Pin the resolved sidecar root: it derives from the machine-specific
|
||||
# settings directory, which would make the snapshot non-deterministic.
|
||||
monkeypatch.setattr(
|
||||
"py.routes.handlers.misc_handlers.describe_sidecar_root",
|
||||
lambda: {
|
||||
"root": "/sidecars",
|
||||
"is_default": True,
|
||||
"inside_repo": False,
|
||||
},
|
||||
)
|
||||
settings_service = DummySettings({
|
||||
"civitai_api_key": "test-key",
|
||||
"language": "en",
|
||||
|
||||
@@ -533,6 +533,58 @@ async def test_open_backup_location_uses_settings_directory(tmp_path, monkeypatc
|
||||
assert calls == [["xdg-open", str(backup_dir)]]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_open_sidecar_location_opens_configured_root(tmp_path, monkeypatch):
|
||||
from py.services.settings_manager import get_settings_manager
|
||||
|
||||
root = tmp_path / "sidecars"
|
||||
get_settings_manager().set("sidecar_storage_path", str(root))
|
||||
|
||||
handler = FileSystemHandler(settings_service=SimpleNamespace())
|
||||
|
||||
calls = []
|
||||
|
||||
def fake_popen(args):
|
||||
calls.append(args)
|
||||
return MagicMock()
|
||||
|
||||
monkeypatch.setattr(subprocess, "Popen", fake_popen)
|
||||
monkeypatch.setattr("py.routes.handlers.misc_handlers._is_docker", lambda: False)
|
||||
monkeypatch.setattr("py.routes.handlers.misc_handlers._is_wsl", lambda: False)
|
||||
|
||||
response = await handler.open_sidecar_location(FakeRequest()) # pyright: ignore[reportArgumentType]
|
||||
payload = _json_payload(response)
|
||||
|
||||
assert response.status == 200
|
||||
assert payload["success"] is True
|
||||
assert payload["path"] == str(root)
|
||||
# Created on demand so the button works before any migration ran.
|
||||
assert root.is_dir()
|
||||
assert calls == [["xdg-open", str(root)]]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_settings_includes_resolved_sidecar_root(tmp_path):
|
||||
from py.services.settings_manager import get_settings_manager
|
||||
|
||||
root = tmp_path / "sidecars-custom"
|
||||
get_settings_manager().set("sidecar_storage_path", str(root))
|
||||
|
||||
handler = SettingsHandler(
|
||||
settings_service=DummySettings(),
|
||||
metadata_provider_updater=noop_async,
|
||||
downloader_factory=dummy_downloader_factory,
|
||||
)
|
||||
|
||||
response = await handler.get_settings(FakeRequest()) # pyright: ignore[reportArgumentType]
|
||||
payload = _json_payload(response)
|
||||
|
||||
assert payload["success"] is True
|
||||
assert payload["settings"]["sidecar_storage_root"] == str(root)
|
||||
assert payload["settings"]["sidecar_storage_root_is_default"] is False
|
||||
assert payload["settings"]["sidecar_storage_root_in_repo"] is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_open_settings_location_headless_returns_clipboard_mode(tmp_path, monkeypatch):
|
||||
"""Without a GUI session xdg-open cannot work; the handler must hand the
|
||||
|
||||
@@ -97,19 +97,29 @@ class _FakeCache:
|
||||
|
||||
|
||||
class _FakeScanner:
|
||||
def __init__(self, raw_data: List[Dict[str, Any]]) -> None:
|
||||
def __init__(
|
||||
self, raw_data: List[Dict[str, Any]], excluded: List[str] | None = None
|
||||
) -> None:
|
||||
self._cache = _FakeCache(raw_data)
|
||||
self._excluded = list(excluded or [])
|
||||
self.persist_calls = 0
|
||||
|
||||
async def get_cached_data(self) -> _FakeCache:
|
||||
return self._cache
|
||||
|
||||
def get_excluded_models(self) -> List[str]:
|
||||
return list(self._excluded)
|
||||
|
||||
async def _persist_current_cache(self) -> None:
|
||||
self.persist_calls += 1
|
||||
|
||||
|
||||
def _make_use_case(model_paths: List[str]) -> SidecarMigrationUseCase:
|
||||
scanner = _FakeScanner([{"file_path": path} for path in model_paths])
|
||||
def _make_use_case(
|
||||
model_paths: List[str], excluded: List[str] | None = None
|
||||
) -> SidecarMigrationUseCase:
|
||||
scanner = _FakeScanner(
|
||||
[{"file_path": path} for path in model_paths], excluded=excluded
|
||||
)
|
||||
|
||||
async def scanner_factory() -> _FakeScanner:
|
||||
return scanner
|
||||
@@ -504,3 +514,40 @@ async def test_migrate_root_missing_old_tree_is_noop(
|
||||
|
||||
assert summary["success"] is True
|
||||
assert summary["moved"] == 0
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_migrate_covers_excluded_models(
|
||||
library_root: Path, sidecar_root: Path
|
||||
):
|
||||
"""Excluded models are absent from the cache; their sidecars still move.
|
||||
|
||||
Otherwise un-excluding a model later would leave the scanner looking for
|
||||
a sidecar in the new layout that was never migrated.
|
||||
"""
|
||||
|
||||
_set_mode("centralized")
|
||||
cached = _write_model(library_root, "cached")
|
||||
_write_sidecar(library_root, "cached", cached)
|
||||
(library_root / "cached.preview.webp").write_bytes(b"preview")
|
||||
excluded = _write_model(library_root / "hidden", "excluded")
|
||||
_write_sidecar(library_root / "hidden", "excluded", excluded, preview_ext=None)
|
||||
(library_root / "hidden" / "excluded.preview.webp").write_bytes(b"preview")
|
||||
|
||||
use_case = _make_use_case([str(cached)], excluded=[str(excluded)])
|
||||
summary = await use_case.migrate_to_centralized(force=True)
|
||||
|
||||
assert summary["success"] is True
|
||||
assert summary["models_total"] == 2
|
||||
assert summary["moved"] == 4
|
||||
assert summary["sidecar_root"] == str(sidecar_root)
|
||||
|
||||
mirror = _mirror_dir(library_root, sidecar_root, "hidden")
|
||||
assert (mirror / "excluded.metadata.json").exists()
|
||||
assert (mirror / "excluded.preview.webp").exists()
|
||||
assert not (library_root / "hidden" / "excluded.metadata.json").exists()
|
||||
assert not (library_root / "hidden" / "excluded.preview.webp").exists()
|
||||
|
||||
# The excluded model is not in the cache, so cache reconciliation is a
|
||||
# no-op for it and only the cached entry gets persisted.
|
||||
assert use_case._test_scanner.persist_calls == 1
|
||||
|
||||
@@ -283,3 +283,39 @@ class TestSettingsValidation:
|
||||
settings = get_settings_manager()
|
||||
settings.set("sidecar_storage_path", None)
|
||||
assert settings.get("sidecar_storage_path") == ""
|
||||
|
||||
|
||||
class TestDescribeSidecarRoot:
|
||||
def test_configured_root(self, tmp_path: Path):
|
||||
settings = get_settings_manager()
|
||||
root = tmp_path / "sidecars-custom"
|
||||
settings.set("sidecar_storage_path", str(root))
|
||||
|
||||
info = sidecar_paths.describe_sidecar_root()
|
||||
|
||||
assert info["root"] == os.path.abspath(str(root))
|
||||
assert info["is_default"] is False
|
||||
assert info["inside_repo"] is False
|
||||
|
||||
def test_default_root_marks_is_default(self):
|
||||
settings = get_settings_manager()
|
||||
settings.set("sidecar_storage_path", "")
|
||||
|
||||
info = sidecar_paths.describe_sidecar_root()
|
||||
|
||||
assert info["root"].endswith(os.sep + "sidecars")
|
||||
assert info["is_default"] is True
|
||||
|
||||
def test_inside_repo_detection(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch):
|
||||
monkeypatch.setattr(
|
||||
sidecar_paths, "_installation_root", lambda: str(tmp_path / "repo")
|
||||
)
|
||||
settings = get_settings_manager()
|
||||
settings.set(
|
||||
"sidecar_storage_path", str(tmp_path / "repo" / "sidecars")
|
||||
)
|
||||
|
||||
assert sidecar_paths.describe_sidecar_root()["inside_repo"] is True
|
||||
|
||||
settings.set("sidecar_storage_path", str(tmp_path / "elsewhere"))
|
||||
assert sidecar_paths.describe_sidecar_root()["inside_repo"] is False
|
||||
|
||||
Reference in New Issue
Block a user