Skip to content

fix: restore cron approval env after job runs - #46167

Closed
renanpsantos wants to merge 1 commit into
NousResearch:mainfrom
renanpsantos:fix/cron-approval-env-leak
Closed

fix: restore cron approval env after job runs#46167
renanpsantos wants to merge 1 commit into
NousResearch:mainfrom
renanpsantos:fix/cron-approval-env-leak

Conversation

@renanpsantos

Copy link
Copy Markdown

Summary

  • save the previous HERMES_CRON_SESSION value before cron jobs set it
  • restore or unset the variable in _run_job_impl() cleanup
  • prevent cron approval mode from leaking into normal gateway/chat sessions after a cron run

Root cause

HERMES_CRON_SESSION was being set process-wide for cron execution and left behind afterward. When the same gateway process later handled a normal chat session, approval logic could still see the cron flag and apply the headless cron policy incorrectly.

Test Plan

  • venv/bin/python -m pytest tests/tools/test_cron_approval_mode.py tests/tools/test_execute_code_approval_cluster.py -q
  • manually verified execute_code works again outside cron after restarting the gateway

@alt-glitch alt-glitch added type/bug Something isn't working comp/cron Cron scheduler and job management duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists labels Jun 14, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

Duplicate of #29854 — same fix: save/restore HERMES_CRON_SESSION around cron job execution so the cron approval policy doesn't leak into subsequent interactive gateway sessions. #29854 is the earliest still-open PR in this cluster (see also #35515, #43549).

@liuhao1024

Copy link
Copy Markdown
Contributor

Verification: reviewed diff + tests — no issues found.

Checked:

  • Env var save/restore in _run_job_impl correctly captures _prior_cron_session before setting HERMES_CRON_SESSION=1
  • finally block restores the prior value (or pops if unset), preventing cron approval policy from leaking into non-cron gateway/chat sessions in the same process
  • The sentinel "_UNSET_" correctly distinguishes "was not set" from "was set to empty string"
  • Test coverage: the change is small and self-contained; the finally pattern matches the existing TERMINAL_CWD restore just below it

This is a clean fix for a real env-var-leak bug that would cause all sessions in a shared scheduler process to inherit headless cron approval behavior.

@renanpsantos

Copy link
Copy Markdown
Author

Closing this as a duplicate of #29854. I hit the same root cause locally (HERMES_CRON_SESSION leaking across cron and interactive gateway sessions), validated the behavior on my side, and will follow #29854 instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cron Cron scheduler and job management duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants