Reload only the config sections a reload file declares - #663
Merged
Merged
Conversation
coopernetes
enabled auto-merge
September 14, 2026 16:09
ConfigHotReloadE2ETest flaked in CI, and both failure directions traced to the same root: a reload always applied every section of the composed config. A reload file is composed over the base config, so a section the file omits came back populated with base defaults — an override that set only `secret-scan:` silently reset `rules:` to the base (deny) on the next Section.ALL reload the file-watch fired. Depending on which reload won a shared guard, a push was then rejected for the wrong reason (unauthorized, not the check under test) or a relaxed rule never took hold. A reload now applies only the sections the source document actually declares; a section it is silent about keeps its current live value instead of reverting. Section keys are read from the file's top-level YAML keys. The guard is also fixed: it was an AtomicBoolean that let an explicit reload be silently dropped when a background reload was in flight while reload() still reported success. It is now a ReentrantLock — an explicit reload blocks and always applies, returning the true outcome; background polls tryLock and skip, since the next poll catches up. With both fixed the tests are deterministic, so the retry loops that papered over the drop are gone. Documents partial reload semantics in docs/configuration/hot-reload.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
coopernetes
force-pushed
the
fix/hot-reload-contention
branch
from
September 14, 2026 16:24
711d65f to
19406a2
Compare
coopernetes
changed the title
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
1 participant
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.
ConfigHotReloadE2ETestflaked in CI in both directions, and both traced to one root cause: a reload always applied every section of the composed config. A reload file is composed over the base config, so a section the file omits comes back populated with base defaults — an override that sets onlysecret-scan:silently resetrules:to the base (deny) on the nextSection.ALLreload the file-watch fired. Depending on which reload won a shared guard, a push was then rejected for the wrong reason ("not allowed to push" instead of the check under test), or a relaxed rule never took hold.Fix
A reload now applies only the sections the source document actually declares. A section it is silent about keeps its current live value instead of reverting to a default — a partial reload file patches the sections it names and leaves the rest alone. Declared sections are read from the file's top-level YAML keys.
The reload guard is also fixed. It was an
AtomicBooleanthat let an explicit reload (POST /api/config/reload, and the e2e harness) be silently dropped when a background reload was in flight — whilereload()still returned"Reloaded ...". It's now aReentrantLock: an explicit reload blocks and always applies, returning the true outcome; background file-watch/git-polltryLockand skip when busy, since the next poll catches up.With both fixed the tests are deterministic, so the retry loops (
awaitReload/awaitPush) that papered over the drop are gone.Docs
docs/configuration/hot-reload.mdnow documents partial reload semantics and the "must still be structurally valid on its own" requirement (a provider referenced by the file must exist and be enabled in the base config).Verified
ConfigHotReloadE2ETestrun locally against Testcontainers Gitea — 5/5 green, repeated.🤖 Generated with Claude Code