feat(modules): registerTeamProvider, and a call path that cannot answer "empty"

The registration a module uses to become the authoritative source of Teams (docs/website/TEAMS.md §2.3), plus the wrapper core calls it through. registerTeamProvider is the first registration where core CALLS THE MODULE and waits for an answer. Every existing one is either the module claiming a mount or core notifying it; the closest precedent is registerAnnounceLeg's dispatch, and this is modelled on it rather than invented. It also holds a single value rather than a map, unlike every other registry: Teams have one authoritative source by construction, and two modules answering "what teams exist" would produce two disjoint sets under one `teams` table with no rule for merging them. A second registration is therefore a collision, named against the module that holds it. teamProvider.js is where invariant 1 -- module unavailability is staleness, never emptiness -- is actually enforced. It is deliberately generous about what counts as a failure: a rejected promise, a synchronous throw, a timeout, a non-object, a bare array, a missing `ok`, or a structurally malformed row all leave as the same `{ ok: false }` a module would have sent on purpose. There is no shape a broken provider can produce that arrives at the reconciler looking like an authoritative empty list -- which is the entire argument for the envelope, since a bare array has exactly one such shape and it is the one a module returns while its sidecar is still connecting. A malformed row fails the whole call rather than being dropped. Salvaging is the dangerous option: one unreadable member quietly omitted from a roster is indistinguishable, downstream, from that member having left, and the sync would mark them departed on the strength of a broken payload. Refusing costs one stale interval. The deadline timer is unreffed as well as cleared. Clearing covers the case where the race settles; it cannot cover a module promise that never settles at all, where nothing exists to clear until the deadline fires. Caught by the test file taking 10.2s to run 265ms of assertions -- the same class of bug as the mariadb pool that used to hold the suite open (test/_setup.js). 292ms now. 28 tests. Full suite 770 passed, 0 failed. Refs docs/website/TEAMS.md §2.3, Part 12 phase 2 Co-Authored-By: Claude <noreply@anthropic.com>
2026-08-17 14:44:45 -05:00
parent 225663d62e
commit 8b63ffc725
4 changed files with 536 additions and 2 deletions
--- a/server/test/teamProvider.test.js
+++ b/server/test/teamProvider.test.js
@@ -0,0 +1,289 @@
+// The Team provider registration and the guarded call path
+// (docs/website/TEAMS.md §2.3).
+//
+// Almost every test here is invariant 1 asked a different way: **module
+// unavailability is staleness, never emptiness.** The value of this file is that
+// it enumerates the shapes a broken provider can produce and asserts that none of
+// them arrives at the reconciler looking like authoritative data.
+const { test, beforeEach, afterEach } = require('node:test')
+const assert = require('node:assert/strict')
+
+const registries = require('../src/modules/registries')
+const teamProvider = require('../src/model/teams/teamProvider')
+
+// Register a provider the way a module does: stage, then commit.
+function register(owner, provider) {
+  const api = registries.stage(owner)
+  api.registerTeamProvider(provider)
+  registries.apply(api.staged)
+}
+
+const ok = () => ({
+  getTeams: async () => ({ ok: true, teams: [{ externalId: 'g1', name: 'The Silver Hand' }] }),
+  getTeamMembers: async () => ({ ok: true, members: [{ memberKey: '0x1', displayName: 'Aldric' }] }),
+  getTeamLeaders: async () => ({ ok: true, leaders: ['0x1'] }),
+})
+
+beforeEach(() => registries._reset())
+afterEach(() => registries._reset())
+
+// ── Registration ───────────────────────────────────────────────────────────
+
+test('a provider is readable only after apply(), not at stage time', () => {
+  const api = registries.stage('uo')
+  api.registerTeamProvider(ok())
+  assert.equal(registries.hasTeamProvider(), false, 'staging must not publish')
+
+  registries.apply(api.staged)
+  assert.equal(registries.hasTeamProvider(), true)
+  assert.equal(registries.registeredTeamProvider().owner, 'uo')
+})
+
+test('all three methods are required', () => {
+  const api = registries.stage('uo')
+  for (const missing of ['getTeams', 'getTeamMembers', 'getTeamLeaders']) {
+    const provider = ok()
+    delete provider[missing]
+    assert.throws(() => api.registerTeamProvider(provider), new RegExp(`${missing}\\(\\) is missing`))
+  }
+  // A non-function is the same failure, and is the likelier typo.
+  assert.throws(() => api.registerTeamProvider({ ...ok(), getTeams: 'yes' }), /getTeams\(\) is missing or not a function/)
+})
+
+test('a second provider is a collision naming the module that holds it', () => {
+  register('uo', ok())
+  const second = registries.stage('other')
+  second.registerTeamProvider(ok())
+  assert.throws(() => registries.apply(second.staged), /already registered by "uo"/)
+  // The first registration is untouched by the rejected second.
+  assert.equal(registries.registeredTeamProvider().owner, 'uo')
+})
+
+test('one module registering twice in one batch is rejected', () => {
+  const api = registries.stage('uo')
+  api.registerTeamProvider(ok())
+  api.registerTeamProvider(ok())
+  assert.throws(() => registries.apply(api.staged), /more than one team provider/)
+  assert.equal(registries.hasTeamProvider(), false, 'the whole batch is refused')
+})
+
+test('a rejected batch leaves no provider behind, even when its other claims are fine', () => {
+  const api = registries.stage('uo')
+  api.registerTeamProvider(ok())
+  api.registerNotificationStreams([{ id: 'uo.thing', label: 'Thing' }])
+  api.registerNotificationStreams([{ id: 'uo.thing', label: 'Thing' }]) // duplicate
+  assert.throws(() => registries.apply(api.staged))
+  assert.equal(registries.hasTeamProvider(), false, 'validate-then-commit covers the provider too')
+})
+
+// ── The call path: every failure shape becomes { ok: false } ───────────────
+
+test('no registered provider is a refusal, not an empty answer', async () => {
+  const answer = await teamProvider.getTeams()
+  assert.equal(answer.ok, false)
+  assert.equal(answer.teams, undefined, 'a refusal never carries a teams array')
+  assert.equal(teamProvider.providerModuleId(), null)
+})
+
+test('a provider that throws is a refusal', async () => {
+  register('uo', { ...ok(), getTeams: async () => { throw new Error('sidecar unreachable') } })
+  const answer = await teamProvider.getTeams()
+  assert.equal(answer.ok, false)
+  assert.match(answer.reason, /sidecar unreachable/)
+  assert.equal(answer.teams, undefined)
+})
+
+test('a provider that throws SYNCHRONOUSLY is a refusal too', async () => {
+  register('uo', { ...ok(), getTeams: () => { throw new Error('boom') } })
+  const answer = await teamProvider.getTeams()
+  assert.equal(answer.ok, false)
+  assert.match(answer.reason, /boom/)
+})
+
+test('a deliberate { ok: false } keeps its reason for team_sync_state', async () => {
+  register('uo', { ...ok(), getTeams: async () => ({ ok: false, reason: 'cache cold' }) })
+  assert.deepEqual(await teamProvider.getTeams(), { ok: false, reason: 'cache cold' })
+})
+
+test('a missing ok field is not read as authority', async () => {
+  register('uo', { ...ok(), getTeams: async () => ({ teams: [] }) })
+  const answer = await teamProvider.getTeams()
+  assert.equal(answer.ok, false, 'a forgotten field must not become an authoritative empty list')
+})
+
+test('a bare array — the shape the envelope exists to outlaw — is a refusal', async () => {
+  register('uo', { ...ok(), getTeams: async () => [] })
+  const answer = await teamProvider.getTeams()
+  assert.equal(answer.ok, false)
+  assert.match(answer.reason, /not an envelope/)
+})
+
+test('null, undefined and a string are all refusals', async () => {
+  for (const bad of [null, undefined, 'ok', 42]) {
+    register('uo', { ...ok(), getTeams: async () => bad })
+    // eslint-disable-next-line no-await-in-loop
+    assert.equal((await teamProvider.getTeams()).ok, false, `${String(bad)} must not be authoritative`)
+    registries._reset()
+  }
+})
+
+test('ok:true with no teams array is a refusal, not zero teams', async () => {
+  register('uo', { ...ok(), getTeams: async () => ({ ok: true }) })
+  const answer = await teamProvider.getTeams()
+  assert.equal(answer.ok, false)
+  assert.match(answer.reason, /no teams array/)
+})
+
+test('an ok answer with a genuinely empty list stays ok — §2.4 decides what to do with it', async () => {
+  register('uo', { ...ok(), getTeams: async () => ({ ok: true, teams: [] }) })
+  const answer = await teamProvider.getTeams()
+  assert.equal(answer.ok, true, 'this file must not second-guess an authoritative empty answer')
+  assert.deepEqual(answer.teams, [])
+})
+
+// ── Malformed rows fail the call rather than being salvaged ────────────────
+
+test('a team with no externalId fails the whole call', async () => {
+  register('uo', { ...ok(), getTeams: async () => ({ ok: true, teams: [{ name: 'Nameless' }] }) })
+  const answer = await teamProvider.getTeams()
+  assert.equal(answer.ok, false)
+  assert.match(answer.reason, /no externalId/)
+})
+
+test('a team with no name fails the whole call', async () => {
+  register('uo', { ...ok(), getTeams: async () => ({ ok: true, teams: [{ externalId: 'g1', name: '  ' }] }) })
+  assert.match((await teamProvider.getTeams()).reason, /"g1" has no name/)
+})
+
+test('one unreadable member refuses the roster rather than dropping the member', async () => {
+  // Dropping it would be indistinguishable, downstream, from the member leaving —
+  // the sync would mark them departed on the strength of a malformed payload.
+  register('uo', {
+    ...ok(),
+    getTeamMembers: async () => ({ ok: true, members: [{ memberKey: '0x1' }, { displayName: 'ghost' }] }),
+  })
+  const answer = await teamProvider.getTeamMembers('g1')
+  assert.equal(answer.ok, false)
+  assert.equal(answer.members, undefined)
+})
+
+test('a duplicated memberKey is refused rather than collapsed', async () => {
+  register('uo', {
+    ...ok(),
+    getTeamMembers: async () => ({ ok: true, members: [{ memberKey: '0x1' }, { memberKey: '0x1' }] }),
+  })
+  assert.match((await teamProvider.getTeamMembers('g1')).reason, /appears twice/)
+})
+
+// ── Normalisation of a good answer ─────────────────────────────────────────
+
+test('team fields are trimmed, and meta is passed through opaquely', async () => {
+  register('uo', {
+    ...ok(),
+    getTeams: async () => ({
+      ok: true,
+      teams: [{ externalId: ' g1 ', name: ' The Silver Hand ', abbr: ' TSH ', meta: { crest: 7 } }],
+    }),
+  })
+  const { teams } = await teamProvider.getTeams()
+  assert.deepEqual(teams, [{ externalId: 'g1', name: 'The Silver Hand', abbr: 'TSH', meta: { crest: 7 } }])
+})
+
+test('a non-object meta is dropped rather than stored as a scalar', async () => {
+  register('uo', {
+    ...ok(),
+    getTeams: async () => ({ ok: true, teams: [{ externalId: 'g1', name: 'X', meta: 'crest' }] }),
+  })
+  assert.equal((await teamProvider.getTeams()).teams[0].meta, null)
+})
+
+test('member booleans are coerced and userId is accepted only as a positive integer', async () => {
+  register('uo', {
+    ...ok(),
+    getTeamMembers: async () => ({
+      ok: true,
+      members: [
+        { memberKey: '0x1', displayName: 'Aldric', rankLabel: 'Warlord', leader: 1, online: 'yes', userId: 7 },
+        { memberKey: '0x2', userId: 0 },
+        { memberKey: '0x3', userId: '7' },
+        { memberKey: '0x4', userId: 1.5 },
+      ],
+    }),
+  })
+  const { members } = await teamProvider.getTeamMembers('g1')
+  assert.equal(members[0].leader, true)
+  assert.equal(members[0].online, true)
+  assert.equal(members[0].userId, 7)
+  assert.equal(members[1].userId, null, '0 is not a user id')
+  assert.equal(members[2].userId, null, 'a numeric string is not a resolved link')
+  assert.equal(members[3].userId, null)
+  // Absent optional fields become null rather than undefined, so a column write
+  // does not depend on the module having spelled the key.
+  assert.equal(members[1].displayName, null)
+  assert.equal(members[1].rankLabel, null)
+})
+
+test('complete defaults to true and is honoured when false', async () => {
+  register('uo', ok())
+  assert.equal((await teamProvider.getTeams()).complete, true)
+  registries._reset()
+
+  register('uo', { ...ok(), getTeams: async () => ({ ok: true, complete: false, teams: [] }) })
+  assert.equal((await teamProvider.getTeams()).complete, false)
+})
+
+test('duplicate leaders are collapsed and blanks refused', async () => {
+  register('uo', { ...ok(), getTeamLeaders: async () => ({ ok: true, leaders: ['0x1', '0x1', ' 0x2 '] }) })
+  assert.deepEqual((await teamProvider.getTeamLeaders('g1')).leaders, ['0x1', '0x2'])
+  registries._reset()
+
+  register('uo', { ...ok(), getTeamLeaders: async () => ({ ok: true, leaders: ['0x1', ''] }) })
+  assert.equal((await teamProvider.getTeamLeaders('g1')).ok, false)
+})
+
+test('the external id is passed through to the module unchanged', async () => {
+  const seen = []
+  register('uo', { ...ok(), getTeamMembers: async (id) => { seen.push(id); return { ok: true, members: [] } } })
+  await teamProvider.getTeamMembers('g-42')
+  assert.deepEqual(seen, ['g-42'])
+})
+
+test('providerModuleId names the registrant, which is what sync state is keyed on', async () => {
+  register('uo', ok())
+  assert.equal(teamProvider.providerModuleId(), 'uo')
+})
+
+// ── The timeout ────────────────────────────────────────────────────────────
+
+test('a provider that never answers becomes a refusal at the deadline', async (t) => {
+  // Mocked timers rather than a real ten-second wait: this exercises the
+  // production path exactly — the same setTimeout, the same deadline — without
+  // putting ten seconds into every CI run.
+  t.mock.timers.enable({ apis: ['setTimeout'] })
+  register('uo', { ...ok(), getTeams: () => new Promise(() => {}) })
+
+  const pending = teamProvider.getTeams()
+  t.mock.timers.tick(teamProvider.CALL_TIMEOUT_MS)
+
+  const answer = await pending
+  assert.equal(answer.ok, false)
+  assert.match(answer.reason, /did not answer within 10000ms/)
+  assert.equal(answer.teams, undefined, 'a hung module never produces data')
+})
+
+test('a hung call does not hold the process open until its deadline', async () => {
+  // The timer is unreffed, so a call left pending at shutdown cannot keep the
+  // event loop alive. Asserted directly, because the symptom — a test FILE that
+  // passes in milliseconds and then sits for ten seconds — is invisible in a
+  // green summary.
+  register('uo', { ...ok(), getTeams: () => new Promise(() => {}) })
+  const before = process.getActiveResourcesInfo().filter((r) => r === 'Timeout').length
+  teamProvider.getTeams()
+  await new Promise((resolve) => { setImmediate(resolve) })
+  const after = process.getActiveResourcesInfo().filter((r) => r === 'Timeout').length
+  assert.equal(after, before, 'the deadline timer must not count as an active resource')
+})
+
+test('the budget is the documented ten seconds', () => {
+  assert.equal(teamProvider.CALL_TIMEOUT_MS, 10_000)
+})