docker-bash: no teardown() — in-flight execs are orphaned when the owning matbot exits

Observed on v0.4.17 (67cb3df), @matatbread/matbot-tool-docker-bash.

What happens

Each bash call runs its script as a long-lived process inside the shared container, in its own session and process group:

plugins/docker-bash/src/index.ts:410

args.push(cfg.name, 'setsid', '-w', 'bash', '-c', 'echo $$ > "$MATBOT_PIDFILE"; exec bash -c "$MATBOT_SCRIPT"');

The host locates that group through a pidfile and kills it by negative pid — the script, and everything it spawned:

plugins/docker-bash/src/index.ts:93

async function killGroup(containerName: string, hostPidfile: string): Promise<void> {
  let pid: string;
  try {
    pid = (await readFile(hostPidfile, 'utf8')).trim();
  } catch {
    return; // not written yet, or already cleaned up — nothing to kill
  }

and then, once the pid is validated, at :101:

  await dockerExec(['exec', containerName, 'bash', '-c', `kill -KILL -${pid}`]).catch(() => {});

killGroup has exactly one caller: the timeout/abort path (:421). The plugin declares no teardown() — its spec carries apiVersion and setup() only — so when the process exits, nothing signals the execs it started.

The local client spawn('docker', args, ...) (:417) dies with matbot, but it is not the parent of the in-container script in any signal sense; the docker CLI is the only host-side child this plugin creates. The script, and anything it spawned, carries on.

That is by design, not oversight:

plugins/docker-bash/src/index.ts:496

// We deliberately do NOT reconcile a stale container at boot. A sleeping container from a
// previous run is reused (matched by name) so the restarted matbot just continues with it.

Observed

On the reporting host: a leaked yes ran roughly 4h38m and held about two cores. Reaping the container also found 13 zombies, the oldest about six days old.

Those are two different faults, and only one of them is closed. The zombies were dead processes never reaped — fixed by --init in 67cb3df. The yes was a live orphan: no reaper helps with that, because nothing points at it any more.

Honest limit on the evidence: the mechanism above is verified in the code, but the provenance of that particular process — which instance started it, and whether that instance died gracefully — is not established. It is consistent with the mechanism, not proof of it.

Why it matters

• Leaked work is invisible. Once the owning matbot is gone the pidfile is only a file, nothing holds it, and the container is reused by name, so the next instance adopts a container that already holds a stranger's process.
• An orphan need not be idle. The observed one saturated two cores, and the container has no CPU ceiling unless one was configured.
• --init does not cover it. tini reaps zombies; it does not kill a live orphan.

Suggested fix

Implement the hook that already exists. MatbotPluginSpec declares teardown?(): Promise<void> (plugin-api/src/plugin.ts:528), which teardownPlugins() runs in reverse-registration order on the way to process exit, and which unloadPlugin() runs on hot-unload — both bounded, both logging rather than throwing.

docker-bash already knows the groups it started, so an in-memory set of exec ids is enough (the pidfile is a filesystem handle; for a graceful teardown the process is still alive to remember):

async teardown(): Promise<void> {
  for (const execId of activeExecs) {
    await killGroup(cfg.name, hostPidfileFor(execId));
  }
}

That covers every graceful path — server-mode SIGTERM/SIGINT, and the single-turn and REPL finally blocks in apps/cli/src/index.ts (:1020, :1130, :1153) — plus hot-unload, which matters because plugins are reloaded repeatedly during development.

A stronger option — stdin as a lease

teardown() only reaches the graceful paths: every one of its call sites is a finally, so SIGKILL, a crash or an OOM-kill runs nothing and the exec is still orphaned.

The exec's stdin carries the liveness signal that stdout cannot. Verified against a running container: an in-container reader with stdin held open got EOF as soon as its host-side docker CLI client was SIGKILLed. The client's death closes the connection, and EOF propagates into the container.

Because that fires on any client death — graceful or not, and including the host's own SIGKILL — it is strictly stronger than a teardown. It is also about three lines: keep the wrapper alive, run the script as a child, read stdin, and on EOF kill the process group — reusing killGroup.

Why stdout cannot do this

To the writing process, "consumed" and "discarded" are the same event: write() returns successfully either way. A writer can only observe the opposite condition, a closed reader, as EPIPE/SIGPIPE — which is already the default disposition, so there is no option to enable. No set -o, shopt or docker flag reaches it.

Measured, not assumed: a process flooding stdout kept running in state R for six seconds after its client was killed, counter still climbing. That state is the proof — if nothing were draining it, 64 KiB of pipe would have filled and parked it in S on a blocking write. It is being read and discarded. Adding -t does not change it (no hangup on connection loss), and docker exec has no "die with the client" flag.

The constraint

The exec's stdin must belong to the lease, so a script cannot also use it. Today docker-bash passes -i and never feeds the script through stdin — it arrives by -e MATBOT_SCRIPT — so the channel is free, but this turns an unused detail into a real constraint.

So: for anything that does not need stdin, this is the cheaper and more complete fix. Whether docker-bash's own use can tolerate the constraint is an open question, and the comment on this issue is the place to settle it.

The technique is not docker-specific: the plain bash plugin (#47, #48) never touches
its child's stdin either. So the same lease is available to anything that does not need
stdin — which today is both plugins.

One thing to check if this is adopted: on timeout/abort the harness kills the client, so the lease would fire on that path too — which may let it subsume the existing timeout/abort kill rather than sit beside it. That interacts with the pidfile race the current code already guards (:416), so it wants care rather than a delete.

What teardown() alone does not cover

SIGKILL, a crash, an OOM-kill: no finally runs, so in-flight execs are still orphaned. Covering that means knowing whose work it is — per-exec ownership metadata plus a liveness check, so that a later instance can tell an orphan from a live sibling's exec. Options worth designing before building:

• owner tagging plus a liveness probe, reaped by a later instance;
• an in-container supervisor that tracks exec sessions and reaps when the owning connection goes away;
• a container per instance, so lifecycle ownership is trivial.

A startup sweep that kills every session other than the container init's is not a safe shortcut: the container is shared by a main server, background sub-agents and ad-hoc CLI runs, so a starting instance cannot tell an orphan from another instance's in-flight work.

Land teardown() first — small, and no new API. The ungraceful case is a separate design question, and it wants evidence before it wants machinery.

Related

• #47 and #48 — the same class of fault in the plain bash plugin. #47's suggested fix cites docker-bash's group-kill as the correct pattern. The pattern is right; the difficulty is that here it can be reached only from the timeout/abort path.

Start in plugins/docker-bash/src/index.ts, tracing active exec creation, the existing killGroup timeout path, and the plugin lifecycle in plugin-api/src/plugin.ts. Check the teardownPlugins() and unloadPlugin() entry points, plus the CLI finally blocks in apps/cli/src/index.ts. Done means graceful shutdown and hot-unload terminate every exec group started by the plugin; the ungraceful lease and ownership designs remain separate.