deer-flow/backend/tests/test_github_token_plumbing.py
Zheng Feng dcb2e687d5
feat(channels): add GitHub as a webhook-driven channel (#3754)
* feat(channels): add GitHub event-driven agents (#3754)

Add a webhook-driven GitHub channel with fail-closed webhook routing, deterministic per-agent PR/issue threads, mention-gated trigger fan-out, GitHub App token injection for sandboxed gh/git commands, and backend/AGENTS.md documentation.

* fix(llm-middleware): classify bare IndexError as transient

Upstream chat providers occasionally return 200 OK with an empty
generations list (observed against Volces "coding" on
ark.cn-beijing.volces.com). When that happens,
langchain_core.language_models.chat_models.ainvoke raises
``IndexError: list index out of range`` at
``llm_result.generations[0][0].message`` and kills the run.

Treat a bare IndexError reaching the middleware as a transient
upstream-payload glitch and route it through the existing
retry/backoff path instead of failing the whole agent run. The
retry budget and backoff schedule are unchanged.

Adds three regression tests covering the classifier and both the
recover-on-retry and exhausted-retries paths.

* fix(runtime): ignore stale LLM fallback markers from prior runs

When a run on a thread ends with the LLM-error-handling middleware emitting
a `deerflow_error_fallback`-marked AIMessage (e.g. after the IndexError
empty-generations classification fix lands), that message is persisted to
the thread's checkpoint as part of the messages channel. LangGraph replays
the full message history in `stream_mode="values"` chunks, so every
subsequent run on the same thread re-streams the stale fallback marker —
and the worker's chunk scanner faithfully picks it up, flipping
`RunStatus.success` to `RunStatus.error` for runs that themselves had
no LLM failure at all.

Snapshot the set of pre-existing message ids from the pre-run checkpoint
and thread it through `_extract_llm_error_fallback_message` /
`_try_extract_from_message` as a filter. Markers on history messages are
ignored; markers on fresh messages produced during this run still trip
the error path. Falls back to an empty set when the checkpointer is
absent or the snapshot can't be captured, preserving the prior behavior
on first-run / no-state paths.

Adds unit tests for the new filter (helper-level and `_collect_pre_existing_message_ids`)
plus an integration test exercising the full `run_agent` path with a stale
history checkpointer.

* fix(channels): make github channel fire-and-forget to avoid httpx.ReadTimeout on long runs

GitHub agent runs (clone -> edit -> test -> push -> PR) routinely exceed
the langgraph_sdk default 300s read deadline. The manager's runs.wait
call kept an HTTP stream open for the entire run lifetime, so the long
run blew up with httpx.ReadTimeout and the outer except branch then
released the dedupe key and emitted a false 'internal error' outbound.

The GitHub channel's outbound send is log-only by design: agents post to
the issue/PR via the gh CLI in the sandbox when they choose to comment
or create a PR. There is nothing for the manager to ferry back, so the
long-poll was pure overhead.

This change adds ChannelRunPolicy.fire_and_forget (default False) and
sets it True for the github channel. When fire_and_forget is True,
_handle_chat dispatches via client.runs.create (short POST, returns
once the run is pending) instead of client.runs.wait, and skips the
response-extraction + outbound-publish block. ConflictError on a busy
thread still trips the standard THREAD_BUSY_MESSAGE path so behavior on
the busy case is preserved for any future non-github fire-and-forget
channel.

Other (non-github) channels are unchanged: their policy defaults
fire_and_forget=False and they continue to dispatch via runs.wait.

Adds 6 regression tests in tests/test_channels.py::TestGithubFireAndForget:
- Default ChannelRunPolicy.fire_and_forget is False.
- The github policy registers fire_and_forget=True.
- github inbound calls runs.create, not runs.wait, with the right kwargs.
- github inbound publishes no outbound on success.
- ConflictError from runs.create still emits THREAD_BUSY_MESSAGE.
- Non-github channels (slack) still dispatch via runs.wait.

* test(lead-agent): accept user_id kwarg in skill-policy test stubs

The two GitHub-channel tests added in #3754 stubbed
_load_enabled_skills_for_tool_policy with a lambda that only accepted
`available_skills` and `app_config`, but the real function (and its call
site in agent.py) also passes `user_id`. This raised TypeError on every
run, failing backend-unit-tests.

Add `user_id=None` to match the three sibling stubs in the same file.

* refactor(gateway): disambiguate context-key set names

The two frozensets _INTERNAL_ONLY_CONTEXT_KEYS and _CONTEXT_ONLY_KEYS
shared a confusable "CONTEXT_ONLY" token in different orders, and the
first broke the _CONTEXT_<X>_KEYS pattern of its sibling
_CONTEXT_CONFIGURABLE_KEYS. Rename to make the distinct axes explicit:

  _CONTEXT_INTERNAL_CALLER_KEYS  - WHO: internal callers (scheduler) only
  _CONTEXT_RUNTIME_ONLY_KEYS     - WHERE: runtime context only, never configurable

Pure rename, no behavior change.
2026-07-04 22:56:24 +08:00

756 lines
28 KiB
Python

"""Tests for the GitHub App installation-token plumbing.
Covers the end-to-end path that lets a GitHub-driven agent actually push
code and open PRs:
dispatcher carries ``installation_id`` + a deterministic
``preferred_thread_id`` in ``InboundMessage.metadata``
-> ChannelManager mints an installation token and injects it into
``run_context["github_token"]``
-> the value flows through ``context=`` into ``runtime.context``
-> the bash tool exposes it as ``GH_TOKEN`` / ``GITHUB_TOKEN`` via a
per-call ``env`` overlay on ``Sandbox.execute_command``
The per-call overlay (rather than mutating ``os.environ``) is what keeps
concurrent runs on different repos from clobbering each other's token.
"""
from __future__ import annotations
from pathlib import Path
from types import SimpleNamespace
from unittest.mock import AsyncMock, patch
import httpx
import pytest
from langgraph_sdk.errors import ConflictError
from app.channels.manager import ChannelManager
from app.channels.message_bus import InboundMessage, InboundMessageType, MessageBus
from app.channels.store import ChannelStore
from deerflow.sandbox.local.local_sandbox import LocalSandbox
from deerflow.sandbox.tools import _github_env_from_runtime, bash_tool
def _make_conflict_error(detail: str = "thread_id already exists") -> ConflictError:
"""Mint a ConflictError that matches what langgraph_sdk would raise on a
409 from ``POST /threads``. Constructing the SDK error directly requires
an httpx.Response with an attached Request, so this helper hides the
boilerplate.
"""
req = httpx.Request("POST", "http://gateway/api/threads")
resp = httpx.Response(409, json={"detail": detail}, request=req)
return ConflictError(detail, response=resp, body={"detail": detail})
# ---------------------------------------------------------------------------
# Sandbox.execute_command env
# ---------------------------------------------------------------------------
def test_local_sandbox_env_overlay_reaches_subprocess(monkeypatch: pytest.MonkeyPatch) -> None:
"""``env`` is layered on top of a sanitized os.environ for the subprocess
call — inherited benign vars survive, the injected secret wins."""
import deerflow.sandbox.local.local_sandbox as local_sandbox
captured: dict = {}
def fake_run_posix(args, timeout, env=None):
captured["env"] = env
return ("", "", 0, False)
monkeypatch.setattr(LocalSandbox, "_run_posix_command", staticmethod(fake_run_posix))
monkeypatch.setattr(LocalSandbox, "_get_shell", staticmethod(lambda: "/bin/bash"))
monkeypatch.setattr(local_sandbox.os, "environ", {"PATH": "/usr/bin", "EXISTING": "kept"})
LocalSandbox("local:t").execute_command("echo $GITHUB_TOKEN", env={"GITHUB_TOKEN": "tok-123"})
env = captured["env"]
assert env["GITHUB_TOKEN"] == "tok-123"
# Inherited vars survive the overlay.
assert env["EXISTING"] == "kept"
def test_local_sandbox_no_env_passes_sanitized_environ(monkeypatch: pytest.MonkeyPatch) -> None:
"""Without ``env`` the subprocess still gets a sanitized environ — platform
secrets are scrubbed (#3861), only benign inherited vars survive."""
import deerflow.sandbox.local.local_sandbox as local_sandbox
captured: dict = {}
def fake_run_posix(args, timeout, env=None):
captured["env"] = env
return ("", "", 0, False)
monkeypatch.setattr(LocalSandbox, "_run_posix_command", staticmethod(fake_run_posix))
monkeypatch.setattr(LocalSandbox, "_get_shell", staticmethod(lambda: "/bin/bash"))
monkeypatch.setattr(local_sandbox.os, "environ", {"PATH": "/usr/bin", "OPENAI_API_KEY": "sk-leak"})
LocalSandbox("local:t").execute_command("echo hi")
env = captured["env"]
# Platform credentials are scrubbed (#3861) — never inherited by skills.
assert "OPENAI_API_KEY" not in env
# Benign vars survive.
assert env["PATH"] == "/usr/bin"
def test_aio_sandbox_env_routes_through_bash_exec() -> None:
"""Per-call ``env`` is forwarded to the ``bash.exec`` API (structured env
field on a fresh session) so secrets like ``GITHUB_TOKEN`` reach the
command without being spliced into the command string. Replaces the old
persistent-shell ``export … unset`` overlay, which could not keep secrets
out of the command string.
"""
from deerflow.community.aio_sandbox.aio_sandbox import AioSandbox
captured: dict = {}
class _FakeBash:
def exec(self, *, command, env=None, **kwargs):
captured["command"] = command
captured["env"] = env
return SimpleNamespace(data=SimpleNamespace(stdout="ok", stderr=None))
sbx = AioSandbox.__new__(AioSandbox)
sbx._lock = __import__("threading").Lock()
sbx._client = SimpleNamespace(bash=_FakeBash())
sbx._DEFAULT_NO_CHANGE_TIMEOUT = 30
sbx._DEFAULT_HARD_TIMEOUT = 30
sbx._bash_exec_unsupported = False
out = sbx.execute_command("gh pr create", env={"GH_TOKEN": "tok-123"})
assert out == "ok"
assert captured["command"] == "gh pr create"
assert captured["env"] == {"GH_TOKEN": "tok-123"}
def test_aio_sandbox_no_env_leaves_command_unchanged() -> None:
from deerflow.community.aio_sandbox.aio_sandbox import AioSandbox
captured: dict = {}
class _FakeData:
output = "ok"
class _FakeResult:
data = _FakeData()
class _FakeShell:
def exec_command(self, *, command, no_change_timeout=None, **kwargs):
captured["command"] = command
return _FakeResult()
sbx = AioSandbox.__new__(AioSandbox)
sbx._lock = __import__("threading").Lock()
sbx._client = SimpleNamespace(shell=_FakeShell())
sbx._DEFAULT_NO_CHANGE_TIMEOUT = 30
sbx.execute_command("echo hello")
assert captured["command"] == "echo hello"
# ---------------------------------------------------------------------------
# extra_env key validation (regression pin for willem-bd #5)
# ---------------------------------------------------------------------------
@pytest.mark.parametrize(
"bad_key",
[
# Shell metachar in key — the actual injection vector flagged by
# the review (would render as `export X;rm -rf /;Y='v'; <cmd>`).
"X;rm -rf /mnt/user-data;Y",
"X`whoami`",
"X$(id)",
"X&Y",
"X|Y",
"X>Y",
"X<Y",
"X Y", # space
"X\tY", # tab
"X\nY", # newline
# Leading digit — not a valid POSIX env-var name even though it
# contains no metacharacters; rejecting these too keeps the rule
# simple and matches POSIX.
"1FOO",
# Empty / whitespace-only.
"",
" ",
# Non-str keys (a dict can have int keys at runtime).
123,
],
)
def test_extra_env_rejects_invalid_keys(bad_key) -> None:
"""Regression pin for willem-bd's finding #5 on PR #3754.
The abstract ``Sandbox.execute_command(env=...)`` contract validates
keys against the POSIX env-var rule ``^[A-Za-z_][A-Za-z0-9_]*$``. Today
no implementation splices a key into a shell string — the local sandbox
merges them into ``subprocess.run(env=...)`` (no shell), the AIO sandbox
forwards them via the ``bash.exec`` structured env field, and e2b
forwards them as the SDK's ``envs``. The rule is defense-in-depth for
the contract: future callers deriving a key from config / payload /
user input fail fast with ``ValueError`` rather than producing a latent
injection should a future implementation regress to splicing keys into
a shell command string.
The same rule applies to all implementations even though none currently
route a key through a shell — the contract is what matters, not each
implementation's current escaping rules.
"""
from deerflow.sandbox.sandbox import _validate_extra_env
with pytest.raises(ValueError, match="extra_env key"):
_validate_extra_env({bad_key: "value"})
@pytest.mark.parametrize(
"good_key",
[
"GH_TOKEN",
"GITHUB_TOKEN",
"_HIDDEN",
"foo123",
"MIXED_case_42",
"X",
],
)
def test_extra_env_accepts_valid_keys(good_key: str) -> None:
"""POSIX env-var names round-trip cleanly."""
from deerflow.sandbox.sandbox import _validate_extra_env
# No exception => acceptance.
_validate_extra_env({good_key: "any value with spaces and $metachars"})
def test_extra_env_none_and_empty_pass_through() -> None:
"""``None`` and empty dicts are the common case — must not raise."""
from deerflow.sandbox.sandbox import _validate_extra_env
_validate_extra_env(None)
_validate_extra_env({})
def test_local_sandbox_rejects_invalid_env_key(monkeypatch: pytest.MonkeyPatch) -> None:
"""End-to-end: a bad key reaches the implementation's ``execute_command``
and is rejected before any subprocess is spawned.
"""
import deerflow.sandbox.local.local_sandbox as local_sandbox
fake_run_called = False
def fake_run(*args, **kwargs):
nonlocal fake_run_called
fake_run_called = True
return SimpleNamespace(stdout="", stderr="", returncode=0)
monkeypatch.setattr(local_sandbox.subprocess, "run", fake_run)
with pytest.raises(ValueError, match="extra_env key"):
LocalSandbox("local:t").execute_command(
"echo hi",
env={"X;rm -rf /mnt/user-data;Y": "v"},
)
assert fake_run_called is False, "subprocess.run must not run when key is invalid"
def test_aio_sandbox_rejects_invalid_env_key() -> None:
"""End-to-end on the AIO sandbox path — the injection vector flagged in
the review never reaches the shell's ``exec_command``.
"""
from deerflow.community.aio_sandbox.aio_sandbox import AioSandbox
exec_called = False
class _FakeShell:
def exec_command(self, *, command, no_change_timeout=None, **kwargs):
nonlocal exec_called
exec_called = True
return SimpleNamespace(data=SimpleNamespace(output="ok"))
sbx = AioSandbox.__new__(AioSandbox)
sbx._lock = __import__("threading").Lock()
sbx._client = SimpleNamespace(shell=_FakeShell())
sbx._DEFAULT_NO_CHANGE_TIMEOUT = 30
with pytest.raises(ValueError, match="extra_env key"):
sbx.execute_command(
"echo hi",
env={"X;rm -rf /mnt/user-data;Y": "v"},
)
assert exec_called is False, "shell.exec_command must not run when key is invalid"
# ---------------------------------------------------------------------------
# bash_tool token read-through
# ---------------------------------------------------------------------------
def test_github_env_from_runtime_returns_token_pair() -> None:
runtime = SimpleNamespace(context={"github_token": "tok-abc"})
env = _github_env_from_runtime(runtime)
assert env == {"GH_TOKEN": "tok-abc", "GITHUB_TOKEN": "tok-abc"}
def test_github_env_from_runtime_resolves_provider_callable() -> None:
"""A callable in context["github_token"] is invoked per bash call.
This is the refresh seam: long autonomous github runs that span past
the 60-minute installation-token TTL need every bash invocation to
re-ask the provider, which transparently re-mints via the app-side
cache when the token's leeway tripped.
"""
calls = {"n": 0}
def _provider() -> str:
calls["n"] += 1
return f"tok-call-{calls['n']}"
runtime = SimpleNamespace(context={"github_token": _provider})
env_1 = _github_env_from_runtime(runtime)
assert env_1 == {"GH_TOKEN": "tok-call-1", "GITHUB_TOKEN": "tok-call-1"}
env_2 = _github_env_from_runtime(runtime)
assert env_2 == {"GH_TOKEN": "tok-call-2", "GITHUB_TOKEN": "tok-call-2"}
assert calls["n"] == 2 # called once per bash invocation
def test_github_env_from_runtime_returns_none_when_provider_raises() -> None:
"""A misbehaving provider must NOT crash the bash tool — it just falls
back to the no-token path so the run can still execute read-only.
"""
def _broken() -> str:
raise RuntimeError("mint failed")
runtime = SimpleNamespace(context={"github_token": _broken})
assert _github_env_from_runtime(runtime) is None
def test_github_env_from_runtime_returns_none_when_provider_returns_empty() -> None:
runtime = SimpleNamespace(context={"github_token": lambda: ""})
assert _github_env_from_runtime(runtime) is None
def test_github_env_from_runtime_none_when_no_token() -> None:
runtime = SimpleNamespace(context={"thread_id": "t1"})
assert _github_env_from_runtime(runtime) is None
def test_github_env_from_runtime_none_when_empty() -> None:
runtime = SimpleNamespace(context={"github_token": ""})
assert _github_env_from_runtime(runtime) is None
def test_bash_tool_passes_token_as_env(monkeypatch: pytest.MonkeyPatch) -> None:
"""When runtime.context carries github_token, bash forwards it to the sandbox."""
runtime = SimpleNamespace(
state={"sandbox": {"sandbox_id": "aio:xyz"}}, # non-local -> simpler branch
context={"thread_id": "t1", "github_token": "tok-from-manager"},
config={},
)
captured: dict = {}
class _Sandbox:
def execute_command(self, command, env=None, timeout=None):
captured["command"] = command
captured["env"] = env
return "done"
monkeypatch.setattr("deerflow.sandbox.tools.ensure_sandbox_initialized", lambda runtime: _Sandbox())
monkeypatch.setattr("deerflow.sandbox.tools.ensure_thread_directories_exist", lambda runtime: None)
result = bash_tool.func(runtime=runtime, description="push", command="git push")
assert "done" in result
assert captured["env"] == {"GH_TOKEN": "tok-from-manager", "GITHUB_TOKEN": "tok-from-manager"}
def test_bash_tool_no_env_without_token(monkeypatch: pytest.MonkeyPatch) -> None:
runtime = SimpleNamespace(
state={"sandbox": {"sandbox_id": "aio:xyz"}},
context={"thread_id": "t1"}, # no github_token
config={},
)
captured: dict = {}
class _Sandbox:
def execute_command(self, command, env=None, timeout=None):
captured["env"] = env
return "done"
monkeypatch.setattr("deerflow.sandbox.tools.ensure_sandbox_initialized", lambda runtime: _Sandbox())
monkeypatch.setattr("deerflow.sandbox.tools.ensure_thread_directories_exist", lambda runtime: None)
bash_tool.func(runtime=runtime, description="ls", command="ls")
assert captured["env"] is None
# ---------------------------------------------------------------------------
# ChannelManager._apply_channel_policy — the unified per-channel run-policy
# hook. The github channel registers (is_interactive=False,
# default_recursion_limit=250, credentials_provider=inject_github_credentials,
# requires_bound_identity=False) via CHANNEL_RUN_POLICY; this section
# exercises that one hook for github and the no-op path for unregistered
# channels (Slack, Telegram, …).
# ---------------------------------------------------------------------------
def _github_msg(installation_id: int | None = 140594274) -> InboundMessage:
return InboundMessage(
channel_name="github",
chat_id="zhfeng/llm-gateway",
user_id="zhfeng",
text="a PR was opened",
msg_type=InboundMessageType.CHAT,
# topic_id pairs PR number with agent name to keep each agent on
# its own deterministic thread; see app/gateway/github/dispatcher.py.
topic_id="7:coding-llm-gateway",
owner_user_id="default",
metadata={
"agent_name": "coding-llm-gateway",
"github": {
"repo": "zhfeng/llm-gateway",
"number": 7,
"installation_id": installation_id,
},
"preferred_thread_id": "uuid5-fixed",
},
)
def _new_manager() -> ChannelManager:
bus = MessageBus()
store = ChannelStore(path=Path("/tmp/nonexistent-store-test.json"))
return ChannelManager(bus=bus, store=store)
@pytest.mark.asyncio
async def test_run_context_after_apply_channel_policy_is_json_serializable(monkeypatch: pytest.MonkeyPatch) -> None:
"""Regression pin: ``run_context`` survives the langgraph SDK's JSON encoder.
The channel path calls ``client.runs.wait(thread_id, assistant_id,
context=run_context, …)``. The SDK encodes the body with ``orjson``
before sending it over HTTP. Anything in ``run_context`` that is not
JSON-serializable (notably the previous closure-based token provider)
raises ``TypeError: Type is not JSON serializable: function`` and the
entire delivery fails. This test pins the contract so we never
silently regress to shipping a closure again.
"""
import json
manager = _new_manager()
mint = AsyncMock(return_value="ghs_installation_token")
monkeypatch.setattr("app.gateway.github.app_auth.mint_installation_token", mint)
run_context: dict = {"thread_id": "t1", "user_id": "u1"}
await manager._apply_channel_policy(_github_msg(), run_context)
# Will raise TypeError if anything in run_context is not JSON-serializable.
encoded = json.dumps(run_context)
assert '"github_token": "ghs_installation_token"' in encoded
@pytest.mark.asyncio
async def test_apply_channel_policy_degrades_on_mint_failure(monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture) -> None:
"""A failed mint must not crash the run; the agent proceeds read-only.
``_apply_channel_policy`` catches credential-provider exceptions and
logs a warning so a transient GitHub API outage or a misconfigured
installation_id degrades to "no token injected" rather than dropping
the delivery.
"""
manager = _new_manager()
async def boom(_installation_id):
raise RuntimeError("GITHUB_APP_ID not set")
monkeypatch.setattr("app.gateway.github.app_auth.mint_installation_token", boom)
run_context: dict = {}
with caplog.at_level("WARNING", logger="app.channels.manager"):
await manager._apply_channel_policy(_github_msg(), run_context)
# Must not crash, must not set a token, must warn.
assert "github_token" not in run_context
assert any("credentials_provider raised" in r.message for r in caplog.records)
@pytest.mark.asyncio
async def test_apply_channel_policy_skips_token_without_installation_id(monkeypatch: pytest.MonkeyPatch) -> None:
"""A binding with no ``installation_id`` runs without a minted token —
no mint call, no ``github_token`` in run_context. The non-interactive
flag still gets set because that is decided by the channel-policy
entry, not by the credentials provider.
"""
manager = _new_manager()
mint = AsyncMock(return_value="should-not-be-called")
monkeypatch.setattr("app.gateway.github.app_auth.mint_installation_token", mint)
run_context: dict = {}
await manager._apply_channel_policy(_github_msg(installation_id=None), run_context)
mint.assert_not_awaited()
assert "github_token" not in run_context
# disable_clarification is set unconditionally for github (it is on
# the policy entry, not the credentials provider).
assert run_context["disable_clarification"] is True
# ---------------------------------------------------------------------------
# ChannelRunPolicy registration + _apply_channel_policy
# ---------------------------------------------------------------------------
def test_github_policy_is_registered_on_import() -> None:
"""Importing the gateway.github subpackage registers the run policy.
The manager doesn't carry GitHub-specific branches anymore — the
policy entry is the single source of truth. Tests, gateway
bootstrap, and ad-hoc scripts all get the same registration via the
same import side-effect.
"""
# Importing app.gateway.github runs run_policy.register_policy() as
# an import side-effect.
import app.gateway.github # noqa: F401
from app.channels.run_policy import CHANNEL_RUN_POLICY
policy = CHANNEL_RUN_POLICY.get("github")
assert policy is not None
assert policy.is_interactive is False
assert policy.default_recursion_limit == 250
assert policy.credentials_provider is not None
@pytest.mark.asyncio
async def test_apply_channel_policy_installs_token_for_github(monkeypatch: pytest.MonkeyPatch) -> None:
"""The unified policy hook installs the github credentials and the
non-interactive flag — both in one call.
The token in ``run_context`` is the minted **string**, not a closure,
so the value survives the langgraph SDK's JSON encoder on its way to
``client.runs.wait(context=…)``.
"""
manager = _new_manager()
mint = AsyncMock(return_value="ghs_unified")
monkeypatch.setattr("app.gateway.github.app_auth.mint_installation_token", mint)
run_context: dict = {}
await manager._apply_channel_policy(_github_msg(), run_context)
mint.assert_awaited_once_with(140594274)
assert run_context["github_token"] == "ghs_unified"
# Non-interactive flag is set in the same call — one method, one place.
assert run_context["disable_clarification"] is True
@pytest.mark.asyncio
async def test_apply_channel_policy_is_noop_for_unregistered_channels(monkeypatch: pytest.MonkeyPatch) -> None:
"""Slack/Telegram/etc. have no entry in CHANNEL_RUN_POLICY and stay untouched."""
manager = _new_manager()
mint = AsyncMock(return_value="should-not-be-called")
monkeypatch.setattr("app.gateway.github.app_auth.mint_installation_token", mint)
msg = InboundMessage(channel_name="slack", chat_id="C1", user_id="u", text="hi", metadata={})
run_context: dict = {}
await manager._apply_channel_policy(msg, run_context)
mint.assert_not_awaited()
assert "github_token" not in run_context
assert "disable_clarification" not in run_context
# ---------------------------------------------------------------------------
# _create_thread honors preferred_thread_id
# ---------------------------------------------------------------------------
@pytest.mark.asyncio
async def test_create_thread_uses_preferred_thread_id() -> None:
manager = _new_manager()
msg = _github_msg()
created_kwargs: dict = {}
class _FakeClient:
class threads:
@staticmethod
async def create(**kwargs):
created_kwargs.update(kwargs)
return {"thread_id": "uuid5-fixed"}
with patch.object(manager, "_store_thread_id", new=AsyncMock()):
thread_id = await manager._create_thread(_FakeClient(), msg)
assert thread_id == "uuid5-fixed"
assert created_kwargs["thread_id"] == "uuid5-fixed"
assert "metadata" in created_kwargs
@pytest.mark.asyncio
async def test_create_thread_without_preferred_id_omits_thread_id_kwarg() -> None:
manager = _new_manager()
msg = InboundMessage(
channel_name="slack",
chat_id="C1",
user_id="u",
text="hi",
metadata={}, # no preferred_thread_id
)
created_kwargs: dict = {}
class _FakeClient:
class threads:
@staticmethod
async def create(**kwargs):
created_kwargs.update(kwargs)
return {"thread_id": "random-from-gateway"}
with patch.object(manager, "_store_thread_id", new=AsyncMock()):
await manager._create_thread(_FakeClient(), msg)
assert "thread_id" not in created_kwargs
@pytest.mark.asyncio
async def test_create_thread_handles_race_on_preferred_id() -> None:
"""Two concurrent deliveries for the same (repo, number) collide on the
deterministic thread id. The losing writer hits a 409 ConflictError
from the underlying thread_store; we verify the thread actually exists
and reuse the deterministic id rather than dropping the run.
"""
manager = _new_manager()
msg = _github_msg() # carries preferred_thread_id="uuid5-fixed"
class _FakeClient:
class threads:
@staticmethod
async def create(**kwargs):
raise _make_conflict_error()
@staticmethod
async def get(thread_id, **kwargs):
# The winning concurrent create succeeded; threads.get
# confirms the row is there before we cache the mapping.
return {"thread_id": thread_id}
stored: dict = {}
async def _fake_store(_msg, thread_id):
stored["thread_id"] = thread_id
with patch.object(manager, "_store_thread_id", new=_fake_store):
thread_id = await manager._create_thread(_FakeClient(), msg)
# Recovered: returns the deterministic id and persisted the mapping.
assert thread_id == "uuid5-fixed"
assert stored["thread_id"] == "uuid5-fixed"
@pytest.mark.asyncio
async def test_create_thread_non_conflict_failure_propagates_and_does_not_poison_store() -> None:
"""Regression pin for willem-bd #1 on PR #3754.
A transient DB/network failure on threads.create (anything other than a
409 ConflictError) must propagate cleanly. Previously this branch
swallowed bare Exception and wrote ``preferred_thread_id`` into the
store, mapping every subsequent webhook for the same (repo, PR) to a
thread that never existed — runs.create then 404'd forever with no
retry path.
The narrow ``except ConflictError`` lets non-conflict failures surface
so the caller fails the delivery cleanly and the store stays clean.
"""
manager = _new_manager()
msg = _github_msg() # carries preferred_thread_id="uuid5-fixed"
class _FakeClient:
class threads:
@staticmethod
async def create(**kwargs):
# Anything that is NOT ConflictError: connection error, 500
# from the underlying store, JSON decode error, etc.
raise RuntimeError("HTTP 500: Failed to create thread")
@staticmethod
async def get(thread_id, **kwargs):
# Should never be called on the non-conflict path.
raise AssertionError("threads.get must not be called on non-conflict failure")
store_calls: list = []
async def _fake_store(_msg, thread_id):
store_calls.append(thread_id)
with patch.object(manager, "_store_thread_id", new=_fake_store):
with pytest.raises(RuntimeError, match="500"):
await manager._create_thread(_FakeClient(), msg)
# The mapping must NOT have been written — that was the bug.
assert store_calls == []
@pytest.mark.asyncio
async def test_create_thread_conflict_with_get_failure_propagates_and_does_not_poison_store() -> None:
"""If ConflictError fires but the follow-up threads.get also fails, the
store underneath is in an inconsistent state. Surfacing the failure is
better than caching a mapping to a thread that may not exist — every
future delivery on this issue/PR would 404 forever.
"""
manager = _new_manager()
msg = _github_msg()
class _FakeClient:
class threads:
@staticmethod
async def create(**kwargs):
raise _make_conflict_error()
@staticmethod
async def get(thread_id, **kwargs):
# Conflict was reported but the thread is not actually there.
raise RuntimeError("HTTP 404: thread not found")
store_calls: list = []
async def _fake_store(_msg, thread_id):
store_calls.append(thread_id)
with patch.object(manager, "_store_thread_id", new=_fake_store):
with pytest.raises(RuntimeError, match="404"):
await manager._create_thread(_FakeClient(), msg)
# No mapping was cached — that's the whole point of the verify step.
assert store_calls == []
@pytest.mark.asyncio
async def test_create_thread_without_preferred_id_propagates_error() -> None:
"""Without a deterministic id we have no recovery anchor — the original
exception must surface so the dispatch loop can handle/report it.
"""
manager = _new_manager()
msg = InboundMessage(
channel_name="slack",
chat_id="C1",
user_id="u",
text="hi",
metadata={}, # no preferred_thread_id
)
class _FakeClient:
class threads:
@staticmethod
async def create(**kwargs):
raise RuntimeError("HTTP 500: Failed to create thread")
with patch.object(manager, "_store_thread_id", new=AsyncMock()):
with pytest.raises(RuntimeError, match="500"):
await manager._create_thread(_FakeClient(), msg)