morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #2368] [MERGED] fix(setup): keep Hermes config numbers as written #5192

New issue

Closed

opened 2026-10-31 01:04:41 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:04:41 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2368
Author: @Chaoqi31
Created: 10/12/2026
Status: ✅ Merged
Merged: 10/13/2026
Merged by: @morluto

Base: main ← Head: fix/hermes-yaml-exact-scalars

📝 Commits (1)

0f6fc78 fix(setup): keep Hermes config numbers as written

📊 Changes

4 files changed (+141 additions, -12 deletions)

View changed files

📝 src/application/ClientConfigurationDocument.ts (+14 -12)
➕ src/application/HermesConfigurationYaml.test.ts (+21 -0)
➕ src/application/HermesConfigurationYaml.ts (+74 -0)
📝 tests/boundary/filesystem/hermesClientConfiguration.test.ts (+32 -0)

📄 Description

Summary

rea setup and rea uninstall rewrote every number in an existing Hermes config.yaml, not only REA's own entry, and some rewrites change what Hermes reads. Discord channel IDs lost their low digits, so a discord.channel_prompts entry no longer matched its channel, and 0600 (octal 384 in YAML 1.1) became 600. Two channel IDs that round to the same double also made setup refuse a file Hermes reads without error. Setup and uninstall now leave unrelated numbers exactly as written.

No issue is linked. I found this while running setup and uninstall against synthetic Hermes configurations in a temporary HOME.

Problem and expected behavior

Smallest trigger: a Hermes config.yaml with an unquoted integer above 2^53, for example a Discord channel ID used as a channel_prompts key:

discord:
  channel_prompts:
    1187654321098765433: Build questions.

Hermes reads config.yaml through hermes_yaml.safe_load, which uses ruamel.yaml with YAML 1.1; before September 2026 it used PyYAML's safe loader. Integers stay exact, 0600 is octal, and gateway/config_loader.py stringifies channel_prompts keys (_str_keyed), so the key must stay 1187654321098765433.

serializeHermesConfiguration edited the file through the yaml package's Document API, which keeps comments, but Document.toString() prints every number from its JavaScript value. Integers above 2^53 round, 0600 is printed as the YAML 1.2 decimal 600, and small floats switch to exponent notation such as 1e-7, which YAML 1.1 does not define as a float. parseDocument read the same file with default options, so two IDs that round to one double failed as duplicate keys before setup could register. Uninstall goes through the same serializer, so the changes stayed after REA was removed.

Expected: only REA's mcp_servers.rea entry changes, and every other scalar keeps its original text.

Change and scope

Hermes YAML parsing and editing move to a new module, src/application/HermesConfigurationYaml.ts, which owns the yaml options both use. ClientConfigurationDocument calls its parseHermesConfigurationYaml and editHermesConfigurationYaml.

A customTags function wraps the built-in int and float tags. A parsed number is written with its source text while that text still reads as the node's current value; otherwise it is printed from the value. This is the rule yaml's own boolean tag follows, so a later in-place edit cannot write a stale spelling. Numbers REA creates for its own entry have no source text and print as before.
uniqueKeys treats two keys as duplicates only when both their values and their source spellings match. IDs that round to one double, and 0600 beside 600, are distinct keys to Hermes and are now accepted. A repeated key, such as a and "a", is still refused. Number keys spelled differently that Hermes reads as one value, such as 1.0 and 1.00, were refused on main; they are now left for Hermes to reject, and setup leaves their text unchanged.

Parsed values are still JavaScript numbers. REA reads only mcp_servers from the parsed object and edits the file text through the document, so rounding in unrelated parsed values does not reach the file. Other YAML and JSON client formats are unchanged. Comment placement and spacing are unchanged.

#2096 (comment-only Hermes files) edits the same two yaml calls and already conflicts with main. If this lands first, its root handling would move into editHermesConfigurationYaml.

Contribution scope: not applicable (ordinary fix within existing boundaries).

Contract and boundary impact

Semantic owner and earliest changed stage: the new HermesConfigurationYaml module, which parseDocument and serializeHermesConfiguration in src/application/ClientConfigurationDocument.ts now call.
Contribution scope fit and linked maintainer decision: ordinary fix; not applicable.
CLI and MCP/tool-catalog contract: none. Setup and uninstall report the same statuses; a file with colliding IDs is now configured instead of reported as malformed.
Provider, bridge, target-format, or platform compatibility: Hermes client configuration only.
Evidence, artifact, provenance, or reconstruction contract: not applicable.
Target selection, authority, upstream behavior, and cleanup details (if applicable): setup still writes only mcp_servers.rea, and uninstall still removes only that entry; unrelated values are no longer altered as a side effect.
Generated metadata (docs/public/product-catalog.json), package, or installation impact: none.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

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.

The checked configuration change is setup and uninstall editing Hermes config.yaml, as before; this PR narrows what that edit touches.

Evidence and regression coverage

Tests added or updated: tests/boundary/filesystem/hermesClientConfiguration.test.ts adds "keeps unrelated numbers as written, including Discord IDs". It configures a file with a Discord ID list, two channel_prompts keys one apart (they round to the same double), mode: 0600, min_score: 0.0000001 and another MCP server, then checks that the original text is an exact prefix of the result, that the REA entry reads back, and that uninstall restores the original bytes. src/application/HermesConfigurationYaml.test.ts covers what the public workflow cannot reach: an in-place number edit is written from its new value, and 0600/600 keys are accepted while a repeated key is refused.
Base reproduction or other evidence: with ClientConfigurationDocument.ts restored from main (fd958808), the boundary test fails with expected 'failed' to be 'configured'.
User-visible CLI/MCP output (if applicable): rea setup --yes --client hermes --skill=false and rea uninstall, with HOME set to a temporary directory holding:
```
discord:
  require_mention: true
  free_response_channels:
    - 1187654321098765432
  channel_prompts:
    1187654321098765433: "You answer build questions."
    1187654321098765434: "You answer release questions."
  mode: 0600
  min_score: 0.0000001
```
The Hermes lines below read the files with Hermes's own hermes_yaml.safe_load.

Before (main), setup returns needs_human and reports the file as malformed (Map keys must be unique at line 7, column 5), and uninstall reports failed. With the second prompt removed, setup succeeds but rewrites the numbers, and uninstall leaves them rewritten:
```
Hermes before: [1187654321098765432] [1187654321098765433] mode=384 min_score=1e-07
Hermes after : [1187654321098765300] [1187654321098765300] mode=600 min_score=1e-07 warnings=['MantissaNoDotYAML1_1Warning']
```
PyYAML reads that rewritten min_score as the string '1e-7'.

After, setup configures both files, the text above stays byte-for-byte ahead of the new mcp_servers.rea entry, and Hermes reads the same values:
```
Hermes before: [1187654321098765432] [1187654321098765433, 1187654321098765434] mode=384 min_score=1e-07
Hermes after : [1187654321098765432] [1187654321098765433, 1187654321098765434] mode=384 min_score=1e-07
```
Uninstall then removes the entry. Because this file had no mcp_servers key, an empty mcp_servers: {} remains, as it does on main.
Remaining proof gaps: I read the results with Hermes's hermes_yaml.py (ruamel.yaml 0.18.16, on Python 3.9 rather than Hermes's 3.11+) and with PyYAML 6.0.3, but did not start Hermes itself.

For evidence-bearing changes:

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

On the branch rebased onto main fd958808, with dependencies installed by npm ci: npm run check:fast passes.
npm run test:focused over 90 test files: every src/application/*.test.ts, the client configuration, setup, uninstall and Hermes suites in tests/boundary/filesystem/, and tests/acceptance/setup/. All 1103 tests pass.
The before/after CLI runs above, with npm run build:cached on each checkout.

Compatibility, safety, and release

Breaking changes or migration steps: none. Files that setup previously refused because of colliding IDs are now configured.
Real Hopper/Ghidra, browser, or OS coverage: macOS, real rea setup --client hermes and rea uninstall with an isolated HOME; Hermes's YAML module and PyYAML as oracles.
Package or release metadata impact: none.
Security, privacy, process, or containment review: none; the edit target and write path are unchanged.

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.

_{🔄 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/2368 **Author:** [@Chaoqi31](https://github.com/Chaoqi31) **Created:** 10/12/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/hermes-yaml-exact-scalars` --- ### 📝 Commits (1) - [`0f6fc78`](https://github.com/morluto/rea/commit/0f6fc78f7cb9f0038d5ccd41ce9128561c2fad51) fix(setup): keep Hermes config numbers as written ### 📊 Changes **4 files changed** (+141 additions, -12 deletions) <details> <summary>View changed files</summary> 📝 `src/application/ClientConfigurationDocument.ts` (+14 -12) ➕ `src/application/HermesConfigurationYaml.test.ts` (+21 -0) ➕ `src/application/HermesConfigurationYaml.ts` (+74 -0) 📝 `tests/boundary/filesystem/hermesClientConfiguration.test.ts` (+32 -0) </details> ### 📄 Description ## Summary `rea setup` and `rea uninstall` rewrote every number in an existing Hermes `config.yaml`, not only REA's own entry, and some rewrites change what Hermes reads. Discord channel IDs lost their low digits, so a `discord.channel_prompts` entry no longer matched its channel, and `0600` (octal 384 in YAML 1.1) became `600`. Two channel IDs that round to the same double also made setup refuse a file Hermes reads without error. Setup and uninstall now leave unrelated numbers exactly as written. No issue is linked. I found this while running setup and uninstall against synthetic Hermes configurations in a temporary `HOME`. ## Problem and expected behavior Smallest trigger: a Hermes `config.yaml` with an unquoted integer above 2^53, for example a Discord channel ID used as a `channel_prompts` key: ```yaml discord: channel_prompts: 1187654321098765433: Build questions. ``` Hermes reads `config.yaml` through `hermes_yaml.safe_load`, which uses ruamel.yaml with YAML 1.1; before September 2026 it used PyYAML's safe loader. Integers stay exact, `0600` is octal, and `gateway/config_loader.py` stringifies `channel_prompts` keys (`_str_keyed`), so the key must stay `1187654321098765433`. `serializeHermesConfiguration` edited the file through the `yaml` package's Document API, which keeps comments, but `Document.toString()` prints every number from its JavaScript value. Integers above 2^53 round, `0600` is printed as the YAML 1.2 decimal `600`, and small floats switch to exponent notation such as `1e-7`, which YAML 1.1 does not define as a float. `parseDocument` read the same file with default options, so two IDs that round to one double failed as duplicate keys before setup could register. Uninstall goes through the same serializer, so the changes stayed after REA was removed. Expected: only REA's `mcp_servers.rea` entry changes, and every other scalar keeps its original text. ## Change and scope Hermes YAML parsing and editing move to a new module, `src/application/HermesConfigurationYaml.ts`, which owns the `yaml` options both use. `ClientConfigurationDocument` calls its `parseHermesConfigurationYaml` and `editHermesConfigurationYaml`. - A `customTags` function wraps the built-in int and float tags. A parsed number is written with its source text while that text still reads as the node's current value; otherwise it is printed from the value. This is the rule `yaml`'s own boolean tag follows, so a later in-place edit cannot write a stale spelling. Numbers REA creates for its own entry have no source text and print as before. - `uniqueKeys` treats two keys as duplicates only when both their values and their source spellings match. IDs that round to one double, and `0600` beside `600`, are distinct keys to Hermes and are now accepted. A repeated key, such as `a` and `"a"`, is still refused. Number keys spelled differently that Hermes reads as one value, such as `1.0` and `1.00`, were refused on `main`; they are now left for Hermes to reject, and setup leaves their text unchanged. Parsed values are still JavaScript numbers. REA reads only `mcp_servers` from the parsed object and edits the file text through the document, so rounding in unrelated parsed values does not reach the file. Other YAML and JSON client formats are unchanged. Comment placement and spacing are unchanged. #2096 (comment-only Hermes files) edits the same two `yaml` calls and already conflicts with `main`. If this lands first, its root handling would move into `editHermesConfigurationYaml`. Contribution scope: not applicable (ordinary fix within existing boundaries). ## Contract and boundary impact - Semantic owner and earliest changed stage: the new `HermesConfigurationYaml` module, which `parseDocument` and `serializeHermesConfiguration` in `src/application/ClientConfigurationDocument.ts` now call. - Contribution scope fit and linked maintainer decision: ordinary fix; not applicable. - CLI and MCP/tool-catalog contract: none. Setup and uninstall report the same statuses; a file with colliding IDs is now configured instead of reported as malformed. - Provider, bridge, target-format, or platform compatibility: Hermes client configuration only. - Evidence, artifact, provenance, or reconstruction contract: not applicable. - Target selection, authority, upstream behavior, and cleanup details (if applicable): setup still writes only `mcp_servers.rea`, and uninstall still removes only that entry; unrelated values are no longer altered as a side effect. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: none. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [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. The checked configuration change is setup and uninstall editing Hermes `config.yaml`, as before; this PR narrows what that edit touches. ## Evidence and regression coverage - Tests added or updated: `tests/boundary/filesystem/hermesClientConfiguration.test.ts` adds "keeps unrelated numbers as written, including Discord IDs". It configures a file with a Discord ID list, two `channel_prompts` keys one apart (they round to the same double), `mode: 0600`, `min_score: 0.0000001` and another MCP server, then checks that the original text is an exact prefix of the result, that the REA entry reads back, and that uninstall restores the original bytes. `src/application/HermesConfigurationYaml.test.ts` covers what the public workflow cannot reach: an in-place number edit is written from its new value, and `0600`/`600` keys are accepted while a repeated key is refused. - Base reproduction or other evidence: with `ClientConfigurationDocument.ts` restored from `main` (fd958808), the boundary test fails with `expected 'failed' to be 'configured'`. - User-visible CLI/MCP output (if applicable): `rea setup --yes --client hermes --skill=false` and `rea uninstall`, with `HOME` set to a temporary directory holding: ```yaml discord: require_mention: true free_response_channels: - 1187654321098765432 channel_prompts: 1187654321098765433: "You answer build questions." 1187654321098765434: "You answer release questions." mode: 0600 min_score: 0.0000001 ``` The `Hermes` lines below read the files with Hermes's own `hermes_yaml.safe_load`. Before (`main`), setup returns `needs_human` and reports the file as malformed (`Map keys must be unique at line 7, column 5`), and uninstall reports `failed`. With the second prompt removed, setup succeeds but rewrites the numbers, and uninstall leaves them rewritten: ``` Hermes before: [1187654321098765432] [1187654321098765433] mode=384 min_score=1e-07 Hermes after : [1187654321098765300] [1187654321098765300] mode=600 min_score=1e-07 warnings=['MantissaNoDotYAML1_1Warning'] ``` PyYAML reads that rewritten `min_score` as the string `'1e-7'`. After, setup configures both files, the text above stays byte-for-byte ahead of the new `mcp_servers.rea` entry, and Hermes reads the same values: ``` Hermes before: [1187654321098765432] [1187654321098765433, 1187654321098765434] mode=384 min_score=1e-07 Hermes after : [1187654321098765432] [1187654321098765433, 1187654321098765434] mode=384 min_score=1e-07 ``` Uninstall then removes the entry. Because this file had no `mcp_servers` key, an empty `mcp_servers: {}` remains, as it does on `main`. - Remaining proof gaps: I read the results with Hermes's `hermes_yaml.py` (ruamel.yaml 0.18.16, on Python 3.9 rather than Hermes's 3.11+) and with PyYAML 6.0.3, but did not start Hermes itself. For evidence-bearing changes: - [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 - On the branch rebased onto `main` fd958808, with dependencies installed by `npm ci`: `npm run check:fast` passes. - `npm run test:focused` over 90 test files: every `src/application/*.test.ts`, the client configuration, setup, uninstall and Hermes suites in `tests/boundary/filesystem/`, and `tests/acceptance/setup/`. All 1103 tests pass. - The before/after CLI runs above, with `npm run build:cached` on each checkout. ## Compatibility, safety, and release - Breaking changes or migration steps: none. Files that setup previously refused because of colliding IDs are now configured. - Real Hopper/Ghidra, browser, or OS coverage: macOS, real `rea setup --client hermes` and `rea uninstall` with an isolated `HOME`; Hermes's YAML module and PyYAML as oracles. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none; the edit target and write path are unchanged. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:04:41 +00:00

closed this issue
added the
pull-request
label

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#5192

No description provided.

Rows
Columns