From 4c559bf2a466fe0dbd5c65ceaa00b196adfe8fe4 Mon Sep 17 00:00:00 2001 From: Lucas de Castro Zanoni Date: Mon, 24 Aug 2026 21:52:35 -0300 Subject: [PATCH] fix(clawde): replace a sidecar still running superseded code The supervisor asked pgrep for the sidecar's match pattern and, finding anything alive, declared the sidecar reconciled. The pattern deliberately carries no store path, so a bridge launched from an older generation of bridge.py matches it exactly as well as the current one does. A rebuild therefore could never put new bridge code on a running agent: monster on chise has carried the same bridge process for two days across four system generations. The sidecar now records the command it was launched from beside its log, and a live process whose record no longer matches the desired command is terminated, waited for, and replaced. A sidecar with no record at all is replaced once and records itself, so the first reconcile after this change refreshes every bridge and later ones leave it alone. Terminating and waiting before spawning keeps the replacement from holding the bot token at the same moment as the process it supersedes, which is the same double-answer this module already guards against for duplicates. Agent-Machine: kira Agent-Resume: claude --resume efee1316-8d94-438e-951e-e9c978626531 --- .../sidecar_process_reconcile.py | 69 ++++++++ .../unit/sidecar_process_test_support.py | 10 ++ ...lawde_service_sidecar_process_reconcile.py | 15 +- ...de_service_sidecar_process_supersession.py | 147 ++++++++++++++++++ 4 files changed, 238 insertions(+), 3 deletions(-) create mode 100644 module/scripts/tests/unit/test_clawde_service_sidecar_process_supersession.py diff --git a/module/scripts/clawde-service/sidecar_process_reconcile.py b/module/scripts/clawde-service/sidecar_process_reconcile.py index 82d0fae..27fad0b 100644 --- a/module/scripts/clawde-service/sidecar_process_reconcile.py +++ b/module/scripts/clawde-service/sidecar_process_reconcile.py @@ -2,6 +2,11 @@ import pathlib import signal import subprocess +import time + +SPAWNED_COMMAND_RECORD_SUFFIX = ".spawned-command" +TERMINATION_DEADLINE_SECONDS = 5.0 +TERMINATION_POLL_SECONDS = 0.1 def find_sidecar_process_ids(process_match_pattern: str) -> list[int]: @@ -20,6 +25,66 @@ def terminate_sidecar_process(process_id: int) -> None: pass +def process_is_alive(process_id: int) -> bool: + try: + os.kill(process_id, 0) + except ProcessLookupError: + return False + return True + + +def termination_poll_attempts() -> int: + return int(TERMINATION_DEADLINE_SECONDS / TERMINATION_POLL_SECONDS) + + +def wait_for_process_to_exit(process_id: int) -> None: + for _attempt in range(termination_poll_attempts()): + if not process_is_alive(process_id): + return + time.sleep(TERMINATION_POLL_SECONDS) + + +def spawned_command_record_for(sidecar_specification: dict) -> pathlib.Path: + return pathlib.Path( + sidecar_specification["log_file"] + SPAWNED_COMMAND_RECORD_SUFFIX + ) + + +def recorded_spawned_command(sidecar_specification: dict) -> str | None: + record_file = spawned_command_record_for(sidecar_specification) + try: + return record_file.read_text() + except OSError: + return None + + +def record_spawned_command(sidecar_specification: dict) -> None: + record_file = spawned_command_record_for(sidecar_specification) + record_file.parent.mkdir(parents=True, exist_ok=True) + record_file.write_text(sidecar_specification["command"]) + + +def live_processes_run_the_current_command(sidecar_specification: dict) -> bool: + return ( + recorded_spawned_command(sidecar_specification) + == (sidecar_specification["command"]) + ) + + +def replace_processes_running_superseded_code( + sidecar_specification: dict, live_process_ids: list[int] +) -> list[int]: + if not live_process_ids or live_processes_run_the_current_command( + sidecar_specification + ): + return live_process_ids + for process_id in live_process_ids: + terminate_sidecar_process(process_id) + for process_id in live_process_ids: + wait_for_process_to_exit(process_id) + return [] + + def open_sidecar_log_file(log_file_path: str): log_file = pathlib.Path(log_file_path) log_file.parent.mkdir(parents=True, exist_ok=True) @@ -31,6 +96,7 @@ def detach_from_supervisor(command: str) -> str: def spawn_sidecar_process(sidecar_specification: dict) -> None: + record_spawned_command(sidecar_specification) with open_sidecar_log_file(sidecar_specification["log_file"]) as log_file: subprocess.run( ["sh", "-c", detach_from_supervisor(sidecar_specification["command"])], @@ -51,6 +117,9 @@ def reconcile_one_sidecar_process( for process_id in live_process_ids: terminate_sidecar_process(process_id) return False + live_process_ids = replace_processes_running_superseded_code( + sidecar_specification, live_process_ids + ) for duplicate_process_id in live_process_ids[1:]: terminate_sidecar_process(duplicate_process_id) if live_process_ids: diff --git a/module/scripts/tests/unit/sidecar_process_test_support.py b/module/scripts/tests/unit/sidecar_process_test_support.py index dbfefaf..41ab577 100644 --- a/module/scripts/tests/unit/sidecar_process_test_support.py +++ b/module/scripts/tests/unit/sidecar_process_test_support.py @@ -23,6 +23,11 @@ def make_sidecar_specification_with_lifetime(tmp_path, lifetime, enabled=True): } +def record_the_sidecar_as_launched_from_its_current_command(specification): + sidecar_process_reconcile.record_spawned_command(specification) + return specification + + def make_session_specification(tmp_path): return { "name": "clawde", @@ -54,4 +59,9 @@ def record_process_lookups(monkeypatch, live_process_ids): "terminate_sidecar_process", terminated_process_ids.append, ) + monkeypatch.setattr( + sidecar_process_reconcile, + "wait_for_process_to_exit", + lambda _process_id: None, + ) return spawned_specifications, terminated_process_ids diff --git a/module/scripts/tests/unit/test_clawde_service_sidecar_process_reconcile.py b/module/scripts/tests/unit/test_clawde_service_sidecar_process_reconcile.py index 878edf6..81444f3 100644 --- a/module/scripts/tests/unit/test_clawde_service_sidecar_process_reconcile.py +++ b/module/scripts/tests/unit/test_clawde_service_sidecar_process_reconcile.py @@ -14,6 +14,7 @@ make_sidecar_specification_with_lifetime, make_session_specification, record_process_lookups, + record_the_sidecar_as_launched_from_its_current_command, ) @@ -35,7 +36,10 @@ def test_a_live_sidecar_is_never_relaunched(tmp_path, monkeypatch): ) sidecar_process_reconcile.reconcile_one_sidecar_process( - make_sidecar_specification(tmp_path), True + record_the_sidecar_as_launched_from_its_current_command( + make_sidecar_specification(tmp_path) + ), + True, ) assert spawned_specifications == [] @@ -50,7 +54,10 @@ def test_duplicate_sidecar_processes_are_culled_down_to_the_oldest( ) sidecar_process_reconcile.reconcile_one_sidecar_process( - make_sidecar_specification(tmp_path), True + record_the_sidecar_as_launched_from_its_current_command( + make_sidecar_specification(tmp_path) + ), + True, ) assert terminated_process_ids == [450, 700] @@ -175,7 +182,9 @@ def test_a_service_lifetime_sidecar_survives_its_agents_dormancy(tmp_path, monke spawned_specifications, terminated_process_ids = record_process_lookups( monkeypatch, [4321] ) - specification = make_sidecar_specification_with_lifetime(tmp_path, "service") + specification = record_the_sidecar_as_launched_from_its_current_command( + make_sidecar_specification_with_lifetime(tmp_path, "service") + ) sidecar_process_reconcile.reconcile_one_sidecar_process( specification, diff --git a/module/scripts/tests/unit/test_clawde_service_sidecar_process_supersession.py b/module/scripts/tests/unit/test_clawde_service_sidecar_process_supersession.py new file mode 100644 index 0000000..4f48d8e --- /dev/null +++ b/module/scripts/tests/unit/test_clawde_service_sidecar_process_supersession.py @@ -0,0 +1,147 @@ +import pathlib +import sys + +sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent)) +sys.path.insert( + 0, str(pathlib.Path(__file__).resolve().parent.parent.parent / "clawde-service") +) + +import sidecar_process_reconcile +from sidecar_process_test_support import ( + SIDECAR_NAME, + make_sidecar_specification, + record_process_lookups, + record_the_sidecar_as_launched_from_its_current_command, +) + + +def reconcile_a_running_sidecar(specification, monkeypatch, live_process_ids=(4321,)): + spawned_specifications, terminated_process_ids = record_process_lookups( + monkeypatch, list(live_process_ids) + ) + sidecar_process_reconcile.reconcile_one_sidecar_process(specification, True) + return spawned_specifications, terminated_process_ids + + +def test_a_sidecar_still_running_superseded_code_is_replaced(tmp_path, monkeypatch): + specification = record_the_sidecar_as_launched_from_its_current_command( + make_sidecar_specification(tmp_path, command="python3 /nix/store/old-bridge.py") + ) + specification["command"] = "python3 /nix/store/new-bridge.py" + + spawned_specifications, terminated_process_ids = reconcile_a_running_sidecar( + specification, monkeypatch + ) + + assert terminated_process_ids == [4321] + assert [specification["name"] for specification in spawned_specifications] == [ + SIDECAR_NAME + ] + + +def test_every_process_of_a_superseded_sidecar_is_terminated_before_the_replacement( + tmp_path, monkeypatch +): + waited_for_process_ids = [] + specification = record_the_sidecar_as_launched_from_its_current_command( + make_sidecar_specification(tmp_path, command="python3 /nix/store/old-bridge.py") + ) + specification["command"] = "python3 /nix/store/new-bridge.py" + spawned_specifications, terminated_process_ids = record_process_lookups( + monkeypatch, [700, 120, 450] + ) + monkeypatch.setattr( + sidecar_process_reconcile, + "wait_for_process_to_exit", + waited_for_process_ids.append, + ) + + sidecar_process_reconcile.reconcile_one_sidecar_process(specification, True) + + assert terminated_process_ids == [120, 450, 700] + assert waited_for_process_ids == [120, 450, 700] + assert len(spawned_specifications) == 1 + + +def test_a_sidecar_running_the_current_code_is_left_alone(tmp_path, monkeypatch): + specification = record_the_sidecar_as_launched_from_its_current_command( + make_sidecar_specification(tmp_path, command="python3 /nix/store/bridge.py") + ) + + spawned_specifications, terminated_process_ids = reconcile_a_running_sidecar( + specification, monkeypatch + ) + + assert spawned_specifications == [] + assert terminated_process_ids == [] + + +def test_a_sidecar_launched_before_this_record_existed_is_replaced_exactly_once( + tmp_path, monkeypatch +): + specification = make_sidecar_specification(tmp_path, command="true") + + _, first_terminated_process_ids = reconcile_a_running_sidecar( + specification, monkeypatch + ) + sidecar_process_reconcile.record_spawned_command(specification) + _, second_terminated_process_ids = reconcile_a_running_sidecar( + specification, monkeypatch + ) + + assert first_terminated_process_ids == [4321] + assert second_terminated_process_ids == [] + + +def test_spawning_records_the_command_it_launched(tmp_path): + specification = make_sidecar_specification(tmp_path, command="true") + + sidecar_process_reconcile.spawn_sidecar_process(specification) + + assert sidecar_process_reconcile.recorded_spawned_command(specification) == "true" + + +def test_a_superseded_sidecar_that_should_stop_is_terminated_without_replacement( + tmp_path, monkeypatch +): + specification = record_the_sidecar_as_launched_from_its_current_command( + make_sidecar_specification(tmp_path, command="python3 /nix/store/old-bridge.py") + ) + specification["command"] = "python3 /nix/store/new-bridge.py" + spawned_specifications, terminated_process_ids = record_process_lookups( + monkeypatch, [4321] + ) + + sidecar_process_reconcile.reconcile_one_sidecar_process(specification, False) + + assert terminated_process_ids == [4321] + assert spawned_specifications == [] + + +def test_waiting_returns_as_soon_as_the_process_is_gone(monkeypatch): + monkeypatch.setattr( + sidecar_process_reconcile, "process_is_alive", lambda _process_id: False + ) + monkeypatch.setattr( + sidecar_process_reconcile, + "time", + type("NeverSleeps", (), {"sleep": staticmethod(lambda _seconds: None)}), + ) + + sidecar_process_reconcile.wait_for_process_to_exit(4321) + + +def test_waiting_gives_up_on_a_process_that_refuses_to_exit(monkeypatch): + slept_seconds = [] + monkeypatch.setattr( + sidecar_process_reconcile, "process_is_alive", lambda _process_id: True + ) + monkeypatch.setattr( + sidecar_process_reconcile, + "time", + type("CountsSleeps", (), {"sleep": staticmethod(slept_seconds.append)}), + ) + + sidecar_process_reconcile.wait_for_process_to_exit(4321) + + assert len(slept_seconds) == sidecar_process_reconcile.termination_poll_attempts()