Skip to content

fix(dsh-plugin): re-arm lazy tools from history after a plugin reload - #283

Open
drakeo338 wants to merge 1 commit into
Tencent:mainfrom
drakeo338:fix/269-rearm-lazy-tools-after-plugin-reload
Open

drakeo338 wants to merge 1 commit into
Tencent:mainfrom
drakeo338:fix/269-rearm-lazy-tools-after-plugin-reload

Conversation

@drakeo338

Copy link
Copy Markdown

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/result is live only — it needs a fresh skill invocation, which is
    exactly what you don't have after a restart.
  • session/created never fires for a session that already existed before the
    reload.
  • the boot scan runs once inside apply(), and ctx.get("sessions") returns
    nothing 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 as unknown tool.

The thing that made this feel like the right fix rather than a patch: the
session/event handler already receives the session, and was ignoring it
(_session). The proof is sitting right there on every event. Reading its
history 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 reveal
the 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 lint runs (biome, stylelint, the package
typecheck 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, vom
and ext:test, and pnpm lint does chain the dsh-plugin suite, but if that's
incidental 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.

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.
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.

[Bug][dsh-plugin] browser_* tools stay unregistered after a plugin reload — lazy reveal is not restored (unknown tool "browser_session")

1 participant