Skip to content

fix(cron): deliver profile-store jobs with the owning profile's bot under multiplex - #91795

Closed
vinoth12940 wants to merge 2 commits into
NousResearch:mainfrom
vinoth12940:fix/multiplex-cron-delivery-identity
Closed

vinoth12940 wants to merge 2 commits into
NousResearch:mainfrom
vinoth12940:fix/multiplex-cron-delivery-identity

Conversation

@vinoth12940

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes cron delivery identity for secondary profiles under a multiplexed gateway (multiplex_profiles: true): a job owned by profile X now delivers through profile X's own bot instead of the default profile's bot.

Implements the expected behavior from #83182 end-to-end:

  1. Secret scope held through delivery (cron/scheduler.py) — the job-owning profile's secret scope was previously reset in a finally around run_job alone, before _deliver_result ran. Delivery then resolved TELEGRAM_BOT_TOKEN with no scope active — falling back to os.environ (empty or another profile's value in a multiplex unit). The scope is now installed by the run_one_job wrapper and reset in the wrapper's finally, after the full execute → save → deliver → mark sequence (including the failure-path delivery in the body's outer except).

  2. Per-profile adapter map for the tick (cron/scheduler_provider.py) — the multiplex ticker passed the shared default-profile adapter map to every profile's cron_tick. It now resolves each profile's own map via the new profile_adapters kwarg (_profile_tick_adapters helper): secondary profiles get _profile_adapters[<name>]; the default profile — and profiles with no secondary adapters — keep the shared map, so single-profile gateways are unchanged.

  3. Gateway wiring (gateway/run.py) — start_gateway now passes runner._profile_adapters to the cron provider alongside the shared map (additive kwarg; external providers ignore it).

Related Issue

Fixes #83182

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✅ Tests (adding or improving test coverage)

Changes Made

  • cron/scheduler.py — move the profile secret-scope install/reset from _run_one_job_body (around run_job alone) to the run_one_job wrapper, so the scope spans the entire body including both _deliver_result call sites
  • cron/scheduler_provider.py — new profile_adapters kwarg on InProcessCronScheduler.start / _start_multiplex; _profile_tick_adapters(profile, shared, profile_adapters) resolves each tick's adapter map; the per-profile loop uses it
  • gateway/run.py — pass runner._profile_adapters (when populated) as profile_adapters in cron_start_kwargs under multiplex
  • tests/cron/test_cron_multiplex_delivery_identity.py — 7 regression tests: scope active at delivery time, scope torn down after completion, per-profile adapter resolution (own / default / unknown / unsupplied), and the multiplex tick passing the profile's own map to cron_tick

How to Test

Unit/regression (repo venv, verified):

pytest tests/cron/test_cron_multiplex_delivery_identity.py -q   # 7 passed
pytest tests/cron/ -q                                           # 0 new failures vs origin/main baseline
pytest tests/gateway/ -q -k "multiplex or profile"              # 253 passed, 2 skipped

E2E reproduction (how the bug was originally observed): on a multiplex gateway with a secondary profile trader that owns a Telegram bot token and a telegram:<chat>-delivered cron job in its store, fire the job on the scheduler — before the fix the message arrives from the default profile's bot (visible in the gateway log as a delivery flush keyed agent:main:...); with this PR it arrives from the trader bot's identity, and the log shows the tick's delivery routing through the profile-owned credential. Verified live on a 7-profile multiplex gateway (all scheduled briefings now deliver from their own bots).

Checklist

Code

@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 21, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference; please use your judgment.

  • cron/scheduler.py:6409-6427 — set_secret_scope(...) now runs before the _running_lock registration block, but the resetting finally belongs to a later try; any early return or raise between the install and that try (ownership/interrupt checks live in exactly that window) leaves the ticker thread permanently scoped to this profile, and every subsequently-run job — any profile — resolves ITS credentials through the wrong .env. Either move the install to immediately precede the try with its own try/finally pairing, or use an ExitStack, and add a test asserting the scope is gone after an ownership-lost early exit.
  • gateway/run.py:30791-30798 — getattr(runner, "_profile_adapters", None) makes the fix silently self-disabling: rename/remove that private attribute and multiplex delivery quietly falls back to the shared adapter map, i.e. Cron delivery uses wrong bot/chat under multiplex (secret-scope reset before delivery + shared-adapter dict) #83182 resurfaces with no error anywhere. Read the attribute explicitly (or promote it to a documented config field) and log a warning when multiplex is on but no per-profile map could be found.
  • tests/cron/test_cron_multiplex_delivery_identity.py:100 — tok = ss.set_secret_scope(None) if False else None is dead code left mid-test; it confuses readers about whether a scope side effect is intended. Delete the line.
  • tests/cron/test_cron_multiplex_delivery_identity.py:60-110 — contracts covered stop at the happy path; missing are (a) the leak scenario above (exception/early-exit between stages still resets the scope), and (b) an end-to-end assertion that a default-profile job under multiplex still delivers through the shared adapters — the unit matrix covers the resolver but not the wiring in run.py, which is where the getattr risk lives.
  • cron/scheduler_provider.py:694-701 — plain-string entries in profile_homes yield profile_name=None and therefore always fall back to shared adapters; that asymmetry (tuple entries get per-profile bots, bare paths never do) is undocumented at the profile_homes call sites. State the contract in the docstring or normalize entries upstream so both shapes resolve names. (nit)

load_gateway_config() reads TELEGRAM_BOT_TOKEN from os.environ, which under
multiplex holds the DEFAULT profile's token. The ticking profile's token
lives in the secret scope (installed by run_one_job), not os.environ. Patch
pconfig.token from get_secret() so the standalone delivery lane sends with
the correct profile's bot identity.

Fixes NousResearch#83182 (delivery identity)
@teknium1

Copy link
Copy Markdown
Collaborator

Closing after #99375 (merged, commit 9eb832a) landed the delivery-identity fix via earlier submitters @MuhammadUsamaMX (#73363) and @ghosty93 (#92526), whose versions carried tests. Same root cause, later submission. Thanks for the correct analysis.

@teknium1 teknium1 closed this Aug 31, 2026
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.

Cron delivery uses wrong bot/chat under multiplex (secret-scope reset before delivery + shared-adapter dict)

4 participants