feat(ts-builder): dev-server sidecar for block-ui serve#1657
Conversation
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a dev-server sidecar mechanism for the block-ui target in ts-builder serve. It implements atomic writing and lifecycle cleanup of a .dev-server sidecar JSON file, programmatically spawns Vite, and applies temporary dev-mode security relaxations (relaxing CSP for WASM and mocking process.env to prevent runtime crashes). The review feedback provides valuable improvements to make the sidecar mechanism more robust, including resolving Vite relative to the consumer's configuration path, adding a fallback for retrieving the local server URL, correctly re-raising signals on termination to preserve exit codes, and hardening the CSP regex replacement to handle missing or case-variant script-src directives.
| 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; | ||
| } |
There was a problem hiding this comment.
Resolving vite directly via import("vite") relies on Node's default resolution from the ts-builder installation directory. In monorepos, symlinked environments, or global installations, this can fail or resolve to an incorrect version. It is safer to resolve vite relative to the consumer's configuration file path using createRequire and pathToFileURL.
let vite: typeof import("vite");
try {
const { createRequire } = await import("node:module");
const { pathToFileURL } = await import("node:url");
const localRequire = createRequire(opts.configPath);
vite = await import(pathToFileURL(localRequire.resolve("vite")).href);
} catch (error) {
console.error("Failed to load `vite`. Ensure it's installed as a dependency.", error);
process.exit(1);
return;
}| const url = server.resolvedUrls?.local?.[0]; | ||
| if (!url) { | ||
| console.warn("vite did not report a local URL; skipping dev-server sidecar."); | ||
| return; | ||
| } |
There was a problem hiding this comment.
In some network environments or containerized setups, server.resolvedUrls might be empty or slow to populate. We can fall back to constructing the URL from the underlying HTTP server's bound address and port to ensure the sidecar is successfully written.
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;
}| 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); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
Exiting with code 0 on signals like SIGINT or SIGTERM hides the interruption from parent processes (such as pnpm, npm, or shell scripts), which can cause chained commands (e.g., ts-builder serve && do-something-else) to continue running even after the user presses Ctrl+C. It is standard practice to clean up, remove the signal listener, and re-raise the signal to let the process terminate naturally with the correct status.
| 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); | |
| }); | |
| }; | |
| const exitOn = (signal: NodeJS.Signals) => { | |
| const handler = () => { | |
| cleanup(); | |
| process.removeListener(signal, handler); | |
| process.kill(process.pid, signal); | |
| }; | |
| process.on(signal, handler); | |
| }; |
| const relaxed = content.replace( | ||
| /script-src([^;]*)/, | ||
| (m, srcs: string) => `script-src${srcs} 'wasm-unsafe-eval'`, | ||
| ); |
There was a problem hiding this comment.
The current replacement assumes script-src is always present and is written in lowercase. If script-src is missing (e.g., the CSP only defines default-src), the replacement will fail silently. Additionally, CSP directives are case-insensitive. We should make the regex case-insensitive and provide a fallback to default-src or append a new directive if script-src is not found.
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'";
}![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| 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": "({})", | ||
| }, |
There was a problem hiding this comment.
"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.
| 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": "({})", | |
| }, | |
| 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": "({})" } : {}), | |
| }, |
Prompt To Fix With AI
This 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.| 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); | ||
| }); | ||
| }; |
There was a problem hiding this comment.
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.
| 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); | |
| }); | |
| }; | |
| 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); | |
| }; |
Prompt To Fix With AI
This 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.| const cspMetaRegex = | ||
| /<meta\s+http-equiv="Content-Security-Policy"\s+content="([^"]*)"\s*\/?>/i; |
There was a problem hiding this comment.
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.
| const cspMetaRegex = | |
| /<meta\s+http-equiv="Content-Security-Policy"\s+content="([^"]*)"\s*\/?>/i; | |
| // Match regardless of attribute order (http-equiv before or after content). | |
| const cspMetaRegex = | |
| /<meta\b[^>]*\bhttp-equiv="Content-Security-Policy"[^>]*\bcontent="([^"]*)"[^>]*\/?>/i; |
Prompt To Fix With AI
This 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.| if (!url) { | ||
| console.warn("vite did not report a local URL; skipping dev-server sidecar."); | ||
| return; | ||
| } |
There was a problem hiding this comment.
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.
Prompt To Fix With AI
This 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.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1657 +/- ##
==========================================
+ Coverage 53.07% 53.21% +0.13%
==========================================
Files 270 270
Lines 15715 15715
Branches 3411 3411
==========================================
+ Hits 8341 8363 +22
+ Misses 6260 6235 -25
- Partials 1114 1117 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
…-ui` Switches the block-ui branch of `ts-builder serve` from a child-process Vite spawn to a programmatic `createServer` call. After Vite reports its resolved local URL, write `<cwd>/dist/.dev-server` (a JSON sidecar with schema, url, pid) so the desktop app can pick up the live dev URL on the next block load. Cleanup on SIGINT / SIGTERM / exit / programmatic close. `browser` and `browser-lib` targets keep the existing spawn behavior — they don't need a sidecar. Adds `--sidecar-out <path>` for unusual block layouts.
Two `vite dev`-only relaxations so the dev-server-sidecar workflow can
load the block at all over http:
1. `transformIndexHtml` plugin appends `'wasm-unsafe-eval'` to the
`script-src` in the block's CSP meta tag. Production block UIs are
served via `block-ui://` which is registered with `bypassCSP: true`,
so the meta CSP never bites; http origin enforces it normally and
the in-page WASM driver (PFrameSpec) cannot compile without this.
CSP merging is by intersection — a response-header CSP cannot
loosen a meta-tag CSP, so rewriting the HTML at serve time is the
only mechanism.
2. `define: { "process.env": "({})" }` so transitive imports that
touch `process.env.X` at module top level (e.g.
pf-spec-driver/logging.ts) don't throw `process is not defined` in
the browser. Rolldown strips these in production builds; `vite dev`
does not.
Both are dev-only (the plugin is `apply: "serve"`, the define is
applied universally but the production rolldown pipeline overrides
it). Documented inline with the migration plan: drop the bypassCSP
hack on `block-ui://`, ship a canonical CSP in block templates that
includes `'wasm-unsafe-eval'`, replace the blunt `process.env`
substitution with a precise allowlist, and do a security review of
the block runtime end-to-end before treating either as steady state.
The first version of the `'wasm-unsafe-eval'` injector excluded both quote types from the content character class, so the capture group ended at the first single quote inside `content="script-src 'self' blob:"`. The replace never fired and the block view still hit `CompileError` on WebAssembly compile. CSP source keywords are always single-quoted inside a double-quoted content attribute. Restrict the regex to that shape — exclude only the delimiter (`[^"]*`). Verified end-to-end against `enter-numbers-v3` over `vite dev`: PFrameSpec / SpecDriver compiles, block renders, HMR confirmed by live-editing a .vue file with no reload.
Node fires same-signal handlers in registration order. When the cleanup
hooks were registered AFTER `vite.createServer().listen()`, Vite's own
internal handlers (and anyone else hooking signals during init) ran
first and could exit the process before our unlink ran, leaving stale
sidecars behind.
Restructure into a `setupSidecarLifecycle(path)` factory that:
- Registers SIGINT/SIGTERM/SIGHUP/exit/uncaughtException up-front
(before Vite has a chance to attach its own handlers).
- Exposes `publish(payload)` for after the server URL is known.
- Tracks a `live` flag so signal-triggered cleanup is idempotent and
a no-op when nothing was written.
- Still proxies through `server.close` so HMR-driven restarts and
programmatic shutdowns also unlink.
Verified end-to-end: direct `kill -TERM <pid>` against the node process
running `ts-builder serve` now removes the sidecar reliably.
dbolotin
force-pushed
the
feat/dev-server-sidecar
branch
from
b17e5bd to
54036b8
Compare
|
this is not ts-builder responsibility, it should be in block-tools. ts-builder - don't know nothing about platform |
2 participants
Summary
Adds dev-server sidecar mechanism: when block authors run
ts-builder serve --target block-ui, ts-builder writes<ui>/dist/.dev-server(atomic JSON:schema,url,pid) signalling the URL where the live Vite dev server is reachable. The desktop app reads this file and routes the block view to Vite instead of theblock-ui://folder protocol, enabling HMR. File is removed on clean shutdown; the desktop app HEAD-probes the URL before using it, so a stale sidecar safely falls back to folder mode.serve --target block-uiswitches from spawning Vite to using its programmatic API so we can write the sidecar with the actually-bound URL (port may differ from 5173).setupSidecarLifecycle()registersSIGINT/SIGTERM/SIGHUP/exit/uncaughtExceptioncleanup hooks beforecreateServer()to win the race against Vite's own signal handlers.apply: "serve") injects'wasm-unsafe-eval'into the index.html CSP meta tag so in-page WASM (used bySpecDriver) loads under dev. Production loads viablock-ui://withbypassCSP: trueand is unchanged. AFOLLOW-UPblock at the top ofcreateViteDevConfig.tsflags the migration debt (drop bypassCSP everywhere + security review).define: { "process.env": "({})" }so browser-side packages that runtime-referenceprocess.env.*(e.g.pf-spec-driver/logging.ts) don't crash on load.Consumer side (desktop app) lives in milaboratory/platforma-desktop-app# on branch
feat/dev-server-sidecar.Test plan
pnpm buildcleanenter-numbers-v3block: serve → desktop loads via Vite, label edit hot-reloadsGreptile Summary
This PR introduces a dev-server sidecar mechanism for
ts-builder serve --target block-ui: rather than spawning Vite as a child process, it uses Vite's programmatic API to discover the actual bound URL and atomically writes a.dev-serverJSON file that the desktop app reads to enable HMR. It also adds a dev-only Vite plugin to relax thescript-srcCSP for WASM and aprocess.envshim to silence transitive Node references in the browser.dev-server-sidecar.ts): registers SIGINT/SIGTERM/SIGHUP/exit/uncaughtExceptionhooks beforevite.createServer()to win the race against Vite's own signal handlers; atomic rename prevents half-written reads;server.close()is monkey-patched to cover programmatic/HMR restart paths.createViteDevConfig.ts):apply: \"serve\"scoped plugin rewrites theindex.htmlCSP meta tag to add'wasm-unsafe-eval'; accompanied by a detailed follow-up block listing migration debt.process.envdefine: not guarded byisServe, which may affect productionvite buildruns; the CSP regex also requires a specific attribute order that could silently miss non-canonical<meta>tags.Confidence Score: 3/5
Mostly safe to merge for dev-only use, but the unconditional process.env define warrants a check before landing.
The core sidecar mechanism is well-designed and the signal-ordering rationale is sound. The main concern is the process.env define being applied without an isServe guard — if createViteDevConfig is used for production builds, process.env.NODE_ENV will be undefined in bundles, potentially disabling dead-code elimination and leaving Vue dev warnings active in shipped builds. The CSP regex silently failing on non-canonical attribute order is a secondary concern.
tools/ts-builder/src/configs/utils/createViteDevConfig.ts — the unconditional process.env define and attribute-order-sensitive CSP regex both live here.
Important Files Changed
Sequence Diagram
sequenceDiagram participant Author as Block Author (CLI) participant TS as ts-builder serve participant SC as SidecarLifecycle participant Vite as Vite Dev Server participant FS as dist/.dev-server participant Desktop as Desktop App Author->>TS: ts-builder serve --target block-ui TS->>SC: setupSidecarLifecycle(sidecarPath) Note over SC: registers SIGINT/SIGTERM/SIGHUP/exit/uncaughtException TS->>Vite: vite.createServer(configFile, port, host) Vite-->>TS: server instance TS->>Vite: server.listen() Vite-->>TS: resolvedUrls.local[0] TS->>SC: "sidecar.publish({schema:1, url, pid})" SC->>FS: atomic write (tmp then rename) TS->>Vite: patch server.close() to call sidecar.remove() Desktop->>FS: read .dev-server JSON Desktop->>Vite: HEAD probe URL Vite-->>Desktop: 200 OK Desktop->>Vite: load block UI (HMR enabled) Author->>TS: Ctrl-C (SIGINT) TS->>SC: cleanup() SC->>FS: fs.unlink(.dev-server) TS->>TS: process.exit(0) Note over Desktop,FS: Stale sidecar (SIGKILL path): HEAD probe fails, fallback to block-ui:// folder protocolPrompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix(ts-builder): register sidecar cleanu..." | Re-trigger Greptile