Skip to content

Commit 599ab5d

Browse files
committed
T5-P1-A4-WP1: surface crashed result at dispatch for fix-required hooks
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. The helper takes applicable_guards (a BackendCapabilities field) instead of a backend name to satisfy test_no_backend_name_bypass.py. Arch test rule updates: extend the server/tools allowed-imports set and cascade map entry to permit hook_registry imports from tools_execution.
1 parent 9bb70b0 commit 599ab5d

6 files changed

Lines changed: 199 additions & 7 deletions

File tree

src/autoskillit/server/tools/tools_execution.py

Lines changed: 25 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,16 @@ 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 _has_fix_required_hooks(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 not frozenset(Path(s).stem for s in h.scripts).issubset(applicable_guards)
97+
]
98+
99+
89100
def _check_backend_compat(
90101
skill_command: str,
91102
resolved_command: str,
@@ -133,6 +144,20 @@ def _check_backend_compat(
133144
skill_command=resolved_command,
134145
order_id=effective_order_id,
135146
).to_json()
147+
fix_required_matchers = _has_fix_required_hooks(
148+
effective_backend_obj.capabilities.applicable_guards,
149+
)
150+
if fix_required_matchers:
151+
return SkillResult.crashed(
152+
exception=RuntimeError(
153+
f"Cannot dispatch skill {target_name!r} on backend "
154+
f"{effective_backend!r}: HOOK_REGISTRY contains fix-required "
155+
f"entries [{', '.join(fix_required_matchers)}] that cannot be "
156+
f"enforced by this backend."
157+
),
158+
skill_command=resolved_command,
159+
order_id=effective_order_id,
160+
).to_json()
136161
return None
137162

138163

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: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,3 +192,166 @@ 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.get("result") or ""
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_allows_codex_dispatch_when_no_fix_required_hooks(
280+
self, monkeypatch: pytest.MonkeyPatch
281+
) -> None:
282+
from unittest.mock import MagicMock
283+
284+
from autoskillit.core import AGENT_BACKEND_CODEX, CodingAgentBackend
285+
from autoskillit.hook_registry import HookDef
286+
from autoskillit.server.tools import tools_execution
287+
from autoskillit.server.tools.tools_execution import _check_backend_compat
288+
289+
registry = [
290+
HookDef(matcher=r"Read|Write", codex_status="works-as-is"),
291+
HookDef(matcher=r"Bash", codex_status="not-applicable"),
292+
]
293+
monkeypatch.setattr(tools_execution, "HOOK_REGISTRY", registry)
294+
295+
backend = MagicMock(spec=CodingAgentBackend)
296+
backend.name = AGENT_BACKEND_CODEX
297+
backend.capabilities.applicable_guards = frozenset({"write_guard"})
298+
299+
skill_info = MagicMock()
300+
skill_info.backend_requirements = frozenset({AGENT_BACKEND_CODEX})
301+
302+
result = _check_backend_compat(
303+
skill_command="/autoskillit:test",
304+
resolved_command="/autoskillit:test",
305+
effective_order_id="",
306+
target_name="test",
307+
skill_info=skill_info,
308+
effective_backend_obj=backend,
309+
skill_resolver=MagicMock(),
310+
)
311+
assert result is None
312+
313+
def test_crash_message_lists_affected_matchers(self, monkeypatch: pytest.MonkeyPatch) -> None:
314+
import json
315+
from unittest.mock import MagicMock
316+
317+
from autoskillit.core import AGENT_BACKEND_CODEX, CodingAgentBackend
318+
from autoskillit.hook_registry import HookDef
319+
from autoskillit.server.tools import tools_execution
320+
from autoskillit.server.tools.tools_execution import _check_backend_compat
321+
322+
registry = [
323+
HookDef(
324+
matcher=r"Read|Write",
325+
scripts=["guards/guard_a.py"],
326+
codex_status="fix-required",
327+
),
328+
HookDef(
329+
matcher=r"Bash|Grep",
330+
scripts=["guards/guard_b.py"],
331+
codex_status="fix-required",
332+
),
333+
]
334+
monkeypatch.setattr(tools_execution, "HOOK_REGISTRY", registry)
335+
336+
backend = MagicMock(spec=CodingAgentBackend)
337+
backend.name = AGENT_BACKEND_CODEX
338+
backend.capabilities.applicable_guards = frozenset({"write_guard"})
339+
340+
skill_info = MagicMock()
341+
skill_info.backend_requirements = frozenset({AGENT_BACKEND_CODEX})
342+
343+
result = _check_backend_compat(
344+
skill_command="/autoskillit:test",
345+
resolved_command="/autoskillit:test",
346+
effective_order_id="ord-2",
347+
target_name="test",
348+
skill_info=skill_info,
349+
effective_backend_obj=backend,
350+
skill_resolver=MagicMock(),
351+
)
352+
assert result is not None
353+
parsed = json.loads(result)
354+
assert parsed["subtype"] == "crashed"
355+
error_text = parsed.get("result") or ""
356+
assert "Read|Write" in error_text
357+
assert "Bash|Grep" in error_text

0 commit comments

Comments
 (0)