diff --git a/server/db/schema.sql b/server/db/schema.sql index 4ce751b..b370e20 100644 --- a/server/db/schema.sql +++ b/server/db/schema.sql @@ -47,7 +47,7 @@ CREATE TABLE IF NOT EXISTS uo_link_config ( base_url VARCHAR(255) NULL, ws_url VARCHAR(255) NULL, auth_token_enc TEXT NULL, - protocol INT NOT NULL DEFAULT 3, + protocol INT NOT NULL DEFAULT 4, enabled TINYINT(1) NOT NULL DEFAULT 0, status VARCHAR(20) NOT NULL DEFAULT 'disconnected', status_detail VARCHAR(500) NULL, @@ -675,6 +675,35 @@ UPDATE uo_link_config SET protocol = 3 -- not cut over yet. INSERT IGNORE INTO settings (`key`, value) VALUES ('uo_link_protocol_3_migrated', '1'); +-- Protocol 4 cutover: the same migration one step later, and the one this module +-- OWED and did not pay. +-- +-- The protocol-4 work shipped across three repos — `link`'s PROTOCOL_VERSION, the +-- overlay's `overlay.toml`, and this module's `guild.roster` / `guild.leave` ingest — +-- but the pinned version stayed at 3 on both of its declaration sites here. A fresh +-- install therefore came up speaking 3 to a sidecar speaking 4, and a sidecar answers +-- a stale client with `409 protocol version mismatch` rather than mis-parsing it. The +-- symptom is total: every REST read fails and the WS closes on ws.hello, so a new +-- deployment shows an empty marketplace, an empty guild board and no shard status, +-- with the cause visible only in the server log. Found while standing up a demo +-- deployment for the marketing site's screenshots. +-- +-- Same shape as the block above, for the same reasons: MODIFY fixes the column +-- default for databases created before the bump, and the UPDATE is one-shot against +-- its own marker so that an operator who deliberately pins an older sidecar in +-- Admin → Shard stays pinned. `protocol < 4` and not `= 3`, so an install that +-- somehow never took the protocol-3 migration is carried the whole way rather than +-- one step. +ALTER TABLE uo_link_config MODIFY COLUMN protocol INT NOT NULL DEFAULT 4; +UPDATE uo_link_config SET protocol = 4 + WHERE id = 1 AND protocol < 4 + AND NOT EXISTS (SELECT 1 FROM settings WHERE `key` = 'uo_link_protocol_4_migrated'); +-- The marker is written HERE, in this module's fragment, for the reason spelled out +-- above: core's schema is replayed in full BEFORE any module fragment, so a marker +-- left in core would already exist when this UPDATE read it and the one-shot could +-- never fire. +INSERT IGNORE INTO settings (`key`, value) VALUES ('uo_link_protocol_4_migrated', '1'); + -- ── Settings rows this module owns ───────────────────────────────────────── -- -- Both keys predate the module system and both name a game concept, so core diff --git a/server/model/uoLinkConfig/uoLinkConfig.model.js b/server/model/uoLinkConfig/uoLinkConfig.model.js index 0e04e23..80ae4f3 100644 --- a/server/model/uoLinkConfig/uoLinkConfig.model.js +++ b/server/model/uoLinkConfig/uoLinkConfig.model.js @@ -10,7 +10,14 @@ const { secretBox } = require('../../core') // The wire protocol this build speaks (link/sidecar/src/main.rs PROTOCOL_VERSION). // Only used before an admin has saved anything — the stored row wins once it exists, // and UOLINK_PROTOCOL still overrides for an operator running an older sidecar. -const DEFAULT_PROTOCOL = Number(process.env.UOLINK_PROTOCOL) || 3 +// +// This says 4 because this build handles protocol 4's frames: `guild.roster` and +// `guild.leave` ingest landed with the Teams cutover. It said 3 for a while after +// that, which is the bug this constant is now the fix for — a FRESH install pinned +// 3, the sidecar answered `409 protocol version mismatch` to every REST call, and a +// new deployment read nothing from its shard until an admin edited the number by +// hand in Admin → Shard. See the matching cutover in db/schema.sql. +const DEFAULT_PROTOCOL = Number(process.env.UOLINK_PROTOCOL) || 4 function toSafe(row) { if (!row) { diff --git a/server/test/schemaFragment.test.js b/server/test/schemaFragment.test.js index cdf29c0..57352b4 100644 --- a/server/test/schemaFragment.test.js +++ b/server/test/schemaFragment.test.js @@ -121,7 +121,11 @@ test('every table this fragment declares is prefixed shard_ or uo_link_', () => // ── The settings rows this module owns ────────────────────────────────────── -const SETTINGS_KEYS = ['game_account_signup', 'uo_link_protocol_3_migrated'] +const SETTINGS_KEYS = [ + 'game_account_signup', + 'uo_link_protocol_3_migrated', + 'uo_link_protocol_4_migrated', +] test('both settings seeds are INSERT IGNORE, so a replay never resets a value', () => { for (const key of SETTINGS_KEYS) { @@ -131,6 +135,57 @@ test('both settings seeds are INSERT IGNORE, so a replay never resets a value', } }) + +// ── The protocol pin ──────────────────────────────────────────────────────── +// +// Two declaration sites and one constant have to agree, and for a while they did +// not: the protocol-4 cutover moved `link`, the overlay and this module's ingest, +// and left both pins here at 3. A fresh install then spoke 3 to a protocol-4 +// sidecar, which 409s every REST call — an install that reads nothing from its +// shard, with the cause only in the log. These tests are the guard. + +test('the column default pins the protocol this build speaks', () => { + const create = statements.find((s) => /CREATE TABLE.*uo_link_config/is.test(s)) + assert.ok(create, 'uo_link_config is gone') + assert.match( + create, + /protocol\s+INT\s+NOT NULL DEFAULT 4/i, + 'the CREATE TABLE default must name the protocol this build speaks', + ) + + // The last MODIFY wins on replay, so it is the one that decides an existing + // database's default. + const modifies = statements.filter((s) => + /^ALTER TABLE\s+uo_link_config\s+MODIFY COLUMN protocol/i.test(s), + ) + assert.ok(modifies.length > 0, 'the default-fixing MODIFY is gone') + assert.match(modifies[modifies.length - 1], /DEFAULT 4/i) +}) + +test('the protocol-4 marker is written AFTER the update that reads it', () => { + const update = statements.findIndex( + (s) => /^UPDATE\s+uo_link_config/i.test(s) && s.includes('uo_link_protocol_4_migrated'), + ) + const marker = statements.findIndex( + (s) => /^INSERT/i.test(s) && s.includes("'uo_link_protocol_4_migrated'"), + ) + assert.ok(update >= 0, 'the protocol-4 migration is gone') + assert.ok(marker >= 0, 'the one-shot marker is gone') + assert.ok(marker > update, 'the marker is written before the UPDATE reads it') +}) + +test('the protocol-4 one-shot carries an install forward from any older pin', () => { + const update = statements.find( + (s) => /^UPDATE\s+uo_link_config/i.test(s) && s.includes('uo_link_protocol_4_migrated'), + ) + assert.match( + update, + /protocol\s*<\s*4/, + 'must be `protocol < 4`, not `= 3`: an install that never took the protocol-3 ' + + 'migration has to be carried the whole way rather than one step', + ) +}) + test('the protocol-3 marker is written AFTER the update that reads it', () => { const update = statements.findIndex( (s) => /^UPDATE\s+uo_link_config/i.test(s) && s.includes('uo_link_protocol_3_migrated'),