Merge pull request #100 from BGforgeNet/dev/dialog-editor

Dev/dialog editor

Author: burner1024

.github/workflows/harness.yml

@@ -0,0 +1,64 @@
+name: "Harness"
+
+# Render/drive the binary-editor and dialog-editor webview harnesses in headless Chromium as a regression
+# suite (scripts/test-harness.sh). Each driver mounts the real webview bundle and asserts through PASS/FAIL
+# gates - catching render/mount/CSP/layout regressions the node/jsdom vitest suites cannot see.
+#
+# Kept in its OWN job rather than the main "Build" gate so the Chromium download (~150MB) and the browser
+# runs add no wall-clock to every other check. See the harness READMEs and docs/architecture.md.
+
+on:
+  pull_request:
+  push:
+    branches:
+      - "**"
+
+# Cancel superseded runs on the same ref (branch pushes and PR updates).
+concurrency:
+  group: ${{ github.workflow }}-${{ github.ref }}
+  cancel-in-progress: true
+
+permissions:
+  contents: read
+
+jobs:
+  harness:
+    name: harness
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@df4cb1c069e1874edd31b4311f1884172cec0e10 # v6.0.3
+        with:
+          persist-credentials: false
+
+      - name: Install pnpm
+        uses: pnpm/action-setup@0ebf47130e4866e96fce0953f49152a61190b271 # v6.0.9
+        with:
+          run_install: false
+
+      - name: Install Node.js
+        uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e # v6.4.0
+        with:
+          node-version: 22
+          cache: pnpm
+
+      - name: Install dependencies
+        run: pnpm install --frozen-lockfile
+
+      # The dialog-editor edit drivers parse real .d through the weidu-d tree-sitter WASM (via the fake host),
+      # which is a gitignored build artifact.
+      - name: Build tree-sitter grammar
+        run: pnpm build:grammar
+
+      # playwright is a pinned devDep, but pnpm's build-script gate skips its browser postinstall, so pull the
+      # one browser the harnesses launch here. --with-deps installs the OS libraries Chromium needs on the runner.
+      - name: Install Chromium for Playwright
+        run: pnpm exec playwright install --with-deps chromium
+
+      # Several binary-editor drivers read real .itm/.cre/.map files from the gitignored-but-reproducible
+      # external/ trees (the dialog drivers and the deep-jump driver use committed testFixture/ and need none).
+      - name: Reset external fixture repos
+        run: ./scripts/reset-external.sh
+
+      - name: Run render/drive harnesses
+        run: ./scripts/test-harness.sh

.gitignore

@@ -11,6 +11,7 @@ bgforge-mls-kate*/
 .reports/
 .vscode-test/
 .dev/
+.playwright-mcp/
 
 client/node_modules
 client/out

.oxfmtrc.json

@@ -32,6 +32,8 @@
         "themes/vs-seti-icon-theme.json",
         "themes/bgforge-monokai.json",
         "themes/bgforge-icon-theme.json",
-        "binary-editor/test/harness/*.html"
+        "binary-editor/test/harness/*.html",
+        "client/src/dialog-editor/test/harness/app.html",
+        "client/src/dialog-editor/test/harness/real-model.ts"
     ]
 }

.oxlintrc.json

@@ -266,21 +266,33 @@
             // `document.getElementById` is clearer and faster than
             // `.querySelector('#id')`; the rule's premise doesn't hold.
             // Webview scripts are the only DOM consumers in the codebase.
-            "files": ["client/src/**/*-webview*.ts"],
+            "files": ["client/src/**/*-webview*.ts", "client/src/dialog-editor/webview/**"],
             "rules": {
                 "unicorn/prefer-query-selector": "off"
             }
         },
+        {
+            // Svelte 5 runes require `let`: `let x = $state(...)` and
+            // `let { ... } = $props()` are reassigned by the compiler's reactivity,
+            // which oxlint cannot see - it reports them as never-reassigned. `sort-vars`
+            // likewise fights the rune/prop declaration order. Scope: the dialog editor's
+            // Svelte webview components.
+            "files": ["client/src/dialog-editor/webview/**/*.svelte"],
+            "rules": {
+                "prefer-const": "off",
+                "sort-vars": "off"
+            }
+        },
         {
             // VS Code's `webview.postMessage()` (extension side),
             // `acquireVsCodeApi().postMessage()` (webview side), and
             // `worker_threads` `parentPort.postMessage()` are not the browser's
             // `window.postMessage`; none take a `targetOrigin`.
             "files": [
                 "client/src/editors/**",
-                "client/src/dialog-tree/**",
                 "client/src/webview-utils.ts",
-                "client/src/binary-editor/**"
+                "client/src/binary-editor/**",
+                "client/src/dialog-editor/**"
             ],
             "rules": {
                 "unicorn/require-post-message-target-origin": "off"
@@ -366,6 +378,7 @@
         "**/out/**",
         "**/*.d.ts",
         "server/src/user-messages.ts",
-        "binary-editor/test/harness/**"
+        "binary-editor/test/harness/**",
+        "client/src/dialog-editor/test/harness/**"
     ]
 }

AGENTS.md

@@ -18,7 +18,7 @@ Dependabot version-update PRs are intentionally disabled - there is no `.github/
 - **User-facing messages:** Never call `connection.window.showInformationMessage/showWarningMessage/showErrorMessage` directly in server code. Use `showInfo()`, `showWarning()`, `showError()`, or `showErrorWithActions()` from `user-messages.ts` - they auto-decode `file://` URIs to readable paths. An oxlint rule enforces this.
 - **Temporary artifacts:** Put transient test/build files under the repo-level `tmp/` directory (or `os.tmpdir()` when system temp is required). Do not create ad hoc temp directories under source trees like `server/test/`, `binary/test/`, or `scripts/**`.
 - **Webview CSP - styles need `cspSource`, not a bare nonce.** A webview's `style-src` must include `{{cspSource}}`. VS Code's wrapped webview silently drops a `style-src 'nonce-...'` stylesheet that lacks `cspSource` (raw Chromium honors it, so headless/standalone renders pass while the real panel renders fully unstyled). Load CSS as `webview.asWebviewUri()` `<link>` elements with `style-src {{cspSource}}` (keep the nonce for `script-src` only); add each CSS dir to `localResourceRoots`. See `docs/architecture.md` (Webview CSP); guarded by `client/test/webview-csp.test.ts`.
-- **Binary-editor webview changes can be rendered - don't fall back to a sketch without checking.** A headless Playwright harness in `binary-editor/test/harness/` loads the real `App.svelte` bundle in Chromium and writes per-format PNG screenshots (e.g. `render-pro-eff.mts` -> `shot-pro.png` + `shot-eff.png`, `render-itm.mts`, `render-spl.mts`, `render-cre.mts`, `render-map.mts`, `render-primitives.mts`). For any visual/CSS/layout change to the binary editor, render and inspect the screenshot rather than reasoning about the cascade blind. Run order: `cd binary && pnpm build` (only if `binary/src` changed), then `pnpm exec tsx binary-editor/test/harness/build.mts` (rebuilds `app.html` after any webview/Svelte/`styles.css` edit), then a driver `pnpm exec tsx binary-editor/test/harness/render-pro-eff.mts`. Prereqs (environment, not repo deps): Playwright + a Chromium browser on `PATH`. It is e2e-tier and intentionally excluded from `pnpm test`/`pnpm test:all`. See `binary-editor/test/harness/README.md` for the harness, and **`docs/binary-editor-ui-guidelines.md`** for the project's UI conventions and the review brief - it indexes the co-located writer-facing rules (`client/src/binary-editor/webview/AGENTS.md` for the render layer, `binary/src/AGENTS.md` for the layout schema), which auto-load when you edit those dirs. Review rendered screenshots against the guidelines.
+- **Binary-editor webview changes can be rendered - don't fall back to a sketch without checking.** A headless Playwright harness in `binary-editor/test/harness/` loads the real `App.svelte` bundle in Chromium and writes per-format PNG screenshots (e.g. `render-pro-eff.mts` -> `shot-pro.png` + `shot-eff.png`, `render-itm.mts`, `render-spl.mts`, `render-cre.mts`, `render-map.mts`, `render-primitives.mts`). For any visual/CSS/layout change to the binary editor, render and inspect the screenshot rather than reasoning about the cascade blind. Run order: `cd binary && pnpm build` (only if `binary/src` changed), then `pnpm exec tsx binary-editor/test/harness/build.mts` (rebuilds `app.html` after any webview/Svelte/`styles.css` edit), then a driver `pnpm exec tsx binary-editor/test/harness/render-pro-eff.mts`. `playwright` is a pinned devDep; the Chromium browser it drives is the environment prereq (`pnpm exec playwright install chromium`). The drivers assert through PASS/FAIL gates and now run in CI as a regression suite via the separate `Harness` workflow (`scripts/test-harness.sh` - both editors' harnesses); they stay excluded from `pnpm test`/`pnpm test:all`, which run browserless. See `binary-editor/test/harness/README.md` for the harness, and **`docs/binary-editor-ui-guidelines.md`** for the project's UI conventions and the review brief - it indexes the co-located writer-facing rules (`client/src/binary-editor/webview/AGENTS.md` for the render layer, `binary/src/AGENTS.md` for the layout schema), which auto-load when you edit those dirs. Review rendered screenshots against the guidelines.
 - **To run the whole extension in a real VS Code instance, use `pnpm dev:web`.** It starts [code-server](https://github.com/coder/code-server) (VS Code in a browser) with this repo loaded as an unpacked extension, so the LSP server and binary custom editor actually run - the screenshot harness above only renders the webview in isolation. Default serves plain HTTP on `0.0.0.0:8080` (override with `CODE_SERVER_PORT`/`CODE_SERVER_HOST`); it bootstraps a pinned code-server into the gitignored `.dev/` on first run, then builds via `build:dev`. The command runs a long-lived server in the foreground - background it (or run it as a separate task) if you need the shell, bind to the port your environment exposes, and confirm it is up before reporting a URL. The binary editor is a webview, so the browser must reach it over a secure context (`http://localhost` via a port-forward, or a trusted cert) or the editor renders blank. Full details and configuration: **`scripts/dev-web.md`**.
 - **Use `pnpm` exclusively. Never use `npx`.** This is a hard requirement. Every time you reach for `npx`, `npm`, or any npm-series command, stop and use `pnpm exec` instead. Example: `pnpm exec playwright` not `npx playwright`. This applies to all contexts including one-off commands, scripts, subagents, and delegated tasks. If pnpm is not available in a context, install it first or use the workspace package.json scripts.
 - **Milestone close-out commands (scope to what you touched).** Binary changes -> `pnpm exec vitest run --config binary/vitest.config.ts` (run from the repo root, not `cd binary` - several tests resolve `external/` and `client/testFixture/` fixtures relative to cwd); webview/Svelte changes -> the client tests plus the binary-editor render harness. Reserve `pnpm build:all` + `pnpm test:all` (~9 min) for changes that span subsystems or touch shared build infra, grammars, transpilers, or the server.
@@ -120,6 +120,8 @@ cd grammars/weidu-tp2 && pnpm test   # or any grammars/*/
 
 ```
 
+**Testing against real external files.** The `external/` mod trees are gitignored but REPRODUCIBLE - `pnpm test:external` (via `scripts/reset-external.sh` + `scripts/external-repos-lib.sh`) clones/checks them out at pinned refs. So real-corpus coverage belongs in a committed test, not a throwaway: tests that exercise real external files live under `server/test/integration/**` (run by `pnpm test:integration`, config `server/vitest.integration.config.ts`), using `test/integration/test-helpers.ts` (`FALLOUT_FIXTURES` = `external/fallout`, `IE_FIXTURES` = `external/infinity-engine`, `loadFixture`/`loadFixtures`). Gate a corpus sweep with `describe.skipIf(files.length === 0)` so it skips cleanly when the corpus is not checked out. Before placing any real-file test, read a sibling there (`integration/weidu-d.test.ts`, `fallout-ssl/rename.test.ts`) for the fixture/init conventions. Do NOT commit copies of gitignored `external/` files as fixtures, and do NOT hand-run a one-off script where this suite is the home.
+
 ## Publishing & Release
 
 Releases are tag-driven via GitHub Actions. `docs/releasing.md` is the canonical reference: the tag scheme (which tag form triggers which workflow), and the per-stream release procedures for the extension, the three libraries, and the reusable Action - including the root/`server` version-identity invariant. The server and VSIX bundle their `@bgforge/*` libraries rather than depending on them at runtime, so the extension and the libraries release in any order. Consult it before cutting any release. See `docs/architecture.md` for packaging mechanics.
@@ -132,6 +134,7 @@ Releases are tag-driven via GitHub Actions. `docs/releasing.md` is the canonical
 - `ts-morph` is pinned to `^27.0.2` across the server runtime and all transpiler subpackages so the bundled TypeScript matches the project's own `typescript ^5.9.3` pin. ts-morph bundles its TypeScript dependency rather than treating it as a peer, so the ts-morph version effectively chooses the TS compiler that runs against transpiler ASTs regardless of the workspace `typescript` pin. ts-morph 28 bundles TS 6.0 - TS 6 is on a 6.0.x line with no 6.1 published, fails the standard "matured target major" upgrade rule, and ts-morph 28 is therefore deferred until TS 6.1 ships. ts-morph 27.0.x is technically also on a 0-minor line, but the TS 5.9 alignment is the deciding factor. Bump in lockstep with the project's `typescript` pin: when `typescript` moves to 6.x, ts-morph moves to whichever ts-morph major bundles that TS line.
 - `@types/node` tracks the latest LTS Node major, currently `^24.x`. The extension targets LTS Node only (minimum supported is 20), so do not bump `@types/node` to odd-numbered "Current" majors (e.g. 25.x) - that would expose type definitions for APIs not present at the supported runtime floor. Move it forward only when a new even-numbered Node release reaches LTS.
 - `ini` (runtime dep of `@bgforge/format`) is held at `^6.x`; `7.0.0` is a major with potential parse/stringify behavior changes that need a changelog review before adoption.
+- `playwright` (devDep) is pinned to an EXACT version (no caret) because the webview harnesses launch a browser from Playwright's version-keyed cache: the devbox and the `Harness` CI job download that browser via `playwright install`, and a caret drift to a version whose browser revision is not cached would break the harness run until a re-download. Bump the pin and re-run `playwright install` together. Its browser postinstall is skipped by pnpm's build-script gate (not in `onlyBuiltDependencies`), so a plain `pnpm install` never pulls ~150MB of browser - the harness paths install Chromium explicitly.
 - `sslc-emscripten-noderawfs` (the built-in SSL compiler WASM, `server/package.json`) is an HTTPS GitHub-release tarball, and its `pnpm-lock.yaml` entry carries a hand-maintained `integrity` field. pnpm 11.4+ fails closed (`ERR_PNPM_MISSING_TARBALL_INTEGRITY`) on any tarball lockfile entry that lacks integrity, and URL-tarball resolvers only learn the hash on download - so when the package is reused from the store, the field is never emitted. Practical rule: **evolve the lockfile incrementally** (`pnpm install` / `pnpm update`); pnpm 11.5.0+ preserves the existing integrity across those, so `pnpm update` keeps working. Do **not** `rm pnpm-lock.yaml` to regenerate from scratch - that drops the field and breaks `pnpm update` until it is re-added. The release asset is immutable, so the hash is stable; if a full regen is ever unavoidable, restore the line `resolution: {integrity: sha512-oQEOCRLmCodC1MlcIVAqGyTb3aWLpJ7c9tQm4CF8hrf9feZoxcIvSfwKpkcq0FykigqVGdpFpkbwQBS8yqo28Q==, tarball: <url>}`. The same dep is also listed under `minimumReleaseAgeExclude` in `pnpm-workspace.yaml` (a non-registry tarball has no publish timestamp for the 24h maturity window to check).
 
 ## Architecture

binary-editor/test/harness/README.md

@@ -74,17 +74,14 @@ It is type-checked via `test/harness/tsconfig.json`, which includes the DOM lib
 
 ## Prerequisites
 
-- **Playwright + a Chromium browser** must be available on `PATH` in whatever environment runs the harness.
-  Install Playwright and its browsers globally:
+- **Playwright** is a pinned devDependency (`pnpm install` provides it), so `pnpm exec tsx <driver>` resolves
+  `import { chromium } from "playwright"` with no global install. Its browser postinstall is skipped by pnpm's
+  build-script gate, so install the one browser the drivers launch:
 
   ```
-  npm install -g playwright
-  playwright install chromium
+  pnpm exec playwright install chromium
   ```
 
-  Playwright is intentionally NOT listed in any `package.json` in this repo. It is an environment
-  prerequisite, not a repo dependency.
-
 - **Node 20+** (matched to the project's minimum supported runtime).
 
 - **`tsx`** - available via the repo's dev dependencies (`pnpm exec tsx ...`).

binary-editor/test/harness/out-dir.ts

@@ -0,0 +1,14 @@
+import { mkdirSync } from "node:fs";
+import path from "node:path";
+import { fileURLToPath } from "node:url";
+
+// Repo-level tmp/ for harness screenshots, keeping the source tree clean (project convention).
+// This module lives in binary-editor/test/harness/, so the repo root is three levels up.
+const here = path.dirname(fileURLToPath(import.meta.url));
+export const SHOT_DIR = path.resolve(here, "../../../tmp");
+mkdirSync(SHOT_DIR, { recursive: true });
+
+/** Absolute path for a harness screenshot, under the repo tmp/ dir. */
+export function shotPath(name: string): string {
+    return path.join(SHOT_DIR, name);
+}

binary-editor/test/harness/render-clip-sweep.mts

@@ -99,7 +99,6 @@ async function runFormat(browser: Browser, label: string, uri: string, bytes: Ui
     });
     await page.goto("file://" + path.join(here, "app.html"));
     await page.waitForSelector(".layout-root", { timeout: 5000 });
-    await page.waitForTimeout(200);
 
     const found: ClipViolation[] = [];
     // Check whatever a freshly-opened detail/list selection renders too: select the first list row if one is
@@ -109,7 +108,11 @@ async function runFormat(browser: Browser, label: string, uri: string, bytes: Ui
         const firstRow = page.locator(".master .vlist .vrow").first();
         if (await firstRow.count()) {
             await firstRow.click().catch(() => undefined);
-            await page.waitForTimeout(150);
+            await page
+                .waitForFunction(() => document.querySelector(".detail .layout-root .field") !== null, undefined, {
+                    timeout: 5000,
+                })
+                .catch(() => undefined);
             found.push(...(await collectClipViolations(page, ctx + " (detail)")));
         }
     };
@@ -123,7 +126,18 @@ async function runFormat(browser: Browser, label: string, uri: string, bytes: Ui
             const tab = tabs.nth(i);
             const name = ((await tab.textContent()) ?? `tab${i}`).replace(/\s*\(\d.*\)\s*$/, "").trim();
             await tab.click();
-            await page.waitForTimeout(200);
+            await page
+                .waitForFunction(
+                    (expectedLabel) => {
+                        const active = document.querySelector(
+                            '.bb-tabs.primary button[role="tab"][aria-selected="true"]',
+                        );
+                        return !!active && (active.textContent ?? "").includes(expectedLabel);
+                    },
+                    name,
+                    { timeout: 5000 },
+                )
+                .catch(() => undefined);
             await sweepCurrentView(`${label} > ${name}`);
         }
     }