Skip to content

fix(tui): run every command from piped stdin, not just the first#49

Open
clawedassistant26 wants to merge 1 commit into
moshcoder:mainfrom
clawedassistant26:fix/piped-stdin-runs-only-first-command
Open

fix(tui): run every command from piped stdin, not just the first#49
clawedassistant26 wants to merge 1 commit into
moshcoder:mainfrom
clawedassistant26:fix/piped-stdin-runs-only-first-command

Conversation

@clawedassistant26

Copy link
Copy Markdown
Contributor

The bug

Piping commands into moshcode runs only the first one. Everything after it is silently dropped, /quit included, and the process still exits 0.

On unmodified main (5029919):

$ printf '/pwd\n/pwd\n/pwd\n/quit\n' | node bin/moshcode.mjs
...
mosh ▸   ~/mc-qa/moshcode-run
  repo   moshcode-run on main
  root   ~/mc-qa/moshcode-run
  origin https://github.com/moshcoder/moshcode.git
mosh ▸
$ echo $?
0

One /pwd ran. The other two never did, /quit was never reached, and the code hard, mosh harder. 🤘 farewell never printed. Exit status is 0, so a script has no way to tell.

Why

mkrl() deliberately supports non-TTY input (terminal: Boolean(process.stdin.isTTY)), but the loop asks for lines with a one-shot rl.question. With terminal: false, readline emits every line of an incoming chunk as soon as that chunk lands. rl.question registers a single handler, takes line 1, and the remaining lines are emitted with nobody listening. The loop then awaits input that has already gone by, stdin ends, the promise never settles, the event loop drains and the process exits quietly.

Reduced to plain readline, no moshcode involved:

const rl = readline.createInterface({ input: process.stdin, output: process.stdout, terminal: false });
const ask = () => new Promise((r) => rl.question("> ", r));
for (;;) { const line = await ask(); console.log("GOT:", line); if (line === "quit") break; }
$ printf 'a\nb\nc\nquit\n' | node rl.mjs
> GOT: a
> Warning: Detected unsettled top-level await

Worth noting: the tests in test/tui.test.mjs all pipe "<command>\n/quit\n" and assert status === 0. That assertion passes today for the wrong reason — /quit is never executed, the process just falls off the end.

The fix

src/tui.mjs only. Queue lines as readline emits them and hand them out one at a time.

  • The queue sits at module scope so it survives the interface being torn down and rebuilt around a passthrough child (/shell, /install, /start, …).
  • End of input is tracked with process.stdin.on("end"), not the interface's close, because rl.close() fires on every one of those teardowns and is not the end of the input.
  • Running out of input now leaves the pit the same way Ctrl-D does, so the farewell prints and the loop exits deliberately.
  • The TTY path still goes through rl.question and is unchanged.

Verified after the fix: all three /pwds run and /quit is honoured; /shell echo FIRST/pwd/shell echo SECOND/quit all run in order across two interface rebuilds; input ending without /quit exits cleanly; an interactive pty session behaves as before.

Tests

Two added to test/tui.test.mjs, both driving the real binary through the file's existing runTui helper:

  • every command from piped stdin runs, and /quit is reached
  • piped input ending without /quit still leaves cleanly

Stash-verified with git stash push -- src/: both fail unpatched (9 pass / 2 fail), 11/11 with the fix.

Full suite node --test: 166 tests, 163 pass, 0 fail, 3 skipped (main baseline: 164 / 161 / 0 fail / 3 skipped).

Piped stdin is delivered in chunks and readline emits every line of a
chunk as soon as it lands. The prompt loop asked for input with a
one-shot `rl.question`, so it captured the first line and dropped the
rest, then waited forever on input that had already gone by.

    printf '/pwd\n/pwd\n/quit\n' | moshcode

ran one /pwd, skipped the second, never reached /quit, and still exited
0 with no farewell. Anything driving moshcode from a script or CI lost
every command after the first, silently.

Queue the lines as they arrive and hand them out one at a time. The
queue lives at module scope so it survives the interface being torn
down and rebuilt around a passthrough child, and end of input is
tracked on process.stdin rather than the interface, since rl.close()
fires on every teardown. End of input now leaves the pit the way Ctrl-D
does. The TTY path is untouched.
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