Skip to content

Commit f3a53a4

Browse files
committed
feat: extend dead-canonical-constant guard to registry/tools/tags/names
Add a generalized arch test that scans every module-level constant matching *_REGISTRY, *_TOOLS, *_TAGS, or *_NAMES in _type_constants_registries.py and _type_constants.py, asserting each is either imported by production src/ code or documented in _REGISTRY_EXEMPTIONS with a rationale. Two real consumers were missing and are now wired up: - FLEET_DISPATCH_TOOLS is imported in server/tools/tools_recipe.py with a load-bearing structural assertion that it is a subset of GATED_TOOLS. - RETIRED_AGENT_NAMES is imported in server/tools/tools_agents.py and used to short-circuit unlock_agent_pack with a 'has been retired' error before touching enable_components. FREE_RANGE_TOOLS and FLEET_TOOLS are exempt (alias-derived and test-consumed respectively) with documented rationales. Tests: adds test_registry_constants_have_production_consumer, test_exemption_rationales_are_nonempty, test_exemptions_reference_real_constants, and test_unlock_agent_pack_rejects_retired_names.
1 parent 2fdc0f4 commit f3a53a4

4 files changed

Lines changed: 117 additions & 2 deletions

File tree

src/autoskillit/server/tools/tools_agents.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
from fastmcp.dependencies import CurrentContext
77
from fastmcp.exceptions import ResourceError
88

9-
from autoskillit.core import AGENT_PACK_REGISTRY, get_logger, pkg_root
9+
from autoskillit.core import AGENT_PACK_REGISTRY, RETIRED_AGENT_NAMES, get_logger, pkg_root
1010
from autoskillit.server import mcp
1111
from autoskillit.server.tools._cancellation_shield import _cancellation_shield
1212

@@ -53,6 +53,10 @@ async def unlock_agent_pack(pack_name: str, ctx: Context = CurrentContext()) ->
5353
Never raises.
5454
"""
5555
try:
56+
if pack_name in RETIRED_AGENT_NAMES:
57+
return json.dumps(
58+
{"success": False, "error": f"Agent pack {pack_name!r} has been retired."}
59+
)
5660
if pack_name not in AGENT_PACK_TAGS:
5761
return json.dumps({"success": False, "error": f"Unknown agent pack: {pack_name}"})
5862
tags = AGENT_PACK_TAGS[pack_name]

src/autoskillit/server/tools/tools_recipe.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
build_config_authoritative_layer,
1414
resolve_ingredient_defaults,
1515
)
16-
from autoskillit.core import get_logger, temp_dir_display_str
16+
from autoskillit.core import FLEET_DISPATCH_TOOLS, get_logger, temp_dir_display_str
1717
from autoskillit.pipeline import GATED_TOOLS, UNGATED_TOOLS # noqa: F401
1818
from autoskillit.server import mcp
1919
from autoskillit.server._guards import _require_enabled
@@ -30,6 +30,11 @@
3030
)
3131
from autoskillit.server.tools._cancellation_shield import _cancellation_shield
3232

33+
assert FLEET_DISPATCH_TOOLS <= GATED_TOOLS, (
34+
"FLEET_DISPATCH_TOOLS must be a subset of GATED_TOOLS — "
35+
f"extra: {sorted(FLEET_DISPATCH_TOOLS - GATED_TOOLS)}"
36+
)
37+
3338
logger = get_logger(__name__)
3439

3540

tests/arch/test_canonical_constant_consumption.py

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,3 +69,93 @@ def test_env_forward_constants_have_production_consumer() -> None:
6969
f"consumers: {unconsumed}. Each env-var-set constant must be imported and consumed "
7070
f"by production code to prevent dead-canonical-constant drift."
7171
)
72+
73+
74+
_REGISTRY_CANONICAL_PATTERN = re.compile(r"^[A-Z][A-Z0-9_]*(?:_REGISTRY|_TOOLS|_TAGS|_NAMES)$")
75+
76+
_REGISTRY_EXEMPTIONS: dict[str, str] = {
77+
"FREE_RANGE_TOOLS": (
78+
"alias-derived: backing constant for UNGATED_TOOLS which is imported "
79+
"in pipeline/gate.py and server/tools/tools_recipe.py; "
80+
"direct import would be redundant"
81+
),
82+
"FLEET_TOOLS": (
83+
"test-consumed: architectural enforcement constant imported by "
84+
"test_layer_enforcement.py, test_transforms_hygiene.py, and "
85+
"test_lifespan_fleet_boot.py for fleet tool-tag parity guards; "
86+
"no runtime production consumer needed"
87+
),
88+
}
89+
90+
91+
def _find_registry_constants(constants_file: Path) -> list[str]:
92+
"""Find all module-level names matching registry/tools/tags/names patterns."""
93+
tree = ast.parse(constants_file.read_text())
94+
names: list[str] = []
95+
for node in ast.iter_child_nodes(tree):
96+
if isinstance(node, ast.AnnAssign) and isinstance(node.target, ast.Name):
97+
if _REGISTRY_CANONICAL_PATTERN.match(node.target.id):
98+
names.append(node.target.id)
99+
elif isinstance(node, ast.Assign):
100+
for target in node.targets:
101+
if isinstance(target, ast.Name) and _REGISTRY_CANONICAL_PATTERN.match(target.id):
102+
names.append(target.id)
103+
return names
104+
105+
106+
def test_registry_constants_have_production_consumer() -> None:
107+
"""Every registry/tools/tags/names constant must be imported by production code or exempted."""
108+
from autoskillit.core import paths
109+
110+
src_root = paths.pkg_root()
111+
constants_files = [
112+
src_root / "core" / "types" / "_type_constants_registries.py",
113+
src_root / "core" / "types" / "_type_constants.py",
114+
]
115+
116+
all_constants: list[tuple[str, Path]] = []
117+
for cf in constants_files:
118+
for name in _find_registry_constants(cf):
119+
all_constants.append((name, cf))
120+
121+
assert all_constants, "No registry constants found — test premise broken"
122+
123+
unconsumed = []
124+
for name, def_file in all_constants:
125+
if name in _REGISTRY_EXEMPTIONS:
126+
continue
127+
if not _has_production_import(src_root, name, def_file):
128+
unconsumed.append(name)
129+
130+
assert not unconsumed, (
131+
f"Registry constants (*_REGISTRY / *_TOOLS / *_TAGS / *_NAMES) with zero "
132+
f"production consumers and no exemption: {unconsumed}. Each constant must be "
133+
f"imported by production src/ code or documented in _REGISTRY_EXEMPTIONS "
134+
f"with a rationale."
135+
)
136+
137+
138+
def test_exemption_rationales_are_nonempty() -> None:
139+
"""Every exemption must have a non-empty rationale string."""
140+
empty = [k for k, v in _REGISTRY_EXEMPTIONS.items() if not v.strip()]
141+
assert not empty, f"Exemptions with empty rationales: {empty}"
142+
143+
144+
def test_exemptions_reference_real_constants() -> None:
145+
"""Every exempted name must exist as a constant in the scanned files."""
146+
from autoskillit.core import paths
147+
148+
src_root = paths.pkg_root()
149+
constants_files = [
150+
src_root / "core" / "types" / "_type_constants_registries.py",
151+
src_root / "core" / "types" / "_type_constants.py",
152+
]
153+
all_names: set[str] = set()
154+
for cf in constants_files:
155+
all_names.update(_find_registry_constants(cf))
156+
157+
stale = set(_REGISTRY_EXEMPTIONS.keys()) - all_names
158+
assert not stale, (
159+
f"Exemption dict contains names not found in constants files: {sorted(stale)}. "
160+
f"Remove stale entries."
161+
)

tests/server/test_tools_agents.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,22 @@ async def test_unlock_agent_pack_unknown_name():
122122
assert "nonexistent" in data["error"]
123123

124124

125+
# T7b: unlock_agent_pack with retired name returns error
126+
@pytest.mark.anyio
127+
async def test_unlock_agent_pack_rejects_retired_names():
128+
"""Retired agent names must be rejected with a structured error."""
129+
from autoskillit.core import RETIRED_AGENT_NAMES
130+
from autoskillit.server.tools.tools_agents import unlock_agent_pack
131+
132+
assert RETIRED_AGENT_NAMES, "RETIRED_AGENT_NAMES must not be empty — test premise broken"
133+
name = next(iter(sorted(RETIRED_AGENT_NAMES)))
134+
mock_ctx = _make_mock_ctx()
135+
result = json.loads(await unlock_agent_pack(name, ctx=mock_ctx))
136+
assert result["success"] is False
137+
assert "retired" in result["error"].lower()
138+
assert name in result["error"]
139+
140+
125141
# T8: unlock_agent_pack enables agent resources in same session
126142
@pytest.mark.anyio
127143
async def test_unlock_agent_pack_enables_resources():

0 commit comments

Comments
 (0)