Skip to content

feat(uninstall): remove config root on --purge#246

Merged
BlackHole1 merged 2 commits into
mainfrom
improve-uninstall
May 29, 2026
Merged

feat(uninstall): remove config root on --purge#246
BlackHole1 merged 2 commits into
mainfrom
improve-uninstall

Conversation

@BlackHole1
Copy link
Copy Markdown
Member

--purge previously deleted only explicitly targeted child items (auth.toml, settings.toml, logs, registry skills), leaving the app-support/config root behind with residual files and an empty skills/ parent. Append the config root as the last user-data target so it sweeps anything the explicit items did not cover. On Windows it is deferred for the same reason as the data directory, since the running process may still hold open SQLite handles.

Removing the config root also deletes the telemetry directory, so suppress this invocation's telemetry when purging; otherwise the teardown flush would re-create the directory we just removed.

`--purge` previously deleted only explicitly targeted child items
(auth.toml, settings.toml, logs, registry skills), leaving the
app-support/config root behind with residual files and an empty
`skills/` parent. Append the config root as the last user-data
target so it sweeps anything the explicit items did not cover. On
Windows it is deferred for the same reason as the data directory,
since the running process may still hold open SQLite handles.

Removing the config root also deletes the telemetry directory, so
suppress this invocation's telemetry when purging; otherwise the
teardown flush would re-create the directory we just removed.

Signed-off-by: Kevin Cui <bh@bugs.cc>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 29, 2026

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d25df72f-9a3d-45c5-826d-b206fba3d832

📥 Commits

Reviewing files that changed from the base of the PR and between c5772cd and 2de67e6.

📒 Files selected for processing (1)
  • src/application/commands/uninstall.cli.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/application/commands/uninstall.cli.test.ts

Summary by CodeRabbit

  • Bug Fixes

    • --purge now removes the entire configuration directory, including leftover unmanaged files.
    • Telemetry is suppressed during purge to avoid recreating system directories during uninstall teardown.
    • Platform-specific uninstall handling refined: config removal is immediate on non-Windows and deferred on Windows to avoid disrupting running processes.
  • Tests

    • Tests updated to validate purge behavior and platform-specific removal ordering.

Walkthrough

This PR extends the uninstall --purge command to remove the entire config root directory, not just managed subdirectories. The core change adds the store root to the self-update uninstall plan with platform-specific handling (deferred removal on Windows, immediate on others). Telemetry is suppressed during purge to prevent teardown from recreating the telemetry directory that purge removed. Unit tests verify platform-specific config root path resolution and ordering within the removal plan. CLI tests confirm residual unmanaged files within the config root are deleted along with the directory itself.

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant UninstallCommand
  participant Telemetry
  participant addUserDataItems
  participant FileSystem
  participant WindowsScheduler
  User->>UninstallCommand: run `uninstall --purge`
  UninstallCommand->>Telemetry: suppressCurrentInvocation() (if purge && !dryRun)
  UninstallCommand->>addUserDataItems: build uninstall plan (user-data items + config root)
  addUserDataItems->>FileSystem: remove user-data items immediately (non-Windows)
  addUserDataItems->>WindowsScheduler: schedule config-root removal (Windows deferred)
  WindowsScheduler->>FileSystem: perform deferred removal (later)
Loading

Possibly related PRs

  • oomol-lab/oo-cli#245: This PR directly extends the uninstall implementation added in #245 by configuring the self-update uninstall plan to additionally remove the config-store root directory during --purge operations.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The pull request title follows the required format 'feat(uninstall): remove config root on --purge' and accurately describes the main change.
Description check ✅ Passed The pull request description is directly related to the changeset, clearly explaining the rationale for removing the config root during purge and handling of telemetry suppression.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch improve-uninstall

Comment @coderabbitai help to get the list of available commands and usage tips.

The `--yes --purge` CLI test asserted the config root (and a residual
leftover.txt within it) was gone right after the in-process run. On
Windows the config root removal is deferred to the post-exit helper,
since the running process may still hold open SQLite handles under it, so
the root is still present when the command returns. Branch the test on
platform: on Windows expect a scheduled cleanup with the root retained,
on Unix expect the in-process removal as before.
@BlackHole1 BlackHole1 merged commit 8d78cb4 into main May 29, 2026
6 checks passed
@BlackHole1 BlackHole1 deleted the improve-uninstall branch May 29, 2026 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant