fix(cli): sync alias data loss, remove --dry-run side effects, cache clean guard

[Kamirus]

mops sync could silently delete a dependency. Given the pinned-alias layout the docs recommend:

map = "9.0.1"
"map@8.1.0" = "8.1.0"

sync built its "used" set from imports (mo:map@8.1.0/Map → map@8.1.0) but its "declared" set through getDepName, which collapses both keys to map. The two sets lived in different namespaces, so the alias was reported as both missing and unused. Adding it split on @ and wrote map = "8.1.0", clobbering 9.0.1; and because both sets are computed before any mutation, one run could add and then remove, leaving the project with no map at all.

$ mops sync
Before: map = "8.1.0"        # 9.0.1 overwritten, alias key gone
After:  map = "9.0.1"        # unchanged
        "map@8.1.0" = "8.1.0"

It evaded detection because mops sync has no test coverage at all — cli.test.ts notes that add/remove/update/sync "all route through the same checkIntegrity code path tested above", which is true of the lock write but not of the set arithmetic that decides what to add and remove.

Three smaller sync defects came out of covering it: packages imported only from test/bench sources were added to [dependencies]; a package declared in both sections was only ever removed from [dependencies], leaving a dangling entry the next run reported again; and moc --print-deps was spawned twice per file because the used-set was recomputed for the missing and unused queries independently.

mops remove --dry-run was not a dry run

It deleted local cache directories, and ran checkIntegrity, which rewrites mops.lock whenever it was already stale. It also printed Package removed … as though it had happened.

mops cache clean was broken on Windows, always

The safety guard compared a path.join result against the literal suffix mops/cache. On Windows path.join yields backslashes, so the guard could never match and the command failed with "Invalid cache directory" on every run. Network-scoped directories (mops/cache/<network>) matched no clause either.

The replacement is separator-agnostic and strictly narrower than what it replaces: it also requires the target to be contained in the global cache root, so a traversing MOPS_NETWORK is rejected rather than accepted on a suffix match. Separately, when run outside a project getRootDir() returns empty and path.join("", ".mops") resolved to ./.mops in whatever directory the user happened to be standing in — that local delete is now skipped.

Migration

mops cache clean still removes the project's .mops as documented; the new --global opts out. No other behaviour changes for existing invocations.

Scope

First of three stacked PRs splitting the Cargo/pnpm audit work, which was originally opened as #724. This one is the self-contained command fixes and touches no install, resolution or integrity code. Remaining audit findings are catalogued in #723.

🤖 Generated with Claude Code

[github-actions]

Cursor AI review

👍 APPROVE — looks safe to merge

Verified the three command fixes against base behavior, add(..., asName) (same path as update.ts), installMopsDep/getDepName for aliases, and the new tests/docs/changelog/skills sync.

Category	Assessment	Details
Summary	✅	Fixes mops sync pinned-alias set arithmetic (and related prod/dev classification), makes mops remove --dry-run side-effect free, and hardens mops cache clean path guards; adds --dry-run / --global plus docs, changelog, skills, and tests.
Code Quality	✅	Reuses add's existing asName (same pattern as update.ts); extracts computeSyncPlan / parseImportedPackage / assertGlobalCacheDir; drops unused addCache (no remaining callers).
Consistency	✅	Dry-run wording matches other CLI dry-runs; sibling docs (05-mops-sync.md, 03-mops-cache.md), cli/CHANGELOG.md ## Next, --help options in cli/cli.ts, and .agents/skills/mops-cli/SKILL.md updated together.
Security	✅	Traced assertGlobalCacheDir containment + network-segment stripping vs old suffix endsWith; dry-run now returns before writeConfig / syncLocalCache / checkIntegrity in remove.ts; local .mops delete skipped when getRootDir() is empty.
Tests	✅	Alias/dev plan covered in sync-plan.test.ts; cache guard + --global in cache-clean.test.ts; remove dry-run disk/lock assertions in remove-dry-run.test.ts (targeted toMatch/toBe, appropriate for these paths).
Maintainability	✅	Planning logic is pure and unit-tested; cache safety is named and separator-agnostic; changelog documents the user-visible fixes clearly.

Verdict

Decision: APPROVE
Risk: Low
Reason: Contained CLI bugfixes outside install/resolve/integrity/backend; alias and dry-run/cache guards match intended behavior and are backed by focused tests, docs, and changelog.

---

Generated for commit f677966

[automation-sa-sre]

Automated approval: the AI review verdict for f677966 is APPROVE. See the "Cursor AI review" comment for details.