refactor(mobile): one notifyTerminalFrame for the frame's layout

The frame's onLayout made four calls and held the classification itself. It now calls `notifyTerminalFrame({ width, height })`, and the session's terminal-webview hook keeps the one frame ref, notifies the height, subscribes the document held back for the first layout, and notifies a later width change. `handleTerminalFrameLayout` is named for what it does: `subscribeIntendedActiveTerminal`. The layout tests move to that hook. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
2026-10-01 00:02:10 +00:00 · 2026-09-28 02:43:51 -04:00
parent dc7e64a59c
commit 85d421963c
6 changed files with 89 additions and 104 deletions
@@ -32,7 +32,6 @@ export function MobileSessionActiveContent({
    setShowCreateTabDrawer,
    dictationMode,
    toastMessage,
-    terminalFrameRef,
    handleTerminalTap,
    browserScreencastSupported,
    showToast,
@@ -52,11 +51,9 @@ export function MobileSessionActiveContent({
    copyMarkdownLocalContent,
    discardMarkdownLocalContent,
    saveMarkdownTab,
-    notifyTerminalFrameHeight,
-    notifyTerminalFrameWidth,
    setTerminalWebViewRef,
    handleTerminalWebReady,
-    handleTerminalFrameLayout,
+    notifyTerminalFrame,
    notifyTerminalCellBoxChange,
    handleFileTap,
    handleNativeChatFileTap,
@@ -253,20 +250,7 @@ export function MobileSessionActiveContent({
      style={styles.contentFrame}
      onLayout={(e) => {
        const { width, height } = e.nativeEvent.layout
-        // Why: notify imperatively so dock settling re-fits the PTY without rerendering SessionScreen.
-        notifyTerminalFrameHeight(Math.round(height))
-        // Why: the page reports a hidden frame as 0x0; it keeps the box it was laid out at.
-        if (width <= 0) {
-          return
-        }
-        const previous = terminalFrameRef.current
-        terminalFrameRef.current = { width, height }
-        if (!previous) {
-          // Why: a ready document held back for its frame subscribes on the first layout.
-          handleTerminalFrameLayout()
-        } else if (width !== previous.width) {
-          notifyTerminalFrameWidth()
-        }
+        notifyTerminalFrame({ width, height })
      }}
    >
      {content}
@@ -98,14 +98,17 @@ const HOST_COMPONENT_NAMES = new Set([
 // width ref and `handleTerminalFrameLayout` subscribe a known box on layout (hooks 282, callbacks 79),
 // and the pane's `onCellBoxChange` goes to the viewport refit.
 // Again when one frame ref replaced the height ref, width ref and width state (hooks 280).
-const HEAD_MAIN_HOOK_SHA256 = '17366146730f1d910344ea4aad2e74738baea45797380c2b075e63effa1e7fcc'
+// Again when one `notifyTerminalFrame` took the frame's layout (hooks 281, callbacks 80).
+const HEAD_MAIN_HOOK_SHA256 = '004b011722b17ac82c96f0b3c8e303d39b2431a216424e9b86a1ee6a4896f23e'
 // Moved when the prompt-cancel flag became one structured-session host support object (main).
 // Re-recorded against the merged tree. Again when the frame-layout and cell-box-change callbacks
 // named the refs they read in their dependency lists (react-doctor).
 // Again when the frame's width and height became one `terminalFrameRef`.
-const HEAD_HOOK_BINDING_SHA256 = 'b23d3fb3f8219d1959645f8d4f22cf9414102e9e47abcd68eacec30f4656e7f3'
+// Again when the frame's layout moved into `notifyTerminalFrame`.
+const HEAD_HOOK_BINDING_SHA256 = 'c1bcb859202aaf8d612d3023cb4d895719da513879c61b01545a249a0bda662b'
+// Moved when `notifyTerminalFrame` joined and `handleTerminalFrameLayout` became `subscribeIntendedActiveTerminal`.
 const HEAD_CALLBACK_IDENTITY_SHA256 =
-  '19201fe156b3fd9d1bd55418c2c54e2d2319c652bfe55732b26f331c458f56c9'
+  '9c966301b373c11b27359ef1388c593b638c7f6831186d5c5321553504183e07'
 // Pins that no callback body in the route changed unnoticed. Body text, not behaviour: the sends
 // and repo reads inside them now name their `RpcOperation` instead of the raw `sendRequest` port.
 // Refreshed in step 6 for the gesture flush, whose `terminal.send` became `terminalInputSend` and
@@ -132,7 +135,8 @@ const HEAD_CALLBACK_IDENTITY_SHA256 =
 // Again when the subscribe sized its viewport inline instead of through a helper.
 // Again when the document took the hold rule and the subscribe stopped holding its grid.
 // Again when an init took one options object.
-const HEAD_CALLBACK_BODY_SHA256 = 'aae361e46d8bf59a4957dfcf9a1167c86af9314a998676235f73b44a3557d52e'
+// Again when the frame's layout became one `notifyTerminalFrame`.
+const HEAD_CALLBACK_BODY_SHA256 = '8f2ecfc86d6b50dfcf2248135c12a1a97c0b63a50176fc330781e805f388bbd3'
 // Refreshed for the startup effect: both `worktree.activate` sends became `worktreeActivate`, and
 // the sleeping-agent check reads that operation's verdict instead of the reply envelope. Refreshed
 // again when the reporter took the reply and interpreted it itself, retiring the hand-built
@@ -201,7 +205,8 @@ const HEAD_RUNTIME_STRING_SHA256 =
 // Moved again when the terminal frame kept its laid-out width unrounded, for every fit.
 // Again when the frame's onLayout wrote one frame ref and notified a new width imperatively.
 // Again when the frame's first laid-out layout alone subscribes a held-back document.
-const HEAD_HOST_JSX_SHA256 = '5e2d0ec5bdb838560d06d4b79c8b59d72d1f553394d1580e2250b352cda18401'
+// Again when the frame's onLayout made one `notifyTerminalFrame` call.
+const HEAD_HOST_JSX_SHA256 = '8ab32926fff2e610ab92bde8437283a11ad2328c6343cd862a1c00b2ed8ed5a6'
 const HEAD_LEAF_JSX_SHA256 = '62eb05c6e2ac0be6d553a141fc8aa1641fcb0c678777d5d539f490aab8648417'
 const HEAD_STYLE_REFERENCE_SHA256 =
  '56a005a1f65b30c11092e3422caef67810e1ec50f66fdd06471c370138b1eeb6'
@@ -599,10 +604,10 @@ describe('mobile session route extraction parity', () => {
    const contentBindings = CONTENT_COMPONENT_NAMES.flatMap(
      (name) => readHookFacts(name, definitions).bindings
    )
-    expect(main.hooks).toHaveLength(280)
+    expect(main.hooks).toHaveLength(281)
    expect(hash(main.hooks)).toBe(HEAD_MAIN_HOOK_SHA256)
    expect(hash(main.bindings)).toBe(HEAD_HOOK_BINDING_SHA256)
-    expect(main.callbacks).toHaveLength(79)
+    expect(main.callbacks).toHaveLength(80)
    expect(hash(main.callbacks)).toBe(HEAD_CALLBACK_IDENTITY_SHA256)
    expect(hash(main.callbackBodies)).toBe(HEAD_CALLBACK_BODY_SHA256)
    expect(main.effects).toHaveLength(24)
@@ -49,12 +49,7 @@ type Branch = 'loading' | 'pending' | 'terminal'

 function controller(
  branch: Branch | boolean,
-  notifyTerminalFrameHeight: (height: number) => void,
-  frame: {
-    terminalFrameRef?: { current: { width: number; height: number } | null }
-    notifyTerminalFrameWidth?: () => void
-    handleTerminalFrameLayout?: () => void
-  } = {}
+  notifyTerminalFrame: (frame: { width: number; height: number }) => void
 ): Controller {
  const shown = branch === true ? 'loading' : branch === false ? 'terminal' : branch
  const scope = {
@@ -63,10 +58,7 @@ function controller(
    isPendingTerminalRecoveryParked: false,
    showEmptyState: false,
    terminals: [],
-    terminalFrameRef: frame.terminalFrameRef ?? { current: null },
-    handleTerminalFrameLayout: frame.handleTerminalFrameLayout ?? (() => {}),
-    notifyTerminalFrameHeight,
-    notifyTerminalFrameWidth: frame.notifyTerminalFrameWidth ?? (() => {}),
+    notifyTerminalFrame,
    dictation: { isRecording: false },
    nativeChatSendError: { message: null, clear: () => {} }
  }
@@ -77,8 +69,8 @@ function controller(
 describe('the terminal frame on the page', () => {
  it('reports its height when the session opens from the loading state', () => {
    const heights: number[] = []
-    const notify = (height: number): void => {
-      heights.push(height)
+    const notify = (frame: { height: number }): void => {
+      heights.push(frame.height)
    }
    let renderer: ReturnType<typeof create> | undefined
    act(() => {
@@ -97,8 +89,8 @@ describe('the terminal frame on the page', () => {
  it('stays one mounted frame across loading, a pending terminal and the terminal', () => {
    // A frame that remounts per branch reports again on every return; one that stays reports once.
    const heights: number[] = []
-    const notify = (height: number): void => {
-      heights.push(height)
+    const notify = (frame: { height: number }): void => {
+      heights.push(frame.height)
    }
    let renderer: ReturnType<typeof create> | undefined
    const show = (branch: Branch) => {
@@ -119,57 +111,4 @@ describe('the terminal frame on the page', () => {
    show('terminal')
    expect(heights).toEqual([FRAME.height])
  })
-
-  it('keeps one frame, notifies a new width, and keeps the box through a hidden 0x0 layout', () => {
-    const terminalFrameRef: { current: { width: number; height: number } | null } = {
-      current: null
-    }
-    const notifyTerminalFrameWidth = vi.fn()
-    let renderer: ReturnType<typeof create> | undefined
-    act(() => {
-      renderer = create(
-        createElement(MobileSessionActiveContent, {
-          controller: controller(false, () => {}, { terminalFrameRef, notifyTerminalFrameWidth })
-        })
-      )
-    })
-    expect(terminalFrameRef.current).toEqual(FRAME)
-    const layOut = (width: number, height: number) =>
-      act(() => {
-        renderer!.root
-          .findAll((node) => typeof node.props.onLayout === 'function')[0]!
-          .props.onLayout({ nativeEvent: { layout: { width, height } } })
-      })
-    layOut(FRAME.width, 560)
-    expect(notifyTerminalFrameWidth).not.toHaveBeenCalled()
-    layOut(0, 0)
-    expect(terminalFrameRef.current).toEqual({ width: FRAME.width, height: 560 })
-    layOut(360.5, 560)
-    expect(terminalFrameRef.current).toEqual({ width: 360.5, height: 560 })
-    expect(notifyTerminalFrameWidth).toHaveBeenCalledTimes(1)
-  })
-
-  it('subscribes on the first laid-out frame only, not on every layout after it', () => {
-    const terminalFrameRef: { current: { width: number; height: number } | null } = {
-      current: null
-    }
-    const handleTerminalFrameLayout = vi.fn()
-    let renderer: ReturnType<typeof create> | undefined
-    act(() => {
-      renderer = create(
-        createElement(MobileSessionActiveContent, {
-          controller: controller(false, () => {}, { terminalFrameRef, handleTerminalFrameLayout })
-        })
-      )
-    })
-    const layOut = (width: number, height: number) =>
-      act(() => {
-        renderer!.root
-          .findAll((node) => typeof node.props.onLayout === 'function')[0]!
-          .props.onLayout({ nativeEvent: { layout: { width, height } } })
-      })
-    layOut(FRAME.width, 560)
-    layOut(360, 560)
-    expect(handleTerminalFrameLayout).toHaveBeenCalledTimes(1)
-  })
 })
@@ -97,10 +97,13 @@ function subscriptionHarness(opts: {
    markdownDocs: new Map(),
    fileDocs: new Map(),
    readMarkdownTab: vi.fn(),
-    readFileTab: vi.fn()
+    readFileTab: vi.fn(),
+    notifyTerminalFrameHeight: vi.fn(),
+    notifyTerminalFrameWidth: vi.fn()
  }
  let subscribe: ((handle: string) => void) | undefined
  let webReady: ((handle: string) => void) | undefined
+  let notifyFrame: ((frame: { width: number; height: number }) => void) | undefined
  function Probe() {
    // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the hook destructures only the fields built above.
    const scope = fields as unknown as MobileSessionTerminalSubscriptionFoundationModel
@@ -108,7 +111,9 @@ function subscriptionHarness(opts: {
    const withSubscribe = { ...fields, subscribeToTerminal: subscribe }
    // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the web-ready handler reads only the fields built above.
    const webviewScope = withSubscribe as unknown as MobileSessionTabSwitchingModel
-    webReady = useMobileSessionTerminalWebview(webviewScope).handleTerminalWebReady
+    const webview = useMobileSessionTerminalWebview(webviewScope)
+    webReady = webview.handleTerminalWebReady
+    notifyFrame = webview.notifyTerminalFrame
    return null
  }
  act(() => {
@@ -127,9 +132,10 @@ function subscriptionHarness(opts: {
    documentReady: () => {
      act(() => webReady!(HANDLE))
    },
-    layOutFrame: (width: number) => {
-      terminalFrameRef.current = { width, height: 751 }
-    }
+    fields,
+    terminalFrameRef,
+    // The frame's onLayout.
+    layOutFrame: (width: number, height = 751) => act(() => notifyFrame!({ width, height }))
  }
 }

@@ -184,11 +190,31 @@ describe('a terminal first subscribe', () => {
    const harness = subscriptionHarness({ fit: null, webReady: true, frameWidth: 0 })
    harness.subscribe()
    expect(harness.order).toEqual([])
+    // The frame's first layout subscribes the document held back for it.
    harness.layOutFrame(427)
-    harness.subscribe()
    expect(harness.order).toEqual(['subscribe null'])
  })

+  it('keeps one frame, notifies a new width, and subscribes on the first layout only', () => {
+    const harness = subscriptionHarness({ fit: null, webReady: true, frameWidth: 0 })
+    const { notifyTerminalFrameHeight, notifyTerminalFrameWidth } = harness.fields
+    harness.layOutFrame(0, 0)
+    expect(harness.terminalFrameRef.current).toBeNull()
+    harness.layOutFrame(427, 751)
+    harness.layOutFrame(427, 700)
+    expect(harness.order).toEqual(['subscribe null'])
+    expect(notifyTerminalFrameWidth).not.toHaveBeenCalled()
+    // A hidden 0x0 layout keeps the box it was laid out at.
+    harness.layOutFrame(0, 0)
+    expect(harness.terminalFrameRef.current).toEqual({ width: 427, height: 700 })
+    harness.layOutFrame(360.5, 700)
+    expect(harness.terminalFrameRef.current).toEqual({ width: 360.5, height: 700 })
+    expect(notifyTerminalFrameWidth).toHaveBeenCalledTimes(1)
+    expect(notifyTerminalFrameHeight.mock.calls.map(([height]) => height)).toEqual([
+      0, 751, 700, 0, 700
+    ])
+  })
+
  it('goes without dims when no cell box was reported, and the fit pass resubscribes once', async () => {
    const harness = subscriptionHarness({ fit: null, webReady: true })
    harness.subscribe()
@@ -1,5 +1,6 @@
 import { useEffect, useCallback } from 'react'
 import type { TerminalWebViewHandle } from '../terminal/terminal-webview-contract'
+import type { TerminalFrame } from '../terminal/terminal-webview-messages'
 import type { MobileSessionTabSwitchingModel } from './use-mobile-session-tab-switching'

 export function useMobileSessionTerminalWebview(scope: MobileSessionTabSwitchingModel) {
@@ -21,7 +22,10 @@ export function useMobileSessionTerminalWebview(scope: MobileSessionTabSwitching
    subscribeToTerminal,
    nativeChatStream,
    readMarkdownTab,
-    readFileTab
+    readFileTab,
+    terminalFrameRef,
+    notifyTerminalFrameHeight,
+    notifyTerminalFrameWidth
  } = scope
  // Why: only store the ref; subscribe on web-ready to avoid the blank-terminal race (init queued before xterm.js loaded).
  const setTerminalWebViewRef = useCallback((handle: string, ref: TerminalWebViewHandle | null) => {
@@ -70,14 +74,39 @@ export function useMobileSessionTerminalWebview(scope: MobileSessionTabSwitching
    [nativeChatStream, subscribeToTerminal, unsubscribeTerminal]
  )

-  /** The frame has a size: a ready document held back for it subscribes now. */
-  const handleTerminalFrameLayout = useCallback(() => {
+  const subscribeIntendedActiveTerminal = useCallback(() => {
    const handle = pendingActiveTerminalHandleRef.current ?? activeHandleRef.current
    if (handle && !terminalUnsubsRef.current.has(handle)) {
      subscribeToTerminal(handle)
    }
  }, [activeHandleRef, pendingActiveTerminalHandleRef, subscribeToTerminal, terminalUnsubsRef])

+  /** The terminal frame React Native laid out: kept in the one frame ref, then what it changed. */
+  const notifyTerminalFrame = useCallback(
+    (frame: TerminalFrame) => {
+      // Why: notify height imperatively so dock settling re-fits the PTY without rerendering SessionScreen.
+      notifyTerminalFrameHeight(Math.round(frame.height))
+      // Why: the page reports a hidden frame as 0x0; it keeps the box it was laid out at.
+      if (frame.width <= 0) {
+        return
+      }
+      const previous = terminalFrameRef.current
+      terminalFrameRef.current = frame
+      if (!previous) {
+        // Why: a ready document held back for its frame subscribes on the first layout.
+        subscribeIntendedActiveTerminal()
+      } else if (frame.width !== previous.width) {
+        notifyTerminalFrameWidth()
+      }
+    },
+    [
+      notifyTerminalFrameHeight,
+      notifyTerminalFrameWidth,
+      subscribeIntendedActiveTerminal,
+      terminalFrameRef
+    ]
+  )
+
  useEffect(() => {
    if (activeSessionTab?.type !== 'markdown') {
      return
@@ -100,7 +129,7 @@ export function useMobileSessionTerminalWebview(scope: MobileSessionTabSwitching
  return {
    setTerminalWebViewRef,
    handleTerminalWebReady,
-    handleTerminalFrameLayout
+    notifyTerminalFrame
  }
 }

@@ -15,7 +15,8 @@ import {
 const hookSource = readFileSync(new URL('./terminal-viewport-refit.ts', import.meta.url), 'utf8')
 const sessionSource = [
  readMobileSessionRouteSource('../session/use-mobile-session-keyboard-state.ts'),
-  readMobileSessionRouteSource('../session/MobileSessionActiveContent.tsx')
+  readMobileSessionRouteSource('../session/MobileSessionActiveContent.tsx'),
+  readMobileSessionRouteSource('../session/use-mobile-session-terminal-webview.ts')
 ].join('\n')

 describe('terminal viewport refit', () => {
@@ -152,7 +153,8 @@ describe('terminal viewport refit', () => {
    expect(sessionSource).toContain('tabStripVisible: terminals.length > 1')
    expect(sessionSource).toContain('textScale: terminalTextScale')
    expect(sessionSource).toContain('connState,')
-    expect(sessionSource).toContain('notifyTerminalFrameHeight(Math.round(height))')
+    expect(sessionSource).toContain('notifyTerminalFrame({ width, height })')
+    expect(sessionSource).toContain('notifyTerminalFrameHeight(Math.round(frame.height))')
    expect(sessionSource).toContain('notifyTerminalFrameWidth()')
    // One seam for both facts: on the page they come apart, because the shell shortens the WebView
    // and the keyboard covers nothing the screen has to lift for.