diff --git a/AGENTS.md b/AGENTS.md index 848e6ac..121be78 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -450,7 +450,9 @@ the terminal, with both a full-screen TUI and scriptable CLI commands. - 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, + must quit from text fields; the ANSI driver can report bare Esc followed by + Ctrl+Q as Ctrl+Alt+Q, which must also quit. Keep the opt-in PTY quit regression. + 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 diff --git a/agent_docs/tui-pty-testing.md b/agent_docs/tui-pty-testing.md index f335579..68bae6e 100644 --- a/agent_docs/tui-pty-testing.md +++ b/agent_docs/tui-pty-testing.md @@ -105,6 +105,12 @@ 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. +In a baseline headless PTY, Terminal.Gui can hold a bare Esc byte and combine +it with the next key as Alt+key, even after a delay. Use Tab to leave the query +field when a scripted walkthrough needs a separate focus change. The opt-in +PTY test covers Esc followed by Ctrl+Q; SkillView accepts the resulting +Ctrl+Alt+Q as quit. + ### 2. Search works even when naive detectors say it failed The reliable sign is the log line: diff --git a/src/SkillView.Core/Cli/CliDispatcher.cs b/src/SkillView.Core/Cli/CliDispatcher.cs index 5f8979b..f6f19ae 100644 --- a/src/SkillView.Core/Cli/CliDispatcher.cs +++ b/src/SkillView.Core/Cli/CliDispatcher.cs @@ -1442,7 +1442,9 @@ internal static ParsedCleanupArgs ParseCleanupArgs(IReadOnlyList args) kinds = a["--candidates=".Length..].Split(',', StringSplitOptions.RemoveEmptyEntries).ToList(); continue; } - if (a == "--candidates" && i + 1 < args.Count) + if (a == "--candidates" + && i + 1 < args.Count + && !args[i + 1].StartsWith("-", StringComparison.Ordinal)) { kinds = args[++i].Split(',', StringSplitOptions.RemoveEmptyEntries).ToList(); continue; diff --git a/src/SkillView.Core/Inventory/LocalInventoryService.cs b/src/SkillView.Core/Inventory/LocalInventoryService.cs index dea812e..9ba370d 100644 --- a/src/SkillView.Core/Inventory/LocalInventoryService.cs +++ b/src/SkillView.Core/Inventory/LocalInventoryService.cs @@ -1,6 +1,7 @@ using System.Collections.Immutable; using System.Diagnostics; using System.IO; +using System.Runtime.InteropServices; using SkillView.Gh; using SkillView.Gh.Models; using SkillView.Inventory.Models; @@ -211,10 +212,11 @@ internal static ImmutableArray MergeWithCancellation( // Build a key→record index for the scan output. var scanIndex = new Dictionary(StringComparer.Ordinal); + var caseSensitivityByParent = new Dictionary(StringComparer.Ordinal); foreach (var s in scanned) { cancellationToken.ThrowIfCancellationRequested(); - scanIndex[PathIdentity.NormalizeKey(s.ResolvedPath)] = s; + scanIndex[NormalizeMergeKey(s.ResolvedPath, caseSensitivityByParent)] = s; } var outputBuilder = ImmutableArray.CreateBuilder(scanned.Length + ghRecords.Length); @@ -223,7 +225,7 @@ internal static ImmutableArray MergeWithCancellation( foreach (var rec in ghRecords) { cancellationToken.ThrowIfCancellationRequested(); - var key = ResolveKey(rec); + var key = ResolveKey(rec, caseSensitivityByParent); if (key is not null && scanIndex.TryGetValue(key, out var match)) { outputBuilder.Add(match with { Provenance = Provenance.Both }); @@ -290,10 +292,58 @@ private static ImmutableArray FilterWithCancellation( return builder.ToImmutable(); } - private static string? ResolveKey(GhSkillListRecord rec) + private static string? ResolveKey( + GhSkillListRecord rec, + Dictionary caseSensitivityByParent) { var path = rec.ResolvedPath ?? rec.Path; - return string.IsNullOrEmpty(path) ? null : PathIdentity.NormalizeKey(path); + return string.IsNullOrEmpty(path) + ? null + : NormalizeMergeKey(path, caseSensitivityByParent); + } + + private static string NormalizeMergeKey( + string path, + Dictionary caseSensitivityByParent) + { + // gh and the filesystem scanner can spell the same existing install + // through different ancestor symlinks (notably /var and /private/var + // on macOS). Resolve those aliases for inventory matching only; keep + // the original paths in the records for display and removal policy. + var normalized = PathIdentity.Normalize(path); + if (OperatingSystem.IsMacOS() || OperatingSystem.IsLinux()) + { + var resolved = UnixNative.realpath(normalized, IntPtr.Zero); + if (resolved != IntPtr.Zero) + { + try + { + normalized = Marshal.PtrToStringUTF8(resolved) ?? normalized; + } + finally + { + UnixNative.free(resolved); + } + } + } + + var parent = Path.GetDirectoryName(normalized) ?? normalized; + if (!caseSensitivityByParent.TryGetValue(parent, out var caseSensitive)) + { + caseSensitive = PathIdentity.IsCaseSensitive(normalized); + caseSensitivityByParent[parent] = caseSensitive; + } + + return PathIdentity.NormalizeKey(normalized, caseSensitive); + } + + private static class UnixNative + { + [DllImport("libc")] + internal static extern IntPtr realpath(string path, IntPtr resolvedPath); + + [DllImport("libc")] + internal static extern void free(IntPtr pointer); } internal static Scope? ParseScope(string? scope) diff --git a/src/SkillView.Core/Ui/SkillViewApp.cs b/src/SkillView.Core/Ui/SkillViewApp.cs index e20f9fe..92f0ebb 100644 --- a/src/SkillView.Core/Ui/SkillViewApp.cs +++ b/src/SkillView.Core/Ui/SkillViewApp.cs @@ -778,7 +778,7 @@ private static bool IsPrintableTextInputKey(Key key) } internal static bool IsUnconditionalQuitKey(Key key) => - key.KeyCode == (KeyCode.Q | KeyCode.CtrlMask); + (key.KeyCode & ~KeyCode.AltMask) == (KeyCode.Q | KeyCode.CtrlMask); /// Switch active tab. All three (Discover / Installed / Changes) are /// embedded views — flipping the Visible flags swaps them in-place diff --git a/tests/SkillView.IntegrationTests/Ui/SkillViewAppIntegrationTests.cs b/tests/SkillView.IntegrationTests/Ui/SkillViewAppIntegrationTests.cs index bb7ad2f..063460c 100644 --- a/tests/SkillView.IntegrationTests/Ui/SkillViewAppIntegrationTests.cs +++ b/tests/SkillView.IntegrationTests/Ui/SkillViewAppIntegrationTests.cs @@ -133,6 +133,39 @@ public async Task RunAsync_QuitPaths_StopFromEveryTopLevelWorkspace(string scena Assert.Equal(ExitCodes.Success, exitCode); } + [Fact] + public async Task RunAsync_RapidTabSwitches_ThenQuit_CompletesWithoutTimeout() + { + var services = TuiServices.Build(new Logger(LogLevel.Debug)); + var options = new AppOptions( + InvocationMode.Standalone, + DispatchMode.Tui, + Debug: false, + Theme: AppTheme.Default, + ScanRoots: [], + SubcommandName: null, + SubcommandArgs: []); + var keys = new List { new(KeyCode.Esc) }; + for (var index = 0; index < 10; index++) + { + keys.Add(new Key('2')); + keys.Add(new Key('3')); + keys.Add(new Key('1')); + } + keys.Add(new Key(KeyCode.Q | KeyCode.CtrlMask)); + using var timeout = new CancellationTokenSource(TimeSpan.FromSeconds(10)); + var app = new SkillViewApp( + services, + options, + () => CreateAnsiAppWithInput(keys), + probeOnRun: false); + + var exitCode = await app.RunAsync(timeout.Token); + + Assert.False(timeout.IsCancellationRequested); + Assert.Equal(ExitCodes.Success, exitCode); + } + private static IApplication CreateAnsiApp() { var app = Application.Create(); diff --git a/tests/SkillView.Tests/Cli/CliDispatcherParserTests.cs b/tests/SkillView.Tests/Cli/CliDispatcherParserTests.cs index f758258..6b0cd22 100644 --- a/tests/SkillView.Tests/Cli/CliDispatcherParserTests.cs +++ b/tests/SkillView.Tests/Cli/CliDispatcherParserTests.cs @@ -174,6 +174,15 @@ public void Cleanup_CandidateFilterIsCommaSplit() Assert.True(p.Yes); } + [Fact] + public void Cleanup_BareCandidatesDoesNotConsumeJsonFlag() + { + var p = CliDispatcher.ParseCleanupArgs(new[] { "--candidates", "--json" }); + + Assert.Null(p.KindFilter); + Assert.True(p.Json); + } + [Fact] public void Cleanup_OutputPathCaptured() { diff --git a/tests/SkillView.Tests/Inventory/LocalInventoryServiceMergeTests.cs b/tests/SkillView.Tests/Inventory/LocalInventoryServiceMergeTests.cs index e5257f7..8fa31f4 100644 --- a/tests/SkillView.Tests/Inventory/LocalInventoryServiceMergeTests.cs +++ b/tests/SkillView.Tests/Inventory/LocalInventoryServiceMergeTests.cs @@ -55,6 +55,37 @@ public void Matching_records_collapse_to_both_provenance() Assert.Equal(Provenance.Both, merged[0].Provenance); } + [Fact] + public void Ancestor_symlink_aliases_collapse_to_one_install_on_unix() + { + if (!OperatingSystem.IsMacOS() && !OperatingSystem.IsLinux()) return; + + var root = Directory.CreateTempSubdirectory("skillview-merge-alias-"); + try + { + var target = Path.Combine(root.FullName, "target"); + var alias = Path.Combine(root.FullName, "alias"); + var skill = Path.Combine(target, "foo"); + Directory.CreateDirectory(skill); + Directory.CreateSymbolicLink(alias, target); + + var scanned = ImmutableArray.Create(Scan("foo", skill)); + var gh = ImmutableArray.Create(new GhSkillListRecord + { + Name = "foo", + Path = Path.Combine(alias, "foo"), + }); + + var merged = LocalInventoryService.Merge(scanned, gh); + + Assert.Equal(Provenance.Both, Assert.Single(merged).Provenance); + } + finally + { + root.Delete(recursive: true); + } + } + [Fact] public void Cli_only_records_surface_as_CliList_provenance() { diff --git a/tests/SkillView.Tests/Ui/PtyStartupTests.cs b/tests/SkillView.Tests/Ui/PtyStartupTests.cs index 9698e22..3ed09b4 100644 --- a/tests/SkillView.Tests/Ui/PtyStartupTests.cs +++ b/tests/SkillView.Tests/Ui/PtyStartupTests.cs @@ -14,7 +14,7 @@ public sealed class PtyStartupTests StringComparison.OrdinalIgnoreCase); [Fact] - public void BuiltBinary_StartsInsidePty_AndRendersSearchView() + public void BuiltBinary_StartsInsidePty_AndEscapeThenCtrlQQuits() { if (!ShouldRun) { @@ -88,6 +88,17 @@ public void BuiltBinary_StartsInsidePty_AndRendersSearchView() } Assert.True(ready, $"SkillView did not reach PTY startup readiness. stderr: {stderr}"); + + // Some terminals send a bare Escape key. Follow it with Ctrl+Q + // after the parser's normal escape-sequence timeout to verify the + // documented quit path still works through the real PTY driver. + process.StandardInput.Write("\u001b"); + process.StandardInput.Flush(); + Thread.Sleep(150); + process.StandardInput.Write("\u0011"); + process.StandardInput.Flush(); + Assert.True(process.WaitForExit(5000), "SkillView did not quit after Escape then Ctrl+Q."); + Assert.Equal(0, process.ExitCode); } finally { diff --git a/tests/SkillView.Tests/Ui/SkillViewAppTests.cs b/tests/SkillView.Tests/Ui/SkillViewAppTests.cs index 111b948..ab184bc 100644 --- a/tests/SkillView.Tests/Ui/SkillViewAppTests.cs +++ b/tests/SkillView.Tests/Ui/SkillViewAppTests.cs @@ -1292,6 +1292,8 @@ public void CtrlQ_IsAnUnconditionalQuitKey() { Assert.True(SkillViewApp.IsUnconditionalQuitKey( new Key(KeyCode.Q | KeyCode.CtrlMask))); + Assert.True(SkillViewApp.IsUnconditionalQuitKey( + new Key(KeyCode.Q | KeyCode.CtrlMask | KeyCode.AltMask))); Assert.False(SkillViewApp.IsUnconditionalQuitKey(new Key('q'))); } }