Skip to content

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

Open
alex-clickhouse wants to merge 5 commits into
config-refactor/11-self-modifyfrom
config-refactor/12-reload-all
Open

Broaden hot-reload: unified reload_all + POST /api/config/reload (12/13)#215
alex-clickhouse wants to merge 5 commits into
config-refactor/11-self-modifyfrom
config-refactor/12-reload-all

Conversation

@alex-clickhouse

@alex-clickhouse alex-clickhouse commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

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.

🤖 Generated with Claude Code

@alex-clickhouse alex-clickhouse changed the title Broaden hot-reload: unified reload_all + POST /api/config/reload Broaden hot-reload: unified reload_all + POST /api/config/reload (12/13) Jul 28, 2026
@alex-clickhouse
alex-clickhouse requested a review from Copilot July 28, 2026 10:00

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread nerve/agent/backends/codex/backend.py
@alex-clickhouse
alex-clickhouse force-pushed the config-refactor/12-reload-all branch from e3d20b9 to dabe2d1 Compare July 28, 2026 12:11
@alex-clickhouse
alex-clickhouse force-pushed the config-refactor/12-reload-all branch from dabe2d1 to 631a9e9 Compare July 28, 2026 12:36
@alex-clickhouse
alex-clickhouse force-pushed the config-refactor/12-reload-all branch from 631a9e9 to e6d42c8 Compare July 28, 2026 13:15
@alex-clickhouse
alex-clickhouse force-pushed the config-refactor/12-reload-all branch from e6d42c8 to f5bd49a Compare July 28, 2026 14:23
@alex-clickhouse
alex-clickhouse force-pushed the config-refactor/12-reload-all branch from f5bd49a to eec1ffc Compare July 28, 2026 14:54
@alex-clickhouse
alex-clickhouse force-pushed the config-refactor/12-reload-all branch from eec1ffc to 87af896 Compare July 28, 2026 15:34
@alex-clickhouse
alex-clickhouse force-pushed the config-refactor/12-reload-all branch from 87af896 to 0b521ba Compare July 28, 2026 16:03
@alex-clickhouse
alex-clickhouse force-pushed the config-refactor/12-reload-all branch 2 times, most recently from 705ebc9 to bc92c18 Compare July 28, 2026 18:40
@alex-clickhouse
alex-clickhouse force-pushed the config-refactor/12-reload-all branch from bc92c18 to 2d76e6a Compare July 29, 2026 08:29
@alex-clickhouse
alex-clickhouse force-pushed the config-refactor/12-reload-all branch 2 times, most recently from 0ca29fd to e4ed320 Compare July 29, 2026 09:31
@alex-clickhouse
alex-clickhouse force-pushed the config-refactor/12-reload-all branch from e4ed320 to df056c0 Compare July 29, 2026 10:31
@alex-clickhouse
alex-clickhouse force-pushed the config-refactor/12-reload-all branch from df056c0 to 92a1428 Compare July 29, 2026 13:28
@alex-clickhouse
alex-clickhouse force-pushed the config-refactor/12-reload-all branch from 92a1428 to b9c46f9 Compare July 30, 2026 08:01
@alex-clickhouse
alex-clickhouse force-pushed the config-refactor/12-reload-all branch 2 times, most recently from 3aed829 to 077ee42 Compare July 30, 2026 08:55
@alex-clickhouse
alex-clickhouse force-pushed the config-refactor/12-reload-all branch from 077ee42 to 348b35b Compare July 30, 2026 09:04
@alex-clickhouse
alex-clickhouse force-pushed the config-refactor/12-reload-all branch 2 times, most recently from 89cec21 to f15ffa5 Compare July 30, 2026 10:06
One reload path covers the config object, the services holding their own
reference to it, cron jobs, cron sources, MCP servers and skills, behind a single
route. Sync applies what it merged through that same path rather than a parallel
one, so there is a single definition of what "applied" means.

A reload reports what actually happened: ok is derived from what the subsystems
returned, and errors names the ones that failed, instead of reporting a success it
did not earn. It stays best-effort by design — an invalid settings.yaml must not
stop a valid cron edit from being applied — but it says so rather than implying
everything landed. Sources are no longer silently dropped, closures no longer
capture a stale config object, the engine/backend seam is closed, and the
hot-versus-restart-only tables in the docs are completed and corrected against
what the code does.

Co-Authored-By: Claude <noreply@anthropic.com>
@alex-clickhouse
alex-clickhouse force-pushed the config-refactor/12-reload-all branch from f15ffa5 to ed551d2 Compare July 30, 2026 10:47
alex-clickhouse and others added 4 commits July 30, 2026 14:20
`nerve codex doctor` raised `TypeError: 'NerveConfig' object is not callable`
and did nothing else. Same in scripts/codex_smoke.py.

Making `config` a callable on BackendDeps changed what every construction site
has to hand over. The engine was updated; these two were not. The doctor is the
only shipped caller that assembles deps itself, and no test covered it, so the
break shipped as a broken command rather than a failure.

The other repair Copilot offered — have the backend accept either shape — is the
one that costs something. A backend that tolerates a captured config accepts the
stale-config bug this commit exists to remove, and accepts it silently, since the
tolerant call site reads exactly like the correct one.

The test builds the real backend and stubs only preflight, the step that needs the
codex binary. Asserting CODEX_HOME appears under tmp_path is what pins config
resolution: the directory is created during construction from the config the CLI
loaded, so its absence would mean the deps callable returned something else.

Co-Authored-By: Claude <noreply@anthropic.com>
The rule is already stated twice above it — the section opens with a config file
changing on disk not being a reload, and the trigger list with a reload always
being explicit. What the paragraph added was the case for that rule, at length,
in a section documenting what reloads.

Co-Authored-By: Claude <noreply@anthropic.com>
`nerve reload` refused to run without `auth.jwt_secret`, but that is not what the
endpoint requires. `require_auth` runs open when the secret is empty and the
instance is unlocked — dev mode — so the command was declining to make a call the
gateway would have accepted, and telling the operator to POST it by hand. The
whole reason this command exists is that an operator applying a hand edit should
not have to do that, and a box with no auth configured is where hand edits are the
normal way to change anything.

Now the token is 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 —
operations with more reach than a reload.

Lockdown is the one case where an empty secret is a dead end, and it stays a
refusal: a locked gateway never takes the open path, so nothing that shell sends
can be authenticated. The message says that, rather than the old advice to run
`nerve init` and POST it manually, neither of which helps there. It also covers
the case where the daemon does have a secret from an environment variable this
shell is missing, since from here the two are indistinguishable and the fix is the
same.

`auth.password_hash` is not a second source of a token. It gates the browser
login, which is what mints one from it; `require_auth` reads `auth.jwt_secret`
alone. So a password does not make this endpoint ask for a token, and there is a
test saying so, because "auth is configured" and "the endpoint is authenticated"
are not the same statement here.

The 401 message now covers both directions of a secret mismatch, since with the
token omitted a rejection means the daemon has one this config does not supply.

Co-Authored-By: Claude <noreply@anthropic.com>
It was after the restart table, two screens below the list of what reloads, which
made "the first table" a back-reference the reader has to resolve. Directly under
that table it is "all of that", and the section reads as what reloads followed by
what the call reports.

Co-Authored-By: Claude <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@alex-clickhouse
alex-clickhouse marked this pull request as ready for review July 30, 2026 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants