Skip to content

fix(cron): allow per-job memory_enabled so cron jobs can opt into the memory system - #34098

Closed
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/cron-memory-enabled-per-job
Closed

fix(cron): allow per-job memory_enabled so cron jobs can opt into the memory system#34098
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/cron-memory-enabled-per-job

Conversation

@liuhao1024

@liuhao1024 liuhao1024 commented May 28, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

The cron scheduler hardcoded skip_memory=True for all cron jobs, disabling the entire memory system (holographic memory, fact_store, memory tool). This blocked legitimate use cases:

  • Nightly memory optimisation jobs that mine session history for facts
  • Cron jobs that need to check user preferences before acting
  • Jobs that discover routing rules and want to persist them

Solution

Add a memory_enabled field (default False) to the cron job config. The scheduler reads it at runtime:

skip_memory=not job.get("memory_enabled", False)

Users opt in per job:

cronjob(action="create", schedule="0 3 * * *", prompt="...", memory_enabled=True)

Backward compatible — existing jobs continue to run with memory disabled by default.

Related Issue

Fixes #34094

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

File Change
cron/jobs.py Add memory_enabled param to create_job(), store in job dict
cron/scheduler.py Read memory_enabled from job config instead of hardcoding True
tools/cronjob_tools.py Expose memory_enabled in cronjob() create/update actions

How to Test

  1. Run pytest tests/ -q — all tests should pass
  2. Verify the specific scenario described above is resolved

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 26.4.1

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture and workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A

Code Intelligence

  • Analyzed: cron/scheduler.py:_run_job_impl (single call site for skip_memory), cron/jobs.py:create_job (job schema), tools/cronjob_tools.py:cronjob (user-facing tool)
  • Blast radius: LOW — adds a new field with safe default; no behavior change for existing jobs
  • Related patterns: skip_context_files already follows the same per-job opt-in pattern (not bool(_job_workdir))

Changes

File Change
cron/jobs.py Add memory_enabled param to create_job(), store in job dict
cron/scheduler.py Read memory_enabled from job config instead of hardcoding True
tools/cronjob_tools.py Expose memory_enabled in cronjob() create/update actions

Testing

  • tests/cron/test_scheduler.py — 128 passed ✅
  • tests/cron/test_jobs.py — 87 passed ✅
  • tests/tools/test_cronjob_tools.py — 56 passed ✅

… memory system

Closes NousResearch#34094

The cron scheduler previously hardcoded skip_memory=True for all jobs,
blocking legitimate use cases like nightly fact-mining or preference
reads. This change adds a memory_enabled field (default False) to the
job config so individual jobs can opt in.

- cron/jobs.py: add memory_enabled param to create_job(), store in dict
- cron/scheduler.py: read memory_enabled from job config
- tools/cronjob_tools.py: expose memory_enabled in cronjob tool create/update
@ValentinSergief

Copy link
Copy Markdown

Hi @liuhao1024 — opened #45769 which provides the default safe behavior (memory reads ON by default, writes blocked at dispatch sites). It explicitly complements your per-job memory_enabled approach here as the opt-in override for trusted maintenance jobs. No conflict — they stack. Would appreciate your review.

@ValentinSergief

Copy link
Copy Markdown

Thanks for the review on #45769! Updated the guard to check platform == "cron" so human follow-ups in cron sessions unlock memory writes (when you click a cron session and chat via TUI, _make_agent sets platform = "tui").

This keeps automated protection while allowing interactive memory cleanup — complements your per-job memory_enabled opt-in, which enables writes during the automated run itself. Nice layering.

@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 identifying the hard-coded cron memory exclusion; current main still passes skip_memory=True at cron/scheduler.py:3073.

Problems

  • The changed cronjob() parameter is not usable by the model tool as submitted: CRONJOB_SCHEMA has no memory_enabled property (tools/cronjob_tools.py:989-1088), and the registered handler does not forward it (tools/cronjob_tools.py:1119-1144).
  • The PR changes no tests. tests/cron/test_scheduler.py:960-1004 already captures AIAgent kwargs and is the natural place to cover absent/false/true job values.
  • The user-facing statement that cron uses skip_memory=True remains in website/docs/user-guide/features/spotify.md:218.

Suggested changes

  • Add schema and handler forwarding for memory_enabled, then test creation, update, and the scheduler's resulting skip_memory value.
  • Document the default-disabled per-job opt-in.

This is an automated hermes-sweeper review.

Comment thread tools/cronjob_tools.py
workdir: Optional[str] = None,
profile: Optional[str] = None,
no_agent: Optional[bool] = None,
memory_enabled: Optional[bool] = 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.

Adding this Python parameter alone does not expose it to the model: CRONJOB_SCHEMA currently has no memory_enabled property and the registered handler does not forward it to cronjob (tools/cronjob_tools.py:989-1144). Please add both wiring points and a regression test.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 13, 2026
@teknium1 teknium1 added the area/memory Memory subsystem: store, providers, sync, background reviews label Jul 19, 2026
@teknium1

Copy link
Copy Markdown
Contributor

Thanks @liuhao1024 — per-job memory_enabled was a solid design. We went with the simpler global flip in PR #91447 (merged as ef04d84): cron agents get memory unconditionally like every other agent, with the user-level agent.disabled_toolsets denylist as the opt-out. Closing as superseded.

@teknium1 teknium1 closed this Aug 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/memory Memory subsystem: store, providers, sync, background reviews comp/cron Cron scheduler and job management P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state tool/memory Memory tool and memory providers type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

cron: skip_memory=True blocks fact_store/memory tools from all cron jobs

4 participants