[PR #1177] [MERGED] fix(setup): stop uninstall before removals when a client config is unsafe #2700

Closed
opened 2026-10-17 21:09:14 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/1177
Author: @kaoru0822-kitauji
Created: 10/9/2026
Status: ✅ Merged
Merged: 10/9/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: fix/uninstall-preflight-client-config


📝 Commits (2)

  • 5cc841d fix(setup): stop uninstall before removals when a client config is unsafe
  • fd975c7 fix(setup): stop remaining uninstall removals after a client fails

📊 Changes

4 files changed (+221 additions, -29 deletions)

View changed files

📝 docs/installation.md (+5 -2)
📝 src/application/Uninstall.ts (+109 -27)
📝 src/cliLogging.test.ts (+1 -0)
📝 tests/boundary/filesystem/clientLifecycle.test.ts (+106 -0)

📄 Description

Summary

docs/installation.md says malformed client configuration stops rea uninstall. Instead, uninstall reported that client as failed and kept going: it removed other clients' REA registrations, the bundled skill, and (with --purge-data) REA's cache and state, leaving a partially uninstalled environment.

Fixes #1164

Change

  • src/application/Uninstall.ts: runUninstall first asks the host to inspect every client configuration (UninstallHost.inspectClient). If any configuration is at an unsafe path, unreadable, or malformed, uninstall returns failed with those items plus one uninstall: skipped item ("No REA registration, skill, or data was removed. Repair each failed configuration, then rerun uninstall."), and nothing is removed. These are the conditions rea setup already treats as blocking in inspectClientConfiguration, so both commands now refuse the same inputs before mutating anything.
  • The read/parse step is shared between the inspection and removeClient, so both use the same diagnostics. A missing or unsupported configuration is still skipped and does not block. Failures that happen while writing (backup, update, restore) are still reported per client, as before.
  • docs/installation.md: the stop applies to client configuration. A purge path that is a symbolic link is retained and reported rather than followed; tests/boundary/filesystem/clientLifecycle.test.ts already asserts that behavior (uninstalls only owned entries and refuses purge symlinks), so the guide now describes it instead of calling it a stop.

Issue reproduction (built branch, disposable HOME)

{"status":"failed","items":[
 {"name":"codex","status":"failed","detail":"Configuration is not valid TOML and was not changed. Repair it, then rerun uninstall."},
 {"name":"uninstall","status":"skipped","detail":"No REA registration, skill, or data was removed. Repair each failed configuration, then rerun uninstall."},
 {"name":"analysis_engine","status":"retained","detail":"Hopper is not owned by REA uninstall."}]}

Exit 1. Cursor's rea entry, the skill directory, and ~/.rea/cache and ~/.rea/state remain. On main, the same run removed all four.

Tests

tests/boundary/filesystem/clientLifecycle.test.ts — stops before any removal when one client configuration is malformed: malformed Codex TOML plus an owned Cursor registration, the skill, cache, and state. Uninstall fails without touching any of them; after repairing the Codex file, a rerun completes and removes them. The test fails on main. src/cliLogging.test.ts fake host gains inspectClient.

Verification

  • npm run check:fast, npm run jscpd (2.49%), npm run docs:check
  • npm run test:focused -- tests/boundary/filesystem/{clientLifecycle,clientConfigurationBom,clientConfigurationDialects,windowsNpxRegistration}.test.ts src/cliLogging.test.ts tests/boundary/cli/cliSetup.test.ts (109 passed, 1 skipped)
  • Built CLI run of the issue's reproduction (output above)

No Hopper/Ghidra verification was needed: the change is limited to uninstall's client-configuration handling.

🤖 Generated with Claude Code


🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/1177 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/9/2026 **Status:** ✅ Merged **Merged:** 10/9/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/uninstall-preflight-client-config` --- ### 📝 Commits (2) - [`5cc841d`](https://github.com/morluto/rea/commit/5cc841d5c154f692bdc257e106cc4b212dc19dc0) fix(setup): stop uninstall before removals when a client config is unsafe - [`fd975c7`](https://github.com/morluto/rea/commit/fd975c75fa353840e6e887232ca68496cf8705ea) fix(setup): stop remaining uninstall removals after a client fails ### 📊 Changes **4 files changed** (+221 additions, -29 deletions) <details> <summary>View changed files</summary> 📝 `docs/installation.md` (+5 -2) 📝 `src/application/Uninstall.ts` (+109 -27) 📝 `src/cliLogging.test.ts` (+1 -0) 📝 `tests/boundary/filesystem/clientLifecycle.test.ts` (+106 -0) </details> ### 📄 Description ## Summary `docs/installation.md` says malformed client configuration stops `rea uninstall`. Instead, uninstall reported that client as `failed` and kept going: it removed other clients' REA registrations, the bundled skill, and (with `--purge-data`) REA's cache and state, leaving a partially uninstalled environment. Fixes #1164 ## Change - `src/application/Uninstall.ts`: `runUninstall` first asks the host to inspect every client configuration (`UninstallHost.inspectClient`). If any configuration is at an unsafe path, unreadable, or malformed, uninstall returns `failed` with those items plus one `uninstall: skipped` item ("No REA registration, skill, or data was removed. Repair each failed configuration, then rerun uninstall."), and nothing is removed. These are the conditions `rea setup` already treats as blocking in `inspectClientConfiguration`, so both commands now refuse the same inputs before mutating anything. - The read/parse step is shared between the inspection and `removeClient`, so both use the same diagnostics. A missing or unsupported configuration is still `skipped` and does not block. Failures that happen while writing (backup, update, restore) are still reported per client, as before. - `docs/installation.md`: the stop applies to client configuration. A purge path that is a symbolic link is retained and reported rather than followed; `tests/boundary/filesystem/clientLifecycle.test.ts` already asserts that behavior (`uninstalls only owned entries and refuses purge symlinks`), so the guide now describes it instead of calling it a stop. ## Issue reproduction (built branch, disposable `HOME`) ```json {"status":"failed","items":[ {"name":"codex","status":"failed","detail":"Configuration is not valid TOML and was not changed. Repair it, then rerun uninstall."}, {"name":"uninstall","status":"skipped","detail":"No REA registration, skill, or data was removed. Repair each failed configuration, then rerun uninstall."}, {"name":"analysis_engine","status":"retained","detail":"Hopper is not owned by REA uninstall."}]} ``` Exit 1. Cursor's `rea` entry, the skill directory, and `~/.rea/cache` and `~/.rea/state` remain. On `main`, the same run removed all four. ## Tests `tests/boundary/filesystem/clientLifecycle.test.ts` — `stops before any removal when one client configuration is malformed`: malformed Codex TOML plus an owned Cursor registration, the skill, cache, and state. Uninstall fails without touching any of them; after repairing the Codex file, a rerun completes and removes them. The test fails on `main`. `src/cliLogging.test.ts` fake host gains `inspectClient`. ## Verification - `npm run check:fast`, `npm run jscpd` (2.49%), `npm run docs:check` - `npm run test:focused -- tests/boundary/filesystem/{clientLifecycle,clientConfigurationBom,clientConfigurationDialects,windowsNpxRegistration}.test.ts src/cliLogging.test.ts tests/boundary/cli/cliSetup.test.ts` (109 passed, 1 skipped) - Built CLI run of the issue's reproduction (output above) No Hopper/Ghidra verification was needed: the change is limited to uninstall's client-configuration handling. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
bsakaguchi 2026-10-17 21:09:14 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
morluto/rea#2700
No description provided.