fix(config): parse structured config values

[kannishk]

Summary

• parse list-typed hermes config set values into real YAML lists instead of quoted strings
• accept flow-style list and mapping literals for unknown keys and enforce declared types for known list/mapping settings
• reject structured input for known scalar and nullable-scalar settings
• reject malformed, recursive, non-finite, non-string-keyed, and otherwise non-JSON-compatible structured values before writing either config.yaml or .env
• preserve string-typed settings and verify terminal env mirroring round-trips through dotenv + JSON

Root cause

set_config_value() only coerced booleans and numbers. A command such as:

hermes config set terminal.docker_volumes '["/tmp/demo-host:/client:rw"]'

therefore persisted a quoted YAML scalar rather than the documented list shape.

For terminal.docker_volumes, the canonical terminal config-to-env bridge can recover a valid JSON array at runtime; this PR fixes the persisted type at the write boundary rather than relying on that downstream recovery path. It also validates the declared default type so structured input cannot corrupt scalar settings such as terminal.timeout.

Verification

• scripts/run_tests.sh tests/hermes_cli/test_config.py tests/hermes_cli/test_set_config_value.py tests/tools/test_terminal_config_env_sync.py -q — 223 passed
• uv run ruff check hermes_cli/config.py tests/hermes_cli/test_set_config_value.py
• git diff --check upstream/main...HEAD
• end-to-end CLI smoke with temporary HERMES_HOME values confirmed:
  • a valid volume list reloads as a Python list
  • a YAML date value is rejected before config.yaml is created

Fixes #64323

[kannishk]

Follow-up after independent review:

The first review correctly identified three blockers: structured values could corrupt known scalar settings, YAML-native/recursive values could make env mirroring fail after the config write, and malformed unknown structures leaked parser exceptions.

The pushed follow-up now:

• distinguishes unknown keys from declared nullable settings with an explicit sentinel;
• enforces list, mapping, boolean/integer/number/scalar declarations before writes;
• validates accepted structures with strict JSON encoding (allow_nan=False), including circular-reference protection;
• cleanly rejects malformed input;
• covers dotenv/JSON env round-tripping and preservation of structured-looking string settings.

Post-rebase verification: 219 targeted config/env tests passed, Ruff passed, and git diff --check passed.

[kannishk]

Second independent-review follow-up is now pushed (head 98583a1e8):

• strict JSON validation already rejects .nan/.inf via allow_nan=False;
• recursive validation now requires every mapping key, including nested keys, to be a string, preventing YAML/config vs JSON/env key mutation;
• added behavior coverage for known mapping acceptance + env round-trip, known mapping/list type mismatch, non-string mapping-key rejection, and unknown flow-list parsing.

Final post-update gate: 223 targeted config/env tests passed, Ruff passed, and git diff --check passed.

[teknium1]

Thanks for the focused config-boundary fix. Current main still stores flow-style list input as a string: hermes_cli/config.py:8279-8289 only coerces boolean and numeric values, while terminal.docker_volumes is declared as a list at hermes_cli/config.py:1241.

Problems

• website/docs/user-guide/configuring-models.md:216 says hermes config set “only writes scalar values.” The proposed structured list/mapping support makes that statement inaccurate.

Suggested changes

• Update that sentence to explain that individual model.aliases.<name> entries are scalar, and document structured flow-style input where appropriate.

This is an automated hermes-sweeper review.

[GottZ]

This was generated by AI during triage.

Summary

Three PRs address the configuration-write side of #64323 by parsing structured values in set_config_value(); none changes Docker’s mount assembly or its handling of non-list docker_volumes values. #64389 provides the broadest validation, #64399 adds permissive YAML parsing but regresses declared-string handling and includes unrelated defaults, and closed #66439 implements the narrower known-list/mapping subset.

Related pull requests

• fix(config): parse structured config values #64389 best fix — (+231/-9) — n/a: Parses declared and unknown structured values, enforces declared types, rejects malformed or non-JSON-compatible data before writing, preserves string settings, and tests YAML persistence plus env round-trips. The automated keep-open verdict identifies high salvageability and one remaining documentation update: revise website/docs/user-guide/configuring-models.md:216 to match the new structured-input behavior.
• fix(cli): hermes config set now parses YAML lists/dicts (#64323) #64399 duplicate — (+133/-63) — n/a: Parses bracketed YAML lists/mappings at the same write boundary, but the diff removes the declared-string guard—allowing approvals.mode=off to become boolean false—and mixes in unrelated DEFAULT_CONFIG changes. Despite the COMMENTED keep-open review, its documented blockers and the stronger guarded implementation in fix(config): parse structured config values #64389 support closing it as a duplicate of fix(config): parse structured config values #64389; contributor review also identifies it as a duplicate of fix(cli): parse JSON object values in config set #40546.
• fix(config): parse structured config values #66439 [closed] partial — (+33/-4) — n/a: Narrowly parses values only for known list/mapping defaults while preserving existing scalar and unknown-key coercion; it remains relevant as the minimal subset now covered more comprehensively by fix(config): parse structured config values #64389. It is already closed as superseded by fix(config): parse structured config values #64389.

Duplicates

#64399 and #64389 target the same set_config_value() structured-literal cause, with #64389 retaining the string guard and adding validation; #64399 was also identified by contributor review as a duplicate of #40546. #66439 overlaps the known list/mapping portion of #64389 and is already closed as superseded by it.

Suggested consolidation

Keep #64389 open with a salvage path: retain its validated config-boundary implementation and tests, and update website/docs/user-guide/configuring-models.md:216 as requested by the automated keep-open review. Close #64399 as a duplicate of #64389 because its diff regresses string-typed settings and contains unrelated configuration changes; leave #66439 closed as superseded, while tracking Docker’s separate silent non-list/mount-assembly behavior outside these config-only diffs.

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
    I64323(["issue #64323 (open)"])
    subgraph Dup64389 ["PRs duplicating each other"]
        P64389["PR #64389 (open)"]
        P64399["PR #64399 (open)"]
    end
    P64389 -->|best fix| I64323
    class I64323 open
    class P64389 open
    class P64399 open
    class P64389 best
    class P64389 target
    click I64323 "https://github.com/NousResearch/hermes-agent/issues/64323"
    click P64389 "https://github.com/NousResearch/hermes-agent/pull/64389"
    click P64399 "https://github.com/NousResearch/hermes-agent/pull/64399"

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 1 issue in this complex. Each diff was read against this issue; Assessment working set: 28 kB of PR diffs, 6 kB of issue/PR text, 4 kB of discussion (7 comments), 5 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

[alt-glitch]

This was generated by AI during triage.

Related: #40546, #59182, and #76470 address the same config set structured-value gap with different coercion and validation policies. A maintainer should select one contract rather than merge overlapping parsers.

[teknium1]

Resolved via PR #88163 (merged) — hermes config set now parses structured list/dict values (yaml.safe_load, string-typed-key guard respected, conservative trigger). This was an 8-PR duplicate cluster: first submitter was @liuhao1024 (#37460), and the merged base was #59182 (@sam7894604) with authorship preserved; all cluster authors are credited in the PR body. Thanks for taking a run at it!