Move TOOL_CATEGORIES from L0 Core to L3 Server Layer (#439)#679
Conversation
Define local _DISPLAY_CATEGORIES constants in server/tools_kitchen.py and cli/_cook.py, splitting CI tools out of "GitHub" into a new "CI & Automation" category. Both consumers now use the local constant instead of importing TOOL_CATEGORIES from L0 core. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Delete the TOOL_CATEGORIES constant from _type_constants.py and remove all re-exports from core/__init__.py and core/types.py. The presentation constant now lives exclusively in its L3 consumers. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace test_tool_categories_covers_all_tools with three new tests: - test_display_categories_sync: validates both L3 copies cover all tools - test_tool_categories_not_in_core: asserts L0 no longer exports it - test_ci_tools_not_in_github_category: verifies CI tool recategorization Update test_open_kitchen_includes_categorized_tool_listing to import from the local server constant instead of core.types. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
|
||
| from autoskillit.cli._terminal import terminal_guard | ||
|
|
||
| _DISPLAY_CATEGORIES: tuple[tuple[str, tuple[str, ...]], ...] = ( |
There was a problem hiding this comment.
[warning] arch: _DISPLAY_CATEGORIES is now defined identically in both cli/_cook.py and server/tools_kitchen.py. These two copies must be kept manually in sync. A single canonical definition in a shared L1/L2 helper module consumed by both L3 consumers would remove the divergence risk entirely. The current approach trades an L0 layering violation for a maintenance hazard between two L3 modules. The test guard (test_display_categories_sync) catches drift but does not prevent it.
|
|
||
| logger = get_logger(__name__) | ||
|
|
||
| _DISPLAY_CATEGORIES: tuple[tuple[str, tuple[str, ...]], ...] = ( |
There was a problem hiding this comment.
[warning] arch: Identical _DISPLAY_CATEGORIES literal duplicated in server/tools_kitchen.py and cli/_cook.py. Both copies must stay in sync manually; the test guard catches drift but does not prevent it. Consider extracting to a shared module if the layering architecture permits.
|
|
||
| kitchen_flat = {t for _, tools in kitchen_cats for t in tools} | ||
| cook_flat = {t for _, tools in cook_cats for t in tools} | ||
| assert kitchen_flat == cook_flat, "kitchen and cook _DISPLAY_CATEGORIES differ" |
There was a problem hiding this comment.
[warning] cohesion: test_display_categories_sync asserts flat set equality of tool names but does not verify structural parity — category names and their per-category membership are not compared. A divergence in how tools are grouped across categories (e.g., a tool moved from 'GitHub' to 'CI & Automation' in one copy but not the other) would pass this test while producing inconsistent displays to users. Consider asserting kitchen_cats == cook_cats for full structural identity.
Summary
Move the
TOOL_CATEGORIESpresentation constant out of the L0 foundation layer (core/_type_constants.py) into local_DISPLAY_CATEGORIESconstants in its two L3 consumers (server/tools_kitchen.pyandcli/_cook.py). Fix the CI tool miscategorization (from "GitHub" to "CI & Automation") in both copies. Delete the original constant from L0. Replace the existing coverage test with a new sync test that validates both local copies, and add a layer enforcement test confirming L0 no longer exportsTOOL_CATEGORIES.Requirements
CONST
TOOL_CATEGORIESmust not be defined in or exported fromsrc/autoskillit/core/.tools_kitchen.py,_cook.py) must define its own local_DISPLAY_CATEGORIESconstant.core/.CAT
wait_for_ci,wait_for_merge_queue,toggle_auto_merge,get_ci_status) must appear under a category name consistent with theirTOOL_SUBSET_TAGSclassification ("ci"), not under"GitHub".GATED_TOOLS | HEADLESS_TOOLS | FREE_RANGE_TOOLSmust appear in exactly one_DISPLAY_CATEGORIESentry in each consumer (coverage invariant preserved).TEST
_DISPLAY_CATEGORIEScopies cover all registered tools exactly.core/_type_constants.pyno longer exportsTOOL_CATEGORIES.Architecture Impact
Module Dependency Diagram
%%{init: {'flowchart': {'nodeSpacing': 50, 'rankSpacing': 70, 'curve': 'basis'}}}%% graph TB %% CLASS DEFINITIONS %% classDef cli fill:#1a237e,stroke:#7986cb,stroke-width:2px,color:#fff; classDef stateNode fill:#004d40,stroke:#4db6ac,stroke-width:2px,color:#fff; classDef handler fill:#e65100,stroke:#ffb74d,stroke-width:2px,color:#fff; classDef phase fill:#6a1b9a,stroke:#ba68c8,stroke-width:2px,color:#fff; classDef integration fill:#c62828,stroke:#ef9a9a,stroke-width:2px,color:#fff; classDef detector fill:#b71c1c,stroke:#ef5350,stroke-width:2px,color:#fff; classDef output fill:#00695c,stroke:#4db6ac,stroke-width:2px,color:#fff; subgraph L3_Server ["L3 — SERVER"] direction LR TK["● server/tools_kitchen.py<br/>━━━━━━━━━━<br/>+_DISPLAY_CATEGORIES<br/>open/close_kitchen handlers"] SH["server/helpers.py<br/>━━━━━━━━━━<br/>Shared server helpers"] SI["server/__init__.py<br/>━━━━━━━━━━<br/>FastMCP app + gating"] end subgraph L3_CLI ["L3 — CLI"] direction LR CK["● cli/_cook.py<br/>━━━━━━━━━━<br/>+_DISPLAY_CATEGORIES<br/>cook interactive session"] CA["cli/app.py<br/>━━━━━━━━━━<br/>CLI entry point"] end subgraph L3_Tests ["L3 — TESTS (arch enforcement)"] direction LR TLE["● tests/arch/test_layer_enforcement.py<br/>━━━━━━━━━━<br/>+_DISPLAY_CATEGORIES sync tests<br/>-TOOL_CATEGORIES test"] TTK["● tests/server/test_tools_kitchen.py<br/>━━━━━━━━━━<br/>Updated tool listing assertions"] end subgraph L1 ["L1 — SERVICES"] direction LR CFG["config/<br/>━━━━━━━━━━<br/>Settings, defaults"] PIP["pipeline/<br/>━━━━━━━━━━<br/>Gate, audit, tokens<br/>re-exports GATED_TOOLS"] EXE["execution/<br/>━━━━━━━━━━<br/>Headless, subprocess"] WKS["workspace/<br/>━━━━━━━━━━<br/>Skills, sessions, clones"] end subgraph L0 ["L0 — FOUNDATION"] direction LR CINIT["● core/__init__.py<br/>━━━━━━━━━━<br/>Package gateway<br/>-TOOL_CATEGORIES re-export"] TCON["● core/_type_constants.py<br/>━━━━━━━━━━<br/>-TOOL_CATEGORIES (removed)<br/>GATED_TOOLS, PACK_REGISTRY, etc."] CTYP["core/types.py<br/>━━━━━━━━━━<br/>Re-export hub<br/>from _type_constants import *"] end %% L0 INTERNAL CHAIN %% TCON -->|"wildcard re-export"| CTYP CTYP -->|"re-export"| CINIT %% L3 SERVER → lower layers %% TK -->|"PIPELINE_FORBIDDEN_TOOLS<br/>atomic_write, get_logger, pkg_root"| CINIT TK -->|"resolve_ingredient_defaults"| CFG TK -->|"create_background_task"| PIP TK -->|"helpers"| SH TK -->|"mcp instance"| SI SI -->|"_build_tool_category_listing"| TK %% L3 CLI → lower layers %% CK -->|"configure_logging<br/>find_latest_session_id, pkg_root"| CINIT CK -->|"load_config"| CFG CK -->|"build_interactive_cmd"| EXE CK -->|"DefaultSessionSkillManager<br/>SkillsDirectoryProvider"| WKS CA -->|"cook"| CK %% L1 → L0 %% PIP -->|"GATED_TOOLS, UNGATED_TOOLS"| CINIT CFG -->|"CATEGORY_TAGS, PACK_REGISTRY"| CINIT WKS -->|"PACK_REGISTRY"| CINIT %% TESTS → targets %% TLE -.->|"validates sync"| TK TLE -.->|"validates sync"| CK TLE -.->|"asserts absence"| TCON TTK -.->|"tests handlers"| TK %% _DISPLAY_CATEGORIES duplication link %% TK -.-|"identical _DISPLAY_CATEGORIES<br/>(enforced by test_display_categories_sync)"| CK %% CLASS ASSIGNMENTS %% class TK,SI,SH cli; class CK,CA cli; class CFG,PIP,EXE,WKS phase; class CINIT,TCON,CTYP stateNode; class TLE,TTK detector;Closes #439
Implementation Plan
Plan file:
/home/talon/projects/autoskillit-runs/impl-20260408-171455-140418/.autoskillit/temp/make-plan/move_tool_categories_plan_2026-04-08_171455.md🤖 Generated with Claude Code via AutoSkillit
Token Usage Summary