diff --git a/src/cli/config_yaml_edit.c b/src/cli/config_yaml_edit.c index 2d07e86f4..fda822201 100644 --- a/src/cli/config_yaml_edit.c +++ b/src/cli/config_yaml_edit.c @@ -99,6 +99,13 @@ typedef struct { size_t indent; bool blank; bool comment; + /* Double-quoted scalars may continue across lines via a trailing `\` + * (#1631). dquote_opens: this line ends inside an open double quote whose + * last byte is the continuation backslash. dquote_cont: this line is the + * textual continuation of such a scalar — VALUE BYTES, not structure; + * every structural walker must skip it. */ + bool dquote_opens; + bool dquote_cont; } yaml_line_t; typedef struct { @@ -1079,6 +1086,76 @@ static void yaml_doc_free(yaml_doc_t *doc) { memset(doc, 0, sizeof(*doc)); } +/* Precompute the double-quote continuation flags (#1631): a line whose scan + * ends inside a double-quoted scalar with a trailing `\` opens a continuation; + * the following line is value text and may itself close, or re-open, the + * scalar. Any OTHER way of ending a line inside a quote stays an error at the + * validators. Returns YAML_ERROR only when the document ENDS inside an open + * continuation (the quote can never close). */ +static int yaml_mark_dquote_continuations(yaml_doc_t *doc) { + bool cont = false; + for (size_t i = 0; i < doc->line_count; i++) { + yaml_line_t *line = &doc->lines[i]; + line->dquote_cont = cont; + line->dquote_opens = false; + if (!cont && (line->blank || line->comment)) { + continue; + } + size_t start = cont ? line->start : line->start + line->indent; + char quote = cont ? '"' : '\0'; + bool opens = false; + char previous = '\0'; + size_t j = start; + while (j < line->text_end) { + char c = doc->data[j]; + if (quote == '"') { + if (c == '\\') { + if (j + YAML_UNIT == line->text_end) { + opens = true; + break; + } + j += YAML_ENTRY_INDENT; + continue; + } + if (c == '"') { + quote = '\0'; + } + j++; + continue; + } + if (quote == '\'') { + if (c == '\'') { + if (j + YAML_UNIT < line->text_end && doc->data[j + YAML_UNIT] == '\'') { + j += YAML_ENTRY_INDENT; + } else { + quote = '\0'; + j++; + } + continue; + } + j++; + continue; + } + if ((c == '"' || c == '\'') && + (previous == '\0' || previous == ':' || previous == '-')) { + quote = c; + j++; + continue; + } + if (c == '#' && (j == start || doc->data[j - YAML_UNIT] == ' ')) { + break; + } + if (c != ' ' && c != '\t') { + previous = c; + } + j++; + } + line->dquote_opens = opens; + cont = opens; + } + return cont ? YAML_ERROR : 0; +} + static int yaml_doc_init(yaml_doc_t *doc, const char *data, size_t len) { memset(doc, 0, sizeof(*doc)); doc->data = data; @@ -1086,9 +1163,13 @@ static int yaml_doc_init(yaml_doc_t *doc, const char *data, size_t len) { if (yaml_build_lines(doc) != 0) { return YAML_ERROR; } + if (yaml_mark_dquote_continuations(doc) != 0) { + yaml_doc_free(doc); + return YAML_ERROR; + } for (size_t i = 0; i < doc->line_count; i++) { const yaml_line_t *line = &doc->lines[i]; - if (line->indent == 0U && !line->blank && !line->comment) { + if (line->indent == 0U && !line->blank && !line->comment && !line->dquote_cont) { size_t len_no_eol = line->text_end - line->start; if ((len_no_eol == YAML_DOC_MARKER_LEN && memcmp(doc->data + line->start, "---", YAML_DOC_MARKER_LEN) == 0) || @@ -1122,7 +1203,7 @@ static size_t yaml_trim_spaces_end(const char *data, size_t start, size_t end) { static int yaml_line_matches_key(const yaml_doc_t *doc, const yaml_line_t *line, size_t indent, const char *key, size_t key_len, size_t *out_colon) { - if (line->blank || line->comment || line->indent != indent) { + if (line->blank || line->comment || line->dquote_cont || line->indent != indent) { return 0; } size_t start = line->start + indent; @@ -1130,6 +1211,14 @@ static int yaml_line_matches_key(const yaml_doc_t *doc, const yaml_line_t *line, if (start >= end) { return 0; } + /* A block-sequence item at this indent is a known non-key line (#1631): + * YAML puts items at the same indent as their mapping key, so a column-0 + * `- item` is ordinary structure, not a malformed key. The root-mapping + * walker already validated that items only follow a value-less key. */ + if (doc->data[start] == '-' && + (start + YAML_UNIT == end || doc->data[start + YAML_UNIT] == ' ')) { + return 0; + } size_t colon = 0U; if (yaml_find_mapping_colon(doc->data, start, end, &colon) != 0) { return YAML_ERROR; @@ -1193,7 +1282,7 @@ static int yaml_find_unique_key(const yaml_doc_t *doc, size_t indent, const char static size_t yaml_top_level_section_end(const yaml_doc_t *doc, size_t header_line) { for (size_t i = header_line + YAML_UNIT; i < doc->line_count; i++) { const yaml_line_t *line = &doc->lines[i]; - if (!line->blank && !line->comment && line->indent == 0U) { + if (!line->blank && !line->comment && !line->dquote_cont && line->indent == 0U) { return i; } } @@ -1232,6 +1321,23 @@ static int yaml_scan_quote(const char *data, size_t end, size_t *pos, char *quot return 0; } +/* Positional variant (#1631): a quote character OPENS a scalar only where a + * node begins — at the range start, after a mapping colon, or after a block- + * sequence dash. Mid-word apostrophes (`LET'S`) and inch marks are ordinary + * text, exactly like the `&`/`*` indicator rule above. Closing behaviour for + * an already-open quote is unchanged. */ +static int yaml_scan_quote_positional(const char *data, size_t end, size_t *pos, char *quote, + bool *is_plain, char previous) { + if (*quote == '\0') { + char c = data[*pos]; + if ((c == '\'' || c == '"') && !(previous == '\0' || previous == ':' || previous == '-')) { + *is_plain = true; + return 0; + } + } + return yaml_scan_quote(data, end, pos, quote, is_plain); +} + static int yaml_find_comment(const char *data, size_t start, size_t end, size_t *out_comment) { char quote = '\0'; for (size_t i = start; i < end; i++) { @@ -1267,8 +1373,21 @@ static int yaml_find_comment(const char *data, size_t start, size_t end, size_t * honoured as one when nothing has appeared before it in the value. `key: *a` * is still an alias and still refused; `key: 2 * 3` and `key: a*b` are text. * `{`/`}` keep their existing treatment here; mapping-body validation admits - * exact empty flow mappings through a context-specific exception. */ + * exact empty flow mappings through a context-specific exception. + * + * allow_open_dquote: the caller established (via the doc-level continuation + * flags, #1631) that this range legitimately ends inside a double-quoted + * scalar continued on the next line — an open `"` at range end is then not an + * error. Every other caller keeps the fail-closed unterminated-quote check. */ +static int yaml_range_has_unsupported_ex(const char *data, size_t start, size_t end, + bool allow_open_dquote); + static int yaml_range_has_unsupported(const char *data, size_t start, size_t end) { + return yaml_range_has_unsupported_ex(data, start, end, false); +} + +static int yaml_range_has_unsupported_ex(const char *data, size_t start, size_t end, + bool allow_open_dquote) { char quote = '\0'; /* Last significant character seen, so an indicator can be judged by what * PRECEDES it. The scanned range covers a whole line including its key, so @@ -1277,9 +1396,14 @@ static int yaml_range_has_unsupported(const char *data, size_t start, size_t end * first. A node begins at the range start, after a mapping colon, or after * a block-sequence dash. */ char previous = '\0'; + /* A continuation range ends with the escaping backslash itself; keep it + * out of the scan so yaml_scan_quote never sees a dangling escape. */ + if (allow_open_dquote && end > start && data[end - YAML_UNIT] == '\\') { + end--; + } for (size_t i = start; i < end; i++) { bool is_plain = false; - if (yaml_scan_quote(data, end, &i, "e, &is_plain) != 0) { + if (yaml_scan_quote_positional(data, end, &i, "e, &is_plain, previous) != 0) { return YAML_MATCH; } if (!is_plain) { @@ -1307,6 +1431,9 @@ static int yaml_range_has_unsupported(const char *data, size_t start, size_t end return YAML_MATCH; } } + if (quote == '"' && allow_open_dquote) { + return 0; + } return quote != '\0'; } @@ -1334,9 +1461,13 @@ static int yaml_find_mapping_colon(const char *data, size_t start, size_t end, s static int yaml_validate_root_mapping(const yaml_doc_t *doc) { bool have_top_level_entry = false; + /* YAML allows a block sequence at the same indent as its mapping key + * (#1631): `agents:` followed by `- alpha` at column 0. Item lines are + * only legal directly after a value-less top-level key or another item. */ + bool in_column_zero_sequence = false; for (size_t i = 0U; i < doc->line_count; i++) { const yaml_line_t *line = &doc->lines[i]; - if (line->blank || line->comment) { + if (line->blank || line->comment || line->dquote_cont) { continue; } if (line->indent != 0U) { @@ -1347,6 +1478,14 @@ static int yaml_validate_root_mapping(const yaml_doc_t *doc) { } size_t start = line->start; + bool is_item = doc->data[start] == '-' && + (start + YAML_UNIT == line->text_end || doc->data[start + YAML_UNIT] == ' '); + if (is_item) { + if (!in_column_zero_sequence) { + return YAML_ERROR; + } + continue; + } if (doc->data[start] == '%' || doc->data[start] == '?' || doc->data[start] == ':') { return YAML_ERROR; } @@ -1365,6 +1504,8 @@ static int yaml_validate_root_mapping(const yaml_doc_t *doc) { return YAML_ERROR; } have_top_level_entry = true; + size_t tail = yaml_skip_spaces(doc->data, colon + YAML_UNIT, line->text_end); + in_column_zero_sequence = tail == line->text_end || doc->data[tail] == '#'; } return 0; } @@ -1387,17 +1528,35 @@ static int yaml_tail_is_explicit_empty_mapping(const char *data, size_t colon, s return 0; } +/* Exact empty flow collection (`{}` or `[]`) as the whole value, optionally + * followed by a comment. Used ONLY by the document VALIDATORS (#1631/#1673): + * an empty flow collection is preserved opaque user content. Section-header + * semantics keep the mapping-only helper above — `section: []` is a sequence, + * never an empty mapping section. */ +static int yaml_tail_is_explicit_empty_flow(const char *data, size_t colon, size_t end, + bool *out_empty) { + size_t comment = 0U; + if (yaml_find_comment(data, colon + YAML_UNIT, end, &comment) != 0) { + return YAML_ERROR; + } + size_t value_start = yaml_skip_spaces(data, colon + YAML_UNIT, comment); + size_t value_end = yaml_trim_spaces_end(data, value_start, comment); + *out_empty = value_end - value_start == YAML_QUOTED_MIN_LEN && + ((data[value_start] == '{' && data[value_start + YAML_UNIT] == '}') || + (data[value_start] == '[' && data[value_start + YAML_UNIT] == ']')); + return 0; +} + static int yaml_mapping_body_line_has_unsupported(const yaml_doc_t *doc, const yaml_line_t *line) { size_t start = line->start + line->indent; size_t colon = 0U; - bool empty_mapping = false; + bool empty_flow = false; if (yaml_find_mapping_colon(doc->data, start, line->text_end, &colon) == 0 && - yaml_tail_is_explicit_empty_mapping(doc->data, colon, line->text_end, &empty_mapping) == - 0 && - empty_mapping) { + yaml_tail_is_explicit_empty_flow(doc->data, colon, line->text_end, &empty_flow) == 0 && + empty_flow) { return yaml_range_has_unsupported(doc->data, start, colon + YAML_UNIT); } - return yaml_range_has_unsupported(doc->data, start, line->text_end); + return yaml_range_has_unsupported_ex(doc->data, start, line->text_end, line->dquote_opens); } static int yaml_value_starts_multiline(const char *data, size_t colon, size_t end) { @@ -1413,7 +1572,7 @@ static int yaml_validate_mapping_body(const yaml_doc_t *doc, size_t first_line, bool have_entry = false; for (size_t i = first_line; i < end_line; i++) { const yaml_line_t *line = &doc->lines[i]; - if (line->blank || line->comment) { + if (line->blank || line->comment || line->dquote_cont) { continue; } if (line->indent < YAML_ENTRY_INDENT || (line->indent & YAML_UNIT) != 0U || @@ -2390,7 +2549,7 @@ static size_t yaml_sequence_nested_end(const yaml_doc_t *doc, size_t header_line size_t indent = doc->lines[header_line].indent; for (size_t i = header_line + YAML_UNIT; i < parent_end; i++) { const yaml_line_t *line = &doc->lines[i]; - if (!line->blank && !line->comment && line->indent <= indent) { + if (!line->blank && !line->comment && !line->dquote_cont && line->indent <= indent) { return i; } } @@ -2399,16 +2558,31 @@ static size_t yaml_sequence_nested_end(const yaml_doc_t *doc, size_t header_line static int yaml_sequence_line_has_unsupported(const yaml_doc_t *doc, const yaml_line_t *line) { size_t start = line->start + line->indent; - if (yaml_range_has_unsupported(doc->data, start, line->text_end)) { + /* Exact empty flow collections as values (`envs: {}`, `plugins: []`) are + * preserved opaque user content (#1631/#1673): validate only the key. */ + size_t flow_colon = 0U; + bool empty_flow = false; + if (yaml_find_mapping_colon(doc->data, start, line->text_end, &flow_colon) == 0 && + yaml_tail_is_explicit_empty_flow(doc->data, flow_colon, line->text_end, &empty_flow) == 0 && + empty_flow) { + return yaml_range_has_unsupported(doc->data, start, flow_colon + YAML_UNIT); + } + size_t scan_end = line->text_end; + if (line->dquote_opens && scan_end > start && doc->data[scan_end - YAML_UNIT] == '\\') { + scan_end--; + } + if (yaml_range_has_unsupported_ex(doc->data, start, line->text_end, line->dquote_opens)) { return YAML_MATCH; } char quote = '\0'; - for (size_t i = start; i < line->text_end; i++) { + char previous = '\0'; + for (size_t i = start; i < scan_end; i++) { bool is_plain = false; - if (yaml_scan_quote(doc->data, line->text_end, &i, "e, &is_plain) != 0) { + if (yaml_scan_quote_positional(doc->data, scan_end, &i, "e, &is_plain, previous) != 0) { return YAML_MATCH; } if (!is_plain) { + previous = 'q'; continue; } char value = doc->data[i]; @@ -2418,6 +2592,12 @@ static int yaml_sequence_line_has_unsupported(const yaml_doc_t *doc, const yaml_ if (value == '[' || value == ']' || value == '|' || value == '>') { return YAML_MATCH; } + if (value != ' ' && value != '\t') { + previous = value; + } + } + if (quote == '"' && line->dquote_opens) { + return 0; } return quote != '\0' ? YAML_MATCH : 0; } @@ -2425,7 +2605,7 @@ static int yaml_sequence_line_has_unsupported(const yaml_doc_t *doc, const yaml_ static int yaml_sequence_validate_document(const yaml_doc_t *doc) { for (size_t i = 0U; i < doc->line_count; i++) { const yaml_line_t *line = &doc->lines[i]; - if (line->blank || line->comment) { + if (line->blank || line->comment || line->dquote_cont) { continue; } if ((line->indent & YAML_UNIT) != 0U || yaml_sequence_line_has_unsupported(doc, line)) { @@ -2456,11 +2636,15 @@ static int yaml_sequence_decode_field_key(const yaml_doc_t *doc, size_t start, s static int yaml_sequence_validate_mapping_range(const yaml_doc_t *doc, size_t begin_line, size_t end_line, size_t direct_indent) { bool have_direct = false; + /* YAML allows a block sequence at the same indent as its mapping key + * (#1631): a dash line directly after a value-less key at this indent is + * that key's opaque foreign value, not malformed structure. */ + bool prev_key_open = false; yaml_sequence_key_vec_t keys = {0}; int result = 0; for (size_t i = begin_line; i < end_line; i++) { const yaml_line_t *line = &doc->lines[i]; - if (line->blank || line->comment) { + if (line->blank || line->comment || line->dquote_cont) { continue; } if (line->indent < direct_indent || (line->indent > direct_indent && !have_direct)) { @@ -2471,10 +2655,19 @@ static int yaml_sequence_validate_mapping_range(const yaml_doc_t *doc, size_t be continue; } size_t start = line->start + direct_indent; - if (start >= line->text_end || doc->data[start] == '-') { + if (start >= line->text_end) { result = YAML_ERROR; break; } + if (doc->data[start] == '-') { + bool is_item = + start + YAML_UNIT == line->text_end || doc->data[start + YAML_UNIT] == ' '; + if (!is_item || !prev_key_open) { + result = YAML_ERROR; + break; + } + continue; + } size_t colon = 0U; char *key = NULL; if (yaml_sequence_decode_field_key(doc, start, line->text_end, &colon, &key) != 0 || @@ -2483,6 +2676,7 @@ static int yaml_sequence_validate_mapping_range(const yaml_doc_t *doc, size_t be break; } have_direct = true; + prev_key_open = yaml_tail_is_empty(doc->data, colon, line->text_end) != 0; } yaml_sequence_key_vec_free(&keys); return result; @@ -2557,7 +2751,7 @@ static int yaml_sequence_parse_item(const yaml_doc_t *doc, size_t start_line, si int result = 0; for (size_t i = start_line; i < end_line; i++) { const yaml_line_t *line = &doc->lines[i]; - if (line->blank || line->comment) { + if (line->blank || line->comment || line->dquote_cont) { continue; } size_t field_start = 0U; @@ -2611,7 +2805,7 @@ static int yaml_sequence_parse_block(const yaml_doc_t *doc, size_t begin_line, s size_t start_count = 0U; for (size_t i = begin_line; i < end_line; i++) { const yaml_line_t *line = &doc->lines[i]; - if (line->blank || line->comment) { + if (line->blank || line->comment || line->dquote_cont) { continue; } if (line->indent == item_indent) { @@ -3037,6 +3231,112 @@ static int yaml_remove_mapping_entry_locked(const char *file_path, const char *s return rc; } +/* #1631 / goose upgrades: an existing entry under OUR key whose bytes differ + * from today's canonical is still OURS — and repairable — when it parses as a + * shape a previous release wrote AND its command value's basename is our + * binary. Recognized prior shapes, nothing else: + * header ` :` then EITHER exactly one ` command: V` line (the YAML + * stdio schema; early releases wrote V unquoted), OR the goose block + * `type/cmd/args/enabled` in order with an optional leading `name:` (the + * pre-#1675 block had no name). V may be plain or double-quoted; its + * basename must be codebase-memory-mcp[.exe]. Anything else stays FOREIGN + * (fail-closed). */ +static bool yaml_owned_value_is_our_binary(const char *v, size_t vlen) { + if (vlen >= 2U && v[0] == '"' && v[vlen - 1U] == '"') { + v++; + vlen -= 2U; + } + size_t base = 0U; + for (size_t i = 0; i < vlen; i++) { + if (v[i] == '/' || v[i] == '\\') { + base = i + 1U; + } + } + const char *name = v + base; + size_t name_len = vlen - base; + static const char plain[] = "codebase-memory-mcp"; + static const char exe[] = "codebase-memory-mcp.exe"; + return (name_len == sizeof(plain) - 1U && memcmp(name, plain, name_len) == 0) || + (name_len == sizeof(exe) - 1U && memcmp(name, exe, name_len) == 0); +} + +static bool yaml_owned_entry_is_prior_shape(const char *data, size_t len, const char *entry_key, + size_t entry_key_len) { + /* Split into lines, tolerating CRLF. */ + enum { PRIOR_MAX_LINES = 8 }; + struct { + const char *text; + size_t len; + } lines[PRIOR_MAX_LINES]; + size_t count = 0U; + size_t pos = 0U; + while (pos < len) { + const char *nl = memchr(data + pos, '\n', len - pos); + size_t line_len = nl ? (size_t)(nl - (data + pos)) : len - pos; + size_t trimmed = line_len; + while (trimmed > 0U && (data[pos + trimmed - 1U] == '\r')) { + trimmed--; + } + if (trimmed > 0U) { + if (count == PRIOR_MAX_LINES) { + return false; + } + lines[count].text = data + pos; + lines[count].len = trimmed; + count++; + } + pos += line_len + (nl ? 1U : 0U); + } + if (count < 2U) { + return false; + } + /* Header: ` :` exactly. */ + if (lines[0].len != entry_key_len + 3U || memcmp(lines[0].text, " ", 2U) != 0 || + memcmp(lines[0].text + 2U, entry_key, entry_key_len) != 0 || + lines[0].text[2U + entry_key_len] != ':') { + return false; + } + /* Body lines: 4-space indent, known fields only. */ + static const char cmd_prefix[] = " command: "; + static const char goose_name[] = " name: codebase-memory-mcp"; + static const char goose_type[] = " type: stdio"; + static const char goose_cmd[] = " cmd: "; + static const char goose_args[] = " args: []"; + static const char goose_enabled[] = " enabled: true"; + if (count == 2U && lines[1].len > sizeof(cmd_prefix) - 1U && + memcmp(lines[1].text, cmd_prefix, sizeof(cmd_prefix) - 1U) == 0) { + return yaml_owned_value_is_our_binary(lines[1].text + sizeof(cmd_prefix) - 1U, + lines[1].len - (sizeof(cmd_prefix) - 1U)); + } + size_t i = 1U; + if (i < count && lines[i].len == sizeof(goose_name) - 1U && + memcmp(lines[i].text, goose_name, lines[i].len) == 0) { + i++; + } + if (i + 4U != count) { + return false; + } + if (lines[i].len != sizeof(goose_type) - 1U || + memcmp(lines[i].text, goose_type, lines[i].len) != 0) { + return false; + } + i++; + if (lines[i].len <= sizeof(goose_cmd) - 1U || + memcmp(lines[i].text, goose_cmd, sizeof(goose_cmd) - 1U) != 0 || + !yaml_owned_value_is_our_binary(lines[i].text + sizeof(goose_cmd) - 1U, + lines[i].len - (sizeof(goose_cmd) - 1U))) { + return false; + } + i++; + if (lines[i].len != sizeof(goose_args) - 1U || + memcmp(lines[i].text, goose_args, lines[i].len) != 0) { + return false; + } + i++; + return lines[i].len == sizeof(goose_enabled) - 1U && + memcmp(lines[i].text, goose_enabled, lines[i].len) == 0; +} + static int yaml_edit_owned_mapping_entry_locked(const char *file_path, const char *section_key, const char *entry_key, const char *canonical_entry_block, bool remove) { @@ -3096,14 +3396,17 @@ static int yaml_edit_owned_mapping_entry_locked(const char *file_path, const cha } entry_start = header->start; size_t existing_entry_len = target.entry_end - entry_start; - if (existing_entry_len != canonical.len || - memcmp(doc.data + entry_start, canonical.data, canonical.len) != 0) { + bool canonical_match = existing_entry_len == canonical.len && + memcmp(doc.data + entry_start, canonical.data, canonical.len) == 0; + if (!canonical_match && + !yaml_owned_entry_is_prior_shape(doc.data + entry_start, existing_entry_len, entry_key, + entry_len)) { yaml_buf_free(&canonical); yaml_doc_free(&doc); free(data); return CBM_YAML_IDENTITY_EDIT_FOREIGN; } - if (!remove) { + if (!remove && canonical_match) { yaml_buf_free(&canonical); yaml_doc_free(&doc); free(data); @@ -3123,7 +3426,7 @@ static int yaml_edit_owned_mapping_entry_locked(const char *file_path, const cha ? yaml_build_last_mapping_entry_removal(&out, &doc, &target) : yaml_splice(&out, &doc, entry_start, target.entry_end, NULL, 0U); } else { - build_result = yaml_build_mapping_update(&out, &doc, &target, NULL, section_key, + build_result = yaml_build_mapping_update(&out, &doc, &target, header, section_key, section_len, &canonical); } int result = build_result == 0 ? yaml_commit_if_changed(file_path, data, len, &snapshot, &out) diff --git a/tests/test_config_yaml_edit.c b/tests/test_config_yaml_edit.c index 23fb9e308..077976a10 100644 --- a/tests/test_config_yaml_edit.c +++ b/tests/test_config_yaml_edit.c @@ -913,6 +913,216 @@ TEST(config_yaml_edit_goose_still_rejects_merge_key_with_empty_mapping_issue1673 PASS(); } +/* ── #1631: legal YAML constructs in real Hermes configs the editor refused ── + * + * Each construct is distilled from the reporters' actual config.yaml files + * (iandol + galaxy gists) and was verified RED end-to-end via `install` + * against those files. The document validators must accept them; the owned + * edit still only appends/replaces our entry and preserves every original + * byte of the user's content. */ + +static const char *const yaml_hermes_block = " command: \"/opt/codebase-memory-mcp\"\n"; + +static int yaml_hermes_upsert(const yaml_fixture_t *fixture) { + return cbm_yaml_upsert_owned_mapping_entry(fixture->path, "mcp_servers", "codebase-memory-mcp", + yaml_hermes_block); +} + +/* Both real Hermes ops: mcp_install (owned mapping entry) AND + * pre_llm_hook_install (mapping sequence item) — E2E showed three of the four + * constructs break only in the second. */ +static int yaml_hermes_hook_upsert(const yaml_fixture_t *fixture) { + static const char *const path[] = {"hooks", "pre_llm_call"}; + return cbm_yaml_upsert_mapping_sequence_item( + fixture->path, path, 2U, "id", "\"cbm-context\"", + "- id: \"cbm-context\"\n type: \"command\"\n command: \"/opt/codebase-memory-mcp\"\n"); +} + +TEST(config_yaml_edit_hermes_accepts_empty_flow_sequence_value_issue1631) { + const char *initial = "name: test\n" + "plugins: []\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_NOT_NULL(strstr(after, "plugins: []\n")); + ASSERT_NOT_NULL(strstr(after, "command: \"/opt/codebase-memory-mcp\"\n")); + free(after); + th_cleanup(fixture.dir); + PASS(); +} + +TEST(config_yaml_edit_hermes_accepts_empty_flow_mapping_value_issue1631) { + const char *initial = "name: test\n" + "tool_choice: {}\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_NOT_NULL(strstr(after, "tool_choice: {}\n")); + ASSERT_NOT_NULL(strstr(after, "command: \"/opt/codebase-memory-mcp\"\n")); + free(after); + th_cleanup(fixture.dir); + PASS(); +} + +TEST(config_yaml_edit_hermes_accepts_column_zero_block_sequence_issue1631) { + /* YAML allows a block sequence at the same indent as its mapping key. */ + const char *initial = "agents:\n" + "- alpha\n" + "- beta\n" + "model: gpt\n"; + yaml_fixture_t fixture; + ASSERT_EQ(yaml_fixture_init(&fixture, initial), 0); + ASSERT_EQ(yaml_hermes_upsert(&fixture), CBM_YAML_IDENTITY_EDIT_OK); + char *after = yaml_read_alloc(fixture.path); + ASSERT_NOT_NULL(after); + ASSERT_NOT_NULL(strstr(after, "agents:\n- alpha\n- beta\n")); + ASSERT_NOT_NULL(strstr(after, "command: \"/opt/codebase-memory-mcp\"\n")); + free(after); + th_cleanup(fixture.dir); + PASS(); +} + +TEST(config_yaml_edit_hermes_accepts_escaped_newline_in_double_quote_issue1631) { + /* A double-quoted scalar may continue across lines with a trailing `\`. */ + const char *initial = "name: test\n" + "persona: \"line one \\\n" + " line two\"\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_NOT_NULL(strstr(after, "persona: \"line one \\\n")); + ASSERT_NOT_NULL(strstr(after, "command: \"/opt/codebase-memory-mcp\"\n")); + free(after); + th_cleanup(fixture.dir); + PASS(); +} + +TEST(config_yaml_edit_hermes_hook_accepts_column_zero_sequence_issue1631) { + /* The pre_llm_call hook op walks the same document: a column-0 sequence + * elsewhere in the file must not abort the hook install. */ + static const char *const path[] = {"hooks", "pre_llm_call"}; + const char *initial = "agents:\n" + "- alpha\n" + "model: gpt\n"; + yaml_fixture_t fixture; + ASSERT_EQ(yaml_fixture_init(&fixture, initial), 0); + ASSERT_EQ(cbm_yaml_upsert_mapping_sequence_item(fixture.path, path, 2U, "id", "\"cbm-context\"", + "- id: \"cbm-context\"\n type: \"command\"\n" + " command: \"/opt/codebase-memory-mcp\"\n"), + CBM_YAML_IDENTITY_EDIT_OK); + char *after = yaml_read_alloc(fixture.path); + ASSERT_NOT_NULL(after); + ASSERT_NOT_NULL(strstr(after, "agents:\n- alpha\n")); + ASSERT_NOT_NULL(strstr(after, "cbm-context")); + free(after); + th_cleanup(fixture.dir); + PASS(); +} + +TEST(config_yaml_edit_hermes_accepts_interior_apostrophe_and_plain_wrap_issue1631) { + /* Distilled from the iandol config's `personalities:` block: mid-word + * apostrophes in a plain scalar (quotes are indicators only at a node + * start), and a plain scalar folded onto a deeper-indented next line. */ + const char *initial = "agent:\n" + " personalities:\n" + " hype: YOOO LET'S GOOOO!!! I am SO PUMPED! Every question\n" + " is AMAZING and we're gonna CRUSH IT together!\n" + " verbose: false\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_NOT_NULL(strstr(after, "LET'S GOOOO")); + ASSERT_NOT_NULL(strstr(after, "cbm-context")); + 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 + * produced: v0.10.x wrote `command:` unquoted (the galaxy reporter's file), + * and the goose block gained a required `name:` field — without repair, every + * such entry is declared FOREIGN forever and install fails. An existing entry + * under OUR key is repairable when it parses as a known prior shape and its + * command basename is our binary. Truly foreign shapes stay refused. */ + +TEST(config_yaml_edit_repairs_prior_unquoted_command_entry_issue1631) { + const char *initial = "mcp_servers:\n" + " obsidian:\n" + " url: \"\"\n" + " codebase-memory-mcp:\n" + " command: C:/Users/Administrator/AppData/Local/Programs/" + "codebase-memory-mcp/codebase-memory-mcp.exe\n"; + yaml_fixture_t fixture; + ASSERT_EQ(yaml_fixture_init(&fixture, initial), 0); + ASSERT_EQ(yaml_hermes_upsert(&fixture), CBM_YAML_IDENTITY_EDIT_OK); + char *after = yaml_read_alloc(fixture.path); + ASSERT_NOT_NULL(after); + ASSERT_NOT_NULL(strstr(after, " obsidian:\n url: \"\"\n")); + ASSERT_NOT_NULL(strstr(after, "command: \"/opt/codebase-memory-mcp\"\n")); + ASSERT_NULL(strstr(after, "Administrator")); + free(after); + th_cleanup(fixture.dir); + PASS(); +} + +TEST(config_yaml_edit_repairs_prior_goose_block_without_name) { + const char *initial = "extensions:\n" + " codebase-memory-mcp:\n" + " type: stdio\n" + " cmd: \"/old/place/codebase-memory-mcp\"\n" + " args: []\n" + " enabled: true\n"; + const char *block = " name: codebase-memory-mcp\n" + " type: stdio\n" + " cmd: \"/opt/codebase-memory-mcp\"\n" + " args: []\n" + " enabled: true\n"; + yaml_fixture_t fixture; + ASSERT_EQ(yaml_fixture_init(&fixture, initial), 0); + ASSERT_EQ(cbm_yaml_upsert_owned_mapping_entry(fixture.path, "extensions", "codebase-memory-mcp", + block), + CBM_YAML_IDENTITY_EDIT_OK); + char *after = yaml_read_alloc(fixture.path); + ASSERT_NOT_NULL(after); + ASSERT_NOT_NULL(strstr(after, "name: codebase-memory-mcp\n")); + ASSERT_NOT_NULL(strstr(after, "cmd: \"/opt/codebase-memory-mcp\"\n")); + ASSERT_NULL(strstr(after, "/old/place/")); + free(after); + th_cleanup(fixture.dir); + PASS(); +} + +TEST(config_yaml_edit_still_refuses_truly_foreign_entry_under_our_key) { + /* Same key, but the body is not any shape we ever wrote — refuse and + * leave the file byte-identical. */ + const char *initial = "mcp_servers:\n" + " codebase-memory-mcp:\n" + " command: /usr/bin/somebody-elses-tool\n"; + yaml_fixture_t fixture; + ASSERT_EQ(yaml_fixture_init(&fixture, initial), 0); + ASSERT_EQ(yaml_hermes_upsert(&fixture), CBM_YAML_IDENTITY_EDIT_FOREIGN); + char *unchanged = yaml_read_alloc(fixture.path); + ASSERT_NOT_NULL(unchanged); + ASSERT_STR_EQ(unchanged, initial); + free(unchanged); + th_cleanup(fixture.dir); + PASS(); +} + TEST(config_yaml_edit_owned_agent_mapping_installs_idempotently_and_removes_exact_state) { struct owned_mapping_case { const char *section; @@ -1694,6 +1904,15 @@ SUITE(config_yaml_edit) { RUN_TEST(config_yaml_edit_hermes_creates_missing_section); RUN_TEST(config_yaml_edit_goose_extensions_preserve_siblings); RUN_TEST(config_yaml_edit_goose_accepts_empty_flow_mapping_in_sibling_issue1673); + RUN_TEST(config_yaml_edit_hermes_accepts_empty_flow_sequence_value_issue1631); + RUN_TEST(config_yaml_edit_hermes_accepts_empty_flow_mapping_value_issue1631); + RUN_TEST(config_yaml_edit_hermes_accepts_column_zero_block_sequence_issue1631); + RUN_TEST(config_yaml_edit_hermes_accepts_escaped_newline_in_double_quote_issue1631); + RUN_TEST(config_yaml_edit_hermes_hook_accepts_column_zero_sequence_issue1631); + RUN_TEST(config_yaml_edit_hermes_accepts_interior_apostrophe_and_plain_wrap_issue1631); + 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); RUN_TEST(config_yaml_edit_goose_still_rejects_nonempty_flow_mapping_issue1673); RUN_TEST(config_yaml_edit_goose_still_rejects_merge_key_with_empty_mapping_issue1673); RUN_TEST(config_yaml_edit_owned_agent_mapping_installs_idempotently_and_removes_exact_state);