Skip to content

Commit aa6b884

Browse files
Trecekclaude
andauthored
[FIX] Fix-Required Dispatch Gate Cross-Registry Immunity (#4086)
## Summary The Codex backend is silently bricked whenever a `HookDef` in `HOOK_REGISTRY` is reclassified to `codex_status="fix-required"` and its guard script is not in `BackendCapabilities.applicable_guards` for the Codex backend. The dispatch gate in `_check_backend_compat` (`tools_execution.py:150–163`) refuses all Codex `run_skill` dispatches when this cross-registry invariant is violated. CI does not catch it because every test exercising the gate uses monkeypatched synthetic registries — no test validates the real `HOOK_REGISTRY` against real backend capabilities. This is the fourth instance of the same bug class: 1. `test_check` capability misclassified as `not-applicable` (#3804) 2. `run_skill` capability misclassified as `not-applicable` (#3838) 3. Severity promotion of semantic rule broke all fleet dispatches (#3949) 4. `skill_load_guard` hook reclassified to `fix-required`, bricking Codex dispatch (current — #4082) The architectural weakness is that `codex_status` is a manually-declared static field validated only against its own internal consistency, not against actual runtime dispatch behavior. The fix must make cross-registry invariant violations fail at test time, not at runtime dispatch. ## Requirements ## Conflict Resolution Decisions The following files had merge conflicts that were automatically resolved. Closes #4082 ## Implementation Plan Plan file: `/home/talon/projects/autoskillit-runs/remediation-20260611-215147-829611/.autoskillit/temp/rectify/rectify_fix_required_dispatch_gate_cross_registry_immunity_2026-06-11_215739.md` 🤖 Generated with [Claude Code](https://claude.com/claude-code) via AutoSkillit <!-- autoskillit:pipeline-signature steps=prepare_pr,run_arch_lenses,compose_pr,annotate_pr_diff,review_pr --> ## Token Usage Summary | Step | Model | count | uncached | output | cache_read | peak_ctx | turns | cache_write | time | |------|-------|-------|----------|--------|------------|----------|-------|-------------|------| | rectify* | opus[1m] | 1 | 90 | 27.5k | 3.6M | 149.7k | 84 | 135.7k | 24m 23s | | review_approach* | opus[1m] | 1 | 24 | 5.7k | 232.3k | 52.5k | 13 | 34.6k | 6m 2s | | dry_walkthrough* | opus | 2 | 3.6k | 18.9k | 1.6M | 77.7k | 60 | 94.7k | 10m 33s | | implement* | MiniMax-M3 | 2 | 5.2M | 10.9k | 0 | 0 | 101 | 0 | 11m 10s | | retry_worktree* | opus[1m] | 1 | 45 | 6.0k | 1.5M | 77.3k | 44 | 60.4k | 10m 38s | | audit_impl* | opus[1m] | 2 | 61 | 17.7k | 653.2k | 60.5k | 39 | 69.3k | 9m 1s | | make_plan* | opus[1m] | 1 | 46 | 3.7k | 504.4k | 58.0k | 17 | 38.1k | 2m 36s | | prepare_pr* | MiniMax-M3 | 1 | 284.4k | 1.3k | 0 | 0 | 12 | 0 | 47s | | compose_pr* | MiniMax-M3 | 1 | 299.0k | 1.9k | 0 | 0 | 12 | 0 | 54s | | **Total** | | | 5.8M | 93.7k | 8.2M | 149.7k | | 432.8k | 1h 16m | \* *Step used a non-Anthropic provider; caching behavior may differ.* ## Token Efficiency | Step | LoC Changed | cache_read/LoC | cache_write/LoC | output/LoC | |------|-------------|----------------|-----------------|------------| | rectify | 0 | — | — | — | | review_approach | 0 | — | — | — | | dry_walkthrough | 0 | — | — | — | | implement | 149 | 0.0 | 0.0 | 73.2 | | retry_worktree | 7 | 220617.0 | 8630.6 | 864.0 | | audit_impl | 0 | — | — | — | | make_plan | 0 | — | — | — | | prepare_pr | 0 | — | — | — | | compose_pr | 0 | — | — | — | | **Total** | **156** | 52316.9 | 2774.6 | 600.6 | ## Model Usage Breakdown | Model | steps | uncached | output | cache_read | cache_write | time | |-------|-------|----------|--------|------------|-------------|------| | opus[1m] | 5 | 266 | 60.7k | 6.6M | 338.1k | 52m 43s | | opus | 1 | 3.6k | 18.9k | 1.6M | 94.7k | 10m 33s | | MiniMax-M3 | 3 | 5.8M | 14.1k | 0 | 0 | 12m 51s | --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent befdb00 commit aa6b884

5 files changed

Lines changed: 141 additions & 1 deletion

File tree

src/autoskillit/core/types/_type_constants_registries.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -310,6 +310,14 @@ def required_backends(self) -> frozenset[str]:
310310
return frozenset()
311311

312312

313+
# Semantics divergence: fix-required has different enforcement in HOOK_REGISTRY vs
314+
# SKILL_CAPABILITY_REGISTRY. In HOOK_REGISTRY, fix-required triggers _check_backend_compat
315+
# to block dispatch on backends whose applicable_guards don't cover the hook's scripts.
316+
# In SKILL_CAPABILITY_REGISTRY, fix-required is advisory only — it does NOT block dispatch
317+
# (required_backends returns frozenset() for all non-not-applicable statuses). Skills
318+
# declaring fix-required capabilities (agent_subagent, agent_model, cross_skill_ref) are
319+
# still Codex-dispatchable because the capability is documentary about the feature
320+
# being incomplete, not a hard blocker. Do not conflate the two registries' semantics.
313321
SKILL_CAPABILITY_REGISTRY: dict[str, SkillCapabilityDef] = {
314322
"agent_subagent": SkillCapabilityDef(
315323
description="Agent(subagent_type=...) tool — delegates to specialized subagent",

src/autoskillit/server/_lifespan.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
session_type as _resolve_session_type,
4646
)
4747
from autoskillit.execution import (
48+
BACKEND_REGISTRY,
4849
RecordingSubprocessRunner,
4950
ensure_codex_mcp_registered,
5051
)
@@ -54,6 +55,7 @@
5455
sweep_stale_dispatch_labels,
5556
)
5657
from autoskillit.hook_registry import (
58+
HOOK_REGISTRY,
5759
HOOK_REGISTRY_HASH,
5860
find_broken_hook_scripts,
5961
generate_hooks_json,
@@ -126,6 +128,42 @@ def run_startup_hook_health_check() -> list[str]:
126128
return []
127129

128130

131+
def run_startup_fix_required_coverage_check() -> None:
132+
"""Validate that fix-required hook script stems are covered by at least one backend.
133+
134+
The dispatch gate in tools_execution._check_backend_compat refuses all skill
135+
dispatches on a backend if HOOK_REGISTRY contains fix-required hooks whose
136+
script stems are not in that backend's applicable_guards. This check provides
137+
defense-in-depth: if the cross-registry invariant is violated, the server
138+
fails to start rather than accepting requests it will later crash on.
139+
140+
Raises RuntimeError if any fix-required hook's script stems are not covered
141+
by the union of all registered backends' applicable_guards. A fix-required
142+
hook that IS covered by at least one backend is valid and does not raise.
143+
"""
144+
all_guards: set[str] = set()
145+
for cls in BACKEND_REGISTRY.values():
146+
try:
147+
all_guards.update(cls().capabilities.applicable_guards)
148+
except Exception as exc:
149+
raise RuntimeError(
150+
f"Backend {cls.__name__!r} constructor raised during startup "
151+
f"fix-required coverage check: {exc}"
152+
) from exc
153+
for h in HOOK_REGISTRY:
154+
if h.codex_status != "fix-required":
155+
continue
156+
stems = frozenset(Path(s).stem for s in h.scripts) if h.scripts else frozenset()
157+
if stems and not stems.issubset(all_guards):
158+
missing = sorted(stems - all_guards)
159+
raise RuntimeError(
160+
f"HOOK_REGISTRY fix-required entry (matcher={h.matcher!r}) has "
161+
f"guard scripts {missing} not covered by any backend's "
162+
f"applicable_guards. This will brick dispatch for backends "
163+
f"missing these guards."
164+
)
165+
166+
129167
def _finalize_recorder() -> None:
130168
"""Finalize the recording subprocess runner if one is active."""
131169
ctx = _get_ctx_or_none()
@@ -529,6 +567,8 @@ async def _autoskillit_lifespan(server: Any) -> Any:
529567
try:
530568
from autoskillit.server import _state # circular-break
531569

570+
run_startup_fix_required_coverage_check()
571+
532572
event = _asyncio.Event()
533573
_state._startup_ready = event
534574
write_readiness_sentinel()

tests/arch/AGENTS.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ AST enforcement, sub-package layer contracts, and architectural invariant tests.
1919
| `test_canonical_constant_consumption.py` | Architectural invariant: every *_ENV_FORWARD_VARS constant must have a production consumer |
2020
| `test_backend_name_sync.py` | Architectural invariant: KNOWN_BACKEND_NAMES (IL-0) must match BACKEND_REGISTRY keys (IL-1) |
2121
| `test_capability_consistency.py` | Behavioral arch tests: BackendCapabilities filesystem consistency — applicable guards exist on disk, required session files are created, session_dir_symlinks entries are symlinks |
22+
| `test_cross_registry_dispatch_sufficiency.py` | Cross-registry dispatch sufficiency: real HOOK_REGISTRY × real BackendCapabilities — detects fix-required hooks that would brick backends with insufficient applicable_guards |
2223
| `test_capability_consumption.py` | Architectural invariant: every BackendCapabilities field must be consumed in production |
2324
| `test_capability_docstrings.py` | Architectural invariant: BackendCapabilities must have class and field documentation |
2425
| `test_no_backend_name_bypass.py` | Architectural invariant: backend-specific behavior must use capability fields, not name comparisons |
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
"""Cross-registry dispatch sufficiency: real HOOK_REGISTRY × real BackendCapabilities.
2+
3+
Regression suite for issue #4082 — guards against a class of bug where HOOK_REGISTRY
4+
fix-required entries silently brick dispatch on backends whose applicable_guards do
5+
not cover the hook's script stems. The dispatch gate in tools_execution._check_backend_compat
6+
correctly refuses dispatch in this state, but the bug class had no test that crossed the
7+
HOOK_REGISTRY ↔ BackendCapabilities boundary until now.
8+
9+
These tests exercise the real registries through the real gate logic, not synthetic
10+
monkeypatched copies. This is the defense-in-depth layer that catches reclassifications
11+
which would brick dispatch silently.
12+
"""
13+
14+
from __future__ import annotations
15+
16+
from pathlib import Path
17+
18+
import pytest
19+
20+
from autoskillit.execution.backends import BACKEND_REGISTRY
21+
from autoskillit.hook_registry import HOOK_REGISTRY
22+
from autoskillit.server.tools.tools_execution import _get_fix_required_hook_matchers
23+
24+
pytestmark = [pytest.mark.layer("arch"), pytest.mark.small]
25+
26+
27+
def _collect_fix_required_stems() -> set[str]:
28+
"""Extract unique script stems from all fix-required hooks in HOOK_REGISTRY."""
29+
stems: set[str] = set()
30+
for h in HOOK_REGISTRY:
31+
if h.codex_status == "fix-required":
32+
stems.update(Path(s).stem for s in h.scripts)
33+
return stems
34+
35+
36+
def _all_fix_required_matchers() -> list[str]:
37+
"""Extract matchers from all fix-required hooks in HOOK_REGISTRY."""
38+
return [h.matcher for h in HOOK_REGISTRY if h.codex_status == "fix-required"]
39+
40+
41+
@pytest.mark.parametrize("backend_name,backend_cls", sorted(BACKEND_REGISTRY.items()))
42+
def test_no_backend_bricked_by_fix_required_hooks(backend_name: str, backend_cls: type) -> None:
43+
"""Every backend's applicable_guards must cover all fix-required hook scripts.
44+
45+
Runs the exact same function the dispatch gate uses — _get_fix_required_hook_matchers —
46+
against the real HOOK_REGISTRY and the real backend's real applicable_guards set.
47+
A non-empty result means this backend would be bricked at dispatch time.
48+
"""
49+
backend = backend_cls()
50+
blockers = _get_fix_required_hook_matchers(backend.capabilities.applicable_guards)
51+
assert not blockers, (
52+
f"Backend {backend_name!r} would be bricked at dispatch by fix-required "
53+
f"hooks with matchers {blockers}. Either reclassify the hook's codex_status "
54+
f"or add the missing guard to applicable_guards."
55+
)
56+
57+
58+
@pytest.mark.parametrize("backend_name,backend_cls", sorted(BACKEND_REGISTRY.items()))
59+
def test_applicable_guards_covers_all_fix_required_scripts(
60+
backend_name: str, backend_cls: type
61+
) -> None:
62+
"""Set-level invariant: applicable_guards ⊇ fix-required hook script stems.
63+
64+
A parallel, structural assertion that doesn't go through the gate function.
65+
Detects the same class of bug from the registry-composition angle.
66+
"""
67+
backend = backend_cls()
68+
fix_required_stems = _collect_fix_required_stems()
69+
missing = fix_required_stems - backend.capabilities.applicable_guards
70+
assert not missing, (
71+
f"Backend {backend_name!r} applicable_guards is missing script stems "
72+
f"{sorted(missing)} required by fix-required hooks in HOOK_REGISTRY."
73+
)
74+
75+
76+
def test_test_matrix_covers_all_registered_backends() -> None:
77+
"""Meta-test: the parametrized matrix must cover every backend in BACKEND_REGISTRY.
78+
79+
Prevents silent matrix shrinkage if a backend is removed from parametrization.
80+
"""
81+
expected = {"claude-code", "codex"}
82+
actual = set(BACKEND_REGISTRY.keys())
83+
assert len(actual) >= 2, (
84+
f"BACKEND_REGISTRY has only {len(actual)} backend(s) — "
85+
f"expected at least 2 (claude-code + codex). If a backend was removed, "
86+
f"update this lower bound."
87+
)
88+
assert actual == expected, (
89+
f"BACKEND_REGISTRY keys {sorted(actual)} != expected {sorted(expected)}. "
90+
f"Update the expected set in this meta-test when backends are added or removed."
91+
)

tests/infra/test_schema_version_convention.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -117,7 +117,7 @@ def _is_yaml_dump(node: ast.expr) -> bool:
117117
# staleness_cache.py — cache dict
118118
("src/autoskillit/recipe/staleness_cache.py", 67),
119119
# _lifespan.py — hooks.json self-heal on startup drift (co-owned with Claude plugin system)
120-
("src/autoskillit/server/_lifespan.py", 87),
120+
("src/autoskillit/server/_lifespan.py", 89),
121121
# tools_kitchen.py — hook config, quota guard, git_ops_policy, ingredient locks overlay
122122
("src/autoskillit/server/tools/tools_kitchen.py", 167),
123123
("src/autoskillit/server/tools/tools_kitchen.py", 186),

0 commit comments

Comments
 (0)