fix(deps): normalize case-insensitive package identity#2098
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR makes GitHub and registry owner/repo package identities case-insensitive by normalizing them to lowercase at a shared model boundary, ensuring consistent lock/cache keys, canonical strings, publish API paths, and install directories while preserving casing for unknown git hosts.
Changes:
- Introduces a shared
normalize_package_repo_url()helper and applies it in dependency reference and lockfile models. - Canonicalizes
apm publish --packageparsing to align publish identity with install identity. - Adds/updates regression tests and updates docs to document the normalization behavior.
Show a summary per file
| File | Description |
|---|---|
src/apm_cli/models/dependency/identity.py |
Adds shared normalization helper for repo identity casing. |
src/apm_cli/models/dependency/reference.py |
Normalizes repo_url in DependencyReference.__post_init__ so downstream identity consumers are consistent. |
src/apm_cli/deps/lockfile.py |
Normalizes LockedDependency.repo_url at construction/load time to converge legacy mixed-case entries. |
src/apm_cli/commands/publish.py |
Ensures --package IDs are normalized before constructing publish paths. |
tests/test_apm_package_models.py |
Adds regression tests covering identity/key/install-path convergence and unknown-host case preservation. |
tests/test_lockfile.py |
Adds regression test proving mixed-case GitHub lock entries normalize on load/serialize. |
tests/unit/commands/test_publish_registry_archive.py |
Adds test ensuring publish package IDs normalize to lowercase. |
tests/unit/integration/test_skill_integrator.py |
Updates expected integration diagnostic package key to the normalized form. |
docs/src/content/docs/consumer/manage-dependencies.md |
Documents new normalization behavior and unknown-host exception. |
packages/apm-guide/.apm/skills/apm-usage/dependencies.md |
Syncs bundled dependency guidance with the normalization behavior. |
CHANGELOG.md |
Adds an Unreleased “Fixed” entry describing the behavior change. |
Review details
- Files reviewed: 11/11 changed files
- Comments generated: 1
- Review effort level: Low
| - GitHub and package-registry references now normalize owner/repository casing | ||
| before deriving identity, lock/cache keys, canonical strings, publish paths, | ||
| and install directories, while unknown git hosts retain case-sensitive paths. | ||
| (closes #2073) |
Add integration-level install, resolution, collision, install-path, and lockfile evidence while documenting the canonical identity boundary and publish behavior. Addresses review-panel follow-ups for PR #2098. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
APM Review Panel:
|
| Persona | B | R | N | Takeaway |
|---|---|---|---|---|
| Python Architect | 0 | 0 | 0 | One normalization boundary now owns package casing. |
| CLI Logging Expert | 0 | 0 | 0 | No output regression; canonical names remain clear. |
| DevX UX Expert | 0 | 0 | 0 | Mixed-case references now behave as one package. |
| Supply Chain Security Expert | 0 | 0 | 0 | Identity confusion is closed for the stated hosts. |
| OSS Growth Hacker | 0 | 0 | 0 | The changelog now leads with the user-visible fix. |
| Auth Expert | 0 | 0 | 0 | Auth and token resolution remain unchanged. |
| Doc Writer | 0 | 0 | 0 | Install, lockfile, publish, and bundled guidance agree. |
| Test Coverage Expert | 0 | 0 | 0 | Unit and integration evidence cover the critical promise. |
| Performance Expert | 0 | 0 | 0 | Constant-time string normalization is negligible and improves cache convergence. |
B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.
Folded in this run
- (panel) Added an empirical install/resolution integration test covering deduplication, zero collisions, one lowercase install path, and lockfile roundtrip -- resolved in
926a13fe1287d400e1ed224b34e2a6410137e798. - (panel) Documented lockfile and publish normalization in the Starlight references and bundled APM usage guide -- resolved in
926a13fe1287d400e1ed224b34e2a6410137e798. - (panel) Documented the single casing-normalization boundary and removed a redundant default-host lookup -- resolved in
926a13fe1287d400e1ed224b34e2a6410137e798. - (panel) Reframed the changelog around duplicate identity, lock/cache, and install-path prevention -- resolved in
926a13fe1287d400e1ed224b34e2a6410137e798.
Copilot signals reviewed
Copilot produced no inline findings on the final revision.
Regression-trap evidence (mutation-break gate)
tests/integration/test_package_identity_case_install.py::test_mixed_case_install_resolves_once_without_collision-- removed the GitHub lowercase return innormalize_package_repo_url; the test failed because two case variants survived as distinct installs; the guard was restored and the test passed.
Lint contract
Ruff check and format, YAML I/O safety, file length, portable relative paths, pylint R0801, and auth-signal checks all passed before push. The full unit suite also passed.
CI
All checks passed on exact head 926a13fe1287d400e1ed224b34e2a6410137e798: https://github.com/microsoft/apm/actions/runs/29063285281
Mergeability status
| PR | head SHA | CEO stance | iters | folds | defers | Copilot rounds | CI | mergeable | mergeStateStatus | notes |
|---|---|---|---|---|---|---|---|---|---|---|
| #2098 | 926a13f |
ship_now | 1 | 4 | 0 | 1 | green | MERGEABLE | BLOCKED | awaiting required review |
Recommendation
All in-scope findings are folded, the mutation is killed, and exact-head CI is green. Ready for maintainer review.
Full per-persona findings
The final pass found no unresolved in-scope findings across architecture, CLI logging, DevX, supply-chain security, growth, auth, documentation, test coverage, or performance.
This panel is advisory. Re-apply the panel-review label after addressing feedback to re-run.
fix(deps): normalize case-insensitive package identity
TL;DR
GitHub and package-registry owner/repository paths now become lowercase at a shared identity boundary. Mixed-case references therefore share canonical strings, lock/cache keys, publish paths, and install directories, while unknown git hosts retain their original path casing.
Note
This closes #2073 and automatically rewrites legacy mixed-case GitHub lock entries on the next lockfile emission.
Problem (WHY)
Owner/example-packageandowner/example-packagepreviously produced distinct identities, lock/cache keys, andapm_modules/paths, as reproduced in [BUG] Packages are case sensitive but they should not be #2073.apm publish --package Owner/Repoaddressed a different case-sensitive registry URL path than install used for the same logical package.The regression tests turn the issue contract into executable evidence: "Grounding outputs in deterministic tool execution transforms probabilistic generation into verifiable action."
Approach (WHAT)
Implementation (HOW)
src/apm_cli/models/dependency/identity.pysrc/apm_cli/models/dependency/reference.pyDependencyReference.__post_init__, before canonical, dedup, cache, display, and install-path consumers run.src/apm_cli/deps/lockfile.pyLockedDependencyvalues, migrating mixed-case GitHub lock entries on serialization.src/apm_cli/commands/publish.pyOWNER/REPObefore constructing publish API paths.tests/test_apm_package_models.pytests/test_lockfile.pytests/unit/commands/test_publish_registry_archive.pytests/unit/integration/test_skill_integrator.pydocs/src/content/docs/consumer/manage-dependencies.mdpackages/apm-guide/.apm/skills/apm-usage/dependencies.mdCHANGELOG.mdDiagrams
Legend: the dashed node is the new shared boundary; downstream consumers only receive its canonical result.
flowchart LR subgraph Inputs[Package identity inputs] I1[DependencyReference] I2[LockedDependency] I3[Registry publish ID] end subgraph Normalize[Identity boundary] N1[normalize_package_repo_url] N2{GitHub or registry?} end subgraph Outputs[Identity consumers] O1[Canonical string] O2[Lock and cache key] O3[Install and publish path] end I1 --> N1 I2 --> N1 I3 --> N1 N1 --> N2 N2 -->|yes| L1[Lowercase owner and repo] N2 -->|no| P1[Preserve repository path case] L1 --> O1 L1 --> O2 L1 --> O3 P1 --> O1 P1 --> O2 P1 --> O3 classDef new stroke-dasharray: 5 5; class N1 new;Trade-offs
Benefits
apm_modules/owner/repodirectory.Validation
uv run --extra dev pytest <six casing regression tests> -q:Mutation-break proof (replace both normalization returns with the unmodified input, then run the five normalization traps):
Full unit suite and CI lint mirror
uv run --extra dev pytest tests/unit -q --tb=short:CI lint mirror:
Scenario Evidence
Owner/Repoandowner/repocreates one package identity, lock/cache key, and directory.tests/test_apm_package_models.py::TestDependencyReference::test_github_owner_repo_casing_has_one_identity::test_github_owner_repo_casing_has_one_dedup_key::test_github_owner_repo_casing_has_one_install_path(regression-trap for #2073)tests/test_lockfile.py::TestLockedDependency::test_github_owner_repo_casing_migrates_to_one_lock_keytests/unit/commands/test_publish_registry_archive.py::TestResolvePackageId::test_package_identity_is_lowercasetests/test_apm_package_models.py::TestDependencyReference::test_generic_git_host_preserves_case_sensitive_repo_pathtests/unit/integration/test_skill_integrator.py::TestNativeSkillIntegration::test_native_skill_collision_diagnostic_package_is_current_keyHow to test
6 passed.uv run --extra dev pytest tests/unit -q; expect all unit tests to pass.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com