From d67d30bb640eab8eca724d284397a71b3fb437e8 Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Mon, 13 Jul 2026 16:03:09 +0100 Subject: [PATCH] =?UTF-8?q?fix(compile):=20PR=20#113=20round=201=20?= =?UTF-8?q?=E2=80=94=20coordinates,=20recovery,=20rules,=20types,=20EOF?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Finding-by-finding (framing revision 7; every fix bite-verified via scripts/bite against the pre-fix tree): 1. Stored coordinates must be finite integers, and both cursor walks are movement-bounded — they clamp at EOF, and the column walk clamps at the target row's EOL instead of marching onto later rows. An astronomical %d+ capture can no longer hang the editor. 2. The grep panel gains the same immediate buffer.after-edit recovery trigger as the compile slots: M-x buffer.undo after a COMPLETED search is marked synchronously. 3. The rustc arrow rule uses the framing's ([^:]+) spelling — paths with spaces capture whole. 4. All pattern captures are collected (index 4+ reads the real capture, not nil-as-column-0); capture indexes must be positive integers; a rule naming a column its match didn't produce rejects the match. 5. emit_text_raw is module-local — a user global could shadow the helper the terminal-event path depends on, and its error consumed the terminal event before pump cleanup/forget ran. 6. stdin/group spec fields reject wrong Lua types as hard errors; group is matched as a raw Value because mlua's bool conversion applies Lua truthiness ("true" would silently coerce). 7. resync also nils the public line_start_byte — total pre-marker anchor invalidation includes the byte anchor. 8. The inherited cwd resolves through pmacs.instance.identity().working_directory; the header always names a real path and relative error files get an explicit base. 9. New AnsiParser::finish() + parser:finish() (additions #5): a truncated multibyte sequence at process EOF surfaces as U+FFFD before the exit marker instead of vanishing. 10. The built-in default rules are a private deep copy — in-place mutations of the public table no longer survive the "using built-in defaults" degradation. Eleven new tests (r1f1a/b–r1f10); bites: 9 fail against pre-fix compile.lua, r1f2 against pre-fix default.lua, r1f6 against pre-fix lua_bindings/mod.rs — all clean assertion failures. Gates: fmt, clippy workspace all-targets, lib 1522, crdt lib 1696, compile acceptance 45, crdt acceptance 1, m4 101, GPU 59, workspace sweep 2493/0, git diff --check. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01VoiEyuPjoBhvwACf8HAnLB --- builtin/commands/default.lua | 34 +++- builtin/runtime/compile.lua | 140 ++++++++++++---- docs/compile-mode-framing.md | 99 ++++++++--- src/ansi.rs | 20 +++ src/lua_bindings/mod.rs | 46 ++++-- tests/compile_mode_acceptance.rs | 275 +++++++++++++++++++++++++++++++ 6 files changed, 551 insertions(+), 63 deletions(-) diff --git a/builtin/commands/default.lua b/builtin/commands/default.lua index 7baa098..7b761b9 100644 --- a/builtin/commands/default.lua +++ b/builtin/commands/default.lua @@ -773,14 +773,44 @@ local function ensure_search_panel() return p end +-- Immediate command-path recovery for the panel (the Q#CM2 trigger +-- the compile slots already have): M-x/menu edits fire +-- buffer.after-edit, and after a COMPLETED search no producer write +-- or navigation may ever come — without this, an M-x buffer.undo +-- left corrupted output unmarked indefinitely (PR #113 round-1 +-- finding 2). Hook edits don't re-fire the hook, so the resync +-- marker can be appended from here safely. +pmacs.hook.add("buffer.after-edit", function() + local cur = pmacs.window.buffer() + if cur and search_panel_alive() and cur == search_panel.buf then + search_panel_check_rev() + end +end) + -- Cursor walk via primitives (the lsp.lua visit idiom; 0-based). +-- Movement-bounded like compile.lua's walk: a match pointing past +-- EOF/EOL clamps instead of looping (the file may have changed on +-- disk since the worker scanned it). local function search_move_cursor_to(line, col) pmacs.editor.move_line_start() while pmacs.editor.cursor_line() > 0 do pmacs.editor.move_up() end - for _ = 1, line do pmacs.editor.move_down() end - for _ = 1, col do pmacs.editor.move_right() end + for _ = 1, line do + local before = pmacs.editor.cursor_line() + pmacs.editor.move_down() + if pmacs.editor.cursor_line() == before then break end + end + local row = pmacs.editor.cursor_line() + for _ = 1, col do + local before = pmacs.editor.cursor() + pmacs.editor.move_right() + if pmacs.editor.cursor() == before then break end + if pmacs.editor.cursor_line() ~= row then + pmacs.editor.move_left() + break + end + end end local function visit_match(idx) diff --git a/builtin/runtime/compile.lua b/builtin/runtime/compile.lua index 2e0840a..07f4565 100644 --- a/builtin/runtime/compile.lua +++ b/builtin/runtime/compile.lua @@ -79,8 +79,10 @@ pmacs.command.define { -- `severity` is an override; nil falls back to keyword sniffing on -- the matched line. User-extensible from init.lua. pmacs.compile.rules = { - -- rustc/cargo arrow lines: " --> src/foo.rs:12:4" - { pattern = "%-%->%s+([^:%s]+):(%d+):(%d+)", file = 1, line = 2, col = 3 }, + -- rustc/cargo arrow lines: " --> src/foo.rs:12:4". `[^:]+` (the + -- framing's spelling), NOT `[^:%s]+` — paths may contain spaces + -- (PR #113 round-1 finding 3). + { pattern = "%-%->%s+([^:]+):(%d+):(%d+)", file = 1, line = 2, col = 3 }, -- gcc/clang/grep-format: "file:line:col:" (also matches most Unix tools) { pattern = "([^%s:][^:]*):(%d+):(%d+):", file = 1, line = 2, col = 3 }, -- Python tracebacks: 'File "foo.py", line 12' @@ -89,6 +91,13 @@ pmacs.compile.rules = { { pattern = "([^%s:][^:]*):(%d+):", file = 1, line = 2 }, } +-- A capture index must be a positive INTEGER (a fractional index +-- silently reads a neighbouring capture via Lua table coercion — +-- round-1 finding 4). +local function is_capture_index(v) + return type(v) == "number" and v >= 1 and v == math.floor(v) +end + local function rule_is_valid(rule) if type(rule) ~= "table" then return false end if type(rule.pattern) ~= "string" then return false end @@ -96,9 +105,9 @@ local function rule_is_valid(rule) -- pattern is caught (and counted in the status note) here at -- validation time, not silently at match time. if not pcall(string.match, "", rule.pattern) then return false end - if type(rule.file) ~= "number" or rule.file < 1 then return false end - if type(rule.line) ~= "number" or rule.line < 1 then return false end - if rule.col ~= nil and (type(rule.col) ~= "number" or rule.col < 1) then return false end + if not is_capture_index(rule.file) then return false end + if not is_capture_index(rule.line) then return false end + if rule.col ~= nil and not is_capture_index(rule.col) then return false end if rule.severity ~= nil and rule.severity ~= "error" and rule.severity ~= "warning" then return false end @@ -109,7 +118,19 @@ end -- per entry (Q#CM4): a non-table container degrades to the built-in -- defaults; malformed entries are skipped; one status note per run -- counts the skips. Never raises — this feeds the per-frame pump. -local BUILTIN_RULES = pmacs.compile.rules +-- +-- The defaults are a private deep copy taken at load time: an alias +-- of the public table would keep in-place user mutations live after +-- the "using built-in defaults" degradation (round-1 finding 10). +local BUILTIN_RULES = {} +for i, rule in ipairs(pmacs.compile.rules) do + local copy = {} + for k, v in pairs(rule) do + copy[k] = v + end + BUILTIN_RULES[i] = copy +end + local function validated_rules() local rules = pmacs.compile.rules if type(rules) ~= "table" then @@ -267,7 +288,11 @@ end local function resync(slot) local buf = slot.buf for _, e in ipairs(slot.errors) do + -- Both in-buffer anchors: the display row AND the public byte + -- anchor — pre-marker byte offsets are exactly as untrustworthy + -- as rows after an unknown edit (round-1 finding 7). e.row = nil + e.line_start_byte = nil end local len = buf:len() buf:insert(len, DESYNC_MARKER, { bypass_intercept = true }) @@ -389,18 +414,33 @@ local SEVERITY_STYLE = { warning = { fg = 3 }, -- indexed yellow } +-- A stored coordinate must be a finite integer ≥ 1: `%d+` happily +-- captures digit runs whose tonumber is astronomically large or +-- math.huge, and an unbounded value would drive the cursor walk +-- loops effectively forever (round-1 finding 1). The cursor walk +-- also clamps independently — belt and braces. +local function valid_coordinate(n) + return n ~= nil and n >= 1 and n < math.huge and n == math.floor(n) +end + local function parse_line(slot, line, abs_start) if not slot.parse_errors then return end for _, rule in ipairs(slot.rules) do - local ok, c1, c2, c3 = pcall(string.match, line, rule.pattern) - if ok and c1 then - local caps = { c1, c2, c3 } + -- Capture EVERYTHING the pattern produced: validation accepts + -- any positive integer index, so truncating at three silently + -- misread four-capture rules (round-1 finding 4). + local caps = { pcall(string.match, line, rule.pattern) } + local ok = table.remove(caps, 1) + if ok and caps[1] then local file = caps[rule.file] local lnum = tonumber(caps[rule.line]) local cnum = rule.col and tonumber(caps[rule.col]) or nil - -- 1-based contract; below-1 captures fail closed (a raw -1 - -- would walk the cursor loops to a silent (0,0) landing). - if type(file) == "string" and lnum and lnum >= 1 and (cnum == nil or cnum >= 1) then + -- 1-based contract; below-1, non-integral, and non-finite + -- captures fail closed. A rule that NAMES a column capture the + -- match didn't produce also fails closed (silently storing + -- column 0 would misreport the location). + local col_ok = (rule.col == nil and cnum == nil) or valid_coordinate(cnum) + if type(file) == "string" and valid_coordinate(lnum) and col_ok then local severity = rule.severity or sniff_severity(line) slot.errors[#slot.errors + 1] = { file = file, @@ -454,6 +494,19 @@ local function project_root_of_active() return nil end +-- The daemon's actual working directory: the last-resort cwd when +-- there is no explicit opt and no detectable project. Resolving it +-- (rather than leaving nil and printing "(inherited)") gives the +-- header a real path and relative error files an explicit base +-- (round-1 finding 8). +local function daemon_working_directory() + local ok, id = pcall(pmacs.instance.identity) + if ok and type(id) == "table" and type(id.working_directory) == "string" then + return id.working_directory + end + return nil +end + local function format_exit_marker(label, ev) if ev.kind == "exited" then return string.format("\n[%s exited with code %d]\n", label, ev.code or 0) @@ -465,12 +518,32 @@ local function format_exit_marker(label, ev) return string.format("\n[%s exited]\n", label) end --- Terminal event: finalize the pending unterminated line (a final +-- Plain append at end, no overwrite/style tracking — markers and +-- headers. LOCAL by design: a global here would let user config +-- shadow a helper the terminal-event path depends on, and an error +-- thrown from that shadow would consume the terminal event before +-- pump cleanup/forget ran (round-1 finding 5). +local function emit_text_raw(slot, text) + local buf = slot.buf + buf:insert(buf:len(), text, { bypass_intercept = true }) + slot.out_pos = buf:len() + if slot.parse_line_start > slot.out_pos then + slot.parse_line_start = slot.out_pos + end +end + +-- Terminal event: drain the parser's cross-feed state (an +-- incomplete UTF-8 sequence at process EOF can never complete — the +-- parser's finish() emits its replacement character, round-1 +-- finding 9), finalize the pending unterminated line (a final -- diagnostic emitted without a trailing newline is complete at EOF -- and must not be dropped — Q#CM4), then the exit marker. local function finish_run(slot, ev) if not check_rev(slot) then return end local buf = slot.buf + if slot.parser then + apply_events(slot, slot.parser:finish()) + end local len = buf:len() if slot.parse_errors and slot.parse_line_start < len then parse_line(slot, buf:slice(slot.parse_line_start, len), slot.parse_line_start) @@ -489,17 +562,6 @@ local function finish_run(slot, ev) end end --- Plain append at end, no overwrite/style tracking — markers and --- headers. -function emit_text_raw(slot, text) - local buf = slot.buf - buf:insert(buf:len(), text, { bypass_intercept = true }) - slot.out_pos = buf:len() - if slot.parse_line_start > slot.out_pos then - slot.parse_line_start = slot.out_pos - end -end - local function feed_bytes(slot, bytes) if not check_rev(slot) then return end apply_events(slot, slot.parser:feed(bytes)) @@ -564,7 +626,7 @@ local function start_run(slot, cmdline, opts) if cur and not pmacs.compile.is_generated_buffer(cur) then slot.prev = cur end - local cwd = opts.cwd or project_root_of_active() + local cwd = opts.cwd or project_root_of_active() or daemon_working_directory() -- Supersede (Q#CM9): terminate the old group and tombstone its -- pump entry; its terminal event still drives forget. @@ -587,7 +649,9 @@ local function start_run(slot, cmdline, opts) local buf = slot.buf local len = buf:len() if len > 0 then buf:delete(0, len, { bypass_intercept = true }) end - local header = string.format("$ %s\nDirectory: %s\n\n", cmdline, cwd or "(inherited)") + -- The identity fallback should always resolve; "(unknown)" only + -- survives if the instance API itself failed. + local header = string.format("$ %s\nDirectory: %s\n\n", cmdline, cwd or "(unknown)") buf:insert(0, header, { bypass_intercept = true }) slot.out_pos = buf:len() slot.parse_line_start = slot.out_pos @@ -631,14 +695,32 @@ end -- Cursor walk via primitives so overlay observers see the motion -- (the lsp.lua visit idiom; 0-based line/col; the col walk shares --- lsp.lua's inherited per-codepoint residual). +-- lsp.lua's inherited per-codepoint residual). Both walks stop when +-- movement stops moving — a diagnostic pointing past EOF/EOL clamps +-- there instead of looping to its nominal coordinate (round-1 +-- finding 1; parse-time validation bounds the values, the clamp +-- bounds the walk regardless). local function move_active_cursor_to(line, col) pmacs.editor.move_line_start() while pmacs.editor.cursor_line() > 0 do pmacs.editor.move_up() end - for _ = 1, line do pmacs.editor.move_down() end - for _ = 1, col do pmacs.editor.move_right() end + for _ = 1, line do + local before = pmacs.editor.cursor_line() + pmacs.editor.move_down() + if pmacs.editor.cursor_line() == before then break end -- EOF + end + local row = pmacs.editor.cursor_line() + for _ = 1, col do + local before = pmacs.editor.cursor() + pmacs.editor.move_right() + if pmacs.editor.cursor() == before then break end -- buffer end + if pmacs.editor.cursor_line() ~= row then + -- Ran off the line's end onto the next row: step back to EOL. + pmacs.editor.move_left() + break + end + end end local function resolve_error_path(slot, file) diff --git a/docs/compile-mode-framing.md b/docs/compile-mode-framing.md index 2983313..8048656 100644 --- a/docs/compile-mode-framing.md +++ b/docs/compile-mode-framing.md @@ -1,7 +1,30 @@ # Compile-mode — framing (Arc 5 stage 1, terminal) -**Revision 6 — 2026-07-13. Status: awaiting approval; no branch, no -implementation.** +**Revision 7 — 2026-07-13. Status: implemented on branch +`compile-mode` (PR #113); revision 7 folds in PR round 1.** + +Revision 7 (PR #113 round 1, findings 1–10): stored coordinates must +be finite integers and both cursor walks are movement-bounded +(clamping at EOF/EOL) — an astronomical `%d+` capture can no longer +hang the editor; the grep panel gains the same immediate +`buffer.after-edit` recovery trigger as the compile slots; the rustc +rule uses the framing's `([^:]+)` spelling (paths with spaces); +rule capture indexes must be positive integers, all pattern captures +are honored (not just the first three), and a rule that names a +column its match didn't produce rejects the match; the marker/header +append helper is module-local (a user global could shadow it and its +error consumed the terminal event before forget); `stdin`/`group` +spec fields reject wrong Lua types instead of silently defaulting +(strict boolean check — mlua truthiness would coerce `"true"`); +resync also nils the public `line_start_byte` (total pre-marker +anchor invalidation includes the byte anchor); the inherited cwd +resolves through `pmacs.instance.identity().working_directory` so +the header always names a real path; a new `parser:finish()` +(additions #5) flushes a truncated UTF-8 sequence at process EOF as +U+FFFD before the exit marker; the built-in default rules are a +private deep copy, immune to in-place mutation of the public table. +Eleven bite tests, each verified failing against the pre-fix tree +via `scripts/bite`. Revision 6 (responding to review round 5, findings 1–4, plus the follow-up blocker audit): group-aware final drain now has an absolute @@ -249,6 +272,11 @@ No protocol change, no frontend change. immediately (zero-race; strictly better here than exposing `close_stdin` post-spawn). `write_stdin` on such a process errors with the existing stdin-not-piped message. Rejected under PTY. + The Lua spec parsing for `stdin` and `group` treats wrong types + as HARD errors (Revision 7) — a silently-defaulted `stdin = true` + or `group = "true"` would undo exactly the guarantees the fields + carry; `group` is matched as a raw Value because mlua's bool + conversion applies Lua truthiness. 2. **`group = true`** (`ProcessSpec`, pipes-only) — a full lifecycle policy, not just a spawn flag: - **Spawn**: `process_group(0)` — the child leads a fresh group. @@ -369,6 +397,13 @@ No protocol change, no frontend change. same-length replaces, so undoing one changes content without changing length. Revision increments on every edit, undo, and redo. +5. **`AnsiParser::finish()` + `parser:finish()` (Revision 7)** — + stream-end finalization: the feed-boundary contract deliberately + buffers an incomplete UTF-8 sequence for the next feed, but at + process EOF there is no next feed — `finish()` emits U+FFFD for + the pending prefix (the same posture an interrupting control byte + gets) and flushes the text run. Compile-mode calls it once at the + terminal event, before finalizing the pending line. ## Decisions @@ -457,10 +492,12 @@ invoke time, so commands/runtime load order stays irrelevant for it. length: the CR-overwrite path emits same-length replaces, so an undone overwrite changes content while preserving length — a length guard provably misses it (round-3 finding 3). **Anchor - invalidation is total (Revision 5):** a revision mismatch + invalidation is total (Revisions 5/7):** a revision mismatch carries no edit range, and a same-length replace can remove or move newlines while every anchor stays in bounds — so ALL - **pre-marker** in-buffer anchors are dropped on any mismatch. + **pre-marker** in-buffer anchors are dropped on any mismatch, + the public `line_start_byte` included (a pre-marker byte offset + is exactly as untrustworthy as a row). Immediately after recovery, `n`/`p` report "no more errors" and RET reports "no error on this line" for the damaged pre-marker content. The marker establishes a fresh anchor epoch: diagnostic @@ -511,8 +548,10 @@ invoke time, so commands/runtime load order stays irrelevant for it. `GROUP_TERM_GRACE + 2 * READER_SEND_POLL_INTERVAL`; eliminating it requires the deferred tick-driven drain. - `cwd`: explicit opt > `pmacs.project.detect(active buffer - path).root` > the daemon process cwd. The header line prints the - resolved cwd. + path).root` > `pmacs.instance.identity().working_directory` + (Revision 7 — actually *resolved*, so the header always names a + real path and relative error files get an explicit base rather + than an implicit pass-through). ### Q#CM4 — Error parsing: ordered Lua-pattern rules, parsed at newline time @@ -539,26 +578,39 @@ invoke time, so commands/runtime load order stays irrelevant for it. a named deferral. - **Coordinate normalization:** rule captures follow compiler convention — **1-based line and column**; that is the public rule - contract. Captured values **below 1 fail closed** (Revision 4): - the match is discarded — `%d+` accepts `0`, and an unvalidated - `0 - 1 = -1` would walk the cursor loops to a silent (0,0) - landing. Valid stored entries are **0-based** (what - `move_active_cursor_to` consumes): `line - 1`; `col - 1` when - captured, else `0`. The Python rule has no column: `line - 1`, - col `0`. (Grep normalization is in Q#CM7.) + contract. Captured values **below 1, non-integral, or non-finite + fail closed** (Revisions 4/7): the match is discarded — `%d+` + accepts `0` (an unvalidated `0 - 1 = -1` would walk the cursor + loops to a silent (0,0) landing) and also accepts digit runs whose + `tonumber` is `math.huge` (an unbounded loop bound). The cursor + walks are independently **movement-bounded** (Revision 7): they + stop when motion stops moving, clamping at EOF, and the column + walk clamps at the target row's EOL instead of marching onto later + rows. Rule capture **indexes** must be positive integers; all + pattern captures are collected (an index above three reads the + real capture, not nil-as-column-0); a rule that names a column + capture its match didn't produce rejects the match. Valid stored + entries are **0-based** (what `move_active_cursor_to` consumes): + `line - 1`; `col - 1` when captured, else `0`. The Python rule has + no column: `line - 1`, col `0`. (Grep normalization is in Q#CM7.) - **Parse each line exactly once, when its `\n` lands** — CR/erase rewrites happen within the current *unterminated* line, so the content parsed at newline time is the line's final form. **At the - terminal event, the pending unterminated line (if any) is - finalized and parsed once before the exit marker is appended.** + terminal event: first `parser:finish()` drains the cross-feed + state (a truncated multibyte sequence at process EOF surfaces as + U+FFFD instead of vanishing — Revision 7, additions #5), then the + pending unterminated line (if any) is finalized and parsed once + before the exit marker is appended.** - **Rule-table robustness (fail-closed per entry):** on each run's first use the table is validated — non-table `pmacs.compile.rules` - degrades to the built-in defaults with one status note (the - pair.lua non-table-sets precedent); malformed entries are skipped - and pattern matching is pcall'd so an invalid Lua pattern skips - that entry too. One status note per run counts the skipped - entries; a later valid rule still matches; the per-frame pump - never raises. + degrades to **a private deep copy of the built-in defaults** + (Revision 7: an alias of the public table would keep in-place user + mutations live after the degradation); malformed entries are + skipped, invalid Lua patterns are caught (and counted) at + validation time via a probe match, and match-time pattern calls + stay pcall'd as belt-and-braces. One status note per run counts + the skipped entries; a later valid rule still matches; the + per-frame pump never raises. - Each match appends `{ file, line, col, severity, line_start_byte }` to the run's ordered error list (reset per run). Relative paths resolve against the run's cwd. @@ -630,6 +682,11 @@ user puts it (an auto-scroll option is deferred). Checking only process-pump appends is insufficient: a no-hook edit to `*search-results*` followed by a worker batch would otherwise advance the expected revision and permanently mask the external edit. +- **The panel has the same immediate `buffer.after-edit` recovery + trigger as the compile slots (Revision 7):** after a COMPLETED + search no producer write or navigation may ever come, so an `M-x + buffer.undo` in the panel must be marked synchronously, not on the + next guarded operation. - **Root retention across interactive supersedes (Revision 3):** the panel stores the root each search ran with. Resolution order: explicit `opts.root` > *if the active buffer is the results panel, diff --git a/src/ansi.rs b/src/ansi.rs index 37f19fe..5a1d2f9 100644 --- a/src/ansi.rs +++ b/src/ansi.rs @@ -393,6 +393,26 @@ impl AnsiParser { events } + /// Stream-end finalization. The feed-boundary contract keeps an + /// incomplete UTF-8 sequence buffered because its trailing bytes + /// are expected in the next feed — but at process EOF there IS no + /// next feed, so the pending prefix can never complete. Emit + /// U+FFFD for it (the same posture `flush_text_run` takes when a + /// control byte interrupts a sequence) and flush the resulting + /// text run. Idempotent once drained. First consumer: + /// compile-mode's terminal-event path (Q#CM4). + pub fn finish(&mut self) -> Vec { + let mut events = Vec::new(); + self.flush_pending_utf8_as_replacement(); + if !self.text_run.is_empty() && !self.alt_screen_active { + let run = std::mem::take(&mut self.text_run); + events.push(AnsiEvent::Text(run)); + } else { + self.text_run.clear(); + } + events + } + fn feed_byte(&mut self, b: u8, events: &mut Vec) { // ESC-anywhere rule: ECMA-48 §10.2 ("Cancel"). Aborts any // in-progress sequence and starts a fresh Escape state. The diff --git a/src/lua_bindings/mod.rs b/src/lua_bindings/mod.rs index 8abb7f3..ae990af 100644 --- a/src/lua_bindings/mod.rs +++ b/src/lua_bindings/mod.rs @@ -3009,6 +3009,19 @@ impl UserData for AnsiParserLua { Ok(()) }); + // Stream-end finalization: flushes cross-feed state (an + // incomplete UTF-8 sequence becomes the replacement + // character). Compile-mode calls this once at the process's + // terminal event (Q#CM4; PR #113 round-1 finding 9). + methods.add_method("finish", |lua, this, ()| { + let events = this.0.borrow_mut().finish(); + let out = lua.create_table_with_capacity(events.len(), 0)?; + for (i, ev) in events.iter().enumerate() { + out.set(i + 1, event_to_lua_table(lua, ev)?)?; + } + Ok(out) + }); + methods.add_meta_method(mlua::MetaMethod::ToString, |_, _this, ()| { Ok("AnsiParser".to_string()) }); @@ -6988,12 +7001,15 @@ fn lua_to_spec(table: &Table) -> mlua::Result { // Compile-mode process shape (Q#CM3). Both options are // pipe-mode-only; the supervisor rejects them at spawn under PTY // so misconfiguration surfaces as a spawn error, not silence. - let stdin = match table - .get::>("stdin") - .ok() - .flatten() - .as_deref() - { + // Type errors are HARD errors, not silent defaults: `stdin = + // true` quietly becoming a piped stdin (hang) or `group = + // "true"` quietly becoming false (descendant leak) would undo + // exactly the guarantees these fields exist to carry (PR #113 + // round-1 finding 6). + let stdin_raw: Option = table.get("stdin").map_err(|_| { + mlua::Error::external("stdin must be the string \"piped\" or \"null\"".to_owned()) + })?; + let stdin = match stdin_raw.as_deref() { None | Some("piped") => crate::process::StdinMode::Piped, Some("null") => crate::process::StdinMode::Null, Some(other) => { @@ -7002,11 +7018,19 @@ fn lua_to_spec(table: &Table) -> mlua::Result { ))); } }; - let group = table - .get::>("group") - .ok() - .flatten() - .unwrap_or(false); + // Read as a raw Value: mlua's `bool` conversion applies Lua + // truthiness, so `group = "true"` would silently coerce instead + // of erroring. + let group = match table.get::("group") { + Ok(mlua::Value::Nil) | Err(_) => false, + Ok(mlua::Value::Boolean(b)) => b, + Ok(other) => { + return Err(mlua::Error::external(format!( + "group must be a boolean; got {}", + other.type_name() + ))); + } + }; Ok(ProcessSpec { label, command, diff --git a/tests/compile_mode_acceptance.rs b/tests/compile_mode_acceptance.rs index fbd2f3c..7db88e8 100644 --- a/tests/compile_mode_acceptance.rs +++ b/tests/compile_mode_acceptance.rs @@ -1447,3 +1447,278 @@ fn acc33_round_trip_input_is_set_on_generated_buffers() { "*search-results*" ); } + +// --------------------------------------------------------------------------- +// PR #113 round 1 — bite tests (one per finding; each observed +// failing against the pre-fix tree via scripts/bite) +// --------------------------------------------------------------------------- + +#[test] +fn r1f1_non_finite_coordinates_fail_closed() { + // A 400-digit line capture tonumbers to math.huge; pre-fix it + // was stored and any visit walked forever. + let dir = tempfile::tempdir().unwrap(); + let mut s = editor(); + let digits = "9".repeat(400); + compile_and_finish( + &mut s, + &format!("printf 'h.c:{digits}:1: error: e\\n'"), + dir.path(), + ); + assert!( + compile_errors(&s).is_empty(), + "a non-finite line coordinate must be discarded; got {:?}", + compile_errors(&s) + ); +} + +#[test] +fn r1f1_beyond_eol_column_clamps_to_the_target_row() { + // Column past EOL: pre-fix the walk marched move_right across + // newlines and landed rows away from the diagnostic's line (and + // an astronomical value walked effectively forever). + let dir = tempfile::tempdir().unwrap(); + std::fs::write(dir.path().join("w.c"), "ab\ncd\nef\n").unwrap(); + let mut s = editor(); + compile_and_finish(&mut s, "printf 'w.c:1:500: error: e\\n'", dir.path()); + press(&mut s, KeyCode::Char('n')); + press(&mut s, KeyCode::Enter); + assert!(active_buffer_name(&s).ends_with("w.c")); + let line: i64 = eval(&s, "return pmacs.editor.cursor_line()"); + assert_eq!( + line, 0, + "the column walk must clamp at the target row's EOL, not run \ + onto later rows" + ); +} + +#[test] +fn r1f2_grep_command_path_undo_after_completed_search_recovers() { + let dir = tempfile::tempdir().unwrap(); + grep_fixture(dir.path()); + let mut s = editor(); + search(&s, "zqxvbn_needle_77", dir.path()); + assert!(pump_until(&mut s, 10_000, search_done), "search completes"); + // M-x buffer.undo in the completed panel: no producer write or + // navigation may ever come — recovery must be immediate via the + // buffer.after-edit subscription. + alt(&mut s, 'x'); + type_str(&mut s, "buffer.undo"); + press(&mut s, KeyCode::Enter); + let text = named_text(&s, "*search-results*"); + assert!( + text.contains(DESYNC), + "desync marker must appear immediately in the grep panel; \ + buffer:\n{text}" + ); + assert!( + errors_buffer(&s).is_empty(), + "clean *errors*: {}", + errors_buffer(&s) + ); +} + +#[test] +fn r1f3_rustc_arrow_paths_with_spaces_parse() { + let dir = tempfile::tempdir().unwrap(); + let script = write_script( + dir.path(), + "space.sh", + "printf ' --> /tmp/my dir/foo.rs:12:4\\n'\n", + ); + let mut s = editor(); + compile_and_finish(&mut s, &format!("sh {script}"), dir.path()); + assert_eq!( + compile_errors(&s), + vec![("/tmp/my dir/foo.rs".to_owned(), 11, 3, None)], + "the rustc rule must capture space-containing paths whole \ + (pre-fix the two-part fallback recorded arrow junk at col 0)" + ); +} + +#[test] +fn r1f4_capture_indexes_above_three_and_absent_columns() { + let dir = tempfile::tempdir().unwrap(); + // (a) a valid four-capture rule with col = 4 must store the + // fourth capture, not silently column 0. + let mut s = editor(); + exec( + &s, + r#" + pmacs.compile.rules = { + { pattern = "(q%.c):(%d+):(x):(%d+)", file = 1, line = 2, col = 4 }, + } + "#, + ); + compile_and_finish(&mut s, "printf 'q.c:7:x:9 error\\n'", dir.path()); + assert_eq!( + compile_errors(&s), + vec![("q.c".to_owned(), 6, 8, Some("error".to_owned()))], + "capture index 4 must be honored" + ); + + // (b) fractional indexes are malformed; (c) a rule that names a + // column its match didn't produce rejects the match. + let mut s = editor(); + exec( + &s, + r#" + pmacs.compile.rules = { + { pattern = "(a%.c):(%d+):", file = 1, line = 2, col = 1.5 }, + { pattern = "(r%.c):(%d+):?(%d*)", file = 1, line = 2, col = 3 }, + } + "#, + ); + compile_run(&s, "printf 'a.c:3: e\\nr.c:5: e\\n'", dir.path()); + assert!( + status(&s).contains("skipped 1 malformed"), + "the fractional index is rejected at validation; got: {}", + status(&s) + ); + assert!(pump_until(&mut s, 10_000, |s| compilation_text(s) + .contains("[compile exited"))); + assert!( + compile_errors(&s).is_empty(), + "an empty column capture under a col-naming rule must reject \ + the match, not store column 0; got {:?}", + compile_errors(&s) + ); +} + +#[test] +fn r1f5_user_global_cannot_shadow_the_marker_helper() { + let dir = tempfile::tempdir().unwrap(); + let mut s = editor(); + // A hostile (or merely colliding) user global with the helper's + // old name: pre-fix this replaced the module's function and its + // error consumed the terminal event before forget ran. + exec(&s, "_G.emit_text_raw = function() error('shadowed') end"); + compile_and_finish(&mut s, "echo fine", dir.path()); + assert!( + compilation_text(&s).contains("[compile exited with code 0]"), + "the exit marker must come from the module's local helper" + ); + assert!( + errors_buffer(&s).is_empty(), + "no error spam: {}", + errors_buffer(&s) + ); + assert!( + pump_until(&mut s, 3_000, |s| process_count(s) == 0), + "terminal-event cleanup (forget) must have run" + ); +} + +#[test] +fn r1f6_wrong_spec_types_error_instead_of_defaulting() { + let s = editor(); + let (ok, err): (bool, String) = eval( + &s, + r#" + local ok, err = pcall(pmacs.process.spawn, + { label = "t", command = "/bin/true", stdin = true }) + return ok, tostring(err) + "#, + ); + assert!(!ok, "stdin = true (boolean) must be a hard error"); + assert!(err.contains("stdin must be"), "pointed message; got: {err}"); + let (ok, err): (bool, String) = eval( + &s, + r#" + local ok, err = pcall(pmacs.process.spawn, + { label = "t", command = "/bin/true", group = "true" }) + return ok, tostring(err) + "#, + ); + assert!(!ok, "group = \"true\" (string) must be a hard error"); + assert!( + err.contains("group must be a boolean"), + "pointed message; got: {err}" + ); +} + +#[test] +fn r1f7_resync_invalidates_the_public_byte_anchor() { + let dir = tempfile::tempdir().unwrap(); + let mut s = editor(); + compile_and_finish(&mut s, "printf 'a.c:1:1: error: e\\n'", dir.path()); + let has_anchor: bool = eval( + &s, + "return pmacs.compile.errors()[1].line_start_byte ~= nil", + ); + assert!(has_anchor, "pre-desync entries carry the byte anchor"); + // Trigger the guard through the command path. + alt(&mut s, 'x'); + type_str(&mut s, "buffer.undo"); + press(&mut s, KeyCode::Enter); + assert!(compilation_text(&s).contains(DESYNC), "marker appended"); + let has_anchor: bool = eval( + &s, + "return pmacs.compile.errors()[1].line_start_byte ~= nil", + ); + assert!( + !has_anchor, + "total pre-marker anchor invalidation includes the public \ + line_start_byte, not just the display row" + ); +} + +#[test] +fn r1f8_inherited_cwd_resolves_to_the_daemon_working_directory() { + // Pathless scratch buffer, no opts.cwd, no project: the header + // must print the real working directory, and relative error + // paths must resolve against it explicitly. + let mut s = editor(); + exec(&s, "pmacs.compile.run('echo hi')"); + assert!(pump_until(&mut s, 10_000, |s| compilation_text(s) + .contains("[compile exited"))); + let cwd = std::env::current_dir().unwrap().display().to_string(); + let text = compilation_text(&s); + assert!( + text.contains(&format!("Directory: {cwd}")), + "the header must name the daemon's actual working directory; \ + got:\n{text}" + ); + assert!( + !text.contains("(inherited)") && !text.contains("(unknown)"), + "no placeholder when the identity API is available:\n{text}" + ); +} + +#[test] +fn r1f9_truncated_utf8_at_eof_becomes_the_replacement_character() { + let dir = tempfile::tempdir().unwrap(); + // \303 (0xC3) opens a two-byte sequence that never completes: + // the parser's cross-feed buffer holds it, and only the new + // stream-end finish() can flush it as U+FFFD. + let script = write_script(dir.path(), "trunc.sh", "printf 'abc\\303'\n"); + let mut s = editor(); + compile_and_finish(&mut s, &format!("sh {script}"), dir.path()); + let text = compilation_text(&s); + assert!( + text.contains("abc\u{FFFD}"), + "the truncated sequence must surface as U+FFFD, not vanish; \ + buffer:\n{text}" + ); +} + +#[test] +fn r1f10_builtin_default_rules_survive_in_place_mutation() { + let dir = tempfile::tempdir().unwrap(); + let mut s = editor(); + // Corrupt an entry IN PLACE, then degrade the container: the + // "built-in defaults" fallback must be a true copy, not an alias + // of the mutated table. + exec( + &s, + "pmacs.compile.rules[2] = 'junk'; pmacs.compile.rules = 42", + ); + compile_and_finish(&mut s, "printf 'foo.c:7:2: warning: w\\n'", dir.path()); + assert_eq!( + compile_errors(&s), + vec![("foo.c".to_owned(), 6, 1, Some("warning".to_owned()))], + "the gcc three-part rule from the TRUE defaults must match \ + (the aliased pre-fix table lost it and the two-part fallback \ + stored column 0)" + ); +}