Forget open tabs of files a Pull deletes

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 `<stem>_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. 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 `<stem>_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.<window.name>.<editorId>` (`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`.
 
 **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`.
 

README.md

@@ -68,7 +68,7 @@ When the mentor updates the upstream shared repository, each team pulls the chan
 
 In rough priority order:
 
-1. **Open-tab cleanup on delete** — when Pull deletes a file, also clean up its entry in Pybricks' "open tabs" state so the page doesn't log a non-fatal error after reload.
+Nothing queued right now.
 
 ## License
 

src/content.js

@@ -478,6 +478,9 @@ 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);
         btn.textContent = `↓ +${summary.added} ~${summary.changed} -${summary.deleted}`;
@@ -492,7 +495,22 @@ async function pull(btn) {
         // dexie-observable doesn't see raw IDB writes, so reload to refresh
         // the React UI. Brief delay so the user can see the summary.
         if (summary.added || summary.changed || summary.deleted) {
-            setTimeout(() => location.reload(), 1500);
+            setTimeout(() => {
+                // Prune right before the reload, not earlier: the page's
+                // in-memory tab history rewrites sessionStorage whenever a tab
+                // opens or closes, so an earlier prune could be undone.
+                try {
+                    const entries = Object.keys(sessionStorage).map((k) => [k, sessionStorage.getItem(k)]);
+                    for (const [key, value] of planTabPrunes(entries, goneUuids)) {
+                        sessionStorage.setItem(key, value);
+                    }
+                } catch (err) {
+                    // Storage can be unavailable; the cost is only Pybricks'
+                    // own "file not found" toast after the reload.
+                    console.warn('[pybricks-git] open-tab cleanup failed:', err);
+                }
+                location.reload();
+            }, 1500);
         } else {
             setTimeout(() => (btn.textContent = original), 3000);
         }

src/pullmerge.js

@@ -72,3 +72,55 @@ function planPull({ local, repo, base = {}, protectedPaths = [] }) {
     }
     return { files, rescued };
 }
+
+// --- Open-tab cleanup ---------------------------------------------------------
+//
+// Pybricks remembers open editor tabs in sessionStorage, as a JSON array of
+// file uuids under `editor.activeFileHistory.<window.name>.<editorId>`
+// (pybricks-code src/editor/lib.ts ActiveFileHistoryManager), and reopens each
+// on load. A uuid whose file a Pull deleted fails that reopen with an
+// "unexpected error" toast ("file with uuid '…' not found"). These helpers
+// find the uuids a Pull removes and plan pruning them from that history;
+// content.js does the sessionStorage reads and writes.
+
+const OPEN_TAB_HISTORY_PREFIX = 'editor.activeFileHistory.';
+
+// uuids of the editor's files that are absent from `keptPaths` — exactly what
+// apply-files deletes when handed a file set with those paths.
+//   metadata   [{path, uuid}]   the editor's metadata rows before the apply
+//   keptPaths  [path]           the paths passed to apply-files
+function deletedUuids(metadata, keptPaths) {
+    const kept = new Set(keptPaths);
+    return metadata.filter((m) => !kept.has(m.path)).map((m) => m.uuid);
+}
+
+// One history value with `uuids` removed. Returns the new JSON string, or null
+// when nothing changes (including a value that isn't a JSON array — Pybricks
+// itself treats that as empty, so it's left alone).
+function pruneTabHistory(value, uuids) {
+    let history;
+    try {
+        history = JSON.parse(value);
+    } catch {
+        return null;
+    }
+    if (!Array.isArray(history)) return null;
+    const drop = new Set(uuids);
+    const pruned = history.filter((u) => !drop.has(u));
+    return pruned.length === history.length ? null : JSON.stringify(pruned);
+}
+
+// Plans the rewrites for every open-tab history entry. `entries` is
+// [[key, value]] as read from sessionStorage by the caller (content.js does
+// the storage I/O; this stays pure). Returns [[key, newValue]] for just the
+// keys that change.
+function planTabPrunes(entries, uuids) {
+    if (!uuids.length) return [];
+    const writes = [];
+    for (const [key, value] of entries) {
+        if (typeof key !== 'string' || !key.startsWith(OPEN_TAB_HISTORY_PREFIX)) continue;
+        const next = pruneTabHistory(value, uuids);
+        if (next !== null) writes.push([key, next]);
+    }
+    return writes;
+}

test/e2e/README.md

@@ -79,15 +79,21 @@ have to rediscover them:
 8. **Merge on Pull.** Writes local edits straight into IndexedDB via
    `upsert-files` (a new `scratch.py`, an edited `starter.py`, an edited
    `coach.py`), pushes a competing commit to the bare repo
-   (`starter.py`/`keep.py`/`coach.py` changed, `gone.py` deleted), real-clicks
-   **Pull** again → asserts label `↓ +1 ~3 -1` → waits for reload → asserts the
+   (`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
    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.)
 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
@@ -138,6 +144,9 @@ 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 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
 [e2e] PASS: a protected file is overwritten with no rescue copy
 

test/e2e/drive.mjs

@@ -652,6 +652,46 @@ 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.
+        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 page.send('Page.addScriptToEvaluateOnNewDocument', { source: `
+            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 });
+        ` });
+        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;
+        for (const uuid of [keepUuid, goneUuid]) {
+            await evalMain(`findAppStore().dispatch({ type: 'editor.action.activateFile', uuid: ${JSON.stringify(uuid)} })`);
+            await poll(
+                () => evalMain(`findAppStore().getState().editor.openFileUuids.includes(${JSON.stringify(uuid)})`),
+                { timeout: 15000, what: 'a file to open in an editor tab' },
+            );
+        }
+        const tabHistory = () =>
+            evalMain(
+                `Object.keys(sessionStorage).filter((k) => k.startsWith('editor.activeFileHistory.')).flatMap((k) => JSON.parse(sessionStorage.getItem(k)))`,
+            );
+        const historyBefore = await tabHistory();
+        assert(
+            historyBefore.includes(goneUuid) && historyBefore.includes(keepUuid),
+            'gone.py and keep.py are open editor tabs before the Pull',
+        );
+
         await trustedClick(await buttonRect('Pull'));
         const mergeLabel = await poll(
             async () => {
@@ -703,6 +743,25 @@ async function main() {
             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.)
+        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)})`,
+        );
+        const historyAfter = await tabHistory();
+        assert(!historyAfter.includes(goneUuid), "the deleted gone.py left Pybricks' open-tab history");
+        assert(historyAfter.includes(keepUuid), 'the kept keep.py is still a remembered tab');
         const afterMerge = await evalIsolated(`pageRequest('list-files')`);
         const mergedPaths = afterMerge.contents.map((c) => c.path);
         const mergedBy = new Map(afterMerge.contents.map((c) => [c.path, c.contents]));

test/load-pullmerge.mjs

@@ -12,7 +12,7 @@ const srcPath = path.join(here, '..', 'src', 'pullmerge.js');
 export function loadPullMerge() {
     const src =
         readFileSync(srcPath, 'utf8') +
-        '\n;globalThis.__pybricksPullMergeTest = { rescueName, planPull };';
+        '\n;globalThis.__pybricksPullMergeTest = { rescueName, planPull, deletedUuids, pruneTabHistory, planTabPrunes };';
     // eslint-disable-next-line no-new-func
     new Function(src)();
     return globalThis.__pybricksPullMergeTest;

test/pullmerge.test.mjs

@@ -2,7 +2,7 @@ import { describe, test } from 'node:test';
 import assert from 'node:assert/strict';
 import { loadPullMerge } from './load-pullmerge.mjs';
 
-const { rescueName, planPull } = loadPullMerge();
+const { rescueName, planPull, deletedUuids, pruneTabHistory, planTabPrunes } = loadPullMerge();
 
 describe('rescueName', () => {
     test('inserts _mine before the extension', () => {
@@ -192,3 +192,43 @@ describe('planPull', () => {
         assert.deepEqual(rescued, []);
     });
 });
+
+describe('open-tab cleanup', () => {
+    test('deletedUuids names the files missing from the kept set', () => {
+        const meta = [
+            { path: 'a.py', uuid: 'u-a' },
+            { path: 'gone.py', uuid: 'u-gone' },
+            { path: 'b.py', uuid: 'u-b' },
+        ];
+        assert.deepEqual(deletedUuids(meta, ['a.py', 'b.py', 'new.py']), ['u-gone']);
+        assert.deepEqual(deletedUuids(meta, ['a.py', 'gone.py', 'b.py']), []);
+    });
+
+    test('pruneTabHistory drops deleted uuids and keeps order', () => {
+        assert.equal(pruneTabHistory('["u-a","u-gone","u-b"]', ['u-gone']), '["u-a","u-b"]');
+        assert.equal(pruneTabHistory('["u-gone"]', ['u-gone']), '[]');
+    });
+
+    test('pruneTabHistory returns null when nothing changes or the value is not an array', () => {
+        assert.equal(pruneTabHistory('["u-a"]', ['u-gone']), null);
+        assert.equal(pruneTabHistory('not json', ['u-gone']), null);
+        assert.equal(pruneTabHistory('{"u-gone":1}', ['u-gone']), null);
+        assert.equal(pruneTabHistory(null, ['u-gone']), null);
+    });
+
+    test('planTabPrunes rewrites only open-tab history keys that change', () => {
+        const entries = [
+            ['activities.selectedActivity', '"activity.explorer"'],
+            ['editor.activeFileHistory.win1.vs.editor.ICodeEditor:1', '["u-a","u-gone"]'],
+            ['editor.activeFileHistory.win1.vs.editor.ICodeEditor:2', '["u-b"]'],
+            ['other.key', '["u-gone"]'],
+        ];
+        assert.deepEqual(planTabPrunes(entries, ['u-gone']), [
+            ['editor.activeFileHistory.win1.vs.editor.ICodeEditor:1', '["u-a"]'],
+        ]);
+    });
+
+    test('planTabPrunes plans nothing with no deleted uuids', () => {
+        assert.deepEqual(planTabPrunes([['editor.activeFileHistory.w.e', '["u-a"]']], []), []);
+    });
+});