Cron hot-reload: CronService.reload + POST /api/cron/reload (5/13)

docs/cron.md

@@ -26,6 +26,43 @@ Both files use the same format. On startup, CronService loads and merges both:
 
 Running `nerve init` on an existing install regenerates `system.yaml` (e.g., to pick up updated prompts from a Nerve update) without touching `jobs.yaml`.
 
+### Hot-reload (no restart)
+
+`POST /api/cron/reload` re-reads `jobs.yaml` and `system.yaml` and applies them
+to the running daemon, so a job you add, remove, reschedule, enable or disable
+takes effect **without a restart**. The same call re-reads the gate-plugins
+directory, so an added, edited or deleted plugin file lands with it. A
+`prompt_file` needs no reload: its contents are read on every run.
+
+A reload rebuilds every enabled job from the file, and leaves source runners and
+internal cleanup/wakeup jobs alone.
+
+Notes:
+- Rebuilding a job normally leaves its next fire time alone: a crontab is
+  absolute, and an interval is measured from the job's last successful run. An
+  interval job that has never succeeded has no such anchor, so each reload
+  restarts its countdown. On a fresh install, where nothing has run yet, editing
+  the file repeatedly keeps every interval job's wait starting over — they
+  settle once each has run for the first time.
+- A **malformed** `jobs.yaml`/`system.yaml` (bad YAML, or a job that doesn't
+  parse) is refused: reload returns `400` and the running schedule is left
+  untouched, so a typo can't wipe your crons.
+- A reload is **all-or-nothing**. The whole change set is computed (files parsed,
+  every trigger built) before the scheduler is touched, so a reload that fails
+  for any reason leaves every job on its existing schedule instead of applying
+  half the change. Gate plugins are covered too: a refused reload leaves the
+  registered gates as they were, so a deleted plugin file only takes its gate
+  away once a reload succeeds.
+- A job holding a [reserved id](#job-fields) is **skipped**, at reload and at
+  startup alike. The reload itself still succeeds; only that job is dropped.
+- An **invalid schedule** (a crontab whose fields the scheduler rejects, e.g.
+  `99 * * * *`) is refused the same way at reload: `400`, nothing applied.
+  Startup is the one case that differs — the daemon logs an error naming the job
+  and comes up without it, rather than taking every other cron down over one
+  typo. `GET /api/cron/jobs` still lists the job, with a null `next_run`.
+- `show_session_label` is **restart-only**: changing it and reloading has no
+  effect until the daemon restarts.
+
 ## Job Definition
 
 ```yaml
@@ -83,8 +120,8 @@ prompt definition.
 
 | Field | Type | Required | Description |
 |-------|------|----------|-------------|
-| `id` | string | yes | Unique job identifier |
-| `schedule` | string | yes | Crontab expression or interval (`2h`, `30m`) |
+| `id` | string | yes | Unique job identifier. `cleanup`, `wakeup_sweep` and anything starting with `source:` are **reserved by the daemon** — a job using one is skipped (with a warning naming it in the daemon log) and never scheduled, so rename it |
+| `schedule` | string | yes | Crontab expression or interval (`2h`, `30m`, `1h30m`, `0.5h`) — see [Interval syntax](#interval-syntax). A 5-field crontab with an out-of-range field (`99 * * * *`) is **rejected**, never reinterpreted as an interval — reload returns `400` and startup skips that one job with an error in the daemon log |
 | `prompt` | string | yes* | Message sent to the agent |
 | `prompt_file` | string | yes* | Path to a file containing the prompt (relative to the YAML's directory). Read fresh each run; shareable between jobs. *One of `prompt`/`prompt_file` is required |
 | `description` | string | no | Human-readable description |
@@ -101,6 +138,31 @@ prompt definition.
 | `run_if` | list | no | Run gates — preconditions that must all hold for the job to fire. See [Run Gates](#run-gates) |
 | `workflow` | map | yes* | Launch a budget-capped [workflow run](workflow-runs.md) instead of a prompt: `{engine, prompt, budget_usd[, title, model, effort, cwd]}`. Takes precedence over `prompt`/`prompt_file`; the cron job only launches the run (fire-and-forget) — the run notifies on its own |
 
+### Interval syntax
+
+A `schedule` that isn't a 5-field crontab is read as an interval: a run of
+`<number><unit>` tokens, where the unit is `h`, `m` or `s`. Tokens add up, and
+fractions are allowed.
+
+| Schedule | Interval |
+|----------|----------|
+| `4h` | 4 hours |
+| `30m` | 30 minutes |
+| `90s` | 90 seconds |
+| `1h30m` | 90 minutes (`1h 30m` works too) |
+| `0.5h` | 30 minutes |
+| `10.5m` | 10 minutes 30 seconds |
+
+A fractional interval is rounded to the nearest whole second (`1.333m` → 80s).
+
+Anything that isn't a whole string of such tokens is **not an interval** and
+silently falls back to **every 2 hours** — `hourly`, `@daily`, `4x`, `1h junk`,
+and a zero interval (`0h`, or a fraction that rounds down to zero seconds), which
+would otherwise mean "fire as fast as you can". The daemon keeps running on a
+conservative cadence rather than refusing to start over one mistyped field, so a
+job firing every 2 hours when you asked for something else means the schedule
+string didn't parse.
+
 ## Run Gates
 
 A **run gate** is a precondition evaluated right before a job fires. It answers
@@ -134,9 +196,17 @@ When multiple gates are listed, the job runs only if every one passes:
         sources: [gmail, github]
 ```
 
-Gates are **fail-open**: if a gate errors while checking (e.g. a transient DB
-issue), the run proceeds rather than being skipped — an occasional wasted run
-beats a cron that silently never fires.
+Gates are **fail-open while checking**: if a gate raises during evaluation (e.g.
+a transient DB issue), the run proceeds rather than being skipped — an occasional
+wasted run beats a cron that silently never fires.
+
+A gate that can't be *built* is a different matter and is **refused**. If a
+`run_if` entry names a type nothing provides — a typo, or a gate plugin that was
+deleted or fails to import — the job does not load: reload returns `400` naming
+it, and at startup the daemon logs an error and comes up without that job. The
+alternative would be to drop the gate and run the job anyway, which turns "only
+when this holds" into "every time" — a precondition removed by accident is not a
+precondition.
 
 ### Gate types
 
@@ -230,16 +300,23 @@ run_if:
 A gate must implement the same three methods as a built-in (`is_satisfied`,
 `describe`, `from_config`).
 
-**Rules** (all fail-safe — a bad plugin never crashes the daemon):
+**Rules** (a bad plugin never crashes the daemon — though it does cost the jobs
+that name its gate, see below):
 
 - Files whose name starts with `_` (and `__pycache__`) are ignored.
 - A plugin whose `type` collides with an already-registered gate is skipped
   with a warning: a **built-in always wins**, and among two plugins the **first
   loaded (filename-sorted) wins**.
 - Any import error in a plugin file is logged (naming the file) and that file
   is skipped; the rest still load.
-- **No hot-reload:** adding or changing a plugin requires a daemon restart —
-  the same as every other piece of cron config.
+- **Hot-reloadable:** `POST /api/cron/reload` re-reads the directory from
+  scratch, so an added, edited, deleted or renamed plugin takes effect without a
+  daemon restart. Built-in gates are never dropped.
+- A plugin that stops registering its gate — deleted, renamed, or newly failing
+  to import — takes the jobs that name that gate with it: the reload is refused
+  with a `400`, and the running schedule is left as it was. Restore the file (or
+  remove the `run_if` entry) and reload again. Deleting a plugin is not a way to
+  switch a gate off; it stops those jobs instead of ungating them.
 
 > **Context is DB-only.** A gate receives `GateContext{job_id, db}`, which is
 > enough for DB-driven conditions (task counts, source cursors, age filters). A

nerve/cli.py

@@ -955,7 +955,7 @@ def doctor_report(config, config_source: str = "", check_api: bool = False) -> s
 
     # Check cron files (merge like the scheduler does: user jobs override system by ID)
     try:
-        from nerve.cron.jobs import load_jobs
+        from nerve.cron.jobs import describe_reserved_job_ids, is_reserved_job_id, load_jobs
 
         system_jobs = load_jobs(config.cron.system_file) if config.cron.system_file.exists() else []
         user_jobs = load_jobs(config.cron.jobs_file) if config.cron.jobs_file.exists() else []
@@ -966,6 +966,18 @@ def doctor_report(config, config_source: str = "", check_api: bool = False) -> s
         for j in user_jobs:
             merged[j.id] = j
 
+        # Jobs on a reserved id are dropped by the scheduler, so counting them
+        # would report crons that can never fire. Call them out instead.
+        reserved_ids = sorted(jid for jid in merged if is_reserved_job_id(jid))
+        for jid in reserved_ids:
+            del merged[jid]
+        if reserved_ids:
+            warnings.append(
+                f"[WARN] Cron job(s) {', '.join(reserved_ids)} use ids reserved "
+                f"by the daemon ({describe_reserved_job_ids()}) and are never "
+                f"scheduled — rename them"
+            )
+
         all_jobs = list(merged.values())
         enabled = sum(1 for j in all_jobs if j.enabled)
 
@@ -1314,7 +1326,7 @@ async def _run():
                 await cron_svc.run_job(job_id)
             else:
                 click.echo("Available jobs:")
-                from nerve.cron.jobs import load_jobs
+                from nerve.cron.jobs import is_reserved_job_id, load_jobs
 
                 # Load from both files, show provenance
                 system_jobs = load_jobs(config.cron.system_file)
@@ -1331,7 +1343,13 @@ async def _run():
                         all_jobs.append(("system", j))
 
                 for source, job in all_jobs:
-                    status = "enabled" if job.enabled else "disabled"
+                    # A reserved id is never scheduled by the daemon. Show the
+                    # job rather than hiding it — the reason it isn't running is
+                    # the whole thing the reader is here to find out.
+                    if is_reserved_job_id(job.id):
+                        status = "RESERVED ID — never scheduled, rename it"
+                    else:
+                        status = "enabled" if job.enabled else "disabled"
                     click.echo(
                         f"  [{source:6s}] {job.id}: "
                         f"{job.description or job.schedule} ({status})"

nerve/cron/gate_plugins.py

@@ -45,8 +45,17 @@ def from_config(cls, spec: dict) -> "StaleTasksGate":
 * A gate gets only ``GateContext{job_id, db}`` (DB-only). A liveness/registry
   based gate is out of scope for this loader — it would need the context
   widened, a separate change.
-* No hot-reload: adding or changing a plugin requires a daemon restart, the
-  same as every other piece of cron config.
+* Hot-reload: ``load_gate_plugins(dir, replace=True)`` (used by
+  :meth:`nerve.cron.service.CronService.reload`) re-reads the directory from
+  scratch — a *new* file is registered, an *edited* file's new code replaces the
+  old class, and a *deleted* file's gate is unregistered. Reload then rebuilds
+  every job's gates from the refreshed registry and swaps the rebuilt job into
+  the scheduler, so the change reaches the running job and not just the
+  registry. Without ``replace`` the first-registered class wins (used on fresh
+  startup, and by validation so a candidate bundle can't swap the live
+  registry). A reload that is then refused rolls the registry back, so the same
+  rule holds for it: only a change that is actually applied reaches the live
+  registry.
 
 **Trust model.** Files in the gate-plugins directory are imported (i.e.
 executed) at daemon startup. This is the same trust model as ``config.yaml``,
@@ -65,13 +74,36 @@ def from_config(cls, spec: dict) -> "StaleTasksGate":
 
 logger = logging.getLogger(__name__)
 
+#: Module-name prefix stamped on every plugin-loaded module (see
+#: :func:`_import_module`). It's how we tell a plugin-registered gate class apart
+#: from a built-in (whose ``__module__`` is ``nerve.cron.gates``) — used by
+#: ``replace=True`` to drop only plugin-owned entries on reload.
+_PLUGIN_MODULE_PREFIX = "nerve_cron_gate_plugin_"
 
-def load_gate_plugins(plugins_dir: Path) -> int:
+
+def load_gate_plugins(
+    plugins_dir: Path, *, replace: bool = False, warn_vanished: bool = True,
+) -> int:
     """Discover and register :class:`CronGate` subclasses from *plugins_dir*.
 
     Returns the number of gate classes newly registered into
     :data:`GATE_REGISTRY`. A missing directory is a no-op (returns ``0``).
 
+    With ``replace=True`` the directory is re-read from scratch: every
+    plugin-owned entry is dropped from the registry first, so an *edited*
+    plugin's new code takes effect, a *deleted* plugin's gate is unregistered,
+    and a *renamed* type is moved — the registry ends up reflecting exactly the
+    files on disk. This is the hot-reload path (:meth:`CronService.reload`).
+    Built-ins are never dropped. The default (``replace=False``) is
+    first-registered-wins and is used on fresh startup and by config validation
+    (so validating a candidate bundle can't mutate the live registry).
+
+    ``warn_vanished=False`` suppresses the gate-disappeared warning. A caller
+    that can still abandon the load — :meth:`CronService.reload` rolls the
+    registry back if the rest of the reload is refused — passes ``False`` here
+    and calls :func:`warn_vanished_gates` once the change is committed, so the
+    warning describes what happened rather than what was attempted.
+
     Never raises: a broken plugin file is logged and skipped so it can't take
     down daemon startup (mirrors :func:`nerve.cron.gates.build_gates`' existing
     tolerance of a bad spec).
@@ -82,8 +114,15 @@ def load_gate_plugins(plugins_dir: Path) -> int:
         logger.warning("Invalid cron gate plugins dir %r: %s", plugins_dir, e)
         return 0
 
+    # Drop previously plugin-loaded gates before rescanning so edits/deletes are
+    # reflected. Do this even if the dir has since disappeared (deleted plugins
+    # must still unregister). Built-ins stay put.
+    removed_types = _unregister_plugin_gates() if replace else set()
+
     if not plugins_dir.is_dir():
         # Missing dir is the normal case — most installs have no custom gates.
+        if warn_vanished:
+            _warn_vanished(removed_types)
         return 0
 
     registered = 0
@@ -97,9 +136,60 @@ def load_gate_plugins(plugins_dir: Path) -> int:
         logger.info(
             "Loaded %d custom cron gate(s) from %s", registered, plugins_dir,
         )
+    if replace and warn_vanished:
+        _warn_vanished(removed_types)
     return registered
 
 
+def warn_vanished_gates(before: dict[str, type[CronGate]]) -> None:
+    """Warn about gate types in *before* that are no longer registered.
+
+    The deferred half of ``load_gate_plugins(..., warn_vanished=False)``: pass
+    the registry snapshot taken before the load and call this once the reload
+    that prompted it has committed. Built-ins are never dropped, so including
+    them in the snapshot cannot produce a spurious warning.
+    """
+    _warn_vanished(set(before))
+
+
+def _unregister_plugin_gates() -> set[str]:
+    """Remove every plugin-loaded gate from :data:`GATE_REGISTRY`; keep built-ins.
+
+    Returns the set of gate types removed, so the caller can warn about any that
+    fail to come back (a plugin that was deleted or no longer imports cleanly).
+    """
+    removed: set[str] = set()
+    for gate_type, cls in list(GATE_REGISTRY.items()):
+        if getattr(cls, "__module__", "").startswith(_PLUGIN_MODULE_PREFIX):
+            del GATE_REGISTRY[gate_type]
+            removed.add(gate_type)
+    return removed
+
+
+def _warn_vanished(removed_types: set[str]) -> None:
+    """Warn about gate types that were unregistered and did not re-register.
+
+    A gate that disappears on reload means its plugin file was deleted or now
+    fails to import. A job whose ``run_if`` names it no longer builds at all —
+    ``build_gates`` raises on a type nothing registered — which refuses that
+    reload before it can commit, so such a reload never reaches this warning. Its
+    400 is the signal instead, and it names the job.
+
+    What reaches here is the other case: the plugin went away and no job
+    referenced it, so the reload committed. That is harmless right now, and still
+    worth a line, because the next job to name that type will be refused and the
+    cause will be this pull rather than that edit.
+    """
+    for gate_type in sorted(removed_types - set(GATE_REGISTRY)):
+        logger.warning(
+            "Cron gate %r is no longer registered after reload — its plugin file "
+            "was removed or failed to re-import. No job uses it, or this reload "
+            "would have been refused; one that names it will be refused until "
+            "the plugin is back",
+            gate_type,
+        )
+
+
 def _load_file(path: Path) -> int:
     """Import one plugin file and register its gate classes. Returns the count."""
     module = _import_module(path)
@@ -146,7 +236,11 @@ def _import_module(path: Path):
     throwaway namespace whose only purpose is to surface the gate classes it
     defines, so it never pollutes the global module table.
     """
-    mod_name = f"nerve_cron_gate_plugin_{path.stem}"
+    # Derived from the shared constant, never spelled out again: the prefix is
+    # the only link between a module loaded here and the registry entries
+    # ``_unregister_plugin_gates`` is allowed to drop, so two copies drifting
+    # apart would silently turn hot-reload into "plugin gates never unregister".
+    mod_name = f"{_PLUGIN_MODULE_PREFIX}{path.stem}"
     try:
         spec = importlib.util.spec_from_file_location(mod_name, path)
         if spec is None or spec.loader is None:

nerve/cron/gates.py

@@ -461,17 +461,21 @@ def build_gate(spec: dict) -> CronGate:
 def build_gates(specs: list[dict]) -> list[CronGate]:
     """Build gates from a list of config specs.
 
-    Invalid specs are logged and skipped rather than raising, so one bad
-    gate can't take down the whole cron service at load time. A job whose
-    gates all fail to build behaves as if it has no gates (runs normally).
+    Raises :class:`GateConfigError` if any spec cannot be built — an unknown
+    ``type`` above all, which is what a deleted or unimportable gate plugin looks
+    like from here. This runs from ``CronJob.__post_init__``, so it takes the job
+    with it: skipped with an error at startup, and a ``400`` that refuses the
+    whole change set at reload.
+
+    Skipping the spec instead would be worse than losing the job. A gate is a
+    *precondition*, so dropping one makes the job run more often, not less: a
+    job that asked to run "only when the inbox is busy" starts running every
+    time, and the config that said otherwise is still sitting there. With
+    hot-reload that arrives unattended, from a workspace pull that removed a
+    plugin file. Refusing is the visible failure of the two — the reload names
+    the job in its 400, startup logs it, and no job silently changes meaning.
     """
-    gates: list[CronGate] = []
-    for spec in specs or []:
-        try:
-            gates.append(build_gate(spec))
-        except GateConfigError as e:
-            logger.warning("Ignoring invalid cron gate %s: %s", spec, e)
-    return gates
+    return [build_gate(spec) for spec in specs or []]
 
 
 async def evaluate_gates(