Skip to content

fix(dsh-plugin): show the connected browsers a selector miss reports - #404

Open
PerryLink wants to merge 1 commit into
Tencent:mainfrom
PerryLink:fix/dsh-plugin-selector-miss-candidates
Open

PerryLink wants to merge 1 commit into
Tencent:mainfrom
PerryLink:fix/dsh-plugin-selector-miss-candidates

Conversation

@PerryLink

Copy link
Copy Markdown
Contributor

#313 taught the CLI to list the connected browsers when a selector matches nothing, so a caller can pick a real instance or label instead of probing every browser in turn. The list reaches the JSON envelope; it does not reach a DSH user.

bsk --json reports the miss as NotFound with the candidates in data.browsers:

{ "code": "not_found", "message": "requested browser is not connected", "data": { "browsers": [ … ] } }

parseBskJson reads code, message and hint and drops data, and BskError has no field to carry it, so the candidates stop there. Putting them on the error object would not be enough either: the tool layer re-throws and the host shows error.message, so the list has to travel in the message.

Change

One helper, used in the place the CLI's own renderer puts the table — between the summary and the hint:

bsk session start failed: requested browser is not connected; connected browsers: alpha (chrome 131, label "Personal", 0 sessions); beta (edge 130, 1 session) (hint: run bsk browsers to list them)

It is deliberately one line. tools.ts keeps error.message.split("\n")[0] for the action card, so a multi-line list would be truncated in the UI while looking complete in the transcript.

Only data.browsers renders. Every other envelope is untouched, including the two that also use data — session_busy's reason, which drives the one-shot retry, and the ambiguous-label ids. An empty list renders nothing, matching the CLI, which leaves a miss against an empty registry unrendered too; an entry with no instance_id is skipped rather than printed as a blank row.

Verification

The two revisions were executed side by side over twelve envelopes. Two messages changed — the selector miss, and one case with a malformed candidate — and the other ten are byte-identical, covering session_busy, the fill_value_mismatch envelope, timeouts, killed-by-interrupt children, non-JSON stderr, and both success paths.

Three cases were added to tests/runner.test.ts: the candidate list (asserting the wording, the summary → candidates → hint order, and that the result is a single line), an empty list, and a data payload that is not a candidate list.

I have not run the package's own vitest here — the plugin's build step is what I could not reproduce locally — so the added cases are written to the file's existing conventions rather than reported green. The envelope comparison above runs the real runner.ts for both revisions.

Thanks again for #313; this is the follow-up you flagged in review.

A selector miss surfaces as `bsk session start failed: requested browser is not
connected (hint: run bsk browsers)`. The CLI prints the connected browsers, but a
DSH tool result keeps only the message, so the model names a browser that is not
connected and misses again. Append the `data.browsers` list the error already
carries to the message the model sees.

Rebuilt on current main: the `withHint` assembly keeps the newer `dialog_pending`
branch that landed after this branch was written.
@PerryLink
PerryLink force-pushed the fix/dsh-plugin-selector-miss-candidates branch from 0f19398 to 5078078 Compare November 2, 2026 12:41
@PerryLink

Copy link
Copy Markdown
Contributor Author

Rebuilt on current main (7840c46) — one conflict, three lines.

runner.ts gained a dialog_pending branch and switched withHint to let after this branch was written; the rebuilt hunk keeps that branch and appends the browser list on top of it. The rest of the change and the whole test block applied unchanged. I also added the CHANGELOG.md entry under Unreleased → Fixed, which the original branch did not carry.

Local runs on the rebuilt tree:

vitest run tests/runner.test.ts        -> 77 passed  (74 before the test block)
runner.test.ts + runner-exit.test.ts + queue.test.ts -> 101 passed

The new case fails on clean main with expected 'bsk session start failed: requested b…' to contain 'alpha (chrome 131, label "Personal", …', so it still pins the behaviour: the tool result keeps only the message, which is why the connected browsers have to be in it.

Unchanged on main as of this rebuild: describeBrowser / data.browsers are still unused in the plugin's src/, and crates/bsk-cli/src/cli/session.rs:449-460 still emits the same data.browsers[] shape this renders.

This branch has not been deployed

No deployments
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