Config migration silently drops user data sections

Fixes on record

Merged to main. A stamp counts the issues a fix is linked to.

• #51675 fix(docker): make boot-time config migration transactional (#51579)

  merged to main

  files: scripts/docker_config_migrate.py, tests/tools/test_docker_config_migrate.py

• #64606 fix(config): merge_existing mode stops partial saves dropping platforms (#62723)

  merged to main

  files: hermes_cli/config.py, hermes_cli/web_server.py, tests/hermes_cli/test_config.py

  Show the diff

  • hermes_cli/config.py

    @@ -5505,10 +5505,13 @@ def _persist_migration(config: Dict[str, Any]) -> None:
     
         Every migration step MUST route its write through this helper instead of
         calling ``save_config`` directly. It is a thin wrapper over
    -    ``save_config(config)`` (default-stripping ON); centralising the call makes
    -    the invariant impossible to regress one migration at a time. Correctness
    -    across seeds, non-default values, behaviour flips, and data transforms is
    -    verified by the migration parity tests.
    +    ``save_config(config)`` (default-stripping ON, no ``merge_existing``);
    +    centralising the call makes the invariant impossible to regress one
    +    migration at a time. Callers must pass the full raw config returned by
    +    ``read_raw_config()`` after in-place mutations (including key removals);
    +    deep-merging the on-disk file back in would resurrect keys the migration
    +    just deleted. Partial-save preservation for unrelated top-level sections
    +    belongs on ``save_config(..., merge_existing=True)``, not here.
         """
         save_config(config)
     
    @@ -6283,6 +6286,25 @@ def migrate_config(interactive: bool = True, quiet: bool = False) -> Dict[str, A
         return results
     
     
    +def _merge_partial_save(raw: dict, override: dict) -> dict:
    +    """Merge *override* over *raw* for partial ``save_config`` writes.
    +
    +    Top-level sections omitted from *override* are preserved from *raw*.
    +    Shared top-level dict sections are deep-merged so a caller can update one
    +    nested key without dropping sibling keys from disk. Intentional key
    +    removals within a section are not supported here — migration writes must
    +    route through ``_persist_migration`` with a full ``read_raw_config()`` dict
    +    instead.
    +    """
    +    result = copy.deepcopy(override)
    +    for key, value in raw.items():
    +        if key not in result:
    +            result[key] = copy.deepcopy(value)
    +        elif isinstance(result.get(key), dict) and isinstance(value, dict):
    +            result[key] = _deep_merge(value, result[key])
    +    return result
    +
    +
     def _deep_merge(base: dict, override: dict) -> dict:
         """Recursively merge *override* into *base*, preserving nested defaults.
     
    @@ -7165,6 +7187,7 @@ def save_config(
         *,
         strip_defaults: bool = True,
         preserve_keys: Optional[Set[Tuple[str, ...]]] = None,
    +    merge_existing: bool = False,
     ):
         """Save configuration to ~/.hermes/config.yaml.\n
     
    @@ -7173,6 +7196,12 @@ def save_config(
         before any normalisation).  This prevents config.yaml from being
         contaminated with schema defaults on every save, which makes future
         default changes invisible to users.
    +
    +    When ``merge_existing`` is True, the on-disk raw config is deep-merged
    +    under *config* before writing so partial callers (migration steps via
    +    ``_persist_migration``) cannot drop unrelated sections the caller omitted.
    +    Full-document replacement callers (dashboard raw YAML editor, callers that
    +    already deep-merge) must leave this False so intentional deletions survive.
         """
         with _CONFIG_LOCK:
             if is_managed():
    @@ -7207,11 +7236,17 @@ def save_config(
             explicit_raw_paths: Optional[Set[Tuple[str, ...]]] = (
                 _explicit_config_paths(_raw_for_paths) if _raw_for_paths else None
             )
    +        if merge_existing and _raw_for_paths:
    +            config = _merge_partial_save(_raw_for_paths, config)
             # ----------------------------------------------------------------
     
             current_normalized = _normalize_root_model_keys(_normalize_max_turns_config(config))
             normalized = current_normalized
    -        raw_existing = _normalize_root_model_keys(_normalize_max_turns_config(read_raw_config()))
    +        raw_existing = (
    +            _normalize_root_model_keys(_normalize_max_turns_config(_raw_for_paths))
    +            if _raw_for_paths
    +            else {}
    +        )
             if raw_existing:
                 normalized = _preserve_env_ref_templates(
                     normalized,
    @@ -7259,6 +7294,7 @@ def save_config(
                 extra_content="".join(parts) if parts else None,
             )
             _secure_file(config_path)
    +        _RAW_CONFIG_CACHE.pop(str(config_path), None)
             _LAST_EXPANDED_CONFIG_BY_PATH[str(config_path)] = copy.deepcopy(current_normalized)
     
     

  • hermes_cli/web_server.py

    @@ -14142,7 +14142,9 @@ async def update_config_raw(body: RawConfigUpdate, profile: Optional[str] = None
             if not isinstance(parsed, dict):
                 raise HTTPException(status_code=400, detail="YAML must be a mapping")
             with _profile_scope(body.profile or profile):
    -            save_config(parsed)
    +            # Full-document replacement: the editor owns the whole file; do not
    +            # merge omitted sections back from disk (#62723).
    +            save_config(parsed, merge_existing=False)
             return {"ok": True}
         except yaml.YAMLError as e:
             raise HTTPException(status_code=400, detail=f"Invalid YAML: {e}")

  • tests/hermes_cli/test_config.py

    @@ -1645,6 +1645,114 @@ def test_version_bump_keeps_config_lean(self, tmp_path, start_version):
             assert loaded["display"]["compact"] == DEFAULT_CONFIG["display"]["compact"]
     
     
    +class TestSaveConfigPartialWritePreservation:
    +    """Regression for #62723: partial migration writes must not drop unrelated sections."""
    +
    +    def test_merge_existing_preserves_platforms_on_partial_write(self, tmp_path):
    +        body = """_config_version: 30
    +model:
    +  default: deepseek-v4-pro
    +  provider: deepseek
    +agent:
    +  max_turns: 60
    +platforms:
    +  feishu:
    +    enabled: true
    +    extra:
    +      app_id: cli_xxx
    +      app_secret: xxx
    +feishu:
    +  require_mention: true
    +"""
    +        (tmp_path / "config.yaml").write_text(body, encoding="utf-8")
    +        with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}):
    +            save_config(
    +                {
    +                    "_config_version": 30,
    +                    "model": {"default": "deepseek-v4-pro", "provider": "deepseek"},
    +                    "agent": {"max_turns": 60, "verify_on_stop": False},
    +                },
    +                merge_existing=True,
    +            )
    +            raw = yaml.safe_load((tmp_path / "config.yaml").read_text(encoding="utf-8"))
    +
    +        assert raw["platforms"]["feishu"]["extra"]["app_id"] == "cli_xxx"
    +        assert raw["feishu"]["require_mention"] is True
    +        assert raw["agent"]["verify_on_stop"] is False
    +
    +    def test_partial_write_without_merge_drops_omitted_sections(self, tmp_path):
    +        """Full-replacement callers (raw YAML editor) rely on merge_existing=False."""
    +        body = """_config_version: 30
    +model:
    +  default: deepseek-v4-pro
    +  provider: deepseek
    +platforms:
    +  feishu:
    +    enabled: true
    +"""
    +        (tmp_path / "config.yaml").write_text(body, encoding="utf-8")
    +        with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}):
    +            save_config({"model": {"default": "other-model", "provider": "openrouter"}})
    +            raw = yaml.safe_load((tmp_path / "config.yaml").read_text(encoding="utf-8"))
    +
    +        assert raw["model"]["default"] == "other-model"
    +        assert "platforms" not in raw
    +
    +    def test_persist_migration_writes_full_read_raw_config(self, tmp_path):
    +        from hermes_cli.config import _persist_migration, read_raw_config
    +
    +        body = """_config_version: 30
    +model:
    +  default: deepseek-v4-pro
    +  provider: deepseek
    +agent:
    +  max_turns: 60
    +platforms:
    +  feishu:
    +    enabled: true
    +    extra:
    +      app_id: cli_xxx
    +      app_secret: xxx
    +"""
    +        (tmp_path / "config.yaml").write_text(body, encoding="utf-8")
    +        with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}):
    +            config = read_raw_config()
    +            config.setdefault("agent", {})["verify_on_stop"] = False
    +            config["_config_version"] = 32
    +            _persist_migration(config)
    +            raw = yaml.safe_load((tmp_path / "config.yaml").read_text(encoding="utf-8"))
    +
    +        assert raw["platforms"]["feishu"]["extra"]["app_id"] == "cli_xxx"
    +        assert raw["agent"]["verify_on_stop"] is False
    +        assert raw["agent"]["max_turns"] == 60
    +        assert raw["_config_version"] == 32
    +
    +    def test_v30_to_latest_migration_keeps_platforms(self, tmp_path):
    +        """End-to-end: reporter's v30 feishu profile survives version bump."""
    +        body = """_config_version: 30
    +model:
    +  default: deepseek-v4-pro
    +  provider: deepseek
    +agent:
    +  max_turns: 60
    +platforms:
    +  feishu:
    +    enabled: true
    +    extra:
    +      app_id: cli_xxx
    +      app_secret: xxx
    +feishu:
    +  require_mention: true
    +"""
    +        (tmp_path / "config.yaml").write_text(body, encoding="utf-8")
    +        with patch.dict(os.environ, {"HERMES_HOME": str(tmp_path)}):
    +            migrate_config(interactive=False, quiet=True)
    +            raw = yaml.safe_load((tmp_path / "config.yaml").read_text(encoding="utf-8"))
    +
    +        assert raw["platforms"]["feishu"]["extra"]["app_id"] == "cli_xxx"
    +        assert raw["feishu"]["require_mention"] is True
    +
    +
     class TestVerifyOnStopMigration:
         """v30 → v31: switch verify_on_stop OFF once, preserving explicit choices."""