Skip to content

fix(cron): deliver manual runs on gateway loop - #63586

Closed
Fly-onlyone wants to merge 1 commit into
NousResearch:mainfrom
Fly-onlyone:fix/matrix-manual-cron-loop
Closed

fix(cron): deliver manual runs on gateway loop#63586
Fly-onlyone wants to merge 1 commit into
NousResearch:mainfrom
Fly-onlyone:fix/matrix-manual-cron-loop

Conversation

@Fly-onlyone

Copy link
Copy Markdown
Contributor

Summary

  • pass the live gateway adapter map and owning event loop into immediate/manual cron execution
  • keep CLI-only manual runs on the existing standalone delivery path without importing the gateway
  • add regressions for both live-gateway and standalone contexts

Fixes #61495.

Why

cronjob(action="run") called run_one_job(job) without the gateway context. Matrix then reused a live mautrix/aiohttp adapter from a fresh asyncio.run() loop and failed with Timeout context manager should be used inside a task. Scheduled gateway ticks already pass adapters and loop; this makes the manual path do the same.

Tests

  • uv run --extra dev python -m pytest tests/tools/test_cronjob_run_immediate.py tests/tools/test_cronjob_tools.py -q (84 passed)
  • uv run --extra dev python -m pytest tests/tools/test_windows_native_support.py -q (66 passed)
  • uvx ruff check tools/cronjob_tools.py tests/tools/test_cronjob_run_immediate.py

Copilot AI review requested due to automatic review settings July 13, 2026 04:56

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes manual/immediate cron job runs initiated from a live gateway session so that result delivery is executed on the gateway’s owning asyncio event loop (and with the live adapter map), matching the scheduled ticker path. This prevents async client misuse (notably mautrix/aiohttp for Matrix) that can occur when delivery is attempted from a standalone asyncio.run() context.

Changes:

  • Pass the live gateway adapters map and _gateway_loop into cron.scheduler.run_one_job() during _execute_job_now() by retrieving the running gateway runner via sys.modules["gateway.run"] (when present).
  • Preserve the standalone/CLI behavior by falling back to adapters=None, loop=None when the gateway context is absent.
  • Add regression tests covering both the live-gateway context and the standalone context for _execute_job_now().

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
tools/cronjob_tools.py Threads live gateway adapters + event loop into run_one_job() for manual runs to keep delivery loop-safe.
tests/tools/test_cronjob_run_immediate.py Adds tests asserting manual runs pass live gateway context when available and remain standalone otherwise.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cron Cron scheduler and job management platform/matrix Matrix adapter (E2EE) sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages labels Jul 13, 2026

@teknium1 teknium1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for routing manual cron runs through the scheduler’s loop-aware delivery path; current main still invokes run_one_job(job) without that context at tools/cronjob_tools.py:641.

Problems

  • tools/cronjob_tools.py:651 always selects runner.adapters. In multiplex mode, gateway/run.py:2871-2877 documents that this map is only for the default/active profile; secondary-profile adapters live in runner._profile_adapters. A manual run from a secondary profile therefore still misses its live adapter context, which can reintroduce the standalone delivery path this change is intended to avoid.

Suggested changes

  • Resolve the adapter map using the current profile runtime context, and add a secondary-profile regression alongside the default-gateway and standalone cases.

Automated hermes-sweeper review.

Comment thread tools/cronjob_tools.py
gateway_module = sys.modules.get("gateway.run")
runner_ref = getattr(gateway_module, "_gateway_runner_ref", None)
runner = runner_ref() if callable(runner_ref) else None
adapters = getattr(runner, "adapters", None) if runner is not None else None

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GatewayRunner.adapters is deliberately only the default/active profile map in multiplex mode; secondary adapters are in runner._profile_adapters (gateway/run.py:2871-2877). Resolve the map for the invoking profile here, otherwise a secondary-profile manual cron run still cannot use its own live adapter.

@teknium1 teknium1 added sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 16, 2026
@ciabata-git

Copy link
Copy Markdown
Contributor

I reproduced this issue and carried this PR's original commit forward with Fly's authorship preserved as the first commit in #71132.

The follow-up commit implements the maintainer-requested multiplex-profile handling and adds fail-closed coverage for:

  • context-scoped secondary profile adapter selection;
  • absent profile registries;
  • partial maps that lack Matrix;
  • enabled, disabled, or missing multiplex metadata;
  • stale/stopped/closed gateway contexts;
  • exact standalone run_one_job(job) compatibility.

The final targeted cron suite is 155/155 passing. I opened #71132 as the hardened continuation because I cannot update this contributor-owned branch directly; maintainers can treat it as the replacement while retaining this PR's contributor credit.

@teknium1

teknium1 commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Thanks @Fly-onlyone — this was the correct diagnosis and the correct fix: manual runs called run_one_job(job) without the gateway adapters/loop, so Matrix delivery died with "Timeout context manager should be used inside a task". Your commit is now on main via #80838, cherry-picked with your authorship preserved and rewired onto the shared _run_claimed_job body that both the sync and background manual-run paths use (post-#80807), so live-gateway context now applies to background runs too. Fixes the delivery half of #61495.

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 P3 Low — cosmetic, nice to have platform/matrix Matrix adapter (E2EE) sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Matrix manual cron delivery fails with "Timeout context manager should be used inside a task"

5 participants