diff --git a/CLAUDE.md b/CLAUDE.md index 29cfbeba..13b17e3b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -22,7 +22,6 @@ mcs sync --dry-run # Preview what would change mcs sync --customize # Per-pack component selection mcs sync --global # Sync global scope (MCP servers, brew, plugins to ~/.claude/) mcs sync --lock # Checkout locked versions from mcs.lock.yaml -mcs sync --update # [DEPRECATED — use 'mcs update'] Fetch latest and force-write mcs.lock.yaml mcs update # Fetch latest pack versions and re-apply across every configured scope mcs update --global # Refresh only the global scope mcs update --project # Refresh only the current project's scope @@ -119,7 +118,7 @@ mcs config set # Set a configuration value (true/false) - `SectionValidator.swift` — validation of CLAUDE.local.md section markers ### Commands (`Sources/mcs/Commands/`) -- `SyncCommand.swift` — primary command (`mcs sync`), handles both project-scoped and global-scoped sync with `--pack`, `--all`, `--dry-run`, `--customize`, `--global`, `--lock`, `--update` flags. `guardClaudeHomeCwd()` at the top of `perform()` rejects/redirects runs from `~/.claude` or `$HOME` to prevent silent corruption of the global scope +- `SyncCommand.swift` — primary command (`mcs sync`), handles both project-scoped and global-scoped sync with `--pack`, `--all`, `--dry-run`, `--customize`, `--global`, `--lock` flags. `guardClaudeHomeCwd()` at the top of `perform()` rejects/redirects runs from `~/.claude` or `$HOME` to prevent silent corruption of the global scope - `DoctorCommand.swift` — health checks with optional --fix and --pack filter - `CleanupCommand.swift` — backup file management with --force flag - `PackCommand.swift` — `mcs pack add/remove/list/update/validate` subcommands; uses `PackSourceResolver` for 3-tier input detection (URL schemes → filesystem paths → GitHub shorthand) @@ -203,7 +202,7 @@ swiftlint --fix - **Backup for mixed-ownership files**: timestamped backup before modifying files with user content (CLAUDE.local.md); tool-managed files are not backed up since they can be regenerated - **Component-derived doctor checks**: `ComponentDefinition` is the single source of truth — `deriveDoctorCheck()` auto-generates verification from `installAction`, supplementary checks handle extras - **Project awareness**: doctor detects project root (walk-up for `.git/`), resolves packs from `.claude/.mcs-project` before falling back to section marker inference, then to global manifest -- **Lockfile support (opt-in)**: `mcs.lock.yaml` pins pack commits for reproducible builds. Generation is opt-in — enable with `mcs config set generate-lockfile true` to write on every sync, or pass `--update` to fetch latest and write once. `--lock` checks out pinned commits from an existing lockfile. Tri-state semantics on `generate-lockfile`: `true` writes, `false` is silent (explicit opt-out), `nil` (never configured) surfaces a one-time drift warning if a stale lockfile exists — the upgrade nudge +- **Lockfile support (opt-in)**: `mcs.lock.yaml` pins pack commits for reproducible builds. Generation is opt-in — enable with `mcs config set generate-lockfile true` to write on every sync. `--lock` checks out pinned commits from an existing lockfile. Tri-state semantics on `generate-lockfile`: `true` writes, `false` is silent (explicit opt-out), `nil` (never configured) surfaces a one-time drift warning if a stale lockfile exists — the upgrade nudge - **Local packs**: `mcs pack add /path` registers a pack read in-place — no git clone, no `mcs pack update`, no directory deletion on remove. Uses `isLocal: Bool?` on `PackEntry` (backward-compatible) and `commitSHA: "local"` sentinel. Trust verification is skipped since scripts change during development - **GitHub shorthand**: `mcs pack add user/repo` expands to `https://github.com/user/repo.git`. Filesystem paths are checked before shorthand regex to prevent ambiguity with relative paths like `org/pack` - **Cross-project reference counting**: `ProjectIndex` (`~/.mcs/projects.yaml`) tracks which projects use which packs; `ResourceRefCounter` checks all scopes before removing shared brew packages or plugins. Conservative by default — if state is unreadable, assume resource is still needed. MCP servers are project-independent (scoped via `-s local`) and skip ref counting diff --git a/Sources/mcs/Commands/SyncCommand.swift b/Sources/mcs/Commands/SyncCommand.swift index 2e15895a..aca45892 100644 --- a/Sources/mcs/Commands/SyncCommand.swift +++ b/Sources/mcs/Commands/SyncCommand.swift @@ -22,9 +22,6 @@ struct SyncCommand: LockedCommand { @Flag(name: .long, help: "Checkout locked pack versions from mcs.lock.yaml before syncing") var lock = false - @Flag(name: .long, help: "Fetch latest pack versions and update mcs.lock.yaml") - var update = false - @Flag(name: .long, help: "Customize which components to include per pack") var customize = false @@ -51,20 +48,6 @@ struct SyncCommand: LockedCommand { // First-run: prompt for update notification preference let config = promptForUpdateCheckIfNeeded(env: env, output: output) - // Handle --update: fetch latest for all packs before loading - if update { - output.warn( - "'mcs sync --update' is deprecated. Use 'mcs update' to fetch and re-apply across all configured scopes; " - + "combine with 'mcs config set generate-lockfile true' if you want a lockfile." - ) - let lockOps = LockfileOperations(environment: env, output: output, shell: shell) - // On a non-interactive hard failure this throws before the sync phase below, so - // healthy packs are fetched (and their SHAs saved) but not re-applied this run — - // unlike `mcs update`, which re-applies first and signals failure last. Accepted - // for this deprecated path; `mcs update` is the supported fetch-and-apply command. - try lockOps.updatePacks() - } - let registry = TechPackRegistry.loadWithExternalPacks( environment: env, output: output @@ -183,7 +166,7 @@ struct SyncCommand: LockedCommand { try configurator.interactiveConfigure(dryRun: dryRun, customize: customize) } - switch Self.lockfileAction(dryRun: dryRun, update: update, config: config) { + switch Self.lockfileAction(dryRun: dryRun, config: config) { case .write: try lockOps.writeLockfile(at: projectPath) case .reportDrift: @@ -203,9 +186,9 @@ struct SyncCommand: LockedCommand { case skip } - static func lockfileAction(dryRun: Bool, update: Bool, config: MCSConfig) -> LockfileAction { + static func lockfileAction(dryRun: Bool, config: MCSConfig) -> LockfileAction { guard !dryRun else { return .skip } - if update || config.isLockfileGenerationEnabled { return .write } + if config.isLockfileGenerationEnabled { return .write } if config.isLockfileGenerationUnset { return .reportDrift } return .skip } diff --git a/Sources/mcs/Commands/UpdateCommand.swift b/Sources/mcs/Commands/UpdateCommand.swift index baca35b7..43a3e2f7 100644 --- a/Sources/mcs/Commands/UpdateCommand.swift +++ b/Sources/mcs/Commands/UpdateCommand.swift @@ -4,7 +4,7 @@ import Foundation /// Refresh-only orchestration: fetch latest pack contents (with trust verification), /// then re-apply the existing configured set in both the global scope and the current /// project's scope. Does not add or remove packs (use `mcs sync`). Lockfile writes are -/// gated by `generate-lockfile`, unlike `mcs sync --update` which force-writes. +/// gated by `generate-lockfile`. struct UpdateCommand: LockedCommand { static let configuration = CommandConfiguration( commandName: "update", diff --git a/Sources/mcs/Sync/LockfileOperations.swift b/Sources/mcs/Sync/LockfileOperations.swift index a8925ab6..f28a0560 100644 --- a/Sources/mcs/Sync/LockfileOperations.swift +++ b/Sources/mcs/Sync/LockfileOperations.swift @@ -80,68 +80,6 @@ struct LockfileOperations { } } - /// Fetch latest commits for all registered git packs. Local packs are skipped. - /// Re-validates trust when scripts change (mirrors `mcs pack update` behavior). - func updatePacks() throws { - let registryFile = PackRegistryFile(path: environment.packsRegistry) - let registryData = try registryFile.load() - - if registryData.packs.isEmpty { - output.info("No packs registered. Nothing to update.") - return - } - - output.info("Fetching latest pack commits...") - let updater = PackUpdater( - fetcher: PackFetcher(shell: shell, output: output, packsDirectory: environment.packsDirectory), - trustManager: PackTrustManager(output: output), - environment: environment, - output: output - ) - - var updatedData = registryData - var attemptedCount = 0 - var failedPacks: [String] = [] - for entry in registryData.packs { - if entry.isLocalPack { - output.dimmed(" \(entry.identifier): local pack (skipped)") - continue - } - - attemptedCount += 1 - guard let packPath = entry.resolvedPath(packsDirectory: environment.packsDirectory) else { - output.warn(" \(entry.identifier): invalid path — skipping") - failedPacks.append(entry.identifier) - continue - } - - let result = updater.updateGitPack(entry: entry, packPath: packPath, registry: registryFile) - switch result { - case .alreadyUpToDate: - output.dimmed(" \(entry.identifier): already up to date") - case let .updated(updatedEntry): - registryFile.register(updatedEntry, in: &updatedData) - output.success(" \(entry.identifier): updated (\(updatedEntry.shortSHA))") - case .trustDeclined: - output.info(" \(entry.identifier): \(result.reason ?? "trust not granted") (will re-prompt next run)") - case .fetchFailed, .manifestInvalid, .internalError: - output.warn(" \(entry.identifier): \(result.reason ?? "update failed")") - failedPacks.append(entry.identifier) - } - } - - try registryFile.save(updatedData) - - if PackUpdater.shouldExitNonZero( - failedCount: failedPacks.count, - attemptedCount: attemptedCount, - isInteractive: output.hasInteractiveStdin - ) { - output.error("Failed to update: \(failedPacks.joined(separator: ", "))") - throw ExitCode.failure - } - } - /// Write the lockfile after a successful sync. func writeLockfile(at projectPath: URL) throws { let registryFile = PackRegistryFile(path: environment.packsRegistry) diff --git a/Sources/mcs/Sync/PackUpdater.swift b/Sources/mcs/Sync/PackUpdater.swift index 091c5b81..3597dc67 100644 --- a/Sources/mcs/Sync/PackUpdater.swift +++ b/Sources/mcs/Sync/PackUpdater.swift @@ -1,7 +1,7 @@ import Foundation /// Handles the fetch → validate → trust cycle for updating a single git pack. -/// Used by both `UpdatePack` (interactive) and `LockfileOperations` (`--update`). +/// Used by both `UpdatePack` (interactive) and `UpdateCommand` (multi-scope refresh). struct PackUpdater { let fetcher: PackFetcher let trustManager: PackTrustManager @@ -135,9 +135,9 @@ struct PackUpdater { extension PackUpdater { /// Exit-code policy shared by every multi-pack update caller (`mcs pack update`, - /// `mcs sync --update`, `mcs update`): a hard failure exits non-zero when running - /// non-interactively (CI), or when every attempted pack failed. A lone trust-decline is - /// not a failure. Centralized so the three callers can't drift out of contract. + /// `mcs update`): a hard failure exits non-zero when running non-interactively (CI), or + /// when every attempted pack failed. A lone trust-decline is not a failure. Centralized so + /// the callers can't drift out of contract. static func shouldExitNonZero(failedCount: Int, attemptedCount: Int, isInteractive: Bool) -> Bool { failedCount > 0 && (!isInteractive || failedCount == attemptedCount) } diff --git a/Tests/MCSTests/LockfileOperationsTests.swift b/Tests/MCSTests/LockfileOperationsTests.swift index 605c8768..5ede5329 100644 --- a/Tests/MCSTests/LockfileOperationsTests.swift +++ b/Tests/MCSTests/LockfileOperationsTests.swift @@ -497,48 +497,6 @@ struct LockfileOperationsTests { } } - // MARK: - updatePacks: Empty registry - - @Test("updatePacks returns early when no packs registered") - func updatePacksEmptyRegistry() throws { - let home = try makeTmpDir() - defer { try? FileManager.default.removeItem(at: home) } - - try writeRegistry([], home: home) - - let ops = makeOperations(home: home) - // Should not throw — early return for empty registry - try ops.updatePacks() - } - - @Test("updatePacks exits non-zero when a pack fails in a non-interactive process") - func updatePacksThrowsOnFailureNonInteractive() throws { - let home = try makeTmpDir() - defer { try? FileManager.default.removeItem(at: home) } - - // A git pack whose checkout doesn't exist on disk — the real fetch fails, producing - // a hard failure. The test process has no TTY, so updatePacks must surface a non-zero - // exit rather than silently warning. - let ghost = PackRegistryFile.PackEntry( - identifier: "ghost", - displayName: "Ghost Pack", - author: nil, - sourceURL: "file:///fake/ghost.git", - ref: nil, - commitSHA: "abcdef0123", - localPath: "ghost", - addedAt: "2026-03-21T00:00:00Z", - trustedScriptHashes: [:], - isLocal: nil - ) - try writeRegistry([ghost], home: home) - - let ops = makeOperations(home: home) - #expect(throws: ExitCode.self) { - try ops.updatePacks() - } - } - // MARK: - checkoutLockedCommits: Git operations (mock-based) /// Set up a locked pack with a real pack directory and lockfile, returning diff --git a/Tests/MCSTests/SyncCommandTests.swift b/Tests/MCSTests/SyncCommandTests.swift index 202986fa..37467bb4 100644 --- a/Tests/MCSTests/SyncCommandTests.swift +++ b/Tests/MCSTests/SyncCommandTests.swift @@ -12,7 +12,6 @@ struct SyncCommandTests { #expect(cmd.all == false) #expect(cmd.dryRun == false) #expect(cmd.lock == false) - #expect(cmd.update == false) #expect(cmd.customize == false) #expect(cmd.global == false) } @@ -47,12 +46,6 @@ struct SyncCommandTests { #expect(cmd.lock == true) } - @Test("Parses --update flag") - func parsesUpdate() throws { - let cmd = try SyncCommand.parse(["--update"]) - #expect(cmd.update == true) - } - @Test("skipLock is true when --dry-run is set") func skipLockWhenDryRun() throws { let cmd = try SyncCommand.parse(["--dry-run"]) @@ -78,7 +71,6 @@ struct SyncCommandTests { #expect(cmd.pack == ["ios"]) #expect(cmd.dryRun == true) #expect(cmd.lock == true) - #expect(cmd.update == false) #expect(cmd.all == false) } @@ -123,39 +115,29 @@ struct SyncCommandTests { var config = MCSConfig() for flag in [nil, true, false] as [Bool?] { config.generateLockfile = flag - #expect(SyncCommand.lockfileAction(dryRun: true, update: false, config: config) == .skip) - #expect(SyncCommand.lockfileAction(dryRun: true, update: true, config: config) == .skip) - } - } - - @Test("Dispatch: --update forces write regardless of config") - func dispatchUpdateForcesWrite() { - var config = MCSConfig() - for flag in [nil, true, false] as [Bool?] { - config.generateLockfile = flag - #expect(SyncCommand.lockfileAction(dryRun: false, update: true, config: config) == .write) + #expect(SyncCommand.lockfileAction(dryRun: true, config: config) == .skip) } } - @Test("Dispatch: generate-lockfile=true writes without --update") + @Test("Dispatch: generate-lockfile=true writes") func dispatchConfigTrueWrites() { var config = MCSConfig() config.generateLockfile = true - #expect(SyncCommand.lockfileAction(dryRun: false, update: false, config: config) == .write) + #expect(SyncCommand.lockfileAction(dryRun: false, config: config) == .write) } @Test("Dispatch: generate-lockfile=nil (unset) reports drift — upgrade path") func dispatchConfigNilReportsDrift() { let config = MCSConfig() #expect(config.generateLockfile == nil) - #expect(SyncCommand.lockfileAction(dryRun: false, update: false, config: config) == .reportDrift) + #expect(SyncCommand.lockfileAction(dryRun: false, config: config) == .reportDrift) } @Test("Dispatch: generate-lockfile=false stays silent — explicit opt-out") func dispatchConfigFalseSkips() { var config = MCSConfig() config.generateLockfile = false - #expect(SyncCommand.lockfileAction(dryRun: false, update: false, config: config) == .skip) + #expect(SyncCommand.lockfileAction(dryRun: false, config: config) == .skip) } } diff --git a/docs/architecture.md b/docs/architecture.md index 74a05670..f3c141b3 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -99,7 +99,7 @@ Before modifying files with user content (e.g., `CLAUDE.local.md`), a timestampe ### Lockfile (`Core/Lockfile.swift`) -`mcs.lock.yaml` pins pack commits for reproducible builds. Generation is **opt-in** (default off) — enable persistently with `mcs config set generate-lockfile true` or write once with the deprecated `mcs sync --update`. Tri-state on `generate-lockfile`: `true` writes on every sync; `false` is fully silent (explicit opt-out); `nil` (never configured) reports SHA drift against a pre-existing lockfile so users upgrading from the auto-generation era see their stale lockfile. Used with `--lock` to checkout pinned commits. `mcs update` respects the config (writes only when `true`) and never force-writes the way `--update` does. +`mcs.lock.yaml` pins pack commits for reproducible builds. Generation is **opt-in** (default off) — enable persistently with `mcs config set generate-lockfile true`. Tri-state on `generate-lockfile`: `true` writes on every sync; `false` is fully silent (explicit opt-out); `nil` (never configured) reports SHA drift against a pre-existing lockfile so users upgrading from the auto-generation era see their stale lockfile. Used with `--lock` to checkout pinned commits. `mcs update` respects the config (writes only when `true`). ### ClaudeIntegration (`Core/ClaudeIntegration.swift`) @@ -172,7 +172,7 @@ Verbose form is also supported — see [Tech Pack Schema](techpack-schema.md). 10. **Run pack configure hooks**: pack-specific setup (e.g., generate config files) 11. **Ensure gitignore entries**: add `.claude/` entries to global gitignore 12. **Save project state**: write `.mcs-project` with artifact records for each pack -13. **Write lockfile (opt-in)**: if `--update` was passed or `generate-lockfile: true`, save `mcs.lock.yaml` with current pack state. When `generate-lockfile` is unset (upgrade path) and a stale lockfile exists, emit a drift warning with a migration hint. Explicit `generate-lockfile: false` is silent +13. **Write lockfile (opt-in)**: if `generate-lockfile: true`, save `mcs.lock.yaml` with current pack state. When `generate-lockfile` is unset (upgrade path) and a stale lockfile exists, emit a drift warning with a migration hint. Explicit `generate-lockfile: false` is silent The `--pack` flag bypasses multi-select for CI use: `mcs sync --pack ios --pack web`. @@ -341,7 +341,7 @@ The command (`Commands/ExportCommand.swift`) is a read-only `ParsableCommand` (n | **Non-Destructive** | User content in `CLAUDE.local.md` is preserved via `` section markers. Content outside markers is never touched. | | **Convergent** | Deselected packs are fully cleaned up — MCP servers removed, project files deleted, template sections stripped, settings keys cleaned. No orphaned artifacts. | | **Trust Verification** | Pack scripts are SHA-256 hashed at `mcs pack add` time and verified at load time. Modified scripts are detected and the user is prompted to re-trust before proceeding. Local packs skip verification since scripts change during development. | -| **Lockfile (opt-in)** | `mcs.lock.yaml` pins pack commits for reproducible environments. Generation is off by default; enable with `mcs config set generate-lockfile true` or write once with the deprecated `mcs sync --update`. Explicit `generate-lockfile: false` is silent; only the never-configured `nil` state surfaces drift warnings against a stale pre-existing lockfile. Use `--lock` to check out pinned versions from an existing lockfile. `mcs update` honours `generate-lockfile` and never force-writes. | +| **Lockfile (opt-in)** | `mcs.lock.yaml` pins pack commits for reproducible environments. Generation is off by default; enable with `mcs config set generate-lockfile true`. Explicit `generate-lockfile: false` is silent; only the never-configured `nil` state surfaces drift warnings against a stale pre-existing lockfile. Use `--lock` to check out pinned versions from an existing lockfile. `mcs update` honours `generate-lockfile`. | ## Concurrency Model diff --git a/docs/cli.md b/docs/cli.md index 78163b05..612d3543 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -14,7 +14,6 @@ mcs sync --dry-run # Preview what would change mcs sync --customize # Per-pack component selection mcs sync --global # Install to global scope (~/.claude/) mcs sync --lock # Checkout locked versions from mcs.lock.yaml -mcs sync --update # Fetch latest and write/update mcs.lock.yaml (opt-in) ``` | Flag | Description | @@ -26,7 +25,6 @@ mcs sync --update # Fetch latest and write/update mcs.lock.yaml ( | `--customize` | Per-pack component selection (deselect individual components). | | `--global` | Sync global-scope components (brew packages, plugins, MCP servers to `~/.claude/`). | | `--lock` | Check out the commits pinned in `mcs.lock.yaml`. | -| `--update` | **Deprecated** — use `mcs update` instead. Fetches latest pack versions and force-writes `mcs.lock.yaml` regardless of the `generate-lockfile` config. | `mcs sync` is also the default command — running `mcs` alone is equivalent to `mcs sync`. @@ -58,7 +56,7 @@ mcs update --dry-run # Preview without making changes - **Refresh-only** — does not add or remove packs. Use `mcs sync` to change the configured set. - **Multi-scope by default** — when configured packs exist in both global and project scopes, both are refreshed in one command (this was the original pain point that motivated the verb). -- **Lockfile is gated by config** — `mcs update` writes `mcs.lock.yaml` only when `generate-lockfile: true`. Drift is reported when the key is unset and a lockfile is present (the upgrade nudge). This is intentionally different from the deprecated `mcs sync --update`, which force-writes the lockfile regardless of config. +- **Lockfile is gated by config** — `mcs update` writes `mcs.lock.yaml` only when `generate-lockfile: true`. Drift is reported when the key is unset and a lockfile is present (the upgrade nudge). **Trust prompts:** when a pack's scripts have changed, `mcs update` prompts for trust. Denying the prompt skips the pack for this run (the registry stays at the old SHA, and the pack is excluded from re-apply so untrusted scripts don't auto-install). The prompt re-fires on the next `mcs update` run.