feat(cli): lockfileVersion 3 and qualified install paths (#130)

[maiconfz]

Summary

• lockfileVersion 3 with per-package pathEncodingVersion
• Extract/remove from qualified-leaf deployment ZIPs (no install-time rewrite)
• doctor: legacy encoding + cross-package path collision checks

Related Issues

Closes #130
Depends on agents-repo/registry#230

Test plan

• npm run env:check && npm run lint:all && npm run test:run && npm run typecheck
• Integration: two packages, same source agent id, no on-disk collision

Made with Cursor

[maiconfz]

Pre-ready validation evidence

• npm run lint:all — pass (1 sonar complexity warning in doctorPathChecks)
• npm run typecheck — pass
• npm run test — 475 passed (includes collision integration: two packages, shared source agent id)
• Specs: lock-schema.md, command-contracts.md
• Migration: docs/path-encoding-migration.md

Depends on agents-repo/registry#231

Closes #130

[Copilot]

🟡 Changes recommended

doctor’s legacy encoding check currently only detects legacy skill paths (missing legacy Claude flat paths), and the new doctor checks re-download artifacts independently, which can significantly increase runtime for larger installs.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR updates the CLI install/lock workflow to support lockfileVersion 3 with a per-package pathEncodingVersion that tracks qualified install-leaf deployment paths, and extends doctor to detect legacy path encoding and cross-package install path collisions.

Changes:

• Bump lock schema to lockfileVersion 3, adding optional per-package pathEncodingVersion (persist + parse/serialize + specs/tests).
• Accept qualified deployment ZIP layouts (no install-time rewrite) and persist pathEncodingVersion from manifest or inferred ZIP entry layout.
• Add doctor checks for legacy_path_encoding and agent_path_collision, plus migration/docs updates and broader test fixture updates (including a collision integration test).

File summaries

File	Description
tests/modules/install/packageRemover.test.ts	Updates expected extracted paths/content to qualified install-leaf layout.
tests/modules/install/packageExtractor.test.ts	Updates ZIP entry mapping/extraction assertions for qualified paths and new filenames.
tests/modules/install/installService.test.ts	Updates install flow assertions for qualified skill output paths/content.
tests/modules/install/installPersistenceRollback.test.ts	Updates rollback assertions to new qualified extract paths.
tests/modules/install/ciInstallService.test.ts	Updates CI install assertions for qualified skill/agent paths and content.
tests/modules/install/bulkInstallService.test.ts	Adds collision test and updates expectations for lockfileVersion 3 + qualified paths.
tests/modules/config/lockFileService.test.ts	Adjusts lock read/write expectations to support v2+v3 and v3 pathEncodingVersion.
tests/modules/cli/removeCommand.test.ts	Updates CLI subprocess remove/unlink tests to qualified path layout.
tests/modules/cli/installCommand.test.ts	Updates CLI subprocess install tests (including global scope) to qualified path layout.
tests/fixtures/installFixtures.ts	Updates fixtures to emit qualified ZIPs and manifests including pathEncoding.
src/modules/registry/domain/manifest.ts	Extends manifest artifact type with optional pathEncoding.
src/modules/install/infrastructure/zipSecurityScanner.ts	Extends ZIP validation to allow qualified skill/Claude entry layouts (while keeping legacy support).
src/modules/install/domain/pathEncoding.ts	Introduces path encoding constants + inference helpers for ZIP entry layouts.
src/modules/install/application/installPersistence.ts	Persists per-package pathEncodingVersion into the lock entry.
src/modules/install/application/bulkInstallService.ts	Captures mapped ZIP entry list for persistence inference.
src/modules/config/infrastructure/agentsLockRepository.ts	Serializes lock entries according to the document’s lockfileVersion.
src/modules/config/domain/packageLockEntry.ts	Adds pathEncodingVersion to lock entries and v3-aware parse/serialize rules.
src/modules/config/domain/configConstants.ts	Sets default lockfile version to 3 and supports reading 2+3.
src/modules/config/application/lockFileService.ts	Reads lockfileVersion 2 or 3; serializes entries based on lockfileVersion.
src/modules/config/application/doctorService.ts	Adds two new doctor checks and wires them into the doctor run.
src/modules/config/application/doctorPathChecks.ts	Implements legacy encoding detection and cross-package collision detection by enumerating ZIP-mapped paths.
specs/lock-schema.md	Documents lockfileVersion 3 and pathEncodingVersion rules.
specs/command-contracts.md	Updates doctor contract to include the two new checks and v3 lock presence.
docs/path-encoding-migration.md	Adds migration guidance for mixed legacy/new artifacts and how doctor helps.
docs/commands/doctor.md	Documents the new doctor checks.

Review details

• Files reviewed: 25/25 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

One change introduces an import-order issue likely to break linting, and there’s an identified completeness gap around single-install lock encoding inference that should be addressed or explicitly justified.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

tests/modules/install/bulkInstallService.test.ts:213

• There’s coverage for greenfield lockfileVersion: 3 creation, but no test asserting migration behavior when an existing lockfileVersion: 2 lock is present (e.g., running bulk install upgrades the document to v3 while preserving entries and adding pathEncodingVersion when applicable). Adding an explicit migration test would reduce the risk of regressions during the v2→v3 rollout.

src/modules/install/application/installPersistence.ts:23

• InstallPersistence.save() only passes artifact.pathEncoding into resolveLockPathEncodingVersion, so single-package installs cannot fall back to ZIP-shape inference when the manifest omits pathEncoding. Bulk installs and doctor already compute ZIP entry names, so consider extending InstallPersistenceInput (and its callers) to optionally supply zipEntryNames for consistent lock annotation.

export interface InstallPersistenceInput {
  readonly resolved: ResolvedAgentsConfig
  readonly packageId: string
  readonly version: string
  readonly target: InstallTargetId
  readonly artifact: ManifestArtifact
  readonly resolvedRef: string
  readonly adHocInstall: boolean
}

• Files reviewed: 26/26 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

There are a few correctness/contract gaps (notably around manifest pathEncoding handling) and a measurable doctor performance regression due to redundant ZIP re-processing that should be addressed before approval.

Review details

Suppressed comments (3)

Previously missed (3) — in code that hasn't changed since the last review.

src/modules/install/domain/pathEncoding.ts:16

• computeInstallLeaf takes a parameter named packageId, but it’s actually the package name/slug (the middle segment in namespace-package-source). Using packageId here is ambiguous and makes it easy to accidentally pass a fully-qualified id like acme/alpha, which would produce an invalid leaf (includes /) and break path patterns.
  src/modules/install/domain/pathEncoding.ts:26
• resolvePathEncodingVersionFromManifest silently treats any non-1 pathEncoding value as “unset”. If the registry accidentally ships an unsupported value, the CLI will fall back to legacy behavior rather than failing fast, which can mask a bad publish and potentially reintroduce path-collision risk.
  src/modules/config/application/doctorService.ts:195
• verifyInstallPathsFromArtifacts re-opens and re-scans each ZIP via planArtifactExtractFromZip, even though loadLockSlotArtifacts already scanned the ZIP and produced mappedPaths. This duplicates CPU work (ZIP scan + entry iteration + hashing) and will make doctor noticeably slower on large installs.

• Files reviewed: 26/26 changed files
• Comments generated: 0 new
• Review effort level: Lite

[maiconfz]

Re: Copilot review (5175299023): Fixed in 0a82147:

• Renamed computeInstallLeaf's middle segment parameter from packageId to packageName to clarify it is the package slug, not a fully-qualified id.
• resolvePathEncodingVersionFromManifest now throws InstallRuntimeError for unsupported pathEncoding values instead of silently falling back.
• verifyInstallPathsFromArtifacts reuses precomputed mappedPaths from loadLockSlotArtifacts instead of re-opening and re-scanning each ZIP.

[Copilot]

🔵 Needs a closer look

It changes lockfile schema, install path layout, and doctor behavior across multiple subsystems, warranting careful human validation of migration and edge cases.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

src/modules/config/domain/packageLockEntry.ts:157

• The error message for an unsupported pathEncodingVersion does not include the actual value found or the expected value, which makes lockfile debugging harder.

• Files reviewed: 26/26 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🔵 Needs a closer look

doctor currently retains full artifact ZIP buffers in memory and can overcount missing install paths due to duplicates, which risks unnecessary memory pressure and inaccurate diagnostics.

Review details

Suppressed comments (3)

Previously missed (2) — in code that hasn't changed since the last review.

src/modules/config/application/doctorPathChecks.ts:38

• LockSlotArtifact stores the full downloaded ZIP buffer (zipBytes) but none of the downstream checks use it (they only use mappedPaths). Keeping these buffers for the whole doctor run can significantly increase peak memory usage for large installs.

This issue also appears on line 91 of the same file.
src/modules/config/application/doctorService.ts:198

• missingPaths can include duplicate absolute paths (e.g. if two artifacts map to the same relative path), inflating the reported count and repeating paths in the preview. Dedupe before pushing to keep the error message accurate.

src/modules/config/application/doctorPathChecks.ts:95

• After removing zipBytes from LockSlotArtifact, the pushed artifact object should also drop the zipBytes field to avoid retaining large buffers unnecessarily.

        target,
        version: lockEntry.version,
        zipBytes,
        mappedPaths,
      })

• Files reviewed: 26/26 changed files
• Comments generated: 0 new
• Review effort level: Lite

[maiconfz]

Re: Copilot review (5175379983): Fixed in 34dceb6:

• Removed zipBytes from LockSlotArtifact so doctor no longer retains full artifact ZIP buffers after path mapping.
• Deduplicated missing install paths in verifyInstallPathsFromArtifacts so duplicate relative mappings do not inflate the reported count or preview.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[Copilot]

🔵 Needs a closer look

It changes lock schema semantics and install/doctor behavior across multiple modules, so a final human review should validate end-to-end compatibility and migration scenarios.

Review details

• Files reviewed: 26/26 changed files
• Comments generated: 0 new
• Review effort level: Lite

[agents-repo-cli-release]

🎉 This PR is included in version 1.22.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀