fix(ws): standardize auth rejection on accept-then-close-with-reason across all WS handlers (#12366)

[mrveiss]

Thinking Path

#12366 is an owner-decided reconciliation: two WS auth-reject conventions coexisted — websockets.py's websocket_endpoint/ws/npu-workers/ws-test and transcripts.py's analyze_transcript_ws closed with code=4001 before accept() (HTTP 403 on the wire, per the old #2818 convention), while live_events.py already did accept() then close() (real WS close frame). The close-before-accept form made an auth-rejected socket indistinguishable from a missing route — root cause of the #12340 misdiagnosis.

Grepped every @router.websocket in autobot-backend (28 endpoints) and every close(4001/close(code=4001 site to find all auth-reject call sites, then checked accept-timing at each:

• Close-before-accept (fixed here): websockets.py ×3, transcripts.py ×1, and the shared ws_security.enforce_ws_admin helper (used by advanced_control.py's /ws/monitoring + /ws/desktop/{id}).
• Already accept-then-close (no change needed): live_events.py (the template), task_workspace_ws.py's workspace_shell (accepts before its admin-auth close), and autobot-slm-backend/api/websocket.py (already documents the exact same GH#10459 uvicorn-403 rationale — good corroborating precedent).
• Out of scope by design: the CSWSH/Origin check (enforce_ws_origin, 1008) intentionally stays close-before-accept — it's a different security concern (cross-site WS hijacking), not the Security: add authentication to WebSocket endpoints #2818 JWT-auth convention this issue reconciles, and is deliberately pre-handshake elsewhere too (task_workspace_ws.py fix(ws): task_workspace_ws — Origin gate requires Authorization header but admin-auth accepts cookies → same-origin cookie-admin lockout #11016). desktop_streaming_manager.py's handle_websocket_client uses the raw websockets library (not FastAPI) — the handshake completes automatically before the handler runs, so there's no .accept() call to reorder there.

Checked the frontend WS clients per the task's client-impact ask: LiveEventService.ts already special-cases close code 4001 as terminal (no reconnect). GlobalWebSocketService.ts (the client for /ws, one of the endpoints converted here) had no handling for 4001 at all — before this fix the browser never even saw it (close-before-accept collapses to an opaque code-1006 abnormal closure in the browser's native WebSocket API), so this was a latent infinite-reconnect-against-a-bad-token bug that would only start manifesting once the backend actually delivers 4001. Fixed alongside the backend change.

What Changed

Backend — accept-then-close on auth rejection (same auth logic, only the timing changed):

• autobot-backend/api/websockets.py: websocket_test_endpoint (/ws-test), websocket_endpoint (/ws), npu_workers_websocket_endpoint (/ws/npu-workers) — each now does await websocket.accept() then await websocket.close(code=4001, reason="Authentication required") instead of closing before accept.
• autobot-backend/api/transcripts.py: analyze_transcript_ws — same conversion, reason="Unauthorized".
• autobot-backend/api/ws_security.py: enforce_ws_admin (shared helper for /ws/monitoring + /ws/desktop/{session_id} in advanced_control.py) — same conversion, reason="Authentication required (admin)".

Frontend — fail-loud handling for the now-visible 4001 close code:

• autobot-frontend/src/services/GlobalWebSocketService.ts: scheduleReconnect now treats close code 4001 as terminal (like LiveEventService already does for /ws/live) — sets connectionState to 'error' with a clear message and stops the reconnect loop, instead of retrying against a rejected token forever.

Tests:

• New autobot-backend/tests/test_websockets_auth_reject_12366.py: parametrized over /ws-test, /ws, /ws/npu-workers — asserts an unauthenticated connect is accepted then closed with 4001 (not a handshake-level rejection), and that a valid bearer proceeds normally.
• Updated tests/test_ws_security_admin.py::test_enforce_ws_admin_accepts_then_closes_4001_when_denied (renamed from _closes_4001_when_denied) to assert accept() is called before close().
• Updated tests/api/test_transcripts.py::test_ws_analyze_rejects_unauthenticated and tests/test_websocket_auth_smoke.py docstrings/assertions for the new accept-then-close semantics (starlette's TestClient now needs an explicit receive_text()/receive_json() to observe the close frame, since the handshake itself no longer fails).

Filed (not fixed — different scope): #12526 — discovered a genuine unrelated bug while writing the /ws/npu-workers happy-path test: NPUWorkerManager._load_workers_from_config (sync, runs via asyncio.to_thread) calls the async def _save_workers_to_config without await, silently discarding the first-run auto-save (RuntimeWarning: coroutine ... was never awaited).

Verification

$ python3 -m pytest autobot-backend/tests/test_ws_security_admin.py autobot-backend/tests/test_websocket_auth_smoke.py autobot-backend/tests/api/test_transcripts.py autobot-backend/tests/test_websockets_auth_reject_12366.py -p no:xdist -q
======================== 35 passed, 1 warning in 0.88s =========================

(the 1 warning is the pre-existing, now-filed #12526 npu_worker_manager bug — unrelated to this PR's WS changes)

$ python3 -m black --check api/transcripts.py api/websockets.py api/ws_security.py tests/api/test_transcripts.py tests/test_websocket_auth_smoke.py tests/test_ws_security_admin.py tests/test_websockets_auth_reject_12366.py
All done! ✨ 🍰 ✨  7 files would be left unchanged.

$ python3 -m flake8 api/transcripts.py api/websockets.py api/ws_security.py tests/api/test_transcripts.py tests/test_websocket_auth_smoke.py tests/test_ws_security_admin.py tests/test_websockets_auth_reject_12366.py
0

Frontend: node_modules isn't installed in this WSL worktree (frontend build/vue-tsc runs on the Windows host per project convention), so vue-tsc couldn't be run directly here. The GlobalWebSocketService.ts change is a 2-line addition using the exact same this.connectionState.value = 'error' / this.state.lastError = <string> patterns already used elsewhere in the same file (lines 434/458), and a brace-balance check on the file passed.

Confirmed pre-existing, unrelated failures in tests/test_cross_worker_registries.py::TestTakeoverManagerCrossWorker (MagicMock-await errors in takeover_manager.py — a file untouched by this PR) reproduce in isolation on this branch; not introduced by this change.

Closes #12366

Model Used

Claude Sonnet 5

[github-actions]

✅ SSOT Configuration Compliance: Passing

🎉 No hardcoded values detected that have SSOT config equivalents!