Commit f5207cc4 authored by Mahmoud Aglan's avatar Mahmoud Aglan

fix(matchmaking): test accounts and real players never meet in a public queue

The play tests run against production, and a test account waiting in the
public chess queue was seated across a real player (karimelshafei22), who
then played an account that vanished with the test's teardown. Nothing in
any queue knew a test account from a person.

profiles.is_test (migrations/2026-10-27-test-accounts.sql, applied) marks
an account the tests made. It is a column, not a name: usernames and
display names are the player's own choice, so a name marker could drop a
real player into the test pool. The profiles guard keeps the column out of
a player's token, both ways. The migration also marks the 23 ZZZ-* test
accounts earlier runs left behind.

includes/match-pool.php keeps every search to the searcher's pool: the
queue call and the status pickup of an earlier queuer in chess, ludo,
domino and backgammon, and domino's public tables of three and four.
Unknown means no: a profile that cannot be read is seated with nobody. The
bot fill only ever holds the player's own row, after that search, so it
needs no change. Real players among themselves pair exactly as before.
Co-Authored-By: 's avatarClaude Opus 5.5 <noreply@anthropic.com>
parent 90ab18f7
...@@ -10,6 +10,7 @@ require_once __DIR__ . '/../includes/auth.php'; ...@@ -10,6 +10,7 @@ require_once __DIR__ . '/../includes/auth.php';
require_once __DIR__ . '/../includes/hand-off.php'; require_once __DIR__ . '/../includes/hand-off.php';
require_once __DIR__ . '/../includes/match-presence.php'; require_once __DIR__ . '/../includes/match-presence.php';
require_once __DIR__ . '/../includes/ghost.php'; require_once __DIR__ . '/../includes/ghost.php';
require_once __DIR__ . '/../includes/match-pool.php';
$token = requireAuth(); $token = requireAuth();
$userId = getUserId($token); $userId = getUserId($token);
...@@ -125,18 +126,19 @@ function handleQueue(string $userId, array $input): void { ...@@ -125,18 +126,19 @@ function handleQueue(string $userId, array $input): void {
// STEP 3: Search for an opponent who wants the same game: same variant and // STEP 3: Search for an opponent who wants the same game: same variant and
// the same match length (a 1-point game and a 7-point match are not the // the same match length (a 1-point game and a 7-point match are not the
// same request). // same request), and of our own pool (test accounts never meet real
$opponents = $sdb->get('backgammon_queue', [ // players, includes/match-pool.php).
$opponents = matchPoolKeep($sdb, $userId, $sdb->get('backgammon_queue', [
'user_id' => 'neq.' . $userId, 'user_id' => 'neq.' . $userId,
'match_id' => 'is.null', 'match_id' => 'is.null',
'variant' => 'eq.' . $variant, 'variant' => 'eq.' . $variant,
'match_length' => 'eq.' . $matchLength, 'match_length' => 'eq.' . $matchLength,
'select' => 'id,user_id', 'select' => 'id,user_id',
'order' => 'created_at.asc', 'order' => 'created_at.asc',
'limit' => 1, 'limit' => MATCH_POOL_SCAN,
]); ]), 'user_id');
if (is_array($opponents) && !isset($opponents['error']) && isset($opponents[0])) { if (isset($opponents[0])) {
$opponent = $opponents[0]; $opponent = $opponents[0];
// STEP 4: Atomically claim opponent — set match_id to a placeholder to prevent races // STEP 4: Atomically claim opponent — set match_id to a placeholder to prevent races
...@@ -359,7 +361,8 @@ function bgMatchFromStatus($sdb, string $userId, string $myRowId): ?array { ...@@ -359,7 +361,8 @@ function bgMatchFromStatus($sdb, string $userId, string $myRowId): ?array {
$mine = $sdb->get('backgammon_queue', ['id' => 'eq.' . $myRowId, 'select' => 'id,variant,match_length,created_at', 'limit' => 1]); $mine = $sdb->get('backgammon_queue', ['id' => 'eq.' . $myRowId, 'select' => 'id,variant,match_length,created_at', 'limit' => 1]);
if (!is_array($mine) || isset($mine['error']) || empty($mine) || empty($mine[0]['created_at'])) return null; if (!is_array($mine) || isset($mine['error']) || empty($mine) || empty($mine[0]['created_at'])) return null;
$mine = $mine[0]; $mine = $mine[0];
$older = $sdb->get('backgammon_queue', [ // Our own pool only (includes/match-pool.php).
$older = matchPoolKeep($sdb, $userId, $sdb->get('backgammon_queue', [
'user_id' => 'neq.' . $userId, 'user_id' => 'neq.' . $userId,
'match_id' => 'is.null', 'match_id' => 'is.null',
'variant' => 'eq.' . $mine['variant'], 'variant' => 'eq.' . $mine['variant'],
...@@ -367,9 +370,9 @@ function bgMatchFromStatus($sdb, string $userId, string $myRowId): ?array { ...@@ -367,9 +370,9 @@ function bgMatchFromStatus($sdb, string $userId, string $myRowId): ?array {
'and' => '(created_at.lt."' . $mine['created_at'] . '",created_at.gt."' . gmdate('c', time() - 90) . '")', 'and' => '(created_at.lt."' . $mine['created_at'] . '",created_at.gt."' . gmdate('c', time() - 90) . '")',
'select' => 'id,user_id', 'select' => 'id,user_id',
'order' => 'created_at.asc', 'order' => 'created_at.asc',
'limit' => 1, 'limit' => MATCH_POOL_SCAN,
]); ]), 'user_id');
if (!is_array($older) || isset($older['error']) || empty($older)) return null; if (!$older) return null;
$opp = $older[0]; $opp = $older[0];
$release = function (string $id) use ($sdb) { $release = function (string $id) use ($sdb) {
......
...@@ -10,6 +10,7 @@ require_once __DIR__ . '/../includes/auth.php'; ...@@ -10,6 +10,7 @@ require_once __DIR__ . '/../includes/auth.php';
require_once __DIR__ . '/../includes/hand-off.php'; require_once __DIR__ . '/../includes/hand-off.php';
require_once __DIR__ . '/../includes/match-presence.php'; require_once __DIR__ . '/../includes/match-presence.php';
require_once __DIR__ . '/../includes/ghost.php'; require_once __DIR__ . '/../includes/ghost.php';
require_once __DIR__ . '/../includes/match-pool.php';
$token = requireAuth(); $token = requireAuth();
$userId = getUserId($token); $userId = getUserId($token);
...@@ -165,16 +166,17 @@ function handleQueue(string $userId, array $input): void { ...@@ -165,16 +166,17 @@ function handleQueue(string $userId, array $input): void {
'created_at' => 'lt.' . $staleTime 'created_at' => 'lt.' . $staleTime
]); ]);
// STEP 3: Search for available opponent // STEP 3: Search for available opponent, of our own pool (test accounts
$opponents = $sdb->get('domino_queue', [ // never meet real players, includes/match-pool.php).
$opponents = matchPoolKeep($sdb, $userId, $sdb->get('domino_queue', [
'user_id' => 'neq.' . $userId, 'user_id' => 'neq.' . $userId,
'match_id' => 'is.null', 'match_id' => 'is.null',
'select' => 'id,user_id', 'select' => 'id,user_id',
'order' => 'created_at.asc', 'order' => 'created_at.asc',
'limit' => 1, 'limit' => MATCH_POOL_SCAN,
]); ]), 'user_id');
if (is_array($opponents) && !isset($opponents['error']) && isset($opponents[0])) { if (isset($opponents[0])) {
$opponent = $opponents[0]; $opponent = $opponents[0];
// STEP 4: Atomically claim opponent — prevents race conditions // STEP 4: Atomically claim opponent — prevents race conditions
...@@ -341,15 +343,16 @@ function dominoCreateMatch($sdb, string $hostId, string $userId): ?array { ...@@ -341,15 +343,16 @@ function dominoCreateMatch($sdb, string $hostId, string $userId): ?array {
*/ */
function dominoMatchFromStatus($sdb, string $userId, array $mine): ?array { function dominoMatchFromStatus($sdb, string $userId, array $mine): ?array {
if (empty($mine['created_at'])) return null; if (empty($mine['created_at'])) return null;
$older = $sdb->get('domino_queue', [ // Our own pool only (includes/match-pool.php).
$older = matchPoolKeep($sdb, $userId, $sdb->get('domino_queue', [
'user_id' => 'neq.' . $userId, 'user_id' => 'neq.' . $userId,
'match_id' => 'is.null', 'match_id' => 'is.null',
'and' => '(created_at.lt."' . $mine['created_at'] . '",created_at.gt."' . gmdate('c', time() - 90) . '")', 'and' => '(created_at.lt."' . $mine['created_at'] . '",created_at.gt."' . gmdate('c', time() - 90) . '")',
'select' => 'id,user_id', 'select' => 'id,user_id',
'order' => 'created_at.asc', 'order' => 'created_at.asc',
'limit' => 1, 'limit' => MATCH_POOL_SCAN,
]); ]), 'user_id');
if (!is_array($older) || isset($older['error']) || empty($older)) return null; if (!$older) return null;
$opp = $older[0]; $opp = $older[0];
$release = function (string $id) use ($sdb) { $release = function (string $id) use ($sdb) {
...@@ -832,7 +835,9 @@ function handleTableCreate(string $userId, array $input): void { ...@@ -832,7 +835,9 @@ function handleTableCreate(string $userId, array $input): void {
/** /**
* Sit at the oldest public table of this size that is still waiting and whose * Sit at the oldest public table of this size that is still waiting and whose
* host is still there; open one when there is none. * host is still there; open one when there is none. Only a table whose people
* are all of our own pool: test accounts never sit with real players
* (includes/match-pool.php).
*/ */
function handleTableQuick(string $userId, array $input): void { function handleTableQuick(string $userId, array $input): void {
$sdb = supabaseService(); $sdb = supabaseService();
...@@ -850,7 +855,8 @@ function handleTableQuick(string $userId, array $input): void { ...@@ -850,7 +855,8 @@ function handleTableQuick(string $userId, array $input): void {
'order' => 'created_at.asc', 'order' => 'created_at.asc',
'limit' => 8, 'limit' => 8,
]); ]);
if (is_array($open) && !isset($open['error'])) { $open = matchPoolKeepTables($sdb, $userId, $open, fn($t) => dominoJson($t['players'] ?? null));
if ($open) {
foreach ($open as $t) { foreach ($open as $t) {
$players = dominoJson($t['players'] ?? null); $players = dominoJson($t['players'] ?? null);
if (in_array($userId, $players, true) || count($players) >= $count || !$players) continue; if (in_array($userId, $players, true) || count($players) >= $count || !$players) continue;
......
...@@ -10,6 +10,7 @@ require_once __DIR__ . '/../includes/auth.php'; ...@@ -10,6 +10,7 @@ require_once __DIR__ . '/../includes/auth.php';
require_once __DIR__ . '/../includes/hand-off.php'; require_once __DIR__ . '/../includes/hand-off.php';
require_once __DIR__ . '/../includes/match-presence.php'; require_once __DIR__ . '/../includes/match-presence.php';
require_once __DIR__ . '/../includes/ghost.php'; require_once __DIR__ . '/../includes/ghost.php';
require_once __DIR__ . '/../includes/match-pool.php';
$token = requireAuth(); $token = requireAuth();
$userId = getUserId($token); $userId = getUserId($token);
...@@ -137,9 +138,11 @@ function handleLudoQueue(string $userId, array $input): void { ...@@ -137,9 +138,11 @@ function handleLudoQueue(string $userId, array $input): void {
} }
/** /**
* Waiting opponents, oldest first. With $olderThan set, only rows queued * Waiting opponents, oldest first, of our own pool only (test accounts never
* before it and still fresh: a waiting player only ever claims the players * meet real players, includes/match-pool.php). With $olderThan set, only rows
* who were waiting before them, so two pollers can never claim each other. * queued before it and still fresh: a waiting player only ever claims the
* players who were waiting before them, so two pollers can never claim each
* other.
*/ */
function ludoFindOpponents($sdb, string $userId, int $limit, ?string $olderThan, ?int $playerCount = null): array { function ludoFindOpponents($sdb, string $userId, int $limit, ?string $olderThan, ?int $playerCount = null): array {
$params = [ $params = [
...@@ -147,18 +150,16 @@ function ludoFindOpponents($sdb, string $userId, int $limit, ?string $olderThan, ...@@ -147,18 +150,16 @@ function ludoFindOpponents($sdb, string $userId, int $limit, ?string $olderThan,
'match_id' => 'is.null', 'match_id' => 'is.null',
'select' => 'id,user_id,queued_at', 'select' => 'id,user_id,queued_at',
'order' => 'queued_at.asc', 'order' => 'queued_at.asc',
'limit' => $olderThan === null ? $limit : 20, 'limit' => max($limit, MATCH_POOL_SCAN),
]; ];
if ($olderThan !== null) $params['queued_at'] = 'lt.' . $olderThan; if ($olderThan !== null) $params['queued_at'] = 'lt.' . $olderThan;
// Only players who asked for the same table: a two-player search used to // Only players who asked for the same table: a two-player search used to
// seat someone waiting for four at a two-seat table. // seat someone waiting for four at a two-seat table.
if ($playerCount !== null) $params['player_count'] = 'eq.' . $playerCount; if ($playerCount !== null) $params['player_count'] = 'eq.' . $playerCount;
$rows = $sdb->get('ludo_queue', $params); $rows = matchPoolKeep($sdb, $userId, $sdb->get('ludo_queue', $params), 'user_id');
if (!is_array($rows) || isset($rows['error'])) return [];
if ($olderThan === null) return array_values($rows);
// Rows nobody has re-queued for 90 seconds belong to players who left. // Rows nobody has re-queued for 90 seconds belong to players who left.
$fresh = array_filter($rows, fn($r) => strtotime($r['queued_at'] ?? '') > time() - 90); if ($olderThan !== null) $rows = array_values(array_filter($rows, fn($r) => strtotime($r['queued_at'] ?? '') > time() - 90));
return array_slice(array_values($fresh), 0, $limit); return array_slice($rows, 0, $limit);
} }
/** Claim each row by swapping its null match_id for the placeholder; keep the ones we won. */ /** Claim each row by swapping its null match_id for the placeholder; keep the ones we won. */
......
...@@ -10,6 +10,7 @@ require_once __DIR__ . '/../includes/auth.php'; ...@@ -10,6 +10,7 @@ require_once __DIR__ . '/../includes/auth.php';
require_once __DIR__ . '/../includes/hand-off.php'; require_once __DIR__ . '/../includes/hand-off.php';
require_once __DIR__ . '/../includes/chess.php'; require_once __DIR__ . '/../includes/chess.php';
require_once __DIR__ . '/../includes/ghost.php'; require_once __DIR__ . '/../includes/ghost.php';
require_once __DIR__ . '/../includes/match-pool.php';
$token = requireAuth(); $token = requireAuth();
$userId = getUserId($token); $userId = getUserId($token);
...@@ -121,18 +122,20 @@ function handleQueue($db, string $userId, array $input): void { ...@@ -121,18 +122,20 @@ function handleQueue($db, string $userId, array $input): void {
// This was a hand-rolled curl that re-implemented auth and URL building — // This was a hand-rolled curl that re-implemented auth and URL building —
// the only query in the file that bypassed the client. Going through the // the only query in the file that bypassed the client. Going through the
// client keeps one code path for headers, timeouts and error shape. // client keeps one code path for headers, timeouts and error shape.
// Only someone of our own pool: test accounts never meet real players
// (includes/match-pool.php).
$excludeIds = array_merge([$userId], $blockedIds); $excludeIds = array_merge([$userId], $blockedIds);
$opponents = $sdb->get('matchmaking_queue', [ $opponents = matchPoolKeep($sdb, $userId, $sdb->get('matchmaking_queue', [
'game_key' => 'eq.' . $gameKey, 'game_key' => 'eq.' . $gameKey,
'time_control' => 'eq.' . $timeControl, 'time_control' => 'eq.' . $timeControl,
'status' => 'eq.waiting', 'status' => 'eq.waiting',
'player_id' => 'not.in.(' . implode(',', $excludeIds) . ')', 'player_id' => 'not.in.(' . implode(',', $excludeIds) . ')',
'select' => 'id,player_id,rating', 'select' => 'id,player_id,rating',
'order' => 'queued_at.asc', 'order' => 'queued_at.asc',
'limit' => 1, 'limit' => MATCH_POOL_SCAN,
]); ]), 'player_id');
if (!empty($opponents) && !isset($opponents['error']) && isset($opponents[0])) { if (isset($opponents[0])) {
$opponent = $opponents[0]; $opponent = $opponents[0];
// STEP 6: Atomically claim opponent — mark them as matched FIRST // STEP 6: Atomically claim opponent — mark them as matched FIRST
...@@ -407,7 +410,8 @@ function matchFromStatus($sdb, string $userId): ?array { ...@@ -407,7 +410,8 @@ function matchFromStatus($sdb, string $userId): ?array {
foreach ($blocks as $b) $exclude[] = ($b['player_id'] === $userId) ? $b['blocked_id'] : $b['player_id']; foreach ($blocks as $b) $exclude[] = ($b['player_id'] === $userId) ? $b['blocked_id'] : $b['player_id'];
} }
$older = $sdb->get('matchmaking_queue', [ // Our own pool only (includes/match-pool.php).
$older = matchPoolKeep($sdb, $userId, $sdb->get('matchmaking_queue', [
'game_key' => 'eq.' . $row['game_key'], 'game_key' => 'eq.' . $row['game_key'],
'time_control' => 'eq.' . $row['time_control'], 'time_control' => 'eq.' . $row['time_control'],
'status' => 'eq.waiting', 'status' => 'eq.waiting',
...@@ -415,9 +419,9 @@ function matchFromStatus($sdb, string $userId): ?array { ...@@ -415,9 +419,9 @@ function matchFromStatus($sdb, string $userId): ?array {
'and' => '(queued_at.lt."' . $row['queued_at'] . '",queued_at.gt."' . gmdate('c', time() - 90) . '")', 'and' => '(queued_at.lt."' . $row['queued_at'] . '",queued_at.gt."' . gmdate('c', time() - 90) . '")',
'select' => 'id,player_id', 'select' => 'id,player_id',
'order' => 'queued_at.asc', 'order' => 'queued_at.asc',
'limit' => 1, 'limit' => MATCH_POOL_SCAN,
]); ]), 'player_id');
if (!is_array($older) || isset($older['error']) || empty($older)) return null; if (!$older) return null;
$opponent = $older[0]; $opponent = $older[0];
// Lock our own row. // Lock our own row.
......
...@@ -21,6 +21,11 @@ ...@@ -21,6 +21,11 @@
* Only a client that says it can play one (the status poll carries ghost: 1) * Only a client that says it can play one (the status poll carries ghost: 1)
* is ever given one: an older build would sit in front of an opponent that * is ever given one: an older build would sit in front of an opponent that
* never moves. * never moves.
*
* A bot fill holds only the player's own queue row, never anyone else's, and
* runs after the status poll's search for a waiting person, which keeps to the
* player's own pool (includes/match-pool.php). So a test account and a real
* player are never handed each other's row here either.
*/ */
require_once __DIR__ . '/supabase.php'; require_once __DIR__ . '/supabase.php';
......
<?php
/**
* Test accounts and real players never meet in a public queue.
*
* The automated play tests (tests/play) run against the production database.
* A test account waiting in the public chess queue was seated across a real
* player, who then played an account that vanished with the test's teardown.
*
* Every account belongs to one pool, read from profiles.is_test (true for the
* accounts the tests make; migrations/2026-10-27-test-accounts.sql). A player's
* own token cannot change that column, so a name a player picks can never move
* them into the test pool. Every search for an opponent (the queue call, the
* status poll that picks up an earlier queuer, domino's public tables) keeps
* only candidates of the searcher's own pool, and the bot fill that follows a
* minute of nobody only ever holds the player's own row.
*
* Unknown means no: a candidate whose profile cannot be read is seated with
* nobody, and so is everyone when the searcher's own profile cannot be read.
* A failed lookup costs one poll, never a mixed table.
*/
require_once __DIR__ . '/supabase.php';
/** Candidates read per search, so the oldest of our own pool is still found behind a few of the other. */
const MATCH_POOL_SCAN = 20;
/** @return array<string, bool> profile id => is_test, for the ids whose profile could be read */
function matchPoolFlags($sdb, array $ids): array {
$ids = array_values(array_unique(array_filter($ids, fn($x) => is_string($x)
&& preg_match('/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i', $x))));
if (!$ids) return [];
$rows = $sdb->get('profiles', ['id' => 'in.(' . implode(',', $ids) . ')', 'select' => 'id,is_test']);
if (!is_array($rows) || isset($rows['error'])) return [];
$out = [];
foreach ($rows as $r) {
if (is_array($r) && isset($r['id']) && array_key_exists('is_test', $r)) $out[$r['id']] = $r['is_test'] === true;
}
return $out;
}
/**
* The rows (queue rows, each naming its player under $key) whose player is in
* $userId's pool, in the order given.
*/
function matchPoolKeep($sdb, string $userId, $rows, string $key): array {
if (!is_array($rows) || isset($rows['error']) || !$rows) return [];
$rows = array_values($rows);
$flags = matchPoolFlags($sdb, array_merge([$userId], array_column($rows, $key)));
if (!array_key_exists($userId, $flags)) return [];
$mine = $flags[$userId];
return array_values(array_filter($rows, function ($r) use ($flags, $mine, $key) {
$id = is_array($r) ? ($r[$key] ?? null) : null;
return is_string($id) && array_key_exists($id, $flags) && $flags[$id] === $mine;
}));
}
/**
* Of these tables (each with its seated `players`), the ones every person at
* which is in $userId's pool. Computer seats ('bot_...') belong to no pool.
*/
function matchPoolKeepTables($sdb, string $userId, $tables, callable $playersOf): array {
if (!is_array($tables) || isset($tables['error']) || !$tables) return [];
$people = fn($t) => array_values(array_filter($playersOf($t), fn($p) => is_string($p) && !str_starts_with($p, 'bot_')));
$ids = [$userId];
foreach ($tables as $t) $ids = array_merge($ids, $people($t));
$flags = matchPoolFlags($sdb, $ids);
if (!array_key_exists($userId, $flags)) return [];
$mine = $flags[$userId];
return array_values(array_filter($tables, function ($t) use ($flags, $mine, $people) {
foreach ($people($t) as $p) {
if (!array_key_exists($p, $flags) || $flags[$p] !== $mine) return false;
}
return true;
}));
}
-- Test accounts: one marker, read by every public queue.
--
-- The automated play tests (tests/play/*.mjs) run against this database. A
-- test account waiting in the public chess queue was seated across a real
-- player, who then played an account that vanished with the test's teardown.
-- profiles.is_test marks an account the tests made; the queues (chess, ludo,
-- domino, backgammon, and domino's public tables) seat a test account only
-- with test accounts and a real player only with real players
-- (includes/match-pool.php).
--
-- Why a column and not a name: a username or display name is the player's
-- own choice and can be changed with their own token, so a real player could
-- land in the test pool by picking a name. This column is not on the
-- profiles_guard allow-list (2026-10-26-profiles-guard.sql), so a player's
-- token cannot change it; only the server and the tests (service key) can.
--
-- Idempotent: safe to run again.
BEGIN;
SET LOCAL lock_timeout = '5s';
-- A constant default: no table rewrite, every existing row reads false.
ALTER TABLE public.profiles ADD COLUMN IF NOT EXISTS is_test boolean NOT NULL DEFAULT false;
COMMENT ON COLUMN public.profiles.is_test IS
'An account made by the automated tests. Public queues never seat it with a real player. Set with the service key only.';
-- The test pool is a handful of rows: a partial index finds them without
-- touching the real ones.
CREATE INDEX IF NOT EXISTS idx_profiles_is_test ON public.profiles (id) WHERE is_test;
-- Accounts earlier runs left behind, named with the tests' tag (ZZZ-PLAYTEST,
-- ZZZ-LUDO-TEST, ZZZ-DOMINO-TEST, ZZZ-GHOST, ZZZ-SHELL, ZZZ-CYCLE, ...).
-- Bounded by the date this marker arrived, so running this again can never
-- sweep up a real player who names themselves ZZZ-something later.
UPDATE public.profiles
SET is_test = true
WHERE NOT is_test
AND display_name LIKE 'ZZZ-%'
AND created_at < '2026-10-27T00:00:00Z';
COMMIT;
-- PostgREST reads the new column at once.
NOTIFY pgrst, 'reload schema';
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment