feat(iterm): preserve Pet through native scrollback - #18
Conversation
e49e744 to
ba2c4ad
Compare
💡 Codex Reviewgajae-code/packages/tui/src/tui.ts Line 4967 in 8e60887  Avoid clearing the leased cells during suffix repaint
On every eligible fixed-suffix append with an iTerm raster lease, this loop emits  Preserve a one-byte capability marker prefix
When terminal input splits the capability OSC immediately after its initial AGENTS.md reference: AGENTS.md:L146-L150 gajae-code/packages/coding-agent/src/modes/interactive-mode.ts Lines 886 to 887 in 8e60887 Wait for the iTerm probe before showing unavailability
When an iTerm transport exists, this reuses the 1,000 ms Sixel settle timer even though the iTerm probe may legally spend up to 100 ms draining input and then another 1,000 ms awaiting its reply. A valid reply arriving near the iTerm deadline therefore appears after the warning has already declared the terminal incompatible, leaving a false warning even as the Pet activates. Tie warning settlement to transport completion or allow the drain plus query timeout. AGENTS.md reference: AGENTS.md:L146-L150 ℹ️ 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". |
8e60887 to
49a432f
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
gajae-code/packages/tui/src/tui.ts
Line 4967 in 49a432f
Avoid repainting through the leased raster rectangle
On every eligible transcript append, this clears and rewrites the entire bottom-pinned suffix, including the composer columns occupied by the iTerm Pet lease. The existing lease-preserving render paths intentionally emit only #unleasedRowSegments() because ordinary cell erasure or text can erase an inline image; here 2K plus the full-width padded suffix touches those protected cells while retaining the lease, so the Pet can disappear without invalidation or re-upload. Clip each suffix row around the active lease rectangle, or invalidate and redraw the lease instead.
ℹ️ 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".
| * it is the sole active lease. | ||
| */ | ||
| armFixedSuffixScrollRegion(token: FixedSuffixScrollRegionToken): number | undefined { | ||
| armFixedSuffixScrollRegion(token: FixedSuffixScrollRegionToken, rasterLease?: RasterLeaseToken): number | undefined { |
There was a problem hiding this comment.
This adds a public TUI API extension and new coding-agent Pet behavior, but the commit updates neither packages/tui/CHANGELOG.md nor packages/coding-agent/CHANGELOG.md. Add entries under each package's ## [Unreleased] section so the release notes include this user-visible feature.
AGENTS.md reference: AGENTS.md:L178-L178
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 9703258. Added Unreleased entries for the TUI raster-bound suffix API and the coding-agent iTerm Pet behavior.
49a432f to
1dcbab7
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 970325886a
ℹ️ 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".
| const suffixRow = regionBottom + suffixIndex + 1; | ||
| const suffixLine = newLines[nextTranscriptLineCount + suffixIndex] ?? ""; | ||
| fixedSuffixBuffer += `\x1b[${suffixRow};1H\x1b[2K${this.#padLineToWidth(suffixLine, width)}`; | ||
| fixedSuffixBuffer += `\x1b[${suffixRow};1H${suffixLinePrefix}${this.#padLineToWidth(suffixLine, width)}`; |
There was a problem hiding this comment.
When the bound iTerm lease overlaps a bottom-pinned row, as the Pet normally does beside the composer, this writes #padLineToWidth(...) from column 1 through the entire row. Omitting EL is insufficient because ordinary text and padding spaces written into leased cells erase or overpaint the inline image; the existing raster-preserving render paths avoid this by using #unleasedRowSegments. Consequently, each fixed-suffix append can corrupt or blank the Pet despite retaining its lease, so these suffix rows need to repaint only the unleased segments.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e8a95f646
ℹ️ 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.#releaseFixedSuffixScrollRegion(); | ||
| const fixedSuffixScrollRegionToken = this.#ui.acquireFixedSuffixScrollRegion(this.#itermOwner); | ||
| if (!fixedSuffixScrollRegionToken || !current() || this.#itermLease !== token) { | ||
| if (fixedSuffixScrollRegionToken) this.#ui.releaseFixedSuffixScrollRegion(fixedSuffixScrollRegionToken); | ||
| return; | ||
| } | ||
| this.#fixedSuffixScrollRegionToken = fixedSuffixScrollRegionToken; | ||
| if (this.#ui.armFixedSuffixScrollRegion(fixedSuffixScrollRegionToken, token) === undefined) |
There was a problem hiding this comment.
When the transcript identity is replaced while the Pet's semantic state remains unchanged, TUI.resetViewportAnchorIntent() clears the armed fixed-suffix token but leaves the raster lease active. The next #tickIterm returns early because the semantic and lease still match, so this acquisition block is never reached again; subsequent transcript appends therefore lose native-scrollback admission until an unrelated working, flexing, mode, or geometry transition triggers another GIF submission. Re-arm independently of GIF submission, or invalidate the widget's cached token when the TUI resets it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 73e36d9904
ℹ️ 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 === 1 && | ||
| fixedSuffixScrollRegionToken !== undefined && | ||
| fixedSuffixScrollRegionRasterLease !== undefined && | ||
| this.#rasterLeases.get(fixedSuffixScrollRegionToken.ownerId)?.token === | ||
| fixedSuffixScrollRegionRasterLease)); |
There was a problem hiding this comment.
When the bound raster crosses into rows 0..regionBottom-1, the IND operations scroll part of the image even though this condition treats the lease as preserved and bypasses invalidation. The new preserves a bound raster lease test exercises this exact geometry (row: 2 on a five-row terminal whose two-line suffix makes regionBottom 3), but the virtual terminal cannot reveal raster movement. Require the lease rectangle to lie entirely below regionBottom, or invalidate it before taking this path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aa35223827
ℹ️ 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".
| const fixedSuffixNativeAppendPreservesRasterLease = | ||
| fixedSuffixNativeAppend && | ||
| this.#rasterCleanup.size === 0 && | ||
| (this.#rasterLeases.size === 0 || | ||
| (this.#rasterLeases.size === 1 && | ||
| fixedSuffixScrollRegionToken !== undefined && | ||
| fixedSuffixScrollRegionRasterLease !== undefined && | ||
| this.#rasterLeases.get(fixedSuffixScrollRegionToken.ownerId)?.token === | ||
| fixedSuffixScrollRegionRasterLease)); |
There was a problem hiding this comment.
When raster ingress is already pending, this sole/current-lease check is only a snapshot: the fixed-suffix buffer is queued at lines 5002–5020, and another disjoint lease acquisition can complete before that buffer reaches the terminal. The preserving ingress does not revalidate the bound token or lease count, while the buffer's unleased segments were computed before the new lease existed, so it can repaint over a newly acquired suffix raster or scroll one in the DECSTBM region. Recheck the same sole-bound-lease invariant at the write boundary and fall back to protected ingress if it changed.
Useful? React with 👍 / 👎.
aa35223 to
3e6cfbb
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3e6cfbbbdf
ℹ️ 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".
| const regionBottom = | ||
| fixedSuffixNativeAppendPreservesRasterLease && fixedSuffixRasterLease !== undefined | ||
| ? Math.min(suffixRegionBottom, fixedSuffixRasterLease.token.rect.row) | ||
| : suffixRegionBottom; |
There was a problem hiding this comment.
When the bound lease starts above the fixed suffix, reducing regionBottom excludes transcript rows as well as raster cells, but the append loop writes only the newly appended lines at the reduced margin and never shifts or repaints those excluded transcript rows. With the added five-row test geometry, appending line-4 to line-1..3/status/composer produces line-2,line-4,line-3,status,composer instead of line-2,line-3,line-4,status,composer. In the current revision, this new Math.min(..., rect.row) avoids scrolling the lease but provides fresh evidence of transcript corruption; invalidate overlapping leases or explicitly repaint every excluded transcript row around the raster.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
gajae-code/packages/tui/src/tui.ts
Line 5048 in ae16125
Record the rewritten transcript tail as durable
When fixedSuffixTailRewrite is true, the transaction rewrites previousTranscriptLineCount - 1 before appending, but this cache update starts at previousTranscriptLineCount, leaving the old tail in #durableRawLines and #durableRenderedLines. If the transcript later contracts below that row and regrows with the same revised tail, hasDistinctPostContractionRows compares against the stale cache and treats the already-admitted row as new, duplicating it in native scrollback. Include the rewritten tail in the durable range.
ℹ️ 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".
| fixedSuffixScrollRegionToken !== undefined && | ||
| fixedSuffixScrollRegionRasterLease !== undefined && | ||
| fixedSuffixRasterLease?.token === fixedSuffixScrollRegionRasterLease && | ||
| fixedSuffixRasterLease.token.rect.row > 0)); |
There was a problem hiding this comment.
When the Pet lease begins on zero-based row 1—for example, in a short terminal with a one-row hook below the composer—this condition preserves the lease and later emits CSI 1;1 r. DECSTBM requires the top margin to precede the bottom margin, so terminals ignore this one-row region; the following IND then neither admits the displaced transcript row to scrollback nor stays clear of the raster. Require at least two rows above the lease, or invalidate the lease and use the protected fallback.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68a4793f4e
ℹ️ 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".
| fixedSuffixScrollRegionRasterLease !== undefined && | ||
| fixedSuffixRasterLease?.token === fixedSuffixScrollRegionRasterLease && | ||
| fixedSuffixRasterLease.token.rect.row > 0)); | ||
| const soleRasterLease = this.#rasterLeases.size === 1 ? this.#rasterLeases.values().next().value : undefined; |
There was a problem hiding this comment.
During the pre-arm iTerm upload, a disjoint second raster lease can coexist with the Pet lease, but this makes soleRasterLease undefined even though one active lease is nativeScrollbackEligible. On an overflowing append, rasterMustYieldForNativeAdmission therefore remains false and the active-raster shortcut repaints the viewport instead of admitting the displaced row to native scrollback, losing host history during the exact pre-arm interval this flag is intended to protect. Check whether any active lease is eligible rather than only checking when the Pet is the sole lease.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e1978d7093
ℹ️ 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".
| } | ||
|
|
||
| #resetFixedSuffixScrollRegions(): void { | ||
| if (this.#fixedSuffixScrollPlane !== undefined) this.#fixedSuffixScrollRegionResetPending = true; |
There was a problem hiding this comment.
When an iTerm Pet has established #fixedSuffixScrollPlane, this reset only records #fixedSuffixScrollRegionResetPending; TUI.stop() invokes it and then shuts down using direct terminal writes, without passing through any helper that emits the pending CSI r. ProcessTerminal.stop() also does not reset DECSTBM, so a normal exit while the Pet plane is active leaves the user's shell constrained to the Pet's former scroll region and subsequent shell output can scroll incorrectly. Emit the reset synchronously during shutdown rather than merely deferring it to a future render.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a7a846934e
ℹ️ 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".
| const physicalTop = | ||
| plane?.token === fixedSuffixScrollRegionToken && plane.upperBottom === fixedPlaneUpperBottom | ||
| ? plane.transcriptTop | ||
| : prevViewportTop; |
There was a problem hiding this comment.
After the plane has admitted rows, any non-append render such as a status/composer-only update retires it and repaints the natural viewport while #nativeScrollbackViewportTop remains at the later admitted frontier. On the next transcript append, plane is undefined and this fallback uses prevViewportTop, which is behind that frontier by the suffix/raster-excluded rows; the loop below consequently emits IND for rows already present in native scrollback, duplicating history after ordinary suffix updates. Resume from the retained native frontier or preserve the plane's transcriptTop across the repaint.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eb026f85e8
ℹ️ 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 (!fixedPlaneEligible && this.#fixedSuffixScrollPlane !== undefined) { | ||
| this.#fixedSuffixScrollPlane = undefined; | ||
| this.#fixedSuffixScrollRegionResetPending = true; |
There was a problem hiding this comment.
When requestRender(true) runs after the iTerm fixed plane is established, #requestRenderCore sets the previous dimensions to -1, so #doRender takes the width-change return around line 4607 before reaching this retirement block. On a plain iTerm host that path performs fullRender while the old DECSTBM margin is still active; its CRLF sequence then scrolls only the upper Pet plane, corrupting the viewport and native history before raster invalidation can schedule a reset. Retire and reset the plane before any earlier full-render or viewport-repaint return.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a33c67902e
ℹ️ 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 (fixedPlaneEligible) { | ||
| const plane = this.#fixedSuffixScrollPlane; | ||
| const physicalTop = fixedPlanePhysicalTop; | ||
| const desiredTop = Math.max(0, nextTranscriptLineCount - fixedPlaneUpperBottom); |
There was a problem hiding this comment.
When the bound raster starts above the suffix boundary, fixedPlaneUpperBottom is smaller than the viewport's transcript capacity, so computing desiredTop from that reduced value admits the intervening transcript rows to scrollback as though they were no longer visible. In the five-row test geometry (rect.row = 2, three transcript rows, two suffix rows), the first one-line append advances from physicalTop = 0 to desiredTop = 2, emitting two INDs instead of one; row 2 is then neither part of the upper-plane repaint nor the suffix repaint and retains stale text beside the raster. Fresh evidence in the final persistent-plane path is that the loop at lines 4887–4902 paints only rows below fixedPlaneUpperBottom, while the next repaint begins at suffixRegionBottom, leaving this gap unhandled.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d8b26d690d
ℹ️ 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 ( | ||
| !this.#writeFixedSuffixResetBefore(bytes => this.#guardTerminalOperation(() => this.terminal.write(bytes))) | ||
| ) | ||
| return failed(); |
There was a problem hiding this comment.
When submitTerminalOutput() runs while #fixedSuffixScrollPlane is active, this reset is a no-op because #fixedSuffixScrollRegionResetPending remains false for a live plane. A generic-render, generic-full-redraw, or queued payload containing scrolling control characters is therefore written under the Pet's persistent DECSTBM margins; for generic output, the lease callback retires the plane only after the combined erase/render write, so it is already too late to prevent partial-region scrolling and viewport/history corruption. Retire the live plane and reset the margins before any non-plane terminal ingress.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 770e45acb7
ℹ️ 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".
| fixedPlaneBuffer += `${sliceByColumn(suffixLine, segment.column, segment.width, true)}${SEGMENT_RESET}`; | ||
| } | ||
| } | ||
| const { seq, toRow } = this.#cursorControlSequence(cursorPos, newLines.length, this.#hardwareCursorRow); |
There was a problem hiding this comment.
When the persistent iTerm plane is active and the cursor-bearing composer has rows beneath it, appending transcript rows increases cursorPos.row even though DECRC restores the cursor to the same physical suffix row. Passing the unchanged logical #hardwareCursorRow as fromRow therefore makes #cursorControlSequence emit one CUD per admitted row, moving the hardware/IME cursor into the floor or hook rows until it reaches the terminal bottom. Account for desiredTop - physicalTop when supplying the restored logical row so the fixed composer cursor stays physically stationary.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 63b7e92016
ℹ️ 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".
| fixedSuffixLease.nativeScrollbackEligible && | ||
| fixedSuffixLease.nativeScrollbackArmed && | ||
| fixedSuffixLease.token.rect.row > 0 && |
There was a problem hiding this comment.
When requestRender(true) or another full-render trigger runs while the iTerm multipart upload is pending, the lease is nativeScrollbackEligible but not yet nativeScrollbackArmed, so this check falls through to a protected full render. Because that render is queued behind the multipart operation, #writeProtectedRenderIngress() subsequently erases the GIF that was just uploaded, causing the Pet to disappear or be reuploaded with visible flicker. Treat the eligible pre-arm lease as preserving, or re-evaluate the lease state when the queued render reaches the terminal.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 319df8f6d0
ℹ️ 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 (newLines.slice(Math.max(0, newLines.length - height)).some(line => TERMINAL.isImageLine(line))) { | ||
| // A competing image protocol cannot be clipped safely around the iTerm | ||
| // raster. Preserve the current frame rather than erasing the resident GIF. | ||
| return; |
There was a problem hiding this comment.
When an ordinary iTerm2 image, such as an image tool result, enters the visible transcript while the Pet lease is armed, this branch returns without writing the frame, invalidating the Pet lease, or scheduling an alternative render. Subsequent renders encounter the same image line and return again, so the image remains invisible until the lease is released; yield the Pet lease or otherwise render the competing image rather than silently suppressing the frame.
Useful? React with 👍 / 👎.
| this.overlayStack.length === 0 && | ||
| previousKittyPlacementSpans.length === 0 && | ||
| nextKittyPlacementSpans.length === 0 && | ||
| !newLines.some(line => TERMINAL.isImageLine(line)) && |
There was a problem hiding this comment.
After an iTerm2 image scrolls out of view but remains in the transcript, this whole-frame predicate keeps fixedPlaneEligible false for every later append. The armed-lease fallback then sees no image in the visible tail, repeatedly viewport-repaints, and never reaches native admission, so all subsequent transcript rows are omitted from host scrollback; restrict the image exclusion to rows the fixed-plane transaction will actually touch.
Useful? React with 👍 / 👎.
Add lease-based OSC 1337 GIF rendering for iTerm2 while preserving Sixel, Kitty, tmux, viewport, and ordinary renderer behavior.
Bind the iTerm Pet's existing raster lease to the fixed-suffix DECSTBM owner.\n\nTranscript growth now enters native scrollback without erasing or reuploading the GIF, while other raster protocols retain their existing renderer path.\n\nLore-id: c07f1e5b\nConstraint: only preserve the sole current bound raster lease\nConstraint: reset DECSTBM in every transaction\nConstraint: preserve manual viewport and managed tmux behavior\nTested: focused TUI fixed-suffix/raster/render tests; iTerm widget/transport/QA tests; package type checks\nNot-tested: direct and managed iTerm human session\nScope-risk: narrow\nReversibility: straightforward
A bound iTerm raster lease now keeps its suffix cells free of erase-line controls while the padded suffix repaint preserves native scrollback. Lore-id: decstbm-fix-02 Constraint: retain generic suffix clearing without a raster binding Tested: fixed suffix, raster lease, and render commit regressions Scope-risk: narrow
Release notes now cover the raster-bound suffix primitive and the iTerm Pet behavior it enables. Lore-id: decstbm-docs-01 Tested: focused TUI and coding-agent checks Scope-risk: narrow
An ineligible fixed-suffix raster binding now releases the exceptional lease through protected ingress and keeps the DECSTBM transcript transaction, instead of repainting past rows that host scrollback never received. Lore-id: decstbm-fix-03 Constraint: preserve the normal sole iTerm lease no-flicker path Tested: fixed suffix, raster lease, and render commit regressions; TUI check Scope-risk: narrow
A raster lease exists while the iTerm GIF upload is still pending, before its fixed-suffix token is armed. Mark only that lease as eligible so overflow yields through protected native admission instead of silently repainting away host history.\n\nConstraint: preserve Kitty, Sixel, and generic raster lease behavior\nTested: focused TUI raster/fixed-suffix/render tests; focused Pet transport/QA tests; package checks\nScope-risk: narrow\nReversibility: straightforward
DECSTBM kept the composer suffix fixed but rewrote every suffix cell, including the live iTerm raster footprint. Paint only complementary cell spans while the bound lease remains active.\n\nConstraint: retain native scrollback without GIF reupload\nTested: fixed-suffix, raster-lease, and render-commit tests; TUI package check\nScope-risk: narrow\nReversibility: straightforward
A transparent iTerm canvas may extend above rendered suffix lines. Bound DECSTBM at the current raster lease top so IND never scrolls a Pet row while the physical composer stays at its original rows.\n\nConstraint: preserve native scrollback and no GIF flicker\nTested: fixed-suffix, raster-lease, and render-commit tests; TUI package check\nScope-risk: narrow\nReversibility: straightforward
Streaming Markdown can rewrite the prior tail row while appending a wrapped row. Admit that bounded tail rewrite inside the fixed scroll transaction instead of falling through to protected lease erasure.\n\nConstraint: retain native scrollback without Pet flicker\nTested: fixed-suffix, raster-lease, and render-commit tests; TUI package check\nScope-risk: narrow\nReversibility: straightforward
A persistent DECSTBM plane is valid only for append frames with a current iTerm lease. Retire it before non-append output, reset margins ahead of generic/shutdown writes, and record every batch-admission row in durable state.\n\nConstraint: preserve native history, manual follow, graphics isolation, and lower cursor ownership\nTested: bun test test/fixed-suffix-scroll-region.test.ts test/raster-lease.test.ts test/render-commit.test.ts; bun run check\nScope-risk: focused\nReversibility: direct
A preserved raster render could otherwise retain DECSTBM margins after the fixed plane retired. Prefix every next render ingress with the margin reset so generic rendering always resumes full-terminal semantics.\n\nConstraint: preserve live raster leases while restoring full margins\nTested: bun test test/fixed-suffix-scroll-region.test.ts test/raster-lease.test.ts test/render-commit.test.ts; bun run check; git diff --check\nScope-risk: narrow\nReversibility: direct
A row leaving the DECSTBM plane must be painted with its latest content before IND commits it to host scrollback. Reject frames that would remap already-admitted history, and reset active margins during terminal teardown.\n\nConstraint: preserve the lower Pet/composer plane without duplicate or stale host history\nTested: bun test test/fixed-suffix-scroll-region.test.ts test/raster-lease.test.ts test/render-commit.test.ts; bun run check; git diff --check\nScope-risk: narrow\nReversibility: direct
Queued terminal output bypassed the render ingress that restores full DECSTBM margins after a fixed plane released. Restore them at the shared queue boundary before any subsequent terminal bytes.\n\nConstraint: queued protocol output must never inherit a retired scroll region\nTested: bun test test/fixed-suffix-scroll-region.test.ts test/raster-lease.test.ts test/render-commit.test.ts; bun run check; git diff --check\nScope-risk: narrow\nReversibility: direct
Lease invalidation and lifecycle cleanup bypassed queued output's DECSTBM reset. Centralize the pending-margin guard across terminal, multipart, cleanup, and lifecycle writes.\n\nConstraint: no queued terminal byte may inherit a retired fixed scroll region\nTested: bun test test/fixed-suffix-scroll-region.test.ts test/raster-lease.test.ts test/render-commit.test.ts; bun run check; git diff --check\nScope-risk: narrow\nReversibility: direct
Lifecycle recovery dispatched retained disjoint generic bytes without the fixed-plane reset. Route that final ingress through the shared writer.\n\nConstraint: deferred terminal output must not inherit retired DECSTBM margins\nTested: bun test test/fixed-suffix-scroll-region.test.ts test/raster-lease.test.ts test/render-commit.test.ts; bun run check; git diff --check\nScope-risk: narrow\nReversibility: direct
An iTerm working or burst GIF cannot be replaced without flashing its transparent canvas. When it reached an opaque intermediate frame, the idle Pet remained a solid orange box. Use the stable idle loop for iTerm submissions.\n\nConstraint: preserve iTerm lease continuity without opaque idle frames\nTested: bun test test/gajae-pet-widget.test.ts test/modes/components/iterm-pet-transport.test.ts test/qa-iterm-pet.test.ts; bun run check; git diff --check\nScope-risk: narrow\nReversibility: direct
A native-eligible lease yielded during its GIF upload window, repeatedly erasing the Pet before fixed-suffix ownership could arm. Separate authorization from armed native admission so pre-arm output preserves the raster.\n\nConstraint: native admission begins only after the iTerm GIF and fixed suffix owner commit\nTested: bun test test/fixed-suffix-scroll-region.test.ts test/raster-lease.test.ts test/render-commit.test.ts; bun run check; git diff --check\nScope-risk: narrow\nReversibility: direct
A static idle GIF avoided stale frames but removed the ParaPara working animation. Keep one working loop resident and append a safe base frame for iTerm implementations that stop after one pass.\n\nConstraint: preserve iTerm continuity without an opaque terminal frame\nTested: bun test test/gajae-pet-widget.test.ts test/modes/components/iterm-pet-transport.test.ts test/qa-iterm-pet.test.ts; bun run check; git diff --check\nScope-risk: narrow\nReversibility: direct
Restore the full terminal margin before lower-pane and multipart iTerm output.\nKeep an armed plane from expanding when the composer changes size, avoiding raster erasure during streaming updates.\n\nLore-id: 5f913e2d\nConstraint: preserve native scrollback without iTerm Pet flicker\nConstraint: keep Kitty/Sixel and manual viewport paths unchanged\nConfidence: medium\nScope-risk: focused\nReversibility: easy\nTested: focused fixed-suffix, raster lease, render commit, and iTerm pet widget suites\nTested: TUI and coding-agent package checks
A growing composer reduces the usable transcript plane.\nExercise the old margin for ordered admission before repainting within the smaller region.\n\nLore-id: 8a4ce190\nConstraint: raster lease must stay resident during composer growth\nTested: focused fixed-suffix, raster lease, and render commit suite\nTested: TUI package check
Prove real TUI post-render and queued multipart bytes follow a full DECSTBM reset.\n\nLore-id: 7c2b481f\nConstraint: iTerm multipart bytes must not inherit a scroll margin\nTested: focused fixed-suffix, raster lease, and render commit suite\nTested: TUI package check
Historical Markdown rewrites above the live DECSTBM plane are already immutable host history.\nContinue the fixed-plane transaction and repair each departing row instead of erasing the iTerm raster through generic rendering.\n\nLore-id: 0d97be4a\nConstraint: streaming reflow must not erase the resident iTerm Pet\nConstraint: preserve ordered native admission for displaced live rows\nConfidence: high\nScope-risk: focused\nReversibility: easy\nTested: focused fixed-suffix, raster lease, and render commit suite\nTested: TUI package check
An armed DECSTBM lease previously yielded to generic or forced rendering whenever a transient frame could not form a fixed-plane transaction. Protected ingress erased the GIF before drawing that frame.\n\nKeep current armed iTerm leases on clipped rendering, stage later native admission, clear stale arm state, and re-arm reset owners without uploading another GIF. Lore-id: b7c3e91a\nConstraint: preserve native iTerm scrollback and resident Pet animation\nConstraint: keep Kitty/Sixel and generic raster behavior unchanged\nConfidence: high\nScope-risk: focused\nReversibility: straightforward\nTested: focused fixed-suffix, raster-lease, render-commit, and iTerm widget suites\nTested: packages/tui and packages/coding-agent checks
A current native fixed-suffix lease still reached protected ingress through a non-preserving append branch while a multipart barrier was active.\n\nRestrict that branch to preserving transactions and defer unsafe frames until raster cleanup settles, keeping the resident GIF intact. Lore-id: e4d721b9\nConstraint: never erase an armed iTerm lease for ordinary streaming fallback\nConstraint: preserve generic raster and non-iTerm behavior\nConfidence: high\nScope-risk: focused\nReversibility: straightforward\nTested: focused fixed-suffix, raster-lease, and render-commit suites\nTested: packages/tui check
A frame containing another terminal image protocol cannot be safely repainted through the fixed iTerm plane. Preserve the resident iTerm GIF instead of routing that frame through an ingress that erases its lease. Lore-id: 92e1ad3c Constraint: keep Kitty and Sixel paths outside the iTerm native-scrollback plane Constraint: do not erase or re-upload a resident iTerm GIF for image-bearing frames Confidence: high Scope-risk: narrow Reversibility: straightforward Tested: bun test test/fixed-suffix-scroll-region.test.ts test/raster-lease.test.ts test/render-commit.test.ts; bun run check
Releasing a fixed-suffix owner disarms its lease. A later arm must restore native admission without erasing the resident iTerm raster. Lore-id: 760e9c3f Constraint: preserve the resident iTerm GIF across release and re-arm Confidence: high Scope-risk: narrow Reversibility: straightforward Tested: bun test test/fixed-suffix-scroll-region.test.ts test/raster-lease.test.ts test/render-commit.test.ts
Viewport-repaint branches run before fixed-plane admission. When another image is visible, those branches must not route a resident iTerm Pet through protected ingress and erase it. Lore-id: 39b2e6a1 Constraint: preserve a current armed iTerm lease when a visible frame contains another image protocol Constraint: keep generic raster and Kitty/Sixel behavior unchanged Confidence: high Scope-risk: narrow Reversibility: straightforward Tested: bun test test/fixed-suffix-scroll-region.test.ts test/raster-lease.test.ts test/render-commit.test.ts; bun run check; git diff --check
A viewport-repaint image frame must neither erase the resident iTerm lease nor enter the DECSTBM fixed plane. Lore-id: 6fb5291a Constraint: retain raster lease across competing image viewport frames Confidence: high Scope-risk: narrow Reversibility: straightforward Tested: bun test test/fixed-suffix-scroll-region.test.ts test/raster-lease.test.ts test/render-commit.test.ts; bun run check; git diff --check
MultipartFile has no payload delimiter. The prior leading separator and colon were not part of iTerm2's multipart protocol and could leave its image decoder with malformed transfer metadata. Lore-id: c4d920ab Constraint: retain 256-byte managed-tmux records Constraint: preserve standard direct and managed iTerm2 multipart parsing Confidence: high Scope-risk: narrow Reversibility: straightforward Tested: bun test test/gajae-pet.test.ts test/iterm2-protocol.test.ts test/fixed-suffix-scroll-region.test.ts test/raster-lease.test.ts test/render-commit.test.ts; bun run check; git diff --check
The widget-level direct transport must retain the metadata-only MultipartFile header rather than accepting File's colon delimiter. Lore-id: 182f07cb Constraint: direct and managed iTerm records use canonical MultipartFile metadata Confidence: high Scope-risk: narrow Reversibility: straightforward Tested: bun test test/gajae-pet-widget.test.ts; bun run check; git diff --check
A resident iTerm GIF cannot react to later activity changes. Keying the existing lease-backed upload by idle or working state restores idle after work without frame-by-frame uploads. Lore-id: 89de42c7 Constraint: retain canonical MultipartFile transport Constraint: retain native scrollback and raster lease behavior Constraint: do not erase the resident iTerm raster on activity transitions Confidence: medium Scope-risk: narrow Reversibility: straightforward Tested: bun test test/gajae-pet-widget.test.ts; bun --cwd=packages/coding-agent run check; bun --cwd=packages/tui test test/gajae-pet.test.ts test/iterm2-protocol.test.ts test/fixed-suffix-scroll-region.test.ts test/raster-lease.test.ts test/render-commit.test.ts; bun --cwd=packages/tui run check; git diff --check
The release rebase kept the current parser expectation when the historical iTerm transport test conflicted. Lore-id: b1c9a3e4 Constraint: preserve release 0.12.8 alignment Confidence: high Scope-risk: narrow Reversibility: straightforward Tested: bun --cwd=packages/coding-agent run check; bun --cwd=packages/tui run check; focused iTerm TUI and widget tests; git diff --check
6d20788 to
d3201e2
Compare
39bc1c3 to
1883e1b
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1883e1b099
ℹ️ 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.#viewportTopRow = Math.max(0, newLines.length - height); | ||
| this.#nativeScrollbackViewportTop = Math.max(this.#nativeScrollbackViewportTop, this.#viewportTopRow); |
There was a problem hiding this comment.
When a multipart GIF upload is pending while a fixed plane already exists, #rasterPending disables the persistent-plane branch, but this fallback still emits IND operations and updates only #viewportTopRow; it leaves #fixedSuffixScrollPlane.transcriptTop at its pre-admission value. The next eligible append therefore calculates desiredTop - physicalTop from a stale frontier, emits extra INDs, and can admit the same transcript row twice into native scrollback. Fresh evidence beyond the earlier frontier report is that activity-phase GIF replacements now make this pending-multipart fallback occur during ordinary idle/working transitions; update or retire the persistent plane when this callback commits.
Useful? React with 👍 / 👎.
Working GIF frames use transparent pixels and iTerm rendered their restore-to-background disposal as an orange canvas during transcript scroll. Restore the prior terminal plane between working frames while leaving idle and non-iTerm artifacts unchanged. Lore-id: f7c2d4a1 Constraint: preserve native scrollback and resident raster lease Constraint: preserve idle GIF and Kitty/Sixel behavior Confidence: medium Scope-risk: narrow Reversibility: straightforward Tested: bun --cwd=packages/tui test test/gajae-pet.test.ts test/iterm2-protocol.test.ts test/fixed-suffix-scroll-region.test.ts test/raster-lease.test.ts test/render-commit.test.ts; bun --cwd=packages/tui run check; bun --cwd=packages/coding-agent test test/gajae-pet-widget.test.ts; bun --cwd=packages/coding-agent run check; git diff --check
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ede469c7ad
ℹ️ 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".
| import { VirtualTerminal } from "./virtual-terminal"; | ||
|
|
||
| const REPORT_PATH = "artifacts/g015-qa-report.json"; | ||
| const REPORT_PATH = path.join(fs.mkdtempSync(path.join(os.tmpdir(), "g015-qa-")), "g015-qa-report.json"); |
There was a problem hiding this comment.
When these QA suites are run to regenerate their committed reports, this randomized /tmp path means the output is no longer written to packages/tui/artifacts/g015-qa-report.json and the temporary location is never exposed to callers, leaving the checked-in QA evidence stale. The same regression was introduced in the G003, G011, and G014 report tests; retain their stable artifact paths or explicitly publish/copy the temporary reports.
Useful? React with 👍 / 👎.
An activity edge may finish its captured same-lease upload before the next timer tick corrects the resident GIF. Assert both artifacts so the bounded transition window cannot regress into a stale steady state. Lore-id: c4f7319a Constraint: preserve same-lease activity transitions Confidence: high Scope-risk: narrow Reversibility: straightforward Tested: bun --cwd=packages/coding-agent test test/gajae-pet-widget.test.ts; bun --cwd=packages/coding-agent run check; git diff --check
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 127be49054
ℹ️ 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".
| // it must not force a generic raster erase. The departing plane row is | ||
| // repaired immediately before its IND below, so native admission stays | ||
| // correct for every newly displaced live row. | ||
| nextTranscriptLineCount >= previousTranscriptLineCount && |
There was a problem hiding this comment.
When a streamed Markdown block gains rendered rows above fixedPlanePhysicalTop in the same frame that appends output, this count-only condition still admits the frame. The admission loop then compares rows by their shifted indexes, so it writes a later row into the top of the plane before IND; native scrollback can consequently omit the inserted row and duplicate a previously admitted row. Verify that the prefix above the physical frontier has not shifted, or use a preserving repaint instead of fixed-plane admission for this case.
Useful? React with 👍 / 👎.
| @@ -1,10 +1,12 @@ | |||
| import { afterAll, beforeEach, describe, expect, it, vi } from "bun:test"; | |||
| import { mkdirSync, writeFileSync } from "node:fs"; | |||
| import { mkdtempSync, writeFileSync } from "node:fs"; | |||
There was a problem hiding this comment.
When these QA modules load, the newly added mkdtempSync calls perform synchronous directory I/O, and the named Node imports introduced here are repeated in the G011 and G014 report tests. Move temporary-directory creation into asynchronous setup using namespace imports from node:fs/promises, as required by the repository filesystem convention.
AGENTS.md reference: AGENTS.md:L122-L122
Useful? React with 👍 / 👎.
1 participant
iTerm Pet integration for fixed-suffix native scrollback
Fork-only stacked integration PR based on #17, both rebased on
devrelease0.12.8(be3940aa).Verification
bun --cwd=packages/coding-agent run checkbun --cwd=packages/tui run checkgit diff --checkLive iTerm direct and managed-tmux acceptance remains required for compositor behavior.