fix(cli): explain Codex hook preflight refusals

[astandrik]

What does this PR do?

Restores the Codex CLI: heading before hook preflight errors and carries stable failure reasons out of the TOML reconciler. Ambiguous configs still fail closed and remain byte-identical. Diagnostics expose only a category such as reason=ambiguous_hook_ownership, never config content.

The missing heading starts with 0cf0ccb5, which moved preflight ahead of install_generic_agent_config(). I reproduced the refusal with an inline hooks.SessionStart value that mixes a CBM-owned command with a foreign command. The exact reporter config from #1560 is not public, so this is the closest reproducible ownership case, not a claim that the input is identical.

This is the editor-reason follow-up described in #1633. #1629 remains complementary because it adds target metadata to generic config errors.

Verification

• Focused ASan/UBSan suites: one clean 311 passed run before the latest main update, which only changed scripts/security-fuzz.sh
• Current-base repeats: both new regression tests pass; the full suite ended at 310 passed, 1 failed after an unrelated activation cleanup refusal in cli_uninstall_removes_every_claude_hook_shim
• clang-format --dry-run --Werror passes for all changed files
• make -f Makefile.cbm lint-no-suppress passes
• git diff --check passes
• scripts/check-dco.sh origin/main..HEAD passes

Checklist

• Every commit is signed off (git commit -s) — required, CI rejects
  unsigned commits (DCO, see CONTRIBUTING.md)
• Tests pass locally (make -f Makefile.cbm test)
• Lint passes (make -f Makefile.cbm lint-ci); cppcheck is not installed locally
• New behavior is covered by a test (reproduce-first for bug fixes)

Fixes #1633

[DeusData]

Reviewed, and this is the right fix — it is the one I described in #1633 and #1631 as the real solution rather than the stopgap.

I verified the property I most cared about before saying so: every reason is a fixed category — ambiguous_hook_ownership, malformed_config, unsafe_content, conflicting_hook_representations — and the only new output is reason=%s carrying one of those. No config content reaches the diagnostic. That matters more than usual here: these files routinely hold API keys, and a well-meaning "here is the line we choked on" would have turned a diagnostic improvement into a secret-disclosure bug. You got that right without being asked.

Two other things worth naming. You traced the vanished Codex CLI: heading to your own 0cf0ccb5 moving preflight ahead of install_generic_agent_config() — self-diagnosing a regression in your own merged PR is not a small thing. And you were explicit that the reproduction is "the closest reproducible ownership case, not a claim that the input is identical" to the reporter's. That distinction is exactly right and most people skip it.

You are also correct that #1629 is complementary rather than competing: yours carries the editor's reason out, mine reports what the target file observably is. Together they answer "which check refused" and "what was it looking at". They do both touch record_agent_config_error, so whichever lands second needs a small rebase — yours is the more valuable half, so I will rebase mine onto yours rather than the other way round.

Merging once CI clears. Thank you — the original hook-reconciliation fix solved a genuinely nasty bug (a duplicated install breaking Codex permanently), and following it through to the diagnostics is the part most contributors would have left to us.

[Copilot]

Pull request overview

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Restores the Codex CLI: heading for Codex hook preflight refusals and threads stable, content-free failure reasons from the TOML reconciler up to CLI diagnostics.

Changes:

• Add a detailed Codex hook reconcile API that returns a structured failure category and a stable string name.
• Update CLI preflight error reporting to include the restored heading and an optional reason=... field.
• Add regression tests for stable TOML refusal reasons and CLI preflight heading/reason output.

Reviewed changes

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

Show a summary per file

File	Description
tests/test_config_toml_edit.c	Adds tests asserting stable failure reason enums/names and byte-identical “fail closed” behavior.
tests/test_cli.c	Adds a regression test ensuring Codex CLI preflight prints the heading and reason=... while leaving files untouched.
src/cli/config_toml_edit.h	Introduces a public failure enum and a new detailed reconcile API + failure-name helper.
src/cli/config_toml_edit.c	Implements failure propagation across TOML scanning/reconciliation and provides stable failure-name strings.
src/cli/cli.c	Threads detailed failure reasons into CLI diagnostics and restores the Codex CLI: heading before refusal output.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[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.

[Copilot]

Pull request overview

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

Suppressed comments (5)

src/cli/cli.c:8475

• The Codex CLI: heading is printed to stdout while the actual error (record_agent_config_error_with_reason) is printed to stderr. When the streams are not merged (common in real usage and logging), the heading can be missing or appear out of order relative to the error. To make the diagnostic deterministic, print the heading to stderr (or have record_agent_config_error_with_reason optionally emit the heading) so both lines are on the same stream.

            if (!g_install_plan) {
                printf("Codex CLI:\n");
                fflush(stdout);
            }

src/cli/cli.c:8482

• The Codex CLI: heading is printed to stdout while the actual error (record_agent_config_error_with_reason) is printed to stderr. When the streams are not merged (common in real usage and logging), the heading can be missing or appear out of order relative to the error. To make the diagnostic deterministic, print the heading to stderr (or have record_agent_config_error_with_reason optionally emit the heading) so both lines are on the same stream.

            record_agent_config_error_with_reason(
                false, "Codex CLI", commands_ok ? "hook_preflight" : "hook_command_build", cp,
                reason);

src/cli/cli.c:10742

• The Codex CLI: heading is printed to stdout while the actual error (record_agent_config_error_with_reason) is printed to stderr. When the streams are not merged (common in real usage and logging), the heading can be missing or appear out of order relative to the error. To make the diagnostic deterministic, print the heading to stderr (or have record_agent_config_error_with_reason optionally emit the heading) so both lines are on the same stream.

            printf("Codex CLI:\n");
            fflush(stdout);

src/cli/cli.c:10746

• The Codex CLI: heading is printed to stdout while the actual error (record_agent_config_error_with_reason) is printed to stderr. When the streams are not merged (common in real usage and logging), the heading can be missing or appear out of order relative to the error. To make the diagnostic deterministic, print the heading to stderr (or have record_agent_config_error_with_reason optionally emit the heading) so both lines are on the same stream.

            record_agent_config_error_with_reason(true, "Codex CLI", "hook_preflight", cp, reason);

src/cli/cli.c:3474

• cbm_reconcile_codex_hooks_command_detailed accepts a failure out-param but returns early on invalid arguments without setting it. This can produce a misleading reason (or no reason) even though the failure is an argument error. Set *failure to CBM_TOML_CODEX_HOOK_FAILURE_INVALID_ARGUMENT (or at least reset to NONE) before returning CLI_ERR when the pointer is provided.

    if (!config_path || !command || !command_windows) {
        return CLI_ERR;
    }