feat(cli): match MCP resource completions by name and discover servers - #5733
Conversation
`@server:` completion previously matched only the resource URI, case-sensitively, and there was no way to reach a server without typing its full name first. - Match the partial after the colon case-insensitively against the resource's friendly name/title (`title || name`) as well as the URI, ranked URI-prefix > name-prefix > URI-substring > name-substring, and surface the name as the suggestion description. - Before the colon, suggest configured MCP servers that expose resources and whose name prefixes the input, prepended to the file results (never hiding files). Selecting one expands to `@server:` and drills into the resource list (reuses the directory `isDirectory` continuation). Refs QwenLM#5601, follows QwenLM#5635.
|
Thanks for the PR! Template looks good ✓ On direction: solid follow-up to #5544 and #5635. The CHANGELOG has On approach: focused and minimal — case-insensitive name + URI matching with a clean 4-tier ranking (URI prefix → name prefix → URI substring → name substring), server discovery before the colon that reuses the existing 中文说明感谢贡献！ 模板完整 ✓ 方向：是 #5544 和 #5635 的自然后续。CHANGELOG 中有 方案：聚焦且精简——大小写不敏感的名称 + URI 匹配，4 级排序（URI 前缀 → 名称前缀 → URI 子串 → 名称子串），冒号前的 server 发现复用已有的 — Qwen Code · qwen3.7-max |
Code ReviewNo blockers. The implementation is clean and well-scoped:
TestsReal-Scenario TUI Testing (tmux)Set up a stdio MCP server exposing two resources with friendly titles ( Bare
|
|
This PR ships exactly what it promises: the The implementation is minimal and well-placed. The 4-tier ranking is the right level of sophistication — no over-engineered scoring system, just prefix > substring across URI and name. The server discovery reuses the existing Unit tests (26 passing, 8 new) cover the right edges: name match, case insensitivity, ranking order, description display, server discovery, empty- The diff is focused — one hook, 3 i18n keys, a docs paragraph. No drive-by refactors, no scope creep. This is how a good feature PR looks. LGTM. Approving. ✅ 中文说明本 PR 兑现了承诺： 实现精简且位置恰当。4 级排序的复杂度恰到好处——没有过度设计的评分系统，只是 URI 和名称上的前缀 > 子串。server 发现复用了已有的 单测（26 通过，8 个新增）覆盖了正确的边界：名称匹配、大小写不敏感、排序、描述显示、server 发现、空 diff 聚焦——一个 hook、3 个 i18n key、一段文档。无顺手重构，无范围膨胀。这是一个优秀 feature PR 该有的样子。 通过。✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
LGTM. Clean follow-up to #5635 — the @ picker now matches a resource's name/title case-insensitively and surfaces servers before the colon.
Verified the three mechanisms it leans on:
matchMcpServerPrefixrequires the literalname:, so@dem(no colon) falls through to server discovery and can't collide with the resource path.- The
demo:server entry rides the existingisDirectorydrill-in —useCommandCompletionkeys off the flag (not a trailing/), so it suppresses the trailing space and re-triggers into the resource list as intended. - i18n coverage (en/zh/zh-TW) matches the existing pattern — the neighboring
Reference in chatkey lives in exactly those three locales, the rest fall back toen.
The 4-tier rank and the 8 new tests line up, and the scope is tight: one hook, 3 keys, a docs paragraph. Nothing blocking.
| * straight into that server's resource list (the `getMcpResourceSuggestions` | ||
| * path above). | ||
| */ | ||
| function getMcpServerSuggestions( |
There was a problem hiding this comment.
[Suggestion] getMcpServerSuggestions returns all matching servers with no .slice() cap, unlike both file results and MCP resource results which are capped at MAX_SUGGESTIONS_TO_SHOW * 3. With many configured MCP servers sharing a common prefix (e.g., aws-s3, aws-lambda, aws-dynamodb), all of them appear before any file result. Since only MAX_SUGGESTIONS_TO_SHOW (8) rows are visible at once, a user with 10+ matching servers sees only server entries and no files at all.
| function getMcpServerSuggestions( | |
| return Object.keys(mcpServers) | |
| .filter( | |
| (name) => | |
| name.toLowerCase().startsWith(query) && | |
| (registry.getResourcesByServer(name)?.length ?? 0) > 0, | |
| ) | |
| .sort((a, b) => a.localeCompare(b)) | |
| .slice(0, 5) | |
| .map((name) => ({ | |
| label: `${name}:`, | |
| value: `${name}:`, | |
| description: t('MCP resource server'), | |
| isDirectory: true, | |
| })); |
— Claude 3.5 Sonnet via Qwen Code /review
| .filter( | ||
| (name) => | ||
| name.toLowerCase().startsWith(query) && | ||
| (registry.getResourcesByServer(name)?.length ?? 0) > 0, |
There was a problem hiding this comment.
[Suggestion] getResourcesByServer(name) iterates the entire resources Map, pushes matching entries into a new array, then sorts it — but the caller discards the array immediately and only checks .length > 0. This runs on every keystroke for each prefix-matching server.
Add an O(M) existence-check method to ResourceRegistry that short-circuits on the first match:
hasResourcesForServer(serverName: string): boolean {
for (const resource of this.resources.values()) {
if (resource.serverName === serverName) return true;
}
return false;
}Then replace the .length > 0 check with registry.hasResourcesForServer(name).
— Claude 3.5 Sonnet via Qwen Code /review
| const query = pattern.toLowerCase(); | ||
| return Object.keys(mcpServers) | ||
| .filter( | ||
| (name) => |
There was a problem hiding this comment.
[Suggestion] getMcpServerSuggestions matches server names case-insensitively (name.toLowerCase().startsWith(query)), but the downstream consumer matchMcpServerPrefix in mcpResourceRef.ts uses case-sensitive matching (input.startsWith(\${name}:`)`).
The auto-complete flow works today because the suggestion value always uses the canonical-cased server name from Object.keys(mcpServers). However, any future code path that constructs an @server:uri string without going through the auto-complete picker (e.g., programmatic prompt assembly, paste handling, retry-with-same-references) would silently fail to resolve a case-mismatched server name.
Consider either:
- Making
matchMcpServerPrefixcase-insensitive (returning the canonical-cased server name from the configured set), or - Adding a comment documenting that the inserted
valueMUST use the exact casing fromObject.keys(mcpServers), plus a regression test that round-trips a case-mismatched input throughparseMcpResourceRef.
— qwen3.7-max via Qwen Code /review
| dispatch({ type: 'ERROR' }); | ||
| // A file-search failure shouldn't swallow server matches we already | ||
| // have; show those rather than dropping to an error state. | ||
| if (serverSuggestions.length > 0) { |
There was a problem hiding this comment.
[Suggestion] The catch block now has a new error-fallback branch that dispatches SEARCH_SUCCESS with server suggestions when file search fails, but no test exercises this path.
The catch block has two distinct branches (server suggestions available vs. not), and neither is covered by the test suite. Mocking fileSearch.current.search() to throw a non-abort error would be straightforward.
// Example: test that server suggestions are shown when file search fails
it('shows server suggestions when file search throws', async () => {
// Mock fileSearch.current.search() to throw
// Configure an MCP server with resources matching the typed partial
// Assert server suggestions are surfaced, not error state
});— qwen3.7-max via Qwen Code /review
| }); | ||
| expect(result.current.suggestions[0].description).toBe('Project Spec'); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
[Suggestion] All 4 server-discovery tests use exact-case server names (e.g., 'my' against 'myserver'), but getMcpServerSuggestions explicitly uses name.toLowerCase().startsWith(query) for case-insensitive matching — a behavior highlighted in the docs update as a user-facing feature.
A test with a case-mismatched query (e.g., 'MY' against server 'MyServer') would guard against regressions that reintroduce case-sensitive matching.
— qwen3.7-max via Qwen Code /review
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Two new code branches in search() lack test coverage (file-index-not-ready + catch-block fallback). Three additional suggestions for consistency and test robustness.
| if (!fileSearch.current) { | ||
| // File index not ready yet; still surface any server matches so | ||
| // discovery doesn't have to wait on the crawler. | ||
| if (serverSuggestions.length > 0) { |
There was a problem hiding this comment.
[Critical] This branch — dispatching server suggestions when fileSearch.current is null — has no test coverage. The companion branch in the catch block at line 404 is also untested.
Both are new fallback paths introduced by this PR. A regression (e.g., dropping the dispatch or dispatching ERROR instead) would silently revert to the old behavior with no test to catch it.
Consider adding two tests:
- Mock a delayed
FileSearchFactory.createsofileSearch.currentis null during the search, type a pattern matching a server name, and assert the server suggestion appears immediately. - Make
fileSearch.search()reject with a non-abort error, type a pattern matching a server name, and assert server suggestions are shown (not an error state).
— qwen3.7-max via Qwen Code /review
| // Only surface the friendly name when it adds information beyond the URI | ||
| // (mirrors the `/mcp` resource list, which dims a redundant name). | ||
| description: | ||
| m.friendly && m.friendly !== m.resource.uri ? m.friendly : undefined, |
There was a problem hiding this comment.
[Suggestion] This comparison uses case-sensitive !==, but the ranking logic above (line 69) lowercases both uri and friendly before comparison. If a resource has name: "MyDoc" and uri: "mydoc", the rank treats them as identical (both lowercase to "mydoc") but this check sees "MyDoc" !== "mydoc" as true and surfaces a redundant description.
| m.friendly && m.friendly !== m.resource.uri ? m.friendly : undefined, | |
| m.friendly && m.friendly.toLowerCase() !== m.resource.uri.toLowerCase() ? m.friendly : undefined, |
— qwen3.7-max via Qwen Code /review
| return Object.keys(mcpServers) | ||
| .filter( | ||
| (name) => | ||
| name.toLowerCase().startsWith(query) && |
There was a problem hiding this comment.
[Suggestion] The .toLowerCase() here is never actually exercised by the test suite. All 4 server-discovery tests use exact-case server names (myserver with pattern my), so removing .toLowerCase() would pass identically.
Consider using a mixed-case test scenario (e.g., server MyServer with pattern MY) to verify the case-insensitive matching actually works as intended.
— qwen3.7-max via Qwen Code /review
4 participants
What this PR does
Improves the in-chat
@server:uriMCP resource completion (shipped in #5544, surfaced in the/mcpdialog by #5635) so it behaves like a normal fuzzy mention picker:title || namethe/mcpdialog shows — in addition to its URI. The matched name is shown as the suggestion's description so a name-only hit is self-explanatory. The injectedvalueis still the canonical@server:urireference. Ranking, best first: URI prefix → name prefix → URI substring → name substring.@<partial>(no colon yet) now also suggests configured MCP servers that expose at least one resource and whose name prefixes the input, prepended to (never replacing) the filesystem results. Selecting one expands to@server:and drills straight into that server's resource list — it reuses the existingisDirectorydirectory-continuation, so Tab drills in and Enter inserts-and-closes, identical to picking a folder. The bare@trigger is intentionally left files-only.Why it's needed
After #5635 a user can browse a server's resources in the
/mcpdialog, but the in-chat@completion only matched the URI, case-sensitively, and only once the full@server:prefix was typed. So a user who remembered a resource by its human name ("Project Spec"), typed a different case, or didn't recall the exact server name couldn't complete it — the discoverability #5635 added in the dialog didn't carry over to the place you actually type the reference. This closes that gap.Reviewer Test Plan
How to verify
resources/listwith friendlytitle/name(e.g.uri: file:///docs/spec.md,title: "Project Spec").qwenin a trusted folder; wait for MCP discovery to settle.@demo:Project→ completes@demo:file:///docs/spec.md(matched the title), shown with descriptionProject Spec.@demo:SPEC(uppercase) → same match (case-insensitive on the URI).@dem→ ademo:entry (labeledMCP resource server) appears above the file matches; pressing Tab drills into the resource list.Evidence — real-TUI A/B
Built the bundle and drove the real TUI in tmux against a live stdio MCP server exposing two resources (
file:///docs/spec.md"Project Spec",file:///config/app.json"App Config"), in a workspace also containingdemo-notes.txt/readme.md:@demo:Project(title match)demo:file:///docs/spec.md·Project Spec@demo:SPEC(uppercase URI)demo:file:///docs/spec.md·Project Spec@dem(server discovery)demo-notes.txt,readme.md(files only)demo:·MCP resource server+demo-notes.txt,readme.md@demthenTab@demo:→ lists both resources@demo:(list)App Config/Project SpecdescriptionsUnit tests:
npx vitest run --root packages/cli src/ui/hooks/useAtCompletion.test.ts→ 26 passing (8 new: name match, case-insensitivity, 4-level ranking, description, server discovery, empty-@files-only, no-resource server excluded, untrusted folder).useCommandCompletion26 passing (no regression).tsc --noEmit,eslint,prettier --check, andcheck-i18nall green.Tested on
Windows/Linux not run manually — covered by the cross-platform CI unit tests.
Risk & Scope
useAtCompletion.ts), plus 3 i18n keys and a docs note. No change to MCP discovery, the resource read/inject path, or core.@menu stays files-only, so the most common case is unchanged. ThegetResourcesByServermembership check is&&-short-circuited behind a name-prefix test, so it runs only for prefix-matching servers (normally 0–1 per keystroke).Linked Issues
Refs #5601, follows #5635.
中文说明
这个 PR 做了什么
把对话框里的
@server:uriMCP 资源补全(#5544 实现,#5635 在/mcp面板里暴露)改成更像普通的模糊提及选择器:/mcp面板显示的title || name)。命中的名称作为候选项的描述显示,这样"按名字命中"时一目了然。注入的value仍然是规范的@server:uri引用。排序优先级:URI 前缀 → 名称前缀 → URI 子串 → 名称子串。@<partial>会把"暴露了资源、且名称以输入为前缀"的 MCP server 作为候选项前插到文件结果里(不替换、不隐藏文件)。选中后展开成@server:并直接钻入该 server 的资源列表——复用已有的isDirectory目录续接机制,所以 Tab 钻入、Enter 插入并关闭,和选文件夹完全一致。裸@有意保持只列文件。为什么需要
#5635 之后用户可以在
/mcp面板里浏览资源,但对话框里的@补全只匹配 URI、区分大小写,而且必须先敲完整的@server:前缀。于是按人类名称("Project Spec")记忆、大小写不符、或记不住确切 server 名的用户都补不出来——#5635 在面板里加的可发现性没有延续到真正输入引用的地方。本 PR 补上这一环。评审验证计划
如何验证
resources/list暴露带友好title/name资源的 stdio MCP server(如uri: file:///docs/spec.md、title: "Project Spec")。qwen,等 MCP 发现完成。@demo:Project→ 补全@demo:file:///docs/spec.md(命中标题),并显示描述Project Spec。@demo:SPEC(大写)→ 同样命中(URI 大小写不敏感)。@dem→demo:项(标MCP resource server)出现在文件匹配上方;按 Tab 钻入资源列表。证据 — 真实 TUI A/B
构建 bundle 后用 tmux 驱动真实 TUI,对接一个暴露两个资源(
file:///docs/spec.md"Project Spec"、file:///config/app.json"App Config")的真实 stdio MCP server,工作区另含demo-notes.txt/readme.md。结果见上方英文表格:@demo:Project、@demo:SPEC在改前无候选、改后命中;@dem改前只有文件、改后demo:前插且文件仍在;@dem+Tab 钻入资源列表;@demo:改前只有 URI、改后带友好名称描述。单测:
useAtCompletion26 通过(8 个新增),useCommandCompletion26 通过(无回归);tsc/eslint/prettier --check/check-i18n全绿。测试平台
macOS ✅;Windows⚠️ 未手动测试;Linux ⚠️ 未手动测试(均由跨平台 CI 单测覆盖)。
风险与范围
useAtCompletion.ts)里,外加 3 个 i18n key 和一处文档。不改 MCP 发现、资源读取/注入路径,也不动 core。@仍只列文件,最常见场景不变。getResourcesByServer的"是否有资源"检查在名称前缀判断后用&&短路,只对前缀匹配的 server 触发(正常每次按键 0~1 个)。关联 Issue
Refs #5601,follows #5635。