diff --git a/src/cli/cli.c b/src/cli/cli.c index 63e73c686..1f850de8f 100644 --- a/src/cli/cli.c +++ b/src/cli/cli.c @@ -1672,16 +1672,45 @@ static size_t cbm_json_mcp_ownership_fields(cbm_json_mcp_schema_t schema, const * populated from config content. */ static CBM_TLS const char *g_previous_managed_mcp_command = NULL; +/* Path-shape-insensitive equality for OUR OWN binary path (#1582): clients + * and installers spell the same Windows file with different separators (the + * entry stores `C:\...\cbm.exe`, the installer compares `C:/.../cbm.exe`), + * and Windows filesystems are case-insensitive. Separators always compare + * equal; case only folds on Windows. POSIX byte-exactness otherwise holds. */ +static bool cbm_json_mcp_paths_equal(const char *a, const char *b) { + while (*a && *b) { + char ca = *a; + char cb = *b; + if (ca == '\\') { + ca = '/'; + } + if (cb == '\\') { + cb = '/'; + } +#ifdef _WIN32 + ca = (char)tolower((unsigned char)ca); + cb = (char)tolower((unsigned char)cb); +#endif + if (ca != cb) { + return false; + } + a++; + b++; + } + return *a == *b; +} + static bool cbm_json_mcp_owned_command(const char *command, const char *expected_binary, const char *previous_managed_binary) { if (!command || command[0] == '\0') { return false; } - if (expected_binary && expected_binary[0] && strcmp(command, expected_binary) == 0) { + if (expected_binary && expected_binary[0] && + cbm_json_mcp_paths_equal(command, expected_binary)) { return true; } if (previous_managed_binary && previous_managed_binary[0] && - strcmp(command, previous_managed_binary) == 0) { + cbm_json_mcp_paths_equal(command, previous_managed_binary)) { return true; } return strcmp(command, "codebase-memory-mcp") == 0 || @@ -1694,7 +1723,12 @@ static bool cbm_json_mcp_owned_command(const char *command, const char *expected * missing. Upsert repairs it; remove leaves it alone like any other non-owned * entry. POSIX never probes config-supplied paths: only the running executable * identity above can authorize a moved-install refresh. */ -enum { CBM_JSON_MCP_OWNERSHIP_STALE = 100 }; +enum { + CBM_JSON_MCP_OWNERSHIP_STALE = 100, + /* Repairable like STALE, but the entry carries client-added keys: only a + * FIELD-level repair may touch it — full replacement would drop them. */ + CBM_JSON_MCP_OWNERSHIP_EXTRAS_STALE = 101, +}; #ifdef _WIN32 typedef enum { @@ -1973,7 +2007,8 @@ static int cbm_json_mcp_snapshot_ownership(const char *document, size_t document const char *const *object_path, size_t path_len, cbm_json_mcp_schema_t schema, const char *entry_name, const char *argument, const char *expected_binary, - const char *previous_managed_binary) { + const char *previous_managed_binary, + char **command_out) { cbm_json_like_object_field_t fields[3]; size_t field_count = cbm_json_mcp_ownership_fields(schema, argument, fields); char *command = NULL; @@ -1983,17 +2018,47 @@ static int cbm_json_mcp_snapshot_ownership(const char *document, size_t document result == CBM_JSON_LIKE_OBJECT_MATCH_WITH_EXTRAS) && !cbm_json_mcp_owned_command(command, expected_binary, previous_managed_binary)) { #ifdef _WIN32 - result = cbm_json_mcp_command_availability(command) == CBM_JSON_MCP_COMMAND_MISSING - ? CBM_JSON_MCP_OWNERSHIP_STALE - : CBM_JSON_LIKE_OBJECT_MISMATCH; + bool had_extras = result == CBM_JSON_LIKE_OBJECT_MATCH_WITH_EXTRAS; + result = + cbm_json_mcp_command_availability(command) == CBM_JSON_MCP_COMMAND_MISSING + ? (had_extras ? CBM_JSON_MCP_OWNERSHIP_EXTRAS_STALE : CBM_JSON_MCP_OWNERSHIP_STALE) + : CBM_JSON_LIKE_OBJECT_MISMATCH; #else result = CBM_JSON_LIKE_OBJECT_MISMATCH; #endif } - free(command); + if (command_out) { + *command_out = command; + } else { + free(command); + } return result; } +/* Render just the `command` member's VALUE for a schema — the raw JSON the + * field-level repair splices in place of a moved install's stale path. */ +static char *cbm_json_mcp_render_command_value(const char *binary_path, + cbm_json_mcp_schema_t schema) { + yyjson_mut_doc *doc = yyjson_mut_doc_new(NULL); + if (!doc) { + return NULL; + } + yyjson_mut_val *command = cbm_json_mcp_command_is_array(schema) + ? yyjson_mut_arr(doc) + : yyjson_mut_strcpy(doc, binary_path); + bool ok = command != NULL; + if (ok && cbm_json_mcp_command_is_array(schema)) { + ok = yyjson_mut_arr_add_strcpy(doc, command, binary_path); + } + char *json = NULL; + if (ok) { + yyjson_mut_doc_set_root(doc, command); + json = yyjson_mut_write(doc, YYJSON_WRITE_NOFLAG, NULL); + } + yyjson_mut_doc_free(doc); + return json; +} + static int cbm_upsert_json_named_mcp(const char *binary_path, const char *config_path, const char *const *object_path, size_t path_len, cbm_json_mcp_schema_t schema, const char *entry_name, @@ -2008,9 +2073,10 @@ static int cbm_upsert_json_named_mcp(const char *binary_path, const char *config return CLI_ERR; } if (read_result == 0) { + char *command = NULL; int ownership = cbm_json_mcp_snapshot_ownership( document, document_length, object_path, path_len, schema, entry_name, argument, - binary_path, g_previous_managed_mcp_command); + binary_path, g_previous_managed_mcp_command, &command); /* An entry that already says what we would say, but carries extra keys * the client added, is ALREADY SATISFIED. Return success without * touching the file. @@ -2028,11 +2094,55 @@ static int cbm_upsert_json_named_mcp(const char *binary_path, const char *config * while preserving the rest is the fuller fix and is tracked there; * this makes the common case work without risking anyone's config. */ if (ownership == CBM_JSON_LIKE_OBJECT_MATCH_WITH_EXTRAS) { + /* Owned via the PREVIOUS managed binary during a relocating + * update: the annotated entry still names the old location, and a + * wholesale rewrite would drop the client's keys — repair only the + * command member (#1630's deferred field-merge). An entry already + * naming the current binary needs nothing. */ + bool via_previous = command && g_previous_managed_mcp_command && + strcmp(command, g_previous_managed_mcp_command) == 0 && + strcmp(command, binary_path) != 0; + if (!via_previous) { + free(command); + free(document); + return CLI_OK; + } + char *value = cbm_json_mcp_render_command_value(binary_path, schema); + if (!value) { + free(command); + free(document); + return CLI_ERR; + } + int repair = cbm_json_like_replace_field_raw_if_unchanged( + config_path, object_path, path_len, entry_name, "command", value, document, + document_length); + free(value); + free(command); free(document); - return CLI_OK; + return repair == 0 ? CLI_OK : CLI_ERR; + } + /* Windows only: our shape, annotated, and the named binary + * conclusively missing from the local fixed drives — repair ONLY the + * command member so the client's keys survive (#1630 field-merge). */ + if (ownership == CBM_JSON_MCP_OWNERSHIP_EXTRAS_STALE) { + char *value = cbm_json_mcp_render_command_value(binary_path, schema); + if (!value) { + free(command); + free(document); + return CLI_ERR; + } + int repair = cbm_json_like_replace_field_raw_if_unchanged( + config_path, object_path, path_len, entry_name, "command", value, document, + document_length); + free(value); + free(command); + free(document); + return repair == 0 ? CLI_OK : CLI_ERR; } - /* STALE (our exact shape, dead binary path) is repairable — that is - * the update contract. Only a genuinely foreign shape refuses. */ + /* STALE (our exact shape, dead binary path on Windows) is repairable + * — that is the update contract. Only a genuinely foreign shape + * refuses. */ + free(command); if (ownership != CBM_JSON_LIKE_OBJECT_MATCH && ownership != CBM_JSON_LIKE_OBJECT_MISSING && ownership != CBM_JSON_MCP_OWNERSHIP_STALE) { free(document); @@ -2080,7 +2190,7 @@ static int cbm_remove_json_named_mcp(const char *config_path, const char *const } int ownership = cbm_json_mcp_snapshot_ownership(document, document_length, object_path, path_len, schema, - entry_name, argument, expected_binary, NULL); + entry_name, argument, expected_binary, NULL, NULL); if (ownership == CBM_JSON_LIKE_OBJECT_MISSING || ownership == CBM_JSON_LIKE_OBJECT_MISMATCH || ownership == CBM_JSON_MCP_OWNERSHIP_STALE) { free(document); @@ -3342,14 +3452,21 @@ int cbm_upsert_codex_mcp(const char *binary_path, const char *config_path) { * handshake closes during initialization, so Codex exposes no cbm tools at * all. * - * The name is listed unconditionally rather than only when the variable is + * The names are listed unconditionally rather than only when a variable is * set at install time: env_vars names variables to FORWARD IF PRESENT, so - * listing it costs nothing when unset and keeps working for someone who - * sets it after installing — which install-time detection would silently - * fail to cover. */ + * listing them costs nothing when unset and keeps working for someone who + * sets one after installing — which install-time detection would silently + * fail to cover. + * + * CBM_RUNTIME_DIR joined the list with #1664: since #1645 it 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. Both names decide + * WHICH daemon a process talks to; behavioural knobs stay unforwarded. */ int written = snprintf(block, sizeof(block), CODEX_CMM_SECTION "\ncommand = \"%s\"\nargs = []\n" - "env_vars = [\"CBM_CACHE_DIR\"]\n", + "env_vars = [\"CBM_CACHE_DIR\", " + "\"CBM_RUNTIME_DIR\"]\n", escaped); if (written < 0 || (size_t)written >= sizeof(block) || cbm_remove_codex_legacy_mcp(config_path) != 0) { @@ -3716,7 +3833,11 @@ static int cbm_build_yaml_stdio_mcp_block(const char *binary_path, bool goose_sc free(encoded); return CLI_ERR; } + /* goose's ExtensionConfig::Stdio requires `name` (serde, no default) and + * its loader silently drops entries that fail to deserialize (#1675) — an + * entry without it installs cleanly and is then invisible in goose. */ int written = goose_schema ? snprintf(block, block_size, + " name: codebase-memory-mcp\n" " type: stdio\n" " cmd: %s\n" " args: []\n" @@ -3727,6 +3848,19 @@ static int cbm_build_yaml_stdio_mcp_block(const char *binary_path, bool goose_sc return written > 0 && (size_t)written < block_size ? CLI_OK : CLI_ERR; } +#ifdef CBM_CLI_ENABLE_TEST_API +/* Test seam for the agent-config block writers: the exact bytes written into a + * coding agent's config file are a compatibility contract with THAT agent's + * parser (goose deserializes with serde and silently drops entries that fail — + * #1675), so tests must be able to assert the block verbatim. */ +int cbm_cli_build_yaml_stdio_mcp_block_for_test(const char *binary_path, bool goose_schema, + char *block, size_t block_size) { + return cbm_build_yaml_stdio_mcp_block(binary_path, goose_schema, block, block_size) == CLI_OK + ? 0 + : -1; +} +#endif + static int cbm_upsert_yaml_stdio_mcp(const char *binary_path, const char *config_path, const char *section_key, bool goose_schema) { char block[CLI_BUF_8K]; @@ -3809,7 +3943,7 @@ static int cbm_junie_mcp_preflight(const char *binary_path, const char *config_p for (size_t i = 0U; i < sizeof(entries) / sizeof(entries[0]); i++) { int ownership = cbm_json_mcp_snapshot_ownership( document, document_length, path, 1U, CBM_JSON_MCP_STANDARD, entries[i].name, - entries[i].argument, binary_path, g_previous_managed_mcp_command); + entries[i].argument, binary_path, g_previous_managed_mcp_command, NULL); if (ownership != CBM_JSON_LIKE_OBJECT_MATCH && ownership != CBM_JSON_LIKE_OBJECT_MISSING && ownership != CBM_JSON_MCP_OWNERSHIP_STALE) { result = CLI_ERR; diff --git a/src/cli/config_json_like.c b/src/cli/config_json_like.c index 4d0ecc8d8..dc16c3c5c 100644 --- a/src/cli/config_json_like.c +++ b/src/cli/config_json_like.c @@ -2213,6 +2213,86 @@ int cbm_json_like_upsert_entry_if_unchanged(const char *file_path, const char *c expected_content, expected_length); } +int cbm_json_like_replace_field_raw_if_unchanged(const char *file_path, + const char *const *object_path, size_t path_len, + const char *entry_key, const char *field_key, + const char *raw_value, + const char *expected_content, + size_t expected_length) { + if (jl_validate_arguments(file_path, object_path, path_len, entry_key) != 0 || !field_key || + field_key[0] == '\0' || !raw_value) { + return -1; + } + size_t value_offset = 0U; + size_t value_length = 0U; + if (jl_validate_entry(raw_value, &value_offset, &value_length) != 0) { + return -1; + } + + char *source = NULL; + size_t source_length = 0; + bool missing = false; + jl_file_snapshot_t snapshot; + if (jl_read_file(file_path, &source, &source_length, &missing, &snapshot) != 0) { + return -1; + } + bool content_matches = + expected_content + ? !missing && source_length == expected_length && + (expected_length == 0U || memcmp(source, expected_content, expected_length) == 0) + : missing; + if (!content_matches || missing || source_length == 0U) { + free(source); + return -1; + } + + size_t object_start = 0; + if (jl_validate_document(source, source_length, &object_start) != 0) { + free(source); + return -1; + } + for (size_t i = 0; i < path_len; i++) { + jl_object_t parent; + if (jl_scan_object(source, source_length, object_start, object_path[i], &parent) != 0 || + parent.match_count != 1U || source[parent.match.value_start] != '{') { + free(source); + return -1; + } + object_start = parent.match.value_start; + } + jl_object_t entry; + if (jl_scan_object(source, source_length, object_start, entry_key, &entry) != 0 || + entry.match_count != 1U || source[entry.match.value_start] != '{') { + free(source); + return -1; + } + jl_object_t field; + if (jl_scan_object(source, source_length, entry.match.value_start, field_key, &field) != 0 || + field.match_count != 1U) { + free(source); + return -1; + } + + size_t head = field.match.value_start; + size_t tail = field.match.value_end; + size_t updated_length = head + value_length + (source_length - tail); + char *updated = (char *)malloc(updated_length + 1U); + if (!updated) { + free(source); + return -1; + } + memcpy(updated, source, head); + memcpy(updated + head, raw_value + value_offset, value_length); + memcpy(updated + head + value_length, source + tail, source_length - tail); + updated[updated_length] = '\0'; + + int result = + jl_write_document(file_path, updated, updated_length, source, source_length, &snapshot); + free(updated); + free(source); + return result; +} + static int jl_remove_entry(const char *file_path, const char *const *object_path, size_t path_len, const char *entry_key, bool enforce_expected, const char *expected_content, size_t expected_length) { diff --git a/src/cli/config_json_like.h b/src/cli/config_json_like.h index 905721d67..23a8687e5 100644 --- a/src/cli/config_json_like.h +++ b/src/cli/config_json_like.h @@ -89,6 +89,19 @@ int cbm_json_like_upsert_entry_if_unchanged(const char *file_path, const char *c const char *entry_json, const char *expected_content, size_t expected_length); +/* Replace ONE member's value inside the entry object at object_path/entry_key, + * preserving every other byte of the document (comments, ordering, and any + * keys the client added around it). The field-merge primitive for repairing an + * annotated entry (#1630): replacing the whole entry would drop the client's + * keys. raw_value must be a single complete JSON value; the field must exist + * exactly once. Same expected-content contract as the _if_unchanged editors. */ +int cbm_json_like_replace_field_raw_if_unchanged(const char *file_path, + const char *const *object_path, size_t path_len, + const char *entry_key, const char *field_key, + const char *raw_value, + const char *expected_content, + size_t expected_length); + /* Remove entry_key from the object at object_path. A missing path or entry is * a successful no-op. Returns 0 on success and -1 on invalid input or I/O. */ int cbm_json_like_remove_entry(const char *file_path, const char *const *object_path, diff --git a/src/cli/config_yaml_edit.c b/src/cli/config_yaml_edit.c index 2d07e86f4..9be176e18 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 { @@ -291,13 +298,20 @@ static int yaml_validate_key(const char *key, size_t *out_len) { return 0; } +static int yaml_validate_text_bytes_from(const char *data, size_t len, size_t from); + static int yaml_validate_text_bytes(const char *data, size_t len) { if (len >= YAML_UTF8_BOM_LEN && (unsigned char)data[0] == YAML_BOM_BYTE_0 && (unsigned char)data[YAML_UNIT] == YAML_BOM_BYTE_1 && (unsigned char)data[YAML_ENTRY_INDENT] == YAML_BOM_BYTE_2) { return YAML_ERROR; } - for (size_t i = 0; i < len; i++) { + return yaml_validate_text_bytes_from(data, len, 0U); +} + +/* Body of the byte validation, entered past any prologue. */ +static int yaml_validate_text_bytes_from(const char *data, size_t len, size_t from) { + for (size_t i = from; i < len; i++) { unsigned char c = (unsigned char)data[i]; if (c == '\t' || c == '\0' || c == YAML_DELETE_BYTE) { return YAML_ERROR; @@ -730,9 +744,21 @@ static int yaml_read_file(const char *path, char **out_data, size_t *out_len, #endif #endif data[len] = '\0'; - if (yaml_validate_text_bytes(data, len) != 0) { - free(data); - return YAML_ERROR; + /* Documents may open with a UTF-8 BOM (#1656) — validated past it here, + * treated as a prologue by yaml_doc_init, preserved verbatim on write. + * Non-document inputs (keys, blocks, identity scalars) keep the strict + * no-BOM rule via yaml_validate_text_bytes. */ + { + size_t prologue = 0U; + if (len >= YAML_UTF8_BOM_LEN && (unsigned char)data[0] == YAML_BOM_BYTE_0 && + (unsigned char)data[YAML_UNIT] == YAML_BOM_BYTE_1 && + (unsigned char)data[YAML_ENTRY_INDENT] == YAML_BOM_BYTE_2) { + prologue = YAML_UTF8_BOM_LEN; + } + if (yaml_validate_text_bytes_from(data, len, prologue) != 0) { + free(data); + return YAML_ERROR; + } } *out_data = data; *out_len = len; @@ -1079,6 +1105,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 +1182,29 @@ static int yaml_doc_init(yaml_doc_t *doc, const char *data, size_t len) { if (yaml_build_lines(doc) != 0) { return YAML_ERROR; } + /* A UTF-8 BOM is a prologue, not content (#1656): Windows-authored + * configs routinely carry one (PowerShell 5.1 Set-Content -Encoding UTF8 + * writes it), and treating its bytes as the first key made every edit op + * fail content-independently. Skip it for structure; edits splice ranges + * at line offsets, so the BOM survives every write byte-for-byte. */ + if (doc->line_count > 0U && doc->len >= YAML_DOC_MARKER_LEN && + (unsigned char)data[0] == 0xEFU && (unsigned char)data[1] == 0xBBU && + (unsigned char)data[2] == 0xBFU) { + yaml_line_t *first = &doc->lines[0]; + if (first->start == 0U && !first->blank) { + first->start = YAML_DOC_MARKER_LEN; + if (first->start + first->indent >= first->text_end) { + first->blank = true; + } + } + } + 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 +1238,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 +1246,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 +1317,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 +1356,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 +1408,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 +1431,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 +1466,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 +1496,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 +1513,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 +1539,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 +1563,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 +1607,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 +2584,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 +2593,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 +2627,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 +2640,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 +2671,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 +2690,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 +2711,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 +2786,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 +2840,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 +3266,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 +3431,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 +3461,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_cli.c b/tests/test_cli.c index d1524fe47..8eda0bb3f 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -52,6 +52,8 @@ int cbm_cli_checksum_manifest_digest(const char *manifest_path, const char *arch void cbm_cli_set_activation_cleanup_failure_for_test(bool enabled); int cbm_cli_activation_abort_cleanup_probe_for_test(void); bool cbm_cli_activation_test_ops_installed(void); +int cbm_cli_build_yaml_stdio_mcp_block_for_test(const char *binary_path, bool goose_schema, + char *block, size_t block_size); TEST(cli_progress_visibility_policy) { ASSERT_TRUE(cbm_cli_progress_enabled(true, false)); @@ -2938,6 +2940,140 @@ TEST(cli_junie_mcp_repairs_all_known_previous_aliases_atomically) { PASS(); } +TEST(cli_goose_block_carries_required_name_issue1675) { + /* goose's ExtensionConfig::Stdio declares `name` as a required serde field + * with no default, and its loader silently drops entries that fail to + * deserialize — an entry without `name:` installs "successfully" and is + * then invisible in goose. The block is the compatibility contract. */ + char block[512]; + ASSERT_EQ(cbm_cli_build_yaml_stdio_mcp_block_for_test("/opt/codebase-memory-mcp", true, block, + sizeof(block)), + 0); + ASSERT(strstr(block, "name: codebase-memory-mcp\n") != NULL); + ASSERT(strstr(block, "type: stdio\n") != NULL); + ASSERT(strstr(block, "enabled: true\n") != NULL); + + /* The non-goose YAML schema (command-only) must stay name-free. */ + ASSERT_EQ(cbm_cli_build_yaml_stdio_mcp_block_for_test("/opt/codebase-memory-mcp", false, block, + sizeof(block)), + 0); + ASSERT(strstr(block, "name:") == NULL); + PASS(); +} + +TEST(cli_editor_mcp_field_repairs_annotated_entry_via_previous_issue1630) { + /* The relocating-update flow is the AUTHORIZED repair channel: the entry + * still names the previous managed binary and the client annotated it, so + * a wholesale rewrite would drop those keys. Only the command member may + * change; comments and client keys survive byte-for-byte. */ + char tmpdir[256]; + snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-oc-XXXXXX"); + if (!cbm_mkdtemp(tmpdir)) + FAIL("cbm_mkdtemp failed"); + char configpath[512]; + snprintf(configpath, sizeof(configpath), "%s/.claude.json", tmpdir); + write_test_file(configpath, "{\n" + " // user config\n" + " \"mcpServers\": {\n" + " \"codebase-memory-mcp\": {\n" + " \"command\": \"/old/place/codebase-memory-mcp\",\n" + " \"enabled\": true,\n" + " \"timeout\": 5\n" + " },\n" + " },\n" + "}\n"); + ASSERT_EQ(cbm_install_editor_mcp_with_previous_for_testing( + "/opt/codebase-memory-mcp", "/old/place/codebase-memory-mcp", configpath), + 0); + const char *data = read_test_file(configpath); + ASSERT_NOT_NULL(data); + ASSERT(strstr(data, "\"command\": \"/opt/codebase-memory-mcp\"") != NULL); + ASSERT(strstr(data, "/old/place/") == NULL); + ASSERT(strstr(data, "\"enabled\": true") != NULL); + ASSERT(strstr(data, "\"timeout\": 5") != NULL); + ASSERT(strstr(data, "// user config") != NULL); + test_rmdir_r(tmpdir); + PASS(); +} + +TEST(cli_opencode_moved_entry_without_authority_refuses_issue1630) { + /* POSIX never trusts a config-supplied path — with no previous-managed + * identity and no dead-path proof, a moved-looking entry is preserved + * byte-for-byte and install fails loudly for the user to inspect. */ + char tmpdir[256]; + snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-oc-XXXXXX"); + if (!cbm_mkdtemp(tmpdir)) + FAIL("cbm_mkdtemp failed"); + char configpath[512]; + snprintf(configpath, sizeof(configpath), "%s/opencode.jsonc", tmpdir); +#ifdef _WIN32 + /* A conclusively-missing fixed-drive path authorizes the repair; a + * POSIX-shaped or non-local path can never be proven absent (PATHEXT / + * remote rules) and stays refused. */ + const char *initial = "{\n" + " \"mcp\": {\n" + " \"codebase-memory-mcp\": {\n" + " \"command\": " + "[\"C:\\\\cbm-definitely-missing\\\\codebase-memory-mcp.exe\"],\n" + " \"type\": \"local\"\n" + " }\n" + " }\n" + "}\n"; + write_test_file(configpath, initial); + ASSERT_EQ(cbm_upsert_opencode_mcp("/opt/codebase-memory-mcp", configpath), 0); +#else + const char *initial = "{\n" + " \"mcp\": {\n" + " \"codebase-memory-mcp\": {\n" + " \"command\": [\"/old/place/codebase-memory-mcp\"],\n" + " \"type\": \"local\"\n" + " }\n" + " }\n" + "}\n"; + write_test_file(configpath, initial); + ASSERT(cbm_upsert_opencode_mcp("/opt/codebase-memory-mcp", configpath) != 0); + const char *data = read_test_file(configpath); + ASSERT_NOT_NULL(data); + ASSERT(strstr(data, "/old/place/") != NULL); +#endif + test_rmdir_r(tmpdir); + PASS(); +} + +TEST(cli_opencode_owns_backslash_command_issue1582) { + /* gotspatel's live file: the entry stores the Windows path with + * backslashes while the installer compares its own path with forward + * slashes — the same file, refused over the separator spelling. Ownership + * comparison must be separator-insensitive. */ + char tmpdir[256]; + snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-oc-XXXXXX"); + if (!cbm_mkdtemp(tmpdir)) + FAIL("cbm_mkdtemp failed"); + char configpath[512]; + snprintf(configpath, sizeof(configpath), "%s/opencode.json", tmpdir); + const char *initial = "{\n" + " \"mcp\": {\n" + " \"codebase-memory-mcp\": {\n" + " \"enabled\": true,\n" + " \"type\": \"local\",\n" + " \"command\": [\"C:\\\\Users\\\\Admin\\\\Programs\\\\" + "codebase-memory-mcp\\\\codebase-memory-mcp.exe\"]\n" + " }\n" + " }\n" + "}\n"; + write_test_file(configpath, initial); + ASSERT_EQ( + cbm_upsert_opencode_mcp( + "C:/Users/Admin/Programs/codebase-memory-mcp/codebase-memory-mcp.exe", configpath), + 0); + const char *data = read_test_file(configpath); + ASSERT_NOT_NULL(data); + /* Already satisfied: the annotated entry names this binary — preserved. */ + ASSERT(strcmp(data, initial) == 0); + test_rmdir_r(tmpdir); + PASS(); +} + TEST(cli_gemini_mcp_install) { /* Port of TestGeminiMCPInstall */ char tmpdir[256]; @@ -10476,7 +10612,7 @@ TEST(cli_upsert_codex_mcp_fresh) { * subprocess. Without CBM_CACHE_DIR the spawned server uses the DEFAULT * cache while the daemon uses the configured one, the two disagree, and the * handshake closes — Codex then shows no cbm tools at all. */ - ASSERT(strstr(data, "env_vars = [\"CBM_CACHE_DIR\"]") != NULL); + ASSERT(strstr(data, "env_vars = [\"CBM_CACHE_DIR\", \"CBM_RUNTIME_DIR\"]") != NULL); test_rmdir_r(tmpdir); PASS(); @@ -12868,6 +13004,10 @@ SUITE(cli) { RUN_TEST(cli_editor_mcp_uninstall); RUN_TEST(cli_junie_mcp_install_issue651); RUN_TEST(cli_junie_mcp_repairs_all_known_previous_aliases_atomically); + RUN_TEST(cli_goose_block_carries_required_name_issue1675); + RUN_TEST(cli_editor_mcp_field_repairs_annotated_entry_via_previous_issue1630); + RUN_TEST(cli_opencode_moved_entry_without_authority_refuses_issue1630); + RUN_TEST(cli_opencode_owns_backslash_command_issue1582); RUN_TEST(cli_gemini_mcp_install); RUN_TEST(cli_openclaw_mcp_install_uses_nested_servers); RUN_TEST(cli_openclaw_mcp_preserves_existing_config); diff --git a/tests/test_config_yaml_edit.c b/tests/test_config_yaml_edit.c index 23fb9e308..0c431fddd 100644 --- a/tests/test_config_yaml_edit.c +++ b/tests/test_config_yaml_edit.c @@ -913,6 +913,261 @@ 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(); +} + +TEST(config_yaml_edit_accepts_utf8_bom_issue1656) { + /* PowerShell 5.1's `Set-Content -Encoding UTF8` writes a BOM, so real + * Windows-authored Hermes configs start with EF BB BF — and every edit op + * failed content-independently (#1656's 26-byte repro is the reporter's + * 23-byte file plus this BOM). The BOM is a prologue: skip it for + * structure, preserve it byte-for-byte on write. */ + const char *initial = "\xEF\xBB\xBFmodel:\n default: test\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(memcmp(after, "\xEF\xBB\xBFmodel:", 9), 0); + ASSERT_NOT_NULL(strstr(after, " default: test\n")); + ASSERT_NOT_NULL(strstr(after, "command: \"/opt/codebase-memory-mcp\"\n")); + ASSERT_NOT_NULL(strstr(after, "cbm-context")); + free(after); + th_cleanup(fixture.dir); + PASS(); +} + +TEST(config_yaml_edit_bom_before_our_own_section_stays_single_issue1656) { + /* When the BOM immediately precedes OUR section key, the key lookup must + * still see `mcp_servers` — otherwise the upsert misses the existing + * section and appends a duplicate. */ + const char *initial = "\xEF\xBB\xBFmcp_servers:\n" + " codebase-memory-mcp:\n" + " command: \"/opt/codebase-memory-mcp\"\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); + size_t sections = 0U; + for (const char *c = after; (c = strstr(c, "mcp_servers:")) != NULL; c++) { + sections++; + } + ASSERT_EQ((int)sections, 1); + ASSERT_EQ(memcmp(after, "\xEF\xBB\xBF", 3), 0); + 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 +1949,17 @@ 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_accepts_utf8_bom_issue1656); + RUN_TEST(config_yaml_edit_bom_before_our_own_section_stays_single_issue1656); + 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);