Skip to content

fix(config): route every migration write through one default-stripping chokepoint - #55715

Merged
kshitijk4poor merged 1 commit into
NousResearch:mainfrom
kshitijk4poor:fix/config-migration-no-default-expansion
Jun 30, 2026
Merged

fix(config): route every migration write through one default-stripping chokepoint#55715
kshitijk4poor merged 1 commit into
NousResearch:mainfrom
kshitijk4poor:fix/config-migration-no-default-expansion

Conversation

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Summary

hermes update / hermes -p no longer rewrites a hand-curated config.yaml into a near-full DEFAULT_CONFIG dump on a version bump. All config migration writes now flow through a single default-stripping chokepoint, so only values that differ from the schema default (plus explicit user-data removals/renames) ever land on disk — defaults merge transparently at read time via load_config().

Root cause: migrate_config() had ~16 independent save_config() call sites. Each migration author decided ad hoc whether to materialise a value, and many persisted pure schema defaults with strip_defaults=False, bypassing the default-stripping protection added in #27539/#53132. Because there was no single rule, every prior fix patched individual sites and the bug-class kept returning. Writing a default to disk is not just bloat — it shadows future default changes (the on-disk value wins the merge forever).

Changes

  • hermes_cli/config.py:
    • New _persist_migration(config) chokepoint — a thin wrapper over save_config(config) (default-stripping ON) documenting the migration write invariant.
    • All 17 migration write sites (including the version-bump finalizer) route through it; strip_defaults=False is gone from the migration path.
    • The catch-all get_missing_config_fields() finalizer no longer injects every missing default to disk — it only surfaces the list for the informational "N new config option(s) available" display and persists the version bump.
  • tests/hermes_cli/test_config.py:
    • TestMigrationWriteInvariant — AST guard asserting migrate_config() makes no direct save_config() call (regression-proof), plus a full-range v1→latest leanness test.
    • Two change-detector tests that froze the on-disk representation of default-valued keys (write_approval, interim_assistant_messages) rewritten to assert the effective value via load_config() (behavior contract, not snapshot).

The invariant (enforced in one place)

A migration may persist only values that differ from the current schema default, plus explicit removals/renames of user data. Verified empirically for every category:

  • pure-default seeds (timezone, curator/auxiliary.curator blocks, interim flag, curator.consolidate, empty plugins.enabled) → stripped, merged in at read time;
  • non-default values (write_approval=True, ttl_hours=1) → preserved via save_config's explicit-raw-path preservation;
  • behavior flips (agent.verify_on_stop=False, whose schema default is still "auto") → preserved because False != "auto";
  • data transforms (custom_providers→providers, stt.model relocation, write_mode→write_approval, compression.summary_* removal, MCP-disable) → persist their removals/renames.

An explicitly user-set non-default value (e.g. matrix.require_mention: false) is preserved across the bump.

Validation

Before After
lean v1→latest migration ~567 B (defaults dump) ~196 B (user config + version bump)
explicit non-default value preserved preserved
schema defaults written to disk merged at read time, absent from disk

scripts/run_tests.sh tests/hermes_cli/test_config.py tests/hermes_cli/test_setup.py → 148 passed. Migration-adjacent suites (profiles, curator, migrate_xai, apply_profile_override) → 196 passed. ruff clean.

Relates to the config-bloat reports addressed piecemeal in #27354 / #40821 / #27539 / #53132; this makes the fix structural so the bug-class can't recur.

…g chokepoint

A single 'hermes update' / 'hermes -p' could rewrite a hand-curated config.yaml
into a near-full DEFAULT_CONFIG dump (the 'you blow up my profile config on one
tweak' reports). Root cause: migrate_config() had ~16 independent save_config()
call sites, each author deciding ad hoc whether to materialise a value, and many
persisted pure schema defaults with strip_defaults=False. Defaults already merge
transparently at read time via load_config(), so writing them is pure bloat that
also shadows future default changes (see save_config's docstring).

Architectural fix (not a per-site patch): introduce a single _persist_migration()
chokepoint that enforces one invariant — a migration may persist only values that
DIFFER from the current schema default, plus explicit removals/renames of user
data; pure defaults are never written. Every migration write (all 17 sites incl.
the version-bump finalizer) now routes through it. The invariant is mechanically
correct for all cases and verified empirically:
  - pure-default seeds (timezone='', curator/auxiliary.curator blocks, interim
    flag, curator.consolidate=False, empty plugins.enabled) are stripped → merged
    in at read time;
  - non-default values (write_approval=True, model_catalog.ttl_hours=1) preserved
    via explicit-raw-path preservation;
  - behaviour flips (agent.verify_on_stop=False, schema default still 'auto')
    preserved because False != 'auto';
  - data transforms (custom_providers->providers, stt.model relocation,
    write_mode->write_approval, compression.summary_* removal, MCP-disable)
    persist their removals/renames.

An explicitly user-set non-default value (e.g. matrix.require_mention: false) is
preserved across the bump.

Guard tests lock the architecture: an AST check asserts migrate_config() makes no
direct save_config() call (all writes go through _persist_migration), and a
full-range v1->latest test asserts a lean config is never dumped. Two existing
change-detector tests that froze the on-disk representation of default-valued
keys are rewritten to assert the effective value via load_config() (behaviour
contract, not snapshot).

Validation: lean v1->latest migration drops from ~567 bytes to ~196 bytes;
148 config+setup and 196 profile/curator/migrate tests pass on scripts/run_tests.sh.
@kshitijk4poor
kshitijk4poor merged commit efd87a1 into NousResearch:main Jun 30, 2026
31 checks passed
waefrebeorn pushed a commit to waefrebeorn/slermes that referenced this pull request Jul 2, 2026
…igration-no-default-expansion

fix(config): route every migration write through one default-stripping chokepoint
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard area/config Config system, migrations, profiles sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades P2 Medium — degraded but workaround exists labels Jul 4, 2026
habarmc1223-sudo pushed a commit to habarmc1223-sudo/hermes-agent-fluxmem that referenced this pull request Jul 8, 2026
…igration-no-default-expansion

fix(config): route every migration write through one default-stripping chokepoint
santhreal pushed a commit to santhreal/hermes-agent that referenced this pull request Jul 13, 2026
…igration-no-default-expansion

fix(config): route every migration write through one default-stripping chokepoint
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
…igration-no-default-expansion

fix(config): route every migration write through one default-stripping chokepoint
@kshitijk4poor
kshitijk4poor deleted the fix/config-migration-no-default-expansion branch August 5, 2026 07:09
leewenjie pushed a commit to leewenjie/hermes-agent that referenced this pull request Aug 7, 2026
…igration-no-default-expansion

fix(config): route every migration write through one default-stripping chokepoint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/config Config system, migrations, profiles comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants