review followups: pcall shield, :resume guard, shell quoting, nits
CONCERNs from the Phase 1 review pass:
ffi/curl.lua:
- SSE write_cb body is now pcall-wrapped. A Lua error in on_event (or
in the parse loop itself) is captured into cb_error and surfaced
after curl_easy_perform rather than propagating across the FFI
callback boundary (which LuaJIT documents as process-fatal). The
EOS flush path gets the same shield. Errors return
(nil, "callback: <msg>") from post_sse.
history.lua:
- sh_singlequote() escapes shell metacharacters; the mkdir -p and
ls -1 shell-outs no longer double-quote (where $(...) and $VAR
still expand) — single-quote with embedded-' escaping is the
safe form.
- M.load now returns (turns, meta) instead of (meta, turns). turns
is ALWAYS a table on success, never nil-when-no-header; failure
path is the unambiguous (nil, err). Callers can `if not turns
then` without the previous ambiguity. repl.lua :resume updated
to the new shape.
repl.lua :resume:
- Refuse to resume into a non-empty ctx — silent overwrite was the
Q15 default, but the review surfaced the no-undo / no-warning
failure mode. User must :reset (or :save then re-launch) to
express intent. The current session's on-disk log is unaffected
either way.
NITs:
- ffi/libc.lua READ_BUF: comment noting it's module-shared and
Phase 1 has no reentrant readers; revisit when that changes.
- PHASE1.md §7: \C-x\C-c reservation pinned to Phase 3 ("deferred
from Phase 1 — no consumer here") rather than the previous
dangling "(or here)".
Regression suite verifies:
- history.load new signature on success + failure paths
- shell-quoted history.dir with $ doesn't trip
- aish scripted run: ctx with 2 turns refuses :resume anchor with
a clear status; user must :reset first
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -230,16 +230,24 @@ function M.run(config)
|
||||
local name = args:match("^%s*(%S+)")
|
||||
if not name then renderer.status("usage: :resume <name>"); return end
|
||||
if not sessions_dir then renderer.status("(no history.dir configured)"); return end
|
||||
-- Refuse to silently clobber an active conversation; the user has
|
||||
-- to :reset first to express intent. The current session log on
|
||||
-- disk is unaffected by either choice.
|
||||
if #ctx.turns > 0 then
|
||||
renderer.status(("resume into non-empty ctx refused (%d turns); :reset first")
|
||||
:format(#ctx.turns))
|
||||
return
|
||||
end
|
||||
name = name:gsub("%.jsonl$", "")
|
||||
local path = sessions_dir .. "/" .. name .. ".jsonl"
|
||||
local meta_hdr, turns = history.load(path)
|
||||
if not (meta_hdr or turns) then
|
||||
local turns, _meta_hdr = history.load(path)
|
||||
if not turns then
|
||||
renderer.status("resume failed: cannot load " .. path)
|
||||
return
|
||||
end
|
||||
ctx:reset()
|
||||
for _, t in ipairs(turns or {}) do ctx:append(t) end
|
||||
renderer.status(("resumed %d turns from %s"):format(#(turns or {}), name))
|
||||
for _, t in ipairs(turns) do ctx:append(t) end
|
||||
renderer.status(("resumed %d turns from %s"):format(#turns, name))
|
||||
end,
|
||||
help = function() io.write(HELP) end,
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user