Skip to content

fix: make active panes follow DOM focus - #882

Merged
mariusvniekerk merged 25 commits into
mainfrom
t3code/fix-pgdown-textarea-scrolling
Aug 13, 2026
Merged

fix: make active panes follow DOM focus#882
mariusvniekerk merged 25 commits into
mainfrom
t3code/fix-pgdown-textarea-scrolling

Conversation

@mariusvniekerk

@mariusvniekerk mariusvniekerk commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Split panes could show an active border on one surface while Page Up or Page Down routed to another. Mousewheel was incorrectly treated as focus, so the UI could claim a pane was active even though browser focus had not moved.

This change makes actual DOM focus the only live source of active-pane state across PR, Issue, Activity, and Workspaces layouts. Wheel scrolling stays local and does not change focus. Deliberate terminal focus still changes the active pane because it moves DOM focus.

Narrow layouts use the exact leaf identity rendered on screen. Responsive flatten and unflatten transitions preserve that renderer-owned identity and reclaim focus only when the browser strands it on the document body. Same-leaf tab replacement uses the same rule.

Workflow focus ownership continues to update while workspace mutations are blocked. Parking a workspace host or removing focused Workflow content clears stale ownership. Readiness changes and Workflow content replacement run bounded post-render recovery independently of reactive reruns, and restore the Workspace root only when no deliberate destination received focus. Connected pooled terminals keep their own restoration path. Successful command-palette promotion queues focus for the exact promoted session. Activity commit diffs use the same live focus ownership as other review panes.

A dedicated Files route retains global diff shortcuts only while no pane, external dock, or modal has live ownership. Once one of those scopes claims the keyboard, the Files diff stops consuming shortcuts.

Workspace sidebar shortcuts yield to active modal frames. If focused Workspace Details or bottom Terminal disappears while the host stays visible, focus returns to the Workspace root only when the browser leaves focus on the document body. Resetting pane arrangement preserves a live external dock claim; DOM focusout, dock unmount, or renderer teardown releases it.

Validation
  • Workspace terminal component suite: 132 passed.
  • Formatting, kit UI checks, Svelte checks, Effect diagnostics, and repository lint passed; only existing lint warnings remain.
  • Svelte autofixer reported no issues.
  • Full-stack Chromium terminal focus and pane-move suite: 3 passed, including promotion, input, and focus recovery.
  • Context structural validation passed.
  • Exact-head CI passed every applicable build, lint, unit, browser, mock e2e, full-stack e2e, documentation, and RoboRev e2e job in Chromium and Firefox.

generated by a clanker

mariusvniekerk and others added 7 commits August 10, 2026 11:15
Split review panes need one predictable owner for global paging. Focus alone is insufficient because blank pane surfaces do not receive focus, and an invisible owner makes keyboard behavior hard to anticipate.

Record pointer, wheel, and focus activation together with a restrained active-border treatment before changing the shared pane renderer.

Validation: context-sync structural check passed.

Generated with Codex (gpt-5.6-sol)
Co-authored-by: Codex <noreply@openai.com>
Split panes could change route ownership only when a focusable descendant received focus. Clicking or wheeling a blank pane left keyboard ownership on a different surface, with no visual indication of where global commands would land.

Unify focus, pointer, and wheel activation in the shared pane renderer and show the current owner with a restrained inset accent border. Wheel events remain native and local to their target.

Validation: TabbedPanelTree unit suite passed; Svelte analyzer reported no new issues.

Generated with Codex (gpt-5.6-sol)
Co-authored-by: Codex <noreply@openai.com>
A visible files pane kept its window-level Page Up and Page Down listener active while the conversation pane owned the interaction. That made page keys move the diff after a maintainer clicked or wheeled elsewhere.

Thread route-derived ownership into pull request diff layouts, retain local paging for directly targeted diff input, and prefer live pane focus while route navigation settles. Apply the same contract to PR and Activity detail surfaces.

Validation: full frontend unit suite passed with 3,894 tests and 2 skips; frontend checks and Effect diagnostics completed with zero errors; the PR detail pane Playwright suite passed 6 tests across Chromium and Firefox.

Generated with Codex (gpt-5.6-sol)
Co-authored-by: Codex <noreply@openai.com>
Issue detail panes had no default input owner, while standalone Workspaces panes did not promote pointer, focus, or wheel interaction into the shared ownership model. That left the active border absent or stale outside the PR and Activity surfaces.

Apply the same single-owner contract to every detail pane and standalone workflow tree so keyboard routing and the visible one-pixel indicator stay predictable wherever panes are split.

Validation: full frontend unit suite passed with 3,896 tests and 2 skips; frontend checks and Effect diagnostics completed with zero errors; the complete workspace-sidebar and PR detail pane Playwright files passed 169 tests with 1 skip across Chromium and Firefox.

Generated with Codex (gpt-5.6-sol)
Co-authored-by: Codex <noreply@openai.com>
Live pointer and wheel ownership could diverge from route-derived keyboard handling, while stale or hidden pane state could leave no visible owner. Nested workspace regions also painted their own focus marker when the enclosing PR or issue pane was inactive.

Keep keyboard dispatch and the one-pixel ownership treatment on the same validated live pane, and model Workflow, Details, and bottom Terminal as sibling owners within a workspace. This preserves one coherent input target across PR, Issue, Activity, and Workspaces surfaces.

Validation: frontend-check passed; full unit suite passed with 3,900 tests and 2 skips; mock Playwright passed 169 tests with 1 skip; full-stack Playwright passed 76 tests across Chromium and Firefox.

Generated with Codex (gpt-5.6-sol)
Co-authored-by: Codex <noreply@openai.com>
Files Changed could paint its toolbar above the active-pane outline and layer the selected-tab accent onto the same edge, producing a broken double border. Nested workspaces also left the enclosing detail leaf highlighted alongside the workflow or terminal that actually owned input.

Keep pane ownership visually singular at every depth so the border identifies the surface that receives keyboard and wheel input, without changing layout or control focus styling.

Validation: frontend-check passed; TabbedPanelTree and DiffToolbar unit tests passed (26 tests); PR detail pane Playwright passed across Chromium and Firefox (6 tests); full-stack nested issue/workspace/terminal ownership regression passed in Chromium; live Chrome inspection confirmed the corrected strips.

Generated with Codex (gpt-5.6-sol)
Co-authored-by: Codex <noreply@openai.com>
The rendered pane tree and hosted workspace could disagree after fallback, zoom, or container retirement because nested regions reread persisted focus and the surface dock had no way to claim ownership. That also left diff-local paging and workspace shortcuts active for a different surface.

Publish the renderer-validated owner through the pane report and model an external dock claim that ordinary pane activation revokes. This keeps visual ownership and keyboard dispatch singular even when the workspace container is no longer in the pane tree.

Validation: frontend-check passed; full frontend unit suite passed with 3,903 tests and 2 skips; PR detail Playwright passed 6 tests across Chromium and Firefox; focused full-stack issue ownership and PR row-only external-dock regressions passed in Chromium.

Generated with Codex (gpt-5.6-sol)
Co-authored-by: Codex <noreply@openai.com>
@roborev-ci

roborev-ci Bot commented Aug 11, 2026

Copy link
Copy Markdown

roborev: Combined Review (b4d0423)

Code review found two medium-severity input-ownership issues; no high or critical findings.

Medium

  • frontend/src/lib/components/shared/DetailPaneLayout.svelte:253focusPane deduplicates before calling layout.noteFocused. If an external terminal dock claims ownership after a pane was last reported, clicking or wheeling that same pane cannot revoke the external claim, leaving its active border visible and global paging disabled. Always call layout.noteFocused(tabKey) and deduplicate only the host callback.

  • frontend/src/lib/components/terminal/WorkspaceTerminalView.svelte:1214 — External ownership is cleared only while surfaceLayout remains non-null. When an inline claim ends and the host is parked, the previous surface store retains externalInputActive, so the next PR or issue can render without an active pane and with paging disabled after the external dock disappears. Release external ownership from the previous layout whenever surfaceLayout changes or the hosted view leaves that surface.


Reviewers: 2 done | Synthesis: codex, 11s | Total: 10m28s

External dock ownership could survive a return to the same pane or the hosted workspace leaving its surface. Agent terminals could also consume wheel input before the dock claimed ownership, so the visible border and keyboard target could disagree.

Transfer ownership before deduplicating route callbacks, release external claims with their surface lifecycle, and observe terminal wheel input during capture without moving DOM focus. A browser-focused diff now suppresses native page keys whenever it is not the canonical owner.

Generated with Codex (gpt-5.6-sol)
Co-authored-by: Codex <noreply@openai.com>
@roborev-ci

roborev-ci Bot commented Aug 11, 2026

Copy link
Copy Markdown

roborev: Combined Review (3bb3c08)

Two medium-severity input-ownership gaps remain in the frontend pane handling.

Medium

  • frontend/src/lib/components/terminal/WorkspaceTerminalView.svelte:4334 — Details ownership is only visual. WorkspaceRightSidebar keeps diff keyboard handlers enabled when Workflow or Terminal owns input, so PageUp/PageDown and global diff shortcuts can still affect the inactive Details pane. Pass the Details ownership state into WorkspaceRightSidebar and gate nested diff keyboardActive/pageKeyboardActive handlers with it.

  • frontend/src/lib/components/shared/TabbedPanelTree.svelte:681 — Pane activation only listens on the body, and wheel activation occurs during bubbling. Focusing tab/header controls does not activate the leaf, while connected agent terminals can stop wheel propagation, leaving the previous pane as visible and keyboard owner. Handle focus and pointer activation across the entire leaf and capture wheel activation before children can stop propagation, resolving ownership to the leaf’s active tab.


Reviewers: 2 done | Synthesis: codex, 11s | Total: 7m46s

Pointer and wheel-derived ownership let the active border and pane-scoped PageDown routing disagree with actual browser focus. An unfocused Files Changed pane could therefore consume page keys, and wheel input could appear to transfer focus.

Make DOM focus the sole live signal. Keep remembered pane state only for restoration and command targeting, so wheel input remains focus-neutral while deliberate terminal focus still updates the active pane.

Generated with Codex (gpt-5.6-sol)
Co-authored-by: Codex <noreply@openai.com>
@mariusvniekerk mariusvniekerk changed the title fix: keep input ownership visible across panes fix: make active panes follow DOM focus Aug 11, 2026
@roborev-ci

roborev-ci Bot commented Aug 11, 2026

Copy link
Copy Markdown

roborev: Combined Review (8e94b85)

The focus-ownership update has three medium-severity gaps affecting narrow layouts and keyboard routing.

Medium

  • frontend/src/lib/components/shared/DetailPaneLayout.svelte:263 — Flattened layouts persist the original leaf ID, but render the synthetic tabbed-panel-flattened ID. As a result, activeInputTabKey stays null, preventing active-pane styling and pane-scoped paging in narrow layouts. Track the focused tab directly or resolve focus against activeTree, and add narrow-layout behavioral coverage.

  • frontend/src/lib/views/ActivityFeedView.svelte:550 — The branch-commit pane does not forward inputActive to CommitDiffPanel, so DiffView retains its global keyboard handler. Page keys can scroll the commit diff after focus returns to the activity feed. Propagate keyboard activity through CommitDiffPanel to DiffView and cover this in the Activity end-to-end flow.

  • frontend/src/lib/components/shared/DetailPaneLayout.svelte:144 — Focus ownership is stored only by leaf ID. Replacing the selected tab within the same leaf can unmount the focused pane without a reliable focusout, silently transferring keyboard ownership while DOM focus is on the document body. Revalidate ownership against document.activeElement after visibility or active-tab changes.


Reviewers: 2 done | Synthesis: codex, 13s | Total: 5m53s

@mariusvniekerk mariusvniekerk self-assigned this Aug 11, 2026
Dedicated Files routes load with focus on the document body, so focus-only ownership disabled Page Up, Page Down, and file navigation despite the accepted route contract.\n\nKeep the route fallback only until a pane or external dock claims live focus. This preserves focus-only active styling while preventing inactive diff shortcuts from taking input.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@roborev-ci

roborev-ci Bot commented Aug 11, 2026

Copy link
Copy Markdown

roborev: Combined Review (18adb1e)

Focus ownership has two medium-severity correctness issues in DetailPaneLayout; no security issues were found.

Medium

  • Synthetic flattened leaf cannot retain focus ownershipfrontend/src/lib/components/shared/DetailPaneLayout.svelte:263
    focusPane stores the persisted leaf ID, while flattened layouts render a synthetic tabbed-panel-flattened leaf. Because the IDs never match, focus is immediately cleared, so narrow layouts never report an active pane. This disables focus-local paging and nested workspace ownership.
    Fix: Resolve the focused ID from renderedLeaves, including the synthetic flattened leaf, and add flattened-width focus coverage.

  • Switching tabs can leave stale focus ownershipfrontend/src/lib/components/shared/DetailPaneLayout.svelte:150
    Focus is cleared only when the entire leaf disappears. If routing activates another tab in the same leaf, the focused diff subtree can be removed without firing focusout, leaving the replacement tab incorrectly styled and assigned ownership.
    Fix: After tab/tree changes, verify that document.activeElement remains inside the recorded rendered leaf; otherwise, clear focus ownership.


Reviewers: 2 done | Synthesis: codex, 12s | Total: 8m49s

The Files pane can share its leaf with another tab, so a role-only locator failed before the keyboard-routing assertions ran. Name the Files changed tab explicitly so the cross-browser regression reaches the behavior it is meant to verify.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@roborev-ci

roborev-ci Bot commented Aug 11, 2026

Copy link
Copy Markdown

roborev: Combined Review (ff24b4d)

Keyboard focus routing is incomplete in the Activity commit pane, causing shortcuts to target an inactive pane.

Medium

  • frontend/src/lib/views/ActivityFeedView.svelte:550 — The commit diff ignores inputActive, so DiffView defaults to globally active keyboard handling. PageUp/PageDown and diff shortcuts can still affect the commit pane while focus remains in the Activity rail, despite no active-pane border.
    • Fix: Thread inputActive through CommitDiffPanel to DiffView.keyboardActive, and add Activity e2e coverage for branch-commit focus routing.

Reviewers: 2 done | Synthesis: codex, 13s | Total: 6m53s

Narrow layouts render one synthetic leaf, so recovering focus ownership from the persisted tree could leave the visible pane without keyboard control. A same-leaf tab replacement could also strand focus on the document body, while Activity commit diffs kept global shortcuts whenever they were visible.

Use renderer-owned leaf identity, reclaim only focus that actually falls to the document body, and route Activity commit shortcuts through the same live inputActive contract as other panes. This preserves one keyboard owner without stealing focus from a clicked tab or another control.

Review findings fixed:
- Narrow-layout rendered leaf identity: #882 (comment)
- Activity commit diff ownership: #882 (comment) and #882 (comment)
- Same-leaf tab replacement ownership: #882 (comment) and #882 (comment)

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@roborev-ci

roborev-ci Bot commented Aug 11, 2026

Copy link
Copy Markdown

roborev: Combined Review (461dc2a)

Focus ownership improvements have two medium-severity stale-state issues that can misroute shortcuts and active styling.

Medium

  • frontend/src/lib/components/terminal/WorkspaceTerminalView.svelte:691workspaceInputRegion is not reconciled when focused workflow content disappears or the hosted workspace is parked. DOM removal may not emit focusout, leaving a stale region active; a replacement pane can then receive the active border and workspace shortcuts even though focus is on document.body. After visibility changes, verify that document.activeElement remains within the recorded region, clear stale ownership, and reclaim body focus where appropriate. Add an e2e test covering session disappearance or host parking.

  • frontend/src/lib/components/terminal/WorkflowSplitTree.svelte:165 — Focus reporting is suppressed while disabled is true, although disabled blocks mutations rather than focus. During events such as externally initiated workspace deletion, focus can move to another leaf while the active border remains on the previous leaf. Always forward onFocusPane, retaining disabled checks only for selection and mutation handlers, or add a separate focus-only ownership callback.


Reviewers: 2 done | Synthesis: codex, 11s | Total: 7m48s

Responsive flattening, parked workspace hosts, and disappearing sessions could remove the focused DOM node without leaving a current keyboard owner. Blocked workspace actions could also suppress legitimate focus changes, while the dedicated Files fallback could keep handling keys beneath a modal.

Keep focus ownership tied to renderer-provided leaf identity, let focus updates pass through disabled Workflow trees, and give modal frames precedence over Files shortcuts. Restore focus after DOM replacement only when the browser leaves it on the document body.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@roborev-ci

roborev-ci Bot commented Aug 12, 2026

Copy link
Copy Markdown

roborev: Combined Review (c5d7a37)

The changes need fixes for two medium-severity focus and keyboard-routing issues.

Medium

  • frontend/src/lib/components/terminal/WorkspaceTerminalView.svelte:1545 — The workspace sidebar shortcut remains active during modal frames. Because unmatched modal chords do not set defaultPrevented, Ctrl/Cmd+] can toggle or unmount sidebar content behind an open dialog. Gate the listener on getStackDepth() === 0 and add coverage for modal-open behavior.

  • frontend/src/lib/components/terminal/WorkspaceTerminalView.svelte:695 — Removing the focused Details region with Ctrl/Cmd+], or moving the focused bottom terminal into Workflow, clears ownership but leaves focus on document.body. After removing a visible region, focus workspaceRoot when the host remains visible and focus has fallen to document.body.


Reviewers: 2 done | Synthesis: codex, 12s | Total: 14m58s

Workspace shortcuts could still change background layout while a modal owned input. Hiding focused Details or moving a focused bottom Terminal into Workflow could also leave keyboard focus on the document body.

Give modal frames precedence over the workspace sidebar shortcut. When a focused workspace region disappears, restore the workspace root only if no replacement control claimed focus and the host remains visible.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@roborev-ci

roborev-ci Bot commented Aug 12, 2026

Copy link
Copy Markdown

roborev: Combined Review (d32e078)

Medium-severity focus reconciliation issue remains; no security issues were found.

Medium

  • frontend/src/lib/components/shared/DetailPaneLayout.svelte:151 — Focus reconciliation misses pane-content identity replacements. A PR identity change can replace the keyed diff subtree while preserving the same leaf and tab; during history navigation, the focused diff may be removed without focusout, leaving focusedInputLeafID stale. The replacement remains highlighted and consumes Page-key shortcuts even though focus has moved to document.body.
    • Fix: Reconcile focus after content identity replacements by clearing or reclaiming ownership when the layout no longer contains document.activeElement. Add an end-to-end history-navigation test with a focused split diff.

Reviewers: 2 done | Synthesis: codex, 11s | Total: 10m19s

Firefox drops DOM focus while a pooled terminal moves through its parking host. The old detail leaf can remain rendered during a split, so disappearance-only ownership tracking let the layout fallback claim focus before the terminal pool restored it.\n\nKeep live ownership with the exact rendered tab when it moves to another leaf. A connected parked terminal keeps its restoration window, while removed content still falls back to the layout host.\n\nThe full unit lane could not complete locally because unrelated tests timed out and left Vitest consuming host CPU; focused units, frontend checks, both mock browser files, Chromium full-stack, and Firefox post-move terminal input passed.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@roborev-ci

roborev-ci Bot commented Aug 12, 2026

Copy link
Copy Markdown

roborev: Combined Review (336744a)

Medium-severity focus ownership issues remain.

Medium

  • frontend/src/lib/components/shared/DetailPaneLayout.svelte:159 — Focus tracking records the leaf’s active tab instead of the exact focused tab header. Dragging an inactive tab into a new split can leave focusedInputTabKey pointing to the old tab, causing keyboard input and the active border to target the wrong pane.

    Fix: Track the exact focused tab element, locate its destination by tab membership after structural edits, and reclaim focus if it disappears. Add coverage for focusing and splitting an initially inactive tab.

  • frontend/src/lib/components/terminal/WorkspaceTerminalView.svelte:683 — When surfaceLayout is null, workspaceContainerInputActive is always true. The standalone Workspaces view can therefore consume Cmd/Ctrl+] while focus is outside .terminal-view, such as in the application header, violating live-focus ownership.

    Fix: Handle the shortcut only when the event target or document.activeElement is inside workspaceRoot, while preserving modal and terminal precedence. Add coverage for focus in external application chrome.


Reviewers: 2 done | Synthesis: codex, 12s | Total: 10m17s

A focused inactive tab header could report the selected tab instead of itself, so a later split left live ownership on the wrong pane. The standalone Workspaces shortcut could also toggle its sidebar while application chrome held focus.\n\nReport the exact focused tab from renderer DOM identity. Let Cmd/Ctrl+] run only for unclaimed body focus or focus inside the workspace root, while preserving modal and terminal precedence.\n\nValidation: focused pane units passed; frontend checks passed; workspace shortcut and PR detail browser scopes passed in Chromium and Firefox.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@roborev-ci

roborev-ci Bot commented Aug 12, 2026

Copy link
Copy Markdown

roborev: Combined Review (4eb059d)

The change is generally sound, but one medium-severity nested-pane focus bug remains.

Medium

  • Nested tabs can corrupt outer pane ownership trackingfrontend/src/lib/components/shared/TabbedPanelTree.svelte:494

    The unscoped closest("[data-tabbed-panel-tab-key]") may select a tab from a nested pane tree. If a workflow tab inside an embedded Workspace receives focus, the outer detail tree can report the inner session key instead of workspace, associating command memory and ownership with the wrong outer pane or leaf.

    Restrict the discovered tab to one whose nearest .tabbed-panel-leaf is the current leaf; otherwise fall back to leaf.activeTabKey. Add coverage for focusing an embedded workflow tab before moving or issuing commands to the outer workspace pane.


Reviewers: 2 done | Synthesis: codex, 12s | Total: 8m37s

History navigation can replace a focused pane subtree while keeping the same leaf and selected tab. No layout state changes and DOM removal may emit no focusout, so the replacement could keep stale ownership while focus sat on the document body.\n\nObserve the renderer boundary through the app Effect runtime. Release ownership only when the recorded focus node is disconnected; a pooled terminal parked elsewhere remains connected and keeps its own restoration path.\n\nValidation: DetailPaneLayout units passed; frontend checks passed; Chromium full-stack terminal move passed; Firefox reached the post-move input assertions before its known later tmux prompt detector failure.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@roborev-ci

roborev-ci Bot commented Aug 12, 2026

Copy link
Copy Markdown

roborev: Combined Review (2a7115d)

UI focus handling has two medium-severity gaps that can strand focus on document.body and leave stale terminal ownership.

Medium

  • Focused leaf removal does not reclaim focusfrontend/src/lib/components/shared/DetailPaneLayout.svelte:161
    When a focused leaf disappears without its tab moving to a replacement leaf, ownership is cleared but focus is not reclaimed. Workspace-wide “Collapse Terminal” uses setHidden() rather than closePane(); reparenting the focused host into parking keeps it connected, so the mutation observer ignores it and focus is stranded on document.body.
    Fix: Call reclaimFocus() when the focused leaf no longer renders and no replacement exists. Its active-element guard prevents stealing legitimate focus.

  • Terminal session removal can retain stale focus ownershipfrontend/src/lib/components/terminal/DockedTerminalPanel.svelte:198, frontend/src/lib/components/terminal/WorkspaceTerminalView.svelte:695
    Bottom-terminal ownership is released only by focusout or removal of the entire region. Removing or replacing the focused terminal session may emit no focusout; if another terminal remains, the panel stays visible with an active border and retains external/workspace ownership while focus falls to document.body.
    Fix: Track the focused descendant and detect terminal subtree or session-identity replacement, then clear ownership and reclaim the appropriate workspace/layout focus when the browser leaves focus on document.body.


Reviewers: 2 done | Synthesis: codex, 12s | Total: 15m39s

Nested tab trees could report an inner session as the outer pane owner. History navigation could also clear the focused node before the layout observer saw that keyed Files content had been replaced, leaving focus on the document body.

Keep tab identity within its closest rendered leaf, and retain a no-destination focus record long enough for the Effect-scoped observer to distinguish disconnected content from a connected pooled terminal.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@roborev-ci

roborev-ci Bot commented Aug 12, 2026

Copy link
Copy Markdown

roborev: Combined Review (7e4203b)

The focus-ownership changes need one medium-severity fix before merge.

Medium

  • frontend/src/lib/components/terminal/WorkspaceTerminalView.svelte:1248workflowContentKeyFor identifies focused session content only by leaf ID and tab key. Replacing or relaunching a terminal under the same session key leaves the content key unchanged; if the focused terminal DOM is removed without focusout, stale workflow ownership remains and the replacement pane appears active even though focus has fallen to document.body.
    • Fix: Include the workspace/session host generation or another renderer identity in the key, or observe whether the focused descendant remains connected, as DetailPaneLayout does.

Reviewers: 2 done | Synthesis: codex, 11s | Total: 12m47s

An ordinary blur could leave a pane focus record alive until an unrelated later DOM change, which could pull focus back after the user had left. A terminal session could also disappear without focusout while its dock remained visible, leaving stale dock and workspace ownership.

Expire no-destination pane records after the immediate replacement window. Track dock focus by exact DOM containment, restore a surviving dock only for disconnected content, and leave connected pooled terminal moves to their own focus handoff.

Review decision: focused-leaf removal without a replacement is not an issue. Existing Effect reconciliation covers disconnected pane content, and Collapse Terminal moves focus into its portalled controls before hiding panes.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@roborev-ci

roborev-ci Bot commented Aug 12, 2026

Copy link
Copy Markdown

roborev: Combined Review (8d42b79)

Code needs a small focus-ownership fix before merging.

Medium

  • frontend/src/lib/components/shared/DetailPaneLayout.svelte:362 — The mutation observer ignores a focused node whenever it remains connected, even if it has been reparented outside the layout. A pooled terminal can move to parking without emitting focusout, leaving the old leaf styled and reported as the active keyboard owner. Clear pane ownership when !el.contains(focused); reclaim layout focus only when the node is disconnected, since connected nodes should let their new destination restore focus.

Reviewers: 2 done | Synthesis: codex, 13s | Total: 10m31s

mariusvniekerk and others added 2 commits August 12, 2026 11:13
Browsers suspend animation frames in hidden tabs. A no-destination blur could therefore keep a pane or dock focus record alive until later content removal and pull focus back after the user had left.

Race the immediate replacement window against the document becoming hidden. This keeps keyed replacement recovery intact while guaranteeing that ordinary blur ownership expires when rendering pauses.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
A focused pooled terminal can leave a detail layout without dispatching focusout. The old leaf then keeps its active border and keyboard ownership even though the terminal is moving through another host.

Release the layout claim whenever its exact focused node leaves the renderer. Restore layout focus only for disconnected content, so connected terminal moves remain under the pool's existing focus handoff.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@roborev-ci

roborev-ci Bot commented Aug 12, 2026

Copy link
Copy Markdown

roborev: Combined Review (3763faa)

Changes need a focus-ownership fix during workspace switches.

Medium

  • frontend/src/lib/components/terminal/WorkspaceTerminalView.svelte:686 — Region visibility ignores runtimeLive and workspaceDetailsReady. During an in-place workspace switch, focused workflow/sidebar content can be replaced by loading placeholders without reliably emitting focusout, leaving stale input ownership and an active border after focus falls to document.body.
    • Suggested fix: Treat readiness loss or content identity replacement as region disappearance, clear ownership, and reclaim the workspace root after the DOM update when focus is unclaimed. Add end-to-end coverage for switching workspaces while workflow or details content is focused.

Reviewers: 2 done | Synthesis: codex, 8s | Total: 16m42s

The full-stack terminal move regression proved focus and tmux input recovery but did not prove that the old leaf released its active border and keyboard claim.

Assert source and destination ownership around the existing real move so the scenario now covers the complete user-visible focus contract without adding another workspace launch.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@roborev-ci

roborev-ci Bot commented Aug 12, 2026

Copy link
Copy Markdown

roborev: Combined Review (1d22c59)

Focus handling still has two medium-severity regressions involving terminal promotion and layout reset.

Medium

  • frontend/src/lib/components/terminal/WorkspaceTerminalView.svelte:1268 — Workflow recovery focuses workspaceRoot whenever focus temporarily falls to document.body. Promoting a focused pooled terminal reparents it through the parking node, creating this transient state; focusing the root revokes pool focus ownership, so the terminal reaches its new pane without keyboard focus. Track the focused descendant and reclaim the root only when it is disconnected. For connected pooled terminals, clear local ownership and let the terminal pool restore focus. Add an e2e test that promotes a terminal through the command palette while its xterm input is focused.

  • frontend/src/lib/stores/paneLayout.svelte.ts:379reset() clears externalInputActive even when DOM focus remains inside a surviving external terminal dock. Resetting from the command palette while a row-only dock is focused removes active styling and may reactivate dedicated-files shortcuts without a focus change. Preserve live external focus state during layout reset and let dock focusout/unmount or renderer teardown release it. Add coverage for resetting while an external dock retains focus.


Reviewers: 2 done | Synthesis: codex, 17s | Total: 15m1s

An in-place workspace switch can replace focused Workflow or Details content with loading placeholders before the browser delivers a useful focusout. The old region then keeps stale keyboard ownership while focus falls to the document body.

Treat runtime and detail readiness as part of rendered ownership. Keep the guarded one-tick recovery independent of phased readiness reruns so a later phase cannot interrupt the focus handoff it just requested.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@roborev-ci

roborev-ci Bot commented Aug 12, 2026

Copy link
Copy Markdown

roborev: Combined Review (0e600fa)

Review verdict: One medium-severity focus recovery issue should be addressed before merging.

Medium

  • frontend/src/lib/components/terminal/WorkspaceTerminalView.svelte:1271 — Workflow focus recovery can be canceled when removing focused content also changes focusedWorkflowTabKey. The effect cleanup interrupts the scheduled recovery; when the effect reruns, workspaceInputRegion may already have been cleared by focusout, so no new recovery is scheduled and focus can remain on document.body.
    • Suggested fix: Keep this bounded recovery independent of effect reruns, or defer clearing null-destination focus ownership until after the post-update focus check.

Reviewers: 2 done | Synthesis: codex, 10s | Total: 19m29s

Command-palette promotion can move a focused pooled terminal while the palette subtree is still inert. Generic Workflow recovery and modal fallback could then replace xterm focus with a layout root, even though the terminal stayed connected and had a destination.

Leave connected Workflow content to the pool, queue explicit focus for the exact promoted session, and keep a surviving external dock claim across pane-layout reset. DOM focusout and teardown remain responsible for releasing live ownership.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@roborev-ci

roborev-ci Bot commented Aug 13, 2026

Copy link
Copy Markdown

roborev: Combined Review (3b86ded)

The focus-routing changes introduce one medium-severity navigation consistency issue; no security issues were found.

Medium

  • Inactive tab focus can switch the visible pane without updating the routefrontend/src/lib/components/shared/DetailPaneLayout.svelte:295

    Focusing an inactive tab header always persists it through layout.noteFocused(). In narrow layouts, activeTree uses lastFocusedTabKey() as the flattened selection, so keyboard-tabbing onto an inactive header changes the visible pane without invoking onSelectTab or updating the route.

    Suggested fix: Track the exactly focused tab separately for live ownership and drag recovery. Update flattened-selection memory only when a tab is activated, or route flattened header focus through the normal selection callback.


Reviewers: 2 done | Synthesis: codex, 14s | Total: 13m54s

Embedded workspace content can change twice while the browser reports a delayed focusout. The second reactive pass interrupted the one-tick recovery, so focus could remain on the document body after the focused session exited.

Let the bounded recovery finish independently of reactive reruns. Its live-focus and connected-content guards still prevent it from replacing a deliberate focus destination or the terminal pool's handoff.

Generated with Codex
Co-authored-by: Codex <noreply@openai.com>
@roborev-ci

roborev-ci Bot commented Aug 13, 2026

Copy link
Copy Markdown

roborev: Combined Review (2a5051d)

No issues found.


Reviewers: 2 done | Synthesis: codex | Total: 10m23s

@mariusvniekerk
mariusvniekerk merged commit e19202f into main Aug 13, 2026
17 checks passed
@mariusvniekerk
mariusvniekerk deleted the t3code/fix-pgdown-textarea-scrolling branch August 13, 2026 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant