Skip to content

fix(config): parse list/mapping literals in hermes config set - #59182

Closed
sam7894604 wants to merge 1 commit into
NousResearch:mainfrom
sam7894604:fix/config-set-parse-list-mapping
Closed

fix(config): parse list/mapping literals in hermes config set#59182
sam7894604 wants to merge 1 commit into
NousResearch:mainfrom
sam7894604:fix/config-set-parse-list-mapping

Conversation

@sam7894604

Copy link
Copy Markdown
Contributor

What

hermes config set <key> <value> stored list/mapping values as raw strings. A value that looks like a list or a map — e.g. ["line","discord"] or {line: false} — was saved as a plain string instead of the structured value the config consumer expects.

Why

Structured config keys (lists of platforms, mapping overrides, etc.) could not be set from the CLI: the value round-tripped as a string, so downstream code expecting a list/dict either broke or silently ignored it, and the only workaround was hand-editing the YAML. This makes config set parse list/mapping literals before storing, so structured values can be saved naturally from the CLI.

Fixes #40545 #50168

Changes

  • hermes_cli/config.py — detect and parse list/mapping literals in config set before persisting.
  • tests/hermes_cli/test_config_set_list_values.py — new coverage for list and mapping values.

Testing

pytest tests/hermes_cli/test_config_set_list_values.py -q -> 5 passed.

Cleanly cherry-picked onto current main; touches only the two files above (no unrelated changes).

@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 P3 Low — cosmetic, nice to have labels Jul 5, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused reproduction and coverage. Current main still has the reported failure: hermes_cli/config.py:8267-8279 only coerces scalar values, while hermes_cli/tools_config.py:1661-1664 requires a native list for an explicit platform toolset.

Problems

  • website/docs/user-guide/configuring-models.md:216 currently says hermes config set only writes scalar values. This behavior change would make that statement incorrect.
  • The branch predates the current string-typed-value safeguard from e4ea0a0ed and is currently marked dirty by GitHub. Salvage must retain the guard at current hermes_cli/config.py:8268 so string settings do not regain YAML coercion.

Suggested changes

  • Update the scalar-only documentation statement and add a quoted list/mapping-literal example where useful.
  • Integrate the parser within the current non-string coercion block; the PR's persistence tests are a good base for that focused salvage.

Automated hermes-sweeper review.

@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
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
@sam7894604
sam7894604 force-pushed the fix/config-set-parse-list-mapping branch from 30d4eb1 to 859bedf Compare July 24, 2026 19:36
@sam7894604

Copy link
Copy Markdown
Contributor Author

Thanks for the review — addressed both points and rebased onto the latest main.

  • Preserve the string-typed-value guard. The parser now lives inside the existing not isinstance(_default_value_for_key(key), str) coercion block (the guard added in e4ea0a0ed) rather than running unconditionally. A genuinely string-typed setting whose value merely starts with [ or { is therefore left untouched, while non-string keys get inline JSON/YAML flow literals parsed into real lists/dicts. Existing coercion (approvals.mode, bool/int/float) is unchanged.
  • Docs. Updated website/docs/user-guide/configuring-models.md — the "config set only writes scalar values" note is no longer accurate; it now documents list/mapping support with a quoted example (platform_toolsets.line, display.tool_progress_overrides).
  • Tests. tests/hermes_cli/test_config_set_list_values.py passes (5/5), including the string-passthrough (tts.provider) and scalar cases; the broader set_config_value / config suites stay green under this change.

Rebased cleanly on top of current main.

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.
@teknium1

Copy link
Copy Markdown
Contributor

Merged via PR #88163 — your commit was cherry-picked onto current main with your authorship preserved in git log. Yours was the only variant in the 8-PR cluster that folded the structured parse inside the string-typed-key guard, which is why it was chosen as the base; on top we widened the trigger to multi-line YAML blocks while keeping dash-prefixed scalars as strings. First-submitter credit for the bug goes to @liuhao1024 (#37460). Thanks for the clean implementation!

@teknium1 teknium1 closed this Aug 17, 2026
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>
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 P3 Low — cosmetic, nice to have 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.

[Bug]: Hermes config set writes JSON object values as strings in config.yaml

3 participants