From 2be2ff25559ba6351cafd5393e60bbf3a7764493 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 17:44:18 +0000 Subject: [PATCH 01/30] Remove TelemetryDeck and the telemetry config key TelemetryDeck is an Apple-only SDK and its only identity source was IOKit's IOPlatformUUID, so it is the first hard blocker for a Linux build. Delete the SDK dependency, Core/Telemetry.swift, the 24 MCSAnalytics call sites, and the `telemetry` config key rather than leaving a key that accepts a value and does nothing. Existing ~/.mcs/config.yaml files keep loading: the synthesized decoder ignores the now-unknown `telemetry:` line. `mcs config set telemetry ...` reports an unknown key, and the first `config set` after upgrading re-encodes the file without the stale line. Two new tests pin the decode and `config list` paths. Removing trackCommand also drops a 200 ms RunLoop wait at the end of every command on macOS. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01V9KvZKADJzXojs4fwCgWAR --- Package.swift | 2 - .../mcs/Commands/CheckUpdatesCommand.swift | 4 - Sources/mcs/Commands/CleanupCommand.swift | 2 - Sources/mcs/Commands/ConfigCommand.swift | 6 - Sources/mcs/Commands/DoctorCommand.swift | 2 - Sources/mcs/Commands/ExportCommand.swift | 2 - Sources/mcs/Commands/PackCommand.swift | 5 - Sources/mcs/Commands/SyncCommand.swift | 2 - Sources/mcs/Commands/UpdateCommand.swift | 2 - .../mcs/Commands/ValidatePackCommand.swift | 1 - Sources/mcs/Core/Constants.swift | 10 -- Sources/mcs/Core/MCSConfig.swift | 16 --- Sources/mcs/Core/Telemetry.swift | 126 ------------------ Tests/MCSTests/MCSConfigTests.swift | 71 +++++----- 14 files changed, 40 insertions(+), 211 deletions(-) delete mode 100644 Sources/mcs/Core/Telemetry.swift diff --git a/Package.swift b/Package.swift index 24fb5636..5b21fedf 100644 --- a/Package.swift +++ b/Package.swift @@ -10,7 +10,6 @@ let package = Package( dependencies: [ .package(url: "https://github.com/apple/swift-argument-parser.git", from: "1.5.0"), .package(url: "https://github.com/jpsim/Yams.git", from: "5.0.0"), - .package(url: "https://github.com/TelemetryDeck/SwiftSDK.git", from: "2.0.0"), ], targets: [ .executableTarget( @@ -18,7 +17,6 @@ let package = Package( dependencies: [ .product(name: "ArgumentParser", package: "swift-argument-parser"), .product(name: "Yams", package: "Yams"), - .product(name: "TelemetryDeck", package: "SwiftSDK"), ], path: "Sources/mcs", swiftSettings: [ diff --git a/Sources/mcs/Commands/CheckUpdatesCommand.swift b/Sources/mcs/Commands/CheckUpdatesCommand.swift index ea08baa4..c17c7430 100644 --- a/Sources/mcs/Commands/CheckUpdatesCommand.swift +++ b/Sources/mcs/Commands/CheckUpdatesCommand.swift @@ -16,10 +16,6 @@ struct CheckUpdatesCommand: ParsableCommand { func run() throws { let env = Environment() let output = CLIOutput() - if !hook { - MCSAnalytics.initialize(env: env, output: output) - } - defer { MCSAnalytics.trackCommand(.checkUpdates) } let shell = ShellRunner(environment: env) let registry = PackRegistryFile(path: env.packsRegistry) diff --git a/Sources/mcs/Commands/CleanupCommand.swift b/Sources/mcs/Commands/CleanupCommand.swift index 273b7945..3b9b4bed 100644 --- a/Sources/mcs/Commands/CleanupCommand.swift +++ b/Sources/mcs/Commands/CleanupCommand.swift @@ -19,8 +19,6 @@ struct CleanupCommand: LockedCommand { func perform() throws { let env = Environment() let output = CLIOutput() - MCSAnalytics.initialize(env: env, output: output) - defer { MCSAnalytics.trackCommand(.cleanup) } output.header("Backup Cleanup") diff --git a/Sources/mcs/Commands/ConfigCommand.swift b/Sources/mcs/Commands/ConfigCommand.swift index a3534955..16991f84 100644 --- a/Sources/mcs/Commands/ConfigCommand.swift +++ b/Sources/mcs/Commands/ConfigCommand.swift @@ -20,8 +20,6 @@ struct ListConfig: ParsableCommand { func run() throws { let env = Environment() let output = CLIOutput() - MCSAnalytics.initialize(env: env, output: output) - defer { MCSAnalytics.trackCommand(.configList) } let config = MCSConfig.load(from: env.mcsConfigFile, output: output) output.header("Configuration") @@ -53,8 +51,6 @@ struct GetConfig: ParsableCommand { func run() throws { let env = Environment() let output = CLIOutput() - MCSAnalytics.initialize(env: env, output: output) - defer { MCSAnalytics.trackCommand(.configGet) } let config = MCSConfig.load(from: env.mcsConfigFile, output: output) guard MCSConfig.knownKeys.contains(where: { $0.key == key }) else { @@ -88,8 +84,6 @@ struct SetConfig: ParsableCommand { func run() throws { let env = Environment() let output = CLIOutput() - MCSAnalytics.initialize(env: env, output: output) - defer { MCSAnalytics.trackCommand(.configSet) } var config = MCSConfig.load(from: env.mcsConfigFile, output: output) guard MCSConfig.knownKeys.contains(where: { $0.key == key }) else { diff --git a/Sources/mcs/Commands/DoctorCommand.swift b/Sources/mcs/Commands/DoctorCommand.swift index 18795a9e..f1c812f0 100644 --- a/Sources/mcs/Commands/DoctorCommand.swift +++ b/Sources/mcs/Commands/DoctorCommand.swift @@ -26,8 +26,6 @@ struct DoctorCommand: LockedCommand { func perform() throws { let env = Environment() let output = CLIOutput() - MCSAnalytics.initialize(env: env, output: output) - defer { MCSAnalytics.trackCommand(.doctor) } let shell = ShellRunner(environment: env) let registry = TechPackRegistry.loadWithExternalPacks( environment: env, diff --git a/Sources/mcs/Commands/ExportCommand.swift b/Sources/mcs/Commands/ExportCommand.swift index 9bba2494..b3e36e9b 100644 --- a/Sources/mcs/Commands/ExportCommand.swift +++ b/Sources/mcs/Commands/ExportCommand.swift @@ -29,8 +29,6 @@ struct ExportCommand: ParsableCommand { func run() throws { let env = Environment() let output = CLIOutput() - MCSAnalytics.initialize(env: env, output: output) - defer { MCSAnalytics.trackCommand(.export) } output.header("Export Configuration") diff --git a/Sources/mcs/Commands/PackCommand.swift b/Sources/mcs/Commands/PackCommand.swift index 043a45c6..70f936ab 100644 --- a/Sources/mcs/Commands/PackCommand.swift +++ b/Sources/mcs/Commands/PackCommand.swift @@ -12,7 +12,6 @@ struct PackCommandContext { output = CLIOutput() shell = ShellRunner(environment: env) registry = PackRegistryFile(path: env.packsRegistry) - MCSAnalytics.initialize(env: env, output: output) } func loadRegistry() throws -> PackRegistryFile.RegistryData { @@ -56,7 +55,6 @@ struct AddPack: LockedCommand { func perform() throws { let ctx = PackCommandContext() - defer { MCSAnalytics.trackCommand(.packAdd) } let resolver = PackSourceResolver() let packSource: PackSource @@ -437,7 +435,6 @@ struct RemovePack: LockedCommand { func perform() throws { let ctx = PackCommandContext() - defer { MCSAnalytics.trackCommand(.packRemove) } let fetcher = PackFetcher( shell: ctx.shell, output: ctx.output, @@ -625,7 +622,6 @@ struct UpdatePack: LockedCommand { func perform() throws { let ctx = PackCommandContext() - defer { MCSAnalytics.trackCommand(.packUpdate) } let updater = PackUpdater( fetcher: PackFetcher(shell: ctx.shell, output: ctx.output, packsDirectory: ctx.env.packsDirectory), @@ -725,7 +721,6 @@ struct ListPacks: ParsableCommand { func run() throws { let ctx = PackCommandContext() - defer { MCSAnalytics.trackCommand(.packList) } ctx.output.header("Tech Packs") diff --git a/Sources/mcs/Commands/SyncCommand.swift b/Sources/mcs/Commands/SyncCommand.swift index e4c5ae71..4dfd028f 100644 --- a/Sources/mcs/Commands/SyncCommand.swift +++ b/Sources/mcs/Commands/SyncCommand.swift @@ -35,8 +35,6 @@ struct SyncCommand: LockedCommand { func perform() throws { let env = Environment() let output = CLIOutput() - MCSAnalytics.initialize(env: env, output: output) - defer { MCSAnalytics.trackCommand(.sync) } let shell = ShellRunner(environment: env) guard ensureClaudeCLI(shell: shell, environment: env, output: output) else { diff --git a/Sources/mcs/Commands/UpdateCommand.swift b/Sources/mcs/Commands/UpdateCommand.swift index 4adaccf6..2962a46e 100644 --- a/Sources/mcs/Commands/UpdateCommand.swift +++ b/Sources/mcs/Commands/UpdateCommand.swift @@ -36,8 +36,6 @@ struct UpdateCommand: LockedCommand { func perform() throws { let env = Environment() let output = CLIOutput() - MCSAnalytics.initialize(env: env, output: output) - defer { MCSAnalytics.trackCommand(.update) } let shell = ShellRunner(environment: env) guard ensureClaudeCLI(shell: shell, environment: env, output: output) else { diff --git a/Sources/mcs/Commands/ValidatePackCommand.swift b/Sources/mcs/Commands/ValidatePackCommand.swift index eb70de84..ff10fd5e 100644 --- a/Sources/mcs/Commands/ValidatePackCommand.swift +++ b/Sources/mcs/Commands/ValidatePackCommand.swift @@ -12,7 +12,6 @@ struct ValidatePack: ParsableCommand { func run() throws { let ctx = PackCommandContext() - defer { MCSAnalytics.trackCommand(.packValidate) } let packPath = try resolvePackPath(ctx: ctx) ctx.output.info("Validating pack at \(packPath.path)...") diff --git a/Sources/mcs/Core/Constants.swift b/Sources/mcs/Core/Constants.swift index 579d27f1..cbbccf61 100644 --- a/Sources/mcs/Core/Constants.swift +++ b/Sources/mcs/Core/Constants.swift @@ -156,16 +156,6 @@ enum Constants { static let brewFormula = "mcs-cli/tap/mcs" } - // MARK: - Telemetry - - enum Telemetry { - /// The TelemetryDeck app identifier. - static let appID = "A6CB95A6-0DD5-4B46-82AA-5A319797ACA0" - - /// Marker file indicating the telemetry notice has been shown. - static let telemetryNoticedFile = ".telemetry-noticed" - } - // MARK: - Plugins enum Plugins { diff --git a/Sources/mcs/Core/MCSConfig.swift b/Sources/mcs/Core/MCSConfig.swift index 04ef561d..f933d325 100644 --- a/Sources/mcs/Core/MCSConfig.swift +++ b/Sources/mcs/Core/MCSConfig.swift @@ -6,21 +6,14 @@ import Yams struct MCSConfig: Codable { var updateCheckPacks: Bool? var updateCheckCLI: Bool? - var telemetry: Bool? var generateLockfile: Bool? enum CodingKeys: String, CodingKey, CaseIterable { case updateCheckPacks = "update-check-packs" case updateCheckCLI = "update-check-cli" - case telemetry case generateLockfile = "generate-lockfile" } - /// Whether telemetry is enabled. Defaults to `true` when unconfigured (`nil`). - var isTelemetryEnabled: Bool { - telemetry != false - } - /// Whether any update check is enabled (at least one key is true). var isUpdateCheckEnabled: Bool { (updateCheckPacks ?? false) || (updateCheckCLI ?? false) @@ -63,11 +56,6 @@ struct MCSConfig: Codable { description: "Automatically check for new mcs versions on Claude Code session start", defaultValue: "false" ), - ConfigKey( - key: CodingKeys.telemetry.rawValue, - description: "Enable anonymous usage telemetry", - defaultValue: "true" - ), ConfigKey( key: CodingKeys.generateLockfile.rawValue, description: "Write mcs.lock.yaml after each sync (pin pack commits for reproducible setups)", @@ -106,7 +94,6 @@ struct MCSConfig: Codable { switch key { case CodingKeys.updateCheckPacks.rawValue: updateCheckPacks case CodingKeys.updateCheckCLI.rawValue: updateCheckCLI - case CodingKeys.telemetry.rawValue: telemetry case CodingKeys.generateLockfile.rawValue: generateLockfile default: nil } @@ -121,9 +108,6 @@ struct MCSConfig: Codable { case CodingKeys.updateCheckCLI.rawValue: updateCheckCLI = value return true - case CodingKeys.telemetry.rawValue: - telemetry = value - return true case CodingKeys.generateLockfile.rawValue: generateLockfile = value return true diff --git a/Sources/mcs/Core/Telemetry.swift b/Sources/mcs/Core/Telemetry.swift deleted file mode 100644 index 5c6bf902..00000000 --- a/Sources/mcs/Core/Telemetry.swift +++ /dev/null @@ -1,126 +0,0 @@ -import CryptoKit -import Foundation -import IOKit -import TelemetryDeck - -/// Lightweight telemetry wrapper around TelemetryDeck SDK. -/// Sends anonymous usage signals (command name + version) to understand -/// how many users are using the CLI. No PII is collected. -/// -/// - Enabled by default (opt-out); disable with `mcs config set telemetry false`. -/// - User identity is derived from the hardware UUID (IOPlatformUUID), salted -/// and SHA-256 hashed — never stored on disk, cannot be tampered with. -/// - All methods silently no-op on failure; telemetry must never break a command. -enum MCSAnalytics { - /// Type-safe command names for telemetry signals. - enum Command: String { - case sync - case update - case doctor - case cleanup - case packAdd = "pack.add" - case packRemove = "pack.remove" - case packList = "pack.list" - case packUpdate = "pack.update" - case packValidate = "pack.validate" - case export - case checkUpdates = "check-updates" - case configList = "config.list" - case configGet = "config.get" - case configSet = "config.set" - } - - /// Single-threaded CLI — no synchronization needed. - private nonisolated(unsafe) static var enabled = false - - // MARK: - Public API - - /// Initialize telemetry. Call once at the start of a command. - /// Loads config to check opt-out, derives anonymous ID, initializes TelemetryDeck SDK. - /// Shows a one-time notice on first use. - static func initialize(env: Environment, output: CLIOutput) { - guard !enabled else { return } - - let config = MCSConfig.load(from: env.mcsConfigFile, output: output) - guard config.isTelemetryEnabled else { return } - - showFirstRunNoticeIfNeeded(env: env, output: output) - - let tdConfig = TelemetryDeck.Config(appID: Constants.Telemetry.appID) - tdConfig.defaultUser = anonymousUserID() - tdConfig.sendNewSessionBeganSignal = false - TelemetryDeck.initialize(config: tdConfig) - - enabled = true - } - - /// Send a command execution signal, flush, and wait briefly for delivery. - /// Call once at the end of a command — blocks for up to 200ms. - static func trackCommand(_ command: Command) { - guard enabled else { return } - TelemetryDeck.signal( - "cli.command", - parameters: [ - "command": command.rawValue, - "mcsVersion": MCSVersion.current, - ] - ) - TelemetryDeck.requestImmediateSync() - RunLoop.current.run(until: Date(timeIntervalSinceNow: 0.2)) - } - - // MARK: - Anonymous ID - - /// Derive a stable anonymous identifier from the hardware UUID. - /// The result is `SHA256("mcs-telemetry-" + IOPlatformUUID)` as a hex string. - private static func anonymousUserID() -> String { - guard let uuid = hardwareUUID() else { - // Fallback: random UUID for this session only (no persistence, no inflation risk) - return UUID().uuidString - } - let salted = "mcs-telemetry-" + uuid - let digest = SHA256.hash(data: Data(salted.utf8)) - return digest.map { String(format: "%02x", $0) }.joined() - } - - /// Read the IOPlatformUUID from IOKit (hardware UUID burned into firmware). - private static func hardwareUUID() -> String? { - let service = IOServiceGetMatchingService( - kIOMainPortDefault, - IOServiceMatching("IOPlatformExpertDevice") - ) - defer { IOObjectRelease(service) } - guard service != IO_OBJECT_NULL else { return nil } - guard let uuidRef = IORegistryEntryCreateCFProperty( - service, - "IOPlatformUUID" as CFString, - kCFAllocatorDefault, - 0 - ) else { return nil } - return uuidRef.takeRetainedValue() as? String - } - - // MARK: - First-Run Notice - - /// Show a one-time notice about telemetry on the very first run. - private static func showFirstRunNoticeIfNeeded(env: Environment, output: CLIOutput) { - let markerPath = env.mcsDirectory - .appendingPathComponent(Constants.Telemetry.telemetryNoticedFile) - guard !FileManager.default.fileExists(atPath: markerPath.path) else { return } - - output.info("mcs collects anonymous usage telemetry to improve the tool.") - output.dimmed("Run 'mcs config set telemetry false' to disable.") - - // Create the marker file so the notice is only shown once. - let dir = markerPath.deletingLastPathComponent() - do { - try FileManager.default.createDirectory(at: dir, withIntermediateDirectories: true) - } catch { - output.warn("Could not create telemetry marker directory: \(error.localizedDescription)") - return - } - if !FileManager.default.createFile(atPath: markerPath.path, contents: nil) { - output.warn("Could not write telemetry marker file — notice will repeat next run") - } - } -} diff --git a/Tests/MCSTests/MCSConfigTests.swift b/Tests/MCSTests/MCSConfigTests.swift index 94cc84a2..cc07b5c3 100644 --- a/Tests/MCSTests/MCSConfigTests.swift +++ b/Tests/MCSTests/MCSConfigTests.swift @@ -21,7 +21,6 @@ struct MCSConfigTests { let config = MCSConfig.load(from: path) #expect(config.updateCheckPacks == nil) #expect(config.updateCheckCLI == nil) - #expect(config.telemetry == nil) } @Test("Load parses valid YAML") @@ -33,14 +32,12 @@ struct MCSConfigTests { let yaml = """ update-check-packs: true update-check-cli: false - telemetry: false """ try yaml.write(to: path, atomically: true, encoding: .utf8) let config = MCSConfig.load(from: path) #expect(config.updateCheckPacks == true) #expect(config.updateCheckCLI == false) - #expect(config.telemetry == false) } @Test("Load returns empty config for corrupt YAML") @@ -79,13 +76,11 @@ struct MCSConfigTests { var config = MCSConfig() config.updateCheckPacks = true config.updateCheckCLI = false - config.telemetry = false try config.save(to: path) let reloaded = MCSConfig.load(from: path) #expect(reloaded.updateCheckPacks == true) #expect(reloaded.updateCheckCLI == false) - #expect(reloaded.telemetry == false) } @Test("Save creates parent directories") @@ -145,26 +140,6 @@ struct MCSConfigTests { #expect(!config.isUnconfigured) } - @Test("isTelemetryEnabled defaults to true when nil") - func isTelemetryEnabledNil() { - let config = MCSConfig() - #expect(config.isTelemetryEnabled) - } - - @Test("isTelemetryEnabled returns true when explicitly true") - func isTelemetryEnabledTrue() { - var config = MCSConfig() - config.telemetry = true - #expect(config.isTelemetryEnabled) - } - - @Test("isTelemetryEnabled returns false when explicitly false") - func isTelemetryEnabledFalse() { - var config = MCSConfig() - config.telemetry = false - #expect(!config.isTelemetryEnabled) - } - @Test("isLockfileGenerationEnabled defaults to false when nil") func isLockfileGenerationEnabledNil() { let config = MCSConfig() @@ -222,12 +197,10 @@ struct MCSConfigTests { var config = MCSConfig() config.updateCheckPacks = true config.updateCheckCLI = false - config.telemetry = true config.generateLockfile = true #expect(config.value(forKey: "update-check-packs") == true) #expect(config.value(forKey: "update-check-cli") == false) - #expect(config.value(forKey: "telemetry") == true) #expect(config.value(forKey: "generate-lockfile") == true) #expect(config.value(forKey: "unknown-key") == nil) } @@ -244,16 +217,16 @@ struct MCSConfigTests { #expect(cliSet) #expect(config.updateCheckCLI == false) - let telemetrySet = config.setValue(false, forKey: "telemetry") - #expect(telemetrySet) - #expect(config.telemetry == false) - let lockfileSet = config.setValue(true, forKey: "generate-lockfile") #expect(lockfileSet) #expect(config.generateLockfile == true) let unknownSet = config.setValue(true, forKey: "unknown-key") #expect(!unknownSet) + + // Telemetry was removed; the key must be rejected like any other unknown key. + let telemetrySet = config.setValue(false, forKey: "telemetry") + #expect(!telemetrySet) } // MARK: - Known Keys @@ -264,4 +237,40 @@ struct MCSConfigTests { let knownKeyValues = Set(MCSConfig.knownKeys.map(\.key)) #expect(codingKeyValues == knownKeyValues) } + + // MARK: - Telemetry Removal Compatibility + + @Test("A config file left over from a telemetry-enabled release still decodes") + func loadTolerateStaleTelemetryKey() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let path = tmpDir.appendingPathComponent("config.yaml") + let yaml = """ + update-check-packs: true + telemetry: false + """ + try yaml.write(to: path, atomically: true, encoding: .utf8) + + let config = MCSConfig.load(from: path) + #expect(config.updateCheckPacks == true, "a stale key must not discard the live ones") + } + + @Test("The config list rendering ignores a stale telemetry key") + func listIgnoresStaleTelemetryKey() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let path = tmpDir.appendingPathComponent("config.yaml") + try "telemetry: false\n".write(to: path, atomically: true, encoding: .utf8) + + let config = MCSConfig.load(from: path) + // Mirrors ListConfig.run(): every known key is rendered from value(forKey:). + let rendered = MCSConfig.knownKeys.map { known -> String in + let current = config.value(forKey: known.key) + return "\(known.key): \(current.map { String($0) } ?? "(not set)")" + } + #expect(!rendered.contains { $0.contains("telemetry") }) + #expect(rendered.count == MCSConfig.CodingKeys.allCases.count) + } } From ca35b9beef327d41409299e6e909162cb8fd013d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 17:45:44 +0000 Subject: [PATCH 02/30] Use swift-crypto for SHA-256 on Linux, CryptoKit on Darwin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Foundation has no SHA-256 on Linux and the three hashing sites use nothing but SHA256.hash(data:), so a `#if canImport(CryptoKit)` import shim is the whole change. swift-crypto is declared as a Linux-only target dependency: on Darwin its Crypto target ships a PrivacyInfo.xcprivacy resource bundle that SwiftPM would emit next to the release binary — release.yml tars a single file — and macOS keeps hashing through the system CryptoKit, which is what makes a digest recorded in .mcs-project state on one platform validate on the other. That contract is now pinned by two known-answer tests, one per hasher, so a divergence between the two implementations fails the suite instead of silently invalidating state files. Resolving on macOS now fetches two more repositories, swift-crypto and its transitive swift-asn1; the CI cache key is keyed on Package.swift, so it is paid once per manifest change. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01V9KvZKADJzXojs4fwCgWAR --- Package.swift | 2 ++ Sources/mcs/Core/FileHasher.swift | 10 +++++++++- Sources/mcs/Core/SettingsHasher.swift | 4 ++++ Sources/mcs/Doctor/SectionValidator.swift | 4 ++++ Tests/MCSTests/FileHasherTests.swift | 10 ++++++++++ Tests/MCSTests/SettingsHasherTests.swift | 8 ++++++++ 6 files changed, 37 insertions(+), 1 deletion(-) diff --git a/Package.swift b/Package.swift index 5b21fedf..08bb9946 100644 --- a/Package.swift +++ b/Package.swift @@ -10,6 +10,7 @@ let package = Package( dependencies: [ .package(url: "https://github.com/apple/swift-argument-parser.git", from: "1.5.0"), .package(url: "https://github.com/jpsim/Yams.git", from: "5.0.0"), + .package(url: "https://github.com/apple/swift-crypto.git", from: "3.0.0"), ], targets: [ .executableTarget( @@ -17,6 +18,7 @@ let package = Package( dependencies: [ .product(name: "ArgumentParser", package: "swift-argument-parser"), .product(name: "Yams", package: "Yams"), + .product(name: "Crypto", package: "swift-crypto", condition: .when(platforms: [.linux])), ], path: "Sources/mcs", swiftSettings: [ diff --git a/Sources/mcs/Core/FileHasher.swift b/Sources/mcs/Core/FileHasher.swift index f6671f0e..a68f8ba9 100644 --- a/Sources/mcs/Core/FileHasher.swift +++ b/Sources/mcs/Core/FileHasher.swift @@ -1,7 +1,15 @@ +#if canImport(CryptoKit) import CryptoKit +#else +// Foundation has no SHA-256 on Linux, so the Glibc build gets the digest from swift-crypto, whose +// `Crypto` module is API-identical. The dependency is declared for Linux only so macOS keeps +// hashing through the system CryptoKit — a digest written into `.mcs-project` state on one +// platform has to validate on the other. +import Crypto +#endif import Foundation -/// Pure CryptoKit utilities for SHA-256 file hashing. +/// SHA-256 file hashing utilities. /// Extracted from the deleted `Manifest` type — used by `PackTrustManager` /// for trust verification and by `ComponentExecutor` for directory copies. enum FileHasher { diff --git a/Sources/mcs/Core/SettingsHasher.swift b/Sources/mcs/Core/SettingsHasher.swift index 28d5fd3f..1681ade8 100644 --- a/Sources/mcs/Core/SettingsHasher.swift +++ b/Sources/mcs/Core/SettingsHasher.swift @@ -1,4 +1,8 @@ +#if canImport(CryptoKit) import CryptoKit +#else +import Crypto +#endif import Foundation /// Deterministic SHA-256 hashing of pack-contributed settings values. diff --git a/Sources/mcs/Doctor/SectionValidator.swift b/Sources/mcs/Doctor/SectionValidator.swift index dabcfccd..4ad26470 100644 --- a/Sources/mcs/Doctor/SectionValidator.swift +++ b/Sources/mcs/Doctor/SectionValidator.swift @@ -1,4 +1,8 @@ +#if canImport(CryptoKit) import CryptoKit +#else +import Crypto +#endif import Foundation /// Validates managed sections in composed template files. diff --git a/Tests/MCSTests/FileHasherTests.swift b/Tests/MCSTests/FileHasherTests.swift index f04b8cf7..ac23b833 100644 --- a/Tests/MCSTests/FileHasherTests.swift +++ b/Tests/MCSTests/FileHasherTests.swift @@ -25,6 +25,16 @@ struct FileHasherTests { #expect(hash == expected) } + @Test("sha256(data:) matches the NIST known answer") + func knownAnswerForData() { + // Pins the digest across CryptoKit (Darwin) and swift-crypto (Linux): a hash recorded in + // .mcs-project state on one platform has to validate on the other. + #expect( + FileHasher.sha256(data: Data("abc".utf8)) + == "ba7816bf8f01cfea414140de5dae2223b00361a396177a9cb410ff61f20015ad" + ) + } + @Test("directoryFileHashes returns sorted entries for all files") func directoryHashes() throws { let tmpDir = try makeTmpDir() diff --git a/Tests/MCSTests/SettingsHasherTests.swift b/Tests/MCSTests/SettingsHasherTests.swift index dc6db071..12921ec9 100644 --- a/Tests/MCSTests/SettingsHasherTests.swift +++ b/Tests/MCSTests/SettingsHasherTests.swift @@ -19,6 +19,14 @@ struct SettingsHasherTests { #expect(hash1 == hash2) } + @Test("the canonical form hashes to a pinned digest") + func pinnedDigest() { + // SHA-256 of the canonical form "alwaysThinkingEnabled=true\n". Pinned so the digest cannot + // drift between CryptoKit and swift-crypto — see FileHasherTests.knownAnswerForData. + let hash = SettingsHasher.hash(keyPaths: ["alwaysThinkingEnabled"], in: ["alwaysThinkingEnabled": true]) + #expect(hash == "2bdbc4888ab5234a3b483dfb56870d73b941e36a5fe86d6d5bcd8497b9df73a9") + } + @Test("dotted key path extracts nested value") func dottedKeyPath() { let json: [String: Any] = ["env": ["FOO": "bar"]] From 785b35d779943fd462f3123f0e1df32060d30967 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 17:46:42 +0000 Subject: [PATCH 03/30] Replace OSAllocatedUnfairLock with a portable Locked value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `import os` is Darwin-only and Synchronization.Mutex is macOS 15+, above this package's macOS 13 floor, so the two lock sites move to a small NSLock-backed box with the same withLock { $0 } shape. Both sites are cold — a warning tally and a one-shot timeout flag — so the difference in lock cost does not matter. The @unchecked Sendable on Locked is the type's purpose, not a way to quiet the checker: the lock supplies an invariant the compiler cannot be told about, and the doc comment says so at the one site that owns it. Deviation from the plan: the WarningCounter-sharing case it asked for already exists (DoctorRunnerIntegrationTests.swift:467) and now covers the swap unchanged, so LockedTests instead pins withLock's return/mutation semantics and the captured-box shape ScriptRunner depends on. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01V9KvZKADJzXojs4fwCgWAR --- Sources/mcs/Core/CLIOutput.swift | 3 +-- Sources/mcs/Core/Locked.swift | 23 ++++++++++++++++ Sources/mcs/ExternalPack/ScriptRunner.swift | 3 +-- Tests/MCSTests/LockedTests.swift | 30 +++++++++++++++++++++ 4 files changed, 55 insertions(+), 4 deletions(-) create mode 100644 Sources/mcs/Core/Locked.swift create mode 100644 Tests/MCSTests/LockedTests.swift diff --git a/Sources/mcs/Core/CLIOutput.swift b/Sources/mcs/Core/CLIOutput.swift index cdbf380f..46552f7e 100644 --- a/Sources/mcs/Core/CLIOutput.swift +++ b/Sources/mcs/Core/CLIOutput.swift @@ -1,5 +1,4 @@ import Foundation -import os /// Shared, mutable tally of warnings emitted through a `CLIOutput`. /// @@ -11,7 +10,7 @@ import os /// `Sendable` so `CLIOutput` stays `Sendable` (it's captured in isolated /// closures, e.g. via `ScriptRunner`); the lock supplies that guarantee. final class WarningCounter: Sendable { - private let lock = OSAllocatedUnfairLock(initialState: 0) + private let lock = Locked(0) var count: Int { lock.withLock { $0 } diff --git a/Sources/mcs/Core/Locked.swift b/Sources/mcs/Core/Locked.swift new file mode 100644 index 00000000..e1843cb1 --- /dev/null +++ b/Sources/mcs/Core/Locked.swift @@ -0,0 +1,23 @@ +import Foundation + +/// A mutable value guarded by a lock, with the `withLock { $0 }` shape of `OSAllocatedUnfairLock`, +/// which is Darwin-only. `Synchronization.Mutex` would be the modern answer but is macOS 15+, above +/// this package's macOS 13 deployment target. +/// +/// `@unchecked Sendable` is the point of the type rather than a way to quiet the checker: the +/// invariant — every access to `value` happens under `lock` — is real and cannot be expressed to +/// the compiler. +final class Locked: @unchecked Sendable { + private let lock = NSLock() + private var value: Value + + init(_ value: Value) { + self.value = value + } + + func withLock(_ body: (inout Value) throws -> Result) rethrows -> Result { + lock.lock() + defer { lock.unlock() } + return try body(&value) + } +} diff --git a/Sources/mcs/ExternalPack/ScriptRunner.swift b/Sources/mcs/ExternalPack/ScriptRunner.swift index 7f72ed46..2bbf3e5f 100644 --- a/Sources/mcs/ExternalPack/ScriptRunner.swift +++ b/Sources/mcs/ExternalPack/ScriptRunner.swift @@ -1,5 +1,4 @@ import Foundation -import os /// Higher-level wrapper around `ShellRunner` for executing scripts from external packs. /// Adds pack-specific concerns: path containment validation, standard environment @@ -187,7 +186,7 @@ struct ScriptRunner { // Best-effort killer — see `exceededTimeout` for why the verdict does not rely on it. let start = ProcessInfo.processInfo.systemUptime - let killerFired = OSAllocatedUnfairLock(initialState: false) + let killerFired = Locked(false) let workItem = DispatchWorkItem { [process] in if process.isRunning { killerFired.withLock { $0 = true } diff --git a/Tests/MCSTests/LockedTests.swift b/Tests/MCSTests/LockedTests.swift new file mode 100644 index 00000000..5acbf0e7 --- /dev/null +++ b/Tests/MCSTests/LockedTests.swift @@ -0,0 +1,30 @@ +import Foundation +@testable import mcs +import Testing + +struct LockedTests { + @Test("withLock returns the body's value and the mutation is visible to the next access") + func returnsBodyValueAndPersistsMutation() { + let locked = Locked(1) + + let doubled = locked.withLock { value -> Int in + value *= 2 + return value + } + + #expect(doubled == 2) + #expect(locked.withLock { $0 } == 2) + } + + @Test("A box captured by a closure is the same box the caller reads") + func mutationThroughACaptureIsVisibleToTheOwner() { + // The shape ScriptRunner relies on: a flag set inside a DispatchWorkItem and read back on + // the calling thread after the work item has finished. + let flag = Locked(false) + let setter = { flag.withLock { $0 = true } } + + setter() + + #expect(flag.withLock { $0 }) + } +} From c870547bb02528a6d42bb7bd5c788abbda8eddd1 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 17:50:24 +0000 Subject: [PATCH 04/30] Build the terminal and process layer on Glibc as well as Darwin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five files reach libc directly; they now select Darwin or Glibc explicitly rather than relying on Foundation's Darwin re-export. The termios differences are real values, not spellings: tcflag_t is UInt on Darwin and UInt32 on Glibc, and c_cc is a tuple whose length and layout differ (VMIN is index 16 there, 6 here). TerminalAttributes widens every mask through tcflag_t and indexes c_cc by the platform's own constant, so the magic indices that were hardcoded in withRawTerminal have no second place to get wrong. TIOCGWINSZ widens to UInt, a no-op on Darwin where it already is one. In runInteractive, Glibc types strdup as returning an optional where Darwin implicitly unwraps it, which left the child dereferencing optionals. Allocating and checking the command path and working directory in the parent keeps the child free of anything that could trap through the Swift runtime after fork. fileExists(atPath:isDirectory:) and createFile(atPath:contents:) are @discardableResult only on Darwin, so four call sites were unused-result errors under -warnings-as-errors. They use the result instead of discarding it: the copy paths now require the source to exist before treating it as a directory (unchanged behaviour — isDirectory is only written when the file exists), and the gitignore bootstrap writes through Data, surfacing the error it was dropping. The two test sites assert the chdir succeeded. The PTY integration test now asserts the marker file the command creates, so it proves forkpty, execve and the bridge loop ran rather than that the component was recorded. Linux `swift build` and `swift build --build-tests` now succeed; `swift test` is 1424/1429, with the five remaining failures triaged in the next commit. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01V9KvZKADJzXojs4fwCgWAR --- Sources/mcs/Core/CLIOutput.swift | 14 ++++--- Sources/mcs/Core/FileLock.swift | 5 +++ Sources/mcs/Core/GitignoreManager.swift | 2 +- Sources/mcs/Core/GlobMatcher.swift | 6 ++- Sources/mcs/Core/ShellRunner.swift | 38 +++++++++++++++---- Sources/mcs/Core/TerminalAttributes.swift | 38 +++++++++++++++++++ Sources/mcs/Sync/ComponentExecutor.swift | 12 +++--- Tests/MCSTests/FileLockTests.swift | 5 +++ .../MCSTests/LifecycleIntegrationTests.swift | 10 +++-- Tests/MCSTests/PackSourceResolverTests.swift | 4 +- Tests/MCSTests/SyncCommandTests.swift | 2 +- Tests/MCSTests/TerminalAttributesTests.swift | 32 ++++++++++++++++ 12 files changed, 140 insertions(+), 28 deletions(-) create mode 100644 Sources/mcs/Core/TerminalAttributes.swift create mode 100644 Tests/MCSTests/TerminalAttributesTests.swift diff --git a/Sources/mcs/Core/CLIOutput.swift b/Sources/mcs/Core/CLIOutput.swift index 46552f7e..5f2c391d 100644 --- a/Sources/mcs/Core/CLIOutput.swift +++ b/Sources/mcs/Core/CLIOutput.swift @@ -1,4 +1,9 @@ import Foundation +#if canImport(Darwin) +import Darwin +#elseif canImport(Glibc) +import Glibc +#endif /// Shared, mutable tally of warnings emitted through a `CLIOutput`. /// @@ -85,7 +90,7 @@ struct CLIOutput { private var terminalColumns: Int { var ws = winsize() - if ioctl(STDOUT_FILENO, TIOCGWINSZ, &ws) == 0, ws.ws_col > 0 { + if ioctl(STDOUT_FILENO, UInt(TIOCGWINSZ), &ws) == 0, ws.ws_col > 0 { return Int(ws.ws_col) } return 80 @@ -691,7 +696,7 @@ struct CLIOutput { private func readByte() -> UInt8 { var byte: UInt8 = 0 - _ = Darwin.read(STDIN_FILENO, &byte, 1) + _ = read(STDIN_FILENO, &byte, 1) return byte } @@ -707,10 +712,7 @@ struct CLIOutput { private func withRawTerminal(_ body: () -> T) -> T { var original = termios() tcgetattr(STDIN_FILENO, &original) - var raw = original - raw.c_lflag &= ~UInt(ICANON | ECHO) - raw.c_cc.16 = 1 // VMIN = 1 - raw.c_cc.17 = 0 // VTIME = 0 + var raw = TerminalAttributes.rawMode(from: original) tcsetattr(STDIN_FILENO, TCSANOW, &raw) write("\u{1B}[?25l") defer { diff --git a/Sources/mcs/Core/FileLock.swift b/Sources/mcs/Core/FileLock.swift index 43a1886f..b77dd56f 100644 --- a/Sources/mcs/Core/FileLock.swift +++ b/Sources/mcs/Core/FileLock.swift @@ -1,5 +1,10 @@ import ArgumentParser import Foundation +#if canImport(Darwin) +import Darwin +#elseif canImport(Glibc) +import Glibc +#endif /// Errors thrown when the process lock cannot be acquired. enum FileLockError: Error, LocalizedError { diff --git a/Sources/mcs/Core/GitignoreManager.swift b/Sources/mcs/Core/GitignoreManager.swift index 4f194182..9252e4a0 100644 --- a/Sources/mcs/Core/GitignoreManager.swift +++ b/Sources/mcs/Core/GitignoreManager.swift @@ -47,7 +47,7 @@ struct GitignoreManager { try fm.createDirectory(at: dir, withIntermediateDirectories: true) } if !fm.fileExists(atPath: path.path) { - fm.createFile(atPath: path.path, contents: nil) + try Data().write(to: path) } } diff --git a/Sources/mcs/Core/GlobMatcher.swift b/Sources/mcs/Core/GlobMatcher.swift index 3ea706fe..cbdc8501 100644 --- a/Sources/mcs/Core/GlobMatcher.swift +++ b/Sources/mcs/Core/GlobMatcher.swift @@ -1,5 +1,9 @@ -import Darwin import Foundation +#if canImport(Darwin) +import Darwin +#elseif canImport(Glibc) +import Glibc +#endif /// Thin wrapper over POSIX `fnmatch(3)` for path-vs-pattern matching. /// diff --git a/Sources/mcs/Core/ShellRunner.swift b/Sources/mcs/Core/ShellRunner.swift index dacb9f51..1405d257 100644 --- a/Sources/mcs/Core/ShellRunner.swift +++ b/Sources/mcs/Core/ShellRunner.swift @@ -1,4 +1,9 @@ import Foundation +#if canImport(Darwin) +import Darwin +#elseif canImport(Glibc) +import Glibc +#endif /// Result of running a shell command. struct ShellResult { @@ -200,13 +205,32 @@ struct ShellRunner: ShellRunning { } + [nil] defer { envp.compactMap(\.self).forEach { free($0) } } - let argv: [UnsafeMutablePointer?] = ([executable] + arguments).map { strdup($0) } + [nil] - defer { argv.compactMap(\.self).forEach { free($0) } } + // After fork() only async-signal-safe calls are legal, so the child has no way to report + // an allocation failure, and a force-unwrap there would trap through the Swift runtime. + // Every C string the child needs is therefore allocated and checked here, in the parent. + // (Glibc types `strdup` as returning an optional; Darwin implicitly unwraps it.) + guard let executablePath = strdup(executable) else { + return ShellResult( + exitCode: 1, stdout: "", stderr: "Could not allocate the command path for \(executable)" + ) + } + let argv: [UnsafeMutablePointer?] = [executablePath] + arguments.map { strdup($0) } + [nil] + defer { argv.compactMap(\.self).forEach { free($0) } } // frees executablePath too — it is argv[0] // Pre-convert workingDirectory to a C string before fork so the child // doesn't need to invoke Swift's String-to-CString bridge (not fork-safe). - let cwdCStr = workingDirectory.map { strdup($0) } - defer { cwdCStr.map { free($0) } } + let workingDirectoryPath: UnsafeMutablePointer? + if let workingDirectory { + guard let copy = strdup(workingDirectory) else { + return ShellResult( + exitCode: 1, stdout: "", stderr: "Could not allocate the working directory path" + ) + } + workingDirectoryPath = copy + } else { + workingDirectoryPath = nil + } + defer { workingDirectoryPath.map { free($0) } } // Save the terminal's current attributes so we can restore them after. var originalTermios = termios() @@ -226,7 +250,7 @@ struct ShellRunner: ShellRunning { // After fork, only async-signal-safe functions are safe. Avoid Swift // runtime calls (String interpolation, ARC) — they can deadlock on // locks held by other threads in the parent at fork time. - if let cwd = cwdCStr { + if let cwd = workingDirectoryPath { if chdir(cwd) != 0 { let err = strerror(errno) _ = write(STDERR_FILENO, "chdir failed: ", 14) @@ -235,8 +259,8 @@ struct ShellRunner: ShellRunning { _exit(126) } } - // Use argv[0] (already a C string from strdup) instead of the Swift String. - execve(argv[0], argv, envp) + // Use the pre-allocated C string instead of the Swift String. + execve(executablePath, argv, envp) let err = strerror(errno) _ = write(STDERR_FILENO, "execve failed: ", 15) if let err { _ = write(STDERR_FILENO, err, strlen(err)) } diff --git a/Sources/mcs/Core/TerminalAttributes.swift b/Sources/mcs/Core/TerminalAttributes.swift new file mode 100644 index 00000000..e10928ed --- /dev/null +++ b/Sources/mcs/Core/TerminalAttributes.swift @@ -0,0 +1,38 @@ +#if canImport(Darwin) +import Darwin +#elseif canImport(Glibc) +import Glibc +#endif + +/// termios helpers shared by the raw-mode picker. +/// +/// `c_cc` imports as a fixed-size tuple whose length and element order are platform-defined +/// (`NCCS` is 20 on Darwin and 32 on Glibc; `VMIN` is index 16 there and 6 here), so the index has +/// to come from the platform's own constant rather than a literal. `tcflag_t` likewise differs — +/// `UInt` on Darwin, `UInt32` on Glibc — so every flag mask is widened through `tcflag_t(...)`. +enum TerminalAttributes { + /// Reads one control character. Exists for `TerminalAttributesTests`: asserting raw mode + /// without a TTY is the only way to cover this on both platforms. + static func controlCharacter(_ attributes: termios, _ index: Int32) -> cc_t { + var attributes = attributes + return withUnsafeBytes(of: &attributes.c_cc) { raw in + raw.bindMemory(to: cc_t.self)[Int(index)] + } + } + + static func setControlCharacter(_ attributes: inout termios, _ index: Int32, to value: cc_t) { + withUnsafeMutableBytes(of: &attributes.c_cc) { raw in + raw.bindMemory(to: cc_t.self)[Int(index)] = value + } + } + + /// Non-canonical, no-echo attributes: one byte satisfies a read and nothing is echoed, so arrow + /// keys reach the picker as they are typed. + static func rawMode(from attributes: termios) -> termios { + var raw = attributes + raw.c_lflag &= ~tcflag_t(ICANON | ECHO) + setControlCharacter(&raw, VMIN, to: 1) + setControlCharacter(&raw, VTIME, to: 0) + return raw + } +} diff --git a/Sources/mcs/Sync/ComponentExecutor.swift b/Sources/mcs/Sync/ComponentExecutor.swift index ddec1a82..b9c0ed85 100644 --- a/Sources/mcs/Sync/ComponentExecutor.swift +++ b/Sources/mcs/Sync/ComponentExecutor.swift @@ -139,10 +139,10 @@ struct ComponentExecutor { ) var isDir: ObjCBool = false - fm.fileExists(atPath: source.path, isDirectory: &isDir) + let sourceExists = fm.fileExists(atPath: source.path, isDirectory: &isDir) var installedHashes: [String: String] = [:] - if isDir.boolValue { + if sourceExists, isDir.boolValue { // Source is a directory — copy all files recursively try fm.createDirectory(at: destURL, withIntermediateDirectories: true) let contents = try fm.contentsOfDirectory(at: source, includingPropertiesForKeys: nil) @@ -226,11 +226,11 @@ struct ComponentExecutor { ) var isDir: ObjCBool = false - fm.fileExists(atPath: source.path, isDirectory: &isDir) + let sourceExists = fm.fileExists(atPath: source.path, isDirectory: &isDir) var installedPaths: [String] = [] var installedHashes: [String: String] = [:] - if isDir.boolValue { + if sourceExists, isDir.boolValue { try fm.createDirectory(at: destURL, withIntermediateDirectories: true) let contents = try fm.contentsOfDirectory(at: source, includingPropertiesForKeys: nil) for file in contents { @@ -281,9 +281,9 @@ struct ComponentExecutor { ) throws { let fm = FileManager.default var isDir: ObjCBool = false - fm.fileExists(atPath: source.path, isDirectory: &isDir) + let sourceExists = fm.fileExists(atPath: source.path, isDirectory: &isDir) - if isDir.boolValue { + if sourceExists, isDir.boolValue { try fm.createDirectory(at: destination, withIntermediateDirectories: true) let contents = try fm.contentsOfDirectory(at: source, includingPropertiesForKeys: nil) for child in contents { diff --git a/Tests/MCSTests/FileLockTests.swift b/Tests/MCSTests/FileLockTests.swift index 4a4b40fa..fbe2e5f5 100644 --- a/Tests/MCSTests/FileLockTests.swift +++ b/Tests/MCSTests/FileLockTests.swift @@ -1,4 +1,9 @@ import Foundation +#if canImport(Darwin) +import Darwin +#elseif canImport(Glibc) +import Glibc +#endif @testable import mcs import Testing diff --git a/Tests/MCSTests/LifecycleIntegrationTests.swift b/Tests/MCSTests/LifecycleIntegrationTests.swift index 82edc831..c7dcebbc 100644 --- a/Tests/MCSTests/LifecycleIntegrationTests.swift +++ b/Tests/MCSTests/LifecycleIntegrationTests.swift @@ -1279,7 +1279,7 @@ struct ShellCommandLifecycleTests { #expect(FileManager.default.fileExists(atPath: markerPath)) } - @Test("shellCommand with interactive flag is accepted and state is recorded") + @Test("shellCommand with interactive flag really runs under a PTY") func shellCommandInteractiveAccepted() throws { let bed = try LifecycleTestBed() defer { bed.cleanup() } @@ -1303,12 +1303,14 @@ struct ShellCommandLifecycleTests { ) let registry = TechPackRegistry(packs: [pack]) - // Configure — interactive commands use forkpty() in real ShellRunner, - // but the test verifies the component is accepted and state is recorded. + // Interactive commands go through forkpty() in the real ShellRunner, so the marker file + // proves the whole path ran — fork, execve and the terminal/PTY bridge loop — not just + // that the component was recorded. let configurator = bed.makeGlobalSyncConfigurator(registry: registry) try configurator.configure(packs: [pack], confirmRemovals: false) - // Verify state records the pack + #expect(FileManager.default.fileExists(atPath: markerPath)) + let state = try ProjectState(stateFile: bed.env.globalStateFile) #expect(state.configuredPacks.contains("interactive-pack")) } diff --git a/Tests/MCSTests/PackSourceResolverTests.swift b/Tests/MCSTests/PackSourceResolverTests.swift index 6acb7c4e..50006d76 100644 --- a/Tests/MCSTests/PackSourceResolverTests.swift +++ b/Tests/MCSTests/PackSourceResolverTests.swift @@ -169,8 +169,8 @@ struct PackSourceResolverTests { // Change CWD to tmpDir so "org/pack" resolves to the directory let originalDir = FileManager.default.currentDirectoryPath - FileManager.default.changeCurrentDirectoryPath(tmpDir.path) - defer { FileManager.default.changeCurrentDirectoryPath(originalDir) } + #expect(FileManager.default.changeCurrentDirectoryPath(tmpDir.path)) + defer { #expect(FileManager.default.changeCurrentDirectoryPath(originalDir)) } let result = try PackSourceResolver().resolve("org/pack") guard case let .localPath(url) = result else { diff --git a/Tests/MCSTests/SyncCommandTests.swift b/Tests/MCSTests/SyncCommandTests.swift index 9eb2e3b9..06b08c5c 100644 --- a/Tests/MCSTests/SyncCommandTests.swift +++ b/Tests/MCSTests/SyncCommandTests.swift @@ -393,7 +393,7 @@ struct SyncCommandGuardTests { let env = Environment(home: home) let originalCwd = FileManager.default.currentDirectoryPath - defer { FileManager.default.changeCurrentDirectoryPath(originalCwd) } + defer { #expect(FileManager.default.changeCurrentDirectoryPath(originalCwd)) } let cmd = try SyncCommand.parse([env.claudeDirectory.path, "--global", "--pack", "foo"]) let effective = try cmd.guardClaudeHomeCwd(env: env, output: silentOutput()) diff --git a/Tests/MCSTests/TerminalAttributesTests.swift b/Tests/MCSTests/TerminalAttributesTests.swift new file mode 100644 index 00000000..db93011b --- /dev/null +++ b/Tests/MCSTests/TerminalAttributesTests.swift @@ -0,0 +1,32 @@ +#if canImport(Darwin) +import Darwin +#elseif canImport(Glibc) +import Glibc +#endif +@testable import mcs +import Testing + +struct TerminalAttributesTests { + @Test("rawMode clears ICANON and ECHO, keeps ISIG, and sets VMIN/VTIME") + func rawModeFlagsAndControlCharacters() { + var original = termios() + original.c_lflag = tcflag_t(ICANON | ECHO | ISIG) + + let raw = TerminalAttributes.rawMode(from: original) + + #expect(raw.c_lflag & tcflag_t(ICANON) == 0) + #expect(raw.c_lflag & tcflag_t(ECHO) == 0) + #expect(raw.c_lflag & tcflag_t(ISIG) != 0, "Ctrl-C must still generate SIGINT") + #expect(TerminalAttributes.controlCharacter(raw, VMIN) == 1) + #expect(TerminalAttributes.controlCharacter(raw, VTIME) == 0) + } + + @Test("setControlCharacter writes only the requested index") + func setControlCharacterIsIndexed() { + var attributes = termios() + TerminalAttributes.setControlCharacter(&attributes, VMIN, to: 7) + + #expect(TerminalAttributes.controlCharacter(attributes, VMIN) == 7) + #expect(TerminalAttributes.controlCharacter(attributes, VTIME) == 0) + } +} From 769db0d52fafc466d2c60951b73af5e4861e0d5d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 18:06:09 +0000 Subject: [PATCH 05/30] Fix the Linux-only test failures and start the compatibility matrix MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four tests compared URL values for the same directory. On corelibs Foundation deletingLastPathComponent() and resolvingSymlinksInPath() return directory-flagged URLs carrying a trailing slash that standardizedFileURL does not strip, so file:///tmp/a/ != file:///tmp/a although .path is equal. The assertion these tests intend is about the path, and the product already compares paths for exactly this reason (PathContainment, Environment.isInsideClaudeHome), so they compare .path too. Nothing is weakened: the same directory identity is still asserted. The fifth failure, UpdateCheckerTests.registryWriteFailureContract, needs no change — it chmods a directory to 0555 and root ignores that. Verified passing as a non-root user, which is why the CI job must run on the runner rather than in a root container. docs/linux-support.md is added as a skeleton whose section 4 is the per-feature compatibility checklist. Twenty-seven rows are filled from features actually run against a Linux build of mcs in a sandbox home, including the raw-mode picker driven through a real PTY, the non-TTY fallback picker, the forkpty interactive component, MCP and plugin registration, doctor --fix and the process lock. The `brew:` and Claude Code prerequisite rows stay open for the commit that changes their behaviour. Linux: 1429 tests, all passing as a non-root user. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01V9KvZKADJzXojs4fwCgWAR --- Tests/MCSTests/ProjectDetectorTests.swift | 12 ++-- Tests/MCSTests/SyncCommandTests.swift | 2 +- docs/linux-support.md | 67 +++++++++++++++++++++++ 3 files changed, 76 insertions(+), 5 deletions(-) create mode 100644 docs/linux-support.md diff --git a/Tests/MCSTests/ProjectDetectorTests.swift b/Tests/MCSTests/ProjectDetectorTests.swift index b42a1f88..ea8d418d 100644 --- a/Tests/MCSTests/ProjectDetectorTests.swift +++ b/Tests/MCSTests/ProjectDetectorTests.swift @@ -24,7 +24,11 @@ struct ProjectDetectorTests { try FileManager.default.createDirectory(at: sourcesDir, withIntermediateDirectories: true) let root = ProjectDetector.findProjectRoot(from: sourcesDir) - #expect(root?.standardizedFileURL == tmpDir.standardizedFileURL) + // Compared as paths, not URLs: corelibs Foundation flags a directory URL produced by + // deletingLastPathComponent() with a trailing slash that standardizedFileURL does not + // strip, so two URLs for the same directory are unequal on Linux. The product compares + // paths for the same reason (PathContainment, Environment.isInsideClaudeHome). + #expect(root?.standardizedFileURL.path == tmpDir.standardizedFileURL.path) } @Test("Finds project root via CLAUDE.local.md") @@ -41,7 +45,7 @@ struct ProjectDetectorTests { try FileManager.default.createDirectory(at: subDir, withIntermediateDirectories: true) let root = ProjectDetector.findProjectRoot(from: subDir) - #expect(root?.standardizedFileURL == tmpDir.standardizedFileURL) + #expect(root?.standardizedFileURL.path == tmpDir.standardizedFileURL.path) } @Test("Returns nil when no project root found") @@ -77,7 +81,7 @@ struct ProjectDetectorTests { try FileManager.default.createDirectory(at: subDir, withIntermediateDirectories: true) let root = ProjectDetector.findProjectRoot(from: subDir) - #expect(root?.standardizedFileURL == tmpDir.standardizedFileURL) + #expect(root?.standardizedFileURL.path == tmpDir.standardizedFileURL.path) } @Test("Prefers .git over CLAUDE.local.md at same level") @@ -95,7 +99,7 @@ struct ProjectDetectorTests { ) let root = ProjectDetector.findProjectRoot(from: tmpDir) - #expect(root?.standardizedFileURL == tmpDir.standardizedFileURL) + #expect(root?.standardizedFileURL.path == tmpDir.standardizedFileURL.path) } } diff --git a/Tests/MCSTests/SyncCommandTests.swift b/Tests/MCSTests/SyncCommandTests.swift index 06b08c5c..c0445f1f 100644 --- a/Tests/MCSTests/SyncCommandTests.swift +++ b/Tests/MCSTests/SyncCommandTests.swift @@ -401,7 +401,7 @@ struct SyncCommandGuardTests { let resultCwd = URL(fileURLWithPath: FileManager.default.currentDirectoryPath) .resolvingSymlinksInPath() - #expect(resultCwd == env.homeDirectory.resolvingSymlinksInPath()) + #expect(resultCwd.path == env.homeDirectory.resolvingSymlinksInPath().path) } @Test("Guard triggers for nested paths like ~/.claude/skills") diff --git a/docs/linux-support.md b/docs/linux-support.md new file mode 100644 index 00000000..3eac7e71 --- /dev/null +++ b/docs/linux-support.md @@ -0,0 +1,67 @@ +# Linux support + +`mcs` runs on macOS and on Linux (glibc). This document records what is verified, what differs +between the two platforms, and why each porting decision was made. + +Sections 1–3 and 6 describe the platform. Section 4 is the per-feature compatibility checklist: +a row is filled in only after that feature has been **run** on Linux. Section 5 carries one +ADR-style entry per decision. + +## 1. Status and supported configurations + +*(Phase 10)* + +## 2. Prerequisites + +*(Phase 10)* + +## 3. Install, build and test on Linux + +*(Phase 10)* + +## 4. Compatibility matrix + +Legend: `verified` — run on Linux and observed; `pending` — not yet exercised. + +| Feature | macOS | Linux | Notes | +|---|---|---|---| +| `mcs sync` (project) | supported | verified | `mcs sync --pack linux-probe` in a git project: components installed, `settings.local.json` composed, `CLAUDE.local.md` generated, `.mcs-project` written. | +| `mcs sync --global` | supported | verified | `mcs sync --global --pack git-probe`: artifacts under `~/.claude/`, `settings.json` composed, `global-state.json` written. | +| `--customize` raw-mode picker | supported | verified | Driven under a real PTY: `↓` moved the cursor, `Space` toggled, `Enter` applied; in-place redraw and cursor hide/show correct. | +| Non-TTY fallback picker | supported | verified | stdin a PTY, stdout a pipe: numeric toggle + `Enter`, colours disabled. | +| `shell:` components | supported | verified | `shell: touch ` created the marker. | +| `shellInteractive: true` (PTY/sudo) | supported | verified | `forkpty` path ran the command; marker file created. Also asserted by `LifecycleIntegrationTests`. | +| `brew:` components | supported | pending | | +| `mcp:` (`claude mcp add`) | supported | verified | `demo-server` registered via `claude mcp add -s local`; `claude mcp list` shows it; removal deregisters it. | +| `plugin:` | supported | verified | `hookify@claude-code-plugins` installed through `claude plugin install`; `claude plugin list` shows it enabled. | +| `hook:` / `command:` / `skill:` / `agent:` copies | supported | verified | All four installed; hooks namespaced under `/`; interpreter inferred (`bash` for `.sh`, `python3` for `.py`). | +| `settingsFile:` | supported | verified | Deep-merged into `settings.local.json` with `__GREETING__` substituted. | +| `gitignore:` | supported | verified | Entry added to `~/.config/git/ignore`; reference counting kept it when one of two scopes was removed. | +| `mcs update` | supported | verified | `mcs update --project` re-applied the pack and refreshed `mcs.lock.yaml`. | +| `mcs doctor` | supported | verified | 20 checks across every section; `✓`/`⚠` rendering correct. | +| `mcs doctor --fix` | supported | verified | Deleting `.claude` from the global gitignore then `doctor --fix` re-added it (`GitignoreCheck.fix`). | +| `mcs pack add` (git) | supported | verified | `mcs pack add git://…/gitprobe` cloned, prompted for trust, registered. | +| `mcs pack add` (local) | supported | verified | `mcs pack add ` registered in place with `commitSHA: local`. | +| `mcs pack remove` / `list` / `update` / `validate` | supported | verified | `remove` unconfigured every affected scope; `update` reported "already up to date"; `validate` produced the python3 heuristic warning. | +| Pack trust hashing | supported | verified | Trust prompt shown on add for both a local and a git pack; re-validated on `pack update` (SHA-256 now from swift-crypto). | +| `mcs export` (incl. brew formula hints) | supported | verified | Exported `techpack.yaml`, hooks, skill, command, agent, settings and template from a live project. Brew hints empty without Linuxbrew. | +| `mcs cleanup` | supported | verified | Found and deleted a `CLAUDE.local.md.backup.*` file, with and without `--force`. | +| `mcs check-updates` + SessionStart hook | supported | verified | `check-updates`, `--json` and `--hook` all run; `mcs config set update-check-cli true` registered `mcs check-updates --hook` in `~/.claude/settings.json` and the cooldown file was written. | +| `mcs config` | supported | verified | `list` / `get` / `set` against `~/.mcs/config.yaml`. | +| File lock (`flock`) | supported | verified | Two concurrent syncs: the second exited 1 with "Another mcs process is running". | +| Lockfile (`mcs.lock.yaml`) | supported | verified | Written after sync with `generate-lockfile true`; `mcs sync --lock` consumed it. | +| Terminal colours / width | supported | verified | ANSI colour and the wrapped/re-rendered picker observed under a PTY; colours suppressed when stdout is a pipe. | +| Claude Code prerequisite | supported | pending | | +| Telemetry | removed | removed | Deleted outright — see ADR D1. | + +## 5. Decisions (ADR entries) + +*(Phase 10 — one entry per D1…D10)* + +## 6. Known limitations + +*(Phase 10)* + +## 7. How to add a platform-specific path + +*(Phase 10)* From e0e380fdf96cdba59e5c7184235c33d8a3785613 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 18:13:12 +0000 Subject: [PATCH 06/30] Stop the interactive bridge spinning on a broken stdin The poll loop already gives up on stdin when read() returns 0 or less, but a descriptor that comes back POLLERR or POLLNVAL without POLLIN takes no branch at all, so poll returns immediately forever and the bridge burns a core until the child exits. Dropping the descriptor lets the loop keep draining the PTY, which is what the rest of the loop already does on EOF. POLLHUP stays out of that mask on purpose: a hung-up stdin can still hold buffered data, and the existing read drains it. Verified on Linux with the interactive component run both with stdin from /dev/null and under a real controlling terminal. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01V9KvZKADJzXojs4fwCgWAR --- Sources/mcs/Core/ShellRunner.swift | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/Sources/mcs/Core/ShellRunner.swift b/Sources/mcs/Core/ShellRunner.swift index 1405d257..958af9db 100644 --- a/Sources/mcs/Core/ShellRunner.swift +++ b/Sources/mcs/Core/ShellRunner.swift @@ -304,6 +304,14 @@ struct ShellRunner: ShellRunning { break } + // A descriptor that errored or was closed under us never becomes readable, so poll(2) + // would keep returning immediately with no branch taken. poll(2) skips negative fds, + // so dropping it here lets the loop carry on draining the PTY. POLLHUP is deliberately + // not included: a hung-up stdin can still have buffered data, which the read drains. + if fds[0].revents & Int16(POLLERR | POLLNVAL) != 0 { + fds[0].fd = -1 + } + // Terminal → PTY (user typing, including password input) if fds[0].revents & Int16(POLLIN) != 0 { let n = read(STDIN_FILENO, &buf, buf.count) From 0298bd640ff02ab08bc2b33e03a6ad8c44fe1b98 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 18:13:32 +0000 Subject: [PATCH 07/30] Run build and tests on Ubuntu in CI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds one Linux job to pr-checks.yml so the port cannot regress. It runs on the ubuntu-latest runner with swiftly rather than in a swift container: the official images run as root, and UpdateCheckerTests.registryWriteFailureContract chmods a directory to 0555 and asserts the write fails, which root ignores. The toolchain version lives in one workflow env var so the release workflow can pin the same one. Lint stays on macOS — swiftformat and swiftlint give the same verdicts on both, so running them twice only adds version skew — and the macOS jobs are untouched. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01V9KvZKADJzXojs4fwCgWAR --- .github/workflows/pr-checks.yml | 41 +++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index d9bd2398..0aae2625 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -10,6 +10,10 @@ concurrency: group: ${{ github.workflow }}-${{ github.ref }} cancel-in-progress: ${{ github.event_name == 'pull_request' }} +env: + # One place for the Linux toolchain version; release.yml pins the same value. + SWIFT_VERSION: '6.2.0' + jobs: lint: name: Lint @@ -57,6 +61,43 @@ jobs: - name: Test run: swift test + build-and-test-linux: + name: Build & Test (linux) + needs: lint + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v6 + + # Not a `container:` job: the official swift images run as root, and + # UpdateCheckerTests.registryWriteFailureContract asserts that a write into a 0555 + # directory fails, which root ignores. + - name: Install Swift via swiftly + run: | + curl -O https://download.swift.org/swiftly/linux/swiftly-$(uname -m).tar.gz + tar zxf "swiftly-$(uname -m).tar.gz" + ./swiftly init --quiet-shell-followup --assume-yes --skip-install + . "$HOME/.local/share/swiftly/env.sh" + swiftly install --use "$SWIFT_VERSION" --assume-yes + echo "$HOME/.local/share/swiftly/bin" >> "$GITHUB_PATH" + + - uses: actions/cache@v5 + with: + path: | + .build/checkouts + .build/repositories + .build/workspace-state.json + key: spm-linux-${{ hashFiles('Package.swift') }} + restore-keys: spm-linux- + + - name: Swift version + run: swift --version + + - name: Resolve packages + run: swift package resolve + + - name: Test + run: swift test + build-universal: name: Universal Binary needs: lint From d218041accaee51542e325f23f4fb69ead735b25 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 18:25:37 +0000 Subject: [PATCH 08/30] Resolve Homebrew prefixes and installation guidance per platform MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The prefix derivation resolved symlinks before walking two components up from $PREFIX/bin/brew. Linuxbrew — and Intel macOS — install that entry point as a symlink into $PREFIX/Homebrew/bin/brew, so resolving first yielded $PREFIX/Homebrew, whose bin holds only brew. pathWithBrew then prepended a directory with nothing in it and hid $PREFIX/bin, where every formula's symlink lives, from the PATH probe that `brew:` components rest on. Dropping the resolution fixes Linuxbrew and is a latent macOS fix on Intel; the derivation is extracted so a test can pin it, since the resolved brew path comes from the real machine and cannot be injected. The fallback prefix and allPrefixes gain the Linuxbrew locations. `provides` is deliberately unchanged: a formula name is not a command name, and no function can map between them, so on Linux without Linuxbrew a `brew:` component is satisfied only when the two coincide. That is now stated in the compatibility matrix rather than papered over. Four messages assumed Homebrew exists. Where it does not, telling the user to run `mcs sync` is a loop with no exit — sync prints "Homebrew not found" and sends them back to doctor. They now name the package and the system package manager. The command-exists check drops its "run mcs sync" hint too: nothing there knows which component, if any, would install the command. ensureClaudeCLI no longer offers a Homebrew install on Linux. `claude-code` is a cask, and Linuxbrew has no casks, so the offer would have failed on exactly the machines that have brew; Linux prints the native-installer and npm commands instead. mcs does not run either: piping an installer into a shell is the user's trust decision. macOS behaviour is untouched. The CLI upgrade instruction is one constant, selected per platform, so the two places that print it cannot drift — Linux users install from a tarball, not the tap. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01V9KvZKADJzXojs4fwCgWAR --- Sources/mcs/Core/ClaudePrerequisite.swift | 25 +++++++-- Sources/mcs/Core/Constants.swift | 14 +++++ Sources/mcs/Core/Environment.swift | 35 +++++++++--- Sources/mcs/Core/Homebrew.swift | 34 +++++++++++- Sources/mcs/Core/UpdateChecker.swift | 4 +- Sources/mcs/Doctor/CoreDoctorChecks.swift | 6 ++- .../ExternalPack/ExternalDoctorCheck.swift | 4 +- Sources/mcs/Sync/ComponentExecutor.swift | 4 +- .../DoctorRunnerIntegrationTests.swift | 34 ++++++++++++ Tests/MCSTests/EnvironmentTests.swift | 53 +++++++++++++++++++ Tests/MCSTests/HomebrewTests.swift | 29 ++++++++++ .../MCSTests/LifecycleIntegrationTests.swift | 46 ++++++++++++++-- Tests/MCSTests/UpdateCheckerTests.swift | 13 +++++ docs/linux-support.md | 12 ++--- 14 files changed, 283 insertions(+), 30 deletions(-) diff --git a/Sources/mcs/Core/ClaudePrerequisite.swift b/Sources/mcs/Core/ClaudePrerequisite.swift index ae803dc3..9d5a4ea8 100644 --- a/Sources/mcs/Core/ClaudePrerequisite.swift +++ b/Sources/mcs/Core/ClaudePrerequisite.swift @@ -1,6 +1,6 @@ import Foundation -/// Verify that the Claude Code CLI is available, offering to install via Homebrew if missing. +/// Verify that the Claude Code CLI is available, offering an install where mcs can perform one. /// /// Returns `true` if Claude CLI is available (either already installed or successfully installed). /// Returns `false` if the user declines installation or installation fails. @@ -17,14 +17,15 @@ func ensureClaudeCLI( output.error("Claude Code CLI not found.") output.plain(" mcs requires the Claude Code CLI to function.") + #if canImport(Darwin) let brew = Homebrew(shell: shell, environment: environment) guard brew.isInstalled else { - output.plain(" Install it manually: https://docs.anthropic.com/en/docs/claude-code") + printManualClaudeInstallInstructions(output) return false } guard output.askYesNo("Install Claude Code via Homebrew?", default: true) else { - output.plain(" Install it manually: https://docs.anthropic.com/en/docs/claude-code") + printManualClaudeInstallInstructions(output) return false } @@ -39,6 +40,22 @@ func ensureClaudeCLI( if !result.stderr.isEmpty { output.dimmed(String(result.stderr.prefix(200))) } - output.plain(" Install it manually: https://docs.anthropic.com/en/docs/claude-code") + printManualClaudeInstallInstructions(output) + return false + #else + // `claude-code` is a Homebrew *cask*, and Linuxbrew has no casks, so offering the brew install + // would fail on exactly the machines that have brew. Print what does work instead. + printManualClaudeInstallInstructions(output) return false + #endif +} + +/// How to install Claude Code by hand. mcs never runs these itself — piping an installer into a +/// shell, or writing into npm's global prefix, is a trust decision that belongs to the user. +private func printManualClaudeInstallInstructions(_ output: CLIOutput) { + output.plain(" Install it manually: https://docs.anthropic.com/en/docs/claude-code") + #if !canImport(Darwin) + output.plain(" curl -fsSL https://claude.ai/install.sh | bash") + output.plain(" npm install -g @anthropic-ai/claude-code") + #endif } diff --git a/Sources/mcs/Core/Constants.swift b/Sources/mcs/Core/Constants.swift index cbbccf61..ab0a109a 100644 --- a/Sources/mcs/Core/Constants.swift +++ b/Sources/mcs/Core/Constants.swift @@ -154,6 +154,20 @@ enum Constants { /// The Homebrew formula name for mcs. static let brewFormula = "mcs-cli/tap/mcs" + + /// How the user upgrades mcs on this platform. Only macOS ships through the tap; the + /// Linux artifact is a release tarball, so telling a Linux user to run `brew upgrade` + /// would send them somewhere mcs did not come from. + static var upgradeInstruction: String { + #if canImport(Darwin) + "brew upgrade \(brewFormula)" + #else + "download the latest release from \(releasesURL)" + #endif + } + + /// Where the release tarballs are published. + static let releasesURL = "https://github.com/mcs-cli/mcs/releases/latest" } // MARK: - Plugins diff --git a/Sources/mcs/Core/Environment.swift b/Sources/mcs/Core/Environment.swift index f1b12c37..3332b7e4 100644 --- a/Sources/mcs/Core/Environment.swift +++ b/Sources/mcs/Core/Environment.swift @@ -52,21 +52,40 @@ struct Environment { if let resolvedBrew = Self.resolvedBrewPath { brewPath = resolvedBrew - brewPrefix = URL(fileURLWithPath: resolvedBrew) - .resolvingSymlinksInPath() - .deletingLastPathComponent().deletingLastPathComponent().path + brewPrefix = Self.brewPrefix(forBrewPath: resolvedBrew) } else { - #if arch(arm64) - brewPrefix = "/opt/homebrew" - #else - brewPrefix = "/usr/local" - #endif + brewPrefix = Self.defaultBrewPrefix brewPath = "\(brewPrefix)/bin/brew" } gitPath = Self.resolvedGitPath } + /// The Homebrew prefix implied by the path `brew` was found at. + /// + /// Two components up from `$PREFIX/bin/brew`, deliberately *without* resolving symlinks: + /// Linuxbrew and Intel macOS install `bin/brew` as a symlink into `$PREFIX/Homebrew/bin/brew`, + /// and resolving it first would yield `$PREFIX/Homebrew`, whose `bin` holds only `brew` — + /// hiding `$PREFIX/bin`, where every formula's symlink lives, from `pathWithBrew`. + static func brewPrefix(forBrewPath path: String) -> String { + URL(fileURLWithPath: path) + .deletingLastPathComponent().deletingLastPathComponent().path + } + + /// Where Homebrew installs itself when no `brew` is on PATH to ask. + static var defaultBrewPrefix: String { + #if canImport(Darwin) + #if arch(arm64) + "/opt/homebrew" + #else + "/usr/local" + #endif + #else + // Linuxbrew's documented multi-user prefix; unlike macOS it does not vary by architecture. + "/home/linuxbrew/.linuxbrew" + #endif + } + /// Directory where external tech pack checkouts live (`~/.mcs/packs/`). var packsDirectory: URL { mcsDirectory.appendingPathComponent(Constants.ExternalPacks.packsDirectory) diff --git a/Sources/mcs/Core/Homebrew.swift b/Sources/mcs/Core/Homebrew.swift index c471ff23..06c37492 100644 --- a/Sources/mcs/Core/Homebrew.swift +++ b/Sources/mcs/Core/Homebrew.swift @@ -2,8 +2,15 @@ import Foundation /// Manages Homebrew package installation and service management. struct Homebrew { - /// Both Homebrew prefix paths — arm64 and x86_64. - static let allPrefixes = ["/opt/homebrew", "/usr/local"] + /// Every prefix Homebrew installs itself at on this platform: both macOS architectures, or + /// Linuxbrew's multi-user and single-user locations. + static var allPrefixes: [String] { + #if canImport(Darwin) + ["/opt/homebrew", "/usr/local"] + #else + ["/home/linuxbrew/.linuxbrew", NSHomeDirectory() + "/.linuxbrew"] + #endif + } let shell: any ShellRunning let environment: Environment @@ -56,6 +63,29 @@ struct Homebrew { shell.run(environment.brewPath, arguments: ["uninstall", name]) } + /// What to tell the user about `package` when Homebrew is not installed. + /// + /// With no `brew` there is nothing `mcs sync` can do, so the advice must not point back at it + /// — that loop has no exit. On Linux, Homebrew is the unusual case: the package almost + /// certainly comes from the distribution's own package manager, so that is what is named. + static func manualInstallAdvice(for package: String) -> String { + #if canImport(Darwin) + "Homebrew not found — install it from https://brew.sh, then re-run 'mcs sync' to get \(package)" + #else + "Homebrew not found — install \(package) with your system package manager" + + " (apt, dnf, pacman, …) or install Homebrew from https://brew.sh" + #endif + } + + /// The counterpart of `manualInstallAdvice(for:)` for a package mcs wanted to remove. + static func manualUninstallAdvice(for package: String) -> String { + #if canImport(Darwin) + "Homebrew not found — remove '\(package)' yourself if nothing else needs it" + #else + "Homebrew not found — remove '\(package)' with your system package manager if nothing else needs it" + #endif + } + /// Detects the Homebrew formula that provides a command by reading the immediate /// symlink target in the Homebrew bin directory. Returns nil if the command isn't /// brew-installed. diff --git a/Sources/mcs/Core/UpdateChecker.swift b/Sources/mcs/Core/UpdateChecker.swift index 6f5171e5..a66d835e 100644 --- a/Sources/mcs/Core/UpdateChecker.swift +++ b/Sources/mcs/Core/UpdateChecker.swift @@ -602,7 +602,7 @@ struct UpdateChecker { if let cli = result.cliUpdate { output.warn( "mcs \(cli.latestVersion) available (current: \(cli.currentVersion)). " - + "Run 'brew upgrade \(Constants.MCSRepo.brewFormula)' to update." + + "To update, \(Constants.MCSRepo.upgradeInstruction)." ) } if !result.packUpdates.isEmpty { @@ -635,7 +635,7 @@ struct UpdateChecker { if let cli = result.cliUpdate { lines.append( "- New mcs version \(cli.latestVersion) is available (installed: \(cli.currentVersion))." - + " The user should run: brew upgrade \(Constants.MCSRepo.brewFormula)" + + " To update, the user should \(Constants.MCSRepo.upgradeInstruction)." ) } if !result.packUpdates.isEmpty { diff --git a/Sources/mcs/Doctor/CoreDoctorChecks.swift b/Sources/mcs/Doctor/CoreDoctorChecks.swift index 53ea7904..9a19f157 100644 --- a/Sources/mcs/Doctor/CoreDoctorChecks.swift +++ b/Sources/mcs/Doctor/CoreDoctorChecks.swift @@ -43,7 +43,11 @@ struct BrewPackageCheck: DoctorCheck { } func fix() -> FixResult { - .notFixable("Run 'mcs sync' to install dependencies") + let shell = ShellRunner(environment: environment) + guard Homebrew(shell: shell, environment: environment).isInstalled else { + return .notFixable(Homebrew.manualInstallAdvice(for: package)) + } + return .notFixable("Run 'mcs sync' to install dependencies") } } diff --git a/Sources/mcs/ExternalPack/ExternalDoctorCheck.swift b/Sources/mcs/ExternalPack/ExternalDoctorCheck.swift index 0da5b286..7cccf241 100644 --- a/Sources/mcs/ExternalPack/ExternalDoctorCheck.swift +++ b/Sources/mcs/ExternalPack/ExternalDoctorCheck.swift @@ -81,7 +81,9 @@ struct ExternalCommandExistsCheck: DoctorCheck { func fix() -> FixResult { guard let fixCommand else { - return .notFixable("Run 'mcs sync' to install dependencies") + // The pack declares no fix, and nothing here knows which component — if any — would + // install this command, so pointing at 'mcs sync' would be a guess that often no-ops. + return .notFixable("Install '\(command)' and make sure it is on PATH") } let result = scriptRunner.runCommand(fixCommand) if result.succeeded { diff --git a/Sources/mcs/Sync/ComponentExecutor.swift b/Sources/mcs/Sync/ComponentExecutor.swift index b9c0ed85..5f548e6a 100644 --- a/Sources/mcs/Sync/ComponentExecutor.swift +++ b/Sources/mcs/Sync/ComponentExecutor.swift @@ -15,7 +15,7 @@ struct ComponentExecutor { let brew = Homebrew(shell: shell, environment: environment) if brew.provides(package) { return true } guard brew.isInstalled else { - output.warn("Homebrew not found, cannot install \(package)") + output.warn(Homebrew.manualInstallAdvice(for: package)) return false } let result = brew.install(package) @@ -67,7 +67,7 @@ struct ComponentExecutor { func uninstallBrewPackage(_ package: String) -> Bool { let brew = Homebrew(shell: shell, environment: environment) guard brew.isInstalled else { - output.warn("Homebrew not found, cannot uninstall '\(package)'") + output.warn(Homebrew.manualUninstallAdvice(for: package)) return false } guard brew.isPackageInstalled(package) else { return true } diff --git a/Tests/MCSTests/DoctorRunnerIntegrationTests.swift b/Tests/MCSTests/DoctorRunnerIntegrationTests.swift index 33f82a1e..a5430f8a 100644 --- a/Tests/MCSTests/DoctorRunnerIntegrationTests.swift +++ b/Tests/MCSTests/DoctorRunnerIntegrationTests.swift @@ -614,3 +614,37 @@ struct DoctorSummaryWarningCountTests { #expect(withGhost.warnings == baseline.warnings + 1) } } + +// MARK: - Brew Guidance + +struct BrewPackageCheckGuidanceTests { + @Test("A brew package whose formula name is also a command on PATH passes") + func formulaNameMatchingCommandPasses() { + // `git` stands in for that whole class: the PATH probe is provenance-blind, so any + // installer satisfies the declaration. This is the only shape that works on a Linux box + // with no Homebrew. + let check = BrewPackageCheck(name: "git", section: "Dependencies", package: "git") + #expect(!check.check().isFailOrWarn) + } + + @Test("The fix for a missing package never advertises a command that cannot work") + func fixDoesNotPointAtSyncWhenBrewIsAbsent() { + let package = "mcs-nonexistent-formula-for-tests" + let check = BrewPackageCheck(name: package, section: "Dependencies", package: package) + #expect(check.check().isFailOrWarn) + + guard case let .notFixable(message) = check.fix() else { + Issue.record("Installing a package is sync's job, so the fix must be .notFixable") + return + } + + // Asserted against whichever machine runs this: with brew present `mcs sync` really can + // install the package, without it that advice is a loop with no exit. + if FileManager.default.fileExists(atPath: Environment().brewPath) { + #expect(message.contains("mcs sync")) + } else { + #expect(!message.contains("mcs sync")) + #expect(message.contains(package)) + } + } +} diff --git a/Tests/MCSTests/EnvironmentTests.swift b/Tests/MCSTests/EnvironmentTests.swift index 1a5ddce9..1299be0a 100644 --- a/Tests/MCSTests/EnvironmentTests.swift +++ b/Tests/MCSTests/EnvironmentTests.swift @@ -104,4 +104,57 @@ struct EnvironmentTests { #expect(!env.isInsideClaudeHome(env.homeDirectory)) } + + // MARK: - Homebrew prefix + + @Test("brewPrefix keeps the symlinked entry point's own prefix") + func brewPrefixDoesNotFollowSymlinks() throws { + let home = try makeTmpHome() + defer { try? FileManager.default.removeItem(at: home) } + + // The shape Linuxbrew and Intel macOS install: $PREFIX/bin/brew -> ../Homebrew/bin/brew. + let prefix = home.appendingPathComponent("prefix") + let repositoryBin = prefix.appendingPathComponent("Homebrew/bin") + let prefixBin = prefix.appendingPathComponent("bin") + try FileManager.default.createDirectory(at: repositoryBin, withIntermediateDirectories: true) + try FileManager.default.createDirectory(at: prefixBin, withIntermediateDirectories: true) + try Data().write(to: repositoryBin.appendingPathComponent("brew")) + try FileManager.default.createSymbolicLink( + atPath: prefixBin.appendingPathComponent("brew").path, + withDestinationPath: "../Homebrew/bin/brew" + ) + + // Resolving the symlink first would yield /Homebrew, whose bin holds only brew. + #expect(Environment.brewPrefix(forBrewPath: prefixBin.appendingPathComponent("brew").path) == prefix.path) + } + + @Test("brewPrefix handles a real file at $PREFIX/bin/brew (arm64 macOS shape)") + func brewPrefixForRealFile() { + #expect(Environment.brewPrefix(forBrewPath: "/opt/homebrew/bin/brew") == "/opt/homebrew") + } + + @Test("The fallback prefix is the platform's own default") + func defaultBrewPrefixPerPlatform() { + #if canImport(Darwin) + #if arch(arm64) + #expect(Environment.defaultBrewPrefix == "/opt/homebrew") + #else + #expect(Environment.defaultBrewPrefix == "/usr/local") + #endif + #else + #expect(Environment.defaultBrewPrefix == "/home/linuxbrew/.linuxbrew") + #endif + } + + @Test("pathWithBrew prepends the prefix bin directory exactly once") + func pathWithBrewPrependsOnce() throws { + let home = try makeTmpHome() + defer { try? FileManager.default.removeItem(at: home) } + let env = Environment(home: home) + + let path = env.pathWithBrew + let brewBin = "\(env.brewPrefix)/bin" + #expect(path.contains(brewBin)) + #expect(path.components(separatedBy: brewBin).count == 2, "brew bin must appear exactly once") + } } diff --git a/Tests/MCSTests/HomebrewTests.swift b/Tests/MCSTests/HomebrewTests.swift index 9c48f6c3..6acba7a7 100644 --- a/Tests/MCSTests/HomebrewTests.swift +++ b/Tests/MCSTests/HomebrewTests.swift @@ -15,6 +15,35 @@ struct HomebrewTests { #expect(Homebrew.bareName(of: package) == expected) } + // MARK: - allPrefixes + + @Test("allPrefixes lists this platform's Homebrew locations") + func allPrefixesPerPlatform() { + #if canImport(Darwin) + #expect(Homebrew.allPrefixes == ["/opt/homebrew", "/usr/local"]) + #else + #expect(Homebrew.allPrefixes == ["/home/linuxbrew/.linuxbrew", NSHomeDirectory() + "/.linuxbrew"]) + #endif + } + + // MARK: - Guidance when Homebrew is absent + + @Test("Install advice names the package and never sends the user back to 'mcs sync'") + func manualInstallAdviceIsActionable() { + let advice = Homebrew.manualInstallAdvice(for: "ripgrep") + #expect(advice.contains("ripgrep")) + #if canImport(Darwin) + #expect(advice.contains("brew.sh")) + #else + #expect(advice.contains("system package manager")) + #endif + } + + @Test("Uninstall advice names the package") + func manualUninstallAdviceNamesPackage() { + #expect(Homebrew.manualUninstallAdvice(for: "ripgrep").contains("ripgrep")) + } + // MARK: - provides @Test("provides takes the PATH fast path without spawning brew") diff --git a/Tests/MCSTests/LifecycleIntegrationTests.swift b/Tests/MCSTests/LifecycleIntegrationTests.swift index c7dcebbc..1a5f4be4 100644 --- a/Tests/MCSTests/LifecycleIntegrationTests.swift +++ b/Tests/MCSTests/LifecycleIntegrationTests.swift @@ -21,11 +21,14 @@ private struct LifecycleTestBed { try? FileManager.default.removeItem(at: home) } - func makeConfigurator(registry: TechPackRegistry = TechPackRegistry()) -> Configurator { + func makeConfigurator( + registry: TechPackRegistry = TechPackRegistry(), + shell: (any ShellRunning)? = nil + ) -> Configurator { Configurator( environment: env, output: CLIOutput(colorsEnabled: false), - shell: ShellRunner(environment: env), + shell: shell ?? ShellRunner(environment: env), registry: registry, strategy: ProjectSyncStrategy(projectPath: project, environment: env), claudeCLI: mockCLI @@ -45,12 +48,13 @@ private struct LifecycleTestBed { func makeGlobalSyncConfigurator( registry: TechPackRegistry = TechPackRegistry(), - warningCounter: WarningCounter? = nil + warningCounter: WarningCounter? = nil, + shell: (any ShellRunning)? = nil ) -> Configurator { Configurator( environment: env, output: CLIOutput(colorsEnabled: false, warningCounter: warningCounter), - shell: ShellRunner(environment: env), + shell: shell ?? ShellRunner(environment: env), registry: registry, strategy: GlobalSyncStrategy(environment: env), claudeCLI: mockCLI @@ -3184,4 +3188,38 @@ struct BrewPackageDoctorTests { #expect(summary.warnings == 0) #expect(summary.issues == 0) } + + /// The Linux happy path in one assertion: a `brew:` component whose command is on PATH is + /// satisfied without Homebrew existing at all, which is what makes `brew:` usable on a machine + /// that has no brew. Global scope, because that is where `configure` installs brew packages + /// inline. The package name is deliberately one no machine has, so the mocked PATH hit — not + /// the real machine — is what decides the outcome. + @Test("A brew package satisfied on PATH spawns no brew subprocess") + func pathSatisfiedBrewPackageSpawnsNoSubprocess() throws { + let bed = try LifecycleTestBed() + defer { bed.cleanup() } + + let package = "mcs-nonexistent-formula-for-tests" + let shell = MockShellRunner(environment: bed.env) + shell.commandExistsResult = true + + let pack = MockTechPack( + identifier: "brew-pack", + displayName: "Brew Pack", + components: [bed.brewComponent(pack: "brew-pack", id: "tool", package: package)] + ) + let registry = TechPackRegistry(packs: [pack]) + + try bed.makeGlobalSyncConfigurator(registry: registry, shell: shell) + .configure(packs: [pack], confirmRemovals: false) + + #expect(shell.commandExistsCalls.contains(package)) + #expect( + !shell.runCalls.contains { $0.executable == bed.env.brewPath }, + "a PATH hit must settle the question without asking brew" + ) + + let state = try ProjectState(stateFile: bed.env.globalStateFile) + #expect(state.configuredPacks.contains("brew-pack")) + } } diff --git a/Tests/MCSTests/UpdateCheckerTests.swift b/Tests/MCSTests/UpdateCheckerTests.swift index deef23a7..9a49e110 100644 --- a/Tests/MCSTests/UpdateCheckerTests.swift +++ b/Tests/MCSTests/UpdateCheckerTests.swift @@ -107,6 +107,19 @@ struct UpdateCheckerCacheTests { #expect(checker.loadCache() == nil) } + @Test("The upgrade instruction matches how mcs is distributed on this platform") + func upgradeInstructionIsPlatformAppropriate() { + let instruction = Constants.MCSRepo.upgradeInstruction + #if canImport(Darwin) + #expect(instruction.contains("brew upgrade")) + #expect(instruction.contains(Constants.MCSRepo.brewFormula)) + #else + // The Linux artifact is a release tarball, so brew is the wrong place to send the user. + #expect(!instruction.contains("brew")) + #expect(instruction.contains(Constants.MCSRepo.releasesURL)) + #endif + } + @Test("invalidateCache deletes the cache file") func invalidateCache() throws { let tmpDir = try makeTmpDir() diff --git a/docs/linux-support.md b/docs/linux-support.md index 3eac7e71..d7a81f57 100644 --- a/docs/linux-support.md +++ b/docs/linux-support.md @@ -21,7 +21,7 @@ ADR-style entry per decision. ## 4. Compatibility matrix -Legend: `verified` — run on Linux and observed; `pending` — not yet exercised. +Legend: `verified` — run on Linux and observed; `verified (with a difference)` — works, but not identically to macOS; the Notes column says how. | Feature | macOS | Linux | Notes | |---|---|---|---| @@ -30,8 +30,8 @@ Legend: `verified` — run on Linux and observed; `pending` — not yet exercise | `--customize` raw-mode picker | supported | verified | Driven under a real PTY: `↓` moved the cursor, `Space` toggled, `Enter` applied; in-place redraw and cursor hide/show correct. | | Non-TTY fallback picker | supported | verified | stdin a PTY, stdout a pipe: numeric toggle + `Enter`, colours disabled. | | `shell:` components | supported | verified | `shell: touch ` created the marker. | -| `shellInteractive: true` (PTY/sudo) | supported | verified | `forkpty` path ran the command; marker file created. Also asserted by `LifecycleIntegrationTests`. | -| `brew:` components | supported | pending | | +| `shellInteractive: true` (PTY/sudo) | supported | verified | `forkpty` path ran the command; marker file created, with stdin from `/dev/null` and under a real controlling terminal. Also asserted by `LifecycleIntegrationTests`. The PTY is allocated with a default 0×0 window size on both platforms. | +| `brew:` components | supported | verified (with a difference) | Same predicate on both platforms. Without Linuxbrew a `brew:` component is satisfied only when the formula name is also the command name — `brew: node` passes with `node` on PATH, `brew: ripgrep` does not because the command is `rg`. Doctor then reports it missing and names the system package manager instead of pointing back at `mcs sync`. Installing a formula still needs Linuxbrew. | | `mcp:` (`claude mcp add`) | supported | verified | `demo-server` registered via `claude mcp add -s local`; `claude mcp list` shows it; removal deregisters it. | | `plugin:` | supported | verified | `hookify@claude-code-plugins` installed through `claude plugin install`; `claude plugin list` shows it enabled. | | `hook:` / `command:` / `skill:` / `agent:` copies | supported | verified | All four installed; hooks namespaced under `/`; interpreter inferred (`bash` for `.sh`, `python3` for `.py`). | @@ -44,14 +44,14 @@ Legend: `verified` — run on Linux and observed; `pending` — not yet exercise | `mcs pack add` (local) | supported | verified | `mcs pack add ` registered in place with `commitSHA: local`. | | `mcs pack remove` / `list` / `update` / `validate` | supported | verified | `remove` unconfigured every affected scope; `update` reported "already up to date"; `validate` produced the python3 heuristic warning. | | Pack trust hashing | supported | verified | Trust prompt shown on add for both a local and a git pack; re-validated on `pack update` (SHA-256 now from swift-crypto). | -| `mcs export` (incl. brew formula hints) | supported | verified | Exported `techpack.yaml`, hooks, skill, command, agent, settings and template from a live project. Brew hints empty without Linuxbrew. | +| `mcs export` (incl. brew formula hints) | supported | verified (with a difference) | Exported `techpack.yaml`, hooks, skill, command, agent, settings and template from a live project. The brew formula hints are empty without Linuxbrew — `detectFormula` reads symlinks under the Linuxbrew prefixes. | | `mcs cleanup` | supported | verified | Found and deleted a `CLAUDE.local.md.backup.*` file, with and without `--force`. | -| `mcs check-updates` + SessionStart hook | supported | verified | `check-updates`, `--json` and `--hook` all run; `mcs config set update-check-cli true` registered `mcs check-updates --hook` in `~/.claude/settings.json` and the cooldown file was written. | +| `mcs check-updates` + SessionStart hook | supported | verified (with a difference) | `check-updates`, `--json` and `--hook` all run; `mcs config set update-check-cli true` registered `mcs check-updates --hook` in `~/.claude/settings.json` and the cooldown file was written. The upgrade instruction differs: `brew upgrade` on macOS, download the release tarball on Linux. | | `mcs config` | supported | verified | `list` / `get` / `set` against `~/.mcs/config.yaml`. | | File lock (`flock`) | supported | verified | Two concurrent syncs: the second exited 1 with "Another mcs process is running". | | Lockfile (`mcs.lock.yaml`) | supported | verified | Written after sync with `generate-lockfile true`; `mcs sync --lock` consumed it. | | Terminal colours / width | supported | verified | ANSI colour and the wrapped/re-rendered picker observed under a PTY; colours suppressed when stdout is a pipe. | -| Claude Code prerequisite | supported | pending | | +| Claude Code prerequisite | supported | verified (with a difference) | With `claude` off PATH, Linux prints the native-installer and npm commands and returns false. macOS still offers the Homebrew install; `claude-code` is a cask and Linuxbrew has no casks. | | Telemetry | removed | removed | Deleted outright — see ADR D1. | ## 5. Decisions (ADR entries) From 4382102220bd37aaee758076d7af5fe596876727 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 18:28:19 +0000 Subject: [PATCH 09/30] Publish a Linux release artifact alongside the macOS universal binary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit build-and-release splits into build-macos, build-linux and release so two artifacts can be produced in parallel and published once. The macOS build, version verification and tarball are unchanged; the Linux job builds on ubuntu-22.04 with swiftly and --static-swift-stdlib, which sets the glibc floor of the published binary at 2.35, and gets the same SPM cache every other job has. The release job downloads both artifacts, checksums them with sha256sum rather than shasum — identical output format, present on every runner, and the tap step no longer runs on macOS — and creates one release carrying both tarballs and a single SHA256SUMS. The Homebrew formula still points at the macOS tarball, so tap users see no change. Measured locally on Linux: the release binary is 95 MB, the tarball holds exactly one file, the unpacked binary prints its version, and ldd shows it still needs libstdc++.so.6 — recorded in the compatibility matrix. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01V9KvZKADJzXojs4fwCgWAR --- .github/workflows/release.yml | 98 +++++++++++++++++++++++++++++++---- docs/linux-support.md | 1 + 2 files changed, 89 insertions(+), 10 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index e9e9e54f..53cbc12c 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -11,6 +11,10 @@ permissions: concurrency: group: release +env: + # Same toolchain the Linux PR job installs; kept in one place per workflow. + SWIFT_VERSION: '6.2.0' + jobs: verify-version: name: Verify Version @@ -67,8 +71,8 @@ jobs: - name: Test run: swift test - build-and-release: - name: Build and Release + build-macos: + name: Build macOS Universal Binary needs: [test] runs-on: macos-latest steps: @@ -99,25 +103,98 @@ jobs: fi echo "Binary verified: ${BUILT_VERSION}" - - name: Prepare artifacts + - name: Prepare artifact + env: + TAG_NAME: ${{ github.ref_name }} + run: tar -czf "mcs-${TAG_NAME}-macos-universal.tar.gz" -C "${BIN_DIR}" mcs + + - uses: actions/upload-artifact@v4 + with: + name: mcs-macos-universal + path: mcs-${{ github.ref_name }}-macos-universal.tar.gz + if-no-files-found: error + + build-linux: + name: Build Linux x86_64 Binary + needs: [test] + # ubuntu-22.04 on purpose: it sets the glibc floor of the published binary at 2.35. + runs-on: ubuntu-22.04 + steps: + - uses: actions/checkout@v6 + + - name: Install Swift via swiftly + run: | + curl -O https://download.swift.org/swiftly/linux/swiftly-$(uname -m).tar.gz + tar zxf "swiftly-$(uname -m).tar.gz" + ./swiftly init --quiet-shell-followup --assume-yes --skip-install + . "$HOME/.local/share/swiftly/env.sh" + swiftly install --use "$SWIFT_VERSION" --assume-yes + echo "$HOME/.local/share/swiftly/bin" >> "$GITHUB_PATH" + + - uses: actions/cache@v5 + with: + path: | + .build/checkouts + .build/repositories + .build/workspace-state.json + key: spm-linux-${{ hashFiles('Package.swift') }} + restore-keys: spm-linux- + + - name: Build release binary + run: | + swift build -c release --static-swift-stdlib + echo "BIN_DIR=$(swift build -c release --static-swift-stdlib --show-bin-path)" >> "$GITHUB_ENV" + + - name: Verify binary + env: + TAG_NAME: ${{ github.ref_name }} + run: | + BUILT_VERSION=$("${BIN_DIR}/mcs" --version) + if [ "$BUILT_VERSION" != "$TAG_NAME" ]; then + echo "::error::Built binary reports version '${BUILT_VERSION}' but tag is '${TAG_NAME}'" + exit 1 + fi + echo "Binary verified: ${BUILT_VERSION}" + + - name: Prepare artifact + env: + TAG_NAME: ${{ github.ref_name }} + run: tar -czf "mcs-${TAG_NAME}-linux-x86_64.tar.gz" -C "${BIN_DIR}" mcs + + - uses: actions/upload-artifact@v4 + with: + name: mcs-linux-x86_64 + path: mcs-${{ github.ref_name }}-linux-x86_64.tar.gz + if-no-files-found: error + + release: + name: Release + needs: [build-macos, build-linux] + runs-on: ubuntu-latest + steps: + - uses: actions/download-artifact@v4 + with: + merge-multiple: true + + - name: Checksum artifacts env: TAG_NAME: ${{ github.ref_name }} run: | - TARBALL="mcs-${TAG_NAME}-macos-universal.tar.gz" - tar -czf "${TARBALL}" -C "${BIN_DIR}" mcs - shasum -a 256 "${TARBALL}" > SHA256SUMS - echo "TARBALL=${TARBALL}" >> "$GITHUB_ENV" + # sha256sum, not shasum: identical " " output and present on every runner. + sha256sum *.tar.gz > SHA256SUMS + cat SHA256SUMS + echo "TARBALL=mcs-${TAG_NAME}-macos-universal.tar.gz" >> "$GITHUB_ENV" - name: Create GitHub Release env: GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} TAG_NAME: ${{ github.ref_name }} - TARBALL: ${{ env.TARBALL }} run: | gh release create "${TAG_NAME}" \ + --repo "${{ github.repository }}" \ --verify-tag \ --generate-notes \ - "${TARBALL}" SHA256SUMS + *.tar.gz SHA256SUMS - name: Update Homebrew tap env: @@ -126,7 +203,8 @@ jobs: TARBALL: ${{ env.TARBALL }} if: env.HOMEBREW_TAP_TOKEN != '' run: | - SHA=$(shasum -a 256 "${TARBALL}" | cut -d' ' -f1) + # The formula keeps pointing at the macOS tarball; Linux ships as a plain release asset. + SHA=$(sha256sum "${TARBALL}" | cut -d' ' -f1) git clone "https://x-access-token:${HOMEBREW_TAP_TOKEN}@github.com/mcs-cli/homebrew-tap.git" tap cd tap diff --git a/docs/linux-support.md b/docs/linux-support.md index d7a81f57..2a51a7a1 100644 --- a/docs/linux-support.md +++ b/docs/linux-support.md @@ -52,6 +52,7 @@ Legend: `verified` — run on Linux and observed; `verified (with a difference)` | Lockfile (`mcs.lock.yaml`) | supported | verified | Written after sync with `generate-lockfile true`; `mcs sync --lock` consumed it. | | Terminal colours / width | supported | verified | ANSI colour and the wrapped/re-rendered picker observed under a PTY; colours suppressed when stdout is a pipe. | | Claude Code prerequisite | supported | verified (with a difference) | With `claude` off PATH, Linux prints the native-installer and npm commands and returns false. macOS still offers the Homebrew install; `claude-code` is a cask and Linuxbrew has no casks. | +| Release artifact | `.tar.gz` (universal) | verified (with a difference) | `swift build -c release --static-swift-stdlib` produces one 95 MB binary; the tarball holds exactly one file and `./mcs --version` prints the version. `ldd` still shows `libstdc++.so.6`, `libgcc_s.so.1`, `libm`, `libc` and `ld-linux`. | | Telemetry | removed | removed | Deleted outright — see ADR D1. | ## 5. Decisions (ADR entries) From c752e4f619e894f4ffeeca5b575f826845b4e566 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 18:32:42 +0000 Subject: [PATCH 10/30] Document Linux support, decisions, and platform differences MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit docs/linux-support.md is completed: supported configurations and the glibc floor, the prerequisites mcs assumes (/usr/bin/which, /usr/bin/env, /bin/bash, libstdc++6) with the failure symptom each one has, how to install and build on Linux, the per-feature compatibility matrix filled in from real runs, one ADR-style entry per decision D1–D10 with the options considered and what each one costs, the known limitations, and the rules for adding a platform-specific path so this does not spread beyond the four files that own it. The existing docs stop implying mcs is macOS-only: README gains a Linux badge, a split install section and a supported-platforms line; architecture.md gains the two new Core types, the platform-dependent brew prefix and a platform section; troubleshooting.md makes the Homebrew and Claude Code entries platform-aware and adds the two Linux-specific symptoms — "everything reports not found" when /usr/bin/which is missing, and a bare binary that is not on PATH. creating-tech-packs.md and techpack-schema.md state the `brew:` rule pack authors need: on Linux without Linuxbrew a component is satisfied only when the formula name is also the command name, and there is no per-platform gating yet. CLAUDE.md drops the removed telemetry key, lists Locked and TerminalAttributes, and adds a platform-support rule under Code Style. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01V9KvZKADJzXojs4fwCgWAR --- CLAUDE.md | 18 +- README.md | 20 ++- docs/README.md | 4 +- docs/architecture.md | 18 +- docs/creating-tech-packs.md | 4 +- docs/linux-support.md | 327 +++++++++++++++++++++++++++++++++++- docs/techpack-schema.md | 10 +- docs/troubleshooting.md | 33 +++- 8 files changed, 413 insertions(+), 21 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index bf08db58..ed396374 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -12,7 +12,8 @@ Swift CLI tool (`mcs`) that configures Claude Code with MCP servers, plugins, sk # Development swift build # Build the CLI swift test # Run tests -swift build -c release --arch arm64 --arch x86_64 # Universal binary +swift build -c release --arch arm64 --arch x86_64 # macOS universal binary +swift build -c release --static-swift-stdlib # Linux release binary # CLI usage (after install) mcs sync [path] # Sync project: multi-select packs, compose artifacts (default command) @@ -60,7 +61,7 @@ mcs config set # Set a configuration value (true/false) ## Architecture ### Swift Package Structure -- **Package.swift** — swift-tools-version: 6.0, macOS 13+, deps: swift-argument-parser, Yams +- **Package.swift** — swift-tools-version: 6.0, macOS 13+ and Linux (glibc), deps: swift-argument-parser, Yams, swift-crypto (Linux only) - **Sources/mcs/** — main executable target - **Tests/MCSTests/** — test target @@ -80,6 +81,8 @@ mcs config set # Set a configuration value (true/false) - `Homebrew.swift` — brew detection, package install/uninstall, and `provides(_:)` — the one availability predicate shared by `ComponentExecutor` and `BrewPackageCheck` (PATH under `bareName(of:)`, falling back to `brew list`) - `FileHasher.swift` — SHA-256 file and directory hashing via CryptoKit (used by `PackTrustManager` and `ComponentExecutor`) - `FileLock.swift` — POSIX `flock()` process lock and `LockedCommand` protocol for mutually exclusive CLI commands +- `Locked.swift` — `NSLock`-backed value box with the `withLock { $0 }` shape of the Darwin-only `OSAllocatedUnfairLock` (`Synchronization.Mutex` is macOS 15+, above the macOS 13 floor) +- `TerminalAttributes.swift` — termios helpers for the raw-mode picker; `c_cc` is indexed through the platform's own `VMIN`/`VTIME` and flag masks are widened through `tcflag_t` - `Lockfile.swift` — `mcs.lock.yaml` model for pinning pack commits - `PathContainment.swift` — centralized path-boundary checks and relative-path utilities (symlink-safe containment, traversal prevention) - `PluginRef.swift` — parsed `name@repo` plugin references with marketplace resolution @@ -88,7 +91,7 @@ mcs config set # Set a configuration value (true/false) - `ProjectState.swift` — per-project `.claude/.mcs-project` JSON state (configured packs, per-pack `PackArtifactRecord` with ownership tracking, version) - `ProjectIndex.swift` — cross-project index (`~/.mcs/projects.yaml`) mapping project paths to pack IDs for reference counting - `MCSError.swift` — error types for the CLI -- `MCSConfig.swift` — user preferences (`~/.mcs/config.yaml`): `update-check-packs`, `update-check-cli`, `telemetry`, `generate-lockfile` +- `MCSConfig.swift` — user preferences (`~/.mcs/config.yaml`): `update-check-packs`, `update-check-cli`, `generate-lockfile` - `UpdateChecker.swift` — pack freshness checks (`git ls-remote`), CLI version checks (`git ls-remote --tags`), cooldown management ### TechPack System (`Sources/mcs/TechPack/`) @@ -180,6 +183,15 @@ swiftlint --fix - CI runs both in strict mode (warnings become errors) with GitHub Actions inline annotations - SwiftLint excludes `Tests/` — only Sources and Package.swift are linted - **Never use `try?` to silently discard errors** — use `do/catch` and surface the error (via `output.warn()`, logging, or propagation). `try?` hides root causes and makes debugging impossible. The only acceptable use is when the absence of a result is the *entire* semantic meaning (e.g., `FileManager.fileExists` alternative) +- **Platform support**: `mcs` builds for macOS 13+ and Linux (glibc). The canonical shape is +```swift +#if canImport(Darwin) +import Darwin +#elseif canImport(Glibc) +import Glibc +#endif +``` + Platform knowledge lives **only** in `Core/TerminalAttributes.swift`, `Core/Environment.swift`, `Core/Homebrew.swift` and `Core/Constants.swift` — everything else calls into them. A `#if` is for a value or API that genuinely differs, never to make a diagnostic go away; the same goes for `_ =`. Several Foundation methods are `@discardableResult` on Darwin and not on Linux — use the result, it always means something. A test gated out on one platform is a coverage regression: give the `#else` branch the equivalent assertion. Rationale and the compatibility matrix are in `docs/linux-support.md` - **Comments carry the non-obvious "why", not a narration of the code** — don't restate the line below, don't describe what the code used to do. If the code already says it, delete the comment; if the rationale needs more than a line or two, it belongs in the issue or a memory. State a given rationale once, at the site that owns it, rather than repeating it at every call site ## Testing diff --git a/README.md b/README.md index a394d56e..5717b282 100644 --- a/README.md +++ b/README.md @@ -10,6 +10,7 @@ [![Swift 6.0+](https://img.shields.io/badge/Swift-6.0+-F05138.svg?logo=swift&logoColor=white)](https://swift.org) [![macOS](https://img.shields.io/badge/macOS-13+-000000.svg?logo=apple&logoColor=white)](https://www.apple.com/macos/) +[![Linux](https://img.shields.io/badge/Linux-glibc-FCC624.svg?logo=linux&logoColor=black)](docs/linux-support.md) [![License: MIT](https://img.shields.io/badge/License-MIT-blue.svg)](LICENSE) [![Homebrew](https://img.shields.io/badge/Homebrew-tap-FBB040.svg?logo=homebrew&logoColor=white)](https://github.com/mcs-cli/homebrew-tap) [![Tech Packs](https://img.shields.io/badge/Tech_Packs-Browse-8B5CF6.svg)](https://techpacks.mcs-cli.dev) @@ -21,10 +22,23 @@ ### 1. Install +**macOS** — Homebrew: + ```bash brew install mcs-cli/tap/mcs ``` +**Linux** (x86_64, glibc 2.35+) — download the release tarball: + +```bash +tar -xzf mcs--linux-x86_64.tar.gz +install -m 0755 mcs ~/.local/bin/mcs +``` + +Supported platforms: macOS 13+ and Linux (glibc, x86_64). See +[Linux support](docs/linux-support.md) for prerequisites, the compatibility matrix and the +handful of behaviours that differ. + ### 2. Add tech packs Browse available packs at **[techpacks.mcs-cli.dev](https://techpacks.mcs-cli.dev)**, then add them: @@ -135,7 +149,7 @@ Use `mcs pack validate` to check your pack for structural errors and best practi ## 🔍 Verifying Your Setup with Poirot -After `mcs sync`, want to confirm everything landed correctly? [**Poirot**](https://github.com/leonardocardoso/poirot) is a native macOS companion that gives you a visual overview of your Claude Code configuration — MCP servers, settings, sessions, and more — all in one place. +After `mcs sync`, want to confirm everything landed correctly? [**Poirot**](https://github.com/leonardocardoso/poirot) (macOS only) is a native companion that gives you a visual overview of your Claude Code configuration — MCP servers, settings, sessions, and more — all in one place. The perfect complement to `mcs`: configure your environment with `mcs`, then use Poirot to see exactly what's installed and running. @@ -150,6 +164,7 @@ The perfect complement to `mcs`: configure your environment with `mcs`, then use | 📋 [Tech Pack Schema](docs/techpack-schema.md) | Complete `techpack.yaml` field reference | | 🏗️ [Architecture](docs/architecture.md) | Internal design, sync flow, safety guarantees, and extension points | | 🔧 [Troubleshooting](docs/troubleshooting.md) | Common issues and fixes | +| 🐧 [Linux support](docs/linux-support.md) | Supported configurations, prerequisites, compatibility matrix, and porting decisions | --- @@ -158,7 +173,8 @@ The perfect complement to `mcs`: configure your environment with `mcs`, then use ```bash swift build # Build swift test # Run tests -swift build -c release --arch arm64 --arch x86_64 # Universal binary +swift build -c release --arch arm64 --arch x86_64 # macOS universal binary +swift build -c release --static-swift-stdlib # Linux release binary ``` See [Architecture](docs/architecture.md) for project structure and design decisions. diff --git a/docs/README.md b/docs/README.md index 0b26d3de..20b5e504 100644 --- a/docs/README.md +++ b/docs/README.md @@ -22,6 +22,7 @@ Creating your own shareable Claude Code configuration: How mcs works under the hood: 1. [Architecture](architecture.md) — Package structure, sync flow, convergence model, safety guarantees, and extension points +2. [Linux support](linux-support.md) — Supported configurations, prerequisites, the per-feature compatibility matrix, and the decisions behind every platform branch ## Quick Links @@ -36,7 +37,8 @@ How mcs works under the hood: | Diagnose issues | [CLI > mcs doctor](cli.md#mcs-doctor) | | Understand internals | [Architecture](architecture.md) | | Fix a problem | [Troubleshooting](troubleshooting.md) | +| Run mcs on Linux | [Linux support](linux-support.md) | --- -[Home](README.md) | [CLI Reference](cli.md) | [Creating Tech Packs](creating-tech-packs.md) | [Schema](techpack-schema.md) | [Architecture](architecture.md) | [Troubleshooting](troubleshooting.md) +[Home](README.md) | [CLI Reference](cli.md) | [Creating Tech Packs](creating-tech-packs.md) | [Schema](techpack-schema.md) | [Architecture](architecture.md) | [Troubleshooting](troubleshooting.md) | [Linux support](linux-support.md) diff --git a/docs/architecture.md b/docs/architecture.md index b0766d96..65c2b0fc 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -5,7 +5,7 @@ This document describes the internal architecture of `mcs` for contributors and ## Package Structure ``` -Package.swift # swift-tools-version: 6.0, macOS 13+ +Package.swift # swift-tools-version: 6.0, macOS 13+ and Linux (glibc) Sources/mcs/ CLI.swift # @main entry, version, subcommand registration Core/ # Shared infrastructure @@ -31,7 +31,7 @@ The primary command is **`mcs sync`**, which handles both global and per-project ### Environment (`Core/Environment.swift`) -Central path resolution for all file locations. Detects architecture (arm64/x86_64), resolves Homebrew path, and locates the user's shell RC file. Key paths: +Central path resolution for all file locations. Detects architecture (arm64/x86_64), resolves the Homebrew path, and locates the user's shell RC file. The Homebrew prefix is platform-dependent — `/opt/homebrew` or `/usr/local` on macOS, `/home/linuxbrew/.linuxbrew` on Linux — and is derived from a `brew` on `PATH` when there is one, without resolving symlinks (see [Linux support, D8](linux-support.md#d8--homebrew-on-linux)). Key paths: - `~/.claude/` — Claude Code configuration directory - `~/.claude/settings.json` — user settings (global) @@ -95,6 +95,14 @@ Written by `mcs sync` after convergence. | **Format** | JSON | JSON | | **Tracks** | Globally installed components, pack IDs, file hashes | Per-pack artifact records, configured pack IDs | +### Locked (`Core/Locked.swift`) + +A value guarded by an `NSLock`, with the `withLock { $0 }` shape of `OSAllocatedUnfairLock`, which is Darwin-only. Used by `WarningCounter` and by `ScriptRunner`'s timeout flag. `Synchronization.Mutex` would be the modern answer but is macOS 15+, above this package's deployment target. + +### TerminalAttributes (`Core/TerminalAttributes.swift`) + +termios helpers for the raw-mode picker. `c_cc` is a fixed-size tuple whose length and element order are platform-defined and `tcflag_t` differs in width, so the control-character index comes from the platform's own `VMIN`/`VTIME` and every flag mask is widened through `tcflag_t`. + ### Backup (`Core/Backup.swift`) Before modifying files with user content (e.g., `CLAUDE.local.md`), a timestamped backup is created (e.g., `CLAUDE.local.md.backup.20260222_143000`). Tool-managed files are not backed up since they can be regenerated. The `mcs cleanup` command discovers and deletes these backups. @@ -368,9 +376,13 @@ The command (`Commands/ExportCommand.swift`) is a read-only `ParsableCommand` (n The codebase uses Swift 6's strict concurrency. All core types conform to `Sendable`. `TechPack` is a `Sendable` protocol. No mutable global state exists outside the installer's in-progress mutation context. +## Platform Support + +`mcs` builds for macOS 13+ and for Linux (glibc, x86_64). Platform-dependent knowledge is confined to `Core/TerminalAttributes.swift`, `Core/Environment.swift`, `Core/Homebrew.swift` and `Core/Constants.swift`; everything else calls into them. `swift-crypto` is a Linux-only target dependency supplying SHA-256, which Foundation does not have there. See [Linux support](linux-support.md) for the compatibility matrix, the decisions behind each platform branch, and the rules for adding a new one. + --- -**Next**: Having issues? See [Troubleshooting](troubleshooting.md). +**Next**: Having issues? See [Troubleshooting](troubleshooting.md). Running on Linux? See [Linux support](linux-support.md). --- diff --git a/docs/creating-tech-packs.md b/docs/creating-tech-packs.md index f892eb33..2f556116 100644 --- a/docs/creating-tech-packs.md +++ b/docs/creating-tech-packs.md @@ -122,6 +122,8 @@ components: When a user runs `mcs sync`, these get installed via `brew install`. The engine auto-verifies them with `mcs doctor`, which looks for the command on `PATH` and falls back to `brew list` — so a cask, a versioned formula like `node@22`, or a formula whose command is spelled differently (`ripgrep` installs `rg`) still verifies correctly. +> **On Linux, Homebrew is usually absent.** Verification then has only the `PATH` probe, so a `brew:` component is satisfied only when the **formula name is also the command name**: `brew: node` passes when `node` is on `PATH`, `brew: ripgrep` does not, because the command is `rg`. mcs cannot install it either — that needs [Linuxbrew](https://docs.brew.sh/Homebrew-on-Linux) — so `mcs doctor` reports the package missing and tells the user to install it with their system package manager. Prefer formulae whose name matches their command when you can, and say in your pack's description which tools a Linux user has to install themselves. There is no per-platform component gating in `techpack.yaml` yet; see [Linux support](linux-support.md). + A tap-qualified package (`owner/tap/formula`) works too, but note that installing one taps a third-party repository without asking anyone; `mcs pack validate` warns when your pack declares one. Need to depend on Homebrew itself? That's a special case — Homebrew can't install itself, so use `shell:` with an explicit doctor check: @@ -129,7 +131,7 @@ Need to depend on Homebrew itself? That's a special case — Homebrew can't inst ```yaml - id: homebrew displayName: Homebrew - description: macOS package manager + description: macOS package manager (this component is macOS-only in practice) type: brewPackage shell: '/bin/bash -c "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)"' doctorChecks: diff --git a/docs/linux-support.md b/docs/linux-support.md index 2a51a7a1..6571c12b 100644 --- a/docs/linux-support.md +++ b/docs/linux-support.md @@ -9,15 +9,79 @@ ADR-style entry per decision. ## 1. Status and supported configurations -*(Phase 10)* +`mcs` is built and tested on **Linux x86_64 with glibc**. Every feature works, and section 4 records +how each one was verified; five features behave differently from macOS, and the matrix says how. + +| | | +|---|---| +| Architecture | x86_64. No aarch64 artifact is published yet. | +| libc | glibc. musl is untested; there is no `canImport(Musl)` branch. | +| glibc floor | 2.35 — the published binary is built on Ubuntu 22.04. | +| Tested on | Ubuntu 24.04 (development) and the `ubuntu-latest` GitHub runner (CI). | +| Other distributions | Any glibc ≥ 2.35 distribution is expected to work. **Nothing is claimed about Fedora, Alpine or NixOS** — they have not been tested. | + +CI runs `swift build` and the full test suite on Linux for every pull request, alongside the two +macOS jobs. Lint runs on macOS only: SwiftFormat and SwiftLint give the same verdicts on both, so a +second run would only add version skew. ## 2. Prerequisites -*(Phase 10)* +`mcs` shells out to a few tools by absolute path and assumes they exist: + +| Path | Used for | Present on | +|---|---|---| +| `/usr/bin/which` | resolving every command name to a path | Ubuntu/Debian (`debianutils`), most distributions | +| `/usr/bin/env` | invoking the `claude` CLI | coreutils | +| `/bin/bash` | running `shell:` components and pack scripts | every mainstream distribution | +| `libstdc++.so.6` | the published binary links against it dynamically | most distributions; minimal containers need `apt-get install -y libstdc++6` | + +**If `/usr/bin/which` is missing** (it is absent on NixOS, and Debian has been retiring it), mcs does +not error — it reports an empty machine. `ShellRunner.resolvedPath` returns `nil` for *every* +command, so `Homebrew.provides`, the Claude Code prerequisite and every command-based doctor check +report "not found". The symptom is that `mcs doctor` says nothing is installed on a machine where +everything is. See ADR D9 for why this is documented rather than worked around. + +Homebrew is **optional** on Linux. Without it, `brew:` components are verified through `PATH` only +and cannot be installed by mcs — see the `brew:` row in section 4 and ADR D8. ## 3. Install, build and test on Linux -*(Phase 10)* +### Install + +Download the tarball for the release you want from +, unpack it, and put `mcs` on your `PATH`: + +```bash +tar -xzf mcs--linux-x86_64.tar.gz +install -m 0755 mcs ~/.local/bin/mcs +mcs --version +``` + +There is no Homebrew formula for Linux: the tap publishes the macOS universal binary only. +`mcs check-updates` therefore tells Linux users to download the latest release rather than to run +`brew upgrade`. + +### Build from source + +```bash +# Install a toolchain, e.g. via swiftly (https://swift.org/install/linux/) +swift build +swift test +swift build -c release --static-swift-stdlib # what the release workflow ships +``` + +`swift test` output does not display in some terminals, so redirect it and read the file: + +```bash +mkdir -p .test-output && swift test > .test-output/results.txt 2>&1 +``` + +`.test-output/` is gitignored. + +**Do not run the suite as root.** `UpdateCheckerTests.registryWriteFailureContract` makes a +directory read-only and asserts that a write into it fails; root ignores directory permissions, so +that one test fails under `sudo` or in a root container. This is why the CI job runs on the +`ubuntu-latest` runner rather than in a `swift:*` container image. ## 4. Compatibility matrix @@ -57,12 +121,263 @@ Legend: `verified` — run on Linux and observed; `verified (with a difference)` ## 5. Decisions (ADR entries) -*(Phase 10 — one entry per D1…D10)* +Each entry records the context, the options considered, the decision, and what it costs. + +### D1 — TelemetryDeck, `MCSAnalytics` and the `telemetry` config key are deleted + +**Context.** The TelemetryDeck SDK is Apple-only, and the anonymous user id was derived from IOKit's +`IOPlatformUUID`. Together they were the first hard blocker for a Linux build: 24 call sites across +nine command files, one config key, and one dependency. + +**Options considered.** (a) Delete everything. (b) Keep the config key with a no-op implementation. +(c) Keep the key, delete the implementation. (d) `#if canImport(IOKit)`-gate it so macOS keeps +sending signals. + +**Decision: (a).** A config key that accepts `true` and does nothing lies to the user in +`mcs config list`, and 24 no-op call sites are exactly the dead narration the repo's comment rule +exists to prevent. (d) would fork behaviour between platforms permanently and keep an Apple-only +package in the dependency graph for one feature. + +**Consequences.** +- On disk it is backward compatible: `MCSConfig` is a synthesized `Codable` struct, and synthesized + decoders ignore unknown keys, so an existing `~/.mcs/config.yaml` containing `telemetry: false` + still loads. Pinned by a test. +- **The first `mcs config set ` after upgrading re-encodes the struct and silently drops + the user's `telemetry:` line.** Harmless, but it is a real change to a file the user owns. +- `mcs config set telemetry false` now reports an unknown key. The key was never documented in the + CLI reference. +- `~/.mcs/.telemetry-noticed` is orphaned. It is left in place: deleting other people's files to + tidy up is worse than an empty file. +- Every command got faster on macOS too — `trackCommand` ended with a 200 ms `RunLoop` wait. +- Telemetry can be restored from git history in one commit if it is ever wanted back. + +### D2 — swift-crypto is a Linux-only target dependency; CryptoKit stays on Darwin + +**Context.** Three files hash with `SHA256.hash(data:)` from CryptoKit. Foundation has no SHA-256 on +Linux, so a dependency is unavoidable. + +**Options considered.** (a) An unconditional swift-crypto dependency and `import Crypto` everywhere. +(b) A conditional *target* dependency plus a `#if canImport(CryptoKit)` import shim. (c) Hand-roll +SHA-256. (d) Shell out to `sha256sum`. + +**Decision: (b).** + +```swift +.product(name: "Crypto", package: "swift-crypto", condition: .when(platforms: [.linux])) +``` + +**Why not (a).** On Darwin the `Crypto` target declares `resources: [.copy("PrivacyInfo.xcprivacy")]`, +so putting it in the macOS graph makes SwiftPM emit a `swift-crypto_Crypto.bundle` next to the +release binary — and the release workflow tars a single file. (b) keeps the tarball one file. +Separately, macOS digests stay on the system CryptoKit, which is the "bit-identical to today" +guarantee for the hashes persisted in `.mcs-project` state and used for drift detection. +(It is *not* true that swift-crypto compiles BoringSSL on macOS: its manifest gates +`CCryptoBoringSSL` on `[.linux, .android, .windows, .wasi]`.) + +**Why not (c)/(d).** Hand-rolled crypto is a maintenance liability even for a hash; shelling out +costs a process per file and makes `FileHasher` untestable offline. + +**Consequences.** `swift package resolve` fetches **two** more repositories on macOS — swift-crypto +and its transitive `swift-asn1`. The CI cache key is `hashFiles('Package.swift')`, so that is paid +once per manifest change. `Package.resolved` is gitignored, so `from: "3.0.0"` floats across 3.x — +the same exposure the two existing dependencies already have. `FileHasherTests` and +`SettingsHasherTests` each pin a known digest, so a divergence between the two implementations fails +the suite instead of silently invalidating state files. + +### D3 — `Locked` replaces `OSAllocatedUnfairLock` + +**Context.** `import os` is Darwin-only. Two sites used `OSAllocatedUnfairLock`: the warning counter +behind `CLIOutput` and a one-shot timeout flag in `ScriptRunner`. + +**Options considered.** (a) `NSLock` inline at both sites. (b) One small `Locked`. +(c) `Synchronization.Mutex`. (d) A serial `DispatchQueue`. (e) An actor. + +**Decision: (b).** One named type reads better than two bespoke lock/unlock pairs, and it is twenty +lines. **Why not (c):** `Mutex` is macOS 15+, and this package's floor is macOS 13 — raising it would +drop macOS 13 and 14 users for a lock. **Why not (e):** `ScriptRunner` reads its flag synchronously +after `waitUntilExit()`; an actor forces `await` into a synchronous path. + +**Consequences.** `NSLock` is marginally slower than an unfair lock. Both sites are cold. The +`@unchecked Sendable` on `Locked` is the type's purpose rather than a way to quiet the checker: the +invariant — every access happens under the lock — is real and cannot be expressed to the compiler, +and the doc comment says so at the one site that owns it. + +### D4 — Platform imports are an explicit `#if canImport` chain, per file + +**Context.** Five files call libc directly. On macOS they got those symbols from Foundation's Darwin +re-export or an outright `import Darwin`. + +**Options considered.** (a) A chain in each file. (b) A `Platform.swift` with `@_exported import`. +(c) A C shim target. + +**Decision: (a)**, verbatim in each of the five files: + +```swift +#if canImport(Darwin) +import Darwin +#elseif canImport(Glibc) +import Glibc +#endif +``` + +**Why not (b).** `@_exported` is an underscored attribute, and it hides from every reader of +`ShellRunner.swift` that the file depends on libc. **Why not (c).** Nothing is missing from +SwiftGlibc — see D5. No `canImport(Musl)` branch is added: nothing here builds for musl, and an +untested branch is worse than no branch. + +### D5 — `forkpty()` is called from Glibc; no C shim, no `posix_openpt` rewrite + +**Context.** `ShellRunner.runInteractive` allocates a real PTY so `sudo` can read a password. The +open question was whether SwiftGlibc exposes `forkpty`. + +**Decision: it does.** `pty.h` is in `SwiftGlibc.h.gyb`, the Glibc modulemap declares `link "util"`, +and the call links and runs on glibc 2.39 with no extra linker flag. `cfmakeraw` is exposed too. + +**Why not the alternatives.** A C shim or `@_silgen_name` would work around a declaration that is +not missing; reimplementing with `posix_openpt`/`grantpt`/`setsid`/`ioctl(TIOCSCTTY)` duplicates +forty lines of subtle libc behaviour for no benefit. + +**Consequences.** One real difference did surface inside the fork child: Glibc types `strdup` as +returning an optional where Darwin implicitly unwraps it, so the child was left dereferencing +optionals for the command path and the working directory. A force-unwrap there would trap through +the Swift runtime after `fork()`, where only async-signal-safe calls are legal, so both strings are +now allocated and checked in the parent. If a future toolchain drops `pty.h` the build fails +immediately and the C shim remains a ~15-line fallback. + +### D6 — termios: index `c_cc` through `VMIN`/`VTIME`, widen masks through `tcflag_t` + +**Context.** The raw-mode picker set `raw.c_cc.16` and `raw.c_cc.17` with `// VMIN` / `// VTIME` +comments, and masked with `~UInt(ICANON | ECHO)`. Both are Darwin facts: `tcflag_t` is `UInt` there +and `UInt32` on Glibc, and `c_cc` is a tuple whose length and order differ (`NCCS` is 20 against 32, +`VMIN` is index 16 against 6). + +**Options considered.** (a) `#if canImport(Darwin) raw.c_cc.16 = 1 #else raw.c_cc.6 = 1 #endif`. +(b) A small `TerminalAttributes` type that binds `c_cc` as `cc_t` memory and subscripts it with the +platform's own constant. + +**Decision: (b).** (a) is a `#if` hiding a real difference behind two magic numbers; indexing by +`VMIN` has no second place to get wrong. `withUnsafeMutableBytes` + `bindMemory(to: cc_t.self)` is +sound — `c_cc` is already a tuple of `cc_t` — and nothing in Foundation or the stdlib does it for +you. `TerminalAttributesTests` asserts the same things on both platforms without needing a TTY, +which is why the read accessor exists; its doc comment says so, so it is not deleted as dead code. + +### D7 — `TIOCGWINSZ` widens to `UInt` + +`TIOCGWINSZ` imports as `Int32` on Glibc and `UInt` on Darwin, and `ioctl` takes a `UInt` request on +both. `ioctl(STDOUT_FILENO, UInt(TIOCGWINSZ), &ws)` compiles everywhere and is a no-op conversion on +Darwin, so no `#if` is needed. In the same file `Darwin.read(...)` loses its module qualifier: +unqualified `read` resolves to libc on both platforms, because `CLIOutput` has a `write` member but +no `read` member. + +### D8 — Homebrew on Linux + +**Context.** mcs resolves a Homebrew prefix, puts its `bin` on the PATH it hands to subprocesses, +and verifies `brew:` components. Linuxbrew installs at `/home/linuxbrew/.linuxbrew`, and most Linux +users have no Homebrew at all. + +**Decisions.** + +1. **The prefix derivation no longer resolves symlinks.** It takes two components up from + `$PREFIX/bin/brew`. Homebrew's installer creates `$PREFIX/bin/brew -> ../Homebrew/bin/brew` on + Linux *and on Intel macOS*; only arm64 macOS has a real file there. Resolving first yielded + `$PREFIX/Homebrew`, whose `bin` holds only `brew`, so `pathWithBrew` prepended an empty directory + and hid `$PREFIX/bin`, where every formula's symlink lives — breaking the PATH-first probe that + `brew:` components rest on. This is also a latent fix for Intel macOS. +2. **The fallback prefix and `allPrefixes` are platform-aware.** Linux gets + `/home/linuxbrew/.linuxbrew` and `~/.linuxbrew`. No `brew --prefix` subprocess is spawned. +3. **`Homebrew.provides` does not change.** It probes PATH under the name with any tap qualifier + stripped, then asks `brew list`. **The consequence, stated plainly: on Linux without Linuxbrew a + `brew:` component is satisfied only when the formula name is also the command name.** `brew: node` + passes when `node` is on PATH; `brew: ripgrep` does not, because the command is `rg`. There is no + alias table, because no function can map formula names to command names in general; adding + heuristics would trade a clear rule for a list that is always incomplete. +4. **The messages say what actually works.** When Homebrew is absent, telling the user to run + `mcs sync` is a loop with no exit: sync prints "Homebrew not found" and sends them back to doctor. + `ComponentExecutor` (install and uninstall) and `BrewPackageCheck.fix()` now name the package and + the system package manager. The command-exists check drops its `mcs sync` hint entirely, because + nothing there knows which component — if any — would install that command. +5. **`ensureClaudeCLI` never offers Homebrew on Linux.** `claude-code` is a Homebrew *cask*, and + Linuxbrew has no casks, so the offer would have failed on exactly the machines that have brew. + Linux prints the native-installer and npm commands and returns `false`. + +**Rejected.** Auto-installing Linuxbrew (a package manager installing a package manager, unprompted) +and auto-installing Claude Code via `curl | bash` or npm (a trust decision mcs cannot make for the +user; npm's global prefix may be root-owned). Also rejected, for now: a `platforms:` key in +`techpack.yaml` so a pack could mark a component macOS-only. It is the right long-term answer, but it +is a manifest schema change with ecosystem impact and is not needed to make mcs work on Linux. + +### D9 — `/usr/bin/which` stays, documented as a prerequisite + +**Context.** `ShellRunner.resolvedPath` and `Environment.resolveCommand` shell out to +`/usr/bin/which`. It is present on Ubuntu 24.04, absent on NixOS, and Debian has been retiring it. + +**Options considered.** (a) Keep it and document it. (b) Replace it with an in-process PATH scan. +(c) Keep it and add a `mcs doctor` check that fails loudly when it is missing. + +**Decision: (a).** (b) widens a portability PR with a behaviour change to a hot path shared by every +command. (c) sounds small but is not: `FileExistsCheck.fix()` returns "Run 'mcs sync' to install", +which is exactly the dead-end message decision D8.4 removes, so it would need a new parameter; the +check would then print a permanently-passing line in every `mcs doctor` run on *both* platforms; and +touching doctor mandates an integration test. Three files and a test, for a condition that does not +occur on the platforms mcs ships for. + +**Consequences.** The failure mode is documented in section 2 rather than detected: with no `which`, +mcs reports an empty machine instead of erroring. + +### D10 — No Subprocess 1.0, no tools-version bump, macOS 13 floor stays + +swift-tools-version 6.0 already supports `.product(…, condition: .when(platforms:))`, and +swift-crypto is tools-version 5.9 / macOS 10.15, so nothing forces a bump. Adopting Subprocess 1.0 +would rewrite `ShellRunner`, `ScriptRunner`, `PackFetcher`, `ClaudeIntegration` and `UpdateChecker` +against an async API, push `async` through `ParsableCommand.run()`, and require re-verifying the PTY +story — a rewrite of the process layer in the middle of a port, for no benefit to this goal. The +platforms list stays `[.macOS(.v13)]`; Linux needs no entry. ## 6. Known limitations -*(Phase 10)* +- **x86_64 only.** No aarch64 Linux tarball is published yet. +- **glibc ≥ 2.35**, because the release binary is built on Ubuntu 22.04. musl is untested. +- **The binary is ~95 MB and needs `libstdc++6`.** `--static-swift-stdlib` links the *Swift* runtime + statically only; `ldd` still shows `libstdc++.so.6`, `libgcc_s.so.1`, `libm`, `libc` and + `ld-linux`. A minimal container needs `apt-get install -y libstdc++6`. A fully static binary would + need the Static Linux SDK (musl), which would require a `canImport(Musl)` branch in every import + chain and a musl verification pass. +- **`/usr/bin/which` is required**, with the "everything reports not found" symptom described in + section 2. +- **No Linuxbrew auto-install**, and **no Claude Code auto-install** on Linux. +- **`brew:` components are name-sensitive** without Linuxbrew — see D8 decision 3. +- **`mcs export`'s brew formula hints are empty** without Linuxbrew: `detectFormula` reads symlinks + under the Homebrew prefixes. +- **The PTY is allocated with a 0×0 window size** — the same on macOS, so full-screen TUIs run inside + a `shellInteractive` component see no size. Line-oriented programs like `sudo` are unaffected. +- **Telemetry is removed on both platforms.** `mcs config set telemetry` now reports an unknown key, + the first `config set` rewrites `config.yaml` without the stale line, and `~/.mcs/.telemetry-noticed` + is left in place. +- **`techpack.yaml` cannot mark a component macOS-only.** A pack declaring `brew: mas` will run on + Linux and fail that component with a clear message. See D8's rejected options. +- **Nothing is claimed about Fedora, Alpine or NixOS** — they are untested. ## 7. How to add a platform-specific path -*(Phase 10)* +The canonical shape, which the repo's `.swiftformat` (`--ifdef noindent`) leaves alone: + +```swift +#if canImport(Darwin) +import Darwin +#elseif canImport(Glibc) +import Glibc +#endif +``` + +Three rules keep this from spreading: + +1. **Platform knowledge lives in four places only** — `Core/TerminalAttributes.swift`, + `Core/Environment.swift`, `Core/Homebrew.swift` and `Core/Constants.swift`. Everything else calls + into them. If a fifth file needs a `#if`, that is a sign the value belongs in one of these. +2. **A `#if` is for a value or API that genuinely differs**, never for making a diagnostic go away. + The same applies to `_ =`, `try?`, `@unchecked` and `nonisolated(unsafe)`: fix the cause. When a + Foundation method is `@discardableResult` on Darwin and not on Linux, use the result — it always + means something. +3. **Write the assertion for both platforms.** A test that is `#if`-gated out on one platform is a + coverage regression; give the `#else` branch the equivalent assertion, as + `TerminalAttributesTests` and `EnvironmentTests` do. diff --git a/docs/techpack-schema.md b/docs/techpack-schema.md index e3a4d6d0..6628f9e1 100644 --- a/docs/techpack-schema.md +++ b/docs/techpack-schema.md @@ -66,6 +66,14 @@ Tap-qualified (`owner/tap/formula`) and versioned (`node@22`) names work, as do passed to `brew` verbatim; only the PATH probe behind the auto-derived check uses the last path component. +> **Linux**: verification falls back to `brew list` only when Homebrew is installed, which on Linux +> it usually is not. Without it a `brew:` component is satisfied only when the formula name is also +> the command name (`node` yes, `ripgrep` → `rg` no), and `mcs` cannot install the formula — that +> needs Linuxbrew. `mcs doctor` reports the package missing and points the user at their system +> package manager. There is no per-platform component gating in this schema yet, so a macOS-only +> formula will simply fail its component on Linux with that message. See +> [Linux support](linux-support.md). + > **Tapped formulae install from an unaudited source.** `brew install owner/tap/formula` clones the > tap's repository and evaluates its formula. Homebrew taps it without asking, and `mcs` runs `brew` > without a terminal, so Homebrew's own plan prompt cannot appear either. Nothing in `mcs` gates @@ -320,7 +328,7 @@ Infers: `type: configuration`, `installAction: gitignoreEntries` | Field | Type | Description | |-------|------|-------------| | `shell` | `String` | Shell command to execute | -| `shellInteractive` | `Bool` | When `true`, allocates a PTY so commands like `sudo` can prompt for passwords securely. Default: `false` | +| `shellInteractive` | `Bool` | When `true`, allocates a PTY so commands like `sudo` can prompt for passwords securely. Default: `false`. Works the same on macOS and Linux — `forkpty` on both — with a 0×0 window size on both | **Does not infer `type`** — you must provide `type:` explicitly. This is because a shell command could install anything (a brew package, a skill, a tool). diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 42751a84..8ac89093 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -12,14 +12,14 @@ mcs doctor --global # Check globally-configured packs only ### Homebrew not installed -**Symptom**: `mcs sync` fails with "Homebrew is required but not installed." +**Symptom**: `mcs sync` prints `Homebrew not found — install ...` and the package is not installed. -**Fix**: Install Homebrew first: +**Fix on macOS**: install Homebrew, then re-run `mcs sync`: ```bash /bin/bash -c "$(curl -fsSL https://raw.githubusercontent.com/Homebrew/install/HEAD/install.sh)" ``` -Then re-run `mcs sync`. +**Fix on Linux**: Homebrew is optional. Install the package with your distribution's package manager (`apt`, `dnf`, `pacman`, …) and make sure it is on your `PATH`; `mcs doctor` verifies `brew:` components through `PATH` first, so that is enough. Note that this only works when the formula name is also the command name — `brew: ripgrep` installs a command called `rg`, which mcs cannot know. See [Linux support](linux-support.md#d8--homebrew-on-linux). If you want mcs to install formulae for you, install [Linuxbrew](https://docs.brew.sh/Homebrew-on-Linux). ### Xcode Command Line Tools missing @@ -47,16 +47,41 @@ If you manage Node.js through nvm or similar, make sure it's available in your P **Symptom**: MCP servers and plugins can't be registered. -**Fix**: Install Claude Code: +**Fix on macOS**: ```bash brew install claude-code ``` +**Fix on Linux**: `claude-code` is a Homebrew cask, and Linuxbrew has no casks, so mcs does not offer a brew install there. Use the native installer or npm: +```bash +curl -fsSL https://claude.ai/install.sh | bash +# or +npm install -g @anthropic-ai/claude-code +``` + Verify: ```bash claude --version ``` +### `mcs doctor` reports that nothing is installed (Linux) + +**Symptom**: every command-based check says "not found" on a machine where the tools are clearly present. + +**Cause**: mcs resolves command names through `/usr/bin/which`. If that binary is missing — it is absent on NixOS, and Debian has been retiring it — every resolution returns nothing, so mcs reports an empty machine instead of erroring. + +**Fix**: install it (`apt-get install debianutils`, or your distribution's equivalent) and re-run `mcs doctor`. + +### `mcs: command not found` after unpacking the Linux tarball + +**Symptom**: the tarball unpacked, but the shell cannot find `mcs`. + +**Fix**: the tarball contains a bare binary, not an installer. Put it somewhere on your `PATH`: +```bash +install -m 0755 mcs ~/.local/bin/mcs +``` +If the binary is found but fails to start with an error about `libstdc++.so.6`, install it — the released binary links the Swift runtime statically but still needs the C++ runtime (`apt-get install -y libstdc++6`). + ## MCP Servers ### MCP server not registered From 72e5968396040681223abda651890795b2c11f1e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 18:43:39 +0000 Subject: [PATCH 11/30] Flatten the nested #if in the default Homebrew prefix A nested `#if arch(arm64)` inside a `#if canImport(Darwin)` in a single-expression computed property is the one construct in this port that cannot be compiled on the Linux toolchain used to write it. Combining the conditions into one chain removes that uncertainty and reads better; the values are unchanged, and the test's expectation is flattened the same way. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01V9KvZKADJzXojs4fwCgWAR --- Sources/mcs/Core/Environment.swift | 6 ++---- Tests/MCSTests/EnvironmentTests.swift | 6 ++---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/Sources/mcs/Core/Environment.swift b/Sources/mcs/Core/Environment.swift index 3332b7e4..c45f10c7 100644 --- a/Sources/mcs/Core/Environment.swift +++ b/Sources/mcs/Core/Environment.swift @@ -74,12 +74,10 @@ struct Environment { /// Where Homebrew installs itself when no `brew` is on PATH to ask. static var defaultBrewPrefix: String { - #if canImport(Darwin) - #if arch(arm64) + #if canImport(Darwin) && arch(arm64) "/opt/homebrew" - #else + #elseif canImport(Darwin) "/usr/local" - #endif #else // Linuxbrew's documented multi-user prefix; unlike macOS it does not vary by architecture. "/home/linuxbrew/.linuxbrew" diff --git a/Tests/MCSTests/EnvironmentTests.swift b/Tests/MCSTests/EnvironmentTests.swift index 1299be0a..e8e1c594 100644 --- a/Tests/MCSTests/EnvironmentTests.swift +++ b/Tests/MCSTests/EnvironmentTests.swift @@ -135,12 +135,10 @@ struct EnvironmentTests { @Test("The fallback prefix is the platform's own default") func defaultBrewPrefixPerPlatform() { - #if canImport(Darwin) - #if arch(arm64) + #if canImport(Darwin) && arch(arm64) #expect(Environment.defaultBrewPrefix == "/opt/homebrew") - #else + #elseif canImport(Darwin) #expect(Environment.defaultBrewPrefix == "/usr/local") - #endif #else #expect(Environment.defaultBrewPrefix == "/home/linuxbrew/.linuxbrew") #endif From 62b005d4fcdbcaeb1b0d3e55559a5b9e118ba1b1 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 18:50:59 +0000 Subject: [PATCH 12/30] Resolve the home directory from $HOME on both platforms MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit corelibs Foundation's NSHomeDirectory() reads the passwd entry and ignores $HOME; Darwin's honours it. So every path mcs derives — ~/.claude, ~/.mcs, the global gitignore — silently escaped a HOME override on Linux, which is what a container, a CI runner or `sudo -H` sets up. Environment.defaultHomeDirectory() now prefers a non-empty $HOME and falls back to the passwd entry, which is a no-op on macOS and makes the two platforms agree. It takes the environment as a parameter so it can be tested as a pure function: swift-testing runs in parallel and setenv would leak into every other test in flight. Homebrew.allPrefixes uses the same helper so the single-user Linuxbrew prefix cannot drift from it. Not changed: the two places that expand a tilde typed by a user — a path given to `mcs pack add` and a pack's doctor `path:` — still go through Foundation's expandingTildeInPath, which has the same divergence. Routing them through the helper needs tilde parsing at both sites, which is more than this fix is worth; the difference is recorded in the compatibility matrix and the known limitations instead. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01V9KvZKADJzXojs4fwCgWAR --- Sources/mcs/Core/Environment.swift | 16 +++++++++++++++- Sources/mcs/Core/GitignoreManager.swift | 9 +++++---- Sources/mcs/Core/Homebrew.swift | 2 +- Tests/MCSTests/EnvironmentTests.swift | 25 +++++++++++++++++++++++++ Tests/MCSTests/HomebrewTests.swift | 5 ++++- docs/linux-support.md | 9 ++++++++- 6 files changed, 58 insertions(+), 8 deletions(-) diff --git a/Sources/mcs/Core/Environment.swift b/Sources/mcs/Core/Environment.swift index c45f10c7..327834ad 100644 --- a/Sources/mcs/Core/Environment.swift +++ b/Sources/mcs/Core/Environment.swift @@ -30,7 +30,7 @@ struct Environment { private static let resolvedBrewPath: String? = resolveCommand("brew") init(home: URL? = nil) { - let home = home ?? URL(fileURLWithPath: NSHomeDirectory()) + let home = home ?? URL(fileURLWithPath: Self.defaultHomeDirectory()) homeDirectory = home let claudeDir = home.appendingPathComponent(Constants.FileNames.claudeDirectory) @@ -72,6 +72,20 @@ struct Environment { .deletingLastPathComponent().deletingLastPathComponent().path } + /// The user's home directory, `$HOME` first. + /// + /// corelibs Foundation's `NSHomeDirectory()` reads the passwd entry and ignores `$HOME`, where + /// Darwin's honours it — so without this, `HOME=… mcs …` would mean different things on the two + /// platforms. Takes the environment as a parameter so it can be tested as a pure function. + static func defaultHomeDirectory( + environment: [String: String] = ProcessInfo.processInfo.environment + ) -> String { + guard let home = environment["HOME"], !home.isEmpty else { + return NSHomeDirectory() + } + return home + } + /// Where Homebrew installs itself when no `brew` is on PATH to ask. static var defaultBrewPrefix: String { #if canImport(Darwin) && arch(arm64) diff --git a/Sources/mcs/Core/GitignoreManager.swift b/Sources/mcs/Core/GitignoreManager.swift index 9252e4a0..5645a778 100644 --- a/Sources/mcs/Core/GitignoreManager.swift +++ b/Sources/mcs/Core/GitignoreManager.swift @@ -17,10 +17,11 @@ struct GitignoreManager { /// /// Every path here is anchored to `shell.environment.homeDirectory`, never to the process's /// own home. In production the two are the same (`Environment()` derives from - /// `NSHomeDirectory()`), but they differ under an injected `Environment(home:)` — and this - /// type both reads *and writes* the file, so resolving the process home let a sandboxed - /// caller mutate the real user's global gitignore. `HOME` is passed to `git config` for the - /// same reason: `--global` resolves `$HOME/.gitconfig`, which would otherwise escape too. + /// `Environment.defaultHomeDirectory()`), but they differ under an injected + /// `Environment(home:)` — and this type both reads *and writes* the file, so resolving the + /// process home let a sandboxed caller mutate the real user's global gitignore. `HOME` is + /// passed to `git config` for the same reason: `--global` resolves `$HOME/.gitconfig`, which + /// would otherwise escape too. func resolveGlobalGitignorePath() -> URL { let home = shell.environment.homeDirectory let result = shell.run( diff --git a/Sources/mcs/Core/Homebrew.swift b/Sources/mcs/Core/Homebrew.swift index 06c37492..49c3b309 100644 --- a/Sources/mcs/Core/Homebrew.swift +++ b/Sources/mcs/Core/Homebrew.swift @@ -8,7 +8,7 @@ struct Homebrew { #if canImport(Darwin) ["/opt/homebrew", "/usr/local"] #else - ["/home/linuxbrew/.linuxbrew", NSHomeDirectory() + "/.linuxbrew"] + ["/home/linuxbrew/.linuxbrew", Environment.defaultHomeDirectory() + "/.linuxbrew"] #endif } diff --git a/Tests/MCSTests/EnvironmentTests.swift b/Tests/MCSTests/EnvironmentTests.swift index e8e1c594..e12b1644 100644 --- a/Tests/MCSTests/EnvironmentTests.swift +++ b/Tests/MCSTests/EnvironmentTests.swift @@ -105,6 +105,31 @@ struct EnvironmentTests { #expect(!env.isInsideClaudeHome(env.homeDirectory)) } + // MARK: - Home directory + + @Test("The default home directory prefers $HOME") + func defaultHomeDirectoryPrefersHOME() { + // Tested as a pure function rather than by mutating the process environment: swift-testing + // runs in parallel, and setenv would leak into every other test in flight. + #expect(Environment.defaultHomeDirectory(environment: ["HOME": "/sandbox/home"]) == "/sandbox/home") + } + + @Test("An unset or empty $HOME falls back to the passwd entry") + func defaultHomeDirectoryFallsBackToPasswd() { + #expect(Environment.defaultHomeDirectory(environment: [:]) == NSHomeDirectory()) + #expect(Environment.defaultHomeDirectory(environment: ["HOME": ""]) == NSHomeDirectory()) + } + + @Test("Environment derives every path from the resolved home") + func environmentUsesResolvedHome() { + let home = Environment.defaultHomeDirectory(environment: ["HOME": "/sandbox/home"]) + let env = Environment(home: URL(fileURLWithPath: home)) + + #expect(env.homeDirectory.path == "/sandbox/home") + #expect(env.claudeDirectory.path == "/sandbox/home/.claude") + #expect(env.mcsDirectory.path == "/sandbox/home/.mcs") + } + // MARK: - Homebrew prefix @Test("brewPrefix keeps the symlinked entry point's own prefix") diff --git a/Tests/MCSTests/HomebrewTests.swift b/Tests/MCSTests/HomebrewTests.swift index 6acba7a7..a4b2c6e5 100644 --- a/Tests/MCSTests/HomebrewTests.swift +++ b/Tests/MCSTests/HomebrewTests.swift @@ -22,7 +22,10 @@ struct HomebrewTests { #if canImport(Darwin) #expect(Homebrew.allPrefixes == ["/opt/homebrew", "/usr/local"]) #else - #expect(Homebrew.allPrefixes == ["/home/linuxbrew/.linuxbrew", NSHomeDirectory() + "/.linuxbrew"]) + #expect( + Homebrew.allPrefixes + == ["/home/linuxbrew/.linuxbrew", Environment.defaultHomeDirectory() + "/.linuxbrew"] + ) #endif } diff --git a/docs/linux-support.md b/docs/linux-support.md index 6571c12b..e8d9b6de 100644 --- a/docs/linux-support.md +++ b/docs/linux-support.md @@ -10,7 +10,7 @@ ADR-style entry per decision. ## 1. Status and supported configurations `mcs` is built and tested on **Linux x86_64 with glibc**. Every feature works, and section 4 records -how each one was verified; five features behave differently from macOS, and the matrix says how. +how each one was verified; six features behave differently from macOS, and the matrix says how. | | | |---|---| @@ -112,6 +112,7 @@ Legend: `verified` — run on Linux and observed; `verified (with a difference)` | `mcs cleanup` | supported | verified | Found and deleted a `CLAUDE.local.md.backup.*` file, with and without `--force`. | | `mcs check-updates` + SessionStart hook | supported | verified (with a difference) | `check-updates`, `--json` and `--hook` all run; `mcs config set update-check-cli true` registered `mcs check-updates --hook` in `~/.claude/settings.json` and the cooldown file was written. The upgrade instruction differs: `brew upgrade` on macOS, download the release tarball on Linux. | | `mcs config` | supported | verified | `list` / `get` / `set` against `~/.mcs/config.yaml`. | +| `$HOME` override | supported | verified (with a difference) | `mcs` resolves its home from `$HOME`, falling back to the passwd entry, so `HOME=… mcs …` behaves the same on both platforms. One gap: a `~` typed in a pack path or a pack's doctor `path:` is still expanded from the passwd entry on Linux — see known limitations. | | File lock (`flock`) | supported | verified | Two concurrent syncs: the second exited 1 with "Another mcs process is running". | | Lockfile (`mcs.lock.yaml`) | supported | verified | Written after sync with `generate-lockfile true`; `mcs sync --lock` consumed it. | | Terminal colours / width | supported | verified | ANSI colour and the wrapped/re-rendered picker observed under a PTY; colours suppressed when stdout is a pipe. | @@ -355,6 +356,12 @@ platforms list stays `[.macOS(.v13)]`; Linux needs no entry. is left in place. - **`techpack.yaml` cannot mark a component macOS-only.** A pack declaring `brew: mas` will run on Linux and fail that component with a clear message. See D8's rejected options. +- **A literal `~` in a user-supplied path ignores `$HOME` on Linux.** `mcs` resolves its own home + through `Environment.defaultHomeDirectory()`, which prefers `$HOME`, but the two places that + expand a tilde typed by a user — a path passed to `mcs pack add`, and a pack's doctor `path:` — + go through Foundation's `expandingTildeInPath`, which reads the passwd entry on corelibs and the + `$HOME` value on Darwin. This only shows up when `$HOME` differs from the passwd home, e.g. under + `sudo -H` or in a container. Pass an absolute path there if you override `$HOME`. - **Nothing is claimed about Fedora, Alpine or NixOS** — they are untested. ## 7. How to add a platform-specific path From d1c2d8ddd0a4a27c3ec3ee170676eb11d0f5d361 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 18:51:19 +0000 Subject: [PATCH 13/30] Close the process lock descriptor on exec MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A flock belongs to the open file description, which fork+exec shares with the child and preserves for any descriptor not marked close-on-exec. mcs holds ~/.mcs/lock for the whole command while it runs pack components, so a child that outlives it — a daemon started by a shellInteractive component, an installer that backgrounds itself — inherited the descriptor and kept the lock held after mcs exited, making every later mcs command fail with "Another mcs process is running" until that process died. O_CLOEXEC is the whole fix, and it is correct on Darwin and Glibc alike; this was never Linux-specific. No test: reproducing it needs a child that survives the lock holder, and the only path that leaks is ShellRunner's forkpty + execve — Foundation's Process already closes inherited descriptors, so a Process-based test passes with or without the fix and would prove nothing. A forkpty-based one would have to spawn and reap a long-lived process inside the suite, which is exactly the kind of timing-dependent test that turns into a flake. The behaviour is instead pinned by a standalone C probe recorded in the implementation log. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01V9KvZKADJzXojs4fwCgWAR --- Sources/mcs/Core/FileLock.swift | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Sources/mcs/Core/FileLock.swift b/Sources/mcs/Core/FileLock.swift index b77dd56f..731fb88b 100644 --- a/Sources/mcs/Core/FileLock.swift +++ b/Sources/mcs/Core/FileLock.swift @@ -78,7 +78,10 @@ func withFileLock(at path: URL, body: () throws -> T) throws -> T { try fm.createDirectory(at: dir, withIntermediateDirectories: true) } - let fd = open(path.path, O_CREAT | O_RDWR, 0o644) + // O_CLOEXEC: a flock belongs to the open file description, which fork+exec shares, so without + // it a child outliving the command — a daemon started by a shellInteractive component, say — + // inherits the descriptor and keeps the lock held after mcs has exited. + let fd = open(path.path, O_CREAT | O_RDWR | O_CLOEXEC, 0o644) guard fd >= 0 else { throw FileLockError.openFailed(path: path.path, errno: errno) } From 434737af787abe0327bc6e8640fda58e5aa22dd4 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 19:28:49 +0000 Subject: [PATCH 14/30] Stop the raw-mode pickers spinning when stdin ends MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit readByte() returned a zero byte on EOF and on error, and zero matches no case in any of the three picker loops, so each one span at 100% CPU until the process was killed. Reachable whenever the terminal goes away under a raw-mode prompt — demonstrated by closing the PTY master mid-prompt with SIGHUP ignored, the way nohup and setsid leave a process: 400 CPU ticks in 4s, 800 in 8s, never exits. Without SIGHUP ignored the kernel kills the process first, which is why this has never been seen. readByte now returns UInt8?, nil at end of input, and each loop takes the same exit as Ctrl-D — which is this condition typed by hand. readCSIArrowKey needed no change: it already compares and returns optionals. This also removes the last `_ =` in the file, the one the directives allow only with a comment saying the discarded value carries no meaning; here it was load-bearing. Re-run against the rebuilt binary, the same driver now reports the process exiting on EOF with its CPU time frozen at 4 ticks. It exits via a fatalError inside corelibs Foundation's FileHandle.write, which raises EIO once the terminal is gone; that trap is pre-existing for every write in this file and is not what this commit is about. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01V9KvZKADJzXojs4fwCgWAR --- Sources/mcs/Core/CLIOutput.swift | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/Sources/mcs/Core/CLIOutput.swift b/Sources/mcs/Core/CLIOutput.swift index 5f2c391d..1cf0d021 100644 --- a/Sources/mcs/Core/CLIOutput.swift +++ b/Sources/mcs/Core/CLIOutput.swift @@ -267,7 +267,11 @@ struct CLIOutput { renderYesNo(prompt: prompt, selected: selected) while true { - let byte = readByte() + // End of input is Ctrl-D by another name, so it takes the same exit. + guard let byte = readByte() else { + write("\n") + return defaultValue + } switch byte { case 0x0A, 0x0D, 0x20: // Enter or Space — confirm @@ -408,7 +412,10 @@ struct CLIOutput { renderSingleSelectList(title: title, items: items, cursor: cursor) while true { - let byte = readByte() + guard let byte = readByte() else { + write("\n") + return cursor + } switch byte { case 0x0A, 0x0D, 0x20: // Enter or Space — confirm selection @@ -541,7 +548,10 @@ struct CLIOutput { renderInteractiveList(groups: groups, cursor: cursor) while true { - let byte = readByte() + guard let byte = readByte() else { + write("\n") + return collectSelected(from: groups) + } switch byte { case 0x0A, 0x0D: // Enter @@ -694,9 +704,15 @@ struct CLIOutput { write(output) } - private func readByte() -> UInt8 { + /// One byte from stdin, or `nil` at end of input. + /// + /// Optional because the pickers below loop until they recognise a key: on EOF or a read error + /// no key will ever arrive, and a zero byte matches no case, so returning one would spin at + /// 100% CPU until the process is killed. Reachable whenever stdin closes under a raw-mode + /// prompt — a hung-up terminal in a `nohup`/`setsid` wrapper that ignores SIGHUP, for instance. + private func readByte() -> UInt8? { var byte: UInt8 = 0 - _ = read(STDIN_FILENO, &byte, 1) + guard read(STDIN_FILENO, &byte, 1) > 0 else { return nil } return byte } From 21285c11fbb4e93a09bbe06e6b14fc2e1aeaac5c Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 19:34:25 +0000 Subject: [PATCH 15/30] Harden both ends of the PTY bridge and check every child allocation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The POLLERR/POLLNVAL guard added earlier covered stdin only. The PTY master has the same shape: its branch is gated on POLLIN | POLLHUP, so a revents of POLLERR alone takes no branch and poll returns immediately forever — the exact spin the stdin guard set out to stop, one descriptor over. POSIX reports POLLERR in revents whether or not it was requested, so it is reachable, just rarer. An errored master means the command is gone, so it ends the bridge rather than being dropped; the shared comment now states both halves and why POLLHUP is in neither mask. The fork-child comment claimed every C string the child needs is allocated and checked in the parent, but only argv[0] and the working directory were: a nil anywhere in argv[1...] or envp would have silently truncated the vector, since execve stops at the first NULL. One guard over both vectors, skipping the NULL terminator each ends with, makes the comment true. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01V9KvZKADJzXojs4fwCgWAR --- Sources/mcs/Core/ShellRunner.swift | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/Sources/mcs/Core/ShellRunner.swift b/Sources/mcs/Core/ShellRunner.swift index 958af9db..ddd60c2a 100644 --- a/Sources/mcs/Core/ShellRunner.swift +++ b/Sources/mcs/Core/ShellRunner.swift @@ -207,8 +207,10 @@ struct ShellRunner: ShellRunning { // After fork() only async-signal-safe calls are legal, so the child has no way to report // an allocation failure, and a force-unwrap there would trap through the Swift runtime. - // Every C string the child needs is therefore allocated and checked here, in the parent. - // (Glibc types `strdup` as returning an optional; Darwin implicitly unwraps it.) + // Every C string the child needs is therefore allocated and checked here, in the parent — + // a nil in the middle of argv or envp would otherwise truncate the vector at execve, which + // stops at the first NULL. (Glibc types `strdup` as returning an optional; Darwin + // implicitly unwraps it.) guard let executablePath = strdup(executable) else { return ShellResult( exitCode: 1, stdout: "", stderr: "Could not allocate the command path for \(executable)" @@ -217,6 +219,15 @@ struct ShellRunner: ShellRunning { let argv: [UnsafeMutablePointer?] = [executablePath] + arguments.map { strdup($0) } + [nil] defer { argv.compactMap(\.self).forEach { free($0) } } // frees executablePath too — it is argv[0] + // dropLast() skips the NULL terminator each vector deliberately ends with. + guard !argv.dropLast().contains(where: { $0 == nil }), + !envp.dropLast().contains(where: { $0 == nil }) + else { + return ShellResult( + exitCode: 1, stdout: "", stderr: "Could not allocate the command line for \(executable)" + ) + } + // Pre-convert workingDirectory to a C string before fork so the child // doesn't need to invoke Swift's String-to-CString bridge (not fork-safe). let workingDirectoryPath: UnsafeMutablePointer? @@ -305,12 +316,17 @@ struct ShellRunner: ShellRunning { } // A descriptor that errored or was closed under us never becomes readable, so poll(2) - // would keep returning immediately with no branch taken. poll(2) skips negative fds, - // so dropping it here lets the loop carry on draining the PTY. POLLHUP is deliberately - // not included: a hung-up stdin can still have buffered data, which the read drains. + // would keep returning immediately with no branch taken. stdin is dropped rather than + // fatal — poll(2) skips negative fds, so the loop carries on draining the PTY — while + // an errored PTY means the command is gone, so it ends the bridge. POLLHUP is + // deliberately in neither mask: a hung-up descriptor can still have buffered data, + // which the reads below drain. if fds[0].revents & Int16(POLLERR | POLLNVAL) != 0 { fds[0].fd = -1 } + if fds[1].revents & Int16(POLLERR | POLLNVAL) != 0 { + break bridgeLoop + } // Terminal → PTY (user typing, including password input) if fds[0].revents & Int16(POLLIN) != 0 { From a1539befd9be853083381c04bdf4c3add9258fb1 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 19:34:36 +0000 Subject: [PATCH 16/30] Keep the brew hint where brew can act, and own the platform branch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three related corrections around the Claude Code and command-exists paths. ExternalCommandExistsCheck.fix() was changed to drop "Run 'mcs sync'" unconditionally, which regressed macOS: a pack that declares `brew: ollama` alongside a `command: ollama` check really would have been fixed by sync. It now mirrors BrewPackageCheck.fix() — the sync hint stands while Homebrew is there to install with, the manual text only when brew is absent and sync provably cannot act. printManualClaudeInstallInstructions used `#if !canImport(Darwin)`, the inverse of the shape this PR documents. Flipped, with the Darwin arm saying why it is empty. ClaudePrerequisite.swift is a fifth file holding a platform branch, which the rule written in CLAUDE.md and docs/linux-support.md said could not exist. The rule was wrong rather than the file: what differs here is control flow, not a value, so there is no constant to move into Constants. Both lists now name it and say why, and architecture.md matches. New ClaudePrerequisiteTests covers the one changed product path that had none: a claude already on PATH is accepted with no subprocess, and a missing one is reported without attempting an install mcs cannot perform. The Darwin arm runs that second case only when the machine has no brew — with brew the function reaches a yes/no prompt, and reading real stdin under a parallel test runner is exactly what this suite must not do. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01V9KvZKADJzXojs4fwCgWAR --- CLAUDE.md | 4 +- Sources/mcs/Core/ClaudePrerequisite.swift | 4 +- .../ExternalPack/ExternalDoctorCheck.swift | 11 +++-- Tests/MCSTests/ClaudePrerequisiteTests.swift | 42 +++++++++++++++++++ docs/architecture.md | 2 +- docs/linux-support.md | 9 ++-- 6 files changed, 62 insertions(+), 10 deletions(-) create mode 100644 Tests/MCSTests/ClaudePrerequisiteTests.swift diff --git a/CLAUDE.md b/CLAUDE.md index ed396374..acf1ebc2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -77,7 +77,7 @@ mcs config set # Set a configuration value (true/false) - `Backup.swift` — timestamped backups for mixed-ownership files (CLAUDE.local.md), backup discovery and deletion - `GitignoreManager.swift` — global gitignore management, core entry list - `ClaudeIntegration.swift` — `claude mcp add/remove` (with scope support), `claude plugin install/remove` -- `ClaudePrerequisite.swift` — Claude Code CLI availability check with optional Homebrew auto-install +- `ClaudePrerequisite.swift` — Claude Code CLI availability check; macOS offers a Homebrew install, Linux prints the native-installer and npm commands - `Homebrew.swift` — brew detection, package install/uninstall, and `provides(_:)` — the one availability predicate shared by `ComponentExecutor` and `BrewPackageCheck` (PATH under `bareName(of:)`, falling back to `brew list`) - `FileHasher.swift` — SHA-256 file and directory hashing via CryptoKit (used by `PackTrustManager` and `ComponentExecutor`) - `FileLock.swift` — POSIX `flock()` process lock and `LockedCommand` protocol for mutually exclusive CLI commands @@ -191,7 +191,7 @@ import Darwin import Glibc #endif ``` - Platform knowledge lives **only** in `Core/TerminalAttributes.swift`, `Core/Environment.swift`, `Core/Homebrew.swift` and `Core/Constants.swift` — everything else calls into them. A `#if` is for a value or API that genuinely differs, never to make a diagnostic go away; the same goes for `_ =`. Several Foundation methods are `@discardableResult` on Darwin and not on Linux — use the result, it always means something. A test gated out on one platform is a coverage regression: give the `#else` branch the equivalent assertion. Rationale and the compatibility matrix are in `docs/linux-support.md` + Platform knowledge lives **only** in `Core/TerminalAttributes.swift`, `Core/Environment.swift`, `Core/Homebrew.swift`, `Core/Constants.swift` and `Core/ClaudePrerequisite.swift` — everything else calls into them. The last one is there because what differs is control flow, not a value: macOS offers a Homebrew install of Claude Code and Linux cannot, so there is no constant to move. A `#if` is for a value or API that genuinely differs, never to make a diagnostic go away; the same goes for `_ =`. Several Foundation methods are `@discardableResult` on Darwin and not on Linux — use the result, it always means something. A test gated out on one platform is a coverage regression: give the `#else` branch the equivalent assertion. Rationale and the compatibility matrix are in `docs/linux-support.md` - **Comments carry the non-obvious "why", not a narration of the code** — don't restate the line below, don't describe what the code used to do. If the code already says it, delete the comment; if the rationale needs more than a line or two, it belongs in the issue or a memory. State a given rationale once, at the site that owns it, rather than repeating it at every call site ## Testing diff --git a/Sources/mcs/Core/ClaudePrerequisite.swift b/Sources/mcs/Core/ClaudePrerequisite.swift index 9d5a4ea8..42e5e8d2 100644 --- a/Sources/mcs/Core/ClaudePrerequisite.swift +++ b/Sources/mcs/Core/ClaudePrerequisite.swift @@ -54,7 +54,9 @@ func ensureClaudeCLI( /// shell, or writing into npm's global prefix, is a trust decision that belongs to the user. private func printManualClaudeInstallInstructions(_ output: CLIOutput) { output.plain(" Install it manually: https://docs.anthropic.com/en/docs/claude-code") - #if !canImport(Darwin) + #if canImport(Darwin) + // macOS has already been offered the Homebrew cask, so the docs link is the whole answer. + #else output.plain(" curl -fsSL https://claude.ai/install.sh | bash") output.plain(" npm install -g @anthropic-ai/claude-code") #endif diff --git a/Sources/mcs/ExternalPack/ExternalDoctorCheck.swift b/Sources/mcs/ExternalPack/ExternalDoctorCheck.swift index 7cccf241..51c4a0a8 100644 --- a/Sources/mcs/ExternalPack/ExternalDoctorCheck.swift +++ b/Sources/mcs/ExternalPack/ExternalDoctorCheck.swift @@ -81,9 +81,14 @@ struct ExternalCommandExistsCheck: DoctorCheck { func fix() -> FixResult { guard let fixCommand else { - // The pack declares no fix, and nothing here knows which component — if any — would - // install this command, so pointing at 'mcs sync' would be a guess that often no-ops. - return .notFixable("Install '\(command)' and make sure it is on PATH") + // The pack declares no fix and nothing here knows which component — if any — provides + // this command, so 'mcs sync' is a guess either way. It is at least a *possible* one + // while Homebrew is there to install with; with no brew it provably cannot act. + let shell = ShellRunner(environment: environment) + guard Homebrew(shell: shell, environment: environment).isInstalled else { + return .notFixable("Install '\(command)' and make sure it is on PATH") + } + return .notFixable("Run 'mcs sync' to install dependencies") } let result = scriptRunner.runCommand(fixCommand) if result.succeeded { diff --git a/Tests/MCSTests/ClaudePrerequisiteTests.swift b/Tests/MCSTests/ClaudePrerequisiteTests.swift new file mode 100644 index 00000000..0fc4c480 --- /dev/null +++ b/Tests/MCSTests/ClaudePrerequisiteTests.swift @@ -0,0 +1,42 @@ +import Foundation +@testable import mcs +import Testing + +struct ClaudePrerequisiteTests { + private func silentOutput() -> CLIOutput { + CLIOutput(colorsEnabled: false) + } + + @Test("A claude already on PATH is accepted without spawning anything") + func acceptsInstalledCLI() { + let shell = MockShellRunner() + shell.commandExistsResult = true + + #expect(ensureClaudeCLI(shell: shell, environment: shell.environment, output: silentOutput())) + #expect(shell.commandExistsCalls.contains(Constants.CLI.claudeCommand)) + #expect(shell.runCalls.isEmpty) + } + + @Test("A missing claude is reported, and brew is never asked to install it without brew") + func missingCLIWithoutHomebrew() { + let shell = MockShellRunner() + shell.commandExistsResult = false + let environment = shell.environment + + #if canImport(Darwin) + // macOS offers the Homebrew cask when brew is there, and that offer is a prompt — calling + // it here would read real stdin under swift-testing's parallel runner. So on a machine that + // does have brew, assert instead what makes the guard safe to evaluate: it is a filesystem + // check, not a subprocess, which is why gating on it costs nothing. + guard !Homebrew(shell: shell, environment: environment).isInstalled else { + #expect(shell.runCalls.isEmpty) + return + } + #endif + + // Without brew — always on Linux, where `claude-code` is a cask Linuxbrew does not have — + // the answer is the same on both platforms: report it and do not try to install anything. + #expect(!ensureClaudeCLI(shell: shell, environment: environment, output: silentOutput())) + #expect(shell.runCalls.isEmpty, "mcs must not attempt an install it cannot perform") + } +} diff --git a/docs/architecture.md b/docs/architecture.md index 65c2b0fc..a37e4d2f 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -378,7 +378,7 @@ The codebase uses Swift 6's strict concurrency. All core types conform to `Senda ## Platform Support -`mcs` builds for macOS 13+ and for Linux (glibc, x86_64). Platform-dependent knowledge is confined to `Core/TerminalAttributes.swift`, `Core/Environment.swift`, `Core/Homebrew.swift` and `Core/Constants.swift`; everything else calls into them. `swift-crypto` is a Linux-only target dependency supplying SHA-256, which Foundation does not have there. See [Linux support](linux-support.md) for the compatibility matrix, the decisions behind each platform branch, and the rules for adding a new one. +`mcs` builds for macOS 13+ and for Linux (glibc, x86_64). Platform-dependent knowledge is confined to `Core/TerminalAttributes.swift`, `Core/Environment.swift`, `Core/Homebrew.swift`, `Core/Constants.swift` and `Core/ClaudePrerequisite.swift`; everything else calls into them. `swift-crypto` is a Linux-only target dependency supplying SHA-256, which Foundation does not have there. See [Linux support](linux-support.md) for the compatibility matrix, the decisions behind each platform branch, and the rules for adding a new one. --- diff --git a/docs/linux-support.md b/docs/linux-support.md index e8d9b6de..8b2b71ad 100644 --- a/docs/linux-support.md +++ b/docs/linux-support.md @@ -378,9 +378,12 @@ import Glibc Three rules keep this from spreading: -1. **Platform knowledge lives in four places only** — `Core/TerminalAttributes.swift`, - `Core/Environment.swift`, `Core/Homebrew.swift` and `Core/Constants.swift`. Everything else calls - into them. If a fifth file needs a `#if`, that is a sign the value belongs in one of these. +1. **Platform knowledge lives in five places only** — `Core/TerminalAttributes.swift`, + `Core/Environment.swift`, `Core/Homebrew.swift`, `Core/Constants.swift` and + `Core/ClaudePrerequisite.swift`. Everything else calls into them. If a sixth file needs a `#if`, + that is a sign the value belongs in one of these. `ClaudePrerequisite` is on the list because + what differs there is *control flow*, not a value: macOS offers a Homebrew install of Claude + Code and Linux has no cask to offer, so there is no constant to move into `Constants`. 2. **A `#if` is for a value or API that genuinely differs**, never for making a diagnostic go away. The same applies to `_ =`, `try?`, `@unchecked` and `nonisolated(unsafe)`: fix the cause. When a Foundation method is `@discardableResult` on Darwin and not on Linux, use the result — it always From 6308109d5f44aa37d7b5efa13cd730c1f7c6cc45 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 19:37:21 +0000 Subject: [PATCH 17/30] Assert what the tests claim to assert MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four test-side corrections, no product change beyond one simplification. listIgnoresStaleTelemetryKey re-implemented ListConfig.run()'s rendering loop in its own body and asserted on the copy, so it would have passed even if the command started printing raw YAML keys. It now asserts the two properties every key-driven surface is built from — telemetry is absent from knownKeys and value(forKey:) returns nil for it — and is named for that. Driving ListConfig.run() itself would mean capturing process stdout, which CLIOutput has no seam for; the comment says so. allPrefixesPerPlatform compared the single-user Linuxbrew entry against the same expression the source evaluates, so that half could never disagree. It now asserts the composition by shape, and notes that a literal home cannot be injected because allPrefixes is static — the helper's contract is pinned in EnvironmentTests. pathWithBrewPrependsOnce counted occurrences in the ambient PATH, so a developer whose profile is sourced twice failed an unrelated suite. It now asserts the behaviour either way: unchanged when the brew bin is already there, prefixed exactly once when it is not. TerminalAttributes.controlCharacter copied its parameter into a local var only to take its address; withUnsafeBytes(of:) has a borrowing overload. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01V9KvZKADJzXojs4fwCgWAR --- Sources/mcs/Core/TerminalAttributes.swift | 3 +-- Tests/MCSTests/EnvironmentTests.swift | 14 +++++++++++--- Tests/MCSTests/HomebrewTests.swift | 12 ++++++++---- Tests/MCSTests/MCSConfigTests.swift | 18 ++++++++---------- 4 files changed, 28 insertions(+), 19 deletions(-) diff --git a/Sources/mcs/Core/TerminalAttributes.swift b/Sources/mcs/Core/TerminalAttributes.swift index e10928ed..4fa92e35 100644 --- a/Sources/mcs/Core/TerminalAttributes.swift +++ b/Sources/mcs/Core/TerminalAttributes.swift @@ -14,8 +14,7 @@ enum TerminalAttributes { /// Reads one control character. Exists for `TerminalAttributesTests`: asserting raw mode /// without a TTY is the only way to cover this on both platforms. static func controlCharacter(_ attributes: termios, _ index: Int32) -> cc_t { - var attributes = attributes - return withUnsafeBytes(of: &attributes.c_cc) { raw in + withUnsafeBytes(of: attributes.c_cc) { raw in raw.bindMemory(to: cc_t.self)[Int(index)] } } diff --git a/Tests/MCSTests/EnvironmentTests.swift b/Tests/MCSTests/EnvironmentTests.swift index e12b1644..3e13b3f8 100644 --- a/Tests/MCSTests/EnvironmentTests.swift +++ b/Tests/MCSTests/EnvironmentTests.swift @@ -175,9 +175,17 @@ struct EnvironmentTests { defer { try? FileManager.default.removeItem(at: home) } let env = Environment(home: home) - let path = env.pathWithBrew let brewBin = "\(env.brewPrefix)/bin" - #expect(path.contains(brewBin)) - #expect(path.components(separatedBy: brewBin).count == 2, "brew bin must appear exactly once") + let currentPath = ProcessInfo.processInfo.environment["PATH"] ?? "/usr/bin:/bin" + let path = env.pathWithBrew + + // Asserted against the ambient PATH rather than a fixed shape: a developer whose profile + // is sourced twice genuinely has the brew bin in there more than once, and that is not a + // failure of this code. + if currentPath.contains(brewBin) { + #expect(path == currentPath, "an already-present brew bin must not be prepended again") + } else { + #expect(path.hasPrefix("\(brewBin):")) + } } } diff --git a/Tests/MCSTests/HomebrewTests.swift b/Tests/MCSTests/HomebrewTests.swift index a4b2c6e5..06d405b0 100644 --- a/Tests/MCSTests/HomebrewTests.swift +++ b/Tests/MCSTests/HomebrewTests.swift @@ -22,10 +22,14 @@ struct HomebrewTests { #if canImport(Darwin) #expect(Homebrew.allPrefixes == ["/opt/homebrew", "/usr/local"]) #else - #expect( - Homebrew.allPrefixes - == ["/home/linuxbrew/.linuxbrew", Environment.defaultHomeDirectory() + "/.linuxbrew"] - ) + // Asserted by shape, not by re-evaluating the source's own expression: `allPrefixes` is + // static and reads the process home, so a literal home cannot be injected here — that + // contract is pinned in EnvironmentTests instead. + let prefixes = Homebrew.allPrefixes + #expect(prefixes.count == 2) + #expect(prefixes[0] == "/home/linuxbrew/.linuxbrew") + #expect(prefixes[1].hasSuffix("/.linuxbrew")) + #expect(prefixes[1] != prefixes[0], "the single-user prefix is the user's home, not the shared one") #endif } diff --git a/Tests/MCSTests/MCSConfigTests.swift b/Tests/MCSTests/MCSConfigTests.swift index cc07b5c3..eb071c60 100644 --- a/Tests/MCSTests/MCSConfigTests.swift +++ b/Tests/MCSTests/MCSConfigTests.swift @@ -256,21 +256,19 @@ struct MCSConfigTests { #expect(config.updateCheckPacks == true, "a stale key must not discard the live ones") } - @Test("The config list rendering ignores a stale telemetry key") - func listIgnoresStaleTelemetryKey() throws { + @Test("A stale telemetry key is unreachable from every key-driven surface") + func staleTelemetryKeyIsUnreachable() throws { let tmpDir = try makeTmpDir() defer { try? FileManager.default.removeItem(at: tmpDir) } let path = tmpDir.appendingPathComponent("config.yaml") try "telemetry: false\n".write(to: path, atomically: true, encoding: .utf8) - let config = MCSConfig.load(from: path) - // Mirrors ListConfig.run(): every known key is rendered from value(forKey:). - let rendered = MCSConfig.knownKeys.map { known -> String in - let current = config.value(forKey: known.key) - return "\(known.key): \(current.map { String($0) } ?? "(not set)")" - } - #expect(!rendered.contains { $0.contains("telemetry") }) - #expect(rendered.count == MCSConfig.CodingKeys.allCases.count) + + // `mcs config list`, `get` and `set` are all driven by these two, so proving telemetry is + // absent from them proves it cannot be printed or written. Running ListConfig.run() itself + // would mean capturing process stdout, which CLIOutput has no seam for. + #expect(!MCSConfig.knownKeys.contains { $0.key == "telemetry" }) + #expect(config.value(forKey: "telemetry") == nil) } } From 111771455b1c0eb129c6ae819f356345602937b6 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 19:39:50 +0000 Subject: [PATCH 18/30] Run the Linux suite serially and say why MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The intermittent failure previously logged as "transient, container-specific, not reproducible" is neither transient nor ours. NSCocoaErrorDomain 256 out of Process.run() is ETXTBSY, and strace shows corelibs Foundation opening files for writing without O_CLOEXEC on both of its paths: atomic: openat(…, ".dat.nosync1A64.47ae5Z", O_RDWR|O_CREAT|O_EXCL, 0666) non-atomic: openat(…, "b.sh", O_WRONLY|O_CREAT|O_TRUNC, 0666) So when one test forks for its own subprocess while another is mid-write, the child inherits that write descriptor and the later exec of the same file fails. About one full parallel run in four. This is the same class of bug as the O_CLOEXEC fix on the process lock, one level up and inside Foundation, so there is no open-side fix available to us here. Because the non-atomic path leaks too, the test-side fix — writing scripts non-atomically before executing them — is not available either. Marking one suite .serialized would not work: the interfering forks come from unrelated suites. Retrying would hide it. So the Linux CI job runs --no-parallel, which costs about a quarter more wall clock (42s against 34s measured here) and makes the job deterministic. The macOS jobs stay parallel. mcs itself is unaffected: it never executes a file it has just written. Both the workflow and the known limitations say so, so nobody reads the flag as a product caveat. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01V9KvZKADJzXojs4fwCgWAR --- .github/workflows/pr-checks.yml | 8 +++++++- docs/linux-support.md | 14 ++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index 0aae2625..4df49ec3 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -95,8 +95,14 @@ jobs: - name: Resolve packages run: swift package resolve + # --no-parallel: corelibs Foundation opens files for writing without O_CLOEXEC, on both its + # atomic and non-atomic paths, so a test that forks while another is mid-write leaks that + # write descriptor into the child and a later exec of the same file fails with ETXTBSY. + # Measured at roughly one failure in four full runs. mcs itself is unaffected — it never + # executes a file it has just written — so this is a test-runner constraint, not a product + # limitation, and the macOS jobs stay parallel. - name: Test - run: swift test + run: swift test --no-parallel build-universal: name: Universal Binary diff --git a/docs/linux-support.md b/docs/linux-support.md index 8b2b71ad..37ba112d 100644 --- a/docs/linux-support.md +++ b/docs/linux-support.md @@ -83,6 +83,10 @@ directory read-only and asserts that a write into it fails; root ignores directo that one test fails under `sudo` or in a root container. This is why the CI job runs on the `ubuntu-latest` runner rather than in a `swift:*` container image. +**Run it with `--no-parallel` on Linux.** CI does, for the reason in the known limitations below; +without it roughly one full run in four fails with a spurious `ETXTBSY`. Serial costs about a +quarter more wall-clock time (42s against 34s here). + ## 4. Compatibility matrix Legend: `verified` — run on Linux and observed; `verified (with a difference)` — works, but not identically to macOS; the Notes column says how. @@ -362,6 +366,16 @@ platforms list stays `[.macOS(.v13)]`; Linux needs no entry. go through Foundation's `expandingTildeInPath`, which reads the passwd entry on corelibs and the `$HOME` value on Darwin. This only shows up when `$HOME` differs from the passwd home, e.g. under `sudo -H` or in a container. Pass an absolute path there if you override `$HOME`. +- **The test suite runs serially on Linux (`swift test --no-parallel`).** corelibs Foundation opens + files for writing without `O_CLOEXEC` — measured with `strace` on both its atomic path + (`openat(…, ".dat.nosyncXXXX", O_RDWR|O_CREAT|O_EXCL, 0666)`) and its non-atomic one + (`openat(…, "b.sh", O_WRONLY|O_CREAT|O_TRUNC, 0666)`) — so when one test forks a subprocess while + another is mid-write, the child inherits that write descriptor and a later `exec` of the same file + fails with `ETXTBSY`, surfacing as `NSCocoaErrorDomain 256`. Measured at about one failure in four + full parallel runs. **mcs itself is unaffected**: it never executes a file it has just written. + Since the leak is in Foundation rather than in mcs, there is no `open`-side fix available the way + there was for the process lock, and the interfering forks come from unrelated suites, so marking + one suite `.serialized` would not close it. The macOS jobs stay parallel. - **Nothing is claimed about Fedora, Alpine or NixOS** — they are untested. ## 7. How to add a platform-specific path From f803b1fc956c902aada8be5159b81aabf5cecd9b Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 19:41:40 +0000 Subject: [PATCH 19/30] Fix the tap commit's missing identity and bring the workflows up to date MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Homebrew tap step moved out of the macOS job into a checkout-less Ubuntu job, where nothing configures git's identity and the runner's dotless hostname stops git auto-detecting one — so the first tagged release would have published its assets and then failed to update the tap. It now sets the same identity auto-release.yml already uses. release.yml also gains a Linux test job that build-linux depends on: a tag cut from a commit green on macOS and red on Linux would otherwise publish a Linux tarball nothing had run the suite against. pr-checks.yml gains a --static-swift-stdlib build step, so a break in the shape the release ships shows up on the PR rather than on the tag. Both Linux test runs use --no-parallel for the reason recorded in the previous commit. Both swiftly installs now unpack under $RUNNER_TEMP instead of into the checkout, which was leaving swiftly and its tarball as untracked files in the tree the next steps build and test in. They also resolve env.sh through $XDG_DATA_HOME and fail loudly if it is not there — swiftly puts its home under that variable when it is set — and take the bin directory from $SWIFTLY_BIN_DIR, which env.sh exports, instead of hardcoding the same path. Action versions, all three workflows. Release notes read for breaking changes across every skipped major before bumping: - checkout v6 -> v7: refuses fork PR code under pull_request_target and workflow_run unless allow-unsafe-pr-checkout is set. Neither trigger is used here, so no input change. - cache and cache/restore v5 -> v6: ESM migration and dependency bumps; the restore sub-action's inputs (key, path, restore-keys) are unchanged. - upload-artifact v4 -> v7: if-no-files-found keeps warn/error/ignore, and artifact names must still be unique per run; ours are distinct. - download-artifact v4 -> v8: merge-multiple is unchanged, and downloading all artifacts without a name still lands them flat, which is what the SHA256SUMS glob needs. v5's change applies only to downloads by artifact-ids, which this workflow does not use; v8 adds digest-mismatch defaulting to error, which is stricter and wanted. - create-github-app-token stays at v3, the current major. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01V9KvZKADJzXojs4fwCgWAR --- .github/workflows/auto-release.yml | 2 +- .github/workflows/pr-checks.yml | 28 +++++++---- .github/workflows/release.yml | 76 ++++++++++++++++++++++++------ 3 files changed, 81 insertions(+), 25 deletions(-) diff --git a/.github/workflows/auto-release.yml b/.github/workflows/auto-release.yml index 0b25b897..8f4fe368 100644 --- a/.github/workflows/auto-release.yml +++ b/.github/workflows/auto-release.yml @@ -27,7 +27,7 @@ jobs: private-key: ${{ secrets.APP_PRIVATE_KEY }} - name: Checkout with full history - uses: actions/checkout@v6 + uses: actions/checkout@v7 with: token: ${{ steps.app-token.outputs.token }} fetch-depth: 0 diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index 4df49ec3..fb47c083 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -19,7 +19,7 @@ jobs: name: Lint runs-on: macos-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - name: Install linters run: | @@ -41,9 +41,9 @@ jobs: matrix: os: [macos-15, macos-26] steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - - uses: actions/cache@v5 + - uses: actions/cache@v6 with: path: | .build/checkouts @@ -66,21 +66,27 @@ jobs: needs: lint runs-on: ubuntu-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 # Not a `container:` job: the official swift images run as root, and # UpdateCheckerTests.registryWriteFailureContract asserts that a write into a 0555 # directory fails, which root ignores. - name: Install Swift via swiftly + # Unpacked outside the checkout so the tree the next steps build and test in stays clean. run: | + cd "$RUNNER_TEMP" curl -O https://download.swift.org/swiftly/linux/swiftly-$(uname -m).tar.gz tar zxf "swiftly-$(uname -m).tar.gz" ./swiftly init --quiet-shell-followup --assume-yes --skip-install - . "$HOME/.local/share/swiftly/env.sh" + # env.sh lives under $XDG_DATA_HOME when that is set; fail loudly rather than silently + # continuing without swiftly on PATH if the layout ever moves. + ENV_SH="${XDG_DATA_HOME:-$HOME/.local/share}/swiftly/env.sh" + test -f "$ENV_SH" || { echo "::error::swiftly env.sh not found at $ENV_SH"; exit 1; } + . "$ENV_SH" swiftly install --use "$SWIFT_VERSION" --assume-yes - echo "$HOME/.local/share/swiftly/bin" >> "$GITHUB_PATH" + echo "$SWIFTLY_BIN_DIR" >> "$GITHUB_PATH" - - uses: actions/cache@v5 + - uses: actions/cache@v6 with: path: | .build/checkouts @@ -104,14 +110,18 @@ jobs: - name: Test run: swift test --no-parallel + # The shape release.yml ships. Without this, a break in it first shows up on a tag. + - name: Build release binary + run: swift build -c release --static-swift-stdlib + build-universal: name: Universal Binary needs: lint runs-on: macos-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - - uses: actions/cache@v5 + - uses: actions/cache@v6 with: path: | .build/checkouts diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 53cbc12c..98e73f79 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -20,7 +20,7 @@ jobs: name: Verify Version runs-on: macos-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - name: Verify tag matches source version env: @@ -38,9 +38,9 @@ jobs: needs: verify-version runs-on: macos-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - - uses: actions/cache@v5 + - uses: actions/cache@v6 with: path: | .build/checkouts @@ -57,9 +57,9 @@ jobs: needs: resolve runs-on: macos-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - - uses: actions/cache/restore@v5 + - uses: actions/cache/restore@v6 with: path: | .build/checkouts @@ -76,9 +76,9 @@ jobs: needs: [test] runs-on: macos-latest steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - - uses: actions/cache/restore@v5 + - uses: actions/cache/restore@v6 with: path: | .build/checkouts @@ -108,30 +108,72 @@ jobs: TAG_NAME: ${{ github.ref_name }} run: tar -czf "mcs-${TAG_NAME}-macos-universal.tar.gz" -C "${BIN_DIR}" mcs - - uses: actions/upload-artifact@v4 + - uses: actions/upload-artifact@v7 with: name: mcs-macos-universal path: mcs-${{ github.ref_name }}-macos-universal.tar.gz if-no-files-found: error + test-linux: + name: Test (linux) + needs: verify-version + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + + - name: Install Swift via swiftly + # Unpacked outside the checkout so the tree the next steps build and test in stays clean. + run: | + cd "$RUNNER_TEMP" + curl -O https://download.swift.org/swiftly/linux/swiftly-$(uname -m).tar.gz + tar zxf "swiftly-$(uname -m).tar.gz" + ./swiftly init --quiet-shell-followup --assume-yes --skip-install + # env.sh lives under $XDG_DATA_HOME when that is set; fail loudly rather than silently + # continuing without swiftly on PATH if the layout ever moves. + ENV_SH="${XDG_DATA_HOME:-$HOME/.local/share}/swiftly/env.sh" + test -f "$ENV_SH" || { echo "::error::swiftly env.sh not found at $ENV_SH"; exit 1; } + . "$ENV_SH" + swiftly install --use "$SWIFT_VERSION" --assume-yes + echo "$SWIFTLY_BIN_DIR" >> "$GITHUB_PATH" + + - uses: actions/cache@v6 + with: + path: | + .build/checkouts + .build/repositories + .build/workspace-state.json + key: spm-linux-${{ hashFiles('Package.swift') }} + restore-keys: spm-linux- + + # --no-parallel for the same reason as pr-checks.yml: corelibs Foundation leaks write + # descriptors into concurrently forked children, which surfaces as a spurious ETXTBSY. + - name: Test + run: swift test --no-parallel + build-linux: name: Build Linux x86_64 Binary - needs: [test] + needs: [test, test-linux] # ubuntu-22.04 on purpose: it sets the glibc floor of the published binary at 2.35. runs-on: ubuntu-22.04 steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 - name: Install Swift via swiftly + # Unpacked outside the checkout so the tree the next steps build and test in stays clean. run: | + cd "$RUNNER_TEMP" curl -O https://download.swift.org/swiftly/linux/swiftly-$(uname -m).tar.gz tar zxf "swiftly-$(uname -m).tar.gz" ./swiftly init --quiet-shell-followup --assume-yes --skip-install - . "$HOME/.local/share/swiftly/env.sh" + # env.sh lives under $XDG_DATA_HOME when that is set; fail loudly rather than silently + # continuing without swiftly on PATH if the layout ever moves. + ENV_SH="${XDG_DATA_HOME:-$HOME/.local/share}/swiftly/env.sh" + test -f "$ENV_SH" || { echo "::error::swiftly env.sh not found at $ENV_SH"; exit 1; } + . "$ENV_SH" swiftly install --use "$SWIFT_VERSION" --assume-yes - echo "$HOME/.local/share/swiftly/bin" >> "$GITHUB_PATH" + echo "$SWIFTLY_BIN_DIR" >> "$GITHUB_PATH" - - uses: actions/cache@v5 + - uses: actions/cache@v6 with: path: | .build/checkouts @@ -161,7 +203,7 @@ jobs: TAG_NAME: ${{ github.ref_name }} run: tar -czf "mcs-${TAG_NAME}-linux-x86_64.tar.gz" -C "${BIN_DIR}" mcs - - uses: actions/upload-artifact@v4 + - uses: actions/upload-artifact@v7 with: name: mcs-linux-x86_64 path: mcs-${{ github.ref_name }}-linux-x86_64.tar.gz @@ -172,7 +214,7 @@ jobs: needs: [build-macos, build-linux] runs-on: ubuntu-latest steps: - - uses: actions/download-artifact@v4 + - uses: actions/download-artifact@v8 with: merge-multiple: true @@ -208,6 +250,10 @@ jobs: git clone "https://x-access-token:${HOMEBREW_TAP_TOKEN}@github.com/mcs-cli/homebrew-tap.git" tap cd tap + # This job has no checkout and Ubuntu runners have dotless hostnames, so git cannot + # auto-detect an author and would refuse the commit. Same identity as auto-release.yml. + git config user.email "autorelease@mcs-cli.dev" + git config user.name "MCS Auto Release" cat > Formula/mcs.rb << FORMULA class Mcs < Formula desc "Configure Claude Code with MCP servers, plugins, skills, and hooks" From e4107461a863a4290542f88117e0faa496ffb24d Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 19:49:33 +0000 Subject: [PATCH 20/30] Finish the documentation sweep: doc wording fixes and stale comments Corrects the CI-verification claim to say the Linux job lands with this PR rather than asserting a run that has not happened, states the real /usr/bin/which failure mode (mcs sync and mcs update refuse to run, not just an empty-looking machine), names ClaudePrerequisite.swift as the fifth platform-knowledge file in both CLAUDE.md and docs/linux-support.md, and adds ADR D11 (release is now all-or-nothing across platforms) and D12 ($HOME resolution, including the Darwin CFFIXED_USER_HOME footnote). Also fixes two comments left over from the telemetry deletion, the CLAUDE.md line still crediting CryptoKit alone for hashing, the README release-tarball link, and the troubleshooting.md Homebrew message which quoted only the Linux wording. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01V9KvZKADJzXojs4fwCgWAR --- CLAUDE.md | 2 +- README.md | 3 +- Sources/mcs/Core/UpdateChecker.swift | 4 +- docs/linux-support.md | 85 ++++++++++++++++++++++++---- docs/troubleshooting.md | 2 +- 5 files changed, 80 insertions(+), 16 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index acf1ebc2..6bebb8db 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -79,7 +79,7 @@ mcs config set # Set a configuration value (true/false) - `ClaudeIntegration.swift` — `claude mcp add/remove` (with scope support), `claude plugin install/remove` - `ClaudePrerequisite.swift` — Claude Code CLI availability check; macOS offers a Homebrew install, Linux prints the native-installer and npm commands - `Homebrew.swift` — brew detection, package install/uninstall, and `provides(_:)` — the one availability predicate shared by `ComponentExecutor` and `BrewPackageCheck` (PATH under `bareName(of:)`, falling back to `brew list`) -- `FileHasher.swift` — SHA-256 file and directory hashing via CryptoKit (used by `PackTrustManager` and `ComponentExecutor`) +- `FileHasher.swift` — SHA-256 file and directory hashing via CryptoKit on Darwin, swift-crypto on Linux (used by `PackTrustManager` and `ComponentExecutor`) - `FileLock.swift` — POSIX `flock()` process lock and `LockedCommand` protocol for mutually exclusive CLI commands - `Locked.swift` — `NSLock`-backed value box with the `withLock { $0 }` shape of the Darwin-only `OSAllocatedUnfairLock` (`Synchronization.Mutex` is macOS 15+, above the macOS 13 floor) - `TerminalAttributes.swift` — termios helpers for the raw-mode picker; `c_cc` is indexed through the platform's own `VMIN`/`VTIME` and flag masks are widened through `tcflag_t` diff --git a/README.md b/README.md index 5717b282..d4e0901d 100644 --- a/README.md +++ b/README.md @@ -28,7 +28,8 @@ brew install mcs-cli/tap/mcs ``` -**Linux** (x86_64, glibc 2.35+) — download the release tarball: +**Linux** (x86_64, glibc 2.35+) — download the release tarball from +[the latest release](https://github.com/mcs-cli/mcs/releases/latest): ```bash tar -xzf mcs--linux-x86_64.tar.gz diff --git a/Sources/mcs/Core/UpdateChecker.swift b/Sources/mcs/Core/UpdateChecker.swift index a66d835e..723039a2 100644 --- a/Sources/mcs/Core/UpdateChecker.swift +++ b/Sources/mcs/Core/UpdateChecker.swift @@ -189,7 +189,7 @@ struct UpdateChecker { case material([String]) case unknown(UnknownReason) - /// Why a classification could not be made. Carries enough context for telemetry + /// Why a classification could not be made. Carries enough context for diagnostics /// without requiring callers to inspect the orchestrator's call sites. enum UnknownReason: Equatable { case missingClone @@ -280,7 +280,7 @@ struct UpdateChecker { // `resolvedPath` only validates the path shape; it doesn't stat the filesystem. If the // clone was deleted out from under us (e.g. user `rm -rf`'d `~/.mcs/packs/foo`), classify // as `.missingClone` instead of letting git fail with a bogus cwd — same outcome at the - // call site (notification surfaces) but accurate telemetry and one fewer subprocess. + // call site (notification surfaces) but accurate classification and one fewer subprocess. guard let workDirURL = entry.resolvedPath(packsDirectory: environment.packsDirectory), FileManager.default.fileExists(atPath: workDirURL.path) else { diff --git a/docs/linux-support.md b/docs/linux-support.md index 37ba112d..882cc713 100644 --- a/docs/linux-support.md +++ b/docs/linux-support.md @@ -17,11 +17,12 @@ how each one was verified; six features behave differently from macOS, and the m | Architecture | x86_64. No aarch64 artifact is published yet. | | libc | glibc. musl is untested; there is no `canImport(Musl)` branch. | | glibc floor | 2.35 — the published binary is built on Ubuntu 22.04. | -| Tested on | Ubuntu 24.04 (development) and the `ubuntu-latest` GitHub runner (CI). | +| Tested on | Ubuntu 24.04 (development); the `ubuntu-latest` GitHub runner once this PR's CI has run. | | Other distributions | Any glibc ≥ 2.35 distribution is expected to work. **Nothing is claimed about Fedora, Alpine or NixOS** — they have not been tested. | -CI runs `swift build` and the full test suite on Linux for every pull request, alongside the two -macOS jobs. Lint runs on macOS only: SwiftFormat and SwiftLint give the same verdicts on both, so a +CI is configured to run `swift build`, the full test suite and the release build on Linux for every +pull request, alongside the two macOS jobs; the Linux job lands with this change, so its first run is +this PR's. Lint runs on macOS only: SwiftFormat and SwiftLint give the same verdicts on both, so a second run would only add version skew. ## 2. Prerequisites @@ -35,11 +36,19 @@ second run would only add version skew. | `/bin/bash` | running `shell:` components and pack scripts | every mainstream distribution | | `libstdc++.so.6` | the published binary links against it dynamically | most distributions; minimal containers need `apt-get install -y libstdc++6` | -**If `/usr/bin/which` is missing** (it is absent on NixOS, and Debian has been retiring it), mcs does -not error — it reports an empty machine. `ShellRunner.resolvedPath` returns `nil` for *every* -command, so `Homebrew.provides`, the Claude Code prerequisite and every command-based doctor check -report "not found". The symptom is that `mcs doctor` says nothing is installed on a machine where -everything is. See ADR D9 for why this is documented rather than worked around. +**If `/usr/bin/which` is missing** (it is absent on NixOS, and Debian has been retiring it), +`ShellRunner.resolvedPath` returns `nil` for *every* command, so `Homebrew.provides`, the Claude Code +prerequisite and every command-based doctor check report "not found". Measured with `which` masked +on a machine where Claude Code is genuinely installed: + +| Command | Result | +|---|---| +| `mcs sync` | **exits 1 and does nothing** — "Claude Code CLI not found", then the install instructions | +| `mcs update` | **exits 1 and does nothing** — same message | +| `mcs doctor` | exits 0, reporting everything as missing | + +So the two commands that change anything refuse to run and tell the user to install software they +already have. See ADR D9 for why this is documented rather than worked around. Homebrew is **optional** on Linux. Without it, `brew:` components are verified through `PATH` only and cannot be installed by mcs — see the `brew:` row in section 4 and ADR D8. @@ -230,6 +239,14 @@ import Glibc SwiftGlibc — see D5. No `canImport(Musl)` branch is added: nothing here builds for musl, and an untested branch is worse than no branch. +**Consequence beyond the imports.** Four Foundation methods are `@discardableResult` on Darwin and +not on Linux, so their results had to be used rather than dropped. Three sites now require the file +to exist before treating it as a directory, which is behaviour-identical. The fourth is a small +**macOS-visible change**: `GitignoreManager`'s bootstrap used `createFile(atPath:contents:)`, whose +`false` return was ignored, and now writes through `Data` — so a failure to create the global +gitignore throws out of `ensureFileExists()` instead of passing silently and failing later at the +read. + ### D5 — `forkpty()` is called from Glibc; no C shim, no `posix_openpt` rewrite **Context.** `ShellRunner.runInteractive` allocates a real PTY so `sudo` can read a password. The @@ -326,8 +343,14 @@ check would then print a permanently-passing line in every `mcs doctor` run on * touching doctor mandates an integration test. Three files and a test, for a condition that does not occur on the platforms mcs ships for. -**Consequences.** The failure mode is documented in section 2 rather than detected: with no `which`, -mcs reports an empty machine instead of erroring. +**Consequences.** The failure mode is documented in section 2 rather than detected, and it is worse +than "reports an empty machine": `mcs sync` and `mcs update` refuse to run and blame a missing Claude +Code CLI that is in fact installed. That is a bad failure, and it is why the decision is documented +prominently rather than left implicit — but it does not change the arithmetic, because the doctor +check considered in (c) would not have helped either. `mcs doctor` is the one command that still +completes, so a user who runs it sees a `✗ which` line; a user who runs `sync` gets the misleading +error and never reaches doctor. Detecting it where it actually bites means a check in `sync`'s +prerequisite path, which is a different change from the one (c) proposed. ### D10 — No Subprocess 1.0, no tools-version bump, macOS 13 floor stays @@ -338,9 +361,49 @@ against an async API, push `async` through `ParsableCommand.run()`, and require story — a rewrite of the process layer in the middle of a port, for no benefit to this goal. The platforms list stays `[.macOS(.v13)]`; Linux needs no entry. +### D11 — One release, two artifacts + +The release workflow builds the macOS universal binary and the Linux x86_64 binary in parallel and +publishes both from a single job, so one `SHA256SUMS` covers them and the Homebrew tap is updated +once. **Consequence: the release is now all-or-nothing across platforms** — a Linux build or test +failure blocks the macOS tarball, the GitHub release and the tap update, which could not happen +before. That is the intended trade (a half-published release is worse than a late one), but it is a +new way for a Linux problem to hold up macOS users, and it is why `test-linux` runs before +`build-linux` rather than after the release is cut. + +### D12 — The home directory comes from `$HOME`, falling back to the passwd entry + +**Context.** Every path mcs owns hangs off one home directory. corelibs Foundation's +`NSHomeDirectory()` reads the passwd entry and ignores `$HOME`; Darwin's honours it. So on Linux a +`HOME=… mcs …` invocation — what a container, a CI runner and `sudo -H` all set up — silently wrote +to the real user's `~/.claude`, `~/.mcs` and global gitignore. + +**Options considered.** (a) Leave it and document the divergence. (b) Prefer a non-empty `$HOME`, +falling back to `NSHomeDirectory()`. (c) Route the two tilde-expansion sites through the same helper +as well. + +**Decision: (b).** It is a no-op on macOS, where `NSHomeDirectory()` already prefers `$HOME`, and it +makes the two platforms agree. `Environment.defaultHomeDirectory(environment:)` takes the +environment as a parameter so it can be tested as a pure function — swift-testing runs in parallel, +so a test that called `setenv` would leak into every other test in flight. `Homebrew.allPrefixes` +uses the same helper, so the single-user Linuxbrew prefix cannot drift from it. + +**Why not (c).** The two places that expand a tilde a *user* typed — a path given to `mcs pack add` +and a pack's doctor `path:` — go through Foundation's `expandingTildeInPath`, which has the same +divergence. Routing them through the helper means parsing `~`, `~/…` and `~user/…` at both sites, +i.e. a third shared helper and its own tests, for a case that only appears when `$HOME` disagrees +with the passwd home. The gap is listed under known limitations instead. + +**Consequences.** One footnote on Darwin: `NSHomeDirectory()` consults `CFFIXED_USER_HOME` before +`$HOME`, while this helper checks `$HOME` first — they disagree only when both are set and differ, +which in practice means CoreFoundation's own test harnesses. + ## 6. Known limitations -- **x86_64 only.** No aarch64 Linux tarball is published yet. +- **x86_64 only, by decision rather than by obstacle.** GitHub does offer `ubuntu-24.04-arm` and + `ubuntu-22.04-arm` runners, so an aarch64 tarball is buildable today; it is deliberately left as a + follow-up so this port does not widen into a second artifact, its own naming and its own smoke + test. - **glibc ≥ 2.35**, because the release binary is built on Ubuntu 22.04. musl is untested. - **The binary is ~95 MB and needs `libstdc++6`.** `--static-swift-stdlib` links the *Swift* runtime statically only; `ldd` still shows `libstdc++.so.6`, `libgcc_s.so.1`, `libm`, `libc` and diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 8ac89093..c3b7c567 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -12,7 +12,7 @@ mcs doctor --global # Check globally-configured packs only ### Homebrew not installed -**Symptom**: `mcs sync` prints `Homebrew not found — install ...` and the package is not installed. +**Symptom**: `mcs sync` prints `Homebrew not found — ...` and the package is not installed. The rest of the line differs by platform: macOS points at , Linux names your system package manager. **Fix on macOS**: install Homebrew, then re-run `mcs sync`: ```bash From 5dd6b7ee1659cbb920cd16d279e2c90566517ca7 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 19:56:04 +0000 Subject: [PATCH 21/30] Install libcurl4-openssl-dev before swiftly on Linux runners The first CI run of the Linux jobs failed: 'swiftly install' downloads and verifies the toolchain fine, but then refuses to finish, printing that libcurl4-openssl-dev must be installed first. ubuntu-latest and ubuntu-22.04 do not ship it. One apt-get step before each swiftly-install step, in both the PR job and both release-workflow Linux jobs. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01V9KvZKADJzXojs4fwCgWAR --- .github/workflows/pr-checks.yml | 6 ++++++ .github/workflows/release.yml | 12 ++++++++++++ 2 files changed, 18 insertions(+) diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index fb47c083..b12a8254 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -71,6 +71,12 @@ jobs: # Not a `container:` job: the official swift images run as root, and # UpdateCheckerTests.registryWriteFailureContract asserts that a write into a 0555 # directory fails, which root ignores. + - name: Install Swift toolchain build dependencies + # swiftly's installer needs libcurl's dev headers to build/verify the toolchain + # (confirmed by the exact error swiftly prints when it's missing); ubuntu-latest + # does not ship them. + run: sudo apt-get update && sudo apt-get install -y libcurl4-openssl-dev + - name: Install Swift via swiftly # Unpacked outside the checkout so the tree the next steps build and test in stays clean. run: | diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 98e73f79..2e7b1ff4 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -121,6 +121,12 @@ jobs: steps: - uses: actions/checkout@v7 + - name: Install Swift toolchain build dependencies + # swiftly's installer needs libcurl's dev headers to build/verify the toolchain + # (confirmed by the exact error swiftly prints when it's missing); ubuntu-latest + # does not ship them. + run: sudo apt-get update && sudo apt-get install -y libcurl4-openssl-dev + - name: Install Swift via swiftly # Unpacked outside the checkout so the tree the next steps build and test in stays clean. run: | @@ -158,6 +164,12 @@ jobs: steps: - uses: actions/checkout@v7 + - name: Install Swift toolchain build dependencies + # swiftly's installer needs libcurl's dev headers to build/verify the toolchain + # (confirmed by the exact error swiftly prints when it's missing); ubuntu-latest + # does not ship them. + run: sudo apt-get update && sudo apt-get install -y libcurl4-openssl-dev + - name: Install Swift via swiftly # Unpacked outside the checkout so the tree the next steps build and test in stays clean. run: | From 819b40bf7be05f880ae5cf3082b70f8ff27fe98b Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 20:22:15 +0000 Subject: [PATCH 22/30] Address the Copilot review: stdin hangup, doctor hint, install path, lock test, docs rule - ShellRunner: a stdin that reports POLLHUP with nothing readable was left in the poll set, so poll returned immediately for as long as the child lived. Measured with 'true | mcs sync' and a 3s shellInteractive sleep: 3.2s of CPU before, 0.17s after. Readable data is still drained first; only a descriptor with nothing to read and nothing coming is dropped. - ExternalCommandExistsCheck.fix: the hint named only one route. A command: check can sit next to a shell: installer as well as a brew: package, so the hint now names both 'mcs sync' and manual installation and no longer asks Homebrew whether sync could act. - README, docs/linux-support.md, docs/troubleshooting.md: install -D so the documented command creates ~/.local/bin on a fresh account. - FileLockTests: regression for the O_CLOEXEC fix using posix_spawn, which inherits descriptors the way forkpty+execve does (Foundation.Process closes them, so it cannot detect the leak). Verified the test fails without the flag. - CLAUDE.md and docs/linux-support.md: the five-file rule covers platform values and control flow; the per-file import chain from D4 is exempt. - docs/troubleshooting.md: the /usr/bin/which symptom now matches section 2. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01V9KvZKADJzXojs4fwCgWAR --- CLAUDE.md | 2 +- README.md | 2 +- Sources/mcs/Core/ShellRunner.swift | 20 +++++------ .../ExternalPack/ExternalDoctorCheck.swift | 12 +++---- Tests/MCSTests/FileLockTests.swift | 34 +++++++++++++++++++ docs/linux-support.md | 19 +++++++---- docs/troubleshooting.md | 4 +-- 7 files changed, 64 insertions(+), 29 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 6bebb8db..2fe4ef71 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -191,7 +191,7 @@ import Darwin import Glibc #endif ``` - Platform knowledge lives **only** in `Core/TerminalAttributes.swift`, `Core/Environment.swift`, `Core/Homebrew.swift`, `Core/Constants.swift` and `Core/ClaudePrerequisite.swift` — everything else calls into them. The last one is there because what differs is control flow, not a value: macOS offers a Homebrew install of Claude Code and Linux cannot, so there is no constant to move. A `#if` is for a value or API that genuinely differs, never to make a diagnostic go away; the same goes for `_ =`. Several Foundation methods are `@discardableResult` on Darwin and not on Linux — use the result, it always means something. A test gated out on one platform is a coverage regression: give the `#else` branch the equivalent assertion. Rationale and the compatibility matrix are in `docs/linux-support.md` + Platform-dependent **values and control flow** live only in `Core/TerminalAttributes.swift`, `Core/Environment.swift`, `Core/Homebrew.swift`, `Core/Constants.swift` and `Core/ClaudePrerequisite.swift` — everything else calls into them. The last one is there because what differs is control flow, not a value: macOS offers a Homebrew install of Claude Code and Linux cannot, so there is no constant to move. The `#if canImport(Darwin) / #elseif canImport(Glibc)` import chain at the top of a file that calls libc directly (`ShellRunner`, `CLIOutput`, `FileLock`, `GlobMatcher`) or picks a crypto module (`FileHasher`, `SettingsHasher`, `SectionValidator`) is not platform knowledge in this sense — it selects the same API from a different module — and is allowed wherever it is needed. A `#if` is for a value or API that genuinely differs, never to make a diagnostic go away; the same goes for `_ =`. Several Foundation methods are `@discardableResult` on Darwin and not on Linux — use the result, it always means something. A test gated out on one platform is a coverage regression: give the `#else` branch the equivalent assertion. Rationale and the compatibility matrix are in `docs/linux-support.md` - **Comments carry the non-obvious "why", not a narration of the code** — don't restate the line below, don't describe what the code used to do. If the code already says it, delete the comment; if the rationale needs more than a line or two, it belongs in the issue or a memory. State a given rationale once, at the site that owns it, rather than repeating it at every call site ## Testing diff --git a/README.md b/README.md index d4e0901d..a4e87fc0 100644 --- a/README.md +++ b/README.md @@ -33,7 +33,7 @@ brew install mcs-cli/tap/mcs ```bash tar -xzf mcs--linux-x86_64.tar.gz -install -m 0755 mcs ~/.local/bin/mcs +install -D -m 0755 mcs ~/.local/bin/mcs ``` Supported platforms: macOS 13+ and Linux (glibc, x86_64). See diff --git a/Sources/mcs/Core/ShellRunner.swift b/Sources/mcs/Core/ShellRunner.swift index ddd60c2a..0b5963b4 100644 --- a/Sources/mcs/Core/ShellRunner.swift +++ b/Sources/mcs/Core/ShellRunner.swift @@ -315,28 +315,26 @@ struct ShellRunner: ShellRunning { break } - // A descriptor that errored or was closed under us never becomes readable, so poll(2) - // would keep returning immediately with no branch taken. stdin is dropped rather than - // fatal — poll(2) skips negative fds, so the loop carries on draining the PTY — while - // an errored PTY means the command is gone, so it ends the bridge. POLLHUP is - // deliberately in neither mask: a hung-up descriptor can still have buffered data, - // which the reads below drain. - if fds[0].revents & Int16(POLLERR | POLLNVAL) != 0 { - fds[0].fd = -1 - } + // An errored PTY means the command is gone, so it ends the bridge. if fds[1].revents & Int16(POLLERR | POLLNVAL) != 0 { break bridgeLoop } - // Terminal → PTY (user typing, including password input) + // Terminal → PTY (user typing, including password input). Readable data is drained + // first — a hung-up pipe can still carry buffered bytes — and only a descriptor with + // nothing to read and nothing coming (hangup, error, closed) is dropped. Dropping it + // matters: poll(2) reports POLLHUP/POLLERR whether or not they were requested, so + // leaving the fd in the set would make poll return immediately, forever, with no + // branch taken. poll(2) skips negative fds, so the loop carries on draining the PTY. if fds[0].revents & Int16(POLLIN) != 0 { let n = read(STDIN_FILENO, &buf, buf.count) if n <= 0 { - // stdin EOF — stop monitoring, let PTY drain remaining output fds[0].fd = -1 } else { writeAll(fd: ptyFD, buf: buf, count: n) } + } else if fds[0].revents & Int16(POLLHUP | POLLERR | POLLNVAL) != 0 { + fds[0].fd = -1 } // PTY → Terminal (command output, prompts, progress bars) diff --git a/Sources/mcs/ExternalPack/ExternalDoctorCheck.swift b/Sources/mcs/ExternalPack/ExternalDoctorCheck.swift index 51c4a0a8..8e2a8452 100644 --- a/Sources/mcs/ExternalPack/ExternalDoctorCheck.swift +++ b/Sources/mcs/ExternalPack/ExternalDoctorCheck.swift @@ -82,13 +82,11 @@ struct ExternalCommandExistsCheck: DoctorCheck { func fix() -> FixResult { guard let fixCommand else { // The pack declares no fix and nothing here knows which component — if any — provides - // this command, so 'mcs sync' is a guess either way. It is at least a *possible* one - // while Homebrew is there to install with; with no brew it provably cannot act. - let shell = ShellRunner(environment: environment) - guard Homebrew(shell: shell, environment: environment).isInstalled else { - return .notFixable("Install '\(command)' and make sure it is on PATH") - } - return .notFixable("Run 'mcs sync' to install dependencies") + // this command: it may be a brew: package, a shell: installer, or nothing at all. So + // the hint names both routes rather than guessing from what Homebrew can do. + return .notFixable( + "Run 'mcs sync' if a pack component installs '\(command)'; otherwise install it and make sure it is on PATH" + ) } let result = scriptRunner.runCommand(fixCommand) if result.succeeded { diff --git a/Tests/MCSTests/FileLockTests.swift b/Tests/MCSTests/FileLockTests.swift index fbe2e5f5..37ea40da 100644 --- a/Tests/MCSTests/FileLockTests.swift +++ b/Tests/MCSTests/FileLockTests.swift @@ -125,6 +125,40 @@ struct FileLockTests { } } + @Test("withFileLock does not leak the lock into exec'd children") + func lockIsNotInheritedByChildren() throws { + let tmpDir = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let lockFile = tmpDir.appendingPathComponent("lock") + + // posix_spawn hands every descriptor without FD_CLOEXEC to the child, exactly like the + // forkpty + execve path in ShellRunner. Foundation.Process closes inherited descriptors + // itself, so a Process-based child would pass with or without O_CLOEXEC on the lock. + var child: pid_t = 0 + try withFileLock(at: lockFile) { + let argv: [UnsafeMutablePointer?] = [strdup("/bin/sleep"), strdup("30"), nil] + defer { argv.forEach { free($0) } } + let envp: [UnsafeMutablePointer?] = [nil] + #expect(posix_spawn(&child, "/bin/sleep", nil, nil, argv, envp) == 0) + } + defer { + if child > 0 { + kill(child, SIGKILL) + waitpid(child, nil, 0) + } + } + #expect(child > 0) + + // The body has returned and the parent's descriptor is closed, so only a copy inherited by + // the still-running child could hold the lock now. + let fd = open(lockFile.path, O_RDWR) + #expect(fd >= 0) + defer { close(fd) } + + #expect(flock(fd, LOCK_EX | LOCK_NB) == 0, "lock is still held by the exec'd child") + } + @Test("FileLockError.acquireFailed has descriptive message") func errorMessage() { let error = FileLockError.acquireFailed(path: "/tmp/lock") diff --git a/docs/linux-support.md b/docs/linux-support.md index 882cc713..0765ef9a 100644 --- a/docs/linux-support.md +++ b/docs/linux-support.md @@ -62,7 +62,7 @@ Download the tarball for the release you want from ```bash tar -xzf mcs--linux-x86_64.tar.gz -install -m 0755 mcs ~/.local/bin/mcs +install -D -m 0755 mcs ~/.local/bin/mcs mcs --version ``` @@ -455,12 +455,17 @@ import Glibc Three rules keep this from spreading: -1. **Platform knowledge lives in five places only** — `Core/TerminalAttributes.swift`, - `Core/Environment.swift`, `Core/Homebrew.swift`, `Core/Constants.swift` and - `Core/ClaudePrerequisite.swift`. Everything else calls into them. If a sixth file needs a `#if`, - that is a sign the value belongs in one of these. `ClaudePrerequisite` is on the list because - what differs there is *control flow*, not a value: macOS offers a Homebrew install of Claude - Code and Linux has no cask to offer, so there is no constant to move into `Constants`. +1. **Platform-dependent values and control flow live in five places only** — + `Core/TerminalAttributes.swift`, `Core/Environment.swift`, `Core/Homebrew.swift`, + `Core/Constants.swift` and `Core/ClaudePrerequisite.swift`. Everything else calls into them. If + a sixth file needs a `#if` around a *value or a branch*, that is a sign it belongs in one of + these. `ClaudePrerequisite` is on the list because what differs there is *control flow*, not a + value: macOS offers a Homebrew install of Claude Code and Linux has no cask to offer, so there + is no constant to move into `Constants`. + The import chain above is the one `#if` this rule does not cover: a file that calls libc + directly (`ShellRunner`, `CLIOutput`, `FileLock`, `GlobMatcher`, and `FileLockTests`) or picks + the SHA-256 module (`FileHasher`, `SettingsHasher`, `SectionValidator`) carries it at the top, + per D4. It selects the same API from a different module and encodes no platform behaviour. 2. **A `#if` is for a value or API that genuinely differs**, never for making a diagnostic go away. The same applies to `_ =`, `try?`, `@unchecked` and `nonisolated(unsafe)`: fix the cause. When a Foundation method is `@discardableResult` on Darwin and not on Linux, use the result — it always diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index c3b7c567..e5e4c5d1 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -68,7 +68,7 @@ claude --version **Symptom**: every command-based check says "not found" on a machine where the tools are clearly present. -**Cause**: mcs resolves command names through `/usr/bin/which`. If that binary is missing — it is absent on NixOS, and Debian has been retiring it — every resolution returns nothing, so mcs reports an empty machine instead of erroring. +**Cause**: mcs resolves command names through `/usr/bin/which`. If that binary is missing — it is absent on NixOS, and Debian has been retiring it — every resolution returns nothing: `mcs sync` and `mcs update` refuse to run (they report Claude Code as missing even when it is installed) and `mcs doctor` reports everything as not found. **Fix**: install it (`apt-get install debianutils`, or your distribution's equivalent) and re-run `mcs doctor`. @@ -78,7 +78,7 @@ claude --version **Fix**: the tarball contains a bare binary, not an installer. Put it somewhere on your `PATH`: ```bash -install -m 0755 mcs ~/.local/bin/mcs +install -D -m 0755 mcs ~/.local/bin/mcs ``` If the binary is found but fails to start with an error about `libstdc++.so.6`, install it — the released binary links the Swift runtime statically but still needs the C++ runtime (`apt-get install -y libstdc++6`). From 8b82a73efc95c64c67b94c646db0e21bba8b2d18 Mon Sep 17 00:00:00 2001 From: Bruno Guidolim Date: Wed, 16 Sep 2026 01:06:42 +0200 Subject: [PATCH 23/30] Resolve the home directory and Homebrew prefix from one place on both platforms - NSHomeDirectory() ignores $HOME on Darwin too; document the $HOME-first resolution as a behavior change and expand user-typed ~ against the same home - brewPrefix resolves symlinks then strips a trailing Homebrew component, so shims and the installer's own $PREFIX/bin/brew link both yield the real prefix - ScopedPathCheck and PackSourceResolver carry an Environment; tests pin the init wiring, the tilde expansion and the shim case Claude-Session: https://claude.ai/code/session_014tZGxbvJUwXqd6fU1S3fXu --- Sources/mcs/Commands/PackCommand.swift | 2 +- Sources/mcs/Core/Environment.swift | 36 +++++++--- .../ExternalPack/ExternalDoctorCheck.swift | 29 ++++---- .../mcs/ExternalPack/PackSourceResolver.swift | 5 +- Tests/MCSTests/EnvironmentTests.swift | 44 +++++++++++- Tests/MCSTests/ExternalDoctorCheckTests.swift | 21 ++++++ Tests/MCSTests/PackSourceResolverTests.swift | 16 +++++ docs/linux-support.md | 68 +++++++++---------- 8 files changed, 155 insertions(+), 66 deletions(-) diff --git a/Sources/mcs/Commands/PackCommand.swift b/Sources/mcs/Commands/PackCommand.swift index 70f936ab..0bf6b1ff 100644 --- a/Sources/mcs/Commands/PackCommand.swift +++ b/Sources/mcs/Commands/PackCommand.swift @@ -56,7 +56,7 @@ struct AddPack: LockedCommand { func perform() throws { let ctx = PackCommandContext() - let resolver = PackSourceResolver() + let resolver = PackSourceResolver(environment: ctx.env) let packSource: PackSource do { packSource = try resolver.resolve(source) diff --git a/Sources/mcs/Core/Environment.swift b/Sources/mcs/Core/Environment.swift index 327834ad..40bee39d 100644 --- a/Sources/mcs/Core/Environment.swift +++ b/Sources/mcs/Core/Environment.swift @@ -63,20 +63,27 @@ struct Environment { /// The Homebrew prefix implied by the path `brew` was found at. /// - /// Two components up from `$PREFIX/bin/brew`, deliberately *without* resolving symlinks: - /// Linuxbrew and Intel macOS install `bin/brew` as a symlink into `$PREFIX/Homebrew/bin/brew`, - /// and resolving it first would yield `$PREFIX/Homebrew`, whose `bin` holds only `brew` — - /// hiding `$PREFIX/bin`, where every formula's symlink lives, from `pathWithBrew`. + /// Symlinks are resolved first so a shim elsewhere on PATH (`~/.local/bin/brew` pointing at the + /// real install) still yields the prefix formulae link into. The installer itself links + /// `$PREFIX/bin/brew -> ../Homebrew/bin/brew` on Linux and Intel macOS, which resolves to the + /// repository checkout rather than the prefix — hence the trailing `Homebrew` is stripped. static func brewPrefix(forBrewPath path: String) -> String { - URL(fileURLWithPath: path) - .deletingLastPathComponent().deletingLastPathComponent().path + var prefix = URL(fileURLWithPath: path) + .resolvingSymlinksInPath() + .deletingLastPathComponent().deletingLastPathComponent() + if prefix.lastPathComponent == "Homebrew" { + prefix = prefix.deletingLastPathComponent() + } + return prefix.path } /// The user's home directory, `$HOME` first. /// - /// corelibs Foundation's `NSHomeDirectory()` reads the passwd entry and ignores `$HOME`, where - /// Darwin's honours it — so without this, `HOME=… mcs …` would mean different things on the two - /// platforms. Takes the environment as a parameter so it can be tested as a pure function. + /// Foundation's `NSHomeDirectory()` resolves the passwd entry on Darwin and corelibs alike and + /// consults `$HOME` only when there is none, so `HOME=… mcs …` used to be ignored on every + /// platform. Preferring a non-empty `$HOME` is what makes containers, `sudo -H`-style launchers + /// and test sandboxes work. Takes the environment as a parameter so it can be tested as a pure + /// function. static func defaultHomeDirectory( environment: [String: String] = ProcessInfo.processInfo.environment ) -> String { @@ -86,6 +93,17 @@ struct Environment { return home } + /// Expands a leading `~` against `homeDirectory`, not the passwd entry Foundation's + /// `expandingTildeInPath` reads — otherwise a `~/…` pack path or doctor check would resolve + /// under a different home than `~/.mcs` whenever `$HOME` is overridden. + func expandingTilde(_ path: String) -> String { + if path == "~" { + return homeDirectory.path + } + guard path.hasPrefix("~/") else { return path } + return homeDirectory.appendingPathComponent(String(path.dropFirst(2))).path + } + /// Where Homebrew installs itself when no `brew` is on PATH to ask. static var defaultBrewPrefix: String { #if canImport(Darwin) && arch(arm64) diff --git a/Sources/mcs/ExternalPack/ExternalDoctorCheck.swift b/Sources/mcs/ExternalPack/ExternalDoctorCheck.swift index 8e2a8452..09a92319 100644 --- a/Sources/mcs/ExternalPack/ExternalDoctorCheck.swift +++ b/Sources/mcs/ExternalPack/ExternalDoctorCheck.swift @@ -105,6 +105,7 @@ struct ExternalFileExistsCheck: ScopedPathCheck { let path: String let scope: ExternalDoctorCheckScope let projectRoot: URL? + var environment: Environment = .init() func check() -> CheckResult { let resolved: String @@ -132,6 +133,7 @@ struct ExternalDirectoryExistsCheck: ScopedPathCheck { let path: String let scope: ExternalDoctorCheckScope let projectRoot: URL? + var environment: Environment = .init() func check() -> CheckResult { let resolved: String @@ -161,6 +163,7 @@ struct ExternalFileContainsCheck: ScopedPathCheck { let pattern: String let scope: ExternalDoctorCheckScope let projectRoot: URL? + var environment: Environment = .init() func check() -> CheckResult { let resolved: String @@ -192,6 +195,7 @@ struct ExternalFileNotContainsCheck: ScopedPathCheck { let pattern: String let scope: ExternalDoctorCheckScope let projectRoot: URL? + var environment: Environment = .init() func check() -> CheckResult { let resolved: String @@ -517,7 +521,8 @@ enum ExternalDoctorCheckFactory { section: section, path: path, scope: scope, - projectRoot: projectRoot + projectRoot: projectRoot, + environment: environment ) case .directoryExists: @@ -532,7 +537,8 @@ enum ExternalDoctorCheckFactory { section: section, path: path, scope: scope, - projectRoot: projectRoot + projectRoot: projectRoot, + environment: environment ) case .fileContains: @@ -550,7 +556,8 @@ enum ExternalDoctorCheckFactory { path: path, pattern: pattern, scope: scope, - projectRoot: projectRoot + projectRoot: projectRoot, + environment: environment ) case .fileNotContains: @@ -568,7 +575,8 @@ enum ExternalDoctorCheckFactory { path: path, pattern: pattern, scope: scope, - projectRoot: projectRoot + projectRoot: projectRoot, + environment: environment ) case .shellScript: @@ -640,6 +648,7 @@ protocol ScopedPathCheck: DoctorCheck { var path: String { get } var scope: ExternalDoctorCheckScope { get } var projectRoot: URL? { get } + var environment: Environment { get } } enum PathResolveResult { @@ -652,7 +661,7 @@ extension ScopedPathCheck { func resolvePath() -> PathResolveResult { switch scope { case .global: - return .resolved(expandTilde(path)) + return .resolved(environment.expandingTilde(path)) case .project: guard let root = projectRoot else { return .noProjectRoot } guard let safe = PathContainment.safePath(relativePath: path, within: root) else { @@ -746,13 +755,3 @@ extension SettingsReadingCheck { return SettingsProbe(match: nil, readErrors: readErrors, anyFileExisted: anyFileExisted) } } - -// MARK: - Helpers - -/// Expand `~` at the start of a path to the user's home directory. -func expandTilde(_ path: String) -> String { - if path.hasPrefix("~/") { - return NSString(string: path).expandingTildeInPath - } - return path -} diff --git a/Sources/mcs/ExternalPack/PackSourceResolver.swift b/Sources/mcs/ExternalPack/PackSourceResolver.swift index 82d4cd43..4ee0a4f6 100644 --- a/Sources/mcs/ExternalPack/PackSourceResolver.swift +++ b/Sources/mcs/ExternalPack/PackSourceResolver.swift @@ -35,6 +35,8 @@ struct PackSourceResolver { /// character to exclude path-like inputs such as `../foo` or `./bar`. static let shorthandPattern = #"^[a-zA-Z0-9][a-zA-Z0-9_.-]*/[a-zA-Z0-9][a-zA-Z0-9_.-]*$"# + var environment: Environment = .init() + func resolve(_ input: String) throws -> PackSource { guard !input.hasPrefix("-") else { throw PackSourceError.invalidInput("must not start with '-'") @@ -59,11 +61,10 @@ struct PackSourceResolver { input } - // expandingTildeInPath handles ~/... and is a no-op for other paths. // URL(fileURLWithPath:) resolves relative paths (../, ./) against CWD. // Extract .path first to get the absolute string, then re-wrap — // calling .standardized directly on a relative URL mangles ".." components. - let expanded = NSString(string: pathString).expandingTildeInPath + let expanded = environment.expandingTilde(pathString) let resolved = URL(fileURLWithPath: URL(fileURLWithPath: expanded).path) var isDir: ObjCBool = false diff --git a/Tests/MCSTests/EnvironmentTests.swift b/Tests/MCSTests/EnvironmentTests.swift index 3e13b3f8..3877324e 100644 --- a/Tests/MCSTests/EnvironmentTests.swift +++ b/Tests/MCSTests/EnvironmentTests.swift @@ -130,10 +130,26 @@ struct EnvironmentTests { #expect(env.mcsDirectory.path == "/sandbox/home/.mcs") } + @Test("The default initializer wires the resolved home through") + func defaultInitUsesDefaultHomeDirectory() { + #expect(Environment().homeDirectory.path == Environment.defaultHomeDirectory()) + } + + @Test("A leading tilde expands against the environment's home, not the passwd entry") + func expandingTildeUsesEnvironmentHome() { + let env = Environment(home: URL(fileURLWithPath: "/sandbox/home")) + + #expect(env.expandingTilde("~") == "/sandbox/home") + #expect(env.expandingTilde("~/packs/ios") == "/sandbox/home/packs/ios") + #expect(env.expandingTilde("/abs/path") == "/abs/path") + #expect(env.expandingTilde("relative/~/x") == "relative/~/x") + #expect(env.expandingTilde("~other/x") == "~other/x") + } + // MARK: - Homebrew prefix - @Test("brewPrefix keeps the symlinked entry point's own prefix") - func brewPrefixDoesNotFollowSymlinks() throws { + @Test("brewPrefix strips the Homebrew repository component the installer's symlink resolves to") + func brewPrefixStripsRepositoryCheckout() throws { let home = try makeTmpHome() defer { try? FileManager.default.removeItem(at: home) } @@ -149,10 +165,32 @@ struct EnvironmentTests { withDestinationPath: "../Homebrew/bin/brew" ) - // Resolving the symlink first would yield /Homebrew, whose bin holds only brew. + // Resolving alone yields /Homebrew, whose bin holds only brew. #expect(Environment.brewPrefix(forBrewPath: prefixBin.appendingPathComponent("brew").path) == prefix.path) } + @Test("brewPrefix follows a shim symlink back to the real prefix") + func brewPrefixFollowsShim() throws { + let home = try makeTmpHome() + defer { try? FileManager.default.removeItem(at: home) } + + // A user-made ~/.local/bin/brew -> /bin/brew with /bin off PATH: the + // prefix formulae link into is the target's, not the shim's. + let prefix = home.appendingPathComponent("opt/homebrew") + let prefixBin = prefix.appendingPathComponent("bin") + let shimBin = home.appendingPathComponent(".local/bin") + try FileManager.default.createDirectory(at: prefixBin, withIntermediateDirectories: true) + try FileManager.default.createDirectory(at: shimBin, withIntermediateDirectories: true) + try Data().write(to: prefixBin.appendingPathComponent("brew")) + try FileManager.default.createSymbolicLink( + atPath: shimBin.appendingPathComponent("brew").path, + withDestinationPath: prefixBin.appendingPathComponent("brew").path + ) + + let expected = prefix.resolvingSymlinksInPath().path + #expect(Environment.brewPrefix(forBrewPath: shimBin.appendingPathComponent("brew").path) == expected) + } + @Test("brewPrefix handles a real file at $PREFIX/bin/brew (arm64 macOS shape)") func brewPrefixForRealFile() { #expect(Environment.brewPrefix(forBrewPath: "/opt/homebrew/bin/brew") == "/opt/homebrew") diff --git a/Tests/MCSTests/ExternalDoctorCheckTests.swift b/Tests/MCSTests/ExternalDoctorCheckTests.swift index 0dc5badd..ca027379 100644 --- a/Tests/MCSTests/ExternalDoctorCheckTests.swift +++ b/Tests/MCSTests/ExternalDoctorCheckTests.swift @@ -117,6 +117,27 @@ struct ExternalDoctorCheckTests { } } + @Test("A global-scoped tilde path resolves under the injected home") + func fileExistsResolvesTildeAgainstInjectedHome() throws { + let home = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: home) } + try "content".write(to: home.appendingPathComponent(".marker"), atomically: true, encoding: .utf8) + + let check = ExternalFileExistsCheck( + name: "marker", + section: "Files", + path: "~/.marker", + scope: .global, + projectRoot: nil, + environment: Environment(home: home) + ) + if case .pass = check.check() { + // expected + } else { + Issue.record("Expected .pass against the injected home, got \(check.check())") + } + } + @Test("File exists check fails for missing file") func fileExistsFail() { let check = ExternalFileExistsCheck( diff --git a/Tests/MCSTests/PackSourceResolverTests.swift b/Tests/MCSTests/PackSourceResolverTests.swift index 50006d76..897d8a9b 100644 --- a/Tests/MCSTests/PackSourceResolverTests.swift +++ b/Tests/MCSTests/PackSourceResolverTests.swift @@ -109,6 +109,22 @@ struct PackSourceResolverTests { #expect(url.standardizedFileURL.path == tmpDir.standardizedFileURL.path) } + @Test("A tilde path resolves under the injected home, not the passwd entry") + func tildeResolvesUnderInjectedHome() throws { + let home = try makeTmpDir() + defer { try? FileManager.default.removeItem(at: home) } + let pack = home.appendingPathComponent("packs/ios") + try FileManager.default.createDirectory(at: pack, withIntermediateDirectories: true) + + let resolver = PackSourceResolver(environment: Environment(home: home)) + let result = try resolver.resolve("~/packs/ios") + guard case let .localPath(url) = result else { + Issue.record("Expected .localPath, got \(result)") + return + } + #expect(url.standardizedFileURL.path == pack.standardizedFileURL.path) + } + @Test("file:// prefix is stripped and treated as local path") func fileScheme() throws { let tmpDir = try makeTmpDir() diff --git a/docs/linux-support.md b/docs/linux-support.md index 0765ef9a..b60e4611 100644 --- a/docs/linux-support.md +++ b/docs/linux-support.md @@ -125,7 +125,7 @@ Legend: `verified` — run on Linux and observed; `verified (with a difference)` | `mcs cleanup` | supported | verified | Found and deleted a `CLAUDE.local.md.backup.*` file, with and without `--force`. | | `mcs check-updates` + SessionStart hook | supported | verified (with a difference) | `check-updates`, `--json` and `--hook` all run; `mcs config set update-check-cli true` registered `mcs check-updates --hook` in `~/.claude/settings.json` and the cooldown file was written. The upgrade instruction differs: `brew upgrade` on macOS, download the release tarball on Linux. | | `mcs config` | supported | verified | `list` / `get` / `set` against `~/.mcs/config.yaml`. | -| `$HOME` override | supported | verified (with a difference) | `mcs` resolves its home from `$HOME`, falling back to the passwd entry, so `HOME=… mcs …` behaves the same on both platforms. One gap: a `~` typed in a pack path or a pack's doctor `path:` is still expanded from the passwd entry on Linux — see known limitations. | +| `$HOME` override | supported (behavior change) | verified | `mcs` resolves its home from `$HOME`, falling back to the passwd entry, on both platforms — including a `~` typed in a pack path or a pack's doctor `path:`. Previously `$HOME` was ignored everywhere (ADR D12). | | File lock (`flock`) | supported | verified | Two concurrent syncs: the second exited 1 with "Another mcs process is running". | | Lockfile (`mcs.lock.yaml`) | supported | verified | Written after sync with `generate-lockfile true`; `mcs sync --lock` consumed it. | | Terminal colours / width | supported | verified | ANSI colour and the wrapped/re-rendered picker observed under a PTY; colours suppressed when stdout is a pipe. | @@ -299,12 +299,14 @@ users have no Homebrew at all. **Decisions.** -1. **The prefix derivation no longer resolves symlinks.** It takes two components up from - `$PREFIX/bin/brew`. Homebrew's installer creates `$PREFIX/bin/brew -> ../Homebrew/bin/brew` on - Linux *and on Intel macOS*; only arm64 macOS has a real file there. Resolving first yielded - `$PREFIX/Homebrew`, whose `bin` holds only `brew`, so `pathWithBrew` prepended an empty directory - and hid `$PREFIX/bin`, where every formula's symlink lives — breaking the PATH-first probe that - `brew:` components rest on. This is also a latent fix for Intel macOS. +1. **The prefix derivation resolves symlinks, then strips a trailing `Homebrew` component.** + Homebrew's installer creates `$PREFIX/bin/brew -> ../Homebrew/bin/brew` on Linux *and on Intel + macOS*; only arm64 macOS has a real file there. Resolving alone therefore yields the repository + checkout, `$PREFIX/Homebrew`, whose `bin` holds only `brew`, and `pathWithBrew` prepended that + useless directory. Not resolving at all would instead break a shim — `~/.local/bin/brew` pointing + at the real install with `$PREFIX/bin` off PATH — by reporting the shim's directory as the prefix. + Resolve-then-strip handles arm64, Intel, Linuxbrew and shims alike. `brewPrefix` feeds only + `pathWithBrew`; formula detection walks `Homebrew.allPrefixes`. 2. **The fallback prefix and `allPrefixes` are platform-aware.** Linux gets `/home/linuxbrew/.linuxbrew` and `~/.linuxbrew`. No `brew --prefix` subprocess is spawned. 3. **`Homebrew.provides` does not change.** It probes PATH under the name with any tap qualifier @@ -373,30 +375,30 @@ new way for a Linux problem to hold up macOS users, and it is why `test-linux` r ### D12 — The home directory comes from `$HOME`, falling back to the passwd entry -**Context.** Every path mcs owns hangs off one home directory. corelibs Foundation's -`NSHomeDirectory()` reads the passwd entry and ignores `$HOME`; Darwin's honours it. So on Linux a +**Context.** Every path mcs owns hangs off one home directory. Foundation's `NSHomeDirectory()` +resolves the passwd entry first on Darwin and corelibs alike, and consults `$HOME` only when there +is no passwd entry (`CFFIXED_USER_HOME`, when set, replaces both). So on *every* platform a `HOME=… mcs …` invocation — what a container, a CI runner and `sudo -H` all set up — silently wrote -to the real user's `~/.claude`, `~/.mcs` and global gitignore. - -**Options considered.** (a) Leave it and document the divergence. (b) Prefer a non-empty `$HOME`, -falling back to `NSHomeDirectory()`. (c) Route the two tilde-expansion sites through the same helper -as well. - -**Decision: (b).** It is a no-op on macOS, where `NSHomeDirectory()` already prefers `$HOME`, and it -makes the two platforms agree. `Environment.defaultHomeDirectory(environment:)` takes the -environment as a parameter so it can be tested as a pure function — swift-testing runs in parallel, -so a test that called `setenv` would leak into every other test in flight. `Homebrew.allPrefixes` -uses the same helper, so the single-user Linuxbrew prefix cannot drift from it. - -**Why not (c).** The two places that expand a tilde a *user* typed — a path given to `mcs pack add` -and a pack's doctor `path:` — go through Foundation's `expandingTildeInPath`, which has the same -divergence. Routing them through the helper means parsing `~`, `~/…` and `~user/…` at both sites, -i.e. a third shared helper and its own tests, for a case that only appears when `$HOME` disagrees -with the passwd home. The gap is listed under known limitations instead. - -**Consequences.** One footnote on Darwin: `NSHomeDirectory()` consults `CFFIXED_USER_HOME` before -`$HOME`, while this helper checks `$HOME` first — they disagree only when both are set and differ, -which in practice means CoreFoundation's own test harnesses. +to the real user's `~/.claude`, `~/.mcs` and global gitignore. Verified on macOS with a compiled +probe: `HOME=/tmp/x` still returned the passwd home. + +**Options considered.** (a) Leave it and document it. (b) Prefer a non-empty `$HOME`, falling back +to `NSHomeDirectory()`. (c) Also route the two tilde-expansion sites through the same home. + +**Decision: (b) and (c).** This is a deliberate behavior change on macOS as much as on Linux: an +invocation whose `$HOME` differs from the passwd home — `sudo` with `env_keep`, a launchd agent, a +sandboxed test — now reads and writes under `$HOME`. `Environment.defaultHomeDirectory(environment:)` +takes the environment as a parameter so it can be tested as a pure function — swift-testing runs in +parallel, so a test that called `setenv` would leak into every other test in flight. +`Homebrew.allPrefixes` uses the same helper, so the single-user Linuxbrew prefix cannot drift from +it. The two places that expand a tilde a *user* typed — a path given to `mcs pack add`, and a pack's +doctor `path:` — go through `Environment.expandingTilde(_:)` rather than Foundation's +`expandingTildeInPath`, so `~/pack` and `~/.mcs` can never name different homes. + +**Consequences.** `HOME= mcs …` is now a working sandbox for the whole binary. A `$HOME` that +points at a missing directory is not validated; `mcs sync --global` creates the tree, `mcs doctor` +reports everything under it as missing with the path shown. `~user/…` is not expanded — neither +site ever accepted it. ## 6. Known limitations @@ -423,12 +425,6 @@ which in practice means CoreFoundation's own test harnesses. is left in place. - **`techpack.yaml` cannot mark a component macOS-only.** A pack declaring `brew: mas` will run on Linux and fail that component with a clear message. See D8's rejected options. -- **A literal `~` in a user-supplied path ignores `$HOME` on Linux.** `mcs` resolves its own home - through `Environment.defaultHomeDirectory()`, which prefers `$HOME`, but the two places that - expand a tilde typed by a user — a path passed to `mcs pack add`, and a pack's doctor `path:` — - go through Foundation's `expandingTildeInPath`, which reads the passwd entry on corelibs and the - `$HOME` value on Darwin. This only shows up when `$HOME` differs from the passwd home, e.g. under - `sudo -H` or in a container. Pass an absolute path there if you override `$HOME`. - **The test suite runs serially on Linux (`swift test --no-parallel`).** corelibs Foundation opens files for writing without `O_CLOEXEC` — measured with `strace` on both its atomic path (`openat(…, ".dat.nosyncXXXX", O_RDWR|O_CREAT|O_EXCL, 0666)`) and its non-atomic one From 1691b3c863ba09fa72375e5cff71162bba9cdbba Mon Sep 17 00:00:00 2001 From: Bruno Guidolim Date: Wed, 16 Sep 2026 01:07:50 +0200 Subject: [PATCH 24/30] Treat a hangup under a raw-mode yes/no prompt as No, and say so - A closed stdin on the raw path is a hangup on what was a terminal, and the default may be the destructive answer; No is safe at every call site - All three raw-mode pickers leave an "Input closed" warning in scrollback - readByte takes the descriptor so the EOF exit is covered by a pipe test Claude-Session: https://claude.ai/code/session_014tZGxbvJUwXqd6fU1S3fXu --- Sources/mcs/Core/CLIOutput.swift | 26 ++++++++++++------ Tests/MCSTests/CLIOutputTests.swift | 42 +++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 8 deletions(-) create mode 100644 Tests/MCSTests/CLIOutputTests.swift diff --git a/Sources/mcs/Core/CLIOutput.swift b/Sources/mcs/Core/CLIOutput.swift index 1cf0d021..26d75ee0 100644 --- a/Sources/mcs/Core/CLIOutput.swift +++ b/Sources/mcs/Core/CLIOutput.swift @@ -267,10 +267,12 @@ struct CLIOutput { renderYesNo(prompt: prompt, selected: selected) while true { - // End of input is Ctrl-D by another name, so it takes the same exit. + // Unlike the text fallback, this path only runs when stdin *was* a terminal, so + // end of input here is a hangup, not an unattended run — and the default may be + // the destructive answer. No is safe at every call site. guard let byte = readByte() else { - write("\n") - return defaultValue + reportInputClosed() + return false } switch byte { @@ -413,7 +415,7 @@ struct CLIOutput { while true { guard let byte = readByte() else { - write("\n") + reportInputClosed() return cursor } @@ -549,7 +551,7 @@ struct CLIOutput { while true { guard let byte = readByte() else { - write("\n") + reportInputClosed() return collectSelected(from: groups) } @@ -704,15 +706,23 @@ struct CLIOutput { write(output) } - /// One byte from stdin, or `nil` at end of input. + /// Leaves a line in scrollback saying why the prompt ended without a key — the pickers return + /// a value either way, and without this the transcript shows only the question. + private func reportInputClosed() { + write("\n") + warn("Input closed before the prompt was answered.") + } + + /// One byte from `fd`, or `nil` at end of input. /// /// Optional because the pickers below loop until they recognise a key: on EOF or a read error /// no key will ever arrive, and a zero byte matches no case, so returning one would spin at /// 100% CPU until the process is killed. Reachable whenever stdin closes under a raw-mode /// prompt — a hung-up terminal in a `nohup`/`setsid` wrapper that ignores SIGHUP, for instance. - private func readByte() -> UInt8? { + /// The descriptor is a parameter so the EOF path can be driven from a pipe under test. + func readByte(from fd: Int32 = STDIN_FILENO) -> UInt8? { var byte: UInt8 = 0 - guard read(STDIN_FILENO, &byte, 1) > 0 else { return nil } + guard read(fd, &byte, 1) > 0 else { return nil } return byte } diff --git a/Tests/MCSTests/CLIOutputTests.swift b/Tests/MCSTests/CLIOutputTests.swift new file mode 100644 index 00000000..ef7c24df --- /dev/null +++ b/Tests/MCSTests/CLIOutputTests.swift @@ -0,0 +1,42 @@ +import Foundation +@testable import mcs +import Testing + +struct CLIOutputTests { + private func makePipe() -> (read: Int32, write: Int32) { + var fds: [Int32] = [0, 0] + #expect(pipe(&fds) == 0) + return (fds[0], fds[1]) + } + + @Test("readByte returns the byte that was written") + func readByteReturnsWrittenByte() { + let (readEnd, writeEnd) = makePipe() + defer { close(readEnd) } + + var byte: UInt8 = 0x6A + #expect(write(writeEnd, &byte, 1) == 1) + close(writeEnd) + + #expect(CLIOutput(colorsEnabled: false).readByte(from: readEnd) == 0x6A) + } + + @Test("readByte returns nil once the writer has closed, so a picker can exit instead of spinning") + func readByteReturnsNilAtEOF() { + let (readEnd, writeEnd) = makePipe() + defer { close(readEnd) } + close(writeEnd) + + #expect(CLIOutput(colorsEnabled: false).readByte(from: readEnd) == nil) + } + + @Test("readByte returns nil for a descriptor that cannot be read") + func readByteReturnsNilOnReadError() { + let (readEnd, writeEnd) = makePipe() + close(readEnd) + defer { close(writeEnd) } + + // Reading the write end fails with EBADF; the caller must see that as "no key coming". + #expect(CLIOutput(colorsEnabled: false).readByte(from: writeEnd) == nil) + } +} From 680b5d9be1f7f28cc661b4c109ceb9002d884822 Mon Sep 17 00:00:00 2001 From: Bruno Guidolim Date: Wed, 16 Sep 2026 01:09:21 +0200 Subject: [PATCH 25/30] Table-test the PTY bridge's poll decisions - Extract the per-descriptor revents handling into PTYBridge so the /dev/null-stdin spin and the drain-before-drop rule are asserted, not narrated - The Glibc difference in the fork child is __nonnull on execve/chdir, not an optional strdup; say so in the comment and D5 Claude-Session: https://claude.ai/code/session_014tZGxbvJUwXqd6fU1S3fXu --- CLAUDE.md | 3 +- Sources/mcs/Core/PTYBridge.swift | 46 ++++++++++++++++++++++++++++ Sources/mcs/Core/ShellRunner.swift | 29 ++++++++---------- Tests/MCSTests/PTYBridgeTests.swift | 47 +++++++++++++++++++++++++++++ docs/linux-support.md | 11 ++++--- 5 files changed, 114 insertions(+), 22 deletions(-) create mode 100644 Sources/mcs/Core/PTYBridge.swift create mode 100644 Tests/MCSTests/PTYBridgeTests.swift diff --git a/CLAUDE.md b/CLAUDE.md index 2fe4ef71..7f6ebb7a 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -73,6 +73,7 @@ mcs config set # Set a configuration value (true/false) - `Environment.swift` — paths, arch detection, brew path, claude-home cwd detection (`isInsideClaudeHome(_:)`) - `CLIOutput.swift` — ANSI colors, logging, prompts, multi-select, doctor summary - `ShellRunner.swift` — Process execution wrapper +- `PTYBridge.swift` — the per-descriptor `poll(2)` decisions of the interactive PTY bridge, pure so every `revents` combination is table-tested - `Settings.swift` — Codable model for `settings.json` and `settings.local.json`, deep-merge - `Backup.swift` — timestamped backups for mixed-ownership files (CLAUDE.local.md), backup discovery and deletion - `GitignoreManager.swift` — global gitignore management, core entry list @@ -191,7 +192,7 @@ import Darwin import Glibc #endif ``` - Platform-dependent **values and control flow** live only in `Core/TerminalAttributes.swift`, `Core/Environment.swift`, `Core/Homebrew.swift`, `Core/Constants.swift` and `Core/ClaudePrerequisite.swift` — everything else calls into them. The last one is there because what differs is control flow, not a value: macOS offers a Homebrew install of Claude Code and Linux cannot, so there is no constant to move. The `#if canImport(Darwin) / #elseif canImport(Glibc)` import chain at the top of a file that calls libc directly (`ShellRunner`, `CLIOutput`, `FileLock`, `GlobMatcher`) or picks a crypto module (`FileHasher`, `SettingsHasher`, `SectionValidator`) is not platform knowledge in this sense — it selects the same API from a different module — and is allowed wherever it is needed. A `#if` is for a value or API that genuinely differs, never to make a diagnostic go away; the same goes for `_ =`. Several Foundation methods are `@discardableResult` on Darwin and not on Linux — use the result, it always means something. A test gated out on one platform is a coverage regression: give the `#else` branch the equivalent assertion. Rationale and the compatibility matrix are in `docs/linux-support.md` + Platform-dependent **values and control flow** live only in `Core/TerminalAttributes.swift`, `Core/Environment.swift`, `Core/Homebrew.swift`, `Core/Constants.swift` and `Core/ClaudePrerequisite.swift` — everything else calls into them. The last one is there because what differs is control flow, not a value: macOS offers a Homebrew install of Claude Code and Linux cannot, so there is no constant to move. The `#if canImport(Darwin) / #elseif canImport(Glibc)` import chain at the top of a file that calls libc directly (`ShellRunner`, `PTYBridge`, `CLIOutput`, `FileLock`, `GlobMatcher`) or picks a crypto module (`FileHasher`, `SettingsHasher`, `SectionValidator`) is not platform knowledge in this sense — it selects the same API from a different module — and is allowed wherever it is needed. A `#if` is for a value or API that genuinely differs, never to make a diagnostic go away; the same goes for `_ =`. Several Foundation methods are `@discardableResult` on Darwin and not on Linux — use the result, it always means something. A test gated out on one platform is a coverage regression: give the `#else` branch the equivalent assertion. Rationale and the compatibility matrix are in `docs/linux-support.md` - **Comments carry the non-obvious "why", not a narration of the code** — don't restate the line below, don't describe what the code used to do. If the code already says it, delete the comment; if the rationale needs more than a line or two, it belongs in the issue or a memory. State a given rationale once, at the site that owns it, rather than repeating it at every call site ## Testing diff --git a/Sources/mcs/Core/PTYBridge.swift b/Sources/mcs/Core/PTYBridge.swift new file mode 100644 index 00000000..79c48b7f --- /dev/null +++ b/Sources/mcs/Core/PTYBridge.swift @@ -0,0 +1,46 @@ +#if canImport(Darwin) +import Darwin +#elseif canImport(Glibc) +import Glibc +#endif + +/// The per-descriptor decisions of `ShellRunner.runInteractive`'s poll loop, kept free of I/O so +/// every `revents` combination can be tabled in a test. +/// +/// Two facts drive the shapes below. `poll(2)` reports `POLLHUP`, `POLLERR` and `POLLNVAL` +/// whether or not they were requested, so a descriptor that keeps reporting one of them without +/// `POLLIN` makes `poll` return immediately, forever, unless the loop stops watching it — macOS +/// returns `POLLNVAL` for a `/dev/null` stdin, which is what a hook or `mcs sync --all StdinAction { + if revents & Int16(POLLIN) != 0 { return .forward } + if revents & Int16(POLLHUP | POLLERR | POLLNVAL) != 0 { return .drop } + return .idle + } + + static func ptyAction(revents: Int16) -> PTYAction { + if revents & Int16(POLLERR | POLLNVAL) != 0 { return .close } + if revents & Int16(POLLIN | POLLHUP) != 0 { return .read } + return .idle + } +} diff --git a/Sources/mcs/Core/ShellRunner.swift b/Sources/mcs/Core/ShellRunner.swift index 0b5963b4..f1ed007b 100644 --- a/Sources/mcs/Core/ShellRunner.swift +++ b/Sources/mcs/Core/ShellRunner.swift @@ -209,8 +209,9 @@ struct ShellRunner: ShellRunning { // an allocation failure, and a force-unwrap there would trap through the Swift runtime. // Every C string the child needs is therefore allocated and checked here, in the parent — // a nil in the middle of argv or envp would otherwise truncate the vector at execve, which - // stops at the first NULL. (Glibc types `strdup` as returning an optional; Darwin - // implicitly unwraps it.) + // stops at the first NULL. (Glibc marks the `execve`/`chdir` parameters `__nonnull`, so + // Swift imports them non-optional there and rejects the `Optional` Darwin's unannotated + // signatures accept.) guard let executablePath = strdup(executable) else { return ShellResult( exitCode: 1, stdout: "", stderr: "Could not allocate the command path for \(executable)" @@ -270,7 +271,6 @@ struct ShellRunner: ShellRunning { _exit(126) } } - // Use the pre-allocated C string instead of the Swift String. execve(executablePath, argv, envp) let err = strerror(errno) _ = write(STDERR_FILENO, "execve failed: ", 15) @@ -315,30 +315,27 @@ struct ShellRunner: ShellRunning { break } - // An errored PTY means the command is gone, so it ends the bridge. - if fds[1].revents & Int16(POLLERR | POLLNVAL) != 0 { - break bridgeLoop - } + let ptyAction = PTYBridge.ptyAction(revents: fds[1].revents) + if ptyAction == .close { break bridgeLoop } - // Terminal → PTY (user typing, including password input). Readable data is drained - // first — a hung-up pipe can still carry buffered bytes — and only a descriptor with - // nothing to read and nothing coming (hangup, error, closed) is dropped. Dropping it - // matters: poll(2) reports POLLHUP/POLLERR whether or not they were requested, so - // leaving the fd in the set would make poll return immediately, forever, with no - // branch taken. poll(2) skips negative fds, so the loop carries on draining the PTY. - if fds[0].revents & Int16(POLLIN) != 0 { + // Terminal → PTY (user typing, including password input). poll(2) skips negative + // fds, so a dropped stdin leaves the loop draining the PTY alone. + switch PTYBridge.stdinAction(revents: fds[0].revents) { + case .forward: let n = read(STDIN_FILENO, &buf, buf.count) if n <= 0 { fds[0].fd = -1 } else { writeAll(fd: ptyFD, buf: buf, count: n) } - } else if fds[0].revents & Int16(POLLHUP | POLLERR | POLLNVAL) != 0 { + case .drop: fds[0].fd = -1 + case .idle: + break } // PTY → Terminal (command output, prompts, progress bars) - if fds[1].revents & Int16(POLLIN | POLLHUP) != 0 { + if ptyAction == .read { let n = read(ptyFD, &buf, buf.count) if n <= 0 { break bridgeLoop } // Child closed the PTY writeAll(fd: STDOUT_FILENO, buf: buf, count: n) diff --git a/Tests/MCSTests/PTYBridgeTests.swift b/Tests/MCSTests/PTYBridgeTests.swift new file mode 100644 index 00000000..ca67dd27 --- /dev/null +++ b/Tests/MCSTests/PTYBridgeTests.swift @@ -0,0 +1,47 @@ +import Foundation +@testable import mcs +import Testing + +struct PTYBridgeTests { + @Test( + "The terminal side forwards while readable and drops only once nothing more can arrive", + arguments: [ + (Int16(POLLIN), PTYBridge.StdinAction.forward), + (Int16(POLLIN | POLLHUP), .forward), + (Int16(POLLHUP), .drop), + (Int16(POLLERR), .drop), + (Int16(POLLNVAL), .drop), + (Int16(0), .idle), + ] + ) + func stdinAction(revents: Int16, expected: PTYBridge.StdinAction) { + #expect(PTYBridge.stdinAction(revents: revents) == expected) + } + + @Test( + "The child side reads through a hangup and closes only on a bad descriptor", + arguments: [ + (Int16(POLLIN), PTYBridge.PTYAction.read), + (Int16(POLLIN | POLLHUP), .read), + (Int16(POLLHUP), .read), + (Int16(POLLERR), .close), + (Int16(POLLNVAL), .close), + (Int16(POLLIN | POLLERR), .close), + (Int16(0), .idle), + ] + ) + func ptyAction(revents: Int16, expected: PTYBridge.PTYAction) { + #expect(PTYBridge.ptyAction(revents: revents) == expected) + } + + @Test("A /dev/null stdin, which macOS reports as POLLNVAL, is dropped rather than polled forever") + func devNullStdinIsDropped() { + let fd = open("/dev/null", O_RDONLY) + #expect(fd >= 0) + defer { close(fd) } + + var fds = [pollfd(fd: fd, events: Int16(POLLIN), revents: 0)] + #expect(poll(&fds, 1, 0) == 1) + #expect(PTYBridge.stdinAction(revents: fds[0].revents) != .idle) + } +} diff --git a/docs/linux-support.md b/docs/linux-support.md index b60e4611..68458393 100644 --- a/docs/linux-support.md +++ b/docs/linux-support.md @@ -259,11 +259,12 @@ and the call links and runs on glibc 2.39 with no extra linker flag. `cfmakeraw` not missing; reimplementing with `posix_openpt`/`grantpt`/`setsid`/`ioctl(TIOCSCTTY)` duplicates forty lines of subtle libc behaviour for no benefit. -**Consequences.** One real difference did surface inside the fork child: Glibc types `strdup` as -returning an optional where Darwin implicitly unwraps it, so the child was left dereferencing -optionals for the command path and the working directory. A force-unwrap there would trap through -the Swift runtime after `fork()`, where only async-signal-safe calls are legal, so both strings are -now allocated and checked in the parent. If a future toolchain drops `pty.h` the build fails +**Consequences.** One real difference did surface inside the fork child: glibc declares the +`execve` and `chdir` parameters `__nonnull`, so Swift imports them as non-optional and rejects the +`Optional` element of the `argv` array that Darwin's unannotated signatures accept (`strdup` itself +imports as an implicitly unwrapped optional on both). A force-unwrap there would trap through the +Swift runtime after `fork()`, where only async-signal-safe calls are legal, so every C string the +child needs is allocated and checked in the parent. If a future toolchain drops `pty.h` the build fails immediately and the C shim remains a ~15-line fallback. ### D6 — termios: index `c_cc` through `VMIN`/`VTIME`, widen masks through `tcflag_t` From 10dd1a9c7a9da5d7e251b06bdc1e20eccbce6319 Mon Sep 17 00:00:00 2001 From: Bruno Guidolim Date: Wed, 16 Sep 2026 01:10:10 +0200 Subject: [PATCH 26/30] Print why an interactive command failed when the bridge itself did - A forkpty or allocation failure never reaches the terminal, so "see output above" pointed at nothing; the result's stderr is the only record - One pure message function shared by the three interactive call sites Claude-Session: https://claude.ai/code/session_014tZGxbvJUwXqd6fU1S3fXu --- Sources/mcs/Core/ShellRunner.swift | 8 ++++++++ Sources/mcs/Sync/GlobalSyncStrategy.swift | 2 +- Sources/mcs/Sync/PackInstaller.swift | 2 +- Sources/mcs/Sync/ProjectSyncStrategy.swift | 2 +- Tests/MCSTests/ShellRunnerTests.swift | 12 ++++++++++++ 5 files changed, 23 insertions(+), 3 deletions(-) diff --git a/Sources/mcs/Core/ShellRunner.swift b/Sources/mcs/Core/ShellRunner.swift index f1ed007b..7c1e94e4 100644 --- a/Sources/mcs/Core/ShellRunner.swift +++ b/Sources/mcs/Core/ShellRunner.swift @@ -77,6 +77,14 @@ extension ShellRunning { /// Runs shell commands and captures output. struct ShellRunner: ShellRunning { + /// The warning for a failed interactive command. The child's own output already went to the + /// terminal, so the result's `stderr` is empty unless the bridge itself failed before `exec` — + /// `forkpty` refused, a C string could not be allocated — and then it is the only record of + /// why, so it is what gets printed. + static func interactiveFailureMessage(name: String, stderr: String) -> String { + stderr.isEmpty ? "\(name) failed (see output above)" : "\(name) failed: \(stderr)" + } + let environment: Environment /// Check if a command exists on PATH. diff --git a/Sources/mcs/Sync/GlobalSyncStrategy.swift b/Sources/mcs/Sync/GlobalSyncStrategy.swift index 7830a0f7..d3ba3873 100644 --- a/Sources/mcs/Sync/GlobalSyncStrategy.swift +++ b/Sources/mcs/Sync/GlobalSyncStrategy.swift @@ -137,7 +137,7 @@ struct GlobalSyncStrategy: SyncStrategy { if result.succeeded { output.success(" \(component.displayName) installed") } else if interactive { - output.warn(" \(component.displayName) failed (see output above)") + output.warn(" " + ShellRunner.interactiveFailureMessage(name: component.displayName, stderr: result.stderr)) } else { output.warn(" \(component.displayName) requires manual installation:") output.plain(" \(command)") diff --git a/Sources/mcs/Sync/PackInstaller.swift b/Sources/mcs/Sync/PackInstaller.swift index 911d3705..c00d2fd8 100644 --- a/Sources/mcs/Sync/PackInstaller.swift +++ b/Sources/mcs/Sync/PackInstaller.swift @@ -96,7 +96,7 @@ struct PackInstaller { let result = shell.shell(command, interactive: interactive) if !result.succeeded { if interactive { - output.warn(" \(component.displayName) failed (see output above)") + output.warn(" " + ShellRunner.interactiveFailureMessage(name: component.displayName, stderr: result.stderr)) } else { output.warn(String(result.stderr.prefix(200))) } diff --git a/Sources/mcs/Sync/ProjectSyncStrategy.swift b/Sources/mcs/Sync/ProjectSyncStrategy.swift index 042693b6..3ad99a8f 100644 --- a/Sources/mcs/Sync/ProjectSyncStrategy.swift +++ b/Sources/mcs/Sync/ProjectSyncStrategy.swift @@ -115,7 +115,7 @@ struct ProjectSyncStrategy: SyncStrategy { let result = shell.shell(command, interactive: interactive) if !result.succeeded { if interactive { - output.warn(" \(component.displayName) failed (see output above)") + output.warn(" " + ShellRunner.interactiveFailureMessage(name: component.displayName, stderr: result.stderr)) } else { output.warn(" \(component.displayName) failed: \(String(result.stderr.prefix(200)))") } diff --git a/Tests/MCSTests/ShellRunnerTests.swift b/Tests/MCSTests/ShellRunnerTests.swift index 15e2195d..afa7eddf 100644 --- a/Tests/MCSTests/ShellRunnerTests.swift +++ b/Tests/MCSTests/ShellRunnerTests.swift @@ -7,6 +7,18 @@ struct ShellRunnerTests { ShellRunner(environment: Environment()) } + @Test("An interactive failure points at the terminal unless the bridge itself reported why") + func interactiveFailureMessageShapes() { + #expect( + ShellRunner.interactiveFailureMessage(name: "Ollama", stderr: "") + == "Ollama failed (see output above)" + ) + #expect( + ShellRunner.interactiveFailureMessage(name: "Ollama", stderr: "forkpty failed: No such file or directory") + == "Ollama failed: forkpty failed: No such file or directory" + ) + } + @Test("run captures stdout") func capturesStdout() { let result = shell.run("/bin/echo", arguments: ["hello"]) From 0a9adf23d1a2865e2f4db2c54069c62b4f8b2772 Mon Sep 17 00:00:00 2001 From: Bruno Guidolim Date: Wed, 16 Sep 2026 01:11:37 +0200 Subject: [PATCH 27/30] Make every branch of ensureClaudeCLI testable without a host brew or a terminal - brewInstalled and confirmInstall default to the real check and prompt; tests inject both, so the macOS decline/accept paths are asserted instead of skipped - Collapse the comment-only Darwin branch in the manual instructions Claude-Session: https://claude.ai/code/session_014tZGxbvJUwXqd6fU1S3fXu --- Sources/mcs/Core/ClaudePrerequisite.swift | 18 ++++-- Tests/MCSTests/ClaudePrerequisiteTests.swift | 65 +++++++++++++++----- 2 files changed, 60 insertions(+), 23 deletions(-) diff --git a/Sources/mcs/Core/ClaudePrerequisite.swift b/Sources/mcs/Core/ClaudePrerequisite.swift index 42e5e8d2..5f73509a 100644 --- a/Sources/mcs/Core/ClaudePrerequisite.swift +++ b/Sources/mcs/Core/ClaudePrerequisite.swift @@ -4,11 +4,17 @@ import Foundation /// /// Returns `true` if Claude CLI is available (either already installed or successfully installed). /// Returns `false` if the user declines installation or installation fails. +/// +/// `brewInstalled` and `confirmInstall` default to the real filesystem check and the real prompt; +/// tests pass both so every macOS branch runs without a Homebrew on the host or a terminal on +/// stdin. @discardableResult func ensureClaudeCLI( shell: any ShellRunning, environment: Environment, - output: CLIOutput + output: CLIOutput, + brewInstalled: Bool? = nil, + confirmInstall: (() -> Bool)? = nil ) -> Bool { if shell.commandExists(Constants.CLI.claudeCommand) { return true @@ -19,12 +25,13 @@ func ensureClaudeCLI( #if canImport(Darwin) let brew = Homebrew(shell: shell, environment: environment) - guard brew.isInstalled else { + guard brewInstalled ?? brew.isInstalled else { printManualClaudeInstallInstructions(output) return false } - guard output.askYesNo("Install Claude Code via Homebrew?", default: true) else { + let confirm = confirmInstall ?? { output.askYesNo("Install Claude Code via Homebrew?", default: true) } + guard confirm() else { printManualClaudeInstallInstructions(output) return false } @@ -54,9 +61,8 @@ func ensureClaudeCLI( /// shell, or writing into npm's global prefix, is a trust decision that belongs to the user. private func printManualClaudeInstallInstructions(_ output: CLIOutput) { output.plain(" Install it manually: https://docs.anthropic.com/en/docs/claude-code") - #if canImport(Darwin) - // macOS has already been offered the Homebrew cask, so the docs link is the whole answer. - #else + #if !canImport(Darwin) + // On macOS the caller owns the Homebrew route; here the manual ones are all there is. output.plain(" curl -fsSL https://claude.ai/install.sh | bash") output.plain(" npm install -g @anthropic-ai/claude-code") #endif diff --git a/Tests/MCSTests/ClaudePrerequisiteTests.swift b/Tests/MCSTests/ClaudePrerequisiteTests.swift index 0fc4c480..8d227d0e 100644 --- a/Tests/MCSTests/ClaudePrerequisiteTests.swift +++ b/Tests/MCSTests/ClaudePrerequisiteTests.swift @@ -17,26 +17,57 @@ struct ClaudePrerequisiteTests { #expect(shell.runCalls.isEmpty) } - @Test("A missing claude is reported, and brew is never asked to install it without brew") + @Test("A missing claude without brew is reported and nothing is installed") func missingCLIWithoutHomebrew() { let shell = MockShellRunner() shell.commandExistsResult = false - let environment = shell.environment - - #if canImport(Darwin) - // macOS offers the Homebrew cask when brew is there, and that offer is a prompt — calling - // it here would read real stdin under swift-testing's parallel runner. So on a machine that - // does have brew, assert instead what makes the guard safe to evaluate: it is a filesystem - // check, not a subprocess, which is why gating on it costs nothing. - guard !Homebrew(shell: shell, environment: environment).isInstalled else { - #expect(shell.runCalls.isEmpty) - return - } - #endif - - // Without brew — always on Linux, where `claude-code` is a cask Linuxbrew does not have — - // the answer is the same on both platforms: report it and do not try to install anything. - #expect(!ensureClaudeCLI(shell: shell, environment: environment, output: silentOutput())) + + // The same on both platforms: on Linux `claude-code` is a cask Linuxbrew does not have. + #expect(!ensureClaudeCLI( + shell: shell, environment: shell.environment, output: silentOutput(), + brewInstalled: false, confirmInstall: { Issue.record("no prompt without brew"); return true } + )) #expect(shell.runCalls.isEmpty, "mcs must not attempt an install it cannot perform") } + + #if canImport(Darwin) + @Test("Declining the Homebrew offer installs nothing") + func declinedHomebrewInstall() { + let shell = MockShellRunner() + shell.commandExistsResult = false + + #expect(!ensureClaudeCLI( + shell: shell, environment: shell.environment, output: silentOutput(), + brewInstalled: true, confirmInstall: { false } + )) + #expect(shell.runCalls.isEmpty) + } + + @Test("Accepting the Homebrew offer runs brew install, then re-probes for claude") + func acceptedHomebrewInstall() { + let shell = MockShellRunner() + shell.commandExistsResult = false + + // The install "succeeds" but claude still is not on PATH, so the outcome is a failure — + // the assertion that matters is which command was run. + #expect(!ensureClaudeCLI( + shell: shell, environment: shell.environment, output: silentOutput(), + brewInstalled: true, confirmInstall: { true } + )) + #expect(shell.runCalls.map(\.arguments) == [["install", "claude-code"]]) + #expect(shell.runCalls.first?.executable == shell.environment.brewPath) + } + #else + @Test("Linux never offers the Homebrew cask, even with brew present and a willing user") + func linuxNeverOffersHomebrew() { + let shell = MockShellRunner() + shell.commandExistsResult = false + + #expect(!ensureClaudeCLI( + shell: shell, environment: shell.environment, output: silentOutput(), + brewInstalled: true, confirmInstall: { Issue.record("no prompt on Linux"); return true } + )) + #expect(shell.runCalls.isEmpty) + } + #endif } From ad88107cb8bbdd5cdbf0a0e578670a66ced82083 Mon Sep 17 00:00:00 2001 From: Bruno Guidolim Date: Wed, 16 Sep 2026 01:12:23 +0200 Subject: [PATCH 28/30] Assert the doctor hints as they are, not as the Linux wording assumed - The brew-absent branch failed on a Mac without Homebrew: the Darwin advice does end in 'mcs sync', after installing brew - Pin the command name in the commandExists hint Claude-Session: https://claude.ai/code/session_014tZGxbvJUwXqd6fU1S3fXu --- Tests/MCSTests/DoctorRunnerIntegrationTests.swift | 6 ++++-- Tests/MCSTests/ExternalDoctorCheckTests.swift | 8 ++++---- Tests/MCSTests/HomebrewTests.swift | 2 +- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/Tests/MCSTests/DoctorRunnerIntegrationTests.swift b/Tests/MCSTests/DoctorRunnerIntegrationTests.swift index a5430f8a..8db2b349 100644 --- a/Tests/MCSTests/DoctorRunnerIntegrationTests.swift +++ b/Tests/MCSTests/DoctorRunnerIntegrationTests.swift @@ -639,12 +639,14 @@ struct BrewPackageCheckGuidanceTests { } // Asserted against whichever machine runs this: with brew present `mcs sync` really can - // install the package, without it that advice is a loop with no exit. + // install the package; without it the hint must name the package and where to get brew or + // the package (the Darwin text still ends in `mcs sync`, after installing brew). if FileManager.default.fileExists(atPath: Environment().brewPath) { #expect(message.contains("mcs sync")) } else { - #expect(!message.contains("mcs sync")) + #expect(message == Homebrew.manualInstallAdvice(for: package)) #expect(message.contains(package)) + #expect(message.contains("brew.sh") || message.contains("system package manager")) } } } diff --git a/Tests/MCSTests/ExternalDoctorCheckTests.swift b/Tests/MCSTests/ExternalDoctorCheckTests.swift index ca027379..e0dafebb 100644 --- a/Tests/MCSTests/ExternalDoctorCheckTests.swift +++ b/Tests/MCSTests/ExternalDoctorCheckTests.swift @@ -74,7 +74,7 @@ struct ExternalDoctorCheckTests { } } - @Test("Command exists fix returns notFixable when no fix command") + @Test("Command exists fix returns notFixable naming the command when no fix command") func commandExistsNoFix() { let check = ExternalCommandExistsCheck( name: "test", @@ -85,11 +85,11 @@ struct ExternalDoctorCheckTests { scriptRunner: makeScriptRunner() ) let result = check.fix() - if case .notFixable = result { - // expected - } else { + guard case let .notFixable(message) = result else { Issue.record("Expected .notFixable, got \(result)") + return } + #expect(message.contains("'nonexistent'")) } // MARK: - ExternalFileExistsCheck diff --git a/Tests/MCSTests/HomebrewTests.swift b/Tests/MCSTests/HomebrewTests.swift index 06d405b0..51d69c43 100644 --- a/Tests/MCSTests/HomebrewTests.swift +++ b/Tests/MCSTests/HomebrewTests.swift @@ -35,7 +35,7 @@ struct HomebrewTests { // MARK: - Guidance when Homebrew is absent - @Test("Install advice names the package and never sends the user back to 'mcs sync'") + @Test("Install advice names the package and the platform's way to get it") func manualInstallAdviceIsActionable() { let advice = Homebrew.manualInstallAdvice(for: "ripgrep") #expect(advice.contains("ripgrep")) From cf6405a1ad30e954f99380d714ec65b43ed1977e Mon Sep 17 00:00:00 2001 From: Bruno Guidolim Date: Wed, 16 Sep 2026 01:13:23 +0200 Subject: [PATCH 29/30] Correct the comments and docs the port left imprecise - Name the platforms instead of "there/here", say CryptoKit is Apple-only, drop the FileHasher history line, count D4's methods and call sites correctly - The package-manager hint is a --fix hint; CI wording no longer refers to this PR; zsh's absence on Linux is a listed limitation Claude-Session: https://claude.ai/code/session_014tZGxbvJUwXqd6fU1S3fXu --- CLAUDE.md | 2 +- Sources/mcs/Core/FileHasher.swift | 7 +++---- Sources/mcs/Core/TerminalAttributes.swift | 7 ++++--- Sources/mcs/TechPack/HookInterpreter.swift | 5 ++++- docs/creating-tech-packs.md | 2 +- docs/linux-support.md | 19 ++++++++++++------- docs/techpack-schema.md | 4 ++-- 7 files changed, 27 insertions(+), 19 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 7f6ebb7a..25fe0f23 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -192,7 +192,7 @@ import Darwin import Glibc #endif ``` - Platform-dependent **values and control flow** live only in `Core/TerminalAttributes.swift`, `Core/Environment.swift`, `Core/Homebrew.swift`, `Core/Constants.swift` and `Core/ClaudePrerequisite.swift` — everything else calls into them. The last one is there because what differs is control flow, not a value: macOS offers a Homebrew install of Claude Code and Linux cannot, so there is no constant to move. The `#if canImport(Darwin) / #elseif canImport(Glibc)` import chain at the top of a file that calls libc directly (`ShellRunner`, `PTYBridge`, `CLIOutput`, `FileLock`, `GlobMatcher`) or picks a crypto module (`FileHasher`, `SettingsHasher`, `SectionValidator`) is not platform knowledge in this sense — it selects the same API from a different module — and is allowed wherever it is needed. A `#if` is for a value or API that genuinely differs, never to make a diagnostic go away; the same goes for `_ =`. Several Foundation methods are `@discardableResult` on Darwin and not on Linux — use the result, it always means something. A test gated out on one platform is a coverage regression: give the `#else` branch the equivalent assertion. Rationale and the compatibility matrix are in `docs/linux-support.md` + Platform-dependent **values and control flow** live only in `Core/TerminalAttributes.swift`, `Core/Environment.swift`, `Core/Homebrew.swift`, `Core/Constants.swift` and `Core/ClaudePrerequisite.swift` — everything else calls into them. `TerminalAttributes` is on the list without a single `#if`: it owns the termios layout, which differs per platform through `VMIN`/`VTIME`/`tcflag_t` rather than through a branch. `ClaudePrerequisite` is there because what differs is control flow, not a value: macOS offers a Homebrew install of Claude Code and Linux cannot, so there is no constant to move. The `#if canImport(Darwin) / #elseif canImport(Glibc)` import chain at the top of a file that calls libc directly (`ShellRunner`, `PTYBridge`, `CLIOutput`, `FileLock`, `GlobMatcher`) or picks a crypto module (`FileHasher`, `SettingsHasher`, `SectionValidator`) is not platform knowledge in this sense — it selects the same API from a different module — and is allowed wherever it is needed. A `#if` is for a value or API that genuinely differs, never to make a diagnostic go away; the same goes for `_ =`. Several Foundation methods are `@discardableResult` on Darwin and not on Linux — use the result, it always means something. A test gated out on one platform is a coverage regression: give the `#else` branch the equivalent assertion. Rationale and the compatibility matrix are in `docs/linux-support.md` - **Comments carry the non-obvious "why", not a narration of the code** — don't restate the line below, don't describe what the code used to do. If the code already says it, delete the comment; if the rationale needs more than a line or two, it belongs in the issue or a memory. State a given rationale once, at the site that owns it, rather than repeating it at every call site ## Testing diff --git a/Sources/mcs/Core/FileHasher.swift b/Sources/mcs/Core/FileHasher.swift index a68f8ba9..e6c91774 100644 --- a/Sources/mcs/Core/FileHasher.swift +++ b/Sources/mcs/Core/FileHasher.swift @@ -1,7 +1,7 @@ #if canImport(CryptoKit) import CryptoKit #else -// Foundation has no SHA-256 on Linux, so the Glibc build gets the digest from swift-crypto, whose +// CryptoKit is Apple-only, so the Glibc build gets the digest from swift-crypto, whose // `Crypto` module is API-identical. The dependency is declared for Linux only so macOS keeps // hashing through the system CryptoKit — a digest written into `.mcs-project` state on one // platform has to validate on the other. @@ -9,9 +9,8 @@ import Crypto #endif import Foundation -/// SHA-256 file hashing utilities. -/// Extracted from the deleted `Manifest` type — used by `PackTrustManager` -/// for trust verification and by `ComponentExecutor` for directory copies. +/// SHA-256 file hashing utilities, used by `PackTrustManager` for trust verification and by +/// `ComponentExecutor` for directory copies. enum FileHasher { /// Compute SHA-256 hash of a file. static func sha256(of url: URL) throws -> String { diff --git a/Sources/mcs/Core/TerminalAttributes.swift b/Sources/mcs/Core/TerminalAttributes.swift index 4fa92e35..24df4882 100644 --- a/Sources/mcs/Core/TerminalAttributes.swift +++ b/Sources/mcs/Core/TerminalAttributes.swift @@ -7,9 +7,10 @@ import Glibc /// termios helpers shared by the raw-mode picker. /// /// `c_cc` imports as a fixed-size tuple whose length and element order are platform-defined -/// (`NCCS` is 20 on Darwin and 32 on Glibc; `VMIN` is index 16 there and 6 here), so the index has -/// to come from the platform's own constant rather than a literal. `tcflag_t` likewise differs — -/// `UInt` on Darwin, `UInt32` on Glibc — so every flag mask is widened through `tcflag_t(...)`. +/// (`NCCS` is 20 on Darwin and 32 on Glibc; `VMIN` is index 16 on Darwin and 6 on Glibc), so the +/// index has to come from the platform's own constant rather than a literal. `tcflag_t` likewise +/// differs — `UInt` on Darwin, `UInt32` on Glibc — so every flag mask is converted through +/// `tcflag_t(...)`. enum TerminalAttributes { /// Reads one control character. Exists for `TerminalAttributesTests`: asserting raw mode /// without a TTY is the only way to cover this on both platforms. diff --git a/Sources/mcs/TechPack/HookInterpreter.swift b/Sources/mcs/TechPack/HookInterpreter.swift index c894b50a..cee5fa8d 100644 --- a/Sources/mcs/TechPack/HookInterpreter.swift +++ b/Sources/mcs/TechPack/HookInterpreter.swift @@ -33,7 +33,10 @@ enum HookInterpreter { /// author at publish time to declare `hookInterpreter` explicitly. private static let ambiguousExtensions: Set = ["ts", "mts", "cts", "tsx"] - /// Interpreters whose absence is not worth reporting — always present on macOS. + /// Interpreters whose absence is not worth reporting — always present on macOS. `zsh` is not + /// installed by default on Debian, Ubuntu or Fedora, so a `.zsh` hook's missing interpreter goes + /// unreported there; a per-platform set would be a sixth home for platform knowledge for one + /// entry, so this is listed as a known limitation instead. private static let assumedPresent: Set = ["bash", "sh", "zsh"] /// Longest interpreter string accepted, guarding against a pathological manifest value. diff --git a/docs/creating-tech-packs.md b/docs/creating-tech-packs.md index 2f556116..f276da93 100644 --- a/docs/creating-tech-packs.md +++ b/docs/creating-tech-packs.md @@ -122,7 +122,7 @@ components: When a user runs `mcs sync`, these get installed via `brew install`. The engine auto-verifies them with `mcs doctor`, which looks for the command on `PATH` and falls back to `brew list` — so a cask, a versioned formula like `node@22`, or a formula whose command is spelled differently (`ripgrep` installs `rg`) still verifies correctly. -> **On Linux, Homebrew is usually absent.** Verification then has only the `PATH` probe, so a `brew:` component is satisfied only when the **formula name is also the command name**: `brew: node` passes when `node` is on `PATH`, `brew: ripgrep` does not, because the command is `rg`. mcs cannot install it either — that needs [Linuxbrew](https://docs.brew.sh/Homebrew-on-Linux) — so `mcs doctor` reports the package missing and tells the user to install it with their system package manager. Prefer formulae whose name matches their command when you can, and say in your pack's description which tools a Linux user has to install themselves. There is no per-platform component gating in `techpack.yaml` yet; see [Linux support](linux-support.md). +> **On Linux, Homebrew is usually absent.** Verification then has only the `PATH` probe, so a `brew:` component is satisfied only when the **formula name is also the command name**: `brew: node` passes when `node` is on `PATH`, `brew: ripgrep` does not, because the command is `rg`. mcs cannot install it either — that needs [Linuxbrew](https://docs.brew.sh/Homebrew-on-Linux) — so `mcs doctor` reports the package missing and `mcs doctor --fix` names the system package manager as the way to get it. Prefer formulae whose name matches their command when you can, and say in your pack's description which tools a Linux user has to install themselves. There is no per-platform component gating in `techpack.yaml` yet; see [Linux support](linux-support.md). A tap-qualified package (`owner/tap/formula`) works too, but note that installing one taps a third-party repository without asking anyone; `mcs pack validate` warns when your pack declares one. diff --git a/docs/linux-support.md b/docs/linux-support.md index 68458393..8b675591 100644 --- a/docs/linux-support.md +++ b/docs/linux-support.md @@ -17,12 +17,11 @@ how each one was verified; six features behave differently from macOS, and the m | Architecture | x86_64. No aarch64 artifact is published yet. | | libc | glibc. musl is untested; there is no `canImport(Musl)` branch. | | glibc floor | 2.35 — the published binary is built on Ubuntu 22.04. | -| Tested on | Ubuntu 24.04 (development); the `ubuntu-latest` GitHub runner once this PR's CI has run. | +| Tested on | Ubuntu 24.04 (development); the `ubuntu-latest` GitHub runner in CI. | | Other distributions | Any glibc ≥ 2.35 distribution is expected to work. **Nothing is claimed about Fedora, Alpine or NixOS** — they have not been tested. | CI is configured to run `swift build`, the full test suite and the release build on Linux for every -pull request, alongside the two macOS jobs; the Linux job lands with this change, so its first run is -this PR's. Lint runs on macOS only: SwiftFormat and SwiftLint give the same verdicts on both, so a +pull request, alongside the two macOS jobs. Lint runs on macOS only: SwiftFormat and SwiftLint give the same verdicts on both, so a second run would only add version skew. ## 2. Prerequisites @@ -108,7 +107,7 @@ Legend: `verified` — run on Linux and observed; `verified (with a difference)` | Non-TTY fallback picker | supported | verified | stdin a PTY, stdout a pipe: numeric toggle + `Enter`, colours disabled. | | `shell:` components | supported | verified | `shell: touch ` created the marker. | | `shellInteractive: true` (PTY/sudo) | supported | verified | `forkpty` path ran the command; marker file created, with stdin from `/dev/null` and under a real controlling terminal. Also asserted by `LifecycleIntegrationTests`. The PTY is allocated with a default 0×0 window size on both platforms. | -| `brew:` components | supported | verified (with a difference) | Same predicate on both platforms. Without Linuxbrew a `brew:` component is satisfied only when the formula name is also the command name — `brew: node` passes with `node` on PATH, `brew: ripgrep` does not because the command is `rg`. Doctor then reports it missing and names the system package manager instead of pointing back at `mcs sync`. Installing a formula still needs Linuxbrew. | +| `brew:` components | supported | verified (with a difference) | Same predicate on both platforms. Without Linuxbrew a `brew:` component is satisfied only when the formula name is also the command name — `brew: node` passes with `node` on PATH, `brew: ripgrep` does not because the command is `rg`. Doctor then reports it missing, and `--fix` names the system package manager instead of pointing back at `mcs sync`. Installing a formula still needs Linuxbrew. | | `mcp:` (`claude mcp add`) | supported | verified | `demo-server` registered via `claude mcp add -s local`; `claude mcp list` shows it; removal deregisters it. | | `plugin:` | supported | verified | `hookify@claude-code-plugins` installed through `claude plugin install`; `claude plugin list` shows it enabled. | | `hook:` / `command:` / `skill:` / `agent:` copies | supported | verified | All four installed; hooks namespaced under `/`; interpreter inferred (`bash` for `.sh`, `python3` for `.py`). | @@ -239,9 +238,11 @@ import Glibc SwiftGlibc — see D5. No `canImport(Musl)` branch is added: nothing here builds for musl, and an untested branch is worse than no branch. -**Consequence beyond the imports.** Four Foundation methods are `@discardableResult` on Darwin and -not on Linux, so their results had to be used rather than dropped. Three sites now require the file -to exist before treating it as a directory, which is behaviour-identical. The fourth is a small +**Consequence beyond the imports.** Two Foundation methods — `fileExists(atPath:isDirectory:)` and +`createFile(atPath:contents:)` — are `@discardableResult` on Darwin and not on Linux, so their +results had to be used rather than dropped, at four call sites. Three now require the file to exist +before treating it as a directory: identical in practice, and now defined, where Apple documents +`isDirectory` as undefined for a missing path. The fourth is a small **macOS-visible change**: `GitignoreManager`'s bootstrap used `createFile(atPath:contents:)`, whose `false` return was ignored, and now writes through `Data` — so a failure to create the global gitignore throws out of `ensureFileExists()` instead of passing silently and failing later at the @@ -417,6 +418,10 @@ site ever accepted it. section 2. - **No Linuxbrew auto-install**, and **no Claude Code auto-install** on Linux. - **`brew:` components are name-sensitive** without Linuxbrew — see D8 decision 3. +- **A `.zsh` hook's missing interpreter is not reported on Linux.** `HookInterpreter` treats `zsh` + as always present, which is true on macOS and false on Debian, Ubuntu and Fedora by default. Packs + that ship zsh hooks should say so; a per-platform set was not worth a sixth home for platform + knowledge. - **`mcs export`'s brew formula hints are empty** without Linuxbrew: `detectFormula` reads symlinks under the Homebrew prefixes. - **The PTY is allocated with a 0×0 window size** — the same on macOS, so full-screen TUIs run inside diff --git a/docs/techpack-schema.md b/docs/techpack-schema.md index 6628f9e1..5466e15f 100644 --- a/docs/techpack-schema.md +++ b/docs/techpack-schema.md @@ -69,8 +69,8 @@ component. > **Linux**: verification falls back to `brew list` only when Homebrew is installed, which on Linux > it usually is not. Without it a `brew:` component is satisfied only when the formula name is also > the command name (`node` yes, `ripgrep` → `rg` no), and `mcs` cannot install the formula — that -> needs Linuxbrew. `mcs doctor` reports the package missing and points the user at their system -> package manager. There is no per-platform component gating in this schema yet, so a macOS-only +> needs Linuxbrew. `mcs doctor` reports the package missing and `mcs doctor --fix` points the user at +> their system package manager. There is no per-platform component gating in this schema yet, so a macOS-only > formula will simply fail its component on Linux with that message. See > [Linux support](linux-support.md). From 0bfda1ffd9350109583721b66f41945713220a39 Mon Sep 17 00:00:00 2001 From: Bruno Guidolim Date: Wed, 16 Sep 2026 01:14:11 +0200 Subject: [PATCH 30/30] Install the Linux toolchain through one composite action - The libcurl + swiftly + PATH block was copied into three jobs across two workflows; a toolchain tweak now happens once and cannot drift between them - Say what libcurl is for: swiftly checks Swift's system deps, it builds nothing Claude-Session: https://claude.ai/code/session_014tZGxbvJUwXqd6fU1S3fXu --- .github/actions/setup-swift-linux/action.yml | 34 ++++++++++++++ .github/workflows/pr-checks.yml | 25 ++-------- .github/workflows/release.yml | 48 +++----------------- 3 files changed, 45 insertions(+), 62 deletions(-) create mode 100644 .github/actions/setup-swift-linux/action.yml diff --git a/.github/actions/setup-swift-linux/action.yml b/.github/actions/setup-swift-linux/action.yml new file mode 100644 index 00000000..31cc85df --- /dev/null +++ b/.github/actions/setup-swift-linux/action.yml @@ -0,0 +1,34 @@ +name: Set up Swift on Linux +description: Install a pinned Swift toolchain on an Ubuntu runner via swiftly and put it on PATH. + +inputs: + swift-version: + description: The toolchain version swiftly installs and selects (for example 6.2.0). + required: true + +runs: + using: composite + steps: + - name: Install the toolchain's system dependencies + # swiftly refuses to install a toolchain until Swift's documented Ubuntu packages are + # present; libcurl4-openssl-dev is the one ubuntu-latest lacks. + shell: bash + run: sudo apt-get update && sudo apt-get install -y libcurl4-openssl-dev + + - name: Install Swift via swiftly + # Unpacked outside the checkout so the tree the caller builds and tests in stays clean. + shell: bash + env: + SWIFT_VERSION: ${{ inputs.swift-version }} + run: | + cd "$RUNNER_TEMP" + curl -O "https://download.swift.org/swiftly/linux/swiftly-$(uname -m).tar.gz" + tar zxf "swiftly-$(uname -m).tar.gz" + ./swiftly init --quiet-shell-followup --assume-yes --skip-install + # env.sh lives under $XDG_DATA_HOME when that is set; fail loudly rather than silently + # continuing without swiftly on PATH if the layout ever moves. + ENV_SH="${XDG_DATA_HOME:-$HOME/.local/share}/swiftly/env.sh" + test -f "$ENV_SH" || { echo "::error::swiftly env.sh not found at $ENV_SH"; exit 1; } + . "$ENV_SH" + swiftly install --use "$SWIFT_VERSION" --assume-yes + echo "$SWIFTLY_BIN_DIR" >> "$GITHUB_PATH" diff --git a/.github/workflows/pr-checks.yml b/.github/workflows/pr-checks.yml index b12a8254..26882ec2 100644 --- a/.github/workflows/pr-checks.yml +++ b/.github/workflows/pr-checks.yml @@ -11,7 +11,7 @@ concurrency: cancel-in-progress: ${{ github.event_name == 'pull_request' }} env: - # One place for the Linux toolchain version; release.yml pins the same value. + # Keep in sync with release.yml, which pins the same toolchain for the release build. SWIFT_VERSION: '6.2.0' jobs: @@ -71,26 +71,9 @@ jobs: # Not a `container:` job: the official swift images run as root, and # UpdateCheckerTests.registryWriteFailureContract asserts that a write into a 0555 # directory fails, which root ignores. - - name: Install Swift toolchain build dependencies - # swiftly's installer needs libcurl's dev headers to build/verify the toolchain - # (confirmed by the exact error swiftly prints when it's missing); ubuntu-latest - # does not ship them. - run: sudo apt-get update && sudo apt-get install -y libcurl4-openssl-dev - - - name: Install Swift via swiftly - # Unpacked outside the checkout so the tree the next steps build and test in stays clean. - run: | - cd "$RUNNER_TEMP" - curl -O https://download.swift.org/swiftly/linux/swiftly-$(uname -m).tar.gz - tar zxf "swiftly-$(uname -m).tar.gz" - ./swiftly init --quiet-shell-followup --assume-yes --skip-install - # env.sh lives under $XDG_DATA_HOME when that is set; fail loudly rather than silently - # continuing without swiftly on PATH if the layout ever moves. - ENV_SH="${XDG_DATA_HOME:-$HOME/.local/share}/swiftly/env.sh" - test -f "$ENV_SH" || { echo "::error::swiftly env.sh not found at $ENV_SH"; exit 1; } - . "$ENV_SH" - swiftly install --use "$SWIFT_VERSION" --assume-yes - echo "$SWIFTLY_BIN_DIR" >> "$GITHUB_PATH" + - uses: ./.github/actions/setup-swift-linux + with: + swift-version: ${{ env.SWIFT_VERSION }} - uses: actions/cache@v6 with: diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 2e7b1ff4..da9fe168 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -12,7 +12,7 @@ concurrency: group: release env: - # Same toolchain the Linux PR job installs; kept in one place per workflow. + # Keep in sync with pr-checks.yml. SWIFT_VERSION: '6.2.0' jobs: @@ -121,26 +121,9 @@ jobs: steps: - uses: actions/checkout@v7 - - name: Install Swift toolchain build dependencies - # swiftly's installer needs libcurl's dev headers to build/verify the toolchain - # (confirmed by the exact error swiftly prints when it's missing); ubuntu-latest - # does not ship them. - run: sudo apt-get update && sudo apt-get install -y libcurl4-openssl-dev - - - name: Install Swift via swiftly - # Unpacked outside the checkout so the tree the next steps build and test in stays clean. - run: | - cd "$RUNNER_TEMP" - curl -O https://download.swift.org/swiftly/linux/swiftly-$(uname -m).tar.gz - tar zxf "swiftly-$(uname -m).tar.gz" - ./swiftly init --quiet-shell-followup --assume-yes --skip-install - # env.sh lives under $XDG_DATA_HOME when that is set; fail loudly rather than silently - # continuing without swiftly on PATH if the layout ever moves. - ENV_SH="${XDG_DATA_HOME:-$HOME/.local/share}/swiftly/env.sh" - test -f "$ENV_SH" || { echo "::error::swiftly env.sh not found at $ENV_SH"; exit 1; } - . "$ENV_SH" - swiftly install --use "$SWIFT_VERSION" --assume-yes - echo "$SWIFTLY_BIN_DIR" >> "$GITHUB_PATH" + - uses: ./.github/actions/setup-swift-linux + with: + swift-version: ${{ env.SWIFT_VERSION }} - uses: actions/cache@v6 with: @@ -164,26 +147,9 @@ jobs: steps: - uses: actions/checkout@v7 - - name: Install Swift toolchain build dependencies - # swiftly's installer needs libcurl's dev headers to build/verify the toolchain - # (confirmed by the exact error swiftly prints when it's missing); ubuntu-latest - # does not ship them. - run: sudo apt-get update && sudo apt-get install -y libcurl4-openssl-dev - - - name: Install Swift via swiftly - # Unpacked outside the checkout so the tree the next steps build and test in stays clean. - run: | - cd "$RUNNER_TEMP" - curl -O https://download.swift.org/swiftly/linux/swiftly-$(uname -m).tar.gz - tar zxf "swiftly-$(uname -m).tar.gz" - ./swiftly init --quiet-shell-followup --assume-yes --skip-install - # env.sh lives under $XDG_DATA_HOME when that is set; fail loudly rather than silently - # continuing without swiftly on PATH if the layout ever moves. - ENV_SH="${XDG_DATA_HOME:-$HOME/.local/share}/swiftly/env.sh" - test -f "$ENV_SH" || { echo "::error::swiftly env.sh not found at $ENV_SH"; exit 1; } - . "$ENV_SH" - swiftly install --use "$SWIFT_VERSION" --assume-yes - echo "$SWIFTLY_BIN_DIR" >> "$GITHUB_PATH" + - uses: ./.github/actions/setup-swift-linux + with: + swift-version: ${{ env.SWIFT_VERSION }} - uses: actions/cache@v6 with: