Skip to content

Implementation Plan: Surface a Structured crashed Result at Dispatch for Fix-Required Hooks#4070

Merged
Trecek merged 4 commits into
developfrom
t5-p1-a4-wp1-surface-a-structured-crashed-result-at-dispatch/4001
Jun 11, 2026
Merged

Implementation Plan: Surface a Structured crashed Result at Dispatch for Fix-Required Hooks#4070
Trecek merged 4 commits into
developfrom
t5-p1-a4-wp1-surface-a-structured-crashed-result-at-dispatch/4001

Conversation

@Trecek

@Trecek Trecek commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

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 via AutoSkillit

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
review_pr* sonnet 1 681 40.4k 1.4M 101.1k 54 112.9k 10m 30s
resolve_review* sonnet 1 229 12.9k 1.8M 80.0k 67 80.8k 7m 2s
Total 4.4M 127.2k 5.5M 108.8k 385.7k 56m 38s

* 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
review_pr 0
resolve_review 47 37473.1 1719.9 274.9
Total 253 21667.0 1524.4 502.7

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 4 1.9k 77.0k 3.6M 292.3k 28m 51s
MiniMax-M3 3 4.4M 20.8k 0 0 14m 39s

Trecek added 4 commits June 11, 2026 13:23
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.
…_matchers; guard empty scripts

_has_* prefix conventionally signals a bool return in this file (_has_active_locks → bool).
Rename to _get_fix_required_hook_matchers to match collection-returning helper convention.

Also add `not h.scripts` guard before the issubset check: an empty frozenset is always a
subset of any set, so a fix-required HookDef with scripts=[] would silently pass the
coverage check and be omitted from matchers, allowing dispatch to proceed unchecked.
…ult') or ''

Weak .get() fallback to empty string masks missing 'result' keys with an obscure
string-contains failure. Direct dict access raises KeyError immediately, surfacing
the real problem rather than a misleading assertion failure.
The `not h.scripts` guard added in _get_fix_required_hook_matchers treats a
fix-required HookDef with scripts=[] as always-unenforced (dispatch blocked).
No test covered this guard branch. Add test to verify the behavior explicitly.
@Trecek Trecek force-pushed the t5-p1-a4-wp1-surface-a-structured-crashed-result-at-dispatch/4001 branch from 599ab5d to 86a965e Compare June 11, 2026 20:23
@Trecek Trecek added this pull request to the merge queue Jun 11, 2026
Merged via the queue into develop with commit bfc5d2c Jun 11, 2026
3 checks passed
@Trecek Trecek deleted the t5-p1-a4-wp1-surface-a-structured-crashed-result-at-dispatch/4001 branch June 11, 2026 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant