docs(operations): canonical config-reload field table derived from build_reload_plan#5642
Merged
Conversation
…ild_reload_plan Add a single ops-facing reference answering "I edited config.toml and called POST /api/config/reload — does it take effect or do I need a restart?" One row per KernelConfig field, classified RequiresRestart / HotReload / Ignore-noop and grouped by config section, transcribed from build_reload_plan / build_reload_plan_with_caps. - New doc docs/operations/config-reload.md (one row per field; intro on the three-bucket plan, the per-agent-cap-needs-respawn gotcha, and the conditional log_level reloader caveat). - external_auth is documented as R/H to match the planner's actual classification: IdP-identity changes (enabled, issuer_url, per-provider id/issuer_url/jwks_uri) hot-reload via ReloadExternalAuth (cache eviction, no restart), while non-IdP sub-fields (session_ttl_secs, allowed_domains, redirect_url, scopes, audience, require_email_verified) keep the conservative restart since no hot reapply path is wired. - Drift guard doc_reload_table_matches_classified_reload_fields parses the doc's field column and asserts set-equality with classified_reload_fields() in both directions; with every_config_field_is_reload_classified a new KernelConfig field now fails the build unless wired into the planner AND listed in the doc. - Cross-reference pointers: CLAUDE.md, docs/architecture/ message-history-trimming.md, docs/architecture/ trigger-dispatch-concurrency.md. This carries only the additive docs + drift test. The competing external_auth code change is dropped; main's post-restore classification (restart gated on a non-IdP change) is the source of truth. Verification: - cargo test -p librefang-kernel --lib config_reload — 42 passed, 0 failed (incl. doc_reload_table_... and every_config_field_...). - cargo check --workspace --lib --exclude librefang-desktop — clean. - cargo clippy -p librefang-kernel --lib -- -D warnings — clean. - cargo fmt -p librefang-kernel -- --check — clean. Closes #5641
houko
force-pushed
the
docs/config-reload-ops-table
branch
from
May 23, 2026 03:24
66fd257 to
86e65ac
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds a single canonical, ops-facing reference for config hot-reload behaviour, fixing the "semantics scattered across config_reload.rs / CLAUDE.md / two arch docs" problem.
New doc —
docs/operations/config-reload.mdKernelConfigfield with its reload classification (RequiresRestart / HotReload / Ignore-noop) and a one-line meaning, grouped by config section.POST /api/config/reloadsemantics (the three-bucket plan frombuild_reload_plan), the per-agent-cap-needs-respawn gotcha (from CLAUDE.md), and the conditionallog_levelreloader caveat.How the table was derived
Every row is transcribed from
build_reload_plan/build_reload_plan_with_capsincrates/librefang-kernel/src/config_reload.rs— the hand-tuned branches at the top plus the #5619 backfilled RESTART/NOOP blocks. The field set is the literal mirror ofclassified_reload_fields(). One-line meanings are taken from theKernelConfigfield doc-comments inlibrefang-types/src/config/types.rs, not invented.external_authrow reflects main's post-#5652 classificationexternal_authis documented as R/H, matching whatbuild_reload_planactually does after #5652 restored main:enabled,issuer_url, per-providerid/issuer_url/jwks_uri— seeexternal_auth_idp_changed) are HotReload: they emitReloadExternalAuthto flush the OIDC discovery + JWKS caches, no restart.session_ttl_secs,allowed_domains,redirect_url, scopes, audience,require_email_verified) are RequiresRestart: no hot reapply path is wired beyond the bare ArcSwap swap, so the planner conservatively flags a restart.Cross-references (minimal pointer lines only)
CLAUDE.md— new bullet in the config/hot-reload area pointing at the canonical doc.docs/architecture/message-history-trimming.md— note thatmax_history_messagesis read live (no restart) + link.docs/architecture/trigger-dispatch-concurrency.md— new "Reload behaviour" note (queue caps hot-reload; per-agent cap needs respawn) + link.Drift guard
doc_reload_table_matches_classified_reload_fields(new test inconfig_reload.rs) parses the doc's field column and asserts it equalsclassified_reload_fields()in both directions. Combined with the existingevery_config_field_is_reload_classifiedtest, a newKernelConfigfield now fails the build unless it is wired into the planner AND listed in the doc.Scope note: dedup vs #5646 / #5652
An earlier revision of this PR also removed the
external_authrestart branch inbuild_reload_plan. That code change is dropped here. The behaviour question — should non-IdPexternal_authedits require a restart? — was settled by #5652 (merged), which keeps the conservative restart for non-IdP fields while hot-reloading IdP-identity changes. #5646, which proposed the broader "no restart for anyexternal_authfield" behaviour, was closed as superseded. This PR now carries only additive docs + the drift test, and the doc/test reflect main's actual classification.Verification
cargo test -p librefang-kernel --lib config_reload— 42 passed, 0 failed (incl. the newdoc_reload_table_matches_classified_reload_fieldsand the existingevery_config_field_is_reload_classified; allexternal_auth_*tests green against main's classification).cargo check --workspace --lib --exclude librefang-desktop— clean.cargo clippy -p librefang-kernel --lib -- -D warnings— clean.cargo fmt -p librefang-kernel -- --check— clean.Closes #5641