fix(sdk): reclaim the process a failed service leaves behind

[matej21]

Production evidence

A project's E2B sandbox (4 GB, no swap) hosts every session of that project, and each
session's astro-dev (pletivo) dev server costs 535–613 MB. A live census found a dev
server for session 019fefe1 holding 535 MB, alive, while services.list reported
its astro-dev as failed. That process was 3h42m older than the agent process, so
it had already survived at least one agent restart.

It is a self-reinforcing loop: memory pressure → startup timeout → failed → ~535 MB
leaked → more pressure → more timeouts.

On the same sandbox another session had astro-dev failed with no process alive —
failed does not imply "leaked process", so nothing here kills on failed alone.

What was actually wrong

Services are configured with startupTimeoutMs: 30000 and a readyPattern. A startup
timeout ends the wait, not the process.

1. Abandoned spawns. The startup-timeout handler did already SIGKILL the process
group (verified against origin/main — see "Verification"), but it was the only path
that did, it was unguarded against PID reuse, and the surrounding paths abandoned
processes freely:

• ServiceExecutor.start() over an entry that is failed, paused or stopping
  overwrote this.services[type] and the pid-registry record — both keyed by
  (session, type) — so the previous generation's pid was lost for good. The new
  process then tried to bind the very port the abandoned one was still holding, since a
  restart deliberately reuses it.
• stop() returned Err("Service X is failed, cannot stop"). The platform-side reaper
  is the one caller that can free a leaked dev server's memory, and it had no way to.
• The child.on('error') handler marked the service failed without reclaiming
  anything.

2. The boot orphan sweep skipped it. onSessionReady reconciled only
entry.status === 'starting' | 'ready' | 'paused', so failed was never reclaimed at
the next boot either.

3. (found while verifying) The pid was erased before the sweep could use it. Two
reducers deleted the only handle the sweep has:

• the failed branch cleared pid/pidStartTime, so extending the status list in (2)
  alone would have reconciled nothing;
• session_restarted — emitted by openSession immediately before onSessionReady
  — marked running services stopped and cleared their pid, without killing anything.
  This is the likeliest path for the observed production artifact: a gen-1 dev server
  becomes invisible to every later boot, keeps its port, and the failed in the
  projection belongs to a later, different spawn that then lost the port race.

Changes

For (1) — reclaim at the source. New reapServiceProcessGroup(fs, logger, target, context) in service.ts: one PID-reuse-guarded kill(-pid, SIGKILL). Every path that
gives up on a service now goes through it — startup timeout, child.on('error'), a
start that supersedes an entry still owning a process (reaped just before the spawn, so
the port is free), and stop(), which now reclaims a failed service and settles it as
stopped instead of refusing. RunningService carries the pidStartTime captured at
spawn plus an exited flag so an already-exited entry is never re-killed. The pid
registry's killIfStillOurs and the boot reconcile now share that same function instead
of keeping two copies of the guard.

Also fixed while in there: the close handler dropped the pid-registry record without
checking it still owned the type, so a superseded generation's exit could delete its
successor's record.

For (2) — sweep what earlier generations left. The onSessionReady reconcile is
driven off the recorded pid rather than a list of running statuses: a process we spawned
and never saw exit is an orphan whatever the entry says. failed keeps its status and
its error through the reconcile — only its claim on a process is taken away — while
everything else settles as stopped, as before.

For (3). The failed reducer branch now takes the pid from the event (the executor
reports the pid it could not confirm gone, and nothing once the process is known to have
exited), and session_restarted no longer clears it.

Guarding throughout is the existing discipline: pidStartTime from /proc/<pid>/stat
is compared before every kill, and a mismatch logs PID reuse detected — refusing to kill and does nothing.

Verification

bun run ts:build clean; bun run lint clean (1 pre-existing warning in
plugins/uploads/preprocessors/markitdown-preprocessor.ts, untouched here); full suite
1884 pass / 106 skip / 0 fail.

Six new integration tests in services.integration.test.ts, following the existing
"kills orphaned process group from previous server instance" model with real detached
processes. Each was run against unmodified origin/main sources with the new tests in
place — 4 of the 6 fail there and pass with this change:

test	on origin/main
starting over an entry that still owns a process reclaims the old one	❌ old process still alive
stopping a failed service settles it instead of refusing	❌ stop() returns Err
reclaims a failed entry a previous server instance left owning a process	❌ orphan survives the boot
reclaims an orphan that session_restarted had already marked stopped	❌ orphan survives the boot
refuses to reclaim a failed entry whose pid the kernel has recycled	✅ (guard already correct — locks in the counter-example)
a startup timeout reclaims the process group it spawned	✅ (see note)

Honest note on the last row: the brief described the startup timeout as abandoning
its process outright. It does not on origin/main — the SIGKILL has been there since
the initial commit, and a probe confirmed the process dies. That test therefore locks in
existing behaviour (now routed through the guarded reap) rather than proving a fix. The
leak reaches production through the other paths above, above all the pid erasure in
(3). If the deployed agent bundle predates that kill, this change covers both.

Not fixed here: ServiceExecutor.shutdown() still skips failed entries. After
this change they are reaped at the moment they fail, so a survivor there would have to
be one the PID-reuse guard refused — i.e. not ours. Left alone rather than adding a
defensive sweep.

The mirror-image blind spot in the worker-side reaper (it stops only ready services)
is being fixed separately; stop() accepting a failed service is what makes that fix
able to do anything.

🤖 Generated with Claude Code