fix(mirror): let the session page type into a running engine - #453
Merged
Conversation
Arrow keys sent from app.moshcode.sh/sessions never reached an engine, so
Claude's "do you trust this folder?" arrived on the page sitting on No with
no way to move off it.
The keys were being delivered; they were delivered to the wrong place.
pressKey ended in `stdin.emit("data", …)`, a synthesised event on this
process's own stdin object. Readline hears that. A child spawned with
`stdio: "inherit"` reads a file descriptor and hears nothing, and by then the
pit's readline had been closed for the hand-off, so ↓ went nowhere at all.
So own the child's stdin. captureSpec grows an `input` option that puts a
fifo there instead of the tty, which script(1) reads and copies to the pty
master exactly as it would a terminal. Two things have to be paid back for
that, and both are, rather than being written off:
- script takes the pty's geometry from its own stdin, and a fifo has none,
so the child would start on a 0x0 terminal. It sizes itself on the way in
with stty, which can ioctl the master from inside where we cannot from
out here.
- script can no longer forward SIGWINCH for the same reason. The child
records its pty path on the way in, so a real window resize still reaches
it via `stty -F`.
The person at the keyboard keeps working throughout: local stdin is relayed
byte-for-byte into the same fifo, in raw mode, because the pty on the far end
is now the one echoing and splitting lines.
Cursor keys are sent in the form the child asked for. A full-screen program
usually sets DECCKM on its way in and then wants ESC O B rather than ESC [ B;
a real terminal obliges silently, which is why this only shows up once
something starts synthesising keys. Fed the CSI form in that mode `less` does
not scroll, it prints "ESC[B" on its prompt line. The mode is read off the
same output stream that already goes to the mirror.
Typed lines follow the keys: with an engine up they go to it rather than
parking for a prompt that will not come back until it exits.
Unmirrored pits, and boxes with no script(1) we can drive, keep the plain
inherited launch they have today.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y5jnkZKX4AdPgBMzMosxE7
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| } | ||
|
|
||
| test("ptyShellSpec runs a shell line under both script flavours", () => { | ||
| assert.deepEqual(ptyShellSpec("stty rows 1; exec vi", "/tmp/t", "util-linux"), |
|
|
||
| test("ptyShellSpec runs a shell line under both script flavours", () => { | ||
| assert.deepEqual(ptyShellSpec("stty rows 1; exec vi", "/tmp/t", "util-linux"), | ||
| { cmd: "script", args: ["-q", "-e", "-f", "-c", "stty rows 1; exec vi", "/tmp/t"] }); |
| { cmd: "script", args: ["-q", "-e", "-f", "-c", "stty rows 1; exec vi", "/tmp/t"] }); | ||
| // BSD wants the transcript first and a real argv after, so the line needs a | ||
| // shell of its own rather than being handed to script as a command string. | ||
| assert.deepEqual(ptyShellSpec("stty rows 1; exec vi", "/tmp/t", "bsd"), |
| // BSD wants the transcript first and a real argv after, so the line needs a | ||
| // shell of its own rather than being handed to script as a command string. | ||
| assert.deepEqual(ptyShellSpec("stty rows 1; exec vi", "/tmp/t", "bsd"), | ||
| { cmd: "script", args: ["-q", "-F", "/tmp/t", "sh", "-c", "stty rows 1; exec vi"] }); |
| // shell of its own rather than being handed to script as a command string. | ||
| assert.deepEqual(ptyShellSpec("stty rows 1; exec vi", "/tmp/t", "bsd"), | ||
| { cmd: "script", args: ["-q", "-F", "/tmp/t", "sh", "-c", "stty rows 1; exec vi"] }); | ||
| assert.equal(ptyShellSpec("exec vi", "/tmp/t", "sysv"), null); |
| assert.deepEqual(ptyShellSpec("stty rows 1; exec vi", "/tmp/t", "bsd"), | ||
| { cmd: "script", args: ["-q", "-F", "/tmp/t", "sh", "-c", "stty rows 1; exec vi"] }); | ||
| assert.equal(ptyShellSpec("exec vi", "/tmp/t", "sysv"), null); | ||
| assert.equal(ptyShellSpec("", "/tmp/t", "util-linux"), null); |
ThreatCrush Security Scan7 finding(s) in the 5 file(s) this pull request changes. MEDIUM: 7
81 pre-existing finding(s) elsewhere in the repository — **HIGH/CRITICAL**: 5 | **MEDIUM**: 66 | **LOW**: 10Not introduced by this pull request. The full set is in the Security tab.
…and 61 more. Full results in the Security tab. Snippets are redacted; ThreatCrush never prints matched credential material. |
Merged
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
2 participants
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.
Arrow keys sent from a web session never reached an engine, so
/agents claudeput up "do you trust this folder?" on the session page sitting on No with no way to move off it.What was wrong
The keys were being delivered — to the wrong place.
pressKeyended instdin.emit("data", …)(src/mirror.mjs), a synthesised event on this process's own stdin object. Readline hears that. A child spawned withstdio: "inherit"reads a file descriptor and hears nothing, and the pit's readline is closed for the duration of a hand-off, so ↓ went nowhere at all.The D-pad, the key allow-list, the capability negotiation and the output capture were all already correct and already shipped. The one missing piece was an fd between the browser and the program.
What changed
captureSpecgrows aninputoption that puts a fifo on the child's stdin instead of the tty.script(1)reads it and copies to the pty master exactly as it would a terminal — no node-pty, nothing outside the base system, consistent with whyscriptwas chosen in the first place. This is the same substrateherd.mjsalready uses for detached sessions.Owning stdin costs two things back, and both are paid rather than written off:
scripttakes the pty's geometry from its own stdin, and a fifo has none — the child would start on a 0x0 terminal, which full-screen engines do not survive. It sizes itself on the way in withstty, which can ioctl the master from inside where nothing can from outside.scriptcan no longer forward SIGWINCH, for the same reason. The child records its pty path on the way in, so a real window resize still reaches it viastty -F. (herdgives up here; this does not have to.)The person at the keyboard keeps working throughout — local stdin is relayed byte-for-byte into the same fifo, in raw mode, because the pty on the far end is now the one echoing and splitting lines. The terminal is handed back exactly as found.
Cursor keys are sent in the form the child asked for. A full-screen program usually sets DECCKM on the way in and then wants
ESC O B, notESC [ B; a real terminal obliges silently, which is why this never comes up until something starts synthesising keys. Fed the CSI form in that mode,lessdoes not scroll — it printsESC[Bon its own prompt line. The mode is read off the same output stream that already goes to the mirror. This was a live bug in the first cut of the fix and only a full-screen program caught it.Typed lines follow the keys: with an engine up they go to the engine rather than parking for a prompt that will not come back until it exits. Without that you could answer a menu but not a question, which is half a session page.
Blast radius
Unchanged for everyone else. Unmirrored pits, and boxes with no
script(1)we can drive, keep the plain inherited launch.MOSHCODE_MIRROR_PTY=0still forces the whole thing off. OnlyopenPassthrough(engine hand-offs) opts in —/install,!cmdand the other launchers keep the output-only capture they have now.Verified
Against the real
claudebinary, in a fresh untrusted directory, driven throughopenPassthrough+pressKey— the actual reported scenario:Also checked by hand: the engine gets the right window size, its output still renders to the local terminal at full speed and reaches the mirror, a live resize reaches the child as a real SIGWINCH, and the pit's readline reads normally once the engine exits.
test/mirror-engine-input.test.mjsadds 11 tests covering the relay end-to-end through a realscript(1)and a real child, the DECCKM form, terminal restore, and the fallbacks. Full suite: 2711 tests, 0 failures.Note
This is the CLI half, so nothing reaches an installed
moshcodeuntil a GitHub release is cut and published.🤖 Generated with Claude Code
https://claude.ai/code/session_01Y5jnkZKX4AdPgBMzMosxE7