fix(terminal): reconcile project action executions across clients (#3362)
This commit is contained in:
committed by
GitHub
parent
4e0eed717d
commit
d37ce34a2e
@@ -2,7 +2,7 @@
|
||||
|
||||
## Ownership
|
||||
|
||||
`runtime.js` owns terminal identity, PTY processes, launch mode, session purpose, ordered output, bounded scrollback, WebSocket attachments, and lifecycle routes. `shells.js` discovers executable shell families, resolves the persisted shell ID, and builds the per-shell argv for interactive versus command launches. Clients own tab arrangement and choose stable terminal IDs. Electron uses this same runtime in-process; VS Code returns an explicit unsupported error.
|
||||
`runtime.js` owns terminal identity, PTY processes, launch mode, session purpose, ordered output, bounded scrollback, WebSocket attachments, and lifecycle routes. `shells.js` discovers executable shell families, resolves the persisted shell ID, and builds the per-shell argv for interactive versus command launches. Clients own tab arrangement. Interactive terminals use stable IDs; project actions keep a stable UI tab and allocate a fresh terminal ID for each command execution. Electron uses this same runtime in-process; VS Code returns an explicit unsupported error.
|
||||
|
||||
## Protocol
|
||||
|
||||
@@ -33,7 +33,8 @@ HTTP remains the authenticated command plane for create, resize, appearance upda
|
||||
- `GET /api/terminal/shells` reports shell IDs available on the active server using the same augmented PATH provided to spawned PTYs, plus whether each executable has a supported login-mode argument. `auto` preserves environment/platform fallback order; an explicit unavailable shell fails creation instead of silently running a different shell. Login mode is opt-in and uses only built-in arguments for known shells. Interactive shells still launch as before. Command-mode launches reuse the same environment and login support, but switch argv by shell family: POSIX and Fish use interactive `-c`, Nushell uses `-c`, PowerShell uses `-Command`, and cmd uses `/d /s /c`. Preference changes affect new sessions and explicit restarts, not running PTYs.
|
||||
- PTY data and exit callbacks enter one FIFO queue. The runtime wires those listeners in the same synchronous turn that receives the PTY object. `node-pty` and `bun-pty` both expose the PTY before dispatching registered callbacks. If a backend emitted exit before listener registration, this layer could not recover it, so the wiring stays adjacent to PTY creation.
|
||||
- Scrollback is retained on the server and capped at 512 KiB with UTF-8-safe trimming. Device-status, device-attribute, cursor-position reply, and color-query exchanges are removed from replay history with incomplete control sequences carried across PTY chunks; live output remains byte-for-byte unchanged.
|
||||
- Exited sessions remain attachable until explicit close or idle cleanup.
|
||||
- Exited sessions remain attachable until explicit close, idle cleanup, or a successful replacement of the same project action. Creating a replacement retires only exited records for the same resolved directory and action, after the new PTY starts. Failed creation preserves the old record and output. These replaced records do not exhaust the terminal capacity limit.
|
||||
- Deduplicated create responses may describe another client's execution. Cancellation cleanup closes only the terminal ID allocated for the cancelled request; it never closes an adopted peer execution.
|
||||
- Restarts are serialized per terminal. Each restart spawns and wires the replacement before terminating the old process, retaining the terminal ID. Command-mode sessions reject restart with HTTP 400 instead of silently turning into interactive shells with stale action metadata.
|
||||
- A delete that arrives while create is still pending leaves a cancellation tombstone. When the PTY arrives, the runtime terminates it immediately, never inserts the session into the live map, and returns a create error while the delete still succeeds.
|
||||
- Close uses SIGTERM with bounded SIGKILL escalation. Force-kill, idle cleanup, and runtime shutdown terminate process groups immediately where supported. Removal explicitly sends a fatal scoped closure and evicts client projections even when a PTY backend fails to emit `onExit`; attached terminals are not considered idle.
|
||||
|
||||
@@ -310,7 +310,11 @@ export function createTerminalRuntime({
|
||||
applyAppearance(session, { themeMode, terminalBackground, terminalForeground });
|
||||
return session;
|
||||
}
|
||||
if (!existing && sessions.size + pendingSessionCreates.size >= MAX_SESSIONS) throw new Error('Maximum terminal sessions reached');
|
||||
const superseded = normalizedPurpose.type === 'project-action'
|
||||
? [...sessions.values()].filter((session) => session.id !== id && session.status === 'exited'
|
||||
&& path.resolve(session.cwd) === resolvedCwd && isPurposeActionMatch(getSessionPurpose(session), normalizedPurpose))
|
||||
: [];
|
||||
if (!existing && sessions.size - superseded.length + pendingSessionCreates.size >= MAX_SESSIONS) throw new Error('Maximum terminal sessions reached');
|
||||
const pendingEntry = { cwd: resolvedCwd, shell: normalizedShell, loginShell, mode: launchMode.mode, command: launchMode.command, purpose: normalizedPurpose, cancelled: false, promise: null };
|
||||
const creation = (async () => {
|
||||
const session = existing ?? { id, sequence: 0, history: '', pendingHistoryControlSequence: '', pendingThemeControlSequence: '', eventQueue: [], draining: false, createdAt: Date.now() };
|
||||
@@ -320,6 +324,11 @@ export function createTerminalRuntime({
|
||||
await terminateProcess(ptyProcess, true);
|
||||
throw new Error('Terminal session was closed during creation');
|
||||
}
|
||||
for (const previous of superseded) {
|
||||
if (sessions.get(previous.id) !== previous || previous.status !== 'exited') continue;
|
||||
sessions.delete(previous.id);
|
||||
closeAttachments(previous.id, 'SUPERSEDED', 'Terminal replaced by a new action run');
|
||||
}
|
||||
sessions.set(id, session);
|
||||
return session;
|
||||
})();
|
||||
|
||||
@@ -191,6 +191,45 @@ describe('terminal runtime', () => {
|
||||
return { routes, processes, runtime };
|
||||
};
|
||||
|
||||
it('replaces only exited action runs and keeps session capacity available across reruns', async () => {
|
||||
const harness = createHarness();
|
||||
try {
|
||||
const create = harness.routes.post.get('/api/terminal/create');
|
||||
await create({ body: { sessionId: 'interactive', cwd: '/repo' } }, createResponse());
|
||||
for (let index = 0; index < 25; index += 1) {
|
||||
const id = `execution-${index}`;
|
||||
const response = createResponse();
|
||||
await create({ body: { sessionId: id, cwd: '/repo', mode: 'command', command: 'echo hello', purpose: { type: 'project-action', actionId: 'build', executionId: id } } }, response);
|
||||
expect(response.statusCode).toBe(200);
|
||||
const listed = createResponse();
|
||||
harness.routes.get.get('/api/terminal/sessions')({ query: { cwd: '/repo' } }, listed);
|
||||
expect(listed.body.sessions.map(session => session.sessionId)).toEqual(['interactive', id]);
|
||||
expect(harness.processes[0].killed).toBe(false);
|
||||
harness.processes.at(-1).emitExit(0);
|
||||
}
|
||||
} finally { await harness.runtime.shutdown(); }
|
||||
});
|
||||
|
||||
it('retains completed output when the replacement command fails to start', async () => {
|
||||
let available = true;
|
||||
const harness = createHarness({ fs: { promises: { stat: async () => ({ isDirectory: () => available }) } } });
|
||||
try {
|
||||
const create = harness.routes.post.get('/api/terminal/create');
|
||||
const options = { cwd: '/repo', mode: 'command', command: 'echo hello', purpose: { type: 'project-action', actionId: 'build', executionId: 'old' } };
|
||||
await create({ body: { ...options, sessionId: 'old' } }, createResponse());
|
||||
harness.processes[0].emitData('old output');
|
||||
harness.processes[0].emitExit(0);
|
||||
available = false;
|
||||
const response = createResponse();
|
||||
await create({ body: { ...options, sessionId: 'new', purpose: { ...options.purpose, executionId: 'new' } } }, response);
|
||||
expect(response.statusCode).toBe(400);
|
||||
const listed = createResponse();
|
||||
harness.routes.get.get('/api/terminal/sessions')({ query: { cwd: '/repo' } }, listed);
|
||||
expect(listed.body.sessions.map(session => session.sessionId)).toEqual(['old']);
|
||||
expect(harness.processes).toHaveLength(1);
|
||||
} finally { await harness.runtime.shutdown(); }
|
||||
});
|
||||
|
||||
it('rejects regular files as terminal working directories', async () => {
|
||||
const postRoutes = new Map();
|
||||
const app = {
|
||||
|
||||
Reference in New Issue
Block a user