fix(#913): scheduler honors per-agent execution_timeout_seconds

[obasilakis]

Summary

• agent_schedules.timeout_seconds becomes Optional[int] and round-trips None through the scheduler to /api/internal/execute-task, so the backend's per-agent fallback at task_execution_service.py:281 finally fires for cron-triggered runs.
• Migration null_legacy_schedule_timeouts nulls existing rows at the historical defaults (900, 3600) so PUT /api/agents/{name}/timeout takes effect on already-deployed schedules.
• Drive-by: S-03 canary invariant is made decay-invariant (it would fire by 1s on every fresh slot once bug: scheduled runs ignore per-agent execution_timeout_seconds (canary S-03 + E-01 surfaced) #913 makes slot TTL match the floor exactly).

Root cause (#913)

Before this PR, every schedule row carried a concrete timeout_seconds (DEFAULT 900, later rewritten to 3600 by _migrate_default_execution_timeout_to_3600). The scheduler passed that to the backend, so the backend's if timeout_seconds is None: timeout_seconds = db.get_execution_timeout(agent_name) fallback was dead code on the scheduler path. PUT /api/agents/{name}/timeout was silently ineffective for cron — the per-agent value never reached slot acquisition, slot HASH metadata, or cleanup-watchdog termination.

The canary harness fired S-03 / E-01 every cycle on any agent whose per-agent timeout differed from the schedule default.

Fix — Option A (round-trip None through scheduler)

• ScheduleCreate / Schedule / ScheduleResponse / scheduler Schedule: timeout_seconds: Optional[int] = None
• _row_to_schedule (backend + scheduler): NULL → None (not 900/3600)
• scheduler/service.py: _call_backend_execute_task, _poll_execution_completion, _execute_retry accept Optional[int]; polling deadline falls back to _POLL_DEADLINE_WHEN_NULL = 7200 (the upper clamp of PUT /api/agents/{name}/timeout) so the scheduler never gives up before the backend's own enforcement
• schema.py: drops DEFAULT 3600 on agent_schedules.timeout_seconds
• Migration: null_legacy_schedule_timeouts nulls rows at 900/3600 (same fidelity tradeoff as the prior _migrate_default_execution_timeout_to_3600)

The cleanup-side COALESCE in get_running_executions_with_agent_info already preferred the per-agent value when the schedule's column was NULL; this PR is what finally makes the column actually carry NULL.

S-03 decay-invariance follow-up

After #913 the slot TTL exactly equals the floor at creation, so the raw ttl < floor check fires by ~1s on natural decay — Redis TTL decays linearly from EXPIRE. The fix reconstructs the initial TTL via ttl + age, where age = snapshot_time - slot_score (slot_score is the unix epoch recorded by SlotService at ZADD time). A real #226-class bug (initial TTL set to 1200s while floor is 3900s) still surfaces; natural decay does not.

1s tolerance (floor - 1) absorbs Redis's float→int rounding on the wire.

Test plan

• tests/scheduler_tests/test_per_agent_timeout.py — 6 new tests pin the round-trip:
  • DB NULL → Schedule.timeout_seconds is None
  • DB explicit int → unchanged on read
  • Dispatch payload carries "timeout_seconds": null (not a default integer)
  • Explicit per-schedule overrides still propagate unchanged
  • _POLL_DEADLINE_WHEN_NULL == 7200
  • _poll_execution_completion does not TypeError on None
• tests/test_canary_invariants.py — added test_natural_decay_not_below_floor as S-03 regression; existing S-03 fixtures updated to use realistic acquire scores
• All 85 canary unit tests + 6 scheduler tests pass locally
• Slot HASH on canary-fleet-slow: timeout_seconds=180 (per-agent), TTL=480 — was 3600/3900 before
• Slot HASH on canary-fleet-burst: timeout_seconds=3600 (per-agent), TTL=3900 — was 900/1200 before
• 3-min live run spanning burst (/2) and slow (/3) cron cadences: 0 canary violations
• Review

Note on supersedence

PR #920 was opened earlier on feature/882-canary-phase2, which carried duplicate Phase 2/3 canary commits already merged to dev via #884. Closed in favor of this clean two-commit PR off dev.

Fixes #913

🤖 Generated with Claude Code

[vybe]

Validation Report — REQUEST CHANGES

The core fix is well-reasoned and correctly implemented: round-tripping None through the scheduler boundary, the migration that nulls legacy 900/3600 rows, and the S-03 decay-invariance fix. One line was missed that turns every default-timeout schedule creation into a 500.

❌ Critical — TypeError on schedule creation

ScheduleCreate.timeout_seconds is now Optional[int] = None (the whole point of the PR), but create_schedule at routers/schedules.py:269 calls _enforce_timeout_below_agent_cap(name, schedule_data.timeout_seconds) unconditionally.

Inside that function:

if requested_seconds > cap:   # None > int → TypeError in Python 3

Result: POST /api/agents/{name}/schedules with the default timeout_seconds (i.e., every consumer that doesn't explicitly set it) returns 500 after this lands.

Fix — mirrors the already-correct UPDATE path at line 330:

# routers/schedules.py:269
if schedule_data.timeout_seconds is not None:
    _enforce_timeout_below_agent_cap(name, schedule_data.timeout_seconds)

---

⚠️ Warnings (non-blocking)

1. architecture.md S-03 description is stale — the doc says S-03 checks TTL ≥ floor directly; after this PR it compares reconstructed initial TTL (ttl + age) against floor - 1. Worth updating while the change is fresh.

2. agent_schedules schema in architecture.md missing timeout_seconds — pre-existing gap, but worth adding timeout_seconds INTEGER, -- #913: NULL = inherit per-agent timeout to the DDL block since we're touching this column.

---

Required before merge

• routers/schedules.py:269 — add if schedule_data.timeout_seconds is not None: guard before the _enforce_timeout_below_agent_cap call

Everything else (DB layer, scheduler service, migration, S-03 fix, tests) looks correct. One guard and this is ready.

[obasilakis]

Addressed all three points in 854758d (rebased onto current dev first so #929's _enforce_timeout_below_agent_cap is in scope).

Critical — TypeError on schedule creation ✅
Added the if schedule_data.timeout_seconds is not None: guard in create_schedule at routers/schedules.py:271–273, mirroring the UPDATE path you flagged. Also tightened the helper's docstring so a future caller can't unknowingly reintroduce the bug.

Architecture S-03 description (stale) ✅
Rewrote the S-03 paragraph in docs/memory/architecture.md to describe the reconstructed initial TTL (ttl + age, where age = snapshot_time − slot_score) compared against floor − 1, plus the 1s tolerance rationale.

agent_schedules DDL missing timeout_seconds ✅
Added timeout_seconds INTEGER, -- #913: NULL = inherit agent_ownership.execution_timeout_seconds to the DDL block.

Regression tests — three new in tests/unit/test_929_timeout_validation.py:

1. test_enforce_helper_raises_typeerror_on_none — pins the helper's behavior on None so the guard's necessity is documented in code.
2. test_create_schedule_path_skips_enforcement_when_timeout_none — invokes the route function directly with stubbed deps; asserts _enforce_timeout_below_agent_cap is not called when caller omits timeout_seconds. Would have caught your reported 500.
3. test_create_schedule_path_runs_enforcement_when_timeout_set — counterpart proving the guard doesn't accidentally disable enforcement when the caller does set the field.

Verification: 9/9 in test_929_timeout_validation.py, 91/91 across tests/scheduler_tests/test_per_agent_timeout.py + tests/test_canary_invariants.py, 23/23 in tests/test_schedules.py.

[vybe]

All four items from the prior review are addressed cleanly:

• ✅ routers/schedules.py:271 — is not None guard added on the create path, mirroring the UPDATE path. Helper docstring now documents the precondition.
• ✅ architecture.md S-03 description rewritten for decay-invariance (ttl + age vs floor - 1).
• ✅ agent_schedules.timeout_seconds added to the schema DDL block with the inherit-on-NULL comment.
• ✅ Regression tests in tests/unit/test_929_timeout_validation.py pin both sides — helper TypeErrors on None (so the guard can't silently regress), create-path skips the enforcer on None, enforcer still runs when caller sets timeout explicitly.

Bonus catch: switched asyncio.get_event_loop() → asyncio.run() after a CI-only failure on Python 3.11 — clean test hygiene.

CI green across pytest (×6 seeds), CodeQL, schema-parity, regression diff. Approving.