mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-06-27 11:22:03 +00:00
fix(cron): avoid provider package shadowing core cron
This commit is contained in:
parent
31bced1607
commit
0d4cecb352
12 changed files with 72 additions and 38 deletions
|
|
@ -14,7 +14,7 @@ delivery.
|
|||
|
||||
The built-in InProcessCronScheduler runs the historical 60s daemon-thread
|
||||
ticker. Alternative providers (e.g. Chronos, a NAS-mediated managed-cron
|
||||
provider for scale-to-zero deployments) live under plugins/cron/<name>/ and are
|
||||
provider for scale-to-zero deployments) live under plugins/cron_providers/<name>/ and are
|
||||
selected via the `cron.provider` config key (empty = built-in).
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
|
@ -134,7 +134,7 @@ def resolve_cron_scheduler() -> "CronScheduler":
|
|||
return InProcessCronScheduler()
|
||||
|
||||
try:
|
||||
from plugins.cron import load_cron_scheduler
|
||||
from plugins.cron_providers import load_cron_scheduler
|
||||
provider = load_cron_scheduler(name)
|
||||
if provider is None:
|
||||
logger.warning("cron.provider '%s' not found; using built-in ticker", name)
|
||||
|
|
|
|||
|
|
@ -3448,7 +3448,7 @@ class APIServerAdapter(BasePlatformAdapter):
|
|||
against double-fire on a NAS/scheduler retry.
|
||||
"""
|
||||
from hermes_cli.config import cfg_get, load_config
|
||||
from plugins.cron.chronos.verify import get_fire_verifier
|
||||
from plugins.cron_providers.chronos.verify import get_fire_verifier
|
||||
|
||||
auth = request.headers.get("Authorization", "")
|
||||
token = auth[7:].strip() if auth.startswith("Bearer ") else ""
|
||||
|
|
|
|||
|
|
@ -2340,7 +2340,7 @@ DEFAULT_CONFIG = {
|
|||
"cron": {
|
||||
# Active cron SCHEDULER provider (Axis B — the trigger that decides
|
||||
# WHEN a due job fires). Empty string = the built-in in-process 60s
|
||||
# ticker (default). Name an installed provider (plugins/cron/<name>/ or
|
||||
# ticker (default). Name an installed provider (plugins/cron_providers/<name>/ or
|
||||
# $HERMES_HOME/plugins/<name>/) to relocate the trigger — e.g. "chronos",
|
||||
# the NAS-mediated managed-cron provider for scale-to-zero deployments.
|
||||
# An unknown or unavailable provider falls back to the built-in, so cron
|
||||
|
|
|
|||
|
|
@ -7916,7 +7916,7 @@ async def cron_fire_webhook(request: Request):
|
|||
Returns 202 immediately and runs the job in the background so a long agent
|
||||
turn never trips NAS's HTTP timeout.
|
||||
"""
|
||||
from plugins.cron.chronos.verify import get_fire_verifier
|
||||
from plugins.cron_providers.chronos.verify import get_fire_verifier
|
||||
|
||||
auth = request.headers.get("Authorization", "")
|
||||
token = auth[7:].strip() if auth.startswith("Bearer ") else ""
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@
|
|||
|
||||
Scans two directories for cron scheduler provider plugins:
|
||||
|
||||
1. Bundled providers: ``plugins/cron/<name>/`` (shipped with hermes-agent)
|
||||
1. Bundled providers: ``plugins/cron_providers/<name>/`` (shipped with hermes-agent)
|
||||
2. User-installed providers: ``$HERMES_HOME/plugins/<name>/``
|
||||
|
||||
Each subdirectory must contain ``__init__.py`` with a class implementing the
|
||||
|
|
@ -19,7 +19,7 @@ Only ONE provider can be active at a time, selected via ``cron.provider`` in
|
|||
config.yaml (empty = built-in). See ``cron.scheduler_provider.resolve_cron_scheduler``.
|
||||
|
||||
Usage:
|
||||
from plugins.cron import discover_cron_schedulers, load_cron_scheduler
|
||||
from plugins.cron_providers import discover_cron_schedulers, load_cron_scheduler
|
||||
|
||||
available = discover_cron_schedulers() # [(name, desc, available), ...]
|
||||
provider = load_cron_scheduler("chronos") # CronScheduler instance
|
||||
|
|
@ -52,7 +52,7 @@ def _register_synthetic_package(name: str, search_locations: List[str]) -> None:
|
|||
in ``sys.modules``, any relative import inside the plugin
|
||||
(``from . import config``) fails with
|
||||
``ModuleNotFoundError: No module named '_hermes_user_cron'`` — the same
|
||||
reason the loader already registers ``plugins`` and ``plugins.cron`` for
|
||||
reason the loader already registers ``plugins`` and ``plugins.cron_providers`` for
|
||||
bundled providers.
|
||||
"""
|
||||
if name in sys.modules:
|
||||
|
|
@ -101,7 +101,7 @@ def _iter_provider_dirs() -> List[Tuple[str, Path]]:
|
|||
seen: set = set()
|
||||
dirs: List[Tuple[str, Path]] = []
|
||||
|
||||
# 1. Bundled providers (plugins/cron/<name>/)
|
||||
# 1. Bundled providers (plugins/cron_providers/<name>/)
|
||||
if _CRON_PLUGINS_DIR.is_dir():
|
||||
for child in sorted(_CRON_PLUGINS_DIR.iterdir()):
|
||||
if not child.is_dir() or child.name.startswith(("_", ".")):
|
||||
|
|
@ -190,7 +190,7 @@ def discover_cron_schedulers() -> List[Tuple[str, str, bool]]:
|
|||
def load_cron_scheduler(name: str) -> Optional["CronScheduler"]: # noqa: F821
|
||||
"""Load and return a CronScheduler instance by name.
|
||||
|
||||
Checks both bundled (``plugins/cron/<name>/``) and user-installed
|
||||
Checks both bundled (``plugins/cron_providers/<name>/``) and user-installed
|
||||
(``$HERMES_HOME/plugins/<name>/``) directories. Bundled takes precedence
|
||||
on name collisions.
|
||||
|
||||
|
|
@ -223,7 +223,7 @@ def _load_provider_from_dir(provider_dir: Path) -> Optional["CronScheduler"]: #
|
|||
# Use a separate namespace for user-installed plugins so they don't
|
||||
# collide with bundled providers in sys.modules.
|
||||
_is_bundled = _CRON_PLUGINS_DIR in provider_dir.parents or provider_dir.parent == _CRON_PLUGINS_DIR
|
||||
module_name = f"plugins.cron.{name}" if _is_bundled else f"{_USER_NAMESPACE}.{name}"
|
||||
module_name = f"plugins.cron_providers.{name}" if _is_bundled else f"{_USER_NAMESPACE}.{name}"
|
||||
init_file = provider_dir / "__init__.py"
|
||||
|
||||
if not init_file.exists():
|
||||
|
|
@ -236,7 +236,7 @@ def _load_provider_from_dir(provider_dir: Path) -> Optional["CronScheduler"]: #
|
|||
mod = cached
|
||||
else:
|
||||
# Ensure the parent packages are registered (for relative imports)
|
||||
for parent in ("plugins", "plugins.cron"):
|
||||
for parent in ("plugins", "plugins.cron_providers"):
|
||||
if parent not in sys.modules:
|
||||
parent_path = Path(__file__).parent
|
||||
if parent == "plugins":
|
||||
|
|
@ -270,6 +270,7 @@ def _load_provider_from_dir(provider_dir: Path) -> Optional["CronScheduler"]: #
|
|||
|
||||
mod = importlib.util.module_from_spec(spec)
|
||||
sys.modules[module_name] = mod
|
||||
loaded_submodules = []
|
||||
|
||||
# Register submodules so relative imports work
|
||||
# e.g., "from ._nas_client import NasCronClient" in the chronos plugin
|
||||
|
|
@ -287,6 +288,7 @@ def _load_provider_from_dir(provider_dir: Path) -> Optional["CronScheduler"]: #
|
|||
sys.modules[full_sub_name] = sub_mod
|
||||
try:
|
||||
sub_spec.loader.exec_module(sub_mod)
|
||||
loaded_submodules.append((sub_name, sub_mod))
|
||||
except Exception as e:
|
||||
logger.debug("Failed to load submodule %s: %s", full_sub_name, e)
|
||||
|
||||
|
|
@ -297,6 +299,16 @@ def _load_provider_from_dir(provider_dir: Path) -> Optional["CronScheduler"]: #
|
|||
sys.modules.pop(module_name, None)
|
||||
return None
|
||||
|
||||
# Manual importlib loading bypasses the normal import machinery that
|
||||
# binds child modules onto their parent packages. Restore that shape so
|
||||
# later dotted imports and pytest monkeypatch paths resolve normally.
|
||||
parent_name, child_name = module_name.rsplit(".", 1)
|
||||
parent_mod = sys.modules.get(parent_name)
|
||||
if parent_mod is not None:
|
||||
setattr(parent_mod, child_name, mod)
|
||||
for sub_name, sub_mod in loaded_submodules:
|
||||
setattr(mod, sub_name, sub_mod)
|
||||
|
||||
# Try register(ctx) pattern first (how our plugins are written)
|
||||
if hasattr(mod, "register"):
|
||||
collector = _ProviderCollector()
|
||||
|
|
@ -235,7 +235,7 @@ class ChronosCronScheduler(CronScheduler):
|
|||
def register(ctx) -> None:
|
||||
"""Plugin entrypoint — register the Chronos provider with the loader.
|
||||
|
||||
Mirrors the memory-plugin shape; plugins/cron discovery calls this and
|
||||
Mirrors the memory-plugin shape; plugins/cron_providers discovery calls this and
|
||||
collects the provider via register_cron_scheduler.
|
||||
"""
|
||||
ctx.register_cron_scheduler(ChronosCronScheduler())
|
||||
|
|
@ -172,20 +172,42 @@ def test_default_config_cron_provider_is_empty():
|
|||
|
||||
|
||||
def test_discover_cron_schedulers_returns_list():
|
||||
"""Discovery returns a list. May be empty — the built-in is core, not
|
||||
discovered, and no bundled non-default provider ships yet."""
|
||||
from plugins.cron import discover_cron_schedulers
|
||||
"""Discovery returns bundled non-default providers.
|
||||
|
||||
The built-in is core, not discovered here.
|
||||
"""
|
||||
from plugins.cron_providers import discover_cron_schedulers
|
||||
|
||||
result = discover_cron_schedulers()
|
||||
assert isinstance(result, list)
|
||||
assert any(name == "chronos" for name, _desc, _available in result)
|
||||
|
||||
|
||||
def test_load_unknown_cron_scheduler_returns_none():
|
||||
from plugins.cron import load_cron_scheduler
|
||||
from plugins.cron_providers import load_cron_scheduler
|
||||
|
||||
assert load_cron_scheduler("does-not-exist-xyz") is None
|
||||
|
||||
|
||||
def test_cron_provider_package_does_not_shadow_core_cron_package(monkeypatch):
|
||||
"""Putting plugins/ first on sys.path must not hide the core cron package."""
|
||||
from importlib.machinery import PathFinder
|
||||
from pathlib import Path
|
||||
|
||||
repo_root = Path(__file__).resolve().parents[2]
|
||||
|
||||
monkeypatch.syspath_prepend(str(repo_root))
|
||||
monkeypatch.syspath_prepend(str(repo_root / "plugins"))
|
||||
|
||||
cron_spec = PathFinder.find_spec("cron")
|
||||
assert cron_spec is not None
|
||||
assert Path(cron_spec.origin).resolve() == repo_root / "cron" / "__init__.py"
|
||||
|
||||
jobs_spec = PathFinder.find_spec("cron.jobs", [str(repo_root / "cron")])
|
||||
assert jobs_spec is not None
|
||||
assert Path(jobs_spec.origin).resolve() == repo_root / "cron" / "jobs.py"
|
||||
|
||||
|
||||
def test_resolve_defaults_to_builtin(monkeypatch):
|
||||
"""Empty cron.provider → built-in."""
|
||||
import hermes_cli.config as cfg
|
||||
|
|
@ -219,7 +241,7 @@ def test_resolve_unknown_provider_falls_back_to_builtin(monkeypatch):
|
|||
def test_resolve_unavailable_provider_falls_back(monkeypatch):
|
||||
"""A provider that loads but reports is_available()==False → built-in."""
|
||||
import hermes_cli.config as cfg
|
||||
import plugins.cron as pc
|
||||
import plugins.cron_providers as pc
|
||||
from cron import scheduler_provider as sp
|
||||
from cron.scheduler_provider import CronScheduler
|
||||
|
||||
|
|
@ -243,7 +265,7 @@ def test_resolve_unavailable_provider_falls_back(monkeypatch):
|
|||
def test_resolve_available_provider_is_used(monkeypatch):
|
||||
"""A provider that loads and is available is returned (not the fallback)."""
|
||||
import hermes_cli.config as cfg
|
||||
import plugins.cron as pc
|
||||
import plugins.cron_providers as pc
|
||||
from cron import scheduler_provider as sp
|
||||
from cron.scheduler_provider import CronScheduler
|
||||
|
||||
|
|
|
|||
|
|
@ -21,7 +21,7 @@ def temp_home(tmp_path, monkeypatch):
|
|||
@pytest.fixture
|
||||
def chronos(monkeypatch):
|
||||
"""A ChronosCronScheduler with a fake NAS client capturing calls."""
|
||||
from plugins.cron.chronos import ChronosCronScheduler
|
||||
from plugins.cron_providers.chronos import ChronosCronScheduler
|
||||
|
||||
class FakeClient:
|
||||
def __init__(self):
|
||||
|
|
@ -47,7 +47,7 @@ def chronos(monkeypatch):
|
|||
fake = FakeClient()
|
||||
prov._client = fake
|
||||
# callback_url is read via _cfg; patch the module helper to avoid config.
|
||||
monkeypatch.setattr("plugins.cron.chronos._cfg",
|
||||
monkeypatch.setattr("plugins.cron_providers.chronos._cfg",
|
||||
lambda *k, default="": "https://agent.example/" if k[-1] == "callback_url" else "https://portal.test")
|
||||
return prov, fake
|
||||
|
||||
|
|
@ -55,15 +55,15 @@ def chronos(monkeypatch):
|
|||
# -- is_available -------------------------------------------------------------
|
||||
|
||||
def test_is_available_false_without_config(temp_home, monkeypatch):
|
||||
from plugins.cron.chronos import ChronosCronScheduler
|
||||
from plugins.cron_providers.chronos import ChronosCronScheduler
|
||||
|
||||
monkeypatch.setattr("plugins.cron.chronos._cfg", lambda *k, default="": "")
|
||||
monkeypatch.setattr("plugins.cron_providers.chronos._cfg", lambda *k, default="": "")
|
||||
assert ChronosCronScheduler().is_available() is False
|
||||
|
||||
|
||||
def test_is_available_true_with_config_and_token(temp_home, monkeypatch):
|
||||
import plugins.cron.chronos as mod
|
||||
from plugins.cron.chronos import ChronosCronScheduler
|
||||
import plugins.cron_providers.chronos as mod
|
||||
from plugins.cron_providers.chronos import ChronosCronScheduler
|
||||
|
||||
monkeypatch.setattr(mod, "_cfg", lambda *k, default="": "https://x" )
|
||||
monkeypatch.setattr("hermes_cli.auth.get_provider_auth_state",
|
||||
|
|
@ -73,8 +73,8 @@ def test_is_available_true_with_config_and_token(temp_home, monkeypatch):
|
|||
|
||||
def test_is_available_makes_no_network(temp_home, monkeypatch):
|
||||
"""is_available must not construct the NAS client / hit network."""
|
||||
import plugins.cron.chronos as mod
|
||||
from plugins.cron.chronos import ChronosCronScheduler
|
||||
import plugins.cron_providers.chronos as mod
|
||||
from plugins.cron_providers.chronos import ChronosCronScheduler
|
||||
|
||||
monkeypatch.setattr(mod, "_cfg", lambda *k, default="": "https://x")
|
||||
monkeypatch.setattr("hermes_cli.auth.get_provider_auth_state",
|
||||
|
|
|
|||
|
|
@ -54,7 +54,7 @@ def _base_claims(**over):
|
|||
|
||||
|
||||
def test_valid_token_returns_claims(rsa_keys):
|
||||
from plugins.cron.chronos.verify import verify_nas_fire_token
|
||||
from plugins.cron_providers.chronos.verify import verify_nas_fire_token
|
||||
|
||||
priv, pub = rsa_keys
|
||||
token = _mint(priv, _base_claims())
|
||||
|
|
@ -66,7 +66,7 @@ def test_valid_token_returns_claims(rsa_keys):
|
|||
|
||||
|
||||
def test_wrong_audience_rejected(rsa_keys):
|
||||
from plugins.cron.chronos.verify import verify_nas_fire_token
|
||||
from plugins.cron_providers.chronos.verify import verify_nas_fire_token
|
||||
|
||||
priv, pub = rsa_keys
|
||||
token = _mint(priv, _base_claims(aud="agent:someone-else"))
|
||||
|
|
@ -76,7 +76,7 @@ def test_wrong_audience_rejected(rsa_keys):
|
|||
|
||||
def test_missing_purpose_rejected(rsa_keys):
|
||||
"""A general agent JWT (no purpose=cron_fire) can't fire jobs."""
|
||||
from plugins.cron.chronos.verify import verify_nas_fire_token
|
||||
from plugins.cron_providers.chronos.verify import verify_nas_fire_token
|
||||
|
||||
priv, pub = rsa_keys
|
||||
claims = _base_claims()
|
||||
|
|
@ -87,7 +87,7 @@ def test_missing_purpose_rejected(rsa_keys):
|
|||
|
||||
|
||||
def test_wrong_purpose_rejected(rsa_keys):
|
||||
from plugins.cron.chronos.verify import verify_nas_fire_token
|
||||
from plugins.cron_providers.chronos.verify import verify_nas_fire_token
|
||||
|
||||
priv, pub = rsa_keys
|
||||
token = _mint(priv, _base_claims(purpose="inference"))
|
||||
|
|
@ -96,7 +96,7 @@ def test_wrong_purpose_rejected(rsa_keys):
|
|||
|
||||
|
||||
def test_expired_token_rejected(rsa_keys):
|
||||
from plugins.cron.chronos.verify import verify_nas_fire_token
|
||||
from plugins.cron_providers.chronos.verify import verify_nas_fire_token
|
||||
|
||||
priv, pub = rsa_keys
|
||||
now = int(time.time())
|
||||
|
|
@ -106,7 +106,7 @@ def test_expired_token_rejected(rsa_keys):
|
|||
|
||||
|
||||
def test_wrong_issuer_rejected(rsa_keys):
|
||||
from plugins.cron.chronos.verify import verify_nas_fire_token
|
||||
from plugins.cron_providers.chronos.verify import verify_nas_fire_token
|
||||
|
||||
priv, pub = rsa_keys
|
||||
token = _mint(priv, _base_claims(iss="https://evil.example"))
|
||||
|
|
@ -118,7 +118,7 @@ def test_tampered_signature_rejected(rsa_keys):
|
|||
"""A token signed by a DIFFERENT key must fail signature verification."""
|
||||
from cryptography.hazmat.primitives import serialization
|
||||
from cryptography.hazmat.primitives.asymmetric import rsa
|
||||
from plugins.cron.chronos.verify import verify_nas_fire_token
|
||||
from plugins.cron_providers.chronos.verify import verify_nas_fire_token
|
||||
|
||||
_, pub = rsa_keys
|
||||
attacker = rsa.generate_private_key(public_exponent=65537, key_size=2048)
|
||||
|
|
@ -135,7 +135,7 @@ def test_tampered_signature_rejected(rsa_keys):
|
|||
|
||||
def test_no_key_configured_refuses(rsa_keys):
|
||||
"""No JWKS/key configured → refuse (never fall back to unsigned decode)."""
|
||||
from plugins.cron.chronos.verify import verify_nas_fire_token
|
||||
from plugins.cron_providers.chronos.verify import verify_nas_fire_token
|
||||
|
||||
priv, _ = rsa_keys
|
||||
token = _mint(priv, _base_claims())
|
||||
|
|
@ -144,7 +144,7 @@ def test_no_key_configured_refuses(rsa_keys):
|
|||
|
||||
|
||||
def test_empty_token_refused(rsa_keys):
|
||||
from plugins.cron.chronos.verify import verify_nas_fire_token
|
||||
from plugins.cron_providers.chronos.verify import verify_nas_fire_token
|
||||
|
||||
_, pub = rsa_keys
|
||||
assert verify_nas_fire_token(token="", expected_audience=AUD, jwks_or_key=pub) is None
|
||||
|
|
@ -152,7 +152,7 @@ def test_empty_token_refused(rsa_keys):
|
|||
|
||||
def test_jwks_url_path_resolves_key(rsa_keys, monkeypatch):
|
||||
"""The JWKS-URL branch resolves the signing key via PyJWKClient."""
|
||||
from plugins.cron.chronos.verify import verify_nas_fire_token
|
||||
from plugins.cron_providers.chronos.verify import verify_nas_fire_token
|
||||
|
||||
priv, pub = rsa_keys
|
||||
token = _mint(priv, _base_claims())
|
||||
|
|
@ -177,6 +177,6 @@ def test_jwks_url_path_resolves_key(rsa_keys, monkeypatch):
|
|||
|
||||
|
||||
def test_get_fire_verifier_returns_nas_verifier():
|
||||
from plugins.cron.chronos.verify import get_fire_verifier, verify_nas_fire_token
|
||||
from plugins.cron_providers.chronos.verify import get_fire_verifier, verify_nas_fire_token
|
||||
|
||||
assert get_fire_verifier() is verify_nas_fire_token
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue