fix(terminal): emit DEC 2048 in-band size reports#311
Merged
Conversation
forketyfork
force-pushed
the
fix/agent-scoped-resize-holds
branch
from
6262be5 to
75fe4dd
Compare
forketyfork
changed the title
![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: 75fe4ddfd2
ℹ️ 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.
Pull request overview
This PR narrows terminal resize and synchronized-output “output hold” behavior so stale-texture cache reuse only applies to AI-agent sessions (Codex/Claude/Gemini), restoring live repaint behavior for normal TUIs (e.g., nvim) during SIGWINCH resizes.
Changes:
- Adds agent ownership tracking for synchronized-output and terminal-resize holds, and only treats holds as active when an agent is associated.
- Delays PTY/VT resize by one frame after agent grid→full expansion so a stable full-mode cache frame exists for the hold to display.
- Updates terminal resize path to start resize holds only when an agent session is detected; updates docs and tests accordingly.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/session/state.zig |
Tracks which agent owns output holds; gates hold activation/expiry logic on agent ownership; adds detection helper + tests. |
src/app/runtime.zig |
Adds one-frame delay of terminal layout resize after agent expansion to populate a full-mode cache frame first; adds test. |
src/app/layout.zig |
Starts terminal resize holds only when detectOutputHoldAgent() identifies an agent session. |
docs/ARCHITECTURE.md |
Documents agent-scoped holds and the pre-resize full-frame behavior for agent expansions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
forketyfork
force-pushed
the
fix/agent-scoped-resize-holds
branch
5 times, most recently
from
8b8fae1 to
836189a
Compare
forketyfork
force-pushed
the
fix/agent-scoped-resize-holds
branch
from
836189a to
390f2f2
Compare
Issue: After Architect started answering DECRQM mode queries (commit bde3b89), apps that opt into DEC mode 2048 stopped reflowing on grid/full view toggle and window resize. nvim is the most visible case: it treats a positive 2048 response as a contract that the terminal will send `CSI 48 ; rows ; cols ; px_h ; px_w t` whenever geometry changes. Architect responded to the query but never emitted the reports, so those apps deferred to an in-band channel that never arrived. Solution: Emit the mode 2048 size report when the app first enables the mode and after every PTY/VT resize, matching ghostty's sizeReportLocked. With the root cause addressed, the layered workarounds added earlier on this branch are no longer needed and are removed: manual kill(-pgrp, SIGWINCH), the focus-in / second SIGWINCH / Ctrl-L nudge ladder, slave-side TIOCSWINSZ fallback, agent-scoped output holds, the agent-frame-delay in animation completion, the held-texture path in the renderer, and the resize-trace diagnostics. The spawn path is aligned with ghostty: dup2 before childPreExec, close master/slave fds inside childPreExec after TIOCSCTTY, no SIGWINCH disposition reset.
forketyfork
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ee3337329b
ℹ️ 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".
6 tasks
Issue: Four unresolved review threads on PR #311 flagged genuine problems: a forked child branch that could unwind back to the parent on childPreExec error, duplicate logging of resize failures, missing DEC 2048 reports on pixel-only resizes, and a foreground-pgrp lookup duplicated between detectForegroundAgent and foregroundPgrp. Solution: In the child branch of Shell.spawn, swap the bare `try` for `catch std.c._exit(1)` so a childPreExec error terminates the child instead of returning to the runtime. In applyTerminalResize, drop the std.debug.print duplicates and route failures through log.warn only; compare the full winsize struct so the DEC 2048 in-band report fires on pixel-only resizes too while the VT-resize stays gated on cell count. detectForegroundAgent now calls foregroundPgrp for the sysctl/tcgetpgrp fallback, eliminating the duplicated lookup. Two review threads about agent-scoped output hold staleness are left as won't-fix in this PR because the entire output-hold mechanism is removed in the follow-up.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Solution
After Architect started answering DECRQM mode queries (commit
bde3b89), apps that opt into DEC mode 2048 in-band size reports stopped reflowing on grid/full view toggle and window resize. nvim is the case this was diagnosed against: it sees Architect's positive 2048 response, enables the mode, and then waits forCSI 48 ; rows ; cols ; px_h ; px_w treports that the terminal never emits. SIGWINCH still fires; nvim's TUI layer defers to the in-band channel that Architect was silently failing to honor.This PR fixes the root cause by emitting the mode 2048 report when the app enables the mode and after every PTY/VT resize, matching ghostty's
src/termio/Termio.zig:sizeReportLockedmode_2048 branch.With that addressed, several layers of resize-related code added earlier on this branch are no longer needed:
kill(-pgrp, SIGWINCH)afterTIOCSWINSZ, and the focus-in / second SIGWINCH / Ctrl-L nudge ladder for vim-family processesTIOCSWINSZfallback andttynametracking inpty.zigdetectOutputHoldAgentExpanding/Collapsinganimation completionoutputHoldActive/shouldHoldSessionCacheRefresh/renderHeldSessionTexturein the rendererpty.zig/session/state.zig/app/layout.zig/app/runtime.zigThe PTY layer is now ghostty-shaped:
pty.setSizeis a singleioctl(master, TIOCSWINSZ),dup2runs beforechildPreExec,childPreExeccloses the original master and slave fds afterTIOCSCTTY, andchildPreExecno longer touches the SIGWINCH disposition.expireSynchronizedOutputis kept (without agent scoping) as a stuck-mode safety net for apps that enter synchronized output and never end it.Net: +113 / −284 vs main.
Test plan
nvim --clean somefile.txtand repeat the toggle. Behavior matches the regular config.