From abc993aa49cde48964cfed17243aeccb242aa6f8 Mon Sep 17 00:00:00 2001 From: Markus Fritsche Date: Sun, 10 May 2026 17:41:35 +0000 Subject: [PATCH] review followup: empty-input guards, ~/ symmetry, CMD: filter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses three concerns + one nit from the Phase 0 review pass. executor.lua: - M.exec guards empty / whitespace-only cmd up front, returns "(empty command)" / -1 instead of running the wrapper on nothing. - On sentinel-parse failure with empty output (typical of shell parse errors — the syntax error itself escapes to the popen parent's stderr because 2>&1 is inside the unparsable subshell), surface "(no output — possible shell parse error)" rather than a silent empty frame. - extract_cmd_lines now skips whitespace-only / empty bodies; a bare `CMD: ` line in assistant output no longer turns into an "execute ''? [y/N]" prompt. - "what" comments cleaned in maybe_chdir. router.lua: - path_like now matches `~` and `~/foo` so `~/scripts/build.sh` classifies as shell (was: ai). Restores symmetry with executor's maybe_chdir, which already expands `~` on `cd`. repl.lua: - :exec and :ask trim args and renderer.status a usage line on empty rather than running an empty cmd / sending an empty turn to broker. Regression: full prior smoke suite still passes — known_commands shell paths, all maybe_chdir branches, CMD: extraction with non-empty bodies, exec exit-code recovery, all router branches. Co-Authored-By: Claude Opus 4.7 (1M context) --- executor.lua | 16 +++++++++++----- repl.lua | 14 +++++++++++--- router.lua | 9 ++++++--- 3 files changed, 28 insertions(+), 11 deletions(-) diff --git a/executor.lua b/executor.lua index 50d031c..856f421 100644 --- a/executor.lua +++ b/executor.lua @@ -16,9 +16,12 @@ 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 if the sentinel could --- not be parsed (e.g. popen itself failed, or output collided with the tag). +-- 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). 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 @@ -27,6 +30,9 @@ function M.exec(cmd) 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 + end return output, -1 end @@ -40,10 +46,9 @@ function M.maybe_chdir(cmd) or cmd:match("^%s*cd%s+(.+)$") if not rest then return nil end - -- trim local target = rest:match("^%s*(.-)%s*$") or "" - -- defaults + ~ expansion (Phase 0: no $OLDPWD / `cd -`) + -- Phase 0: no $OLDPWD support, so `cd -` is not handled. if target == "" then target = os.getenv("HOME") or "/" end if target == "~" then target = os.getenv("HOME") or "/" end if target:sub(1, 2) == "~/" then @@ -60,7 +65,8 @@ function M.extract_cmd_lines(text) local cmds = {} for line in (text or ""):gmatch("[^\n]+") do local cmd = line:match("^CMD: (.*)$") - if cmd then cmds[#cmds + 1] = cmd end + -- Skip whitespace-only / empty bodies; "CMD: " alone is degenerate. + if cmd and cmd:match("%S") then cmds[#cmds + 1] = cmd end end return cmds end diff --git a/repl.lua b/repl.lua index 70c18a8..e10c5f0 100644 --- a/repl.lua +++ b/repl.lua @@ -126,9 +126,17 @@ function M.run(config) i, t.role, t.content:gsub("\n", " "))) end end, - exec = function(args) run_shell(args) end, - ask = function(args) ask_ai(args) end, - help = function() io.write(HELP) end, + exec = function(args) + args = (args or ""):match("^%s*(.-)%s*$") + if args == "" then renderer.status("usage: :exec "); return end + run_shell(args) + end, + ask = function(args) + args = (args or ""):match("^%s*(.-)%s*$") + if args == "" then renderer.status("usage: :ask "); return end + ask_ai(args) + end, + help = function() io.write(HELP) end, } -- Main loop. diff --git a/router.lua b/router.lua index 9d44f5d..6c9f7fc 100644 --- a/router.lua +++ b/router.lua @@ -25,11 +25,14 @@ local function known_commands_set(config) return set end --- §5.1 path-like: ./foo, ../foo, /usr/bin/foo. Quoted/escaped paths are --- intentionally out of scope in Phase 0. +-- §5.1 path-like: ./foo, ../foo, /usr/bin/foo, ~/foo, bare ~. Quoted / +-- escaped paths are intentionally out of scope in Phase 0. ~ is included +-- for symmetry with executor.maybe_chdir, which expands ~ on `cd ~/foo`. local function path_like(token) - return token:sub(1, 1) == "/" + return token == "~" + or token:sub(1, 1) == "/" or token:sub(1, 2) == "./" + or token:sub(1, 2) == "~/" or token:sub(1, 3) == "../" end