The future-instant guard in claim_job_for_fire dropped the occurrence
identity for ANY claim ahead of the stored next_run_at. A hosted/webhook
fire for the armed slot that arrives a few seconds early (the fire
scheduler's clock runs ahead of ours) was therefore treated as an
off-tick run: it ran occurrence-free, mark_job_run recomputed the same
cron slot from a now still before it, and the tick/misfire backstop then
ran the slot a second time.
Only claims at least FIRE_CLAIM_SKEW_SECONDS (60 s) ahead of the slot are
now classified off-tick, so dashboard/manual far-future fires stay
occurrence-free while a skewed early fire keeps the slot identity.
completed_occurrence honours the same window so the early run's
completion row (finished just before the slot) still proves the slot
done instead of being discarded as poison.
Review finding: claim_job_for_fire future-instant guard had no skew tolerance; an early hosted fire for the armed slot ran twice.
Replace the POSIX-only jobs-flock contention test (skipped off-POSIX,
~120 LOC of monkeypatched flock plumbing) with a single invariant test
that fails on pre-fix code in <1s: hold the per-job fire fence from a
worker thread, assert the heartbeat still returns True on the calling
thread, and that a takeover is still detected (False). The docstring on
heartbeat_fire_claim now records WHY it is not under the fence, so the
next refactor does not put it back.
Co-authored-by: Oliver Heckmann <46627487+oheckmann74@users.noreply.github.com>
Co-authored-by: salch-cred <141555468+salch-cred@users.noreply.github.com>
heartbeat_fire_claim only CAS-refreshes claim.at via _with_job; wrapping
_under_fire_fence across save_jobs let a blocked .jobs.lock pin the fence
and cause mark_job_run to fail closed on completed jobs.
claim_job_for_fire refused any claim younger than FIRE_CLAIM_TTL_SECONDS (300s)
regardless of whether its owner was still alive, so a `hermes cron run` killed
mid-flight (timeout, Ctrl-C, OOM) blocked the next manual run with "already being
fired" for up to five minutes. The executions table already reaps dead owners on
sight; the job-record fire_claim now does the same via gateway.status._pid_exists
when the claim's `by` names a pid on this host. Foreign hosts, explicit
HERMES_MACHINE_ID values, and probe failures keep the TTL (fail safe).
Second entry of the same bug class: POST /api/cron/jobs/{id}/trigger →
_fire_cron_job_for_profile → CronScheduler.fire_due → claim_fire built its claim
without `manual`, so an off-tick run from the web UI stamped the future slot exactly
like the tools path #105704 fixes. fire_due/claim_fire gain `manual` (forwarded only
when set, mirroring `force`, so third-party providers keep working) and the dashboard
trigger passes it when the provider's signature accepts it. Webhook and misfire
catch-up fires run the slot that is due and keep the stamp.
Also drops the base-green tick-stamp test (the same contract is pinned by
tests/cron/test_scheduled_occurrence.py) and documents `manual` vs `force`.
claim_job_for_fire() derives the occurrence identity from next_run_at
before the same function advances it. On a scheduler tick next_run_at is
the occurrence being run, which is correct; on an off-tick manual run it
is the NEXT occurrence, so the execution is stamped with the identity of
a slot that has not happened yet. _job_is_due() then finds a completed
execution carrying that identity and skips the real slot, returning
before the last_dispatch write — no error, no log line, no dispatch
record.
The manual flag already guards this and both _job_is_due() and
claim_job_for_fire() honour it; the agent-facing run-now path never
declared itself. Add a keyword-only manual= parameter and pass it from
_claim_for_manual_run(). Deliberately not force=True: force also calls
_activate_job_record(), which would resume a paused or disabled job, and
the run-now tool depends on continuing to refuse those.
The local flag is renamed to manual_fire so the new parameter is not
shadowed inside the apply closure, which would raise UnboundLocalError.
Three existing tests in tests/tools/ pinned the old call signature via
assert_called_once_with; they now pin manual=True, so dropping the flag
again fails loudly rather than silently reintroducing the skip.
Restores the intent stated in #104790 — the column records the scheduled
instant an execution was claimed for, and an off-tick manual run was
claimed for none.
Fixes#105690
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sibling tests outside the salvaged PR's files created one-shots via bare
'30m', which is now a recurring interval per the corrected contract.
Fixtures whose assertions depend on kind='once' (run-claim clearing,
terminal-record rearm, web-server completed-snapshot) now use 'in 30m';
sites indifferent to kind keep the bare form.
- claim_job_for_fire returns the atomically claimed snapshot with a unique
fire owner; heartbeat_fire_claim renews the lease; mark_job_run fences
terminal writes by expected_fire_owner so a stale worker cannot record
over a replacement claim.
- run_one_job heartbeats the fire claim and forwards a combined cancel
event (ownership loss OR external cancel) into run_job; the agent path
is interrupted cooperatively and script-based jobs (no_agent + pre-run
scripts) are hard-stopped with a process-tree kill (POSIX killpg
SIGTERM then SIGKILL for surviving group members; Windows
taskkill /T /F), with a bounded pipe drain so a SIGTERM-ignoring
descendant cannot wedge the worker on communicate() EOF.
- Shutdown interruption is scoped to the exact execution token instead of
the bare job ID, so a replacement run of the same job never consumes a
stale interrupted flag.
- fire_claim_fence serializes save/deliver side effects per profile+job
with a cross-process flock; remove_job prunes the fence-lock entry.
- Preserves upstream BaseException terminal recording (#73973),
completed one-shot retention (#80624), blocked_config preflight
(T1-26), and the advance_next_runs batch on top of current main.
* Revert "fix(cron): scope job execution to its owning profile (#32091 follow-up) (#50993)"
This reverts commit 660e36f097.
* Revert "fix(cron): anchor cron storage at the default root home (not the active profile)"
This reverts commit a5c09fd176.
`cron/jobs.py` resolved `HERMES_DIR`/`JOBS_FILE` from `get_hermes_home()`,
which follows the active profile override. So a job created from a
profile-scoped agent session (`hermes -p myprofile chat`, where the in-process
`cronjob` tool calls `create_job`) was written to
`~/.hermes/profiles/myprofile/cron/jobs.json`, while the profile-less gateway
(`hermes gateway run`) reads only `~/.hermes/cron/jobs.json`. The job was
silently orphaned: `cronjob action=list` from the same profile reported it
healthy (same file), but the gateway ticker never saw it and it never fired.
`last_run_at` stayed null forever. (#32091)
Fix: resolve the cron store from `get_default_hermes_root()` — the
purpose-built "profile-level operations" root that returns `<root>` even when
`HERMES_HOME` is `<root>/profiles/<name>` (and handles Docker/custom layouts).
Now the creator, the gateway scheduler, and the dashboard all agree on a
single jobs.json at the root, so a job created under any profile is visible to
the gateway.
Scope: this is the storage-location half of the fix. Making a job *execute*
under its originating profile's config/skills (a per-job `profile` field +
runtime context scoping, the #48649 sibling) is a separate, riskier change and
will follow as its own PR — keeping this layer minimal and safe.
Salvaged from #32117 by @mohamedorigami-jpg (authorship preserved). The
comprehensive #33839 (@sweetcornna) takes the same Option-A storage approach
and additionally adds the per-job profile execution scoping; this PR lands the
safe storage layer first.
Tests: `tests/cron/test_cron_profile_storage.py` — asserts the store anchors
at `<root>/cron` under a profile HERMES_HOME (not `<profile>/cron`), and is
unchanged when no profile is active. Full `tests/cron/` suite: 511 passed.
Fixes#32091
Co-authored-by: mohamedorigami-jpg <mohamed.origami@gmail.com>
Phase 4C. claim_job_for_fire(job_id, *, claim_ttl_seconds=300) in cron/jobs.py:
under the existing _jobs_lock() file lock, claim a job for a single external
fire so that across N gateway replicas exactly ONE wins. Single-machine
deployments always win (unaffected).
Semantics:
- missing / disabled / paused job → False.
- a fresh fire_claim (younger than claim_ttl_seconds) already present → False
(someone else holds it). Stale claim (crashed winner) → overwrite, so a job
is never wedged forever.
- on win: stamp fire_claim={at, by:_machine_id()}; for recurring (cron/interval)
advance next_run_at (mirrors advance_next_run's at-most-once bump so a stale
re-delivery can't re-fire); one-shots keep next_run_at but the fresh claim
blocks a duplicate retry for the same fire.
- mark_job_run now clears fire_claim on completion so a re-armed recurring job
is claimable again next fire.
_machine_id() (HERMES_MACHINE_ID env, else hostname:pid) is attribution-only;
correctness is the file lock + fresh-claim check, not the id.
This is consumed by CronScheduler.fire_due (Phase 4B). tick is untouched — it
still uses advance_next_run, so the built-in single-machine path is unaffected.
Tests (real store, temp HERMES_HOME): claim-once-then-block + next_run advance,
one-shot no-double-claim, unknown→False, paused→False, stale-claim reclaimable,
mark_job_run clears the claim (recurring re-claimable). tests/cron/ 470 passed.
teknium1