Make the tool menu the selection surface - #78
Merged
Conversation
`/tools` built its rows from the list the CLI had already composed, so it could only subtract. Everything else CUA can offer — `playwright_execute`, the computer toolset, a provider-native surface the default did not pick, `browser_act` on a model whose default excluded it — was invisible. The menu is the product's differentiator and the CLI hid most of it. `cuaToolMenu(model, selected)` returns every offerable tool, each marked available or not. It decides availability by compiling the candidate catalog rather than restating the compiler's rules, so the menu cannot drift from what `compileCuaToolCatalog` accepts: a row is available exactly when selecting it compiles, and the reason shown when it is not is the compiler's own message. A test asserts that equivalence across models with a native browser surface, a native computer surface, neither, a quirk, and an id pi-ai's registry does not carry. Availability is relative to the current selection, because several rules are pairwise: two providers' native surfaces cannot coexist, and a native surface derives a transport the rest of the selection must suit. The picker therefore rebuilds its rows after every toggle rather than fixing them when it opens, unavailable rows cannot be staged, and bulk actions skip them. `cua tools` prints the same menu for a model without the TUI, with `--json`.
The ptywright scenario pinned `22/22 enabled`, from when the picker's rows were exactly the application-composed baseline. The footer now counts selectable rows, of which the baseline is the part enabled on open, so the scenario derives both numbers the same way the picker does and asserts ctrl+a grows the selection past the baseline rather than merely restoring it.
Contributor
Author
|
bugbot run |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
There are 3 total unresolved issues (including 2 from previous reviews).
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Ctrl+A wait matches prior footer
- I changed the Ctrl+A synchronization wait to
${selectable}/${selectable} enabled, which uniquely matches the post-select-all footer and eliminates the stale pre-keypress match race.
- I changed the Ctrl+A synchronization wait to
Or push these changes by commenting:
@cursor push ad56e4b65d
Preview (ad56e4b65d)
diff --git a/packages/cli/test/tui.fixture.test.ts b/packages/cli/test/tui.fixture.test.ts
--- a/packages/cli/test/tui.fixture.test.ts
+++ b/packages/cli/test/tui.fixture.test.ts
@@ -389,7 +389,7 @@
// ctrl+a enables every selectable row — including tools the application
// never composed — and ctrl+x clears it; both are staged.
session.press(KeyCtrlA);
- await session.waitForVisible(`/${selectable} enabled`, { timeoutMs: WAIT_MS });
+ await session.waitForVisible(`${selectable}/${selectable} enabled`, { timeoutMs: WAIT_MS });
await session.waitForStable(STABLE_MS, { timeoutMs: WAIT_MS });
const enabledAll = /(\d+)\/\d+ enabled/.exec(session.snapshot().visible);
assert.ok(enabledAll && Number(enabledAll[1]) > baseline, "ctrl+a grows the selection past the baseline");You can send follow-ups to the cloud agent here.
Three things, all real: A row that is already installed now contributes the exact spec object in use rather than a freshly built one. The CLI enables `javascript` on Anthropic's native browser, and rebuilding the catalog from the menu's own specs would have dropped `enable_javascript_exec` on a no-op apply or a reset-then-apply. `ctrl+r` now restages like every other mutation. Toggle and the bulk actions recompute availability against the staged set; reset did not, so rows could keep availability from the previous stage while the footer already showed defaults. The fixture's `ctrl+a` wait could resolve on the pre-keypress screen, since `waitForVisible` is a substring check and the footer already contained `/<selectable> enabled`. It now settles the screen and asserts the count grew.
Contributor
Author
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit d0371a1. Configure here.
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.

Summary
Phase 3.
/toolsbuilt its rows from the list the CLI had already composed, so it could only subtract:Everything else CUA can offer was invisible —
playwright_execute, the computer toolset, a provider-native surface the default didn't pick,browser_acton a model whose default excluded it. The tool menu is the differentiator and the CLI hid most of it.The menu decides availability by compiling
cuaToolMenu(model, selected)returns every offerable tool, each marked available or not. It does not restate the compiler's rules — it compiles the candidate catalog and reports the result:So the menu cannot drift from what
harness.setTools()accepts: a row is available exactly when selecting it compiles, and the reason shown is the compiler's own message.compileCuaToolCatalogis documented as pure and declaration-only — it builds no executable tools and retains none of its inputs — so probing per entry is cheap and side-effect-free.A test asserts that equivalence directly, across a model with a native browser surface, one with a native computer surface, one with neither, one carrying a quirk, and one synthesized from an id pi-ai's registry doesn't carry.
Availability is pairwise, so rows are re-evaluated as you stage
Two providers' native surfaces can't coexist, and a native surface derives a transport the rest of the selection must suit. A per-tool boolean computed once can't express that. The picker rebuilds its rows after every toggle, unavailable rows can't be staged, and bulk actions skip them.
cua toolsSame menu, no TUI, with
--json:Testing
npm run typecheckclean; 94 / 296 / 142 tests pass./toolstests were rewritten rather than deleted, and gained one that pins the new capability: enablingplaywright_executeon a model whose baseline never included it.ctrl+rrestoring model defaults, staged-until-confirm, the mutation queue against a/modelswitch, and refusal to open mid-turn.One caveat worth flagging: a vitest file-level collect failure reports as "Test Files 1 failed" without any
×line, so a grep watching only for failed assertions can miss it entirely. I hit that mid-change and only caught it because the total test count dropped.Note
Medium Risk
Expands which tools a session can enable and changes transport/catalog combinations users can stage, though
harness.setTools()still compiles before applying; mistakes affect agent behavior rather than security boundaries.Overview
cuaToolMenu(model, selected)in@onkernel/cua-ailists every offerable CUA tool for a model and marks each row available only when adding it to the current selection would makecompileCuaToolCatalogsucceed, surfacing the compiler’s error when it would not. Availability is relative to the staged selection (native surfaces, transport pinning), so callers rebuild the menu after each change.The CLI
/toolspicker no longer mirrors only the composed baseline: it shows the full menu plus application tools, lets users add tools the app never composed (e.g.playwright_execute, computer tools), shows unavailable rows with reasons, and restages rows after every toggle/bulk action. Applies use menu order viatoolsForSelection;ctrl+rstill restores model defaults.cua toolsprints the same menu to stdout (--jsonoptional).describeMenupreserves live spec instances for already-installed tools (e.g. Anthropic native browser options) so a no-op apply does not drop configuration.Reviewed by Cursor Bugbot for commit d0371a1. Bugbot is set up for automated code reviews on this repo. Configure here.