#77: /admin/reload-config — hot-reload env-driven globals#94
Open
BrettKinny wants to merge 2 commits into
Open
#77: /admin/reload-config — hot-reload env-driven globals#94BrettKinny wants to merge 2 commits into
BrettKinny wants to merge 2 commits into
Conversation
…bearing The face-detected bare "Hi!" path previously suppressed only when at least one roster member had an `appearance:` field. Households where members were registered with just display_name + calendar prefix (the common configuration) still saw a bare greet stack on top of the room-view named greet, producing a double-greeting on each walk-in. Suppress when the registry has any members — accept silent-listen-mode for unidentified faces as the design (`Closes #23`, approach b). - HouseholdRegistry: add `__len__` (with the standard `_reload_if_changed` guard). - FaceGreeter: `_roster_has_appearances()` → `_roster_is_populated()`, checking `bool(self._household)`. - Tests: new non-empty case (display_name only fixture); existing appearance-bearing case preserved. Closes #23 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Iterating on .env config required a full bridge restart. Add a maintenance POST /admin/reload-config that re-reads a whitelist of env vars into the module globals in place, returning a diff of what changed. Scope is deliberately tight: vars whose only consumer is the request path (VISION_*, VLM_*, WEATHER_*, CALENDAR_TTL_SEC). Vars tied to background loops (CALENDAR_POLL_SEC, CALENDAR_IDS), file-watcher paths, LOCAL_TZ, or the voice-provider selector are excluded — those need a real restart. The route lives on the existing _admin_router (localhost-only via _admin_require_localhost), so no CSRF, no extra auth. Secrets are masked in the response: first4…last4, or *** for short values. Tests bypass the localhost guard via dependency_overrides and toggle bridge.csrf._ENFORCE (the documented kill-switch) since TestClient requests go through CSRFMiddleware. Closes #77 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a scoped admin endpoint to hot-reload a whitelist of env-driven config globals in bridge.py, and updates the behaviour-layer face greeter to suppress bare greetings whenever the household registry is non-empty (with accompanying tests).
Changes:
- Add
POST /admin/reload-configto reload selectedWEATHER_*,VISION_*,VLM_*, andCALENDAR_TTL_SECglobals and return a structured “reloaded vs unchanged” response with secret masking. - Change
FaceGreetersuppression logic from “roster has appearances” to “roster is non-empty”, enabled by addingHouseholdRegistry.__len__. - Add tests covering the new reload endpoint and the updated face greeter suppression behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
bridge.py |
Implements /admin/reload-config and secret-masking helpers on the localhost-only admin router. |
tests/test_bridge_routes.py |
Adds boundary tests for /admin/reload-config, including masking and live-global update assertions. |
dotty-behaviour/consumers/face_greeter.py |
Updates face-detected suppression criteria to trigger whenever the household registry is populated. |
dotty-behaviour/household/registry.py |
Adds __len__ to support efficient truthiness checks with hot-reload semantics. |
dotty-behaviour/tests/test_consumer_face_greeter.py |
Adds a test ensuring bare greeting is suppressed when the roster is non-empty (even without appearances). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+5637
to
+5648
| old = g.get(var_name) | ||
| raw = os.environ.get(env_name, "") | ||
| try: | ||
| new = caster(raw) if raw != "" else ( | ||
| "" if caster is str else caster(0) | ||
| ) | ||
| except (TypeError, ValueError): | ||
| log.warning( | ||
| "reload-config: cast failed for %s=%r as %s", | ||
| env_name, raw, caster.__name__, | ||
| ) | ||
| continue |
Comment on lines
+5628
to
+5632
| @_admin_router.post("/reload-config") | ||
| async def _admin_reload_config() -> dict: | ||
| """Re-read whitelisted env vars into module globals. Returns | ||
| {ok, reloaded:[{name, old, new}], unchanged:[name]}. Secrets are | ||
| masked in the response.""" |
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.
Summary
POST /admin/reload-configthat re-reads a whitelisted set of env vars into the matching module globals — no bridge restart needed.{ok, reloaded:[{name, old, new}], unchanged:[name]}. Secrets are masked asfirst4…last4(API_KEY,_KEY,TOKENin the var name)._admin_router(localhost-only via_admin_require_localhost) — no CSRF or extra auth needed.Why
The issue body called for a scoped reload endpoint and a design call on which vars are reload-safe. The whitelist above is the conservative cut: pure data with no background-task or watcher coupling.
Test plan
.venv/bin/pytest tests/test_bridge_routes.py -q— 38 passed (35 prior + 3 new).venv/bin/pytest tests/ -q— 262 passeddependency_overridesto bypass the localhost guard and togglebridge.csrf._ENFORCE(the documented kill-switch) because TestClient requests go through CSRFMiddleware.Closes #77
🤖 Generated with Claude Code