fix(cron): multiplex delivery uses owning profile's bot token and adapter (#83182) - #83557
dongjiang1989 wants to merge 1 commit into
Conversation
|
cc @alt-glitch PTAL, thanks |
…pter (NousResearch#83182) Two root causes under multiplex gateway: 1. Secret scope reset before delivery: run_one_job's inner finally block reset the profile's secret scope after run_job returned but BEFORE _deliver_result ran. load_gateway_config → _getenv fell back to os.environ (empty in the multiplex unit) — TELEGRAM_BOT_TOKEN resolved to the wrong bot and delivery routed to the wrong chat. Fix: move reset_secret_scope to an outer finally so the scope stays installed through BOTH execution and delivery. 2. Shared adapters dict for delivery: The cron ticker received only runner.adapters (default profile's live adapters). Secondary-profile jobs therefore delivered via the default profile's adapter — even though Gateway._profile_adapters[profile] had the right per-profile adapter live and ready. Fix: gateway/run.py builds profile_adapters_by_home (resolved hermes home path → per-profile adapter map) and passes it through the cron chain: scheduler_provider → tick → run_one_job → _deliver_result. run_one_job resolves the owning profile's adapter map via the current hermes home (set by _start_multiplex's per-profile override), so delivery picks the right bot token. Files changed: cron/scheduler.py - scope restructure + profile_adapters param chain cron/scheduler_provider.py - propagate profile_adapters_by_home gateway/run.py - build profile_adapters_by_home from runner tests/cron/test_cron_multiplex_delivery_83182.py - 6 new regression tests tests/cron/test_run_one_job.py - fake_deliver signature tests/cron/test_preflight_config.py - fake_deliver signature Fixes NousResearch#83182 Signed-off-by: dongjiang <dongjiang1989@126.com>
bf17819 to
0d4ceb7
Compare
…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.
fix(cron): multiplex delivery uses owning profile's bot token and adapter (#83182)
|
|
Thanks for the review. Analyzed all 3 points: 1. Tuple unpacking — not an issue. 2. Stale adapter snapshot — not an issue in practice. Adapter instances in 3. Docstring — fair nit but the behavior is correct. "Live adapter map" refers to the fact that the dict holds live adapter references (shared with gateway), not that the dict itself is refreshed. Adding "(shared references to live adapter instances)" would clarify without changing semantics; will do if desired. |
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). (cherry picked from commit ee79a0f7b6396e41f64df7ed76e9a759edbf8719)
…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 (#80337 contract; #80487 recovery context) - cells 4-5: documented stubs interlocked with #82956-#82959 and #83197/#83557 Journal-mode matrix (resolver default / DELETE / WAL-with-skip-gate) per cell; every wait deadline-bounded; writers asserted alive at kill time.
…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.
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).
|
Thanks @dongjiang1989 — you proposed hoisting the secret-scope reset out of the run-block
Closing as redundant with credit; #83182 is closed as fixed with your name on it. |
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). (cherry picked from commit ee79a0f7b6396e41f64df7ed76e9a759edbf8719)
…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.
…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.
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). (cherry picked from commit ee79a0f7b6396e41f64df7ed76e9a759edbf8719)
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). (cherry picked from commit ee79a0f7b6396e41f64df7ed76e9a759edbf8719)
Summary
Closes: #83182
Under a multiplex gateway (multiple per-profile Telegram bots etc.), cron jobs owned by a secondary profile deliver to the wrong bot/chat. Telegram logs show
Thread 9539 not found ... retrying without message_thread_id— the thread is dropped because the bot token used at delivery time does not own it.The root cause has two parts:
cron/scheduler.py::run_one_jobinstalls the job-owning profile's secret scope, then resets it in thefinallyblock betweenrun_joband_deliver_result. By the time delivery runs,current_secret_scope()isNone—load_gateway_config()→_getenv()falls back toos.environ(empty in the multiplex unit) —TELEGRAM_BOT_TOKENresolves to the wrong bot (or nothing).The gateway starts the cron ticker with the shared
runner.adaptersdict (the default profile's live adapters)._deliver_result→resolve_delivery_transport(...)only ever consults that shared dict. It never reachesGateway._profile_adapters[<job profile>], so even when the token happens to be right, cron delivery cannot reach the secondary profile's live adapter.This is distinct from existing issues #51853 and #54675 — their fix (PR #59315, commit
0f154e7) covers only adapter startup, not cron delivery-time scope reset / shared-adapter usage.What does this PR do?
Two fixes, both required to close the bug:
Part 1 — Scope lifetime: Move
reset_secret_scopefrom the innerfinally(afterrun_job) to an outerfinally(after delivery). The profile's secret scope now stays installed through both execution and delivery, soload_gateway_config()picks up the rightTELEGRAM_BOT_TOKENfrom the owning profile's.env.Part 2 — Per-profile adapters:
gateway/run.pybuildsprofile_adapters_by_home— a mapping from resolved profile home path to that profile's live adapter map (sourced fromrunner._profile_adaptersfor secondary profiles,runner.adaptersfor the default). This map is threaded through the cron chain:scheduler_provider.start→_start_multiplex→cron_tick→run_one_job→_deliver_result.run_one_jobresolves the owning profile's adapter map via the current hermes home (already set by_start_multiplex's per-profile override), so delivery picks the right bot token.When
profile_adapters_by_homeis not set (single-profile mode, direct CLI ticks, external providers), the code falls back to the sharedadaptersdict — fully backward compatible.Related Issue
Fixes #83182
Type of Change
Changes Made
cron/scheduler.pyrun_one_job. The innerfinally: reset_secret_scope(_scope_token)is replaced with an outerfinallythat wraps bothrun_joband the delivery block. Agent teardown stays in the innerfinally(its existing contract, unchanged).profile_adapters_by_homeparameter totickandrun_one_job; addprofile_adaptersparameter to_deliver_result.run_one_jobresolves the owning profile's adapters from the map keyed by the current hermes home path._deliver_resultprefers the profile-specific adapters over the shared dict when both are present.DeliveryRouterandresolve_delivery_transportcall sites updated to use the resolveddelivery_adapters.cron/scheduler_provider.pyInProcessCronScheduler.startand_start_multiplexaccept and propagateprofile_adapters_by_homedown tocron_tick.gateway/run.pymultiplex_profilesis on, buildprofile_adapters_by_homefromrunner._profile_adapters(secondary profiles) plusrunner.adapters(default profile), keyed by resolved hermes home path. Pass to the cron scheduler viacron_start_kwargs.tests/cron/test_cron_multiplex_delivery_83182.py(new)TestSecretScopeThroughDelivery— verifies the scope-reset ordering (deliver → reset, not reset → deliver).TestProfileAdapterSelection— verifies profile-specific adapters are preferred over shared, with fallback.TestRunOneJobProfileAdaptersResolution— verifiesrun_one_jobcorrectly looks up the right adapter map fromprofile_adapters_by_home.tests/cron/test_run_one_job.py/test_preflight_config.pyfake_deliversignatures to accept the newprofile_adapters=Nonekwarg. No behavioral changes.How to Test
scripts/run_tests.sh tests/cron/test_cron_multiplex_delivery_83182.py— 6 tests pass.scripts/run_tests.sh tests/cron/test_run_one_job.py tests/cron/test_preflight_config.py— 13 tests pass, no regression.Thread ... not found. Post-fix: delivers via the secondary profile's bot to the correct chat.Checklist
Code
Documentation & Housekeeping
cli-config.yaml.exampleif I added/changed config keys — N/A (no config changes)CONTRIBUTING.mdorAGENTS.mdif I changed architecture — N/A