From a960a40d6a72c72b6703088187d5959eb0a99920 Mon Sep 17 00:00:00 2001 From: dhaern Date: Mon, 5 Oct 2026 14:12:31 +0000 Subject: [PATCH] fix(plugin): handle session deletion and disposal Wire V2 session deletion to PTY cleanup and return SDK-typed teardown from PluginV2.setup. Dispose V1 servers on unload; keep PTYs alive across plugin reloads. The V2 cleanup only aborts the event subscription. The server and the PTYs it shows are shared by every location in the process, and the host unloads a location after an hour without session events, so stopping the server there would kill the web UI of the other locations while the PTYs keep running. --- src/plugin.ts | 1 + src/v2/index.ts | 17 +++++++ src/v2/types.ts | 3 +- test/plugin-lifecycle.test.ts | 89 +++++++++++++++++++++++++++++++++++ 4 files changed, 109 insertions(+), 1 deletion(-) create mode 100644 test/plugin-lifecycle.test.ts diff --git a/src/plugin.ts b/src/plugin.ts index 6365c033..fd730fc9 100644 --- a/src/plugin.ts +++ b/src/plugin.ts @@ -18,6 +18,7 @@ export const PTYPlugin = async (context: PluginContext): Promise = let ptyServer: PTYServer | undefined return { + dispose: async () => ptyServer?.[Symbol.dispose](), 'command.execute.before': async (input) => { if (input.command !== ptyOpenClientCommand && input.command !== ptyShowServerUrlCommand) { return diff --git a/src/v2/index.ts b/src/v2/index.ts index ad044831..52fed0c5 100644 --- a/src/v2/index.ts +++ b/src/v2/index.ts @@ -61,6 +61,23 @@ export const Plugin: PluginV2 = define({ if (ctx.options?.autostart) { await getOrCreateServer(serverOptions) } + + const events = new AbortController() + const eventDomain = ctx.event + if (typeof eventDomain?.subscribe === 'function') { + void (async () => { + for await (const event of eventDomain.subscribe({ signal: events.signal })) { + if (event.type === 'session.deleted') { + adapter.onSessionDeleted?.(event.data.sessionID) + } + } + })().catch((error) => { + if (!events.signal.aborted) console.error('[opencode-pty] event subscription failed', error) + }) + } + // The server is left running: like the PTYs it shows, it is shared by every location in the + // process, and the host unloads a location after an hour without session activity. + return () => events.abort() }, }) diff --git a/src/v2/types.ts b/src/v2/types.ts index 0ea72205..56cbf249 100644 --- a/src/v2/types.ts +++ b/src/v2/types.ts @@ -60,6 +60,7 @@ export interface ToolDraft { export interface PluginContextV2 { readonly options?: OpencodePtyOptions & Record + readonly event?: Plugin.Context['event'] readonly command?: { transform( callback: (commands: CommandDraft) => Promise | void @@ -84,7 +85,7 @@ export interface PluginContextV2 { export interface PluginV2 { readonly id: string - readonly setup: (context: PluginContextV2) => Promise | void + readonly setup: (context: PluginContextV2) => ReturnType } export function define(plugin: PluginV2): PluginV2 { diff --git a/test/plugin-lifecycle.test.ts b/test/plugin-lifecycle.test.ts new file mode 100644 index 00000000..96230fd8 --- /dev/null +++ b/test/plugin-lifecycle.test.ts @@ -0,0 +1,89 @@ +import { afterEach, describe, expect, mock, spyOn, test } from 'bun:test' +import { PTYPlugin } from '../src/plugin.ts' +import { manager } from '../src/plugin/pty/manager.ts' +import { setPermissionAuthorizer } from '../src/plugin/pty/permissions.ts' +import type { PluginContext } from '../src/plugin/types.ts' +import { originsForProject } from '../src/shared/runtime.ts' +import { Plugin, getActiveServer, stopActiveServer } from '../src/v2/index.ts' +import type { PluginContextV2 } from '../src/v2/types.ts' +import { PTYServer } from '../src/web/server/server.ts' + +type EventDomain = NonNullable +type HostEvent = + ReturnType extends AsyncIterable ? Event : never + +function createEvents() { + let pending = Promise.withResolvers>() + const subscribe = mock((options?: Parameters[0]) => { + options?.signal?.addEventListener('abort', () => pending.reject(options.signal?.reason), { + once: true, + }) + return { + [Symbol.asyncIterator]: () => ({ next: () => pending.promise }), + } + }) + return { + subscribe, + emit(event: HostEvent) { + const current = pending + pending = Promise.withResolvers>() + current.resolve({ done: false, value: event }) + }, + } +} + +afterEach(() => { + mock.restore() + stopActiveServer() + manager.setNotifier(null) + setPermissionAuthorizer(null) +}) + +describe('Plugin lifecycle', () => { + test('v2 cleans up PTYs when their parent session is deleted', async () => { + const events = createEvents() + const cleanupBySession = spyOn(manager, 'cleanupBySession') + const cleanup = await Plugin.setup({ event: events }) + try { + events.emit({ type: 'session.deleted', data: { sessionID: 'session-deleted' } } as HostEvent) + await Promise.resolve() + expect(cleanupBySession).toHaveBeenCalledWith('session-deleted') + expect(cleanupBySession).toHaveBeenCalledTimes(1) + } finally { + if (typeof cleanup === 'function') await cleanup() + } + }) + + test('v2 cleanup aborts the event subscription and leaves the shared server running', async () => { + const events = createEvents() + const cleanup = await Plugin.setup({ + event: events, + options: { autostart: true, port: 0, hostname: '127.0.0.1' }, + }) + const server = getActiveServer() + expect(server).not.toBeNull() + if (typeof cleanup === 'function') await cleanup() + expect(getActiveServer()).toBe(server) + expect(events.subscribe.mock.calls[0]?.[0]?.signal?.aborted).toBe(true) + }) + + test('v1 dispose removes the server origin record', async () => { + const directory = '/test/plugin-lifecycle' + using server = await PTYServer.createServer({ directory, port: 0, hostname: '127.0.0.1' }) + spyOn(PTYServer, 'createServer').mockResolvedValue(server) + const hooks = await PTYPlugin({ + client: { session: { prompt: mock(async () => {}) } }, + directory, + worktree: directory, + } as unknown as PluginContext) + await expect( + hooks['command.execute.before']?.( + { command: 'pty-show-server-url', sessionID: 'session-v1', arguments: '' }, + { parts: [] } + ) + ).rejects.toThrow('Command handled by PTY plugin') + expect(originsForProject(directory)).toHaveLength(1) + await hooks.dispose?.() + expect(originsForProject(directory)).toEqual([]) + }) +})