Skip to content

feat(python-client): send bearer token on WS upgrade#1400

Open
sridhar-3009 wants to merge 1 commit into
ruvnet:mainfrom
sridhar-3009:fix/python-client-bearer-token-1395
Open

feat(python-client): send bearer token on WS upgrade#1400
sridhar-3009 wants to merge 1 commit into
ruvnet:mainfrom
sridhar-3009:fix/python-client-bearer-token-1395

Conversation

@sridhar-3009

Copy link
Copy Markdown

Fixes #1395.

Problem

SensingClient (python/wifi_densepose/client/ws.py) had no credential path at all — git grep -n "Authorization\|RUVIEW_API_TOKEN" python/ returned nothing. Against a sensing-server with RUVIEW_API_TOKEN set, the Python client simply cannot connect. The TS MCP client (tools/ruview-mcp/src/http.ts) has honoured this token for a while, so this was a gap between the two clients rather than a missing feature overall.

Fix

  • SensingClient.__init__ accepts a token keyword, defaulting to RUVIEW_API_TOKEN when not passed explicitly.
  • When a token is present, __aenter__ sends Authorization: Bearer <token> on the WS upgrade. Python isn't browser-constrained the way the web UI is, so this goes straight on the upgrade request — no ticket/round-trip indirection, per the issue's explicit guidance.
  • Handled the websockets>=12.0 keyword rename deliberately, per the issue's callout: the header kwarg to websockets.connect() is extra_headers below version 14 and additional_headers from 14 onward. Detected via inspect.signature(websockets.connect).parameters at import time, not by parsing __version__ — so it can't be thrown off by a version-string format change on either side of the cutover.

Testing

I didn't just trust the issue's description of the version split — I installed websockets 12.0, 13.1, and 16.1.1 in three isolated venvs and inspected the actual connect() signature in each to confirm exactly where extra_headers becomes additional_headers before writing the branch logic.

Added three tests to test_client_ws.py using the same in-process websockets.serve() pattern the file already uses:

  • token passed to the constructor arrives as the correct Authorization: Bearer <token> header
  • no token passed and RUVIEW_API_TOKEN unset → no Authorization header sent at all (not an empty one)
  • RUVIEW_API_TOKEN env var is honoured when no token is passed explicitly

Ran the full test_client_ws.py suite (9 tests, including the 3 new ones) against both websockets 13.1 and 16.1.1 in separate venvs — all pass on both sides of the keyword rename. Also confirmed the change doesn't newly break ruff check beyond one pre-existing style debt already present in this file (Optional[X] vs X | None) that I matched rather than mixed styles.

SensingClient had no credential path at all — grep for
Authorization|RUVIEW_API_TOKEN across python/ returned nothing — so it
cannot connect to a sensing-server with RUVIEW_API_TOKEN set. The TS
MCP client has honoured this token for a while; this closes the gap
for the Python client.

Added a token param to SensingClient.__init__, defaulting to
RUVIEW_API_TOKEN when not passed explicitly, sent as
'Authorization: Bearer <token>' on the WS upgrade when present. Python
isn't browser-constrained like the web UI, so the header goes straight
on the upgrade request — no ticket/round-trip indirection needed.

Handled the websockets>=12.0 keyword-argument rename deliberately: the
custom-headers kwarg to websockets.connect() is extra_headers below
version 14 and additional_headers from 14 onward. Detected via
inspect.signature() at import time rather than parsing __version__, so
it can't be thrown off by a version string format change. Verified
against 12.0, 13.1, and 16.1.1 directly (installed each in an isolated
venv and inspected the real connect() signature) before writing the
branch — a naive single-keyword implementation would work on whichever
version happened to be installed and raise TypeError on the other side
of the cutover.

Tests cover: token sent as the correct header value, no token passed
and RUVIEW_API_TOKEN unset means no Authorization header is sent at
all (not an empty one), and the RUVIEW_API_TOKEN env fallback. All
pass against both websockets 13.1 (extra_headers) and 16.1.1
(additional_headers) — ran the whole suite against each in isolated
venvs to confirm the branch picks the right keyword on both sides of
the rename.

Fixes ruvnet#1395
@PrinceAudre

Copy link
Copy Markdown

Heads-up: #1400 and #1396 implement the same feature — bearer-token auth on the SensingClient WebSocket upgrade — editing the same lines of python/wifi_densepose/client/ws.py and python/tests/test_client_ws.py (both close #1395). So only one can merge; the other will conflict.

Both look sound. This one (#1400) fits the file's existing Optional[...] style and resolves the additional_headers/extra_headers kwarg once at import; #1396 adds a dedicated unit test for that version-split branch. Either is a fine choice — just pick one and close the other.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

python: client cannot authenticate — no token handling at all, so it fails against any auth-enabled server

2 participants