fix(approval): use per-job ContextVar for cron-session flag instead of leaking env var (#56771) - #650
Open
hashbender wants to merge 1 commit into
Open
fix(approval): use per-job ContextVar for cron-session flag instead of leaking env var (#56771)#650hashbender wants to merge 1 commit into
hashbender wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
execute_code(and dangerous-command guards) were blocked in interactive gateway/CLI/TUI sessions wheneverHERMES_CRON_SESSION=1was present in the process environment — even though the user never ran a cron job in that session (NousResearch#56771).Root cause
cron/scheduler.pysetos.environ["HERMES_CRON_SESSION"] = "1"process-wide at job start and never cleared it. The approval system then gates onenv_var_enabled("HERMES_CRON_SESSION")at 4 sites. When the gateway and scheduler share a process (the normal architecture), the env var leaks via inheritance into every concurrent interactive session, so the approval system treats user chats as cron and blocksexecute_code/ dangerous commands.This was the only place in the codebase that set the var, and
approval.pywas the only consumer — so replacing the process-global set has no other side effects.Fix
Replace the process-global env var with a task-local
ContextVarso the cron flag cannot leak into concurrent interactive sessions:gateway/session_context.py— add_CRON_SESSIONContextVar (mirroring the existing_UNSETsentinel pattern of the session vars) plusset_cron_session()/clear_cron_session()/is_cron_session()helpers.is_cron_session()checks the ContextVar first and falls back to the env var only for backward compat with tests / CLI cron paths that set it directly (in production the scheduler no longer sets it, so interactive sessions fall through toFalse).cron/scheduler.py— replace theos.environset withset_cron_session(True); addclear_cron_session()to therun_job()finally block. The flag is set before the scheduler's existingcopy_context()(line 2861) that the agent thread runs inside (_cron_context.run(agent.run_conversation)), so real cron jobs still seeTrueandcron_mode: denykeeps working.tools/approval.py— add_is_cron_session()(contextvar-aware, lazy-imported like the existingget_session_envcalls) and replace all 4env_var_enabled("HERMES_CRON_SESSION")checks with it.Why ContextVar and not env-var reordering
The scheduler sets the cron flag before spawning the agent in a worker thread via
copy_context().run(...). ContextVars propagate downward throughctx.run, so the agent thread seesTrue; concurrent interactive sessions run in their own tasks/contexts where the var is_UNSET, so they resolve toFalse. By contrastos.environis shared across all threads/process-children, which is precisely the leak this fixes. Competing approaches that merely reorder the gateway/cron precedence checks (or gate on interactive indicators) leave the env var leaking into spawned subprocesses and reverse the original design invariant that cron takes absolute priority over gateway approval.How verified
tests/tools/test_cron_session_leak.py): contextvar resolution, the core thread-isolation mechanism (scheduler thread sets the flag, concurrent gateway thread does not see it), env-var fallback for backward compat, contextvar-overrides-leaked-env, and the fullcheck_execute_code_guard+_is_gateway_approval_contextmatrix (cron blocks, interactive allowed, interactive-after-scheduler-ran still allowed).main(0 1).Closes NousResearch#56771.
Auto-published by Moonsong via Path B automated pipeline.
Mirror-of: NousResearch#56796
NousResearch#56796