From 5a1f5d90db7a50be41d910b0aea06c7e0711f059 Mon Sep 17 00:00:00 2001
From: RissRIce <jsdavid278@gmail.com>
Date: Wed, 12 Aug 2026 22:20:18 -0600
Subject: [PATCH] fix(games): reject duplicate queue entries (#117)

* test(games): reproduce duplicate queue deadlock

* fix(games): reject duplicate queue entries
---
 cmd/agentbbs/games.go             |  3 +++
 cmd/agentbbs/gamesws.go           |  6 ++++-
 internal/games/matchmaker.go      |  9 ++++++-
 internal/games/matchmaker_test.go | 40 +++++++++++++++++++++++++++++++
 4 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/cmd/agentbbs/games.go b/cmd/agentbbs/games.go
index e985278..67e6560 100644
--- a/cmd/agentbbs/games.go
+++ b/cmd/agentbbs/games.go
@@ -70,6 +70,9 @@ func (a *app) handleGame(s ssh.Session) {
 	case errors.Is(err, games.ErrNoOpponent):
 		_ = conn.Send(errEnvelope("no opponent found — try again later"))
 		_ = s.Exit(1)
+	case errors.Is(err, games.ErrAlreadyQueued):
+		_ = conn.Send(errEnvelope("this account is already queued for that game"))
+		_ = s.Exit(1)
 	case err != nil && !errors.Is(err, games.ErrUnknownGame):
 		// Unknown-game is already handled above; anything else is a wait abort
 		// (e.g. the agent disconnected) and needs no message.
diff --git a/cmd/agentbbs/gamesws.go b/cmd/agentbbs/gamesws.go
index 8329b24..2e16ca6 100644
--- a/cmd/agentbbs/gamesws.go
+++ b/cmd/agentbbs/gamesws.go
@@ -77,9 +77,13 @@ func (a *app) handleGameWS(w http.ResponseWriter, r *http.Request) {
 	defer func() { _ = a.st.EndSession(sessID) }()
 
 	_ = p.Send(map[string]any{"type": "queued", "game": gameID})
-	if err := a.mm.Play(context.Background(), gameID, p); errors.Is(err, games.ErrNoOpponent) {
+	err = a.mm.Play(context.Background(), gameID, p)
+	if errors.Is(err, games.ErrNoOpponent) {
 		_ = p.Send(errEnvelope("no opponent found — try again later"))
 	}
+	if errors.Is(err, games.ErrAlreadyQueued) {
+		_ = p.Send(errEnvelope("this account is already queued for that game"))
+	}
 }
 
 // wsPlayer adapts a gorilla WebSocket connection to games.PlayerIO. The match
diff --git a/internal/games/matchmaker.go b/internal/games/matchmaker.go
index 35ab9cb..d06c620 100644
--- a/internal/games/matchmaker.go
+++ b/internal/games/matchmaker.go
@@ -13,6 +13,9 @@ var ErrNoOpponent = errors.New("no opponent found")
 // ErrUnknownGame means the requested game id is not in the registry.
 var ErrUnknownGame = errors.New("unknown game")
 
+// ErrAlreadyQueued means the same player already has a connection waiting for this game.
+var ErrAlreadyQueued = errors.New("player already queued")
+
 // Store persists finished matches and tracks per-game ELO ratings. The SQLite
 // store implements it; the matchmaker stays storage-agnostic.
 type Store interface {
@@ -77,7 +80,11 @@ func (mm *Matchmaker) Play(ctx context.Context, gameID string, io PlayerIO) erro
 	}
 
 	mm.mu.Lock()
-	if w, waiting := mm.queue[gameID]; waiting && w.io.Name() != io.Name() {
+	if w, waiting := mm.queue[gameID]; waiting {
+		if w.io.Name() == io.Name() {
+			mm.mu.Unlock()
+			return ErrAlreadyQueued
+		}
 		// An opponent is waiting — pair up and run the match.
 		delete(mm.queue, gameID)
 		mm.mu.Unlock()
diff --git a/internal/games/matchmaker_test.go b/internal/games/matchmaker_test.go
index d0db8a2..665fd06 100644
--- a/internal/games/matchmaker_test.go
+++ b/internal/games/matchmaker_test.go
@@ -2,6 +2,7 @@ package games
 
 import (
 	"context"
+	"errors"
 	"math"
 	"sync"
 	"testing"
@@ -133,3 +134,42 @@ func TestMatchmakerUnknownGame(t *testing.T) {
 		t.Fatalf("want ErrUnknownGame, got %v", err)
 	}
 }
+
+func TestMatchmakerRejectsDuplicateWaiter(t *testing.T) {
+	mm := NewMatchmaker(Catalog(), nil, time.Second, time.Minute)
+	firstCtx, cancelFirst := context.WithCancel(context.Background())
+	firstDone := make(chan error, 1)
+	go func() {
+		firstDone <- mm.Play(firstCtx, "ttt", &firstLegalPlayer{name: "same-agent"})
+	}()
+
+	deadline := time.Now().Add(time.Second)
+	for {
+		mm.mu.Lock()
+		queued := mm.queue["ttt"] != nil
+		mm.mu.Unlock()
+		if queued {
+			break
+		}
+		if time.Now().After(deadline) {
+			t.Fatal("first player never entered the queue")
+		}
+		time.Sleep(time.Millisecond)
+	}
+
+	duplicateCtx, cancelDuplicate := context.WithTimeout(context.Background(), 50*time.Millisecond)
+	defer cancelDuplicate()
+	if err := mm.Play(duplicateCtx, "ttt", &firstLegalPlayer{name: "same-agent"}); err == nil || errors.Is(err, context.DeadlineExceeded) {
+		t.Fatalf("duplicate Play error = %v, want immediate already-queued rejection", err)
+	}
+
+	cancelFirst()
+	select {
+	case err := <-firstDone:
+		if !errors.Is(err, context.Canceled) {
+			t.Fatalf("first Play error = %v, want context.Canceled", err)
+		}
+	case <-time.After(100 * time.Millisecond):
+		t.Fatal("first player remained blocked after its context was canceled")
+	}
+}