fix(terminal): keep slow clients connected under heavy output#107348
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 867e11a054
ℹ️ 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".
| } | ||
| } | ||
| return cmd.parent?.opts?.() as BrowserParentOpts; | ||
| return cmd.optsWithGlobals<BrowserParentOpts>(); |
There was a problem hiding this comment.
Keep child --url out of gateway options
Commander v15's optsWithGlobals() returns merged local and global option values (its source notes that globals overwrite locals), so when the user does not pass a browser-level gateway --url, any child command that defines its own --url now appears in parent.url. For example, openclaw browser cookies set ... --url https://example.com and openclaw browser wait --url '**/ready' pass that page/cookie URL through callBrowserRequest as the Gateway WebSocket URL instead of leaving it undefined for the configured gateway, causing those commands to connect to the wrong endpoint; this helper should keep returning only the browser command's Gateway options or otherwise filter child-local collisions.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fac6f53670
ℹ️ 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".
| httpServer.once("listening", onListening); | ||
| httpServer.once("error", onError); | ||
| const httpServer = await new Promise<Server>((resolve, reject) => { | ||
| const server = expressApp.listen(port, (err) => (err ? reject(err) : resolve(server))); |
There was a problem hiding this comment.
Restore the listen-time error listener
When the Teams webhook port is already in use or bind fails, http.Server.listen reports that through the server's error event, not by passing an argument to the listen callback (the callback is invoked with zero args on success). Because this Promise only waits for the callback and the httpServer.on("error") handler is attached after the await, startup bind errors are emitted with no listener, so the monitor can crash or hang instead of rejecting/logging as the previous once("error") path did.
Useful? React with 👍 / 👎.
Dependency graph guard clearedThis PR no longer has blocked dependency graph changes. A future dependency graph change requires a fresh
|
|
Merged via squash.
|
…aw#107348) * fix(terminal): coalesce output and throttle slow clients * fix(terminal): satisfy lint and deadcode gates
Related: #107214
What Problem This Solves
Fixes an issue where operators using the embedded terminal could be disconnected as slow consumers when a shell produced sustained output, while every small PTY chunk also incurred its own WebSocket frame.
Why This Change Was Made
Terminal output now coalesces for up to 4 ms or 64 KiB, with immediate delivery for small keystroke echoes. Local PTYs pause when the owning socket crosses a high buffered-byte watermark and resume below a lower watermark, with periodic state reassertion; node-relay terminals retain their existing node-host progress backpressure.
User Impact
Embedded terminals remain responsive during interactive input and tolerate sustained output without eagerly closing the operator connection. Release note: fixed embedded-terminal disconnects under heavy output by batching frames and applying PTY backpressure.
Evidence
node scripts/run-vitest.mjs src/gateway/terminal: 171 tests passed on Blacksmith Testboxtbx_01kxfysrtfdbvtmfn6msc7dq0e.node scripts/run-vitest.mjs src/gateway/gateway-misc.test.ts: 43 tests passed on the same Testbox.pnpm build: passed on the same Testbox against the final changed content.pnpm check:changed: conflict, LOC-ratchet, attribution, dependency, formatting, and export guards passed; the Plugin SDK baseline hash mismatch reproduces unchanged fromorigin/mainin an isolated archive.