Add REDIS_URL to [program:bot] to un-brick Neon idle gating (#468) - #470
Merged
Conversation
The Redis next-due gate added in #468 was inert in production: the bot process (bot/message_handler.py, run_polling) is the primary target of that fix, but [program:bot] in supervisord.conf never had REDIS_URL set. Only [program:api] and [program:interactive-agent-layer] did. Without REDIS_URL, shared.notify_due's fail-open design kicks in correctly but permanently: no Redis client -> is_due() always returns True -> the polling loop's anchor/followup checks hit Postgres on every tick exactly as before #468, with no visible error (this is fail-open working as designed, just against a config gap rather than a real outage). Fix: add REDIS_URL="redis://localhost:6379" to [program:bot], matching the other two programs, so it points at the same supervisord-managed local Redis instance and shares one due_queue with the API process. Verified every current caller of shared.notify_due (api/routes/internal.py, api/routes/anchors.py -> [program:api]; bot/message_handler.py -> [program:bot]) now has REDIS_URL. Added a regression test (tests/infra/test_supervisord_redis_env.py) that parses supervisord.conf and fails if any notify_due-calling program is missing REDIS_URL, or if bot/api point at different Redis instances -- this would have caught the original gap without needing a running supervisord/Redis instance.
Companion to the REDIS_URL fix: the missing REDIS_URL on [program:bot] had no runtime signal beyond a per-tick WARNING easy to lose in logs, which is exactly why the gating being inert went unnoticed. Two small additions: - log_startup_status(): each process (api's lifespan, bot's main()) logs once at boot whether REDIS_URL is configured -- INFO if so, ERROR with an explicit "gating fails open in this process" message if not. This is the single line that would have made the bug obvious immediately. - _get_client() now logs the "REDIS_URL not configured" case once per process at ERROR (via the same self-check), instead of every gated call logging its own WARNING -- collapses what was per-~30s-tick spam into a single, loud, one-time signal. No behavior change: this is logging-only. Fail-open semantics are untouched (confirmed via the existing shared/notify_due.py test suite).
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
Completes #468. The Redis next-due gate added there was inert in the process that matters most:
bot/message_handler.py'srun_polling()is the primary target of the whole Neon-idle-spin-down fix (the polling loop ticks every ~30s regardless of activity), but[program:bot]insupervisord.confnever hadREDIS_URLset — only[program:api]and[program:interactive-agent-layer]did.Without
REDIS_URL,shared.notify_due's fail-open design does exactly what it's documented to do: no Redis client →is_due()always returnsTrue→ every check runs unconditionally, same as before #468 shipped. This is fail-open working correctly, just against a supervisord config gap rather than a real Redis outage — no error, no crash, just a silently-inert optimization.Fix
REDIS_URL="redis://localhost:6379"to[program:bot]'senvironment=line, matching[program:api]/[program:interactive-agent-layer]— points at the same supervisord-managed local Redis instance so bot and api share onedue_queue.shared.notify_due(api/routes/internal.py,api/routes/anchors.py→[program:api];bot/message_handler.py→[program:bot]) now hasREDIS_URL.agent_pool_managerandmcpdon't callnotify_due— no change needed there. tether-premium no longer referencesnotify_dueat all (per the resolved "meeting" component design — nothing to fix there).tests/infra/test_supervisord_redis_env.py: parsessupervisord.confand fails if anynotify_due-calling program is missingREDIS_URL, or if bot/api point at different Redis instances. Confirmed this test goes red against the pre-fix config (origin/dev) and green after the fix — it would have caught this gap in review without needing a running supervisord/Redis instance.Fail-open confirmation
The
[program:bot]REDIS_URL fix is config-only — no application code touched, soshared/notify_due.py's fail-open behavior (unconfigured Redis / connection errors → treat as due, never silently skip a real notification) is unaffected. Re-ran the fulltests/shared/fail-open suite — all green, confirming notifications still fire correctly if Redis is ever briefly unavailable; this change only makes the happy path actually gate as designed.Follow-up commit: observability (log-once + startup self-check)
The regression test above catches this bug class in CI, but there was no runtime signal that made the missing REDIS_URL obvious — it hid through a whole investigation cycle before being caught by review. Added two small, logging-only changes:
notify_due.log_startup_status(): each process (api's lifespan, bot'smain()) logs once at boot whetherREDIS_URLis configured — INFO if so, ERROR with an explicit "gating fails open in this process" message if not._get_client()now logs the "REDIS_URL not configured" case once per process at ERROR (via the same mechanism), instead of every gated call logging its own WARNING — collapses what was effectively per-~30s-tick log spam into a single loud one-time signal.No behavior change — logging only. Confirmed via 3 new tests (log-once-per-process, startup INFO/ERROR branches) plus the full existing fail-open suite, all still green.
Test plan
tests/infra/test_supervisord_redis_env.py— 3 tests, confirmed red onorigin/dev's supervisord.conf, green after the fixtests/shared/(29 tests total now, incl. fail-open coverage from Gate notification checks on a Redis next-due cache to let Neon suspend #468 + 3 new observability tests) — all passingtests/api/+tests/bot/+tests/shared/+tests/infra/suite: 434 passed, 11 pre-existing failures unrelated to this change (stale venv missingclaude_agent_sdk+ one pre-existing async-cancellation flake intest_redis_pubsub.py, untouched by this PR)