fix(pet): stabilize iTerm2 Gajae Pet placement, drag handling, and animation rendering - #4591
Conversation
![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: 10ded2afa0
ℹ️ 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
gajae-code/packages/tui/src/terminal.ts
Line 890 in 53b6b7a
Replace the timeout promise with Promise.withResolvers
The new capability-probe quiescence loop constructs a timeout promise directly, contrary to the repository contract requiring Promise.withResolvers() instead of new Promise(...); rewrite this wait using the mandated resolver pattern.
AGENTS.md reference: AGENTS.md:L119-L119
ℹ️ 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".
|
Validated both review findings against the live input/protocol paths and applied only the behavior-preserving fixes in
Validation:
|
|
Maintained review/salvage is now active on this PR at exact head — |
|
Validated both lifecycle findings against the stop/start and initialization order before changing them. Applied the narrow fixes in
Validation:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d30ea2251
ℹ️ 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".
|
Validated both newly detected findings against current source and direct dogfooding before applying
Validation:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1f76721d2a
ℹ️ 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".
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Fix-forward applied at exact head — |
|
Validated the repeated managed-topology report and applied the focused fix in
Validation:
|
Yeachan-Heo
left a comment
Yeachan-Heo
left a comment
There was a problem hiding this comment.
MERGE_READY at exact head 676000d8 (base d332b789, digest sha256:03163b59ac2d12d8643c3973f51415e3cca7c4c55fa835bd97cfbcd63912c949).
Adversarial review conclusions on this exact head — all seven tracked findings proven fixed, plus the two late contributor commits verified:
- Destructive probe drain (P1, fixed at 53b6b7a) —
drainPendingInputwaits on stdin quiescence only: no\x1b[<u, no\x1b[>4;0m, no#inputHandlerdetach, input-handler restore not needed (never detached);drainPetProbeInputprefers it and only falls back to the shutdown drain when absent.terminal-probe-reply-leak.test.tsproves keyboard modes survive and startup keys still arrive, including with the pet setting off. - Multiplexer forced-Kitty (P2, fixed at 53b6b7a) —
getPetPixelProtocol()gates Kitty on!isUnderTerminalMultiplexer(); re-proved live across tmux (TMUX/TMUX_PANE), screen (STY,TERM=screen*/TERM=tmux*), zellij (ZELLIJ) withPI_FORCE_IMAGE_PROTOCOL=kitty→ null every time, while forced Sixel and the separately verified managed-iTerm transport remain allowed, and plain Kitty outside multiplexers still resolves. - Stopped-TUI animation writes (P1, fixed at 2d30ea2) —
TUI.isRunninggates the iTerm tick entry, and the asynccurrent()freshness predicate re-checksisRunningbefore every acquire/submit/write, closing the delayed-acquisition race during external-editor stop; restart resumes submissions. Widget test covers stopped → no output/invalidated lease → resumed → GIF written. - Capability deadline ordering (P2, fixed at 2d30ea2) — the settle timer is now registered after
ui.start()and after probe/poll initiation; the fake-timer regression proves pre-start initialization cannot consume the deadline and the full window remains post-probe. - Skin frame registry (P1, fixed at 1f76721) — crab-only
IDLE_LOOP/WORK_LOOPhard-coding replaced byPET_SKINS[mode]metadata (baseFrame, idle/work, workEnter/workExit transitions, bursts, workBursts);#itermTimelinemirrors it for GIFs and the semantic key includes the timeline signature. Re-proved at runtime: red/blue/ouroboros idle, work, transitions, and burst timelines resolve only to real frames for Kitty/Sixel pixel maps and iTerm GIF builds — no undefined frame, noUnknown ouroboros GIF frame. - Bash/Python drag-path ordering (P2, fixed at 1f76721) — the marked
.gajae-pet.(png|gif)drag check now runs before the shell-mode bypass, so the pet artifact never lands in!/$composers, while ordinary image paths still fall through to literal/attachment handling (covered for both modes and both extensions). Promise.withResolverscontract (fixed-forward by maintainer at 23a5e38) — the PR-addeddrainPendingInputquiescence wait now usesPromise.withResolvers(); no PR-introducednew Promise(...)remains (the pre-existingdrainInputoccurrence is untouched base code). Bounded drain timing re-proven.
Late delta 23a5e38d..676000d8 (one-line revoke() idempotence guard + 97-line managed-topology test) verified against all revoke side effects: identical-reason re-revoke short-circuits without epoch churn or duplicate listener events; changed-reason and available-state revocations still take the full path; pending-probe resolution and dispose idempotence unaffected; recovery then re-revocation emits exactly the expected transition sequence.
Evidence on this head: focused battery 248 pass / 2080 expectations across 10 files (including the new iterm-pet-transport.test.ts), bun --cwd=packages/tui run check and bun --cwd=packages/coding-agent run check green, state-writer fast gate clean, GIF cache bounds (≤32 entries / 8 MiB with eviction) re-proven, capability-reply parser boundaries re-proven (F → complete, malformed → invalid, incomplete → undefined).
The PR contract bootstrap red is the expected external gate: it intentionally fails while the verdict is needs-human and then requires an authenticated exact-head APPROVED review from a write-capable non-author reviewer. Requesting @probepark (primary) and @HaD0Yun (backup) for the exact-head approval; on approval the single verdict line will be promoted to merge-approved with this same digest and the PR merged to dev.
—
[repo owner's gaebal-gajae (clawdbot) 🦞]
|
@probepark @HaD0Yun — requesting an exact-head review approval on — |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 676000d8ef
ℹ️ 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".
| @@ -1537,6 +1569,11 @@ export class InteractiveMode implements InteractiveModeContext { | |||
| this.#petUnavailableWarningDisposer = undefined; | |||
| this.petWidget?.dispose(); | |||
There was a problem hiding this comment.
When InteractiveMode.stop() is invoked directly rather than through shutdown(), dispose() only schedules the Kitty/Sixel erase on the Promise-backed raster queue, while this method proceeds synchronously to ui.stop(). The erase can therefore be written after raw mode and cursor state have already been restored, interleaving escape sequences with the parent shell prompt; ensure pet cleanup is completed or synchronously registered and flushed before stopping the TUI.
Useful? React with 👍 / 👎.
| if (start < 0) { | ||
| const suffixLength = Math.min(marker.length - 1, combined.length - offset); | ||
| const candidate = combined.slice(combined.length - suffixLength); | ||
| const keep = candidate.length > 1 && marker.startsWith(candidate) ? candidate : ""; |
There was a problem hiding this comment.
When an iTerm capability reply is split immediately after its initial ESC byte, this candidate.length > 1 condition forwards that ESC to the editor and forgets it, so the following ]1337;Capabilities=... chunk is never recognized and the probe incorrectly times out. Input streams may split at any byte boundary, so retain a one-byte marker prefix with a safe probe-end flush path and cover that fragmentation boundary.
AGENTS.md reference: AGENTS.md:L158-L158
Useful? React with 👍 / 👎.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Fix-forward at new exact head — |
iTerm2 now uses a geometry-derived three-row canvas with transparent insets instead of a fixed pixel box.\n\nThe iTerm anchor no longer receives the shared one-row safety lift, so the padded canvas aligns to the composer boundary.\n\nLore-id: iterm-pet-center\nConfidence: high\nScope-risk: narrow\nReversibility: simple\nTested: bun test packages/coding-agent/test/gajae-pet-widget.test.ts packages/tui/test/gajae-pet.test.ts
iTerm turns a native inline-image drag into a bracketed temporary-file path paste.\n\nTag pet frames with a dedicated filename and consume only that decoded path in the composer, keeping native dragging and ordinary image-path pastes intact.\n\nLore-id: iterm-pet-drag-paste\nConfidence: high\nScope-risk: narrow\nReversibility: simple\nTested: bun test packages/coding-agent/test/input-controller-keybindings.test.ts packages/coding-agent/test/gajae-pet-widget.test.ts packages/tui/test/gajae-pet.test.ts\nTested: bun --cwd=packages/coding-agent run check\nTested: bun --cwd=packages/tui run check
iTerm replaces inline images at a stable cell block.\n\nErasing that block before each animation frame exposed the composer background while the next PNG loaded. Keep explicit erasure for Sixel and lifecycle cleanup only.\n\nLore-id: iterm-pet-animation-flash\nConfidence: medium\nScope-risk: narrow\nReversibility: simple\nTested: bun test packages/coding-agent/test/gajae-pet-widget.test.ts packages/coding-agent/test/input-controller-keybindings.test.ts packages/tui/test/gajae-pet.test.ts\nTested: bun --cwd=packages/coding-agent run check
Per-frame PNG writes raced with full-line TUI erases, exposing transient background frames and repeatedly reloading the pet.\n\nUse a lease-protected iTerm GIF record that changes only with meaningful pet state, preserving the normal renderer outside its exact cell rectangle.\n\nLore-id: iterm-pet-stable-rendering\nConfidence: high\nScope-risk: focused\nReversibility: isolated-renderer\nTested: bun --cwd=packages/tui run check\nTested: bun --cwd=packages/coding-agent run check\nTested: bun test packages/tui/test/gajae-pet.test.ts packages/tui/test/raster-lease.test.ts packages/coding-agent/test/gajae-pet-widget.test.ts packages/coding-agent/test/input-controller-keybindings.test.ts\nTested: bun test packages/tui/test/render-commit.test.ts packages/tui/test/mouse-sgr.test.ts
Capability probes used the shutdown drain and allowed forced Kitty graphics through multiplexers.\n\nProbe input now waits non-destructively, while pet capability selection keeps raw Kitty unavailable under tmux, screen, and zellij.\n\nLore-id: iterm-pet-probe-guard\nConfidence: high\nScope-risk: narrow\nReversibility: simple\nTested: bun --cwd=packages/tui run check\nTested: bun --cwd=packages/coding-agent run check\nTested: bun test packages/tui/test/terminal-probe-reply-leak.test.ts packages/tui/test/raster-lease.test.ts packages/tui/test/gajae-pet.test.ts\nTested: bun test packages/coding-agent/test/modes/components/pet-capability.test.ts packages/coding-agent/test/gajae-pet-widget.test.ts
Stopped TUI sessions could reacquire an iTerm raster lease and start the capability warning before its probe began.\n\nGate asynchronous animation submission on TUI lifecycle state and start the warning deadline only after UI startup initiates probing.\n\nLore-id: iterm-pet-lifecycle-guard\nConfidence: high\nScope-risk: narrow\nReversibility: simple\nTested: bun --cwd=packages/tui run check\nTested: bun --cwd=packages/coding-agent run check\nTested: bun test packages/coding-agent/test/gajae-pet-widget.test.ts packages/coding-agent/test/interactive-mode-editor-component.test.ts packages/tui/test/raster-lease.test.ts packages/tui/test/terminal-probe-reply-leak.test.ts\nTested: bun test packages/coding-agent/test/modes/components/pet-capability.test.ts packages/tui/test/gajae-pet.test.ts packages/coding-agent/test/input-controller-keybindings.test.ts
The stable iTerm port reused crab frame names for every skin and bypassed marked drag-path suppression in shell modes.\n\nUse each skin's authored animation registry end to end, and consume only marked pet paths before the Bash/Python literal-paste bypass.\n\nLore-id: pet-skin-and-shell-drag-guard\nConfidence: high\nScope-risk: narrow\nReversibility: simple\nTested: bun --cwd=packages/coding-agent run check\nTested: bun test packages/coding-agent/test/gajae-pet-widget.test.ts packages/coding-agent/test/input-controller-keybindings.test.ts
The new drainPendingInput quiescence loop constructed its sleep promise with new Promise(...), violating the repository contract that mandates Promise.withResolvers(); the executor only wired a setTimeout resolve. Lore-id: pr4591-withresolvers Confidence: high Scope-risk: narrow Reversibility: simple Tested: bun test packages/tui/test/terminal-probe-reply-leak.test.ts; bun --cwd=packages/tui run check
An ineligible managed tmux topology was revoked on every polling interval, repeatedly forcing pet lifecycle work.\n\nSuppress duplicate unavailable transitions for the same reason while preserving changed-reason and recovery transitions.\n\nLore-id: iterm-pet-topology-idempotence\nConfidence: high\nScope-risk: narrow\nReversibility: simple\nTested: bun --cwd=packages/coding-agent run check\nTested: bun --cwd=packages/tui run check\nTested: bun test packages/coding-agent/test/modes/components/iterm-pet-transport.test.ts packages/tui/test/terminal-probe-reply-leak.test.ts packages/coding-agent/test/gajae-pet-widget.test.ts packages/coding-agent/test/input-controller-keybindings.test.ts
The capability-input fragment filter kept only one fixed-length suffix candidate, so a reply split after the ESC byte (or any shorter valid marker prefix inside a longer chunk) forwarded the prefix to the editor and the reply was never recognized; the probe then timed out. Scan suffix lengths descending and retain the longest marker prefix. Lore-id: pr4591-marker-fragments Constraint: must not weaken the ST-terminator regression or leak capability replies to the editor Confidence: high Scope-risk: narrow Reversibility: simple Tested: bun test packages/coding-agent/test/modes/components/iterm-pet-transport.test.ts (10 pass incl. 5 split matrices) Tested: focused battery 255 pass / 2102 expectations across 10 files; both package checks green
6d69a33 to
e5abb7d
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Rebased onto current — |
submitTerminalOutput invoked queued shouldWrite() freshness predicates outside the terminal-operation guard at both the early gate and the post-barrier gate. A throwing predicate rejected the raster-queue promise; the iTerm pet submits fire-and-forget, so that rejection surfaced as an unhandled rejection and production fatal handling escalates it to process exit. afterPrefix already degraded via try/catch, so wrap both predicate callsites the same way and return the typed failed ack instead of rejecting. Lore-id: pr4591-shouldwrite-guard Constraint: false must still classify stale-token and true must still write; unrelated terminal errors must keep failing normally Confidence: high Scope-risk: narrow Reversibility: simple Tested: bun test packages/tui/test/raster-lease.test.ts (33 pass incl. throwing/false/true predicate matrix) Tested: focused battery 256 pass / 2108 expectations across 10 files; tui + coding-agent checks green
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Fix-forward at new exact head — |
Direct InteractiveMode.stop() queued pet erase through the raster queue and then restored the terminal synchronously; the deferred queue body still wrote after restoration, interleaving escapes with the parent shell. A multipart body that already began could also resume after an await (flush/afterPrefix) or a user predicate that stops the terminal and then throws/returns false, emitting suffix, abort, and cursor restoration bytes post-stop. Capture a raster lifecycle epoch in #enqueueRaster and increment it as the first action of stop(). Queue bodies receive an isCurrentLifecycle() predicate, evaluated at entry and re-checked after every await and before every later terminal write; stale bodies settle with their typed failed/stale-token/rejected/retained results. abortBarrier refuses to write once stale, so no abort or cursor-restoration bytes escape. A retained cleanup payload is delivered by the next start(), which keeps stop() synchronous and preserves the stop-flush cleanup window. Also gate the pet-unavailable warning suffix on an observed iTerm transport reason so non-iTerm terminals stop rendering '(unknown)'. Lore-id: pr4591-raster-stop-epoch Constraint: stop() stays synchronous; synchronous stop cleanup before restoration must keep working; false must still classify stale-token and true must still write Confidence: high Scope-risk: narrow Reversibility: simple Tested: bun test packages/tui/test/raster-lease.test.ts (34 pass incl. gated-ingress, paused-at-await, predicate-stop, restart-delivery matrix) Tested: focused battery 259 pass / 2142 expectations across 10 files; tui + coding-agent checks green
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Fix-forward at new exact head (1) Post-stop raster writes (direct-stop finding, confirmed ACTIVE by reproduction). Direct (2) Verified on this head: focused battery 259 pass / 2142 expectations across 10 files; — |
|
Exact head
— |
The shared pet-policy test asserted the multiplexer warning variant, which only renders when TMUX/STY/ZELLIJ are set; CI runs without them and failed on the base variant. Assert the variant-agnostic prefix. Lore-id: pr4591-warning-env-agnostic Confidence: high Scope-risk: narrow Reversibility: simple Tested: bun test packages/coding-agent/test/interactive-mode-editor-component.test.ts under both host-TMUX and TMUX-less env (32 pass each) Tested: focused battery 259 pass / 2142 expectations across 10 files
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
New exact head — |
|
CI terminal classification for exact head Adversarial review is complete on this head: seven tracked findings plus six review-lane findings all proven fixed (probe drain, multiplexer Kitty matrix, stopped-TUI async writes, capability deadline ordering, ouroboros skin registry, bash/python drag ordering, withResolvers contract, revoke idempotence, marker-prefix fragmentation, shouldWrite throw classification, raster stop-epoch lifecycle incl. flush/afterPrefix/predicate-stop boundaries, Remaining blocker is exactly one: an effective APPROVED review at — |
|
Classification of the five current-head Codex comments (re-anchored re-posts of findings already fixed on this branch, verified against exact head — |
Yeachan-Heo
left a comment
Yeachan-Heo
left a comment
There was a problem hiding this comment.
MERGE_READY (owner classification, exact head 2b75098e, base 91a35114, digest sha256:56669e9a7b741947635f495009c2f93bc72d67d47b7be8908832095759154832 — independently recomputed from the GitHub head and matching the body verdict line).
Superseded-run classification: run 31910887721's single red (job 95075028753) was the interactive-mode-editor-component.test.ts shared-policy assertion, which had baked in the multiplexer warning variant while CI runs TMUX-less; fixed by 2b75098e (variant-agnostic prefix, verified under both host-TMUX and TMUX-less env). After rerun, 31910887721 is also all-green except bootstrap — not treated as final or as product failure. Authoritative replacement 31910918162 is terminal: every technical job green (affected plan, native-build, all test/check shards including the previously failing one, evidence producer, aggregate, state gates, virtual integration). The only red on both same-head runs is PR contract bootstrap = the intentional needs-human gate, with a current digest (no stale-digest diagnostic).
Review comments: every inline comment anchored at 2b75098e is a re-anchor of a comment originally filed at an older head (53b6b7a / 2d30ea2 / 1f76721 / 676000d); each was previously reproduced/classified and carries a fix + deterministic regression on this branch (multiplexer guard, stopped-TUI writes, drag ordering, revoke idempotence, stop-cleanup epoch, plus the later fragmentation/shouldWrite/warning fixes). No new findings exist at this head.
Outstanding blocker — exactly one: an effective APPROVED review at 2b75098e from a write-authorized non-author reviewer. @probepark (primary) and @HaD0Yun (backup) are formally requested and unresponded. Per the repository contract the verdict cannot be self-approved; on approval I will promote the single verdict line to merge-approved with this same digest, let bootstrap go green, and squash-merge to dev immediately.
—
[repo owner's gaebal-gajae (clawdbot) 🦞]
probepark
left a comment
probepark
left a comment
There was a problem hiding this comment.
Approve at 2b75098e7
fix(pet): stabilize iTerm2 Gajae Pet placement, drag handling plus a raster-lease boundary. 19
files, +5416 - the largest thing in this batch.
Differential
# base 91a35114d, with this head's test files applied
82 pass 41 fail
# head 2b75098e7
240 pass 0 fail
$ bun --cwd=packages/tui run check -> exit 0
$ bun --cwd=packages/coding-agent run check -> exit 0
41 failures on base against a suite that is fully green at head. For a change this size that ratio is
what I want to see - the tests are not decoration around a refactor, they are the specification of
new behaviour.
The raster-lease boundary is the substantive part
The pet placement is the visible feature; the lease lifecycle is where the risk lives. These
assertions cover it properly:
retains failed cleanup, blocks reacquire, then reports retry counts- a failed cleanup must not
silently free the lease. Blocking reacquire until recovery is what stops two owners drawing into
the same cells.retries two cleanup records independently and releases only recovered dependent FIFO work-
independent retry with dependent release. Coupling them would let one stuck record hold everything
behind it, or worse, release work whose dependency never recovered.erases raster leases before entering the manual history viewportandbefore revealing a manual transcript anchor- inline images that survive a viewport change are the classic terminal-graphics
artifact, and they persist across redraws.never writes queued raster work after stop and resumes cleanly after start- the stop/start
boundary, where queued work escaping after teardown corrupts whatever the terminal shows next.
The head commit is the right instinct
"make warning assertion independent of multiplexer env" - a TUI test that behaves differently under
tmux than in a bare terminal is one that fails for whoever happens to run it in the wrong place. Same
class as the platform-bound test I blocked in #4539, caught here without prompting.
merge-approved.
Reviewed by @probepark - method: fresh-worktree run and both package typechecks at the exact head, separate clean base worktree with the head's test files to prove the differential.
Yeachan-Heo
left a comment
Yeachan-Heo
left a comment
There was a problem hiding this comment.
Verdict promoted to merge-approved with the exact-head digest sha256:56669e9a7b741947635f495009c2f93bc72d67d47b7be8908832095759154832, backed by @probepark's APPROVED review at 2b75098e (write-authorized, non-author). The 04:03 PR-contract run raced the body edit against the approval event; this comment re-triggers the exact-head contract validation.
—
[repo owner's gaebal-gajae (clawdbot) 🦞]
|
Post-merge reconciliation complete. PR #4591 was squash-merged to dev as The merge surfaced three latent regressions that the PR's affected-path matrix had never executed, each reproduced, root-caused, fixed, and verified on dev (fix-forward commits, Final state: dev head — |
…rk failure When moveTreeNoReplace fails during managed fork artifact publication (e.g. an identity_mismatch at the rename boundary), the failed move has already disposed of the staging tree. The catch-block cleanup then calls removeTreeExpected on a staging directory that no longer exists, and the native exact-remove reports not_found — which the wrapper treated as an independently real cleanup failure, superseding the primary error with "Failed to clean up managed fork artifacts: not_found" (primary preserved only as cause). Per the precedent set by f6fb5c8, only a cleanup failure that can leave a live artifact behind may supersede the primary. A not_found on the staging tree is the strongest possible evidence that nothing is left to clean: the tree is absent. Treat it like cleanup_pending — authorized cleanup — so the primary failure reaches the caller verbatim. Reconstructed onto dev 52dad45 (first fully green canonical dev after the shared-shard repair: #4597 apply-patch marker fix, #4591/#4604 pet-tui cluster, #4595, #4598, #4600). Identical one-line delta as originally submitted at 87bee51. Lore-id: fork-staging-notfound-masking Constraint: only cleanup failures that can leave a live artifact behind may supersede the primary error Confidence: high Scope-risk: narrow Reversibility: trivial Tested: discrimination suite green at reconstructed head with red control on one-line revert; session-manager/ 307 pass / 0 fail; transition-seam + session-import 55/0; biome + tsc clean Not-tested: Windows exact-remove codes Supersedes: 3059adb
…rk failure (#4588) When moveTreeNoReplace fails during managed fork artifact publication (e.g. an identity_mismatch at the rename boundary), the failed move has already disposed of the staging tree. The catch-block cleanup then calls removeTreeExpected on a staging directory that no longer exists, and the native exact-remove reports not_found — which the wrapper treated as an independently real cleanup failure, superseding the primary error with "Failed to clean up managed fork artifacts: not_found" (primary preserved only as cause). Per the precedent set by f6fb5c8, only a cleanup failure that can leave a live artifact behind may supersede the primary. A not_found on the staging tree is the strongest possible evidence that nothing is left to clean: the tree is absent. Treat it like cleanup_pending — authorized cleanup — so the primary failure reaches the caller verbatim. Reconstructed onto dev 52dad45 (first fully green canonical dev after the shared-shard repair: #4597 apply-patch marker fix, #4591/#4604 pet-tui cluster, #4595, #4598, #4600). Identical one-line delta as originally submitted at 87bee51. Lore-id: fork-staging-notfound-masking Constraint: only cleanup failures that can leave a live artifact behind may supersede the primary error Confidence: high Scope-risk: narrow Reversibility: trivial Tested: discrimination suite green at reconstructed head with red control on one-line revert; session-manager/ 307 pass / 0 fail; transition-seam + session-import 55/0; biome + tsc clean Not-tested: Windows exact-remove codes Supersedes: 3059adb Co-authored-by: Yeachan Heo <yeachan.heo@gmail.com>
3 participants
What
Why
In iTerm2, the pet was vertically misaligned and undersized at non-default geometry. Dragging it inserted
/var/folders/.../iTerm2.…gajae-pet.*into the composer. During idle and work animation, per-frame direct writes raced with genericCSI 2Kredraws, causing repeated flicker and transient background-color frames.Review found six valid regressions in the first renderer port: probes could use shutdown-only input draining, forced Kitty could bypass multiplexer policy, stopped TUI sessions could reacquire raster leases, pre-start initialization could consume the iTerm capability warning deadline, non-crab skins could request crab-only frame names, and Bash/Python mode could bypass marked pet drag-path suppression.
Related issues/PRs: none found in the upstream open iTerm-pet issue/PR search.
Behavior and recovery
modifyOtherKeys, or detach the active input handler. Shutdown still uses the existing destructive drain.Testing
bun --cwd=packages/tui run checkbun --cwd=packages/coding-agent run checkbun test packages/tui/test/gajae-pet.test.ts packages/tui/test/raster-lease.test.ts packages/coding-agent/test/gajae-pet-widget.test.ts packages/coding-agent/test/input-controller-keybindings.test.ts packages/tui/test/render-commit.test.ts packages/tui/test/mouse-sgr.test.tsbun test packages/tui/test/terminal-probe-reply-leak.test.ts packages/tui/test/raster-lease.test.ts packages/tui/test/gajae-pet.test.ts— 65 passing tests, 569 expectations.bun test packages/coding-agent/test/modes/components/pet-capability.test.ts packages/coding-agent/test/gajae-pet-widget.test.ts— 68 passing tests, 220 expectations.bun test packages/coding-agent/test/gajae-pet-widget.test.ts packages/coding-agent/test/interactive-mode-editor-component.test.ts packages/tui/test/raster-lease.test.ts packages/tui/test/terminal-probe-reply-leak.test.ts— 135 passing tests, 590 expectations.bun test packages/coding-agent/test/modes/components/pet-capability.test.ts packages/tui/test/gajae-pet.test.ts packages/coding-agent/test/input-controller-keybindings.test.ts— 89 passing tests, 657 expectations.bun test packages/coding-agent/test/gajae-pet-widget.test.ts packages/coding-agent/test/input-controller-keybindings.test.ts— 121 passing tests, 1208 expectations.GJC verdict
devbun checkpasses (focused package checks were run; GitHub CI validates the final head)