morluto/rea

Fork 0

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

[GH-ISSUE #2514] fix(setup): unblock comment-only and marker-only Hermes configs #830

New issue

Closed

opened 2026-10-15 18:28:49 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-15 18:28:49 +00:00

Owner

Originally created by @coygeek on GitHub (Oct 13, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/2514

Summary

A Hermes config.yaml containing only a comment or a YAML document marker cannot receive the REA registration. The setup workflow reports that the configuration is malformed, although it contains no settings to preserve. The same file also blocks uninstall before removal starts. Whitespace-only files already work, and adding an empty mcp_servers mapping restores both workflows while retaining the comment.

Runtime or environment

REA failure verified at a2d8b94dd5 and still present at 8b181716ac, package version 6.4.0, Node.js v26.10.0, npm 11.19.1, and locked dependencies including yaml 2.9.0 and Vitest 5.0.3. Verification used isolated filesystem fixtures and source application workflows. No Hermes runtime or CLI subprocess was executed.

Steps to reproduce

At the stated baseline, save this test as src/application/hermesEmptyYaml.test.ts and run the focused command below. Each fixture creates a separate home, resolves the supported Hermes configuration path, and writes only the indicated comment or document marker. It invokes the production inspection, configuration, and uninstall workflows without changing a real client profile.

import { mkdtemp, mkdir, writeFile, rm } from "node:fs/promises";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { expect, it } from "vitest";
import { supportedClients } from "./SupportedClients.js";
import {
  configureClientConfiguration,
  inspectClientConfiguration,
} from "./SetupClientConfiguration.js";
import { runUninstall, systemUninstallHost } from "./Uninstall.js";

for (const original of ["# Hermes configuration\n", "---\n"]) {
  it(`configures an empty YAML document ${JSON.stringify(original)}`, async () => {
    const home = await mkdtemp(join(tmpdir(), "rea-empty-yaml-"));
    try {
      const client = supportedClients(home, process.platform, {})
        .find(({ name }) => name === "hermes")!;
      await mkdir(client.markerPath!, { recursive: true });
      await writeFile(client.configPath, original);
      const command = ["rea", "mcp"];
      const inspection = await inspectClientConfiguration(client, {}, command);
      const configured = await configureClientConfiguration(client, {}, command);
      const uninstall = await runUninstall(
        false, systemUninstallHost(home, undefined, {}),
      );
      console.log({ inspection, configured, uninstall });
      expect(configured.status).toBe("configured");
    } finally {
      await rm(home, { recursive: true, force: true });
    }
  });
}

npm run test:focused -- src/application/hermesEmptyYaml.test.ts

The same input reaches the shared workflows from rea setup --client hermes and rea uninstall. This CLI reachability was checked from source rather than through subprocess execution. Uninstall has no client-selection option.

Expected behavior

Setup should add an enabled REA entry under mcp_servers to a Hermes configuration with no settings, including one that contains only comments or a document marker. It should retain the existing comment. Uninstall should treat an empty configuration with no REA entry as having nothing to remove and continue its preflight, as it does for other empty documents.

Actual behavior

Each of the comment-only, marker-only, and marker-plus-comment inputs returned an invalid inspection with the diagnostic "The configuration root must be an object". Configuration returned status failed with reason readback. Uninstall returned status failed and a failed Hermes item before any removal. Both operations left the original bytes unchanged. These results were independently observed through the production filesystem workflows. Source tracing shows that setup projects the invalid inspection to needs_human.

Whitespace-only input configured successfully and uninstalled successfully. A comment followed by mcp_servers: {} also worked and kept the comment, demonstrating a practical workaround. A comment followed by an unrelated model setting worked and retained that setting after uninstall.

Evidence

Expected source: docs/client-configuration.md:143-147 promises Hermes registration with comment and unrelated-setting preservation; src/application/ClientConfigurationDocument.ts:98-118 defines no-settings documents; tests/boundary/filesystem/hermesClientConfiguration.test.ts:167-215,252-274 covers comments and empty files.
Failure source: src/application/ClientConfigurationDocument.ts:118-121,147-148 rejects the null result produced for otherwise empty YAML with ClientConfigurationShapeError, a TypeError subclass; src/application/ClientConfigurationFiles.ts:57-67 reports malformed configuration; src/application/SetupPlan.ts:139-162 blocks setup; src/application/Uninstall.ts:103-127,265-292 blocks uninstall. The inline filesystem reproduction exposes the contrary results.
Evidence provenance: observed
Local verification: reproduced
Reproduction completeness: complete

The focused verification passed seven observation checks covering the three failing inputs, the whitespace control, both mapping controls, and direct parser/serializer behavior. A direct serialization call with a marker-only original separately throws "Expected a YAML collection as document contents" at src/application/ClientConfigurationDocument.ts:1422-1425. Comment-only serialization succeeds. The marker serialization failure is a restoration constraint because the current setup path stops at parsing first.

Related reports are reference-only. The Hermes first-run warning report concerns configuration version warnings after creation. The closed configuration-shape diagnostic report corrected how invalid structures are described. Neither establishes a fix for acceptance of otherwise empty Hermes YAML.

Restoration check

Repeat the comment-only, marker-only, and marker-plus-comment cases. Inspection should allow registration, configuration should write an enabled REA entry with command rea and args [mcp], and readback should confirm the entry. Preserve the original comment when present. Also run uninstall directly against each original empty document before adding REA; it should skip that absent registration and allow the remaining operation to complete. After successful registration, uninstall should remove REA while preserving unrelated content.

Keep whitespace-only configuration working and retain comments and unrelated settings in existing mappings. Verify that malformed YAML and nonempty scalar or sequence roots still receive an appropriate error. The marker case must complete serialization as well as parsing; accepting the root alone does not restore registration.

Triage assessment

Impact level: P2
Assessment status: supported
Impact basis: A bounded class of empty Hermes configurations blocks REA registration and the global uninstall preflight. Existing data remains unchanged, and a practical workaround is available. The frequency of these inputs among users is unknown.
Workaround status: available
Workaround basis: Adding mcp_servers: {} after the existing comment was tested; configuration then succeeded, uninstall completed, and the comment survived both operations.

Originally created by @coygeek on GitHub (Oct 13, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/2514 ## Summary A Hermes config.yaml containing only a comment or a YAML document marker cannot receive the REA registration. The setup workflow reports that the configuration is malformed, although it contains no settings to preserve. The same file also blocks uninstall before removal starts. Whitespace-only files already work, and adding an empty mcp_servers mapping restores both workflows while retaining the comment. ## Runtime or environment REA failure verified at a2d8b94dd50dab8d966239dd63ab84718aa8b4b4 and still present at 8b181716aca2cb467be499cba226abfb5ae19141, package version 6.4.0, Node.js v26.10.0, npm 11.19.1, and locked dependencies including yaml 2.9.0 and Vitest 5.0.3. Verification used isolated filesystem fixtures and source application workflows. No Hermes runtime or CLI subprocess was executed. ## Steps to reproduce At the stated baseline, save this test as src/application/hermesEmptyYaml.test.ts and run the focused command below. Each fixture creates a separate home, resolves the supported Hermes configuration path, and writes only the indicated comment or document marker. It invokes the production inspection, configuration, and uninstall workflows without changing a real client profile. ```ts import { mkdtemp, mkdir, writeFile, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { expect, it } from "vitest"; import { supportedClients } from "./SupportedClients.js"; import { configureClientConfiguration, inspectClientConfiguration, } from "./SetupClientConfiguration.js"; import { runUninstall, systemUninstallHost } from "./Uninstall.js"; for (const original of ["# Hermes configuration\n", "---\n"]) { it(`configures an empty YAML document ${JSON.stringify(original)}`, async () => { const home = await mkdtemp(join(tmpdir(), "rea-empty-yaml-")); try { const client = supportedClients(home, process.platform, {}) .find(({ name }) => name === "hermes")!; await mkdir(client.markerPath!, { recursive: true }); await writeFile(client.configPath, original); const command = ["rea", "mcp"]; const inspection = await inspectClientConfiguration(client, {}, command); const configured = await configureClientConfiguration(client, {}, command); const uninstall = await runUninstall( false, systemUninstallHost(home, undefined, {}), ); console.log({ inspection, configured, uninstall }); expect(configured.status).toBe("configured"); } finally { await rm(home, { recursive: true, force: true }); } }); } ``` ```sh npm run test:focused -- src/application/hermesEmptyYaml.test.ts ``` The same input reaches the shared workflows from rea setup --client hermes and rea uninstall. This CLI reachability was checked from source rather than through subprocess execution. Uninstall has no client-selection option. ## Expected behavior Setup should add an enabled REA entry under mcp_servers to a Hermes configuration with no settings, including one that contains only comments or a document marker. It should retain the existing comment. Uninstall should treat an empty configuration with no REA entry as having nothing to remove and continue its preflight, as it does for other empty documents. ## Actual behavior Each of the comment-only, marker-only, and marker-plus-comment inputs returned an invalid inspection with the diagnostic "The configuration root must be an object". Configuration returned status failed with reason readback. Uninstall returned status failed and a failed Hermes item before any removal. Both operations left the original bytes unchanged. These results were independently observed through the production filesystem workflows. Source tracing shows that setup projects the invalid inspection to needs_human. Whitespace-only input configured successfully and uninstalled successfully. A comment followed by mcp_servers: {} also worked and kept the comment, demonstrating a practical workaround. A comment followed by an unrelated model setting worked and retained that setting after uninstall. ## Evidence - Expected source: docs/client-configuration.md:143-147 promises Hermes registration with comment and unrelated-setting preservation; src/application/ClientConfigurationDocument.ts:98-118 defines no-settings documents; tests/boundary/filesystem/hermesClientConfiguration.test.ts:167-215,252-274 covers comments and empty files. - Failure source: src/application/ClientConfigurationDocument.ts:118-121,147-148 rejects the null result produced for otherwise empty YAML with `ClientConfigurationShapeError`, a `TypeError` subclass; src/application/ClientConfigurationFiles.ts:57-67 reports malformed configuration; src/application/SetupPlan.ts:139-162 blocks setup; src/application/Uninstall.ts:103-127,265-292 blocks uninstall. The inline filesystem reproduction exposes the contrary results. - Evidence provenance: observed - Local verification: reproduced - Reproduction completeness: complete The focused verification passed seven observation checks covering the three failing inputs, the whitespace control, both mapping controls, and direct parser/serializer behavior. A direct serialization call with a marker-only original separately throws "Expected a YAML collection as document contents" at src/application/ClientConfigurationDocument.ts:1422-1425. Comment-only serialization succeeds. The marker serialization failure is a restoration constraint because the current setup path stops at parsing first. Related reports are reference-only. [The Hermes first-run warning report](https://github.com/morluto/rea/issues/1633) concerns configuration version warnings after creation. [The closed configuration-shape diagnostic report](https://github.com/morluto/rea/issues/2152) corrected how invalid structures are described. Neither establishes a fix for acceptance of otherwise empty Hermes YAML. ## Restoration check Repeat the comment-only, marker-only, and marker-plus-comment cases. Inspection should allow registration, configuration should write an enabled REA entry with command rea and args [mcp], and readback should confirm the entry. Preserve the original comment when present. Also run uninstall directly against each original empty document before adding REA; it should skip that absent registration and allow the remaining operation to complete. After successful registration, uninstall should remove REA while preserving unrelated content. Keep whitespace-only configuration working and retain comments and unrelated settings in existing mappings. Verify that malformed YAML and nonempty scalar or sequence roots still receive an appropriate error. The marker case must complete serialization as well as parsing; accepting the root alone does not restore registration. ## Triage assessment - Impact level: P2 - Assessment status: supported - Impact basis: A bounded class of empty Hermes configurations blocks REA registration and the global uninstall preflight. Existing data remains unchanged, and a practical workaround is available. The frequency of these inputs among users is unknown. - Workaround status: available - Workaround basis: Adding mcp_servers: {} after the existing comment was tested; configuration then succeeded, uninstall completed, and the comment survived both operations.

bsakaguchi

2026-10-15 18:28:49 +00:00

closed this issue
added the
bug
label

bsakaguchi commented

2026-10-15 18:28:50 +00:00

Author

Owner

@morluto commented on GitHub (Oct 13, 2026):

Verified during the bug/fix-PR audit. Comment-only and marker-only YAML roots are handled as editable empty mappings by the current Hermes YAML owner.

Current implementation: src/application/HermesConfigurationYaml.ts. Validation on macOS arm64 with Node.js 24.18.0: npm run test:local -- src/application/HermesConfigurationYaml.test.ts src/ida/IdaSessionClient.test.ts src/native/NativeMachoInspection.test.ts src/browser/PlaywrightElectronActiveProvider.test.ts passed (84 tests). This is source/provider-boundary regression coverage; no real IDA GUI session was launched for this check.

Closing because the reported behavior is already addressed on main.

@morluto commented on GitHub (Oct 13, 2026): Verified during the bug/fix-PR audit. Comment-only and marker-only YAML roots are handled as editable empty mappings by the current Hermes YAML owner. Current implementation: `src/application/HermesConfigurationYaml.ts`. Validation on macOS arm64 with Node.js 24.18.0: `npm run test:local -- src/application/HermesConfigurationYaml.test.ts src/ida/IdaSessionClient.test.ts src/native/NativeMachoInspection.test.ts src/browser/PlaywrightElectronActiveProvider.test.ts` passed (84 tests). This is source/provider-boundary regression coverage; no real IDA GUI session was launched for this check. Closing because the reported behavior is already addressed on main.

bsakaguchi referenced this issue

2026-10-16 21:16:33 +00:00

[PR #830] [MERGED] refactor(domain): extract artifact path syntax helpers #2270

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

No description provided.

Rows
Columns