refactor(core): decompose plugin_validator.py behind stable compatibility exports

[Jamie-BitFlight]

Parent: #280

Outcome

Reduce packages/skilllint/plugin_validator.py from a multi-responsibility central module into a compatibility facade over smaller owner modules, without breaking current public/internal imports during the migration.

Current evidence

On main, plugin_validator.py is roughly 4,930 lines and contains dozens of top-level functions and classes spanning:

• shared validation models/contracts;
• policy/config/suppression;
• frontmatter/YAML repair;
• validators;
• fix orchestration;
• adapter/platform routing;
• CLI commands;
• runtime orchestration;
• compatibility exports.

Many tests, rule modules, scripts, and examples import types or behavior from skilllint.plugin_validator, so a big-bang move would create unnecessary churn.

Current-main reconciliation — 2026-09-29

Current main has already completed the contract-first foundation:

• the import/dependency inventory and compatibility requirements have been exercised by the earlier decomposition work;
• shared validation contracts now originate in packages/skilllint/models.py;
• skilllint.plugin_validator re-exports those contracts for compatibility;
• scan_runtime.py and reporting.py already own their extracted runtime/reporting seams.

Policy/config ownership is complete via PR #301. PR #302 then made explicit-platform routing declaration-driven without combining a #283 extraction slice: adapter rule routing still lives in plugin_validator.py, while discovery remains in scan_runtime.py.

Current-main reconciliation — 2026-09-30 after #302

The next coherent #283 slice is fix authorization and execution orchestration (Slice D1), not validation routing or CLI extraction.

Current ownership evidence on main:

• FIXER_TRIGGER_CODES and get_fixer_trigger_codes() live in plugin_validator.py and define fail-closed rule-to-fixer authorization.
• validate_single_path() owns the generic fix loop: trigger intersection, can_fix() gating, ordered fix() calls, AppliedFix recording, and revalidation.
• _get_fixers_for_path() still selects the reporting validators plus the fix-only NameFormatValidator, preserving an important ordering contract.
• Concrete mutations remain domain-specific: frontmatter normalization/name repair, hook execute-bit mutation, and symlink repair.
• safe_load_yaml_with_colon_fix() is also still in the legacy module and has lazy imports from rule/boundary code; moving it casually with the coordinator would create unnecessary dependency/cycle risk.

Recommended next slice:

1. Add a dependency-light fixing.py owner for FIXER_TRIGGER_CODES, get_fixer_trigger_codes(), and a generic coordinator that receives an already ordered fixer sequence plus raw_codes, executes authorized fixes, records AppliedFix, and returns whether revalidation is needed.
2. Keep _get_fixers_for_path() and concrete validator fix() implementations in place for this slice. That avoids importing validator classes back into fixing.py and keeps validator selection for Slice E.
3. Preserve plugin_validator compatibility re-exports for moved names.
4. Keep the existing fixer-gating tests as the primary contract; add only the import-identity/coordinator evidence needed to prove the new owner is active.
5. Re-measure import/startup behavior, but do not fold perf(tests): suite spends ~two thirds of its wall clock on subprocess startup #148 performance work into the extraction.

A later Slice D2 can assess frontmatter mutation helpers separately once the generic coordinator is no longer embedded in validation dispatch. Slice E can then extract validator selection/collection and validate_file / validate_single_path around a smaller, already-separated fixing seam.

Current-main reconciliation — 2026-09-30 after #303/#304

PR #303 completed Slice D1: fixing.py now owns fail-closed fixer authorization and generic ordered execution while plugin_validator preserves compatibility re-exports. PR #304 completed the coherent D2 syntax seam: frontmatter_yaml.py owns YAML parsing/repair primitives and rule/boundary YAML recovery no longer reaches into the legacy validator.

A direct Slice E move of _get_validators_for_path, _collect_validator_results, validate_single_path, and validate_file is still premature. Concrete validator classes remain owned by plugin_validator.py, and several rule modules still perform deferred reverse imports from that module for unrelated utilities. Moving validation orchestration now would create or conceal a validation -> plugin_validator -> rules -> validation/plugin_validator dependency cycle.

The next coherent prerequisite is Slice E0 — remove rule-layer reverse dependencies on the legacy module without changing behavior:

1. Move SKILL.md document parsing to the existing frontmatter parsing owner and preserve the plugin_validator.parse_skill_md compatibility alias.
2. Move plugin-root/marketplace-root ancestry lookup to scan_runtime.py, which already owns path discovery, and preserve legacy aliases.
3. Put the frontmatter-exempt filename constant with the frontmatter contract rather than importing it from the CLI/validator module.
4. Let PA rules use rule_registry.rule_reference and their own registry IDs rather than the legacy ErrorCode wrapper.
5. Put HK005's Git execute-bit observation beside HK005 detection, while keeping the legacy private alias for tests/callers.
6. Repoint LK/MCP/PA/PR/AS rule modules to those owners and verify no product rule module imports plugin_validator afterwards.
7. Keep imports deferred where they currently protect import skilllint.rules startup cost; perf(tests): suite spends ~two thirds of its wall clock on subprocess startup #148 remains the performance-measurement owner.

Once E0 lands, reassess Slice E against the now one-way dependency graph before moving validator selection/collection.

Current-main reconciliation — 2026-09-30 after #305

PR #305 completed Slice E0. Product rule modules now have a one-way dependency on domain owners and an AST contract test rejects any rule import of skilllint.plugin_validator. Compatibility aliases remain available from the legacy module.

A direct validation-orchestration move is still blocked by one smaller ownership seam: FileType, frontmatter requirement classification, and the name-bearing file-type set still originate in plugin_validator.py. Any extracted concrete validator that needs file-type context would otherwise have to import the legacy module and recreate the cycle from a different direction.

The next coherent prerequisite is Slice E1 — extract file/capability classification:

1. Add a dependency-light file_types.py owner for FileType and frontmatter requirement classification.
2. Move the name-bearing file-type set and the quick frontmatter-presence helper with that classification contract.
3. Depend only on scan_runtime for scan context/manifest lookup and frontmatter_core for the exemption contract; do not import validators or the legacy facade.
4. Preserve plugin_validator.FileType, _FrontmatterRequirement, _frontmatter_requirement, _file_has_frontmatter, and _NAME_BEARING_FILE_TYPES as compatibility aliases while callers migrate.
5. Use existing file-type/frontmatter behavior tests as primary evidence and add only compatibility identity/architecture proof needed for the new owner.

After E1, concrete validators can be extracted by domain without importing plugin_validator; reassess the smallest validator-owner slice before moving validate_single_path itself.

Current-main reconciliation — 2026-09-30 after #306

PR #306 completed Slice E1. file_types.py now owns ScanContext, FileType, and frontmatter-requirement classification; plugin_manifest.py owns the dependency-light cached Claude plugin manifest reader. plugin_validator and scan_runtime preserve their previous symbol paths as compatibility aliases.

The next coherent slice is Slice E2 — extract rule-series validator adapters, not a generic validators.py move.

Current-main evidence:

• ProgressiveDisclosureValidator, InternalLinkValidator, NamespaceReferenceValidator, and AsSeriesValidator primarily adapt rule-series functions into the shared Validator protocol and do not own mutation, subprocess execution, plugin-tree orchestration, or schema repair.
• Their rule logic already lives in rules/pd_series.py, rules/lk_series.py, rules/nr_series.py, and rules/as_series.py.
• Moving the adapter classes directly into those rule modules would make the eagerly loaded rule registry pull additional policy/frontmatter dependencies; keeping adapter packaging in a separate validators/rule_series.py preserves the lightweight rule-registration path.
• DescriptionValidator, ComplexityValidator, and MarkdownTokenCounter still own additional parsing/counting behavior and should be assessed separately rather than swept into E2.
• Fixable SymlinkTargetValidator, NameFormatValidator, FrontmatterValidator, and HookValidator, plus plugin-tree/subprocess validators, remain out of scope.

Recommended E2:

1. Establish a validators/ package and a focused rule_series.py owner for the four rule-series adapters.
2. Use dependency-light domain owners (models, policy, frontmatter_yaml, rule_registry, rule modules) only; never import plugin_validator or scan/CLI orchestration.
3. Preserve the four legacy class imports from plugin_validator by identity.
4. Keep existing validator behavior tests as the primary proof; add only architecture/identity checks needed to protect the new seam.
5. Reassess the remaining concrete validators after E2 before extracting validation orchestration.

Current-main reconciliation — 2026-09-30 after #307

PR #307 completed Slice E2. validators/rule_series.py now owns the read-only protocol adapters for AS, LK001, NR, and PD while rules/ remains the lightweight rule-truth/registration layer.

The next coherent slice is Slice E3 — extract read-only content quality/token validators:

1. Add a focused validators/content.py owner for DescriptionValidator, ComplexityValidator, and MarkdownTokenCounter.
2. Preserve their existing parsing/counting and policy-threshold behavior exactly; this is ownership movement, not a rewrite onto new helpers.
3. Depend only on file_types, frontmatter parsing, models, policy, rule functions, rule_registry, and token counting. Do not import the legacy facade, scan orchestration, fixing, or CLI.
4. Preserve all three plugin_validator class imports by identity.
5. Keep ComplexityMetrics in place for now: it has no current consumers, and moving an unused compatibility type merely to reduce line count is not a coherent extraction.
6. Keep frontmatter-specific issue normalization with frontmatter validation for this slice; do not broaden E3 into the mutation/schema seam.

After E3, reassess fixable validators, plugin-tree/subprocess validators, and validator ownership/selection metadata before extracting validate_single_path.

Current-main reconciliation — 2026-09-30 after #308

PR #308 completed Slice E3. validators/content.py now owns DescriptionValidator, ComplexityValidator, and MarkdownTokenCounter; the legacy facade only re-exports those active classes.

The next coherent prerequisite is Slice E4 — extract validator routing metadata:

1. Add validators/metadata.py as the dependency-light owner of ValidatorOwnership, VALIDATOR_OWNERSHIP, VALIDATOR_CONSTRAINT_SCOPES, get_validator_ownership(), get_validator_constraint_scopes(), and filter_validators_by_constraint_scopes().
2. Depend only on the shared Validator protocol and stdlib; do not import concrete validators, the legacy facade, scan runtime, rules, fixing, or CLI.
3. Preserve every existing plugin_validator metadata symbol by identity.
4. Keep class-name keyed metadata unchanged for this slice. Converting it to concrete-type registration would couple the metadata owner back to all validator implementations and defeat the extraction.
5. Keep existing ownership/routing tests as the behavioral proof and add only compatibility/dependency-boundary evidence.

After E4, reassess concrete fixable/plugin validators and then validator selection/collection. validate_single_path should move only after selection no longer requires implementation ownership from the legacy module.

Current-main reconciliation — 2026-09-30 after #309

PR #309 completed Slice E4. validators/metadata.py now owns validator ownership and provider constraint-scope routing without depending on concrete implementations.

The next coherent slice is Slice E5 — extract non-frontmatter filesystem-mutation validators:

1. Move SymlinkTargetValidator to a focused validators/symlinks.py owner. SL001 detection remains in rules/sl_series.py; the validator owns the safe symlink rewrite.
2. Move HookValidator to validators/hooks.py. HK detection remains in rules/hk_series.py; the validator owns execute-bit repair and rule-result packaging.
3. Preserve both legacy plugin_validator class imports by identity.
4. Do not combine FrontmatterValidator / NameFormatValidator: their schema/YAML/name mutation seam is substantially larger and should be reviewed as its own slice.
5. Do not combine plugin structure/registration/link-escape validators: those own plugin-tree/subprocess semantics and form a separate responsibility group.
6. Keep existing hook/symlink/fixer-gating tests as primary behavior proof; add only compatibility/dependency-boundary evidence.

After E5, reassess the plugin validator group and frontmatter validator group. Once concrete classes no longer originate in the legacy module, validator selection/collection can move without reverse implementation ownership.

Current-main reconciliation — 2026-09-30 after #310

PR #310 completed Slice E5. validators/hooks.py and validators/symlinks.py now own the non-frontmatter filesystem mutation validators; the legacy facade preserves both class identities.

The next coherent slice is Slice E6 — extract the frontmatter validation/mutation subsystem:

1. Add validators/frontmatter.py as the owner of FrontmatterValidator, NameFormatValidator, and their schema/YAML/name-specific helper functions.
2. Move the frontmatter-local naming constants/helpers (NAME_PATTERN, _normalize_skill_name, skill-directory validation) with that subsystem.
3. Depend only on file_types, frontmatter_core, frontmatter_yaml, shared models, rule metadata/functions, and validators/hooks.py for embedded hook-reference checks. Do not import the legacy facade, scan runtime, fixing orchestration, CLI, or reporting.
4. Use canonical rule IDs / rule_reference inside the new owner rather than depending on the legacy ErrorCode enum. Keep ErrorCode and its aliases in the compatibility facade.
5. Preserve existing plugin_validator.FrontmatterValidator, NameFormatValidator, NAME_PATTERN, _normalize_skill_name, and moved private helper names as compatibility imports where practical.
6. Keep existing frontmatter/name/fixer-gating/rule-truth tests as primary behavior proof. Add only owner/identity dependency-boundary evidence.
7. Do not combine plugin registration, plugin link-escape, or Claude CLI structure validation; those are a separate plugin-tree/subprocess responsibility.

After E6, the only concrete validator implementations left in the legacy module should be the plugin-tree/subprocess group. Extract that group before moving validator selection/collection and validate_single_path.

Current-main reconciliation — 2026-09-30 after #311

PR #311 completed Slice E6. validators/frontmatter.py now owns frontmatter schema validation, normalization, FM009 state, FM010/name repair, and the frontmatter-specific helper graph. The legacy facade preserves class/helper/model aliases.

The only concrete validators still implemented in plugin_validator.py are now the plugin-tree/subprocess group, so the next coherent slice is Slice E7 — extract plugin validation and Claude CLI integration:

1. Add validators/plugins.py as the owner of PluginLinkEscapeValidator, PluginRegistrationValidator, and PluginStructureValidator.
2. Move the plugin-only helper graph with them: LK004 plugin-root markers/scope helper, Claude subprocess invocation, Git-Bash resolution, nested-session skip detection, is_claude_available, and validate_with_claude.
3. Depend on existing domain seams (rules/lk_series.py, rules/pl_series.py, rules/pr_series.py, policy.py, and scan_runtime.py) rather than the legacy facade. scan_runtime.run_validation_loop already uses callback injection and does not import validation, so this dependency does not recreate the cycle.
4. Use canonical rule IDs / rule_reference in the new owner; keep legacy ErrorCode aliases in the facade.
5. Preserve legacy class/constants/helper imports by identity where practical.
6. Existing tests that monkeypatch private Claude helpers through plugin_validator should patch the new actual owner after extraction. Preserving replacement side effects of monkeypatching a private compatibility alias would require a reverse dependency and is not a product contract.
7. Keep existing plugin-link/registration/structure/external-tool/CLI tests as the primary behavior proof; add only owner/identity architecture evidence.

After E7, plugin_validator.py should contain no concrete validator class implementations. Reassess validator selection/collection and validate_single_path against that state before moving them.

Dependency inventory — policy/config slice (2026-09-30)

Observed before finalizing Slice C:

• Owner after this slice: policy.py owns ValidationPolicy, IgnoreConfig, config parsing/loading/discovery, threshold/severity configuration, policy/ignore caches, suppression matching, and suppression filtering.
• Validation consumer: plugin_validator.py consumes the policy objects/resolvers/filter but retains finding reclassification, validator dispatch, fixing, adapter routing, and CLI wiring for later refactor(core): decompose plugin_validator.py behind stable compatibility exports #283 slices.
• Compatibility facade: plugin_validator.py continues to expose the moved policy/config names used by existing tests/consumers. Token threshold constants remain imported from token_counter there because validation still consumes them and existing callers import them through the legacy module.
• Unmoved dependency: msgspec.json remains a direct plugin_validator.py dependency because plugin-manifest validation outside the policy block still decodes JSON.
• Dependency direction: policy.py -> models.py + token_counter.py + msgspec + stdlib; plugin_validator.py -> policy.py. No policy-to-validator dependency or local-import cycle is introduced.
• Behavioral proof: existing policy/config and ignore/suppression tests remain the primary contract; one import-identity characterization verifies the compatibility facade for the moved owner objects.
• Performance proof: the PR benchmark supplies paired base/compare startup/import measurements; any material regression must be reconciled before merge.

Target direction

A likely end state is approximately:

packages/skilllint/
├── cli.py
├── models.py
├── validation.py
├── policy.py
├── fixing.py
├── scan_runtime.py
├── reporting.py
├── rule_registry.py
├── rules/
├── adapters/
├── boundary/
├── schemas/
└── plugin_validator.py   # compatibility/re-export facade during migration

Names are illustrative; preserve or adjust them based on actual responsibility boundaries discovered during implementation.

Migration plan

Slice A — dependency inventory and contract

• Inventory imports of skilllint.plugin_validator across product, tests, scripts, rules, and documentation.
• Classify imported symbols as public compatibility, internal compatibility, or implementation detail.
• Add characterization/import-identity tests where compatibility is required.

Slice B — extract shared models/contracts first

Move stable types such as the validation issue/result/fix/file-type contracts into a small dependency-light module.

This should remove the need for rule modules to TYPE_CHECKING-import fundamental result types from the central validator and should reduce circular/deferred imports.

Keep re-exports from plugin_validator.py while consumers migrate.

Slice C — extract policy/config

Move suppression, thresholds, severity/config discovery and related policy models to an owner module. Preserve behavior and public compatibility.

Slice D — extract fixing/frontmatter mutation orchestration

Separate mutation/fixer eligibility and YAML repair orchestration from validation dispatch where the existing behavior permits a clean boundary.

Slice E — extract validation orchestration

Move validator selection/collection and validate_file / validate_single_path ownership into a focused validation module while keeping the compatibility facade.

Slice F — extract CLI wiring

Make the Typer application/commands depend on the smaller domain seams rather than making the domain depend on the CLI module.

Slice G — retire compatibility exports only when proven safe

Do not remove legacy imports merely because internal code no longer needs them. Removal requires an explicit compatibility decision and release treatment.

Architectural constraints

• Preserve existing rule, adapter, schema, scan-runtime and reporting seams where they already work.
• Do not move Add opt-in instruction-context static analysis backed by shared host semantics #279 instruction-context implementation into plugin_validator.py; it should enter through an explicit analysis subsystem.
• Avoid moving code merely to reduce line count. Each extracted module must have a coherent reason to change.
• Do not create cycles hidden behind local imports as the final architecture.
• Measure CLI import/startup effects because perf(tests): suite spends ~two thirds of its wall clock on subprocess startup #148 shows startup cost materially affects the test suite and user/agent loop.

Acceptance criteria

• A documented import/dependency inventory exists before broad extraction.
• Shared validation contracts no longer originate from the central validator module.
• Rule modules do not require the legacy module merely to name core result/domain types.
• Policy, fixing, validation orchestration, and CLI ownership are individually identifiable.
• Existing required skilllint.plugin_validator imports continue working through compatibility exports during migration.
• Public CLI behavior and exit semantics remain covered end to end.
• No new circular-import workaround becomes a permanent architecture.
• Import/startup performance is measured before and after material extraction steps.
• The final module is primarily compatibility/wiring rather than the default implementation home for unrelated features.