fix(config): refuse scalar writes to list-valued keys in config set

[paoloantinori]

Fixes #76138.

Summary

During a coding session, a Hermes agent ran hermes config set on a list-valued key (plugins.enabled), which wrote a scalar string into config.yaml instead of a YAML list, silently corrupting the config. The function had no awareness of the target key's schema type.

Changes

• List-valued key guard in set_config_value (hermes_cli/config.py): before coercing/writing, check _default_value_for_key(key) against DEFAULT_CONFIG. If the default is a list, refuse with a clear error that shows the correct YAML list form and directs the user to hermes config edit. This mirrors the existing scalar-over-mapping guard (which refuses dict-section overwrites without --force).

Test plan

• 76 tests pass (74 existing + 2 new).
• New: list-valued key (toolsets) refused with "list-valued" in the error; scalar key (terminal.backend) still set normally.
• Self-reviewed for reuse/simplification/efficiency/altitude (proportionate for a 15-line guard).

Backward compatibility

The guard fires only for keys whose DEFAULT_CONFIG default is a list. Unknown keys and scalar keys are unaffected. Users who need to set list values edit config.yaml directly (which is already the documented path for complex values).

[teknium1]

Thanks for isolating the scalar-write hazard and adding focused coverage.

Problems

• The new DEFAULT_CONFIG lookup does not cover the reported path. plugins is intentionally absent from DEFAULT_CONFIG (hermes_cli/config.py:1844-1858), so plugins.enabled bypasses the added guard and still reaches the scalar assignment in set_config_value() (hermes_cli/config.py:4948). Plugin loading treats a non-list allow-list as malformed (hermes_cli/plugins.py:243-268).
• website/docs/user-guide/features/browser.md:422 currently documents hermes config set toolsets '["hermes-cli", "browser"]'; because toolsets is a list default (hermes_cli/config_defaults.py:12), this change rejects the documented command.

Suggested changes

• Cover plugins.enabled/plugins.disabled through an appropriate schema source and add a regression test that verifies rejection leaves the file unchanged.
• Reconcile the documented toolsets command with the chosen list-setting behavior.

Automated hermes-sweeper review.

[teknium1]

plugins is intentionally absent from DEFAULT_CONFIG (hermes_cli/config.py:1844-1858), so _default_value_for_key("plugins.enabled") returns None. This leaves the issue's reported command unguarded; please cover the plugin allow/deny lists through a schema source that includes them.

[alt-glitch]

This was generated by AI during triage.

Related: #64389, #44226, and #40546 address the same list-scalar corruption at set_config_value, but parse structured CLI input. This PR instead refuses list-valued writes; a maintainer policy choice is required.

[paoloantinori]

Thanks @teknium1, both findings addressed in 0d64dc9f9:

1. plugins.enabled bypass: the guard now also checks the existing config value at the key path (inline traversal through user_config), so keys absent from DEFAULT_CONFIG but list-valued in the live config are detected. Additionally, a leading [ in the input is treated as unconditional list intent: _parse_list_value runs regardless of detection, so JSON-array input is never written as a scalar string even on first-set of an unknown key.

2. Documented toolsets command: the guard no longer refuses. It parses the input as a list (JSON array '["a", "b"]', comma-separated "a,b", or single value ["x"]) and writes a proper YAML list. The documented hermes config set toolsets '["hermes-cli", "browser"]' now works correctly.

79 tests pass (74 existing + 5 new: JSON array parse, comma-separated parse, existing-list-key detection, malformed-JSON refusal, scalar key unaffected). Self-reviewed for reuse/simplification/efficiency/altitude; the one real gap (first-set of a DEFAULT_CONFIG-absent key with non-[ input still writes a scalar) is acceptable: plugins enable is the realistic path for that key, and the [-intent guard covers JSON-array input regardless.

[GottZ]

This was generated by AI during triage.

Summary

Three PRs address the same scalar-write corruption in set_config_value: #37460 parses bracket-delimited YAML lists, revised #76139 detects list intent or list-typed keys and parses supported input, while #76234 refuses writes to detected list-valued keys unless forced.

Related pull requests

• fix(config): parse bracket-delimited list values in config set #37460 duplicate — (+61/-0) — narrow parser with salvageable tests: it parses bracket-delimited YAML such as [icarus], directly covering config set: string value overwrites list for 'enabled' key, silently breaking plugin loading #37455, but its malformed-input test never enters the parsing branch and the patch conflicts with current main. Despite the keep_open review on fix(config): parse bracket-delimited list values in config set #37460, its useful parsing cases and corrected fallback test should be transplanted into the broader current implementation rather than preserving this stale competing patch.
• fix(config): refuse scalar writes to list-valued keys in config set #76139 best fix — (+121/-1) — broadest current fix, but incomplete: the revised diff parses JSON arrays, comma-separated values, and detected list keys, preserving the documented toolsets workflow and preventing bracket-shaped input from becoming a scalar. The keep_open review remains applicable because first-time bare writes to plugins.enabled are still not recognized when that path is absent from both DEFAULT_CONFIG and the existing config; add explicit first-set plugin-list coverage and reconcile whether [icarus] should retain fix(config): parse bracket-delimited list values in config set #37460's YAML-list behavior.
• fix: guard against list-valued keys in hermes config set (#76138) #76234 partial — (+61/-0) — refusal-only guard: it detects existing/default list values, but rejects the documented toolsets command, adds no regression tests, and introduces an undocumented --force corruption bypass. Despite the keep_open review on fix: guard against list-valued keys in hermes config set (#76138) #76234, the diff's guard logic is subsumed by revised fix(config): refuse scalar writes to list-valued keys in config set #76139 while retaining those documented-contract and test gaps.

Duplicates

#76139 and #76234 target the same #76138 write path; revised #76139 subsumes #76234's list-key detection while choosing parsing instead of refusal. #37460 addresses the same underlying coercion defect more narrowly and contributes distinct YAML-bracket parsing cases that should be salvaged into #76139.

Suggested consolidation

Keep #76139 open with a salvage path: add a regression for first-time plugins.enabled, decide and test the [icarus] compatibility behavior using #37460's useful cases, and ensure malformed list input leaves the file unchanged. Then close #37460 as duplicate of #76139 after salvaging those cases, and close #76234 as duplicate of #76139 because the revised target already contains the broader detection logic without breaking the documented toolsets workflow.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    I37455(["issue #37455 (open)"])
    I76138(["issue #76138 (open)"])
    subgraph Dup37460 ["PRs duplicating each other"]
        P37460["PR #37460 (open)"]
        P76139["PR #76139 (open)"]
    end
    P76139 -->|best fix| I37455
    P76139 -->|best fix| I76138
    class I37455 open
    class I76138 open
    class P37460 open
    class P76139 open
    class P37460 best
    class P76139 best
    class P76139 best
    class P76139 target
    click I37455 "https://github.com/NousResearch/hermes-agent/issues/37455"
    click I76138 "https://github.com/NousResearch/hermes-agent/issues/76138"
    click P37460 "https://github.com/NousResearch/hermes-agent/pull/37460"
    click P76139 "https://github.com/NousResearch/hermes-agent/pull/76139"

Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed (state tag in the node label).

Cross-PR triage: Reviewed 3 pull requests and 2 issues in this complex. Each diff was read against this issue; Assessment working set: 15 kB of PR diffs, 9 kB of issue/PR text, 8 kB of discussion (13 comments), 10 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

[paoloantinori]

Heads-up on the red checks: across two CI runs on this head, three different gateway tests failed (slice 6/8: test_goal_verdict_send, test_goal_continuation_drain; rerun slice 10: test_session_api.py::test_session_chat_stream_treats_pre_existing_poisoned_row_as_no_model) while the previously-failed ones passed on the rerun. None reproduce locally on this branch or on clean main, none touch config (this PR only modifies hermes_cli/config.py + its tests), and main's own CI has been red on the same gateway-goal subsystem today (runs 32111461142, 32114523334). Looks like the current flake wave on main rather than anything from this diff; a re-run of the failed slice should clear it.

[paoloantinori]

Update for whoever reruns/merges: CI run 32116384893 at this head is fully green across all slices; the red slice entries in the rollup come from parallel flake runs (different gateway test each time, none config-related, none reproducible locally). Main's own CI has been red on the same gateway-goal tests today as well.