From fdd47ae9400d50af8078b945b1185d49285aa208 Mon Sep 17 00:00:00 2001 From: Guillaume De Saint Martin Date: Tue, 28 Jul 2026 17:04:55 +0200 Subject: [PATCH 1/3] [Binary] include new deps requirements in build --- bin/start.spec | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/bin/start.spec b/bin/start.spec index a4dfa8c62c..18c0e03875 100644 --- a/bin/start.spec +++ b/bin/start.spec @@ -1,7 +1,13 @@ # -*- mode: python -*- +from PyInstaller.utils.hooks import collect_data_files + block_cipher = None +# eth_account.hdaccount reads BIP39 wordlists from disk (hdaccount/wordlist/*.txt). +# hiddenimports only bundles Python modules; collect_data_files includes those data files. +eth_account_datas = collect_data_files("eth_account") + OCTOBOT_PACKAGES_FILES = REQUIRED = [s.strip() for s in open('bin/octobot_packages_files.txt').readlines()] # hiddenimports=['numpy.core._dtype_ctypes'] from https://github.com/pyinstaller/pyinstaller/issues/3982 a = Analysis( @@ -9,11 +15,12 @@ a = Analysis( pathex=['../'], datas=[ ('../octobot/config', 'octobot/config'), - ('../octobot/strategy_optimizer/optimizer_data_files', 'octobot/strategy_optimizer/optimizer_data_files') - ], + ('../octobot/strategy_optimizer/optimizer_data_files', 'octobot/strategy_optimizer/optimizer_data_files'), + ] + eth_account_datas, # required for node wallet mnemonic generation (web3.Account.create_with_mnemonic) hiddenimports=[ "colorlog", "numpy.core._dtype_ctypes", "dotenv", "pgpy", "imghdr", + "web3", "eth_account", "aiosqlite", "aiohttp", "pyarrow", "pyiceberg", "psutil", @@ -22,6 +29,7 @@ a = Analysis( "asyncpraw", "simplifiedpytrends", "simplifiedpytrends.exceptions", "simplifiedpytrends.request", "pyngrok", "pyngrok.ngrok", "openai", "flask", "flask_login", "flask_wtf", "flask_caching", "flask_compress", "flask_socketio", "flask_cors", + "werkzeug.middleware", "werkzeug.middleware.proxy_fix", "wtforms", "wtforms.fields", "gevent", "geventwebsocket", "vaderSentiment", "vaderSentiment.vaderSentiment", "coingecko_openapi_client", From abcb3acec0a5c7959eac35d849c86a8712e34888 Mon Sep 17 00:00:00 2001 From: Guillaume De Saint Martin Date: Tue, 28 Jul 2026 21:32:40 +0200 Subject: [PATCH 2/3] [WebInterface] fix medias path for process bots --- .../web_interface/controllers/medias.py | 1 + .../Interfaces/web_interface/models/medias.py | 39 ++++++++++- .../web_interface/tests/test_medias_models.py | 66 +++++++++++++++++++ 3 files changed, 104 insertions(+), 2 deletions(-) create mode 100644 packages/tentacles/Services/Interfaces/web_interface/tests/test_medias_models.py diff --git a/packages/tentacles/Services/Interfaces/web_interface/controllers/medias.py b/packages/tentacles/Services/Interfaces/web_interface/controllers/medias.py index b318a73145..a1a493592f 100644 --- a/packages/tentacles/Services/Interfaces/web_interface/controllers/medias.py +++ b/packages/tentacles/Services/Interfaces/web_interface/controllers/medias.py @@ -43,6 +43,7 @@ def profile_media(path): if models.is_valid_profile_image_path(path): # reference point is the web interface directory: use OctoBot root folder as a reference return _send_file("../../../..", path) + flask.abort(404) @blueprint.route('/exchange_logo/') diff --git a/packages/tentacles/Services/Interfaces/web_interface/models/medias.py b/packages/tentacles/Services/Interfaces/web_interface/models/medias.py index e2f25f1b51..245437a81c 100644 --- a/packages/tentacles/Services/Interfaces/web_interface/models/medias.py +++ b/packages/tentacles/Services/Interfaces/web_interface/models/medias.py @@ -13,6 +13,9 @@ # # You should have received a copy of the GNU Lesser General Public # License along with this library. +import os + +import octobot_commons.constants as commons_constants import octobot_tentacles_manager.constants as tentacles_manager_constants import octobot_commons.user_root_folder_provider as user_root_folder_provider @@ -24,15 +27,47 @@ def _is_valid_path(path, header): return path.startswith(header) and ".." not in path +def _profile_image_allowed_roots(): + # Local profiles under the process user root, plus master profiles when this + # process is an automation child (OCTOBOT_SYNC_DATA_ROOT points at the executor user/). + allowed_roots = [user_root_folder_provider.get_user_profiles_folder()] + sync_profiles_root = os.path.join( + user_root_folder_provider.get_sync_data_root(), + commons_constants.PROFILES_FOLDER, + ) + normalized_roots = {os.path.normcase(os.path.normpath(root)) for root in allowed_roots} + if os.path.normcase(os.path.normpath(sync_profiles_root)) not in normalized_roots: + allowed_roots.append(sync_profiles_root) + return allowed_roots + + +def _is_path_under_root(path, root): + # Reject traversal before normalization; commonpath handles absolute vs relative + # paths on Windows (avatar_path is often an absolute master profiles path in URLs). + if ".." in path: + return False + normalized_path = os.path.normcase(os.path.normpath(path)) + normalized_root = os.path.normcase(os.path.normpath(root)) + try: + common_path = os.path.commonpath([normalized_path, normalized_root]) + except ValueError: + return False + return common_path == normalized_root + + def is_valid_tentacle_image_path(path): path_ending = path.split(".")[-1].lower() return path_ending in ALLOWED_IMAGE_FORMATS and _is_valid_path(path, tentacles_manager_constants.TENTACLES_PATH) def is_valid_profile_image_path(path): + """True when path is a supported image under local or sync-data profiles.""" path_ending = path.split(".")[-1].lower() - return path_ending in ALLOWED_IMAGE_FORMATS and _is_valid_path( - path, user_root_folder_provider.get_user_profiles_folder() + if path_ending not in ALLOWED_IMAGE_FORMATS: + return False + return any( + _is_path_under_root(path, allowed_root) + for allowed_root in _profile_image_allowed_roots() ) diff --git a/packages/tentacles/Services/Interfaces/web_interface/tests/test_medias_models.py b/packages/tentacles/Services/Interfaces/web_interface/tests/test_medias_models.py new file mode 100644 index 0000000000..f6e9948bd3 --- /dev/null +++ b/packages/tentacles/Services/Interfaces/web_interface/tests/test_medias_models.py @@ -0,0 +1,66 @@ +# Drakkar-Software OctoBot-Tentacles +# Copyright (c) Drakkar-Software, All rights reserved. +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 3.0 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library. + +import os + +import pytest + +import octobot_commons.constants as commons_constants +import octobot_commons.user_root_folder_provider as user_root_folder_provider +import tentacles.Services.Interfaces.web_interface.models.medias as medias_model + + +class TestIsValidProfileImagePath: + @pytest.fixture(autouse=True) + def setup_provider(self, tmp_path, monkeypatch): + self.child_user_root = tmp_path / "child_user" + self.master_user_root = tmp_path / "master_user" + self.child_user_root.mkdir() + self.master_user_root.mkdir() + provider = user_root_folder_provider.UserRootFolderProvider.instance() + provider.set_root(str(self.child_user_root)) + monkeypatch.setenv( + commons_constants.ENV_OCTOBOT_SYNC_DATA_ROOT, + str(self.master_user_root), + ) + yield + provider.set_root(None) + monkeypatch.delenv(commons_constants.ENV_OCTOBOT_SYNC_DATA_ROOT, raising=False) + + def test_accepts_child_local_profile_image(self): + profiles_root = self.child_user_root / commons_constants.PROFILES_FOLDER + profile_path = profiles_root / "daily_trading" / "default_profile.png" + profile_path.parent.mkdir(parents=True) + profile_path.touch() + assert medias_model.is_valid_profile_image_path(str(profile_path)) + + def test_accepts_master_sync_data_profile_image(self): + profiles_root = self.master_user_root / commons_constants.PROFILES_FOLDER + profile_path = profiles_root / "daily_trading" / "default_profile.png" + profile_path.parent.mkdir(parents=True) + profile_path.touch() + assert medias_model.is_valid_profile_image_path(str(profile_path)) + + def test_rejects_path_outside_allowed_roots(self): + outside_path = self.child_user_root / "outside.png" + outside_path.touch() + assert medias_model.is_valid_profile_image_path(str(outside_path)) is False + + def test_rejects_path_traversal(self): + profiles_root = self.child_user_root / commons_constants.PROFILES_FOLDER + profiles_root.mkdir(parents=True) + traversal_path = os.path.join(str(profiles_root), "..", "secret.png") + assert medias_model.is_valid_profile_image_path(traversal_path) is False From dbda6d0cd23ba9799ba361efff364545d4cc3a92 Mon Sep 17 00:00:00 2001 From: Guillaume De Saint Martin Date: Tue, 28 Jul 2026 21:33:08 +0200 Subject: [PATCH 3/3] [Process bots] fix PID & frozen env issues --- octobot/commands.py | 5 +- .../operators/process_bound_operator_mixin.py | 13 + .../managed_child_process_registry.py | 27 ++ packages/commons/octobot_commons/os_util.py | 18 +- .../commons/octobot_commons/process_util.py | 9 + .../test_managed_child_process_registry.py | 69 ++++ packages/commons/tests/test_os_util.py | 23 ++ .../octobot_process_ops.py | 68 ++-- .../tests/test_octobot_process_ops.py | 296 +++++++++++++++++- tests/unit_tests/test_commands.py | 42 +++ 10 files changed, 543 insertions(+), 27 deletions(-) diff --git a/octobot/commands.py b/octobot/commands.py index fa492ae387..64897320f5 100644 --- a/octobot/commands.py +++ b/octobot/commands.py @@ -28,6 +28,7 @@ import octobot_commons.logging as logging import octobot_commons.constants as commons_constants import octobot_commons.errors as commons_errors +import octobot_commons.os_util as os_util import octobot_commons.aiohttp_util as aiohttp_util import octobot_tentacles_manager.api as tentacles_manager_api @@ -343,7 +344,7 @@ def get_bot_file(): def restart_bot(): - argv = (f'{a}' for a in sys.argv if a not in IGNORED_COMMAND_WHEN_RESTART) + argv = [f'{argument}' for argument in sys.argv if argument not in IGNORED_COMMAND_WHEN_RESTART] if get_bot_file().endswith(".py"): os.execl(sys.executable, f'{sys.executable}', *argv) elif get_bot_file().endswith(constants.PROJECT_NAME): @@ -353,7 +354,7 @@ def restart_bot(): # restart from binary # from https://pyinstaller.org/en/stable/common-issues-and-pitfalls.html#using-sys-executable-to-spawn-subprocesses-that-outlive-the-application-process-implementing-application-restart # Restart the application - subprocess.Popen([sys.executable], env={**os.environ, "PYINSTALLER_RESET_ENVIRONMENT": "1"}) + subprocess.Popen(argv, env={**os.environ, **os_util.PYINSTALLER_RESET_ENVIRONMENT_VARS}) # force stop and restart os._exit(0) diff --git a/packages/commons/octobot_commons/dsl_interpreter/operators/process_bound_operator_mixin.py b/packages/commons/octobot_commons/dsl_interpreter/operators/process_bound_operator_mixin.py index 7c1212f3c8..e8069a3ac6 100644 --- a/packages/commons/octobot_commons/dsl_interpreter/operators/process_bound_operator_mixin.py +++ b/packages/commons/octobot_commons/dsl_interpreter/operators/process_bound_operator_mixin.py @@ -92,6 +92,19 @@ def spawn_subprocess( self.pid = proc.pid return proc + def bind_authoritative_child_pid( + self, + authoritative_pid: int, + *, + spawn_pid: typing.Optional[int] = None, + ) -> None: + """Point this operator and the managed-child registry at the authoritative app pid.""" + previous_pid = spawn_pid if spawn_pid is not None else (self.pid or 0) + if previous_pid == authoritative_pid and self.pid == authoritative_pid: + return + process_util.rebind_managed_child_pid(previous_pid, authoritative_pid) + self.pid = authoritative_pid + @staticmethod def reject_user_path_segment(path_value: str) -> None: """Reject obvious path traversal in user-supplied relative paths.""" diff --git a/packages/commons/octobot_commons/managed_child_process_registry.py b/packages/commons/octobot_commons/managed_child_process_registry.py index 2504a31a32..8db1f3c4d0 100644 --- a/packages/commons/octobot_commons/managed_child_process_registry.py +++ b/packages/commons/octobot_commons/managed_child_process_registry.py @@ -56,6 +56,33 @@ def unregister(self, pid: int) -> None: self._pids.discard(pid) self._logger.debug("Unregistered managed child pid=%s", pid) + def rebind_managed_child_pid(self, spawn_pid: int, authoritative_pid: int) -> None: + """ + Replace a bootstrap/spawn pid with the authoritative app pid (e.g. PyInstaller child). + + Unregisters ``spawn_pid`` when it differs from ``authoritative_pid``, then registers + the authoritative pid. No-op when ``authoritative_pid`` is not positive or already bound. + """ + if authoritative_pid <= 0: + return + with self._lock: + authoritative_already_registered = authoritative_pid in self._pids + spawn_in_registry = spawn_pid > 0 and spawn_pid in self._pids + if authoritative_already_registered and ( + spawn_pid <= 0 or spawn_pid == authoritative_pid + ): + return + if spawn_pid > 0 and spawn_pid != authoritative_pid and spawn_in_registry: + self.unregister(spawn_pid) + if not authoritative_already_registered: + self.register(authoritative_pid) + if spawn_pid > 0 and spawn_pid != authoritative_pid: + self._logger.debug( + "Rebound managed child pid from %s to %s", + spawn_pid, + authoritative_pid, + ) + def snapshot_running_pids(self) -> frozenset[int]: """Prune dead pids, return a point-in-time copy of still-running entries.""" with self._lock: diff --git a/packages/commons/octobot_commons/os_util.py b/packages/commons/octobot_commons/os_util.py index ecb434b4c2..e82fad513b 100644 --- a/packages/commons/octobot_commons/os_util.py +++ b/packages/commons/octobot_commons/os_util.py @@ -95,6 +95,22 @@ def get_octobot_type(): return enums.OctoBotTypes.BINARY.value +# PyInstaller 6.9+: environment entries for spawning an independent frozen child process. +# Required when re-execing or subprocess-spawning the same frozen executable so each child +# unpacks and runs as its own instance (see PyInstaller common issues and pitfalls). +PYINSTALLER_RESET_ENVIRONMENT_VARS = {"PYINSTALLER_RESET_ENVIRONMENT": "1"} + + +def is_frozen_binary_octobot() -> bool: + """ + Return whether OctoBot is running as a PyInstaller frozen binary. + + Uses PyInstaller's ``sys.frozen`` flag (not ``get_octobot_type()``), so ``python -m pytest`` + and other interpreter invocations are not misclassified as frozen binaries. + """ + return bool(getattr(sys, "frozen", False)) + + def get_os(): """ Return the OS name @@ -194,7 +210,7 @@ def tcp_port_is_free(bind_host: str, port: int) -> bool: return True -_HOST_WIDE_LISTENER_PROBE_HOSTS = ("0.0.0.0", "127.0.0.1") +_HOST_WIDE_LISTENER_PROBE_HOSTS = ("0.0.0.0", "127.0.0.1") # bind probes when psutil is unavailable def tcp_port_has_listener_on_host(port: int) -> bool: diff --git a/packages/commons/octobot_commons/process_util.py b/packages/commons/octobot_commons/process_util.py index d13f832911..834db51af5 100644 --- a/packages/commons/octobot_commons/process_util.py +++ b/packages/commons/octobot_commons/process_util.py @@ -218,6 +218,15 @@ async def wait_until_pid_stopped_async( ) +def rebind_managed_child_pid(spawn_pid: int, authoritative_pid: int) -> None: + """Replace spawn pid with authoritative app pid in the managed-child registry.""" + import octobot_commons.managed_child_process_registry as managed_child_process_registry + managed_child_process_registry.ManagedChildProcessRegistry.instance().rebind_managed_child_pid( + spawn_pid, + authoritative_pid, + ) + + async def graceful_stop_managed_children( *, timeout_seconds: float, diff --git a/packages/commons/tests/test_managed_child_process_registry.py b/packages/commons/tests/test_managed_child_process_registry.py index f6edd3faf6..2d5b7f2757 100644 --- a/packages/commons/tests/test_managed_child_process_registry.py +++ b/packages/commons/tests/test_managed_child_process_registry.py @@ -84,6 +84,75 @@ def test_unregister_unknown_pid_is_no_op(self): assert registry.snapshot_running_pids() == frozenset() +class TestRebindManagedChildPid: + def test_rebind_replaces_spawn_pid_with_authoritative_pid(self): + registry = managed_child_process_registry.ManagedChildProcessRegistry.instance() + with mock.patch.object(process_util, "pid_is_running", return_value=True): + registry.register(20520) + registry.rebind_managed_child_pid(20520, 25044) + assert registry.snapshot_running_pids() == frozenset({25044}) + + def test_rebind_no_op_for_non_positive_authoritative_pid(self): + registry = managed_child_process_registry.ManagedChildProcessRegistry.instance() + with mock.patch.object(process_util, "pid_is_running", return_value=True): + registry.register(101) + registry.rebind_managed_child_pid(101, 0) + assert registry.snapshot_running_pids() == frozenset({101}) + + def test_rebind_same_pid_is_idempotent(self): + registry = managed_child_process_registry.ManagedChildProcessRegistry.instance() + with mock.patch.object(process_util, "pid_is_running", return_value=True): + registry.register(505) + with mock.patch.object(registry, "register") as register_mock: + registry.rebind_managed_child_pid(505, 505) + register_mock.assert_not_called() + assert registry.snapshot_running_pids() == frozenset({505}) + + def test_second_rebind_to_same_authoritative_pid_is_no_op(self): + registry = managed_child_process_registry.ManagedChildProcessRegistry.instance() + with mock.patch.object(process_util, "pid_is_running", return_value=True): + registry.register(20520) + registry.rebind_managed_child_pid(20520, 25044) + with mock.patch.object(registry, "register") as register_mock: + registry.rebind_managed_child_pid(20520, 25044) + register_mock.assert_not_called() + assert registry.snapshot_running_pids() == frozenset({25044}) + + @pytest.mark.asyncio + async def test_graceful_stop_all_signals_authoritative_pid_after_rebind(self): + registry = managed_child_process_registry.ManagedChildProcessRegistry.instance() + stop_requested = False + + def pid_is_running_side_effect(pid): + if pid == 25044 and stop_requested: + return False + return True + + def sigterm_side_effect(pid, *, logger=None): + nonlocal stop_requested + stop_requested = True + return {"status": "stopped", "signal": "sigterm"} + + with ( + mock.patch.object( + managed_child_process_registry.process_util, + "pid_is_running", + side_effect=pid_is_running_side_effect, + ), + mock.patch.object( + managed_child_process_registry.process_util, + "request_graceful_stop_via_sigterm", + side_effect=sigterm_side_effect, + ) as sigterm_mock, + ): + registry.register(20520) + registry.rebind_managed_child_pid(20520, 25044) + outcomes = await registry.graceful_stop_all(timeout_seconds=0.1, poll_interval=0.01) + sigterm_mock.assert_called_once_with(25044, logger=registry._logger) + assert outcomes[25044] == "stopped" + assert 20520 not in outcomes + + class TestManagedChildProcessRegistrySnapshotRunningPids: def test_lazy_prunes_dead_pids(self): registry = managed_child_process_registry.ManagedChildProcessRegistry.instance() diff --git a/packages/commons/tests/test_os_util.py b/packages/commons/tests/test_os_util.py index 011cab57ee..378d22c5fd 100644 --- a/packages/commons/tests/test_os_util.py +++ b/packages/commons/tests/test_os_util.py @@ -15,6 +15,7 @@ # License along with this library. import mock import socket +import sys import pytest import octobot_commons.os_util as os_util @@ -30,6 +31,28 @@ def test_get_cpu_and_ram_usage(): assert unique_ram > 0 +class TestIsFrozenBinaryOctobot: + def test_returns_true_when_sys_frozen_is_set(self): + with mock.patch.object(sys, "frozen", True, create=True): + assert os_util.is_frozen_binary_octobot() is True + + def test_returns_false_for_interpreter_argv_without_sys_frozen(self): + with mock.patch.object(sys, "argv", [sys.executable]), mock.patch.object( + sys, "frozen", False, create=True + ): + assert os_util.is_frozen_binary_octobot() is False + + def test_returns_false_when_sys_frozen_absent(self): + original_frozen = getattr(sys, "frozen", None) + if hasattr(sys, "frozen"): + delattr(sys, "frozen") + try: + assert os_util.is_frozen_binary_octobot() is False + finally: + if original_frozen is not None: + sys.frozen = original_frozen + + class TestTcpPortIsFree: def test_returns_false_when_tcp_listener_holds_port(self): with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as listener: diff --git a/packages/tentacles/Meta/DSL_operators/octobot_process_operators/octobot_process_ops.py b/packages/tentacles/Meta/DSL_operators/octobot_process_operators/octobot_process_ops.py index 4c4e84cc60..dcdb4ae9cf 100644 --- a/packages/tentacles/Meta/DSL_operators/octobot_process_operators/octobot_process_ops.py +++ b/packages/tentacles/Meta/DSL_operators/octobot_process_operators/octobot_process_ops.py @@ -226,18 +226,17 @@ def _resolve_bound_pid( recall_state: octobot_process_state_import.OctobotProcessState, loaded_state: typing.Optional[process_bot_state_import.ProcessBotState], ) -> typing.Optional[int]: - """Bind operating PID from recall or fresh dump; None if metadata.pid dead (no raise).""" + """Bind operating PID from recall or fresh dump; None if no running pid (no raise).""" + if loaded_state is not None and _is_state_timestamp_fresh(loaded_state): + state_pid = loaded_state.metadata.pid + if state_pid <= 0: + raise commons_errors.DSLInterpreterError( + "process_bot_state.json is live but metadata.pid is missing or invalid." + ) + if process_util.pid_is_running(state_pid): + return state_pid if _stored_pid_is_running(recall_state): return recall_state.pid - if loaded_state is None or not _is_state_timestamp_fresh(loaded_state): - return None - state_pid = loaded_state.metadata.pid - if state_pid <= 0: - raise commons_errors.DSLInterpreterError( - "process_bot_state.json is live but metadata.pid is missing or invalid." - ) - if process_util.pid_is_running(state_pid): - return state_pid return None @@ -770,20 +769,32 @@ def _ensure_child_environ( child_env[commons_constants.ENV_OCTOBOT_SYNC_DATA_ROOT] = os.path.normpath( os.path.join(working_directory, commons_constants.USER_FOLDER) ) + if os_util.is_frozen_binary_octobot(): + child_env.update(os_util.PYINSTALLER_RESET_ENVIRONMENT_VARS) return child_env +def _octobot_spawn_argv_prefix(working_directory: str) -> list[str]: + if os_util.is_frozen_binary_octobot(): + return [sys.executable] + start_script = os.path.join(working_directory, "start.py") + if not os.path.isfile(start_script): + raise commons_errors.DSLInterpreterError( + f"start.py not found at {start_script} (current working directory must be the OctoBot project root)." + ) + return [sys.executable, start_script] + + def _ensure_start_cmd( - start_script: str, + argv_prefix: list[str], rel_user: str, rel_log: str, no_telegram: bool, state_file_path: str, ) -> list[str]: - """Argv for `python start.py --user-folder … --log-folder … --standalone` (+ optional -nt, --dump-state).""" + """Argv for OctoBot child: prefix + --user-folder … --standalone (+ optional -nt, --dump-state).""" cmd: list[str] = [ - sys.executable, - start_script, + *argv_prefix, "--user-folder", rel_user, "--log-folder", @@ -1137,7 +1148,13 @@ async def _pre_compute_recall_path( ): resolved_pid = _resolve_bound_pid(recall_state, loaded_state) if resolved_pid is not None: - self.pid = resolved_pid + if resolved_pid != recall_state.pid: + self.bind_authoritative_child_pid( + resolved_pid, + spawn_pid=recall_state.pid, + ) + else: + self.pid = resolved_pid _release_recall_state_listen_ports(recall_state) self.value = self.request_graceful_stop(logger=_get_logger()) raise commons_errors.DSLInterpreterError( @@ -1163,7 +1180,12 @@ async def _pre_compute_recall_path( resolved_pid, recall_state.pid, ) - self.pid = resolved_pid + self.bind_authoritative_child_pid( + resolved_pid, + spawn_pid=recall_state.pid, + ) + else: + self.pid = resolved_pid recall_state = _apply_resolved_pid_to_state(recall_state, resolved_pid) _get_logger().info("process state path (re-call path): %s", state_path) # Running: stored recall pid or child-confirmed-alive → init_state_ok, optional EAE. @@ -1242,11 +1264,7 @@ async def _pre_compute_first_spawn( web_port, node_port = _allocate_child_listen_port_pair( probe_host, web_b, node_b, user_folder ) - start_script = os.path.join(working_directory, "start.py") - if not os.path.isfile(start_script): - raise commons_errors.DSLInterpreterError( - f"start.py not found at {start_script} (current working directory must be the OctoBot project root)." - ) + argv_prefix = _octobot_spawn_argv_prefix(working_directory) process_sync_user_id = params.get("user_id") if not process_sync_user_id or not str(process_sync_user_id).strip(): raise commons_errors.DSLInterpreterError( @@ -1265,7 +1283,7 @@ async def _pre_compute_first_spawn( os.path.join(user_root, octobot_constants.PROCESS_BOT_STATE_FILE_NAME) ) cmd = _ensure_start_cmd( - start_script, + argv_prefix, rel_user, rel_log, bool(params.get("no_telegram", True)), @@ -1279,6 +1297,7 @@ async def _pre_compute_first_spawn( environment=child_env, hide_console_window=True, ) + spawn_pid = self.pid or 0 scheme = str(params.get("http_scheme") or "http").rstrip(":/") http_base_url = f"{scheme}://{bind_host}:{web_port}" state = octobot_process_state_import.OctobotProcessState( @@ -1304,7 +1323,10 @@ async def _pre_compute_first_spawn( "process_bot_state.json is live but metadata.pid is missing or invalid." ) if process_util.pid_is_running(state_pid): - self.pid = state_pid + if state_pid != spawn_pid: + self.bind_authoritative_child_pid(state_pid, spawn_pid=spawn_pid) + else: + self.pid = state_pid state = state.model_copy(update={"pid": state_pid}) _get_logger().info( "OctoBot is running (first-spawn path): user_folder=%r base_url=%r pid=%s", diff --git a/packages/tentacles/Meta/DSL_operators/octobot_process_operators/tests/test_octobot_process_ops.py b/packages/tentacles/Meta/DSL_operators/octobot_process_operators/tests/test_octobot_process_ops.py index 10aa7cfece..d5cd43ba68 100644 --- a/packages/tentacles/Meta/DSL_operators/octobot_process_operators/tests/test_octobot_process_ops.py +++ b/packages/tentacles/Meta/DSL_operators/octobot_process_operators/tests/test_octobot_process_ops.py @@ -1693,11 +1693,72 @@ def test_sets_process_bot_sync_user_id_env(self, tmp_path): assert child_env["DISTRIBUTION"] == commons_constants.DEFAULT_DISTRIBUTION assert child_env[services_constants.ENV_ENABLE_NODE_API] == "false" + def test_sets_pyinstaller_reset_environment_in_binary_mode(self, tmp_path): + with mock.patch.object( + octobot_process_ops.os_util, + "is_frozen_binary_octobot", + return_value=True, + ): + child_env = octobot_process_ops._ensure_child_environ( + 20050, + 30050, + "127.0.0.1", + "wallet-user", + str(tmp_path), + ) + assert child_env["PYINSTALLER_RESET_ENVIRONMENT"] == "1" + + def test_omits_pyinstaller_reset_environment_in_python_mode(self, tmp_path): + with mock.patch.object( + octobot_process_ops.os_util, + "is_frozen_binary_octobot", + return_value=False, + ): + child_env = octobot_process_ops._ensure_child_environ( + 20050, + 30050, + "127.0.0.1", + "wallet-user", + str(tmp_path), + ) + assert "PYINSTALLER_RESET_ENVIRONMENT" not in child_env + + +class TestOctobotSpawnArgvPrefix: + def test_binary_mode_returns_executable_only(self): + with mock.patch.object( + octobot_process_ops.os_util, + "is_frozen_binary_octobot", + return_value=True, + ): + argv_prefix = octobot_process_ops._octobot_spawn_argv_prefix("/any/cwd") + assert argv_prefix == [sys.executable] + + def test_python_mode_requires_start_py(self, tmp_path): + with mock.patch.object( + octobot_process_ops.os_util, + "is_frozen_binary_octobot", + return_value=False, + ): + with pytest.raises(commons_errors.DSLInterpreterError, match="start.py not found"): + octobot_process_ops._octobot_spawn_argv_prefix(str(tmp_path)) + + def test_python_mode_includes_start_script(self, tmp_path): + start_script = tmp_path / "start.py" + start_script.write_text("#", encoding="utf-8") + with mock.patch.object( + octobot_process_ops.os_util, + "is_frozen_binary_octobot", + return_value=False, + ): + argv_prefix = octobot_process_ops._octobot_spawn_argv_prefix(str(tmp_path)) + assert argv_prefix == [sys.executable, str(start_script)] + class TestEnsureStartCmd: def test_includes_standalone_flag(self): cmd = octobot_process_ops._ensure_start_cmd( - "start.py", + [sys.executable, "start.py"], "user/automations/bot-1", "logs/automations/bot-1", no_telegram=False, @@ -1706,6 +1767,18 @@ def test_includes_standalone_flag(self): assert "--standalone" in cmd assert cmd.index("--standalone") < cmd.index("--dump-state") + def test_binary_prefix_omits_start_script(self): + cmd = octobot_process_ops._ensure_start_cmd( + [sys.executable], + "user/automations/bot-1", + "logs/automations/bot-1", + no_telegram=True, + state_file_path="/tmp/process_bot_state.json", + ) + assert cmd[0] == sys.executable + assert "start.py" not in cmd + assert cmd[1:3] == ["--user-folder", "user/automations/bot-1"] + class TestEnsureOctobotProcessOperatorPrecompute: async def test_raises_when_user_id_missing_at_spawn(self, tmp_path): @@ -1740,6 +1813,53 @@ async def test_raises_when_user_id_missing_at_spawn(self, tmp_path): with pytest.raises(commons_errors.DSLInterpreterError, match="requires user_id"): await op.pre_compute() + async def test_spawns_child_in_binary_mode_without_start_py(self, tmp_path): + op = EnsureOctobotProcessOperator( + user_folder="ub", + user_id=_PROCESS_TEST_USER_ID, + profile_data=_MINIMAL_PROFILE_DATA, + last_execution_result=None, + ) + with mock.patch.object( + octobot_process_ops.os_util, + "is_frozen_binary_octobot", + return_value=True, + ), mock.patch.object( + octobot_process_ops.os, + "getcwd", + return_value=str(tmp_path), + ), mock.patch.object( + octobot_process_ops, + "ensure_user_profile_and_layout", + new=mock.AsyncMock( + return_value={ + "user_root": str( + tmp_path / commons_constants.USER_FOLDER / commons_constants.AUTOMATIONS_FOLDER / "ub" + ), + "profile_id": "x", + "already_prepared": True, + } + ), + ), mock.patch.object( + octobot_process_ops, + "_listen_port_pair_with_shared_scan_offset", + return_value=(20050, 30050), + ), mock.patch.object( + process_util, + "spawn_managed_subprocess", + ) as spawn_mock, mock.patch.object( + octobot_process_ops, + "_load_process_bot_state", + new=mock.AsyncMock(side_effect=_async_return_none_mock), + ): + spawn_mock.return_value.pid = 99999 + await op.pre_compute() + spawn_argv = spawn_mock.call_args.args[0] + assert spawn_argv[0] == sys.executable + assert "start.py" not in spawn_argv + assert spawn_argv[1] == "--user-folder" + assert spawn_mock.call_args.kwargs["environment"]["PYINSTALLER_RESET_ENVIRONMENT"] == "1" + async def test_returns_recallable_when_process_bot_state_not_live(self, tmp_path): start_script = tmp_path / "start.py" start_script.write_text("#", encoding="utf-8") @@ -2108,6 +2228,10 @@ async def test_run_octobot_process_via_dsl(self, tmp_path, monkeypatch): ) try: with mock.patch.object( + octobot_process_ops.os_util, + "is_frozen_binary_octobot", + return_value=False, + ), mock.patch.object( octobot_process_ops, "_load_process_bot_state", new=mock.AsyncMock(side_effect=_async_return_none_mock), @@ -3392,3 +3516,173 @@ async def test_missing_executor_id_falls_through_to_first_spawn(self, tmp_path): spawn_mock.return_value = mock.Mock(spec=["pid"], pid=30007) await op.pre_compute() spawn_mock.assert_called_once() + + +class TestResolveBoundPid: + pytestmark = [] + + def test_prefers_metadata_pid_when_both_running_but_differ(self): + recall_state = octobot_process_state_import.OctobotProcessState( + http_base_url="http://127.0.0.1:20050", + web_port=20050, + node_port=30050, + user_root="/x/ub", + user_folder="ub", + log_folder="/x/logs/ub", + profile_id="p", + pid=20520, + state_file_path="/x/ub/process_bot_state.json", + executor_id=TEST_EXECUTOR_ID, + ) + loaded_state = process_bot_state_import.ProcessBotState( + metadata=process_bot_state_import.Metadata( + updated_at=octobot_process_ops.time.time() - 0.1, + next_updated_at=octobot_process_ops.time.time() + 1.0, + pid=25044, + ), + exchange_account_elements=octobot_flow_entities.ExchangeAccountElements(), + ) + with mock.patch.object( + process_util, + "pid_is_running", + side_effect=lambda process_id: process_id in {20520, 25044}, + ): + resolved_pid = octobot_process_ops._resolve_bound_pid(recall_state, loaded_state) + assert resolved_pid == 25044 + + def test_returns_stored_pid_when_metadata_missing(self): + recall_state = octobot_process_state_import.OctobotProcessState( + http_base_url="http://127.0.0.1:20050", + web_port=20050, + node_port=30050, + user_root="/x/ub", + user_folder="ub", + log_folder="/x/logs/ub", + profile_id="p", + pid=10002, + state_file_path="/x/ub/process_bot_state.json", + executor_id=TEST_EXECUTOR_ID, + ) + with mock.patch.object( + process_util, + "pid_is_running", + side_effect=lambda process_id: process_id == 10002, + ): + resolved_pid = octobot_process_ops._resolve_bound_pid(recall_state, None) + assert resolved_pid == 10002 + + def test_returns_none_when_both_dead(self): + recall_state = octobot_process_state_import.OctobotProcessState( + http_base_url="http://127.0.0.1:20050", + web_port=20050, + node_port=30050, + user_root="/x/ub", + user_folder="ub", + log_folder="/x/logs/ub", + profile_id="p", + pid=10002, + state_file_path="/x/ub/process_bot_state.json", + executor_id=TEST_EXECUTOR_ID, + ) + loaded_state = process_bot_state_import.ProcessBotState( + metadata=process_bot_state_import.Metadata( + updated_at=octobot_process_ops.time.time() - 0.1, + next_updated_at=octobot_process_ops.time.time() + 1.0, + pid=20002, + ), + exchange_account_elements=octobot_flow_entities.ExchangeAccountElements(), + ) + with mock.patch.object(process_util, "pid_is_running", return_value=False): + resolved_pid = octobot_process_ops._resolve_bound_pid(recall_state, loaded_state) + assert resolved_pid is None + + +class TestRecallPathRebindsManagedChildRegistry: + @pytest.fixture(autouse=True) + def reset_managed_child_process_registry(self): + import octobot_commons.managed_child_process_registry as managed_child_process_registry + import octobot_commons.singleton.singleton_class as singleton_class + + singleton_class.Singleton._instances.pop( + managed_child_process_registry.ManagedChildProcessRegistry, + None, + ) + yield + singleton_class.Singleton._instances.pop( + managed_child_process_registry.ManagedChildProcessRegistry, + None, + ) + + async def test_recall_rebinds_registry_when_metadata_pid_differs(self, tmp_path): + import octobot_commons.managed_child_process_registry as managed_child_process_registry + + registry = managed_child_process_registry.ManagedChildProcessRegistry.instance() + inner = _healthy_recall_inner(pid=20520, tmp_path=tmp_path) + op = EnsureOctobotProcessOperator( + user_folder="ub", + user_id=_PROCESS_TEST_USER_ID, + profile_data=_MINIMAL_PROFILE_DATA, + last_execution_result=_re_calling_ensure_value(inner), + ) + + async def live_state_with_authoritative_pid(*_unused): + return await _async_live_process_bot_state_mock(metadata_pid=25044) + + with mock.patch.object( + octobot_process_ops.os, + "getcwd", + return_value=str(tmp_path), + ), mock.patch.object( + process_util, + "pid_is_running", + side_effect=lambda process_id: process_id in {20520, 25044}, + ), mock.patch.object( + process_util, + "spawn_managed_subprocess", + ) as spawn_mock, mock.patch.object( + octobot_process_ops, + "_load_process_bot_state", + new=mock.AsyncMock(side_effect=live_state_with_authoritative_pid), + ): + registry.register(20520) + await op.pre_compute() + spawn_mock.assert_not_called() + with mock.patch.object(process_util, "pid_is_running", return_value=True): + assert registry.snapshot_running_pids() == frozenset({25044}) + assert op.pid == 25044 + + async def test_recall_does_not_rebind_when_authoritative_pid_unchanged(self, tmp_path): + inner = _healthy_recall_inner(pid=25044, tmp_path=tmp_path) + op = EnsureOctobotProcessOperator( + user_folder="ub", + user_id=_PROCESS_TEST_USER_ID, + profile_data=_MINIMAL_PROFILE_DATA, + last_execution_result=_re_calling_ensure_value(inner), + ) + + async def live_state_with_same_pid(*_unused): + return await _async_live_process_bot_state_mock(metadata_pid=25044) + + with mock.patch.object( + octobot_process_ops.os, + "getcwd", + return_value=str(tmp_path), + ), mock.patch.object( + process_util, + "pid_is_running", + side_effect=lambda process_id: process_id == 25044, + ), mock.patch.object( + process_util, + "spawn_managed_subprocess", + ) as spawn_mock, mock.patch.object( + octobot_process_ops, + "_load_process_bot_state", + new=mock.AsyncMock(side_effect=live_state_with_same_pid), + ), mock.patch.object( + process_util, + "rebind_managed_child_pid", + ) as rebind_mock: + await op.pre_compute() + spawn_mock.assert_not_called() + rebind_mock.assert_not_called() + assert op.pid == 25044 diff --git a/tests/unit_tests/test_commands.py b/tests/unit_tests/test_commands.py index a529e1d862..24343838f6 100644 --- a/tests/unit_tests/test_commands.py +++ b/tests/unit_tests/test_commands.py @@ -279,3 +279,45 @@ async def test_update_or_repair_damaged_respects_install_default_true(self): community_auth, tentacles_setup_config, config ) mock_install.assert_awaited_once_with(config, [], False) + + +class TestRestartBot: + pytestmark = [] + + def test_binary_restart_preserves_argv(self, monkeypatch): + executable = r"C:\path\OctoBot.exe" + argv = [ + executable, + "--user-folder", r"user\automations\abc", + "--standalone", + "-nt", + "--update", + ] + monkeypatch.setattr(commands.sys, "argv", argv) + monkeypatch.setattr(commands, "get_bot_file", lambda: executable) + + popen_calls = [] + + def mock_popen(command, env): + popen_calls.append((command, env)) + return mock.Mock() + + monkeypatch.setattr(commands.subprocess, "Popen", mock_popen) + + def mock_exit(exit_code): + raise SystemExit(exit_code) + + monkeypatch.setattr(commands.os, "_exit", mock_exit) + + with pytest.raises(SystemExit): + commands.restart_bot() + + assert len(popen_calls) == 1 + command, env = popen_calls[0] + assert command == [ + executable, + "--user-folder", r"user\automations\abc", + "--standalone", + "-nt", + ] + assert env["PYINSTALLER_RESET_ENVIRONMENT"] == "1"