Merge pull request #166 from levineuwirth/gpu-terminal-input
fix(daemon): stop resizing a semantic frontend's PTY twice per tick
This commit is contained in:
commit
b889873b83
25
COHERENCE.md
25
COHERENCE.md
|
|
@ -363,7 +363,7 @@ Full verdict table:
|
||||||
| 5 | Edit | **Works** | Full CUA + Emacs keymap in 161 lines (`builtin/keymaps/default.lua`); isearch, query-replace, kill ring, undo/redo, auto-indent/pair/comment, atomic save. Genuinely excellent zero-config |
|
| 5 | Edit | **Works** | Full CUA + Emacs keymap in 161 lines (`builtin/keymaps/default.lua`); isearch, query-replace, kill ring, undo/redo, auto-indent/pair/comment, atomic save. Genuinely excellent zero-config |
|
||||||
| 6 | Language intelligence | **Partial** | Rust grammar bundled and auto-attaches; rust-analyzer preconfigured (`builtin/runtime/lsp.lua:44-52`) — but a missing binary fails silently (§1.2) and highlighting masks it. No LSP status command exists to diagnose |
|
| 6 | Language intelligence | **Partial** | Rust grammar bundled and auto-attaches; rust-analyzer preconfigured (`builtin/runtime/lsp.lua:44-52`) — but a missing binary fails silently (§1.2) and highlighting masks it. No LSP status command exists to diagnose |
|
||||||
| 7 | Find symbol / file | **File: missing → in flight (PR #162). Symbol: works but undiscoverable** | No find-file/dired/picker existed at audit; `M-.`/`M-?`/`C-c o` bound but advertised nowhere and server-gated; no workspace-symbol command; `pmacs.index.*` has no UI |
|
| 7 | Find symbol / file | **File: missing → in flight (PR #162). Symbol: works but undiscoverable** | No find-file/dired/picker existed at audit; `M-.`/`M-?`/`C-c o` bound but advertised nowhere and server-gated; no workspace-symbol command; `pmacs.index.*` has no UI |
|
||||||
| 8 | Open terminal | **Works but undiscoverable** | Full PTY with scrollback + modeline segment — reachable only as `M-x terminal`, no keybinding |
|
| 8 | Open terminal | **Works but undiscoverable** | Full PTY with scrollback + modeline segment — reachable only as `M-x terminal`, no keybinding. *Was broken outright on the GPU frontend until the double terminal-layout sync was fixed: the child took a `SIGWINCH` storm at tick cadence, so typing into it was impossible while output still flowed.* |
|
||||||
| 9 | Build / test | **Partial** | `M-x compile.run` works, defaults cwd to detected project root, parses Rust `-->` errors — but no keybinding, an **empty first prompt** (`initial = last and last.cmdline or ""`, `builtin/runtime/compile.lua:1134-1138`), and no `cargo build`/`cargo test` suggestion despite `ProjectKind::Cargo` existing (`src/project.rs:77`) |
|
| 9 | Build / test | **Partial** | `M-x compile.run` works, defaults cwd to detected project root, parses Rust `-->` errors — but no keybinding, an **empty first prompt** (`initial = last and last.cmdline or ""`, `builtin/runtime/compile.lua:1134-1138`), and no `cargo build`/`cargo test` suggestion despite `ProjectKind::Cargo` existing (`src/project.rs:77`) |
|
||||||
| 10 | Inspect error | **Partial (good once reached)** | `E:n W:n` modeline counts, underlines, `M-g n/p` + ``C-x ` `` walking a unified compile/grep/diag source, message echo, `RET` visits. Gated entirely on step 6 or 9 succeeding first |
|
| 10 | Inspect error | **Partial (good once reached)** | `E:n W:n` modeline counts, underlines, `M-g n/p` + ``C-x ` `` walking a unified compile/grep/diag source, message echo, `RET` visits. Gated entirely on step 6 or 9 succeeding first |
|
||||||
| 11 | See background work | **Works but undiscoverable** | `*workers*` view via `M-x editor.list-workers`; `C-c C-k` cancel-at-point. No keybinding, no statusline spinner/progress indicator anywhere (§9) |
|
| 11 | See background work | **Works but undiscoverable** | `*workers*` view via `M-x editor.list-workers`; `C-c C-k` cancel-at-point. No keybinding, no statusline spinner/progress indicator anywhere (§9) |
|
||||||
|
|
@ -671,9 +671,18 @@ Facts that define the gap:
|
||||||
optimistic-apply correctness), and `dispatch_paste`
|
optimistic-apply correctness), and `dispatch_paste`
|
||||||
(`editor.rs:1129-1140`).
|
(`editor.rs:1129-1140`).
|
||||||
- Off-path hardcodes: client-side **F12 detach** (`is_detach_key`,
|
- Off-path hardcodes: client-side **F12 detach** (`is_detach_key`,
|
||||||
`src/attach.rs:997-1006`) and the GPU **optimistic key classifier**
|
`src/attach.rs:997-1006`) and the replica frontends' **optimistic key
|
||||||
(`crate::optimistic::classify_key`) — the latter is classification,
|
classifiers** — classification, not routing, and kept honest by
|
||||||
not routing, and is kept honest by `dispatch_idle_for`.
|
`dispatch_idle_for`. There are **two, one per replica frontend**, and the
|
||||||
|
original audit named only one: `crate::optimistic::classify_key` belongs to
|
||||||
|
the **`pmacs --attach` TUI** replica (`src/attach.rs:843` is its only
|
||||||
|
consumer), while `pmacs-gpu` has its own, unrelated
|
||||||
|
`optimistic_insert_text` / `optimistic_crdt_insert`
|
||||||
|
(`pmacs-gpu/src/main.rs:2694`/`3306`). The "kept honest by
|
||||||
|
`dispatch_idle_for`" claim was **verified for both** while investigating the
|
||||||
|
GPU terminal input defect: a focused terminal buffer is in
|
||||||
|
`round_trip_buffers` (`src/terminal/session.rs:338`), so `dispatch_idle_for`
|
||||||
|
reports false and neither classifier can fire there.
|
||||||
|
|
||||||
**The counter-example that proves the idiom:** the entire picker/panel
|
**The counter-example that proves the idiom:** the entire picker/panel
|
||||||
family — listview (references, outline), project-search, buffer-list,
|
family — listview (references, outline), project-search, buffer-list,
|
||||||
|
|
@ -1245,6 +1254,14 @@ its asks are already practiced.**
|
||||||
selected from the negotiated `semantic_render` bit) so a grid
|
selected from the negotiated `semantic_render` bit) so a grid
|
||||||
frontend collapses folds while a simultaneous GPU session does not
|
frontend collapses folds while a simultaneous GPU session does not
|
||||||
skip lines (#149/#148).
|
skip lines (#149/#148).
|
||||||
|
- **But it is enforced by convention, not by structure.** The GPU terminal
|
||||||
|
input defect was a per-frontend-kind operation applied to *both* kinds:
|
||||||
|
the dispatcher's grid and semantic terminal-layout syncs were written as
|
||||||
|
twins and executed as siblings, so a GPU session's PTY was resized twice
|
||||||
|
per tick forever. `sync_terminal_layouts_for_tick` now makes that one
|
||||||
|
exclusive by construction; every other per-frontend-kind pair in the
|
||||||
|
dispatcher remains two adjacent `if`s that a reader must notice are
|
||||||
|
alternatives.
|
||||||
- The GPU frontend exceeds the TUI (minimap, squiggles, typography)
|
- The GPU frontend exceeds the TUI (minimap, squiggles, typography)
|
||||||
without the TUI losing the model — the "no privileged frontend" rule
|
without the TUI losing the model — the "no privileged frontend" rule
|
||||||
is holding under real divergence pressure.
|
is holding under real divergence pressure.
|
||||||
|
|
|
||||||
|
|
@ -227,6 +227,58 @@ If it does not, stop and repair the remote/fetch configuration.
|
||||||
buffer a directory should resolve *to*, and `pmacs .` should route into
|
buffer a directory should resolve *to*, and `pmacs .` should route into
|
||||||
it rather than growing a second directory surface.
|
it rather than growing a second directory surface.
|
||||||
|
|
||||||
|
## GPU terminal input lane — IN REVIEW
|
||||||
|
|
||||||
|
- Portable branch: `githubsucks/gpu-terminal-input`, worktree
|
||||||
|
`../pmacs-gui-term-input`, based on `githubsucks/main` @ `46a1b8f`.
|
||||||
|
- Approved framing: `docs/gpu-terminal-input-framing.md` revision 2,
|
||||||
|
committed as the branch's first commit (`9a0df21`). Bug fix, not a
|
||||||
|
feature; **no protocol change (stays v20)**.
|
||||||
|
- Reported as "text input within the terminal doesn't work on GUI, this is
|
||||||
|
fine in TUI". Root cause: the dispatcher applied **both** terminal-layout
|
||||||
|
syncs to **every** attached frontend, and a semantic session satisfies both
|
||||||
|
conditions (a `term_sizes` entry from `AttachRequest` *and* a terminal
|
||||||
|
declaration). Its PTY was resized twice per tick forever — grid arm installs
|
||||||
|
the TUI placement size, semantic arm installs the declared content
|
||||||
|
rectangle, each arm's idempotence guard seeing only what the other just
|
||||||
|
wrote — so the child took a `SIGWINCH` storm at tick cadence.
|
||||||
|
- **The fix is a split, not a guard.** The grid arm is also the only per-tick
|
||||||
|
controller-liveness release a semantic frontend gets, and
|
||||||
|
`sync_semantic_terminal_layout` cannot take that over: the buffer-follow
|
||||||
|
snapshot clears the viewport declaration (`on_buffer_snapshot_sent`), so
|
||||||
|
that arm stops running in exactly the switch-away case that needs the
|
||||||
|
release. `sync_terminal_layout` is therefore split into a
|
||||||
|
frontend-kind-neutral half (panel reconcile + liveness) and a grid-only
|
||||||
|
geometry half, with the loop body extracted to
|
||||||
|
`sync_terminal_layouts_for_tick` so the exclusivity is structural and tests
|
||||||
|
drive the real thing.
|
||||||
|
- **Trap for anyone touching this again:** the release at the "no
|
||||||
|
`window_placements` entry" arm reads like liveness and is grid geometry. A
|
||||||
|
semantic frontend has no placement entry at all, so moving it into the
|
||||||
|
neutral half releases a GPU controller every tick.
|
||||||
|
- Bite-verified against **two** pre-images, because the naive guard fixes the
|
||||||
|
storm and introduces the leak:
|
||||||
|
|
||||||
|
| pin | `main` | naive guard | the 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 evidence: a quiet child trapping `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.
|
||||||
|
- **Deliberately out of scope, named:** interactive-shell echo on a raw-mode
|
||||||
|
PTY (Q#GT5 — reproduces in-process too, so it is not the GUI/TUI
|
||||||
|
asymmetry), and a geometry change appearing to clear the visible screen
|
||||||
|
(reproduces pre-fix; why acceptance 4 latches its observation across
|
||||||
|
frames).
|
||||||
|
- Verification on this branch: `cargo fmt --check` clean; strict workspace
|
||||||
|
Clippy clean; 1,829 default + 2,006 CRDT library tests; vterm Stage 1/2/3
|
||||||
|
10 / 6 / 9 CRDT; bottom-panel Stage 1 46; M4 121; required GPU 155;
|
||||||
|
**isolated-config workspace sweep 3,177 across 92 suites, zero failures**;
|
||||||
|
`git diff --check` clean. Gates were run against the committed tree.
|
||||||
|
|
||||||
## Bottom-panel lane (window placement + side windows) — Stage 1 IN REVIEW
|
## Bottom-panel lane (window placement + side windows) — Stage 1 IN REVIEW
|
||||||
|
|
||||||
- Portable branch: `githubsucks/bottom-panel`, worktree
|
- Portable branch: `githubsucks/bottom-panel`, worktree
|
||||||
|
|
|
||||||
|
|
@ -704,6 +704,42 @@ final variant — its own round-trip cannot detect a discriminant shift.
|
||||||
|
|
||||||
## 5. Hard-won ops lessons
|
## 5. Hard-won ops lessons
|
||||||
|
|
||||||
|
- **Two operations that must be alternatives are not made alternatives by
|
||||||
|
being adjacent.** The dispatcher applied its grid and semantic
|
||||||
|
terminal-layout syncs to every attached frontend; a semantic session
|
||||||
|
satisfies both conditions, so its PTY was resized twice per tick forever
|
||||||
|
and the child took a `SIGWINCH` storm that made a GPU terminal untypable
|
||||||
|
while output still flowed. Each arm had a correct `old_size == size`
|
||||||
|
idempotence guard — **individually sound, jointly useless**, because each
|
||||||
|
saw only the size the other had just written. Write mutually exclusive
|
||||||
|
per-frontend-kind work as one `if`/`else` keyed on the same fact session
|
||||||
|
establishment uses, and extract the loop body so a test can drive the real
|
||||||
|
thing.
|
||||||
|
- **Bite against every pre-image the fix could plausibly have taken, not just
|
||||||
|
`main`.** For the same defect, the obvious one-line guard (skip the grid arm
|
||||||
|
for semantic frontends) *does* fix the storm — and silently introduces a
|
||||||
|
controller leak, because that arm was also the only per-tick
|
||||||
|
controller-liveness release a semantic frontend got. A single revert would
|
||||||
|
have scored the fix complete. The pin that catches it (`acc 6`) deliberately
|
||||||
|
**passes on `main`** and fails only against the naive guard: today's defect
|
||||||
|
supplies the release by the accident of running an arm it should not.
|
||||||
|
- **A quiet child is an instrument.** A frame storm is invisible against a
|
||||||
|
fixture that legitimately emits hundreds of frames, and an assertion like
|
||||||
|
`frames >= 2` cannot see one. The same applies to geometry: a
|
||||||
|
"did a frame at the new width arrive" readout is satisfied by a geometry
|
||||||
|
*oscillating through* that width. Assert upper bounds over a fixed window
|
||||||
|
against a child that produces nothing, and let the child self-report the
|
||||||
|
signal you care about (a `SIGWINCH` trap printing a **fresh distinct**
|
||||||
|
breadcrumb per signal — repeated identical markers paint nothing, because
|
||||||
|
`cell::diff` skips already-matching cells).
|
||||||
|
- **`TerminalMode::Raw` makes `sh`-based input fixtures useless.** There is no
|
||||||
|
`ICRNL`, so Enter delivers CR and a `read -r` loop waits forever for a LF
|
||||||
|
that never comes — the test then "proves" input never arrived. Use
|
||||||
|
`exec cat`, which copies stdin to stdout byte by byte. It is also the right
|
||||||
|
echo instrument for the opposite reason people assume: termios `ECHO` is
|
||||||
|
*off* in raw mode, so nothing double-echoes and one keystroke yields exactly
|
||||||
|
one cell.
|
||||||
|
|
||||||
- **The checkout may be shared with the user.** Check `git status` for
|
- **The checkout may be shared with the user.** Check `git status` for
|
||||||
foreign uncommitted work before any stash/checkout/branch surgery;
|
foreign uncommitted work before any stash/checkout/branch surgery;
|
||||||
never assume dirty files are yours. (Their uncommitted fix was nearly
|
never assume dirty files are yours. (Their uncommitted fix was nearly
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,438 @@
|
||||||
|
# GPU terminal input — the double terminal-layout sync
|
||||||
|
|
||||||
|
**Revision 2 — approved 2026-07-25. Scouted against canonical `main` @
|
||||||
|
`8c86d34`; implemented on branch `gpu-terminal-input` off `main` @ `46a1b8f`,
|
||||||
|
whose only delta (#161) touches no file on this fix surface. Protocol stays
|
||||||
|
v20.**
|
||||||
|
|
||||||
|
Revision 2 answers Q#GT4 from the code instead of deferring it, which changes
|
||||||
|
the proposed fix from a one-line guard to a **split of `sync_terminal_layout`
|
||||||
|
into a frontend-kind-neutral liveness half and a grid-only geometry half**;
|
||||||
|
rescores B1 as half-false; gives acceptance criteria 2 and 3 a landable
|
||||||
|
observation seam; and corrects four line citations plus the criterion-4
|
||||||
|
rationale. Revision 1's diagnosis is unchanged — the defect, its measurements,
|
||||||
|
and the three falsified hypotheses all stand.
|
||||||
|
|
||||||
|
Reported symptom: *"Text input within the terminal doesn't work on GUI, this
|
||||||
|
is fine in TUI."*
|
||||||
|
|
||||||
|
This is a bug-fix framing, not a feature. It repairs a defect in Vterm Stage 3
|
||||||
|
(#135) that ships on `main` today, and it closes the acceptance hole that let
|
||||||
|
the defect ship: the Stage 3 real-path acceptance drives a terminal session
|
||||||
|
end to end, and *still could not see this*.
|
||||||
|
|
||||||
|
## Summary of the defect
|
||||||
|
|
||||||
|
Every dispatcher tick, the daemon applies **both** terminal-layout syncs to
|
||||||
|
**every** attached frontend:
|
||||||
|
|
||||||
|
```rust
|
||||||
|
// src/daemon.rs:1536-1554 (current main)
|
||||||
|
for frontend_id in &attached_fids {
|
||||||
|
if let Some(size) = term_sizes.get(frontend_id).copied() {
|
||||||
|
editor.sync_terminal_layout(*frontend_id, size); // GRID path
|
||||||
|
}
|
||||||
|
if let Some((buffer_id, size)) = semantic_states
|
||||||
|
.get(frontend_id)
|
||||||
|
.and_then(SemanticRenderState::terminal_viewport)
|
||||||
|
{
|
||||||
|
editor.sync_semantic_terminal_layout(*frontend_id, buffer_id, size); // SEMANTIC path
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
They are written as twins — the comment on the semantic arm even says *"right
|
||||||
|
beside the grid sync"* — but they are applied as **siblings, not
|
||||||
|
alternatives**. A GPU session has an entry in `term_sizes` (its `AttachRequest`
|
||||||
|
carries an initial cell size, and `Resize` events maintain it) *and* a
|
||||||
|
semantic terminal declaration. So both run, every tick.
|
||||||
|
|
||||||
|
The two disagree by construction, and the semantic arm's own doc comment says
|
||||||
|
why:
|
||||||
|
|
||||||
|
> 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**.
|
||||||
|
|
||||||
|
That is exactly what the grid arm then does. Measured, on a real daemon with a
|
||||||
|
real PTY and the real GPU attach client:
|
||||||
|
|
||||||
|
```
|
||||||
|
PROBE sync_semantic old=Some(24x80) declared=25x92
|
||||||
|
PROBE manager.resize BufferId(3) 25x92
|
||||||
|
PROBE manager.resize BufferId(3) 22x80
|
||||||
|
PROBE sync_semantic old=Some(22x80) declared=25x92
|
||||||
|
PROBE manager.resize BufferId(3) 25x92
|
||||||
|
PROBE manager.resize BufferId(3) 22x80
|
||||||
|
...
|
||||||
|
```
|
||||||
|
|
||||||
|
The PTY is resized **twice per dispatcher tick, forever**. Each resize is a
|
||||||
|
`TIOCSWINSZ` + `SIGWINCH` to the child and a screen reflow in
|
||||||
|
`TerminalScreen`, so the child gets a SIGWINCH storm at tick cadence and the
|
||||||
|
screen alternates between two geometries. An interactive line editor
|
||||||
|
(readline, zle, fish's reader) redraws on every SIGWINCH, so what the user
|
||||||
|
types is continuously destroyed before it can settle — while ordinary child
|
||||||
|
*output* keeps flowing, which is why the terminal looks alive.
|
||||||
|
|
||||||
|
Measured user-visible effect, real bash `-i` in the real GPU path, typing one
|
||||||
|
character:
|
||||||
|
|
||||||
|
| | frames for a static screen | typed `Z` ever visible at the prompt |
|
||||||
|
|---|---|---|
|
||||||
|
| `main` today | **730** in a 20 s window | **no** |
|
||||||
|
| with the guard | **2** | (see Q#GT5 — a separate question) |
|
||||||
|
|
||||||
|
The TUI is unaffected: a grid session has no semantic terminal declaration, so
|
||||||
|
only one arm ever runs for it. This is a **frontend-kind** defect, which is
|
||||||
|
why it presents as "GUI broken, TUI fine".
|
||||||
|
|
||||||
|
## Ground truth (measured this session, not inferred)
|
||||||
|
|
||||||
|
Everything below was established against `main` @ `8c86d34` with a real
|
||||||
|
daemon, a real PTY child, and the real `pmacs-gpu` attach client. The probe
|
||||||
|
harness is preserved (see "Verification plan").
|
||||||
|
|
||||||
|
### What is *not* wrong — three hypotheses falsified
|
||||||
|
|
||||||
|
Recording these because each is a plausible-looking cause that a future
|
||||||
|
reader (or a review round) will re-propose.
|
||||||
|
|
||||||
|
1. **The GPU's optimistic-CRDT path is not implicated.** The first hypothesis
|
||||||
|
was that a typed character becomes a `CrdtOp` against the read-only
|
||||||
|
terminal identity buffer and is dropped. It does not. Terminal buffers are
|
||||||
|
already marked round-trip — `core.set_round_trip_input(buffer_id, true)`
|
||||||
|
at `src/terminal/session.rs:338`, beside `set_read_only(true)` — so
|
||||||
|
`dispatch_idle_for` returns **false** while a terminal window is focused,
|
||||||
|
the daemon publishes `DispatchIdle { idle: false }`, and the GPU's
|
||||||
|
`daemon_intercepts_keys()` is true. Measured on the wire:
|
||||||
|
`dispatch_idle_in_terminal=false`, `intercept_in_terminal=true`,
|
||||||
|
`input_route=send_key(intercept)`. The optimistic gate is shut.
|
||||||
|
2. **Key transport is not implicated.** The keystroke reaches the daemon,
|
||||||
|
resolves a terminal view key, encodes, and is written to the PTY without
|
||||||
|
error: `PROBE dispatch_key ... terminal_key=Some(TerminalViewKey { .. })`,
|
||||||
|
`PROBE terminal transport encode=Some([90])`, `PROBE after send status=""`.
|
||||||
|
With a `cat` child the byte comes back on screen through the whole real GPU
|
||||||
|
path (`echoed_typed_char=true`).
|
||||||
|
3. **The `pmacs --attach` TUI replica does *not* share the defect.** It gates
|
||||||
|
its optimistic path on `dispatch_idle` alone (`src/attach.rs:843`), and
|
||||||
|
that signal is already correct for terminals per (1).
|
||||||
|
|
||||||
|
### The mechanism
|
||||||
|
|
||||||
|
- `EditorInstance::sync_terminal_layout` (`src/editor.rs:1195`) is the grid
|
||||||
|
path: it runs the TUI placement helper over the frontend's *frame* size.
|
||||||
|
- `EditorInstance::sync_semantic_terminal_layout` (`src/editor.rs:1331`) is
|
||||||
|
the semantic path: it consumes a declared *content* rectangle directly.
|
||||||
|
- Both resolve the same controller and call `TerminalManager::resize` on the
|
||||||
|
same session. Each has a correct `old_size == size` idempotence guard
|
||||||
|
(`src/editor.rs:1239` grid, `src/editor.rs:1360` semantic) — the guards are
|
||||||
|
individually sound and jointly useless, because each arm sees the size the
|
||||||
|
*other* just installed.
|
||||||
|
- `TerminalViewStore::record_view_size` (`src/terminal/view.rs:276-292`)
|
||||||
|
returns `true` for any valid declaration with no unchanged-size dedupe,
|
||||||
|
which is why the semantic arm re-fires every tick against the grid arm's
|
||||||
|
flip rather than settling.
|
||||||
|
- Loop order is grid first, semantic second, so the screen *ends* each tick at
|
||||||
|
the declared size. That is why rendering looks alive while the child is
|
||||||
|
whipsawed — and why a frame-based assertion is the wrong instrument
|
||||||
|
(acceptance criteria 2 and 3).
|
||||||
|
- `TerminalScreen::changed` bumps `generation` per mutation
|
||||||
|
(`src/terminal/screen.rs:1467`), which is why generation advances by
|
||||||
|
**exactly 2** per tick — one bump per resize.
|
||||||
|
- Frame suppression is full-struct equality
|
||||||
|
(`self.last_terminal_frame.as_ref() == Some(&frame)`,
|
||||||
|
`src/semantic_render.rs:882`). It is behaving correctly: the frames really
|
||||||
|
do differ. The churn is upstream, and fixing the churn fixes the frame
|
||||||
|
storm. **No suppression change is proposed.**
|
||||||
|
|
||||||
|
### Why the Stage 3 acceptance could not catch it
|
||||||
|
|
||||||
|
`a37_real_daemon_real_pty_and_headless_gpu_render_one_terminal_session`
|
||||||
|
(`tests/vterm_stage3_acceptance.rs:637`) is a genuine real-daemon +
|
||||||
|
real-PTY + real-wgpu path, and it still passes on the broken tree. Three
|
||||||
|
reasons, each worth keeping:
|
||||||
|
|
||||||
|
1. Its child is `sh` printing 400 rows on a timer. **A frame storm is
|
||||||
|
invisible against a child that legitimately produces ~400 frames**, and its
|
||||||
|
only frame-count assertion is `frames >= 2`.
|
||||||
|
2. Its input step is `client.send_key(...)` called **directly**
|
||||||
|
(`pmacs-gpu/src/main.rs:784-785`), so it pins transport, not routing — and
|
||||||
|
it asserts nothing about the result of that input reaching the child.
|
||||||
|
3. It resizes **once, deliberately**, and asserts the new width comes back.
|
||||||
|
A geometry that oscillates *through* the asserted width satisfies that
|
||||||
|
assertion. This is the project's own "a geometric readout is not a state
|
||||||
|
predicate" lesson (`docs/active-work.md`, bottom-panel round 2) in a new
|
||||||
|
place: `observed_resized_frame` says "a frame at this width arrived", not
|
||||||
|
"the geometry settled at this width".
|
||||||
|
|
||||||
|
## Decisions
|
||||||
|
|
||||||
|
**Q#GT1 — Where does the fix go?** `sync_terminal_layout` is **split**, and
|
||||||
|
only its geometry half is gated by frontend kind. A bare "skip the grid arm
|
||||||
|
for semantic frontends" guard is wrong — see Q#GT4, which establishes that the
|
||||||
|
grid arm is also the only per-tick controller-liveness release. Not by
|
||||||
|
removing `term_sizes` for semantic sessions: semantic key and mouse dispatch
|
||||||
|
hard-depend on it (`src/daemon.rs:2191-2213`).
|
||||||
|
|
||||||
|
The function has three separable concerns
|
||||||
|
(`src/editor.rs:1195-1260`), and they do not split where the name suggests:
|
||||||
|
|
||||||
|
| lines | concern | frontend kind |
|
||||||
|
|---|---|---|
|
||||||
|
| 1199 | `reconcile_panel_layout` (Q#BP2b per-tick defensive) | **neutral** |
|
||||||
|
| 1200-1221 | controller liveness: released when the frontend has no view, or its active window no longer shows that terminal | **neutral** — reads only `core.views` / `core.windows` / the controller, never `term_size` |
|
||||||
|
| 1222-1259 | TUI placement (`window_placements`) + `resize` | **grid only** |
|
||||||
|
|
||||||
|
So the daemon loop becomes: run the neutral half for every attached frontend
|
||||||
|
every tick, then exactly one geometry arm per frontend kind.
|
||||||
|
`sync_terminal_layout` survives as the composition of both halves, so
|
||||||
|
`editor::run`'s in-process loop and `LOCAL` keep byte-identical behavior. The
|
||||||
|
liveness half must run **once** per frontend per tick — reconciliation is
|
||||||
|
idempotent, so a double call is safe rather than wrong, but the loop should
|
||||||
|
not pay for it.
|
||||||
|
|
||||||
|
**The trap inside the split:** the third release, at `src/editor.rs:1226`
|
||||||
|
(no placement found for the window), looks like liveness and is **not** — it
|
||||||
|
is grid geometry. A semantic frontend has no `window_placements` entry at all,
|
||||||
|
so moving that arm into the neutral half would release a GPU session's
|
||||||
|
controller on every single tick. That would be a new defect of exactly the
|
||||||
|
family this framing fixes, so it stays in the grid half.
|
||||||
|
|
||||||
|
**Q#GT2 — Which arm wins for a semantic frontend?** The semantic one,
|
||||||
|
unconditionally. It is the only arm that consumes a *content* rectangle; the
|
||||||
|
grid arm's modeline subtraction is meaningless for a frontend that draws no
|
||||||
|
modeline into the terminal band. A GPU frontend that has not yet declared a
|
||||||
|
terminal viewport gets **neither** arm, which is correct: the terminal keeps
|
||||||
|
the geometry it was opened with until the frontend declares one.
|
||||||
|
|
||||||
|
**Q#GT3 — Is the guard "no semantic state" or "not a semantic session"?**
|
||||||
|
`semantic_states` keyed by frontend id is the same map the semantic arm reads
|
||||||
|
one line later, so the two arms become provably exclusive by construction
|
||||||
|
rather than by two independent predicates that could drift apart. Rejected
|
||||||
|
alternative: keying on the negotiated `semantic_render` capability bit — it is
|
||||||
|
the *same* fact one indirection away, and the pair could then disagree.
|
||||||
|
|
||||||
|
**Q#GT4 — Does anything else in `sync_terminal_layout` need to keep running
|
||||||
|
for a semantic frontend? Yes: the controller-liveness release, and the
|
||||||
|
semantic arm neither performs it nor can be made to.** Revision 1 left this
|
||||||
|
open; the code answers it.
|
||||||
|
|
||||||
|
`release_controller` is called from exactly five sites, all in
|
||||||
|
`src/editor.rs`: the three grid-arm early returns (1210, 1219, 1226),
|
||||||
|
`dispatch_focus(gained = false)` (1189), and `reconcile_panel_layout`'s
|
||||||
|
unsatisfiable-panel path (848). **`sync_semantic_terminal_layout` releases
|
||||||
|
nothing.** When the window has switched away, `semantic_terminal_key` returns
|
||||||
|
`None` (`src/editor.rs:1278` — `window.buffer_id != buffer_id`) and the arm
|
||||||
|
returns `false` without touching the controller.
|
||||||
|
|
||||||
|
Growing a release inside the semantic arm — revision 1's stated fallback for
|
||||||
|
B1 — **cannot work**, and the reason is worth keeping: when a GPU window
|
||||||
|
switches from the terminal to a document, the buffer-follow snapshot clears
|
||||||
|
the viewport declaration (`on_buffer_snapshot_sent` sets
|
||||||
|
`terminal_viewport = None`, `src/semantic_render.rs:574`), so
|
||||||
|
`terminal_viewport()` returns `None` and the semantic arm **stops running
|
||||||
|
entirely** for that frontend. A release placed inside it would never execute
|
||||||
|
in precisely the scenario that needs it.
|
||||||
|
|
||||||
|
Nor do the other two sites cover it: `dispatch_focus(false)` fires on
|
||||||
|
whole-frontend focus loss, not on a window or buffer switch, and semantic
|
||||||
|
sessions are not panel-capable yet (`panel_capable_for` is false for them —
|
||||||
|
`src/daemon.rs:1893-1898`), so 848 never fires either.
|
||||||
|
|
||||||
|
Consequence of shipping revision 1's guard as written: a GPU frontend that
|
||||||
|
switches away from its terminal **holds the controller indefinitely**. Because
|
||||||
|
another frontend's grid sync early-returns on a
|
||||||
|
`controller_view_for_frontend` mismatch, that peer then cannot resize the PTY
|
||||||
|
until it explicitly re-claims. This is why Q#GT1 splits the function instead
|
||||||
|
of gating it.
|
||||||
|
|
||||||
|
**Q#GT7 — The per-tick defensive panel reconcile stays for semantic
|
||||||
|
frontends.** It is the only per-tick pre-paint reconcile the Q#BP2b contract
|
||||||
|
names (`src/editor.rs:822-830`), and today the grid arm supplies it for GPU
|
||||||
|
sessions too. Putting it in the neutral half of the split preserves that
|
||||||
|
exactly. It is harmless-either-way today — semantic sessions have unknown
|
||||||
|
frame geometry until the bottom-panel GPU band lands — but "harmless today"
|
||||||
|
is not a reason to remove a contract's only per-tick enforcement point in a
|
||||||
|
PR about something else.
|
||||||
|
|
||||||
|
**Q#GT5 — Typed characters not echoing by an interactive shell is a
|
||||||
|
*separate* question and is deliberately out of scope.** Measured: with `bash
|
||||||
|
--norc -i` on a `TerminalMode::Raw` PTY, typed characters are not echoed to
|
||||||
|
the screen — **and this reproduces identically in-process**, i.e. on the TUI's
|
||||||
|
own path, where the user reports the terminal works. Because it is not
|
||||||
|
frontend-specific it cannot be the GUI/TUI asymmetry, and folding it in would
|
||||||
|
make this PR two features. It gets its own scout: whether `TerminalMode::Raw`
|
||||||
|
is the right mode for a `pmacs.terminal.open` child, and what pmacs owes a
|
||||||
|
child that expects to own its termios. Named, not silently dropped.
|
||||||
|
|
||||||
|
**Q#GT6 — Protocol impact: none.** No wire shape, no negotiation, no version
|
||||||
|
change. Stays v20.
|
||||||
|
|
||||||
|
## Bets
|
||||||
|
|
||||||
|
- **B1 — SCORED HALF-FALSE before implementation (revision 2).** "Removing the
|
||||||
|
grid arm for semantic frontends removes the storm without removing any
|
||||||
|
behavior a GPU session relies on." The first clause holds (measured). The
|
||||||
|
second is **false**: it also removes the only controller-liveness release
|
||||||
|
and the only per-tick Q#BP2b reconcile a GPU session gets (Q#GT4, Q#GT7).
|
||||||
|
Its stated contingency — "the semantic arm grows the release" — is false
|
||||||
|
too, for a structural reason (`terminal_viewport` is cleared by the very
|
||||||
|
snapshot that signals the switch-away). Hence the split in Q#GT1. Recorded
|
||||||
|
rather than deleted: the failure mode is one a reviewer or a future
|
||||||
|
simplification will re-propose.
|
||||||
|
- **B2.** The user's reported symptom is this defect. *Partially scored: the
|
||||||
|
storm is proven and GUI-only, and its shape (line editor unusable, output
|
||||||
|
still flowing) matches the report. Not fully scored until the user, or an
|
||||||
|
acceptance running the **user's own shell**, confirms typing works after the
|
||||||
|
fix. Q#GT5 is the reason this bet is stated rather than assumed.*
|
||||||
|
- **B3.** No other pair of per-frontend-kind daemon operations is applied as
|
||||||
|
siblings rather than alternatives. *Scored by an explicit audit of the
|
||||||
|
dispatcher's per-frontend loop during implementation — this defect's shape
|
||||||
|
is "twins applied as siblings", and it would be negligent to fix one
|
||||||
|
instance without looking for others.*
|
||||||
|
|
||||||
|
## 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.
|
||||||
|
- The `a37` probe's structural weaknesses beyond what the acceptance below
|
||||||
|
fixes (it still cannot exercise `App::window_event`'s routing, because that
|
||||||
|
logic is inline in the winit handler with no extractable seam). Making GPU
|
||||||
|
key routing testable is a real refactor and belongs to its own lane.
|
||||||
|
|
||||||
|
## Acceptance criteria
|
||||||
|
|
||||||
|
**The observation seam (revision 2).** Criteria 2, 3 and 6 assert daemon-side
|
||||||
|
state, and `TestDaemon` runs the daemon as a **subprocess**
|
||||||
|
(`tests/common/daemon.rs:90`), so nothing in-process can see it and the
|
||||||
|
scouting instrumentation does not land. The seam that does land: **extract the
|
||||||
|
dispatcher loop's per-frontend terminal-layout step into a named function**
|
||||||
|
that takes `(&mut EditorState, &[FrontendId], &term_sizes, &semantic_states)`.
|
||||||
|
That is required by Q#GT1's split anyway, it makes the grid/semantic
|
||||||
|
exclusivity structural rather than two adjacent `if`s, and it lets an
|
||||||
|
in-process test in the style of the existing `src/daemon.rs` unit tests
|
||||||
|
(3375ff) drive **the real loop body** rather than a re-implementation — which
|
||||||
|
is the a37 lesson applied to this PR's own tests.
|
||||||
|
|
||||||
|
The observable is `TerminalScreen::generation`, reachable through
|
||||||
|
`TerminalManager::snapshot(..).generation`. It advances once per screen
|
||||||
|
mutation (`src/terminal/screen.rs:1467`), so with a quiet child it is a
|
||||||
|
**state predicate**, not a readout: "the geometry settled" is exactly
|
||||||
|
"generation stopped advancing".
|
||||||
|
|
||||||
|
1. On a real daemon + real PTY + real GPU attach, a terminal session that
|
||||||
|
receives no child output produces a **bounded** number of terminal frames
|
||||||
|
(settling to zero new frames once the screen is static) — not one per tick.
|
||||||
|
Fails on `main` with ~730 frames in 20 s; passes with ≤ a small constant.
|
||||||
|
2. Driving the extracted loop body N times against a semantic frontend with a
|
||||||
|
fixed declaration and a quiet child: `TerminalManager::resize` takes effect
|
||||||
|
**exactly once** (generation advances once, then is constant for the
|
||||||
|
remaining N-1 iterations). Fails on `main`, where generation advances by
|
||||||
|
two per iteration.
|
||||||
|
3. After a declaration, `screen_size(buffer)` **equals the declared content
|
||||||
|
rectangle and stays equal** across subsequent iterations — the state
|
||||||
|
predicate, not the "a frame at this width arrived" readout that
|
||||||
|
`observed_resized_frame` provides today.
|
||||||
|
4. A character sent through the real GPU attach client reaches the child and
|
||||||
|
its echo appears in a rendered frame. **This is a keep-working pin, not a
|
||||||
|
fix discriminator: it already passes on today's broken `main`** (falsified
|
||||||
|
hypothesis 2 measured `echoed_typed_char=true`). Pinned with a `cat` child
|
||||||
|
— not because `cat` echoes (termios `ECHO` is off in raw mode; nothing
|
||||||
|
echoes) but because `cat` *copies stdin to stdout*, so the byte comes back
|
||||||
|
exactly once, with no line discipline and no double echo to disambiguate.
|
||||||
|
5. A grid (TUI) session's terminal resize behavior is **unchanged** — pinned
|
||||||
|
against the existing Stage 2 real-TUI PTY smoke, which must stay green
|
||||||
|
without modification.
|
||||||
|
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 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 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.
|
||||||
|
|
||||||
|
## Coherence impact (`COHERENCE.md` §20)
|
||||||
|
|
||||||
|
- **§2 golden journey, step 8 ("Open a terminal")** — currently graded *"Works
|
||||||
|
but undiscoverable"*. On the GPU frontend it does not work; this restores
|
||||||
|
the step for the frontend the document calls the more capable one. Priority
|
||||||
|
1 explicitly treats journey regressions as release blockers.
|
||||||
|
- **§16 Productize the Semantic Frontend Architecture** — graded *strong*,
|
||||||
|
with "graceful per-frontend degradation is practiced, not aspirational" as
|
||||||
|
its evidence, citing per-frontend fold projection. This defect is the
|
||||||
|
counter-example: a per-frontend-kind operation applied to both kinds at
|
||||||
|
once. The section's claim survives, but the audit should record that the
|
||||||
|
practice is enforced by convention, not by structure — two arms that must be
|
||||||
|
alternatives are currently just two adjacent `if`s. Q#GT1's extracted loop
|
||||||
|
body makes this one structural; the audit note should say the *pattern* is
|
||||||
|
still convention-enforced everywhere else (B3).
|
||||||
|
- **§6 Eliminate Hardcoded Interaction Islands** — the audit's row 6 note that
|
||||||
|
the GPU optimistic classifier "is kept honest by `dispatch_idle_for`" is
|
||||||
|
**confirmed correct** by this investigation (falsified hypothesis 1), and the
|
||||||
|
§6 citation `crate::optimistic::classify_key` should be corrected: that
|
||||||
|
symbol is `src/optimistic.rs`, the **`pmacs --attach` TUI replica's**
|
||||||
|
classifier. The GPU's separate, unrelated classifier is
|
||||||
|
`optimistic_insert_text` / `optimistic_crdt_insert` in
|
||||||
|
`pmacs-gpu/src/main.rs`. Two replica frontends, two classifiers; the audit
|
||||||
|
conflates them.
|
||||||
|
- **§19 Product Coherence Acceptance Tests** — this is a concrete instance of
|
||||||
|
the section's thesis. Every subsystem test passed; the defect lives in how
|
||||||
|
two correct subsystems compose per frontend kind. Criterion 1's quiet-child
|
||||||
|
shape is the transferable technique.
|
||||||
|
- No interaction island added, no config registry surface, no background-work
|
||||||
|
attribution change.
|
||||||
|
|
||||||
|
## Verification plan
|
||||||
|
|
||||||
|
Full gate suite per `CLAUDE.md`, plus:
|
||||||
|
|
||||||
|
- `cargo test --features crdt --test vterm_stage3_acceptance` (the suite this
|
||||||
|
repairs) and `--test vterm_stage2_acceptance` (the TUI no-regression pin).
|
||||||
|
- `PMACS_REQUIRE_GPU=1 cargo test -p pmacs-gpu`.
|
||||||
|
- The scouting harness is preserved and should be re-run against the branch:
|
||||||
|
a quiet-child variant of the `a37` probe plus daemon-side resize tracing,
|
||||||
|
saved as `scratch_gui_terminal_input.rs`, `scratch_inproc_input.rs`, and
|
||||||
|
`gui-terminal-probe-instrumentation.patch`. The instrumentation is scratch;
|
||||||
|
the acceptance criteria above are what lands.
|
||||||
|
- Manual confirmation with the user's own shell (fish) in a real GPU window,
|
||||||
|
since B2 is not fully scored by any automated test (Q#GT5).
|
||||||
|
|
||||||
|
**Ops.** This doc is currently untracked in a detached-HEAD worktree
|
||||||
|
(`../pmacs-gui-term-input`), so it does not travel. On approval it becomes the
|
||||||
|
branch's first commit before any implementation, per the standing workflow —
|
||||||
|
no cross-machine expectation should attach to it until then.
|
||||||
|
|
@ -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 _ = 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_input = false;
|
||||||
let mut sent_resize = false;
|
let mut sent_resize = false;
|
||||||
while std::time::Instant::now() < deadline {
|
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) {
|
if pixels.iter().any(|&b| b != first) {
|
||||||
facts.rendered_nonuniform_frames += 1;
|
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;
|
sent_input = true;
|
||||||
// Real child input over the real wire.
|
// 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);
|
let _ = client.send_key(ProtocolKey::Enter, Modifiers::NONE);
|
||||||
}
|
}
|
||||||
if !sent_resize && facts.frames >= 2 {
|
if !quiet && !sent_resize && facts.frames >= 2 {
|
||||||
sent_resize = true;
|
sent_resize = true;
|
||||||
state.resize(700, 500);
|
state.resize(700, 500);
|
||||||
if let Some((buffer_id, size)) = state.terminal_declaration_if_changed()
|
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;
|
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;
|
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, "resized_cols={}", facts.resized_cols);
|
||||||
let _ = writeln!(out, "last_title={}", facts.last_title.unwrap_or_default());
|
let _ = writeln!(out, "last_title={}", facts.last_title.unwrap_or_default());
|
||||||
let _ = writeln!(out, "last_frame_text={}", facts.last_frame_text);
|
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());
|
let _ = writeln!(out, "disconnect={}", facts.disconnect.unwrap_or_default());
|
||||||
if let Err(error) = std::fs::write(report, out) {
|
if let Err(error) = std::fs::write(report, out) {
|
||||||
eprintln!(
|
eprintln!(
|
||||||
|
|
@ -1037,9 +1057,20 @@ struct ProbeFacts {
|
||||||
resized_cols: u32,
|
resized_cols: u32,
|
||||||
last_title: Option<String>,
|
last_title: Option<String>,
|
||||||
last_frame_text: 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>,
|
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.
|
/// One-line printable text of a terminal frame, for probe reporting.
|
||||||
fn frame_probe_text(frame: &TerminalFrame) -> String {
|
fn frame_probe_text(frame: &TerminalFrame) -> String {
|
||||||
let mut text = String::new();
|
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,
|
// Accepted terminal context controls PTY size. Apply any focus,
|
||||||
// window, or resize changes before consuming another child-output
|
// window, or resize changes before consuming another child-output
|
||||||
// batch so screen reflow and subsequent bytes share one geometry.
|
// batch so screen reflow and subsequent bytes share one geometry.
|
||||||
for frontend_id in &attached_fids {
|
sync_terminal_layouts_for_tick(editor, &attached_fids, &term_sizes, &semantic_states);
|
||||||
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);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// `tick_async` last: the M4.5 async bridge settles awaiters
|
// `tick_async` last: the M4.5 async bridge settles awaiters
|
||||||
// inside `tick_lsp` (via the message bus); draining + resuming
|
// 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
|
/// Dispatch a semantic (grid-less) frontend's input event into the
|
||||||
/// shared editor core (Phase B, session B1). Mirrors the `Key` / `Mouse`
|
/// shared editor core (Phase B, session B1). Mirrors the `Key` / `Mouse`
|
||||||
/// arms of [`apply_event`] but takes no `RenderState` — a semantic
|
/// 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]
|
#[test]
|
||||||
fn frontend_events_from_uninstalled_sessions_are_dropped_without_state_access() {
|
fn frontend_events_from_uninstalled_sessions_are_dropped_without_state_access() {
|
||||||
let source = FrontendId(77);
|
let source = FrontendId(77);
|
||||||
|
|
|
||||||
|
|
@ -1189,14 +1189,84 @@ impl EditorState {
|
||||||
let _ = self.terminal_manager.borrow_mut().release_controller(key);
|
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`.
|
/// Resize the one session durably controlled by `frontend_id`.
|
||||||
///
|
///
|
||||||
/// This is called before process drain and paint, never from rendering.
|
/// 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 {
|
pub fn sync_terminal_layout(&mut self, frontend_id: FrontendId, term_size: CellSize) -> bool {
|
||||||
// Bottom-panel arc (Q#BP2b): a panel that just became
|
self.sync_terminal_controller_liveness(frontend_id)
|
||||||
// unsatisfiable must have released its controller before this
|
&& self.sync_terminal_grid_geometry(frontend_id, term_size)
|
||||||
// runs, or the child would be resized against a dead rect.
|
}
|
||||||
self.reconcile_panel_layout(frontend_id);
|
|
||||||
|
/// 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
|
let Some(key) = self
|
||||||
.terminal_manager
|
.terminal_manager
|
||||||
.borrow()
|
.borrow()
|
||||||
|
|
@ -1206,23 +1276,11 @@ impl EditorState {
|
||||||
};
|
};
|
||||||
let content = {
|
let content = {
|
||||||
let core = self.core.borrow();
|
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)
|
let Some(placement) = window_placements(&core, frontend_id, term_size)
|
||||||
.get(&key.window_id)
|
.get(&key.window_id)
|
||||||
.copied()
|
.copied()
|
||||||
else {
|
else {
|
||||||
|
drop(core);
|
||||||
let _ = self.terminal_manager.borrow_mut().release_controller(key);
|
let _ = self.terminal_manager.borrow_mut().release_controller(key);
|
||||||
return false;
|
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())
|
.terminate(terminal_buffer, &mut state.process_supervisor.borrow_mut())
|
||||||
.expect("terminate child");
|
.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