Skip to content

fix: unsynced line buffer on backspace or on keyboard type #645

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

Open
zhravan wants to merge 2 commits into
base: master
Choose a base branch
from
+50 −14
Open

fix: unsynced line buffer on backspace or on keyboard type #645

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
77cb02a
fix: unsynced line buffer on backspace or on keyboard type
zhravan Dec 7, 2025
d51faf7
fix: unsynced line buffer on backspace or on keyboard type
zhravan Dec 7, 2025
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
2 changes: 1 addition & 1 deletion api/api/versions.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
{
"version": "v1",
"status": "active",
"release_date": "2025-12-06T19:40:52.670434+05:30",
"release_date": "2025-12-07T12:32:26.918875+05:30",
"end_of_life": "0001-01-01T00:00:00Z",
"changes": [
"Initial API version"
Expand Down
6 changes: 0 additions & 6 deletions api/internal/features/terminal/init.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,12 +161,6 @@ func (t *Terminal) readOutput(r io.Reader) {
return
}

func() {
t.wsLock.Lock()
defer t.wsLock.Unlock()
t.outputBuf = append(t.outputBuf, buf[:n]...)
}()

msg := TerminalMessage{
TerminalId: t.TerminalId,
Type: "stdout",
Expand Down
12 changes: 11 additions & 1 deletion view/app/terminal/terminal.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,15 +63,25 @@ const TerminalSession: React.FC<{
useEffect(() => {
if (isTerminalOpen && isActive && isContainerReady) {
initializeTerminal();
} else {
// Cleanup: destroy terminal when it's closed or becomes inactive
destroyTerminal();
}
}, [isTerminalOpen, isActive, isContainerReady, initializeTerminal]);
}, [isTerminalOpen, isActive, isContainerReady, initializeTerminal, destroyTerminal]);

useEffect(() => {
if (fitAddonRef) {
setFitAddonRef(fitAddonRef);
}
}, [fitAddonRef, setFitAddonRef]);

// Cleanup on unmount
useEffect(() => {
return () => {
destroyTerminal();
};
}, [destroyTerminal]);

return (
<div
ref={terminalRef}
Expand Down
44 changes: 38 additions & 6 deletions view/app/terminal/utils/useTerminal.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,23 @@ export const useTerminal = (
const { sendJsonMessage, message, isReady } = useWebSocket();
const [terminalInstance, setTerminalInstance] = useState<any | null>(null);
const resizeTimeoutRef = useRef<NodeJS.Timeout | undefined>(undefined);
const terminalInstanceRef = useRef<any | null>(null);

const destroyTerminal = useCallback(() => {
if (terminalInstance) {
terminalInstance.dispose();
const instance = terminalInstanceRef.current;
if (instance) {
instance.dispose();
terminalInstanceRef.current = null;
setTerminalInstance(null);
}
// Clear the terminal container to remove any stale input
if (terminalRef.current) {
terminalRef.current.innerHTML = '';
}
if (resizeTimeoutRef.current) {
clearTimeout(resizeTimeoutRef.current);
}
}, [terminalInstance, terminalId]);
}, []);

useEffect(() => {
if (isStopped && terminalInstance) {
Expand Down Expand Up @@ -70,6 +77,8 @@ export const useTerminal = (
if (parsedMessage.type === OutputType.EXIT) {
destroyTerminal();
} else if (parsedMessage.data) {
// Write output from backend - this includes echoed input
// Using write ensures proper synchronization with terminal state
terminalInstance.write(parsedMessage.data);
}
}
Expand All @@ -78,6 +87,19 @@ export const useTerminal = (
}
}, [message, terminalInstance, destroyTerminal, terminalId]);

// Cleanup effect: destroy terminal when isTerminalOpen becomes false or component unmounts
useEffect(() => {
if (!isTerminalOpen && terminalInstanceRef.current) {
destroyTerminal();
}
return () => {
// Cleanup on unmount
if (terminalInstanceRef.current) {
destroyTerminal();
}
};
}, [isTerminalOpen, destroyTerminal]);

const initializeTerminal = useCallback(async () => {
if (!terminalRef.current || terminalInstance || !isReady) return;

Expand Down Expand Up @@ -158,26 +180,35 @@ export const useTerminal = (
if (allowInput) {
term.attachCustomKeyEventHandler((event: KeyboardEvent) => {
const key = event.key.toLowerCase();

// Handle Ctrl+J or Cmd+J (toggle terminal shortcut)
if (key === 'j' && (event.ctrlKey || event.metaKey)) {
return false;
} else if (key === 'c' && (event.ctrlKey || event.metaKey) && !event.shiftKey) {
}

// Handle Ctrl+C or Cmd+C for copy (when there's a selection)
if (key === 'c' && (event.ctrlKey || event.metaKey) && !event.shiftKey) {
if (event.type === 'keydown') {
try {
const selection = term.getSelection();
if (selection) {
navigator.clipboard.writeText(selection).then(() => {
term.clearSelection(); // Clear selection after successful copy
term.clearSelection();
});
return false;
}
} catch (error) {
console.error('Error in Ctrl+C handler:', error);
}
}
return false;
}
Comment on lines +189 to 204
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 7, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Add error handling for clipboard write failure.

The navigator.clipboard.writeText() promise rejection is not caught. If clipboard access fails (e.g., due to permissions), the error will be silently swallowed and clearSelection() won't be called.

                  if (selection) {
-                   navigator.clipboard.writeText(selection).then(() => {
-                     term.clearSelection();
-                   });
+                   navigator.clipboard.writeText(selection)
+                     .then(() => {
+                       term.clearSelection();
+                     })
+                     .catch((err) => {
+                       console.error('Failed to copy to clipboard:', err);
+                     });
                    return false;
                  }
🤖 Prompt for AI Agents 
In view/app/terminal/utils/useTerminal.ts around lines 189 to 204, the clipboard
write call uses navigator.clipboard.writeText(selection) without handling
promise rejections so failures will be silent and term.clearSelection() may not
run; wrap the clipboard write in a try/catch (or attach .catch) and ensure
term.clearSelection() runs in a finally block (or after both success and
failure) and still return false after handling the selection so the key event is
swallowed.


// Allow xterm to process all other keys normally
return true;
});

// onData is called when xterm processes input
// Send all input to backend - backend echo will handle display
term.onData((data) => {
sendJsonMessage({
action: 'terminal',
Expand All @@ -198,6 +229,7 @@ export const useTerminal = (
});
}

terminalInstanceRef.current = term;
setTerminalInstance(term);
} catch (error) {
console.error('Error initializing terminal:', error);
Expand Down
Loading