feat(desktop): dev launcher to run the app in dev mode - #34
Merged
Conversation
…erver `pnpm dev:desktop` (or `pnpm --filter @open-wiki/desktop run dev:app`) builds the main/preload bundle, starts Vite on a fixed port 5173 (--strictPort), and once Vite is ready spawns Electron pointed at http://localhost:5173 via VITE_DEV_SERVER_URL — the hook the main process already had but no script wired up. Args after `--` are forwarded to Electron as literal argv, so `--project <dir>` opens a specific wiki instead of the launcher. Electron and Vite are spawned directly (no shell) so a `--project` path with spaces or metacharacters is not re-parsed by cmd.exe, and tearing down the launcher kills the real child processes instead of orphaning them on Windows (the inverse direction — Vite dying leaving Electron alive — is covered too). Co-Authored-By: Claude <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe PR adds root and desktop package scripts for a Node-based desktop development launcher. The launcher builds the main process, starts Vite, waits for readiness, launches Electron, forwards arguments, and handles failures and signals. ChangesDesktop development workflow
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Developer
participant PackageScripts
participant Launcher
participant Build
participant Vite
participant Electron
Developer->>PackageScripts: run dev:desktop
PackageScripts->>Launcher: invoke desktop launcher
Launcher->>Build: build main and preload
Build-->>Launcher: report build status
Launcher->>Vite: start on port 5173
Vite-->>Launcher: report readiness
Launcher->>Electron: launch with development URL and forwarded arguments
Electron-->>Launcher: report exit status
Launcher->>Vite: terminate Vite
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/dev-desktop.mjs`:
- Around line 112-120: Update the Vite stdout handler around the ready flag and
launchElectron so it accumulates startup chunks in a buffer before testing for
the readiness pattern. Preserve forwarding each chunk to process.stdout, and
call launchElectron once when the buffered output contains the complete “ready
in” message, including when that text spans multiple chunks.
- Around line 76-79: Handle spawn errors and peer-process cleanup across
scripts/dev-desktop.mjs: in the build promise around runPnpm, add a p error
listener that rejects through the existing non-zero shutdown path; in the vite
process flow, add an error handler that terminates Electron if it has started;
and in the electron process flow, add an error handler that terminates Vite.
Ensure all failures use one consistent non-zero shutdown path and prevent Vite
from remaining alive.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2e296974-0dc8-4ba0-a678-f178edcd0b26
📒 Files selected for processing (3)
apps/desktop/package.jsonpackage.jsonscripts/dev-desktop.mjs
Comment on lines
+76
to
+79
| await new Promise((res, rej) => { | ||
| const p = runPnpm(["--filter", "@open-wiki/desktop", "run", "build:main"]); | ||
| p.on("exit", (code) => (code === 0 ? res() : rej(new Error(`build-main exited ${code}`)))); | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- file ---'
cat -n scripts/dev-desktop.mjs
printf '%s\n' '--- spawn and lifecycle references ---'
rg -n -C 3 'runPnpm|spawn\\(|\\.on\\("(error|exit|close|spawn)"|vite\\.kill|electron\\.kill|process\\.exit' scripts/dev-desktop.mjs
printf '%s\n' '--- package/runtime context ---'
rg -n -C 2 '"(dev-desktop|build:main|vite|electron)"|dev-desktop' package.json pnpm-workspace.yaml scripts 2>/dev/null || trueRepository: protonspy/open-wiki
Length of output: 7153
🏁 Script executed:
#!/bin/bash
set -eu
node - <<'JS'
const { spawn } = require("node:child_process");
function probe(label, command, args, options = {}) {
return new Promise((resolve) => {
const child = spawn(command, args, options);
const events = [];
child.on("error", (err) => events.push(`error:${err.code}`));
child.on("exit", (code, signal) => {
events.push(`exit:${code}:${signal}`);
resolve({ label, events });
});
});
}
(async () => {
console.log(await probe("missing executable, shell:false", "__missing_spawn_probe__", [], {
stdio: "ignore",
shell: false,
}));
console.log(await probe("missing executable, shell:true", "__missing_spawn_probe__", [], {
stdio: "ignore",
shell: true,
}));
const child = spawn(process.execPath, ["-e", "process.exit(0)"], {
stdio: "ignore",
shell: false,
});
let errorListenerCount = 0;
errorListenerCount += child.listenerCount("error");
await new Promise((resolve) => child.once("exit", resolve));
console.log({ successfulSpawnErrorListenersBeforeExit: errorListenerCount });
})();
JS
printf '%s\n' '--- relevant source lines ---'
sed -n '64,132p' scripts/dev-desktop.mjsRepository: protonspy/open-wiki
Length of output: 2713
🏁 Script executed:
#!/bin/bash
set -eu
node - <<'JS'
const { spawn } = require("node:child_process");
async function withErrorHandler() {
const child = spawn("__missing_spawn_probe__", [], { stdio: "ignore", shell: false });
const events = [];
child.on("error", (error) => events.push(`error:${error.code}`));
child.on("exit", (code, signal) => events.push(`exit:${code}:${signal}`));
await new Promise((resolve) => child.on("close", resolve));
return events;
}
console.log(JSON.stringify(await withErrorHandler()));
const noHandler = spawn(
process.execPath,
["-e", "require('node:child_process').spawn('__missing_spawn_probe__')"],
{ stdio: ["ignore", "pipe", "pipe"] },
);
let stderr = "";
noHandler.stderr.on("data", (chunk) => (stderr += chunk));
const result = await new Promise((resolve) =>
noHandler.on("close", (code, signal) => resolve({ code, signal, stderr })),
);
console.log(JSON.stringify({
unhandledErrorExit: result.code,
unhandledErrorSignal: result.signal,
mentionsErrorEvent: /Error|error/i.test(result.stderr),
}));
const source = require("node:fs").readFileSync("scripts/dev-desktop.mjs", "utf8");
for (const variable of ["p", "vite", "electron"]) {
const declaration = source.indexOf(`const ${variable} =`);
const next = source.indexOf("\n", declaration);
const window = source.slice(declaration, declaration + 500);
console.log(JSON.stringify({
variable,
hasErrorListenerNearDeclaration: /\.on\(["']error["']|\.once\(["']error["']/.test(window),
}));
}
JSRepository: protonspy/open-wiki
Length of output: 1001
🏁 Script executed:
#!/bin/bash
set -eu
node --input-type=module - <<'JS'
import { spawn } from "node:child_process";
import { readFileSync } from "node:fs";
async function withErrorHandler() {
const child = spawn("__missing_spawn_probe__", [], { stdio: "ignore", shell: false });
const events = [];
child.on("error", (error) => events.push(`error:${error.code}`));
child.on("exit", (code, signal) => events.push(`exit:${code}:${signal}`));
await new Promise((resolve) => child.on("close", resolve));
return events;
}
console.log(JSON.stringify(await withErrorHandler()));
const noHandler = spawn(
process.execPath,
["-e", "require('node:child_process').spawn('__missing_spawn_probe__')"],
{ stdio: ["ignore", "pipe", "pipe"] },
);
let stderr = "";
noHandler.stderr.on("data", (chunk) => (stderr += chunk));
const result = await new Promise((resolve) =>
noHandler.on("close", (code, signal) => resolve({ code, signal, stderr })),
);
console.log(JSON.stringify({
unhandledErrorExit: result.code,
unhandledErrorSignal: result.signal,
mentionsErrorEvent: /Error|error/i.test(result.stderr),
}));
const source = readFileSync("scripts/dev-desktop.mjs", "utf8");
for (const variable of ["p", "vite", "electron"]) {
const declaration = source.indexOf(`const ${variable} =`);
const window = source.slice(declaration, declaration + 500);
console.log(JSON.stringify({
variable,
hasErrorListenerNearDeclaration: /\.on\(["']error["']|\.once\(["']error["']/.test(window),
}));
}
JSRepository: protonspy/open-wiki
Length of output: 434
Handle ChildProcess spawn failures and clean up the peer process.
When a child emits "error" without a listener, the launcher exits. If Electron fails after Vite starts, Vite remains alive and can keep port 5173 occupied. Add error handlers that route failures through one non-zero shutdown path:
p: reject the build promise.vite: terminate Electron if it started.electron: terminate Vite.
📍 Affects 1 file
scripts/dev-desktop.mjs#L76-L79(this comment)scripts/dev-desktop.mjs#L85-L89scripts/dev-desktop.mjs#L98-L109
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/dev-desktop.mjs` around lines 76 - 79, Handle spawn errors and
peer-process cleanup across scripts/dev-desktop.mjs: in the build promise around
runPnpm, add a p error listener that rejects through the existing non-zero
shutdown path; in the vite process flow, add an error handler that terminates
Electron if it has started; and in the electron process flow, add an error
handler that terminates Vite. Ensure all failures use one consistent non-zero
shutdown path and prevent Vite from remaining alive.
Comment on lines
+112
to
+120
| // Vite prints ` VITE vX.Y.Z ready in Nms` once the dev server is listening. | ||
| let ready = false; | ||
| vite.stdout.on("data", (chunk) => { | ||
| process.stdout.write(chunk); | ||
| if (!ready && /ready in/i.test(chunk.toString())) { | ||
| ready = true; | ||
| launchElectron(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
file=$(fd -t f '^dev-desktop\.mjs$' . | head -n 1)
printf 'FILE=%s\n' "$file"
ast-grep outline "$file"
printf '\n--- relevant source ---\n'
sed -n '1,180p' "$file"
printf '\n--- related references ---\n'
rg -n -C 3 'launchElectron|vite\.stdout|ready in|dev-desktop' . --glob '!node_modules'Repository: protonspy/open-wiki
Length of output: 19065
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- Vite declarations ---'
rg -n -C 2 '"vite"|vite:' package.json apps packages pnpm-lock.yaml --glob 'package.json' --glob 'pnpm-lock.yaml' | head -n 120
printf '%s\n' '--- split-chunk behavior ---'
node - <<'JS'
let launches = 0;
let ready = false;
const original = [" VITE v6.0.0 ready", " in 12ms\n"];
for (const chunk of original) {
if (!ready && /ready in/i.test(chunk.toString())) {
ready = true;
launches++;
}
}
console.log({ handler: "current", chunks: original, ready, launches });
launches = 0;
ready = false;
let startupOutput = "";
for (const chunk of original) {
if (!ready) {
startupOutput += chunk.toString();
if (/ready in/i.test(startupOutput)) {
ready = true;
launches++;
}
}
}
console.log({ handler: "buffered", chunks: original, ready, launches });
JSRepository: protonspy/open-wiki
Length of output: 2884
Buffer Vite startup output before matching readiness.
"data" handlers receive arbitrary chunks. If ready in spans two chunks, launchElectron() never runs. Accumulate startup output before applying the readiness check.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/dev-desktop.mjs` around lines 112 - 120, Update the Vite stdout
handler around the ready flag and launchElectron so it accumulates startup
chunks in a buffer before testing for the readiness pattern. Preserve forwarding
each chunk to process.stdout, and call launchElectron once when the buffered
output contains the complete “ready in” message, including when that text spans
multiple chunks.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed
Adds a dev launcher so the Electron desktop app can be run for visual
development against the Vite dev server — the
VITE_DEV_SERVER_URLhook themain process already had (
apps/desktop/src/main/index.ts), but no scriptwired up.
scripts/dev-desktop.mjs— builds the main/preload bundle, startsVite on a fixed port
5173(--strictPort), and once Vite is ready spawnsElectron pointed at
http://localhost:5173viaVITE_DEV_SERVER_URL.apps/desktop/package.json→"dev:app": "node ../../scripts/dev-desktop.mjs"package.json(root) →"dev:desktop"aliasUsage
Args after
--are forwarded to Electron as literal argv, so--projectworks as in the
owshim (seeapps/desktop/src/main/project.ts). The mainbundle is built once up front; editing
src/main/**requires restarting.The renderer hot-reloads through Vite.
How it was verified
pnpm --filter @open-wiki/desktop run test→ 12 files, 278 passedpnpm lint(root, covers the new script) +pnpm --filter @open-wiki/desktop run lint→ cleanscc validate→ no findingsbuild-main~120 ms, Viteready in ~185 ms, Electronwindow opens; on exit, port 5173 is released (no orphaned children)
Review
code-reviewandsecurity-reviewran on the diff (two rounds — the secondconfirmed the fixes to the first round's findings). Verdicts: security clean;
code-review findings all addressed:
directly (
shell:false, literal argv) rather than viapnpm exec …+ ashell string, so
--project C:\Users\Jane Doe\wikiis not re-split bycmd.exe.node <vite-cli>sovite.kill()reaches the real process (previouslykilled only a
cmd.exewrapper, leaving Vite holding port 5173 and breakingthe next launch's
--strictPort). The inverse direction (Vite dying leavingElectron alive) is handled too.
handler noted as POSIX-only.
Notes
.claude/rules/routing.mdthesmallest change ("the what was never in doubt") is a bare checklist; this
is a single dev-script addition driven by a direct request, so a plan was
not created.
pnpm installwas run during development to refresh stalenode_modulessymlinks (the repo directory had been moved and pnpm's symlinks pointed at a
dead path). The lockfile was already up to date; no dependency changed, so
docs/stack.mdis unaffected.🤖 Generated with Claude Code
Summary by CodeRabbit