Skip to content

Commit ca04734

Browse files
yacosta738yacosta738
committed
feat(providers): emit Warning for 429 and no-key configurations
Refactor provider health_check() implementations to use the classify_status_code helper from rook_core. The new behavior: - OpenAI: differentiates 401/403/429/5xx/network; emits Warning for 429 rate-limited responses and no-key configurations. - Ollama: 2-step probe now emits Warning for 429 from the chat probe (uses chat probe latency, not total elapsed). - Gemini: implements real probe via GET /v1beta/models with x-goog-api-key header; replaces the Unknown placeholder. - Groq: implements real probe via GET /openai/v1/models; replaces the Unknown placeholder. - Anthropic: unchanged (still returns Unknown — no probe). This is PR 2 of the credential validation warning feature. PR 1 established the data shape (HealthStatus::Warning variant + TestConnectionResult wire shape) and dashboard binding. This PR lights up the new behavior end-to-end. Verified: cargo test (788 tests), vitest (110 tests), cargo clippy, cargo fmt, cargo doc, cargo audit. Refs: openspec/changes/2026-06-07-credential-validation-warning/
1 parent 50f6d03 commit ca04734

8 files changed

Lines changed: 873 additions & 70 deletions

File tree

‎crates/infrastructure/providers-gemini/src/lib.rs‎

Lines changed: 67 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,73 @@ impl ProviderPort for GeminiProvider {
9797
}
9898

9999
async fn health_check(&self) -> HealthStatus {
100-
HealthStatus::Unknown {
101-
provider: self.config.id.clone(),
102-
reason: "health_check_not_supported".to_string(),
100+
// No API key configured — surface as a yellow warning so the
101+
// user can still Save and add a key later via Edit.
102+
if self.config.api_key.is_empty() {
103+
return HealthStatus::Warning {
104+
provider: self.config.id.clone(),
105+
latency_ms: 0,
106+
reason: "No API key configured. You can add one later via Edit.".to_string(),
107+
};
108+
}
109+
110+
let start = std::time::Instant::now();
111+
let probe_url = format!("{}/v1beta/models", self.base_url());
112+
match self
113+
.client
114+
.get(&probe_url)
115+
.header("x-goog-api-key", &self.config.api_key)
116+
.send()
117+
.await
118+
{
119+
Ok(resp) => {
120+
let latency_ms = start.elapsed().as_millis() as u64;
121+
let status = resp.status();
122+
match rook_core::probes::classify_status_code(status.as_u16()) {
123+
rook_core::probes::ProbeClassification::Ok => HealthStatus::Healthy {
124+
provider: self.config.id.clone(),
125+
latency_ms,
126+
},
127+
rook_core::probes::ProbeClassification::RateLimited => HealthStatus::Warning {
128+
provider: self.config.id.clone(),
129+
latency_ms,
130+
reason: "Rate limited, but credentials are valid".to_string(),
131+
},
132+
rook_core::probes::ProbeClassification::AuthRejected(code) => {
133+
HealthStatus::Unhealthy {
134+
provider: self.config.id.clone(),
135+
latency_ms: Some(latency_ms),
136+
error: format!(
137+
"auth rejected: HTTP {code} — check that your API key is valid and has access to the model"
138+
),
139+
}
140+
}
141+
rook_core::probes::ProbeClassification::ServerError(code)
142+
| rook_core::probes::ProbeClassification::ClientError(code) => {
143+
HealthStatus::Unhealthy {
144+
provider: self.config.id.clone(),
145+
latency_ms: Some(latency_ms),
146+
error: format!("GET /v1beta/models returned HTTP {code}"),
147+
}
148+
}
149+
// Network errors are constructed by the Err arm below.
150+
rook_core::probes::ProbeClassification::NetworkError(_) => {
151+
HealthStatus::Unhealthy {
152+
provider: self.config.id.clone(),
153+
latency_ms: Some(latency_ms),
154+
error: "GET /v1beta/models returned an unknown status".to_string(),
155+
}
156+
}
157+
}
158+
}
159+
Err(e) => {
160+
let latency_ms = start.elapsed().as_millis() as u64;
161+
HealthStatus::Unhealthy {
162+
provider: self.config.id.clone(),
163+
latency_ms: Some(latency_ms),
164+
error: format!("GET /v1beta/models failed: {e}"),
165+
}
166+
}
103167
}
104168
}
105169

‎crates/infrastructure/providers-gemini/tests/provider.rs‎

Lines changed: 146 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,29 +2,78 @@ use providers_gemini::{GeminiProvider, GeminiProviderConfig};
22
use rook_core::{CompletionRequest, HealthStatus, ModelId, ProviderPort, Role};
33
use shared_kernel::{ProviderId, RequestId};
44

5+
// ---------------------------------------------------------------------------
6+
// health_check tests
7+
// ---------------------------------------------------------------------------
8+
59
#[tokio::test]
6-
async fn health_check_returns_unknown() {
7-
// Gemini provider's health_check is not implemented — it always returns Unknown
10+
async fn health_check_returns_healthy_on_2xx() {
11+
// Gemini's /v1beta/models endpoint accepts x-goog-api-key. A 200
12+
// response means credentials are valid and the model catalog is
13+
// available.
14+
let server = wiremock::MockServer::start().await;
15+
wiremock::Mock::given(wiremock::matchers::method("GET"))
16+
.and(wiremock::matchers::path("/v1beta/models"))
17+
.and(wiremock::matchers::header("x-goog-api-key", "test-key"))
18+
.respond_with(
19+
wiremock::ResponseTemplate::new(200).set_body_json(serde_json::json!({
20+
"models": [
21+
{ "name": "models/gemini-2.0-flash" }
22+
]
23+
})),
24+
)
25+
.mount(&server)
26+
.await;
27+
828
let provider = GeminiProvider::new(GeminiProviderConfig {
929
id: ProviderId::new("gemini-test"),
1030
api_key: "test-key".to_string(),
11-
base_url: None,
31+
base_url: Some(server.uri()),
1232
models: vec![ModelId::new("gemini-2.0-flash")],
1333
timeout_secs: 10,
1434
})
1535
.unwrap();
1636

1737
let status = provider.health_check().await;
18-
assert!(matches!(status, HealthStatus::Unknown { .. }));
38+
assert!(matches!(status, HealthStatus::Healthy { .. }));
1939
}
2040

2141
#[tokio::test]
22-
async fn health_check_is_unhealthy_on_error() {
23-
// Even with a bad response, health_check returns Unknown (not implemented)
42+
async fn health_check_returns_warning_on_429() {
2443
let server = wiremock::MockServer::start().await;
2544
wiremock::Mock::given(wiremock::matchers::method("GET"))
2645
.and(wiremock::matchers::path("/v1beta/models"))
27-
.respond_with(wiremock::ResponseTemplate::new(403))
46+
.respond_with(wiremock::ResponseTemplate::new(429))
47+
.mount(&server)
48+
.await;
49+
50+
let provider = GeminiProvider::new(GeminiProviderConfig {
51+
id: ProviderId::new("gemini-test"),
52+
api_key: "test-key".to_string(),
53+
base_url: Some(server.uri()),
54+
models: vec![ModelId::new("gemini-2.0-flash")],
55+
timeout_secs: 10,
56+
})
57+
.unwrap();
58+
59+
let status = provider.health_check().await;
60+
match status {
61+
HealthStatus::Warning { reason, .. } => {
62+
assert!(
63+
reason.to_lowercase().contains("rate limit"),
64+
"expected reason to mention rate limit, got: {reason}"
65+
);
66+
}
67+
other => panic!("expected Warning, got {other:?}"),
68+
}
69+
}
70+
71+
#[tokio::test]
72+
async fn health_check_returns_unhealthy_on_401() {
73+
let server = wiremock::MockServer::start().await;
74+
wiremock::Mock::given(wiremock::matchers::method("GET"))
75+
.and(wiremock::matchers::path("/v1beta/models"))
76+
.respond_with(wiremock::ResponseTemplate::new(401))
2877
.mount(&server)
2978
.await;
3079

@@ -38,8 +87,96 @@ async fn health_check_is_unhealthy_on_error() {
3887
.unwrap();
3988

4089
let status = provider.health_check().await;
41-
// health_check is not implemented, so it always returns Unknown
42-
assert!(matches!(status, HealthStatus::Unknown { .. }));
90+
match status {
91+
HealthStatus::Unhealthy { error, .. } => {
92+
assert!(
93+
error.contains("auth rejected") && error.contains("401"),
94+
"expected 'auth rejected' and '401' in error, got: {error}"
95+
);
96+
}
97+
other => panic!("expected Unhealthy, got {other:?}"),
98+
}
99+
}
100+
101+
#[tokio::test]
102+
async fn health_check_returns_unhealthy_on_500() {
103+
let server = wiremock::MockServer::start().await;
104+
wiremock::Mock::given(wiremock::matchers::method("GET"))
105+
.and(wiremock::matchers::path("/v1beta/models"))
106+
.respond_with(wiremock::ResponseTemplate::new(500))
107+
.mount(&server)
108+
.await;
109+
110+
let provider = GeminiProvider::new(GeminiProviderConfig {
111+
id: ProviderId::new("gemini-test"),
112+
api_key: "test-key".to_string(),
113+
base_url: Some(server.uri()),
114+
models: vec![ModelId::new("gemini-2.0-flash")],
115+
timeout_secs: 10,
116+
})
117+
.unwrap();
118+
119+
let status = provider.health_check().await;
120+
match status {
121+
HealthStatus::Unhealthy { error, .. } => {
122+
assert!(
123+
error.contains("500"),
124+
"expected '500' in error, got: {error}"
125+
);
126+
}
127+
other => panic!("expected Unhealthy, got {other:?}"),
128+
}
129+
}
130+
131+
#[tokio::test]
132+
async fn health_check_returns_unhealthy_on_network_error() {
133+
// Port 1 is reserved and refuses connections — network error path.
134+
let provider = GeminiProvider::new(GeminiProviderConfig {
135+
id: ProviderId::new("gemini-test"),
136+
api_key: "test-key".to_string(),
137+
base_url: Some("http://127.0.0.1:1".to_string()),
138+
models: vec![ModelId::new("gemini-2.0-flash")],
139+
timeout_secs: 2,
140+
})
141+
.unwrap();
142+
143+
let status = provider.health_check().await;
144+
match status {
145+
HealthStatus::Unhealthy { error, .. } => {
146+
assert!(
147+
error.contains("/v1beta/models") || error.contains("failed"),
148+
"expected error to mention the probe path or 'failed', got: {error}"
149+
);
150+
}
151+
other => panic!("expected Unhealthy, got {other:?}"),
152+
}
153+
}
154+
155+
#[tokio::test]
156+
async fn health_check_returns_warning_on_no_key() {
157+
// No API key configured — the probe must short-circuit before
158+
// touching the network.
159+
let server = wiremock::MockServer::start().await;
160+
// No wiremock routes mounted.
161+
let provider = GeminiProvider::new(GeminiProviderConfig {
162+
id: ProviderId::new("gemini-test"),
163+
api_key: String::new(),
164+
base_url: Some(server.uri()),
165+
models: vec![ModelId::new("gemini-2.0-flash")],
166+
timeout_secs: 10,
167+
})
168+
.unwrap();
169+
170+
let status = provider.health_check().await;
171+
match status {
172+
HealthStatus::Warning { reason, .. } => {
173+
assert!(
174+
reason.to_lowercase().contains("no api key"),
175+
"expected reason to mention no API key, got: {reason}"
176+
);
177+
}
178+
other => panic!("expected Warning, got {other:?}"),
179+
}
43180
}
44181

45182
#[tokio::test]

‎crates/infrastructure/providers-groq/src/lib.rs‎

Lines changed: 67 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -269,9 +269,73 @@ impl ProviderPort for GroqProvider {
269269
}
270270

271271
async fn health_check(&self) -> HealthStatus {
272-
HealthStatus::Unknown {
273-
provider: self.config.id.clone(),
274-
reason: "health_check_not_supported".to_string(),
272+
// No API key configured — surface as a yellow warning so the
273+
// user can still Save and add a key later via Edit.
274+
if self.config.api_key.is_empty() {
275+
return HealthStatus::Warning {
276+
provider: self.config.id.clone(),
277+
latency_ms: 0,
278+
reason: "No API key configured. You can add one later via Edit.".to_string(),
279+
};
280+
}
281+
282+
let start = std::time::Instant::now();
283+
let probe_url = format!("{}/openai/v1/models", self.config.base_url());
284+
match self
285+
.client
286+
.get(&probe_url)
287+
.header("Authorization", format!("Bearer {}", self.config.api_key))
288+
.send()
289+
.await
290+
{
291+
Ok(resp) => {
292+
let latency_ms = start.elapsed().as_millis() as u64;
293+
let status = resp.status();
294+
match rook_core::probes::classify_status_code(status.as_u16()) {
295+
rook_core::probes::ProbeClassification::Ok => HealthStatus::Healthy {
296+
provider: self.config.id.clone(),
297+
latency_ms,
298+
},
299+
rook_core::probes::ProbeClassification::RateLimited => HealthStatus::Warning {
300+
provider: self.config.id.clone(),
301+
latency_ms,
302+
reason: "Rate limited, but credentials are valid".to_string(),
303+
},
304+
rook_core::probes::ProbeClassification::AuthRejected(code) => {
305+
HealthStatus::Unhealthy {
306+
provider: self.config.id.clone(),
307+
latency_ms: Some(latency_ms),
308+
error: format!(
309+
"auth rejected: HTTP {code} — check that your API key is valid and has access to the model"
310+
),
311+
}
312+
}
313+
rook_core::probes::ProbeClassification::ServerError(code)
314+
| rook_core::probes::ProbeClassification::ClientError(code) => {
315+
HealthStatus::Unhealthy {
316+
provider: self.config.id.clone(),
317+
latency_ms: Some(latency_ms),
318+
error: format!("GET /openai/v1/models returned HTTP {code}"),
319+
}
320+
}
321+
// Network errors are constructed by the Err arm below.
322+
rook_core::probes::ProbeClassification::NetworkError(_) => {
323+
HealthStatus::Unhealthy {
324+
provider: self.config.id.clone(),
325+
latency_ms: Some(latency_ms),
326+
error: "GET /openai/v1/models returned an unknown status".to_string(),
327+
}
328+
}
329+
}
330+
}
331+
Err(e) => {
332+
let latency_ms = start.elapsed().as_millis() as u64;
333+
HealthStatus::Unhealthy {
334+
provider: self.config.id.clone(),
335+
latency_ms: Some(latency_ms),
336+
error: format!("GET /openai/v1/models failed: {e}"),
337+
}
338+
}
275339
}
276340
}
277341

0 commit comments

Comments
 (0)