From d9b77f363f255b694d3123a8583d5e08ba0f0c3b Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 12 Jul 2026 17:52:16 +0000 Subject: [PATCH] Online: per-color abandonment forfeit timers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A LiveGame held a single shared abandonTask, so when both players disconnected the second disconnect overwrote the first's timer reference (the orphaned task kept running but became uncancellable), and a later reconnect cancelled whichever forfeit the shared field then pointed at — the still-absent opponent's. The abandoner escaped the 60s forfeit and the game hung until a clock happened to expire. Track forfeit timers per color so a reconnecting player cancels only their own; the opponent's still stands. The grace period is now injectable so the regression test resolves in ~1.5s instead of 60. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01E9dkcpbwWWHrdGzpvWZd1T --- .../Sources/App/Online/GameCoordinator.swift | 47 ++++++++++++++----- .../Tests/AppTests/MatchFlowTests.swift | 42 +++++++++++++++++ 2 files changed, 77 insertions(+), 12 deletions(-) diff --git a/chess-server/Sources/App/Online/GameCoordinator.swift b/chess-server/Sources/App/Online/GameCoordinator.swift index bf2bf12..441a7b9 100644 --- a/chess-server/Sources/App/Online/GameCoordinator.swift +++ b/chess-server/Sources/App/Online/GameCoordinator.swift @@ -49,8 +49,8 @@ struct MatchmakingConfig: Sendable { /// and clocks/timeouts are enforced here. All mutation happens on this actor; /// sockets are only written to from here. actor GameCoordinator { - /// How long a disconnected player has to return before forfeiting. - static let abandonGracePeriod: Duration = .seconds(60) + /// Default time a disconnected player has to return before forfeiting. + static let defaultAbandonGracePeriod: Duration = .seconds(60) struct Seat { let userID: UUID @@ -67,7 +67,11 @@ actor GameCoordinator { var black: Seat let timeControl: TimeControl let clock: ClockConfig - var abandonTask: Task? + /// Per-color forfeit timers. Keyed by color so a reconnecting player + /// cancels only their own pending forfeit — never their still-absent + /// opponent's. (A single shared task let one player's return cancel the + /// other's abandonment forfeit when both had dropped.) + var abandonTasks: [PieceColor: Task] = [:] // Clock state: the side to move has been burning time since `turnStartedAt`. var whiteSeconds: Double @@ -110,6 +114,18 @@ actor GameCoordinator { if black.userID == userID { black.socket = socket } } + /// Cancels and clears the pending abandonment forfeit for one color. + func cancelAbandonTask(for color: PieceColor) { + abandonTasks[color]?.cancel() + abandonTasks[color] = nil + } + + /// Cancels both colors' forfeit timers (game teardown). + func cancelAllAbandonTasks() { + for task in abandonTasks.values { task.cancel() } + abandonTasks.removeAll() + } + func remainingSeconds(of color: PieceColor) -> Double { color == .white ? whiteSeconds : blackSeconds } @@ -209,17 +225,21 @@ actor GameCoordinator { private let rematchWindow: Duration /// Rating-window pairing rules; tests inject fast-widening variants. private let matchmaking: MatchmakingConfig + /// Grace before a disconnected player forfeits; tests inject a short one. + private let abandonGracePeriod: Duration init( app: Application, clock: ClockConfig? = nil, rematchWindow: Duration = .seconds(60), - matchmaking: MatchmakingConfig = .standard + matchmaking: MatchmakingConfig = .standard, + abandonGracePeriod: Duration = GameCoordinator.defaultAbandonGracePeriod ) { self.app = app self.clockOverride = clock self.rematchWindow = rematchWindow self.matchmaking = matchmaking + self.abandonGracePeriod = abandonGracePeriod } private func clockConfig(for control: TimeControl) -> ClockConfig { @@ -236,10 +256,9 @@ actor GameCoordinator { socketsByUser[userID] = socket // Reconnect to a game in progress, if any. - if let game = activeGame(for: userID) { + if let game = activeGame(for: userID), let color = game.color(of: userID) { game.setSocket(socket, for: userID) - game.abandonTask?.cancel() - game.abandonTask = nil + game.cancelAbandonTask(for: color) send(gameStartMessage(game, for: userID), to: socket) send(.opponentStatus(connected: game.opponentSeat(of: userID)?.socket != nil), to: socket) send(.opponentStatus(connected: true), to: game.opponentSeat(of: userID)?.socket) @@ -253,15 +272,19 @@ actor GameCoordinator { removeFromAllQueues(userID: userID) abandonRematch(userID: userID) - guard let game = activeGame(for: userID) else { return } + guard let game = activeGame(for: userID), let color = game.color(of: userID) else { return } game.setSocket(nil, for: userID) send(.opponentStatus(connected: false), to: game.opponentSeat(of: userID)?.socket) // Forfeit if the player doesn't come back in time. (The chess clock - // keeps running regardless and may end the game sooner.) + // keeps running regardless and may end the game sooner.) Keyed by + // color so this timer is independent of the opponent's — if both drop + // and one returns, the other's forfeit still stands. let gameID = game.id - game.abandonTask = Task { [weak self] in - try? await Task.sleep(for: Self.abandonGracePeriod) + let grace = abandonGracePeriod + game.cancelAbandonTask(for: color) + game.abandonTasks[color] = Task { [weak self] in + try? await Task.sleep(for: grace) guard !Task.isCancelled else { return } await self?.forfeitIfStillGone(gameID: gameID, userID: userID) } @@ -634,7 +657,7 @@ actor GameCoordinator { // MARK: - Game teardown private func finish(_ game: LiveGame) async { - game.abandonTask?.cancel() + game.cancelAllAbandonTasks() game.timeoutTask?.cancel() gamesByID[game.id] = nil gameIDByUser[game.white.userID] = nil diff --git a/chess-server/Tests/AppTests/MatchFlowTests.swift b/chess-server/Tests/AppTests/MatchFlowTests.swift index af777b3..d7b7067 100644 --- a/chess-server/Tests/AppTests/MatchFlowTests.swift +++ b/chess-server/Tests/AppTests/MatchFlowTests.swift @@ -243,6 +243,48 @@ final class MatchFlowTests: XCTestCase { try await match.black.close() } + func testReconnectDoesNotCancelAbsentOpponentsForfeit() async throws { + // Sub-second grace so the abandonment forfeit resolves quickly. + app.gameCoordinator = GameCoordinator(app: app, abandonGracePeriod: .milliseconds(1500)) + + let match = try await startMatch() + + // Both players drop — White first, then Black. Under a single shared + // abandon task, the field would now hold Black's forfeit, not White's. + try await match.white.close() + try await Task.sleep(for: .milliseconds(250)) + try await match.black.close() + try await Task.sleep(for: .milliseconds(250)) + + // White returns inside the grace window on a fresh socket. This must + // cancel only White's own pending forfeit. + let whiteBack = try await TestSocket.connect( + port: port, token: match.whiteAuth.accessToken, on: app.eventLoopGroup + ) + + // Black never came back: its abandonment forfeit must still fire, and + // White (reconnected) must be told they won. Pre-fix, White's reconnect + // cancelled Black's forfeit, so no game_over ever arrived and the game + // hung until Black's clock happened to expire. + var result: String? + var reason: String? + drain: for _ in 0..<6 { + do { + if case .gameOver(let over) = try await whiteBack.next(timeoutSeconds: 4) { + result = over.result + reason = over.reason + break drain + } + } catch TestSocketError.timeout { + break drain + } + } + XCTAssertEqual(result, "1-0", "absent opponent must forfeit despite our reconnect") + XCTAssertEqual(reason, "abandoned") + + try await whiteBack.close() + } + func testMatchmakingIsolatesTimeControls() async throws { // Four players, two controls, interleaved joins: bullet players pair // only with each other, rapid players only with each other, and each