Skip to content

fix(terminal): keep a brand-new remote pane's typing for the terminal it creates - #33859

Merged
nwparker merged 5 commits into
mainfrom
nwparker/wsl-new-terminal-typeahead
Oct 26, 2026
Merged

nwparker merged 5 commits into
mainfrom
nwparker/wsl-new-terminal-typeahead

Conversation

@nwparker

@nwparker nwparker commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor
Files Added Deleted Net
Test 3 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​403 $\color{#cf222e}{\Huge{\mathbf{−}}}$​4 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​399
Prod 6 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​119 $\color{#cf222e}{\Huge{\mathbf{−}}}$​9 $\color{#1a7f37}{\Huge{\mathbf{+}}}$​110

ELI5

Keys you type into a brand-new terminal on a WSL host or paired server, before its shell exists yet, used to be thrown away. Now they're kept and sent to that new shell once it's created.

What Changed

Before: on a WSL host, opening a new terminal and typing right away lost those keys. That happened even when the terminal already had keyboard focus, because the host hadn't finished creating the shell.

After: a brand-new terminal pane delivers what you typed before and during its creation, in order, to the shell it creates.

When it decides:

  • At connect start, once: whether the pane looked blank. That means no terminal bound to or restored for its leaf, and no output on screen.
  • At bind: only things the fresh create can't cause itself are re-checked: startup work appearing, and replayed history. The create writes its own leaf binding before the connect settles, and its prompt can already be parsed by then, so re-checking those at bind would drop the keys every time.

Mechanism: paired-server panes, which include WSL hosts, hold typing in remote-runtime-recovery-input-hold.ts until the pane binds a terminal. When a connect created a fresh terminal instead of restoring one, connectHoldingRestoredInput called expect(null), which discards the held typing. That guard exists so keys typed on an old, restored screen never run in a replacement shell (#10065). But it also dropped a brand-new pane's keys, where there's no old screen at all.

  • New pane option: keepTypeAheadForNewTerminal. pane-transport-options.ts sets it only for a pane with no startup work (the same check fix(terminal): keep keys typed before a pane's terminal exists #33281 uses) and no terminal bound to or restored for its leaf.
  • First create: for such a pane's first create, the hold uses the new expectNewTerminal(). The held typing is kept and, once the create binds, attached to the terminal it made, then released as usual.

Held typing is delivered only when the first create is a genuine fresh create: the host's result has neither isReattach nor coldRestore. If the host adopts an existing terminal for that tab and leaf, or restores history into the new one, the typing is dropped as before. Otherwise text typed for a blank pane could run at a live REPL or agent prompt, or under restored history (#10065).

Held typing is also dropped when:

  • the create binds no terminal;
  • saved history was replayed into the pane, even if its binding was cleared. restoreScrollbackBuffers flags the terminal, and the flag is checked at connect and again at bind;
  • it's a host-mirrored web-surface tab;
  • startup work appeared before the bind (the releasePreconnectInput recheck from fix(terminal): keep keys typed before a pane's terminal exists #33281);
  • the pane showed an old terminal;
  • it's a later connect with no session id.

Local, WSL-via-IPC and SSH panes don't use this transport and are unchanged.

Why

The guard is right for restored screens, so this keeps it and carves out only the case it was never meant for: a pane that never showed a terminal. Reusing the existing hold keeps ordering, limits and acknowledged writes the same as for reattach type-ahead.

Linked Issue

N/A (internal). Found on a Windows WSL host.

Visual Proof

N/A: nothing changes on screen except that typed keys arrive. The tests below cover it.

Testing

  • remote-runtime-pty-transport-reattach-type-ahead.test.ts, under "a brand-new pane":

    • Typing before and during the first create arrives as echo hi\r once the terminal binds. This fails without the fix.
    • Typing is dropped when startup work appeared before the bind.
    • Typing is dropped when the create fails.
    • The existing "discards pre-entry typing when the first connect does not restore this pane" cases still pass, because they don't set the new option.
  • New pty-connection-remote-new-terminal-typeahead.test.ts runs connectPanePty with the real options builder and the real remote transport, so the fresh create writes the leaf binding as in the app:

    • typed keys reach the terminal as echo hi\r, and the leaf ends up bound to the new terminal;
    • an adopted terminal drops the keys, and so does a cold-restored one;
    • saved history replayed through the real restoreScrollbackBuffers before the bind drops the keys. This test fails without the replay mark;
    • re-checking the binding at bind makes the delivery test fail, which reproduces the inert version.
  • Also in the transport test file:

    • typing held across a failed first create is dropped when a later connect creates.
    • typing is dropped when the first create returns an adopted terminal, and when it returns a cold-restored one. Both fail without the fresh-create check;
    • typing is dropped when replayed history appears before the bind.
  • pty-connection-preconnect-input.test.ts:

    • a new pane keeps its typing;
    • a pane with a restored terminal doesn't;
    • a pane stops keeping it once replayed history shows.
  • pnpm tc passes. The full pnpm lint and the terminal-pane suite run in CI. Locally, under very heavy machine load, the suite timed out in unrelated files.

  • Not run against a real WSL host.

  • I manually tested these changes locally

  • Automated tests added/updated, or explained why not below

AI Disclosure

Review

Agent skill upstream boundary

  • Not applicable, or this change copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.

Notes

  • Layout-owner build: under ORCA_LAYOUT_OWNER_IS_MAIN, restoreScrollbackBuffers receives no buffers, so nothing sets the replay mark. That's safe today, because there history arrives through a reattach with a session id, which never takes the new-terminal path. Revisit before phase F turns that flag on.
  • Older paired servers: no wire change. The keys go out as ordinary input after the bind.
  • Agent panes: an agent pane on a remote host isn't affected, because it has startup work.

Checklist

  • This PR is small and focused
  • I explained what changed and why (ELI5, the user-facing before/after, the mechanism, and why over the alternatives)
  • Before/after screenshots or videos attached for UI changes, or N/A with reason
  • Self-reviewed for correctness, security, and performance
  • Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)
  • pnpm lint, pnpm typecheck, pnpm test, and pnpm build pass (or CI will cover; local preferred)

@coderabbitai

coderabbitai Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 2 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used all 10 included reviews currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 94cbf047-2906-48ef-9e7f-286fc39d9db3
Commits

Reviewing files that changed from the base of the PR and between b5a950f and 6c5fe66.

Files selected for processing: 2
  • src/renderer/src/components/terminal-pane/pty-connection-preconnect-input.test.ts
  • src/renderer/src/components/terminal-pane/replayed-history-terminals.ts

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 00045dea-5a59-47fa-a13e-d7aa2612e756
Commits

Reviewing files that changed from the base of the PR and between 4227c70 and ddfeb81.

Files selected for processing: 1
  • src/renderer/src/components/terminal-pane/pty-connection-remote-new-terminal-typeahead.test.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

Walkthrough

The change adds a phase-aware option that determines whether a pane may retain input typed before its remote terminal exists. The transport can assign held input to a newly created terminal. It drops held input when the connection is unbound, creation fails, or the terminal is adopted or restored. Pane checks use restored PTY mappings, visible output, web terminal-surface tabs, and replayed history.

Priority: Low

Merge Risk: Moderate · previous review ddfeb

Typing may still be lost while a new remote terminal connects, and the anti-slop audit is expected to fail. Resolve both issues before merging.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 46.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 15 functions across 9 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Title check Passed The title names the main change: a brand-new remote pane keeps the typing it made before its terminal was created, and delivers it to that terminal. It is specific, follows the conventional commit sty…
Description check Passed The description follows the repository template closely. It has ELI5, What Changed (before/after and mechanism), Why, Testing with specific test files and cases, a Visual Proof N/A with reason, Notes …
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.
Full details: Title check

Explanation

The title names the main change: a brand-new remote pane keeps the typing it made before its terminal was created, and delivers it to that terminal. It is specific, follows the conventional commit style, and matches the PR objectives and file changes (keepTypeAheadForNewTerminal, expectNewTerminal, the new test files).

Full details: Description check

Explanation

The description follows the repository template closely. It has ELI5, What Changed (before/after and mechanism), Why, Testing with specific test files and cases, a Visual Proof N/A with reason, Notes covering backwards compatibility and remote behavior, and the checklist. Gaps: the Linked Issue section says 'N/A (internal)', but the template requires an issue for outside contributors and the 'Fixes #' line is not filled in. The AI Disclosure and Review sections are empty. The manual testing box is unchecked, and the lint/typecheck/test/build checklist item is unchecked. The description also says the full lint and terminal-pane suite were not run locally.

Resolution

Optional improvements: state whether the author is an internal contributor so the Linked Issue requirement can be confirmed, fill in or remove the empty AI Disclosure and Review sections, and either tick the manual test box or note that manual WSL testing was not done. The description already says it was not run against a real WSL host. Complete the lint, typecheck, test and build checklist item once CI passes.

Finishing Touches: 💡 2
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


Review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 34d2c4a4-0e4e-4e31-9c53-ef6d4f2b5b7c
Commits

Reviewing files that changed from the base of the PR and between 351705b and 677666e.

Files selected for processing: 6
  • src/renderer/src/components/terminal-pane/pty-connection-preconnect-input.test.ts
  • src/renderer/src/components/terminal-pane/pty-connection/pane-transport-options.ts
  • src/renderer/src/components/terminal-pane/pty-transport-types.ts
  • src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-reattach-type-ahead.test.ts
  • src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts
  • src/renderer/src/components/terminal-pane/remote-runtime-recovery-input-hold.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.

!options.sessionId && isFirstConnect && opts.keepTypeAheadForNewTerminal === true
isFirstConnect = false
const expected = newTerminal
? recoveryInputHold.expectNewTerminal()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Major · Functional Correctness · Quick win

Keep new-terminal input held until the expectation settles.

If the subscription becomes ready before connectPane() returns, subscribeToHandle() calls releaseHeldInput() while this expectation still has a null endpoint. RemoteRuntimeRecoveryInputHold.release() in src/renderer/src/components/terminal-pane/remote-runtime-recovery-input-hold.ts Lines 170-180 discards the typing and replaces it with an empty hold. The successful create then binds only that empty hold. Defer release until settlement assigns the created endpoint. Test a subscribed response that arrives before connecting resolves.

@nwparker
nwparker force-pushed the nwparker/wsl-new-terminal-typeahead branch from 677666e to 8d04351 Compare October 26, 2026 19:59

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1


Review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 472a3ef8-40af-4b13-8658-d92c48994cb1
Commits

Reviewing files that changed from the base of the PR and between 677666e and 4227c70.

Files selected for processing: 8
  • src/renderer/src/components/terminal-pane/layout-serialization.ts
  • src/renderer/src/components/terminal-pane/pty-connection-preconnect-input.test.ts
  • src/renderer/src/components/terminal-pane/pty-connection-remote-new-terminal-typeahead.test.ts
  • src/renderer/src/components/terminal-pane/pty-connection/pane-transport-options.ts
  • src/renderer/src/components/terminal-pane/pty-transport-types.ts
  • src/renderer/src/components/terminal-pane/remote-runtime-pty-transport-reattach-type-ahead.test.ts
  • src/renderer/src/components/terminal-pane/remote-runtime-pty-transport.ts
  • src/renderer/src/components/terminal-pane/replayed-history-terminals.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.

@nwparker
nwparker force-pushed the nwparker/wsl-new-terminal-typeahead branch from 4227c70 to ddfeb81 Compare October 26, 2026 21:32
@nwparker
nwparker force-pushed the nwparker/wsl-new-terminal-typeahead branch from ddfeb81 to b5a950f Compare October 26, 2026 22:09
@nwparker
nwparker merged commit 73ede0f into main Oct 26, 2026
46 checks passed
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