From 7f7d4578ceae118e7f6cc1801f41dd12451ff025 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 24 Aug 2026 19:17:51 -0500 Subject: [PATCH] fix(uo-link): pin protocol 4, the version this build actually speaks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The protocol-4 cutover moved `link`'s PROTOCOL_VERSION, the overlay's `overlay.toml` and this module's ingest — `guild.roster` and `guild.leave` landed with the Teams cutover — but left both of this module's pin sites at 3. A fresh install therefore came up speaking 3 to a protocol-4 sidecar, and a sidecar answers a stale client with `409 protocol version mismatch` rather than mis-parsing it. The failure is total and silent: every REST read fails, the WS closes on ws.hello, and the operator sees an empty marketplace, an empty guild board and no shard status, with the cause only in the server log. It cleared only when an admin edited the number by hand in Admin → Shard. Found while standing up a demo deployment for the marketing site's screenshots. - `DEFAULT_PROTOCOL` → 4 (the constant used before an admin has saved anything) - the `uo_link_config.protocol` column default → 4, at both declaration sites - a protocol-4 one-shot mirroring the protocol-3 one, guarded by its own marker so an operator who deliberately pins an older sidecar stays pinned, and written `protocol < 4` so an install that never took the protocol-3 migration is carried the whole way rather than one step - three regression tests: the column default, the marker ordering, and the `< 4` predicate Co-Authored-By: Claude --- server/db/schema.sql | 31 +++++++++- .../model/uoLinkConfig/uoLinkConfig.model.js | 9 ++- server/test/schemaFragment.test.js | 57 ++++++++++++++++++- 3 files changed, 94 insertions(+), 3 deletions(-) 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'),