[PR #3785] [MERGED] fix(hermes): detach aliased server tables before editing #5896

Closed
opened 2026-10-31 01:10:16 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/3785
Author: @CmsChase
Created: 10/15/2026
Status: ✅ Merged
Merged: 10/22/2026
Merged by: @morluto

Base: main ← Head: fix/hermes-aliased-server-tables


📝 Commits (2)

  • f4e0ccd fix(hermes): detach aliased server tables before editing
  • 347c6cd test(setup): isolate Hermes flows from host Hopper detection

📊 Changes

3 files changed (+420 additions, -8 deletions)

View changed files

📝 src/application/HermesConfigurationYaml.test.ts (+51 -0)
📝 src/application/HermesConfigurationYaml.ts (+54 -8)
➕ tests/acceptance/setup/hermesAliasedTableSetup.test.ts (+315 -0)

📄 Description

fix(hermes): detach aliased server tables before editing

Summary

Fixes #3320. Allow approved Hermes setup and owned-registration removal to edit an aliased mcp_servers table while preserving its source and other consumers.

Problem and expected behavior

Hermes configuration admission resolves mcp_servers: *servers into a valid mapping, but the YAML writer cannot edit an Alias node as a collection. Setup can create a backup and then fail with needs_human; uninstall of an owned registration in that table also fails. Both operations should edit the selected registration without changing the referenced defaults or unrelated consumers.

Change and scope

Materialize aliased parent mappings along the edit path. Resolve copied references against the original document and remove anchors from private copies, preserving bindings even when anchor names are reused. Retain source formatting and comments; return the original bytes when a successful deletion removes no node. Cyclic and unresolved copied aliases remain rejected.

This is an ordinary configuration fix within the existing setup/uninstall boundary; an advance maintainer scope decision is not applicable. The separate anchored-registration-leaf problem in explicit tables (#3325) remains outside this change.

Contract and boundary impact

  • Semantic owner and earliest changed stage: HermesConfigurationYaml, YAML AST editing after configuration admission.
  • Contribution scope fit and linked maintainer decision: ordinary existing client configuration behavior; not applicable.
  • CLI and MCP/tool-catalog contract: aliased-table setup can reach ready/aligned, repeated setup plans no actions, and owned uninstall succeeds. No command, schema or catalog change.
  • Provider, bridge, target-format, or platform compatibility: Hermes YAML 1.1 and explicit 1.2 covered on Windows x64; no installed Hermes loader or POSIX verification.
  • Evidence, artifact, provenance, or reconstruction contract: source mappings, shared consumers, comments, exact backups and successful no-op bytes are covered; no analysis-evidence change.
  • Target selection, authority, upstream behavior, and cleanup details: existing approved setup and owned-registration uninstall determine writes. Missing registrations still skip public uninstall without a backup.
  • Generated metadata, package, or installation impact: no dependency, metadata, package-layout or installation-policy changes.

Expected effects of the changed configuration workflow:

  • Read existing files or captured data.
  • Launch or attach to a process.
  • Inject instrumentation or run scripts/code.
  • Change process memory, registers, files, or configuration.
  • Contact network services.
  • Recover protected code or work around protections.
  • Documentation or usability only; no target interaction.
  • Not sure yet; investigation or maintainer guidance needed.

Evidence and regression coverage

  • Twelve real CLI cases cover add/refresh, repeated setup, doctor, uninstall/repeated uninstall, exact backups, comments, foreign consumers, shadowed anchors and invalid-input refusal across both YAML versions. On unmodified 9a44fea3479a2ad463fdad2b14e86b412717f771, seven fail and five pass; with the fix, all twelve pass.

  • Twenty YAML owner tests pass, including ten exact-byte no-op cases for aliased/nested tables, shadowed anchors, shared consumers and explicit CRLF flow mappings. The implementation without the no-op guard fails all ten byte assertions. Upstream direct alias deletion instead throws a collection error; upstream explicit-map deletion normalizes formatting.

  • Actual CLI replay confirms aliased-table setup/repeat/doctor exits change from 1/1/1 to 0/0/0, with ready/aligned diagnostics. Eight additional baseline/fixed absent-registration uninstall calls all return exit 0 and skipped, preserving bytes without creating backups.

  • The same fixed owner/CLI selection passes 32/32 on the separately tested main revision 2a9252e4e6e04d8f24dda816ea61b76c75005e39.

  • Remaining proof gaps: installed Hermes/Python loader, real providers, POSIX, full suite/CI and complete installed-package verification.

  • Observed, derived, and inferred claims remain distinguishable.

  • Artifact identity, source provenance, and failed attempts remain preserved.

  • Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

Windows x64, Node 24.18.0, npm 11.16.0, locked dependencies:

  • npm run test:focused -- src/application/HermesConfigurationYaml.test.ts tests/acceptance/setup/hermesAliasedTableSetup.test.ts — 32 passed. The final test-group-only split was independently rerun: owner 20/20 passed.
  • npm run test:focused -- src/application/HermesConfigurationYaml.test.ts tests/boundary/filesystem/hermesClientConfiguration.test.ts tests/acceptance/setup/hermesClientSetup.test.ts — fixed 62 passed / 2 failed, including twelve new owner cases; unmodified baseline 50 passed / the same 2 failed. Complete failure assertions match: Windows drive-prefix expectations (D:\custom\agent versus \custom\agent, and D:\srv\hermes\config.yaml versus \srv\hermes\config.yaml). Existing Hermes CLI tests pass 13/13 on both.
  • npm run check — passed: typecheck, formatting, lint, knip, metadata, module boundaries and architecture guards; 0 lint errors, with only the same pre-existing timestamp-parser warning in changed files.
  • npm run verify:test-discovery — 1299 test files discovered exactly once.
  • git diff --check and the normal staged formatting/lint commit hook — passed. The committed changes are byte-identical to the reviewed diff.
  • Earlier npm run verify:package / npm run verify:package:windows runs on baseline and fixed code were blocked by Windows spawn npm ENOENT and the absent native/windows/build/manifest.json, respectively. These were not rerun after the YAML-only no-op correction; complete package verification remains unavailable.

Compatibility, safety, and release

  • Breaking changes or migration steps: none intended; valid aliased tables become editable and successful empty deletions preserve source bytes.
  • Real Hopper/Ghidra, browser, or OS coverage: isolated Windows REA CLI configurations; no installed Hermes client, live provider or POSIX claim.
  • Package or release metadata impact: unchanged; packaging limitations are recorded above.
  • Security, privacy, process, or containment review: existing admission/approval/ownership checks remain in force; invalid copied references still fail before publishing an update. No new target execution or network capability.

Author readiness

Human owner: @CmsChase

  • I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
  • I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
  • I declare this PR ready for initial maintainer review.

Before merge

  • This PR addresses a concrete problem or an agreed enhancement.
  • Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
  • The PR has one focused outcome and the title follows type(scope): outcome.
  • Related issue is linked, or the reason for not linking one is stated above.
  • Tests cover changed observable behavior and meaningful failure paths.
  • Owning docs, contracts, and generated metadata are updated where needed.
  • User-visible CLI/MCP changes include representative output.
  • I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

Maintainer blocker verification

The public Hermes acceptance comparison did not reproduce the earlier timeout concern. On this host, current main passed 7/12 cases; the prior PR passed 10/12, with the two remaining failures caused by ambient Hopper autodetection in setup. The test fixture now supplies HOPPER_LAUNCHER_PATH=process.execPath, isolating the intended setup input without changing production code. All 12 acceptance cases then passed (14.31s), as did npm run check:fast.

These are local CLI/configuration acceptance results, not installed Hermes-client or real-provider verification. Earlier Windows packaging gaps remain unverified by this pass. The former integration hold based on an unexplained timeout is superseded by the completed functional comparison.


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/3785 **Author:** [@CmsChase](https://github.com/CmsChase) **Created:** 10/15/2026 **Status:** ✅ Merged **Merged:** 10/22/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/hermes-aliased-server-tables` --- ### 📝 Commits (2) - [`f4e0ccd`](https://github.com/morluto/rea/commit/f4e0ccd1d15cdba4d8727080ef4995dfe6da9773) fix(hermes): detach aliased server tables before editing - [`347c6cd`](https://github.com/morluto/rea/commit/347c6cd294ac3df540c8eef42ba055866df20071) test(setup): isolate Hermes flows from host Hopper detection ### 📊 Changes **3 files changed** (+420 additions, -8 deletions) <details> <summary>View changed files</summary> 📝 `src/application/HermesConfigurationYaml.test.ts` (+51 -0) 📝 `src/application/HermesConfigurationYaml.ts` (+54 -8) ➕ `tests/acceptance/setup/hermesAliasedTableSetup.test.ts` (+315 -0) </details> ### 📄 Description # fix(hermes): detach aliased server tables before editing ## Summary Fixes #3320. Allow approved Hermes setup and owned-registration removal to edit an aliased `mcp_servers` table while preserving its source and other consumers. ## Problem and expected behavior Hermes configuration admission resolves `mcp_servers: *servers` into a valid mapping, but the YAML writer cannot edit an Alias node as a collection. Setup can create a backup and then fail with `needs_human`; uninstall of an owned registration in that table also fails. Both operations should edit the selected registration without changing the referenced defaults or unrelated consumers. ## Change and scope Materialize aliased parent mappings along the edit path. Resolve copied references against the original document and remove anchors from private copies, preserving bindings even when anchor names are reused. Retain source formatting and comments; return the original bytes when a successful deletion removes no node. Cyclic and unresolved copied aliases remain rejected. This is an ordinary configuration fix within the existing setup/uninstall boundary; an advance maintainer scope decision is not applicable. The separate anchored-registration-leaf problem in explicit tables (#3325) remains outside this change. ## Contract and boundary impact - Semantic owner and earliest changed stage: `HermesConfigurationYaml`, YAML AST editing after configuration admission. - Contribution scope fit and linked maintainer decision: ordinary existing client configuration behavior; not applicable. - CLI and MCP/tool-catalog contract: aliased-table setup can reach `ready`/`aligned`, repeated setup plans no actions, and owned uninstall succeeds. No command, schema or catalog change. - Provider, bridge, target-format, or platform compatibility: Hermes YAML 1.1 and explicit 1.2 covered on Windows x64; no installed Hermes loader or POSIX verification. - Evidence, artifact, provenance, or reconstruction contract: source mappings, shared consumers, comments, exact backups and successful no-op bytes are covered; no analysis-evidence change. - Target selection, authority, upstream behavior, and cleanup details: existing approved setup and owned-registration uninstall determine writes. Missing registrations still skip public uninstall without a backup. - Generated metadata, package, or installation impact: no dependency, metadata, package-layout or installation-policy changes. Expected effects of the changed configuration workflow: - [x] Read existing files or captured data. - [ ] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [x] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. ## Evidence and regression coverage - Twelve real CLI cases cover add/refresh, repeated setup, doctor, uninstall/repeated uninstall, exact backups, comments, foreign consumers, shadowed anchors and invalid-input refusal across both YAML versions. On unmodified `9a44fea3479a2ad463fdad2b14e86b412717f771`, seven fail and five pass; with the fix, all twelve pass. - Twenty YAML owner tests pass, including ten exact-byte no-op cases for aliased/nested tables, shadowed anchors, shared consumers and explicit CRLF flow mappings. The implementation without the no-op guard fails all ten byte assertions. Upstream direct alias deletion instead throws a collection error; upstream explicit-map deletion normalizes formatting. - Actual CLI replay confirms aliased-table setup/repeat/doctor exits change from `1/1/1` to `0/0/0`, with `ready`/`aligned` diagnostics. Eight additional baseline/fixed absent-registration uninstall calls all return exit 0 and `skipped`, preserving bytes without creating backups. - The same fixed owner/CLI selection passes 32/32 on the separately tested main revision `2a9252e4e6e04d8f24dda816ea61b76c75005e39`. - Remaining proof gaps: installed Hermes/Python loader, real providers, POSIX, full suite/CI and complete installed-package verification. - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed Windows x64, Node 24.18.0, npm 11.16.0, locked dependencies: - `npm run test:focused -- src/application/HermesConfigurationYaml.test.ts tests/acceptance/setup/hermesAliasedTableSetup.test.ts` — 32 passed. The final test-group-only split was independently rerun: owner 20/20 passed. - `npm run test:focused -- src/application/HermesConfigurationYaml.test.ts tests/boundary/filesystem/hermesClientConfiguration.test.ts tests/acceptance/setup/hermesClientSetup.test.ts` — fixed 62 passed / 2 failed, including twelve new owner cases; unmodified baseline 50 passed / the same 2 failed. Complete failure assertions match: Windows drive-prefix expectations (`D:\custom\agent` versus `\custom\agent`, and `D:\srv\hermes\config.yaml` versus `\srv\hermes\config.yaml`). Existing Hermes CLI tests pass 13/13 on both. - `npm run check` — passed: typecheck, formatting, lint, knip, metadata, module boundaries and architecture guards; 0 lint errors, with only the same pre-existing timestamp-parser warning in changed files. - `npm run verify:test-discovery` — 1299 test files discovered exactly once. - `git diff --check` and the normal staged formatting/lint commit hook — passed. The committed changes are byte-identical to the reviewed diff. - Earlier `npm run verify:package` / `npm run verify:package:windows` runs on baseline and fixed code were blocked by Windows `spawn npm ENOENT` and the absent `native/windows/build/manifest.json`, respectively. These were not rerun after the YAML-only no-op correction; complete package verification remains unavailable. ## Compatibility, safety, and release - Breaking changes or migration steps: none intended; valid aliased tables become editable and successful empty deletions preserve source bytes. - Real Hopper/Ghidra, browser, or OS coverage: isolated Windows REA CLI configurations; no installed Hermes client, live provider or POSIX claim. - Package or release metadata impact: unchanged; packaging limitations are recorded above. - Security, privacy, process, or containment review: existing admission/approval/ownership checks remain in force; invalid copied references still fail before publishing an update. No new target execution or network capability. ## Author readiness Human owner: @CmsChase - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. ## Before merge - [ ] This PR addresses a concrete problem or an agreed enhancement. - [ ] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [ ] The PR has one focused outcome and the title follows `type(scope): outcome`. - [ ] Related issue is linked, or the reason for not linking one is stated above. - [ ] Tests cover changed observable behavior and meaningful failure paths. - [ ] Owning docs, contracts, and generated metadata are updated where needed. - [ ] User-visible CLI/MCP changes include representative output. - [ ] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. <!-- maintainer-blocker-review --> ### Maintainer blocker verification The public Hermes acceptance comparison did not reproduce the earlier timeout concern. On this host, current main passed 7/12 cases; the prior PR passed 10/12, with the two remaining failures caused by ambient Hopper autodetection in setup. The test fixture now supplies `HOPPER_LAUNCHER_PATH=process.execPath`, isolating the intended setup input without changing production code. All 12 acceptance cases then passed (14.31s), as did `npm run check:fast`. These are local CLI/configuration acceptance results, not installed Hermes-client or real-provider verification. Earlier Windows packaging gaps remain unverified by this pass. The former integration hold based on an unexplained timeout is superseded by the completed functional comparison. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-10-31 01:10:16 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#5896
No description provided.