fix(daemon): stop resizing a semantic frontend's PTY twice per tick
The dispatcher loop applied BOTH terminal-layout syncs to EVERY attached frontend. A semantic session satisfies both conditions, because it has a term_sizes entry from AttachRequest and a semantic terminal declaration, so its PTY was resized twice on every tick forever: the grid arm installed the TUI placement size, the semantic arm installed the declared content rectangle, and each arm's own idempotence guard only ever saw the size the other had just written. The child took a SIGWINCH storm at tick cadence and the screen reflowed continuously, which is what made typing into a GPU terminal impossible while output kept flowing. The grid arm is also the only per-tick controller-liveness release a semantic frontend gets, so simply skipping it for those frontends trades one defect for another: a GPU window that switches away from its terminal would hold the controller forever, and no peer could resize that PTY again. The semantic arm cannot take over that job, because the buffer-follow snapshot clears the viewport declaration that would drive it. sync_terminal_layout is therefore split into a frontend-kind-neutral half (panel reconciliation plus controller liveness, which read only views, windows and the controller) and a grid-only geometry half (TUI placement plus the resize). The dispatcher runs the neutral half for every attached frontend once per tick, then exactly one geometry arm per frontend kind. sync_terminal_layout survives as the composition of both halves, so the in-process editor loop and LOCAL are unchanged. The loop body is extracted into sync_terminal_layouts_for_tick, which makes the grid/semantic exclusivity structural rather than two adjacent ifs, and lets the tests drive the real loop body instead of a re-implementation. The release that fires when a window has no placement stays in the grid half deliberately: a semantic frontend has no window_placements entry at all, so moving it into the neutral half would release a GPU session's controller on every tick. Bite-verified against two pre-images, because one is not enough here -- the naive guard fixes the storm and introduces the controller leak, so a single revert would score the fix complete when it is not: pin main naive guard split settle (acc 2+3) FAIL pass pass controller release (acc 6) pass FAIL pass grid still resizes (acc 5) pass pass pass Real-path acceptance: a quiet child that counts SIGWINCH reports 144 frames in 4 s and WINCH 1..12 on screen against the pre-fix tree, versus a settled screen with the fix. Acceptance 4 (input reaches the child and returns) is a keep-working pin and passes on both sides -- key transport was never the defect. No protocol change; stays v20.
This commit is contained in:
parent
9a0df21618
commit
b775f1703a
|
|
@ -295,6 +295,16 @@ change. Stays v20.
|
|||
## Deferred (named)
|
||||
|
||||
- Interactive-shell echo on a raw-mode PTY (Q#GT5) — its own scout.
|
||||
- **A geometry change appears to clear the visible screen.** Observed while
|
||||
building acceptance 4: after the probe's deliberate 25×92 → 20×71 resize,
|
||||
the next frame's visible grid is entirely blank even though the content
|
||||
(two short lines near the top) should survive a shrink of that size. It
|
||||
reproduces on the pre-fix tree, so it is neither caused nor fixed here, and
|
||||
it is why acceptance 4 latches its observation across frames instead of
|
||||
reading the final one. Not investigated: it could be correct reflow
|
||||
behaviour given where the child leaves its cursor (frames show the cursor
|
||||
on the bottom row), or a real reflow defect. Named because the next person
|
||||
to write a resize assertion will hit it.
|
||||
- `TerminalFrame` suppression including `screen_generation` in its equality:
|
||||
correct today and load-bearing for correctness, but it means any future
|
||||
content-neutral generation bump re-emits a frame. Recorded, not changed.
|
||||
|
|
@ -349,16 +359,29 @@ mutation (`src/terminal/screen.rs:1467`), so with a quiet child it is a
|
|||
6. A semantic frontend whose window stops showing the terminal **releases its
|
||||
controller** (Q#GT4), pinned through the extracted loop body — driven by an
|
||||
actual buffer switch, not by calling the release directly. This one bites
|
||||
against revision 1's guard as well as against `main`.
|
||||
against **revision 1's naive guard**, and deliberately **passes on `main`**:
|
||||
today's sibling arms do supply the release, by the accident of the grid arm
|
||||
running for a frontend it should never have run for. It is the pin that
|
||||
stops the fix from trading one defect for another.
|
||||
7. End-to-end SIGWINCH count through the real PTY: a child trapping `WINCH`
|
||||
and printing a **fresh distinct breadcrumb per signal** (`WINCH 1`,
|
||||
`WINCH 2`, …) shows a bounded count. The distinctness is load-bearing —
|
||||
the established PTY-paint trap is that cell diffing skips both spaces and
|
||||
already-matching cells, so a repeated identical marker can assert nothing.
|
||||
8. Bite-verified: reverting the split fails 2, 3, 6 and 7 specifically.
|
||||
Criteria 1 and 7 fail against `main`; criterion 6 fails against **both**
|
||||
`main` and revision 1's guard, which is the point of keeping B1's
|
||||
half-false score on the record.
|
||||
8. Bite-verified against **two** pre-images, because one is not enough here —
|
||||
the naive guard fixes the storm and introduces a different defect, so a
|
||||
single revert would score the fix complete when it is not. Measured
|
||||
(`cargo test --lib`, manual revert since these tests share `src/daemon.rs`
|
||||
with the production code):
|
||||
|
||||
| pin | `main` (sibling arms) | rev-1 naive guard | the split |
|
||||
|---|---|---|---|
|
||||
| acc 2+3 settle | **FAIL** | pass | pass |
|
||||
| acc 6 controller release | pass | **FAIL** | pass |
|
||||
| acc 5 grid still resizes | pass | pass | pass |
|
||||
|
||||
The middle column is B1's half-false score made executable: the naive
|
||||
guard's first clause holds (the storm stops) and its second does not.
|
||||
|
||||
Criteria 1, 2 and 7 are deliberately expressed as **quiet-child** assertions,
|
||||
because the existing acceptance's chatty child is exactly what hid this.
|
||||
|
|
|
|||
|
|
@ -728,7 +728,22 @@ fn run_headless_probe(socket: &Path, report: &Path) -> i32 {
|
|||
let _ = client.send_key(ProtocolKey::Char(chord), Modifiers::CTRL | Modifiers::ALT);
|
||||
}
|
||||
|
||||
let deadline = std::time::Instant::now() + std::time::Duration::from_secs(20);
|
||||
// Quiet-observation mode. `PMACS_GPU_PROBE_OBSERVE_MS` makes the probe
|
||||
// send NO input and request NO resize, and observe for exactly that long
|
||||
// instead of stopping at its usual condition.
|
||||
//
|
||||
// This exists because the ordinary probe cannot see a frame storm: it
|
||||
// stops as soon as it has watched a resize land, so a session emitting a
|
||||
// frame every tick and one emitting three in total both satisfy it. A
|
||||
// fixed window over a child that produces no output turns "how many
|
||||
// frames did the daemon send?" into a number worth asserting on.
|
||||
let observe_window = std::env::var("PMACS_GPU_PROBE_OBSERVE_MS")
|
||||
.ok()
|
||||
.and_then(|value| value.parse::<u64>().ok())
|
||||
.map(std::time::Duration::from_millis);
|
||||
let quiet = observe_window.is_some();
|
||||
let deadline = std::time::Instant::now()
|
||||
+ observe_window.unwrap_or_else(|| std::time::Duration::from_secs(20));
|
||||
let mut sent_input = false;
|
||||
let mut sent_resize = false;
|
||||
while std::time::Instant::now() < deadline {
|
||||
|
|
@ -778,13 +793,17 @@ fn run_headless_probe(socket: &Path, report: &Path) -> i32 {
|
|||
if pixels.iter().any(|&b| b != first) {
|
||||
facts.rendered_nonuniform_frames += 1;
|
||||
}
|
||||
if !sent_input && facts.frames >= 1 {
|
||||
if facts.last_frame_text.contains(PROBE_INPUT_CHAR) {
|
||||
facts.input_echo_observed = true;
|
||||
}
|
||||
if !quiet && !sent_input && facts.frames >= 1 {
|
||||
sent_input = true;
|
||||
// Real child input over the real wire.
|
||||
let _ = client.send_key(ProtocolKey::Char('x'), Modifiers::NONE);
|
||||
let _ =
|
||||
client.send_key(ProtocolKey::Char(PROBE_INPUT_CHAR), Modifiers::NONE);
|
||||
let _ = client.send_key(ProtocolKey::Enter, Modifiers::NONE);
|
||||
}
|
||||
if !sent_resize && facts.frames >= 2 {
|
||||
if !quiet && !sent_resize && facts.frames >= 2 {
|
||||
sent_resize = true;
|
||||
state.resize(700, 500);
|
||||
if let Some((buffer_id, size)) = state.terminal_declaration_if_changed()
|
||||
|
|
@ -800,7 +819,7 @@ fn run_headless_probe(socket: &Path, report: &Path) -> i32 {
|
|||
facts.observed_resized_frame = true;
|
||||
}
|
||||
}
|
||||
if facts.observed_resized_frame && facts.rendered_nonuniform_frames >= 2 {
|
||||
if !quiet && facts.observed_resized_frame && facts.rendered_nonuniform_frames >= 2 {
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
|
@ -832,6 +851,7 @@ fn run_headless_probe(socket: &Path, report: &Path) -> i32 {
|
|||
let _ = writeln!(out, "resized_cols={}", facts.resized_cols);
|
||||
let _ = writeln!(out, "last_title={}", facts.last_title.unwrap_or_default());
|
||||
let _ = writeln!(out, "last_frame_text={}", facts.last_frame_text);
|
||||
let _ = writeln!(out, "input_echo_observed={}", facts.input_echo_observed);
|
||||
let _ = writeln!(out, "disconnect={}", facts.disconnect.unwrap_or_default());
|
||||
if let Err(error) = std::fs::write(report, out) {
|
||||
eprintln!(
|
||||
|
|
@ -1037,9 +1057,20 @@ struct ProbeFacts {
|
|||
resized_cols: u32,
|
||||
last_title: Option<String>,
|
||||
last_frame_text: String,
|
||||
/// Whether any frame carried the probe's own typed character back.
|
||||
///
|
||||
/// Latched ACROSS frames, not read off the final one: a later geometry
|
||||
/// change reflows the screen, so "the echo arrived" and "the echo is
|
||||
/// still on the last frame" are different questions and only the first
|
||||
/// one is about input reaching the child.
|
||||
input_echo_observed: bool,
|
||||
disconnect: Option<String>,
|
||||
}
|
||||
|
||||
/// The character the probe types into the child. Distinct from anything the
|
||||
/// acceptance children print themselves, so its appearance is unambiguous.
|
||||
const PROBE_INPUT_CHAR: char = 'x';
|
||||
|
||||
/// One-line printable text of a terminal frame, for probe reporting.
|
||||
fn frame_probe_text(frame: &TerminalFrame) -> String {
|
||||
let mut text = String::new();
|
||||
|
|
|
|||
297
src/daemon.rs
297
src/daemon.rs
|
|
@ -1536,23 +1536,7 @@ fn dispatcher_loop(
|
|||
// Accepted terminal context controls PTY size. Apply any focus,
|
||||
// window, or resize changes before consuming another child-output
|
||||
// batch so screen reflow and subsequent bytes share one geometry.
|
||||
for frontend_id in &attached_fids {
|
||||
if let Some(size) = term_sizes.get(frontend_id).copied() {
|
||||
editor.sync_terminal_layout(*frontend_id, size);
|
||||
}
|
||||
// Vterm Stage 3 — the semantic twin, right beside the grid
|
||||
// sync so both frontend kinds resize the screen before the
|
||||
// next child-output drain. The frontend declared a CONTENT
|
||||
// rectangle, so this consumes the size directly instead of
|
||||
// running the TUI placement helper, which would subtract a
|
||||
// modeline the GPU never drew.
|
||||
if let Some((buffer_id, size)) = semantic_states
|
||||
.get(frontend_id)
|
||||
.and_then(crate::semantic_render::SemanticRenderState::terminal_viewport)
|
||||
{
|
||||
editor.sync_semantic_terminal_layout(*frontend_id, buffer_id, size);
|
||||
}
|
||||
}
|
||||
sync_terminal_layouts_for_tick(editor, &attached_fids, &term_sizes, &semantic_states);
|
||||
|
||||
// `tick_async` last: the M4.5 async bridge settles awaiters
|
||||
// inside `tick_lsp` (via the message bus); draining + resuming
|
||||
|
|
@ -3064,6 +3048,56 @@ fn build_presence_snapshot(editor: &EditorState, frontend_id: FrontendId) -> Pre
|
|||
}
|
||||
}
|
||||
|
||||
/// One dispatcher tick's terminal-layout step, for every attached frontend.
|
||||
///
|
||||
/// Extracted from the dispatcher loop so the grid/semantic exclusivity is
|
||||
/// **structural** rather than two adjacent `if`s, and so acceptance tests can
|
||||
/// drive the real loop body instead of re-implementing it (Q#GT1).
|
||||
///
|
||||
/// The shape that matters: liveness is frontend-kind NEUTRAL and runs for
|
||||
/// everyone, exactly once; the geometry arms are EXCLUSIVE alternatives keyed
|
||||
/// on the same `semantic_states` membership that session establishment uses,
|
||||
/// so a session can never be caught by both.
|
||||
///
|
||||
/// Before this existed, both arms ran for every frontend. A semantic session
|
||||
/// has a `term_sizes` entry (from `AttachRequest`) *and* a terminal
|
||||
/// declaration, so its PTY was resized twice per tick, forever: the grid arm
|
||||
/// installed the TUI placement size, the semantic arm installed the declared
|
||||
/// content rectangle, and each arm's own idempotence guard saw only the size
|
||||
/// the other had just written. The child got a `SIGWINCH` storm at tick
|
||||
/// cadence, which is what made typing into a GPU terminal impossible while
|
||||
/// output kept flowing.
|
||||
fn sync_terminal_layouts_for_tick(
|
||||
editor: &mut EditorState,
|
||||
attached_fids: &[FrontendId],
|
||||
term_sizes: &HashMap<FrontendId, CellSize>,
|
||||
semantic_states: &HashMap<FrontendId, crate::semantic_render::SemanticRenderState>,
|
||||
) {
|
||||
for frontend_id in attached_fids {
|
||||
// Neutral half: panel reconciliation (Q#BP2b's only per-tick
|
||||
// enforcement point) and the release of a controller whose window
|
||||
// moved away. A semantic frontend gets this from nowhere else —
|
||||
// its own arm stops running the moment the buffer-follow snapshot
|
||||
// clears the declaration (Q#GT4/Q#GT7).
|
||||
editor.sync_terminal_controller_liveness(*frontend_id);
|
||||
|
||||
// Geometry: exactly one arm per frontend kind.
|
||||
if let Some(state) = semantic_states.get(frontend_id) {
|
||||
// Vterm Stage 3 — the frontend declared a CONTENT rectangle,
|
||||
// so this consumes the size directly instead of running the
|
||||
// TUI placement helper, which would subtract a modeline the
|
||||
// GPU never drew. A semantic frontend with no declaration yet
|
||||
// gets NO resize at all, which is correct: the terminal keeps
|
||||
// the geometry it was opened with until one arrives.
|
||||
if let Some((buffer_id, size)) = state.terminal_viewport() {
|
||||
editor.sync_semantic_terminal_layout(*frontend_id, buffer_id, size);
|
||||
}
|
||||
} else if let Some(size) = term_sizes.get(frontend_id).copied() {
|
||||
editor.sync_terminal_grid_geometry(*frontend_id, size);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Dispatch a semantic (grid-less) frontend's input event into the
|
||||
/// shared editor core (Phase B, session B1). Mirrors the `Key` / `Mouse`
|
||||
/// arms of [`apply_event`] but takes no `RenderState` — a semantic
|
||||
|
|
@ -3365,6 +3399,235 @@ mod tests {
|
|||
);
|
||||
}
|
||||
|
||||
// ---- GPU terminal input: the double terminal-layout sync -------------
|
||||
//
|
||||
// These drive `sync_terminal_layouts_for_tick` — the REAL dispatcher loop
|
||||
// body, not a re-implementation of it. That distinction is the whole
|
||||
// point: the Stage 3 acceptance sent input through `client.send_key`
|
||||
// directly and therefore pinned transport rather than routing, which is
|
||||
// how the defect these pin shipped.
|
||||
//
|
||||
// The observable is `TerminalScreen::generation`. It advances once per
|
||||
// screen mutation, so with a child that produces no output and no
|
||||
// `tick_processes` call, "generation stopped advancing" is exactly "the
|
||||
// geometry settled" — a state predicate, not a readout.
|
||||
|
||||
/// Open a quiet terminal and give `frontend_id` a view that shows it,
|
||||
/// holding its controller — the state the dispatcher loop runs against.
|
||||
fn quiet_terminal_for(
|
||||
editor: &EditorState,
|
||||
frontend_id: FrontendId,
|
||||
) -> (crate::buffer::BufferId, crate::window::WindowId) {
|
||||
let mut spec = crate::terminal::TerminalSpec::new("/bin/sh");
|
||||
spec.args = vec!["-c".into(), "sleep 30".into()];
|
||||
spec.rows = 24;
|
||||
spec.cols = 80;
|
||||
let buffer_id = editor
|
||||
.terminal_manager
|
||||
.borrow_mut()
|
||||
.open(
|
||||
spec,
|
||||
&mut editor.core.borrow_mut(),
|
||||
&mut editor.process_supervisor.borrow_mut(),
|
||||
)
|
||||
.expect("open terminal");
|
||||
|
||||
let window_id = crate::window::WindowId::next();
|
||||
{
|
||||
let mut core = editor.core.borrow_mut();
|
||||
let text_view = {
|
||||
let registry = core.registry.clone();
|
||||
let registry = registry.borrow();
|
||||
let buffer = registry.get(buffer_id).expect("terminal buffer");
|
||||
crate::text_view::TextView::new(buffer)
|
||||
};
|
||||
core.windows.insert(
|
||||
window_id,
|
||||
crate::window::Window::new(window_id, buffer_id, text_view),
|
||||
);
|
||||
core.register_frontend_view(
|
||||
frontend_id,
|
||||
crate::window::FrontendView {
|
||||
layout: crate::window::Layout::single(window_id),
|
||||
active: window_id,
|
||||
fold_projection: true,
|
||||
panel_capable: false,
|
||||
frame_geometry: None,
|
||||
panel_hidden: false,
|
||||
},
|
||||
);
|
||||
}
|
||||
let key = crate::terminal::TerminalViewKey::new(frontend_id, window_id, buffer_id);
|
||||
let mut manager = editor.terminal_manager.borrow_mut();
|
||||
manager.register_view(key);
|
||||
manager.claim_controller(key);
|
||||
(buffer_id, window_id)
|
||||
}
|
||||
|
||||
fn screen_generation(editor: &EditorState, buffer_id: crate::buffer::BufferId) -> u64 {
|
||||
editor
|
||||
.terminal_manager
|
||||
.borrow()
|
||||
.snapshot(buffer_id)
|
||||
.expect("terminal snapshot")
|
||||
.screen_generation
|
||||
}
|
||||
|
||||
/// Acceptance 2 and 3: one declaration produces exactly one resize, and
|
||||
/// the screen then STAYS at the declared content rectangle.
|
||||
///
|
||||
/// Against the pre-split tree both arms ran for the semantic frontend and
|
||||
/// generation advanced by two per iteration forever, because each arm's
|
||||
/// idempotence guard only ever saw the size the other had just written.
|
||||
#[test]
|
||||
fn semantic_terminal_geometry_settles_after_one_declaration() {
|
||||
let fid = FrontendId(41);
|
||||
let mut editor = EditorState::new();
|
||||
let (buffer_id, _window) = quiet_terminal_for(&editor, fid);
|
||||
|
||||
// The GPU declares a CONTENT rectangle; the grid size it also
|
||||
// reported at attach is deliberately DIFFERENT, which is the
|
||||
// collision the defect fed on.
|
||||
let declared = CellSize::new(25, 92);
|
||||
let mut semantic = crate::semantic_render::SemanticRenderState::for_peer(fid, 20);
|
||||
semantic.set_terminal_viewport(buffer_id, declared);
|
||||
let semantic_states = HashMap::from([(fid, semantic)]);
|
||||
let term_sizes = HashMap::from([(fid, CellSize::new(24, 80))]);
|
||||
let attached = vec![fid];
|
||||
|
||||
sync_terminal_layouts_for_tick(&mut editor, &attached, &term_sizes, &semantic_states);
|
||||
let after_first = screen_generation(&editor, buffer_id);
|
||||
assert_eq!(
|
||||
editor.terminal_manager.borrow().screen_size(buffer_id),
|
||||
Some(declared),
|
||||
"the declared content rectangle must win"
|
||||
);
|
||||
|
||||
// Acceptance 2: every further tick is a no-op.
|
||||
for _ in 0..8 {
|
||||
sync_terminal_layouts_for_tick(&mut editor, &attached, &term_sizes, &semantic_states);
|
||||
}
|
||||
assert_eq!(
|
||||
screen_generation(&editor, buffer_id),
|
||||
after_first,
|
||||
"an unchanged declaration must not mutate the screen again \
|
||||
(pre-split: +2 per tick, forever)"
|
||||
);
|
||||
// Acceptance 3: the state predicate, not "a frame at this width
|
||||
// arrived at some point".
|
||||
assert_eq!(
|
||||
editor.terminal_manager.borrow().screen_size(buffer_id),
|
||||
Some(declared),
|
||||
"the geometry must SETTLE at the declared rectangle"
|
||||
);
|
||||
|
||||
editor.process_supervisor.borrow_mut().shutdown();
|
||||
}
|
||||
|
||||
/// Acceptance 6: a semantic frontend whose window switches away releases
|
||||
/// its terminal controller.
|
||||
///
|
||||
/// This bites against BOTH the pre-split tree's sibling arms and against
|
||||
/// the naive "skip the grid arm for semantic frontends" guard, which is
|
||||
/// why B1 is recorded as half-false. The release cannot live in
|
||||
/// `sync_semantic_terminal_layout`: the buffer-follow snapshot clears the
|
||||
/// viewport declaration, so that arm stops running in exactly this case —
|
||||
/// modelled here by dropping the declaration alongside the switch.
|
||||
#[test]
|
||||
fn semantic_frontend_releases_its_terminal_controller_when_its_window_switches_away() {
|
||||
let fid = FrontendId(42);
|
||||
let mut editor = EditorState::new();
|
||||
let (buffer_id, window_id) = quiet_terminal_for(&editor, fid);
|
||||
|
||||
let declared = CellSize::new(25, 92);
|
||||
let mut semantic = crate::semantic_render::SemanticRenderState::for_peer(fid, 20);
|
||||
semantic.set_terminal_viewport(buffer_id, declared);
|
||||
let mut semantic_states = HashMap::from([(fid, semantic)]);
|
||||
let term_sizes = HashMap::from([(fid, CellSize::new(24, 80))]);
|
||||
let attached = vec![fid];
|
||||
|
||||
sync_terminal_layouts_for_tick(&mut editor, &attached, &term_sizes, &semantic_states);
|
||||
assert_eq!(
|
||||
editor
|
||||
.terminal_manager
|
||||
.borrow()
|
||||
.controller_view_for_frontend(fid),
|
||||
Some(crate::terminal::TerminalViewKey::new(
|
||||
fid, window_id, buffer_id
|
||||
)),
|
||||
"precondition: the frontend holds the controller"
|
||||
);
|
||||
|
||||
// The window switches to a document, and the snapshot that announces
|
||||
// it clears the semantic declaration — `on_buffer_snapshot_sent`.
|
||||
let document = editor.core.borrow().registry.borrow_mut().create("doc");
|
||||
{
|
||||
let mut core = editor.core.borrow_mut();
|
||||
let text_view = {
|
||||
let registry = core.registry.clone();
|
||||
let registry = registry.borrow();
|
||||
let buffer = registry.get(document).expect("document buffer");
|
||||
crate::text_view::TextView::new(buffer)
|
||||
};
|
||||
let window = core.windows.get_mut(&window_id).expect("window");
|
||||
*window = crate::window::Window::new(window_id, document, text_view);
|
||||
}
|
||||
semantic_states
|
||||
.get_mut(&fid)
|
||||
.expect("semantic state")
|
||||
.on_buffer_snapshot_sent(document);
|
||||
|
||||
sync_terminal_layouts_for_tick(&mut editor, &attached, &term_sizes, &semantic_states);
|
||||
assert_eq!(
|
||||
editor
|
||||
.terminal_manager
|
||||
.borrow()
|
||||
.controller_view_for_frontend(fid),
|
||||
None,
|
||||
"a semantic frontend that left its terminal must release the \
|
||||
controller, or no peer can resize that PTY again"
|
||||
);
|
||||
|
||||
editor.process_supervisor.borrow_mut().shutdown();
|
||||
}
|
||||
|
||||
/// Acceptance 5 at the unit seam: a GRID frontend still gets its
|
||||
/// placement-derived resize. The split must not turn the storm fix into
|
||||
/// "semantic frontends win everywhere".
|
||||
#[test]
|
||||
fn grid_terminal_geometry_still_syncs_for_a_grid_frontend() {
|
||||
let fid = FrontendId(43);
|
||||
let mut editor = EditorState::new();
|
||||
let (buffer_id, _window) = quiet_terminal_for(&editor, fid);
|
||||
|
||||
let semantic_states = HashMap::new();
|
||||
let term_sizes = HashMap::from([(fid, CellSize::new(40, 100))]);
|
||||
let attached = vec![fid];
|
||||
|
||||
let before = editor.terminal_manager.borrow().screen_size(buffer_id);
|
||||
sync_terminal_layouts_for_tick(&mut editor, &attached, &term_sizes, &semantic_states);
|
||||
let after = editor.terminal_manager.borrow().screen_size(buffer_id);
|
||||
|
||||
assert_ne!(before, after, "a grid frontend must still resize its PTY");
|
||||
assert_eq!(
|
||||
after.map(|size| size.cols),
|
||||
Some(100),
|
||||
"the grid arm supplies the full declared width"
|
||||
);
|
||||
// And it too settles.
|
||||
let settled = screen_generation(&editor, buffer_id);
|
||||
for _ in 0..4 {
|
||||
sync_terminal_layouts_for_tick(&mut editor, &attached, &term_sizes, &semantic_states);
|
||||
}
|
||||
assert_eq!(
|
||||
screen_generation(&editor, buffer_id),
|
||||
settled,
|
||||
"an unchanged grid size must not mutate the screen again"
|
||||
);
|
||||
|
||||
editor.process_supervisor.borrow_mut().shutdown();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn frontend_events_from_uninstalled_sessions_are_dropped_without_state_access() {
|
||||
let source = FrontendId(77);
|
||||
|
|
|
|||
|
|
@ -1189,14 +1189,84 @@ impl EditorState {
|
|||
let _ = self.terminal_manager.borrow_mut().release_controller(key);
|
||||
}
|
||||
|
||||
/// Reconcile panels and release a controller whose window moved away.
|
||||
///
|
||||
/// **Frontend-kind neutral, and deliberately so** (Q#GT1/Q#GT4): this
|
||||
/// half reads only `core.views`, `core.windows`, and the controller —
|
||||
/// never a grid size — so it is the half the dispatcher runs for EVERY
|
||||
/// attached frontend once per tick. It was previously fused into
|
||||
/// [`Self::sync_terminal_layout`], which meant a semantic frontend got
|
||||
/// its controller-liveness release only as a side effect of a grid
|
||||
/// resize it should never have received.
|
||||
///
|
||||
/// [`Self::sync_semantic_terminal_layout`] cannot substitute for this:
|
||||
/// when a GPU window switches away from its terminal, the buffer-follow
|
||||
/// snapshot clears the viewport declaration
|
||||
/// (`SemanticRenderState::on_buffer_snapshot_sent`), so the semantic arm
|
||||
/// stops running entirely in exactly the case that needs the release.
|
||||
///
|
||||
/// Returns `true` while `frontend_id` still holds a live controller.
|
||||
pub fn sync_terminal_controller_liveness(&mut self, frontend_id: FrontendId) -> bool {
|
||||
// Bottom-panel arc (Q#BP2b): a panel that just became
|
||||
// unsatisfiable must have released its controller before any
|
||||
// resize runs, or the child would be resized against a dead rect.
|
||||
// This is the contract's only per-tick enforcement point, and it
|
||||
// stays neutral so semantic frontends keep it (Q#GT7).
|
||||
self.reconcile_panel_layout(frontend_id);
|
||||
let Some(key) = self
|
||||
.terminal_manager
|
||||
.borrow()
|
||||
.controller_view_for_frontend(frontend_id)
|
||||
else {
|
||||
return false;
|
||||
};
|
||||
let core = self.core.borrow();
|
||||
let Some(view) = core.views.get(&frontend_id) else {
|
||||
drop(core);
|
||||
let _ = self.terminal_manager.borrow_mut().release_controller(key);
|
||||
return false;
|
||||
};
|
||||
if view.active != key.window_id
|
||||
|| core
|
||||
.windows
|
||||
.get(&key.window_id)
|
||||
.is_none_or(|window| window.buffer_id != key.buffer_id)
|
||||
{
|
||||
drop(core);
|
||||
let _ = self.terminal_manager.borrow_mut().release_controller(key);
|
||||
return false;
|
||||
}
|
||||
true
|
||||
}
|
||||
|
||||
/// Resize the one session durably controlled by `frontend_id`.
|
||||
///
|
||||
/// This is called before process drain and paint, never from rendering.
|
||||
///
|
||||
/// Composition of the two halves, preserved verbatim for the in-process
|
||||
/// `editor::run` loop and `LOCAL`. The daemon dispatcher calls the halves
|
||||
/// separately, because only the geometry half is grid-specific.
|
||||
pub fn sync_terminal_layout(&mut self, frontend_id: FrontendId, term_size: CellSize) -> bool {
|
||||
// Bottom-panel arc (Q#BP2b): a panel that just became
|
||||
// unsatisfiable must have released its controller before this
|
||||
// runs, or the child would be resized against a dead rect.
|
||||
self.reconcile_panel_layout(frontend_id);
|
||||
self.sync_terminal_controller_liveness(frontend_id)
|
||||
&& self.sync_terminal_grid_geometry(frontend_id, term_size)
|
||||
}
|
||||
|
||||
/// The grid half: TUI placement plus the resize it implies.
|
||||
///
|
||||
/// **Grid frontends only** (Q#GT1). The placement lookup below is why:
|
||||
/// a semantic frontend has no `window_placements` entry at all, so the
|
||||
/// "no placement" arm would release its controller on EVERY tick. That
|
||||
/// release reads like liveness and is not — it is grid geometry, and
|
||||
/// moving it into [`Self::sync_terminal_controller_liveness`] would
|
||||
/// reintroduce this framing's own defect in a new place.
|
||||
///
|
||||
/// Assumes liveness already ran: the controller is live and its window
|
||||
/// still shows the terminal.
|
||||
pub fn sync_terminal_grid_geometry(
|
||||
&mut self,
|
||||
frontend_id: FrontendId,
|
||||
term_size: CellSize,
|
||||
) -> bool {
|
||||
let Some(key) = self
|
||||
.terminal_manager
|
||||
.borrow()
|
||||
|
|
@ -1206,23 +1276,11 @@ impl EditorState {
|
|||
};
|
||||
let content = {
|
||||
let core = self.core.borrow();
|
||||
let Some(view) = core.views.get(&frontend_id) else {
|
||||
let _ = self.terminal_manager.borrow_mut().release_controller(key);
|
||||
return false;
|
||||
};
|
||||
if view.active != key.window_id
|
||||
|| core
|
||||
.windows
|
||||
.get(&key.window_id)
|
||||
.is_none_or(|window| window.buffer_id != key.buffer_id)
|
||||
{
|
||||
let _ = self.terminal_manager.borrow_mut().release_controller(key);
|
||||
return false;
|
||||
}
|
||||
let Some(placement) = window_placements(&core, frontend_id, term_size)
|
||||
.get(&key.window_id)
|
||||
.copied()
|
||||
else {
|
||||
drop(core);
|
||||
let _ = self.terminal_manager.borrow_mut().release_controller(key);
|
||||
return false;
|
||||
};
|
||||
|
|
|
|||
|
|
@ -1087,3 +1087,228 @@ fn a28_a30_a_v18_semantic_peer_has_no_terminal_surface() {
|
|||
.terminate(terminal_buffer, &mut state.process_supervisor.borrow_mut())
|
||||
.expect("terminate child");
|
||||
}
|
||||
|
||||
// ---- GPU terminal input: the double terminal-layout sync -----------------
|
||||
//
|
||||
// Acceptance 1, 4 and 7 of `docs/gpu-terminal-input-framing.md`, on the real
|
||||
// path: real daemon, real PTY child, real `pmacs-gpu` attach client.
|
||||
//
|
||||
// `a37` above passes on the broken tree, and these are shaped around exactly
|
||||
// why. Its child prints 400 rows on a timer, so a frame storm hides inside
|
||||
// legitimate output; its only frame-count assertion is `frames >= 2`; and its
|
||||
// resize assertion is satisfied by a geometry that oscillates THROUGH the
|
||||
// asserted width. The children below are therefore deliberately QUIET, and
|
||||
// the assertions are upper bounds.
|
||||
|
||||
/// A terminal child that produces nothing on its own and prints one fresh,
|
||||
/// DISTINCT breadcrumb per `SIGWINCH`.
|
||||
///
|
||||
/// Distinctness is load-bearing: `cell::diff` skips both spaces and
|
||||
/// already-matching cells, so a repeated identical marker can never be
|
||||
/// asserted on — the second and later copies would paint nothing.
|
||||
#[cfg(feature = "crdt")]
|
||||
const WINCH_PROBE_INIT_LUA: &str = r#"
|
||||
pmacs.command.define {
|
||||
name = "vterm-probe.open",
|
||||
description = "Open a quiet terminal that counts SIGWINCH.",
|
||||
fn = function()
|
||||
return pmacs.terminal.open {
|
||||
command = "/bin/sh",
|
||||
args = { "-c",
|
||||
"n=0; trap 'n=$((n+1)); printf \"WINCH %d\r\n\" \"$n\"' WINCH; " ..
|
||||
"printf 'READY\r\n'; while :; do sleep 0.2; done" },
|
||||
}
|
||||
end,
|
||||
}
|
||||
pmacs.keymap.bind { scope = "global", sequence = "C-M-t", command = "vterm-probe.open" }
|
||||
"#;
|
||||
|
||||
/// A terminal child that echoes input by copying stdin to stdout.
|
||||
///
|
||||
/// `cat` is the right instrument precisely because it does NOT echo: termios
|
||||
/// `ECHO` is off on a `TerminalMode::Raw` PTY, so nothing in the kernel line
|
||||
/// discipline reflects the byte. `cat` copies it exactly once, which makes a
|
||||
/// single typed character produce a single unambiguous cell.
|
||||
#[cfg(feature = "crdt")]
|
||||
const CAT_PROBE_INIT_LUA: &str = r#"
|
||||
pmacs.command.define {
|
||||
name = "vterm-probe.open",
|
||||
description = "Open a terminal child that copies stdin to stdout.",
|
||||
fn = function()
|
||||
return pmacs.terminal.open {
|
||||
command = "/bin/sh",
|
||||
args = { "-c", "printf 'READY\r\n'; exec cat" },
|
||||
}
|
||||
end,
|
||||
}
|
||||
pmacs.keymap.bind { scope = "global", sequence = "C-M-t", command = "vterm-probe.open" }
|
||||
"#;
|
||||
|
||||
/// Run the headless GPU probe against a daemon built from `init_lua`, and
|
||||
/// return its parsed report. `observe_ms` selects quiet-observation mode.
|
||||
#[cfg(feature = "crdt")]
|
||||
fn run_gpu_probe(
|
||||
init_lua: &str,
|
||||
observe_ms: Option<u64>,
|
||||
) -> Option<std::collections::HashMap<String, String>> {
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
fn gpu_binary() -> PathBuf {
|
||||
Path::new(env!("CARGO_BIN_EXE_pmacs"))
|
||||
.parent()
|
||||
.expect("test binary directory")
|
||||
.join("pmacs-gpu")
|
||||
}
|
||||
|
||||
let required = std::env::var_os("PMACS_REQUIRE_GPU").is_some();
|
||||
let binary = gpu_binary();
|
||||
if !binary.exists() {
|
||||
assert!(
|
||||
!required,
|
||||
"PMACS_REQUIRE_GPU is set but {} is not built",
|
||||
binary.display()
|
||||
);
|
||||
eprintln!("skipping: {} is not built", binary.display());
|
||||
return None;
|
||||
}
|
||||
|
||||
let daemon = common::daemon::TestDaemon::spawn_with_env_and_init(
|
||||
&[
|
||||
("PMACS_INSTANCE_SEMANTIC_RENDER", "1"),
|
||||
("PMACS_INSTANCE_MULTI_FRONTEND", "1"),
|
||||
],
|
||||
init_lua,
|
||||
);
|
||||
let report = daemon
|
||||
.socket_path()
|
||||
.parent()
|
||||
.expect("socket parent")
|
||||
.join("gpu-probe.txt");
|
||||
let mut command = std::process::Command::new(&binary);
|
||||
command
|
||||
.arg("--headless-probe")
|
||||
.arg(daemon.socket_path())
|
||||
.arg(&report)
|
||||
.env("PMACS_GPU_PROBE_OPEN_KEY", "t");
|
||||
if let Some(ms) = observe_ms {
|
||||
command.env("PMACS_GPU_PROBE_OBSERVE_MS", ms.to_string());
|
||||
}
|
||||
let output = command.output().expect("run the headless GPU probe");
|
||||
if !output.status.success() {
|
||||
let stderr = String::from_utf8_lossy(&output.stderr);
|
||||
let no_adapter = output.status.code() == Some(3);
|
||||
assert!(
|
||||
no_adapter && !required,
|
||||
"headless GPU probe failed (status {:?}):\n{stderr}",
|
||||
output.status.code()
|
||||
);
|
||||
eprintln!("skipping: no wgpu adapter available");
|
||||
return None;
|
||||
}
|
||||
let text = std::fs::read_to_string(&report).expect("probe report");
|
||||
Some(
|
||||
text.lines()
|
||||
.filter_map(|line| line.split_once('='))
|
||||
.map(|(key, value)| (key.to_owned(), value.to_owned()))
|
||||
.collect(),
|
||||
)
|
||||
}
|
||||
|
||||
/// Acceptance 1 and 7: a GPU session showing a quiet terminal must settle.
|
||||
///
|
||||
/// Both assertions are upper bounds over a fixed observation window, which is
|
||||
/// the only shape that can see this defect. On the pre-fix tree the dispatcher
|
||||
/// resized the PTY twice per tick forever, so the child took a `SIGWINCH`
|
||||
/// storm and the daemon emitted a terminal frame per tick — measured at ~730
|
||||
/// frames in 20 s against a child that printed one line and then slept.
|
||||
#[cfg(feature = "crdt")]
|
||||
#[test]
|
||||
fn gpu_terminal_geometry_settles_and_stops_signalling_the_child() {
|
||||
const OBSERVE_MS: u64 = 4_000;
|
||||
let Some(facts) = run_gpu_probe(WINCH_PROBE_INIT_LUA, Some(OBSERVE_MS)) else {
|
||||
return;
|
||||
};
|
||||
let report = || format!("{facts:#?}");
|
||||
|
||||
assert_eq!(
|
||||
facts.get("entered_terminal_mode").map(String::as_str),
|
||||
Some("true"),
|
||||
"precondition: the GPU entered terminal mode from a real frame: {}",
|
||||
report()
|
||||
);
|
||||
// Non-vacuity for the whole test: the child really did run, and the
|
||||
// breadcrumb mechanism really does paint.
|
||||
let screen = facts.get("last_frame_text").cloned().unwrap_or_default();
|
||||
assert!(
|
||||
screen.contains("READY"),
|
||||
"precondition: the child's own output must reach the frame: {}",
|
||||
report()
|
||||
);
|
||||
|
||||
// Acceptance 1 — a quiet child must not produce a frame per tick. The
|
||||
// bound is generous: the session legitimately emits a first frame, plus a
|
||||
// frame for the geometry it settles at, plus the WINCH breadcrumb.
|
||||
let frames: u32 = facts
|
||||
.get("frames")
|
||||
.and_then(|value| value.parse().ok())
|
||||
.unwrap_or_default();
|
||||
assert!(
|
||||
(1..=12).contains(&frames),
|
||||
"a quiet terminal must settle, got {frames} frames in {OBSERVE_MS} ms \
|
||||
(pre-fix: one per dispatcher tick): {}",
|
||||
report()
|
||||
);
|
||||
|
||||
// Acceptance 7 — bounded SIGWINCH, counted by the child itself through
|
||||
// the real PTY. At most one resize is legitimate here (the frontend's
|
||||
// first declaration); the probe requests none in quiet mode.
|
||||
assert!(
|
||||
!screen.contains("WINCH 3"),
|
||||
"the child must not be signalled repeatedly: {}",
|
||||
report()
|
||||
);
|
||||
}
|
||||
|
||||
/// Acceptance 4: a character typed through the real GPU attach client reaches
|
||||
/// the child and its copy comes back in a rendered frame.
|
||||
///
|
||||
/// **This is a keep-working pin, not a fix discriminator** — it passes on the
|
||||
/// pre-fix tree too. Key transport was never the defect (falsified hypothesis
|
||||
/// 2 in the framing), and this exists so that a future change to the routing
|
||||
/// or transport cannot quietly break what the resize fix was not about.
|
||||
#[cfg(feature = "crdt")]
|
||||
#[test]
|
||||
fn gpu_terminal_input_reaches_the_child_and_returns_in_a_frame() {
|
||||
let Some(facts) = run_gpu_probe(CAT_PROBE_INIT_LUA, None) else {
|
||||
return;
|
||||
};
|
||||
let report = || format!("{facts:#?}");
|
||||
|
||||
assert_eq!(
|
||||
facts.get("entered_terminal_mode").map(String::as_str),
|
||||
Some("true"),
|
||||
"precondition: terminal mode: {}",
|
||||
report()
|
||||
);
|
||||
let frames: u32 = facts
|
||||
.get("frames")
|
||||
.and_then(|value| value.parse().ok())
|
||||
.unwrap_or_default();
|
||||
assert!(
|
||||
frames >= 1,
|
||||
"precondition: the child ran and painted: {}",
|
||||
report()
|
||||
);
|
||||
// The probe types `x`; `cat` copies it back exactly once. The observation
|
||||
// is LATCHED across frames rather than read off the last one: the probe
|
||||
// also requests a geometry change, and a reflow rewrites the visible grid.
|
||||
// "did the byte come back" and "is it still on screen at the end" are
|
||||
// different questions, and only the first is about input reaching the
|
||||
// child.
|
||||
assert_eq!(
|
||||
facts.get("input_echo_observed").map(String::as_str),
|
||||
Some("true"),
|
||||
"the typed character must reach the child and return: {}",
|
||||
report()
|
||||
);
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue