Skip to content

fix(cli): skip reserved flag names in canonical MCP command generator#444

Open
typefield wants to merge 1 commit into
DingTalk-Real-AI:mainfrom
typefield:fix/canonical-reserved-flag-collision-v2
Open

fix(cli): skip reserved flag names in canonical MCP command generator#444
typefield wants to merge 1 commit into
DingTalk-Real-AI:mainfrom
typefield:fix/canonical-reserved-flag-collision-v2

Conversation

@typefield

Copy link
Copy Markdown
Contributor

Summary

  • Fix panic: flag redefined: params when MCP tool inputSchema contains properties named params, json, or any name colliding with built-in/inherited CLI flags
  • Add dynamic collision check in applyFlagSpecs using cmd.Flags().Lookup / cmd.InheritedFlags().Lookup
  • Colliding alias names are also skipped

Root Cause

The canonical MCP command path (internal/cli/canonical.go) auto-generates CLI flags from tools/list inputSchema. When a tool (e.g. chat_permission_grant) declares a property named params, the generator tries to register --params which conflicts with the built-in --params flag. pflag panics on duplicate registration.

The compat/overlay path already filters reserved names, but the canonical path did not.

Fix

applyFlagSpecs now checks if the flag name (or alias) already exists before registering. This covers all built-in and inherited persistent flags dynamically. Users can still pass colliding values via --json '{"params":"..."}'.

Test plan

  • New unit tests: TestApplyFlagSpecsSkipsReservedAndInheritedFlags, TestApplyFlagSpecsSkipsCollidingAlias
  • go test ./internal/cli/ — all pass
  • cli_to_mcp E2E: chat (201 passed), todo (51 passed), full suite (1469 passed, 0 panic)

🤖 Generated with Claude Code

MCP tools whose inputSchema contains properties named "params", "json",
"format", "timeout", "yes", or any other name that collides with built-in
or inherited persistent flags cause a pflag panic ("flag redefined") when
the canonical command tree is constructed.

The compat/overlay path already filters "json" and "params", but the
canonical path in applyFlagSpecs did not. This adds a dynamic collision
check using cmd.Flags().Lookup so any future persistent flag additions
are automatically covered. Colliding alias names are also skipped.

Users can still pass these values via --json '{"params":"..."}'.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@typefield

Copy link
Copy Markdown
Contributor Author

测试验证报告

1. 单元测试

$ go test ./internal/cli/ -count=1
ok  github.com/DingTalk-Real-AI/dingtalk-workspace-cli/internal/cli  0.777s
$ go test ./internal/... -count=1
ok  internal/cli       2.451s
ok  internal/compat    2.833s
ok  internal/app       (all pass)
... 全部 29 个包 PASS

2. cli_to_mcp E2E 全量测试(17 个产品)

使用 dws-wukong develop 分支构建的二进制(含本次 fix),逐产品运行 pytest:

产品 Passed Failed Skipped 耗时
aitable 167 0 3 4:33
contact 73 0 12 1:10
calendar 122 0 2 2:38
todo 51 3 6 1:02
doc 212 0 21 3:21
wiki 113 0 0 1:17
chat 201 0 8 3:33
conference 12 0 0 0:08
minutes 276 5 44 5:00
attendance 142 0 7 1:38
mail 78 5 71 1:28
oa 2 3 0 0:03
report 7 0 0 0:05
blackboard 13 0 7 0:16
bot 0 0 15 0:00
ding 0 0 13 0:00
workbench 0 0 6 0:00
总计 1469 16 215 ~27 min

3. 关键验证点

  • chat 产品 201 passed, 0 failedchat_permission_grant 所在服务全部命令正常,零 panic
  • 16 个失败均为已有业务问题(todo list-sub 断言、minutes mind-graph、mail 参数回归、oa 日期范围),与本次 fix 无关
  • 零 panic,零 CLI 崩溃

4. dws-wukong Go 测试

ok  gitlab.internal/dws/dws-wukong/wukong                              1.839s
ok  gitlab.internal/dws/dws-wukong/wukong/extensions                   0.457s
ok  gitlab.internal/dws/dws-wukong/wukong/extensions/vendors/dingtalk  1.160s
ok  gitlab.internal/dws/dws-wukong/wukong/pat                          0.748s
ok  gitlab.internal/dws/dws-wukong/wukong/products/doc                 1.641s

wukong/productswukong/skill 的失败项改前改后完全一致(已有问题),本次 fix 零回归。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant