Skip to content

feat(terminal): carry the bound pty key on canvas.TerminalState - #221

Open
sonhyrd wants to merge 1 commit into
vercel-labs:mainfrom
sonhyrd:terminal-state-pty-key
Open

feat(terminal): carry the bound pty key on canvas.TerminalState#221
sonhyrd wants to merge 1 commit into
vercel-labs:mainfrom
sonhyrd:terminal-state-pty-key

Conversation

@sonhyrd

@sonhyrd sonhyrd commented Jul 27, 2026

Copy link
Copy Markdown

Summary

Adds pty: u64 to canvas.TerminalState, stamped with the bound key, so an on-terminal dispatch says which terminal it is about.

The problem

With more than one <terminal> mounted at once, every pane dispatches the same Msg tag carrying the same payload type — and the payload has no identity:

pub const TerminalState = struct {
    scrollback: u32 = 0,
    history: u32 = 0,
    cols: u16 = 0,
    rows: u16 = 0,
};

Two panes with the same geometry report byte-identical states.

There was no way for the app to add one, either. on-terminal takes a bare tag, and an authored payload is a hard error — on_terminal_payload_message (src/primitives/canvas/ui_markup.zig:1282), refused at ui_markup_view.zig:1935 and ui_markup_compiled.zig:1992, with a test pinning it (on-terminal requires a bare tag: an authored payload is refused). So on-terminal="term_state:{pty_key}" — the shape every other handler accepts — is exactly what the schema rejects.

The mounting shape this breaks on is the ordinary one:

<for each="panes" key="pane_id" as="p">
  <terminal pty="{p.pty_key}" grow="1" label="{'Shell ' ++ p.number}" />
</for>

What it costs

A per-pane scrollback: u32 in the model is unwritable: update can never correctly attribute an echo to a pane, and a field it can never write is a lie in the model. The common case survives without it (each pane is its own widget and the runtime retains its offset across rebuilds), but anything where the app must react to a specific pane does not: persisting per-pane scroll position, "scrolled up" chrome on the right pane's header, a jump-to-bottom button.

Where the key comes from

It was already in hand at the dispatch sites — WidgetLayoutNode carries terminal.pty (ui.zig:756), and the wheel path reads it three times immediately before calling msgForTerminal (ui_app.zig:5547-5556).

This stamps it one level lower instead, at the store's public seam: EnabledStore.reconcile and currentState both take the key as a parameter, and every path that reaches an app comes through one of them. A Session is found by key and carries none, so nothing new is stored — the stamp is two lines.

Cost and compatibility

  • 8 bytes on a struct the app copies once per event.
  • terminalMsg (ui.zig:1125) needs no change: the tag's payload just grows a field.
  • For transpiled cores, pty joins the declared-record vocabulary as an optional field: a core declaring {scrollback, history, cols, rows} still matches, and one that wants the key declares five. That needed a required-plus-optional variant of the structural matcher, since the existing one pins an exact field count.

Passing the key as a second constructor argument instead would work equally well; putting it in the struct is the smaller diff.

Verification

  • zig build test: no new failures (this checkout has 6 pre-existing failed command lanes locally — codesign identity, packaging fixtures — identical before and after).
  • The markup test lane run directly: the terminal element binds its pty key, scrollback echo, and view-state handler passes with the new assertion.
  • The enabled-path test skips in this repo's own suiteterminalVtModule always resolves the stub here, and build.zig says the terminal examples own that coverage. It was run in an app that pins ghostty with terminal_sessions = true: that app's session-store lane went 12 → 13 cases, all passing.
  • zig fmt --check clean.

Tests

  • every reported state names its pty, so N mounted terminals stay distinguishable (src/runtime/terminal_session_tests.zig) — two panes reconciled with the same geometry report distinct keys and identical everything else; the wheel path's currentState carries the key of the pane the pointer was over, and the untouched pane still reports its own key and unmoved position.
  • The existing markup test now asserts the bound key arrives in the constructed Msg.

@vercel

vercel Bot commented Jul 27, 2026

Copy link
Copy Markdown

@sondh0127 is attempting to deploy a commit to the Vercel Labs Team on Vercel.

A member of the Team first needs to authorize it.

With more than one `<terminal>` mounted at once, an `on-terminal`
dispatch could not say which one it was about. The payload is
`canvas.TerminalState` — scrollback, history, cols, rows — and none of
those is an identity; two panes with the same geometry report
byte-identical states.

There was no way for the app to add one, either: `on-terminal` takes a
bare Msg tag and an authored payload is refused
(`on_terminal_payload_message`, with a test pinning it), so
`on-terminal="term_state:{pty_key}"` — the shape every other handler
accepts — is exactly what the schema rejects.

The key was already in hand at the dispatch sites. `WidgetLayoutNode`
carries `terminal.pty`, and the wheel path reads it three times
immediately before calling `msgForTerminal`. This stamps it onto the
state at the store's public seam instead — `EnabledStore.reconcile` and
`currentState` both take the key as a parameter, and every path that
reaches an app comes through one of them, so a `Session` (found BY key,
carrying none) needs no new field.

Cost is 8 bytes on a struct the app copies once per event.
`terminalMsg` needs no change: the tag's payload just grows a field. For
transpiled cores, `pty` joins the declared-record vocabulary as an
OPTIONAL field — a core written before this still matches on the four,
one that wants the key declares five.

The enabled-path test skips in this repo's own suite, where
`terminal_vt` is always the stub; it runs in an app that pins ghostty.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants