fix(stack): release log followers on exit (CLI-2521)

[7ttp]

TL;DR

stops the stack runtime from signalling a reused pid when a container log stream dies long before its service stops

what might hurt?

the stack runtime spawned docker logs --follow into the service launch scope
when that follower exited non zero while the container kept running, its release waited for service stop and then sent kill(-pid, SIGTERM) to a pid that an unrelated process could own by then.

sorted by:

giving the follower its own scope that closes as soon as it exits, so the release lands within milliseconds of the exit.

a follower still running at service stop is killed exactly as before, and the serve tests now also pin the per attempt log scoping from #6594

ref:

• related: functions serve: SIGTERM to a stale process kills unrelated processes via kill(-pid) on reused child PIDs #6858
• missed in: feat(stack): replace the experimental runtime and integrate CLI commands #6684

[github-actions]

🤖 AI Review

The new log-follower scope has a cleanup gap when spawning fails, and the new integration test does not verify that output remains readable after the scope closes. The claimed retry race conflates this container runtime with a separate, already scoped CLI log-attach path. I could not verify the stale-PID claim without the spawner finalizer implementation.

Findings

Severity	Location	Category	Sources	Claim
🟡 MINOR	packages/stack/src/runtime/Container.integration.test.ts:308	test-coverage	claude	The new exited-follower test does not verify that output written before exit remains available to consumers after the follower scope closes.
🟡 MINOR	packages/stack/src/runtime/Container.ts:534	resource-management	codex	A failed or interrupted follower spawn leaves its forked scope open until the owner scope closes.
⚪ NIT	packages/stack/src/runtime/Container.ts:545	correctness	claude	Closing the follower scope after exit may still signal a reused PID if the spawner finalizer signals unconditionally.

Refuted findings (kept for transparency, not posted as review comments)

• packages/stack/src/runtime/Container.ts:545 (concurrency): A retry can spawn another log follower before this follower's scope has been released.
  Refuted: The cited retry uses a separate CLI path: serve.ts:1328-1375 scopes each attach and drains its output and exit code before returning; serve.ts:1402 starts the next attempt only after that return. It does not retry the follower created by Container.ts.

Stats

Claude findings: 2 · Codex findings: 2 · Confirmed: 2 · Refuted: 1 · Uncertain: 1

---

Models: claude-opus-5-5 + gpt-6-sol · Trigger: auto · Workflow run

This review runs once per PR. A maintainer can request another with a /ai-review comment.

[Coly010]

Nice fix, thank you for this one. Giving the follower its own scope and closing it off the back of exitCode is a tidy way to do it, and it leaves the still-running case (follower killed at service stop) exactly as it was.

I checked a couple of things locally:

1. Reverted only Container.ts to the base and the new "exits while its container keeps running" test fails on the release timeout, then goes green again with the fix, so the regression test is genuinely pinning the bug
2. Swept the other long-lived spawns in packages/stack to see if anything else has the same shape. Native services and startup commands both get their scopes closed promptly on exit (stopNow in Service.ts), so as far as I can tell the log follower was the one place a scope could outlive its child for hours

I'm approving so you're unblocked, with one non-blocking ask:

The remaining kill(-pid) still comes from the spawner's release in @effect/platform-node-shared. When the child has already exited non-zero it calls killProcessGroup again, even though its own exit listener already did the group cleanup at exit time. This PR shrinks that window to a few ms, which I think is fine in practice, but it means safety still depends on every call site scoping tightly, and the first expected behaviour on #6858 ("a release for a child that has already exited must not send any signal") isn't covered yet.

Since we already carry a pnpm patch for that package, it might be worth either extending it to skip the group kill in the release when exited is true (and raising it upstream with Effect), or splitting that into a follow-up so CLI-2521 doesn't get closed by this merge with that part still open. Happy with either, and if you think the patch is overkill that's totally fine too

The replies on the two AI review threads look right to me fwiw.

Otherwise, looks good to me!

[7ttp]

if you think the patch is overkill that's totally fine too

the leftover few ms window is the pgid reuse limitation effect already documents upstream

• (fix(platform-node): clean process groups after successful leader exit Effect-TS/effect#8245)
  so i think patching it here might be a bit overkill for now..

edit: merging this as is tho am noting this off on the linear thread, if it ever bites us again I'll pick it up