Skip to content

@jules fix: #684

@badMade

Description

@badMade

@jules fix:
The failing job is dominated by one broad regression: shared session-context helpers were renamed/refactored, but callers and tests still expect the old parameter names and symbols.

Most failures point to this directly:

  • NameError: name 'team_id' is not defined
  • NameError: name 'user_id' is not defined
  • TypeError: set_session_vars() got an unexpected keyword argument 'user_id'

The best fix is to restore backward-compatible session/context APIs in the session context module used by:

  • tests/gateway/test_session_env.py
  • many gateway authorization tests
  • cron / cleanup tests

Primary fix

Your tests import from gateway.session_context and expect set_session_vars(...) to accept:

  • platform
  • chat_id
  • chat_name
  • user_id
  • user_name
  • thread_id
  • session_key
  • terminal_cwd

They also expect matching contextvars/env lookups for:

  • HERMES_SESSION_USER_ID
  • HERMES_SESSION_USER_NAME

So update gateway/session_context.py to restore those fields and ensure the variable map contains all expected names.

Suggested shape:

import contextvars
import os

_UNSET = object()

_PLATFORM = contextvars.ContextVar("HERMES_SESSION_PLATFORM", default=_UNSET)
_CHAT_ID = contextvars.ContextVar("HERMES_SESSION_CHAT_ID", default=_UNSET)
_CHAT_NAME = contextvars.ContextVar("HERMES_SESSION_CHAT_NAME", default=_UNSET)
_USER_ID = contextvars.ContextVar("HERMES_SESSION_USER_ID", default=_UNSET)
_USER_NAME = contextvars.ContextVar("HERMES_SESSION_USER_NAME", default=_UNSET)
_THREAD_ID = contextvars.ContextVar("HERMES_SESSION_THREAD_ID", default=_UNSET)
_SESSION_KEY = contextvars.ContextVar("HERMES_SESSION_KEY", default=_UNSET)
_TERMINAL_CWD = contextvars.ContextVar("TERMINAL_CWD", default=_UNSET)

_VAR_MAP = {
    "HERMES_SESSION_PLATFORM": _PLATFORM,
    "HERMES_SESSION_CHAT_ID": _CHAT_ID,
    "HERMES_SESSION_CHAT_NAME": _CHAT_NAME,
    "HERMES_SESSION_USER_ID": _USER_ID,
    "HERMES_SESSION_USER_NAME": _USER_NAME,
    "HERMES_SESSION_THREAD_ID": _THREAD_ID,
    "HERMES_SESSION_KEY": _SESSION_KEY,
}

def set_session_vars(
    *,
    platform=None,
    chat_id=None,
    chat_name=None,
    user_id=None,
    user_name=None,
    thread_id=None,
    session_key=None,
    terminal_cwd=None,
):
    tokens = {}

    def _set_if_provided(var, value):
        if value is not None:
            tokens[var] = var.set("" if value is None else str(value))

    _set_if_provided(_PLATFORM, platform)
    _set_if_provided(_CHAT_ID, chat_id)
    _set_if_provided(_CHAT_NAME, chat_name)
    _set_if_provided(_USER_ID, user_id)
    _set_if_provided(_USER_NAME, user_name)
    _set_if_provided(_THREAD_ID, thread_id)
    _set_if_provided(_SESSION_KEY, session_key)

    if terminal_cwd is not None:
        tokens[_TERMINAL_CWD] = _TERMINAL_CWD.set(str(terminal_cwd))

    return tokens

def clear_session_vars(tokens):
    for var in _VAR_MAP.values():
        if var in tokens:
            var.reset(tokens[var])
        else:
            var.set("")
    if _TERMINAL_CWD in tokens:
        _TERMINAL_CWD.reset(tokens[_TERMINAL_CWD])
    else:
        _TERMINAL_CWD.set("")

def get_session_env(name, default=""):
    var = _VAR_MAP.get(name)
    if var is None:
        return os.getenv(name, default)
    value = var.get()
    if value is _UNSET:
        return os.getenv(name, default)
    return value

def get_terminal_cwd(default=None):
    value = _TERMINAL_CWD.get()
    if value is _UNSET:
        return os.getenv("TERMINAL_CWD", default if default is not None else os.getcwd())
    if value == "":
        return default if default is not None else os.getcwd()
    return value

Why this is the right fix

tests/gateway/test_session_env.py explicitly requires:

  • set_session_vars(user_id=..., user_name=...)
  • get_session_env("HERMES_SESSION_USER_ID")
  • _VAR_MAP to include those entries
  • _TERMINAL_CWD to remain _UNSET if terminal_cwd was not passed
  • clear_session_vars() to suppress env fallback by setting cleared vars to ""

Those expectations are visible in the test file:

  • tests/gateway/test_session_env.py
  • https://github.com/badMade/hermes-agent/blob/90957d1946489ba953c871f024b33c9364995e20/tests/gateway/test_session_env.py

This same missing API likely causes the large cluster of team_id/user_id NameErrors across gateway tests.

Secondary fix for team_id

The team_id failures strongly suggest a similar refactor bug in an auth/session helper where code still references team_id after it was removed or renamed. Since the failures span many gateway auth tests, look for a helper building authorization/session vars and restore a safe default, e.g.:

team_id = getattr(source, "team_id", None) or ""

or, if team scoping is optional, remove the bare reference and guard it:

if getattr(source, "team_id", None):
    ...

The key is: no bare team_id reference should exist unless it is defined locally or read from source/config.

Additional concrete fixes from this job

1. Webhook placeholder secret validation

Failing test:

  • tests/gateway/test_webhook_adapter.py::TestValidateSignature::test_validate_placeholder_secret_rejects_literal_hmac

Current validation in gateway/platforms/webhook.py accepts any non-empty secret and computes HMAC directly:

  • gateway/platforms/webhook.py:589
  • https://github.com/badMade/hermes-agent/blob/90957d1946489ba953c871f024b33c9364995e20/gateway/platforms/webhook.py

Add a guard to reject placeholder/env-template secrets such as ${WEBHOOK_SECRET}:

def _looks_unresolved_secret(secret: str) -> bool:
    s = (secret or "").strip()
    return bool(re.fullmatch(r"\$\{[A-Za-z_][A-Za-z0-9_]*\}", s))

def _validate_signature(self, request, body, secret: str) -> bool:
    if _looks_unresolved_secret(secret):
        logger.warning("[webhook] Unresolved placeholder secret configured")
        return False
    ...

2. ddgs wiring regression

Tests expect _ddgs_package_available, but implementation renamed it to _ddgs_package_importable.

Simplest compatibility fix:

def _ddgs_package_available() -> bool:
    return _ddgs_package_importable()

Then use that name consistently in:

  • _get_backend()
  • _is_backend_available()

This will fix the monkeypatch-based tests.

3. ddgs auto-detect behavior

One test expects ddgs not to become the last-resort auto-detected backend. Right now _get_backend() includes:

("ddgs", _ddgs_package_importable()),

and returns the first available backend. That makes ddgs win when nothing else is configured.

If you want tests to pass, remove ddgs from fallback auto-detection and allow it only when explicitly configured:

if configured in {"parallel", "firecrawl", "tavily", "exa", "searxng", "brave-free", "ddgs"}:
    return configured

backend_candidates = (
    ("firecrawl", ...),
    ("parallel", ...),
    ("tavily", ...),
    ("exa", ...),
    ("searxng", ...),
    ("brave-free", ...),
)

Keep _is_backend_available("ddgs") and check_web_api_key() support for explicit config.

4. SSRF error wording regression

Tests expect "private or internal address" but current code returns "private/internal address" after redirect in tools/web_tools.py:1377, 1707, 1810.

Normalize the message everywhere to the tested text:

"Blocked: URL targets a private or internal network address"

and for redirect cases avoid the alternate phrasing:

"Blocked: redirect landed on a private or internal address"

Use one canonical string if tests assert on substrings.

Minimal high-value patch set

If you want the fastest path to stabilizing this PR, do these first:

  1. Restore backward-compatible gateway.session_context API with user_id/user_name and proper clear/fallback semantics.
  2. Fix all bare team_id references by defining/guarding them.
  3. Add _ddgs_package_available() compatibility alias and stop ddgs last-resort auto-detect.
  4. Reject unresolved webhook placeholder secrets.
  5. Normalize SSRF error strings.

Relevant files

The root cause is not the workflow; it’s source incompatibility introduced by a refactor. Restoring compatibility in session/auth helpers should eliminate the majority of the 140 failures at once.

Originally posted by @badMade in #682 (comment)

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions