feat(desktop): merge MCP command and args into one command-line field - #2918
Conversation
Astro-Han
left a comment
There was a problem hiding this comment.
This slice looks very good overall. The split is clean and independently reviewable, the command-line field remains a tokenizer rather than a shell, the stored { command, args } contract is preserved, and the formatter/parser provide one clear round-trip authority. The focused tests and CI evidence also look solid. I did not find any P0–P3 issues.
The only thing missing for me is a small before/after screenshot of the stdio editor dialog. This changes the visible form from separate command and arguments fields to one command-line field, and changes the primary grid from equal columns to a 1:2 ratio. A screenshot at the normal dialog width should be enough to confirm that the command help text, environment field, and working-directory field remain visually balanced.
Once that screenshot is present, I expect this slice to be ready to approve. Thanks for extracting this into such a focused first step.
AI-assisted review disclosure: Codex reviewed the parsing, formatting, validation, renderer wiring, tests, and CI evidence. Astro-Han verified the architectural boundary, found no P0–P3 code issues, and made the final decision to request the remaining visual evidence.
|
Here is the before/after of the stdio editor dialog at the normal dialog width: Before — separate 命令 field and 参数 textarea, id/command in equal columns: After — one command-line field with the shell-quoting hint; the id shrinks to slug width and the command line takes the remaining 2/3, with 环境变量 and 工作目录 unchanged below: |
d9974a9 to
a1dca7a
Compare
|
CI note: all jobs green except two known-flaky/inherited reds — |
The Add/Edit MCP dialog's stdio form asked for the command and a
one-arg-per-line textarea separately. Users naturally type the whole
line ("uvx mcp-science timer") into the command box. The 命令 field now
accepts the full command line: quote-aware tokenization (no shell
semantics) maps it onto the mcp.json command/args shape, and editing an
existing server formats command+args back into one line losslessly.
Unbalanced quotes surface as an inline field error instead of a save
failure, and the primary-fields grid gives the endpoint column twice
the width of the server-id slug.
Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Ac5rv6WUKWtMZgQb5QPN16
a1dca7a to
563c786
Compare
|
Thanks for the split — the boundary with #2653 is clean (only the MCP editor's 7 files, no OAuth content), the storage contract is untouched ( Conclusion: PASS — no blocking findings (no P0/P1/P2). All findings are P3 (nice-to-have, non-blocking):
F1/F2 are one-liners if you touch the file anyway; F5 is a fine follow-up. AI-assisted review disclosure: this review was produced with AI assistance (pi review subagent on 中文摘要(AI 辅助审查)结论:PASS,无 P0/P1/P2,全部 P3。与 #2653 边界干净(只动 MCP editor 7 个文件,无 OAuth 内容);存储契约零变化(McpStdioServerConfig 与运行时消费路径未动、mcp.json schema 不变);旧配置经 formatCommandLine 无损往返;手写 tokenizer 刻意非 shell 语义(无展开/通配/操作符),与旧"不经过 shell 解析"一致。解析器经 40+ 边界用例实测正确(含 30 个刁钻 token 的属性式往返测试全过),PR 分支内旧结构零残留。P3 项:F1 configFromDraft 防御性 throw 只防一半('""' 静默存 command:'',但唯一调用方先校验不可达);F2 空字符串参数行为变更未声明(旧 UI 过滤空行,新 UI 可经 "" 存 args:['']);F3 Windows 尾反斜杠路径报 unbalanced-quote(标准 shell 行为,文案可补充);F4 CSS 1fr/2fr 同时影响 remote 表单 URL 字段(纯视觉);F5 测试缺口可延后(无单引号/混合引号往返测试、无组件级测试);F6 注释混入中文。 |


Extracted from #2653 per the review recommendation to split it into independently reviewable units. This is the command-line slice, standalone and independent of the OAuth work.
What
Merges the MCP editor's separate command / args inputs into one command-line field, with shell-style tokenization (quotes, escapes) parsed in
mcp-command-line.tsand validated bymcp-editor-validation.ts. The dialog shows one field that round-trips to the stored{command, args}shape.Tests
apps/desktop/src/main/__tests__/mcp-command-line.test.tscovers tokenization (quoting, escaping, unterminated quotes) and round-tripping; editor-validation tests updated for the merged field.Co-Authored-By: Claude noreply@anthropic.com
https://claude.ai/code/session_01TMwYxgNEbz2RFmuK6AXGcj