Skip to content

fix(terminal): skip already-focused refocus — stop Ink ?1004 focus-in frame stacking#342

Merged
kjgbot merged 1 commit into
mainfrom
render-fix/terminal-dup
Jun 14, 2026
Merged

fix(terminal): skip already-focused refocus — stop Ink ?1004 focus-in frame stacking#342
kjgbot merged 1 commit into
mainfrom
render-fix/terminal-dup

Conversation

@kjgbot

@kjgbot kjgbot commented Jun 14, 2026

Copy link
Copy Markdown
Contributor

The vector

The Pear terminal pane stacked Claude Code's Ink TUI output — tool cards, the "currently unavailable" banner, input echoes, the bottom status line all rendered 2–3×, accumulating with focus/tab interactions.

Root cause: the renderer's focus helpers run a container.focus()textarea.focus() dance on every pointerdown / keydown / paste. xterm already focuses its textarea synchronously on mousedown, so re-running the dance blurs the still-focused textarea (the container is tabIndex=0, so container.focus() pulls focus off it) and immediately refocuses it. Under DECSET ?1004 — which Claude Code's Ink UI enables — that pointless blur+focus pair emits \x1b[O\x1b[I to the PTY on every interaction. The TUI reads it as "the user looked away / back" and re-commits a frame, so duplicate frames pile up in scrollback and grow with each click.

Why a creation-vector fix (and the reconciler is untouched)

The broker snapshot — terminal-reconciler.ts's ground truth — stays correct, and the reconciler only ever repairs the viewport, never scrollback (its repair payload is self-framing and explicitly leaves scrollback alone). So the stacked frames that scroll up are unreachable by the backstop; the only durable fix is to stop emitting the spurious focus reports at the source. The reconciler and all its gates are left fully intact.

The fix

focusTerminalTextarea() skips the focus dance entirely when the xterm textarea is already the active element. Genuine focus transitions (first mount; input arriving while focus is elsewhere) still focus exactly once — the textarea isn't the active element in those cases. Applied to the mount-focus helper and the pointerdown/keydown/paste handlers.

Reproduce-before-fix (no vacuous change)

The existing fidelity-no-duplication and rendering-corruption suites all pass and do not reproduce this — none model a ?1004 focus-in→redraw TUI. New spec tests/playwright/terminal-focus-stacking.spec.ts does:

  • A new ipc-mock setFocusRedraw knob models a ?1004 TUI: every focus report the renderer emits (\x1b[I/\x1b[O) re-commits a frame to the PTY stream (the stacking behavior).
  • The test enables ?1004, lays a baseline frame, then clicks an already-focused terminal repeatedly and asserts the frame stack does not grow.

RED → GREEN evidence

  • RED (current code / on revert of the guard): 6 clicks stacked the frame settled 4 → 16 (~2 frames per click, unbounded growth).
  • GREEN (with fix): the count stays flat across clicks; activeElement stays on the xterm textarea throughout.
  • revert-RED confirmed: reverting only the if (already focused) return guard reproduces the failure (settled at 4, now 16).

Verification

  • New test RED→GREEN, revert-RED confirmed.
  • npm run test:fidelity8/8 green (new spec + all existing fidelity/corruption tests, incl. "window focus events during stream" and "runtime remount mid-stream").
  • npm run typecheck:web → green.
  • use-terminal.dom.test.ts (incl. "focuses xterm exactly once on first mount") and terminal-runtime-registry.dom.test.ts → green.
  • terminal-reconciler.ts not modified.

🤖 Generated with Claude Code

… frame stacking

The Pear terminal pane stacked Claude Code's Ink TUI output (tool cards,
banners, status line rendered 2-3x, accumulating with focus/tab
interactions). Root cause: the renderer's focus helpers run a
container.focus() -> textarea.focus() dance on every pointerdown / keydown /
paste. xterm already focuses its textarea synchronously on mousedown, so the
dance blurs the still-focused textarea (the container is tabIndex=0, so
container.focus() pulls focus off it) and immediately refocuses it. Under
DECSET ?1004 — which Claude Code's Ink UI enables — that pointless blur+focus
pair emits "\x1b[O\x1b[I" to the PTY on every interaction; the TUI re-commits
a frame per report, so duplicates stack in scrollback. The broker snapshot
stays correct, so terminal-reconciler.ts (which repairs the viewport only)
cannot undo the scrollback pile-up — the spurious reports must not be emitted.

Fix: focusTerminalTextarea() skips the focus dance entirely when the xterm
textarea is already the active element. Genuine transitions (first mount,
input arriving while focus is elsewhere) still focus exactly once. The
reconciler backstop is left fully intact.

Reproduction (new, faithful — the existing fidelity/corruption suites all
pass and do NOT reproduce this): tests/playwright/terminal-focus-stacking
models a ?1004 TUI via a new ipc-mock setFocusRedraw knob that re-commits a
frame on every focus report the renderer emits, then clicks an
already-focused terminal repeatedly and asserts the frame stack does not
grow.

  RED (before fix / on revert): 6 clicks stacked the frame to 16 (~2 per click)
  GREEN (after fix): the count stays flat across clicks
  Verified: new test RED->GREEN, revert-RED confirmed (settled 4 -> 16),
  `npm run test:fidelity` 8/8 green, `npm run typecheck:web` green,
  use-terminal/runtime-registry dom tests green.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@gemini-code-assist

Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@coderabbitai

coderabbitai Bot commented Jun 14, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: e907fe58-cc2f-41e0-9197-2b20abff6715

📥 Commits

Reviewing files that changed from the base of the PR and between e7571b7 and 9e6f8d4.

📒 Files selected for processing (4)
  • playwright.fidelity.config.ts
  • src/renderer/src/hooks/use-terminal.ts
  • src/renderer/src/lib/ipc-mock.ts
  • tests/playwright/terminal-focus-stacking.spec.ts

📝 Walkthrough

Walkthrough

Adds a focusTerminalTextarea guard helper to use-terminal.ts that skips refocusing when the xterm textarea is already active, replacing all inline focus sequences. Extends the IPC mock harness with a setFocusRedraw knob that injects a frame on focus escape sequences. Adds a Playwright regression test verifying no extra frames stack on repeated clicks to an already-focused terminal, and registers it in the fidelity config.

Changes

Terminal Focus-Stacking Regression Fix

Layer / File(s) Summary
focusTerminalTextarea helper and all call sites
src/renderer/src/hooks/use-terminal.ts
Introduces focusTerminalTextarea(container, term) with a guard that returns early when document.activeElement === term.textarea. Replaces all inline focus sequences in focusTerminal, pointerdown, keydown, and paste handlers with requestAnimationFrame(() => focusTerminalTextarea(...)) calls.
ipc-mock setFocusRedraw knob
src/renderer/src/lib/ipc-mock.ts
Adds mockFocusRedrawFrame state, a setFocusRedraw(frame) method to PearMockHarness, its implementation on pearMockHarness, and logic in sendInputFast that detects \x1b[I/\x1b[O escape sequences and immediately injects the stored frame via injectPtyChunk.
Playwright regression test and config wiring
tests/playwright/terminal-focus-stacking.spec.ts, playwright.fidelity.config.ts
Adds the full terminal-focus-stacking.spec.ts suite with bootWithAgent, card-count utilities, and a regression test that arms ?1004, captures a settled card count, runs repeated clicks, and asserts the count does not grow. Registers the new spec in the testMatch pattern.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • AgentWorkforce/pear#176: Introduced playwright.fidelity.config.ts and the mock harness APIs (injectPtyChunk, setInputEcho) that this PR extends with the new setFocusRedraw knob and the additional testMatch entry.

Poem

🐇 Hop, hop — the cursor blinks!
No more double-frames in sync,
focusTerminalTextarea guards the gate,
Escape sequences settle straight.
The stacking cards now hold their count —
One click, one frame, no extra mount! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and specifically describes the main fix: preventing redundant focus operations on already-focused terminal elements to stop focus-report frame stacking.
Description check ✅ Passed The description comprehensively explains the root cause, the fix, and provides detailed verification evidence including RED/GREEN test results and comprehensive test coverage.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch render-fix/terminal-dup

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed due to a network error.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@agent-relay-code

Copy link
Copy Markdown
Contributor

Findings
No blocking issues found in the PR changes. I did not apply any file edits.

Addressed Comments

  • gemini-code-assist[bot]: quota-limit warning only; no code finding to validate or fix.
  • coderabbitai[bot]: review-in-progress/status comment only. Current GitHub status reports CodeRabbit completed successfully; no inline findings to address.

Verification

  • npm ci
  • npm run verify:mcp-resources-drift
  • npm run lint passed with existing warnings
  • npm run typecheck:web
  • npm run typecheck:node
  • npm test
  • npx vitest run
  • npm run build
  • npm run build:web
  • npx playwright install --with-deps chromium
  • npx playwright test --config playwright.fidelity.config.ts
  • npx playwright test --config playwright.redraw.config.ts

GitHub API reports PR #342 mergeable with mergeable_state: clean; GitHub Actions checks checks, playwright, and packaged-mcp-smoke are completed successfully. The Cubic AI reviewer check is neutral due to quota, not a code/test failure.

@kjgbot kjgbot merged commit 89d5a88 into main Jun 14, 2026
5 checks passed
@kjgbot kjgbot deleted the render-fix/terminal-dup branch June 14, 2026 18:36
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.

1 participant