Skip to content

feat(desktop): restart daemon from terminal-unreachable toast #4962

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
Kitenite wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(desktop): restart daemon from terminal-unreachable toast #4962

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
43 changes: 41 additions & 2 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
@@ -1,3 +1,4 @@
import { toast } from "@superset/ui/sonner";
import type { FitAddon } from "@xterm/addon-fit";
import type { SearchAddon } from "@xterm/addon-search";
import type { Terminal as XTerm } from "@xterm/xterm";
Expand Down Expand Up @@ -250,6 +251,25 @@ export const Terminal = memo(function Terminal({
// Auto-retry connection with exponential backoff
const retryCountRef = useRef(0);
const MAX_RETRIES = 5;
const daemonFailureToastShownRef = useRef(false);

const restartDaemonMutation = electronTrpc.terminal.restartDaemon.useMutation(
{
onSuccess: () => {
toast.success("Daemon restarted", {
description:
"All sessions killed and daemon restarted. The app will use a fresh daemon.",
});
},
onError: (error) => {
toast.error("Failed to restart daemon", {
description: error.message,
});
},
},
);
const restartDaemonMutationRef = useRef(restartDaemonMutation);
restartDaemonMutationRef.current = restartDaemonMutation;

// Stream handling
const { handleTerminalExit, handleStreamError, handleStreamData } =
Expand All @@ -272,9 +292,28 @@ export const Terminal = memo(function Terminal({

// Auto-retry when connection error is set
useEffect(() => {
if (!connectionError) return;
if (!connectionError) {
daemonFailureToastShownRef.current = false;
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot May 28, 2026

Choose a reason for hiding this comment

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

P2: The persistent "Terminal daemon unreachable" toast is never dismissed on reconnect, so a stale error can remain visible after recovery.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx, line 296:

<comment>The persistent "Terminal daemon unreachable" toast is never dismissed on reconnect, so a stale error can remain visible after recovery.</comment>

<file context>
@@ -272,9 +292,28 @@ export const Terminal = memo(function Terminal({
 	useEffect(() => {
-		if (!connectionError) return;
+		if (!connectionError) {
+			daemonFailureToastShownRef.current = false;
+			return;
+		}
</file context>

return;
}
if (isExitedRef.current) return;
if (retryCountRef.current >= MAX_RETRIES) return;
if (retryCountRef.current >= MAX_RETRIES) {
if (!daemonFailureToastShownRef.current) {
daemonFailureToastShownRef.current = true;
toast.error("Terminal daemon unreachable", {
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot May 28, 2026

Choose a reason for hiding this comment

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

P2: Multiple terminal panes will each show their own "Terminal daemon unreachable" toast since daemonFailureToastShownRef is per-instance and no shared id is passed to toast.error(). Use a module-level constant toast ID (e.g., const DAEMON_UNREACHABLE_TOAST_ID = "daemon-unreachable") so Sonner deduplicates rather than stacking identical toasts.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx, line 303:

<comment>Multiple terminal panes will each show their own "Terminal daemon unreachable" toast since `daemonFailureToastShownRef` is per-instance and no shared `id` is passed to `toast.error()`. Use a module-level constant toast ID (e.g., `const DAEMON_UNREACHABLE_TOAST_ID = "daemon-unreachable"`) so Sonner deduplicates rather than stacking identical toasts.</comment>

<file context>
@@ -272,9 +292,28 @@ export const Terminal = memo(function Terminal({
+		if (retryCountRef.current >= MAX_RETRIES) {
+			if (!daemonFailureToastShownRef.current) {
+				daemonFailureToastShownRef.current = true;
+				toast.error("Terminal daemon unreachable", {
+					description:
+						"Couldn't reconnect after several attempts. Restart the daemon to recover.",
</file context>
Suggested change
toast.error("Terminal daemon unreachable", {
toast.error("Terminal daemon unreachable", {
id: "daemon-unreachable",

description:
"Couldn't reconnect after several attempts. Restart the daemon to recover.",
duration: Number.POSITIVE_INFINITY,
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot May 28, 2026

Choose a reason for hiding this comment

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

P2: The infinite-duration toast is never dismissed when this component unmounts (e.g., user closes the terminal pane). The toast remains visible and functional with a stale ref. Assign a stable id to the toast and dismiss it in a useEffect cleanup to prevent interaction with an unmounted component's state.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx, line 306:

<comment>The infinite-duration toast is never dismissed when this component unmounts (e.g., user closes the terminal pane). The toast remains visible and functional with a stale ref. Assign a stable `id` to the toast and dismiss it in a `useEffect` cleanup to prevent interaction with an unmounted component's state.</comment>

<file context>
@@ -272,9 +292,28 @@ export const Terminal = memo(function Terminal({
+				toast.error("Terminal daemon unreachable", {
+					description:
+						"Couldn't reconnect after several attempts. Restart the daemon to recover.",
+					duration: Number.POSITIVE_INFINITY,
+					action: {
+						label: "Restart daemon",
</file context>

action: {
label: "Restart daemon",
onClick: () => {
restartDaemonMutationRef.current.mutate();
},
Comment on lines +309 to +311
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 28, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Guard against concurrent mutation calls.

The action button can be clicked multiple times before the mutation completes, potentially triggering concurrent daemon restarts. Add an isPending check to prevent this.

🛡️ Proposed fix to add pending state guard 
 					action: {
 						label: "Restart daemon",
 						onClick: () => {
+							if (!restartDaemonMutationRef.current.isPending) {
 								restartDaemonMutationRef.current.mutate();
+							}
 						},
 					},
📝 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
onClick: () => {
restartDaemonMutationRef.current.mutate();
},
onClick: () => {
if (!restartDaemonMutationRef.current.isPending) {
restartDaemonMutationRef.current.mutate();
}
},
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx`
around lines 309 - 311, Guard the mutation call in the onClick handler by
checking the mutation's pending/loading state before invoking mutate: in
Terminal.tsx locate the onClick that calls
restartDaemonMutationRef.current.mutate() and change it to first read the ref
(e.g. const m = restartDaemonMutationRef.current) and return early if
m?.isPending || m?.isLoading (or the appropriate flag your mutation exposes);
only call m.mutate() (or m.mutateAsync()) when not pending. This prevents
concurrent restarts while keeping the existing restartDaemonMutationRef and
mutate call intact.

},
});
Comment on lines +303 to +313
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot May 28, 2026

Choose a reason for hiding this comment

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

P2 Multiple failing panes accumulate persistent toasts

Because each Terminal instance manages its own daemonFailureToastShownRef and shows the toast independently, opening 3 terminal panes when the daemon is unreachable results in 3 stacked "Terminal daemon unreachable" toasts, each with its own "Restart daemon" button. Since this is a daemon-level (not pane-level) failure, a shared toast ID (e.g., a module-level constant like DAEMON_TOAST_ID) would allow Sonner to deduplicate updates to the same toast rather than stacking them. This mirrors how useUpdateListener uses UPDATE_TOAST_ID to prevent duplicate update toasts.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
Line: 303-313

Comment:
**Multiple failing panes accumulate persistent toasts**

Because each `Terminal` instance manages its own `daemonFailureToastShownRef` and shows the toast independently, opening 3 terminal panes when the daemon is unreachable results in 3 stacked "Terminal daemon unreachable" toasts, each with its own "Restart daemon" button. Since this is a daemon-level (not pane-level) failure, a shared toast ID (e.g., a module-level constant like `DAEMON_TOAST_ID`) would allow Sonner to deduplicate updates to the same toast rather than stacking them. This mirrors how `useUpdateListener` uses `UPDATE_TOAST_ID` to prevent duplicate update toasts.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

}
return;
Comment on lines 299 to +315
Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot May 28, 2026

Choose a reason for hiding this comment

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

P1 Persistent toast not dismissed on component unmount

The toast is created with duration: Number.POSITIVE_INFINITY but no cleanup runs when this Terminal instance unmounts (e.g., user closes the pane). The toast stays visible and functional indefinitely. If the user then clicks "Restart daemon," the mutation fires from an unmounted component — the daemon restarts, but onSuccess/onError can run against a detached component lifecycle. The existing useUpdateListener in this codebase shows the correct pattern: assign a stable id to the toast and call toast.dismiss(id) in a useEffect cleanup. For example, store the returned toast ID in a ref (const toastIdRef = useRef<string | number | null>(null)) and add a useEffect(() => () => { if (toastIdRef.current != null) toast.dismiss(toastIdRef.current); }, []) to guarantee cleanup on unmount.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/Terminal.tsx
Line: 299-315

Comment:
**Persistent toast not dismissed on component unmount**

The toast is created with `duration: Number.POSITIVE_INFINITY` but no cleanup runs when this `Terminal` instance unmounts (e.g., user closes the pane). The toast stays visible and functional indefinitely. If the user then clicks "Restart daemon," the mutation fires from an unmounted component — the daemon restarts, but `onSuccess`/`onError` can run against a detached component lifecycle. The existing `useUpdateListener` in this codebase shows the correct pattern: assign a stable `id` to the toast and call `toast.dismiss(id)` in a `useEffect` cleanup. For example, store the returned toast ID in a ref (`const toastIdRef = useRef<string | number | null>(null)`) and add a `useEffect(() => () => { if (toastIdRef.current != null) toast.dismiss(toastIdRef.current); }, [])` to guarantee cleanup on unmount.

How can I resolve this? If you propose a fix, please make it concise.

}

if (retryCountRef.current === 0) {
xtermRef.current?.writeln(
Expand Down
Loading