From dc135159276ce26275428793e74baf3bc7d71733 Mon Sep 17 00:00:00 2001 From: wtclaude Date: Fri, 4 Sep 2026 14:57:26 -0500 Subject: [PATCH] feat(events): send the idempotency key, and declare champ.boss.killed (Phase 11a) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The website's half of protocol 6. Every event-driven write now carries the step's idempotency key, and `uo.broadcast` stops being un-retryable. Phase 9 shipped it answering `retry: false` to everything including a 503 from a shard that was merely restarting, with a comment naming the line that would change when the wire could refuse a repeat. This is that line: it defers to `sidecarFailure`, the same helper its two siblings already used, so the hand-rolled variant that forced every outcome terminal is gone rather than re-tuned. One verb was less idempotent than its own id made it look. Both keyed verbs post under a run-scoped id and a repeat replaces — but `news.add` with `announce: true` makes the criers proclaim the title on every post, so a retry replaced the article silently and proclaimed it again. The key stops the second proclamation. `champ.boss.killed` is mapped to the `champs` feature (rule 2 would otherwise fail it closed to admin), with `damagers` a nested `staff` field rule: the kill is public because a champion falling is what the board is for, the ranked roll of who was strong enough to fell it is not. `uo.champ.boss_killed` is declared as a trigger — which is what makes it usable as an event PHASE CONDITION, since a condition is written over a trigger firing — and it carries `damagerCount`, never a damager name, because a trigger variable reaches mail an operator may address to every subscriber. Its seeded rule is its own group, `champ-boss-killed-v1`: `triggers-v1` is stamped once under a settings guard, so appending a 27th entry would have reached fresh installs and nothing else. It also ships email+inapp and NOT push, and the comment says why — no trigger in this module is also a registered stream, so no engagement rule here can push. That is pre-existing in twenty rules and flagged rather than fixed; this one declines to be the twenty-first. Co-Authored-By: Claude --- server/config/engagementSeeds.js | 95 +++++++++++++++++++++++++++-- server/config/shardTriggers.js | 36 +++++++++++ server/config/uoEventActions.js | 89 ++++++++++++++++++--------- server/test/engagementSeeds.test.js | 20 ++++-- server/test/shardEngagement.test.js | 83 ++++++++++++++++++++++++- server/test/shardVisibility.test.js | 67 +++++++++++++++++++- server/test/uoEventActions.test.js | 59 +++++++++++++----- server/utils/shardEngagement.js | 47 ++++++++++++++ server/utils/shardVisibility.js | 26 +++++++- server/utils/uoLinkClient.js | 43 +++++++++++-- 10 files changed, 501 insertions(+), 64 deletions(-) diff --git a/server/config/engagementSeeds.js b/server/config/engagementSeeds.js index 7548123..8c06444 100644 --- a/server/config/engagementSeeds.js +++ b/server/config/engagementSeeds.js @@ -467,6 +467,38 @@ const TEMPLATES = [ '{{champsUrl}}', ), + // ── The champion falls (Protocol 6) ───────────────────────────────────── + // + // The other half of the pair above, and the half the wire could not report + // until protocol 6 gave the shard a kind for it. Written as the crier's own + // follow-up: the same voice that announced the champion walking is the one + // that reports it did not walk far. + // + // `{{damagerNote}}` is a single-token block, so an unattributed kill renders + // the paragraph without it rather than as a sentence with a hole in it. + email( + 'uo.champ.boss-killed', + 'Champion spawn — the champion falls (town crier)', + 'uo.champ.boss_killed', + 'Hear ye — {{bossName}} has fallen', + [ + heading('h', 'Hear ye, hear ye'), + text('p1', + '{{bossName}} has fallen{{atPlace}}.{{damagerNote}} The altar is quiet again, and it ' + + 'will not stay quiet.'), + button('cta', 'See the altars', '{{champsUrl}}'), + ], + ), + inapp( + 'uo.champ.boss-killed-inapp', + 'Champion spawn — the champion falls (in-app)', + 'uo.champ.boss_killed', + '{{bossName}} has fallen', + '{{bossName}} has fallen{{atPlace}}.{{damagerNote}}', + 'See the altars', + '{{champsUrl}}', + ), + // ── A guildmaster of the craft ────────────────────────────────────────── email( 'uo.skill.capped', @@ -624,6 +656,14 @@ const TEMPLATES = [ const CHANNELS_OWNER = ['email', 'inapp'] const CHANNELS_BROADCAST = ['email', 'inapp', 'push'] +// The same two channels as CHANNELS_OWNER and a different reason for them: a +// rule that goes to every subscriber but cannot be PUSHED, because push is +// keyed on a subscription id and no trigger in this module is also a registered +// stream. Same value, different fact — folding them into one constant would lose +// the distinction the moment somebody added push to whichever one they read as +// "the broadcast-ish list". See `uo.champ.boss_killed`. +const CHANNELS_CONTENT = ['email', 'inapp'] + /** In-universe: both bodies are this module's, the digest is core's. */ const bodies = (key) => ({ email: `uo.${key}`, @@ -863,6 +903,32 @@ const RULES = [ cooldown_seconds: 1800, max_sends_per_hour: 1000, }, + { + trigger_id: 'uo.champ.boss_killed', + name: 'Champion spawn — the champion falls', + audience: 'subscribers', + // **`CHANNELS_CONTENT`, not `CHANNELS_BROADCAST`** — this is the one rule in + // the file that leaves push out, and it is not an oversight. + // + // Push delivery is keyed on the SUBSCRIPTION id, and a subscription row only + // ever exists for an id the preferences screen offered a push toggle for — + // which core's catalog grants to registered STREAMS and nothing else. This + // module's stream ids (`champ.start`, `idoc.warning`, …) and its trigger ids + // (`uo.champ.started`, …) are disjoint sets, so no trigger here can be pushed + // through the engagement path at all: the tickle resolves to zero endpoints + // while the send log records it delivered. + // + // That is true of every sibling rule above and is a pre-existing defect, not + // one this rule introduces. What this rule declines to do is add a + // twenty-first instance of it. See EVENTS_PLAN.md Phase 11a. + channels: CHANNELS_CONTENT, + template_keys: bodies('champ.boss-killed'), + // The same half-hour as its `boss_up` twin, and on the SAME subject — the + // spawn — so an altar that pops and is cleared inside the window produces the + // walk or the fall, not both. + cooldown_seconds: 1800, + max_sends_per_hour: 1000, + }, { trigger_id: 'uo.server.up', name: 'Shard — came online', @@ -960,10 +1026,29 @@ const RULES = [ }, ] -const RULE_GROUPS = [{ - key: 'triggers-v1', - note: 'UO notifications stay off until an operator enables one', - rules: RULES, -}] +// A group is seeded ONCE, under its own settings guard. So a rule appended to an +// existing group reaches fresh installs and nothing else: every deployment that +// has already stamped `triggers-v1` is done with it forever, and the new rule +// would silently never arrive. That is Engagement Phase 11's seed-key finding, +// and core applied the same remedy in Events Phase 10 — a NEW key per addition, +// never an edit to an old one. +// +// So protocol 6's `uo.champ.boss_killed` rule ships as its own group rather than +// as a twenty-seventh entry above. `RULES` remains the whole declared set, which +// is what the "every declared trigger has exactly one rule" invariant reads. +const BOSS_KILLED = RULES.filter((r) => r.trigger_id === 'uo.champ.boss_killed') + +const RULE_GROUPS = [ + { + key: 'triggers-v1', + note: 'UO notifications stay off until an operator enables one', + rules: RULES.filter((r) => !BOSS_KILLED.includes(r)), + }, + { + key: 'champ-boss-killed-v1', + note: 'The champion-falls notice, added with protocol 6; off like every other', + rules: BOSS_KILLED, + }, +] module.exports = { TEMPLATES, RULES, RULE_GROUPS } diff --git a/server/config/shardTriggers.js b/server/config/shardTriggers.js index 5b73676..6d231d6 100644 --- a/server/config/shardTriggers.js +++ b/server/config/shardTriggers.js @@ -600,6 +600,42 @@ const COME_ONLINE = [ description: 'A trailing fragment, LEADING SPACE included, or empty when the frame carries no location.' }, ], }, + { + // Protocol 6, and the reason the kind exists at all. Its first consumer is not + // a mail rule but an EVENT PHASE CONDITION: `{ on: 'uo.champ.boss_killed', + // where: [...], count: 1 }` is how an author says "move to the next phase when + // the boss falls", and a condition is expressed over a trigger firing. That is + // also why it is declared here rather than only ingested — a kind nothing + // declares is a kind no event can wait on. + id: 'uo.champ.boss_killed', + label: 'A champion boss was defeated', + description: 'Players brought down a champion spawn boss.', + kind: 'event', + subjectKey: 'spawnSerial', + audience: 'subscribers', + ceiling: 'authenticated', + version: V1, + variables: [ + { name: 'spawnSerial', type: 'string', required: true, example: '0x40012345', + description: 'The spawn controller, or the boss itself where the shard could not name an altar. Also the cooldown subject.' }, + { name: 'bossName', type: 'string', required: true, example: 'Semidar', + description: 'The boss that fell.' }, + { name: 'category', type: 'string', required: false, example: 'champion', + description: 'champion or sea.' }, + { name: 'location', type: 'string', required: false, example: 'Felucca 5187, 570 (Destard)', + description: 'Where, already formatted for reading.' }, + { name: 'killerName', type: 'string', required: false, example: 'Aldric', + description: 'Who struck the last blow, when the shard names one.' }, + { name: 'damagerCount', type: 'int', required: false, example: 14, + description: 'How many players did damage to it. The names themselves are staff-only and are deliberately not offered here.' }, + { name: 'damagerNote', type: 'string', required: false, example: ' 14 players fought it.', + description: 'A trailing sentence, LEADING SPACE included, or empty when nobody is credited.' }, + { name: 'champsUrl', type: 'url', required: false, example: '/uo/champs', + description: 'Site-relative path to the champions page.' }, + { name: 'atPlace', type: 'string', required: false, example: ' at Felucca 1480, 1600 (Destard)', + description: 'A trailing fragment, LEADING SPACE included, or empty when the frame carries no location.' }, + ], + }, { id: 'uo.server.up', label: 'The shard came online', diff --git a/server/config/uoEventActions.js b/server/config/uoEventActions.js index c73c0a4..e4e7c3d 100644 --- a/server/config/uoEventActions.js +++ b/server/config/uoEventActions.js @@ -24,14 +24,31 @@ // // That is not a tuning detail. It is the whole of what makes rule 2 true. // -// **2. A broadcast is attempted exactly once.** `on_failure` is what happens -// AFTER `EVENT_STEP_MAX_ATTEMPTS` retries, and `skip` — the default for `notify` -// — is a disposition, not a retry policy; there is no per-action lever that says -// "do not retry me". The lever a module HAS is the failure envelope, so -// `uo.broadcast` answers `retry: false` to everything. A retried broadcast is a -// second announcement to everyone online, and there is no idempotency key on the -// wire until Phase 11 to make the shard refuse the repeat. A lost announcement is -// cheaper than a doubled one. +// **2. A broadcast is retried, and protocol 6 is what changed that.** Wave 1 +// shipped `uo.broadcast` answering `retry: false` to everything, because a +// retried broadcast was a second announcement to everyone online and nothing on +// the wire could make the shard refuse the repeat. A lost announcement was +// cheaper than a doubled one, and that was the whole argument. +// +// Protocol 6 removes its premise. Every write below now carries the step's +// `idempotencyKey`; the shard executes a key at most once and answers a repeat +// with the ORIGINAL reply rather than re-running it. So a retry of a broadcast +// whose acknowledgement was lost cannot announce twice — it collects the answer +// the first attempt never delivered. A shard restarting mid-run is now recovered +// from rather than written off, which is the case rule 2 used to throw away +// knowingly. +// +// Rule 1 is what keeps this true rather than merely intended: if core's deadline +// fired first the module would never be asked, and the retry would be core's +// unconditional one — carrying the same key, so still safe, but classified +// without the module's judgement. + +// **2a. The one status that is new here.** A repeat arriving while the original +// is still in flight on the shard is answered `bridge.busy`, which the sidecar +// maps to **425**. It is transient by construction: the work is happening. It is +// not in `PERMANENT_STATUSES` and `classify()` falls through to retry, so it +// needs no arm of its own — but it is named so that a future tightening of that +// list has to decide about it deliberately. // // **3. What a shard restart wipes, `reconcile()` reports gone — and it knows // which restart it was without asking.** There is no "list the town-crier lines" @@ -227,7 +244,7 @@ const ACTIONS = [ }, ], - async perform({ runId, params, verify }) { + async perform({ runId, idempotencyKey, params, verify }) { const text = String(params.text == null ? '' : params.text).trim() // Checked here rather than left to the sidecar's 400, so the DRY RUN shows // the author the refusal — which is the whole point of having one. @@ -250,29 +267,25 @@ const ACTIONS = [ actor: `event:${runId}`, text, hue: params.hue === undefined || params.hue === null ? undefined : Number(params.hue), + // Protocol 6, and the line rule 2 said would change. The key is the + // step's, so every attempt at this step carries the same one and the + // shard refuses the repeat — which is what makes the retry below safe to + // ask for at all. + idempotencyKey, }) if (result.ok) return { ok: true } - // **Every failure is terminal, deliberately** — see rule 2 in the header. - // A 503 from a shard that is merely restarting IS transient and this throws - // that retry away; that is the trade, taken knowingly, because the failure - // this refuses to risk is announcing twice to everyone online. Phase 11 - // puts an idempotency key on the wire and this line is what changes. + // **A transient failure is now retried**, where wave 1 gave up on it. What + // used to make a retry unsafe was that the shard could not tell a repeat + // from a fresh command; it can now, so a 503 from a shard that is merely + // restarting is recovered from instead of being written off. // - // **The clause is only added where a retry was genuinely given up**, and - // the rig is what made that distinction matter. A 403 — the shard's admin - // write plane switched off — will not succeed on any attempt, so telling an - // operator it was "not retried because a repeat would announce twice" points - // them at a policy decision when what they need is the sentence the shard - // already wrote: "admin write plane disabled". A reason that explains the - // wrong thing is worse than a bare status code. - const reason = sidecarReason(result, 'broadcast') - if (PERMANENT_STATUSES.has(result.status)) return { ok: false, retry: false, error: reason } - return { - ok: false, - retry: false, - error: `${reason} (not retried: a repeat would announce twice)`, - } + // The classification itself is `sidecarFailure`'s — the announce leg's own + // judgement about this transport, deferred to rather than second-guessed, + // exactly as the two keyed verbs below already do. That this action now + // uses the SAME helper as its siblings, instead of a hand-rolled variant + // that forced every outcome terminal, is most of the change here. + return sidecarFailure(result, 'broadcast') }, }, @@ -329,7 +342,18 @@ const ACTIONS = [ const id = resourceId(idempotencyKey) const bootId = await currentBootId() - const result = await uoLinkClient.postTownCrier({ id, lines: parsed.lines, durationSec }) + // Protocol 6. This verb was already safe to retry — a repeat under the same + // `id` REPLACES the crier entry rather than stacking a second one — so the + // key buys no new safety here. It is sent because it costs nothing and + // makes the retry a no-op on the shard rather than a redundant world write, + // and because a write plane where only some commands are keyed is one + // somebody will later have to reason about per verb. + const result = await uoLinkClient.postTownCrier({ + id, + lines: parsed.lines, + durationSec, + idempotencyKey, + }) if (!result.ok) return sidecarFailure(result, 'town-crier post') // The stamp rule 3 rests on. `runId` rides along so a row read out of the @@ -435,6 +459,13 @@ const ACTIONS = [ params.image === undefined || params.image === null ? undefined : Number(params.image), url: params.url || undefined, announce: params.announce === undefined || params.announce === null ? true : Boolean(params.announce), + // Protocol 6, for the same reason the crier carries one — except that + // here it does buy something. `announce: true` makes the criers proclaim + // the article's title when it is posted, so a re-post under the same id + // replaces the article silently but proclaims it AGAIN. The key stops the + // second proclamation, which was the one part of this verb that was never + // as idempotent as its `id` made it look. + idempotencyKey, }) if (!result.ok) return sidecarFailure(result, 'news article') diff --git a/server/test/engagementSeeds.test.js b/server/test/engagementSeeds.test.js index a0f66ed..508fd41 100644 --- a/server/test/engagementSeeds.test.js +++ b/server/test/engagementSeeds.test.js @@ -96,8 +96,10 @@ test('the seventeen in-universe families have both channels; the nine plain ones // letter from anybody. assert.equal(r.template_keys.digest, 'notify.digest', `${r.trigger_id} digests generically`) } - assert.equal(bespoke, 17) - assert.equal(seeds.TEMPLATES.length, 34) + // Eighteen since protocol 6: the champion FALLS, in the same crier's voice as + // the champion walking, because they are one story told in two mails. + assert.equal(bespoke, 18) + assert.equal(seeds.TEMPLATES.length, 36) }) test('a template key is core\'s grammar — dots and hyphens, never an underscore', () => { @@ -222,7 +224,7 @@ test('every declared fragment carries an example that shows its own shape', () = // The `example` is what the template editor previews and test-sends with, so a // trailing fragment whose example omits the leading space teaches an author the // wrong thing about where to put one. - const TRAILING = ['slainBy', 'atPlace', 'inSuccessionTo', 'candidateNote'] + const TRAILING = ['slainBy', 'atPlace', 'inSuccessionTo', 'candidateNote', 'damagerNote'] for (const t of TRIGGERS) { for (const v of t.variables.filter((x) => TRAILING.includes(x.name))) { assert.ok(v.example.startsWith(' '), `${t.id}.${v.name} example leads with its space`) @@ -236,7 +238,17 @@ test('one rule group, and appending to it later would reach fresh installs only' // A group is seeded ONCE under its own settings guard, which is 11a's seed-key // finding as a mechanism. This assertion exists so that adding a twenty-sixth // rule has to edit a test whose name says what appending costs. - assert.equal(seeds.RULE_GROUPS.length, 1) + // TWO groups since protocol 6, and the second one is this test's whole point + // made concrete: `uo.champ.boss_killed` could not be appended to `triggers-v1`, + // because a deployment that has already stamped that key would never have + // received it. A new rule gets a new key. + assert.equal(seeds.RULE_GROUPS.length, 2) assert.equal(seeds.RULE_GROUPS[0].key, 'triggers-v1') assert.equal(seeds.RULE_GROUPS[0].rules.length, 26) + assert.equal(seeds.RULE_GROUPS[1].key, 'champ-boss-killed-v1') + assert.deepEqual(seeds.RULE_GROUPS[1].rules.map((r) => r.trigger_id), ['uo.champ.boss_killed']) + // No rule belongs to two groups, and between them they are the whole set. + const grouped = seeds.RULE_GROUPS.flatMap((g) => g.rules.map((r) => r.trigger_id)) + assert.equal(new Set(grouped).size, grouped.length) + assert.deepEqual([...grouped].sort(), seeds.RULES.map((r) => r.trigger_id).sort()) }) diff --git a/server/test/shardEngagement.test.js b/server/test/shardEngagement.test.js index 06aa935..b7f46fa 100644 --- a/server/test/shardEngagement.test.js +++ b/server/test/shardEngagement.test.js @@ -30,7 +30,11 @@ const one = (event) => { // ── The catalogue itself ─────────────────────────────────────────────────── test('the declared set is the one ENGAGEMENT.md §8.6 commits to, carve-outs included', () => { - assert.equal(TRIGGERS.length, 26) + // 27 since protocol 6: `uo.champ.boss_killed` joins the twenty-six §8.6 named. + // It is not one of the four carve-outs below being reinstated — it is a row the + // catalogue could not have, because until protocol 6 the wire had no kind for a + // boss defeat and the inference from `champ.update` was not good enough to mail. + assert.equal(TRIGGERS.length, 27) // The four rows that do NOT ship, each with its reason recorded in §8.6. This // assertion is the guard on the carve-outs: adding one back is a decision, and // a decision should have to edit a test that says so. @@ -113,6 +117,10 @@ test('every url variable a body can interpolate is actually SUPPLIED', () => { 'uo.election.opened': [city(), city({ electionPhase: 'nominate', autoPickAt: inHours(48), candidates: 2 })], 'uo.champ.started': [champ({ active: false }), champ({ active: true })], 'uo.champ.boss_up': [champ({ bossUp: false }), champ({ bossUp: true })], + // Protocol 6. A single frame, unlike its two neighbours: a defeat is an + // EVENT on the wire rather than a change spotted between two snapshots, which + // is the whole reason the kind was worth a protocol bump. + 'uo.champ.boss_killed': bossKilled(), 'uo.server.up': { kind: 'server.hello', shard: 'Rig' }, 'uo.server.down': { kind: 'server.shutdown' }, 'uo.page.new': { kind: 'page.new', type: 'Bug', sender: { name: 'Darrow' }, message: 'stuck' }, @@ -320,6 +328,21 @@ test('the pre-decision attempt kind is not mapped at all', () => { const champ = (over) => ({ kind: 'champ.update', serial: '0x40012345', name: 'Abyss', category: 'champion', map: 'Felucca', x: 5187, y: 570, ...over }) +// Protocol 6. The spawn serial matches `champ`'s, so the pair can be walked as +// one altar's story: the boss goes up, then it comes down. +const bossKilled = (over) => ({ + kind: 'champ.boss.killed', + serial: '0x40012345', + bossSerial: '0x901', category: 'champion', boss: 'Semidar', bossType: 'Semidar', + map: 'Felucca', x: 5187, y: 570, region: 'Destard', + killer: { serial: '0x55', name: 'Aldric', acct: 'seed_002', player: true }, + damagers: [ + { serial: '0x55', name: 'Aldric', acct: 'seed_002', player: true, damage: 900 }, + { serial: '0x56', name: 'Bran', acct: 'seed_003', player: true, damage: 120 }, + ], + ...over, +}) + test('a first sighting is never a transition — a reconnect is not twenty spawns starting', () => { assert.deepEqual(ids(champ({ active: true })), []) assert.deepEqual(ids(champ({ active: true })), []) // still no change @@ -339,6 +362,64 @@ test('champ.remove forgets the spawn, so its next appearance is a first sighting assert.deepEqual(ids(champ({ active: true })), []) }) +// ── champ.boss.killed (Protocol 6) ───────────────────────────────────────── + +test('a defeat fires on the frame itself, with no baseline to compare against', () => { + // Unlike its two neighbours above. `champ.update` is a SNAPSHOT, so a first + // sighting can never be a transition; a defeat is an event, so a first sighting + // is exactly the thing being reported. + const hit = one(bossKilled()) + assert.equal(hit.triggerId, 'uo.champ.boss_killed') + assert.equal(hit.data.bossName, 'Semidar') + assert.equal(hit.data.killerName, 'Aldric') + assert.equal(hit.data.damagerCount, 2) + assert.equal(hit.data.damagerNote, ' 2 players fought it.') + assert.equal(hit.data.location, 'Felucca 5187, 570 (Destard)') +}) + +test('the subject is the SPAWN, so boss_up and boss_killed share one cooldown subject', () => { + map(champ({ active: true, bossUp: false })) + const up = one(champ({ active: true, bossUp: true })) + const down = one(bossKilled()) + assert.equal(up.triggerId, 'uo.champ.boss_up') + assert.equal(down.data.spawnSerial, up.data.spawnSerial) +}) + +test('a defeat the shard could not attribute to an altar stands on the boss itself', () => { + // The sweep learns which altar a champion belongs to; a boss that popped and + // died between two sweeps arrives with no `serial`. A subject that exists once + // is all a cooldown needs, so the boss's own serial stands in rather than the + // firing being dropped. + const hit = one(bossKilled({ serial: undefined })) + assert.equal(hit.data.spawnSerial, '0x901') +}) + +test('a defeat clears the tracker, so the next boss on that altar is a transition again', () => { + map(champ({ active: true, bossUp: false })) + map(champ({ active: true, bossUp: true })) // fires boss_up + map(bossKilled()) + // Without the tracker reset this would emit nothing: the tracker would still + // believe a boss is up, so the next one would not look like a change. + assert.deepEqual(ids(champ({ active: true, bossUp: true })), ['uo.champ.boss_up']) +}) + +test('the damage TABLE never becomes trigger data, only its size', () => { + // `damagers` is `staff` in the visibility config. A trigger variable is + // interpolated into mail an operator may address to every subscriber, so a + // damager name reaching `data` would undo that field rule one layer up. + const hit = one(bossKilled()) + const rendered = JSON.stringify(hit.data) + assert.equal(rendered.includes('Bran'), false, 'no damager name reaches the data') + assert.equal(rendered.includes('seed_003'), false, 'no damager account reaches the data') + assert.equal(hit.data.damagers, undefined) +}) + +test('an unattributed kill renders no damager sentence rather than an empty one', () => { + const hit = one(bossKilled({ damagers: [] })) + assert.equal(hit.data.damagerCount, undefined) + assert.equal(hit.data.damagerNote, undefined) +}) + const city = (over) => ({ kind: 'city.update', city: 'Britain', electionPhase: 'none', ...over }) test('a governor change is a transition, and never on first sight', () => { diff --git a/server/test/shardVisibility.test.js b/server/test/shardVisibility.test.js index 771e874..a6e4dc5 100644 --- a/server/test/shardVisibility.test.js +++ b/server/test/shardVisibility.test.js @@ -95,6 +95,58 @@ test('an unknown viewer level cannot see a gated kind or a locked field', async assert.equal('webId' in out.leader, false) }) +// ── Protocol 6: the champion defeat ────────────────────────────────── + +const KILL = { + kind: 'champ.boss.killed', + serial: '0x40012345', + boss: 'Semidar', + killer: { serial: '0x55', name: 'Aldric', acct: 'seed_002', player: true }, + damagers: [ + { serial: '0x55', name: 'Aldric', acct: 'seed_002', webId: '7', player: true, damage: 900 }, + { serial: '0x56', name: 'Bran', acct: 'seed_003', player: true, damage: 120 }, + ], +} + +test('the kill is public and its damage table is not', () => { + const config = visibility.compileDefaults() + // The whole shape of this addition in one assertion: a champion falling is + // content the public board is FOR, and a ranked roll of who was strong enough + // to fell it is a performance record nobody published on purpose. + assert.equal(visibility.kindVisibleTo('champ.boss.killed', 'anonymous', config), true) + for (const level of ['anonymous', 'logged_in', 'player']) { + const out = visibility.projectFeature('champs', KILL, level, config) + assert.equal(out.boss, 'Semidar', `${level} sees which boss fell`) + assert.equal('damagers' in out, false, `${level} must not see the damage table`) + } + assert.equal(visibility.projectFeature('champs', KILL, 'staff', config).damagers.length, 2) +}) + +test('the killer rides the frame the way mob.killed already publishes one', () => { + // Deliberately NOT a configurable field. It is one actor, announced in-game to + // everyone present, and the same disclosure the public activity feed has made + // through `mob.killed` since before this framework existed. + const config = visibility.compileDefaults() + const out = visibility.projectFeature('champs', KILL, 'anonymous', config) + assert.equal(out.killer.name, 'Aldric') + assert.equal('acct' in out.killer, false, 'rule 1 still applies inside it') +}) + +test('an admin who lowers the damager rule still cannot see an account inside it', () => { + // Rule 1 beats a field rule wherever the two meet, and a damager entry is an + // actor object like any other. An admin who opens the table to everyone has + // published character names, which is what they chose; they have not published + // account names, which is not theirs to choose. + const config = visibility.compileDefaults() + config.champs.fields = { ...config.champs.fields, damagers: 'anonymous' } + const out = visibility.projectFeature('champs', KILL, 'anonymous', config) + assert.equal(out.damagers.length, 2) + assert.equal(out.damagers[0].name, 'Aldric') + assert.equal(out.damagers[0].damage, 900) + assert.equal('acct' in out.damagers[0], false) + assert.equal('webId' in out.damagers[0], false) +}) + // ── Rule 1: locked fields ────────────────────────────────────────────────── test('acct and webId are stripped below admin regardless of feature config', () => { @@ -360,10 +412,21 @@ const V3_ADDED_PUBLIC_KINDS = ['world.ruleset', 'points.board'] // inside the roster's member array (see the roster test above). const V4_ADDED_PUBLIC_KINDS = ['guild.roster', 'guild.leave'] -test('derived PUBLIC_KINDS is exactly the pre-v3 allowlist plus the v3 and v4 additions', () => { +// v6 adds the champion defeat. It rides the existing `champs` feature, which is +// already anonymous, so the KIND is public — while the `damagers` table on it is +// `staff` by field rule. That split is the point: a shard announces that its +// champion fell without publishing a roll of who was strong enough to fell it. +const V6_ADDED_PUBLIC_KINDS = ['champ.boss.killed'] + +test('derived PUBLIC_KINDS is exactly the pre-v3 allowlist plus the v3, v4 and v6 additions', () => { assert.deepEqual( [...visibility.PUBLIC_KINDS].sort(), - [...PRE_V3_PUBLIC_KINDS, ...V3_ADDED_PUBLIC_KINDS, ...V4_ADDED_PUBLIC_KINDS].sort(), + [ + ...PRE_V3_PUBLIC_KINDS, + ...V3_ADDED_PUBLIC_KINDS, + ...V4_ADDED_PUBLIC_KINDS, + ...V6_ADDED_PUBLIC_KINDS, + ].sort(), ) }) diff --git a/server/test/uoEventActions.test.js b/server/test/uoEventActions.test.js index 7fb22fe..cc838c6 100644 --- a/server/test/uoEventActions.test.js +++ b/server/test/uoEventActions.test.js @@ -133,30 +133,57 @@ test('a broadcast spends the one budget dimension the module declares', () => { assert.equal(byId('uo.news.post').cost, undefined) }) -// ── uo.broadcast: attempted exactly once ─────────────────────────────────── +// ── uo.broadcast: retried, because protocol 6 made that safe ─────────────── -test('a broadcast is never retried, whatever the sidecar says', async () => { +test('a broadcast is retried on a transient failure and never on a permanent one', async () => { const broadcast = byId('uo.broadcast') - // Every failure this transport can produce: no route to the sidecar, a data - // refusal, a bad token, a protocol mismatch, a shard that is not connected and - // a shard that timed out. The last two are genuinely transient, and this is - // the trade being taken knowingly — a lost announcement is cheaper than one - // delivered twice to everyone online. - for (const status of [0, 400, 401, 403, 409, 503, 504]) { + // Wave 1 asserted the opposite of this — every failure terminal, including the + // two that are plainly transient — because nothing on the wire could stop a + // retry announcing to everyone twice. Protocol 6 puts an idempotency key on the + // command and the shard refuses the repeat, so the trade that test recorded is + // no longer one that has to be made. + // + // 425 is the new status in this list: `bridge.busy`, the shard saying a command + // under this key is still in flight. Transient by construction. + const TRANSIENT = new Set([0, 425, 503, 504]) + for (const status of [0, 400, 401, 403, 409, 425, 503, 504]) { uoLinkClient.adminBroadcast = async () => ({ ok: false, status, error: `status ${status}` }) const result = await broadcast.perform({ runId: 7, params: { text: 'hear ye' }, verify: false }) assert.equal(result.ok, false) - assert.equal(result.retry, false, `a ${status} must not be retried`) - // The clause belongs only where a retry was genuinely given up. On a - // permanent status it would explain the wrong thing. - if (!actions.PERMANENT_STATUSES.has(status)) { - assert.match(result.error, /announce twice/, 'a discarded retry must say why') - } else { - assert.doesNotMatch(result.error, /announce twice/, `a ${status} was never retryable`) - } + assert.equal(result.retry, TRANSIENT.has(status), `a ${status} retries iff it is transient`) } }) +test('every write carries the step idempotency key, unchanged', async () => { + // The key is what makes the retry above safe, so a verb that dropped it would + // silently restore the wave-1 hazard while every other assertion still passed. + // Asserted per verb rather than once, because each builds its own body. + const KEY = 'a'.repeat(40) + const seen = {} + + uoLinkClient.adminBroadcast = async (body) => { seen.broadcast = body; return { ok: true } } + uoLinkClient.postTownCrier = async (body) => { seen.crier = body; return { ok: true } } + uoLinkClient.postNews = async (body) => { seen.news = body; return { ok: true } } + + await byId('uo.broadcast').perform({ + runId: 7, idempotencyKey: KEY, params: { text: 'hear ye' }, verify: false, + }) + await byId('uo.towncrier.post').perform({ + runId: 7, idempotencyKey: KEY, params: { lines: 'hear ye' }, verify: false, + }) + await byId('uo.news.post').perform({ + runId: 7, idempotencyKey: KEY, params: { title: 'A thing', body: 'happened' }, verify: false, + }) + + assert.equal(seen.broadcast.idempotencyKey, KEY) + assert.equal(seen.crier.idempotencyKey, KEY) + assert.equal(seen.news.idempotencyKey, KEY) + // The two keyed verbs post under an id DERIVED from the key. Both travel: the + // id is what makes a repeat replace, the key is what stops it re-announcing. + assert.equal(seen.crier.id, `evt-${KEY}`) + assert.equal(seen.news.id, `evt-${KEY}`) +}) + test("the shard's own words reach the run log, not just a status code", async () => { // **The rig found this.** The sidecar refuses a broadcast with // `{"reason":"admin write plane disabled"}` and `legError` looks for diff --git a/server/utils/shardEngagement.js b/server/utils/shardEngagement.js index 13914ee..783480e 100644 --- a/server/utils/shardEngagement.js +++ b/server/utils/shardEngagement.js @@ -662,6 +662,53 @@ const MAPPERS = { } }, + // Protocol 6. A boss defeat, which until now could only be GUESSED at from + // `champ.update` losing its `bossUp` — a signal that also fires when a spawn is + // reset by a GM, when a boss despawns, and when the sweep simply reconnects. + // This one fires on the death itself. + // + // **The subject is the SPAWN, so it matches `uo.champ.boss_up`'s.** A rule with + // a cooldown on one altar therefore counts a boss going up and that same boss + // coming down as the same subject, which is what an operator writing "not more + // than once an hour about Destard" means. A kill the shard could not attribute + // to an altar carries no spawn, so the boss's own serial stands in — it is a + // subject that exists exactly once, which is all a cooldown needs of it. + // + // **Damagers are not surfaced as variables.** The table is on the frame and it + // is `staff` in the visibility config; putting names into a trigger's data + // would route them into mail an operator can address to `subscribers`, which is + // the field rule undone one layer up. `damagerCount` is a number and says the + // thing worth saying: how many took part. + 'champ.boss.killed': (ev, tracker, out) => { + const spawnSerial = ev.serial == null ? null : String(ev.serial) + const bossSerial = ev.bossSerial == null ? null : String(ev.bossSerial) + const subject = spawnSerial || bossSerial + if (!subject) return + + // The board no longer has a boss on this altar. Kept in step with the sweep's + // own view so the next `champ.update` carrying `bossUp: true` is read as a + // transition rather than as more of the same. + if (spawnSerial) tracker.champBossUp.set(spawnSerial, false) + + const damagers = Array.isArray(ev.damagers) ? ev.damagers : [] + + out.push({ + triggerId: 'uo.champ.boss_killed', + data: defined({ + spawnSerial: subject, + champsUrl: PATHS.champs, + bossName: ev.boss || ev.bossType || 'the champion', + category: ev.category || undefined, + location: place(ev), + atPlace: trailing(place(ev), (p) => ` at ${p}`), + killerName: actorName(ev.killer), + damagerCount: damagers.length || undefined, + damagerNote: trailing(damagers.length || null, (n) => + n === 1 ? ' One player fought it.' : ` ${n} players fought it.`), + }), + }) + }, + 'champ.remove': (ev, tracker) => { if (ev.serial == null) return tracker.champActive.delete(String(ev.serial)) diff --git a/server/utils/shardVisibility.js b/server/utils/shardVisibility.js index 8f50510..de7c387 100644 --- a/server/utils/shardVisibility.js +++ b/server/utils/shardVisibility.js @@ -83,7 +83,27 @@ const FEATURES = { // ── Shipped before v3. Defaults reproduce the previous hardcoded behavior. ── status: { audience: 'anonymous', fields: {} }, activity: { audience: 'anonymous', fields: {} }, - champs: { audience: 'anonymous', fields: {} }, + // Protocol 6 adds `champ.boss.killed` to this feature, and with it the first + // field on a champs frame that is about PEOPLE rather than about an altar. + // + // `damagers` is the ranked table of who fought the boss and for how much. It is + // the honest basis for "who slew the champion" and it is also a performance + // record of named players that nobody consented to publish, which is precisely + // the tension the ladder exists to let a shard resolve for itself. It defaults + // to `staff`: the kill is public (a champion falling is announced in-world and + // is the content the board is for), the roll of who did the damage is not. A + // shard that wants a public board lowers one rule. + // + // Nested for the same reason `market.fees` and `houses.schedule` are: one rule + // covers the whole table rather than a rule per column, and the columns here + // are actor objects whose `acct`/`webId` remain admin-only by the locked-field + // rule regardless of what this is set to. + // + // `killer` is deliberately NOT listed. It is the single actor whose blow landed + // last, it is announced in-game to everyone present, and it is the same shape + // and the same disclosure `mob.killed` has published on the public activity + // feed since before this framework existed. + champs: { audience: 'anonymous', fields: { damagers: 'staff' } }, guilds: { audience: 'anonymous', fields: {} }, governors: { audience: 'anonymous', fields: {} }, // The public Houses page showed IDOC location only; owner/price were staff. @@ -189,6 +209,10 @@ const KIND_FEATURE = new Map( // boards 'champ.update': 'champs', 'champ.remove': 'champs', + // Protocol 6. Without this line rule 2 would fail the new kind closed to + // admin-only — correct as a default, and wrong as an outcome: a champion + // falling is exactly what the public board is for. + 'champ.boss.killed': 'champs', 'guild.update': 'guilds', 'guild.remove': 'guilds', 'guild.join': 'guilds', diff --git a/server/utils/uoLinkClient.js b/server/utils/uoLinkClient.js index 5233836..880b3df 100644 --- a/server/utils/uoLinkClient.js +++ b/server/utils/uoLinkClient.js @@ -11,6 +11,34 @@ // `X-UOLink-Version: ` so a protocol mismatch is caught (409) rather // than mis-parsed. Config is cached for a few seconds to avoid decrypting the // token on every call. +// +// ── Protocol 6: `idempotencyKey` on a write ──────────────────────────────── +// +// The three write helpers the event engine drives take an optional +// `idempotencyKey`, which the sidecar passes to the shard verbatim. The shard +// executes a key at most once and answers a repeat with the ORIGINAL reply, which +// is what makes retrying a world write safe — before it, a lost acknowledgement +// and a command that never applied were the same event seen from here. +// +// **A key is a function of the caller's unit of work, never of the attempt.** The +// event runner derives it from `sha256(runId|stepId)`, so every retry of one step +// carries the same key and a different step never collides with it. Passing a +// fresh value per call would satisfy the type and defeat the entire mechanism. +// +// **The DELETEs deliberately take no key.** Their idempotency is inherent — the +// second removal of a town-crier entry or a news article is a no-op the shard is +// already happy to perform — and the sidecar builds those commands from the path +// rather than from a body, so carrying one would be a protocol change bought for +// a guarantee that already holds. +// +// A caller that sends no key gets exactly the pre-protocol-6 behaviour, which is +// what leaves the admin screens (which send none, being driven by a human who can +// see whether the thing happened) unchanged. +// +// One new status can now come back from a keyed write: **425**, the sidecar's +// mapping of `bridge.busy` — a command under this key is still in flight on the +// shard. It is transient and retryable, and `shardAnnounce.classify` already +// treats it so by falling through to its retry case. const uoLinkConfig = require('../model/uoLinkConfig/uoLinkConfig.model') const log = require('../core').logger('uo-link-client') @@ -168,15 +196,18 @@ const createAccount = ({ actor, account, password, websiteUserId, ip }) => }) const unlinkAccount = ({ actor, account }) => call(`/link/${encodeURIComponent(account)}`, { method: 'DELETE', body: { actor } }) -const postTownCrier = ({ id, lines, durationSec }) => - call('/towncrier', { method: 'POST', body: { id, lines, durationSec } }) +const postTownCrier = ({ id, lines, durationSec, idempotencyKey }) => + call('/towncrier', { method: 'POST', body: { id, lines, durationSec, idempotencyKey } }) const deleteTownCrier = (id) => call(`/towncrier/${encodeURIComponent(id)}`, { method: 'DELETE' }) // Town Cryer News gump (Protocol 2.1). A full article (title/HTML body/image/URL) // in the in-game News window; re-posting the same id REPLACES it. `announce` // (default true on the sidecar) controls whether the criers proclaim the title. -const postNews = ({ id, title, body, image, url, announce }) => - call('/news', { method: 'POST', body: { id: String(id), title, body, image, url, announce } }) +const postNews = ({ id, title, body, image, url, announce, idempotencyKey }) => + call('/news', { + method: 'POST', + body: { id: String(id), title, body, image, url, announce, idempotencyKey }, + }) const deleteNews = (id) => call(`/news/${encodeURIComponent(id)}`, { method: 'DELETE' }) // ── Staff write plane (§6) ───────────────────────────────────────────────── @@ -189,8 +220,8 @@ const adminBan = ({ actor, account, serial, durationSec, reason }) => call('/admin/ban', { method: 'POST', body: { actor, account, serial, durationSec, reason } }) const adminUnban = ({ actor, account }) => call('/admin/unban', { method: 'POST', body: { actor, account } }) -const adminBroadcast = ({ actor, text, hue }) => - call('/admin/broadcast', { method: 'POST', body: { actor, text, hue } }) +const adminBroadcast = ({ actor, text, hue, idempotencyKey }) => + call('/admin/broadcast', { method: 'POST', body: { actor, text, hue, idempotencyKey } }) // ── Help-page (support) queue commands (§6) ──────────────────────────────── const respondPage = (pageId, { message, close }) => -- 2.49.1