diff --git a/src/cli/cli.c b/src/cli/cli.c index d85ebf0b6..010387adb 100644 --- a/src/cli/cli.c +++ b/src/cli/cli.c @@ -2282,6 +2282,140 @@ static int cbm_remove_openclaw_compaction(const char *config_path) { : CLI_ERR; } +/* ── OpenHands settings.json mcp_config (#1826) ─────────────── + * The mcpServers-style config installed above (cbm_install_editor_mcp into + * ~/.openhands/mcp.json) is not enough: OpenHands only loads a global MCP + * server it finds registered under settings.json -> mcp_config, in its own + * shape ({transport, command, enabled} — no args array). Agent profiles then + * opt in individually via mcp_server_refs (below). */ + +static size_t cbm_openhands_ownership_fields(cbm_json_like_object_field_t fields[3]) { + fields[0] = (cbm_json_like_object_field_t){ + .key = "transport", + .shape = CBM_JSON_LIKE_VALUE_STRING, + .expected_string = "stdio", + .flags = CBM_JSON_LIKE_FIELD_REQUIRED, + }; + fields[1] = (cbm_json_like_object_field_t){ + .key = "command", + .shape = CBM_JSON_LIKE_VALUE_STRING, + .expected_string = NULL, + .flags = CBM_JSON_LIKE_FIELD_REQUIRED | CBM_JSON_LIKE_FIELD_CAPTURE_STRING, + }; + fields[2] = (cbm_json_like_object_field_t){ + .key = "enabled", + .shape = CBM_JSON_LIKE_VALUE_LITERAL, + .expected_string = "true", + .flags = CBM_JSON_LIKE_FIELD_REQUIRED, + }; + return 3U; +} + +static char *cbm_build_openhands_mcp_entry(const char *binary_path) { + yyjson_mut_doc *doc = yyjson_mut_doc_new(NULL); + if (!doc) { + return NULL; + } + yyjson_mut_val *root = yyjson_mut_obj(doc); + bool ok = root && yyjson_mut_obj_add_strcpy(doc, root, "transport", "stdio") && + yyjson_mut_obj_add_strcpy(doc, root, "command", binary_path) && + yyjson_mut_obj_add_bool(doc, root, "enabled", true); + char *json = NULL; + if (ok) { + yyjson_mut_doc_set_root(doc, root); + json = yyjson_mut_write(doc, YYJSON_WRITE_NOFLAG, NULL); + } + yyjson_mut_doc_free(doc); + return json; +} + +/* Insert or leave alone: an already-owned entry (exact match, or annotated + * with extra keys the client added) needs no write. A same-named entry that + * does not match our shape is left untouched and reported as an error rather + * than overwritten, matching cbm_upsert_json_named_mcp's fail-closed rule for + * every other editor client. */ +static int cbm_upsert_openhands_settings_mcp(const char *binary_path, const char *settings_path) { + if (!binary_path || !settings_path) { + return CLI_ERR; + } + static const char *const path[] = {"mcp_config"}; + char *document = NULL; + size_t document_length = 0U; + int read_result = cbm_json_like_read_document(settings_path, &document, &document_length); + if (read_result < 0) { + return CLI_ERR; + } + if (read_result == 0) { + cbm_json_like_object_field_t fields[3]; + size_t field_count = cbm_openhands_ownership_fields(fields); + char *command = NULL; + int ownership = cbm_json_like_match_object_entry(document, document_length, path, 1U, + CBM_DEFAULT_MCP_SERVER_NAME, fields, + field_count, &command); + free(command); + if (ownership == CBM_JSON_LIKE_OBJECT_MATCH || + ownership == CBM_JSON_LIKE_OBJECT_MATCH_WITH_EXTRAS) { + free(document); + return CLI_OK; + } + if (ownership != CBM_JSON_LIKE_OBJECT_MISSING) { + free(document); + return CLI_ERR; + } + } + char *entry = cbm_build_openhands_mcp_entry(binary_path); + if (!entry) { + free(document); + return CLI_ERR; + } + int edit_result = cbm_json_like_upsert_entry_if_unchanged( + settings_path, path, 1U, CBM_DEFAULT_MCP_SERVER_NAME, entry, + read_result == 1 ? NULL : document, document_length); + free(entry); + free(document); + return edit_result == 0 ? CLI_OK : CLI_ERR; +} + +/* Remove only an entry that is still recognisably ours (an annotated entry is + * left in place and reported, same rule as insertion above); a missing file, + * path, or entry is a successful no-op. */ +static int cbm_remove_openhands_settings_mcp(const char *settings_path) { + if (!settings_path) { + return CLI_ERR; + } + static const char *const path[] = {"mcp_config"}; + char *document = NULL; + size_t document_length = 0U; + int read_result = cbm_json_like_read_document(settings_path, &document, &document_length); + if (read_result == 1) { + free(document); + return CLI_OK; + } + if (read_result < 0) { + free(document); + return CLI_ERR; + } + cbm_json_like_object_field_t fields[3]; + size_t field_count = cbm_openhands_ownership_fields(fields); + char *command = NULL; + int ownership = cbm_json_like_match_object_entry(document, document_length, path, 1U, + CBM_DEFAULT_MCP_SERVER_NAME, fields, + field_count, &command); + free(command); + if (ownership == CBM_JSON_LIKE_OBJECT_MISSING || ownership == CBM_JSON_LIKE_OBJECT_MISMATCH) { + free(document); + return CLI_OK; + } + if (ownership != CBM_JSON_LIKE_OBJECT_MATCH) { + free(document); + return CLI_ERR; + } + int edit_result = cbm_json_like_remove_entry_if_unchanged( + settings_path, path, 1U, CBM_DEFAULT_MCP_SERVER_NAME, document, document_length); + free(document); + return edit_result == 0 ? CLI_OK : CLI_ERR; +} + /* ── VS Code MCP (servers key with type:stdio) ────────────────── */ int cbm_install_vscode_mcp(const char *binary_path, const char *config_path) { @@ -9228,6 +9362,47 @@ static void uninstall_vscode_profile_configs(const char *code_user, const char * cbm_closedir(directory); } +static bool cbm_filename_has_suffix(const char *name, const char *suffix) { + size_t name_len = strlen(name); + size_t suffix_len = strlen(suffix); + return name_len >= suffix_len && strcmp(name + (name_len - suffix_len), suffix) == 0; +} + +/* Register or unregister our server against every existing OpenHands agent + * profile's mcp_server_refs array (#1826). A missing agent-profiles/ + * directory is a silent no-op in both directions — install must never invent + * it, and uninstall has nothing to undo there. Only *.json entries are + * touched; a profile directory may hold arbitrary notes alongside profiles. */ +static void openhands_update_profile_refs(const char *profiles_dir, bool installing, bool dry_run) { + cbm_dir_t *d = cbm_opendir(profiles_dir); + if (!d) { + return; + } + cbm_dirent_t *ent; + while ((ent = cbm_readdir(d)) != NULL) { + if (strcmp(ent->name, ".") == 0 || strcmp(ent->name, "..") == 0 || + !cbm_filename_has_suffix(ent->name, ".json")) { + continue; + } + char profile_path[CLI_BUF_1K]; + snprintf(profile_path, sizeof(profile_path), "%s/%s", profiles_dir, ent->name); + struct stat state; + if (stat(profile_path, &state) != 0 || !S_ISREG(state.st_mode) || dry_run) { + continue; + } + int result = installing ? cbm_json_like_add_unique_string(profile_path, "mcp_server_refs", + CBM_DEFAULT_MCP_SERVER_NAME) + : cbm_json_like_remove_string(profile_path, "mcp_server_refs", + CBM_DEFAULT_MCP_SERVER_NAME); + if (result != CLI_OK) { + record_agent_config_error( + !installing, "OpenHands", + installing ? "profile_refs_install" : "profile_refs_uninstall", profile_path); + } + } + cbm_closedir(d); +} + /* Install MCP configs for editor-based agents (Zed, KiloCode, VS Code, OpenClaw). */ static void install_editor_agent_configs(const cbm_detected_agents_t *agents, const char *home, const char *binary_path, bool force, bool dry_run) { @@ -9490,11 +9665,32 @@ static void install_additional_agent_configs(const cbm_detected_agents_t *agents if (agents->openhands) { char cp[CLI_BUF_1K]; char skills_dir[CLI_BUF_1K]; + char settings_path[CLI_BUF_1K]; + char profiles_dir[CLI_BUF_1K]; snprintf(cp, sizeof(cp), "%s/.openhands/mcp.json", home); snprintf(skills_dir, sizeof(skills_dir), "%s/.agents/skills", home); + snprintf(settings_path, sizeof(settings_path), "%s/.openhands/settings.json", home); + snprintf(profiles_dir, sizeof(profiles_dir), "%s/.openhands/agent-profiles", home); install_generic_agent_config("OpenHands", binary_path, cp, NULL, dry_run, cbm_install_editor_mcp); install_agent_skill("OpenHands", skills_dir, force, dry_run); + /* #1826: the mcpServers-shaped mcp.json above is not enough — OpenHands + * only loads a server registered under settings.json -> mcp_config, and + * only for agent profiles that reference it. agent-profiles/ is never + * invented; a missing directory means nothing to register into yet. */ + if (g_install_plan) { + plan_record("OpenHands", "mcp_config", settings_path); + } else { + if (!dry_run) { + if (!prepare_config_parent(settings_path) || + cbm_upsert_openhands_settings_mcp(binary_path, settings_path) != CLI_OK) { + record_agent_config_error(false, "OpenHands", "settings_mcp_install", + settings_path); + } + } + printf(" settings mcp_config: %s\n", settings_path); + openhands_update_profile_refs(profiles_dir, true, dry_run); + } } if (agents->augment) { char cp[CLI_BUF_1K]; @@ -11750,11 +11946,21 @@ static void uninstall_additional_agents(const cbm_detected_agents_t *agents, con if (agents->openhands) { char cp[CLI_BUF_1K]; char skills_dir[CLI_BUF_1K]; + char settings_path[CLI_BUF_1K]; + char profiles_dir[CLI_BUF_1K]; snprintf(cp, sizeof(cp), "%s/.openhands/mcp.json", home); snprintf(skills_dir, sizeof(skills_dir), "%s/.agents/skills", home); + snprintf(settings_path, sizeof(settings_path), "%s/.openhands/settings.json", home); + snprintf(profiles_dir, sizeof(profiles_dir), "%s/.openhands/agent-profiles", home); uninstall_agent_mcp_instr((mcp_uninstall_args_t){"OpenHands", cp, NULL}, dry_run, cbm_remove_editor_mcp_owned); printf(" removed %d skill(s)\n", cbm_remove_skills(skills_dir, dry_run)); + /* #1826 counterpart: undo the settings.json registration and every + * agent profile's mcp_server_refs entry the install above added. */ + if (!dry_run && cbm_remove_openhands_settings_mcp(settings_path) != CLI_OK) { + record_agent_config_error(true, "OpenHands", "settings_mcp_uninstall", settings_path); + } + openhands_update_profile_refs(profiles_dir, false, dry_run); } if (agents->augment) { char cp[CLI_BUF_1K]; diff --git a/src/cli/config_json_like.c b/src/cli/config_json_like.c index dc16c3c5c..3ac7802a3 100644 --- a/src/cli/config_json_like.c +++ b/src/cli/config_json_like.c @@ -1223,8 +1223,19 @@ static int jl_make_insertion(const char *text, size_t length, size_t object_star return 0; } - if (object->close_pos == gap_start || - !jl_is_space((unsigned char)text[object->close_pos - 1U])) { + /* Zero-width insertion right at close_pos: whatever byte already sits at + * close_pos - 1 is untouched and stays in the output. When the original + * had a real gap there (close_pos != gap_start, e.g. "[ \"a\" ]"), that + * preserved byte already supplies a separator on the leading side, so + * mirroring it on the trailing side (space before the bracket) matches + * the array's own loose style. The fully tight case ("[\"a\"]", nothing + * at all between the last value and the bracket) has no such byte to + * lean on: manufacture ONE space so `,new` reads `, new` — the + * comma-spacing convention every other insertion path uses — and add + * nothing after, so `new]` stays `new]` rather than gaining a trailing + * space the original never had (#1826 byte-for-byte round-trip). */ + bool tight = object->close_pos == gap_start; + if (tight || !jl_is_space((unsigned char)text[object->close_pos - 1U])) { if (jl_buffer_char(insertion, ' ') != 0) { return -1; } @@ -1235,7 +1246,7 @@ static int jl_make_insertion(const char *text, size_t length, size_t object_star if (object->trailing_comma && jl_buffer_char(insertion, ',') != 0) { return -1; } - return jl_buffer_char(insertion, ' '); + return tight ? 0 : jl_buffer_char(insertion, ' '); } static int jl_apply_edits(const char *source, size_t source_length, jl_edit_t *edits, @@ -2406,6 +2417,13 @@ int cbm_json_like_remove_entry_if_unchanged(const char *file_path, const char *c expected_length); } +/* True when [start, end) is the bare 4-byte literal token null (never a + * quoted "null" string, which is 6 bytes with the quotes) — the documented + * OpenHands "no list yet" shape for mcp_server_refs (#1826). */ +static bool jl_is_null_literal(const char *text, size_t start, size_t end) { + return end - start == 4U && memcmp(text + start, "null", 4U) == 0; +} + int cbm_json_like_add_unique_string_at_path(const char *file_path, const char *const *object_path, size_t path_len, const char *array_key, const char *string_value) { @@ -2497,6 +2515,24 @@ int cbm_json_like_add_unique_string_at_path(const char *file_path, const char *c result = jl_insert_member(source, source_length, object_start, &object, object_path, path_len, SIZE_MAX, array_key, array_json.data, array_json.length, &updated, &updated_length); + } else if (jl_is_null_literal(source, object.match.value_start, object.match.value_end)) { + /* OpenHands profiles ship `"mcp_server_refs": null` as the documented + * "no list yet" shape (#1826). Splice the built one-element array over + * the bare null token in place, preserving every other byte (trailing + * comments included) the way the object-member and array-element + * edits below already do for their own value spans. */ + size_t head = object.match.value_start; + size_t tail = object.match.value_end; + updated_length = head + array_json.length + (source_length - tail); + updated = (char *)malloc(updated_length + 1U); + if (!updated) { + result = -1; + } else { + memcpy(updated, source, head); + memcpy(updated + head, array_json.data, array_json.length); + memcpy(updated + head + array_json.length, source + tail, source_length - tail); + updated[updated_length] = '\0'; + } } else if (source[object.match.value_start] != '[') { result = -1; } else { @@ -2759,6 +2795,24 @@ static int jl_decode_field_string(const char *text, size_t start, size_t end, } return jl_decode_string_value(text, start, end, value_out) == 0 ? 0 : 1; } + if (shape == CBM_JSON_LIKE_VALUE_LITERAL) { + /* The value's token boundaries already exclude surrounding trivia + * (jl_parse_value advances pos past exactly the token). Copy the raw + * bytes verbatim so a quoted "true" never equals the bare literal + * true, and the caller's expected_string comparison decides match. */ + if (start >= end) { + return 1; + } + size_t length = end - start; + char *copy = (char *)malloc(length + 1U); + if (!copy) { + return 1; + } + memcpy(copy, text + start, length); + copy[length] = '\0'; + *value_out = copy; + return 0; + } if (shape != CBM_JSON_LIKE_VALUE_SINGLE_STRING_ARRAY || start >= end || text[start] != '[') { return 1; } @@ -2827,11 +2881,13 @@ int cbm_json_like_match_object_entry(const char *document, size_t document_lengt size_t capture_count = 0U; for (size_t i = 0U; i < field_count; ++i) { if (!fields[i].key || fields[i].key[0] == '\0' || - fields[i].shape > CBM_JSON_LIKE_VALUE_SINGLE_STRING_ARRAY || + fields[i].shape > CBM_JSON_LIKE_VALUE_LITERAL || (fields[i].flags & ~(CBM_JSON_LIKE_FIELD_REQUIRED | CBM_JSON_LIKE_FIELD_CAPTURE_STRING)) != 0U || ((fields[i].flags & CBM_JSON_LIKE_FIELD_CAPTURE_STRING) != 0U && - fields[i].shape == CBM_JSON_LIKE_VALUE_EMPTY_ARRAY)) { + (fields[i].shape == CBM_JSON_LIKE_VALUE_EMPTY_ARRAY || + fields[i].shape == CBM_JSON_LIKE_VALUE_LITERAL)) || + (fields[i].shape == CBM_JSON_LIKE_VALUE_LITERAL && !fields[i].expected_string)) { return -1; } capture_count += (fields[i].flags & CBM_JSON_LIKE_FIELD_CAPTURE_STRING) != 0U ? 1U : 0U; diff --git a/src/cli/config_json_like.h b/src/cli/config_json_like.h index 23a8687e5..57cbfbfbf 100644 --- a/src/cli/config_json_like.h +++ b/src/cli/config_json_like.h @@ -47,6 +47,11 @@ typedef enum { CBM_JSON_LIKE_VALUE_STRING, CBM_JSON_LIKE_VALUE_EMPTY_ARRAY, CBM_JSON_LIKE_VALUE_SINGLE_STRING_ARRAY, + /* An exact bare token (true/false/null/a number) matched byte-for-byte + * against expected_string. Requires expected_string and may not be + * combined with CBM_JSON_LIKE_FIELD_CAPTURE_STRING — a fixed token has + * nothing meaningful to capture for the caller. */ + CBM_JSON_LIKE_VALUE_LITERAL, } cbm_json_like_value_shape_t; enum { diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 7aa40605c..a1d1f48b5 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -17383,6 +17383,16 @@ static void register_watcher_if_enabled(cbm_mcp_server_t *srv) { } /* Background auto-index thread function */ +/* Extraction builds a THREAD-LOCAL node-type bitset cache (cbm_kind_in_set). + * Every worker thread that runs extraction must free that cache before it exits, + * or the calloc'd bitsets are orphaned when the thread's TLS is torn down and + * LeakSanitizer reports them at process exit. Parallel workers do this in + * pass_parallel.c; the in-process (sequential) auto-index runs extraction on + * THIS short-lived thread, so it must free its own cache too. Declared extern + * (not via internal/cbm/helpers.h) to avoid pulling the extraction layer's + * header into the MCP TU — the same pattern test_main.c uses for teardown. */ +extern void cbm_kind_in_set_free_cache(void); + static void *autoindex_thread(void *arg) { cbm_mcp_server_t *srv = (cbm_mcp_server_t *)arg; @@ -17422,7 +17432,8 @@ static void *autoindex_thread(void *arg) { cbm_pipeline_unlock(); cbm_pipeline_free(p); - cbm_mem_collect(); /* return mimalloc pages to OS after indexing (in-process only) */ + cbm_kind_in_set_free_cache(); /* free THIS thread's extraction bitset cache (see above) */ + cbm_mem_collect(); /* return mimalloc pages to OS after indexing (in-process only) */ if (rc == 0) { cbm_log_info("autoindex.done", "project", srv->session_project); @@ -17523,7 +17534,7 @@ static void maybe_auto_index(cbm_mcp_server_t *srv) { (void)snprintf(limit, sizeof(limit), "%d", file_limit); cbm_log_warn("autoindex.skip", "reason", file_count >= 0 ? "too_many_files" : "unsafe_or_unavailable_path", "files", - files, "limit", limit); + files, "limit", limit, "root", srv->session_root); return; } diff --git a/tests/test_cli.c b/tests/test_cli.c index 976280868..a8d793644 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -3814,6 +3814,251 @@ TEST(cli_openclaw_uninstall_removes_compaction_when_workspace_is_ambiguous) { PASS(); } +/* ═══════════════════════════════════════════════════════════════════ + * OpenHands settings.json + agent-profiles registration (#1826) + * + * A bare ~/.openhands/mcp.json is not enough: OpenHands registers global MCP + * servers under settings.json → mcp_config, and every agent profile under + * agent-profiles/ must list the server in mcp_server_refs before that agent + * may use it. Foreign content survives byte-for-byte; uninstall removes + * exactly what install added. + * ═══════════════════════════════════════════════════════════════════ */ + +typedef struct { + char home[256]; + char binary[512]; /* the path `uninstall` expects: /.local/bin/... */ + char settings[640]; + char mcp[640]; + char profiles_dir[640]; + char default_profile[768]; + char custom_profile[768]; + char note[768]; + char *saved_home; + char *saved_path; + char *saved_cache; +} openhands_fixture_t; + +static bool openhands_fixture_open(openhands_fixture_t *fx, bool with_profiles) { + snprintf(fx->home, sizeof(fx->home), "/tmp/cli-openhands-XXXXXX"); + if (!cbm_mkdtemp(fx->home)) { + return false; + } + snprintf(fx->settings, sizeof(fx->settings), "%s/.openhands/settings.json", fx->home); +#ifdef _WIN32 + snprintf(fx->binary, sizeof(fx->binary), "%s/.local/bin/codebase-memory-mcp.exe", fx->home); +#else + snprintf(fx->binary, sizeof(fx->binary), "%s/.local/bin/codebase-memory-mcp", fx->home); +#endif + snprintf(fx->mcp, sizeof(fx->mcp), "%s/.openhands/mcp.json", fx->home); + snprintf(fx->profiles_dir, sizeof(fx->profiles_dir), "%s/.openhands/agent-profiles", fx->home); + snprintf(fx->default_profile, sizeof(fx->default_profile), "%s/default.json", fx->profiles_dir); + snprintf(fx->custom_profile, sizeof(fx->custom_profile), "%s/custom.json", fx->profiles_dir); + snprintf(fx->note, sizeof(fx->note), "%s/README.txt", fx->profiles_dir); + char openhands_dir[512]; + snprintf(openhands_dir, sizeof(openhands_dir), "%s/.openhands", fx->home); + test_mkdirp(with_profiles ? fx->profiles_dir : openhands_dir); + fx->saved_home = save_test_env("HOME"); + fx->saved_path = save_test_env("PATH"); + fx->saved_cache = save_test_env("CBM_CACHE_DIR"); + cbm_setenv("HOME", fx->home, 1); + cbm_setenv("PATH", fx->home, 1); + cbm_unsetenv("CBM_CACHE_DIR"); + return true; +} + +static void openhands_fixture_close(openhands_fixture_t *fx) { + restore_test_env("HOME", fx->saved_home); + restore_test_env("PATH", fx->saved_path); + restore_test_env("CBM_CACHE_DIR", fx->saved_cache); + test_rmdir_r(fx->home); +} + +static yyjson_doc *openhands_read_doc(const char *path) { + char *data = read_test_file_alloc(path); + if (!data) { + return NULL; + } + yyjson_doc *doc = yyjson_read(data, strlen(data), 0); + free(data); + return doc; +} + +/* settings.json → mcp_config. must be {transport:"stdio", command:, + * enabled:true}; returns false for any deviation. */ +static bool openhands_settings_entry_ok(const char *settings_path, const char *name, + const char *binary) { + yyjson_doc *doc = openhands_read_doc(settings_path); + yyjson_val *root = doc ? yyjson_doc_get_root(doc) : NULL; + yyjson_val *mcp_config = root ? yyjson_obj_get(root, "mcp_config") : NULL; + yyjson_val *entry = mcp_config ? yyjson_obj_get(mcp_config, name) : NULL; + bool ok = entry && yyjson_is_obj(entry) && yyjson_obj_size(entry) == 3U; + yyjson_val *transport = ok ? yyjson_obj_get(entry, "transport") : NULL; + yyjson_val *command = ok ? yyjson_obj_get(entry, "command") : NULL; + yyjson_val *enabled = ok ? yyjson_obj_get(entry, "enabled") : NULL; + ok = ok && transport && yyjson_is_str(transport) && + strcmp(yyjson_get_str(transport), "stdio") == 0; + ok = ok && command && yyjson_is_str(command) && strcmp(yyjson_get_str(command), binary) == 0; + ok = ok && enabled && yyjson_is_true(enabled); + yyjson_doc_free(doc); + return ok; +} + +static bool openhands_settings_entry_absent(const char *settings_path, const char *name) { + yyjson_doc *doc = openhands_read_doc(settings_path); + yyjson_val *root = doc ? yyjson_doc_get_root(doc) : NULL; + yyjson_val *mcp_config = root ? yyjson_obj_get(root, "mcp_config") : NULL; + bool absent = root && (!mcp_config || !yyjson_obj_get(mcp_config, name)); + yyjson_doc_free(doc); + return absent; +} + +/* profile → mcp_server_refs must be exactly the given strings in order. */ +static bool openhands_profile_refs_equal(const char *profile_path, const char *const *expected, + size_t expected_count) { + yyjson_doc *doc = openhands_read_doc(profile_path); + yyjson_val *root = doc ? yyjson_doc_get_root(doc) : NULL; + yyjson_val *refs = root ? yyjson_obj_get(root, "mcp_server_refs") : NULL; + bool ok = refs && yyjson_is_arr(refs) && yyjson_arr_size(refs) == expected_count; + for (size_t i = 0; ok && i < expected_count; i++) { + yyjson_val *item = yyjson_arr_get(refs, i); + ok = item && yyjson_is_str(item) && strcmp(yyjson_get_str(item), expected[i]) == 0; + } + yyjson_doc_free(doc); + return ok; +} + +static bool openhands_file_equals(const char *path, const char *expected) { + char *data = read_test_file_alloc(path); + bool equal = data && expected && strcmp(data, expected) == 0; + free(data); + return equal; +} + +static const char openhands_settings_before[] = "{\n" + " \"language\": \"en\",\n" + " \"mcp_config\": {\n" + " \"other-mcp\": {\"transport\": \"stdio\", " + "\"command\": \"/opt/other\", \"enabled\": false}\n" + " },\n" + " \"llm_model\": \"gpt-x\"\n" + "}\n"; +static const char openhands_default_profile_before[] = "{\n" + " \"name\": \"default\",\n" + " \"mcp_server_refs\": null,\n" + " \"tools\": [\"bash\"]\n" + "}\n"; +static const char openhands_custom_profile_before[] = + "{\"name\": \"custom\", \"mcp_server_refs\": [\"other-mcp\"]}\n"; +static const char openhands_note_before[] = "not a profile\n"; + +/* Foreign settings keys and a foreign mcp_config sibling survive verbatim; + * a null profile ref list becomes ours, an existing list is appended once; + * a second install is byte-idempotent; uninstall restores the foreign files + * byte-for-byte and removes only our ref. */ +TEST(cli_openhands_registers_settings_mcp_config_and_profile_refs_issue1826) { + openhands_fixture_t fx; + if (!openhands_fixture_open(&fx, true)) + FAIL("cbm_mkdtemp failed"); + write_test_file(fx.settings, openhands_settings_before); + write_test_file(fx.default_profile, openhands_default_profile_before); + write_test_file(fx.custom_profile, openhands_custom_profile_before); + write_test_file(fx.note, openhands_note_before); + + cbm_install_agent_configs(fx.home, fx.binary, false, false); + + const char *const only_ours[] = {"codebase-memory-mcp"}; + const char *const appended[] = {"other-mcp", "codebase-memory-mcp"}; + bool ours_ok = openhands_settings_entry_ok(fx.settings, "codebase-memory-mcp", fx.binary); + char *settings_after = read_test_file_alloc(fx.settings); + bool foreign_ok = + settings_after && strstr(settings_after, "\"language\": \"en\"") && + strstr(settings_after, "\"llm_model\": \"gpt-x\"") && + strstr(settings_after, "\"other-mcp\": {\"transport\": \"stdio\", \"command\": " + "\"/opt/other\", \"enabled\": false}"); + bool default_ok = openhands_profile_refs_equal(fx.default_profile, only_ours, 1U); + bool custom_ok = openhands_profile_refs_equal(fx.custom_profile, appended, 2U); + char *default_after = read_test_file_alloc(fx.default_profile); + bool default_foreign_ok = default_after && strstr(default_after, "\"name\": \"default\"") && + strstr(default_after, "\"tools\": [\"bash\"]"); + bool note_ok = openhands_file_equals(fx.note, openhands_note_before); + const char *const standard_json[] = {"mcpServers", "codebase-memory-mcp", fx.binary}; + bool mcp_json_ok = test_file_contains_all(fx.mcp, standard_json, 3); + char *custom_after = read_test_file_alloc(fx.custom_profile); + + /* Second install: byte-idempotent on every touched file. */ + cbm_install_agent_configs(fx.home, fx.binary, false, false); + bool idempotent = openhands_file_equals(fx.settings, settings_after) && + openhands_file_equals(fx.default_profile, default_after) && + openhands_file_equals(fx.custom_profile, custom_after); + free(settings_after); + free(default_after); + free(custom_after); + + char *argv[] = {"uninstall", "--yes"}; + int rc = cli_test_cmd_uninstall(2, argv); + bool settings_restored = openhands_file_equals(fx.settings, openhands_settings_before); + bool custom_restored = + openhands_file_equals(fx.custom_profile, openhands_custom_profile_before); + /* null → ["codebase-memory-mcp"] → [] : the ref is gone; an empty list is + * the minimal edit (the editor cannot know the list was null before). */ + bool default_cleared = openhands_profile_refs_equal(fx.default_profile, only_ours, 0U); + bool note_restored = openhands_file_equals(fx.note, openhands_note_before); + openhands_fixture_close(&fx); + + if (!ours_ok) + FAIL("settings.json must register mcp_config.codebase-memory-mcp {stdio, binary, enabled}"); + if (!foreign_ok) + FAIL("settings.json foreign keys and the foreign mcp_config sibling must survive verbatim"); + if (!default_ok) + FAIL("a null mcp_server_refs must become [\"codebase-memory-mcp\"]"); + if (!custom_ok) + FAIL("an existing mcp_server_refs list must gain codebase-memory-mcp exactly once"); + if (!default_foreign_ok) + FAIL("profile keys around mcp_server_refs must survive verbatim"); + if (!note_ok) + FAIL("non-JSON files under agent-profiles must not be touched"); + if (!mcp_json_ok) + FAIL("the existing ~/.openhands/mcp.json registration must be kept"); + if (!idempotent) + FAIL("a second install must be byte-idempotent"); + if (rc != 0 || !settings_restored) + FAIL("uninstall must restore settings.json byte-for-byte"); + if (!custom_restored) + FAIL("uninstall must restore a profile with foreign refs byte-for-byte"); + if (!default_cleared) + FAIL("uninstall must remove our ref from the null-origin profile"); + if (!note_restored) + FAIL("uninstall must not touch non-JSON files under agent-profiles"); + PASS(); +} + +/* Fresh ~/.openhands without settings.json or agent-profiles: settings.json is + * created with only our entry, no profile directory is invented, and uninstall + * removes the entry again. */ +TEST(cli_openhands_creates_settings_and_skips_missing_profiles_issue1826) { + openhands_fixture_t fx; + if (!openhands_fixture_open(&fx, false)) + FAIL("cbm_mkdtemp failed"); + + cbm_install_agent_configs(fx.home, fx.binary, false, false); + bool ours_ok = openhands_settings_entry_ok(fx.settings, "codebase-memory-mcp", fx.binary); + struct stat st; + bool no_profiles_invented = stat(fx.profiles_dir, &st) != 0; + + char *argv[] = {"uninstall", "--yes"}; + int rc = cli_test_cmd_uninstall(2, argv); + bool removed = openhands_settings_entry_absent(fx.settings, "codebase-memory-mcp"); + openhands_fixture_close(&fx); + + if (!ours_ok) + FAIL("a missing settings.json must be created with mcp_config.codebase-memory-mcp"); + if (!no_profiles_invented) + FAIL("install must never create ~/.openhands/agent-profiles"); + if (rc != 0 || !removed) + FAIL("uninstall must remove mcp_config.codebase-memory-mcp from settings.json"); + PASS(); +} + /* ═══════════════════════════════════════════════════════════════════ * VS Code MCP config tests * ═══════════════════════════════════════════════════════════════════ */ @@ -5168,6 +5413,7 @@ TEST(cli_new_agent_install_plans_use_documented_paths) { "/.hermes/skills/codebase-memory/SKILL.md", "\"openhands\"", "/.openhands/mcp.json", + "/.openhands/settings.json", "/.agents/skills/codebase-memory/SKILL.md", "\"cline\"", "/.cline/mcp.json", @@ -14858,6 +15104,8 @@ SUITE(cli) { RUN_TEST(cli_openclaw_mcp_preserves_valid_json5); RUN_TEST(cli_openclaw_mcp_uninstall_uses_nested_servers); RUN_TEST(cli_openclaw_compaction_preserves_user_owned_section); + RUN_TEST(cli_openhands_registers_settings_mcp_config_and_profile_refs_issue1826); + RUN_TEST(cli_openhands_creates_settings_and_skips_missing_profiles_issue1826); RUN_TEST(cli_openclaw_profile_uses_profile_state_and_default_workspace); RUN_TEST(cli_openclaw_uninstall_removes_compaction_when_workspace_is_ambiguous); diff --git a/tests/test_config_json_like.c b/tests/test_config_json_like.c index c6f29a387..8e6a2b28c 100644 --- a/tests/test_config_json_like.c +++ b/tests/test_config_json_like.c @@ -671,6 +671,176 @@ TEST(config_json_like_top_level_array_unique_string) { PASS(); } +/* OpenHands profiles ship `"mcp_server_refs": null` (#1826). A null value is + * the documented "no list yet" shape, so adding a unique string must replace + * exactly that token with a one-element array, preserving everything around + * it, and stay idempotent. Removal leaves the minimal edit: an empty array. */ +TEST(config_json_like_array_add_replaces_null_value_issue1826) { + jl_fixture_t fixture; + ASSERT_EQ(jl_fixture_open(&fixture), 0); + const char *original = "{\n" + " \"name\": \"default\",\n" + " \"mcp_server_refs\": null, // keep this user comment\n" + " \"tools\": [\"bash\"]\n" + "}\n"; + ASSERT_EQ(jl_write(fixture.path, original), 0); + ASSERT_EQ( + cbm_json_like_add_unique_string(fixture.path, "mcp_server_refs", "codebase-memory-mcp"), 0); + char *first = jl_read(fixture.path); + ASSERT_NOT_NULL(first); + ASSERT_STR_EQ(first, + "{\n" + " \"name\": \"default\",\n" + " \"mcp_server_refs\": [\"codebase-memory-mcp\"], // keep this user comment\n" + " \"tools\": [\"bash\"]\n" + "}\n"); + + ASSERT_EQ( + cbm_json_like_add_unique_string(fixture.path, "mcp_server_refs", "codebase-memory-mcp"), 0); + char *second = jl_read(fixture.path); + ASSERT_NOT_NULL(second); + ASSERT_STR_EQ(second, first); + free(first); + free(second); + + ASSERT_EQ(cbm_json_like_remove_string(fixture.path, "mcp_server_refs", "codebase-memory-mcp"), + 0); + char *removed = jl_read(fixture.path); + ASSERT_NOT_NULL(removed); + ASSERT_STR_EQ(removed, "{\n" + " \"name\": \"default\",\n" + " \"mcp_server_refs\": [], // keep this user comment\n" + " \"tools\": [\"bash\"]\n" + "}\n"); + free(removed); + + /* Only the literal null converts; any other non-array value still fails + * closed byte-identically. */ + const char *wrong_type = "{\"mcp_server_refs\": \"null\", \"keep\": true}\n"; + ASSERT_EQ(jl_write(fixture.path, wrong_type), 0); + ASSERT_EQ(cbm_json_like_add_unique_string(fixture.path, "mcp_server_refs", "x"), -1); + char *content = jl_read(fixture.path); + ASSERT_NOT_NULL(content); + ASSERT_STR_EQ(content, wrong_type); + free(content); + jl_fixture_close(&fixture); + PASS(); +} + +/* The OpenHands settings entry carries `enabled: true` (#1826). A literal shape + * lets ownership matching require that token exactly, so uninstall recognises + * our own entry instead of treating the boolean as a foreign annotation. */ +static int jl_match_openhands_entry(const char *document, + const cbm_json_like_object_field_t *fields, size_t field_count, + char **captured) { + static const char *const path[] = {"mcp_config"}; + return cbm_json_like_match_object_entry(document, strlen(document), path, 1U, + "codebase-memory-mcp", fields, field_count, captured); +} + +TEST(config_json_like_match_object_entry_literal_shape_issue1826) { + const cbm_json_like_object_field_t fields[] = { + {.key = "command", + .shape = CBM_JSON_LIKE_VALUE_STRING, + .expected_string = NULL, + .flags = CBM_JSON_LIKE_FIELD_REQUIRED | CBM_JSON_LIKE_FIELD_CAPTURE_STRING}, + {.key = "transport", + .shape = CBM_JSON_LIKE_VALUE_STRING, + .expected_string = "stdio", + .flags = CBM_JSON_LIKE_FIELD_REQUIRED}, + {.key = "enabled", + .shape = CBM_JSON_LIKE_VALUE_LITERAL, + .expected_string = "true", + .flags = CBM_JSON_LIKE_FIELD_REQUIRED}, + }; + char *captured = NULL; + const char *ours = "{\"mcp_config\": {\"codebase-memory-mcp\": {\"transport\": \"stdio\", " + "\"command\": \"/x/cbm\", \"enabled\": true}}}\n"; + ASSERT_EQ(jl_match_openhands_entry(ours, fields, 3U, &captured), CBM_JSON_LIKE_OBJECT_MATCH); + ASSERT_NOT_NULL(captured); + ASSERT_STR_EQ(captured, "/x/cbm"); + free(captured); + + /* Trivia around the token is not part of the value. */ + const char *spaced = "{\"mcp_config\": {\"codebase-memory-mcp\": {\"transport\": \"stdio\", " + "\"command\": \"/x/cbm\", \"enabled\" : true /* on */ }}}\n"; + ASSERT_EQ(jl_match_openhands_entry(spaced, fields, 3U, &captured), CBM_JSON_LIKE_OBJECT_MATCH); + free(captured); + + const char *disabled = "{\"mcp_config\": {\"codebase-memory-mcp\": {\"transport\": \"stdio\", " + "\"command\": \"/x/cbm\", \"enabled\": false}}}\n"; + ASSERT_EQ(jl_match_openhands_entry(disabled, fields, 3U, &captured), + CBM_JSON_LIKE_OBJECT_MISMATCH); + ASSERT_NULL(captured); + + const char *quoted = "{\"mcp_config\": {\"codebase-memory-mcp\": {\"transport\": \"stdio\", " + "\"command\": \"/x/cbm\", \"enabled\": \"true\"}}}\n"; + ASSERT_EQ(jl_match_openhands_entry(quoted, fields, 3U, &captured), + CBM_JSON_LIKE_OBJECT_MISMATCH); + ASSERT_NULL(captured); + + const char *missing = "{\"mcp_config\": {\"codebase-memory-mcp\": {\"transport\": \"stdio\", " + "\"command\": \"/x/cbm\"}}}\n"; + ASSERT_EQ(jl_match_openhands_entry(missing, fields, 3U, &captured), + CBM_JSON_LIKE_OBJECT_MISMATCH); + ASSERT_NULL(captured); + + /* A literal field can neither lack its token nor be captured. */ + const cbm_json_like_object_field_t no_token[] = { + fields[0], + {.key = "enabled", + .shape = CBM_JSON_LIKE_VALUE_LITERAL, + .expected_string = NULL, + .flags = CBM_JSON_LIKE_FIELD_REQUIRED}, + }; + ASSERT_EQ(jl_match_openhands_entry(ours, no_token, 2U, &captured), -1); + const cbm_json_like_object_field_t captured_literal[] = { + {.key = "enabled", + .shape = CBM_JSON_LIKE_VALUE_LITERAL, + .expected_string = "true", + .flags = CBM_JSON_LIKE_FIELD_REQUIRED | CBM_JSON_LIKE_FIELD_CAPTURE_STRING}, + }; + ASSERT_EQ(jl_match_openhands_entry(ours, captured_literal, 1U, &captured), -1); + PASS(); +} + +/* An OpenHands profile with an existing single-line list (#1826): the added + * name follows the list's own spacing, and removing it again restores the + * original bytes exactly — no stray space before the closing bracket. */ +static bool jl_round_trips(jl_fixture_t *fixture, const char *original, const char *expected) { + if (jl_write(fixture->path, original) != 0 || + cbm_json_like_add_unique_string(fixture->path, "mcp_server_refs", "codebase-memory-mcp") != + 0) { + return false; + } + char *added = jl_read(fixture->path); + bool ok = added && strcmp(added, expected) == 0; + free(added); + if (!ok || + cbm_json_like_remove_string(fixture->path, "mcp_server_refs", "codebase-memory-mcp") != 0) { + return false; + } + char *restored = jl_read(fixture->path); + ok = restored && strcmp(restored, original) == 0; + free(restored); + return ok; +} + +TEST(config_json_like_single_line_array_round_trips_byte_for_byte_issue1826) { + jl_fixture_t fixture; + ASSERT_EQ(jl_fixture_open(&fixture), 0); + ASSERT_TRUE(jl_round_trips( + &fixture, "{\"name\": \"custom\", \"mcp_server_refs\": [\"other-mcp\"]}\n", + "{\"name\": \"custom\", \"mcp_server_refs\": [\"other-mcp\", \"codebase-memory-mcp\"]}\n")); + ASSERT_TRUE( + jl_round_trips(&fixture, "{ \"mcp_server_refs\": [ \"other-mcp\" ] }\n", + "{ \"mcp_server_refs\": [ \"other-mcp\", \"codebase-memory-mcp\" ] }\n")); + ASSERT_TRUE(jl_round_trips(&fixture, "{\"mcp_server_refs\": [ ]}\n", + "{\"mcp_server_refs\": [ \"codebase-memory-mcp\" ]}\n")); + jl_fixture_close(&fixture); + PASS(); +} + TEST(config_json_like_top_level_array_create_escape_and_fail_closed) { jl_fixture_t fixture; ASSERT_EQ(jl_fixture_open(&fixture), 0); @@ -1025,6 +1195,9 @@ SUITE(config_json_like) { RUN_TEST(config_json_like_removes_first_middle_last_and_only); RUN_TEST(config_json_like_removal_preserves_comments_and_siblings); RUN_TEST(config_json_like_top_level_array_unique_string); + RUN_TEST(config_json_like_array_add_replaces_null_value_issue1826); + RUN_TEST(config_json_like_match_object_entry_literal_shape_issue1826); + RUN_TEST(config_json_like_single_line_array_round_trips_byte_for_byte_issue1826); RUN_TEST(config_json_like_top_level_array_create_escape_and_fail_closed); RUN_TEST(config_json_like_nested_array_creates_missing_path_and_escapes); RUN_TEST(config_json_like_nested_array_preserves_jsonc_and_is_idempotent); diff --git a/tests/test_mcp.c b/tests/test_mcp.c index 6fbaaf66c..3234542c5 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -18943,22 +18943,72 @@ TEST(mcp_auto_watch_false_skips_watcher_on_connect) { } /* ══════════════════════════════════════════════════════════════════ - * #1466 — autoindex.skip must report the effective numeric limit + * #1466 / #713 — the auto_index_limit guard + * + * #1466: autoindex.skip must report the effective numeric limit. + * #713: the guard must bound NON-git roots. The 0.8.x guard counted + * `git ls-files | wc -l`, which is 0 outside a checkout, so a plain + * directory of 60k files was walked in full (tens of GB RSS). The + * bounded discovery count applies to every root, and the skip line + * names the limit AND the root so the reporter can act on it. * ══════════════════════════════════════════════════════════════════ */ static char autoindex_skip_log[1024]; +static bool autoindex_saw_done; -/* Keeps only the too_many_files skip line, so later lines cannot displace it. */ -static void autoindex_skip_capture_log(const char *line) { - if (line && strstr(line, "msg=autoindex.skip") && strstr(line, "reason=too_many_files")) { +/* Keeps only the too_many_files skip line, so later lines cannot displace it, + * and records whether an admitted auto-index actually ran to completion. */ +static void autoindex_limit_capture_log(const char *line) { + if (!line) { + return; + } + if (strstr(line, "msg=autoindex.skip") && strstr(line, "reason=too_many_files")) { snprintf(autoindex_skip_log, sizeof(autoindex_skip_log), "%s", line); } + if (strstr(line, "msg=autoindex.done")) { + autoindex_saw_done = true; + } +} + +typedef struct { + int files; /* indexable .py files written into the root */ + int limit; /* auto_index_limit */ + bool git_root; /* emulate a checkout via /.git/HEAD (no git binary) */ +} autoindex_limit_probe_t; + +static bool autoindex_limit_write_fixture(const char *repodir, + const autoindex_limit_probe_t *probe) { + if (th_mkdir_p(repodir) != 0) { + return false; + } + for (int i = 0; i < probe->files; i++) { + char path[640]; + char body[96]; + snprintf(path, sizeof(path), "%s/f%d.py", repodir, i); + snprintf(body, sizeof(body), "def f%d():\n return %d\n", i, i); + if (th_write_file(path, body) != 0) { + return false; + } + } + if (probe->git_root) { + char gitdir[640]; + char head[700]; + snprintf(gitdir, sizeof(gitdir), "%s/.git", repodir); + snprintf(head, sizeof(head), "%s/HEAD", gitdir); + if (th_mkdir_p(gitdir) != 0 || th_write_file(head, "ref: refs/heads/main\n") != 0) { + return false; + } + } + return true; } -/* Drive initialize → maybe_auto_index over a fresh project holding more tracked - * files than auto_index_limit, and capture the resulting skip warning. +/* Drive initialize → maybe_auto_index over a fresh root holding probe->files + * indexable files under auto_index_limit=probe->limit. skip_out receives the + * too_many_files warning (empty when admitted); *done_out reports whether an + * admitted auto-index ran to completion (the server free joins the thread). * Returns false on fixture setup failure. */ -static bool autoindex_skip_warning(char *out, size_t out_size) { +static bool autoindex_limit_probe(const autoindex_limit_probe_t *probe, char *skip_out, + size_t skip_size, bool *done_out) { char cache[256]; snprintf(cache, sizeof(cache), "%s/cbm-autoindex-limit-XXXXXX", cbm_tmpdir()); if (!cbm_mkdtemp(cache)) { @@ -18967,12 +19017,7 @@ static bool autoindex_skip_warning(char *out, size_t out_size) { char repodir[512]; snprintf(repodir, sizeof(repodir), "%s/repo", cache); - char file_a[640]; - char file_b[640]; - snprintf(file_a, sizeof(file_a), "%s/a.py", repodir); - snprintf(file_b, sizeof(file_b), "%s/b.py", repodir); - if (th_mkdir_p(repodir) != 0 || th_write_file(file_a, "def a():\n return 1\n") != 0 || - th_write_file(file_b, "def b():\n return 2\n") != 0) { + if (!autoindex_limit_write_fixture(repodir, probe)) { th_rmtree(cache); return false; } @@ -18992,27 +19037,31 @@ static bool autoindex_skip_warning(char *out, size_t out_size) { bool ok = false; cbm_config_t *cfg = cbm_config_open(cache); if (cfg) { + char limit[32]; + snprintf(limit, sizeof(limit), "%d", probe->limit); cbm_config_set(cfg, CBM_CONFIG_AUTO_INDEX, "true"); - cbm_config_set(cfg, CBM_CONFIG_AUTO_INDEX_LIMIT, "1"); + cbm_config_set(cfg, CBM_CONFIG_AUTO_INDEX_LIMIT, limit); cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); if (srv) { autoindex_skip_log[0] = '\0'; + autoindex_saw_done = false; CBMLogLevel prev_level = cbm_log_get_level(); - cbm_log_set_level(CBM_LOG_WARN); + cbm_log_set_level(CBM_LOG_INFO); cbm_log_set_format(CBM_LOG_FORMAT_TEXT); - cbm_log_set_sink_ex(autoindex_skip_capture_log, CBM_LOG_SINK_REPLACE); + cbm_log_set_sink_ex(autoindex_limit_capture_log, CBM_LOG_SINK_REPLACE); cbm_mcp_server_set_config(srv, cfg); char *resp = cbm_mcp_server_handle( srv, "{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"initialize\",\"params\":{}}"); free(resp); - cbm_mcp_server_free(srv); + cbm_mcp_server_free(srv); /* joins an admitted autoindex thread */ cbm_log_set_sink(NULL); cbm_log_set_level(prev_level); - snprintf(out, out_size, "%s", autoindex_skip_log); + snprintf(skip_out, skip_size, "%s", autoindex_skip_log); + *done_out = autoindex_saw_done; ok = true; } cbm_config_close(cfg); @@ -19028,8 +19077,10 @@ static bool autoindex_skip_warning(char *out, size_t out_size) { /* RED before the fix: the warning carries `limit=auto_index_limit`, the config * key constant, instead of the configured value. */ TEST(autoindex_skip_reports_numeric_limit_issue1466) { + autoindex_limit_probe_t probe = {.files = 2, .limit = 1, .git_root = false}; char warning[1024]; - if (!autoindex_skip_warning(warning, sizeof(warning))) { + bool done = false; + if (!autoindex_limit_probe(&probe, warning, sizeof(warning), &done)) { PASS(); /* fixture setup failed (tmpdir/cwd unavailable) — skip */ } /* Not vacuous: the skip path must actually have been taken. */ @@ -19038,6 +19089,58 @@ TEST(autoindex_skip_reports_numeric_limit_issue1466) { ASSERT_NOT_NULL(strstr(warning, "files=2")); ASSERT_NOT_NULL(strstr(warning, "limit=1")); ASSERT_NULL(strstr(warning, "limit=auto_index_limit")); + ASSERT_FALSE(done); + PASS(); +} + +/* #713 at the reporter's shape scaled down (60 files vs limit 50, from + * 60k vs 50k): a PLAIN directory over the limit is refused, and the line + * names the limit and the root. RED on the `git ls-files` guard (count 0 → + * admitted → indexed) and RED while the line does not name the root. */ +TEST(autoindex_limit_guards_non_git_root_issue713) { + autoindex_limit_probe_t probe = {.files = 60, .limit = 50, .git_root = false}; + char warning[1024]; + bool done = false; + if (!autoindex_limit_probe(&probe, warning, sizeof(warning), &done)) { + PASS(); /* fixture setup failed (tmpdir/cwd unavailable) — skip */ + } + ASSERT_NOT_NULL(strstr(warning, "msg=autoindex.skip")); + ASSERT_NOT_NULL(strstr(warning, "reason=too_many_files")); + ASSERT_NOT_NULL(strstr(warning, "limit=50")); + const char *root = strstr(warning, "root="); + ASSERT_NOT_NULL(root); + ASSERT_NOT_NULL(strstr(root, "/repo")); + ASSERT_FALSE(done); + PASS(); +} + +/* The same plain directory one file UNDER the limit is admitted and indexed: + * the guard bounds, it does not fail closed on every non-git root. */ +TEST(autoindex_limit_admits_non_git_root_under_limit_issue713) { + autoindex_limit_probe_t probe = {.files = 49, .limit = 50, .git_root = false}; + char warning[1024]; + bool done = false; + if (!autoindex_limit_probe(&probe, warning, sizeof(warning), &done)) { + PASS(); /* fixture setup failed (tmpdir/cwd unavailable) — skip */ + } + ASSERT(warning[0] == '\0'); + ASSERT_TRUE(done); + PASS(); +} + +/* A checkout keeps its behaviour: one bounded count for both root kinds. */ +TEST(autoindex_limit_guards_git_root_issue713) { + autoindex_limit_probe_t probe = {.files = 60, .limit = 50, .git_root = true}; + char warning[1024]; + bool done = false; + if (!autoindex_limit_probe(&probe, warning, sizeof(warning), &done)) { + PASS(); /* fixture setup failed (tmpdir/cwd unavailable) — skip */ + } + ASSERT_NOT_NULL(strstr(warning, "msg=autoindex.skip")); + ASSERT_NOT_NULL(strstr(warning, "reason=too_many_files")); + ASSERT_NOT_NULL(strstr(warning, "limit=50")); + ASSERT_NOT_NULL(strstr(warning, "root=")); + ASSERT_FALSE(done); PASS(); } @@ -20320,6 +20423,9 @@ SUITE(mcp) { RUN_TEST(mcp_auto_watch_false_skips_watcher_on_connect); RUN_TEST(mcp_auto_watch_false_skips_supervised_autoindex_issue853); RUN_TEST(autoindex_skip_reports_numeric_limit_issue1466); + RUN_TEST(autoindex_limit_guards_non_git_root_issue713); + RUN_TEST(autoindex_limit_admits_non_git_root_under_limit_issue713); + RUN_TEST(autoindex_limit_guards_git_root_issue713); } /* Kept separate so daemon-coordination regressions can be iterated without