Conversation
`session.start --browser` takes an extension instance id or a unique label, and rejects a selector that matches nothing with `not_found` / "requested browser is not connected". That error carried no payload, so the caller could not tell a typo from an offline target, and could not see which instances were actually connected. The two sibling outcomes of the same selector resolution already carry their candidates: `multiple_browsers_online` emits a `browsers` snapshot and the ambiguous-label case emits `instance_ids`, and the CLI already renders a connected-browsers table and a candidate bullet list for those two. `not_found` was the third case, and the only one with nothing to show. Attach the same snapshot to `BrowserNotFound`, which becomes a struct variant, and render the connected-browsers table for it. The message is unchanged, and an empty registry still emits no `data` payload, so that wire shape and the existing `details:` line are untouched.
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.
What
When
session.startis given a--browserselector that matches no connected browser, the daemon returnsnot_found/requested browser is not connectedwith no payload, so the caller learns only that the selector was wrong — not what would have been right, and not whether the target is offline or the selector was a typo.This attaches the same connected-browser snapshot that a sibling error already carries, and renders it the same way.
Output below is copied from the new test's rendered stderr, not hand-written:
Why this shape
not_foundis one of three outcomes of the same selector resolution, and the only one that carries nothing:multiple_browsers_online{ "browsers": [...] }invalid_params){ "instance_ids": [...] }not_foundThe
RenderExtrashook incli/session.rsenumerates the first two, so the third reads as a case that was never considered rather than one deliberately left bare. This makes it match the two that already exist, reusing the same snapshot helper (snapshot_status_entries) and the same table renderer.On
bsk browsersThe
hint:line already tells the reader to runbsk browsersto see current state, anddocs/browser-profiles.mddocuments the same discovery step, so this adds no new mechanism and does not replace either. What it changes is only that the information arrives at the point of the error instead of one command later — which matters for the two cases that doc calls out, verifying a mapping and dealing with an offline target, because at the moment of this error neither can be done without first running something else. It also makes an interactive typo (alhpa) distinguishable from an offline-but-correct instance, which it currently is not.The message, the
details:line, thehint:line and the ordering contract are unchanged; a new test asserts the summary → extras → hint order explicitly.Backward compatibility
BrowserNotFoundgains a field, so it changes from a unit variant to a struct variant. That is a source-level change for anyone matching on it exhaustively, though this is the CLI binary rather than a published library.datapayload is emitted at all and that wire shape is unchanged. A test assertsdata.is_none()for exactly that case.Tests
Four new tests, all green locally:
daemon::ipc::selector_miss_payload_tests::browser_selector_miss_lists_connected_candidates— the payload, and that the message is unchanged.daemon::ipc::selector_miss_payload_tests::browser_selector_miss_without_browsers_keeps_message_only_payload— the empty-registry shape.cli::session::i3_tests::selector_miss_extras_render_candidate_table— the rendered table, including the summary → extras → hint order.cli::session::i3_tests::selector_miss_with_empty_candidates_renders_no_extras— additive for existing output.The daemon-side module is deliberately placed outside that file's
#[cfg(all(test, unix))]transport module, so it runs on Windows as well.Local verification, and a caveat about Windows
Two pre-existing failures and four pre-existing clippy errors show up on native Windows and are not from this change — I confirmed the test failures reproduce with this commit stashed, and none of the clippy sites are in this diff:
skill_install::harness::tests::{detects_hermes_from_home_layout, skills_dirs_match_harness_spec}fail on Windows. Filed separately as Two Hermes skills-dir tests fail on native Windows: they assume ~/.hermes, but the resolver documents %LOCALAPPDATA%\hermes #312.cargo clippy --workspace --all-targets --locked -- -D warningsreportsdaemon/file_transfer.rs:417,daemon/file_transfer.rs:426,skill_install/harness.rs:339anddaemon/ipc.rs:743. The first three are in files this change does not touch, and the last is a function that this change neither added nor removed a caller for.Both are Windows-only and CI's
rustjob runs onubuntu-latest, so neither is visible upstream. I left them alone rather than widening this diff to unrelated code.Related
Refs #213 — that report describes having to try profiles one at a time until one works. This does not fix an internal retry loop; selection is a single match and I could not find a loop to fix. It removes the need to guess, by naming the candidates at the miss.
Prepared with AI assistance; the verification above was run locally and the failures reported are real observed output.