diff --git a/src/cli/config_yaml_edit.c b/src/cli/config_yaml_edit.c index 9be176e18..e084b59c1 100644 --- a/src/cli/config_yaml_edit.c +++ b/src/cli/config_yaml_edit.c @@ -3601,8 +3601,49 @@ int cbm_yaml_upsert_mapping_entry(const char *file_path, const char *section_key return result == 0 && release_result == 0 ? 0 : YAML_ERROR; } +/* Removal is idempotent: a config that does not exist has nothing to remove. + * The JSON removers already answer OK here (cbm_json_like_read_document's + * "missing" result); the YAML removers instead took the lock first, and the + * lock directory is created beside the target — so with the parent directory + * absent (`~/.hermes/` never created because Hermes was detected by its + * binary on PATH, not by its home) every uninstall reported + * "does not exist or cannot be inspected" and refused to remove the + * executable. Only a true ENOENT short-circuits: a dangling symlink still + * reaches the reader and its byte-identical rejection. */ +static bool yaml_remove_target_absent(const char *path) { + if (!path) { + return false; + } +#ifdef _WIN32 + /* FILE_FLAG_OPEN_REPARSE_POINT, as in toml_read_file: opens the link + * itself rather than following it, so a dangling symlink does NOT count as + * absent and still reaches the reader — matching the POSIX branch, where + * lstat does not follow either. */ + wchar_t *wide = cbm_utf8_to_wide(path); + if (!wide) { + return false; + } + HANDLE handle = CreateFileW( + wide, GENERIC_READ, FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, NULL, + OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL | FILE_FLAG_OPEN_REPARSE_POINT, NULL); + DWORD error = handle == INVALID_HANDLE_VALUE ? GetLastError() : 0; + free(wide); + if (handle != INVALID_HANDLE_VALUE) { + CloseHandle(handle); + return false; + } + return error == ERROR_FILE_NOT_FOUND || error == ERROR_PATH_NOT_FOUND; +#else + struct stat path_state; + return lstat(path, &path_state) != 0 && errno == ENOENT; +#endif +} + int cbm_yaml_remove_mapping_entry(const char *file_path, const char *section_key, const char *entry_key) { + if (yaml_remove_target_absent(file_path)) { + return 0; + } yaml_config_lock_t lock; if (yaml_lock_acquire(file_path, &lock) != 0) { return YAML_ERROR; @@ -3626,6 +3667,9 @@ int cbm_yaml_upsert_owned_mapping_entry(const char *file_path, const char *secti int cbm_yaml_remove_owned_mapping_entry(const char *file_path, const char *section_key, const char *entry_key, const char *canonical_entry_block) { + if (yaml_remove_target_absent(file_path)) { + return CBM_YAML_IDENTITY_EDIT_OK; + } yaml_config_lock_t lock; if (yaml_lock_acquire(file_path, &lock) != 0) { return CBM_YAML_IDENTITY_EDIT_ERROR; @@ -3653,6 +3697,9 @@ int cbm_yaml_upsert_mapping_sequence_item(const char *file_path, const char *con int cbm_yaml_remove_mapping_sequence_item(const char *file_path, const char *const *sequence_path, size_t sequence_path_len, const char *identity_key, const char *identity_scalar, const char *canonical_item) { + if (yaml_remove_target_absent(file_path)) { + return CBM_YAML_IDENTITY_EDIT_OK; + } yaml_config_lock_t lock; if (yaml_lock_acquire(file_path, &lock) != 0) { return CBM_YAML_IDENTITY_EDIT_ERROR; @@ -3675,6 +3722,9 @@ int cbm_yaml_upsert_string_list_item(const char *file_path, const char *key, con } int cbm_yaml_remove_string_list_item(const char *file_path, const char *key, const char *item) { + if (yaml_remove_target_absent(file_path)) { + return 0; + } yaml_config_lock_t lock; if (yaml_lock_acquire(file_path, &lock) != 0) { return YAML_ERROR; diff --git a/tests/test_config_yaml_edit.c b/tests/test_config_yaml_edit.c index 0c431fddd..3e290240c 100644 --- a/tests/test_config_yaml_edit.c +++ b/tests/test_config_yaml_edit.c @@ -2,6 +2,7 @@ * test_config_yaml_edit.c — Conservative YAML config editor tests. */ #include "../src/foundation/compat.h" +#include "../src/foundation/compat_fs.h" #include "test_framework.h" #include "test_helpers.h" @@ -1826,6 +1827,37 @@ TEST(config_yaml_edit_nested_sequence_removes_only_exact_canonical_item) { PASS(); } +/* Uninstall must be idempotent against a config that was never written. Hermes + * is detected by its binary on PATH, so a HOME without `.hermes/` reached the + * YAML removers with an absent target — and they failed acquiring the lock + * beside a file whose directory does not exist, which `uninstall` then + * counted as an agent cleanup error and refused to remove the executable. + * Every remover answers OK for a genuinely absent path and creates nothing. + * Runs on every platform: the fix has a separate Windows branch. */ +TEST(config_yaml_edit_remove_on_absent_config_is_a_no_op) { + yaml_fixture_t fixture; + ASSERT_EQ(yaml_fixture_init(&fixture, NULL), 0); + char absent[sizeof(fixture.path) + 32U]; + ASSERT(snprintf(absent, sizeof(absent), "%s/no-such-dir/config.yaml", fixture.dir) > 0); + + ASSERT_EQ(cbm_yaml_remove_mapping_sequence_item(absent, yaml_hook_sequence_path, 2U, "id", + yaml_hook_identity, yaml_hook_canonical_item), + CBM_YAML_IDENTITY_EDIT_OK); + ASSERT_EQ(cbm_yaml_remove_owned_mapping_entry(absent, "mcp_servers", "codebase-memory-mcp", + " command: \"/opt/cbm\"\n"), + CBM_YAML_IDENTITY_EDIT_OK); + ASSERT_EQ(cbm_yaml_remove_mapping_entry(absent, "hooks", "pre_llm_call"), 0); + ASSERT_EQ(cbm_yaml_remove_string_list_item(absent, "read", "AGENTS.md"), 0); + + /* Nothing was created. cbm_path_info_utf8 rather than lstat: same meaning + * — it reports the link instead of following it — and it compiles on + * Windows, which is exactly where the fix has its own branch. */ + cbm_path_info_t absent_info; + ASSERT(cbm_path_info_utf8(absent, &absent_info) != 0); + th_cleanup(fixture.dir); + PASS(); +} + TEST(config_yaml_edit_nested_sequence_ambiguity_fails_byte_identically) { const char *cases[] = { "hooks:\n pre_llm_call:\n - id: \"bad-indent\"\n", @@ -1980,6 +2012,7 @@ SUITE(config_yaml_edit) { RUN_TEST(config_yaml_edit_nested_sequence_foreign_identity_is_preserved); RUN_TEST(config_yaml_edit_nested_sequence_removes_only_exact_canonical_item); RUN_TEST(config_yaml_edit_nested_sequence_ambiguity_fails_byte_identically); + RUN_TEST(config_yaml_edit_remove_on_absent_config_is_a_no_op); #ifndef _WIN32 RUN_TEST(config_yaml_edit_accepts_interior_asterisk_in_plain_scalar_issue1631); RUN_TEST(config_yaml_edit_still_refuses_leading_alias_issue1631);