Skip to content

fix: solve #2570 — restart terminal-host daemon on app startup to fix stale macOS security session#2571

Closed
github-actions[bot] wants to merge 1 commit intomainfrom
triage/issue-2570-23235014531
Closed

fix: solve #2570 — restart terminal-host daemon on app startup to fix stale macOS security session#2571
github-actions[bot] wants to merge 1 commit intomainfrom
triage/issue-2570-23235014531

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot commented Mar 18, 2026

Summary

On macOS with Fast User Switching, the terminal-host daemon can be spawned while the user is a background user, inheriting a degraded security context (no Keychain/Security.framework access). When the user becomes the console owner and relaunches Superset, the app reuses the stale daemon via its live socket, causing all Go binaries (gh, etc.) and security CLI calls to fail with:

tls: failed to verify certificate: x509: OSStatus -26276
security: SecKeychainCopySearchList: One or more parameters passed to a function were not valid.

Root cause

TerminalHostClient.spawnDaemon() checks if the socket is live and reuses the existing daemon process. The daemon was designed to persist across app restarts for session continuity. However, this means a daemon spawned under a stale macOS security session (from Fast User Switching) is never replaced.

Fix

  • Added restartDaemon() method to TerminalHostClient that gracefully shuts down the existing daemon (or falls back to SIGTERM via PID file) and spawns a fresh one
  • DaemonTerminalManager.reconcileOnStartup() now calls restartDaemon() before listing sessions, ensuring every app launch gets a daemon with the current user's security session context
  • Terminal sessions from the old daemon are recovered via the existing cold restore mechanism (scrollback from disk history)

Tests

  • client.test.ts — 3 new tests for restartDaemon():
    • Graceful shutdown of running daemon + stale file cleanup + fresh spawn
    • Fallback to SIGTERM when socket is not connectable
    • Fresh spawn when no existing daemon is running
  • daemon-manager.test.ts — 1 new test verifying reconcileOnStartup() calls restartDaemon()
  • All 40 existing terminal-host tests continue to pass

Closes #2570


Summary by cubic

Always restart the terminal-host daemon on app startup to avoid reusing a daemon launched under a stale macOS security session after Fast User Switching. This fixes TLS/Keychain failures seen by Go CLIs like gh and security commands, with session scrollback recovered via cold restore.

  • Bug Fixes
    • Added TerminalHostClient.restartDaemon() with graceful shutdown, SIGTERM fallback via PID, stale file cleanup, and fresh spawn + reconnect.
    • DaemonTerminalManager.reconcileOnStartup() now calls restartDaemon() before listing sessions so the daemon inherits the current user’s security context.

Written for commit ea15fd7. Summary will update on new commits.

…e macOS security session

On macOS with Fast User Switching, the terminal-host daemon can be spawned
while the user is a background user, inheriting a degraded security context.
When the user becomes the console owner and relaunches Superset, the app
reuses the stale daemon, causing all Go binaries (gh, etc.) and Security.framework
calls to fail with TLS/Keychain errors.

Fix: always restart the daemon on app startup via reconcileOnStartup() so it
inherits the current user's security session. The cold restore mechanism
recovers terminal scrollback from disk history.

Closes #2570
Copy link
Copy Markdown
Contributor

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

Choose a reason for hiding this comment

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

3 issues found across 4 files

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/main/lib/terminal/daemon/daemon-manager.ts">

<violation number="1" location="apps/desktop/src/main/lib/terminal/daemon/daemon-manager.ts:86">
P2: This restart is unconditional even though the stale security-session bug is macOS-specific. Gate it to darwin so Windows/Linux keep their existing daemon lifecycle.

(Based on your team's feedback about gating platform-specific fixes with runtime checks.) [FEEDBACK_USED]</violation>
</file>

<file name="apps/desktop/src/main/lib/terminal-host/client.ts">

<violation number="1" location="apps/desktop/src/main/lib/terminal-host/client.ts:1452">
P1: Avoid PID-file SIGTERM when no daemon socket exists; stale PID files can kill unrelated processes.</violation>

<violation number="2" location="apps/desktop/src/main/lib/terminal-host/client.ts:1456">
P1: Check that shutdown actually completed before deleting daemon files and reconnecting, otherwise restart can leave two daemons running.</violation>
</file>

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

this.killDaemonFromPidFile();
}

await this.waitForDaemonShutdown();
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Mar 18, 2026

Choose a reason for hiding this comment

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

P1: Check that shutdown actually completed before deleting daemon files and reconnecting, otherwise restart can leave two daemons running.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/main/lib/terminal-host/client.ts, line 1456:

<comment>Check that shutdown actually completed before deleting daemon files and reconnecting, otherwise restart can leave two daemons running.</comment>

<file context>
@@ -1411,6 +1411,65 @@ export class TerminalHostClient extends EventEmitter {
+			this.killDaemonFromPidFile();
+		}
+
+		await this.waitForDaemonShutdown();
+
+		// Clean up stale files
</file context>
Suggested change
await this.waitForDaemonShutdown();
await this.waitForDaemonShutdown();
if (existsSync(SOCKET_PATH) && (await this.isSocketLive())) {
throw new Error("Failed to restart terminal daemon: previous daemon is still running");
}
Fix with Cubic

Comment on lines +1452 to +1453
// Socket not connectable but may still exist - try SIGTERM
this.killDaemonFromPidFile();
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Mar 18, 2026

Choose a reason for hiding this comment

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

P1: Avoid PID-file SIGTERM when no daemon socket exists; stale PID files can kill unrelated processes.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/main/lib/terminal-host/client.ts, line 1452:

<comment>Avoid PID-file SIGTERM when no daemon socket exists; stale PID files can kill unrelated processes.</comment>

<file context>
@@ -1411,6 +1411,65 @@ export class TerminalHostClient extends EventEmitter {
+				this.resetConnectionState({ emitDisconnected: false });
+			}
+		} else {
+			// Socket not connectable but may still exist - try SIGTERM
+			this.killDaemonFromPidFile();
+		}
</file context>
Suggested change
// Socket not connectable but may still exist - try SIGTERM
this.killDaemonFromPidFile();
// Socket exists but is not connectable - try SIGTERM
if (existsSync(SOCKET_PATH)) this.killDaemonFromPidFile();
Fix with Cubic

// macOS where Fast User Switching can leave a stale daemon with a
// degraded security context (causing TLS/Keychain failures).
// Sessions are recovered via cold restore from disk history.
await this.client.restartDaemon();
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot Mar 18, 2026

Choose a reason for hiding this comment

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

P2: This restart is unconditional even though the stale security-session bug is macOS-specific. Gate it to darwin so Windows/Linux keep their existing daemon lifecycle.

(Based on your team's feedback about gating platform-specific fixes with runtime checks.)

View Feedback

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/desktop/src/main/lib/terminal/daemon/daemon-manager.ts, line 86:

<comment>This restart is unconditional even though the stale security-session bug is macOS-specific. Gate it to darwin so Windows/Linux keep their existing daemon lifecycle.

(Based on your team's feedback about gating platform-specific fixes with runtime checks.) </comment>

<file context>
@@ -78,6 +78,13 @@ export class DaemonTerminalManager extends EventEmitter {
+			// macOS where Fast User Switching can leave a stale daemon with a
+			// degraded security context (causing TLS/Keychain failures).
+			// Sessions are recovered via cold restore from disk history.
+			await this.client.restartDaemon();
+
 			const response = await this.client.listSessions();
</file context>
Suggested change
await this.client.restartDaemon();
if (process.platform === "darwin") {
await this.client.restartDaemon();
}
Fix with Cubic

Haknt added a commit to Haknt/superset that referenced this pull request Apr 19, 2026
Adds the implementation plan and architecture design for resolving
issue superset-sh#2570 (stale macOS bootstrap context in terminal-host daemon).

The approach delegates PTY spawn to the Electron main process (always
fresh context) rather than killing running sessions on every restart
(as proposed in superset-sh#2571).

Refs superset-sh#2570
Haknt added a commit to Haknt/superset that referenced this pull request Apr 27, 2026
Adds the implementation plan and architecture design for resolving
issue superset-sh#2570 (stale macOS bootstrap context in terminal-host daemon).

The approach delegates PTY spawn to the Electron main process (always
fresh context) rather than killing running sessions on every restart
(as proposed in superset-sh#2571).

Refs superset-sh#2570
@Kitenite
Copy link
Copy Markdown
Collaborator

Kitenite commented May 6, 2026

Closing as stale: bot PR created 3+ weeks ago with no iteration since. Referenced issue remains open — re-attempt from a fresh branch if a fix is still wanted. Bulk audit 2026-05-06.

@Kitenite Kitenite closed this May 6, 2026
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.

[bug] Terminal-host process persists across app restarts, inherits stale macOS security session after Fast User Switching

1 participant