fix(macapp): make the launch container sweep safe and complete (#284)

docs/designs/remote-workrooms.md

@@ -1540,6 +1540,26 @@ these are the subsystems that actually gate "a remote workroom is a real workroo
     than the grace (20 minutes by default, longer than a commit's 15-minute bound) is left, since
     it may be a create or derive still running, in this app or another one on the same daemon.
     Images are removed without `--force`, so one a container still uses stays.
+  - **Changed by #284 (2026-10-07):** a container goes only once two sweeps in a row (two
+    launches) found it unknown, and at most 3 go per sweep; the rest go on later launches. Each
+    sweep writes what it found unknown to `hosts/unknown-containers-<runtime>[@<context>].json`.
+    A lost, restored or half-written config so costs nothing on the launch that reads it.
+  - **Changed by #284 (2026-10-08):** a sweep also runs when config records nothing, but then
+    takes only containers a create left pending. Before running a container, a driver writes its
+    name to `hosts/pending-containers-<runtime>[@<context>].json`. The name comes off once config
+    records the host (`RemoteHosts.adopt`), or once a rollback, a delete or a sweep removes it. A
+    launch sweeps every runtime and context with a name pending. With nothing recorded it removes
+    only pending names, still under the two-launch rule and the cap, and no images. So a crash
+    before a project's first base was recorded no longer leaves its container for good. A lost
+    config, or a Dev launch with a throwaway `HOME` (the CLI's config follows `HOME`, the app's
+    `hosts/` does not), cannot make a live workroom's container look disposable. A Mac that never
+    made a container workroom has nothing pending and is never asked. A name is not dropped for
+    being missing from a listing, since a key with no context lists whichever daemon is current.
+  - **Changed by #284 (2026-10-08):** the once-per-launch sweep is tracked per runtime and
+    context, and one counts as done only once it listed its containers. A runtime that is down
+    at launch is swept on a later reload instead of being skipped for the whole launch. A
+    runtime and context whose sweep ran, or is still running, is never swept again in that
+    launch, since the two-launch rule would count a second sweep as the next launch.
 - **As built (#253, part 2: creating one).** Remote workrooms are on only in a Nightly or Dev
   build with the hidden `remoteWorkroomsPreview` setting (`RemoteWorkrooms.isEnabled`); a stable
   build shows no remote UI, including the Settings section, whatever the setting says. The
@@ -1923,7 +1943,7 @@ these are the subsystems that actually gate "a remote workroom is a real workroo
     badge warns when a box's boxd idle timer is under 90 s, host prompt cards scroll past two, and a
     stalled IDLE reading on a sleeping host shows "status unknown".
   - **Not swept.** Unrecorded boxd machines are left alone: names carry no build, so one build's
-    sweep would take another's live workrooms (#284 has the boxd case).
+    sweep would take another's live workrooms (#373 has the boxd case).
   - **Letting go of an idle box.** boxd's idle meter counts inbound traffic, and the first live run
     (2026-10-06) found an attached box never slept: the service connection's ssh keepalives (15 s) and
     the badge's polls (10 s) held it, idle or past an unanswered prompt. So a boxd host whose agent

macapp/WorkroomApp/Core/ContainerHostDriver.swift

@@ -391,6 +391,7 @@ final class ContainerHostDriver: HostTerminalDriver, @unchecked Sendable {
       provisioned[id] = nil
       destroyed.insert(id)
     }
+    forgetPending([made.container])
     try? FileManager.default.removeItem(at: directory.appendingPathComponent(id.uuidString))
   }
 
@@ -466,12 +467,25 @@ final class ContainerHostDriver: HostTerminalDriver, @unchecked Sendable {
   /// ponytail: a resource made before `createdLabel` existed has no age and counts as old.
   /// `images` are images other drivers' hosts were run from, kept too: a driver for another Docker
   /// context can reach the same daemon (#309).
+  ///
+  /// A container is a live workroom's until proven otherwise, and removing one loses its unpushed
+  /// work (#284), so it goes only once two sweeps in a row (two launches: the app sweeps once per
+  /// launch) found it unknown, and at most `cap` go per sweep (`confirmedUnknown`). A config that
+  /// was lost, restored or half-written then costs nothing on the launch that reads it. Images are
+  /// not held back: one a container still runs from is never removed.
+  ///
+  /// `onlyPending` sweeps only containers a create began and no record has named since
+  /// (`pendingContainers`), and no images: for when config records nothing, which is also what a
+  /// lost config or a launch with a throwaway `HOME` reads, where every live container would
+  /// otherwise look unknown.
   func sweep(
-    keeping known: Set<UUID>, images: Set<String> = [], grace: TimeInterval = 20 * 60
+    keeping known: Set<UUID>, images: Set<String> = [], grace: TimeInterval = 20 * 60,
+    cap: Int = 3, onlyPending: Bool = false
   ) async -> [String] {
     guard let provisioning, !provisioning.labels.isEmpty else { return [] }
     if provisioning.dialect == .apple {
-      return await appleSweep(keeping: known, images: images, grace: grace)
+      return await appleSweep(
+        keeping: known, images: images, grace: grace, cap: cap, onlyPending: onlyPending)
     }
     let filters = provisioning.labels.flatMap { ["--filter", "label=\($0)"] }
     let cutoff = Date().timeIntervalSince1970 - grace
@@ -482,21 +496,33 @@ final class ContainerHostDriver: HostTerminalDriver, @unchecked Sendable {
     let keptImages = Set(
       (lock.withLock { known.compactMap { provisioned[$0]?.image } } + images).map(
         Self.shortImageID))
+    let pending = onlyPending ? pendingContainers : []
     do {
+      var unknown: [String] = []
       for line in try await runtime(
         ["ps", "-a"] + filters + ["--format", "{{.Names}}\t{{.Label \"\(Self.createdLabel)\"}}"],
         allLines: true
       ).split(separator: "\n") {
         let fields = line.split(separator: "\t", omittingEmptySubsequences: false)
         let name = String(fields[0])
-        guard !keptContainers.contains(name), old(fields.count > 1 ? fields[1] : "") else {
-          continue
+        if !keptContainers.contains(name), old(fields.count > 1 ? fields[1] : ""),
+          !onlyPending || pending.contains(name)
+        {
+          unknown.append(name)
         }
-        do { _ = try await runtime(["rm", "--force", "--volumes", name]) } catch {
+      }
+      var removed: [String] = []
+      for name in confirmedUnknown(unknown, cap: cap, failed: &failed) {
+        do {
+          _ = try await runtime(["rm", "--force", "--volumes", name])
+          removed.append(name)
+        } catch {
           failed.append("container \(name): \(error.localizedDescription)")
         }
       }
-    } catch { failed.append("listing containers: \(error.localizedDescription)") }
+      forgetPending(removed)
+    } catch { failed.append(Self.listingFailed + error.localizedDescription) }
+    guard !onlyPending else { return failed }
 
     do {
       let images = try await runtime(["images", "-aq"] + filters, allLines: true)
@@ -524,9 +550,10 @@ final class ContainerHostDriver: HostTerminalDriver, @unchecked Sendable {
   /// Apple's sweep (#309): the same rules as Docker's, read from JSON, since its CLI has no
   /// `--filter` and no templates. An image is kept while any container at all was run from it:
   /// Apple removes an image a container still uses, where Docker refuses.
-  private func appleSweep(keeping known: Set<UUID>, images kept: Set<String>, grace: TimeInterval)
-    async -> [String]
-  {
+  private func appleSweep(
+    keeping known: Set<UUID>, images kept: Set<String>, grace: TimeInterval, cap: Int,
+    onlyPending: Bool
+  ) async -> [String] {
     guard let provisioning else { return [] }
     let cutoff = Date().timeIntervalSince1970 - grace
     func ours(_ labels: [String: String]) -> Bool {
@@ -557,7 +584,9 @@ final class ContainerHostDriver: HostTerminalDriver, @unchecked Sendable {
     }
     let keptContainers = Set(known.map(Self.containerName))
     var inUse = kept
+    let pending = onlyPending ? pendingContainers : []
     do {
+      var unknown: [String] = []
       for container in try AppleContainerCLI.objects(
         try await runtime(["list", "--all", "--format", "json"], allLines: true))
       {
@@ -569,17 +598,27 @@ final class ContainerHostDriver: HostTerminalDriver, @unchecked Sendable {
           inUse.insert(image)
         }
         let labels = AppleContainerCLI.labels(of: container)
-        guard let id = AppleContainerCLI.id(of: container), ours(labels), old(labels),
-          !keptContainers.contains(id)
-        else { continue }
-        do { _ = try await runtime(["delete", "--force", id]) } catch {
+        if let id = AppleContainerCLI.id(of: container), ours(labels), old(labels),
+          !keptContainers.contains(id), !onlyPending || pending.contains(id)
+        {
+          unknown.append(id)
+        }
+      }
+      var removed: [String] = []
+      for id in confirmedUnknown(unknown, cap: cap, failed: &failed) {
+        do {
+          _ = try await runtime(["delete", "--force", id])
+          removed.append(id)
+        } catch {
           failed.append("container \(id): \(error.localizedDescription)")
         }
       }
+      forgetPending(removed)
     } catch {
       // Without the list, no image is known to be unused.
-      return ["listing containers: \(error.localizedDescription)"]
+      return [Self.listingFailed + error.localizedDescription]
     }
+    guard !onlyPending else { return failed }
     do {
       for image in try AppleContainerCLI.objects(
         try await runtime(["image", "list", "--format", "json"], allLines: true))
@@ -596,6 +635,116 @@ final class ContainerHostDriver: HostTerminalDriver, @unchecked Sendable {
     return failed
   }
 
+  /// Which of `unknown`, this sweep's unknown containers, it removes (#284): those the previous
+  /// sweep found unknown too, and of them at most `cap`, in listing order. Writes `unknown` down for
+  /// the next sweep, so the ones past the cap go on later launches, `cap` at a time; one this sweep
+  /// removes is gone from the next one's listing. Only a sweep whose listing was read writes it,
+  /// so a failed listing keeps the last one.
+  private func confirmedUnknown(_ unknown: [String], cap: Int, failed: inout [String]) -> [String] {
+    let file = unknownContainersFile
+    let before = Set(
+      (try? JSONDecoder().decode([String].self, from: Data(contentsOf: file))) ?? [])
+    do {
+      try FileManager.default.createDirectory(
+        at: file.deletingLastPathComponent(), withIntermediateDirectories: true)
+      try JSONEncoder().encode(unknown).write(to: file, options: .atomic)
+    } catch {
+      // The list left behind is an older sweep's, not this one's predecessor: confirming against
+      // it, now or next time, would take a container that was known in between. So it goes, and
+      // this sweep removes nothing.
+      try? FileManager.default.removeItem(at: file)
+      failed.append("recording unknown containers: \(error.localizedDescription)")
+      return []
+    }
+    let confirmed = unknown.filter(before.contains)
+    if confirmed.count > cap {
+      failed.append(
+        "\(confirmed.count - cap) unknown container(s) left for a later launch: at most \(cap) go "
+          + "per sweep")
+    }
+    return Array(confirmed.prefix(cap))
+  }
+
+  /// Where a sweep writes down the containers it found unknown (#284): beside this driver's hosts,
+  /// one file per runtime and Docker context, since each context is swept by its own driver.
+  private var unknownContainersFile: URL {
+    directory.appendingPathComponent(
+      "unknown-containers-\(Self.suffix(provisioning?.dialect ?? .docker, provisioning?.context)).json"
+    )
+  }
+
+  /// The containers this driver began running that no config record names yet (#284), one file per
+  /// runtime and Docker context. A name goes in before its `run`, and out once config records its
+  /// host (`RemoteHosts.adopt`) or it is removed. So a crash before a project's first base was
+  /// recorded still leaves its container named here, and a launch whose config records nothing
+  /// sweeps only these (`sweep(onlyPending:)`), never a live workroom that config fails to show.
+  /// ponytail: a container removed by hand stays listed, costing its runtime one listing a launch;
+  /// a name is not dropped for being missing, since a context with none named lists whichever
+  /// daemon is current, and the leftover may be on another.
+  var pendingContainers: Set<String> {
+    pendingLock.withLock { Set(readPending()) }
+  }
+  private var pendingFile: URL {
+    directory.appendingPathComponent(
+      Self.pendingPrefix + Self.suffix(provisioning?.dialect ?? .docker, provisioning?.context)
+        + ".json")
+  }
+  private static let pendingPrefix = "pending-containers-"
+  private let pendingLock = NSLock()
+
+  private func readPending() -> [String] {
+    (try? JSONDecoder().decode([String].self, from: Data(contentsOf: pendingFile))) ?? []
+  }
+
+  /// Rewrites the pending list as `change` leaves it, removing the file once it is empty.
+  private func updatePending(_ change: (inout [String]) -> Void) throws {
+    try pendingLock.withLock {
+      var names = readPending()
+      let before = names
+      change(&names)
+      guard names != before else { return }
+      guard !names.isEmpty else {
+        try FileManager.default.removeItem(at: pendingFile)
+        return
+      }
+      try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true)
+      try JSONEncoder().encode(names).write(to: pendingFile, options: .atomic)
+    }
+  }
+
+  /// Takes `names` off the pending list: config records their hosts now, or they are gone.
+  func forgetPending(_ names: [String]) {
+    guard !names.isEmpty else { return }
+    let drop = Set(names)
+    try? updatePending { $0.removeAll(where: drop.contains) }
+  }
+
+  /// A file name's runtime and Docker context. `@` is in no context's name, so no context (the
+  /// current one) is told apart from one named `default`.
+  private static func suffix(_ dialect: Dialect, _ context: String?) -> String {
+    (dialect == .apple ? "apple" : "docker") + (context.map { "@\($0)" } ?? "")
+  }
+
+  /// The runtimes and Docker contexts whose drivers, with hosts in `directory`, have containers
+  /// pending (`pendingContainers`).
+  static func pending(in directory: URL) -> [(dialect: Dialect, context: String?)] {
+    ((try? FileManager.default.contentsOfDirectory(atPath: directory.path)) ?? []).compactMap {
+      guard $0.hasPrefix(pendingPrefix), $0.hasSuffix(".json") else { return nil }
+      let parts = $0.dropFirst(pendingPrefix.count).dropLast(".json".count)
+        .split(separator: "@", maxSplits: 1)
+      let context = parts.count > 1 ? String(parts[1]) : nil
+      switch parts.first {
+      case "docker": return (.docker, context)
+      case "apple": return (.apple, nil)
+      default: return nil
+      }
+    }
+  }
+
+  /// How a sweep's failure starts when it could not list the containers, so swept nothing: the
+  /// runtime was down, say. `RemoteHosts.adopt` sweeps that runtime again on a later reload.
+  static let listingFailed = "listing containers: "
+
   static func containerName(_ id: UUID) -> String { "workroom-\(id.uuidString.lowercased())" }
 
   private static func created() -> String {
@@ -631,6 +780,8 @@ final class ContainerHostDriver: HostTerminalDriver, @unchecked Sendable {
     // Named here, not by the runtime's own ID, which is known only from its output: a run that
     // outlives its CLI still leaves a container this name removes.
     let container = Self.containerName(id)
+    // Before the container exists: a crash from here on leaves its name pending (#284).
+    try updatePending { $0.append(container) }
     do {
       // Docker's `unless-stopped`: a Docker or Mac restart brings it back, on the port its record
       // names. Apple's runtime has no restart policy; opening the workroom starts it
@@ -670,6 +821,7 @@ final class ContainerHostDriver: HostTerminalDriver, @unchecked Sendable {
           return nil
         } catch { return "container \(container): \(error.localizedDescription)" }
       }.value
+      if removal == nil { forgetPending([container]) }
       // The login wait wrote the host's ssh_config and pinned key here.
       try? FileManager.default.removeItem(at: directory.appendingPathComponent(id.uuidString))
       guard let removal else { throw error }