Rectify: Feature Gate Silently Blocks Explicitly Requested Skills — Policy Layering Violation by Trecek · Pull Request #1649 · TalonT-Org/AutoSkillit

Trecek · 2026-05-03T04:15:53Z

Summary

The init_session method in session_skills.py has two independent, uncoordinated skill-gating mechanisms. When the orchestrator explicitly requests skills via allow_only (computed from compute_skill_closure), the feature gate in _is_skill_disabled can silently block every requested skill, producing an empty session directory with no error. This violates the Policy Layering Principle: explicit higher-priority requests must override implicit background policies. The pack system (PACK_REGISTRY + _resolve_effective_disabled) correctly implements this principle with a subtraction path (packs_enabled ∪ recipe_packs), but the feature gate system (FEATURE_REGISTRY + _is_skill_disabled) has no equivalent override channel.

The fix unifies the gating model by treating allow_only as a policy override for feature gates, adds a post-condition invariant that catches any future silent-empty-result, and adds cross-axis tests that would have caught this bug and will catch any future variant.

Closes #1641

Implementation Plan

Plan file: /home/talon/projects/autoskillit-runs/remediation-20260502-192857-360284/.autoskillit/temp/rectify/rectify_feature_gate_allow_only_policy_layering_2026-05-02_192857.md

🤖 Generated with Claude Code via AutoSkillit

Token Usage Summary

Step	uncached	output	cache_read	cache_write	count	time
rectify	65	12.7k	1.1M	65.1k	1	7m 10s
dry_walkthrough	51	10.3k	1.5M	64.5k	1	5m 9s
implement	252	30.3k	2.0M	76.7k	1	8m 57s
prepare_pr	60	4.9k	235.1k	31.8k	1	1m 34s
compose_pr	67	2.9k	212.6k	35.7k	1	1m 6s
review_pr	885	65.6k	1.4M	333.2k	2	17m 28s
resolve_review	658	35.3k	4.1M	329.6k	2	17m 19s
Total	2.0k	161.9k	10.5M	936.6k		58m 46s

Token Efficiency

Step	LoC Changed	cache_read/LoC	cache_write/LoC	output/LoC
rectify	0	—	—	—
dry_walkthrough	0	—	—	—
implement	250	7891.2	306.7	121.1
assess	19	68157.3	3654.1	1021.2
prepare_pr	0	—	—	—
compose_pr	0	—	—	—
Total	269	23443.5	1276.0	299.0

Tests 1A–1E and cross-axis matrix verify that allow_only overrides feature-gate defaults, explicit subsets.disabled still wins, and the post-condition invariant fires on zero achievable skills written. Rename test_allow_only_intersects_effective_disabled_disabled_wins to test_allow_only_does_not_override_explicit_subsets_disabled. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…1641) Add allow_only parameter to _is_skill_disabled and _should_inject_skill. When allow_only is set and the skill is explicitly requested, the FEATURE_REGISTRY suppression branch is bypassed while explicit subsets.disabled entries continue to block. Add post-condition invariant in init_session that raises RuntimeError when allow_only is non-empty but zero achievable skills are written, catching any future silent-empty result. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Features whose tool_tags overlap skill_categories (e.g. fleet, where both are {"fleet"}) inject their tag into effective_disabled via disabled_feature_tags. The disabled list check in _is_skill_disabled then blocks the skill before the FEATURE_REGISTRY branch (which already had the allow_only bypass) is reached. Extend the bypass to the disabled list check: when allow_only is active and the blocking tag came from a feature's tool_tags, skip it — same policy as the FEATURE_REGISTRY skill_categories bypass below. Fixes test_allow_only_overrides_all_feature_registry_entries[fleet-fleet].

Trecek

AutoSkillit PR Review — Verdict: changes_requested

Trecek · 2026-05-03T04:57:08Z

            gated = (not cook_session) and (skill_info.name in tier2_skills)
            content = self._provider.get_skill_content(skill_info.name, gated=gated)
            atomic_write(skill_dir / "SKILL.md", content)
+        if allow_only is not None and allow_only:


[warning] defense: The achievable-skills guard raises RuntimeError only when the intersection written & achievable is completely empty (zero achievable skills written). If allow_only contains multiple skills and at least one is written, skills silently omitted due to gating conflicts are not diagnosed. Consider whether all achievable skills must be written or only at least one.

Valid observation — flagged for design decision. The current at-least-one semantics were introduced specifically to diagnose the zero-skills scenario from issue #1641. Whether partial blocking (some achievable skills gated out while others pass) should also raise is a genuine design question: changing to all-must-be-written could produce false positives for users who intentionally gate a subset of allow_only skills via packs or categories. No code change made; human review recommended before tightening this invariant.

[AUDIT] — acknowledged, no action taken

Trecek · 2026-05-03T04:57:08Z

+        mgr = DefaultSessionSkillManager(provider, ephemeral_root=root)
+
+        features = {"planner": True} if feature_enabled else {}
+        config = make_test_config(


[warning] cohesion: Indentation inconsistency in make_test_config call: features=features, at L361 is at 8-space indent while experimental_enabled=False, at L362 is at 12-space indent. ruff format will auto-fix this on pre-commit, but it indicates pre-commit hooks may not have been run before this PR was opened.

Acknowledged — minor suggestion noted.

Trecek

AutoSkillit review: changes_requested. 1 critical finding must be resolved before merge.

Blocking (critical):

src/autoskillit/workspace/session_skills.py:174 [defense] _feature_tool_tags collects tool_tags from ALL features (including enabled ones), violating the docstring guarantee that explicit user-configured disabled entries are never bypassed. Restrict to tool_tags from disabled features only.

Warnings (non-blocking but flagged):

src/autoskillit/workspace/session_skills.py:505 [defense] Achievable-skills guard raises only when zero skills are written — partial gating failures (some allow_only skills silently omitted) are not detected. (Design decision required)
tests/workspace/test_session_skills_allow_only_and_closure.py:219 [tests] test_allow_only_nonempty_but_zero_skills_raises calls init_session without config — confirm the default is safe and the RuntimeError path is reached, not a TypeError/AttributeError.
tests/workspace/test_session_skills_allow_only_and_closure.py:360 [cohesion] Indentation inconsistency in make_test_config call (keyword args at mixed indent levels). Indicates pre-commit hooks may not have been run.

Info:
See inline comments for 4 additional low-priority observations.

_feature_tool_tags was collecting tool_tags from ALL features in FEATURE_REGISTRY (both enabled and disabled). The bypass at the disabled-list check was therefore triggered even for tags from currently-enabled features, allowing a user-explicitly-disabled tag that shares a name with an enabled feature's tool_tag to be silently skipped — violating the docstring guarantee that explicit disabled entries are never bypassed. Filter by `not is_feature_enabled(...)` to mirror the disabled_tool_tags computation in init_session, ensuring the bypass only applies to tags actually injected by the feature gate (disabled features only). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Trecek

AutoSkillit PR Review — Verdict: approved_with_comments

Trecek · 2026-05-03T05:42:04Z

+
+
+_CROSS_AXIS_PARAMS = [
+    # (allow_only, feature_enabled, subsets_disabled, cook_session, expected_target_written)


[info] slop: Inline tuple-field legend comment duplicates the parameter names visible in each pytest.param call; adds no information the code does not already convey.

Acknowledged — minor suggestion noted.

Trecek · 2026-05-03T05:42:04Z

+        [],
+        False,
+        True,
+        id="ao-set_feat-off_disabled-no_cook-no",  # THE BUG CASE


[info] slop: Trailing comment # THE BUG CASE is a development artifact; the test id ao-set_feat-off_disabled-no_cook-no already encodes the scenario. Remove the comment.

Acknowledged — minor suggestion noted.

Trecek

AutoSkillit review: warning-only findings detected. See inline comments — no blocking changes required.

… load Adds module-level assertion so test_allow_only_overrides_all_feature_registry_entries fails loudly instead of silently skipping when FEATURE_REGISTRY has no skill_categories. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Pre-computes the allow_only membership check once before the union-model for-loop, mirroring the _feature_tool_tags pattern established at L183. Eliminates per-iteration re-evaluation of the same boolean expression. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…olicy Layering Violation (#1649) ## Summary The `init_session` method in `session_skills.py` has two independent, uncoordinated skill-gating mechanisms. When the orchestrator explicitly requests skills via `allow_only` (computed from `compute_skill_closure`), the feature gate in `_is_skill_disabled` can silently block every requested skill, producing an empty session directory with no error. This violates the **Policy Layering Principle**: explicit higher-priority requests must override implicit background policies. The pack system (`PACK_REGISTRY` + `_resolve_effective_disabled`) correctly implements this principle with a subtraction path (`packs_enabled ∪ recipe_packs`), but the feature gate system (`FEATURE_REGISTRY` + `_is_skill_disabled`) has no equivalent override channel. The fix unifies the gating model by treating `allow_only` as a policy override for feature gates, adds a post-condition invariant that catches any future silent-empty-result, and adds cross-axis tests that would have caught this bug and will catch any future variant. Closes #1641 ## Implementation Plan Plan file: `/home/talon/projects/autoskillit-runs/remediation-20260502-192857-360284/.autoskillit/temp/rectify/rectify_feature_gate_allow_only_policy_layering_2026-05-02_192857.md` 🤖 Generated with [Claude Code](https://claude.com/claude-code) via AutoSkillit  ## Token Usage Summary | Step | uncached | output | cache_read | cache_write | count | time | |------|----------|--------|------------|-------------|-------|------| | rectify | 65 | 12.7k | 1.1M | 65.1k | 1 | 7m 10s | | dry_walkthrough | 51 | 10.3k | 1.5M | 64.5k | 1 | 5m 9s | | implement | 252 | 30.3k | 2.0M | 76.7k | 1 | 8m 57s | | assess | 190 | 19.4k | 1.3M | 69.4k | 1 | 9m 29s | | prepare_pr | 60 | 4.9k | 235.1k | 31.8k | 1 | 1m 34s | | compose_pr | 67 | 2.9k | 212.6k | 35.7k | 1 | 1m 6s | | **Total** | 685 | 80.4k | 6.3M | 343.3k | | 33m 28s | ## Token Efficiency | Step | LoC Changed | cache_read/LoC | cache_write/LoC | output/LoC | |------|-------------|----------------|-----------------|------------| | rectify | 0 | — | — | — | | dry_walkthrough | 0 | — | — | — | | implement | 250 | 7891.2 | 306.7 | 121.1 | | assess | 19 | 68157.3 | 3654.1 | 1021.2 | | prepare_pr | 0 | — | — | — | | compose_pr | 0 | — | — | — | | **Total** | **269** | 23443.5 | 1276.0 | 299.0 | --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>

Trecek and others added 3 commits May 2, 2026 19:58

Trecek commented May 3, 2026

View reviewed changes

Trecek and others added 2 commits May 2, 2026 22:52

Trecek added this pull request to the merge queue May 3, 2026

Merged via the queue into develop with commit a7dfb4e May 3, 2026
2 checks passed

Trecek deleted the feature-gate-silently-blocks-all-planner-skills-in-non-dev-i/1641 branch May 3, 2026 06:13

Trecek mentioned this pull request May 25, 2026

Tier 2 skills silently omitted from ephemeral session directories — 86+ recipe skills broken #2976

Open



		_CROSS_AXIS_PARAMS = [
		# (allow_only, feature_enabled, subsets_disabled, cook_session, expected_target_written)

Conversation

Trecek commented May 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Implementation Plan

Token Usage Summary

Token Efficiency

Uh oh!

Trecek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Trecek left a comment

Choose a reason for hiding this comment

Uh oh!

Trecek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Trecek left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Trecek commented May 3, 2026 •

edited

Loading