Twenty loops across 17 files hand-wrote the same thing:
deadline = time.monotonic() + timeout
while time.monotonic() < deadline:
if condition(): return ...
time.sleep(interval)
with small drifts between copies: some re-checked once after the loop,
some did not, one slept before its first check, and update_cmd_windows
kept a private _poll_until. A copy without the final check can miss a
condition that becomes true during the last sleep.
agent/deadline.py (already the home of clamp_timeout / run_bounded_*)
gains poll_until(predicate, timeout, interval): checks at once, cuts the
last sleep to the time left, checks again at the deadline, and returns
the predicate's last value so callers that wait for a pid / row /
identity get it back directly. timeout <= 0 is one check.
Converted: outbound_webhooks.flush, cron delivery wait and jobs flock,
host_attach channel wait, browser profile unlock, data-cleanup drain,
gateway dbus/bus waits, launchd fresh-pid and supervision waits,
supervised restart, gateway_migrate live-gateway wait, Windows
drain/absent waits, kanban init lock, local-runtime boot wait,
update_cmd_windows (private helper deleted), bot desktop start/stop
waits, TUI browser connect.
Left as is: loops whose body does more than check (progress callbacks,
interrupt handling, two distinct exits) and one site whose blind except
the health gate would reject once moved.
Behaviour changes, all small:
- loops that stopped at the deadline without re-checking now make one
more check there;
- loops that already re-checked after a full last sleep now cut that
sleep to the time left, so the check lands at the deadline (up to one
interval sooner);
- a timeout <= 0 now means one check (some loops made none);
- the local-runtime boot wait checks before its first sleep instead of
after it;
- _wait_for_user_dbus_socket also exports the user systemd env when the
socket appears on that final check (before, that path returned True
without it).
update_cmd_windows is imported by update_cmd on every OS, so the
fresh-source-install acceptance fixture (which keeps agent/ out of the
update path) now copies the stdlib-only agent/deadline.py too.
Tests: 4 pins for poll_until (two red against the classic
no-final-check variant), plus the existing suites for every touched
module.
recover_abandoned() fences a 'delivering' row whose gateway died as 'unknown'
(never retried), but nothing wrote that outcome back: the firer's settle_quietly
had already run against a still-'queued' row, and the drain's own write-back only
covers rows it finished itself. The job stayed last_status=delivery_queued with
no error and the ledger stayed delivery_outcome=queued forever, so `hermes cron
list` reported a delivery in progress for a send that may never have happened.
recover_abandoned now collects the job id of every row it fences (before the
same-transaction prune redacts job_json) and, after commit, calls
settle_quietly for each: job -> delivery_failed with the unknown-send error,
ledger -> failed (delivery_outcome maps unknown to failed). Execution-id fencing
in _settle_job_record keeps a later run's status untouched. No resend.
Red on the previous source: test_dead_drain_owner_settles_the_run_it_fenced_unknown
sees last_status 'delivery_queued'. Green after; test_delivery_queue.py and
test_delivery_outcome_writeback.py still pass.
Review: https://github.com/NousResearch/hermes-agent/pull/106742#pullrequestreview-5460639040
Agent cron runs hand their send to the durable delivery queue and book
last_status=delivery_queued / ledger delivery_outcome=queued. Nothing ever
wrote the drain's result back, so after a gateway sent (or refused) the
message the job stayed "delivery still in progress" forever and a refused
send never surfaced as delivery_failed + last_delivery_error (review M1;
main records ok/delivered and delivery_failed/failed + error).
cron/delivery_outcome.py settles a terminal queue row onto:
- the execution ledger: only a terminal attempt whose delivery_outcome is
still 'queued' (new executions.settle_delivery_outcome), and
- the job record: only while the execution is both the ledger's latest
attempt for the job and the job's latest canonical completion, so a later
run's status is never rewritten.
It runs after the drain's _finish, after the run's own bookkeeping (a drain
can win the race before mark_job_run lands; _finish_completed_run also
books the settled outcome instead of 'queued'), and after receipt
recovery. Write-back failures are logged, never undo the at-most-once send.
Restores main's assertions in the rewritten W1 tests, checked after an
explicit drain: test_warning_execution_outcome (ok/delivered|suppressed),
test_cron_failure_deliver (delivered / failed outcome after drain),
test_fire_claim_lost_after_delivery (last_status ok),
test_script_claim_heartbeat self-removal (delivery_outcome delivered).
New invariant test: test_delivery_outcome_writeback (sent/refused x drain
after run / drain before bookkeeping).
(cherry picked from commit 0402eff3d91050bbfa68b83ba6f3d7169700da3f)
The cron SQLite stores write hermes_time.now().isoformat(), an ISO string
that carries the local UTC offset. That offset changes at a DST transition
and after a timezone config change, so the text order of these columns is
not their time order. Across a fall-back, 01:10-05:00 is 20 minutes after
01:50-04:00 but sorts before it. latest_execution(), latest_executions()
(which fills job["latest_execution"] for `hermes cron list` and the
dashboard), list_executions() and its before_claimed_at cursor, the
delivery queue's pending-claim order, delivery and execution pruning, and
list_incidents() all returned the older row first.
Every ORDER BY and the cursor comparison on these columns now use
julianday(col), which SQLite resolves to the same instant for any offset.
The raw text stays as the next sort key, so rows within the same
millisecond (julianday's resolution) keep their microsecond order, and the
cursor compares the same (instant, text) key on both sides. Stored values
and the schema are unchanged, so existing rows with mixed offsets order
correctly without a migration.
latest_executions() moves from a correlated per-row subquery to one
ROW_NUMBER() window. With julianday() in the ORDER BY the index on
claimed_at no longer serves the per-row LIMIT 1, and the correlated form
took about 90 ms at 1000 rows; the window form takes about 1 ms.
Refs #86520
Thanks to #86889 (zhao0112) for the executions-ledger diagnosis.
A restart-safe cron worker runs outside the gateway, queues its final send in
cron/deliveries.db and polls that row once a second. The gateway drained the
queue only as a housekeeping chore at the 60 s tick, so a reminder whose turn
ended in 8 s reached the user a minute later (the issue's 60.86 s silence).
The housekeeping thread now watches the queue file (and its WAL) between
ticks and drains as soon as a stamp moved; the tick's drain stays as the
fallback. The watch never resets itself after draining: the drain's own status
write costs one empty pass, a worker that enqueued during the drain still
wakes it, and an empty pass writes nothing, so the stamp settles.
Fixes#117307
The suppression feature needs one cron cell: a failure notice whose target hides warning
notifications is recorded as `suppressed` (bot_chat_pending record, deliveries queue row,
execution delivery_outcome) instead of being sent. That is kept.
Everything else the PR added to cron/ is an independent ledger rework and is removed here:
execution delivery manifests + filesystem manifest journal, `schema_meta` fence and the
"legacy intent adoption" layer, incident occurrence generations + trigger, jobs projection
CAS (`bind_delivery_execution`/`update_delivery_projection`), the per-tick projection
reconciler, and the `_deliver_targets`/`_settle_manifest` split. origin/main has none of
the state that layer migrates from (zero occurrences of delivery_manifest / manifest_journal
/ schema_meta); the "pre-flag old writer" the r5-r9 tests simulate is a vendored snapshot of
this PR's own earlier revision (tests/cron/_r5_prior_executions.py). Those fixes may have
merit on their own and should land as separate PRs with a main-reproducing test each.
Also restores main's `_classify_delivery_outcome` precedence (`failed` before `queued`)
and drops the SHA-pinned `git show <PR commit>` test, which would go red the moment a
rebase-merge rewrote that commit.
Squash of the 54 commits on victor-kyriazakos:feat/user-channel-warning-suppression
(PR #112302, head f45c640e55) so the contributor's authorship survives a rebase-merge;
the commits interleave with a cron delivery-ledger rework that the salvage removes in
follow-up commits, so per-commit cherry-picks were not practical.
Adds display.suppress_warning_notifications (global + per-platform, default false):
one resolver (gateway/warning_notifications.py), BasePlatformAdapter.emit_warning /
emit_media_warning / warning_text, a notification_category classification carried
through wakes, queues and persistence, and render/present boundaries for CLI/TUI.
cron/ledger.py (e24c8499) existed so a long-running scheduler that lazily imports
notepad/incidents AFTER `hermes update` never needs new names from a module it already has
cached. The dedup deleted it and imported open_db/transaction from hermes_cli.sqlite_util at
module level; a pre-upgrade daemon has the OLD sqlite_util cached (executions imported
add_column_if_missing from it), so the first job tick after an upgrade would ImportError in
scheduler_prompt._build_job_prompt until restart.
- cron/{notepad,incidents,executions,delivery_queue}: import open_db/transaction/
add_column_if_missing and cron.jobs._ensure_cron_dir inside _connect/_transaction/
_initialize_schema. This also stops the 3.8k-line cron.jobs being pulled eagerly by
importing a store (it was lazy in cron/ledger.open_ledger).
- gateway/hosted_rooms_common, hosted_room_policy_checkpoint: same treatment; the gateway
imports hosted_rooms lazily from request handlers, so it has the same skew exposure.
- tests/cron/test_upgrade_module_skew.py: simulate the real skew (delete open_db/transaction
from the cached sqlite_util, then import each store). The previous repoint deleted names
from cron.executions, which notepad/incidents do not import from, so it passed regardless.
Sabotage: a module-level `from hermes_cli.sqlite_util import open_db` in notepad fails it
with "cannot import name 'open_db'".
Twelve modules each carried their own sqlite3.connect + PRAGMA + `with conn:`
stack. The #69567 fd-leak fix (a `with conn:` commits but never closes, so each
call leaked a connection and its WAL/SHM fds until GC) was pasted as code plus
docstring into six of them and hosted_room_policy_checkpoint never received
it; plugins/plugin_storage.plugin_db was the only production caller issuing a
raw `PRAGMA journal_mode=WAL`, bypassing the network-FS fallback, the
WAL-reset-bug gate and the never-live-downgrade invariant that
hermes_state_wal.apply_wal_with_fallback carries.
hermes_cli/sqlite_util.py (already home to add_column_if_missing/write_txn,
imported by cron, gateway and hermes_cli alike) gains `open_db(path, *,
db_label, busy_timeout_ms, wal, foreign_keys, synchronous_full, row_factory,
check_same_thread, wal_lock_retries, initialize)` and `transaction(conn,
immediate=)`; cron/ledger.py is deleted and hosted_rooms_common's
open_sqlite/connect/transaction become 1-3 line forwarders. Migrated:
agent/verification_evidence, cron/{executions,incidents,notepad,
delivery_queue}, gateway/{delivery_ledger,hosted_room_policy_checkpoint,
hosted_rooms_common (-> hosted_rooms, hosted_room_driver)}, hermes_cli/
projects_db, tools/async_delegation, plugins/plugin_storage.
Behavior changes (each module keeps its effective PRAGMA set otherwise):
- hosted_room_policy_checkpoint: connection now closed after every use and
on init failure (was leaked per call), busy_timeout PRAGMA set explicitly.
- projects_db: gains busy_timeout=5000 (was the sqlite3 default 5 s connect
timeout with no PRAGMA); explicit and observable.
- delivery_ledger / async_delegation: busy_timeout PRAGMA now mirrors the
10 s connect timeout they already had.
- plugin_storage.plugin_db: WAL through apply_wal_with_fallback (DELETE on
network filesystems / WAL-reset-vulnerable builds instead of raw WAL);
busy_timeout=5000.
- cron/incidents._redact_error: redact_sensitive_text(force=True) — the
error text is persisted to disk.
- delivery_ledger's private duplicate-column guard and the unguarded
`ALTER TABLE ADD COLUMN` sites (shared_metrics, api_server_run_idempotency,
holographic store, kanban model_override) go through add_column_if_missing.
- hermes_state.py::_scrub_surrogates: dead byte-copy of
hermes_state_messages._scrub_surrogates (0 callers) deleted.
hermes_state.py: delete every '# noqa: F401 (re-exported...)' import block (hermes_state_common/errors/guard/
readpool/sessions/fts/dbfile/wal/repair/registry + agent.context_compressor _DB_PERSISTED_MARKER_KEY); keep
only the names hermes_state.py itself uses, without noqa.
hermes_state_registry.py: drop get_shared_session_db/release_shared_session_db/close_shared_session_dbs
aliases; every caller (gateway/, tools/, tui_gateway/, cron/, mcp_serve, run_agent, tests) now imports
acquire/release/close_all/release_or_close from hermes_state_registry.
hermes_state_titles.py: drop set_auto_title_if_empty shim (title_generator keeps its getattr fallback).
Re-remove shim-only names restored by 34abf954bd: latest_user_message_row_id (tests call
latest_message_row_id(key, role='user'); role-targeting assertions kept) and get_session_activity (tests
build the snapshot via agent.session_activity.build_activity_snapshot over db.get_session(sid)).
hermes_state_wal._log_once resolves its dedupe sets as module globals instead of via hermes_state;
hermes_state_repair helpers call module globals directly (tests patch hermes_state_repair.<name>).
Frozen updater surface untouched (update_cmd_maint imports only SessionDB from hermes_state).
Review fold-in on the salvage of #101877:
- `_deliver_result` routed to the durable queue whenever the worker's
`_HERMES_CRON_EXTERNAL_WORKER` marker was set, regardless of WHICH job was
delivering. A worker whose script dispatches another job in-process
(`hermes cron run <other>`) inherits that env and would have queued the
nested job's message under the outer execution id — `INSERT OR IGNORE`
then drops it silently. Match the marker against the delivering job's own
`execution_id`, as `run_one_job` already does. Regression test added
(mutation-checked: fails with the guard removed).
- A `pending` row left queued at the worker's wait timeout was still
reported as a delivery error, so `mark_job_run` recorded
`last_status=delivery_failed` for a message the next gateway's drain goes
on to send, and nothing ever corrects the job record. Log and return
success instead; the deliveries row is the authority for the send.
- Reuse `cron.executions._TERMINAL_STATES` in the parent wait loop instead
of a second hardcoded terminal set.
Follow-up to the salvaged restart-safe worker (#101877):
- delivery_queue: a row still `pending` at the worker's wait timeout was
marked `failed` and never drained, so any gateway outage longer than the
300s budget (e.g. a restart that runs `hermes update`) silently lost the
delivery. Unclaimed rows are certainly unsent, not uncertain — leave them
queued for the next gateway; only mid-send rows are fenced `unknown`.
- delivery_queue: stop running the full-table prune UPDATE+COUNT inside
every transaction (each `get_status` poll paid for it; terminalizing
paths already prune explicitly); poll at 1s instead of 250ms.
- delivery_queue/executions: use `hermes_state.apply_wal_with_fallback`
(bare `journal_mode=WAL` raises on NFS/SMB homes) and the race-safe
`hermes_cli.sqlite_util.add_column_if_missing`; drop the copied
owner-liveness helpers in favour of the ones in cron.executions.
- scheduler: the parent waited on the worker by re-opening the executions
ledger every 50ms for the whole run (~20 opens/s, hours). Wait on the
process with a 1s timeout instead — the worker commits its terminal row
before exiting — and reap stranded payload/ack files once terminal.
- scheduler: skip the housekeeping drain until a worker has actually
created deliveries.db, so non-systemd gateways never open it.
- scheduler: set up hermes logging in the detached worker entrypoint; it
runs with stdout/stderr on DEVNULL and previously logged nowhere.
- tests: test_lost_fire_claim_stops_stale_delivery still mocked
`mark_execution_running -> None`, which now means "ownership lost, return
before run_job" — the test passed without ever reaching the path it
names. Mocking `{}` restores it (mutation-checked).
kshitijk4poor