Skip to content

feat(cron): make skip_memory configurable via config.yaml - #51309

Closed
YeuSheng wants to merge 1 commit into
NousResearch:mainfrom
YeuSheng:feat/cron-skip-memory-config
Closed

YeuSheng wants to merge 1 commit into
NousResearch:mainfrom
YeuSheng:feat/cron-skip-memory-config

Conversation

@YeuSheng

Copy link
Copy Markdown

Summary

Currently skip_memory is hardcoded to True for all cron jobs in cron/scheduler.py:2077:

skip_memory=True,  # Cron system prompts would corrupt user representations

This prevents users who want scheduled tasks to benefit from conversation memory (e.g., context-aware daily briefings, memory-informed status reports) from opting in.

Change

Replace the hardcoded value with a config.yaml lookup:

skip_memory=bool(_cfg.get("cron", {}).get("skip_memory", True)),

Default remains True — zero breaking change for existing setups. Users who want memory context in their cron jobs can set:

cron:
  skip_memory: false

Motivation

Some scheduled tasks (e.g., daily self-review, weekly retrospectives) benefit from knowing the user's accumulated preferences and context. The hardcoded True forces all cron sessions into a memory-agnostic mode regardless of user intent.

Test Plan

  • Verified locally: cron jobs run with skip_memory=true (default) and skip_memory=false without errors
  • Existing cron behavior unchanged when config key is absent

Previously, skip_memory was hardcoded to True for all cron jobs.
This prevented users who want cron jobs to access conversation
memory (e.g., context-aware scheduled tasks) from opting in.

Now reads from config.yaml:

  cron:
    skip_memory: true   # default, preserves existing behavior

The default remains True to avoid breaking existing setups, but
users can set it to false when their scheduled tasks benefit from
memory context.
@alt-glitch alt-glitch added type/feature New feature or request comp/cron Cron scheduler and job management P3 Low — cosmetic, nice to have duplicate This issue or pull request already exists labels Jun 23, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Duplicate of #9825 — same one-line change in cron/scheduler.py making skip_memory configurable via config.yaml (cron.skip_memory, default True). #9825 is the earlier open PR and also includes regression tests. Tracking feature request: #9763.

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review Summary

Verdict: Approved

13-line change making skip_memory configurable via config.yaml. Default remains True (cron system prompts would corrupt user representations). Clean and backwards-compatible.


Reviewed by Hermes Agent

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for addressing a real cron-memory limitation. The current implementation needs a safer scope before it can be considered.

Problems

  • cron/scheduler.py:3073 is an intentional isolation boundary: commit 005e0ec4 added skip_memory=True to prevent cron system prompts from corrupting user representations (bug(honcho): cron sessions write system instructions to Honcho as user messages, corrupting user representation #4052). A global cron.skip_memory: false would remove that protection for every cron job.
  • This is broader than enabling a memory query path. With skip_memory=False, agent/agent_init.py:1363-1444 initializes both local memory and external providers; run_agent.py:3403-3424 then synchronizes each completed cron turn and queues provider prefetch.
  • The one-line diff adds no regression coverage, DEFAULT_CONFIG entry, or documentation for the new setting.

Suggested changes

  • Keep cron memory isolated by default and use an explicit per-job opt-in that separates provider tools from automatic prompt injection and turn synchronization.
  • Add tests proving default cron jobs remain isolated and that any opted-in mode follows its intended read/write lifecycle.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform labels Jul 15, 2026
@teknium1

Copy link
Copy Markdown
Collaborator

Closing as superseded by PR #91447 (merged as ef04d84) — cron memory is now enabled unconditionally rather than config-gated. @nightq's #9825 was the earliest PR in this cluster. Thanks for the contribution @YeuSheng.

@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

comp/cron Cron scheduler and job management duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants