From c63ef9176e3f7f56239c1a4f12ae87d4142c4a39 Mon Sep 17 00:00:00 2001 From: Elias Bakken Date: Fri, 18 Sep 2026 08:56:17 +0200 Subject: [PATCH 1/5] Write transfers to a .part file, so a cancel cannot destroy the image you have Both goDownload and uploadStart opened the destination with os.Create on the final name. That truncates an existing file, so starting a transfer of an image already on the drive destroyed it before a single new byte arrived - and the cleanup on cancel, added for #153, then removed what was left. Pressing Download on an image you already had and changing your mind lost it, as did a dropped network or a board reset mid-transfer. The bytes now land in .img.xz.part and are renamed into place only once they are all there, which also makes the replacement atomic. The suffix keeps partials out of getLocalImages' *.img.xz glob, so they never appear in the install list, and refreshProgress samples the partial because that is where the download is writing. TestDownloadCancelStopsTheTransfer watched the final name for its "the transfer has started" signal, which now only appears on completion; it watches the partial instead and additionally asserts that no image is left under the real name. Verified on A8 s/n 0498: with rebuild-barebone-v1.0.2.img.xz on the drive, starting the same download and cancelling left the original intact - same size, same mtime - and removed the .part. Co-Authored-By: Claude Opus 5 --- reflash/download_test.go | 11 +++- reflash/server.go | 46 +++++++++++--- reflash/server_test.go | 125 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 173 insertions(+), 9 deletions(-) diff --git a/reflash/download_test.go b/reflash/download_test.go index 4f99927..dd94bf1 100644 --- a/reflash/download_test.go +++ b/reflash/download_test.go @@ -63,17 +63,24 @@ func TestDownloadCancelStopsTheTransfer(t *testing.T) { startTestDownload(t, srv.URL, "slow.img.xz", 64<<20) path := filepath.Join(images_folder, "slow.img.xz") + // The bytes land in the partial now, and only become the real name once + // they are all there (#159) - so that is what says the transfer is under + // way. + part := path + ".part" waitFor(t, func() bool { - fi, err := os.Stat(path) + fi, err := os.Stat(part) return err == nil && fi.Size() > 0 }) cancelDownload(httptest.NewRecorder(), httptest.NewRequest("PUT", "/api/cancel_download", nil)) waitForState(t, CANCELLED) waitFor(t, disconnected.Load) - if _, err := os.Stat(path); !os.IsNotExist(err) { + if _, err := os.Stat(part); !os.IsNotExist(err) { t.Errorf("the cancelled download is still on the drive, stat err %v", err) } + if _, err := os.Stat(path); !os.IsNotExist(err) { + t.Errorf("a cancelled download must not leave an image under the real name, stat err %v", err) + } got, _ := os.ReadFile(mounts) if !strings.HasSuffix(strings.TrimSpace(string(got)), "mounted "+MODE_RO) { t.Errorf("drive not left read-only; mount calls %q", got) diff --git a/reflash/server.go b/reflash/server.go index c819c69..a6929c2 100644 --- a/reflash/server.go +++ b/reflash/server.go @@ -914,25 +914,26 @@ func startDownload(w http.ResponseWriter, r *http.Request) { func goDownload(ctx context.Context, filename string, url string) { disarmReboot() path := images_folder + "/" + filename + part := partialPath(filename) // Every way out removes what was written and puts the drive back to // read-only. This used to panic on a failed create or request, taking the // server down with it. fail := func(what string, err error) { logError(what + ": " + err.Error()) - os.Remove(path) + os.Remove(part) mountUsb(MODE_RO) state.State = ERROR state.Error = "The download failed: " + err.Error() } cancelled := func() { logInfo("Download cancelled.") - os.Remove(path) + os.Remove(part) mountUsb(MODE_RO) state.State = CANCELLED } - out, err := os.Create(path) + out, err := os.Create(part) if err != nil { fail("Could not create "+filename, err) return @@ -982,6 +983,13 @@ func goDownload(ctx context.Context, filename string, url string) { return } + // All the bytes are here, so the new image earns the real name now. Until + // this rename the drive still holds whatever it held before (#159). + if err := os.Rename(part, path); err != nil { + fail("Could not put "+filename+" in place", err) + return + } + duration := time.Since(timeStart) logInfo(fmt.Sprintf("Download finished in %d minutes and %d seconds", int(duration.Minutes()), int(duration.Seconds())%60)) mountUsb(MODE_RO) @@ -1022,7 +1030,7 @@ func uploadStart(w http.ResponseWriter, r *http.Request) { timeStart = time.Now() logInfo("Starting upload at " + timeStart.Format("15:04:05")) logInfo("Filename: " + state.Filename) - f, err := os.Create(images_folder + "/" + state.Filename) + f, err := os.Create(partialPath(state.Filename)) if err != nil { // Report the failure instead of log.Fatal. This runs on the USB // drive, so a full disk or a drive that dropped off the bus is an @@ -1508,6 +1516,16 @@ func uploadFinish(w http.ResponseWriter, r *http.Request) { } state.File = nil } + // Everything is written and flushed, so the upload earns the real name. Any + // image already on the drive under that name survived until this point + // (#159). + if err := os.Rename(partialPath(state.Filename), images_folder+"/"+state.Filename); err != nil { + logError("Could not put " + state.Filename + " in place: " + err.Error()) + state.Error = "The image was uploaded but could not be saved to the USB drive." + mountUsb(MODE_RO) + state.State = ERROR + return + } mountUsb(MODE_RO) duration := time.Since(timeStart) logInfo(fmt.Sprintf("Upload finished in %d minutes and %d seconds", int(duration.Minutes()), int(duration.Seconds())%60)) @@ -1553,9 +1571,11 @@ func uploadCancel(w http.ResponseWriter, r *http.Request) { } if !magic { // A cancelled or failed upload is a truncated image; left in the list - // it could only ever fail its integrity check (#153). + // it could only ever fail its integrity check (#153). Only the partial + // goes - an image already on the drive under the same name was never + // touched, and removing the final name here destroyed it (#159). if state.Filename != "" { - os.Remove(images_folder + "/" + state.Filename) + os.Remove(partialPath(state.Filename)) } mountUsb(MODE_RO) } @@ -1758,7 +1778,9 @@ func refreshProgress() { } state.BytesNow = i } else if state.State == DOWNLOADING { - fi, err := os.Stat(images_folder + "/" + state.Filename) + // The partial, not the final name: that is where the bytes are landing + // until the download completes and renames it into place (#159). + fi, err := os.Stat(partialPath(state.Filename)) if err == nil { state.BytesNow = int(fi.Size()) } @@ -1839,6 +1861,16 @@ func getProgress(w http.ResponseWriter, r *http.Request) { } } +// partialPath is where a download or an upload writes until it has every byte. +// Both used to write straight onto the final name, which os.Create truncates - +// so starting a transfer of an image the drive already held destroyed it before +// a single new byte arrived, and cancelling (or any failure) left nothing at +// all. The suffix keeps it out of getLocalImages' *.img.xz glob, so a partial +// never appears in the install list (#159). +func partialPath(filename string) string { + return images_folder + "/" + filename + ".part" +} + func getLocalImages() []Image { entries, err := filepath.Glob(images_folder + "/*.img.xz") if err != nil { diff --git a/reflash/server_test.go b/reflash/server_test.go index dd5a31c..6d6e65a 100644 --- a/reflash/server_test.go +++ b/reflash/server_test.go @@ -1785,3 +1785,128 @@ func TestRunCommand2TimeoutPassesOutputThrough(t *testing.T) { t.Errorf("stdout = %q, want \"hello\"", strings.TrimSpace(out)) } } + +// An image the drive already holds has to survive a transfer of the same name +// until every new byte has arrived. Both paths wrote straight onto the final +// name, so os.Create truncated the existing image on the first write and the +// cleanup on cancel then removed what was left - pressing Download or Upload on +// an image you already had and changing your mind destroyed it (#159). +func TestCancelledDownloadKeepsTheImageAlreadyOnTheDrive(t *testing.T) { + setupTest(t) + state = &State{State: IDLE} + + filename := "keepme.img.xz" + final := filepath.Join(images_folder, filename) + original := []byte("the image the user already had") + if err := os.WriteFile(final, original, 0o644); err != nil { + t.Fatal(err) + } + + // Holds the response open so the cancel lands mid-transfer rather than + // after the copy has already finished. + release := make(chan struct{}) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte("replacement bytes")) + w.(http.Flusher).Flush() + <-release + })) + // Deferred after srv.Close so LIFO releases the handler first: closing the + // server while a request is still parked on the channel deadlocks, and a + // failed assertion below would otherwise hang the whole test binary rather + // than reporting. + defer srv.Close() + defer close(release) + + body, _ := json.Marshal(map[string]any{ + "filename": filename, "url": srv.URL, "size": 1 << 20, "start_time": 0, + }) + startDownload(httptest.NewRecorder(), httptest.NewRequest("PUT", "/api/start_download", bytes.NewReader(body))) + + // Wait until bytes are actually landing, so this tests a cancel during the + // transfer and not a race with its start. + waitFor(t, func() bool { + fi, err := os.Stat(final + ".part") + return err == nil && fi.Size() > 0 + }) + + cancelDownload(httptest.NewRecorder(), httptest.NewRequest("PUT", "/api/cancel_download", nil)) + waitFor(t, func() bool { return state.State == CANCELLED }) + + got, err := os.ReadFile(final) + if err != nil { + t.Fatalf("the image the user already had is gone: %v", err) + } + if !bytes.Equal(got, original) { + t.Errorf("existing image was modified: got %q, want %q", got, original) + } + if _, err := os.Stat(final + ".part"); !os.IsNotExist(err) { + t.Error("the partial file was left behind") + } +} + +// The other half: a download that completes does replace the old image, and +// leaves no partial file in the images folder. +func TestCompletedDownloadReplacesTheImage(t *testing.T) { + setupTest(t) + state = &State{State: IDLE} + + filename := "replaceme.img.xz" + final := filepath.Join(images_folder, filename) + if err := os.WriteFile(final, []byte("stale"), 0o644); err != nil { + t.Fatal(err) + } + + fresh := []byte("a complete new image") + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write(fresh) + })) + defer srv.Close() + + body, _ := json.Marshal(map[string]any{ + "filename": filename, "url": srv.URL, "size": len(fresh), "start_time": 0, + }) + startDownload(httptest.NewRecorder(), httptest.NewRequest("PUT", "/api/start_download", bytes.NewReader(body))) + waitFor(t, func() bool { return state.State == FINISHED }) + + got, err := os.ReadFile(final) + if err != nil { + t.Fatalf("reading the downloaded image: %v", err) + } + if !bytes.Equal(got, fresh) { + t.Errorf("image content = %q, want %q", got, fresh) + } + if _, err := os.Stat(final + ".part"); !os.IsNotExist(err) { + t.Error("the partial file was left behind") + } +} + +// Same guarantee for the upload path, which had the same os.Create. +func TestCancelledUploadKeepsTheImageAlreadyOnTheDrive(t *testing.T) { + setupTest(t) + state = &State{State: IDLE} + + filename := "keepme-upload.img.xz" + final := filepath.Join(images_folder, filename) + original := []byte("the image the user already had") + if err := os.WriteFile(final, original, 0o644); err != nil { + t.Fatal(err) + } + + startBody, _ := json.Marshal(map[string]any{ + "filename": filename, "size": 1 << 20, "start_time": 0, + }) + uploadStart(httptest.NewRecorder(), httptest.NewRequest("PUT", "/api/upload_start", bytes.NewReader(startBody))) + uploadChunk(httptest.NewRecorder(), httptest.NewRequest("POST", "/api/upload_chunk", bytes.NewReader([]byte("partial bytes")))) + uploadCancel(httptest.NewRecorder(), httptest.NewRequest("PUT", "/api/upload_cancel", nil)) + + got, err := os.ReadFile(final) + if err != nil { + t.Fatalf("the image the user already had is gone: %v", err) + } + if !bytes.Equal(got, original) { + t.Errorf("existing image was modified: got %q, want %q", got, original) + } + if _, err := os.Stat(final + ".part"); !os.IsNotExist(err) { + t.Error("the partial file was left behind") + } +} From 7c5f8ee33e71cce75a944b642cb8942e909bcb14 Mon Sep 17 00:00:00 2001 From: Elias Bakken Date: Fri, 18 Sep 2026 08:56:30 +0200 Subject: [PATCH 2/5] Fix three things the install row got wrong (#160, #161, #162) All three were found in one pass over the UI on A8 s/n 0498, and all three live in App.vue. #160, the one that can cost you an image: the grid cell holding the select, the integrity icon and Delete is a fixed fraction of the row - 125px at a 960px viewport - while its contents are wider than that. With nowrap they overflowed the cell and were drawn on top of the Install button in the next one, so the two shared the same pixels and a click meant for Install could delete instead. Wrapping puts Delete on its own line. Measured at 860, 960, 1100 and full width: no overlap at any of them, and desktop widths are unchanged because everything still fits on one line. #161: getInfo() is fetched once per page load, which is right for the version, revision and serial number but not for emmc_version - a flash is precisely what changes it. The pipeline went on showing the image that was on the eMMC before, on the one screen meant to confirm the flash worked. The three paths that write the eMMC now share onFlashFinished(), so the refresh cannot be added to one and forgotten in the others. #162: onSelectedFileChanged told the integrity checker only when a file was selected, so clearing the selection - which is what deleting the selected image does - left the old verdict on screen. A green check next to "Please select one" reads as "the thing I am about to install is fine". fileSelected() already hides the icon for an empty name; it just has to be called. #160 and #161 were verified on the board. #162 has unit tests rather than a live check: after an install the drive is unmounted, so the image list is empty and there is nothing to select. Co-Authored-By: Claude Opus 5 --- client/src/App.vue | 39 ++++++++++++++++++++----- client/tests/unit/InstallGating.spec.js | 36 +++++++++++++++++++++++ 2 files changed, 67 insertions(+), 8 deletions(-) diff --git a/client/src/App.vue b/client/src/App.vue index 49818a4..30c81ae 100644 --- a/client/src/App.vue +++ b/client/src/App.vue @@ -155,7 +155,15 @@ {{ this.computeTransferButtonText() }} - + + { expect(w.vm.spinner_visible).toBe(false); }); }); + +// The badge describes one specific file. Clearing the selection - which is what +// deleting the selected image does - has to clear the verdict with it, or a +// green check from the file that is now gone sits next to "Please select one" +// and reads as "the thing I am about to install is fine" (#162). +describe('the integrity verdict belongs to the selected image (#162)', () => { + function stand(selected) { + const fileSelected = vi.fn(); + const self = { + selectedLocalImage: selected, + imageIntegrity: true, + $refs: { integritychecker: { fileSelected } }, + }; + return { self, fileSelected }; + } + + it('tells the checker when the selection is cleared', () => { + const { self, fileSelected } = stand(null); + + App.methods.onSelectedFileChanged.call(self); + + expect(self.imageIntegrity).toBe(null); + // Called with the empty selection rather than skipped: fileSelected() + // hides the icon for an empty name, and skipping the call is exactly what + // left the old verdict on screen. + expect(fileSelected).toHaveBeenCalledWith(null); + }); + + it('still checks a newly selected image', () => { + const { self, fileSelected } = stand('fresh.img.xz'); + + App.methods.onSelectedFileChanged.call(self); + + expect(fileSelected).toHaveBeenCalledWith('fresh.img.xz'); + }); +}) From f93bbbbdacb132111e468ea5f7fb9d185433972d Mon Sep 17 00:00:00 2001 From: Elias Bakken Date: Fri, 18 Sep 2026 08:56:42 +0200 Subject: [PATCH 3/5] Ask twice before rebooting, and give the serial number dialog a way out (#163) Reboot now and Shut down sit next to each other and fired on a single click, while Delete - which destroys far less - asks twice. The two outcomes are not equally cheap either: a stray Shut down on a headless board needs someone to walk over and power-cycle it. Both now use the same two-click pattern as Delete, including its reason for not using confirm(): a native dialog blocks the page. Arming one disarms the other, so a click meant for Reboot cannot confirm a Shut down armed moments earlier. The Recore Serial Number dialog offered only "Set Serial Number", so anyone who opened it to read the number had to know that clicking the backdrop dismisses it - and on the board's own touchscreen there is not much backdrop to aim at. Adding Cancel surfaced a second bug: the dialog never told the parent it had closed, so App's openSerialNumber stayed true after any dismissal, the false -> true watch never fired again, and the button stopped opening the dialog altogether. It now emits close whichever way it goes away. Verified on A8 s/n 0498: first click arms, clicking the other button disarms the first, both disarm after a few seconds; Cancel closes the dialog and the button reopens it. Co-Authored-By: Claude Opus 5 --- client/src/components/TheConfigUpdater.vue | 16 +++++++ client/src/components/TheOptions.vue | 36 +++++++++++++-- client/tests/unit/TheOptions.spec.js | 54 ++++++++++++++++++++++ 3 files changed, 102 insertions(+), 4 deletions(-) diff --git a/client/src/components/TheConfigUpdater.vue b/client/src/components/TheConfigUpdater.vue index 30443b3..e224eb6 100644 --- a/client/src/components/TheConfigUpdater.vue +++ b/client/src/components/TheConfigUpdater.vue @@ -18,6 +18,10 @@ >

Config has been updated

+ Set Serial Number + Cancel diff --git a/client/src/components/TheOptions.vue b/client/src/components/TheOptions.vue index c7cd0a9..3d3168f 100644 --- a/client/src/components/TheOptions.vue +++ b/client/src/components/TheOptions.vue @@ -58,12 +58,19 @@

Actions

+
- - Reboot now + + {{ pending === "reboot" ? "Reboot?" : "Reboot now" }} - Shut down{{ + pending === "shutdown" ? "Shut down?" : "Shut down" + }}
@@ -84,6 +91,24 @@ export default { this.setOption(data); this.$emit("set-option", name, value); }, + // First click arms the button, second one within a few seconds does it. + // Arming one disarms the other, so a click meant for Reboot cannot confirm + // a pending Shut down. + confirmAction(which) { + clearTimeout(this.pendingTimer); + if (this.pending !== which) { + this.pending = which; + this.pendingTimer = setTimeout(() => (this.pending = null), 4000); + return; + } + this.pending = null; + this.$emit(which === "reboot" ? "reboot-board" : "shutdown-board"); + }, + }, + // A drawer that is closed and reopened must not still be holding an armed + // button from minutes ago. + beforeUnmount() { + clearTimeout(this.pendingTimer); }, computed: mapGetters(["options"]), created() { @@ -95,6 +120,9 @@ export default { open: Boolean, }, data: () => ({ + // "reboot", "shutdown", or null when neither is armed. + pending: null, + pendingTimer: null, radioItems: [ { label: "Normal", value: 0 }, { label: "90 degrees", value: 90 }, diff --git a/client/tests/unit/TheOptions.spec.js b/client/tests/unit/TheOptions.spec.js index 84e4172..b2554cc 100644 --- a/client/tests/unit/TheOptions.spec.js +++ b/client/tests/unit/TheOptions.spec.js @@ -59,3 +59,57 @@ describe('TheOptions', () => { expect(binding[1]).toBe('screenRotation') }) }) + +// Reboot and Shut down sit next to each other and used to fire on one click. +// The two outcomes are not equally cheap: a stray Shut down on a headless board +// needs a person to go and power-cycle it (#163). +describe('TheOptions destructive actions', () => { + it('does not reboot or shut down on the first click', () => { + const { wrapper } = mountOptions() + + wrapper.vm.confirmAction('reboot') + wrapper.vm.confirmAction('shutdown') + + expect(wrapper.emitted('reboot-board')).toBeUndefined() + expect(wrapper.emitted('shutdown-board')).toBeUndefined() + }) + + it('acts on the second click of the same button', () => { + const { wrapper } = mountOptions() + + wrapper.vm.confirmAction('reboot') + wrapper.vm.confirmAction('reboot') + + expect(wrapper.emitted('reboot-board')).toHaveLength(1) + expect(wrapper.emitted('shutdown-board')).toBeUndefined() + }) + + // Arming one has to disarm the other, or a click meant for Reboot would + // confirm a Shut down that was armed moments earlier - the precise mistake + // the confirmation exists to prevent. + it('arming the other button cancels the pending one', () => { + const { wrapper } = mountOptions() + + wrapper.vm.confirmAction('reboot') + wrapper.vm.confirmAction('shutdown') + wrapper.vm.confirmAction('shutdown') + + expect(wrapper.emitted('reboot-board')).toBeUndefined() + expect(wrapper.emitted('shutdown-board')).toHaveLength(1) + }) + + it('disarms itself after a few seconds so a stale click does not act', () => { + vi.useFakeTimers() + try { + const { wrapper } = mountOptions() + + wrapper.vm.confirmAction('reboot') + vi.advanceTimersByTime(5000) + wrapper.vm.confirmAction('reboot') + + expect(wrapper.emitted('reboot-board')).toBeUndefined() + } finally { + vi.useRealTimers() + } + }) +}) From 82ac64a4bccb3f38943554fba8a1d62858f62319 Mon Sep 17 00:00:00 2001 From: Elias Bakken Date: Fri, 18 Sep 2026 08:56:53 +0200 Subject: [PATCH 4/5] Never count down past zero, and make the progress line readable (#163) The remaining time is a projection off a clock, and it could run backwards: the figure was split into minutes and seconds without ever being clamped, and Math.floor(-4 % 60) is -4, so "0m:-4s" reached the screen. Two ways in - a start time in the future, because the board and the browser do not share a clock, and a progress figure at or past 100 while the estimate is still being recomputed. Clamped before the split, so neither part can carry the sign. The elapsed time gets the same treatment, for the same reason. Two readability fixes alongside it. The metrics popup was rgba(20,20,20,0.92), so the page behind showed through the plots - the REFLASH wordmark and the version line ran underneath the numbers being read. And the three figures under the bar are spread by justify-space-between, which leaves no space at all once the row is narrow: elapsed, rate and remaining ran together as "3.7 MB/s3m:13s". The .wrapper class they sit in had no styles at all; it has a gap now. Co-Authored-By: Claude Opus 5 --- client/src/components/ProgressBar.vue | 28 ++++++++++++++-- client/tests/unit/ProgressBar.spec.js | 47 +++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 3 deletions(-) diff --git a/client/src/components/ProgressBar.vue b/client/src/components/ProgressBar.vue index acdc995..a2cbb24 100644 --- a/client/src/components/ProgressBar.vue +++ b/client/src/components/ProgressBar.vue @@ -111,7 +111,12 @@ export default { }, update: function() { let model = this.progress; - let timePassedSeconds = (Date.now() - model.timeStarted)/1000; + // Never below zero. Both figures are derived from a clock and a + // projection, and either can run backwards: a start time in the future + // (the board and the browser do not share a clock), or a progress figure + // at or past 100 while the estimate is still being recomputed. The + // result was a countdown showing a negative number of seconds. + let timePassedSeconds = Math.max(0, (Date.now() - model.timeStarted)/1000); this.seconds = Math.floor(timePassedSeconds % 60) ; this.minutes = Math.floor(timePassedSeconds / (60)); let progress = model.progress/100; @@ -120,9 +125,15 @@ export default { let secondsTotal = (timePassedSeconds/progress); let timeFinished = new Date(new Date(model.timeStarted).getTime() + secondsTotal*1000); let timeRemaining = (timeFinished - Date.now())/1000; + // Clamped before it is split into minutes and seconds, so neither part + // can carry the sign: Math.floor(-4 % 60) is -4, which is how "0m:-4s" + // reached the screen. + if (!isFinite(timeRemaining) || timeRemaining < 0) { + timeRemaining = 0; + } this.secondsR = Math.floor(timeRemaining % 60); this.minutesR = Math.floor(timeRemaining / 60); - if(isNaN(this.secondsR) || this.seconds == -1){ + if(isNaN(this.secondsR)){ this.secondsR = 0 this.minutesR = 0 } @@ -138,6 +149,14 @@ export default { padding: 0.35em 0; cursor: crosshair; } +/* The three figures under the bar are spread by justify-space-between, which + leaves no space at all once the row is narrow: elapsed, rate and remaining + ran together as "3.7 MB/s3m:13s". A gap keeps them apart, and nowrap stops a + single figure being broken across lines (#163). */ +.wrapper { + gap: 0.6em; + white-space: nowrap; +} .metrics-popup { position: fixed; width: min(820px, calc(100vw - 16px)); @@ -146,7 +165,10 @@ export default { z-index: 1000; padding: 0.4em 0.6em; border-radius: 6px; - background: rgba(20, 20, 20, 0.92); + /* Opaque. At 0.92 the page behind showed through the panel - the REFLASH + wordmark and the version line ran underneath the plots, which is exactly + where the numbers are read from (#163). */ + background: #141414; color: #eee; box-shadow: 0 2px 10px rgba(0, 0, 0, 0.45); pointer-events: auto; diff --git a/client/tests/unit/ProgressBar.spec.js b/client/tests/unit/ProgressBar.spec.js index 038548e..02d5fbf 100644 --- a/client/tests/unit/ProgressBar.spec.js +++ b/client/tests/unit/ProgressBar.spec.js @@ -96,3 +96,50 @@ describe('ProgressBar board metrics hover', () => { expect(w.vm.metricsVisible).toBe(false); }); }); + +// The countdown is a projection off a clock, so it can run backwards: a start +// time in the future (the board and the browser do not share one), or a +// progress figure at 100 while the estimate is still being recomputed. Neither +// is a reason to show the user "0m:-4s". +describe('the remaining time is never negative', () => { + function mountWith(progressModel) { + return shallowMount(ProgressBar, { + props: { revision: 'A8' }, + global: { + mocks: { $store: { getters: { progress: progressModel } } }, + stubs: { 'w-progress': true, 'w-flex': true, TheMetrics: true } + } + }); + } + + it('clamps a start time in the future', () => { + const w = mountWith({ progress: 50, bandwidth: 1.5, timeStarted: Date.now() + 60000 }); + w.vm.update(); + expect(w.vm.secondsR).toBeGreaterThanOrEqual(0); + expect(w.vm.minutesR).toBeGreaterThanOrEqual(0); + expect(w.vm.seconds).toBeGreaterThanOrEqual(0); + expect(w.vm.minutes).toBeGreaterThanOrEqual(0); + }); + + it('clamps a progress figure past 100', () => { + const w = mountWith({ progress: 140, bandwidth: 8, timeStarted: Date.now() - 30000 }); + w.vm.update(); + expect(w.vm.secondsR).toBeGreaterThanOrEqual(0); + expect(w.vm.minutesR).toBeGreaterThanOrEqual(0); + }); + + it('reports zero rather than NaN before any progress has been made', () => { + const w = mountWith({ progress: 0, bandwidth: 0, timeStarted: Date.now() }); + w.vm.update(); + expect(w.vm.secondsR).toBe(0); + expect(w.vm.minutesR).toBe(0); + }); + + it('still reports a real estimate for an ordinary transfer', () => { + // Half done after 60s, so about 60s left. + const w = mountWith({ progress: 50, bandwidth: 5, timeStarted: Date.now() - 60000 }); + w.vm.update(); + expect(w.vm.minutesR * 60 + w.vm.secondsR).toBeGreaterThan(50); + expect(w.vm.minutesR * 60 + w.vm.secondsR).toBeLessThan(70); + }); +}); From 478cfeb2d8f87f503619c8250b8be0f6c90bf6c4 Mon Sep 17 00:00:00 2001 From: Elias Bakken Date: Fri, 18 Sep 2026 10:13:19 +0200 Subject: [PATCH 5/5] Simplify the countdown guard to a plain clamp The branch on isFinite and the explicit negative test were doing what Math.max(0, ...) does on its own. The pre-existing isNaN guard below still covers progress == 0, where the projection is Infinity and the subtraction comes out NaN - Math.max passes that through untouched. No behaviour change; the four tests around it are unchanged and still pass. Co-Authored-By: Claude Opus 5 --- client/src/components/ProgressBar.vue | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/client/src/components/ProgressBar.vue b/client/src/components/ProgressBar.vue index a2cbb24..cf9d955 100644 --- a/client/src/components/ProgressBar.vue +++ b/client/src/components/ProgressBar.vue @@ -111,11 +111,8 @@ export default { }, update: function() { let model = this.progress; - // Never below zero. Both figures are derived from a clock and a - // projection, and either can run backwards: a start time in the future - // (the board and the browser do not share a clock), or a progress figure - // at or past 100 while the estimate is still being recomputed. The - // result was a countdown showing a negative number of seconds. + // Clamped: a start time in the future (the board and the browser do not + // share a clock) otherwise runs the elapsed count backwards too. let timePassedSeconds = Math.max(0, (Date.now() - model.timeStarted)/1000); this.seconds = Math.floor(timePassedSeconds % 60) ; this.minutes = Math.floor(timePassedSeconds / (60)); @@ -124,13 +121,9 @@ export default { let secondsTotal = (timePassedSeconds/progress); let timeFinished = new Date(new Date(model.timeStarted).getTime() + secondsTotal*1000); - let timeRemaining = (timeFinished - Date.now())/1000; - // Clamped before it is split into minutes and seconds, so neither part - // can carry the sign: Math.floor(-4 % 60) is -4, which is how "0m:-4s" - // reached the screen. - if (!isFinite(timeRemaining) || timeRemaining < 0) { - timeRemaining = 0; - } + // Clamped before the split, so neither part can carry the sign: + // Math.floor(-4 % 60) is -4, which is how "0m:-4s" reached the screen. + let timeRemaining = Math.max(0, (timeFinished - Date.now())/1000); this.secondsR = Math.floor(timeRemaining % 60); this.minutesR = Math.floor(timeRemaining / 60); if(isNaN(this.secondsR)){