feat(tui): add stable iTerm2 Pet rendering - #9
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99cceea4e9
ℹ️ 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: a7c4792b2f
ℹ️ 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".
754006b to
9094929
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 86bf7a5764
ℹ️ 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: 988a52a547
ℹ️ 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: b5c2bbac34
ℹ️ 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".
42c9d9f to
3277a47
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
1 similar comment
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
d5fa880 to
f70ae33
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
f70ae33 to
5363011
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
5363011 to
e3ae830
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 826b2f799c
ℹ️ 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".
4031dba to
c678db0
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c678db0dfe
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d38e9027e3
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e7f04633a
ℹ️ 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".
6e7f046 to
817d259
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 817d2599f4
ℹ️ 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".
817d259 to
b8db948
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b8db948a56
ℹ️ 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".
b8db948 to
fb8ee70
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb8ee70c59
ℹ️ 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".
fb8ee70 to
66db9f7
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66db9f736c
ℹ️ 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.#rasterLeases.size > 0 && | ||
| this.#rasterCleanup.size === 0 && | ||
| !newLines.slice(Math.max(0, newLines.length - height)).some(line => TERMINAL.isImageLine(line)) | ||
| ) { | ||
| viewportRepaint("changed frame with active raster lease"); |
There was a problem hiding this comment.
Preserve native scrollback while a raster lease is active
When the iTerm Pet holds its normal long-lived raster lease, every changed frame is routed through viewportRepaint(), including transcript growth. That repaint uses absolute cursor positioning and emits no scrolling newline, then records the new frame as #previousLines; subsequent appends therefore never admit those rows into native terminal scrollback, and removing the lease later cannot recover them because they are no longer considered changed. Users who leave the Pet enabled can consequently lose the entire session transcript from host scrollback; invalidate/reacquire the lease around scroll-producing appends or otherwise preserve native admission.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4bf34d2. A growing frame that advances the live viewport now releases the raster lease before using the normal scroll-producing renderer; non-scrolling frames continue clipping around the active lease. Added a native-scrollback regression in raster-lease.test.ts. Verified: focused Pet/TUI suite 183 pass / 0 fail / 748 assertions; bun --cwd=packages/tui run check; bun --cwd=packages/coding-agent run check; git diff --check.
There was a problem hiding this comment.
The 4bf34d2 lease-release change caused a live iTerm regression: streaming work output erased and reacquired the GIF on each scroll, producing visible Pet flicker. It has been reverted in fork head 01280a7, restoring continuous lease-backed viewport repaint during streaming. The regression test again asserts no erase/re-upload during overflow. Focused TUI validation: 61 pass / 0 fail / 270 assertions; bun --cwd=packages/tui run check; git diff --check. Keeping this thread open because the original native-scrollback concern needs a design that does not sacrifice live animation continuity.
Add lease-based OSC 1337 GIF rendering for iTerm2 while preserving Sixel, Kitty, tmux, viewport, and ordinary renderer behavior.
66db9f7 to
074002d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 074002d17a
ℹ️ 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".
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
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4bf34d2548
ℹ️ 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".
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
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6567ed305a
ℹ️ 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".
| } catch { | ||
| if (!isCurrent()) return this.availability; | ||
| return this.#finish("topology-ineligible"); |
There was a problem hiding this comment.
Restore pane state when client discovery throws
When the managed tmux runner throws during a later list-clients poll, this catch calls #finish() rather than revoke(), so a pane whose allow-passthrough option was enabled during probing remains modified indefinitely while discovery keeps failing. The earlier nonzero-status branch now restores the option, but this neighboring exception branch is fresh evidence that spawn or command-runner exceptions still bypass restoration.
Useful? React with 👍 / 👎.
| const componentBytes = stats.gifBytes + stats.multipartBytes + stats.tmuxDcsBytes; | ||
| if ( | ||
| stats.size !== MAX_ENTRIES || | ||
| stats.bytes !== componentBytes || |
There was a problem hiding this comment.
Include base64 bytes in the benchmark consistency check
getGajaePetGifCacheStats().bytes includes base64Bytes, but componentBytes sums only GIF, multipart, and tmux-DCS bytes. Since every cached artifact has a nonempty base64 representation, this equality is always false and the newly added benchmark always throws before reporting timing results.
Useful? React with 👍 / 👎.
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
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 424a11fca1
ℹ️ 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".
| export function getPetPixelProtocol(): PetPixelProtocol | null { | ||
| if (TERMINAL.imageProtocol === ImageProtocol.Kitty) return "kitty"; | ||
| if (TERMINAL.imageProtocol === ImageProtocol.Sixel) return "sixel"; | ||
| if (verifiedItermAvailability?.available) return "iterm"; |
There was a problem hiding this comment.
Honor forced graphics-off before enabling iTerm pets
When GJC_FORCE_IMAGE_PROTOCOL=off or PI_FORCE_IMAGE_PROTOCOL=off is set in iTerm, terminal-capabilities.ts intentionally leaves TERMINAL.imageProtocol null and suppresses runtime capability probes, but this fallback returns "iterm" once the independent transport verifies. InteractiveMode then reapplies the saved Pet mode and emits multipart GIF records, defeating the user's explicit graphics opt-out; gate the iTerm transport or this fallback on the forced-protocol setting, including forced off.
Useful? React with 👍 / 👎.
| } | ||
| }, | ||
| }); | ||
| this.#itermPetTransport = createNativePetTransport({ ui: this.ui }); |
There was a problem hiding this comment.
Register Pet teardown with signal cleanup
When a managed iTerm session exits through SIGINT, SIGTERM, or SIGHUP instead of the graceful /quit path, postmortem runs the registered terminal/session cleanup but this newly created transport and widget have no postmortem teardown. Consequently the raster may remain visible and the pane's saved allow-passthrough value may never be restored before process exit; register a bounded, awaited Pet cleanup that runs before terminal restoration.
Useful? React with 👍 / 👎.
iTerm2 Pet rendering with bounded raster ownership
This fork-only PR adds capability-gated iTerm2 Pet rendering while preserving the existing renderer and non-iTerm image protocols.
dev424a11fca1cef1a9f29ff880a970458f16fc1d9bNo upstream branch, PR, or repository metadata has been changed.
User-visible behavior
Raster lifecycle and compatibility
Review hardening
The implementation includes focused fixes for real lifecycle boundaries found during review: multiplexer admission, managed tmux passthrough restoration, non-destructive probe input draining, stale raster ownership, terminal cleanup ordering, geometry freshness, and Kitty image-ID lifetime. These changes stay within the Pet transport/lease boundary and its directly affected TUI tests.
Validation
Automated on this exact head:
Manual iTerm2 validation covered composer placement, idle/working transitions, direct GIF rendering, manual-history suspension/reacquisition, and drag-drop path suppression. The PTY capture artifact used during investigation remains untracked and is intentionally excluded from this PR.
Release notes
packages/coding-agent/CHANGELOG.mdandpackages/tui/CHANGELOG.mdincludeUnreleasedentries for the new iTerm2 Pet surface.