Skip to content
Merged
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
56 changes: 56 additions & 0 deletions apps/desktop/src/renderer/lib/terminal/launch-command.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ import {
launchCommandInPane,
writeCommandsInPane,
} from "./launch-command";
import {
clearTerminalSessionReady,
markTerminalSessionReady,
rejectTerminalSessionReady,
} from "./session-readiness";

describe("launchCommandInPane", () => {
it("creates a terminal session and writes the command with a newline", async () => {
Expand Down Expand Up @@ -103,6 +108,57 @@ describe("launchCommandInPane", () => {
throwOnError: true,
});
});

it("waits for the mounted session before writing when requested", async () => {
const paneId = "pane-mounted-session-ready";
const createOrAttach = mock(async () => ({}));
const write = mock(async () => ({}));

const launchPromise = launchCommandInPane({
paneId,
tabId: "tab-1",
workspaceId: "ws-1",
command: "echo hello",
createOrAttach,
write,
waitForMountedSession: true,
});

expect(createOrAttach).not.toHaveBeenCalled();
expect(write).not.toHaveBeenCalled();

markTerminalSessionReady(paneId);
await launchPromise;
clearTerminalSessionReady(paneId);

expect(write).toHaveBeenCalledWith({
paneId,
data: "echo hello\n",
throwOnError: true,
});
});

it("propagates mounted-session readiness failures", async () => {
const paneId = "pane-mounted-session-failure";
const createOrAttach = mock(async () => ({}));
const write = mock(async () => ({}));

const launchPromise = launchCommandInPane({
paneId,
tabId: "tab-1",
workspaceId: "ws-1",
command: "echo hello",
createOrAttach,
write,
waitForMountedSession: true,
});

rejectTerminalSessionReady(paneId, new Error("attach failed"));

await expect(launchPromise).rejects.toThrow("attach failed");
expect(createOrAttach).not.toHaveBeenCalled();
expect(write).not.toHaveBeenCalled();
});
});

describe("buildTerminalCommand", () => {
Expand Down
13 changes: 13 additions & 0 deletions apps/desktop/src/renderer/lib/terminal/launch-command.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { isTerminalAttachCanceledMessage } from "renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/Terminal/attach-cancel";
import { waitForTerminalSessionReady } from "./session-readiness";

interface TerminalCreateOrAttachInput {
paneId: string;
Expand All @@ -23,6 +24,11 @@ interface LaunchCommandInPaneOptions {
createOrAttach: (input: TerminalCreateOrAttachInput) => Promise<unknown>;
write: (input: TerminalWriteInput) => Promise<unknown>;
noExecute?: boolean;
/**
* Only use this for panes that will mount immediately in the active tab.
* Background tabs must use the helper-side attach path instead.
*/
waitForMountedSession?: boolean;
}

function normalizeTerminalCommand(command: string): string {
Expand Down Expand Up @@ -82,7 +88,14 @@ export async function launchCommandInPane({
createOrAttach,
write,
noExecute,
waitForMountedSession,
}: LaunchCommandInPaneOptions): Promise<void> {
if (waitForMountedSession) {
await waitForTerminalSessionReady(paneId);
await writeCommandInPane({ paneId, command, write, noExecute });
return;
}

await ensureTerminalAttached({
paneId,
tabId,
Expand Down
54 changes: 54 additions & 0 deletions apps/desktop/src/renderer/lib/terminal/session-readiness.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
type SessionReadyWaiter = {
resolve: () => void;
reject: (error: Error) => void;
};

const readyPaneIds = new Set<string>();
const waitersByPaneId = new Map<string, Set<SessionReadyWaiter>>();

function resolveWaiters(paneId: string): void {
const waiters = waitersByPaneId.get(paneId);
if (!waiters) return;
waitersByPaneId.delete(paneId);
for (const waiter of waiters) {
waiter.resolve();
}
}

function rejectWaiters(paneId: string, error: Error): void {
const waiters = waitersByPaneId.get(paneId);
if (!waiters) return;
waitersByPaneId.delete(paneId);
for (const waiter of waiters) {
waiter.reject(error);
}
}

export function clearTerminalSessionReady(paneId: string): void {
readyPaneIds.delete(paneId);
}

export function markTerminalSessionReady(paneId: string): void {
readyPaneIds.add(paneId);
resolveWaiters(paneId);
}

export function rejectTerminalSessionReady(paneId: string, error: Error): void {
readyPaneIds.delete(paneId);
rejectWaiters(paneId, error);
}

export function waitForTerminalSessionReady(paneId: string): Promise<void> {
if (readyPaneIds.has(paneId)) {
return Promise.resolve();
}

return new Promise<void>((resolve, reject) => {
let waiters = waitersByPaneId.get(paneId);
if (!waiters) {
waiters = new Set();
waitersByPaneId.set(paneId, waiters);
}
waiters.add({ resolve, reject });
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,11 @@ import type { MutableRefObject, RefObject } from "react";
import { useCallback, useEffect, useRef, useState } from "react";
import { writeCommandInPane } from "renderer/lib/terminal/launch-command";
import type { DetectedLink } from "renderer/lib/terminal/links";
import {
clearTerminalSessionReady,
markTerminalSessionReady,
rejectTerminalSessionReady,
} from "renderer/lib/terminal/session-readiness";
import { electronTrpcClient } from "renderer/lib/trpc-client";
import { useTabsStore } from "renderer/stores/tabs/store";
import { killTerminalForPane } from "renderer/stores/tabs/utils/terminal-cleanup";
Expand Down Expand Up @@ -371,6 +376,7 @@ export function useTerminalLifecycle({
const requestId = nextAttachRequestId();
cancelAttachRequest(activeAttachRequestId);
activeAttachRequestId = requestId;
clearTerminalSessionReady(paneId);
createOrAttachRef.current(
{
paneId,
Expand Down Expand Up @@ -566,6 +572,7 @@ export function useTerminalLifecycle({
!isUnmounted && !attachCanceled && attachId === activeAttachId;

markAttachInFlight(paneId, attachId);
clearTerminalSessionReady(paneId);

const finishAttach = () => {
clearAttachInFlight(paneId, attachId);
Expand Down Expand Up @@ -600,6 +607,7 @@ export function useTerminalLifecycle({
// flow through the component's registered handler.
v1TerminalCache.startStream(paneId);
v1TerminalCache.setStreamReady(paneId);
markTerminalSessionReady(paneId);

const storedColdRestore = coldRestoreState.get(paneId);
if (storedColdRestore?.isRestored) {
Expand Down Expand Up @@ -666,6 +674,10 @@ export function useTerminalLifecycle({
}
const workspaceRun = getPaneWorkspaceRun(paneId);
if (error.message?.includes("TERMINAL_SESSION_KILLED")) {
rejectTerminalSessionReady(
paneId,
new Error(error.message || "Terminal session killed"),
);
if (workspaceRun) {
setPaneWorkspaceRunState(paneId, "stopped-by-user");
}
Expand All @@ -677,6 +689,10 @@ export function useTerminalLifecycle({
return;
}
console.error("[Terminal] Failed to create/attach:", error);
rejectTerminalSessionReady(
paneId,
new Error(error.message || "Failed to connect to terminal"),
);
if (workspaceRun) {
setPaneWorkspaceRunState(paneId, "stopped-by-exit");
}
Expand Down
84 changes: 55 additions & 29 deletions apps/desktop/src/renderer/stores/tabs/useTabsWithPresets.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,13 +103,17 @@ export function useTabsWithPresets(projectId?: string | null) {
}, []);

const launchPresetCommand = useCallback(
({ paneId, tabId, workspaceId, command, cwd }: PresetPaneLaunch) => {
(
{ paneId, tabId, workspaceId, command, cwd }: PresetPaneLaunch,
options?: { waitForMountedSession?: boolean },
) => {
void launchCommandInPane({
paneId,
tabId,
workspaceId,
command,
cwd,
waitForMountedSession: options?.waitForMountedSession,
createOrAttach: (input) => createOrAttach.mutateAsync(input),
write: (input) => writeToTerminal.mutateAsync(input),
}).catch((error) => {
Expand All @@ -125,9 +129,12 @@ export function useTabsWithPresets(projectId?: string | null) {
);

const launchPresetCommands = useCallback(
(launches: PresetPaneLaunch[]) => {
(
launches: PresetPaneLaunch[],
options?: { waitForMountedSession?: boolean },
) => {
for (const launch of launches) {
launchPresetCommand(launch);
launchPresetCommand(launch, options);
}
},
[launchPresetCommand],
Expand All @@ -145,13 +152,16 @@ export function useTabsWithPresets(projectId?: string | null) {
if (firstPresetCommand === null) return;
const workspaceId = resolveWorkspaceIdForTab(tabId);
if (!workspaceId) return;
launchPresetCommand({
paneId,
tabId,
workspaceId,
command: firstPresetCommand,
cwd: firstPreset?.cwd || undefined,
});
launchPresetCommand(
{
paneId,
tabId,
workspaceId,
command: firstPresetCommand,
cwd: firstPreset?.cwd || undefined,
},
{ waitForMountedSession: true },
);
},
[
firstPreset,
Expand All @@ -162,20 +172,27 @@ export function useTabsWithPresets(projectId?: string | null) {
);

const launchFirstPresetInFocusedPane = useCallback(
(tabId: string, previousFocusedPaneId: string | undefined) => {
(
tabId: string,
previousFocusedPaneId: string | undefined,
options?: { waitForMountedSession?: boolean },
) => {
if (firstPresetCommand === null) return;
const state = useTabsStore.getState();
const paneId = state.focusedPaneIds[tabId];
if (!paneId || paneId === previousFocusedPaneId) return;
const tab = state.tabs.find((tabItem) => tabItem.id === tabId);
if (!tab) return;
launchPresetCommand({
paneId,
tabId,
workspaceId: tab.workspaceId,
command: firstPresetCommand,
cwd: firstPreset?.cwd || undefined,
});
launchPresetCommand(
{
paneId,
tabId,
workspaceId: tab.workspaceId,
command: firstPresetCommand,
cwd: firstPreset?.cwd || undefined,
},
options,
);
},
[firstPreset, firstPresetCommand, launchPresetCommand],
);
Expand Down Expand Up @@ -302,7 +319,7 @@ export function useTabsWithPresets(projectId?: string | null) {
];
},
);
launchPresetCommands(launches);
launchPresetCommands(launches, { waitForMountedSession: true });
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Stop deferring preset launch for non-visible active tabs

Passing waitForMountedSession: true here makes launchCommandInPane skip createOrAttach and wait for markTerminalSessionReady, which is only emitted by useTerminalLifecycle after the pane is mounted. That works for the currently rendered tab, but openPreset(..., { target: "active-tab" }) is also invoked from background setup flows (e.g. workspace init), where the workspace tab can exist without being mounted; in that case the promise never resolves and the preset command is never sent. Please keep helper-side attach for these active-tab preset paths unless visibility/mount is guaranteed.

Useful? React with 👍 / 👎.

return { tabId: activeTabId, paneId: paneIds[0] };
}
return executePresetInNewTab(workspaceId, preset);
Expand All @@ -315,13 +332,16 @@ export function useTabsWithPresets(projectId?: string | null) {
});
if (paneId) {
if (command !== null) {
launchPresetCommand({
paneId,
tabId: activeTabId,
workspaceId,
command,
cwd: preset.initialCwd,
});
launchPresetCommand(
{
paneId,
tabId: activeTabId,
workspaceId,
command,
cwd: preset.initialCwd,
},
{ waitForMountedSession: true },
);
}
return { tabId: activeTabId, paneId };
}
Expand Down Expand Up @@ -436,7 +456,9 @@ export function useTabsWithPresets(projectId?: string | null) {
const previousFocusedPaneId =
useTabsStore.getState().focusedPaneIds[tabId];
storeSplitPaneVertical(tabId, sourcePaneId, path, firstPresetOptions);
launchFirstPresetInFocusedPane(tabId, previousFocusedPaneId);
launchFirstPresetInFocusedPane(tabId, previousFocusedPaneId, {
waitForMountedSession: true,
});
},
[
storeSplitPaneVertical,
Expand All @@ -458,7 +480,9 @@ export function useTabsWithPresets(projectId?: string | null) {
const previousFocusedPaneId =
useTabsStore.getState().focusedPaneIds[tabId];
storeSplitPaneHorizontal(tabId, sourcePaneId, path, firstPresetOptions);
launchFirstPresetInFocusedPane(tabId, previousFocusedPaneId);
launchFirstPresetInFocusedPane(tabId, previousFocusedPaneId, {
waitForMountedSession: true,
});
},
[
storeSplitPaneHorizontal,
Expand Down Expand Up @@ -493,7 +517,9 @@ export function useTabsWithPresets(projectId?: string | null) {
path,
firstPresetOptions,
);
launchFirstPresetInFocusedPane(tabId, previousFocusedPaneId);
launchFirstPresetInFocusedPane(tabId, previousFocusedPaneId, {
waitForMountedSession: true,
});
},
[storeSplitPaneAuto, firstPresetOptions, launchFirstPresetInFocusedPane],
);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
import { rejectTerminalSessionReady } from "../../../lib/terminal/session-readiness";
import { electronTrpcClient } from "../../../lib/trpc-client";

/**
* Uses standalone tRPC client to avoid React hook dependencies
*/
export const killTerminalForPane = (paneId: string): void => {
rejectTerminalSessionReady(
paneId,
new Error("Terminal pane was closed before the session became ready"),
);
electronTrpcClient.terminal.kill.mutate({ paneId }).catch((error) => {
console.warn(`Failed to kill terminal for pane ${paneId}:`, error);
});
Expand Down
Loading