fix(cli): complete the OpenHands integration (settings.json mcp_config + agent-profile refs) (#1826)

install wrote only part of the OpenHands config. Register the server globally
under ~/.openhands/settings.json -> mcp_config {transport:stdio, command, enabled}
(owned-entry, unrelated content preserved), and add "codebase-memory-mcp" to
each ~/.openhands/agent-profiles/<p>.json mcp_server_refs array (null -> [it],
existing -> appended once). uninstall removes exactly what it added.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This commit is contained in:
Martin VogelandClaude Fable 5.1 committed 2026-09-08 20:38:22 +02:00
1 parent 208ee99c86
commit 3a4160b461
5 files changed
+696 -5

No files matched your search

+209
View File
@@ -2282,6 +2282,141 @@ 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 +9363,49 @@ 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 +9668,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 +11949,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];
+61 -5
View File
@@ -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;
+5
View File
@@ -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 {
+248
View File
@@ -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: <home>/.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.<name> must be {transport:"stdio", command:<binary>,
* 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);
+173
View File
@@ -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);