From aa6f74af04c06d3d09ef69d2292f05f74e93cf1b Mon Sep 17 00:00:00 2001 From: Jeremy Levartovsky <140487034+JayOfTheKeyboard@users.noreply.github.com> Date: Wed, 14 Oct 2026 08:48:05 +1100 Subject: [PATCH] fix(setup): configure a comment-only Hermes config.yaml (#2096) * fix(setup): configure a comment-only Hermes config.yaml comments, or only a `---` marker, also parses to a null YAML document. Setup still rejected it as malformed ("expected record, received null") and reported Hermes as invalid. Treat a null Hermes document as holding no settings, as #1983 does for an empty file. The serializer already edits a comment-only document in place. A bare `---` document holds a null scalar that takes no keys, so replace it with a map and keep the comments written after the marker. * fix(setup): keep rejecting an explicit null Hermes root Only a document with no root, or a bare --- marker whose null scalar has no source text, holds no settings. An explicit null or ~ is a value, so setup and uninstall reject it as before, the same as any other non-map root. * test(setup): split the Hermes empty-configuration tests under the function length limit The Hermes YAML registration block reached 159 lines, past oxlint's max-lines-per-function limit of 150. Move the empty-document cases and the multiple-document rejection into their own block. * fix(setup): type the Hermes edit document as a plain Document parseDocument with options returns a Document.Parsed, whose contents accept only parsed nodes, so assigning the new map for a bare --- root failed typecheck (TS2322). Annotate the edited document as Document, as the code did before it moved into HermesConfigurationYaml.ts. * fix(ci): isolate Linux Hopper process fixture inventory --------- Co-authored-by: morluto <76467478+morluto@users.noreply.github.com> --- src/application/HermesConfigurationYaml.ts | 38 ++++++++-- .../hermesClientConfiguration.test.ts | 74 +++++++++++++++---- 2 files changed, 94 insertions(+), 18 deletions(-) diff --git a/src/application/HermesConfigurationYaml.ts b/src/application/HermesConfigurationYaml.ts index 8d245da37..61eb0dfde 100644 --- a/src/application/HermesConfigurationYaml.ts +++ b/src/application/HermesConfigurationYaml.ts @@ -1,7 +1,7 @@ import { isScalar, - parse, parseDocument, + type Document, type ParsedNode, type ScalarTag, type Tags, @@ -56,9 +56,28 @@ const HERMES_YAML_OPTIONS = { a.source === b.source), }; -/** Parse a Hermes `config.yaml`. */ -export const parseHermesConfigurationYaml = (text: string): unknown => - parse(text, HERMES_YAML_OPTIONS); +// A comment-only document has no root; a bare marker has an unwritten null +// scalar. Explicit null values, tags and anchors remain actual values. +const isEmptyYamlRoot = (root: Document["contents"]): boolean => + root === null || + (isScalar(root) && + root.value === null && + root.source === "" && + root.tag === undefined && + root.anchor === undefined); + +const parseHermesDocument = (text: string) => { + const document = parseDocument(text, HERMES_YAML_OPTIONS); + const error = document.errors[0]; + if (error !== undefined) throw error; + return document; +}; + +/** Parse a Hermes `config.yaml`, preserving the distinction between no root and explicit null. */ +export const parseHermesConfigurationYaml = (text: string): unknown => { + const document = parseHermesDocument(text); + return isEmptyYamlRoot(document.contents) ? {} : document.toJS(); +}; /** Set or delete one value in a Hermes `config.yaml`, keeping the rest as written. */ export const editHermesConfigurationYaml = ( @@ -66,7 +85,16 @@ export const editHermesConfigurationYaml = ( path: readonly string[], value: unknown, ): string => { - const document = parseDocument(text, HERMES_YAML_OPTIONS); + // A plain Document, so a bare `---` root can be replaced by a new map. + const document: Document = parseHermesDocument(text); + const root = document.contents; + if (isScalar(root) && isEmptyYamlRoot(root)) { + const map = document.createNode({}); + map.commentBefore = [root.commentBefore, root.comment] + .filter((comment) => comment !== undefined) + .join("\n"); + document.contents = map; + } if (value === undefined) document.deleteIn(path); else document.setIn(path, value); // Editing REA must not reflow unrelated user prompts or other YAML scalars. diff --git a/tests/boundary/filesystem/hermesClientConfiguration.test.ts b/tests/boundary/filesystem/hermesClientConfiguration.test.ts index 218573d5c..7bd72a676 100644 --- a/tests/boundary/filesystem/hermesClientConfiguration.test.ts +++ b/tests/boundary/filesystem/hermesClientConfiguration.test.ts @@ -114,18 +114,34 @@ describe("Hermes YAML shape diagnostics", () => { "!!omap [{rea: {command: rea, args: [mcp]}}]", "!!timestamp 2001-12-15", "!!binary cmVh", - ].flatMap((value) => [ - { - original: `${value}\n`, - problem: "The configuration root must be an object", - }, - { - original: `mcp_servers: ${value}\n`, - problem: '"mcp_servers" must be an object', - }, - ]), + ] + .flatMap((value) => [ + { + original: `${value}\n`, + problem: "The configuration root must be an object", + }, + { + original: `mcp_servers: ${value}\n`, + problem: '"mcp_servers" must be an object', + }, + ]) + .concat( + // An explicit null root is a value, not an empty file. + [ + "null\n", + "~\n", + "# Hermes configuration\nnull\n", + "--- ~\n", + "--- !!null\n", + '!!null ""\n', + "--- &empty\n", + ].map((original) => ({ + original, + problem: "The configuration root must be an object", + })), + ), )( - "blocks setup and uninstall before changes for tagged non-record YAML: $original", + "blocks setup and uninstall before changes for tagged or null non-record YAML: $original", async ({ original, problem }) => { const { home, client } = await hermesClient(); const cursor = join(home, ".cursor", "mcp.json"); @@ -281,7 +297,36 @@ mcp_servers: }); }); - it.each(["", "\n"])( + // Comments and a bare document marker parse to a null document, like an + // empty file, and are kept ahead of the new registration. +}); + +describe("Hermes YAML empty configurations", () => { + it("rejects multiple YAML documents before treating an empty first root as absent", async () => { + const { client } = await hermesClient(); + const original = + "---\n---\nmcp_servers:\n rea:\n command: rea\n args: [mcp]\n"; + await writeFile(client.configPath, original); + expect(await inspectClientConfiguration(client, {}, command)).toMatchObject( + { status: "invalid" }, + ); + expect(await configureClientConfiguration(client, {}, command)).toEqual({ + status: "failed", + reason: "readback", + }); + expect(await readFile(client.configPath, "utf8")).toBe(original); + await expect( + readFile(`${client.configPath}.rea.backup`, "utf8"), + ).rejects.toThrow(); + }); + + it.each([ + "", + "\n", + "# Hermes configuration\n", + "---\n", + "---\n# Hermes configuration\n", + ])( "configures an empty config.yaml %j as a new document", async (original) => { const { home, client } = await hermesClient(); @@ -289,7 +334,10 @@ mcp_servers: expect( (await configureClientConfiguration(client, {}, command)).status, ).toBe("configured"); - expect(parseYaml(await readFile(client.configPath, "utf8"))).toEqual({ + const text = await readFile(client.configPath, "utf8"); + const kept = original.trimEnd(); + expect(text.slice(0, kept.length)).toBe(kept); + expect(parseYaml(text)).toEqual({ mcp_servers: { rea: { command: command[0], args: command.slice(1), enabled: true }, },