fix(control-pane): settle the poll guard against the last settled request

The staleness guard compared a poll against the newest poll STARTED, and
only the failure path used it. Two orderings were wrong.

An older success settling after a newer one had no guard at all, so it
overwrote the newer counts, the canvas label and the announcement with
older data. Both paths now check the token.

An older failure settling while a newer poll was still in flight was
discarded, because a newer poll had merely started. The page then kept
the last successful counts and its no-steering message as if they were
current. Anchoring the guard to the newest poll that has SETTLED fixes
that: the older failure is still the newest thing to have settled, so it
lands, and the pending poll restores the view when it answers.

That single change covers both orderings, so the guard no longer needs
to know which path it is on. Each poll records itself as settled when it
finishes, whatever the outcome.

A poll also had no timeout, so a request that never answered left the
last steering guidance on screen indefinitely. Each poll now aborts
after ten seconds through an AbortController, and that abort is what
wakes the poll, so a timeout is reported as an outage instead of
silently leaving stale guidance.

The test harness can now hold a request open indefinitely and fire
pending timers on demand, so the timeout is exercised without waiting.
New assertions cover an older success losing to a newer one, an older
failure landing while a newer poll is in flight and then being
recovered by it, and a hung poll timing out into the offline state.
Removing the success guard, re-anchoring the guard to the last started
poll, and removing the timeout each fail one of those assertions, and
the unmutated file passes all of them.

Co-authored-by: CommandCodeBot <noreply@commandcode.ai>

scripts/lib/control-pane/control-plane-view-ui.js

@@ -243,10 +243,18 @@ function renderControlPlaneViewHtml() {
   // Wording shared by the canvas label and the live region so an outage reads
   // the same way however the operator reaches it.
   var UNAVAILABLE = 'Control-plane data is unavailable. Advisories and steering are unknown.';
-  // Monotonic poll id. Polls are not sequenced, so a slow older request can
-  // settle after a newer one. Only the newest poll may touch the view, which
-  // stops a late failure from overwriting a newer success.
-  var latestPoll = 0;
+  // Polls are not sequenced, so a slow request can settle out of order. Only
+  // the newest poll that has already settled may update the view: a success
+  // from a superseded poll would show older counts, and a failure from a
+  // superseded poll would erase newer counts. Anchoring to the last settled
+  // poll rather than the last started one also lets a failure land while a
+  // newer poll is still pending, instead of leaving stale guidance on screen.
+  var settledPoll = 0;
+  // Monotonic id handed to each poll as it starts.
+  var pollSeq = 0;
+  // A poll that never answers must not stay pending forever, or the last
+  // steering guidance stays on screen indefinitely.
+  var TIMEOUT_MS = 10000;
 
   // Polling runs every few seconds, so only speak when the advisory and
   // steering counts actually move. Repeating an unchanged summary would talk
@@ -257,6 +265,15 @@ function renderControlPlaneViewHtml() {
     document.getElementById('announce').textContent = message;
   }
 
+  function unavailable() {
+    document.getElementById('status').textContent = 'offline';
+    // The last guidance is now stale, so replace it rather than leaving the
+    // live region claiming the airspace is clear. The canvas label goes with
+    // it, or it would still report the last successful counts.
+    canvas.setAttribute('aria-label', UNAVAILABLE);
+    announce(UNAVAILABLE);
+  }
+
   function apply(data) {
     if (!data || data.schemaVersion !== 'ecc.control-plane.view.v1' ||
         !['tasks', 'lanes', 'pairs', 'events'].every(function (key) { return Array.isArray(data[key]); }) ||
@@ -282,21 +299,30 @@ function renderControlPlaneViewHtml() {
   }
 
   function poll() {
-    var token = ++latestPoll;
-    fetch('/api/control-plane').then(function (r) {
+    var token = ++pollSeq;
+    var timer = null;
+    var controller = new AbortController();
+    // Reject on a timer so a hung request cannot keep the previous guidance on
+    // screen forever. The abort is what wakes this poll up, so a timeout is
+    // reported as an outage rather than swallowed.
+    timer = setTimeout(function () { controller.abort(); }, TIMEOUT_MS);
+    // A poll that has already answered, or timed out, owns the view. Anything
+    // that settles later is superseded and must be dropped.
+    function settle() {
+      clearTimeout(timer);
+      settledPoll = token;
+    }
+    fetch('/api/control-plane', { signal: controller.signal }).then(function (r) {
       if (!r.ok) throw new Error('Control-plane request failed');
       return r.json();
-    }).then(apply).catch(function () {
-      // A newer poll has already answered, so this failure is stale and must
-      // not overwrite the newer counts.
-      if (token !== latestPoll) return;
-      document.getElementById('status').textContent = 'offline';
-      // The last guidance is now stale, so replace it rather than leaving the
-      // live region claiming the airspace is clear. The canvas label goes with
-      // it, or it would still report the last successful counts.
-      canvas.setAttribute('aria-label', UNAVAILABLE);
-      announce(UNAVAILABLE);
-    });
+    }).then(function (data) {
+      // A newer poll already owns the view, so do not resurrect older counts.
+      if (token < settledPoll) return;
+      apply(data);
+    }).catch(function () {
+      if (token < settledPoll) return;
+      unavailable();
+    }).then(settle, settle);
   }
 
   resize();

tests/lib/control-plane-view-ui.test.js

@@ -83,13 +83,25 @@ function element(tag, context) {
   return node;
 }
 
+// A poll settles through several chained promise callbacks, so draining needs a
+// few turns of the event loop rather than a single tick.
+function settle() {
+  return new Promise(resolve => {
+    let remaining = 5;
+    const step = () => (remaining-- > 0 ? setImmediate(step) : resolve());
+    step();
+  });
+}
+
 // Drives the view's inline script against a queue of poll responses, so one run
 // can cover several polls and the state each one leaves behind. A response may
-// carry a `hold` function, which lets a test settle two polls out of order.
+// carry a `hold` promise to park until the test releases it, or `never: true` to
+// stay pending so the request timeout can be exercised.
 async function render(responses) {
   const context = createContext();
   const elements = new Map();
   const timers = [];
+  const timeouts = [];
   const queue = responses.slice();
   const document = {
     getElementById(id) { if (!elements.has(id)) elements.set(id, element(null, context)); return elements.get(id); },
@@ -103,12 +115,24 @@ async function render(responses) {
   vm.runInNewContext(code, {
     document,
     window: { addEventListener() {}, devicePixelRatio: 1 },
+    AbortController,
     setInterval(fn) { timers.push(fn); },
-    fetch: async () => {
+    // Timers are collected rather than run, so a test can fire the request
+    // timeout on demand instead of waiting ten seconds for it.
+    setTimeout(fn, ms) { const entry = { fn, ms, fired: false }; timeouts.push(entry); return entry; },
+    clearTimeout(entry) { if (entry) entry.fired = true; },
+    fetch: async (url, options) => {
       const next = queue.length > 1 ? queue.shift() : queue[0];
-      // `hold` parks this response until the test releases it, which is how a
-      // slow older poll is made to settle after a newer one.
+      if (next.never) {
+        // A request that never answers. Honour the abort the view sends, so the
+        // timeout can drive it to a failure.
+        return new Promise((_, reject) => {
+          if (!options || !options.signal) return;
+          options.signal.addEventListener('abort', () => reject(new Error('aborted')));
+        });
+      }
       if (next.hold) await next.hold;
+      if (options && options.signal && options.signal.aborted) throw new Error('aborted');
       return { ok: next.ok, json: async () => next.data };
     }
   });
@@ -118,9 +142,17 @@ async function render(responses) {
     context,
     writesTo(id) { return elements.get(id).writes; },
     labelOf(id) { return elements.get(id).attributes['aria-label']; },
+    // Fire every pending request timeout, then let the rejections propagate.
+    async fireTimeouts() {
+      for (const entry of timeouts) {
+        if (!entry.fired) { entry.fired = true; entry.fn(); }
+      }
+      await settle();
+    },
+    timeoutBudgetMs() { return timeouts.length ? timeouts[0].ms : null; },
     async pollAgain() {
       for (const fn of timers) fn();
-      await new Promise(resolve => setImmediate(resolve));
+      await settle();
     }
   };
 }
@@ -285,6 +317,84 @@ let passed = 0;
     'a superseded failure must not mark the view offline');
   passed += 1;
 
+  // The mirror of the case above. An older SUCCESS settling after a newer
+  // success must not drag the view back to the older counts.
+  let releaseStale;
+  const staleSettles = new Promise(resolve => { releaseStale = resolve; });
+  const staleSuccess = await render([
+    { ok: true, data: populatedView() },
+    { ok: true, data: populatedView({ counts: { tasks: 3, lanes: 1, agents: 2, advisories: 2, resolutions: 1 } }), hold: staleSettles },
+    { ok: true, data: populatedView({ counts: { tasks: 3, lanes: 1, agents: 2, advisories: 7, resolutions: 0 } }) }
+  ]);
+  await staleSuccess.pollAgain();
+  await staleSuccess.pollAgain();
+  assert.ok(staleSuccess.labelOf('c').includes('7 advisories'),
+    `the newer success should land first, got ${staleSuccess.labelOf('c')}`);
+  releaseStale();
+  await settle();
+  assert.ok(staleSuccess.labelOf('c').includes('7 advisories'),
+    `a superseded success must not overwrite the newer counts, got ${staleSuccess.labelOf('c')}`);
+  assert.ok(!staleSuccess.elements.get('announce').textContent.includes('Steering is required.'),
+    `a superseded success must not re-announce the older guidance, got ${staleSuccess.elements.get('announce').textContent}`);
+  passed += 1;
+
+  // A failure must still surface while a newer poll is already in flight and has
+  // not answered. The older failure is still the newest thing to have settled,
+  // so a guard anchored to the last settled poll lets it through, while one
+  // anchored to the last poll started would silently drop it and leave stale
+  // steering guidance on screen.
+  let releaseFailure;
+  const failureSettles = new Promise(resolve => { releaseFailure = resolve; });
+  let releasePending;
+  const pendingSettles = new Promise(resolve => { releasePending = resolve; });
+  const failureWhilePending = await render([
+    { ok: true, data: populatedView() },
+    { ok: false, data: { ok: false, error: 'snapshot unavailable' }, hold: failureSettles },
+    { ok: true, data: populatedView({ counts: { tasks: 3, lanes: 1, agents: 2, advisories: 9, resolutions: 0 } }), hold: pendingSettles }
+  ]);
+  // Start the failing poll, then the newer pending one, so the failure settles
+  // with a newer request still in flight.
+  await failureWhilePending.pollAgain();
+  await failureWhilePending.pollAgain();
+  releaseFailure();
+  await settle();
+  assert.strictEqual(failureWhilePending.elements.get('status').textContent, 'offline',
+    'a failure must surface while a newer poll is still pending');
+  assert.ok(/unavailable/i.test(failureWhilePending.labelOf('c')),
+    `a failure must clear the canvas counts while a newer poll is pending, got ${failureWhilePending.labelOf('c')}`);
+  assert.ok(/unavailable/i.test(failureWhilePending.elements.get('announce').textContent),
+    `a failure must announce the outage while a newer poll is pending, got ${failureWhilePending.elements.get('announce').textContent}`);
+  passed += 1;
+
+  // The newer poll that was pending must still be able to restore the view.
+  await failureWhilePending.pollAgain();
+  releasePending();
+  await settle();
+  assert.ok(failureWhilePending.labelOf('c').includes('9 advisories')
+    && !/unavailable/i.test(failureWhilePending.labelOf('c')),
+    `the pending poll must still restore the counts, got ${failureWhilePending.labelOf('c')}`);
+  passed += 1;
+
+  // A poll that never answers must not leave the last steering guidance on
+  // screen forever.
+  const hung = await render([
+    { ok: true, data: populatedView() },
+    { ok: false, data: { ok: false, error: 'snapshot unavailable' }, never: true }
+  ]);
+  await hung.pollAgain();
+  assert.ok(!/unavailable/i.test(hung.labelOf('c')),
+    'the hung poll should not have reported anything yet');
+  assert.ok(hung.timeoutBudgetMs() !== null && hung.timeoutBudgetMs() <= 15000,
+    `a request timeout should be bounded, got ${hung.timeoutBudgetMs()}`);
+  await hung.fireTimeouts();
+  assert.strictEqual(hung.elements.get('status').textContent, 'offline',
+    'a poll that never answers must time out into the offline state');
+  assert.ok(/unavailable/i.test(hung.labelOf('c')),
+    `a timed out poll must clear the canvas label, got ${hung.labelOf('c')}`);
+  assert.ok(/unavailable/i.test(hung.elements.get('announce').textContent),
+    `a timed out poll must announce the outage, got ${hung.elements.get('announce').textContent}`);
+  passed += 1;
+
   console.log(`Results: Passed: ${passed}, Failed: 0`);
 })().catch(error => {
   console.error(error.message);