Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 32 additions & 15 deletions CLI/CMUXCLI+CodexWriterFinalGuard.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,10 @@ extension CMUXCLI {
///
/// The app admission RPC is unavailable when a newer CLI talks to an older
/// cmux. This final guard keeps that compatibility path from silently
/// entering Codex's read-only mode. It never waits, removes a lock, or
/// signals the process that owns it; Codex remains the atomic authority
/// after this advisory observation.
/// entering Codex's read-only mode. It waits only a bounded time while
/// Codex changes its lock namespace, and never removes a lock or signals
/// the process that owns it; Codex remains the atomic authority after
/// this advisory observation.
func requireCodexWriterAvailable(
invocation: AgentRestoreInvocation,
workingDirectory: String
Expand All @@ -22,26 +23,42 @@ extension CMUXCLI {
workingDirectory: workingDirectory,
fallbackHome: NSHomeDirectory()
)
var inspection = CodexWriterLockInspector().inspect(
sessionID: sessionID,
codexHome: home
)
// Another Codex starting during a relaunch can hold the namespace
// briefly; retry that alone so it is not mistaken for an owner.
let probe = { CodexWriterLockInspector().inspect(sessionID: sessionID, codexHome: home) }
var inspection = CodexWriterLockSettle().inspection(probe: probe)
var candidates: [CodexWriterProcessInspector.Candidate] = []
if inspection.state == .active {
let observed = inspection
candidates = CodexWriterProcessInspector().candidates(for: observed)
inspection = CodexWriterLockInspector().inspect(sessionID: sessionID, codexHome: home)
inspection = CodexWriterLockSettle().inspection(probe: probe)
if !inspection.deviceAndInodeMatch(observed) { candidates = [] }
}
guard inspection.state == .available else {
switch inspection.state {
case .available:
return
case .active:
throw loggedRestoreError(
stage: "session.writer-lock-held",
detail: "session=\(sessionID)",
message: CodexWriterRestoreNotice().message(candidates: candidates)
)
case .changing:
throw loggedRestoreError(
stage: "session.writer-lock-changing",
detail: "session=\(sessionID)",
message: String(
localized: "agentRestore.writerLock.changing",
defaultValue: "Codex was still updating its session locks, so cmux could not confirm this conversation is free. Retry 'cmux restore --surface'."
)
)
case .unavailable:
throw loggedRestoreError(
stage: inspection.state == .active
? "session.writer-lock-held"
: "session.writer-check-unavailable",
stage: "session.writer-check-unavailable",
detail: "session=\(sessionID)",
message: inspection.state == .active ? CodexWriterRestoreNotice().message(candidates: candidates) : String(
localized: "agentRestore.admission.unavailable",
defaultValue: "cmux could not verify whether this agent session is already running. Retry 'cmux restore --surface'."
message: String(
localized: "agentRestore.writerLock.unreadable",
defaultValue: "cmux could not read this conversation's Codex session lock, so it could not confirm the conversation is free. Retry 'cmux restore --surface'."
)
)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
import Darwin
import Foundation

/// Re-probes Codex's writer lock while Codex is changing its lock namespace.
///
/// `.changing` means another Codex held the shared coordination lock or
/// swapped a lock file during the probe. A relaunch resumes dozens of threads
/// in the same second, each holding coordination briefly, so a single probe
/// often lands in that window even though nobody owns this conversation.
/// Only `.changing` is retried; `.active` and `.unavailable` return at once.
public struct CodexWriterLockSettle: Sendable {
/// Creates a stateless settling policy.
public init() {}

/// About 3.5 seconds, front-loaded: one Codex holds coordination for
/// milliseconds, while a whole relaunch burst can take a few seconds.
public static let delays: [TimeInterval] = [0.02, 0.05, 0.1, 0.2, 0.4, 0.8, 1, 1]

/// Probes until the lock is not `.changing`, or the delay budget is spent.
///
/// - Parameters:
/// - delays: the longest wait before each re-probe; its count bounds the probes.
/// - awaitChange: waits at most the given interval for the namespace to change.
/// - probe: one nonblocking lock inspection.
/// - Returns: The first settled observation, or the last `.changing` one.
public func inspection(
delays: [TimeInterval] = Self.delays,
awaitChange: (CodexWriterLockInspection, TimeInterval) -> Void = Self.awaitNamespaceChange,
probe: () -> CodexWriterLockInspection
) -> CodexWriterLockInspection {
var inspection = probe()
for delay in delays where inspection.state == .changing {
awaitChange(inspection, delay)
inspection = probe()
}
return inspection
}

/// Returns when Codex creates, renames, or unlinks a thread lock, or after `timeout`.
///
/// Releasing a flock produces no file event, so the timeout is the
/// re-probe bound rather than a guess at when the writer finishes.
public static func awaitNamespaceChange(_ inspection: CodexWriterLockInspection, timeout: TimeInterval) {
let directory = URL(fileURLWithPath: inspection.lockPath).deletingLastPathComponent().path
let descriptor = open(directory, O_EVTONLY | O_CLOEXEC)
// `.changing` implies the directory exists; without an event source
// the bound alone paces the next probe.
guard descriptor >= 0 else {
_ = DispatchSemaphore(value: 0).wait(timeout: .now() + timeout)
return
}
let changed = DispatchSemaphore(value: 0)
let source = DispatchSource.makeFileSystemObjectSource(
fileDescriptor: descriptor, eventMask: [.write, .delete, .rename], queue: .global(qos: .utility)
)
source.setEventHandler { changed.signal() }
source.setCancelHandler { close(descriptor) }
source.resume()
defer { source.cancel() }
_ = changed.wait(timeout: .now() + timeout)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,70 @@ import Testing
#expect(flock(coordination, LOCK_EX | LOCK_NB) == 0)
}

@Test func settleRetriesOnlyNamespaceChangesUntilTheyEnd() throws {
let fixture = try Fixture()
defer { fixture.remove() }
let coordination = try fixture.hold(fixture.locks.appendingPathComponent(".coordination.lock").path)
defer { close(coordination) }
var waits: [TimeInterval] = []
// Another Codex finishes its own lock update while this restore waits.
let settled = CodexWriterLockSettle().inspection(
awaitChange: { observed, delay in
#expect(observed.state == .changing)
waits.append(delay)
if waits.count == 3 { #expect(flock(coordination, LOCK_UN) == 0) }
},
probe: fixture.inspect
)
#expect(settled.state == .available)
#expect(waits == Array(CodexWriterLockSettle.delays.prefix(3)))
}

@Test func settleReportsPersistentChangeAfterItsBudget() throws {
let fixture = try Fixture()
defer { fixture.remove() }
let coordination = try fixture.hold(fixture.locks.appendingPathComponent(".coordination.lock").path)
defer { close(coordination) }
var waits = 0
let settled = CodexWriterLockSettle().inspection(awaitChange: { _, _ in waits += 1 }, probe: fixture.inspect)
#expect(settled.state == .changing)
#expect(waits == CodexWriterLockSettle.delays.count)
}

@Test func settleNeverRetriesAHeldOrUnreadableLock() throws {
let fixture = try Fixture()
defer { fixture.remove() }
let writer = try fixture.hold()
defer { close(writer) }
let held = CodexWriterLockSettle().inspection(
awaitChange: { _, _ in Issue.record("A held writer is an owner, not a transient change") },
probe: fixture.inspect
)
#expect(held.state == .active)
let unreadable = CodexWriterLockSettle().inspection(
awaitChange: { _, _ in Issue.record("Unreadable state must fail closed at once") },
probe: { CodexWriterLockInspector().inspect(sessionID: "../escape", codexHome: fixture.home.path) }
)
#expect(unreadable.state == .unavailable)
}

@Test func namespaceWaitReturnsOnALockFileChange() throws {
let fixture = try Fixture()
defer { fixture.remove() }
let coordination = try fixture.hold(fixture.locks.appendingPathComponent(".coordination.lock").path)
defer { close(coordination) }
let observed = fixture.inspect()
try #require(observed.state == .changing)
let path = fixture.path
DispatchQueue.global().asyncAfter(deadline: .now() + 0.2) {
FileManager.default.createFile(atPath: path, contents: nil)
}
let start = ContinuousClock.now
// The created lock file, not the generous bound, ends this wait.
CodexWriterLockSettle.awaitNamespaceChange(observed, timeout: 30)
#expect(start.duration(to: .now) < .seconds(10))
}

@Test func malformedAndNonregularPathsFailClosed() throws {
let fixture = try Fixture()
defer { fixture.remove() }
Expand Down
Loading
Loading