Fix up-arrow command history requiring 2 keystrokes per entry

PolyPilot.Tests/CommandHistoryTests.cs

@@ -0,0 +1,161 @@
+using PolyPilot.Models;
+using Xunit;
+
+namespace PolyPilot.Tests;
+
+public class CommandHistoryTests
+{
+    [Fact]
+    public void Navigate_EmptyHistory_ReturnsNull()
+    {
+        var history = new CommandHistory();
+        Assert.Null(history.Navigate(up: true));
+        Assert.Null(history.Navigate(up: false));
+    }
+
+    [Fact]
+    public void Navigate_Up_ReturnsCursorAtStart()
+    {
+        var history = new CommandHistory();
+        history.Add("first");
+        history.Add("second");
+
+        var result = history.Navigate(up: true);
+        Assert.NotNull(result);
+        Assert.Equal("second", result!.Value.Text);
+        Assert.True(result.Value.CursorAtStart, "ArrowUp should place cursor at start for immediate re-fire");
+    }
+
+    [Fact]
+    public void Navigate_Down_ReturnsCursorAtEnd()
+    {
+        var history = new CommandHistory();
+        history.Add("first");
+        history.Add("second");
+
+        // Navigate up twice then down
+        history.Navigate(up: true);
+        history.Navigate(up: true);
+        var result = history.Navigate(up: false);
+        Assert.NotNull(result);
+        Assert.False(result!.Value.CursorAtStart, "ArrowDown should place cursor at end for immediate re-fire");
+    }
+
+    [Fact]
+    public void Navigate_Up_CyclesThroughAllEntries()
+    {
+        var history = new CommandHistory();
+        history.Add("first");
+        history.Add("second");
+        history.Add("third");
+
+        // Each up-press should return the previous command (single stroke, not double)
+        Assert.Equal("third", history.Navigate(up: true)!.Value.Text);
+        Assert.Equal("second", history.Navigate(up: true)!.Value.Text);
+        Assert.Equal("first", history.Navigate(up: true)!.Value.Text);
+        // Stays at oldest
+        Assert.Equal("first", history.Navigate(up: true)!.Value.Text);
+    }
+
+    [Fact]
+    public void Navigate_UpThenDown_NavigatesCorrectly()
+    {
+        var history = new CommandHistory();
+        history.Add("first");
+        history.Add("second");
+        history.Add("third");
+
+        Assert.Equal("third", history.Navigate(up: true)!.Value.Text);
+        Assert.Equal("second", history.Navigate(up: true)!.Value.Text);
+        // Down goes back toward newest
+        Assert.Equal("third", history.Navigate(up: false)!.Value.Text);
+        // Past end clears to empty
+        Assert.Equal("", history.Navigate(up: false)!.Value.Text);
+    }
+
+    [Fact]
+    public void Add_ResetsCursorToEnd()
+    {
+        var history = new CommandHistory();
+        history.Add("first");
+        history.Add("second");
+
+        // Navigate up
+        history.Navigate(up: true);
+        Assert.Equal(1, history.Index);
+
+        // Add new command resets index past end
+        history.Add("third");
+        Assert.Equal(3, history.Index);
+
+        // Next up should get "third" (the most recent)
+        Assert.Equal("third", history.Navigate(up: true)!.Value.Text);
+    }
+
+    [Fact]
+    public void Add_SkipsDuplicateConsecutive()
+    {
+        var history = new CommandHistory();
+        history.Add("same");
+        history.Add("same");
+        history.Add("same");
+        Assert.Equal(1, history.Count);
+    }
+
+    [Fact]
+    public void Add_IgnoresEmptyAndNull()
+    {
+        var history = new CommandHistory();
+        history.Add("");
+        history.Add(null!);
+        Assert.Equal(0, history.Count);
+    }
+
+    [Fact]
+    public void Add_EnforcesMaxEntries()
+    {
+        var history = new CommandHistory();
+        for (int i = 0; i < 55; i++)
+            history.Add($"cmd-{i}");
+
+        Assert.Equal(50, history.Count);
+        // Most recent should still be accessible
+        Assert.Equal("cmd-54", history.Navigate(up: true)!.Value.Text);
+        // Navigate all the way up to oldest surviving entry
+        for (int i = 0; i < 49; i++)
+            history.Navigate(up: true);
+        // Oldest surviving is cmd-5 (first 5 were trimmed)
+        Assert.Equal("cmd-5", history.Navigate(up: true)!.Value.Text);
+    }
+
+    [Fact]
+    public void IsNavigating_TrueAfterUpFalseAfterReturningToEnd()
+    {
+        var history = new CommandHistory();
+        history.Add("first");
+        history.Add("second");
+        history.Add("third");
+
+        Assert.False(history.IsNavigating, "IsNavigating should be false at start");
+        history.Navigate(up: true);  // index -> 2, shows "third"
+        Assert.True(history.IsNavigating, "IsNavigating should be true after first ArrowUp");
+        history.Navigate(up: true);  // index -> 1, shows "second"
+        Assert.True(history.IsNavigating, "IsNavigating should be true after second ArrowUp");
+        history.Navigate(up: false); // index -> 2, shows "third"
+        Assert.True(history.IsNavigating, "IsNavigating should be true when on 'third' (not yet at end)");
+        history.Navigate(up: false); // index -> 3, past end, shows ""
+        Assert.False(history.IsNavigating, "IsNavigating should be false after returning past end");
+    }
+
+    [Fact]
+    public void Navigate_Down_PastEnd_ReturnsEmpty()
+    {
+        var history = new CommandHistory();
+        history.Add("cmd");
+
+        // Already past end, navigate down
+        var result = history.Navigate(up: false);
+        Assert.NotNull(result);
+        Assert.Equal("", result!.Value.Text);
+    }
+}

PolyPilot.Tests/PolyPilot.Tests.csproj

@@ -31,6 +31,7 @@
     <Compile Include="../PolyPilot/Models/SessionOrganization.cs" Link="Shared/SessionOrganization.cs" />
     <Compile Include="../PolyPilot/Models/FiestaModels.cs" Link="Shared/FiestaModels.cs" />
     <Compile Include="../PolyPilot/Models/ModelHelper.cs" Link="Shared/ModelHelper.cs" />
+    <Compile Include="../PolyPilot/Models/CommandHistory.cs" Link="Shared/CommandHistory.cs" />
     <Compile Include="../PolyPilot/Models/RepositoryInfo.cs" Link="Shared/RepositoryInfo.cs" />
     <Compile Include="../PolyPilot/Models/RenderThrottle.cs" Link="Shared/RenderThrottle.cs" />
     <Compile Include="../PolyPilot/Models/DiffParser.cs" Link="Shared/DiffParser.cs" />

PolyPilot/Components/Pages/Dashboard.razor

@@ -351,8 +351,7 @@
     private Dictionary<string, string> inputModeBySession = new();
     private Dictionary<string, List<PendingImage>> pendingImagesBySession = new();
     private Dictionary<string, string> shellCwdBySession = new();
-    private Dictionary<string, List<string>> commandHistoryBySession = new();
-    private Dictionary<string, int> historyIndexBySession = new();
+    private Dictionary<string, CommandHistory> commandHistoryBySession = new();
     private int fontSize = 20;
     private string? expandedSession;
     private string mobileRemoteUrl = "";
@@ -649,12 +648,18 @@
                             var ta = e.target;
                             var atStart = ta.selectionStart === 0 && ta.selectionEnd === 0;
                             var atEnd = ta.selectionStart === ta.value.length;
-                            if ((e.key === 'ArrowUp' && atStart) || (e.key === 'ArrowDown' && atEnd)) {
-                                var card = ta.closest('[data-session]');
-                                var sessionName = card ? card.dataset.session : '';
+                            var card = ta.closest('[data-session]');
+                            var sessionName = card ? card.dataset.session : '';
+                            var histNavActive = sessionName && window.__histNavActive && window.__histNavActive[sessionName];
+                            if ((e.key === 'ArrowUp' && atStart) || (e.key === 'ArrowDown' && (atEnd || histNavActive))) {
                                 if (sessionName && window.__dashRef) {
                                     e.preventDefault();
-                                    window.__dashRef.invokeMethodAsync('JsNavigateHistory', sessionName, e.key === 'ArrowUp');
+                                    window.__dashRef.invokeMethodAsync('JsNavigateHistory', sessionName, e.key === 'ArrowUp').then(function(isNav) {
+                                        if (!window.__histNavActive) window.__histNavActive = {};
+                                        window.__histNavActive[sessionName] = isNav;
+                                    }).catch(function() {
+                                        if (window.__histNavActive) window.__histNavActive[sessionName] = false;
+                                    });
                                 }
                             }
                         }
@@ -716,6 +721,15 @@
                             }
                         }
                     });
+                    // Reset histNavActive when user edits text, so ArrowDown doesn't
+                    // overwrite their changes with a history entry.
+                    document.addEventListener('input', function(e) {
+                        if (e.target.matches && e.target.matches('.card-input input, .card-input textarea, .input-row textarea')) {
+                            var card = e.target.closest('[data-session]');
+                            var sn = card ? card.dataset.session : '';
+                            if (sn && window.__histNavActive) window.__histNavActive[sn] = false;
+                        }
+                    });
                 }
             ");
             try
@@ -1159,14 +1173,7 @@
         {
             if (!commandHistoryBySession.ContainsKey(sessionName))
                 commandHistoryBySession[sessionName] = new();
-            var history = commandHistoryBySession[sessionName];
-            // Avoid consecutive duplicates
-            if (history.Count == 0 || history[^1] != finalPrompt)
-            {
-                history.Add(finalPrompt);
-                if (history.Count > 50) history.RemoveAt(0);
-            }
-            historyIndexBySession[sessionName] = history.Count; // past the end = "no selection"
+            commandHistoryBySession[sessionName].Add(finalPrompt);
         }
 
         // Handle ! shell commands
@@ -2681,24 +2688,19 @@
     }
 
     [JSInvokable]
-    public async Task JsNavigateHistory(string sessionName, bool up)
+    public async Task<bool> JsNavigateHistory(string sessionName, bool up)
     {
-        if (!commandHistoryBySession.TryGetValue(sessionName, out var history) || history.Count == 0)
-            return;
+        if (!commandHistoryBySession.TryGetValue(sessionName, out var hist))
+            return false;
 
-        if (!historyIndexBySession.ContainsKey(sessionName))
-            historyIndexBySession[sessionName] = history.Count;
-
-        var idx = historyIndexBySession[sessionName];
-        if (up)
-            idx = Math.Max(0, idx - 1);
-        else
-            idx = Math.Min(history.Count, idx + 1);
+        var result = hist.Navigate(up);
+        if (result == null) return false;
 
-        historyIndexBySession[sessionName] = idx;
+        var (text, cursorAtStart) = result.Value;
         var inputId = $"input-{sessionName.Replace(" ", "-")}";
-        var text = idx < history.Count ? history[idx] : "";
-        await JS.InvokeVoidAsync("eval", $"(function(){{ var el = document.getElementById('{inputId}'); if(el){{ el.value = {System.Text.Json.JsonSerializer.Serialize(text)}; el.setSelectionRange(el.value.length, el.value.length); }} }})()");
+        var cursorExpr = cursorAtStart ? "0" : "el.value.length";
+        await JS.InvokeVoidAsync("eval", $"(function(){{ var el = document.getElementById('{inputId}'); if(el){{ el.value = {System.Text.Json.JsonSerializer.Serialize(text)}; var p = {cursorExpr}; el.setSelectionRange(p, p); }} }})()");
+        return hist.IsNavigating;
     }
 
     [JSInvokable]

PolyPilot/Models/CommandHistory.cs

@@ -0,0 +1,46 @@
+namespace PolyPilot.Models;
+
+/// <summary>
+/// Manages per-session command history with up/down navigation.
+/// </summary>
+public class CommandHistory
+{
+    private readonly List<string> _entries = new();
+    private int _index;
+    private const int MaxEntries = 50;
+
+    public int Count => _entries.Count;
+    public int Index => _index;
+    /// <summary>True when the user has navigated up and has not yet returned to the "live" position.</summary>
+    public bool IsNavigating => _index < _entries.Count;
+
+    public void Add(string command)
+    {
+        if (string.IsNullOrEmpty(command)) return;
+        if (_entries.Count == 0 || _entries[^1] != command)
+        {
+            _entries.Add(command);
+            if (_entries.Count > MaxEntries) _entries.RemoveAt(0);
+        }
+        _index = _entries.Count; // past the end = "no selection"
+    }
+
+    /// <summary>
+    /// Navigate history. Returns (text, cursorAtStart).
+    /// cursorAtStart is true when navigating up (so next ArrowUp fires immediately),
+    /// false when navigating down (so next ArrowDown fires immediately).
+    /// Returns null if history is empty.
+    /// </summary>
+    public (string Text, bool CursorAtStart)? Navigate(bool up)
+    {
+        if (_entries.Count == 0) return null;
+
+        if (up)
+            _index = Math.Max(0, _index - 1);
+        else
+            _index = Math.Min(_entries.Count, _index + 1);
+
+        var text = _index < _entries.Count ? _entries[_index] : "";
+        return (text, up);
+    }
+}