diff --git a/docs/active-work.md b/docs/active-work.md index 2a87a12..bba7579 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -332,9 +332,22 @@ from #171 and #215 — the correction the 1b lane missed, honoured here. `Accepted`, or is `Refused`) because four rows in these rounds passed vacuously: cells that were out of grid, or that clamped to byte 0, exercised a refusal instead of the path they named. - - **REMAINING, in order:** task 18's pending-release slot and drains, - task 19's four stranding transitions, then the full head-exact gate - and the PR. + - **LANDED: the pending-release SLOT and its drain order (task 18).** + Cancellation parks the record instead of returning it into a + context that drops it — two of the three cancellation sites are + inside frame production, where no target effect can run. The drain + pays it **before any subsequent panel-pointer effect**, **before + detach teardown**, and **at the projection seam** between + `render_frame` returning and its messages being written. + - **Witnessed: Q1, Q2, Q3, Q4, Q6.** Q3 asserts ORDER, not arrival, + and the mutation that keeps the drain but moves it after the + press effect fails exactly that assertion. + - **Q5 is OWED.** The projection-seam drain needs a row that drives + the real per-frontend frame loop; the unit rows call + `render_frame` directly and never enter it. Recorded rather than + treated as covered by its neighbours. + - **REMAINING, in order:** task 19's four stranding transitions, Q5's + acceptance-shaped row, then the full head-exact gate and the PR. - **Two test seams added for this:** an opt-in child-input tap (`start_send_tap_for_test`) and a drag-state read (`view_is_dragging_for_test`). Nothing else exposes what the child diff --git a/src/daemon.rs b/src/daemon.rs index 776c032..0cdcae1 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -1019,6 +1019,40 @@ fn panel_event_epochs_are_current( .is_some_and(|geometry| geometry.geometry_epoch == geometry_epoch) } +/// Deliver the release this frontend is owed, if any (parent 48). +/// +/// **Order is the ruling, not just the existence of a slot.** A +/// cancellation raised inside frame production cannot deliver its own +/// release, so the record is parked; this is where it is paid. It runs +/// at three points, and each is chosen against a specific way the +/// release would otherwise arrive too late or not at all: +/// +/// * **before any subsequent panel-pointer effect**, so the old +/// gesture's release reaches the child ahead of the new gesture's +/// press rather than after it; +/// * **before detach teardown**, because detach removes the state that +/// holds the record and there is no later opportunity; +/// * **after semantic projection returns and before its messages are +/// written**, so the successor frame cannot overtake the release its +/// own new mapping required. +/// +/// The synthetic release carries NO modifiers: nothing is physically +/// held, and inventing a modifier state would report a chord the user +/// never made. +fn drain_pending_release( + editor: &mut EditorState, + semantic_states: &mut HashMap, + source: FrontendId, +) { + let Some(record) = semantic_states + .get_mut(&source) + .and_then(crate::semantic_render::SemanticRenderState::take_pending_release) + else { + return; + }; + editor.complete_panel_gesture(source, &record, pmacs_protocol::Modifiers::default()); +} + /// Parent 48 Q#BP-R4 — the authoritative lifecycle table. /// /// The disposition is decided BEFORE any target effect, and the live @@ -1043,6 +1077,12 @@ fn replay_panel_pointer( use crate::editor::PanelPointerOutcome as Outcome; use pmacs_protocol::{MouseButton, MouseKind}; + // DRAIN FIRST. An owed release has to reach the child before this + // gesture's press does; draining afterwards would put them on the + // wire in the wrong order, which reads to the child as a press + // followed by a release of the gesture BEFORE it. + drain_pending_release(editor, semantic_states, source); + let disposition = editor.classify_panel_pointer(source, buffer_id, coord, kind); let outcome = disposition.outcome(); if outcome == Outcome::Refused { @@ -1540,6 +1580,16 @@ fn dispatcher_loop( render_state.render_frame(editor, *fid, &terminal_snapshots, &other_presences) }; + // Parent 48 — THE PROJECTION SEAM. A mapping-generation + // advance cancels the live gesture INSIDE `render_frame`, + // while the successor `PresentMapped` is still being built, + // so "before that frame is produced" is not a place that + // exists. This is the first point that is: projection has + // returned and none of what it returned has been written. + // Draining here keeps the release ahead of the frame whose + // own new mapping is what required it. + drain_pending_release(editor, &mut semantic_states, *fid); + // Vterm Stage 3 — a semantic frontend showing a terminal has // no document cursor: the identity buffer is empty, so a // `CursorByte` would describe byte 0 of a buffer with no @@ -2928,6 +2978,9 @@ fn handle_dispatcher_event( } } DispatcherEvent::SessionDetached { frontend_id } => { + // Before ANY teardown: the next line drops the state that + // holds an owed release, and detach has no later chance. + drain_pending_release(editor, semantic_states, frontend_id); render_states.remove(&frontend_id); semantic_states.remove(&frontend_id); streams.remove(&frontend_id); @@ -8391,6 +8444,304 @@ mod tests { ); } + // ----------------------------------------------------------------- + // Q1-Q6 — the pending-release slot, and the ORDER it drains in. + // + // A cancellation raised inside frame production cannot deliver its + // own release. The slot is where the record waits; these rows are + // about it being paid, and paid at the right moment. + // ----------------------------------------------------------------- + + /// Drive a real `SessionDetached` through the dispatcher. + fn detach_session( + editor: &mut crate::editor::EditorState, + semantic_states: &mut HashMap, + render_states: &mut HashMap, + fid: FrontendId, + ) { + let mut streams = HashMap::new(); + let mut term_sizes = HashMap::new(); + let mut last_idle = HashMap::new(); + let mut last_active = HashMap::new(); + let mut bells = HashMap::new(); + let mut registry = SessionRegistry::new(); + registry.register_session(fid, session(LEGACY_PANEL_VERSION, true)); + handle_dispatcher_event( + DispatcherEvent::SessionDetached { frontend_id: fid }, + editor, + render_states, + semantic_states, + &mut streams, + &mut term_sizes, + &mut last_idle, + &mut last_active, + &mut bells, + &mut registry, + ); + } + + /// Cancel the live gesture from INSIDE PROJECTION, by taking the + /// panel away. + /// + /// `publish_absent_panel` cancels while `render_frame` is building + /// the frame, which is the shape these rows are about: a + /// cancellation with nowhere to deliver its release. + /// + /// A mapping-generation advance is the other such trigger and would + /// read more naturally, but it cannot be driven on a TERMINAL + /// panel: that key tracks the screen and anchor, not the buffer, so + /// a foreign edit does not move it. `Absent` is family-independent + /// and reaches the same parking path. + /// Returns the epochs a later gesture must echo: re-showing the + /// panel ships a NEW declaration, and the old epochs are stale. + fn cancel_by_panel_absence( + editor: &crate::editor::EditorState, + states: &mut HashMap, + fid: FrontendId, + ) -> (u64, u64) { + editor.hide_panel_for_test(fid); + { + let sem = states.get_mut(&fid).expect("projection"); + let _ = sem.render_frame(editor); + } + // Re-shown and re-declared, because these rows are about what + // happens to the OWED RELEASE afterwards. A panel left `Absent` + // fails the inbound ladder, so no later gesture would reach the + // drain at all and the row would be observing the ladder rather + // than the slot. + editor.show_panel_for_test(fid); + shipped_declaration(editor, fid, states) + } + + /// Q1/Q2 — a cancelled gesture's release is PARKED and then PAID. + /// + /// The cancellation happens inside projection, where no target + /// effect can run; dropping the record there is how the child ended + /// up holding a button with nothing left to lift it. + #[test] + fn q1_q2_a_cancelled_gesture_release_is_parked_then_delivered() { + let fid = FrontendId(810); + let (mut editor, mut states, mut render, _panel, buffer_id, epochs) = + terminal_panel_session(fid, true); + let cell = pmacs_protocol::CellCoord::new(1, 2); + let press = pmacs_protocol::MouseKind::Down(pmacs_protocol::MouseButton::Left); + let none = pmacs_protocol::Modifiers::default(); + + send_panel( + &mut editor, + &mut states, + &mut render, + fid, + epochs, + buffer_id, + cell, + press, + none, + ); + assert_eq!( + child_stream(&editor), + vec![SGR_PRESS_1_2.to_vec()], + "fixture: the press reached the child" + ); + + let epochs = cancel_by_panel_absence(&editor, &mut states, fid); + assert!( + !states[&fid].has_accepted_gesture(), + "fixture: the advance cancelled the gesture" + ); + assert!( + states[&fid].has_pending_release(), + "Q1: the record must be PARKED, not returned into a context \ + that drops it" + ); + assert!( + child_stream(&editor).is_empty(), + "and not delivered from inside projection, which cannot run \ + a target effect" + ); + + // A later panel event: the drain runs ahead of it. + send_panel( + &mut editor, + &mut states, + &mut render, + fid, + epochs, + buffer_id, + cell, + press, + none, + ); + + let sent = child_stream(&editor); + assert!( + sent.contains(&SGR_RELEASE_1_2.to_vec()), + "Q2: the parked release must be PAID --- parking it and never \ + draining leaves the child exactly as stranded, got {sent:?}" + ); + } + + /// Q3 — the owed release reaches the child BEFORE the next press. + /// + /// Draining after the dispatch instead of before puts them on the + /// wire reversed, which the child reads as a press followed by the + /// release of the gesture before it. + #[test] + fn q3_the_owed_release_precedes_the_next_press() { + let fid = FrontendId(811); + let (mut editor, mut states, mut render, _panel, buffer_id, epochs) = + terminal_panel_session(fid, true); + let cell = pmacs_protocol::CellCoord::new(1, 2); + let press = pmacs_protocol::MouseKind::Down(pmacs_protocol::MouseButton::Left); + let none = pmacs_protocol::Modifiers::default(); + + send_panel( + &mut editor, + &mut states, + &mut render, + fid, + epochs, + buffer_id, + cell, + press, + none, + ); + let epochs = cancel_by_panel_absence(&editor, &mut states, fid); + let _ = child_stream(&editor); + + send_panel( + &mut editor, + &mut states, + &mut render, + fid, + epochs, + buffer_id, + cell, + press, + none, + ); + + let sent = child_stream(&editor); + let release_at = sent.iter().position(|b| b == SGR_RELEASE_1_2); + let press_at = sent.iter().position(|b| b == SGR_PRESS_1_2); + assert!( + release_at.is_some() && press_at.is_some(), + "fixture: both the owed release and the new press must be on \ + the wire, got {sent:?}" + ); + assert!( + release_at < press_at, + "Q3: ORDER, not merely arrival --- the old gesture's release \ + must precede the new gesture's press, got {sent:?}" + ); + } + + /// Q4 — detach pays what it owes before tearing the state down. + /// + /// `SessionDetached` drops `semantic_states` for the frontend, and + /// there is no later opportunity: a release not delivered here is + /// never delivered. + #[test] + fn q4_detach_delivers_an_owed_release_before_teardown() { + let fid = FrontendId(812); + let (mut editor, mut states, mut render, _panel, buffer_id, epochs) = + terminal_panel_session(fid, true); + let cell = pmacs_protocol::CellCoord::new(1, 2); + let press = pmacs_protocol::MouseKind::Down(pmacs_protocol::MouseButton::Left); + let none = pmacs_protocol::Modifiers::default(); + + send_panel( + &mut editor, + &mut states, + &mut render, + fid, + epochs, + buffer_id, + cell, + press, + none, + ); + // Detach never sends another gesture, so the fresh epochs are + // not needed here --- only the parked release is. + let _ = cancel_by_panel_absence(&editor, &mut states, fid); + assert!( + states[&fid].has_pending_release(), + "fixture: a release is owed" + ); + let _ = child_stream(&editor); + + detach_session(&mut editor, &mut states, &mut render, fid); + + assert_eq!( + child_stream(&editor), + vec![SGR_RELEASE_1_2.to_vec()], + "Q4: the owed release must be paid before teardown --- the \ + next statement in that arm drops the state holding it" + ); + assert!( + !states.contains_key(&fid), + "fixture: and the teardown really did run" + ); + } + + // Q5 IS NOT WITNESSED HERE, and this is why. + // + // The third drain sits in the daemon's per-frontend frame loop, + // between `sem.render_frame(editor)` returning and the loop that + // writes what it returned. These rows drive `render_frame` + // directly, so they never enter that loop and cannot observe the + // seam; the two drains they DO reach — before a panel-pointer + // effect, and before detach teardown — are witnessed by Q1-Q4. + // + // The seam is still load-bearing: a cancellation raised inside + // projection with no following panel event and no detach would + // otherwise let the successor frame reach the frontend ahead of the + // release its own new mapping required. Witnessing it needs a row + // that drives the real frame loop, which is acceptance-suite shaped + // rather than unit shaped. Recorded as OWED rather than assumed + // covered by its neighbours. + + /// Q6 — the arm-over-pending invariant, as a BACKSTOP. + /// + /// The ordering is what prevents this; the assertion exists so that + /// a future path which cancels twice without draining is loud + /// rather than silently losing one release. + #[test] + fn q6_arming_never_happens_over_a_pending_release() { + let fid = FrontendId(813); + let (mut editor, mut states, mut render, _panel, buffer_id, epochs) = + terminal_panel_session(fid, true); + let cell = pmacs_protocol::CellCoord::new(1, 2); + let press = pmacs_protocol::MouseKind::Down(pmacs_protocol::MouseButton::Left); + let none = pmacs_protocol::Modifiers::default(); + + for _ in 0..3 { + send_panel( + &mut editor, + &mut states, + &mut render, + fid, + epochs, + buffer_id, + cell, + press, + none, + ); + let _ = cancel_by_panel_absence(&editor, &mut states, fid); + assert!( + states[&fid].has_pending_release(), + "fixture: each round parks a release" + ); + } + + assert!( + !states[&fid].has_pending_release() || states[&fid].accepted_gesture().is_none(), + "Q6: a release may be owed, or a gesture armed, but arming \ + OVER an owed release would lose one --- the drain before \ + each effect is what keeps these from overlapping" + ); + } + /// P12 — a panel WIDER THAN 512 COLUMNS still routes pointer input. /// /// A panel deliberately does not inherit the terminal's per-axis PTY diff --git a/src/editor.rs b/src/editor.rs index 40e0ec7..7be75ce 100644 --- a/src/editor.rs +++ b/src/editor.rs @@ -2948,6 +2948,14 @@ impl EditorState { } } + /// Re-show this frontend's panel, for the Q rows. + #[doc(hidden)] + pub fn show_panel_for_test(&self, frontend_id: FrontendId) { + if let Some(view) = self.core.borrow_mut().views.get_mut(&frontend_id) { + view.panel_hidden = false; + } + } + /// Classify an authenticated panel gesture, WITHOUT applying it /// (Q#BP-R4). /// diff --git a/src/semantic_render.rs b/src/semantic_render.rs index 4758b9c..d4c9d1c 100644 --- a/src/semantic_render.rs +++ b/src/semantic_render.rs @@ -464,6 +464,21 @@ pub struct SemanticRenderState { /// match the press it ends, and a stale `Up` with no accepted /// `Down` must be inert rather than synthesising one. accepted_gesture: Option, + /// §5b/parent 48 — a release this frontend is OWED, parked until + /// somewhere that can deliver it. + /// + /// Two of the three cancellation sites are inside frame production + /// — the mapping-generation advance and `publish_absent_panel` — + /// where no target effect can run. `cancel_accepted_gesture` + /// returned the record into those contexts and they dropped it, so + /// the gesture ended with the child still holding its button. + /// + /// **A SLOT, not a queue.** The latch holds at most one gesture per + /// frontend, so at most one release can be owed, and the bound is + /// structural rather than a cap someone had to choose. The drain + /// runs ahead of the next panel-pointer effect, so a second + /// cancellation cannot arrive while one is still parked. + pending_release: Option, /// §5b — how many armed gestures an authority loss has ended this /// session. /// @@ -686,6 +701,7 @@ impl SemanticRenderState { panel_mapping: None, panel_mapping_exhausted: false, accepted_gesture: None, + pending_release: None, panel_gesture_cancellations: 0, panel_epoch_used: 0, panel_presentation: None, @@ -776,12 +792,42 @@ impl SemanticRenderState { /// without inventing a second cancellation. pub fn cancel_accepted_gesture(&mut self) -> Option { let cancelled = self.accepted_gesture.take(); - if cancelled.is_some() { + if let Some(record) = cancelled { self.panel_gesture_cancellations = self.panel_gesture_cancellations.saturating_add(1); + // PARKED, not just returned. Returning was the whole bug: + // the callers inside frame production cannot deliver a + // release, so the record went out of scope and the gesture + // ended with nothing terminated. + // + // Overwriting a still-parked release would lose one, so it + // is an invariant violation rather than a silent drop. It is + // a BACKSTOP: the ordering — drain before the next effect — + // is what actually prevents it. + debug_assert!( + self.pending_release.is_none(), + "a release was still owed when another gesture was \ + cancelled; the drain must run before any subsequent \ + panel-pointer effect" + ); + self.pending_release = Some(record); } cancelled } + /// Take the release this frontend is owed, if any. + /// + /// The RETURN of `cancel_accepted_gesture` is for inspection; THIS + /// is the delivery path. + pub fn take_pending_release(&mut self) -> Option { + self.pending_release.take() + } + + /// Whether a release is still owed, for assertions. + #[must_use] + pub fn has_pending_release(&self) -> bool { + self.pending_release.is_some() + } + /// The live gesture record, if any — parent 48 Q#BP-R4's /// "live record" test, and the source of the recorded completion. #[must_use]