Skip to content

fix(install): Hermes hook edit no longer refuses unrelated YAML sections (#2209) - #2355

Merged
DeusData merged 1 commit into
mainfrom
fix/issue-2209
Sep 30, 2026
Merged

DeusData merged 1 commit into
mainfrom
fix/issue-2209

Conversation

@DeusData

Copy link
Copy Markdown
Owner

The hooks.pre_llm_call edit (nested-sequence YAML editor) checked every line of config.yaml and refused any flow collection or block scalar anywhere in the file. The stock Hermes config has platform_toolsets: cli: [hermes-cli], so install failed at pre_llm_hook_install (and uninstall at pre_llm_hook_uninstall), while the mcp_servers edit on the same file succeeded.

The validator now checks only the top-level section it descends into (hooks); other sections are opaque user content, matching the mapping-entry editor. The strict rules inside hooks: are unchanged, and the canonical-item self-check is still fully validated.

Review note: this deliberately narrows the #1924 / #1631 fail-closed scope from the whole file to the subtree being edited (the same precedent the mapping-entry editor already follows). The two #1924 pins now assert the refusal of |, > and [a, b] inside hooks:, where it still applies byte-identically.

  • Proof on the real stock Hermes cli-config.yaml.example (122 KB): before, op=pre_llm_hook_install error and activation stopped; after, hook installed, reinstall byte-identical, uninstall clean, and the result parses with a real YAML parser.
  • Tests: new config_yaml_edit_nested_sequence_ignores_foreign_flow_and_block_scalars_issue2209 (RED 3/3 before, RED on revert); config_yaml_edit 66/66, cli 322; make lint-ci green.

Fixes #2209

…ons (#2209)

`install` on a Hermes Agent config wrote the mcp_servers entry and the skill,
then failed op=pre_llm_hook_install on the same config.yaml. The stock Hermes
config (cli-config.yaml.example) reproduces it on its own.

Root cause: the nested-sequence editor behind the hooks.pre_llm_call item
validated EVERY line of the document and refused any flow collection or block
scalar anywhere. The stock config carries flow sequences in sections the hook
edit never reads or rewrites (`platform_toolsets:` -> `cli: [hermes-cli]`,
reporters' `plugins:` -> `enabled: [...]`). The mapping-entry editor that
writes mcp_servers into the same file already treats other top-level sections
as opaque user content, which is why that op succeeded.

Fix: yaml_sequence_validate_document now validates only the top-level section
the edit descends into (the first sequence_path key). The root mapping is
still validated at parse time, so section boundaries stay sound. An absent
section is appended as a new top-level key and touches no existing line.
Inside the edited `hooks` subtree the fail-closed rules are unchanged. This
also unblocks uninstall (op=pre_llm_hook_uninstall), which shares the analyzer.

Tests: new config_yaml_edit case with foreign flow sequences plus `|`/`>`
block scalars (install, byte-idempotent reinstall, remove). The two #1924
"still refuses" pins now place the unsupported construct inside `hooks:`,
where the refusal still applies byte-identically.

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
@DeusData
DeusData merged commit 029919c into main Sep 30, 2026
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

install on Hermes Agent: pre_llm_hook_install fails (mcp_servers + skill succeed)

1 participant