Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions cron/scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -2517,7 +2517,7 @@ def run_job(job: dict) -> tuple[bool, str, str, Optional[str]]:

# Use ContextVars for per-job session/delivery state so parallel jobs
# don't clobber each other's targets (os.environ is process-global).
from gateway.session_context import set_session_vars, clear_session_vars, _VAR_MAP
from gateway.session_context import set_session_vars, clear_session_vars, reset_cron_delivery_vars, _VAR_MAP

Comment on lines 2518 to 2521
# Cron execution is an internal scheduler context, not a live inbound
# gateway message. Do not seed HERMES_SESSION_* contextvars from the
Expand Down Expand Up @@ -3113,8 +3113,12 @@ def run_job(job: dict) -> tuple[bool, str, str, Optional[str]]:
_terminal_cwd_lock.release_read()
# Clean up ContextVar session/delivery state for this job.
clear_session_vars(_ctx_tokens)
for _var_name in _cron_delivery_vars:
_VAR_MAP[_var_name].set("")
# Restore the cron-delivery vars to their *never set* (_UNSET) state, not
# "" — an empty string suppresses the os.environ fallback in
# get_session_env, so setting "" here would leak an empty cron-delivery
# value into every later caller in this thread/task (cross-run / cross-
# test pollution). reset_cron_delivery_vars() returns them to _UNSET.
reset_cron_delivery_vars()
if _session_db:
# Title the cron session from the job (name → short prompt → id) so
# sidebars/history show a meaningful label instead of the injected
Expand Down
22 changes: 22 additions & 0 deletions gateway/session_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -290,6 +290,28 @@ def reset_session_vars() -> None:
pass


def reset_cron_delivery_vars() -> None:
"""Restore the cron auto-delivery ContextVars to their *never set* state.

Distinct from setting them to ``""``: an empty string is the
"explicitly cleared" state that suppresses the ``os.environ`` fallback in
``get_session_env`` (correct WHILE a job runs, so a job's own
``send_message`` can detect a duplicate auto-delivery target). After a job
finishes, the vars must be returned to the ``_UNSET`` sentinel so the next
caller in the same thread/task falls back to ``os.environ`` again. Without
this, a cron run leaks an empty cron-delivery value into every later caller
in the process — observed as cross-test pollution where a later
``send_message`` no longer detects the cron duplicate-target case because
the ContextVar shadows the env the caller set.
"""
for var in (
_CRON_AUTO_DELIVER_PLATFORM,
_CRON_AUTO_DELIVER_CHAT_ID,
_CRON_AUTO_DELIVER_THREAD_ID,
):
var.set(_UNSET)


def get_session_env(name: str, default: str = "") -> str:
"""Read a session context variable by its legacy ``HERMES_SESSION_*`` name.

Expand Down
56 changes: 56 additions & 0 deletions tests/gateway/test_session_env.py
Original file line number Diff line number Diff line change
Expand Up @@ -394,3 +394,59 @@ async def test_gateway_executor_refuses_resurrection_after_shutdown():
finally:
runner._shutdown_executor()


# ---------------------------------------------------------------------------
# reset_cron_delivery_vars: cron auto-delivery vars must return to the *never
# set* (_UNSET) state after a job, NOT "" — otherwise an empty value leaks
# into every later same-thread caller and shadows the os.environ fallback.
# ---------------------------------------------------------------------------

from gateway.session_context import ( # noqa: E402
reset_cron_delivery_vars,
_CRON_AUTO_DELIVER_PLATFORM,
_CRON_AUTO_DELIVER_CHAT_ID,
_CRON_AUTO_DELIVER_THREAD_ID,
)

_CRON_DELIVERY_VARS = (
"HERMES_CRON_AUTO_DELIVER_PLATFORM",
"HERMES_CRON_AUTO_DELIVER_CHAT_ID",
"HERMES_CRON_AUTO_DELIVER_THREAD_ID",
)
Comment on lines +411 to +415


def test_reset_cron_delivery_vars_restores_unset_sentinel():
"""After a cron job, the delivery vars must hold _UNSET, not ""."""
# Simulate run_job setting the delivery target during execution.
_CRON_AUTO_DELIVER_PLATFORM.set("telegram")
_CRON_AUTO_DELIVER_CHAT_ID.set("-1001")
_CRON_AUTO_DELIVER_THREAD_ID.set("")

reset_cron_delivery_vars()

assert _CRON_AUTO_DELIVER_PLATFORM.get() is _UNSET
assert _CRON_AUTO_DELIVER_CHAT_ID.get() is _UNSET
assert _CRON_AUTO_DELIVER_THREAD_ID.get() is _UNSET


def test_reset_cron_delivery_vars_reenables_environ_fallback(monkeypatch):
"""The bug: a cron run set the vars to "" (the explicitly-cleared state),
which suppresses the os.environ fallback in get_session_env, so a later
same-thread caller read "" instead of the env it had set. After the fix,
reset_cron_delivery_vars() returns the vars to _UNSET so the fallback works.
"""
# A later caller (e.g. send_message) relies on the env var.
monkeypatch.setenv("HERMES_CRON_AUTO_DELIVER_PLATFORM", "discord")
monkeypatch.setenv("HERMES_CRON_AUTO_DELIVER_CHAT_ID", "999")

# Reproduce the OLD leak: a finished cron job left the vars at "".
_CRON_AUTO_DELIVER_PLATFORM.set("")
_CRON_AUTO_DELIVER_CHAT_ID.set("")
# With "" set, the contextvar shadows os.environ — fallback suppressed.
assert get_session_env("HERMES_CRON_AUTO_DELIVER_PLATFORM") == ""
assert get_session_env("HERMES_CRON_AUTO_DELIVER_CHAT_ID") == ""

# The fix restores _UNSET, so get_session_env falls back to os.environ.
reset_cron_delivery_vars()
assert get_session_env("HERMES_CRON_AUTO_DELIVER_PLATFORM") == "discord"
assert get_session_env("HERMES_CRON_AUTO_DELIVER_CHAT_ID") == "999"
Loading