From 012f76c6528826260ffe81b4ae67320f31eadabd Mon Sep 17 00:00:00 2001 From: YZJF <195568136+YZJF@users.noreply.github.com> Date: Wed, 16 Sep 2026 16:36:58 +0800 Subject: [PATCH 1/2] refactor(decision-context): name the two capability id slots Refs #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> --- .../decision_context/extension_provider.py | 10 ++- .../capabilities/decision_context/packets.py | 9 ++- ...st_decision_context_capability_id_slots.py | 63 +++++++++++++++++++ 3 files changed, 78 insertions(+), 4 deletions(-) create mode 100644 tests/capabilities/test_decision_context_capability_id_slots.py diff --git a/loopx/capabilities/decision_context/extension_provider.py b/loopx/capabilities/decision_context/extension_provider.py index 3b48ced734..a0d5b7af9d 100644 --- a/loopx/capabilities/decision_context/extension_provider.py +++ b/loopx/capabilities/decision_context/extension_provider.py @@ -21,7 +21,13 @@ EXTENSION_CONTEXT_PROVIDER_ID = "extension" -DECISION_CONTEXT_CAPABILITY_ID = "decision-context" +# Extension-binding namespace. Catalog entry ids, CLI command names and +# extension capability bindings all use the hyphenated spelling (every +# loopx/capabilities/*/catalog_entry.py id, e.g. "material-lifecycle"). This +# value is looked up through the extension runtime, so it must stay equal to +# DECISION_CONTEXT_CATALOG_ENTRY["id"]; it is a different slot from the +# underscore packet-contract id in packets.py and must not be unified with it. +DECISION_CONTEXT_EXTENSION_CAPABILITY_ID = "decision-context" DECISION_CONTEXT_ADVISORY_PROVIDER_PROTOCOL = ( "decision_context_advisory_provider_v0" ) @@ -244,7 +250,7 @@ def retrieve( resolution = resolve_optional_capability_binding( state_file=self.state_file, extension_id=self.extension_id, - capability_id=DECISION_CONTEXT_CAPABILITY_ID, + capability_id=DECISION_CONTEXT_EXTENSION_CAPABILITY_ID, protocol=DECISION_CONTEXT_ADVISORY_PROVIDER_PROTOCOL, permission=DECISION_CONTEXT_ADVISORY_PERMISSION, ) diff --git a/loopx/capabilities/decision_context/packets.py b/loopx/capabilities/decision_context/packets.py index 627e9306ed..73154c06a6 100644 --- a/loopx/capabilities/decision_context/packets.py +++ b/loopx/capabilities/decision_context/packets.py @@ -15,7 +15,12 @@ DECISION_REVIEW_RECEIPT_SCHEMA_VERSION = "decision_review_receipt_v0" DECISION_OUTCOME_RECEIPT_SCHEMA_VERSION = "decision_outcome_receipt_v0" -DECISION_CONTEXT_CAPABILITY_ID = "decision_context" +# Packet-contract namespace. Capability packets identify the capability with +# the underscore spelling (the sibling capability emits "material_lifecycle"). +# No consumer joins this value with the hyphenated catalog/extension id in +# extension_provider.py: they are two slots for the same capability, both +# consistent with their own siblings, so the spellings stay separate. +DECISION_CONTEXT_PACKET_CAPABILITY_ID = "decision_context" DECISION_OUTCOME_VERIFICATION_STATUSES = { "pending", "verified", @@ -204,7 +209,7 @@ def _packet_ref(prefix: str, packet: Mapping[str, Any]) -> str: def _capability_contract(*, packet_role: str) -> dict[str, Any]: return { - "capability_id": DECISION_CONTEXT_CAPABILITY_ID, + "capability_id": DECISION_CONTEXT_PACKET_CAPABILITY_ID, "scope": "goal", "default_enabled": False, "packet_role": packet_role, diff --git a/tests/capabilities/test_decision_context_capability_id_slots.py b/tests/capabilities/test_decision_context_capability_id_slots.py new file mode 100644 index 0000000000..3ad29e9608 --- /dev/null +++ b/tests/capabilities/test_decision_context_capability_id_slots.py @@ -0,0 +1,63 @@ +"""Slot classification for the Decision Context capability ids (Refs #4447). + +Track A of #4447 flags ``DECISION_CONTEXT_CAPABILITY_ID`` as a same-name, +different-value fork: ``decision-context`` in ``extension_provider.py`` and +``decision_context`` in ``packets.py``. Checking the callers shows these are +two slots, not a conflict: + +* the hyphenated spelling is the extension/catalog/CLI namespace, and it is + the spelling every ``loopx/capabilities/*/catalog_entry.py`` id uses; +* the underscore spelling is the capability packet contract, and it is the + spelling the sibling capability emits (``material_lifecycle``). + +No consumer joins the two, and each namespace is internally consistent, so +both values are retained. The constants now name their slot; these tests lock +the classification so a later "unification" cannot quietly break either +extension lookup or packet consumers. +""" + +from __future__ import annotations + +from loopx.capabilities.decision_context.catalog_entry import ( + DECISION_CONTEXT_CATALOG_ENTRY, +) +from loopx.capabilities.decision_context.extension_provider import ( + DECISION_CONTEXT_EXTENSION_CAPABILITY_ID, +) +from loopx.capabilities.decision_context.packets import ( + DECISION_CONTEXT_PACKET_CAPABILITY_ID, +) +from loopx.capabilities.material_lifecycle.catalog_entry import ( + MATERIAL_LIFECYCLE_CATALOG_ENTRY, +) + + +def test_extension_binding_id_uses_the_hyphenated_catalog_spelling() -> None: + assert DECISION_CONTEXT_EXTENSION_CAPABILITY_ID == "decision-context" + assert ( + DECISION_CONTEXT_EXTENSION_CAPABILITY_ID + == DECISION_CONTEXT_CATALOG_ENTRY["id"] + ) + assert "_" not in DECISION_CONTEXT_EXTENSION_CAPABILITY_ID + + +def test_packet_contract_id_uses_the_underscore_spelling() -> None: + assert DECISION_CONTEXT_PACKET_CAPABILITY_ID == "decision_context" + assert "-" not in DECISION_CONTEXT_PACKET_CAPABILITY_ID + + +def test_the_two_slots_are_not_the_same_value() -> None: + assert ( + DECISION_CONTEXT_PACKET_CAPABILITY_ID + != DECISION_CONTEXT_EXTENSION_CAPABILITY_ID + ) + + +def test_sibling_capability_follows_the_same_two_slot_split() -> None: + """`material-lifecycle` / `material_lifecycle` is the same split. + + The constant pairs differ only in namespace, so the sibling capability is + the evidence that these are conventions and not one-off drift. + """ + assert MATERIAL_LIFECYCLE_CATALOG_ENTRY["id"] == "material-lifecycle" + assert DECISION_CONTEXT_CATALOG_ENTRY["id"] == "decision-context" From 93ff89a3076fcf99892fa07faf31c9435a0c00aa Mon Sep 17 00:00:00 2001 From: YZJF <195568136+YZJF@users.noreply.github.com> Date: Wed, 16 Sep 2026 16:36:58 +0800 Subject: [PATCH 2/2] fix(kunluncode): single-source the adapter mcp pin Refs #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 (892faa2c9). 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> --- loopx/kunluncode_goal_mode/cli.py | 17 ++++++++++++++--- tests/test_kunluncode_goal_mode.py | 19 +++++++++++++++++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/loopx/kunluncode_goal_mode/cli.py b/loopx/kunluncode_goal_mode/cli.py index cec051dbec..99b141beb8 100644 --- a/loopx/kunluncode_goal_mode/cli.py +++ b/loopx/kunluncode_goal_mode/cli.py @@ -25,7 +25,17 @@ from loopx.registry import atomic_write_json -MCP_REQUIREMENT = "mcp==1.28.1" +# Single source for the adapter's `mcp` pin. It is an exact pin, not a range: +# loopx/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 (892faa2c9 +# "fix(security): upgrade the MCP SDK pin"). It belongs to the KunlunCode +# adapter venv only; loopx/claude_goal_mode/scripts/install.py provisions its +# own venv and spells the same dependency as a range ("mcp<2"). The two are +# separate packaging boundaries, so they are not required to agree on a +# spelling. Bump MCP_SDK_VERSION alone; the probe and the user-facing message +# below derive from it. +MCP_SDK_VERSION = "1.28.1" +MCP_REQUIREMENT = f"mcp=={MCP_SDK_VERSION}" MCP_SCRIPT = Path(__file__).with_name("server.py").resolve() DEFAULT_MCP_VENV = ( Path.home() / ".local" / "share" / "loopx" / "kunluncode-mcp" / ".venv" @@ -65,7 +75,7 @@ def _compatible_python(value: str | Path) -> bool: "from importlib.metadata import version; " "from mcp.server.fastmcp import FastMCP; " "import loopx.kunluncode_goal_mode.server; " - "assert version('mcp') == '1.28.1'" + f"assert version('mcp') == '{MCP_SDK_VERSION}'" ), ], timeout=30, @@ -192,7 +202,8 @@ def install_mcp(*, python: str | None, dry_run: bool, replace: bool) -> str: selected_python = provision_mcp_python(dry_run=dry_run) if not dry_run and not _compatible_python(selected_python): raise RuntimeError( - f"{selected_python} must import LoopX and mcp==1.28.1; use uv to sync the adapter environment" + f"{selected_python} must import LoopX and {MCP_REQUIREMENT}; " + "use uv to sync the adapter environment" ) existing = next( ( diff --git a/tests/test_kunluncode_goal_mode.py b/tests/test_kunluncode_goal_mode.py index abca8286da..3b685e7ae3 100644 --- a/tests/test_kunluncode_goal_mode.py +++ b/tests/test_kunluncode_goal_mode.py @@ -1,6 +1,7 @@ from __future__ import annotations import asyncio +import inspect import json import subprocess from pathlib import Path @@ -1597,3 +1598,21 @@ def factory(command: list[str], **kwargs) -> _FakeAppServer: assert state is not None assert state["native"]["status"] == "complete" assert state["native"]["verified"] is False + + +def test_mcp_pin_has_one_source_of_truth() -> None: + """Refs #4447: the `mcp` pin is defined once and derived everywhere else. + + The pin is deliberately exact, not a range: the server imports + `mcp.server.fastmcp`, which the MCP SDK 2.x line no longer ships, and the + pin was set by a security fix. Before this change the version string was + repeated in the requirement, in the compatibility probe and in the + user-facing error, so a bump could leave a stale probe behind. + """ + probe_source = inspect.getsource(cli._compatible_python) + + assert cli.MCP_SDK_VERSION + assert cli.MCP_REQUIREMENT == f"mcp=={cli.MCP_SDK_VERSION}" + assert cli.MCP_REQUIREMENT.startswith("mcp==") + assert cli.MCP_SDK_VERSION not in probe_source + assert "MCP_SDK_VERSION" in probe_source