fix(config): coerce list/dict literals in 'hermes config set' - #79479
fix(config): coerce list/dict literals in 'hermes config set'#79479cadamec wants to merge 1 commit into
Conversation
set_config_value only coerced bool/int/float. `hermes config set platform_toolsets.discord '["clarify","file"]'` was stored as a raw string, and every reader gated on isinstance(..., list) (_get_platform_tools, _get_enabled_set, _get_disabled_set) silently ignored it and fell back to its default — the setting looked saved but never took effect (NousResearch#57063). Values starting with [ or { are now parsed with yaml.safe_load; only real list/dict results are accepted. Non-list/dict results and YAML errors warn on stderr and keep the legacy string behavior. Scalar coercion (bool/int/float) is unchanged.
The a2a client-tool fix and the config coercion fix are unrelated bugs that happened to land in one PR. Per triage feedback, this PR is now a2a-only; the config half lives in NousResearch#79479.
Duplicate of #57063: both patches correct hermes config set persisting list/dict literals as strings, which silently defeats list-gated configuration readers. |
|
Fair flag, but this isn't a clean duplicate of #57063 — and my own empirical check on #57063 (Aug 5) shows why. #57063 bundles two distinct failure modes, and the primary one no longer reproduces on current main. Commit 52d9e57 ("feat: dynamic toolset generation for plugin platforms") already fixed the "eight platforms resolve to zero tools" case — I verified all eight now resolve to the full 61-tool core toolset after discovery, in any process. That half of #57063 is dead. What still reproduces live are the two remaining halves, and #79479 targets one of them: So rather than a duplicate of a healthy PR, this is the current, rebased, tested fix for the one live half of #57063 — a successor, not a clone. The a2a half is tracked separately in #78050. Happy to rebase/retarget or close #57063-scope accordingly; goal is the fix landing on current main. |
|
Correction: the triage bot was right and my earlier reply was wrong. I checked #57063's diff (not just its body/merge state) and it already contains this exact fix — same Closing this as a duplicate. The real gap is that #57063 is stale (CONFLICTING with main) and its body still describes the 8-bundle half that 52d9e57 already made obsolete — I'm going to rebase it onto current main and narrow it to the live config half, preserving the original author's work per the contribution rubric. |
|
Closing as duplicate of #57063 (see correction above). |
What does this PR do?
hermes config setstores list/dict literals as raw strings, silently defeating every list-gated reader.The bug
set_config_value(hermes_cli/config.py) only coerces bool/int/float. A list literal likeis written to
config.yamlas the string'["clarify","file"]'. Every reader gated onisinstance(..., list)—_get_platform_tools,_get_enabled_set,_get_disabled_set— silently ignores it and falls back to its default. The setting looks saved (the echo says ✓, it's in the YAML) but never takes effect. This is the config half of #57063, which reported a deployment running for weeks with a zero-tool agent becauseplatform_toolsetshad been set this way.The fix
Values whose first non-whitespace char is
[or{are parsed withyaml.safe_load. Only reallist/dictresults are accepted; anything else (malformed YAML, a string like"[unterminated"that happens to parse to a scalar) warns on stderr and keeps the legacy string behavior. Bool/int/float coercion and string-typed defaults (approvals.mode: offstaying a string) are untouched.Tests
tests/hermes_cli/test_config_set_list_values.py:Affected suites: 95 passed (
test_config_set_list_values.py+test_set_config_value.py).Related