app: a button that looked live and did nothing, one line after being disabled

subscribe() delivers the CURRENT state at once, and boot.js registered it right
after wire() had deliberately disabled Generate for the length of the reattach
lookup. That first delivery drew an idle run, and drawing an idle run hands the
button back -- so on every single page load Generate looked available while the
page still had no idea whether it already had a run. The queue stayed safe,
because the generator waits on the same lookup, but a live-looking button that
answers a click with nothing is its own fault.

Fixed twice over, because either alone would rot. The subscription moved into
wire() with the rest of the wiring, and the button being held is now a state the
transport keeps rather than something a caller does once: draw() runs on every
change, and each of those was free to undo it.

The fake run had no subscribe() at all -- which is exactly why the suite could
not see this. That is the second time a fake was tidier than the real object and
hid the fault it stood in for, so it now delivers state on subscribe like the
real one, and announces changes.

boot.js is down to assembling six things and mounting the modules.

app/boot.js

@@ -8,7 +8,7 @@ import { fetchManifest } from "./shell/manifest.js";
 import { mountAll } from "./shell/panels.js";
 import { all as allValues } from "./shell/values.js";
 import { composer } from "./composer/composer.js";
-import { createRun, viewUrl } from "./shell/run.js";
+import { createRun } from "./shell/run.js";
 import { clientId, connect, runningFor, finishedFor } from "./shell/client.js";
 import { wire } from "./shell/session.js";
 import { check } from "./shell/pipeline.js";
@@ -25,12 +25,6 @@ async function start() {
   });
   const session = wire({ run, page, check, id, runningFor, finishedFor });
 
-  run.subscribe((state) => {
-    page.transport.draw(state);
-    const last = state.images[state.images.length - 1];
-    if (last) page.viewer.setSource(viewUrl(last), kindOf(last));
-  });
-
   let manifest;
   try {
     manifest = await fetchManifest();
@@ -63,9 +57,4 @@ async function start() {
   };
 }
 
-/** Video and image results arrive the same way and cannot be shown the same way. */
-function kindOf(image) {
-  return /\.(mp4|webm|mov|mkv)$/i.test(image.filename || "") ? "video" : "image";
-}
-
 start();

app/shell/session.js

@@ -1,5 +1,10 @@
 // The two things the page does around a run: start one, and take back over one
 // that was already going.
+
+import { viewUrl } from "./run.js";
+
+/** Video and image results arrive the same way and cannot be shown the same way. */
+const kindOf = (image) => (/\.(mp4|webm|mov|mkv)$/i.test(image.filename || "") ? "video" : "image");
 //
 // Kept out of boot.js because boot.js runs on import and needs a document, so
 // nothing in it could be driven by a test -- and both of the faults that lived
@@ -82,6 +87,16 @@ export function createGenerator({ run, transport, check, ready }) {
  * it.
  */
 export function wire({ run, page, check, id, runningFor, finishedFor }) {
+  // One subscription draws everything a run affects. It lives here rather than
+  // beside it in boot.js because subscribe() delivers the CURRENT state at
+  // once, and doing that after the button was deliberately disabled handed it
+  // straight back -- live-looking, and doing nothing when pressed.
+  run.subscribe((state) => {
+    page.transport.draw(state);
+    const last = state.images[state.images.length - 1];
+    if (last && page.viewer) page.viewer.setSource(viewUrl(last), kindOf(last));
+  });
+
   // The socket opens at load, not at Generate. A generation queued before a
   // reload keeps running on the server, and a page that only listens once it
   // starts one of its own hears nothing about it.
@@ -90,12 +105,10 @@ export function wire({ run, page, check, id, runningFor, finishedFor }) {
   // Not awaited by the caller, but the button waits on it. Until this settles
   // nobody can know whether this page already has a run, and starting another
   // one on the strength of "phase is idle" is starting one on a guess.
-  page.transport.generate.setDisabled(true);
-  page.transport.say("Looking for a run already in progress");
+  page.transport.hold("Looking for a run already in progress");
 
   const ready = reattach(run, id, { runningFor, finishedFor }).then((adopted) => {
-    page.transport.say(null, run.state);
-    page.transport.draw(run.state);
+    page.transport.release(run.state);
     return adopted;
   });
 

app/shell/tests/session.test.js

@@ -50,11 +50,26 @@ function fakeRun(phase = "idle") {
     },
     started,
     release: () => release(),
-    state: { phase },
+    state: { phase, promptId: null, progress: null, images: [], audio: [], error: null, node: null },
     listened: 0,
     listen() { this.listened += 1; },
+    // The real run delivers the CURRENT state on subscribe, at once. Leaving
+    // that off is how a fake stays tidier than the thing it stands in for: the
+    // synchronous first delivery is exactly what handed a deliberately disabled
+    // button straight back, and a fake without it could not show that.
+    subscribers: new Set(),
+    subscribe(fn) {
+      this.subscribers.add(fn);
+      try { fn(this.state); } catch { /* as the real one does */ }
+      return () => this.subscribers.delete(fn);
+    },
+    announce() { for (const fn of this.subscribers) fn(this.state); },
     seen: () => [],
-    adopt(id) { this.state = { ...this.state, phase: "running", promptId: id }; return true; },
+    adopt(id) {
+      this.state = { ...this.state, phase: "running", promptId: id, images: [] };
+      this.announce();
+      return true;
+    },
     async start(prompt) {
       started.push(prompt);
       this.state = { ...this.state, phase: "queued" };
@@ -311,3 +326,46 @@ test("a queue that cannot be reached still gives the button back", async () => {
   assert.equal(t.disabled, false);
   assert.equal(t.text, "Ready");
 });
+
+test("a state delivered while the page is still looking does not hand Generate back", () => {
+  // subscribe() delivers the current state at once, and every later change
+  // redraws. Each of those draws decides the button from the run's phase alone,
+  // so an idle run -- which is what a page has while it is still finding out
+  // whether it has a run at all -- re-enabled a button that had just been
+  // deliberately disabled. Live-looking, and doing nothing when pressed.
+  const t = transport();
+  const run = fakeRun();
+  wire({
+    run, page: { transport: t }, check: plan(), id: "me",
+    runningFor: () => new Promise(() => {}),      // never answers
+    finishedFor: async () => null,
+  });
+
+  assert.equal(t.disabled, true);
+  run.announce();                                 // any redraw at all
+  assert.equal(t.disabled, true, "a redraw handed the button back mid-lookup");
+  t.draw(run.state);
+  assert.equal(t.disabled, true, "drawing the idle run handed the button back");
+});
+
+test("the result of a run is shown as it arrives", async () => {
+  // The subscription draws the transport AND the preview. Moving it had to
+  // carry both, and the viewer is the half with nothing else watching it.
+  const shown = [];
+  const t = transport();
+  const run = fakeRun();
+  wire({
+    run,
+    page: { transport: t, viewer: { setSource: (src, kind) => shown.push([src, kind]) } },
+    check: plan(), id: "me",
+    runningFor: async () => null, finishedFor: async () => null,
+  });
+
+  run.state = { ...run.state, phase: "done", images: [{ filename: "a.png", subfolder: "", type: "output" }] };
+  run.announce();
+  assert.deepEqual(shown, [["/view?filename=a.png&subfolder=&type=output", "image"]]);
+
+  run.state = { ...run.state, images: [{ filename: "clip.mp4", subfolder: "", type: "output" }] };
+  run.announce();
+  assert.equal(shown[1][1], "video", "a video result was handed to an <img>");
+});

app/shell/transport.js

@@ -37,6 +37,13 @@ export function createTransport({ onGenerate, onCancel } = {}) {
   // run says "Ready", which wiped the reason a moment after it appeared.
   let note = null;
 
+  // Held for a reason that is not a run: the page is still working out whether
+  // it already has one. Kept as a flag rather than left to whoever disabled the
+  // button, because draw() runs on every state change and each of those would
+  // otherwise hand the button back -- which is exactly what a subscription
+  // delivering the current state did, one line after the button was disabled.
+  let held = false;
+
   // Hidden by attribute rather than removed: the row's shape then does not jump
   // as a run starts and finishes, and there is one node to find in a test.
   const hide = (handle, yes) => {
@@ -51,7 +58,7 @@ export function createTransport({ onGenerate, onCancel } = {}) {
     // says outranks a note about an attempt that never started.
     if (state.phase !== IDLE) note = null;
     const working = WORKING.has(state.phase);
-    generate.setDisabled(working);
+    generate.setDisabled(working || held);
     generate.setBusy(working);
     hide(cancel, !working);
 
@@ -72,7 +79,21 @@ export function createTransport({ onGenerate, onCancel } = {}) {
     status.setText(note || describe(state || { phase: IDLE, images: [] }));
   }
 
-  return { node: bar.node, draw, say, generate, cancel, progress, status };
+  /** Stop offering Generate for a reason of the page's own, and say why. */
+  function hold(reason) {
+    held = Boolean(reason);
+    if (reason) say(reason);
+    generate.setDisabled(true);
+  }
+
+  /** Offer it again. The run's own state decides from here. */
+  function release(state) {
+    held = false;
+    say(null, state);
+    draw(state);
+  }
+
+  return { node: bar.node, draw, say, hold, release, generate, cancel, progress, status };
 }
 
 /** One line saying where the run is. The only place these words are decided. */