[PR #2197] [MERGED] fix(cli): label each uninstall skill root by its client #5076

New issue

Closed

opened 2026-10-31 01:03:09 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:03:09 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2197
Author: @wjl8636
Created: 10/12/2026
Status: ✅ Merged
Merged: 10/13/2026
Merged by: @morluto

Base: main ← Head: fix/uninstall-skill-labels

📝 Commits (4)

ff52b3e fix(cli): label each uninstall skill root by its client
5562991 chore: merge main and preserve client and path uninstall labels
9fb0842 test(cli): align skill summary assertions with client and path labels
d78a094 chore: reconcile captured main with skill uninstall labels

📊 Changes

3 files changed (+55 additions, -14 deletions)

View changed files

📝 src/application/Uninstall.ts (+9 -4)
📝 tests/boundary/cli/clientProfileLifecycle.test.ts (+5 -2)
📝 tests/boundary/filesystem/clientLifecycle.test.ts (+41 -8)

📄 Description

Fixes #2111

Summary

rea uninstall inspected every managed skill destination but labeled each
result only as skill (Claude Code was the sole exception, shown as
Claude Code skill). When several directories were retained or failed, the
summary could not say which directory needed attention.

Problem and expected behavior

Trigger: two different skill roots are retained by uninstall (for example a
non-REA-owned manifest on the shared root and a symbolic link under another
client's directory). Before this change the aggregated skill item's detail
showed several skill: retained (...) entries with no way to match a result
to its directory. Expected: each retained/failed/skipped root is identified
by its client.

Change and scope

removeManagedSkills passed a generic "skill" name into removeManagedSkill
for every destination. It now passes the destination's client identifier
(shared, claude_code, antigravity, hermes, ...) exposed by
skillDestinations, so each root is named independently in the aggregated
detail. No contract change: the top-level item stays name: "skill"; only the
detail now names each directory.

Contract and boundary impact

Semantic owner and earliest changed stage: application uninstall output; no
structural contract change.
CLI and MCP/tool-catalog contract: none (uninstall is CLI-only; item name
and status enumeration unchanged).
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: none.
Process execution, authorization, cleanup, or containment impact: none.
Generated metadata / package / installation impact: none.

Evidence and regression coverage

Tests added or updated: updated the "reports a managed-path removal failure"
assertion (detail now begins shared: failed () and added a regression test
that places a non-REA-owned manifest on the shared root and an owned skill
on a Claude Code root, expecting shared: retained ( and
claude_code: removed ( in the aggregated skill item's detail.
Base reproduction or other evidence: the new test fails on main; the
aggregated detail previously collapsed both directories under the generic
skill label.
User-visible CLI/MCP output: representative output below.
Remaining proof gaps: none for the changed behavior.

Representative output (isolated HOME):

$ rea uninstall
  skill,retained,"hermes: skipped (Skill directory does not exist.) shared: retained (Skill manifest is not REA-owned.) claude_code: skipped (Skill directory does not exist.) antigravity: retained (Skill path is not a directory.)"

Validation performed

npm run check:fast — 2 tasks (typecheck, lint) successful.
npm run test:focused -- tests/boundary/filesystem/clientLifecycle.test.ts —
44 passed; 5 failed, all with EPERM: symlink when the test creates a
symbolic link under this Windows host (no developer mode). These 5 fail on
the base revision here for the same reason and are unrelated to this change;
the skill-root tests affected by this change all pass.
Local CLI check with an isolated USERPROFILE confirmed the aggregated
detail now names each root.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: none required; uninstall is
host-independent, and the symlink-dependent tests require a POSIX-style
creation privilege this Windows host does not grant.
Package or release metadata impact: none.
Security, privacy, process, or containment review: no new retained-process
or filesystem authority involved.

Review checklist

This PR addresses a concrete problem or an agreed enhancement.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

_{🔄 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/2197 **Author:** [@wjl8636](https://github.com/wjl8636) **Created:** 10/12/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/uninstall-skill-labels` --- ### 📝 Commits (4) - [`ff52b3e`](https://github.com/morluto/rea/commit/ff52b3e65a63182f6de2168214a3da0758875900) fix(cli): label each uninstall skill root by its client - [`5562991`](https://github.com/morluto/rea/commit/55629912f2ed8a94cbbf4b6175f7f273253922a8) chore: merge main and preserve client and path uninstall labels - [`9fb0842`](https://github.com/morluto/rea/commit/9fb0842020624ed4d9e4c155b9c335232cc05637) test(cli): align skill summary assertions with client and path labels - [`d78a094`](https://github.com/morluto/rea/commit/d78a094fd53bf59045d83fa9d2d33b84ecaf4218) chore: reconcile captured main with skill uninstall labels ### 📊 Changes **3 files changed** (+55 additions, -14 deletions) <details> <summary>View changed files</summary> 📝 `src/application/Uninstall.ts` (+9 -4) 📝 `tests/boundary/cli/clientProfileLifecycle.test.ts` (+5 -2) 📝 `tests/boundary/filesystem/clientLifecycle.test.ts` (+41 -8) </details> ### 📄 Description Fixes #2111 ## Summary `rea uninstall` inspected every managed skill destination but labeled each result only as `skill` (Claude Code was the sole exception, shown as `Claude Code skill`). When several directories were retained or failed, the summary could not say which directory needed attention. ## Problem and expected behavior Trigger: two different skill roots are retained by uninstall (for example a non-REA-owned manifest on the shared root and a symbolic link under another client's directory). Before this change the aggregated skill item's detail showed several `skill: retained (...)` entries with no way to match a result to its directory. Expected: each retained/failed/skipped root is identified by its client. ## Change and scope `removeManagedSkills` passed a generic `"skill"` name into `removeManagedSkill` for every destination. It now passes the destination's client identifier (`shared`, `claude_code`, `antigravity`, `hermes`, ...) exposed by `skillDestinations`, so each root is named independently in the aggregated detail. No contract change: the top-level item stays `name: "skill"`; only the detail now names each directory. ## Contract and boundary impact - Semantic owner and earliest changed stage: application uninstall output; no structural contract change. - CLI and MCP/tool-catalog contract: none (uninstall is CLI-only; item name and status enumeration unchanged). - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: none. - Process execution, authorization, cleanup, or containment impact: none. - Generated metadata / package / installation impact: none. ## Evidence and regression coverage - Tests added or updated: updated the "reports a managed-path removal failure" assertion (detail now begins `shared: failed (`) and added a regression test that places a non-REA-owned manifest on the shared root and an owned skill on a Claude Code root, expecting `shared: retained (` and `claude_code: removed (` in the aggregated skill item's detail. - Base reproduction or other evidence: the new test fails on `main`; the aggregated detail previously collapsed both directories under the generic `skill` label. - User-visible CLI/MCP output: representative output below. - Remaining proof gaps: none for the changed behavior. Representative output (isolated HOME): ``` $ rea uninstall skill,retained,"hermes: skipped (Skill directory does not exist.) shared: retained (Skill manifest is not REA-owned.) claude_code: skipped (Skill directory does not exist.) antigravity: retained (Skill path is not a directory.)" ``` ## Validation performed - `npm run check:fast` — 2 tasks (typecheck, lint) successful. - `npm run test:focused -- tests/boundary/filesystem/clientLifecycle.test.ts` — 44 passed; 5 failed, all with `EPERM: symlink` when the test creates a symbolic link under this Windows host (no developer mode). These 5 fail on the base revision here for the same reason and are unrelated to this change; the skill-root tests affected by this change all pass. - Local CLI check with an isolated `USERPROFILE` confirmed the aggregated detail now names each root. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: none required; uninstall is host-independent, and the symlink-dependent tests require a POSIX-style creation privilege this Windows host does not grant. - Package or release metadata impact: none. - Security, privacy, process, or containment review: no new retained-process or filesystem authority involved. ## Review checklist - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:03:09 +00:00