fix: web UI client review findings (step 2a)

- One socket at a time: a socket being replaced is let go without a word,
  and connect() or a new token while a reconnect waits opens no second one.
  A new token reconnects at once, without a refused.
- Events that came before the schema go as soon as it is whole, ahead of
  the sync frames formatted after them, not at syncEnd over newer state.
- A schema's hash counts only once it has come whole, so a sync cut short
  gets it again.
- Feeds are subscribed again after a reconnect; an unknown feed is refused.
- A long op refused as busy without its id goes again.
- A sync without repeaters clears them; order changes name their list; a
  refused resync is called off.
- refused: a listener can close() to stop the retries, which the UI does
  once /loginContext says a login is needed. The client itself can't tell a
  refused token from a device that is down.
- localStorage keeps the schema by default. minify.py removes its work
  folder even when a bundle fails. ADR 0005 records the two bundles.
- Smaller: ProtoSchema.ROWS/withoutRows, ProtoModel.ITEMS, named type
  numbers, unused feeds map and FakeSocket.all gone.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

test/js/client.test.js

@@ -3,13 +3,13 @@
 const test = require('node:test');
 const assert = require('node:assert/strict');
 const { ProtoClient, ProtoError } = require('../../data-src/js/client.js');
-const { hello, sync, recordedItems, recordedSchema, FakeSocket, FakeTimers, FakeStorage } = require('./helpers.js');
+const { hello, sync, syncWithoutSchema, recordedItems, recordedSchema, FakeSocket, FakeTimers, FakeStorage } = require('./helpers.js');
 
 function client(opts) {
     FakeSocket.reset();
     const timers = new FakeTimers();
     const c = new ProtoClient(Object.assign({ host: 'device', WebSocket: FakeSocket, setTimeout: timers.setTimeout,
-        clearTimeout: timers.clearTimeout }, opts || {}));
+        clearTimeout: timers.clearTimeout, storage: null }, opts || {}));
     c.timers = timers;
     return c;
 }
@@ -131,17 +131,36 @@ test('events update the model, and are told by name', () => {
     assert.deepEqual(seen, [{ shadeId: shade.shadeId, target: 30, direction: 1 }]);
 });
 
-test('an event before the schema is kept until the sync ends', () => {
+test('an event before the schema waits for it, and goes before the sync frames formatted after it', () => {
     const c = client();
+    const states = [];
+    c.on('shadeState', (d) => states.push(d));
     c.connect();
     const ws = FakeSocket.last;
     ws.open();
     ws.deliver(hello);
     // Before the schema: the client can't read it yet.
-    const shadeId = recordedItems('S')[0].s;
-    ws.deliver({ v: 2, t: T.shadeState, d: { s: shadeId, t: 55 } });
+    const recorded = recordedItems('S')[0];
+    ws.deliver({ v: 2, t: T.shadeState, d: { s: recorded.s, t: 55 } });
+    assert.deepEqual(states, []);
     for (const f of sync) ws.deliver(f);
-    assert.equal(c.model.shades.get(shadeId).target, 55);
+    assert.deepEqual(states, [{ shadeId: recorded.s, target: 55 }]);
+    // The shade's sync frame was formatted after the event, so it wins.
+    assert.equal(c.model.shades.get(recorded.s).target, recorded.t || 0);
+});
+
+test('an event after the schema goes at once, during the sync', () => {
+    const c = client();
+    c.connect();
+    const ws = FakeSocket.last;
+    ws.open();
+    ws.deliver(hello);
+    const syncFrames = sync.filter((f) => f.t === T.sync);
+    for (const f of syncFrames) ws.deliver(f);
+    const shadeId = recordedItems('S')[0].s;
+    ws.deliver({ v: 2, t: T.shadeState, d: { s: shadeId, t: 66 } });
+    assert.equal(c.ready, false);
+    assert.equal(c.model.shades.get(shadeId).target, 66);
 });
 
 test('the schema is kept, and the device is told it has it', () => {
@@ -153,16 +172,101 @@ test('the schema is kept, and the device is told it has it', () => {
     assert.equal(ws.url, `ws://device/ws?schema=${hello.d.sh}`);
     ws.open();
     ws.deliver(hello);
-    // Without the schema's rows, as the device sends a client that has it.
-    for (const f of sync) {
-        const d = Object.assign({}, f.d);
-        for (const k of ['T', 'F', 'E', 'Q']) delete d[k];
-        ws.deliver(Object.assign({}, f, { d: d }));
-    }
+    for (const f of syncWithoutSchema()) ws.deliver(f);
     assert.ok(c.ready);
     assert.equal(c.model.shades.size, recordedItems('S').length);
 });
 
+test('a schema cut off by a lost socket is not kept, so it comes whole again', () => {
+    const c = client();
+    c.connect();
+    const ws = FakeSocket.last;
+    ws.open();
+    ws.deliver(hello);
+    ws.deliver(sync[0]);
+    ws.drop();
+    c.timers.runAll();
+    assert.equal(FakeSocket.last.url, 'ws://device/ws');
+    FakeSocket.last.open();
+    FakeSocket.last.deliver(hello);
+    for (const f of sync) FakeSocket.last.deliver(f);
+    assert.equal(Object.keys(c.schema.fields).length, Object.keys(schema.fields).length);
+    assert.equal(c.schema.hash, hello.d.sh);
+});
+
+test('a socket is replaced, never doubled', () => {
+    const { c, ws } = connected();
+    c.connect();
+    assert.equal(FakeSocket.count, 1);
+    // A new token while a reconnect is waiting: one socket, and no timer left.
+    ws.drop();
+    c.setToken('new');
+    assert.equal(FakeSocket.count, 2);
+    assert.deepEqual(c.timers.runAll(), []);
+    // While it connects: the old one goes without a refused.
+    let refused = 0;
+    c.on('refused', () => refused++);
+    c.setToken('newer');
+    assert.equal(refused, 0);
+    assert.equal(FakeSocket.count, 3);
+    assert.match(FakeSocket.last.url, /apikey=newer/);
+});
+
+test('a listener can stop the retries after a refusal', () => {
+    const c = client();
+    c.on('refused', () => c.close());
+    c.connect();
+    FakeSocket.last.drop();
+    assert.deepEqual(c.timers.runAll(), []);
+    assert.equal(FakeSocket.count, 1);
+});
+
+test('feeds are subscribed again after a reconnect', () => {
+    const { c, ws } = connected();
+    // Its reply never comes: the socket closes first.
+    c.subscribe('frames').catch(() => {});
+    ws.drop();
+    c.timers.runAll();
+    const again = FakeSocket.last;
+    again.open();
+    again.deliver(hello);
+    for (const f of sync) again.deliver(f);
+    const sent = again.sent.filter((f) => f.t === T.subscribe);
+    assert.deepEqual(sent.map((f) => f.d), [{ fe: schema.feedNumbers.frames }]);
+});
+
+test('a feed the device lacks is refused here', async () => {
+    const { c } = connected();
+    await assert.rejects(c.subscribe('nope'), (err) => err.code === 'badRequest');
+});
+
+test('a long op refused as busy without its id goes again', () => {
+    const { c, ws } = connected();
+    c.op('saveRoom', { roomId: 1, name: 'x'.repeat(1500) }).catch(() => {});
+    const sent = ws.sent.length;
+    ws.deliver(reply(undefined, { e: schema.errorNumbers.busy, x: 'Another long message is on its way' }));
+    c.timers.runAll();
+    assert.equal(ws.sent.length, sent + 1);
+    assert.equal(ws.sent[ws.sent.length - 1].d.n.length, 1500);
+});
+
+test('a refusal without an id for nothing long is reported', () => {
+    const { c, ws } = connected();
+    let error = null;
+    c.on('error', (err) => { error = err; });
+    ws.deliver(reply(undefined, { e: schema.errorNumbers.badRequest, x: 'Malformed message' }));
+    assert.equal(error.code, 'badRequest');
+});
+
+test('a sync asked for and refused is called off', async () => {
+    const { c, ws } = connected();
+    const p = c.resync();
+    const id = ws.sent[ws.sent.length - 1].id;
+    ws.deliver(reply(id, { e: schema.errorNumbers.busy, x: 'busy' }));
+    await assert.rejects(p);
+    assert.equal(c.model.seen, null);
+});
+
 test('a kept schema of another firmware is dropped', () => {
     const storage = new FakeStorage();
     connected({ storage: storage });
@@ -171,7 +275,8 @@ test('a kept schema of another firmware is dropped', () => {
     const ws = FakeSocket.last;
     ws.open();
     ws.deliver({ v: 2, t: 1, d: { fw: 'v4.0.0', sh: 'ffffffff' } });
-    assert.equal(c.schema.hash, 'ffffffff');
+    // Its hash only counts once the new one has come whole.
+    assert.equal(c.schema.hash, null);
     assert.ok(c.schema.empty);
 });
 

test/js/helpers.js

@@ -17,11 +17,11 @@ function recordedSchema() {
 }
 // The recorded sync's device sections, expanded, frame by frame.
 function deviceSections(schema) {
-    return sync.filter((f) => f.t === 4).map((f) => {
-        const d = {};
-        for (const k of Object.keys(f.d)) if (!['T', 'F', 'E', 'Q'].includes(k)) d[k] = f.d[k];
-        return schema.expand(d);
-    });
+    return sync.filter((f) => f.t === 4).map((f) => schema.expand(ProtoSchema.withoutRows(f.d)));
+}
+// The recorded sync as the device sends it to a client that has the schema.
+function syncWithoutSchema() {
+    return sync.map((f) => Object.assign({}, f, { d: ProtoSchema.withoutRows(f.d || {}) }));
 }
 // The recorded sync's items under their short keys.
 function recordedItems(key) {
@@ -36,7 +36,7 @@ class FakeSocket {
         this.sent = [];
         this.closed = false;
         FakeSocket.last = this;
-        FakeSocket.all.push(this);
+        FakeSocket.count++;
     }
     send(text) { this.sent.push(JSON.parse(text)); }
     close() {
@@ -49,8 +49,9 @@ class FakeSocket {
     // The device closing it, or refusing it before it opened.
     drop() { this.close(); }
 }
-FakeSocket.all = [];
-FakeSocket.reset = () => { FakeSocket.all = []; FakeSocket.last = null; };
+// How many the client has opened.
+FakeSocket.count = 0;
+FakeSocket.reset = () => { FakeSocket.count = 0; FakeSocket.last = null; };
 
 // Timers the test runs by hand.
 class FakeTimers {
@@ -64,9 +65,9 @@ class FakeTimers {
         };
         this.clearTimeout = (id) => { this.timers.delete(id); };
     }
-    // Runs every timer waiting, and returns their delays.
+    // Runs every timer waiting, the shortest first, and returns their delays.
     runAll() {
-        const due = [...this.timers.entries()];
+        const due = [...this.timers.entries()].sort((a, b) => a[1].ms - b[1].ms);
         this.timers.clear();
         for (const [, t] of due) t.fn();
         return due.map(([, t]) => t.ms);
@@ -79,4 +80,5 @@ class FakeStorage {
     setItem(key, value) { this.items[key] = String(value); }
 }
 
-module.exports = { hello, sync, schemaJson, recordedSchema, deviceSections, recordedItems, FakeSocket, FakeTimers, FakeStorage };
+module.exports = { hello, sync, schemaJson, recordedSchema, deviceSections, syncWithoutSchema, recordedItems, FakeSocket,
+    FakeTimers, FakeStorage };

test/js/model.test.js

@@ -79,7 +79,26 @@ test('removals, orders and repeaters', () => {
     assert.equal(m.shades.get(second.shadeId).sortOrder, 0);
     m.applyEvent('repeaters', { repeaters: [7, 8] });
     assert.deepEqual(m.repeaters, [7, 8]);
-    assert.deepEqual(changes.map((c) => c[0]), ['shadeRemoved', 'order', 'repeaters']);
+    // An order says which list it is.
+    assert.deepEqual(changes.map((c) => c[0]), ['shadeRemoved', 'shadeOrder', 'repeaters']);
+});
+
+test('a sync without repeaters clears them, as the device leaves the list out when it has none', () => {
+    const m = synced();
+    m.applyEvent('repeaters', { repeaters: [7] });
+    m.beginSync();
+    for (const d of deviceSections(m.schema)) m.applySync(Object.assign({}, d, { repeaters: undefined }));
+    m.endSync();
+    assert.deepEqual(m.repeaters, []);
+});
+
+test('a sync that is called off removes nothing', () => {
+    const m = synced();
+    const shades = m.shades.size;
+    m.beginSync();
+    m.cancelSync();
+    m.endSync();
+    assert.equal(m.shades.size, shades);
 });
 
 test('a sync drops what it no longer lists, and keeps what an event added during it', () => {