Vote for your favorite SkillMD. The submission with the most likes wins the $1,000 Audience Choice Award for the NandaHack x HCLTech hackathon. Voting is open through September 25.Vote now →
mergedhumanPR #275Other

fix(cli): stop Track processes when the CLI receives SIGTERM

Follow-up to #264–#272, from the same correctness audit of main at df0b5f1.

Author

JamesCarnley avatar

@JamesCarnley

github profile →
Status
Merged
Merged on
Sep 14
Branch
fix/cli-sigterm-cleanup

Description

The pitch.

Follow-up to #264–#272, from the same correctness audit of `main` at `df0b5f1`.
It merges cleanly with the other open PRs.

### Problem

Stopping `nandatown run`, `test-agent` or `campaign` with SIGTERM (for example,
a CI job timeout) leaves the coordinator and participants running
indefinitely.

The coordinator runs in its own session, and only `run_town`'s `finally` block
stops it. Python's default SIGTERM handling exits without running `finally`
blocks. SIGINT did run the cleanup, but a signal that arrived in the few
milliseconds between starting a process and registering it for cleanup could
still orphan that process.

### Change

- **SIGTERM cleanup.** For the three commands that call `run_town`, SIGTERM now
  takes the same cleanup path as SIGINT. Once cleanup has run, the CLI restores
  SIGTERM's default handling and re-raises the signal, so the parent still sees
  a process killed by signal 15, exactly as before.
  - A second SIGTERM cannot interrupt the cleanup.
  - The previous handler is restored when the command returns.
  - No handler is installed on non-POSIX platforms, off the main thread, or when
    SIGTERM is already ignored or handled.
- **No startup gaps.** The admin client is built before any process starts.
  Each Town process is started and recorded for cleanup inside a short guard
  that holds SIGINT and SIGTERM until the process is registered, then re-raises
  them. Ignored signals stay ignored. While the previous handlers are put
  back, both signals are briefly blocked in the calling thread, so a signal
  arriving mid-restore still reaches its restored handler exactly once.

### Compatibility

- Exit status for SIGTERM is unchanged: death by signal 15.
- SIGINT behaves as before, except that it can no longer orphan a process
  during startup.
- The server commands (`coordinator`, `a2a serve`, `ui --web`) behave as before.
- One known gap remains: the terminal UI runs `run_town` in worker threads, so
  SIGTERM there still skips cleanup.

### Verification

- **Real-process tests.** New `tests/test_cli_signals.py` starts the real
  `test-agent --wait`, records every process the CLI started, sends SIGTERM or
  SIGINT, and asserts they are all gone and the exit status is `-signum`. It
  cleans up leftovers even when an assertion fails.
  - On `main`, the SIGTERM case orphans the coordinator and the stock buyer. It
    passes with this change.
  - It passed 20 of 20 loop runs.
- **Timing harness.** Signals were injected at startup, just after each start
  call returns, during the health wait, while waiting for an external agent,
  with a `--cmd` seller and its child, during campaign trials, as a second
  SIGTERM during cleanup, and at finish. Each case now leaves 0 processes and
  exits `-15` (or `-2` for SIGINT). Before this change, several left the
  coordinator or seller running.
- Full suite: 825 passed, twice.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Try it

Open PR on GitHubView diff

Checkout locally

git fetch origin pull/275/head:pr-275
git checkout pr-275