Broaden hot-reload: unified reload_all + POST /api/config/reload (12/13)

[alex-clickhouse]

What

One reload path — reload_all and POST /api/config/reload — covering the process config object, cron jobs, cron sources, MCP servers, skills and gates, with a per-subsystem summary so a partial failure is reported rather than averaged into a success.

Long-lived services that captured the config object at construction are re-pointed explicitly. Config that a reload cannot apply is compared against the previous values and reported as restart_required.

Plus nerve reload, a CLI front end for that endpoint: it calls the local gateway, authenticating with auth.jwt_secret from the config it just read, prints the per-subsystem result, and exits non-zero on a partial reload so it can gate a deploy step rather than only informing a human.

Why

Before this there were several partial reload paths and no way for a caller to tell which subsystems had taken a change. A reload that reports success while half the process runs the old config is worse than one that fails, because nothing indicates which half is which.

Re-pointing the services is the same problem one level down. set_config replaces the module-level object, but a service holding its own reference keeps the old one, so the reload reads as complete while those services stay stale indefinitely.

restart_required closes the last gap. Some settings genuinely cannot be applied in place — the gateway socket is already bound, the Telegram bot is already polling — and previously the summary listed only what was applied, so a change to one of those was indistinguishable from no change at all. That became load-bearing once gateway.host/port moved into the tracked layer, since the change can now arrive by workspace sync rather than a local edit. It is a separate field from errors and does not affect ok: nothing failed, and the reload did everything it can.

nerve reload exists because an operator editing a file on the box should not have to mint a JWT and curl an endpoint to apply it. Now that no config file applies itself when it changes on disk — the cron-directory watcher was dropped in #209 — asking for a reload is the documented action, so it needs to be one word. It reports the same thing the endpoint does, including the restart-required warning, and refuses clearly when there is no daemon to talk to (a stopped daemon reads config fresh at start-up, so there is nothing to reload).

reload_all no longer takes a reload_cron parameter. It existed so a caller who knew the file watcher would reload cron could skip it; with the watcher gone, a caller has no way to ask for cron jobs and cron sources separately, so a jobs edit and a sources edit can never be applied apart.

The docs table records what follows a reload, what waits for the next cron reload, and what still needs a restart.

Added in review

nerve codex doctor was broken by this PR. Making BackendDeps.config a callable changed what every construction site has to hand over. The engine was updated; the two callers that assemble deps by hand were not, so the doctor raised TypeError: 'NerveConfig' object is not callable before doing anything at all, and scripts/codex_smoke.py the same. Fixed by passing a callable rather than by having the backend accept either shape: a backend that tolerates a captured config accepts the stale-config bug this PR exists to remove, and accepts it silently, since the tolerant call site reads exactly like the correct one. The doctor now has a test, which it did not before — that is why this shipped as a broken command instead of a failure.

nerve reload no longer requires auth.jwt_secret. require_auth runs open when the secret is empty and the instance is unlocked, so the command was declining to make a call the gateway would have accepted, and telling the operator to POST it by hand — the thing it exists to avoid, on the boxes where hand edits are the normal way to change anything. The token is now sent when there is a secret to sign one with and omitted when there is not, which is what _gateway_request already does for start/kill. Lockdown with no secret stays a refusal, because a locked gateway never takes the open path, and the message says so instead of advising nerve init. auth.password_hash is not a second source of a token — it gates the browser login, and require_auth reads auth.jwt_secret alone — with a test saying so, because "auth is configured" and "the endpoint is authenticated" are not the same statement here.

Two docs edits in the same section: the response-shape paragraph moved under the table it describes, and the argument for reloads being explicit is gone, since the rule is stated twice above where it sat.

Three more, from a review of the stack's end state.

Git HEAD was being used as application state. sync_workspace returned
early when the fetched revision equalled HEAD, and callers reloaded only when a
pull had merged something — so "checked out" and "running" were the same
question. They come apart two ways, both reproduced: a fast-forward from nerve config sync or a bare git pull is never applied by the running daemon (three
cycles, zero reload_all calls), and a partial reload failure is never retried.
Neither is transient. Both docstrings pointed the operator at recoveries that do
nothing — the sync route answers "up to date" and reloads nothing; only nerve reload or a restart worked, and neither said so. The loop now tracks what it
applied and advances that only when every subsystem took the config.

A blocked sync was invisible. Widening the dirty-state check made this matter,
because an agent writing a skill directly is now enough to stop the merge, and the
only evidence was a WARNING repeating every cycle. On a locked box that silently
defeats the deployment. Each cycle now publishes state, entering the blocked state
sends one notification naming the paths and what clears them, nerve doctor
answers from the shell, and GET /api/config/sync serves the record.

The restart-only report did not cover what it claimed. The docs say "the check
covers the unconditional entries below" and the code claims to mirror that table;
six unconditional rows were uncovered. langfuse.enabled could never have fired —
no such attribute, so the entry was silently skipped. telegram.dm_policy and
workflows.review_loop.* are made live rather than listed, which makes the
value correct instead of documenting that it is wrong: a dm_policy tightening
previously reported success and went on admitting strangers until a restart. The
new guard requires every entry to resolve against a default NerveConfig and to
have a row in the docs table, in both directions; it rejected proxy.host on its
own, since four more proxy.* fields are baked into the spawned subprocess. It
also caught that restart_required interpolated both values into a string that is
logged and returned by the endpoint, with telegram.bot_token already listed —
so rotating it published the old and new token. Secrets now report changed.

Source reload was not transactional. It removed every old source job before
parsing a single new trigger, in a loop that logs and continues on a bad one. An
invalid edit destroyed a working */15 schedule, moved another source from thirty
minutes to two hours via _parse_interval's 7200 fallback, reported the missing
runner under sources, and returned ok: true. Now planned first: every trigger
is constructed before any job is removed, and an unreadable one raises. Startup
keeps the lenient path, where skipping is right.

🤖 Generated with Claude Code

[Copilot]

Pull request overview

Adds a unified, best-effort hot-reload pathway (reload_all) and exposes it via POST /api/config/reload, then aligns workspace sync and long-lived services to report/handle partial reload failures per subsystem (instead of collapsing outcomes into a single success/failure).

Changes:

• Introduces nerve.config_reload.reload_all() + reload_failures() and wires a new POST /api/config/reload route to return per-subsystem reload details/errors.
• Refactors workspace sync application (_apply_sync, periodic loop, sync route) to use the unified reload summary and accurately report “partly applied” vs “not applied”.
• Updates cron/source reload behavior and agent backend config access so reloads re-point long-lived components; adds tests and updates operator docs accordingly.

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/test_ultracode_integration.py	Updates backend deps in tests to pass config via callable.
tests/test_sync_service.py	Extends sync tests to assert unified reload behavior and partial-failure reporting.
tests/test_server_periodic_loops.py	Adds coverage ensuring periodic loops follow config reloads where intended.
tests/test_external_agents_sync_service.py	Adds coverage for external agents service adopting reloaded config and interval updates.
tests/test_engine.py	Updates backend construction in tests to pass config via callable.
tests/test_cron_reload.py	Updates cron reload expectations and adds coverage for rejected (reserved) job IDs.
tests/test_config_reload.py	New comprehensive test suite for unified reload behavior and route semantics.
tests/test_codex_protocol.py	Updates Codex client deps wiring in tests to use callable config.
tests/test_codex_appserver.py	Updates BackendDeps usage in tests to use callable config.
tests/test_cache_policy.py	Updates backend construction in tests to pass config via callable.
nerve/sync_service.py	Refactors sync application to call unified reload and log/report partial failures correctly.
nerve/gateway/server.py	Ensures periodic background loops re-read live config per cycle; wires cron notification service earlier.
nerve/gateway/routes/config.py	Adds POST /api/config/reload and enhances /api/config/sync response to include reload summary/errors.
nerve/external_agents/sync_service.py	Adds update_config() and ensures sweep interval is read each loop cycle.
nerve/cron/service.py	Reports reserved job rejections, adds reload_sources(), and wires source runner notifications on rebuild.
nerve/config_reload.py	New unified reload implementation and summary parsing (reload_failures).
nerve/agent/engine.py	Makes engine config re-pointable via setter; seeds/re-seeds session manager defaults on reload.
nerve/agent/backends/codex/backend.py	Switches to resolving config per read via deps callable (but currently breaks non-callable deps in some callers).
nerve/agent/backends/claude.py	Switches to resolving config per read via deps callable.
nerve/agent/backends/init.py	Updates BackendDeps to make config a callable and documents why.
docs/cron.md	Documents rejected job IDs in cron reload responses.
docs/config.md	Adds Hot-Reload documentation and clarifies reload triggers, scope, and restart-only settings.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 27 out of 27 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

nerve/cli.py:1177

• _gateway_url builds URLs by simple string interpolation, which breaks when gateway.host is an IPv6 literal (e.g. "::1"), because RFC 3986 requires IPv6 hosts to be wrapped in brackets. As-is, "http://::1:8900/x" is not a valid URL and httpx will fail to connect. Handle IPv6 literals by bracketing the host (while still rewriting wildcard binds to 127.0.0.1).

    scheme = "https" if config.gateway.ssl.enabled else "http"
    host = config.gateway.host
    if host in _WILDCARD_BINDS:
        host = "127.0.0.1"
    return f"{scheme}://{host}:{config.gateway.port}{path}"

nerve/sync_service.py:585

• _apply_sync logs an INFO/WARNING about the reload summary, but run_periodic_sync (its main in-process caller) already logs both the apply attempt and detailed WARNINGs for partial application. This results in duplicate log lines for every changed sync, and double-warning spam on partial reloads. Consider making _apply_sync return the summary without logging, and let the caller decide what (and how loudly) to report.

    from nerve.config_reload import reload_all, reload_failures

    summary = await reload_all(engine, cron_service, config_dir)
    if reload_failures(summary):
        logger.warning("Applied synced config, with failures: %s", summary)
    else:
        logger.info("Applied synced config: %s", summary)
    return summary

[Copilot]

Pull request overview

Copilot reviewed 32 out of 32 changed files in this pull request and generated no new comments.

Suppressed comments (2)

nerve/cron/service.py:705

• reload_sources() builds its removed list from self._source_runners, which can include runners that were never scheduled (e.g., when a runner exists but its config section is absent). In that case the response can incorrectly claim a source job was removed even though it was never on the scheduler. Compute old_ids from actually-scheduled jobs (or filter by scheduler.get_job(...) is not None) so removed reflects real scheduler changes.

            old_ids = {r.job_id for r in self._source_runners}
            for jid in old_ids:
                if self.scheduler.get_job(jid) is not None:
                    self.scheduler.remove_job(jid)
            self._source_runners = new_runners

nerve/gateway/routes/config.py:49

• The docstring says restart_required is “a separate field rather than a line in detail”, but the route returns it in both places: detail is the raw reload_all() summary (which may include a restart_required key) and there’s also a top-level restart_required. Either drop it from detail or adjust the docstring/response contract; keeping it in both is redundant and makes the response shape harder for clients to reason about.

        "ok": not failures,
        "detail": summary,
        "errors": failures,
        "restart_required": summary.get("restart_required", ""),
    }