Merge pull request 'fix(uo-link): pin protocol 4, the version this build actually speaks' (#20) from fix/protocol-4-pin into main

Reviewed-on: #20
Reviewed-by: Colby Whitlock <whitlocktech@gmail.com>

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

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) {

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'),