fix(gateway): read uvicorn's WEB_CONCURRENCY in the remaining worker-count readers (#6253)

* fix(gateway): read uvicorn's WEB_CONCURRENCY in the remaining worker-count readers

* fix(gateway): name the worker spelling an operator can act on

The browser refusal and the dedupe warning both took the count but printed
GATEWAY_WORKERS, so on the launches that pass uvicorn no --workers the advice
silences the gate while the extra processes keep starting. Resolve which
variable set the count and print that, and let an unparsable GATEWAY_WORKERS
fall through to WEB_CONCURRENCY instead of reporting a single worker.

Also stop the comments from claiming the readers are equivalent: they share
the two spellings and the blank-means-unset rule, but channel_connections.py
turns an unparsable value into 0 and refuses while these stay inert.

* docs: state the single-worker requirement under both spellings

The browser tool group and the inbound dedupe warning now refuse or warn on
WEB_CONCURRENCY as well, so guidance that names only GATEWAY_WORKERS=1 is
incomplete for the shipped Dockerfile and serve.sh launches, where uvicorn
takes the process count from WEB_CONCURRENCY.

* test(features): isolate the browser capability surface from the shell's worker count

browser_capability() asks for the Gateway worker count, which this change makes
readable from WEB_CONCURRENCY, so an exported count in the invoking shell turned
test_features_reports_browser_control_enabled_when_configured_and_runtime_available
into a failure that has nothing to do with the router.
This commit is contained in:
Grapette.L 2026-10-04 16:36:01 +08:00 committed by GitHub
parent fcd1d315de
commit e7e5a5e9a3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 240 additions and 21 deletions

View File

@ -2039,7 +2039,7 @@ uv sync --extra browser
uv run playwright install chromium
```
Then uncomment the `group: browser` tool entries in `config.yaml` (`browser_navigate`, `browser_snapshot`, `browser_click`, `browser_type`, `browser_get_text`, `browser_back`, `browser_screenshot`, `browser_close`). `make dev` / Docker startup detects an enabled `browser_navigate` tool and preserves the `browser` extra on dependency syncs. The Gateway fails startup if browser control is configured but Playwright is missing, and `/api/features` hides the Browser UI unless the backend can actually serve it. Keep `headless: true` and `allow_private_addresses: false` for anything but local, trusted debugging. Attaching to an existing Chrome with `cdp_url` cannot enforce DeerFlow's subresource/redirect SSRF guard and therefore fails closed unless `allow_unguarded_cdp: true` explicitly acknowledges that risk; use it only with a trusted local browser. Browser sessions are process-local; keep `GATEWAY_WORKERS=1` while this tool group is enabled because ordinary uvicorn worker dispatch does not provide thread affinity.
Then uncomment the `group: browser` tool entries in `config.yaml` (`browser_navigate`, `browser_snapshot`, `browser_click`, `browser_type`, `browser_get_text`, `browser_back`, `browser_screenshot`, `browser_close`). `make dev` / Docker startup detects an enabled `browser_navigate` tool and preserves the `browser` extra on dependency syncs. The Gateway fails startup if browser control is configured but Playwright is missing, and `/api/features` hides the Browser UI unless the backend can actually serve it. Keep `headless: true` and `allow_private_addresses: false` for anything but local, trusted debugging. Attaching to an existing Chrome with `cdp_url` cannot enforce DeerFlow's subresource/redirect SSRF guard and therefore fails closed unless `allow_unguarded_cdp: true` explicitly acknowledges that risk; use it only with a trusted local browser. Browser sessions are process-local; keep the Gateway to a single worker process while this tool group is enabled because ordinary uvicorn worker dispatch does not provide thread affinity. That means `GATEWAY_WORKERS=1`, and on the launches that pass uvicorn no worker count at all (`backend/Dockerfile`, `scripts/serve.sh`) also `WEB_CONCURRENCY` unset or `1`, since uvicorn takes the process count from it.
Existing, non-mock Custom Agent chats expose the same Browser Live controls when browser control is available and the agent either leaves `tool_groups` unrestricted or includes the `browser` group. An explicit tool-group allowlist without `browser` keeps those controls hidden.

View File

@ -198,12 +198,38 @@ class PostgresInboundDedupeStore:
logger.exception("PostgresInboundDedupeStore.release failed; key left for TTL expiry (fail-open)")
def _gateway_workers() -> int:
"""Mirror deps._enforce_postgres_for_multi_worker's worker detection."""
try:
return int(os.environ.get("GATEWAY_WORKERS", "1") or 1)
except (TypeError, ValueError):
return 1
# The gateway worker count has two spellings: ``GATEWAY_WORKERS``, which
# docker-compose forwards as ``--workers``, and ``WEB_CONCURRENCY``, which uvicorn reads
# when no count is passed at all (``backend/Dockerfile``, ``scripts/serve.sh``). Read
# both, otherwise a multi-worker pod that shares no dedupe state reports itself as
# single-worker.
_WORKER_COUNT_ENV_VARS = ("GATEWAY_WORKERS", "WEB_CONCURRENCY")
def _gateway_workers() -> tuple[int, str]:
"""Resolve the Gateway worker count the same way the startup gates do.
Returns the count and the name of the variable that set it, so a warning can name
the knob the operator actually has to change. A blank value means "unset".
This is a second reader of the same two spellings, not an import of
``app/gateway/deps.py``'s helper, so the two can drift; and it differs on purpose
from ``app/gateway/routers/channel_connections.py``, which turns an unparsable value
into 0 and refuses, while here an unparsable value is skipped and only a resolved
count above 1 warns.
"""
for name in _WORKER_COUNT_ENV_VARS:
raw = os.environ.get(name)
if not raw or not raw.strip():
continue
try:
return int(raw), name
except (TypeError, ValueError):
# Do not let an unparsable ``GATEWAY_WORKERS`` mask a real count in
# ``WEB_CONCURRENCY``: on the launchers that pass no ``--workers`` the
# former never reaches uvicorn, so the latter is what starts the processes.
continue
return 1, _WORKER_COUNT_ENV_VARS[0]
def _build_postgres_store() -> InboundDedupeStore:
@ -242,7 +268,8 @@ def make_inbound_dedupe_store(app_config: Any | None = None) -> InboundDedupeSto
db_backend = getattr(db, "backend", None)
db_is_postgres = db_backend == "postgres"
multi_worker = _gateway_workers() > 1
workers, worker_env = _gateway_workers()
multi_worker = workers > 1
if backend == "postgres":
if not db_is_postgres:
@ -255,7 +282,7 @@ def make_inbound_dedupe_store(app_config: Any | None = None) -> InboundDedupeSto
if backend == "memory":
if multi_worker:
logger.warning(
"dedupe_storage=memory with GATEWAY_WORKERS>1: inbound webhook dedupe "
f"dedupe_storage=memory with {worker_env}>1: inbound webhook dedupe "
"is per-pod and will NOT drop redeliveries routed to a different replica. "
"Use dedupe_storage=postgres (or remove the setting to let 'auto' pick it) "
"for multi-worker deployments. See issue #4120."

View File

@ -516,7 +516,7 @@ Notes:
- **Upgrade note:** before upgrading a deployment with `GATEWAY_WORKERS > 1` and `scheduler.enabled: true`, either run the scheduler on exactly one Gateway worker or enable `scheduler.multi_instance: true` with shared Postgres, `run_ownership.heartbeat_enabled: true`, and `run_events.backend: db`. The startup gate now rejects the unsafe combination instead of allowing it to start silently.
- **Upgrade note:** in multi-instance mode, `max_concurrent_runs` is cluster-wide rather than per Pod and counts `launching`/`running` occurrences. Waiting `queued` rows remain outside the execution cap; capacity does not multiply with the replica count.
- **Upgrade note:** `scheduler.multi_instance` and its related scheduler, ownership, and run-event settings are startup-only. Restart all Gateway Pods together after changing them; a ConfigMap update without a coordinated restart leaves the running service on its previous mode.
- Multi-worker deployments (`GATEWAY_WORKERS > 1`) must use the Postgres database backend, enable run ownership heartbeats, and set `run_events.backend: db`. SQLite silently ignores row-level locks, while memory and JSONL run-event stores are process-local and cannot enforce singleton delivery receipts across workers; startup rejects these combinations. The process-local agentic browser tool group is incompatible with multiple Gateway workers; keep `GATEWAY_WORKERS=1` while `browser_navigate` is enabled. Browser control also requires the backend `browser` extra (`cd backend && uv sync --extra browser && uv run playwright install chromium`); startup detects enabled browser config and fails fast when Playwright is missing, and `/api/features` reports `browser_control.enabled=false` until the runtime is available.
- Multi-worker deployments (`GATEWAY_WORKERS > 1`) must use the Postgres database backend, enable run ownership heartbeats, and set `run_events.backend: db`. SQLite silently ignores row-level locks, while memory and JSONL run-event stores are process-local and cannot enforce singleton delivery receipts across workers; startup rejects these combinations. The process-local agentic browser tool group is incompatible with multiple Gateway workers; keep the Gateway to one worker process while `browser_navigate` is enabled — `GATEWAY_WORKERS=1`, plus `WEB_CONCURRENCY` unset or `1` on launches that pass uvicorn no worker count (`backend/Dockerfile`, `scripts/serve.sh`), where uvicorn takes the process count from it. Browser control also requires the backend `browser` extra (`cd backend && uv sync --extra browser && uv run playwright install chromium`); startup detects enabled browser config and fails fast when Playwright is missing, and `/api/features` reports `browser_control.enabled=false` until the runtime is available.
- The MVP supports thread reuse and fresh-thread-per-run execution modes.
- Create/update accept optional `assistant_id` (`lead_agent` by default, or an existing custom agent for the task owner).
- Create/update accept `once`, `cron`, and `interval`. Interval uses `schedule_spec.every_seconds` (UTC `now + N`, no missed-beat catch-up). N is at least `min_once_delay_seconds` (default 60) and at most 30 days.

View File

@ -143,16 +143,49 @@ _DEFAULT_MAX_SESSIONS = 32
_DEFAULT_IDLE_TIMEOUT_S = 30 * 60.0
def browser_multi_worker_error(workers: int | None = None) -> str | None:
"""Return the fail-closed reason for process-local browser sessions."""
if workers is None:
# ``GATEWAY_WORKERS`` is the documented knob and the one docker-compose forwards as
# ``--workers``; ``backend/Dockerfile`` and ``scripts/serve.sh`` start uvicorn with no
# worker count at all, and uvicorn then takes the count from ``WEB_CONCURRENCY``. Both
# must be read, or this gate stays inert in exactly the deployment it exists to refuse.
# ``app/gateway/routers/channel_connections.py`` reads the same two spellings and treats a
# blank value as unset the same way, but deliberately differs on unparsable input: that
# route resolves it to 0 and refuses, while this one falls through to the next spelling
# and stays inert, because uvicorn rejects ``--workers abc`` itself before serving.
_WORKER_COUNT_ENV_VARS = ("GATEWAY_WORKERS", "WEB_CONCURRENCY")
def _worker_count_from_env() -> tuple[int, str]:
"""Resolve the worker count and the variable that set it; a blank value is unset."""
for name in _WORKER_COUNT_ENV_VARS:
raw = os.environ.get(name)
if not raw or not raw.strip():
continue
try:
workers = int(os.environ.get("GATEWAY_WORKERS", "1"))
return int(raw), name
except (TypeError, ValueError):
workers = 1
# Unparsable here must not hide a valid count in the other spelling: an
# exported ``GATEWAY_WORKERS=abc`` never reaches uvicorn on the launchers
# that pass no ``--workers``, so ``WEB_CONCURRENCY`` is still what decides
# how many processes start.
continue
return 1, _WORKER_COUNT_ENV_VARS[0]
def browser_multi_worker_error(workers: int | None = None, env_name: str | None = None) -> str | None:
"""Return the fail-closed reason for process-local browser sessions.
``env_name`` is the spelling the caller resolved the count from; the refusal has to
name it, because an operator who only ever set ``WEB_CONCURRENCY`` cannot act on a
message about ``GATEWAY_WORKERS``. Without it the name is resolved from the
environment, which is the same source the count came from.
"""
if workers is None:
workers, env_name = _worker_count_from_env()
elif env_name is None:
env_name = _worker_count_from_env()[1]
if workers <= 1:
return None
return f"GATEWAY_WORKERS={workers} cannot enable agentic browser tools: browser sessions are process-local and uvicorn does not provide thread affinity. Set GATEWAY_WORKERS=1 or disable the browser_navigate tool."
return f"{env_name}={workers} cannot enable agentic browser tools: browser sessions are process-local and uvicorn does not provide thread affinity. Set {env_name}=1 or disable the browser_navigate tool."
def ensure_browser_worker_compatibility() -> None:

View File

@ -26,9 +26,22 @@ from deerflow.community.browser_automation.session import (
BrowserSessionManager,
PageSnapshot,
SnapshotElement,
browser_multi_worker_error,
)
@pytest.fixture(autouse=True)
def isolated_worker_env(monkeypatch):
"""Keep the suite independent of the invoking shell's worker count.
``WEB_CONCURRENCY`` is a common ambient convention on uvicorn/gunicorn/PaaS hosts,
and either spelling now decides whether process-local browser sessions are refused.
The tests that exercise the refusal set the spelling they mean to exercise.
"""
monkeypatch.delenv("GATEWAY_WORKERS", raising=False)
monkeypatch.delenv("WEB_CONCURRENCY", raising=False)
@pytest.fixture
def public_dns():
"""Keep mocked tool calls offline while exercising the real URL safety check."""
@ -882,11 +895,22 @@ class TestSessionManager:
async def test_get_session_rejects_runtime_multi_worker_browser_use(self, monkeypatch):
monkeypatch.setenv("GATEWAY_WORKERS", "2")
monkeypatch.delenv("WEB_CONCURRENCY", raising=False)
manager = BrowserSessionManager()
with pytest.raises(RuntimeError, match="process-local"):
manager.get_session("thread-a")
async def test_get_session_rejects_worker_count_from_the_uvicorn_fallback(self, monkeypatch):
# backend/Dockerfile and scripts/serve.sh start uvicorn with no --workers, so
# WEB_CONCURRENCY is what really decides the process count here too.
monkeypatch.delenv("GATEWAY_WORKERS", raising=False)
monkeypatch.setenv("WEB_CONCURRENCY", "2")
manager = BrowserSessionManager()
with pytest.raises(RuntimeError, match=r"WEB_CONCURRENCY=2 cannot enable agentic browser tools"):
manager.get_session("thread-a")
async def test_cdp_requires_explicit_unguarded_trust_opt_in(self):
manager = BrowserSessionManager()
@ -948,6 +972,58 @@ class TestSessionManager:
assert session.active_refs == 0
@pytest.mark.parametrize(
("gateway_workers", "web_concurrency", "expected"),
[
pytest.param(None, "4", 4, id="uvicorn-fallback-counts"),
pytest.param("2", None, 2, id="documented-knob-counts"),
pytest.param("1", "4", 1, id="documented-knob-wins-when-both-set"),
pytest.param(" ", "3", 3, id="blank-is-unset"),
pytest.param("abc", "4", 4, id="unparsable-knob-does-not-mask-the-fallback"),
pytest.param(None, None, 1, id="nothing-set"),
pytest.param(None, "auto", 1, id="uvicorn_rejects_it_so_stay_inert"),
],
)
def test_worker_count_resolution_reads_both_spellings(monkeypatch, gateway_workers, web_concurrency, expected):
for name, value in (("GATEWAY_WORKERS", gateway_workers), ("WEB_CONCURRENCY", web_concurrency)):
if value is None:
monkeypatch.delenv(name, raising=False)
else:
monkeypatch.setenv(name, value)
assert session_mod._worker_count_from_env()[0] == expected
def test_worker_count_error_names_the_variable_that_set_the_count(monkeypatch):
monkeypatch.delenv("GATEWAY_WORKERS", raising=False)
monkeypatch.setenv("WEB_CONCURRENCY", "4")
error = browser_multi_worker_error()
assert error is not None
assert error.startswith("WEB_CONCURRENCY=4 ")
assert "Set WEB_CONCURRENCY=1" in error
def test_worker_count_error_names_the_fallback_for_a_passed_in_count(monkeypatch):
# The Gateway startup gate resolves the count and calls this with the number only;
# the refusal still has to name the knob the operator set, or its advice would
# silence the gate while uvicorn keeps starting the extra processes.
monkeypatch.delenv("GATEWAY_WORKERS", raising=False)
monkeypatch.setenv("WEB_CONCURRENCY", "4")
assert browser_multi_worker_error(4).startswith("WEB_CONCURRENCY=4 ")
def test_worker_count_error_keeps_naming_the_documented_knob_when_passed_in(monkeypatch):
# With neither spelling in the environment there is nothing to attribute the count
# to, so a passed-in number keeps naming GATEWAY_WORKERS as it always has.
monkeypatch.delenv("GATEWAY_WORKERS", raising=False)
monkeypatch.delenv("WEB_CONCURRENCY", raising=False)
assert browser_multi_worker_error(3).startswith("GATEWAY_WORKERS=3 ")
def test_resolve_session_always_reads_browser_navigate_config():
"""Launch config is deterministic regardless of which tool runs first.

View File

@ -9,6 +9,19 @@ from app.gateway.deps import get_config
from app.gateway.routers import features
@pytest.fixture(autouse=True)
def isolated_worker_env(monkeypatch):
"""Keep the feature surface independent of the invoking shell's worker count.
``browser_capability()`` consults the Gateway worker count, which uvicorn takes
from ``WEB_CONCURRENCY`` on the launches that pass no ``--workers``. An exported
value there would otherwise make the browser-control assertions below report a
refusal the code under test did not produce.
"""
monkeypatch.delenv("GATEWAY_WORKERS", raising=False)
monkeypatch.delenv("WEB_CONCURRENCY", raising=False)
def _app_with_config(
*,
agents_api_enabled: bool,

View File

@ -18,6 +18,18 @@ from app.channels.dedupe_store import (
)
@pytest.fixture(autouse=True)
def isolated_worker_env(monkeypatch):
"""Keep config-resolution tests independent of the invoking shell.
``WEB_CONCURRENCY`` is a common ambient convention on uvicorn/gunicorn/PaaS hosts,
and it now decides whether a deployment counts as multi-worker here too. The tests
that exercise that path set the spelling they mean to exercise.
"""
monkeypatch.delenv("GATEWAY_WORKERS", raising=False)
monkeypatch.delenv("WEB_CONCURRENCY", raising=False)
class _FakeDedupe:
def __init__(self, backend: str) -> None:
self.backend = backend # plain string, mimics DedupeStorageBackend.value
@ -106,6 +118,60 @@ def test_factory_warns_when_auto_resolves_memory_under_multi_worker(monkeypatch,
assert any("Multi-worker deployment detected but dedupe_storage=auto" in r.message for r in caplog.records)
def test_factory_warns_under_the_uvicorn_worker_fallback(monkeypatch, caplog):
import logging
# backend/Dockerfile and scripts/serve.sh start uvicorn with no --workers, so
# WEB_CONCURRENCY is what decides how many processes share no dedupe state.
monkeypatch.delenv("GATEWAY_WORKERS", raising=False)
monkeypatch.setenv("WEB_CONCURRENCY", "2")
app = _FakeApp("memory", "sqlite")
with caplog.at_level(logging.WARNING):
store = make_inbound_dedupe_store(app)
assert isinstance(store, MemoryInboundDedupeStore)
assert any("dedupe_storage=memory with WEB_CONCURRENCY>1" in r.message for r in caplog.records)
def test_factory_warns_when_an_unparsable_knob_hides_the_fallback_count(monkeypatch, caplog):
import logging
# GATEWAY_WORKERS never reaches uvicorn on the launchers that pass no --workers, so
# an unusable value there must not report the deployment as single-worker while
# WEB_CONCURRENCY still starts the processes.
monkeypatch.setenv("GATEWAY_WORKERS", "abc")
monkeypatch.setenv("WEB_CONCURRENCY", "4")
app = _FakeApp("memory", "sqlite")
with caplog.at_level(logging.WARNING):
make_inbound_dedupe_store(app)
assert any("dedupe_storage=memory with WEB_CONCURRENCY>1" in r.message for r in caplog.records)
def test_factory_prefers_the_documented_knob_when_both_are_set(monkeypatch, caplog):
import logging
# docker-compose forwards GATEWAY_WORKERS as --workers, which outranks the
# WEB_CONCURRENCY uvicorn would otherwise read.
monkeypatch.setenv("GATEWAY_WORKERS", "1")
monkeypatch.setenv("WEB_CONCURRENCY", "4")
app = _FakeApp("memory", "sqlite")
with caplog.at_level(logging.WARNING):
make_inbound_dedupe_store(app)
assert not any("dedupe_storage=memory with" in r.message for r in caplog.records)
def test_factory_treats_a_blank_gateway_workers_as_unset(monkeypatch, caplog):
import logging
# The compose command spells the fallback ${GATEWAY_WORKERS:-1}, so an exported
# empty value means "unset" rather than "no workers".
monkeypatch.setenv("GATEWAY_WORKERS", " ")
monkeypatch.setenv("WEB_CONCURRENCY", "3")
app = _FakeApp("auto", "sqlite")
with caplog.at_level(logging.WARNING):
make_inbound_dedupe_store(app)
assert any("Multi-worker deployment detected" in r.message for r in caplog.records)
# ---------------------------------------------------------------------------
# PostgresInboundDedupeStore (unit, mocked session factory)
# ---------------------------------------------------------------------------

View File

@ -1129,7 +1129,9 @@ tools:
# # # CDP handshake. If so, use headless: false to launch
# # # Playwright's own visible browser instead.
# # Multi-worker note: browser sessions live in one Gateway worker's memory.
# # Keep GATEWAY_WORKERS=1 while this tool group is enabled.
# # Keep the Gateway to one worker process while this tool group is enabled:
# # GATEWAY_WORKERS=1, and WEB_CONCURRENCY unset or 1 on the launches that pass
# # uvicorn no --workers (backend/Dockerfile, scripts/serve.sh).
# - name: browser_snapshot
# group: browser
# use: deerflow.community.browser_automation.tools:browser_snapshot_tool

View File

@ -306,10 +306,12 @@ Memory backend async boundary:
CI runs these regression tests for every pull request via [.github/workflows/backend-unit-tests.yml](../.github/workflows/backend-unit-tests.yml).
Agentic browser sessions are process-local. The Gateway startup safety gate rejects
`GATEWAY_WORKERS > 1` when `browser_navigate` is configured, because ordinary
uvicorn worker dispatch does not provide thread affinity for browser tools, REST
navigation, and the Live WebSocket.
Agentic browser sessions are process-local. Browser use is refused when the Gateway runs
more than one worker process, because ordinary uvicorn worker dispatch does not provide
thread affinity for browser tools, REST navigation, and the Live WebSocket. Keep
`GATEWAY_WORKERS=1`, and on the launchers that pass uvicorn no `--workers`
(`scripts/serve.sh`, `backend/Dockerfile`) keep `WEB_CONCURRENCY` unset or `1` too — that
is where uvicorn takes the process count from.
Browser Live screenshots remain JPEG bytes inside the harness and the Gateway's
bounded, drop-oldest frame queue. WebSocket clients that request