review BLOCKER: PTY input forwarding + raw mode toggle
Phase 1 review caught a structural gap: executor.exec only drained the
PTY master fd, never forwarded user keystrokes — vim/less/htop/nano
would render and hang on input. PHASE1.md §5 specified bidirectional
multiplex but only the read leg landed. tcgetattr/tcsetattr were also
missing, so even with input forwarding the parent's line discipline
would buffer until newline (breaking single-key UIs).
ffi/libc:
- struct termios opaque buffer + tcgetattr/tcsetattr + cfmakeraw
- M.set_raw(fd) saves termios + applies cfmakeraw; returns saved or
(nil, err) when fd isn't a tty (scripted / piped-stdin runs)
- M.restore_termios(fd, saved)
- struct pollfd + M.poll (POLLIN constant)
executor:
- multiplex(sess): poll(stdin, master); reads master on any revents
(POLLHUP fires when child closes its slave end, not POLLIN — the
revents != 0 check catches both); forwards stdin keystrokes to
master; loop exits when master read returns 0 (EOF / child gone)
- stdin polling is only enabled when stdin_is_tty (set_raw succeeded);
piped-stdin runs (tests / scripted) would otherwise drain queued
aish commands into the child of the *current* cmd, swallowing them
- raw mode is restored before returning so the user lands back at the
aish prompt in canonical mode
renderer + repl:
- exec_output(out, code) split into exec_begin() (top rule, before
spawn) + exec_end(code) (closing rule with exit, after wait). PTY
multiplex streams the body live to stdout in between; the renderer
never re-prints the body.
PHASE1.md §3:
- tcgetattr/tcsetattr changed from "optional" to "required for
single-key UIs to work — done-criteria #2"; poll added to the libc
row description.
Verified:
- non-interactive smoke (echo / false / exit 7 / ls /nonexistent /
printf multi-line) — all exit codes correct, output streamed live,
a\nb\nc\n preserved byte-for-byte
- scripted-stdin run reaches all expected lines (no stdin draining
into a non-interactive child)
- aish prompt + framed exec block + exit-code line all render in
correct order
Live interactive verification (vim / less / htop in a real terminal)
still needs a user-test pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
+1
-1
@@ -63,7 +63,7 @@ No new module file names beyond the §4 stubs already present (`ffi/pty.lua`,
|
|||||||
|---|---|---|
|
|---|---|---|
|
||||||
| `ffi/curl.lua` | Blocking POST; response captured into a Lua string | Add `M.post_sse(url, body, headers, on_event)`. `on_event(delta)` is called per parsed SSE `data:` line. The Phase 0 `M.post` stays for non-streaming consumers. |
|
| `ffi/curl.lua` | Blocking POST; response captured into a Lua string | Add `M.post_sse(url, body, headers, on_event)`. `on_event(delta)` is called per parsed SSE `data:` line. The Phase 0 `M.post` stays for non-streaming consumers. |
|
||||||
| `ffi/pty.lua` | Stub | Implement: `M.spawn(argv) -> handle`; handle exposes `:read()`, `:write(data)`, `:close()`, `:wait() -> exit_code`. Uses `forkpty` + `waitpid`. |
|
| `ffi/pty.lua` | Stub | Implement: `M.spawn(argv) -> handle`; handle exposes `:read()`, `:write(data)`, `:close()`, `:wait() -> exit_code`. Uses `forkpty` + `waitpid`. |
|
||||||
| `ffi/libc.lua` | `chdir`, `errno`, `strerror` | Add `waitpid`, `WEXITSTATUS` (macro materialized in Lua), `read`, `write`, `close`, `kill`, optional `tcgetattr`/`tcsetattr` for raw-mode toggle on the controlling tty. |
|
| `ffi/libc.lua` | `chdir`, `errno`, `strerror` | Add `waitpid`, `WEXITSTATUS` (macro materialized in Lua), `read`, `write`, `close`, `kill`, `tcgetattr`/`tcsetattr` + `cfmakeraw` for raw-mode toggle on the controlling tty (required for single-key UIs to work — done-criteria #2), `poll` for stdin↔master multiplex in executor. |
|
||||||
| `ffi/readline.lua` | `readline`, `add_history` | Add `rl_bind_keyseq` binding; expose `M.bind(seq, fn)`. |
|
| `ffi/readline.lua` | `readline`, `add_history` | Add `rl_bind_keyseq` binding; expose `M.bind(seq, fn)`. |
|
||||||
| `broker.lua` | `M.chat(cfg, msgs)` blocking | Add `M.chat_stream(cfg, msgs, on_delta)`. `M.chat` becomes a thin wrapper that buffers deltas. |
|
| `broker.lua` | `M.chat(cfg, msgs)` blocking | Add `M.chat_stream(cfg, msgs, on_delta)`. `M.chat` becomes a thin wrapper that buffers deltas. |
|
||||||
| `executor.lua` | `popen` + sentinel exit-code recovery + `cd` interception + `CMD:` extract | Replace popen path with `pty.spawn`. The sentinel hack is deleted. `cd` interception unchanged (still routes through `libc.chdir`). `CMD:` extract unchanged. |
|
| `executor.lua` | `popen` + sentinel exit-code recovery + `cd` interception + `CMD:` extract | Replace popen path with `pty.spawn`. The sentinel hack is deleted. `cd` interception unchanged (still routes through `libc.chdir`). `CMD:` extract unchanged. |
|
||||||
|
|||||||
+68
-14
@@ -1,14 +1,77 @@
|
|||||||
-- executor.lua — command execution.
|
-- executor.lua — command execution.
|
||||||
-- Phase 1: forkpty via ffi/pty. Replaces Phase 0's io.popen + sentinel-echo
|
-- Phase 1: forkpty via ffi/pty + bidirectional multiplex. Replaces Phase 0's
|
||||||
-- exit-code workaround; vim/less/htop now work because the child runs on a
|
-- io.popen + sentinel-echo workaround. The multiplex loop forwards stdin
|
||||||
-- real PTY. `cd` interception is unchanged (still libc.chdir per §3, §7).
|
-- keystrokes to the child master fd while streaming master output to stdout,
|
||||||
|
-- so vim / less / htop / nano are usable end-to-end. Parent's tty (fd 0) is
|
||||||
|
-- flipped to raw mode for the duration so single-key UIs work.
|
||||||
|
-- `cd` interception is unchanged (still libc.chdir per §3, §7).
|
||||||
-- See docs/PHASE0.md §7 and docs/PHASE1.md §5.
|
-- See docs/PHASE0.md §7 and docs/PHASE1.md §5.
|
||||||
|
|
||||||
|
local ffi = require("ffi")
|
||||||
|
local bit = require("bit")
|
||||||
local libc = require("ffi.libc")
|
local libc = require("ffi.libc")
|
||||||
local pty = require("ffi.pty")
|
local pty = require("ffi.pty")
|
||||||
|
|
||||||
local M = {}
|
local M = {}
|
||||||
|
|
||||||
|
local pollfd_arr2 = ffi.typeof("struct pollfd[2]")
|
||||||
|
|
||||||
|
-- Multiplex stdin (fd 0) <-> sess.master_fd until the child writes EOF.
|
||||||
|
-- Output is streamed live to stdout AND collected for the (output, code)
|
||||||
|
-- return so context.append_exec_output still has the body to inject into
|
||||||
|
-- the next user turn.
|
||||||
|
local function multiplex(sess)
|
||||||
|
local saved_termios = libc.set_raw(0) -- nil if stdin isn't a tty
|
||||||
|
local stdin_is_tty = (saved_termios ~= nil)
|
||||||
|
|
||||||
|
local fds = pollfd_arr2()
|
||||||
|
-- Only poll stdin when it's a tty. With piped stdin (scripted runs /
|
||||||
|
-- tests), aish's stdin holds the *next* aish commands queued for the
|
||||||
|
-- repl loop — draining it into the child would swallow those.
|
||||||
|
fds[0].fd = stdin_is_tty and 0 or -1
|
||||||
|
fds[0].events = libc.POLLIN
|
||||||
|
fds[1].fd = sess.master_fd
|
||||||
|
fds[1].events = libc.POLLIN
|
||||||
|
|
||||||
|
local chunks = {}
|
||||||
|
while true do
|
||||||
|
fds[0].revents = 0
|
||||||
|
fds[1].revents = 0
|
||||||
|
local rc = libc.poll(fds, 2, -1)
|
||||||
|
if rc < 0 then
|
||||||
|
if libc.errno() == libc.EINTR then
|
||||||
|
-- signal during poll; loop and retry
|
||||||
|
else
|
||||||
|
break
|
||||||
|
end
|
||||||
|
else
|
||||||
|
-- Drain master first (output priority). Read on *any* revents —
|
||||||
|
-- POLLHUP fires (and POLLIN doesn't) when the child closes its
|
||||||
|
-- slave PTY end on exit; reading then returns 0 = EOF.
|
||||||
|
if fds[1].revents ~= 0 then
|
||||||
|
local data, n = sess:read()
|
||||||
|
if not data or n == 0 then break end
|
||||||
|
chunks[#chunks + 1] = data
|
||||||
|
io.write(data); io.flush()
|
||||||
|
end
|
||||||
|
-- Forward stdin keystrokes (or piped-in bytes) to the child.
|
||||||
|
if fds[0].revents ~= 0 then
|
||||||
|
local input, n = libc.read(0, 4096)
|
||||||
|
if input and n > 0 then
|
||||||
|
sess:write(input)
|
||||||
|
elseif input == "" then
|
||||||
|
-- aish's own stdin closed; stop forwarding but keep
|
||||||
|
-- draining master until child exits
|
||||||
|
fds[0].fd = -1
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
if saved_termios then libc.restore_termios(0, saved_termios) end
|
||||||
|
return chunks
|
||||||
|
end
|
||||||
|
|
||||||
-- Execute a shell command.
|
-- Execute a shell command.
|
||||||
-- Returns: (output_string, exit_code).
|
-- Returns: (output_string, exit_code).
|
||||||
-- 0 success
|
-- 0 success
|
||||||
@@ -25,22 +88,13 @@ function M.exec(cmd)
|
|||||||
return "(pty.spawn failed: " .. tostring(err) .. ")", -1
|
return "(pty.spawn failed: " .. tostring(err) .. ")", -1
|
||||||
end
|
end
|
||||||
|
|
||||||
-- Drain until the child closes its end. PTY combines stdout+stderr
|
local chunks = multiplex(sess)
|
||||||
-- on the master fd (no 2>&1 needed); CR LF gets normalized below.
|
|
||||||
local chunks = {}
|
|
||||||
while true do
|
|
||||||
local data, n = sess:read()
|
|
||||||
if not data then break end
|
|
||||||
if n == 0 then break end
|
|
||||||
chunks[#chunks + 1] = data
|
|
||||||
end
|
|
||||||
|
|
||||||
local kind, code = sess:wait()
|
local kind, code = sess:wait()
|
||||||
sess:close()
|
sess:close()
|
||||||
|
|
||||||
-- PTY line discipline emits \r\n for every \n the child writes; collapse
|
-- PTY line discipline emits \r\n for every \n the child writes; collapse
|
||||||
-- back to \n so the Phase 0 caller contract ("output uses \n separators")
|
-- back to \n so the Phase 0 caller contract ("output uses \n separators")
|
||||||
-- still holds.
|
-- still holds for context-injection purposes.
|
||||||
local output = table.concat(chunks):gsub("\r\n", "\n")
|
local output = table.concat(chunks):gsub("\r\n", "\n")
|
||||||
|
|
||||||
if kind == "exit" then return output, code end
|
if kind == "exit" then return output, code end
|
||||||
|
|||||||
@@ -21,6 +21,18 @@ ssize_t read (int fd, void *buf, size_t count);
|
|||||||
ssize_t write (int fd, const void *buf, size_t count);
|
ssize_t write (int fd, const void *buf, size_t count);
|
||||||
int close (int fd);
|
int close (int fd);
|
||||||
int kill (pid_t pid, int sig);
|
int kill (pid_t pid, int sig);
|
||||||
|
|
||||||
|
/* termios for raw-mode toggle around interactive PTY children. The struct
|
||||||
|
is treated as opaque — cfmakeraw fills it; size 64 is comfortably larger
|
||||||
|
than glibc's struct termios (60 bytes) on aarch64/x86_64 Linux. */
|
||||||
|
struct termios { char _opaque[64]; };
|
||||||
|
int tcgetattr(int fd, struct termios *tio);
|
||||||
|
int tcsetattr(int fd, int actions, const struct termios *tio);
|
||||||
|
void cfmakeraw(struct termios *tio);
|
||||||
|
|
||||||
|
/* poll for stdin↔master multiplex in executor. */
|
||||||
|
struct pollfd { int fd; short events; short revents; };
|
||||||
|
int poll(struct pollfd *fds, unsigned long nfds, int timeout);
|
||||||
]]
|
]]
|
||||||
|
|
||||||
local C = ffi.C
|
local C = ffi.C
|
||||||
@@ -106,4 +118,37 @@ function M.kill(pid, sig)
|
|||||||
return false, ffi.string(C.strerror(C.__errno_location()[0]))
|
return false, ffi.string(C.strerror(C.__errno_location()[0]))
|
||||||
end
|
end
|
||||||
|
|
||||||
|
-- ---------------------------------------------------------------- termios
|
||||||
|
-- Save current tty mode and switch to raw via cfmakeraw. Returns the saved
|
||||||
|
-- termios pointer (to be passed back to M.restore_termios) or (nil, err) if
|
||||||
|
-- fd isn't a tty (e.g. stdin redirected from a file in CI / scripted runs).
|
||||||
|
local TCSANOW = 0
|
||||||
|
|
||||||
|
function M.set_raw(fd)
|
||||||
|
local saved = ffi.new("struct termios")
|
||||||
|
if C.tcgetattr(fd, saved) < 0 then
|
||||||
|
return nil, M.strerror(M.errno())
|
||||||
|
end
|
||||||
|
local raw = ffi.new("struct termios")
|
||||||
|
ffi.copy(raw, saved, ffi.sizeof("struct termios"))
|
||||||
|
C.cfmakeraw(raw)
|
||||||
|
if C.tcsetattr(fd, TCSANOW, raw) < 0 then
|
||||||
|
return nil, M.strerror(M.errno())
|
||||||
|
end
|
||||||
|
return saved
|
||||||
|
end
|
||||||
|
|
||||||
|
function M.restore_termios(fd, saved)
|
||||||
|
return C.tcsetattr(fd, TCSANOW, saved) == 0
|
||||||
|
end
|
||||||
|
|
||||||
|
-- ---------------------------------------------------------------- poll
|
||||||
|
M.POLLIN = 0x0001
|
||||||
|
M.EINTR = 4
|
||||||
|
|
||||||
|
-- Returns: rc (>= 0 fds ready, 0 timeout, -1 error)
|
||||||
|
function M.poll(fds_arr, nfds, timeout_ms)
|
||||||
|
return C.poll(fds_arr, nfds, timeout_ms or -1)
|
||||||
|
end
|
||||||
|
|
||||||
return M
|
return M
|
||||||
|
|||||||
+8
-5
@@ -31,12 +31,15 @@ function M.assistant(text)
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
-- Frame captured shell-exec output with a top rule + the body + a closing
|
-- Phase 1: executor.exec streams output live to stdout (PTY multiplex), so
|
||||||
-- rule that carries the exit code (red on non-zero).
|
-- the frame is split — exec_begin before the spawn, exec_end after wait().
|
||||||
function M.exec_output(output, exit_code)
|
-- The body is not re-rendered here; live output lands directly between the
|
||||||
output = (output or ""):gsub("\n$", "")
|
-- two rules.
|
||||||
|
function M.exec_begin()
|
||||||
emit(A.dim, "─── exec output ───", A.reset, "\n")
|
emit(A.dim, "─── exec output ───", A.reset, "\n")
|
||||||
if output ~= "" then emit(output, "\n") end
|
end
|
||||||
|
|
||||||
|
function M.exec_end(exit_code)
|
||||||
if exit_code and exit_code ~= 0 then
|
if exit_code and exit_code ~= 0 then
|
||||||
emit(A.dim, "─── exit ", A.reset,
|
emit(A.dim, "─── exit ", A.reset,
|
||||||
A.red, tostring(exit_code), A.reset,
|
A.red, tostring(exit_code), A.reset,
|
||||||
|
|||||||
@@ -96,8 +96,9 @@ function M.run(config)
|
|||||||
end
|
end
|
||||||
return
|
return
|
||||||
end
|
end
|
||||||
|
renderer.exec_begin()
|
||||||
local out, code = executor.exec(cmd)
|
local out, code = executor.exec(cmd)
|
||||||
renderer.exec_output(out, code)
|
renderer.exec_end(code)
|
||||||
if config.shell and config.shell.capture_output then
|
if config.shell and config.shell.capture_output then
|
||||||
ctx:append_exec_output(out)
|
ctx:append_exec_output(out)
|
||||||
end
|
end
|
||||||
|
|||||||
Reference in New Issue
Block a user