Harden asynchronous removal lifecycles

AGENTS.md

@@ -132,10 +132,26 @@ the terminal, with both a full-screen TUI and scriptable CLI commands.
   mount point, or broken link) as a leaf, revalidate containment immediately
   before deletion, and keep cancellation/depth bounds explicit. Keep traversal
   lazy with per-depth enumerator frames so cancellation runs between entries
-  and retained state stays O(depth). Do not claim the path checks are atomic
-  against a hostile same-user process: supported .NET 10 deletion APIs remain
-  path-based on Windows and Unix; native handle-relative deletion is separate
-  follow-up work.
+  and retained state stays O(depth). TUI removal must use `RemoveAsync` /
+  `RemoveManyAsync`, whose progress is throttled to 10 updates/second; keep the
+  modal alive until cancellation quiesces and rescan when cancellation made
+  any partial filesystem change. Runtime reports retain at most 128 individual
+  errors plus an omission summary while preserving the exact error count. Do
+  not overwrite a mid-target cancellation snapshot with completed-target-only
+  totals: the batch progress adapter owns the latest aggregate state and keeps
+  processed-target and successfully-deleted-target counts distinct. Retryable
+  remove dialogs accumulate filesystem mutation totals across attempts and
+  compact-to-wizard escalation so later failures cannot hide a required rescan.
+  Every async removal entry point must publish a terminal canceled update even
+  when its token was already canceled; synthetic cancellation reports retain
+  the exact observed runtime-error count and mark cancellation explicitly.
+  All non-canceled return paths, including validation refusals, must publish a
+  terminal completed progress update with exact processed/deleted/error counts.
+  CLI JSON must not duplicate validation refusals or unaccepted warnings into
+  `runtimeErrors`. Do not claim the path checks are atomic
+  against a hostile same-user process:
+  supported .NET 10 deletion APIs remain path-based on Windows and Unix;
+  native handle-relative deletion is separate follow-up work.
 - Use `Logger.SubscribeWithReplay` whenever a consumer needs retained history
   plus live entries. Do not recreate snapshot-then-subscribe logic. Preserve
   the logger's message/total-character budgets and the file sink's date+size

agent_docs/ui-lifecycle-and-resource-bounds.md

@@ -60,6 +60,26 @@ logging, or subprocess adapters.
   lazy-enumerator `MoveNext` calls inside the I/O exception boundary because
   disconnects, ACL changes, and disappearing directories fail there rather
   than when the enumerable is created.
+- Removal uses the same thread-pool boundary: compact remove, the advanced
+  wizard, cleanup batches, and agent-link unlinking call the asynchronous
+  `RemoveService` APIs. Progress callbacks are throttled to 10 per second and
+  immediately handed to `IApplication.Invoke`; they must not use `Progress<T>`
+  and add another implicit queue. Esc cancels the active traversal, and each
+  removal window cancels and drains its owned task before disposing controls.
+  Partial cancellation counts still trigger inventory invalidation/rescan.
+  `BatchProgressAdapter` retains the latest aggregate snapshot so the outer
+  cancellation boundary cannot replace mid-target file/directory counts with
+  completed-target-only totals. Progress tracks processed targets separately
+  from successfully deleted targets; cancellation summaries must never treat a
+  failed-but-processed target as removed. Each async removal entry point emits
+  a final `IsCanceled` update for already-canceled tokens as well as in-flight
+  work. Retryable remove dialogs retain cumulative file/directory mutation
+  totals across attempts and across compact-to-wizard escalation, while the
+  displayed progress remains per-attempt. Synthetic cancellation reports mark
+  cancellation explicitly and retain the exact runtime-error count even when
+  the individual logged details are unavailable.
+  Traversal retains O(depth) enumerator state and no more than 128 detailed
+  runtime errors plus one omission summary.
 - Logger subscriptions are disposable. Every long-lived subscriber must retain
   and dispose its subscription. Disposal deactivates registrations that were
   already snapshotted and waits for an in-flight callback, so no callback can
@@ -83,5 +103,6 @@ logging, or subprocess adapters.
 Run `dotnet build`, then `dotnet test --no-build`. Resource-focused coverage
 includes large subprocess output, LRU eviction, subscription disposal,
 callback-safe cancellation gates, bounded inventory files, iteration failures,
-overlapping preview cancellation, superseded inventory scans, inline busy
-state, and layout guards at 80×24, 100×30, and 140×42.
+overlapping preview cancellation, superseded inventory scans, asynchronous
+removal cancellation/progress/error bounds, inline busy state, and layout
+guards at 80×24, 100×30, and 140×42.

docs/reviews/adversarial-concurrency-resource-cancellation-audit-2026-08-28.md

@@ -7,10 +7,13 @@ PR follow-up reviewed: `df62a22` (`fix: close remaining lifecycle review gaps`),
 merged by PR #11 as `40bbf0b`
 Hardening branch: `fix/adversarial-hardening`
 Second hardening branch: `fix/resource-lifecycle-hardening-2`
-Status: Active remediation. PR #12 is merged. Its final disk-budget follow-up,
-the analogous cancellation-callback lock hazard, and bounded/cancellable local
-inventory I/O are implemented on the second hardening branch. Removal UI work,
-hostile same-user path replacement, and findings 7-11 and 14 remain scheduled.
+Third hardening branch: `fix/removal-lifecycle-hardening`
+Status: Active remediation. PRs #12 and #13 are merged. Aggregate log retention,
+callback-safe cancellation ownership, bounded/cancellable inventory I/O, and
+portable asynchronous removal are implemented. Native handle-relative deletion
+for hostile same-user mutation remains a separate security design; metadata
+scheduling, install-modal ownership, root CLI cancellation, path identity, Esc/
+LRU coverage, and the lower-risk remainder stay scheduled.
 
 ## Executive summary
 
@@ -788,6 +791,12 @@ rejected rather than canceling and superseding the stale request.
 
 Severity: **Medium**
 
+Implementation status: **Install flows remain open. The removal modal/wizard
+now demonstrate the target lifecycle on `fix/removal-lifecycle-hardening`:
+stable token capture, an explicitly retained operation task, inner queued-
+callback lifetime checks, cancellation, and task drain before control
+disposal.**
+
 Locations:
 
 - `src/SkillView.Core/Ui/InstallConfirmModal.cs`, async accepting handler around lines 189-264.
@@ -881,6 +890,10 @@ is the notable exception.
 
 Severity: **Medium**
 
+Implementation status: **Completed on `fix/removal-lifecycle-hardening`. The
+portable path remains non-atomic against hostile same-user path replacement,
+as documented under Finding 1.**
+
 Locations:
 
 - `src/SkillView.Core/Inventory/RemoveService.cs`, `.ToList()` traversal around lines 106-121.
@@ -890,9 +903,35 @@ Locations:
 
 ### Current behavior
 
-`RemoveService` materializes every file path and every directory path before
-deleting anything. The UI calls it synchronously from Terminal.Gui event
-handlers. There is no cancellation token or progress reporting.
+The original implementation materialized every file path and directory path
+before deletion and ran synchronously from Terminal.Gui event handlers. The
+earlier branch replaced that traversal with O(depth) enumerator frames. This
+branch completes the remediation: compact remove, advanced remove, cleanup
+batches, cleanup validation, and agent-link unlinking run off the UI thread;
+Esc cancels active work; progress is throttled to 10 updates per second; and
+each owning window cancels and drains its task before disposing controls.
+
+Cancellation publishes exact aggregate progress, including cancellation
+between batch targets. The workflow invalidates and rescans inventory whenever
+even a partial target deleted files or directories. Runtime failure detail is
+bounded to 128 messages plus an omission summary while `ErrorCount` preserves
+the exact total. PR #14's first Copilot review found two terminal-progress
+gaps: already-canceled link removal omitted its final canceled event, and the
+outer batch catch could overwrite exact mid-target counts with completed-target
+totals. Both are fixed by explicit early-cancellation publication and by making
+the batch adapter retain and republish its latest aggregate snapshot.
+
+PR #14's Balanced follow-up review surfaced six suppressed accounting issues
+in the same change set. All were accepted and fixed in the PR: progress now
+distinguishes processed targets from successfully deleted targets; compact and
+advanced removal flows retain mutation totals across retries and escalation;
+cancellation reports preserve the exact observed runtime-error count while
+marking cancellation separately; and remove JSON only emits runtime-error
+fields after validation and confirmation permit an actual removal or dry run.
+The subsequent Balanced review also found that a refused direct removal
+returned after its initial progress event. Refusals now publish a forced
+terminal completed snapshot with one processed target, zero deleted targets,
+and the refusal error count.
 
 ### Impact
 
@@ -1169,14 +1208,17 @@ coordinated within their current scope:
 6. [x] Add log character/byte budgets and correct disk rotation.
 7. [x] Enforce aggregate disk retention during active-file growth and remove
    cancellation-callback execution from request/slot ownership locks.
-8. [~] Move inventory and removal I/O off the UI thread with cancellation, and
+8. [x] Move inventory and removal I/O off the UI thread with cancellation, and
    evaluate native handle-relative deletion for hostile same-user mutation.
-   Inventory scanning and cleanup classification are complete; asynchronous
-   removal/progress and native deletion evaluation remain.
+   The portable inventory/removal work is complete. The native evaluation
+   confirmed that supported .NET 10 APIs cannot make path validation and
+   deletion atomic; an audited Unix/Windows native implementation remains a
+   separate Finding 1 security follow-up rather than an implicit portable fix.
 9. [ ] Finish metadata-preview deadlines and bounded scheduling (search
    supersession and a whole-request deadline are complete).
-10. [ ] Make modal operation lifetimes awaitable and disposal-safe, including
-    stable token capture before the first await.
+10. [~] Make modal operation lifetimes awaitable and disposal-safe, including
+    stable token capture before the first await. Removal modals are complete;
+    the three install flows remain.
 11. [ ] Wire root CLI cancellation and bounded post-kill waiting.
 12. [ ] Centralize cross-platform path identity semantics.
 13. [ ] Correct Esc focus behavior and strengthen the LRU contract test.

src/SkillView.Core/Cli/CliDispatcher.cs

@@ -1188,7 +1188,7 @@ private static void WriteRemoveText(
         }
         else
         {
-            Console.Error.WriteLine($"  remove failed with {r.Errors.Length} error(s)");
+            Console.Error.WriteLine($"  remove failed with {r.ErrorCount} error(s)");
             foreach (var e in r.Errors) Console.Error.WriteLine($"  · {e}");
         }
     }
@@ -1206,6 +1206,8 @@ internal static string RenderRemoveJson(
         ParsedRemoveArgs p,
         RemoveValidator.RemoveValidation validation)
     {
+        var removalAttempted = validation.Allowed
+            && (!validation.RequiresSecondConfirm || p.Yes);
         using var ms = new MemoryStream();
         using (var w = new Utf8JsonWriter(ms, new JsonWriterOptions { Indented = true }))
         {
@@ -1218,6 +1220,7 @@ internal static string RenderRemoveJson(
             w.WriteString("scope", target.Scope.ToString());
             w.WriteNumber("filesDeleted", r.FilesDeleted);
             w.WriteNumber("directoriesDeleted", r.DirectoriesDeleted);
+            w.WriteNumber("runtimeErrorCount", removalAttempted ? r.ErrorCount : 0);
             w.WriteStartArray("errors");
             foreach (var e in validation.Errors)
             {
@@ -1237,7 +1240,10 @@ internal static string RenderRemoveJson(
             }
             w.WriteEndArray();
             w.WriteStartArray("runtimeErrors");
-            foreach (var e in r.Errors) w.WriteStringValue(e);
+            if (removalAttempted)
+            {
+                foreach (var e in r.Errors) w.WriteStringValue(e);
+            }
             w.WriteEndArray();
             w.WriteEndObject();
         }