Skip to content

fix(cron): delivery uses owning profile's secret scope + adapters under multiplex - #83197

Open
mjshorty wants to merge 3 commits into
NousResearch:mainfrom
mjshorty:fix/cron-delivery-secret-scope-multiplex
Open

fix(cron): delivery uses owning profile's secret scope + adapters under multiplex#83197
mjshorty wants to merge 3 commits into
NousResearch:mainfrom
mjshorty:fix/cron-delivery-secret-scope-multiplex

Conversation

@mjshorty

Copy link
Copy Markdown

Summary

Fixes cron delivery using the wrong bot/chat under multiplex (BUG 1): a multiplexed secondary-profile cron job delivered with the owning profile's secret scope torn down and the shared default-profile adapter map — so the wrong/empty TELEGRAM_BOT_TOKEN was used and the original thread was dropped ("Thread not found").

Two root causes, two parts:

Part 1 — profile secret scope dropped before delivery

run_one_job() installed the job profile's secret scope, but reset_secret_scope(_scope_token) ran in the run-job finally before _deliver_result. At delivery time current_secret_scope() was None, so load_gateway_config() → _getenv only fell back to os.environ (no profile scope) and the owning profile's .env was ignored.

Fix: keep the scope installed through delivery — remove the inner finally reset, reset it in the outer delivery-block finally (after save + delivery). The run-job except path still resets before propagating.

Part 2 — delivery only used the shared default adapter map

The gateway started the cron ticker with cron_start_kwargs = {"adapters": runner.adapters, ...} (the shared default-profile dict). _deliver_result / DeliveryRouter never consulted Gateway._profile_adapters[<job profile>], so even with the right token a secondary-profile job couldn't reach its live adapter.

Fix: thread the gateway's per-profile adapter map through the cron scheduler:

  • New _deliver_adapters_for_job() resolves Gateway._profile_adapters[profile] (mirroring gateway/authz_mixin.py), falling back to the shared adapters dict for the default profile / non-multiplex path.
  • Thread profile_adapters through run_one_job → _deliver_result.
  • Thread profile_adapters through CronScheduler.start / fire_due / InProcessCronScheduler.start / _start_multiplex → tick → run_one_job.
  • gateway/run.py passes runner._profile_adapters into cron_start_kwargs.

Tests

  • test_run_one_job_keeps_secret_scope_through_delivery — asserts the profile secret scope is still installed (and the profile secret resolves) at _deliver_result time. Fails on old code, passes with the fix.
  • test_run_one_job_delivery_uses_profile_adapters / ..._falls_back_to_shared_adapters_for_default — assert _deliver_adapters_for_job selects the profile map vs the shared fallback.
  • test_run_one_job_threads_profile_adapters_to_delivery — asserts the gateway's per-profile map reaches _deliver_result.

Verification

  • python -m pytest tests/cron/ -q → 519 passed, 1 skipped
  • python -m pytest tests/gateway/test_multiplex_adapter_registry.py tests/gateway/test_cron_fire_webhook.py tests/gateway/test_multiplex_profile_authz.py -q → 25 passed
  • Pre-existing gateway failures (config/matrix/telegram/session-store) reproduce on baseline main; the 4 discord flakes pass in isolation — none introduced by this change.
  • python -c "import cron.scheduler, gateway.run" → clean

@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/cron Cron scheduler and job management comp/gateway Gateway runner, session dispatch, delivery platform/telegram Telegram bot adapter area/auth Authentication, OAuth, credential pools area/config Config system, migrations, profiles area/profiles Multi-profile isolation, HERMES_HOME scoping sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades labels Aug 10, 2026
@ghosty-11

ghosty-11 commented Aug 11, 2026

Copy link
Copy Markdown

I rebased this patch locally onto current NousResearch/main (f51aa6a9b) as 994ffa82c. The intended scope-lifetime fix is sound, and the relevant suite is green:

python -m pytest tests/cron tests/gateway/test_cron_fire_webhook.py tests/hermes_cli/test_web_server_cron_profiles.py tests/plugins/test_chronos_cron.py -q
537 passed, 1 skipped

I found two blocking edge cases that the current tests do not exercise:

  1. The new startup kwarg breaks external providers. gateway/run.py adds profile_adapters to cron_start_kwargs whenever runner._profile_adapters is non-empty, before the InProcessCronScheduler check. The bundled ChronosCronScheduler.start(stop_event, *, adapters=None, loop=None, interval=60) does not accept it. This also conflicts with cron/scheduler_provider.py's interface rule that start() signature growth must not break providers.
ChronosCronScheduler().start(..., profile_adapters={"secondary": {}})
TypeError: ChronosCronScheduler.start() got an unexpected keyword argument 'profile_adapters'

Please pass this kwarg only to InProcessCronScheduler (the only provider consuming the in-process live-adapter registry), rather than widening the generic provider call, and add a multiplex + external-provider startup regression.

  1. A missing/empty secondary-profile adapter map falls back to the default profile's live adapter. For a non-default profile, _deliver_adapters_for_job() returns shared adapters when profile_adapters is None, lacks the profile, contains {profile: {}}, or profile inference raises. That recreates the wrong-identity path precisely when the per-profile registry is incomplete:
profile=secondary, shared={discord: root_adapter}
profile_adapters={root: shared}  -> root_adapter
profile_adapters={secondary: {}} -> root_adapter
profile_adapters=None            -> root_adapter

For any resolved non-default profile, please return its non-empty map or {}—never the default profile map. {} still permits the existing standalone delivery path to use the active profile's scoped credential. On profile-resolution failure, the authority-safe fallback is also {}. Add missing, empty, None, and resolver-error cases; the current helper tests cover only present-profile and default-profile success.

I would also add scope-reset tests for save_job_output raising and delivery raising a BaseException; the new finally appears correct, but the regression contract currently proves only the successful delivery path.

These are merge blockers for a multiplex deployment because one causes a deterministic provider startup TypeError and the other can cross profile identity boundaries. No model, network delivery, or service restart was used for these probes.

ayushnangia added a commit to ayushnangia/hermes-agent that referenced this pull request Aug 14, 2026
…ousResearch#80921)

Deterministic, LLM-free conformance cells against the real SessionDB with
real SIGKILL mid-write, per the tracking issue's spot-probe method:

- cell 1: acknowledged-append durability + recovery determinism (adapted
  from the issue's 29.5K probe, scaled kill window, identical assertions)
- cell 2: consume-once under 8-process concurrent claim_handoff
- cell 3 (new): compression-rotation atomicity — never a compression-ended
  parent without a continuation (NousResearch#80337 contract; NousResearch#80487 recovery context)
- cells 4-5: documented stubs interlocked with NousResearch#82956-NousResearch#82959 and
  NousResearch#83197/NousResearch#83557

Journal-mode matrix (resolver default / DELETE / WAL-with-skip-gate) per
cell; every wait deadline-bounded; writers asserted alive at kill time.
ayushnangia added a commit to ayushnangia/hermes-agent that referenced this pull request Aug 14, 2026
One xfail per open member of the class mapped on NousResearch#82936: profile-scoped
state resolved from ambient process state at use time instead of bound
to the owning profile/session at creation. Test-only; fixes nothing;
flips to XPASS as per-site fixes land.

Members: NousResearch#82936 (multiplex terminal env), NousResearch#81952 (corrupt config silent
fallback), NousResearch#83346 (ambient session-key profile), NousResearch#80318 (profile scope
hides root MoA presets), NousResearch#83197/NousResearch#83557 (cron delivery scope reset before
delivery).
@mjshorty
mjshorty force-pushed the fix/cron-delivery-secret-scope-multiplex branch from 411557c to 4e15112 Compare August 18, 2026 08:59
@mjshorty

Copy link
Copy Markdown
Author

rebased onto current main

@mjshorty

Copy link
Copy Markdown
Author

rebased onto current main (follow-up: thread profile_adapters through _run_one_job_body so per-profile delivery works with the newer scheduler refactor)

…er multiplex

BUG 1 root cause: run_one_job() reset the profile secret scope in the inner
finally BEFORE _deliver_result ran, so at delivery time current_secret_scope()
was None. load_gateway_config -> _getenv reads TELEGRAM_BOT_TOKEN through the
scope and only falls back to os.environ when NO scope is installed — so a
multiplexed job's owning profile .env was ignored at delivery, producing the
wrong/empty token (wrong bot/chat, dropped thread).

Additionally, the gateway started the cron ticker with only the SHARED
default-profile adapters dict; _deliver_result / DeliveryRouter never
consulted Gateway._profile_adapters[<job profile>], so even with the right
token a secondary-profile cron job couldn't reach its live adapter.

PART 1: keep the profile secret scope installed THROUGH delivery.
- Remove reset_secret_scope from the inner run_job finally.
- Reset it in the delivery block's outer finally (after save + delivery),
  so run AND delivery both run inside the owning profile's scope.
- The run_job except path still resets the scope before propagating (that
  path never reaches the delivery finally).

PART 2: route cron delivery through the job profile's live adapters.
- New _deliver_adapters_for_job() resolves Gateway._profile_adapters[profile]
  (mirroring authz_mixin) with fallback to the shared adapters dict for the
  default profile / non-multiplex path.
- Thread profile_adapters through run_one_job -> _deliver_result.
- Thread profile_adapters through CronScheduler.start / fire_due /
  InProcessCronScheduler.start / _start_multiplex -> tick -> run_one_job.
- gateway/run.py passes runner._profile_adapters into cron_start_kwargs.

Tests: run_one_job keeps secret scope through delivery; _deliver_adapters_for_job
selects profile adapters vs shared fallback; run_one_job threads the map through.
Cron suite 519 passed; gateway multiplex/cron-fire 25 passed.
…iplex

Addresses ghosty-11's review on NousResearch#83197 (merge blockers for multiplex):

1. gateway/run.py no longer injects ``profile_adapters`` into the generic
   cron provider start kwargs. The kwarg is now passed ONLY to
   InProcessCronScheduler (the sole consumer), via a testable
   ``_build_cron_start_kwargs`` helper. External providers (Chronos) keep
   their ``start(stop_event, *, adapters, loop, interval)`` contract — a
   multiplex deployment with an external provider previously crashed with
   a deterministic TypeError.

2. _deliver_adapters_for_job no longer falls back to the default
   profile's shared adapter map for a non-default profile. A missing,
   empty, or unresolvable per-profile registry now resolves to {} (never
   the shared map), so delivery cannot cross profile identity boundaries.
   The standalone delivery path still works off the active profile's
   scoped credential.

3. ChronosCronScheduler.fire_claimed accepts and forwards
   profile_adapters (provider interface growth is additive).

4. Regression tests: external-provider kwarg isolation, in-process
   profile_adapters wiring, missing/empty/None/resolver-error profile
   map handling, standalone fallback with {}, and scope-reset guarantees
   when save_job_output raises or delivery raises a BaseException.
@mjshorty
mjshorty force-pushed the fix/cron-delivery-secret-scope-multiplex branch from 12676ea to 29ed094 Compare August 20, 2026 20:46
@mjshorty

Copy link
Copy Markdown
Author

Thanks for the detailed review — both blocking points are addressed in the updated branch (rebased onto current main).

1. Provider kwarg break (external providers). gateway/run.py no longer injects profile_adapters into the generic cron_start_kwargs. The map is now threaded through a testable _build_cron_start_kwargs helper that only adds profile_adapters/profile_homes/can_dispatch when the resolved provider is InProcessCronScheduler. ChronosCronScheduler.start() keeps its (stop_event, *, adapters, loop, interval) contract unchanged. Added regression tests:

  • test_cron_start_kwargs_external_provider_gets_no_profile_kwargs — a Chronos-like provider never receives the in-process-only kwargs
  • test_cron_start_kwargs_inprocess_gets_profile_adapters_under_multiplex — the positive path still wires the map
  • test_cron_start_kwargs_inprocess_without_registry_omits_key

2. Missing/empty per-profile map falls back to the default profile's adapter. _deliver_adapters_for_job now returns {} — never the shared default map — for ANY resolved non-default profile whose registry is missing, empty, or unresolvable (guard is now is not None, so an explicitly empty profile_adapters dict is still treated as multiplex). {} preserves the standalone path, which uses the active profile's scoped credential. Regressions:

  • missing profile entry → {}
  • empty profile map → {}
  • profile_adapters=None → shared map (unchanged non-multiplex behavior)
  • resolver error → {}
  • end-to-end: {} reaches resolve_delivery_transport so standalone delivery uses the owning profile's token

Also:

  • ChronosCronScheduler.fire_claimed accepts + forwards profile_adapters (additive growth; fixes the TypeError the new kwarg caused for the external provider's inbound fire path).
  • Added the requested scope-reset regressions: save_job_output raising and delivery raising a BaseException both still tear the profile secret scope down in the finally.

Test results on the rebased branch (current main base):

tests/cron tests/gateway/test_cron_fire_webhook.py \
tests/hermes_cli/test_web_server_cron_profiles.py \
tests/plugins/test_chronos_cron.py -q
883 passed, 1 skipped

The single failure (test_scheduler_cron_session_isolation.py::...pollute_later_gateway_execute_code) is pre-existing on upstream main and unrelated to this PR's diff.

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

Labels

area/auth Authentication, OAuth, credential pools area/config Config system, migrations, profiles area/profiles Multi-profile isolation, HERMES_HOME scoping comp/cron Cron scheduler and job management comp/gateway Gateway runner, session dispatch, delivery P2 Medium — degraded but workaround exists platform/telegram Telegram bot adapter sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants