diff --git a/agent/agent_init.py b/agent/agent_init.py index 88985abbdc..db92f18487 100644 --- a/agent/agent_init.py +++ b/agent/agent_init.py @@ -1816,9 +1816,10 @@ def init_agent( # 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. + # A toolset on disabled_toolsets is not a request: a caller that denylists + # memory while its default toolset still names it must not get MEMORY.md + # loaded by an enabled-only check. (Cron agents now run with + # skip_memory=False and take the normal path here.) _enabled_toolsets = agent.enabled_toolsets or [] _disabled_toolsets = agent.disabled_toolsets or [] _memory_toolset_requested = ( diff --git a/cron/scheduler.py b/cron/scheduler.py index 59ce8fafa8..dd82623ea1 100644 --- a/cron/scheduler.py +++ b/cron/scheduler.py @@ -358,13 +358,9 @@ class CronPromptInjectionBlocked(Exception): def _resolve_cron_disabled_toolsets(cfg: dict) -> list[str]: """Toolsets a cron-spawned agent must never receive. - Three toolsets are always disabled in cron context regardless of config: + Two 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 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`` @@ -379,9 +375,9 @@ def _resolve_cron_disabled_toolsets(cfg: dict) -> list[str]: """ cron_cfg = (cfg or {}).get("cron") or {} if cron_cfg.get("allow_agent_scheduling"): - disabled = ["messaging", "clarify", "memory"] + disabled = ["messaging", "clarify"] else: - disabled = ["cronjob", "messaging", "clarify", "memory"] + disabled = ["cronjob", "messaging", "clarify"] agent_cfg = (cfg or {}).get("agent") or {} from agent.skill_utils import parse_config_string_list @@ -438,10 +434,6 @@ 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 — @@ -449,12 +441,10 @@ def _resolve_cron_enabled_toolsets(job: dict, cfg: dict) -> list[str] | None: """ per_job = job.get("enabled_toolsets") if per_job: - return _strip_cron_memory_toolset( - _merge_mcp_into_per_job_toolsets(list(per_job), cfg or {}) - ) + return _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 _strip_cron_memory_toolset(sorted(_get_platform_tools(cfg or {}, "cron"))) + return sorted(_get_platform_tools(cfg or {}, "cron")) except Exception as exc: logger.warning( "Cron toolset resolution failed, falling back to full default toolset: %s", @@ -463,17 +453,6 @@ 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. @@ -5833,7 +5812,11 @@ def run_job( # Without a workdir, keep cwd context discovery disabled. skip_context_files=not bool(_job_workdir), load_soul_identity=True, - skip_memory=True, # Cron system prompts would corrupt user representations + # Memory is enabled for cron agents like any other agent run: + # MEMORY.md / USER.md load into the system prompt and the memory + # tool follows normal toolset resolution, so jobs benefit from + # (and can update) the user's persistent memory. + skip_memory=False, skip_background_review=True, # Cron has no human-in-the-loop need for skill/memory review forks (~30K tok/event) platform="cron", session_id=_cron_session_id, diff --git a/tests/cron/test_agent_scheduling_gate.py b/tests/cron/test_agent_scheduling_gate.py index abbe6a9fb4..b8dc88de2a 100644 --- a/tests/cron/test_agent_scheduling_gate.py +++ b/tests/cron/test_agent_scheduling_gate.py @@ -7,8 +7,8 @@ default off) makes that denial opt-out-able: - gate off / absent: byte-exact current behavior — ``cronjob`` denied. - gate on: ``cronjob`` dropped from the base denylist; ``messaging`` and - ``clarify`` (interactivity constraints) and ``memory`` (cron agents run - with skip_memory=True) are ALWAYS denied regardless of the gate. + ``clarify`` (interactivity constraints) are ALWAYS denied regardless of + the gate. - user-level ``agent.disabled_toolsets`` still layers on top, so a user who denies ``cronjob`` globally keeps it denied even with the gate on (per-job enabled_toolsets can never widen past the config denylist). @@ -20,26 +20,27 @@ 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 stays off in cron runs (skip_memory=True, toolset denylisted). -ALWAYS_DISABLED = ["messaging", "clarify", "memory"] +# agent-scheduling gate says: messaging/clarify are interactive-only. +# ``memory`` is intentionally NOT here — cron agents get memory like any +# other agent run. +ALWAYS_DISABLED = ["messaging", "clarify"] class TestGateOffDefault: def test_empty_config_denies_cronjob(self): assert _resolve_cron_disabled_toolsets({}) == [ - "cronjob", "messaging", "clarify", "memory", + "cronjob", "messaging", "clarify", ] def test_none_config_denies_cronjob(self): assert _resolve_cron_disabled_toolsets(None) == [ - "cronjob", "messaging", "clarify", "memory", + "cronjob", "messaging", "clarify", ] def test_cron_section_present_but_gate_absent(self): cfg = {"cron": {"preflight": True}} assert _resolve_cron_disabled_toolsets(cfg) == [ - "cronjob", "messaging", "clarify", "memory", + "cronjob", "messaging", "clarify", ] def test_explicit_false_matches_default(self): @@ -60,12 +61,18 @@ class TestGateOn: disabled = _resolve_cron_disabled_toolsets(cfg) assert "cronjob" not in disabled - def test_interactivity_and_memory_denials_survive_the_gate(self): + def test_interactivity_denials_survive_the_gate(self): cfg = {"cron": {"allow_agent_scheduling": True}} disabled = _resolve_cron_disabled_toolsets(cfg) for name in ALWAYS_DISABLED: assert name in disabled + def test_memory_not_denied(self): + # Cron agents run with memory enabled like any other agent run + # (skip_memory=False); the toolset must not be policy-denied. + for cfg in ({}, {"cron": {"allow_agent_scheduling": True}}): + assert "memory" not in _resolve_cron_disabled_toolsets(cfg) + def test_user_denylist_wins_over_gate(self): # A user who denies cronjob in agent.disabled_toolsets keeps it # denied even with the gate on — the gate only removes the built-in @@ -94,6 +101,12 @@ class TestUserLayerUnchanged: # No duplicate when the user names an already-denied toolset. assert disabled.count("cronjob") == 1 + def test_user_can_still_deny_memory_for_cron(self): + # Memory is no longer policy-denied, but a user-level denylist + # entry still applies to cron runs. + cfg = {"agent": {"disabled_toolsets": ["memory"]}} + assert "memory" in _resolve_cron_disabled_toolsets(cfg) + def test_blank_and_whitespace_entries_ignored(self): cfg = { "cron": {"allow_agent_scheduling": True}, diff --git a/tests/cron/test_scheduler.py b/tests/cron/test_scheduler.py index 4bfe7b27c0..64033d0b09 100644 --- a/tests/cron/test_scheduler.py +++ b/tests/cron/test_scheduler.py @@ -118,23 +118,22 @@ 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): + def test_resolver_keeps_memory_in_per_job_list(self): result = _resolve_cron_enabled_toolsets( {"enabled_toolsets": ["memory", "file"]}, {"mcp_servers": {}}, ) - assert "memory" not in result + assert "memory" in result assert "file" in result - def test_resolver_strips_memory_from_platform_fallback(self): + def test_resolver_keeps_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 + assert result == ["file", "memory", "web"] class TestResolveOrigin: @@ -628,15 +627,15 @@ class TestRunJobSessionPersistence: yield fake_db, mock_agent_cls - def test_run_job_memory_toolset_disabled_in_cron(self, tmp_path): - """memory toolset must be disabled in cron sessions — issue #38129. + def test_run_job_memory_enabled_in_cron(self, tmp_path): + """Cron agents get memory like any other agent run. - 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. + skip_memory=False and the memory toolset is not policy-denied, so + MEMORY.md/USER.md load and the memory tool follows normal toolset + resolution. """ job = { - "id": "memory-hide-job", + "id": "memory-enabled-job", "name": "test", "prompt": "hello", } @@ -644,17 +643,13 @@ class TestRunJobSessionPersistence: run_job(job) kwargs = mock_agent_cls.call_args.kwargs - assert "memory" in (kwargs["disabled_toolsets"] or []), ( - "memory toolset should be disabled in cron to match skip_memory=True" + assert kwargs["skip_memory"] is False + assert "memory" not in (kwargs["disabled_toolsets"] or []), ( + "memory toolset must not be policy-denied in cron" ) - 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 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. - """ + def test_run_job_keeps_per_job_memory_toolset(self, tmp_path): + """A per-job enabled_toolsets naming memory keeps it.""" job = { "id": "memory-toolset-job", "name": "test", @@ -665,10 +660,10 @@ class TestRunJobSessionPersistence: run_job(job) kwargs = mock_agent_cls.call_args.kwargs - assert kwargs["skip_memory"] is True - assert kwargs["enabled_toolsets"] == ["file"] - assert "memory" not in (kwargs["enabled_toolsets"] or []) - assert "memory" in kwargs["disabled_toolsets"] + assert kwargs["skip_memory"] is False + assert "memory" in (kwargs["enabled_toolsets"] or []) + assert "file" in (kwargs["enabled_toolsets"] or []) + assert "memory" not in kwargs["disabled_toolsets"] def test_tick_skips_due_jobs_while_dispatch_is_paused(self, tmp_path): """The drain gate runs before advancing a due job's schedule.""" diff --git a/website/docs/developer-guide/cron-internals.md b/website/docs/developer-guide/cron-internals.md index 13a342324c..a18c1a9dd6 100644 --- a/website/docs/developer-guide/cron-internals.md +++ b/website/docs/developer-guide/cron-internals.md @@ -176,7 +176,9 @@ agent↔Nous wire contract lives in `docs/chronos-managed-cron-contract.md`. Each cron job runs in a completely fresh agent session: - No conversation history from previous runs -- No memory of previous cron executions (unless persisted to memory/files) +- No memory of previous cron executions (persistent memory — MEMORY.md / + USER.md — does load, like any other agent run, so durable preferences and + facts carry over; per-run conversation context does not) - The prompt must be self-contained — cron jobs cannot ask clarifying questions - The `cronjob` toolset is disabled (recursion guard) diff --git a/website/docs/guides/automate-with-cron.md b/website/docs/guides/automate-with-cron.md index dec05e43fd..a1a787fe88 100644 --- a/website/docs/guides/automate-with-cron.md +++ b/website/docs/guides/automate-with-cron.md @@ -123,7 +123,7 @@ Otherwise, provide a concise summary of the activity." --name "Repo watcher" --d ``` :::warning Self-Contained Prompts -Notice how the prompt includes the exact `gh` commands. The cron agent has no memory of previous runs or your preferences — spell everything out. +Notice how the prompt includes the exact `gh` commands. The cron agent has no conversation history from previous runs — spell everything out. (Persistent memory does load, so durable preferences saved to MEMORY.md carry over, but don't rely on it for job-critical details.) ::: ---