From 4e9420a69acb6210231394d1ddfc3fb88e8ed12b Mon Sep 17 00:00:00 2001 From: Levi Neuwirth Date: Fri, 21 Aug 2026 10:40:54 +0200 Subject: [PATCH] fix(panel): a live gesture is paid BEFORE its replacement press lands Answers review of ab8ddae. The entry drain could not close this: it looks for an OWED release, and a gesture that is still LIVE owes nothing yet. Arming was what cancelled it, and arming runs after the replacement press has already reached the target --- so a second press with the first never released put `old press, new press, old release` on the wire. Two presses outstanding, then a release arriving for the wrong one. The Down arm now ends the live gesture and drains it before applying the replacement, so the child sees `old press, old release, new press`. The invariant moved to where it is relied on. arm_accepted_gesture now asserts that neither a live gesture nor an owed release remains, at the point of ARMING rather than inside cancellation --- arming is what the ordering protects, and checking during cancellation cannot see the case where nothing has been cancelled yet. The defensive cancel stays for release builds, because parking late is recoverable and overwriting is not. Q6 was rewritten, because the old one never sent a second press while the first was live and so could not observe any of this; its final assertion also ran after a further cancellation. It now expects the exact bytes `release(1,2), press(2,4)` in that order. Both layers are witnessed separately. Reverting the ordering trips the new debug assertion at the point of arming; reverting it AND compiling that assertion out --- which is what a release build does --- fails the byte-order assertion instead, with the child receiving only the new press. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_016bqGA6s9tTUFzYpbeW3tai --- docs/active-work.md | 17 +++++-- src/daemon.rs | 111 ++++++++++++++++++++++++++++++----------- src/semantic_render.rs | 30 ++++++++--- 3 files changed, 119 insertions(+), 39 deletions(-) diff --git a/docs/active-work.md b/docs/active-work.md index bba7579..d2717aa 100644 --- a/docs/active-work.md +++ b/docs/active-work.md @@ -339,9 +339,20 @@ from #171 and #215 — the correction the 1b lane missed, honoured here. 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. + - **A LIVE gesture is ended and PAID before a replacement press + lands.** The entry drain alone was not enough: it looks for an + OWED release, and a live gesture owes nothing yet — arming was + what cancelled it, which happens after the replacement has + already reached the target. The child saw `old press, new press, + old release`. + - **Witnessed: Q1, Q2, Q3, Q4, Q6.** Q3 asserts ORDER, not arrival. + Q6 sends a second press with the first still live and expects + exactly `old release, new press` in the child's stream. Both + layers of Q6 are proven separately: the invariant now asserts at + the point of ARMING (not inside cancellation) and fires in debug, + and with that assert compiled out the byte-order assertion + catches the same defect — which is what a release build relies + on. - **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 diff --git a/src/daemon.rs b/src/daemon.rs index 0cdcae1..17d2b97 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -1100,6 +1100,23 @@ fn replay_panel_pointer( // A chrome press begins nothing. return; } + // A SECOND PRESS WITH THE FIRST STILL LIVE. The entry drain + // above saw nothing, because the old gesture had not been + // cancelled yet --- it was live, not owed. Arming used to do + // the cancelling, which happens AFTER this press has already + // reached the target, so the child received + // `old press, new press, old release`. + // + // End the old gesture and PAY it here, before the + // replacement lands, so the wire carries + // `old press, old release, new press`. + if live { + if let Some(state) = semantic_states.get_mut(&source) { + let _ = state.cancel_accepted_gesture(); + } + drain_pending_release(editor, semantic_states, source); + } + // ARMED FROM THE EFFECT RESULT, and only if there was one. // `None` means the target refused the press --- a terminal // view that is gone, for instance --- and arming over that @@ -7904,6 +7921,8 @@ mod tests { const SGR_RELEASE_1_2: &[u8] = b"\x1b[<0;3;2m"; /// The same release with SHIFT held: the button code gains 4. const SGR_RELEASE_1_2_SHIFT: &[u8] = b"\x1b[<4;3;2m"; + /// A left press at cell (2, 4), the replacement gesture's cell. + const SGR_PRESS_2_4: &[u8] = b"\x1b[<0;5;3M"; /// A panel session whose side window holds a live TERMINAL, with /// the send tap armed. @@ -8701,48 +8720,80 @@ mod tests { // rather than unit shaped. Recorded as OWED rather than assumed // covered by its neighbours. - /// Q6 — the arm-over-pending invariant, as a BACKSTOP. + /// Q6 — a SECOND PRESS with the first still live: the old release + /// reaches the child before the new press. /// - /// 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. + /// This is the ordering the entry drain alone cannot give. That + /// drain looks for an OWED release, and a live gesture owes + /// nothing yet — it is cancelled by arming, which happens after the + /// replacement press has already reached the target. The child then + /// saw `old press, new press, old release`: two presses + /// outstanding, and a release arriving for the wrong one. + /// + /// An earlier version of this row never sent a second press while + /// the first was live, so it could not observe any of that. #[test] - fn q6_arming_never_happens_over_a_pending_release() { + fn q6_a_second_press_pays_the_first_before_it_lands() { 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 first = pmacs_protocol::CellCoord::new(1, 2); + let second = pmacs_protocol::CellCoord::new(2, 4); 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" - ); - } - + send_panel( + &mut editor, + &mut states, + &mut render, + fid, + epochs, + buffer_id, + first, + press, + none, + ); + assert_eq!( + child_stream(&editor), + vec![SGR_PRESS_1_2.to_vec()], + "fixture: the first press reached the child" + ); 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" + states[&fid].has_accepted_gesture(), + "fixture: and it is still LIVE --- no release was sent" + ); + + // The second press, with the first never released. + send_panel( + &mut editor, + &mut states, + &mut render, + fid, + epochs, + buffer_id, + second, + press, + none, + ); + + assert_eq!( + child_stream(&editor), + vec![SGR_RELEASE_1_2.to_vec(), SGR_PRESS_2_4.to_vec()], + "the first gesture's release must reach the child BEFORE the \ + replacement press --- the release is at the first gesture's \ + own cell, and it comes first" + ); + assert!( + states[&fid].has_accepted_gesture(), + "and the replacement is armed" + ); + assert!( + !states[&fid].has_pending_release(), + "with nothing left owed" ); } - /// P12 — a panel WIDER THAN 512 COLUMNS still routes pointer input. + /// P12 — a panel WIDER THAN 512 COLUMNS still routes pointer input. /// P12 — a panel WIDER THAN 512 COLUMNS still routes pointer input. /// /// A panel deliberately does not inherit the terminal's per-axis PTY /// caps (Bet B5'): a 4K surface at a small font is legitimately diff --git a/src/semantic_render.rs b/src/semantic_render.rs index d4c9d1c..e3cb9cb 100644 --- a/src/semantic_render.rs +++ b/src/semantic_render.rs @@ -757,12 +757,30 @@ impl SemanticRenderState { /// per §5b's split table, because the release it denies does not /// exist on this branch. pub fn arm_accepted_gesture(&mut self, gesture: AcceptedPanelGesture) { - // A second accepted press while one is already armed means the - // first gesture's release never arrived — a dropped `Up`, or an - // outbox that closed under a stall. END it rather than - // overwrite it: overwriting discards the record silently, and - // once replay attaches effects to that record the child is left - // holding a button down with nothing left to release it. + // THE INVARIANT IS ASSERTED WHERE IT IS RELIED ON. A caller must + // have ended AND paid any live gesture before arming a + // replacement: a second accepted press while one is armed means + // the first gesture's release never arrived, and its release has + // to reach the target before this press does. Cancelling here + // instead is too late by exactly one effect --- the replacement + // has already landed. + // + // Checked at the point of arming rather than inside + // cancellation, because arming is what the ordering protects. + debug_assert!( + self.accepted_gesture.is_none(), + "arming over a LIVE gesture: the caller must cancel and \ + drain it first, or the replacement press overtakes the old \ + gesture's release" + ); + debug_assert!( + self.pending_release.is_none(), + "arming over an OWED release: the drain must run before the \ + effect that arms" + ); + // Defensive in release builds: ending it parks the record for a + // later drain, which is late but not lost. Overwriting would + // discard it outright and leave the child holding a button. self.cancel_accepted_gesture(); self.accepted_gesture = Some(gesture); }