Skip to content

fix(config): parse JSON list/dict values in config set - #60558

Closed
AIalliAI wants to merge 3 commits into
NousResearch:mainfrom
AIalliAI:fix/60551-config-set-list-values
Closed

fix(config): parse JSON list/dict values in config set#60558
AIalliAI wants to merge 3 commits into
NousResearch:mainfrom
AIalliAI:fix/60551-config-set-list-values

Conversation

@AIalliAI

@AIalliAI AIalliAI commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #60551: hermes config set stores list values like '["A","B"]' as quoted YAML scalar strings instead of YAML sequences.

Root Cause

set_config_value() in hermes_cli/config.py coerces values to bool/int/float but has no JSON parsing step for list and dict shapes. When a user runs:

hermes config set terminal.env_passthrough '["FITNESS_TRACKER_TOKEN","FITNESS_TRACKER_URL"]'

The value stays as a string, and YAML serialization writes it as a quoted scalar:

terminal:
  env_passthrough: '["FITNESS_TRACKER_TOKEN","FITNESS_TRACKER_URL"]'   # string, not list

Fix

Add JSON parsing for values starting with { or [ after the existing type coercion. If parse succeeds and result is a dict or list, use the parsed value — YAML serialization then writes it as a proper sequence/mapping.

terminal:
  env_passthrough:
    - FITNESS_TRACKER_TOKEN
    - FITNESS_TRACKER_URL

Test Plan

  • Negative control: unpatched code stores ["A","B"] as scalar string
  • Positive test: patched code stores as YAML sequence - A\n- B
  • Existing bool/int/float/string coercion paths unchanged

Change

1 file, +10 lines: hermes_cli/config.py

@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 P2 Medium — degraded but workaround exists duplicate This issue or pull request already exists labels Jul 7, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #40546 — same fix (JSON json.loads detection in set_config_value() at hermes_cli/config.py) at the same code site; #40546 is the earliest open PR for this. Also related to the anchor issue #40545 / #60551 and the open array+comma variant #44226. Flagging the cluster for a maintainer to pick the canonical config-set JSON fix.

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the focused fix. The underlying issue is still present on current main: hermes_cli/config.py:8268-8279 performs scalar coercion only, then writes JSON-looking input unchanged.

Problems

  • The proposed parser at hermes_cli/config.py:8042 was authored before current main added the declared-string preservation guard in hermes_cli/config.py:8264-8278 (e4ea0a0ed7). Salvaging the old hunk verbatim would conflict with that later invariant.
  • The PR diff changes no tests. Current coverage in tests/hermes_cli/test_set_config_value.py includes string-default behavior, but no JSON list/dict round-trip regression for config set.

Suggested changes

  • Integrate JSON list/dict parsing into the current non-string coercion branch, retaining _default_value_for_key() protection for declared string leaves.
  • Add round-trip coverage for terminal.env_passthrough as a JSON array, a JSON mapping, malformed JSON fallback, and the string-default invariant.

Automated hermes-sweeper review.

Comment thread hermes_cli/config.py Outdated
stripped = value.strip()
if stripped.startswith(('{', '[')):
try:
parsed = json.loads(stripped)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Current main now preserves values for keys whose DEFAULT_CONFIG leaf is a string (e4ea0a0ed7, now hermes_cli/config.py:8264-8278). Please integrate this structured parse under that non-string guard when salvaging, so the newer string-value invariant remains intact.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026
@AIalliAI
AIalliAI force-pushed the fix/60551-config-set-list-values branch from b089cf2 to 72e64da Compare July 26, 2026 13:07
hermes config set stores list values like '["A","B"]' as
quoted YAML scalar strings instead of YAML sequences because
set_config_value() only coerces booleans, ints, and floats.

Add JSON parsing for values starting with '{' or '[' so list
and dict values are serialized as proper YAML sequences/mappings.

Also loosen the value parameter type from str to Any since it
can now hold list/dict values after JSON parsing.

Closes NousResearch#60551
@AIalliAI
AIalliAI force-pushed the fix/60551-config-set-list-values branch from 72e64da to 6bd780a Compare July 26, 2026 15:41
AIalliAI added 2 commits July 26, 2026 17:18
… file

feat(bench): add tool_search_livetest2 benchmark script for deferred-tool scenarios
@teknium1

Copy link
Copy Markdown
Contributor

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!

@teknium1 teknium1 closed this Aug 17, 2026
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 duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform 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.

config.yaml: agent write guard, and 'hermes config set' writes string scalar for list keys

3 participants