Skip to content

fix(cron): non-dict schedule no longer freezes the whole scheduler - #61525

Closed
necoweb3 wants to merge 1 commit into
NousResearch:mainfrom
necoweb3:fix/cron-schedule-normalize
Closed

necoweb3 wants to merge 1 commit into
NousResearch:mainfrom
necoweb3:fix/cron-schedule-normalize

Conversation

@necoweb3

@necoweb3 necoweb3 commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Problem

A job record in jobs.json can have a non-dict value for the "schedule" field (e.g. null, a string, or an old format) when it is created or edited directly, or due to corruption.

In _get_due_jobs_locked:

schedule = job.get("schedule", {})
kind = schedule.get("kind")
...
recovered_next = compute_next_run(schedule, ...)
if kind in {"cron", "interval"}:
    ...

When schedule is not a dictionary, the code crashes with an AttributeError (or TypeError). This aborts the entire due-jobs scan before save_jobs() is called. As a result, healthy jobs lose their fast-forwarded next_run_at values and are never persisted.

This is the exact same failure mode as the recently fixed id-less job bug.

Root Cause

job.get("schedule", {}) returns the actual value when the key exists, even if it is not a dict. Later code assumes schedule is always a dictionary and calls .get("kind") or direct key access (schedule["kind"], schedule["expr"], schedule["minutes"]). There was no early normalization for this field.

Fix

Normalize non-dict schedule values to {} at the very beginning of _get_due_jobs_locked, before any code touches the schedule data. This follows the same approach used for id-less records.

Additionally, added defensive guards in compute_next_run, _compute_grace_seconds, and _recoverable_oneshot_run_at to prevent crashes if a bad schedule somehow reaches them.

Test

Added regression test test_bad_schedule_does_not_crash_or_block_sibling_jobs:

  • Creates one job with "schedule": null and one healthy past-due job.
  • Verifies that get_due_jobs() does not raise.
  • Verifies that the healthy sibling job is still returned.
  • The test fails on current main and passes with the fix.

A job record in jobs.json can have a non-dict 'schedule' value (null, string,
etc.) from direct edit or old writers.

In _get_due_jobs_locked:
  schedule = job.get('schedule', {})
  kind = schedule.get('kind')

This (and direct schedule['kind'] in compute_next_run etc.) raises and
aborts the entire due-jobs scan before save_jobs() or advancing next_run_at
for healthy jobs. Exactly the same failure mode as the id-less job P1.

Fix: normalize non-dict schedules to {} early (before any use), matching the
defense added for id-less records. Also added defensive guards in compute
functions.

Added regression test that a bad schedule does not crash and healthy sibling
is still returned.

Refs similar pattern in NousResearch#61382.
@alt-glitch alt-glitch added type/bug Something isn't working P1 High — major feature broken, no workaround comp/cron Cron scheduler and job management labels Jul 9, 2026
@alt-glitch

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Related cluster (cron/jobs.py malformed-record scheduler freeze) — same failure family as #61382 (id-less job aborts _get_due_jobs_locked before save_jobs()) and closed superset #51267 (per-job try/except quarantine so one bad job can't stall the whole scan); #40740 also guards compute_next_run against non-dict schedules. This PR targets a DIFFERENT malformed field (non-dict schedule) at the same function, so it is complementary rather than a duplicate. Verified on main: line ~1691 does schedule = job.get("schedule", {}); kind = schedule.get("kind"), which raises on a non-dict value and aborts the tick before persistence — the mechanism this PR fixes is real. Maintainer to pick canonical / merge order across #61382, #51267, #40740, and this PR.

teknium1 added a commit that referenced this pull request Jul 10, 2026
… class

Structural completion of the malformed-job freeze fixes (#61382 id-less,
#61525 non-dict schedule, #61581 bad next_run_at): wrap the per-job body
of _get_due_jobs_locked in try/except so any FUTURE malformed-field
variant degrades to skipping that one job for the tick instead of
aborting the scan before save_jobs() and freezing the whole profile's
scheduler.

Also: restore test_repeated_concurrent_runs_accumulate_completed_count
to TestMarkJobRunConcurrency (accidentally re-parented by the #61581
diff), add a containment regression test, and AUTHOR_MAP for hydracoco7.

E2E: one jobs.json carrying all five malformed shapes (drifted job_id,
missing id, null schedule, garbage next_run_at, non-string last_run_at)
plus a healthy sibling — single tick contains all five, sibling fires,
repairs persist, second tick stable. 670 cron tests green.
@teknium1

Copy link
Copy Markdown
Collaborator

Merged via PR #61723 — your commit was cherry-picked onto current main with your authorship preserved in git log (rebase merge). Thanks @necoweb3! Your schedule normalization landed alongside @hydracoco7's id repair (#61382, the earliest of the cluster) and your #61581 timestamp fix, plus a per-job containment guard that hardens the whole class. Your compute_next_run guards from this PR were kept as the cleaner of the two variants.

@teknium1 teknium1 closed this Jul 10, 2026
santhreal pushed a commit to santhreal/hermes-agent that referenced this pull request Jul 13, 2026
… class

Structural completion of the malformed-job freeze fixes (NousResearch#61382 id-less,
NousResearch#61525 non-dict schedule, NousResearch#61581 bad next_run_at): wrap the per-job body
of _get_due_jobs_locked in try/except so any FUTURE malformed-field
variant degrades to skipping that one job for the tick instead of
aborting the scan before save_jobs() and freezing the whole profile's
scheduler.

Also: restore test_repeated_concurrent_runs_accumulate_completed_count
to TestMarkJobRunConcurrency (accidentally re-parented by the NousResearch#61581
diff), add a containment regression test, and AUTHOR_MAP for hydracoco7.

E2E: one jobs.json carrying all five malformed shapes (drifted job_id,
missing id, null schedule, garbage next_run_at, non-string last_run_at)
plus a healthy sibling — single tick contains all five, sibling fires,
repairs persist, second tick stable. 670 cron tests green.
justemu pushed a commit to justemu/hermes-agent that referenced this pull request Jul 18, 2026
… class

Structural completion of the malformed-job freeze fixes (NousResearch#61382 id-less,
NousResearch#61525 non-dict schedule, NousResearch#61581 bad next_run_at): wrap the per-job body
of _get_due_jobs_locked in try/except so any FUTURE malformed-field
variant degrades to skipping that one job for the tick instead of
aborting the scan before save_jobs() and freezing the whole profile's
scheduler.

Also: restore test_repeated_concurrent_runs_accumulate_completed_count
to TestMarkJobRunConcurrency (accidentally re-parented by the NousResearch#61581
diff), add a containment regression test, and AUTHOR_MAP for hydracoco7.

E2E: one jobs.json carrying all five malformed shapes (drifted job_id,
missing id, null schedule, garbage next_run_at, non-string last_run_at)
plus a healthy sibling — single tick contains all five, sibling fires,
repairs persist, second tick stable. 670 cron tests green.
Gravezzz pushed a commit to Gravezzz/hermes-agent that referenced this pull request Jul 21, 2026
… class

Structural completion of the malformed-job freeze fixes (NousResearch#61382 id-less,
NousResearch#61525 non-dict schedule, NousResearch#61581 bad next_run_at): wrap the per-job body
of _get_due_jobs_locked in try/except so any FUTURE malformed-field
variant degrades to skipping that one job for the tick instead of
aborting the scan before save_jobs() and freezing the whole profile's
scheduler.

Also: restore test_repeated_concurrent_runs_accumulate_completed_count
to TestMarkJobRunConcurrency (accidentally re-parented by the NousResearch#61581
diff), add a containment regression test, and AUTHOR_MAP for hydracoco7.

E2E: one jobs.json carrying all five malformed shapes (drifted job_id,
missing id, null schedule, garbage next_run_at, non-string last_run_at)
plus a healthy sibling — single tick contains all five, sibling fires,
repairs persist, second tick stable. 670 cron tests green.
xyshanren added a commit to xyshanren/hermes-agent-cn that referenced this pull request Aug 6, 2026
…0 改旧)

跟 plan CAND-003 1:1 配对 (跟 K-7 k7_commands.py + CAND-001/008 1:1 配对 0 改旧):
- 新 hermes_cli/cron_containment.py (跟 CAND-008 1:1 配对 additive 0 改旧):
  * _CRON_JOB_EXCEPTIONS: 6 异常类型 (KeyError/TypeError/ValueError/AttributeError/
    RuntimeError/OSError, 跟 upstream 10c0d9b 1:1 配对 NousResearch#61382/NousResearch#61525/NousResearch#61581 等
    历史 fix)
  * 2 functions: is_cron_exception (pure read, 0 副作用) / safe_run_due_job
    (try/except whitelist + on_error 注入让 test 不静默吞, 跟 CAND-008 1:1 配对)
- 0 改 hermes_cli 现有 file (跟 UX 倒退审计 1:1 配对 additive 0 改)
- whitelist pattern 跟 CAND-008 fnmatch 0 命 1:1 配对 (0 类型 propagate 0 catch)
- 1 个坏 job 0 卡整个 scheduler (跟 plan 1:1 配对)

4 test pass (2 静态 + 2 live, 跟 K-10 1:1 配对):
- test_cron_containment_module_exists
- test_cron_6_exception_types (verify 6 异常类型完整)
- test_is_cron_exception_live (9 场景含 6 已知 True + 2 未知 False + None False)
- test_safe_run_due_job_live (5 场景含 成功 / KeyError / ValueError / 后续 0 影响 / unknown propagate)

跟 mavis 4 件套 1:1 配对:
- 后端先调查再设计: 借 CAND-001 + CAND-008 0 改旧 1:1 配对
- Cherry-pick split bug class: 0 cherry-pick
- UX 倒退审计: 0 改 hermes_cli 现有 file, 抽 file additive 0 改
- 估时前必 verify 引擎能力: 实际 0.25h (跟 K-10 1:1 配对)

跟 AIMC 4 铁律 1:1: 0 改 upstream / CN 端可维护 / 0 改 upstream 决策边界
(跟 upstream 10c0d9b 1:1 配对 cron due scan 容错)
xyshanren added a commit to xyshanren/hermes-agent-cn that referenced this pull request Aug 7, 2026
…0 改旧)

跟 plan CAND-003 1:1 配对 (跟 K-7 k7_commands.py + CAND-001/008 1:1 配对 0 改旧):
- 新 hermes_cli/cron_containment.py (跟 CAND-008 1:1 配对 additive 0 改旧):
  * _CRON_JOB_EXCEPTIONS: 6 异常类型 (KeyError/TypeError/ValueError/AttributeError/
    RuntimeError/OSError, 跟 upstream 10c0d9b 1:1 配对 NousResearch#61382/NousResearch#61525/NousResearch#61581 等
    历史 fix)
  * 2 functions: is_cron_exception (pure read, 0 副作用) / safe_run_due_job
    (try/except whitelist + on_error 注入让 test 不静默吞, 跟 CAND-008 1:1 配对)
- 0 改 hermes_cli 现有 file (跟 UX 倒退审计 1:1 配对 additive 0 改)
- whitelist pattern 跟 CAND-008 fnmatch 0 命 1:1 配对 (0 类型 propagate 0 catch)
- 1 个坏 job 0 卡整个 scheduler (跟 plan 1:1 配对)

4 test pass (2 静态 + 2 live, 跟 K-10 1:1 配对):
- test_cron_containment_module_exists
- test_cron_6_exception_types (verify 6 异常类型完整)
- test_is_cron_exception_live (9 场景含 6 已知 True + 2 未知 False + None False)
- test_safe_run_due_job_live (5 场景含 成功 / KeyError / ValueError / 后续 0 影响 / unknown propagate)

跟 mavis 4 件套 1:1 配对:
- 后端先调查再设计: 借 CAND-001 + CAND-008 0 改旧 1:1 配对
- Cherry-pick split bug class: 0 cherry-pick
- UX 倒退审计: 0 改 hermes_cli 现有 file, 抽 file additive 0 改
- 估时前必 verify 引擎能力: 实际 0.25h (跟 K-10 1:1 配对)

跟 AIMC 4 铁律 1:1: 0 改 upstream / CN 端可维护 / 0 改 upstream 决策边界
(跟 upstream 10c0d9b 1:1 配对 cron due scan 容错)
leewenjie pushed a commit to leewenjie/hermes-agent that referenced this pull request Aug 7, 2026
… class

Structural completion of the malformed-job freeze fixes (NousResearch#61382 id-less,
NousResearch#61525 non-dict schedule, NousResearch#61581 bad next_run_at): wrap the per-job body
of _get_due_jobs_locked in try/except so any FUTURE malformed-field
variant degrades to skipping that one job for the tick instead of
aborting the scan before save_jobs() and freezing the whole profile's
scheduler.

Also: restore test_repeated_concurrent_runs_accumulate_completed_count
to TestMarkJobRunConcurrency (accidentally re-parented by the NousResearch#61581
diff), add a containment regression test, and AUTHOR_MAP for hydracoco7.

E2E: one jobs.json carrying all five malformed shapes (drifted job_id,
missing id, null schedule, garbage next_run_at, non-string last_run_at)
plus a healthy sibling — single tick contains all five, sibling fires,
repairs persist, second tick stable. 670 cron tests green.
randlee pushed a commit to randlee/hermes-agent that referenced this pull request Aug 11, 2026
… class

Structural completion of the malformed-job freeze fixes (NousResearch#61382 id-less,
NousResearch#61525 non-dict schedule, NousResearch#61581 bad next_run_at): wrap the per-job body
of _get_due_jobs_locked in try/except so any FUTURE malformed-field
variant degrades to skipping that one job for the tick instead of
aborting the scan before save_jobs() and freezing the whole profile's
scheduler.

Also: restore test_repeated_concurrent_runs_accumulate_completed_count
to TestMarkJobRunConcurrency (accidentally re-parented by the NousResearch#61581
diff), add a containment regression test, and AUTHOR_MAP for hydracoco7.

E2E: one jobs.json carrying all five malformed shapes (drifted job_id,
missing id, null schedule, garbage next_run_at, non-string last_run_at)
plus a healthy sibling — single tick contains all five, sibling fires,
repairs persist, second tick stable. 670 cron tests green.
pierrenode added a commit to pierrenode/hermes-agent that referenced this pull request Aug 11, 2026
… repair doesn't reach

A stored or caller-supplied non-dict schedule (null, a stray int, etc.)
is repaired by _get_due_jobs_locked() during the periodic due-scan tick
(NousResearch#61525), but resume_job(), mark_job_run(), claim_dispatch(),
advance_next_run()/advance_next_runs(), claim_job_for_fire(), and
update_job()'s inherited-schedule fallback can all run on a record the
scan hasn't repaired yet — e.g. right after a direct jobs.json edit, or
on a paused job the due-scan skips. Each of those raw job["schedule"].get()
call sites crashed with AttributeError instead of gracefully treating the
record as an empty schedule.

Introduces _job_schedule_dict(job), which repairs job["schedule"] in
place (mirroring the due-scan tick's own repair), and threads it through
all six sites. advance_next_runs() and claim_dispatch() additionally
persist the in-place repair via save_jobs() even on paths that return
before their normal save would otherwise run it, so the fix survives a
restart instead of re-raising on the next direct call.

update_job()'s guard is narrowed to (dict, str) so a legitimate raw
string schedule update (e.g. "every 10m") is still parsed correctly.
melon-xf added a commit to melon-xf/hermes-agent that referenced this pull request Sep 3, 2026
… class

Structural completion of the malformed-job freeze fixes (NousResearch#61382 id-less,
NousResearch#61525 non-dict schedule, NousResearch#61581 bad next_run_at): wrap the per-job body
of _get_due_jobs_locked in try/except so any FUTURE malformed-field
variant degrades to skipping that one job for the tick instead of
aborting the scan before save_jobs() and freezing the whole profile's
scheduler.

Also: restore test_repeated_concurrent_runs_accumulate_completed_count
to TestMarkJobRunConcurrency (accidentally re-parented by the NousResearch#61581
diff), add a containment regression test, and AUTHOR_MAP for hydracoco7.

E2E: one jobs.json carrying all five malformed shapes (drifted job_id,
missing id, null schedule, garbage next_run_at, non-string last_run_at)
plus a healthy sibling — single tick contains all five, sibling fires,
repairs persist, second tick stable. 670 cron tests green.
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 P1 High — major feature broken, no workaround type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants