Improve TUI usability, quit behavior, and resource safety

AGENTS.md

@@ -150,6 +150,11 @@ the terminal, with both a full-screen TUI and scriptable CLI commands.
   paste through Terminal.Gui's default `Command.Paste` pipeline, while read-only
   panes ignore paste events. `TerminalEscapeSanitizer` still applies to rendered
   remote content and is separate from Terminal.Gui's pasted-input sanitization.
+- Keep the main shell's contextual header, inline busy indicators, quit routing,
+  cancellation ownership, and memory bounds aligned with
+  `agent_docs/ui-lifecycle-and-resource-bounds.md`. In particular, `Ctrl+Q`
+  must quit from text fields, subprocess capture and caches must stay bounded,
+  and superseded preview/inventory work must be canceled.
 - If Copilot-specific, Claude-specific, or other agent-platform guidance turns
   out to matter for this repo, capture the repo-relevant part here so future
   agents do not need to rediscover it from external docs.
@@ -164,3 +169,5 @@ the terminal, with both a full-screen TUI and scriptable CLI commands.
   and the current Homebrew / WinGet dark-launch artifacts.
 - `agent_docs/tui-pty-testing.md` — sandboxed PTY workflow, synchronization
   strategy, verification scripts, and known pitfalls for terminal UI testing.
+- `agent_docs/ui-lifecycle-and-resource-bounds.md` — shell conventions,
+  cancellation ownership, and subprocess/cache/log memory limits.

README.md

@@ -182,8 +182,9 @@ Navigation:
 | `Tab` / `Shift+Tab` | Move focus between list and detail |
 | `/` | Jump to Discover and focus the search box |
 | `?` or `F1` | Open the help overlay |
-| `Esc` | Back out of the current sub-view / modal |
-| `q` | Quit |
+| `Esc` | Leave a field or back out of the current sub-view / modal |
+| `q` | Quit from a top-level list or preview |
+| `Ctrl+Q` | Quit from anywhere, including while typing |
 
 Discover tab:
 

agent_docs/tui-pty-testing.md

@@ -92,8 +92,10 @@ immediately.
 
 ### 1. Startup focus is in the query field
 
-At startup, plain letters go into the query field. If you want global shortcuts
-like `d`, `u`, `c`, `I`, or `q`, first send `Esc`.
+At startup, plain letters go into the query field. If you want single-letter
+global shortcuts like `d`, `u`, `c`, or `I`, first send `Esc`. `Ctrl+Q` is the
+unconditional quit path and works without leaving the field; plain `q` quits
+only when a top-level read-only view has focus.
 
 ### 2. Search works even when naive detectors say it failed
 
@@ -213,7 +215,7 @@ These are current repo-specific pitfalls, not generic PTY problems:
 2. Create a temp sandbox (`HOME_DIR`, `WORK_DIR`, `SCAN_ROOT`).
 3. Pick a query and row index from `gh skill search <query> --json ...`.
 4. Launch the built `skillview` binary in a PTY with `SKILLVIEW_LOG=debug`.
-5. Send `Esc` before global shortcuts.
+5. Send `Esc` before single-letter global shortcuts; use `Ctrl+Q` to quit from any focus state.
 6. Wait on log conditions for search/preview milestones.
 7. Verify install/remove with CLI shell scripts after each mutation.
 8. Save cleaned PTY snapshots for UX review.

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

@@ -0,0 +1,52 @@
+# UI lifecycle and resource bounds
+
+Use these rules when changing the main shell, asynchronous tab work, previews,
+logging, or subprocess adapters.
+
+## Shell and interaction conventions
+
+- The window border owns the `SkillView — skillview` / `gh skillview` brand.
+  Do not add another persistent logo. `ContextBarView` owns the active workspace
+  title and its optional context chips.
+- Keep search scope beside the relevant search field. Put actions in the bottom
+  status strip and keep table abbreviations in a separate legend.
+- A `SpinnerView` must be immediately before its status label on the same row.
+  The main spinner is owned by `StatusStripView`; tab and modal spinners use the
+  same inline layout.
+- Configure rendered Markdown through `TuiHelpers.ConfigureMarkdownPane`, which
+  hides literal heading prefixes while retaining semantic heading styles.
+- `Ctrl+Q` is unconditional and must be handled before printable-key routing.
+  Plain `q` is a top-level quit shortcut; `Esc` means leave field, back, or close.
+- Layouts below 80×24 show `TerminalSizeGuardView` instead of overlapping panes.
+
+## Lifetime and memory rules
+
+- `LatestRequestGate` owns Discover preview cancellation. A new preview cancels
+  the previous request, and every completion checks `Lease.IsCurrent` before
+  updating UI.
+- Installed, Changes, and Updates cancel superseded inventory loads. Pass their
+  cancellation token through to inventory capture; do not add generation-only
+  refreshes that leave obsolete scans running. Use `CancellationTokenSourceSlot`
+  so source replacement, cancellation, and lease disposal share one ownership
+  boundary; never cancel a source that another path can independently dispose.
+- Async update/install controls stay disabled while their operation is active.
+  Continue to pass cancellation into subprocess-backed services.
+- `ProcessRunner` retains at most 1 MiB of child output per stdout/stderr stream,
+  plus a small truncation marker. It drains both streams in fixed-size chunks;
+  do not use line-based process events because a newline-free child can make the
+  framework retain an unbounded line before SkillView sees it. Increase the
+  capture limit only with evidence that a supported command needs more output.
+- `SearchAgentMetadataCache` is a thread-safe 512-entry LRU. Do not replace it
+  with an unbounded dictionary.
+- 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
+  begin after disposal returns. The visible log pane retains at most 512
+  formatted lines and coalesces burst refreshes without lost wakeups.
+
+## Focused verification
+
+Run `dotnet build`, then `dotnet test --no-build`. Resource-focused coverage
+includes large subprocess output, LRU eviction, subscription disposal,
+overlapping preview cancellation, superseded inventory scans, inline busy
+state, and layout guards at 80×24, 100×30, and 140×42.

src/SkillView.Core/Bootstrapping/EntryPoint.cs

@@ -51,9 +51,10 @@ public static async Task<int> RunAsync(string[] args)
 
         // In CLI mode, `--debug` streams structured log lines to stderr for
         // immediate visibility.
+        IDisposable? consoleSubscription = null;
         if (options.Debug && options.DispatchMode == DispatchMode.Cli)
         {
-            logger.Subscribe(entry => Console.Error.WriteLine(Logger.Format(entry)));
+            consoleSubscription = logger.Subscribe(entry => Console.Error.WriteLine(Logger.Format(entry)));
         }
 
         var services = TuiServices.Build(logger, fileSink, logDir);
@@ -73,6 +74,7 @@ public static async Task<int> RunAsync(string[] args)
         }
         finally
         {
+            consoleSubscription?.Dispose();
             fileSink?.Dispose();
         }
     }

src/SkillView.Core/Logging/FileLogSink.cs

@@ -22,30 +22,44 @@ public sealed class FileLogSink : IDisposable
     private DateOnly _currentDay;
     private bool _disposed;
     private bool _trimPending;
+    private IDisposable? _subscription;
+    private readonly Action? _beforeAppendLockForTests;
 
     public FileLogSink(string directory, Func<DateTimeOffset>? clock = null)
+        : this(directory, clock, beforeAppendLockForTests: null)
+    {
+    }
+
+    internal FileLogSink(
+        string directory,
+        Func<DateTimeOffset>? clock,
+        Action? beforeAppendLockForTests)
     {
         _directory = directory;
         _clock = clock ?? (() => DateTimeOffset.Now);
+        _beforeAppendLockForTests = beforeAppendLockForTests;
     }
 
     public string Directory => _directory;
 
     public void Attach(Logger logger)
     {
+        ObjectDisposedException.ThrowIf(_disposed, this);
+        _subscription?.Dispose();
         // Emit the ring-buffer snapshot first so the file reflects pre-attach entries too.
         foreach (var entry in logger.Snapshot())
         {
             Append(entry);
         }
-        logger.Subscribe(Append);
+        _subscription = logger.Subscribe(Append);
     }
 
     public void Append(LogEntry entry)
     {
-        if (_disposed) return;
+        _beforeAppendLockForTests?.Invoke();
         lock (_gate)
         {
+            if (_disposed) return;
             try
             {
                 var today = DateOnly.FromDateTime(entry.Timestamp.ToLocalTime().DateTime);
@@ -193,11 +207,26 @@ private static void TrySetPosixMode(string path)
 
     public void Dispose()
     {
+        IDisposable? subscription;
         lock (_gate)
         {
             if (_disposed) return;
             _disposed = true;
+            subscription = _subscription;
+            _subscription = null;
+        }
+
+        // Logger subscription disposal waits for an in-flight callback to
+        // finish. Never wait while holding _gate: Append takes the logger's
+        // registration lock first and then this lock, so the reverse order
+        // here would deadlock shutdown.
+        subscription?.Dispose();
+
+        lock (_gate)
+        {
             CloseWriterLocked();
         }
     }
+
+    internal bool IsDisposedForTests => Volatile.Read(ref _disposed);
 }

src/SkillView.Core/Logging/Logger.cs

@@ -1,4 +1,3 @@
-using System.Collections.Concurrent;
 using System.Globalization;
 
 namespace SkillView.Logging;
@@ -8,9 +7,11 @@ namespace SkillView.Logging;
 public sealed class Logger
 {
     private readonly object _gate = new();
+    private readonly object _observerGate = new();
     private readonly LinkedList<LogEntry> _ring = new();
     private readonly int _capacity;
-    private readonly ConcurrentBag<Action<LogEntry>> _observers = new();
+    private readonly Dictionary<long, ObserverRegistration> _observers = new();
+    private long _nextObserverId;
 
     public Logger(LogLevel minimumLevel = LogLevel.Info, int capacity = 2048)
     {
@@ -20,7 +21,17 @@ public Logger(LogLevel minimumLevel = LogLevel.Info, int capacity = 2048)
 
     public LogLevel MinimumLevel { get; set; }
 
-    public void Subscribe(Action<LogEntry> observer) => _observers.Add(observer);
+    public IDisposable Subscribe(Action<LogEntry> observer)
+    {
+        ArgumentNullException.ThrowIfNull(observer);
+        lock (_observerGate)
+        {
+            var id = ++_nextObserverId;
+            var registration = new ObserverRegistration(observer);
+            _observers.Add(id, registration);
+            return new Subscription(this, id, registration);
+        }
+    }
 
     public void Log(LogLevel level, string category, string message)
     {
@@ -44,9 +55,14 @@ public void Log(LogLevel level, string category, string message)
             }
         }
 
-        foreach (var observer in _observers)
+        ObserverRegistration[] observers;
+        lock (_observerGate)
         {
-            try { observer(entry); }
+            observers = _observers.Values.ToArray();
+        }
+        foreach (var observer in observers)
+        {
+            try { observer.Invoke(entry); }
             catch { /* observer faults must not kill the logger */ }
         }
     }
@@ -74,4 +90,65 @@ public static string Format(LogEntry entry)
             entry.Category,
             entry.Message);
     }
+
+    private void Unsubscribe(long id, ObserverRegistration registration)
+    {
+        lock (_observerGate)
+        {
+            _observers.Remove(id);
+        }
+
+        // Do not hold the collection lock while waiting for an in-flight
+        // callback. This avoids lock inversion if an observer subscribes or
+        // disposes a subscription from inside its callback.
+        registration.Deactivate();
+    }
+
+    private sealed class ObserverRegistration
+    {
+        private readonly object _gate = new();
+        private readonly Action<LogEntry> _observer;
+        private bool _active = true;
+
+        internal ObserverRegistration(Action<LogEntry> observer)
+        {
+            _observer = observer;
+        }
+
+        internal void Invoke(LogEntry entry)
+        {
+            lock (_gate)
+            {
+                if (_active)
+                {
+                    _observer(entry);
+                }
+            }
+        }
+
+        internal void Deactivate()
+        {
+            lock (_gate)
+            {
+                _active = false;
+            }
+        }
+    }
+
+    private sealed class Subscription : IDisposable
+    {
+        private Logger? _owner;
+        private readonly long _id;
+        private readonly ObserverRegistration _registration;
+
+        internal Subscription(Logger owner, long id, ObserverRegistration registration)
+        {
+            _owner = owner;
+            _id = id;
+            _registration = registration;
+        }
+
+        public void Dispose() =>
+            Interlocked.Exchange(ref _owner, null)?.Unsubscribe(_id, _registration);
+    }
 }