diff --git a/docs/development/testing.md b/docs/development/testing.md index fc2ff8d..f42feb7 100644 --- a/docs/development/testing.md +++ b/docs/development/testing.md @@ -4,7 +4,7 @@ The full variable list for each target is in [Environment variables](../referenc ## Unit Tests -The launcher, the tongs layer, and the container-side translators are covered by stdlib `unittest` tests in `tests/test_*.py`. A test module is named for the source module it covers — `tests/test_tongs_.py` for `swarmforge/tongs/.py`, `tests/test_anvil_.py` for `swarmforge/anvil/.py` — so the file that covers a change is the one named after it. Two modules have no namesake file because they have nothing to assert on their own: `swarmforge/anvil/readiness.py` is exercised through `run_with_tongs`, and `swarmforge/anvil/errors.py` holds one exception class. Fixtures that more than one test module needs live in `tests/tongs_fixtures.py`, `tests/anvil_fixtures.py` and `tests/harness_fixtures.py`, which the discovery glob skips. +The launcher, the tongs layer, and the container-side translators are covered by stdlib `unittest` tests in `tests/test_*.py`. A test module is named for the source module it covers — `tests/test_tongs_.py` for `swarmforge/tongs/.py`, `tests/test_anvil_.py` for `swarmforge/anvil/.py` — so the file that covers a change is the one named after it. Two modules have no namesake file because they have nothing to assert on their own: `swarmforge/anvil/readiness.py` is exercised through `run_with_tongs`, and `swarmforge/anvil/errors.py` holds the launcher's shared exception classes. Fixtures that more than one test module needs live in `tests/tongs_fixtures.py`, `tests/anvil_fixtures.py` and `tests/harness_fixtures.py`, which the discovery glob skips. Four files assert on the shape of the repo rather than on any one module. `tests/test_image_layout.py` holds the Dockerfile and the entrypoint to the same import root, `tests/test_package_layering.py` keeps the package's imports acyclic and keeps loading a module from a file path out of everything but the `bin/` shims, `tests/test_harness_conformance.py` holds every registered harness to the contract described under [Harness lifecycle](architecture.md), and `tests/test_docs_links.py` keeps every relative link in this tree and in the root `README.md` resolving, and every page reachable from [the index](../README.md). All four fail the way a build should — before anything reaches a container. diff --git a/docs/tongs/README.md b/docs/tongs/README.md index 990ec9c..efaef4c 100644 --- a/docs/tongs/README.md +++ b/docs/tongs/README.md @@ -13,7 +13,7 @@ The host-side launcher (`swarmforge/anvil/`, run through `bin/run-anvil`) discov 1. (Only if the tong needs secrets) configure the secret-provider table (see [Secret providers](secrets.md)). 2. Drop a tong definition into a layer directory, e.g. `~/.swarmforge/tongs/.yaml` (personal) or `/.swarmforge/tongs/.yaml` (project). 3. Run the anvil as usual (`oc`, or `make run_claude PROJECT_DIR=$(pwd)`). A **workspace**-sourced tong prints a privilege summary and asks for approval on first run (see [First-run approval](approval.md)). The launcher resolves secrets (which may prompt your provider CLI to unlock), starts the tong, waits for readiness, injects reachability into the anvil, then runs the anvil in the foreground. -4. On exit (including Ctrl-C), the per-session network and its `session` tongs are torn down. +4. On exit — Ctrl-C, a closed terminal or tmux pane (SIGHUP), or a plain `kill` (SIGTERM) — the `session` tongs and the per-session network are removed. If the anvil command has no `--network`, its container can outlive a killed docker client and hold the network, which then stays behind. Only `kill -9`, `Ctrl-\` (SIGQUIT), the OOM killer, a launcher crash, or a host reboot skips teardown. ## Where definitions live diff --git a/swarmforge/anvil/__init__.py b/swarmforge/anvil/__init__.py index e58df81..f2ee447 100644 --- a/swarmforge/anvil/__init__.py +++ b/swarmforge/anvil/__init__.py @@ -23,7 +23,7 @@ docker the one seam every docker invocation goes through readiness waiting for a started tong to report ready orchestrate starting the tongs, running the anvil, tearing down after it - errors the one failure raised from more than one of the above + errors the failures and interruptions raised from more than one of the above Every public name is re-exported below, so a caller imports `swarmforge.anvil` and never has to know which module a function sits in; a module's own private @@ -41,11 +41,12 @@ `session` tong is per-session: when any exists the launcher creates a per-session network, starts the `session` tongs on it under their canonical aliases, connects each network-facing `shared` tong to it, and joins the anvil to it (plus the base -`NETWORK=` network). On exit -- including SIGINT -- the `session` tongs and the -per-session network are torn down (and the connected `shared` tongs disconnected) -while the long-lived `shared` tongs keep running. A `port` tong's reachability is -injected into the anvil as environment; an `mcp` tong's as generated MCP config -(see "MCP config"); a `none` tong has no anvil-facing surface. +`NETWORK=` network). On exit -- including SIGINT, SIGHUP, and SIGTERM -- the +`session` tongs and the per-session network are torn down (and the connected +`shared` tongs disconnected) while the long-lived `shared` tongs keep running. A +`port` tong's reachability is injected into the anvil as environment; an `mcp` +tong's as generated MCP config (see "MCP config"); a `none` tong has no +anvil-facing surface. A tong's secret references are resolved on the host (see "Secret delivery") and handed to the tong as environment, so the launcher starts `shared` and `session` @@ -130,7 +131,7 @@ parse_args, ) from .docker import DockerCLI, DockerError -from .errors import OrchestrationError +from .errors import OrchestrationError, TerminationSignal from .orchestrate import ( MCP_CONFIG_CONTAINER_PATH, MCP_FILE_ENV, @@ -162,6 +163,7 @@ "DockerCLI", "DockerError", "OrchestrationError", + "TerminationSignal", "MCP_CONFIG_CONTAINER_PATH", "MCP_FILE_ENV", "ensure_mcp_harness_supported", diff --git a/swarmforge/anvil/cli.py b/swarmforge/anvil/cli.py index 3ec1147..640af6f 100644 --- a/swarmforge/anvil/cli.py +++ b/swarmforge/anvil/cli.py @@ -6,16 +6,21 @@ then either execs the anvil argv verbatim (no tongs discovered) or hands the launch to the orchestrator. Every failure the launcher reports is mapped to a process exit code here. + +While the orchestrator runs, SIGHUP and SIGTERM unwind it through its teardown +the way Ctrl-C does. """ import collections +import contextlib import os +import signal from swarmforge import tongs from .approval import ApprovalDenied, gate_workspace_tongs from .docker import DockerCLI, DockerError -from .errors import OrchestrationError +from .errors import OrchestrationError, TerminationSignal from .orchestrate import ( ensure_mcp_harness_supported, exec_anvil, @@ -157,6 +162,73 @@ def discover_tongs(layer_dirs): return tongs.merge_tongs(tongs.discover(layer_dirs)) +_TERMINATION_SIGNALS = (signal.SIGHUP, signal.SIGTERM) + + +class _TerminationTrap: + """SIGHUP/SIGTERM handler that raises `TerminationSignal`, or only records it. + + Only the first signal is kept in `received`; later ones are dropped. It + raises unless `deferred` is entering or leaving, when it is only recorded + for `main` to turn into the exit status. SIGINT is left alone. + """ + + def __init__(self): + self.received = None + self._deferring = False + + def handle(self, signum, frame): + if self.received is not None: + return + self.received = signum + if not self._deferring: + raise TerminationSignal(signum) + + @contextlib.contextmanager + def deferred(self): + """Teardown guard: ignore SIGHUP/SIGTERM until the block has finished. + + Ignored rather than blocked, because teardown's `docker` children + inherit it: a Go program clears an inherited signal mask but keeps an + inherited SIGHUP ignore, so a later hangup cannot cut a `docker rm` + short. Go does not keep SIGTERM ignored. A signal ignored here is lost. + """ + self._deferring = True + trapped = [s for s in _TERMINATION_SIGNALS if signal.getsignal(s) == self.handle] + try: + for signum in trapped: + signal.signal(signum, signal.SIG_IGN) + yield + finally: + for signum in trapped: + signal.signal(signum, self.handle) + self._deferring = False + + +@contextlib.contextmanager +def _termination_signals_raise(): + """Install a `_TerminationTrap` on SIGHUP/SIGTERM for the duration; yields it. + + Python's default action for both skips every `finally`, where the + orchestrator tears down. A signal ignored on entry (`nohup`) stays ignored, + and the previous handlers are put back on the way out. + """ + trap = _TerminationTrap() + previous = {} + try: + for signum in _TERMINATION_SIGNALS: + handler = signal.getsignal(signum) + if handler is signal.SIG_IGN: + continue + previous[signum] = handler + signal.signal(signum, trap.handle) + yield trap + finally: + for signum, handler in previous.items(): + # None means a handler installed outside Python; the default is the best match. + signal.signal(signum, signal.SIG_DFL if handler is None else handler) + + def main(argv): try: opts, anvil_cmd = parse_args(argv) @@ -221,13 +293,25 @@ def main(argv): tongs.warn(str(exc)) return 1 + # Outermost, so a signal landing in an except clause is caught too. try: - return run_with_tongs( - merged, anvil_cmd, opts, docker=DockerCLI(), providers=providers - ) - except (OrchestrationError, DockerError, SecretResolutionError) as exc: - tongs.warn(str(exc)) - return 1 - except KeyboardInterrupt: - # 128 + SIGINT; the shared tongs stay running by design. - return 130 + with _termination_signals_raise() as trap: + try: + rc = run_with_tongs( + merged, anvil_cmd, opts, docker=DockerCLI(), providers=providers, + teardown_guard=trap.deferred, + ) + except (OrchestrationError, DockerError, SecretResolutionError) as exc: + # Nothing written after a hangup: the terminal may be gone. + if trap.received is not None: + return 128 + trap.received + tongs.warn(str(exc)) + return 1 + except KeyboardInterrupt: + # 128 + SIGINT, even if a HUP/TERM followed. + return 130 + if trap.received is not None: + return 128 + trap.received + return rc + except TerminationSignal as exc: + return 128 + exc.signum diff --git a/swarmforge/anvil/docker.py b/swarmforge/anvil/docker.py index 0e955e0..df91266 100644 --- a/swarmforge/anvil/docker.py +++ b/swarmforge/anvil/docker.py @@ -11,6 +11,8 @@ from swarmforge import tongs +from .errors import TerminationSignal + class DockerError(Exception): """A docker command the launch depends on failed; the launch must stop.""" @@ -22,6 +24,9 @@ class DockerError(Exception): ) _INSPECT_HEALTH_FORMAT = "{{if .State.Health}}{{.State.Health.Status}}{{end}}" +# Teardown waits on this; under a TTY the forwarded signal just ends the client. +_FOREGROUND_STOP_GRACE_S = 3 + class DockerCLI: def __init__(self, run=None): @@ -209,22 +214,34 @@ def run_foreground_multi(self, argv, extra_networks, container): ) def _wait_foreground(self, argv): - """Run a foreground command, reaping it on Ctrl-C before re-raising. + """Run a foreground command, reaping it on any interrupt before re-raising. - Popen + wait (rather than exec) so the launcher regains control after the - process exits. On Ctrl-C the SIGINT reaches both this process and the child - through the controlling terminal's process group; the child handles it and - exits, we reap it, and the KeyboardInterrupt propagates to the caller. + A SIGHUP/SIGTERM may have reached this process alone, so it is forwarded + to the child, which gets `_FOREGROUND_STOP_GRACE_S` to exit before it is + killed. The child is always reaped before the exception propagates, so + teardown never races a live client. """ try: proc = subprocess.Popen(argv) except OSError as exc: raise DockerError("cannot run anvil %r: %s" % (argv[:2], exc)) try: - return proc.wait() - except KeyboardInterrupt: - proc.wait() + try: + return proc.wait() + except KeyboardInterrupt: + proc.wait() + raise + except TerminationSignal as exc: + proc.send_signal(exc.signum) + try: + proc.wait(timeout=_FOREGROUND_STOP_GRACE_S) + except subprocess.TimeoutExpired: + pass raise + finally: + if proc.poll() is None: + proc.kill() + proc.wait() def _decode(output): diff --git a/swarmforge/anvil/errors.py b/swarmforge/anvil/errors.py index 4dad3fe..136fcb8 100644 --- a/swarmforge/anvil/errors.py +++ b/swarmforge/anvil/errors.py @@ -1,11 +1,23 @@ -"""The one launch failure raised from more than one launcher module. +"""The launch failures and interruptions more than one launcher module needs. Every other error the launcher raises is defined by the module that raises it. -This one has two sources -- the secret channel, when a tong never takes delivery -of its secrets, and the orchestrator, when a tong cannot be started or made -ready -- so it sits on its own and neither has to import the other. +These two cross modules: `OrchestrationError` is raised by both the secret +channel and the orchestrator, and `TerminationSignal` is raised by the entry +point but caught by the docker seam. Keeping them here means none of those +modules imports another. """ class OrchestrationError(Exception): """A tong could not be started/made ready; the launch stops.""" + + +class TerminationSignal(BaseException): + """SIGHUP or SIGTERM `signum` arrived mid-launch; unwinds through teardown. + + A `BaseException`, like `KeyboardInterrupt`, so no `except Exception` swallows it. + """ + + def __init__(self, signum): + super().__init__(signum) + self.signum = signum diff --git a/swarmforge/anvil/orchestrate.py b/swarmforge/anvil/orchestrate.py index 91a486a..1d880ff 100644 --- a/swarmforge/anvil/orchestrate.py +++ b/swarmforge/anvil/orchestrate.py @@ -14,6 +14,7 @@ argv is exec'd verbatim. """ +import contextlib import json import os import shutil @@ -151,11 +152,11 @@ def _start_one_tong(docker, name, defn, *, container, network, alias, Once the argv is assembled, any existing container of the same name is removed so a stale or stopped one is replaced cleanly -- but a definition the argv - builder refuses removes nothing, since it started nothing. If anything fails - after the container starts -- a docker error, a delivery timeout, or a Ctrl-C -- - the container is removed before re-raising, so a half-configured `shared` tong - (stamped with its config-hash label) is not reused on the next session despite - missing its secret. + builder refuses removes nothing here, since it started nothing (teardown still + removes a `session` tong by name). If anything fails after the container + starts -- a docker error, a delivery timeout, or a Ctrl-C -- the container is + removed before re-raising, so a half-configured `shared` tong (stamped with its + config-hash label) is not reused on the next session despite missing its secret. """ plan = tongs.plan_tong_secrets(defn.get("env"), resolver) plain_env = plan["env"] @@ -285,7 +286,7 @@ def _mcp_injection(mcp_config, harness, mcp_dir): def run_with_tongs(merged, anvil_cmd, opts, *, docker, providers=None, - make_channel=SecretChannel, + make_channel=SecretChannel, teardown_guard=contextlib.nullcontext, sleep=time.sleep, monotonic=time.monotonic): """Start the discovered tongs, run the anvil, and tear down session state. @@ -307,9 +308,11 @@ def run_with_tongs(merged, anvil_cmd, opts, *, docker, providers=None, vars and, for `mcp` tongs, the per-harness MCP config -- and the anvil runs in the foreground. - On exit -- including SIGINT -- the `session` tongs and the per-session network - are torn down (and the connected `shared` tongs disconnected) while the - long-lived `shared` tongs are left running. + On exit -- including SIGINT, and SIGHUP/SIGTERM once `cli.main` makes them + raise -- the `session` tongs and the per-session network are torn down (and + the connected `shared` tongs disconnected) while the long-lived `shared` + tongs are left running. Teardown runs inside the caller's `teardown_guard()`, + which keeps signals from cutting it short; the default guards nothing. Returns the anvil's exit code. Raises `OrchestrationError` if a tong never becomes ready (the anvil does not run against a half-up environment) or a @@ -352,10 +355,11 @@ def run_with_tongs(merged, anvil_cmd, opts, *, docker, providers=None, joined_shared_networks = [] anvil_multi = False mcp_dir = None + # Recorded before creation: an interrupt can land mid-create. try: if plan["create"]: - docker.ensure_network(plan["create"]) created_network = plan["create"] + docker.ensure_network(plan["create"]) ready_checks = [] # (name, defn, alias, container, probe_network) for name in sorted(merged): @@ -363,22 +367,22 @@ def run_with_tongs(merged, anvil_cmd, opts, *, docker, providers=None, alias = tongs.canonical_alias(name, defn) if defn.get("lifecycle") == "session": container = tongs.session_container_name(session_id, name) + started_sessions.append(container) _start_one_tong( docker, name, defn, container=container, network=plan["network"], alias=alias, resolver=resolver, workspace=opts.workspace, label_hash=tongs.config_hash(defn), make_channel=make_channel, ) - started_sessions.append(container) probe_network = plan["network"] else: scope = org_token if merged[name]["source"] == tongs.ORG else None container = tongs.shared_container_name(name, scope=scope) if scope: tong_network = tongs.shared_network_name(scope) - docker.ensure_network(tong_network) if tong_network not in joined_shared_networks: joined_shared_networks.append(tong_network) + docker.ensure_network(tong_network) probe_network = tong_network else: tong_network = base_network @@ -397,10 +401,10 @@ def run_with_tongs(merged, anvil_cmd, opts, *, docker, providers=None, # Reached only over its own org network, never the session fabric. continue container = tongs.shared_container_name(name) + connected_shared.append((plan["network"], container)) # A stale endpoint from a hard-killed session would fail the connect. docker.network_disconnect(plan["network"], container) docker.network_connect(plan["network"], container, aliases=aliases) - connected_shared.append((plan["network"], container)) # A scoped shared tong lives only on its org network, so probe where the anvil dials. for name, defn, alias, container, probe_network in ready_checks: @@ -429,21 +433,22 @@ def run_with_tongs(merged, anvil_cmd, opts, *, docker, providers=None, return docker.run_foreground_multi(injected, extra_networks, session_id) return docker.run_foreground(injected) finally: - # Order matters: docker refuses to remove a network while endpoints remain. - for container in started_sessions: - docker.rm_force(container) - # rm_force covers a create path that failed before its own --rm could fire. - if anvil_multi: - docker.rm_force(session_id) - for network, container in connected_shared: - docker.network_disconnect(network, container) - if created_network: - docker.network_rm(created_network) - # Best-effort: docker refuses while the long-lived shared tong is still attached. - for network in joined_shared_networks: - docker.network_rm(network) - if mcp_dir: - shutil.rmtree(mcp_dir, ignore_errors=True) + with teardown_guard(): + # Order matters: docker refuses to remove a network while endpoints remain. + for container in started_sessions: + docker.rm_force(container) + # rm_force covers a create path that failed before its own --rm could fire. + if anvil_multi: + docker.rm_force(session_id) + for network, container in connected_shared: + docker.network_disconnect(network, container) + if created_network: + docker.network_rm(created_network) + # Best-effort: docker refuses while the long-lived shared tong is still attached. + for network in joined_shared_networks: + docker.network_rm(network) + if mcp_dir: + shutil.rmtree(mcp_dir, ignore_errors=True) def exec_anvil(anvil_cmd): diff --git a/tests/test_anvil_cli.py b/tests/test_anvil_cli.py index 51e7a41..c93cd2f 100644 --- a/tests/test_anvil_cli.py +++ b/tests/test_anvil_cli.py @@ -3,6 +3,7 @@ import json import os +import signal import subprocess import sys import tempfile @@ -430,5 +431,263 @@ def test_shared_workspace_mount_refused_without_exec(self): self.assertIn("workspace", completed.stderr) +def _write_shared_tong(tmp): + """A tong layer under `tmp` holding one startable `shared` tong; returns its path.""" + tongs_dir = os.path.join(tmp, "tongs") + os.makedirs(tongs_dir) + with open(os.path.join(tongs_dir, "shipper.yaml"), "w") as handle: + handle.write( + "lifecycle: shared\nimage: x\ninterface:\n kind: none\n" + "readiness:\n mode: none\n" + ) + return tongs_dir + + +_TERMINATION_SIGNALS = (signal.SIGHUP, signal.SIGTERM) + + +def _handlers(): + return {signum: signal.getsignal(signum) for signum in _TERMINATION_SIGNALS} + + +class TerminationSignalTests(unittest.TestCase): + """SIGHUP/SIGTERM unwind an orchestrated run through its teardown.""" + + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + self.addCleanup(self.tmp.cleanup) + self.tongs_dir = _write_shared_tong(self.tmp.name) + # Keeps main from reading the real user layer's provider table. + patcher = mock.patch.dict( + os.environ, {"SWARMFORGE_USER_ASSETS_DIR": self.tmp.name}) + patcher.start() + self.addCleanup(patcher.stop) + + def _main_with_run(self, fake_run): + # Patching the package re-export misses main's binding and hits docker. + with mock.patch.object(launcher.cli, "run_with_tongs", side_effect=fake_run): + return launcher.main(["--repo-tongs", self.tongs_dir, "--", "/no/such/binary-xyz"]) + + @staticmethod + def _deliver(signum): + """Invoke the installed handler as the interpreter would on a signal.""" + signal.getsignal(signum)(signum, None) + + def test_handlers_raise_only_while_orchestrating_then_are_restored(self): + before = _handlers() + seen = {} + + def fake_run(*args, **kwargs): + seen.update(_handlers()) + return 0 + + self.assertEqual(self._main_with_run(fake_run), 0) + for signum in _TERMINATION_SIGNALS: + self.assertIsInstance(seen[signum].__self__, launcher.cli._TerminationTrap) + self.assertEqual(_handlers(), before) + + def test_passthrough_leaves_handlers_alone(self): + before = _handlers() + seen = {} + + def fake_exec(anvil_cmd): + seen.update(_handlers()) + return 0 + + with mock.patch.object(launcher.cli, "exec_anvil", side_effect=fake_exec): + self.assertEqual(launcher.main(["--", "/no/such/binary-xyz"]), 0) + self.assertEqual(seen, before) + + def test_an_inherited_ignore_is_kept(self): + # Under nohup the launcher must stay deaf to the hangup it was told to ignore. + saved = signal.signal(signal.SIGHUP, signal.SIG_IGN) + self.addCleanup(signal.signal, signal.SIGHUP, saved) + seen = {} + + def fake_run(*args, **kwargs): + seen.update(_handlers()) + return 0 + + self.assertEqual(self._main_with_run(fake_run), 0) + self.assertIs(seen[signal.SIGHUP], signal.SIG_IGN) + self.assertIsInstance(seen[signal.SIGTERM].__self__, launcher.cli._TerminationTrap) + self.assertIs(signal.getsignal(signal.SIGHUP), signal.SIG_IGN) + + def test_signal_mid_run_returns_128_plus_signum_and_restores_handlers(self): + previous = mock.Mock() # stands in for an embedding caller's handler + for signum, expected in ((signal.SIGHUP, 129), (signal.SIGTERM, 143)): + with self.subTest(signal=signum.name): + saved = signal.signal(signum, previous) + try: + def fake_run(*args, **kwargs): + self._deliver(signum) + + self.assertEqual(self._main_with_run(fake_run), expected) + self.assertIs(signal.getsignal(signum), previous) + finally: + signal.signal(signum, saved) + + def _teardown_run(self, start, steps): + """A fake run that ends via `start`, then delivers signals mid-teardown.""" + def fake_run(*args, teardown_guard, **kwargs): + try: + return start() + finally: + with teardown_guard(): + steps.append("first") + os.kill(os.getpid(), signal.SIGTERM) + os.kill(os.getpid(), signal.SIGHUP) + steps.append("last") + return fake_run + + def test_repeat_signal_cannot_interrupt_teardown(self): + steps = [] + + def hang_up(): + self._deliver(signal.SIGHUP) + + self.assertEqual(self._main_with_run(self._teardown_run(hang_up, steps)), 129) + self.assertEqual(steps, ["first", "last"]) + + def test_signal_during_teardown_after_a_clean_exit_is_ignored(self): + steps = [] + self.assertEqual(self._main_with_run(self._teardown_run(lambda: 0, steps)), 0) + self.assertEqual(steps, ["first", "last"]) + + def test_signal_during_teardown_after_ctrl_c_keeps_the_ctrl_c_status(self): + steps = [] + + def ctrl_c(): + raise KeyboardInterrupt + + self.assertEqual(self._main_with_run(self._teardown_run(ctrl_c, steps)), 130) + self.assertEqual(steps, ["first", "last"]) + + def test_signal_during_teardown_after_an_error_keeps_the_report(self): + steps = [] + + def fail(): + raise launcher.OrchestrationError("tong 'shipper' did not become ready") + + with mock.patch.object(launcher.cli.tongs, "warn") as warn: + self.assertEqual(self._main_with_run(self._teardown_run(fail, steps)), 1) + warn.assert_called_once_with("tong 'shipper' did not become ready") + self.assertEqual(steps, ["first", "last"]) + + def test_teardown_ignores_the_signals_then_restores_the_trap(self): + during, after = {}, {} + + def fake_run(*args, teardown_guard, **kwargs): + with teardown_guard(): + during.update(_handlers()) + after.update(_handlers()) + return 0 + + self.assertEqual(self._main_with_run(fake_run), 0) + self.assertEqual(during, dict.fromkeys(_TERMINATION_SIGNALS, signal.SIG_IGN)) + for signum in _TERMINATION_SIGNALS: + self.assertIsInstance(after[signum].__self__, launcher.cli._TerminationTrap) + + def test_signal_while_reporting_an_error_still_returns_its_code(self): + def fake_run(*args, **kwargs): + raise launcher.OrchestrationError("tong 'shipper' did not become ready") + + def hangup_on_warn(message): + self._deliver(signal.SIGHUP) + + with mock.patch.object(launcher.cli.tongs, "warn", side_effect=hangup_on_warn): + self.assertEqual(self._main_with_run(fake_run), 129) + + +# Teardown's real child shares the launcher's process group, as `docker rm -f` does. +_TEARDOWN_CHILD = ( + "import sys\n" + "print('tearing down', flush=True)\n" + "sys.stdin.readline()\n" + "open(sys.argv[1], 'w').close()\n" +) + +# Runs the real main with only the orchestrator faked; `mode` says when the signal lands. +_SIGNALLED_LAUNCHER = r""" +import os, signal, subprocess, sys, time +from unittest import mock +repo_root, tongs_dir, marker, mode, teardown_child = sys.argv[1:6] +sys.path.insert(0, repo_root) +from swarmforge.anvil import cli + +def run_with_tongs(*args, teardown_guard, **kwargs): + try: + if mode == "mid-run": + print("running", flush=True) + while True: + time.sleep(0.1) + if mode == "after-ctrl-c": + os.kill(os.getpid(), signal.SIGINT) + while True: + time.sleep(0.1) + return 0 + finally: + with teardown_guard(): + if mode == "mid-run": + os.kill(os.getpid(), signal.SIGHUP) + os.kill(os.getpid(), signal.SIGTERM) + else: + child = subprocess.run([sys.executable, "-c", teardown_child, marker + ".child"]) + assert child.returncode == 0, child.returncode + with open(marker, "w") as handle: + handle.write("torn down") + +with mock.patch.object(cli, "run_with_tongs", run_with_tongs): + sys.exit(cli.main(["--repo-tongs", tongs_dir, "--", "true"])) +""" + + +class RealTerminationSignalTests(unittest.TestCase): + """A real SIGHUP/SIGTERM to the launcher's process group runs the whole teardown. + + The stand-in leads its own session, so signalling its group never reaches this runner. + """ + + def _signal_launcher(self, mode, signum): + """Returns `(exit code, teardown finished, teardown child finished, stderr)`.""" + with tempfile.TemporaryDirectory() as tmp: + tongs_dir = _write_shared_tong(tmp) + marker = os.path.join(tmp, "teardown-ran") + env = dict(os.environ, SWARMFORGE_USER_ASSETS_DIR=tmp) + proc = subprocess.Popen( + [sys.executable, "-c", _SIGNALLED_LAUNCHER, REPO_ROOT, tongs_dir, marker, + mode, _TEARDOWN_CHILD], + stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE, + text=True, env=env, start_new_session=True, + ) + try: + self.assertIn(proc.stdout.readline(), ("running\n", "tearing down\n")) + os.killpg(proc.pid, signum) + # Released only after the signal, which the child must have survived. + _, stderr = proc.communicate("done\n", timeout=30) + finally: + if proc.poll() is None: + proc.kill() + proc.communicate() + return (proc.returncode, os.path.exists(marker), + os.path.exists(marker + ".child"), stderr) + + def test_sighup_mid_run_runs_teardown_and_exits_129(self): + self.assertEqual( + self._signal_launcher("mid-run", signal.SIGHUP), (129, True, False, "")) + + def test_sigterm_mid_run_runs_teardown_and_exits_143(self): + self.assertEqual( + self._signal_launcher("mid-run", signal.SIGTERM), (143, True, False, "")) + + def test_group_sighup_during_teardown_spares_its_child(self): + self.assertEqual( + self._signal_launcher("after-exit", signal.SIGHUP), (0, True, True, "")) + + def test_group_sighup_during_teardown_after_ctrl_c_exits_130(self): + self.assertEqual( + self._signal_launcher("after-ctrl-c", signal.SIGHUP), (130, True, True, "")) + + if __name__ == "__main__": unittest.main(verbosity=2) diff --git a/tests/test_anvil_docker.py b/tests/test_anvil_docker.py index a86db48..ea5d603 100644 --- a/tests/test_anvil_docker.py +++ b/tests/test_anvil_docker.py @@ -2,8 +2,14 @@ """Unit tests for swarmforge.anvil.docker. Run: python3 tests/test_anvil_docker.py""" import os +import queue +import shutil +import signal import subprocess import sys +import tempfile +import threading +import time import unittest from unittest import mock @@ -92,6 +98,34 @@ def test_network_disconnect_and_rm_are_best_effort(self): self.assertIn(["docker", "network", "disconnect", "net", "ctr"], rec.argvs) self.assertIn(["docker", "network", "rm", "net"], rec.argvs) + def test_termination_signal_is_forwarded_to_the_anvil_and_it_is_reaped(self): + cli = launcher.DockerCLI(run=_RecordingRun()) + with mock.patch.object(launcher.docker.subprocess, "Popen") as popen: + proc = popen.return_value + proc.wait.side_effect = [launcher.TerminationSignal(signal.SIGHUP), 0, 0] + proc.poll.return_value = 0 + with self.assertRaises(launcher.TerminationSignal): + cli.run_foreground(["docker", "run", "img"]) + proc.send_signal.assert_called_once_with(signal.SIGHUP) + proc.wait.assert_any_call(timeout=3) + proc.kill.assert_not_called() + + def test_anvil_ignoring_a_forwarded_signal_is_killed_after_the_grace(self): + cli = launcher.DockerCLI(run=_RecordingRun()) + with mock.patch.object(launcher.docker.subprocess, "Popen") as popen: + proc = popen.return_value + proc.wait.side_effect = [ + launcher.TerminationSignal(signal.SIGTERM), + subprocess.TimeoutExpired(["docker"], 3), + -9, + ] + proc.poll.return_value = None + with self.assertRaises(launcher.TerminationSignal): + cli.run_foreground(["docker", "run", "img"]) + proc.send_signal.assert_called_once_with(signal.SIGTERM) + proc.kill.assert_called_once_with() + self.assertEqual(proc.wait.call_count, 3) + def test_run_foreground_multi_creates_connects_then_starts(self): rec = _RecordingRun() cli = launcher.DockerCLI(run=rec) @@ -174,5 +208,143 @@ def run(argv, **kwargs): (None, "")) +# Unbuffered writes: a handler writing into a buffered file mid-write raises "reentrant call". +_ANVIL_STANDIN = r""" +import os, signal, sys, time +fifo, survived = sys.argv[1], [name for name in sys.argv[2].split(",") if name] +report = os.open(fifo, os.O_WRONLY) +def survive(signum, frame): + os.write(report, b"%d\n" % signum) +for name in survived: + signal.signal(getattr(signal, name), survive) +os.write(report, b"ready\n") +time.sleep(60) +""" + + +class ForegroundInterruptTests(unittest.TestCase): + """`_wait_foreground` against a real child, interrupted by real signals.""" + + def setUp(self): + tmp = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, tmp) + self.fifo = os.path.join(tmp, "report") + os.mkfifo(self.fifo) + # As under the launcher's trap, only the first of each signal raises. + for signum, interrupt in ((signal.SIGTERM, launcher.TerminationSignal), + (signal.SIGINT, lambda _signum: KeyboardInterrupt())): + previous = signal.signal(signum, self._first_only(interrupt)) + self.addCleanup(signal.signal, signum, previous) + self.procs = [] + self.waits = queue.Queue() + waits = self.waits + + class ObservedPopen(subprocess.Popen): + """Announces each wait, so a driver can tell the launcher has moved on.""" + + def wait(self, timeout=None): + waits.put(timeout) + return super().wait(timeout) + + def recording_popen(argv): + proc = ObservedPopen(argv) + self.procs.append(proc) + return proc + + patcher = mock.patch.object(launcher.docker.subprocess, "Popen", recording_popen) + patcher.start() + self.addCleanup(patcher.stop) + self.addCleanup(self._kill_leftovers) + + @staticmethod + def _first_only(interrupt): + taken = [] + + def handler(signum, frame): + if not taken: + taken.append(signum) + raise interrupt(signum) + + return handler + + def _kill_leftovers(self): + for proc in self.procs: + if proc.poll() is None: + proc.kill() + proc.wait() + + def _run(self, survived, drive, grace): + """Run the stand-in, letting `drive(report, deliver)` interrupt it. + + `deliver(signum)` signals the launcher and returns once it has entered + its next wait. Returns `(exception raised, stand-in's exit code, seconds taken)`. + """ + main_thread = threading.main_thread().ident + + def deliver(signum): + # One landing just before a blocking waitpid waits for it to return, so re-send. + while True: + signal.pthread_kill(main_thread, signum) + try: + self.waits.get(timeout=0.5) + return + except queue.Empty: + pass + + def driver(): + with open(self.fifo) as report: + if report.readline() == "ready\n": + self.waits.get() + drive(report, deliver) + report.read() # held open until the stand-in exits, so its reports never break + + threading.Thread(target=driver, daemon=True).start() + argv = [sys.executable, "-c", _ANVIL_STANDIN, self.fifo, survived] + started = time.monotonic() + with mock.patch.object(launcher.docker, "_FOREGROUND_STOP_GRACE_S", grace): + with self.assertRaises(BaseException) as raised: + launcher.DockerCLI().run_foreground(argv) + elapsed = time.monotonic() - started + (proc,) = self.procs + self.assertIsNotNone(proc.returncode, "the anvil client was left unreaped") + return raised.exception, proc.returncode, elapsed + + def test_forwarded_signal_ends_the_anvil(self): + exc, code, elapsed = self._run( + "", lambda report, deliver: deliver(signal.SIGTERM), grace=30) + self.assertIsInstance(exc, launcher.TerminationSignal) + self.assertEqual(code, -signal.SIGTERM) + self.assertLess(elapsed, 20) + + def test_anvil_surviving_the_forwarded_signal_is_killed_after_the_grace(self): + exc, code, _ = self._run( + "SIGTERM", lambda report, deliver: deliver(signal.SIGTERM), grace=0.2) + self.assertIsInstance(exc, launcher.TerminationSignal) + self.assertEqual(code, -signal.SIGKILL) + + def test_signal_while_waiting_out_a_ctrl_c_is_forwarded(self): + # The anvil survived the Ctrl-C (as Claude's input-clearing does), so the launcher waits. + def drive(report, deliver): + deliver(signal.SIGINT) + deliver(signal.SIGTERM) + + exc, code, elapsed = self._run("", drive, grace=30) + self.assertIsInstance(exc, launcher.TerminationSignal) + self.assertIsInstance(exc.__context__, KeyboardInterrupt) + self.assertEqual(code, -signal.SIGTERM) + self.assertLess(elapsed, 20) + + def test_ctrl_c_during_the_grace_kills_the_anvil(self): + def drive(report, deliver): + deliver(signal.SIGTERM) + if report.readline() == "%d\n" % signal.SIGTERM: + deliver(signal.SIGINT) + + exc, code, elapsed = self._run("SIGTERM", drive, grace=30) + self.assertIsInstance(exc, KeyboardInterrupt) + self.assertEqual(code, -signal.SIGKILL) + self.assertLess(elapsed, 20) + + if __name__ == "__main__": unittest.main(verbosity=2) diff --git a/tests/test_anvil_orchestrate.py b/tests/test_anvil_orchestrate.py index 8e7a64a..4ea650c 100644 --- a/tests/test_anvil_orchestrate.py +++ b/tests/test_anvil_orchestrate.py @@ -1,9 +1,11 @@ #!/usr/bin/env python3 """Unit tests for swarmforge.anvil.orchestrate. Run: python3 tests/test_anvil_orchestrate.py""" +import contextlib import json import os import shutil +import signal import subprocess import sys import tempfile @@ -743,6 +745,66 @@ def interrupt(argv, extra_networks, container): self.assertIn(("rm_force", "claude-myproject"), docker.calls) self.assertIn(("network_rm", net), docker.calls) + def test_teardown_runs_inside_the_callers_guard(self): + docker = FakeDocker() + net = tongs.session_network_name("claude-myproject") + + @contextlib.contextmanager + def guard(): + docker.calls.append("guard-enter") + yield + docker.calls.append("guard-exit") + + launcher.run_with_tongs( + _merged("pg", SESSION_PORT, source=tongs.REPO), ANVIL_ARGV, _opts(), + docker=docker, teardown_guard=guard, sleep=lambda _s: None, monotonic=_Clock(), + ) + guarded = docker.calls[docker.calls.index("guard-enter"):] + self.assertEqual(guarded[-1], "guard-exit") + self.assertIn(("rm_force", "claude-myproject-tong-pg"), guarded) + self.assertIn(("network_rm", net), guarded) + + def test_session_teardown_runs_on_termination_signal(self): + docker = FakeDocker() + + def hang_up(argv, extra_networks, container): + raise launcher.TerminationSignal(signal.SIGHUP) + + docker.run_foreground_multi = hang_up + net = tongs.session_network_name("claude-myproject") + with self.assertRaises(launcher.TerminationSignal): + self._run(docker, _merged("pg", SESSION_PORT, source=tongs.REPO)) + self.assertIn(("rm_force", "claude-myproject-tong-pg"), docker.calls) + self.assertIn(("rm_force", "claude-myproject"), docker.calls) + self.assertIn(("network_rm", net), docker.calls) + + def test_tong_whose_docker_run_is_cut_short_is_removed(self): + # The daemon may have started it before the client was killed. + docker = FakeDocker() + container = "claude-myproject-tong-pg" + + def cut_short(argv): + docker.calls.append(("run_detached", container)) + raise launcher.TerminationSignal(signal.SIGTERM) + + docker.run_detached = cut_short + with self.assertRaises(launcher.TerminationSignal): + self._run(docker, _merged("pg", SESSION_PORT, source=tongs.REPO)) + started = docker.calls.index(("run_detached", container)) + self.assertIn(("rm_force", container), docker.calls[started:]) + + def test_network_whose_create_is_cut_short_is_removed(self): + docker = FakeDocker() + net = tongs.session_network_name("claude-myproject") + + def cut_short(name): + raise launcher.TerminationSignal(signal.SIGHUP) + + docker.ensure_network = cut_short + with self.assertRaises(launcher.TerminationSignal): + self._run(docker, _merged("pg", SESSION_PORT, source=tongs.REPO)) + self.assertIn(("network_rm", net), docker.calls) + def test_session_tong_without_anvil_name_raises_before_any_docker_call(self): docker = FakeDocker() anvil = ["docker", "run", "-it", "--rm", "--network", "opencode-net", "img"]