Skip to content

fix: Subagent list: focus falls to the body when the region unmounts on the way back to main - #8638

Merged
usirin merged 2 commits into
mainfrom
build/8470-composer-focus-on-return-973445a9
Sep 8, 2026
Merged

fix: Subagent list: focus falls to the body when the region unmounts on the way back to main#8638
usirin merged 2 commits into
mainfrom
build/8470-composer-focus-on-return-973445a9

Conversation

@usirin

@usirin usirin commented Sep 8, 2026

Copy link
Copy Markdown
Member

Leaving a subagent view now puts DOM focus on the composer, every time.

Before this, SubagentList asked for its own first line on the way back. When the only worker had
finished, main had no rows, the whole region unmounted and there was no line to focus — DOM focus
fell to the document body, by Escape and by the back button alike.

The founder ruled the Claude Code model on #8470 (comment 5589657679): nothing auto-returns, and on
leaving, focus lands on the composer. So the destination moved out of the list and into the window.

  • SubagentList no longer requests focus for the way back; pickMain was a one-line wrapper over
    onMain and is gone with it. The "more" row still names its own destination, unchanged.
  • ChatWindow.showMain flags the leave and a layout effect places focus once the commit has landed.
    It runs after the render that re-enables the composer (The Tuval composer says nothing about where a prompt goes while a subagent view is open #8466's disabled={viewing !== null}), so
    the field takes the focus rather than refusing it while disabled.
  • AgentChatInput gained an optional ref to its prompt field. The composer is the only control a
    host can be asked to focus, so the ref is the textarea and not a handle.

Four cases in subagent-navigator.unit.test.tsx: the last finished worker by Escape and by the back
button (the region gone, composer focused and enabled), and the two-workers-remain path where the
navigator survives and focus still leaves it. The finished-worker setup asserts Q9 on the way — the
view stays open under the worker that finished, until the operator leaves.

Fixes #8470

Deviations

  • Pre-existing test or fixture changedSaid: Subagent list: focus falls to the body when the region unmounts on the way back to main #8470 names the focus destination on return.
    Did: rewrote two standing cases in subagent-navigator.unit.test.tsx that asserted focus
    landed back on the list's first row. Why: those cases pinned the behaviour the founder's
    ruling replaces; leaving them would have been a red on the ruled outcome.
    Disposition: stated here.
  • Out-of-scope changeSaid: the issue's pointers are SubagentList.tsx and
    ChatWindow.tsx. Did: also added an optional ref prop to AgentChatInput in
    packages/design. Why: the field's id is a useId value and the component exposed no handle,
    so the window had no typed way to reach the textarea; a class-name query from the consumer would
    couple Tuval to a design-package internal. Renders nothing new and changes no default.
    Disposition: stated here.

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

🚀 Preview deployed

  • web — Stage pr-8638 torn down.

@usirin

usirin commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

review-code: PASS @ 1302421 content:f0ad1036ac89 — merge-ready

Text gate over the four changed files at this head. CI at the head is green: 45 runs, 41 success, 4 skipped, 28 of 40 repo-authored workflows produced a run. A routed review-ui row is raised on this PR; that namespace is not judged here.

Criteria

  1. Escape and the back button from the last finished worker land on the composer, never the body. Met. The new describe("leaving the last finished worker") opens one worker, flips it to finished under its own view, and exercises both exits. Each case asserts the main transcript is back, document.querySelector(".tuval-chat-subagents") is null (the region really did go), the composer is enabled, and document.activeElement is the composer.

  2. Returning while other workers remain keeps the navigator and still lands focus on the composer. Met. Both standing cases were rewritten against twoRunning(): the back-row case asserts rows() still has length 2 and focus is on the composer, and the Escape case asserts the same destination. So the destination is the composer whether or not the list outlives the view.

  3. Nothing auto-returns; the Tuval chat shows a running subagent as a live group head: name, last line, elapsed, tokens #8384 Q9 retained view is unchanged and asserted. Met. No auto-return was added on either side; SubagentList only dropped its own focus request for the way back. The openFinished setup asserts the retained view directly: after the worker commits finished, the subagent transcript log still resolves and the composer is still disabled. That assertion sits in the setup, so both new cases carry it.

  4. Flag-off / no-list chord behaviour unchanged. Met. navigatorRef and the useForwardedKey handler are untouched, the no-list chord block is untouched, and the whole apps/tuval/src/shell/chat suite runs green at this head: 27 files, 372 tests.

  5. Composes with The Tuval composer says nothing about where a prompt goes while a subagent view is open #8466: the composer is re-enabled before focus moves. Met, and the ordering holds for the reason the code states plus one it does not. showMain sets leaving and commits viewMain; the layout effect guards if (!leaving || viewing !== null) return. The composer's disabled={viewing !== null} reads that same viewing, so the effect can only fire on a render where the field is already enabled in the DOM, layout effects running after the commit.
    The inline note claims both writes land in one render; even where they do not, the guard holds the flag and the effect re-fires when viewing later becomes null, so the ordering is right on both paths rather than resting on the batching claim. The tests pin it by asserting composer().disabled === false beside the focus assertion.

The rest of the sweep

  • pickMain inlined. Correct. It was a one-line wrapper whose only extra work was setRequest({kind: "first"}), exactly the request the ruling removes. Both call sites, the way-back row's onPick and the Escape branch of the key handler, now take onMain directly, and the dependency list moved with them. The "more" row's own focus request is untouched: expand still names its destination and the surviving layout effect still places it.

  • The cross-package ref on packages/design's AgentChatInput. Minimal and default-preserving. It is optional, forwarded to the prompt Textarea and nowhere else, adds no element, and every consumer that omits it renders as before: 25 AgentChatInput.test.tsx tests green. React 19's ref-as-prop is why no forwardRef is needed. The disclosed alternative, a class-name query from Tuval, would have coupled the app to a design-package internal, and the field's id is a useId value, so there was no other typed way in. Typing it as the textarea rather than an imperative handle is the right call: the field is the only control a host can be asked to focus.

  • Two non-blocking observations, neither a criterion miss. First, composerRef.current?.focus() swallows a null ref, and the failure it swallows is the very bug being fixed, so a future regression where the field does not mount would go quiet; the new cases cover the positive path only. Second, leaving stays true if a showMain commit never resolves to a null view, so a later unrelated transition to a null view would move focus to the composer. No path today produces that, and the outcome would be benign.

Deviations

Both disclosed entries check out against the diff, and nothing undisclosed was visible to this gate.

  • Pre-existing tests changed: two standing cases rewritten off the assertion that focus landed back on the list's first row. Accurate; review deviations flags the same removed assertion at line 167. It pinned the behaviour the founder's 2026-09-08 ruling replaces, so leaving it would have red on the ruled outcome.
  • Out-of-scope: the optional ref on AgentChatInput. Accurate and complete. Renders nothing new and changes no default.

Verdict-written: 2026-09-08T21:48:20Z

@usirin

usirin commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

routed-elsewhere: review-ui @ 1302421 — Tuval surface; no admissible renderer until #7306 lands (founder ruling 2026-09-06). Text review PASS + builder hand-verification stand in.

Tuval surface; no admissible renderer until #7306 lands (founder ruling 2026-09-06). Text review PASS at 1302421 (review-code) and the builder's hand-verification stand in: 27 files / 372 tests under apps/tuval/src/shell/chat green plus AgentChatInput's 25; focus destination asserted on document.activeElement for Escape and the back button; the founder ruled the Claude Code model on 2026-09-08. Sunset list entry 53 on #7306.

@usirin
usirin added this pull request to the merge queue Sep 8, 2026
Merged via the queue into main with commit ba04364 Sep 8, 2026
45 checks passed
@usirin
usirin deleted the build/8470-composer-focus-on-return-973445a9 branch September 8, 2026 21:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Subagent list: focus falls to the body when the region unmounts on the way back to main

1 participant