fix: uninstall removes the executable and indexes even when an agent config cleanup fails

[CaptainMittens]

Takes two of the three directions settled in #1954. The third — following a
config symlink owned by the invoking user — is not here; see "Not in this PR"
below.

1. Agent config cleanup no longer holds the executable and the indexes hostage

cli_uninstall_activate returned early as soon as any one agent configuration
failed to clean up, before index and executable removal had started. The
reporter's trigger was ~/.cursor/mcp.json symlinked into a dotfiles
repository: the JSON editors open with O_NOFOLLOW on purpose and refuse to
write through the link, and that one refusal left a 282 MB executable and a
104 MB index cache on disk after a run that had already printed a per-client
report. All four leftovers had to be removed by hand.

Cleaning up an agent's configuration and removing this tool's own files are
separate jobs, so the gate is gone. Nothing about the failure gets quieter:

• every failure is still named on stderr as it happens, by
  record_agent_config_error;
• the closing line now states the count, because a reader who watches stdout
  alone never sees the stderr lines;
• the exit code is still non-zero — return g_agent_uninstall_errors == 0 ? 0 : CLI_TRUE; was already there and is unchanged.

CBM_AGENT_EDIT_NOT_APPLICABLE also stopped counting as a failure in the agent
client registry loop. That value means the client has no MCP editor for its
config shape, so there was never an entry of ours to take out. Treating it as an
error was a second route to the same refusal over a file this tool had never
written to.

2. The report says what the run actually did

Every per-client line was written in the past tense whatever happened. So:

• --dry-run said removed for files it never opened;
• a step that had just recorded an error said removed on the next line;
• a preserved foreign MCP entry printed preserved and removed one after the
  other.

Reading either output alone, an operator would conclude a dozen configuration
files had been rewritten — which is exactly what the report describes.

Two small helpers carry the fix:

• uninstall_verb(dry_run) returns would remove or removed, so a dry run
  never claims a past action;
• uninstall_steps_succeeded(errors_before) ties a line that covers several
  steps at once (removed MCP config + hooks + instructions) to whether every
  one of those steps worked.

A step that failed now prints no line of its own; record_agent_config_error
has already named the agent, the operation and the path on stderr.

The shape copies the Qoder and Devin blocks, which already reported
planned / complete / failed correctly — the correct version was already in
the tree.

Tests

Both in tests/test_cli.c, against a temporary HOME with PATH redirected so
a real agent binary on the developer's machine cannot add an unrelated failure.
Neither runs a real uninstall against a real home.

• cli_uninstall_removes_binary_and_index_despite_agent_config_failure
  (POSIX only, needs symlink) builds the reporter's fixture — .cursor/mcp.json
  as a symlink into a dotfiles directory — and asserts the executable and the
  index are gone, the link's target is untouched, the exit code is still 1, the
  new failure count appears, and the old abort message does not.
• cli_uninstall_dry_run_report_uses_the_future_tense asserts a --dry-run
  prints Cursor: would remove MCP config entry, never
  Cursor: removed MCP config entry, and leaves the config file byte-identical.

Red before the change, green after.

Two existing tests changed on purpose

Both encoded the contract this PR sets out to change, so both had to be
inverted rather than kept:

• cli_agent_uninstall_reports_safe_editor_refusal asserted that a malformed
  OpenClaw config left the executable in place, and that the activation guard
  produced an "executable was kept" diagnostic. It now asserts the executable is
  gone and no partial-activation diagnostic is produced. The two claims that
  mattered are unchanged: the exit code is still non-zero, and the malformed
  file is still byte-identical afterwards — it is never written through.
• cli_codex_migrates_to_single_hook_representation checked
  stat(binary_path) == 0 after an uninstall whose Codex hook preflight failed,
  with the fixture content "installed binary must survive failed cleanup". Now
  stat(binary_path) != 0. Everything else it checks — the non-zero exit, the
  removed skill and agent files, the cleaned hooks, the emptied AGENTS.md — is
  untouched.

No test asserted any of the report wording, so the tense and outcome changes
broke nothing.

Checked, not changed

The install path does not have the same shape. install_generic_agent_config
prints the config path ( mcp: <path>) rather than a past-tense claim, and its
dry run goes through the install plan (g_install_plan). Nothing to fix there.

Not in this PR

• Following a config symlink owned by the invoking user. That is the
  O_NOFOLLOW decision in src/cli/config_json_like.c, and fix(cli): follow agent-root symlinks owned by the invoking user #2060 is already
  adding an ownership helper on the install side. Doing it here would collide.
• A config file that exists but holds no entry of ours still reports
  removed. The removers answer OK both when they deleted our entry and when
  there was nothing of ours to delete, so the caller cannot tell those apart
  without a fourth return value across the JSON and YAML removers. That is a
  wider change than this one and I would rather send it separately — say the
  word and I will.

Refs #1954

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

[DeusData]

Thank you for addressing the uninstall failure path and adding CLI coverage. The current cleanup-error guard can indeed stop uninstall before binary removal; this is also tracked in #1954 and #2106. We need more time to review this alongside #2110 so the overlapping fixes are handled coherently and your contribution is not lost.

The review queue is currently full, so detailed feedback may take a little time. We are working through it carefully and appreciate the work you have put into supporting the project. Thank you for your patience.