From 92dcae512211675b8ea77a11af37d07846b7783b Mon Sep 17 00:00:00 2001 From: sjcoetzeesj-coder Date: Mon, 19 Oct 2026 13:40:36 +0200 Subject: [PATCH] fix(cipc): stop deadlocking wrapper on agent-browser daemon start runAb() in scripts/_cipc_service.js resolved only on proc.on('close'), but agent-browser is a daemon-per-session CLI. When it has to START the session browser daemon (first call after a reboot, after the daemon died, or after the app restarts), the new daemon inherits the spawned CLI stdout pipe. The pipe then never reaches EOF, so Node 'close' event never fires - even though the CLI exited cleanly with code 0. The 30s timeout in runAb called proc.kill(SIGTERM), but the process had already exited, so the kill was a no-op and the promise never settled. The wrapper serialised mutex then held inFlight forever, every later request queued behind it, and CIPC search silently 500-ed for days with no log line to show why - which is what made this look like a flaky / restart- only issue. Fixes: - runAb now settles on 'exit' (with a 250ms flush grace for stdio to drain), escalates the timeout kill to SIGKILL, and guarantees an unconditional forced settle if neither fires. The promise can no longer remain pending. - app/api/cipc/search/route.ts: ensureWrapperService() now probes port 6002 on every search and respawns the wrapper (rate-limited to once per 30s) if it is down. Hot reloads still no-op once a wrapper is up, but a dead wrapper now self-heals instead of 500-ing forever. Verified end to end: killed the browser daemon and the wrapper, started a fresh wrapper, its first isWarm() spawned a new daemon (the previous deadlock path) and returned in under 1s. A real enterprise search of K2018435959 came back in 31.7s on the cold path and 50.9s on the warm path with full company + director data. --- Workflow/app/api/cipc/search/route.ts | 22 +++++++++--- Workflow/scripts/_cipc_service.js | 50 ++++++++++++++++++++------- 2 files changed, 54 insertions(+), 18 deletions(-) diff --git a/Workflow/app/api/cipc/search/route.ts b/Workflow/app/api/cipc/search/route.ts index 1de7633..cf25f95 100644 --- a/Workflow/app/api/cipc/search/route.ts +++ b/Workflow/app/api/cipc/search/route.ts @@ -57,11 +57,19 @@ const SCRIPTS_DIR = path.join(process.cwd(), "scripts"); // fails the build and 500s the entire app. Route handlers are compiled for // the Node runtime, so the builtins resolve correctly. // -// Guarded by a module-level flag + a port probe, so repeated searches -// never spawn duplicates (including across dev-server hot reloads). +// Probes port 6002 on every search and respawns the wrapper (rate-limited) +// if it is down. The probe means the route self-heals: if the wrapper dies +// (or the agent-browser session it owns hangs) the next search brings it +// back instead of silently timing out at 165 s and 500-ing forever. Hot +// reloads in `next dev` are also fine — once a wrapper is up, the probe +// returns true and we no-op. // Set CIPC_AUTOSTART=0 to disable. const WRAPPER_PORT = 6002; -let autostartAttempted = false; +// Cooldown between respawns when the port is closed. A single bad spawn +// (e.g. the wrapper crashed on startup) must not trigger a respawn loop +// that pegs the CPU. +let lastWrapperSpawnMs = 0; +const WRAPPER_RESPAWN_COOLDOWN_MS = 30_000; function portIsOpen(port: number, ms: number): Promise { // Local require rather than the module-level `net` const declared further @@ -81,13 +89,17 @@ function portIsOpen(port: number, ms: number): Promise { } async function ensureWrapperService(): Promise { - if (autostartAttempted) return; - autostartAttempted = true; if (process.env.CIPC_AUTOSTART === "0") return; try { // Already running? Leave it alone. if (await portIsOpen(WRAPPER_PORT, 700)) return; + // Port is closed — respawn, but rate-limited so a startup failure + // can't turn into a tight respawn loop. + const now = Date.now(); + if (now - lastWrapperSpawnMs < WRAPPER_RESPAWN_COOLDOWN_MS) return; + lastWrapperSpawnMs = now; + const scriptPath = path.join(SCRIPTS_DIR, "_cipc_service.js"); if (!fs.existsSync(scriptPath)) { console.warn("[cipc] _cipc_service.js not found — skipping autostart"); diff --git a/Workflow/scripts/_cipc_service.js b/Workflow/scripts/_cipc_service.js index 5b5dcb7..21b74bc 100644 --- a/Workflow/scripts/_cipc_service.js +++ b/Workflow/scripts/_cipc_service.js @@ -92,30 +92,54 @@ function runAb(args, timeoutMs = 30_000) { // Runs a single agent-browser command by spawning the .exe directly // (no cmd.exe wrapper), which avoids quoting surprises on Windows. // Returns trimmed stdout; throws on timeout or spawn error. + // + // Why we settle on 'exit' rather than 'close': agent-browser is a + // daemon-per-session CLI. When it has to START the session's browser + // daemon (first call after a reboot, or after the daemon died), the new + // daemon inherits the spawned CLI's stdout pipe. The pipe then never + // reaches EOF, so Node's 'close' event never fires — even though the CLI + // exited cleanly with code 0. Listening for 'close' used to permanently + // wedge this mutex, so every later request queued forever and CIPC search + // 500-ed for days with no log line to explain it. We now settle on 'exit' + // after a short flush grace for stdio to drain, escalate the timeout kill + // to SIGKILL, and guarantee an unconditional forced settle if neither fires + // — so the promise cannot remain pending. return new Promise((resolve, reject) => { - let stdout = '', stderr = '', killed = false; + let stdout = '', stderr = '', settled = false, killEscalate = null; let proc; try { proc = spawn(AGENT_BROWSER_EXE, ['--session', SESSION, ...args], { windowsHide: true, }); } catch (e) { return reject(e); } + const finish = (fn, val) => { + if (settled) return; + settled = true; + clearTimeout(timer); + if (killEscalate) clearTimeout(killEscalate); + fn(val); + }; const timer = setTimeout(() => { - killed = true; - try { proc.kill('SIGTERM'); } catch {} + // SIGKILL maps to TerminateProcess on Windows and is a no-op if the + // process has already exited, so we also schedule an unconditional + // forced settle shortly after. + try { proc.kill('SIGKILL'); } catch {} + killEscalate = setTimeout( + () => finish(reject, new Error(`agent-browser timeout (${timeoutMs}ms)`)), + 2000 + ); }, timeoutMs); proc.stdout.on('data', (d) => (stdout += d)); proc.stderr.on('data', (d) => (stderr += d)); - proc.on('error', (e) => { - clearTimeout(timer); - reject(e); - }); - proc.on('close', (code) => { - clearTimeout(timer); - if (killed) return reject(new Error('agent-browser timeout')); - // agent-browser returns 0 even on bad JSON-eval results — the - // actual error message is encoded in the JSON-stringified stdout. - resolve({ stdout: stdout.trim(), stderr: stderr.trim(), code }); + proc.on('error', (e) => finish(reject, e)); + proc.on('exit', (code) => { + // 'close' can never fire reliably here (see header); give stdio a + // brief moment to flush, then settle. agent-browser returns 0 even + // on bad JSON-eval results — the actual error is in the stdout. + setTimeout( + () => finish(resolve, { stdout: stdout.trim(), stderr: stderr.trim(), code }), + 250 + ); }); }); }