diff --git a/src/autoskillit/agents/AGENTS.md b/src/autoskillit/agents/AGENTS.md index b15e6d11c0..280d6a965a 100644 --- a/src/autoskillit/agents/AGENTS.md +++ b/src/autoskillit/agents/AGENTS.md @@ -39,7 +39,7 @@ Agent(subagent_type="autoskillit:{agent-name}", prompt="...") Claude Code automatically applies tool restrictions, model, maxTurns, and the markdown body as the system prompt from the agent definition frontmatter. -**Used by:** make-plan Steps 6-9, rectify Steps 5-7 +**Used by:** make-plan Steps 6-9, rectify Steps 4-6 ### MCP Resource path (available for programmatic access) @@ -52,7 +52,7 @@ agent definitions are readable via `ReadMcpResourceTool` at `agent://{pack}/{nam | Pack | Tag | Agents | Used By | |------|-----|--------|---------| -| `plan-review` | `plan-review` | 3 adversarial reviewers | make-plan Steps 6-9, rectify Steps 5-7 | +| `plan-review` | `plan-review` | 3 adversarial reviewers | make-plan Steps 6-9, rectify Steps 4-6 | | _(none)_ | _(none)_ | `wp-elaborator` | planner-elaborate-wps (subagent_type-only) | | _(none)_ | _(none)_ | `pipeline-health-scanner` | analyze-pipeline-health (subagent_type-only) | | _(none)_ | _(none)_ | `audit-impl-slice-auditor` | audit-impl Step 3 (subagent_type-only) | @@ -72,4 +72,4 @@ agent definitions are readable via `ReadMcpResourceTool` at `agent://{pack}/{nam `agent://{pack}/{name}` resource path and can be skipped for packless agents. Current packless agents: `wp-elaborator`, `pipeline-health-scanner`, `audit-impl-slice-auditor`, `pr-review-auditor-baseline`, `pr-review-auditor-v1-precision`, `pr-review-auditor-v2-contrastive`, -`pr-review-auditor-v3-simulation`, `audit-impl-deviation-evaluator`. \ No newline at end of file +`pr-review-auditor-v3-simulation`, `audit-impl-deviation-evaluator`. diff --git a/src/autoskillit/core/types/_type_constants.py b/src/autoskillit/core/types/_type_constants.py index 5fd34f0aae..90982d9acb 100644 --- a/src/autoskillit/core/types/_type_constants.py +++ b/src/autoskillit/core/types/_type_constants.py @@ -364,7 +364,7 @@ class SkillFamilyDef(NamedTuple): # Pipeline skills that must declare specific activate_deps. Contract test # test_required_activate_deps_present enforces this invariant at CI time. SKILL_ACTIVATE_DEPS_REQUIRED: dict[str, frozenset[str]] = { - "make-plan": frozenset({"arch-lens", "write-recipe"}), + "make-plan": frozenset({"write-recipe"}), "implement-worktree": frozenset({"write-recipe"}), "implement-worktree-no-merge": frozenset({"write-recipe"}), } diff --git a/src/autoskillit/skills_extended/make-plan/SKILL.md b/src/autoskillit/skills_extended/make-plan/SKILL.md index 9ed27cae9c..6a0af4c60a 100644 --- a/src/autoskillit/skills_extended/make-plan/SKILL.md +++ b/src/autoskillit/skills_extended/make-plan/SKILL.md @@ -1,7 +1,7 @@ --- name: make-plan -uses_capabilities: [agent_model, agent_subagent, cross_skill_ref] -activate_deps: [arch-lens, write-recipe] +uses_capabilities: [agent_model, agent_subagent] +activate_deps: [write-recipe] description: Planning executor. ALWAYS invoke this skill when instructed to create, devise, or write an implementation plan. Do not explore the codebase or draft a plan directly — use this skill first to load the planning workflow. hooks: PreToolUse: @@ -87,63 +87,9 @@ tool **before** beginning any analysis. Use the returned `content` field as the **DO NOT evaluate based on:** implementation effort, risk, number of changes, test breakage, or ease of rollback. These are not engineering criteria. -5. **Visualize with Architecture Lens** - After finalizing the plan, determine which architecture lens best illustrates the proposed changes, then create a mermaid diagram. - -**5a. Select the lens based on what the plan primarily affects:** - -| If the plan primarily involves... | Use Lens | -|-----------------------------------|----------| -| Adding/modifying containers, services, or integrations | C4 Container | -| Changing workflow logic, state machines, or decision flow | Process Flow | -| Altering data storage, transformations, or information flow | Data Lineage | -| Restructuring modules, changing dependencies, or layering | Module Dependency | -| Adding/modifying parallel execution or thread handling | Concurrency | -| Changing error handling, retry logic, or recovery paths | Error/Resilience | -| Modifying repository patterns or data access | Repository Access | -| Changing CLI commands, config, or monitoring | Operational | -| Adding/modifying validation, trust boundaries, or isolation | Security | -| Changing build tools, test framework, or quality gates | Development | -| Affecting multiple user journeys or cross-component flows | Scenarios | -| Modifying state contracts, field lifecycles, or resume logic | State Lifecycle | -| Changing deployment topology or infrastructure | Deployment | - -**5b. Write your lens selection rationale to a file using the Write tool:** - -- **Path:** `{{AUTOSKILLIT_TEMP}}/make-plan/arch_lens_selection_{YYYY-MM-DD_HHMMSS}.md` -- **Content:** Which lens was selected and why (1-2 sentences of rationale). - -**5c. MANDATORY: LOAD the appropriate arch-lens skill using the Skill tool:** - -| Lens | Skill to LOAD | -|------|---------------| -| C4 Container | `/autoskillit:arch-lens-c4-container` | -| Process Flow | `/autoskillit:arch-lens-process-flow` | -| Data Lineage | `/autoskillit:arch-lens-data-lineage` | -| Module Dependency | `/autoskillit:arch-lens-module-dependency` | -| Concurrency | `/autoskillit:arch-lens-concurrency` | -| Error/Resilience | `/autoskillit:arch-lens-error-resilience` | -| Repository Access | `/autoskillit:arch-lens-repository-access` | -| Operational | `/autoskillit:arch-lens-operational` | -| Security | `/autoskillit:arch-lens-security` | -| Development | `/autoskillit:arch-lens-development` | -| Scenarios | `/autoskillit:arch-lens-scenarios` | -| State Lifecycle | `/autoskillit:arch-lens-state-lifecycle` | -| Deployment | `/autoskillit:arch-lens-deployment` | - -If the Skill tool cannot be used (disable-model-invocation) or refuses this invocation, proceed without the architectural diagram. - -**5d. Create the diagram following the loaded skill's instructions:** -- Focus on the PROPOSED changes (use `newComponent` class for new elements) -- Show how new components integrate with existing architecture -- Use `●` prefix for modified existing components -- Use `★` prefix for new components - -Include the diagram in the plan document under a "## Proposed Architecture" section. -More than one lens diagram is okay if it is complex plan (don't do more than 3, and make sure to load each appropriate skill). - -**5e. Complexity-Gated Adversarial Review Decision** - -Before spawning adversarial review agents, determine the review level. +5. **Complexity-Gated Adversarial Review Decision** + +Draft the complete plan from the selected approach using the Output format before calculating complexity or spawning adversarial review agents. Then determine the review level. **Reading the override:** Check if ARGUMENTS contains a line matching `adversarial_review_level=`. Valid values: `auto`, `full`, `none`. @@ -224,7 +170,7 @@ The `summary` field is **required** when `message` is a string — omitting it c The `summary` field is **required** when `message` is a string — omitting it causes `InputValidationError`. If the resumed agent still returns truncated, proceed without its findings rather than retrying further. 9. **Plan Revision** - Read all available adversarial reports (0, 1, 2, or 3 - depending on the complexity gate decision in Step 5e). For each valid finding (where the agent identified a real gap, not a hypothetical): + depending on the complexity gate decision in Step 5). For each valid finding (where the agent identified a real gap, not a hypothetical): - Add missing consumers to implementation steps - Add missing entity categories to search/update operations - Replace invalid assumptions with verified facts @@ -271,18 +217,6 @@ handles correctly. --- -## Skill Loading Checklist - -Before writing the final plan, verify: - -- [ ] Determined which architecture lens best fits the proposed changes -- [ ] LOADED the corresponding `/autoskillit:arch-lens-*` skill using the Skill tool -- [ ] The arch-lens skill LOADED the `/autoskillit:mermaid` skill for styling -- [ ] Diagram uses ONLY the classDef styles from the mermaid skill (no invented colors) -- [ ] Diagram includes a color legend table -- [ ] Every new component, class, or function is wired into the call chain — nothing is created but left unconnected -- [ ] Adversarial review pass completed per complexity gate (Steps 5e-9) — skipped agents documented in plan - ## Critical Constraints **NEVER use EnterPlanMode.** This skill IS the planning process. Execute the planning steps directly — explore with subagents, design the approach, write the plan file to `{{AUTOSKILLIT_TEMP}}/make-plan/` (relative to the current working directory). Do not enter plan mode, do not call ExitPlanMode. Just do the work and deliver the plan. @@ -324,8 +258,9 @@ Before writing the final plan, verify: - Be willing to recommend significant refactoring if that's the right answer - Issue all Task calls in a single message to maximize parallelism - The plan must cover every remediation item enumerated in the source issue; if an item cannot be delivered, stop and surface it — do not descope it in the plan +- Every new component, class, or function is wired into the call chain — nothing is created but left unconnected -**Requirement Echo Rule:** Every behavioral requirement stated in `## Summary`, `## Proposed Architecture`, or `## Design Decisions` prose MUST be echoed as an explicit `## Implementation Steps` directive. After drafting the plan: +**Requirement Echo Rule:** Every behavioral requirement stated in `## Summary` or `## Design Decisions` prose MUST be echoed as an explicit `## Implementation Steps` directive. After drafting the plan: 1. Enumerate every behavioral constraint in the prose sections above. 2. For each constraint, verify it maps to at least one explicit step directive. @@ -422,11 +357,6 @@ verdict = false_positive ## Summary {Brief overview of what will be implemented} -## Proposed Architecture -{Mermaid diagram showing the proposed changes using the selected lens} - -**Lens Used:** {lens name} - {why this lens was chosen} - ## Tests {Tests to write first — should fail now, pass after implementation} @@ -439,7 +369,7 @@ verdict = false_positive ## Requirements Map | Requirement (from prose) | Implementation Step | |---|---| -| {behavioral constraint from Summary/Architecture/Design Decisions} | Step {N.M}: {step description} | +| {behavioral constraint from Summary/Design Decisions} | Step {N.M}: {step description} | ``` **Plan structure (multi-part — use for EACH part file):** @@ -451,11 +381,6 @@ verdict = false_positive ## Summary {What THIS part covers. Explicitly note what is deferred: "Part B will cover X (separate task). Part C will cover Y (separate task)."} -## Proposed Architecture -{Mermaid diagram showing the proposed changes using the selected lens} - -**Lens Used:** {lens name} - {why this lens was chosen} - ## Tests {Tests for THIS part only — should fail now, pass after THIS part's implementation} @@ -468,5 +393,5 @@ verdict = false_positive ## Requirements Map | Requirement (from prose) | Implementation Step | |---|---| -| {behavioral constraint from THIS part's Summary/Architecture/Design Decisions} | Step {N.M}: {step description} | -``` \ No newline at end of file +| {behavioral constraint from THIS part's Summary/Design Decisions} | Step {N.M}: {step description} | +``` diff --git a/src/autoskillit/skills_extended/rectify/SKILL.md b/src/autoskillit/skills_extended/rectify/SKILL.md index 1377f5e778..cbacde378e 100644 --- a/src/autoskillit/skills_extended/rectify/SKILL.md +++ b/src/autoskillit/skills_extended/rectify/SKILL.md @@ -1,7 +1,6 @@ --- name: rectify -uses_capabilities: [agent_model, agent_subagent, cross_skill_ref] -activate_deps: [arch-lens] +uses_capabilities: [agent_model, agent_subagent] description: Deep investigation of test gaps and architectural weaknesses following an investigation, then devise a plan for architectural immunity rather than direct fixes. Use when user says "rectify", "rectify this", or wants to address root architectural causes after an investigation. hooks: PreToolUse: @@ -77,6 +76,7 @@ Do not change any code. This token is MANDATORY — the pipeline cannot capture the output without it. - The solution must solve more than just the immediate issue - The plan must cover every remediation item enumerated in the source issue; if an item cannot be delivered, stop and surface it — do not descope it in the plan +- Every new component, class, or function is wired into the call chain — nothing is created but left unconnected ## Context Limit Behavior @@ -143,62 +143,9 @@ Design an approach that provides **immunity** rather than a fix: **Test-Driven Approach:** The plan must lead with tests. Before any implementation step, define a test that reproduces the issue or captures the gap. Each subsequent implementation step should make that test pass. This applies to the initial fix and to any broader architectural changes—write the failing test first, then the code that makes it green. -### Step 4: Visualize with Architecture Lens - -After finalizing the plan, determine which architecture lens best illustrates the proposed changes, then create a mermaid diagram. - -**4a. Select the lens based on what the plan primarily affects:** - -| If the plan primarily involves... | Use Lens | -|-----------------------------------|----------| -| Adding/modifying containers, services, or integrations | C4 Container | -| Changing workflow logic, state machines, or decision flow | Process Flow | -| Altering data storage, transformations, or information flow | Data Lineage | -| Restructuring modules, changing dependencies, or layering | Module Dependency | -| Adding/modifying parallel execution or thread handling | Concurrency | -| Changing error handling, retry logic, or recovery paths | Error/Resilience | -| Modifying repository patterns or data access | Repository Access | -| Changing CLI commands, config, or monitoring | Operational | -| Adding/modifying validation, trust boundaries, or isolation | Security | -| Changing build tools, test framework, or quality gates | Development | -| Affecting multiple user journeys or cross-component flows | Scenarios | -| Modifying state contracts, field lifecycles, or resume logic | State Lifecycle | -| Changing deployment topology or infrastructure | Deployment | - -**4b. Write your lens selection rationale to a file using the Write tool:** - -- **Path:** `{{AUTOSKILLIT_TEMP}}/rectify/arch_lens_selection_{YYYY-MM-DD_HHMMSS}.md` -- **Content:** Which lens was selected and why (1-2 sentences of rationale). - -**4c. MANDATORY: LOAD the appropriate arch-lens skill using the Skill tool:** - -| Lens | Skill to LOAD | -|------|---------------| -| C4 Container | `/autoskillit:arch-lens-c4-container` | -| Process Flow | `/autoskillit:arch-lens-process-flow` | -| Data Lineage | `/autoskillit:arch-lens-data-lineage` | -| Module Dependency | `/autoskillit:arch-lens-module-dependency` | -| Concurrency | `/autoskillit:arch-lens-concurrency` | -| Error/Resilience | `/autoskillit:arch-lens-error-resilience` | -| Repository Access | `/autoskillit:arch-lens-repository-access` | -| Operational | `/autoskillit:arch-lens-operational` | -| Security | `/autoskillit:arch-lens-security` | -| Development | `/autoskillit:arch-lens-development` | -| Scenarios | `/autoskillit:arch-lens-scenarios` | -| State Lifecycle | `/autoskillit:arch-lens-state-lifecycle` | -| Deployment | `/autoskillit:arch-lens-deployment` | - -If the Skill tool cannot be used (disable-model-invocation) or refuses this invocation, omit the diagram and proceed without the architectural diagram. - -**4d. Create the diagram following the loaded skill's instructions:** -- Focus on the PROPOSED changes (use `newComponent` class for new elements) -- Show how new components integrate with existing architecture -- Use `●` prefix for modified existing components -- Use `★` prefix for new components - -Include the diagram in the plan document under a "## Proposed Architecture" section. - -### Step 5: Foundation Audit +Draft the complete immunity plan from Step 3's selected approach using the Output template before spawning adversarial reviewers. + +### Step 4: Foundation Audit Spawn 1 Foundation Auditor via `Agent(subagent_type="autoskillit:plan-foundation-auditor")`. Pass the full draft immunity plan text and the codebase root. Prepend the contrastive frame to the prompt: @@ -213,11 +160,11 @@ The Foundation Auditor performs step-by-step control-flow analysis: enumerates f The `summary` field is **required** when `message` is a string — omitting it causes `InputValidationError`. If the resumed agent still returns truncated, proceed without its findings rather than retrying further. -After reading the agent's findings, revise the draft plan by incorporating all valid findings (real gaps, not hypotheticals) before proceeding to Step 6. +After reading the agent's findings, revise the draft plan by incorporating all valid findings (real gaps, not hypotheticals) before proceeding to Step 5. -### Step 6: Interface Mapping +### Step 5: Interface Mapping -Spawn 1 Interface Mapper via `Agent(subagent_type="autoskillit:plan-interface-mapper")`. Pass the **revised** draft plan text (from Step 5) and the codebase root. Prepend the contrastive frame to the prompt: +Spawn 1 Interface Mapper via `Agent(subagent_type="autoskillit:plan-interface-mapper")`. Pass the **revised** draft plan text (from Step 4) and the codebase root. Prepend the contrastive frame to the prompt: > "A junior engineer reviewed this plan's variable usage and found it correct. What did they miss?" @@ -232,11 +179,11 @@ The Interface Mapper traces variable SET/READ points with full hop-by-hop proven The `summary` field is **required** when `message` is a string — omitting it causes `InputValidationError`. If the resumed agent still returns truncated, proceed without its findings rather than retrying further. -After reading the agent's findings, revise the draft plan by incorporating all valid findings before proceeding to Step 7. +After reading the agent's findings, revise the draft plan by incorporating all valid findings before proceeding to Step 6. -### Step 7: Registry Trace +### Step 6: Registry Trace -Spawn 1 Registry Tracer via `Agent(subagent_type="autoskillit:plan-registry-tracer")`. Pass the **revised** draft plan text (from Step 6) and the codebase root. Prepend the contrastive frame to the prompt: +Spawn 1 Registry Tracer via `Agent(subagent_type="autoskillit:plan-registry-tracer")`. Pass the **revised** draft plan text (from Step 5) and the codebase root. Prepend the contrastive frame to the prompt: > "A junior engineer reviewed this plan's registry coverage and found it complete. What did they miss?" @@ -255,18 +202,6 @@ After reading the agent's findings, apply all valid findings. The plan is now fu --- -## Skill Loading Checklist - -Before writing the final plan, verify: - -- [ ] Determined which architecture lens best fits the proposed changes -- [ ] LOADED the corresponding `/autoskillit:arch-lens-*` skill using the Skill tool -- [ ] The arch-lens skill LOADED the `/autoskillit:mermaid` skill for styling -- [ ] Diagram uses ONLY the classDef styles from the mermaid skill (no invented colors) -- [ ] Diagram includes a color legend table -- [ ] Every new component, class, or function is wired into the call chain — nothing is created but left unconnected -- [ ] Adversarial review pass completed (Steps 5-7) — all 3 agents spawned and findings applied - ## Output If the plan exceeds 500 lines, split it into multiple files (`_part_a`, `_part_b`, etc.). Each part must be a **self-contained, independently implementable plan** executed sequentially. Split by functional scope (e.g., Part A = "fix core bug + tests", Part B = "add guards + enforcement"), NOT by document structure. Each file must have its own failing tests, implementation steps, and verification. @@ -321,11 +256,6 @@ plan_parts = {path_to_part_a} ## Architectural Analysis {Map of affected components and their connections} -## Proposed Architecture -{Mermaid diagram showing the proposed changes using the selected lens} - -**Lens Used:** {lens name} - {why this lens was chosen} - ## Immunity Plan ### Step 1: Failing Tests diff --git a/tests/contracts/test_activate_deps_completeness.py b/tests/contracts/test_activate_deps_completeness.py index 378c4d6637..3cd8ce8d16 100644 --- a/tests/contracts/test_activate_deps_completeness.py +++ b/tests/contracts/test_activate_deps_completeness.py @@ -1,7 +1,7 @@ """Contracts: SKILL.md activate_deps must cover invoked Skill tool calls. -Catches the four known cases (rectify, open-integration-pr, elaborate-phase, -make-arch-diag) and prevents future regressions where a SKILL.md body invokes +Catches known dependency cases such as open-integration-pr, elaborate-phase, +and make-arch-diag, and prevents future regressions where a SKILL.md body invokes ``/autoskillit:`` via the Skill tool without declaring the corresponding ``activate_deps`` entry. """ @@ -20,7 +20,11 @@ _parse_write_paths, compute_skill_closure, ) -from autoskillit.workspace.skills import bundled_skills_dir, bundled_skills_extended_dir +from autoskillit.workspace.skills import ( + _read_skill_frontmatter, + bundled_skills_dir, + bundled_skills_extended_dir, +) pytestmark = [pytest.mark.layer("contracts"), pytest.mark.medium] @@ -75,9 +79,27 @@ def test_elaborate_phase_declares_dry_walkthrough_dep(provider: SkillsDirectoryP assert "dry-walkthrough" in closure, closure -def test_rectify_declares_arch_lens_dep(provider: SkillsDirectoryProvider) -> None: - closure = compute_skill_closure("rectify", provider) - assert any(name.startswith("arch-lens-") for name in closure), closure +def test_planning_skill_frontmatter_and_closures( + provider: SkillsDirectoryProvider, +) -> None: + make_plan_md = _find_skill_md("make-plan") + assert make_plan_md is not None + assert _get_activate_deps(make_plan_md) == ["write-recipe"] + assert compute_skill_closure("make-plan", provider) == frozenset({"make-plan", "write-recipe"}) + + rectify_md = _find_skill_md("rectify") + assert rectify_md is not None + rectify_frontmatter = _read_skill_frontmatter(rectify_md) + assert set(rectify_frontmatter.get("uses_capabilities", [])) == { + "agent_model", + "agent_subagent", + } + assert "activate_deps" not in rectify_frontmatter + assert compute_skill_closure("rectify", provider) == frozenset({"rectify"}) + + +def test_make_plan_required_dependency_registry_is_exact() -> None: + assert SKILL_ACTIVATE_DEPS_REQUIRED["make-plan"] == frozenset({"write-recipe"}) def test_open_integration_pr_declares_arch_lens_dep(provider: SkillsDirectoryProvider) -> None: diff --git a/tests/contracts/test_make_plan_echo_rule.py b/tests/contracts/test_make_plan_echo_rule.py index 6457d63e79..d6b81d1654 100644 --- a/tests/contracts/test_make_plan_echo_rule.py +++ b/tests/contracts/test_make_plan_echo_rule.py @@ -24,8 +24,7 @@ def _read_skill_md() -> str: def test_make_plan_skill_contains_echo_rule() -> None: """make-plan SKILL.md must mandate that every behavioral requirement - in Design Decisions / Summary / Proposed Architecture is echoed as - an explicit Implementation Steps directive.""" + in Design Decisions / Summary is echoed as an Implementation Steps directive.""" content = _read_skill_md() assert "echo" in content.lower() or "traced" in content.lower(), ( "make-plan/SKILL.md must contain an echo validation rule requiring behavioral " @@ -33,12 +32,13 @@ def test_make_plan_skill_contains_echo_rule() -> None: ) assert "Implementation Steps" in content assert "Design Decisions" in content or "Summary" in content + assert "## Proposed Architecture" not in content def test_make_plan_template_has_traceability() -> None: """Plan template must include a requirement traceability section that maps each prose constraint to an Implementation Steps item.""" content = _read_skill_md() - assert "Requirements Map" in content or "Requirements Traceability" in content, ( - "make-plan/SKILL.md must include a Requirements Map or Requirements Traceability section" + assert "Requirements Map" in content, ( + "make-plan/SKILL.md must include a Requirements Map section" ) diff --git a/tests/contracts/test_target_skill_invocability.py b/tests/contracts/test_target_skill_invocability.py index 12c9714284..b20d718146 100644 --- a/tests/contracts/test_target_skill_invocability.py +++ b/tests/contracts/test_target_skill_invocability.py @@ -135,12 +135,13 @@ class TestDepSkillsNotGatedAfterActivation: """After activating a target with activate_deps, dependency skills are also ungated.""" def test_dep_skills_not_gated_after_activation(self, tmp_path: Path) -> None: - """After activating make-plan, arch-lens-* and mermaid skills are ungated.""" + """Activating make-plan delivers and ungates only its retained dependency.""" config = load_config() provider = SkillsDirectoryProvider() mgr = DefaultSessionSkillManager(provider, tmp_path) session_id = "test-dep-activation" closure = mgr.compute_skill_closure("make-plan") + assert closure == frozenset({"make-plan", "write-recipe"}) mgr.init_session( session_id, cook_session=False, @@ -150,25 +151,11 @@ def test_dep_skills_not_gated_after_activation(self, tmp_path: Path) -> None: mgr.activate_skill_deps(session_id, "make-plan") skills_base = tmp_path / session_id / ".claude" / "skills" - # Check arch-lens skills are ungated - for skill_dir in sorted(skills_base.iterdir()): - if not skill_dir.is_dir(): - continue - name = skill_dir.name - if not name.startswith("arch-lens-"): - continue - content = (skill_dir / "SKILL.md").read_text() - assert "disable-model-invocation: true" not in content, ( - f"arch-lens skill '{name}' should be ungated after activating make-plan" - ) - - # Check mermaid is ungated - mermaid_md = skills_base / "mermaid" / "SKILL.md" - assert mermaid_md.exists(), "mermaid skill dir should exist after init_session" - content = mermaid_md.read_text() - assert "disable-model-invocation: true" not in content, ( - "mermaid should be ungated via transitive dependency from make-plan" - ) + delivered = {path.parent.name for path in skills_base.glob("*/SKILL.md")} + assert delivered == set(closure) + for name in closure: + content = (skills_base / name / "SKILL.md").read_text() + assert "disable-model-invocation: true" not in content class TestAllRecipeSkillCommandsInvocable: diff --git a/tests/execution/test_headless_path_validation.py b/tests/execution/test_headless_path_validation.py index 7a7f7a0705..5daa627c12 100644 --- a/tests/execution/test_headless_path_validation.py +++ b/tests/execution/test_headless_path_validation.py @@ -747,7 +747,9 @@ def test_synthesis_uses_last_write_not_first(self, make_headless_session): { "name": "Write", "id": "t1", - "file_path": "/cwd/.autoskillit/temp/make-plan/arch_lens_selection.md", + "file_path": ( + "/cwd/.autoskillit/temp/make-plan/task_plan_draft_2026-01-01.md" + ), }, { "name": "Write", @@ -763,7 +765,7 @@ def test_synthesis_uses_last_write_not_first(self, make_headless_session): assert ( "plan_path = /cwd/.autoskillit/temp/make-plan/task_plan_2026-01-01.md" in result.result ) - assert "arch_lens_selection" not in result.result + assert "task_plan_draft_2026-01-01.md" not in result.result def test_synthesizes_path_from_backslash_s_plus_pattern(self, make_headless_session): """Synthesis must work for \\S+-terminated patterns, not only /.+-terminated ones.""" diff --git a/tests/execution/test_headless_synthesis.py b/tests/execution/test_headless_synthesis.py index a5cb8696e3..28840062a2 100644 --- a/tests/execution/test_headless_synthesis.py +++ b/tests/execution/test_headless_synthesis.py @@ -1011,7 +1011,7 @@ def test_synthesis_not_run_for_unparseable_channel_b_with_write_evidence(self) - "id": "t1", "name": "Write", "input": { - "file_path": "/cwd/.autoskillit/temp/make-plan/arch_lens_selection_2026-01-01.md" # noqa: E501 + "file_path": "/cwd/.autoskillit/temp/make-plan/task_plan_draft_2026-01-01.md" # noqa: E501 }, } ) @@ -1045,7 +1045,7 @@ def test_synthesis_not_run_for_timeout_channel_b_with_write_evidence(self) -> No "id": "t1", "name": "Write", "input": { - "file_path": "/cwd/.autoskillit/temp/make-plan/arch_lens_selection_2026-01-01.md" # noqa: E501 + "file_path": "/cwd/.autoskillit/temp/make-plan/task_plan_draft_2026-01-01.md" # noqa: E501 }, } ) @@ -1075,7 +1075,7 @@ def test_synthesis_not_run_for_timeout_channel_b_with_write_evidence(self) -> No backend=ClaudeCodeBackend(), ) assert sr.success is False - assert "arch_lens_selection" not in sr.result + assert "task_plan_draft_2026-01-01.md" not in sr.result def test_synthesis_skipped_for_channel_b_session_complete(self) -> None: """CHANNEL_B + SUCCESS + write evidence but no pattern in assistant_messages. diff --git a/tests/execution/test_session_adjudication_success.py b/tests/execution/test_session_adjudication_success.py index 6409d768da..164c216b1b 100644 --- a/tests/execution/test_session_adjudication_success.py +++ b/tests/execution/test_session_adjudication_success.py @@ -374,7 +374,9 @@ def test_channel_b_bypass_fails_for_failure_subtype_with_injected_pattern( session = ClaudeSessionResult( subtype=CliSubtype.UNPARSEABLE, is_error=True, - result="plan_path = /cwd/.autoskillit/temp/make-plan/arch_lens_2026.md\n", + result=( + "plan_path = /cwd/.autoskillit/temp/make-plan/task_plan_2026-01-01.md\n" + ), session_id="s1", ) success = _compute_success( diff --git a/tests/server/test_admission_dispatch_agreement.py b/tests/server/test_admission_dispatch_agreement.py index 6e174946c6..99cdd9964a 100644 --- a/tests/server/test_admission_dispatch_agreement.py +++ b/tests/server/test_admission_dispatch_agreement.py @@ -409,8 +409,7 @@ def _real_skill_resolver(): def test_make_plan_reroutes_on_codex() -> None: - """make-plan declares [agent_model, agent_subagent, cross_skill_ref] — - all worker_routable post-fix — so it must reroute to claude-code on Codex.""" + """make-plan's agent capabilities require Claude Code routing from Codex.""" from unittest.mock import MagicMock from autoskillit.config._config_dataclasses import ProvidersConfig @@ -421,7 +420,7 @@ def test_make_plan_reroutes_on_codex() -> None: skill_info = resolver.resolve("make-plan") assert skill_info is not None, "make-plan must be resolvable from bundled SKILL.md" assert _has_routing_capability(skill_info.uses_capabilities) is True, ( - "make-plan declares agent_model, agent_subagent, cross_skill_ref — all worker_routable" + "make-plan declares agent_model and agent_subagent — both worker_routable" ) step = MagicMock() diff --git a/tests/server/test_tools_execution_routing.py b/tests/server/test_tools_execution_routing.py index 562e0552db..5387ee53d3 100644 --- a/tests/server/test_tools_execution_routing.py +++ b/tests/server/test_tools_execution_routing.py @@ -211,10 +211,10 @@ async def test_run_skill_no_target_skill_passes_none_allow_only( @pytest.mark.anyio -async def test_run_skill_make_plan_closure_includes_arch_lens_pack( +async def test_run_skill_make_plan_passes_exact_retained_closure( tool_ctx_kitchen_open, monkeypatch ) -> None: - """End-to-end: /make-plan resolves a closure containing the entire arch-lens pack.""" + """End-to-end: /make-plan forwards its exact retained activation closure.""" from unittest.mock import MagicMock from autoskillit.core import ValidatedAddDir @@ -262,11 +262,7 @@ def cleanup_stale(self, max_age_seconds=86400): await run_skill("/autoskillit:make-plan refactor", "/tmp") closure = captured["allow_only"] - assert closure is not None - assert "make-plan" in closure - assert "mermaid" in closure - arch_members = {n for n in closure if n.startswith("arch-lens-")} - assert len(arch_members) >= 1 + assert closure == frozenset({"make-plan", "write-recipe"}) @pytest.mark.anyio diff --git a/tests/skills/test_adversarial_review_contracts.py b/tests/skills/test_adversarial_review_contracts.py index 8c1f1d567f..7bbf9dc2d3 100644 --- a/tests/skills/test_adversarial_review_contracts.py +++ b/tests/skills/test_adversarial_review_contracts.py @@ -6,6 +6,38 @@ pytestmark = [pytest.mark.layer("skills"), pytest.mark.medium] +_REMOVED_DIAGRAM_WORKFLOW_MARKERS = ( + "Visualize with Architecture Lens", + "arch_lens_selection_", + "/autoskillit:arch-lens-", + "/autoskillit:mermaid", +) +_REMOVED_DIAGRAM_OUTPUT_TEMPLATE_MARKERS = ( + "{Mermaid diagram showing the proposed changes using the selected lens}", + "**Lens Used:** {lens name} - {why this lens was chosen}", +) +_CONNECTED_CALL_CHAIN_INVARIANT = ( + "Every new component, class, or function is wired into the call chain — " + "nothing is created but left unconnected" +) + + +def _assert_obsolete_diagram_workflow_absent(text: str, workflow_heading: str) -> None: + workflow_idx = text.find(workflow_heading) + assert workflow_idx != -1 + output_idx = text.find("\n## Output", workflow_idx) + assert output_idx != -1 + + workflow_section = text[workflow_idx:output_idx] + for marker in _REMOVED_DIAGRAM_WORKFLOW_MARKERS: + assert marker not in workflow_section, ( + f"obsolete diagram workflow marker remains: {marker}" + ) + + output_section = text[output_idx:] + for marker in _REMOVED_DIAGRAM_OUTPUT_TEMPLATE_MARKERS: + assert marker not in output_section, f"obsolete diagram output marker remains: {marker}" + @pytest.fixture(scope="module") def make_plan_text() -> str: @@ -66,15 +98,11 @@ def test_make_plan_step9_plan_revision_exists(make_plan_text: str) -> None: ), "Step 9 must reference reading the three adversarial reports" -def test_make_plan_checklist_includes_adversarial_review(make_plan_text: str) -> None: - checklist_idx = make_plan_text.find("## Skill Loading Checklist") - assert checklist_idx != -1 - next_heading = make_plan_text.find("\n## ", checklist_idx + 1) - end = next_heading if next_heading != -1 else len(make_plan_text) - checklist_section = make_plan_text[checklist_idx:end].lower() - assert "adversarial" in checklist_section, ( - "Skill Loading Checklist must include an adversarial review checklist item" - ) +def test_make_plan_omits_diagram_workflow_and_retains_call_chain_invariant( + make_plan_text: str, +) -> None: + _assert_obsolete_diagram_workflow_absent(make_plan_text, "## Planning Steps") + assert _CONNECTED_CALL_CHAIN_INVARIANT in make_plan_text def test_make_plan_steps_6_through_9_ordered(make_plan_text: str) -> None: @@ -135,24 +163,22 @@ def test_plan_registry_tracer_turn_budget_instruction() -> None: ) -def test_make_plan_steps_5e_through_9_ordered(make_plan_text: str) -> None: - """Verify Steps 5e and 6-9 appear in order in the document.""" +def test_make_plan_steps_5_through_9_ordered(make_plan_text: str) -> None: + """Verify the promoted Step 5 and Steps 6-9 appear in order.""" planning_idx = make_plan_text.find("## Planning Steps") assert planning_idx != -1 - step5e_idx = make_plan_text.find( - "**5e. Complexity-Gated Adversarial Review Decision", planning_idx + step5_idx = make_plan_text.find( + "5. **Complexity-Gated Adversarial Review Decision**", planning_idx ) step6_idx = make_plan_text.find("**Foundation Audit", planning_idx) step7_idx = make_plan_text.find("**Interface Mapping", planning_idx) step8_idx = make_plan_text.find("**Registry Trace", planning_idx) step9_idx = make_plan_text.find("**Plan Revision", planning_idx) - assert step5e_idx != -1, ( - "Step 5e must exist with 'Complexity-Gated Adversarial Review Decision' heading" + assert step5_idx != -1, ( + "Step 5 must exist with 'Complexity-Gated Adversarial Review Decision' heading" ) assert all(i != -1 for i in (step6_idx, step7_idx, step8_idx, step9_idx)) - assert step5e_idx < step6_idx < step7_idx < step8_idx < step9_idx, ( - "Steps 5e-9 must appear in order in the document" - ) + assert step5_idx < step6_idx < step7_idx < step8_idx < step9_idx def test_make_plan_interface_mapping_and_registry_trace_responsibilities( @@ -254,18 +280,16 @@ def test_rectify_adversarial_steps_ordered(rectify_text: str) -> None: "Adversarial steps must appear in order: " "Foundation Audit < Interface Mapping < Registry Trace" ) + assert "### Step 4: Foundation Audit" in rectify_text + assert "### Step 5: Interface Mapping" in rectify_text + assert "### Step 6: Registry Trace" in rectify_text -def test_rectify_checklist_includes_adversarial_review(rectify_text: str) -> None: - """Rectify Skill Loading Checklist must include adversarial review item.""" - checklist_idx = rectify_text.find("## Skill Loading Checklist") - assert checklist_idx != -1 - next_heading = rectify_text.find("\n## ", checklist_idx + 1) - end = next_heading if next_heading != -1 else len(rectify_text) - section = rectify_text[checklist_idx:end].lower() - assert "adversarial" in section, ( - "Skill Loading Checklist must include an adversarial review checklist item" - ) +def test_rectify_omits_diagram_workflow_and_retains_call_chain_invariant( + rectify_text: str, +) -> None: + _assert_obsolete_diagram_workflow_absent(rectify_text, "## Rectify Workflow") + assert _CONNECTED_CALL_CHAIN_INVARIANT in rectify_text def test_rectify_interface_mapping_rules(rectify_text: str) -> None: @@ -340,9 +364,9 @@ def test_rectify_adversarial_steps_contain_continuation_protocol(rectify_text: s rt_end = next_heading if next_heading != -1 else len(rectify_text) for label, start, end in [ - ("Step 5 (Foundation Audit)", fa_idx, im_idx), - ("Step 6 (Interface Mapping)", im_idx, rt_idx), - ("Step 7 (Registry Trace)", rt_idx, rt_end), + ("Step 4 (Foundation Audit)", fa_idx, im_idx), + ("Step 5 (Interface Mapping)", im_idx, rt_idx), + ("Step 6 (Registry Trace)", rt_idx, rt_end), ]: section = rectify_text[start:end].lower() assert "sendmessage" in section, f"{label} must mention SendMessage" diff --git a/tests/skills/test_make_plan_capability_accuracy.py b/tests/skills/test_make_plan_capability_accuracy.py index 7bb5da1265..d002730bab 100644 --- a/tests/skills/test_make_plan_capability_accuracy.py +++ b/tests/skills/test_make_plan_capability_accuracy.py @@ -1,4 +1,4 @@ -"""Semantic guard: make-plan must not declare git_metadata_write capability.""" +"""Semantic guard for make-plan's exact worker-routable capabilities.""" from __future__ import annotations @@ -10,7 +10,7 @@ pytestmark = [pytest.mark.small] -def test_make_plan_does_not_declare_git_metadata_write(): +def test_make_plan_declares_exact_worker_capabilities() -> None: fm = _read_skill_frontmatter(pkg_root() / "skills_extended" / "make-plan" / "SKILL.md") caps = set(fm.get("uses_capabilities", [])) - assert "git_metadata_write" not in caps + assert caps == {"agent_model", "agent_subagent"} diff --git a/tests/workspace/test_session_skills_allow_only_and_closure.py b/tests/workspace/test_session_skills_allow_only_and_closure.py index 2c6a070dc6..d3c69f51aa 100644 --- a/tests/workspace/test_session_skills_allow_only_and_closure.py +++ b/tests/workspace/test_session_skills_allow_only_and_closure.py @@ -407,15 +407,12 @@ def test_closure_standalone_returns_only_self(self, tmp_path: Path) -> None: provider = _make_synthetic_provider(tmp_path, {"lone": {}}) assert compute_skill_closure("lone", provider) == frozenset({"lone"}) - def test_closure_pack_dep_expands_to_pack_members(self) -> None: + def test_make_plan_production_closure_is_exact(self) -> None: from autoskillit.workspace.session_skills import compute_skill_closure provider = SkillsDirectoryProvider() closure = compute_skill_closure("make-plan", provider) - assert "make-plan" in closure - assert "mermaid" in closure # transitive via arch-lens-* deps - arch_members = {n for n in closure if n.startswith("arch-lens-")} - assert len(arch_members) >= 1 + assert closure == frozenset({"make-plan", "write-recipe"}) def test_closure_individual_skill_dep(self, tmp_path: Path) -> None: from autoskillit.workspace.session_skills import compute_skill_closure diff --git a/tests/workspace/test_session_skills_deps.py b/tests/workspace/test_session_skills_deps.py index 69c21ea255..4907fe91f4 100644 --- a/tests/workspace/test_session_skills_deps.py +++ b/tests/workspace/test_session_skills_deps.py @@ -79,8 +79,8 @@ def test_activate_skill_deps_resolves_pack_deps(self, tmp_path: Path) -> None: _write_skill_md( tmp_path, session_id, - "make-plan", - f"---\nname: make-plan\nactivate_deps: [arch-lens]\n{gate}\n---\n# Plan", + "make-arch-diag", + f"---\nname: make-arch-diag\nactivate_deps: [arch-lens]\n{gate}\n---\n# Diagram", ) # Three arch-lens skills for name in ["arch-lens-a", "arch-lens-b", "arch-lens-c"]: @@ -114,9 +114,9 @@ def resolve_fn(name: str) -> SkillInfo | None: provider.list_skills.return_value = [] mgr = DefaultSessionSkillManager(provider, ephemeral_root=tmp_path) - result = mgr.activate_skill_deps(session_id, "make-plan") + result = mgr.activate_skill_deps(session_id, "make-arch-diag") assert result is True - assert not _is_gated(tmp_path, session_id, "make-plan") + assert not _is_gated(tmp_path, session_id, "make-arch-diag") for name in ["arch-lens-a", "arch-lens-b", "arch-lens-c"]: assert not _is_gated(tmp_path, session_id, name), f"{name} should be ungated" @@ -149,7 +149,7 @@ def test_activate_skill_deps_resolves_individual_skill_dep(self, tmp_path: Path) assert not _is_gated(tmp_path, session_id, "mermaid") def test_activate_skill_deps_resolves_two_level_transitive(self, tmp_path: Path) -> None: - """make-plan -> arch-lens-* -> mermaid: all three levels get ungated.""" + """make-arch-diag -> arch-lens-* -> mermaid: all levels get ungated.""" from unittest.mock import MagicMock from autoskillit.core.types import SkillSource @@ -160,8 +160,8 @@ def test_activate_skill_deps_resolves_two_level_transitive(self, tmp_path: Path) _write_skill_md( tmp_path, session_id, - "make-plan", - f"---\nname: make-plan\nactivate_deps: [arch-lens]\n{gate}\n---\n# Plan", + "make-arch-diag", + f"---\nname: make-arch-diag\nactivate_deps: [arch-lens]\n{gate}\n---\n# Diagram", ) _write_skill_md( tmp_path, @@ -196,8 +196,8 @@ def resolve_fn(name: str) -> SkillInfo | None: provider.list_skills.return_value = [] mgr = DefaultSessionSkillManager(provider, ephemeral_root=tmp_path) - mgr.activate_skill_deps(session_id, "make-plan") - assert not _is_gated(tmp_path, session_id, "make-plan") + mgr.activate_skill_deps(session_id, "make-arch-diag") + assert not _is_gated(tmp_path, session_id, "make-arch-diag") assert not _is_gated(tmp_path, session_id, "arch-lens-x") assert not _is_gated(tmp_path, session_id, "mermaid")