fix(opencode): clear the activity marker when OpenCode 1 disposes the plugin - #4129
worktrunk-bot wants to merge 2 commits into
Conversation
worktrunk-bot
left a comment
There was a problem hiding this comment.
dispose clears the marker without waiting for the event hook's in-flight write, so on a fast exit the clear lands before the last set and leaves exactly the stale marker this PR removes.
The OpenCode 1 host dispatches the event hook fire-and-forget — in packages/opencode/src/plugin/index.ts the events.listen callback body is void hook["event"]?.({ event: ... }), with no await and no handle kept — so the wt config state marker set 💬 subprocess spawned by the final session.idle is still running when the finalizer calls dispose. Both write worktrunk.state.<branch>.marker; whichever wt finishes last decides the marker.
That gap is narrowest in #4128's own repro. opencode run "wait" goes idle and exits immediately, so the two spawns are separated by shutdown work rather than by user think-time.
Driving the returned hooks the way the host does — void event({type: "session.idle"}), then await dispose() — against a stub wt that records each invocation:
config state marker set 🤖
config state marker clear
config state marker set 💬 ← final state: the stale marker
There is a second hazard with the same root. Effect closes scope finalizers LIFO, and the dispose finalizer is registered after the one holding unsubscribe, so the event listener is still live while dispose runs; an event delivered in that window re-sets the marker after the clear.
The OpenCode 2 path already pays for the first of these, and says why — "Awaiting the watcher before clearing keeps an in-flight set from landing after the clear and leaving a marker behind." The server path has no equivalent because it holds nothing it can await. Chaining every marker call onto one promise gives it one: the clear enqueues last, and a disposed flag drops whatever the host still delivers.
I've pushed that fix. With it, both orderings above end on config state marker clear.
One call left open: there's no test harness for dev/*.ts in the repo, so neither the original dispose nor this ordering fix is pinned by anything — the checks here only assert the file is packaged and installed. Committing the Node harness would be a new test surface rather than a change to an existing one, so I've left that decision to you.
…ght write The OpenCode 1 host dispatches the `event` hook fire-and-forget, so the `marker set` subprocess it spawns for the last `session.idle` is still running when the scope finalizer calls `dispose`. Both write `worktrunk.state.<branch>.marker`, and whichever `wt` finishes last decides it — so the clear could land first and leave the stale marker this path was meant to remove, in the narrow exit-right-after-idle gap that `opencode run` produces. Every marker call now chains onto one promise, which is the handle the `server` path previously lacked: the clear enqueues behind whatever is outstanding. Effect closes scope finalizers LIFO and the `dispose` finalizer is registered after the one holding `unsubscribe`, so the listener is still live during teardown; `disposed` drops what arrives in that window rather than letting it re-set after the clear. The OpenCode 2 path already carried the equivalent guard, by awaiting its watcher before clearing.
Problem
On OpenCode 1.x the plugin's
serverexport drives the marker, and its only clearing path is thesession.deletedevent — which fires when a session is explicitly deleted, not when one ends normally. So a finished session leaves its lastset 💬in place andwt listkeeps showing a dead session as active untilwt config state marker clearis run by hand. Reported in #4128 against worktrunk 0.77.0 and OpenCode 1.18.18.The
serverpath does have a teardown hook; the plugin just wasn't implementing it. OpenCode's v1 plugin interface declaresdispose?: () => Promise<void>as the first member ofHooks, and the host calls it from anEffect.addFinalizerregistered alongside the event subscription inpackages/opencode/src/plugin/index.ts— the same place it tears the event listener down.disposehas been in that interface since v1.16.0, which is the floor theserverexport already targets, and older hosts ignore an unknown key, so there is no version gate to add.This also means the issue's suggested route — having OpenCode 1.18 honor
setup()'s return value as a teardown — is not the path: that loader is the v2 one, and the v1 hook object already carries the teardown.Solution
server()now returns adisposethat clears the marker, mirroring what the OpenCode 2setup()path already does in its returned teardown. The OpenCode 2 path is untouched.A
disposethat only callsclearisn't enough on its own, because the host dispatches theeventhook fire-and-forget —void hook["event"]?.({ event: ... }), with noawaitand no handle kept. Thewt config state marker set 💬subprocess spawned by the finalsession.idleis therefore still running when the finalizer callsdispose, both writeworktrunk.state.<branch>.marker, and whicheverwtfinishes last decides it. That gap is narrowest in the issue's own repro:opencode run "wait"goes idle and exits immediately.So every marker call chains onto one promise, which is the handle the
serverpath otherwise lacks — theclearenqueues behind whatever is outstanding. Effect closes scope finalizers LIFO and thedisposefinalizer is registered after the one holdingunsubscribe, so the listener is still live during teardown; adisposedflag drops what arrives in that window rather than letting it re-set after the clear.The doc sentence asserting that every plugin clears on session end was, for this path, false; it is true again with this change. The same sentence still said "all four plugins" while the list above it names five (Claude Code, Codex, OpenCode, Pi, Gemini), so that count is corrected in the same edit. The two
claude-code.mdmirrors are the sync test's regeneration of the primary indocs/src/content/docs/.Testing
Node's type stripping runs
dev/opencode-plugin.tsdirectly, so the hook shape and its subprocess calls are observable without OpenCode: with a stubwtonPATHrecording each invocation and its cwd,server({ directory })is called, fed events the way the host dispatches them, and then disposed the way the host's finalizer would.Before any of this the returned object exposed only
event, there was nothing for the finalizer to call, and the recorded calls ended atconfig state marker set 💬— the stale marker. Withdisposealone, dispatchingsession.idleunawaited and then disposing still ended onset 💬, because the clear overtook the in-flight set. With the ordering fix the recording ends onconfig state marker clearin both that case and the one where an event is delivered afterdisposehas begun.cargo test --test integration test_docs_are_in_sync, the 13opencodeintegration tests, andpackaged_assetspass.What this cannot verify from CI is the host half — that OpenCode actually closes that scope on a normal
opencode runexit rather than only on an explicit shutdown. That rests on reading the finalizer registration above, not on an observed run; the reporter offered to test a fix and is the fastest confirmation of the end-to-end behavior. ASIGKILLed process still skips the finalizer, which is the stale-marker caveat the docs already carry.There is also no test harness for
dev/*.tsin the repo, so none of this is pinned by CI — the checks here only assert that the plugin file is packaged and installed. Committing the Node harness would add a test surface rather than extend one, so it is left as a maintainer call.Closes #4128 — automated triage