nprasad2077/codebase-memory-mcp
1
0 Fork 0
mirror of https://github.com/DeusData/codebase-memory-mcp.git synced 2026-11-04 10:48:20 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(install): Hermes hook edit no longer refuses unrelated YAML sections (#2209)

Browse Source 
`install` on a Hermes Agent config wrote the mcp_servers entry and the skill,
then failed op=pre_llm_hook_install on the same config.yaml. The stock Hermes
config (cli-config.yaml.example) reproduces it on its own.

Root cause: the nested-sequence editor behind the hooks.pre_llm_call item
validated EVERY line of the document and refused any flow collection or block
scalar anywhere. The stock config carries flow sequences in sections the hook
edit never reads or rewrites (`platform_toolsets:` -> `cli: [hermes-cli]`,
reporters' `plugins:` -> `enabled: [...]`). The mapping-entry editor that
writes mcp_servers into the same file already treats other top-level sections
as opaque user content, which is why that op succeeded.

Fix: yaml_sequence_validate_document now validates only the top-level section
the edit descends into (the first sequence_path key). The root mapping is
still validated at parse time, so section boundaries stay sound. An absent
section is appended as a new top-level key and touches no existing line.
Inside the edited `hooks` subtree the fail-closed rules are unchanged. This
also unblocks uninstall (op=pre_llm_hook_uninstall), which shares the analyzer.

Tests: new config_yaml_edit case with foreign flow sequences plus `|`/`>`
block scalars (install, byte-idempotent reinstall, remove). The two #1924
"still refuses" pins now place the unsupported construct inside `hooks:`,
where the refusal still applies byte-identically.

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This commit is contained in:
Martin Vogel committed 2026-09-25 22:56:39 +02:00
1 parent 64c23fab6c
commit ae578bc1cc
2 files changed
+86 -12
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

src/cli/config_yaml_edit.c
+23 -3
View File
@@ -2698,8 +2698,28 @@ static int yaml_sequence_line_has_unsupported(const yaml_doc_t *doc, const yaml_
return quote != '\0' ? YAML_MATCH : 0;
}
static int yaml_sequence_validate_document(const yaml_doc_t *doc) {
for (size_t i = 0U; i < doc->line_count; i++) {
/* Validates only the top-level section the edit descends into. The root
* mapping itself was already validated when the document was parsed, so the
* section boundaries are sound; every OTHER top-level section is opaque user
* content, the same contract the mapping-entry editor applies. Validating the
* whole document refused the stock Hermes config for flow sequences such as
* `cli: [hermes-cli]` under `platform_toolsets:`, which the hook edit never
* reads or rewrites (#2209). An absent root key means the sequence is appended
* as a new top-level section, which touches no existing line. */
static int yaml_sequence_validate_document(const yaml_doc_t *doc, const char *root_key,
size_t root_key_len) {
bool found = false;
size_t section_line = 0U;
size_t section_colon = 0U;
if (yaml_find_unique_key(doc, 0U, root_key, root_key_len, &found, &section_line,
&section_colon) != 0) {
return YAML_ERROR;
}
if (!found) {
return 0;
}
size_t section_end = yaml_top_level_section_end(doc, section_line);
for (size_t i = section_line; i < section_end; i++) {
const yaml_line_t *line = &doc->lines[i];
if (line->blank || line->comment || line->dquote_cont) {
continue;
@@ -2947,7 +2967,7 @@ static int yaml_sequence_analyze(const yaml_doc_t *doc, const char *const *seque
const char *identity_key, const char *identity_value,
yaml_mapping_sequence_target_t *target) {
memset(target, 0, sizeof(*target));
if (yaml_sequence_validate_document(doc) != 0) {
if (yaml_sequence_validate_document(doc, sequence_path[0], path_lengths[0]) != 0) {
return YAML_ERROR;
}
size_t parent_begin = 0U;
tests/test_config_yaml_edit.c
+63 -9
View File
@@ -1228,12 +1228,14 @@ TEST(config_yaml_edit_hermes_accepts_interior_brackets_in_plain_scalar_issue1924
TEST(config_yaml_edit_hermes_hook_still_refuses_block_scalar_value_issue1924) {
/* A `>` or `|` that BEGINS a value is a real block scalar whose
* indentation contract this editor does not model: the fail-closed
* refusal stays, and the file stays byte-identical. */
* indentation contract this editor does not model: inside the `hooks`
* subtree the edit descends into, the fail-closed refusal stays and the
* file stays byte-identical. The same construct in an untouched section
* is opaque user content (#2209). */
static const char *const initials[] = {
"agent:\n personalities:\n probe: >\n folded face\n",
"agent:\n personalities:\n probe: |\n literal face\n",
"agent:\n personalities:\n probe: >-\n chomped face\n",
"hooks:\n probe: >\n folded face\n",
"hooks:\n probe: |\n literal face\n",
"hooks:\n probe: >-\n chomped face\n",
};
for (size_t i = 0U; i < sizeof(initials) / sizeof(initials[0]); i++) {
yaml_fixture_t fixture;
@@ -1250,10 +1252,10 @@ TEST(config_yaml_edit_hermes_hook_still_refuses_block_scalar_value_issue1924) {
TEST(config_yaml_edit_hermes_hook_still_refuses_nonempty_flow_sequence_value_issue1924) {
/* Pins the #1631 line: exact `[]` is accepted, a non-empty flow sequence
* value is still unsupported in the sequence-document scan. */
const char *initial = "agent:\n"
" personalities:\n"
" probe: [a, b]\n";
* value inside the edited `hooks` subtree is still unsupported. Outside
* it, the value is opaque user content (#2209). */
const char *initial = "hooks:\n"
" probe: [a, b]\n";
yaml_fixture_t fixture;
ASSERT_EQ(yaml_fixture_init(&fixture, initial), 0);
ASSERT_EQ(yaml_hermes_hook_upsert(&fixture), CBM_YAML_IDENTITY_EDIT_ERROR);
@@ -2027,6 +2029,57 @@ TEST(config_yaml_edit_remove_on_absent_config_is_a_no_op) {
PASS();
}
/* #2209: the stock Hermes config carries flow sequences and block scalars in
* sections the hook edit never touches (`platform_toolsets:` -> `cli:
* [hermes-cli]`, `plugins:` -> `enabled: [...]`, `|` prompts). They are opaque
* user content, exactly as for the mapping-entry editor that writes
* `mcp_servers` into the same file; only the `hooks` subtree is validated. */
TEST(config_yaml_edit_nested_sequence_ignores_foreign_flow_and_block_scalars_issue2209) {
const char *initial = "plugins:\n"
" enabled: [disk-cleanup]\n"
"platform_toolsets:\n"
" cli: [hermes-cli]\n"
" telegram: [hermes-telegram]\n"
"toolsets: [all]\n"
"agent:\n"
" system_prompt: |\n"
" You are helpful.\n"
" - keep: [this, text]\n"
" note: >\n"
" folded\n"
"model:\n"
" default: \"anthropic/claude-opus-4.6\"\n";
yaml_fixture_t fixture;
ASSERT_EQ(yaml_fixture_init(&fixture, initial), 0);
ASSERT_EQ(cbm_yaml_upsert_mapping_sequence_item(fixture.path, yaml_hook_sequence_path, 2U, "id",
yaml_hook_identity, yaml_hook_canonical_item),
CBM_YAML_IDENTITY_EDIT_OK);
char *installed = yaml_read_alloc(fixture.path);
ASSERT_NOT_NULL(installed);
ASSERT_EQ(strncmp(installed, initial, strlen(initial)), 0);
ASSERT_NOT_NULL(strstr(installed, "hooks:\n"
" pre_llm_call:\n"
" - id: \"codebase-memory-mcp\"\n"));
ASSERT_EQ(cbm_yaml_upsert_mapping_sequence_item(fixture.path, yaml_hook_sequence_path, 2U, "id",
yaml_hook_identity, yaml_hook_canonical_item),
CBM_YAML_IDENTITY_EDIT_OK);
char *second = yaml_read_alloc(fixture.path);
ASSERT_NOT_NULL(second);
ASSERT_STR_EQ(second, installed);
free(second);
free(installed);
ASSERT_EQ(cbm_yaml_remove_mapping_sequence_item(fixture.path, yaml_hook_sequence_path, 2U, "id",
yaml_hook_identity, yaml_hook_canonical_item),
CBM_YAML_IDENTITY_EDIT_OK);
char *removed = yaml_read_alloc(fixture.path);
ASSERT_NOT_NULL(removed);
ASSERT_NULL(strstr(removed, "codebase-memory-mcp"));
ASSERT_EQ(strncmp(removed, initial, strlen(initial)), 0);
free(removed);
th_cleanup(fixture.dir);
PASS();
}
TEST(config_yaml_edit_nested_sequence_ambiguity_fails_byte_identically) {
const char *cases[] = {
"hooks:\n pre_llm_call:\n - id: \"bad-indent\"\n",
@@ -2192,6 +2245,7 @@ SUITE(config_yaml_edit) {
RUN_TEST(config_yaml_edit_nested_sequence_preserves_crlf);
RUN_TEST(config_yaml_edit_nested_sequence_foreign_identity_is_preserved);
RUN_TEST(config_yaml_edit_nested_sequence_removes_only_exact_canonical_item);
RUN_TEST(config_yaml_edit_nested_sequence_ignores_foreign_flow_and_block_scalars_issue2209);
RUN_TEST(config_yaml_edit_nested_sequence_ambiguity_fails_byte_identically);
RUN_TEST(config_yaml_edit_remove_on_absent_config_is_a_no_op);
#ifndef _WIN32