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
2 changes: 1 addition & 1 deletion Sources/CodexReview/ReviewRuntimeLifecycle.swift
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ package func applyRuntimeAuthenticationSnapshot(
@MainActor
package protocol RuntimeLifecycleHandle: AnyObject, Sendable {
func activate() async throws
func closeAdmission() async
func closeAdmission()
func close(purpose: ReviewRuntimeTransitionPurpose) async throws
func waitUntilClosed() async throws
}
Expand Down
16 changes: 13 additions & 3 deletions Sources/CodexReview/Store/CodexReviewStore.swift
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ public final class CodexReviewStore {
break
}

closePublishedRuntimeAdmission(in: previousState)
let generation = previousState.generation.successor()
switch previousState {
case .running(_, let runtime, let mcp):
Expand Down Expand Up @@ -253,6 +254,7 @@ public final class CodexReviewStore {
}

let previousState = runtimeState
closePublishedRuntimeAdmission(in: previousState)
let generation = previousState.generation.successor()
if case .failed(let message) = intent.finalState {
transitionToFailed(message)
Expand Down Expand Up @@ -303,7 +305,6 @@ public final class CodexReviewStore {
await stopMCPServer()

case .running(_, let runtime, _):
await runtime.handle.closeAdmission()
await stopPublishedRuntimeSemantics(intent: intent)
await stopMCPServer()
await closeRuntime(
Expand Down Expand Up @@ -358,6 +359,7 @@ public final class CodexReviewStore {

package func admitRuntimeRecycleAfterAccountChange() -> Task<Void, Never>? {
let previousState = runtimeState
closePublishedRuntimeAdmission(in: previousState)
let predecessor: Task<Void, Never>?
let context: RuntimeAcquisitionContext
switch previousState {
Expand Down Expand Up @@ -710,7 +712,6 @@ public final class CodexReviewStore {
private func closePublishedRuntimeForReplacement(
_ runtime: PreparedRuntime
) async {
await runtime.handle.closeAdmission()
await stopPublishedRuntimeSemantics(intent: .explicitStop)
await closeRuntime(
runtime,
Expand Down Expand Up @@ -746,7 +747,7 @@ public final class CodexReviewStore {
admissionAlreadyClosed: Bool = false
) async {
if admissionAlreadyClosed == false {
await runtime.handle.closeAdmission()
runtime.handle.closeAdmission()
}
do {
try await runtime.handle.close(purpose: purpose)
Expand All @@ -760,6 +761,15 @@ public final class CodexReviewStore {
}
}

private func closePublishedRuntimeAdmission(
in state: ReviewStoreRuntimeState
) {
guard case .running(_, let runtime, _) = state else {
return
}
runtime.handle.closeAdmission()
}

private func stopMCPServer() async {
do {
try await backend.mcpServerLifecycle.stop()
Expand Down
2 changes: 1 addition & 1 deletion Sources/CodexReviewHost/CodexReviewHost.swift
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ private final class DirectRuntimeLifecycleHandle: RuntimeLifecycleHandle {
onActivate()
}

func closeAdmission() async {
func closeAdmission() {
onCloseAdmission()
}

Expand Down
2 changes: 1 addition & 1 deletion Sources/CodexReviewHost/LiveCodexReviewStoreBackend.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2051,7 +2051,7 @@ private final class LiveRuntimeLifecycleHandle: RuntimeLifecycleHandle {
isActivated = true
}

func closeAdmission() async {
func closeAdmission() {
owner?.closeRuntimeAdmission(self)
}

Expand Down
2 changes: 1 addition & 1 deletion Sources/CodexReviewTesting/TestSupport.swift
Original file line number Diff line number Diff line change
Expand Up @@ -884,7 +884,7 @@ package final class TestingRuntimeLifecycleHandle: RuntimeLifecycleHandle {
onActivate()
}

package func closeAdmission() async {
package func closeAdmission() {
closeAdmissionCallCount += 1
}

Expand Down
4 changes: 4 additions & 0 deletions Tests/CodexReviewHostTests/CodexReviewHostTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1352,7 +1352,11 @@ struct CodexReviewHostTests {
)
await firstTransport.waitForRequestCount(requestCount + 1)

let generationBeforeRestart = store.runtimeLifecycleAdmissionGeneration
let restart = Task { @MainActor in await store.restart() }
try #require(await waitUntil(timeout: .seconds(2)) {
store.runtimeLifecycleAdmissionGeneration > generationBeforeRestart
})
await staleReadGate.open()
await restart.value

Expand Down
20 changes: 20 additions & 0 deletions Tests/CodexReviewTests/CodexReviewStoreLifecycleTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ struct CodexReviewStoreLifecycleTests {
let retiringHandle = try #require(backend.lastPreparedRuntimeHandle)

let recycle = try #require(store.admitRuntimeRecycleAfterAccountChange())
#expect(retiringHandle.closeAdmissionCallCount == 1)
store.requestRuntimeTeardown(intent: .explicitStop)
await store.stop()
await recycle.value
Expand All @@ -164,6 +165,7 @@ struct CodexReviewStoreLifecycleTests {
let retiringHandle = try #require(backend.lastPreparedRuntimeHandle)

let recycle = try #require(store.admitRuntimeRecycleAfterAccountChange())
#expect(retiringHandle.closeAdmissionCallCount == 1)
let successor = try #require(store.admitRuntimeRecycleAfterAccountChange())
await successor.value
await recycle.value
Expand All @@ -179,6 +181,22 @@ struct CodexReviewStoreLifecycleTests {
await store.stop()
}

@Test func explicitStopAdmissionClosesPublishedRuntimeBeforeTeardownTaskEntry() async throws {
let backend = TestingCodexReviewStoreBackend(
reviewBackend: FakeCodexReviewBackend()
)
let store = CodexReviewStore.makeTestingStore(backend: backend)
await store.start()
let handle = try #require(backend.lastPreparedRuntimeHandle)

store.requestRuntimeTeardown(intent: .explicitStop)

#expect(handle.closeAdmissionCallCount == 1)
await store.stop()
#expect(handle.closePurposes == [.stop])
#expect(handle.waitUntilClosedCallCount == 1)
}

@Test func sameAccountRestartRetainsMCPListenerAndURL() async throws {
let endpoint = try #require(URL(string: "http://127.0.0.1:19417/mcp"))
let mcpOwner = TestingMCPServerLifecycleOwner(serverURL: endpoint)
Expand Down Expand Up @@ -342,6 +360,7 @@ struct CodexReviewStoreLifecycleTests {

let restart = Task { @MainActor in await store.restart() }
try await waitForCutoverStatus(.draining, service: store.settingsService)
#expect(firstHandle.closeAdmissionCallCount == 1)
await store.updateSettingsModel("deferred-edit")
#expect(backend.lastPreparedRuntimeHandle === firstHandle)
#expect(await reviewBackend.recordedCommands().filter {
Expand Down Expand Up @@ -419,6 +438,7 @@ struct CodexReviewStoreLifecycleTests {
handle.holdClose(with: closeGate)

store.requestRuntimeFailure(handle: handle, cause: "Injected runtime failure.")
#expect(handle.closeAdmissionCallCount == 1)
await handle.waitForClose()
let expected = "Review runtime stopped unexpectedly: Injected runtime failure."
#expect(store.serverState == .failed(expected))
Expand Down