Skip to content

fix(config): parse YAML list/dict values in set_config_value - #88066

Closed
HouMinXi wants to merge 1 commit into
NousResearch:mainfrom
HouMinXi:fix/config-set-yaml-parse
Closed

HouMinXi wants to merge 1 commit into
NousResearch:mainfrom
HouMinXi:fix/config-set-yaml-parse

Conversation

@HouMinXi

Copy link
Copy Markdown

Problem

hermes config set custom_providers '- name: oneapi\n type: openai...' writes the value as a quoted string, not a YAML array. The config file ends up with custom_providers: '[{"name":"oneapi",...}]' (string) instead of proper YAML list structure. Hermes then fails to parse custom_providers as a list.

Root cause

set_config_value in hermes_cli/config.py receives its value as a str from argparse. The existing coercion block (lines 5284-5295) only handles bool/int/float — no YAML parse attempt. Multi-line YAML input stays as a raw string and gets written verbatim to config.yaml.

Fix

After the existing bool/int/float coercion, add a YAML parse fallback:

  • If the value contains a newline or starts with -, {, or [ (YAML collection indicators), attempt yaml.safe_load(value).
  • If it succeeds and returns a non-str type (list, dict, etc.), use the parsed value.
  • If it fails (YAMLError) or returns a str, keep the original coerced value.
  • Scalars already coerced to bool/int/float above are unaffected — the isinstance(value, str) guard skips them.

yaml is already imported at the top of config.py (line 330), so no new import needed.

Tests

New file tests/hermes_cli/test_config_set_yaml.py (16 tests):

  1. Bug-injection proof — multi-line YAML list stored as list (not str). Verified: removing the fix → 4 tests FAIL; restoring → all pass.
  2. Scalar regression — bool/int/float/str/enum coercion unchanged (6 bool params + int + float + plain string + string-typed enum).
  3. Single-line dict{enabled: true, name: test} parses to dict.
  4. Single-line list[alpha, beta, gamma] parses to list.
  5. Invalid YAML fallback — value with newline that parses back to str stays str; malformed YAML ({a: 1) raises YAMLError and keeps original string.

All 99 tests pass (16 new + 83 existing test_set_config_value.py).

hermes config set receives its value as a str from argparse.  The existing
coercion block only handled bool/int/float, so multi-line YAML input like
custom_providers was written to config.yaml as a quoted string instead of a
proper YAML collection.  Hermes then failed to parse it as a list.

Add a YAML parse fallback after the scalar coercion: when the value
contains a newline or starts with a YAML collection indicator (-, {, [),
attempt yaml.safe_load.  Adopt the parsed value only when it is not itself
a str; on YAMLError keep the original.  Scalars (already coerced above)
are unaffected because the type check skips non-str values.

Tests cover the primary bug (multi-line list), single-line dict/list,
scalar regression (bool/int/float/str/enum), and invalid-YAML fallback.
@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 sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades duplicate This issue or pull request already exists labels Aug 17, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for the fix — the bug is real (hermes config set still stores structured values as quoted strings on main), and this is one of the better-built entries in the class: yaml.safe_load, parsed-plain-strings left alone, real isolated-HERMES_HOME tests.

Closing as a duplicate though: this exact fix class has at least 7 earlier open PRs, starting with #37460 (June 2) and with #59182 matching your list/mapping scope most closely. We'll resolve the cluster through the earliest submissions with credit to all. One note for the record if you iterate elsewhere: the new YAML fallback fires even for string-typed keys, bypassing the _default_value_for_key guard main uses to keep string settings (e.g. approvals.mode) from being coerced — any consolidated fix needs to respect that guard.

Appreciate the contribution!

@teknium1 teknium1 closed this Aug 17, 2026
teknium1 added a commit that referenced this pull request Aug 17, 2026
…s with a conservative trigger

Consolidation follow-up on top of #59182's cherry-picked base:

- Add _looks_structured_value(): triggers a yaml.safe_load structured
  parse only when the value starts with '[' / '{' or spans multiple
  lines with YAML list-item ('- x') or mapping-entry ('key: v') shaped
  lines. Deliberately avoids the over-broad leading '-' trigger from
  #88066 so '-5' and '--flag' stay strings.
- Stays folded INSIDE the string-typed-key guard: keys whose
  DEFAULT_CONFIG type is str (e.g. approvals.mode) are never coerced.
- Tests: multi-line YAML list/dict, string-typed key given '[x]' and
  '-5' stays string, dash-prefixed scalars stay strings, plain
  multi-line prose stays a string, load_config round-trip.
  Sabotage-verified: 7 of the suite's tests fail on main without the fix.
mikeholownych added a commit to mikeholownych/charterforge that referenced this pull request Aug 17, 2026
…ite (#8)

* fix(config): parse list/mapping literals in hermes config set

Fold the list/mapping parser INSIDE the existing string-typed-value coercion guard (the `not isinstance(_default_value_for_key(key), str)` block from e4ea0a0) instead of running it unconditionally, so a genuinely string-typed setting whose value merely starts with '[' or '{' is left untouched while non-string keys get JSON/YAML flow literals parsed to real lists/dicts.

Update website/docs/user-guide/configuring-models.md: the `config set only writes scalar values` note is no longer accurate; document the list/mapping support with a quoted example.

Fixes NousResearch#40545 NousResearch#50168

* fix(config): extend structured-value parsing to multi-line YAML blocks with a conservative trigger

Consolidation follow-up on top of NousResearch#59182's cherry-picked base:

- Add _looks_structured_value(): triggers a yaml.safe_load structured
  parse only when the value starts with '[' / '{' or spans multiple
  lines with YAML list-item ('- x') or mapping-entry ('key: v') shaped
  lines. Deliberately avoids the over-broad leading '-' trigger from
  NousResearch#88066 so '-5' and '--flag' stay strings.
- Stays folded INSIDE the string-typed-key guard: keys whose
  DEFAULT_CONFIG type is str (e.g. approvals.mode) are never coerced.
- Tests: multi-line YAML list/dict, string-typed key given '[x]' and
  '-5' stays string, dash-prefixed scalars stay strings, plain
  multi-line prose stays a string, load_config round-trip.
  Sabotage-verified: 7 of the suite's tests fail on main without the fix.

* Port from can1357/oh-my-pi#7553: allow quoted shell metacharacters in allowlist matching

command_allowlist glob rules (e.g. 'cargo *') rejected any command whose
quoted arguments contained shell metacharacters — a cargo benchmark
regex filter like '^layer3/write/(a|b)$' disqualified the whole command
even though those characters are literal to the shell.

_has_allowlist_shell_operator is now quote-aware:
- metacharacters inside single/double quotes or behind a backslash are
  treated as literal arguments;
- $ and backtick inside DOUBLE quotes still disqualify (expansion is
  active there);
- quoted/escaped control characters still disqualify when the command
  carries a -c/-e/--command/--eval-style option that hands the payload
  to another interpreter (sh -c '...', git -c alias.x='!...' x);
- unterminated quotes disqualify (shape can't be reasoned about).

Compound commands (unquoted ; & | < > backtick $( newline) are rejected
exactly as before. hermes_cli/approvals_suggest.derive_glob picks up the
same semantics via its existing import.

* feat(terminal): interpret signal-termination exit codes for the model

Port from Kilo-Org/kilocode#12698: report signal-terminated commands with
a human-readable note instead of a bare numeric exit code.

Kilo's fix settles a signal-killed process as the conventional 128+signum
exit code so its bash tool stops hanging. Hermes already produces numeric
codes for signal deaths (subprocess -signum, or the shell's 128+signum),
but the model saw a bare exit_code=-9 or 137 and burned turns
mis-diagnosing (137 = OOM kill being the most common). This adapts the
idea to Hermes' existing exit-code semantics tier:

- _interpret_signal_exit(): maps negative codes (definite signal death)
  and the 128+signum band (hedged with 'usually') to a note naming the
  signal and its likely cause, wired into _interpret_exit_code() ahead of
  the per-command semantics table.
- Curated signal table (SIGKILL/SIGSEGV/SIGTERM/SIGABRT/...) so ambiguous
  application exit codes are never mislabeled; uncurated 128+N codes stay
  silent, SIGINT is excluded (executor's interrupt-marker path owns
  rc=130).
- Notes surface via the existing exit_code_meaning result field.

E2E verified against real SIGSEGV/SIGKILL processes.

* Port from MoonshotAI/kimi-code#2596/NousResearch#2600: surface MCP tool-result _meta to the model, minus protocol-reserved keys

MCP tool results carry a server _meta mapping (exposed as .meta by the
Python SDK) alongside structuredContent. Servers return namespaced
machine-readable contracts there (validated payloads, browser-handoff
URLs); Hermes previously dropped the field entirely, so that data was
invisible to the agent.

Now _meta is included in the JSON tool output, after filtering
protocol-reserved keys per the MCP spec's key-name rules: a prefix is
reserved when a modelcontextprotocol or mcp label is followed by at
least one more label (modelcontextprotocol.io/..., tools.mcp.com/...).
Vendor namespaces with a trailing reserved word (com.example.mcp/...)
and unprefixed keys pass through. Non-serializable metadata drops the
extras rather than failing the call.

* fix(telegram): rebind TypeHandler in the deferred SDK import

`check_telegram_requirements()` re-imports python-telegram-bot after a
lazy install and rebinds the module-level aliases that the top-level
`except ImportError` block set to `typing.Any`. TypeHandler was left out
of all three places: the `global` declaration, the
`from telegram.ext import (...)` list, and the assignments.

So whenever the top-level import fails and the deferred path runs, every
other alias is restored and TELEGRAM_AVAILABLE flips to True, while
TypeHandler stays `Any`. Handler registration then raises
`TypeError: Any cannot be instantiated` and the gateway reports:

    [Telegram] Failed to connect to Telegram: Any cannot be instantiated
    Gateway started with no connected platforms

The 22.6 -> 22.8 pin bump named in NousResearch#85272 is the trigger rather than the
defect: it makes the top-level import fail, which is what routes the
module through the deferred path where the omission has always been.

* Port from aaif-goose/goose#10746: strip invisible Unicode TAG chars from MCP content

Unicode TAG characters (U+E0000-U+E007F) render as nothing in terminals
and chat UIs but are fully visible to LLM tokenizers, making them an
ASCII-smuggling prompt-injection channel for untrusted MCP servers.

- tools/ansi_strip.py: new strip_unicode_tags() with fast path; unlike
  goose we preserve valid emoji tag sequences (U+1F3F4 base + tag spec +
  U+E007F cancel), so regional flags survive.
- tools/mcp_tool.py: applied at every MCP text ingestion point — tool
  result text blocks, embedded resource text, read_resource contents,
  get_prompt message content, and tool descriptions entering the schema.
- tests/tools/test_unicode_tag_strip.py: smuggled-instruction vectors,
  goose's test vector, emoji-tag-sequence preservation, ZWJ untouched.

* feat(business-os): implement Waves 1-33 strategic enhancements and full E2E QA suite

- Add Waves 1-33 capabilities across finance, governance, GTM, SEO, and top-tier web/graphic design.
- Implement Design System Tokens, WebGL Shaders, Motion Architecture, and Skeleton Shimmer Loaders.
- Add WCAG 2.1 AAA Accessible Keyboard Focus Rings, Adaptive Breakpoints, and Fluid Typography Scalers.
- Fix pre-existing unit test timing issues and model catalog mocking in test_models, test_objective_worker, and authority_integrity.
- Create comprehensive E2E functional test suite (test_full_qa_e2e_workflow.py) verified across 381 unit & integration tests.

* chore(contributors): add contributor mapping for paul.lesyuk@gmail.com

---------

Co-authored-by: Sam Liu <sam7894604@gmail.com>
Co-authored-by: Teknium <127238744+teknium1@users.noreply.github.com>
Co-authored-by: Pavel Lesyuk <paul.lesyuk@gmail.com>
lisajlau pushed a commit to lisajlau/hermes-agent that referenced this pull request Aug 20, 2026
…s with a conservative trigger

Consolidation follow-up on top of NousResearch#59182's cherry-picked base:

- Add _looks_structured_value(): triggers a yaml.safe_load structured
  parse only when the value starts with '[' / '{' or spans multiple
  lines with YAML list-item ('- x') or mapping-entry ('key: v') shaped
  lines. Deliberately avoids the over-broad leading '-' trigger from
  NousResearch#88066 so '-5' and '--flag' stay strings.
- Stays folded INSIDE the string-typed-key guard: keys whose
  DEFAULT_CONFIG type is str (e.g. approvals.mode) are never coerced.
- Tests: multi-line YAML list/dict, string-typed key given '[x]' and
  '-5' stays string, dash-prefixed scalars stay strings, plain
  multi-line prose stays a string, load_config round-trip.
  Sabotage-verified: 7 of the suite's tests fail on main without the fix.
bobaba76 pushed a commit to bobaba76/hermes-agent that referenced this pull request Aug 27, 2026
…s with a conservative trigger

Consolidation follow-up on top of NousResearch#59182's cherry-picked base:

- Add _looks_structured_value(): triggers a yaml.safe_load structured
  parse only when the value starts with '[' / '{' or spans multiple
  lines with YAML list-item ('- x') or mapping-entry ('key: v') shaped
  lines. Deliberately avoids the over-broad leading '-' trigger from
  NousResearch#88066 so '-5' and '--flag' stay strings.
- Stays folded INSIDE the string-typed-key guard: keys whose
  DEFAULT_CONFIG type is str (e.g. approvals.mode) are never coerced.
- Tests: multi-line YAML list/dict, string-typed key given '[x]' and
  '-5' stays string, dash-prefixed scalars stay strings, plain
  multi-line prose stays a string, load_config round-trip.
  Sabotage-verified: 7 of the suite's tests fail on main without the fix.
melon-xf added a commit to melon-xf/hermes-agent that referenced this pull request Sep 3, 2026
…s with a conservative trigger

Consolidation follow-up on top of NousResearch#59182's cherry-picked base:

- Add _looks_structured_value(): triggers a yaml.safe_load structured
  parse only when the value starts with '[' / '{' or spans multiple
  lines with YAML list-item ('- x') or mapping-entry ('key: v') shaped
  lines. Deliberately avoids the over-broad leading '-' trigger from
  NousResearch#88066 so '-5' and '--flag' stay strings.
- Stays folded INSIDE the string-typed-key guard: keys whose
  DEFAULT_CONFIG type is str (e.g. approvals.mode) are never coerced.
- Tests: multi-line YAML list/dict, string-typed key given '[x]' and
  '-5' stays string, dash-prefixed scalars stay strings, plain
  multi-line prose stays a string, load_config round-trip.
  Sabotage-verified: 7 of the suite's tests fail on main without the fix.
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: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.

3 participants