From 2d1d91e3722b070fef92ff89cfe2cca145681235 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 18 Aug 2026 18:53:45 -0500 Subject: [PATCH 1/2] feat(guilds): /guild, the module's own chat command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first command through `api.registerSlashCommands` (MODULE_API 1.6.0, TEAMS.md §7.1). The definition and the handler both live here; the bot pulls the definition and runs no line of this module. `/guild` and not `/team`, deliberately. Core does not own the word for a Team — that is what deleted its Team pages in phase 3 — so it does not publish the noun in a channel either. Core ships the dispatcher and zero commands. The audience rungs are re-resolved in the handler rather than assumed: a shard that gates guilds to staff does not become public because the question arrived over Discord. The provider's own staleness guard is honoured too, so a stale board answers "not connected" instead of reporting what it still holds, and `resolveUserId` is exported rather than copied so "linked" means here what it means on the roster. Co-Authored-By: Claude --- server/commands/guild.command.js | 189 ++++++++++++++++++ server/index.js | 11 + .../model/teamProvider/teamProvider.model.js | 7 +- server/test/_fakes.js | 5 + server/test/guildCommand.test.js | 139 +++++++++++++ 5 files changed, 350 insertions(+), 1 deletion(-) create mode 100644 server/commands/guild.command.js create mode 100644 server/test/guildCommand.test.js diff --git a/server/commands/guild.command.js b/server/commands/guild.command.js new file mode 100644 index 0000000..2958a56 --- /dev/null +++ b/server/commands/guild.command.js @@ -0,0 +1,189 @@ +// ── `/guild` — the first chat command through the module contract ────────── +// +// Registered with `api.registerSlashCommands` (MODULE_API 1.6.0, TEAMS.md §7.1). +// The definition and this handler live here; the bot pulls the definition over +// the app's internal API and runs nothing of ours. Nothing in this file knows +// what Discord is — it is handed an `actor` and returns an envelope, and the +// same handler would serve a second platform unchanged. +// +// **Why `/guild` and not `/team`.** Teams are core's primitive and "guild" is +// this module's word for one; core does not own the word, so it does not publish +// the noun in a channel either. That is the same correction that deleted core's +// Team pages in phase 3, applied to the chat surface. +// +// **The audience rungs are enforced here, exactly as they are on the website.** +// A shard whose `guilds` feature is gated to staff does not become public +// because the question arrived over Discord — this handler resolves the caller's +// rung through the same `shardVisibility` config the routes use. It is the one +// piece of this file that is a security boundary rather than presentation. +const core = require('../core') +const db = require('../model/teamProvider/teamProvider.db') +const provider = require('../model/teamProvider/teamProvider.model') +const visibility = require('../utils/shardVisibility') + +const log = core.logger('guild-command') + +// How many guilds the no-argument form lists. A Discord embed takes 25 fields; +// ten is a summary a person reads rather than a table they scroll past. +const LIST_LIMIT = 10 + +/** + * Where the caller sits on this module's ladder. + * + * The same resolution `projectRoster` does, and it is duplicated in shape rather + * than shared because the inputs differ: that one is handed a viewer core + * described, this one an actor. Both end at `viewerLevel`, and both answer + * `anonymous` DIRECTLY for a caller with no site account — handing `viewerLevel` + * a synthetic empty request makes it fall through to `auth.getUserFromRequest`, + * which expects real cookies and throws (the phase 3 bug). + */ +async function levelFor(actor) { + if (!actor || !actor.userId) return 'anonymous' + return visibility.viewerLevel({ user: { id: actor.userId, role: actor.role } }) +} + +// The nudge §9 answer 5 asks for, and only when it is TRUE. An unlinked caller +// who was told nothing because the shard publishes nothing is not helped by +// being invited to link; the prompt appears when linking is what would actually +// change the answer. +function linkPrompt(actor, audience) { + if (actor.isLinked) return null + if (audience === 'anonymous') return null + return 'Link your account on the site to see more — this shard shows guild information to linked players.' +} + +const pageUrl = (externalId) => + `${core.baseUrl}${provider.pageUrlTemplate.replace('{externalId}', externalId)}` + +// Match on abbreviation first, then an exact name, then a unique prefix. Players +// type the abbreviation — it is what appears over a character's head — and a +// wrong-guild answer is worse than "say which one". +function findByName(rows, wanted) { + const needle = wanted.trim().toLowerCase() + const byAbbr = rows.filter((r) => (r.abbr || '').toLowerCase() === needle) + if (byAbbr.length === 1) return { guild: byAbbr[0] } + const exact = rows.filter((r) => r.name.toLowerCase() === needle) + if (exact.length === 1) return { guild: exact[0] } + const partial = rows.filter((r) => r.name.toLowerCase().includes(needle)) + if (partial.length === 1) return { guild: partial[0] } + if (partial.length > 1) return { ambiguous: partial.slice(0, LIST_LIMIT) } + return {} +} + +/** The counts for one guild, from the roster rather than the board's assertions. */ +async function summarise(guild) { + const members = await db.listGuildMembers(guild.id) + const leaders = members + .filter((m) => Number(m.rank) >= db.LEADER_RANK) + .map((m) => m.name) + // The board's founder-leader is folded in as a floor, the same way + // getTeamLeaders does it: it arrives on a different frame, and a shard whose + // roster predates the rank amendment has no other leadership signal. + if (guild.leader_name && !leaders.includes(guild.leader_name)) leaders.push(guild.leader_name) + + return { + // `members`/`online` are the BOARD's counts, which is what the shard asserts; + // the roster is what it enumerated, and the two legitimately disagree for the + // moment between a membership change and the sweep that reports it. The + // assertion is the more current of the two, so it is what is shown. + members: guild.members, + online: guild.online, + linked: members.filter((m) => provider.resolveUserId(m) !== null).length, + leaders, + } +} + +async function detail(guild, actor, audience) { + const counts = await summarise(guild) + const fields = [ + { name: 'Members', value: String(counts.members ?? '—'), inline: true }, + { name: 'Online', value: String(counts.online ?? 0), inline: true }, + { name: 'Linked accounts', value: String(counts.linked), inline: true }, + ] + if (counts.leaders.length) { + fields.push({ name: 'Leaders', value: counts.leaders.join(', ') }) + } + return { + title: guild.abbr ? `${guild.name} [${guild.abbr}]` : guild.name, + text: guild.alliance ? `Alliance: ${guild.alliance}` : undefined, + fields, + url: pageUrl(guild.id), + notice: linkPrompt(actor, audience), + } +} + +/** + * `/guild [name]` — one guild's summary, or the shard's largest guilds. + * + * Never throws for an ordinary miss: "no such guild" and "the shard is offline" + * are answers, and letting either become an exception would turn a routine + * question into "that command failed" with nothing an operator could act on. + */ +async function handler({ options, actor }) { + const config = await visibility.getConfig() + const feature = config.guilds + + // An admin turned guilds off. The switch means "this shard does not publish + // guild data" — over any surface, to anyone, staff included. + if (!feature || !feature.enabled) { + return { text: 'This shard does not publish guild information.', ephemeral: true } + } + + const level = await levelFor(actor) + if (!visibility.meets(level, feature.audience)) { + return { + text: 'Guild information on this shard is not shown to your account.', + ephemeral: true, + notice: linkPrompt(actor, feature.audience), + } + } + + // The provider's own staleness guard, asked before any board read: an + // unreachable sidecar means the board is a snapshot of unknown age, and + // reporting it as current here would contradict what every other surface says. + const ready = await provider.boardIsCurrent() + if (!ready.ok) { + log.info('guild command answered offline', { reason: ready.reason }) + return { text: 'The shard is not connected right now, so guild information may be out of date.', ephemeral: true } + } + + const rows = await db.listGuilds() + if (!rows.length) return { text: 'No guilds are on the board yet.', ephemeral: true } + + const wanted = options && typeof options.name === 'string' ? options.name : null + if (!wanted) { + const top = [...rows].sort((a, b) => (b.members || 0) - (a.members || 0)).slice(0, LIST_LIMIT) + return { + title: `Guilds on ${core.baseUrl.replace(/^https?:\/\//, '')}`, + fields: top.map((g) => ({ + name: g.abbr ? `${g.name} [${g.abbr}]` : g.name, + value: `${g.members || 0} members · ${g.online || 0} online`, + inline: true, + })), + notice: linkPrompt(actor, feature.audience), + } + } + + const { guild, ambiguous } = findByName(rows, wanted) + if (ambiguous) { + return { + text: `Several guilds match “${wanted}”: ${ambiguous.map((g) => g.name).join(', ')}`, + ephemeral: true, + } + } + if (!guild) return { text: `No guild matches “${wanted}”.`, ephemeral: true } + return detail(guild, actor, feature.audience) +} + +module.exports = { + name: 'guild', + description: 'Show a guild on this shard — members, who is online, and its leaders', + options: [ + { name: 'name', type: 'string', description: 'Guild name or abbreviation', required: false }, + ], + // Everyone, deliberately. The gate that matters is the shard's own audience + // rung, resolved inside the handler — `access: 'linked'` would hide the command + // from exactly the unlinked members §9 answer 5 wants to invite to link. + access: 'everyone', + handler, +} diff --git a/server/index.js b/server/index.js index 96c91fc..1421178 100644 --- a/server/index.js +++ b/server/index.js @@ -46,6 +46,7 @@ module.exports = function register(ctx, api) { const shardStreams = require('./config/shardStreams') const townCrierLeg = require('./utils/shardAnnounce') const teamProvider = require('./model/teamProvider/teamProvider.model') + const guildCommand = require('./commands/guild.command') const boot = require('./boot') /* eslint-enable global-require */ @@ -96,6 +97,16 @@ module.exports = function register(ctx, api) { // the database, and registration must not. api.registerTeamProvider(teamProvider) + // `/guild` — the chat surface for the same guilds (MODULE_API 1.6.0, TEAMS.md + // §7.1). The definition travels to the bot; the handler stays here and runs in + // the website process, because the bot container has no `modules` volume and + // cannot load a line of this module's code. + // + // Core registers NO commands of its own. "Guild" is this module's word — core + // does not own it on a page (phase 3) and does not publish it in a channel + // either. + api.registerSlashCommands([guildCommand]) + api.onBoot(boot.onBoot) api.onShutdown(boot.onShutdown) diff --git a/server/model/teamProvider/teamProvider.model.js b/server/model/teamProvider/teamProvider.model.js index 4196520..5e61592 100644 --- a/server/model/teamProvider/teamProvider.model.js +++ b/server/model/teamProvider/teamProvider.model.js @@ -331,4 +331,9 @@ async function projectRoster(externalId, members, viewer) { // data and not a callback. const pageUrlTemplate = '/uo/guilds/{externalId}' -module.exports = { getTeams, getTeamMembers, getTeamLeaders, projectRoster, boardIsCurrent, pageUrlTemplate } +// `resolveUserId` is exported for the `/guild` chat command, which counts linked +// members and must decide "linked" by the same rule the roster does — a second +// copy of that two-source check is a copy that drifts. +module.exports = { + getTeams, getTeamMembers, getTeamLeaders, projectRoster, boardIsCurrent, pageUrlTemplate, resolveUserId, +} diff --git a/server/test/_fakes.js b/server/test/_fakes.js index 8aaee4d..6397e16 100644 --- a/server/test/_fakes.js +++ b/server/test/_fakes.js @@ -96,6 +96,7 @@ function fakeApi() { streams: null, legs: [], teamProvider: null, + slashCommands: [], hooks: {}, } const called = new Set() @@ -112,6 +113,10 @@ function fakeApi() { // deployment — a second registration is a collision there, so it has to be // one here too, or this suite would pass a shape core rejects at load. registerTeamProvider(provider) { once('registerTeamProvider'); record.teamProvider = provider }, + // MODULE_API 1.6.0, live since phase 7. `once` for the same reason core + // takes it: a second call is a module changing its mind halfway through + // register(), which core rejects. + registerSlashCommands(commands) { once('registerSlashCommands'); record.slashCommands = commands }, onBoot(fn) { once('onBoot'); record.hooks.onBoot = fn }, onShutdown(fn) { once('onShutdown'); record.hooks.onShutdown = fn }, } diff --git a/server/test/guildCommand.test.js b/server/test/guildCommand.test.js new file mode 100644 index 0000000..af33659 --- /dev/null +++ b/server/test/guildCommand.test.js @@ -0,0 +1,139 @@ +// `/guild` — the chat command registered through `api.registerSlashCommands` +// (TEAMS.md §7.1, MODULE_API 1.6.0). +// +// The properties worth pinning are all about the ANSWER being the same answer +// the website gives, because that is the whole risk of a second surface: the +// audience rungs are re-resolved here rather than assumed, the shard's own +// offline guard is honoured, and the link prompt appears only when linking would +// actually change what the caller is told. + +const { test, afterEach } = require('node:test') +const assert = require('node:assert/strict') + +const command = require('../commands/guild.command') +const db = require('../model/teamProvider/teamProvider.db') +const provider = require('../model/teamProvider/teamProvider.model') +const visibility = require('../utils/shardVisibility') + +const originals = { + getConfig: visibility.getConfig, + viewerLevel: visibility.viewerLevel, + boardIsCurrent: provider.boardIsCurrent, + listGuilds: db.listGuilds, + listGuildMembers: db.listGuildMembers, +} + +afterEach(() => { + visibility.getConfig = originals.getConfig + visibility.viewerLevel = originals.viewerLevel + provider.boardIsCurrent = originals.boardIsCurrent + db.listGuilds = originals.listGuilds + db.listGuildMembers = originals.listGuildMembers +}) + +const GUILDS = [ + { id: 7, name: 'Knights of the Codex', abbr: 'KOC', alliance: 'The Accord', members: 12, online: 3, leader_name: 'Dain' }, + { id: 9, name: 'Knights Hospitaller', abbr: 'KH', alliance: null, members: 4, online: 0, leader_name: null }, +] + +const MEMBERS = [ + { serial: 1, name: 'Dain', rank: 4, web_id: '31', linked_user_id: null }, + { serial: 2, name: 'Elowen', rank: 4, web_id: null, linked_user_id: 44 }, + { serial: 3, name: 'Wat', rank: 2, web_id: null, linked_user_id: null }, +] + +function stub({ audience = 'anonymous', enabled = true, level = 'anonymous', current = true } = {}) { + visibility.getConfig = async () => ({ guilds: { enabled, audience } }) + visibility.viewerLevel = async () => level + provider.boardIsCurrent = async () => (current ? { ok: true } : { ok: false, reason: 'socket down' }) + db.listGuilds = async () => GUILDS + db.listGuildMembers = async () => MEMBERS +} + +const anonymous = { platform: 'discord', platformUserId: '1', userId: null, role: null, isLinked: false, isStaff: false } +const linked = { platform: 'discord', platformUserId: '2', userId: 31, role: 'player', isLinked: true, isStaff: false } + +test('the definition stays inside the option schema §7.1.1 allows', () => { + assert.equal(command.name, 'guild') + assert.equal(command.access, 'everyone') + for (const option of command.options) { + assert.ok(['string', 'integer', 'boolean', 'user'].includes(option.type)) + assert.ok(option.description.length <= 100) + } +}) + +test('the guilds feature being off withholds everything, staff included', async () => { + stub({ enabled: false, level: 'admin' }) + const res = await command.handler({ options: {}, actor: { ...linked, role: 'admin', isStaff: true } }) + assert.match(res.text, /does not publish guild information/) + assert.equal(res.ephemeral, true) +}) + +// The reason this command is not a thin wrapper over a public route: a rung +// below the feature's audience must be refused HERE, or a shard that gates +// guilds to staff would publish them to a Discord channel. +test('a caller below the feature audience is refused', async () => { + stub({ audience: 'staff', level: 'anonymous' }) + const res = await command.handler({ options: {}, actor: anonymous }) + assert.match(res.text, /not shown to your account/) + assert.equal(res.ephemeral, true) +}) + +test('an unlinked caller is invited to link — but only when linking would change the answer', async () => { + stub({ audience: 'player', level: 'anonymous' }) + const gated = await command.handler({ options: {}, actor: anonymous }) + assert.match(gated.notice, /Link your account/) + + // Public guilds: there is nothing more to see, so there is nothing to prompt. + stub({ audience: 'anonymous', level: 'anonymous' }) + const open = await command.handler({ options: {}, actor: anonymous }) + assert.equal(open.notice, null) +}) + +test('a stale board answers offline rather than reporting what it still holds', async () => { + stub({ current: false }) + const res = await command.handler({ options: {}, actor: anonymous }) + assert.match(res.text, /not connected right now/) +}) + +test('no argument lists the largest guilds', async () => { + stub() + const res = await command.handler({ options: {}, actor: anonymous }) + assert.equal(res.fields.length, 2) + assert.match(res.fields[0].name, /Knights of the Codex/) + assert.match(res.fields[0].value, /12 members · 3 online/) +}) + +test('a name resolves by abbreviation, then exactly, then by unique prefix', async () => { + stub() + const byAbbr = await command.handler({ options: { name: 'koc' }, actor: anonymous }) + assert.match(byAbbr.title, /Knights of the Codex/) + + const exact = await command.handler({ options: { name: 'Knights Hospitaller' }, actor: anonymous }) + assert.match(exact.title, /Hospitaller/) + + // "knights" hits both, and answering with either would be worse than asking. + const ambiguous = await command.handler({ options: { name: 'knights' }, actor: anonymous }) + assert.match(ambiguous.text, /Several guilds match/) + assert.equal(ambiguous.ephemeral, true) +}) + +test('a miss is an answer, not a failure', async () => { + stub() + const res = await command.handler({ options: { name: 'nobody' }, actor: anonymous }) + assert.match(res.text, /No guild matches/) +}) + +// `linked` counts BOTH sources the roster uses — the shard's asserted web id and +// the link table — because that is what "linked" means everywhere else here. +test('the detail carries the counts, the leaders and a link to the module page', async () => { + stub({ level: 'player' }) + const res = await command.handler({ options: { name: 'KOC' }, actor: linked }) + const field = (name) => res.fields.find((f) => f.name === name).value + assert.equal(field('Members'), '12') + assert.equal(field('Online'), '3') + assert.equal(field('Linked accounts'), '2') + assert.equal(field('Leaders'), 'Dain, Elowen') + assert.match(res.url, /\/uo\/guilds\/7$/) + assert.equal(res.notice, null) +}) -- 2.49.1 From 466842c6f27fcbcb1fcab8dce833ceb3f9b69ad1 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 18 Aug 2026 19:08:22 -0500 Subject: [PATCH 2/2] fix(guilds): do not offer linking where linking cannot reach MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found on the live rig, with the shard's guild feature gated to staff: the refusal still read "link your account — this shard shows guild information to linked players". Signing in reaches `logged_in` and linking a game account reaches `player`; `staff` and `admin` are roles an operator grants, and no amount of linking earns them. Inviting someone to do something that changes nothing is worse than plainly saying no. Also drops the host name from the list embed's title. `ctx.site` carries a base URL and no brand name, so naming the deployment there could only ever mean printing its hostname into a title on the shard's own Discord server. Co-Authored-By: Claude --- server/commands/guild.command.js | 24 ++++++++++++++++++------ server/test/guildCommand.test.js | 9 +++++++++ 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/server/commands/guild.command.js b/server/commands/guild.command.js index 2958a56..9908016 100644 --- a/server/commands/guild.command.js +++ b/server/commands/guild.command.js @@ -42,13 +42,22 @@ async function levelFor(actor) { return visibility.viewerLevel({ user: { id: actor.userId, role: actor.role } }) } -// The nudge §9 answer 5 asks for, and only when it is TRUE. An unlinked caller -// who was told nothing because the shard publishes nothing is not helped by -// being invited to link; the prompt appears when linking is what would actually -// change the answer. +// The nudge §9 answer 5 asks for, and only when it is TRUE. +// +// **Linking reaches exactly two rungs and no further.** Signing in gets a caller +// to `logged_in` and linking a game account to `player`; `staff` and `admin` are +// roles an operator grants and no amount of linking will earn. So a shard that +// gates guilds to staff refuses an unlinked caller WITHOUT the invitation — +// telling them to link would be telling them to do something that changes +// nothing, which is worse than saying no. +// +// The live walk found this: gated to `staff`, the refusal still read "this shard +// shows guild information to linked players". +const LINKING_REACHES = new Set(['logged_in', 'player']) + function linkPrompt(actor, audience) { if (actor.isLinked) return null - if (audience === 'anonymous') return null + if (!LINKING_REACHES.has(audience)) return null return 'Link your account on the site to see more — this shard shows guild information to linked players.' } @@ -154,7 +163,10 @@ async function handler({ options, actor }) { if (!wanted) { const top = [...rows].sort((a, b) => (b.members || 0) - (a.members || 0)).slice(0, LIST_LIMIT) return { - title: `Guilds on ${core.baseUrl.replace(/^https?:\/\//, '')}`, + // Not "Guilds on ": `ctx.site` carries a base URL and no brand name, + // so naming the deployment here can only mean printing its hostname into + // an embed title, which is noise on a shard's own Discord server. + title: 'Guilds on this shard', fields: top.map((g) => ({ name: g.abbr ? `${g.name} [${g.abbr}]` : g.name, value: `${g.members || 0} members · ${g.online || 0} online`, diff --git a/server/test/guildCommand.test.js b/server/test/guildCommand.test.js index af33659..7782632 100644 --- a/server/test/guildCommand.test.js +++ b/server/test/guildCommand.test.js @@ -88,6 +88,14 @@ test('an unlinked caller is invited to link — but only when linking would chan stub({ audience: 'anonymous', level: 'anonymous' }) const open = await command.handler({ options: {}, actor: anonymous }) assert.equal(open.notice, null) + + // Gated to staff: linking reaches `player` and stops there, so the invitation + // would be an instruction to do something that changes nothing. Found on the + // live rig, where a staff-gated shard still offered it. + stub({ audience: 'staff', level: 'anonymous' }) + const unreachable = await command.handler({ options: {}, actor: anonymous }) + assert.match(unreachable.text, /not shown to your account/) + assert.equal(unreachable.notice, null) }) test('a stale board answers offline rather than reporting what it still holds', async () => { @@ -99,6 +107,7 @@ test('a stale board answers offline rather than reporting what it still holds', test('no argument lists the largest guilds', async () => { stub() const res = await command.handler({ options: {}, actor: anonymous }) + assert.equal(res.title, 'Guilds on this shard') assert.equal(res.fields.length, 2) assert.match(res.fields[0].name, /Knights of the Codex/) assert.match(res.fields[0].value, /12 members · 3 online/) -- 2.49.1