fix(cron): do not load MEMORY.md into scheduled jobs
Cron already sets skip_memory=True and denylists the memory toolset. The default cron toolset still names memory, so init treated that as a request and built MemoryStore. MEMORY.md then landed in the job prompt. Treat a denylisted toolset as not requested, and strip memory from the cron enabled list. Flush agents that actually want the memory tool are unchanged (#65429).
This commit is contained in:
+11
-7
@@ -1815,13 +1815,17 @@ def init_agent(
|
||||
agent._memory_nudge_interval = 10
|
||||
agent._turns_since_memory = 0
|
||||
agent._iters_since_skill = 0
|
||||
# A flush/background agent may pass skip_memory=True to avoid spinning up an
|
||||
# external memory *provider*, but if the caller also explicitly enables the
|
||||
# "memory" toolset it still needs the built-in file-backed store — otherwise
|
||||
# the memory tool dispatches with store=None and every call fails (#65429).
|
||||
# So the built-in store is created unless memory is globally disabled, while
|
||||
# the external-provider block below stays gated on skip_memory.
|
||||
_memory_toolset_requested = "memory" in (agent.enabled_toolsets or [])
|
||||
# skip_memory=True skips the external memory *provider*. Flush/background
|
||||
# agents can still pass enabled_toolsets=["memory"] so the built-in file
|
||||
# store exists and the memory tool does not fail with store=None (#65429).
|
||||
# A toolset on disabled_toolsets is not a request. Cron always denylists
|
||||
# memory, but the default cron toolset still names it, so an enabled-only
|
||||
# check would load MEMORY.md into an auto-approve job.
|
||||
_enabled_toolsets = agent.enabled_toolsets or []
|
||||
_disabled_toolsets = agent.disabled_toolsets or []
|
||||
_memory_toolset_requested = (
|
||||
"memory" in _enabled_toolsets and "memory" not in _disabled_toolsets
|
||||
)
|
||||
if not skip_memory or _memory_toolset_requested:
|
||||
try:
|
||||
from tools.memory_tool import (
|
||||
|
||||
+24
-4
@@ -361,8 +361,10 @@ def _resolve_cron_disabled_toolsets(cfg: dict) -> list[str]:
|
||||
Three toolsets are always disabled in cron context regardless of config:
|
||||
- ``messaging`` — interactive, needs a live gateway session
|
||||
- ``clarify`` — interactive, blocks waiting for user input
|
||||
- ``memory`` — cron agents are constructed with ``skip_memory=True``, so
|
||||
exposing this tool only gives the model an unbacked tool that fails
|
||||
- ``memory`` — cron agents run with ``skip_memory=True``. The tool is
|
||||
hidden, and ``memory`` is stripped from enabled_toolsets so the
|
||||
built-in store is not created either (MEMORY.md would otherwise
|
||||
land in the cron system prompt).
|
||||
|
||||
``cronjob`` is policy-denied by default (loop prevention, not a security
|
||||
boundary) and config-gated: setting ``cron.allow_agent_scheduling: true``
|
||||
@@ -436,6 +438,10 @@ def _resolve_cron_enabled_toolsets(job: dict, cfg: dict) -> list[str] | None:
|
||||
3. ``None`` on any lookup failure — AIAgent loads the full default set
|
||||
(legacy behavior before this change, preserved as the safety net).
|
||||
|
||||
``memory`` is always stripped. Cron denylists that toolset and passes
|
||||
``skip_memory=True``. Leaving it in enabled_toolsets still constructs
|
||||
the built-in MemoryStore and injects MEMORY.md into the job prompt.
|
||||
|
||||
_DEFAULT_OFF_TOOLSETS ({moa, homeassistant, rl}) are removed by
|
||||
``_get_platform_tools`` for unconfigured platforms, so fresh installs
|
||||
get cron WITHOUT ``moa`` by default (issue reported by Norbert —
|
||||
@@ -443,10 +449,12 @@ def _resolve_cron_enabled_toolsets(job: dict, cfg: dict) -> list[str] | None:
|
||||
"""
|
||||
per_job = job.get("enabled_toolsets")
|
||||
if per_job:
|
||||
return _merge_mcp_into_per_job_toolsets(list(per_job), cfg or {})
|
||||
return _strip_cron_memory_toolset(
|
||||
_merge_mcp_into_per_job_toolsets(list(per_job), cfg or {})
|
||||
)
|
||||
try:
|
||||
from hermes_cli.tools_config import _get_platform_tools # lazy: avoid heavy import at cron module load
|
||||
return sorted(_get_platform_tools(cfg or {}, "cron"))
|
||||
return _strip_cron_memory_toolset(sorted(_get_platform_tools(cfg or {}, "cron")))
|
||||
except Exception as exc:
|
||||
logger.warning(
|
||||
"Cron toolset resolution failed, falling back to full default toolset: %s",
|
||||
@@ -455,6 +463,17 @@ def _resolve_cron_enabled_toolsets(job: dict, cfg: dict) -> list[str] | None:
|
||||
return None
|
||||
|
||||
|
||||
def _strip_cron_memory_toolset(enabled: list[str] | None) -> list[str] | None:
|
||||
"""Drop ``memory`` from a cron enabled-toolset list.
|
||||
|
||||
``None`` means "full default set" and is left alone. skip_memory=True plus
|
||||
the memory denylist still keep the store off on that fallback path.
|
||||
"""
|
||||
if enabled is None:
|
||||
return None
|
||||
return [name for name in enabled if name != "memory"]
|
||||
|
||||
|
||||
def _resolve_job_reasoning_config(job: dict, cfg: dict, model: str) -> dict | None:
|
||||
"""Resolve the effective reasoning config for a cron run.
|
||||
|
||||
@@ -496,6 +515,7 @@ def _resolve_job_reasoning_config(job: dict, cfg: dict, model: str) -> dict | No
|
||||
)
|
||||
return resolve_reasoning_config(cfg if isinstance(cfg, dict) else {}, str(model))
|
||||
|
||||
|
||||
# Valid delivery platforms — used to validate user-supplied platform names
|
||||
# in cron delivery targets, preventing env var enumeration via crafted names.
|
||||
_KNOWN_DELIVERY_PLATFORMS = frozenset({
|
||||
|
||||
@@ -24,7 +24,9 @@ class _FakeOpenAI:
|
||||
pass
|
||||
|
||||
|
||||
def _make_agent(monkeypatch, enabled_toolsets=None, skip_memory=True):
|
||||
def _make_agent(
|
||||
monkeypatch, enabled_toolsets=None, disabled_toolsets=None, skip_memory=True
|
||||
):
|
||||
monkeypatch.setattr("run_agent.get_tool_definitions", lambda **kw: [])
|
||||
monkeypatch.setattr("run_agent.check_toolset_requirements", lambda: {})
|
||||
monkeypatch.setattr("run_agent.OpenAI", _FakeOpenAI)
|
||||
@@ -38,6 +40,7 @@ def _make_agent(monkeypatch, enabled_toolsets=None, skip_memory=True):
|
||||
skip_context_files=True,
|
||||
skip_memory=skip_memory,
|
||||
enabled_toolsets=enabled_toolsets,
|
||||
disabled_toolsets=disabled_toolsets,
|
||||
)
|
||||
|
||||
|
||||
@@ -96,3 +99,36 @@ def test_skip_memory_memory_tool_handler_works_and_provider_skipped(
|
||||
memory_md = tmp_path / "hm" / "memories" / "MEMORY.md"
|
||||
assert memory_md.exists()
|
||||
assert "User prefers concise answers." in memory_md.read_text()
|
||||
|
||||
|
||||
def test_skip_memory_disabled_toolset_does_not_load_store(monkeypatch, tmp_path):
|
||||
"""Cron shape: skip_memory=True, memory named in enabled AND disabled.
|
||||
|
||||
#65429 must not load MEMORY.md just because the default cron toolset
|
||||
still lists memory while the denylist hides the tool.
|
||||
"""
|
||||
home = tmp_path / "hm"
|
||||
monkeypatch.setenv("HERMES_HOME", str(home))
|
||||
mem_dir = home / "memories"
|
||||
mem_dir.mkdir(parents=True)
|
||||
secret = "cron-should-never-see-this-memory"
|
||||
(mem_dir / "MEMORY.md").write_text(secret + "\n")
|
||||
(mem_dir / "USER.md").write_text("cron-should-never-see-this-profile\n")
|
||||
|
||||
agent = _make_agent(
|
||||
monkeypatch,
|
||||
enabled_toolsets=["memory", "file"],
|
||||
disabled_toolsets=["memory"],
|
||||
skip_memory=True,
|
||||
)
|
||||
assert agent._memory_store is None
|
||||
assert agent._memory_manager is None
|
||||
assert agent._memory_enabled is False
|
||||
assert agent._user_profile_enabled is False
|
||||
|
||||
from agent.system_prompt import build_system_prompt_parts
|
||||
|
||||
parts = build_system_prompt_parts(agent)
|
||||
blob = " ".join(str(v) for v in parts.values())
|
||||
assert secret not in blob
|
||||
assert "cron-should-never-see-this-profile" not in blob
|
||||
|
||||
@@ -21,7 +21,7 @@ from cron.scheduler import _resolve_cron_disabled_toolsets
|
||||
|
||||
# The toolsets that must be denied in cron context no matter what the
|
||||
# agent-scheduling gate says: messaging/clarify are interactive-only,
|
||||
# memory is unbacked in cron runs (skip_memory=True).
|
||||
# memory stays off in cron runs (skip_memory=True, toolset denylisted).
|
||||
ALWAYS_DISABLED = ["messaging", "clarify", "memory"]
|
||||
|
||||
|
||||
|
||||
@@ -118,6 +118,24 @@ class TestPerJobToolsetMcpMerge:
|
||||
assert m_platform.call_args[0][1] == "cron"
|
||||
assert set(result) == set(sentinel)
|
||||
|
||||
def test_resolver_strips_memory_from_per_job_list(self):
|
||||
result = _resolve_cron_enabled_toolsets(
|
||||
{"enabled_toolsets": ["memory", "file"]},
|
||||
{"mcp_servers": {}},
|
||||
)
|
||||
assert "memory" not in result
|
||||
assert "file" in result
|
||||
|
||||
def test_resolver_strips_memory_from_platform_fallback(self):
|
||||
job = {"enabled_toolsets": None}
|
||||
with patch(
|
||||
"hermes_cli.tools_config._get_platform_tools",
|
||||
return_value={"web", "memory", "file"},
|
||||
):
|
||||
result = _resolve_cron_enabled_toolsets(job, {})
|
||||
assert result == ["file", "web"]
|
||||
assert "memory" not in result
|
||||
|
||||
|
||||
class TestResolveOrigin:
|
||||
def test_full_origin(self):
|
||||
@@ -613,10 +631,9 @@ class TestRunJobSessionPersistence:
|
||||
def test_run_job_memory_toolset_disabled_in_cron(self, tmp_path):
|
||||
"""memory toolset must be disabled in cron sessions — issue #38129.
|
||||
|
||||
Cron agents are constructed with skip_memory=True, so the memory
|
||||
backend is not initialised. Exposing the memory tool only gives the
|
||||
model an unbacked tool that fails at runtime with
|
||||
"Memory is not available." Hiding it from the schema prevents that.
|
||||
Cron agents are constructed with skip_memory=True. The memory tool
|
||||
stays off the schema, and memory is stripped from enabled_toolsets
|
||||
so MEMORY.md is not loaded into the job prompt.
|
||||
"""
|
||||
job = {
|
||||
"id": "memory-hide-job",
|
||||
@@ -634,10 +651,9 @@ class TestRunJobSessionPersistence:
|
||||
def test_run_job_disables_memory_even_when_per_job_enables_it(self, tmp_path):
|
||||
"""Cron runs pass skip_memory=True, so memory must not be exposed.
|
||||
|
||||
A cron job can request the memory tool through enabled_toolsets, but
|
||||
there is no MemoryStore injected for cron agents. Keep memory in the
|
||||
disabled set so AIAgent filters the unbacked tool out before the model
|
||||
can call it and receive "Memory is not available" failures.
|
||||
A cron job can name the memory toolset in enabled_toolsets. The
|
||||
resolver drops it, and the denylist still lists it, so the model
|
||||
never gets the tool and init never builds MemoryStore.
|
||||
"""
|
||||
job = {
|
||||
"id": "memory-toolset-job",
|
||||
@@ -650,7 +666,8 @@ class TestRunJobSessionPersistence:
|
||||
|
||||
kwargs = mock_agent_cls.call_args.kwargs
|
||||
assert kwargs["skip_memory"] is True
|
||||
assert kwargs["enabled_toolsets"] == ["memory", "file"]
|
||||
assert kwargs["enabled_toolsets"] == ["file"]
|
||||
assert "memory" not in (kwargs["enabled_toolsets"] or [])
|
||||
assert "memory" in kwargs["disabled_toolsets"]
|
||||
|
||||
def test_tick_skips_due_jobs_while_dispatch_is_paused(self, tmp_path):
|
||||
|
||||
Reference in New Issue
Block a user