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
2 changes: 1 addition & 1 deletion src/browser/components/ChatPane/WorkspaceFooterBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -400,7 +400,7 @@ export const WorkspaceFooterBar: React.FC<WorkspaceFooterBarProps> = (props) =>
>
{/* min-h rather than a fixed height: mobile raises these buttons to 44px touch targets, and a
capped row would clip them along the same axis overflow-x-auto makes scrollable. */}
<div className="scrollbar-none flex min-h-7 items-center gap-2 overflow-x-auto px-2 text-xs whitespace-nowrap">
<div className="scrollbar-none scroll-fade-x flex min-h-7 items-center gap-2 overflow-x-auto px-2 text-xs whitespace-nowrap">
<RuntimeBadge
runtimeConfig={props.runtimeConfig}
isWorking={isWorking}
Expand Down
4 changes: 3 additions & 1 deletion src/browser/components/SkillIndicator/SkillIndicator.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,9 @@ const SkillsPopoverContent: React.FC<SkillsPopoverContentProps> = (props) => {
)}
{isLoaded && <Check className="text-success ml-1 inline h-3 w-3" />}
</span>
<span className="text-muted-foreground line-clamp-1 text-[11px] leading-snug">
{/* Full text (#5697): the list already scrolls, and a clamp left no way to read
the rest. */}
<span className="text-muted-foreground text-[11px] leading-snug break-words">
{skill.description}
</span>
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -813,7 +813,7 @@ export function TimelinePanelView(props: TimelinePanelViewProps) {
}}
>
<div className="border-border shrink-0 border-b px-3 py-2.5">
<div className="scrollbar-none flex min-w-0 gap-1.5 overflow-x-auto">
<div className="scrollbar-none scroll-fade-x flex min-w-0 gap-1.5 overflow-x-auto">
{FILTERS.map((item) => (
<button
key={item.value}
Expand Down
46 changes: 46 additions & 0 deletions src/browser/styles/scrollbar-none.css
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,49 @@
scrollbar-width: none;
scrollbar-color: auto;
}

/*
* Edge fade for rows that scroll sideways with a hidden scrollbar (#5696): an edge fades out
* while more items sit past it, so cut-off items do not look missing. Scroll-driven CSS, no JS:
* a row that fits has an inactive scroll timeline and shows no fade, and browsers without scroll
* timelines keep the plain row.
*/
@property --scroll-fade-start {
syntax: "<length>";
inherits: false;
initial-value: 0px;
}
@property --scroll-fade-end {
syntax: "<length>";
inherits: false;
initial-value: 0px;
}
@keyframes scroll-fade-x {
0% {
--scroll-fade-start: 0px;
--scroll-fade-end: 24px;
}
10%,
90% {
--scroll-fade-start: 24px;
--scroll-fade-end: 24px;
}
100% {
--scroll-fade-start: 24px;
--scroll-fade-end: 0px;
}
}
@supports (animation-timeline: scroll()) {
.scroll-fade-x {
mask-image: linear-gradient(
to right,
transparent,
#000 var(--scroll-fade-start),
#000 calc(100% - var(--scroll-fade-end)),
transparent
);
animation: scroll-fade-x linear both;
/* After the shorthand, which resets the timeline. */
animation-timeline: scroll(self inline);
}
}
2 changes: 1 addition & 1 deletion src/browser/terminal-window.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ if (!workspaceId || !sessionId) {
// race conditions with WebSocket connections and terminal lifecycle
ReactDOM.createRoot(document.getElementById("root")!).render(
<APIProvider>
<TerminalRouterProvider>
<TerminalRouterProvider popout>
<TerminalWindowContent
workspaceId={workspaceId}
sessionId={sessionId}
Expand Down
6 changes: 4 additions & 2 deletions src/browser/terminal/TerminalRouterContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ const TerminalRouterContext = createContext<TerminalSessionRouter | null>(null);

interface TerminalRouterProviderProps {
children: React.ReactNode;
/** Set in pop-out terminal windows (terminal-window.tsx). */
popout?: boolean;
}

/**
Expand All @@ -35,12 +37,12 @@ export function TerminalRouterProvider(props: TerminalRouterProviderProps) {
}

// Create/cleanup after commit to avoid render-time disposal in concurrent mode.
const nextRouter = new TerminalSessionRouter(api);
const nextRouter = new TerminalSessionRouter(api, { popout: props.popout === true });
setRouter(nextRouter);
return () => {
nextRouter.dispose();
};
}, [api]);
}, [api, props.popout]);

const routerForContext = api && router?.getApi() === api ? router : null;

Expand Down
11 changes: 9 additions & 2 deletions src/browser/terminal/TerminalSessionRouter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,12 @@ export class TerminalSessionRouter {
private readonly api: APIClient;
private sessions = new Map<string, SessionState>();

constructor(api: APIClient) {
/** True in a pop-out terminal window: its attaches keep the session out of the sidebar. */
private readonly popout: boolean;

constructor(api: APIClient, options?: { popout?: boolean }) {
this.api = api;
this.popout = options?.popout === true;
}

/** Get the API client (for identity comparison when recreating router) */
Expand Down Expand Up @@ -287,7 +291,10 @@ export class TerminalSessionRouter {
// Start attach stream (fire-and-forget, but managed by abort controller)
void (async () => {
try {
const iterator = await this.api.terminal.attach({ sessionId }, { signal });
const iterator = await this.api.terminal.attach(
this.popout ? { sessionId, popout: true } : { sessionId },
{ signal }
);
for await (const msg of iterator) {
// Check if session was removed (unsubscribed)
const currentSession = this.sessions.get(sessionId);
Expand Down
6 changes: 5 additions & 1 deletion src/common/orpc/schemas/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2907,7 +2907,11 @@ export const terminal = {
* Guarantees no missed output between state snapshot and live stream.
*/
attach: {
input: z.object({ sessionId: z.string() }),
input: z.object({
sessionId: z.string(),
/** Set by pop-out terminal windows; their sessions stay out of listSessions meanwhile. */
popout: z.boolean().nullish(),
}),
output: eventIterator(
z.discriminatedUnion("type", [
z.object({ type: z.literal("screenState"), data: z.string() }),
Expand Down
24 changes: 24 additions & 0 deletions src/desktop/terminalWindowManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ export class TerminalWindowManager {
private windows = new Map<string, Set<BrowserWindow>>(); // workspaceId -> Set of windows
private windowCount = 0; // Counter for unique window IDs
private readonly config: Config;
private onSessionWindowClosed: ((sessionId: string, closedBy: "user" | "app") => void) | null =
null;
/** Windows closed by closeTerminalWindow rather than by the user. */
private readonly appClosedWindows = new WeakSet<BrowserWindow>();

constructor(
config: Config,
Expand All @@ -34,6 +38,13 @@ export class TerminalWindowManager {
this.config = config;
}

/** Called with the session ID when a pop-out window that showed a session has closed. */
setSessionWindowClosedHandler(
handler: (sessionId: string, closedBy: "user" | "app") => void
): void {
this.onSessionWindowClosed = handler;
}

/**
* Open a new terminal window for a workspace
* Multiple windows can be open for the same workspace
Expand Down Expand Up @@ -117,6 +128,18 @@ export class TerminalWindowManager {
}
}
log.info(`Terminal window ${windowId} closed for workspace: ${workspaceId}`);
// 'closed' fires only when the window really goes away: its reload and a renderer crash
// keep the window, so the session survives those.
if (sessionId) {
try {
this.onSessionWindowClosed?.(
sessionId,
this.appClosedWindows.has(terminalWindow) ? "app" : "user"
);
} catch (err) {
log.error(`Failed to end terminal session ${sessionId} after its window closed:`, err);
}
}
});

// Load the terminal page
Expand Down Expand Up @@ -158,6 +181,7 @@ export class TerminalWindowManager {
if (windowSet) {
for (const window of windowSet) {
if (!window.isDestroyed()) {
this.appClosedWindows.add(window);
window.close();
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/node/orpc/router.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2492,7 +2492,9 @@ export const router = (authToken?: string) => {
attach: t
.input(schemas.terminal.attach.input)
.output(schemas.terminal.attach.output)
.handler(({ context, input, signal }) => attachTerminal(context, input.sessionId, signal)),
.handler(({ context, input, signal }) =>
attachTerminal(context, input.sessionId, signal, input.popout === true)
),
onExit: t
.input(schemas.terminal.onExit.input)
.output(schemas.terminal.onExit.output)
Expand Down
41 changes: 41 additions & 0 deletions src/node/orpc/routerSubscriptions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { SUBSCRIPTION_HEARTBEAT_INTERVAL_MS } from "@/constants/orpcSubscription
import { disposeAppRuntime, makeAppRuntime } from "@/node/services/di/appRuntime";
import type { ORPCContext } from "./context";
import {
attachTerminal,
subscribeWorkspaceActivity,
subscribeDesignExperiment,
subscribeMetadata,
Expand Down Expand Up @@ -246,3 +247,43 @@ test("Design subscriptions publish sibling changes only after client shutdown",
await stream.return(undefined);
}
});

test("a pop-out attach hides its session from listSessions until the stream ends (#5673)", async () => {
const app = makeAppRuntime(TestClock.layer());
const sessions = ["popped"];
let popoutAttaches = 0;
const terminalService = {
onOutput: () => () => undefined,
getScreenState: () => "",
markPopoutAttached: () => {
popoutAttaches++;
return () => {
popoutAttaches--;
};
},
};
// What listSessions returns, given TerminalService's filter.
const listed = () => (popoutAttaches > 0 ? [] : sessions);
const context = { "effect/context": app.context, terminalService } as unknown as ORPCContext;

const sidebar = new AbortController();
const sidebarStream = attachTerminal(context, "popped", sidebar.signal);
const popout = new AbortController();
const popoutStream = attachTerminal(context, "popped", popout.signal, true);
try {
expect((await sidebarStream.next()).value).toEqual({ type: "screenState", data: "" });
expect((await popoutStream.next()).value).toEqual({ type: "screenState", data: "" });
// A main-window reload now finds nothing to adopt as a sidebar tab.
expect(listed()).toEqual([]);

// The pop-out crashed or reloaded: its connection drops and the session can come back.
popout.abort();
await popoutStream.return(undefined);
expect(listed()).toEqual(["popped"]);
} finally {
sidebar.abort();
popout.abort();
await sidebarStream.return(undefined);
await disposeAppRuntime(app.managed);
}
});
16 changes: 13 additions & 3 deletions src/node/orpc/routerSubscriptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -672,13 +672,23 @@ export function subscribeTerminalOutput(
export function attachTerminal(
context: ORPCContext,
sessionId: string,
signal?: AbortSignal
signal?: AbortSignal,
popout = false
): AsyncGenerator<TerminalAttachMessage> {
// Output subscribes before screen capture so attach cannot lose bytes in the handshake.
return runtimeSubscription<TerminalAttachMessage>(context, {
signal,
subscribe: (emit) =>
context.terminalService.onOutput(sessionId, (data) => emit.push({ type: "output", data })),
subscribe: (emit) => {
const unsubscribe = context.terminalService.onOutput(sessionId, (data) =>
emit.push({ type: "output", data })
);
// The stream's end (window closed, crashed, reloaded or disconnected) releases the mark.
const releasePopout = popout ? context.terminalService.markPopoutAttached(sessionId) : null;
return () => {
releasePopout?.();
unsubscribe();
};
},
initial: () => ({
type: "screenState" as const,
data: context.terminalService.getScreenState(sessionId),
Expand Down
51 changes: 51 additions & 0 deletions src/node/services/terminalService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@ const closeTerminalWindowMock = mock(() => {
const mockWindowManager = {
openTerminalWindow: openTerminalWindowMock,
closeTerminalWindow: closeTerminalWindowMock,
setSessionWindowClosedHandler: () => undefined,
} as unknown as TerminalWindowManager;

describe("TerminalService", () => {
Expand Down Expand Up @@ -654,6 +655,56 @@ describe("TerminalService", () => {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(mockPTYService.createSession as any) = createSessionMock;
});
describe("pop-out attachments (#5673)", () => {
it("a session with a live pop-out is not listed, so the sidebar does not adopt it", () => {
getWorkspaceSessionIdsMock.mockImplementation(
() => ["popped", "sidebar"] as unknown as never[]
);

const release = service.markPopoutAttached("popped");
expect(service.getWorkspaceSessionIds("ws-1")).toEqual(["sidebar"]);

// The pop-out's attach stream ended (window closed, crashed or reloaded): the session can
// come back, for example as a sidebar tab on the next load.
release();
expect(service.getWorkspaceSessionIds("ws-1")).toEqual(["popped", "sidebar"]);
});

it("the session stays hidden until every pop-out attach for it has ended", () => {
getWorkspaceSessionIdsMock.mockImplementation(() => ["popped"] as unknown as never[]);

const releaseFirst = service.markPopoutAttached("popped");
const releaseSecond = service.markPopoutAttached("popped");
releaseFirst();
releaseFirst();
expect(service.getWorkspaceSessionIds("ws-1")).toEqual([]);

releaseSecond();
expect(service.getWorkspaceSessionIds("ws-1")).toEqual(["popped"]);
});

it("closing a desktop pop-out window ends its session; app-closed sibling windows do not", () => {
let onClosed: ((sessionId: string, closedBy: "user" | "app") => void) | undefined;
const windowManager = {
...mockWindowManager,
setSessionWindowClosedHandler: (
handler: (sessionId: string, closedBy: "user" | "app") => void
) => {
onClosed = handler;
},
} as unknown as TerminalWindowManager;
service.setTerminalWindowManager(windowManager);
const closeSpy = spyOn(service, "close");

// One pop-out's shell exited, so the app closed every pop-out of the workspace: the
// siblings' shells keep running.
onClosed?.("sibling", "app");
onClosed?.("popped", "user");

expect(closeSpy.mock.calls).toEqual([["popped"]]);
});
});

describe("terminal activity tracking", () => {
let capturedOnData: ((data: string) => void) | undefined;
let capturedOnExit: ((code: number) => void) | undefined;
Expand Down
Loading
Loading