Skip to content

feat(dev-hot-reload): streamline hot-reload #7599

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.

Sign up for GitHub

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

Closed
sid-bruno wants to merge 1 commit into from
Closed

feat(dev-hot-reload): streamline hot-reload #7599

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
320 changes: 254 additions & 66 deletions scripts/dev-hot-reload.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@
# npm run dev:watch -- [options]
*/

const { execSync } = require('child_process');
const { readFileSync } = require('fs');
const { execSync, spawn } = require('child_process');
const { readFileSync, readdirSync } = require('fs');
const path = require('path');

// Get major version from .nvmrc (e.g. v22.1.0 -> v22)
const NODE_VERSION = readFileSync('.nvmrc', 'utf8').trim().split('.')[0];
Expand All @@ -30,7 +31,6 @@ const CONFIG = {
'packages/bruno-js/src/',
'packages/bruno-schema/src/'
],
ELECTRON_START_DELAY: 10, // seconds
NODEMON_WATCH_DELAY: 1000 // milliseconds
};

Expand Down Expand Up @@ -68,6 +68,40 @@ function log(level, msg) {
}
}

class ProcessManager {
constructor() {
this._procs = [];
this._shuttingDown = false;
for (const sig of ['SIGINT', 'SIGTERM', 'SIGHUP']) {
process.on(sig, () => this.shutdown(sig));
}
Comment on lines +75 to +77
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

SIGHUP is Unix-only — ineffective on Windows.

Windows doesn't support SIGHUP. Node.js will ignore the listener or it simply won't fire. Consider conditionally registering it or removing it for a cleaner cross-platform experience.

Suggested fix 
-    for (const sig of ['SIGINT', 'SIGTERM', 'SIGHUP']) {
+    const signals = ['SIGINT', 'SIGTERM'];
+    if (process.platform !== 'win32') {
+      signals.push('SIGHUP');
+    }
+    for (const sig of signals) {
       process.on(sig, () => this.shutdown(sig));
     }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@scripts/dev-hot-reload.js` around lines 75 - 77, The loop registering signal
handlers currently includes 'SIGHUP', which is unsupported on Windows; update
the registration so 'SIGHUP' is only added on non-Windows platforms (e.g., check
process.platform !== 'win32' or use os.platform()) and keep the existing
handlers for 'SIGINT' and 'SIGTERM' calling this.shutdown(sig) — locate the
for-loop that iterates over ['SIGINT','SIGTERM','SIGHUP'] and change it to
conditionally include 'SIGHUP' (or register it separately inside an if block) to
avoid a no-op listener on Windows.

}

register(proc, name) {
this._procs.push({ proc, name });
proc.on('close', code => {
if (!this._shuttingDown && code !== null && code !== 0) {
log(LOG_LEVELS.ERROR, `Process "${name}" exited unexpectedly (code ${code}), shutting down...`);
this.shutdown('child-exit');
}
});
return proc;
}

shutdown(signal) {
if (this._shuttingDown) return;
this._shuttingDown = true;
log(LOG_LEVELS.INFO, `Received ${signal}, shutting down all processes...`);
for (const { proc } of this._procs) {
try { proc.kill(); } catch { /* already dead */ }
}
setTimeout(() => {
log(LOG_LEVELS.WARN, 'Force-exiting after timeout...');
process.exit(0);
}, 5000).unref();
Comment on lines +98 to +101
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ProcessManager.shutdown() always process.exit(0) after the timeout, even when it was triggered by an unexpected child exit (register()’s close handler) or a failed build. This can report success despite failures. Consider tracking an exit code (e.g. set to 1 on child failure) and exiting with that code.

Copilot uses AI. Check for mistakes.
}
}

// Show help documentation
function showHelp() {
console.log(`
Expand Down Expand Up @@ -136,72 +170,227 @@ function reinstallDependencies() {
log(LOG_LEVELS.SUCCESS, 'Dependencies re-installation completed');
}

// Setup development environment
function startDevelopment() {
log(LOG_LEVELS.INFO, 'Starting development servers...');

const concurrently = require('concurrently');
const watchPaths = CONFIG.ELECTRON_WATCH_PATHS.map(path => `--watch "${path}"`).join(' ');

// concurrently command objects: { command, name, prefixColor, env, cwd, ipc }
const commandObjects = [
{
command: 'npm run watch --workspace=packages/bruno-common',
name: 'common',
prefixColor: 'magenta'
},
{
command: 'npm run watch --workspace=packages/bruno-converters',
name: 'converters',
prefixColor: 'green'
},
{
command: 'npm run watch --workspace=packages/bruno-query',
name: 'query',
prefixColor: 'blue'
},
{
command: 'npm run watch --workspace=packages/bruno-graphql-docs',
name: 'graphql',
prefixColor: 'white'
},
{
command: 'npm run watch --workspace=packages/bruno-requests',
name: 'requests',
prefixColor: 'gray'
},
{
command: 'npm run watch --workspace=packages/bruno-filestore',
name: 'filestore',
prefixColor: '#FA8072'
},
{
command: 'npm run dev:web',
name: 'react',
prefixColor: 'cyan'
},
{
command: `sleep ${CONFIG.ELECTRON_START_DELAY} && nodemon ${watchPaths} --ext js,jsx,ts,tsx --delay ${CONFIG.NODEMON_WATCH_DELAY}ms --exec "npm run dev --workspace=packages/bruno-electron"`,
name: 'electron',
prefixColor: 'yellow',
delay: CONFIG.ELECTRON_START_DELAY
// Packages that have a build script but are launched separately
// and must not enter the watcher startup pipeline.
const DEV_EXCLUDED_PACKAGES = new Set(['@usebruno/app']); // launched via dev:web

/**
* Read every packages/* directory and return a descriptor for each
* @usebruno/* package that has at least a `build` npm script.
*
* @param {string} rootDir - absolute path to repo root
* @returns {{ name: string, dir: string, hasWatch: boolean, internalDeps: string[] }[]}
*/
function discoverPackages(rootDir) {
const pkgsDir = path.join(rootDir, 'packages');
const results = [];

for (const folder of readdirSync(pkgsDir)) {
const pjsonPath = path.join(pkgsDir, folder, 'package.json');
let raw;
try {
raw = JSON.parse(readFileSync(pjsonPath, 'utf8'));
} catch {
continue;
}
];

const { result } = concurrently(commandObjects, {
prefix: '[{name}: {pid}]',
killOthers: ['failure', 'success'],
restartTries: 3,
restartDelay: 1000
const name = raw.name || '';
if (!name.startsWith('@usebruno/')) continue;
if (DEV_EXCLUDED_PACKAGES.has(name)) continue;

const scripts = raw.scripts || {};
if (!('build' in scripts)) continue;

const allDeps = { ...raw.dependencies, ...raw.devDependencies };
const internalDeps = Object.keys(allDeps).filter(k => k.startsWith('@usebruno/'));

results.push({ name, dir: path.join(pkgsDir, folder), hasWatch: 'watch' in scripts, internalDeps });
}

return results;
}

/**
* Topologically sort packages into dependency stages using Kahn's algorithm.
* Deps that are not in the buildable set (no build script) are ignored.
* Throws if a cycle is detected.
*
* @param {{ name: string, internalDeps: string[] }[]} packages
* @returns {typeof packages[]}[] array of stages; packages within a stage are independent
*/
function topoSort(packages) {
const byName = new Map(packages.map(p => [p.name, p]));
const inDegree = new Map(packages.map(p => [p.name, 0]));
const adj = new Map(packages.map(p => [p.name, []]));

for (const pkg of packages) {
for (const dep of pkg.internalDeps) {
if (!byName.has(dep)) continue; // dep has no build script — skip
adj.get(dep).push(pkg.name);
inDegree.set(pkg.name, inDegree.get(pkg.name) + 1);
}
}

const stages = [];
let frontier = packages.filter(p => inDegree.get(p.name) === 0).map(p => p.name);

while (frontier.length > 0) {
stages.push(frontier.map(n => byName.get(n)));
const next = [];
for (const n of frontier) {
for (const neighbour of adj.get(n)) {
inDegree.set(neighbour, inDegree.get(neighbour) - 1);
if (inDegree.get(neighbour) === 0) next.push(neighbour);
}
}
frontier = next;
}

if (stages.reduce((acc, s) => acc + s.length, 0) !== packages.length) {
throw new Error('Cycle detected in @usebruno/* dependency graph');
}

return stages;
}

/**
* Discover all buildable @usebruno/* packages and return them as topo-sorted stages.
*
* @param {string} rootDir
* @returns {ReturnType<typeof topoSort>}
*/
function buildDevPackageGraph(rootDir) {
const packages = discoverPackages(rootDir);
log(LOG_LEVELS.DEBUG, `Discovered ${packages.length} buildable @usebruno/* packages`);
const stages = topoSort(packages);
stages.forEach((s, i) =>
log(LOG_LEVELS.DEBUG, `Stage ${i + 1}: ${s.map(p => p.name).join(', ')}`)
);
return stages;
}

// Color palette for auto-assigning colors to discovered package watchers
const WATCHER_COLORS = [
'\x1b[35m', // magenta
'\x1b[32m', // green
'\x1b[34m', // blue
'\x1b[37m', // white
'\x1b[90m', // gray
'\x1b[91m', // salmon
'\x1b[33m', // yellow
'\x1b[36m', // cyan
];
let _watcherColorIndex = 0;
function nextWatcherColor() {
return WATCHER_COLORS[_watcherColorIndex++ % WATCHER_COLORS.length];
}

// Spawn a process and prefix every line of its output with [name]
function spawnWithPrefix(cmd, args, name, color) {
const prefix = `${color}[${name}]${COLORS.nc} `;
const proc = spawn(cmd, args, { stdio: ['ignore', 'pipe', 'pipe'] });

proc.stdout.on('data', data =>
data.toString().split('\n').filter(Boolean).forEach(line =>
process.stdout.write(prefix + line + '\n')
)
);
proc.stderr.on('data', data =>
data.toString().split('\n').filter(Boolean).forEach(line =>
process.stderr.write(prefix + line + '\n')
)
);

return proc;
}

/**
* Start or build all packages in a single topo-sort stage, then resolve.
* - Packages with a `watch` script: spawned as long-lived watchers; waits
* for rollup's "created " signal before resolving.
* - Packages without a `watch` script (e.g. bruno-schema-types): one-time
* build only; not registered with ProcessManager (they exit by design).
*
* All packages in the stage are started concurrently.
*
* @param {{ name: string, dir: string, hasWatch: boolean }[]} stage
* @param {ProcessManager} procManager
* @param {number} stageIndex
*/
async function startWatcherStage(stage, procManager, stageIndex) {
log(LOG_LEVELS.INFO, `Stage ${stageIndex + 1}: ${stage.map(p => p.name).join(', ')}`);

const tasks = stage.map(pkg => {
const shortName = pkg.name.replace('@usebruno/', '');
const workspaceFlag = `--workspace=${pkg.dir}`;

if (pkg.hasWatch) {
const proc = procManager.register(
spawnWithPrefix('npm', ['run', 'watch', workspaceFlag], shortName, nextWatcherColor()),
Comment on lines +324 to +329
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

workspaceFlag is built from pkg.dir, which is an absolute path (path.join(rootDir, 'packages', folder)). Everywhere else in the repo uses --workspace=packages/<name> (relative), and npm’s --workspace generally expects a workspace name or a path relative to the workspace root. Consider storing a relative workspace spec (e.g. packages/${folder}) or using the package name instead, to avoid npm failing to resolve the workspace.

Copilot uses AI. Check for mistakes.
shortName
);
return new Promise(resolve => {
const onData = data => {
if (data.toString().includes('created ')) {
proc.stdout.off('data', onData);
proc.stderr.off('data', onData);
log(LOG_LEVELS.SUCCESS, `${shortName} — initial watch build done`);
resolve();
}
};
proc.stdout.on('data', onData);
proc.stderr.on('data', onData);
});
} else {
// One-time build — not registered with ProcessManager
log(LOG_LEVELS.INFO, `${shortName} — running one-time build (no watch script)`);
return new Promise((resolve, reject) => {
const proc = spawn('npm', ['run', 'build', workspaceFlag], { stdio: 'inherit' });
Comment on lines +345 to +348
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One-time build processes (packages without a watch script) are explicitly not registered with ProcessManager. If the user hits SIGINT during these builds, shutdown() won’t kill them and they can continue running orphaned in the background. Consider registering these build procs as well (possibly with a flag to allow normal exit) so they’re included in shutdown cleanup.

Suggested change
// One-time build — not registered with ProcessManager
log(LOG_LEVELS.INFO, `${shortName} — running one-time build (no watch script)`);
return new Promise((resolve, reject) => {
const proc = spawn('npm', ['run', 'build', workspaceFlag], { stdio: 'inherit' });
// One-time build — register with ProcessManager so it is cleaned up on shutdown
log(LOG_LEVELS.INFO, `${shortName} — running one-time build (no watch script)`);
return new Promise((resolve, reject) => {
const proc = procManager.register(
spawn('npm', ['run', 'build', workspaceFlag], { stdio: 'inherit' }),
shortName
);

Copilot uses AI. Check for mistakes.
proc.on('close', code => {
if (code === 0) {
log(LOG_LEVELS.SUCCESS, `${shortName} — build done`);
resolve();
} else {
reject(new Error(`Build failed for ${shortName} (exit ${code})`));
}
});
proc.on('error', reject);
});
}
});

result
.then(() => log(LOG_LEVELS.SUCCESS, 'All processes completed successfully'))
.catch(err => {
log(LOG_LEVELS.ERROR, 'Development environment failed to start');
console.error(err);
process.exit(1);
});
await Promise.all(tasks);
log(LOG_LEVELS.SUCCESS, `Stage ${stageIndex + 1} complete`);
}

// Setup development environment
async function startDevelopment() {
const rootDir = path.join(__dirname, '..');
const procManager = new ProcessManager();

const stages = buildDevPackageGraph(rootDir);
for (let i = 0; i < stages.length; i++) {
await startWatcherStage(stages[i], procManager, i);
}

log(LOG_LEVELS.SUCCESS, 'All watchers ready — starting React + Electron...');

procManager.register(
spawnWithPrefix('npm', ['run', 'dev:web'], 'react', '\x1b[36m'),
'react'
);

// Build nodemon args without a shell wrapper so signals propagate correctly
const nodemonArgs = [
...CONFIG.ELECTRON_WATCH_PATHS.flatMap(p => ['--watch', p]),
'--ext', 'js,jsx,ts,tsx',
'--delay', `${CONFIG.NODEMON_WATCH_DELAY}ms`,
'--exec', 'npm run dev --workspace=packages/bruno-electron'
];
procManager.register(
spawnWithPrefix('nodemon', nodemonArgs, 'electron', '\x1b[33m'),
'electron'
Comment on lines +383 to +392
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The nodemon invocation passes --exec as a single argv element containing spaces ('npm run dev --workspace=packages/bruno-electron'). nodemon does not shell-parse/split this value; it will treat the entire string as the executable name, which will fail to spawn. Pass --exec as 'npm' and provide the remaining tokens (run, dev, --workspace=...) as subsequent arguments (or use nodemon’s -- args mechanism).

Copilot uses AI. Check for mistakes.
);
}

// Main function
Expand All @@ -228,7 +417,6 @@ function startDevelopment() {
// Ensure required global packages and node version
ensureNodeVersion(CONFIG.NODE_VERSION);
ensureGlobalPackage('nodemon');
ensureGlobalPackage('concurrently');

// Run setup if requested
if (runSetup) {
Comment on lines 421 to 422
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

startDevelopment is now async, but it’s invoked later without await. If buildDevPackageGraph throws (e.g. cycle) or a one-time build rejects, the rejection won’t be caught by the outer .catch, and the script may emit an unhandled promise rejection. Ensure the call site awaits/returns the startDevelopment() promise so failures terminate deterministically.

Copilot uses AI. Check for mistakes.
Expand Down
Loading