fix(background_tasks): warn about the legacy broker variables only for Redis URLs

CHANGELOG.md

@@ -16,6 +16,7 @@ All notable changes to this project are documented in this file. The format is b
 
 ### Fixed
 
+- `background_tasks`: `SM_BG_TASKS_BROKER_URL` and `SM_BG_TASKS_RESULT_BACKEND` are reported as deprecated only when they hold a Redis URL (`redis`, `rediss`), the only values `SM_REDIS_URL` can replace. A production host that runs tasks in-process with `memory://` + `cache+memory://`, or uses an AMQP, SQS or `rpc://` transport, no longer logs a warning on every boot that it cannot act on, and the production localhost-broker error no longer calls those vars deprecated for non-Redis values (#868).
 - `SecurityHeadersMiddleware` no longer overwrites a `Content-Security-Policy` that a response sets. The app policy is sent beside it as a second header and browsers enforce both, so a page can tighten the policy (for example `frame-ancestors 'none'` on a public page) but never relax it. Before, only a policy containing `sandbox` survived and any other was dropped silently. **Behaviour change:** a response that set a looser policy, relying on it being replaced, now has both policies enforced: remove that header, or make it a policy the page can live with alongside the app policy (#870).
 - `alembic check` and SQLite autogenerate no longer report the `file_storage` browse index (`sa.desc(sa.column("created_at"))` instead of `sa.text("created_at DESC")`) or `users_access_token.expires_at` (now `AwareDateTime`) as changed, so the host's `alembic check` on SQLite is clean. `make ci-doctor` now runs `alembic check` against its throwaway SQLite database, and a module index written with `sa.text("col DESC")` is no longer reported as changed on SQLite (#869).
 - `background_tasks`: the Redis invalidation listener now opens the connection a publish uses (one `PING` after every successful subscribe), so the first invalidation after boot or after a Redis outage no longer spends part of its 1 s budget connecting. Under load that connect was cancelled and the message was lost; other workers kept the stale value until its TTL (#880).

docs/framework/invalidation.md

@@ -73,7 +73,7 @@ A transport failure never propagates. The caller has already committed; turning
 
 ## Operating it
 
-`background_tasks` installs the transport during `on_startup`, on its own `broker_url` — which resolves as `SM_BG_TASKS_BROKER_URL` (deprecated) → `SM_REDIS_URL` → stored override → the localhost default. So it always follows Celery's broker, by construction rather than by convention.
+`background_tasks` installs the transport during `on_startup`, on its own `broker_url` — which resolves as `SM_BG_TASKS_BROKER_URL` (deprecated for a Redis URL) → `SM_REDIS_URL` → stored override → the localhost default. So it always follows Celery's broker, by construction rather than by convention.
 
 | Setting | Default | What it does |
 |---|---|---|

docs/reference/env-vars.md

@@ -101,6 +101,8 @@ Three fields are still read from the environment, because they have to work *bef
 | `SM_BG_TASKS_RESULT_BACKEND` | `redis://localhost:6379/1` | Same reasoning as the broker. |
 | `SM_BG_TASKS_TASK_ALWAYS_EAGER` | `false` | Read at module-import time, so test suites that never run the FastAPI lifespan can still flip it. Runs tasks synchronously in the calling process. |
 
+For a Redis broker and result backend, set `SM_REDIS_URL` instead: the two per-field variables log a deprecation warning when they hold a Redis URL (`redis://` or `rediss://`). They stay the way to set any other transport, such as `memory://` + `cache+memory://` for a production host that runs tasks in-process with no Redis, or an AMQP broker, and those values log no warning.
+
 ## File storage module
 
 File storage config is now **DB-backed** — edit it in the admin UI at `/admin/settings/` under Files. The key fields and defaults:

docs/superpowers/plans/2026-11-07-legacy-redis-warning.md

@@ -0,0 +1,18 @@
+# Plan: warn about the legacy broker variables only for Redis URLs (#868)
+
+Design: `docs/superpowers/specs/2026-11-07-legacy-redis-warning-design.md`.
+
+1. **Tests first** (`modules/background_tasks/tests/core/settings/test_background_tasks_redis_url.py`):
+   in-process pair under production is quiet; an `amqp://` broker is quiet;
+   `rediss` and an upper-case `REDIS` scheme still warn, `sentinel` does not; the check
+   is per variable. Done when the three quiet/per-variable tests fail on main.
+2. **Fix** (`background_tasks/core/settings_env_checks.py`): `_REDIS_SCHEMES`
+   and a scheme check in `warn_on_legacy_redis_vars`; reword the production
+   localhost error and the `settings.py` docstring to match. Done when the settings
+   tests pass.
+3. **Docs**: a paragraph under the Background tasks table in
+   `docs/reference/env-vars.md`, the deprecation note in
+   `docs/framework/invalidation.md`; CHANGELOG Fixed entry citing #868.
+4. **Verify**: `make lint`, `make test-py`.
+
+All tasks are sequential and small; none is hard.

docs/superpowers/specs/2026-11-07-legacy-redis-warning-design.md

@@ -0,0 +1,76 @@
+# Warn about the legacy broker variables only when SM_REDIS_URL can replace them (#868)
+
+## Goal & scope
+
+`warn_on_legacy_redis_vars` tells every deployment that sets
+`SM_BG_TASKS_BROKER_URL` / `SM_BG_TASKS_RESULT_BACKEND` to use `SM_REDIS_URL`
+instead. `SM_REDIS_URL` seeds *both* fields with one Redis URL, so the advice
+only works when the value is a Redis URL. A production host that runs tasks
+in-process must set `memory://` + `cache+memory://` (the localhost refusal
+blocks the defaults, and no single URL is valid for both), and gets two
+warnings on every boot that it cannot act on.
+
+In scope: warn only for a value `SM_REDIS_URL` could express; tests; docs;
+CHANGELOG.
+Out of scope: changing the production localhost refusal or eager-mode
+defaults.
+
+## Approach
+
+Chosen: a legacy variable is warned about only when its value uses a Redis
+scheme: `redis` or `rediss`. `SM_REDIS_URL` is also read by the rate limiter,
+leases and the invalidation transport through redis-py's `from_url`, so it can
+carry only a scheme both Celery and redis-py accept. Celery's `redis+socket` and
+`sentinel` fail in redis-py (found in review, round 1 pass 3; the approved design
+listed them). Any other scheme (`memory://`, `cache+memory://`, `amqp://`,
+`rpc://`, `sqs://`, `db+…`) stays quiet. Those variables are the only way to express such
+a value, so they are not really deprecated for it.
+
+Rejected:
+- Skip the localhost refusal when `task_always_eager` is on, or default to
+  in-process transports. That is a behaviour change to a production guard, and
+  `broker_url` is read by more than the task runner (the worker, beat, the
+  health check). It doesn't fit a warning fix.
+- Silence only `memory://` and `cache+memory://`. That still nags RabbitMQ or
+  SQS hosts with advice they cannot follow.
+
+## Design
+
+`modules/background_tasks/background_tasks/core/settings_env_checks.py`:
+- add `_REDIS_SCHEMES = frozenset({"redis", "rediss"})`;
+- in `warn_on_legacy_redis_vars`, skip a variable whose value's scheme (the text
+  before `://`, lowercased) is not in that set;
+- extend the docstring with the reason;
+- reword the production localhost error so it calls the variables deprecated
+  only for a Redis URL, and say they stay the way to set any other transport.
+
+Also reworded to match: the `settings.py` module docstring and the
+`SM_BG_TASKS_BROKER_URL (deprecated)` mention in `docs/framework/invalidation.md`.
+
+## Decisions made
+
+- The check is per variable. A host with a Redis broker and an `rpc://` backend
+  still gets the warning for the broker, which it can move to `SM_REDIS_URL`.
+- No new setting and no new log line.
+- Docs: a paragraph in `docs/reference/env-vars.md` § Background tasks saying the
+  two variables stay the way to set non-Redis transports (the in-process pair
+  for a production host with no Redis), and warn only for Redis URLs.
+- CHANGELOG `## [Unreleased]` → Fixed, citing #868.
+
+## Testing
+
+`modules/background_tasks/tests/core/settings/test_background_tasks_redis_url.py`:
+- the in-process pair (`memory://` + `cache+memory://`) under
+  `SM_ENVIRONMENT=production` boots and logs no deprecation warning;
+- an `amqp://` broker logs no warning;
+- `rediss://` and an upper-case `REDIS://` still warn; `sentinel://` and
+  `redis+socket://` do not (parametrized);
+- the existing tests (`redis://` legacy values warn, `SM_REDIS_URL` alone is
+  quiet) stay green.
+
+No browser QA: settings-only change (`--skip-browser`).
+
+## Risks
+
+Low. A deployment using `redis://` keeps the warning. Only hosts that cannot
+follow the advice stop seeing it.

modules/background_tasks/background_tasks/core/settings.py

@@ -18,10 +18,12 @@
 
 ``SM_BG_TASKS_BROKER_URL`` and ``SM_BG_TASKS_RESULT_BACKEND`` still work and
 take precedence over ``SM_REDIS_URL`` — a deployment that set them meant them —
-but log a deprecation warning. Being real env reads, they also beat a stored
-value (env → DB → default, GH #486). They are answered by pydantic's own env source
-now that ``env_prefix`` is set (GH #283), which is also what stops every other
-field being readable from its bare, unprefixed name.
+but log a deprecation warning when they hold a Redis URL (any other transport,
+such as ``memory://``, is theirs to set and logs nothing, GH #868). Being real
+env reads, they also beat a stored value (env → DB → default, GH #486). They
+are answered by pydantic's own env source now that ``env_prefix`` is set
+(GH #283), which is also what stops every other field being readable from its
+bare, unprefixed name.
 
 The other env read is ``SM_ENVIRONMENT``, consulted by the
 ``@model_validator`` to refuse a localhost broker in production — that's a

modules/background_tasks/background_tasks/core/settings_env_checks.py

@@ -15,6 +15,12 @@
 
 logger = logging.getLogger("background_tasks.settings")
 
+# Schemes one SM_REDIS_URL can carry everywhere it is read: Celery's broker and
+# result backend, and redis-py's ``from_url`` behind the rate limiter, leases and
+# the invalidation transport. Celery's ``redis+socket`` and ``sentinel`` fail
+# there, so those values stay on the per-field vars without a warning.
+_REDIS_SCHEMES = frozenset({"redis", "rediss"})
+
 
 def warn_on_legacy_redis_vars(broker_url: str, result_backend: str) -> None:
     """Nudge deployments off the per-field names onto ``SM_REDIS_URL``.
@@ -25,12 +31,19 @@ def warn_on_legacy_redis_vars(broker_url: str, result_backend: str) -> None:
     than in the default factory because ``env_prefix`` means the env source
     answers these names before any default is consulted, so the factory never
     sees them.
+
+    Only a Redis URL is warned about (#868): for anything else, such as the
+    in-process ``memory://`` + ``cache+memory://`` pair or an AMQP broker,
+    ``SM_REDIS_URL`` can't express the value and these vars are the way to
+    set it.
     """
     for value, legacy_var in (
         (broker_url, f"{ENV_PREFIX}BROKER_URL"),
         (result_backend, f"{ENV_PREFIX}RESULT_BACKEND"),
     ):
-        if os.environ.get(legacy_var) == value:
+        if os.environ.get(legacy_var) != value:
+            continue
+        if value.partition("://")[0].lower() in _REDIS_SCHEMES:
             logger.warning(
                 "%s is deprecated; use %s instead (one URL seeds both the "
                 "broker and the result backend).",
@@ -61,8 +74,8 @@ def forbid_localhost_broker_in_production(broker_url: str, result_backend: str)
             f"{names} must not point at localhost when SM_ENVIRONMENT={env!r}. "
             f"Set {ENV_REDIS_URL} on the container to the Redis service host "
             "(e.g. redis://redis:6379/0) — one URL seeds both the broker and "
-            f"the result backend. ({env_names} still override it per field, "
-            "but are deprecated.) These are read before the DB-backed "
-            "settings exist, so an environment variable is what satisfies "
-            "this at boot."
+            f"the result backend. ({env_names} still override it per field; "
+            "they are deprecated for a Redis URL, but stay the way to set any "
+            "other transport.) Environment variables are read before the "
+            "DB-backed settings exist, so one is what satisfies this at boot."
         )

modules/background_tasks/tests/core/settings/test_background_tasks_redis_url.py

@@ -145,3 +145,76 @@ def test_legacy_var_still_beats_redis_url_and_the_db(monkeypatch: pytest.MonkeyP
 
     assert settings.broker_url == "redis://legacy:6379/0"
     assert settings.result_backend == "redis://cache:6379/2"
+
+
+def test_in_process_transports_boot_production_without_a_warning(
+    monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture
+) -> None:
+    """#868: a production host with no Redis runs tasks in-process. No single
+    SM_REDIS_URL can name both halves, so the per-field vars are the only way
+    to say it and must not be called deprecated."""
+    monkeypatch.setenv("SM_ENVIRONMENT", "production")
+    monkeypatch.setenv("SM_BG_TASKS_BROKER_URL", "memory://")
+    monkeypatch.setenv("SM_BG_TASKS_RESULT_BACKEND", "cache+memory://")
+
+    with caplog.at_level(logging.WARNING):
+        settings = BackgroundTasksSettings()
+
+    assert settings.broker_url == "memory://"
+    assert settings.result_backend == "cache+memory://"
+    assert "deprecated" not in caplog.text.lower()
+
+
+def test_non_redis_broker_logs_no_warning(
+    monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture
+) -> None:
+    monkeypatch.setenv("SM_BG_TASKS_BROKER_URL", "amqp://guest@rabbit:5672//")
+
+    with caplog.at_level(logging.WARNING):
+        BackgroundTasksSettings()
+
+    assert "deprecated" not in caplog.text.lower()
+
+
+@pytest.mark.parametrize(
+    "url",
+    [
+        "rediss://cache:6380/0",
+        "REDIS://cache:6379/0",
+    ],
+)
+def test_redis_schemes_still_warn(
+    monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture, url: str
+) -> None:
+    monkeypatch.setenv("SM_BG_TASKS_BROKER_URL", url)
+
+    with caplog.at_level(logging.WARNING):
+        BackgroundTasksSettings()
+
+    assert "SM_BG_TASKS_BROKER_URL is deprecated" in caplog.text
+
+
+@pytest.mark.parametrize("url", ["sentinel://sentinel:26379/0", "redis+socket:///run/redis.sock"])
+def test_schemes_redis_url_cannot_carry_log_no_warning(
+    monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture, url: str
+) -> None:
+    monkeypatch.setenv("SM_BG_TASKS_BROKER_URL", url)
+
+    with caplog.at_level(logging.WARNING):
+        BackgroundTasksSettings()
+
+    assert "deprecated" not in caplog.text.lower()
+
+
+def test_warning_is_per_variable(
+    monkeypatch: pytest.MonkeyPatch, caplog: pytest.LogCaptureFixture
+) -> None:
+    """A Redis broker can move to SM_REDIS_URL even when the backend can't."""
+    monkeypatch.setenv("SM_BG_TASKS_BROKER_URL", "redis://old:6379/4")
+    monkeypatch.setenv("SM_BG_TASKS_RESULT_BACKEND", "rpc://")
+
+    with caplog.at_level(logging.WARNING):
+        BackgroundTasksSettings()
+
+    assert "SM_BG_TASKS_BROKER_URL is deprecated" in caplog.text
+    assert "SM_BG_TASKS_RESULT_BACKEND" not in caplog.text