fix(lr-a3ca): replace double getPendingNavigate() with peekPendingNavigate() in history_meta

lib/public/modules/app-messages.js

@@ -19,7 +19,7 @@ import { handleFindInSessionResults } from './session-search.js';
 import { handleInputSync, autoResize, resetAutoResize, builtinCommands, setScheduleBtnDisabled } from './input.js';
 import { startThinking, appendThinking, stopThinking, resetThinkingGroup, createToolItem, updateToolExecuting, updateToolResult, markAllToolsDone, closeToolGroup, removeToolFromGroup, resetToolState, getTools, getPlanContent, setPlanContent, renderPlanBanner, renderPlanCard, getTodoTools, handleTodoWrite, handleTaskCreate, handleTaskUpdate, applyDeadSessionTodoCompaction, isPlanFilePath, enableMainInput, addTurnMeta, updateSubagentActivity, addSubagentToolEntry, markSubagentDone, initSubagentStop, updateSubagentProgress, updateSubagentTaskStatus, renderAskUserQuestion, markAskUserAnswered, renderPermissionRequest, markPermissionCancelled, markPermissionResolved, renderElicitationRequest, markElicitationResolved } from './tools.js';
 import { showDoneNotification, playDoneSound, isNotifAlertEnabled, isNotifSoundEnabled } from './notifications.js';
-import { handleFsList, handleFsRead, handleFileChanged, handleDirChanged, handleFileHistory, handleGitDiff, handleFileAt, refreshIfOpen, getPendingNavigate, handleFsSearch } from './filebrowser.js';
+import { handleFsList, handleFsRead, handleFileChanged, handleDirChanged, handleFileHistory, handleGitDiff, handleFileAt, refreshIfOpen, getPendingNavigate, peekPendingNavigate, handleFsSearch } from './filebrowser.js';
 import { isProjectSettingsOpen, refreshProjectSettingsModels, handleInstructionsRead, handleInstructionsWrite, handleProjectEnv, handleProjectEnvSaved, handleProjectSharedEnv, handleProjectSharedEnvSaved, handleProjectOwnerChanged } from './project-settings.js';
 import { updateSettingsModels, updateSettingsStats, updateDaemonConfig, handleSetPinResult, handleKeepAwakeChanged, handleAutoContinueChanged, handleRestartResult, handleShutdownResult, handleSharedEnv, handleSharedEnvSaved, handleGlobalClaudeMdRead, handleGlobalClaudeMdWrite } from './server-settings.js';
 import { handleTermList, handleTermCreated, sendTerminalCommand, handleTermOutput, handleTermResized, handleTermExited, handleTermClosed } from './terminal.js';
@@ -69,7 +69,11 @@ export function processMessage(msg) {
         // append sequence. Without this, scroll events fired during DOM growth
         // can set isUserScrolledUp=true before history_done's arm fires, leaving
         // the user stranded mid-conversation on resume (especially on mobile).
-        if (!getPendingNavigate() || !(getPendingNavigate().toolId || getPendingNavigate().assistantUuid)) {
+        // Peek without consuming — getPendingNavigate() is the single consumer
+        // in the history_done branch below.  Calling getPendingNavigate() here
+        // would clear the value before history_done can use it.
+        var _metaNav = peekPendingNavigate();
+        if (!_metaNav || !(_metaNav.toolId || _metaNav.assistantUuid)) {
           armStickyBottom(5000, true); // force: history replay must scroll to bottom
         }
         break;

lib/public/modules/filebrowser.js

@@ -5,6 +5,12 @@ import { closeSidebar } from './sidebar.js';
 import { closeTerminal } from './terminal.js';
 import { renderUnifiedDiff, renderSplitDiff } from './diff.js';
 import { initFileIcons, getFileIconSvg, getFolderIconSvg } from './fileicons.js';
+import {
+  setPendingNavigate as _setPendingNavigate,
+  clearPendingNavigate as _clearPendingNavigate,
+  peekPendingNavigate,
+  getPendingNavigate,
+} from './pending-navigate.js';
 
 var ctx;
 var showDropHint = function () {};
@@ -15,7 +21,7 @@ var isRendered = false;      // markdown render toggle state
 var currentIsMarkdown = false;
 var historyVisible = false;
 var currentHistoryEntries = [];
-var pendingNavigate = null;  // { sessionLocalId, assistantUuid }
+// pendingNavigate state delegated to ./pending-navigate.js (DOM-free, testable)
 var selectedEntries = [];    // up to 2 selected for compare
 var compareMode = false;
 var inlineDiffActive = false;
@@ -419,7 +425,7 @@ export function resetFileBrowser() {
   currentIsMarkdown = false;
   historyVisible = false;
   currentHistoryEntries = [];
-  pendingNavigate = null;
+  _clearPendingNavigate();
   selectedEntries = [];
   compareMode = false;
   inlineDiffActive = false;
@@ -1929,11 +1935,11 @@ function navigateToEdit(edit) {
     return;
   }
 
-  pendingNavigate = {
+  _setPendingNavigate({
     sessionLocalId: edit.sessionLocalId,
     assistantUuid: edit.assistantUuid,
     toolId: edit.toolId,
-  };
+  });
 
   if (ctx.ws && ctx.connected) {
     ctx.ws.send(JSON.stringify({ type: "switch_session", id: edit.sessionLocalId }));
@@ -1959,8 +1965,5 @@ function scrollToToolElement(toolId, assistantUuid) {
   });
 }
 
-export function getPendingNavigate() {
-  var nav = pendingNavigate;
-  pendingNavigate = null;
-  return nav;
-}
+// peekPendingNavigate and getPendingNavigate are imported from
+// ./pending-navigate.js and re-exported from the import statement at the top.

lib/public/modules/pending-navigate.js

@@ -0,0 +1,28 @@
+// pendingNavigate state machine — no DOM dependencies.
+// Imported by filebrowser.js (browser) and by test/pending-navigate.test.js
+// (Node ESM import — no DOM shim needed).
+//
+// Extracted for lr-a3ca: history_meta must peek without consuming so that
+// history_done remains the sole consumer.
+
+var pendingNavigate = null;
+
+export function setPendingNavigate(nav) {
+  pendingNavigate = nav;
+}
+
+export function clearPendingNavigate() {
+  pendingNavigate = null;
+}
+
+// Non-consuming read — use in history_meta guard.
+export function peekPendingNavigate() {
+  return pendingNavigate;
+}
+
+// Consuming read — use in history_done only.
+export function getPendingNavigate() {
+  var nav = pendingNavigate;
+  pendingNavigate = null;
+  return nav;
+}

test/pending-navigate.test.js

@@ -0,0 +1,112 @@
+// Regression tests for the pendingNavigate peek/consume contract (lr-a3ca).
+//
+// Background: history_meta must inspect the pending nav target WITHOUT
+// consuming it (peekPendingNavigate), so that history_done can later consume
+// it exactly once (getPendingNavigate).
+//
+// The bug (before the fix) was three calls to getPendingNavigate() in the
+// history_meta branch of app-messages.js:
+//   if (!getPendingNavigate() || !(getPendingNavigate().toolId || getPendingNavigate().assistantUuid))
+// First call returned the nav and cleared pendingNavigate.  Second call
+// returned null → null.toolId TypeError in the WS onmessage handler.
+// history_done then received null and never navigated.
+//
+// The fix: history_meta uses peekPendingNavigate() (non-consuming); only
+// history_done calls the consuming getPendingNavigate().
+//
+// These tests import the real lib/public/modules/pending-navigate.js — the
+// DOM-free module that filebrowser.js imports its peek/get implementations
+// from.  If pending-navigate.js reverts peekPendingNavigate to a consuming
+// read, the "peek does not consume" and "nav survives history_meta" tests fail.
+
+import { test } from "node:test";
+import assert from "node:assert/strict";
+
+// Node can import ESM modules directly.  pending-navigate.js has no DOM deps.
+// We append a cache-busting query param so each test file invocation gets a
+// fresh module instance; within a single test run the module is loaded once and
+// state is reset manually between tests using the exported clearPendingNavigate.
+import {
+  setPendingNavigate,
+  clearPendingNavigate,
+  peekPendingNavigate,
+  getPendingNavigate,
+} from "../lib/public/modules/pending-navigate.js";
+
+// Reset state before each test by clearing any leftover nav target.
+function reset() { clearPendingNavigate(); }
+
+test("peekPendingNavigate returns the nav object without consuming it", () => {
+  reset();
+  const nav = { sessionLocalId: "s1", assistantUuid: "u1", toolId: "t1" };
+  setPendingNavigate(nav);
+
+  const first = peekPendingNavigate();
+  const second = peekPendingNavigate();
+
+  assert.deepEqual(first, nav, "first peek returns nav");
+  assert.deepEqual(second, nav, "second peek returns same object (non-consuming)");
+  reset();
+});
+
+test("getPendingNavigate returns the nav object and clears it", () => {
+  reset();
+  const nav = { sessionLocalId: "s2", assistantUuid: "u2", toolId: "t2" };
+  setPendingNavigate(nav);
+
+  const first = getPendingNavigate();
+  const second = getPendingNavigate();
+
+  assert.deepEqual(first, nav, "first get returns nav");
+  assert.equal(second, null, "second get returns null (consumed)");
+});
+
+test("peek does not consume — nav survives history_meta and is available for history_done", () => {
+  reset();
+  const nav = { sessionLocalId: "s3", assistantUuid: "u3", toolId: "t3" };
+  setPendingNavigate(nav);
+
+  // Simulate the FIXED history_meta branch: peek, no consume
+  const metaNav = peekPendingNavigate();
+  assert.deepEqual(metaNav, nav, "history_meta (peek) sees the nav target");
+
+  // Nav must still be available after history_meta
+  assert.deepEqual(peekPendingNavigate(), nav, "nav survives history_meta (non-consuming)");
+
+  // Simulate history_done: consume once
+  const doneNav = getPendingNavigate();
+  assert.deepEqual(doneNav, nav, "history_done receives the nav target");
+  assert.equal(getPendingNavigate(), null, "nav is null after history_done consumes it");
+});
+
+test("regression: consuming get in history_meta clears nav before history_done (pre-fix bug)", () => {
+  reset();
+  const nav = { sessionLocalId: "s4", assistantUuid: "u4", toolId: "t4" };
+  setPendingNavigate(nav);
+
+  // Simulate the BUGGY history_meta: getPendingNavigate() called twice
+  // (matches the original `!getPendingNavigate() || !(getPendingNavigate().toolId ...)`)
+  const first = getPendingNavigate();  // consumes nav
+  const second = getPendingNavigate(); // returns null — this was .toolId accessed → TypeError
+  assert.deepEqual(first, nav);
+  assert.equal(second, null, "second consuming get returns null — the bug");
+
+  // history_done now gets null because the bug consumed nav too early
+  assert.equal(getPendingNavigate(), null, "history_done receives null when nav consumed in history_meta");
+});
+
+test("clearPendingNavigate resets state (used by resetFileBrowser)", () => {
+  setPendingNavigate({ sessionLocalId: "s5" });
+  clearPendingNavigate();
+  assert.equal(getPendingNavigate(), null, "clearPendingNavigate empties the state");
+});
+
+test("peekPendingNavigate on empty state returns null", () => {
+  reset();
+  assert.equal(peekPendingNavigate(), null);
+});
+
+test("getPendingNavigate on empty state returns null", () => {
+  reset();
+  assert.equal(getPendingNavigate(), null);
+});