Skip to content

fix(plugin): handle session deletion and disposal - #73

Open
dhaern wants to merge 1 commit into
shekohex:mainfrom
dhaern:fix/session-deletion-cleanup
Open

dhaern wants to merge 1 commit into
shekohex:mainfrom
dhaern:fix/session-deletion-cleanup

Conversation

@dhaern

@dhaern dhaern commented Oct 5, 2026

Copy link
Copy Markdown

Summary

The plugin misses its cleanup in two places. In V2, deleting a session leaves its PTYs running because nothing listens for session.deleted. In V1, unloading the plugin leaves the PTY web server running and its origin record on disk because the plugin has no dispose hook.

In plain terms: delete a session in OpenCode 2 and the terminals it started keep running. When the host unloads the V1 plugin, its web server and published address stay behind. Now deleting a V2 session cleans up its terminals, and unloading V1 shuts its server down.

Cause

  • V1 handles session.deleted in its event hook (src/plugin.ts:72) and calls adapter.onSessionDeleted, which runs manager.cleanupBySession. The V2 adapter has the same method and nothing calls it.
  • PTYServer already implements [Symbol.dispose]: it stops the server, closes the callback manager and removes the origin record. V1 never called it.
  • PluginV2.setup was typed Promise<void> | void, so it could not return a teardown callback.

Changes

  • plugin.ts (V1): dispose: async () => ptyServer?.[Symbol.dispose](). PTYs are left alone.
  • v2/index.ts: subscribe to ctx.event with an AbortController and call adapter.onSessionDeleted(event.data.sessionID) for each session.deleted. setup returns () => events.abort(). A subscription failure is logged unless the abort caused it.
  • v2/types.ts: add event to PluginContextV2 and type the return of setup after the SDK's Plugin.setup.

The V2 teardown only aborts the subscription and does not stop the server. The server and the PTYs it lists are shared by every location in the process, and the host unloads a location after an hour without session events. Stopping the server there would take the web UI down for PTYs that are still running.

Behavior changes

  • V2: PTYs started by a deleted session are killed and removed, the same as V1 already does.
  • V1: unloading the plugin stops the web server and removes its origin record.

Validation

Three new tests in test/plugin-lifecycle.test.ts: V1 dispose removes the origin record, V2 session.deleted cleans the PTYs of that session, and V2 teardown aborts the subscription and leaves the shared server running. All three fail on main.

Three mutations each fail one test: removing dispose, ignoring session.deleted in V2, and returning a no-op teardown in V2.

bun test over test/*.test.ts, without the npm-pack and live suites, gives 187 passing. bun run typecheck, bun run lint and bunx biome format . are clean. The CI workflow passes on this commit in my fork, including the Playwright e2e. I did not delete a session or unload the plugin in a live host.

Diff

Production: +19 lines (+20/-1) in src/plugin.ts, src/v2/index.ts and src/v2/types.ts.
Tests: +89 lines, one new file (test/plugin-lifecycle.test.ts).

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant