Skip to content
Closed
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
202 changes: 202 additions & 0 deletions Sources/CodexReview/ReviewRuntimeLifecycle.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,202 @@
import Foundation

package struct ReviewRuntimeGeneration: Hashable, Sendable {
package let rawValue: UInt64

package init(rawValue: UInt64) {
self.rawValue = rawValue
}

package func successor() -> Self {
.init(rawValue: rawValue &+ 1)
}
}
package enum ReviewRuntimeTransitionPurpose: Equatable, Sendable {
case start
case restartSameAccount
case stop
case runtimeFailure
}
package struct RuntimePublicationSnapshot: Sendable {
package let authentication: CodexReviewBackendModel.Auth.Snapshot
package let settings: CodexReviewSettings.Snapshot

package init(
authentication: CodexReviewBackendModel.Auth.Snapshot,
settings: CodexReviewSettings.Snapshot
) {
self.authentication = authentication
self.settings = settings
}
}
@MainActor
package func applyRuntimeAuthenticationSnapshot(
_ snapshot: CodexReviewBackendModel.Auth.Snapshot,
to auth: CodexReviewAuthModel
) {
let observedAccounts = snapshot.accounts.compactMap { account -> CodexAccount? in
let label = account.label.trimmingCharacters(in: .whitespacesAndNewlines)
let accountKey = CodexAccount.normalizedEmail(account.id.rawValue)
guard label.isEmpty == false, accountKey.isEmpty == false else {
return nil
}
return CodexAccount(
accountKey: accountKey,
email: label,
planType: account.planType,
kind: account.kind,
capabilities: account.capabilities
)
}
let activeAccountKey = snapshot.activeAccountID.map {
CodexAccount.normalizedEmail($0.rawValue)
}
var accounts = auth.persistedAccounts
for observedAccount in observedAccounts {
if let index = accounts.firstIndex(where: {
$0.accountKey == observedAccount.accountKey
}) {
accounts[index].updateEmail(observedAccount.email)
accounts[index].updateKind(
observedAccount.kind,
capabilities: observedAccount.capabilities
)
accounts[index].updatePlanType(observedAccount.planType)
} else {
accounts.insert(observedAccount, at: 0)
}
}
auth.applyPersistedAccountStates(
accounts.map(savedAccountPayload(from:)),
activeAccountKey: activeAccountKey
)
auth.selectPersistedAccount(activeAccountKey)
auth.updatePhase(.signedOut)
}
@MainActor
package protocol RuntimeLifecycleHandle: AnyObject, Sendable {
func activate() async throws
func closeAdmission() async
func close(purpose: ReviewRuntimeTransitionPurpose) async throws
func waitUntilClosed() async throws
}
package struct PreparedRuntime: Sendable {
package let snapshot: RuntimePublicationSnapshot
package let handle: any RuntimeLifecycleHandle

package init(
snapshot: RuntimePublicationSnapshot,
handle: any RuntimeLifecycleHandle
) {
self.snapshot = snapshot
self.handle = handle
}
}
package struct MCPServerGeneration: Hashable, Sendable {
package let rawValue: UInt64

package init(rawValue: UInt64) {
self.rawValue = rawValue
}
}
package struct PreparedMCPServer: Sendable {
package let generation: MCPServerGeneration

package init(generation: MCPServerGeneration) {
self.generation = generation
}
}
package struct MCPServerPublicationSnapshot: Sendable {
package let serverURL: URL?

package init(serverURL: URL?) {
self.serverURL = serverURL
}
}
package enum ReviewStoreRuntimeState {
case stopped(ReviewRuntimeGeneration)
case acquiring(
generation: ReviewRuntimeGeneration,
task: Task<Void, Never>
)
case running(
generation: ReviewRuntimeGeneration,
runtime: PreparedRuntime,
mcpGeneration: MCPServerGeneration
)
case transitioning(
generation: ReviewRuntimeGeneration,
purpose: ReviewRuntimeTransitionPurpose,
task: Task<Void, Never>
)
case failed(
generation: ReviewRuntimeGeneration,
retainedMCPGeneration: MCPServerGeneration,
retainedMCPServerURL: URL?
)

package var generation: ReviewRuntimeGeneration {
switch self {
case .stopped(let generation),
.acquiring(let generation, _),
.running(let generation, _, _),
.transitioning(let generation, _, _),
.failed(let generation, _, _):
generation
}
}
}

@MainActor
package protocol MCPServerLifecycleOwner: Sendable {
func prepare() async throws -> PreparedMCPServer
func activate(
_ generation: MCPServerGeneration
) async throws -> MCPServerPublicationSnapshot
func stop() async throws
}

@MainActor
package final class NoMCPServerLifecycleOwner: MCPServerLifecycleOwner {
private enum State {
case stopped
case prepared(MCPServerGeneration)
case running(MCPServerGeneration)
}

private var serverURL: URL?
private var nextGeneration: UInt64 = 0
private var state: State = .stopped

package init(serverURL: URL? = nil) {
self.serverURL = serverURL
}

package func updateServerURL(_ serverURL: URL?) {
self.serverURL = serverURL
}

package func prepare() async throws -> PreparedMCPServer {
guard case .stopped = state else {
throw CancellationError()
}
nextGeneration &+= 1
let generation = MCPServerGeneration(rawValue: nextGeneration)
state = .prepared(generation)
return .init(generation: generation)
}

package func activate(
_ generation: MCPServerGeneration
) async throws -> MCPServerPublicationSnapshot {
guard case .prepared(generation) = state else {
throw CancellationError()
}
state = .running(generation)
return .init(serverURL: serverURL)
}

package func stop() async throws {
state = .stopped
}
}
46 changes: 35 additions & 11 deletions Sources/CodexReview/Settings/CodexReviewSettingsService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ package final class CodexReviewSettingsService {
private var pendingRefresh = false
private var pendingSelection: SettingsStore.Selection?
private var lastPersistedSelection: SettingsStore.Selection
private var mutationRevision: UInt64 = 0

package init(
initialSnapshot: CodexReviewSettings.Snapshot,
Expand All @@ -51,6 +52,17 @@ package final class CodexReviewSettingsService {
lastPersistedSelection = settings.currentSelection()
}

package func applyRuntimeSnapshot(_ snapshot: CodexReviewSettings.Snapshot) {
guard let settingsStore else {
return
}
mutationRevision &+= 1
pendingRefresh = false
pendingSelection = nil
settingsStore.apply(snapshot: snapshot)
lastPersistedSelection = settingsStore.currentSelection()
Comment thread
lynnswap marked this conversation as resolved.
}

package func refreshIfRunning(serverState: CodexReviewServerState) async {
guard case .running = serverState else {
return
Expand All @@ -67,14 +79,19 @@ package final class CodexReviewSettingsService {
return
}

let revision = mutationRevision
settingsStore.beginLoading()
do {
let snapshot = try await backend.refreshSettings()
settingsStore.apply(snapshot: snapshot)
lastPersistedSelection = settingsStore.currentSelection()
if mutationRevision == revision {
settingsStore.apply(snapshot: snapshot)
lastPersistedSelection = settingsStore.currentSelection()
}
settingsStore.finishLoading(errorMessage: nil)
} catch {
settingsStore.finishLoading(errorMessage: error.localizedDescription)
settingsStore.finishLoading(
errorMessage: mutationRevision == revision ? error.localizedDescription : nil
)
}
await drainPendingWorkIfNeeded()
}
Expand Down Expand Up @@ -164,23 +181,30 @@ package final class CodexReviewSettingsService {
return
}

let revision = mutationRevision
settingsStore.beginLoading()
do {
try await persistSelection(
trigger: trigger,
previous: previous,
candidate: candidate
)
lastPersistedSelection = settingsStore.selectionAfterPersisting(
trigger: trigger,
previous: previous,
candidate: candidate
)
if mutationRevision == revision {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Reconcile writes that finish after runtime publication

When a runtime snapshot advances mutationRevision while persistSelection is already in flight, this guard suppresses only the local baseline update; it cannot prevent the backend request from committing after the new runtime has read its snapshot. Fresh evidence beyond the resolved thread is that FakeCodexReviewBackend.applySettings applies the stale change after its gate opens (Sources/CodexReviewTesting/TestSupport.swift lines 468-475), and the new regression test manually restores freshSnapshot at line 285 before continuing, masking the resulting backend/UI divergence. In this race the UI shows fresh-runtime-model while persisted configuration contains stale-model, so a subsequent review or restart can use the wrong settings; runtime publication must wait for the write or reconcile the current backend after a stale success.

Useful? React with 👍 / 👎.

lastPersistedSelection = settingsStore.selectionAfterPersisting(
trigger: trigger,
previous: previous,
candidate: candidate
)
}
settingsStore.finishLoading(errorMessage: nil)
} catch {
settingsStore.apply(snapshot: settingsStore.snapshot(selection: previous))
lastPersistedSelection = previous
settingsStore.finishLoading(errorMessage: error.localizedDescription)
if mutationRevision == revision {
settingsStore.apply(snapshot: settingsStore.snapshot(selection: previous))
lastPersistedSelection = previous
}
settingsStore.finishLoading(
errorMessage: mutationRevision == revision ? error.localizedDescription : nil
)
}
await drainPendingWorkIfNeeded()
}
Expand Down
Loading