190 Commits

Author	SHA1	Message	Date
wtclaude	5779d15150	feat(engagement): retention — three sweeps and one recorded refusal ... ENGAGEMENT.md Phase 14, the last phase of the workstream. Four engagement tables grew on every fire and nothing had ever deleted from any of them. Three of them now have a horizon, swept nightly by one worker (utils/engagementRetentionPrune.js — setInterval + unref + stop(), batched 1000 x 50, each table's failure caught on its own so a lock timeout on one does not leave the other two unbounded): engagement_sends 180 days engagement_sends_retain_days (7-3650) engagement_cooldowns 30 days engagement_cooldowns_retain_days (2-3650) engagement_outbox 30 days engagement_outbox_retain_days (2-3650) The fourth, engagement_suppressions, does not expire, and that is the recorded decision rather than an omission: a suppression is a standing decision, and ageing out a hard bounce re-mails an address that already bounced. The way out stays deliberate, and is now reachable per row. Six decisions were settled by the org lead before any code. Two of them widened the phase past what was offered: * the send-log horizon is admin-configurable, so retention got a SCREEN (Admin -> Engagement -> Retention) where team_activity and user_notifications keep theirs in invisible settings rows. The send-log horizon changes what an operator-facing page is able to show, so it has to be visible; the other two came with it, because "what does this deployment keep" is one question. * the suppression purge, which cost a Phase 9 decision. The list deliberately stripped address_hash from every row, so the only way out was a window.prompt asking the operator to retype an address the screen has never shown them. The row had no handle at all. The hash is now returned: this route is admin-only and an admin can already suppress and unsuppress any address they can name, so it grants no capability they lack. GET /sends still strips its own. The outbox sweep is TERMINAL-ONLY and that is a correctness rule: a scheduled row is a send this deployment still intends to make (delay_seconds can put one a day out) and a sending row may be mid-flight. One shipped defect had to be fixed for the sweep to be a bound at all. reclaimStale returned every stale sending row to scheduled, and MAX_ATTEMPTS is consulted only on a graceful retry outcome — so a send that killed the process mid-flight cycled sending -> scheduled -> sending forever, never terminal, therefore never eligible for any sweep. It now fails an exhausted row BEFORE reclaiming the rest; the order is the fix. Two indexes (idx_engo_sweep, idx_engs_sweep): every existing index on those tables has created_at in second position, which serves a per-rule window and is useless to a whole-table horizon. Proved twice: engagementRetentionSql.test.js against a real MariaDB (7 tests, incl. the acceptance case and the wrong reclaim order run deliberately), and the live stack, where a 90-day-old cancelled row was swept and a 90-day-old scheduled row survived. Co-Authored-By: Claude <noreply@anthropic.com>	2026-09-01 15:40:53 -05:00
wtclaude	eec7dbf785	fix(engagement): claim the seed guard atomically, and let a rule name a digest body ... Both defects came out of Phase 13's acceptance walk, and neither was visible to any test. **1. The one-shot rule-group guard was not a guard.** `seedRuleGroup` and `coreRules.seedGroup` both read their settings stamp, inserted the whole group, and wrote the stamp AFTER the loop. Two instances booting in the same moment both read "absent" and both insert -- the walk ended up with 52 module rules where the module ships 26. `docker compose up --scale app=2` and a rolling restart both start two instances on purpose, so this is ordinary rather than exotic. `settings.db` gains `claim(key, value)`: the same `INSERT IGNORE` as `seedDefault`, reporting its own `affectedRows`, so exactly one caller can win a key. The atomicity is the PRIMARY KEY's -- no transaction, no lock, the same bargain `engagementWorker`'s row claim already makes. Both seeders claim before inserting. The trade the code already documented is unchanged, only its order: a process that dies mid-loop leaves the group stamped and partly seeded, and the missing rules are an operator's visit to the "new rule" form. A duplicate rule is two mails per event, for every rule in the group, which both functions' own comments already call the worse outcome. **Why no test caught it:** the stubs supplied `get` and `set` over a Map, which cannot race, so they agreed with the bug -- Phase 4a's `foundRows` finding in a different costume. The fake is now `claim`-shaped and decides without yielding, exactly as the table does, and both suites gained a test that runs two seeders with `Promise.all` and asserts one insert each. Verified by reverting the fix: the module test then reports every rule inserted twice. **2. A rule that names a `digest` body could not be saved.** `registries.js` `checkSeedRule` permits `digest` in as many words -- it is the body `teamDigestWorker` renders for a rule whose email channel an individual set to digest mode, so it never appears in `channels` and never could -- and MODULE_API.md 2.4 tells modules they may point `template_keys` at `notify.digest`. `engagementRules.model.js` then rejected any key that was not one of the rule's channels. So every rule shipping a digest body answered an operator who opened it and pressed Save with a 400 naming a key they had never typed: core's own Team and news rules, and sixteen of module-uo's. The only way to save was to delete the digest body, silently dropping digest support from that rule. The two validators now agree; anything that is neither a channel nor `digest` is still refused, with a test for each direction. No MODULE_API bump: no member is added, removed or changed, and the documented contract is what the code now honours rather than something new. Co-Authored-By: Claude <noreply@anthropic.com>	2026-09-01 14:29:21 -05:00
Claude	c8d45733b6	fix(engagement): two defects the Phase 11b live walk found in core ... Both are invisible to a fixture and loud on a real database, which is why the walk is the phase's acceptance rather than a formality. 1. registerEngagementSeeds validated `max_sends_per_hour` and then dropped it from the normalized rule. The column is NOT NULL, so every one of the 25 module-seeded rules failed to insert at boot. The registry test asserted the REJECTION of a bad ceiling and never that a good one survives; it now asserts the normalized rule against `engagementRules.db.insert`'s own column list, so the next field added is covered the day it is added. 2. The cooldown claim runs inside the engine's per-channel loop and its key was (rule, user, subject). So the first channel of a rule claimed the cooldown and every later one was reported as cooled -- and `inapp` is ranked first deliberately, so a rule naming email + in-app delivered the inbox item and silently never the mail. Core's own `news.post` rule has that shape. Phase 11b's decision 8 requires the letter and the inbox item to fire together. `channel` joins the PRIMARY KEY (the org lead's decision 12: a cooldown is per delivery, not per occasion). Migrated in place behind an information_schema guard, because MariaDB has no conditional form of a key change and replaying schema.sql would otherwise fail on every boot after the first. 1551 core tests green; both new tests verified by reverting each fix in turn. Co-Authored-By: Claude <noreply@anthropic.com>	2026-09-01 07:12:09 -05:00
wtclaude	40ab1ce8d2	fix(modules): bump the CLIENT half of MODULE_API_VERSION to 1.9.0 ... The two halves version ONE contract and a test asserts they agree (client/test/moduleRegistry.test.js). I bumped server/src/modules/version.js and not client/src/modules/version.js, so client-build went red — the job runs the client suite before it builds. Nothing on the client half changed: a seed is server-side data and core's seeders write it on the boot path. It bumps for the reason its own header gives — a module declares one `coreApi` range against both halves, and a client claiming 1.8.0 while the server answers 1.9.0 is two answers to one question. 327 client tests green; client build clean. Co-Authored-By: Claude <noreply@anthropic.com>	2026-09-01 01:22:00 -05:00
wtclaude	0a9149a04f	fix(engagement): a trigger-bound template may reference the unsubscribe link ... Found building module-uo's sixteen in-universe bodies, which are the first trigger-bound templates in the system to carry an unsubscribe line of their own. `emailChannel.deliver` computes an unsubscribe token per recipient and merges it LAST over the projection, so `{{unsubscribeUrl}}` has always RENDERED correctly. But `variablesFor` takes a trigger-bound template's variable list from the trigger's declaration, and a trigger has no business declaring a fact about how the mail was delivered — so the token was undeclared, and the save-time undeclared-variable check would have refused the first operator who tried to EDIT one of those bodies. Rendering right and then refusing the edit is the worst of both. Nothing had ever taken this path: core's generic `notify.event` declares `unsubscribeUrl` in its own seed and is bound to no trigger, so `seedByKey` supplied it there. Adds DELIVERY_VARIABLES beside AMBIENT_VARIABLES — declared separately because they apply to a different set of templates. Ambient facts are about the deployment and reach every body; delivery facts are about the send and reach the trigger-bound ones, which is exactly the set that is engagement mail. Co-Authored-By: Claude <noreply@anthropic.com>	2026-09-01 01:07:16 -05:00
wtclaude	cfd1cb3c3c	feat(engagement): let a module ship its own templates and rules (Phase 11b) ... Phase 11a declared 24 triggers and stopped where the plan said it would. Standing 11b up found that the next sentence — "24 rules, all enabled = 0; bespoke template bodies" — described work with no mechanism to land in: templateSeeds.js and coreRules.js are core files with core arrays in them, and there was no registerTemplates or registerRules anywhere in registries.js. So a module could say what an event's payload was and could never say what the mail should read like. That is tolerable for one trigger and not for a catalogue, and it is decisive once the bodies carry domain prose core must not contain (§5.2). Adds api.registerEngagementSeeds({ templates, ruleGroups }) — MODULE_API 1.9.0. The module supplies data; core keeps seedOne's customized skip, its seed_version comparison and the block registry's validation, which is the whole argument for a registry over the ctx.query a module already holds: a copy of any of those living outside engagement/ would drift the first time core improved the original, and the drift would surface as a mail somebody already received. The two halves behave differently, deliberately: - Templates re-ensure on every boot, so a bumped seedVersion reaches every deployment except the ones where an operator edited that row. - Rule groups are ONE-SHOT, each under its own settings guard — re-ensuring would resurrect a rule an operator deleted and reset one they enabled. This is 11a's seed-key finding stated as an API rather than as a warning: a rule appended to an existing group reaches fresh installs only, and one that must reach stamped deployments takes a new group key. Three prohibitions, each a shipped mistake that would only surface as mail: a seeded rule is always enabled = 0 (Q3's invariant, ignored rather than refused so a typo cannot take a module offline at boot); a module may not mark a template protected; and a rule may only name its own trigger ids and its own or core's template keys, with template keys namespaced because the key column is UNIQUE. Runs from modules/lifecycle.js boot() rather than seedDefaults(), and that is forced rather than chosen: server.js seeds before it requires app.js, and requiring app.js is what runs the loader — at the moment core seeds, no module has registered anything. Placed after the installed_modules reconcile (so a disabled or failed module is skipped) and before the onBoot dispatch (so a module warming a cache may assume its rules exist). 16 new tests; 1549 core tests green; check:modules clean. Refs docs#/ENGAGEMENT.md Phase 11b decision 7. Co-Authored-By: Claude <noreply@anthropic.com>	2026-09-01 00:46:15 -05:00
wtclaude	1d4cd4adae	feat(engagement): the admin ceiling and core's news.post emitter (Phase 11a) ... Core's half of ENGAGEMENT.md Phase 11a: the two decisions the org lead settled before any code that land in core rather than in module-uo. Pairs with Module-uo#22 and docs#194. ## Decision 1 -- a seventh ceiling, `admin`, as a child of `staff` Phase 11's operator-facing triggers (uo.audit.staff_action, uo.economy.milestone, uo.world.saved) are described as admin-audience everywhere, and the narrowest value the lattice had was `staff` -- which ceilings.js defines as admin, editor AND moderator. Ceilinging them there would have let an operator save a rule that mails the staff audit digest to every moderator in it. `admin` is the ONLY genuine refinement in the tree -- every admin is staff, which is exactly the containment every other pair of branches lacks -- so it is a child rather than a seventh leaf, and permits/meet/meetAll needed no change beyond the new PARENT entry. **The one non-obvious consequence, and the reason for ROLE_CEILINGS.** notificationChannelPrefs' `visibleTo` asked `item.ceiling !== 'staff'`. That was correct while `staff` was the only role-gated value, and the day `admin` arrived it would have silently published every admin-ceilinged id -- the staff audit digest, the economy thresholds -- to every player's preferences screen by name. It now reads a TABLE (`ceilings.reachableBy`), so a ceiling added without an entry fails closed instead. An EDITOR is the viewer that tells the two rules apart, and the new tests use one. MODULE_API_VERSION -> 1.8.0 on both halves. Additive: every declaration valid under 1.7.0 is valid now and no stored value changes. ## Decision 5 -- 7.1 Q9: news.post gets an emitter, and it REPLACES the tickle `news.post` has been a declared payload contract with no caller since Phase 2, so a rule naming it could never fire. utils/newsNotify.js is the caller; announceIfNewlyPublished now calls it instead of pushDispatch.publish, gated on the same enqueueIfNeeded job id -- the single "newly published news" transition signal, not re-derived. **News push therefore stops on upgrade** until an operator enables the seeded rule. That is the org lead's decision, taken over keeping the raw call beside the emit "for one release": an exception with a deadline nobody owns, which Phase 6 already refused for Teams. The Rules screen gains a second migration notice naming news, and Phase 13's release note carries it as an upgrade step. **The seed needed its own one-shot key, and this is the trap worth recording.** `engagement_team_rules_seeded` is already stamped on every deployment that has booted since Phase 6, and the guard reads its presence -- so appending news to RULES would have seeded it on fresh installs only, and on exactly the upgrades that lose their raw push, never. One key per seed GROUP is now the rule; seedGroup() is the shared implementation and seedCoreRules() is what boot calls. Also fixes news.post's `postUrl` example, which named `/news/<slug>` -- a path App.jsx does not mount. An example is what the template editor previews and test-sends with, so a wrong one is a preview that looks right and a mail that is not. It is `/site/news`, the list, which is what the Discord and town-crier announcements have always linked. 1550 tests pass (16 new), 327 client tests pass, client builds, check:modules clean -- core still names no module identifier with module-uo now registering 24 UO-named triggers. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-31 20:33:02 -05:00
wtclaude	c208543044	feat(engagement): deliverability — suppression, bounces and the verification gate ... ENGAGEMENT.md Phase 9, closing gap G16. Two mechanisms decide that somebody in a rule's audience does not get the mail, and they sit at deliberately different points in the pipeline. `engagement_suppressions` is checked at DELIVERY: an outbox row can sit through a rule's `delay_seconds` grace window and an address can bounce inside it, so the only correct check is the one taken immediately before the transport call — which is also what produces the `status='suppressed'` row with no transport call at all. The Phase 1b verification gate is applied at ENQUEUE, through a new optional `registerDeliveryChannel({ eligible })` that only `email` declares. Filtering the shared audience would have silenced the wrong sink: a rule spanning email and in-app must still put an item in an unverified user's inbox. The excluded counts reach `summary.ineligible` and the admin reach preview, which until now reported an audience size that was never the number of people who would be mailed. `bounceClassify.js` is the only thing that may write a `bounce` row, and it is deliberately NOT `mailer.PERMANENT_CODES`. That set answers "is retrying pointless?" and contains EAUTH and 554 — an auth failure and a relay-wide policy refusal, neither of which is a fact about the recipient. Reusing it would mean one stale SMTP password suppressing every address the worker touched, silently. The classifier reads the RFC 3463 enhanced status first, falls back to a phrase match only past a veto list and only for 550/551/553, and does not suppress anything it is unsure about. Scope is engagement rules only: resets, invites, verification and the contact form still attempt, matching the posture passwordReset.controller.js already stated. Found on the live rig, against a real MariaDB and a real SMTP conversation: a hard bounce was being recorded as `failed`, so the Send Log's "Bounced" filter — a status `engagement_sends` has carried since §4.5 — matched nothing and always would have. It is now its own outcome; the outbox row stays `failed`, since that ENUM has no `bounced` and a bounced row is one that finished unsuccessfully. `address_masked` is this phase's one addition to §4.5's DDL. A hash-only table cannot be operated — an operator cannot tell three typos from a whole domain refusing mail — and the domain survives while the local part is destroyed, so the column can never be read back as an address book. - schema: `engagement_suppressions` (+ `address_masked`, `created_by`) - `GET/POST/DELETE /api/v1/admin/engagement/suppressions`, and Admin → Engagement → Suppressions, the only way out of the list - `sendNotification` returns `smtp: { code, responseCode, response }` - 26 new tests; swagger, routes manifest and guards regenerated Docs: RunicGateway/docs#191. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-31 10:47:34 -05:00
wtclaude	24a3cd85b3	feat(engagement): the in-app channel, core and web (engagement Phase 7) ... ENGAGEMENT.md Phase 7. `user_notifications`, the in-app DeliveryChannel, the four inbox routes, and the web surface — plus the two pieces earlier phases assigned here that Phase 7's own acceptance line omits. Four decisions settled by the org lead before any code: 1. `inapp` defaults to `instant` — the only channel that does. Push wakes a device somebody is holding and email leaves the building, so both are asked for; an inbox item is a row on a page the user chose to open. Left `off` the channel ships dead. 2. The phase takes push's `deliver` (§2603) and the web per-channel preferences screen (Phase 3's as-built), neither of which its own bullets mention. 3. The inbox takes `/auth/me/notifications` and `/account/notifications`; the preferences screen moves to `…/settings`. The plain word belongs to the content, which is what the bell opens. 4. `ctx.inbox.push` honours the user's in-app preference when `triggerId` names a registered trigger, and writes when it does not. Server - `user_notifications` + `model/userNotifications/`. The dedupe UNIQUE is scoped to the USER, narrower than the outbox's `(rule, user, channel)`: an inbox has no channel dimension, so two rows for one event would be one item shown twice. - `engagement/inappChannel.js` — renders by block ROLE (first heading → title, first button → url, the rest → body) and inserts. `pushChannel.js` — a content-free `{stream, ref}` tickle whose ref deep-links the inbox row. - `engine.liveChannels` orders `inapp` first (`CHANNEL_ORDER`) so that ref resolves on the first sweep. An ordering, not a dependency. - `templates.renderInappByKey` + `resolveTemplate` extracted from `renderByKey`, so both channels take the same fallback chain. - `inapp.event` seed → seedVersion 2: it named `body`/`url`, which nothing supplies. Renamed to the structural vocabulary the projection fills in. - `utils/userNotificationsPrune.js` — nightly, READ items only, horizon in `settings.user_notifications_retain_days` (default 90). - `GET /auth/me/notifications`, `…/unread-count`, `POST …/:id/read`, `POST …/read-all`. Swagger + route manifest + four component schemas. Web - `NotificationBell` in all three headers, polling its badge once a minute and pausing while the tab is hidden. `PlayerInbox` at `/account/notifications`. - The preferences screen becomes a channel matrix over `/auth/me/notifications/channels` — a strict superset of the push-only stream list it replaces. The two legacy endpoints are untouched, so the shipped Android app keeps its wire shape. - Staff get the same two screens at `/admin/notifications…`: `RequirePlayer` keeps them out of `/account`, so without this the inbox was unreachable for every non-player account. `lib/notificationPaths.js` is the one mapping. Verified: 28 new server tests (5 of them against a real MariaDB, for the three index/statement properties that are a server contract rather than a reading of this code) + 3 client. Server suite green, client 327 green. A live rig walked the whole path: two rules on one event produced three outbox rows and exactly one inbox item, the tickle carried `ref: notification:2`, and the retention sweep dropped an aged read row while keeping an equally aged unread one. Docs: RunicGateway/docs#TBD, RunicGateway/runicgateway.com#TBD Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-31 02:07:10 -05:00
wtclaude	065bec7ad8	feat(engagement): the email channel on the engine, and the Teams migration (engagement Phase 6) ... Email becomes a DeliveryChannel driven by rules, and the Team pipeline stops being its own thing. `teamNotify.forumPost` now emits an event; a rule decides who is mailed, through which template, and how often at most. One walk goes forum write -> events.emit -> rule -> outbox -> worker -> email channel -> template -> SMTP. Seven decisions settled by the org lead before any code: - email only moves; the push tickle and the Discord bridge stay direct calls - the EVENT carries its access-checked audience, and `members` resolves to it - the four Team rules are seeded DISABLED, with an admin banner and a note - team_notification_prefs stays, read by the engine as a scoped preference - the payload wins and a structural projection fills the gaps - the digest keeps computing at send time; only its state generalizes - an unsubscribe token turns off the channel it names, and nothing else Three defects found while building it: - `email.button` never absolutized its href, while image and itemList both did. Every rule-driven CTA would have been a dead relative link, because a trigger's url variables are validated site-relative by construction. - Phase 4a enqueued digest-mode recipients for a drain that Phase 6 decided not to build. An outbox row snapshots the payload and so has none of the three properties the digest design exists for, including the security one. - the digest's send-log row carried no address_hash while the instant row beside it did, which would have made half the mail uncorrelatable in Phase 9. Also: engagement_digest_state + a replay-safe backfill, engagement_outbox.scope_key, a v2 unsubscribe token that still verifies v1 forever, and the canonical /public/engagement/unsubscribe pair with the old /public/teams path kept permanently — mail is not editable once sent. Verified with 1464 server tests, 324 client tests, and a live rig (MariaDB + Mailpit + a real Team) covering the instant mail, the digest, the generic template, a pre-migration unsubscribe link and the backfill's replay-safety. Docs: RunicGateway/docs#TBD Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-29 20:11:54 -05:00
wtclaude	3f90070566	feat(engagement): the template editor, the trigger catalog and the send log (engagement Phase 5b) ... Phase 5a gave templates a table, a renderer and nine seeded rows; nothing could change one. This is the screen that lets an operator change one without being able to break the mail the system depends on — plus the two screens Q4 promised Phase 5: Triggers (read-only, from the registries) and the Send Log, which closes G15. The shape follows from one fact: a mail body is rendered by the SERVER, so the preview is too, and framed rather than redrawn in React. A client-side renderer would be a second implementation of the one artifact that matters, agreeing with the send path on the day it was written and drifting from the first Outlook fix on. Settled with the org lead before any code: a shipped default is edited IN PLACE (`protected` blocks deletion and nothing else, `customized = 1` keeps the edit); duplicate is the only way to a new template; `renderByKey` now requires `published`; a test send is logged under a synthetic `core.admin.test-send`; and a template a rule points at refuses deletion with a 409 naming the rules. Three things the plan did not know, found by building it: - The undeclared-variable check cannot be a token scan. `email.itemList.variable` holds a BARE name, so a digest pointed at `itmes` would have saved clean and arrived empty. Blocks now declare `variables(props)`; the editor makes that field a select over the trigger's list variables so the typo is unavailable. - A duplicate that drops `seed_key` loses its variable palette, so duplicating `notify.event` would have been refused for the tokens it was copied with — the one action §4.6.2 offers, refusing itself. The copy inherits it; `customized` is what the seeder actually reads. - `validateEmailBlocks` returns `{ valid, errors }`, not an array, and the first version tested it with `.length` — so block validation never ran at all. Also fixes a Phase 4a defect the live walk found, with the org lead's approval: a rule's template key was checked against a pattern with no dot in it, so no rule could name any template that exists — §4.6.2's whole duplicate-and-point-a-rule-at-it workflow was unreachable. Both models now read one pattern. Verified against the running stack: real multipart mail into a mailpit catcher including an unsaved draft, the draft/published arms both ways through the real mailer path, every refusal, and the end-to-end duplicate → rule → 409 walk. Server 1428 tests green, client 324. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-29 18:13:57 -05:00
wtclaude	12ff201ed5	feat(engagement): templates — the email block family, renderer and seeded set (engagement Phase 5a) ... Every subject and body moves out of `mailer.js` into `engagement_templates` rows an operator can edit. A relocation, not a regression: nothing that sends mail today starts depending on an operator authoring something first. - `email.*` block family in its own registry, sharing the page family's envelope walk and validate-then-sanitize order by binding rather than by copy. - A server-side renderer producing both parts of a multipart message; the text part is byte-identical to the literals this commit deletes. - Nine seeded templates, six of them wired now; the seeder's `customized = 0` guard lives in the UPDATE's own WHERE. - `renderByKey` falls back to the shipped seed when a row is missing or unusable, so no failure of the table can stop a password reset. Also fixes `check:hosts` reading the template key `auth.email-verify` as the hostname `auth.email`. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-29 13:07:39 -05:00
wtclaude	4b45eddb5d	feat(engagement): Admin - Engagement - Rules and Audiences (engagement Phase 4b) ... The admin surface over the Phase 4a engine: two screens, twelve routes and the reach preview. Nothing in the engine changed; what changed is that an operator can now reach it. Four decisions settled by the org lead before any code: - segments get their OWN nav entry, "Audiences", not a tab of the rules screen - the on/off switch is its own PATCH route, not a full PUT - the reach preview is a count only, on demand - a rule can be hard-deleted; the send log survives it The switch is the one with real content in it. A PUT re-validates against the registries as they are NOW, so the rules a re-validating toggle cannot switch off are exactly the three an operator most wants stopped: a rule whose module was uninstalled, one naming a channel that is gone, and one whose trigger has since narrowed its ceiling under a saved audience. PATCH .../enabled writes one column and always works. Switching ON unvalidated is safe because the engine re-checks the ceiling at send time. The preview calls the engine's own resolver rather than a second query that agrees with it today, and answers a count and nothing else - the resolver's output for a module-declared segment is a set of players derived from game data. It reports `capped` at the 5000-row bound (the count is a floor, not a total), `reason` for an `owner` audience (which resolves per event and has no advance answer), and `permitted` so the editor cannot show a healthy number beside a save the server will refuse. Two defects found by walking it against a live server, both in Phase 4a's code: 1. A rule pointing at a DORMANT segment read as healthy. listAnnotated asked only whether the segment ROW existed. The other shape of the same failure is a segment sitting exactly where it was whose every audience belongs to an uninstalled module: same outcome, nothing deleted. Uninstalling a module under an enabled rule produced a rule the screen showed as on and firing. The expression walk now lives in engagement/segments.js as `missingAudiences` and both lists ask it. 2. "1 rule still use this segment" - the delete refusal pluralised the noun and not the verb, in the sentence an operator reads when told no. Also: a rule's trigger is now a stated rule rather than an omission in the UPDATE statement (its cooldowns, queued sends and history are all about one trigger id); a condition tree the editor cannot render is shown read-only rather than flattened, because flattening changes which events fire the rule; and literals are coerced client-side to the type the trigger declared, with anything that does not parse passed through unchanged so the server's refusal names the variable. Tests: 21 new server tests (test/engagementAdmin.test.js) and 25 client ones (client/test/engagementRules.test.js), all green. The single failure in the server suite (`the committed manifest matches the declarations in the tree`) is the known Windows CRLF artifact and fails identically on clean edge. Companion docs PR: docs#184. - [x] AI-assisted: written with Claude Code (Opus) Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-29 12:10:04 -05:00
wtclaude	2079aaf667	feat(engagement): the rules engine, cooldowns and outbox (engagement Phase 4a) ... Phase 4 of docs/website/ENGAGEMENT.md, split 4a/4b at the org lead's direction. This is 4a: the engine, server only, with no HTTP surface at all. A fired trigger now produces outbox rows and send-log entries; Admin - Engagement - Rules and the segment composition UI are 4b. Five tables (rules, audience segments, cooldowns, outbox, sends), the sweep worker, audience resolution, condition evaluation, the grace window and its cancellation, and the save-path validation 4b's form will call. engagementEmit's Phase 2 log line becomes the engine call. Two settled questions this phase was blocked on: Q2 (multi-instance) - neither SKIP LOCKED nor documented single-instance: the outbox claims each row with a compare-and-set into the 'sending' state the ENUM already carried. It makes the outbox safe for two instances, not the deployment. Q4 (admin surface) - its own top-level nav group, built in 4b. Two defects in the plan's own section 4, both found by building it: The global UNIQUE(dedupe_key) was data loss. A dedupe key names the EVENT, and one event is one row per (rule, user, channel) - so a fifty-person audience would have had one row admitted and forty-nine silently ignored. Scoped. Section 4.1's single INSERT ... ON DUPLICATE KEY UPDATE cooldown claim always passes against this codebase's pool: the mariadb connector defaults foundRows:true, so a no-op update reports affectedRows 1 rather than 0. It is two statements now, with the interval guard in a WHERE clause. The second defect is why there is a second test file. The stubbed suite was green against the broken claim, because a stub can only agree with whoever wrote it; engagementEngineSql.test.js runs the raw statements against a real MariaDB and skips when there is none. Verification: 43 new tests green in engagementEngine.test.js, 12 more against MariaDB 11.8, and the whole path exercised end to end against a live database - per-subject cooldowns, conditions, the CAS claim, the send log's honest failure detail, and dormancy on uninstall. The three pre-existing Windows-only CRLF failures in the generated-artifact tests are unchanged from clean edge. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-29 08:07:27 -05:00
wtclaude	b13ffd584f	feat(notifications): per-channel preferences and the delivery-channel registry (engagement Phase 3) ... `notification_subscriptions` answers one question — which streams a user wants PUSHED — because that is the only question the shipped Android client can ask. This adds the general one: which subscribable ids, on which channel, in which mode. The old table becomes the push projection of the new one and keeps its exact wire shape, so the shipped APK needs no update and no delivery path is touched. What lands: - `engagement/channels.js` — `registerDeliveryChannel` (ENGAGEMENT.md §3.1), the declarative half only: id, label, `carriesContent`, `defaultMode`, `supportsDigest`. `addressFor`/`render`/`deliver` wait for Phases 6 and 7, for the reason `transports/index.js` deferred this file at all. `coreChannels.js` declares push / email / inapp through the subsystem's one door. - `notification_channel_prefs` + a replay-safe `INSERT IGNORE … SELECT` backfill, copying the `announce_jobs → announce_job_legs` precedent. - `GET · PUT /auth/me/notifications/channels`. The PUT is SPARSE — only the `(id, channel)` pairs named are written — deliberately unlike the two whole-set PUTs beside it. `off` is a mode rather than an omission, so this endpoint has no empty-array case and the kotlinx DTO gotcha cannot arise here. Three decisions the org lead settled before any code, and one corrects the phase's own acceptance criterion: push's `defaultMode` is `off`, not `instant`. The plan borrowed "push is opt-OUT" from `team_notification_prefs`, where no row does mean notified — but stream subscriptions have never worked that way, so `instant` would have projected the whole catalog into the legacy GET for every existing user and switched every toggle on in the shipped app after an upgrade nobody asked for. A test pins the legacy GET at `{streams:[]}` for a fresh user. One thing not named by the phase, and it is a G24 consequence rather than scope creep: a trigger ceilinged at `staff` can never reach a non-staff user, so offering the toggle would be offering a dead control AND disclosing the event exists — `uo.cheat.detected` would otherwise appear in every player's screen the moment Phase 11 declared it. Filtered from the catalog and gated on write. That gave the `staff` label its first consumer, now written down as `ceilings.STAFF_CEILING_ROLES` (the admin tier's three, deliberately not `teamGrants.STAFF_ROLES`, which answers a different question). 15 new tests; swagger, route manifest and guards regenerated. No web or app surface — those are Phases 7 and 8, where a preference governs something visible. Refs: docs/website/ENGAGEMENT.md Phase 3, §3.1, §4.5 Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-29 07:08:17 -05:00
wtclaude	563199a096	feat(modules): event triggers, audiences and the ceiling lattice (engagement Phase 2) ... The contract half of the engagement system: a module (and core) can DECLARE an event with a payload contract and fire it. Nothing delivers yet — `emit` validates, logs and stops, and Phase 4 replaces that log line with the engine. `api.registerEventTriggers` and `api.registerAudiences` ride the existing stage()/apply() validate-then-commit discipline, so a registrant that throws halfway leaves nothing behind. `ctx.events.emit` is fire-and-forget and binds the owner from the calling module — a module fires its own triggers and no one else's. `ctx.inbox.push` is present and throws until Phase 7, the shape 1.6.0 settled on for a member that arrives a phase late. MODULE_API_VERSION 1.7.0 on both halves. Additions only; module-uo's `coreApi: "^1.3.0"` still resolves. Three design decisions, approved by the org lead before any code: ONE NAMESPACE for trigger ids and notification-stream ids (ENGAGEMENT.md §7.2, against the recommendation in the text). A trigger is a payload contract attached to an id that may also carry a subscription toggle, so an id has exactly one owner across both facets, checked in both directions. Core's five trigger ids ARE its five stream ids, so the same-owner upgrade case is exercised on every boot rather than only by a module. It keeps notification_channel_prefs single-keyed in Phase 3, where two namespaces would have forced a `kind` discriminator into its primary key. Two knock-on effects appeared only once it was implemented. The id grammar had to be RELAXED to admit `_` inside a segment — §4.3's own worked example is `uo.house.idoc_warning`, and two grammars over one namespace would mean an id legal as a trigger and illegal as the stream it is the same event as. And the seven grandfathered `uo.*` ids had to share their legacy allowlist with triggers, because under one namespace `idoc.warning` is a single id. The push catalog is untouched either way: allStreams() still serves the stream facet only, so the shipped Android client sees exactly what it saw before. THE CEILING LATTICE (G24), which the plan named everywhere and defined nowhere. It is containment, not size: everyone ⊃ authenticated ⊃ {subscribers, members, staff, owner}, with the four leaves mutually incomparable. The flat total order the plan's wording invites would let a `staff`-ceilinged trigger be given an `owner` audience — a rule that mails cheat detection to the player it detected. Fewer people is not less exposure. Two incomparable ceilings have no meet at all, so a composition is refused rather than guessed; union-widens is the intuitive implementation and it is the wrong one. `kind: 'event' | 'scheduled'` is declarable now and no evaluator exists (§7.1 Q6). Registration accepts `scheduled` and emit refuses to fire one, so `kind` means something from the moment it can be written rather than from the moment it is honoured. Also: `GET /admin/engagement/{triggers,audiences}`, served from the registries rather than a table so an uninstalled module simply stops appearing; `npm run engagement:manifest` plus its CI `--check`, the twin of the route manifest, because renaming a variable breaks stored templates silently, at send time, in mail someone already received. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-29 06:40:28 -05:00
wtclaude	fbb4b0bd91	feat(auth): unique, changeable, verifiable email addresses (engagement Phase 1b) ... Makes `users.email` unique, de-duplicates the addresses an upgrade will find, and builds the self-service change-and-verify flow that did not exist. The uniqueness index is on a generated `email_norm AS (LOWER(email)) STORED` column under `utf8mb4_bin`, NOT on `email` under a `_ci` collation as the plan specified. Every case-insensitive collation this server offers is also accent-insensitive: `josé@x.com` and `jose@x.com` compare equal, and those are two different mailboxes. The plan's index would have refused the second address forever and the de-duplication would have nulled a legitimate account's. A requested address is STAGED in `email_pending` and only a tokened link installs it, so a typo cannot silently redirect account-recovery mail. `isDuplicateUsername()` now distinguishes the two indexes. All five call sites branch on it; each answers differently on purpose, because a public form, an IdP callback, a half-completed invite and an admin screen do not owe the same person the same amount of truth. SSO reads the IdP's actual `email_verified`/`verified` claim instead of inferring verification from an address merely being present. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-29 01:53:50 -05:00
wtclaude	6e61146678	refactor(api): collapse /admin/account and /player/account onto /auth/me/account ... Self-service account security had three URL surfaces onto one controller. All three mounted the same `admin/account.controller.js` handlers; each of the three router files carried a header comment apologising for the arrangement. `/auth/me/account` was already a strict superset, which settles which to keep: /admin/account 6 routes noindex, isLoggedIn, staffOnly /player/account 8 routes noindex, requireAuth /auth/me/account 10 routes noindex, requireAuth Neither of the deleted surfaces carried recovery codes, and /admin/account carried no username or password change at all — so client.js already called /auth/me/account/recovery-codes/* for two operations on a screen it otherwise served from /admin/account. The split was leaking before this change. Gating is equivalent where it overlapped: /player and /auth/me apply identical `noindex, requireAuth`, and `staffOnly` on /admin/account was strictly narrower while buying nothing, since every handler is self-scoped to req.user.id. There is no CSRF layer to differ. - 14 routes deleted, 0 added, no handler changed. - account.controller.js moves router/v1/admin/ -> router/v1/auth/, beside the one router that still reaches it. - Web client: 14 call sites move onto a root-level api.myAccount / api.changeUsername / ... group, matching the /auth/me methods already there. - Android app: no change. MeApi.kt was already 100% /auth/me/account/*. - Two swagger tags, `Admin · Account` and `Player`, were declared only by the deleted routes and go with them. The orphaned `AccountStatus` schema goes too; `PlayerAccount` is re-described as the any-role /auth/me/account shape (the name is kept so existing $refs resolve). Breaking to the published OpenAPI surface, accepted deliberately: both consumers are in this org, and deprecate-then-delete would leave the next phase deciding whether to add routes to surfaces already marked for removal. Verification: routes.manifest.json shows exactly 14 deletions and 0 additions. The OpenAPI spec loses the same 14 paths with zero surviving path definitions changed; its large textual diff is pure reordering, because removing the first-mounted router shifts every later path. 1203 server tests, 288 client tests, 53 bot tests green; check:modules, check:hosts and routes:manifest --check all pass. Design of record: docs/website/ENGAGEMENT.md Phase 1a. This lands ahead of engagement Phase 1b, which adds a self-service email field — written once here rather than three times. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-29 00:49:25 -05:00
Claude	c4ab8b9b9d	docs(email): SMTP setup, the three postures, and the upgrade note ... The operator-facing half of engagement Phase 1. README's stack table and security section, plus both .env.example files, all pointed at the removed Connect Gmail flow. The env comments now name the three supported postures rather than one provider — a relay as the recommendation, smtp.gmail.com:587 with an app password as the shortest migration, an unauthenticated local MTA as the third — and point at docs/website/UPGRADE_NOTES.md for the deployment this actually happens to. The OpenAPI spec is regenerated: two routes gone, three annotations rewritten, and the dashboard's new warnings[] documented. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-28 20:54:04 -05:00
Claude	47c8b37d45	feat(email): remove Gmail OAuth2, put SMTP behind a transport registry ... Engagement Phase 1 (docs/website/ENGAGEMENT.md §1.2a, §3.1, §3.2). A subtraction and a replacement in one commit, because leaving the OAuth2 flow half-wired across a release is worse than either end state. Deleted, per the §1.2a inventory: GET /admin/email/connect/start and /connect/callback, the connectStart/connectCallback controllers with the email_oauth_tx signed cookie, the PKCE verifier and CSRF nonce plumbing, the https://mail.google.com/ scope, the borrowed `google` auth-providers client, the OAuth2 nodemailer transport with its smtp.gmail.com:465 literals, the refresh-token decrypt in the model, and the client's Connect Gmail button, redirect banner and six Gmail error strings. `provider` and `refresh_token_enc` stay as columns under the additive-only discipline, unread. Added: a mail transport registry (server/src/engagement/transports) with `smtp` as the sole registration. `credentialFields` is the single declaration the admin form renders, the sanitizer filters against, and the "is it secret" answer comes from, so adding a transport is a registration rather than four edits. email_config gains transport / credential_enc (one encrypted JSON blob, since the field list is the transport's to declare) / reply_to. All six call sites keep their exact failure contracts: the contact form's mailto fallback, the invite's copyable link, the reset's generic 200, and sendTeamNotification's never-throws. One deliberate behaviour change: `enabled` now gates every sender rather than only isConfigured() — the connect flow used to set it as a side effect, and with a credential form the toggle has to mean what it says. Send-test becomes the real verification. Under OAuth2 the sender came back from Google and was guaranteed to belong to the credential; operator-typed, it can be refused, so failures name the sender and the SPF/DMARC reason (§1.2a consequence 2). G22, the silent degradation: an upgraded deployment backfills to smtp with no credentials and every sink politely does nothing. The admin dashboard now warns when the deprecated Gmail token is present and no replacement credential is, so the one deployment this happens to is told. A fresh install has never had mail and is not nagged. Guardrails: new `npm run check:hosts` (§3.2 rule 4) with its own self-test, wired into pr-checks before the install; routes.manifest and routes.guards regenerated (-2 routes). Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-28 20:52:55 -05:00
Claude	e25e7ade80	fix(swagger): hoist the one inline predicate that makes the generator run away ... `npm run swagger` cannot run on this tree. It dies with swagger-autogen's "FATAL ERROR: invalid array length - Allocation failed", generating nothing, and it reproduces on a pristine checkout under both Node 20 and Node 24 — so the committed spec cannot be regenerated by anyone, and any PR that adds or changes a route is unable to meet the standing obligation to update it. Bisected to one statement in `teams.router.js`: param('teamId').custom((v) => v === 'default' || TEAM_ID.test(v)) Hoisting that arrow to a named const fixes it outright. Nothing else changes and the regenerated spec is byte-identical to the committed one, so this is a generator fix, not a spec change. The diagnosis worth keeping, because the file's own comment recorded a different one. Phase 8 shipped a bare regex LITERAL before `.test(` and phase 9 hoisted the regex, blaming a per-file route limit measured at twenty statements; the file has sat at nineteen ever since on the theory that it was one under the edge. That theory is wrong. Probing every router file individually, `teams.router.js` at nineteen statements dies while a THREE-route file carrying only this one route also dies — so the trigger is the inline arrow reaching `.test(`, not the count. Hoisting the regex was half the fix; the predicate around it needed hoisting too. The comments in `teams.router.js`, `teamsVoice.router.js` and `admin/index.js` are corrected to say so, since all three currently tell the next person to keep counting statements. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-28 20:47:25 -05:00
wtclaude	f72c92ffbe	fix(teams): the two defects the phase 9 rig walk found ... Walked against real MariaDB, the real app, and a fake standing in for Discord that mounts the bot's real internal routes — everything up to the Discord API call was production code. 47 assertions, and it found two things every unit test in the phase had passed over. 1. **Every query failed: two result columns named `team_id`.** `desiredTeams` and `holdersWithoutClaim` both select `t.id AS team_id`, and the shared column list added `i.team_id` beside it. The `mariadb` driver refuses a result set with a repeated field name outright, so the pass died at its first query with "Error in results, duplicate field name `team_id`" — on the one code path every unit test stubs. It was also the wrong column: `desiredTeams` LEFT JOINs, so `i.team_id` is NULL for exactly the Teams that have no channel yet, which is the create case. The two queries that do not join `teams` now ask for it by name. The regression test checks the INTERPOLATED sql captured from a fake `query`, not the source text — in the source the shared list is still a `${COLUMNS}` placeholder, and a first attempt that read the file passed happily with the bug reintroduced. 2. **"Sync now" said "Nothing was done" while it was doing it.** Saving the settings with voice switched on asks for a pass. An operator who then presses Sync now — the obvious next thing — hit `running` and got back `ran: false, reason: "a pass is already running"`, which the panel renders as nothing having happened, while the pass they triggered was busy creating their channels. A pass in flight is now JOINED and its real outcome returned, the same choice `teamSync.reconcileNow` makes for the same reason. Tests: 1162 server (+2), 53 bot, 284 client. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-19 00:07:27 -05:00
wtclaude	61abb3ec89	feat(teams): phase 9 — one voice channel per Team, granted by a role ... TEAMS.md §7.3. Each qualifying Team gets a Discord voice channel of its own and a role that opens it, kept in step by a reconciler that rides the Team reconcile it already depends on. Access is a per-Team ROLE, always. §7.3 designed per-member overwrites with escalation to a role above ~90 members; the org lead settled on roles always (2026-08-18), which deletes `voice_overwrite_max`, the escalation and the `mode` column — and moves the ceiling. Overwrites are capped per channel, so the old shape's limit was "how big can one Team be"; roles are capped per guild at 250, so the new one is "how many Teams can have voice at all". That is a limit an operator must be told about before they hit it, so the panel reports it and the pass refuses the create rather than letting Discord do it. Three things §7.3 named that this codebase does not have, all settled by asking the operator because nothing in the data model can answer: - "the staff role" — there is no staff-role concept anywhere. Now a list of role ids the admin designates; empty is a normal answer, since guild administrators bypass overwrites and what is really missing is a way to let NON-admin staff in. - the parent category — §7.3 said the bot creates it and gave the id nowhere to live (`team_integrations.team_id` is NOT NULL). The bot creates it and the server stores the id in settings. - whether the bot can act at all — nothing has ever checked. The operator invites the bot by hand and no invite URL with a permission integer exists in the tree, so a deployment can be one unticked box from every call failing. A preflight is now a PRECONDITION to enabling (422), not a per-Team error discovered afterwards. Two more, decided rather than asked: - the threshold counts every active member, not linked ones. §7.3 wrote `voice_min_linked_members`; the operator is judging whether a Team is real, and link state answers a different question. - hidden Teams are never provisioned. A channel name is a game-sourced string published outside the site, which is exactly §2.8's concern — reservedNames.js already names "and eventually a Discord channel name" as a surface it protects — so the screen that suppresses a Team's page suppresses its channel, and a Team that becomes hidden takes the grace window. Turning voice OFF tears nothing down: the pass suspends in both directions and the panel offers per-row removal. A checkbox must not delete structure in somebody's guild. Fixes a phase 8 defect that blocks this phase's own artifact: `npm run swagger` has been unable to run on `edge` at all. `param('teamId').custom((v) => ... || /^[0-9]+$/.test(v))` makes swagger-autogen's parser run away — a regex literal followed directly by `.test(`. Hoisted to a const, as modules.router.js already does. Underneath it, `teams.router.js` sits exactly at that parser's per-file limit: at twenty `teamsRouter.*` statements it dies, at nineteen it generates, and one more statement of ANY shape tips it — an unannotated route does, and so does a bare `use`. So the voice routes are their own router file mounted from `admin/index.js`, and teams.router.js keeps its nineteen. Also breaks a require cycle this phase would have introduced: teamSync -> teamVoiceSync -> teams.model -> teamSync left `teams.model` holding the reconciler's exports object as it stood mid-load — the empty one, since `module.exports = {…}` replaces rather than fills. The symptom is not in the new code: it is `teamSync.intervalSeconds is not a function` thrown out of `syncStatus()`, the freshness banner on every public Team page. Tests: 1160 server (+40), 53 bot (+21), 284 client (+21). Swagger, routes manifest and guards regenerated; the guard shape of the four new routes is byte-identical to the existing admin-only ones. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-18 23:49:28 -05:00
wtclaude	11b4368b57	feat(teams): phase 8 — the notifications bridge, and the gate §7.2 could not check ... 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>	2026-08-18 20:25:30 -05:00
Claude	cecd72915f	feat(teams): the slash-command seam, and the first command through it ... Phase 7 of TEAMS.md. `api.registerSlashCommands` stops throwing: a module registers a command's DEFINITION and its HANDLER together, the bot pulls the definitions over the internal listener and runs none of our code, and the handler executes here — forced by the bot container having no `modules` volume, and the right boundary anyway. Registration validates what Discord would reject as a batch (names, description lengths, the four option types, required-before-optional), because the bot registers the whole set in one PUT and a single bad entry costs every command including the bot's own. Commands are not namespaced under their owner — there is no dot in Discord's name grammar — so collisions are first-come with the holder named. The dispatcher is the access boundary: `linked` has no Discord equivalent, so the platform-side permission default can only ever be advertising. It resolves the actor by `auth_providers.kind` rather than the id slug, treats a banned account as unlinked, bounds a handler under the bot's own timeout, and keeps `ok` outside the envelope so a handler cannot forge it. Liveness is asked at both the pull and the dispatch. The registries have no removal path, so a module an operator disables at runtime would otherwise keep a live handler behind a command Discord still advertises. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-18 18:53:34 -05:00
wtclaude	13312d7fc3	fix(teams): make "replace the whole set" actually replace it ... Found walking the live rig, which is the only place it could be found: every unit test and the settings screen itself send every row, so the bug was invisible to both. `PUT /auth/me/notifications/teams` documents itself as replacing the whole set. It did not — it wrote the entries it was given and left every other preference standing. So `{"teams": []}` cleared nothing, which is precisely the body the route requires the array for: the field is mandatory even when empty so that clearing everything is expressible, and it was the one thing that did not work. A Team the caller could have named and did not now returns to its defaults. RESET rather than deleted, and the difference is `last_digest_at`: that column is the digest worker's state and not a preference, so dropping the row with it would make every visit to the settings screen re-open a day-wide digest window and mail somebody a summary they had already read. Walked again after the fix on the real database: the empty set clears, an entry naming a Team the caller is not in is still dropped, and the digest stamp survives. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-18 18:01:59 -05:00
wtclaude	5fa88baa0a	test(teams): the refusals, which is most of what a notification feature is ... A notification feature is mostly things that correctly do NOT happen, and each of these is invisible until it goes wrong in production: a departed member and a revoked guest are not recipients; a mute subtracts per Team and leaves the user's other Teams alone; the author of a post never receives the notification about it; forums switched off silences the forum streams including the digest; a Team's first roster wakes nobody; a failed send does not stamp `last_digest_at`. Two real defects came out of writing them. `Number(null)` is 0 and 0 is an integer, so a null in a caller's id list survived `filter(Number.isInteger)` and rode into an IN clause as user id 0. No row has id 0, so it was harmless — which is exactly why it would never have been noticed. Fixed in all three places that filter ids. `recipientIds: db.recipientIds` in the model captured the function OBJECT at require time, so the layer below could never be substituted. That is not only untestable; it means the model was not really the seam it claimed to be. Wrapped so `db.x` resolves at call time. The registries catalog assertion is now an exact five-element list, so a shard-content stream creeping back into core's registration fails here rather than shipping. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-18 14:35:23 -05:00
wtclaude	2a56cbf22a	feat(teams): fire the four events, and the routes that configure them ... The roster sync tickles at most ONCE per stream per run, not once per member: a tickle is content-free, so five people joining in one sweep is five identical notifications and one piece of information. Suppressed on a Team's FIRST roster, the same condition the activity feed uses and the half where it matters more — importing a 155-member guild would otherwise wake every one of their phones. Forum notifications fire from the CONTROLLER, not from the forum model. That file takes an already-resolved access decision and reads no membership table by design; the fan-out reads both to compute its recipients, so calling it from inside would make the forum model transitively depend on exactly what its header says it must not touch. The model returns a `notify` key the controller destructures out before the response, so the API's answer to "did my post save" is unchanged. `pageUrlTemplate` joins the team provider — the one thing phase 6 found that the design of record had not anticipated. Phase 3 left core with no Team page and therefore no way to LINK to one, so a notification email could name a Team and not take you to it. It is data rather than a callback: a function would put a module hook on the mail path to produce a string that never varies. Relative paths only, and protocol-relative is refused with absolute. The unsubscribe endpoint is the only write in the public tier and the only route with no `siteMode` — the reader is in their mail client, and the mail went out before the site went into maintenance. POST always answers 200, valid token or forged: distinguishing them would be an oracle for which (user, Team) pairs exist. GET redirects and acts on nothing, so a mail client's link scanner cannot mute Teams nobody asked to leave. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-18 14:34:54 -05:00
wtclaude	686a214979	feat(teams): email as the third sink, with a digest that keeps no queue ... A web-only user on a deployment running neither the Android app nor Discord gets no notification that someone replied to their own thread — which is most users on most deployments, and a forum where replies are invisible is a forum nobody returns to. Email is a third consumer of the recipient set the previous commit builds, not a fourth pipeline. Unlike a push tickle, an email carries content: a mailbox is a destination the recipient chose, not an untrusted relay reached by an unguessable topic. It carries a title and an excerpt, never a full post. The digest COMPUTES AT SEND TIME and keeps no pending-items queue. The only state is `last_digest_at`. Three properties fall out, and the third is why it was chosen: a deployment down for two days sends one correct digest rather than replaying a backlog; a post a moderator hid after it was written is simply not in the query; and a user who lost forum access between the post and the send is no longer in the recipient set, so they are not emailed content they can no longer read. `last_digest_at` is stamped only on a SUCCESSFUL send — stamping first would quietly eat a day of somebody's notifications every time the mail provider had a bad minute. One-click unsubscribe is a stateless HMAC rather than a token table. Every property that makes a password-reset token a row is absent: the link sits in a mailbox for months so it has no useful expiry, and clicking it twice must mean what clicking it once meant. Its whole capability is setting `muted` for one (user, Team) pair. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-18 14:34:38 -05:00
wtclaude	26c23bd603	feat(teams): the notification core — four streams, and a recipient set ... Phase 6's foundation: the fan-out shape the existing pipeline could not express. `pushDispatch.publish` answers "everyone subscribed to a stream" and "this one owner". Team notifications need "these N users", because Team scoping cannot live in a stream id: the catalog is a static registration validated at boot against a namespaced pattern, so a stream per Team is unexpressible, and stream ids are stored in `notification_subscriptions` rows that would need collecting every time a Team archived. So there are FOUR fixed core streams and the Team lives entirely in the recipient set. `team_notification_prefs` is opt-out for push and opt-IN for email — the two sinks default opposite ways, and the asymmetry lives in the column defaults so no condition anywhere has to remember it. One recipient query serves all four streams, because §6.2's two populations are the same set written twice: "active members with a user_id plus active grants" IS "everyone with resolved forum access". Mutes are subtracted in SQL rather than by the caller — there is no function here that returns an unfiltered set. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-18 14:34:24 -05:00
wtclaude	c970caee16	fix(teams): let a post-moderation mistake reach the model that explains it ... Found on the live rig. `moderatePost` answers `pin` with «"pin" applies to a thread, not to a post» and an invented action with "Unknown moderation action" — the distinction exists because they are different mistakes and a caller who made the first one has a bug worth naming precisely. The route's validator listed only the four actions a post accepts, so `pin` never got there: it came back as a generic "Validation failed". The precise message was written, documented, unit-tested — and unreachable through the API, which is the worst of both, because the branch reads as live code and is only exercised by its own test. The validator now lists all eight and lets the model discriminate. Both answers are 400, neither is a security boundary, and widening the list is not removing it — an action outside the enum still stops at the validator, which the added route test asserts alongside the `pin` case. Nothing else the walk exercised needed changing. The whole phase 5 surface was driven against a real server, real MariaDB and real sessions across four identities — an ordinary member, a granted non-member guest, a Team leader and a staffer — plus a browser pass over the forum panel, the reports queue, the per-Team forum ledger and the settings screen. Notably confirmed live: a locked thread refuses replies from all four identities at 409; a hidden post renders for the leader and staff with Unhide and **no Edit control for anyone**; the report queue answers 200 to staff and 403 to the leader, the member and the guest alike; and turning the edit window down to 0 stops the author while leaving staff unbounded. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-18 13:25:21 -05:00
wtclaude	3f7e61af1c	feat(teams): the phase 5 surface — discussion, replies, reports, and two admin screens ... 241 client tests pass (224 before). **The forum panel becomes a forum.** It was "Announcements" with one composer; it now has two, because phase 5 split one server capability into two: `canPost` means "may open a discussion" and every participant may — a granted guest with no game character included, which is path 3 doing its job — while `canAnnounce` is the leader-only half `canPost` used to carry alone. Threads gain replies, an edit control, per-post moderation and a report control, all still inside the one slot the module declares, still navigating by `?thread=`. **Almost nothing here is the client's decision, and the file says so.** `canPost`, `canAnnounce`, `canReply` and each post's `canEdit`/`editableUntil` are read, not computed. The one local judgement is a ticking clock that WITHDRAWS an edit offer whose deadline passed while the page sat open — it can never grant one, because a time-bounded permission must not take its clock from the party it bounds. That asymmetry is the first thing client/test/teamForum.test.js asserts. The panel's pure parts moved to `lib/teamForum.js` so they can be tested without a browser, following teamActivity.js and teamAdmin.js. Two of them are subtler than they look: * `stripToText` decodes entities AFTER stripping tags, and `&amp;` last of all. Decoding first turns an author's literal "&lt;script&gt;" into a real tag the strip pass then deletes — silently losing text that was never dangerous. * `threadSummary` counts REPLIES, which is one fewer than `postCount`. Showing the raw count tells a reader a brand-new thread already has one reply. **Three admin surfaces.** The forum settings screen gains the edit-window field (0 = posts permanent once written). The reports queue is a new screen beside Appeals — under moderation rather than under Teams, because a staffer working a queue should have one place to work and `target_type` is deliberately open-ended, so the next reportable thing arrives as a row rather than as another nav entry. Its copy tells a member where a report lands and that reporting changes nothing, because a member who expects a post to vanish and watches it stay reports it again. There is no leader-facing view and there is not meant to be. And the per-Team forum moderation ledger finally renders: the route and `api.admin.teamForumModeration()` have both existed since phase 4 with nothing calling them, which made `actor_role` — the column that keeps a leader's ordinary housekeeping distinguishable from a staff intervention — readable only from a DB client. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-18 13:08:59 -05:00
wtclaude	128de0ff2e	test(teams): phase 5's server surface, and the negative property under it ... 1008 pass (972 before). The tests worth reading first are the ones that pin a property no screen would look different without: * **The edit window is decided on the server, twice.** One test proves the read path stamps `canEdit` per post per viewer; another proves the WRITE path re-derives it from `created_at` and refuses a stale edit even though the client was told it could — because a time-bounded permission must not take its clock from the party it bounds. * **A locked thread refuses staff too**, asserted over member, leader and staff in one loop, at 409 rather than 403: well-formed request, refusing state. * **delete → restore is reversible for images.** Without the second half of the pair a restored post returns its words and loses its pictures a retention window later, silently — the test asserts both calls and that `hide` makes neither. * **Post moderation recomputes the thread's counters** rather than nudging them; the test runs hide → unhide → hide, which is the cycle a delta gets wrong. * **acceptance: nothing in the report model is reachable by a Team leader.** The negative property is the whole point of §5.6 and negatives are what nobody notices going, so it is asserted directly — the module's function surface is pinned, and `queue`/`handle` are checked not to mention leadership at all. If a leader-facing queue is ever wanted it is the org lead's decision, and this test is what makes somebody ask. * **A report never changes the content it is about**, proved by stubbing every mutation the forum has to throw. If filing a report touched a status then "report" would BE moderation, and the first person to work that out would have found a way to hide anything on the site. The test suite caught one real defect: `describeTarget` returned `undefined` for a hard-deleted target, and `undefined` is dropped by JSON.stringify — so the documented `target: null` would have reached clients as an absent key. Two phase-4 tests were updated rather than added to, both because phase 5 changed what they describe: `canPost` split into `canPost` (open a discussion, everyone) and `canAnnounce` (leaders), and `discussion` is no longer a refused thread type. Phase 5's four new player routes are added to acceptance criterion 2's list, so "with the forum off every forum route 404s" keeps covering the whole surface. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-18 12:58:15 -05:00
wtclaude	fff14848f1	feat(moderation): member-raised abuse reports, to site staff only ... TEAMS.md §5.6. **Core has had no user-facing report flow of any kind** — the `moderation`, `mod_notes` and `appeals` tables are all either staff-initiated or Discord-sanction-shaped, and nothing anywhere let a member say "this is a problem". That was survivable while every piece of content on the site came from staff; phase 5 lets players write to each other, so it stops being. The gap has a specific shape: leaders moderate their own Team's forum, and a Team's leaders are exactly the people who will not report their own Team. So the whole point of this queue is a path that routes AROUND a Team's own leadership. Org lead settled it on 2026-08-18: **reports are site administration only** — there is no leader-facing view of this queue, not even a read-only one scoped to their own Team. §5.6's "a leader may also see and act on reports for their own Team" is not implemented and is not deferred. `content_reports` is deliberately generic — `target_type` is a VARCHAR so a wiki page or a news comment becomes a value rather than a table — and the queue is mounted beside appeals under /admin/moderation rather than under Teams, because a staffer working a queue should have one place to work. **§5.6's literal unique key has a defect and this does not copy it.** Written as (target_type, target_id, reporter_user_id, status) it makes CLOSED rows collide with each other too: reporter reports a post, staff dismiss it, the behaviour recurs, they report again — and the second dismissal is an UPDATE into a tuple that already exists, so working the queue starts throwing duplicate-key errors on the first repeat reporter. The key is on a generated `open_marker` instead, the same trick `team_forum_grants.active_marker` uses: 1 while open, NULL once closed, and NULLs are distinct — which is what §5.6's prose asks for, "one open report per (target, reporter)". Two other departures from the doc, both small and both flagged in the docs PR: `handled_note`, because a queue whose resolution reason lives only in an activity_log line is one where the next staffer to see a repeat report cannot find out why the last was dismissed; and a CASCADE on `team_id`, so a deleted Team does not leave a queue full of reports about content that no longer exists. Also here: a report is filed against a target the model verifies really belongs to the Team the request came through, or the queue's per-Team filter would quietly be lying; the queue resolves every row's target in three batched reads rather than N+1, which is §5.6's fourth rule (uploader, size and sniffed type without hunting) actually paying for §5.5.4's attribution table; a target that has since been hard-deleted comes back null and the report still lists, because "somebody reported this and by the time we looked it was gone" is a fact a moderator needs; and every transition writes activity_log, `dismissed` included — a queue where acting is audited and declining to act is not is one where the cheapest way to make a report vanish leaves no trace. `teams_forum_edit_window_minutes` gains its range validation on the admin settings PUT and is seeded at 15, so the value on the settings screen is the value in force. Route manifest and OpenAPI regenerated: 6 operations added, 0 lost. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-18 12:51:42 -05:00
wtclaude	ae0d27cf27	feat(teams): discussion threads, replies, the edit window and post moderation ... Phase 5's server half — TEAMS.md §5.1's "5b". The schema for all of it landed in phase 4, so this adds no ALTER: every column it needed (`type`, `locked`, `edited_at`, `edited_by`, the post table's `status`, the ledger's `target_type='post'`) was already there waiting. * `teams_forum_edit_window_minutes` (0…1440, default 15) joins the forum's settings. It fails closed to ZERO rather than to its default, which is the opposite of what it looks like it should do: the risk an edit window bounds is an author rewriting a post out from under a reader quoting it or a moderator about to act on a report, so the safe answer during a DB fault is "nobody may edit for the next minute". A stale uploads acknowledgement freezes this key too — it is a forum setting. * Thread creation splits its authority BY TYPE, which is what phase 4's comment said would happen here rather than widening the leader gate. An announcement stays leader-authored; a discussion is open to every participant, and "participant" includes a granted non-member with no game identity — path 3 doing its job. `type` still defaults to `announcement`, so a phase-4 client keeps meaning what it meant. * Replies refuse three ways with deliberately different codes: 404 for absent or hidden, 400 for an announcement (which takes no replies by TYPE, not by being closed), and 409 for locked — well-formed request, refusing state. Locked refuses staff too; they hold `unlock`, and unlock/post/relock reaches the same place leaving three ledger rows that say so. * The edit window is evaluated on the server twice, on purpose. The read path stamps every post with `canEdit`/`editableUntil` so the client knows whether to draw the control; the write re-derives it from `created_at` before allowing anything. A time-bounded permission must not take its clock from the party it bounds. Staff are not time-bounded, and a staff edit of someone else's words writes `activity_log` while a member fixing their own typo does not (§5.3). * Post moderation shares the thread ledger via `target_type='post'`, so "everything moderated in this Team" stays one query. `pin`/`lock` are refused by name rather than as unknown actions — they describe a thread's place in a list and its openness to replies, neither of which a post has. Counters are RECOMPUTED after each action rather than nudged, because hide → unhide → hide is a cycle a delta gets wrong the first time a step is retried. Two fixes to phase 4 code this work reached: `softDeleteUploadsForPost` bound its two arguments in the wrong order (never fired — nothing called it until post deletion did), and it had no inverse, so `delete` → `restore` would have returned a post's words and silently lost its pictures a retention window later. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-18 10:43:09 -05:00
wtclaude	5baada08ef	fix(teams): four defects the live rig found in the forum ... None of these could fail a unit test, and three of them break the feature for the operator rather than for the code. **The uploads acknowledgement was a one-way door.** A settings form sends every field it owns, so once `teams_forum_images` was `uploads`, every later save re-sent `uploads` — and the gate fired on the VALUE being present rather than on the mode being SELECTED. The operator could never change a forum setting again, and the thing they would reach for in a hurry, switching the forum off, was exactly what came back 400. The gate now passes when an acknowledgement for the version in force is already on record AND uploads is already the stored mode: there is no new consent to take. A transition INTO uploads still asks, and a reworded notice is still caught by assertSettingsWritable. **An uploaded image could never become a picture.** `uploads` mode hands the composer `/uploads/<name>.png`, the composer puts it in the body as text — the author never writes markup, which is the whole design — and the renderer only rewrites ANCHORS. The linkifier matched absolute http(s) URLs only, so the write path could not produce the anchor the read path looks for, even though `isEmbeddableImageUrl` had accepted those paths since the first commit. The two halves disagreed and only a real upload showed it. **The embed sat beside its link, not beneath it**, because an <img> is inline, and nothing capped a remote image to the column — one post from a host serving a 4000px file would have blown the layout out. Core now emits `class="forum-embed"` and the stylesheet owns both. A class rather than an inline style because the style would then have to survive the client's DOMPurify pass, and its CSS sanitiser is a larger thing to reason about than one class name. **The panel's buttons had no button styling.** `btn-ghost` is a MODIFIER — every other call site in this codebase pairs it with the base `btn` — so alone it contributed colours and no geometry, and the controls rendered as bare boxes. Small inline actions use `pill`, which is what the rest of the admin surface uses for exactly these. Same class of mistake as the Material one in the Android M12 phase: the modifier carries no base. Also: the post body now re-sanitises client-side like every other body-HTML surface on this site, with `ADD_ATTR: ['referrerpolicy']`. That argument is load-bearing — DOMPurify's default allowlist carries `loading` but not `referrerpolicy`, so a plain sanitize() call silently strips the one attribute limiting what a remote embed leaks to the host serving it, which is the privacy property the admin help text promises. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-18 09:54:05 -05:00
wtclaude	57286594e7	test(teams): the four acceptance criteria, and regenerate the API artifacts ... Four tests are named "acceptance" and are Phase 4's criteria verbatim. Each names a property the code around it can lose without any screen looking different: 1. A granted, unlinked account reads the forum, is absent from the member rows, and is still refused external-platform eligibility. The membership projection is asserted byte-identical across a grant, which is what "non-contamination" means in practice. 2. With the switch off every forum route 404s AND nothing is read or written on the way there — a guard that 404s after loading the thread is one that still bumped a counter. 3. The stored HTML is byte-identical between `disabled` and `remote`; only the rendered output differs. That is the property the renderer-owned design exists to give, and it is what makes flipping the policy back a no-op rather than a migration. 4. Selecting `uploads` without a matching acknowledgement is refused server-side, with the admin checkbox bypassed. Plus the ones that are not criteria but are the same kind of claim: an author cannot smuggle an <img> or its attributes through in any mode, http and non-image URLs stay plain links, a leader cannot revoke a staff-issued grant, a demoted account stops protecting the grants it made, moderation records which authority was exercised, and a RIFF container that is not WebP is not accepted as one. Twelve new routes in the manifest, all annotated and in the OpenAPI spec. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-18 07:24:24 -05:00
wtclaude	4ac353684a	feat(teams): harden the upload path for an uploader who is not an admin ... The existing admin upload path is already good for an admin: an 8 MB cap, a mimetype allowlist, a random filename, an extension derived from the mimetype map and never from originalname, and nosniff forced on serve. All of it is kept. What it does not have is anything that assumes a hostile uploader, because until now it has not had one. Magic-byte sniffing, because `file.mimetype` is the client's own Content-Type header — a player can send image/png with arbitrary bytes and land arbitrary content under a .png. Unrecognised bytes are a rejection and never a fallback to what the header claimed. The file is on disk before it can be sniffed, so the rejection path removes it: a rejected upload left on disk is the same disk-exhaustion vector reached another way. A rolling per-account byte quota and a per-IP rate limit, because community uploads with no ceiling is disk exhaustion on the operator's own host. An attribution row per accepted file. Not bookkeeping: the acknowledgement is meaningless if "who uploaded this" cannot be answered afterwards, which is exactly what the operator has just accepted responsibility for. A nightly sweep for soft-deleted files past retention and for never-referenced orphans, in the same in-process shape as the activity prune. It runs whether or not `uploads` is the current mode, and that is the point — an operator who turns uploads off after a problem still has the files, and a sweep that switched itself off with the setting would strand exactly the bytes they were trying to be rid of. It works from the forum's own rows outward and never from the directory listing inward, because UPLOAD_DIR is shared with the admin upload path. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-18 07:24:02 -05:00
wtclaude	e27c368234	feat(teams): the grant flow, announcements, and the routes behind both guards ... Path 3's WRITE half. The resolver landed in phase 2; this is who may hand access out, to whom, and what stops a leader turning a Team forum into open hosting on the operator's site. Two authorities, and not one authority with different reach. Staff may act on any Team, uncapped, and may revoke anything. A leader may grant and revoke ordinary access on their own Team, is capped at `teams_max_grants_per_team` (default 50), is rate-limited, and may NOT revoke a staff-issued grant — which is what stops a leader undoing a moderation decision. The issuer's role is checked at revoke time rather than stored, so an account that has since lost its staff role stops protecting the grants it made. Nothing on this path writes team_members, in either direction. A grant may name any account, including one with no linked game identity — that is the point of it — and that account stays off the roster, out of every count, and ineligible for external platforms. Announcements are a degenerate thread rather than their own object, so phase 5 adds no migration. Moderation records WHICH authority was exercised: a staff action also writes activity_log, a leader's writes only the Team's own ledger. Merging the two would make a guild leader locking a thread an appealable Discord sanction. Every forum route answers 404 while the switch is off, and 404 — never 403 — to a caller with no access: in a private room the contents and the existence are the same secret. The grant routes deliberately answer even while the forum is OFF, because a toggle-off revokes no grant and the access list has to stay manageable. Under /player rather than /admin: a leader is a player, and the /admin tier gate is requireRole('admin','editor','moderator') — putting a leader endpoint behind it would mean widening that gate. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-18 07:23:47 -05:00
wtclaude	fb70013adf	feat(teams): the forum's own HTML profile, and core's image renderer ... The load-bearing decision of the whole forum design, and deliberately not how the rest of the site works. Core's shared sanitizer allows <img> from any host — it is tuned for rich text from the ADMIN editor, where the author is already trusted. Handing that to arbitrary players would make `teams_forum_images` unenforceable: every post could hotlink in every mode and the setting would be decoration. So the forum derives its own profile in which `img` is never an allowed tag, in any mode. What an author writes is a URL. What decides whether it becomes a picture is this file's renderer, at READ time. Four properties fall out: the policy cannot be evaded, because the only code that can emit an <img> is core's; flipping the setting back to `disabled` un-renders every image on every existing post with no data migration, since the images were never stored; there is no author-supplied srcset, onerror, width or style to smuggle anything through; and a blocked or dead image degrades to the URL the author actually wrote. Two details found while building it: `rel` had to be ADDED to the allowed attributes to make links safer, not laxer. The profile writes rel="noopener noreferrer nofollow" through a transform, and sanitize-html strips any attribute not on the allowlist — including one its own transform just added. Without the entry, every forum link shipped without noopener. The bare-URL linkifier runs AFTER sanitising, over the sanitiser's own output and only on text outside tags. That ordering is the security property: every text node is HTML-escaped by then, so the matched URL is safe in both the href and the link text. Running it first would be an injection point. https: only, because the CSP is `img-src 'self' data: https:` — an http: image is blocked by the browser and renders broken, which presents as "images are broken on my forum" with nothing in any log. And the server never fetches a user-supplied URL: that is an SSRF vector, and an allow-set is useless when the point is arbitrary hosts. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-18 07:23:32 -05:00
wtclaude	11fd9821bf	feat(teams): the forum schema, the operator's two switches, and the ack gate ... The whole forum schema lands at once — threads, posts, the moderation ledger and upload attribution — including the columns only phase 5's discussion threads use. That is TEAMS.md 5.1's split BY LAYER rather than by feature: phase 5 opens paths instead of migrating data. Three settings keys, and only one of them is ordinary. `teams_forums_enabled` and `teams_forum_images` are enum keys on the existing admin settings endpoint; `teams_forum_images` also carries a server-side PRECONDITION, which is why the three live in their own model rather than in the generic setMany() loop where a reader would never find it. The gate is the server's. `PUT teams_forum_images = 'uploads'` is rejected 400 unless the same request carries the acknowledgement version — the admin checkbox is how the gate is presented, never the gate. What is stored is the TEXT VERSION, so "which wording did they agree to" is answerable later; settings already record updated_by/updated_at, and an activity_log row puts it in the staff audit trail. A reworded notice makes a stored acknowledgement stale, and neither obvious answer is right: uploads KEEP WORKING, and no other forum setting may be saved until it is re-given. Non-destructive, and impossible to ignore. Both reads fail closed. A DB fault reports the forum off and images disabled — a forum that 404s for a minute is the cheap failure; a policy that is not a policy is not. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-18 07:23:15 -05:00
wtclaude	5d9d10b245	refactor(teams)!: Teams is a contract, not a surface — invert the slots ... Org lead's correction, and it changes what this phase ships. TEAMS.md §3.1 and §3.5 put four public pages and three nav rows in core. They should never have been core's. **Teams is the platform primitive that the API contract exposes; the module builds the pages on top of it.** module-uo builds guilds; the Rust module that comes next builds clans. Core does not own the word for a Team, so a core page under a noun core invented would have sat beside module-uo's existing /uo/guilds saying the same thing in the wrong vocabulary. Removed: /teams, /teams/:slug, /teams/:slug/roster, /player/teams, the public and portal nav rows, the `teams` feature flag and the core feature provider that answered it. /admin/teams stays — an operator inspecting the primitive is looking at the primitive. Kept, and unchanged: the tables, the reconciler, the access resolver, the activity feed, the retention prune, the whole public/player/admin API, optionalAuth and the roster projection. That is the contract, and it is what this phase was actually for. **So the extension slots invert, which is a new direction in MODULE_API §3.7.** `team.overview` and `team.member.row` assumed core rendered the page. In their place `registry.declareModuleSlot(id, name)` lets a MODULE declare a place on its own page and core fill it. Core fills `uo.guild.detail` with the Team activity feed — the one part of that page core cannot hand over, because only core can resolve whether the viewer is inside the Team and the public/members split is a security boundary. Three things about the inverted direction are load-bearing: - the name is namespaced under the declaring module and that is enforced, not conventional: it is the only thing keeping two modules off one name; - core's fills are applied at MOUNT rather than eagerly. Core's bundle evaluates before every module chunk, so when core registers a fill the slot does not exist yet — filling eagerly would silently do nothing; - a fill for a slot nobody declared is a no-op, never an error. The declaring module is simply not installed, which is the ordinary case. That is the opposite of §3.7, where an unknown slot throws, and the asymmetry is real: there, core declares first, so an unknown name is always a typo. `Slot` becomes the eighth member of the shared UI kit, so a module renders the place with core's own error boundary. It matters more here than anywhere else in the kit: the thing being contained is core's content failing inside the module's page. `GET /public/teams/by-external/:moduleId/:externalId` is added because a module names a Team in its own vocabulary and core keys the feed by slug. The module id is matched rather than trusted — an external id is unique only within a module. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-17 20:58:07 -05:00
wtclaude	203ce9c654	chore(teams): regenerate the OpenAPI spec and the route manifests ... `npm run swagger` + `npm run routes:manifest` for the one added route, `GET /api/v1/public/teams/:slug/activity`, and for `optionalAuth` joining `/teams/:slug/members`. The guards manifest names `optionalAuth` on both, which is the point of that file: a reviewer can see that two public routes now read the caller's identity without reading the routers. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-17 20:16:04 -05:00
wtclaude	03631d7d40	feat(teams): the roster's audience projection, and optionalAuth to resolve it ... TEAMS.md §3.3, as the eighth member of MODULE_API 1.6.0 — amended in place per the org lead, on the rule Protocol 4 was given in phase 2: a contract owes a bump only once it has landed on `main`. Two questions meet on the roster and they belong to different owners. WHICH ROWS a viewer may see is the module's, because the audience rungs and their configuration live there and core does not know what a rung is. WHAT A ROW LOOKS LIKE stays core's. So `projectRoster` answers with member KEYS, not rows. §3.3 said rows, and rows would let a module widen what is published — handing back a `userId` core had withheld — leaving core's field guarantee resting on every module's good behaviour. Core asks which rows and re-normalises the answer through its own public shape, so a module can narrow and cannot widen. "The module declines" needed splitting before it could be implemented. No module at all and a module whose rungs could not be consulted are opposite situations: the first withholds nothing and must serve the roster whole, the second must serve none of it. The refusal carries `projects`, and only `projects: true` fails closed. Without the split, bare core serves an empty roster on every Team page. This is also the first public route whose CONTENT depends on identity, which needed a middleware core did not have. `attachSession` only decodes a token, so a banned account, a password change or a logout would have kept working against the private half of a feed until the JWT expired. `optionalAuth` runs requireAuth's full database re-validation and, on any failure, continues ANONYMOUSLY rather than rejecting — a caller whose session is no longer good sees the public view, which is what they are entitled to. `GET /public/teams/:slug/activity` lands here for the same reason: §2.11's route table had no activity endpoint though §4.3 describes a filtered feed. Paged, with the visibility resolved from the session and never from a parameter. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-17 20:15:36 -05:00
wtclaude	aa332eda82	feat(teams): the activity feed, its two writers and its retention ... TEAMS.md Part 4. `team_activity` takes items from two sources and treats them identically on the read path: core writes its own membership and rename items with source='core', and a module pushes game items through `ctx.teams.activity.push`, which stops throwing and starts working. Core writing here too is deliberate — the rendering path is exercised by core's own content from day one, so the feed is never empty on a deployment whose module pushes nothing. Three rules shape the model: - core never composes a summary. It arrives already rendered and is stored verbatim; core cannot phrase "gained 15,000 gold" for a game whose vocabulary it does not know. - visibility fails closed. An item with no stated visibility is `members`. - a push never throws at its call site. It is called from inside a game-event handler, and a storage problem of core's must not become the module's control flow. Core emits four of the five kinds §4.2 names — `core.forum.thread` has nothing to emit it until the forum lands in phase 4 — and emits none of them for a Team's FIRST roster: importing a 155-member guild is one Team arriving, not 155 people joining, and a join per member would bury every real event under the import and reach the row cap on day one. Retention ships with the feed rather than after someone notices. A nightly worker applies an age horizon and a per-Team row cap, both settings; either alone has a hole, since age lets one busy guild write a million rows inside the window and a cap keeps a dead Team's feed forever. The sync now reads member ROWS rather than keys, replacing the `memberKeys` call rather than adding to it: the feed needs each changing member's display name and prior `is_leader`, and the upsert is about to overwrite both. Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-17 20:15:18 -05:00
wtclaude	cf2666e5bc	feat(teams): the Team read API, the moderation routes, and Admin -> Teams ... The eighteen routes of docs/website/TEAMS.md §2.11, their OpenAPI annotations, and the staff screen that drives them. Two rules shape the read model. Hidden means absent from every public surface -- the index, the lookup and the roster alike, and a hidden Team 404s indistinguishably from one that does not exist, because "absent" includes not confirming it is there. And staleness is surfaced rather than silent: every public payload carries { configured, stale, lastSyncAt }, so a page can say how recently the projection was confirmed instead of presenting stale data as current. The public roster withholds both the member key and the user id -- one is a game-internal identifier, the other names a site account. `linked` answers the only question a public page has without publishing which account. The module's per-audience field projection is phase 3's; this is a conservative core one. The §2.9 gate is enforced per REQUEST, not per route. A moderator may call all eighteen; three of them mean something different when they do, and the server decides from the role it re-validates on every request rather than from a token claim. The client has no "file as request" argument to get wrong. Found by booting the real server against the real database, and not by any test: **the index and the by-slug lookup disagreed about what exists.** listPublic was keyed on a registered team provider while findBySlug is not, so with no module installed `/teams` returned an empty list while `/teams/:slug/members` served a full roster -- the index denying a Team that direct URLs answered for in full. The rows are core's and they outlive the module that filled them: an uninstalled module leaves a projection that is unmaintained, not one that stopped existing, and `configured: false` is how a client learns that. The read side no longer takes the provider into account at all. There is now a test named for the property. Also verified live: the public routes answer anonymously, an unknown and a hidden slug both 404, the player and admin tiers 401 an anonymous caller, a seeded roster projects correctly, and the reconciler logs that it is staying idle with no provider registered rather than failing a boot. Process obligations, all done: #swagger.* annotations on every route, `npm run swagger` regenerated (18 paths in the spec, no dangling $refs, and the schemas they reference added), `npm run routes:manifest` regenerated -- additions only, 184 public routes -- and BACKEND_DESIGN.md updated across the schema section and all three tier tables. Admin -> Teams follows the ModulesAdmin precedent: everything that decides what a row SAYS lives in lib/teamAdmin.js, which is plain JS with tests, and the view renders it. That split earns itself here specifically -- the screen's job is to make "the shard has no Teams" and "core has not been able to ask for two hours" impossible to confuse, and those two produce the same empty table. The four freshness states are named and tested for exactly that reason, and the last provider error is shown verbatim rather than paraphrased. The button labels follow the caller's role: a moderator sees "Request publish", so the pending result is not a surprise. Hiding is offered to everyone with no gate, matching the server. Server 894 passed, client 206 passed, client build clean. 17 route tests, 20 client display tests. Refs docs/website/TEAMS.md §2.11, Part 12 phase 2 Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-17 15:27:02 -05:00
wtclaude	8fe2e01466	feat(teams): reserved-name screening, auto-hide, and the admin-approval gate ... The one place untrusted game data becomes a public page (docs/website/TEAMS.md §2.8), and the gate on releasing it (§2.9). A Team's name is written by a player, in the game, with no review, and this platform turns it into a public page, a URL and eventually a Discord channel name. Someone naming their guild "Admin" or "<Brand> Staff" gets an official-looking page on the operator's own site for free. Hide, never reject. Core cannot refuse a name -- the guild already exists in the game and core is a mirror of it, not an authority over it. A match hides the Team from public surfaces and files it in a review queue, and it keeps working completely for its own members: their forum, their grants, their notifications. The people in it are not being punished for a name their leader chose. That asymmetry -- a false positive costs a human glance, a false negative costs an impersonated staff page -- is what lets the matcher be conservative. It is not licence to be sloppy the other way: a check that fires on "Badminton" gets switched off, and then the real cost is paid in full. So matching is whole WORDS after normalisation, never substrings, following the precedent scripts/checkModuleIdentifiers.js set for exactly this reason. Three matcher gaps found by writing the tests, all real impersonation vectors: - "Guild of Moderators" did not match `moderator`. Only a trailing s off the WHOLE term is stripped, so "Nomads" still does not match `mod`. - "G.M." normalises to two single-letter words and matched nothing. A run of two or more single-letter words is now also offered joined. Deliberately not a whole-name condensation, which would re-admit substring matching. - The multi-word condensed form was already handled and is what makes "RunicGateway" match the two-word term -- the form an impersonator would reach for, since it is what the Gitea org and every URL use. Terms resolve at CHECK time, never baked in, so renaming a deployment protects the new name without a redeploy. A failed settings read falls back to the static role and project terms rather than to an empty list: screening fewer terms is bad, screening none is the whole hole. Re-screening runs on every reconcile, over names no human has ruled on. Names are immutable per row, so it only ever changes an outcome when the TERM LIST changed -- an operator adding one, or a rename -- which is exactly what a create-time-only check would miss forever. `name_reviewed_at` is what makes a staff decision sticky; without it an override would be undone every fifteen minutes. The gate is scoped to three actions because they publish untrusted game-sourced strings, and to nothing else. Ordinary forum grants, leadership overrides, archives and forum moderation still apply immediately and are audited. A moderator initiating one files a pending request; an admin applies at once. Never four-eyes on admins: users.role defaults to admin and `npm run seed` creates exactly one, so most deployments have precisely one and a second-approver rule would wedge them with no way out. Hiding is deliberately NOT gated. Publishing untrusted data needs a second pair of eyes; withdrawing it needs to be possible at once, by whoever is on duty. Two concurrency details worth the review: a decision moves the row out of `pending` under a guard and applies its effect only if the row actually moved, so two admins clicking approve cannot double-apply or overwrite each other's record; and a JSON payload is parsed defensively, because the driver returns JSON columns already parsed on some versions and as a string on others. Screening is stubbed in the reconciler's own tests -- it is a separate unit, and the real call reads settings, which this suite must never do against a live database. That was caught the hard way: the suite went from 11s to hanging, and the cause was the reconciler reaching a dead pool through the new call. 44 tests in the reconciler file (up from 39), 19 for the matcher, 25 for the gate. Full suite 877 passed, 0 failed. Refs docs/website/TEAMS.md §2.8, §2.9, Part 12 phase 2 Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-17 15:08:58 -05:00
wtclaude	bfd844e8fb	feat(teams): the four-path access resolver and staff leadership overrides ... The four authority paths of docs/website/TEAMS.md §2.5, and the rule that they stay four: four tables answering four questions, and no resolver reads another path's table. 1. Is this account a member? module team_members 2. Does this account lead the Team? module team_members.is_leader + override 3. May it use the Team forum? CORE team_forum_grants OR path 1 4. May it get external access? CORE derived, nothing of its own The temptation this resists is collapsing 1 and 3 into one boolean. They answer different questions about different populations: a forum grant may name any Runic Gateway account, including one with no game identity at all -- that is the point of it, since letting an unlinked guildmate into a forum must not require a staff ticket. Reading "has forum access" as "is a member" would put that person on the public roster, into every membership count, and into the external-platform grant, which is where a modelling preference becomes an impersonation risk. Path 4 is deliberately blind to path 3, and the reason is written down so nobody "fixes" it: an integration cannot verify that an unlinked, forum-granted account corresponds to a real game member, so it must not hand that account a privilege on a platform where impersonation has consequences. A forum is a room on the operator's own site with a known moderator; a Discord role is an identity claim in someone else's space. Leadership overrides are applied ON TOP of the synced value at read time, never written into the projection. The sync owns that column and rewrites it every interval, so an override stored there would be undone fifteen minutes after staff set it -- which is the whole reason §2.5.1 is a separate table. The roster carries both the resolved answer and `is_leader_synced`, so an admin sees that a decision was made rather than being shown it as fact. Three tests are named INVARIANT rather than for behaviour, because what they protect is structural and a reasonable-looking refactor destroys it silently: a grant never writes the membership projection, a granted user is absent from the roster, and a grant does not confer external eligibility. None of those failures appears on a screen as a bug -- the first shows up as a stranger on a public roster, the second as a Discord role handed to an account nobody can tie to a real player. Every unit test here stubs the db layer, so the SQL itself was verified separately: all 44 statements across teams.db.js and teamAccess.db.js were run against MariaDB 11 with a throwaway module id and cleaned up after. That run also confirmed live what the reconciler's tests could only assert against a stub -- an upsert does not overwrite is_leader, a revoked grant frees the unique key for a new one while the ledger keeps both, and an archived team stays resolvable at its old slug while its external_id is free for the successor row. 19 tests. Full suite 828 passed, 0 failed. Refs docs/website/TEAMS.md §2.5, §2.5.1, §2.6, Part 12 phase 2 Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-17 14:57:23 -05:00
wtclaude	92631347f9	feat(teams): the reconciler, its four refusal gates, and ctx.teams (API 1.6.0) ... Core's projection of the module's Teams, kept in step (docs/website/TEAMS.md §2.4), plus the two ctx members a module pushes through. The four gates are the file, and each is invariant 1 in a different costume -- module unavailability is staleness, never emptiness: 1. getTeams() not ok -> record the failure, touch NOTHING, return. 2. ok but empty, core holds >=1 -> quarantine; apply only if the NEXT authoritative answer, an interval later, agrees. 3. getTeamMembers() not ok -> that Team's roster untouched and stale; the other Teams sync normally. 4. ok but zero members, had some -> the same two-strikes quarantine, per Team. Gates 2 and 4 exist because an authoritative-looking empty answer during a cold start is the one failure indistinguishable from a real wipe. "Every Team on the shard disbanded at once" costs one interval to confirm; getting it wrong empties every roster on the site. Events are an optimisation, never the source of truth. Member and leadership deltas apply at once for a Team core already knows; team.created and team.disbanded only ask for a run. §2.2 scopes archival to an authoritative full list, so a repeated or spurious disband event costs a reconcile rather than a Team -- and a Team invented from a delta would have no name, no roster and no leaders anyway. Two columns TEAMS.md did not contemplate, both on `teams`: - roster_synced_at, because team_sync_state holds one row per MODULE and gate 3 leaves ONE Team behind while the others sync. Without a per-Team stamp that Team's page would report the module's last success as its own -- exactly the staleness the gate exists to surface. - members_empty_since, gate 4's per-Team quarantine. The twin of team_sync_state.pending_empty_since, which is per module and cannot express it. One real bug found by its own test. The roster upsert was writing is_leader, so a refused getTeamLeaders() left every member demoted -- the roster had already written `leader: false` before the authoritative call was even made. §2.5 is explicit that path 2 is answered by getTeamLeaders(), so is_leader is now set on INSERT only (seeding a Team so it is not leaderless while that call fails) and moved afterwards by setLeaders() alone. Two writers for one column was the whole defect. MODULE_API_VERSION 1.6.0 on both halves -- they state one contract and a module declares one coreApi range. The number covers the whole Team surface per Part 11; the members arrive by phase. registerTeamProvider, ctx.teams.publish and ctx.teams.reconcile are live. ctx.teams.activity.push (§4, phase 3) and api.registerSlashCommands (§7.1, phase 7) are present and THROW with a sentence naming their phase, rather than being absent or silently accepting data into tables that do not exist yet. 39 tests here, and the ctx surface guard in moduleLoader.test.js updated -- it caught the addition, which is what it is for. Server 809 passed, client 192 passed, 0 failed. Refs docs/website/TEAMS.md §2.2, §2.3, §2.4, Part 11, Part 12 phase 2 Co-Authored-By: Claude <noreply@anthropic.com>	2026-08-17 14:53:52 -05:00
wtclaude	8b63ffc725	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