Skip to content

fix(terminal): sync PTY dimensions after restore to prevent autocomplete issues #909

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Kitenite merged 1 commit into from
Jan 23, 2026
Merged

fix(terminal): sync PTY dimensions after restore to prevent autocomplete issues #909

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ...derer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ export const Terminal = ({ tabId, workspaceId }: TerminalProps) => {
onErrorEvent: (event, xterm) => handleStreamErrorRef.current(event, xterm),
onDisconnectEvent: (reason) =>
setConnectionError(reason || "Connection to terminal daemon lost"),
onResize: (cols, rows) => resizeRef.current({ paneId, cols, rows }),
});

// Cold restore handling
Expand Down
19 changes: 18 additions & 1 deletion ...ain/components/WorkspaceView/ContentView/TabsContent/Terminal/hooks/useTerminalRestore.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ export interface UseTerminalRestoreOptions {
xterm: XTerm,
) => void;
onDisconnectEvent: (reason: string | undefined) => void;
/** Callback to send resize to PTY after fit() - ensures PTY dimensions match xterm */
onResize: (cols: number, rows: number) => void;
Comment on lines +24 to +25
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Jan 22, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, verify the file exists and examine its content
cat -n apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/hooks/useTerminalRestore.ts

Repository: superset-sh/superset

Length of output: 10639

🏁 Script executed:

# Search for all onResize usages across the codebase
rg -n 'onResize' --type ts --type tsx -B 2 -A 2

Repository: superset-sh/superset

Length of output: 91

🏁 Script executed:

# Search for all onResize usages with corrected rg syntax
rg 'onResize' --type ts -B 2 -A 2

Repository: superset-sh/superset

Length of output: 10561

🏁 Script executed:

# Search for useTerminalRestore usage to find where onResize callback is provided
rg 'useTerminalRestore' --type ts -B 2 -A 5

Repository: superset-sh/superset

Length of output: 4728

🏁 Script executed:

# Get context around Terminal.tsx onResize usage to see full implementation
rg -n 'onResize:' apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx -B 5 -A 1

Repository: superset-sh/superset

Length of output: 436

🏁 Script executed:

# Get helpers.ts function signature for onResize to understand full context
cat apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/helpers.ts | head -50

Repository: superset-sh/superset

Length of output: 2235

Use a params object for onResize to follow the object-params guideline.

The callback uses positional parameters (cols: number, rows: number) which conflicts with the coding guideline requiring object parameters for functions with 2+ parameters. Refactor to use { cols, rows } for clarity and consistency.

This requires coordinated updates across the Terminal module:

  • Interface definition in useTerminalRestore.ts (line 25)
  • Invocation in useTerminalRestore.ts (line 130)
  • Provider callback in Terminal.tsx (line 192)
  • Function signature and call site in helpers.ts
♻️ Proposed refactor 
-	onResize: (cols: number, rows: number) => void;
+	onResize: (params: { cols: number; rows: number }) => void;
-	onResizeRef.current(xterm.cols, xterm.rows);
+	onResizeRef.current({ cols: xterm.cols, rows: xterm.rows });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/** Callback to send resize to PTY after fit() - ensures PTY dimensions match xterm */
onResize: (cols: number, rows: number) => void;
/** Callback to send resize to PTY after fit() - ensures PTY dimensions match xterm */
onResize: (params: { cols: number; rows: number }) => void;
🤖 Prompt for AI Agents 
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/hooks/useTerminalRestore.ts`
around lines 24 - 25, The onResize callback currently uses positional parameters
(cols: number, rows: number); change its signature to accept a single params
object { cols: number; rows: number } and update all call sites accordingly:
update the type/definition in useTerminalRestore.ts (onResize), change the
invocation inside useTerminalRestore (where onResize(...) is called) to pass an
object { cols, rows }, update the provider callback prop passed from
Terminal.tsx (where you supply onResize) to accept and forward an object, and
update the helper in helpers.ts (both its function signature and any internal
calls) to use and propagate { cols, rows } instead of positional args so all
usages conform to the object-params guideline.

}

export interface UseTerminalRestoreReturn {
Expand Down Expand Up @@ -54,6 +56,7 @@ export function useTerminalRestore({
onExitEvent,
onErrorEvent,
onDisconnectEvent,
onResize,
}: UseTerminalRestoreOptions): UseTerminalRestoreReturn {
// Gate streaming until initial state restoration is applied
const isStreamReadyRef = useRef(false);
Expand All @@ -73,6 +76,8 @@ export function useTerminalRestore({
onErrorEventRef.current = onErrorEvent;
const onDisconnectEventRef = useRef(onDisconnectEvent);
onDisconnectEventRef.current = onDisconnectEvent;
const onResizeRef = useRef(onResize);
onResizeRef.current = onResize;

const flushPendingEvents = useCallback(() => {
const xterm = xtermRef.current;
Expand Down Expand Up @@ -119,7 +124,19 @@ export function useTerminalRestore({
if (xtermRef.current !== xterm) return;
if (restoreSequenceRef.current !== restoreSequence) return;
fitAddon.fit();
scrollToBottom(xterm);
// Send resize to PTY after fit() to ensure dimensions are synced.
// This fixes the race condition where createOrAttach uses stale dimensions
// from before the container was fully laid out.
onResizeRef.current(xterm.cols, xterm.rows);
// Only scroll to bottom for NEW sessions. For reattached sessions,
// the snapshot already positions the viewport correctly and we should
// not override the user's scroll position.
if (result.isNew) {
// Write empty string with callback to ensure all pending writes are
// processed before scrolling. xterm.write() is async and buffers writes,
// so scrollToBottom() called immediately might not see all content.
xterm.write("", () => scrollToBottom(xterm));
}
});
};

Expand Down
Loading