From bb8aad22c4d41dd1b89c85c8d46878a31a9530e5 Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Tue, 8 Sep 2026 13:21:02 +0200 Subject: fix(hub): never collect a large download in the page MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `pipelinedDownload` with no writable allocates `new Array(totalChunks)` and keeps every decrypted chunk, so whatever `_openDownloadTarget` returns null for is held whole in RAM. That floor had no upper bound: the `!window.showSaveFilePicker` branch returned null at any size, so on a browser without the File System Access API a 20 GB film went to memory whenever the streamed path did not answer. Nothing logged, nothing refused; the symptom was the tab dying, with no error attributable to this code. MEMORY_CEILING is 100 MB and every `return null` in that chain now goes through a guard that throws above it. The refusal names the size, the limit and why the streamed path declined, and lands in the transfers panel as a failed transfer rather than in a console nobody opens. This is a guard, not a limit on what can be downloaded: with the streamed path primed and retried (previous commit), a file of any size still goes to disk progressively on every browser. Two things had to change for that to be true: - the streamed path is now tried in "ask" mode too, for a file over the ceiling on a browser with no Save As of its own. The mode decides whether to show a dialog; it was silently deciding whether a film could be downloaded at all; - FilePreview had no size check whatsoever — a multi-gigabyte PDF or .csv was fetched whole, and the text branch decoded all of it to keep 500 000 characters. It refuses above the same ceiling and offers the download. ZIP_MAX_BYTES (512 MB) and the ceiling do not contradict: the archive limit bounds the archive, the ceiling bounds what may be built in the page, so a 400 MB zip is allowed when there is somewhere to stream it and refused when the only route left is memory. The build-in-memory confirmation only appears below the ceiling now. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HCGdheDLxGReuKHga3BtST --- packages/meshbay-hub/tests/test_zip_size_limit.py | 59 ++++++++++++++++++++--- 1 file changed, 53 insertions(+), 6 deletions(-) (limited to 'packages/meshbay-hub/tests/test_zip_size_limit.py') diff --git a/packages/meshbay-hub/tests/test_zip_size_limit.py b/packages/meshbay-hub/tests/test_zip_size_limit.py index 203c10c..44ad59e 100644 --- a/packages/meshbay-hub/tests/test_zip_size_limit.py +++ b/packages/meshbay-hub/tests/test_zip_size_limit.py @@ -6,11 +6,16 @@ folder as a zip" button — Files' single folder, Files' multi-folder selection, and the Photos album button (docs/photos.md §3) — so the limit is checked once, there, and holds for all of them. -Two things are worth pinning. That an oversized folder is refused *before* +Three things are worth pinning. That an oversized folder is refused *before* `_openDownloadTarget`, because a save dialog for an archive that will never be -written is worse than no dialog at all. And that a folder at exactly the limit +written is worse than no dialog at all. That a folder at exactly the limit still goes through, since an off-by-one here silently costs a whole megabyte -of allowance and nobody would ever notice. +of allowance and nobody would ever notice. And that the two limits in play do +not contradict each other: ZIP_MAX_BYTES (512 MB) bounds the archive, while +MEMORY_CEILING (100 MB, test_memory_ceiling.py) bounds what may be built in the +page — so a 400 MB zip is allowed when there is somewhere to stream it and +refused when the only route left is memory. The `confirm()` that offers the +build-in-memory path therefore only ever appears below the ceiling. """ import json @@ -30,7 +35,7 @@ pytestmark = pytest.mark.skipif( MIB = 1024 * 1024 -def _run(total_bytes, tmp_path): +def _run(total_bytes, tmp_path, picker=False): """ Call downloadDirectory over one folder holding `total_bytes`, and report what it did: the errors it set, how many times it put a question to the @@ -44,6 +49,7 @@ def _run(total_bytes, tmp_path): (tmp_path / "package.json").write_text('{"type":"module"}') script = tmp_path / "case.mjs" + picker_js = "true" if picker else "false" script.write_text(f""" const store = new Map(); globalThis.localStorage = {{ @@ -60,6 +66,17 @@ const out = {{ errors: [], started: 0, asked: 0 }}; // asks first. Answering yes is what lets the at-the-limit case get as far as // starting a transfer, and `asked` is how the refusal proves it never did. globalThis.confirm = () => {{ out.asked += 1; return true; }}; +// With `picker`, the browser can stream to a file the person chooses, which is +// the only legal route for an archive over MEMORY_CEILING. Never exercised — +// the stubbed `transfers.start` below does not run the job — it just has to be +// a target rather than null. +if ({picker_js}) {{ + window.showSaveFilePicker = async () => ({{ + name: 'album.zip', + createWritable: async () => ({{ write: async () => {{}}, close: async () => {{}}, + abort: async () => {{}} }}), + }}); +}} const M = await import('{(sandbox / "file-utils.js").as_posix()}'); @@ -101,7 +118,37 @@ def test_an_oversized_folder_is_refused_before_anything_opens(tmp_path): def test_a_folder_exactly_at_the_limit_still_downloads(tmp_path): - """The bound is inclusive: `> ZIP_MAX_BYTES`, not `>=`.""" - result = _run(512 * MIB, tmp_path) + """The bound is inclusive: `> ZIP_MAX_BYTES`, not `>=`. + + Given somewhere to stream to, because 512 MB is five times MEMORY_CEILING + and building it in the page is no longer a route this code will take. That + is what the next test is about; this one is still only about the off-by-one. + """ + result = _run(512 * MIB, tmp_path, picker=True) assert result["errors"] == [] assert result["started"] == 1 + assert result["asked"] == 0, "nothing is built in memory when it can stream" + + +def test_a_zip_over_the_memory_ceiling_is_refused_when_nothing_streams(tmp_path): + """ + Between the two limits — larger than the page may hold, smaller than the + archive limit — and no way to stream it. Before the ceiling existed this + asked "build it in memory?" and, on yes, held 400 MB in the tab. + + The refusal names the memory ceiling, not the zip limit: quoting 512 MB at + someone whose folder is under 512 MB would be a message about the wrong + rule. + """ + result = _run(400 * MIB, tmp_path) + assert result["started"] == 0 + assert result["asked"] == 0, ( + "the person must not be offered a build-in-memory path above the ceiling") + assert result["errors"] and "group.zip_too_large" not in result["errors"][0] + + +def test_a_small_folder_may_still_be_built_in_memory(tmp_path): + """The floor is intact below the ceiling — that is what it is for.""" + result = _run(4 * MIB, tmp_path) + assert result["errors"] == [] + assert result["asked"] == 1 and result["started"] == 1 -- cgit v1.2.3 From 1a495f5ed3f8a55222d406152c833882264dc377 Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Tue, 8 Sep 2026 22:54:16 +0200 Subject: feat(hub): client-side transfer leases and the transfers panel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Steps 5 and 6 of ~/next/improve-downloads.md. The node has handed out slots since step 2 and nothing asked for one; now the client does, and the panel shows what is happening. `transport.openTransfer()` returns a Lease: `acquire()` resolves when the node grants, `release()` gives it back exactly once, and nothing else in the client speaks to the node about slots. Whether a node hands out slots is read from the handshake ack rather than guessed from a timeout — "no answer yet" and "this node will never answer" are indistinguishable in time, and guessing wrong either stalls every download or defeats the cap. Two things exist only because a queue can lie: a watchdog re-asks when a pushed grant does not arrive (the node is idempotent on `tr`, so asking again is free), and a grant for a transfer the page has forgotten is handed straight back rather than held until the node's deadline. The slot is asked for **after** there is somewhere to write, and that ordering is load-bearing: opening a target takes thirty seconds of streamed-download timeouts, or as long as somebody leaves a Save As dialog open, and a grant not taken up in time is revoked. Moving it earlier looked better and broke three downloads into one. Pinned by a test. The panel groups by state — running, waiting, finished — rather than re-sorting a flat list, so a row moves only when its own state does. The ETA is withheld until the speed window holds real measurement: a figure from the first two chunks swings between four seconds and an hour, and people plan around the first number they see. One live region announces state changes and not progress. Three silent paths closed on the way: a download refused for want of a user gesture (a browser grants one file picker per gesture, and downloading three files is one gesture) now falls back to the streamed path, which needs none; a click with no connection says so instead of doing nothing at all; and a queued transfer counts as busy, so a transport is never closed under one that is waiting for a grant that could then never arrive. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HCGdheDLxGReuKHga3BtST --- packages/meshbay-hub/src/meshbay_hub/static/app.js | 173 ++++++++++++----- .../src/meshbay_hub/static/file-utils.js | 96 ++++++++-- .../src/meshbay_hub/static/files-app.js | 14 +- .../src/meshbay_hub/static/group-page.js | 7 +- .../src/meshbay_hub/static/locales/de.js | 12 ++ .../src/meshbay_hub/static/locales/en.js | 12 ++ .../src/meshbay_hub/static/locales/es.js | 12 ++ .../src/meshbay_hub/static/locales/fr.js | 12 ++ .../src/meshbay_hub/static/locales/it.js | 12 ++ .../src/meshbay_hub/static/locales/ja.js | 12 ++ .../src/meshbay_hub/static/locales/nl.js | 12 ++ .../src/meshbay_hub/static/locales/pl.js | 12 ++ .../src/meshbay_hub/static/locales/pt-BR.js | 12 ++ .../src/meshbay_hub/static/locales/zh-CN.js | 12 ++ .../meshbay-hub/src/meshbay_hub/static/style.css | 64 +++++++ .../src/meshbay_hub/static/transfers.js | 105 +++++++++- .../src/meshbay_hub/static/transport.js | 213 ++++++++++++++++++++- packages/meshbay-hub/tests/test_layout_measured.py | 119 +++++++++++- packages/meshbay-hub/tests/test_memory_ceiling.py | 36 +++- packages/meshbay-hub/tests/test_transfers.py | 213 +++++++++++++++++++++ packages/meshbay-hub/tests/test_zip_size_limit.py | 12 +- 21 files changed, 1090 insertions(+), 82 deletions(-) (limited to 'packages/meshbay-hub/tests/test_zip_size_limit.py') diff --git a/packages/meshbay-hub/src/meshbay_hub/static/app.js b/packages/meshbay-hub/src/meshbay_hub/static/app.js index 9d64292..ff066cf 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/app.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/app.js @@ -4,7 +4,7 @@ import { } from './vendor/htm-preact.js'; import { t, getLocale, setLocale, initLocale, LOCALES } from './i18n.js'; import { ZipStream, entriesUnder } from './zipstream.js'; -import { transfers, formatSpeed } from './transfers.js'; +import { transfers, formatSpeed, etaSeconds } from './transfers.js'; import * as platform from './platform.js'; import * as downloads from './downloads.js'; import { Icon } from './icon.js'; @@ -157,69 +157,67 @@ function TransferWidget() { }, [open]); const running = items.filter(i => i.status === 'running'); + const waiting = items.filter(i => i.status === 'queued'); + const finished = items.filter( + i => i.status !== 'running' && i.status !== 'queued'); + const active = running.length + waiting.length; + + // Grouped, and in this order: what is moving, what is waiting, what is over. + // Re-sorting the flat list on every emit made rows jump under the pointer + // each time a neighbour finished — the group is what changes, not the + // position within it, so a row only moves when its own state does. + const groups = [ + ['running', running], + ['waiting', waiting], + ['finished', finished], + ].filter(([, rows]) => rows.length); + if (!items.length) return null; return html`
+ ${/* One live region for the panel, announcing what changed state rather + than every progress tick — a reader that says "62%… 63%… 64%" for a + four-gigabyte film is a reader nobody leaves on. */''} + + ${t('transfers.summary', { running: running.length, waiting: waiting.length })} + ${open && html` -
+
- ${t('transfers.title')} - + ${t('transfers.title')} + ${active > 0 && html` + + ${t('transfers.summary', { + running: running.length, waiting: waiting.length })} + + `} + ${finished.length > 0 && html` + + `}
- ${items.map(it => html` -
-
- - <${Icon} name=${it.kind === 'upload' ? 'upload' : 'download'} /> - - ${it.canOpen - ? html` { e.preventDefault(); transfers.open(it.id); }}>${it.name}` - : html`${it.name}`} - ${it.status === 'running' && html` - - `} -
- ${it.status === 'running' - ? html` -
-
-
-
- ${formatSize(it.done)}${it.total - ? ' / ' + formatSize(it.total) : ''} - ${formatSpeed(it.speed)} -
- ` - : html` -
- - ${it.status === 'done' ? t('transfers.done') - : it.status === 'cancelled' ? t('transfers.cancelled') - : it.error || t('transfers.failed')} - - ${it.canOpen && html` - - `} -
- `} + ${groups.map(([label, rows]) => html` +
+ ${groups.length > 1 && html` +
+ ${t('transfers.group_' + label, { n: rows.length })} +
+ `} + ${rows.map(it => html`<${TransferRow} it=${it} key=${it.id} />`)}
`)}
@@ -228,6 +226,79 @@ function TransferWidget() { `; } +/** One row. Split out so the panel above reads as a layout and this as a state + * machine — they change for different reasons. */ +function TransferRow({ it }) { + const eta = etaSeconds(it); + return html` +
+
+ + ${it.canOpen + ? html` { e.preventDefault(); transfers.open(it.id); }}>${it.name}` + : html`${it.name}`} + ${(it.status === 'running' || it.status === 'queued') && html` + + `} +
+ ${it.status === 'queued' + ? html` +
+
+ ${it.queuedByOwnLimit + ? t('transfers.waiting_own_slots') + : t('transfers.waiting_node', { n: it.ahead })} + ${formatSize(it.total)} +
+ ` + : it.status === 'running' + ? html` +
+
+
+
+ ${formatSize(it.done)}${it.total + ? ' / ' + formatSize(it.total) : ''} + ${[formatSpeed(it.speed), + it.settled && eta !== null ? formatEta(eta) : ''] + .filter(Boolean).join(' · ')} +
+ ` + : html` +
+ + ${it.status === 'done' ? t('transfers.done') + : it.status === 'cancelled' ? t('transfers.cancelled') + : it.error || t('transfers.failed')} + + ${it.canOpen && html` + + `} +
+ `} +
+ `; +} + +/** "4 min left". Coarse on purpose: a per-second countdown on a transfer whose + * speed varies is a number that is wrong most of the time and looks precise. */ +function formatEta(seconds) { + if (seconds < 60) return t('transfers.eta_seconds', { n: Math.ceil(seconds) }); + if (seconds < 3600) return t('transfers.eta_minutes', { n: Math.round(seconds / 60) }); + return t('transfers.eta_hours', { n: Math.round(seconds / 360) / 10 }); +} + // ── Nav ────────────────────────────────────────────────────────────────────── function Nav({ user, theme, onThemeChange, onLogout, onMenuToggle, unreadCount, diff --git a/packages/meshbay-hub/src/meshbay_hub/static/file-utils.js b/packages/meshbay-hub/src/meshbay_hub/static/file-utils.js index a942fd5..61002d6 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/file-utils.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/file-utils.js @@ -164,6 +164,25 @@ async function _openDownloadTarget(filename, size = 0, pickerOpts = {}, return { writable: await handle.createWritable(), name: handle.name || filename }; } catch (err) { if (err.name === 'AbortError') return false; + // "Must be handling a user gesture to show a file picker." + // + // A browser grants one picker per gesture, and downloading three files is + // one gesture. So the second and third throw this, and the person sees a + // failed transfer with a message from Chrome about gestures, for having + // done something entirely reasonable. + // + // The streamed path needs no gesture at all, which makes it the right + // answer here rather than a consolation: the file still lands on disk, in + // the browser's own download folder, written as it arrives. Only the choice + // of folder is lost, and it was already lost — there was no picker to make + // it in. + if (err.name === 'SecurityError' || /user gesture/i.test(err.message || '')) { + console.warn('[MeshBay] no gesture left for a save dialog; streaming ' + + 'this one to the download folder instead'); + const streamed = await downloads.openStreamedDownload(filename, swSize); + if (streamed) return streamed; + if (size <= MEMORY_CEILING) return _memoryFloor(); + } throw err; } } @@ -193,17 +212,47 @@ function _saveBlob(blob, filename) { const CHUNK_RETRY_ATTEMPTS = 6; const CHUNK_RETRY_DELAY_MS = 1500; +// How long one megabyte may take to reach the disk before we call it stuck. +// +// Every other await on this path is bounded and says so when it expires: +// `_sendAndWait` logs a Response timeout, `_fetchChunkResilient` retries and +// then throws. `writable.write()` was the exception — a sink that stops +// consuming (a service-worker stream the browser has stopped reading, a file +// handle that has gone away) leaves it pending for ever. It never rejects, so +// there is no error, no log and no failed transfer: the progress bar simply +// stops, the console stays empty, and the node is perfectly healthy the whole +// time, which is what made this invisible. +// +// Generous on purpose. A megabyte takes milliseconds on any working sink; a +// minute means the sink is gone, not slow. +const WRITE_STALL_MS = 60000; + +/** `writable.write`, but it fails instead of hanging for ever. */ +async function _writeOrStall(writable, bytes, at) { + let timer = 0; + const stalled = new Promise((_, reject) => { + timer = setTimeout(() => reject(new Error( + t('group.download_write_stalled', { seconds: WRITE_STALL_MS / 1000 }) + + ` (chunk ${at})`)), WRITE_STALL_MS); + }); + try { + await Promise.race([writable.write(bytes), stalled]); + } finally { + clearTimeout(timer); + } +} + function _isRetryableTransportError(err) { return err.name === 'TransportLostError' || err.message === 'Response timeout' || (err.message || '').startsWith('DataChannel not open'); } -async function _fetchChunkResilient(transport, fileId, index) { +async function _fetchChunkResilient(transport, fileId, index, tr = '') { let lastErr; for (let attempt = 0; attempt < CHUNK_RETRY_ATTEMPTS; attempt++) { try { - return await transport.fetchChunk(fileId, index); + return await transport.fetchChunk(fileId, index, tr); } catch (err) { if (!_isRetryableTransportError(err)) throw err; lastErr = err; @@ -216,14 +265,14 @@ async function _fetchChunkResilient(transport, fileId, index) { } async function pipelinedDownload(transport, gekKey, fileId, totalChunks, onChunk, - writable, signal) { + writable, signal, tr = '') { const results = writable ? null : new Array(totalChunks); let nextSend = 0, nextRecv = 0; const inflight = new Array(totalChunks); const fire = () => { while (nextSend < totalChunks && nextSend - nextRecv < PIPELINE_WINDOW) { - inflight[nextSend] = _fetchChunkResilient(transport, fileId, nextSend); + inflight[nextSend] = _fetchChunkResilient(transport, fileId, nextSend, tr); nextSend++; } }; @@ -254,7 +303,7 @@ async function pipelinedDownload(transport, gekKey, fileId, totalChunks, onChunk const plaintext = await window.MeshBayCrypto.decryptChunkBin( gekKey, fileId, nextRecv, chunkMsg.nonce, chunkMsg.ct); if (writable) { - await writable.write(plaintext); + await _writeOrStall(writable, plaintext, nextRecv); } else { results[nextRecv] = plaintext; } @@ -272,6 +321,20 @@ async function pipelinedDownload(transport, gekKey, fileId, totalChunks, onChunk * download button — both just want "get this entry to disk". */ async function downloadEntry(transfers, transport, gek, entry) { + // The target FIRST, then the slot — and that order is load-bearing. + // + // Asking for the slot first looks better (the widget could draw a row while + // the target is being chosen) and is wrong: a granted slot has to be taken up + // within the node's acceptance deadline, and opening a target can take thirty + // seconds of streamed-download timeouts, or as long as somebody leaves a Save + // As dialog open. The node then revokes the grant and passes it to the next + // in the queue — `transfer: reclaimed … (not_taken_up)` in its log — and this + // download starts fetching under a `tr` that is no longer granted. + // + // Measured, not reasoned: three downloads started, one arrived, and the + // node's log named the reason. Do not move this again without moving the + // deadline, and the deadline exists so a client that dies between asking and + // starting does not hold a slot nobody can use. let target; try { target = await _openDownloadTarget(entry.name, entry.size); @@ -289,21 +352,25 @@ async function downloadEntry(transfers, transport, gek, entry) { if (target === false) return; // the picker was dismissed const openRef = { url: null }; + const totalChunks = Math.ceil(entry.size / CHUNK_SIZE); transfers.start({ kind: 'download', name: (target && target.name) || entry.name, total: entry.size, transport, + // Asked for here, once there is somewhere to write: see the note above. + lease: transport.openTransfer({ + kind: 'download', bytes: entry.size, chunks: totalChunks }), open: target ? (target.open || null) : () => { if (openRef.url) window.open(openRef.url, '_blank'); }, - run: async ({ signal, onProgress }) => { - const totalChunks = Math.ceil(entry.size / CHUNK_SIZE); + run: async ({ signal, onProgress, lease }) => { let done = 0; const onChunk = (bytes) => { done += bytes; onProgress(done, entry.size); }; if (target) { try { await pipelinedDownload(transport, gek, entry.id, totalChunks, - onChunk, target.writable, signal); + onChunk, target.writable, signal, + lease && lease.tr); await target.writable.close(); } catch (err) { await target.writable.abort().catch(() => {}); @@ -311,7 +378,8 @@ async function downloadEntry(transfers, transport, gek, entry) { } } else { const chunks = await pipelinedDownload( - transport, gek, entry.id, totalChunks, onChunk, null, signal); + transport, gek, entry.id, totalChunks, onChunk, null, signal, + lease && lease.tr); const blob = new Blob(chunks); _saveBlob(blob, entry.name); openRef.url = URL.createObjectURL(blob); @@ -391,10 +459,15 @@ async function downloadDirectory(transfers, transport, gek, entries, dir, { setE transfers.start({ kind: 'download', name: (target && target.name) || suggested, total: totalBytes, transport, + // **One** lease for the archive, not one per file. Dozens of leases for a + // folder would deadlock against the member's own cap: the job cannot finish + // until it holds them all, and it can never hold more than two. + lease: transport.openTransfer({ + kind: 'download', bytes: totalBytes, chunks: files.length }), open: target ? (target.open || null) : () => { if (zipOpenRef.url) window.open(zipOpenRef.url, '_blank'); }, - run: async ({ signal, onProgress }) => { + run: async ({ signal, onProgress, lease }) => { const writable = target ? target.writable : null; const parts = writable ? null : []; let written = 0; @@ -414,7 +487,8 @@ async function downloadDirectory(transfers, transport, gek, entries, dir, { setE transport, gek, entry.id, totalChunks, (bytes) => { written += bytes; onProgress(written, totalBytes); }, // pipelinedDownload writes in order, which the archive needs. - { write: (plaintext) => zip.write(plaintext) }, signal); + { write: (plaintext) => zip.write(plaintext) }, signal, + lease && lease.tr); await zip.end(); } await zip.finish(); diff --git a/packages/meshbay-hub/src/meshbay_hub/static/files-app.js b/packages/meshbay-hub/src/meshbay_hub/static/files-app.js index 36c5af3..fb9d801 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/files-app.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/files-app.js @@ -66,7 +66,15 @@ function FilesPanel({ transport = transportRef.current; gek = gekRef.current; } - if (!transport || !transport.connected) return; + // Never a silent return. A click that produces nothing at all — no + // transfer, no icon, no message — is indistinguishable from a broken + // button, and it is what a download looks like whenever the WebRTC + // connection is not up: on a screen lock, mid-reconnect, or after the node + // restarted. Say so instead. + if (!transport || !transport.connected) { + setError(t('group.download_offline')); + return; + } await downloadEntry(transfers, transport, gek, entry); }, [getTransport]); @@ -86,13 +94,15 @@ function FilesPanel({ for (const file of files) { transfers.start({ kind: 'upload', name: file.name, total: file.size, transport, - run: async ({ signal, onProgress }) => { + lease: transport.openTransfer({ kind: 'upload', bytes: file.size }), + run: async ({ signal, onProgress, lease }) => { await transport.uploadFile(file, { // Bytes the node acknowledged, not bytes read locally. onProgress: (sent) => onProgress(sent, file.size), signal, root: uploadRoot, dir: uploadDir, + tr: lease && lease.tr, }); // The node re-indexes on a filesystem event, so there is nothing to // wait on but the clock. Refreshing here means the file appears in diff --git a/packages/meshbay-hub/src/meshbay_hub/static/group-page.js b/packages/meshbay-hub/src/meshbay_hub/static/group-page.js index 6f41426..c6748f2 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/group-page.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/group-page.js @@ -553,7 +553,12 @@ function GroupPage({ groupId, group, token, username, userId, userPrefs, // toolbar actions call the same shared helper from files-app.js, since // only a single open file/video is ever in play here. const transport = transportRef.current; - if (!transport || !transport.connected) return; + if (!transport || !transport.connected) { + // Same reasoning as files-app.js's downloadFile: a click that does + // nothing at all is worse than a refusal. + setError(t('group.download_offline')); + return; + } await downloadEntry(transfers, transport, gekRef.current, entry); }, []); diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/de.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/de.js index 8ed1ef0..f5c0c78 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/de.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/de.js @@ -162,6 +162,8 @@ export default { 'group.mkdir': 'Neuer Ordner', 'group.mkdir_prompt': 'Name des neuen Ordners', 'group.mkdir_offline': 'Nicht mit dem Node verbunden.', + 'group.download_offline': 'Keine Verbindung zum Node — der Download kann nicht starten. Die Verbindung wird automatisch wiederhergestellt; versuchen Sie es gleich erneut.', + 'group.download_write_stalled': 'Die Datei wird nicht mehr auf die Festplatte geschrieben ({seconds} s ohne Fortschritt). Der Download wurde abgebrochen statt hängen gelassen; versuchen Sie es erneut.', 'device.add_title': 'This browser is not linked to this node yet', 'device.add_hint': 'Your account is known here, but this browser holds a different key. Approve it from a device already linked — no operator needed.', 'device.add_btn': 'Get a linking code', @@ -576,6 +578,16 @@ export default { 'transfers.open': 'Öffnen', 'transfers.done': 'Abgeschlossen', 'transfers.cancelled': 'Abgebrochen', + 'transfers.waiting_own_slots': 'Wartet — Ihre Plätze sind belegt', + 'transfers.waiting_node': 'Wartet — {n} davor', + 'transfers.summary': '{running} laufend · {waiting} wartend', + 'transfers.group_running': 'Laufend', + 'transfers.group_waiting': 'Wartend', + 'transfers.group_finished': 'Abgeschlossen', + 'transfers.cancel_one': '{name} abbrechen', + 'transfers.eta_seconds': 'noch {n} s', + 'transfers.eta_minutes': 'noch {n} Min.', + 'transfers.eta_hours': 'noch {n} Std.', 'transfers.failed': 'Fehlgeschlagen', 'group.select': 'Auswählen', 'group.select_done': 'Fertig', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/en.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/en.js index 285ff7a..e3ed844 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/en.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/en.js @@ -162,6 +162,8 @@ export default { 'group.mkdir': 'New folder', 'group.mkdir_prompt': 'New folder name', 'group.mkdir_offline': 'Not connected to the node.', + 'group.download_offline': 'Not connected to the node — the download cannot start. It reconnects on its own; try again in a moment.', + 'group.download_write_stalled': 'The file stopped being written to disk ({seconds}s with no progress). The download was stopped rather than left hanging; try it again.', 'device.add_title': 'This browser is not linked to this node yet', 'device.add_hint': 'Your account is known here, but this browser holds a different key. Approve it from a device already linked — no operator needed.', 'device.add_btn': 'Get a linking code', @@ -692,6 +694,16 @@ export default { 'transfers.open': 'Open', 'transfers.done': 'Finished', 'transfers.cancelled': 'Cancelled', + 'transfers.waiting_own_slots': 'Waiting — your slots are busy', + 'transfers.waiting_node': 'Waiting — {n} ahead', + 'transfers.summary': '{running} running · {waiting} waiting', + 'transfers.group_running': 'Running', + 'transfers.group_waiting': 'Waiting', + 'transfers.group_finished': 'Finished', + 'transfers.cancel_one': 'Cancel {name}', + 'transfers.eta_seconds': '{n}s left', + 'transfers.eta_minutes': '{n} min left', + 'transfers.eta_hours': '{n} h left', 'transfers.failed': 'Failed', 'group.select': 'Select', 'group.select_done': 'Done', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/es.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/es.js index f74c763..c509599 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/es.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/es.js @@ -160,6 +160,8 @@ export default { 'group.mkdir': 'Nueva carpeta', 'group.mkdir_prompt': 'Nombre de la nueva carpeta', 'group.mkdir_offline': 'Sin conexión con el nodo.', + 'group.download_offline': 'Sin conexión con el nodo — la descarga no puede empezar. Se reconecta sola; inténtelo de nuevo en un momento.', + 'group.download_write_stalled': 'El archivo dejó de escribirse en el disco ({seconds} s sin avance). La descarga se detuvo en lugar de quedarse colgada; inténtelo de nuevo.', 'device.add_title': 'This browser is not linked to this node yet', 'device.add_hint': 'Your account is known here, but this browser holds a different key. Approve it from a device already linked — no operator needed.', 'device.add_btn': 'Get a linking code', @@ -572,6 +574,16 @@ export default { 'transfers.open': 'Abrir', 'transfers.done': 'Terminada', 'transfers.cancelled': 'Cancelada', + 'transfers.waiting_own_slots': 'En espera — sus espacios están ocupados', + 'transfers.waiting_node': 'En espera — {n} por delante', + 'transfers.summary': '{running} en curso · {waiting} en espera', + 'transfers.group_running': 'En curso', + 'transfers.group_waiting': 'En espera', + 'transfers.group_finished': 'Finalizados', + 'transfers.cancel_one': 'Cancelar {name}', + 'transfers.eta_seconds': 'quedan {n} s', + 'transfers.eta_minutes': 'quedan {n} min', + 'transfers.eta_hours': 'quedan {n} h', 'transfers.failed': 'Fallida', 'group.select': 'Seleccionar', 'group.select_done': 'Listo', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/fr.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/fr.js index 0c5103f..49a1071 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/fr.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/fr.js @@ -161,6 +161,8 @@ export default { 'group.mkdir': 'Nouveau dossier', 'group.mkdir_prompt': 'Nom du nouveau dossier', 'group.mkdir_offline': 'Non connecté au nœud.', + 'group.download_offline': 'Pas de connexion au node — le téléchargement ne peut pas démarrer. La reconnexion est automatique, réessayez dans un instant.', + 'group.download_write_stalled': 'L\'écriture du fichier sur le disque s\'est arrêtée ({seconds} s sans progression). Le téléchargement a été interrompu plutôt que laissé en suspens ; réessayez.', 'device.add_title': 'Ce navigateur n’est pas encore lié à ce nœud', 'device.add_hint': 'Votre compte est connu ici, mais ce navigateur détient une autre clé. Approuvez-le depuis un appareil déjà lié — sans passer par l’opérateur.', 'device.add_btn': 'Obtenir un code de liaison', @@ -575,6 +577,16 @@ export default { 'transfers.open': 'Ouvrir', 'transfers.done': 'Terminé', 'transfers.cancelled': 'Annulé', + 'transfers.waiting_own_slots': 'En attente — vos slots sont occupés', + 'transfers.waiting_node': 'En attente — {n} devant', + 'transfers.summary': '{running} en cours · {waiting} en attente', + 'transfers.group_running': 'En cours', + 'transfers.group_waiting': 'En attente', + 'transfers.group_finished': 'Terminés', + 'transfers.cancel_one': 'Annuler {name}', + 'transfers.eta_seconds': '{n} s restantes', + 'transfers.eta_minutes': '{n} min restantes', + 'transfers.eta_hours': '{n} h restantes', 'transfers.failed': 'Échec', 'group.select': 'Sélectionner', 'group.select_done': 'Terminé', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/it.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/it.js index ebac541..8f1ef1e 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/it.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/it.js @@ -161,6 +161,8 @@ export default { 'group.mkdir': 'Nuova cartella', 'group.mkdir_prompt': 'Nome della nuova cartella', 'group.mkdir_offline': 'Non connesso al nodo.', + 'group.download_offline': 'Nessuna connessione al nodo — il download non può iniziare. La riconnessione è automatica, riprovi tra poco.', + 'group.download_write_stalled': 'Il file ha smesso di essere scritto su disco ({seconds} s senza progressi). Il download è stato interrotto invece di restare bloccato; riprovi.', 'device.add_title': 'This browser is not linked to this node yet', 'device.add_hint': 'Your account is known here, but this browser holds a different key. Approve it from a device already linked — no operator needed.', 'device.add_btn': 'Get a linking code', @@ -575,6 +577,16 @@ export default { 'transfers.open': 'Apri', 'transfers.done': 'Completato', 'transfers.cancelled': 'Annullato', + 'transfers.waiting_own_slots': 'In attesa — i suoi posti sono occupati', + 'transfers.waiting_node': 'In attesa — {n} prima', + 'transfers.summary': '{running} in corso · {waiting} in attesa', + 'transfers.group_running': 'In corso', + 'transfers.group_waiting': 'In attesa', + 'transfers.group_finished': 'Completati', + 'transfers.cancel_one': 'Annulla {name}', + 'transfers.eta_seconds': '{n} s rimanenti', + 'transfers.eta_minutes': '{n} min rimanenti', + 'transfers.eta_hours': '{n} h rimanenti', 'transfers.failed': 'Non riuscito', 'group.select': 'Seleziona', 'group.select_done': 'Fine', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/ja.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/ja.js index 1b49ddb..7ea9789 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/ja.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/ja.js @@ -159,6 +159,8 @@ export default { 'group.mkdir': '新しいフォルダー', 'group.mkdir_prompt': '新しいフォルダー名', 'group.mkdir_offline': 'ノードに接続していません。', + 'group.download_offline': 'ノードに接続していません — ダウンロードを開始できません。再接続は自動で行われます。少し待って再試行してください。', + 'group.download_write_stalled': 'ファイルのディスクへの書き込みが止まりました（{seconds} 秒間進みません）。ぶら下がったままにせず中止しました。もう一度お試しください。', 'device.add_title': 'This browser is not linked to this node yet', 'device.add_hint': 'Your account is known here, but this browser holds a different key. Approve it from a device already linked — no operator needed.', 'device.add_btn': 'Get a linking code', @@ -567,6 +569,16 @@ export default { 'transfers.open': '開く', 'transfers.done': '完了', 'transfers.cancelled': 'キャンセル済み', + 'transfers.waiting_own_slots': '待機中 — 自分の枠がすべて使用中です', + 'transfers.waiting_node': '待機中 — 前に {n} 件', + 'transfers.summary': '実行中 {running} · 待機中 {waiting}', + 'transfers.group_running': '実行中', + 'transfers.group_waiting': '待機中', + 'transfers.group_finished': '完了', + 'transfers.cancel_one': '{name} をキャンセル', + 'transfers.eta_seconds': '残り {n} 秒', + 'transfers.eta_minutes': '残り {n} 分', + 'transfers.eta_hours': '残り {n} 時間', 'transfers.failed': '失敗', 'group.select': '選択', 'group.select_done': '完了', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/nl.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/nl.js index 7cf4cd4..74cd269 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/nl.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/nl.js @@ -162,6 +162,8 @@ export default { 'group.mkdir': 'Nieuwe map', 'group.mkdir_prompt': 'Naam van de nieuwe map', 'group.mkdir_offline': 'Niet verbonden met de node.', + 'group.download_offline': 'Geen verbinding met de node — de download kan niet starten. Er wordt automatisch opnieuw verbonden; probeer het zo weer.', + 'group.download_write_stalled': 'Het bestand wordt niet meer naar schijf geschreven ({seconds} s zonder voortgang). De download is gestopt in plaats van te blijven hangen; probeer het opnieuw.', 'device.add_title': 'This browser is not linked to this node yet', 'device.add_hint': 'Your account is known here, but this browser holds a different key. Approve it from a device already linked — no operator needed.', 'device.add_btn': 'Get a linking code', @@ -576,6 +578,16 @@ export default { 'transfers.open': 'Openen', 'transfers.done': 'Voltooid', 'transfers.cancelled': 'Geannuleerd', + 'transfers.waiting_own_slots': 'Wacht — uw plaatsen zijn bezet', + 'transfers.waiting_node': 'Wacht — {n} ervoor', + 'transfers.summary': '{running} bezig · {waiting} wachtend', + 'transfers.group_running': 'Bezig', + 'transfers.group_waiting': 'Wachtend', + 'transfers.group_finished': 'Voltooid', + 'transfers.cancel_one': '{name} annuleren', + 'transfers.eta_seconds': 'nog {n} s', + 'transfers.eta_minutes': 'nog {n} min', + 'transfers.eta_hours': 'nog {n} u', 'transfers.failed': 'Mislukt', 'group.select': 'Selecteren', 'group.select_done': 'Klaar', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/pl.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/pl.js index 6edffd5..584e5f2 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/pl.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/pl.js @@ -165,6 +165,8 @@ export default { 'group.mkdir': 'Nowy folder', 'group.mkdir_prompt': 'Nazwa nowego folderu', 'group.mkdir_offline': 'Brak połączenia z węzłem.', + 'group.download_offline': 'Brak połączenia z węzłem — pobieranie nie może się rozpocząć. Połączenie wróci samo; proszę spróbować za chwilę.', + 'group.download_write_stalled': 'Plik przestał być zapisywany na dysk ({seconds} s bez postępu). Pobieranie zostało przerwane, zamiast wisieć w nieskończoność; proszę spróbować ponownie.', 'device.add_title': 'This browser is not linked to this node yet', 'device.add_hint': 'Your account is known here, but this browser holds a different key. Approve it from a device already linked — no operator needed.', 'device.add_btn': 'Get a linking code', @@ -588,6 +590,16 @@ export default { 'transfers.open': 'Otwórz', 'transfers.done': 'Zakończony', 'transfers.cancelled': 'Anulowany', + 'transfers.waiting_own_slots': 'Oczekiwanie — Twoje miejsca są zajęte', + 'transfers.waiting_node': 'Oczekiwanie — {n} przed', + 'transfers.summary': '{running} w toku · {waiting} oczekuje', + 'transfers.group_running': 'W toku', + 'transfers.group_waiting': 'Oczekuje', + 'transfers.group_finished': 'Zakończone', + 'transfers.cancel_one': 'Anuluj {name}', + 'transfers.eta_seconds': 'pozostało {n} s', + 'transfers.eta_minutes': 'pozostało {n} min', + 'transfers.eta_hours': 'pozostało {n} godz.', 'transfers.failed': 'Nie powiódł się', 'group.select': 'Zaznacz', 'group.select_done': 'Gotowe', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/pt-BR.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/pt-BR.js index 3a6fa22..b034117 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/pt-BR.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/pt-BR.js @@ -162,6 +162,8 @@ export default { 'group.mkdir': 'Nova pasta', 'group.mkdir_prompt': 'Nome da nova pasta', 'group.mkdir_offline': 'Sem conexão com o nó.', + 'group.download_offline': 'Sem conexão com o nó — o download não pode começar. Ele reconecta sozinho; tente de novo em instantes.', + 'group.download_write_stalled': 'O arquivo parou de ser gravado no disco ({seconds}s sem progresso). O download foi interrompido em vez de ficar travado; tente de novo.', 'device.add_title': 'This browser is not linked to this node yet', 'device.add_hint': 'Your account is known here, but this browser holds a different key. Approve it from a device already linked — no operator needed.', 'device.add_btn': 'Get a linking code', @@ -574,6 +576,16 @@ export default { 'transfers.open': 'Abrir', 'transfers.done': 'Concluída', 'transfers.cancelled': 'Cancelada', + 'transfers.waiting_own_slots': 'Aguardando — seus espaços estão ocupados', + 'transfers.waiting_node': 'Aguardando — {n} na frente', + 'transfers.summary': '{running} em andamento · {waiting} aguardando', + 'transfers.group_running': 'Em andamento', + 'transfers.group_waiting': 'Aguardando', + 'transfers.group_finished': 'Concluídos', + 'transfers.cancel_one': 'Cancelar {name}', + 'transfers.eta_seconds': 'faltam {n} s', + 'transfers.eta_minutes': 'faltam {n} min', + 'transfers.eta_hours': 'faltam {n} h', 'transfers.failed': 'Falhou', 'group.select': 'Selecionar', 'group.select_done': 'Concluir', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/zh-CN.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/zh-CN.js index f43ef72..ffe2cb0 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/zh-CN.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/zh-CN.js @@ -158,6 +158,8 @@ export default { 'group.mkdir': '新建文件夹', 'group.mkdir_prompt': '新文件夹名称', 'group.mkdir_offline': '未连接到节点。', + 'group.download_offline': '未连接到节点 — 无法开始下载。连接会自动恢复，请稍后重试。', + 'group.download_write_stalled': '文件停止写入磁盘（{seconds} 秒无进展）。已中止下载而不是让它一直卡住，请重试。', 'device.add_title': 'This browser is not linked to this node yet', 'device.add_hint': 'Your account is known here, but this browser holds a different key. Approve it from a device already linked — no operator needed.', 'device.add_btn': 'Get a linking code', @@ -555,6 +557,16 @@ export default { 'transfers.open': '打开', 'transfers.done': '已完成', 'transfers.cancelled': '已取消', + 'transfers.waiting_own_slots': '等待中 — 您的通道已占满', + 'transfers.waiting_node': '等待中 — 前面还有 {n} 个', + 'transfers.summary': '进行中 {running} · 等待中 {waiting}', + 'transfers.group_running': '进行中', + 'transfers.group_waiting': '等待中', + 'transfers.group_finished': '已完成', + 'transfers.cancel_one': '取消 {name}', + 'transfers.eta_seconds': '剩余 {n} 秒', + 'transfers.eta_minutes': '剩余 {n} 分钟', + 'transfers.eta_hours': '剩余 {n} 小时', 'transfers.failed': '失败', 'group.select': '选择', 'group.select_done': '完成', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/style.css b/packages/meshbay-hub/src/meshbay_hub/static/style.css index dfed44b..22fcd3f 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/style.css +++ b/packages/meshbay-hub/src/meshbay_hub/static/style.css @@ -4243,3 +4243,67 @@ h2 .gn-owner, h3 .gn-owner { font-size: 0.55em; } color: var(--warn); margin-bottom: 2px; } + +/* A transfer waiting for a slot. Deliberately not a progress bar at 0%: it is + not stalled and nothing is wrong, and a bar that never moves is exactly how a + queue comes to look like a hang. The stripes say "not yet", not "broken". */ +.dl-progress.dl-waiting { + background-color: var(--bg-raised); + background-image: repeating-linear-gradient( + 45deg, + var(--accent-bg) 0 8px, + transparent 8px 16px); + animation: dl-waiting-slide 1.1s linear infinite; +} +@keyframes dl-waiting-slide { + from { background-position: 0 0; } + to { background-position: 22.6px 0; } +} +/* The state has to survive without the motion: someone who asked for less of it + still needs to see that this row is waiting rather than stopped. */ +@media (prefers-reduced-motion: reduce) { + .dl-progress.dl-waiting { animation: none; } +} + +/* ── Transfers panel (grouped) ───────────────────────────────────────────── */ + +.transfer-head-title { font-weight: 600; } +.transfer-head-summary { + margin-left: auto; + margin-right: .5rem; + font-size: .85em; + color: var(--text-dim); + /* Never wraps to a second line: it is the one part of the header that grows + with what is happening, and a header that changes height as transfers come + and go pushes every row under it. */ + white-space: nowrap; +} +.transfer-group + .transfer-group { border-top: 1px solid var(--border); } +.transfer-group-head { + padding: .35rem .6rem .2rem; + font-size: .78em; + text-transform: uppercase; + letter-spacing: .04em; + color: var(--text-dim); +} +/* Finished rows recede rather than disappear: somebody who just downloaded + three files wants to see that all three are there. */ +.transfer-item.transfer-done .transfer-name, +.transfer-item.transfer-cancelled .transfer-name { color: var(--text-dim); } + +/* Announced, not shown. The transfers panel needs a live region that says what + changed state without drawing anything — used with aria-live, so it must + stay in the accessibility tree: `display: none` would remove it from there + too and announce nothing at all, which is the usual way this is got wrong. */ +.sr-only { + position: absolute; + width: 1px; + height: 1px; + margin: -1px; + padding: 0; + overflow: hidden; + clip: rect(0 0 0 0); + clip-path: inset(50%); + white-space: nowrap; + border: 0; +} diff --git a/packages/meshbay-hub/src/meshbay_hub/static/transfers.js b/packages/meshbay-hub/src/meshbay_hub/static/transfers.js index fa34c67..46f75a2 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/transfers.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/transfers.js @@ -22,6 +22,12 @@ const SPEED_WINDOW_MS = 5000; +function _abortError() { + const err = new Error('Cancelled'); + err.name = 'AbortError'; + return err; +} + let _nextId = 1; export class TransferStore { @@ -53,8 +59,18 @@ export class TransferStore { total: it.total, done: it.done, status: it.status, + // How many are in front of this one, and whose limit is holding it up: + // "your own two slots are busy" and "the node is full" are different + // situations and the person can act on only one of them. + ahead: it.ahead || 0, + queuedByOwnLimit: Boolean( + it.lease && it.lease.cap && it.lease.used >= it.lease.cap), error: it.error || '', speed: this._speed(it), + // The ETA is drawn only once the window holds a few seconds of real + // measurement -- see etaSeconds. + settled: it.samples.length > 2 + && (it.samples[it.samples.length - 1].t - it.samples[0].t) >= 3000, percent: it.total ? Math.min(100, Math.round(it.done / it.total * 100)) : 0, // Only for a file written into a folder the browser granted us: that is // the one case where the page can read its own download back. @@ -66,6 +82,12 @@ export class TransferStore { return this._items.filter(it => it.status === 'running').length; } + /** Running or waiting for a slot — what the nav badge counts. */ + get pending() { + return this._items.filter( + it => it.status === 'running' || it.status === 'queued').length; + } + _speed(it) { // Over a window rather than since the start: a transfer that stalls should // read as slow immediately, not as its own historical average. @@ -82,12 +104,18 @@ export class TransferStore { * `run` receives `{ signal, onProgress }`. It must poll `signal.aborted` — a * cancel that only sets a flag nobody reads is a button that lies. */ - start({ kind, name, total = 0, transport = null, run, open = null }) { + start({ kind, name, total = 0, transport = null, run, open = null, + lease = null }) { const item = { id: _nextId++, - kind, name, total, transport, open, + kind, name, total, transport, open, lease, done: 0, - status: 'running', + // A transfer that has to wait for a slot starts as 'queued', not + // 'running'. Two different things are true of it — nothing is moving, and + // nothing is wrong — and a status that conflates them is what makes a + // queue look like a hang. + status: lease && lease.state !== 'granted' ? 'queued' : 'running', + ahead: (lease && lease.ahead) || 0, error: '', samples: [{ t: this._now(), done: 0 }], signal: { aborted: false }, @@ -113,8 +141,39 @@ export class TransferStore { this._maybeRelease(item.transport); }; + // The slot is given back in a `finally` around everything, so it survives + // a throw, a cancel and a return alike. A slot not returned is a member who + // cannot start another transfer until the node times it out. + const finished = () => { + if (item.lease) item.lease.release( + item.signal.aborted ? 'cancelled' : 'done'); + }; + + // Installed here and not inside the promise chain below. A state push that + // arrived before the first microtask ran was simply dropped, so a transfer + // could sit at the position it was given when it was created and never + // appear to move — the widget showing "3 ahead" for ever while the node + // quietly worked through the queue. Nothing about that looks wrong from + // either side, which is why it needs a test rather than a reading. + if (item.lease) { + item.lease._onState = (lease) => { + if (item.status !== 'queued' && item.status !== 'running') return; + item.ahead = lease.ahead; + item.status = lease.state === 'granted' ? 'running' : 'queued'; + this._emit(); + }; + } + const promise = Promise.resolve() - .then(() => run({ signal: item.signal, onProgress })) + .then(async () => { + if (item.lease) { + await item.lease.acquire(); + if (item.signal.aborted) throw _abortError(); + item.status = 'running'; + this._emit(); + } + return run({ signal: item.signal, onProgress, lease: item.lease }); + }) .then(() => { if (item.signal.aborted) finish('cancelled'); else { @@ -125,7 +184,8 @@ export class TransferStore { .catch(err => { if (item.signal.aborted || err.name === 'AbortError') finish('cancelled'); else finish('failed', err.message || String(err)); - }); + }) + .finally(finished); item.promise = promise; return item.id; @@ -146,8 +206,12 @@ export class TransferStore { cancel(id) { const item = this._items.find(it => it.id === id); - if (!item || item.status !== 'running') return; + // 'queued' too: a transfer waiting for a slot is exactly the one somebody + // is most likely to give up on, and its queue entry has to go with it or + // the node grants a slot to a transfer that will never use it. + if (!item || (item.status !== 'running' && item.status !== 'queued')) return; item.signal.aborted = true; + if (item.lease) item.lease.release('cancelled'); // Marked at once. The work stops when it next looks, but a cancelled // transfer should not keep reporting progress in the meantime. item.status = 'cancelled'; @@ -157,19 +221,24 @@ export class TransferStore { cancelAll() { for (const it of this._items) { - if (it.status === 'running') this.cancel(it.id); + if (it.status === 'running' || it.status === 'queued') this.cancel(it.id); } } - /** Drop everything finished, keeping what is still running. */ + /** Drop everything finished, keeping what is still running or waiting. */ clearFinished() { - this._items = this._items.filter(it => it.status === 'running'); + this._items = this._items.filter( + it => it.status === 'running' || it.status === 'queued'); this._emit(); } _busy(transport) { + // Queued counts as busy: a transport closed while a transfer waits for a + // slot can never be granted one, and the transfer would sit at "waiting" + // for ever with nothing left to answer it. return this._items.some( - it => it.transport === transport && it.status === 'running'); + it => it.transport === transport + && (it.status === 'running' || it.status === 'queued')); } /** @@ -211,6 +280,22 @@ export class TransferStore { export const transfers = new TransferStore(); +/** + * Seconds left, or null when saying nothing is the honest answer. + * + * Withheld until the speed window has real samples in it: a figure computed + * from the first two chunks of a transfer swings between "4 seconds" and "an + * hour" and back, and a number that behaves like that is worse than a blank — + * people read the first one they see and plan around it. + */ +export function etaSeconds(item) { + if (item.status !== 'running' || !item.total || !item.speed) return null; + const left = item.total - item.done; + if (left <= 0) return null; + const secs = left / item.speed; + return Number.isFinite(secs) ? secs : null; +} + /** Human-readable rate, for a widget that updates several times a second. */ export function formatSpeed(bytesPerSecond) { if (!bytesPerSecond || bytesPerSecond < 1) return ''; diff --git a/packages/meshbay-hub/src/meshbay_hub/static/transport.js b/packages/meshbay-hub/src/meshbay_hub/static/transport.js index c4a24c5..54a1302 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/transport.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/transport.js @@ -283,6 +283,129 @@ const JOIN_REFUSALS = { group_mismatch: 'The node refused a request naming a different group.', }; +/** + * One transfer's slot on the node, from this side. + * + * The contract the transfer store depends on: `acquire()` resolves when the + * node has granted the slot (immediately on a node that hands out none), and + * `release(reason)` gives it back exactly once. Nothing else in the client + * speaks to the node about slots. + * + * Two things here exist only because a queue can lie, and both are the + * difference between "waiting" and "waiting for ever": + * + * - **the watchdog.** A grant is pushed, not polled, so a lost push leaves + * this side waiting on a node that believes it has started. Re-asking is + * free — the node is idempotent on `tr` — and it is the only thing that + * recovers a message that did not arrive. + * - **release is idempotent and unconditional.** A slot given back twice + * costs nothing; one never given back is a member who cannot transfer + * again until a timeout the node runs on its own. + */ +const LEASE_WATCHDOG_MS = 60000; + +class Lease { + constructor(transport, tr, kind, bytes, chunks, onState) { + this.transport = transport; + this.tr = tr; + this.kind = kind; + this.bytes = bytes; + this.chunks = chunks; + this.state = 'opening'; + this.ahead = 0; + this.closed = false; + this._onState = onState; + this._granted = null; + this._watchdog = 0; + this._wait = new Promise((resolve) => { this._granted = resolve; }); + } + + /** No slots on this node: behave as though one was granted at once. */ + _skip() { + this.state = 'granted'; + this._granted(); + } + + _request() { + // A closed channel is not a failure here, and must not throw: the transport + // reconnects on its own, `_reopenTransfers` re-asks for every live lease + // when it does, and the watchdog below asks again meanwhile. + // + // This is the same tolerance `_fetchChunkResilient` already gives a chunk + // request — and before leases existed, a chunk request was the first thing + // to touch the channel, so a download started on a briefly dead connection + // simply retried. Asking for a slot first made `_send` the first contact + // and threw "DataChannel not open (state: closed)" out of `downloadEntry`, + // where nothing catches it: a download that used to recover became an + // error with no row in the widget to show it. Found by downloading a file + // right after a connection dropped. + try { + this.transport._send({ + type: 'transfer_open', v: '0.1', tr: this.tr, kind: this.kind, + bytes: this.bytes, chunks: this.chunks, + }); + } catch (err) { + console.warn('[MeshBay] could not ask for a slot yet:', err.message); + } + this._arm(); + } + + _arm() { + clearTimeout(this._watchdog); + if (this.closed || this.state === 'granted') return; + this._watchdog = setTimeout(() => { + if (this.closed || this.state === 'granted') return; + console.warn('[MeshBay] no answer for transfer', this.tr.slice(0, 8), + '- asking again'); + this._request(); + }, LEASE_WATCHDOG_MS); + } + + _apply(msg) { + if (this.closed) return; + this.state = msg.state; + this.ahead = msg.ahead || 0; + this.used = msg.used; + this.cap = msg.cap; + if (msg.state === 'granted') { + clearTimeout(this._watchdog); + this._granted(); + } else if (msg.state === 'closed') { + // The node ended it: reclaimed as idle, or revoked. Not an error here — + // whoever is running the transfer finds out through its own failure — but + // the slot is gone and asking again is the only way back. + clearTimeout(this._watchdog); + } else { + this._arm(); + } + if (this._onState) { + try { this._onState(this); } catch (e) { + console.error('[MeshBay] lease state handler threw:', e); + } + } + } + + /** Resolves once the node has granted the slot. */ + acquire() { return this._wait; } + + /** + * Give the slot back. Safe to call twice, and safe on a dead transport: a + * lease that is not released is a member who cannot start another transfer + * until the node times it out, so this must never be conditional on anything. + */ + release(reason = 'done') { + if (this.closed) return; + this.closed = true; + clearTimeout(this._watchdog); + this.transport._leases.delete(this.tr); + if (!this.transport.supportsTransferSlots) return; + try { + this.transport._send({ type: 'transfer_close', v: '0.1', tr: this.tr, + reason }); + } catch { /* the connection is gone, and so is the lease with it */ } + } +} + class MeshBayTransport { constructor(hubUrl, accessToken) { this._hubUrl = hubUrl; @@ -315,6 +438,13 @@ class MeshBayTransport { // Names, not ids: the "already being uploaded" guard is about the file the // caller passed, and two `uploadFile` calls for one file draw two ids. this._inFlightUploads = new Set(); + // tr → Lease. A transfer's slot on the node, from the client's side. + this._leases = new Map(); + // Set from the handshake ack: a node that answers with `transfer_limits` + // speaks transfer slots. Used instead of a timeout, because "no answer + // yet" and "this node will never answer" are indistinguishable in time and + // guessing wrong either stalls every download or defeats the cap. + this._transferLimits = null; // Set once close() runs — stops the automatic reconnect from firing on a // connection the caller tore down on purpose (leaving the group, page // unload), which would otherwise race back in right as everything else @@ -376,6 +506,20 @@ class MeshBayTransport { /** The MNP version the connected node declared, or '' before a handshake. */ get nodeVersion() { return this._nodeVersion || ''; } + /** + * Whether this node hands out transfer slots. + * + * Read from the handshake ack rather than from the MNP version: the caps + * shipped before the version bump that will make leases compulsory, so for + * now a node either answers with `transfer_limits` or it predates all of + * this. A node that does not is asked for nothing and enforces nothing — + * every download behaves exactly as it did. + */ + get supportsTransferSlots() { return this._transferLimits !== null; } + + /** This member's own caps in this group, or null when the node said nothing. */ + get transferLimits() { return this._transferLimits; } + /** * Whether the node speaks the per-root and per-app operations MNP 1.1 added: * `root_update`/`root_eject`/`root_plug`, `app_directories`, @@ -880,6 +1024,7 @@ class MeshBayTransport { delete ack.nonce; delete ack.ct; Object.assign(ack, config); + this._transferLimits = ack.transfer_limits || null; // Tell the node which of this account's devices is on this connection. // Deliberately after the ack, and gated on the node's own version rather @@ -1016,6 +1161,10 @@ class MeshBayTransport { } trace('reconnect_ok', { attempt: this._reconnectAttempts }); console.log('[MeshBay] Reconnected after', this._reconnectAttempts, 'attempt(s)'); + // Before the caller's own hook: a transfer that resumes mid-chunk must + // have asked for its slot back first, or its next `file_req` carries a + // `tr` the node has never heard of. + this._reopenTransfers(); if (this._onReconnected) { try { this._onReconnected(); } catch (e) { console.error('[MeshBay] onReconnected handler threw:', e); @@ -1100,12 +1249,52 @@ class MeshBayTransport { return msg; } - async fetchChunk(fileId, chunkIndex) { + // ── Transfer slots ───────────────────────────────────────────────────────── + + /** + * Ask the node for a slot, and wait until it says yes. + * + * `tr` is drawn here, not by the node — 16 random bytes, exactly like + * `upload_id` — which is what makes re-opening after a reconnect idempotent + * rather than a second charge against the member's cap. + * + * On a node that predates transfer slots this resolves at once and costs + * nothing: there is no cap to respect and no message that would be + * understood. + */ + openTransfer({ kind = 'download', bytes = 0, chunks = 0, onState = null } = {}) { + const tr = _hex(crypto.getRandomValues(new Uint8Array(16))); + const lease = new Lease(this, tr, kind, bytes, chunks, onState); + if (!this.supportsTransferSlots) { + lease._skip(); + return lease; + } + this._leases.set(tr, lease); + lease._request(); + return lease; + } + + /** Re-ask for every live lease. Called after a reconnect. */ + _reopenTransfers() { + if (!this.supportsTransferSlots) return; + for (const lease of this._leases.values()) { + // The node lost the lease with the session, so this is a fresh request + // for the same `tr` — which the node treats as the same transfer rather + // than a second one. + if (!lease.closed) lease._request(); + } + } + + async fetchChunk(fileId, chunkIndex, tr = '') { const msg = await this._sendAndWait({ type: 'file_req', v: '0.1', file_id: fileId, chunk_index: chunkIndex, + // Present only when this download holds a slot. The node does not require + // it yet; carrying it is what lets the node see the transfer is alive and + // not reclaim its slot as idle. + ...(tr ? { tr } : {}), }); if (msg.type === 'error') throw new Error(msg.detail); return msg; @@ -2189,7 +2378,8 @@ class MeshBayTransport { * folder on screen to name. Omitting both leaves the node to pick, which it * only does for a client old enough to have had one destination. */ - async uploadFile(file, { chunkSize, onProgress, signal, root, dir } = {}) { + async uploadFile(file, { chunkSize, onProgress, signal, root, dir, + tr = '' } = {}) { // The same file twice at once would confuse the node, which keys its own // upload state by name — and would race for the same destination. The guard // is by name for that reason, even though the map below is keyed by id. @@ -2273,6 +2463,7 @@ class MeshBayTransport { upload_id: uploadId, chunk_index: i, total_chunks: total, + ...(tr ? { tr } : {}), ...sealed, }); } @@ -2992,6 +3183,24 @@ class MeshBayTransport { // "arrived" — and every message after that is one slot off too. Found // live: a group mid-scan corrupted its own handshake and chat history // this way, arriving roughly every 2s for as long as scanning ran. + // Routed by `tr`, and only by `tr`. A grant arrives unsolicited, minutes + // after the request that produced it, so falling through to "the oldest + // pending request" would hand a chat send or a handshake somebody else's + // slot — the class of defect `req_id` was introduced for. + if (msg.type === 'transfer_state') { + const lease = this._leases.get(msg.tr); + if (lease) lease._apply(msg); + else if (msg.state === 'granted') { + // A grant for a transfer this page has forgotten (a reload, a cancel + // that raced the grant). Handing it back at once matters: otherwise the + // node holds it until the 30 s acceptance deadline, and everyone behind + // it waits for nothing. + this._send({ type: 'transfer_close', v: '0.1', tr: msg.tr, + reason: 'cancelled' }); + } + return; + } + if (msg.type === 'index_progress') { if (this._onIndexProgress) { this._onIndexProgress({ diff --git a/packages/meshbay-hub/tests/test_layout_measured.py b/packages/meshbay-hub/tests/test_layout_measured.py index 91f1ed0..a71b6b9 100644 --- a/packages/meshbay-hub/tests/test_layout_measured.py +++ b/packages/meshbay-hub/tests/test_layout_measured.py @@ -54,7 +54,7 @@ NAV = textwrap.dedent("""
↓ - S03E01. Salt and Sea, Fire and Blood.mp4 + Some Saga S03E01 - A Long Enough Title.mp4
@@ -144,3 +144,120 @@ def test_the_page_does_not_scroll_sideways(measured, width): r = measured[str(width)] assert r["docScrollW"] <= r["viewport"]["w"], ( f"the document scrolls to {r['docScrollW']} px on a {width} px screen") + + +# ── The grouped panel (§8.2) ──────────────────────────────────────────────── +# +# The panel gained groups, a header summary and a waiting row. Every one of +# those can push something off a 320 px screen, and none of it can be seen by +# reading the stylesheet: what decides where the panel lands is the button it +# hangs from, which is not at the right edge. That is the defect this file was +# written for, and it comes back with any change to the header's width. + +GROUPED = textwrap.dedent(""" + +""") + +GROUPED_SELECTORS = [".transfer-panel", ".transfer-head", ".transfer-head-summary", + ".transfer-group-head", ".transfer-name", + ".transfer-item.transfer-queued .dl-progress"] + + +@pytest.fixture(scope="module") +def grouped(tmp_path_factory): + fragment = tmp_path_factory.mktemp("grouped") / "fragment.html" + fragment.write_text(GROUPED) + proc = subprocess.run( + ["python3", str(HARNESS), ",".join(str(w) for w in WIDTHS), + str(fragment), *GROUPED_SELECTORS], + capture_output=True, text=True, timeout=180) + assert proc.returncode == 0, f"probe failed: {proc.stdout}{proc.stderr}" + out = json.loads(proc.stdout) + assert "error" not in out, f"no measurement: {out}" + return out + + +def test_the_grouped_panel_stays_on_a_phone_screen(grouped): + for width in WIDTHS: + box = grouped[str(width)]["boxes"][".transfer-panel"] + assert box["offLeft"] == 0, ( + f"at {width} px the panel hangs {box['offLeft']} px off the left — " + "which is where the file names are") + assert box["offRight"] == 0, ( + f"at {width} px the panel hangs {box['offRight']} px off the right") + + +def test_the_file_name_is_on_screen_in_every_group(grouped): + for width in WIDTHS: + box = grouped[str(width)]["boxes"][".transfer-name"] + assert box["offLeft"] == 0 and box["offRight"] == 0, ( + f"at {width} px a file name is cut off: {box}") + assert box["width"] > 40, "the name column collapsed to nothing" + + +def test_the_header_summary_does_not_push_the_header_taller(grouped): + """It is the one part of the header that grows with what is happening. If + it wraps, the header changes height as transfers come and go and every row + below it moves — on the narrowest screen, repeatedly.""" + for width in WIDTHS: + head = grouped[str(width)]["boxes"][".transfer-head"] + summary = grouped[str(width)]["boxes"][".transfer-head-summary"] + assert head["height"] <= 48, ( + f"at {width} px the header is {head['height']} px tall — it wrapped") + assert summary["height"] <= 24, ( + f"at {width} px the summary wrapped to {summary['height']} px") + + +def test_the_waiting_bar_is_as_wide_as_a_progress_bar(grouped): + """A waiting row has no inner fill element — the stripes are on the track + itself. Getting that wrong renders a zero-width bar, which reads as a + transfer stuck at 0% rather than one that has not started.""" + for width in WIDTHS: + bar = grouped[str(width)]["boxes"][ + ".transfer-item.transfer-queued .dl-progress"] + assert bar["width"] > 100, ( + f"at {width} px the waiting bar is {bar['width']} px wide") + assert bar["height"] >= 3, "the waiting bar has no height" diff --git a/packages/meshbay-hub/tests/test_memory_ceiling.py b/packages/meshbay-hub/tests/test_memory_ceiling.py index 9966e48..8430627 100644 --- a/packages/meshbay-hub/tests/test_memory_ceiling.py +++ b/packages/meshbay-hub/tests/test_memory_ceiling.py @@ -92,9 +92,15 @@ const downloads = {{ }}; globalThis.window = {{}}; if ({json.dumps(picker)}) {{ - window.showSaveFilePicker = async () => ({{ - name: 'p', createWritable: async () => ({{}}), - }}); + window.showSaveFilePicker = async () => {{ + if ({json.dumps(picker)} === 'no-gesture') {{ + const e = new Error("Failed to execute 'showSaveFilePicker' on 'Window': " + + "Must be handling a user gesture to show a file picker."); + e.name = 'SecurityError'; + throw e; + }} + return {{ name: 'p', createWritable: async () => ({{}}) }}; + }}; }} {target_fn} @@ -207,3 +213,27 @@ def test_the_guard_is_what_the_preview_uses_too(target_fn): "the preview modal must refuse an oversized entry before fetching it") assert not re.search(r"100\s*\*\s*1024\s*\*\s*1024", files_app), ( "the ceiling is defined once, in file-utils.js") + + +def test_a_lost_gesture_streams_instead_of_failing(target_fn, tmp_path): + """ + A browser grants one file picker per user gesture, and downloading three + files is one gesture — so the second and third throw "Must be handling a + user gesture". The person sees a failed transfer, with a message from Chrome + about gestures, for having done something entirely reasonable. + + The streamed path needs no gesture, so it is the right answer rather than a + consolation: the file lands on disk either way, and the only thing lost is + the choice of folder, which there was no picker to make anyway. + """ + out = _run(target_fn, tmp_path, size=20 * GB, + picker="no-gesture", streamed=True, mode="ask") + assert out == {"kind": "stream", "name": "s"}, out + + +def test_a_lost_gesture_with_nothing_to_stream_to_still_refuses(target_fn, tmp_path): + """And the ceiling still holds underneath: no gesture and no stream is not + a reason to put twenty gigabytes in the page.""" + out = _run(target_fn, tmp_path, size=20 * GB, + picker="no-gesture", streamed=False, mode="ask") + assert out["kind"] == "refused", out diff --git a/packages/meshbay-hub/tests/test_transfers.py b/packages/meshbay-hub/tests/test_transfers.py index 3316615..e805afc 100644 --- a/packages/meshbay-hub/tests/test_transfers.py +++ b/packages/meshbay-hub/tests/test_transfers.py @@ -222,3 +222,216 @@ def test_a_folder_name_carries_no_trailing_slash(): assert "dir-row" in row, "the anchor no longer lands on the directory row" assert "${d}/" not in row, "the folder name is rendered with a trailing slash" assert "${d}" in row + + +# ── Transfer slots, client side ───────────────────────────────────────────── +# +# A queue can lie in two directions, and both are worse than no queue: a +# transfer that shows "waiting" on a node that already granted it, and a slot +# the page holds after it has stopped using it. Everything below is one of +# those two. + +def _lease_stub(): + """A Lease as the store sees it, driveable from the test.""" + return """ +class L { + constructor() { + this.state = 'queued'; this.ahead = 2; this.closed = false; + this.released = []; this.tr = 'tr1'; + this._wait = new Promise(r => { this._go = r; }); + } + acquire() { return this._wait; } + release(reason) { if (!this.closed) { this.closed = true; this.released.push(reason); } } + grant() { this.state = 'granted'; if (this._onState) this._onState(this); this._go(); } + push(state, ahead) { this.state = state; this.ahead = ahead; if (this._onState) this._onState(this); } +} +""" + + +def test_a_transfer_waiting_for_a_slot_is_queued_not_running(tmp_path): + out = _run(_lease_stub() + """ + const t = new TransferStore(); + const lease = new L(); + t.start({ kind: 'download', name: 'f', total: 10, lease, + run: async () => { say('ran'); } }); + say(t.list()[0].status, t.list()[0].ahead); + await new Promise(r => setTimeout(r, 0)); + say('still:' + t.list()[0].status); + """, tmp_path) + assert out[:2] == ["queued", 2] + assert "ran" not in out, "the work started before the slot was granted" + assert out[-1] == "still:queued" + + +def test_the_grant_starts_the_work(tmp_path): + out = _run(_lease_stub() + """ + const t = new TransferStore(); + const lease = new L(); + t.start({ kind: 'download', name: 'f', total: 10, lease, + run: async () => { say('ran:' + t.list()[0].status); } }); + lease.grant(); + await new Promise(r => setTimeout(r, 10)); + say('after:' + t.list()[0].status); + """, tmp_path) + assert out[0] == "ran:running" + assert out[1] == "after:done" + + +def test_the_slot_comes_back_however_the_transfer_ends(tmp_path): + """A slot not returned is a member who cannot transfer again until the node + times it out — so this must hold for a throw as much as for a success.""" + out = _run(_lease_stub() + """ + for (const mode of ['ok', 'throw']) { + const t = new TransferStore(); + const lease = new L(); + t.start({ kind: 'download', name: 'f', total: 10, lease, + run: async () => { if (mode === 'throw') throw new Error('x'); } }); + lease.grant(); + await new Promise(r => setTimeout(r, 10)); + say(mode + ':' + lease.released.join(',') + ':' + t.list()[0].status); + } + """, tmp_path) + assert out == ["ok:done:done", "throw:done:failed"] + + +def test_cancelling_while_queued_gives_the_slot_back(tmp_path): + """The transfer somebody is most likely to give up on is the one that has + not started. Its queue entry has to go, or the node grants a slot to a + transfer that will never use it.""" + out = _run(_lease_stub() + """ + const t = new TransferStore(); + const lease = new L(); + const id = t.start({ kind: 'download', name: 'f', total: 10, lease, + run: async () => { say('ran'); } }); + t.cancel(id); + say(t.list()[0].status, lease.released.join(',')); + lease.grant(); + await new Promise(r => setTimeout(r, 10)); + say('ran?', out.includes('ran')); + """, tmp_path) + assert out[0] == "cancelled" + assert out[1] == "cancelled" + assert out[-1] is False, "a cancelled transfer ran anyway once granted" + + +def test_a_queue_position_update_reaches_the_view(tmp_path): + out = _run(_lease_stub() + """ + const t = new TransferStore(); + const lease = new L(); + const seen = []; + t.subscribe(items => seen.push(items[0].ahead)); + t.start({ kind: 'download', name: 'f', total: 10, lease, run: async () => {} }); + lease.push('queued', 1); + lease.push('queued', 0); + say(seen.join('>')); + """, tmp_path) + assert out[0].endswith("1>0"), "the widget never learns it is moving up" + + +def test_a_transport_with_a_queued_transfer_is_not_closed(tmp_path): + """Closing it would leave the transfer waiting for a grant that can never + arrive — waiting for ever, with nothing left to answer.""" + out = _run(_lease_stub() + """ + const t = new TransferStore(); + const lease = new L(); + let closed = false; + const transport = { close() { closed = true; } }; + t.start({ kind: 'download', name: 'f', total: 10, transport, lease, + run: async () => {} }); + t.releaseWhenIdle(transport); + say('closed while queued:', closed); + lease.grant(); + await new Promise(r => setTimeout(r, 10)); + say('closed after:', closed); + """, tmp_path) + assert out[1] is False + assert out[3] is True + + +def test_clearing_finished_keeps_what_is_waiting(tmp_path): + out = _run(_lease_stub() + """ + const t = new TransferStore(); + const lease = new L(); + t.start({ kind: 'download', name: 'waiting', total: 1, lease, run: async () => {} }); + t.start({ kind: 'download', name: 'done', total: 1, run: async () => {} }); + await new Promise(r => setTimeout(r, 10)); + t.clearFinished(); + say(t.list().map(i => i.name + ':' + i.status).join(',')); + """, tmp_path) + assert out[0] == "waiting:queued" + + +def test_asking_for_a_slot_on_a_dead_channel_does_not_throw(tmp_path): + """ + The transport reconnects on its own and re-asks for every live lease when it + does, so a closed channel at the moment a transfer starts is a wait, not a + failure. `_fetchChunkResilient` has always treated it that way — and before + leases existed a chunk request was the first thing to touch the channel, so + a download begun on a briefly dead connection simply retried. + + Asking for a slot first made `_send` the first contact. It threw + "DataChannel not open (state: closed)" straight out of `downloadEntry`, + where nothing catches it: a download that used to recover became an error + with no row in the widget to show it. Found live, by downloading a file + just after a connection dropped. + """ + module = tmp_path / "transport_lease.mjs" + # The real Lease, lifted out as text — the class is not exported, and a + # second copy of it here would agree with whatever it was copied from. + src = (STATIC / "transport.js").read_text() + # From the constant the class depends on, not from the class: lifting only + # the class left LEASE_WATCHDOG_MS undefined, which the class reads the + # first time it arms its watchdog. + start = src.index("const LEASE_WATCHDOG_MS") + end = src.index("\nclass MeshBayTransport") + module.write_text(src[start:end] + "\nexport { Lease };\n") + + script = tmp_path / "case.mjs" + script.write_text(f""" +import {{ Lease }} from '{module.as_posix()}'; +const out = []; +const transport = {{ + supportsTransferSlots: true, + _leases: new Map(), + _send() {{ throw new Error('DataChannel not open (state: closed)'); }}, +}}; +let threw = null; +const lease = new Lease(transport, 'tr1', 'download', 10, 1, null); +try {{ lease._request(); }} catch (e) {{ threw = e.message; }} +out.push(threw); +// And releasing one must be just as safe: a lease not released is a member who +// cannot start another transfer until the node times it out. +try {{ lease.release('cancelled'); out.push('release ok'); }} +catch (e) {{ out.push('release threw: ' + e.message); }} +clearTimeout(lease._watchdog); +console.log(JSON.stringify(out)); +""") + proc = subprocess.run(["node", str(script)], capture_output=True, text=True) + assert proc.returncode == 0, proc.stderr + out = json.loads(proc.stdout) + assert out[0] is None, f"asking for a slot threw: {out[0]}" + assert out[1] == "release ok" + + +def test_the_slot_is_asked_for_after_there_is_somewhere_to_write(): + """ + A granted slot has to be taken up within the node's acceptance deadline, so + it must not be asked for until the download can actually start. + + Asking first reads better — the widget could draw a row while the target is + being chosen — and is wrong: opening a target takes thirty seconds of + streamed-download timeouts, or as long as somebody leaves a Save As dialog + open. The node revokes the grant, passes it to the next in the queue + (`transfer: reclaimed … (not_taken_up)` in its log), and the download then + fetches under a `tr` that is no longer granted. Three downloads started, one + arrived. + + Source-reading, because the ordering is the whole property and it has no + behaviour of its own to drive: what matters is which call comes first. + """ + src = (STATIC / "file-utils.js").read_text() + fn = src[src.index("async function downloadEntry"):] + fn = fn[:fn.index("\n}\n")] + assert fn.index("_openDownloadTarget") < fn.index("openTransfer"), ( + "downloadEntry asks for a transfer slot before it has anywhere to " + "write — the grant expires before the download can use it") diff --git a/packages/meshbay-hub/tests/test_zip_size_limit.py b/packages/meshbay-hub/tests/test_zip_size_limit.py index 44ad59e..e970c55 100644 --- a/packages/meshbay-hub/tests/test_zip_size_limit.py +++ b/packages/meshbay-hub/tests/test_zip_size_limit.py @@ -81,7 +81,17 @@ if ({picker_js}) {{ const M = await import('{(sandbox / "file-utils.js").as_posix()}'); const transfers = {{ start: () => {{ out.started += 1; }} }}; -const transport = {{ connected: true }}; +// A transport hands out transfer slots now (transfers.py's leases). The stub +// grants at once, which is what a node with no caps does: what this file is +// about is the archive limit, not the queue. +const transport = {{ + connected: true, + openTransfer: () => ({{ + tr: 'stub', state: 'granted', ahead: 0, + acquire: () => Promise.resolve(), + release: () => {{}}, + }}), +}}; // One file, in the folder itself — entriesUnder keys on `path`. const entries = [{{ id: 'f1', name: 'big.bin', path: 'album', size: {total_bytes}, added_at: 0 }}]; -- cgit v1.2.3 From 77615ddb5fead3e74751a94847d3bcc99fc0a96d Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Tue, 8 Sep 2026 23:43:46 +0200 Subject: fix(hub): the transfers row exists from the click MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clicking Download produced nothing — no row, no icon, no panel — for as long as it took to open somewhere to write, and then several rows at once. The streamed path waits for the worker twice; a Save As dialog waits for a person. The row was created after that, so the slowest part of a download happened with nothing on screen to say it had begun. The store gains a `prepare` step, distinct from `run`, and the order is now: row, then target, then slot. That last part is why the obvious fix was wrong. Taking the slot first would let the row appear immediately, and it was tried this morning: a granted slot has to be taken up within the node's deadline, opening a target can outlast it, and three downloads became one. (The diagnosis at the time blamed that ordering for revocations which were in fact a missing `touch()` call — the revert was right for the wrong reason.) `makeLease` is called after `prepare` succeeds, never before. Three behaviours fall out, each with a test: - a dismissed dialog leaves nothing behind. `prepare` returning false drops the row: nothing started, so nothing should remain on screen to explain it; - the row takes the name the file was actually saved under, once known; - a refusal above the memory ceiling fails the row that is already there, rather than creating one to kill it. `preparing` counts as live everywhere — badge, cancel, clearFinished, and `_busy`, since closing a transport under a preparing transfer strands it exactly as under a queued one. Six places asked "is this finished?" and were drifting apart; there is one definition now. Two mistakes in the tests, worth the note: one counted positions in an output array by hand and was one out, which reads exactly like a failing assertion about the code — the values are tagged now, not indexed. And test_zip_size_limit.py's stub did not run `prepare`, so it no longer reached the size check the file is about; it now behaves like the real store. 819 hub, 1169 node, 0 failed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HCGdheDLxGReuKHga3BtST --- packages/meshbay-hub/src/meshbay_hub/static/app.js | 23 +++- .../src/meshbay_hub/static/file-utils.js | 111 +++++++++----------- .../src/meshbay_hub/static/locales/de.js | 1 + .../src/meshbay_hub/static/locales/en.js | 1 + .../src/meshbay_hub/static/locales/es.js | 1 + .../src/meshbay_hub/static/locales/fr.js | 1 + .../src/meshbay_hub/static/locales/it.js | 1 + .../src/meshbay_hub/static/locales/ja.js | 1 + .../src/meshbay_hub/static/locales/nl.js | 1 + .../src/meshbay_hub/static/locales/pl.js | 1 + .../src/meshbay_hub/static/locales/pt-BR.js | 1 + .../src/meshbay_hub/static/locales/zh-CN.js | 1 + .../src/meshbay_hub/static/transfers.js | 90 ++++++++++++---- packages/meshbay-hub/tests/test_transfers.py | 116 +++++++++++++++++++++ packages/meshbay-hub/tests/test_zip_size_limit.py | 17 ++- 15 files changed, 280 insertions(+), 87 deletions(-) (limited to 'packages/meshbay-hub/tests/test_zip_size_limit.py') diff --git a/packages/meshbay-hub/src/meshbay_hub/static/app.js b/packages/meshbay-hub/src/meshbay_hub/static/app.js index ff066cf..6f4bb92 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/app.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/app.js @@ -157,9 +157,11 @@ function TransferWidget() { }, [open]); const running = items.filter(i => i.status === 'running'); - const waiting = items.filter(i => i.status === 'queued'); + const waiting = items.filter( + i => i.status === 'queued' || i.status === 'preparing'); const finished = items.filter( - i => i.status !== 'running' && i.status !== 'queued'); + i => i.status !== 'running' && i.status !== 'queued' + && i.status !== 'preparing'); const active = running.length + waiting.length; // Grouped, and in this order: what is moving, what is waiting, what is over. @@ -240,7 +242,8 @@ function TransferRow({ it }) { ? html` { e.preventDefault(); transfers.open(it.id); }}>${it.name}` : html`${it.name}`} - ${(it.status === 'running' || it.status === 'queued') && html` + ${(it.status === 'running' || it.status === 'queued' + || it.status === 'preparing') && html`
- ${it.status === 'queued' + ${it.status === 'preparing' + ? html` + ${/* Not a progress bar at 0%: nothing is wrong and nothing is + stalled, the download is still finding somewhere to write. The + row exists from the click precisely so this state is visible + instead of being an empty panel. */''} +
+
+ ${t('transfers.preparing')} + ${formatSize(it.total)} +
+ ` + : it.status === 'queued' ? html`
diff --git a/packages/meshbay-hub/src/meshbay_hub/static/file-utils.js b/packages/meshbay-hub/src/meshbay_hub/static/file-utils.js index 61002d6..3475f81 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/file-utils.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/file-utils.js @@ -321,47 +321,34 @@ async function pipelinedDownload(transport, gekKey, fileId, totalChunks, onChunk * download button — both just want "get this entry to disk". */ async function downloadEntry(transfers, transport, gek, entry) { - // The target FIRST, then the slot — and that order is load-bearing. - // - // Asking for the slot first looks better (the widget could draw a row while - // the target is being chosen) and is wrong: a granted slot has to be taken up - // within the node's acceptance deadline, and opening a target can take thirty - // seconds of streamed-download timeouts, or as long as somebody leaves a Save - // As dialog open. The node then revokes the grant and passes it to the next - // in the queue — `transfer: reclaimed … (not_taken_up)` in its log — and this - // download starts fetching under a `tr` that is no longer granted. - // - // Measured, not reasoned: three downloads started, one arrived, and the - // node's log named the reason. Do not move this again without moving the - // deadline, and the deadline exists so a client that dies between asking and - // starting does not hold a slot nobody can use. - let target; - try { - target = await _openDownloadTarget(entry.name, entry.size); - } catch (err) { - // The refusal belongs in the transfers panel, not in a console nobody - // opens: that is where someone who just clicked Download is looking, and a - // failed row naming the reason is the whole point of refusing rather than - // filling the tab. Started only to be failed, deliberately. - transfers.start({ - kind: 'download', name: entry.name, total: entry.size, transport, - run: async () => { throw err; }, - }); - return; - } - if (target === false) return; // the picker was dismissed - - const openRef = { url: null }; const totalChunks = Math.ceil(entry.size / CHUNK_SIZE); + const openRef = { url: null }; + let target = null; + transfers.start({ - kind: 'download', name: (target && target.name) || entry.name, - total: entry.size, transport, - // Asked for here, once there is somewhere to write: see the note above. - lease: transport.openTransfer({ + kind: 'download', name: entry.name, total: entry.size, transport, + + // The row exists from the click. Opening a target is what takes the time — + // the streamed path waits for the worker (twice), a Save As dialog waits + // for a person — and doing it before the row meant three clicks produced no + // panel at all and then several rows at once. + prepare: async () => { + target = await _openDownloadTarget(entry.name, entry.size); + // Dismissed: nothing was started, so nothing is left on screen. + if (target === false) return false; + return target ? { name: target.name } : true; + }, + + // After the target, never before: a granted slot has to be taken up within + // the node's deadline, and opening a target can outlast it. See §8.1 of + // ~/next/improve-downloads.md — the other order was tried and cost two of + // three downloads. + makeLease: () => transport.openTransfer({ kind: 'download', bytes: entry.size, chunks: totalChunks }), - open: target - ? (target.open || null) - : () => { if (openRef.url) window.open(openRef.url, '_blank'); }, + + open: () => (target && target.open) ? target.open() + : (openRef.url ? window.open(openRef.url, '_blank') : undefined), + run: async ({ signal, onProgress, lease }) => { let done = 0; const onChunk = (bytes) => { done += bytes; onProgress(done, entry.size); }; @@ -435,38 +422,36 @@ async function downloadDirectory(transfers, transport, gek, entries, dir, { setE // totalBytes decides how this is delivered, but it is not the archive's // size — headers and the central directory come on top — so it is not // announced as a Content-Length that the download would then miss. - let target; - try { - target = await _openDownloadTarget(suggested, totalBytes, { - types: [{ description: 'ZIP archive', - accept: { 'application/zip': ['.zip'] } }], - }, 0); - } catch (err) { - // Reported beside the folder that was clicked, like zip_too_large just - // above — this function is called in a loop over a selection, and the - // sibling folders must still download. - setError(err.message); - return; - } - if (target === false) return; - if (!target && !confirm(t('group.zip_no_stream', { - size: formatSize(totalBytes), name: suggested, - }))) { - return; - } const zipOpenRef = { url: null }; + let target = null; transfers.start({ - kind: 'download', name: (target && target.name) || suggested, - total: totalBytes, transport, + kind: 'download', name: suggested, total: totalBytes, transport, + + // Same order as downloadEntry: the row first, then the target, then the + // slot. A folder of forty files is exactly where the wait is longest. + prepare: async () => { + target = await _openDownloadTarget(suggested, totalBytes, { + types: [{ description: 'ZIP archive', + accept: { 'application/zip': ['.zip'] } }], + }, 0); + if (target === false) return false; + if (!target && !confirm(t('group.zip_no_stream', { + size: formatSize(totalBytes), name: suggested, + }))) { + return false; + } + return target ? { name: target.name } : true; + }, + // **One** lease for the archive, not one per file. Dozens of leases for a // folder would deadlock against the member's own cap: the job cannot finish // until it holds them all, and it can never hold more than two. - lease: transport.openTransfer({ + makeLease: () => transport.openTransfer({ kind: 'download', bytes: totalBytes, chunks: files.length }), - open: target - ? (target.open || null) - : () => { if (zipOpenRef.url) window.open(zipOpenRef.url, '_blank'); }, + + open: () => (target && target.open) ? target.open() + : (zipOpenRef.url ? window.open(zipOpenRef.url, '_blank') : undefined), run: async ({ signal, onProgress, lease }) => { const writable = target ? target.writable : null; const parts = writable ? null : []; diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/de.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/de.js index f5c0c78..1458cf2 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/de.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/de.js @@ -578,6 +578,7 @@ export default { 'transfers.open': 'Öffnen', 'transfers.done': 'Abgeschlossen', 'transfers.cancelled': 'Abgebrochen', + 'transfers.preparing': 'Wird vorbereitet…', 'transfers.waiting_own_slots': 'Wartet — Ihre Plätze sind belegt', 'transfers.waiting_node': 'Wartet — {n} davor', 'transfers.summary': '{running} laufend · {waiting} wartend', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/en.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/en.js index e3ed844..b0430c0 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/en.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/en.js @@ -694,6 +694,7 @@ export default { 'transfers.open': 'Open', 'transfers.done': 'Finished', 'transfers.cancelled': 'Cancelled', + 'transfers.preparing': 'Preparing…', 'transfers.waiting_own_slots': 'Waiting — your slots are busy', 'transfers.waiting_node': 'Waiting — {n} ahead', 'transfers.summary': '{running} running · {waiting} waiting', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/es.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/es.js index c509599..adfb1cc 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/es.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/es.js @@ -574,6 +574,7 @@ export default { 'transfers.open': 'Abrir', 'transfers.done': 'Terminada', 'transfers.cancelled': 'Cancelada', + 'transfers.preparing': 'Preparando…', 'transfers.waiting_own_slots': 'En espera — sus espacios están ocupados', 'transfers.waiting_node': 'En espera — {n} por delante', 'transfers.summary': '{running} en curso · {waiting} en espera', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/fr.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/fr.js index 49a1071..b409927 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/fr.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/fr.js @@ -577,6 +577,7 @@ export default { 'transfers.open': 'Ouvrir', 'transfers.done': 'Terminé', 'transfers.cancelled': 'Annulé', + 'transfers.preparing': 'Préparation…', 'transfers.waiting_own_slots': 'En attente — vos slots sont occupés', 'transfers.waiting_node': 'En attente — {n} devant', 'transfers.summary': '{running} en cours · {waiting} en attente', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/it.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/it.js index 8f1ef1e..7e6246e 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/it.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/it.js @@ -577,6 +577,7 @@ export default { 'transfers.open': 'Apri', 'transfers.done': 'Completato', 'transfers.cancelled': 'Annullato', + 'transfers.preparing': 'Preparazione…', 'transfers.waiting_own_slots': 'In attesa — i suoi posti sono occupati', 'transfers.waiting_node': 'In attesa — {n} prima', 'transfers.summary': '{running} in corso · {waiting} in attesa', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/ja.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/ja.js index 7ea9789..a74eec9 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/ja.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/ja.js @@ -569,6 +569,7 @@ export default { 'transfers.open': '開く', 'transfers.done': '完了', 'transfers.cancelled': 'キャンセル済み', + 'transfers.preparing': '準備中…', 'transfers.waiting_own_slots': '待機中 — 自分の枠がすべて使用中です', 'transfers.waiting_node': '待機中 — 前に {n} 件', 'transfers.summary': '実行中 {running} · 待機中 {waiting}', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/nl.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/nl.js index 74cd269..a7d64f7 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/nl.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/nl.js @@ -578,6 +578,7 @@ export default { 'transfers.open': 'Openen', 'transfers.done': 'Voltooid', 'transfers.cancelled': 'Geannuleerd', + 'transfers.preparing': 'Voorbereiden…', 'transfers.waiting_own_slots': 'Wacht — uw plaatsen zijn bezet', 'transfers.waiting_node': 'Wacht — {n} ervoor', 'transfers.summary': '{running} bezig · {waiting} wachtend', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/pl.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/pl.js index 584e5f2..5bd34fb 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/pl.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/pl.js @@ -590,6 +590,7 @@ export default { 'transfers.open': 'Otwórz', 'transfers.done': 'Zakończony', 'transfers.cancelled': 'Anulowany', + 'transfers.preparing': 'Przygotowywanie…', 'transfers.waiting_own_slots': 'Oczekiwanie — Twoje miejsca są zajęte', 'transfers.waiting_node': 'Oczekiwanie — {n} przed', 'transfers.summary': '{running} w toku · {waiting} oczekuje', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/pt-BR.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/pt-BR.js index b034117..102ec92 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/pt-BR.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/pt-BR.js @@ -576,6 +576,7 @@ export default { 'transfers.open': 'Abrir', 'transfers.done': 'Concluída', 'transfers.cancelled': 'Cancelada', + 'transfers.preparing': 'Preparando…', 'transfers.waiting_own_slots': 'Aguardando — seus espaços estão ocupados', 'transfers.waiting_node': 'Aguardando — {n} na frente', 'transfers.summary': '{running} em andamento · {waiting} aguardando', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/locales/zh-CN.js b/packages/meshbay-hub/src/meshbay_hub/static/locales/zh-CN.js index ffe2cb0..6b64785 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/locales/zh-CN.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/locales/zh-CN.js @@ -557,6 +557,7 @@ export default { 'transfers.open': '打开', 'transfers.done': '已完成', 'transfers.cancelled': '已取消', + 'transfers.preparing': '准备中…', 'transfers.waiting_own_slots': '等待中 — 您的通道已占满', 'transfers.waiting_node': '等待中 — 前面还有 {n} 个', 'transfers.summary': '进行中 {running} · 等待中 {waiting}', diff --git a/packages/meshbay-hub/src/meshbay_hub/static/transfers.js b/packages/meshbay-hub/src/meshbay_hub/static/transfers.js index 46f75a2..d3b164d 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/transfers.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/transfers.js @@ -22,6 +22,12 @@ const SPEED_WINDOW_MS = 5000; +/** Not finished: still preparing, waiting for a slot, or transferring. One + * definition, because six places ask and they were drifting apart. */ +function _live(status) { + return status === 'preparing' || status === 'queued' || status === 'running'; +} + function _abortError() { const err = new Error('Cancelled'); err.name = 'AbortError'; @@ -84,8 +90,7 @@ export class TransferStore { /** Running or waiting for a slot — what the nav badge counts. */ get pending() { - return this._items.filter( - it => it.status === 'running' || it.status === 'queued').length; + return this._items.filter(it => _live(it.status)).length; } _speed(it) { @@ -104,8 +109,28 @@ export class TransferStore { * `run` receives `{ signal, onProgress }`. It must poll `signal.aborted` — a * cancel that only sets a flag nobody reads is a button that lies. */ + /** + * Start a transfer. + * + * `run` receives `{ signal, onProgress, lease }`. It must poll + * `signal.aborted` — a cancel that only sets a flag nobody reads is a button + * that lies. + * + * `prepare` is optional and runs before anything else, with the row already + * on screen. It is where a download opens its target, which can take tens of + * seconds — the streamed path waits for the worker, twice, and a Save As + * dialog waits for a person. Doing that *before* creating the row meant three + * clicks produced no panel at all, not even the icon, and then several rows + * at once. Returning `false` drops the row again, which is what a dismissed + * dialog should look like: nothing, rather than a cancelled transfer nobody + * started. + * + * `makeLease` is called after `prepare` succeeds, never before. A granted + * slot must be taken up within the node's deadline, so it is asked for once + * there is somewhere to write — see file-utils.js's downloadEntry. + */ start({ kind, name, total = 0, transport = null, run, open = null, - lease = null }) { + lease = null, prepare = null, makeLease = null }) { const item = { id: _nextId++, kind, name, total, transport, open, lease, @@ -114,7 +139,8 @@ export class TransferStore { // 'running'. Two different things are true of it — nothing is moving, and // nothing is wrong — and a status that conflates them is what makes a // queue look like a hang. - status: lease && lease.state !== 'granted' ? 'queued' : 'running', + status: prepare ? 'preparing' + : (lease && lease.state !== 'granted' ? 'queued' : 'running'), ahead: (lease && lease.ahead) || 0, error: '', samples: [{ t: this._now(), done: 0 }], @@ -155,17 +181,32 @@ export class TransferStore { // appear to move — the widget showing "3 ahead" for ever while the node // quietly worked through the queue. Nothing about that looks wrong from // either side, which is why it needs a test rather than a reading. - if (item.lease) { - item.lease._onState = (lease) => { - if (item.status !== 'queued' && item.status !== 'running') return; - item.ahead = lease.ahead; - item.status = lease.state === 'granted' ? 'running' : 'queued'; - this._emit(); - }; - } + if (item.lease) this._watchLease(item); const promise = Promise.resolve() .then(async () => { + if (prepare) { + const ready = await prepare(); + if (item.signal.aborted) throw _abortError(); + if (ready === false) { + // Dismissed. Not a failure and not a cancellation: nothing was ever + // started, so nothing should be left on screen to explain. + this._drop(item.id); + return undefined; + } + if (ready && ready.name) item.name = ready.name; + item.status = 'running'; + this._emit(); + } + if (makeLease && !item.lease) { + item.lease = makeLease(); + this._watchLease(item); + if (item.lease.state !== 'granted') { + item.status = 'queued'; + item.ahead = item.lease.ahead || 0; + this._emit(); + } + } if (item.lease) { await item.lease.acquire(); if (item.signal.aborted) throw _abortError(); @@ -191,6 +232,21 @@ export class TransferStore { return item.id; } + _watchLease(item) { + item.lease._onState = (lease) => { + if (item.status !== 'queued' && item.status !== 'running') return; + item.ahead = lease.ahead; + item.status = lease.state === 'granted' ? 'running' : 'queued'; + this._emit(); + }; + } + + /** Remove a row entirely. Only for a transfer that never started. */ + _drop(id) { + this._items = this._items.filter(it => it.id !== id); + this._emit(); + } + /** * Hand a finished download to the browser to display. * @@ -209,7 +265,7 @@ export class TransferStore { // 'queued' too: a transfer waiting for a slot is exactly the one somebody // is most likely to give up on, and its queue entry has to go with it or // the node grants a slot to a transfer that will never use it. - if (!item || (item.status !== 'running' && item.status !== 'queued')) return; + if (!item || !_live(item.status)) return; item.signal.aborted = true; if (item.lease) item.lease.release('cancelled'); // Marked at once. The work stops when it next looks, but a cancelled @@ -221,14 +277,13 @@ export class TransferStore { cancelAll() { for (const it of this._items) { - if (it.status === 'running' || it.status === 'queued') this.cancel(it.id); + if (_live(it.status)) this.cancel(it.id); } } /** Drop everything finished, keeping what is still running or waiting. */ clearFinished() { - this._items = this._items.filter( - it => it.status === 'running' || it.status === 'queued'); + this._items = this._items.filter(it => _live(it.status)); this._emit(); } @@ -237,8 +292,7 @@ export class TransferStore { // slot can never be granted one, and the transfer would sit at "waiting" // for ever with nothing left to answer it. return this._items.some( - it => it.transport === transport - && (it.status === 'running' || it.status === 'queued')); + it => it.transport === transport && _live(it.status)); } /** diff --git a/packages/meshbay-hub/tests/test_transfers.py b/packages/meshbay-hub/tests/test_transfers.py index e805afc..7e21409 100644 --- a/packages/meshbay-hub/tests/test_transfers.py +++ b/packages/meshbay-hub/tests/test_transfers.py @@ -435,3 +435,119 @@ def test_the_slot_is_asked_for_after_there_is_somewhere_to_write(): assert fn.index("_openDownloadTarget") < fn.index("openTransfer"), ( "downloadEntry asks for a transfer slot before it has anywhere to " "write — the grant expires before the download can use it") + + +# ── the row exists from the click ─────────────────────────────────────────── + +def test_the_row_appears_before_the_target_is_open(tmp_path): + """ + Opening a target is the slow part — the streamed path waits for the worker + twice, a Save As dialog waits for a person — and the row used to be created + only after it returned. Three clicks produced no panel at all, not even the + icon, and then several rows at once. + """ + out = _run(""" + const t = new TransferStore(); + let release; + const opened = new Promise(r => { release = r; }); + t.start({ kind: 'download', name: 'film.mkv', total: 10, + prepare: async () => { await opened; return { name: 'saved.mkv' }; }, + run: async () => { say('ran'); } }); + const shot = (when) => say(when + '=' + t.list().length + ':' + + t.list().map(i => i.status + '/' + i.name).join(',')); + shot('click'); + release(); + await new Promise(r => setTimeout(r, 10)); + shot('after'); + """, tmp_path) + # Tagged, not indexed. An earlier version counted pushes by hand and was one + # out, which reads exactly like a failing assertion about the code. + seen = dict(line.split("=", 1) for line in out + if isinstance(line, str) and "=" in line) + assert {"click", "after"} <= set(seen), f"probe produced: {out}" + assert seen["click"] == "1:preparing/film.mkv", ( + f"no row, or the wrong one, at the moment of the click: {seen['click']}") + assert seen["after"] == "1:done/saved.mkv", ( + f"the row must keep the name it was saved under: {seen['after']}") + + +def test_a_dismissed_dialog_leaves_nothing_behind(tmp_path): + """Dismissing a Save As dialog is not a failure and not a cancellation: + nothing was started, so nothing should be left on screen explaining it.""" + out = _run(""" + const t = new TransferStore(); + t.start({ kind: 'download', name: 'film.mkv', total: 10, + prepare: async () => false, + run: async () => { say('ran'); } }); + say('at click:', t.list().length); + await new Promise(r => setTimeout(r, 10)); + say('after:', t.list().length, out.includes('ran')); + """, tmp_path) + assert out[1] == 1 + assert out[3] == 0, "a dismissed dialog left a row behind" + assert out[4] is False + + +def test_the_slot_is_only_asked_for_once_there_is_somewhere_to_write(tmp_path): + """ + A granted slot must be taken up within the node's deadline, and opening a + target can outlast it. Asking first cost two of three downloads. + """ + out = _run(""" + const t = new TransferStore(); + let release; + const opened = new Promise(r => { release = r; }); + let asked = false; + t.start({ kind: 'download', name: 'f', total: 10, + prepare: async () => { await opened; return true; }, + makeLease: () => { asked = true; return { + state: 'granted', ahead: 0, tr: 'x', + acquire: () => Promise.resolve(), release: () => {} }; }, + run: async () => {} }); + say('while preparing, asked?', asked); + release(); + await new Promise(r => setTimeout(r, 10)); + say('after preparing, asked?', asked, t.list()[0].status); + """, tmp_path) + assert out[1] is False, "the slot was taken before there was a target" + assert out[3] is True + assert out[4] == "done" + + +def test_a_target_that_cannot_be_opened_fails_the_row_it_already_has(tmp_path): + """The refusal above the memory ceiling lands in the panel, on the row that + is already there, rather than in a console nobody opens.""" + out = _run(""" + const t = new TransferStore(); + t.start({ kind: 'download', name: 'film.mkv', total: 10, + prepare: async () => { throw new Error('too large for memory'); }, + run: async () => { say('ran'); } }); + await new Promise(r => setTimeout(r, 10)); + const it = t.list()[0]; + say(it.status, it.error, out.includes('ran')); + """, tmp_path) + assert out[0] == "failed" + assert "too large" in out[1] + assert out[2] is False + + +def test_a_transport_is_not_closed_under_a_preparing_transfer(tmp_path): + """It has no lease yet and has moved no bytes, but closing its transport + would strand it exactly like a queued one.""" + out = _run(""" + const t = new TransferStore(); + let release; + const opened = new Promise(r => { release = r; }); + let closed = false; + const transport = { close() { closed = true; } }; + t.start({ kind: 'download', name: 'f', total: 10, transport, + prepare: async () => { await opened; return true; }, + run: async () => {} }); + t.releaseWhenIdle(transport); + say('closed while preparing:', closed); + release(); + await new Promise(r => setTimeout(r, 10)); + say('closed after:', closed); + """, tmp_path) + assert out[1] is False + assert out[3] is True diff --git a/packages/meshbay-hub/tests/test_zip_size_limit.py b/packages/meshbay-hub/tests/test_zip_size_limit.py index e970c55..28e3a0c 100644 --- a/packages/meshbay-hub/tests/test_zip_size_limit.py +++ b/packages/meshbay-hub/tests/test_zip_size_limit.py @@ -60,7 +60,7 @@ globalThis.localStorage = {{ // Node 22 defines `navigator` itself, so it is left alone; `window` is what // platform.js reaches for to decide it is not running in the desktop app. globalThis.window = globalThis; -const out = {{ errors: [], started: 0, asked: 0 }}; +const out = {{ errors: [], started: 0, asked: 0, dropped: 0 }}; // Reached only once the size check has passed: with no File System Access API // under Node, downloadDirectory falls through to its build-in-memory path and // asks first. Answering yes is what lets the at-the-limit case get as far as @@ -80,7 +80,18 @@ if ({picker_js}) {{ const M = await import('{(sandbox / "file-utils.js").as_posix()}'); -const transfers = {{ start: () => {{ out.started += 1; }} }}; +// Faithful enough to the real store: it runs `prepare` and honours what it +// returns. The target is opened there now — the row exists from the click and +// the slow part happens behind it — so a stub that only counts calls would +// never reach the size check this file is about. +const transfers = {{ start: (opts) => {{ + out.started += 1; + if (!opts.prepare) return; + Promise.resolve() + .then(() => opts.prepare()) + .then((ready) => {{ if (ready === false) {{ out.started -= 1; out.dropped += 1; }} }}) + .catch((e) => {{ out.started -= 1; out.errors.push(e.message); }}); +}} }}; // A transport hands out transfer slots now (transfers.py's leases). The stub // grants at once, which is what a node with no caps does: what this file is // about is the archive limit, not the queue. @@ -100,6 +111,8 @@ await M.downloadDirectory(transfers, transport, null, entries, 'album', {{ setError: (m) => out.errors.push(m), }}); +// `prepare` runs on a microtask, so let it. +await new Promise(r => setTimeout(r, 10)); out.limit = M.ZIP_MAX_BYTES; console.log(JSON.stringify(out)); """, encoding="utf-8") -- cgit v1.2.3 From d6c4808d9a3ef6bc740cda7892508f15ee9ea030 Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Wed, 9 Sep 2026 01:54:10 +0200 Subject: fix(spa): one save dialog per batch, not one per file MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Selecting four files on Chrome produced a Save As dialog for the first, then one for the second only after that file had finished, while the last two timed out; on a later attempt the three remaining transfers appeared frozen. Two things were going on. Opening the target inside `prepare` had removed the accidental serialisation that `for (…) await downloadFile(e)` used to provide, so `_openTargetInTurn` now queues the openings — but a queue whose head is an unanswered dialog is a head-of-line block, which is what the "freeze" was. The code already recovered from a picker with no gesture behind it by streaming instead, on the `SecurityError` Chrome throws. That branch was never reached: Chrome does not throw, it shows the dialog anyway and waits for a human. So anything that has to wait its turn is now marked `batched`, and a batched opening prefers the streamed path whatever the download mode says. The first file of a batch — the one actually holding the gesture — still gets its dialog, so the preference is honoured where it can be. For the rest there is no gesture left to spend and nothing is lost by streaming: the file still lands on disk, in the browser's own download folder. Only the choice of folder goes, and it was not on offer. If the worker does not answer, a batched download falls back to the dialog rather than failing. Also logs which path led to a dialog. A dialog is the one outcome nobody can diagnose after the fact — it looks the same whether it was asked for or fallen back to — and the report this fixes needed three test cycles to narrow. The two harnesses that lift `_openDownloadTarget` as text now route console.info to stderr, since they parse stdout as JSON. Measured against the deployed hub in Chrome 152: the streamed path serves the hidden iframe in 2-3 ms on a normal load, after a hard reload (via the `mbdl-claim` recovery already in `_claimController`), and twice in the same document. Hub suite 824 passed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01HCGdheDLxGReuKHga3BtST --- .../src/meshbay_hub/static/file-utils.js | 58 +++++++++++++++++-- packages/meshbay-hub/tests/test_downloads.py | 66 +++++++++++++++++++++- packages/meshbay-hub/tests/test_memory_ceiling.py | 53 ++++++++++++++++- packages/meshbay-hub/tests/test_transfers.py | 4 +- packages/meshbay-hub/tests/test_zip_size_limit.py | 4 ++ 5 files changed, 177 insertions(+), 8 deletions(-) (limited to 'packages/meshbay-hub/tests/test_zip_size_limit.py') diff --git a/packages/meshbay-hub/src/meshbay_hub/static/file-utils.js b/packages/meshbay-hub/src/meshbay_hub/static/file-utils.js index 3475f81..065079b 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/file-utils.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/file-utils.js @@ -98,7 +98,7 @@ class TooLargeForMemoryError extends Error { * bare `return null` appears in this function. */ async function _openDownloadTarget(filename, size = 0, pickerOpts = {}, - swSize = size) { + swSize = size, { batched = false } = {}) { // "Collect it in the page", or a refusal when that would be too much. const _memoryFloor = () => { if (size > MEMORY_CEILING) throw new TooLargeForMemoryError(filename, size); @@ -144,7 +144,18 @@ async function _openDownloadTarget(filename, size = 0, pickerOpts = {}, // this block left nothing but the in-memory floor. A preference must not cost // a capability. const canPick = typeof window.showSaveFilePicker === 'function'; - if (downloads.getMode() === 'auto' || (size > MEMORY_CEILING && !canPick)) { + // `batched` means this is not the first download of a batch, and it makes the + // streamed path preferred whatever the mode. + // + // "Ask where to save" asks per file, which is right for one file and wrong + // for four: a browser grants one picker per user gesture, so the second + // dialog has no gesture behind it and the third and fourth wait behind a + // dialog that waits for a human — reported from Chrome as three downloads + // frozen. There is no gesture left to spend, so there is nothing to lose by + // streaming instead: the file still lands on disk, in the browser's own + // download folder. Only the choice of folder goes, and it was not on offer. + if (downloads.getMode() === 'auto' || batched + || (size > MEMORY_CEILING && !canPick)) { const streamed = await downloads.openStreamedDownload(filename, swSize); if (streamed) return streamed; // Nothing to stream to: small enough for memory, and no dialog. The old @@ -157,6 +168,13 @@ async function _openDownloadTarget(filename, size = 0, pickerOpts = {}, // No File System Access API — Firefox, Safari. This is the branch that used // to return null at any size. if (!canPick) return _memoryFloor(); + // A dialog is the one outcome nobody can diagnose after the fact: it looks + // the same whether it was asked for, or fallen back to because the worker + // did not answer. Say which, once per download, so the next report from a + // browser we do not have does not need a second round trip. + console.info('[MeshBay] asking where to save %s — mode=%s batched=%s stream=%s', + filename, downloads.getMode(), batched, + downloads.lastStreamFailure() || 'not attempted'); try { const handle = await window.showSaveFilePicker({ suggestedName: filename, ...pickerOpts, @@ -187,6 +205,38 @@ async function _openDownloadTarget(filename, size = 0, pickerOpts = {}, } } +// Target openings run one at a time, across every download on the page. +// +// A browser shows one file picker at a time and grants one per user gesture, so +// four downloads asking at once get one dialog and three failures. That used to +// be prevented by accident: `downloadEntry` awaited the target inline, and +// files-app.js's `for (const e of selected) await downloadFile(e)` serialised +// them. Opening the target inside `prepare` — so the row appears at the click +// instead of tens of seconds later — removed that accident, and four pickers +// raced. Chrome showed one, prompted for a second, and the rest timed out; +// Firefox and Electron never noticed, because neither opens a picker at all. +// +// So the queue is explicit now, and it is the *targets* that queue, not the +// rows: every download still appears the moment it is asked for. +let _targetQueue = Promise.resolve(); + +let _targetsInFlight = 0; + +function _openTargetInTurn(filename, size, pickerOpts, swSize) { + // Anything that has to wait its turn is, by definition, not the first of the + // batch — so it will not be the one holding the user's gesture. + const batched = _targetsInFlight > 0; + _targetsInFlight += 1; + const mine = _targetQueue + .then(() => _openDownloadTarget(filename, size, pickerOpts, swSize, + { batched })) + .finally(() => { _targetsInFlight -= 1; }); + // The chain must not break on a rejection, or one refused download stops + // every later one from ever opening a target. + _targetQueue = mine.catch(() => {}); + return mine; +} + /** The download of last resort, for browsers with no way to stream to disk. */ function _saveBlob(blob, filename) { const url = URL.createObjectURL(blob); @@ -333,7 +383,7 @@ async function downloadEntry(transfers, transport, gek, entry) { // for a person — and doing it before the row meant three clicks produced no // panel at all and then several rows at once. prepare: async () => { - target = await _openDownloadTarget(entry.name, entry.size); + target = await _openTargetInTurn(entry.name, entry.size); // Dismissed: nothing was started, so nothing is left on screen. if (target === false) return false; return target ? { name: target.name } : true; @@ -431,7 +481,7 @@ async function downloadDirectory(transfers, transport, gek, entries, dir, { setE // Same order as downloadEntry: the row first, then the target, then the // slot. A folder of forty files is exactly where the wait is longest. prepare: async () => { - target = await _openDownloadTarget(suggested, totalBytes, { + target = await _openTargetInTurn(suggested, totalBytes, { types: [{ description: 'ZIP archive', accept: { 'application/zip': ['.zip'] } }], }, 0); diff --git a/packages/meshbay-hub/tests/test_downloads.py b/packages/meshbay-hub/tests/test_downloads.py index e68396f..81ab9d3 100644 --- a/packages/meshbay-hub/tests/test_downloads.py +++ b/packages/meshbay-hub/tests/test_downloads.py @@ -154,7 +154,10 @@ def test_a_length_is_only_promised_when_it_is_known(tmp_path): # the ability to refuse an oversized download (test_memory_ceiling.py). # What this test is about -- the `0` -- did not move. app = (STATIC / "file-utils.js").read_text() - zip_call = app[app.index("_openDownloadTarget(suggested"):] + # Anchored on the argument list, not on the function name: the call became + # `_openTargetInTurn(suggested, …)` when target openings were serialised. + # The `0` this test is about did not move. + zip_call = app[app.index("(suggested, totalBytes"):] zip_call = zip_call[:zip_call.index(");") + 2] assert zip_call.rstrip().endswith(", 0);"), ( "the zip download announces a Content-Length it will not match") @@ -363,3 +366,64 @@ def test_the_worker_is_kept_alive_while_it_streams(): # event, but the reply is what tells the page it is talking to the worker # that holds its stream. assert "mbdl-ping" in sw and "mbdl-pong" in sw + + +def test_targets_are_opened_one_at_a_time(tmp_path): + """ + A browser shows one file picker at a time and grants one per user gesture, + so four downloads asking at once get one dialog and three failures. + + That used to be prevented by accident: `downloadEntry` awaited the target + inline and files-app.js's `for (…) await downloadFile(e)` serialised them. + Opening the target inside `prepare` — so the row appears at the click rather + than tens of seconds later — removed the accident, and four pickers raced. + Reported from Chrome: one file downloaded, a prompt for the second, the + other two timed out. Firefox and Electron never noticed, because neither + opens a picker at all, which is why this reached one browser only. + + The queue is on the *targets*, never on the rows: every download still + appears the moment it is asked for. + + Queueing alone was not enough: a second dialog with no gesture behind it + still waits for a human, and the two behind it wait for the dialog. So + everything that has to wait its turn is also marked `batched`, which the + opener reads as "do not ask" — see the streamed-path branch below. + """ + src = (STATIC / "file-utils.js").read_text() + fn = src[src.index("function _openTargetInTurn"):] + fn = fn[:fn.index("\n}\n") + 2] + + script = tmp_path / "case.mjs" + script.write_text(""" +const out = []; +let live = 0, peak = 0; +// Stands in for _openDownloadTarget: records how many are open at once. +const asked = []; +const _openDownloadTarget = async (name, size, opts, swSize, flags) => { + live += 1; peak = Math.max(peak, live); + asked.push(flags && flags.batched); + await new Promise(r => setTimeout(r, 20)); + live -= 1; + if (name === 'boom') throw new Error('refused'); + return { name }; +}; +let _targetQueue = Promise.resolve(); +let _targetsInFlight = 0; +""" + fn + """ +const results = await Promise.allSettled( + ['a', 'boom', 'c', 'd'].map(n => _openTargetInTurn(n))); +out.push(peak); +out.push(results.map(r => r.status).join(',')); +out.push(asked); +console.log(JSON.stringify(out)); +""") + proc = subprocess.run(["node", str(script)], capture_output=True, text=True) + assert proc.returncode == 0, proc.stderr + peak, statuses, batched = json.loads(proc.stdout) + assert peak == 1, f"{peak} targets were being opened at once" + assert batched == [False, True, True, True], ( + "only the first of a batch holds the user's gesture; the rest must be " + "opened without asking") + # And one refusal must not stop the rest: a chain that breaks on a rejection + # leaves every later download unable to open anything at all. + assert statuses == "fulfilled,rejected,fulfilled,fulfilled" diff --git a/packages/meshbay-hub/tests/test_memory_ceiling.py b/packages/meshbay-hub/tests/test_memory_ceiling.py index 8430627..d46cae3 100644 --- a/packages/meshbay-hub/tests/test_memory_ceiling.py +++ b/packages/meshbay-hub/tests/test_memory_ceiling.py @@ -66,12 +66,16 @@ def target_fn(): def _run(target_fn, tmp_path, *, size, native=False, granted=False, - streamed=False, picker=False, mode="auto"): + streamed=False, picker=False, mode="auto", batched=False): """Drive the real function against one browser shape.""" script = tmp_path / "case.mjs" script.write_text(f""" // Stubs for everything the lifted function reaches. `formatSize` and `t` only // build the message; the assertions are about which branch was taken. +// +// stdout carries the outcome and nothing else, so the function's own logging +// goes to stderr -- where it is still shown when a case fails. +console.info = (...a) => console.error(...a); const formatSize = (n) => `${{n}} B`; const t = (key, vars) => key + ' ' + JSON.stringify(vars); const platform = {{ @@ -107,7 +111,8 @@ if ({json.dumps(picker)}) {{ let outcome; try {{ - const r = await _openDownloadTarget('film.mkv', {size}); + const r = await _openDownloadTarget('film.mkv', {size}, {{}}, {size}, + {{ batched: {json.dumps(batched)} }}); outcome = r === null ? {{ kind: 'memory' }} : r === false ? {{ kind: 'cancelled' }} : {{ kind: 'stream', name: r.name }}; @@ -180,6 +185,50 @@ def test_a_browser_with_a_picker_is_offered_one_instead_of_being_refused( assert out == {"kind": "stream", "name": "p"} +# ── One dialog per gesture, not one per file ──────────────────────────────── + +def test_the_first_of_a_batch_still_asks_where_to_save(target_fn, tmp_path): + """The preference is not being taken away. Someone who asked to choose the + folder chooses it, for the download they actually clicked.""" + out = _run(target_fn, tmp_path, size=20 * GB, mode="ask", picker=True, + streamed=True) + assert out == {"kind": "stream", "name": "p"} + + +def test_the_rest_of_a_batch_stream_instead_of_asking(target_fn, tmp_path): + """A browser grants one picker per user gesture and selecting four files is + one gesture. Chrome showed the dialog for the second file anyway and then + waited for a human, so the third and fourth sat behind it until they timed + out — reported as three downloads frozen. + + There is no gesture left to spend, so nothing is lost by streaming: the file + still lands on disk, in the browser's own download folder. Only the choice + of folder goes, and it was not on offer. + """ + out = _run(target_fn, tmp_path, size=20 * GB, mode="ask", picker=True, + streamed=True, batched=True) + assert out == {"kind": "stream", "name": "s"} + + +def test_a_batched_download_falls_back_to_the_dialog_rather_than_failing( + target_fn, tmp_path): + """When the worker does not answer, asking is better than refusing: a + dialog that has to be answered is still a download, and the alternative + here is losing the file. A preference must not cost a capability, and + neither must the fix for one.""" + out = _run(target_fn, tmp_path, size=20 * GB, mode="ask", picker=True, + streamed=False, batched=True) + assert out == {"kind": "stream", "name": "p"} + + +def test_batching_never_pushes_a_large_file_into_memory(target_fn, tmp_path): + """Firefox shape — no picker at all. Nothing about the batch flag may reach + the memory floor above the ceiling.""" + out = _run(target_fn, tmp_path, size=20 * GB, mode="ask", picker=False, + streamed=False, batched=True) + assert out["kind"] == "refused", out + + # ── The one that outlives today's branches ────────────────────────────────── def test_no_unguarded_memory_floor(target_fn): diff --git a/packages/meshbay-hub/tests/test_transfers.py b/packages/meshbay-hub/tests/test_transfers.py index 7e21409..c48e38c 100644 --- a/packages/meshbay-hub/tests/test_transfers.py +++ b/packages/meshbay-hub/tests/test_transfers.py @@ -432,7 +432,9 @@ def test_the_slot_is_asked_for_after_there_is_somewhere_to_write(): src = (STATIC / "file-utils.js").read_text() fn = src[src.index("async function downloadEntry"):] fn = fn[:fn.index("\n}\n")] - assert fn.index("_openDownloadTarget") < fn.index("openTransfer"), ( + # `_openTargetInTurn` since target openings were serialised — same call, + # queued. What is pinned is that it comes before the slot is asked for. + assert fn.index("_openTargetInTurn") < fn.index("openTransfer"), ( "downloadEntry asks for a transfer slot before it has anywhere to " "write — the grant expires before the download can use it") diff --git a/packages/meshbay-hub/tests/test_zip_size_limit.py b/packages/meshbay-hub/tests/test_zip_size_limit.py index 28e3a0c..9471b8a 100644 --- a/packages/meshbay-hub/tests/test_zip_size_limit.py +++ b/packages/meshbay-hub/tests/test_zip_size_limit.py @@ -60,6 +60,10 @@ globalThis.localStorage = {{ // Node 22 defines `navigator` itself, so it is left alone; `window` is what // platform.js reaches for to decide it is not running in the desktop app. globalThis.window = globalThis; +// stdout carries the outcome and nothing else, so file-utils' own logging goes +// to stderr -- where it is still shown when a case fails. It logs before every +// save dialog, which is exactly what this harness provokes. +console.info = (...a) => console.error(...a); const out = {{ errors: [], started: 0, asked: 0, dropped: 0 }}; // Reached only once the size check has passed: with no File System Access API // under Node, downloadDirectory falls through to its build-in-memory path and -- cgit v1.2.3