Skip to content

fix(config): preserve platform_toolsets during v25→v26 config migration (#38798) - #39581

Closed
ashishpatel26 wants to merge 2 commits into
NousResearch:mainfrom
ashishpatel26:fix/config-migration-toolsets-38798
Closed

fix(config): preserve platform_toolsets during v25→v26 config migration (#38798)#39581
ashishpatel26 wants to merge 2 commits into
NousResearch:mainfrom
ashishpatel26:fix/config-migration-toolsets-38798

Conversation

@ashishpatel26

@ashishpatel26 ashishpatel26 commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Summary

  • hermes_cli/config.py: added v25→v26 migration block that preserves existing platform_toolsets entries verbatim, only coercing malformed non-list values; never injects defaults when the key is absent
  • Tests: 3 regression tests in TestPlatformToolsetsPreservedDuringV25ToV26Migration

Root cause (fixes #38798)

The v25→v26 version bump added no migration block. Any migration code that naively set platform_toolsets to a default value would wipe user-configured toolsets, silently disabling all their tools. The explicit preservation block with tests prevents this.

🤖 Generated with Claude Code

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cli CLI entry point, hermes_cli/, setup wizard area/config Config system, migrations, profiles labels Jun 5, 2026
tonydwb
tonydwb approved these changes Jun 5, 2026
View reviewed changes

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: Approved

Fixes a data-loss regression in the v25-to-v26 config migration that previously wiped user-custom platform_toolsets entries. The fix:

  • Replaces the destructive empty-list seeding with an additive preserve-and-coerce strategy
  • Never overwrites existing per-platform keys
  • Coerces malformed non-list entries to [] instead of deleting
  • Adds thorough regression tests covering the exact bug scenario

Looks Good

  • Defender-style coding: coercion within a try/except block prevents migrator crashes from corrupt configs
  • Regression tests explicitly name issue #38798 and pin the expected behavior
  • Tests cover three edge cases: custom toolset lists, absent platform_toolsets, and malformed non-list entries
  • Change is backwards-compatible with DEFAULT_CONFIG deep-merge still filling in new platforms

Suggestions (non-blocking)

  • Consider logging a WARNING-level dedup message when coercion occurs, so power users can detect and clean broken toolset entries

Reviewed by Hermes Agent (cron)

@ashishpatel26

ashishpatel26 commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review and approval. Added the WARNING log in the latest push — when coercion occurs, it now emits:

\
WARNING platform_toolsets[''] has unexpected type (expected list); coercing to [] during v25→v26 migration. Check your config file if tools are missing.
\\

This gives power users a clear signal in logs that something was wrong with their config, without breaking the migration.

@ashishpatel26
fix(config): preserve platform_toolsets during v25->v26 config migrat…
1e90b72 
…ion (NousResearch#38798)

A previous draft of the v25->v26 migration block incorrectly seeded
platform_toolsets with empty lists for platforms that appear in the new
display.platforms streaming defaults (telegram, discord), silently wiping
all configured tools for users who had custom toolset lists.

The fix adds an explicit v25->v26 migration block that:
- Reads the raw on-disk platform_toolsets (not the deep-merged value) so
  only user-written entries are touched.
- Preserves every existing per-platform toolset list verbatim.
- Only coerces non-list entries to [] (defensive, never drops real lists).
- Never injects a default platform_toolsets when the key is absent; it is
  seeded lazily by tools_config.py on first use instead.
- display.platforms streaming defaults (telegram on, discord off) are
  already filled by the DEFAULT_CONFIG deep-merge in load_config() and
  require no explicit migration write.

Adds three regression tests that pin the invariant: custom toolsets on
v25 survive migration intact, absent platform_toolsets stays absent, and
a single-platform cli-only entry is preserved.
@ashishpatel26
fix(config): log WARNING when coercing malformed platform_toolsets en…
497db6b 
…try (NousResearch#38798)
@ashishpatel26
ashishpatel26 force-pushed the fix/config-migration-toolsets-38798 branch from cc0e291 to 497db6b Compare June 12, 2026 18:43
@kshitijk4poor kshitijk4poor mentioned this pull request Jun 26, 2026
Merged
kshitijk4poor pushed a commit that referenced this pull request Jun 26, 2026
@lEWFkRAD @kshitijk4poor
fix(config): surface invalid platform_toolsets instead of silently dr…
41ede84 
…opping tools (#38798)

A config migration (or hand-edit) that leaves an invalid toolset name in
`platform_toolsets` — e.g. the #38798 corruption that rewrote `hermes-cli` to
the non-existent `hermes` — silently disabled all affected tools:
resolve_toolset() returns [] for an unknown name, so the agent quietly lost its
tools with no error, warning, or log entry and degraded to text-only replies.

Surface it loudly at two points:
- After migration (migrate_config): validate platform_toolsets and record/print
  a warning per unknown name, with a `hermes-<platform>` suggestion when that
  would have been valid (the exact #38798 shape).
- At runtime (_get_platform_tools): if a platform was explicitly configured but
  every toolset name is invalid, log a warning when tools are resolved for a
  session — so an ALREADY-corrupted config is caught at startup, not only on the
  next `hermes update`.

Logic lives in a new pure, side-effect-free helper (toolset_validation.py) with
validate_toolset injected, so it is unit-testable without the tool registry.

Note: the original v25→v26 migration that caused the corruption no longer
exists (config format is now v30; no migration step rewrites toolset names).
This change is the durable defense against the silent-failure mode regardless
of cause, matching the issue's "Expected: log a warning".

Salvaged from #39207 by @lEWFkRAD (authorship preserved via cherry-pick).
Tests: 9 helper cases (incl. the #38798 corruption shape, mixed valid/invalid,
zero-tools state, non-dict/scalar/non-string) + a runtime caplog test — both the
helper warning and the runtime guard mutation-verified to fail without the fix.

Closes #38798. Supersedes #39581 (prevent-in-v25→v26 — that path is gone),
#41006 / #40208 (repair-migration for already-corrupted configs).
@kshitijk4poor

kshitijk4poor commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

Thanks @ashishpatel26 — closing in favor of #52920 (#38798). Your approach (preserve platform_toolsets during the v25→v26 migration) addressed the corruption at its source, but that migration step no longer exists on main (config format is now v30; a full-history check shows no migration that rewrites toolset names), so there's nothing left to preserve at that point. #52920 instead makes the silent-failure mode loud (warn at migration + runtime) so any future cause is caught. Appreciate the fix!

pai-scaffolde pushed a commit to pai-scaffolde/hermes-agent that referenced this pull request Jun 28, 2026
@lEWFkRAD @pai-scaffolde
fix(config): surface invalid platform_toolsets instead of silently dr…
1276c99 
…opping tools (NousResearch#38798)

A config migration (or hand-edit) that leaves an invalid toolset name in
`platform_toolsets` — e.g. the NousResearch#38798 corruption that rewrote `hermes-cli` to
the non-existent `hermes` — silently disabled all affected tools:
resolve_toolset() returns [] for an unknown name, so the agent quietly lost its
tools with no error, warning, or log entry and degraded to text-only replies.

Surface it loudly at two points:
- After migration (migrate_config): validate platform_toolsets and record/print
  a warning per unknown name, with a `hermes-<platform>` suggestion when that
  would have been valid (the exact NousResearch#38798 shape).
- At runtime (_get_platform_tools): if a platform was explicitly configured but
  every toolset name is invalid, log a warning when tools are resolved for a
  session — so an ALREADY-corrupted config is caught at startup, not only on the
  next `hermes update`.

Logic lives in a new pure, side-effect-free helper (toolset_validation.py) with
validate_toolset injected, so it is unit-testable without the tool registry.

Note: the original v25→v26 migration that caused the corruption no longer
exists (config format is now v30; no migration step rewrites toolset names).
This change is the durable defense against the silent-failure mode regardless
of cause, matching the issue's "Expected: log a warning".

Salvaged from NousResearch#39207 by @lEWFkRAD (authorship preserved via cherry-pick).
Tests: 9 helper cases (incl. the NousResearch#38798 corruption shape, mixed valid/invalid,
zero-tools state, non-dict/scalar/non-string) + a runtime caplog test — both the
helper warning and the runtime guard mutation-verified to fail without the fix.

Closes NousResearch#38798. Supersedes NousResearch#39581 (prevent-in-v25→v26 — that path is gone),
NousResearch#41006 / NousResearch#40208 (repair-migration for already-corrupted configs).
waefrebeorn pushed a commit to waefrebeorn/slermes that referenced this pull request Jul 2, 2026
@lEWFkRAD @kshitijk4poor
fix(config): surface invalid platform_toolsets instead of silently dr…
a0f2227 
…opping tools (NousResearch#38798)

A config migration (or hand-edit) that leaves an invalid toolset name in
`platform_toolsets` — e.g. the NousResearch#38798 corruption that rewrote `hermes-cli` to
the non-existent `hermes` — silently disabled all affected tools:
resolve_toolset() returns [] for an unknown name, so the agent quietly lost its
tools with no error, warning, or log entry and degraded to text-only replies.

Surface it loudly at two points:
- After migration (migrate_config): validate platform_toolsets and record/print
  a warning per unknown name, with a `hermes-<platform>` suggestion when that
  would have been valid (the exact NousResearch#38798 shape).
- At runtime (_get_platform_tools): if a platform was explicitly configured but
  every toolset name is invalid, log a warning when tools are resolved for a
  session — so an ALREADY-corrupted config is caught at startup, not only on the
  next `hermes update`.

Logic lives in a new pure, side-effect-free helper (toolset_validation.py) with
validate_toolset injected, so it is unit-testable without the tool registry.

Note: the original v25→v26 migration that caused the corruption no longer
exists (config format is now v30; no migration step rewrites toolset names).
This change is the durable defense against the silent-failure mode regardless
of cause, matching the issue's "Expected: log a warning".

Salvaged from NousResearch#39207 by @lEWFkRAD (authorship preserved via cherry-pick).
Tests: 9 helper cases (incl. the NousResearch#38798 corruption shape, mixed valid/invalid,
zero-tools state, non-dict/scalar/non-string) + a runtime caplog test — both the
helper warning and the runtime guard mutation-verified to fail without the fix.

Closes NousResearch#38798. Supersedes NousResearch#39581 (prevent-in-v25→v26 — that path is gone),
NousResearch#41006 / NousResearch#40208 (repair-migration for already-corrupted configs).
habarmc1223-sudo pushed a commit to habarmc1223-sudo/hermes-agent-fluxmem that referenced this pull request Jul 8, 2026
@lEWFkRAD @kshitijk4poor
fix(config): surface invalid platform_toolsets instead of silently dr…
f965f9f 
…opping tools (NousResearch#38798)

A config migration (or hand-edit) that leaves an invalid toolset name in
`platform_toolsets` — e.g. the NousResearch#38798 corruption that rewrote `hermes-cli` to
the non-existent `hermes` — silently disabled all affected tools:
resolve_toolset() returns [] for an unknown name, so the agent quietly lost its
tools with no error, warning, or log entry and degraded to text-only replies.

Surface it loudly at two points:
- After migration (migrate_config): validate platform_toolsets and record/print
  a warning per unknown name, with a `hermes-<platform>` suggestion when that
  would have been valid (the exact NousResearch#38798 shape).
- At runtime (_get_platform_tools): if a platform was explicitly configured but
  every toolset name is invalid, log a warning when tools are resolved for a
  session — so an ALREADY-corrupted config is caught at startup, not only on the
  next `hermes update`.

Logic lives in a new pure, side-effect-free helper (toolset_validation.py) with
validate_toolset injected, so it is unit-testable without the tool registry.

Note: the original v25→v26 migration that caused the corruption no longer
exists (config format is now v30; no migration step rewrites toolset names).
This change is the durable defense against the silent-failure mode regardless
of cause, matching the issue's "Expected: log a warning".

Salvaged from NousResearch#39207 by @lEWFkRAD (authorship preserved via cherry-pick).
Tests: 9 helper cases (incl. the NousResearch#38798 corruption shape, mixed valid/invalid,
zero-tools state, non-dict/scalar/non-string) + a runtime caplog test — both the
helper warning and the runtime guard mutation-verified to fail without the fix.

Closes NousResearch#38798. Supersedes NousResearch#39581 (prevent-in-v25→v26 — that path is gone),
NousResearch#41006 / NousResearch#40208 (repair-migration for already-corrupted configs).
santhreal pushed a commit to santhreal/hermes-agent that referenced this pull request Jul 13, 2026
@lEWFkRAD @kshitijk4poor
fix(config): surface invalid platform_toolsets instead of silently dr…
f1326af 
…opping tools (NousResearch#38798)

A config migration (or hand-edit) that leaves an invalid toolset name in
`platform_toolsets` — e.g. the NousResearch#38798 corruption that rewrote `hermes-cli` to
the non-existent `hermes` — silently disabled all affected tools:
resolve_toolset() returns [] for an unknown name, so the agent quietly lost its
tools with no error, warning, or log entry and degraded to text-only replies.

Surface it loudly at two points:
- After migration (migrate_config): validate platform_toolsets and record/print
  a warning per unknown name, with a `hermes-<platform>` suggestion when that
  would have been valid (the exact NousResearch#38798 shape).
- At runtime (_get_platform_tools): if a platform was explicitly configured but
  every toolset name is invalid, log a warning when tools are resolved for a
  session — so an ALREADY-corrupted config is caught at startup, not only on the
  next `hermes update`.

Logic lives in a new pure, side-effect-free helper (toolset_validation.py) with
validate_toolset injected, so it is unit-testable without the tool registry.

Note: the original v25→v26 migration that caused the corruption no longer
exists (config format is now v30; no migration step rewrites toolset names).
This change is the durable defense against the silent-failure mode regardless
of cause, matching the issue's "Expected: log a warning".

Salvaged from NousResearch#39207 by @lEWFkRAD (authorship preserved via cherry-pick).
Tests: 9 helper cases (incl. the NousResearch#38798 corruption shape, mixed valid/invalid,
zero-tools state, non-dict/scalar/non-string) + a runtime caplog test — both the
helper warning and the runtime guard mutation-verified to fail without the fix.

Closes NousResearch#38798. Supersedes NousResearch#39581 (prevent-in-v25→v26 — that path is gone),
NousResearch#41006 / NousResearch#40208 (repair-migration for already-corrupted configs).
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
@lEWFkRAD @kshitijk4poor
fix(config): surface invalid platform_toolsets instead of silently dr…
db0685d 
…opping tools (NousResearch#38798)

A config migration (or hand-edit) that leaves an invalid toolset name in
`platform_toolsets` — e.g. the NousResearch#38798 corruption that rewrote `hermes-cli` to
the non-existent `hermes` — silently disabled all affected tools:
resolve_toolset() returns [] for an unknown name, so the agent quietly lost its
tools with no error, warning, or log entry and degraded to text-only replies.

Surface it loudly at two points:
- After migration (migrate_config): validate platform_toolsets and record/print
  a warning per unknown name, with a `hermes-<platform>` suggestion when that
  would have been valid (the exact NousResearch#38798 shape).
- At runtime (_get_platform_tools): if a platform was explicitly configured but
  every toolset name is invalid, log a warning when tools are resolved for a
  session — so an ALREADY-corrupted config is caught at startup, not only on the
  next `hermes update`.

Logic lives in a new pure, side-effect-free helper (toolset_validation.py) with
validate_toolset injected, so it is unit-testable without the tool registry.

Note: the original v25→v26 migration that caused the corruption no longer
exists (config format is now v30; no migration step rewrites toolset names).
This change is the durable defense against the silent-failure mode regardless
of cause, matching the issue's "Expected: log a warning".

Salvaged from NousResearch#39207 by @lEWFkRAD (authorship preserved via cherry-pick).
Tests: 9 helper cases (incl. the NousResearch#38798 corruption shape, mixed valid/invalid,
zero-tools state, non-dict/scalar/non-string) + a runtime caplog test — both the
helper warning and the runtime guard mutation-verified to fail without the fix.

Closes NousResearch#38798. Supersedes NousResearch#39581 (prevent-in-v25→v26 — that path is gone),
NousResearch#41006 / NousResearch#40208 (repair-migration for already-corrupted configs).
leewenjie pushed a commit to leewenjie/hermes-agent that referenced this pull request Aug 7, 2026
@lEWFkRAD @kshitijk4poor
fix(config): surface invalid platform_toolsets instead of silently dr…
0281509 
…opping tools (NousResearch#38798)

A config migration (or hand-edit) that leaves an invalid toolset name in
`platform_toolsets` — e.g. the NousResearch#38798 corruption that rewrote `hermes-cli` to
the non-existent `hermes` — silently disabled all affected tools:
resolve_toolset() returns [] for an unknown name, so the agent quietly lost its
tools with no error, warning, or log entry and degraded to text-only replies.

Surface it loudly at two points:
- After migration (migrate_config): validate platform_toolsets and record/print
  a warning per unknown name, with a `hermes-<platform>` suggestion when that
  would have been valid (the exact NousResearch#38798 shape).
- At runtime (_get_platform_tools): if a platform was explicitly configured but
  every toolset name is invalid, log a warning when tools are resolved for a
  session — so an ALREADY-corrupted config is caught at startup, not only on the
  next `hermes update`.

Logic lives in a new pure, side-effect-free helper (toolset_validation.py) with
validate_toolset injected, so it is unit-testable without the tool registry.

Note: the original v25→v26 migration that caused the corruption no longer
exists (config format is now v30; no migration step rewrites toolset names).
This change is the durable defense against the silent-failure mode regardless
of cause, matching the issue's "Expected: log a warning".

Salvaged from NousResearch#39207 by @lEWFkRAD (authorship preserved via cherry-pick).
Tests: 9 helper cases (incl. the NousResearch#38798 corruption shape, mixed valid/invalid,
zero-tools state, non-dict/scalar/non-string) + a runtime caplog test — both the
helper warning and the runtime guard mutation-verified to fail without the fix.

Closes NousResearch#38798. Supersedes NousResearch#39581 (prevent-in-v25→v26 — that path is gone),
NousResearch#41006 / NousResearch#40208 (repair-migration for already-corrupted configs).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@tonydwb tonydwb tonydwb approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/config Config system, migrations, profiles comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

bug: config migration (v25->v26) corrupts platform_toolsets, silently kills all tools

4 participants

@ashishpatel26 @kshitijk4poor @tonydwb @alt-glitch