fix(services): stop embedded-service supervisor retry loop when binary cannot spawn (#9937)

* fix(deps): bump nanoid, dompurify for 2 new Dependabot alerts (#189, #190)

Bumps: nanoid ^3.3.17 (was transitive, now overridden), dompurify ^3.4.13
(with monaco-editor scoped override). Closes Dependabot #189, #190.

Remaining #182-#188 (js-yaml + mermaid) already closed by #9651 merge —
awaiting Dependabot re-scan.

npm audit → 0 vulnerabilities.

* fix(repo): harden .gitignore to also ignore a _tasks symlink (/_tasks)

_tasks is a SEPARATE nested git repo (gitignored). The pattern _tasks/ (trailing
slash) ignores only a directory, not a SYMLINK named _tasks. A self-referential
_tasks symlink can slip in via git add -A and, once pulled, checkout materializes
it over the real _tasks repo (destroying plans/specs/hands-off). Anchored /_tasks
ignores the symlink too, preventing re-capture.

* fix(services): stop embedded-service supervisor retry loop when binary cannot spawn

A non-spawnable supervised binary (ENOENT/EACCES, or an ELF on Windows
where spawn() throws EFTYPE synchronously) left the supervisor in
'starting' forever while the HealthChecker polled the dead port every
healthIntervalMs. Each failed probe fired a full ProxyFetch
dispatcher+native fetch pair, burning CPU and eventually collapsing the
server (observed: 24 warns/min against 127.0.0.1:8317 for 2 days).

- handle synchronous spawn() throws and the child 'error' event: stop
  the poller and transition to an explicit error state
- transition to error and stop polling when FAILURE_THRESHOLD
  consecutive health probes fail, including during startup
- waitForHealthy re-checks the state after its deadline so a
  mid-startup error surfaces as a rejection instead of being overwritten
  by 'running'

---------

Co-authored-by: diegosouzapw <diegosouzapw@users.noreply.github.com>
Co-authored-by: Diego Rodrigues de Sa e Souza <diegosouza.pw@outlook.com>
Co-authored-by: diegosouzapw <diegosouza.pw@gmail.com>
Co-authored-by: herjarsa <herjarsa@users.noreply.github.com>
This commit is contained in:
Hernan Javier Ardila Sanchez
2026-08-10 08:49:38 +02:00
committed by GitHub
parent 0dbc34ea56
commit 0cb7410ca6
2 changed files with 141 additions and 5 deletions

View File

@@ -56,6 +56,19 @@ export class ServiceSupervisor extends EventEmitter {
this.checker = new HealthChecker(config.healthUrl, config.healthIntervalMs, (h) => {
this.health = h;
this.emit("stateChange", this.getStatus());
// A service that fails FAILURE_THRESHOLD consecutive health probes will
// not recover by itself. Stop the poller and surface an explicit error
// state instead of probing the dead port forever — every failed probe
// fires a full ProxyFetch dispatcher+native fetch pair (e.g. against a
// CLIProxyAPI binary that cannot execute on this platform).
if (h === "unhealthy" && (this.state === "running" || this.state === "starting")) {
this.checker.stop();
this.lastError = sanitizeErrorMessage(
`Health probe failed for ${this.config.tool} (port ${this.config.port})`
);
this.setState("error");
void setToolStatus(this.config.tool, "error", undefined, this.lastError);
}
});
}
@@ -135,7 +148,22 @@ export class ServiceSupervisor extends EventEmitter {
const { command, args, env, cwd } = this.config.spawnArgs();
const child = spawn(command, args, buildServiceSpawnOptions(env, cwd));
// spawn() can throw SYNCHRONOUSLY on Windows when the binary is not
// executable (EFTYPE/EINVAL for an ELF or a plain text file) instead of
// emitting the child 'error' event. Handle both paths identically so a
// non-spawnable service surfaces an explicit error state and the health
// poller is stopped instead of hammering a dead port forever.
let child: ChildProcess;
try {
child = spawn(command, args, buildServiceSpawnOptions(env, cwd));
} catch (err) {
this.checker.stop();
const msg = sanitizeErrorMessage(err instanceof Error ? err.message : String(err));
this.lastError = msg;
this.setState("error");
await setToolStatus(this.config.tool, "error", undefined, msg);
return this.getStatus();
}
this.childProcess = child;
this.pid = child.pid ?? null;
@@ -161,6 +189,17 @@ export class ServiceSupervisor extends EventEmitter {
child.once("exit", (code, signal) => {
void this.handleExit(code, signal, spawnTime);
});
// Spawn failures (ENOENT, EACCES, or a non-executable binary such as an
// ELF on Windows) surface via the child 'error' event — NOT 'exit'.
// Without this handler the supervisor stays in "starting" forever and
// the health poller hammers the dead port every healthIntervalMs.
child.once("error", (err) => {
this.checker.stop();
const msg = sanitizeErrorMessage(err instanceof Error ? err.message : String(err));
this.lastError = msg;
this.setState("error");
void setToolStatus(this.config.tool, "error", undefined, msg);
});
this.startedAt = new Date().toISOString();
this.checker.start();
@@ -224,10 +263,12 @@ export class ServiceSupervisor extends EventEmitter {
if (this.state === "error") throw new Error(this.lastError ?? "Service failed to start");
await new Promise((r) => setTimeout(r, 1_000));
}
// Timeout reached without a healthy probe. Surface this so callers /
// dashboards do not see "running" + "unknown" health silently. We do not
// throw — the service may still be initializing — but we DO record a
// degraded marker so /status returns it and operators can act.
// Timeout reached without a healthy probe. The health poller may have
// flipped the state to "error" while we were waiting (FAILURE_THRESHOLD
// consecutive failures) — surface that instead of a degraded marker.
if (this.state === "error") {
throw new Error(this.lastError ?? "Service failed to start");
}
this.lastError = sanitizeErrorMessage(
`Health probe did not succeed within ${timeoutMs}ms — service may still be initializing`
);