council: closing the terminal no longer leaves five agents running

[sanlee-ys]

On macOS and Linux, a council room that ended on a signal took none of its
seats with it. Every vendor process kept running — holding a session, spending
quota — with no room attached and nothing on screen to say so. Five agents, on a
five-seat room.

Windows never had this: a seat there lives in a Job Object created with
JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE, so when telltale's process dies the handle
closes and Windows reaps the tree. A unix process group is not a lifetime. It is
a name for a set of processes, and it dies when something signals it and at no
other moment. runner/proc_unix.go claimed "the same guarantee the Windows job
object gives"; it gives half of it. This PR corrects that comment and makes the
missing half happen.

Windows behaviour is unchanged. watchExitSignals is a documented no-op
there, and no Windows code path is touched.

Measurement

Everything below was run on the Mac (Intel x86_64, macOS 26.5.2) on 2026-08-17,
against charm.land/bubbletea/v2 v2.0.8.

The probe

A throwaway Go program, deleted before this branch was committed: a Bubble Tea
v2.0.8 model plus one sleep 600 child started with
SysProcAttr{Setpgid: true} — the shape runner/proc_unix.go gives every seat.
It never spawned a vendor CLI. Its teardown was reachable only from a key
handler, which is how the shipped room is written. Each run signalled the
program, waited two seconds, and asked whether the child's pid was still alive.

$ sh measure.sh INT baseline
=== signal=INT mode=baseline probe=67122 child=67124
probe:  exited
child:  ORPHANED (still running)
--- probe log
probe pid=67122 child pid=67124 mode=baseline
RUN start
UPDATE saw tea.ColorProfileMsg
UPDATE saw tea.EnvMsg
UPDATE saw tea.WindowSizeMsg
RUN returned err=program was killed: program was interrupted
EXIT without touching the child

$ sh measure.sh TERM baseline
=== signal=TERM mode=baseline probe=67269 child=67271
probe:  exited
child:  ORPHANED (still running)
--- probe log
probe pid=67269 child pid=67271 mode=baseline
RUN start
UPDATE saw tea.EnvMsg
UPDATE saw tea.WindowSizeMsg
UPDATE saw tea.ColorProfileMsg
RUN returned err=<nil>
EXIT without touching the child

$ sh measure.sh HUP baseline
=== signal=HUP mode=baseline probe=67298 child=67300
measure.sh: line 18: 67298 Hangup: 1               PROBE_LOG=$LOG PROBE_MODE=$MODE $D/probe
probe:  exited
child:  ORPHANED (still running)
--- probe log
probe pid=67298 child pid=67300 mode=baseline
RUN start
UPDATE saw tea.WindowSizeMsg
UPDATE saw tea.EnvMsg
UPDATE saw tea.ColorProfileMsg

What Bubble Tea already covered

signal	what Bubble Tea did to the program	did the model's Update run?	the child
SIGINT	p.Run() returned program was killed: program was interrupted	no	orphaned
SIGTERM	p.Run() returned nil	no	orphaned
SIGHUP	nothing — the default disposition killed the process outright	no	orphaned
SIGKILL	nothing, and nothing can	no	orphaned

So Bubble Tea does end the program on SIGINT and SIGTERM, and ending the
program is the whole of what it does. Its handler answers above the model's
head: tea.go's event loop returns on QuitMsg and InterruptMsg before it
calls model.Update, so the model is never handed the message. Council's
teardown is reachable only from the q and ctrl+c key handlers, so it ran on
none of the four. Nothing was covered. The UPDATE saw … lines in the probe
log are the three startup messages and nothing else — no QuitMsg, no
InterruptMsg, on any run.

The SIGKILL row is recorded, not addressed. It is uncatchable, so kill -9 on
a room still orphans every seat, and no line in this PR claims otherwise.

With the handler installed

$ sh measure.sh INT handler
=== signal=INT mode=handler probe=67391 child=67393
probe:  exited
child:  reaped
--- probe log
HANDLER got interrupt
RUN returned err=program was killed: program was interrupted
TEARDOWN killing child group 67393

$ sh measure.sh TERM handler
=== signal=TERM mode=handler probe=67361 child=67363
probe:  exited
child:  reaped
--- probe log
HANDLER got terminated
RUN returned err=<nil>
TEARDOWN killing child group 67363

$ sh measure.sh HUP handler
=== signal=HUP mode=handler probe=67379 child=67381
probe:  exited
child:  reaped
--- probe log
HANDLER got hangup
TEARDOWN killing child group 67381
RUN returned err=program was killed

$ sh measure.sh KILL handler
=== signal=KILL mode=handler probe=67411 child=67413
measure.sh: line 18: 67411 Killed: 9  PROBE_LOG=$LOG PROBE_MODE=$MODE $D/probe
probe:  exited
child:  ORPHANED (still running)

Why the two arms are not the same act

The watcher runs teardown on all three catchable signals, and calls p.Kill()
on SIGHUP only.

• SIGINT / SIGTERM — Bubble Tea's own handler already brings the program
  down, measured above, so a second shutdown initiator here would buy nothing.
  It would also run inside a window the source shows is delicate: at that moment
  Bubble Tea's signal goroutine is blocked on an unbuffered
  p.msgs <- QuitMsg{} (tea.go makes msgs with no capacity) while
  p.shutdown waits for that same goroutine to return.
• SIGHUP — Bubble Tea never registers it, so nothing else will end the
  program, and installing this handler has just displaced the default
  disposition that would have. Kill is safe here for the same reason it is
  unnecessary above — no Bubble Tea goroutine is mid-send on a signal it did not
  subscribe to — and it restores the terminal, which the default disposition
  would not have.

Honesty note on that first bullet. A probe variant that did call Kill on
SIGTERM was run 20 times and never hung:

$ sh hazard.sh 20
runs=20 hung=0

So the claim is only that the Kill is unnecessary there. A deadlock was
not measured; it is a source reading, and the doc comment says so in those
words.

Idempotency, and the test that actually witnesses it

teardown is a one-shot now (sync.Mutex + a flag), because a signal can land
on a room the user is already quitting and the loop deletes from the map it
walks.

The first version of the concurrency test passed with the guard deliberately
removed — the repo's own recorded failure mode, a test that checks the flag
instead of the effect. It was rewritten: each fake seat's Kill holds for 20 ms,
which widens the window past anything a scheduler can close. With the guard
removed, the same test now fails hard:

$ go test ./internal/council -run 'TestATeardownRacing' -count=1
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0xb6c2e7f]

goroutine 20 [running]:
github.com/sanlee-ys/telltale/internal/council.(*Model).teardown(0x4e6cd97a008)
	.../internal/council/dispatch.go:2025 +0x1ff

That line is m.turn.cancel(), on a turn another goroutine had already nil'd. The
counts catch the softer half of the same overlap: sixteen callers each walk a map
none of them has drained yet, so every seat is killed sixteen times.

The sequential test is labelled in its own doc comment as a regression pin
rather than a witness: sequentially, teardown was already once-only by accident
of draining m.procs as it walks it, and the guard must not break that.

Tests

No test in this PR spawns a vendor. Every process is a countedKill fake, so
countSpawns has nothing to guard.

• TestTeardownActsOnceHoweverOftenItIsCalled — sequential pin.
• TestATeardownRacingAnotherNeitherPanicsNorDoubleKills — 16 concurrent
  teardowns, one kill per seat, one turn cancel.
• TestASignalKillsTheSeatsBeforeTheRoomGoesOut — table over SIGINT, SIGTERM
  and SIGHUP: the seats die on all three, and the program is killed on SIGHUP
  alone. The watcher is driven against a fakeProgram rather than a real
  tea.Program, so no terminal or event loop is involved.
• TestTheWatcherStopsWhenTheRoomEndsAnyOtherWay — the returned stop really
  deregisters.

Gates

gate	result
go vet ./...	PASS (clean)
GOOS=windows go vet ./...	PASS (clean) — the Windows arm compiles
go test ./internal/council -timeout 20m	PASS, ok … 44.054s
go test -race ./...	PASS, all 30 packages, exit 0 — internal/council 58.210s, internal/council/runner 16.892s

The race run is not one of this lane's declared gates, but CI's ubuntu-latest
job runs exactly it, and this PR adds both a goroutine and a mutex. The new
signal tests were also run under -race -count=3 on their own: clean.

What is NOT in this PR

STATE.md, Windows console-close handling (CTRL_CLOSE_EVENT is a different
mechanism from a POSIX signal, and the job object already covers the seats
through it), any change to what teardown does to each seat kind, and any claim
about SIGKILL.

🤖 Generated with Claude Code