From 33a61f9b72c837357a27f54a80fc783d10c4beef Mon Sep 17 00:00:00 2001 From: Jedrzej Kosinski Date: Mon, 15 Jun 2026 01:29:39 -0700 Subject: [PATCH] Adopt: keep legacy extra_model_paths.yaml install-scoped Desktop adoption now treats the legacy `extra_model_paths.yaml` (the file ComfyUI auto-loads from the source dir and the Storage tab shows read-only) as install-local, never merging its paths into the global shared model dirs. This avoids one migrated install's arbitrary/external paths leaking into every other install. - `computeModelsDirsToCarry` / `carryLegacySettings` again feed only `extra_models_config.yaml` into the global `modelsDirs` (the user's main models stay shared, as before). - New `preserveInstallExtraModelPaths` ensures the legacy `extra_model_paths.yaml` lands in the adopted install's source dir so ComfyUI loads it for that install and the Storage tab reports it: left as-is when the pre-swap copy already carried it, recovered from the reused basePath in the git-clone fallback, no-op otherwise. - Surfaced as `has_install_extra_model_paths_yaml` in adopt telemetry. The Storage tab already renders the install's extra_model_paths.yaml (buildExtraModelPathsView), so no UI change is needed. Amp-Thread-ID: https://ampcode.com/threads/T-019ebfcb-e545-73d8-a33d-52580e41588a Co-authored-by: Amp --- src/main/lib/desktopAdopt.test.ts | 58 +++++++++++++++++++++++++++++ src/main/lib/desktopAdopt.ts | 62 ++++++++++++++++++++++++++++++- 2 files changed, 118 insertions(+), 2 deletions(-) diff --git a/src/main/lib/desktopAdopt.test.ts b/src/main/lib/desktopAdopt.test.ts index f1daf8c15..3dbda52a6 100644 --- a/src/main/lib/desktopAdopt.test.ts +++ b/src/main/lib/desktopAdopt.test.ts @@ -142,6 +142,7 @@ import { parseExtraModelsSections, deriveLaunchArgs, computeModelsDirsToCarry, + preserveInstallExtraModelPaths, getLegacyVenvUvPath, type AdoptTools, type AdoptDeps, @@ -675,6 +676,63 @@ describe('computeModelsDirsToCarry', () => { expect(result).not.toContain(path.resolve(path.join(yamlBase, 'models'))) expect(result).toContain(path.resolve(yamlBase)) }) + + it('does NOT carry extra_model_paths.yaml dirs into the global list (install-scoped)', () => { + // Only extra_models_config.yaml feeds the global modelsDirs; an + // extra_model_paths.yaml is handled install-locally elsewhere. + const basePath = path.join(tmpRoot, 'primary') + fs.mkdirSync(path.join(basePath, 'models'), { recursive: true }) + const result = computeModelsDirsToCarry(basePath, null, []) + expect(result).toEqual([path.resolve(path.join(basePath, 'models'))]) + }) +}) + +describe('preserveInstallExtraModelPaths', () => { + let tmpRoot: string + beforeEach(() => { + tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'preserveExtra-')) + }) + afterEach(() => { + fs.rmSync(tmpRoot, { recursive: true, force: true }) + }) + + it('keeps the source tree file as-is (pre-swap copy) and never clobbers it', () => { + const basePath = path.join(tmpRoot, 'base') + const destSource = path.join(tmpRoot, 'install', 'ComfyUI') + fs.mkdirSync(destSource, { recursive: true }) + fs.mkdirSync(basePath, { recursive: true }) + fs.writeFileSync(path.join(destSource, 'extra_model_paths.yaml'), 'from_source: {}\n') + // A different basePath copy must NOT overwrite the authoritative source one. + fs.writeFileSync(path.join(basePath, 'extra_model_paths.yaml'), 'from_base: {}\n') + const ok = preserveInstallExtraModelPaths(basePath, destSource, () => {}) + expect(ok).toBe(true) + expect(fs.readFileSync(path.join(destSource, 'extra_model_paths.yaml'), 'utf-8')).toContain( + 'from_source' + ) + }) + + it('recovers the file from basePath when the sourced tree lacks one (clone fallback)', () => { + const basePath = path.join(tmpRoot, 'base') + const destSource = path.join(tmpRoot, 'install', 'ComfyUI') + fs.mkdirSync(destSource, { recursive: true }) + fs.mkdirSync(basePath, { recursive: true }) + fs.writeFileSync(path.join(basePath, 'extra_model_paths.yaml'), 'from_base: {}\n') + const ok = preserveInstallExtraModelPaths(basePath, destSource, () => {}) + expect(ok).toBe(true) + expect(fs.readFileSync(path.join(destSource, 'extra_model_paths.yaml'), 'utf-8')).toContain( + 'from_base' + ) + }) + + it('is a no-op (returns false) when no legacy extra_model_paths.yaml exists', () => { + const basePath = path.join(tmpRoot, 'base') + const destSource = path.join(tmpRoot, 'install', 'ComfyUI') + fs.mkdirSync(destSource, { recursive: true }) + fs.mkdirSync(basePath, { recursive: true }) + const ok = preserveInstallExtraModelPaths(basePath, destSource, () => {}) + expect(ok).toBe(false) + expect(fs.existsSync(path.join(destSource, 'extra_model_paths.yaml'))).toBe(false) + }) }) describe('adoptDesktopInstall', () => { diff --git a/src/main/lib/desktopAdopt.ts b/src/main/lib/desktopAdopt.ts index 04460a0a4..d31d3ef33 100644 --- a/src/main/lib/desktopAdopt.ts +++ b/src/main/lib/desktopAdopt.ts @@ -54,6 +54,11 @@ const ADOPT_INSTALL_NAME = 'ComfyUI' const COMFY_SETTINGS_FILE = 'comfy.settings.json' const DESKTOP_CONFIG_FILE = 'config.json' const EXTRA_MODELS_YAML = 'extra_models_config.yaml' +// The other model-paths file: ComfyUI auto-loads it from the source dir and +// the launcher displays it read-only. Carried alongside EXTRA_MODELS_YAML so +// its dirs survive into v2's modelsDirs (not just the adopted ComfyUI's own +// search), including in the git-clone source fallback where it isn't copied. +const EXTRA_MODEL_PATHS_YAML = 'extra_model_paths.yaml' const WINDOW_FILE = 'window.json' const VENV_VALIDATE_TIMEOUT_MS = 30_000 @@ -429,10 +434,16 @@ function isDangerousRoot(resolved: string): boolean { /** * Cross-install model dirs to register in `settings.modelsDirs` for the - * adopted record. The goal for migrations is to carry EVERY real model - * directory the legacy `extra_models_config.yaml` referenced so nothing + * adopted record. The goal for migrations is to carry the real model + * directories the legacy `extra_models_config.yaml` referenced so nothing * shows as "missing" after adoption. * + * Only the desktop-managed `extra_models_config.yaml` feeds the GLOBAL + * shared list here. Any `extra_model_paths.yaml` is deliberately NOT carried + * globally — its (often install-specific / external) paths stay scoped to the + * adopted install via `preserveInstallExtraModelPaths`, so they don't leak + * into every other install. See that function for the install-local handling. + * * For each section we register a models ROOT (a dir whose type subfolders * `buildYaml` scans): * - Always `/models` (the primary install), first. @@ -523,6 +534,43 @@ function isUnderRoot(child: string, root: string): boolean { return rel === '' || (!rel.startsWith('..') && !path.isAbsolute(rel)) } +/** + * Keep the legacy `extra_model_paths.yaml` as an INSTALL-LOCAL file in the + * adopted source tree, so ComfyUI auto-loads it for this install only (it reads + * `/extra_model_paths.yaml` from `dirname(main.py)`) and the Storage + * tab reports it via `buildExtraModelPathsView`. Deliberately install-scoped — + * its arbitrary/external paths are never merged into the global shared model + * dirs, so they can't leak into other installs; users who want them elsewhere + * can open the file and copy paths out manually. + * + * No-op when the sourced tree already has one (a pre-swap copy carries the + * legacy file verbatim — never clobber it). Otherwise (e.g. the git-clone + * fallback, whose fresh checkout ships only `extra_model_paths.yaml.example`) + * recover a copy from the reused legacy `basePath` when present. Best-effort: + * a missing source or copy failure is logged and swallowed. + * + * Returns true when a file is present in the install afterward (carried or + * recovered), for telemetry/visibility. + */ +export function preserveInstallExtraModelPaths( + basePath: string, + destSource: string, + sendOutput: (t: string) => void +): boolean { + const dest = path.join(destSource, EXTRA_MODEL_PATHS_YAML) + if (fs.existsSync(dest)) return true // pre-swap copy already carried it + const legacy = path.join(basePath, EXTRA_MODEL_PATHS_YAML) + try { + if (!fs.existsSync(legacy)) return false + fs.copyFileSync(legacy, dest) + sendOutput('Preserved legacy extra_model_paths.yaml as an install-local config.\n') + return true + } catch (err) { + sendOutput(`Warning: could not preserve extra_model_paths.yaml: ${(err as Error).message}\n`) + return false + } +} + /** * Best-effort copy of legacy userData files into a timestamped backup folder. * Logged on failure but never throws so adoption can continue. @@ -1150,6 +1198,15 @@ async function runAdoption( // 'retry' loops. } + // Keep the legacy extra_model_paths.yaml as an install-local config (loaded + // by this install's ComfyUI and shown read-only in the Storage tab). Scoped + // to this install — never merged into the global shared model dirs. + const hasInstallExtraModelPaths = preserveInstallExtraModelPaths( + info.basePath, + destSource, + sendOutput + ) + // Adoption preserves the user's existing ComfyUI checkout as-is — it is // not auto-updated to latest stable. A "frozen" install must stay on // whatever version the user was running; ComfyUI updates are opt-in per @@ -1331,6 +1388,7 @@ async function runAdoption( adopted_source_mode: sourceMode, has_venv: info.hasVenv, has_extra_models_yaml: fs.existsSync(path.join(info.configDir, EXTRA_MODELS_YAML)), + has_install_extra_model_paths_yaml: hasInstallExtraModelPaths, models_dir_count: carry.addedModelsDirs.length, carried_keys: carry.carriedKeys, carry_skipped_keys: carry.carrySkippedKeys,