feat(teams): phase 8 — the notifications bridge, and the gate §7.2 could not check
All checks were successful
PR Checks / client-build (pull_request) Successful in 31s
PR Checks / bot-tests (pull_request) Successful in 33s
PR Checks / server-tests (pull_request) Successful in 10m49s

The same Team event as §6, delivered a third time: push, email, and now a
Discord channel the operator configured. Not a second pipeline — teamNotify.js
already computed the recipient set once, so the bridge is a sink beside the two
that were there.

The design's gate has no data source. §7.2 bridges an event only if "its
visibility is public, or its destination channel is configured for a
members-only Team context". The four team.* streams carry no visibility; forum
threads have no public/members column because a forum is members-only by
construction; and core cannot see a Discord channel's permissions. So §7.2's own
example config names exactly the two events that are never public.

The gate is therefore an attributed operator acknowledgement, in the shape
teams_forum_uploads_ack already uses. It is a precondition — 422, not a quiet
drop at delivery — it is re-asked at delivery as well as at the save, and
changing the channel clears it, because an acknowledgement is about a
destination and cannot survive the destination changing underneath it.

The design's DDL cannot hold its own default row: MariaDB coerces every PRIMARY
KEY column to NOT NULL, so `team_id NULL` — the deployment-wide default every
override overrides — is unrepresentable. Proved on a real MariaDB (error 1048).
Replaced with a surrogate id, a generated team_key AS IFNULL(team_id, 0) in the
unique key, and the foreign key the original had no room for.

One-shot, not queued: "identical to announce and mod-reverse" names two
different reliability models, and a Team notification is the moment it
describes.

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
2026-08-18 20:25:30 -05:00
parent 46f43a5fd6
commit 11b4368b57
25 changed files with 2567 additions and 11 deletions

View File

@@ -0,0 +1,122 @@
// SQL for the integration bridge's configuration (TEAMS.md §7.2, phase 8).
//
// One table, and almost all of its subtlety is in the schema comment rather than
// here: `team_key` is a generated `IFNULL(team_id, 0)`, so the deployment-wide
// default and the per-Team overrides live under one UNIQUE key without the
// default row needing a NULL in a primary key it cannot have.
//
// **Reads join `teams` and callers get the Team's name.** Not for display alone:
// the resolver's answer is the input to a message that names a Team, and a second
// round trip per notification to fetch a name the first query already walked past
// is the kind of thing that only shows up under a busy forum.
const { query } = require('../../utils/db')
const COLUMNS = `
c.id, c.platform, c.team_id, c.events, c.channel_ref, c.enabled,
c.members_ack, c.members_ack_by, c.members_ack_at, c.updated_at`
/**
* Every row for a platform — the default first, then the overrides by Team name.
*
* The admin panel's whole listing, in one query. `team_name` is NULL on exactly
* one row (the default), which is also how the client tells them apart without
* needing to reason about `team_id`.
*/
async function listForPlatform(platform) {
return query(
`SELECT ${COLUMNS}, t.name AS team_name, t.slug AS team_slug, t.display_name_override,
u.username AS members_ack_username
FROM team_integration_config c
LEFT JOIN teams t ON t.id = c.team_id
LEFT JOIN users u ON u.id = c.members_ack_by
WHERE c.platform = ?
ORDER BY c.team_id IS NOT NULL, COALESCE(t.name, '')`,
[platform],
)
}
/**
* The row that governs `teamId`, or null.
*
* `team_key` is what makes this one query rather than two: asking for the pair
* (0, teamId) returns the default and the override together, and `ORDER BY
* team_key DESC LIMIT 1` puts the override first when it exists. A caller that
* fetched the default and then looked for an override would do two round trips
* per notification for an answer the index already holds.
*/
async function resolveFor(platform, teamId) {
const rows = await query(
`SELECT ${COLUMNS}, t.name AS team_name, t.display_name_override
FROM team_integration_config c
LEFT JOIN teams t ON t.id = c.team_id
WHERE c.platform = ? AND c.team_key IN (0, ?)
ORDER BY c.team_key DESC
LIMIT 1`,
[platform, Number(teamId)],
)
return rows[0] || null
}
async function getById(id) {
const rows = await query(
`SELECT ${COLUMNS}, t.name AS team_name FROM team_integration_config c
LEFT JOIN teams t ON t.id = c.team_id
WHERE c.id = ? LIMIT 1`,
[Number(id)],
)
return rows[0] || null
}
async function getForTeam(platform, teamId) {
const rows = await query(
`SELECT ${COLUMNS} FROM team_integration_config c
WHERE c.platform = ? AND c.team_key = ? LIMIT 1`,
[platform, teamId === null || teamId === undefined ? 0 : Number(teamId)],
)
return rows[0] || null
}
/**
* Create or replace the row for (platform, team).
*
* A full replace rather than a patch, and the acknowledgement columns are part of
* what is replaced — the model decides what they should be, because "did the
* channel change" is a comparison against the row that is about to be overwritten
* and only the model has both halves.
*/
async function upsert({ platform, teamId, events, channelRef, enabled, membersAck, membersAckBy, membersAckAt }) {
await query(
`INSERT INTO team_integration_config
(platform, team_id, events, channel_ref, enabled, members_ack, members_ack_by, members_ack_at)
VALUES (?, ?, ?, ?, ?, ?, ?, ?)
ON DUPLICATE KEY UPDATE
events = VALUES(events),
channel_ref = VALUES(channel_ref),
enabled = VALUES(enabled),
members_ack = VALUES(members_ack),
members_ack_by = VALUES(members_ack_by),
members_ack_at = VALUES(members_ack_at)`,
[
platform,
teamId === null || teamId === undefined ? null : Number(teamId),
JSON.stringify(events || []),
channelRef || null,
enabled ? 1 : 0,
membersAck ? 1 : 0,
membersAckBy || null,
membersAckAt || null,
],
)
return getForTeam(platform, teamId)
}
async function remove(platform, teamId) {
const res = await query('DELETE FROM team_integration_config WHERE platform = ? AND team_key = ?', [
platform,
teamId === null || teamId === undefined ? 0 : Number(teamId),
])
return Number(res && res.affectedRows) || 0
}
module.exports = { listForPlatform, resolveFor, getById, getForTeam, upsert, remove }

View File

@@ -0,0 +1,272 @@
// ── The integration bridge's configuration and its one precondition ────────
//
// TEAMS.md §7.2, phase 8. An operator says "send these Team events to this
// Discord channel", globally or for one Team, and this file is where that
// sentence is validated, stored and resolved.
//
// **Two of the four streams can never be public, and that is the whole reason
// this file is more than a settings row.** §7.2 gates bridging on "the event's
// visibility is public, or the destination channel is configured for a
// members-only Team context". Neither half exists in the tree and neither can:
// the four `team.*` streams carry no visibility (only `team_activity` rows do,
// and a notification is not an activity row), forum threads have no public/
// members column because a forum is members-only by construction — everything in
// it sits behind `team_forum_grants` — and core cannot see a Discord channel's
// permissions to know what it is.
//
// Only the operator can see that. So the gate becomes an ATTRIBUTED
// ACKNOWLEDGEMENT: enabling a members-only event requires an explicit tick that
// the destination is restricted to that Team's members, recorded with who gave it
// and when, in the same shape `teams_forum_uploads_ack` records the image-policy
// one. It is a precondition, not a preference — `assertEnableable` refuses the
// save rather than quietly dropping the event at delivery time, because a config
// that silently does less than it says is worse than one that will not save.
//
// **Changing the channel clears the acknowledgement.** An acknowledgement is
// about a destination; it cannot survive the destination changing underneath it,
// or an operator would tick "this channel is private", then repoint the row at a
// public one and keep the permission they were granted for a different place.
//
// **Every read fails closed**, like `teamForumSettings`: a DB fault reports no
// bridge configured, because the cost of failing closed is a Discord channel that
// stays quiet for a minute and the cost of failing open is members-only text in a
// room the operator never approved.
const db = require('./teamIntegration.db')
const log = require('../../utils/logger')('team-integration')
// The only platform phase 8 knows. Deliberately a value rather than a hardcoded
// literal at every call site: phase 10 turns this into a lookup against the
// declared-capability registry, and the fewer places that spell 'discord' the
// smaller that change is.
const DISCORD = 'discord'
const PLATFORMS = [DISCORD]
// The four §6.2 streams, and which of them can reach a channel core cannot vet.
//
// A stream is members-only if the CONTENT behind it is: `team.forum.post` and
// `team.announcement` both name a thread nobody outside the Team may read. The
// roster pair is public — Team pages and rosters are public by §1's projection
// rules — so bridging those asserts nothing and needs no tick.
const BRIDGEABLE = [
'team.member.joined',
'team.leadership.changed',
'team.forum.post',
'team.announcement',
]
const MEMBERS_ONLY = new Set(['team.forum.post', 'team.announcement'])
// Discord snowflakes are 17-20 digits today and the format is not promised. The
// check is only that a channel ref is plausibly one and cannot smuggle anything —
// core treats it as opaque and the bot is what resolves it.
const CHANNEL_RE = /^[0-9]{5,32}$/
const isMembersOnly = (streamId) => MEMBERS_ONLY.has(streamId)
/** Does this event list contain anything that would publish members-only text? */
const needsAck = (events) => (events || []).some(isMembersOnly)
/**
* Normalise an operator-supplied event list.
*
* Unknown ids are REJECTED rather than dropped. A silently-dropped event is a
* config screen that shows you saved something you did not, and the set is small
* and fixed enough that a typo is a mistake worth reporting.
*/
function normaliseEvents(events) {
if (!Array.isArray(events)) {
const err = new Error('events must be an array')
err.status = 400
throw err
}
const seen = []
for (const raw of events) {
const id = String(raw || '').trim()
if (!BRIDGEABLE.includes(id)) {
const err = new Error(`unknown event: ${id}`)
err.status = 400
throw err
}
if (!seen.includes(id)) seen.push(id)
}
return seen
}
function normaliseChannel(channelRef) {
const value = String(channelRef || '').trim()
if (!value) return null
if (!CHANNEL_RE.test(value)) {
const err = new Error('channel must be a numeric channel id')
err.status = 400
throw err
}
return value
}
/**
* The gate, as a throw.
*
* Order matters to the message an operator reads: an enabled row with no channel
* is a different mistake from one with an unacknowledged channel, and reporting
* the second when the first is true would send them to tick a box that would not
* have helped.
*/
function assertEnableable({ enabled, events, channelRef, membersAck }) {
if (!enabled) return
if (!channelRef) {
const err = new Error('a destination channel is required to enable this bridge')
err.status = 422
throw err
}
if (events.length === 0) {
const err = new Error('at least one event is required to enable this bridge')
err.status = 422
throw err
}
if (needsAck(events) && !membersAck) {
const err = new Error(
'forum posts and announcements are visible only to a Teams members — confirm the destination channel is restricted to them before enabling',
)
err.status = 422
err.code = 'members_ack_required'
throw err
}
}
/** Rows for the admin panel, `events` already parsed. */
async function list(platform = DISCORD) {
const rows = await db.listForPlatform(platform)
return rows.map(shape)
}
/**
* Parse the stored JSON once, here.
*
* `mariadb` hands a JSON column back as a string on some server versions and as a
* parsed value on others, which is a difference nobody wants to rediscover in a
* controller. Anything unreadable becomes an empty list rather than a throw: a
* row with a corrupt event list should render as a row that bridges nothing, not
* take the whole admin page down.
*/
function shape(row) {
if (!row) return null
let events = row.events
if (typeof events === 'string') {
try {
events = JSON.parse(events)
} catch {
events = []
}
}
return { ...row, events: Array.isArray(events) ? events : [], enabled: !!row.enabled, members_ack: !!row.members_ack }
}
/**
* The row that governs `teamId` — the override if there is one, otherwise the
* deployment default — filtered down to what may actually be delivered.
*
* **The acknowledgement is checked HERE as well as at the save.** A row saved
* with the tick can lose it later: an admin repoints the channel, or a future
* change to what counts as members-only reclassifies a stream a row already
* carries. Re-asking at delivery is what makes the tick a live property of the
* row rather than a note about a save that happened once.
*/
async function resolve(teamId, platform = DISCORD) {
try {
const row = shape(await db.resolveFor(platform, teamId))
if (!row || !row.enabled || !row.channel_ref) return null
const events = row.events.filter((id) => (isMembersOnly(id) ? row.members_ack : true))
if (events.length === 0) return null
return { ...row, events }
} catch (err) {
log.warn('bridge config lookup failed — treating as unconfigured', {
teamId,
platform,
message: err.message,
})
return null
}
}
/** Is `streamId` bridged for this Team? The delivery path's whole question. */
async function destinationFor(teamId, streamId, platform = DISCORD) {
const row = await resolve(teamId, platform)
if (!row || !row.events.includes(streamId)) return null
return { channelRef: row.channel_ref, membersOnly: isMembersOnly(streamId), platform }
}
/**
* Create or replace the row for (platform, team).
*
* `actorId` is the admin doing the saving, and it is what lands in
* `members_ack_by` — the acknowledgement names a person, so it cannot be written
* by a path that does not know who they are.
*/
async function save({ platform = DISCORD, teamId = null, events, channelRef, enabled, membersAck }, actorId) {
if (!PLATFORMS.includes(platform)) {
const err = new Error(`unknown platform: ${platform}`)
err.status = 400
throw err
}
const nextEvents = normaliseEvents(events)
const nextChannel = normaliseChannel(channelRef)
const existing = shape(await db.getForTeam(platform, teamId))
// An acknowledgement survives an ordinary edit and dies with the channel it was
// given for. `membersAck === false` from the client is an explicit withdrawal
// and is honoured; `undefined` means "leave it", which is what a save that only
// toggled an event should do.
const channelChanged = !!existing && existing.channel_ref !== nextChannel
let ack = existing ? existing.members_ack : false
if (membersAck === false) ack = false
else if (membersAck === true) ack = true
if (channelChanged) ack = membersAck === true
const nextEnabled = !!enabled
assertEnableable({ enabled: nextEnabled, events: nextEvents, channelRef: nextChannel, membersAck: ack })
// Re-stamp only when the acknowledgement is newly given, so an unrelated save
// does not rewrite the date on a decision nobody revisited.
//
// `channelChanged` belongs in this condition and it is easy to leave out: an
// acknowledgement given alongside a NEW channel is a new acknowledgement even
// though the column was already 1, and without it the row keeps naming whoever
// vetted the PREVIOUS destination. That attribution is the whole audit value of
// the column — it has to name the person who looked at the channel the row now
// points at.
const freshlyAcked = ack && (channelChanged || !(existing && existing.members_ack))
const row = await db.upsert({
platform,
teamId,
events: nextEvents,
channelRef: nextChannel,
enabled: nextEnabled,
membersAck: ack,
membersAckBy: ack ? (freshlyAcked ? actorId : existing.members_ack_by) : null,
membersAckAt: ack ? (freshlyAcked ? new Date() : existing.members_ack_at) : null,
})
return shape(row)
}
async function remove(platform, teamId) {
return db.remove(platform, teamId)
}
module.exports = {
DISCORD,
PLATFORMS,
BRIDGEABLE,
MEMBERS_ONLY,
isMembersOnly,
needsAck,
normaliseEvents,
normaliseChannel,
assertEnableable,
list,
resolve,
destinationFor,
save,
remove,
}

View File

@@ -213,7 +213,10 @@ async function logRosterActivity(team, { joined, left, promoted, demoted }) {
async function notifyRoster(team, { joined, promoted, demoted }) {
if (!team.roster_synced_at) return
try {
if (joined.length > 0) await teamNotify.memberJoined(team)
// The count rides along for the Discord bridge (§7.2), which has no app on
// the other end to pull the roster after a content-free nudge. The tickle
// itself is unchanged and still carries nothing.
if (joined.length > 0) await teamNotify.memberJoined(team, { count: joined.length })
if (promoted.length > 0 || demoted.length > 0) await teamNotify.leadershipChanged(team)
} catch (err) {
log.warn('roster notification not sent', { teamId: team.id, message: err.message })