From 5773e8aa27c4ef72b943ce2c268da67a6890c201 Mon Sep 17 00:00:00 2001 From: Junyan Qin Date: Tue, 19 May 2026 22:38:17 +0800 Subject: [PATCH] refactor(box): use unified env-override mechanism for box.local config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The box module hand-rolled its own LANGBOT_BOX_LOCAL_* env parsing in two places (connector._get_box_config and service._local_config), duplicating logic that LoadConfigStage._apply_env_overrides_to_config already provides generically via the SECTION__SUBSECTION__KEY convention. - Drop the bespoke LANGBOT_BOX_LOCAL_* parsing; read box.local straight from instance_config (the unified BOX__LOCAL__* overrides are already applied before BoxService initializes) - Harden _load_allowed_mount_roots to accept a comma-separated string, since the generic mechanism stores a freshly-created key as a raw string when config.yaml has no box.local.allowed_mount_roots entry - docker-compose: rename the langbot container env vars to BOX__LOCAL__* (the canonical convention); remove them entirely from the langbot_box container — the Box runtime never reads box.local from env/config.yaml, it is configured via the INIT RPC action Co-Authored-By: Claude Opus 4.7 (1M context) --- docker/docker-compose.yaml | 18 ++++++++++-------- src/langbot/pkg/box/connector.py | 27 +++++++++------------------ src/langbot/pkg/box/service.py | 29 ++++++++++++++--------------- 3 files changed, 33 insertions(+), 41 deletions(-) diff --git a/docker/docker-compose.yaml b/docker/docker-compose.yaml index c816dec3..5791e24c 100644 --- a/docker/docker-compose.yaml +++ b/docker/docker-compose.yaml @@ -33,10 +33,9 @@ services: restart: on-failure environment: - TZ=Asia/Shanghai - - LANGBOT_BOX_LOCAL_HOST_ROOT=${LANGBOT_BOX_ROOT:-${PWD}/data/box} - - LANGBOT_BOX_LOCAL_DEFAULT_WORKSPACE=default - - LANGBOT_BOX_LOCAL_SKILLS_ROOT=skills - - LANGBOT_BOX_LOCAL_ALLOWED_MOUNT_ROOTS=${LANGBOT_BOX_ROOT:-${PWD}/data/box} + # The Box runtime does NOT read box.local.* from config.yaml or env; it + # receives its configuration from LangBot via the INIT RPC action. + # Do not add LANGBOT_BOX_* / BOX__* here — they would be silently ignored. command: ["uv", "run", "--no-sync", "-m", "langbot_plugin.box", "--mode", "ws"] networks: - langbot_network @@ -49,10 +48,13 @@ services: restart: on-failure environment: - TZ=Asia/Shanghai - - LANGBOT_BOX_LOCAL_HOST_ROOT=${LANGBOT_BOX_ROOT:-${PWD}/data/box} - - LANGBOT_BOX_LOCAL_DEFAULT_WORKSPACE=default - - LANGBOT_BOX_LOCAL_SKILLS_ROOT=skills - - LANGBOT_BOX_LOCAL_ALLOWED_MOUNT_ROOTS=${LANGBOT_BOX_ROOT:-${PWD}/data/box} + # Unified env-override convention: SECTION__SUBSECTION__KEY overrides the + # matching config.yaml field (see LoadConfigStage). These map onto + # box.local.* and are forwarded to the Box runtime via INIT RPC. + - BOX__LOCAL__HOST_ROOT=${LANGBOT_BOX_ROOT:-${PWD}/data/box} + - BOX__LOCAL__DEFAULT_WORKSPACE=default + - BOX__LOCAL__SKILLS_ROOT=skills + - BOX__LOCAL__ALLOWED_MOUNT_ROOTS=${LANGBOT_BOX_ROOT:-${PWD}/data/box} ports: - 5300:5300 # For web ui and webhook callback - 2280-2285:2280-2285 # For platform reverse connection diff --git a/src/langbot/pkg/box/connector.py b/src/langbot/pkg/box/connector.py index 8ea8d387..61c269ea 100644 --- a/src/langbot/pkg/box/connector.py +++ b/src/langbot/pkg/box/connector.py @@ -35,26 +35,17 @@ _INTERNAL_BOX_CONFIG_KEYS = frozenset({'runtime'}) def _get_box_config(ap) -> dict: - """Return the 'box' section from instance config, with safe fallbacks.""" + """Return the 'box' section from instance config. + + Environment-variable overrides are handled uniformly by + ``LoadConfigStage._apply_env_overrides_to_config`` using the + ``SECTION__SUBSECTION__KEY`` convention (e.g. ``BOX__LOCAL__HOST_ROOT``, + ``BOX__LOCAL__ALLOWED_MOUNT_ROOTS="/a,/b"``) before this is read, so no + box-specific env parsing is needed here. + """ instance_config = getattr(ap, 'instance_config', None) config_data = getattr(instance_config, 'data', {}) if instance_config is not None else {} - box_config = dict(config_data.get('box', {}) or {}) - local_config = dict(box_config.get('local') or {}) - env_overrides = { - 'host_root': os.getenv('LANGBOT_BOX_LOCAL_HOST_ROOT', ''), - 'default_workspace': os.getenv('LANGBOT_BOX_LOCAL_DEFAULT_WORKSPACE', ''), - 'skills_root': os.getenv('LANGBOT_BOX_LOCAL_SKILLS_ROOT', ''), - } - for key, value in env_overrides.items(): - if value: - local_config[key] = value - - allowed_mount_roots = os.getenv('LANGBOT_BOX_LOCAL_ALLOWED_MOUNT_ROOTS', '') - if allowed_mount_roots: - local_config['allowed_mount_roots'] = [item.strip() for item in allowed_mount_roots.split(',') if item.strip()] - if local_config: - box_config['local'] = local_config - return box_config + return dict(config_data.get('box', {}) or {}) def _get_runtime_endpoint(box_cfg: dict) -> str: diff --git a/src/langbot/pkg/box/service.py b/src/langbot/pkg/box/service.py index e4fad7b2..4b3d6a1a 100644 --- a/src/langbot/pkg/box/service.py +++ b/src/langbot/pkg/box/service.py @@ -452,25 +452,24 @@ class BoxService: } def _local_config(self) -> dict: - local_config = dict(_get_box_config(self.ap).get('local') or {}) - env_overrides = { - 'host_root': os.getenv('LANGBOT_BOX_LOCAL_HOST_ROOT', ''), - 'default_workspace': os.getenv('LANGBOT_BOX_LOCAL_DEFAULT_WORKSPACE', ''), - 'skills_root': os.getenv('LANGBOT_BOX_LOCAL_SKILLS_ROOT', ''), - } - for key, value in env_overrides.items(): - if value: - local_config[key] = value + """Return ``box.local`` from instance config. - allowed_mount_roots = os.getenv('LANGBOT_BOX_LOCAL_ALLOWED_MOUNT_ROOTS', '') - if allowed_mount_roots: - local_config['allowed_mount_roots'] = [ - item.strip() for item in allowed_mount_roots.split(',') if item.strip() - ] - return local_config + Environment overrides are applied uniformly by + ``LoadConfigStage._apply_env_overrides_to_config`` (e.g. + ``BOX__LOCAL__HOST_ROOT``) before this is read, so no box-specific + env parsing happens here. + """ + return dict(_get_box_config(self.ap).get('local') or {}) def _load_allowed_mount_roots(self) -> list[str]: configured_roots = self._local_config().get('allowed_mount_roots', []) + # The unified env-override mechanism stores a brand-new key as a raw + # string when the key is absent from config.yaml. Accept a + # comma-separated string as well as a list so that + # ``BOX__LOCAL__ALLOWED_MOUNT_ROOTS="/a,/b"`` keeps working even when + # the config file has no ``box.local.allowed_mount_roots`` entry. + if isinstance(configured_roots, str): + configured_roots = [item.strip() for item in configured_roots.split(',') if item.strip()] normalized_roots: list[str] = [] for root in configured_roots: