From f277034fe5cd6d9bfca2895c25f3822562a7ad3b Mon Sep 17 00:00:00 2001 From: pcristin Date: Sun, 16 Aug 2026 12:05:07 +0000 Subject: [PATCH] fix(yaml): accept empty flow mappings in mapping bodies Signed-off-by: pcristin --- src/cli/config_yaml_edit.c | 19 ++++++- tests/test_config_yaml_edit.c | 104 ++++++++++++++++++++++++++++++++++ 2 files changed, 120 insertions(+), 3 deletions(-) diff --git a/src/cli/config_yaml_edit.c b/src/cli/config_yaml_edit.c index 48e2039f4..2d07e86f4 100644 --- a/src/cli/config_yaml_edit.c +++ b/src/cli/config_yaml_edit.c @@ -1266,8 +1266,8 @@ static int yaml_find_comment(const char *data, size_t start, size_t end, size_t * The test is positional and deliberately conservative: an indicator is only * honoured as one when nothing has appeared before it in the value. `key: *a` * is still an alias and still refused; `key: 2 * 3` and `key: a*b` are text. - * `{`/`}` keep their existing treatment — empty flow mappings are a separate - * change with its own semantics. */ + * `{`/`}` keep their existing treatment here; mapping-body validation admits + * exact empty flow mappings through a context-specific exception. */ static int yaml_range_has_unsupported(const char *data, size_t start, size_t end) { char quote = '\0'; /* Last significant character seen, so an indicator can be judged by what @@ -1387,6 +1387,19 @@ static int yaml_tail_is_explicit_empty_mapping(const char *data, size_t colon, s return 0; } +static int yaml_mapping_body_line_has_unsupported(const yaml_doc_t *doc, const yaml_line_t *line) { + size_t start = line->start + line->indent; + size_t colon = 0U; + bool empty_mapping = false; + if (yaml_find_mapping_colon(doc->data, start, line->text_end, &colon) == 0 && + yaml_tail_is_explicit_empty_mapping(doc->data, colon, line->text_end, &empty_mapping) == + 0 && + empty_mapping) { + return yaml_range_has_unsupported(doc->data, start, colon + YAML_UNIT); + } + return yaml_range_has_unsupported(doc->data, start, line->text_end); +} + static int yaml_value_starts_multiline(const char *data, size_t colon, size_t end) { size_t pos = yaml_skip_spaces(data, colon + YAML_UNIT, end); return pos < end && (data[pos] == '|' || data[pos] == '>'); @@ -1404,7 +1417,7 @@ static int yaml_validate_mapping_body(const yaml_doc_t *doc, size_t first_line, continue; } if (line->indent < YAML_ENTRY_INDENT || (line->indent & YAML_UNIT) != 0U || - yaml_range_has_unsupported(doc->data, line->start + line->indent, line->text_end)) { + yaml_mapping_body_line_has_unsupported(doc, line)) { return YAML_ERROR; } size_t colon = 0U; diff --git a/tests/test_config_yaml_edit.c b/tests/test_config_yaml_edit.c index 3c963747d..23fb9e308 100644 --- a/tests/test_config_yaml_edit.c +++ b/tests/test_config_yaml_edit.c @@ -812,6 +812,107 @@ TEST(config_yaml_edit_goose_extensions_preserve_siblings) { PASS(); } +TEST(config_yaml_edit_goose_accepts_empty_flow_mapping_in_sibling_issue1673) { + const char *initial = "extensions:\n" + " demo:\n" + " enabled: true\n" + " type: stdio\n" + " cmd: /bin/true\n" + " args:\n" + " - --serve\n" + " envs: {}\n" + " env_keys: []\n" + " timeout: 300\n" + "GOOSE_THINKING_EFFORT: max\n"; + const char *block = " type: stdio\n" + " cmd: \"/opt/codebase-memory-mcp\"\n" + " args: []\n" + " enabled: true\n"; + const char *expected = "extensions:\n" + " demo:\n" + " enabled: true\n" + " type: stdio\n" + " cmd: /bin/true\n" + " args:\n" + " - --serve\n" + " envs: {}\n" + " env_keys: []\n" + " timeout: 300\n" + " codebase-memory-mcp:\n" + " type: stdio\n" + " cmd: \"/opt/codebase-memory-mcp\"\n" + " args: []\n" + " enabled: true\n" + "GOOSE_THINKING_EFFORT: max\n"; + yaml_fixture_t fixture; + ASSERT_EQ(yaml_fixture_init(&fixture, initial), 0); + + ASSERT_EQ(cbm_yaml_upsert_owned_mapping_entry(fixture.path, "extensions", "codebase-memory-mcp", + block), + CBM_YAML_IDENTITY_EDIT_OK); + char *installed = yaml_read_alloc(fixture.path); + ASSERT_NOT_NULL(installed); + ASSERT_STR_EQ(installed, expected); + + free(installed); + th_cleanup(fixture.dir); + PASS(); +} + +TEST(config_yaml_edit_goose_still_rejects_nonempty_flow_mapping_issue1673) { + const char *initial = "extensions:\n" + " demo:\n" + " enabled: true\n" + " type: stdio\n" + " cmd: /bin/true\n" + " args: []\n" + " envs: {FOO: bar}\n" + " env_keys: []\n" + " timeout: 300\n"; + const char *block = " type: stdio\n" + " cmd: \"/opt/codebase-memory-mcp\"\n" + " args: []\n" + " enabled: true\n"; + yaml_fixture_t fixture; + ASSERT_EQ(yaml_fixture_init(&fixture, initial), 0); + + ASSERT_EQ(cbm_yaml_upsert_owned_mapping_entry(fixture.path, "extensions", "codebase-memory-mcp", + block), + CBM_YAML_IDENTITY_EDIT_ERROR); + char *unchanged = yaml_read_alloc(fixture.path); + ASSERT_NOT_NULL(unchanged); + ASSERT_STR_EQ(unchanged, initial); + + free(unchanged); + th_cleanup(fixture.dir); + PASS(); +} + +TEST(config_yaml_edit_goose_still_rejects_merge_key_with_empty_mapping_issue1673) { + const char *initial = "extensions:\n" + " demo:\n" + " type: stdio\n" + " cmd: /bin/true\n" + " <<: {}\n"; + const char *block = " type: stdio\n" + " cmd: \"/opt/codebase-memory-mcp\"\n" + " args: []\n" + " enabled: true\n"; + yaml_fixture_t fixture; + ASSERT_EQ(yaml_fixture_init(&fixture, initial), 0); + + ASSERT_EQ(cbm_yaml_upsert_owned_mapping_entry(fixture.path, "extensions", "codebase-memory-mcp", + block), + CBM_YAML_IDENTITY_EDIT_ERROR); + char *unchanged = yaml_read_alloc(fixture.path); + ASSERT_NOT_NULL(unchanged); + ASSERT_STR_EQ(unchanged, initial); + + free(unchanged); + th_cleanup(fixture.dir); + PASS(); +} + TEST(config_yaml_edit_owned_agent_mapping_installs_idempotently_and_removes_exact_state) { struct owned_mapping_case { const char *section; @@ -1592,6 +1693,9 @@ SUITE(config_yaml_edit) { RUN_TEST(config_yaml_edit_hermes_mapping_lifecycle); RUN_TEST(config_yaml_edit_hermes_creates_missing_section); RUN_TEST(config_yaml_edit_goose_extensions_preserve_siblings); + RUN_TEST(config_yaml_edit_goose_accepts_empty_flow_mapping_in_sibling_issue1673); + RUN_TEST(config_yaml_edit_goose_still_rejects_nonempty_flow_mapping_issue1673); + RUN_TEST(config_yaml_edit_goose_still_rejects_merge_key_with_empty_mapping_issue1673); RUN_TEST(config_yaml_edit_owned_agent_mapping_installs_idempotently_and_removes_exact_state); RUN_TEST(config_yaml_edit_owned_agent_mapping_preserves_foreign_same_name_state); RUN_TEST(config_yaml_edit_mapping_remove_first_middle_last);