Merge pull request #20 from Saul5662/fix/gain-ambiguous-selector

fix(commands): resolve duplicate nicknames in `rs gain`, add a help subcommand

server/src/services/operatorCommandService/commands/gain.js

@@ -1,10 +1,44 @@
 // Operator Gain Command
 // Purpose: Handles the audio gain boost permission for VIPs.
 // Scope: Supports list, grant, and revoke subcommands; resolution stays VIP-only.
-const { mask, resolveIdentitySelector } = require('./resolvers');
+const { mask, normalizeSearchText, resolveIdentitySelector } = require('./resolvers');
 const { getCommandConfig } = require('../../operatorCommandService/config');
 
+/*
+  Every connected socket's canonical user id. One person can hold several sockets
+  across tabs, so this is a set of identities rather than a count of connections.
+*/
+function collectOnlineUserIds(io) {
+  const online = new Set();
+  const sockets = io?.sockets?.sockets;
+  if (!sockets || typeof sockets.forEach !== 'function') return online;
+  sockets.forEach((socket) => {
+    const userId = String(socket?.data?.userId || '').trim();
+    if (userId) online.add(userId);
+  });
+  return online;
+}
+
+/*
+  Candidates whose identity fields equal the selector outright. Nicknames are not
+  unique — the same person re-verifying from a new browser produces a second
+  verified record with the same name — so an exact nickname match can legitimately
+  return several records.
+*/
+function findExactMatches(selector, candidates) {
+  const needle = normalizeSearchText(selector);
+  if (!needle) return [];
+  return (Array.isArray(candidates) ? candidates : []).filter((record) => (
+    normalizeSearchText(record?.nickname) === needle
+    || normalizeSearchText(record?.userId) === needle
+    || normalizeSearchText(record?.id) === needle
+    || normalizeSearchText(record?.cookieUserId) === needle
+    || normalizeSearchText(record?.fingerprintId) === needle
+  ));
+}
+
 function createGainCommand({
+  io,
   listVerifiedUsers,
   listAudioGainBoostUsers,
   grantAudioGainBoost,
@@ -20,6 +54,61 @@ function createGainCommand({
     return `Usage: \`${commandPrefix} gain ${subcommand} <nickname|userId|cookieUserId>\``;
   }
 
+  /*
+    Duplicate nicknames used to make `gain grant <name>` unusable: the shared
+    resolver refuses on ambiguity, which is right for destructive commands like
+    deter and kick but wrong here. Granting a volume ceiling to the wrong one of
+    two accounts belonging to the same person is recoverable, so this command
+    picks one and says which.
+
+    The online account wins, because that is who the admin is reacting to. With
+    nobody online the first stored record is used. The shared fuzzy resolver still
+    handles the no-exact-match case so typo tolerance and its error text are
+    unchanged.
+  */
+  function resolveBoostTarget(selector, candidates) {
+    const exact = findExactMatches(selector, candidates);
+    if (exact.length === 1) return { record: exact[0] };
+
+    if (exact.length > 1) {
+      const onlineUserIds = collectOnlineUserIds(io);
+      const onlineMatches = exact.filter((record) => {
+        const userId = String(record?.userId || '').trim();
+        return userId && onlineUserIds.has(userId);
+      });
+      if (onlineMatches.length) {
+        return { record: onlineMatches[0], duplicates: exact.length, picked: 'online' };
+      }
+      return { record: exact[0], duplicates: exact.length, picked: 'first' };
+    }
+
+    return resolveIdentitySelector(selector, candidates, { includeId: false });
+  }
+
+  function describePick(resolved) {
+    if (!resolved.duplicates) return '';
+    if (resolved.picked === 'online') {
+      return ` ${resolved.duplicates} accounts share that name; picked the one that is online.`;
+    }
+    return ` ${resolved.duplicates} accounts share that name and none are online; picked the first.`;
+  }
+
+  function helpText() {
+    return [
+      '**Audio gain boost**',
+      'Raises a user\'s volume ceiling past the global gains, still bounded by the hard caps.',
+      '',
+      `\`${commandPrefix} gain list\` — show everyone who holds the boost.`,
+      `\`${commandPrefix} gain grant <vip>\` — give the boost to a verified user.`,
+      `\`${commandPrefix} gain revoke <vip>\` — take the boost away.`,
+      `\`${commandPrefix} gain help\` — show this.`,
+      '',
+      'A user can be named by nickname, userId, or cookieUserId. Only verified (VIP)',
+      'users can be granted the boost. If several accounts share a nickname, the one',
+      'that is currently online is used.',
+    ].join('\n');
+  }
+
   /*
     The boost is a VIP-only permission, so candidate matching runs against the
     verified list rather than every known identity. A nickname that only belongs
@@ -32,7 +121,7 @@ function createGainCommand({
       return message.reply({ content: usage(enabled ? 'grant' : 'revoke'), allowedMentions: plain });
     }
     const candidates = enabled ? listVerifiedUsers() : listAudioGainBoostUsers();
-    const resolved = resolveIdentitySelector(selector, candidates, { includeId: false });
+    const resolved = resolveBoostTarget(selector, candidates);
     if (resolved.error) {
       return message.reply({ content: sanitizeMentions(resolved.error), allowedMentions: plain });
     }
@@ -42,7 +131,7 @@ function createGainCommand({
       const user = enabled ? grantAudioGainBoost(target, actor) : revokeAudioGainBoost(target, actor);
       const verb = enabled ? 'Granted' : 'Revoked';
       return message.reply({
-        content: sanitizeMentions(`${verb} audio gain boost for ${user.nickname || 'unknown'} (${mask(user.cookieUserId)}).`),
+        content: sanitizeMentions(`${verb} audio gain boost for ${user.nickname || 'unknown'} (${mask(user.cookieUserId)}).${describePick(resolved)}`),
         allowedMentions: plain,
       });
     } catch (err) {
@@ -60,6 +149,10 @@ function createGainCommand({
     }
     const action = (tokens.shift() || 'list').toLowerCase();
 
+    if (action === 'help') {
+      return message.reply({ content: helpText(), allowedMentions: plain });
+    }
+
     if (action === 'list') {
       const users = listAudioGainBoostUsers();
       if (!users.length) {
@@ -78,7 +171,7 @@ function createGainCommand({
     if (action === 'revoke') return applyBoost(message, tokens, false);
 
     return message.reply({
-      content: `Unknown gain command. Use \`${commandPrefix} gain list\`, \`${commandPrefix} gain grant <vip>\`, or \`${commandPrefix} gain revoke <vip>\`.`,
+      content: `Unknown gain command.\n${helpText()}`,
       allowedMentions: plain,
     });
   };

server/src/services/operatorCommandService/commands/gain.test.js

@@ -10,10 +10,30 @@ const VIPS = [
   { userId: 'usr-other', nickname: 'Baguette', cookieUserId: 'cookie-baguette' },
 ];
 
-function createHarness({ verified = VIPS, boosted = [], isAdmin = true } = {}) {
+// Two verified records sharing one nickname. This is the real shape behind the
+// "matched multiple records" failure: one person re-verifying from a new browser
+// produces a second record with the same name and a different cookie id.
+const DUPLICATE_SAULS = [
+  { userId: 'usr-saul-a', nickname: 'Saul', cookieUserId: 'cu_a28ffffffff33ab5c' },
+  { userId: 'usr-saul-b', nickname: 'Saul', cookieUserId: 'cu_5a5ffffffffb6add3' },
+];
+
+function createSocketRegistry(onlineUserIds = []) {
+  const sockets = new Map();
+  onlineUserIds.forEach((userId, index) => {
+    // Two sockets per identity, so the resolver must dedupe rather than count
+    // connections.
+    sockets.set(`s${index}a`, { id: `s${index}a`, data: { userId } });
+    sockets.set(`s${index}b`, { id: `s${index}b`, data: { userId } });
+  });
+  return { sockets: { sockets } };
+}
+
+function createHarness({ verified = VIPS, boosted = [], isAdmin = true, online = [] } = {}) {
   const calls = [];
   const replies = [];
   const handler = createGainCommand({
+    io: createSocketRegistry(online),
     listVerifiedUsers: () => verified,
     listAudioGainBoostUsers: () => boosted,
     grantAudioGainBoost: (selector, actor) => {
@@ -106,9 +126,122 @@ test('list reports an empty holder set', async () => {
   assert.match(replies[0].content, /No users hold/);
 });
 
+test('duplicate nicknames resolve to the account that is online', async () => {
+  const { handler, message, calls, replies } = createHarness({
+    verified: DUPLICATE_SAULS,
+    online: ['usr-saul-b'],
+  });
+
+  await handler(message, ['grant', 'Saul']);
+
+  assert.deepEqual(calls, [{ action: 'grant', selector: 'usr-saul-b', actor: 'admin' }]);
+  assert.doesNotMatch(replies[0].content, /matched multiple records/i);
+  assert.match(replies[0].content, /2 accounts share that name; picked the one that is online/);
+});
+
+test('duplicate nicknames fall back to the first record when nobody is online', async () => {
+  const { handler, message, calls, replies } = createHarness({
+    verified: DUPLICATE_SAULS,
+    online: [],
+  });
+
+  await handler(message, ['grant', 'Saul']);
+
+  assert.deepEqual(calls, [{ action: 'grant', selector: 'usr-saul-a', actor: 'admin' }]);
+  assert.match(replies[0].content, /none are online; picked the first/);
+});
+
+test('an unrelated online user does not influence the pick', async () => {
+  const { handler, message, calls } = createHarness({
+    verified: DUPLICATE_SAULS,
+    online: ['usr-somebody-else'],
+  });
+
+  await handler(message, ['grant', 'Saul']);
+
+  assert.deepEqual(calls, [{ action: 'grant', selector: 'usr-saul-a', actor: 'admin' }]);
+});
+
+test('several duplicates online pick one deterministically rather than refusing', async () => {
+  const { handler, message, calls, replies } = createHarness({
+    verified: DUPLICATE_SAULS,
+    online: ['usr-saul-a', 'usr-saul-b'],
+  });
+
+  await handler(message, ['grant', 'Saul']);
+
+  assert.deepEqual(calls, [{ action: 'grant', selector: 'usr-saul-a', actor: 'admin' }]);
+  assert.match(replies[0].content, /picked the one that is online/);
+});
+
+test('revoke disambiguates the same way against the boosted list', async () => {
+  const { handler, message, calls } = createHarness({
+    boosted: DUPLICATE_SAULS,
+    online: ['usr-saul-b'],
+  });
+
+  await handler(message, ['revoke', 'Saul']);
+
+  assert.deepEqual(calls, [{ action: 'revoke', selector: 'usr-saul-b', actor: 'admin' }]);
+});
+
+test('a unique nickname reports no disambiguation note', async () => {
+  const { handler, message, replies } = createHarness();
+
+  await handler(message, ['grant', 'Croissant']);
+
+  assert.doesNotMatch(replies[0].content, /accounts share that name/);
+});
+
+test('an exact cookieUserId still selects one record out of a duplicate pair', async () => {
+  const { handler, message, calls } = createHarness({ verified: DUPLICATE_SAULS });
+
+  await handler(message, ['grant', 'cu_5a5ffffffffb6add3']);
+
+  assert.deepEqual(calls, [{ action: 'grant', selector: 'usr-saul-b', actor: 'admin' }]);
+});
+
+test('a typo still resolves through the fuzzy matcher', async () => {
+  const { handler, message, calls } = createHarness();
+
+  await handler(message, ['grant', 'Croissnat']);
+
+  assert.deepEqual(calls, [{ action: 'grant', selector: 'usr-vip', actor: 'admin' }]);
+});
+
+test('help lists every subcommand', async () => {
+  const { handler, message, calls, replies } = createHarness();
+
+  await handler(message, ['help']);
+
+  assert.deepEqual(calls, [], 'help must not change anything');
+  for (const fragment of ['rs gain list', 'rs gain grant <vip>', 'rs gain revoke <vip>', 'rs gain help']) {
+    assert.ok(replies[0].content.includes(fragment), `help should mention ${fragment}`);
+  }
+});
+
+test('an unknown subcommand falls back to the same help text', async () => {
+  const { handler, message, calls, replies } = createHarness();
+
+  await handler(message, ['sideways']);
+
+  assert.deepEqual(calls, []);
+  assert.match(replies[0].content, /Unknown gain command/);
+  assert.ok(replies[0].content.includes('rs gain grant <vip>'));
+});
+
+test('help stays admin-only like the rest of the command', async () => {
+  const { handler, message, replies } = createHarness({ isAdmin: false });
+
+  await handler(message, ['help']);
+
+  assert.match(replies[0].content, /Only admins/);
+});
+
 test('a service rejection is surfaced instead of thrown', async () => {
   const { handler, message, replies } = createHarness();
   const failing = createGainCommand({
+    io: createSocketRegistry([]),
     listVerifiedUsers: () => VIPS,
     listAudioGainBoostUsers: () => [],
     grantAudioGainBoost: () => {

server/src/services/operatorCommandService/registry.js

@@ -53,6 +53,7 @@ function buildCommandRegistry(prefix, timeCommand) {
         `${prefix} gain list`,
         `${prefix} gain grant <vip>`,
         `${prefix} gain revoke <vip>`,
+        `${prefix} gain help`,
       ],
       access: 'Admin',
       permission: 'admin',