docs: clarify comments and remove implementation history

crates/core/ras-auth-core/src/authorize.rs

@@ -1,10 +1,8 @@
 //! Shared request-authorization pipeline for generated services.
 //!
-//! Every service macro (REST, file, JSON-RPC, bidirectional WebSocket) used
-//! to inline its own copy of the credential → CSRF → authenticate →
-//! permission-group sequence. These helpers are the single implementation;
-//! generated code maps the returned [`AuthorizeError`] to its own protocol's
-//! response shape.
+//! REST and file services share credential, CSRF, authentication, and permission
+//! checks here. Generated code maps [`AuthorizeError`] to protocol responses.
+//! JSON-RPC and WebSocket services also use the permission-group helpers.
 
 use crate::{
     AuthError, AuthProvider, AuthTransportConfig, AuthenticatedUser, Caller,
@@ -17,7 +15,7 @@ use http::HeaderMap;
 pub enum AuthorizeError {
     /// No usable credential was found in the request
     MissingCredential,
-    /// Double-submit CSRF validation failed for a cookie credential
+    /// CSRF validation failed for a cookie credential
     CsrfValidationFailed,
     /// The credential did not authenticate
     AuthenticationFailed(AuthError),
@@ -37,7 +35,7 @@ pub enum AuthorizeError {
 /// requirement — `WITH_PERMISSIONS([])`, i.e. no groups or only empty groups.
 /// An empty group mixed with non-empty siblings (`["admin"] | []`) is ignored
 /// here rather than treated as a blanket grant; the service macros reject that
-/// shape at compile time, and this runtime guard is the belt-and-suspenders.
+/// shape at compile time; direct callers receive the same protection.
 pub fn check_permission_groups<P>(
     provider: &P,
     user: &AuthenticatedUser,
@@ -83,8 +81,8 @@ pub fn user_satisfies_permission_groups(user: &AuthenticatedUser, groups: &[Vec<
 /// generated REST and file-service servers.
 ///
 /// `method` is the HTTP method, used to scope CSRF validation to unsafe
-/// requests. Errors are ordered so no work happens for unauthenticated
-/// callers: the request body has not been touched when this returns `Err`.
+/// requests. This helper does not read the body; callers should authorize before
+/// buffering or parsing it.
 pub async fn authorize_request<P>(
     method: &str,
     headers: &HeaderMap,

crates/core/ras-auth-core/src/lib.rs

@@ -1,4 +1,4 @@
-//! Authentication and authorization traits for JSON-RPC services.
+//! Authentication, authorization, and HTTP credential handling for RAS services.
 
 mod authorize;
 mod transport;
@@ -101,12 +101,8 @@ pub struct AuthenticatedUser {
 ///   an unsafe method all resolve to anonymous).
 /// * [`Caller::Authenticated`] — a valid credential was presented.
 ///
-/// Deliberately **not** `Serialize`/`Deserialize`: a `Caller` represents a
-/// *resolved* identity and must only be produced by [`resolve_caller`], never
-/// reconstructed from request input. The `#[must_use]` attribute flags a
-/// discarded [`resolve_caller`] result; note it cannot catch a handler that
-/// receives `caller` as a parameter and never reads it (Rust applies `must_use`
-/// to discarded expression results, not to unused bindings).
+/// A `Caller` represents a resolved identity, so it cannot be deserialized from
+/// request input. Construct it from trusted authentication results.
 #[must_use]
 #[derive(Debug, Clone)]
 pub enum Caller {
@@ -160,8 +156,7 @@ pub type AuthFuture<'a, T = AuthenticatedUser> =
 
 /// Trait for implementing authentication providers.
 ///
-/// This trait allows for flexible authentication mechanisms while providing
-/// a consistent interface for the JSON-RPC service layer.
+/// Services use this interface to validate tokens and check permissions.
 pub trait AuthProvider: Send + Sync + 'static {
     /// Validates a token and returns the authenticated user.
     ///

crates/core/ras-transport-core/src/error.rs

@@ -1,9 +1,7 @@
 //! Typed transport error.
 //!
-//! Generated clients return `Result<T, TransportError>` instead of the old
-//! `Box<dyn std::error::Error + Send + Sync>`, so callers can match on the
-//! failure mode (connection vs. HTTP status vs. (de)serialization vs. a
-//! JSON-RPC application error).
+//! Generated clients expose distinct connection, HTTP status, serialization,
+//! body, and JSON-RPC errors so callers can handle each failure separately.
 
 use thiserror::Error;
 

crates/core/ras-transport-core/src/lib.rs

@@ -1,27 +1,17 @@
 //! HTTP transport abstraction for generated Rust Agent Stack clients.
 //!
-//! Generated REST / JSON-RPC / File clients dispatch through the
-//! [`HttpTransport`] trait instead of hard-coding `reqwest::Client`. Two impls
-//! ship here: [`ReqwestTransport`] (production, `reqwest` feature) and
-//! [`AxumTestTransport`] (in-process, wraps `axum_test::TestServer`, native +
-//! `axum-test` feature) so clients can be exercised end-to-end against a server
-//! with no sockets.
+//! Generated REST, JSON-RPC, and file clients use [`HttpTransport`]. The
+//! `reqwest` feature provides a network adapter; the native `axum-test` feature
+//! provides an in-process adapter for exercising clients without sockets.
 //!
 //! # Relationship to `WebSocketTransport`
 //!
-//! This trait is the HTTP sibling of the `WebSocketTransport` abstraction in
-//! `ras-jsonrpc-bidirectional-client`
-//! (`crates/rpc/bidirectional/ras-jsonrpc-bidirectional-client/src/lib.rs`).
-//! Both follow the same dyn-dispatch + conditional-`Send` pattern:
-//! `#[cfg_attr(target_arch = "wasm32", async_trait(?Send))]` together with the
-//! [`TransportThreadBounds`] marker, so a single `Arc<dyn _>` works on both
-//! native and wasm targets. They are intentionally separate: bidirectional RPC
-//! is WebSocket (full-duplex frames), not request/response HTTP, and must not
-//! be routed through `HttpTransport`.
+//! `ras-jsonrpc-bidirectional-client` owns the separate `WebSocketTransport`
+//! abstraction because full-duplex frames have a different lifecycle from HTTP
+//! requests. Both transport traits support native and WASM implementations.
 //!
-//! On wasm, request bodies cannot be streamed (the fetch API has no streaming
-//! request body), so [`RequestBody::Stream`] is collected before sending;
-//! response bodies still stream.
+//! `ReqwestTransport` streams request and response bodies on native targets
+//! and buffers both on WASM. `AxumTestTransport` buffers both bodies.
 
 use std::pin::Pin;
 
@@ -56,8 +46,6 @@ pub use axum_test_transport::AxumTestTransport;
 /// `::ras_transport_core::http::Method` etc. without a direct dependency.
 pub use http;
 
-// --- Thread-bound marker, mirroring the bidirectional-client precedent. ---
-
 /// Marker for the thread bounds a transport (and its streams) must satisfy.
 ///
 /// `Send + Sync` on native; unconstrained on wasm (single-threaded).
@@ -74,8 +62,6 @@ pub trait TransportThreadBounds {}
 #[cfg(target_arch = "wasm32")]
 impl<T> TransportThreadBounds for T {}
 
-// --- Byte stream alias, conditionally `Send`. ---
-
 /// A streaming sequence of body chunks. `Send` on native, not on wasm.
 #[cfg(not(target_arch = "wasm32"))]
 pub type ByteStream = Pin<Box<dyn Stream<Item = Result<Bytes, TransportError>> + Send>>;
@@ -102,8 +88,6 @@ where
     Box::pin(stream)
 }
 
-// --- The transport trait. ---
-
 /// Abstraction over the wire transport used by a generated HTTP client.
 ///
 /// See the [crate-level docs](crate) for the relationship with
@@ -120,24 +104,14 @@ pub trait HttpTransport: TransportThreadBounds {
     -> Result<TransportResponse, TransportError>;
 }
 
-// --- Query / JSON helpers. ---
-
-/// Serialize a single query value to its `application/x-www-form-urlencoded`
-/// form for one `key`, returning zero or more `(key, value)` pairs.
+/// Convert a query value into decoded `(key, value)` pairs for form encoding.
 ///
-/// Serializing one key/value at a time (rather than a whole struct) means a
-/// `Vec<T>` produces repeated keys and `#[serde(rename = ...)]` enum variants
-/// encode by their rename — matching reqwest's old `.query()` behavior exactly.
-/// `Option::None` produces no pairs.
+/// Sequences produce repeated keys, enum variants honor `#[serde(rename)]`,
+/// and `Option::None` produces no pairs. Encode with [`serialize_query_pairs`].
 pub fn serialize_query_value<T: Serialize>(
     key: &str,
     value: &T,
 ) -> Result<Vec<(String, String)>, TransportError> {
-    // We collect each scalar value into its own `(key, encoded_value)` entry so
-    // that scalars produce one pair, sequences produce repeated keys, and
-    // `None` produces nothing. Each scalar is rendered through
-    // `serde_urlencoded` (one key=value pair) so enum `#[serde(rename)]`s and
-    // numeric/bool formatting match reqwest's old `.query()` byte-for-byte.
     let mut collector = QueryValueCollector { values: Vec::new() };
     value
         .serialize(&mut collector)
@@ -152,12 +126,9 @@ pub fn serialize_query_value<T: Serialize>(
 /// Serialize several `(key, value)` query parameters and join them into a
 /// single query string (without a leading `?`). Empty result if no pairs.
 ///
-/// The final byte-level encoding is delegated to `serde_urlencoded` (the same
-/// crate reqwest's `.query()` uses internally), so the produced wire query
-/// string matches the pre-transport reqwest client exactly — including its
-/// `application/x-www-form-urlencoded` unreserved set (`*` stays raw, `~`
-/// becomes `%7E`, space becomes `+`). Keys are emitted in order, so repeated
-/// keys (from `Vec<T>`) preserve their sequence.
+/// Uses `application/x-www-form-urlencoded` encoding: `*` stays raw, `~`
+/// becomes `%7E`, and space becomes `+`. Pair order, including repeated keys,
+/// is preserved.
 ///
 /// Returns [`TransportError::Serialize`] on encoding failure rather than
 /// silently yielding an empty string — generated clients append the result
@@ -210,11 +181,8 @@ fn hex_digit(nibble: u8) -> char {
     }
 }
 
-// --- internal helpers ---
-
-/// Render one scalar value through `serde_urlencoded` and return the decoded
-/// value string (the part after `=` of a single `k=v` pair), so enum renames
-/// and scalar formatting match reqwest's old `.query()` exactly.
+/// Preserve form-serializer scalar formatting and enum renames while returning
+/// a decoded value for [`serialize_query_pairs`].
 fn encode_scalar<T: Serialize>(value: &T) -> Result<String, serde_json::Error> {
     use serde::ser::Error as _;
     // serde_urlencoded serializes a sequence of (key, value) tuples.

crates/core/ras-transport-core/src/reqwest_transport.rs

@@ -64,7 +64,7 @@ impl HttpTransport for ReqwestTransport {
             RequestBody::Stream(stream) => builder.body(reqwest::Body::wrap_stream(stream)),
             #[cfg(target_arch = "wasm32")]
             RequestBody::Stream(mut stream) => {
-                // wasm fetch cannot stream request bodies; collect first.
+                // The WASM adapter sends buffered request bodies.
                 let mut buf = bytes::BytesMut::new();
                 while let Some(chunk) = stream.next().await {
                     buf.extend_from_slice(&chunk?);
@@ -77,9 +77,7 @@ impl HttpTransport for ReqwestTransport {
         let status = resp.status();
         let headers = resp.headers().clone();
 
-        // Native streams the response body; wasm reqwest lacks the `stream`
-        // feature, so collect into a single chunk (response streaming on wasm
-        // is bounded by the fetch implementation regardless).
+        // The WASM reqwest dependency lacks response streaming support.
         #[cfg(not(target_arch = "wasm32"))]
         let body_stream = byte_stream_from(
             resp.bytes_stream()

crates/core/ras-transport-core/tests/query_serialization.rs

@@ -58,10 +58,7 @@ fn pairs_are_percent_encoded() {
 
 #[test]
 fn encoding_matches_reqwests_urlencoded_unreserved_set() {
-    // reqwest's `.query()` delegates to serde_urlencoded, whose unreserved set
-    // is `[A-Za-z0-9*-._]` (space -> `+`). Regression coverage for the two
-    // characters where the previous hand-rolled encoder diverged:
-    //   `~` must be percent-encoded (`%7E`), and `*` must stay raw (`*`).
+    // Form encoding escapes `~` as `%7E` and leaves `*` raw.
     let pairs = serialize_query_value("q", &"~").unwrap();
     assert_eq!(serialize_query_pairs(&pairs).unwrap(), "q=%7E");
 

crates/identity/ras-identity-local/src/lib.rs

@@ -29,7 +29,7 @@ pub struct LocalUser {
     pub metadata: Option<serde_json::Value>,
 }
 
-/// Redacting `Debug` so the Argon2 `password_hash` never lands in logs (L1).
+/// Redacting `Debug` so the Argon2 `password_hash` never lands in logs.
 impl fmt::Debug for LocalUser {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         f.debug_struct("LocalUser")

crates/identity/ras-identity-oauth2/src/client.rs

@@ -52,7 +52,7 @@ impl OAuth2HttpTransport for ReqwestOAuth2HttpTransport {
 
         if !response.status().is_success() {
             // Never log or propagate the raw provider response body — it can
-            // contain tokens or other sensitive material (L1). Status only.
+            // contain tokens or other sensitive material. Status only.
             let status = response.status();
             error!("Token exchange failed with status {}", status);
             return Err(OAuth2Error::TokenExchangeFailed(format!(
@@ -84,7 +84,7 @@ impl OAuth2HttpTransport for ReqwestOAuth2HttpTransport {
             .map_err(log_upstream_error)?;
 
         if !response.status().is_success() {
-            // Status only; the raw body may echo the bearer token (L1).
+            // Status only; the raw body may echo the bearer token.
             let status = response.status();
             error!("User info request failed with status {}", status);
             return Err(OAuth2Error::UserInfoFailed(format!(
@@ -150,7 +150,7 @@ impl PkceChallenge {
 /// `provider_config.auth_params` nor caller-supplied `additional_params` may
 /// override them — many providers honour the last occurrence of a duplicated
 /// query parameter, so an injected second `redirect_uri` / `state` / PKCE value
-/// would be an authorization-code-theft or CSRF vector (H1).
+/// would be an authorization-code-theft or CSRF vector.
 const RESERVED_AUTH_PARAMS: &[&str] = &[
     "response_type",
     "client_id",
@@ -204,12 +204,11 @@ pub struct OAuth2Client {
 }
 
 impl OAuth2Client {
-    /// Infallible constructor.
+    /// Create a client with bounded HTTP timeouts.
     ///
-    /// Panics if the HTTP client cannot be built. It never silently falls back
-    /// to an unbounded (timeout-less) client — a hung token/userinfo endpoint
-    /// would otherwise stall the flow indefinitely (M6). Use [`Self::try_new`]
-    /// to handle the (near-impossible) build error yourself.
+    /// # Panics
+    /// Panics if the HTTP client cannot be built. Use [`Self::try_new`] to
+    /// handle construction errors.
     pub fn new(
         state_store: Arc<dyn OAuth2StateStore>,
         state_ttl_seconds: u64,
@@ -277,7 +276,7 @@ impl OAuth2Client {
         additional_params: HashMap<String, String>,
         binding: Option<String>,
     ) -> OAuth2Result<(String, String)> {
-        // Reject reserved-parameter overrides before doing any work (H1).
+        // Reject reserved-parameter overrides before doing any work.
         reject_reserved_params(provider_config.auth_params.keys(), "provider auth_params")?;
         reject_reserved_params(additional_params.keys(), "additional_params")?;
 
@@ -463,7 +462,7 @@ struct IdTokenClaims {
 }
 
 /// Subject (`sub`) claim of an id_token, used to bind it to the userinfo
-/// response so a confused-deputy userinfo cannot change the account (M6).
+/// response so a confused-deputy userinfo cannot change the account.
 pub(crate) fn id_token_subject(id_token: &str) -> OAuth2Result<Option<String>> {
     Ok(decode_id_token_claims(id_token)?.sub)
 }
@@ -480,8 +479,8 @@ fn decode_id_token_claims(id_token: &str) -> OAuth2Result<IdTokenClaims> {
         .map_err(|_| OAuth2Error::InvalidIdToken("invalid JSON payload".to_string()))
 }
 
-/// Validate the mandatory id_token claims: issuer (when configured),
-/// audience, expiry, and the nonce echoed from the authorization request.
+/// Validate the id_token issuer, audience, expiry, subject, and expected nonce.
+/// Accepting an id_token requires a configured provider issuer.
 ///
 /// The signature is not verified: the token was received directly from the
 /// token endpoint over TLS, which OIDC Core §3.1.3.7 permits as a substitute
@@ -515,7 +514,7 @@ pub(crate) fn validate_id_token_claims(
 
     // Issuer is fail-closed: an id_token whose issuer is unverified cannot be
     // trusted to identify the account, so accepting one without a configured
-    // `issuer` is refused rather than silently skipped (M6).
+    // `issuer` is refused rather than silently skipped.
     let Some(expected_issuer) = &provider_config.issuer else {
         return Err(OAuth2Error::InvalidIdToken(
             "provider `issuer` must be configured to accept id_tokens".to_string(),
@@ -566,7 +565,7 @@ pub(crate) fn validate_id_token_claims(
     }
 
     // `sub` is REQUIRED by OIDC Core §2. Refuse an id_token without it so the
-    // userinfo <-> id_token subject binding (M6) cannot silently no-op on a
+    // userinfo <-> id_token subject binding cannot silently no-op on a
     // token that carries no subject.
     match claims.sub.as_deref() {
         Some(sub) if !sub.trim().is_empty() => {}
@@ -1006,7 +1005,7 @@ mod tests {
         }));
         assert!(validate_id_token_claims(&config, &good, Some("nonce-1")).is_ok());
 
-        // An otherwise-valid id_token with no `sub` is rejected (M6): the
+        // An otherwise-valid id_token with no `sub` is rejected: the
         // userinfo binding must never run against an absent subject.
         let no_sub = fake_id_token(serde_json::json!({
             "iss": "https://issuer.test",
@@ -1025,7 +1024,7 @@ mod tests {
         }));
         assert!(validate_id_token_claims(&config, &aud_single_array, None).is_ok());
 
-        // Multi-audience token requires azp == client_id (M6).
+        // Multi-audience token requires azp == client_id.
         let aud_array_with_azp = fake_id_token(serde_json::json!({
             "iss": "https://issuer.test",
             "sub": "subject-1",
@@ -1076,7 +1075,7 @@ mod tests {
 
     #[test]
     fn id_token_without_configured_issuer_is_rejected() {
-        // issuer is None on provider_config() -> fail closed (M6).
+        // issuer is None on provider_config() -> fail closed.
         let config = provider_config();
         assert!(config.issuer.is_none());
         let exp = chrono::Utc::now().timestamp() + 600;

crates/identity/ras-identity-oauth2/src/config.rs

@@ -17,8 +17,8 @@ pub struct OAuth2ProviderConfig {
     pub token_endpoint: String,
     pub userinfo_endpoint: Option<String>,
     /// Expected `iss` claim of id_tokens returned by this provider
-    /// (e.g. `https://accounts.google.com`). When set, callbacks carrying
-    /// an id_token with a different issuer are rejected.
+    /// (e.g. `https://accounts.google.com`). Required when a callback returns
+    /// an id_token; missing or mismatched issuers are rejected.
     #[serde(default)]
     pub issuer: Option<String>,
     pub redirect_uri: String,
@@ -74,7 +74,7 @@ fn is_https(url: &str) -> bool {
         .is_some_and(|scheme| scheme.eq_ignore_ascii_case("https://"))
 }
 
-/// Manual `Debug` that redacts `client_secret` so it never lands in logs (L1).
+/// Manual `Debug` that redacts `client_secret` so it never lands in logs.
 impl fmt::Debug for OAuth2ProviderConfig {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         f.debug_struct("OAuth2ProviderConfig")