fix(cli): reconcile normalized Codex hooks

[astandrik]

What does this PR do?

Fixes the Codex configuration corruption reported in #1432 with a narrow ownership-aware reconciler.

• Recognizes strict current and legacy owned SessionStart / SubagentStart inline assignments, including horizontal whitespace and dotted or quoted hook paths.
• Removes the complete owned inline assignment and writes the existing canonical marked array-of-tables block; exact markerless canonical blocks are normalized as well.
• Preserves foreign array-of-tables entries and rejects mixed, malformed, commented, multiline, duplicated, or otherwise ambiguous inline definitions without modifying the file.
• Adds read-only install and uninstall preflight so ambiguous TOML cannot leave partially installed Codex side files; independently owned uninstall artifacts are still cleaned up safely.

This deliberately does not add a general TOML parser, partial inline-array merging, a Kimi collision guard, new dependencies, or public CLI/MCP APIs. The final diff is 500 production additions and 728 additions overall.

Regression evidence

• Fail before: scripts/test.sh --suites cli reported 257 passed and 1 failed on the exact issue fixture. cli_codex_session_hook_issue330 failed at strstr(d, "SessionStart = [") is not NULL, proving install appended a conflicting array-of-tables hook instead of reconciling the normalized inline assignment.
• Pass after: scripts/test.sh --suites config_toml_edit,cli reports 296 passed under ASan/UBSan on current origin/main.
• Full local run: 7402 passed, 3 failed, and 4 skipped. The affected cli and config_toml_edit suites pass. The remaining failures are process-containment cases in untouched daemon_runtime, mcp, and index_supervisor paths; an isolated rerun reproduced the latter two and then hung in daemon_runtime with macOS children in UE state.

Verification

• scripts/test.sh --suites config_toml_edit,cli
• changed-file clang-format --dry-run --Werror
• changed-source -Wall -Wextra -Werror syntax check
• make -f Makefile.cbm lint-no-suppress
• git diff --check origin/main..HEAD
• scripts/check-dco.sh origin/main..HEAD

make -f Makefile.cbm lint-ci is locally blocked before source analysis because cppcheck is not installed; no dependency was installed for this PR. GitHub lint and smoke checks are the authoritative clean-environment runs.

Checklist

• Every commit is signed off (git commit -s) — required, CI rejects unsigned commits
• Tests pass — focused local suites pass and the full GitHub platform/sanitizer matrix is green; the three full-run local environment exceptions are documented above
• Lint passes — GitHub lint and lint-mem pass; local format, strict syntax, and lint-no-suppress gates pass, while local lint-ci cannot start because cppcheck is absent
• New behavior is covered by a test (reproduce-first for bug fixes)

Rollback

Revert the single focused commit:

git revert 0cf0ccb526cd5e396a8687460edb012ca1a7bcdf

Closes #1432

[Copilot]

🟡 Not ready to approve

There is a confirmed validation bug in the new TOML number parser (signed base integers) and an uninstall-path cleanup gap when hook preflight fails.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.

Pull request overview

This PR fixes Codex config.toml corruption (duplicate hooks.SessionStart definitions) by adding structural reconciliation of installer-owned SessionStart/SubagentStart hooks across TOML representations, and by introducing a read-only preflight path to fail closed before mutating Codex artifacts.

Changes:

• Add a new TOML reconciliation engine that can detect/repair owned Codex lifecycle hooks in both inline and array-of-tables forms while preserving foreign entries.
• Wire the CLI installer/uninstaller to run a Codex hook preflight and to migrate/clean up TOML-owned hooks when hooks.json is used.
• Add regression + matrix tests covering inline, mixed, malformed/ambiguous, and BOM/CRLF scenarios.

File summaries

File	Description
tests/test_config_toml_edit.c	Adds extensive reconciliation and fail-closed regression tests for Codex TOML hook editing.
tests/test_cli.c	Adds CLI-level regression tests for issue #1432, read-only preflight, and install/dry-run/idempotency lifecycle.
src/cli/config_toml_edit.h	Introduces the Codex hook reconciliation API and mode enum.
src/cli/config_toml_edit.c	Implements structural parsing + reconciliation for owned Codex hooks and strengthens collision guards.
src/cli/cli.c	Switches Codex hook install/removal to reconciliation API and adds installer/uninstaller preflight gating.

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 2
• Review effort level: Lite

We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Suppressed comments (1)

src/cli/config_toml_edit.c:4278

• edit->replacement can be NULL when replacement_len == 0 (e.g., deletion edits pushed with NULL). This relies on toml_buffer_append() being safe with a NULL pointer and zero length; that’s not guaranteed unless toml_buffer_append() explicitly handles len == 0 without dereferencing the pointer. To make this robust, skip the append when replacement_len == 0, or ensure toml_buffer_append() defensively returns OK for len == 0 regardless of data pointer value.

        if (edit->start < cursor || edit->end > len ||
            toml_buffer_append(output, data + cursor, edit->start - cursor) != TOML_EDIT_OK ||
            toml_buffer_append(output, edit->replacement, edit->replacement_len) != TOML_EDIT_OK) {
            return TOML_EDIT_ERR;
        }

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (3)

src/cli/config_toml_edit.c:2919

• The base-prefixed integer branch only triggers when pos == 0U, so signed hex/octal/binary forms like -0x10 / +0o77 are treated as invalid. TOML allows an optional sign on integers (including base-prefixed), so this can cause hook reconciliation to fail-closed on valid user configs that include signed base-prefixed numbers inside inline hook objects. Consider accepting the base prefix at pos (after the optional sign) and adjusting the digit start/end accordingly; update the invalid-fixture tests that currently assume signed base-prefixed values are malformed.

static int toml_codex_number_is_valid(const char *data, size_t len) {
    size_t pos = 0U;
    if (pos < len && (data[pos] == '+' || data[pos] == '-')) {
        pos++;
    }
    if (pos == len) {
        return 0;
    }
    if ((len - pos == 3U && memcmp(data + pos, "inf", 3U) == 0) ||
        (len - pos == 3U && memcmp(data + pos, "nan", 3U) == 0)) {
        return 1;
    }
    if (pos == 0U && len >= 3U && data[0] == '0' &&
        (data[1] == 'x' || data[1] == 'o' || data[1] == 'b')) {
        int base = data[1] == 'x' ? 16 : (data[1] == 'o' ? 8 : 2);
        return toml_codex_digits_are_valid(data, 2U, len, base, NULL, NULL);
    }

src/cli/config_toml_edit.c:2720

• Duplicate-key detection is O(n) per insertion, which becomes O(n²) for large inline tables/objects. Since TOML_CODEX_MAX_ITEMS is 65536, a crafted inline hook value with many fields could cause very slow parsing even though you ultimately fail-closed. Consider tightening the maximum expected field count for these Codex-specific objects (e.g., a small fixed upper bound), or switching duplicate detection to something sub-quadratic (e.g., sort once then scan, or a small hash set keyed by the rendered key-path).

static int toml_codex_field_vector_push(toml_codex_field_vector_t *vector, toml_key_path_t *key,
                                        size_t value_start, size_t value_end) {
    if (!vector || !key || value_start > value_end || vector->count >= TOML_CODEX_MAX_ITEMS) {
        return TOML_EDIT_ERR;
    }
    for (size_t i = 0U; i < vector->count; ++i) {
        if (toml_key_path_equal(&vector->items[i].key, key)) {
            return TOML_EDIT_ERR;
        }
    }

tests/test_cli.c:7947

• These new CLI tests hardcode /tmp/... for temp directories. This is not portable on Windows (and can be problematic in sandboxed CI), even though other parts of the test suite appear to use helper tempdir utilities. Prefer using an existing cross-platform tempdir helper (or deriving the base temp directory from the environment) and then applying the XXXXXX template under that directory.

    char tmpdir[256];
    snprintf(tmpdir, sizeof(tmpdir), "/tmp/cli-codex-inline-hook-XXXXXX");
    if (!cbm_mkdtemp(tmpdir))
        FAIL("cbm_mkdtemp failed");

[github-actions]

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

• It will not be closed for inactivity. No stale bot touches pull requests here.
• It may still sit a while before a human reads it. That is on us, not on you.
• Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

• Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
• Get CI green, or say which failures you believe are pre-existing.
• Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
• Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.