Conversation
After a daemon restart that reloads the plugin, browser_* calls fail with
unknown tool "browser_session" until the model happens to invoke the skill
again, even though the session's durable history already proves it ran.
None of the three triggers covers that case. The live tools/result hook needs
a fresh invocation. session/created never fires for a session that already
exists. The boot scan runs once during apply(), and ctx.get("sessions") yields
nothing when the sessions service is registered after this plugin, so it covers
nothing at all.
session/event already receives the session and ignored it. Reading its history
when nothing else has revealed the suite closes the gap, and is guarded so the
scan stops once revealed — these events are frequent and re-deriving on each
one after the reveal is waste.
Keeps the reveal derived from durable history rather than adding a persisted
flag, so there is no new state to migrate or keep consistent.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #269.
The reporter's diagnosis was right, so I mostly went looking for which of the
three triggers was supposed to cover a reload and why it doesn't.
None of them does, it turns out:
tools/resultis live only — it needs a fresh skill invocation, which isexactly what you don't have after a restart.
session/creatednever fires for a session that already existed before thereload.
apply(), andctx.get("sessions")returnsnothing when the sessions service happens to be registered after this plugin.
When that's the case it covers nothing at all.
So the suite stays hidden until the model invokes the skill again, and in the
meantime every
browser_*call comes back asunknown tool.The thing that made this feel like the right fix rather than a patch: the
session/eventhandler already receives the session, and was ignoring it(
_session). The proof is sitting right there on every event. Reading itshistory while the suite is still hidden closes the gap without any new state.
It's guarded so the scan stops once the suite is revealed — session events are
frequent and re-deriving from history on each one afterwards would be pure
waste. There's a test for that guard specifically, using a session object that
throws if its history is read.
On the alternative in the issue: I deliberately didn't persist a reveal flag
per session. It would work, but it adds a piece of state that has to be written,
migrated and kept consistent with the event log, and the event log is already
the source of truth. Deriving stays cheaper as long as the derive is actually
reached, which is the bug rather than an argument against the approach.
I also didn't touch
tools/call. Arming before the result returns would revealthe suite for an invocation that then fails, which is a behaviour change to the
"successful invocation" rule rather than a reload fix.
Three tests added. The reload one is a real discriminator — with the fix
stashed it fails and the other 256 pass. The other two guard against ways this
could go wrong rather than reproducing the bug: one checks an event whose
session has no proof still reveals nothing, the other checks the post-reveal
guard.
Verified with the commands
pnpm lintruns (biome, stylelint, the packagetypecheck and its suite): 254 → 257 passing, 15 files, no pre-existing
failures either side.
One thing worth flagging separately, since I hit it while setting up: CI
doesn't appear to run this package's tests. The frontend job runs
i18n,vomand
ext:test, andpnpm lintdoes chain the dsh-plugin suite, but if that'sincidental rather than intended then these 257 tests are only protected by
whoever runs them locally. Happy to open a separate issue if that's useful.