From 574dfbbe55aa4251de9ea40e7c5220317f432984 Mon Sep 17 00:00:00 2001 From: Will Miao Date: Thu, 27 Aug 2026 00:03:50 +0800 Subject: [PATCH] feat(settings): add explicit settings dir override for sandboxed runs Add LORA_MANAGER_SETTINGS_DIR env var and standalone --settings-path to pin the settings location (settings.json, cache/, wildcards/, backups/, logs/, stats/) to an arbitrary directory. The override takes precedence over portable mode and the platform user config dir, and skips legacy migration, so sandboxed dev/E2E runs no longer need to write settings.json in the repo root or collide with the real instance. standalone.py pre-scans argv for --settings-path at import time because the settings location is resolved before main() parses arguments. SettingsManager portable-switch migration is a no-op while the directory is pinned. Update the lora-manager-e2e skill (prefer --settings-path sandboxing; start_server.py passes it through) and the lora-manager-runtime-context skill (document precedence; inspect script honors the override). --- .agents/skills/lora-manager-e2e/SKILL.md | 48 ++++++---- .../lora-manager-e2e/scripts/start_server.py | 21 +++++ .../lora-manager-runtime-context/SKILL.md | 15 +++- .../scripts/inspect_runtime_context.py | 17 ++++ py/services/settings_manager.py | 16 +++- py/utils/settings_paths.py | 88 +++++++++++++++++-- standalone.py | 48 +++++++++- tests/services/test_settings_manager.py | 30 +++++++ tests/test_standalone_settings.py | 43 +++++++++ tests/utils/test_settings_paths.py | 87 +++++++++++++++++- 10 files changed, 383 insertions(+), 30 deletions(-) diff --git a/.agents/skills/lora-manager-e2e/SKILL.md b/.agents/skills/lora-manager-e2e/SKILL.md index e190d6b0..43d2b048 100644 --- a/.agents/skills/lora-manager-e2e/SKILL.md +++ b/.agents/skills/lora-manager-e2e/SKILL.md @@ -11,13 +11,14 @@ This skill provides workflows and utilities for end-to-end testing of LoRa Manag - **`{PORT}`**: The server port. The default candidate is `8188`, but **`8188` is commonly occupied by a live ComfyUI process** and MUST NOT be assumed to be free. Always check availability first (see [Port Selection](#port-selection)) and use a free port (e.g. `8199`) for the E2E run. Substitute the actual port for every `{PORT}` in the commands below. - **``**: The repository/worktree root. Always run commands from the repo or worktree root; never assume a specific absolute path (paths such as `/home//...` differ per machine). The E2E scripts resolve the project root themselves, but fixture/settings paths are relative to ``. +- **``**: The sandboxed explicit settings directory passed via `--settings-path` (see [SANDBOX](#sandbox-mandatory)); substitute the actual path (e.g. `/tmp/opencode/-e2e/settings`) for every `{PATH}` in commands below that target the sandbox config. ## SANDBOX (MANDATORY) > **Read this section before running anything.** Every E2E run MUST target a throwaway sandbox, never the real user data. A fresh subagent that skips this section WILL permanently mutate real user recipes. -1. **Portable settings**: create `/settings.json` (gitignored) with `"use_portable_settings": true` plus sandboxed `folder_paths` (lora/checkpoint roots) and `recipes_path`. This keeps the configuration inside the repo instead of the real user config dir (`~/.config/ComfyUI-LoRA-Manager/settings.json`). -2. **Sandboxed paths**: point `folder_paths` / `recipes_path` / `example_images_path` at disposable dirs — e.g. under `/tmp/opencode/-e2e/` (or worktree-local dirs). NEVER point the E2E at the real library (`~/models/...`), real recipe dir, or real settings. +1. **Explicit settings directory (preferred)**: launch the standalone server with `--settings-path /settings` (or set `LORA_MANAGER_SETTINGS_DIR`). This pins ALL runtime data — `settings.json`, `cache/`, `wildcards/`, `backups/`, `logs/`, `stats/` — under that directory, independent of portable mode and of the real user config dir. **Do NOT** write `/settings.json` for sandboxing: the repo folder is usually the real ComfyUI plugin folder, and a portable `settings.json` there is read by the real instance — exactly the conflict this E2E must avoid. +2. **Sandboxed paths**: point `folder_paths` / `recipes_path` / `example_images_path` at disposable dirs under the sandbox — e.g. `/models/loras`, `/recipes`. NEVER point the E2E at the real library (`~/models/...`), real recipe dir, or real settings. 3. **Never touch the real config**: the real user config at `~/.config/ComfyUI-LoRA-Manager/settings.json` and the real recipe dir must remain byte-identical before and after the run. 4. **Record real-data protection proof** before starting and after finishing: ```bash @@ -27,13 +28,14 @@ This skill provides workflows and utilities for end-to-end testing of LoRa Manag find ~/models/recipes -name '*.recipe.json' -newermt "$(date -Iseconds)" | head # expect empty after run # AFTER: record again, then diff the two snapshots. Any change = the run leaked into real data. ``` - Also confirm `/git status` stays clean for `settings.json`/`cache/` (both are gitignored). + Also confirm `/git status` stays clean (`settings.json`/`cache/` are gitignored and must not be created by the run). -### Portable Settings Example +### Sandbox Settings (via `--settings-path`) + +Write this file as `/settings/settings.json` — `` in the commands below: ```json { - "use_portable_settings": true, "folder_paths": { "loras": ["/tmp/opencode/-e2e/models/loras"], "checkpoints": ["/tmp/opencode/-e2e/models/checkpoints"], @@ -45,7 +47,7 @@ This skill provides workflows and utilities for end-to-end testing of LoRa Manag } ``` -The scanner computes and persists model hashes during the library scan, so the sandbox model dirs just need the model files + `.metadata.json` sidecars (see [Fixture + Fresh-State Guidance](#fixture--fresh-state-guidance)). +The scanner computes and persists model hashes during the library scan, so the sandbox model dirs just need the model files + `.metadata.json` sidecars (see [Fixture + Fresh-State Guidance](#fixture--fresh-state-guidance)). With `--settings-path`, all derived data lands under `` (`cache/`, `backups/`, `logs/`, `stats/`, `wildcards/`), and NO `cache/` appears in ``. ## Time Budgets & Abort Guidance @@ -90,9 +92,10 @@ ss -tlnp | grep ':8188' || echo "8188 is free" ```bash cd # ALWAYS run from the repo/worktree root +mkdir -p /tmp/opencode/-e2e/settings mkdir -p /tmp/opencode/-e2e/models/{loras,checkpoints} mkdir -p /tmp/opencode/-e2e/{recipes,example_images,recipes-before} -# write /settings.json per the portable-settings example above +# write /settings/settings.json per the sandbox-settings example above # record real-data protection proof (see SANDBOX section) ``` @@ -106,16 +109,18 @@ If `{PORT}` is occupied by an unrelated process, pick a free one and use it ever ### 3. Start LoRa Manager Standalone (detached) -The standalone server **dies with the shell unless launched fully detached** — a plain background `&` from the bash tool is killed when the tool call returns. Launch via the helper script: +The standalone server **dies with the shell unless launched fully detached** — a plain background `&` from the bash tool is killed when the tool call returns. Launch via the helper script (note `--settings-path`): ```bash -python .agents/skills/lora-manager-e2e/scripts/start_server.py --port {PORT} --wait --timeout 30 --detach +python .agents/skills/lora-manager-e2e/scripts/start_server.py \ + --port {PORT} --settings-path /tmp/opencode/-e2e/settings \ + --wait --timeout 30 --detach ``` Or manually (equivalent detached form): ```bash -setsid nohup python standalone.py --port {PORT} --host 127.0.0.1 < /dev/null \ +setsid nohup python standalone.py --port {PORT} --settings-path /tmp/opencode/-e2e/settings --host 127.0.0.1 < /dev/null \ >> /tmp/opencode/-e2e/server.log 2>&1 & echo "started" # record the printed/pidfile PID for cleanup ``` @@ -169,7 +174,9 @@ snapshot = take_snapshot() # Stop current server (if running), start with new configuration. # --restart only kills the E2E server this script started before (via its pidfile); # it refuses to blindly kill unrelated processes on the port. -python .agents/skills/lora-manager-e2e/scripts/start_server.py --port {PORT} --restart --wait --detach +python .agents/skills/lora-manager-e2e/scripts/start_server.py \ + --port {PORT} --settings-path /tmp/opencode/-e2e/settings \ + --restart --wait --detach # Wait and refresh browser navigate_page(type="reload", ignoreCache=True) @@ -250,17 +257,21 @@ Each entry point (global / per-recipe / selection-bulk) must start from the same ```bash # 1. Reset fixtures to the before-state snapshot (copy back from recipes-before/) cp /tmp/opencode/-e2e/recipes-before/*.recipe.json /tmp/opencode/-e2e/recipes/ -# 2. Clear the recipe/FTS caches so the stale in-memory/library state is gone -rm -f /cache/recipe/*.sqlite -rm -rf /cache/fts/* +# 2. Clear the recipe/FTS caches so the stale in-memory/library state is gone. +# With --settings-path these live under the sandbox settings dir, NOT /cache. +rm -f /tmp/opencode/-e2e/settings/cache/recipe/*.sqlite +rm -rf /tmp/opencode/-e2e/settings/cache/fts/* # 3. Restart the server (fresh process, fresh scan) -python .agents/skills/lora-manager-e2e/scripts/start_server.py --port {PORT} --restart --wait --timeout 30 --detach +python .agents/skills/lora-manager-e2e/scripts/start_server.py \ + --port {PORT} --settings-path /tmp/opencode/-e2e/settings \ + --restart --wait --timeout 30 --detach # 4. Re-verify server listening + reload the browser page ``` ## Server Lifecycle -- **Detached launch is mandatory**: the standalone server dies with the shell unless launched via `setsid` (or the helper script's `--detach`). Use `setsid nohup python standalone.py --port {PORT} --host 127.0.0.1 ... < /dev/null &`. +- **Detached launch is mandatory**: the standalone server dies with the shell unless launched via `setsid` (or the helper script's `--detach`). Use `setsid nohup python standalone.py --port {PORT} --settings-path /settings --host 127.0.0.1 ... < /dev/null &`. +- **Always pass `--settings-path`** pointing at the sandbox settings dir — this is what keeps the run fully sandboxed (see [SANDBOX](#sandbox-mandatory)). - **Verify with `ss -tlnp`** after every (re)start; do not proceed on a blind "server starting" message. - **Never kill pre-existing processes** — only kill the E2E server PID you started (`start_server.py --restart` kills only PIDs it manages via its pidfile). The live ComfyUI or a stale QA Chrome must never be killed as part of cleanup unless explicitly identified as such (see Chrome troubleshooting). - **Record your PID for cleanup**: note the PID printed/pidfile, and stop exactly that PID at the end (`kill `, then confirm with `ss -tlnp` that `{PORT}` is released). @@ -306,11 +317,12 @@ Testing the rematch-cancel path E2E requires a run long enough to cancel mid-fli Starts or restarts the LoRa Manager standalone server for E2E testing. ```bash -python scripts/start_server.py [--port PORT] [--restart] [--wait] [--timeout SECONDS] [--detach] +python scripts/start_server.py [--port PORT] [--settings-path DIR] [--restart] [--wait] [--timeout SECONDS] [--detach] ``` Options: - `--port`: Server port (default: 8188). The script exits early with a clear message if the port is already in use by an unrelated process. +- `--settings-path`: Explicit sandbox settings directory passed through to `standalone.py` (equivalent to `LORA_MANAGER_SETTINGS_DIR`). Creates the directory if needed and refuses to start if the path exists as a file. **Use this for every sandboxed E2E run.** - `--restart`: Kill the E2E server this script previously managed (tracked via `/tmp/lora-manager-e2e-server-{PORT}.pid`) before starting. If unrelated processes still hold the port after that, the script reports them and aborts instead of killing them. - `--wait`: Wait for the server to be ready before exiting. - `--timeout`: Readiness wait timeout in seconds (default: 30). @@ -369,5 +381,5 @@ results = performance_stop_trace() Always ensure proper cleanup after tests: 1. Stop the standalone server: `kill ` (only the PID you started), then confirm `ss -tlnp | grep ':{PORT}'` is empty. 2. Close browser pages (keep at least one open). -3. Remove the sandbox: `rm -rf /tmp/opencode/-e2e` and `/settings.json` + `/cache` (both gitignored). +3. Remove the sandbox: `rm -rf /tmp/opencode/-e2e`. Verify `` has NOT gained a `settings.json` or `cache/` (with `--settings-path` they never appear there). 4. Re-run the real-data protection check from the SANDBOX section and record the result in your evidence. diff --git a/.agents/skills/lora-manager-e2e/scripts/start_server.py b/.agents/skills/lora-manager-e2e/scripts/start_server.py index 8c1a17bf..d5b2da45 100755 --- a/.agents/skills/lora-manager-e2e/scripts/start_server.py +++ b/.agents/skills/lora-manager-e2e/scripts/start_server.py @@ -211,6 +211,17 @@ def main() -> int: help="Launch the server fully detached (setsid-style) so it survives shell " "death. REQUIRED for E2E: a plain background process dies with the shell", ) + parser.add_argument( + "--settings-path", + type=str, + default=None, + metavar="DIR", + help="Explicit settings directory passed to standalone.py (--settings-path, " + "equivalent to LORA_MANAGER_SETTINGS_DIR). settings.json, cache/, " + "wildcards/, backups/, logs/, stats/ all live under this directory instead " + "of the project root or the user config dir. Recommended for sandboxed E2E " + "so the real instance and the repo stay untouched", + ) args = parser.parse_args() @@ -283,6 +294,16 @@ def main() -> int: "--port", str(args.port), ] + if args.settings_path: + settings_dir = os.path.abspath(os.path.expanduser(args.settings_path)) + if os.path.exists(settings_dir) and not os.path.isdir(settings_dir): + print( + f"ERROR: --settings-path '{settings_dir}' exists but is not a directory." + ) + return 2 + os.makedirs(settings_dir, exist_ok=True) + cmd.extend(["--settings-path", settings_dir]) + print(f"Settings directory: {settings_dir}") if args.detach: # Fully detached launch: new session (setsid), no controlling terminal, diff --git a/.agents/skills/lora-manager-runtime-context/SKILL.md b/.agents/skills/lora-manager-runtime-context/SKILL.md index e3b7cdf7..2752be13 100644 --- a/.agents/skills/lora-manager-runtime-context/SKILL.md +++ b/.agents/skills/lora-manager-runtime-context/SKILL.md @@ -9,7 +9,10 @@ description: Inspect ComfyUI LoRA Manager runtime configuration and local diagno - Treat runtime state as local user data. Prefer read-only inspection unless the user explicitly asks for mutation. - Never print secret-like settings values. Redact keys containing `key`, `token`, `secret`, `password`, `auth`, or `credential`, including `civitai_api_key`. -- Resolve paths from the runtime configuration before guessing. In this environment the settings file is normally `/home/miao/.config/ComfyUI-LoRA-Manager/settings.json`, but portable settings can override this through the repository `settings.json`. +- Resolve paths from the runtime configuration before guessing. Settings-directory precedence (highest first): + 1. **Explicit override** — env `LORA_MANAGER_SETTINGS_DIR` or standalone `--settings-path` (also accepted by the inspect script as `--settings-path DIR`). Pins EVERYTHING (`settings.json`, `cache/`, `wildcards/`, `backups/`, `logs/`, `stats/`) under the given directory; bypasses portable mode and the user config dir. Common when inspecting a sandboxed/E2E instance. + 2. **Portable** — repository `/settings.json` with `"use_portable_settings": true` (or `LORA_MANAGER_PORTABLE=1`): settings dir = ``. + 3. **Default** — `~/.config/ComfyUI-LoRA-Manager` on this machine (`platformdirs.user_config_dir("ComfyUI-LoRA-Manager", appauthor=False)`). - Use the active library when selecting per-library caches and paths. Read `active_library` from settings; fall back to `default` if missing. - Normalize and expand `~` before comparing paths. Symlinks are common in this repo. @@ -32,9 +35,17 @@ python .agents/skills/lora-manager-runtime-context/scripts/inspect_runtime_conte python .agents/skills/lora-manager-runtime-context/scripts/inspect_runtime_context.py sqlite --db /path/to/cache.sqlite --limit 3 ``` +To inspect a sandboxed/E2E instance that pins its settings directory: + +```bash +# --settings-path DIR (or LORA_MANAGER_SETTINGS_DIR) works with every subcommand: +python .agents/skills/lora-manager-runtime-context/scripts/inspect_runtime_context.py \ + --settings-path /tmp/opencode/-e2e/settings summary +``` + ## Runtime Path Rules -- Settings directory: use `py/utils/settings_paths.py`. Default platform path is `platformdirs.user_config_dir("ComfyUI-LoRA-Manager", appauthor=False)`. +- Settings directory: resolve via `py/utils/settings_paths.py` — `get_settings_dir()` honors the `LORA_MANAGER_SETTINGS_DIR` / programmatic override first, then portable mode, then `platformdirs.user_config_dir("ComfyUI-LoRA-Manager", appauthor=False)`. The inspect script mirrors this precedence in `resolve_settings_path()`. - Settings file: `/settings.json`. - Cache root: `/cache`. - Canonical cache files: diff --git a/.agents/skills/lora-manager-runtime-context/scripts/inspect_runtime_context.py b/.agents/skills/lora-manager-runtime-context/scripts/inspect_runtime_context.py index b573fcce..16a29a53 100755 --- a/.agents/skills/lora-manager-runtime-context/scripts/inspect_runtime_context.py +++ b/.agents/skills/lora-manager-runtime-context/scripts/inspect_runtime_context.py @@ -14,6 +14,7 @@ from typing import Any SECRET_PATTERN = re.compile(r"(key|token|secret|password|auth|credential)", re.IGNORECASE) APP_NAME = "ComfyUI-LoRA-Manager" +SETTINGS_DIR_ENV = "LORA_MANAGER_SETTINGS_DIR" CACHE_SQLITE = { "model": ("model", "{library}.sqlite"), "recipe": ("recipe", "{library}.sqlite"), @@ -30,6 +31,15 @@ CACHE_JSON = { def main() -> int: parser = argparse.ArgumentParser(description="Inspect LoRA Manager runtime state read-only.") + parser.add_argument( + "--settings-path", + type=str, + default=None, + metavar="DIR", + help="Explicit settings directory (same as LORA_MANAGER_SETTINGS_DIR / " + "standalone --settings-path). Overrides portable mode and the default " + "user config dir.", + ) subparsers = parser.add_subparsers(dest="command", required=True) subparsers.add_parser("summary", help="Print redacted settings and resolved paths.") @@ -44,6 +54,8 @@ def main() -> int: sqlite_parser.add_argument("--limit", type=int, default=3, help="Rows to sample from each user table.") args = parser.parse_args() + if args.settings_path: + os.environ[SETTINGS_DIR_ENV] = args.settings_path context = build_context() if args.command == "summary": @@ -78,6 +90,11 @@ def build_context() -> dict[str, Any]: def resolve_settings_path() -> Path: + # Explicit override: LORA_MANAGER_SETTINGS_DIR env or --settings-path. + explicit = os.environ.get(SETTINGS_DIR_ENV) + if explicit: + return Path(explicit).expanduser() / "settings.json" + repo_root = find_repo_root() portable = repo_root / "settings.json" if portable.exists(): diff --git a/py/services/settings_manager.py b/py/services/settings_manager.py index 86278fe8..2b33fc63 100644 --- a/py/services/settings_manager.py +++ b/py/services/settings_manager.py @@ -34,6 +34,8 @@ from ..utils.settings_paths import ( APP_NAME, ensure_settings_file, get_legacy_settings_path, + get_settings_dir_override, + is_settings_dir_pinned, ) from ..utils.tag_priorities import ( PriorityTagEntry, @@ -156,7 +158,10 @@ class SettingsManager: self._check_environment_variables() self._collect_configuration_warnings() - if os.environ.get("LORA_MANAGER_PORTABLE", "0") == "1": + if ( + os.environ.get("LORA_MANAGER_PORTABLE", "0") == "1" + and not is_settings_dir_pinned() + ): if not self.settings.get("use_portable_settings"): self.settings["use_portable_settings"] = True self._save_settings() @@ -1641,6 +1646,15 @@ class SettingsManager: def _prepare_portable_switch(self, use_portable: bool) -> None: """Prepare switching the settings storage location.""" + if is_settings_dir_pinned(): + logger.info( + "Portable-mode switch ignored: settings directory is pinned via " + "%s/--settings-path (%s)", + "LORA_MANAGER_SETTINGS_DIR", + get_settings_dir_override(), + ) + return + legacy_path = get_legacy_settings_path() user_dir = self._get_user_config_directory() user_settings_path = os.path.join(user_dir, "settings.json") diff --git a/py/utils/settings_paths.py b/py/utils/settings_paths.py index 3ddfc01a..fd0b93bd 100644 --- a/py/utils/settings_paths.py +++ b/py/utils/settings_paths.py @@ -13,6 +13,15 @@ from platformdirs import user_config_dir APP_NAME = "ComfyUI-LoRA-Manager" _LM_PORTABLE_ENV = "LORA_MANAGER_PORTABLE" + +# Explicit settings-directory override. Setting this (env var, or standalone's +# ``--settings-path`` which publishes it) pins the settings location: settings.json, +# cache/, wildcards/, backups/, logs/, stats/ all resolve under this directory, +# bypassing portable mode and the platform user config dir. Useful for sandboxed +# development/E2E runs that must not touch the real user data or the project root. +SETTINGS_DIR_ENV = "LORA_MANAGER_SETTINGS_DIR" +_settings_dir_override: Optional[str] = None + _LOGGER = logging.getLogger(__name__) @@ -22,6 +31,51 @@ def get_project_root() -> str: return os.path.dirname(os.path.dirname(os.path.dirname(__file__))) +def _normalize_settings_dir(path: str) -> str: + """Expand ``~`` and absolutize a user-supplied settings directory.""" + + return os.path.abspath(os.path.expanduser(path)) + + +def set_settings_dir_override(path: Optional[str]) -> Optional[str]: + """Set or clear the programmatic settings-directory override. + + Args: + path: Absolute/relative directory to pin, or ``None`` to clear the + override. ``~`` is expanded and the path absolutized. + + Returns: + The previous override value (``None`` when none was active). + """ + + global _settings_dir_override + previous = _settings_dir_override + _settings_dir_override = ( + _normalize_settings_dir(path) if path else None + ) + return previous + + +def get_settings_dir_override() -> Optional[str]: + """Return the active explicit settings-directory override, if any. + + The ``LORA_MANAGER_SETTINGS_DIR`` environment variable takes precedence over + the programmatic override so that standalone's ``--settings-path`` (which + publishes itself through the environment) wins over embedded callers. + """ + + env_path = os.environ.get(SETTINGS_DIR_ENV) + if env_path: + return _normalize_settings_dir(env_path) + return _settings_dir_override + + +def is_settings_dir_pinned() -> bool: + """Return ``True`` when an explicit settings-directory override is active.""" + + return get_settings_dir_override() is not None + + def get_legacy_settings_path() -> str: """Return the legacy location of ``settings.json`` within the project tree.""" @@ -31,6 +85,11 @@ def get_legacy_settings_path() -> str: def get_settings_dir(create: bool = True) -> str: """Return the user configuration directory for the application. + An explicit override (``LORA_MANAGER_SETTINGS_DIR`` or + :func:`set_settings_dir_override`) takes precedence. Otherwise the portable + project-root ``settings.json`` is used when enabled, falling back to the + platform-specific user configuration directory. + Args: create: Whether to create the directory if it does not already exist. @@ -38,11 +97,15 @@ def get_settings_dir(create: bool = True) -> str: The absolute path to the user configuration directory. """ - legacy_path = get_legacy_settings_path() - if _should_use_portable_settings(legacy_path, _LOGGER): - config_dir = os.path.dirname(legacy_path) + override = get_settings_dir_override() + if override: + config_dir = override else: - config_dir = user_config_dir(APP_NAME, appauthor=False) + legacy_path = get_legacy_settings_path() + if _should_use_portable_settings(legacy_path, _LOGGER): + config_dir = os.path.dirname(legacy_path) + else: + config_dir = user_config_dir(APP_NAME, appauthor=False) if create and config_dir: os.makedirs(config_dir, exist_ok=True) @@ -58,9 +121,14 @@ def get_settings_file_path(create_dir: bool = True) -> str: def ensure_settings_file(logger: Optional[logging.Logger] = None) -> str: """Ensure the settings file resides in the user configuration directory. - If a legacy ``settings.json`` is detected in the project root it is migrated to - the platform-specific user configuration folder. The caller receives the path - to the settings file irrespective of whether a migration was needed. + An explicit override (``LORA_MANAGER_SETTINGS_DIR`` or + :func:`set_settings_dir_override`) pins the settings file to + ``/settings.json`` and skips legacy migration entirely. + + Otherwise, if a legacy ``settings.json`` is detected in the project root it is + migrated to the platform-specific user configuration folder. The caller + receives the path to the settings file irrespective of whether a migration was + needed. Args: logger: Optional logger used for migration messages. Falls back to a @@ -71,6 +139,12 @@ def ensure_settings_file(logger: Optional[logging.Logger] = None) -> str: """ logger = logger or _LOGGER + + override = get_settings_dir_override() + if override: + os.makedirs(override, exist_ok=True) + return os.path.join(override, "settings.json") + legacy_path = get_legacy_settings_path() if _should_use_portable_settings(legacy_path, logger): diff --git a/standalone.py b/standalone.py index 7e3576d1..9afeed45 100644 --- a/standalone.py +++ b/standalone.py @@ -8,12 +8,36 @@ from typing import Any, cast sys.path.insert(0, os.path.dirname(os.path.abspath(__file__))) from py.middleware.cache_middleware import cache_control from py.middleware.error_middleware import api_json_error -from py.utils.settings_paths import ensure_settings_file +from py.utils.settings_paths import SETTINGS_DIR_ENV, ensure_settings_file # Set environment variable to indicate standalone mode os.environ["LORA_MANAGER_STANDALONE"] = "1" +def _apply_settings_dir_from_argv(argv=None): + """Apply ``--settings-path`` from argv before any settings resolution runs. + + Standalone resolves the settings location at import time (session logging and + the settings manager run before ``main()`` parses arguments), so pre-scan + argv and publish the explicit directory through ``LORA_MANAGER_SETTINGS_DIR``, + which ``py.utils.settings_paths`` honors in both standalone and plugin modes. + + Args: + argv: Argument list to scan; defaults to ``sys.argv[1:]``. + """ + args = list(sys.argv[1:] if argv is None else argv) + for index, arg in enumerate(args): + if arg == "--settings-path" and index + 1 < len(args): + os.environ[SETTINGS_DIR_ENV] = args[index + 1] + return + if arg.startswith("--settings-path="): + os.environ[SETTINGS_DIR_ENV] = arg.split("=", 1)[1] + return + + +_apply_settings_dir_from_argv() + + # Create mock modules for py/nodes directory - add this before any other imports def mock_nodes_directory(): """Create mock modules for all Python files in the py/nodes directory""" @@ -395,6 +419,16 @@ def parse_args(): # help="Additional paths to LoRA model directories (optional if settings.json has paths)") # parser.add_argument("--checkpoints", type=str, nargs="+", # help="Additional paths to checkpoint model directories (optional if settings.json has paths)") + parser.add_argument( + "--settings-path", + type=str, + default=None, + metavar="DIR", + help="Explicit settings directory: settings.json, cache/, wildcards/, " + "backups/, logs/, stats/ all live under this directory. Overrides portable " + "mode and the default user config dir. Equivalent to the " + "LORA_MANAGER_SETTINGS_DIR environment variable.", + ) parser.add_argument( "--log-level", type=str, @@ -414,6 +448,18 @@ async def main(): """Main entry point for standalone mode""" args = parse_args() + # Normalize and validate the explicit settings directory (the pre-import + # argv scan already applied it; re-derive so --settings-path wins over any + # pre-existing LORA_MANAGER_SETTINGS_DIR and is canonicalized the same way). + if args.settings_path: + settings_dir = os.path.abspath(os.path.expanduser(args.settings_path)) + if os.path.exists(settings_dir) and not os.path.isdir(settings_dir): + logger.error( + "--settings-path '%s' exists but is not a directory.", settings_dir + ) + return + os.environ[SETTINGS_DIR_ENV] = settings_dir + # Set log level (verbose flag overrides to DEBUG) log_level = "DEBUG" if args.verbose else args.log_level logging.getLogger().setLevel(getattr(logging, log_level)) diff --git a/tests/services/test_settings_manager.py b/tests/services/test_settings_manager.py index 3597b189..980de6e8 100644 --- a/tests/services/test_settings_manager.py +++ b/tests/services/test_settings_manager.py @@ -293,6 +293,36 @@ def test_switching_back_to_user_config_moves_subdirectories(tmp_path, monkeypatc ) == "project_wildcard" +def test_portable_switch_ignored_when_settings_dir_pinned(tmp_path, monkeypatch): + """An explicit settings dir (--settings-path) must never trigger the + portable-mode directory migration between project root and user config.""" + project_root, user_dir, user_settings = _setup_storage_paths(tmp_path, monkeypatch) + _populate_settings_dir(user_dir) + + custom_dir = tmp_path / "custom_settings" + custom_dir.mkdir() + monkeypatch.setattr( + "py.services.settings_manager.ensure_settings_file", + lambda logger=None: str(custom_dir / "settings.json"), + ) + settings_paths.set_settings_dir_override(str(custom_dir)) + try: + manager = SettingsManager() + manager.settings_file = str(custom_dir / "settings.json") + + manager.set("use_portable_settings", True) + + # Settings file stays pinned; no directories are migrated anywhere and + # the settings file is not mirrored to the user config dir. + assert manager.settings_file == str(custom_dir / "settings.json") + assert not (project_root / "cache").exists() + assert not (project_root / "backups").exists() + assert not (project_root / "settings.json").exists() + assert not user_settings.exists() + finally: + settings_paths.set_settings_dir_override(None) + + def test_download_path_template_parses_json_string(manager): templates = {"lora": "{author}", "checkpoint": "{author}", "embedding": "{author}"} manager.settings["download_path_templates"] = json.dumps(templates) diff --git a/tests/test_standalone_settings.py b/tests/test_standalone_settings.py index a14c423b..90c7a480 100644 --- a/tests/test_standalone_settings.py +++ b/tests/test_standalone_settings.py @@ -1,5 +1,7 @@ import importlib import json +import os +import sys from pathlib import Path from typing import Any @@ -112,3 +114,44 @@ def test_validate_settings_logs_warnings(tmp_path, monkeypatch, caplog): messages = [record.message for record in caplog.records] assert any("Standalone mode is using fallback configuration values." in message for message in messages) + + +@pytest.mark.no_settings_dir_isolation +def test_explicit_settings_dir_env_used_by_manager(tmp_path, monkeypatch): + """LORA_MANAGER_SETTINGS_DIR pins the settings file for the manager.""" + custom_dir = tmp_path / "custom" + monkeypatch.setenv("LORA_MANAGER_SETTINGS_DIR", str(custom_dir)) + reset_settings_manager() + + manager = get_settings_manager() + + assert settings_paths.is_settings_dir_pinned() + assert Path(manager.settings_file) == custom_dir / "settings.json" + assert settings_paths.get_settings_dir() == str(custom_dir) + + +def test_apply_settings_dir_from_argv(): + """standalone's argv pre-scan publishes --settings-path into the env.""" + import standalone + + # The helper writes to os.environ directly; manage the variable manually so + # monkeypatch's undo stack cannot restore a stale value after the test. + previous = os.environ.pop("LORA_MANAGER_SETTINGS_DIR", None) + try: + standalone._apply_settings_dir_from_argv( + ["--port", "8199", "--settings-path", "/tmp/xyz-e2e-settings"] + ) + assert os.environ["LORA_MANAGER_SETTINGS_DIR"] == "/tmp/xyz-e2e-settings" + + os.environ.pop("LORA_MANAGER_SETTINGS_DIR", None) + standalone._apply_settings_dir_from_argv(["--settings-path=/tmp/abc-e2e"]) + assert os.environ["LORA_MANAGER_SETTINGS_DIR"] == "/tmp/abc-e2e" + + os.environ.pop("LORA_MANAGER_SETTINGS_DIR", None) + standalone._apply_settings_dir_from_argv(["--port", "8199"]) + assert "LORA_MANAGER_SETTINGS_DIR" not in os.environ + finally: + if previous is None: + os.environ.pop("LORA_MANAGER_SETTINGS_DIR", None) + else: + os.environ["LORA_MANAGER_SETTINGS_DIR"] = previous diff --git a/tests/utils/test_settings_paths.py b/tests/utils/test_settings_paths.py index a82c93de..332d7e65 100644 --- a/tests/utils/test_settings_paths.py +++ b/tests/utils/test_settings_paths.py @@ -6,7 +6,26 @@ import os import pytest -from py.utils.settings_paths import _should_use_portable_settings +from py.utils.settings_paths import ( + SETTINGS_DIR_ENV, + _should_use_portable_settings, + ensure_settings_file, + get_settings_dir, + get_settings_dir_override, + is_settings_dir_pinned, + set_settings_dir_override, +) + + +def _redirect_paths(tmp_path, monkeypatch): + """Pin project root and user config dir resolution to temp paths.""" + monkeypatch.setattr( + "py.utils.settings_paths.get_project_root", lambda: str(tmp_path / "repo") + ) + monkeypatch.setattr( + "py.utils.settings_paths.user_config_dir", + lambda *args, **kwargs: str(tmp_path / "user_config"), + ) class TestShouldUsePortableSettings: @@ -54,3 +73,69 @@ class TestShouldUsePortableSettings: mp.setenv("LORA_MANAGER_PORTABLE", "1") result = _should_use_portable_settings(str(missing), logging.getLogger()) assert result is True + + +class TestExplicitSettingsDirOverride: + """Tests for the LORA_MANAGER_SETTINGS_DIR / --settings-path override.""" + + def test_env_override_wins_over_portable_and_user_config(self, tmp_path, monkeypatch): + _redirect_paths(tmp_path, monkeypatch) + repo = tmp_path / "repo" + repo.mkdir() + (repo / "settings.json").write_text( + json.dumps({"use_portable_settings": True}), encoding="utf-8" + ) + custom_dir = tmp_path / "custom" / "e2e" + + monkeypatch.setenv(SETTINGS_DIR_ENV, str(custom_dir)) + monkeypatch.delenv("LORA_MANAGER_PORTABLE", raising=False) + + assert is_settings_dir_pinned() + target = get_settings_dir(create=True) + assert target == str(custom_dir.resolve()) + assert target == os.path.abspath(str(custom_dir)) + assert custom_dir.is_dir() + + # The override is independent of the effective use_portable flag. + with pytest.MonkeyPatch.context() as mp: + mp.setenv("LORA_MANAGER_PORTABLE", "1") + assert get_settings_dir(create=False) == os.path.abspath(str(custom_dir)) + + def test_env_override_normalizes_tilde(self, monkeypatch): + monkeypatch.setenv(SETTINGS_DIR_ENV, "~/lm-e2e-settings") + override = get_settings_dir_override() + assert override == os.path.abspath(os.path.expanduser("~/lm-e2e-settings")) + + def test_ensure_settings_file_pins_path_and_skips_migration(self, tmp_path, monkeypatch): + _redirect_paths(tmp_path, monkeypatch) + repo = tmp_path / "repo" + repo.mkdir() + legacy = repo / "settings.json" + legacy.write_text(json.dumps({"language": "ja"}), encoding="utf-8") + custom_dir = tmp_path / "custom" + + monkeypatch.setenv(SETTINGS_DIR_ENV, str(custom_dir)) + + settings_file = ensure_settings_file() + assert settings_file == os.path.join(os.path.abspath(str(custom_dir)), "settings.json") + assert os.path.isdir(str(custom_dir)) + # The legacy (project-root) file must NOT be migrated into the custom dir. + assert legacy.exists() + assert not (custom_dir / "settings.json").exists() + assert not (tmp_path / "user_config" / "settings.json").exists() + + def test_programmatic_override_and_clear(self, tmp_path, monkeypatch): + _redirect_paths(tmp_path, monkeypatch) + monkeypatch.delenv(SETTINGS_DIR_ENV, raising=False) + + custom_dir = tmp_path / "prog" + previous = set_settings_dir_override(str(custom_dir)) + assert previous is None + assert is_settings_dir_pinned() + assert get_settings_dir(create=False) == os.path.abspath(str(custom_dir)) + + previous = set_settings_dir_override(None) + assert previous == os.path.abspath(str(custom_dir)) + assert not is_settings_dir_pinned() + # Falls back to the (redirected) platform user config dir. + assert get_settings_dir(create=False) == str(tmp_path / "user_config")