diff --git a/CLAUDE.md b/CLAUDE.md index b7fbe9d..cdee4fa 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -76,7 +76,9 @@ The line-1 comment carries the entire Blockly workspace state. The rest of the f ## The dexie-observable gotcha -Pybricks wraps Dexie with `dexie-observable`, which records mutations in a hidden `_changes` table via Dexie's hook system. Our raw-IndexedDB writes bypass those hooks, so the running React UI does not see our changes. After applying a Pull, `content.js` does `location.reload()` — that's the only currently-working refresh path. +Pybricks wraps Dexie with `dexie-observable`, which records mutations in a hidden `_changes` table via Dexie's hook system. Our raw-IndexedDB writes bypass those hooks, so the running React UI does not see our changes. After applying a Pull, `content.js` does `location.reload()`. + +**The no-reload path: `write-files-live`.** A reload drops the hub's Bluetooth connection, so the menu panel's Save instead writes through Pybricks' own Redux store. `inject.js:findAppStore()` walks React's fiber tree from `#root` (`__reactContainer$…`) to the react-redux `` and only accepts a store whose state has `editor.openFileUuids` and `fileStorage.isInitialized === true`. `planLiveWrites()` then mirrors Pybricks' own Explorer import (`pybricks-code` `src/explorer/sagas.ts:importPythonFile`): a file open in an editor tab gets `{type: 'editor.action.replaceFile', uuid, value}`, which updates the open Monaco model in place with an undo stop and lets the app persist it; any other file gets `{type: 'fileStorage.action.writeFile', path, contents}`, a real Dexie write that `useLiveQuery` and dexie-observable both see. Files whose sha already matches get no action. `writeFilesLive()` then polls IndexedDB until every file's contents *and* sha256 match, and resolves `{live: false, reason}` when there's no store, when anything throws (hashing, IndexedDB, the app's reducers), or when nothing is confirmed within 5s. It never rejects. **Callers must fall back to `upsert-files` + reload on `live: false`**, so if an upstream change renames an action or moves the store, Save degrades to the old reload behaviour instead of losing data. Pybricks' editor and file-storage code is MIT-licensed on GitHub (`pybricks/pybricks-code`), so check the action shapes there. Hub downloads read imported modules (`menu_config.py` included) straight from IndexedDB (`pybricksMicropython/lib.ts:resolveModule`), so even the raw path was only unsafe for the open-tab and file-list cases. Pull, new-program and Update-robot-setup still use raw writes + reload. If this becomes painful, the fix paths are (in order of effort): bundle Dexie into the extension and write through it; reverse-engineer `_changes` row format and write directly; or expose Pybricks' Dexie instance via a hook into the page's React tree. None are necessary for the current prototype. @@ -89,7 +91,8 @@ The ISOLATED scripts reach IndexedDB by `window.postMessage`-ing `inject.js` (`p | `list-databases` | — | `indexedDB.databases()` result | discovery/debug helper | | `list-files` | — | `{metadata, contents}` | `contents` is `[{path, contents}]` with binary fields stripped | | `apply-files` | `{files}` | `{added, changed, deleted, unchanged}` | full-sync: adds/updates listed paths **and DELETES any IDB path not in `files`**. Used by Pull. **Never reuse for single-file writes.** | -| `upsert-files` | `{files}` | `{added, changed, deleted, unchanged}` | partial write: updates/inserts only the listed paths, **never deletes** (`deleted` is always 0). Used by the menu panel's Save. | +| `upsert-files` | `{files}` | `{added, changed, deleted, unchanged}` | partial write: updates/inserts only the listed paths, **never deletes** (`deleted` is always 0). Used by new-program, Update-robot-setup, and as the menu Save's fallback. | +| `write-files-live` | `{files}` | `{live: true, dispatched}` or `{live: false, reason}` | writes through the app's Redux store so the running UI (and an open editor tab) sees it with **no reload**; confirmed by IDB read-back. Used by the menu panel's Save. See "The dexie-observable gotcha". | `apply-files` and `upsert-files` are the same `writeFiles(files, deleteUnlisted)` with the delete pass toggled; both preserve each existing metadata row's `viewState`/`uuid` and only touch `sha256`/`contents`. @@ -130,7 +133,7 @@ The hub's on-device menu is driven by a `menu_config.py` file (a `MENU_ITEMS` li **This adds two top-level statements to the file**, which the starter repo's `check_project.py` does not yet allow — see "Starter-repo follow-ups" below. -- **`menu-panel.js` — `makeMenuPanel(deps)`.** A draggable floating panel listing the current slots (reorder by drag or ▲/▼, toggle `enabled`, remove, edit the hub display via a number/char/5×5-grid popover) and the addable programs. Position and open state persist under the **`menuPanel`** storage key (`{left, top, open}`); `content.js` reopens the panel after a reload when `open` was true. **Save always reloads.** Save regenerates the file and writes it via `upsert-files` (single-path, never deletes), then `location.reload()`s — because dexie-observable can't see our raw IDB write, and if `menu_config.py` happens to be open in Monaco the app's stale buffer would clobber our save on its next write; reloading discards that buffer. The panel resolves the config path and protected set from `lastPullManifest` (defaulting to `menu_config.py`); it intersects `protected` with the live `list-files` result and then **excludes** those protected files (and the config file itself) from the "Programs you can add" list. +- **`menu-panel.js` — `makeMenuPanel(deps)`.** A draggable floating panel listing the current slots (reorder by drag or ▲/▼, toggle `enabled`, remove, edit the hub display via a number/char/5×5-grid popover) and the addable programs. Position and open state persist under the **`menuPanel`** storage key (`{left, top, open}`); `content.js` reopens the panel after a reload when `open` was true. **Save does not reload** (a reload drops the hub's Bluetooth link). Save regenerates the file and writes it via `write-files-live`, which goes through the app's own store. If `menu_config.py` is open in an editor tab, its Monaco model is updated in place. On `live: true` the panel re-runs `loadState()` and shows `Saved ✓`. The slot controls stay live while a save is in flight, so every edit bumps an `editRevision` counter in `markDirty()`. Save re-checks the counter after every await, and again when the fallback's 800 ms reload timer fires. If it moved, the panel keeps the newer (still dirty) slots instead of refreshing over them or reloading them away, and asks for another Save. The fallback regenerates from the current slots right before its raw write. A `saving` flag keeps the re-rendered Save button disabled until the in-flight save settles, so two saves can't race. On `live: false` it falls back to the old path: `upsert-files` (single-path, never deletes), then `location.reload()`, because a raw write is invisible to the app and an open tab's stale buffer would clobber it on the next write. The panel resolves the config path and protected set from `lastPullManifest` (defaulting to `menu_config.py`); it intersects `protected` with the live `list-files` result and then **excludes** those protected files (and the config file itself) from the "Programs you can add" list. - **`file-list.js` — `makeFileListWatcher(deps)`.** A `MutationObserver` on `document.body` (debounced 250ms) that finds the page's file rows and (a) adds a 🔒 badge to protected files and (b) attaches right-click / long-press "Add to menu" gestures that call back into `menuPanel.addSlot`. **The selectors are documented in `test/e2e/file-list-dom.md`** — primary path is the Blueprint `[role="tree"][aria-label="Files"]` / `li[role="treeitem"]` / `span.bp5-tree-node-label` structure, with a scoped exact-text fallback. **Gated to the mounted Explorer:** it bails when neither `div.pb-activities-tabview` nor the tree is present, because the Explorer unmounts when closed (the default and the post-Pull-reload state) and without the gate every settled editor keystroke would trigger a `list-files` round-trip and a text-walk that could badge editor chrome. The badge is inserted as a **sibling after** the label (never inside it) so the label's `textContent` stays a clean path for the next decorate. `protected` here also comes from `lastPullManifest`. Consumers of `lastPullManifest.protected` (panel and watcher) **must intersect it with the live file list** before badging/hiding — a manifest can name paths the editor doesn't have. diff --git a/README.md b/README.md index cd41532..05a99c6 100644 --- a/README.md +++ b/README.md @@ -68,8 +68,7 @@ When the mentor updates the upstream shared repository, each team pulls the chan In rough priority order: -1. Avoid needing a page refresh, which breaks bluetooth connection to the Prime hub, when the menu is updated. -2. **Open-tab cleanup on delete** — when Pull deletes a file, also clean up its entry in Pybricks' "open tabs" state so the page doesn't log a non-fatal error after reload. +1. **Open-tab cleanup on delete** — when Pull deletes a file, also clean up its entry in Pybricks' "open tabs" state so the page doesn't log a non-fatal error after reload. ## License diff --git a/src/inject.js b/src/inject.js index 3cb53d3..a293cc1 100644 --- a/src/inject.js +++ b/src/inject.js @@ -31,6 +31,8 @@ async function handle(op, payload) { return await applyFiles(payload); case 'upsert-files': return await upsertFiles(payload); + case 'write-files-live': + return await writeFilesLive(payload); default: throw new Error(`unknown op: ${op}`); } @@ -166,6 +168,120 @@ async function writeFiles(files, deleteUnlisted) { } } +// --- Writing through the app (no reload) -------------------------------- +// +// Raw IDB writes (above) are invisible to the running app, so callers used to +// follow them with a page reload — which drops the hub's Bluetooth link. +// writeFilesLive instead asks Pybricks' own Redux store to do the write, the +// way its Explorer "Import file" does: a file open in an editor tab gets +// `editor.action.replaceFile` (the open Monaco model is updated in place, with +// an undo stop, and the app persists it), any other file gets +// `fileStorage.action.writeFile` (a Dexie write, so the file list and every +// dexie-observable subscriber see it). Action shapes are from pybricks-code +// src/editor/actions.ts + src/fileStorage/actions.ts. +// +// Everything here is best effort and self-verifying: it resolves +// {live: true} only once IndexedDB holds exactly the requested contents, and +// {live: false, reason} otherwise — store not found, app not initialized, or +// no confirmation within timeoutMs. On {live: false} the caller falls back +// to upsert-files + reload, so an upstream UI change degrades to the old +// behaviour instead of losing a save. + +const LIVE_WRITE_TIMEOUT_MS = 5000; + +// The app's Redux store, found by walking React's fiber tree from the root +// container down to the react-redux . Only a store whose +// state has the shape we rely on counts. Null when anything is missing. +function findAppStore(rootEl = document.getElementById('root')) { + if (!rootEl) return null; + const key = Object.keys(rootEl).find((k) => k.startsWith('__reactContainer$')); + if (!key) return null; + const stack = [rootEl[key]]; + // The Provider sits near the top of the tree; the cap only bounds a + // pathological walk if it ever moves or disappears. + for (let visited = 0; stack.length && visited < 5000; visited++) { + const fiber = stack.pop(); + if (!fiber) continue; + const props = fiber.memoizedProps; + const store = props && typeof props === 'object' ? props.store : null; + if (store && typeof store.dispatch === 'function' && typeof store.getState === 'function') { + const st = store.getState(); + if ( + st && st.editor && Array.isArray(st.editor.openFileUuids) && + st.fileStorage && st.fileStorage.isInitialized === true + ) { + return store; + } + } + if (fiber.sibling) stack.push(fiber.sibling); + if (fiber.child) stack.push(fiber.child); + } + return null; +} + +// Pure: which action writes each file. Files already holding the requested +// contents get none (no spurious undo stop in an open tab). +function planLiveWrites(files, metadata, openFileUuids) { + const metaByPath = new Map(metadata.map((m) => [m.path, m])); + const open = new Set(openFileUuids); + const actions = []; + for (const f of files) { + const existing = metaByPath.get(f.path); + if (existing && existing.sha256 === f.sha) continue; + if (existing && open.has(existing.uuid)) { + actions.push({ type: 'editor.action.replaceFile', uuid: existing.uuid, value: f.contents }); + } else { + actions.push({ type: 'fileStorage.action.writeFile', path: f.path, contents: f.contents }); + } + } + return actions; +} + +async function writeFilesLive({ files, timeoutMs = LIVE_WRITE_TIMEOUT_MS, rootEl } = {}) { + const store = findAppStore(rootEl); + if (!store) return { live: false, reason: 'Pybricks app store not found' }; + // Never reject: a throw from hashing, IDB, or the app's own reducers is + // just another reason to fall back. + try { + return await liveWriteAttempt(store, files, timeoutMs); + } catch (err) { + return { live: false, reason: `Pybricks could not save the file this way: ${err && err.message ? err.message : err}` }; + } +} + +async function liveWriteAttempt(store, files, timeoutMs) { + const wanted = await Promise.all( + files.map(async (f) => ({ path: f.path, contents: f.contents, sha: await sha256(f.contents) })), + ); + const before = await readStores(); + const actions = planLiveWrites(wanted, before.metadata, store.getState().editor.openFileUuids); + for (const action of actions) store.dispatch(action); + + const deadline = Date.now() + timeoutMs; + for (;;) { + const now = await readStores(); + const byPath = new Map(now.contents.map((c) => [c.path, c.contents])); + const shaByPath = new Map(now.metadata.map((m) => [m.path, m.sha256])); + const done = wanted.every( + (f) => byPath.get(f.path) === f.contents && shaByPath.get(f.path) === f.sha, + ); + if (done) return { live: true, dispatched: actions.length }; + if (Date.now() >= deadline) { + return { live: false, reason: 'Pybricks did not confirm the write in time' }; + } + await new Promise((resolve) => setTimeout(resolve, 100)); + } +} + +async function readStores() { + const db = await openPybricksDb(); + try { + return { metadata: await readAll(db, 'metadata'), contents: await readAll(db, '_contents') }; + } finally { + db.close(); + } +} + async function sha256(text) { const buf = new TextEncoder().encode(text); const hash = await crypto.subtle.digest('SHA-256', buf); diff --git a/src/menu-panel.js b/src/menu-panel.js index dfe585f..4511e2f 100644 --- a/src/menu-panel.js +++ b/src/menu-panel.js @@ -24,6 +24,15 @@ function makeMenuPanel(deps) { // Removes the popover plus its capture-phase window listeners; close() and // a reopen both route through it so nothing leaks (see openDisplayEditor). let displayEditorDismiss = null; + // Bumped by every slot edit (all of them go through markDirty). Save + // compares it across its awaits: the slot controls stay live while a save + // is in flight, and an edit made then must not be overwritten by the + // post-save refresh or discarded by the fallback reload. + let editRevision = 0; + // True from a Save click until it settles (and through a pending fallback + // reload). An edit mid-save re-renders the footer, and the Save button it + // draws must stay disabled so two saves can't race. + let saving = false; async function toggle() { if (panel) close(); @@ -308,9 +317,9 @@ function makeMenuPanel(deps) { const save = document.createElement('button'); save.dataset.pybricksGitSave = '1'; save.textContent = state.dirty ? 'Save menu' : 'Saved'; - save.disabled = !state.dirty; + save.disabled = !state.dirty || saving; styleMiniButton(save); - save.addEventListener('click', () => void saveConfig(save)); + save.addEventListener('click', () => void saveConfig()); if (state.teamSetup) { const newBtn = miniIconButton( '+ New program', @@ -680,6 +689,7 @@ function makeMenuPanel(deps) { } function markDirty() { + editRevision++; state.dirty = true; render(); } @@ -890,32 +900,107 @@ function makeMenuPanel(deps) { // --- save ------------------------------------------------------------ - // Save = regenerate the whole file and upsert ONLY that path. Always - // reload afterwards: dexie-observable can't see raw IDB writes, and if - // menu_config.py is open in Monaco a stale buffer would clobber this save - // on the app's next write. The persisted open flag reopens the panel. - async function saveConfig(saveBtn) { + // Save = regenerate the whole file and write ONLY that path. First choice + // is write-files-live: Pybricks' own store does the write (updating an open + // menu_config.py tab in place), so there's no page reload — a reload drops + // the hub's Bluetooth connection. If the app can't be driven (store not + // found, write not confirmed), fall back to the raw upsert + reload: with + // a raw write dexie-observable sees nothing, and an open menu_config.py tab + // would clobber the save on its next write. The persisted open flag + // reopens the panel after that reload. + async function saveConfig() { + if (saving) return; + saving = true; + let reloading = false; + try { + reloading = await doSave(); + } finally { + // A scheduled reload keeps Save disabled until the page goes away. + saving = reloading; + const btn = panel && panel.querySelector('[data-pybricks-git-save]'); + if (btn) btn.disabled = !state.dirty || saving; + } + } + + // Resolves true when it has scheduled the fallback reload. + async function doSave() { for (const [i, item] of state.items.entries()) { const problem = validateItem(item); if (problem) { setStatus(`Slot ${i + 1}: ${problem}`); - return; + return false; } } - saveBtn.disabled = true; setStatus('Saving…'); + const savedRevision = editRevision; + // Every await below is a window for a slot edit; re-check after each. + const changedSince = (rev) => { + if (editRevision === rev) return false; + render(); // the newer slots, still dirty, with Save enabled + setStatus('Saved — but the menu changed while saving. Save again to keep those changes.'); + return true; + }; + const files = [{ path: state.menuConfigPath, contents: generateMenuConfig(state.items) }]; + let live; + try { + live = await pageRequest('write-files-live', { files }); + } catch (err) { + live = { live: false, reason: err.message }; + } + if (live.live) { + // The earlier version is saved, but if the slots changed since, + // keep the newer edits instead of refreshing over them. + if (changedSince(savedRevision)) return false; + try { + const refreshed = await loadState(); + if (changedSince(savedRevision)) return false; + state = refreshed; + } catch (err) { + if (changedSince(savedRevision)) return false; + // The file is saved; only the panel refresh failed. Keep the + // edited slots but mark them clean. + console.warn('[pybricks-git] menu panel refresh after save failed:', err); + state.dirty = false; + state.banner = ''; + } + render(); + setStatus('Saved ✓'); + return false; + } + console.warn('[pybricks-git] live menu save unavailable, reloading instead:', live.reason); + // The fallback reload would discard any edit made while the live + // attempt ran, so write the slots as they are NOW. + for (const [i, item] of state.items.entries()) { + const problem = validateItem(item); + if (problem) { + setStatus(`Slot ${i + 1}: ${problem}`); + return false; + } + } + const fallbackRevision = editRevision; try { - const text = generateMenuConfig(state.items); await pageRequest('upsert-files', { - files: [{ path: state.menuConfigPath, contents: text }], + files: [{ path: state.menuConfigPath, contents: generateMenuConfig(state.items) }], }); + if (changedSince(fallbackRevision)) return false; await persist(true); + if (changedSince(fallbackRevision)) return false; setStatus('Saved ✓ — reloading…'); - setTimeout(() => reload(), 800); + // The pause lets the status be read; an edit made during it + // cancels the reload rather than being thrown away by it. + setTimeout(() => { + if (editRevision === fallbackRevision) { + reload(); + return; + } + saving = false; // reload cancelled: the newer edits need a Save + changedSince(fallbackRevision); + }, 800); + return true; } catch (err) { console.error('[pybricks-git] menu save failed:', err); setStatus(`Save failed: ${err.message}`); - saveBtn.disabled = false; + return false; } } diff --git a/test/e2e/README.md b/test/e2e/README.md index a0d693f..9616f60 100644 --- a/test/e2e/README.md +++ b/test/e2e/README.md @@ -213,12 +213,29 @@ plain `mission_01.py`, and a setup-only **blocks** file `arm_moves.py` — then: offers `[data-pybricks-git-add="arm_moves.lift_arm"]` while **excluding** the protected `menu.py` from the programs list. 4. Clicks that add button → slot count grows to **2** → clicks - `[data-pybricks-git-save]`, which rewrites `menu_config.py` via `upsert-files` - and reloads; asserts the panel **auto-reopens** from the persisted `open` flag. + `[data-pybricks-git-save]`, which rewrites `menu_config.py` through the app's + own store (`write-files-live`); asserts the status reads `Saved ✓` and the page + did **not** reload (same isolated context, a window marker survives). 5. Reads `list-files` from the isolated world and asserts the regenerated `menu_config.py` parses to 2 items with the second being `{display:2, module:"arm_moves", function:"lift_arm", blocks:true}` — and that the raw text contains `"module": "arm_moves", "function": "lift_arm", "blocks": True`. +5b. Opens `menu_config.py` in an editor tab (dispatching `editor.action.activateFile` + on the app's store from the MAIN world), adds a third slot and Saves again — + still no reload — then types `# kid edit` at the end of the open tab and + waits for Pybricks to persist it. The persisted file must still hold all + **3** slots: a stale Monaco model would write back the 2-slot copy. (Verified + by breaking the open-tab branch of `planLiveWrites`: this assertion fails.) +5c. Clicks Save and a slot's ▲ in the same synchronous tick, so the move lands + while the write is in flight. Asserts Save reports "changed while saving", + stays enabled, the panel keeps the move, the file holds the pre-move + version, and a second Save persists the move. (Verified by disabling the + revision check in `saveConfig`: this step fails.) +5d. Forces the **fallback** by setting the MAIN-world `findAppStore = () => null`. + A Save then schedules its reload, and a slot move during the 800 ms pause must + **cancel** it and ask for another Save. A clean fallback Save must reload + (the panel reopens from the `open` flag) and persist the newest slots. (Verified + by making the pause always reload: this step fails.) 6. **Commit**s (trusted-typed message, Enter) → asserts the label reaches `✓ ↑`, then harness-side: the pushed `menu_config.py` carries the `arm_moves` line and `menu.py` is **byte-identical to the seed** (protection @@ -241,10 +258,26 @@ Recorded from a real passing run (Chromium 1228): [e2e-menu] PASS: programs list offers add button for arm_moves.lift_arm [e2e-menu] PASS: no add button for protected menu.py (excluded from programs) [e2e-menu] PASS: adding arm_moves.lift_arm grows slots to 2 -[e2e-menu] PASS: panel auto-reopened after Save reload (persisted open flag) +[e2e-menu] PASS: Save finished without a reload (status "Saved ✓") +[e2e-menu] PASS: the page did not reload after Save +[e2e-menu] PASS: Save button reads "Saved" (panel state refreshed from the write) [e2e-menu] PASS: menu_config.py contains the arm_moves slot line ("module": "arm_moves", "function": "lift_arm", "blocks": True) [e2e-menu] PASS: menu_config.py parses to exactly 2 items [e2e-menu] PASS: second item = {display:2, module:arm_moves, function:lift_arm, blocks:true} +[e2e-menu] PASS: menu_config.py is open in an editor tab +[e2e-menu] PASS: Save with the tab open finished without a reload (status "Saved ✓") +[e2e-menu] PASS: the page did not reload after the second Save +[e2e-menu] PASS: menu_config.py holds 3 slots after the second Save ({"error":null,"len":3}) +[e2e-menu] PASS: typing in the open tab kept all 3 saved slots ({"error":null,"len":3}) +[e2e-menu] PASS: Save reports that the menu changed while saving +[e2e-menu] PASS: Save stays enabled for the unsaved move +[e2e-menu] PASS: the mid-save move is still in the panel (slot 2 = "≡3mission_01 (whole program)▲▼✕") +[e2e-menu] PASS: the file holds the version saved before the move (toggle yes, move no) +[e2e-menu] PASS: the second Save persisted the move (…) +[e2e-menu] PASS: still no reload after the in-flight edit and second Save +[e2e-menu] PASS: an edit during the pause cancelled the fallback reload +[e2e-menu] PASS: the cancelled reload asks for another Save (…) +[e2e-menu] PASS: the fallback Save persisted the newest slots, incl. the mid-pause move (…) [e2e-menu] PASS: commit label shows "✓ ↑" (got "✓ c95a7d9 ↑") [e2e-menu] PASS: pushed menu_config.py contains the arm_moves slot line [e2e-menu] PASS: protected menu.py is byte-identical to the seed (protection held end-to-end) @@ -253,7 +286,11 @@ Recorded from a real passing run (Chromium 1228): ``` `menu-panel.png` (committed alongside this README) is the screenshot after the -push, with the menu panel reopened over the editor. +push. It is taken after step 5d's clean fallback Save reloaded the page, so it +shows the panel reopened from the persisted `open` flag over the restored +`menu_config.py` tab. That tab holds the regenerated file: 3 slots, the first and +third disabled. The typed `# kid edit` is gone, because Save rewrites the whole +file and comments aren't kept. # `drive-splice.mjs` — phase-4 setup-splice round-trip diff --git a/test/e2e/drive-menu.mjs b/test/e2e/drive-menu.mjs index 977d95f..ed72b39 100644 --- a/test/e2e/drive-menu.mjs +++ b/test/e2e/drive-menu.mjs @@ -3,7 +3,8 @@ // Self-contained sibling of drive.mjs: starts the in-repo git HTTP harness // (Task 2), launches Playwright's Chromium with the unpacked extension, and // drives the real Pull → open Menu panel → add a program slot → Save (rewrites -// menu_config.py via upsert-files) → Commit flow on https://code.pybricks.com +// menu_config.py via write-files-live, no reload — also with the file open in +// an editor tab) → Commit flow on https://code.pybricks.com // over raw CDP (Node 22's built-in WebSocket, no npm deps). Asserts on the // browser side (panel DOM, slot counts, add buttons), the editor IndexedDB // (regenerated menu_config.py), the SW storage (lastPullManifest), and the @@ -568,8 +569,12 @@ async function main() { 'no add button for protected menu.py (excluded from programs)', ); - // -- Add a slot, then Save ------------------------------------------ - step(6, 'Add arm_moves.lift_arm; Save (rewrites menu_config.py) + reload'); + // -- Add a slot, then Save (no reload) ------------------------------ + // Save writes through Pybricks' own store (write-files-live), so the + // page must NOT reload — a reload drops the hub's Bluetooth link. A + // marker on the isolated world's window proves the same document + // survived; a reload would wipe it along with the context. + step(6, 'Add arm_moves.lift_arm; Save (rewrites menu_config.py) with no reload'); await clickSelector( '[data-pybricks-git-add="arm_moves.lift_arm"]', 'arm_moves.lift_arm add button', @@ -580,24 +585,30 @@ async function main() { ); assert(slotCount1 === 2, 'adding arm_moves.lift_arm grows slots to 2'); + await evalIsolated(`window.__pbgitNoReload = 1`, false); + const ctxBeforeSave = isolatedCtx; await clickSelector('[data-pybricks-git-save]', 'Save button'); - log('Save clicked; waiting for reload...'); - await poll(() => isolatedCtx === null, { - timeout: 15000, - what: 'reload to clear isolated context after Save', - }).catch(() => log('note: did not observe context clear (may have raced)')); - await poll(async () => (await buttonRect('Pull')) != null, { - timeout: 40000, - what: 'buttons to remount after Save reload', - }); - // The persisted open flag reopens the panel on load, unattended. - await poll(() => exists('[data-pybricks-git-panel]'), { - timeout: 15000, - what: 'menu panel to auto-reopen after reload', - }); + const saveStatus = await poll( + () => + evalIsolated( + `(() => { const s = document.querySelector('[data-pybricks-git-status]'); return s && /^Saved|failed/.test(s.textContent) ? s.textContent : null; })()`, + false, + ), + { timeout: 15000, interval: 100, what: 'Save status' }, + ); + log('save status =', JSON.stringify(saveStatus)); + assert(saveStatus === 'Saved ✓', `Save finished without a reload (status "${saveStatus}")`); + await sleep(1500); // a fallback reload fires 800ms after the status assert( - await exists('[data-pybricks-git-panel]'), - 'panel auto-reopened after Save reload (persisted open flag)', + isolatedCtx === ctxBeforeSave && (await evalIsolated(`window.__pbgitNoReload === 1`, false)), + 'the page did not reload after Save', + ); + assert( + await evalIsolated( + `document.querySelector('[data-pybricks-git-save]').textContent === 'Saved'`, + false, + ), + 'Save button reads "Saved" (panel state refreshed from the write)', ); // -- Verify the regenerated menu_config.py in the editor IDB -------- @@ -630,6 +641,204 @@ async function main() { 'second item = {display:2, module:arm_moves, function:lift_arm, blocks:true}', ); + // -- Save while menu_config.py is open in an editor tab -------------- + // The dangerous case the old reload guarded against: an open Monaco + // model holds its own copy of the file and writes it back on the next + // keystroke. The live save must update that model in place, so typing + // in the tab afterwards keeps BOTH the new slot and the kid's edit. + step('7b', 'Save with menu_config.py open in the editor, then type in it'); + const evalMain = async (expression) => { + const r = await page.send('Runtime.evaluate', { + expression, + awaitPromise: true, + returnByValue: true, + }); + if (r.exceptionDetails) { + throw new Error('main eval threw: ' + (r.exceptionDetails.exception?.description || r.exceptionDetails.text)); + } + return r.result.value; + }; + const configUuid = listing.metadata.find((m) => m.path === 'menu_config.py').uuid; + // Open the tab the way the Explorer does: editor.action.activateFile on + // the app's own store (findAppStore is inject.js's, in the MAIN world). + await evalMain( + `findAppStore().dispatch({ type: 'editor.action.activateFile', uuid: ${JSON.stringify(configUuid)} })`, + ); + await poll( + () => + evalMain( + `findAppStore().getState().editor.openFileUuids.includes(${JSON.stringify(configUuid)}) && !!document.querySelector('.monaco-editor .view-lines')`, + ), + { timeout: 15000, what: 'menu_config.py to open in an editor tab' }, + ); + assert(true, 'menu_config.py is open in an editor tab'); + + await clickSelector('[data-pybricks-git-add="mission_01.run"], [data-pybricks-git-add="mission_01"]', 'a mission_01 add button'); + await poll( + async () => ((await count('[data-pybricks-git-slot]')) === 3 ? 3 : null), + { timeout: 10000, what: 'slot count to grow to 3' }, + ); + await clickSelector('[data-pybricks-git-save]', 'Save button (tab open)'); + const saveStatus2 = await poll( + () => + evalIsolated( + `(() => { const s = document.querySelector('[data-pybricks-git-status]'); return s && /^Saved|failed/.test(s.textContent) ? s.textContent : null; })()`, + false, + ), + { timeout: 15000, interval: 100, what: 'Save status (tab open)' }, + ); + assert(saveStatus2 === 'Saved ✓', `Save with the tab open finished without a reload (status "${saveStatus2}")`); + await sleep(1500); + assert(isolatedCtx === ctxBeforeSave, 'the page did not reload after the second Save'); + + const saved3 = (await evalIsolated(`pageRequest('list-files')`)).contents.find( + (c) => c.path === 'menu_config.py', + ).contents; + const parsed3 = await evalIsolated( + `(() => { const p = parseMenuConfig(${JSON.stringify(saved3)}); return { error: p.error, len: p.items && p.items.length }; })()`, + false, + ); + assert(parsed3.error === null && parsed3.len === 3, `menu_config.py holds 3 slots after the second Save (${JSON.stringify(parsed3)})`); + + // Type at the end of the open tab; Pybricks persists the model. If the + // model were stale this write would drop the third slot. + await clickSelector('.monaco-editor .view-lines', 'the Monaco editor'); + for (const type of ['keyDown', 'keyUp']) { + await page.send('Input.dispatchKeyEvent', { + type, + modifiers: 2, // Ctrl + windowsVirtualKeyCode: 35, + key: 'End', + code: 'End', + }); + } + await page.send('Input.insertText', { text: '\n# kid edit\n' }); + const afterTyping = await poll( + async () => { + const c = (await evalIsolated(`pageRequest('list-files')`)).contents.find( + (x) => x.path === 'menu_config.py', + ).contents; + return c.includes('# kid edit') ? c : null; + }, + { timeout: 15000, interval: 250, what: "the kid's edit to be persisted by Pybricks" }, + ); + const parsedAfter = await evalIsolated( + `(() => { const p = parseMenuConfig(${JSON.stringify(afterTyping)}); return { error: p.error, len: p.items && p.items.length }; })()`, + false, + ); + assert( + parsedAfter.error === null && parsedAfter.len === 3, + `typing in the open tab kept all 3 saved slots (${JSON.stringify(parsedAfter)})`, + ); + + // -- An edit made while a save is in flight is kept ---------------- + // Save and a slot move in the same synchronous tick: saveConfig runs up + // to its first await (the write-files-live round-trip), then the move + // lands mid-save. The earlier version is written, the move must stay in + // the panel (still unsaved), and a second Save must persist it. + step('7c', 'A slot move made while Save is in flight is kept, not overwritten'); + await evalIsolated( + `document.querySelector('[data-pybricks-git-slot="2"] [data-pybricks-git-slot-enabled]').click()`, + false, + ); + await evalIsolated( + `(() => { document.querySelector('[data-pybricks-git-save]').click(); document.querySelector('[data-pybricks-git-slot="2"] [data-pybricks-git-slot-up]').click(); })()`, + false, + ); + const raceStatus = await poll( + () => + evalIsolated( + `(() => { const s = document.querySelector('[data-pybricks-git-status]'); return s && /^Saved|failed/.test(s.textContent) ? s.textContent : null; })()`, + false, + ), + { timeout: 15000, interval: 100, what: 'Save status (edit in flight)' }, + ); + log('in-flight save status =', JSON.stringify(raceStatus)); + assert(/changed while saving/.test(raceStatus), 'Save reports that the menu changed while saving'); + const raceUi = await evalIsolated( + `(() => ({ saveEnabled: !document.querySelector('[data-pybricks-git-save]').disabled, slot1: document.querySelector('[data-pybricks-git-slot="1"]').textContent }))()`, + false, + ); + assert(raceUi.saveEnabled, 'Save stays enabled for the unsaved move'); + assert(/mission_01 \(whole program\)/.test(raceUi.slot1), `the mid-save move is still in the panel (slot 2 = ${JSON.stringify(raceUi.slot1)})`); + const midItems = await evalIsolated( + `pageRequest('list-files').then((l) => parseMenuConfig(l.contents.find((c) => c.path === 'menu_config.py').contents).items)`, + ); + assert( + midItems[1].module === 'arm_moves' && midItems[2].enabled === false, + 'the file holds the version saved before the move (toggle yes, move no)', + ); + await clickSelector('[data-pybricks-git-save]', 'Save button (second save)'); + await poll( + () => + evalIsolated( + `document.querySelector('[data-pybricks-git-status]')?.textContent === 'Saved ✓'`, + false, + ), + { timeout: 15000, interval: 100, what: 'second Save to finish' }, + ); + const finalItems = await evalIsolated( + `pageRequest('list-files').then((l) => parseMenuConfig(l.contents.find((c) => c.path === 'menu_config.py').contents).items)`, + ); + assert( + finalItems.length === 3 && finalItems[1].module === 'mission_01' && !finalItems[1].function && finalItems[1].enabled === false, + `the second Save persisted the move (${JSON.stringify(finalItems)})`, + ); + assert(isolatedCtx === ctxBeforeSave, 'still no reload after the in-flight edit and second Save'); + + // -- Fallback path: no store → raw write + reload ------------------ + // Hide the app store (inject.js's findAppStore is a MAIN-world global) + // so write-files-live resolves {live:false} and Save falls back. First + // an edit during the 800ms pre-reload pause must cancel the reload; + // then a clean fallback Save must reload and persist the latest slots. + step('7d', 'Fallback Save: an edit during the reload pause cancels it; a clean one reloads'); + await evalMain(`findAppStore = () => null`); + await evalIsolated( + `document.querySelector('[data-pybricks-git-slot="0"] [data-pybricks-git-slot-enabled]').click()`, + false, + ); + await clickSelector('[data-pybricks-git-save]', 'Save button (fallback)'); + await poll( + () => + evalIsolated( + `/reloading/.test(document.querySelector('[data-pybricks-git-status]')?.textContent || '')`, + false, + ), + { timeout: 15000, interval: 50, what: 'fallback Save to schedule its reload' }, + ); + await evalIsolated( + `document.querySelector('[data-pybricks-git-slot="2"] [data-pybricks-git-slot-up]').click()`, + false, + ); + await sleep(1500); + assert(isolatedCtx === ctxBeforeSave, 'an edit during the pause cancelled the fallback reload'); + const pauseUi = await evalIsolated( + `(() => ({ status: document.querySelector('[data-pybricks-git-status]').textContent, saveEnabled: !document.querySelector('[data-pybricks-git-save]').disabled }))()`, + false, + ); + assert( + /changed while saving/.test(pauseUi.status) && pauseUi.saveEnabled, + `the cancelled reload asks for another Save (${JSON.stringify(pauseUi)})`, + ); + await clickSelector('[data-pybricks-git-save]', 'Save button (fallback, clean)'); + await poll(() => isolatedCtx === null || isolatedCtx !== ctxBeforeSave, { + timeout: 15000, + what: 'the fallback Save to reload the page', + }); + await poll(() => exists('[data-pybricks-git-panel]'), { + timeout: 40000, + what: 'menu panel to reopen after the fallback reload', + }); + const fallbackItems = await evalIsolated( + `pageRequest('list-files').then((l) => parseMenuConfig(l.contents.find((c) => c.path === 'menu_config.py').contents).items)`, + ); + assert( + fallbackItems.length === 3 && + fallbackItems[0].enabled === false && + fallbackItems[1].module === 'arm_moves', + `the fallback Save persisted the newest slots, incl. the mid-pause move (${JSON.stringify(fallbackItems)})`, + ); + // -- Commit --------------------------------------------------------- step(8, 'Commit; assert the push landed menu_config.py, menu.py untouched'); const commitPt = await buttonRect('Commit'); diff --git a/test/e2e/menu-panel.png b/test/e2e/menu-panel.png index 099230f..8bfd6ba 100644 Binary files a/test/e2e/menu-panel.png and b/test/e2e/menu-panel.png differ diff --git a/test/inject.test.mjs b/test/inject.test.mjs index 784f193..e2a1641 100644 --- a/test/inject.test.mjs +++ b/test/inject.test.mjs @@ -11,7 +11,7 @@ import { createHash } from 'node:crypto'; import { IDBFactory } from 'fake-indexeddb'; import { loadInject } from './load-inject.mjs'; -const { applyFiles, upsertFiles, sha256 } = loadInject(); +const { applyFiles, upsertFiles, sha256, findAppStore, planLiveWrites, writeFilesLive } = loadInject(); // Reference SHA-256 hex, computed independently of the code under test. const hexSha = (s) => createHash('sha256').update(s, 'utf8').digest('hex'); @@ -314,3 +314,181 @@ test('discovers the Pybricks DB by its store names, not its name', async () => { const summary = await applyFiles({ files: [{ path: 'a.py', contents: 'a\n' }] }); assert.equal(summary.added, 1); }); + +// --- writeFilesLive: writing through the app's Redux store --- + +// A fake React root: #root carries a __reactContainer$ fiber whose subtree +// holds the react-redux Provider (memoizedProps.store) a few levels down, +// behind a sibling, with a text fiber (string props) on the way. +function fakeRoot(store) { + const provider = { memoizedProps: { store, children: {} }, child: null, sibling: null }; + const textNode = { memoizedProps: 'hello', child: null, sibling: provider }; + const app = { memoizedProps: {}, child: textNode, sibling: null }; + return { '__reactContainer$abc123': { memoizedProps: null, child: app, sibling: null } }; +} + +// A fake store that behaves like Pybricks' sagas: replaceFile/writeFile end +// in a Dexie write, done here with upsertFiles (after a tick, as a saga would). +function fakeStore({ openFileUuids = [], initialized = true, onDispatch } = {}) { + const dispatched = []; + return { + dispatched, + getState: () => ({ editor: { openFileUuids }, fileStorage: { isInitialized: initialized } }), + dispatch(action) { + dispatched.push(action); + if (onDispatch) onDispatch(action); + }, + }; +} + +async function actLikePybricks(action, db) { + await new Promise((r) => setTimeout(r, 20)); + if (action.type === 'fileStorage.action.writeFile') { + await upsertFiles({ files: [{ path: action.path, contents: action.contents }] }); + } else if (action.type === 'editor.action.replaceFile') { + const meta = await getAll(db, 'metadata'); + const row = meta.find((m) => m.uuid === action.uuid); + await upsertFiles({ files: [{ path: row.path, contents: action.value }] }); + } +} + +describe('findAppStore', () => { + test('finds the Provider store in the fiber tree', () => { + const store = fakeStore(); + assert.equal(findAppStore(fakeRoot(store)), store); + }); + + test('returns null without a React container, or with an unexpected state shape', () => { + assert.equal(findAppStore(null), null); + assert.equal(findAppStore({}), null); + const odd = { getState: () => ({ other: 1 }), dispatch() {} }; + assert.equal(findAppStore(fakeRoot(odd)), null); + }); + + test('ignores the store until file storage is initialized', () => { + assert.equal(findAppStore(fakeRoot(fakeStore({ initialized: false }))), null); + }); +}); + +describe('planLiveWrites', () => { + const meta = [ + { path: 'menu_config.py', uuid: 'u-menu', sha256: 'old' }, + { path: 'open.py', uuid: 'u-open', sha256: 'old' }, + { path: 'same.py', uuid: 'u-same', sha256: 'same' }, + ]; + + test('open files go through the editor, others through file storage, unchanged ones not at all', () => { + const actions = planLiveWrites( + [ + { path: 'menu_config.py', contents: 'M', sha: 'new' }, + { path: 'open.py', contents: 'O', sha: 'new' }, + { path: 'same.py', contents: 'S', sha: 'same' }, + { path: 'fresh.py', contents: 'F', sha: 'new' }, + ], + meta, + ['u-open', 'u-same'], + ); + assert.deepEqual(actions, [ + { type: 'fileStorage.action.writeFile', path: 'menu_config.py', contents: 'M' }, + { type: 'editor.action.replaceFile', uuid: 'u-open', value: 'O' }, + { type: 'fileStorage.action.writeFile', path: 'fresh.py', contents: 'F' }, + ]); + }); +}); + +describe('writeFilesLive', () => { + test('writes a closed file through fileStorage and confirms it in IndexedDB', async () => { + const db = await openPybricks(); + await seed(db, [{ path: 'menu_config.py', contents: 'old\n', uuid: 'u-menu', viewState: { top: 3 } }]); + const store = fakeStore({ onDispatch: (a) => actLikePybricks(a, db) }); + const res = await writeFilesLive({ + files: [{ path: 'menu_config.py', contents: 'new\n' }], + rootEl: fakeRoot(store), + }); + assert.deepEqual(res, { live: true, dispatched: 1 }); + assert.equal(store.dispatched[0].type, 'fileStorage.action.writeFile'); + const snap = await snapshot(db); + assert.equal(snap.byPath['menu_config.py'], 'new\n'); + assert.deepEqual(snap.metaByPath['menu_config.py'].viewState, { top: 3 }); + }); + + test('replaces an open file through the editor', async () => { + const db = await openPybricks(); + await seed(db, [{ path: 'menu_config.py', contents: 'old\n', uuid: 'u-menu' }]); + const store = fakeStore({ openFileUuids: ['u-menu'], onDispatch: (a) => actLikePybricks(a, db) }); + const res = await writeFilesLive({ + files: [{ path: 'menu_config.py', contents: 'new\n' }], + rootEl: fakeRoot(store), + }); + assert.equal(res.live, true); + assert.deepEqual(store.dispatched, [{ type: 'editor.action.replaceFile', uuid: 'u-menu', value: 'new\n' }]); + }); + + test('creates a missing file through fileStorage', async () => { + const db = await openPybricks(); + const store = fakeStore({ onDispatch: (a) => actLikePybricks(a, db) }); + const res = await writeFilesLive({ + files: [{ path: 'menu_config.py', contents: 'new\n' }], + rootEl: fakeRoot(store), + }); + assert.equal(res.live, true); + assert.equal((await snapshot(db)).byPath['menu_config.py'], 'new\n'); + }); + + test('an unchanged file dispatches nothing and still confirms', async () => { + const db = await openPybricks(); + await seed(db, [{ path: 'menu_config.py', contents: 'same\n', uuid: 'u-menu' }]); + const store = fakeStore({ openFileUuids: ['u-menu'] }); + const res = await writeFilesLive({ + files: [{ path: 'menu_config.py', contents: 'same\n' }], + rootEl: fakeRoot(store), + }); + assert.deepEqual(res, { live: true, dispatched: 0 }); + assert.deepEqual(store.dispatched, []); + }); + + test('reports live:false when the app never writes (so the caller can fall back)', async () => { + const db = await openPybricks(); + await seed(db, [{ path: 'menu_config.py', contents: 'old\n', uuid: 'u-menu' }]); + const store = fakeStore(); // swallows the action, as a renamed action type would + const res = await writeFilesLive({ + files: [{ path: 'menu_config.py', contents: 'new\n' }], + rootEl: fakeRoot(store), + timeoutMs: 300, + }); + assert.equal(res.live, false); + assert.match(res.reason, /did not confirm/); + assert.equal((await snapshot(db)).byPath['menu_config.py'], 'old\n', 'nothing written behind the app'); + }); + + test('a throwing dispatch resolves live:false instead of rejecting', async () => { + const db = await openPybricks(); + await seed(db, [{ path: 'menu_config.py', contents: 'old\n', uuid: 'u-menu' }]); + const store = fakeStore({ + onDispatch: () => { + throw new Error('reducer exploded'); + }, + }); + const res = await writeFilesLive({ + files: [{ path: 'menu_config.py', contents: 'new\n' }], + rootEl: fakeRoot(store), + }); + assert.equal(res.live, false); + assert.match(res.reason, /could not save the file this way: reducer exploded/); + }); + + test('a missing Pybricks database resolves live:false instead of rejecting', async () => { + // Fresh IDBFactory from beforeEach: no DB with metadata/_contents exists. + const res = await writeFilesLive({ + files: [{ path: 'menu_config.py', contents: 'new\n' }], + rootEl: fakeRoot(fakeStore()), + }); + assert.equal(res.live, false); + assert.match(res.reason, /no Pybricks IndexedDB found/); + }); + + test('reports live:false without touching IndexedDB when no store is found', async () => { + const res = await writeFilesLive({ files: [{ path: 'a.py', contents: 'a' }], rootEl: {} }); + assert.deepEqual(res, { live: false, reason: 'Pybricks app store not found' }); + }); +}); diff --git a/test/load-inject.mjs b/test/load-inject.mjs index 74736da..8f0f784 100644 --- a/test/load-inject.mjs +++ b/test/load-inject.mjs @@ -21,7 +21,7 @@ export function loadInject() { } const src = readFileSync(injectPath, 'utf8') + - '\n;globalThis.__pybricksGitTest = { applyFiles, upsertFiles, sha256, listFiles, openPybricksDb };'; + '\n;globalThis.__pybricksGitTest = { applyFiles, upsertFiles, sha256, listFiles, openPybricksDb, findAppStore, planLiveWrites, writeFilesLive };'; // eslint-disable-next-line no-new-func new Function(src)(); return globalThis.__pybricksGitTest;