fix(install): Hermes YAML constructs, goose required name, annotated MCP entry repair (#1631, #1675, #1630) - #1683
Merged
Merged
Conversation
Four legal-YAML constructs from the reporters' real Hermes configs made
`install` fail permanently (any one of them aborted mcp_install and/or
pre_llm_hook_install):
- exact empty flow collections as values (`plugins: []`, `tool_choice: {}`)
— now validated key-only in both the mapping-body and sequence document
scans, mirroring the #1673 empty-mapping exception; non-empty flow
collections stay rejected.
- block sequences at the same indent as their mapping key (column-0 `- item`)
— item lines directly after a value-less key are structure, not malformed
keys, in the root walker, the key matcher, and the sequence mapping-range
walker.
- double-quoted scalars continued across lines with a trailing `\` — the doc
loader now precomputes per-line continuation flags; continuation lines are
value bytes every structural walker skips, and a document ending inside an
open continuation stays an error.
- mid-word quote characters in plain scalars (`LET'S`) — quotes are scalar
indicators only at a node start (range start, after `:`, after `-`),
exactly like the #1639 anchor/alias rule; real quoted values keep their
protection.
Byte-identity alone also froze users on canonicals older releases wrote:
galaxy's entry had `command:` unquoted, and the goose block gained `name:`
(#1675), so the existing entry was declared FOREIGN forever. An entry under
our key now repairs when it parses as a known prior shape (single command
line, or the pre-name goose block) with a codebase-memory-mcp[.exe] command
basename; anything else stays FOREIGN and the file untouched.
End-to-end: both reporters' full configs (iandol 15.6 KB, galaxy 15.2 KB) now
install with zero agent_config errors, every original line byte-preserved,
and the goose upgrade path rewrites the old block in place. Each construct
carries a distilled regression test proven RED on the unfixed editor.
Fixes #1631.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
… MCP entries Two install failures with the same root theme — the entry we write is a compatibility contract with the agent's parser, and both sides of that contract needed repair: goose (#1675): ExtensionConfig::Stdio declares `name` as a required serde field with no default, and goose's loader silently drops entries that fail to deserialize — install reported success and the extension was invisible. The goose block now carries `name: codebase-memory-mcp`; the non-goose YAML schema stays name-free. A CBM_CLI_ENABLE_TEST_API seam asserts the exact block bytes per schema. annotated MCP entries (#1630, the deferred field-merge): dbd20ea recognised an entry the client annotated ("enabled": true beside our command/type) but could only leave it untouched, because replacing the whole entry would drop the client's keys. config_json_like gains cbm_json_like_replace_field_raw_if_unchanged — splice ONE member's value, preserving every other byte (comments, ordering, client keys) — and the upsert flow uses it on the two AUTHORIZED repair channels only: - a relocating update (the entry names the previous managed binary), and - the existing Windows dead-path probe, which previously fell back to a wholesale rewrite and lost the annotations. POSIX keeps its doctrine unchanged: a config-supplied path is never trusted, so a moved-looking entry without that authority is preserved byte-for-byte and install fails loudly (cli_editor_mcp_preserves_unrecorded_posix_absolute_ entries_without_probe holds). All repair/refusal paths are covered by tests proven RED on the unfixed flow. Fixes #1675. Fixes #1630. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
PowerShell 5.1's `Set-Content -Encoding UTF8` writes a BOM, so real Windows-authored Hermes configs start with EF BB BF — and both edit ops failed content-independently (the reporter's 26-byte reproduction is their 23-byte file plus exactly this BOM; reproduced RED on macOS with the same bytes, so the platform was never the variable). The document read now validates past a leading BOM and yaml_doc_init treats it as a prologue: the first line's structure starts after it, the key lookup still sees our own section when the BOM immediately precedes it (guarded by a dedicated no-duplicate-section test), and every edit splices interior ranges, so the BOM survives writes byte-for-byte. Non-document inputs — keys, entry blocks, identity scalars — keep the strict no-BOM rule. Also makes the moved-entry cli test fixture platform-correct: the Windows dead-path probe can only prove a fixed-drive path absent, so the Windows branch uses one; a POSIX-shaped path is refused there by design. Fixes #1656. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
…rship (#1582) gotspatel's live opencode.json stores our entry with backslashes (`C:\...\codebase-memory-mcp.exe`) while the installer compares its own path with forward slashes — the same file on disk, refused over the separator spelling, so op=mcp_install failed on a correctly-installed machine (and on Windows the dead-path probe rightly reported the binary PRESENT, which turned the mismatch into a hard refusal). Ownership comparison now treats `\` and `/` as equal everywhere and folds case on Windows only, where the filesystem is case-insensitive; POSIX byte-exactness otherwise holds. The annotated entry that names this binary is recognised as already satisfied and preserved byte-for-byte. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
#1664) Codex sanitizes stdio MCP subprocess environments to the names listed in env_vars. Since #1645 CBM_RUNTIME_DIR relocates the daemon rendezvous, so a Codex subprocess that does not receive it looks for the daemon in the DEFAULT location and never finds it — the same silent client/daemon split CBM_CACHE_DIR caused in #1562. Both names decide WHICH daemon a process talks to and are now forwarded unconditionally (forward-if-present semantics); behavioural knobs (log level, workers, budgets) deliberately stay unforwarded — that broader list remains #1664's open enhancement question. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This was referenced Aug 17, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the install-degradation cluster for 0.10.6: #1631 (both reporters' real Hermes configs now install end-to-end with zero
agent_configerrors and every original byte preserved), #1675 (goose entries carry the serde-requirednameand old blocks repair on upgrade), and #1630's deferred field-merge (annotated MCP entries repair on the authorized channels without dropping client keys; POSIX fail-closed doctrine unchanged).fix(yaml) — four legal-YAML constructs the strict editor refused, each distilled from the reporters' files into a RED-proven regression test: exact empty flow collections (
[]/{}) as values, column-0 block sequences,\-continued double-quoted scalars (doc-level continuation flags; every structural walker skips continuation lines), and mid-word quote characters in plain scalars (quotes are indicators only at a node start, like the #1639 anchor rule). Plus owned-entry repair for shapes our own past writers produced (unquotedcommand:, pre-namegoose blocks) with acodebase-memory-mcp[.exe]basename; anything else stays FOREIGN, byte-untouched.fix(cli) — the goose block writer emits
name:(with a test seam asserting exact block bytes per schema), andconfig_json_likegainscbm_json_like_replace_field_raw_if_unchangedfor single-member splices that preserve comments and client-added keys. Used only where repair is authorized: a relocating update (entry names the previous managed binary) and the existing Windows dead-path probe (which previously rewrote wholesale and dropped annotations).Verification: full macOS suite green on the final tree;
config_yaml_edit+clisuites 336/336; lint-ci + no-skips green; E2E matrix green (4 construct minimals, both reporter gists, goose upgrade, OpenCode jsonc probes). Every fix RED-before and RED-on-revert.🤖 Generated with Claude Code
https://claude.ai/code/session_018RiRAw9RQhvCoshqe7eZHV