diff --git a/Package.resolved b/Package.resolved index 8dfb149..9a802ed 100644 --- a/Package.resolved +++ b/Package.resolved @@ -6,7 +6,7 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/lynnswap/CodexKit.git", "state" : { - "revision" : "23276c81cf160b773b3f9fb5ae710d257838cad0" + "revision" : "d2a694d7f633c1f01d4d260585863c0a3eae21af" } }, { diff --git a/Package.swift b/Package.swift index 350b2f5..39c8d66 100644 --- a/Package.swift +++ b/Package.swift @@ -33,9 +33,9 @@ let package = Package( .package(url: "https://github.com/modelcontextprotocol/swift-sdk.git", exact: "0.12.1"), .package(url: "https://github.com/apple/swift-nio.git", from: "2.97.1"), .package(url: "https://github.com/lynnswap/ObservationBridge.git", .upToNextMinor(from: "0.12.0")), - // CodexKit has no tagged releases yet; pin the final review response - // contract fix (lynnswap/CodexKit#17). - .package(url: "https://github.com/lynnswap/CodexKit.git", revision: "23276c81cf160b773b3f9fb5ae710d257838cad0"), + // CodexKit has no tagged releases yet; pin the reviewed native-login + // and final review response contract revision. + .package(url: "https://github.com/lynnswap/CodexKit.git", revision: "d2a694d7f633c1f01d4d260585863c0a3eae21af"), ], targets: [ .target( diff --git a/Sources/CodexReviewAppServer/AppServerCodexReviewBackend.swift b/Sources/CodexReviewAppServer/AppServerCodexReviewBackend.swift index ac70063..8f720dd 100644 --- a/Sources/CodexReviewAppServer/AppServerCodexReviewBackend.swift +++ b/Sources/CodexReviewAppServer/AppServerCodexReviewBackend.swift @@ -86,20 +86,31 @@ package actor AppServerCodexReviewBackend: CodexReviewBackend, CodexModelActor { try await appServer.rateLimits() } - package func startLogin(_: CodexReviewBackendModel.Login.Request) async throws + package func startLogin(_ request: CodexReviewBackendModel.Login.Request) async throws -> CodexReviewBackendModel.Login.Challenge { - // CodexAppServerKit's loginChatGPT cannot yet forward the native - // web-authentication callback scheme to account/login/start, so the - // server never shapes the auth URL for a native session; the host - // deliberately falls back to the external browser until that API - // exists. Do not mark the challenge native here without it. + if let callbackScheme = request.nativeWebAuthenticationCallbackScheme { + let login = try await appServer.loginChatGPT( + nativeWebAuthentication: .init(callbackURLScheme: callbackScheme) + ) + return login.backendChallenge() + } let handle = try await appServer.loginChatGPT() return try handle.backendChallenge( nativeWebAuthenticationCallbackScheme: nil ) } + package func completeLogin( + _ challenge: CodexReviewBackendModel.Login.Challenge, + callbackURL: URL + ) async throws { + try await appServer.completeLogin( + id: .init(rawValue: challenge.id), + callbackURL: callbackURL + ) + } + package func cancelLogin(_ challenge: CodexReviewBackendModel.Login.Challenge) async throws { try await appServer.cancelLogin(id: .init(rawValue: challenge.id)) } @@ -828,6 +839,16 @@ private extension CodexModel { } } +private extension CodexChatGPTLogin { + func backendChallenge() -> CodexReviewBackendModel.Login.Challenge { + .init( + id: id.rawValue, + verificationURL: authenticationURL, + nativeWebAuthenticationCallbackScheme: nativeWebAuthentication?.callbackURLScheme + ) + } +} + private extension CodexLoginHandle { func backendChallenge( nativeWebAuthenticationCallbackScheme: String? diff --git a/Sources/CodexReviewHost/LiveCodexReviewStoreBackend.swift b/Sources/CodexReviewHost/LiveCodexReviewStoreBackend.swift index 5b8142e..fedc387 100644 --- a/Sources/CodexReviewHost/LiveCodexReviewStoreBackend.swift +++ b/Sources/CodexReviewHost/LiveCodexReviewStoreBackend.swift @@ -865,7 +865,6 @@ private final class LiveCodexReviewStoreBackend: CodexReviewStoreBackend { logger.info("Received ChatGPT login challenge") let nativeCallbackScheme = challenge.nativeWebAuthenticationCallbackScheme let usesNativeAuthentication = nativeAuthenticationConfiguration != nil - && nativeCallbackScheme != nil && challenge.verificationURL != nil auth.updatePhase(.signingIn(.init( title: "Sign in to Codex", @@ -873,10 +872,7 @@ private final class LiveCodexReviewStoreBackend: CodexReviewStoreBackend { browserURL: challenge.verificationURL?.absoluteString, userCode: challenge.userCode ))) - guard usesNativeAuthentication, - let nativeAuthenticationConfiguration, - challenge.verificationURL != nil - else { + guard let nativeAuthenticationConfiguration, challenge.verificationURL != nil else { if let verificationURL = challenge.verificationURL { externalURLOpener(verificationURL) } @@ -912,6 +908,7 @@ private final class LiveCodexReviewStoreBackend: CodexReviewStoreBackend { await self.monitorAuthenticationSession( challenge: challenge, session: session, + completesLoginThroughCallback: nativeCallbackScheme != nil, auth: auth ) } @@ -946,10 +943,22 @@ private final class LiveCodexReviewStoreBackend: CodexReviewStoreBackend { private func monitorAuthenticationSession( challenge: CodexReviewBackendModel.Login.Challenge, session: any CodexReviewNativeAuthentication.WebSession, + completesLoginThroughCallback: Bool, auth: CodexReviewAuthModel ) async { do { - _ = try await session.waitForCallbackURL() + let callbackURL = try await session.waitForCallbackURL() + guard loginChallenge?.id == challenge.id else { + return + } + guard completesLoginThroughCallback else { + logger.info("Authentication session completed; waiting for app-server login completion notification") + return + } + guard let loginBackend else { + throw CodexReviewAPI.Error.io("Authentication runtime is not available.") + } + try await loginBackend.completeLogin(challenge, callbackURL: callbackURL) guard loginChallenge?.id == challenge.id else { return } diff --git a/Tests/CodexReviewHostTests/CodexReviewHostTests.swift b/Tests/CodexReviewHostTests/CodexReviewHostTests.swift index be7639c..4eaeafa 100644 --- a/Tests/CodexReviewHostTests/CodexReviewHostTests.swift +++ b/Tests/CodexReviewHostTests/CodexReviewHostTests.swift @@ -507,7 +507,7 @@ struct CodexReviewHostTests { ]) } - @Test func liveStoreCompletesBrowserLoginFromAccountNotifications() async throws { + @Test func liveStoreCompletesNativeLoginFromAuthenticationSessionAndAccountNotifications() async throws { let transport = FakeCodexAppServerTransport() try await transport.enqueue(AppServerAPI.Initialize.Response(), for: "initialize") try await transport.enqueue(AppServerAPI.Account.Read.Response(), for: "account/read") @@ -524,6 +524,10 @@ struct CodexReviewHostTests { ), for: "account/login/start" ) + try await transport.enqueue( + AppServerAPI.Account.Login.Complete.Response(), + for: "account/login/complete" + ) try await transport.enqueue( AppServerAPI.Account.Read.Response(account: .init(email: "new@example.com", planType: "plus")), for: "account/read" @@ -554,8 +558,28 @@ struct CodexReviewHostTests { await store.addAccount() await transport.waitForRequestCount(5) #expect(store.auth.isAuthenticating) - #expect(sessions.createdSessionCount == 0) - #expect(externalURLOpener.openedURLs == [URL(string: "https://example.com/auth")!]) + #expect(sessions.createdSessionCount == 1) + #expect(externalURLOpener.openedURLs == []) + let loginRequest = try #require(await transport.recordedRequests().first { + $0.method == "account/login/start" + }) + let loginParams = try JSONDecoder().decode(AppServerAPI.Account.Login.Params.self, from: loginRequest.params) + #expect(loginParams.nativeWebAuthentication == .init( + callbackURLScheme: "lynnpd.CodexReviewMonitor.auth" + )) + let session = await sessions.waitForSession() + await session.waitUntilWaitingForCallback() + session.complete(with: URL(string: "lynnpd.CodexReviewMonitor.auth://callback?code=abc")!) + await transport.waitForRequestCount(6) + let completeRequest = try #require(await transport.recordedRequests().first { + $0.method == "account/login/complete" + }) + let completeParams = try JSONDecoder().decode( + AppServerAPI.Account.Login.Complete.Params.self, + from: completeRequest.params + ) + #expect(completeParams.loginID == "login-1") + #expect(completeParams.callbackURL == "lynnpd.CodexReviewMonitor.auth://callback?code=abc") try await transport.emitServerNotification( method: "account/login/completed", params: TestLoginCompletedNotification(loginID: "login-1", success: true) @@ -567,12 +591,91 @@ struct CodexReviewHostTests { await waitUntil { store.auth.selectedAccount?.accountKey == "new@example.com" } // The post-login rate-limit refresh lands asynchronously after the // account update; wait for the full request sequence before asserting. - await transport.waitForRequestCount(7) + await transport.waitForRequestCount(8) + let methods = await transport.recordedRequests().map(\.method) + #expect(methods == [ + "initialize", + "account/read", + "config/read", + "model/list", + "account/login/start", + "account/login/complete", + "account/read", + "account/rateLimits/read", + ]) + await store.stop() + } + + @Test func liveStoreUsesNativeAuthenticationWhenServerDoesNotReturnNativeMetadata() async throws { + let transport = FakeCodexAppServerTransport() + try await transport.enqueue(AppServerAPI.Initialize.Response(), for: "initialize") + try await transport.enqueue(AppServerAPI.Account.Read.Response(), for: "account/read") + try await transport.enqueue( + AppServerAPI.Config.Read.Response(config: .init(model: "gpt-5")), + for: "config/read" + ) + try await transport.enqueue(AppServerAPI.Model.List.Response(data: []), for: "model/list") + try await transport.enqueue( + AppServerAPI.Account.Login.Response.chatgpt( + loginID: "login-1", + authURL: "https://example.com/auth", + nativeWebAuthentication: nil + ), + for: "account/login/start" + ) + try await transport.enqueue( + AppServerAPI.Account.Read.Response(account: .init(email: "new@example.com", planType: "plus")), + for: "account/read" + ) + try await transport.enqueue( + AppServerAPI.Account.RateLimits.Response(rateLimits: .init( + limitID: "codex", + primary: .init(usedPercent: 20, windowDurationMins: 300) + )), + for: "account/rateLimits/read" + ) + let sessions = FakeWebAuthenticationSessions() + let externalURLOpener = FakeExternalURLOpener() + let store = CodexReviewStore.makeLiveStoreForTesting( + environment: ["HOME": try temporaryHome().path], + nativeAuthenticationConfiguration: .init( + callbackScheme: "lynnpd.CodexReviewMonitor.auth", + browserSessionPolicy: .ephemeral, + presentationAnchorProvider: { NSWindow() } + ), + webAuthenticationSessionFactory: sessions.makeSession, + externalURLOpener: externalURLOpener.open, + transport: transport + ) + + await store.start(forceRestartIfNeeded: true) + await transport.waitForNotificationStreamCount(1) + await store.addAccount() + await transport.waitForRequestCount(5) + + #expect(store.auth.isAuthenticating) + #expect(sessions.createdSessionCount == 1) + #expect(externalURLOpener.openedURLs == []) let loginRequest = try #require(await transport.recordedRequests().first { $0.method == "account/login/start" }) let loginParams = try JSONDecoder().decode(AppServerAPI.Account.Login.Params.self, from: loginRequest.params) - #expect(loginParams.nativeWebAuthentication == nil) + #expect(loginParams.nativeWebAuthentication == .init( + callbackURLScheme: "lynnpd.CodexReviewMonitor.auth" + )) + let session = await sessions.waitForSession() + await session.waitUntilWaitingForCallback() + session.complete(with: URL(string: "lynnpd.CodexReviewMonitor.auth://callback?code=abc")!) + try await transport.emitServerNotification( + method: "account/login/completed", + params: TestLoginCompletedNotification(loginID: "login-1", success: true) + ) + try await transport.emitServerNotification( + method: "account/updated", + params: EmptyResponse() + ) + await waitUntil { store.auth.selectedAccount?.accountKey == "new@example.com" } + await transport.waitForRequestCount(7) let methods = await transport.recordedRequests().map(\.method) #expect(methods == [ "initialize", @@ -586,7 +689,7 @@ struct CodexReviewHostTests { await store.stop() } - @Test func liveStoreUsesExternalBrowserWhenNativeSessionFactoryIsConfigured() async throws { + @Test func liveStoreCancelsLoginWhenNativeSessionFactoryFails() async throws { let transport = FakeCodexAppServerTransport() try await transport.enqueue(AppServerAPI.Initialize.Response(), for: "initialize") try await transport.enqueue(AppServerAPI.Account.Read.Response(), for: "account/read") @@ -626,17 +729,18 @@ struct CodexReviewHostTests { await store.start(forceRestartIfNeeded: true) await store.addAccount() - await transport.waitForRequestCount(5) + await transport.waitForRequestCount(6) - #expect(didCreateNativeSession == false) - #expect(store.auth.isAuthenticating) - #expect(externalURLOpener.openedURLs == [URL(string: "https://example.com/auth")!]) - #expect(Array(await transport.recordedRequests().map(\.method).prefix(5)) == [ + #expect(didCreateNativeSession) + #expect(failedMessage(from: store.auth.phase) == "Authentication presentation failed.") + #expect(externalURLOpener.openedURLs == []) + #expect(await transport.recordedRequests().map(\.method) == [ "initialize", "account/read", "config/read", "model/list", "account/login/start", + "account/login/cancel", ]) await store.stop() } @@ -671,10 +775,14 @@ struct CodexReviewHostTests { AppServerAPI.Account.Login.Response.chatgpt( loginID: "login-2", authURL: "https://example.com/auth", - nativeWebAuthentication: nil + nativeWebAuthentication: .init(callbackURLScheme: "lynnpd.CodexReviewMonitor.auth") ), for: "account/login/start" ) + try await authTransport.enqueue( + AppServerAPI.Account.Login.Complete.Response(), + for: "account/login/complete" + ) try await authTransport.enqueue( AppServerAPI.Account.Read.Response( account: .init(email: "new@example.com", planType: "plus") @@ -742,13 +850,28 @@ struct CodexReviewHostTests { await store.addAccount() await authTransport.waitForNotificationStreamCount(1) await authTransport.waitForRequestCount(2) - #expect(sessions.createdSessionCount == 0) - #expect(externalURLOpener.openedURLs == [URL(string: "https://example.com/auth")!]) + #expect(sessions.createdSessionCount == 1) + #expect(externalURLOpener.openedURLs == []) let loginRequest = try #require(await authTransport.recordedRequests().first { $0.method == "account/login/start" }) let loginParams = try JSONDecoder().decode(AppServerAPI.Account.Login.Params.self, from: loginRequest.params) - #expect(loginParams.nativeWebAuthentication == nil) + #expect(loginParams.nativeWebAuthentication == .init( + callbackURLScheme: "lynnpd.CodexReviewMonitor.auth" + )) + let session = await sessions.waitForSession() + await session.waitUntilWaitingForCallback() + session.complete(with: URL(string: "lynnpd.CodexReviewMonitor.auth://callback?code=abc")!) + await authTransport.waitForRequestCount(3) + let completeRequest = try #require(await authTransport.recordedRequests().first { + $0.method == "account/login/complete" + }) + let completeParams = try JSONDecoder().decode( + AppServerAPI.Account.Login.Complete.Params.self, + from: completeRequest.params + ) + #expect(completeParams.loginID == "login-2") + #expect(completeParams.callbackURL == "lynnpd.CodexReviewMonitor.auth://callback?code=abc") try await authTransport.emitServerNotification( method: "account/login/completed", params: TestLoginCompletedNotification(loginID: "login-2", success: true) @@ -771,6 +894,7 @@ struct CodexReviewHostTests { #expect(await authTransport.recordedRequests().map(\.method) == [ "initialize", "account/login/start", + "account/login/complete", "account/read", "account/rateLimits/read", ]) @@ -887,6 +1011,10 @@ struct CodexReviewHostTests { ), for: "account/login/start" ) + try await transport.enqueue( + AppServerAPI.Account.Login.Complete.Response(), + for: "account/login/complete" + ) try await transport.enqueue( AppServerAPI.Account.Read.Response(account: .init(email: "new@example.com", planType: "plus")), for: "account/read" @@ -921,8 +1049,12 @@ struct CodexReviewHostTests { await store.addAccount() await transport.waitForRequestCount(5) - #expect(sessions.createdSessionCount == 0) - #expect(externalURLOpener.openedURLs == [URL(string: "https://example.com/auth")!]) + #expect(sessions.createdSessionCount == 1) + #expect(externalURLOpener.openedURLs == []) + let session = await sessions.waitForSession() + await session.waitUntilWaitingForCallback() + session.complete(with: URL(string: "lynnpd.CodexReviewMonitor.auth://callback?code=abc")!) + await transport.waitForRequestCount(6) try await transport.emitServerNotification( method: "account/login/completed", params: TestLoginCompletedNotification(loginID: "login-new", success: true) @@ -948,12 +1080,13 @@ struct CodexReviewHostTests { "config/read", "model/list", "account/login/start", + "account/login/complete", "account/read", "account/rateLimits/read", ]) } - @Test func liveStoreAddAccountUsesExternalBrowserWhenNativeSessionFactoryFails() async throws { + @Test func liveStoreAddAccountCancelsLoginWhenNativeSessionFactoryFails() async throws { let homeURL = try temporaryHome() let mainCodexHomeURL = homeURL.appendingPathComponent(".codex_review", isDirectory: true) try writeRegistry( @@ -1019,18 +1152,19 @@ struct CodexReviewHostTests { await store.start(forceRestartIfNeeded: true) let previousFailureCount = store.auth.authenticationFailureCount await store.addAccount() - await loginTransport.waitForRequestCount(2) + await loginTransport.waitForRequestCount(3) let resolvedIsolatedCodexHomeURL = try #require(isolatedCodexHomeURL) - #expect(store.auth.authenticationFailureCount == previousFailureCount) - #expect(store.auth.isAuthenticating) + #expect(store.auth.authenticationFailureCount == previousFailureCount + 1) + #expect(failedMessage(from: store.auth.phase) == "Authentication presentation failed.") #expect(store.auth.selectedAccount?.accountKey == "active@example.com") - #expect(externalURLOpener.openedURLs == [URL(string: "https://example.com/auth")!]) + #expect(externalURLOpener.openedURLs == []) await store.stop() #expect(FileManager.default.fileExists(atPath: resolvedIsolatedCodexHomeURL.path) == false) - #expect(Array(await loginTransport.recordedRequests().map(\.method).prefix(2)) == [ + #expect(await loginTransport.recordedRequests().map(\.method) == [ "initialize", "account/login/start", + "account/login/cancel", ]) } @@ -1514,11 +1648,12 @@ struct CodexReviewHostTests { AppServerAPI.Account.Login.Response.chatgpt( loginID: "login-1", authURL: "https://example.com/auth", - nativeWebAuthentication: .init(callbackURLScheme: "lynnpd.CodexReviewMonitor.auth") + nativeWebAuthentication: nil ), for: "account/login/start" ) let externalURLOpener = FakeExternalURLOpener() + let sessions = FakeWebAuthenticationSessions() var isolatedCodexHomeURL: URL? let store = CodexReviewStore.makeLiveStoreForTesting( environment: ["HOME": homeURL.path], @@ -1527,7 +1662,7 @@ struct CodexReviewHostTests { browserSessionPolicy: .ephemeral, presentationAnchorProvider: { NSWindow() } ), - webAuthenticationSessionFactory: FakeWebAuthenticationSessions().makeSession, + webAuthenticationSessionFactory: sessions.makeSession, externalURLOpener: externalURLOpener.open, transportFactory: { codexHomeURL in if codexHomeURL == mainCodexHomeURL { @@ -1542,9 +1677,11 @@ struct CodexReviewHostTests { await store.start(forceRestartIfNeeded: true) await mainTransport.waitForNotificationStreamCount(1) await store.addAccount() + let session = await sessions.waitForSession() + await session.waitUntilWaitingForCallback() let resolvedIsolatedCodexHomeURL = try #require(isolatedCodexHomeURL) #expect(FileManager.default.fileExists(atPath: resolvedIsolatedCodexHomeURL.path)) - #expect(externalURLOpener.openedURLs == [URL(string: "https://example.com/auth")!]) + #expect(externalURLOpener.openedURLs == []) await mainTransport.finishNotificationStreams(throwing: TestTransportClosedError()) await waitUntil { @@ -1728,11 +1865,12 @@ struct CodexReviewHostTests { AppServerAPI.Account.Login.Response.chatgpt( loginID: "login-2", authURL: "https://example.com/auth", - nativeWebAuthentication: .init(callbackURLScheme: "lynnpd.CodexReviewMonitor.auth") + nativeWebAuthentication: nil ), for: "account/login/start" ) let externalURLOpener = FakeExternalURLOpener() + let sessions = FakeWebAuthenticationSessions() var isolatedCodexHomeURL: URL? let store = CodexReviewStore.makeLiveStoreForTesting( environment: ["HOME": homeURL.path], @@ -1741,7 +1879,7 @@ struct CodexReviewHostTests { browserSessionPolicy: .ephemeral, presentationAnchorProvider: { NSWindow() } ), - webAuthenticationSessionFactory: FakeWebAuthenticationSessions().makeSession, + webAuthenticationSessionFactory: sessions.makeSession, externalURLOpener: externalURLOpener.open, transportFactory: { codexHomeURL in if codexHomeURL == mainCodexHomeURL { @@ -1756,7 +1894,9 @@ struct CodexReviewHostTests { await store.start(forceRestartIfNeeded: true) await store.addAccount() await loginTransport.waitForNotificationStreamCount(1) - #expect(externalURLOpener.openedURLs == [URL(string: "https://example.com/auth")!]) + let session = await sessions.waitForSession() + await session.waitUntilWaitingForCallback() + #expect(externalURLOpener.openedURLs == []) try await loginTransport.emitServerNotification( method: "account/login/completed", params: TestLoginCompletedNotification( diff --git a/Tools/ReviewMonitor/CodexReviewMonitor.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/Tools/ReviewMonitor/CodexReviewMonitor.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index d55cce3..a100014 100644 --- a/Tools/ReviewMonitor/CodexReviewMonitor.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/Tools/ReviewMonitor/CodexReviewMonitor.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -6,7 +6,7 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/lynnswap/CodexKit.git", "state" : { - "revision" : "23276c81cf160b773b3f9fb5ae710d257838cad0" + "revision" : "d2a694d7f633c1f01d4d260585863c0a3eae21af" } }, {