Harden log retention, cancellation gates, and inventory scanning

[harder]

Summary

Starts the second adversarial hardening batch after PR #12. This closes the final PR #12 log-retention follow-up, fixes two analogous lock/callback hazards found by re-auditing the prior Copilot misses as bug families, and completes the bounded/cancellable local-inventory portion of the remaining audit.

Why

The final PR #12 review noted that aggregate log usage was only enforced when a writer opened or rotated. Older parts could remain while the active part grew, temporarily exceeding the advertised disk budget.

The follow-up audit then found the same broader patterns elsewhere:

• Request cancellation ran registered callbacks while holding ownership locks, allowing a callback to wait for lease release and deadlock.
• Local inventory scanning still ran synchronous filesystem work before its first await, ignored cancellation through several phases, and read complete third-party files.
• Cleanup classification contained another uncancellable lazy-directory walk whose iteration failures were outside its exception boundary.
• The inventory cache invoked its injected clock delegate under the cache lock.

Changes

Enforce aggregate log retention during active growth

• Track retained log bytes after the initial trim.
• Re-run retention on the first append that crosses the aggregate budget, without waiting for file rotation.
• Keep the active part protected from deletion.
• When the active part alone is oversized or an old part cannot be removed, retry after bounded additional growth instead of enumerating the directory for every line.
• Add a regression where old and active parts initially fit, then active-only growth forces old-part removal before rotation.

Remove cancellation callbacks from ownership locks

• Refactor CancellationTokenSourceSlot and LatestRequestGate to publish ownership changes under their gates, then call Cancel() outside the gate.
• Defer source disposal until an in-progress cancellation call returns, preserving the previous cancel/dispose race protection.
• Capture the token struct when a lease is created so later code never dereferences a disposed source.
• Add deterministic cross-thread tests where a cancellation callback waits for another thread to dispose the lease; both gates now complete without lock inversion.

Bound and cancel local inventory work

• Move root resolution, filesystem scanning, and package-lock enrichment off the Terminal.Gui thread.
• Run local scanning concurrently with gh skill list so disk and subprocess latency do not add serially.
• Propagate cancellation through root resolution, candidate scanning, merge/enrichment/filter passes, package-lock parsing, and cleanup classification.
• Check cancellation between roots, entries, agents, bounded read chunks, and in-memory merge phases.
• Catch filesystem errors from lazy enumerator MoveNext, where disappearing directories, ACL changes, and disconnected mounts actually surface.
• Preserve independent filesystem and gh duration diagnostics while the two sources run concurrently.

Bound untrusted local files

• Read at most 64 KiB from each SKILL.md, enough for normal front matter without retaining the full Markdown body.
• Read .skill-lock.json through a pooled buffer and reject manifests over 1 MiB.
• Use read/write/delete sharing for these best-effort inventory reads so concurrent updates do not unnecessarily pin files on Windows.
• Add tests for mid-scan cancellation, an oversized front-matter block, oversized lockfiles, pre-I/O cancellation, and a simulated iteration-time disconnect.

Avoid injected work under the cache lock

• Capture the cache clock outside _gate.
• Convert loader-completion clock failures into the shared operation outcome so waiters cannot be stranded.
• Add a re-entrant clock test that coordinates invalidation from another thread.

Documentation and follow-up ordering

• Record the final PR Harden removal, async lifecycles, cache, and logging #12 disposition and the analogous re-audit in the full adversarial audit.
• Mark bounded inventory scanning complete while keeping asynchronous removal/progress and native handle-relative deletion evaluation as the next ordered sub-batch.
• Sharpen the existing install-modal finding: its async void handlers repeatedly access a using-owned cancellation source after awaits, the same disposed-source family found in Discover.
• Update AGENTS.md and the lifecycle/resource guide with the durable cancellation-lock, bounded-inventory, and incremental-retention rules.

Validation

• dotnet build --no-restore: passed with 0 warnings and 0 errors.
• dotnet test --no-build: 625 passed, 0 failed, including the ANSI-driver integration suite.
• Focused cache, inventory, logging, cancellation-slot, and latest-request tests passed.
• macOS ARM64 AOT publish passed for the standalone app and gh extension.
• Both native artifacts passed --version smoke checks.
• git diff --check: passed.

CI will repeat tests and AOT smoke publishing on Linux, macOS, and Windows, plus CodeQL.

Release notes

• Keeps total SkillView log storage within its configured budget as the active log grows, rather than waiting for the next rotation.
• Prevents request cancellation callbacks from deadlocking against request/operation lease ownership.
• Keeps Installed, Changes, Updates, startup, rescan, and cleanup inventory work responsive by moving local filesystem scanning off the UI thread and honoring cancellation throughout.
• Prevents oversized third-party SKILL.md and .skill-lock.json files from causing large inventory-time allocations.
• Makes inventory scanning more resilient to disappearing directories, permission changes, removable media, and disconnected/network-backed paths across Windows, macOS, and Linux.

Remaining ordered work

The next checkpoint is intentionally separate because it crosses destructive-operation and modal-lifetime boundaries:

1. Run removal asynchronously with cancellation and throttled progress.
2. Evaluate audited native handle-relative/no-follow deletion for the hostile same-user mutation threat model.
3. Finish metadata-preview deadlines and bounded scheduling.
4. Make install modal operations awaitable and disposal-safe.
5. Continue with root CLI cancellation, path identity, Esc/LRU behavior, and remaining stress coverage.

[Copilot]

🔵 Needs a closer look

It modifies multiple cross-cutting concurrency and filesystem I/O paths (locks, cancellation, retention, inventory capture) where subtle regressions are hard to fully validate without final human review despite strong test coverage.

Pull request overview

This PR continues the repo’s adversarial hardening work by tightening resource bounds and cancellation/locking behavior across logging, request gates, inventory scanning, and the gh skill list cache, with regression tests added to lock in the new guarantees.

Changes:

• Enforces aggregate file-log retention during active-file growth (not just on rotation), with regression coverage.
• Refactors cancellation gates (LatestRequestGate, CancellationTokenSourceSlot) to avoid running Cancel() callbacks under ownership locks, with deterministic deadlock-regression tests.
• Makes local inventory capture bounded and cancellation-aware end-to-end (root resolution, filesystem enumeration, bounded SKILL.md reads, bounded lockfile reads), and runs it concurrently with gh skill list.

File summaries

File	Description
tests/SkillView.Tests/Ui/LatestRequestGateTests.cs	Adds a regression test proving cancellation callbacks don’t execute under the gate lock.
tests/SkillView.Tests/Ui/CancellationTokenSourceSlotTests.cs	Adds a regression test proving slot cancellation callbacks don’t execute under the slot lock.
tests/SkillView.Tests/Logging/FileLogSinkTests.cs	Adds coverage that retention is rechecked as the active log file grows.
tests/SkillView.Tests/Inventory/SkillLockFileReaderTests.cs	New tests for bounded lockfile parsing and pre-I/O cancellation.
tests/SkillView.Tests/Inventory/LocalSkillScannerTests.cs	Adds tests for mid-scan cancellation, bounded SKILL.md reads, and iteration-time enumeration failures.
tests/SkillView.Tests/Inventory/LocalInventoryServiceMergeTests.cs	Adds coverage that merge honors pre-canceled tokens.
tests/SkillView.Tests/Inventory/CleanupClassifierTests.cs	Adds coverage that cleanup classification honors pre-canceled tokens before enumeration.
tests/SkillView.Tests/Gh/GhSkillListCacheTests.cs	Adds a regression ensuring the injected clock callback runs outside the cache lock.
src/SkillView.Core/Ui/SkillViewWorkflowCoordinator.cs	Routes cleanup classification through the cancellation-aware classifier entrypoint.
src/SkillView.Core/Ui/LatestRequestGate.cs	Refactors ownership/cancellation/disposal sequencing to avoid lock + callback hazards.
src/SkillView.Core/Ui/CancellationTokenSourceSlot.cs	Refactors ownership/cancellation/disposal sequencing to avoid lock + callback hazards.
src/SkillView.Core/Logging/FileLogSink.cs	Tracks retained bytes and re-enforces aggregate disk budget during active growth without per-line directory scans.
src/SkillView.Core/Inventory/SkillLockFileReader.cs	Adds a hard size bound (1 MiB) with pooled-buffer reads and cancellation checks.
src/SkillView.Core/Inventory/ScanRootResolver.cs	Adds cancellation-aware root resolution and git-root probing.
src/SkillView.Core/Inventory/LocalSkillScanner.cs	Makes enumeration cancellation-aware, catches iteration-time failures, and bounds SKILL.md reads (64 KiB).
src/SkillView.Core/Inventory/LocalInventoryService.cs	Moves sync filesystem work off the UI thread, runs local scan concurrently with gh skill list, and propagates cancellation throughout.
src/SkillView.Core/Inventory/CleanupClassifier.cs	Adds cancellation-aware classification and fixes lazy-enumerator failure handling for scan-root enumeration.
src/SkillView.Core/Gh/GhSkillListCache.cs	Captures clock outside cache lock and ensures clock failures become shared load outcomes.
docs/reviews/adversarial-concurrency-resource-cancellation-audit-2026-08-28.md	Updates audit status and documents the new remediation checkpoint details.
AGENTS.md	Records durable repo rules for callback-safe cancellation and bounded inventory/log retention behavior.
agent_docs/ui-lifecycle-and-resource-bounds.md	Updates lifecycle/resource-bound guidance to match the new cancellation + inventory + retention invariants.

Review details

• Files reviewed: 21/21 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.