refactor: simplify ToolStrip to session-only toggle state

docs/plans/fix-tool-strip-showtools-toggle.md

@@ -0,0 +1,189 @@
+# Fix Tool Strip showTools Toggle Behavior
+
+## Overview
+
+This plan addresses the tool strip toggle behavior to make it session-only (not persisted to localStorage) and controlled by the `showTools` prop as the default state.
+
+### Requirements
+
+1. `showTools` is the default state at render
+2. `showTools=false`: strip collapsed, all tools collapsed
+3. `showTools=true`: strip expanded, all tools expanded
+4. Strip chevron toggles strip only (show/hide individual tools list)
+5. Tool chevron toggles that specific tool only
+6. All toggles are session-only (lost on refresh)
+7. On reload: reset to `showTools` default
+
+## Files to Modify
+
+### 1. `src/components/agent-chat/ToolStrip.tsx`
+
+**Changes:**
+- Remove `useSyncExternalStore` and related imports from `browser-preferences`
+- Remove localStorage-based persistence
+- Replace `expandedPref` with local `useState` initialized to `showTools`
+- Pass `initialExpanded={showTools}` to each `ToolBlock` instead of `initialExpanded={shouldAutoExpand}`
+- Remove the `autoExpandAbove` and `completedToolOffset` props (no longer needed)
+
+**Before:**
+```tsx
+import { memo, useMemo, useSyncExternalStore } from 'react'
+import {
+  getToolStripExpandedPreference,
+  setToolStripExpandedPreference,
+  subscribeToolStripPreference,
+} from '@/lib/browser-preferences'
+
+// ...
+const expandedPref = useSyncExternalStore(
+  subscribeToolStripPreference,
+  getToolStripExpandedPreference,
+  () => false,
+)
+const expanded = showTools && expandedPref
+
+const handleToggle = () => {
+  setToolStripExpandedPreference(!expandedPref)
+}
+```
+
+**After:**
+```tsx
+import { memo, useMemo, useState } from 'react'
+
+// ...
+const [stripExpanded, setStripExpanded] = useState(showTools)
+
+const handleToggle = () => {
+  setStripExpanded(!stripExpanded)
+}
+
+// In ToolBlock rendering:
+<ToolBlock
+  key={pair.id}
+  name={pair.name}
+  input={pair.input}
+  output={pair.output}
+  isError={pair.isError}
+  status={pair.status}
+  initialExpanded={showTools}
+/>
+```
+
+### 2. `src/lib/browser-preferences.ts`
+
+**Changes:**
+- Remove `toolStrip` from `BrowserPreferencesRecord` type
+- Remove `toolStrip` handling in `normalizeRecord()`
+- Remove `toolStrip` handling in `patchBrowserPreferencesRecord()`
+- Remove `toolStrip` handling in `migrateLegacyKeys()`
+- Remove `getToolStripExpandedPreference()` function
+- Remove `setToolStripExpandedPreference()` function
+- Remove `subscribeToolStripPreference()` function
+- Remove `LEGACY_TOOL_STRIP_STORAGE_KEY` constant
+
+**Removed exports:**
+- `getToolStripExpandedPreference`
+- `setToolStripExpandedPreference`
+- `subscribeToolStripPreference`
+
+### 3. `src/components/agent-chat/MessageBubble.tsx`
+
+**Changes:**
+- Remove `completedToolOffset` and `autoExpandAbove` props from the interface
+- Remove the `toolGroupOffsets` useMemo (no longer needed)
+- Remove `completedToolOffset` and `autoExpandAbove` from ToolStrip props
+
+**Before:**
+```tsx
+interface MessageBubbleProps {
+  // ...
+  completedToolOffset?: number
+  autoExpandAbove?: number
+}
+
+// ...
+<ToolStrip
+  key={`tools-${group.startIndex}`}
+  pairs={group.pairs}
+  isStreaming={isStreaming}
+  completedToolOffset={toolGroupOffsets[group.toolGroupIndex]}
+  autoExpandAbove={autoExpandAbove}
+  showTools={showTools}
+/>
+```
+
+**After:**
+```tsx
+interface MessageBubbleProps {
+  // ...
+  // Remove completedToolOffset and autoExpandAbove
+}
+
+// ...
+<ToolStrip
+  key={`tools-${group.startIndex}`}
+  pairs={group.pairs}
+  isStreaming={isStreaming}
+  showTools={showTools}
+/>
+```
+
+### 4. `src/components/agent-chat/ToolBlock.tsx`
+
+**No changes required.** The component already supports `initialExpanded` prop which controls the initial expanded state.
+
+## Test Updates
+
+### `test/unit/client/components/agent-chat/ToolStrip.test.tsx`
+
+**Remove tests:**
+- `'expands on chevron click and persists to browser preferences'` - no longer persists
+- `'starts expanded when browser preferences have a stored preference'` - no longer reads from localStorage
+- `'collapses on second chevron click and stores false in browser preferences'` - no longer persists
+- `'passes autoExpandAbove props through to ToolBlocks in expanded mode'` - autoExpandAbove removed
+- `'migrates the legacy tool-strip key through the browser preferences helper'` - legacy migration removed
+
+**Modify tests:**
+- `'always shows collapsed view when showTools is false, even if localStorage says expanded'` - simplify to just `'always shows collapsed view when showTools is false'`
+
+**Add new tests:**
+- `'starts expanded when showTools is true'`
+- `'starts collapsed when showTools is false'`
+- `'strip toggle is session-only (not persisted to localStorage)'`
+- `'ToolBlocks start expanded when showTools is true'`
+- `'ToolBlocks start collapsed when showTools is false'`
+- `'individual ToolBlock toggles work independently'`
+
+### `test/unit/client/components/agent-chat/MessageBubble.test.tsx`
+
+**Modify tests:**
+- Remove `completedToolOffset` and `autoExpandAbove` from any test setup if present
+- Update tests that verify localStorage interaction to verify session-only behavior instead
+
+### `test/unit/lib/browser-preferences.test.ts` (if exists)
+
+**Remove tests:**
+- Any tests for `getToolStripExpandedPreference`, `setToolStripExpandedPreference`, `subscribeToolStripPreference`
+- Any tests for `toolStrip` field handling
+
+## Implementation Steps
+
+1. **browser-preferences.ts**: Remove tool strip persistence functions and types
+2. **ToolStrip.tsx**: Replace localStorage with local state, pass `showTools` to ToolBlocks
+3. **MessageBubble.tsx**: Remove unused props
+4. **Update tests**: Remove localStorage-related tests, add session-only behavior tests
+5. **Run full test suite**: `npm test`
+6. **Manual verification**: Test in browser
+
+## Commit Message
+
+```
+fix: make tool strip toggle session-only, controlled by showTools prop
+
+- Remove localStorage persistence for tool strip expanded state
+- ToolStrip now uses local useState initialized from showTools prop
+- ToolBlocks inherit initial expanded state from showTools
+- Remove autoExpandAbove/completedToolOffset props (no longer needed)
+- All toggle state is session-only, resets on page refresh
+```

src/components/agent-chat/AgentChatView.tsx

@@ -451,28 +451,7 @@ export default function AgentChatView({ tabId, paneId, paneContent, hidden }: Ag
   const timelineItems = useMemo(() => session?.timelineItems ?? [], [session?.timelineItems])
   const timelineBodies = session?.timelineBodies ?? {}
 
-  // Auto-expand: count completed tools across all messages, expand the most recent N
-  const RECENT_TOOLS_EXPANDED = 3
   const messages = useMemo(() => session?.messages ?? [], [session?.messages])
-  const { completedToolOffsets, autoExpandAbove } = useMemo(() => {
-    let totalCompletedTools = 0
-    const offsets: number[] = []
-    for (const msg of messages) {
-      offsets.push(totalCompletedTools)
-      for (const b of msg.content) {
-        if (b.type === 'tool_use' && b.id) {
-          const hasResult = msg.content.some(
-            r => r.type === 'tool_result' && r.tool_use_id === b.id
-          )
-          if (hasResult) totalCompletedTools++
-        }
-      }
-    }
-    return {
-      completedToolOffsets: offsets,
-      autoExpandAbove: Math.max(0, totalCompletedTools - RECENT_TOOLS_EXPANDED),
-    }
-  }, [messages])
 
   // Debounce streaming text to limit markdown re-parsing to ~20x/sec
   const debouncedStreamingText = useStreamDebounce(
@@ -661,8 +640,6 @@ export default function AgentChatView({ tabId, paneId, paneContent, hidden }: Ag
                     showThinking={paneContent.showThinking ?? defaultShowThinking}
                     showTools={paneContent.showTools ?? defaultShowTools}
                     showTimecodes={paneContent.showTimecodes ?? defaultShowTimecodes}
-                    completedToolOffset={completedToolOffsets[item.msgIndices[1]]}
-                    autoExpandAbove={autoExpandAbove}
                   />
                 </React.Fragment>
               )
@@ -679,8 +656,6 @@ export default function AgentChatView({ tabId, paneId, paneContent, hidden }: Ag
                 showThinking={paneContent.showThinking ?? defaultShowThinking}
                 showTools={paneContent.showTools ?? defaultShowTools}
                 showTimecodes={paneContent.showTimecodes ?? defaultShowTimecodes}
-                completedToolOffset={completedToolOffsets[item.msgIndex]}
-                autoExpandAbove={autoExpandAbove}
               />
             )
           })

src/components/agent-chat/MessageBubble.tsx

@@ -4,7 +4,6 @@ import type { ChatContentBlock } from '@/store/agentChatTypes'
 import { LazyMarkdown } from '@/components/markdown/LazyMarkdown'
 import ToolStrip, { type ToolPair } from './ToolStrip'
 
-/** Strip SDK-injected <system-reminder>...</system-reminder> tags from text. */
 function stripSystemReminders(text: string): string {
   return text.replace(/<system-reminder>[\s\S]*?<\/system-reminder>/g, '').trim()
 }
@@ -23,14 +22,7 @@ interface MessageBubbleProps {
   showThinking?: boolean
   showTools?: boolean
   showTimecodes?: boolean
-  /** When true, unpaired tool_use blocks show a spinner (they may still be running).
-   *  When false (default), unpaired tool_use blocks show as complete — their results
-   *  arrived in a later message. */
   isLastMessage?: boolean
-  /** Index offset for this message's completed tool blocks in the global sequence. */
-  completedToolOffset?: number
-  /** Completed tools at globalIndex >= this value get initialExpanded=true. */
-  autoExpandAbove?: number
 }
 
 function MessageBubble({
@@ -43,11 +35,8 @@ function MessageBubble({
   showTools = true,
   showTimecodes = false,
   isLastMessage = false,
-  completedToolOffset,
-  autoExpandAbove,
 }: MessageBubbleProps) {
   const resolvedSpeaker = speaker ?? role ?? 'assistant'
-  // Build a map of tool_use_id -> tool_result for pairing
   const resultMap = useMemo(() => {
     const map = new Map<string, ChatContentBlock>()
     for (const block of content) {
@@ -58,7 +47,6 @@ function MessageBubble({
     return map
   }, [content])
 
-  // Group content blocks into render groups: text, thinking, or contiguous tool runs.
   const groups = useMemo(() => {
     const result: RenderGroup[] = []
     let currentToolPairs: ToolPair[] | null = null
@@ -80,7 +68,6 @@ function MessageBubble({
           currentToolPairs = []
           toolStartIndex = i
         }
-        // Look up the matching tool_result
         const resultBlock = block.id ? resultMap.get(block.id) : undefined
         const rawResult = resultBlock
           ? (typeof resultBlock.content === 'string' ? resultBlock.content : JSON.stringify(resultBlock.content))
@@ -99,15 +86,12 @@ function MessageBubble({
       }
 
       if (block.type === 'tool_result') {
-        // If we're in a tool group, skip (already consumed via resultMap pairing above).
         if (currentToolPairs) continue
 
-        // If it has a matching tool_use elsewhere in this message, skip (already consumed)
         if (block.tool_use_id && content.some(b => b.type === 'tool_use' && b.id === block.tool_use_id)) {
           continue
         }
 
-        // Orphaned result: render as standalone tool strip
         const raw = typeof block.content === 'string'
           ? block.content
           : block.content != null ? JSON.stringify(block.content) : ''
@@ -127,7 +111,6 @@ function MessageBubble({
         continue
       }
 
-      // Non-tool block: flush any pending tool group
       flushTools()
 
       if (block.type === 'text' && block.text) {
@@ -137,16 +120,11 @@ function MessageBubble({
       }
     }
 
-    // Flush any trailing tool group
     flushTools()
 
     return result
   }, [content, resultMap, isLastMessage])
 
-  // Check if any blocks will be visible after applying toggle filters.
-  // Note: tool groups are unconditionally visible (collapsed summary always shows),
-  // so showTools is intentionally absent from the dependency array. Only thinking
-  // blocks are conditionally hidden via their toggle.
   const hasVisibleContent = useMemo(() => {
     return groups.some((group) => {
       if (group.kind === 'text') return true
@@ -156,19 +134,6 @@ function MessageBubble({
     })
   }, [groups, showThinking])
 
-  // Track completed tool offset across tool groups for auto-expand
-  const toolGroupOffsets = useMemo(() => {
-    const offsets: number[] = []
-    let offset = completedToolOffset ?? 0
-    for (const group of groups) {
-      if (group.kind === 'tools') {
-        offsets.push(offset)
-        offset += group.pairs.filter(p => p.status === 'complete').length
-      }
-    }
-    return offsets
-  }, [groups, completedToolOffset])
-
   if (!hasVisibleContent) return null
 
   return (
@@ -219,8 +184,6 @@ function MessageBubble({
               key={`tools-${group.startIndex}`}
               pairs={group.pairs}
               isStreaming={isStreaming}
-              completedToolOffset={toolGroupOffsets[group.toolGroupIndex]}
-              autoExpandAbove={autoExpandAbove}
               showTools={showTools}
             />
           )