From c22f46909f3c0bb41f6fa60cb33deba07ddc1a78 Mon Sep 17 00:00:00 2001 From: Brendon Thiede Date: Fri, 18 Sep 2026 16:59:06 -0400 Subject: [PATCH 1/3] feat: Pull, New program and Update robot setup skip the reload A page reload drops the hub's Bluetooth connection. Every editor write now goes through Pybricks' own Redux store (write-files-live), with the raw write + reload kept only as the fallback. write-files-live gains deleteUnlisted (Pull's full sync, same semantics as apply-files) and mirrors Pybricks' Explorer: deletions close the file's tab first, then fileStorage.deleteFile; an open block program is closed, written, and reopened (never replaced under a live Blockly workspace); open text files are still replaced in place. It returns the apply-files summary and verifies deletions in its read-back. Pull renders the rescue notice immediately and refreshes the menu panel (keeping unsaved slot edits) and the file-list badges in place. New program and Update robot setup refresh the panel and show the splice report without reloading. E2E: all three drivers assert no reload; drive.mjs checks the open tabs (deleted tab closed, changed text tab shows the pulled text) and forces the fallback (step 7a); drive-splice.mjs opens the block program Update rewrites and checks it comes back as the active tab. Negative controls verified for the in-place text replace and the fallback tab prune. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 26 +++-- README.md | 2 +- src/content.js | 65 ++++++++++--- src/file-list.js | 22 ++++- src/inject.js | 176 +++++++++++++++++++++++++-------- src/menu-panel.js | 74 +++++++++++--- test/e2e/README.md | 83 +++++++++++----- test/e2e/drive-menu.mjs | 19 ++-- test/e2e/drive-splice.mjs | 86 ++++++++++++++--- test/e2e/drive.mjs | 135 +++++++++++++++++--------- test/inject.test.mjs | 198 ++++++++++++++++++++++++++++++++------ 11 files changed, 687 insertions(+), 199 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 2e70c03..3cf9f96 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -27,7 +27,7 @@ Everything runs in the browser; the only external party is `github.com`: **The ISOLATED world is six classic scripts**, listed in `manifest.json` `content_scripts` in load order: `menu-config.js` (pure parse/generate/analyze helpers, no DOM), `blocksplice.js` (pure block-file setup splicing, no DOM — phase 4), `pullmerge.js` (pure Pull-merge planning, no DOM — see "Pull merges, it does not clobber" below), then `menu-panel.js` (`makeMenuPanel`) and `file-list.js` (`makeFileListWatcher`) which depend on those helpers being in scope, then `content.js` last — it wires the toolbar and constructs the panel/watcher. They share one global scope with no ESM `export`s, so **order matters** — a helper must be defined in an earlier file than its caller. The three pure-helper files have Node test shims (`test/load-menu-config.mjs`, `test/load-blocksplice.mjs`, `test/load-pullmerge.mjs`, same pattern as `load-inject.mjs`) and unit tests; the three DOM-heavy files have no Node loader and are exercised only via the browser E2E path. See the "Menu manager (phase 3)" and "Setup propagation (phase 4)" sections below. -**Pull merges, it does not clobber.** `content.js:pull()` never hands the repo's file set straight to `apply-files` — that op deletes every path it isn't given, which used to destroy uncommitted local work. `src/pullmerge.js:planPull()` (pure, no DOM, unit-tested via `test/load-pullmerge.mjs`) computes the complete desired set from the editor's files, the repo's files, the `lastPullShas` base, and the protected list. Files untouched since the last Pull take the repo's version; locally edited files let the repo keep the canonical path and are rescued to `_mine.py` (a valid module name, so the hub can still import it); files created locally and never committed keep their own name; protected paths **that the repo actually has** are overwritten with no rescue copy. Protection is checked after the never-committed rule on purpose — a manifest can reserve a path the repo doesn't have (the starter reserves `robot_setup.py`, never authored), and with no repo version to restore, "the repo wins" would just delete the file the kid made. Rescues are reported through the `pullRescued` key as a notice on the next load. **Open tabs of deleted files are forgotten.** Pybricks remembers open editor tabs as a JSON array of file uuids in sessionStorage, under `editor.activeFileHistory..` (`ActiveFileHistoryManager`, pybricks-code `src/editor/lib.ts`), and reopens them after a reload. A uuid whose file the Pull deleted raises Pybricks' "unexpected error … file with uuid '…' not found" toast. So `pull()` computes the doomed uuids before `apply-files` (`pullmerge.js:deletedUuids`) and prunes them from every such key (`planTabPrunes` plans the rewrites purely; `content.js` does the sessionStorage reads and writes) **inside the reload timer, immediately before `location.reload()`**. Earlier would be undone, because the page's in-memory history rewrites the key whenever a tab opens or closes. Design: `docs/superpowers/specs/2026-08-09-pull-merge-design.md`. +**Pull merges, it does not clobber.** `content.js:pull()` never hands the repo's file set straight to `apply-files` — that op deletes every path it isn't given, which used to destroy uncommitted local work. `src/pullmerge.js:planPull()` (pure, no DOM, unit-tested via `test/load-pullmerge.mjs`) computes the complete desired set from the editor's files, the repo's files, the `lastPullShas` base, and the protected list. Files untouched since the last Pull take the repo's version; locally edited files let the repo keep the canonical path and are rescued to `_mine.py` (a valid module name, so the hub can still import it); files created locally and never committed keep their own name; protected paths **that the repo actually has** are overwritten with no rescue copy. Protection is checked after the never-committed rule on purpose — a manifest can reserve a path the repo doesn't have (the starter reserves `robot_setup.py`, never authored), and with no repo version to restore, "the repo wins" would just delete the file the kid made. Rescues are reported as a notice: immediately after a live Pull, or through the `pullRescued` key on the next load when Pull fell back to a reload. The apply itself goes through `write-files-live` with `deleteUnlisted` (no reload); `apply-files` + reload is the fallback. **Open tabs of deleted files are forgotten.** Pybricks remembers open editor tabs as a JSON array of file uuids in sessionStorage, under `editor.activeFileHistory..` (`ActiveFileHistoryManager`, pybricks-code `src/editor/lib.ts`), and reopens them after a reload. A uuid whose file the Pull deleted raises Pybricks' "unexpected error … file with uuid '…' not found" toast. A live Pull avoids this, because it closes those tabs through the app. On the raw fallback, `pull()` computes the doomed uuids before `apply-files` (`pullmerge.js:deletedUuids`) and prunes them from every such key (`planTabPrunes` plans the rewrites purely; `content.js` does the sessionStorage reads and writes) **inside the reload timer, immediately before `location.reload()`**. Earlier would be undone, because the page's in-memory history rewrites the key whenever a tab opens or closes. Design: `docs/superpowers/specs/2026-08-09-pull-merge-design.md`. **Why an ISOLATED/MAIN split:** The MAIN-world script (`inject.js`) can see the page's globals (and could in principle use the page's Dexie instance, though we don't); the ISOLATED-world scripts have access to `chrome.runtime` APIs so they can message the service worker. The split is mandatory — only the MAIN world reaches the page's IndexedDB, only the ISOLATED world reaches `chrome.runtime`. They communicate via `window.postMessage` with `pybricks-git:request` / `pybricks-git:response` envelopes; `content.js` reaches the service worker via `chrome.runtime.sendMessage`. @@ -47,8 +47,8 @@ Everything runs in the browser; the only external party is `github.com`: **Settings** live in `chrome.storage.local` under the key `settings`: `{repoUrl, branch, token, name, email, login}`, set via the action popup (`src/popup.html` / `popup.js`). `token` is either a GitHub OAuth token (from **Sign in with GitHub**, Device Flow) or a fine-grained PAT pasted under the popup's **Advanced** section. `login` is the GitHub login, set by OAuth sign-in and empty for pasted PATs. `email` is derived from the login (`@users.noreply.github.com`, or `team@users.noreply.github.com` when unknown) during sign-in or **Test connection**, not typed. `branch` defaults to `main`. **Sign out** clears `token`/`email`/`login` but keeps `repoUrl`/`branch`/`name`. The engine and UI keep more keys under `chrome.storage.local`: -- `lastPullShas` — `{path: sha256}` over the `.py` files the last non-empty Pull returned: the base state the editor and the repo last agreed on. **Written by `content.js`, not the engine, and only after `apply-files` resolves** — the base may not claim agreement the editor doesn't hold. `pullOp` returns the map as `shas` (null with no head) and stores nothing; if the apply throws, the base stays where it was, and the next Commit's untouched-skip guard still protects a teammate's push. Also written, after a successful push, by Commit — which folds in just what it wrote (overlaid paths get the new sha, deleted paths drop the entry, guard-skipped and diverged-protected paths keep their old one, since the editor and the repo don't agree on those). Replaces the old `lastPullPaths` (whose keys it subsumes; `commitOp` still falls back to that key for installs that haven't pulled since the upgrade). Read by `planPull` to tell a locally edited file from an upstream-changed one, and by `commitOp` to skip payload files the editor never touched. -- `pullRescued` — `[{path, savedAs}]`, written by a Pull that rescued local edits and rendered as a notice on the next page load, then cleared. +- `lastPullShas` — `{path: sha256}` over the `.py` files the last non-empty Pull returned: the base state the editor and the repo last agreed on. **Written by `content.js`, not the engine, and only once the editor holds the files** (a live Pull's read-back confirmed, or `apply-files` resolved) — the base may not claim agreement the editor doesn't hold. `pullOp` returns the map as `shas` (null with no head) and stores nothing; if the apply throws, the base stays where it was, and the next Commit's untouched-skip guard still protects a teammate's push. Also written, after a successful push, by Commit — which folds in just what it wrote (overlaid paths get the new sha, deleted paths drop the entry, guard-skipped and diverged-protected paths keep their old one, since the editor and the repo don't agree on those). Replaces the old `lastPullPaths` (whose keys it subsumes; `commitOp` still falls back to that key for installs that haven't pulled since the upgrade). Read by `planPull` to tell a locally edited file from an upstream-changed one, and by `commitOp` to skip payload files the editor never touched. +- `pullRescued` — `[{path, savedAs}]`, written by a Pull that rescued local edits **and fell back to a reload**, rendered as a notice on the next page load, then cleared. A live Pull renders the notice immediately and never writes the key. - `lastPullManifest` — `{protected, menuConfig, setupTemplate, teamSetup}`, written by every non-empty Pull (see "The git engine", "Menu manager", "Setup propagation"). `protected` is a plain array of paths; consumers **must intersect it with the live file list** before badging/hiding, because a manifest can name paths that don't exist in the editor. `teamSetup`/`setupTemplate` are file names (or null) from `.pybricks-git.json`; a null `teamSetup` hides the whole phase-4 new-program/propagate feature. - `menuPanel` — the floating panel's persisted `{left, top, open}` (see "Menu manager"). - `spliceReport` — `{when, updated:[paths], skipped:[{path, reason}]}`, written by an Update-robot-setup run and rendered as the dismissable report block on the next panel open; dismiss sets it to null (see "Setup propagation"). @@ -76,9 +76,15 @@ 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()`. +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, and the only refresh for a raw write is `location.reload()`, which drops the hub's Bluetooth connection. So every editor write now goes through `write-files-live` first, and raw writes + reload are only the fallback. -**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. +**The no-reload path: `write-files-live`.** Menu Save, Pull, New program and Update robot setup all write 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()` (pure, unit-tested) then mirrors Pybricks' own Explorer (`pybricks-code` `src/explorer/sagas.ts`: `importPythonFile` for writes, `handleExplorerDeleteFile` for deletes): +- a **text** 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 write gets `{type: 'fileStorage.action.writeFile', path, contents}`, a real Dexie write that `useLiveQuery` and dexie-observable both see; +- with `deleteUnlisted` (Pull's full sync, the same semantics as `apply-files`), every other file gets `{type: 'fileStorage.action.deleteFile', path}`, after its tab (if open) is closed with `editor.action.closeFile`. That's the Explorer's order: deleting an open file fails as "in use", and closing also drops it from the remembered tabs. The close's own save can't race the delete, because both are Dexie read-write transactions on the same stores, which IndexedDB runs in creation order; +- a **block** program open in a tab (old or new contents start with `# pybricks blocks file:`) is **closed, written through `fileStorage`, then reopened**, one tab at a time with the originally active tab last. The block editor keeps its own Blockly workspace, and nothing shows it reloads when the model is replaced underneath it; a stale workspace would later write the old program back. Close and reopen leaves it exactly where a page reload would. This isn't proven against a live Blockly workspace: that needs a licensed run of `drive-splice.mjs`, and without a licence the block editor never goes live. Without a licence, reopening a block tab brings back Pybricks' "Enable block coding" dialog, just as a reload restoring that tab did. + +Files whose sha already matches get no action. `writeFilesLive()` then polls IndexedDB until every file's contents *and* sha256 match and every deleted path is gone, and resolves `{live: false, reason}` when there's no store, when anything throws (hashing, IndexedDB, the app's reducers), or when a tab won't close or nothing is confirmed within 5s. Closed block tabs are reopened either way (in a `finally`). It returns the same `{added, changed, deleted, unchanged}` summary as `apply-files`, and never rejects. **Callers must fall back to the raw write + reload on `live: false`** (`upsert-files`, or `apply-files` for Pull), so if an upstream change renames an action or moves the store, each feature 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. After a live write, whatever a reload used to refresh is refreshed in place. Pull renders the rescue notice at once (`renderRescueNotice`) and calls `menuPanel.refresh()` (re-reads files, manifest and splice report, keeping unsaved slot edits) and `fileListWatcher.refresh()` (re-reads the manifest and removes badges from files that are no longer protected). 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. @@ -92,7 +98,7 @@ The ISOLATED scripts reach IndexedDB by `window.postMessage`-ing `inject.js` (`p | `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 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". | +| `write-files-live` | `{files, deleteUnlisted?}` | `{live: true, dispatched, summary}` or `{live: false, reason}` | writes (and, with `deleteUnlisted`, deletes like `apply-files`) through the app's Redux store, so the running UI and open editor tabs see it with **no reload**; confirmed by IDB read-back. Used by Pull, menu Save, New program and Update robot setup. 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`. @@ -134,7 +140,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 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`. +- **`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 state, and the state after a reload) 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. @@ -149,12 +155,12 @@ Teams share one robot setup (the `blockGlobalSetup` chain — hub, motors, drive - `setupSignature(contents) → {signature, error}` — a canonical **string** (`JSON.stringify`) of the setup chain with block/shadow **ids and canvas x/y stripped** and `VAR` id-refs resolved to `{name,type}`. Two setups are "the same" iff their signatures are `===`. This is what the nudge and the splice compare on. - `spliceSetup(target, template) → {contents, changed, error}` — replace `target`'s setup chain with `template`'s, **remapping variable ids by name**: template-only devices are ADDED; a device the target has in setup but the template lacks → **skip** (`error`, "its own device"); a name/type mismatch or id collision → **skip**. `changed:false` when the signatures already match (no-op). - `newProgramContents(teamSetupContents) → {contents, error}` — graft the team's setup chain onto the editor-authored **empty-program scaffold** (so the kid gets a `blockGlobalStart` to program under; a verbatim setup-only copy has none — see blocks-format.md). -- **New program from team setup** (`menu-panel.js` footer button `[data-pybricks-git-new-program]` + `file-list.js` context entry `[data-pybricks-git-context-item="new-program"]`, shown on every row incl. protected). Seeds a new `.py` via `newProgramContents` → `upsert-files`. Fully hidden when `lastPullManifest.teamSetup` is null. Name-checked against every editor path + the reserved config/setup/protected names. +- **New program from team setup** (`menu-panel.js` footer button `[data-pybricks-git-new-program]` + `file-list.js` context entry `[data-pybricks-git-context-item="new-program"]`, shown on every row incl. protected). Seeds a new `.py` via `newProgramContents` → `write-files-live` (fallback: `upsert-files` + reload), then refreshes the panel in place. Fully hidden when `lastPullManifest.teamSetup` is null. Name-checked against every editor path + the reserved config/setup/protected names. - **Update robot setup** (`menu-panel.js` footer button `[data-pybricks-git-update-setup]`, shown only when ≥1 block program's signature differs from the team setup's — those rows get a `[data-pybricks-git-setup-differs]` ⚠ nudge). The propagate flow, whose **safety rails are non-negotiable**: 1. **Snapshot first.** Commit the entire editor tree as `Before robot setup update` (via the `commit` op, which pushes). Any throw ABORTS with nothing changed; a `committed:false` "no changes" is fine and proceeds. **No editor file is mutated until the snapshot resolves** — this is the whole safety story. 2. Splice each eligible target (block program that isn't the team setup / setup template / menuConfig / protected). Collect `updated` (`changed && !error`) and `skipped` (`{path, reason}`); a no-op is neither. - 3. `upsert-files` the updated files (never `apply-files`), persist a `spliceReport`, and reload. Only-skips → inline report, no reload; nothing at all → "All programs already match." - 4. On reload the panel renders a dismissable `[data-pybricks-git-splice-report]` (updated list + per-skip reason) from the persisted report. + 3. Write the updated files through `write-files-live` (never a delete; fallback: `upsert-files` + reload), and persist a `spliceReport`. A spliced block program that's open in a tab is closed, written and reopened (see "The dexie-observable gotcha"). Only-skips → inline report; nothing at all → "All programs already match." + 4. The panel renders a dismissable `[data-pybricks-git-splice-report]` (updated list + per-skip reason) from the persisted report: in place via `refresh()` on the live path, or after the reload on the fallback. Protected files and the team-setup/template files themselves are **never** spliced. The committed E2E `test/e2e/drive-splice.mjs` exercises the whole round-trip (nudge → new program → snapshot-first update → editor regeneration → Commit). diff --git a/README.md b/README.md index a20b117..fe94efc 100644 --- a/README.md +++ b/README.md @@ -59,7 +59,7 @@ When the mentor updates the upstream shared repository, each team pulls the chan ## Known limitations -- **The page reloads after a Pull that changes files.** Pybricks wraps Dexie with `dexie-observable`, and the extension's raw IndexedDB writes bypass its hook system, so React doesn't see them until a reload. +- **The page can still reload in rare cases.** Pull, menu Save, New program and Update robot setup write through the Pybricks app itself, so the page doesn't reload and the hub stays connected over Bluetooth. If a future code.pybricks.com update stops that from working, the extension falls back to writing the files directly and reloading the page (which drops the Bluetooth connection) rather than risk losing work. - **The credential is stored in `chrome.storage.local`.** Whether you sign in with GitHub (an OAuth token with the `public_repo` scope) or paste a PAT, it lands in `chrome.storage.local` — device-local, but readable by anyone who can use that Chrome profile. The OAuth token can be revoked any time at GitHub → *Settings → Applications*; a pasted PAT should be scoped to the single fork with Contents-only write, as in Setup. - **A Commit made before the first Pull preserves unknown files rather than deleting them.** Since the extension has no snapshot of what the fork contained, it won't delete starter code it has never seen. This is by design; the preserved paths are logged to the console. - **Block programming on code.pybricks.com requires a Pybricks licence.** This is upstream's pricing, not something this extension controls or can unlock. Opening a block file raises an **"Enable block coding"** dialog offering a licence code, a Patreon subscription, or a self-serve **7-day trial**; teachers can email `sales@pybricks.com` for a **free 30-day class trial**. Licences are per-user, so a team that builds in blocks needs coverage for the machines its students work on — worth budgeting for before a season starts. **Plain Python programs are unaffected and always free.** Pybricks Git itself needs no licence either way: it treats block files as opaque text, so Pull, Commit, and protected files work on an unlicensed machine. What a licence buys is the ability to *open and edit* those blocks in the editor — which the shared-robot-setup features assume, since students edit the spliced programs as blocks. diff --git a/src/content.js b/src/content.js index 04a027c..78e466f 100644 --- a/src/content.js +++ b/src/content.js @@ -375,14 +375,19 @@ function showErrorPanel(opName, err) { document.body.appendChild(box); } -// Kid-facing report of what Pull rescued. Rendered on the page load *after* -// the pull's reload, because that's when the rescued files are actually -// visible in the file list. Click, Escape, or the timeout dismisses it. +// Kid-facing report of what Pull rescued. A live Pull renders it at once +// (renderRescueNotice); a Pull that fell back to a reload persists it under +// pullRescued and it renders on the next load, when the rescued files are +// actually visible. Click, Escape, or the timeout dismisses it. async function showRescueNotice() { const rescued = await storageGet('pullRescued'); if (!rescued || !rescued.length) return; await storageSet({ pullRescued: [] }); + renderRescueNotice(rescued); +} +function renderRescueNotice(rescued) { + document.querySelector('[data-pybricks-git-rescue]')?.remove(); const box = document.createElement('div'); box.dataset.pybricksGitRescue = '1'; box.setAttribute('role', 'status'); @@ -478,18 +483,52 @@ async function pull(btn) { console.warn('[pybricks-git] rescued local edits:', plan.rescued); } - // The files apply-files is about to delete — their uuids must also - // leave Pybricks' open-tab history, or the reload tries to reopen them. - const goneUuids = deletedUuids(editor.metadata, plan.files.map((f) => f.path)); - const summary = await pageRequest('apply-files', { files: plan.files }); - console.log('[pybricks-git] applied:', summary); + // First choice: sync through Pybricks' own store (write-files-live with + // deleteUnlisted — the same full-sync semantics as apply-files), so the + // page never reloads and the hub's Bluetooth link survives. It closes + // the tabs of deleted files itself. Any doubt → the raw apply-files + + // reload below, exactly as before. + let live; + try { + live = await pageRequest('write-files-live', { files: plan.files, deleteUnlisted: true }); + } catch (err) { + live = { live: false, reason: err.message }; + } + let summary; + let goneUuids = []; + if (live.live) { + summary = live.summary; + } else { + console.warn('[pybricks-git] live Pull unavailable, reloading instead:', live.reason); + // The files apply-files is about to delete — their uuids must also + // leave Pybricks' open-tab history, or the reload tries to reopen them. + goneUuids = deletedUuids(editor.metadata, plan.files.map((f) => f.path)); + summary = await pageRequest('apply-files', { files: plan.files }); + } + console.log('[pybricks-git] applied:', summary, live.live ? '(live)' : '(raw)'); btn.textContent = `↓ +${summary.added} ~${summary.changed} -${summary.deleted}`; - // Both keys are written only after apply-files resolves. The base must - // never claim agreement the editor doesn't hold: if the apply throws, - // the editor is still on the old files, and an advanced base would let - // the next Commit push them over whatever the repo now has. A stale - // pullRescued would likewise render a false notice on the next load. + // Written only once the editor holds the files (live: confirmed by + // read-back; raw: after apply-files resolves). The base must never + // claim agreement the editor doesn't hold: if the apply throws, the + // editor is still on the old files, and an advanced base would let the + // next Commit push them over whatever the repo now has. await storageSet({ lastPullShas: result.shas }); + + if (live.live) { + // Nothing reloads, so everything a reload used to refresh is + // refreshed here: the rescue notice shows now, and the panel and + // file list pick up the new files and the new manifest. + if (plan.rescued.length) renderRescueNotice(plan.rescued); + await Promise.all([ + menuPanel.refresh().catch((err) => console.warn('[pybricks-git] panel refresh failed:', err)), + fileListWatcher.refresh().catch((err) => console.warn('[pybricks-git] file-list refresh failed:', err)), + ]); + setTimeout(() => (btn.textContent = original), 3000); + return; + } + + // A stale pullRescued would render a false notice on the next load, + // so it too waits for apply-files. if (plan.rescued.length) await storageSet({ pullRescued: plan.rescued }); // dexie-observable doesn't see raw IDB writes, so reload to refresh diff --git a/src/file-list.js b/src/file-list.js index 5feaf46..542303a 100644 --- a/src/file-list.js +++ b/src/file-list.js @@ -13,15 +13,33 @@ function makeFileListWatcher(deps) { let teamSetup = null; let debounceTimer = null; - async function start() { + async function loadManifest() { const manifest = await storageGet('lastPullManifest'); protectedPaths = new Set((manifest && manifest.protected) || []); teamSetup = (manifest && manifest.teamSetup) || null; + } + + async function start() { + await loadManifest(); const observer = new MutationObserver(scheduleDecorate); observer.observe(document.body, { childList: true, subtree: true }); scheduleDecorate(); } + // Re-reads the manifest after a Pull that didn't reload the page (a reload + // used to restart the watcher). Badges on files that are no longer + // protected come off; decorate() adds the new ones. + async function refresh() { + await loadManifest(); + for (const badge of document.querySelectorAll('[data-pybricks-git-badge]')) { + const row = badge.closest('li[role="treeitem"]') || badge.parentNode; + const label = row && row.querySelector('span.bp5-tree-node-label'); + const path = label ? label.textContent : null; + if (!path || !protectedPaths.has(path)) badge.remove(); + } + scheduleDecorate(); + } + function scheduleDecorate() { clearTimeout(debounceTimer); debounceTimer = setTimeout(() => void decorate().catch(() => {}), 250); @@ -226,5 +244,5 @@ function makeFileListWatcher(deps) { document.body.appendChild(menu); } - return { start }; + return { start, refresh }; } diff --git a/src/inject.js b/src/inject.js index a293cc1..662b2bb 100644 --- a/src/inject.js +++ b/src/inject.js @@ -172,22 +172,38 @@ async function writeFiles(files, deleteUnlisted) { // // 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. +// writeFilesLive instead asks Pybricks' own Redux store to do the work, the +// way its Explorer does (pybricks-code src/explorer/sagas.ts: importPythonFile +// for writes, handleExplorerDeleteFile for deletes): +// - a text 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 write gets `fileStorage.action.writeFile` (a Dexie write, so +// the file list and every dexie-observable subscriber see it); +// - a delete gets `fileStorage.action.deleteFile`, after closing its tab +// with `editor.action.closeFile` (the Explorer's order — deleting an open +// file fails as "in use", and closing also drops it from the remembered +// tabs); +// - a BLOCK program open in a tab is closed, written, then reopened. The +// block editor keeps its own Blockly workspace, and nothing shows it +// reloads when the model is replaced underneath it; a stale workspace +// would later write the old program back. Close + reopen leaves it exactly +// where a page reload would. +// 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. +// {live: true, summary} only once IndexedDB holds exactly the requested +// contents (and none of the deleted paths), and {live: false, reason} +// otherwise — store not found, app not initialized, anything thrown, or no +// confirmation in time. It never rejects. On {live: false} the caller falls +// back to the raw write + reload, so an upstream UI change degrades to the +// old behaviour instead of losing data. const LIVE_WRITE_TIMEOUT_MS = 5000; +const BLOCKS_SENTINEL = '# pybricks blocks file:'; + +const isBlocksFile = (text) => typeof text === 'string' && text.startsWith(BLOCKS_SENTINEL); // 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 @@ -219,57 +235,139 @@ function findAppStore(rootEl = document.getElementById('root')) { 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])); +// Pure: how to bring the editor to `files` through the app. +// files [{path, contents, sha}] the desired contents +// before {metadata, contents} the editor's IndexedDB now +// openFileUuids [uuid] tabs open in the editor, in order +// deleteUnlisted true → also delete every file not +// in `files` (Pull's full sync) +// Returns {close, writes, deletes, reopen, summary}: `close` = tab uuids to +// close first; `writes`/`deletes` = actions to dispatch; `reopen` = closed +// block-program tabs to reopen afterwards (open-tab order); `summary` = the +// same counts apply-files/upsert-files report. Files already holding the +// requested contents get no action (no spurious undo stop in an open tab). +function planLiveWrites({ files, before, openFileUuids, deleteUnlisted = false }) { + const metaByPath = new Map(before.metadata.map((m) => [m.path, m])); + const oldContents = new Map(before.contents.map((c) => [c.path, c.contents])); const open = new Set(openFileUuids); - const actions = []; + const close = new Set(); + const reopen = new Set(); + const writes = []; + const deletes = []; + const summary = { added: 0, changed: 0, deleted: 0, unchanged: 0 }; + const wanted = new Set(); for (const f of files) { + wanted.add(f.path); 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 }); + if (!existing) { + summary.added++; + writes.push({ type: 'fileStorage.action.writeFile', path: f.path, contents: f.contents }); + continue; + } + if (existing.sha256 === f.sha) { + summary.unchanged++; + continue; + } + summary.changed++; + if (!open.has(existing.uuid)) { + writes.push({ type: 'fileStorage.action.writeFile', path: f.path, contents: f.contents }); + } else if (isBlocksFile(oldContents.get(f.path)) || isBlocksFile(f.contents)) { + close.add(existing.uuid); + reopen.add(existing.uuid); + writes.push({ type: 'fileStorage.action.writeFile', path: f.path, contents: f.contents }); } else { - actions.push({ type: 'fileStorage.action.writeFile', path: f.path, contents: f.contents }); + writes.push({ type: 'editor.action.replaceFile', uuid: existing.uuid, value: f.contents }); + } + } + if (deleteUnlisted) { + for (const m of before.metadata) { + if (wanted.has(m.path)) continue; + summary.deleted++; + if (open.has(m.uuid)) close.add(m.uuid); + deletes.push({ type: 'fileStorage.action.deleteFile', path: m.path }); } } - return actions; + return { + close: openFileUuids.filter((u) => close.has(u)), + writes, + deletes, + reopen: openFileUuids.filter((u) => reopen.has(u)), + summary, + }; } -async function writeFilesLive({ files, timeoutMs = LIVE_WRITE_TIMEOUT_MS, rootEl } = {}) { +async function writeFilesLive({ files, deleteUnlisted = false, 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); + return await liveWriteAttempt(store, files, deleteUnlisted, 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) { +// Polls `check` every 100ms until it returns true or the deadline passes. +async function waitUntil(check, deadline) { + for (;;) { + if (await check()) return true; + if (Date.now() >= deadline) return false; + await new Promise((resolve) => setTimeout(resolve, 100)); + } +} + +async function liveWriteAttempt(store, files, deleteUnlisted, 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 { editor } = store.getState(); + const plan = planLiveWrites({ files: wanted, before, openFileUuids: editor.openFileUuids, deleteUnlisted }); + const openNow = () => store.getState().editor.openFileUuids; + try { + for (const uuid of plan.close) store.dispatch({ type: 'editor.action.closeFile', uuid }); + const closed = await waitUntil( + () => plan.close.every((u) => !openNow().includes(u)), + Date.now() + timeoutMs, + ); + if (!closed) return { live: false, reason: 'Pybricks did not close the affected tabs in time' }; - 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, + for (const action of [...plan.writes, ...plan.deletes]) store.dispatch(action); + + const gone = plan.deletes.map((a) => a.path); + const confirmed = await waitUntil(async () => { + 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])); + return ( + wanted.every((f) => byPath.get(f.path) === f.contents && shaByPath.get(f.path) === f.sha) && + gone.every((p) => !byPath.has(p) && !shaByPath.has(p)) + ); + }, Date.now() + timeoutMs); + if (!confirmed) return { live: false, reason: 'Pybricks did not confirm the write in time' }; + return { + live: true, + dispatched: plan.writes.length + plan.deletes.length, + summary: plan.summary, + }; + } finally { + // Best effort, success or not: bring back the block tabs we closed, + // one at a time so the originally active file ends up active again. + await reopenTabs(store, plan.reopen, editor.activeFileUuid, timeoutMs); + } +} + +async function reopenTabs(store, uuids, activeUuid, timeoutMs) { + const order = uuids.includes(activeUuid) + ? [...uuids.filter((u) => u !== activeUuid), activeUuid] + : uuids; + for (const uuid of order) { + store.dispatch({ type: 'editor.action.activateFile', uuid }); + await waitUntil( + () => store.getState().editor.openFileUuids.includes(uuid), + Date.now() + timeoutMs, ); - 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)); } } diff --git a/src/menu-panel.js b/src/menu-panel.js index 4511e2f..6551216 100644 --- a/src/menu-panel.js +++ b/src/menu-panel.js @@ -429,10 +429,14 @@ function makeMenuPanel(deps) { } setStatus('Creating…'); try { - await pageRequest('upsert-files', { files: [{ path, contents: seed.contents }] }); - await persist(true); - setStatus(`Created ${path} — reloading…`); - setTimeout(() => reload(), 800); + const { reloading } = await writeOrFallback([{ path, contents: seed.contents }]); + if (reloading) { + setStatus(`Created ${path} — reloading…`); + setTimeout(() => reload(), 800); + return; + } + await refresh(); + setStatus(`Created ${path} ✓`); } catch (err) { setStatus(`Couldn't create it: ${err.message}`); } @@ -489,11 +493,12 @@ function makeMenuPanel(deps) { if (res.error) { skipped.push({ path: p.path, reason: res.error }); continue; } if (res.changed) updated.push({ path: p.path, contents: res.contents }); } + let reloading = false; if (updated.length) { try { - await pageRequest('upsert-files', { - files: updated.map((u) => ({ path: u.path, contents: u.contents })), - }); + ({ reloading } = await writeOrFallback( + updated.map((u) => ({ path: u.path, contents: u.contents })), + )); } catch (err) { setStatus(`Saved the snapshot, but couldn't write the updates: ${err.message}`); if (btn) btn.disabled = false; @@ -507,13 +512,18 @@ function makeMenuPanel(deps) { } const report = { when: new Date().toISOString(), updated: updated.map((u) => u.path), skipped }; await storageSet({ spliceReport: report }); - if (updated.length) { - // Editor IDB changed under dexie-observable's back — reload so the - // app rebuilds from our write (same rule as Save/new-program). The - // report renders after the reload from the persisted spliceReport. - await persist(true); + if (reloading) { + // The raw fallback wrote under dexie-observable's back — reload so + // the app rebuilds from it. The report renders after the reload + // from the persisted spliceReport. setStatus(`Updated ${updated.length} program(s)… reloading`); setTimeout(() => reload(), 800); + } else if (updated.length) { + // Written through the app: refresh in place. refresh() re-reads the + // persisted spliceReport, so the report block shows right away. + await refresh(); + setStatus(`Updated ${updated.length} program(s) ✓`); + if (btn) btn.disabled = false; } else { // Only skips — nothing was written, so no reload. Show the report // inline so the kid sees why each program was left alone. @@ -898,6 +908,44 @@ function makeMenuPanel(deps) { displayEditorDismiss = dismiss; } + // --- refresh + live writes ------------------------------------------- + + // Re-reads files, manifest and splice report into an open panel — what a + // page reload used to do after a Pull or a setup change. Unsaved slot + // edits survive: `state.dirty` is read AFTER the await, so an edit made + // while loading is kept too. Skipped mid-save; saveConfig reloads state + // itself when it finishes. + async function refresh() { + if (!panel || saving) return; + const fresh = await loadState(); + if (!panel || saving) return; + if (state && state.dirty) { + fresh.items = state.items; + fresh.dirty = true; + fresh.banner = state.banner; + } + state = fresh; + render(); + } + + // Writes `files` through the app (write-files-live: no reload, the hub's + // Bluetooth link survives). If the app can't be driven, falls back to the + // raw upsert-files and resolves {reloading: true}: the caller must then + // schedule the reload, because the raw write is invisible to the app. + async function writeOrFallback(files) { + let live; + try { + live = await pageRequest('write-files-live', { files }); + } catch (err) { + live = { live: false, reason: err.message }; + } + if (live.live) return { reloading: false }; + console.warn('[pybricks-git] live write unavailable, reloading instead:', live.reason); + await pageRequest('upsert-files', { files }); + await persist(true); + return { reloading: true }; + } + // --- save ------------------------------------------------------------ // Save = regenerate the whole file and write ONLY that path. First choice @@ -1016,5 +1064,5 @@ function makeMenuPanel(deps) { showNewProgramRow(); } - return { toggle, open, close, isOpen, addSlot, newProgram }; + return { toggle, open, close, isOpen, addSlot, newProgram, refresh }; } diff --git a/test/e2e/README.md b/test/e2e/README.md index b25c20b..2b425ca 100644 --- a/test/e2e/README.md +++ b/test/e2e/README.md @@ -46,9 +46,12 @@ have to rediscover them: center returns the overlay `DIV`, not the `BUTTON`). The driver dismisses the tour (clicks its `data-action="close"` button, Escape fallback) before driving the toolbar. This is environmental — not an extension bug. -- **Pull triggers a reload.** After a non-empty apply, `content.js` schedules - `location.reload()` ~1.5s later; execution contexts are torn down and rebuilt, - so the driver re-enumerates the isolated world after the reload. +- **Pull normally does NOT reload.** It writes through Pybricks' own store + (`write-files-live`), and the drivers assert that the page survived (same + isolated-context id, a `window` marker still set). Only the raw fallback + (forced in step 7a by nulling the MAIN-world `findAppStore`) schedules + `location.reload()` ~1.5s after the label. Execution contexts are then torn + down and rebuilt, so the driver re-enumerates the isolated world. ### What the driver does (maps to acceptance steps) @@ -69,8 +72,8 @@ have to rediscover them: and captures `Runtime.exceptionThrown` (tagged `page:`) from attach onward. 5. Waits for the toolbar buttons, dismisses the Welcome Tour, then **Pull** (trusted click) → asserts label `↓ +4 ~0 -0` (the four seeded `.py` files; - the manifest isn't `.py`, so it never reaches the editor) → waits for - reload → `pageRequest('list-files')` contains `starter.py`. + the manifest isn't `.py`, so it never reaches the editor) → asserts the + page did **not** reload → `pageRequest('list-files')` contains `starter.py`. 6. Seeds a second file (`e2e.py`) via `pageRequest('apply-files', …)`, real-clicks **Commit**, trusted-types `e2e message`, trusted Enter → asserts the label timeline `Committing…` → `✓ ↑`. @@ -81,23 +84,31 @@ have to rediscover them: `coach.py`), pushes a competing commit to the bare repo (`starter.py`/`keep.py`/`coach.py` changed, `gone.py` deleted), opens `keep.py` and `gone.py` as editor tabs (`editor.action.activateFile` on the - app's store), real-clicks **Pull** again → asserts label `↓ +1 ~3 -1` → waits for reload → asserts the - rescue notice names `starter.py`/`starter_mine.py` and says nothing about the + app's store), real-clicks **Pull** again → asserts label `↓ +1 ~3 -1` → asserts the + rescue notice (rendered immediately, no reload) names `starter.py`/`starter_mine.py` and says nothing about the untouched `keep.py` or the protected `coach.py` → asserts the post-merge IndexedDB: `scratch.py` (never committed) survives untouched, `starter.py` holds the repo's competing version, the local edit is rescued to `starter_mine.py`, `keep.py` silently took the upstream version with no `keep_mine.py` sibling, `gone.py` is gone, and `coach.py` took the repo's version with **no** `coach_mine.py` — protection overwrites, never rescues. - **Open-tab cleanup:** once Pybricks has reopened its remembered tabs, asserts - no "file with uuid '…' not found" toast was shown (toasts are recorded from - page load, since they auto-dismiss after 5s), that `gone.py` left the - sessionStorage tab history, and that `keep.py` is still in it. (Verified by - disabling the prune in `content.js`: the toast assertion fails.) + **Tabs, live:** the page did not reload; Pybricks closed `gone.py`'s tab + itself; `keep.py` is still open and its Monaco view shows the pulled + `keep v2` (the model was replaced in place, not left stale; verified by + sending open text tabs through `fileStorage.writeFile` instead: this + assertion fails); no "file with uuid '…' not found" toast (toasts are + recorded by a MutationObserver, since they auto-dismiss after 5s); and the + sessionStorage tab history lost `gone.py` but kept `keep.py`. 9. **Deletion freshness.** Pushes another competing `keep.py`, deletes `keep.py` from IndexedDB (`apply-files` with it withheld), real-clicks **Commit** → asserts the push succeeded, the bare repo still holds the teammate's `keep.py`, and a `[data-pybricks-git-notice]` names the skipped deletion. +9a. **Fallback Pull.** Closes every tab, opens only `e2e.py`, pushes its upstream + deletion, nulls the MAIN-world `findAppStore` so `write-files-live` returns + `{live:false}`, and Pulls → asserts the page **reloads** (`apply-files` path), + that no "not found" toast appears after the reload, that `e2e.py` left the tab + history, and that it was deleted. (Verified by disabling the tab prune in + `content.js`: the toast assertion fails.) 10. Asserts zero **extension** exceptions across **both** the page and the service worker. 11. Captures a screenshot, writing `toolbar.png`. @@ -113,11 +124,12 @@ driver's own comments use for it: [e2e] === STEP 2: Configure settings via the service_worker target === [e2e] PASS: settings written to chrome.storage.local via SW -[e2e] === STEP 3: Pull: real-click, expect label "↓ +4 ~0 -0", then reload === +[e2e] === STEP 3: Pull: real-click, expect label "↓ +4 ~0 -0", with no reload === [e2e] pull label -> "Pulling…" [e2e] pull label -> "↓ +4 ~0 -0" [e2e] PASS: Pull label is "↓ +4 ~0 -0" (got "↓ +4 ~0 -0") -[e2e] PASS: starter.py present in editor IndexedDB after Pull+reload +[e2e] PASS: the page did not reload after Pull +[e2e] PASS: starter.py present in editor IndexedDB after Pull [e2e] === STEP 4: Seed a second file, then Commit with message "e2e message" === [e2e] apply-files summary: { added: 1, changed: 0, deleted: 0, unchanged: 4 } @@ -144,7 +156,11 @@ driver's own comments use for it: [e2e] PASS: exactly one keep.py in the editor [e2e] PASS: no keep_mine.py sibling for the untouched file [e2e] PASS: untouched gone.py went away with the upstream deletion -[e2e] PASS: no "file … not found" toast after the reload ([]) +[e2e] PASS: the page did not reload after the merge Pull +[e2e] PASS: the deleted gone.py's tab was closed +[e2e] PASS: the changed keep.py is still an open tab +[e2e] PASS: keep.py's open tab shows the pulled version (model replaced in place) +[e2e] PASS: no "file … not found" toast after the Pull ([]) [e2e] PASS: the deleted gone.py left Pybricks' open-tab history [e2e] PASS: the kept keep.py is still a remembered tab [e2e] PASS: the protected coach.py took the repo's version despite the local edit @@ -156,6 +172,12 @@ driver's own comments use for it: [e2e] PASS: the teammate's keep.py survived a local deletion it never saw [e2e] PASS: the skipped deletion is reported to the kid by name +[e2e] === STEP 7a: Fallback Pull (no store) reloads and leaves no stale tab behind === +[e2e] PASS: the fallback Pull reloaded the page +[e2e] PASS: no "file … not found" toast after the fallback reload ([]) +[e2e] PASS: the deleted e2e.py left Pybricks' open-tab history +[e2e] PASS: e2e.py was deleted by the fallback Pull + [e2e] === STEP 8: Zero extension exceptions (page + service worker) === [e2e] PASS: zero extension exceptions (saw 0) @@ -170,9 +192,9 @@ driver's own comments use for it: | Action | Button label timeline | |---|---| -| Pull | `Pull` → `Pulling…` → `↓ +4 ~0 -0` → (page reloads) | +| Pull | `Pull` → `Pulling…` → `↓ +4 ~0 -0` → `Pull` (no reload) | | Commit | `Commit` → `Committing…` → `✓ ↑` | -| Merge Pull | `Pull` → `Pulling…` → `↓ +1 ~3 -1` → (page reloads) | +| Merge Pull | `Pull` → `Pulling…` → `↓ +1 ~3 -1` → `Pull` (no reload) | `toolbar.png` (committed alongside this README) is the final screenshot, taken after the deletion-freshness commit — the Commit button still reads @@ -214,7 +236,7 @@ plain `mission_01.py`, and a setup-only **blocks** file `arm_moves.py` — then: 1. Writes settings via the SW, dismisses the Welcome Tour, and **Pull**s (label `↓ +4 ~0 -0`; `.pybricks-git.json` is non-`.py`, so only the four `.py` - files apply), waiting for the reload. + files apply), asserting the page did **not** reload. 2. Asserts the engine persisted `chrome.storage.local.lastPullManifest === {protected:["menu.py"], menuConfig:"menu_config.py"}` (read via the SW target). 3. Opens the **Menu** panel (`[data-pybricks-git-menu-btn]`), asserting the @@ -338,8 +360,8 @@ Two related facts, both cost real debugging time: - **The file tree's label centre opens Rename, not the file.** Each row carries a `.pb-explorer-file-tree-action-toolbar`, and its rename button sits under the label centre. Click the row's **icon gutter** (`left + 14px`) to open a file. -- **The Explorer is a toggle that survives a reload.** After the post-Update - reload the app restores it already open, so clicking the toolbar button blind +- **The Explorer is a toggle that survives a reload.** After a reload the app + restores it already open, so clicking the toolbar button blind closes it. Check for the tree first, click only if it is absent. ## What it covers @@ -352,21 +374,30 @@ derived from it by JSON surgery on the setup chain: `prog_match.py` (chain identical), `prog_differs.py` (one motor port changed → splices cleanly), and `prog_renamed.py` (a device renamed → `spliceSetup` skips). Then: -1. Writes settings via the SW, dismisses the Welcome Tour, **Pull**s (`↓ +6 ~0 -0`), - and asserts `lastPullManifest` carries `teamSetup` + `protected`. +1. Writes settings via the SW, dismisses the Welcome Tour, **Pull**s (`↓ +6 ~0 -0`, + no reload), and asserts `lastPullManifest` carries `teamSetup` + `protected`. 2. Opens the Menu panel and asserts the **setup-differs nudge**: `[data-pybricks-git-setup-differs]` marks `prog_differs`/`prog_renamed`, not the matching `prog_match`; the `[data-pybricks-git-update-setup]` and `[data-pybricks-git-new-program]` buttons are present. 3. **New program:** creates `my_new_one` via `[data-pybricks-git-new-program]`; - after the reload asserts `my_new_one.py` exists with `setupSignature` equal to + asserts the status `Created my_new_one.py ✓`, **no reload**, the open panel + already lists it (refreshed in place), and that `my_new_one.py` exists with `setupSignature` equal to `robot_setup.py`'s and a `blockGlobalStart` block. (Creating a local file also diverges the editor from the remote, so the next step's snapshot lands a real commit.) -4. **Update robot setup:** clicks the button and, after the reload, asserts the +4. **Update robot setup:** first opens `prog_differs.py` (a block program the + Update will rewrite) as the **active tab**, then clicks the button. Asserts + `Updated 1 program(s) ✓`, **no reload**, and that the block tab was reopened + and is still active (open block programs are closed, written and reopened, + never replaced under a live Blockly workspace). Without a licence, the + reopened tab brings back the "Enable block coding" dialog (a reload restoring + the tab did too), so the driver dismisses it. Then it asserts the **snapshot-first rail** harness-side — the remote gained a `Before robot setup - update` commit whose tree holds the **PRE-splice** `prog_differs.py` (the - spliced version lives only in editor IDB until the manual Commit), with + update` commit whose tree holds the **PRE-splice** `prog_differs.py` setup + (compared by `setupSignature`, not bytes: opening the file made the editor + regenerate its Python body from the blocks. The spliced version lives only in + editor IDB until the manual Commit), with `prog_renamed.py`/`menu.py` byte-identical. Browser-side it asserts the `[data-pybricks-git-splice-report]` block (updated: `prog_differs`; skipped: `prog_renamed` with the kid-facing reason) and that the editor `prog_differs.py` diff --git a/test/e2e/drive-menu.mjs b/test/e2e/drive-menu.mjs index ed72b39..58f52f3 100644 --- a/test/e2e/drive-menu.mjs +++ b/test/e2e/drive-menu.mjs @@ -501,7 +501,9 @@ async function main() { } // -- Pull ----------------------------------------------------------- - step(3, 'Pull the template repo, then wait for the reload'); + step(3, 'Pull the template repo, with no reload'); + await evalIsolated(`window.__pbgitNoReload = 1`, false); + const ctxBeforePull = isolatedCtx; const pullPt = await buttonRect('Pull'); await trustedClick(pullPt); const rawPull = () => @@ -524,16 +526,11 @@ async function main() { `Pull label is "↓ +4 ~0 -0" (got "${pullLabel}")`, ); - log('waiting for post-Pull reload...'); - await poll(() => isolatedCtx === null, { - timeout: 15000, - what: 'reload to clear isolated context', - }).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 reload', - }); - log('page reloaded, buttons remounted'); + await sleep(2500); // the raw fallback reloads 1.5s after the label + assert( + isolatedCtx === ctxBeforePull && (await evalIsolated(`window.__pbgitNoReload === 1`, false)), + 'the page did not reload after Pull', + ); // -- Assert the persisted manifest (SW storage) --------------------- step(4, 'Assert lastPullManifest persisted by the engine'); diff --git a/test/e2e/drive-splice.mjs b/test/e2e/drive-splice.mjs index fff23ef..7126fe0 100644 --- a/test/e2e/drive-splice.mjs +++ b/test/e2e/drive-splice.mjs @@ -343,7 +343,20 @@ async function main() { } // -- Pull ----------------------------------------------------------- - step(3, 'Pull the template repo, then wait for the reload'); + step(3, 'Pull the template repo, with no reload'); + // Pull, New program and Update robot setup all write through Pybricks' + // own store (write-files-live), so the page must never reload — a + // reload drops the hub's Bluetooth link. A marker on the isolated + // world's window and the context id prove the same document survived. + await evalIsolated(`window.__pbgitNoReload = 1`, false); + const ctxBefore = isolatedCtx; + const noReload = async () => + isolatedCtx === ctxBefore && (await evalIsolated(`window.__pbgitNoReload === 1`, false)); + 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; + }; await trustedClick(await buttonRect('Pull')); const pullLabel = await poll( async () => { @@ -355,8 +368,8 @@ async function main() { log('pull label =', JSON.stringify(pullLabel)); // 6 .py: menu, menu_config, robot_setup, prog_match, prog_differs, prog_renamed. assert(pullLabel === '↓ +6 ~0 -0', `Pull label is "↓ +6 ~0 -0" (got "${pullLabel}")`); - await poll(() => isolatedCtx === null, { timeout: 15000, what: 'reload to clear isolated context' }).catch(() => {}); - await poll(async () => (await buttonRect('Pull')) != null, { timeout: 40000, what: 'buttons to remount after reload' }); + await sleep(2500); // the raw fallback reloads 1.5s after the label + assert(await noReload(), 'the page did not reload after Pull'); // -- Manifest ------------------------------------------------------- step(4, 'Assert lastPullManifest persisted teamSetup'); @@ -379,14 +392,24 @@ async function main() { // -- New program from the team setup -------------------------------- // Creating a local file also DIVERGES the editor from the remote, so the // safety snapshot committed by Update (next step) lands a real commit. - step(6, 'Create my_new_one via New program; assert seed after reload'); + step(6, 'Create my_new_one via New program; assert seed, no reload'); await clickSelector('[data-pybricks-git-new-program]', 'New program button'); await poll(() => exists('[data-pybricks-git-new-name]'), { timeout: 10000, what: 'name input row' }); await clickSelector('[data-pybricks-git-new-name]', 'name input'); await page.send('Input.insertText', { text: 'my_new_one' }); await clickSelector('[data-pybricks-git-new-create]', 'Create button'); - await poll(() => isolatedCtx === null, { timeout: 15000, what: 'reload after create' }).catch(() => {}); - await poll(async () => (await buttonRect('Pull')) != null, { timeout: 40000, what: 'buttons remount post-create' }); + const createStatus = await poll( + () => evalIsolated(`(() => { const s=document.querySelector('[data-pybricks-git-status]'); return s && /^Created|Couldn/.test(s.textContent) ? s.textContent : null; })()`, false), + { timeout: 15000, interval: 100, what: 'New program status' }, + ); + log('create status =', JSON.stringify(createStatus)); + assert(/^Created my_new_one\.py ✓$/.test(createStatus), `New program finished without a reload (status "${createStatus}")`); + await sleep(1500); // the raw fallback reloads 800ms after the status + assert(await noReload(), 'the page did not reload after New program'); + assert( + (await rowHasMarker('my_new_one')) !== 'NO_ROW', + 'the open panel lists my_new_one without a reload (refreshed in place)', + ); const robotRow0 = (await evalIsolated(`pageRequest('list-files')`)).contents.find((c) => c.path === 'robot_setup.py'); const robotSig = await sigOf(robotRow0.contents); const listingAfterNew = await evalIsolated(`pageRequest('list-files')`); @@ -400,10 +423,40 @@ async function main() { step(7, 'Record remote state, then Update robot setup'); const subjectsBefore = bareSubjects(bare); assert(!subjectsBefore.includes('Before robot setup update'), 'no snapshot commit exists before Update'); - await poll(() => exists('[data-pybricks-git-panel]'), { timeout: 15000, what: 'panel reopened after create reload' }); + // Open the block program the Update will rewrite as the ACTIVE tab: an + // open block program must be closed, written, and reopened (never + // replaced under a live Blockly workspace). The tab and its active + // status must survive. + const differsUuid = listingAfterNew.metadata.find((m) => m.path === 'prog_differs.py').uuid; + await evalMain(`findAppStore().dispatch({ type: 'editor.action.activateFile', uuid: ${JSON.stringify(differsUuid)} })`); + await poll(() => evalMain(`findAppStore().getState().editor.openFileUuids.includes(${JSON.stringify(differsUuid)})`), { + timeout: 15000, + what: 'prog_differs.py to open in a tab', + }); + // Without a licence Pybricks pops "Enable block coding" over a block + // file; dismiss it so it can't block the panel click. + for (const type of ['keyDown', 'keyUp']) await page.send('Input.dispatchKeyEvent', { type, windowsVirtualKeyCode: 27, key: 'Escape', code: 'Escape' }); await clickSelector('[data-pybricks-git-update-setup]', 'Update robot setup button'); - await poll(() => isolatedCtx === null, { timeout: 20000, what: 'reload after Update' }).catch(() => {}); - await poll(async () => (await buttonRect('Pull')) != null, { timeout: 40000, what: 'buttons remount post-Update' }); + const updateStatus = await poll( + () => evalIsolated(`(() => { const s=document.querySelector('[data-pybricks-git-status]'); return s && /^Updated|Couldn|Saved the snapshot/.test(s.textContent) ? s.textContent : null; })()`, false), + { timeout: 30000, interval: 200, what: 'Update robot setup status' }, + ); + log('update status =', JSON.stringify(updateStatus)); + assert(/^Updated 1 program\(s\) ✓$/.test(updateStatus), `Update finished without a reload (status "${updateStatus}")`); + await sleep(1500); + assert(await noReload(), 'the page did not reload after Update robot setup'); + const tabsAfterUpdate = await evalMain(`findAppStore().getState().editor`); + assert(tabsAfterUpdate.openFileUuids.includes(differsUuid), 'the rewritten block program is an open tab again'); + assert(tabsAfterUpdate.activeFileUuid === differsUuid, 'the rewritten block program is still the active tab'); + // Reopening the block tab brings back the licence dialog when there's + // no licence — exactly what the old reload did when it restored the + // tab. Dismiss it so it can't swallow later clicks. + const dialogTitle = await evalMain(`document.querySelector('.bp5-dialog .bp5-heading')?.textContent ?? null`); + log('dialog after Update =', JSON.stringify(dialogTitle)); + if (dialogTitle) { + for (const type of ['keyDown', 'keyUp']) await page.send('Input.dispatchKeyEvent', { type, windowsVirtualKeyCode: 27, key: 'Escape', code: 'Escape' }); + await poll(() => evalMain(`!document.querySelector('.bp5-dialog')`), { timeout: 5000, what: 'the dialog to close' }); + } // -- Snapshot-first + report ---------------------------------------- step(8, 'Assert snapshot-first commit + splice report + editor outcomes'); @@ -413,7 +466,16 @@ async function main() { // The snapshot's tree holds the PRE-splice prog_differs (port E); the // spliced port-F version lives only in editor IDB until the manual // Commit below. That ordering IS the snapshot-first proof. - assert(bareFile(bare, 'prog_differs.py') === PROG_DIFFERS, 'snapshot commit holds the PRE-splice prog_differs.py'); + // Semantic, not byte-exact: prog_differs.py was open in a tab, and + // opening a block file makes the editor regenerate its Python body + // from the blocks (the fixture's JSON-surgery body said Port.F; the + // regenerated one says Port.E). The snapshot faithfully holds that + // editor state — what matters is that its SETUP is still pre-splice. + const snapDiffers = bareFile(bare, 'prog_differs.py'); + assert( + (await sigOf(snapDiffers)) === (await sigOf(PROG_DIFFERS)) && (await sigOf(snapDiffers)) !== robotSig, + 'snapshot commit holds the PRE-splice prog_differs.py setup', + ); assert(bareFile(bare, 'prog_renamed.py') === PROG_RENAMED, 'snapshot commit holds prog_renamed.py verbatim'); assert(bareFile(bare, 'menu.py') === SEED_FILES['menu.py'], 'protected menu.py untouched in the snapshot'); await poll(() => exists('[data-pybricks-git-splice-report]'), { timeout: 15000, what: 'splice report block' }); @@ -438,8 +500,8 @@ async function main() { if (!licence) { log('SKIP: PYBRICKS_LICENSE unset — the editor will not open block files ungated'); } else { - // The Explorer is a TOGGLE and the app restores it open across the - // post-Update reload, so clicking blind can close it. + // The Explorer is a TOGGLE (and the app restores it open across a + // reload), so clicking blind can close it. const treeUp = () => exists('[role="tree"][aria-label="Files"]'); if (!(await treeUp())) { const explorerPt = await rectOf('#pb-toolbar-explorer-button'); diff --git a/test/e2e/drive.mjs b/test/e2e/drive.mjs index ec36f6c..c066e6f 100644 --- a/test/e2e/drive.mjs +++ b/test/e2e/drive.mjs @@ -482,7 +482,13 @@ async function main() { log('no welcome tour overlay present'); } - step(3, 'Pull: real-click, expect label "↓ +4 ~0 -0", then reload'); + step(3, 'Pull: real-click, expect label "↓ +4 ~0 -0", with no reload'); + // Pull syncs 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 and the context id prove the same + // document survived. + await evalIsolated(`window.__pbgitNoReload = 1`, false); + const ctxBeforePull = isolatedCtx; const pullPt = await buttonRect('Pull'); log('Pull button center:', JSON.stringify(pullPt)); log('elementFromPoint(pull center):', await elementAt(pullPt.x, pullPt.y)); @@ -513,25 +519,17 @@ async function main() { `Pull label is "↓ +4 ~0 -0" (got "${pullLabel}")`, ); - // content.js reloads ~1.5s after a non-empty apply; wait for the - // context to be torn down and rebuilt. - log('waiting for post-Pull reload...'); - await poll(() => isolatedCtx === null, { - timeout: 15000, - what: 'reload to clear isolated context', - }).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 reload', - }); - log('page reloaded, buttons remounted'); + await sleep(2500); // the raw fallback reloads 1.5s after the label + const noReload = async () => + isolatedCtx === ctxBeforePull && (await evalIsolated(`window.__pbgitNoReload === 1`, false)); + assert(await noReload(), 'the page did not reload after Pull'); const afterPull = await evalIsolated(`pageRequest('list-files')`); const pulledPaths = afterPull.contents.map((c) => c.path); log('editor files after pull:', pulledPaths); assert( pulledPaths.some((p) => p === 'starter.py' || p.endsWith('/starter.py')), - 'starter.py present in editor IndexedDB after Pull+reload', + 'starter.py present in editor IndexedDB after Pull', ); // -- Commit --------------------------------------------------------- @@ -652,11 +650,11 @@ async function main() { ); log('pushed competing change to the bare repo'); - // Open gone.py (deleted upstream below) and keep.py (kept) in editor - // tabs, the way the Explorer does. Pybricks remembers open tabs in - // sessionStorage and reopens them after the reload; a stale uuid for - // gone.py would raise its "unexpected error … not found" toast. That - // toast auto-dismisses after 5s, so record every toast from load on. + // Open gone.py (deleted upstream below) and keep.py (changed upstream) + // in editor tabs, the way the Explorer does. The live Pull must close + // gone.py's tab itself (Pybricks' delete flow) and update keep.py's open + // model in place. Toasts auto-dismiss after 5s, so record every toast — + // in this document and in any later one (the fallback path reloads). const evalMain = async (expression) => { const r = await page.send('Runtime.evaluate', { expression, awaitPromise: true, returnByValue: true }); if (r.exceptionDetails) { @@ -664,14 +662,16 @@ async function main() { } return r.result.value; }; - await page.send('Page.addScriptToEvaluateOnNewDocument', { source: ` - window.__toasts = []; + const toastRecorder = ` + window.__toasts = window.__toasts || []; new MutationObserver(() => { for (const t of document.querySelectorAll('.bp5-toast')) { if (!window.__toasts.includes(t.textContent)) window.__toasts.push(t.textContent); } }).observe(document, { childList: true, subtree: true, characterData: true }); - ` }); + `; + await page.send('Page.addScriptToEvaluateOnNewDocument', { source: toastRecorder }); + await evalMain(toastRecorder); const tabMeta = (await evalIsolated(`pageRequest('list-files')`)).metadata; const goneUuid = tabMeta.find((m) => m.path === 'gone.py').uuid; const keepUuid = tabMeta.find((m) => m.path === 'keep.py').uuid; @@ -710,13 +710,6 @@ async function main() { `merge Pull label is "↓ +1 ~3 -1" (got "${mergeLabel}")`, ); - log('waiting for post-merge reload...'); - await poll(() => isolatedCtx === null, { - timeout: 15000, - what: 'reload to clear isolated context', - }).catch(() => log('note: did not observe context clear (may have raced)')); - // Read the notice before waiting on the toolbar: it is rendered by - // content.js at load and self-removes after 20s. const rescueText = await poll( () => evalIsolated( @@ -738,26 +731,28 @@ async function main() { !/coach\.py/.test(rescueText), 'rescue notice says nothing about the protected coach.py (overwritten, never rescued)', ); + await sleep(2500); // the raw fallback reloads 1.5s after the label + assert(await noReload(), 'the page did not reload after the merge Pull'); - await poll(async () => (await buttonRect('Pull')) != null, { - timeout: 40000, - what: 'buttons to remount after the merge reload', - }); - // Pybricks reopens remembered tabs as the editor mounts; give it time - // to try (and to toast, if a stale uuid were still there). - await poll(() => evalMain(`findAppStore()?.getState().editor.openFileUuids.length > 0`), { - timeout: 20000, - what: 'Pybricks to reopen the remembered tabs', - }); - await sleep(2000); - // The toast is the decisive check: once Pybricks reopens keep.py it - // rewrites the history from memory, which drops a stale gone.py uuid - // anyway — but only AFTER the failed reopen has already toasted. - // (Verified: with the prune disabled, only this assertion fails.) + // Tabs, handled live the way Pybricks' own Explorer would: + const tabs = await evalMain(`findAppStore().getState().editor`); + assert(!tabs.openFileUuids.includes(goneUuid), "the deleted gone.py's tab was closed"); + assert(tabs.openFileUuids.includes(keepUuid), 'the changed keep.py is still an open tab'); + // gone.py was active; closing it activates keep.py, whose open Monaco + // model must now show the pulled text (replaced in place, not stale). + const keepShown = await poll( + () => + evalMain( + // Monaco renders spaces as U+00A0; normalise before matching. + `(document.querySelector('.monaco-editor .view-lines')?.textContent || '').replace(/\\u00a0/g, ' ').includes('keep v2')`, + ), + { timeout: 10000, interval: 250, what: "keep.py's open tab to show the pulled text" }, + ).catch(() => false); + assert(keepShown, "keep.py's open tab shows the pulled version (model replaced in place)"); const staleToasts = (await evalMain(`window.__toasts`)).filter((t) => /not found/.test(t)); assert( staleToasts.length === 0, - `no "file … not found" toast after the reload (${JSON.stringify(staleToasts)})`, + `no "file … not found" toast after the Pull (${JSON.stringify(staleToasts)})`, ); const historyAfter = await tabHistory(); assert(!historyAfter.includes(goneUuid), "the deleted gone.py left Pybricks' open-tab history"); @@ -868,6 +863,56 @@ async function main() { 'the skipped deletion is reported to the kid by name', ); + // -- Fallback Pull: raw apply + reload, with open-tab cleanup -------- + // Hide the app store so write-files-live resolves {live:false}; Pull + // must fall back to apply-files + reload, and prune the deleted file's + // remembered tab first (otherwise the reload toasts "not found"). + step('7a', 'Fallback Pull (no store) reloads and leaves no stale tab behind'); + // Step 7 deleted keep.py behind the app's back (raw apply-files) while + // its tab was open — that stale tab is the test's doing, not Pull's, so + // start from a clean slate: close every tab, then open only e2e.py. + for (const uuid of await evalMain(`findAppStore().getState().editor.openFileUuids`)) { + await evalMain(`findAppStore().dispatch({ type: 'editor.action.closeFile', uuid: ${JSON.stringify(uuid)} })`); + } + await poll(() => evalMain(`findAppStore().getState().editor.openFileUuids.length === 0`), { + timeout: 10000, + what: 'all tabs to close', + }); + const e2eUuid = (await evalIsolated(`pageRequest('list-files')`)).metadata.find((m) => m.path === 'e2e.py').uuid; + await evalMain(`findAppStore().dispatch({ type: 'editor.action.activateFile', uuid: ${JSON.stringify(e2eUuid)} })`); + await poll(() => evalMain(`findAppStore().getState().editor.openFileUuids.includes(${JSON.stringify(e2eUuid)})`), { + timeout: 10000, + what: 'e2e.py to open in a tab', + }); + pushCompeting(bare, { 'e2e.py': null }, 'teammate deletes e2e.py'); + await evalMain(`findAppStore = () => null`); + const ctxBeforeFallback = isolatedCtx; + await trustedClick(await buttonRect('Pull')); + await poll(() => isolatedCtx !== ctxBeforeFallback && isolatedCtx !== null, { + timeout: 30000, + what: 'the fallback Pull to reload the page', + }); + await poll(async () => (await buttonRect('Pull')) != null, { + timeout: 40000, + what: 'buttons to remount after the fallback reload', + }); + assert(true, 'the fallback Pull reloaded the page'); + await poll(() => evalMain(`!!findAppStore()?.getState().editor.isReady`), { + timeout: 20000, + what: 'the editor to be ready after the reload', + }); + await sleep(3000); // let Pybricks try to reopen remembered tabs (and toast) + const fallbackToasts = (await evalMain(`window.__toasts`)).filter((t) => /not found/.test(t)); + assert( + fallbackToasts.length === 0, + `no "file … not found" toast after the fallback reload (${JSON.stringify(fallbackToasts)})`, + ); + assert(!(await tabHistory()).includes(e2eUuid), "the deleted e2e.py left Pybricks' open-tab history"); + assert( + !(await evalIsolated(`pageRequest('list-files')`)).contents.some((c) => c.path === 'e2e.py'), + 'e2e.py was deleted by the fallback Pull', + ); + // -- A failed Pull explains itself ----------------------------------- // Point the extension at a repo the harness doesn't serve: the Pull // must fail with the error panel (HTTP 404, the repo URL, a hint), not diff --git a/test/inject.test.mjs b/test/inject.test.mjs index e2a1641..3b06b35 100644 --- a/test/inject.test.mjs +++ b/test/inject.test.mjs @@ -328,20 +328,26 @@ function fakeRoot(store) { } // 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 } = {}) { +// in a Dexie write and deleteFile in a Dexie delete (done here with raw IDB, +// after a tick, as a saga would); closeFile/activateFile update the open-tab +// state. `state.editor` is live so tests can inspect tabs afterwards. +function fakeStore({ openFileUuids = [], activeFileUuid = null, initialized = true, onDispatch, db } = {}) { const dispatched = []; - return { + const editor = { openFileUuids: [...openFileUuids], activeFileUuid }; + const store = { dispatched, - getState: () => ({ editor: { openFileUuids }, fileStorage: { isInitialized: initialized } }), + editor, + getState: () => ({ editor, fileStorage: { isInitialized: initialized } }), dispatch(action) { dispatched.push(action); - if (onDispatch) onDispatch(action); + if (onDispatch) return onDispatch(action); + if (db) return actLikePybricks(action, db, editor); }, }; + return store; } -async function actLikePybricks(action, db) { +async function actLikePybricks(action, db, editor = { openFileUuids: [] }) { await new Promise((r) => setTimeout(r, 20)); if (action.type === 'fileStorage.action.writeFile') { await upsertFiles({ files: [{ path: action.path, contents: action.contents }] }); @@ -349,6 +355,23 @@ async function actLikePybricks(action, db) { const meta = await getAll(db, 'metadata'); const row = meta.find((m) => m.uuid === action.uuid); await upsertFiles({ files: [{ path: row.path, contents: action.value }] }); + } else if (action.type === 'fileStorage.action.deleteFile') { + // One path only, like Pybricks' handleDeleteFile (a whole-set rewrite + // here would race the concurrent writes). + const row = (await getAll(db, 'metadata')).find((m) => m.path === action.path); + await new Promise((resolve, reject) => { + const tx = db.transaction(['metadata', '_contents'], 'readwrite'); + tx.objectStore('metadata').delete(row.uuid); + tx.objectStore('_contents').delete(action.path); + tx.oncomplete = () => resolve(); + tx.onerror = () => reject(tx.error); + }); + } else if (action.type === 'editor.action.closeFile') { + editor.openFileUuids = editor.openFileUuids.filter((u) => u !== action.uuid); + if (editor.activeFileUuid === action.uuid) editor.activeFileUuid = null; + } else if (action.type === 'editor.action.activateFile') { + if (!editor.openFileUuids.includes(action.uuid)) editor.openFileUuids.push(action.uuid); + editor.activeFileUuid = action.uuid; } } @@ -371,28 +394,71 @@ describe('findAppStore', () => { }); 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' }, + const BLOCKS = '# pybricks blocks file:{"blocks":{}}\nprint(1)\n'; + const before = { + metadata: [ + { 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' }, + { path: 'blocks.py', uuid: 'u-blocks', sha256: 'old' }, + { path: 'gone.py', uuid: 'u-gone', sha256: 'g' }, + { path: 'gone_open.py', uuid: 'u-gone-open', sha256: 'g' }, + ], + contents: [ + { path: 'menu_config.py', contents: 'm' }, + { path: 'open.py', contents: 'o' }, + { path: 'same.py', contents: 's' }, + { path: 'blocks.py', contents: BLOCKS }, + { path: 'gone.py', contents: 'g' }, + { path: 'gone_open.py', contents: 'g' }, + ], + }; + const files = [ + { path: 'menu_config.py', contents: 'M', sha: 'new' }, + { path: 'open.py', contents: 'O', sha: 'new' }, + { path: 'same.py', contents: 'S', sha: 'same' }, + { path: 'blocks.py', contents: BLOCKS + '# v2\n', sha: 'new' }, + { path: 'fresh.py', contents: 'F', sha: 'new' }, ]; + const openFileUuids = ['u-gone-open', 'u-blocks', 'u-open', 'u-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, [ + test('upsert: open text files go through the editor, others through file storage', () => { + const plan = planLiveWrites({ files, before, openFileUuids }); + assert.deepEqual(plan.writes, [ { type: 'fileStorage.action.writeFile', path: 'menu_config.py', contents: 'M' }, { type: 'editor.action.replaceFile', uuid: 'u-open', value: 'O' }, + { type: 'fileStorage.action.writeFile', path: 'blocks.py', contents: BLOCKS + '# v2\n' }, { type: 'fileStorage.action.writeFile', path: 'fresh.py', contents: 'F' }, ]); + assert.deepEqual(plan.deletes, [], 'upsert never deletes'); + assert.deepEqual(plan.summary, { added: 1, changed: 3, deleted: 0, unchanged: 1 }); + }); + + test('an open block program is closed, written through file storage, and reopened', () => { + const plan = planLiveWrites({ files, before, openFileUuids }); + assert.deepEqual(plan.close, ['u-blocks']); + assert.deepEqual(plan.reopen, ['u-blocks']); + }); + + test('a text file becoming a block program also takes the close/reopen path', () => { + const plan = planLiveWrites({ + files: [{ path: 'open.py', contents: BLOCKS, sha: 'new' }], + before, + openFileUuids, + }); + assert.deepEqual(plan.close, ['u-open']); + assert.deepEqual(plan.writes, [{ type: 'fileStorage.action.writeFile', path: 'open.py', contents: BLOCKS }]); + }); + + test('deleteUnlisted deletes every other file, closing (not reopening) open ones', () => { + const plan = planLiveWrites({ files, before, openFileUuids, deleteUnlisted: true }); + assert.deepEqual(plan.deletes, [ + { type: 'fileStorage.action.deleteFile', path: 'gone.py' }, + { type: 'fileStorage.action.deleteFile', path: 'gone_open.py' }, + ]); + assert.deepEqual(plan.close, ['u-gone-open', 'u-blocks'], 'close keeps open-tab order'); + assert.deepEqual(plan.reopen, ['u-blocks'], 'a deleted file is never reopened'); + assert.deepEqual(plan.summary, { added: 1, changed: 3, deleted: 2, unchanged: 1 }); }); }); @@ -400,12 +466,12 @@ 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 store = fakeStore({ db }); const res = await writeFilesLive({ files: [{ path: 'menu_config.py', contents: 'new\n' }], rootEl: fakeRoot(store), }); - assert.deepEqual(res, { live: true, dispatched: 1 }); + assert.deepEqual(res, { live: true, dispatched: 1, summary: { added: 0, changed: 1, deleted: 0, unchanged: 0 } }); assert.equal(store.dispatched[0].type, 'fileStorage.action.writeFile'); const snap = await snapshot(db); assert.equal(snap.byPath['menu_config.py'], 'new\n'); @@ -415,7 +481,7 @@ describe('writeFilesLive', () => { 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 store = fakeStore({ openFileUuids: ['u-menu'], db }); const res = await writeFilesLive({ files: [{ path: 'menu_config.py', contents: 'new\n' }], rootEl: fakeRoot(store), @@ -426,7 +492,7 @@ describe('writeFilesLive', () => { test('creates a missing file through fileStorage', async () => { const db = await openPybricks(); - const store = fakeStore({ onDispatch: (a) => actLikePybricks(a, db) }); + const store = fakeStore({ db }); const res = await writeFilesLive({ files: [{ path: 'menu_config.py', contents: 'new\n' }], rootEl: fakeRoot(store), @@ -443,7 +509,7 @@ describe('writeFilesLive', () => { files: [{ path: 'menu_config.py', contents: 'same\n' }], rootEl: fakeRoot(store), }); - assert.deepEqual(res, { live: true, dispatched: 0 }); + assert.deepEqual(res, { live: true, dispatched: 0, summary: { added: 0, changed: 0, deleted: 0, unchanged: 1 } }); assert.deepEqual(store.dispatched, []); }); @@ -461,6 +527,84 @@ describe('writeFilesLive', () => { assert.equal((await snapshot(db)).byPath['menu_config.py'], 'old\n', 'nothing written behind the app'); }); + test('deleteUnlisted syncs like apply-files: writes, deletes, and closes a deleted tab', async () => { + const db = await openPybricks(); + await seed(db, [ + { path: 'keep.py', contents: 'k1\n', uuid: 'u-keep' }, + { path: 'gone.py', contents: 'g\n', uuid: 'u-gone' }, + ]); + const store = fakeStore({ openFileUuids: ['u-gone', 'u-keep'], activeFileUuid: 'u-gone', db }); + const res = await writeFilesLive({ + files: [ + { path: 'keep.py', contents: 'k2\n' }, + { path: 'new.py', contents: 'n\n' }, + ], + deleteUnlisted: true, + rootEl: fakeRoot(store), + }); + assert.deepEqual(res, { live: true, dispatched: 3, summary: { added: 1, changed: 1, deleted: 1, unchanged: 0 } }); + const snap = await snapshot(db); + assert.deepEqual(Object.keys(snap.byPath).sort(), ['keep.py', 'new.py']); + assert.equal(snap.byPath['keep.py'], 'k2\n'); + assert.deepEqual(store.editor.openFileUuids, ['u-keep'], "the deleted file's tab was closed, not reopened"); + const types = store.dispatched.map((a) => a.type); + assert.ok( + types.indexOf('editor.action.closeFile') < types.indexOf('fileStorage.action.deleteFile'), + 'the tab is closed before the delete is dispatched', + ); + }); + + test('an open block program is closed, rewritten, and reopened as the active tab', async () => { + const db = await openPybricks(); + const BLOCKS = '# pybricks blocks file:{"blocks":{}}\n'; + await seed(db, [ + { path: 'prog.py', contents: BLOCKS + 'v1\n', uuid: 'u-prog' }, + { path: 'other.py', contents: 'o\n', uuid: 'u-other' }, + ]); + const store = fakeStore({ openFileUuids: ['u-other', 'u-prog'], activeFileUuid: 'u-prog', db }); + const res = await writeFilesLive({ + files: [{ path: 'prog.py', contents: BLOCKS + 'v2\n' }], + rootEl: fakeRoot(store), + }); + assert.equal(res.live, true); + assert.equal((await snapshot(db)).byPath['prog.py'], BLOCKS + 'v2\n'); + assert.deepEqual( + store.dispatched.map((a) => a.type), + ['editor.action.closeFile', 'fileStorage.action.writeFile', 'editor.action.activateFile'], + ); + assert.deepEqual(store.editor.openFileUuids, ['u-other', 'u-prog']); + assert.equal(store.editor.activeFileUuid, 'u-prog'); + }); + + test('closed block tabs are reopened even when the write is not confirmed', async () => { + const db = await openPybricks(); + const BLOCKS = '# pybricks blocks file:{"blocks":{}}\n'; + await seed(db, [{ path: 'prog.py', contents: BLOCKS + 'v1\n', uuid: 'u-prog' }]); + const store = fakeStore({ + openFileUuids: ['u-prog'], + // Tabs work, but writes are swallowed (a renamed action type). + onDispatch: (a) => (a.type.startsWith('editor.') ? actLikePybricks(a, db, store.editor) : undefined), + }); + const res = await writeFilesLive({ + files: [{ path: 'prog.py', contents: BLOCKS + 'v2\n' }], + rootEl: fakeRoot(store), + timeoutMs: 300, + }); + assert.equal(res.live, false); + assert.deepEqual(store.editor.openFileUuids, ['u-prog'], 'the tab came back'); + }); + + test('a tab that will not close resolves live:false before anything is written', async () => { + const db = await openPybricks(); + await seed(db, [{ path: 'gone.py', contents: 'g\n', uuid: 'u-gone' }]); + const store = fakeStore({ openFileUuids: ['u-gone'], onDispatch: () => {} }); + const res = await writeFilesLive({ files: [], deleteUnlisted: true, rootEl: fakeRoot(store), timeoutMs: 300 }); + assert.equal(res.live, false); + assert.match(res.reason, /did not close/); + assert.deepEqual(store.dispatched.map((a) => a.type), ['editor.action.closeFile']); + assert.equal((await snapshot(db)).byPath['gone.py'], 'g\n'); + }); + 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' }]); From 146562b856a667dd27d37acc51e48014fe18a949 Mon Sep 17 00:00:00 2001 From: Brendon Thiede Date: Fri, 18 Sep 2026 17:15:00 -0400 Subject: [PATCH 2/3] fix: Address CodeRabbit review on live Pull and setup - reopenTabs puts focus back on the originally active tab even when it was an unaffected file, and reports block tabs that would not reopen as tabsNotReopened (logged by callers) instead of failing a verified write into a reload that could not restore them either. - drive-splice.mjs dismisses only the expected "Enable block coding" dialog (via its x button) and fails on any other dialog. - README: the Pull row no longer says every Pull reloads. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 4 ++-- README.md | 2 +- src/content.js | 3 +++ src/inject.js | 36 +++++++++++++++++++++++++----------- src/menu-panel.js | 7 ++++++- test/e2e/README.md | 5 ++++- test/e2e/drive-splice.mjs | 31 +++++++++++++++++++++++-------- test/inject.test.mjs | 36 ++++++++++++++++++++++++++++++++++++ 8 files changed, 100 insertions(+), 24 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 3cf9f96..815fa07 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -84,7 +84,7 @@ Pybricks wraps Dexie with `dexie-observable`, which records mutations in a hidde - with `deleteUnlisted` (Pull's full sync, the same semantics as `apply-files`), every other file gets `{type: 'fileStorage.action.deleteFile', path}`, after its tab (if open) is closed with `editor.action.closeFile`. That's the Explorer's order: deleting an open file fails as "in use", and closing also drops it from the remembered tabs. The close's own save can't race the delete, because both are Dexie read-write transactions on the same stores, which IndexedDB runs in creation order; - a **block** program open in a tab (old or new contents start with `# pybricks blocks file:`) is **closed, written through `fileStorage`, then reopened**, one tab at a time with the originally active tab last. The block editor keeps its own Blockly workspace, and nothing shows it reloads when the model is replaced underneath it; a stale workspace would later write the old program back. Close and reopen leaves it exactly where a page reload would. This isn't proven against a live Blockly workspace: that needs a licensed run of `drive-splice.mjs`, and without a licence the block editor never goes live. Without a licence, reopening a block tab brings back Pybricks' "Enable block coding" dialog, just as a reload restoring that tab did. -Files whose sha already matches get no action. `writeFilesLive()` then polls IndexedDB until every file's contents *and* sha256 match and every deleted path is gone, and resolves `{live: false, reason}` when there's no store, when anything throws (hashing, IndexedDB, the app's reducers), or when a tab won't close or nothing is confirmed within 5s. Closed block tabs are reopened either way (in a `finally`). It returns the same `{added, changed, deleted, unchanged}` summary as `apply-files`, and never rejects. **Callers must fall back to the raw write + reload on `live: false`** (`upsert-files`, or `apply-files` for Pull), so if an upstream change renames an action or moves the store, each feature 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. After a live write, whatever a reload used to refresh is refreshed in place. Pull renders the rescue notice at once (`renderRescueNotice`) and calls `menuPanel.refresh()` (re-reads files, manifest and splice report, keeping unsaved slot edits) and `fileListWatcher.refresh()` (re-reads the manifest and removes badges from files that are no longer protected). +Files whose sha already matches get no action. `writeFilesLive()` then polls IndexedDB until every file's contents *and* sha256 match and every deleted path is gone, and resolves `{live: false, reason}` when there's no store, when anything throws (hashing, IndexedDB, the app's reducers), or when a tab won't close or nothing is confirmed within 5s. Closed block tabs are reopened either way (in a `finally`), and focus then goes back to the originally active tab, even when that was an unaffected file, since reopening a tab activates it. A tab that won't reopen is reported as `tabsNotReopened` on a `live: true` result and logged by the callers. It isn't treated as a failed write: the files are verified, and the raw fallback's reload couldn't restore the tab either, because closing it already removed it from the remembered tabs. It returns the same `{added, changed, deleted, unchanged}` summary as `apply-files`, and never rejects. **Callers must fall back to the raw write + reload on `live: false`** (`upsert-files`, or `apply-files` for Pull), so if an upstream change renames an action or moves the store, each feature 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. After a live write, whatever a reload used to refresh is refreshed in place. Pull renders the rescue notice at once (`renderRescueNotice`) and calls `menuPanel.refresh()` (re-reads files, manifest and splice report, keeping unsaved slot edits) and `fileListWatcher.refresh()` (re-reads the manifest and removes badges from files that are no longer protected). 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. @@ -98,7 +98,7 @@ The ISOLATED scripts reach IndexedDB by `window.postMessage`-ing `inject.js` (`p | `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 new-program, Update-robot-setup, and as the menu Save's fallback. | -| `write-files-live` | `{files, deleteUnlisted?}` | `{live: true, dispatched, summary}` or `{live: false, reason}` | writes (and, with `deleteUnlisted`, deletes like `apply-files`) through the app's Redux store, so the running UI and open editor tabs see it with **no reload**; confirmed by IDB read-back. Used by Pull, menu Save, New program and Update robot setup. See "The dexie-observable gotcha". | +| `write-files-live` | `{files, deleteUnlisted?}` | `{live: true, dispatched, summary, tabsNotReopened?}` or `{live: false, reason}` | writes (and, with `deleteUnlisted`, deletes like `apply-files`) through the app's Redux store, so the running UI and open editor tabs see it with **no reload**; confirmed by IDB read-back. Used by Pull, menu Save, New program and Update robot setup. 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`. diff --git a/README.md b/README.md index fe94efc..a32c73d 100644 --- a/README.md +++ b/README.md @@ -51,7 +51,7 @@ Until a Client ID is set, `GITHUB_CLIENT_ID` is empty: **Sign in with GitHub** s | Action | What it does | |---|---| | Click **Commit** | Opens a message input under the button — **Enter** commits (blank message = timestamped default), **Escape** cancels. The extension fetches the fork's head, builds a commit from the editor's files, and pushes it. Button shows `✓ ↑` (committed and pushed), `no changes`, `setup needed` (extension not configured yet), or `error` (see the console). | -| Click **Pull** | The extension fetches the fork and applies its files into the editor. Button shows `↓ +N ~N -N` (added / changed / deleted), or `nothing to pull` when the fork has no commits on the configured branch yet (nothing is applied in that case). When anything changed, the page reloads so the editor picks up the new files. | +| Click **Pull** | The extension fetches the fork and applies its files into the editor. Button shows `↓ +N ~N -N` (added / changed / deleted), or `nothing to pull` when the fork has no commits on the configured branch yet (nothing is applied in that case). The editor updates in place without reloading the page, so the hub stays connected. (Only if the extension can't drive the editor does it fall back to reloading the page.) | ## Shared-code updates diff --git a/src/content.js b/src/content.js index 78e466f..274c739 100644 --- a/src/content.js +++ b/src/content.js @@ -498,6 +498,9 @@ async function pull(btn) { let goneUuids = []; if (live.live) { summary = live.summary; + if (live.tabsNotReopened) { + console.warn('[pybricks-git] Pull left some block-program tabs closed:', live.tabsNotReopened); + } } else { console.warn('[pybricks-git] live Pull unavailable, reloading instead:', live.reason); // The files apply-files is about to delete — their uuids must also diff --git a/src/inject.js b/src/inject.js index 662b2bb..9e0e5b8 100644 --- a/src/inject.js +++ b/src/inject.js @@ -325,13 +325,14 @@ async function liveWriteAttempt(store, files, deleteUnlisted, timeoutMs) { const { editor } = store.getState(); const plan = planLiveWrites({ files: wanted, before, openFileUuids: editor.openFileUuids, deleteUnlisted }); const openNow = () => store.getState().editor.openFileUuids; + let result; try { for (const uuid of plan.close) store.dispatch({ type: 'editor.action.closeFile', uuid }); const closed = await waitUntil( () => plan.close.every((u) => !openNow().includes(u)), Date.now() + timeoutMs, ); - if (!closed) return { live: false, reason: 'Pybricks did not close the affected tabs in time' }; + if (!closed) return (result = { live: false, reason: 'Pybricks did not close the affected tabs in time' }); for (const action of [...plan.writes, ...plan.deletes]) store.dispatch(action); @@ -345,30 +346,43 @@ async function liveWriteAttempt(store, files, deleteUnlisted, timeoutMs) { gone.every((p) => !byPath.has(p) && !shaByPath.has(p)) ); }, Date.now() + timeoutMs); - if (!confirmed) return { live: false, reason: 'Pybricks did not confirm the write in time' }; - return { + if (!confirmed) return (result = { live: false, reason: 'Pybricks did not confirm the write in time' }); + result = { live: true, dispatched: plan.writes.length + plan.deletes.length, summary: plan.summary, }; + return result; } finally { - // Best effort, success or not: bring back the block tabs we closed, - // one at a time so the originally active file ends up active again. - await reopenTabs(store, plan.reopen, editor.activeFileUuid, timeoutMs); + // Best effort, success or not: bring back the block tabs we closed and + // put focus back where it was. A tab that won't reopen is reported, + // not treated as a failed write: the files are written and verified, + // and the raw fallback's reload couldn't restore the tab either (the + // close already dropped it from Pybricks' remembered tabs). + const missing = await reopenTabs(store, plan.reopen, editor.activeFileUuid, timeoutMs); + if (missing.length && result && result.live) result.tabsNotReopened = missing; } } +// Reopens `uuids` one at a time, then restores `activeUuid` as the active tab +// (reopening a tab activates it, which would otherwise steal focus from an +// unaffected file). Resolves the uuids that did not reopen. async function reopenTabs(store, uuids, activeUuid, timeoutMs) { - const order = uuids.includes(activeUuid) - ? [...uuids.filter((u) => u !== activeUuid), activeUuid] - : uuids; - for (const uuid of order) { + const missing = []; + for (const uuid of uuids) { store.dispatch({ type: 'editor.action.activateFile', uuid }); - await waitUntil( + const reopened = await waitUntil( () => store.getState().editor.openFileUuids.includes(uuid), Date.now() + timeoutMs, ); + if (!reopened) missing.push(uuid); } + const { editor } = store.getState(); + if (uuids.length && activeUuid && editor.openFileUuids.includes(activeUuid) && editor.activeFileUuid !== activeUuid) { + store.dispatch({ type: 'editor.action.activateFile', uuid: activeUuid }); + await waitUntil(() => store.getState().editor.activeFileUuid === activeUuid, Date.now() + timeoutMs); + } + return missing; } async function readStores() { diff --git a/src/menu-panel.js b/src/menu-panel.js index 6551216..f534448 100644 --- a/src/menu-panel.js +++ b/src/menu-panel.js @@ -939,7 +939,12 @@ function makeMenuPanel(deps) { } catch (err) { live = { live: false, reason: err.message }; } - if (live.live) return { reloading: false }; + if (live.live) { + if (live.tabsNotReopened) { + console.warn('[pybricks-git] some block-program tabs were left closed:', live.tabsNotReopened); + } + return { reloading: false }; + } console.warn('[pybricks-git] live write unavailable, reloading instead:', live.reason); await pageRequest('upsert-files', { files }); await persist(true); diff --git a/test/e2e/README.md b/test/e2e/README.md index 2b425ca..5cf30b6 100644 --- a/test/e2e/README.md +++ b/test/e2e/README.md @@ -392,7 +392,10 @@ identical), `prog_differs.py` (one motor port changed → splices cleanly), and and is still active (open block programs are closed, written and reopened, never replaced under a live Blockly workspace). Without a licence, the reopened tab brings back the "Enable block coding" dialog (a reload restoring - the tab did too), so the driver dismisses it. Then it asserts the + the tab did too), so the driver dismisses it with its × button. Any *other* + dialog fails the run instead of being dismissed unseen. The × gets a DOM + click: Escape doesn't work because focus sits on ``, and a coordinate + click can land while the dialog is still animating in. Then it asserts the **snapshot-first rail** harness-side — the remote gained a `Before robot setup update` commit whose tree holds the **PRE-splice** `prog_differs.py` setup (compared by `setupSignature`, not bytes: opening the file made the editor diff --git a/test/e2e/drive-splice.mjs b/test/e2e/drive-splice.mjs index 7126fe0..91a78cf 100644 --- a/test/e2e/drive-splice.mjs +++ b/test/e2e/drive-splice.mjs @@ -357,6 +357,25 @@ async function main() { if (r.exceptionDetails) throw new Error('main eval threw: ' + (r.exceptionDetails.exception?.description || r.exceptionDetails.text)); return r.result.value; }; + // Dismisses Pybricks' "Enable block coding" dialog if it is up (no + // licence + a block tab opened). Any OTHER dialog fails the run rather + // than being Escaped away unseen. + const dismissLicenceDialog = async (after) => { + const title = await poll( + () => evalMain(`document.querySelector('.bp5-dialog .bp5-heading')?.textContent ?? ''`).then((t) => t || null), + { timeout: 3000, interval: 200, what: 'a dialog' }, + ).catch(() => null); + log(`dialog after ${after} =`, JSON.stringify(title)); + if (!title) return; + assert(/^Enable block coding$/.test(title.trim()), `only the expected licence dialog after ${after} (got "${title}")`); + // Its Close (×) button, not Escape: focus sits on , where + // Blueprint never sees the key. A DOM click, not a trusted one: the + // dialog animates in (the button measured 18px, then 30px), and a + // coordinate click landed mid-animation did nothing. This dismisses + // a third-party dialog; it isn't simulating the kid. + await evalMain(`document.querySelector('.bp5-dialog .bp5-dialog-close-button').click()`); + await poll(() => evalMain(`!document.querySelector('.bp5-dialog')`), { timeout: 5000, what: 'the licence dialog to close' }); + }; await trustedClick(await buttonRect('Pull')); const pullLabel = await poll( async () => { @@ -434,8 +453,9 @@ async function main() { what: 'prog_differs.py to open in a tab', }); // Without a licence Pybricks pops "Enable block coding" over a block - // file; dismiss it so it can't block the panel click. - for (const type of ['keyDown', 'keyUp']) await page.send('Input.dispatchKeyEvent', { type, windowsVirtualKeyCode: 27, key: 'Escape', code: 'Escape' }); + // file; dismiss it so it can't block the panel click. Only that dialog: + // anything else would be an error, and Escape would hide it. + await dismissLicenceDialog('opening prog_differs.py'); await clickSelector('[data-pybricks-git-update-setup]', 'Update robot setup button'); const updateStatus = await poll( () => evalIsolated(`(() => { const s=document.querySelector('[data-pybricks-git-status]'); return s && /^Updated|Couldn|Saved the snapshot/.test(s.textContent) ? s.textContent : null; })()`, false), @@ -451,12 +471,7 @@ async function main() { // Reopening the block tab brings back the licence dialog when there's // no licence — exactly what the old reload did when it restored the // tab. Dismiss it so it can't swallow later clicks. - const dialogTitle = await evalMain(`document.querySelector('.bp5-dialog .bp5-heading')?.textContent ?? null`); - log('dialog after Update =', JSON.stringify(dialogTitle)); - if (dialogTitle) { - for (const type of ['keyDown', 'keyUp']) await page.send('Input.dispatchKeyEvent', { type, windowsVirtualKeyCode: 27, key: 'Escape', code: 'Escape' }); - await poll(() => evalMain(`!document.querySelector('.bp5-dialog')`), { timeout: 5000, what: 'the dialog to close' }); - } + await dismissLicenceDialog('Update robot setup'); // -- Snapshot-first + report ---------------------------------------- step(8, 'Assert snapshot-first commit + splice report + editor outcomes'); diff --git a/test/inject.test.mjs b/test/inject.test.mjs index 3b06b35..de66c2f 100644 --- a/test/inject.test.mjs +++ b/test/inject.test.mjs @@ -576,6 +576,42 @@ describe('writeFilesLive', () => { assert.equal(store.editor.activeFileUuid, 'u-prog'); }); + test('reopening a block tab does not steal focus from an unaffected active file', async () => { + const db = await openPybricks(); + const BLOCKS = '# pybricks blocks file:{"blocks":{}}\n'; + await seed(db, [ + { path: 'prog.py', contents: BLOCKS + 'v1\n', uuid: 'u-prog' }, + { path: 'other.py', contents: 'o\n', uuid: 'u-other' }, + ]); + const store = fakeStore({ openFileUuids: ['u-prog', 'u-other'], activeFileUuid: 'u-other', db }); + const res = await writeFilesLive({ + files: [{ path: 'prog.py', contents: BLOCKS + 'v2\n' }], + rootEl: fakeRoot(store), + }); + assert.equal(res.live, true); + assert.deepEqual(new Set(store.editor.openFileUuids), new Set(['u-prog', 'u-other'])); + assert.equal(store.editor.activeFileUuid, 'u-other', 'focus went back to the unaffected file'); + }); + + test('a block tab that will not reopen is reported, and the write still counts as live', async () => { + const db = await openPybricks(); + const BLOCKS = '# pybricks blocks file:{"blocks":{}}\n'; + await seed(db, [{ path: 'prog.py', contents: BLOCKS + 'v1\n', uuid: 'u-prog' }]); + const store = fakeStore({ + openFileUuids: ['u-prog'], + // Everything works except reopening (activateFile is swallowed). + onDispatch: (a) => (a.type === 'editor.action.activateFile' ? undefined : actLikePybricks(a, db, store.editor)), + }); + const res = await writeFilesLive({ + files: [{ path: 'prog.py', contents: BLOCKS + 'v2\n' }], + rootEl: fakeRoot(store), + timeoutMs: 300, + }); + assert.equal(res.live, true, 'the files are written and verified — no reason to fall back'); + assert.deepEqual(res.tabsNotReopened, ['u-prog']); + assert.equal((await snapshot(db)).byPath['prog.py'], BLOCKS + 'v2\n'); + }); + test('closed block tabs are reopened even when the write is not confirmed', async () => { const db = await openPybricks(); const BLOCKS = '# pybricks blocks file:{"blocks":{}}\n'; From 2ff6760ed88e59f777c9a3b7c781a61d6c604573 Mon Sep 17 00:00:00 2001 From: Brendon Thiede Date: Fri, 18 Sep 2026 17:26:33 -0400 Subject: [PATCH 3/3] fix: Report unrestored focus, survive a failed panel refresh CodeRabbit re-review: - reopenTabs reports focus it could not put back as activeNotRestored (logged by callers) rather than failing a verified write into a reload; the tabs/focus to restore are captured by value up front. - Update robot setup and New program no longer fail when only the panel refresh after a successful live write fails: Update shows the report it has and re-enables its button; New program no longer says "Couldn't create it" for a file it created. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 4 ++-- src/content.js | 7 +++++-- src/inject.js | 25 ++++++++++++++++++------- src/menu-panel.js | 23 ++++++++++++++++++----- test/inject.test.mjs | 27 +++++++++++++++++++++++++++ 5 files changed, 70 insertions(+), 16 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 815fa07..0438324 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -84,7 +84,7 @@ Pybricks wraps Dexie with `dexie-observable`, which records mutations in a hidde - with `deleteUnlisted` (Pull's full sync, the same semantics as `apply-files`), every other file gets `{type: 'fileStorage.action.deleteFile', path}`, after its tab (if open) is closed with `editor.action.closeFile`. That's the Explorer's order: deleting an open file fails as "in use", and closing also drops it from the remembered tabs. The close's own save can't race the delete, because both are Dexie read-write transactions on the same stores, which IndexedDB runs in creation order; - a **block** program open in a tab (old or new contents start with `# pybricks blocks file:`) is **closed, written through `fileStorage`, then reopened**, one tab at a time with the originally active tab last. The block editor keeps its own Blockly workspace, and nothing shows it reloads when the model is replaced underneath it; a stale workspace would later write the old program back. Close and reopen leaves it exactly where a page reload would. This isn't proven against a live Blockly workspace: that needs a licensed run of `drive-splice.mjs`, and without a licence the block editor never goes live. Without a licence, reopening a block tab brings back Pybricks' "Enable block coding" dialog, just as a reload restoring that tab did. -Files whose sha already matches get no action. `writeFilesLive()` then polls IndexedDB until every file's contents *and* sha256 match and every deleted path is gone, and resolves `{live: false, reason}` when there's no store, when anything throws (hashing, IndexedDB, the app's reducers), or when a tab won't close or nothing is confirmed within 5s. Closed block tabs are reopened either way (in a `finally`), and focus then goes back to the originally active tab, even when that was an unaffected file, since reopening a tab activates it. A tab that won't reopen is reported as `tabsNotReopened` on a `live: true` result and logged by the callers. It isn't treated as a failed write: the files are verified, and the raw fallback's reload couldn't restore the tab either, because closing it already removed it from the remembered tabs. It returns the same `{added, changed, deleted, unchanged}` summary as `apply-files`, and never rejects. **Callers must fall back to the raw write + reload on `live: false`** (`upsert-files`, or `apply-files` for Pull), so if an upstream change renames an action or moves the store, each feature 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. After a live write, whatever a reload used to refresh is refreshed in place. Pull renders the rescue notice at once (`renderRescueNotice`) and calls `menuPanel.refresh()` (re-reads files, manifest and splice report, keeping unsaved slot edits) and `fileListWatcher.refresh()` (re-reads the manifest and removes badges from files that are no longer protected). +Files whose sha already matches get no action. `writeFilesLive()` then polls IndexedDB until every file's contents *and* sha256 match and every deleted path is gone, and resolves `{live: false, reason}` when there's no store, when anything throws (hashing, IndexedDB, the app's reducers), or when a tab won't close or nothing is confirmed within 5s. Closed block tabs are reopened either way (in a `finally`), and focus then goes back to the originally active tab, even when that was an unaffected file, since reopening a tab activates it. A tab that won't reopen is reported as `tabsNotReopened`, and focus that won't go back as `activeNotRestored`, on a `live: true` result; the callers log both. It isn't treated as a failed write: the files are verified, and the raw fallback's reload couldn't restore the tab either, because closing it already removed it from the remembered tabs. It returns the same `{added, changed, deleted, unchanged}` summary as `apply-files`, and never rejects. **Callers must fall back to the raw write + reload on `live: false`** (`upsert-files`, or `apply-files` for Pull), so if an upstream change renames an action or moves the store, each feature 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. After a live write, whatever a reload used to refresh is refreshed in place. Pull renders the rescue notice at once (`renderRescueNotice`) and calls `menuPanel.refresh()` (re-reads files, manifest and splice report, keeping unsaved slot edits) and `fileListWatcher.refresh()` (re-reads the manifest and removes badges from files that are no longer protected). 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. @@ -98,7 +98,7 @@ The ISOLATED scripts reach IndexedDB by `window.postMessage`-ing `inject.js` (`p | `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 new-program, Update-robot-setup, and as the menu Save's fallback. | -| `write-files-live` | `{files, deleteUnlisted?}` | `{live: true, dispatched, summary, tabsNotReopened?}` or `{live: false, reason}` | writes (and, with `deleteUnlisted`, deletes like `apply-files`) through the app's Redux store, so the running UI and open editor tabs see it with **no reload**; confirmed by IDB read-back. Used by Pull, menu Save, New program and Update robot setup. See "The dexie-observable gotcha". | +| `write-files-live` | `{files, deleteUnlisted?}` | `{live: true, dispatched, summary, tabsNotReopened?, activeNotRestored?}` or `{live: false, reason}` | writes (and, with `deleteUnlisted`, deletes like `apply-files`) through the app's Redux store, so the running UI and open editor tabs see it with **no reload**; confirmed by IDB read-back. Used by Pull, menu Save, New program and Update robot setup. 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`. diff --git a/src/content.js b/src/content.js index 274c739..c647097 100644 --- a/src/content.js +++ b/src/content.js @@ -498,8 +498,11 @@ async function pull(btn) { let goneUuids = []; if (live.live) { summary = live.summary; - if (live.tabsNotReopened) { - console.warn('[pybricks-git] Pull left some block-program tabs closed:', live.tabsNotReopened); + if (live.tabsNotReopened || live.activeNotRestored) { + console.warn('[pybricks-git] Pull could not fully restore the editor tabs:', { + tabsNotReopened: live.tabsNotReopened, + activeNotRestored: live.activeNotRestored, + }); } } else { console.warn('[pybricks-git] live Pull unavailable, reloading instead:', live.reason); diff --git a/src/inject.js b/src/inject.js index 9e0e5b8..055cb05 100644 --- a/src/inject.js +++ b/src/inject.js @@ -322,8 +322,9 @@ async function liveWriteAttempt(store, files, deleteUnlisted, timeoutMs) { files.map(async (f) => ({ path: f.path, contents: f.contents, sha: await sha256(f.contents) })), ); const before = await readStores(); - const { editor } = store.getState(); - const plan = planLiveWrites({ files: wanted, before, openFileUuids: editor.openFileUuids, deleteUnlisted }); + // Captured now, by value: the tabs and focus to plan around and restore. + const { openFileUuids, activeFileUuid } = store.getState().editor; + const plan = planLiveWrites({ files: wanted, before, openFileUuids, deleteUnlisted }); const openNow = () => store.getState().editor.openFileUuids; let result; try { @@ -359,14 +360,20 @@ async function liveWriteAttempt(store, files, deleteUnlisted, timeoutMs) { // not treated as a failed write: the files are written and verified, // and the raw fallback's reload couldn't restore the tab either (the // close already dropped it from Pybricks' remembered tabs). - const missing = await reopenTabs(store, plan.reopen, editor.activeFileUuid, timeoutMs); - if (missing.length && result && result.live) result.tabsNotReopened = missing; + const { missing, activeRestored } = await reopenTabs(store, plan.reopen, activeFileUuid, timeoutMs); + if (result && result.live) { + if (missing.length) result.tabsNotReopened = missing; + if (!activeRestored) result.activeNotRestored = activeFileUuid; + } } } // Reopens `uuids` one at a time, then restores `activeUuid` as the active tab // (reopening a tab activates it, which would otherwise steal focus from an -// unaffected file). Resolves the uuids that did not reopen. +// unaffected file). Resolves {missing: uuids that did not reopen, +// activeRestored: false only when focus could not be put back}. Neither is a +// failed write — callers report them rather than reload (a reload would drop +// the hub's Bluetooth link to fix a tab, and couldn't restore it anyway). async function reopenTabs(store, uuids, activeUuid, timeoutMs) { const missing = []; for (const uuid of uuids) { @@ -378,11 +385,15 @@ async function reopenTabs(store, uuids, activeUuid, timeoutMs) { if (!reopened) missing.push(uuid); } const { editor } = store.getState(); + let activeRestored = true; if (uuids.length && activeUuid && editor.openFileUuids.includes(activeUuid) && editor.activeFileUuid !== activeUuid) { store.dispatch({ type: 'editor.action.activateFile', uuid: activeUuid }); - await waitUntil(() => store.getState().editor.activeFileUuid === activeUuid, Date.now() + timeoutMs); + activeRestored = await waitUntil( + () => store.getState().editor.activeFileUuid === activeUuid, + Date.now() + timeoutMs, + ); } - return missing; + return { missing, activeRestored }; } async function readStores() { diff --git a/src/menu-panel.js b/src/menu-panel.js index f534448..f2091f9 100644 --- a/src/menu-panel.js +++ b/src/menu-panel.js @@ -435,7 +435,9 @@ function makeMenuPanel(deps) { setTimeout(() => reload(), 800); return; } - await refresh(); + // The file exists now; a failed panel refresh only means the + // programs list is stale — never report it as a failed create. + await refresh().catch((err) => console.warn('[pybricks-git] panel refresh after create failed:', err)); setStatus(`Created ${path} ✓`); } catch (err) { setStatus(`Couldn't create it: ${err.message}`); @@ -520,8 +522,16 @@ function makeMenuPanel(deps) { setTimeout(() => reload(), 800); } else if (updated.length) { // Written through the app: refresh in place. refresh() re-reads the - // persisted spliceReport, so the report block shows right away. - await refresh(); + // persisted spliceReport, so the report block shows right away. If + // the refresh fails, the programs are still updated — show the + // report we already have and keep the control usable. + try { + await refresh(); + } catch (err) { + console.warn('[pybricks-git] panel refresh after update failed:', err); + state.spliceReport = report; + render(); + } setStatus(`Updated ${updated.length} program(s) ✓`); if (btn) btn.disabled = false; } else { @@ -940,8 +950,11 @@ function makeMenuPanel(deps) { live = { live: false, reason: err.message }; } if (live.live) { - if (live.tabsNotReopened) { - console.warn('[pybricks-git] some block-program tabs were left closed:', live.tabsNotReopened); + if (live.tabsNotReopened || live.activeNotRestored) { + console.warn('[pybricks-git] could not fully restore the editor tabs:', { + tabsNotReopened: live.tabsNotReopened, + activeNotRestored: live.activeNotRestored, + }); } return { reloading: false }; } diff --git a/test/inject.test.mjs b/test/inject.test.mjs index de66c2f..f52c6cf 100644 --- a/test/inject.test.mjs +++ b/test/inject.test.mjs @@ -612,6 +612,33 @@ describe('writeFilesLive', () => { assert.equal((await snapshot(db)).byPath['prog.py'], BLOCKS + 'v2\n'); }); + test('focus that will not go back is reported, and the write still counts as live', async () => { + const db = await openPybricks(); + const BLOCKS = '# pybricks blocks file:{"blocks":{}}\n'; + await seed(db, [ + { path: 'prog.py', contents: BLOCKS + 'v1\n', uuid: 'u-prog' }, + { path: 'other.py', contents: 'o\n', uuid: 'u-other' }, + ]); + const store = fakeStore({ + openFileUuids: ['u-prog', 'u-other'], + activeFileUuid: 'u-other', + // Reopening u-prog works; re-activating u-other is swallowed. + onDispatch: (a) => + a.type === 'editor.action.activateFile' && a.uuid === 'u-other' + ? undefined + : actLikePybricks(a, db, store.editor), + }); + const res = await writeFilesLive({ + files: [{ path: 'prog.py', contents: BLOCKS + 'v2\n' }], + rootEl: fakeRoot(store), + timeoutMs: 300, + }); + assert.equal(res.live, true, 'files written and verified — focus alone is no reason to reload'); + assert.equal(res.activeNotRestored, 'u-other'); + assert.equal(res.tabsNotReopened, undefined); + assert.equal((await snapshot(db)).byPath['prog.py'], BLOCKS + 'v2\n'); + }); + test('closed block tabs are reopened even when the write is not confirmed', async () => { const db = await openPybricks(); const BLOCKS = '# pybricks blocks file:{"blocks":{}}\n';