25 KiB

Raw Blame History

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:

// 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 fix	2	yes

Bet B2 is scored TRUE: with the fix deployed, the reporter confirmed typing into a GPU terminal works. The earlier caveat here pointed at Q#GT5, which is now retracted — see "Deferred (named)".

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.

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.
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).
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:

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.
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.
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 — SCORED TRUE 2026-07-25. "The user's reported symptom is this defect." Confirmed in real use after the fix was deployed: typing into a GPU terminal works. The confirmation needed a daemon restart built from a tree containing the fix — the first attempt reported no change because a pre-fix daemon still owned the socket, which is worth remembering whenever a daemon-side fix is being validated by hand.
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)~~ — RETRACTED 2026-07-25. The observation behind it (a bash --norc -i fixture not echoing typed characters) does not reproduce in real use: with the fix deployed, typing into a GPU terminal echoes normally. The fixture was almost certainly measuring its own timing — polling a published screen snapshot before readline had finished initialising — not a product behaviour. Recorded as retracted rather than deleted so nobody re-derives it from the framing's earlier revision and spends a scout on it.
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 ifs, 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".

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.
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.
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.
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.
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.
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.
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.
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.

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

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 ifs. 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.

25 KiB Raw Blame History Unescape Escape