Trim comments and docstrings to the essentials

Cut the comment and docstring volume of the new tests by half: docstring
first lines are single short sentences, narration and restating-the-code
comments are gone, and surviving comments carry only what the code cannot
say - provenance labels, the divergence re-pin instructions, and the
non-obvious traps (coverage branch quirks, opaque-state shape). No code or
assertion changes: every file parses identically with docstrings stripped.

Author: maxisbey

tests/interaction/auth/_harness.py

@@ -133,22 +133,16 @@ class HeadlessOAuth:
     request does not re-enter the locked auth flow), parses `code` and `state` from the 302
     `Location`, and stashes them; `callback_handler` returns the stashed pair. Tests inspect
     `authorize_url` to assert what the SDK put on the authorize request, and `iss`/`error` to
-    assert what the redirect carried — both record the redirect regardless of the
-    callback-boundary levers below.
+    assert what the redirect carried.
 
     `state_override`: when set, `callback_handler` returns this value as the state instead of
     the one parsed from the redirect, so tests can drive the state-mismatch path.
 
     `iss_override`: when set, `callback_handler` returns this value as the RFC 9207 issuer
     instead of the one parsed from the redirect, so tests can drive the iss-mismatch path.
 
-    `code_override`: when set, `callback_handler` returns this value as the authorization code
-    instead of the one parsed from the redirect, so tests can drive the token-endpoint
-    rejection path.
-
-    `omit_iss`: when set, `callback_handler` returns no iss regardless of what the redirect
-    carried or what `iss_override` supplies (omission wins when both are set), so tests can
-    drive the missing-iss paths (the `iss_override` sentinel cannot express absence).
+    `code_override`: when set, returned as the code instead of the parsed one (token-endpoint rejection path).
+    `omit_iss`: when set, no iss is returned, overriding everything (`iss_override` cannot express absence).
     """
 
     def __init__(
@@ -341,9 +335,7 @@ def step_up_shim(www_authenticate: str, *, on_nth_authenticated_post: int = 2, p
     after the 401 branch, where the generator ends without inspecting the response), so a 403
     there would not reach the step-up handler.
 
-    `persist`: when set, every authenticated POST from the Nth onward receives the 403 challenge
-    instead of only the Nth, so tests can drive a further `insufficient_scope` challenge on the
-    request retried after a step-up.
+    `persist`: when set, 403s every authenticated POST from the Nth onward, re-challenging the step-up retry.
     """
     seen = 0
     fired = False

tests/interaction/auth/_provider.py

@@ -49,9 +49,8 @@ class InMemoryAuthorizationServerProvider(
         `fail_next_refresh`: the next refresh-token exchange raises `invalid_grant` once.
         `reject_all_tokens`: `load_access_token` returns None for every token, so the bearer
             middleware 401s every authenticated request.
-        `rotate_refresh_tokens`: when False, the refresh exchange issues only a new access
-            token (the response carries no `refresh_token` and the presented one stays valid),
-            modelling an RFC 6749 §6 non-rotating authorization server.
+        `rotate_refresh_tokens`: when False, the refresh response carries no `refresh_token` and
+            the presented one stays valid (an RFC 6749 §6 non-rotating server).
     """
 
     def __init__(
@@ -183,11 +182,7 @@ async def load_refresh_token(self, client: OAuthClientInformationFull, refresh_t
     async def exchange_refresh_token(
         self, client: OAuthClientInformationFull, refresh_token: RefreshToken, scopes: list[str]
     ) -> OAuthToken:
-        """Mint a new access token and rotate the refresh token, consuming the old one.
-
-        Unless `rotate_refresh_tokens` is off: then only a new access token is minted, the
-        response carries no `refresh_token`, and the presented one stays valid.
-        """
+        """Mint a new access token, and rotate the refresh token unless rotation is disabled."""
         assert client.client_id is not None
         if self._fail_next_refresh:
             self._fail_next_refresh = False

tests/interaction/auth/test_as_handlers.py

@@ -307,14 +307,7 @@ async def test_register_echoes_native_for_a_client_that_registered_application_t
 ) -> None:
     """A client registering `application_type: "web"` is told `"native"` in the registration echo.
 
-    Pins the known gap recorded on the requirement (divergence): the registration handler's
-    field-by-field passthrough omits `application_type`, so the model default fills the echo
-    where RFC 7591 §3.2.1 requires the registered value -- and the SDK OAuth client adopts the
-    echo into persisted storage, so the corruption is client-visible end to end. When the
-    one-line passthrough fix lands this test fails: re-pin the echo to `"web"`, delete the
-    Divergence, and add the echo assertion to
-    `test_dcr_sends_a_consumer_set_application_type_verbatim` (test_flow.py) per the
-    requirement's note.
+    When the passthrough fix lands: re-pin the echo to `"web"` and delete the Divergence.
     """
     http, _ = as_app
     metadata = OAuthClientMetadata(
@@ -323,10 +316,8 @@ async def test_register_echoes_native_for_a_client_that_registered_application_t
 
     response = await http.post("/register", content=metadata.model_dump_json())
 
-    # Registration itself succeeds: the divergence is in the echo, not in acceptance.
     assert response.status_code == 201
     body = response.json()
-    # The request carried "web" (the metadata above); the echo says "native" -- the pinned gap.
     assert body["application_type"] == "native"
     # The omission is specific to application_type, not a generally lossy echo.
     assert body["client_name"] == "interaction-suite"

tests/interaction/auth/test_authorize_token.py

@@ -193,11 +193,8 @@ async def test_a_mismatched_iss_on_the_callback_aborts_the_flow() -> None:
     """A callback whose RFC 9207 iss does not match the authorization server issuer aborts the flow.
 
     `iss_override` makes the headless callback return an issuer the AS never advertised; the SDK
-    compares it to `oauth_metadata.issuer` and raises `OAuthFlowError` before the token exchange --
-    the recorded traffic shows no /token POST, so the tainted authorization code is never exchanged.
-    Also pins the mismatch arm of `client-auth:iss:unadvertised-present-validated`: the served AS
-    metadata never advertises `authorization_response_iss_parameter_supported`, so this rejection is
-    RFC 9207 validation table row 3 -- a present iss is validated regardless of advertisement.
+    compares it to `oauth_metadata.issuer` and raises `OAuthFlowError` before the token exchange.
+    Also the row-3 mismatch arm: a present iss is validated even though the AS never advertises iss support.
     """
     recorded, on_request = record_requests()
     provider = InMemoryAuthorizationServerProvider()
@@ -431,13 +428,8 @@ async def test_an_authorize_error_on_the_callback_aborts_the_flow_before_the_tok
 async def test_a_token_endpoint_error_response_aborts_the_flow_without_a_bearer_request() -> None:
     """A token-endpoint error response aborts the flow as `OAuthTokenError`, and no bearer is ever sent.
 
-    SDK-defined surfacing (no spec mandate governs client-side token-error handling):
-    `code_override` forges the authorization code at the callback boundary, so the SDK's own
-    token handler produces a genuine RFC 6749 `invalid_grant` 400 -- no shim anywhere. The
-    matched message pins only the SDK-authored prefix naming the HTTP status; the tail is the
-    server handler's JSON, and the absent machine-readable code is the deferred
-    `client-auth:token-error:machine-readable-code`. The recorded traffic proves the failed
-    exchange was not retried and no request ever carried a bearer token.
+    SDK-defined behaviour. The match pins only the SDK-authored status prefix; the missing
+    machine-readable error code is the deferred `client-auth:token-error:machine-readable-code`.
     """
     recorded, on_request = record_requests()
     provider = InMemoryAuthorizationServerProvider()
@@ -450,21 +442,16 @@ async def test_a_token_endpoint_error_response_aborts_the_flow_without_a_bearer_
         ):
             await connect_with_oauth(server, provider=provider, headless=headless, on_request=on_request).__aenter__()
 
-    # The flow genuinely reached the token step via a real authorize round trip...
+    # Guards that the failure happened at the token step, not earlier in the flow.
     assert find(recorded, "GET", "/authorize") != []
-    # ...the failed exchange was not retried (no loop)...
     assert len(find(recorded, "POST", "/token")) == 1
-    # ...and no request ever carried a bearer: the failure pre-empted token use entirely.
     assert all("authorization" not in r.headers for r in find(recorded, "POST", "/mcp"))
 
 
 def canned_asm(*, iss_advertised: bool | None) -> dict[str, bytes]:
-    """Build a `serve=` override: canned AS metadata with a slash-explicit issuer literal.
+    """Build a `serve=` override: canned AS metadata pinning the iss-advertisement arm.
 
-    The SDK server's `build_metadata` never sets `authorization_response_iss_parameter_supported`,
-    so the advertising arms of the RFC 9207 validation table need this override; `None` omits the
-    field (`exclude_none`), keeping the document an unadvertising AS whose issuer bytes are still
-    harness-pinned.
+    Needed because the SDK server's `build_metadata` never advertises iss support; `None` omits the field.
     """
     override = OAuthMetadata(
         issuer=AnyHttpUrl(f"{BASE_URL}/"),
@@ -483,13 +470,8 @@ def canned_asm(*, iss_advertised: bool | None) -> dict[str, bytes]:
 async def test_a_matching_iss_lets_the_flow_redeem_the_code_when_the_as_advertises_iss_support() -> None:
     """A callback iss equal to the recorded metadata issuer proceeds to redeem the code (RFC 9207 table row 1).
 
-    Spec-mandated: advertised support + present matching iss -> compare and proceed. The shim
-    serves AS metadata advertising `authorization_response_iss_parameter_supported`; the suite's
-    provider stamps the matching iss on the success redirect. `headless.iss` proves the callback
-    really carried the recorded issuer (completion alone could also mean the value was never
-    looked at -- the rejection arms of the family are the discriminators). Whether the SDK
-    consulted the advertisement flag is not asserted: rows 1 and 3 share one unconditional
-    comparison branch, so the flag's effect is only observable on the absent-iss arms.
+    Spec-mandated. `headless.iss` proves the callback really carried the issuer; whether the SDK
+    consulted the advertisement flag is only observable on the absent-iss arms, so it is not asserted.
     """
     recorded, on_request = record_requests()
     provider = InMemoryAuthorizationServerProvider()
@@ -509,7 +491,6 @@ async def test_a_matching_iss_lets_the_flow_redeem_the_code_when_the_as_advertis
     assert result.tools[0].name == "echo"
     assert storage.tokens is not None
     assert headless.iss == f"{BASE_URL}/"
-    # One authorize and one token exchange: the flow proceeded directly, no rejection/retry loop.
     assert len(find(recorded, "GET", "/authorize")) == 1
     assert len(find(recorded, "POST", "/token")) == 1
 
@@ -518,14 +499,8 @@ async def test_a_matching_iss_lets_the_flow_redeem_the_code_when_the_as_advertis
 async def test_an_iss_differing_only_by_a_trailing_slash_is_rejected_without_normalization() -> None:
     """An iss equal to the recorded issuer up to a trailing slash is a mismatch: nothing is normalized away.
 
-    Spec-mandated: the comparison is RFC 9207 simple string comparison, and the client MUST NOT
-    apply scheme or host case folding, default-port elision, trailing-slash, or percent-encoding
-    normalization before comparing; the trailing-slash arm is pinned as the representative class
-    (the SDK's comparison is a single string inequality). Both spellings are harness literals:
-    the canned metadata carries the slash-explicit issuer and the provider stamps the slash-less
-    redirect iss. Natural metadata would instead source the slash from the SDK server's issuer
-    serialization -- the load-bearing, churn-prone arm of that difference -- whereas the client's
-    metadata parse preserves the served bytes and contributes nothing to it.
+    Spec-mandated: RFC 9207 simple string comparison with no normalization; the trailing-slash
+    arm is pinned as the representative class (the SDK's comparison is a single string inequality).
     """
     recorded, on_request = record_requests()
     provider = InMemoryAuthorizationServerProvider(issuer=BASE_URL)
@@ -550,10 +525,7 @@ async def test_an_iss_differing_only_by_a_trailing_slash_is_rejected_without_nor
 async def test_a_missing_iss_is_rejected_when_the_as_advertises_iss_support() -> None:
     """A callback without iss is rejected before the code is redeemed when the AS advertises iss support (row 2).
 
-    Spec-mandated: advertised support + absent iss -> reject. The SDK's whole authorization-response
-    input is the callback's `AuthorizationCodeResult`, so the absence is constructed at that boundary
-    with `omit_iss` (the suite's AS emits iss on every success redirect; the redirect itself is
-    unchanged). No /token POST is recorded -- the authorization code is never exchanged.
+    Spec-mandated. The callback is the SDK's whole authorization-response input, so `omit_iss` removes iss there.
     """
     recorded, on_request = record_requests()
     provider = InMemoryAuthorizationServerProvider()
@@ -584,11 +556,8 @@ async def test_a_missing_iss_is_rejected_when_the_as_advertises_iss_support() ->
 async def test_a_missing_iss_is_tolerated_when_the_as_does_not_advertise_iss_support() -> None:
     """A callback without iss proceeds with the code exchange when the AS does not advertise iss support (row 4).
 
-    Spec-mandated: no advertisement + absent iss -> proceed. The SDK server's `build_metadata`
-    never sets `authorization_response_iss_parameter_supported`, so the natural metadata route IS
-    the unadvertising AS; if it ever started advertising, this test would fail loudly (absent iss
-    + advertised -> the row-2 reject), so the precondition cannot silently rot. The absent iss is
-    constructed at the callback boundary with `omit_iss`.
+    Spec-mandated. Natural metadata is already the unadvertising arm; if the SDK server ever
+    started advertising, absent iss would hit the row-2 reject, so the precondition cannot rot.
     """
     recorded, on_request = record_requests()
     provider = InMemoryAuthorizationServerProvider()
@@ -604,20 +573,15 @@ async def test_a_missing_iss_is_tolerated_when_the_as_does_not_advertise_iss_sup
 
     assert result.tools[0].name == "echo"
     assert storage.tokens is not None
-    # Exactly one token exchange: the flow proceeded directly, no rejection/retry detour.
     assert len(find(recorded, "POST", "/token")) == 1
 
 
 @requirement("client-auth:iss:unadvertised-present-validated")
 async def test_a_present_iss_is_validated_and_accepted_even_when_the_as_does_not_advertise_support() -> None:
     """A present iss is compared against the recorded issuer even without metadata advertisement (row 3, match arm).
 
-    Spec-mandated, and the point where the MCP spec deliberately exceeds RFC 9207's local-policy
-    provision: a present iss is validated regardless of advertisement. The natural AS metadata is
-    the unadvertising AS and the provider stamps the matching iss; `headless.iss` proves it was
-    present on the callback. The match arm alone cannot prove the comparison ran -- the same
-    entry's mismatch arm is pinned by `test_a_mismatched_iss_on_the_callback_aborts_the_flow`,
-    which drives a mismatched iss against the same unadvertising AS.
+    Spec-mandated; MCP exceeds RFC 9207's local-policy provision here. The mismatch arm is pinned
+    by `test_a_mismatched_iss_on_the_callback_aborts_the_flow`.
     """
     recorded, on_request = record_requests()
     provider = InMemoryAuthorizationServerProvider()
@@ -641,14 +605,8 @@ async def test_a_present_iss_is_validated_and_accepted_even_when_the_as_does_not
 async def test_an_error_redirect_with_a_mismatched_iss_is_rejected_on_iss_before_the_missing_code_error() -> None:
     """iss validation applies equally to error responses: the mismatch is raised before the missing-code error.
 
-    Spec-mandated at 2026-07-28 (SEP-2468): the AS denies the authorize request, producing a real
-    RFC 6749 error redirect (`error=access_denied`, no code, no iss), and `iss_override` supplies
-    the mismatching issuer at the callback boundary -- the SDK never parses a callback URL, so its
-    whole authorization-response input is the callback's `AuthorizationCodeResult`. Raising the
-    iss mismatch in preference to "No authorization code received" (the arm the error-surfaces
-    test above pins) is the observable proving the validation ran on an error response. The
-    MUST-NOT-act-on-or-display half is not asserted: the callback contract carries no error
-    fields, so the negative would be vacuously true by construction (the manifest note records it).
+    Spec-mandated at 2026-07-28 (SEP-2468). The mismatch pre-empting the missing-code error proves
+    validation ran; the MUST-NOT-act-on half is vacuous (no error fields in the callback contract).
     """
     recorded, on_request = record_requests()
     provider = InMemoryAuthorizationServerProvider(deny_authorize=True)
@@ -661,7 +619,6 @@ async def test_an_error_redirect_with_a_mismatched_iss_is_rejected_on_iss_before
         ):
             await connect_with_oauth(server, provider=provider, headless=headless, on_request=on_request).__aenter__()
 
-    # The callback genuinely was an error response, and the tainted exchange never started.
     assert headless.error == "access_denied"
     # The recorded unauthenticated trigger POST guards the negative below against an unwired hook.
     assert find(recorded, "POST", "/mcp") != []

tests/interaction/auth/test_bearer.py

@@ -158,16 +158,11 @@ async def test_a_token_missing_a_required_scope_is_answered_403_insufficient_sco
 
 @requirement("hosting:auth:scope-403:all-scopes")
 async def test_a_token_missing_two_required_scopes_is_challenged_with_only_the_first() -> None:
-    """A token missing both required scopes is challenged for one scope per round trip.
-
-    The spec says servers SHOULD include all scopes required for the current operation in a
-    single challenge; the bearer middleware instead checks required scopes in order and 403s on
-    the first missing one, naming only that scope in `error_description` (and emitting no
-    `scope` parameter at all -- the sibling `hosting:auth:scope-403` divergence). Pins the known
-    gap recorded on the requirement (divergence); when the middleware aggregates, this test
-    fails -- re-pin to a challenge naming both scopes and delete the Divergence. The two-scope
-    app is built inline: the file's `protected` fixture is single-scope, and a two-scope deficit
-    is this entry's distinct observable.
+    """A token missing both required scopes is challenged for only the first missing scope.
+
+    Pins the recorded divergence: the middleware checks required scopes in order and names
+    only the first missing one, where the spec wants all of them in a single challenge.
+    When the middleware aggregates: re-pin to a challenge naming both scopes and delete the Divergence.
     """
     settings = auth_settings(required_scopes=["mcp:read", "mcp:write"])
     verifier = StaticTokenVerifier(
@@ -178,8 +173,6 @@ async def test_a_token_missing_two_required_scopes_is_challenged_with_only_the_f
         response = await post_mcp(http, bearer="tok-zeroscope")
 
     assert response.status_code == 403
-    # Full-dict equality: only the FIRST missing scope (registration order) is named, and no
-    # `scope` key appears.
     assert parse_www_authenticate(response.headers["www-authenticate"]) == {
         "error": "insufficient_scope",
         "error_description": "Required scope: mcp:read",