From bba57e1b3998f678a4ab551d900d76ab8be43be3 Mon Sep 17 00:00:00 2001 From: Vuong Hoang Date: Sun, 19 Jul 2026 12:58:10 -0700 Subject: [PATCH] =?UTF-8?q?fix(#20):=20heid-code-review=20fixups=20?= =?UTF-8?q?=E2=80=94=20stale=20docstring=20+=20None-cursor=20test=20(slice?= =?UTF-8?q?-6)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Panel (Gróa + Hulda + Regin): 3/3 no drift — the admin adapter honors the contract (route map, re-wrap/degrade, error-map ORDER, admin_auth-on-client, INV-CUT-1). Only minor doc/test looseness, both fixed: - Stale docstring: `_session_bifrost_endpoint` still said "the wrapper overrides the Authorization header with it" — corrected to "rides on the wt client's admin_auth" (slice-6 moved admin auth off the per-call header; line 79 already said the new way). - Test-gap: the admin-stream ConnectionDropped test only exercised the cursor-set case; added the connect-time None-cursor case (ConnectionDropped(None) → last_seen_sse_id None) to back the map's "both cursor shapes" claim. Not acted on: `admin_key`→`admin_auth` unit assertion (the SDK's use of admin_auth is SDK-internal/private — out of scope per "assess use, not definitions"; the LIVE SMOKE already proved the wiring end-to-end). Hulda's "web endpoints under-tested" flag was source-VOIDED by Heid: those endpoints ARE covered in test_web_server.py, which wasn't in the consult embed (excerpt-elides-tests trap). Suite 491 green; ruff clean. Docs + test only — no version bump (SemVer skip rule). --- src/ratatoskr/web/server.py | 7 ++++--- tests/test_wt.py | 14 +++++++++++--- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/src/ratatoskr/web/server.py b/src/ratatoskr/web/server.py index 80518f2..70269d2 100644 --- a/src/ratatoskr/web/server.py +++ b/src/ratatoskr/web/server.py @@ -572,9 +572,10 @@ async def _session_bifrost_endpoint(request: Request) -> JSONResponse: """GET /api/sessions/{session_id}/bifrost → admin-scoped Bifrost dispatch state (#176). The admin key is SERVER-HELD (app.state.admin_key) and never reaches the - browser (INV-003 precedent — upstream credentials stay server-side); the - wrapper overrides the Authorization header with it. Fail-visible when the - admin key isn't configured (never a silent empty pane).""" + browser (INV-003 precedent — upstream credentials stay server-side); it rides on + the wt client's `admin_auth` (`_wt_client(admin_key=…)`), which the SDK uses for + the `admin.*` routes (NOT a per-call header — slice-6). Fail-visible when the admin + key isn't configured (never a silent empty pane).""" session_id = request.path_params["session_id"] admin_key = request.app.state.admin_key if not admin_key: # PRE-001: fail-visible, never silent diff --git a/tests/test_wt.py b/tests/test_wt.py index 47814f7..cabf3c4 100644 --- a/tests/test_wt.py +++ b/tests/test_wt.py @@ -1212,10 +1212,18 @@ class TestStreamAdminEventsWt: await _drain(stream_admin_events(_wtad(fake))) assert ei.value.status == 502 - async def test_connection_dropped_maps_and_carries_cursor(self) -> None: - # Both a connect-time failure (cursor None) and a mid-stream drop / resumable - # EOF (cursor set) surface as ConnectionDropped → SseConnectionDropped. + async def test_connection_dropped_carries_cursor(self) -> None: + # A mid-stream drop / resumable EOF carries the resume cursor. fake = _FakeAdmin(stream_error=wtsdk.ConnectionDropped("42")) with pytest.raises(SseConnectionDropped) as ei: await _drain(stream_admin_events(_wtad(fake))) assert ei.value.last_seen_sse_id == "42" + + async def test_connection_dropped_none_cursor_connect_time(self) -> None: + # A connect-time transport failure surfaces as ConnectionDropped(None) → + # SseConnectionDropped(last_seen_sse_id=None) (the map's other cursor shape; + # heid-code-review slice-6 test-gap). + fake = _FakeAdmin(stream_error=wtsdk.ConnectionDropped(None)) + with pytest.raises(SseConnectionDropped) as ei: + await _drain(stream_admin_events(_wtad(fake))) + assert ei.value.last_seen_sse_id is None