feat: raw command channel for et - #854
Merged
Merged
Conversation
Cursor needs a non-pty command channel with binary stdio and separate stderr. Co-authored-by: Cursor <cursoragent@cursor.com>
Hold stdout/stderr write ends until after child dup2 via a ready pipe, and switch tests to octal escapes that dash and FreeBSD sh honor. Co-authored-by: Cursor <cursoragent@cursor.com>
Avoid the stdio stderr macro clash that breaks OpenWrt cross-compiles of generated ETerminal.pb.h accessors. Co-authored-by: Cursor <cursoragent@cursor.com>
PipeUserTerminalWindows socketpair helper was resolving bind to std::bind and failing the Windows vcpkg matrix (which then cancelled sibling jobs). Co-authored-by: Cursor <cursoragent@cursor.com>
FreeBSD sh execs `cat >file` as the last -c command, closing the pipe write end while cat still reads stdin. Close stdin on stdout EOF and end the test command with `true` so waitid cannot hang. Co-authored-by: Cursor <cursoragent@cursor.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #854 +/- ##
==========================================
- Coverage 79.35% 79.29% -0.06%
==========================================
Files 121 125 +4
Lines 12769 13090 +321
Branches 8219 8441 +222
==========================================
+ Hits 10133 10380 +247
- Misses 1568 1614 +46
- Partials 1068 1096 +28 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Gate POSIX headers and use io.h/_fileno so the vcpkg Windows matrix can compile TerminalClientMain with et -T. Co-authored-by: Cursor <cursoragent@cursor.com>
echo OUT exited before FakeConsole connected, so the handler tore down and the collector FATAL'd on an invalid socket. Mirror Unix cat with findstr waiting for EOF. Co-authored-by: Cursor <cursoragent@cursor.com>
cmd /c treats ^ as an escape, so findstr \"^\" never behaved as intended and the integration test hung until the 600s ctest timeout. Co-authored-by: Cursor <cursoragent@cursor.com>
Close bridge sockets before joining threads so recv cannot hang, and terminate a stubborn child after a short wait so uth join cannot stall the raw-command integration test for 600s. Co-authored-by: Cursor <cursoragent@cursor.com>
Avoid LOG(FATAL) on an invalid socket when the Windows session ends before the collector starts draining stdout. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The cross-platform process, pipe, buffering, and protocol lifecycle changes require final human validation.
Review effort: Balanced
Findings: None
What changed in this PR
Adds a raw command channel for et -T -c, using pipes rather than a PTY while preserving the existing interactive path.
Changes:
- Adds
no_pty, command, and stderr metadata to the protocol. - Introduces Unix/Windows pipe-backed terminals and binary stdio handling.
- Routes stdout/stderr separately and adds integration coverage.
| File | Description |
|---|---|
proto/ETerminal.proto |
Adds raw-command and stderr protocol fields. |
src/terminal/BinaryStdioConsole.hpp |
Adds binary local stdio console. |
src/terminal/ParseConfigFile.hpp |
Marks header-defined functions inline. |
src/terminal/PipeUserTerminal.hpp |
Selects the platform implementation. |
src/terminal/PipeUserTerminalUnix.hpp |
Implements Unix child pipes. |
src/terminal/PipeUserTerminalWindows.hpp |
Implements Windows pipes and socket bridges. |
src/terminal/TerminalClient.cpp |
Sends raw-mode metadata and separates output streams. |
src/terminal/TerminalClient.hpp |
Extends client state and constructor. |
src/terminal/TerminalClientMain.cpp |
Adds -T and selects binary stdio. |
src/terminal/TerminalServer.cpp |
Relays packet-framed raw streams. |
src/terminal/UserTerminal.hpp |
Generalizes terminal input/stderr descriptors. |
src/terminal/UserTerminalHandler.hpp |
Adds raw-mode state and forwarding helper. |
src/terminal/UserTerminalHandlerUnix.cpp |
Pumps Unix stdin/stdout/stderr pipes. |
src/terminal/UserTerminalHandlerWindows.cpp |
Pumps Windows bridge sockets. |
test/integration_tests/RawCommandChannelTest.cpp |
Tests binary stream separation and command handling. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This was referenced Sep 24, 2026
4 of 5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
-Tremote commands on pipes instead of a pty: binary stdin/stdout, separate stderr, no motd, and no shell; exitinjection.Test plan
ctest --test-dir build --output-on-failure --parallel-T -ccommand: stdout and stderr are distinct and binary-safe-Tstill allocates a ptyMade with Cursor