Demo janitor: the `_DEMO_JANITOR_STARTED` re-entry guard is dead (operator precedence) — a second startup leaks a janitor thread

[byrongamatos]

Found while carving server.py (R3b). Pre-existing — deliberately moved verbatim so the carve stays behaviour-neutral.

server.py, in startup_events():

if getenv_compat("FEEDBACK_DEMO_MODE") or getenv_compat("FEEDBACK_DEMO_MODE") == "1" and not _DEMO_JANITOR_STARTED:
    _DEMO_JANITOR_STARTED = True
    ...
    _DEMO_JANITOR_THREAD = threading.Thread(target=_janitor, daemon=True, name="demo-janitor")
    _DEMO_JANITOR_THREAD.start()

and binds tighter than or, so this parses as:

A or (B == "1" and not C)

The not _DEMO_JANITOR_STARTED check is dead whenever FEEDBACK_DEMO_MODE is truthy — which is precisely the only case where any of this runs.

env="1", already started=True  -> guard fires? True    <-- should be False
intended: (env truthy) and (not started) -> False

Impact

If startup_events() ever runs twice in one process with demo mode on, a second janitor thread starts. _DEMO_JANITOR_THREAD is then overwritten with the new one, so shutdown_events() joins only the last — the first janitor thread is leaked and keeps firing hooks hourly, forever.

_DEMO_JANITOR_STARTED is exactly the re-entry guard someone wrote to prevent this. It has never worked.

Fix

demo_on = getenv_compat("FEEDBACK_DEMO_MODE")
if demo_on and not _DEMO_JANITOR_STARTED:

(The or ... == "1" half is also redundant on its own terms: if the value is truthy the first operand already wins, and if it's "" then "" == "1" is False. It adds nothing. The same redundant shape appears in _demo_mode_guard's middleware check, where it is harmless.)

Test

Call startup_events() twice with FEEDBACK_DEMO_MODE=1 and assert only one thread named demo-janitor is alive, and that shutdown_events() joins it.