diff --git a/builtin/runtime/compile.lua b/builtin/runtime/compile.lua index 19bb779..25fb74f 100644 --- a/builtin/runtime/compile.lua +++ b/builtin/runtime/compile.lua @@ -353,20 +353,40 @@ local function add_style_span(slot, from, to) slot.overlay:add(from, to, slot.cur_style) end +-- True when byte `b` is a UTF-8 continuation byte (0x80–0xBF). +local function is_utf8_continuation(b) + return b >= 0x80 and b < 0xC0 +end + -- Append `text` at the tracked output position with overwrite -- semantics (CR progress bars rewrite the current line in place). +-- +-- UTF-8 safety (PR #113 round-3 finding 1): overwrite ranges are +-- byte-counted but must consume WHOLE existing codepoints — the +-- range end is aligned forward past continuation bytes — and the +-- incoming text event is applied as ONE atomic replace, never split +-- (parser text events carry only complete scalars). Splitting either +-- side left malformed bytes on the plain rope and made the +-- byte-native CRDT edit reject the range, aborting the pump +-- mid-batch after its events were already consumed. `out_pos` stays +-- on codepoint boundaries by induction: it moves to end-of-write, +-- line starts (after \n), or a boundary-aligned backspace target. local function emit_text(slot, text) if #text == 0 then return end local buf = slot.buf local len = buf:len() local pos = math.min(slot.out_pos, len) - local overwrite = math.min(#text, len - pos) - if overwrite > 0 then - buf:replace(pos, pos + overwrite, text:sub(1, overwrite), { bypass_intercept = true }) - end - if #text > overwrite then - buf:insert(pos + overwrite, text:sub(overwrite + 1), { bypass_intercept = true }) + -- The current line's remainder — the only bytes an overwrite may + -- touch (no \n exists past out_pos). + local tail = buf:slice(pos, len) + local overwrite = math.min(#text, #tail) + -- Align the overwrite end forward to a codepoint boundary of the + -- EXISTING content: replacing 1 byte of a 2-byte é must consume + -- both of its bytes (é and X both occupy one terminal column). + while overwrite < #tail and is_utf8_continuation(tail:byte(overwrite + 1)) do + overwrite = overwrite + 1 end + buf:replace(pos, pos + overwrite, text, { bypass_intercept = true }) slot.out_pos = pos + #text add_style_span(slot, pos, pos + #text) end @@ -405,8 +425,17 @@ local function apply_events(slot, events) elseif kind == "carriage_return" then slot.out_pos = current_line_start(slot) elseif kind == "backspace" then - if slot.out_pos > current_line_start(slot) then - slot.out_pos = slot.out_pos - 1 + -- Step back over one whole CODEPOINT, not one byte — a + -- mid-codepoint out_pos would make the next overwrite split + -- the character (round-3 finding 1). + local ls = current_line_start(slot) + if slot.out_pos > ls then + local prefix = slot.buf:slice(ls, slot.out_pos) + local i = #prefix + while i > 1 and is_utf8_continuation(prefix:byte(i)) do + i = i - 1 + end + slot.out_pos = ls + i - 1 end elseif kind == "erase_to_eol" then local len = buf:len() diff --git a/docs/compile-mode-framing.md b/docs/compile-mode-framing.md index 776bb0f..d04ecd9 100644 --- a/docs/compile-mode-framing.md +++ b/docs/compile-mode-framing.md @@ -1,7 +1,24 @@ # Compile-mode — framing (Arc 5 stage 1, terminal) -**Revision 8 — 2026-07-13. Status: implemented on branch -`compile-mode` (PR #113); revisions 7–8 fold in PR rounds 1–2.** +**Revision 9 — 2026-07-13. Status: implemented on branch +`compile-mode` (PR #113); revisions 7–9 fold in PR rounds 1–3.** + +Revision 9 (PR #113 round 3, findings 1–3): the CR/backspace +renderer is UTF-8-safe — overwrites consume WHOLE existing +codepoints (range end aligned forward past continuation bytes) in +ONE atomic replace of the complete text event, and backspace steps +to the previous codepoint boundary; pre-fix, byte-counted splits +left malformed bytes on the plain rope and made the byte-native CRDT +edit reject mid-codepoint ranges, aborting the pump after its events +were consumed (default + CRDT bites: é\rX, X\ré, é\bX). +`parser:finish()`'s reset is now OBSERVABLE: balancing events — +`AlternateScreenExit` for an unclosed enter, a default `SetStyle` +for a non-default running style — let consumers unwind mirrored +state from the event stream alone (unit applies events to consumer +state; Lua twin). The spawn-spec `stdin`/`group` fields are RAW +reads: metatable-provided fields are deliberately not honored (the +compile.lua posture) and a raising `__index` can no longer be +silently absorbed as `group = false`, disabling isolation. Revision 8 (PR #113 round 2, findings 1–5): rule validation is a stable, total snapshot — validated scalar fields are copied into @@ -294,7 +311,11 @@ No protocol change, no frontend change. 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. + conversion applies Lua truthiness. Both fields are RAW reads + (Revision 9): a spec table is plain data, metatable-provided + fields are deliberately not honored (the compile.lua rawget + posture), and a raising `__index` cannot be silently absorbed + into `group = false`, quietly disabling isolation. 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. @@ -421,11 +442,15 @@ No protocol change, no frontend change. 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, **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. + (in-flight CSI/OSC/escape state, alt-screen suppression, and the + running SGR style): a feed after finish parses a NEW stream** + rather than continuing a pre-EOF escape, staying suppressed, or + inheriting stale color. The reset is OBSERVABLE (Revision 9): + balancing events — `AlternateScreenExit` for an unclosed enter, a + default `SetStyle` for a non-default running style — let + consumers that mirror parser state unwind from the event stream + alone. Compile-mode calls it once at the terminal event, before + finalizing the pending line. ## Decisions @@ -468,7 +493,15 @@ invoke time, so commands/runtime load order stays irrelevant for it. intra-line treatment so progress bars collapse. Alt-screen suppression comes free. One parser, one running style, one output position — coherent because the child delivers **one merged - stream** (Q#CM3). + stream** (Q#CM3). **UTF-8 safety (Revision 9):** overwrite ranges + are byte-counted but consume whole existing codepoints (the range + end aligns forward past continuation bytes) and each text event is + applied as ONE atomic replace, never split; backspace steps to the + previous codepoint boundary. `out_pos` stays on codepoint + boundaries by induction. A split on either side previously left + malformed bytes on the plain rope, and under CRDT made the + byte-native edit reject — aborting the pump after events_take had + already consumed the batch. - **External-edit resilience (Revision 4–6).** Generated-buffer keys round-trip, so honest frontend optimistic undo is not an escape from the local bindings; accepted replica ops also fire diff --git a/src/ansi.rs b/src/ansi.rs index f94e556..5cb50e3 100644 --- a/src/ansi.rs +++ b/src/ansi.rs @@ -402,11 +402,17 @@ impl AnsiParser { /// 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). + /// state, alt-screen suppression, AND the running SGR style — so + /// a `feed` after `finish` parses a NEW stream from a clean + /// slate rather than continuing a pre-EOF escape sequence, + /// staying suppressed, or inheriting stale color (PR #113 + /// round-2 finding 4; round-3 finding 2). The reset is + /// OBSERVABLE: consumers that mirror parser state from the event + /// stream receive balancing events — an `AlternateScreenExit` + /// for an unclosed enter, a default `SetStyle` when the running + /// style was non-default — so they unwind without out-of-band + /// knowledge. 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(); @@ -416,11 +422,19 @@ impl AnsiParser { } else { self.text_run.clear(); } - self.reset(); - // `reset` deliberately preserves alt-screen suppression (a + // Balancing state events, in unwind order. `reset` alone + // 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; + // stream END does end it, observably. + if self.alt_screen_active { + self.alt_screen_active = false; + events.push(AnsiEvent::AlternateScreenExit); + } + if self.current_style != Style::default() { + self.current_style = Style::default(); + events.push(AnsiEvent::SetStyle(Style::default())); + } + self.reset(); events } @@ -1841,4 +1855,40 @@ mod tests { "a stream END ends suppression; a new stream starts unsuppressed" ); } + + #[test] + fn finish_emits_balancing_events_for_consumer_state() { + // A consumer mirrors parser state from the event stream + // alone (PR #113 round-3 finding 2): apply every event to a + // consumer-side mirror and require finish() to unwind it — + // not merely make subsequent text visible. + let mut p = AnsiParser::new(); + let mut consumer_alt = false; + let mut consumer_style = Style::default(); + let apply = |evs: &[AnsiEvent], alt: &mut bool, style: &mut Style| { + for ev in evs { + match ev { + AnsiEvent::AlternateScreenEnter => *alt = true, + AnsiEvent::AlternateScreenExit => *alt = false, + AnsiEvent::SetStyle(s) => *style = *s, + _ => {} + } + } + }; + let evs = p.feed(b"\x1b[31mred\x1b[?1049h"); + apply(&evs, &mut consumer_alt, &mut consumer_style); + assert!(consumer_alt, "enter observed"); + assert_ne!(consumer_style, Style::default(), "red observed"); + let evs = p.finish(); + apply(&evs, &mut consumer_alt, &mut consumer_style); + assert!( + !consumer_alt, + "finish must balance the unclosed AlternateScreenEnter" + ); + assert_eq!( + consumer_style, + Style::default(), + "finish must reset the running style observably" + ); + } } diff --git a/src/lua_bindings/mod.rs b/src/lua_bindings/mod.rs index 084ff5e..5f992b6 100644 --- a/src/lua_bindings/mod.rs +++ b/src/lua_bindings/mod.rs @@ -7009,8 +7009,12 @@ fn lua_to_spec(table: &Table) -> mlua::Result { // 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 = table.get("stdin").map_err(|_| { + // round 2 finding 5). RAW reads (round-3 finding 3): a spec + // table is plain data — metatable-provided fields are + // deliberately not honored (the compile.lua rawget posture), and + // a raising __index must not be silently absorbed into "false" + // and quietly disable process-group isolation. + let stdin_raw: Option = table.raw_get("stdin").map_err(|_| { mlua::Error::external("stdin must be the string \"piped\" or \"null\"".to_owned()) })?; let stdin = match stdin_raw.as_deref() { @@ -7024,9 +7028,10 @@ fn lua_to_spec(table: &Table) -> mlua::Result { }; // 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, + // of erroring. A raw Value read cannot raise; any residual error + // is still a hard error, never a silent default. + let group = match table.raw_get::("group") { + Ok(mlua::Value::Nil) => false, Ok(mlua::Value::Boolean(b)) => b, Ok(other) => { return Err(mlua::Error::external(format!( @@ -7034,6 +7039,7 @@ fn lua_to_spec(table: &Table) -> mlua::Result { other.type_name() ))); } + Err(e) => return Err(e), }; Ok(ProcessSpec { label, diff --git a/tests/compile_mode_acceptance.rs b/tests/compile_mode_acceptance.rs index d9c5a13..75dfce4 100644 --- a/tests/compile_mode_acceptance.rs +++ b/tests/compile_mode_acceptance.rs @@ -1948,3 +1948,140 @@ fn r1f10_builtin_default_rules_survive_in_place_mutation() { stored column 0)" ); } + +// --------------------------------------------------------------------------- +// PR #113 round 3 — bite tests +// --------------------------------------------------------------------------- + +#[test] +fn r3f1_cr_and_backspace_are_utf8_safe() { + // Overwrites must consume whole existing codepoints in one + // atomic replace, and backspace must step to the previous UTF-8 + // boundary. Pre-fix, é\rX left "X\xA9" (malformed) and é\bX left + // "\xC3X"; under CRDT the mid-codepoint edit rejects and aborts + // the pump (see the CRDT twin). + let dir = tempfile::tempdir().unwrap(); + let script = write_script( + dir.path(), + "uni.sh", + concat!( + "printf '\\303\\251\\rX\\n'\n", // é\rX → X + "printf 'X\\r\\303\\251\\n'\n", // X\ré → é + "printf '\\303\\251\\bX\\n'\n", // é\bX → X + ), + ); + let mut s = editor(); + compile_and_finish(&mut s, &format!("sh {script}"), dir.path()); + let text = compilation_text(&s); + assert!( + text.contains("\nX\n\u{e9}\nX\n"), + "each overwrite must yield exactly the replacing character, \ + valid UTF-8, no residue; buffer:\n{text:?}" + ); + assert!( + text.contains("[compile exited with code 0]"), + "terminal event must survive the unicode batch:\n{text}" + ); + assert!( + errors_buffer(&s).is_empty(), + "no pump aborts: {}", + errors_buffer(&s) + ); + assert!( + pump_until(&mut s, 3_000, |s| process_count(s) == 0), + "process record forgotten (cleanup ran)" + ); +} + +#[test] +fn r3f2_parser_finish_emits_balancing_state_events() { + // Consumers mirror parser state from the event stream alone: an + // unclosed alt-screen enter must be balanced by an exit, and a + // non-default running style by a default SetStyle — applied to + // consumer state, not just observed as later text. + let s = editor(); + let (alt_balanced, style_reset): (bool, bool) = eval( + &s, + r#" + local p = pmacs.ansi.parser() + p:feed("\27[31mred") + p:feed("\27[?1049h") -- enter alt screen, never exited + local alt = true -- consumer mirror of the enter + local style = { fg = 1 } + for _, ev in ipairs(p:finish()) do + if ev.kind == "alt_screen_exit" then alt = false end + if ev.kind == "set_style" then style = ev.style end + end + local style_is_default = style.fg == "default" + and style.bg == "default" and not style.bold + return alt == false, style_is_default + "#, + ); + assert!( + alt_balanced, + "finish must emit alt_screen_exit for an unclosed enter" + ); + assert!( + style_reset, + "finish must emit a default set_style when the running style \ + was non-default" + ); +} + +#[test] +fn r3f3_spec_fields_are_raw_reads_metatables_not_honored() { + let s = editor(); + // A metatable that provides group = true: honoring it would be + // silent spec-by-metatable; raw reads ignore it (the compile.lua + // rawget posture), so the child must NOT lead its own group. + let pid: i64 = eval( + &s, + r#" + local spec = setmetatable( + { label = "mt", command = "/bin/sh", args = { "-c", "sleep 30" } }, + { __index = function(_, k) + if k == "group" then return true end + return nil + end }) + local id = pmacs.process.spawn(spec) + for _, row in ipairs(pmacs.process.list()) do + if row.state and row.state.pid then return row.state.pid end + end + return -1 + "#, + ); + assert!(pid > 0, "metatable-backed spec must spawn"); + let out = std::process::Command::new("ps") + .args(["-o", "pgid=", "-p", &pid.to_string()]) + .output() + .expect("ps"); + let pgid: i64 = String::from_utf8_lossy(&out.stdout) + .trim() + .parse() + .expect("pgid"); + assert_ne!( + pgid, pid, + "metatable-provided group=true must not be honored (raw reads)" + ); + // A RAISING __index must not be silently absorbed either — with + // raw reads it simply never fires; the spawn succeeds cleanly. + let ok: bool = eval( + &s, + r#" + local spec = setmetatable( + { label = "mt2", command = "/bin/true" }, + { __index = function() error("hostile spec metatable") end }) + local ok = pcall(pmacs.process.spawn, spec) + return ok + "#, + ); + assert!(ok, "raw reads must not trip a raising __index"); + exec( + &s, + r" + for _, row in ipairs(pmacs.process.list()) do + pcall(pmacs.process.terminate, row.id) + end + ", + ); +} diff --git a/tests/compile_mode_crdt_acceptance.rs b/tests/compile_mode_crdt_acceptance.rs index 9618c1e..9c0d106 100644 --- a/tests/compile_mode_crdt_acceptance.rs +++ b/tests/compile_mode_crdt_acceptance.rs @@ -253,3 +253,52 @@ fn compile_run_converges_and_replica_edit_triggers_recovery() { "post-recovery convergence across the reorder seam" ); } + +#[test] +fn r3f1_unicode_cr_backspace_survive_crdt_replication() { + // PR #113 round-3 finding 1, CRDT twin: pre-fix the byte-counted + // overwrite split a 2-byte é mid-codepoint; the byte-native + // UTF-8 CRDT edit REJECTS that range, the pump callback aborts + // after events_take, the run never reaches its exit marker, and + // the process record leaks. Post-fix the whole-codepoint atomic + // replace applies cleanly and both replicas converge. + let dir = tempfile::tempdir().expect("tempdir"); + let script = dir.path().join("uni.sh"); + std::fs::write( + &script, + "printf '\\303\\251\\rX\\n'\nprintf 'X\\r\\303\\251\\n'\nprintf '\\303\\251\\bX\\n'\n", + ) + .unwrap(); + let init = format!( + r#" + pmacs.command.define {{ + name = "test.compile-unicode", + description = "round-3 unicode fixture trigger", + fn = function() + pmacs.compile.run("sh {script}", {{ cwd = "{dir}" }}) + end, + }} + pmacs.keymap.bind {{ scope = "global", sequence = "C-c 8", command = "test.compile-unicode" }} + "#, + script = script.display(), + dir = dir.path().display(), + ); + let daemon = TestDaemon::spawn_with_config(&init); + let mut source = attach_replica(&daemon); + let mut observer = attach_replica(&daemon); + + send_key(&mut source, Key::Char('c'), Modifiers::CTRL); + send_key(&mut source, Key::Char('8'), Modifiers::NONE); + + adopt_next_buffer(&mut source, "source"); + adopt_next_buffer(&mut observer, "observer"); + + let done = |t: &str| t.contains("[compile exited with code 0]"); + let src_text = pump_until_text(&mut source, Duration::from_secs(15), "source run", done); + let obs_text = pump_until_text(&mut observer, Duration::from_secs(15), "observer run", done); + assert_eq!(src_text, obs_text, "byte-identical convergence"); + assert!( + src_text.contains("\nX\n\u{e9}\nX\n"), + "whole-codepoint overwrites replicate as valid UTF-8; got:\n{src_text:?}" + ); +}