officerdev/platform
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

kill a terminal's shell when its panel is actually closed

Browse Source 
Three wrappers held three copies of the same `panelId -> sessionId` bookkeeping, and one of the three
still had the unmount cleanup the other two had removed: `HostTerminalWrapper` dropped its map entry on
every layout or route change, minted a new uuid on the way back, and left the host shell running with
nothing pointing at it. All three now share `useTerminalSession`, which forgets the session and kills
the shell from `usePanelClose` — a real close, and nothing that merely looks like one.

The kill request goes to `/terminal/_officer/sessions/:id`, which is also a fix. `RunningShells` was
asking for `/terminal/sessions`; the proxy strips `/api/terminal` and forwards the rest verbatim, and
the pty sidecar only answers under `/_officer`, so that route 404s. Verified against the live sidecar:
`/sessions` returns `{"error":"not found"}` and `/_officer/sessions` returns the list. The panel has
therefore always read "No shells running" and its kill button has always been a no-op — which is why
the orphaned shells it exists to surface were never actually visible.
This commit is contained in:
pastilhas
2026-08-07 09:55:50 +00:00
parent 198dc71137
commit c92b51cacb
5 changed files with 75 additions and 79 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/workspaces/officerdev/src/apps/Terminal/CommandTerminalWrapper.tsx
+3 -19
View File
@@ -1,12 +1,10 @@
import { useCallback, useEffect } from 'react';
import { useCallback } from 'react';
import { useWorkspace } from '../../components/Workspace';
import { useDashboardState } from 'state/useDashboardState';
import { useGlobal } from 'hooks/useGlobal';
import type { TerminalConnectionState } from './Terminal';
import { TerminalView } from './Terminal';
import { terminalStateKey } from './state-key';
const EMPTY_TERMINALS: Record<string, string> = {};
import { useTerminalSession } from './use-terminal-session';
type CommandTerminalWrapperProps = {
panelId: string;
@@ -16,21 +14,7 @@ type CommandTerminalWrapperProps = {
export const CommandTerminalWrapper = ({ panelId, command, statePrefix }: CommandTerminalWrapperProps) => {
const { workspace, cwd } = useWorkspace();
const stateKey = terminalStateKey(statePrefix, workspace);
const { value: terminals, setValue: setTerminals } = useDashboardState<Record<string, string>>(
stateKey,
EMPTY_TERMINALS,
);
const sessionId = terminals[panelId];
useEffect(() => {
if (!sessionId) {
setTerminals((prev) => ({ ...prev, [panelId]: crypto.randomUUID() }));
}
}, [panelId, sessionId, setTerminals]);
// Kept across unmounts, same as TerminalWrapper: deleting it here meant every layout or route change
// minted a new uuid, abandoned the running shell, and re-ran the command from scratch.
const sessionId = useTerminalSession(panelId, terminalStateKey(statePrefix, workspace));
const [, setConnState] = useGlobal<TerminalConnectionState>(`terminal-conn-${panelId}`, 'disconnected');
const onConnectionChange = useCallback((state: TerminalConnectionState) => setConnState(state), [setConnState]);
src/workspaces/officerdev/src/apps/Terminal/HostTerminalWrapper.tsx
+7 -28
View File
@@ -1,39 +1,18 @@
import { useCallback, useEffect, useRef } from 'react';
import { useCallback } from 'react';
import { useWorkspace } from '../../components/Workspace';
import { useDashboardState } from 'state/useDashboardState';
import { useGlobal } from 'hooks/useGlobal';
import type { TerminalConnectionState } from './Terminal';
import { TerminalView } from './Terminal';
import { terminalStateKey } from './state-key';
import { useTerminalSession } from './use-terminal-session';
const EMPTY_TERMINALS: Record<string, string> = {};
// This one kept the unmount cleanup the other two wrappers had already removed, so every layout or route
// change dropped its `panelId → sessionId` entry, minted a new uuid on the way back, and left the previous
// host shell running with nothing pointing at it. It now forgets the session where the others do: when the
// panel is actually closed.
export const HostTerminalWrapper = ({ panelId }: { panelId: string }) => {
const { workspace, cwd } = useWorkspace();
const stateKey = terminalStateKey('host-terminals', workspace);
const { value: terminals, setValue: setTerminals } = useDashboardState<Record<string, string>>(
stateKey,
EMPTY_TERMINALS,
);
const setTerminalsRef = useRef(setTerminals);
setTerminalsRef.current = setTerminals;
const sessionId = terminals[panelId];
useEffect(() => {
if (!sessionId) {
setTerminals((prev) => ({ ...prev, [panelId]: crypto.randomUUID() }));
}
}, [panelId, sessionId, setTerminals]);
useEffect(() => {
return () => {
setTerminalsRef.current((prev) => {
const { [panelId]: _, ...rest } = prev;
return rest;
});
};
}, [panelId]);
const sessionId = useTerminalSession(panelId, terminalStateKey('host-terminals', workspace));
const [, setConnState] = useGlobal<TerminalConnectionState>(`terminal-conn-${panelId}`, 'disconnected');
const onConnectionChange = useCallback((state: TerminalConnectionState) => setConnState(state), [setConnState]);
src/workspaces/officerdev/src/apps/Terminal/RunningShells.tsx
+10 -5
View File
@@ -5,9 +5,14 @@ import { useClient } from 'hooks/useClient';
// Every shell the pty sidecar is holding, whether or not a panel is pointing at it.
//
// A terminal panel keeps its session id across unmounts so reopening re-attaches — which means a panel
// deleted for good leaves its shell running with nothing pointing at it. This is where those become
// visible and killable. Without it the only way to find one was curl.
// A terminal panel keeps its session id across unmounts so reopening re-attaches, and now kills the shell
// when the panel is genuinely closed (`usePanelClose`). This is the backstop for the rest: shells orphaned
// before that landed, ones whose kill request failed, and anything started outside a panel.
//
// `/_officer/…` is the sidecar's own prefix for the routes it exposes to the platform rather than to a
// terminal. The proxy strips `/api/terminal` and forwards the remainder verbatim, so leaving it out — as
// this file did — reaches the sidecar as `/sessions`, which it answers 404 to. That is a panel that always
// read "No shells running" and a kill button that quietly did nothing.
type PtySession = {
sessionId: string;
@@ -40,13 +45,13 @@ export const RunningShells = () => {
const { data, isLoading, refetch } = useQuery({
queryKey: ['terminal-sessions'],
queryFn: () => client.get<{ sessions: PtySession[] }>('/terminal/sessions'),
queryFn: () => client.get<{ sessions: PtySession[] }>('/terminal/_officer/sessions'),
refetchInterval: 5000,
});
const kill = useCallback(
async (sessionId: string) => {
await client.delete(`/terminal/sessions/${sessionId}`);
await client.delete(`/terminal/_officer/sessions/${sessionId}`);
// The shell dies asynchronously (the sidecar answers on its pty:exit broadcast, not to us), so give
// it a beat before asking again rather than showing a row that is already gone.
setTimeout(() => queryClient.invalidateQueries({ queryKey: ['terminal-sessions'] }), 300);
src/workspaces/officerdev/src/apps/Terminal/TerminalWrapper.tsx
+3 -27
View File
@@ -1,38 +1,14 @@
import { useCallback, useEffect } from 'react';
import { useCallback } from 'react';
import { useWorkspace } from '../../components/Workspace';
import { useDashboardState } from 'state/useDashboardState';
import { useGlobal } from 'hooks/useGlobal';
import type { TerminalConnectionState } from './Terminal';
import { TerminalView } from './Terminal';
import { terminalStateKey } from './state-key';
const EMPTY_TERMINALS: Record<string, string> = {};
import { useTerminalSession } from './use-terminal-session';
export const TerminalWrapper = ({ panelId }: { panelId: string }) => {
const { workspace, cwd } = useWorkspace();
const stateKey = terminalStateKey('terminals', workspace);
const { value: terminals, setValue: setTerminals } = useDashboardState<Record<string, string>>(
stateKey,
EMPTY_TERMINALS,
);
const sessionId = terminals[panelId];
useEffect(() => {
if (!sessionId) {
const id = crypto.randomUUID?.() ?? Math.random().toString(36).slice(2) + Date.now().toString(36);
setTerminals((prev) => ({ ...prev, [panelId]: id }));
}
}, [panelId, sessionId, setTerminals]);
// The panel → session mapping deliberately OUTLIVES the mount. It used to be deleted on unmount, so any
// layout or route change generated a fresh uuid on the way back and abandoned the previous shell — alive,
// unreachable, and never killed, because nothing sends pty:close. Keeping it means reopening a terminal
// panel re-attaches to the shell you left running, which is also what makes the sidecar's replay buffer
// worth having. The map is persisted dashboard state, so this survives a reload too.
//
// The cost is that a panel deleted for good leaves its entry behind; the running-shells list is how you
// find and kill those. Killing on unmount is not an option until the panel system can tell a real close
// from an incidental remount.
const sessionId = useTerminalSession(panelId, terminalStateKey('terminals', workspace));
const [, setConnState] = useGlobal<TerminalConnectionState>(`terminal-conn-${panelId}`, 'disconnected');
const onConnectionChange = useCallback((state: TerminalConnectionState) => setConnState(state), [setConnState]);
src/workspaces/officerdev/src/apps/Terminal/use-terminal-session.ts
+52
View File
@@ -0,0 +1,52 @@
import { useEffect } from 'react';
import { useClient } from 'hooks/useClient';
import { useDashboardState } from 'state/useDashboardState';
import { usePanelClose } from '../../components/Workspace';
const EMPTY_TERMINALS: Record<string, string> = {};
const newSessionId = (): string =>
crypto.randomUUID?.() ?? Math.random().toString(36).slice(2) + Date.now().toString(36);
/**
* The shell a terminal panel is attached to: minted on first mount, remembered until the panel is closed.
*
* The `panelId → sessionId` map deliberately outlives the component. It used to be dropped on unmount, so
* any layout or route change minted a fresh uuid and abandoned the previous shell — alive, unreachable,
* and never killed. Keeping it is what makes reopening a terminal re-attach to what you left running, and
* what makes the sidecar's replay buffer worth having. It is persisted dashboard state, so it survives a
* reload too.
*
* What was missing was the other end: a panel closed for good left its shell running with nothing pointing
* at it, and the only way to find one was the running-shells list. `usePanelClose` fires on a real close
* and on nothing else — not a drag, not a swap, not a mobile panel switch — so the shell can finally be
* killed with it. If that request fails the shell is still in the running-shells list, which is where it
* was before this hook existed; a lost shell is worse than a stale row, so the map entry goes either way.
*/
export function useTerminalSession(panelId: string, stateKey: string): string | undefined {
const client = useClient();
const { value: terminals, setValue: setTerminals } = useDashboardState<Record<string, string>>(
stateKey,
EMPTY_TERMINALS,
);
const sessionId = terminals[panelId];
useEffect(() => {
if (!sessionId) setTerminals((prev) => (prev[panelId] ? prev : { ...prev, [panelId]: newSessionId() }));
}, [panelId, sessionId, setTerminals]);
usePanelClose(panelId, () => {
if (!sessionId) return;
setTerminals((prev) => {
const { [panelId]: _drop, ...rest } = prev;
return rest;
});
client.delete(`/terminal/_officer/sessions/${sessionId}`).catch((err: unknown) => {
console.error(`[terminal] could not kill ${sessionId} after its panel closed`, err);
});
});
return sessionId;
}