feat(companion): Make manual_stop ownership explicit

spawn_process no longer clears manual_stop; spawning is now policy-neutral.
Clearing the flag is owned by start_process and restart_process (which already
do it), and the respawn paths (retry_backoff, restart_pending) only run when
the flag is already false. A manually stopped companion now keeps manual_stop
set through its exit, so it settles in STOPPED and is not auto-restarted.

Add tests: manual_stop preserved through exit, start clears it, spawn leaves
it untouched.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Tanmoy Sarkar 2026-06-09 18:17:44 +05:30
parent 8e0ca34277
commit c82df2ab94
3 changed files with 40 additions and 2 deletions

View File

@ -680,7 +680,7 @@ No per-companion logic in Arbiter.
- [x] Implement `start_process`. - [x] Implement `start_process`.
- [x] Implement `stop_process`. - [x] Implement `stop_process`.
- [x] Implement `restart_process`. - [x] Implement `restart_process`.
- [ ] Preserve and clear `manual_stop` correctly. - [x] Preserve and clear `manual_stop` correctly.
- [ ] Add Unix control socket. - [ ] Add Unix control socket.
- [ ] Implement JSON command protocol. - [ ] Implement JSON command protocol.
- [ ] Implement `status`. - [ ] Implement `status`.

View File

@ -37,13 +37,17 @@ class CompanionManager:
Parent records the pid and moves the companion to STARTING. Child Parent records the pid and moves the companion to STARTING. Child
resolves and runs the target, exiting the worker on any failure so a resolves and runs the target, exiting the worker on any failure so a
crashed companion never leaks back into the manager's control flow. crashed companion never leaks back into the manager's control flow.
Spawning is policy-neutral: it does not touch ``manual_stop``. Clearing
that flag is the job of the commands that intentionally bring a
companion back (:meth:`start_process`, :meth:`restart_process`), and a
companion only ever reaches a respawn path with the flag already false.
""" """
pid = os.fork() pid = os.fork()
if pid != 0: if pid != 0:
proc.pid = pid proc.pid = pid
proc.state = State.STARTING proc.state = State.STARTING
proc.started_at = time.time() proc.started_at = time.time()
proc.manual_stop = False
self.log.info("companion %s started (pid %s)", proc.name, pid) self.log.info("companion %s started (pid %s)", proc.name, pid)
return pid return pid

View File

@ -274,6 +274,40 @@ def test_restart_process_stopping_rejected():
assert not ok and "stopping" in msg assert not ok and "stopping" in msg
def test_manual_stop_preserved_through_exit():
# stop a running companion, then reap its child: it must settle in STOPPED
# with manual_stop still set so it is not auto-restarted.
mgr = make_manager("rq")
proc = mgr.processes["rq"]
proc.state = State.RUNNING
proc.pid = 60
with mock.patch("os.kill"):
mgr.stop_process("rq", now=10.0)
with mock.patch("os.waitpid", side_effect=[(60, 0), (0, 0)]), \
mock.patch("os.fork") as fork:
mgr.reap_processes()
fork.assert_not_called()
assert proc.state == State.STOPPED and proc.manual_stop is True
def test_start_clears_manual_stop():
mgr = make_manager("rq")
proc = mgr.processes["rq"]
proc.manual_stop = True
with mock.patch("os.fork", return_value=61):
mgr.start_process("rq")
assert proc.manual_stop is False
def test_spawn_does_not_touch_manual_stop():
mgr = make_manager("rq")
proc = mgr.processes["rq"]
proc.manual_stop = True
with mock.patch("os.fork", return_value=62):
mgr.spawn_process(proc)
assert proc.manual_stop is True
def test_handle_exit_unexpected_backoff(): def test_handle_exit_unexpected_backoff():
mgr = make_manager("rq") mgr = make_manager("rq")
proc = mgr.processes["rq"] proc = mgr.processes["rq"]