Merge pull request #324 from corbitsdev/cl-5351-react-hooks

Remove unnecessary React hooks (CL-5351 slice B)

Author: TheGreatAxios

src/tui/app.tsx

@@ -351,6 +351,9 @@ export function App({
   const [agentModalUsage, setAgentModalUsage] = useState<string | null>(null);
   const [permissionsOpen, setPermissionsOpen] = useState(false);
   const [settingsOpen, setSettingsOpen] = useState(false);
+  // Mount-only seeds from runner props. Runner does not re-render App when these
+  // change; Settings updates flow through the local setters + onChange* callbacks
+  // that mutate runner-held live values. No prop→state sync effect needed.
   const [liveTelemetryEnabled, setLiveTelemetryEnabled] = useState(telemetryEnabled);
   const [waitForApproval, setWaitForApproval] = useState(waitForApprovalProp);
   const [compactionMode, setCompactionMode] = useState<CompactionMode>(
@@ -555,13 +558,12 @@ export function App({
 
   const { goalActive, goalPhase, showAcceptance, workPrimary } = resolveGoalChrome({ goalSnapshot });
   // Default-expand Work when entering implementing; Ctrl+T can still collapse.
+  // Adjust during render so the panel opens in the same paint as the phase flip.
   const wasWorkPrimary = useRef(false);
-  useEffect(() => {
-    if (workPrimary && !wasWorkPrimary.current) {
-      setTasksExpanded(true);
-    }
-    wasWorkPrimary.current = workPrimary;
-  }, [workPrimary]);
+if (workPrimary && !wasWorkPrimary.current) {
+    setTasksExpanded(true);
+  }
+  wasWorkPrimary.current = workPrimary;
   // Drop the /goal one-shot once Goal chrome is live so it does not stack on
   // the brief / Work checklist (and blow the reserved chrome rows).
   useEffect(() => {

src/tui/components/chat-input.tsx

@@ -1,5 +1,5 @@
 import { Box, Text, useInput, usePaste } from "ink";
-import { useState, useMemo, useEffect, useRef } from "react";
+import { useState, useMemo, useRef } from "react";
 import type { ReactNode } from "react";
 import { getCommand, listCommands } from "../commands/registry.js";
 import type { CommandContext, CommandResult, SubcommandDefinition } from "../commands/registry.js";
@@ -426,10 +426,15 @@ export function ChatInput({
   const selfSetValue = useRef<string | null>(null);
 
   // Reset the cursor to the end only when value changes from the OUTSIDE.
-  useEffect(() => {
-    if (value === selfSetValue.current) return;
-    setCursor(value.length);
-  }, [value]);
+  // Adjust during render (not an effect) so the caret lands in the same paint
+  // as the external value update — no extra commit for the cursor alone.
+  const [prevValue, setPrevValue] = useState(value);
+  if (value !== prevValue) {
+    setPrevValue(value);
+    if (value !== selfSetValue.current) {
+      setCursor(value.length);
+    }
+  }
 
   const atMention = useAtSuggestions(cwd);
 

src/tui/components/plugins-manager.tsx

@@ -1,5 +1,5 @@
 import { Box, Text, useInput } from "ink";
-import { useState, useRef, useEffect } from "react";
+import { useState, useRef } from "react";
 import type { ReactNode } from "react";
 import { color } from "../theme.js";
 import { listPathSuggestions } from "./at-mention/index.js";
@@ -86,14 +86,15 @@ export function PluginsManager({ admin, onClose, cwd }: PluginsManagerProps): Re
   const isEnabled = (id: string): boolean => config[id]?.enabled === true;
   const isConsented = (id: string): boolean => config[id]?.consented === true;
 
-  useEffect(() => {
-    if (addingPath === null) {
-      pathGeneration.current++;
-      lastPathPrefix.current = null;
-      setPathSuggestions([]);
-      setPathSelectedIdx(0);
-    }
-  }, [addingPath]);
+  // Clear path-suggestion state when leaving add-by-path (call at every exit site
+  // instead of watching addingPath with an effect).
+  const clearAddingPath = (): void => {
+    pathGeneration.current++;
+    lastPathPrefix.current = null;
+    setPathSuggestions([]);
+    setPathSelectedIdx(0);
+    setAddingPath(null);
+  };
 
   const fetchPathSuggestions = (prefix: string, gen: number) => {
     void listPathSuggestions(prefix, cwd).then((results) => {
@@ -142,7 +143,7 @@ export function PluginsManager({ admin, onClose, cwd }: PluginsManagerProps): Re
     void Promise.resolve(admin.addPath(path)).then(
       (result) => {
         setAddStatus({ ok: result.ok, message: result.message });
-        if (result.ok) { setAddingPath(null); setVersion((v) => v + 1); }
+        if (result.ok) { clearAddingPath(); setVersion((v) => v + 1); }
       },
       (err: unknown) => setAddStatus({ ok: false, message: err instanceof Error ? err.message : String(err) }),
     );
@@ -160,7 +161,7 @@ export function PluginsManager({ admin, onClose, cwd }: PluginsManagerProps): Re
 
     if (addingPath !== null) {
       if (key.escape) {
-        setAddingPath(null);
+        clearAddingPath();
         setAddStatus(null);
         return;
       }

src/tui/components/session-resume-picker.tsx

@@ -1,6 +1,6 @@
 import { Box, Text, useApp, useInput } from "ink";
 import type { ReactNode } from "react";
-import { useEffect, useMemo, useRef, useState } from "react";
+import { useMemo, useRef, useState } from "react";
 
 import type { SessionSummary } from "../../session/index.js";
 import { formatRelativeTime } from "../format-relative-time.js";
@@ -21,10 +21,9 @@ function formatLabel(session: SessionSummary): string {
 export function SessionResumePicker({ sessions, onSelect, onCancel }: SessionResumePickerProps): ReactNode {
   const { exit } = useApp();
   const [cursor, setCursor] = useState(0);
-  const cursorRef = useRef(0);
-  useEffect(() => {
-    cursorRef.current = cursor;
-  }, [cursor]);
+  // Keep the latest cursor available to useInput without an effect round-trip.
+  const cursorRef = useRef(cursor);
+  cursorRef.current = cursor;
   const rows = useMemo(() => sessions.map((s) => ({ session: s, label: formatLabel(s) })), [sessions]);
   const clamped = rows.length > 0 ? Math.min(cursor, rows.length - 1) : 0;
 

src/tui/hooks/use-gates.ts

@@ -138,9 +138,10 @@ export function useGates({
   activationBlocked = false,
 }: UseGatesArgs): GateController {
   const [activeApproval, setActiveApproval] = useState<ActiveApproval | null>(null);
-  const [permissionQueueDepth, setPermissionQueueDepth] = useState(0);
   const [queuedApprovals, setQueuedApprovals] = useState<readonly QueuedApprovalSummary[]>([]);
   const queue = useRef<GateQueueEntry[]>([]);
+  // Depth is the length of the permission summary list — one source of truth.
+  const permissionQueueDepth = queuedApprovals.length;
 
   function syncQueuedApprovals(): void {
     setQueuedApprovals(
@@ -194,7 +195,6 @@ export function useGates({
     clearEntryTimer(entry);
     detachEntryAbort(entry);
     if (entry.kind === "permission") {
-      setPermissionQueueDepth((depth) => Math.max(0, depth - 1));
       syncQueuedApprovals();
     }
     setGatePendingRef.current(false);
@@ -245,7 +245,6 @@ export function useGates({
   function enqueue(entry: GateQueueEntry): void {
     queue.current.push(entry);
     if (entry.kind === "permission") {
-      setPermissionQueueDepth((depth) => depth + 1);
       syncQueuedApprovals();
     }
     setGatePendingRef.current(true);
@@ -256,7 +255,6 @@ export function useGates({
     const remaining = queue.current.splice(0);
     activeId.current = null;
     setActiveApproval(null);
-    setPermissionQueueDepth(0);
     setQueuedApprovals([]);
     for (const entry of remaining) {
       clearEntryTimer(entry);

src/tui/hooks/use-message-pipeline.ts

@@ -114,6 +114,10 @@ export function useMessagePipeline({
   const sendCounterRef = useRef(0);
   const lastSentMessageRef = useRef<string>("");
   const quotaAutoRetryFiredRef = useRef(false);
+  // Bumped on every sent-history load (and on effect cleanup) so only the latest
+  // loadSentMessages result can write browse state — startNewSession and the
+  // hydrate effect share this so neither path can apply a stale session's history.
+  const sentHistoryLoadGenRef = useRef(0);
 
   sendMessageRef.current = (message: OutboundUserMessage) => {
     lastSentMessageRef.current = message.text;
@@ -208,6 +212,17 @@ export function useMessagePipeline({
 
   requestStopRef.current = requestStop;
 
+  // Start a sent-history load; only the newest generation may apply. Shared by
+  // startNewSession and the hydrate effect so rapid /clear or session switches
+  // cannot write browse from a prior id after a newer load has begun.
+  const loadSentHistoryBrowse = (sessionId: string) => {
+    const gen = ++sentHistoryLoadGenRef.current;
+    void loadSentMessages(cwd, sessionId).then((sent) => {
+      if (gen !== sentHistoryLoadGenRef.current) return;
+      setSentHistoryBrowse(createSentHistoryBrowse(sent));
+    });
+  };
+
   const startNewSessionRef = useRef<() => void>(() => undefined);
   startNewSessionRef.current = () => {
     sendAbortRef.current?.abort();
@@ -230,24 +245,28 @@ export function useMessagePipeline({
     subAgentSessions?.clear();
     onNewSession?.();
     if (getSessionId !== undefined) {
-      void loadSentMessages(cwd, getSessionId()).then((sent) => {
-        setSentHistoryBrowse(createSentHistoryBrowse(sent));
-      });
+      loadSentHistoryBrowse(getSessionId());
     } else {
+      // Invalidate any in-flight load before clearing browse for a no-session path.
+      sentHistoryLoadGenRef.current++;
       setSentHistoryBrowse(createSentHistoryBrowse([]));
     }
     scroll.scrollToBottom();
     forceRender((n) => n + 1);
   };
   const startNewSession = () => startNewSessionRef.current();
 
-  // Send the initial task once the App (and its gate listeners) is mounted, so
-  // the run is driven through the same abortable path as interactive sends.
+  // Hydrate sent-message history for the active session. Cancel stale loads so a
+  // session switch or unmount cannot write history from a prior session id.
   useEffect(() => {
     if (getSessionId === undefined) return;
-    void loadSentMessages(cwd, getSessionId()).then((sent) => {
-      setSentHistoryBrowse(createSentHistoryBrowse(sent));
-    });
+    loadSentHistoryBrowse(getSessionId());
+    return () => {
+      sentHistoryLoadGenRef.current++;
+    };
+    // loadSentHistoryBrowse closes over cwd/setSentHistoryBrowse; re-run when the
+    // session identity source or cwd changes.
+    // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [cwd, getSessionId]);
 
   useEffect(() => {

src/tui/use-stream.ts

@@ -1323,6 +1323,10 @@ export function useAgentStream(
     setDisplayRevision((r) => r + 1);
   };
 
+  // `state` is a stable store object from useState — never recreated. Effects
+  // that re-arm on status/quota changes depend only on those fields; listing
+  // `state` itself would be noise (always same identity).
+
   // ~30fps drain makes streaming feel metronomic rather than bursty. Gated to
   // running/blocked so an idle session schedules no periodic timer.
   useEffect(() => {
@@ -1334,7 +1338,7 @@ export function useAgentStream(
       }
     }, 33);
     return () => clearInterval(interval);
-  }, [state, state.status]);
+  }, [state.status]);
 
   // Line layout is heavier than chrome updates; coalesce it during token
   // streaming. Gated to running/blocked so an idle session schedules no
@@ -1347,7 +1351,7 @@ export function useAgentStream(
       }
     }, 100);
     return () => clearInterval(interval);
-  }, [state, state.status]);
+  }, [state.status]);
 
   // requestStop()/clear() can transition status out of running/blocked with a
   // token delta still buffered in pendingRenderRef/pendingLineRevisionRef —
@@ -1361,7 +1365,7 @@ export function useAgentStream(
       setTick((t) => t + 1);
       bumpDisplayRevision();
     }
-  }, [state, state.status]);
+  }, [state.status]);
 
   useEffect(() => {
     const handler = (event: ReactorEmittedEvent) => {
@@ -1406,7 +1410,8 @@ export function useAgentStream(
       emitter.off("subagent.progress", progressHandler);
       emitter.off("history.hydrate", hydrateHandler);
     };
-  }, [emitter, state]);
+    // state is a stable store; only re-bind when the emitter instance changes.
+  }, [emitter]);
 
   useEffect(() => {
     if (state.status !== "running" && state.status !== "blocked" && state.quotaError === null) return;
@@ -1416,12 +1421,13 @@ export function useAgentStream(
     return () => {
       clearInterval(interval);
     };
-  }, [state, state.status, state.quotaError]);
+  }, [state.status, state.quotaError]);
 
   void tick;
 
+  // state identity is stable; re-wrap only when displayRevision advances.
   return useMemo(
     () => Object.assign(Object.create(state), { displayRevision }),
-    [state, displayRevision],
+    [displayRevision],
   );
 }