fix(agent): isolate periodic scheduler callbacks from blocking siblings (#102574)

[Finn763]

What Problem This Solves

PeriodicScheduler ran every periodic body inline on its single daemon thread (agent/periodic_scheduler.py::_run calling handle._fn()), so one blocked callback — a slow SQLite/lease-refresh call, a wedged filesystem or network-adjacent heartbeat — stalled every other due timer in the process. Since the shared scheduler now hosts safety-critical work (durable turn-lease refresh, turn-liveness watchdog, delegated-child heartbeat), that coupling made a single blocked callback a process-wide liveness failure domain: leases expired while their refresher was starved, and the liveness watchdog never fired.

This change keeps the one daemon scheduler thread as the heap/timing owner, but dispatches each due body onto its own short-lived daemon worker (hermes-periodic-callback-*). A blocked callback can no longer delay an unrelated due sibling. Existing semantics are preserved:

• A handle never overlaps itself: it is re-queued only after its in-flight run returns.
• cancel(wait=) still blocks for that handle's in-flight run (now a worker join), never for unrelated callbacks, and cancel-from-within-a-callback cannot deadlock.
• A False return still stops the callback; a raised exception is still logged at debug and rescheduled.
• No per-handle thread is created at schedule time, so steady-state thread use stays near zero and worker growth is bounded by the number of handles that currently have an in-flight run.

Evidence

Reproduced and verified locally on 866332bfb52c46e543143b2620a9aeee8bce9c77 (Python 3.11.15, Windows 11):

• Red on baseline: the regression from the issue, tests/agent/test_periodic_scheduler.py::test_blocked_callback_does_not_stall_due_sibling, fails on unmodified origin/main with AssertionError: a blocked periodic callback stalled an unrelated due callback (the sibling misses its 300 ms deadline).
• Green after the fix: the same test passes, and test_slow_callback_never_overlaps_itself confirms a slow callback never overlaps itself.
• Sabotage-verify: reverting only agent/periodic_scheduler.py (keeping the new tests) turns the regression red again; restoring the fix turns it green.
• Reliability: tests/agent/test_periodic_scheduler.py passed 20/20 consecutive runs; the regression test passed 15/15 consecutive runs.
• Consumer suites: tests/agent/test_periodic_scheduler.py tests/agent/test_turn_facade_lease.py tests/agent/test_turn_liveness.py tests/run_agent/test_turn_liveness_watchdog.py tests/tools/test_heartbeat_stale_thresholds.py → 35 passed.

Known pre-existing, unrelated flake: tests/run_agent/test_tool_activity_heartbeat.py::test_heartbeat_touches_periodically_and_stops (a wall-clock time.sleep(0.12) vs a 0.05 s cadence) fails intermittently on unmodified origin/main as well (observed 1/5 on baseline, 2/5 with the fix). It does not use periodic_scheduler; agent/tool_executor.py::_run_tool_activity_heartbeat owns its own thread.

Closes #102574

[andrexibiza]

Reviewed exact head e18b5b5218cda55e389d6aff4ca8e59921c9ea1a against exact base/current main@866332bfb52c46e543143b2620a9aeee8bce9c77, including the full two-file diff, scheduler consumers (DurableTurnLease refresh + liveness watchdog and delegated-child heartbeat), #102574, the introducing #102030 change, and the existing #102458 / #102615 / #102657 / #104488 implementation lineage. Exact-head CI, Docker, and Nix are all green; this is a one-commit train and that commit has hosted proof.

The core direction is right: moving callback bodies off the heap owner closes the concrete process-wide starvation failure in #102574, the reschedule-after-return placement preserves per-handle non-overlap, and moving cancel(wait=) to the tracked runner means one handle no longer waits behind unrelated work. The two new regressions cover the principal success path.

P1 — worker-start failure silently retires safety machinery. In _dispatch(), any runner.start() exception sets handle._runner = None and then handle._cancelled = True. That is not equivalent to the callback returning False: it means a transient OS thread-allocation failure permanently deletes the timer without its body ever running. This scheduler directly owns DurableTurnLease.refresh_tick, TurnLivenessWatchdog._tick, and delegated-child heartbeat. None of those callers polls handle.cancelled after scheduling, so a failed worker start can silently remove the lease refresher and watchdog. For the lease path that is materially worse than a delayed tick: the durable lease can expire while the original turn continues, with no refresher callback available to notice/interrupt it, allowing a successor process to acquire the row while the old turn is still alive.

This is also the other side of the scaling tradeoff here. The old bounded-pool attempts (#102615, #102657, later #104488) were correctly rejected because a full pool recreates the very shared starvation domain being fixed. Per-handle workers avoid that coupling, but they make every tick depend on a fresh thread allocation; under the exact fan-out/resource-pressure regime that motivated #102030, start failure becomes part of the safety contract rather than an impossible edge. Please make dispatch failure settle explicitly instead of masquerading as cancellation: e.g. a degraded inline/retry path or another mechanism that preserves the handle and surfaces the failure, with a deterministic regression that forces callback-worker Thread.start() to raise and proves the lease/watchdog timer is not silently retired. A debug-only log plus permanent cancellation is not enough for a safety timer.

There are two non-runtime cleanup points I would include before landing because this branch is now the likely current-main carrier of the fix:

• agent/turn_liveness.py, agent/turn_facade_lease.py, and tools/delegate_tool_child_run.py still describe these bodies as running “on the shared scheduler thread”. #102458 updated the then-current consumer comments, but subsequent refactors moved that ownership. If this lands without the corresponding current locations, the in-tree concurrency contract becomes false exactly where future race reviews will read it.
• Provenance/topology needs to be explicit. #102458 by jfreshpicks predates this PR, implements the same per-handle-worker mechanism, and was explicitly recorded on #102574 as the canonical fix; #102615/#102657/#104488 are distinct bounded-pool alternatives that were closed because they preserve an N-blocked-callback failure domain. Since #102458 is now based on an older tree and this PR is a clean current-main rematerialization, superseding it can make sense, but it should preserve jfreshpicks' prior implementation credit rather than presenting this as an independent ownership line. #102030/teknium1 remains the originating performance change, with the shared-thread coupling called out in its own caveat.

Once the worker-start settlement hole is closed, the architecture here is substantially stronger than current main: timer ordering stays centralized, callback failure domains become per-handle, and the existing bounded-pool dead end is avoided. The exact-head green matrix is a good receipt; the remaining issue is making resource-exhaustion failure preserve the same safety guarantees the scheduler exists to provide.

[alt-glitch]

This was generated by AI during triage.

Duplicate of #102458 — same fix for #102574 (per-handle daemon worker dispatched from the single heap-owning scheduler thread, no self-overlap, cancel(wait=) joins the runner with a current-thread guard). #102458 is earlier and additionally updates the call-site docstrings; predecessors #102615/#102657/#104488 were already closed. Leaving it to the maintainer to pick which one to merge.

[Finn763]

Context for reviewers: #102458 (opened 2026-09-03, branch candidate/scheduler-isolation-20260903) targets the same root cause — isolating periodic scheduler callbacks so one blocked callback can't stall its siblings.

This PR was opened after gh pr list --search "102574" returned nothing: #102458's title doesn't reference the issue number, so it didn't match. As of today #102458 is CONFLICTING with main and has no CI runs recorded. This branch is rebased on main, all 37 checks are green, and it adds regression coverage (test_blocked_callback_does_not_stall_due_sibling, test_slow_callback_never_overlaps_itself).

Happy to defer if you'd rather take the earlier approach — say the word and I'll close this one.

[kshitijk4poor]

Landed via #106308, which cherry-picks this branch onto current main with your authorship preserved (rebase-merge, no squash) and has merged as 2369606f98; closing this one as landed. @jfreshpicks's earlier #102458 is credited in the body for the same design.

Thanks for the fix — the credit stays with you in the commit history and the PR body.