fix(config): stop the migration ladder rewriting unversioned configs
A config.yaml without _config_version reads as v0 and is exempt from the support floor, so the first `hermes update`, profile clone, `hermes doctor --fix` or docker boot ran every one-time migration step on it. Installers seed config.yaml from cli-config.yaml.example, which had no version, and targeted writers (`hermes config set`, /personality, the TUI/Desktop config writers) never stamp one, so this is the normal state of --skip-setup, non-TTY and Desktop (--non-interactive) installs. The value- and absence-based steps then reset the personality, raised the delegation caps, turned verify_on_stop off, shortened the curator windows, dropped model_catalog.ttl_hours and enabled plugins the user had installed but never enabled. - A config with no _config_version now gets only the steps keyed on a legacy key or identifier (LEGACY_KEY_STEPS), then the stamp. - cli-config.yaml.example carries _config_version, so every seeded config (install.sh, install.ps1, docker/stage2-hook.sh, doctor --fix) starts at the current schema. - docker_config_migrate.py no longer refuses a version-less volume with the "predates version 12" warning; like migrate_config() it migrates and stamps it. (cherry picked from commit 97ba11e07009f633662b0c7fa8701aa5b441bd22)
This commit is contained in:
committed by
kshitij
parent
43d3d4e851
commit
a88bef98b2
@@ -4,6 +4,11 @@
|
||||
# This file configures CLI behavior; only documented secret environment
|
||||
# variables in .env take precedence over their corresponding settings.
|
||||
|
||||
# Schema version of this file. The installers copy it to seed config.yaml, and
|
||||
# `hermes update` uses it to know which one-time migrations the file already
|
||||
# has. Hermes manages it: do not copy it into another config.
|
||||
_config_version: 46
|
||||
|
||||
# =============================================================================
|
||||
# Database Configuration
|
||||
# =============================================================================
|
||||
|
||||
@@ -1310,17 +1310,14 @@ def migrate_config(interactive: bool = True, quiet: bool = False) -> Dict[str, A
|
||||
|
||||
# Auto-migration support floor (v12): an EXPLICIT on-disk ``_config_version`` below the
|
||||
# floor is NOT migrated and NOT rewritten — surface a message and leave the file untouched
|
||||
# (deep-merge supplies defaults at read time). A config with NO version key is a fresh
|
||||
# minimal config, not an ancient install: it gets the normal ladder and a version stamp.
|
||||
# (deep-merge supplies defaults at read time). A config with NO version key is not an
|
||||
# ancient install: it gets only the legacy-key steps and a version stamp.
|
||||
# Missing/unparseable files never trip the floor gate.
|
||||
# Imported lazily because the steps call back into this module.
|
||||
from hermes_cli.config_migrations import (
|
||||
SUPPORT_FLOOR_VERSION, run_migrations, support_floor_message)
|
||||
SUPPORT_FLOOR_VERSION, has_version_stamp, run_migrations, support_floor_message)
|
||||
|
||||
try:
|
||||
has_explicit_version = "_config_version" in read_user_config_raw()
|
||||
except Exception:
|
||||
has_explicit_version = False
|
||||
has_explicit_version = has_version_stamp()
|
||||
floor_refused = (
|
||||
has_explicit_version and current_ver < SUPPORT_FLOOR_VERSION and current_ver < latest_ver)
|
||||
if floor_refused:
|
||||
@@ -1331,7 +1328,7 @@ def migrate_config(interactive: bool = True, quiet: bool = False) -> Dict[str, A
|
||||
if not quiet:
|
||||
print(f" ⚠ {msg}")
|
||||
else:
|
||||
run_migrations(current_ver, results, quiet)
|
||||
run_migrations(current_ver, results, quiet, unversioned=not has_explicit_version)
|
||||
|
||||
_disable_suspicious_mcp_servers(results, quiet)
|
||||
_warn_invalid_platform_toolsets(results, quiet)
|
||||
|
||||
@@ -36,6 +36,16 @@ def support_floor_message() -> str:
|
||||
"after reviewing the changelog.")
|
||||
|
||||
|
||||
def has_version_stamp() -> bool:
|
||||
"""Whether config.yaml carries a ``_config_version`` key. ``check_config_version()`` reads a
|
||||
missing one as 0, but such a file is never-stamped current-schema content, not an ancient
|
||||
install: the floor must not refuse it and only :data:`LEGACY_KEY_STEPS` may run on it."""
|
||||
try:
|
||||
return "_config_version" in _cfg().read_user_config_raw()
|
||||
except Exception:
|
||||
return False
|
||||
|
||||
|
||||
def _cfg():
|
||||
"""Return the live ``hermes_cli.config`` module (lazy, cycle-free, monkeypatch-friendly)."""
|
||||
from hermes_cli import config
|
||||
@@ -755,15 +765,26 @@ MIGRATIONS: Tuple[Tuple[int, Callable[[Dict[str, Any], bool], None]], ...] = (
|
||||
(46, _migrate_to_46),
|
||||
)
|
||||
|
||||
#: Steps triggered by a legacy key or identifier (a renamed or retired key, a removed plugin or
|
||||
#: toolset, the plugin-era SOUL.md section): they carry its setting to where the runtime reads it
|
||||
#: or drop what nothing reads, which is right however old the file is. A config.yaml with no
|
||||
#: ``_config_version`` is current-schema content that was never stamped (installers seed it from
|
||||
#: cli-config.yaml.example; targeted writers never stamp), so it gets only these: every other step
|
||||
#: decides by a value or an absence that, in such a file, is the user's own choice. v13 is left
|
||||
#: out: it clears OPENAI_MODEL from .env, a generic name Hermes never reads but the user's tools may.
|
||||
LEGACY_KEY_STEPS = frozenset({12, 14, 16, 17, 29, 33, 38, 39, 41, 42, 43, 46})
|
||||
|
||||
def run_migrations(current_ver: int, results: Dict[str, Any], quiet: bool) -> None:
|
||||
"""Apply every registered migration whose target version exceeds *current_ver*.
|
||||
|
||||
def run_migrations(
|
||||
current_ver: int, results: Dict[str, Any], quiet: bool, *, unversioned: bool = False) -> None:
|
||||
"""Apply every registered migration whose target version exceeds *current_ver*; a config
|
||||
with no ``_config_version`` (*unversioned*) gets only :data:`LEGACY_KEY_STEPS`.
|
||||
|
||||
*current_ver* is the on-disk schema version captured ONCE before any step runs and does not
|
||||
advance between steps — each step is gated on the same initial value.
|
||||
"""
|
||||
for target_ver, migration_fn in MIGRATIONS:
|
||||
if current_ver < target_ver:
|
||||
if current_ver < target_ver and (target_ver in LEGACY_KEY_STEPS or not unversioned):
|
||||
try:
|
||||
migration_fn(results, quiet)
|
||||
except Exception as exc:
|
||||
|
||||
@@ -16,6 +16,7 @@ from hermes_cli.config import (
|
||||
from hermes_cli.config_backups import backup_config, list_config_backups
|
||||
from hermes_cli.config_migrations import (
|
||||
SUPPORT_FLOOR_VERSION,
|
||||
has_version_stamp,
|
||||
support_floor_message,
|
||||
)
|
||||
from utils import env_var_enabled
|
||||
@@ -54,8 +55,10 @@ def main() -> int:
|
||||
# Below the auto-migration support floor: migrate_config() refuses (and
|
||||
# leaves the file untouched), so don't run the backup/verify dance that
|
||||
# would raise "did not advance config version" and block the boot.
|
||||
# Warn-and-continue matches the CLI's fail-safe posture.
|
||||
if current_ver < SUPPORT_FLOOR_VERSION:
|
||||
# Warn-and-continue matches the CLI's fail-safe posture. A config with no
|
||||
# _config_version (a volume seeded from the template) is not below the
|
||||
# floor: migrate_config() stamps it.
|
||||
if current_ver < SUPPORT_FLOOR_VERSION and has_version_stamp():
|
||||
print(
|
||||
f"[config-migrate] WARNING: {support_floor_message()}",
|
||||
file=sys.stderr,
|
||||
|
||||
78
tests/hermes_cli/test_config_unversioned_migration.py
Normal file
78
tests/hermes_cli/test_config_unversioned_migration.py
Normal file
@@ -0,0 +1,78 @@
|
||||
"""A config.yaml without ``_config_version`` is current-schema content that was never stamped:
|
||||
the installers seed it from cli-config.yaml.example and targeted writers (``hermes config set``,
|
||||
/personality) never stamp. The one-time migration ladder must not treat it as a v0 install —
|
||||
its value- and absence-based steps would overwrite what the user chose."""
|
||||
|
||||
import shutil
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
import yaml
|
||||
|
||||
TEMPLATE = Path(__file__).resolve().parents[2] / "cli-config.yaml.example"
|
||||
|
||||
USER_CHOICES = {
|
||||
"delegation.max_concurrent_children": "3",
|
||||
"delegation.max_iterations": "50",
|
||||
"display.background_process_notifications": "all",
|
||||
"agent.verify_on_stop": "true",
|
||||
"curator.stale_after_days": "30",
|
||||
"curator.archive_after_days": "90",
|
||||
"model_catalog.ttl_hours": "24",
|
||||
}
|
||||
WATCHED = [*USER_CHOICES, "display.personality", "plugins.enabled"]
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def hermes_home(tmp_path, monkeypatch):
|
||||
home = tmp_path / ".hermes"
|
||||
home.mkdir()
|
||||
monkeypatch.setattr(Path, "home", lambda: tmp_path) # sibling-profile scan stays in tmp
|
||||
monkeypatch.setenv("HERMES_HOME", str(home))
|
||||
return home
|
||||
|
||||
|
||||
def _raw(home: Path) -> dict:
|
||||
return yaml.safe_load((home / "config.yaml").read_text(encoding="utf-8")) or {}
|
||||
|
||||
|
||||
def _at(raw: dict, dotted: str):
|
||||
for part in dotted.split("."):
|
||||
raw = raw.get(part) if isinstance(raw, dict) else None
|
||||
return raw
|
||||
|
||||
|
||||
def test_update_keeps_user_values_of_an_unversioned_config_and_migrates_legacy_keys(hermes_home):
|
||||
from hermes_cli.config import DEFAULT_CONFIG, set_config_value
|
||||
from hermes_cli.personality import persist_personality
|
||||
from hermes_cli.update_cmd import _check_and_apply_config_migration
|
||||
|
||||
# Seeded before the template carried a stamp; early-2026 templates shipped this retired key.
|
||||
(hermes_home / "config.yaml").write_text(
|
||||
"compression:\n summary_model: google/gemini-3-flash-preview\n", encoding="utf-8")
|
||||
# Installed with `hermes plugins install` and never enabled.
|
||||
plugin = hermes_home / "plugins" / "notes-helper"
|
||||
plugin.mkdir(parents=True)
|
||||
(plugin / "plugin.yaml").write_text("name: notes-helper\nversion: 0.1.0\n", encoding="utf-8")
|
||||
assert persist_personality("kawaii")
|
||||
for key, value in USER_CHOICES.items():
|
||||
set_config_value(key, value)
|
||||
before = _raw(hermes_home)
|
||||
|
||||
_check_and_apply_config_migration()
|
||||
|
||||
after = _raw(hermes_home)
|
||||
assert {k: _at(after, k) for k in WATCHED} == {k: _at(before, k) for k in WATCHED}
|
||||
assert "summary_model" not in after["compression"]
|
||||
assert _at(after, "auxiliary.compression.model") == _at(before, "compression.summary_model")
|
||||
assert after["_config_version"] == DEFAULT_CONFIG["_config_version"]
|
||||
|
||||
|
||||
def test_config_seeded_from_the_template_reads_as_current(hermes_home):
|
||||
"""install.sh, install.ps1, docker/stage2-hook.sh and `hermes doctor --fix` copy the template."""
|
||||
from hermes_cli.config import check_config_version
|
||||
|
||||
shutil.copy(TEMPLATE, hermes_home / "config.yaml")
|
||||
|
||||
current, latest = check_config_version(raise_on_parse_error=True)
|
||||
assert current == latest
|
||||
@@ -101,19 +101,20 @@ def test_docker_config_migrate_skips_below_floor_config_untouched(tmp_path: Path
|
||||
assert not list(tmp_path.glob("*.bak-*"))
|
||||
|
||||
|
||||
def test_docker_config_migrate_skips_unversioned_config_untouched(tmp_path: Path) -> None:
|
||||
"""Unversioned configs coerce to version 0 — below the floor, so refused."""
|
||||
def test_docker_config_migrate_stamps_unversioned_config(tmp_path: Path) -> None:
|
||||
"""A config with no _config_version (a template-seeded volume) is not below the floor, as in
|
||||
migrate_config(): it is stamped and keeps its values."""
|
||||
config_path = tmp_path / "config.yaml"
|
||||
original = yaml.safe_dump({"model": {"default": "m", "provider": "openrouter"}})
|
||||
config_path.write_text(original, encoding="utf-8")
|
||||
model = {"default": "m", "provider": "openrouter"}
|
||||
config_path.write_text(yaml.safe_dump({"model": model}), encoding="utf-8")
|
||||
|
||||
proc = _run_migration(tmp_path)
|
||||
|
||||
assert proc.returncode == 0, proc.stderr
|
||||
assert "Migrating config schema" not in proc.stdout
|
||||
assert "can no longer be auto-migrated" in proc.stderr
|
||||
assert config_path.read_text(encoding="utf-8") == original
|
||||
assert not list(tmp_path.glob("*.bak-*"))
|
||||
assert "can no longer be auto-migrated" not in proc.stderr
|
||||
raw = yaml.safe_load(config_path.read_text(encoding="utf-8"))
|
||||
assert raw["_config_version"] == DEFAULT_CONFIG["_config_version"]
|
||||
assert raw["model"] == model
|
||||
|
||||
|
||||
def test_docker_config_migrate_does_not_rewrite_invalid_yaml(tmp_path: Path) -> None:
|
||||
|
||||
Reference in New Issue
Block a user