fix(config): self-heal an orphan managed marker on removal (#1558)

A duplicated install left a Codex config.toml carrying a CLOSING
`# <<< codebase-memory-mcp SessionStart <<<` with no opener. Every later
install then failed that client with op=legacy_hook_cleanup and aborted the
whole activation, so one bad file made the installer permanently unusable for
that client. Reported by @PsyTech-Rob.

We are the only writer of these markers, which makes the imbalance our own
residue. Refusing to touch a file we cannot parse is the right default in
general — it is the wrong default for a mess we made.

Removal now strips the stray line. The bounds come from the marker scan that
already ran, so nothing is guessed and no surrounding content is touched; the
user's own keys either side survive verbatim.

A WRITE still refuses on an imbalance, deliberately: with only one marker there
is no defensible region to replace, and guessing could destroy user content.
That asymmetry is the whole design — heal where the answer is unambiguous,
refuse where it is not.

Both orientations are tested (orphan opener, orphan closer) and revert-checked:
without the fix each returns the same -1 that aborted the reporter's install.

Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This commit is contained in:
Martin Vogel committed 2026-08-13 17:24:51 +02:00
1 parent a23db3c27e
commit a975f6b61f
2 files changed
+88 -2

No files matched your search

+41 -2
View File
@@ -34,6 +34,10 @@
#define TOML_EDIT_OK 0
#define TOML_EDIT_FOREIGN 1
#define TOML_EDIT_ERR (-1)
/* #1558: exactly one of the two managed markers is present. We are the sole
* writer of these markers, so an imbalance is our own residue — removal can
* strip it, a write still refuses. */
#define TOML_EDIT_ORPHAN_MARKER (-2)
#define TOML_EDIT_MAX_BYTES (16U * 1024U * 1024U)
#define TOML_EDIT_MAX_PATH_BYTES 32768U
@@ -855,6 +859,21 @@ static int toml_find_markers(const char *data, size_t len, const char *begin_mar
return TOML_EDIT_OK;
}
if (begin_count != 1 || end_count != 1 || begin_line->start >= end_line->start) {
/* #1558: an ORPHAN marker — one side present, the other missing — is
* reported separately from other malformations, because we are the only
* writer of these markers and therefore the only possible author of the
* imbalance. A duplicated install left a Codex config carrying a closing
* `# <<< ... <<<` with no opener, and every later install then failed
* that client outright.
*
* Refusing to touch a file we cannot parse is the right default in
* general. It is the wrong default for a mess we made: the caller can
* self-heal on REMOVAL, where deleting the stray line is unambiguous.
* A WRITE still refuses, because an imbalance leaves no defensible
* region to replace and guessing there could destroy user content. */
if ((begin_count == 1) != (end_count == 1) && begin_count + end_count == 1) {
return TOML_EDIT_ORPHAN_MARKER;
}
return TOML_EDIT_ERR;
}
*has_pair = 1;
@@ -1068,8 +1087,28 @@ int cbm_toml_remove_managed_block(const char *file_path, const char *begin_marke
toml_line_t begin_line = {0};
toml_line_t end_line = {0};
int has_pair = 0;
if (toml_find_markers(existing, existing_len, begin_marker, end_marker, &begin_line, &end_line,
&has_pair) != TOML_EDIT_OK) {
int find_rc = toml_find_markers(existing, existing_len, begin_marker, end_marker, &begin_line,
&end_line, &has_pair);
if (find_rc == TOML_EDIT_ORPHAN_MARKER) {
/* Self-heal: drop the stray marker line. Whichever side survived is the
* one toml_find_markers recorded, so the line bounds are known exactly
* — nothing is guessed, and no surrounding content is touched. */
const toml_line_t *stray = begin_line.full_end > 0U ? &begin_line : &end_line;
toml_buffer_t healed = {0};
if (toml_buffer_append(&healed, existing, stray->start) != TOML_EDIT_OK ||
toml_buffer_append(&healed, existing + stray->full_end,
existing_len - stray->full_end) != TOML_EDIT_OK) {
toml_buffer_dispose(&healed);
free(existing);
return TOML_EDIT_ERR;
}
int healed_rc = toml_write_atomic(file_path, existing, existing_len,
healed.data ? healed.data : "", healed.len, &snapshot);
toml_buffer_dispose(&healed);
free(existing);
return healed_rc;
}
if (find_rc != TOML_EDIT_OK) {
free(existing);
return TOML_EDIT_ERR;
}
+47
View File
@@ -1156,7 +1156,54 @@ TEST(config_toml_codex_preserves_bom_crlf_and_foreign_aot) {
PASS();
}
/* #1558: a duplicated install left a Codex config.toml carrying a CLOSING
* managed marker with no opener. Every later install then failed that client
* with op=legacy_hook_cleanup and aborted the whole activation.
*
* We are the only writer of these markers, so the imbalance was our own
* residue. Refusing to touch a file we cannot parse is the right default in
* general; it is the wrong default for a mess we made. Removal now strips the
* stray line — its bounds are known exactly, so nothing is guessed. */
TEST(config_toml_remove_self_heals_orphan_closing_marker_issue1558) {
char dir[CTE_PATH_CAP];
char path[CTE_PATH_CAP];
char actual[CTE_FILE_CAP];
ASSERT_EQ(cte_fixture(dir, sizeof(dir), path, sizeof(path)), 0);
ASSERT_EQ(th_write_file(path, "user_key = true\n"
"# <<< codebase-memory-mcp SessionStart <<<\n"
"other_key = 1\n"),
0);
ASSERT_EQ(cbm_toml_remove_managed_block(path, "# >>> codebase-memory-mcp SessionStart >>>",
"# <<< codebase-memory-mcp SessionStart <<<"),
0);
ASSERT_EQ(cte_read(path, actual, sizeof(actual)), 0);
/* The stray marker is gone; the user's own content is untouched. */
ASSERT_STR_EQ(actual, "user_key = true\nother_key = 1\n");
th_cleanup(dir);
PASS();
}
/* The mirror case: an opening marker with no closer heals the same way. */
TEST(config_toml_remove_self_heals_orphan_opening_marker_issue1558) {
char dir[CTE_PATH_CAP];
char path[CTE_PATH_CAP];
char actual[CTE_FILE_CAP];
ASSERT_EQ(cte_fixture(dir, sizeof(dir), path, sizeof(path)), 0);
ASSERT_EQ(th_write_file(path, "# >>> codebase-memory-mcp SessionStart >>>\n"
"keep = true\n"),
0);
ASSERT_EQ(cbm_toml_remove_managed_block(path, "# >>> codebase-memory-mcp SessionStart >>>",
"# <<< codebase-memory-mcp SessionStart <<<"),
0);
ASSERT_EQ(cte_read(path, actual, sizeof(actual)), 0);
ASSERT_STR_EQ(actual, "keep = true\n");
th_cleanup(dir);
PASS();
}
SUITE(config_toml_edit) {
RUN_TEST(config_toml_remove_self_heals_orphan_closing_marker_issue1558);
RUN_TEST(config_toml_remove_self_heals_orphan_opening_marker_issue1558);
RUN_TEST(config_toml_rejects_stale_content_and_identity);
RUN_TEST(config_toml_missing_target_race_does_not_replace_winner);
RUN_TEST(config_toml_existing_target_swap_after_check_preserves_winner);