From 2652d6fa817eda6a076947af4da3b5d484ce9501 Mon Sep 17 00:00:00 2001 From: Istvan Matejcsok <119620946+matejcsok-ee@users.noreply.github.com> Date: Thu, 16 Jul 2026 13:56:27 +0200 Subject: [PATCH] tool-switch: create the missing counterpart file instead of silently no-opping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tools → "Switch to PCB Editor" in a project with no .kicad_pcb (e.g. created from a lone schematic) did nothing: the nav hook found no file for the target tool and returned false. Now a session that can persist (ToolPage passes the new createFile prop) writes the templated counterpart via createProjectFileIfMissing — no download fallback, and it re-checks existence so a collaborator's file is never clobbered — then navigates to it, matching native KiCad (pcbnew opens a new board at the derived path). Sessions that can't persist (read-only viewers, scratch/local-folder) keep the logged no-op. Also latch the quit dispatcher off before every deliberate tool-switch navigation (markDeliberateNavigation): the wx port's UnloadCallback runs on BEFOREUNLOAD and closes the top frame the moment the navigation starts, so the quit hook history.back()'d over the in-flight navigation — the pagehide latch is too late (it only fires at commit time). e2e: new tests/web/tool-switch-missing-file.spec.ts reproduces the flow via a browser-local (IDB) project created from the home page; playwright-web config gains VITE_LOCAL_PROJECTS=idb and derives STANDALONE_PORT/CORS_ORIGIN from WEB_APP_URL (runs the suite past a squatted :3048); both vars declared in web/turbo.json globalEnv (turbo strict-env strips undeclared vars). Includes the previously-uncommitted tool-switch spec repairs (URL grammar + z-30 boot-overlay wait). Co-Authored-By: Claude Fable 5 --- tests/playwright-web.config.ts | 22 +++++- tests/web/tool-switch-missing-file.spec.ts | 79 +++++++++++++++++++++ tests/web/tool-switch.spec.ts | 15 ++-- web/standalone/src/components/WasmTool.tsx | 80 +++++++++++++++++++++- web/standalone/src/lib/api.ts | 21 ++++++ web/standalone/src/pages/ToolPage.tsx | 6 ++ web/turbo.json | 2 + 7 files changed, 215 insertions(+), 10 deletions(-) create mode 100644 tests/web/tool-switch-missing-file.spec.ts diff --git a/tests/playwright-web.config.ts b/tests/playwright-web.config.ts index cd78356..a5b83a6 100644 --- a/tests/playwright-web.config.ts +++ b/tests/playwright-web.config.ts @@ -6,7 +6,7 @@ import { defineConfig, devices } from '@playwright/test'; * playwright-kicad.config.ts which drives the standalone tool harness HTMLs * under tests/apps/kicad. * - * These tests exercise the real web open paths: navigate to /p///, + * These tests exercise the real web open paths: navigate to /:scope/projects/:name/, * let WasmTool boot the tool in-document (boot.ts), drive the project into MEMFS * and auto-open the file (open-flow.ts via Module.kicadOpenFile), and assert the * editor loaded wizard-free. @@ -21,6 +21,15 @@ import { defineConfig, devices } from '@playwright/test'; */ const FRONTEND_URL = process.env.WEB_APP_URL ?? 'http://localhost:3048'; +// Keep the cold-started stack consistent with WEB_APP_URL overrides (e.g. a +// sibling worktree squatting :3048): vite binds the URL's port and the backend +// allows that origin, so overriding one env var relocates the whole frontend. +const FRONTEND_PORT = new URL(FRONTEND_URL).port || '3048'; +// Backend counterpart — same override story for :3060 squatters. BACKEND_URL +// is the var global-setup-web.ts already probes; the cold-started reference +// backend binds its port and the editor is pointed at it. +const BACKEND_URL = process.env.BACKEND_URL ?? 'http://localhost:3060'; +const BACKEND_PORT = new URL(BACKEND_URL).port || '3060'; export default defineConfig({ globalSetup: './web/global-setup-web.ts', @@ -73,8 +82,15 @@ export default defineConfig({ ...process.env, // resolved against web/backend/ (the backend's cwd) PROJECT_DIR: '../../tests/fixtures/demo', - VITE_API_BASE_URL: 'http://localhost:3060', - CORS_ORIGIN: 'http://localhost:3048', + PORT: BACKEND_PORT, + VITE_API_BASE_URL: BACKEND_URL, + CORS_ORIGIN: FRONTEND_URL, + STANDALONE_PORT: FRONTEND_PORT, + // The missing-file tool-switch spec builds a browser-local (IDB) project + // through the home page — same flag the dev/demo stacks set + // (scripts/dev-gpl.mjs). Only effective on cold starts: with + // reuseExistingServer an already-running stack must have set it itself. + VITE_LOCAL_PROJECTS: 'idb', }, }, }); diff --git a/tests/web/tool-switch-missing-file.spec.ts b/tests/web/tool-switch-missing-file.spec.ts new file mode 100644 index 0000000..835f1b3 --- /dev/null +++ b/tests/web/tool-switch-missing-file.spec.ts @@ -0,0 +1,79 @@ +import { test, expect, type Page } from '@playwright/test'; +import { clickMenuBarItem, clickMenuItemByText, stableShot } from '../e2e/utils/element-tracker'; + +/** + * Tool-switch with a MISSING counterpart: eeschema Tools → "Switch to PCB + * Editor" in a project that has no .kicad_pcb must CREATE the templated board + * and navigate to it (native KiCad opens pcbnew on a new board at the derived + * path) — not silently no-op (the old behavior: the WasmTool nav hook logged + * "[nav] no project file found" and returned false). + * + * The backend demo fixture carries both files, so this spec builds a + * schematic-only project the way a user does: home page Tools grid → + * "Schematic Editor" → NewFileDialog (project "Untitled" → slug "untitled", + * file "main.kicad_sch") → a browser-local IndexedDB project under the @local + * scope. That flow only renders when the standalone runs with + * VITE_LOCAL_PROJECTS=idb (playwright-web.config.ts sets it for cold starts; + * a hand-started stack must set it too — the dialog assertion below names the + * flag so a mis-configured stack fails self-diagnosingly). A fresh Playwright + * context has an empty IDB, so the slug is deterministically "untitled". + */ + +async function waitForToolReady(page: Page, titleRe: RegExp): Promise { + await expect(page.locator('#canvas')).toBeVisible({ timeout: 120000 }); + await expect + .poll(() => page.title(), { + message: `editor never reached title ${titleRe}`, + timeout: 120000, + intervals: [1000], + }) + .toMatch(titleRe); + // The menu helpers drive the rendered-element registry. + await page.waitForFunction( + () => + !!(window as unknown as { wxElementRegistry?: { findAllRendered?: unknown } }) + .wxElementRegistry, + null, + { timeout: 30000 } + ); + // The boot and eager-library overlays (WasmTool, `absolute inset-0 z-30`) + // cover the whole editor including the menubar — synthetic menu clicks land + // on them until they clear (eeschema hydrates the full symbol set post-boot). + await expect(page.locator('div.absolute.inset-0.z-30')).toHaveCount(0, { + timeout: 180000, + }); +} + +test.describe('web app — tool switch creates the missing counterpart', () => { + test('eeschema → Switch to PCB Editor creates and opens main.kicad_pcb', async ({ page }) => { + test.setTimeout(420000); // two full wasm boots + + // Home → Tools grid: with the local store on, launching a document tool + // opens the new-file dialog (HomePage onLaunch → setNewFileTool). + await page.goto('/'); + await page.getByRole('button', { name: 'Schematic Editor', exact: true }).click(); + await expect( + page.locator('#newfile-projectname'), + 'new-file dialog must open — is the stack running with VITE_LOCAL_PROJECTS=idb?' + ).toBeVisible(); + // The defaults are what the URL assertions below rely on (and their + // presence doubles as a dialog-hydrated wait). + await expect(page.locator('#newfile-projectname')).toHaveValue('Untitled'); + await expect(page.locator('#newfile-name')).toHaveValue('main.kicad_sch'); + await page.getByRole('button', { name: 'Create & open' }).click(); + + // NewFileDialog persists to IDB, then hard-navigates to the file route. + await page.waitForURL(/\/@local\/projects\/untitled\/main\.kicad_sch/, { timeout: 30000 }); + await waitForToolReady(page, /main — Schematic Editor/i); + + // The switch. Before the fix nothing happens (no navigation, no dialog), + // so the waitForURL below is where this spec fails. + expect(await clickMenuBarItem(page, 'Tools'), 'Tools menubar item clickable').toBe(true); + await clickMenuItemByText(page, 'Switch to PCB Editor'); + + await page.waitForURL(/\/@local\/projects\/untitled\/main\.kicad_pcb/, { timeout: 30000 }); + await waitForToolReady(page, /main — PCB Editor/i); + + await stableShot(page, 'web-switch-missing-pcb-created.png'); + }); +}); diff --git a/tests/web/tool-switch.spec.ts b/tests/web/tool-switch.spec.ts index 82b1851..83018ca 100644 --- a/tests/web/tool-switch.spec.ts +++ b/tests/web/tool-switch.spec.ts @@ -8,8 +8,9 @@ import { clickMenuBarItem, clickMenuItemByText, stableShot } from '../e2e/utils/ * Native KiCad spawns a process for this via ExecuteFile (common/gestfich.cpp); * the WASM build delegates to window.kicadWebOpenTool (WasmTool.tsx), which * maps the MEMFS file path to the project-relative file and calls - * location.assign(/p/demo//). Each direction is a full page - * navigation followed by a fresh wasm boot — hence the generous timeouts. + * location.assign(/:scope/projects/:name/) — the tool is inferred from + * the file extension. Each direction is a full page navigation followed by a + * fresh wasm boot — hence the generous timeouts. */ async function waitForToolReady(page: Page, titleRe: RegExp): Promise { @@ -29,6 +30,12 @@ async function waitForToolReady(page: Page, titleRe: RegExp): Promise { null, { timeout: 30000 } ); + // The boot and eager-library overlays (WasmTool, `absolute inset-0 z-30`) + // cover the whole editor including the menubar — synthetic menu clicks land + // on them until they clear (eeschema hydrates the full symbol set post-boot). + await expect(page.locator("div.absolute.inset-0.z-30")).toHaveCount(0, { + timeout: 180000, + }); } async function switchTool( @@ -54,7 +61,7 @@ test.describe('web app — tool switching', () => { await switchTool( page, 'Switch to PCB Editor', - /\/p\/demo\/pcbnew\/demo\.kicad_pcb/, + /\/default\/projects\/demo\/demo\.kicad_pcb/, /demo — PCB Editor/i ); @@ -70,7 +77,7 @@ test.describe('web app — tool switching', () => { await switchTool( page, 'Switch to Schematic Editor', - /\/p\/demo\/eeschema\/demo\.kicad_sch/, + /\/default\/projects\/demo\/demo\.kicad_sch/, /demo — Schematic Editor/i ); diff --git a/web/standalone/src/components/WasmTool.tsx b/web/standalone/src/components/WasmTool.tsx index 0b04ad3..c8b8b2a 100644 --- a/web/standalone/src/components/WasmTool.tsx +++ b/web/standalone/src/components/WasmTool.tsx @@ -24,6 +24,7 @@ import { yjsProviderConfig, type DocSource, } from "@/lib/config"; +import { defaultFileName, newFileTemplate, withExtension } from "@/lib/new-file"; import { bootKicadTool } from "@/wasm/boot"; import { resolveWasmBase } from "@/wasm/wasm-assets"; import { @@ -231,9 +232,16 @@ function installToolNavigationHook( slug: string; files: ToolFile[]; targetPath?: string; + /** Persist a new file into the project (see the WasmTool prop). Absent ⇒ + * this session can't create one, and a missing target stays a no-op. */ + createFile?: (relPath: string, bytes: Uint8Array) => Promise; log: (m: string) => void; }, ): () => void { + // One create at a time: a double-fired menu item must not upload twice. + // Cleared only on failure — success navigates the page away. + let pendingCreate: string | null = null; + const hook = (rawToolName: string, rawFileName: string): boolean => { const nextTool = normalizeToolName(rawToolName); @@ -248,8 +256,48 @@ function installToolNavigationHook( : chooseToolFile(opts.files, nextTool, requestedPath, opts.targetPath); if (!FILELESS_TOOLS.has(nextTool) && !nextPath) { - opts.log(`[nav] no project file found for ${nextTool}: ${rawFileName}`); - return false; + // Native KiCad's "Switch to PCB Editor" with no board opens pcbnew on a + // NEW empty board at the derived path — mirror it by creating the + // templated counterpart in the project (the shape NewFileDialog writes) + // and navigating to it. Only sessions that can persist pass `createFile` + // (ToolPage); viewers and scratch/local-folder sessions keep the quiet + // no-op. C++ calls this hook synchronously (EM_ASM_INT) and ignores the + // result beyond a log line, so the create+navigate runs async and we + // answer true optimistically once it's kicked off. + const createFile = opts.createFile; + if (!createFile) { + opts.log(`[nav] no project file found for ${nextTool}: ${rawFileName}`); + return false; + } + if (pendingCreate) { + opts.log(`[nav] create already pending: ${pendingCreate}`); + return true; + } + const relPath = + requestedPath ?? + (opts.targetPath + ? withExtension(nextTool, fileStem(opts.targetPath)) + : defaultFileName(nextTool)); + const url = + projectPath(currentScope(), opts.slug, relPath) + win.location.search; + pendingCreate = relPath; + void (async () => { + try { + const bytes = new TextEncoder().encode( + newFileTemplate(nextTool, crypto.randomUUID()), + ); + await createFile(relPath, bytes); + opts.log(`[nav] created missing ${nextTool} file ${relPath} -> ${url}`); + markDeliberateNavigation(); + win.location.assign(url); + } catch (e) { + pendingCreate = null; + opts.log( + `[nav] create failed for ${relPath}: ${e instanceof Error ? e.message : String(e)}`, + ); + } + })(); + return true; } // Scope/kind/name grammar: a fileless tool boots at `…/-/:tool`; a file route @@ -261,6 +309,7 @@ function installToolNavigationHook( : projectPath(scope, opts.slug, nextPath)) + win.location.search; opts.log(`[nav] ${rawToolName} ${rawFileName || "(no file)"} -> ${url}`); + markDeliberateNavigation(); win.location.assign(url); return true; }; @@ -286,6 +335,20 @@ function installToolNavigationHook( let activeQuitHook: (() => void) | undefined; let quitHandled = false; +/** + * Latch the quit dispatcher off ahead of a deliberate in-app navigation (the + * tool-switch hook's location.assign). The wx port's UnloadCallback runs on + * BEFOREUNLOAD — i.e. the instant the navigation starts, while this document + * keeps running until the next one commits — and closes the top frame, which + * fires wxAppTopWindowClosed. Without the latch the quit hook then + * history.back()s over the in-flight navigation (the pagehide latch below is + * too late: pagehide only fires at commit time). One-shot per document, same + * as the pagehide latch — this page is on its way out. + */ +function markDeliberateNavigation() { + quitHandled = true; +} + const quitDispatcher = () => { if (quitHandled) return; quitHandled = true; @@ -686,6 +749,7 @@ export function WasmTool({ targetPath, fetchBytes, saveBytes, + createFile, docSource, assetBaseUrl, libsSource, @@ -718,6 +782,15 @@ export function WasmTool({ * MEMFS-only (e.g. Y.Doc-backed sessions). */ saveBytes?: SaveBytes; + /** + * Create a new file in the project (tool-switch auto-create: eeschema's + * "Switch to PCB Editor" when no board exists yet). Persisted BEFORE the + * hook navigates, so the next ToolPage load finds it. Omit for sessions + * that can't persist a new project file (read-only viewers, scratch and + * local-folder sessions) — a missing switch target then stays a logged + * no-op. + */ + createFile?: (relPath: string, bytes: Uint8Array) => Promise; /** * Where this project's DOCUMENT lives (see lib/config docSourceConfig): * "ydoc" materializes the target file from its collab room when the room has @@ -953,6 +1026,7 @@ export function WasmTool({ slug, files, targetPath, + createFile, log: append, }); @@ -969,7 +1043,7 @@ export function WasmTool({ removeNavigationHook(); removeQuitHook(); }; - }, [slug, files, targetPath, append]); + }, [slug, files, targetPath, createFile, append]); React.useEffect(() => { // Guard re-entry: the WASM runtime is process-global and must boot exactly diff --git a/web/standalone/src/lib/api.ts b/web/standalone/src/lib/api.ts index b3b31a9..de09ec0 100644 --- a/web/standalone/src/lib/api.ts +++ b/web/standalone/src/lib/api.ts @@ -108,6 +108,27 @@ export async function uploadFileBytes( } } +/** + * Create a project file that a tool switch found missing (WasmTool's nav + * hook): write `bytes` at `relPath` unless the file already exists on the + * source — the hook's file list is a mount-time snapshot, and a collaborator + * may have created the file since (never clobber it with an empty template). + * Unlike `uploadFileBytes` there is deliberately NO download fallback: a + * read-only source rejects (uploader absent, or the composite's per-slug + * ReadOnlyProjectError) and the caller keeps the editor where it is. + */ +export async function createProjectFileIfMissing( + slug: string, + relPath: string, + bytes: Uint8Array, +): Promise { + const source = projectSource(); + if (!source.uploadFileBytes) throw new ReadOnlyProjectError(slug); + const { files } = await source.getProject(slug); + if (files.some((file) => file.path === relPath)) return; + await source.uploadFileBytes(slug, relPath, bytes); +} + // --- collaboration drift reporting (ysync; backend-only) --- /** diff --git a/web/standalone/src/pages/ToolPage.tsx b/web/standalone/src/pages/ToolPage.tsx index e993fa7..91e5a1c 100644 --- a/web/standalone/src/pages/ToolPage.tsx +++ b/web/standalone/src/pages/ToolPage.tsx @@ -1,6 +1,7 @@ import { useParams, useSearchParams } from "react-router-dom"; import { parseToolParam, toolForFile, type Tool } from "@pcbjam/shared"; import { + createProjectFileIfMissing, fetchFileBytes, uploadFileBytes, useProject, @@ -80,6 +81,11 @@ export function ToolPage() { ? undefined : (relPath, bytes) => uploadFileBytes(slug, relPath, bytes) } + createFile={ + readOnly + ? undefined + : (relPath, bytes) => createProjectFileIfMissing(slug, relPath, bytes) + } docSource={docSource} sourceDescriptor={sourceDescriptor} readOnly={readOnly} diff --git a/web/turbo.json b/web/turbo.json index d1d50a3..5740f11 100644 --- a/web/turbo.json +++ b/web/turbo.json @@ -2,10 +2,12 @@ "$schema": "https://turbo.build/schema.json", "globalEnv": [ "PORT", + "STANDALONE_PORT", "PROJECT_DIR", "CORS_ORIGIN", "WASM_SRC_DIR", "VITE_API_BASE_URL", + "VITE_LOCAL_PROJECTS", "VITE_WASM_ASSET_BASE_URL" ], "tasks": {