diff --git a/src/agent_env/cli/env/state/init.py b/src/agent_env/cli/env/state/init.py index 5a67937bf..ab0be6c0d 100644 --- a/src/agent_env/cli/env/state/init.py +++ b/src/agent_env/cli/env/state/init.py @@ -27,17 +27,12 @@ def _resolve_environment_names(env: Env) -> list[str]: standalone servicedb-backed env contributes its single one. Must match the list the gateway passes to ``prepare`` at deploy (``EnvironmentGatewayProvider._all_environments``, - the authoritative one) — including the ``website_browser`` server the gateway auto-adds whenever an - env has websites. A name missing here yields a base without that schema, and the deploy then builds - it a silently-empty overlay.""" + the authoritative one), which leaves out the website browser the gateway adds. A name missing here yields a + base without that schema, and the deploy then builds it a silently-empty overlay.""" if isinstance(env, MultiEnv): names = [e.environment_name for e in env.mcp_server_envs] + [ e.environment_name for e in env.website_envs ] - if env.website_envs: - from agent_env.config import get_config - - names.append(Env.get(get_config().default_website_browser_env_id).environment_name) elif hasattr(env, "environment_name"): names = [env.environment_name] else: diff --git a/src/agent_env/providers/env_providers/env_gateway_provider.py b/src/agent_env/providers/env_providers/env_gateway_provider.py index 69e0e74c5..e1cd8beb2 100644 --- a/src/agent_env/providers/env_providers/env_gateway_provider.py +++ b/src/agent_env/providers/env_providers/env_gateway_provider.py @@ -84,6 +84,11 @@ _TRAJECTORY_READ_ATTEMPTS = 3 +# The network the env's services share, and the only one its database is on. +_ENV_NETWORK = "env-network" +# The network the website browser shares with the gateway and the sites, and with nothing else in the env. +_SITES_NETWORK = "website-network" + # How much of a service's output a failed deploy's error carries: enough for the end of a traceback. _LOG_TAIL_LINES = 30 _LOG_LINE_CHARS = 300 @@ -108,6 +113,12 @@ class MCPServerConfig: image: str environment_name: str extra_env_vars: dict[str, str] | None = None + networks: tuple[str, ...] = (_ENV_NETWORK,) + + @property + def uses_database(self) -> bool: + """Only the env's network reaches its database, so a server off it gets no schema and no DATABASE_URL.""" + return _ENV_NETWORK in self.networks @dataclass @@ -266,11 +277,11 @@ async def store_exec(argv: list[str]) -> tuple[int, str, str]: logger.warning(f"Failed to install changelog triggers for schema '{environment_name}': {type(e).__name__}: {e}") def _all_environments(self, mcp_servers, website_configs) -> list[str]: - """The DB-client services (one schema each) for a deploy: MCP servers + website backends, - deduped. This is the authoritative list — it reflects any servers the gateway auto-adds (e.g. - website_browser) — and drives both the rendered compose and the state provider's ``prepare``.""" + """The DB-client services (one schema each) for a deploy: the MCP servers that use the database and the website + backends, deduped. This is the authoritative list: it drives the rendered compose, the local store's schemas + and the state provider's ``prepare``.""" return list(dict.fromkeys( - [s.environment_name for s in mcp_servers] + [s.environment_name for s in mcp_servers if s.uses_database] + [wc.environment_name for wc in (website_configs or [])] )) @@ -295,7 +306,9 @@ def create_docker_compose( """Generate docker-compose.yml content for a gateway deployment onto a VM/laptop/arbitrary machine. Supports MCP servers, websites, or both. Website containers (backend + frontend) are added - to the same docker network. The gateway receives WEBSITE_URLS env var for discovery. + to the same docker network. The gateway receives WEBSITE_URLS env var for discovery. The website + browser sits on a network of its own with the gateway and each site's frontend (its backend when it + has none), so it reaches no backing server and no database, and it gets no schema or DATABASE_URL. Args: mcp_servers: List of MCP server configurations for internal servers. @@ -408,17 +421,17 @@ def create_docker_compose( # Add internal server services website_configs = website_configs or [] + sites_network = [_SITES_NETWORK] if any(_SITES_NETWORK in s.networks for s in mcp_servers) else [] website_backend_servers = { wc.environment_name: f"{wc.environment_name}-{AGENT_ENV_WEBSITE_BACKEND_SUFFIX}" for wc in website_configs } for server in mcp_servers: - db_url = state_provider.url_for_environment(server.environment_name, instance=state_instance) lines.extend([ f" {server.environment_name}:", f" image: {server.image}", ]) - dep_lines = list(store_dep_lines) + dep_lines = list(store_dep_lines) if server.uses_database else [] if server.environment_name in website_backend_servers: dep_lines += [ f" {website_backend_servers[server.environment_name]}:", @@ -437,8 +450,10 @@ def create_docker_compose( # only for images that read it themselves, and a later pass drops it. f" - SERVICE_NAME={server.environment_name}", f" - ENVIRONMENT_NAME={server.environment_name}", - f" - DATABASE_URL={db_url}", ]) + if server.uses_database: + db_url = state_provider.url_for_environment(server.environment_name, instance=state_instance) + lines.append(f" - DATABASE_URL={db_url}") if server.extra_env_vars: for key, value in server.extra_env_vars.items(): lines.append(f" - {key}={value}") @@ -449,8 +464,7 @@ def create_docker_compose( " timeout: 5s", f" retries: {MCP_HC_RETRIES}", f" start_period: {MCP_HC_START_PERIOD}", - " networks:", - " - env-network", + *_network_lines(*server.networks), "", ]) @@ -521,8 +535,7 @@ def create_docker_compose( " timeout: 5s", " retries: 10", " start_period: 15s", - " networks:", - " - env-network", + *_network_lines(_ENV_NETWORK, *sites_network), "", ]) @@ -553,8 +566,7 @@ def create_docker_compose( " timeout: 5s", " start_period: 30s", " retries: 3", - " networks:", - " - env-network", + *_network_lines(_ENV_NETWORK, *([] if wc.frontend_image else sites_network)), "", ]) if not wc.frontend_image: @@ -565,8 +577,7 @@ def create_docker_compose( " depends_on:", f" {backend_name}:", " condition: service_healthy", - " networks:", - " - env-network", + *_network_lines(_ENV_NETWORK, *sites_network), "", ]) @@ -585,16 +596,14 @@ def create_docker_compose( " ports:", *(f' - "{spec}"' for spec in port_bindings(host_ips, publish(sc.host_port), sc.container_port)), f" restart: {sc.restart}", - " networks:", - " - env-network", + *_network_lines(_ENV_NETWORK), "", ]) # Add networks lines.extend([ "networks:", - " env-network:", - " driver: bridge", + *(line for network in (_ENV_NETWORK, *sites_network) for line in (f" {network}:", " driver: bridge")), ]) return "\n".join(lines) @@ -773,6 +782,7 @@ async def _deploy_via_vm( browser_server = MCPServerConfig( image=website_browser_env.docker_image_artifact.image_name, environment_name=website_browser_env.environment_name, + networks=(_SITES_NETWORK,), ) mcp_servers = list(mcp_servers) + [browser_server] mcp_server_images = list(mcp_server_images) + [website_browser_env.docker_image_artifact] @@ -825,9 +835,9 @@ async def _stand_up(_spec): ) state_instance = self._state_instance - # Stage 2 (prepare): create per-service schemas now that the final service list is known — - # it includes the auto-added website_browser above. Local no-ops (its servicedb container - # runs the init script declaratively); remote/external stores create schemas here. + # Stage 2 (prepare): create per-service schemas now that the final service list is known. The website browser + # added above takes none. Local no-ops (its servicedb container runs the init script declaratively); + # remote/external stores create schemas here. if self._state_provider is not None: await self._state_provider.prepare( self._all_environments(mcp_servers, website_configs), instance=state_instance @@ -1309,10 +1319,7 @@ async def _build_local_store( LocalPostgresStateProvider, ) - # MCP servers and websites are both DB clients (one schema each); the container path - # passes no websites, so normalize to a list once. - configs = [*mcp_servers, *(website_configs or [])] - environment_names = list(dict.fromkeys(c.environment_name for c in configs)) + environment_names = self._all_environments(mcp_servers, website_configs) provider = self._state_provider # set by _deploy_gateway assert isinstance(provider, LocalPostgresStateProvider) spec = provider.store_spec(environment_names) @@ -1639,6 +1646,10 @@ def _why_down(row: dict) -> str | None: return None +def _network_lines(*networks: str) -> list[str]: + return [" networks:", *(f" - {network}" for network in networks)] + + def _redact_compose_secrets(compose_content: str) -> str: """Mask secret-valued env lines so the logged compose never carries a live secret.""" return "\n".join( diff --git a/tst/integration/env/gateway/gateway_test.py b/tst/integration/env/gateway/gateway_test.py index 435a6136c..a84d8da97 100644 --- a/tst/integration/env/gateway/gateway_test.py +++ b/tst/integration/env/gateway/gateway_test.py @@ -1583,6 +1583,12 @@ async def assert_all_tools_and_website(session, tools_result, tool_names): logger.info(f"browser_navigate result: {content_text[:500]}") assert "Slack Workspace" in content_text, f"Expected 'Slack Workspace' in navigate result, got {content_text}" + for url in ("http://slack/", "http://email/", "http://slack-website-backend:8000/api/health", + "http://servicedb:5432/", "http://pgweb:8081/", "http://db-mcp:8000/mcp"): + result = await session.call_tool("browser_navigate", {"url": url}) + content_text = "".join(c.text for c in result.content if hasattr(c, "text")) + assert "ERR_NAME_NOT_RESOLVED" in content_text, f"The browser reached {url}: {content_text[:300]}" + await session.call_tool("browser_close", {}) async def assert_loaded_data(session, tools_result, tool_names): diff --git a/tst/unit/cli/init_env_state_cli_test.py b/tst/unit/cli/init_env_state_cli_test.py index a7e846fca..f364317e6 100644 --- a/tst/unit/cli/init_env_state_cli_test.py +++ b/tst/unit/cli/init_env_state_cli_test.py @@ -120,30 +120,19 @@ def test_persistent_backend_materializes_base_via_acquire_then_prepare(): assert "esi-base9" in res.output -def test_website_env_includes_the_gateway_auto_added_browser_environment(): - """An env with websites gets a ``website_browser`` MCP server auto-added by the gateway, and the - gateway passes it to ``prepare`` at deploy. The pre-initialized store must carry that schema too — - otherwise the deploy builds an overlay over a base schema that was never created.""" - browser = MagicMock() - browser.environment_name = "website_browser" - - def _get(env_id, *a, **kw): - return browser if env_id == "website-browser-env" else _fake_multi_env( - mcp=("slack",), websites=("shop",) - ) - +def test_website_env_has_no_schema_for_the_browser_the_gateway_adds(): + """The gateway adds a website browser to an env with websites but gives it no schema at deploy, so the + pre-initialized store carries the env's own MCP servers and websites alone, as the deploy's ``prepare`` does.""" provider = MagicMock() provider.prepare = AsyncMock() - cfg = MagicMock(default_website_browser_env_id="website-browser-env") - with patch.object(Env, "get", side_effect=_get), \ + with patch.object(Env, "get", return_value=_fake_multi_env(mcp=("slack",), websites=("shop",))), \ patch("agent_env.providers.env_state.acquire_state_for_deploy", AsyncMock(return_value=_fake_instance())), \ - patch("agent_env.providers.env_state.build_state_provider", return_value=provider), \ - patch("agent_env.config.get_config", return_value=cfg): + patch("agent_env.providers.env_state.build_state_provider", return_value=provider): res = CliRunner().invoke( init_env_state, ["--id", "multi-shop", "--env-state-type", "remote_postgres"] ) assert res.exit_code == 0, res.output - assert provider.prepare.await_args.args[0] == ["slack", "shop", "website_browser"] + assert provider.prepare.await_args.args[0] == ["slack", "shop"] def test_environment_names_are_deduped(): diff --git a/tst/unit/providers/env_providers/env_gateway_provider_test.py b/tst/unit/providers/env_providers/env_gateway_provider_test.py index 5d6142484..6e74ff935 100644 --- a/tst/unit/providers/env_providers/env_gateway_provider_test.py +++ b/tst/unit/providers/env_providers/env_gateway_provider_test.py @@ -1103,6 +1103,42 @@ def test_compose_reads_every_servers_card_and_mcp_on_its_mcp_port_the_website_br assert "18765" in services["website-browser"]["healthcheck"]["test"][1] +@pytest.mark.parametrize("browsing", [True, False], ids=["with-browser", "without"]) +def test_the_website_browser_reaches_only_the_gateway_and_the_sites_and_no_database(browsing): + from agent_env.env.envs.service_db import ServiceDBConfig + + sites = ["website-network"] if browsing else [] + mcp_servers = [MCPServerConfig(image="mcp-slack", environment_name="slack"), + MCPServerConfig(image="browser", environment_name="website-browser", + **({"networks": ("website-network",)} if browsing else {}))] + website_configs = [WebsiteConfig(backend_image="shop-be", frontend_image="shop-fe", environment_name="shop"), + WebsiteConfig(backend_image="notes-be", frontend_image=None, environment_name="notes")] + provider = EnvironmentGatewayProvider() + compose = yaml.safe_load(provider.create_docker_compose( + mcp_servers=mcp_servers, + gateway_image="agent-gateway", + gateway_port=18765, + website_configs=website_configs, + state_provider=LocalPostgresStateProvider(service_db_config=ServiceDBConfig(db_web_image="pgweb", db_mcp_image="db-mcp")), + state_instance=LocalPostgresStateProvider.default_instance(), + )) + + networks = {name: service["networks"] for name, service in compose["services"].items()} + assert networks == { + "servicedb": ["env-network"], "pgweb": ["env-network"], "db-mcp": ["env-network"], + "slack": ["env-network"], "website-browser": sites or ["env-network"], + GATEWAY_SERVICE_NAME: ["env-network", *sites], + "shop-website-backend": ["env-network"], "shop-website-frontend": ["env-network", *sites], + "notes-website-backend": ["env-network", *sites], + } + assert list(compose["networks"]) == ["env-network", *sites] + browser = compose["services"]["website-browser"] + database = ("depends_on" in browser, any(e.startswith("DATABASE_URL=") for e in browser["environment"])) + assert database == (not browsing, not browsing) + assert provider._all_environments(mcp_servers, website_configs) == [ + "slack", *([] if browsing else ["website-browser"]), "shop", "notes"] + + # --- sidecar rendering -------------------------------------------------------- class _FakeImageArtifact: @@ -1331,10 +1367,10 @@ def test_needs_local_postgres_reflects_provider_type(): ({}, "website-browser"), ({"website_browser_env_id": "wb-run"}, "wb-run"), ], ids=["configured-default", "run-override"]) -async def test_deploy_via_vm_prepares_schemas_including_website_browser(overrides, browser_name): - """The gateway auto-adds the browser MCP when websites are present, the run's own when it names one, then drives - stage 2 (prepare) with the full post-append service list — so a remote store provisions the browser's - schema. This replaces the earlier band-aid that made Env.deploy compute that list.""" +async def test_deploy_via_vm_adds_the_website_browser_and_prepares_schemas_without_it(overrides, browser_name): + """The gateway auto-adds the browser MCP when websites are present, the run's own when it names one, on the sites' + network alone, then drives stage 2 (prepare) with the post-append service list, which leaves the browser out: + it reaches no database, so a remote store provisions it no schema.""" from agent_env.config import run_env_ids from agent_env.env.gateway import GatewayMode from agent_env.providers.env_state import DatabaseStateProvider @@ -1379,10 +1415,10 @@ def _fake_env_get(env_id, *a, **k): state_provider.prepare.assert_awaited_once() names = state_provider.prepare.call_args.args[0] - assert set(names) == {"slack", "shop", browser_name} + assert set(names) == {"slack", "shop"} assert compose.call_args.kwargs["mcp_server_name"] == "crm" browser = next(s for s in compose.call_args.kwargs["mcp_servers"] if s.environment_name == browser_name) - assert browser == MCPServerConfig(image=browser.image, environment_name=browser_name) # like any MCP server + assert browser == MCPServerConfig(image=browser.image, environment_name=browser_name, networks=("website-network",)) @pytest.mark.asyncio