Offer released images only, newest first, and keep the panel moving (#170, #171, #172)

client/src/App.vue

@@ -246,6 +246,7 @@ import TheWifiSetup from "./components/TheWifiSetup";
 import WaveUI from "wave-ui";
 import { mapGetters, mapActions } from "vuex";
 import axios from "axios";
+import { selectRebuildImages } from "./rebuildImages";
 
 export default {
   name: "App",
@@ -288,7 +289,10 @@ export default {
     selectedUploadImage: [],
     selectedLocalImage: undefined,
     githubImages: [],
-    rebuildImages: [],
+    // The releases payload as GitHub sent it. Kept raw so the picker can be
+    // filtered here rather than re-fetched: flicking "show pre-releases" is a
+    // question about a list we already have.
+    githubReleases: [],
     localImages: [],
     uploadError: false,
     openInfo: false,
@@ -333,6 +337,12 @@ export default {
       );
     },
     ...mapGetters(["options", "progress", "flash"]),
+    // Derived from the cached payload, so the pre-release switch re-filters a
+    // list that is already here instead of asking GitHub again - and so the
+    // order does not depend on what the API happened to return (#171, #172).
+    rebuildImages() {
+      return selectRebuildImages(this.githubReleases, this.options.showPrereleases);
+    },
     // Rebuild downloads from GitHub via the board itself (flash-from-url) -
     // without internet that can't work, so hide it and leave only the
     // local-file paths (upload, magic, install) - #74.
@@ -1024,20 +1034,6 @@ export default {
         }
       }
     },
-    populateRebuildImages(releases) {
-      for (let release of releases) {
-        for (let asset of release.assets) {
-          if (asset.name.includes("rebuild")) {
-            this.rebuildImages.push({
-              name: asset.name,
-              id: asset.id,
-              url: asset.browser_download_url,
-              size: asset.size,
-            });
-          }
-        }
-      }
-    },
     // Static for as long as the page is open, and every field costs a partition
     // mount on the board - so this is fetched once and never on a state change.
     async getInfo() {
@@ -1084,14 +1080,22 @@ export default {
         setTimeout(this.getStatus, 1000);
       }
     },
+    // Only fetches. What the picker shows - which releases, in what order - is
+    // decided by the rebuildImages computed, so this can be called twice (see
+    // checkInternet()) without duplicating anything, and the pre-release switch
+    // re-filters what is already here rather than asking GitHub again.
     async getGithubImages() {
-      // Reset first - this can now be called again (see checkInternet())
-      // after already having been called once, and populateRebuildImages
-      // appends rather than replaces.
-      this.rebuildImages = [];
-      fetch("https://api.github.com/repos/intelligent-agent/Rebuild/releases")
-        .then((response) => response.json())
-        .then((data) => this.populateRebuildImages(data));
+      try {
+        const response = await fetch(
+          "https://api.github.com/repos/intelligent-agent/Rebuild/releases"
+        );
+        this.githubReleases = await response.json();
+      } catch (err) {
+        // No internet, or GitHub rate-limiting this board's address. The empty
+        // picker already says there is nothing to download, and the methods
+        // list hides Download without internet (#74).
+        this.githubReleases = [];
+      }
     },
     async checkInternet() {
       const response = await axios.get(`/api/has_internet`);

client/src/components/TheOptions.vue

@@ -31,6 +31,17 @@
           label="Magicmode"
         >
         </w-switch>
+        <!-- The picker offers released images only; this puts the release
+             candidates back. Filtered in the page from the releases it already
+             has, so flicking this re-sorts a list rather than asking GitHub
+             again (#172). -->
+        <w-switch
+          @change="onChange('showPrereleases', options.showPrereleases)"
+          v-model="options.showPrereleases"
+          class="ma2"
+          label="Show pre-releases"
+        >
+        </w-switch>
         <w-divider class="my6 mx-3"></w-divider>
         <h4>Screen rotation</h4>
         <w-radios

client/src/rebuildImages.js

@@ -0,0 +1,56 @@
+// Turning GitHub's releases payload into the list behind "Choose image to
+// Download". Its own module because it is pure - payload in, list out - and a
+// pure function is the part of this worth testing.
+//
+// The floor. Reflash has not been tested against anything older and those
+// images are not recommended, so they are not offered (#172). 1.0.2 is the
+// release before the current line.
+const OLDEST_OFFERED = [1, 0, 2];
+
+// "v1.1.0-RC8" -> [1, 1, 0]. The prerelease suffix is deliberately dropped: a
+// release's *version* is what the floor is about, and whether it is a
+// prerelease is already a field of its own on the release.
+function versionOf(tag) {
+  const m = /^v?(\d+)\.(\d+)\.(\d+)/.exec(String(tag || ''));
+  return m ? [Number(m[1]), Number(m[2]), Number(m[3])] : null;
+}
+
+function compare(a, b) {
+  for (let i = 0; i < 3; i++) {
+    if (a[i] !== b[i]) return a[i] - b[i];
+  }
+  return 0;
+}
+
+// The list the picker shows, newest first.
+//
+// Sorted here rather than taken as it arrives: the order used to be whatever
+// the API returned, which put the newest image somewhere down the list (#171).
+// De-duplicated for the same reason - the picker is fed from two places that
+// can both land, and a list with an image in it twice is a list nobody trusts.
+export function selectRebuildImages(releases, showPrereleases) {
+  if (!Array.isArray(releases)) return [];       // an error body is not a list
+
+  const wanted = releases
+    .filter((r) => r && !r.draft)
+    .filter((r) => showPrereleases || !r.prerelease)
+    .map((r) => ({ release: r, version: versionOf(r.tag_name) }))
+    .filter(({ version }) => version && compare(version, OLDEST_OFFERED) >= 0)
+    .sort((a, b) => compare(b.version, a.version));
+
+  const seen = new Set();
+  const images = [];
+  for (const { release } of wanted) {
+    const assets = (release.assets || [])
+      .filter((a) => a && typeof a.name === 'string' && a.name.startsWith('rebuild-'))
+      // Within one release the order is the variants', and GitHub's asset order
+      // is upload order - which is not something anyone chose.
+      .sort((a, b) => a.name.localeCompare(b.name));
+    for (const a of assets) {
+      if (seen.has(a.name)) continue;
+      seen.add(a.name);
+      images.push({ name: a.name, id: a.id, url: a.browser_download_url, size: a.size });
+    }
+  }
+  return images;
+}

client/tests/unit/rebuildImages.spec.js

@@ -0,0 +1,118 @@
+import { describe, it, expect } from 'vitest';
+import { selectRebuildImages } from '@/rebuildImages';
+
+// Shaped like the GitHub releases payload, with only the fields the picker uses.
+function release(tag, { prerelease = false, draft = false, variants = ['barebone', 'fluidd'] } = {}) {
+  return {
+    tag_name: tag,
+    prerelease,
+    draft,
+    assets: variants.map((v, i) => ({
+      name: `rebuild-${v}-${tag}.img.xz`,
+      id: `${tag}-${i}`,
+      browser_download_url: `http://example/${v}-${tag}`,
+      size: 1000 + i,
+    })),
+  };
+}
+
+const names = (list) => list.map((i) => i.name);
+
+describe('the Rebuild image picker', () => {
+  // The order used to be whatever the API handed back. It is the newest image
+  // people want, so it belongs at the top whatever GitHub does (#171).
+  it('puts the newest release first, whatever order the payload arrives in', () => {
+    const out = selectRebuildImages(
+      [release('v1.0.2'), release('v1.1.0'), release('v1.0.3')],
+      false
+    );
+    expect(names(out)[0]).toContain('v1.1.0');
+    expect(names(out)).toEqual([
+      'rebuild-barebone-v1.1.0.img.xz', 'rebuild-fluidd-v1.1.0.img.xz',
+      'rebuild-barebone-v1.0.3.img.xz', 'rebuild-fluidd-v1.0.3.img.xz',
+      'rebuild-barebone-v1.0.2.img.xz', 'rebuild-fluidd-v1.0.2.img.xz',
+    ]);
+  });
+
+  // Reflash has not been tested against them and they are not recommended
+  // (#172). 1.0.2 is the previous release and the floor.
+  it('drops releases older than 1.0.2', () => {
+    const out = selectRebuildImages(
+      [release('v1.0.2'), release('v1.0.1'), release('v1.0.0'), release('v0.1.0')],
+      false
+    );
+    expect(names(out).every((n) => n.includes('v1.0.2'))).toBe(true);
+  });
+
+  it('hides prereleases unless they are asked for', () => {
+    const payload = [release('v1.1.0-RC8', { prerelease: true }), release('v1.0.2')];
+
+    expect(names(selectRebuildImages(payload, false)).some((n) => n.includes('RC8'))).toBe(false);
+    expect(names(selectRebuildImages(payload, true))[0]).toContain('RC8');
+  });
+
+  // A prerelease of a version below the floor is still below the floor.
+  it('applies the floor to prereleases too', () => {
+    const out = selectRebuildImages([release('v1.0.1-RC1', { prerelease: true })], true);
+    expect(out).toEqual([]);
+  });
+
+  it('never shows drafts', () => {
+    const out = selectRebuildImages([release('v1.2.0', { draft: true }), release('v1.0.2')], true);
+    expect(names(out).some((n) => n.includes('v1.2.0'))).toBe(false);
+  });
+
+  // The picker is fed by two calls that can both land (created() and
+  // checkInternet()), and a duplicated list is a list nobody trusts.
+  it('lists each image once even if the payload is repeated', () => {
+    const payload = [release('v1.0.2')];
+    const out = selectRebuildImages([...payload, ...payload], false);
+    expect(names(out)).toEqual([
+      'rebuild-barebone-v1.0.2.img.xz',
+      'rebuild-fluidd-v1.0.2.img.xz',
+    ]);
+  });
+
+  // Only Rebuild images belong in a Rebuild picker; a release can carry other
+  // assets (checksums, a Reflash image someone attached by hand).
+  it('ignores assets that are not Rebuild images', () => {
+    const rel = release('v1.0.2');
+    rel.assets.push({ name: 'SHA256SUMS', id: 'x', browser_download_url: 'u', size: 1 });
+    rel.assets.push({ name: 'reflash-v1.0.2.img.xz', id: 'y', browser_download_url: 'u', size: 1 });
+    expect(names(selectRebuildImages([rel], false))).toEqual([
+      'rebuild-barebone-v1.0.2.img.xz',
+      'rebuild-fluidd-v1.0.2.img.xz',
+    ]);
+  });
+
+  it('survives a payload that is not a list', () => {
+    expect(selectRebuildImages(undefined, false)).toEqual([]);
+    expect(selectRebuildImages({ message: 'API rate limit exceeded' }, false)).toEqual([]);
+  });
+
+  it('keeps the fields the picker and the download need', () => {
+    const [first] = selectRebuildImages([release('v1.0.2')], false);
+    expect(first).toEqual({
+      name: 'rebuild-barebone-v1.0.2.img.xz',
+      id: 'v1.0.2-0',
+      url: 'http://example/barebone-v1.0.2',
+      size: 1000,
+    });
+  });
+});
+
+// The switch is a question about a list the page already has. Re-fetching to
+// answer it would put a GitHub round trip - and its rate limit - behind a
+// toggle.
+describe('the pre-release switch re-filters what is already loaded', () => {
+  it('needs only the cached payload to change the list', () => {
+    const payload = [release('v1.1.0-RC8', { prerelease: true }), release('v1.0.2')];
+
+    const hidden = selectRebuildImages(payload, false);
+    const shown = selectRebuildImages(payload, true);
+
+    expect(hidden).toHaveLength(2);
+    expect(shown).toHaveLength(4);
+    expect(names(shown)[0]).toContain('RC8');
+  });
+});

reflash/server.go

@@ -149,13 +149,18 @@ type AccessPoint struct {
 }
 
 type Options struct {
-	Darkmode       bool   `json:"darkmode"`
-	RebootWhenDone bool   `json:"rebootWhenDone"`
-	EnableSsh      bool   `json:"enableSsh"`
-	Magicmode      bool   `json:"magicmode"`
-	ScreenRotation int    `json:"screenRotation"`
-	WifiSSID       string `json:"SSID"`
-	WifiPSK        string `json:"PSK"`
+	Darkmode       bool `json:"darkmode"`
+	RebootWhenDone bool `json:"rebootWhenDone"`
+	EnableSsh      bool `json:"enableSsh"`
+	Magicmode      bool `json:"magicmode"`
+	// Off by default, so the picker offers only released images. Reflash has
+	// not been tested against prereleases any more than against the old
+	// versions the picker now hides, and an RC is not what someone flashing a
+	// printer wants by accident (#172).
+	ShowPrereleases bool   `json:"showPrereleases"`
+	ScreenRotation  int    `json:"screenRotation"`
+	WifiSSID        string `json:"SSID"`
+	WifiPSK         string `json:"PSK"`
 }
 
 type Download struct {
@@ -237,6 +242,12 @@ var binDir string
 var images_folder string
 var options_file string
 var log_file string
+
+// Where the flashing scripts report their progress. A var rather than the
+// literal it used to be so a test can point it at its sandbox: the helper
+// refuses to write real paths, because a test must not scribble on what a
+// server running on the same machine has open.
+var flash_progress_file = "/tmp/recore-flash-progress"
 var http_port string
 var reflashVersion string
 
@@ -1764,14 +1775,28 @@ func resetTransfer() {
 	bytesAtLastLog = 0
 }
 
+// The watchdog's share of the redraw: sample and repaint while an operation is
+// running, and cost nothing when one is not.
+//
+// Only while busy, because refreshProgress is not free - it reads the progress
+// file, recomputes the bandwidth and redraws the panel - and an idle board has
+// nothing new to say. The idle screen is repainted by the events that change
+// it (drive in, drive out, state transitions), which is where it always was.
+func refreshProgressWhenBusy() {
+	switch state.State {
+	case DOWNLOADING, UPLOADING, INSTALLING, BACKUPING, MAGIC, UPLOADING_MAGIC:
+		refreshProgress()
+	}
+}
+
 // refreshProgress reads the active progress source (the flash-progress file
 // while installing/backing up/magicking, or the downloading file's size),
 // recomputes state.Progress + state.Bandwidth, and redraws the embedded
 // screen. Called from both the HTTP getProgress handler and the USB STATUS
 // dispatcher so both paths keep the on-board display alive.
 func refreshProgress() {
 	if state.State == INSTALLING || state.State == BACKUPING || state.State == MAGIC {
-		bytes := lastLine("/tmp/recore-flash-progress")
+		bytes := lastLine(flash_progress_file)
 		i, err := strconv.Atoi(bytes)
 		if err != nil {
 			i = 0
@@ -2744,6 +2769,13 @@ var startWatchdog = func() {
 			// Give up on an upload whose client has gone away, rather than
 			// sitting in UPLOADING with the drive mounted rw forever (#118).
 			checkUploadLiveness()
+			// Keep the embedded screen moving while something is running, even
+			// with nothing watching. The redraw lived only inside
+			// refreshProgress, which runs from the HTTP poll and the USB STATUS
+			// command - both client-driven - so a flash started from the CI, or
+			// from the control protocol, or simply with the browser closed, ran
+			// to completion behind a frozen panel (#170).
+			refreshProgressWhenBusy()
 		}
 	}()
 }