diff --git a/docs/active-work.md b/docs/active-work.md index 36a99de..a46a908 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -494,14 +494,14 @@ implemented and in review.** #173 also changes `src/editor.rs`, so gates were rerun on the merge result, not the old combination). Five commits: the classified census routing, the painter extraction + acceptance, the lane record, then - the round-1 and round-2 review fixes. **No protocol change; no behavior + the round-1, round-2 and round-3 review fixes. **No protocol change; no behavior change for any frontend today** — with `panel_capable = false` for semantic sessions, `primary_document_window` returns `view.active` in every existing configuration, so this is seam adoption that becomes load-bearing in 2B. - Verification on the merge result: `cargo fmt --check` clean; strict - workspace Clippy clean; **1,832 default + 2,014 CRDT** library tests; - `bottom_panel_stage2a_acceptance` **16**; bottom-panel Stage 1 46; + workspace Clippy clean; **1,832 default + 2,015 CRDT** library tests; + `bottom_panel_stage2a_acceptance` **17**; bottom-panel Stage 1 46; statusline segments 8 CRDT; m11_5 semantic 2 CRDT; GPU initial target 14 CRDT; terminal config 12 CRDT; vterm Stage 1/2 10 / 6; folding Stage 2 48; M4 121; required GPU 202; `git diff --check` clean. @@ -521,7 +521,12 @@ implemented and in review.** under test; (c) a discriminating fixture must make the two routings DISAGREE — comparing two non-terminal buffers, or two windows with no selection, yields the same answer either way and proves nothing. - Round 2 found four of my own pins vacuous by exactly these shapes. + Round 2 found four of my own pins vacuous by exactly these shapes, and + round 3 found two more problems of the same family: a pin placed at a + HELPER while production called it from a producer (reverting only the + producer's call site left every test green), and a socket-pair + assertion whose blocking read made a regression HANG instead of fail. + Both now assert at the producer, with read timeouts on every read. - **Review round 1 closed: 4 P1 + 2 P2, all real.** The P1s were a stale-`Pointer` focus steal (the failed-alignment arm returned the window, so #8's activation focused it before `dispatch_pointer` diff --git a/src/daemon.rs b/src/daemon.rs index 63f6b87..84716eb 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -3483,6 +3483,96 @@ mod tests { ); } + /// Bottom-panel §1.3 #21 through the REAL producer (round 3). + /// + /// A semantic peer with a FOCUSED PANEL must still receive the + /// snapshot for the buffer on its DOCUMENT surface, and must NOT + /// receive one for a buffer visible only in its panel. Asserting the + /// helper alone was insufficient: reverting the producer's call site + /// to focused-window routing left every helper-level test green. + #[cfg(feature = "crdt")] + #[test] + fn snapshot_publication_follows_the_document_under_a_focused_panel() { + let (editor, fid, document, panel) = panel_focused_semantic_fixture(); + let (doc_buf, panel_buf) = { + let core = editor.core.borrow(); + ( + core.windows[&document].buffer_id, + core.windows[&panel].buffer_id, + ) + }; + assert_ne!(doc_buf, panel_buf, "fixture: distinct buffers"); + + let caps = crate::protocol::NegotiatedCapabilities { + multi_frontend: true, + crdt_replica: true, + semantic_render: true, + }; + let mut registry = SessionRegistry::new(); + registry.register_session( + fid, + crate::presence::SessionState::new(PROTOCOL_VERSION, caps, 0), + ); + + // The DOCUMENT buffer's snapshot must be delivered. + { + let (server, mut client) = UnixStream::pair().expect("socketpair"); + // A read timeout on the DELIVERY read too. Without it a + // regression that suppresses the snapshot makes this test + // HANG rather than fail, which is strictly worse than a red + // assertion — found by biting this very test. + client + .set_read_timeout(Some(Duration::from_millis(500))) + .expect("delivery timeout"); + let mut streams = HashMap::from([(fid, server)]); + let message = InstanceMessage::BufferSnapshot { + buffer_id: doc_buf, + crdt_snapshot: vec![1, 2, 3], + }; + publish_buffer_snapshot_to_replicas( + &editor, + doc_buf, + &message, + ®istry, + &mut streams, + &mut HashMap::new(), + ); + let delivered: InstanceMessage = + read_message(&mut client).expect("the document snapshot must arrive"); + assert_eq!( + delivered, message, + "#21: a buffer on the DOCUMENT surface must still be published while a \ + panel holds focus" + ); + } + + // The PANEL-only buffer's snapshot must NOT be delivered. + { + let (server, mut client) = UnixStream::pair().expect("socketpair"); + let mut streams = HashMap::from([(fid, server)]); + let message = InstanceMessage::BufferSnapshot { + buffer_id: panel_buf, + crdt_snapshot: vec![4, 5, 6], + }; + publish_buffer_snapshot_to_replicas( + &editor, + panel_buf, + &message, + ®istry, + &mut streams, + &mut HashMap::new(), + ); + client + .set_read_timeout(Some(Duration::from_millis(50))) + .expect("timeout"); + assert!( + read_message::(&mut client).is_err(), + "#21: a buffer visible only in a PANEL must not replace the peer's \ + document mirror" + ); + } + } + // ---- GPU terminal input: the double terminal-layout sync ------------- // // These drive `sync_terminal_layouts_for_tick` — the REAL dispatcher loop @@ -4827,15 +4917,13 @@ mod tests { "#3: CursorByte must describe the DOCUMENT surface" ); - // #21 publication recipient filter, both directions. - assert!( - peer_displays_buffer_as_document(&editor, fid, doc_buf), - "#21: a buffer visible in the document must still receive publications while a panel holds focus" - ); - assert!( - !peer_displays_buffer_as_document(&editor, fid, panel_buf), - "#21: a buffer visible only in a panel must NOT replace the document mirror" - ); + // #21 is deliberately NOT asserted here. Round 3: pinning it at + // this helper left the real producer free to regress — reverting + // the call site inside `publish_buffer_snapshot_to_replicas` + // kept both this test and the existing socket-pair test green. + // It is pinned through the producer instead, in + // `snapshot_publication_follows_the_document_under_a_focused_panel`. + let _ = panel_buf; } /// Bottom-panel §1.3 #2 — the sharpest census case: the lazy CRDT diff --git a/src/semantic_render.rs b/src/semantic_render.rs index af08741..db96fc4 100644 --- a/src/semantic_render.rs +++ b/src/semantic_render.rs @@ -625,6 +625,22 @@ impl SemanticRenderState { // post-evaluation face inventory must then precede the authoritative // segment replacement in this same frame. Unsupported peers skip the // evaluator entirely and therefore pay no Lua callback/dynamic-face cost. + // Bottom-panel A2A-2, round 3: the document identity used to + // FILTER the results must be the PRE-CALLBACK one. Both outcome + // arms carry phase-1 contexts, and a provider that closes the + // primary document split changes `primary_document_window` + // mid-evaluation — reading it after the fact would compare + // phase-1 contexts against a replacement identity, match + // nothing, and silently suppress the authoritative clear. + let statusline_document_window = self + .peer_knows_statusline_segments + .then(|| { + state + .core + .borrow() + .primary_document_window(self.frontend_id) + }) + .flatten(); let statusline_evaluation = self.peer_knows_statusline_segments.then(|| { evaluate_statusline( state.lua_host.lua(), @@ -810,11 +826,7 @@ impl SemanticRenderState { out.extend(self.font_facts_msg(state)); // Q#SL6/Q#SL8: face inventory must precede segment text. if let Some(evaluation) = statusline_evaluation { - let document_window = state - .core - .borrow() - .primary_document_window(self.frontend_id); - self.emit_statusline_segments(evaluation, document_window, &mut out); + self.emit_statusline_segments(evaluation, statusline_document_window, &mut out); } out } @@ -859,6 +871,16 @@ impl SemanticRenderState { // Evaluate callbacks before `ThemeFacts` for the same reason the // document path does: a callback may register a face, and the // face inventory must precede the segment text that names it. + // Same pre-callback capture as the document path (round 3). + let statusline_document_window = self + .peer_knows_statusline_segments + .then(|| { + state + .core + .borrow() + .primary_document_window(self.frontend_id) + }) + .flatten(); let statusline_evaluation = self.peer_knows_statusline_segments.then(|| { evaluate_statusline( state.lua_host.lua(), @@ -885,7 +907,12 @@ impl SemanticRenderState { // a verdict we hold. if self.last_terminal_frame.as_ref() == Some(&frame) { self.terminal_error_latched = false; - out.extend(self.terminal_chrome(state, buffer_id, statusline_evaluation)); + out.extend(self.terminal_chrome( + state, + buffer_id, + statusline_evaluation, + statusline_document_window, + )); return Some(out); } match frame.validate() { @@ -909,7 +936,12 @@ impl SemanticRenderState { } } - out.extend(self.terminal_chrome(state, buffer_id, statusline_evaluation)); + out.extend(self.terminal_chrome( + state, + buffer_id, + statusline_evaluation, + statusline_document_window, + )); Some(out) } @@ -924,6 +956,7 @@ impl SemanticRenderState { state: &EditorState, buffer_id: BufferId, statusline_evaluation: Option, + statusline_document_window: Option, ) -> Vec { let mut out = Vec::new(); out.extend(self.status_facts_msg(state, buffer_id)); @@ -933,11 +966,7 @@ impl SemanticRenderState { out.extend(self.font_facts_msg(state)); // Q#SL6/Q#SL8: face inventory must precede segment text. if let Some(evaluation) = statusline_evaluation { - let document_window = state - .core - .borrow() - .primary_document_window(self.frontend_id); - self.emit_statusline_segments(evaluation, document_window, &mut out); + self.emit_statusline_segments(evaluation, statusline_document_window, &mut out); } out } diff --git a/tests/bottom_panel_stage2a_acceptance.rs b/tests/bottom_panel_stage2a_acceptance.rs index 1063e9f..d39d804 100644 --- a/tests/bottom_panel_stage2a_acceptance.rs +++ b/tests/bottom_panel_stage2a_acceptance.rs @@ -784,3 +784,115 @@ fn consumer_decorations_follow_the_document_selection_not_the_panel() { "a selection living in the focused PANEL must not decorate the document viewport" ); } + +#[test] +fn a_provider_closing_the_document_split_still_clears_the_statusline() { + use pmacs::protocol::{ByteRange, InstanceMessage}; + use pmacs::semantic_render::SemanticRenderState; + + // Round 3 finding 1. `authoritative_empty` carries PHASE-1 contexts, + // so the identity used to filter them must be the PRE-CALLBACK one. + // A provider that closes the primary document split changes + // `primary_document_window` mid-evaluation; reading it afterwards + // compares phase-1 contexts against a replacement identity, matches + // nothing, and silently suppresses the authoritative clear — leaving + // stale statusline text on screen forever. + // + // Driven on LOCAL, because the Lua window API acts on the ACTIVE + // FRONTEND: a synthetic semantic view would be untouched by + // `pmacs.window.close()` and the identity would never change, which + // is exactly how the first version of this test came back vacuous. + // TWO document windows plus the panel: closing the only document + // window is structurally refused (Q#BP6 forbids a lone side window + // as a resting state), so the first attempt could not change the + // identity at all. Distinct buffers make the target selectable from + // a Lua provider, which has no focus-by-id. + let s = editor(); + exec( + &s, + "DOC_A = pmacs.buffer.create(\"*doc-a*\") + DOC_B = pmacs.buffer.create(\"*doc-b*\") + pmacs.window.display(DOC_A, {}) + pmacs.window.split_horizontal() + pmacs.window.focus_next() + pmacs.window.display(DOC_B, {})", + ); + let (_origin, panel) = focused_panel(&s); + let (document, doc_buf) = { + let core = s.core.borrow(); + let win = core + .primary_document_window(FrontendId::LOCAL) + .expect("a primary document window"); + (win, core.windows[&win].buffer_id) + }; + assert_ne!(document, panel); + + let mut sem = SemanticRenderState::for_peer(FrontendId::LOCAL, 18); + sem.set_viewport(doc_buf, ByteRange { start: 0, end: 0 }, 0); + + // Seed a baseline payload so a CLEAR is observable as a change. + exec( + &s, + r"_G.SL_SEED = pmacs.statusline.register { + name='seed', side='left', priority=10, + fn=function() return 'OLD' end, + }", + ); + let seeded = sem.render_frame(&s); + assert!( + seeded + .iter() + .any(|m| matches!(m, InstanceMessage::StatuslineSegments { .. })), + "non-vacuity: a baseline payload must exist before we test its clear" + ); + + // A provider that unregisters itself (making the evaluation + // Invalidated) AND closes the captured document window. `close()` + // closes the ACTIVE window and Lua has no focus-by-id, so step + // around the ring until the captured buffer is current. + s.lua_host + .lua() + .globals() + .set("TARGET_BUF", pmacs::lua_bindings::BufferIdLua(doc_buf)) + .expect("expose the target buffer"); + exec( + &s, + r"_G.SL_CLOSER = pmacs.statusline.register { + name='closer', side='left', priority=100, + fn=function() + pmacs.statusline.unregister(SL_CLOSER) + for _ = 1, 8 do + if pmacs.window.buffer() == TARGET_BUF then break end + pmacs.window.focus_next() + end + pmacs.window.close() + return 'STALE' + end, + }", + ); + + let msgs = sem.render_frame(&s); + + // The fixture must actually have changed the identity, or this test + // discriminates nothing. + assert_ne!( + s.core.borrow().primary_document_window(FrontendId::LOCAL), + Some(document), + "fixture: the callback must really have changed the document identity" + ); + + let cleared = msgs.iter().any(|m| match m { + InstanceMessage::StatuslineSegments { + buffer_id, + left, + right, + .. + } => *buffer_id == doc_buf && left.is_empty() && right.is_empty(), + _ => false, + }); + assert!( + cleared, + "an invalidated evaluation must still publish the authoritative EMPTY clear for \ + the phase-1 document identity, even when a callback closed that window; got {msgs:?}" + ); +}