fix(terminal): keystrokes typed while a terminal opens are lost; node PTY relay lifecycle races#107214
Conversation
openclaw-barnacle
Bot
added
app: web-ui
5ecc413 to
bcd3716
Compare
Pin node relay invokes to the authorized node connection (ROUTE_CHANGED guard), stop node-side input writes to settled/dead PTYs, finalize sessions on resize failure like write failure, keep surrogate pairs intact across scrollback and relay prelude truncation, bound the relay pre-registration output buffer, and buffer Control UI keystrokes typed while terminal.open/attach is in flight so slow node-relay opens no longer drop input. Correct the terminal.data seq doc to its real contract and pin seq monotonicity, seq-gap bounds, input coercion, and runner event routing with new tests.
Move Control UI startup input buffering into terminal-startup-input.ts (LOC budget), drop a stale session-manager comment, remove a map spread, and fix test typings for event frames and typed mock access.
bcd3716 to
2247173
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 22471737f1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| createBackend = async () => | ||
| await createNodeRelayBackend({ | ||
| registry: context.nodeRegistry, | ||
| nodeId: relay.plan.nodeId, | ||
| expectedConnId: access.node.connId, | ||
| command: relay.plan.command, |
There was a problem hiding this comment.
When an async plugin invoke policy/approval is pending and the node reconnects with the same terminal command, this final authorization picks up the replacement connection and expectedConnId pins the resume invoke to that new connId, even though applyPluginNodeInvokePolicy was evaluated against the earlier access.node. The policy helper's own invokeNode path treats a connId change as ROUTE_CHANGED, so this path should fail closed or rerun the plugin policy for the final node; otherwise a replacement node can receive a terminal command under policy/approval granted for a different connection.
Useful? React with 👍 / 👎.
… PTY relay lifecycle races (openclaw#107214) * fix(terminal): harden node PTY relay and terminal session lifecycle Pin node relay invokes to the authorized node connection (ROUTE_CHANGED guard), stop node-side input writes to settled/dead PTYs, finalize sessions on resize failure like write failure, keep surrogate pairs intact across scrollback and relay prelude truncation, bound the relay pre-registration output buffer, and buffer Control UI keystrokes typed while terminal.open/attach is in flight so slow node-relay opens no longer drop input. Correct the terminal.data seq doc to its real contract and pin seq monotonicity, seq-gap bounds, input coercion, and runner event routing with new tests. * refactor(terminal): extract startup input buffer and fix CI findings Move Control UI startup input buffering into terminal-startup-input.ts (LOC budget), drop a stale session-manager comment, remove a map spread, and fix test typings for event frames and typed mock access.
1 participant
What Problem This Solves
Fixes an issue where keystrokes typed into a freshly opened Control UI terminal were silently dropped until the gateway session finished opening. For "open in terminal" on Claude/Codex catalog sessions that live on a paired node, that window covers a remote eligibility check plus a remote PTY spawn — user-visible seconds of lost input.
It also closes several lifecycle races in the node PTY relay and terminal session management found while auditing the paths end to end:
expectedConnId/ROUTE_CHANGEDfor exactly this; the relay never used it).resizeon a dead PTY left a zombie session alive, while a throwingwritecorrectly finalized it.terminal.dataseqfor gap detection; no client does (transport is ordered) — the description now states the real contract.Why This Change Was Made
Hardening pass over the embedded terminal and the node PTY forwarding path, keeping each fix at its owner boundary: connection pinning in the gateway relay/open handler, input guarding on the node host, buffering in the Control UI panel, and truncation safety in one shared
surrogateSafeTailhelper. Startup input buffering is bounded (8 KiB) and latches off on overflow so replayed input can never be reordered or partially truncated. No protocol shape, config, or plugin SDK changes.User Impact
Typing immediately after opening a terminal — especially resuming a Claude/Codex session on a paired node — now reaches the shell instead of vanishing. Dead terminals are torn down promptly on resize, reattach replay no longer corrupts wide characters, and a node reconnect during open fails closed with a visible error instead of dispatching over an unauthorized route.
Evidence
node scripts/run-vitest.mjs src/gateway/terminal src/gateway/node-registry.test.ts src/gateway/server-methods/terminal.test.ts src/node-host/pty-command.test.ts src/node-host/invoke-payload.test.ts src/node-host/runner.test.ts→ 6 files, 121 tests;node scripts/run-vitest.mjs ui/src/components/terminal→ 3 files, 32 tests.ROUTE_CHANGEDsurfacing, seq-gap buffering bounds (128-frame cap → idle teardown), input-after-exit/abort no-ops, out-of-range resize rejection,coerceNodeInvokeInputPayloadbounds,node.invoke.input/cancel/close runner routing, seq monotonicity across detach→attach, surrogate-safe truncation, startup-input flush ordering + overflow latch + failed-open discard.pnpm check:changeddelegated gates: running at PR creation; will be green on the head SHA before merge.