fix(init): make the universal tool target findable in the picker - #1778
fix(init): make the universal tool target findable in the picker#1778clay-good wants to merge 3 commits into
Conversation
Closes #653 `openspec init`'s tool picker is a searchable list of product names. The vendor-neutral target every unlisted assistant is meant to use was named "Shared .agents skills" — after the directory it writes, which is not a word anyone in that position searches for. Typing "universal", "other" or "generic" returned "No matches", so the escape hatch was unreachable and the reporter had to open an issue to find it. Rename the entry to "Other / Universal (shared .agents skills)" and give choices optional `searchAliases` the filter also matches. The picker also dropped every non-alphanumeric keystroke: readline reports punctuation only in `key.sequence`, leaving `key.name` undefined, so ".agents" and "amazon-q" could not be typed at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Deploying openspec-docs with
|
| Latest commit: |
d36b78c
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://76644796.openspec-docs.pages.dev |
| Branch Preview URL: | https://claude-openspec-issue-triage-rste.openspec-docs.pages.dev |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthroughThe universal ChangesUniversal tool picker
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The picker, alias, input-handling, guidance, test, and documentation changes are consistent and appear ready to merge. Sequence Diagram(s)sequenceDiagram
participant User
participant InitCommand
participant searchableMultiSelect
User->>InitCommand: Open tool selection
InitCommand->>searchableMultiSelect: Provide choices with aliases and emptyHint
User->>searchableMultiSelect: Enter alias, punctuation, or pasted text
searchableMultiSelect-->>User: Display matching tools or universal-tool guidance
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Linked Issues checkExplanation The changes satisfy issue Full details: Out of Scope Changes checkExplanation The changes remain within scope. The picker input handling, update-picker alias support, unknown-tool guidance, tests, and documentation directly support the universal tool discovery objective and related hardening described in the pull request. Full details: Docstring CoverageExplanation Docstring coverage is 45.45% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 11 functions across 8 files. (3 skipped: 3 unsupported.)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…othing "No matches" is where someone whose assistant is not on the list gives up — the picker knows the answer and does not say it. Add an optional `emptyHint` to the searchable multi-select, and have init name the vendor-neutral entry there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
No PR-relevant drift confirmed.
|
Hardening pass over the same defect. Reviewing the first fix turned up four more places the answer was withheld: - `openspec update`'s tool picker builds its own choices and never passed searchAliases through, so the same search failed there. - `--tools <unknown>` printed a bare list of ids. It now names the fallback, the scripted counterpart of the picker's empty hint. The hint I first put on validateTools sat on an unreachable branch; the path users actually hit is the "Invalid tool(s)" parse error, and a test now pins it. - The search box dropped pasted text as well as punctuation. Any sequence whose characters are all printable is now accepted, which also lets a space reach the box so "claude code" filters. Escape sequences carry control characters and are still rejected, and the `name` fallback stays single-character so readline names like 'tab' are never typed. - docs-lab still taught the old label in two places. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
alfred-openspec
left a comment
alfred-openspec
left a comment
There was a problem hiding this comment.
The implementation and test coverage look sound, and CI is green. Please tighten the docs-lab changes before merge: help/faq.md is explicitly a one-line/router surface, but this answer now duplicates the seven-alias how-to detail already owned by reference/supported-tools.md. Keep the detailed search aliases on the supported-tools page and make the FAQ answer a short pointer to that canonical section. Also remove the em dash added to the FAQ; docs-lab/.agents/skills/write-openspec-docs/writing.md forbids em dashes. Since docs-lab changed, final approval from @TabishB is still required by the repository review policy.
2 participants
Status
LGTM. Closes #653
What was wrong
openspec initshows a searchable list of tool names. OpenSpec already ships the answer for an assistant that isn't on that list — the vendor-neutralagentstarget, which writes.agents/skills/openspec-*/SKILL.mdand nothing tool-specific. But it was labeled "Shared .agents skills", after the directory it writes to.Nobody in that position searches for a directory. The reporter — using a proprietary corporate VS Code plugin — typed the words that describe their situation and got back
No matches:universalothergeneric.agentsagents.was silently droppedamazon-qamazonq-was silently droppedclaude codeUnderneath were two separate defects on the same screen:
key.sequenceand leaveskey.nameundefined, and the handler keyed offkey.namealone — so.,-and/were swallowed with no feedback, and pasted text was dropped whole.With no way to reach it, the user's only recourse was to file an issue. It sat unanswered for seven months.
What it does
Makes the entry findable
Other / Universal (shared .agents skills), so it reads as the escape hatch and still says what it writes.searchAliasestoAIToolOptionand to the picker'sChoice; the filter matches name, id, or alias. The universal entry answers touniversal,other,generic,custom,proprietary,unlisted,unsupported,vendor-neutral,agents.md. Aliases are never rendered — they only widen the search.Fixes the search box
key.sequence, falling back to a single-characterkey.name. Any sequence whose characters are all printable is accepted, which covers punctuation and pasted text — so a space can now reach the box andclaude codefilters.namefallback stays single-character so readline names like'tab'and'escape'are never typed as words. The search term is trimmed, so a pasted" claude "still matches.Closes the dead ends, everywhere the answer was withheld
No matchesnow names the fallback, via a genericemptyHinton the prompt so the component stays tool-agnostic:--tools <unknown>used to print a bare list of 42 ids. It now ends with the scripted counterpart of that hint:Same line on the non-interactive "no tools detected" error. Both go through one
universalToolFallbackHint()helper that returns nothing when the target isn't among the tools on offer, so the hint never names a choice the caller can't make.openspec updatebuilds its own choices for the legacy-tool picker and never passed aliases through. Now it does.docs-labtaught the old label in two places; both updated, along withdocs/supported-tools.md. No stale label remains anywhere in the repo.Nothing else moves: no new tool, no new directory, no change to what
agentsinstalls, and--tools agentsis untouched so scripts and CI keep working.Proof it works
Both original defects fail first. On
main, the new prompt tests report:plus the alias cases returning an empty list. All pass here.
Two bugs in my own first pass were caught by writing the tests, and are worth naming since they show the tests aren't decorative:
namefallback to any printable string would have typedtabandescapeinto the search box as literal words. The fallback is now single-character, with a test that presses tab, escape, delete and F1 and asserts the box stays empty.--tools <unknown>hint was first placed invalidateTools, on a branch unreachable from the flag. The test failed with the real message (Invalid tool(s): …), and the hint moved to the path users actually hit.New coverage — 21 tests across three files:
test/prompts/searchable-multi-select.test.ts— alias matching (a term the name doesn't spell), prefix and case-insensitive alias matching, aliases never rendered, no false matches, name/id matching unchanged; punctuation, uppercase, paste, multi-word paste, trimmed padding; rejection of ctrl chords, meta chords, named control keys, arrow escape sequences and newline-bearing pastes; the empty-state hint appearing only when nothing matches.test/core/init.test.ts— the picker really receives the aliases and theemptyHint(the wiring, not just the data), and both error paths name--tools agents.test/core/tool-search-aliases.test.ts— the universal entry keeps its aliases and label, every alias everywhere stays single-word and lowercase (space toggles selection, so a multi-word alias would be unreachable), and the fallback hint stays silent when the target isn't offered.End to end, unchanged output apart from the label:
Full suite: 4449 passed, 2 failed. Those 2 (
config-profile› "confirmed project apply…",artifact-workflow› "creates skills for Cursor tool") fail identically on a clean checkout ofmainat e062b95 — verified in a separate baseline worktree, same two names, same count.npm run lintandtsc --noEmitare clean.Notes / nits
test/core/init.test.tsandtest/core/update.test.ts.openspec/specs/cli-init/spec.mdrequires "a searchable multi-select … filtering by typing to search", which still holds, and theadd-init-agents-targetdelta constrainsskillsDirand detection only. Three of the last four merged fix PRs shipped noopenspec/files either.managed by another tool (Other / Universal (shared .agents skills)). Nested parens, rare path, and reshaping a message every tool shares belongs in its own change.AIToolOption.successLabelis populated for all ~42 tools and read by nothing (grep -rn successLabel src test scriptshits only its own declaration). It looks like it was meant for exactly these setup strings, and wiring it up would also fix the nested parens above. Worth its own PR.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
openspec inittool picker to display the vendor-neutral option as “Other / Universal (shared .agents skills).”.,-, and/.Documentation
Bug Fixes