-
Notifications
You must be signed in to change notification settings - Fork 1
feat(ts-builder): dev-server sidecar for block-ui serve #1657
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
24ed665
a3965ae
ebded76
54036b8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,18 +1,24 @@ | ||
| import { Command } from "commander"; | ||
| import * as path from "node:path"; | ||
| import { | ||
| executeCommand, | ||
| getConfigInfo, | ||
| getGlobalOptions, | ||
| getValidatedConfigPath, | ||
| requireTarget, | ||
| resolveVite, | ||
| setupSidecarLifecycle, | ||
| validateTargetForBrowser, | ||
| } from "./utils/index"; | ||
|
|
||
| export const serveCommand = new Command("serve") | ||
| .description("Start development server") | ||
| .option("-p, --port <port>", "Port number") | ||
| .option("--host <host>", "Host address") | ||
| .option( | ||
| "--sidecar-out <path>", | ||
| "Directory the dev-server sidecar is written into (default: ./dist for block-ui)", | ||
| ) | ||
| .action(async (options, command) => { | ||
| const globalOpts = getGlobalOptions(command); | ||
| const target = requireTarget(globalOpts); | ||
|
|
@@ -25,21 +31,94 @@ export const serveCommand = new Command("serve") | |
| `Starting dev server for ${target} project${useSources ? " with sources condition" : ""}...`, | ||
| ); | ||
|
|
||
| try { | ||
| const viteCommand = resolveVite(); | ||
| const viteArgs = ["dev"]; | ||
| const configInfo = getConfigInfo(target); | ||
| const configPath = getValidatedConfigPath(customServeConfig, configInfo!.filename); | ||
| const configInfo = getConfigInfo(target); | ||
| const configPath = getValidatedConfigPath(customServeConfig, configInfo!.filename); | ||
|
|
||
| viteArgs.push("--config", configPath); | ||
| if (target === "block-ui") { | ||
| await runBlockUiServe({ | ||
| configPath, | ||
| port: options.port ? Number(options.port) : undefined, | ||
| host: options.host, | ||
| useSources, | ||
| sidecarOut: options.sidecarOut, | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| // Other targets keep the existing CLI-spawn behavior — no sidecar is written. | ||
| try { | ||
| const viteCommand = resolveVite(); | ||
| const viteArgs = ["dev", "--config", configPath]; | ||
| if (options.port) viteArgs.push("--port", options.port); | ||
| if (options.host) viteArgs.push("--host", options.host); | ||
|
|
||
| const env = useSources ? { USE_SOURCES: "1" } : undefined; | ||
| await executeCommand(viteCommand, viteArgs, env); | ||
| } catch (error) { | ||
| console.error("Failed to start dev server:", error); | ||
| process.exit(1); | ||
| } | ||
| }); | ||
|
|
||
| interface BlockUiServeOptions { | ||
| configPath: string; | ||
| port?: number; | ||
| host?: string; | ||
| useSources?: boolean; | ||
| sidecarOut?: string; | ||
| } | ||
|
|
||
| async function runBlockUiServe(opts: BlockUiServeOptions): Promise<void> { | ||
| // Vite's config file reads USE_SOURCES from process.env, so set it before | ||
| // we createServer. | ||
| if (opts.useSources) { | ||
| process.env.USE_SOURCES = "1"; | ||
| } | ||
|
|
||
| // Resolve sidecar path BEFORE starting Vite so lifecycle hooks register | ||
| // first — see `setupSidecarLifecycle` for the ordering rationale. | ||
| const sidecarDir = opts.sidecarOut | ||
| ? path.resolve(opts.sidecarOut) | ||
| : path.resolve(process.cwd(), "dist"); | ||
| const sidecarPath = path.join(sidecarDir, ".dev-server"); | ||
| const sidecar = setupSidecarLifecycle(sidecarPath); | ||
|
|
||
| // Vite is a peer dep of ts-builder; resolve it from the consumer package | ||
| // so we use the same version the project's config is written against. | ||
| let vite: typeof import("vite"); | ||
| try { | ||
| vite = await import("vite"); | ||
| } catch (error) { | ||
| console.error("Failed to load `vite`. Ensure it's installed as a dependency.", error); | ||
| process.exit(1); | ||
| return; | ||
| } | ||
|
|
||
| const server = await vite.createServer({ | ||
| configFile: opts.configPath, | ||
| server: { | ||
| port: opts.port, | ||
| host: opts.host, | ||
| }, | ||
| }); | ||
|
|
||
| await server.listen(); | ||
| server.printUrls(); | ||
|
|
||
| const url = server.resolvedUrls?.local?.[0]; | ||
| if (!url) { | ||
| console.warn("vite did not report a local URL; skipping dev-server sidecar."); | ||
| return; | ||
| } | ||
|
Comment on lines
+107
to
+111
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In some network environments or containerized setups, let url = server.resolvedUrls?.local?.[0];
if (!url) {
const address = server.httpServer?.address();
if (address && typeof address === "object") {
const host = address.address === "::" || address.address === "0.0.0.0" ? "localhost" : address.address;
const protocol = server.config.server.https ? "https" : "http";
url = `${protocol}://${host}:${address.port}`;
}
}
if (!url) {
console.warn("vite did not report a local URL; skipping dev-server sidecar.");
return;
}
Comment on lines
+108
to
+111
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Prompt To Fix With AIThis is a comment left during a code review.
Path: tools/ts-builder/src/commands/serve.ts
Line: 108-111
Comment:
**Server left running without sidecar on missing URL**
When `server.resolvedUrls?.local?.[0]` is undefined, the function returns early but the Vite server remains listening. The patched `server.close` that removes the sidecar is never installed, so if a test harness calls `server.close()` directly the server shuts down but cleanup wiring is absent. Consider closing the server explicitly and exiting with an error when no URL is reported.
How can I resolve this? If you propose a fix, please make it concise. |
||
|
|
||
| sidecar.publish({ schema: 1, url, pid: process.pid }); | ||
| console.log(`Wrote dev-server sidecar: ${sidecarPath}`); | ||
|
|
||
| // Make Vite's own close path remove the sidecar too — covers programmatic | ||
| // shutdowns (e.g. test harnesses) and HMR-driven server restarts where | ||
| // signal-based cleanup wouldn't fire. | ||
| const origClose = server.close.bind(server); | ||
| server.close = async () => { | ||
| sidecar.remove(); | ||
| return origClose(); | ||
| }; | ||
| } | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,83 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import * as fs from "node:fs"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import * as path from "node:path"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export interface DevServerSidecar { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| schema: 1; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| url: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pid: number; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Atomically write the sidecar JSON file. The desktop app polls this file | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * when loading a dev-v2 block; an atomic rename avoids the consumer reading | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * a half-written file. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export function writeSidecar(sidecarPath: string, payload: DevServerSidecar): void { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fs.mkdirSync(path.dirname(sidecarPath), { recursive: true }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const tmp = sidecarPath + ".tmp"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fs.writeFileSync(tmp, JSON.stringify(payload, null, 2)); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fs.renameSync(tmp, sidecarPath); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export function removeSidecar(sidecarPath: string): void { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| fs.unlinkSync(sidecarPath); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } catch { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // ignore — already gone | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export interface SidecarLifecycle { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Write the sidecar and mark it as live for cleanup. */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| publish(payload: DevServerSidecar): void; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** Remove the sidecar explicitly (idempotent). */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| remove(): void; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Set up lifecycle hooks for a sidecar file BEFORE the server that owns it | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * is started. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Registration order matters. Node fires same-signal handlers in registration | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * order. Vite's `createServer` / `listen` registers its own SIGINT/SIGHUP/etc. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * handlers that exit the process — if we wait until after `createServer` to | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * register ours, Vite's may run first, exit synchronously, and our cleanup | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * is missed. Install everything up-front, then `publish()` once the URL is | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * known. `remove()` and the signal-triggered cleanup are idempotent. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export function setupSidecarLifecycle(sidecarPath: string): SidecarLifecycle { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let live = false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const cleanup = () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!live) return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| live = false; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| removeSidecar(sidecarPath); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const exitOn = (signal: NodeJS.Signals) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| process.on(signal, () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cleanup(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Re-raise the default behavior with a clean exit code so callers | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // (shell, pnpm) see the expected termination. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| process.exit(0); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+57
to
+64
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Exiting with code
Suggested change
Comment on lines
+57
to
+64
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: tools/ts-builder/src/commands/utils/dev-server-sidecar.ts
Line: 57-64
Comment:
The signal handlers always call `process.exit(0)`, making SIGINT/SIGTERM look like a clean success to any parent process or shell. The conventional exit codes are 130 for SIGINT, 143 for SIGTERM, and 129 for SIGHUP (128 + signal number). pnpm scripts, CI systems, and process supervisors that inspect `$?` may mis-classify the termination. The idiomatic fix is to remove the handler and re-kill the process with the original signal, letting the kernel apply the default disposition.
```suggestion
const exitOn = (signal: NodeJS.Signals) => {
const handler = () => {
cleanup();
// Re-raise the signal with the kernel's default disposition so the
// shell / pnpm sees the conventional exit status (130 for SIGINT,
// 143 for SIGTERM, 129 for SIGHUP).
process.removeListener(signal, handler);
process.kill(process.pid, signal);
};
process.on(signal, handler);
};
```
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| exitOn("SIGINT"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| exitOn("SIGTERM"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| exitOn("SIGHUP"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| process.on("exit", cleanup); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| process.on("uncaughtException", (err) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| cleanup(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| console.error(err); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| process.exit(1); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| publish(payload) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| writeSidecar(sidecarPath, payload); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| live = true; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| remove: cleanup, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| export * from "./command-runner"; | ||
| export * from "./common-options"; | ||
| export * from "./config-manager"; | ||
| export * from "./dev-server-sidecar"; | ||
| export * from "./executable-resolver"; | ||
| export * from "./path-utils"; |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,8 +1,115 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import vue from "@vitejs/plugin-vue"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import sourcemaps from "rollup-plugin-sourcemaps2"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type { ConfigEnv, UserConfig } from "vite"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import type { ConfigEnv, Plugin, UserConfig } from "vite"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import commonjs from "vite-plugin-commonjs"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /* | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * ─────────────────────────────────────────────────────────────────────────── | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * TEMPORARY DEV-MODE SECURITY RELAXATIONS — TRACK AND REMOVE | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * ─────────────────────────────────────────────────────────────────────────── | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * This file contains two `vite dev`-only hacks that weaken the security | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * posture of the block UI runtime relative to production. They exist so the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * dev-server sidecar workflow (block author runs `ts-builder serve` and the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * desktop hot-loads the block from `http://localhost:<port>/`) functions at | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * all. Both are documented at their respective sites below. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * 1. `relaxCspForDevWasm` — rewrites the block's CSP meta tag to add | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * `'wasm-unsafe-eval'` so in-page WebAssembly (PFrameSpec / SpecDriver) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * can compile. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * 2. `define: { "process.env": "({})" }` — masks all `process.env.X` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * references (including `NODE_ENV`) to `undefined` so transitive Node | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * imports don't blow up on `process is not defined`. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Both are dev-only — production block UIs are served via the `block-ui://` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * custom protocol, registered with `bypassCSP: true`, so the CSP is | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * effectively absent in production, and rolldown strips `process.env.X` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * references at build time. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * FOLLOW-UP — DO NOT LEAVE THIS AS THE STEADY STATE: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - Migrate the block template CSP (currently `script-src 'self' blob:`) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * to a canonical policy that lists every source the runtime legitimately | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * uses (including `'wasm-unsafe-eval'`), so dev and prod share one CSP. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - Once block-ui:// no longer needs `bypassCSP: true`, drop the bypass on | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * the protocol registration in | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * `core/platforma-desktop-app/packages/main/src/protocols/block-ui.ts`. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - Re-enable strict CSP enforcement in dev by removing | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * `relaxCspForDevWasm`. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - Replace the blunt `process.env: ({})` substitution with a precise | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * allowlist of vars the SDK genuinely reads (or eliminate the references | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * entirely from browser-bound code paths — most are debug toggles like | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * `MI_LOG_PFRAMES` and can move to a constructor option or a | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * `globalThis` flag). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - Run a security review of the block-ui runtime end-to-end: CSP, preload | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * surface, IPC method allowlist, `webRequest` headers, WASM source set, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * `webSecurity` / `nodeIntegration` / `contextIsolation` settings on | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * `webPreferencesForBlock`, and the dev-server sidecar trust gate | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * (currently dev-v2 only — confirm sufficient for prod hardening). | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Tracking ticket: TODO — create one in MILAB before this lands. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * ─────────────────────────────────────────────────────────────────────────── | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Rewrite the block's index.html CSP meta tag during `vite dev` so WebAssembly | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * modules (e.g. `@milaboratories/pframes-rs-wasm`) can compile in the browser. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Why this is necessary, and why no smaller fix works: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - In production the block UI is served via the `block-ui://` custom | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * protocol, which Electron registers with `bypassCSP: true`. The strict | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * CSP in `index.html` (`script-src 'self' blob:`) never applies and the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * in-page WASM driver (PFrameSpec / `pf-spec-driver`) loads fine. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - In `vite dev` the UI is served over `http://localhost:<port>`. We have | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * no equivalent privilege escape for `http:` — Electron honors the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * standard browser CSP semantics for that scheme. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - The CSP that wins is the *intersection* of every source (meta tag + | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * response header). Adding a permissive `Content-Security-Policy` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * response header via Vite middleware or | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * `session.webRequest.onHeadersReceived` does not loosen the meta — it | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * only tightens further. So a header-side fix cannot work. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - CSP is parse-time baked. DOM mutation of the meta tag after load is | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * ignored. Preload-side intervention runs after parse — too late. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - That leaves rewriting the served HTML. Vite's `transformIndexHtml` hook | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * is the canonical place. Scoped to `apply: "serve"` so production builds | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * are unaffected. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * - WASM is in-page by deliberate architectural choice (see | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * `sdk/ui-vue/src/internal/service_factories.ts`: `PFrameSpec` is the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * synchronous spec driver used inside Vue computed props; moving it to | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * IPC would break the sync reactive chain). So we cannot side-step the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * CSP by relocating the WASM to preload/main. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * Narrowest fix: add `'wasm-unsafe-eval'` (WASM only, NOT JS `eval`). Do not | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * use `'unsafe-eval'` here — that would also allow `eval()`. See the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| * top-of-file follow-up block for the migration plan to remove this entirely. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| function relaxCspForDevWasm(): Plugin { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| name: "ts-builder:relax-csp-for-dev-wasm", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| apply: "serve", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| transformIndexHtml(html) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // CSP source keywords (`'self'`, `'wasm-unsafe-eval'`, ...) are | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // single-quoted inside the double-quoted content attribute, so the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // content character class must exclude only the delimiter. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const cspMetaRegex = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /<meta\s+http-equiv="Content-Security-Policy"\s+content="([^"]*)"\s*\/?>/i; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+99
to
+100
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: tools/ts-builder/src/configs/utils/createViteDevConfig.ts
Line: 99-100
Comment:
The CSP regex matches only when `http-equiv` appears before `content`. HTML attributes can appear in any order; if a block's `index.html` writes `<meta content="…" http-equiv="Content-Security-Policy">`, the regex silently misses the tag and WASM fails to load in dev without any warning.
```suggestion
// Match regardless of attribute order (http-equiv before or after content).
const cspMetaRegex =
/<meta\b[^>]*\bhttp-equiv="Content-Security-Policy"[^>]*\bcontent="([^"]*)"[^>]*\/?>/i;
```
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return html.replace(cspMetaRegex, (match, content: string) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (content.includes("'wasm-unsafe-eval'")) return match; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const relaxed = content.replace( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /script-src([^;]*)/, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (m, srcs: string) => `script-src${srcs} 'wasm-unsafe-eval'`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+103
to
+106
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The current replacement assumes let relaxed = content;
if (/script-src/i.test(content)) {
relaxed = content.replace(
/script-src([^;]*)/i,
(m, srcs: string) => `script-src${srcs} 'wasm-unsafe-eval'`,
);
} else if (/default-src/i.test(content)) {
relaxed = content.replace(
/default-src([^;]*)/i,
(m, srcs: string) => `default-src${srcs} 'wasm-unsafe-eval'`,
);
} else {
relaxed = content ? `${content}; script-src 'wasm-unsafe-eval'` : "script-src 'wasm-unsafe-eval'";
} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return `<meta http-equiv="Content-Security-Policy" content="${relaxed}" />`; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export function createViteDevConfig({ mode, command }: ConfigEnv): UserConfig { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const isProd = mode === "production"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const isServe = command === "serve"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -17,6 +124,7 @@ export function createViteDevConfig({ mode, command }: ConfigEnv): UserConfig { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| plugins: [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| vue(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ...(isServe ? [commonjs({ filter: (id) => id.includes("node_modules") })] : []), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| relaxCspForDevWasm(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| build: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| target: ["chrome140"], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -32,6 +140,18 @@ export function createViteDevConfig({ mode, command }: ConfigEnv): UserConfig { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| define: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "import.meta.vitest": "undefined", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // `vite dev` does not strip `process.env.X` references the way the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // production rolldown build does. Transitive imports (e.g. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // `pf-spec-driver/logging.ts`, `pf-driver/logging.ts`) hit top-level | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // `process.env.MI_LOG_PFRAMES` lookups and throw | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // `ReferenceError: process is not defined` in the browser at module | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // load, leaving the block view white. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Inlining an empty object makes every `process.env.X` resolve to | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // `undefined`. Side effect: `process.env.NODE_ENV` is also `undefined` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // in dev — fine for our current consumers, but flagged in the | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // top-of-file follow-up block as a thing to tighten later. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "process.env": "({})", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
141
to
155
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: tools/ts-builder/src/configs/utils/createViteDevConfig.ts
Line: 141-155
Comment:
The `"process.env": "({})"` define is applied unconditionally — including when `command === "build"` and `mode === "production"`. In a production build, Vite internally sets `"process.env.NODE_ENV"` to `JSON.stringify(mode)`, but the broader `"process.env"` substitution replaces the entire object with `({})` before more-specific keys can win, making `process.env.NODE_ENV` resolve to `undefined`. Libraries like Vue use `process.env.NODE_ENV === "production"` for dead-code elimination; leaving it `undefined` can produce larger, non-optimised production bundles with dev warnings active. Guard this define with `isServe` so it only fires during `vite dev`.
```suggestion
define: {
"import.meta.vitest": "undefined",
// `vite dev` does not strip `process.env.X` references the way the
// production rolldown build does. Transitive imports (e.g.
// `pf-spec-driver/logging.ts`, `pf-driver/logging.ts`) hit top-level
// `process.env.MI_LOG_PFRAMES` lookups and throw
// `ReferenceError: process is not defined` in the browser at module
// load, leaving the block view white.
//
// Inlining an empty object makes every `process.env.X` resolve to
// `undefined`. Side effect: `process.env.NODE_ENV` is also `undefined`
// in dev — fine for our current consumers, but flagged in the
// top-of-file follow-up block as a thing to tighten later.
...(isServe ? { "process.env": "({})" } : {}),
},
```
How can I resolve this? If you propose a fix, please make it concise. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolving
vitedirectly viaimport("vite")relies on Node's default resolution from thets-builderinstallation directory. In monorepos, symlinked environments, or global installations, this can fail or resolve to an incorrect version. It is safer to resolveviterelative to the consumer's configuration file path usingcreateRequireandpathToFileURL.