Skip to content

fix(cron): tag catch-up dispatches with scheduled vs actual time and lateness so late runs surface in UI - #99919

Closed
salch-cred wants to merge 4 commits into
NousResearch:mainfrom
salch-cred:fix/routine-missed-run-policy
Closed

salch-cred wants to merge 4 commits into
NousResearch:mainfrom
salch-cred:fix/routine-missed-run-policy

Conversation

@salch-cred

Copy link
Copy Markdown
Contributor

Problem (#99879)

A daily 0 9 * * * routine scheduled for 09:00 was observed running at 09:16–11:27 — up to 2.5h late. Gateway logs showed the previous gateway exited uncleanly and the overdue job was dispatched at restart. The Routines UI/CLI showed the late run as an ordinary "ok" run with no indication it was hours late. For reminders, that is functionally missed.

Root cause (verified on main)

cron/jobs.py: _get_due_jobs_locked handles overdue recurring jobs correctly as a scheduler — it fast-forwards next_run_at and runs once now to avoid deferring indefinitely. But it did so without provenance: no scheduled vs actual time, no lateness, no kind tag. The due job fell through to due.append(job) as a plain job, and the execution record and Routines UI had no way to distinguish catch-up from on-time.

Fix

Tag the in-memory job at the overdue branch with _dispatch_meta before dispatch:

job["_dispatch_meta"] = {
    "scheduled_at": "2026-09-01T09:00:00+00:00",
    "dispatched_at": "2026-09-01T09:31:00+00:00",
    "lateness_seconds": 1860.0,
    "dispatch_kind": "catch_up",
}
  • Transient: set on the deep-copied jobs entry, not raw_jobs — never persisted to jobs.json, never survives a restart on its own
  • Flows through tick()'s due list so execution records and future UI/CLI work can display "catch-up — scheduled 09:00, ran 09:31 (31m late)" instead of silently "ok"
  • Log line now says late by 1860s explicitly
  • On-time jobs (within grace) get no tag — behavior unchanged

Tests — tests/cron/test_routine_catch_up_dispatch_meta.py (2, isolated via mocks)

Case Contract
overdue 31m past 60s grace _dispatch_meta present with scheduled/dispatched/lateness/kind
due within grace (30s late, 300s grace) no _dispatch_meta

All 51 existing due/catch_up/grace cron tests pass unchanged.

Fixes #99879

Covers the Android psutil installer helpers:
- PsutilAndroidInstallError is a RuntimeError subclass
- MARKER/REPLACEMENT contain expected substrings
- _normalize_member_parts strips the tarball prefix
- PSUTIL_URL points to a .tar.gz for psutil
…lateness (NousResearch#99879)

Recurring jobs whose next_run_at is past the catch-up grace window were
re-dispatched once immediately but with no provenance — the Routines
UI/CLI showed the late run as an ordinary successful run. A 09:00 job
that actually ran at 09:31 (or 11:27 after an overnight gateway crash)
was indistinguishable from an on-time 09:00 run, and hours-late
reminders were functionally missed without the user knowing.

Fix: at the overdue-recurring branch in _get_due_jobs_locked
(cron/jobs.py:4007), before falling through to due.append(job), tag
the in-memory job with _dispatch_meta = { scheduled_at, dispatched_at,
lateness_seconds, dispatch_kind: 'catch_up' }. The tag is transient
(not persisted to jobs.json — it's on the deep-copied jobs entry, not
raw_jobs) and flows through tick()'s due list so execution records and
future UI work can display 'catch-up — scheduled 09:00, ran 09:31
(31m late)' instead of silently 'ok'.

The log line now says 'late by 31s' explicitly. All 51 existing
due/catch_up/grace tests pass; 2 new regression tests pin the tag on
overdue dispatch and the absence on on-time dispatch.

Fixes NousResearch#99879
@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 Sep 1, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference; please use your judgment.

Focused observability fix: catch-up dispatches now carry _dispatch_meta (scheduled vs actual time, lateness, kind) so overdue runs are distinguishable from on-time ones in records/UI, and the log line gains the lateness figure. Placement is correct — the tag is set inside the overdue-beyond-grace branch right before the due-append, so the on-time path stays untagged, and both behaviors are pinned by tests with sensible tolerances on the lateness assertion.

Non-blocking: the job dicts in this code path are storage-shaped, and the same flow sets needs_save = True for the fast-forward; worth confirming _dispatch_meta is stripped (or explicitly excluded) before any save_jobs write, so the underscore-prefixed internal key never lands in persisted state.

Non-blocking: tests/hermes_cli/test_psutil_android.py is unrelated to this change (same file is also added by the FIFO-rescue PR) — the duplicated new-file addition will conflict for whichever merges second, and its _normalize_member_parts assertion is near-tautological.

Verdict: LGTM

salch-cred added a commit to salch-cred/hermes-agent that referenced this pull request Sep 1, 2026
Flagged by automated review on both this PR and NousResearch#99919: the file tests an
unrelated module, was not part of this fix, and the duplicated addition
would conflict for whichever PR merges second. Removing it keeps this
diff scoped to the FIFO overflow rescue only.
Flagged by automated review on this PR and NousResearch#99912: the file tests an
unrelated module and the duplicated addition would conflict for whichever
PR merges second. Removing it keeps this diff scoped to the catch-up
dispatch metadata tagging only.
@salch-cred

Copy link
Copy Markdown
Contributor Author

Thanks — the persistence question was worth checking, and the answer is that the tag is transient by construction:

_get_due_jobs_locked\ deep-copies at the top of the scan — \jobs = [_apply_skill_fields(j) for j in copy.deepcopy(raw_jobs)]\ (cron/jobs.py:3675). _dispatch_meta\ is set on entries in that deep-copied list, while \save_jobs\ persists only
aw_jobs-shaped state (the needs_save fast-forward loops at the overdue branch iterate
aw_jobs, never \jobs). The underscore-prefixed key can never reach jobs.json.

Stale test file: removed in b0ebd1e — same as the note on #99912, it was carried by mistake and would have conflicted for whichever PR merged second.

@kshitijk4poor

Copy link
Copy Markdown
Contributor

Thanks @salch-cred — this was the right diagnosis and the right spot in _get_due_jobs_locked, but it has been superseded on main by #100445 (merged Sep 1), which covers the same #99879 provenance gap with a strictly stronger mechanism:

  • cron/jobs.py now stamps every recurring dispatch (including the beyond-grace catch-up branch) with last_dispatch = {scheduled_at, dispatched_at, lateness_seconds, kind} where kind is classified on_time / late / catch_up against the grace window. The lateness is computed against the original scheduled instant (next_run is not reassigned before the stamp), so the catch-up case carries the full lateness — same fields your _dispatch_meta proposed.
  • Unlike the transient in-memory tag here, the stamp is persisted to jobs.json, so hermes cron list / hermes cron status (separate CLI processes) can actually display "catch-up — scheduled 09:00, ran 09:31" — the transient tag had no consumer that could survive the process boundary.

Closing as superseded — your PR correctly identified the missing provenance and the exact branch that needed it, it just landed second. #99879 stays open pending any remaining UI-surface work.

@salch-cred

Copy link
Copy Markdown
Contributor Author

Acknowledged — #100445's persisted last_dispatch stamp is strictly better than my transient in-memory tag: it survives the process boundary so hermes cron list/status can actually display the provenance, which mine couldn't. Thanks for the close note and the credit on the diagnosis.

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.

Routines run late after gateway downtime with no missed-run status

4 participants