refactor(decision-context): classify the two capability id slots and single-source the kunluncode mcp pin - #4513
Conversation
Refs loopx-project#4447 (Track A) DECISION_CONTEXT_CAPABILITY_ID was defined twice with different values: "decision-context" for the extension binding and "decision_context" for the capability packet contract. They are not a conflict: * the hyphenated value is the catalog/CLI/extension namespace, and every loopx/capabilities/*/catalog_entry.py id uses that spelling; * the underscore value is the packet contract, matching the sibling capability's "material_lifecycle". No consumer joins the two, so both values are retained and the constants now name their slot. Bump-safe parity is locked by a focused regression test instead of a unification that would break either namespace. Signed-off-by: YZJF <195568136+YZJF@users.noreply.github.com>
Refs loopx-project#4447 (Track A) The pinned mcp version was written out three times in cli.py: in MCP_REQUIREMENT, in the _compatible_python probe and in the provision failure message. A future pin bump could therefore update the install requirement while leaving a probe that still asserts the old version. MCP_SDK_VERSION is now the only place the version appears; the requirement, the probe and the message all derive from it. The pin stays exact and is not relaxed to a range: goal_mode_mcp.py imports mcp.server.fastmcp, which the MCP SDK 2.x line no longer ships, and the pin is a deliberate security pin (892faa2). It also stays independent of claude_goal_mode's "mcp<2": the two adapters provision separate venvs, so they are separate packaging boundaries. Signed-off-by: YZJF <195568136+YZJF@users.noreply.github.com>
huangruiteng
left a comment
There was a problem hiding this comment.
动机
这个 PR 处理 #4447 审计里被我归类为"同名不同值"的两处:DECISION_CONTEXT_CAPABILITY_ID 在 packets.py 是 decision_context、在 extension_provider.py 是 decision-context;以及 kunluncode adapter 里 mcp==1.28.1 这个版本串被写在三处(requirement、兼容性探针、用户可见报错)。前者如果被后人"顺手统一",会打破扩展绑定或 packet 契约中的一边;后者在版本升级时容易只改一处,留下断言旧版本的探针。
改动思路
作者没有改动任何一个值,而是给两个槽位分别起名并写清理由:DECISION_CONTEXT_EXTENSION_CAPABILITY_ID(连字符,扩展/目录/CLI 命名空间,等于 DECISION_CONTEXT_CATALOG_ENTRY["id"])与 DECISION_CONTEXT_PACKET_CAPABILITY_ID(下划线,packet 契约命名空间,与兄弟能力 material_lifecycle 一致);adapter 侧把 MCP_SDK_VERSION 作为唯一来源,requirement、探针断言与报错文案都由它派生。配套两个测试把分类钉住。
具体改动
loopx/capabilities/decision_context/extension_provider.py:第 30 行新增槽位常量与说明注释,第 253 行调用点改用它(值不变)。loopx/capabilities/decision_context/packets.py:第 23 行新增 packet 槽位常量,第 212 行_capability_contract改用它(值不变)。loopx/kunluncode_goal_mode/cli.py:第 37-38 行新增MCP_SDK_VERSION/MCP_REQUIREMENT,第 78 行探针、第 122 行安装、第 205 行报错改为派生。tests/capabilities/test_decision_context_capability_id_slots.py(新增 4 个用例)与tests/test_kunluncode_goal_mode.py::test_mcp_pin_has_one_source_of_truth:锁住两个槽位的拼写、二者必须不同,以及探针源码里不再出现版本字面量。
关键代码讲解
loopx/capabilities/decision_context/extension_provider.py:30—DECISION_CONTEXT_EXTENSION_CAPABILITY_ID = "decision-context":注释说明它走扩展运行时、必须与DECISION_CONTEXT_CATALOG_ENTRY["id"]相等。我核对了仓库里全部loopx/capabilities/*/catalog_entry.py,id 清一色是连字符(material-lifecycle、semantic-preference、pull-request-review…),注释的说法站得住。loopx/capabilities/decision_context/packets.py:23—DECISION_CONTEXT_PACKET_CAPABILITY_ID = "decision_context":下划线用于 packet 契约,兄弟能力material_lifecycle的 CLI 里 capability_id 默认值同样是下划线拼写,所以这确实是两种命名空间而不是笔误;新用例test_the_two_slots_are_not_the_same_value明确要求两者不相等。loopx/kunluncode_goal_mode/cli.py:37-38—MCP_SDK_VERSION/MCP_REQUIREMENT = f"mcp=={MCP_SDK_VERSION}":注释解释了为什么是精确 pin(loopx/goal_mode_mcp.py依赖mcp.server.fastmcp,SDK 2.x 已不再提供)以及为什么与claude_goal_mode的mcp<2不必一致(两个独立的打包边界)。探针现在断言version('mcp') == '{MCP_SDK_VERSION}',报错文案使用{MCP_REQUIREMENT}。tests/test_kunluncode_goal_mode.py::test_mcp_pin_has_one_source_of_truth— 除了断言MCP_REQUIREMENT == f"mcp=={MCP_SDK_VERSION}",还断言MCP_SDK_VERSION not in inspect.getsource(cli._compatible_python)且源码里存在MCP_SDK_VERSION;这条"探针里不能再有字面版本"的检查是关键的一步,否则单源只是形式上的。
对主干的风险
没有阻塞项,行为不变。 两个常量都是模块内部的、各自只有一个调用点(extension_provider.py:253、packets.py:212),值一个字符没改;我全仓搜索确认旧名 DECISION_CONTEXT_CAPABILITY_ID 已从代码中消失(只剩 RFC 散文与测试文档字符串提到历史名字)。
P3(非阻断,注释准确性):loopx/kunluncode_goal_mode/cli.py:35 写的是 "Bump MCP_SDK_VERSION alone",但同一个依赖还在 pyproject.toml:29(test extra)以及 loopx/kunluncode_goal_mode/README.md:54、:60 和 zh-CN 适配器指南里以字面量出现。也就是说"单源"的边界是 adapter CLI 内部(requirement/探针/报错三处),不是全仓;照这句话升级会漏掉 test extra 与文档。最小修复是把这句话限定为"下面的探针与报错由它派生",或把需要同步的面一并列出。
P3(非阻断,跟踪文档一致性):docs/architecture/rfcs/semantic-vocabulary-convergence-v0.md:110-114(中文版第 95-98 行)仍然把 DECISION_CONTEXT_CAPABILITY_ID 列为"同名不同值"、把 MCP_REQUIREMENT 列为冲突定义,而本 PR 之后这两条在代码里已不存在/已被判定为刻意不同。该 RFC 自述是对 baseline 的审计,所以这不算本 PR 的缺陷;但补一行状态指针(本 PR 已分类,证据是槽位常量与注释)能避免后来者重新捡起这两条。
验证(全部在 93ff89a3 上跑):tests/capabilities/test_decision_context_capability_id_slots.py 4 passed;tests/test_kunluncode_goal_mode.py -k mcp_pin 1 passed;全仓 rg 确认旧名与字面版本的去向。另外我跑了 tests/architecture/test_semantic_inventory.py + tests/capabilities(21 failed / 1439 passed / 17 skipped),并把其中代表性失败 test_cli_rejects_unknown_capability_without_traceback 在 PR 的 merge base 36c6d8df0 与当前 origin/main 0346a31c2 上各跑一次,两处都同样失败,因此这是既有的环境/仓库状态问题,与本 head 无关;没有任何失败落在本 PR 触及的文件上。按本 lane 配置我不拉取也不等待 GitHub CI。
我的整体评价
APPROVE。这是一个边界清楚、把"隐式知识"显式化的重构:两个 capability id 槽位被分别命名,注释直接解释了为什么两个值必须不同、以及各自的兄弟惯例是什么,并用测试把"二者不相等、连字符槽位等于目录 id、探针里不得再出现字面版本"钉住——这正是防止后人错误统一的最便宜办法。adapter 侧的单源确实让"改 requirement 忘了探针"这类失误在测试里失败。两条 P3 都不影响正确性:一条是注释把"单源"的边界说得过大(pyproject.toml 与适配器文档仍持有字面量),一条是跟踪用的 RFC 行没同步状态。剩下的风险只有一条与代码无关的观察:head 落后于当前 main(merge-tree 干净),以及该仓库在本地环境下有一批与本次改动无关的既有失败,我已在 base 与 main 各复现一次以确认来源。
English verdict: APPROVE — exact head 93ff89a3076fcf99892fa07faf31c9435a0c00aa of #4513. Values are unchanged; the change names the two decision-context capability id slots (decision-context for extension/catalog/CLI bindings, decision_context for the packet contract), which matches the hyphenated loopx/capabilities/*/catalog_entry.py ids and the sibling material_lifecycle packet vocabulary, and derives the kunluncode adapter's requirement, compatibility probe and error message from MCP_SDK_VERSION. Validation at this head: the 4 new slot tests pass, test_mcp_pin_has_one_source_of_truth passes (including the assertion that the probe source no longer contains the literal version), and a tree-wide search shows the old ambiguous name is gone from code. The wider tests/capabilities run shows 21 failures, but I reproduced the representative one at both the merge base 36c6d8d and origin/main 0346a31 where it fails identically, so they are pre-existing and unrelated; no failing test touches a changed file. Two non-blocking P3s: the new comment says "Bump MCP_SDK_VERSION alone" although pyproject.toml:29 and the adapter docs still carry the literal pin, and the convergence RFC still lists both audited rows as unresolved after this classification.
Refs #4447 — Track A, the checklist item:
"Resolve
DECISION_CONTEXT_CAPABILITY_IDhyphen/underscore andMCP_REQUIREMENTpin/range differences after checking actual callers and packaging boundaries."This PR does not close the tracker; it addresses two checklist items and records the classification evidence for both.
1.
DECISION_CONTEXT_CAPABILITY_ID— two slots, not a conflictThe same constant name carried two values:
capabilities/decision_context/extension_provider.py:24decision-contextcapabilities/decision_context/packets.py:18decision_contextCaller check shows both spellings are conventional in their own namespace and neither is a stray:
loopx/capabilities/*/catalog_entry.pyids use it (decision-context,material-lifecycle,periodic-report, …), matching the CLI command and thedecision-contexthelp/catalog surface. The extension value is looked up by the extension runtime, so it must equalDECISION_CONTEXT_CATALOG_ENTRY["id"].material_lifecycle(capabilities/material_lifecycle/architecture.py:39,_validation.py:177), and the six decision-context emitters (architecture.py:33,profile.py:419,runtime.py:321,sources.py:314and:401,outcome_feedback.py:301) all agree.No consumer joins the two values, so both values are retained and the constants are renamed to name their slot:
DECISION_CONTEXT_EXTENSION_CAPABILITY_IDandDECISION_CONTEXT_PACKET_CAPABILITY_ID. Values, packet output and extension lookup behaviour are unchanged. This is a scope clarification, not a debt-count reduction.2.
MCP_REQUIREMENT— pin vs range is justified; the same-file triplication was notloopx/kunluncode_goal_mode/cli.py:28→mcp==1.28.1(exact)loopx/claude_goal_mode/scripts/install.py:83→mcp<2(range)The difference is justified and is kept: the two adapters provision separate venvs (
~/.local/share/loopx/kunluncode-mcp/.venvvs the Claude MCP venv), so they are separate packaging boundaries. The exact pin is also deliberate —loopx/goal_mode_mcp.py:285importsmcp.server.fastmcp, which the MCP SDK 2.x line no longer ships, and the pin came from892faa2c9fix(security): upgrade the MCP SDK pin. Relaxing it to a range would undo a security pin.The real defect was local:
1.28.1was written out three times incli.py— inMCP_REQUIREMENT, in the_compatible_pythonprobe assertion, and in the provisioning failure message. A bump could update the requirement while leaving a probe that asserts the old version.MCP_SDK_VERSIONis now the single source; the requirement, probe and message derive from it.Validation
Base
36c6d8df0(upstream/main), head93ff89a30. Python 3.13.12 (repo requires >=3.11; nopython3.11on this machine).pytest -q tests/capabilities/test_decision_context_capability_id_slots.py tests/test_kunluncode_goal_mode.py tests/capabilities/test_decision_context_packets.py tests/capabilities/test_decision_context_extension_provider.pypytest -q tests/capabilitiestest_repository_change_window.py(SSH/worktree env), untouched by this PRpytest -q tests/architectureruff checkon all changed filesexamples/semantic-vocabulary-drift-smoke.pyTypeScript production parser failed; run npm ci. Not runnable here, unchanged by this PRBoundaries
semantic-vocabulary-convergence-v0.md:110/.zh-CN.md:95) still listDECISION_CONTEXT_CAPABILITY_IDas a same-name conflict. They are now historically resolved; I left the normative bilingual RFCs alone so the update can be recorded as a decision rather than smuggled into a code PR.Signed-off-by.