feat(tui): add fixed-suffix scroll region primitive - #17
Conversation
2a5fa24 to
e38fa80
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2a5fa24e1b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e49e744c15
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
e49e744 to
ba2c4ad
Compare
💡 Codex Reviewgajae-code/packages/tui/src/terminal.ts Lines 780 to 781 in ba2c4ad  Preserve keystrokes while draining probe input
On iTerm 3.5+, gajae-code/packages/tui/src/tui.ts Lines 2043 to 2045 in ba2c4ad Drain all queued writes released by a cleanup
When multiple failed ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 30c164d5ad
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| this.terminal.start( | ||
| data => this.#handleInput(data), | ||
| () => { | ||
| this.#resetFixedSuffixScrollRegions(); |
There was a problem hiding this comment.
On hosts that emit resize callbacks without changing the grid—explicitly including iTerm2 tab switches and window-focus events in requestResizeRender()—this unconditional reset revokes the persistent armed owner before the existing width/height checks can classify the event as spurious. Since the owner receives no invalidation notification, subsequent transcript appends silently stop using the fixed-suffix transaction until it happens to reacquire and re-arm; only reset the owner after confirming that terminal geometry actually changed.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d207880fb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| fixedSuffixBuffer += `\x1b8\x1b[r\x1b[?6l${seq}\x1b[?2026l`; | ||
| this.#fixedSuffixScrollRegionResetPending = true; | ||
| if ( | ||
| !this.#writeRenderBufferAndReanchorImeCursor(fixedSuffixBuffer, cursorPos, newLines.length, () => { |
There was a problem hiding this comment.
When a raster operation or lease is active, #writeRenderBufferAndReanchorImeCursor queues this transaction and returns before it reaches the terminal. If the TUI is stopped, disposed, resized, or the token is released while an earlier asynchronous raster operation is pending, the queued closure never rechecks the token or lifecycle state and can subsequently write the old DECSTBM transaction—after stop() in the worst case, or using stale geometry after a resize—corrupting the user's terminal or native scrollback. Revalidate the token, stopped state, and captured geometry at the actual write boundary.
Useful? React with 👍 / 👎.
Add lease-based OSC 1337 GIF rendering for iTerm2 while preserving Sixel, Kitty, tmux, viewport, and ordinary renderer behavior.
A protected iTerm raster lease must yield before a transcript append scrolls. Queued render writes now settle their generation at the terminal write boundary. Tested: TUI raster lease, render commit, render regressions, overlay scroll, detach, Pet, and iTerm protocol suites Confidence: high Scope-risk: narrow
A synchronous terminal stop could overtake queued lease cleanup. Flush active lease erases before terminal teardown. Tested: TUI raster lease and render commit suites Confidence: high Scope-risk: narrow
Releasing the lease on every scroll-producing append re-uploaded the Pet and caused visible animation flicker during active work. Keep the lease while the live viewport is repainted. Tested: raster lease, render commit, Pet, and iTerm protocol TUI suites Confidence: high Scope-risk: narrow
The Pet QA fixture test used synchronous named Node filesystem APIs. Use Bun content I/O and promised namespace directory APIs instead. Tested: qa-iterm-pet test and coding-agent check Confidence: high Scope-risk: narrow
An isolated Escape is user input, not a capability fragment. Keep multi-character OSC fragments buffered while forwarding standalone Escape. Tested: iTerm Pet transport test and coding-agent check Confidence: high Scope-risk: narrow
The Pet PR does not need unrelated QA report path or timestamp changes. Restore the fork base versions to keep review scope bounded. Confidence: high Scope-risk: narrow
The raster barrier regression now proves the generation remains pending and the updated frame is written only after the barrier releases. Tested: render-commit test and TUI check Confidence: high Scope-risk: narrow
Provide an opt-in, transport-neutral DECSTBM transaction for strict transcript appends with a bottom-pinned suffix.\n\nA current owner remains armed across eligible streaming appends; every transaction resets margins before returning control to the ordinary renderer.\n\nLore-id: 8da7f412\nConstraint: no alternate buffer or DECSCA\nConstraint: keep terminal state transaction-scoped\nTested: fixed-suffix and full TUI suite on primitive branch\nScope-risk: narrow\nReversibility: straightforward
Stopped TUI instances cannot retain suffix owners, cursor restoration follows native transcript scrolls, and the public primitive is released in the changelog. Lore-id: decstbm-fix-01 Constraint: preserve existing renderer when no owner is armed Tested: fixed-suffix scroll-region regression and TUI check Scope-risk: narrow
DECRC can restore origin mode after the scroll-region transaction. Reset margins and DECOM before cursor reanchoring so the primitive leaves no terminal mode state behind.\n\nConstraint: keep fixed-suffix transaction transport-neutral\nTested: fixed-suffix scroll-region tests; TUI package check\nScope-risk: narrow\nReversibility: straightforward
6d20788 to
d3201e2
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d3201e2bed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (abortSuffix || cursorVisibility) | ||
| this.#guardTerminalOperation(() => this.terminal.write(abortSuffix + cursorVisibility)); |
There was a problem hiding this comment.
When an emergency stop() occurs while a managed iTerm multipart operation is awaiting flush() or afterPrefix(), the operation later reaches abortBarrier() and writes its cursor/mode restoration sequence even though the TUI and terminal have already stopped. ProcessTerminal.available remains true after a normal stop, so #guardTerminalOperation does not reject this write, allowing control bytes to leak into and alter the resumed parent shell; recheck the stopped/lifecycle generation before this abort write or settle the raster queue before terminal teardown.
Useful? React with 👍 / 👎.
| t.clients === 1 && | ||
| ((t.paneId !== undefined && t.paneId !== this.#paneId) || | ||
| (t.ownedPaneId !== undefined && t.ownedPaneId !== this.#paneId) || | ||
| (t.clientId !== undefined && t.clientId !== (this.#expectedClientId ?? this.#observedClientId))) |
There was a problem hiding this comment.
When the optional topology provider reports its initial single client with clientId, but no expectedClientId was supplied, both #expectedClientId and #observedClientId are undefined, so this comparison rejects the valid client as topology-ineligible. The assignment immediately below that is intended to learn the initial client identity but is unreachable in this case, preventing the managed transport from ever probing; only compare against an identity that has already been established.
Useful? React with 👍 / 👎.
| this.#petUnavailableWarningDisposer = warnWhenPetCapabilitySettled({ | ||
| probePending: this.#itermPetTransport !== undefined || isPetCapabilityProbePending(), |
There was a problem hiding this comment.
For a saved pet on iTerm, this starts the 1,000 ms unavailable-warning deadline at the same time as a probe that may first drain input for up to 100 ms and then legitimately wait another 1,000 ms for its capability reply; managed mode also performs tmux preparation before sending the query. A valid response arriving late in that documented probe window therefore produces an incompatible-terminal warning before enabling the pet. Base the warning on definitive transport completion or allow at least the complete preparation, drain, and query interval.
Useful? React with 👍 / 👎.
1 participant
Scoped DECSTBM fixed-suffix primitive
Fork-only standalone TUI primitive, rebased on
devrelease0.12.8(be3940aa).Verification
bun --cwd=packages/tui run checkgit diff --check