fix(compile): PR #113 round 2 — rule snapshots, finite indexes, shell isolation, parser reset

Finding-by-finding (framing revision 8; bites via scripts/bite):

1. Rule validation is a stable, total snapshot: validated scalar
   fields are copied into per-run plain tables via raw reads
   (rawget; metatable-provided fields deliberately not honored), so
   post-run mutation of the user's rule objects cannot alter an
   in-flight run and a hostile __index is a counted skip, not an
   error thrown through the pump mid-batch. The container traversal
   is itself pcall-protected; traversal-raise semantics are
   Lua-flavor-dependent (5.2+ ipairs consults __index, LuaJIT reads
   raw) and the test pins both flavors.
2. Capture indexes must be FINITE (floor(math.huge) == math.huge, so
   integrality alone passed it); math.huge is now a counted
   malformed entry.
3. Shell-command never touches the rule table: no spurious
   compile-rule warnings on M-!, and no rule-container state can
   block a run that performs no parsing.
4. AnsiParser::finish() (and parser:finish()) now fully resets the
   parser — in-flight CSI/OSC/escape state and alt-screen
   suppression included — so a post-finish feed parses a fresh
   stream. Three direct unit tests in ansi.rs plus a Lua-driven twin
   in the acceptance suite (the twin exists because a scripts/bite
   file swap replaces the in-file units along with the fix).
5. Comment corrections: fractional capture indexes read a distinct
   absent key (not a neighboring capture); the group-coercion
   comment describes truthiness, not false; the AnsiParserLua
   rustdoc lists finish().

Bites: r2f1 (both shapes), r2f2, r2f3 fail against pre-fix
compile.lua; r2f4 fails against pre-fix ansi.rs. Gates: fmt, clippy
workspace all-targets, lib 1525, crdt lib 1699, compile acceptance
50, crdt acceptance 1, m4 101, GPU 59, workspace sweep 2501/0 (one
flaky-suite rerun per the standing m8 rule), git diff --check.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VoiEyuPjoBhvwACf8HAnLB
This commit is contained in:
Levi Neuwirth 2026-07-13 16:30:41 +01:00
parent d67d30bb64
commit 37fac4324a
5 changed files with 374 additions and 49 deletions

View File

@ -91,46 +91,59 @@ 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).
-- A capture index must be a positive, FINITE integer. Fractional
-- indexes read a distinct (absent) table key, not a capture;
-- math.floor(math.huge) == math.huge, so integrality alone does not
-- imply finiteness (round-1 finding 4; round-2 finding 2).
local function is_capture_index(v)
return type(v) == "number" and v >= 1 and v == math.floor(v)
return type(v) == "number" and v >= 1 and v < math.huge 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
-- Validate one rule via RAW reads (rawget): a metatable-backed entry
-- whose __index raises must be a skipped malformed entry, not an
-- error thrown through the per-frame pump mid-batch (round-2
-- finding 1). Fail-closed posture: metatable-provided fields are
-- deliberately not honored. Returns a plain-table copy of the
-- validated scalar fields, or nil — the copy is the run's snapshot,
-- immune to post-validation mutation of the user's rule object.
local function validated_rule_copy(rule)
if type(rule) ~= "table" then return nil end
local pattern = rawget(rule, "pattern")
if type(pattern) ~= "string" then return nil end
-- Probe the pattern against the empty string so a malformed Lua
-- 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 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
if not pcall(string.match, "", pattern) then return nil end
local file = rawget(rule, "file")
local line = rawget(rule, "line")
local col = rawget(rule, "col")
local severity = rawget(rule, "severity")
if not is_capture_index(file) then return nil end
if not is_capture_index(line) then return nil end
if col ~= nil and not is_capture_index(col) then return nil end
if severity ~= nil and severity ~= "error" and severity ~= "warning" then
return nil
end
return true
return { pattern = pattern, file = file, line = line, col = col, severity = severity }
end
-- Validate the (user-mutable) rule table once per run, fail-closed
-- 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.
--
-- 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
BUILTIN_RULES[i] = validated_rule_copy(rule)
end
-- Validate the (user-mutable) rule table once per run, fail-closed
-- 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 —
-- so the container traversal itself is protected too (a hostile
-- __index on the OUTER table can raise from inside ipairs; round-2
-- finding 1). The returned list holds per-run plain-table copies:
-- validation is a stable, total snapshot, and mutating the user's
-- rule objects after compile.run() cannot alter an in-flight run.
local function validated_rules()
local rules = pmacs.compile.rules
if type(rules) ~= "table" then
@ -138,12 +151,20 @@ local function validated_rules()
return BUILTIN_RULES, 0
end
local valid, skipped = {}, 0
for _, rule in ipairs(rules) do
if rule_is_valid(rule) then
valid[#valid + 1] = rule
else
skipped = skipped + 1
local ok = pcall(function()
for _, rule in ipairs(rules) do
local copy = validated_rule_copy(rule)
if copy then
valid[#valid + 1] = copy
else
skipped = skipped + 1
end
end
end)
if not ok then
pmacs.editor.set_status(
"compile: pmacs.compile.rules raised during traversal; using built-in defaults")
return BUILTIN_RULES, 0
end
return valid, skipped
end
@ -638,8 +659,15 @@ local function start_run(slot, cmdline, opts)
pmacs.editor.set_status(slot.label .. ": superseded previous run")
end
-- Fresh run state.
slot.rules, slot.skipped_rules = validated_rules()
-- Fresh run state. Only error-parsing slots touch the rule table
-- at all: shell-command performs no parsing, so it must neither
-- surface compile-rule warnings nor fail on a hostile rule
-- container (round-2 finding 3).
if slot.parse then
slot.rules, slot.skipped_rules = validated_rules()
else
slot.rules, slot.skipped_rules = {}, 0
end
slot.parse_errors = slot.parse
slot.errors = {}
slot.err_index = 0

View File

@ -1,7 +1,25 @@
# Compile-mode — framing (Arc 5 stage 1, terminal)
**Revision 7 — 2026-07-13. Status: implemented on branch
`compile-mode` (PR #113); revision 7 folds in PR round 1.**
**Revision 8 — 2026-07-13. Status: implemented on branch
`compile-mode` (PR #113); revisions 78 fold in PR rounds 12.**
Revision 8 (PR #113 round 2, findings 15): rule validation is a
stable, total snapshot — validated scalar fields are copied into
per-run plain tables via raw reads (rawget; metatable-provided
fields are deliberately not honored), and the container traversal is
itself protected, so neither post-run mutation of the user's rule
objects nor a hostile `__index` can alter an in-flight run or raise
through the pump mid-batch (traversal-raise semantics are
Lua-flavor-dependent: 5.2+ `ipairs` consults `__index`, LuaJIT reads
raw — both pinned); capture indexes must also be FINITE
(`math.floor(math.huge) == math.huge`); shell-command never touches
the rule table (no spurious warnings, no rule-container failure can
block a run that parses nothing); `parser:finish()` now RESETS the
parser — in-flight CSI/OSC/escape state and alt-screen suppression
included — so a post-finish feed parses a fresh stream (direct unit
tests plus a Lua-driven twin); three stale comments corrected.
Bites: four acceptance tests against pre-fix compile.lua, one
against pre-fix ansi.rs.
Revision 7 (PR #113 round 1, findings 110): stored coordinates must
be finite integers and both cursor walks are movement-bounded
@ -397,13 +415,17 @@ 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)** —
5. **`AnsiParser::finish()` + `parser:finish()` (Revisions 78)** —
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.
gets) and flushes the text run, **then fully resets the parser
(in-flight CSI/OSC/escape state and alt-screen suppression
included): a feed after finish parses a NEW stream** rather than
continuing a pre-EOF escape or staying suppressed. Compile-mode
calls it once at the terminal event, before finalizing the
pending line.
## Decisions
@ -608,9 +630,17 @@ invoke time, so commands/runtime load order stays irrelevant for it.
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.
stay pcall'd as belt-and-braces. **Validation is a stable, total
snapshot (Revision 8):** validated scalar fields are copied into
per-run plain tables via raw reads — mutating the user's rule
objects after `compile.run()` cannot alter the in-flight run, a
hostile `__index` is a counted skip rather than an error through
the pump, and the container traversal itself is protected
(5.2+ `ipairs` consults `__index`; LuaJIT reads raw — both
degrade cleanly). Capture indexes must be positive, FINITE
integers. One status note per run counts the skipped entries; a
later valid rule still matches; the per-frame pump never raises.
Shell-command slots never touch the rule table at all.
- 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.

View File

@ -399,8 +399,14 @@ impl AnsiParser {
/// 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).
/// text run.
///
/// The parser is then fully reset — in-flight CSI/OSC/escape
/// state AND alt-screen suppression included — so a `feed` after
/// `finish` parses a NEW stream from a clean slate rather than
/// continuing a pre-EOF escape sequence or staying suppressed
/// (PR #113 round-2 finding 4). Idempotent once drained. First
/// consumer: compile-mode's terminal-event path (Q#CM4).
pub fn finish(&mut self) -> Vec<AnsiEvent> {
let mut events = Vec::new();
self.flush_pending_utf8_as_replacement();
@ -410,6 +416,11 @@ impl AnsiParser {
} else {
self.text_run.clear();
}
self.reset();
// `reset` deliberately preserves alt-screen suppression (a
// mid-stream reset must not unhide alt-screen contents); a
// stream END does end the suppression.
self.alt_screen_active = false;
events
}
@ -1780,4 +1791,54 @@ mod tests {
let evs_text = p.feed(b"hi");
assert_eq!(collect_text(&evs_text), "hi");
}
// -----------------------------------------------------------------
// Stream-end finish() (compile-mode Q#CM4; PR #113 rounds 12)
// -----------------------------------------------------------------
#[test]
fn finish_flushes_truncated_utf8_as_replacement() {
let mut p = AnsiParser::new();
// 0xC3 opens a two-byte sequence that never completes.
let evs = p.feed(b"abc\xC3");
assert_eq!(collect_text(&evs), "abc", "prefix buffered across feeds");
let evs = p.finish();
assert_eq!(
collect_text(&evs),
"\u{FFFD}",
"stream end must surface the pending prefix as U+FFFD"
);
// Idempotent once drained.
assert!(p.finish().is_empty(), "second finish drains nothing");
}
#[test]
fn feed_after_finish_starts_a_fresh_stream() {
// Mid-CSI at stream end: without the finish-time reset, a
// subsequent feed would keep consuming bytes as CSI
// parameters instead of parsing a new stream (PR #113
// round-2 finding 4).
let mut p = AnsiParser::new();
let _ = p.feed(b"\x1b[3"); // incomplete CSI
let _ = p.finish();
let evs = p.feed(b"plain");
assert_eq!(
collect_text(&evs),
"plain",
"post-finish feeds must not continue a pre-EOF escape"
);
}
#[test]
fn finish_ends_alt_screen_suppression() {
let mut p = AnsiParser::new();
let _ = p.feed(b"\x1b[?1049hhidden"); // enter alt screen
let _ = p.finish();
let evs = p.feed(b"visible");
assert_eq!(
collect_text(&evs),
"visible",
"a stream END ends suppression; a new stream starts unsuppressed"
);
}
}

View File

@ -2984,9 +2984,11 @@ fn attach_style_overlay_to_visible_windows(
/// Lua-facing wrapper around [`crate::ansi::AnsiParser`].
///
/// Constructed via `pmacs.ansi.parser()`; methods `feed(bytes)` and
/// `reset()` mirror the Rust API. `feed` returns an array of event
/// tables --- see [`event_to_lua_table`] for the schema. The wrapper
/// Constructed via `pmacs.ansi.parser()`; methods `feed(bytes)`,
/// `reset()`, and `finish()` mirror the Rust API. `feed` and
/// `finish` return an array of event tables --- see
/// [`event_to_lua_table`] for the schema; `finish` drains stream-end
/// state and resets the parser for a fresh stream. The wrapper
/// is `RefCell`-internal so multiple Lua-side methods can borrow
/// safely; the Lua VM is single-threaded so the borrow can never
/// race.
@ -7001,11 +7003,13 @@ fn lua_to_spec(table: &Table) -> mlua::Result<ProcessSpec> {
// 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.
// 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).
// Type errors are HARD errors, not silent coercions: `stdin =
// true` would quietly keep a piped stdin (hang), and a mistyped
// `group` would coerce through Lua truthiness to whichever
// boolean its truthiness happens to be — either way the caller's
// intent is unverifiable, and these fields carry process-hygiene
// guarantees (PR #113 round-1 finding 6; wording corrected in
// round 2 finding 5).
let stdin_raw: Option<String> = table.get("stdin").map_err(|_| {
mlua::Error::external("stdin must be the string \"piped\" or \"null\"".to_owned())
})?;

View File

@ -1702,6 +1702,208 @@ fn r1f9_truncated_utf8_at_eof_becomes_the_replacement_character() {
);
}
// ---------------------------------------------------------------------------
// PR #113 round 2 — bite tests
// ---------------------------------------------------------------------------
#[test]
fn r2f1_rule_validation_is_a_stable_snapshot() {
// Mutating the user's rule object AFTER compile.run() must not
// alter the in-flight run: validation copies scalar fields into
// per-run plain tables.
let dir = tempfile::tempdir().unwrap();
let script = write_script(dir.path(), "slow.sh", "sleep 0.5\nprintf 'm.c:3: e\\n'\n");
let mut s = editor();
exec(
&s,
r#"pmacs.compile.rules = { { pattern = "(m%.c):(%d+):", file = 1, line = 2 } }"#,
);
compile_run(&s, &format!("sh {script}"), dir.path());
// The output hasn't arrived yet; sabotage the live rule object.
exec(&s, "pmacs.compile.rules[1].pattern = 'nevermatch'");
assert!(pump_until(&mut s, 10_000, |s| compilation_text(s)
.contains("[compile exited")));
assert_eq!(
compile_errors(&s),
vec![("m.c".to_owned(), 2, 0, None)],
"the run must parse with its validated snapshot, not the \
mutated object"
);
}
#[test]
fn r2f1_metatable_backed_rules_cannot_raise_through_the_pump() {
// A rule whose field reads raise (hostile __index) and a
// container whose traversal raises: both must degrade cleanly —
// no error thrown through the per-frame pump, terminal cleanup
// intact.
let dir = tempfile::tempdir().unwrap();
let mut s = editor();
exec(
&s,
r#"
local hostile = setmetatable({}, { __index = function() error("boom") end })
pmacs.compile.rules = { hostile,
{ pattern = "(k%.c):(%d+):", file = 1, line = 2 } }
"#,
);
compile_run(&s, "printf 'k.c:4: e\\n'", dir.path());
assert!(
status(&s).contains("skipped 1 malformed"),
"the hostile entry is a counted skip; got: {}",
status(&s)
);
assert!(pump_until(&mut s, 10_000, |s| compilation_text(s)
.contains("[compile exited with code 0]")));
assert_eq!(
compile_errors(&s),
vec![("k.c".to_owned(), 3, 0, None)],
"the valid entry still parses"
);
assert!(
errors_buffer(&s).is_empty(),
"nothing raised through the pump: {}",
errors_buffer(&s)
);
assert!(
pump_until(&mut s, 3_000, |s| process_count(s) == 0),
"terminal cleanup ran"
);
// Hostile CONTAINER whose traversal raises. Flavor-dependent by
// Lua semantics: 5.2+ `ipairs` consults __index (the raise fires
// and the pcall degrades to defaults with a note); LuaJIT/5.1
// reads raw (the container is simply empty — no rules, no note).
// Both flavors must complete cleanly with nothing thrown
// through the pump.
let mut s = editor();
exec(
&s,
r#"
pmacs.compile.rules = setmetatable({}, {
__index = function() error("container boom") end,
})
"#,
);
compile_run(&s, "printf 'a.c:1:1: error: e\\n'", dir.path());
let is_lua54: bool = eval(&s, "return _VERSION ~= 'Lua 5.1'");
if is_lua54 {
assert!(
status(&s).contains("raised during traversal"),
"degradation note under 5.2+ ipairs semantics; got: {}",
status(&s)
);
}
assert!(pump_until(&mut s, 10_000, |s| compilation_text(s)
.contains("[compile exited")));
if is_lua54 {
assert_eq!(
compile_errors(&s).len(),
1,
"built-in defaults still parse after container degradation"
);
} else {
assert!(
compile_errors(&s).is_empty(),
"under raw-ipairs flavors the hostile container reads as \
an (empty) rule table"
);
}
assert!(
errors_buffer(&s).is_empty(),
"no spam: {}",
errors_buffer(&s)
);
}
#[test]
fn r2f2_infinite_capture_index_is_counted_malformed() {
let dir = tempfile::tempdir().unwrap();
let mut s = editor();
exec(
&s,
r#"
pmacs.compile.rules = {
{ pattern = "(z%.c):(%d+):", file = 1, line = 2, col = math.huge },
}
"#,
);
compile_run(&s, "printf 'z.c:2: e\\n'", dir.path());
assert!(
status(&s).contains("skipped 1 malformed"),
"math.huge is not a capture index (floor(huge) == huge, so \
integrality alone passes it); got: {}",
status(&s)
);
assert!(pump_until(&mut s, 10_000, |s| compilation_text(s)
.contains("[compile exited")));
}
#[test]
fn r2f3_shell_command_ignores_the_compile_rule_table() {
let dir = tempfile::tempdir().unwrap();
let mut s = editor();
// Both degradation shapes at once: a non-table container would
// warn, a raising container would abort — shell-command performs
// no parsing and must see neither.
exec(&s, "pmacs.compile.rules = 42");
exec(
&s,
&format!(
"pmacs.shell.command('echo shellok', {{ cwd = {:?} }})",
dir.path().display().to_string()
),
);
assert!(
!status(&s).contains("not a table"),
"no compile-rule warning on a shell run; got: {}",
status(&s)
);
assert!(
pump_until(&mut s, 10_000, |s| named_text(s, "*shell-command*")
.contains("[shell exited with code 0]")),
"shell-command runs regardless of rule-table state"
);
}
#[test]
fn r2f4_parser_finish_resets_for_a_fresh_stream() {
// Lua-driven twin of the ansi.rs units (which live inside the
// file a scripts/bite swap replaces): after finish(), a feed
// must parse a NEW stream — not continue a pre-EOF escape, not
// stay alt-screen-suppressed.
let s = editor();
let (after_csi, after_alt): (String, String) = eval(
&s,
r#"
local function text_of(evs)
local out = {}
for _, ev in ipairs(evs) do
if ev.kind == "text" then out[#out + 1] = ev.text end
end
return table.concat(out)
end
local p = pmacs.ansi.parser()
p:feed("\27[3") -- incomplete CSI at stream end
p:finish()
local a = text_of(p:feed("plain"))
local q = pmacs.ansi.parser()
q:feed("\27[?1049hhidden") -- alt screen active at stream end
q:finish()
local b = text_of(q:feed("visible"))
return a, b
"#,
);
assert_eq!(
after_csi, "plain",
"post-finish feed must not continue the pre-EOF CSI"
);
assert_eq!(
after_alt, "visible",
"stream end must end alt-screen suppression"
);
}
#[test]
fn r1f10_builtin_default_rules_survive_in_place_mutation() {
let dir = tempfile::tempdir().unwrap();