Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions src/cli/config_yaml_edit.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down
33 changes: 33 additions & 0 deletions tests/test_config_yaml_edit.c
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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);
Expand Down
Loading