fix(cron): null-safe deliver in cron list + re-resolve BSM secrets per run - #56144
Merged
Merged
Conversation
…r run Two live cron bugs, both surfaced by @banditburai in #35616 (whose larger watchdog/supervisor work is already superseded by the CronScheduler provider refactor on main): - #32896: `cron list` crashed on a present-but-null `deliver` field — `job.get("deliver", ["local"])` returns None for an explicit null, which then hit `", ".join(None)`. Coalesce with `or ["local"]` (same pitfall the sibling `repeat` line already guards against). - #33465: cron jobs 401'd on Bitwarden/BSM-backed secrets. The per-run env reload used a bare `load_dotenv(override=True)`, which re-applied only the .env placeholder — startup had already recorded this HERMES_HOME in env_loader._APPLIED_HOMES, so the external-secret re-pull no-oped. Route the reload through load_hermes_dotenv() and call reset_secret_source_cache() first to force the re-pull (Bitwarden's 300s value-cache keeps it off the network; override honours secrets.bitwarden.override_existing, mirroring startup). Tests: null-deliver regression guard in test_cron.py; reset-before-reload ordering guard in test_scheduler.py. Migrated 31 scheduler-reload test seams from patching dotenv.load_dotenv to the new load_hermes_dotenv / reset_secret_source_cache seam.
Migrating the scheduler-reload seam from a single dotenv.load_dotenv patch to two patches (load_hermes_dotenv + reset_secret_source_cache) lengthened the positional list _make_run_job_patches returns, so the 4 callers that applied patches[0..4] silently dropped the resolve_runtime_provider patch (now at [5]). Under CI's hermetic env (all API keys blanked) auth then failed and AIAgent was never constructed → 'NoneType has no attribute kwargs'. Callers now apply patches[0..5]. Passed locally (keys present) but failed on CI shard 5/8.
teknium1
force-pushed
the
fix/cron-null-deliver-bsm-reresolve
branch
from
July 1, 2026 07:48
b99e633 to
bbfbf70
Compare
Contributor
Related (same cron-fix cluster, focused live-only subset): #33527 (omnibus cron hardening), #34041 (BSM-only reload), #36863 (broader env refresh incl. MCP). This PR carries only the two parts of #35616 still live on |
This was referenced Jul 1, 2026
Closed
16 tasks
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.
Summary
Two live cron bugs — a
cron listcrash and cron jobs 401'ing on Bitwarden-backed secrets — are fixed. Both were surfaced by @banditburai in #35616, whose larger watchdog/supervisor rework is already superseded by theCronSchedulerprovider refactor onmain(the old in-process_start_cron_tickerthis PR reworked is now a deprecated shim;except BaseException+logger.error(exc_info=True), the heartbeat, and the honestcron statusbanner all landed since). These two adjacent one-liners were the only parts of that PR still live onmain; #33527 targets the same pair.Changes
hermes_cli/cron.py—cron listusedjob.get("deliver", ["local"]); an explicit"deliver": nullreturnsNone, which then hit", ".join(None)and crashed the whole listing. Coalesce withor ["local"]— the same present-but-null guard the siblingrepeatline already uses (fix: hermes cron list crashes when job has null deliver field #32896).cron/scheduler.py— the per-run env reload used a bareload_dotenv(override=True), which re-applied only the.envplaceholder: startup had already recorded thisHERMES_HOMEinenv_loader._APPLIED_HOMES, so the external-secret re-pull no-oped and Bitwarden/BSM secrets stayed stuck on the placeholder (cron jobs 401'd). Route the reload throughload_hermes_dotenv()and callreset_secret_source_cache()first to force the re-pull. Bitwarden's 300s value-cache keeps the forced re-pull off the network; the override honourssecrets.bitwarden.override_existing, mirroring startup ([Bug]: cron scheduler missing BSM resolution #33465).deliverregression guard intests/hermes_cli/test_cron.py; reset-before-reload ordering guard intests/cron/test_scheduler.py. Migrated 31 scheduler-reload test seams from patchingdotenv.load_dotenvto the newload_hermes_dotenv/reset_secret_source_cacheseam (the old seam no longer intercepts the reload, so leaving it would let the real BSM apply pollute env state across tests within a process).Validation
cron listwith"deliver": nullTypeError: can only join an iterableDeliver: local.envplaceholder re-applied, secret never re-resolved → 401reset_secret_source_cache()forces re-pull each runtests/cron/Fixes verified E2E with real imports against a temp
HERMES_HOME.Closes
cron listno longer crashes on a present-but-nulldeliver.Credit
@banditburai diagnosed both bugs in #35616. The watchdog/supervisor bulk of that PR is superseded by the provider refactor; this ships the two still-live fixes with tests. #33527 (@zccyman) targets the same pair — its BSM swap omits
reset_secret_source_cache, so secrets still wouldn't re-resolve in a long-running process; that's included here.Infographic