-
Notifications
You must be signed in to change notification settings - Fork 161
Refactor: separate package-format detection from classification and normalization strategy #782
Copy link
Copy link
Open
Labels
area/cliCLI command surface, flags, help text (cross-cutting).CLI command surface, flags, help text (cross-cutting).area/testingTest infrastructure, fixtures, e2e harness, coverage.Test infrastructure, fixtures, e2e harness, coverage.priority/lowAccepted but not time-sensitiveAccepted but not time-sensitivestatus/acceptedDirection approved, safe to start work.Direction approved, safe to start work.status/triagedInitial agentic triage complete; pending maintainer ratification (silence = approval).Initial agentic triage complete; pending maintainer ratification (silence = approval).type/refactorInternal restructure, no behavior change.Internal restructure, no behavior change.
Milestone
Metadata
Metadata
Assignees
Labels
area/cliCLI command surface, flags, help text (cross-cutting).CLI command surface, flags, help text (cross-cutting).area/testingTest infrastructure, fixtures, e2e harness, coverage.Test infrastructure, fixtures, e2e harness, coverage.priority/lowAccepted but not time-sensitiveAccepted but not time-sensitivestatus/acceptedDirection approved, safe to start work.Direction approved, safe to start work.status/triagedInitial agentic triage complete; pending maintainer ratification (silence = approval).Initial agentic triage complete; pending maintainer ratification (silence = approval).type/refactorInternal restructure, no behavior change.Internal restructure, no behavior change.
Type
Projects
Status
Todo
Follow-up from #781 (which fixed the immediate ordering bug behind #780).
Problem
detect_package_type()insrc/apm_cli/models/validation.pyis a positional-priority if/elif cascade that conflates three concerns:apm.yml,SKILL.md,hooks/*.json,plugin.json,agents/skills/commands/?PackageTypeenum value to assign.normalize_plugin_directoryforMARKETPLACE_PLUGIN).The cascade's first-match-wins semantics means every new bundled-format introduces a potential ordering bug. #780 was the latest one; the next one is just a matter of time.
Proposed shape
Move to a composition model:
FormatDetectorinterface, one impl per format:ApmYmlDetector,SkillMdDetector,HookJsonDetector,ClaudePluginDetector. Each returnsOptional[FormatEvidence]independent of the others.PackageFormatRegistryruns all detectors and produces aDetectionReport(set of evidences -- not a singlePackageType).NormalizationPlannerreads the report and decides which normalizers to run. Mixed packages become first-class (e.g. a Claude plugin that legitimately ships hooks runs both the plugin synthesizer AND the hooks pass-through, instead of being shoehorned into one classification).The
DetectionEvidencedataclass +gather_detection_evidence()helper landed in #781 are intentionally a step in this direction -- they decouple the evidence scan from classification so observability code can already use the richer payload.Benefits
MARKETPLACE_PLUGINhappens to also handle hooks" becomes explicit).DetectionReportdoubles as the observability payload (verbose detection traces, deploy-summary labels, near-miss warnings).Cost
Medium -- touches
models/validation.py,install/sources.py, plus all consumers ofPackageType(lockfile recording, install summary, drift detection). Should land behind a feature flag for one release if downstream test churn is high.Out of scope for this issue
_has_hook_jsonto only count files matching the actual Claude hooks schema (today: any*.jsoninhooks/).MIXEDPackageType-- theDetectionReportmodel makes the enum redundant for mixed cases.Related