Skip to content

Commit bfc5d2c

Browse files
authored
Implementation Plan: Surface a Structured crashed Result at Dispatch for Fix-Required Hooks (#4070)
## Summary Add a dispatch-time gate in `_check_backend_compat` that refuses sessions when `HOOK_REGISTRY` contains any `HookDef` with `codex_status == "fix-required"` whose guard scripts are not in the backend's `applicable_guards` capability. The gate returns a `SkillResult.crashed()` JSON envelope listing the affected hook matchers. Claude-code sessions pass through because their `applicable_guards` includes `skill_load_guard`; codex sessions are blocked because theirs does not. **Deviation from issue spec:** The issue prescribes `_has_fix_required_hooks(backend_name: str)` with `if backend_name != AGENT_BACKEND_CODEX`. This design violates `tests/arch/test_no_backend_name_bypass.py` which forbids backend-name comparisons in production code. Instead, the helper takes `applicable_guards: frozenset[str]` (a `BackendCapabilities` field) and cross-references fix-required hooks' guard script names against the set. This achieves identical runtime behavior via capability fields — architecturally correct and future-proof for additional backends. Consequently, `AGENT_BACKEND_CODEX` is not imported into `tools_execution.py` (no consumer), and `HookDef` is only imported in the test file. Closes #4001 ## Implementation Plan Plan file: `/home/talon/projects/autoskillit-runs/impl-20260611-091247-856621/.autoskillit/temp/make-plan/t5_p1_a4_wp1_fix_required_hook_dispatch_gate_plan_2026-06-11_091500.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 | |------|-------|-------|----------|--------|------------|----------|-------|-------------|------| | plan* | opus[1m] | 1 | 71 | 29.4k | 1.8M | 108.8k | 55 | 93.4k | 13m 6s | | verify* | sonnet | 1 | 948 | 13.4k | 363.5k | 63.2k | 24 | 42.3k | 7m 9s | | implement* | MiniMax-M3 | 1 | 3.9M | 16.7k | 0 | 0 | 96 | 0 | 12m 59s | | audit_impl* | sonnet | 1 | 46 | 10.3k | 148.0k | 44.6k | 17 | 56.3k | 4m 8s | | prepare_pr* | MiniMax-M3 | 1 | 237.0k | 2.6k | 0 | 0 | 16 | 0 | 55s | | compose_pr* | MiniMax-M3 | 1 | 251.5k | 1.5k | 0 | 0 | 15 | 0 | 45s | | **Total** | | | 4.4M | 73.9k | 2.4M | 108.8k | | 192.0k | 39m 4s | \* *Step used a non-Anthropic provider; caching behavior may differ.* ## Token Efficiency | Step | LoC Changed | cache_read/LoC | cache_write/LoC | output/LoC | |------|-------------|----------------|-----------------|------------| | plan | 0 | — | — | — | | verify | 0 | — | — | — | | implement | 206 | 0.0 | 0.0 | 81.0 | | audit_impl | 0 | — | — | — | | prepare_pr | 0 | — | — | — | | compose_pr | 0 | — | — | — | | **Total** | **206** | 11421.1 | 931.9 | 358.6 | ## Model Usage Breakdown | Model | steps | uncached | output | cache_read | cache_write | time | |-------|-------|----------|--------|------------|-------------|------| | opus[1m] | 1 | 71 | 29.4k | 1.8M | 93.4k | 13m 6s | | sonnet | 2 | 994 | 23.7k | 511.5k | 98.6k | 11m 18s | | MiniMax-M3 | 3 | 4.4M | 20.8k | 0 | 0 | 14m 39s |
1 parent e435dfe commit bfc5d2c

6 files changed

Lines changed: 245 additions & 7 deletions

File tree

src/autoskillit/server/tools/tools_execution.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
from autoskillit.core import current_order_id as _current_order_id
3838
from autoskillit.core import current_step_name as _current_step_name
3939
from autoskillit.core import resolve_skill_temp_dir as _resolve_skill_temp_dir
40+
from autoskillit.hook_registry import HOOK_REGISTRY
4041
from autoskillit.pipeline import canonical_step_name as _canonical_step_name
4142
from autoskillit.server import mcp
4243
from autoskillit.server._guards import (
@@ -86,6 +87,19 @@ def _is_backend_incompatible(skill_info: object, effective_backend: str) -> bool
8687
return bool(reqs and effective_backend not in reqs)
8788

8889

90+
def _get_fix_required_hook_matchers(applicable_guards: frozenset[str]) -> list[str]:
91+
"""Return matchers of fix-required hooks not enforced by the given guard set."""
92+
return [
93+
h.matcher
94+
for h in HOOK_REGISTRY
95+
if h.codex_status == "fix-required"
96+
and (
97+
not h.scripts
98+
or not frozenset(Path(s).stem for s in h.scripts).issubset(applicable_guards)
99+
)
100+
]
101+
102+
89103
def _check_backend_compat(
90104
skill_command: str,
91105
resolved_command: str,
@@ -133,6 +147,20 @@ def _check_backend_compat(
133147
skill_command=resolved_command,
134148
order_id=effective_order_id,
135149
).to_json()
150+
fix_required_matchers = _get_fix_required_hook_matchers(
151+
effective_backend_obj.capabilities.applicable_guards,
152+
)
153+
if fix_required_matchers:
154+
return SkillResult.crashed(
155+
exception=RuntimeError(
156+
f"Cannot dispatch skill {target_name!r} on backend "
157+
f"{effective_backend!r}: HOOK_REGISTRY contains fix-required "
158+
f"entries [{', '.join(fix_required_matchers)}] that cannot be "
159+
f"enforced by this backend."
160+
),
161+
skill_command=resolved_command,
162+
order_id=effective_order_id,
163+
).to_json()
136164
return None
137165

138166

tests/_test_filter.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -848,9 +848,10 @@ class ImportContext(enum.StrEnum):
848848
"hooks",
849849
"cli",
850850
"execution",
851-
# server/ narrowed to 2 files
851+
# server/ narrowed to 3 files
852852
"server/test_tools_kitchen_envelope.py",
853853
"server/test_lifespan.py",
854+
"server/test_run_skill_backend_compat.py",
854855
# infra/ narrowed to 7 files
855856
"infra/test_adr_runtime_guard_coverage.py",
856857
"infra/test_command_guard_completeness.py",

tests/arch/test_import_paths.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,14 +138,15 @@ def test_req_imp_001_no_cross_package_submodule_imports() -> None:
138138

139139
@pytest.mark.parametrize("path", TOOLS_FILES, ids=lambda p: p.name)
140140
def test_req_imp_003_tools_import_namespace(path: Path) -> None:
141-
"""tools_*.py may import from core, pipeline, config, and server."""
141+
"""tools_*.py may import from core, pipeline, config, hook_registry, and server."""
142142
allowed = frozenset(
143143
{
144144
"autoskillit.core",
145145
"autoskillit.pipeline",
146146
"autoskillit.server",
147147
"autoskillit.config",
148148
"autoskillit.fleet",
149+
"autoskillit.hook_registry",
149150
}
150151
)
151152
violations: list[str] = []

tests/arch/test_layer_enforcement.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -874,10 +874,11 @@ def test_no_cross_package_submodule_imports() -> None:
874874

875875
def test_server_tools_import_only_allowed_packages() -> None:
876876
"""REQ-ARCH-003: server/tools/tools_*.py may only import from autoskillit.core,
877-
autoskillit.pipeline, autoskillit.config, and intra-package autoskillit.server.*.
877+
autoskillit.pipeline, autoskillit.config, autoskillit.fleet, autoskillit.hook_registry,
878+
and intra-package autoskillit.server.*.
878879
TYPE_CHECKING exempt.
879880
"""
880-
ALLOWED = {"core", "pipeline", "server", "config", "fleet"}
881+
ALLOWED = {"core", "pipeline", "server", "config", "fleet", "hook_registry"}
881882
tools_files = [
882883
p for p in _SOURCE_FILES if p.parent.name == "tools" and p.stem.startswith("tools_")
883884
]

tests/arch/test_subpackage_isolation.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -971,11 +971,12 @@ def test_data_directories_are_not_python_packages() -> None:
971971
"error propagation from LoadRecipeResult",
972972
),
973973
"tools_execution.py": (
974-
1110,
974+
1130,
975975
"REQ-CNST-010-E8: execution tool handlers — run_cmd/run_python/run_skill are the "
976976
"three primary execution paths; fail-closed existence gate, empty-closure gate "
977-
"for fabricated skill name rejection, and _check_backend_compat fail-closed gate "
978-
"with resolver-absent fallback via extract_skill_name add defense-in-depth checks",
977+
"for fabricated skill name rejection, _check_backend_compat fail-closed gate "
978+
"with resolver-absent fallback via extract_skill_name, and fix-required hook "
979+
"dispatch gate add defense-in-depth checks",
979980
),
980981
"execution/backends/codex.py": (
981982
1045,

tests/server/test_run_skill_backend_compat.py

Lines changed: 206 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,3 +192,209 @@ async def test_provider_override_allows_skill_requiring_claude_code(
192192
assert mock_ssm.init_session.called, (
193193
"Expected init_session to be called after provider override rerouted codex→claude-code"
194194
)
195+
196+
197+
class TestHookFixRequiredDispatchGate:
198+
"""Dispatch-time gate for fix-required hook entries in HOOK_REGISTRY."""
199+
200+
def test_refuses_codex_dispatch_when_fix_required_hook_exists(
201+
self, monkeypatch: pytest.MonkeyPatch
202+
) -> None:
203+
import json
204+
from unittest.mock import MagicMock
205+
206+
from autoskillit.core import AGENT_BACKEND_CODEX, CodingAgentBackend
207+
from autoskillit.hook_registry import HookDef
208+
from autoskillit.server.tools import tools_execution
209+
from autoskillit.server.tools.tools_execution import _check_backend_compat
210+
211+
registry = [
212+
HookDef(
213+
matcher=r"Read|Write",
214+
scripts=["guards/test_guard.py"],
215+
codex_status="fix-required",
216+
),
217+
]
218+
monkeypatch.setattr(tools_execution, "HOOK_REGISTRY", registry)
219+
220+
backend = MagicMock(spec=CodingAgentBackend)
221+
backend.name = AGENT_BACKEND_CODEX
222+
backend.capabilities.applicable_guards = frozenset({"write_guard"})
223+
224+
skill_info = MagicMock()
225+
skill_info.backend_requirements = frozenset({AGENT_BACKEND_CODEX})
226+
227+
result = _check_backend_compat(
228+
skill_command="/autoskillit:test",
229+
resolved_command="/autoskillit:test",
230+
effective_order_id="ord-1",
231+
target_name="test",
232+
skill_info=skill_info,
233+
effective_backend_obj=backend,
234+
skill_resolver=MagicMock(),
235+
)
236+
assert result is not None
237+
parsed = json.loads(result)
238+
assert parsed["subtype"] == "crashed"
239+
error_text = parsed["result"]
240+
assert "Read|Write" in error_text
241+
242+
def test_allows_claude_code_dispatch_even_with_fix_required_hook(
243+
self, monkeypatch: pytest.MonkeyPatch
244+
) -> None:
245+
from unittest.mock import MagicMock
246+
247+
from autoskillit.core import AGENT_BACKEND_CLAUDE_CODE, CodingAgentBackend
248+
from autoskillit.hook_registry import HookDef
249+
from autoskillit.server.tools import tools_execution
250+
from autoskillit.server.tools.tools_execution import _check_backend_compat
251+
252+
registry = [
253+
HookDef(
254+
matcher=r"Read|Write",
255+
scripts=["guards/test_guard.py"],
256+
codex_status="fix-required",
257+
),
258+
]
259+
monkeypatch.setattr(tools_execution, "HOOK_REGISTRY", registry)
260+
261+
backend = MagicMock(spec=CodingAgentBackend)
262+
backend.name = AGENT_BACKEND_CLAUDE_CODE
263+
backend.capabilities.applicable_guards = frozenset({"test_guard"})
264+
265+
skill_info = MagicMock()
266+
skill_info.backend_requirements = frozenset({AGENT_BACKEND_CLAUDE_CODE})
267+
268+
result = _check_backend_compat(
269+
skill_command="/autoskillit:test",
270+
resolved_command="/autoskillit:test",
271+
effective_order_id="",
272+
target_name="test",
273+
skill_info=skill_info,
274+
effective_backend_obj=backend,
275+
skill_resolver=MagicMock(),
276+
)
277+
assert result is None
278+
279+
def test_refuses_codex_dispatch_when_fix_required_hook_has_empty_scripts(
280+
self, monkeypatch: pytest.MonkeyPatch
281+
) -> None:
282+
import json
283+
from unittest.mock import MagicMock
284+
285+
from autoskillit.core import AGENT_BACKEND_CODEX, CodingAgentBackend
286+
from autoskillit.hook_registry import HookDef
287+
from autoskillit.server.tools import tools_execution
288+
from autoskillit.server.tools.tools_execution import _check_backend_compat
289+
290+
# scripts=[] means the hook has no guard scripts to check coverage against;
291+
# the `not h.scripts` guard treats it as always-unenforced and blocks dispatch.
292+
registry = [
293+
HookDef(
294+
matcher=r"Read|Write",
295+
scripts=[],
296+
codex_status="fix-required",
297+
),
298+
]
299+
monkeypatch.setattr(tools_execution, "HOOK_REGISTRY", registry)
300+
301+
backend = MagicMock(spec=CodingAgentBackend)
302+
backend.name = AGENT_BACKEND_CODEX
303+
backend.capabilities.applicable_guards = frozenset({"any_guard"})
304+
305+
skill_info = MagicMock()
306+
skill_info.backend_requirements = frozenset({AGENT_BACKEND_CODEX})
307+
308+
result = _check_backend_compat(
309+
skill_command="/autoskillit:test",
310+
resolved_command="/autoskillit:test",
311+
effective_order_id="ord-empty",
312+
target_name="test",
313+
skill_info=skill_info,
314+
effective_backend_obj=backend,
315+
skill_resolver=MagicMock(),
316+
)
317+
assert result is not None
318+
parsed = json.loads(result)
319+
assert parsed["subtype"] == "crashed"
320+
assert "Read|Write" in parsed["result"]
321+
322+
def test_allows_codex_dispatch_when_no_fix_required_hooks(
323+
self, monkeypatch: pytest.MonkeyPatch
324+
) -> None:
325+
from unittest.mock import MagicMock
326+
327+
from autoskillit.core import AGENT_BACKEND_CODEX, CodingAgentBackend
328+
from autoskillit.hook_registry import HookDef
329+
from autoskillit.server.tools import tools_execution
330+
from autoskillit.server.tools.tools_execution import _check_backend_compat
331+
332+
registry = [
333+
HookDef(matcher=r"Read|Write", codex_status="works-as-is"),
334+
HookDef(matcher=r"Bash", codex_status="not-applicable"),
335+
]
336+
monkeypatch.setattr(tools_execution, "HOOK_REGISTRY", registry)
337+
338+
backend = MagicMock(spec=CodingAgentBackend)
339+
backend.name = AGENT_BACKEND_CODEX
340+
backend.capabilities.applicable_guards = frozenset({"write_guard"})
341+
342+
skill_info = MagicMock()
343+
skill_info.backend_requirements = frozenset({AGENT_BACKEND_CODEX})
344+
345+
result = _check_backend_compat(
346+
skill_command="/autoskillit:test",
347+
resolved_command="/autoskillit:test",
348+
effective_order_id="",
349+
target_name="test",
350+
skill_info=skill_info,
351+
effective_backend_obj=backend,
352+
skill_resolver=MagicMock(),
353+
)
354+
assert result is None
355+
356+
def test_crash_message_lists_affected_matchers(self, monkeypatch: pytest.MonkeyPatch) -> None:
357+
import json
358+
from unittest.mock import MagicMock
359+
360+
from autoskillit.core import AGENT_BACKEND_CODEX, CodingAgentBackend
361+
from autoskillit.hook_registry import HookDef
362+
from autoskillit.server.tools import tools_execution
363+
from autoskillit.server.tools.tools_execution import _check_backend_compat
364+
365+
registry = [
366+
HookDef(
367+
matcher=r"Read|Write",
368+
scripts=["guards/guard_a.py"],
369+
codex_status="fix-required",
370+
),
371+
HookDef(
372+
matcher=r"Bash|Grep",
373+
scripts=["guards/guard_b.py"],
374+
codex_status="fix-required",
375+
),
376+
]
377+
monkeypatch.setattr(tools_execution, "HOOK_REGISTRY", registry)
378+
379+
backend = MagicMock(spec=CodingAgentBackend)
380+
backend.name = AGENT_BACKEND_CODEX
381+
backend.capabilities.applicable_guards = frozenset({"write_guard"})
382+
383+
skill_info = MagicMock()
384+
skill_info.backend_requirements = frozenset({AGENT_BACKEND_CODEX})
385+
386+
result = _check_backend_compat(
387+
skill_command="/autoskillit:test",
388+
resolved_command="/autoskillit:test",
389+
effective_order_id="ord-2",
390+
target_name="test",
391+
skill_info=skill_info,
392+
effective_backend_obj=backend,
393+
skill_resolver=MagicMock(),
394+
)
395+
assert result is not None
396+
parsed = json.loads(result)
397+
assert parsed["subtype"] == "crashed"
398+
error_text = parsed["result"]
399+
assert "Read|Write" in error_text
400+
assert "Bash|Grep" in error_text

0 commit comments

Comments
 (0)