Skip to content

fix(desktop): send correct terminal dimensions after attach for TUI apps - #3154

Merged
Kitenite merged 4 commits into
ParadevAI:mainfrom
Ipriyankrajai:fix/cc-width
Apr 3, 2026
Merged

Kitenite merged 4 commits into
ParadevAI:mainfrom
Ipriyankrajai:fix/cc-width

Conversation

@Ipriyankrajai

@Ipriyankrajai Ipriyankrajai commented Apr 3, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • When opening a terminal via preset (Ctrl+1), ink-based TUI apps like Claude Code rendered at ~1/3 width because the PTY was spawned with stale container dimensions
  • After a successful createOrAttach, schedules a requestAnimationFrame to re-fit xterm and send corrected dimensions to the PTY via SIGWINCH
  • The resize is only sent if dimensions actually changed, making it a no-op when the initial size was already correct
Screen.Recording.2026-04-04.at.12.51.04.AM.mov

Root Cause

fitAddon.fit() runs synchronously in createTerminalInstance before the container has its final layout. The ResizeObserver catches the correct size but is debounced at 150ms. Most CLIs handle the late SIGWINCH fine, but ink-based TUI apps (Claude Code) commit to the initial width during their first render cycle and don't fully reflow.

Test plan

  • Open a Claude Code preset via Ctrl+1 — verify it renders at full terminal width immediately
  • Open other CLI presets (Codex, OpenCode) — verify no regression
  • Resize the window while a terminal is active — verify resize still works correctly
  • Split panes and open presets — verify dimensions are correct in all panes
  • Reattach to an existing session — verify no double resize or flicker

Summary by CodeRabbit

  • Bug Fixes
    • Terminal sizing now waits for the interface layout to settle before finalizing dimensions, ensuring new or attached terminals render at the correct columns/rows and preventing misaligned or incorrectly sized terminal content.

When opening a terminal via preset (Ctrl+1), the PTY was spawned with
stale dimensions because fitAddon.fit() ran before the container had
its final layout. Most CLIs handle the late SIGWINCH from the debounced
ResizeObserver, but ink-based TUI apps like Claude Code commit to the
initial width during their first render cycle, resulting in a narrow UI.

After a successful createOrAttach, schedule a requestAnimationFrame to
re-fit and send corrected dimensions to the PTY. The resize is only
sent if dimensions actually changed, making it a no-op when the initial
size was already correct.
@coderabbitai

coderabbitai Bot commented Apr 3, 2026 •

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

Schedules a post-attachment requestAnimationFrame to run fitAddon.fit() after layout settles, verifies attachment is still active, compares pre- and post-fit xterm cols/rows, and calls resizeRef.current with updated dimensions if they changed.

Changes

Cohort / File(s) Summary
Terminal Lifecycle Enhancement
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/hooks/useTerminalLifecycle.ts
On successful attach, enqueues a requestAnimationFrame callback that captures pre-fit xterm cols/rows, performs fitAddon.fit(), re-checks dimensions, and invokes resizeRef.current(paneId, cols, rows) when post-fit sizes differ to sync PTY sizing.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 I hopped in quiet, timing on my mind,
A frame I waited for, then gentle fit I find.
I checked the rows and cols both before and after,
If sizes changed, I nudged the PTY with laughter.
Hooray — the terminal breathes, precise and kind!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: fixing terminal dimensions being sent to TUI apps after attach, which directly addresses the root cause of the issue described in the PR.
Description check ✅ Passed The pull request description includes all required sections from the template: Summary, Related Issues context, Type of Change (implicitly a bug fix), detailed Testing plan with checkboxes, and Additional Notes explaining the root cause and implementation.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@greptile-apps

greptile-apps Bot commented Apr 3, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a timing bug where ink-based TUI apps (e.g. Claude Code) rendered at the wrong terminal width after being opened via a preset shortcut. The root cause is that fitAddon.fit() runs synchronously during createTerminalInstance before the container has reached its final layout dimensions, while the 150ms-debounced ResizeObserver fires too late for ink's first render cycle.

The fix schedules a requestAnimationFrame inside the onSuccess callback of createOrAttach, re-running fitAddon.fit() once the browser has completed layout and sending SIGWINCH only when dimensions actually changed (making it a no-op when the initial size was already correct).

Key observations:

  • The isAttachActive() guard correctly prevents stale callbacks from firing after unmount or superseding attaches.
  • The local fitAddon closure variable is used (consistent with the rest of the lifecycle effect), not fitAddonRef.
  • No cancelAnimationFrame tracking is added for this new rAF, but this is consistent with scheduleScrollToBottom and is safe because isAttachActive() acts as the guard.
  • One P2 note: the rAF is scheduled unconditionally before the cold-restore early-return paths, meaning it fires and may dispatch a resize mutation even for sessions showing read-only restored scrollback. This is almost certainly a silent no-op in practice, but is semantically unexpected.
  • The restartTerminalSession path (non-mount restarts) does not get this fix; that appears intentional based on the PR scope.

Confidence Score: 4/5

  • Safe to merge; the fix is well-guarded and targeted — one minor P2 cleanup remains around cold-restore interaction.
  • The change is small (20 lines), correctly guarded against stale-attach and unmount races, and addresses a real and well-understood layout-timing bug. The only non-blocking concern is that the rAF fires unconditionally before cold-restore early-return paths, which may dispatch a spurious (likely no-op) resize mutation to a session with no active PTY. No logic or security issues found.
  • No files require special attention beyond the single changed hook.

Important Files Changed

Filename Overview
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/hooks/useTerminalLifecycle.ts Adds a requestAnimationFrame after successful createOrAttach to re-fit xterm and send corrected PTY dimensions via SIGWINCH; fix is well-guarded with isAttachActive() but the rAF fires even for cold-restore early-return paths, potentially sending a spurious resize to a session with no active PTY.

Sequence Diagram

sequenceDiagram
    participant React as React Mount
    participant TI as createTerminalInstance
    participant FA as fitAddon
    participant RO as ResizeObserver (150ms debounce)
    participant CA as createOrAttach (IPC)
    participant RAF as requestAnimationFrame (new)
    participant PTY as PTY / SIGWINCH

    React->>TI: mount effect
    TI->>FA: fitAddon.fit() [stale layout]
    TI->>CA: createOrAttach(cols=stale, rows=stale)
    Note over RO: Container reaches final layout
    RO-->>RO: debounce timer starts (150ms)
    CA-->>React: onSuccess
    React->>RAF: schedule rAF
    RAF->>FA: fitAddon.fit() [correct layout]
    FA-->>RAF: xterm.cols/rows updated
    RAF->>PTY: resizeRef (SIGWINCH) if dims changed
    Note over PTY: TUI app (ink/Claude Code) gets correct width
    RO-->>PTY: resizeRef (150ms later, same dims — no-op for TUI)
Loading

Reviews (1): Last reviewed commit: "fix(desktop): send correct terminal dime..." | Re-trigger Greptile

Comment on lines +565 to +577
requestAnimationFrame(() => {
if (!isAttachActive()) return;
const prevCols = xterm.cols;
const prevRows = xterm.rows;
fitAddon.fit();
if (xterm.cols !== prevCols || xterm.rows !== prevRows) {
resizeRef.current({
paneId,
cols: xterm.cols,
rows: xterm.rows,
});
}
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 rAF fires for cold-restore sessions too

The requestAnimationFrame is scheduled unconditionally before the cold-restore early-return paths (lines ~579–607). When storedColdRestore?.isRestored or result.isColdRestore is true, the function returns early but the queued rAF still fires — calling fitAddon.fit() and potentially dispatching resizeRef.current(...) against a PTY that is showing read-only restored scrollback and may have no active process behind it.

In most implementations resizeRef will silently no-op if there is no live PTY session, so this is unlikely to cause visible breakage. But it is semantically surprising and could produce a spurious resize mutation. Consider guarding the rAF so it is only scheduled for live (non-cold-restore) sessions, or at least documenting why it is safe to resize during a cold restore.

Suggested change
requestAnimationFrame(() => {
if (!isAttachActive()) return;
const prevCols = xterm.cols;
const prevRows = xterm.rows;
fitAddon.fit();
if (xterm.cols !== prevCols || xterm.rows !== prevRows) {
resizeRef.current({
paneId,
cols: xterm.cols,
rows: xterm.rows,
});
}
});
// After attach, the container may have finished layout
// at its final size while the PTY was spawned with stale
// dimensions (from before the container was fully laid out).
// Re-fit and send correct dimensions so TUI apps (e.g.
// Claude Code / ink) receive SIGWINCH before their initial
// render commits to the wrong width.
//
// Only schedule for live sessions; cold-restore paths below
// return early and have no active PTY to receive SIGWINCH.
const isColdRestoreSession =
coldRestoreState.get(paneId)?.isRestored ||
result.isColdRestore;
if (!isColdRestoreSession) {
requestAnimationFrame(() => {
if (!isAttachActive()) return;
const prevCols = xterm.cols;
const prevRows = xterm.rows;
fitAddon.fit();
if (xterm.cols !== prevCols || xterm.rows !== prevRows) {
resizeRef.current({
paneId,
cols: xterm.cols,
rows: xterm.rows,
});
}
});
}

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 1 file

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/hooks/useTerminalLifecycle.ts">

<violation number="1" location="apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/hooks/useTerminalLifecycle.ts:565">
P2: The `requestAnimationFrame` resize is scheduled unconditionally before the cold-restore early-return paths below. For cold-restore sessions (read-only scrollback, no active PTY process), the rAF still fires and may call `fitAddon.fit()` + `resizeRef.current(...)` against a session with no live process. Guard this so it only runs for live sessions, e.g. by checking `coldRestoreState.get(paneId)?.isRestored` before scheduling.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Removed outdated comments regarding terminal dimension handling after attach, as the logic has been updated to ensure correct dimensions are sent to TUI apps. This cleanup enhances code readability and maintains focus on the current implementation.
Reintroduced the requestAnimationFrame logic for updating terminal dimensions after attach, ensuring accurate dimensions are sent to TUI apps. This change enhances the responsiveness of terminal resizing and maintains consistency in the terminal's display behavior.
@Kitenite
Kitenite merged commit b8b11af into ParadevAI:main Apr 3, 2026
2 checks passed
MocA-Love pushed a commit to MocA-Love/superset that referenced this pull request Apr 5, 2026
…pps (ParadevAI#3154)

* fix(desktop): send correct terminal dimensions after attach for TUI apps

When opening a terminal via preset (Ctrl+1), the PTY was spawned with
stale dimensions because fitAddon.fit() ran before the container had
its final layout. Most CLIs handle the late SIGWINCH from the debounced
ResizeObserver, but ink-based TUI apps like Claude Code commit to the
initial width during their first render cycle, resulting in a narrow UI.

After a successful createOrAttach, schedule a requestAnimationFrame to
re-fit and send corrected dimensions to the PTY. The resize is only
sent if dimensions actually changed, making it a no-op when the initial
size was already correct.

* refactor(terminal): remove stale dimension handling comments

Removed outdated comments regarding terminal dimension handling after attach, as the logic has been updated to ensure correct dimensions are sent to TUI apps. This cleanup enhances code readability and maintains focus on the current implementation.

* refactor(terminal): streamline terminal dimension handling logic

Reintroduced the requestAnimationFrame logic for updating terminal dimensions after attach, ensuring accurate dimensions are sent to TUI apps. This change enhances the responsiveness of terminal resizing and maintains consistency in the terminal's display behavior.
MocA-Love added a commit to MocA-Love/superset that referenced this pull request Apr 5, 2026
cherry-pick方式で内容を取り込み済みの14コミットをgit履歴上もマージ済みにする。

取り込み済み (cherry-pick / 手動移植):
- be22b46 ParadevAI#3125 — スキップ (下記参照)
- 88bc7fb ParadevAI#3127 — Revert DA1 ✓
- 92d0ff9 ParadevAI#3054 — DA1 fix ✓
- c48450e ParadevAI#3093 — file viewer pane fix ✓
- fffa8db ParadevAI#3128 — version 1.4.7 ✓
- 589a7c7 ParadevAI#3136 — fuzzy scorer (ハイブリッド方式) ✓
- ceb8c81 ParadevAI#3150 — Electron 40.8.5 ✓
- 8922b94 ParadevAI#3137 — terminalId分離 ✓
- c7508e5 ParadevAI#3152 — GitHub無料化 ✓
- 2b91f11 ParadevAI#3155 — v2 terminal theme ✓
- b8b11af ParadevAI#3154 — TUI dimension fix ✓
- 7599ace ParadevAI#3149 — v2 sidebar file tree (手動統合) ✓
- 4d7c612 ParadevAI#3174 — DnD重複削除 ✓
- 864977d ParadevAI#3157 — Host Service分離 ✓

意図的にスキップ:
- be22b46 ParadevAI#3125 (GitHub polling簡素化)
  フォーク独自のGitHubSyncService (バックエンド集中ポーリング) と
  設計が異なるため不採用。upstreamはフロントエンドhover駆動、フォークは
  バックエンドキャッシュウォーマー方式。詳細は githubQueryPolicy.ts と
  github-sync-service.ts のFORK NOTEを参照。

ゴースト・マージ復元 (revert 134cfd5 で消失した内容):
- 538f306 ParadevAI#3120 — Patch vuln ✓
- 1588d20 ParadevAI#3108 — terminal lifecycle分離 ✓
- 59426f6 ParadevAI#3122 — file tree + FilePane + Alert refactor (手動統合) ✓
- 10d9a5d ParadevAI#3097 — tiptap line-height ✓
- 337a9ae ParadevAI#3121 — Codex hooks削除 ✓
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants