Skip to content

fix(hermes): match config items with inline comments - #1030

Merged
msitarzewski merged 2 commits into
msitarzewski:mainfrom
Tong-bit-art:fix/hermes-config-comments
Oct 6, 2026
Merged

msitarzewski merged 2 commits into
msitarzewski:mainfrom
Tong-bit-art:fix/hermes-config-comments

Conversation

@Tong-bit-art

Copy link
Copy Markdown
Contributor

What does this PR do?

ensure_hermes_plugin_enabled() in scripts/install.sh scans the
plugins.enabled / plugins.disabled item lines and compared raw line
text
against agency-agents-router, so a YAML inline comment became part
of the value it matched. In YAML a # only starts a comment when it follows
whitespace, and users do put comments on plugin entries.

Three concrete failures on a hand-editable config, all reproduced with the
real heredoc extracted from current main (8329468):

  1. Duplicate enabled entry. - agency-agents-router # our router was
    not recognized as an existing entry, so the writer appended the plugin a
    second time.
  2. Install claims success while the plugin stays off. A stale
    - agency-agents-router # temporarily off under disabled: was not
    removed, while the installer printed
    Hermes: enabled plugin agency-agents-router in <config>.
  3. Silent comment corruption. The corrupted-glue repair counted - in
    the whole line, so a comment containing - tripped it and its text
    became list entries:
# before
plugins:
  enabled:
    - chronos  # keep - rotate this one first

# after
plugins:
  enabled:
    - chronos  # keep
    - rotate this one first
    - agency-agents-router

yaml.safe_load confirms the enabled list gained the literal string
rotate this one first.

The fix strips a trailing YAML comment before matching, counting, and
splitting (strip_comment / item_value helpers), and the post-repair
re-check now recognizes a quoted entry too — - basic - "agency-agents-router"
previously produced another duplicate. Comments on untouched lines stay
because the writer only appends; nothing about the config format changes.

Agent Information (if adding/modifying an agent)

Not applicable — installer fix; no agent file added or changed.

  • Agent Name: —
  • Category: Tooling (scripts/install.sh, Hermes config writer)
  • Specialty: Idempotent edits to a user's ~/.hermes/config.yaml

Reproduction and validation

Reproduced by extracting the real heredoc from scripts/install.sh and
running it against the shapes above. What main does, per case:

enabled item with a comment (already enabled)   -> plugin appears 2x in enabled
disabled item with a comment (stale entry)      -> plugin still in disabled
comment containing " - " on an enabled item     -> enabled gains ["rotate this one first"]
corrupted glue with a quoted plugin             -> plugin appears 2x in enabled

scripts/check-hermes-config-rewrite.py (run by the Check Hermes Config
Rewrite workflow) gains four cases for these shapes plus two general
invariants:

  • the plugin appears exactly once in enabled;
  • the writer may not gain or lose unrelated enabled entries, with the
    documented legacy glue repair as the only exception.

On current main the extended suite fails with exactly the four new cases;
with this patch all 20 cases pass, including the existing 16 and their
idempotent re-runs.

Validation on the exact commit:

  • scripts/check-hermes-config-rewrite.sh: 20/20;
  • full bash suite 19/19 and Hermes plugin checks 3/3, including
    test-install.sh, test-install-hermes-destination.sh and the strict
    test-convert-outputs.sh (zero manifest drift);
  • bash -n scripts/install.sh, Python syntax and git diff --check: clean;
  • extra adversarial probes beyond the suite — comment on the plugins: key
    line, CRLF config, comment lines inside the list, quoted entries in both
    lists — all clean after the fix.

Scope notes

  • Two existing files; no generated output, checksum, new tool or CI change.
  • Comment-free configs behave exactly as before: the detection threshold is
    the same (- count after the dash instead of in the whole line) and the
    existing 16 cases still pass unchanged.

AI assistance was used in preparing this patch; the reproduction and test
runs above were executed locally.

The plugins.enabled / plugins.disabled item scan compared raw line text, so
a YAML inline comment became part of the value it matched against. Three
concrete failures followed on configs users can edit by hand:

- an existing enabled entry carrying a comment ("agency-agents-router # x")
  was not recognized, so the plugin was appended a second time;
- a stale disabled entry carrying a comment was not removed, so the
  installer reported "enabled plugin ... in config.yaml" while the plugin
  stayed disabled;
- the corrupted-glue repair counted "- " in the whole line, so a comment
  containing " - " was split into fake list entries: the installer silently
  rewrote a comment such as "# keep - rotate this one first" into two
  enabled entries.

Strip a trailing YAML comment ("#" only starts one after whitespace) before
matching, counting, and splitting, and use the same value helper in the
post-repair re-check so a quoted entry is recognized too. No config format
or install behavior change beyond those shapes.

check-hermes-config-rewrite.py gains four cases for them plus two general
invariants: the plugin appears exactly once, and the writer may not gain or
lose unrelated enabled entries outside the documented glue repair. The
suite fails on the previous code with exactly those four cases and passes
with this patch.
@phant0um

phant0um commented Oct 5, 2026

Copy link
Copy Markdown

I tested this PR against main at 8329468, which is also its base, and the head bc0dbb1. Host: macOS 27, Python 3.14.7, PyYAML 6.0.3. I extracted the heredoc from each install.sh the same way check-hermes-config-rewrite.py does and ran it on temp configs only. Each run below ran 2 times with the same result.

All three failures in the description reproduce on main and are fixed here. Resulting plugins lists, read back with yaml.safe_load:

Config main This PR
- agency-agents-router # our router under enabled enabled has the plugin 2 times once
- agency-agents-router # temporarily off under disabled plugin in both enabled and disabled removed from disabled
- chronos # keep - rotate this one first enabled gains 'rotate this one first' ['chronos', 'agency-agents-router']
- "agency-agents-router" # quoted enabled has the plugin 2 times once
plain config, no comments (control) ['chronos', 'agency-agents-router'] same

The fourth row is not in the description, but the fix covers it too.

The new test catches the bug. With scripts/install.sh from main and the test file from this PR, check-hermes-config-rewrite.py exits 1 with 4 errors across 20 cases: the three cases above plus "Corrupted glue with a quoted plugin". With the PR's install.sh it exits 0. I ran it with HOME set to an empty directory, so the optional ~/.hermes backup case did not run.

Also unchanged by this PR, and the same on both sides: a header comment (enabled: # managed by hand) and enabled: [] # none yet both work. When the plugin is the only disabled entry, removing it leaves disabled: with a null value instead of an empty list. This is not a regression. I mention it only in case Hermes expects a list there.

Not tested: a full install.sh run against a live Hermes install.

Independent verification on the PR (phant0um) covered an existing quoted
entry with an inline comment (`- "agency-agents-router"  # quoted`). The fix
recognizes it through the comment-stripped item value; pin that shape so it
cannot regress. On the previous code the extended suite fails with this case
plus the original four; with the fix all 21 cases pass.
@Tong-bit-art

Copy link
Copy Markdown
Contributor Author

Thanks for the independent verification, @phant0um — your table matches my local runs exactly, including the quoted row, and I've pinned that one in the suite.

Quoted + comment case. Added as a 21st case in check-hermes-config-rewrite.py (commit 153c280). The detection matches through the comment-stripped item value, so - "agency-agents-router" # quoted is recognized and no duplicate is appended. With this branch's install.sh the suite is 21/21; with main's install.sh it fails with exactly five errors — the original four plus this one.

disabled: becoming null. Reproduced on both sides: main at 8329468 and head 153c280 both leave

  disabled:

with no items when the plugin was the only entry, and yaml.safe_load reads that back as None — pre-existing and unchanged here, as you said. I don't have Hermes' config loader in front of me, so I can't show whether it tolerates None (plugins.get("disabled", []) then iterating would fail; plugins.get("disabled") or [] would not). If the maintainers confirm Hermes expects a list there, I'll follow up with a focused patch that leaves an empty list (disabled: []) or drops the key, plus a regression. I kept it out of this PR to stay scoped to comment-aware matching.

Same caveat as yours: a full install.sh run against a live Hermes install is outside what I can run here.

TheRealVitja pushed a commit to TheRealVitja/agency-agents that referenced this pull request Oct 6, 2026
…nts)

Curated sync of the open upstream pull requests as of 2026-10-06:

- Script and CI fixes: msitarzewski#1055, msitarzewski#1030, msitarzewski#865, msitarzewski#860, msitarzewski#967, msitarzewski#870, msitarzewski#869, msitarzewski#889,
  msitarzewski#1056, msitarzewski#755, msitarzewski#868, msitarzewski#771, msitarzewski#867; ported msitarzewski#523, msitarzewski#512 and the permissions
  part of msitarzewski#790.
- Existing-agent fixes: msitarzewski#1033-msitarzewski#1052, msitarzewski#1053, msitarzewski#1054, msitarzewski#1023, msitarzewski#756-msitarzewski#759, msitarzewski#799,
  msitarzewski#805, msitarzewski#715, msitarzewski#752, msitarzewski#784, msitarzewski#793, msitarzewski#858, msitarzewski#812, msitarzewski#789, msitarzewski#1007.
- New agents: msitarzewski#702, msitarzewski#707, msitarzewski#731, msitarzewski#732, msitarzewski#764, msitarzewski#848, msitarzewski#859, msitarzewski#862, msitarzewski#863, msitarzewski#886,
  msitarzewski#908, msitarzewski#982-msitarzewski#985, msitarzewski#1031, msitarzewski#1032.
- Docs: msitarzewski#577, msitarzewski#743, msitarzewski#762, msitarzewski#785, msitarzewski#786, msitarzewski#815, msitarzewski#816.

Fixes found while integrating: Bash 3.2 guard for the msitarzewski#755 worker argv,
Windsurf re-conversion over a stale .windsurfrules, locale-independent
check-divisions.sh, agency- prefix handling in the outputs eval, and the
India Business Navigator's YAML and headings. Resolves upstream issues
msitarzewski#229, msitarzewski#763, msitarzewski#821 and msitarzewski#1027.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NwgfpJ9tGbUh5g84u5VSgv
@msitarzewski
msitarzewski merged commit 58a6884 into msitarzewski:main Oct 6, 2026
6 checks passed
@msitarzewski

Copy link
Copy Markdown
Owner

Merged — thank you, @Tong-bit-art. Another real one, in code we wrote: the raw-line compare in the Hermes rewrite has been there since #881.

Verified in real Hermes (v0.21.5, configs written by hermes plugins enable/disable and then hand-annotated):

  • Commented disabled: entry: main's installer printed success, but hermes plugins list still reported the plugin disabled. With this PR it reports enabled.
  • Commented enabled: entry: main added a duplicate; with this PR there's a single entry.
  • A comment containing -: main split it into a bogus rotate this one first entry; with this PR the comment is left intact.
  • Plus 27/27 suites on macOS and Linux, and the 32-scenario destructive matrix unchanged.

And thank you, @phant0um, for the independent before/after run. Your question about the null disabled: was a good one: Hermes accepts it, plugins list works, and hermes plugins disable/enable still write to it correctly afterwards.

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.

3 participants