3-reviewer simplify pass (reuse/quality/efficiency) findings:
- cron/scheduler.py _run_job_script: the ORIGINAL that
lifecycle_guard._resolve_script_path documents mirroring had the exact
same unguarded expanduser() — a NUL-bearing script value survives
creation (the guard treats it as nothing-to-scan) and crashed the
scheduler at fire time with ValueError instead of a clean job failure.
Same ingestion contract applied; regression test added.
- lifecycle_guard._resolve_script_path: get_hermes_home() -> Path.home()
raises RuntimeError when neither HERMES_HOME nor HOME resolves
(arbitrary-UID containers); the cron entry point called it bare.
Caught -> None; totality test added.
- terminal_tool: stale 'cat ...' docstring updated to the bounded
head -c form.
- lifecycle_guard: dead 'script_text and' condition dropped (guarded by
'if not script_text: continue' directly above).
Efficiency reviewer: no material findings (measured — encode/expand
costs negligible vs walk I/O, no timing regression vs base).
Cron agents are constructed with skip_memory=True, so the memory
backend is not initialised — exposing the memory tool only gives the
model an unbacked tool that fails at runtime with 'Memory is not
available.' Add 'memory' to _resolve_cron_disabled_toolsets() so the
tool is stripped from the schema before the model can call it.
Fixes#38129.
Co-authored-by: Paolo Shamoon <Paolo@Dylans-Mac-Studio.local>
The AST invariant only matched `env["HERMES_KANBAN_X"] = ...` subscript
assignments, so a future dispatcher var added via `env.update({...})`,
`env.setdefault(...)`, or an annotated subscript would have slipped past
the guard and leaked into cron sessions unnoticed.
None of those shapes exist in _default_spawn today; this is about the
guard staying trustworthy as that function evolves.
Verified by injecting an unregistered var into _default_spawn one shape at
a time and requiring the guard to fail: subscript assign, annotated
assign, update(dict literal), setdefault(literal), and update(kwarg) are
all detected. Source restored byte-identical after probing.
tests/cron/ 410 passed; ruff clean.
A kanban worker that fires a cron job in-process no longer leaks its task
identity into the cron agent.
The worker is a normal `hermes chat -q` CLI agent whose default toolset
includes `cronjob`, running with HERMES_KANBAN_TASK legitimately set in its
own environment. `cronjob(action="run")` calls run_one_job() -> run_job()
in that same process, so the cron AIAgent was misidentified as that worker:
kanban toolset force-added, kanban-worker protocol injected into its system
prompt, and kanban_complete defaulting task_id to $HERMES_KANBAN_TASK --
letting an unrelated cron job close the worker's task and overwrite real
results.
Fixed with a ContextVar (`non_dispatcher_owned_context`), not by clearing
os.environ. The env is process-global and shared with three concurrent
readers that all need the real values:
* the worker's own claim heartbeat -- run_agent._touch_activity ->
heartbeat_current_worker_from_env reads TASK/CLAIM_LOCK/RUN_ID, and the
cron-run heartbeat thread drives it every 10s. Clearing them silently
no-ops the heartbeat, so after DEFAULT_CLAIM_TTL_SECONDS (15 min) the
dispatcher reclaims a task whose worker is still alive and re-dispatches
it -- the same duplicate-work failure from the other direction.
* the gateway's kanban watchers, which do their own HERMES_KANBAN_BOARD
save/restore around a slow decompose_task() LLM call.
* concurrent cron jobs, which take a *shared* read lock
(_terminal_cwd_lock.acquire_read) and so interleave: job A clears, job B
snapshots empty, A restores, B clears and its restore no-ops -- the
worker's identity is destroyed permanently.
`is_dispatcher_owned_worker_context()` is now the single predicate every
HERMES_KANBAN_* identity gate consults before trusting those vars. It also
closes a pre-existing gap in agent/skill_utils.py, which read the vars
without consulting the delegate_task ContextVar at all; the `kanban` verdict
additionally bypasses _ENV_DETECT_CACHE, since a context-dependent answer
must not be memoized process-wide.
HERMES_KANBAN_BOARD/DB/WORKSPACES_ROOT are left untouched, so the #20074
board pin and the dispatcher's path overrides keep working.
Tests: 18 new, including thread-isolation, concurrent-cron-jobs, and an AST
invariant over _default_spawn that fails if the dispatcher gains a var that
is neither identity-gated nor explicitly classified behaviour-only. All six
mutations are caught, including one that reintroduces the os.environ clear.
tests/cron/ + kanban suites 440 passed; model_tools/skill_utils/boards 63
passed; ruff clean.
Reported and diagnosed by Geoff Friesen (#78961), who identified the symptom
and the exact gating mechanism.
Co-authored-by: Geoff Friesen <gfriesen1@users.noreply.github.com>
Add activity_hb.join(timeout=2.0) after activity_hb_stop.set() in
direct_api_call's finally block so the heartbeat thread is deterministically
stopped before client teardown. Add test verifying no stray _touch_activity
fires after direct_api_call raises an exception.
Follow-up to PR #78548 by @xxxigm.
Top-level delegate_task runs in the background, and the 450s progress-stall
monitor only sees api_call_count / tool / last_activity_ts. Subagents use
non-streaming direct_api_call, which previously touched activity once and then
went silent — so a healthy local GGUF / long-prefill wait looked frozen and
was interrupted around ~450s as "Operation interrupted: waiting for model
response", even when child_timeout_seconds was raised. Refresh activity while
the inline request is open, and treat last_activity_ts advances as sync
heartbeat progress too.
Re-derivation of #33612 by @LeonSGP43 onto the rewritten scheduler (the
original is 10,692 commits behind; its tick() no longer exists in that
shape, so this is a fresh minimal fix crediting the PR's idea).
The gateway's built-in ticker calls tick(verbose=False) every 60s. The
idle early-return was gated on 'verbose and not due_jobs', so idle
GATEWAY ticks fell through to load_config() + worker-pool resolution
every minute. Return early on ANY idle tick; keep the post-tick MCP
orphan sweep (main intentionally reaps orphaned stdio children on idle
ticks).
3 new tests; mutation-checked (restoring the verbose-gated guard fails
the config-skip test). 66 scheduler tests green.
mark_job_run popped a finite one-shot from jobs.json the moment its
repeat limit was reached and returned early — discarding the
last_status / last_error / last_delivery_error it had just written.
Every finished one-shot vanished from `cronjob action=list` with no
inspectable record, and a delivery failure (agent succeeded, platform
send failed) was silently thrown away with it.
Changes:
- mark_job_run now retires a limit-reached one-shot as a terminal
record (state="completed", enabled=False, next_run_at=None) —
mirroring the existing next_run_at-is-None terminal branch — so the
final status and any delivery error persist and surface in the
cronjob tool's list output (which already emits last_delivery_error
and defaults to include_disabled=True).
- claim_dispatch's stale-job cleanup marks already-ran jobs completed
instead of popping them; genuinely wedged claims (last_run_at never
written) are still removed with the operator-visible diagnostic.
- Retention sweep in the due scan prunes completed one-shot records
older than cron.completed_retention_days (default 7; non-positive
disables) so jobs.json cannot grow unboundedly. Recurring jobs and
non-terminal one-shots are never candidates.
Tests: completion retains record + delivery error, list surfaces it,
completed jobs never re-dispatch, sweep prunes old / keeps recent /
ignores recurring / honors the disable knob; recurring lifecycle
unchanged.
Replace the process-global HERMES_CRON_SESSION env var with a per-session
ContextVar so a cron tick in the gateway process cannot leak into unrelated
live gateway/API/TUI turns. The cron scheduler now sets the ContextVar
inside the job's try/finally scope and resets it on cleanup. Gateway, API
server, ACP adapter, and TUI gateway all pass cron_session='' to explicitly
mark their sessions as non-cron, masking any stale process env.
Co-authored-by: hinablue <hinablue@gmail.com>
Closes#37968
The scheduler's pre-dispatch loop called advance_next_run per due job —
one full load_jobs() + one full save_jobs() of the jobs file each — so
N due jobs cost N reads + N writes of the whole file (gateway-restart
catch-up or co-scheduled bursts). advance_next_runs() does one load +
at most one save for the whole due set with identical per-job semantics;
advance_next_run() is now a thin wrapper over it.
Measured (50 due recurring jobs, real jobs file): 107.9 ms -> 2.5 ms
(45x; 50 loads + 50 saves -> 1 + 1).
Tests: batch advances recurring and skips one-shots, single load + save
I/O pin (fails pre-fix — no such function), no save when nothing
advances, and per-job wrapper semantics unchanged. Related: #60946 and
#75833 both restructure this loop's call site for correctness — neither
addresses the I/O cost, and this batch primitive composes with either
dispatch design; happy to rebase onto whichever lands first.
Fixes#73973.
A finite one-shot whose dispatch was claimed (claim_dispatch increments
repeat.completed BEFORE execution) but whose run died before mark_job_run
was left permanently wedged: completed==times, last_run_at null, state
'scheduled'. The run-claim TTL blocked re-dispatch, and once it expired
the dispatch-limit guard silently removed the job with no output and no
error.
Two complementary fixes:
- cron/scheduler.py run_one_job: the outer handler now catches
BaseException, not just Exception. The inner run_job handler re-raises
CancelledError/KeyboardInterrupt/SystemExit after agent teardown, and
none of those are Exception subclasses, so the outer 'except Exception'
missed them and mark_job_run(False) was never called. Failures are now
recorded first (mark_job_run + finish_execution, each independently
guarded), then non-Exception BaseExceptions are re-raised to preserve
teardown semantics. Plain Exceptions keep the existing behavior
(recorded, return False, no re-raise). Empty str(e) (bare
CancelledError) falls back to the exception class name.
- cron/jobs.py: when either removal site (claim_dispatch or the
get_due_jobs dispatch-limit guard) drops a one-shot whose claimed run
never completed (last_run_at null), _write_wedged_oneshot_diagnostic
now writes an operator-visible .md into cron/output/<job_id>/ instead
of vanishing silently. Best-effort: diagnostics can never break the
removal. No diagnostic when last_run_at is set (normal completion
race).
Issue #65773: run_one_job installs a <home>/.env secret scope around every
job; before c758ded6d (#69057) an installed scope was authoritative even
with multiplexing off, so provider keys injected only via the process
environment (container env vars, systemd Environment=) resolved to empty
inside cron and every provider call went out with the no-key-required
placeholder -> HTTP 401, while interactive turns kept working.
The fix landed in agent/secret_scope.py (scope-miss fallthrough to
os.environ when multiplex is off) with unit tests at that layer only.
These two tests pin the end-to-end contract where the bug actually
surfaced - cron's run_one_job:
- env-injected key resolves during run_job with multiplex OFF (fails on
pre-fix code, verified by mutation against c758ded6d~1)
- .env value still wins when both sources define the key (precedence)
Implementation-agnostic: passes whether the fix is the get_secret
fallthrough (main today) or a multiplex guard at the installation site
(the approach in #65801/#65802/#73037).
Per-job cron inference pins are now user-owned: the agent-facing cronjob
tool schema no longer exposes model/provider/base_url, and the registered
handler ignores them even if a model hallucinates the old parameters.
Users set pins via the dashboard, hermes cron create/edit --model/--provider,
or jobs.json directly — and once set, a pin sticks until the user changes it.
Existing agent-era pins are grandfathered untouched.
New cron.model / cron.model_provider config keys give the cron fleet its
own default model, independent of the chat model. Fire-time resolution:
per-job pin > cron.model > HERMES_MODEL > model.default. An axis covered
by the explicit cron-fleet default is deliberate routing, not drift, so
the #44585 fail-closed guard skips it — switching your chat model with
/model or hermes model no longer breaks unpinned cron fleets.
- tools/cronjob_tools.py: drop model param from agent schema + handler;
remove now-dead _resolve_model_override
- cron/scheduler.py: cron.model/model_provider resolution + per-axis
drift-guard skip
- cron/jobs.py: snapshot resolution mirrors the new precedence
- hermes_cli/subcommands/cron.py + hermes_cli/cron.py: --model/--provider
on hermes cron create/edit
- hermes_cli/config.py: cron.model / cron.model_provider defaults
- docs: cron.md model-resolution tip rewritten
When an operator changes the global model/provider config, warn that
unpinned cron jobs with stored snapshots will fail-closed on their next
run. Adds a cron.model_drift_guard config opt-out (default true) for
fleets that should deliberately track changing global defaults.
Addresses #59031. Original PR #59177 by @doncazper.
Root cause: delegate_task children run through three nested daemon-thread
layers (async-delegation executor -> per-child timeout executor -> the
interrupt worker interruptible_api_call spawns). After multi-day gateway
uptime the deepest layer wedges BEFORE the socket opens — the same
fingerprint as the gateway-cron hang (#62151): zero stale-detector output
(the worker never reaches dispatch), all providers, foreground/restart
works. The cron fix (should_use_direct_api_call) explicitly excluded
delegation 'for lack of evidence' — #60203 is that evidence.
- should_use_direct_api_call: extend the inline gate to delegated
children, detected via the delegation ContextVar set by
_run_single_child (platform='subagent' stamp as fallback). Scope
unchanged otherwise: chat_completions wire only; Codex/Anthropic/
Bedrock/MoA keep their established workers. Interrupts still work —
the inline path registers _active_request_abort, which interrupt()
invokes cross-thread (same mechanism the #72227 stall monitor uses).
- _dump_subagent_timeout_diagnostic: dump ALL thread stacks (bounded,
40), not just the conversation worker — a pre-HTTP wedge is
indistinguishable from a slow provider without seeing where the
nested helper threads sit.
Running any state-writing `hermes cron` CLI command as root (the
default for `docker exec`) rewrote jobs.json via mkstemp +
atomic_replace, leaving it root:root mode 600. The gateway's ticker
(uid 1000 via PUID/PGID) was then locked out of every tick with
PermissionError — silently: the liveness heartbeat stayed fresh,
`hermes cron status` opened with 'Gateway is running — cron jobs will
fire automatically', and in the field ~14h of scheduled jobs were
skipped before a human noticed the absence of messages.
Fixes, per the issue's suggested items 1 and 3:
1. Ownership preservation on save (cron/jobs.py): snapshot the owner
before the atomic replace; when the writer is privileged (euid 0)
and the previous owner differs, chown the rewritten file back.
First-time creation inherits the cron dir's owner. Unprivileged
writers never call chown. POSIX-only (guarded via os.name/getattr),
best-effort — a chown failure logs a warning but never breaks the
save. 0600 hardening is unchanged.
2. Zombie-ticker surfacing: the ticker loop (both single-profile and
multiplex paths) now persists the failure reason to a
ticker_last_error marker next to the heartbeat files on every
failed tick, and clears it on the next clean tick. `hermes cron
status` shows the recorded reason in its 'ticks may be failing'
branch, plus an actionable ownership hint when the error is a
PermissionError (recommend `docker exec -u <uid>:<gid>`).
Fixes#68483
Under multiplex_profiles, the gateway starts a single InProcessCronScheduler
bound to the process-global HERMES_HOME (the default profile's home), so
only that profile's cron/jobs.json is ticked. A job registered from a
secondary-profile session lands in <profile>/cron/jobs.json, reports a valid
next_run_at — and never fires.
Changes:
1. cron/scheduler_provider.py — InProcessCronScheduler.start() now accepts
an optional profile_homes kwarg (list of (name, Path) tuples). When set,
_start_multiplex() iterates tick() over each profile home using
use_cron_store(), so every served profile's cron store is ticked on
every tick cycle. Heartbeats and interrupted-execution recovery are also
scoped per profile via use_cron_store().
2. gateway/run.py — start_gateway() now resolves profiles_to_serve(multiplex=True)
when multiplex_profiles is on and passes them to the cron scheduler as
profile_homes. Only applies to InProcessCronScheduler (the built-in);
external providers are unchanged.
3. cron/jobs.py — record_ticker_heartbeat(), get_ticker_heartbeat_age(), and
get_ticker_success_age() now resolve paths via _current_cron_store()
instead of module-level TICKER_HEARTBEAT_FILE / TICKER_SUCCESS_FILE
constants. This makes heartbeats correctly scoped per profile, so
'hermes cron status' reflects liveness for every profile independently
under multiplex_profiles.
4. tests/cron/test_scheduler_provider.py — two new tests:
- test_multiplex_ticker_ticks_each_profile_once: verifies tick() is called
once per profile per tick cycle.
- test_multiplex_heartbeat_scoped_per_profile: verifies heartbeat files
are written to each profile's cron store.
The in_channel flat-delivery clear was gated on `runtime_adapter is not
None`, but the flat continuation session is seeded ONLY on the live-send
path, which also requires a running event loop. When an adapter is
present but the loop is absent/not-running, the live-send/seed block is
skipped and delivery falls through to the standalone path — clearing
thread_id there flattened an unseeded brief with no continuable session
behind it (and bypassed the D6 capability check).
Factor the live-send condition into `live_adapter_ready` and gate both
the thread_id clear and the live-send block on it, so the clear can no
longer drift from the seed. Add an adapter-present/no-running-loop
regression test (verified it fails against the un-scoped clear).
Addresses review r3609147550.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
cron_continuable_surface=in_channel is meant to deliver a cron brief FLAT
into a channel (thread_id=None) so a plain channel reply continues the job
via the shared-channel session. The scheduler already seeds that flat
session (_seed_cron_channel_session) but never cleared the ORIGINAL
target/origin thread_id before routing the actual delivery, so a job
scheduled from inside a Slack thread still delivered into that origin
thread — the seeded continuable session then never matched where the brief
actually landed.
Clear thread_id once in_channel_surface is finalized, before it is re-read
for route_thread_id / the standalone fallback. Scope the clear to EXACTLY
the target that gets flat-seeded — the origin-continuable target
(mirror_this_target) on a live, in_channel-capable adapter
(runtime_adapter is not None):
- fan-out / broadcast / explicit-thread targets keep their thread_id — they
are not continuable and are never seeded, so flattening them would
reroute an explicit thread and could collapse two targets into duplicate
flat sends;
- the standalone no-live-adapter path keeps the origin thread — it can
never seed the flat session, and without an adapter the D6 capability
fail-safe can't run, so flattening there would both drop the brief out of
any continuable lane and bypass D6.
This keeps the clear in lockstep with the seed condition
(in_channel_surface and mirror_this_target) and the D6 fail-safe.
mirror_this_target / origin_user_id are computed earlier using the original
thread_id to match the origin conversation.
Regression tests in TestCronContinuableSurfaceInChannel:
- a job whose origin carries a thread_id must not forward that thread_id to
the live adapter (DeliveryRouter folds target.thread_id into send
metadata), and the seeded continuable session stays flat (thread_id=None);
- with no live adapter, the standalone send falls back to the origin thread
rather than flattening.
With compression.in_place defaulting True the cron session id never
rotates; get_compression_tip returns the input id. Pin that the
salvaged #67188 fix titles/ends the ORIGINAL cron session in that path
(and for falsy tip returns), i.e. zero behavior change when no
compression rotation happened. Also maps colingreig's contributor
email for attribution CI.
run_job now passes target_model to resolve_runtime_provider; the codex
401-refresh test stubbed it with a requested-only lambda. Widen to
**kwargs like every other cron resolver stub.
Two follow-ups to the per-job model pin surface (#67472 / #49948 review):
- cron/scheduler.py: pass target_model=<effective job model> to
resolve_runtime_provider() on the primary path, so providers with
model-specific api_mode routing derive the mode from the model the job
actually runs (per-job pin > env > config default) instead of the stale
persisted default. The auth-fallback path already did this for its
fb_model.
- hermes_cli/web_server.py: POST /api/cron/jobs (and its sync worker) no
longer hardcodes profile="default" when the request carries no profile
param. A pool backend scoped to a named profile now resolves its own
profile via get_active_profile_name(), so pre-profileScoped desktop
clients can't write a named profile's job into ~/.hermes. Unscoped /
custom HERMES_HOME keeps the legacy default fallback.
Tests: target_model capture test on run_job; two profile-default tests on
the create endpoint.
Windows Notepad and PowerShell 5.1 Set-Content -Encoding UTF8 write a
leading UTF-8 BOM. json.load under encoding=utf-8 raises
JSONDecodeError("Unexpected UTF-8 BOM"), and load_jobs wraps that as
RuntimeError("Cron database corrupted and unrepairable"), taking down
cron CRUD/scheduler for a hand-edited jobs.json.
Read with utf-8-sig on all four independent jobs.json readers
(load_jobs primary + strict=False repair, dump _cron_summary, status
Scheduled Jobs). Write path stays plain utf-8 so the next save_jobs
heals a BOM'd file. Matches the env-class dialect (#65123).
Tests: BOM load (crash repro), bomless regression, empty store,
BOM+bare-list auto-repair, BOM+control-char strict=False arm, dump and
status CLI readers.
Review follow-up: the store-internals tests proved _current_cron_store()
resolves lazily, but not that the PUBLIC job I/O honors it. This exercises
save_jobs()/load_jobs() after a late env repoint and asserts the
import-time jobs.json stays byte-identical to a planted sentinel.
Review follow-up: tests that monkeypatch CRON_DIR/JOBS_FILE/OUTPUT_DIR (the
documented process-wide compatibility surface) were bypassed by the lazy
env fallback — 3 file-permission tests, the cross-process lock test, and
the heartbeat roundtrip regressed. _current_cron_store() now snapshots the
constants at import and honors any deliberate re-point of them ahead of
the env resolution, so the precedence is: use_cron_store() override >
patched constants > fresh HERMES_HOME > import defaults. Adds a test
pinning constants-beat-env; the late-env sentinel behavior is unchanged.
tests/cron: failure set byte-identical to unpatched main on this box
(the 5 regressions gone); 138 pass in the touched files.
Complements ec0227b43 (context-scoped cron store): the ContextVar override
is the right tool for deliberate cross-profile scoping, but with no
override active, _current_cron_store() returned the import-time constants —
so a HERMES_HOME set AFTER cron.jobs import (the filed incident: test
fixtures patching the env too late) still read/wrote the user's real
jobs.json. The fallback now resolves the active profile home fresh via
get_hermes_home() (context-local override, then env) and scopes the store
to it; when the home is unchanged since import, the exact module-level
constants are returned as before (zero change in the common path, and they
remain the documented compatibility surface). use_cron_store() still wins.
Three tests: late env repoint scopes the store; unchanged home returns the
import-time constants identically; an active use_cron_store() override
beats the env.
kshitij
Paolo Shamoon
Jeffrey Quesnelle
xxxigm
Alex Fournier
kshitij
Teknium
spfcraze
Fangliquan
victor-kyriazakos
tachyon-r
doncazper
joaomarcos
Victor Kyriazakos
webtecnica
Jake Long Vu
Wesley Simplicio
Vinay Bikkina
Colin Greig
Paulo Nascimento
Gille
slow4cyl
Trevor Gordon