diff --git a/client/client.ts b/client/client.ts index 437caffa..af37335c 100644 --- a/client/client.ts +++ b/client/client.ts @@ -124,6 +124,9 @@ export class Client { // Track if plugs have been updated since sync cycle fullSyncCompleted = false; private versionMismatchNotified = false; + // True once we've confirmed the server reports the same publicVersion as + // this client bundle and the plugs the server is shipping are aligned with the running build. + private versionsInSync = false; // Set to true once the system is ready (plugs loaded) public systemReady: boolean = false; @@ -202,8 +205,11 @@ export class Client { ); // Seed the full-index readiness flag from persistent state so a - // warm reload doesn't unnecessarily flash loading spinners. - this.fullIndexCompleted = await this.objectIndex.hasFullIndexCompleted(); + // warm reload doesn't unnecessarily flash loading spinners. A stale + // stored version (older than `desiredIndexVersion`) still counts as + // ready: the entries are queryable while `ensureFullIndex` waits for + // sync + version confirmation. See `ObjectIndex.isIndexAvailable`. + this.fullIndexCompleted = await this.objectIndex.isIndexAvailable(); // After the initial index completes (object_index.ts dispatches this), // mark the flag and — if the page list cache previously took the @@ -303,6 +309,11 @@ export class Client { this.systemReady = true; this.maybeDispatchWidgetsReady(); + // When the service worker is disabled check for an up-to-date index immediately + if (this.bootConfig.disableServiceWorker) { + void this.objectIndex.ensureFullIndex(this.space); + } + this.initHeadlessRuntime(); // Load space snapshot and enable events @@ -545,10 +556,10 @@ export class Client { const result = await evalStatement(ast, scriptEnv, sf); const returnValue = result && - typeof result === "object" && - "ctrl" in result && - result.ctrl === "return" && - Array.isArray(result.values) + typeof result === "object" && + "ctrl" in result && + result.ctrl === "return" && + Array.isArray(result.values) ? result.values[0] : result; return (await Promise.resolve(luaValueToJS(returnValue, sf))) ?? null; @@ -594,13 +605,13 @@ export class Client { async updatePageListCache() { console.log("Updating page list cache"); - // Check if the initial sync has been completed - const initialIndexCompleted = - await this.objectIndex.hasFullIndexCompleted(); + // Use the looser `isIndexAvailable` check: stale-but-present index + // entries are still queryable, so we can take the index branch. + const indexAvailable = await this.objectIndex.isIndexAvailable(); let allPages: PageMeta[] = []; - if (initialIndexCompleted) { + if (indexAvailable) { console.log("Initial index complete, loading full page list via index."); // Fetch indexed pages allPages = await this.queryLuaObjects("page", {}); @@ -661,7 +672,7 @@ export class Client { // transform-applied values (the index branch). The fallback branch // produces raw page meta without pageDecoration, so we keep // showing loading widgets until the index is back. - if (initialIndexCompleted) { + if (indexAvailable) { this.pageListLoaded = true; this.maybeDispatchWidgetsReady(); } @@ -934,9 +945,9 @@ export class Client { console.warn("Not loading custom styles, since space style is disabled"); return; } - if (!(await this.objectIndex.hasFullIndexCompleted())) { + if (!(await this.objectIndex.isIndexAvailable())) { console.warn( - "Not loading custom styles, since full indexing has not completed yet", + "Not loading custom styles, since no index is available yet", ); return; } @@ -1014,9 +1025,13 @@ export class Client { case "space-sync-complete": { const isFirstSync = !this.fullSyncCompleted; this.fullSyncCompleted = true; - // Trigger version-bump reindex if needed (must happen here, not in a plug, - // because the plug may not be loaded yet when the first sync completes) - void this.objectIndex.ensureFullIndex(this.space); + // Only trigger a version-bump reindex once we've also confirmed the + // server is on the same publicVersion as this client — otherwise the + // reindex could run against stale plug code that's about to be + // replaced by the in-progress upgrade. + if (this.versionsInSync) { + void this.objectIndex.ensureFullIndex(this.space); + } if (isFirstSync && message.operations > 0) { // First sync pulled new content — reload the current page @@ -1047,10 +1062,15 @@ export class Client { break; } case "server-version": { - if ( - message.serverVersion !== publicVersion && - !this.versionMismatchNotified - ) { + if (message.serverVersion === publicVersion) { + const wasInSync = this.versionsInSync; + this.versionsInSync = true; + // If sync already completed before we learned versions were aligned, + // kick off the deferred reindex check now. + if (!wasInSync && this.fullSyncCompleted) { + void this.objectIndex.ensureFullIndex(this.space); + } + } else if (!this.versionMismatchNotified) { this.versionMismatchNotified = true; this.ui.flashNotification( "A new version of SilverBullet client is available. A reload or two is required to update.", diff --git a/client/client_system.ts b/client/client_system.ts index 1f1e2406..e5e3e8f7 100644 --- a/client/client_system.ts +++ b/client/client_system.ts @@ -193,9 +193,9 @@ export class ClientSystem { console.info("Space Lua scripts are disabled, skipping loading scripts"); return; } - if (!(await this.objectIndex.hasFullIndexCompleted())) { + if (!(await this.objectIndex.isIndexAvailable())) { console.info( - "Not loading space scripts, since full indexing has not completed yet", + "Not loading space scripts, since no index is available yet", ); return; } diff --git a/client/data/object_index.ts b/client/data/object_index.ts index 09e46853..de931009 100644 --- a/client/data/object_index.ts +++ b/client/data/object_index.ts @@ -30,7 +30,7 @@ const pageKey = "ridx"; const indexVersionKey = ["$indexVersion"]; // Bump this one every time a full reindex is needed -const desiredIndexVersion = 9; +const desiredIndexVersion = 10; type TagDefinition = { tagPage?: string; @@ -77,9 +77,9 @@ export class ObjectIndex { indexStarted = true; }); - // Handle initial index completion - void this.hasFullIndexCompleted().then((hasCompleted) => { - if (!hasCompleted) { + // Handle initial index completion for fresh installs only. + void this.getCurrentIndexVersion().then((currentVersion) => { + if (currentVersion === undefined) { const emptyQueueHandler = async () => { console.log("Index queue empty, checking if index is complete"); // Theoretically we could get empty queue notifications before the file:listed event has been triggered, so let's account for this @@ -328,20 +328,39 @@ export class ObjectIndex { return this.ds.get([indexKey, tag, this.cleanKey(ref, page), page]); } + private reindexingForVersionBump = false; + async ensureFullIndex(space: Space) { const currentIndexVersion = await this.getCurrentIndexVersion(); - if (!currentIndexVersion) { + if (currentIndexVersion === undefined) { console.log("No index version found, assuming fresh install"); return; } - if ( - // If the index version is less than the desired version - currentIndexVersion < desiredIndexVersion && - // And the index queue is empty (meaning no indexing is ongoing) - (await this.mq.isQueueEmpty("indexQueue")) - ) { + if (currentIndexVersion >= desiredIndexVersion) { + return; + } + + // Guard against concurrent invocations (e.g. one from + // `space-sync-complete` and another from a follow-up `server-version` + // SW message) — only one version-bump reindex should run at a time. + if (this.reindexingForVersionBump) { + return; + } + this.reindexingForVersionBump = true; + try { + // Wait out any indexing currently in flight (e.g. just-synced files + // being indexed) so we don't fight the worker over the queue. + await this.mq.awaitEmptyQueue("indexQueue"); + + // Re-read the version in case a concurrent path already bumped it + // while we were waiting. + const versionNow = await this.getCurrentIndexVersion(); + if (versionNow !== undefined && versionNow >= desiredIndexVersion) { + return; + } + console.info( "[index]", "Performing a full space reindex, this could take a while...", @@ -353,6 +372,8 @@ export class ObjectIndex { // Dispatch an editor:reloadState event to reload the editor state (render widgets etc.) void this.eventHook.dispatchEvent("editor:reloadState"); + } finally { + this.reindexingForVersionBump = false; } } @@ -379,8 +400,17 @@ export class ObjectIndex { return (await this.ds.get(indexVersionKey)) >= desiredIndexVersion; } - private getCurrentIndexVersion() { - return this.ds.get(indexVersionKey); + public getCurrentIndexVersion(): Promise { + return this.ds.get(indexVersionKey) as Promise; + } + + /** + * True once any full indexing pass has ever completed for this + * space, regardless of whether the stored version matches the current + * `desiredIndexVersion`. + */ + public async isIndexAvailable(): Promise { + return (await this.getCurrentIndexVersion()) !== undefined; } async awaitIndexQueueDrain(): Promise { diff --git a/e2e/fixtures.ts b/e2e/fixtures.ts index 193a61b7..8841f6e3 100644 --- a/e2e/fixtures.ts +++ b/e2e/fixtures.ts @@ -18,9 +18,9 @@ export type SBServer = { type SBFixtures = { spaceFiles: Record; + disableServiceWorker: boolean; sbServer: SBServer; sbPage: Page; - sbPageWithSync: Page; }; async function getFreePort(): Promise { @@ -50,8 +50,9 @@ async function waitForServer(url: string, timeoutMs = 30_000): Promise { export const test = base.extend({ spaceFiles: [{}, { option: true }], + disableServiceWorker: [true, { option: true }], - sbServer: async ({ spaceFiles }, use) => { + sbServer: async ({ spaceFiles, disableServiceWorker }, use) => { const spaceDir = await mkdtemp(join(tmpdir(), "sb-e2e-")); // Seed space with files @@ -69,6 +70,12 @@ export const test = base.extend({ { cwd: join(import.meta.dirname, ".."), stdio: ["ignore", "pipe", "pipe"], + env: { + ...process.env, + ...(disableServiceWorker + ? { SB_DISABLE_SERVICE_WORKER: "1" } + : {}), + }, }, ); @@ -117,18 +124,12 @@ export const test = base.extend({ await gotoSilverBulletPage(page, sbServer); await use(page); }, - - sbPageWithSync: async ({ sbServer, page }, use) => { - await page.goto(sbServer.url); - await page.locator("#sb-editor .cm-editor").waitFor({ state: "visible", timeout: 30_000 }); - await use(page); - }, }); /** * Navigate to a SilverBullet page in the test space and wait for the editor - * to be visible. Disables the service worker (`?enableSW=0`) to match how the - * `sbPage` fixture boots, so the test environment is consistent across tests. + * to be visible. The test server runs with `SB_DISABLE_SERVICE_WORKER=1`, so + * the boot path skips the service worker for deterministic test behavior. * * `pagePath` is the SilverBullet page name without the `.md` extension. Pass * an empty string (the default) to land on the index page. Each path segment @@ -140,7 +141,7 @@ export async function gotoSilverBulletPage( pagePath = "", ): Promise { const encoded = pagePath.split("/").map(encodeURIComponent).join("/"); - await page.goto(`${sbServer.url}/${encoded}?enableSW=0&headless=1`); + await page.goto(`${sbServer.url}/${encoded}?headless=1`); await page.locator("#sb-editor .cm-editor").waitFor({ state: "visible", timeout: 30_000 }); await waitForEditorReady(page); } diff --git a/e2e/index-upgrade.test.ts b/e2e/index-upgrade.test.ts new file mode 100644 index 00000000..3a9d174b --- /dev/null +++ b/e2e/index-upgrade.test.ts @@ -0,0 +1,255 @@ +import type { Page } from "@playwright/test"; +import { expect, gotoSilverBulletPage, test } from "./fixtures.ts"; + +/** + * Tests for the `desiredIndexVersion` upgrade path. + * + * `client/data/object_index.ts` carries a `desiredIndexVersion` constant. + * Bumping it should cause the next-loaded client to run a full space + * reindex, but only once the client is sure it is on the same build as + * the server (and, for the SW-enabled path, the initial space sync has + * completed). These tests simulate "an old client booted after the + * server upgrade" by writing a stale value into the local datastore + * before reloading, then verifying that a real reindex actually fires + * (not just that the stored version number bumps — see the "Performing + * a full space reindex" log line). + */ + +async function readIndexVersion(page: Page): Promise { + return await page.evaluate(async () => { + const client = (globalThis as any).client; + if (!client) return undefined; + return (await client.ds.get(["$indexVersion"])) as number | undefined; + }); +} + +async function writeIndexVersion(page: Page, version: number): Promise { + await page.evaluate( + async (v) => { + const client = (globalThis as any).client; + if (!client) throw new Error("client not initialized"); + await client.ds.set(["$indexVersion"], v); + }, + version, + ); +} + +async function waitForIndexVersionAtLeast( + page: Page, + min: number, + timeoutMs = 30_000, +): Promise { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + const v = await readIndexVersion(page); + if (typeof v === "number" && v >= min) return; + await new Promise((r) => setTimeout(r, 250)); + } + const final = await readIndexVersion(page); + throw new Error( + `Index version did not reach ${min} within ${timeoutMs}ms (last value: ${String(final)})`, + ); +} + +async function waitForEditor(page: Page): Promise { + await page.locator("#sb-editor .cm-editor").waitFor({ state: "visible", timeout: 30_000 }); +} + +async function readFullIndexCompleted(page: Page): Promise { + return await page.evaluate(() => { + const client = (globalThis as any).client; + if (!client) return undefined; + return client.fullIndexCompleted as boolean; + }); +} + +/** + * Polls the live widget render-mode signals on the client. The + * `widgetRenderMode` helper (see client/codemirror/util.ts) returns + * "loading" whenever any of `systemReady`, `clientSystem.scriptsLoaded`, + * `fullIndexCompleted` or `pageListLoaded` is false; this captures the + * same set so a stale-version reload can prove the widgets don't drop + * back into loading mode. + */ +async function waitForWidgetsReady(page: Page, timeoutMs = 15_000): Promise { + const deadline = Date.now() + timeoutMs; + let last: unknown = undefined; + while (Date.now() < deadline) { + last = await page.evaluate(() => { + const client = (globalThis as any).client; + if (!client) return null; + return { + systemReady: !!client.systemReady, + scriptsLoaded: !!client.clientSystem?.scriptsLoaded, + fullIndexCompleted: !!client.fullIndexCompleted, + pageListLoaded: !!client.pageListLoaded, + }; + }); + if ( + last && + typeof last === "object" && + (last as Record).systemReady && + (last as Record).scriptsLoaded && + (last as Record).fullIndexCompleted && + (last as Record).pageListLoaded + ) { + return; + } + await new Promise((r) => setTimeout(r, 100)); + } + throw new Error( + `Widgets did not reach the ready state within ${timeoutMs}ms (last state: ${JSON.stringify(last)})`, + ); +} + +/** + * Capture all `console` messages the page emits. The returned `messages` + * array is updated live; callers can check for substrings after the + * behavior of interest has had a chance to run. + */ +function captureConsole(page: Page): { messages: string[] } { + const messages: string[] = []; + page.on("console", (msg) => { + messages.push(msg.text()); + }); + return { messages }; +} + +const REINDEX_LOG = "Performing a full space reindex"; + +async function waitForReindexLog( + consoleState: { messages: string[] }, + timeoutMs = 30_000, +): Promise { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (consoleState.messages.some((m) => m.includes(REINDEX_LOG))) return; + await new Promise((r) => setTimeout(r, 100)); + } + const tail = consoleState.messages.slice(-50).join("\n "); + throw new Error( + `Did not observe a "${REINDEX_LOG}" log line within ${timeoutMs}ms.\nLast console output:\n ${tail}`, + ); +} + +/** Marker we embed in a `space-style` block to look for in the DOM. */ +const SPACE_STYLE_MARKER = "stale-reindex-marker-color: rebeccapurple"; + +async function customStylesContent(page: Page): Promise { + return (await page.locator("#custom-styles").innerHTML()) ?? ""; +} + +test.describe("index version upgrade (service worker disabled)", () => { + test.use({ + spaceFiles: { + "index.md": "# Index\nFirst page in the upgrade-test space.", + "Other.md": "# Other\nSee [[index]] for the entry point.", + "Styles.md": + "# Styles\n\n```space-style\n/* " + SPACE_STYLE_MARKER + " */\n```\n", + }, + }); + + test("stale index version triggers a real reindex on next boot", async ({ sbServer, page }) => { + const consoleState = captureConsole(page); + + // First boot: let the initial indexing run to completion so we know + // what value the current build considers "desired". + await gotoSilverBulletPage(page, sbServer); + await waitForEditor(page); + await waitForIndexVersionAtLeast(page, 1); + + const baseline = await readIndexVersion(page); + expect(typeof baseline).toBe("number"); + expect(baseline).toBeGreaterThan(0); + // First boot is a fresh install (no stored version), so there + // should be NO "Performing a full space reindex" log line yet. + expect(consoleState.messages.some((m) => m.includes(REINDEX_LOG))).toBe(false); + + // Simulate "this client was previously running an older build that + // stamped an older desiredIndexVersion into the datastore". + await writeIndexVersion(page, 1); + expect(await readIndexVersion(page)).toBe(1); + + // Re-navigate (the SW-disabled path runs the reindex check at the + // end of `Client.init()`). + await gotoSilverBulletPage(page, sbServer); + await waitForEditor(page); + + // A stale-but-present index should not flip widgets into the + // loading/spinner mode at boot — the existing entries are still + // queryable until the actual reindex starts. (Before this fix the + // boot-time flag was set from `hasFullIndexCompleted()`, which is + // `stored >= desired` and therefore false during the wait, and + // `updatePageListCache` would take the fallback branch and leave + // `pageListLoaded` false.) + expect(await readFullIndexCompleted(page)).toBe(true); + await waitForWidgetsReady(page); + // Custom styles must also load from the stale-but-present index — + // before this was wired up, `loadCustomStyles` bailed out on the + // strict `hasFullIndexCompleted` check and the `#custom-styles` + // element stayed empty. + expect(await customStylesContent(page)).toContain(SPACE_STYLE_MARKER); + + // Critical assertion: a real reindex must actually have run, not + // just the stored version having been silently bumped. + await waitForReindexLog(consoleState); + await waitForIndexVersionAtLeast(page, baseline as number); + expect(await readIndexVersion(page)).toBe(baseline); + }); +}); + +test.describe("index version upgrade (service worker enabled)", () => { + // SW + IndexedDB interactions are flaky in Firefox's Playwright + // implementation; mirror what pwa-offline does. + test.skip(({ browserName }) => browserName !== "chromium", "SW path only runs on Chromium"); + test.describe.configure({ retries: 2 }); + + test.use({ + disableServiceWorker: false, + spaceFiles: { + "index.md": "# Index\nFirst page in the SW-enabled upgrade test.", + "Other.md": "# Other\nLink back to [[index]].", + "Styles.md": + "# Styles\n\n```space-style\n/* " + SPACE_STYLE_MARKER + " */\n```\n", + }, + }); + + test("stale index version triggers a real reindex after sync + version match", async ({ sbServer, page }) => { + const consoleState = captureConsole(page); + + // First boot with the SW active: navigate without `?headless=1` so + // the service worker actually registers. + await page.goto(sbServer.url); + await waitForEditor(page); + await waitForIndexVersionAtLeast(page, 1); + + const baseline = await readIndexVersion(page); + expect(typeof baseline).toBe("number"); + expect(baseline).toBeGreaterThan(0); + expect(consoleState.messages.some((m) => m.includes(REINDEX_LOG))).toBe(false); + + await writeIndexVersion(page, 1); + expect(await readIndexVersion(page)).toBe(1); + + // Re-navigate. The SW-enabled path triggers the reindex once both + // `space-sync-complete` and a matching `server-version` message + // have been observed. + await page.goto(sbServer.url); + await waitForEditor(page); + + // Stale-but-present index should keep widgets in their ready state + // during the wait for sync + version-match (see the SW-disabled + // test for the rationale). + expect(await readFullIndexCompleted(page)).toBe(true); + await waitForWidgetsReady(page); + // Custom styles must also load from the stale-but-present index — + // before this was wired up, `loadCustomStyles` bailed out on the + // strict `hasFullIndexCompleted` check and the `#custom-styles` + // element stayed empty. + expect(await customStylesContent(page)).toContain(SPACE_STYLE_MARKER); + + await waitForReindexLog(consoleState); + await waitForIndexVersionAtLeast(page, baseline as number); + expect(await readIndexVersion(page)).toBe(baseline); + }); +}); diff --git a/e2e/pwa-offline.test.ts b/e2e/pwa-offline.test.ts index 74c70be3..829c5388 100644 --- a/e2e/pwa-offline.test.ts +++ b/e2e/pwa-offline.test.ts @@ -78,6 +78,8 @@ test.describe("PWA offline support", () => { test.describe.configure({ retries: 2 }); test.use({ + // This suite exercises real SW behavior, so it needs the SW running. + disableServiceWorker: false, spaceFiles: { "index.md": "# Offline Test Space\nThis content should survive offline.", "TestPage.md": "# Test Page\nOffline page content here.", diff --git a/website/Architecture.md b/website/Architecture.md index a9e70b5e..d235f7a9 100644 --- a/website/Architecture.md +++ b/website/Architecture.md @@ -90,8 +90,6 @@ It does this by intercepting HTTP calls coming from the client aimed at the serv The service worker embeds a [[Sync]] engine, that based on configuration constantly keeps a local copy of your files in sync with the server. To make the sync status visible, it emits events to the Client. -For debugging purposes you can disable the service worker by adding `?enableSW=0` to your URL. This disabling is persistent, to re-enable it use `?enableSW=1`. - # Server The server has only three jobs: