Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 11 additions & 1 deletion apps/desktop/src/lib/trpc/routers/settings/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import {
} from "@superset/local-db";
import { TRPCError } from "@trpc/server";
import { localDb } from "main/lib/local-db";
import { disableDaemonMode, enableDaemonMode } from "main/lib/terminal";
import { resetWorkspaceRuntimeRegistry } from "main/lib/workspace-runtime";
import {
DEFAULT_CONFIRM_ON_QUIT,
DEFAULT_TERMINAL_LINK_BEHAVIOR,
Expand Down Expand Up @@ -253,7 +255,7 @@ export const createSettingsRouter = () => {

setTerminalPersistence: publicProcedure
.input(z.object({ enabled: z.boolean() }))
.mutation(({ input }) => {
.mutation(async ({ input }) => {
localDb
.insert(settings)
.values({ id: 1, terminalPersistence: input.enabled })
Expand All @@ -263,6 +265,14 @@ export const createSettingsRouter = () => {
})
.run();

if (input.enabled) {
await enableDaemonMode();
} else {
await disableDaemonMode();
}

resetWorkspaceRuntimeRegistry();

return { success: true };
}),
});
Expand Down
152 changes: 91 additions & 61 deletions apps/desktop/src/main/lib/terminal/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,27 +25,21 @@ export type {
// Terminal Manager Selection
// =============================================================================

// Cache the daemon mode setting to avoid repeated DB reads
// This is set once at app startup and doesn't change until restart
// Cached daemon mode setting. Updated at startup and via enable/disable functions.
let cachedDaemonMode: boolean | null = null;
const DEBUG_TERMINAL = process.env.SUPERSET_TERMINAL_DEBUG === "1";

/**
* Check if daemon mode is enabled.
* Reads from user settings (terminalPersistence) or falls back to env var.
* The value is cached since it requires app restart to take effect.
* Check if daemon mode is enabled. Caches the result after first read.
*/
export function isDaemonModeEnabled(): boolean {
// Return cached value if available
if (cachedDaemonMode !== null) {
return cachedDaemonMode;
}

// First check environment variable override (for development/testing)
// Environment variable override for development/testing
if (process.env.SUPERSET_TERMINAL_DAEMON === "1") {
console.log(
"[TerminalManager] Daemon mode: ENABLED (via SUPERSET_TERMINAL_DAEMON env var)",
);
console.log("[TerminalManager] Daemon mode: ENABLED (env override)");
cachedDaemonMode = true;
return true;
}
Expand All @@ -55,98 +49,134 @@ export function isDaemonModeEnabled(): boolean {
const row = localDb.select().from(settings).get();
const enabled = row?.terminalPersistence ?? DEFAULT_TERMINAL_PERSISTENCE;
console.log(
`[TerminalManager] Daemon mode: ${enabled ? "ENABLED" : "DISABLED"} (via settings.terminalPersistence)`,
`[TerminalManager] Daemon mode: ${enabled ? "ENABLED" : "DISABLED"}`,
);
cachedDaemonMode = enabled;
return enabled;
} catch (error) {
console.warn(
"[TerminalManager] Failed to read settings, defaulting to disabled:",
error,
);
console.warn("[TerminalManager] Failed to read settings:", error);
cachedDaemonMode = DEFAULT_TERMINAL_PERSISTENCE;
return DEFAULT_TERMINAL_PERSISTENCE;
}
}

/**
* Get the active terminal manager based on current settings.
* Returns either the in-process manager or the daemon-based manager.
* Get the active terminal manager based on current daemon mode setting.
*/
export function getActiveTerminalManager():
| TerminalManager
| DaemonTerminalManager {
const daemonEnabled = isDaemonModeEnabled();
if (DEBUG_TERMINAL) {
console.log(
"[getActiveTerminalManager] Daemon mode enabled:",
daemonEnabled,
);
console.log("[getActiveTerminalManager] Daemon:", daemonEnabled);
}
if (daemonEnabled) {
return getDaemonTerminalManager();
return daemonEnabled ? getDaemonTerminalManager() : terminalManager;
}

// =============================================================================
// Core Daemon Operations
// =============================================================================

/**
* Initialize daemon and reconcile sessions.
* Used by both startup and runtime enable paths.
*/
async function initializeDaemon(): Promise<void> {
const manager = getDaemonTerminalManager();
await manager.reconcileOnStartup();
}

/**
* Shutdown daemon and dispose client.
* Used by both startup orphan cleanup and runtime disable paths.
*/
async function shutdownDaemon(): Promise<{ wasRunning: boolean }> {
try {
const client = getTerminalHostClient();
const result = await client.shutdownIfRunning({ killSessions: true });
return result;
} finally {
disposeTerminalHostClient();
}
return terminalManager;
}

// =============================================================================
// Startup Functions
// =============================================================================

/**
* Reconcile daemon sessions on app startup.
* Should be called on app startup when daemon mode is ENABLED to clean up
* stale sessions from previous app runs.
*
* Current semantics: terminal persistence survives app restarts.
* Reconciliation removes sessions that no longer map to existing workspaces and
* restores state for sessions that can be retained.
* Reconcile daemon sessions on app startup (if daemon mode is enabled).
*/
export async function reconcileDaemonSessions(): Promise<void> {
if (!isDaemonModeEnabled()) {
// Not in daemon mode, nothing to reconcile
return;
}

try {
const manager = getDaemonTerminalManager();
await manager.reconcileOnStartup();
await initializeDaemon();
} catch (error) {
console.warn(
"[TerminalManager] Failed to reconcile daemon sessions:",
error,
);
console.warn("[TerminalManager] Failed to reconcile daemon sessions:", error);
}
}

/**
* Shutdown any orphaned daemon process.
* Should be called on app startup when daemon mode is disabled to clean up
* any daemon left running from a previous session with persistence enabled.
*
* Uses shutdownIfRunning() to avoid spawning a new daemon just to shut it down.
* Shutdown orphaned daemon on app startup (if daemon mode is disabled).
*/
export async function shutdownOrphanedDaemon(): Promise<void> {
if (isDaemonModeEnabled()) {
// Daemon mode is enabled, don't shutdown
return;
}

try {
const client = getTerminalHostClient();
// Use shutdownIfRunning to avoid spawning a daemon if none exists
const { wasRunning } = await client.shutdownIfRunning({
killSessions: true,
});
if (wasRunning) {
console.log("[TerminalManager] Shutdown orphaned daemon successfully");
} else {
console.log("[TerminalManager] No orphaned daemon to shutdown");
}
const { wasRunning } = await shutdownDaemon();
console.log(
`[TerminalManager] Orphan cleanup: ${wasRunning ? "daemon shutdown" : "no daemon"}`,
);
} catch (error) {
// Unexpected error during shutdown attempt
console.warn(
"[TerminalManager] Error during orphan daemon cleanup:",
error,
console.warn("[TerminalManager] Orphan cleanup error:", error);
}
}

// =============================================================================
// Runtime Toggle Functions
// =============================================================================

/**
* Enable daemon mode at runtime. Initializes daemon and reconciles sessions.
*/
export async function enableDaemonMode(): Promise<void> {
if (cachedDaemonMode === true) {
return;
}

console.log("[TerminalManager] Enabling daemon mode");
cachedDaemonMode = true;

try {
await initializeDaemon();
} catch (error) {
console.error("[TerminalManager] Failed to enable daemon mode:", error);
throw error;
}
}
Comment on lines +147 to +161
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Cache is updated before async operation completes.

If initializeDaemon() fails, the cache remains set to true while the daemon isn't actually running. This could cause subsequent calls to getActiveTerminalManager() to return the daemon manager when the daemon isn't initialized.

Consider rolling back the cache on failure:

Proposed fix
 export async function enableDaemonMode(): Promise<void> {
 	if (cachedDaemonMode === true) {
 		return;
 	}

 	console.log("[TerminalManager] Enabling daemon mode");
 	cachedDaemonMode = true;

 	try {
 		await initializeDaemon();
 	} catch (error) {
+		cachedDaemonMode = false;
 		console.error("[TerminalManager] Failed to enable daemon mode:", error);
 		throw error;
 	}
 }
📝 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
export async function enableDaemonMode(): Promise<void> {
if (cachedDaemonMode === true) {
return;
}
console.log("[TerminalManager] Enabling daemon mode");
cachedDaemonMode = true;
try {
await initializeDaemon();
} catch (error) {
console.error("[TerminalManager] Failed to enable daemon mode:", error);
throw error;
}
}
export async function enableDaemonMode(): Promise<void> {
if (cachedDaemonMode === true) {
return;
}
console.log("[TerminalManager] Enabling daemon mode");
cachedDaemonMode = true;
try {
await initializeDaemon();
} catch (error) {
cachedDaemonMode = false;
console.error("[TerminalManager] Failed to enable daemon mode:", error);
throw error;
}
}
🤖 Prompt for AI Agents
In `@apps/desktop/src/main/lib/terminal/index.ts` around lines 147 - 161, The
cachedDaemonMode is set to true before initializeDaemon() completes, so on
failure the cache incorrectly indicates the daemon is active; update
enableDaemonMode to only set cachedDaemonMode = true after await
initializeDaemon() succeeds, or alternatively catch the error and reset
cachedDaemonMode = false before rethrowing; reference enableDaemonMode,
initializeDaemon, cachedDaemonMode and getActiveTerminalManager when making the
change so subsequent calls reflect the real daemon state.


/**
* Disable daemon mode at runtime. Shuts down daemon and terminates all sessions.
*/
export async function disableDaemonMode(): Promise<void> {
if (cachedDaemonMode === false) {
return;
}

console.log("[TerminalManager] Disabling daemon mode");
cachedDaemonMode = false;

try {
const { wasRunning } = await shutdownDaemon();
console.log(
`[TerminalManager] Daemon ${wasRunning ? "shutdown" : "was not running"}`,
);
} finally {
// Always dispose the client to clean up any partial state
disposeTerminalHostClient();
} catch (error) {
console.warn("[TerminalManager] Daemon shutdown error:", error);
}
}
Loading