mirror of
https://github.com/DeusData/codebase-memory-mcp.git
synced 2026-11-04 10:48:20 +00:00
fix(yaml): scope > | [ ] indicators to the value start (#1924)
Hermes `install` failed with `op=pre_llm_hook_install` whenever config.yaml held a PLAIN scalar with a `>` or `|` anywhere after the value start — `probe: kaomoji face >w< here` — while the identical string double-quoted, or a mid-value `*`, installed fine. An agent-config failure aborts activation, so the whole install exited 1 and user-PATH registration was skipped although eleven other agents had configured cleanly. Bisected by @ukind, key by key and persona by persona, down to two persona strings (#1924, continuing #1631/#1656). Root cause: yaml_sequence_line_has_unsupported — the document validator on the mapping-sequence path that the hook upsert/remove uses — ran a second scan after the shared yaml_range_has_unsupported_ex and returned "unsupported" for `[`, `]`, `|` and `>` at ANY position of a plain scalar. The mapping-body path (mcp_servers) never had that check, which is exactly why the mcp op succeeded and only the hook op failed on the same file. Fix: the positional rule the `*`/`&` fix (v0.10.4) and the quote fix (#1631, v0.10.6) already use — an indicator counts only where a NODE begins (range start, after a mapping colon, after a block-sequence dash). Interior ones are text. `probe: >`, `probe: |`, `probe: >-` and `probe: [a, b]` still begin a value and stay unsupported, byte-identically. `[`/`]` get the same treatment on purpose: in block context they are ordinary characters inside a plain scalar (`a: b[0]` parses as the string `b[0]`), a value-start `[` still opens a flow sequence and stays refused (the #1631 line: exact `[]` accepted, non-empty flow sequences unsupported), and the mapping-body path already accepted them mid-value — leaving the sequence scan stricter would keep the mcp-op-passes / hook-op-fails asymmetry this issue is about. Known conservative residue, shared with the `*` rule: a plain-scalar CONTINUATION line that itself starts with one of these characters is still refused (nothing precedes it on its line). Tests (tests/test_config_yaml_edit.c): three accept cases run both real Hermes ops (mcp_servers + pre_llm_call) against a persona corpus with a mid-value `>`, `|` and `[]` and require the original document to survive as an untouched prefix; two pins keep value-start block scalars and a non-empty flow sequence refused with the file byte-identical. RED on the unfixed editor: 61 passed, 3 failed (`yaml_hermes_hook_upsert(&fixture) == -1`); GREEN: 64 passed; RED again with the production change reverted. config_yaml_edit 64 · yaml 73 · cli 306 — 443 passed, 0 failed (macOS, ASan/UBSan test build). Not in this change: the refusal still reaches the user as `op=pre_llm_hook_install path=... (target: regular file, N bytes)` with no reason. The editor collapses every one of its ~236 YAML_ERROR sites into CBM_YAML_IDENTITY_EDIT_ERROR, so naming the refusing rule needs a detailed variant with a typed failure out-parameter, the shape the Codex TOML hook path already has (cbm_toml_codex_hook_failure_t + record_agent_config_error_with_reason). Follow-up. Fixes #1924. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This commit is contained in:
1 parent
aa44c28ea5
commit
7219a24cc6
2 files changed
+119
-1
No files matched your search
@@ -2624,8 +2624,18 @@ static int yaml_sequence_line_has_unsupported(const yaml_doc_t *doc, const yaml_
|
||||
if (value == '#' && (i == start || doc->data[i - YAML_UNIT] == ' ')) {
|
||||
break;
|
||||
}
|
||||
/* Block-scalar (`|`, `>`) and flow-sequence (`[`, `]`) indicators
|
||||
* count only where a NODE begins, the same positional rule
|
||||
* yaml_range_has_unsupported_ex applies to `&`/`*`. Interior ones are
|
||||
* text: `probe: kaomoji face >w< here` is a plain scalar, and a real
|
||||
* Hermes config carried two such personas (#1924). `probe: >`,
|
||||
* `probe: |` and `probe: [a, b]` still begin a value and stay
|
||||
* unsupported here. */
|
||||
if (value == '[' || value == ']' || value == '|' || value == '>') {
|
||||
return YAML_MATCH;
|
||||
bool begins_node = previous == '\0' || previous == ':' || previous == '-';
|
||||
if (begins_node) {
|
||||
return YAML_MATCH;
|
||||
}
|
||||
}
|
||||
if (value != ' ' && value != '\t') {
|
||||
previous = value;
|
||||
|
||||
@@ -1096,6 +1096,109 @@ TEST(config_yaml_edit_bom_before_our_own_section_stays_single_issue1656) {
|
||||
PASS();
|
||||
}
|
||||
|
||||
/* ── #1924: block/flow indicators are indicators only where a value begins ──
|
||||
*
|
||||
* The sequence-document scan (the pre_llm_hook_install path) refused `>`,
|
||||
* `|`, `[` and `]` at ANY position of a plain scalar, so a Hermes persona
|
||||
* such as `probe: kaomoji face >w< here` made the hook op fail while the
|
||||
* mcp_servers op on the same file succeeded (bisected to two persona strings
|
||||
* by @ukind). YAML reads them as indicators only where a value begins — the
|
||||
* rule the `*`/`&` fix and the quote fix (#1631) already apply. Both real
|
||||
* Hermes ops run here; our blocks append, so the original document must
|
||||
* survive as an untouched prefix. */
|
||||
|
||||
TEST(config_yaml_edit_hermes_accepts_interior_gt_in_plain_scalar_issue1924) {
|
||||
const char *initial = "agent:\n"
|
||||
" personalities:\n"
|
||||
" probe: kaomoji face >w< here\n";
|
||||
yaml_fixture_t fixture;
|
||||
ASSERT_EQ(yaml_fixture_init(&fixture, initial), 0);
|
||||
ASSERT_EQ(yaml_hermes_upsert(&fixture), CBM_YAML_IDENTITY_EDIT_OK);
|
||||
ASSERT_EQ(yaml_hermes_hook_upsert(&fixture), CBM_YAML_IDENTITY_EDIT_OK);
|
||||
char *after = yaml_read_alloc(fixture.path);
|
||||
ASSERT_NOT_NULL(after);
|
||||
ASSERT_EQ(strncmp(after, initial, strlen(initial)), 0);
|
||||
ASSERT_NOT_NULL(strstr(after, "cbm-context"));
|
||||
free(after);
|
||||
th_cleanup(fixture.dir);
|
||||
PASS();
|
||||
}
|
||||
|
||||
TEST(config_yaml_edit_hermes_accepts_interior_pipe_in_plain_scalar_issue1924) {
|
||||
const char *initial = "agent:\n"
|
||||
" personalities:\n"
|
||||
" probe: pipe char | mid value\n";
|
||||
yaml_fixture_t fixture;
|
||||
ASSERT_EQ(yaml_fixture_init(&fixture, initial), 0);
|
||||
ASSERT_EQ(yaml_hermes_upsert(&fixture), CBM_YAML_IDENTITY_EDIT_OK);
|
||||
ASSERT_EQ(yaml_hermes_hook_upsert(&fixture), CBM_YAML_IDENTITY_EDIT_OK);
|
||||
char *after = yaml_read_alloc(fixture.path);
|
||||
ASSERT_NOT_NULL(after);
|
||||
ASSERT_EQ(strncmp(after, initial, strlen(initial)), 0);
|
||||
ASSERT_NOT_NULL(strstr(after, "cbm-context"));
|
||||
free(after);
|
||||
th_cleanup(fixture.dir);
|
||||
PASS();
|
||||
}
|
||||
|
||||
TEST(config_yaml_edit_hermes_accepts_interior_brackets_in_plain_scalar_issue1924) {
|
||||
/* In block context `[`/`]` inside a plain scalar are ordinary text; only
|
||||
* a value-start `[` opens a flow sequence. */
|
||||
const char *initial = "agent:\n"
|
||||
" personalities:\n"
|
||||
" probe: see docs[1] and args[] here\n";
|
||||
yaml_fixture_t fixture;
|
||||
ASSERT_EQ(yaml_fixture_init(&fixture, initial), 0);
|
||||
ASSERT_EQ(yaml_hermes_upsert(&fixture), CBM_YAML_IDENTITY_EDIT_OK);
|
||||
ASSERT_EQ(yaml_hermes_hook_upsert(&fixture), CBM_YAML_IDENTITY_EDIT_OK);
|
||||
char *after = yaml_read_alloc(fixture.path);
|
||||
ASSERT_NOT_NULL(after);
|
||||
ASSERT_EQ(strncmp(after, initial, strlen(initial)), 0);
|
||||
ASSERT_NOT_NULL(strstr(after, "cbm-context"));
|
||||
free(after);
|
||||
th_cleanup(fixture.dir);
|
||||
PASS();
|
||||
}
|
||||
|
||||
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. */
|
||||
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",
|
||||
};
|
||||
for (size_t i = 0U; i < sizeof(initials) / sizeof(initials[0]); i++) {
|
||||
yaml_fixture_t fixture;
|
||||
ASSERT_EQ(yaml_fixture_init(&fixture, initials[i]), 0);
|
||||
ASSERT_EQ(yaml_hermes_hook_upsert(&fixture), CBM_YAML_IDENTITY_EDIT_ERROR);
|
||||
char *after = yaml_read_alloc(fixture.path);
|
||||
ASSERT_NOT_NULL(after);
|
||||
ASSERT_STR_EQ(after, initials[i]);
|
||||
free(after);
|
||||
th_cleanup(fixture.dir);
|
||||
}
|
||||
PASS();
|
||||
}
|
||||
|
||||
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";
|
||||
yaml_fixture_t fixture;
|
||||
ASSERT_EQ(yaml_fixture_init(&fixture, initial), 0);
|
||||
ASSERT_EQ(yaml_hermes_hook_upsert(&fixture), CBM_YAML_IDENTITY_EDIT_ERROR);
|
||||
char *after = yaml_read_alloc(fixture.path);
|
||||
ASSERT_NOT_NULL(after);
|
||||
ASSERT_STR_EQ(after, initial);
|
||||
free(after);
|
||||
th_cleanup(fixture.dir);
|
||||
PASS();
|
||||
}
|
||||
|
||||
/* ── Owned-entry repair (#1631 galaxy + goose upgrades) ──────────────────────
|
||||
*
|
||||
* Byte-identity alone freezes users on any OLD canonical our past writers
|
||||
@@ -1989,6 +2092,11 @@ SUITE(config_yaml_edit) {
|
||||
RUN_TEST(config_yaml_edit_hermes_accepts_interior_apostrophe_and_plain_wrap_issue1631);
|
||||
RUN_TEST(config_yaml_edit_accepts_utf8_bom_issue1656);
|
||||
RUN_TEST(config_yaml_edit_bom_before_our_own_section_stays_single_issue1656);
|
||||
RUN_TEST(config_yaml_edit_hermes_accepts_interior_gt_in_plain_scalar_issue1924);
|
||||
RUN_TEST(config_yaml_edit_hermes_accepts_interior_pipe_in_plain_scalar_issue1924);
|
||||
RUN_TEST(config_yaml_edit_hermes_accepts_interior_brackets_in_plain_scalar_issue1924);
|
||||
RUN_TEST(config_yaml_edit_hermes_hook_still_refuses_block_scalar_value_issue1924);
|
||||
RUN_TEST(config_yaml_edit_hermes_hook_still_refuses_nonempty_flow_sequence_value_issue1924);
|
||||
RUN_TEST(config_yaml_edit_repairs_prior_unquoted_command_entry_issue1631);
|
||||
RUN_TEST(config_yaml_edit_repairs_prior_goose_block_without_name);
|
||||
RUN_TEST(config_yaml_edit_still_refuses_truly_foreign_entry_under_our_key);
|
||||
|
||||
Reference in new issue
Block a user