feat(tools-pack): cross-platform WebUI (no-Electron) packaging type - #3509
feat(tools-pack): cross-platform WebUI (no-Electron) packaging type#3509SugarFatFree wants to merge 48 commits into
Conversation
`tools-pack webui build --platform <mac|win|linux> [--arch ...]` produces a terminal-launched distribution (daemon + web in server output mode) that runs on the user's system Node 24 — no Electron. Bundles the workspace node app, the target's prebuilt better-sqlite3, the resource trees, and terminal launchers (open-design start|stop|status) plus double-click wrappers. - assemble.ts: extract the shared assembly core (workspace tarballs, node-app install, buildWorkspaceArtifacts, copyResourceTree) from linux.ts so the Linux AppImage lane and the WebUI lane share one path. - webui.ts: per-platform archive (zip on mac/win, tar.gz on linux), target better-sqlite3 prebuild selection, and pruneBuildOnlyNativeModules to strip the build-only @next/swc compiler (~125MB) the server runtime never loads. - apps/packaged: webui-launcher (start/stop/status) detaches into the background by default (--foreground to stay attached), auto-creates webui.config.json on first run, supports web + daemon port / host / token config, prints the LAN IP for 0.0.0.0 binds, persists an auto-generated remote token, and localizes output (en + zh-CN). startPackagedSidecars gains optional network injection; default callers are unchanged. - apps/daemon: resolveProcessResourcesPath recognizes the packaged node-app layout (daemon under <appRoot>/node_modules) so OD_RESOURCE_ROOT validates for the system-Node runtime; waitForStatus embeds the daemon log tail so a startup crash surfaces in the terminal. Both surfaces (build CLI + launcher CLI) are CLI-only by nature; no web UI surface applies. Validated with pnpm guard, pnpm typecheck, and the tools-pack / packaged / daemon test suites.
lefarcen
added
size/XXL
nettee
left a comment
nettee
left a comment
There was a problem hiding this comment.
Two blocking path/config regressions showed up in the new launcher flow: stop/status do not target the same runtime when config-driven namespace/data-dir overrides are used, and scoped dataDir values are normalized incorrectly. Details inline.
…malize scoped OD_DATA_DIR Two path/config regressions in the WebUI launcher flagged in review: 1. stop/status ignored config-driven namespace. `start` derives its desktop IPC socket namespace from the resolved config (which may set `namespace`), but `commandStopOrStatus` always rebuilt the namespace from OD_PACKAGED_NAMESPACE/defaults. A config-driven instance was therefore reported as "not running" by a later `status`/`stop`. Both surfaces now derive the namespace through one shared `resolveRuntimeNamespace()` helper, and `main()` loads the same config (without scaffolding) for stop/status. 2. scoped OD_DATA_DIR got a doubled `namespaces` segment. `resolveNamespaceBaseRoot()` blindly appended `/namespaces`, so a scoped value (`<base>/namespaces/<ns>/data`, already supported by paths.ts) became `.../data/namespaces`, forking the launcher's runtime/log tree from the daemon data dir. Extracted `resolveWebuiNamespacesRoot()` in paths.ts which reuses the same scoped-vs-unscoped rule as `resolvePackagedDataRoot()`. Adds unit specs for both helpers (paths.test.ts, webui-config.test.ts).
|
Thanks for the careful review — both regressions confirmed and fixed in 4112f46. 1. 2. Scoped Added unit specs for both helpers ( |
nettee
left a comment
nettee
left a comment
There was a problem hiding this comment.
Two correctness issues still need to be closed out in the new WebUI launcher flow: the explicit --config token-persistence path can change the runtime settings on the next restart, and the new --host surface still emits malformed URLs for concrete IPv6 binds. Details inline.
…en scaffolding webui.config.example.json now annotates every field (including the optional namespace/dataDir/lang keys) with `// <field>` sibling comment keys. These keep the example a valid JSON document while explaining each option in place. First-run scaffolding no longer copies the example verbatim. It seeds webui.config.json from the example's real values but strips every `//` documentation key, so the generated live config is pure data: it can never fail JSON.parse and never carries a description string as a bogus value (e.g. an optional field documented only by a `// namespace` key does not leak a `namespace` value). A missing or unparseable example falls back to built-in defaults so first run always yields a valid config.
nettee
left a comment
nettee
left a comment
There was a problem hiding this comment.
Two blocking launcher regressions are still present on the current head: the explicit --config remote-start path can persist a config that changes the next restart's runtime settings, and concrete IPv6 --host values still produce malformed URLs in the user-facing/banner path.
The upstream project is English-first. Translate the WebUI distribution's user-facing resources (README, webui.config.example.json field docs, open-design.sh/.cmd messages, launch-mac.command prompt) and the WebUI source comments/error strings to English. The zh-CN entries in webui-i18n.ts are intentionally kept — they are the launcher's Chinese localization table (the launcher output stays bilingual, selected by --lang / system locale), not comments.
Render the WebUI packaging-type design doc and implementation plan fully in English to match the upstream English-first convention, including the illustrative code/script samples and embedded README inside the plan. Structure, code, paths, flags, and commands are preserved.
…plicit --config
Two correctness issues flagged in review:
1. Concrete IPv6 binds produced malformed URLs. resolveDisplayHost() passes a
bare IPv6 literal (e.g. `fd00::10` from `--host fd00::10`) through unchanged,
but browserUrl()/daemonDirectUrlFor() interpolated it as
`http://${host}:${port}`, yielding the unparseable `http://fd00::10:7456`
that was printed, copied, and handed to openBrowser(). Added
formatHostForUrl()/composeHttpUrl() (a single bracketing chokepoint) and
routed both URLs through it; added IPv6 regression specs.
2. Explicit `--config <path>` token persistence created a lossy config.
discoverConfigFile() deliberately skips scaffolding for an explicit path, so
a remote first start (`start --host 0.0.0.0 --config /missing.json`) wrote a
`{ token }`-only file; the next `start --config` then fell back to defaults
(host/port/namespace/dataDir all lost). persistTokenToConfig() now
materializes the FULL resolved shape when the target file does not exist
(dynamic daemonPort round-trips as 0), so a follow-up start reproduces the
same runtime. Existing files keep their own keys + token as before.
|
Both new issues confirmed and fixed in 747aa0f. 1. Malformed URLs for concrete IPv6 binds ( 2. Explicit I took the "write the resolved config shape" option (the remote flow this feature enables should just work).
|
…rely A second `start` on the same namespace (instance already running) used to spawn a worker that collided on the IPC socket and the web/daemon ports; the worker died and the user saw a confusing "failed to start, see log". Add a pre-flight desktop IPC STATUS probe: if an instance is already serving the namespace, print its address (+ the stop hint) and exit 0. The probe is fast when nothing is running (a non-existent socket errors immediately) and never false-positives on a stale socket. Extracted probeWebuiStatus() and reused it in waitForWebuiReady().
…ailures
1. The Linux double-click entry was never executable. Packaging chmod'd only
open-design.sh and the macOS .command; open-design-webui.desktop (tracked
100644) was copied as-is, so many file managers refuse to launch it — breaking
the double-click contract in the WebUI README. Extracted
stageWebuiLauncherResources() and chmod the staged .desktop to 0o755; added a
tools/pack regression test that stats the staged mode (Linux + macOS).
2. stop/status collapsed every IPC failure into "not running". A 2s timeout,
permission error, or protocol/ok:false rejection was reported as
`{"state":"stopped"}` / "not running", so a wedged-but-live worker made
`status` lie and `stop` silently orphan the service. Added
isNotRunningIpcError() (only ENOENT / ECONNREFUSED mean not-running) and
rethrow everything else so operators see the real error; added a classifier
regression test covering the timeout/permission/protocol cases.
|
Both confirmed and fixed in 6093d91. 1. Linux 2. stop/status swallowed every IPC failure as "not running" (
Note: while here I also made a repeated |
nettee
left a comment
nettee
left a comment
There was a problem hiding this comment.
One blocking launcher-path regression is still present on the current head: the new status-probe helper suppresses real IPC failures and can misclassify a wedged live instance as absent, so start can race into a duplicate launch and the readiness path can hide the underlying bug. Details inline.
probeWebuiStatus() caught every requestJsonIpc() failure and returned null, even though isNotRunningIpcError() was just added to tell a dead socket from a real IPC bug. So the pre-flight `start` path could treat a live-but-wedged worker (timeout, malformed reply, permission error) as "not running" and spawn a second instance on the same namespace/ports, and waitForWebuiReady() would poll to the generic timeout instead of surfacing the actual failure. Extracted probeWebuiStatus() into a side-effect-free, requester-injectable webui-ipc module and gated its catch with isNotRunningIpcError(): only ENOENT / ECONNREFUSED resolve to null (not running); every other failure is rethrown. The `start` pre-flight now surfaces a wedged-worker error instead of racing into a duplicate launch, and waitForWebuiReady() reports the real failure (with the log tail) instead of polling to timeout — while a still-binding socket (ENOENT/ ECONNREFUSED) keeps polling as before. Added a launcher-level regression test covering the timed-out / permission / protocol / not-running / running cases.
|
Confirmed and fixed in c605e5b — and you're right that this is the same class of bug as the
Effects:
Added a launcher-level regression test (
|
nettee
left a comment
nettee
left a comment
There was a problem hiding this comment.
One blocking launcher correctness issue remains on the current head, and there is one release-workflow packaging issue worth tightening before this ships.
🔁 Powered by Looper · runner=reviewer · agent=codex · An autonomous AI dev team for your GitHub repos.…h exactly one archive 1. probeWebuiStatus() returned null when a connected STATUS reply carried no url, conflating a live-but-wedged/regressed worker with a not-running socket. Callers use null to mean "namespace free" (start pre-flight → duplicate launch) and "keep polling" (waitForWebuiReady → hidden error). Once request() resolves the socket is live and the reply shape is the invariant, so a missing/empty url is now thrown as an `invalid STATUS reply` error; only ENOENT/ECONNREFUSED still map to not-running. Updated the launcher-level spec so the no-url case is a reject expectation. 2. The release workflow's artifact upload used a recursive glob over the tools-pack tree, which is broader than the one-archive-per-entry release contract and risks sweeping in internal staging files. Now reads the build JSON's archivePath and uploads exactly that file (validated with test -f), so each matrix entry publishes precisely one archive.
|
Both addressed in 3c15c07. 1. 2. Release workflow — publish exactly one archive per matrix entry ( One factual note for the record: I checked the previous glob
|
nettee
left a comment
nettee
left a comment
There was a problem hiding this comment.
@SugarFatFree I re-checked the WebUI pack lane, the packaged launcher/runtime wiring, the daemon and web host/resource-root handling, and the added regression coverage on the current head. The changed ranges still line up with the runtime invariants around config-driven namespace and data-dir targeting, IPv6-safe URLs and proxying, detached start/status lifecycle, and namespace-scoped archive publishing, and I didn’t find a remaining actionable issue in this diff. I couldn’t rerun the Vitest/package slices locally because this reviewer worktree has no node_modules, but the implementation itself looks consistent and ready to merge. Thanks for pushing through a fairly tricky packaging surface.
|
Friendly nudge 🙏 — head State is @PerishCode @lefarcen whenever you have a moment, could you kick off CI for this head? Happy to rebase again if more upstream lands. Thanks! |
|
Hey @SugarFatFree — thanks for the clear handoff after the upstream merge. I re-checked the changed files on |
|
Gentle morning ping 🌅 — this is still green and ready on head @PerishCode @lefarcen whenever you get a chance, a single click will let CI run and unblock the merge. Happy to re-sync if more upstream lands. Thanks! |
|
Hey @SugarFatFree — saw the update. I re-checked the changed-file set on |
lefarcen
added
the
needs-product-review
![open-design-crew[bot]](https://avatars.githubusercontent.com/in/4197780?s=60&v=4){kind=small}
|
@SugarFatFree Could you rebase this onto the latest |
lefarcen
removed
the
needs-product-review
…tream merge The upstream merge added win-manifest/win-nsis test files whose ToolPackConfig fixtures omit the arch field. This branch makes arch a required field on ToolPackConfig, so those fixtures fail typecheck. Set arch: "x64" to match the idiom used across the existing tools-pack test fixtures.
|
@open-design-crew Done — the branch is now updated to the latest A note on merge vs. rebase: this branch already carries several One small integration fix was required and is included as Local validation is green against the updated base:
Ready for validation/merge on |
|
Hey @SugarFatFree — I re-checked the current head around @nettee's latest review ( The actionable blocker still looks like the same one he called out: the WebUI path now forwards Once |
…es origin validation In the WebUI split-port runtime the browser loads the web sidecar on the browser-facing port (default 7456) and its /api requests are proxied to the daemon (default 7457) carrying Origin: http://<host>:7456. The daemon's origin-validation only allow-lists a browser origin whose port is OD_PORT or OD_WEB_PORT (apps/daemon/src/origin-validation.ts:allowedBrowserPorts). But buildPackagedDaemonSpawnEnv() forwarded the daemon bind host and token yet never OD_WEB_PORT, so the daemon only knew 7457 and 403'd legitimate browser /api traffic even though 'open-design start' succeeded. Forward network.webPort as OD_WEB_PORT when it is a concrete port > 0. The ephemeral --port 0 case cannot be validated up front (the daemon is spawned before the web child binds and has no runtime origin-update channel), so resolveWebuiConfig now rejects a browser-facing port 0 with an actionable message; the internal daemonPort 0 (a loopback port the browser never sees) stays valid. Red specs: sidecars.test.ts pins OD_WEB_PORT forwarding (and its omission when no concrete web port is given); config.test.ts pins the port-0 rejection.
…pback resolveWebuiConfig() documents daemonPort 0 as the single dynamic-loopback escape hatch, but `daemonPortRaw > 0 ? daemonPortRaw : null` also folded every negative value (from --daemon-port, webui.config.json, or OD_PORT) into that same null path. A typo like `--daemon-port -1` would then silently launch the daemon on a random loopback port instead of failing fast, so the config surface lied about which values are valid. Reject a negative (or non-integer) daemonPort with a clear error; 0 still means dynamic and positive ports are honored. Mirrors the browser-facing port <= 0 rejection added just above. Red spec in config.test.ts covers flag/config/env.
|
Hey @SugarFatFree — the actionable blocker on the current head is @nettee's latest config follow-up. Right now |
nettee
left a comment
nettee
left a comment
There was a problem hiding this comment.
@SugarFatFree I re-checked the current head's launcher follow-up around OD_WEB_PORT forwarding, config-driven namespace/data-root resolution, IPv6-safe URL/proxy handling, and the final daemonPort validation tightening. The changed ranges and added regression coverage line up with those invariants, and I didn’t find a remaining actionable issue in this diff. I couldn’t rerun the Vitest/package slices locally because this reviewer worktree has no node_modules, but the implementation itself looks consistent and ready to merge. Thanks for closing out a fairly tricky packaging surface.
|
@lefarcen — both of the follow-ups you called out are now closed on head
Both landed red-spec-first; local gate is green ( The only thing left is the maintainer-side Approve and run workflows on the fork PR (the |
|
Thanks for the detailed write-up — the two launcher/config follow-ups are easy to trace from here, and this looks workflow-side rather than another author-side change at this point. Once the maintainer-side fork workflow gate is cleared, the PR should be back in merge territory. |
|
Gentle daily nudge 🙏 — head |
|
Thanks for the nudge, @SugarFatFree — I re-checked the WebUI packaging/runtime changes on this head. This still looks like a QA-needed runtime path rather than a visual design review case, so I've handed it to QA now. Once that validation clears, the remaining maintainer-side workflow step should be the last gate from the bot side. |
|
Perfect, thanks for routing it to QA @lefarcen 🙏 — that makes sense for a runtime/packaging path. I'm on standby: if QA turns up anything on the terminal-launched WebUI runtime (split-port |
|
Thanks — that’s helpful context. If QA hits the split-port/origin-validation path or wants a quick packaging-lane walkthrough, I’ll point them back to this thread so they can grab repro steps from you directly. |
|
Hi @SugarFatFree, thank you again for the thoughtful proposal and the amount of care you put into this WebUI packaging lane. We really appreciate the detailed implementation, the follow-up fixes, and the patience throughout the review process. After an internal assessment, we are going to close this PR for now. The packaging and distribution area touches our release pipeline, runtime layout, platform-specific assembly, and longer-term maintenance boundaries, and at this stage we prefer to keep this part of the system maintained by the core team. We are also actively working on packaging-related improvements internally, so we want to avoid creating parallel ownership or a public contribution surface that we may not be able to support well yet. This is not a reflection on the quality of your contribution. The PR surfaced useful requirements and edge cases for headless/server-style WebUI usage, and we will keep that context in mind as we continue improving the packaging story internally. Thanks again for the contribution and for all the iteration here. |
6 participants
Closes #3508
Why
Use case: I run Open Design on a headless Linux server and wanted an Electron-free, terminal-launched, browser-accessed deployment that's also easy to embed into another agent through the
od/ launcher CLI. Pain: the only packaged builds are the mac/Windows GUI installers, leaving headless/container setups with no supported path. Opened #3508 first to align on scope.What users will see
tools-pack webui build --platform <mac|win|linux> [--arch ...]command that produces a terminal-launched distribution.open-design start|stop|status: runs in the background by default (--foregroundto stay attached), prints the access URL, configurable port/host/token, auto-createswebui.config.jsonon first run, shows the LAN IP for0.0.0.0binds, and localizes its output (en/zh-CN).Surface area
tools-pack webuisubcommand + theopen-designlauncher CLI (--port/--daemon-port/--host/--token/--foreground/--lang)Screenshots
N/A — CLI only, no UI.
Validation
pnpm guard,pnpm typecheck— greenpnpm --filter @open-design/tools-pack test(168),@open-design/packaged test(118; the 2 desktop suites fail only because this environment has no Electron binary, unrelated to this change),@open-design/daemonserver-paths (13)tools-packandpackagedbuilds passlinux.tsintoassemble.ts(shared by the Linux AppImage lane and the WebUI lane), and repoints upstream fix(pack): add missing download and host packages to Linux INTERNAL_PACKAGES #2837'sinternal-packages-coverageregression test at the relocatedINTERNAL_PACKAGESdefinition.