refactor(rest-api-client): align client with rest-api-protocol and add typed ApiClient

docs/issues/open/1944-1938-si-6-align-rest-api-client.md

@@ -24,6 +24,16 @@ semantic-links:
 
 ## Subissue of REST API Contract-First Migration EPIC
 
+## Clarifying Decisions (from AI agent Q&A with user)
+
+- **`AddKeyForm`**: Use the protocol package's `AddKeyForm` (with field `opt_seconds_valid`) and remove the local `AddKeyForm` from client.
+- **`ClientError` enum variants**:
+  - `TransportError(reqwest::Error)` — network/connection failures
+  - `ApiError { status: StatusCode, body: String }` — non-2xx responses with the error body
+  - `DeserializationError(reqwest::Error)` — JSON parsing failures
+- **Public `get()` function**: Keep as a public free function (used by health_check tests directly).
+- **Re-export strategy**: Re-export both `ApiClient` and `ApiHttpClient` from the crate root for ergonomics.
+
 ## Problem
 
 The REST API client package (`torrust-tracker-rest-api-client`) currently exposes only a **low-level** `Client` struct where all 10 methods return raw `reqwest::Response` values. Callers must manually deserialize responses and handle errors. Some internal methods use `.unwrap()`, panicking on transport errors.
@@ -175,23 +185,23 @@ impl ApiClient {
 
 | ID  | Status | Task                                                         | Notes                                            |
 | --- | ------ | ------------------------------------------------------------ | ------------------------------------------------ |
-| T1  | TODO   | Rename `Client` → `ApiHttpClient` in `client.rs`             | Compiler catches all references                  |
-| T2  | TODO   | Add `rest-api-protocol` to `rest-api-client/Cargo.toml`      |                                                  |
-| T3  | TODO   | Define `ClientError` enum                                    | Wraps reqwest error, deserialization, API errors |
-| T4  | TODO   | Add `ApiClient` struct before `ApiHttpClient` in `client.rs` | New high-level typed client                      |
-| T5  | TODO   | Implement typed methods on `ApiClient` for all endpoints     | Returns `Result<DtoType, ClientError>`           |
-| T6  | TODO   | Verify pre-commit and pre-push checks pass                   |                                                  |
+| T1  | DONE   | Rename `Client` → `ApiHttpClient` in `client.rs`             | Compiler catches all references                  |
+| T2  | DONE   | Add `rest-api-protocol` to `rest-api-client/Cargo.toml`      |                                                  |
+| T3  | DONE   | Define `ClientError` enum                                    | Wraps reqwest error, deserialization, API errors |
+| T4  | DONE   | Add `ApiClient` struct before `ApiHttpClient` in `client.rs` | New high-level typed client                      |
+| T5  | DONE   | Implement typed methods on `ApiClient` for all endpoints     | Returns `Result<DtoType, ClientError>`           |
+| T6  | DONE   | Verify pre-commit and pre-push checks pass                   |                                                  |
 
 ## Verification / Progress
 
-- [ ] `Client` renamed to `ApiHttpClient` across the codebase
-- [ ] `rest-api-protocol` added as dependency
-- [ ] `ClientError` enum defined
-- [ ] `ApiClient` struct with typed methods for all endpoints added
-- [ ] `ApiClient` appears before `ApiHttpClient` in `client.rs`
-- [ ] All existing tests pass unchanged
-- [ ] Pre-commit checks pass
-- [ ] Pre-push checks pass
+- [x] `Client` renamed to `ApiHttpClient` across the codebase
+- [x] `rest-api-protocol` added as dependency
+- [x] `ClientError` enum defined
+- [x] `ApiClient` struct with typed methods for all endpoints added
+- [x] `ApiClient` appears before `ApiHttpClient` in `client.rs`
+- [x] All existing tests pass unchanged
+- [x] Pre-commit checks pass
+- [x] Pre-push checks pass
 
 ### Progress Log
 

docs/issues/open/1969-1938-si-8-eliminate-unwraps-from-rest-api-client.md

@@ -0,0 +1,101 @@
+---
+doc-type: spec
+issue-type: task
+status: planned
+priority: p2
+epic: 1938
+github-issue: 1969
+spec-path: docs/issues/open/1969-1938-si-8-eliminate-unwraps-from-rest-api-client.md
+last-updated-utc: 2026-06-30
+semantic-links:
+  skill-links:
+    - create-issue
+  related-artifacts:
+    - docs/issues/open/1938-rest-api-contract-first-migration/EPIC.md
+    - packages/rest-api-client/
+    - packages/rest-api-client/src/v1/client.rs
+---
+
+<!-- skill-link: create-issue -->
+
+# SI-8: Eliminate all unwraps from the REST API client package
+
+## Subissue of REST API Contract-First Migration EPIC
+
+## Goal
+
+Eliminate all `.unwrap()` calls from the `torrust-tracker-rest-api-client` package. Every operation that can fail must return a `Result`. For operations that are provably infallible, replace bare `.unwrap()` with an explicit `.expect("infallible: ...")` that documents why the operation cannot fail.
+
+## Background
+
+The `ApiClient` was made fully panic-free in SI-6 (PR #1968). However, the low-level `ApiHttpClient` and several free functions/helpers in `client.rs` still contain `.unwrap()` and `.expect()` calls that can panic at runtime.
+
+The calls fall into two categories:
+
+### Transport unwraps (must return `Result`)
+
+These are real failure points — network errors, URL parsing failures, etc. They must return `Result`:
+
+1. **11 public `ApiHttpClient` methods** — thin wrappers that delegate to fallible `*_result()` counterparts but `.unwrap()` the result.
+2. **`post_empty()`, `post_form()`** (private) — same wrapper-with-unwrap pattern.
+3. **`get()` (pub method on `ApiHttpClient`)** — same pattern.
+4. **`get()` (pub free function)** — thin wrapper around `get_result()`.
+5. **`get_request()` (pub on `ApiHttpClient`)** — calls `base_url()` which already returns `Result`.
+
+### Infallible conversions (replace `unwrap` with `expect`)
+
+These are provably infallible operations where a descriptive `expect` message is the right pattern:
+
+1. **`headers_with_request_id()`** — `Uuid::to_string()` always produces a valid ASCII string, and `HeaderValue::from_str()` for ASCII strings never fails.
+2. **`headers_with_auth_token()`** — same pattern, pre-formatted token string.
+3. **`get_request_with_query_result()` auth token inserts** — 2 token-to-HeaderValue conversions, same provably-infallible pattern.
+
+## Scope
+
+### In Scope
+
+- Change all panicking public `ApiHttpClient` methods to return `Result<Response, ClientError>` instead of `Response`.
+- Update all caller sites across the repository (contract tests, E2E tests, integration tests) to handle the new `Result` return types.
+- Change helper functions (`post_empty`, `post_form`, `get`, `get_request`, `get()`) to return `Result`.
+- Replace bare `.unwrap()` with `.expect("infallible: ...")` in `headers_with_request_id()`, `headers_with_auth_token()`, and `get_request_with_query_result()` auth token inserts.
+- Update issue spec and documentation.
+
+### Out of Scope
+
+- Changing the `ApiHttpClient`'s HTTP transport or connection model.
+- Adding retry/timeout policy (tracked separately).
+- Removing the `ApiClient`/`ApiHttpClient` two-tier architecture.
+
+## Implementation Plan
+
+| ID  | Status | Task                                                                                           | Notes                                                                    |
+| --- | ------ | ---------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------ |
+| T1  | TODO   | Move `ApiHttpClient` public methods to return `Result`                                         | 10 methods + `get()` method — return `ClientError`                       |
+| T2  | TODO   | Update all callers in contract tests (`packages/axum-rest-api-server/tests/`)                  | ~65 `ApiHttpClient::new(...)` call sites                                 |
+| T3  | TODO   | Update callers in `src/console/ci/qbittorrent_e2e/tracker/client.rs`                           | E2E test runner wrapper                                                  |
+| T4  | TODO   | Update callers in `tests/servers/api/contract/stats/mod.rs`                                    | Integration test                                                         |
+| T5  | TODO   | Replace bare `.unwrap()` with `.expect("infallible: ...")` for provably infallible conversions | `headers_with_request_id`, `headers_with_auth_token`, auth token inserts |
+| T6  | TODO   | Verify pre-commit and pre-push checks pass                                                     |                                                                          |
+
+## Verification / Progress
+
+- [ ] All `ApiHttpClient` public methods return `Result<Response, ClientError>`
+- [ ] No bare `.unwrap()` calls remain (only `.expect("infallible: ...")` for provably infallible operations)
+- [ ] All contract tests pass unchanged (except for updated `.unwrap()` calls on test side)
+- [ ] E2E tests compile
+- [ ] Pre-commit checks pass
+- [ ] Pre-push checks pass
+
+## Acceptance Criteria
+
+- `ApiHttpClient` never panics on transport/URL failures; all errors are returned as `ClientError`
+- Provably infallible conversions use `.expect("infallible: ...")` with a clear rationale
+- No regressions in existing tests
+- `linter all` passes
+
+### Progress Log
+
+| Date       | Event              |
+| ---------- | ------------------ |
+| 2026-06-30 | Spec drafted       |
+| 2026-06-30 | Spec moved to open |

packages/axum-rest-api-server/tests/server/v1/contract/authentication.rs

@@ -4,7 +4,7 @@ mod given_that_the_token_is_only_provided_in_the_authentication_header {
     use torrust_tracker_rest_api_client::common::http::Query;
     use torrust_tracker_rest_api_client::connection_info::ConnectionInfo;
     use torrust_tracker_rest_api_client::v1::client::{
-        AUTH_BEARER_TOKEN_HEADER_PREFIX, Client, headers_with_auth_token, headers_with_request_id,
+        AUTH_BEARER_TOKEN_HEADER_PREFIX, ApiHttpClient, headers_with_auth_token, headers_with_request_id,
     };
     use torrust_tracker_test_helpers::logging::logs_contains_a_line_with;
     use torrust_tracker_test_helpers::{configuration, logging};
@@ -20,7 +20,7 @@ mod given_that_the_token_is_only_provided_in_the_authentication_header {
 
         let token = env.get_connection_info().api_token.unwrap();
 
-        let response = Client::new(env.get_connection_info())
+        let response = ApiHttpClient::new(env.get_connection_info())
             .unwrap()
             .get_request_with_query("stats", Query::default(), Some(headers_with_auth_token(&token)))
             .await;
@@ -48,7 +48,7 @@ mod given_that_the_token_is_only_provided_in_the_authentication_header {
                 .expect("the auth token is not a valid header value"),
         );
 
-        let response = Client::new(env.get_connection_info())
+        let response = ApiHttpClient::new(env.get_connection_info())
             .unwrap()
             .get_request_with_query("stats", Query::default(), Some(headers))
             .await;
@@ -83,7 +83,7 @@ mod given_that_the_token_is_only_provided_in_the_authentication_header {
 
         let connection_info = ConnectionInfo::anonymous(env.get_connection_info().origin);
 
-        let response = Client::new(connection_info)
+        let response = ApiHttpClient::new(connection_info)
             .unwrap()
             .get_request_with_query("stats", Query::default(), Some(headers))
             .await;
@@ -103,7 +103,7 @@ mod given_that_the_token_is_only_provided_in_the_query_param {
     use torrust_tracker_axum_rest_api_server::testing::environment::Started;
     use torrust_tracker_rest_api_client::common::http::{Query, QueryParam};
     use torrust_tracker_rest_api_client::connection_info::ConnectionInfo;
-    use torrust_tracker_rest_api_client::v1::client::{Client, TOKEN_PARAM_NAME, headers_with_request_id};
+    use torrust_tracker_rest_api_client::v1::client::{ApiHttpClient, TOKEN_PARAM_NAME, headers_with_request_id};
     use torrust_tracker_test_helpers::logging::logs_contains_a_line_with;
     use torrust_tracker_test_helpers::{configuration, logging};
     use uuid::Uuid;
@@ -120,7 +120,7 @@ mod given_that_the_token_is_only_provided_in_the_query_param {
 
         let connection_info = ConnectionInfo::anonymous(env.get_connection_info().origin);
 
-        let response = Client::new(connection_info)
+        let response = ApiHttpClient::new(connection_info)
             .unwrap()
             .get_request_with_query(
                 "stats",
@@ -144,7 +144,7 @@ mod given_that_the_token_is_only_provided_in_the_query_param {
 
         let connection_info = ConnectionInfo::anonymous(env.get_connection_info().origin);
 
-        let response = Client::new(connection_info)
+        let response = ApiHttpClient::new(connection_info)
             .unwrap()
             .get_request_with_query(
                 "stats",
@@ -173,7 +173,7 @@ mod given_that_the_token_is_only_provided_in_the_query_param {
 
         let connection_info = ConnectionInfo::anonymous(env.get_connection_info().origin);
 
-        let response = Client::new(connection_info)
+        let response = ApiHttpClient::new(connection_info)
             .unwrap()
             .get_request_with_query(
                 "stats",
@@ -203,15 +203,15 @@ mod given_that_the_token_is_only_provided_in_the_query_param {
         let connection_info = ConnectionInfo::anonymous(env.get_connection_info().origin);
 
         // At the beginning of the query component
-        let response = Client::new(connection_info)
+        let response = ApiHttpClient::new(connection_info)
             .unwrap()
             .get_request(&format!("torrents?token={token}&limit=1"))
             .await;
 
         assert_eq!(response.status(), 200);
 
         // At the end of the query component
-        let response = Client::new(env.get_connection_info())
+        let response = ApiHttpClient::new(env.get_connection_info())
             .unwrap()
             .get_request(&format!("torrents?limit=1&token={token}"))
             .await;
@@ -227,7 +227,7 @@ mod given_that_not_token_is_provided {
     use torrust_tracker_axum_rest_api_server::testing::environment::Started;
     use torrust_tracker_rest_api_client::common::http::Query;
     use torrust_tracker_rest_api_client::connection_info::ConnectionInfo;
-    use torrust_tracker_rest_api_client::v1::client::{Client, headers_with_request_id};
+    use torrust_tracker_rest_api_client::v1::client::{ApiHttpClient, headers_with_request_id};
     use torrust_tracker_test_helpers::logging::logs_contains_a_line_with;
     use torrust_tracker_test_helpers::{configuration, logging};
     use uuid::Uuid;
@@ -244,7 +244,7 @@ mod given_that_not_token_is_provided {
 
         let connection_info = ConnectionInfo::anonymous(env.get_connection_info().origin);
 
-        let response = Client::new(connection_info)
+        let response = ApiHttpClient::new(connection_info)
             .unwrap()
             .get_request_with_query("stats", Query::default(), Some(headers_with_request_id(request_id)))
             .await;
@@ -263,7 +263,7 @@ mod given_that_not_token_is_provided {
 mod given_that_token_is_provided_via_get_param_and_authentication_header {
     use torrust_tracker_axum_rest_api_server::testing::environment::Started;
     use torrust_tracker_rest_api_client::common::http::{Query, QueryParam};
-    use torrust_tracker_rest_api_client::v1::client::{Client, TOKEN_PARAM_NAME, headers_with_auth_token};
+    use torrust_tracker_rest_api_client::v1::client::{ApiHttpClient, TOKEN_PARAM_NAME, headers_with_auth_token};
     use torrust_tracker_test_helpers::{configuration, logging};
 
     #[tokio::test]
@@ -276,7 +276,7 @@ mod given_that_token_is_provided_via_get_param_and_authentication_header {
 
         let non_authorized_token = "NonAuthorizedToken";
 
-        let response = Client::new(env.get_connection_info())
+        let response = ApiHttpClient::new(env.get_connection_info())
             .unwrap()
             .get_request_with_query(
                 "stats",