From ee4d7f86d65a48afe523883be8c1ad24338e5148 Mon Sep 17 00:00:00 2001 From: Markus Fritsche Date: Sun, 10 May 2026 19:08:27 +0000 Subject: [PATCH] executor: swap popen+sentinel for pty.spawn (Phase 1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the Phase 0 io.popen + sentinel-echo exit-code recovery with forkpty + waitpid via ffi/pty. The §7 amendment paragraph on PHASE0.md is rewritten to point at PHASE1.md §5 — the workaround is gone, not just renamed. User-visible behavioral changes: - Interactive commands (vim, less, htop, top) now work via $cmd / :exec / known-command shell paths because the child has a real PTY for line discipline. - Exit codes are accurate: `false` -> 1, `exit 7` -> 7, signal kill -> 128+N (bash convention), shell parse error -> sh's 2. - Broken-shell-syntax cmd now shows the actual sh diagnostic (e.g. "Syntax error: end of file unexpected") instead of Phase 0's "(no output — possible shell parse error)" guess. - Output normalization: PTY emits CR LF; executor collapses \r\n -> \n to keep the Phase 0 contract ("output uses \n separators"). Code path: pty.spawn(cmd) -> drain master_fd until EOF -> wait() returns ("exit", N) | ("signal", N) | ... -> exit_code mapped: exit -> N, signal -> 128+N, else -1 Phase 0 invariants intact: `cd` interception unchanged (still libc.chdir per §3 + §7), `CMD: ` extraction unchanged. PHASE0.md §7: the "LuaJIT 2.1 popen-close caveat" paragraph is rewritten to "Superseded by Phase 1" — points at PHASE1.md §5 for the live model. The illustrative sketch is left in place as historical context. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/PHASE0.md | 15 +++++++------- executor.lua | 54 ++++++++++++++++++++++++++++++-------------------- 2 files changed, 41 insertions(+), 28 deletions(-) diff --git a/docs/PHASE0.md b/docs/PHASE0.md index 16db635..e338586 100644 --- a/docs/PHASE0.md +++ b/docs/PHASE0.md @@ -171,13 +171,14 @@ local function exec(cmd) end ``` -**LuaJIT 2.1 popen-close caveat.** The sketch above assumes Lua 5.2's -three-return `io.popen():close()` shape. LuaJIT 2.1 follows the Lua 5.1 -ABI and returns just `true` — no exit status. The Phase 0 implementation -recovers the exit code by appending a sentinel echo to the wrapped -command (`(cmd) 2>&1; echo __AISH_EXIT___$?`) and parsing it back -out. Phase 1's PTY work replaces this with `waitpid` via libc FFI; the -sketch becomes accurate at that point. +**Superseded by Phase 1.** The §7 sketch was never quite accurate on +LuaJIT 2.1 (which follows the Lua 5.1 ABI for `io.popen():close()` and +returns only `true` — no exit status). The Phase 0 implementation worked +around this with a sentinel-echo wrapper (`(cmd) 2>&1; echo +__AISH_EXIT___$?`) and parsed the status back out of stdout. Phase 1 +retired the workaround entirely: `executor.lua` now spawns the child via +`forkpty` and recovers exit status via `waitpid(WEXITSTATUS)`. See +docs/PHASE1.md §5 for the current PTY model. Output is captured and: 1. Printed to the terminal diff --git a/executor.lua b/executor.lua index 856f421..3a8ec88 100644 --- a/executor.lua +++ b/executor.lua @@ -1,38 +1,50 @@ -- executor.lua — command execution. --- Phase 0: io.popen with stderr merged. PTY (forkpty) lands in Phase 1. --- `cd` is intercepted before popen and routed through libc.chdir so the --- working directory persists across calls (popen forks; cd inside it would --- otherwise be discarded). See docs/PHASE0.md §6, §7. +-- Phase 1: forkpty via ffi/pty. Replaces Phase 0's io.popen + sentinel-echo +-- exit-code workaround; vim/less/htop now work because the child runs on a +-- real PTY. `cd` interception is unchanged (still libc.chdir per §3, §7). +-- See docs/PHASE0.md §7 and docs/PHASE1.md §5. local libc = require("ffi.libc") +local pty = require("ffi.pty") local M = {} --- LuaJIT 2.1's io.popen():close() returns only `true` (Lua 5.1 ABI) — it --- does not surface the child exit status. Recover it via a sentinel echo --- appended after the command. Phase 1's PTY work will wire waitpid via FFI --- and replace this hack. -local EXIT_SENTINEL = "__AISH_EXIT_4F8E91__" - -- Execute a shell command. -- Returns: (output_string, exit_code). --- exit_code == 0 on success; non-zero on failure; -1 on no-output / sentinel- --- parse failure (popen failed, empty cmd, shell parse error, sentinel collision). +-- 0 success +-- 1..255 child exited with that status +-- 128+N child killed by signal N (bash convention) +-- -1 forkpty / spawn / wait failure function M.exec(cmd) if not cmd or cmd:match("^%s*$") then return "(empty command)", -1 end - local wrapped = string.format("(%s) 2>&1; echo %s$?", cmd, EXIT_SENTINEL) - local handle, err = io.popen(wrapped, "r") - if not handle then return ("popen failed: " .. tostring(err)), -1 end - local output = handle:read("*a") or "" - handle:close() - local body, code = output:match("^(.-)" .. EXIT_SENTINEL .. "(%-?%d+)%s*$") - if code then return body, tonumber(code) end - if output == "" then - return "(no output — possible shell parse error)", -1 + local sess, err = pty.spawn(cmd) + if not sess then + return "(pty.spawn failed: " .. tostring(err) .. ")", -1 end + + -- Drain until the child closes its end. PTY combines stdout+stderr + -- 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() + sess:close() + + -- 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") + -- still holds. + local output = table.concat(chunks):gsub("\r\n", "\n") + + if kind == "exit" then return output, code end + if kind == "signal" then return output, 128 + code end return output, -1 end